Skip to content

feat(dast): extend the reach and BFLA passes to the write verbs (BACKLOG #318) - #923

Open
wshallwshall wants to merge 1 commit into
mainfrom
claude/mgr-318
Open

feat(dast): extend the reach and BFLA passes to the write verbs (BACKLOG #318)#923
wshallwshall wants to merge 1 commit into
mainfrom
claude/mgr-318

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

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 live require*()-closure walk, for write verbs as much as for GET.

Re-measurement, before building

The row's claim held. Re-verified at c57903c2 by execution: the reach and BFLA passes really were GET-only.

The positive control in the same run is that non-GET gated rows exist in quantity — 55 of 102 gated rows, and 43 of 62 BFLA candidates — so GET-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-GET rows, 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/logout answers 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 result is independent of row order, which is what makes the run repeatable;
  • a new self-destroying route is recorded in the receipt as a named rebuild instead of a wall of phantom findings;
  • rebuilds are capped by policy — over the cap is exit 2, and so is a re-mint that fails.

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_reached 40, min_write_bfla_probes 35). 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 bfla canary 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/reauth with a password alone also answers 403). They are entered in the policy's existing unreachable_allowlist with 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 . — passed
  • ruff format --check . — passed
  • mypy messagefoundry (strict) — Success: no issues found in 268 source files
  • mypy --strict --explicit-package-bases --namespace-packages over route_gates.py, dast_target.py, dast_auth_sweep.py (the invocation ADR 0155 specifies; scripts/ is outside CI's mypy scope) — passed
  • pytest tests/test_dast_auth_sweep.py tests/test_dast_claims.py53 passed
  • pytest 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.py137 passed
  • pytest 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.py124 passed
  • The sweep itself, all three modes, by hand: clean exit 0 (PASS), both canaries exit 1.

Skipped: the full pytest suite. Not run in this turn under fleet contention. The DAST sweep and its canaries run inside the existing required test legs, 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

  • The required test legs (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, no pull_request trigger, 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. A workflow_dispatch against main after merge would get it sooner.
  • windows-service-smoke and any other hosted-only leg — never visible to a Builder.

Notes for the Lander

  • Branch was level with origin/main at 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 had git merge origin/main refused inside its own worktree.
  • Ultracode: off. No system-reminder in my context said otherwise, which is the whole test.
  • No new dependency. backlog: two citation corrections, file #1214-#1217 #318's scope calls itself zero-new-dependency and it stays that way — the sweep still runs on uvicorn + httpx, both base runtime deps.
  • ADR 0155's dated increment-1 bullet ("extending past GET needs 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.
  • The INDEPENDENCE_NOTICE constant 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 reviewed label.

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant