-
Notifications
You must be signed in to change notification settings - Fork 66
Harden CI: run the full security-audit scanner (workflows, package-lock, Python source) #5733
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
818ef04
845c79a
3b6c699
7fbca62
3c8bbbc
52c072a
5e9de09
dc6bcf9
7eb8b0a
333e9e9
8d85fbc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -175,6 +175,304 @@ jobs: | |
| GITHUB_TOKEN: ${{ github.token }} | ||
| run: python3 scripts/check_action_refs.py | ||
|
|
||
| actions-security: | ||
|
vivekchand marked this conversation as resolved.
vivekchand marked this conversation as resolved.
vivekchand marked this conversation as resolved.
|
||
| name: Security audit (workflows, package-lock, Python source) | ||
| runs-on: ubuntu-latest | ||
| # CI is part of the supply chain: a workflow step runs with this repo's | ||
| # token and secrets, and its output is packaged into the wheel PyPI | ||
|
vivekchand marked this conversation as resolved.
|
||
| # serves. Both private repos in this family (clawmetry-cloud, | ||
| # clawmetry-pro) have run zizmor on every PR for weeks; this repo, with | ||
| # the most workflow definitions of the three and the only public attack | ||
| # surface, was the one running no Actions-security scanner. Scorecard's | ||
| # `Dangerous-Workflow` and `Token-Permissions` probes below cover part | ||
| # of the same ground, but they run only on push (`if:` above) and score | ||
| # the repository rather than naming the file and line. | ||
| # | ||
| # Three scanning responsibilities, matching the SecurityAuditScanner the | ||
| # private repos already run: | ||
| # 1. workflow definitions -> zizmor | ||
| # 2. dependency manifests -> pip-audit (the `python-deps` job | ||
| # above, on requirements.txt) + `npm audit` here, on every committed | ||
| # package-lock.json. The npm half is the one that had no scanner at | ||
| # all: this repo's largest dependency tree is `frontend/`, and it | ||
| # ships into `clawmetry/static/`. | ||
| # 3. Python source -> bandit | ||
| # pip-audit stays in `python-deps` rather 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. | ||
| steps: | ||
|
vivekchand marked this conversation as resolved.
|
||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| persist-credentials: false | ||
| - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 | ||
| with: | ||
| python-version: "3.11" | ||
| # The npm-audit step below needs a Node of its own. Without this it runs | ||
| # against whatever the runner image happens to ship, which is not a | ||
| # hypothetical: clawmetry-pro#234 hit exactly that, where an unpinned old | ||
| # npm returned a 400 from the retired `audits/quick` endpoint npm 6 uses. | ||
| # That matters more here than it did there, because this job's gate | ||
| # treats an npm-audit outage as RED — an image bump could turn `main` | ||
| # red with nothing in the diff to explain it. | ||
| - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 | ||
| with: | ||
| node-version: "20" | ||
| # A GitHub Actions cache is not trusted input, and this job reads | ||
| # lockfiles to decide whether the tree is vulnerable. Nothing is | ||
| # installed here anyway (`npm audit --package-lock-only`), so the | ||
| # cache would buy nothing and could only muddy the result. Same | ||
| # reasoning as publish.yml. | ||
| package-manager-cache: false | ||
| - run: python3 -m pip install --quiet zizmor bandit | ||
|
|
||
| - name: Audit every workflow definition | ||
| env: | ||
| # zizmor resolves action tags through the GitHub API. Unauthenticated, | ||
| # those calls share the runner's IP rate limit and fail intermittently | ||
| # — which, because a reachability failure aborts the WHOLE audit, is | ||
| # the difference between a scan and no scan. Read-only; the job's | ||
| # `contents: read` default is enough to read public action repos. | ||
| GH_TOKEN: ${{ github.token }} | ||
| run: | | ||
| mkdir -p audit | ||
| # Workflow definitions only, which is the scope the SecurityAuditScanner | ||
| # component describes. | ||
| # | ||
| # The composite action under .github/actions/ is a real gap and is NOT | ||
| # covered here: its steps run inline in the calling job, with that job's | ||
| # token and secrets, so it carries the same rule families a workflow | ||
| # does, and nothing scans it today. Widening this input set to include | ||
| # it is a one-line change, but it is a scope the blueprint does not | ||
| # describe, so it needs the product record first rather than arriving | ||
| # as a side effect of turning the scanner on. Tracked as follow-up. | ||
| inputs=".github/workflows" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| # Record the file set the scanner was handed. A finding list is | ||
| # identical whether it audited thirty-seven files or none, so | ||
| # coverage is the one thing the JSON cannot tell you afterwards. | ||
| # shellcheck disable=SC2086 | ||
| find $inputs -type f \( -name '*.yml' -o -name '*.yaml' \) \ | ||
| | sort > audit/zizmor-inputs.txt | ||
| n_inputs=$(wc -l < audit/zizmor-inputs.txt | tr -d '[:space:]') | ||
| echo "$n_inputs" > audit/zizmor-input-count.txt | ||
| echo "zizmor inputs ($n_inputs file(s)):" | ||
| sed 's/^/ /' audit/zizmor-inputs.txt | ||
|
|
||
| # `--no-exit-codes` makes FINDINGS exit 0, so a non-zero exit here | ||
| # means only "the tool failed". Without it the two cases are | ||
| # indistinguishable, and the usual `|| true` launders a crash — which | ||
| # writes NO json — into an empty file that reads as a clean scan. | ||
| rc=0 | ||
| # shellcheck disable=SC2086 | ||
| zizmor --no-exit-codes --format json --persona regular $inputs \ | ||
| > audit/zizmor.json 2> audit/zizmor.err || rc=$? | ||
| # An empty input set would let zizmor exit 0 having audited nothing: | ||
| # the same "clean scan" impostor as a crash. | ||
| if [ "$n_inputs" -eq 0 ]; then | ||
| echo "zizmor was handed no files to audit" >> audit/zizmor.err | ||
| if [ "$rc" -eq 0 ]; then rc=1; fi | ||
| fi | ||
| # A crash leaves no json at all, so reject anything that does not | ||
| # parse as a list. | ||
| if ! python3 -c "import json; d = json.load(open('audit/zizmor.json')); assert isinstance(d, list); print(f'zizmor: {len(d)} finding(s)')" 2>/dev/null; then | ||
| if [ "$rc" -eq 0 ]; then rc=1; fi | ||
| fi | ||
| if [ "$rc" -ne 0 ]; then | ||
| # Recorded, not raised: the artifact still has to upload, and the | ||
| # gate at the end of the job is what turns this red — once there | ||
| # is something to diagnose it from. | ||
| echo "$rc" > audit/zizmor.failed | ||
| echo "::error::zizmor did not complete (exit $rc); this is a scanner outage, not a clean scan" | ||
| sed 's/^/ /' audit/zizmor.err || true | ||
| fi | ||
|
|
||
| # Responsibility 3: static analysis on this repo's Python source. | ||
| # Scorecard's SAST probe with teeth — it scores the repository, this | ||
| # names the file and line. | ||
| - name: Scan Python source (bandit) | ||
| run: | | ||
| # -ll: medium severity and above. Low-severity bandit over a tree | ||
| # this size is thousands of `assert` and `subprocess` notes, which | ||
| # buries the findings that matter. | ||
| # | ||
| # bandit exits 1 on FINDINGS and on failure alike, so — same | ||
| # reasoning as the zizmor step above — the exit code cannot tell a | ||
| # finding from an outage. The report is what separates them: a real | ||
| # run writes a dict with a `results` list, a crash writes nothing. | ||
| bandit -r . -f json -o audit/bandit.json -ll \ | ||
| -x ./node_modules,./.venv,./venv,./tests,./frontend,./.git || true | ||
| if ! python3 -c "import json; d = json.load(open('audit/bandit.json')); print(f\"bandit: {len(d['results'])} finding(s)\")" 2>/dev/null; then | ||
| echo 1 > audit/bandit.failed | ||
| echo "::error::bandit did not produce a usable report; this is a scanner outage, not a clean scan" | ||
| fi | ||
|
|
||
| # Responsibility 2 (the package-lock half). Every committed lockfile, | ||
| # not just frontend/: /tests/e2e, /.github/scripts and the rest are all | ||
| # installed by CI jobs that run with this repo's token, and auditing one | ||
| # lockfile would report "clean" for the ones nobody scanned. | ||
| - name: Scan npm dependencies (package-lock) | ||
| run: | | ||
| locks=$(find . -name package-lock.json -not -path '*/node_modules/*' -not -path './.git/*' | sort) | ||
| if [ -z "$locks" ]; then | ||
| echo "no package-lock.json — nothing to audit" | ||
| exit 0 | ||
| fi | ||
| while IFS= read -r lock; do | ||
| dir=$(dirname "$lock") | ||
| # `.github/scripts` -> `github-scripts`: one report per lockfile, | ||
| # named after the directory it audited. | ||
| slug=$(printf '%s' "${dir#./}" | tr '/' '-' | tr -d '.') | ||
| if [ "$dir" = "." ]; then slug="root"; fi | ||
| out="audit/npm-audit-$slug.json" | ||
| err="audit/npm-audit-$slug.err" | ||
| echo "auditing $lock -> $out" | ||
| # `npm audit` exits non-zero BOTH when it finds vulnerabilities and | ||
| # when it fails outright, so `|| true` on its own launders a | ||
| # registry error into what reads as a clean scan. The shape of the | ||
| # output is what separates them: a real result carries | ||
| # .metadata.vulnerabilities.total, an error response carries | ||
| # .message instead. | ||
| ( cd "$dir" && npm audit --package-lock-only --json ) > "$out" 2> "$err" || true | ||
| if ! python3 -c "import json; d=json.load(open('$out')); t=d['metadata']['vulnerabilities']['total']; assert isinstance(t, int); print(f' npm audit ($lock): {t} vulnerability(ies)')" 2>/dev/null; then | ||
| echo 1 > audit/npm-audit.failed | ||
| echo "::error::npm audit did not produce a result for $lock; this is a scanner outage, not a clean scan" | ||
| python3 -c "import json; print(' registry said:', json.load(open('$out')).get('message', '<no message>'))" 2>/dev/null || true | ||
| sed 's/^/ /' "$err" 2>/dev/null || true | ||
| fi | ||
| done <<< "$locks" | ||
|
|
||
| # Findings are reported, not gated. Today's tree has findings that are | ||
| # load-bearing by design — the release, quarantine and cloud-pin jobs | ||
| # persist their checkout credential BECAUSE they push with it — and a | ||
| # gate that fails on those would be turned off within a week. The | ||
| # summary is what makes a NEW finding visible in the PR that adds it. | ||
| - name: Summarise findings | ||
| if: always() | ||
| run: | | ||
| python3 - <<'PY' >> "$GITHUB_STEP_SUMMARY" | ||
| import collections, json, pathlib | ||
|
|
||
| print("### Security audit\n") | ||
| print("| Scanner | Scope | Findings |") | ||
| print("| --- | --- | ---: |") | ||
|
|
||
| def bandit_row(): | ||
| if pathlib.Path("audit/bandit.failed").exists(): | ||
| return "**SCAN FAILED**" | ||
| try: | ||
| return str(len(json.loads(pathlib.Path("audit/bandit.json").read_text())["results"])) | ||
| except Exception: | ||
| return "**SCAN FAILED**" | ||
|
|
||
| def npm_rows(): | ||
| if pathlib.Path("audit/npm-audit.failed").exists(): | ||
| yield "npm audit", "package-lock", "**SCAN FAILED**" | ||
| for f in sorted(pathlib.Path("audit").glob("npm-audit-*.json")): | ||
| try: | ||
| t = json.loads(f.read_text())["metadata"]["vulnerabilities"]["total"] | ||
| except Exception: | ||
| t = "**SCAN FAILED**" | ||
| yield "npm audit", f"`{f.stem[len('npm-audit-'):]}`", str(t) | ||
|
|
||
| p = pathlib.Path("audit/zizmor.json") | ||
| n = pathlib.Path("audit/zizmor-input-count.txt") | ||
| covered = n.read_text().strip() if n.exists() else "?" | ||
| zizmor_failed = pathlib.Path("audit/zizmor.failed").exists() or not p.exists() | ||
| findings = [] | ||
| if not zizmor_failed: | ||
| try: | ||
| findings = json.loads(p.read_text()) | ||
| except Exception: | ||
| zizmor_failed = True | ||
| print( | ||
| f"| zizmor | workflow definitions ({covered} file(s)) | " | ||
| + ("**SCAN FAILED**" if zizmor_failed else str(len(findings))) | ||
| + " |" | ||
| ) | ||
| for name, scope, count in npm_rows(): | ||
| print(f"| {name} | {scope} | {count} |") | ||
| print(f"| bandit | Python source (medium+) | {bandit_row()} |") | ||
| print("\nFull reports: the `actions-security-audit` artifact.\n") | ||
|
|
||
| # bandit's total is a baseline number on a tree this size; the | ||
| # severity/confidence split is what says whether anything in it | ||
| # needs reading today. | ||
| try: | ||
| b = json.loads(pathlib.Path("audit/bandit.json").read_text())["results"] | ||
| except Exception: | ||
| b = [] | ||
| if b: | ||
| print("#### Python source (bandit)\n") | ||
| brank = {"HIGH": 0, "MEDIUM": 1, "LOW": 2} | ||
| by_test = collections.Counter( | ||
| (r.get("issue_severity") or "?", r.get("issue_confidence") or "?", | ||
| r.get("test_id") or "?", r.get("test_name") or "?") | ||
| for r in b | ||
| ) | ||
| print("| Severity | Confidence | Rule | Findings |") | ||
| print("| --- | --- | --- | ---: |") | ||
| for (sev, conf, tid, tname), c in sorted( | ||
| by_test.items(), | ||
| key=lambda kv: (brank.get(kv[0][0], 9), brank.get(kv[0][1], 9), kv[0][2]), | ||
| ): | ||
| print(f"| {sev} | {conf} | `{tid}` {tname} | {c} |") | ||
| print() | ||
|
|
||
| # The zizmor breakdown below is the part a reviewer reads to spot a | ||
| # NEW workflow finding in the PR that introduced it. | ||
| print("#### GitHub Actions security (zizmor)\n") | ||
| if zizmor_failed: | ||
| print("**SCAN FAILED** — see the `actions-security-audit` artifact.") | ||
| raise SystemExit(0) | ||
| print(f"{len(findings)} finding(s) across {covered} audited file(s).\n") | ||
| if not findings: | ||
| raise SystemExit(0) | ||
| rank = {"High": 0, "Medium": 1, "Low": 2, "Informational": 3} | ||
| counts = collections.Counter( | ||
| (f.get("determinations", {}).get("severity") or "?") for f in findings | ||
| ) | ||
| print("| Severity | Findings |") | ||
| print("| --- | ---: |") | ||
| for sev, c in sorted(counts.items(), key=lambda kv: rank.get(kv[0], 9)): | ||
| print(f"| {sev} | {c} |") | ||
| print() | ||
| by_rule = collections.Counter( | ||
| ( | ||
| (f.get("determinations", {}).get("severity") or "?"), | ||
| (f.get("ident") or "?"), | ||
| ) | ||
| for f in findings | ||
| ) | ||
| print("| Severity | Rule | Findings |") | ||
| print("| --- | --- | ---: |") | ||
| for (sev, rule), c in sorted( | ||
| by_rule.items(), key=lambda kv: (rank.get(kv[0][0], 9), kv[0][1]) | ||
| ): | ||
| print(f"| {sev} | `{rule}` | {c} |") | ||
| PY | ||
|
|
||
| - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 | ||
| if: always() | ||
| with: | ||
| name: actions-security-audit | ||
| path: audit/ | ||
| retention-days: 30 | ||
|
|
||
| # The only red condition: the scanner did not run. A silent outage is | ||
| # the failure mode this whole job exists to avoid, because it looks | ||
| # exactly like a clean result. | ||
| - name: Fail if a scanner did not run | ||
| if: always() | ||
| run: | | ||
| rc=0 | ||
| for s in zizmor bandit npm-audit; do | ||
| if [ -f "audit/$s.failed" ]; then | ||
| echo "$s did not produce a usable report; treating as a failed scan." | ||
| rc=1 | ||
| fi | ||
| done | ||
| if [ "$rc" -eq 0 ]; then echo "every scanner completed."; fi | ||
| exit "$rc" | ||
|
|
||
| scorecard: | ||
| name: OpenSSF Scorecard | ||
| runs-on: ubuntu-latest | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.