fix(security): bring this repo's gates level with its siblings - #32
Merged
Conversation
Each of the four repositories had a security gate the others lacked, so all four were partly unexamined while every one of them ran a green job named for dependency or security scanning. This repo gains what it was missing, and the shared scanner brings four fixes that porting it across three more codebases exposed: a credential pattern that could match across two separate string literals, caches being scanned (a finding was raised against a cached test-id string), a whole-file marker for a deliberate synthetic-secret corpus, and -- the one worth naming -- the scanner exempting *itself*, because its source contains its own marker constants. scripts/check_action_pins.py resolves every pinned action SHA against the GitHub API and fails when a version comment disagrees with it. The pin is the supply-chain control; the comment is what a reviewer actually reads. It found api-verity-lab calling a v7.0.0 SHA "v5.6.0" in five places and this family of repos disagreeing about the same SHA. tests/test_secret_scan_catches_secrets.py plants a real-shaped secret of every class the scanner claims to detect. Writing it found four classes -- Slack, Google, Stripe, npm -- with no rule at all. A scanner that reports clean is otherwise indistinguishable from one that checks nothing. Verified: ruff, ruff format, mypy, 238 tests pass, coverage 71.74%.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each of the four sibling repositories had a security gate the others lacked, so all four were partly unexamined while every one ran a green job named for dependency or security scanning.
security-extendedThe shared scanner came with four fixes
Porting it across three more codebases exposed problems the original had in every copy:
credential-assignmentmatched across two string literals. Its value class was[^"']{12,}, which can start at the closing quote of one literal and end at the opening quote of the next —write_text("token: " + TOKEN, encoding="utf-8")read as one credential. Real secrets contain no whitespace..pytest_cache.devrepro-doctorneeds for its deliberate synthetic-secret corpus.Pinned actions
scripts/check_action_pins.pyresolves every pinned SHA against the GitHub API and fails when a version comment disagrees. The pin is the supply-chain control; the comment is what a reviewer reads. It foundapi-verity-labcalling a v7.0.0 SHAv5.6.0in five places, and this family disagreeing about the same SHA.The scanner needed its own test
test_secret_scan_catches_secrets.pyplants a real-shaped secret of every class the scanner claims to detect. Writing it found four classes — Slack, Google, Stripe, npm — with no rule at all. Fixtures are assembled from fragments at runtime so no complete credential-shaped literal is committed; GitHub push protection rejected an earlier version that used whole literals, which is useful independent evidence the fixtures are realistic.A scanner that reports clean is otherwise indistinguishable from one that checks nothing.