diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 83bef1c4..5649b670 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -4997,6 +4997,19 @@ a file, which is the fact the whole finding rests on. > **AMENDED 2026-08-14 (lander) -- THE IMPLEMENTATION LANDED IN THIS PR; THE STATUS BANNER IS DELIBERATELY UNCHANGED.** `public_origin` is now required in the declared-terminator PHI enforce posture, which makes the comment at `:2113-2114` true **by construction** rather than by convention. A negative control was run and still discriminates: reverted, the console-OFF case fails while console-ON and non-PHI pass. **Closing this item is a judgement about completeness, not a mechanical consequence of the code landing, so it is left to the ledger seats rather than taken by the seat that merely supplied the paired commit.** > Verdict: build > Closing-act: code +> **PROGRESS 2026-08-26 (lander) -- THIS PR CLOSES THE REMEDIATION-STRING LIMB THIS ITEM'S OWN +> RE-SCORE FLAGGED.** The re-score above names the exact gap: the refusal this item's own fix added +> tells an operator to set `[api].public_origin`, which ADR 0118 relocated and the loader now rejects +> at load, so the remediation fails on the operator's next start. This PR renames the instruction to +> `[security].web_console_public_address` and pins the assertion against `_RELOCATED_TO_SECURITY` +> itself rather than a string literal, so a future relocation cannot drift the two apart silently +> again. **A second instance of the identical defect, found by this branch's own test:** an earlier +> `serve_ui`-gated refusal carries the same stale key name and is fixed alongside. +> **Deliberately incomplete, stated rather than hidden:** the matching `docs/CONFIGURATION.md` +> paragraph is not in this PR -- the collision gate correctly refused it because Builder 2 holds +> uncommitted changes to that file, so it was asked for rather than overridden, and follows +> separately. **Stays OPEN once this lands too**, per this item's own stated design: closing it is a +> judgement about completeness, not a mechanical consequence of a landed commit. **Cluster:** Security / startup gates, ASVS 12.1.1. **Priority:** P2. **Verdict:** build (small). **Severity:** would leave an ASVS 12.1.1 control silently inert in a legitimate deployment posture on first deployment — the TLS floor of the terminator in front of a PHI API would go unmeasured, with nothing reporting the skip. **The gate, measured at `e0482aea`.** `messagefoundry/__main__.py` runs the probe under: diff --git a/messagefoundry/__main__.py b/messagefoundry/__main__.py index da5062d1..c91ef533 100644 --- a/messagefoundry/__main__.py +++ b/messagefoundry/__main__.py @@ -1884,13 +1884,17 @@ def _serve(args: argparse.Namespace) -> int: # DECLARED reverse proxy the request Host header is client-forwardable — without the # exact origin, the /ui same-origin CSRF check degrades to Host comparison and the # WebAuthn rp_id would have anchored to attacker-influenceable input. + # Names the OPERATOR-FACING key (BACKLOG #1026), for the same reason as the ASVS 12.1.1 + # refusal further down: ADR 0118 relocated `[api].public_origin` to + # `[security].web_console_public_address` and REJECTS the old spelling as file or env + # input, so an instruction to set it fails at load. print( - "error: [api].serve_ui with [api].tls_terminated_upstream requires " - '[api].public_origin (e.g. "https://mefor.example.org") — behind a declared ' + "error: serving the web console behind a declared TLS terminator requires an " + 'external origin (e.g. "https://mefor.example.org") — behind a declared ' "reverse proxy the Host header is client-forwardable, so the browser console's " "same-origin CSRF check and the WebAuthn passkey origin binding need the exact " - "external origin. Set [api].public_origin to the origin the browser uses. See " - "docs/security/OFF-LOOPBACK-DEPLOYMENT.md (ADR 0068).", + "external origin. Set [security].web_console_public_address to the origin the " + "browser uses. See docs/security/OFF-LOOPBACK-DEPLOYMENT.md (ADR 0068).", file=sys.stderr, ) return 2 @@ -2153,12 +2157,23 @@ def _serve(args: argparse.Namespace) -> int: and enforcing and not settings.api.public_origin ): + # THE REMEDIATION NAMES THE KEY THE LOADER ACCEPTS, NOT THE FIELD THIS CODE READS + # (BACKLOG #1026). `[api].public_origin` is the INTERNAL field; ADR 0118 relocated the + # operator-facing key to `[security].web_console_public_address` and REJECTS the old + # spelling as file or env input (`_RELOCATED_TO_SECURITY` in config/settings.py). So the + # refusal this block shipped with handed an operator a remediation that fails at load: do + # what it says and the next start dies on "unrecognized config key(s)". + # + # A hard refusal that names an unusable fix is worse than one that names none -- it costs a + # restart cycle to discover, and it reads as authoritative because it is coming from the + # gate itself. tests/test_api_tls.py pins the remediation string AGAINST the relocation map + # so the two cannot drift apart again. print( f"error: refusing to serve on a PHI instance ({env_name!r}) behind a declared TLS " - "terminator under `enforce` without [api].public_origin — the ASVS 12.1.1 TLS-floor " + "terminator under `enforce` without an external origin — the ASVS 12.1.1 TLS-floor " "probe dials that origin, so leaving it unset silently disables the check rather than " - 'failing it. Set [api].public_origin to the origin the browser uses (e.g. "https://' - 'mefor.example.org"). See docs/security/OFF-LOOPBACK-DEPLOYMENT.md.', + "failing it. Set [security].web_console_public_address to the origin the browser uses " + '(e.g. "https://mefor.example.org"). See docs/security/OFF-LOOPBACK-DEPLOYMENT.md.', file=sys.stderr, ) return 2 diff --git a/tests/test_api_tls.py b/tests/test_api_tls.py index f013e424..b8b3083f 100644 --- a/tests/test_api_tls.py +++ b/tests/test_api_tls.py @@ -1443,7 +1443,10 @@ def test_phi_behind_a_declared_terminator_refuses_without_public_origin_console_ _posture_probe_toml(tmp_path, public_origin=None, serve_ui=False) assert _run_posture_b(tmp_path, monkeypatch, env="prod") == 2 err = capsys.readouterr().err - assert "public_origin" in err + # The remediation names the OPERATOR-FACING key, not the internal field. It used to say + # `[api].public_origin`, which ADR 0118 relocated and the loader REJECTS -- see the pinning + # test below (BACKLOG #1026). + assert "web_console_public_address" in err assert "12.1.1" in err, "the refusal must name the control it protects, not just the setting" @@ -1454,7 +1457,7 @@ def test_the_refusal_does_not_depend_on_the_console( property of the POSTURE rather than of an unrelated console setting.""" _posture_probe_toml(tmp_path, public_origin=None, serve_ui=True) assert _run_posture_b(tmp_path, monkeypatch, env="prod") == 2 - assert "public_origin" in capsys.readouterr().err + assert "web_console_public_address" in capsys.readouterr().err def test_a_non_phi_instance_is_not_refused( @@ -1466,3 +1469,40 @@ def test_a_non_phi_instance_is_not_refused( _posture_probe_toml(tmp_path, public_origin=None, serve_ui=False, synthetic=True) rc = _run_posture_b(tmp_path, monkeypatch, env="prod") assert rc != 2 or "public_origin" not in capsys.readouterr().err + + +def test_the_refusal_names_a_key_the_loader_actually_accepts() -> None: + """BACKLOG #1026: the refusal handed the operator a remediation that fails at load. + + `[api].public_origin` is the INTERNAL field this code reads. ADR 0118 relocated the + operator-facing key to `[security].web_console_public_address` and REJECTS the old spelling as + file or env input, so an operator who did what the refusal said got "unrecognized config key(s)" + on the next start. A hard refusal naming an unusable fix is worse than one naming none: it costs + a restart cycle to discover and it reads as authoritative, because it comes from the gate itself. + + PINNED AGAINST THE RELOCATION MAP RATHER THAN AGAINST A STRING LITERAL. Asserting the message + contains some remembered spelling would pass just as well after a future relocation moved the key + again -- the two would drift apart silently, which is the defect this test exists to stop. So the + expected key is READ FROM `_RELOCATED_TO_SECURITY`, and a relocation that is not mirrored in the + message reds here. + """ + from messagefoundry.config.settings import _RELOCATED_TO_SECURITY + + expected = _RELOCATED_TO_SECURITY[("api", "public_origin")] + src = (Path(__file__).resolve().parents[1] / "messagefoundry" / "__main__.py").read_text( + encoding="utf-8" + ) + # The 12.1.1 refusal block, located by the control it names rather than by a line number. + marker = "the ASVS 12.1.1 TLS-floor " + assert marker in src, "the 12.1.1 refusal moved; re-anchor this test on the control it names" + start = src.index(marker) + block = src[max(0, start - 600) : start + 600] + assert expected in block, ( + f"the ASVS 12.1.1 refusal does not name {expected!r}, the key the loader accepts. " + "_RELOCATED_TO_SECURITY says that is where [api].public_origin moved, so a message naming " + "the old spelling tells an operator to write a key that is REFUSED at load." + ) + assert "[api].public_origin" not in block, ( + "the refusal still names the relocated spelling, which the loader rejects as file or env " + "input. Name the [security] key instead." + ) diff --git a/tests/test_cli.py b/tests/test_cli.py index 3f1b212d..b265a899 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1246,6 +1246,26 @@ def _l5b_serve(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, toml: str, env: return main(["serve", "--config", str(SAMPLES_CONFIG), "--env", env]) +def _relocated_public_origin_key() -> str: + """The operator-facing key the loader ACCEPTS for what `[api].public_origin` used to spell. + + BACKLOG #1026. Read from the relocation map rather than written as a literal: ADR 0118 moved the + key and the loader REJECTS the old spelling as file or env input, so a refusal naming the old one + hands an operator a remediation that dies at load. A test hard-coding either spelling would keep + passing after the NEXT relocation while the message drifted away from it -- silently, which is the + whole defect. Mirrors the pin in tests/test_api_tls.py for the ASVS 12.1.1 refusal. + """ + from messagefoundry.config.settings import _RELOCATED_TO_SECURITY + + return _RELOCATED_TO_SECURITY[("api", "public_origin")] + + +# The anchor for "did THIS refusal fire", and deliberately NOT the key name. The ADR 0143 degrade +# notice names `web_console_public_address` too, so a negative assertion on the key would trip on a +# message that is not a refusal at all -- and one test below asserts that degrade notice IS present. +_UI_TERMINATOR_REFUSAL = "declared TLS terminator requires" + + def test_serve_ui_upstream_requires_public_origin( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] ) -> None: @@ -1259,7 +1279,10 @@ def test_serve_ui_upstream_requires_public_origin( '[api]\ntls_terminated_upstream = true\ntrusted_proxies = ["10.0.0.2"]\n', ) assert rc == 2 - assert "requires [api].public_origin" in capsys.readouterr().err + err = capsys.readouterr().err + assert _UI_TERMINATOR_REFUSAL in err + # The remediation must name the key the loader accepts, not the relocated spelling. + assert _relocated_public_origin_key() in err def test_serve_ui_upstream_with_public_origin_starts( @@ -1278,7 +1301,7 @@ def test_serve_ui_upstream_with_public_origin_starts( '[api]\ntls_terminated_upstream = true\ntrusted_proxies = ["10.0.0.2"]\n', ) assert rc == 0 - assert "requires [api].public_origin" not in capsys.readouterr().err + assert _UI_TERMINATOR_REFUSAL not in capsys.readouterr().err def test_serve_ui_http_public_origin_refused_with_declared_tls( @@ -1605,7 +1628,10 @@ def test_serve_ui_explicit_offloopback_still_refuses( '[api]\ntls_terminated_upstream = true\ntrusted_proxies = ["10.0.0.2"]\n', ) assert rc == 2 - assert "requires [api].public_origin" in capsys.readouterr().err + err = capsys.readouterr().err + assert _UI_TERMINATOR_REFUSAL in err + # The remediation must name the key the loader accepts, not the relocated spelling. + assert _relocated_public_origin_key() in err def test_serve_ui_default_on_offloopback_degrades_json_only( @@ -1625,7 +1651,7 @@ def test_serve_ui_default_on_offloopback_degrades_json_only( assert rc == 0 err = capsys.readouterr().err assert "on by default (ADR 0143) for LOCAL loopback binds only" in err - assert "requires [api].public_origin" not in err # the /ui refusal did NOT fire + assert _UI_TERMINATOR_REFUSAL not in err # the /ui refusal did NOT fire def test_serve_ui_default_on_public_origin_degrades_json_only(