fix(console): resend re-asserts step-up behind a body-less confirm step (BACKLOG #1227) - #597
Open
wshallwshall wants to merge 18 commits into
Open
fix(console): resend re-asserts step-up behind a body-less confirm step (BACKLOG #1227)#597wshallwshall wants to merge 18 commits into
wshallwshall wants to merge 18 commits into
Conversation
…ep (BACKLOG #1227) The console invokes the engine's uploaded-logs handler BY REFERENCE across the CoreHandlers seam, so the engine's own require_step_up Depends never runs for a /ui caller. The route stood on a premise instead of a gate -- "a same-origin POST on top of the already-stepped-up browse page" -- and nothing enforced that arrival path, so a form left open past the window still injected. REPRODUCED BEFORE FIXING, stale window, registered and running inbound: browse GET (control, gated) 303 -> /ui/reauth window really is stale resend POST 303 -> the detail page the SUCCESS shape count_messages(channel_id) 1 it injected list_audit(upload.resend) 1 THE SHAPE, decided with the dispatcher under COMMON 2.10a: a body-less confirm step mirroring delete. Stash-and-replay was refused -- it gives a message body a new location, lifetime and deletion question to save one click (CLAUDE.md s9). A bounded exception was refused -- nothing enforces the arrival path, so documenting it is a control resting on a false premise (SDS-3.7). THE ONE THING THE DESIGN GOT WRONG, and it is the reason this is not a two-line change. reauth_next puts ?index=N&to=NAME into `next`, and lookup_ui_action / is_unlock_action FULLMATCH the raw value. Every one of the 27 existing registry patterns forbids `?` via [^/?#]+, so a delete-confirm-style pattern matches nothing and dead-ends the whole flow at /ui with both parameters silently gone. The registered pattern is query-tolerant on purpose and says so. KEEP THE OPTIONAL GROUP rather than pinning \?index=\d+&to=...: the optional form also fullmatches the bare route TEMPLATE, which is what keeps the R1 coverage guard in test_webui.py able to see this entry. A stricter pattern would not match the template and would make that guard silently vacuous -- green, covering nothing. TESTS. The new test asserts non-injection AT THE STORE, not at the redirect: a 303 to /ui/reauth proves where the browser went, only the store proves the message never landed. It carries its own positive control on a fresh window (1 message, 1 audit row) so the two zeros cannot be instruments that see nothing. Mutation: reverting the gate alone reds it, and the location it then gets is the success shape. An existing test BROKE BY CONSTRUCTION and was split, not deleted: test_refused_resend_signal_survives_a_stale_step_up_window reached the refusal path by exploiting the very gap being closed. Its real subject -- a refusal stays legible on arrival -- moves to a fresh window; its positive control about the detail page stripping the flag keeps the stale window as its own test. docs/SECURITY.md said "Two console routes lose a step-up". That is now one, and leaving it would have made a security document assert a weakness that no longer exists. The narrower NEW divergence the confirm page introduces is disclosed in the same block: it cannot be step-up-gated because it IS the re-auth continuation, and it is accepted because it renders no message body. Webconsole suite 378 passed 3 skipped (was 375 + 3 added); doc-drift 41 passed; ruff format + check clean; mypy strict clean on both packages.
pytest flagged SyntaxWarning: "\?" is an invalid escape sequence -- the docstring describing resend-confirm's query-tolerant pattern used \? in a non-raw string. Escaped rather than converting the whole docstring to raw, since this is the only backslash sequence in it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
wshallwshall
enabled auto-merge (squash)
August 25, 2026 21:03
… same-origin POST on trust PR #597 built the design already decided in the 2026-08-22 handoff: the resend POST became a body-less GET behind a step-up-registered confirm page, so a stale window 303s to /ui/reauth before the send fires instead of relying on an unenforced same-origin premise. Flips the item's banner to SHIPPED and updates two passages that asserted the old gap in the present tense, since that gap no longer exists on this branch. Co-Authored-By: Claude Sonnet 5 <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
1 participant
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 console invokes the engine's uploaded-logs handler by reference across the CoreHandlers seam, so
the engine's own `require_step_up` dependency never runs for a `/ui` caller. The resend route stood
on a premise instead of a gate -- "a same-origin POST on top of the already-stepped-up browse page" --
so a form left open past the step-up window still injected. Reproduced before fixing: a stale-window
resend returned the success shape and the message actually sent.
Shape decided under COMMON 2.10a: a body-less confirm step mirroring delete, after two alternatives
were refused -- stash-and-replay (gives a message body a new location/lifetime/deletion question to
save one click) and a documented bounded exception (a control resting on a premise nothing enforces).
Also fixes a real bug the design surfaced: the confirm route needs a query-tolerant registry pattern
(the action's two parameters ride the query), which none of the 27 existing patterns supported --
`lookup_ui_action`/`is_unlock_action` fullmatch the raw value and forbid `?` by default. Kept the
optional-query form rather than pinning it, so the R1 coverage guard in `test_webui.py` still sees
this entry via the bare template.
An existing test broke by construction and was split rather than deleted:
`test_refused_resend_signal_survives_a_stale_step_up_window` had been reaching the refusal path by
exploiting the exact gap this closes. Its real subject (a refusal stays legible on arrival) moves to a
fresh window; the stale-window/detail-page-strips-the-flag behavior stays as its own test.
`docs/SECURITY.md` said "two console routes lose a step-up" -- now one, corrected in the same commit
so the security doc doesn't assert a weakness that no longer exists.
Cherry-picked from a rescued branch (private remote, no PR) onto current main -- clean, zero
conflicts (confirmed with `git merge-tree`), exact 9-file/+315/-40 match to what was reported handed
off. Fixed one thing beyond the original content: a docstring in `_auth.py` used `?` in a non-raw
string, which pytest flagged as SyntaxWarning: invalid escape sequence -- escaped it properly.
Verified locally: 60/60 across `test_uploaded_logs_ui.py` + `test_security_doc_drift.py`, ruff
check + format clean on all three changed source files.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com