Skip to content

ci: add zizmor workflow security scanner#3506

Open
nicktrn wants to merge 7 commits intomainfrom
ci/zizmor
Open

ci: add zizmor workflow security scanner#3506
nicktrn wants to merge 7 commits intomainfrom
ci/zizmor

Conversation

@nicktrn
Copy link
Copy Markdown
Collaborator

@nicktrn nicktrn commented May 1, 2026

Adds zizmor alongside the actionlint job from #3503. Both now run as parallel jobs in a single .github/workflows/workflow-checks.yml, triggered on .github/workflows/** and .github/actions/** changes.

Zizmor is configured with unpinned-uses: hash-pin policy via .github/zizmor.yml, so any future unpinned action will fail CI. Findings upload SARIF to the Security tab alongside CodeQL.

Bulk of the diff is cleanup of the findings zizmor surfaced on first run. zizmor --fix=all handled most of them mechanically; the rest were judgment calls.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 1, 2026

⚠️ No Changeset found

Latest commit: 3662c15

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 1, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f246dd17-f549-4d70-ad85-b781044a85f3

📥 Commits

Reviewing files that changed from the base of the PR and between 3722760 and 3662c15.

📒 Files selected for processing (24)
  • .github/actions/get-image-tag/action.yml
  • .github/workflows/changesets-pr.yml
  • .github/workflows/claude-md-audit.yml
  • .github/workflows/claude.yml
  • .github/workflows/docs.yml
  • .github/workflows/e2e-webapp.yml
  • .github/workflows/e2e.yml
  • .github/workflows/helm-prerelease.yml
  • .github/workflows/pr_checks.yml
  • .github/workflows/publish-webapp.yml
  • .github/workflows/publish-worker-v4.yml
  • .github/workflows/publish-worker.yml
  • .github/workflows/publish.yml
  • .github/workflows/release-helm.yml
  • .github/workflows/release.yml
  • .github/workflows/sdk-compat.yml
  • .github/workflows/typecheck.yml
  • .github/workflows/unit-tests-internal.yml
  • .github/workflows/unit-tests-packages.yml
  • .github/workflows/unit-tests-webapp.yml
  • .github/workflows/unit-tests.yml
  • .github/workflows/vouch-check-pr.yml
  • .github/workflows/workflow-checks.yml
  • .github/zizmor.yml
✅ Files skipped from review due to trivial changes (14)
  • .github/workflows/claude.yml
  • .github/workflows/typecheck.yml
  • .github/zizmor.yml
  • .github/workflows/changesets-pr.yml
  • .github/workflows/sdk-compat.yml
  • .github/workflows/claude-md-audit.yml
  • .github/workflows/unit-tests.yml
  • .github/workflows/e2e.yml
  • .github/workflows/publish-worker.yml
  • .github/actions/get-image-tag/action.yml
  • .github/workflows/unit-tests-webapp.yml
  • .github/workflows/release-helm.yml
  • .github/workflows/helm-prerelease.yml
  • .github/workflows/publish.yml
🚧 Files skipped from review as they are similar to previous changes (4)
  • .github/workflows/unit-tests-internal.yml
  • .github/workflows/docs.yml
  • .github/workflows/vouch-check-pr.yml
  • .github/workflows/unit-tests-packages.yml
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / e2e-webapp / 🧪 E2E Tests: Webapp
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: sdk-compat / Bun Runtime
  • GitHub Check: sdk-compat / Cloudflare Workers
  • GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
  • GitHub Check: sdk-compat / Deno Runtime
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (python)
🔇 Additional comments (23)
.github/workflows/publish-worker-v4.yml (2)

44-45: LGTM!

Adding persist-credentials: false is a security best practice that prevents the checkout action from persisting Git credentials, reducing the attack surface if subsequent steps are compromised.


65-81: LGTM!

Refactoring from inline ${{ steps.*.outputs.* }} interpolation to environment variables is the correct fix for zizmor's script injection warnings. The env var mappings correctly reference:

  • get_repository.outputs.repo for the image repository path
  • get_tag.outputs.tag and get_tag.outputs.is_semver from the composite action

This pattern prevents potential command injection if any of these outputs were ever to contain malicious content.

.github/workflows/publish-webapp.yml (6)

16-18: LGTM!

Declaring the SENTRY_AUTH_TOKEN secret explicitly with required: false is the correct approach for making secrets: inherit explicit while maintaining backward compatibility for callers that don't have Sentry configured.


32-36: LGTM!

Adding persist-credentials: false alongside the existing submodules: recursive is a good security hardening measure.


44-47: LGTM!

Using the built-in GITHUB_SHA environment variable directly in the shell script is cleaner and avoids the ${{ github.sha }} interpolation that zizmor flags for potential injection risks.


49-64: LGTM!

The refactoring correctly maps step outputs to environment variables, preventing script injection while maintaining the same logic for semver detection and tag assembly.


66-80: LGTM!

Good use of built-in environment variables (GITHUB_SHA, GITHUB_REF_NAME) and step output mapping via the env: block. The conditional BUILD_APP_VERSION output for semver tags is preserved correctly.


104-105: LGTM!

Using Docker's secrets: mechanism rather than build-args: for the Sentry auth token is the correct approach—this ensures the token is mounted as a secret file and not baked into image layers or visible in build logs.

.github/workflows/release.yml (9)

33-47: LGTM!

Good security hardening. The permissions: {} correctly restricts token permissions for a job that only writes to the step summary and doesn't need any GitHub API access.


68-82: LGTM!

The zizmor ignore comment correctly documents why persist-credentials: false cannot be used here (the job needs to push tags). Using env variable injection instead of direct ${{ }} interpolation in the shell command prevents potential injection attacks.


121-128: LGTM!

Correctly passes the JSON output via env var and uses jq for parsing. This is the safe pattern for handling structured data in shell.


130-161: LGTM!

Consistent application of the env var pattern for the version output across release creation and tag pushing steps.


164-178: LGTM!

The explicit secrets mapping is more secure and auditable than secrets: inherit. The ${{ }} expression in with: is safe since it's passed as an action input, not interpolated into a shell command.


196-234: LGTM!

The version is safely injected via env var and used consistently throughout the shell script.


238-250: LGTM!

The permissions: {} is correct since this job uses a PAT (CROSS_REPO_PAT) rather than GITHUB_TOKEN for the cross-repo dispatch.


252-267: LGTM!

Correctly adds persist-credentials: false since the prerelease job only publishes to npm and doesn't need to push any tags or commits.


291-307: LGTM!

The prerelease tag is safely passed via env var, consistent with the security pattern applied throughout the workflow.

.github/workflows/workflow-checks.yml (2)

1-14: LGTM — name and path-trigger updates are correct.

Renaming to "Workflow Checks" accurately reflects the expanded scope, and adding .github/zizmor.yml to both path filters ensures config changes retrigger both jobs.


37-51: LGTM — implementation matches the official zizmor integration pattern exactly.

The commit hash b1d7e1f (full: b1d7e1fb5de872772f31590499237e7cce841e8e) is confirmed as the v0.5.3 release commit, so the hash-pin is correct. The permissions and checkout configuration mirror the official integration docs exactly — security-events: write for SARIF upload, contents: read to clone the repo, and actions: read for upload-sarif to read workflow run info.

.github/workflows/pr_checks.yml (2)

25-27: LGTM — explicit DOCKERHUB_* passing for units is correct.

The called unit-tests.yml (and its sub-workflows) declare both secrets as required: false, so this aligns perfectly.


29-35: The concern raised in this review comment is not supported by the workflow files. Neither e2e.yml nor sdk-compat.yml declares any secrets in their on.workflow_call configuration, and neither contains Docker login steps or any explicit credential usage. Both workflows rely only on public GitHub Actions (checkout, setup-node, setup-pnpm, setup-bun, setup-deno, etc.) and standard development tools, which do not require inherited secrets. The removal of secrets: inherit is correct and poses no risk to these jobs.

			> Likely an incorrect or invalid review comment.
.github/workflows/e2e-webapp.yml (2)

8-12: LGTM — secret declarations correctly match the call chain.

All callers (unit-tests.yml, pr_checks.yml, publish.yml) pass both secrets explicitly, and the graceful env.DOCKERHUB_USERNAME guards on lines 67 and 77 handle the absent-secret case cleanly.


52-52: LGTM — persist-credentials: false is the right call here.

No subsequent steps require persisted git credentials, so disabling them reduces the blast radius if a later step is compromised.


Walkthrough

This PR updates many GitHub Actions workflows and one composite action. Changes include adding persist-credentials: false to multiple actions/checkout steps, converting secrets: inherit to explicit on.workflow_call secret declarations or per-job secret mappings (adding optional DockerHub/Sentry secrets in several workflows), tightening top-level permissions and moving scopes to job-level, replacing direct ${{ ... }} interpolation inside shell steps with injected env variables (and wiring step outputs into env), refactoring one composite action to read inputs via env, and adding .github/zizmor.yml with a hash-pin requirement.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely incomplete, missing the required checklist, testing section, and changelog section specified in the repository template. Complete the PR description by adding the required checklist section, testing steps, and changelog description as specified in the contributing guidelines template.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'ci: add zizmor workflow security scanner' clearly and concisely summarizes the main change—adding Zizmor as a workflow security scanner to the CI pipeline.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/zizmor

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-advanced-security
Copy link
Copy Markdown
Contributor

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

coderabbitai[bot]

This comment was marked as resolved.

@nicktrn
Copy link
Copy Markdown
Collaborator Author

nicktrn commented May 2, 2026

ready 🔒

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants