test(webconsole): prove the __Host- prefix holds on every startable topology, and set the cookie through the one resolver (BACKLOG #1118) - #868
Merged
Conversation
…claims (BACKLOG #1118) The prefix already holds. Re-measured at HEAD across every startable topology, so this adds no cookie behaviour -- it closes the gap between that being true and anything being able to show it. test_the_served_https_scheme_resolves_both_cookies_to_their_host_twins handed "https" in as a literal while its docstring claimed a change reintroducing a cleartext default would fail it. The body never touched the serve path, so the claim named a guarantee the instrument did not provide (SDS-3.8). It now derives the scheme from ensure_api_tls_material, the same predicate the serve path uses to decide whether uvicorn gets an ssl_context_factory. Adds the arm nothing covered: the tls_terminated_upstream topology. The engine mints nothing there, so the wire scheme is http and every scheme-keyed reading says "bare name", while the correct answer is the prefixed one -- effective_https reaches it only through its exposure_protected disjunct. That disjunct is a deletion magnet now the default mints TLS, and dropping it would revert both cookies to bare names on the one topology still reaching the app over cleartext, with the rest of the suite staying green. Mutation-checked, worktree tree pinned. Deleting the disjunct leaves the shipped-default test GREEN and turns the new proxy test RED; reverting the mint turns the default test RED; minting in the proxy topology turns the proxy test RED. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ver (BACKLOG #1118) session_cookie_name calls itself "the ONE resolver every set/clear/read site threads through, so the name a response writes and the name a later request reads always agree". The clear and read sites did. The two SET sites recomputed the same expression inline, so the guarantee the docstring asserted did not structurally exist -- a compensating control resting on a false premise (SDS-3.7). Behaviour is unchanged: the two expressions agreed, so nothing was ever wrong on the wire. What is closed is an edit to either copy alone. The split it would produce is silent and total -- the browser holds the name the set wrote, session_token asks for the name the resolver returns, finds nothing, and the operator is bounced back to login with no error naming a cause. `secure` still gets its own effective_https call rather than being read off the name. Those are different conjuncts and #1117 forbids inferring one from the other. Found by mutation: keying the resolver on the wire scheme alone left the whole console suite green before this change, because the set sites did not consult it. It now goes red on the upstream-terminator posture. Adds the parametrised set-name-equals-read-name test across four postures with a vacuity control, plus the emitted-name arm for the declared proxy topology, whose wire scheme is http while the browser's origin is https. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ct it found The premise stays dead and no cookie behaviour needed changing. Re-verified on a later HEAD with a topology table covering every startable posture, and with both packages checked to resolve inside this worktree rather than assumed -- a first run silently imported the primary checkout. Records the three evidence gaps this pass closed: a test that recorded the measurement without performing it, no assertion anywhere on the cookie name in the declared-proxy topology, and the set sites carrying a second copy of the resolver's expression. Banner untouched. The closing act is the vault re-score, which this Builder must not write, so the item stays open by construction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
LABEL DISCLOSURE, per the Manager playbook section 6. I applied the What the label does and does not represent here:
Label timing was verified on three reads rather than on the label being present: the label event post-dates the head commit, and the review-gate run is attempt=1 on a pull_request event, so it is not replaying a frozen payload. |
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.
BACKLOG #1118 -- ASVS 3.3.3, the
__Host-cookie prefixThe premise stayed dead, and no cookie behaviour needed changing. The evidence for that had three gaps, and closing them turned up one real defect.
ADR 0172 already did this item's work: the engine always serves TLS, so the shipped default is https and both cookies resolve to their
__Host-twins with no cookie-code change. PR #782 recorded that. This pass re-verified it on a later HEAD and found that the tests recording it did not measure it.What I measured
messagefoundryandmessagefoundry_webconsoleboth resolving inside this worktree, checked rather than assumed -- a first run silently imported the primary checkout, becausesys.path[0]is the script's own directory.effective_httpshttps__Host-twinstls_terminated_upstream+trusted_proxies)httpexposure_protected__Host-twinstls_terminated_upstreamwithouttrusted_proxiesValidationErrorThe prefix already holds on every startable topology. The
tls_terminated_upstreampath is untouched and still mints nothing.The three gaps
The test that recorded the measurement did not perform it.
test_the_served_https_scheme_resolves_both_cookies_to_their_host_twinshanded"https"in as a literal while its docstring claimed a change reintroducing a cleartext default would fail it. The body never touched the serve path -- SDS-3.8, an instrument answering an adjacent question. It now derives the scheme fromensure_api_tls_material, the same predicate the serve path uses to decide whether uvicorn gets anssl_context_factory.Nothing anywhere asserted the cookie NAME on the declared-proxy topology -- the one ADR 0172 excludes, and the only startable posture still reaching the app over a cleartext wire. Every scheme-keyed reading of it says "bare name"; the correct answer is the prefixed one, because the browser's origin is https and
effective_httpsgets there only through itsexposure_protecteddisjunct, which now reads as redundant.The defect: both SET sites recomputed the cookie name inline instead of calling the shared resolver.
session_cookie_namedescribes itself as "the ONE resolver every set/clear/read site threads through, so the name a response writes and the name a later request reads always agree". The clear and read sites did.set_session_cookieandset_oidc_flow_cookiecarried a second copy of the expression, so the asserted guarantee did not structurally exist -- SDS-3.7. Nothing was wrong on the wire, because the two expressions agreed. What was missing was any barrier to an edit to one of them.Both set sites now call the resolver.
securestill takes its owneffective_httpscall, because those are different conjuncts and #1117 forbids inferring one from the other. All four cookie write sites in the two packages are in_auth.pyand all four now thread through a resolver.Stated in the conditional, per section 0 (zero deployments): a site deploying after such an edit would have the browser hold the name the set wrote while
session_tokenasked for the name the resolver returned. It would find nothing, and the operator would be bounced back to login forever with no error naming a cause.Negative-control evidence
A guard nobody can show firing is not evidence. Each mutation is the realistic refactor the test exists to stop, applied to the real source, with the unmutated tree as the control.
assert material is not Noneexposure_protecteddisjunct fromeffective_httpsSecure; default test stays GREENupstream-terminatorparametrisation REDis NoneassertBoth directions are covered, as asked: the prefixed name under effective https, and a genuinely plaintext bind still logging in. The plaintext login arms already existed and still pass --
test_http_cookie_is_byte_identicalandtest_loopback_http_engages_headers_but_keeps_plain_cookie, the latter asserting the plain cookie authenticates the dashboard. The new tests add the bare-name negative control on a cleartext wire with no declaration.Files touched under
messagefoundry_webconsole/messagefoundry_webconsole/_auth.py-- 11 lines, only the twoset_*name expressions plus their comments.__init__.pyuntouched.packaging/messagefoundry-webconsole/tests/test_ui_hardening.py-- additive; nothing reformatted or reflowed.Also
tests/test_api_tls.pyanddocs/BACKLOG.md(its own commit, last).Checks run
Interpreter:
C:\Users\Scott\Code\MessageFoundry\.venv\Scripts\python.exe, tree proof above.ruff check .-- All checks passedruff format --check .-- 1241 files already formattedmypy messagefoundry-- Success, 267 source filesmypy messagefoundry_webconsole-- Success, 33 source filespytest tests/test_api_tls.py packaging/messagefoundry-webconsole/tests/test_ui_hardening.py-- 105 passedpytest packaging/messagefoundry-webconsole/tests/test_webui.py-- 252 passed (12m36s)pytest packaging/messagefoundry-webconsole/tests/test_ui_csp_canary.py-- 22 passed, 3 skipped (vaulted runbook absent)pytest packaging/messagefoundry-webconsole/tests/test_ui_session_watchdog.py tests/test_threat_model_doc_drift.py-- 36 passed, 89 skipped (vaulted doc absent)pytest tests/test_auth_oidc.py tests/test_auth_oidc_http.py-- 80 passedscripts/docs/backlog_status_check.py-- OK, 664 items; #1118 parsed and confirmed still OPENSkipped deliberately: the full suite. Many live sessions share this box and
tests/test_connscale_smoke.pyhard-codes a 24-port block (#1014). Every test file referencing the cookie set path was run -- located by grep overtests/andpackaging/messagefoundry-webconsole/tests/forset_session_cookie|set_oidc_flow_cookie|mf_session|mf_oidc_flow.Legs only a hosted runner sees must be read after my process exits:
windows-service-smokeand the CodeQL matrix.What this deliberately does not do
__Host-rename. The twin already exists; deleting the conditional satisfies every grep while the browser drops theSet-Cookieon a plaintext bind.Still open, and it is not engine work
Whether Chrome, Firefox and Safari STORE a
__Host-cookie over https served with an untrusted self-signed chain. That is now load-bearing for the shipped default rather than hypothetical, since the shipped default is exactly that chain.Assumptions I made without asking
🤖 Generated with Claude Code