Repository navigation
root - chore: defense - lint workflows with zizmor - #168
Conversation
The new workflow runs zizmor on every pull request. zizmor fails while checkouts persist credentials. zizmor also fails while the release job caches pnpm. Those settings are now fixed. Co-authored-by: Jared Wray <me@jaredwray.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The checklist names the open pull request. Co-authored-by: Jared Wray <me@jaredwray.com>
|
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:
For more information about GitHub Code Scanning, check out the documentation. |
There was a problem hiding this comment.
Code Review
This pull request updates the DEFENSE_IN_DEPTH.md checklist to mark the Socket Firewall integration as completed and labels several other security items as pending. A review comment correctly points out a discrepancy: the PR description mentions adding the zizmor workflow lint, but the corresponding workflow file is missing from the changes.
| - [ ] Every job installs Socket Firewall (`SocketDev/action` SHA-pinned, `firewall-version` pinned); `pnpm install` / `npm install` run as `sfw pnpm install` / `sfw npm install` (PR #167 pending) | ||
| - [ ] `.github/workflows/check-workflows.yaml` lints workflows with zizmor on every PR | ||
| - [x] Every job installs Socket Firewall (`SocketDev/action` SHA-pinned, `firewall-version` pinned); `pnpm install` / `npm install` run as `sfw pnpm install` / `sfw npm install` — PR #167 | ||
| - [ ] `.github/workflows/check-workflows.yaml` lints workflows with zizmor on every PR (PR #168 pending) |
There was a problem hiding this comment.
The pull request title and description state that this PR adds the zizmor workflow lint (.github/workflows/check-workflows.yaml), but the actual workflow file is missing from the changes in this pull request. Please include the workflow file in this PR, or clarify if this tracking document update is intended to be submitted separately.
There was a problem hiding this comment.
The workflow file is in commit 278d8ea. .github/workflows/check-workflows.yaml runs zizmor on every pull request. The zizmor check passed on commit 04e7545. DEFENSE_IN_DEPTH.md line 32 stays open until this pull request merges.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #168 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 9 9
Lines 639 639
Branches 146 151 +5
=========================================
Hits 639 639 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 278d8ea043
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| uses: zizmorcore/zizmor-action@cc914d7f3750a2d13d75c7f184a1060aa0e9d482 # v0.6.4 | ||
| with: | ||
| # Fork PRs cannot upload SARIF (read-only token). Lint with annotations instead. | ||
| advanced-security: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} |
There was a problem hiding this comment.
Make zizmor fail when a pull request has findings
On same-repository pull requests, this expression enables Advanced Security. The action documentation states that this mode does not fail when zizmor reports findings. Thus, an unsafe workflow can pass this check unless separate code-scanning merge protection exists. Disable Advanced Security for pull requests, or configure that protection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The finding is correct. zizmor --format=sarif exits 0 when findings exist, so Advanced Security does not fail the job. Commit dd78f6a sets advanced-security to false on a pull request and enables annotations. The annotation format exits non-zero when findings exist. A push still uploads SARIF.
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| actions: read |
There was a problem hiding this comment.
Replace the comment fragment with a complete sentence
The new SARIF upload. comment has no verb. Replace the fragment with a complete descriptive sentence, as the repository requires.
AGENTS.md reference: AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The finding is correct. The comment had no verb. Commit dd78f6a replaces it with complete sentences. The SARIF upload needs the permission. GitHub ignores the write grant on a fork pull request.
A pull request sets advanced-security to false. Annotations make the job fail when zizmor reports findings. A push still uploads SARIF. Co-authored-by: Jared Wray <me@jaredwray.com>
Summary
This pull request adds a zizmor workflow lint in section 4.
Status
DEFENSE_IN_DEPTH.md: zizmor, checkout credentials, and the release cache → (PR #168 pending). Socket Firewall → PR #167.Verification
.github/workflows/check-workflows.yamlruns on pull requests and on pushes tomain.npx actions-up --dry-runreports no updates.zizmor1.30.1 reports no findings on the workflows.package-manager-cache: false. A pnpm cache there makeszizmorfail.persist-credentials: false. A persisted credential makeszizmorfail.dd78f6a.Notes
zizmorexits with findings on the workflows before this change. The release cache is a high finding. Credential persistence is a low finding. This pull request fixes both so the new check can pass.zizmor --format=sarifexits 0 when findings exist. A pull request therefore uses annotations.Reference
defense-in-depth-nodejs § 4