Repository navigation
Conversation
…enign values The pre-push hook scans with the engine's allowlist option permanently empty — there was no config surface at all for a known-benign value (a public dev-compose password, a documented sample URL). The observed failure mode: rather than bypassing, a worker restructured a credential string across variables until the scanner stopped matching — detector evasion that silences the finding while leaving no record anywhere. A recorded, reviewable suppression surface is strictly better. - bin/gstack-redact-prepush reads .gstack-redact-allowlist at the repo root (one exact string per line, # comments) and passes entries as the scan allowlist (the engine already honors allow.has(span)). - Entries match the CAPTURED SPAN, so for db.url_with_password an entry clears one specific URL, never every use of a password. - Suppression is reported, never silent: the hook prints how many HIGH/MEDIUM findings the allowlist suppressed on each push. - The HIGH block message names the allowlist as the recorded resolution path and explicitly warns against restructuring to dodge the pattern. - Fail-strict: absent/unreadable/oversized (>64KiB) file → scan runs WITHOUT an allowlist; it can never block or loosen a push by failing. Tests: 7 new cases in test/redact-prepush-hook.test.ts (HIGH cleared, suppression notice, exact-span scope, comment parsing, self-bootstrap push of the allowlist file, oversize fallback, block-message hint). Mutation check: dropping the allowlist from the scan opts sends 4 of them RED.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Contributor
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.
Why (in your own words)
The pre-push hook calls the scan engine with its
allowlistoption permanently empty — there is no config surface anywhere for a known-benign value (a public dev-compose password, a documented sample URL). So every push touching such a value warns or blocks, forever, with no legitimate resolution. We watched what that trains people to do on a real repo: rather than bypass the hook, an AI worker restructured a dev-only credential string across variables until the scanner stopped matching. Detector evasion — the finding is silenced AND no record exists anywhere. A reviewable allowlist entry is strictly better than both a bypass and a reshape: it leaves the decision in the repo, in a diff, with a comment.This PR adds that surface: a repo-root
.gstack-redact-allowlistfile (one exact string per line,#comments), passed as the engine's existingallowlistscan option (the engine already honorsallow.has(span); the hook just never fed it).Design points:
db.url_with_passwordthat is the wholescheme://user:pass@host:portURL, so an entry clears one specific benign URL, never every use of that password.Live evidence
Before/after on a real repo, driving the hook with the git pre-push stdin protocol (a docker-compose dev URL, public in the repo it came from):
Test file (7 new cases alongside the existing hook tests):
Mutation check — drop
allowlistfrom the scan opts in the hook and the new tests go RED (real output, then reverted):bun run test(free tier): all shards pass except 7 pre-existing failures inbrowse/test/bun-polyfill.test.ts, verified identical on pristineupstream/mainon the same machine — cause isnodenot installed on this host, unrelated to this change.Scope
bin/gstack-redact-prepush(allowlist load + wiring + suppression notice + block-message hint),test/redact-prepush-hook.test.ts(7 new tests).path.joinongit rev-parse --show-topleveloutput, same as the rest of the hook); the engine's span normalization for non-ASCII allowlist entries (entries are compared against the engine's normalized span — exotic Unicode values may need their NFKC form in the file).Liveness proof (required)
The repo operator (boyard) will attach the
GSTACK PRliveness screenshot and tick the checklist before marking this ready for review — draft until then. (This PR was prepared by an AI agent in the operator's environment; the screenshot requirement exists precisely to put a human on record, so it is his to provide, not mine to generate.)Checklist
GSTACK PRtyped live into a real surface (not edited onto the image)