From 66b12c933d1543510805c3f5511cfe99580f1834 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 3 Sep 2026 16:39:01 -0500 Subject: [PATCH] fix(webconsole): report the browser-hardening opt-out at start (BACKLOG #1118) ADR 0172 dissolved this item's premise. The engine always serves TLS, so effective_https holds on the shipped default and both cookies already resolve to their __Host- twins with no cookie-code change. Measured, with controls, and recorded in the item. What is left is the one residual the item raised but did not grade: MEFOR_WEBCONSOLE_DISABLE_BROWSER_HARDENING reverted the cookies to their bare names and nothing at start said so. The serve ladder now names the variable, the two unprefixed names, and the fact that Secure is not downgraded. This is a report, not a control. Nothing here re-scores the cell on a warning. Also pins the trap the research warned about: a minted certificate must not make exposure_protected true, or a self-signed placeholder would satisfy the off-loopback /ui gate. Co-Authored-By: Claude Opus 5 --- docs/BACKLOG.md | 21 +++++++++- messagefoundry/__main__.py | 33 ++++++++++++++++ messagefoundry_webconsole/__init__.py | 4 ++ tests/test_api_tls.py | 57 +++++++++++++++++++++++++++ tests/test_cli.py | 48 ++++++++++++++++++++++ 5 files changed, 162 insertions(+), 1 deletion(-) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 4b71b3902..7b8fc4cb2 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -407,7 +407,7 @@ Ordered by value descending, then difficulty ascending (cheapest first at equal | 56 | **#1008** | Startup preflight on the store principal's effective privileges (ASVS 13.2.2) | 6 | 4 | _quick win_ | DEMAND-GATE | Gap stands in shipped code: no privilege probe exists in the four scanned packages beyond the two conditional-DDL guard hits at sqlserver.py:924/:931, and settings.py:514 gates credential kind only, so a sysadmin login would go unobserved on first deployment (value 6). The remainder is a probe across SQL Server plus Postgres with SQLite exempt, a settings plus serve gate, the still-underived Postgres grant set, and a paired out-of-repo scorecard edit, which is the difficulty-4 anchor of a feature across a seam exercised on all three backends. | | 57 | **#1085** | Rule 3c discards a `cd` prefix and resolves a relative `-C` against the session cwd, so it refuses a write aimed at an ungoverned repo | 6 | 4 | _quick win_ | P2 | The defect is intact: worktree_gate.ps1:319 takes the -C value and the cd resolution at :321-330 is the else branch, so a relative -C behind a cd prefix resolves against the session cwd and the deny names the wrong repository. Value 6 is rung 6 -- a live false deny on developer tooling whose only workaround is a human overriding a message that actively misinforms. Difficulty 4, not 5, because the change is hoisting the existing cd computation above the -C branch and joining a relative -C to it inside one function, and the deny-text assertion the scorer priced as extra cost is already a shipped test capability. | | 58 | **#1107** | research an honest pass for ASVS 1.2.2 -- what context-encoding the dynamic-URL surface actually needs, beyond the one FHIR search path the residual names | 6 | 4 | _quick win_ | P2 | Value drops from the banner because the cell's named residual is retired in shipped code, but stays at 6 because 1.2.2 is scored across every dynamically built URL and the surface is still unenumerated: 24 transport modules exist and only some have been read, with dicomweb.py:283-296 showing a derived target URL that is quoted but was never assessed under this cell. Difficulty 4 is the enumeration plus an adversarial re-verify of a cell that has never had one, with the code fix already landed elsewhere. | -| 59 | **#1118** | research an honest pass for ASVS 3.3.3 -- keeping the __Host- prefix on the shipped default without breaking a cleartext loopback login | 6 | 4 | _quick win_ | P2 | Verified at HEAD: both cookies resolve to their __Host- twin only under effective_https AND browser_hardening_enabled (messagefoundry_webconsole/_auth.py:739 and :815), HOST_COOKIE_NAME at :70 documents that cleartext keeps the bare name by design, and the opt-out env at :77 reverts to the bare name with nothing at start reporting the reversion. Difficulty 4 covers a research pass that has to settle a loopback-TLS or auto-TLS posture question for the default /ui bind rather than edit cookie code, with cannot-honestly-pass a valid outcome. | +| 59 | **#1118** | research an honest pass for ASVS 3.3.3 -- keeping the __Host- prefix on the shipped default without breaking a cleartext loopback login | 6 | 4 | _quick win_ | P2 | Verified at HEAD: both cookies resolve to their __Host- twin only under effective_https AND browser_hardening_enabled (messagefoundry_webconsole/_auth.py:739 and :815), HOST_COOKIE_NAME at :70 documents that cleartext keeps the bare name by design, and the opt-out env at :77 reverts to the bare name with nothing at start reporting the reversion. Difficulty 4 covers a research pass that has to settle a loopback-TLS or auto-TLS posture question for the default /ui bind rather than edit cookie code, with cannot-honestly-pass a valid outcome. CORRECTED 2026-09-03: that verification is stale. ADR 0172 landed the auto-TLS posture, so effective_https now holds on the shipped default and both cookies resolve to their __Host- twins with no cookie-code change; what is left is the vault re-score, which no engine PR can perform. | | 60 | **#1152** | research an honest pass for ASVS 8.2.2 -- object-level authorization on the uploads family, which carries PHI and no owner check | 6 | 4 | _quick win_ | P2 | The PHI-bearing half is fixed in shipped code, so the cross-operator enumeration, the content oracle and the two-step injection path are gone (app.py:3749, :3941-3971). What remains is the principal-narrowing axis nothing else closes -- identity.py:38 still defaults to every channel -- plus the pageless GET /uploads against the master-test-plan clause, with per-user scoping available as an awkward workaround (value 6). The remainder is a default flip with its first-run console handling plus pagination across API and console, tested but not touching the store (difficulty 4). | | 61 | **#1159** | research an honest pass for ASVS 10.2.3 -- least-privilege OAuth scopes the engine never validates | 6 | 4 | _quick win_ | P2 | Gap stands unchanged: both scope settings reach the wire through a bare str() at smart.py:305 and http_auth.py:305 with no cross-check against the connector's declared interaction, checks.py carries no scope rule, and the wildcard example survives in at least five artifacts including the shipped wiring sample at wiring.py:531 (value 6). The remainder prices at 4: a research call on whether required scope is derivable, then an additive advisory check on the established checks.py CheckResult seam (four such checks already at :219/:253/:292/:424) plus doc narrowing, with a refusing gate ruled out. | | 62 | **#1171** | research an honest pass for ASVS 11.4.1 -- SHA-1 TOTP and a keyed BLAKE2b de-identification seed | 6 | 4 | _quick win_ | P2 | Half the item is gone -- totp.py:84-85 confirms the SHA-256 cutover with the algorithm string derived so the two cannot drift -- and the cell's own last_verified of 2026-08-16 postdates that cutover while still reading partial, which independently confirms the remainder is the non-TOTP limb. Value 6 on that remainder: an L1 requirement short of pass on shipped code, one site rather than two, with the user-facing authenticator-interop question retired. Difficulty 4 because the worst case is a two-file swap plus the inventory registration at crypto_inventory_check.py:104 and :348, ahead of which sits the cheap prior question of whether a de-identification seed is a cryptographic use the verb reaches. | @@ -7723,6 +7723,25 @@ filing. **Proposed work, by subject, all unallocated:** the first-run mint-and-serve work (filed as #1276, not started); the generated-and-serving state that preserves operator precedence without letting a self-signed certificate satisfy the off-loopback exposure gate; the refuse-on-exposure amendment scoped to the JSON API socket rather than presented as this cell's lever; client scheme agreement across tray, `apiclient`, harness and the IDE; rewriting the five DEV-ONLY self-signed prohibitions in place (`pki.py:141-143` plus `__main__.py:578`, `:3642`, `:3677`, `:3689`), recording who overruled them and when; a browser measurement of whether a `__Host-` cookie is STORED over https served with an untrusted self-signed chain, in Chrome, Firefox and Safari, recorded beside the existing http-loopback table; silent-failure detection for the undeclared-proxy topology, which after this build fails closed at login with no error, hung off the existing heuristic at `__main__.py:1894-1905`; and joint re-verification with the Secure-attribute sibling item, since one transport change carries both. +**Re-measured 2026-09-03. THE PREMISE IS DEAD: the work this item proposed has already landed under #1276, and both cookies now carry the `__Host-` prefix on the shipped default with no cookie-code change.** ADR 0172 (Accepted 2026-08-22) makes the engine always serve TLS, minting a self-signed pair on first run. Measured against this worktree's own tree, with `import messagefoundry_webconsole` resolving inside it: + +| What was measured | Reading | Control | +|---|---|---| +| `ensure_api_tls_material(ApiSettings())` | mints a pair, cert and key both on disk | the `tls_terminated_upstream` topology returns `None`, so the proxy's own hop is not broken | +| `effective_https` over the served scheme | `True` at scheme `https` | `False` at scheme `http`, the pre-0172 default | +| `session_cookie_name` | `__Host-mf_session` | `mf_session` over `http` | +| `oidc_flow_cookie_name` | `__Host-mf_oidc_flow` | `mf_oidc_flow` over `http` | + +That covers every startable topology, not just the loopback default. The one posture that still reaches the app over cleartext is a declared upstream terminator, and settings validation forces `trusted_proxies` with it, so `exposure_protected` is true there and `effective_https` holds anyway. There is no switch that turns the API socket back to cleartext. + +**The trap this item warned about was avoided in the shipped code, and it is now pinned by a test.** The research said a generated certificate must not surface through `[api].tls_cert_file`, because `tls_enabled` is `bool(tls_cert_file)` and would flip `exposure_protected` open on every bind. `__main__.py` copies the minted paths into a local `model_copy` that uvicorn reads and no gate does, so `ApiSettings().exposure_protected` stays `False` while the socket serves https. `tests/test_api_tls.py::test_a_minted_pair_does_not_satisfy_the_off_loopback_exposure_gate` holds that, with an operator-chain control. + +**What this PR builds, and what it deliberately does not.** It does not build the cookie work, because there is none left to build. It builds the one residual this item raised but did not grade: the `MEFOR_WEBCONSOLE_DISABLE_BROWSER_HARDENING` opt-out was reported nowhere at start, so an operator who set it, or inherited it from a service environment, got a quietly weaker console with no signal. The serve ladder now names it, says which unprefixed names the cookies revert to, and says Secure is not downgraded. **This is a report, not a control, and the 2026-08-17 ruling still holds: nothing here re-scores the cell on a warning.** It matters more since ADR 0172 than before it, for the plain reason that this env is now the only remaining way a deployment would lose the browser-enforced host binding. + +**Anchor drift, for whoever re-reads this item.** The two resolution sites moved from `:739` and `:815` to `session_cookie_name` and `oidc_flow_cookie_name`; cite them by name, because a sibling PR on #1117 is editing the same file's clear path and any line number recorded today goes stale on its merge. + +**What is left is not a build.** The re-score is the closing act and it lives in the separate vault repository, which this item's Builder must not edit. Whoever holds that record can now read a measurement rather than a plan. The residual disclosures the research listed still belong on it and are unchanged by this pass: on a pure loopback bind the change is close to behaviourally inert since `http://127.0.0.1` was already a secure context, `__Host-` binds to the host so a co-resident `127.0.0.1:*` origin can still shadow by port, and nobody has yet measured whether a browser STORES a prefixed cookie over https served with an untrusted self-signed chain. That last one is the only open measurement, and it is a browser test, not engine work. + ## 1119. research an honest pass for ASVS 3.4.1 -- HSTS on all responses from an engine whose shipped bind is plaintext loopback > ๐Ÿ”ข **Re-scored 2026-08-20 -> P3.** Value **4/10** ยท Difficulty **2/10** ยท _fill-in_. The remainder lands no code: it is a scoping ruling on the honest unit of assessment plus at most a documentation and scorecard change, on an L1 cell with no PHI effect. Difficulty 2 because the coverage construction and the single shared hsts_applies definition already exist. _(was 6/10 ยท 4/10.)_ diff --git a/messagefoundry/__main__.py b/messagefoundry/__main__.py index 04475589d..df26b917e 100644 --- a/messagefoundry/__main__.py +++ b/messagefoundry/__main__.py @@ -1963,6 +1963,39 @@ def _serve(args: argparse.Namespace) -> int: file=sys.stderr, ) + # BACKLOG #1118: REPORT THE BROWSER-HARDENING OPT-OUT AT START. The #192 hardening defaults ON and + # the env below reverts it, and until this arm the reversion was reported NOWHERE โ€” the predicate + # was read only from `_auth.py` and `_security.py`, both per-request, so an operator who set it (or + # inherited it from a service environment) got a quietly weaker console with no signal at all. + # + # WHY IT MATTERS MORE SINCE ADR 0172, not less: the engine now always serves TLS, so `effective_https` + # holds on the shipped default and both cookies resolve to their `__Host-` twins. This env is + # therefore the ONLY remaining way a default deployment loses the browser-enforced host binding. + # Before 0172 a cleartext bind lost it too, which made this one signal among several; now it is the + # signal. + # + # A WARNING IS NOT A CONTROL, and this arm is deliberately not offered as one (owner ruling + # 2026-08-17: a warning earns nothing by itself). It reports an operator's explicit choice; it does + # not gate, refuse, or re-enable anything. Imported from the console package root rather than its + # private `_auth` module, and reached only when serve_ui survived the find_spec gate above, so the + # wheel is present by construction. + if settings.api.serve_ui: + from messagefoundry_webconsole import ( + BROWSER_HARDENING_OPT_OUT_ENV, + browser_hardening_enabled, + ) + + if not browser_hardening_enabled(): + print( + f"warning: {BROWSER_HARDENING_OPT_OUT_ENV} is set โ€” the /ui browser hardening is OFF " + "for this run. The session and OIDC flow cookies revert to their unprefixed names " + "(mf_session / mf_oidc_flow), losing the browser-enforced '__Host-' host binding, and " + "the per-response nonce CSP, COOP and CSP reporting are not emitted. Transport " + "security is NOT downgraded: Secure is still set over https. Unset this variable to " + "restore the secure-by-default posture.", + file=sys.stderr, + ) + # THE SINGLE DEFINITION OF "this instance is exposed" (BACKLOG #326) is derived above, before the # auth-off arm (BACKLOG #1013) that also consumes it, from two fields no earlier arm reassigns โ€” # `is_loopback` and `tls_terminated_upstream` are read straight off the loaded config and are never diff --git a/messagefoundry_webconsole/__init__.py b/messagefoundry_webconsole/__init__.py index fc3c2b29f..4bff7afe9 100644 --- a/messagefoundry_webconsole/__init__.py +++ b/messagefoundry_webconsole/__init__.py @@ -72,6 +72,7 @@ def assert_engine_seam(engine_seam: str) -> None: # and ``messagefoundry_webconsole.pages``. Both are leaf modules (no cycle with :mod:`.mount`). from . import pages # noqa: E402 from ._auth import ( # noqa: E402 + BROWSER_HARDENING_OPT_OUT_ENV, COOKIE_NAME, UI_CSP, WEBAUTHN_EXTRA_MISSING_NOTICE, @@ -81,6 +82,7 @@ def assert_engine_seam(engine_seam: str) -> None: assert_not_cross_site, assert_same_origin, authorize_ui_ws, + browser_hardening_enabled, clear_session_cookie, is_safe_ui_action, is_unlock_action, @@ -99,6 +101,7 @@ def assert_engine_seam(engine_seam: str) -> None: from .mount import mount_ui # noqa: E402 __all__ = [ + "BROWSER_HARDENING_OPT_OUT_ENV", "COOKIE_NAME", "STATIC_DIR", "SUPPORTED_ENGINE_SEAMS", @@ -113,6 +116,7 @@ def assert_engine_seam(engine_seam: str) -> None: "assert_not_cross_site", "assert_same_origin", "authorize_ui_ws", + "browser_hardening_enabled", "clear_session_cookie", "is_safe_ui_action", "is_unlock_action", diff --git a/tests/test_api_tls.py b/tests/test_api_tls.py index 9d8aef3fa..6461fde0c 100644 --- a/tests/test_api_tls.py +++ b/tests/test_api_tls.py @@ -9,6 +9,7 @@ import ssl from collections.abc import Awaitable, Callable from pathlib import Path +from types import SimpleNamespace from typing import Any import httpx @@ -1620,3 +1621,59 @@ def test_the_minted_pair_builds_a_serving_context(tmp_path: Path) -> None: serving = api.model_copy(update={"tls_cert_file": cert, "tls_key_file": key}) ctx = build_api_ssl_context(serving) assert ctx.minimum_version is ssl.TLSVersion.TLSv1_2 + + +# --- BACKLOG #1118 / ASVS 3.3.3: what always-serve-TLS did to the `__Host-` cookie prefix --------- + + +def test_a_minted_pair_does_not_satisfy_the_off_loopback_exposure_gate(tmp_path: Path) -> None: + """THE TRAP, pinned. A generated certificate must never make `exposure_protected` true. + + `tls_enabled` is literally `bool(tls_cert_file)` and `exposure_protected` is + `tls_enabled or (tls_terminated_upstream and trusted_proxies)`. So surfacing the minted pair + through `[api].tls_cert_file` would flip the off-loopback /ui refusal open on EVERY bind, and a + self-signed placeholder with no chain of trust would silently satisfy a gate written for an + operator chain. The serve path avoids it by copying the paths into a LOCAL `model_copy` that + uvicorn reads and no gate does; this test is what stops a later refactor from "simplifying" that + copy back into `settings.api`. + """ + api = ApiSettings() + cert, key = ensure_api_tls_material(api, state_dir=tmp_path) + assert Path(cert).exists() # it really did mint -- otherwise the assertions below are vacuous + assert not api.tls_enabled + assert not api.exposure_protected + # POSITIVE CONTROL: the same two properties DO flip for an operator-supplied chain, which is the + # posture the gate was written for. Without this arm the assertions above would also pass if + # `exposure_protected` were broken to a constant False. + operator = api.model_copy(update={"tls_cert_file": cert, "tls_key_file": key}) + assert operator.tls_enabled and operator.exposure_protected + + +def test_the_served_https_scheme_resolves_both_cookies_to_their_host_twins() -> None: + """The #1118 premise measurement, made durable. + + The item was filed to research how to reach ASVS 3.3.3 without breaking a cleartext loopback + login. ADR 0172 removed the cleartext loopback bind, so the request scheme is https and the + cookie code that was already correct now resolves both names to their `__Host-` twins with no + edit at all. Pinning it here means a change that reintroduces a cleartext default fails a test + naming the consequence, instead of quietly moving the cell back to partial. + + The http arm is the positive control AND the reason an unconditional rename is wrong: a browser + rejects a `__Host-` cookie that is not Secure, so over cleartext the bare name is the correct + answer rather than a weaker one. + """ + from messagefoundry_webconsole._auth import oidc_flow_cookie_name, session_cookie_name + + def conn(scheme: str) -> Any: + return SimpleNamespace( + app=SimpleNamespace(state=SimpleNamespace(exposure_protected=False)), + url=SimpleNamespace(scheme=scheme), + ) + + served = conn("https") # what ADR 0172 puts on the wire by shipped default + assert session_cookie_name(served) == "__Host-mf_session" + assert oidc_flow_cookie_name(served) == "__Host-mf_oidc_flow" + + cleartext = conn("http") # POSITIVE CONTROL -- the pre-0172 default, still correct behaviour + assert session_cookie_name(cleartext) == "mf_session" + assert oidc_flow_cookie_name(cleartext) == "mf_oidc_flow" diff --git a/tests/test_cli.py b/tests/test_cli.py index b265a8996..7f7770d96 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1614,6 +1614,54 @@ def test_serve_ui_default_on_loopback_mounts_ui( assert "refusing to serve the browser ops dashboard" not in capsys.readouterr().err +def _bare_loopback_serve(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> int: + """A bare loopback `serve` โ€” the shipped default, with uvicorn and the app build stubbed out.""" + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("MEFOR_STORE_ENCRYPTION_KEY", "x" * 44) + (tmp_path / "messagefoundry.toml").write_text( + "security.handles_real_patient_data = false\n", encoding="utf-8" + ) + monkeypatch.setattr("messagefoundry.api.create_managed_app", lambda **kw: object()) + monkeypatch.setattr("uvicorn.run", lambda *a, **k: None) + return main(["serve", "--config", str(SAMPLES_CONFIG), "--env", "dev"]) + + +def test_browser_hardening_opt_out_is_reported_at_start( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """BACKLOG #1118: setting the opt-out must SAY so at start, naming what reverts. + + The predicate was read only from `_auth.py` and `_security.py`, both per-request, so an operator + who set this env โ€” or inherited it from a service environment โ€” got a quietly weaker console with + no signal anywhere. Since ADR 0172 made the engine always serve TLS this is the only remaining + way a default deployment loses the `__Host-` binding, which is what makes the silence worth + fixing. The message must name the env, the two unprefixed cookie names, and the fact that Secure + is not downgraded, because an operator who reads only "hardening off" cannot tell which of those + three things happened. + """ + monkeypatch.setenv("MEFOR_WEBCONSOLE_DISABLE_BROWSER_HARDENING", "1") + assert _bare_loopback_serve(tmp_path, monkeypatch) == 0 + err = capsys.readouterr().err + assert "MEFOR_WEBCONSOLE_DISABLE_BROWSER_HARDENING is set" in err + assert "mf_session / mf_oidc_flow" in err # the names it reverts TO + assert "__Host-" in err # what is lost + assert "Secure is still set over https" in err # what is NOT lost + + +def test_browser_hardening_default_reports_nothing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """Positive control for the arm above: on the shipped default the warning must be ABSENT. + + Without this, a warning that fired unconditionally would pass the test above while telling every + operator their console is weakened. `delenv(raising=False)` because the arm keys on the env being + absent, and a leaked value from another test would make this pass for the wrong reason. + """ + monkeypatch.delenv("MEFOR_WEBCONSOLE_DISABLE_BROWSER_HARDENING", raising=False) + assert _bare_loopback_serve(tmp_path, monkeypatch) == 0 + assert "MEFOR_WEBCONSOLE_DISABLE_BROWSER_HARDENING" not in capsys.readouterr().err + + def test_serve_ui_explicit_offloopback_still_refuses( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: