Uh oh!
There was an error while loading. Please reload this page.
fix(caller-drift): the exempt probe read one protection system, not both (backend#1681) - #222
Merged
Merged
Conversation
…oth (backend#1681) `evaluate_protection`'s exempt-staleness check was `if probe.classic_present:`. A branch protected SOLELY by a ruleset returns 404 from the classic endpoint, so it read as unprotected and its exemption stayed silently valid -- the guard would never notice that a role it believes is out of scope is in fact a real, protected branch. That is the two-systems defect the 20-line header of read_protection() exists to prevent, sitting in the one place that decides whether an exemption is still true. `probe.rulesets` is populated by the very same call; it was simply never consulted. The finding now also names WHICH layer covers the branch, so the reader is not sent to re-derive it. The gap survived because the coverage stopped one path short: `_ruleset_only` already existed in the selftest and was only ever pointed at the `required` path, never at `exempt`. It is now used for both. Verified: * selftest 111 -> 117 pass, 0 fail * MUTATION: restoring the old `classic_present`-only condition makes the new case FAIL (findings=[]), so the test genuinely covers the hole rather than passing either way * LIVE against all 20 repos: "No drift. Every repo read, every entry matched." -- no exemption was actually hiding behind this, so closing it costs nothing today and catches the next one. 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 freeto 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.
The hole
evaluate_protection's exempt-staleness check was:A branch protected solely by a ruleset returns 404 from the classic endpoint, so it read as unprotected — and its exemption stayed silently valid. The guard would never notice that a role it believes is out of scope is in fact a real, protected branch.
That is exactly the two-systems defect the 20-line header of
read_protection()exists to prevent, sitting in the one place that decides whether an exemption is still true.probe.rulesetsis populated by the very same call; it was just never consulted.Why it survived
The selftest coverage stopped one path short.
_ruleset_onlyalready existed — and was only ever pointed at therequiredpath, never atexempt. It now covers both.The finding also names which layer covers the branch, so a reader isn't sent to re-derive the ruleset story by hand.
Test plan / evidence
classic_present-only conditionfindings=[])The mutation row is the point: it proves the new test covers the hole rather than passing either way. The live row means closing this costs nothing today — no exemption was actually hiding behind it — and it catches the next one.
Found by the round-2 pipeline audit, backend#1681. Parent epic: backend#1680.
Note
Low Risk
Narrows a guard false negative in offline-tested drift logic; no auth or runtime API behavior beyond more accurate staleness detection.
Overview
Fixes a fail-open in
evaluate_protectionwhen inventory marks a branch role asexempt: staleness used to fire only onprobe.classic_present, so a branch protected only by a ruleset (classic 404) looked unprotected and the exemption stayed valid.The exempt probe now treats
classic_presentorprobe.rulesetsas protected, and findings say whether coverage is classic protection or a named ruleset. A selftest reuses the existing_ruleset_onlystub on the exempt path so ruleset-only protection must report a stale exemption.Reviewed by Cursor Bugbot for commit 8ca024f. Bugbot is set up for automated code reviews on this repo. Configure here.