fix(ci): confirm deny-list negatives too, and fix a cwd-relative shallow probe#524
Merged
Merged
Conversation
…low probe Follow-up to the previous commit, closing two holes found by adding a test for the path production actually takes. 1. The deny-list's NEGATIVE was still trusting local `merge-base` on a partial clone — which is ALWAYS the case in CI (`--filter=tree:0`) and is the one operation measured unreliable there. A treeless clone resolves every commit, so the API fallback was gated on "commits do not resolve" and was therefore dead code in production. Trace for awesome-haskell (pin 5a93d9d, which predates the Hypatia fix and must be denied): local branch taken -> a wrong negative -> `continue` -> not denied -> classify_pin also mis-fires -> API says ahead=90 -> OUT_OF_WINDOW -> notice -> PASS. That is the exact false green this list exists to prevent, on the very cohort that proved the bug. Now: a local POSITIVE is still trusted (no false positives observed, and a deny must not require the network), but a local NEGATIVE is accepted only from a COMPLETE clone; otherwise it is confirmed server-side. 2. `clone_is_complete` used `rev-parse --git-dir`, which returns a path relative to the CWD — and the CWD here is the CONSUMER's checkout, which actions/checkout makes shallow by default. So `.git/shallow` would almost always exist and every standards clone would be called incomplete. Uses `--absolute-git-dir` now. Caught by test 13c, which failed for exactly this reason when run from a shallow clone. 3. governance-reusable.yml passes GITHUB_TOKEN to the staleness step. The server-side confirmation is unauthenticated (60/hr per runner IP) and exhausting it degrades the gate to ::warning. That fail-open is deliberate and loud, and the common path never calls the API (a genuine deny is a local positive) — but a token removes the cliff. It reaches consumers only as they re-pin past this commit, which is precisely the propagation the deny-listed pins need anyway. Tests: 16 -> 19 cases, all passing, still hermetic (the API is pointed at a closed port). The new cases cover the production path that cases 1-12 could not reach, because they all run against a complete `git init` fixture: * partial clone: local positive still denies without the network * partial clone: unverifiable deny-list check is reported SKIPPED, not passed * complete clone: local negative is trusted silently (no SKIPPED warning) run_case_out gained a leading-'!' inversion so a test can assert the gate did NOT say something. Suite verified from two different working directories, since the bug in (2) was cwd-dependent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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.



Follow-up to #522, which merged before this landed. #522 is live on
mainnow, so these two defects are live on ~300 consumers.Both were found by adding a test for the path production actually takes. Cases 1–12 of the fixture suite run against a complete
git initrepo; CI runs against a partial clone (--filter=tree:0). Nothing tested the code that protects production, and it had two bugs.1. The deny-list's negative still trusted local
merge-baseon a partial cloneA treeless clone resolves every commit, so the API fallback was gated on "commits do not resolve" — making it dead code in CI. Trace
awesome-haskell(pin5a93d9d5, which predates the Hypatia fix and must be denied):That is the exact false green the deny-list exists to prevent, on the very cohort that proved
merge-baseis unreliable there.Fix: a local positive is still trusted (no false positives observed, and a deny must not require the network), but a local negative is accepted only from a complete clone — otherwise it is confirmed server-side.
2.
clone_is_completeused a cwd-relative git-dirrev-parse --git-dirreturns.git, and the check was[ -e "$gd/shallow" ]— which resolves against the CWD, i.e. the consumer's checkout, whichactions/checkoutmakes shallow by default. So.git/shallowwould nearly always exist and every standards clone would be classified incomplete, permanently masking the complete-clone fast path.Now uses
--absolute-git-dir. Test 13c failed for exactly this reason, which is how it was found.3.
governance-reusable.ymlpassesGITHUB_TOKENto the staleness stepThe server-side confirmation is unauthenticated — 60/hr per runner IP — and exhausting it degrades the gate to
::warning. That fail-open is deliberate and loud (two warnings naming the pin; never a silent pass), and the common path never calls the API at all, since a genuine deny is a local positive. A token removes the cliff.This reaches a consumer only once it re-pins past this commit — the script is read from
standardsHEAD, but the job definition comes from the pinned SHA — so it lands naturally with the propagation the deny-listed pins need anyway.Tests: 16 → 19, still hermetic
The API is pointed at a closed port (
http://127.0.0.1:1), so these are fast and offline:run_case_outgained a leading-!inversion so a test can assert the gate did not say something. Suite verified from two different working directories, since defect (2) was cwd-dependent. 19/19 pass on top of currentmain;shellcheck -S warningclean.🤖 Generated with Claude Code