feat(dast): extend the reach and BFLA passes to the write verbs (BACKLOG #318) - #923
Open
wshallwshall wants to merge 1 commit into
Open
feat(dast): extend the reach and BFLA passes to the write verbs (BACKLOG #318)#923wshallwshall wants to merge 1 commit into
wshallwshall wants to merge 1 commit into
Conversation
…LOG #318) The sweep's reach and BFLA passes were GET-only. Re-measured at c57903c by execution first: the row was right, and the positive control in the same run is that non-GET gated rows exist in quantity (55 of 102 gated rows, 43 of 62 BFLA candidates), so GET-only was a real restriction and not an artifact. Write BFLA now covers every candidate row the walk derives -- asserted as an equation over run-derived numbers, not a hand count. Write reach measured 47 of 55 rows; the other 8 sit behind an action-scoped step-up gate a password login cannot satisfy and are entered in the policy's unreachable_allowlist with that reason, guarded against naming a row the live table no longer has. ADR 0155 said extending past GET "needs a per-probe store reset". Measuring it showed otherwise. FastAPI solves a route's dependencies -- where every require*() gate lives -- before it validates the body, so an empty probe stops at 422 with the authorization question already answered and the handler never run; 27 of 55 rows do exactly that, and the 20 that execute act on a throwaway temp-dir engine. The real hazard was elsewhere: a probe can destroy the session it is probing with. POST /auth/logout answers a valid administrator bearer with 200 and revokes the session, after which all 44 rows behind it answered 401 and would have landed as unexplained-unreached findings. One real side effect, 44 false ones. The fix is not a hand-kept skip list, which rots the day a route lands: the sweep re-checks the session after every write probe and re-mints it, so the result does not depend on row order and the self-destroying row is measured rather than skipped. Rebuilds are capped; over the cap, and a re-mint that fails, are exit 2. Both new passes carry their own fail-closed floors, and the bfla canary floors its GET and its write violations separately -- with one combined count a write pass that had gone blind would still be certified by its neighbour. scripts/security/route_gates.py, the shared derivation, is untouched.
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.
What this does
Extends the DAST sweep's authorized reach and BFLA passes to the write verbs. They were
GET-only. This is the last item on increment 2's list in BACKLOG #318; the rest of increment 2 continues, so this PR does not close #318.scripts/security/route_gates.py— the shared derivation the security doc-drift guard also reads — is untouched. The expectation still comes from the liverequire*()-closure walk, for write verbs as much as forGET.Re-measurement, before building
The row's claim held. Re-verified at
c57903c2by execution: the reach and BFLA passes really wereGET-only.The positive control in the same run is that non-
GETgated rows exist in quantity — 55 of 102 gated rows, and 43 of 62 BFLA candidates — soGET-only was a real restriction rather than an artifact of there being no write verbs to probe.One correction that came out of it: the route table has grown since the policy's provenance was written (105 rows to 108, 99 gated to 102, GET reach 46 to 47). Every floor still clears. The provenance block now records both measurements rather than replacing the older one.
The design answer on side effects and determinism
A write probe carries a valid privileged token and really executes. Three properties, and the third is the one that was actually load-bearing.
1. No body is ever sent. Not an absent one by accident, and deliberately not a plausible one — a plausible body maximises execution. FastAPI solves a route's dependencies (where every
require*()gate lives) before it validates the body and the path/query parameters, so a gate's 401/403 always answers before a 422. That ordering is what makes the empty probe sound rather than lucky, and it is also what stops an incidental failure manufacturing a false PASS in the BFLA pass: a 422 there means the gate did not refuse, so it scores as a violation, exactly as 404 already did.Measured on the shipped app: of 55 gated non-
GETrows, 27 stop at 422 with the handler never run, and 20 execute — 11 of those answering 404 because the path placeholder names nothing.2. The target is disposable — empty store in a temp dir, empty
Registry, no config dir, destroyed when the sweep returns. An executed handler has nothing real to act on.3. A probe can destroy the session it is probing with, and this is the part that mattered.
POST /auth/logoutanswers a valid administrator bearer with 200 and revokes the session. Measured: every one of the 44 rows after it answered 401, scored as refused, and would have landed as unexplained-unreached findings. One real side effect, 44 false ones, and the collapse of the single number this tier exists to produce.The fix is not a hand-kept skip list — that rots the day a route lands, and it would skip the rows most worth measuring. The sweep re-checks the probing identity's session after every write probe and mints a fresh one when a probe destroyed it. So:
The regression test drives this end to end with logout placed first, and carries a negative control: the pre-logout token is re-probed at the end and must answer 401. Without that control, a logout that had quietly stopped revoking sessions would produce the same green result and the test would guard a shape that cannot occur.
Fail closed, and the canaries
Both new passes carry their own floors (
min_write_rows_reached40,min_write_bfla_probes35). An absent count reads as zero and breaches the floor, so deleting a write pass exits 2 rather than turning a clean run into a cleaner-looking one.The
bflacanary now floors its GET and its write violations separately (bfla_min_write_violations). With one combined count, a write BFLA pass that had gone blind would still be certified by the GET pass beside it.Canaries remain built from supported configuration, not source patches. Nothing here is anchored to a line number. Measured: open-auth 202/204 negative failures and all 62 candidates admitted; bfla 19 GET + 40 write violations. Both exit 1.
The 8 rows that refused the administrator
Write reach measured 47 of 55. The other 8 answer 403 with
X-Step-Up-Action— an action-scoped step-up gate that a password login does not satisfy (POST /me/reauthwith a password alone also answers 403). They are entered in the policy's existingunreachable_allowlistwith that measured reason, and a new guard requires every entry there to name a row the live route table still has, so a stale exemption cannot quietly shrink the scan. Reaching them would mean the scan performing eight per-action re-auths, which changes the scanned posture materially; that is its own piece of work, not this one.Checks
Run in this worktree, on its own venv:
ruff check .— passedruff format --check .— passedmypy messagefoundry(strict) — Success: no issues found in 268 source filesmypy --strict --explicit-package-bases --namespace-packagesoverroute_gates.py,dast_target.py,dast_auth_sweep.py(the invocation ADR 0155 specifies;scripts/is outside CI's mypy scope) — passedpytest tests/test_dast_auth_sweep.py tests/test_dast_claims.py— 53 passedpytest tests/test_security_doc_drift.py tests/test_security_posture.py tests/test_security_static.py tests/test_feature_map_claims.py tests/test_nightly_notice.py— 137 passedpytest tests/test_backlog_citation_check.py tests/test_backlog_status_check.py tests/test_backlog_score_advisory.py tests/test_ledger_check.py tests/test_doc_guards_lane.py— 124 passedSkipped: the full
pytestsuite. Not run in this turn under fleet contention. The DAST sweep and its canaries run inside the existing requiredtestlegs, so a mistake here is merge-blocking for everyone — the targeted modules above plus every neighbouring guard that reads these files were run instead.CI legs somebody must read after I exit
testlegs (all four). This is where the sweep and canaries actually gate. I ran the affected modules locally but not the whole suite.dast.yml— advisory, nopull_requesttrigger, so nothing on this PR reflects it. Its next scheduled run (05:00 UTC) is the first end-to-end evidence of the write passes on a hosted runner. Aworkflow_dispatchagainstmainafter merge would get it sooner.windows-service-smokeand any other hosted-only leg — never visible to a Builder.Notes for the Lander
origin/mainat push (1 ahead, 0 behind). I did not attempt a merge or rebase — the worktree gate keys on the session's registered directory, and a sibling in this wave hadgit merge origin/mainrefused inside its own worktree.system-reminderin my context said otherwise, which is the whole test.uvicorn+httpx, both base runtime deps.GETneeds a per-probe store reset") is marked superseded and left standing, with a dated amendment carrying the current answer and AC-12 to AC-14. The two rows in backlog: two citation corrections, file #1214-#1217 #318's scoring tables that the item says are deliberately left standing were not touched.INDEPENDENCE_NOTICEconstant and ADR 0155's Scope boundary are unchanged, so the byte-identical pin holds and no third carrier was created.I do not merge, enqueue, arm auto-merge, or apply a
reviewedlabel.