Skip to content

fix(ci): unmask security scans and verify every download - #148

Merged
github-actions[bot] merged 17 commits into
mainfrom
fix/ci-scan-masks
Oct 27, 2026
Merged

github-actions[bot] merged 17 commits into
mainfrom
fix/ci-scan-masks

Conversation

@brujack

@brujack brujack commented Oct 27, 2026

Copy link
Copy Markdown
Owner

Summary

PR A of the CI and release hardening plan (docs/superpowers/plans/2026-10-27-ci-release-hardening.md, spec R1-R4).

  • Scans can fail again. continue-on-error is removed from the gitleaks and Snyk steps, so the required Secret Scan and Snyk Security Scan checks report what the tools find. SNYK_TOKEN was unset until 2026-10-27, so every Snyk run had failed authentication behind the mask.
  • Snyk false positives excluded. .snyk excludes tests/test_automerge_bot_hold.py and scripts/test_metrics.py, whose three LOW Snyk Code findings were false positives. Snyk Code cannot ignore a single finding from a repo file (only via the web UI), so this is the path-exclusion fallback the spec approved.
  • Every direct download is pinned and verified. gitleaks 8.30.1, Snyk CLI v1.1308.0 (was cli/latest) and mdbook 0.5.3 (was piped into tar) now download to a file, pass sha256sum -c against the publisher's digest, and check the installed version.
  • Two text-scanning tests enforce it. tests/test_continue_on_error.py fails on any continue-on-error not in its ALLOWED list (or listed but gone, or unrecognised spellings). tests/test_workflow_downloads.py fails on any curl/wget without a pinned https URL, an output file and a fail-capable checksum before use.

Test Plan

  • make test: nextest 1145 passed; pytest 119 passed (42 in the two new modules)
  • Both new tests fail against main's ci.yml (the two masks; the three unpinned downloads)
  • Mutation checks on every scanner fix (killed), and a 200,000-block differential for the C901 refactor of check_block (0 divergences)
  • CI: Secret Scan and Snyk Security Scan pass unmasked; Snyk log shows Open issues: 0; install steps print OK from sha256sum -c
  • Coverage: no Rust change; Python floor 87 applies to scripts/ and .claude/scripts/, untouched

Code review

reviewed-sha: c761064
diff-hash: 3ba680acd9d68d5b78a9e72ba06ac9b9bd8c602b35ec53244d490406e255d31c
reviewers: 5/5

Round 1 (cbfca98): 5/5 reviewers, two informational findings on tests/test_continue_on_error.py docstrings (fixed in fc4fa16). Security-review then found env-var-to-open() in both new test modules (Snyk Code path-traversal shape, now blocking); fixed in 97a53d8.
Round 2 (97a53d8): 5/5 reviewers. Required, all fixed in 7e84a47: a curl/wget with no literal https URL was skipped; the checksum matched the path as a substring; a sha256sum -c that cannot fail the step counted as verification; a "used before checksum" problem suppressed a later download's missing checksum.
Round 3 (verification, 1 reviewer, bug lens; the other four lenses were clean twice on unchanged ground): two further bypasses fixed in 90c1647 (a comment or echoed text counted as the checksum; ./-relative output paths unmatched) plus the || exit 1 false positive.
Round 4 (verification): every fix holds; 5 of 6 line mutations killed. The surviving mutant (comment strip) is now covered by b8d2900, mutation-checked.
Round 5 (verification of later test-only commits: empty-input tests, the C901 split of check_block, a docstring line): no issues; old vs new check_block agree on 8 hand-built cases and 200,000 fuzzed blocks.

Informational

RESOLVED: 9 fixed, 0 kept

Spec alignment (2026-10-27)

  • spec: docs/superpowers/specs/2026-10-27-ci-release-hardening-design.md
  • anchor: 7aed051
  • in scope: R1, R2, R3, R4, R5, R6, R7, R8, R9
  • out of scope: none

Findings

ID Reviewer Verdict Reason Amendment
R5 both NOT-BUILT Looked for changes to release.yml that validate version as MAJOR.MINOR.PATCH against both Cargo.toml files, check for an existing tag and the refs/heads/main ref, ahead of other steps. release.yml does not appear in the diff. - R5 reviewed: not in scope for PR A (plan Tasks 1-3, R1-R4); delivered by PR B (plan Task 4) of the same plan — the spec delivers in four ordered PRs
R6 both NOT-BUILT release.yml is unchanged in the diff. The 'Commit CHANGELOG' push step is still present (ALLOWED keeps it, 'removed by PR B'), and no notes range using git merge-base <previous tag> HEAD appears. - R6 reviewed: not in scope for PR A (plan Tasks 1-3, R1-R4); delivered by PR B (plan Task 4) of the same plan — the spec delivers in four ordered PRs
R7 both NOT-BUILT The diff neither deletes CHANGELOG.md nor adds an ADR reversing ADR-0009's committed-changelog clause. - R7 reviewed: not in scope for PR A (plan Tasks 1-3, R1-R4); delivered by PR B (plan Task 5) of the same plan — the spec delivers in four ordered PRs
R8 both NOT-BUILT The diff has no taiki-e/install-action step, no removal of cargo install, and no test enforcing tool: <name>@<X.Y.Z> / fallback: none. - R8 reviewed: not in scope for PR A (plan Tasks 1-3, R1-R4); delivered by PR C (plan Task 6) of the same plan — the spec delivers in four ordered PRs
R9 both NOT-BUILT The Makefile is not in the diff, so there is no change to build or build-linux output locations and no make install target writing to ${XDG_BIN_HOME:-$HOME/.local/bin}/etch. - R9 reviewed: not in scope for PR A (plan Tasks 1-3, R1-R4); delivered by PR D (plan Task 7) of the same plan — the spec delivers in four ordered PRs

Reviewed

  • R5 reviewed: not in scope for PR A (plan Tasks 1-3, R1-R4); delivered by PR B (plan Task 4) of the same plan — the spec delivers in four ordered PRs
  • R6 reviewed: not in scope for PR A (plan Tasks 1-3, R1-R4); delivered by PR B (plan Task 4) of the same plan — the spec delivers in four ordered PRs
  • R7 reviewed: not in scope for PR A (plan Tasks 1-3, R1-R4); delivered by PR B (plan Task 5) of the same plan — the spec delivers in four ordered PRs
  • R8 reviewed: not in scope for PR A (plan Tasks 1-3, R1-R4); delivered by PR C (plan Task 6) of the same plan — the spec delivers in four ordered PRs
  • R9 reviewed: not in scope for PR A (plan Tasks 1-3, R1-R4); delivered by PR D (plan Task 7) of the same plan — the spec delivers in four ordered PRs

Verifications

  • none

🤖 Generated with Claude Code

https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg

brujack and others added 17 commits October 27, 2026 13:37
Four PRs from the math learnings triage: unmask and checksum the
scans, fix the release workflow, pin tool installs, keep builds in
the repo.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Snyk Code cannot ignore single findings from the repo, so .snyk excludes the two files whose three LOW findings were false positives.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Snyk Code reports an env var reaching open() as path traversal, and
the Snyk step now blocks. The scan functions take the directory as a
parameter instead.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Any curl/wget line is a download; a variable or http:// URL is flagged.
The checksum must name the downloaded path as a whole token, not a substring.
A sha256sum -c under if, !, ||, && or ; cannot fail the step and does not count.
The used-before-checksum suppression is per download, not per block.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
The comment strip in check_block had no test; removing it left the
suite green. A comment naming curl must not count as a download.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xRsBqwrPLVSCJ1hXnhotg
@github-actions
github-actions Bot merged commit 83c9163 into main Oct 27, 2026
12 checks passed
@brujack
brujack deleted the fix/ci-scan-masks branch October 27, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant