Harden CI: run the full security-audit scanner (workflows, package-lock, Python source) - #5733
Conversation
Both private repos in this family run zizmor over their workflows and composite actions on every PR and nightly (clawmetry-cloud security-audit.yml, clawmetry-pro #234). This repo -- 37 workflows and a composite action, the most of the three, and the only public one -- was running no Actions-security scanner at all. The Scorecard job in this same workflow covers part of the ground (Dangerous-Workflow, Token-Permissions), but it is `if: github.event_name != 'pull_request'`, so it never runs on the PR that introduces a problem, and it grades the repository rather than naming a file and a line. Ported from the clawmetry-cloud step, including the discipline that step was built around: a scan that covered nothing must not read as a clean result. - `--no-exit-codes` so FINDINGS exit 0. Without it a findings exit and a crash are the same non-zero, and the usual `|| true` launders a crash -- which writes no json at all -- into an empty file that renders as clean. - Audit composite actions too, named individually. A composite action's steps run inline in the calling job with that job's token and secrets, so it carries the same rule families; naming them individually rather than passing the directory keeps a stray non-action YAML from aborting the audit later. - Record the input list and count. A finding list is identical whether it audited 38 files or none, so coverage is the one thing the report cannot tell you afterwards. - Fail the job ONLY on a scanner outage (no usable report, or an empty input set). Findings are reported in the step summary and the artifact, not gated. Findings are deliberately non-blocking. Today's tree has 42 findings, 5 of them High, and the notable ones are load-bearing by design: the release, auto-quarantine and cloud-pin jobs persist their checkout credential because they push with it, and the `workflow_run` triggers are how those workflows are supposed to fire. A gate that failed on those would be switched off within a week. The summary table is what makes a NEW finding visible in the PR that adds it. Test plan - yaml.safe_load over all 38 workflow files -> parse - bash -n on all three new run blocks -> parse; embedded Python compiles - Ran the audit step against this repo: discovers 38 inputs (37 workflows + setup-openclaw), writes a 42-finding report, exit 0, gate green. Summary renders 5 High / 3 Medium / 9 Low / 25 Informational, broken down by rule. - Negative test, scanner outage: stub zizmor exiting 2 with no json. Step still exits 0 (recorded, not raised) so the artifact uploads, summary renders SCAN FAILED, and the gate fails the job. No-PRD: CI-only change, confined to .github/workflows/. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019q6JAQni1nbx9pqSVA5oj8
|
![8090-software-factory[bot]](https://avatars.githubusercontent.com/in/1847834?s=60&v=4){kind=small}
| # Named individually rather than by directory — pointing zizmor at a | ||
| # directory hands it every YAML inside, and a non-action YAML landing | ||
| # there later would abort the audit rather than be skipped. | ||
| inputs=".github/workflows" |
There was a problem hiding this comment.
The SecurityAuditScanner component specifies scanning "workflow definitions themselves," but the implementation also scans composite actions under .github/actions/. Composite action scanning is not documented in the blueprint.
…ribes Drift Bot flagged the composite-action half of this change: the SecurityAuditScanner component describes scanning "workflow definitions themselves", and .github/actions/ is a scope no product record covers. That is a fair finding, and it applies to a separable slice rather than to the change as a whole -- so this narrows the input set instead of arguing with the gate. Coverage goes from 0 files to 37, which is the whole point of the PR; the composite action is left explicitly uncovered, with a comment saying so and why, rather than silently dropped. Widening the input set to include it is one line, and it is worth doing -- its steps run inline in the calling job with that job's token and secrets, so nothing scans the one place a workflow's privileges are borrowed. But that is a scope change that should arrive with its product record, not as a side effect of turning the scanner on. Baseline moves with the scope: 37 inputs, 40 findings (3 High / 3 Medium / 9 Low / 25 Informational). The two High `github-env` findings were in the composite action and are no longer reported. Re-verified after the change - yaml.safe_load over all 37 workflow files -> parse - bash -n on all three run blocks -> parse - Audit step against this repo: 37 inputs, 40-finding report, exit 0, gate green, summary renders the tables above - Negative test, scanner outage: stub zizmor exiting 2 with no json -> step exits 0, zizmor.failed written, gate exits 1 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019q6JAQni1nbx9pqSVA5oj8
Narrowed to workflow definitions, per the Drift Bot finding
Worth saying why this one was narrowed rather than argued, since two sibling hardening PRs have taken the other path and are still red — #5305 (28 Aug) and #5382 (30 Aug), each blocked on the same shape of finding: the blueprint does not describe the constraint this PR adds. That is close to structural for hardening work, because a hardening change exists precisely to add a constraint the blueprint predates. The difference here is that the finding lands on a separable slice, not on the change as a whole. Coverage still goes from 0 files to 37, which is the whole point of the PR. The composite action stays uncovered, with a comment in the workflow saying so and why, rather than being silently dropped — it is a real gap (its steps run inline in the calling job, with that job's token and secrets, so it is the one place a workflow's privileges are borrowed and nothing scans it), and widening the input set is a one-line follow-up that should arrive with its product record. Baseline moves with the scope: 37 inputs, 40 findings — 3 High / 3 Medium / 9 Low / 25 Informational. The two High Re-verified after the change: all 37 workflow files I'll keep this PR watched until it is green and mergeable. Generated by Claude Code |
|
Not making a second scope change for this: the new finding is factually wrong on both countsRound 2 from Both of the "missing" two are already implemented on this repo, and I checked each before writing this: 1. Python/package dependency auditing — implemented, in this same workflow file. The 2. Python source static analysis — implemented, via CodeQL. So the finding is measuring one new job against a component that describes the whole scanner, and missing two sibling implementations — one in the same file, one outside any workflow file. The gap the component describes is already closed; this PR closes the one part of it that genuinely was not. Why I am not adding bandit here to clear itI already narrowed this PR once at this gate's request ( Where that leaves this PRBlocked on Everything else on this PR is green or running, and the change is verified independently of the gate: all 37 workflow files parse, I will keep this PR watched until it is green, merged, or closed, and I am not pushing further scope changes to chase this gate. Generated by Claude Code |
|
✨ auto-fixed: merged main into branch to clear cancelled Ubuntu CI jobs (queue-priority workflow cancelled them when main moved). Fresh CI triggered. Generated by Claude Code |
|
|
blocked on author decision — skipping (auto-mergeability sweep) E2E Gate (required) fails because the Generated by Claude Code |
|
|
blocked on author decision — skipping (auto-mergeability sweep) Generated by Claude Code |
|
|
✨ auto-fixed: merged latest main into branch to keep it up to date Generated by Claude Code |
|
|
✨ auto-fixed: merged latest main into branch (was BEHIND; no conflicts) Generated by Claude Code |
Drift Bot flagged this PR against the SecurityAuditScanner component, which
describes three scanning responsibilities: workflow definitions, dependency
manifests, and Python source. The job as opened implemented only the first.
This adds the two that were missing, in the same job:
- `npm audit --package-lock-only` over every committed package-lock.json
(frontend, tests/e2e, .github/scripts, .github/claude-cli,
.github/ci-node/auth-bootstrap, .github/actions/setup-openclaw).
pip-audit already covers requirements.txt in the `python-deps` job, but
nothing audited the npm half -- and frontend/ is the repo's largest
dependency tree, shipping into clawmetry/static/.
- `bandit -ll` over the Python source, excluding tests/ and vendored trees.
pip-audit deliberately stays in `python-deps`: it has its own SBOM artifact
and its own red condition, and splitting a passing job to tidy the layout is
churn, not hardening.
Both new scanners follow the discipline the zizmor step already sets. Their
exit codes cannot distinguish a finding from an outage -- bandit exits 1 on
findings, `npm audit` exits non-zero on both -- so neither is trusted. The
shape of the report is the test instead: bandit must write a `results` list,
npm must write `.metadata.vulnerabilities.total`, and anything else records a
`.failed` marker. Findings stay reported, not gated; the only red condition is
still a scanner that did not run, because a silent outage reads exactly like a
clean scan.
The job summary becomes one table across all three scanners, plus a
severity/confidence breakdown for bandit -- on a tree this size the total
alone does not say whether anything in it needs reading today.
Renamed to "Security audit (workflows, package-lock, Python source)" to match
what it now does. The check is not a required E2E Gate leg
(scripts/e2e_gate.py), so the rename moves no branch-protection context.
Verified locally by replaying each `run:` block under `bash -e` as Actions
executes them: bandit 280 findings in 24s (3 HIGH), npm audit clean on five of
six lockfiles, gate green when healthy and red on a simulated outage, and all
38 workflow files parse.
No-PRD: CI-only change under .github/, exempt from the product-record gate.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgwaB1b9mJUUYJJRKybcqe
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
vivekchand
changed the title
The scanner job gained bandit and npm-audit in 5e9de09, but no actions/setup-node. `npm audit` then runs against whatever Node the runner image happens to ship. That is not a hypothetical. clawmetry-pro#234 hit exactly this: with no Node of its own, the step ran on the image's npm and an old npm 6 returned a 400 from the retired `audits/quick` endpoint. The fix there was to pin setup-node; this job needs the same pin for the same reason. It matters more here than it did there. On pro the npm findings were non-blocking, so a bad npm degraded the report. Here the job's gate treats an npm-audit outage as RED -- by design, so a scan that covered nothing cannot read as clean -- which means a runner image bump could turn `main` red with nothing in the diff to explain it. - Pin actions/setup-node to the v7.0.0 commit already used by nine other workflows in this repo, on Node 20. - `package-manager-cache: false`. A GitHub Actions cache is not trusted input and this job reads lockfiles to decide whether the tree is vulnerable. `npm audit --package-lock-only` installs nothing, so the cache buys nothing and could only muddy the result. Same reasoning as publish.yml. Verified - yaml.safe_load over all 38 workflow files -> parse - tests/test_workflow_yaml_valid.py -> 550 passed, 344 skipped - Anchored `uses:` scan -> 0 unpinned action references - bash -n on all five run blocks -> parse - Full job run against this repo: zizmor 42 findings over 38 workflow files, bandit 280, npm audit over all 6 committed lockfiles (5 vulnerabilities in .github/actions/setup-openclaw, 0 elsewhere), summary renders every scanner, gate green - Negative test, npm registry outage: stub npm returning a 400 error body. Step still exits 0 so the artifact uploads, npm-audit.failed is written, and the gate exits 1 rather than reporting a clean scan No-PRD: CI-only change, confined to .github/workflows/. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019q6JAQni1nbx9pqSVA5oj8
Scope changed under me, and one defect in it is fixedFor an accurate record, since my previous comment said I was not pushing further scope changes and the diff now contains one:
The defect I found and fixed —
|
| Scanner | Scope | Findings |
|---|---|---|
| zizmor | 38 workflow definitions | 42 |
| npm audit | 6 committed lockfiles | 5, all in .github/actions/setup-openclaw |
| bandit | Python source, medium+ | 280 |
Two things worth a look on their own merits, neither blocking here: the 5 npm vulnerabilities are in a lockfile CI installs from on every E2E run, and bandit's highest-confidence findings are 2× B324 (hashlib) and 1× B201 (flask_debug_true) — the last of which is the same surface #5382 is trying to harden.
Verification
yaml.safe_load over all 38 workflow files; tests/test_workflow_yaml_valid.py → 550 passed, 344 skipped; anchored uses: scan → 0 unpinned refs; bash -n on all five run blocks; a full job run producing the table above with a green gate; and a negative test stubbing an npm registry 400 → step exits 0 so the artifact still uploads, npm-audit.failed is written, and the gate exits 1 rather than reporting a clean scan.
The drift-bot position is unchanged and I have not touched it: its round-2 claims remain false on the facts, since python-deps runs pip-audit --strict in this same workflow and CodeQL Analyze (python) runs on every PR.
Generated by Claude Code
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
Green. And the thing I argued against is what cleared the gate.
The scanner job ran on a real runner with all three scanners for the first time, and matched the local runs exactly: All 15 steps green, including the Worth recording plainlyI twice declined to widen this PR to satisfy The process reasoning still holds in general — I would not want a bot driving unbounded scope, and my objection to chasing a moving gate was not baseless — but the empirical result on this PR beats it, and pretending otherwise would leave a misleading record. The distinction that matters for #5305 and #5382The two shapes of this finding are not the same problem, and only one of them has this escape hatch:
So this result does not unblock #5305 (red since 28 Aug) or #5382 (since 30 Aug). Those remain genuinely blocked on a Blueprint edit or a policy decision about how hardening changes clear this gate. Two findings worth their own lookNeither blocks anything here, both are now visible because this job exists:
Generated by Claude Code |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
✅ Drift Bot (ClawMetry): no drift detectedDrift Bot analyzed the changed files against this project's blueprints and requirements and found no drift. |
|
Merged and verified live on
It is not passing vacuously. Every step ran, and the Pulled the
The three High findings, assessed rather than filedAll three are
zizmor flags I have not changed them. That is the point of this job being reporting, not gating: a finding that needs judgement should reach a human with its context intact, rather than either blocking a merge or being silently suppressed. Flagging them here so the assessment is on the record and can be revisited if any of those workflows ever gains a fork-reachable trigger. On the 24
|
2 participants
Product record: No-PRD: CI-only change, confined to
.github/workflows/supply-chain.yml.Risk: Low, and contained to CI. One new job in an existing workflow; no existing job, trigger or permission is touched. Findings are non-blocking — the job goes red only when a scanner itself did not produce a usable report, which is the case verified below. If it ever proves noisy, deleting the job restores today's behaviour exactly.
Summary
clawmetry-cloudsecurity-audit.yml;clawmetry-proNeMo Agent Toolkit telemetry exporter for ClawMetry #234). This repo — 38 workflow definitions, the most of the three and the only public one — was running no GitHub-Actions security scanner at all, and nothing was auditing its npm lockfiles.scorecardjob in this same workflow covers part of the ground (Dangerous-Workflow,Token-Permissions), but it isif: github.event_name != 'pull_request', so it never runs on the PR that introduces a problem, and it grades the repository rather than naming a file and a line.The three scanning responsibilities
The
SecurityAuditScannercomponent describes three: workflow definitions, dependency manifests, and Python source. The job now implements all three.zizmor--no-exit-codesso findings exit 0 and a non-zero exit means only "the tool failed".pip-audit+npm auditrequirements.txtin thepython-depsjob. The npm half is new:--package-lock-onlyover every committed lockfile.bandit -lltests/and vendored trees.pip-auditdeliberately stays inpython-depsrather than moving here — it has its own SBOM artifact and its own red condition, and splitting a passing job to tidy the layout is churn, not hardening.Why the npm half matters
Six committed lockfiles, none of them previously audited by anything in CI:
frontend/,tests/e2e/,.github/scripts/,.github/claude-cli/,.github/ci-node/auth-bootstrap/,.github/actions/setup-openclaw/.frontend/is the repo's largest dependency tree and it ships intoclawmetry/static/; the.github/ones are installed by CI jobs that run with this repo's token. Auditing only one of them would have reported "clean" for the five nobody scanned — the same impostor shape as a crashed scan.Outage detection, applied to all three
Neither new scanner's exit code can distinguish a finding from a failure:
banditexits 1 on findings,npm auditexits non-zero on both findings and registry errors. So neither exit code is trusted. The shape of the report is the test instead — bandit must write aresultslist, npm must write.metadata.vulnerabilities.total— and anything else records a.failedmarker that the end-of-job gate turns red. This is not hypothetical: it is the failure mode that made a retired npm advisory endpoint render as "no findings" on cloud.Scope: still workflow definitions only, for zizmor
The first revision also scanned the composite action under
.github/actions/. Drift Bot flagged that, and it was narrowed. That remains narrowed here, and the distinction is the point:.github/actions/is a scope no product record covers, whereas dependency and source scanning are scopes the component already describes. Implementing what the blueprint specifies is not the same kind of change as widening past it.The composite action is therefore still explicitly uncovered, with a comment in the workflow saying so. Follow-up.
Baseline today
Findings are deliberately not gated. The notable zizmor ones are load-bearing by design rather than defects: the release, auto-quarantine and cloud-pin jobs persist their checkout credential precisely because they
git pushwith it, and theworkflow_runtriggers are how those workflows are meant to fire. A gate that failed on those would be switched off within a week. bandit's total is likewise a baseline, not a defect list — which is why the summary prints the severity/confidence split rather than the total alone, so a reader can tell whether anything in it needs reading today. The one High-severity zizmor finding that is a defect is already fixed by #5744.Turning specific rules into a gate, with justified suppressions for the by-design ones, is a reasonable follow-up once the baseline has sat for a few weeks.
Test plan
yaml.safe_loadover all 38 workflow files → every one parsesrun:block extracted from the YAML and replayed underbash -e, exactly as Actions executes them → both scanner steps exit 0bandit: 280 findings in 24s on this tree, report parsesnpm audit: all six lockfiles produce a real.metadata.vulnerabilities.total; none hit the outage pathzizmor: 42 findings across 38 inputs, report parses.failedmarker makes the gate exit 1 while the artifact still uploadsE2E Gateleg (scripts/e2e_gate.py), so the rename moves no branch-protection context🤖 Generated with Claude Code
https://claude.ai/code/session_01FgwaB1b9mJUUYJJRKybcqe
Generated by Claude Code