Skip to content

Conversation

mandreko-bitwarden
Copy link
Contributor

🎟️ Tracking

https://bitwarden.atlassian.net/browse/VULN-285

📔 Objective

Implement Zizmor, similar to ActionLint to perform security checks on GitHub workflow files.

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

Copy link

github-actions bot commented Sep 30, 2025

Logo
Checkmarx One – Scan Summary & Details99634d80-d8aa-4856-8eac-49c0730db743

Great job! No new security vulnerabilities introduced in this pull request

@mandreko-bitwarden mandreko-bitwarden marked this pull request as ready for review October 1, 2025 13:04
@mandreko-bitwarden mandreko-bitwarden requested a review from a team as a code owner October 1, 2025 13:04
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR integrates Zizmor, a security-focused GitHub workflow linter, into the bitwarden workflow linter (bwwl) to perform security checks on GitHub workflow files similar to how ActionLint is currently integrated.

  • Adds a new RunZizmor rule with configurable installation and execution
  • Implements comprehensive test coverage for the new functionality
  • Updates configuration files to enable the new rule with appropriate settings

Reviewed Changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
zizmor.yml Configuration file for zizmor with pinning policies and disabled dangerous triggers
tests/rules/test_run_zizmor.py Comprehensive test suite covering installation, execution, and error scenarios
src/bitwarden_workflow_linter/zizmor_version.yaml Version specification file for zizmor dependency
src/bitwarden_workflow_linter/utils.py Settings class updates to support zizmor version and config URL
src/bitwarden_workflow_linter/rules/run_zizmor.py Main rule implementation with installation, validation, and execution logic
src/bitwarden_workflow_linter/default_settings.yaml Default configuration enabling zizmor rule at warning level
settings.yaml Local settings with commented zizmor rule and config URL
Taskfile.yml Task runner updates with zizmor installation and execution tasks
.github/workflows/examples/*.yaml Example workflow files updated with zizmor-compatible security practices

Comment on lines +157 to +166
# Handle both dict[str, dict] and dict[str, Action]
# for approved_actions for testing help
self.approved_actions = {}
for name, action in approved_actions.items():
if isinstance(action, Action):
# Already an Action object, use it directly
self.approved_actions[name] = action
else:
# Dictionary, create Action object
self.approved_actions[name] = Action(**action)
Copy link
Preview

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change appears to be unrelated to adding Zizmor support and should be in a separate commit. The comment "for testing help" suggests this is a workaround rather than a proper solution.

Suggested change
# Handle both dict[str, dict] and dict[str, Action]
# for approved_actions for testing help
self.approved_actions = {}
for name, action in approved_actions.items():
if isinstance(action, Action):
# Already an Action object, use it directly
self.approved_actions[name] = action
else:
# Dictionary, create Action object
self.approved_actions[name] = Action(**action)
self.approved_actions = approved_actions

Copilot uses AI. Check for mistakes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be a hack? I had to modify it to allow the copying of a mock value. I'm unsure.

Copy link
Member

@vgrassia vgrassia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tabs should be set to 2 spaces to be consistent with other workflows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants