sanitize: allowlist PATH001 false positives from Dependabot digest bumps - #24
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdbd4df7ce
ℹ️ 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".
| "reason": "Immutable public-history instance of an inspected synthetic or closed-runtime fixture; the original path, complete blob OID, exact line, rule, and canonical content hash bind this exception and do not authorize any future blob." | ||
| }, | ||
| { | ||
| "path": "images/network-broker-dialer/Dockerfile@961b080fc5163477962e3198581dac94d32c3609", |
There was a problem hiding this comment.
Remove the flagged path literal from the commit metadata
When this commit becomes reachable during a push, scheduled, or manually dispatched run, these blob exceptions still do not make the history scan pass: the commit body itself contains /share/portable-ghar, and scan_history() scans commit bodies through _scan_public_history_metadata(), converting the resulting PATH001 match to HISTORY_META. I reproduced <commit 3fb4ffae>#body:5:HISTORY_META, so the --tracked --history step in .github/workflows/sanitization.yml remains red even though these 21 blob findings are suppressed. Rewrite the commit message to avoid the contiguous literal; these blob-scoped PATH001 entries cannot suppress a metadata finding.
Useful? React with 👍 / 👎.
The scheduled/push sanitization run (--tracked --history) walks every blob reachable via `git rev-list --objects --all`, which includes open Dependabot digest-bump branches fetched by actions/checkout's fetch-depth:0 (per its docs, 0 = all history for all branches and tags). Each time Dependabot bumps a pinned `debian:bookworm-slim@sha256:...` digest in one of these Dockerfiles, it produces a new blob. The scanner's PATH001 rule (matches a "share" segment immediately followed by a path component) fires on the container's standard /usr/share directory tree, which these images use to hold portable-ghar package-evidence files (dpkg manifests, lockfile copies, etc. -- a normal FHS build-time location, not a real NAS/personal path leak). Four Dockerfiles were affected by the currently-open bump branches (debian digest 1def178 -> 7b140f3): images/network-broker-dialer/Dockerfile, images/network-helper/Dockerfile, images/network-verifier/Dockerfile, and images/runner/Dockerfile. In every case the only diff from the already-allowlisted HEAD blob is the unrelated FROM digest; the flagged lines are byte-identical, confirmed by computing the scanner's own line hash directly against each new blob's content (matches the existing HEAD-equivalent entries exactly). Add 21 new entries to .sanitization-allowlist.json following the established per-blob historical-exception pattern (exact path@blobOID, line, rule, sha256, reason) -- purely additive, no existing entries touched, no rule disabled, no wildcard or path-level exclusion.
bdbd4df to
f5b333d
Compare
What was failing
The scheduled + push sanitization run (
python3 scripts/sanitize_public.py --tracked --history) walks every blob reachable viagit rev-list --objects --all.actions/checkoutis configured withfetch-depth: 0, which per its own docs means "all history for all branches and tags" -- so this includes open Dependabot digest-bump branches, not justmain.Each time Dependabot bumps a pinned
debian:bookworm-slim@sha256:...digest in one of the runner-family Dockerfiles, it creates a new blob for that file. The scanner'sPATH001rule (/share/[A-Za-z0-9_.-]+, meant to catch NAS/UNC-shaped absolute paths) matches the substring/share/portable-gharinside the legitimate container path/usr/share/portable-ghar/...-- a false positive on a standard FHS location, not an operator/NAS/personal path leak.Provenance of the findings
Four Dockerfiles were affected by the currently-open Dependabot bump branches (digest
1def178->7b140f3):images/network-broker-dialer/Dockerfile@961b080(lines 22, 28, 34, 35)images/network-helper/Dockerfile@c3028e9(lines 33, 34, 35)images/network-verifier/Dockerfile@e76ff41(lines 21, 26, 29, 30)images/runner/Dockerfile@4b7460b(lines 42, 91, 92, 94, 96, 97, 98, 131, 132, 133)For every one of these, the only diff from the already-allowlisted HEAD blob of the same file is the unrelated
FROMdigest line -- the flagged lines are byte-identical. I confirmed this two ways: (1)git diffbetween each flagged blob and HEAD's current blob for the same path, and (2) computing the scanner's owncompute_line_hash()directly against each new blob's content, which reproduces the exactsha256already recorded for the equivalent HEAD-blob entries.Fix
HEAD:images/*/Dockerfile(tracked scan) was already clean -- no scrubbing needed there, since these are legitimate container-internal paths, not real leaked personal/NAS paths (nothing in this finding set is actual sensitive data).The repo's only sanctioned suppression mechanism is
.sanitization-allowlist.json(enforced byscripts/sanitize_public.py: exact{path, line, rule, sha256, reason}match, no wildcards, no unknown rules, no path-prefix exclusion -- seeload_allowlist()). This PR adds 21 new entries following the repo's established per-blob historical-exception pattern (path@40-char-blob-oid, same line/rule/sha256/reason shape used for every prior Dependabot-bump blob of these same files). The change is purely additive: no existing entry was modified, no rule was disabled, and no wildcard/path-level exclusion was introduced.Verification
python3 scripts/sanitize_public.py --tracked --historyexits 1 with 21PATH001findings across the four blobs above.sanitization passed.git diff .sanitization-allowlist.jsonis 147 insertions / 0 deletions (net: the trailing]line is regenerated identically by the JSON dump, not an actual removal).No history rewriting was performed or considered necessary -- the existing allowlist mechanism is the sanctioned, narrowly-scoped fix for this exact recurring situation.