From 81ef47d8cc1cc652e253695f7be9c5240e3ed6f6 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Mon, 10 Aug 2026 16:37:46 -0500 Subject: [PATCH 1/6] test(security): follow the VALUE across the factory rename boundary (BACKLOG #1208) A connector factory takes a credential PARAMETER and emits it under a different SETTING name; every redaction control operates on the setting name. Three measured instances of that one crossing have now shipped fixes -- private_key -> sign_private_key and private_key_password -> sign_private_key_password (#1106), and proxy -> proxy_url, which is harmless only because the URL rule happens to cover the destination. Nothing asserted the mapping itself. The guard injects a unique sentinel into ONE parameter at a time, calls the factory, and reads the destination off the emitted settings -- so no rename has to be taught to it. The real redactor is then asked about whatever key the sentinel was found under. A parameter that reaches no setting, or that cannot be built at all, is a failure with a stated home rather than a silent pass. Measured by reverting the shipped redaction to each pre-fix state, over 58 probed parameters: pre-#1106 reddens exactly the 2 renamed with_signing parameters and leaves 56 green; pre-#1207 reddens exactly the 10 URL-bearing ones and leaves 48 green; the shipped code reddens none, before and after. The red is specific, so the control locates the layer that does the work. It also covers surfaces the sibling outcome-level guard cannot reach: de-classifying intake_api_key, intake_api_key_next, credential_password, ws_password and client_key_password turns this file red on all five and test_connection_factory_redaction_domain.py red on none, because that file drops a connector's credential arguments when the connector refuses to be built with them. Test-only. messagefoundry/config/wiring.py is unchanged. --- tests/test_credential_parameter_mapping.py | 565 +++++++++++++++++++++ 1 file changed, 565 insertions(+) create mode 100644 tests/test_credential_parameter_mapping.py diff --git a/tests/test_credential_parameter_mapping.py b/tests/test_credential_parameter_mapping.py new file mode 100644 index 00000000..bcdb3c95 --- /dev/null +++ b/tests/test_credential_parameter_mapping.py @@ -0,0 +1,565 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""A credential factory PARAMETER must land on a SETTING name the redactor covers (BACKLOG #1208). + +THE BOUNDARY, and why it needed its own guard. A connector factory takes a credential parameter and +emits it under a DIFFERENT setting name. Every redaction control operates on the SETTING name. Nothing +asserted the two agree, so a rename silently moved a credential outside the control's domain. Three +measured instances of that one shape, all now closed: + +=================================== ========================== ============================ +factory PARAMETER (classified) emitted SETTING (was not) +=================================== ========================== ============================ +``with_signing`` ``private_key`` ``sign_private_key`` +``with_signing`` ``private_key_password`` ``sign_private_key_password`` +``Rest`` ``proxy`` ``proxy_url`` +=================================== ========================== ============================ + +``_is_secret_setting("private_key")`` was True and ``_is_secret_setting("sign_private_key")`` was +False: the parameter was covered and the setting it became was not. ``proxy`` -> ``proxy_url`` crosses +the same boundary and is harmless only because the URL-userinfo rule happens to cover the destination, +which is luck rather than design. THIS FILE MAKES IT DESIGN. + +WHY THE TWO EXISTING GUARDS CANNOT SEE IT. + +* ``test_every_credential_shaped_factory_param_is_classified`` reads PARAMETER names -- one abstraction + level away from where redaction operates. That is exactly how the ``with_signing`` rename walked + through it. +* ``tests/test_connection_factory_redaction_domain.py`` reads EMITTED settings end to end and proves the + OUTCOME per factory: nothing recognisably secret survives. It does not prove the MAPPING. A parameter + the factory drops, renames into a container, or folds into a composed string emits no sentinel at all, + and "no sentinel survived redaction" is then true for the least interesting reason. + +THIS IS NOT A FOURTH NAME LIST, and that is the whole point of the item. ``private_key`` and +``sign_private_key`` are different strings, so any name-to-name comparison has to be TAUGHT each rename +and therefore cannot catch the next one. **The mapping is discovered by following the VALUE**: inject a +unique sentinel into ONE parameter, call the factory, and search the emitted settings for it. The +destination key is whatever the sentinel is found under -- read off the running code, never declared +here. Only then is the real redactor asked about that destination. + +WHAT EACH PARAMETER MUST DO, one of: + +1. land in at least one setting whose name the redactor MASKS (the credential case), or +2. land in at least one setting whose name the URL-userinfo rule covers (the URL-bearing case), or +3. land in a destination declared in :data:`NON_MATERIAL_DESTINATIONS` -- a key that carries a NAME, a + PATH or an IDENTIFIER rather than material, WITH the reason, or +4. reach no setting at all, and be declared in :data:`NON_EMITTING` WITH the reason. + +A parameter that cannot be probed at all is a FAILURE, never a skip: an unbuildable factory is a hole +in the domain, and a hole that reports "clean" is the defect this family keeps recurring as. + +THE DECLARED TABLES CANNOT GO STALE. ``test_no_declared_exemption_is_stale`` fails when an entry in +either table is not actually reached by the probe. An allowlist nobody re-derives is the same defect one +level up -- a guard closing over an enumeration that stopped matching the code. + +MEASURED 2026-08-10, by reverting the shipped redaction to each pre-fix state in turn and watching what +this file does. 58 parameters probed across the spec-returning domain: + +=============================================== ================== ============================== +reverted to goes RED still GREEN +=============================================== ================== ============================== +shipped code (control) 0 58 +pre-#1106 (``sign_private_key`` unclassified) 2 56 +pre-#1207 (no URL-userinfo mask) 10 48 +shipped code again (control) 0 58 +=============================================== ================== ============================== + +**The red is SPECIFIC, not uniform, and that is the result worth having.** Pre-#1106 turns exactly the +two renamed ``with_signing`` parameters red and leaves 56 passing -- so this file locates the layer that +does the work rather than merely reacting to it. Pre-#1207 turns exactly the ten URL-bearing parameters +red. A control that reddened on all 58 in both cases would have looked stronger and taught nothing. + +WHAT IT ADDS OVER THE SIBLING, measured the same day rather than argued. De-classifying five credential +settings that reach connectors through an ``env()``-only refusal or a coupled argument -- +``intake_api_key``, ``intake_api_key_next``, ``credential_password``, ``ws_password``, +``client_key_password`` -- turns **this** file red on all five and +``test_connection_factory_redaction_domain.py`` red on **none**. That sibling drops a connector's +credential arguments when the connector refuses to be built with them, so those five surfaces were +inside its stated domain and outside what it could actually see. +""" + +from __future__ import annotations + +import importlib +import inspect +from typing import Any, NamedTuple + +import pytest + +import messagefoundry +from messagefoundry.config.wiring import redacted_settings +from tests.test_connection_factory_redaction_domain import ( + _factories_returning_a_spec, + _is_credential_param, +) + +# --------------------------------------------------------------------------------------------------- +# Sentinels. Fixed width with a terminator so no sentinel can be a PREFIX of another -- the first draft +# of this probe injected every parameter at once and reported `private_key` as landing in +# `sign_private_key_password`, because its sentinel was a prefix of the password parameter's. One +# parameter is injected per probe now, which removes the ambiguity at the source rather than papering +# over it, and the fixed width keeps that true if the probe is ever widened again. +# --------------------------------------------------------------------------------------------------- +_SENTINEL_N = 0 + + +def _next_sentinel() -> str: + global _SENTINEL_N + _SENTINEL_N += 1 + return f"MFMAP{_SENTINEL_N:04d}ZZ" + + +#: A SOAP body-secret placeholder must match ``^[A-Za-z0-9_.@-]{16,64}$``; anything shorter is refused. +def _placeholder_sentinel(sentinel: str) -> str: + return f"{sentinel}{'0' * (16 - len(sentinel))}" if len(sentinel) < 16 else sentinel + + +class Landing(NamedTuple): + """Where one parameter's sentinel was actually found.""" + + factory: str + param: str + kind: str # "credential" | "url" + form: str # "inline" | "env()" + keys: tuple[str, ...] + + +# --------------------------------------------------------------------------------------------------- +# Declared tables. Both are asserted REACHED, so neither can quietly stop matching the code. +# --------------------------------------------------------------------------------------------------- + +#: Destination setting keys that a credential-shaped PARAMETER legitimately lands on unredacted, +#: because the key carries a NAME, a PATH or an IDENTIFIER and not material. Each needs a reason. +NON_MATERIAL_DESTINATIONS: dict[str, str] = { + "odbc_password_key": ( + "the NAME of the ODBC keyword to put the password under (default 'PWD'), not the password -- " + "naming indirection. Masking it would hide which keyword an operator has to look at" + ), + "odbc_user_key": "the NAME of the ODBC user keyword (default 'UID'), not the user", + "signing_key": ( + "a PATH to the sender's PEM/DER key, not the material; transports/direct.py _read_file()s it. " + "The engine classifies signing_key_password and NOT signing_key deliberately" + ), + "intake_api_key_header": ( + "the NAME of the header the intake credential arrives in (default 'x-api-key'). The " + "credential itself is intake_api_key, which IS masked -- and is env()-only besides" + ), + "credential_domain": ( + "an AD domain name, documented non-secret in wiring.py: it names the directory the share " + "identity belongs to. credential_username and credential_password are both masked" + ), + "body_secret_tokens": ( + "SOAP body-secret PLACEHOLDER tokens (ADR 0015 amendment). Public BY CONTRACT -- the token " + "sits in committed Handler source and the transport swaps the real env() credential in at " + "send time. The paired body_secret_value_ IS masked" + ), + "proxy_no_proxy": ( + "hostname EXCLUSIONS -- the hosts to bypass the proxy for. Public by nature, and not a URL: " + "it is selected by the URL suffix rule only because the name ends in 'proxy'. Masking it " + "would hide routing configuration an operator has to audit" + ), +} + +#: Parameters whose sentinel reaches NO setting, with the reason. Empty today and asserted so: every +#: credential-shaped parameter on every factory currently lands somewhere. The table exists because +#: "reached nothing" is the interesting outcome the OUTCOME-level guard cannot distinguish from +#: "reached something safe", and it must have a stated home when it happens rather than passing quietly. +NON_EMITTING: dict[tuple[str, str], str] = {} + +#: Arguments a factory REQUIRES alongside the parameter under test, because it refuses the parameter +#: on its own. Connector-specific and therefore declared with a reason rather than inferred -- but the +#: alternative is dropping the parameter, and a probe that drops what it cannot build is a guard that +#: examined nothing. +ENABLING_ARGUMENTS: dict[tuple[str, str], dict[str, Any]] = { + ("Http", "intake_api_key"): {"intake_auth": "api_key"}, + ("Http", "intake_api_key_next"): { + "intake_auth": "api_key", + "intake_api_key": messagefoundry.env("probe_primary_key"), + }, + ("Http", "intake_api_key_header"): { + "intake_auth": "api_key", + "intake_api_key": messagefoundry.env("probe_primary_key"), + }, + ("File", "credential_username"): {"credential_password": messagefoundry.env("probe_share_pw")}, + ("File", "credential_password"): { + "credential_username": messagefoundry.env("probe_share_user") + }, + ("File", "credential_domain"): { + "credential_username": messagefoundry.env("probe_share_user"), + "credential_password": messagefoundry.env("probe_share_pw"), + }, + ("Soap", "basic_password"): {"basic_user": "probe-user"}, + ("Soap", "ws_password"): {"ws_username": "probe-user"}, + ("Soap", "proxy_password"): {"proxy": "http://proxy.invalid:8080", "proxy_user": "probe-user"}, + ("Rest", "proxy_password"): {"proxy": "http://proxy.invalid:8080", "proxy_user": "probe-user"}, + ("FHIR", "proxy_password"): {"proxy": "http://proxy.invalid:8080", "proxy_user": "probe-user"}, + ("DICOMweb", "proxy_password"): { + "proxy": "http://proxy.invalid:8080", + "proxy_user": "probe-user", + }, +} + +#: Parameters whose accepted SHAPE is not a bare string, so a plain sentinel cannot be injected. The +#: builder still follows the value; only the wrapper is declared. +SHAPED_INJECTION: dict[tuple[str, str], Any] = { + ("Soap", "body_secrets"): lambda s: {_placeholder_sentinel(s): messagefoundry.env(s.lower())}, +} + +#: Suffixes that make a parameter URL-BEARING: its value may carry ``user:password@`` userinfo, so the +#: destination must be a key the URL rule covers. A SUFFIX rule, not an enumeration -- and its coverage +#: is checked against the running code by +#: :func:`test_the_url_suffix_rule_selects_every_parameter_that_lands_in_a_url_setting`. +URL_BEARING_SUFFIXES = ("url", "uri", "endpoint", "proxy") + +#: What the redactor's own URL rule covers. Read here only to CHECK the suffix rule above against the +#: code; the mapping itself is still discovered by following the sentinel. +_URL_DESTINATION_SUFFIXES = ("url", "_uri", "endpoint", "_endpoint") + + +def _is_url_bearing_param(name: str) -> bool: + low = name.lower() + return low.endswith(URL_BEARING_SUFFIXES) and not _is_credential_param(name) + + +def _resolve(mod: str, name: str) -> Any: + fn = getattr(importlib.import_module(mod), name, None) + return fn if callable(fn) else None + + +def _takes_a_spec(fn: Any) -> bool: + params = list(inspect.signature(fn).parameters.values()) + return bool(params) and params[0].name == "spec" + + +def _filler(p: inspect.Parameter) -> Any: + """Any plausible value for a REQUIRED non-credential argument -- it keeps the call alive and is + never the thing under assertion.""" + ann = str(p.annotation) + if "url" in p.name.lower(): + return "https://example.invalid/token" + if "int" in ann and "str" not in ann: + return 2575 + if "statement" in p.name: + return "SELECT 1" + return "x" + + +def _usable_params(fn: Any) -> list[inspect.Parameter]: + params = list(inspect.signature(fn).parameters.values()) + rest = params[1:] if _takes_a_spec(fn) else params + return [p for p in rest if p.kind in (p.KEYWORD_ONLY, p.POSITIONAL_OR_KEYWORD)] + + +def _find_sentinel(settings: dict[str, Any], sentinel: str) -> tuple[str, ...]: + """Every settings key whose value carries the sentinel, at any depth. + + ``str(value)`` covers the nested cases the flat scan would miss -- a dict, a list, and an ``EnvRef`` + whose ``key`` IS the sentinel when the factory demanded ``env()``. + """ + low = sentinel.lower() + return tuple(k for k, v in settings.items() if low in str(v).lower()) + + +class ProbeResult(NamedTuple): + landing: Landing | None + refused_inline: bool + error: str | None + + +def _probe(factory: str, fn: Any, p: inspect.Parameter, kind: str) -> ProbeResult: + """Inject ONE sentinel into ONE parameter and report where it landed.""" + sentinel = _next_sentinel() + shaper = SHAPED_INJECTION.get((factory, p.name)) + if shaper is not None: + injected: Any = shaper(sentinel) + elif kind == "url": + injected = f"https://probeuser:{sentinel}@proxy.invalid:8080/path" + else: + injected = sentinel + + kwargs: dict[str, Any] = { + q.name: _filler(q) for q in _usable_params(fn) if q.default is inspect.Parameter.empty + } + kwargs.update(ENABLING_ARGUMENTS.get((factory, p.name), {})) + kwargs[p.name] = injected + base = messagefoundry.Rest(url="https://example.invalid/endpoint") + decorator = _takes_a_spec(fn) + + def build(kw: dict[str, Any]) -> Any: + return fn(base, **kw) if decorator else fn(**kw) + + refused = False + try: + spec = build(kwargs) + form = "inline" + except Exception as inline_exc: # noqa: BLE001 - the refusal message IS the signal here + # THE CONNECTOR REFUSES AN INLINE CREDENTIAL AND DEMANDS env(). That is the STRONGER control -- + # the value never resolves into settings, so no serializer can leak it. The MAPPING question + # survives it: the EnvRef's key is the sentinel, so the destination is still discoverable, and + # the destination is what this file asserts about. + refused = True + kwargs[p.name] = messagefoundry.env(sentinel.lower()) + try: + spec = build(kwargs) + form = "env()" + except Exception as env_exc: # noqa: BLE001 + return ProbeResult( + None, + refused, + f"inline: {type(inline_exc).__name__}: {inline_exc}; " + f"env(): {type(env_exc).__name__}: {env_exc}", + ) + + keys = _find_sentinel(dict(spec.settings), sentinel) + return ProbeResult(Landing(factory, p.name, kind, form, keys), refused, None) + + +def _collect() -> tuple[list[Landing], list[str], set[str]]: + """Probe every credential- and URL-bearing parameter of every spec-returning factory.""" + landings: list[Landing] = [] + errors: list[str] = [] + refused: set[str] = set() + for mod, name in _factories_returning_a_spec(): + fn = _resolve(mod, name) + if fn is None: # pragma: no cover - the sibling's domain test fails first if this happens + errors.append(f"{mod}.{name}: not importable") + continue + for p in _usable_params(fn): + kind = ( + "credential" + if _is_credential_param(p.name) + else "url" + if _is_url_bearing_param(p.name) + else None + ) + if kind is None: + continue + result = _probe(name, fn, p, kind) + if result.refused_inline: + refused.add(f"{name}.{p.name}") + if result.landing is None: + errors.append(f"{name}.{p.name} could not be built -- {result.error}") + continue + landings.append(result.landing) + return landings, errors, refused + + +LANDINGS, PROBE_ERRORS, REFUSED_INLINE = _collect() + +#: The credential-shaped landings, as pytest ids. +_IDS = [f"{ln.factory}.{ln.param}" for ln in LANDINGS] + + +def test_every_credential_and_url_parameter_was_probed() -> None: + """LIVENESS, and it reports WHAT it scanned rather than only a count. + + A probe that built nothing would make every parametrised assertion below vacuous, and an + unbuildable factory is a hole in the domain -- never a skip. + """ + print( + f"[1208] probed {len(LANDINGS)} parameters across {len({ln.factory for ln in LANDINGS})} " + f"factories; {len(REFUSED_INLINE)} refused an inline credential and were followed via env()" + ) + for ln in sorted(LANDINGS): + print( + f"[1208] {ln.factory}.{ln.param} ({ln.kind}, {ln.form}) -> {list(ln.keys) or 'NOTHING'}" + ) + assert not PROBE_ERRORS, ( + "these parameters could not be probed, so nothing here covers them:\n " + + "\n ".join(PROBE_ERRORS) + + "\nAdd the arguments the factory couples them to in ENABLING_ARGUMENTS (with the reason), or " + "the shape it demands in SHAPED_INJECTION. Do NOT drop the parameter: a probe that skips what " + "it cannot build is a guard that examined nothing." + ) + assert LANDINGS, "the probe found no credential- or URL-bearing parameters at all" + + +def test_the_domain_is_every_spec_returning_factory() -> None: + """The domain is DERIVED, and derived ONCE. + + It reuses ``_factories_returning_a_spec`` rather than re-walking the AST: a second derivation of + the same population is free to drift from the first, which is the defect one level up. 23 when + BACKLOG #1206 was fixed, and this asserts AT LEAST that -- a shrinking domain is how this class + recurs. + """ + discovered = {n for _, n in _factories_returning_a_spec()} + print(f"[1208] domain = {len(discovered)} spec-returning factories: {sorted(discovered)}") + assert len(discovered) >= 23, ( + f"the spec-returning domain shrank to {len(discovered)}; it was 23 when #1206 was fixed" + ) + probed = {ln.factory for ln in LANDINGS} + unprobed = discovered - probed + # A factory with no credential- or URL-bearing parameter legitimately contributes no landing. + for name in sorted(unprobed): + mod = next(m for m, n in _factories_returning_a_spec() if n == name) + fn = _resolve(mod, name) + assert fn is not None + interesting = [ + p.name + for p in _usable_params(fn) + if _is_credential_param(p.name) or _is_url_bearing_param(p.name) + ] + assert not interesting, ( + f"{name} has credential/URL parameters {interesting} but produced no landing -- the probe " + "silently dropped it" + ) + + +@pytest.mark.parametrize("landing", LANDINGS, ids=_IDS) +def test_a_credential_parameter_lands_on_a_setting_the_redactor_covers(landing: Landing) -> None: + """THE ASSERTION. The destination is read off the running code; the verdict comes from the real + redactor. + + Measured failure this catches, at engine ``64f6e178``: ``with_signing``'s ``private_key`` parameter + landed on ``sign_private_key``, and ``redacted_settings`` returned it VERBATIM through + ``GET /connections/{name}/metadata`` behind ``MONITORING_READ`` alone. + """ + if not landing.keys: + reason = NON_EMITTING.get((landing.factory, landing.param)) + assert reason, ( + f"{landing.factory}.{landing.param} reaches NO setting. It was consumed, composed into " + "another value, or dropped -- each of those needs a stated reason rather than silence, " + "because the outcome-level guard cannot tell them apart from 'safely redacted'. Declare it " + "in NON_EMITTING with the reason, or fix the factory." + ) + return + + probe = _next_sentinel() + unmasked: list[str] = [] + for key in landing.keys: + if key in NON_MATERIAL_DESTINATIONS: + continue + # Ask the REAL control about the destination the sentinel actually reached. For a credential + # parameter the whole value is secret; for a URL-bearing one only the userinfo is, and masking + # the whole URL would destroy the operator's view rather than protect it. + value = f"https://probeuser:{probe}@host.invalid/p" if landing.kind == "url" else probe + if probe in str(redacted_settings({key: value}).get(key)): + unmasked.append(key) + + assert not unmasked, ( + f"{landing.factory}.{landing.param} ({landing.kind}) lands on {unmasked}, and a credential " + f"placed under {'/'.join(unmasked)} survives redacted_settings. The PARAMETER may already be " + "classified under a DIFFERENT name -- that rename IS the defect this file exists for (BACKLOG " + "#1208). Classify the destination in config/wiring.py (_SECRET_SETTING_KEYS / " + "_is_secret_setting), or declare it in NON_MATERIAL_DESTINATIONS here WITH the reason it " + "carries a name, a path or an identifier rather than material." + ) + + +@pytest.mark.parametrize( + "landing", + [ln for ln in LANDINGS if ln.kind == "url"], + ids=[f"{ln.factory}.{ln.param}" for ln in LANDINGS if ln.kind == "url"], +) +def test_masking_a_url_destination_does_not_destroy_the_operator_view(landing: Landing) -> None: + """THE ASYMMETRY on the URL arm, and it is not decoration. + + A redactor that replaced every URL with ``***`` would satisfy the assertion above while silently + removing the account and host an operator needs to diagnose a connection -- a loss nothing would + report. The control must fail for the userinfo shape and KEEP PASSING for everything beside it. + """ + probe = _next_sentinel() + for key in landing.keys: + if key in NON_MATERIAL_DESTINATIONS: + continue + out = str( + redacted_settings({key: f"https://probeuser:{probe}@host.invalid/p?q=1"}).get(key) + ) + assert "probeuser" in out and "host.invalid/p?q=1" in out, ( + f"{key} lost the user, host or path: {out!r}. Only the secret is supposed to be removed." + ) + plain = str(redacted_settings({key: "https://plain.invalid/path?q=1"}).get(key)) + assert plain == "https://plain.invalid/path?q=1", ( + f"{key} rewrote a URL with no userinfo to {plain!r} -- that mangles ordinary configuration" + ) + + +def test_the_url_suffix_rule_selects_every_parameter_that_lands_in_a_url_setting() -> None: + """The suffix rule's OWN coverage, checked against the running code rather than asserted. + + ``URL_BEARING_SUFFIXES`` is a shape rule, and a shape rule can still miss a parameter. The oracle is + the destination: any parameter whose value lands on a setting the redactor's URL rule covers must + have been selected as URL-bearing, or its userinfo was never checked by anything. + """ + selected = {(ln.factory, ln.param) for ln in LANDINGS if ln.kind == "url"} + missed: list[str] = [] + for mod, name in _factories_returning_a_spec(): + fn = _resolve(mod, name) + if fn is None: # pragma: no cover + continue + for p in _usable_params(fn): + if (name, p.name) in selected or _is_credential_param(p.name): + continue + result = _probe(name, fn, p, "url") + if result.landing is None: + continue # the parameter refuses a URL outright; it cannot carry userinfo + for key in result.landing.keys: + if key.lower().endswith(_URL_DESTINATION_SUFFIXES): + missed.append(f"{name}.{p.name} -> {key}") + print(f"[1208] URL-bearing rule selected {len(selected)} parameters: {sorted(selected)}") + assert not missed, ( + f"these parameters land on a URL-shaped setting but the suffix rule did not select them, so " + f"nothing checked their userinfo: {missed}. Widen URL_BEARING_SUFFIXES." + ) + + +def test_at_least_one_connector_still_refuses_an_inline_credential() -> None: + """The env()-only refusal is a STRONGER control than redaction and it is asserted, not assumed. + + ``Http`` and ``File`` refuse an inline credential outright, so the value never resolves into + settings at all. If this set empties, either the refusals were removed (a real regression) or the + probe stopped detecting them -- and a green would prove neither. + """ + print(f"[1208] parameters that refuse an inline credential: {sorted(REFUSED_INLINE)}") + assert REFUSED_INLINE, ( + "no factory refused an inline credential. Either the env()-only refusals were removed, or the " + "probe stopped detecting them." + ) + + +def test_no_declared_exemption_is_stale() -> None: + """A guard that closes over an enumeration which stopped matching the code is this defect one level + up. Every declared exemption must be REACHED by the probe.""" + reached_keys = {key for ln in LANDINGS for key in ln.keys} + stale_destinations = sorted(set(NON_MATERIAL_DESTINATIONS) - reached_keys) + assert not stale_destinations, ( + f"NON_MATERIAL_DESTINATIONS declares {stale_destinations}, which no probed parameter reaches. " + "Either the factory changed and the entry is dead, or the probe stopped reaching it. Remove " + "the entry only after confirming which." + ) + empty_landings = {(ln.factory, ln.param) for ln in LANDINGS if not ln.keys} + stale_non_emitting = sorted(set(NON_EMITTING) - empty_landings) + assert not stale_non_emitting, ( + f"NON_EMITTING declares {stale_non_emitting}, which now DOES reach a setting. Delete the entry " + "-- the parameter is covered by the mapping assertion again." + ) + for reason in list(NON_MATERIAL_DESTINATIONS.values()) + list(NON_EMITTING.values()): + assert len(reason) > 20, "every exemption needs a real reason, not a placeholder" + + +def test_the_mapping_assertion_goes_red_on_a_renamed_destination() -> None: + """NEGATIVE CONTROL. A gate that has never been red is a claim, not a control. + + ``with_signing`` is rebuilt here in miniature: a factory that takes a classified credential + parameter and emits it under an UNclassified name. The probe must find the destination and the + redactor must be seen failing to mask it -- which is the exact shape of the three closed instances. + """ + sentinel = _next_sentinel() + + def _renaming_factory(*, private_key: str) -> Any: + spec = messagefoundry.Rest(url="https://example.invalid/x") + # A destination the redactor does not classify -- `sign_private_key` before BACKLOG #1106. + spec.settings["vendor_signing_material"] = private_key + return spec + + spec = _renaming_factory(private_key=sentinel) + keys = _find_sentinel(dict(spec.settings), sentinel) + assert keys == ("vendor_signing_material",), keys + # The verdict the parametrised assertion applies, run against this destination. + probe = _next_sentinel() + assert probe in str(redacted_settings({keys[0]: probe}).get(keys[0])), ( + "the control cannot see an unclassified destination, so its green above proves nothing" + ) + # ASYMMETRY: the SAME verdict on a classified destination must stay clean, or the control is just + # failing on everything and cannot tell you which layer does the work. + assert probe not in str(redacted_settings({"sign_private_key": probe}).get("sign_private_key")) From 7ab9f18c28b487f6c6d79e2cb31827892e290c15 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Mon, 10 Aug 2026 16:55:15 -0500 Subject: [PATCH 2/6] test(ci): a negative control per required merge context, and a gate that fails without one (BACKLOG #1000) Thirteen contexts are the entire merge gate and not one of them was proven able to go red. The class has fired at least four times here with no CI signal, each found by hand. Nothing enumerated the required list and asserted a control per entry, so a context added to branch protection tomorrow started life unproven and nothing said so. tests/negative_controls.toml registers, per context, the violation a control plants, the pytest nodes that must fail without it, the shapes it deliberately does NOT break, and the nodes for that half. tests/test_negative_controls.py reconciles the registry against the LIVE required set read from .github/required-contexts.txt -- 13 contexts, 13 covered -- and fails when a context has no control, when a control names a context that is not required, when a node id resolves to no test, when the asymmetry half is empty, or when a `ci` control names a command no step invokes. The reconciliation runs inside the existing `test` legs rather than in a new workflow. That makes it blocking today through contexts that are already required, adds no new required context, and needs no branch-protection change. .github/required-contexts.txt is read-only here. tests/test_merge_gate_controls.py supplies the controls the contexts lacked. The backlog-hygiene one lifts the workflow's OWN shell and runs it against a synthetic repository shaped like the PR it polices: the shipped three-dot diff exits 1 and the pre-fix two-dot form exits 0 on the identical fixture, which reproduces the recorded defect rather than asserting it. The others cover the ci-gate roll-up's terminal states, whether a failing test can make the runner exit non-zero (proven again under a hostile child encoding, not merely a favourable one), the gitleaks allowlist as a neutering path, severity floors on bandit and npm audit, and the cla context string. Every control is asymmetric and each was watched fail. Deleting `cancelled` from ci.yml's roll-up condition reddened 2 of 4 and left 2 green; widening one .gitleaks.toml allowlist entry reddened 2 of 4; adding `-ll` to the real bandit invocation reddened 1 of 4. Dropping a registry entry, renaming a red node away, and emptying an asymmetry half each reddened the registry gate. Every mutated file was restored in the same run. Two rows added to test_ci_docs_only_detector so the required-contexts file and this registry classify as CODE: pytest is gated on `code == 'true'`, so a docs-only classification would skip this gate on exactly the PR shape it exists for. --- tests/_negative_controls.py | 172 ++++++++ tests/negative_controls.toml | 386 +++++++++++++++++ tests/test_ci_docs_only_detector.py | 7 + tests/test_merge_gate_controls.py | 623 ++++++++++++++++++++++++++++ tests/test_negative_controls.py | 163 ++++++++ 5 files changed, 1351 insertions(+) create mode 100644 tests/_negative_controls.py create mode 100644 tests/negative_controls.toml create mode 100644 tests/test_merge_gate_controls.py create mode 100644 tests/test_negative_controls.py diff --git a/tests/_negative_controls.py b/tests/_negative_controls.py new file mode 100644 index 00000000..5a94df74 --- /dev/null +++ b/tests/_negative_controls.py @@ -0,0 +1,172 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Load and validate the negative-control registry (BACKLOG #1000). + +Shared rather than inlined for the same reason ``_workflow_contexts`` is: this reconciliation is run +from pytest (so it blocks merge through the existing ``test`` contexts, adding no new required context) +and can be run standalone against a checkout. Two copies of the rule set would be free to disagree, +which is the class the registry exists to catch. + +WHAT IT VALIDATES, and each rule is a decay mode that has actually happened somewhere in this repo: + +* **Every required context has a control.** Otherwise a context added to branch protection tomorrow + starts life unproven and nothing says so -- the decay mode that makes any one-off audit worthless. +* **Every registered context is required.** A control for a context nobody requires is effort spent + guarding nothing, and it inflates the coverage count. +* **Node ids resolve.** A control naming a test that does not exist is a control that does not exist. +* **`red` and `green` are both non-empty.** The asymmetry is mandatory: a control that only reddens + cannot tell you which layer does the work. +* **`ci` commands are wired into the job they claim.** A fixture asserter nobody invokes is a file. + +WHAT THE NODE-ID CHECK DOES AND DOES NOT PROVE, stated because the instrument has to answer the +question asked of it. It resolves each id by parsing the named file and looking for a test function of +that name, so it proves the test EXISTS and is spelled right. It does not prove the test is collected +or that it passes -- that is what running the suite does, and every registered id lives under ``tests/`` +precisely so the required ``test`` legs execute it. +""" + +from __future__ import annotations + +import ast +import tomllib +from dataclasses import dataclass +from pathlib import Path + +from tests._workflow_contexts import ROOT, jobs_of, required_contexts + +REGISTRY = Path(__file__).resolve().parent / "negative_controls.toml" + +#: Prose fields that must carry a real explanation. Short enough to admit a terse one, long enough that +#: "n/a" and "TODO" do not pass -- the two ways a mandatory field becomes decoration. +_MIN_PROSE = 40 + + +@dataclass(frozen=True) +class Control: + context: str + plants: str + holds: str + observed: str + red: tuple[str, ...] + green: tuple[str, ...] + ci: str | None + workflow: str | None + + +def load() -> list[Control]: + raw = tomllib.loads(REGISTRY.read_text(encoding="utf-8")) + controls = raw.get("control", []) + return [ + Control( + context=str(c.get("context", "")), + plants=str(c.get("plants", "")).strip(), + holds=str(c.get("holds", "")).strip(), + observed=str(c.get("observed", "")).strip(), + red=tuple(str(n) for n in c.get("red", [])), + green=tuple(str(n) for n in c.get("green", [])), + ci=str(c["ci"]) if "ci" in c else None, + workflow=str(c["workflow"]) if "workflow" in c else None, + ) + for c in controls + ] + + +def _test_names(path: Path) -> set[str]: + tree = ast.parse(path.read_text(encoding="utf-8")) + return { + node.name + for node in ast.walk(tree) + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef) + and node.name.startswith("test_") + } + + +def unresolved_nodes(controls: list[Control]) -> list[str]: + """Node ids that name no test function. A dangling id is a control that does not exist.""" + cache: dict[str, set[str]] = {} + missing: list[str] = [] + for control in controls: + for node in control.red + control.green: + rel, _, name = node.partition("::") + if not name: + missing.append(f"{node} (not a `file::test_name` node id)") + continue + path = ROOT / rel + if not path.is_file(): + missing.append(f"{node} (no such file)") + continue + if rel not in cache: + cache[rel] = _test_names(path) + if name not in cache[rel]: + missing.append(f"{node} (no test function of that name in {rel})") + return missing + + +def unwired_ci_commands(controls: list[Control]) -> list[str]: + """`ci` commands that do not appear in any step of the workflow they claim. + + A fixture asserter that nobody invokes is a file, not a control -- and it would still satisfy every + other rule here. + """ + unwired: list[str] = [] + for control in controls: + if control.ci is None: + continue + if control.workflow is None: + unwired.append(f"{control.ci} declares no `workflow`") + continue + body = "\n".join( + str(step.get("run", "")) + for job in jobs_of(control.workflow).values() + for step in job.get("steps", []) + ) + if control.ci not in body: + unwired.append(f"{control.ci!r} appears in no `run:` step of {control.workflow}") + return unwired + + +def reconcile() -> tuple[list[str], dict[str, int]]: + """Return (problems, {context: control count}) over the live required set.""" + controls = load() + required = required_contexts() + coverage = dict.fromkeys(required, 0) + problems: list[str] = [] + + for control in controls: + if control.context not in coverage: + problems.append( + f"control for {control.context!r} names a context that is NOT in " + ".github/required-contexts.txt -- either the context was removed from branch " + "protection or the string is a typo" + ) + continue + coverage[control.context] += 1 + for field, value in ( + ("plants", control.plants), + ("holds", control.holds), + ("observed", control.observed), + ): + if len(value) < _MIN_PROSE: + problems.append( + f"{control.context!r}: `{field}` is empty or a placeholder. A control without a " + "stated violation, a stated asymmetry and a stated observation is a claim." + ) + if not control.red: + problems.append(f"{control.context!r}: no `red` nodes -- nothing asserts it can fail") + if not control.green: + problems.append( + f"{control.context!r}: no `green` nodes. The asymmetry is mandatory: a control that " + "reddens uniformly cannot tell you which layer does the work, and cannot distinguish " + "'the other cases are safe by design' from 'safe by luck'." + ) + + for ctx, count in coverage.items(): + if count == 0: + problems.append( + f"required context {ctx!r} has NO negative control. It blocks merge and nobody has " + "watched it fail. Register one in tests/negative_controls.toml." + ) + + problems.extend(f"dangling control node: {m}" for m in unresolved_nodes(controls)) + problems.extend(f"unwired ci control: {m}" for m in unwired_ci_commands(controls)) + return problems, coverage diff --git a/tests/negative_controls.toml b/tests/negative_controls.toml new file mode 100644 index 00000000..63d8e389 --- /dev/null +++ b/tests/negative_controls.toml @@ -0,0 +1,386 @@ +# NEGATIVE CONTROLS FOR THE REQUIRED MERGE CONTEXTS (BACKLOG #1000) +# +# WHY THIS FILE EXISTS. `.github/required-contexts.txt` names the contexts that block merge. It answers +# "is this check blocking?". It cannot answer the question underneath it -- "has anyone ever seen this +# check go red?" -- and a gate nobody has watched fail is an assumption wearing a green tick. The class +# has fired at least four times in this repository, each found by hand and none by CI: a required +# backlog gate computing a two-dot diff and crediting every PR with an older base; a required SAST gate +# scanning a two-directory allow-list while its sibling declared 56 more files in scope; a leak gate +# exiting 0 on content carrying a real site code; the same gate's home-path detector matching one of +# four spellings of the same Windows path. +# +# WHAT A CONTROL IS HERE. Not a re-test of what the gate checks -- the gates' own suites do that. One +# property per context: THIS GATE IS CAPABLE OF GOING RED, evidenced by a planted violation and an +# observed refusal. +# +# EVERY ENTRY IS ASYMMETRIC, and that field is mandatory. It is not enough that neutering a rule turns +# a control red: the control must fail for exactly the shapes that rule covers and KEEP PASSING for the +# shapes some other layer catches, or it cannot tell you which layer does the work. Measured 2026-08-05 +# on a different guard -- a fix believed to cover two NTFS alternate-data-stream spellings turned out to +# be load-bearing for exactly one, and only an eight-case control that reddened on one of them said so. +# A uniform red looks stronger and teaches nothing. So `holds` and `green` are as required as `plants` +# and `red`, and `tests/test_negative_controls.py` fails when either is missing. +# +# FORMAT. One `[[control]]` per context (a context may have several). +# context the exact string from .github/required-contexts.txt -- checked against it, both ways +# plants the violation the control introduces +# red pytest node ids that MUST fail if the gate stops catching that violation +# holds what the control deliberately does NOT break, and which layer covers it +# green pytest node ids for that half -- the shapes that must keep passing +# observed what was actually seen, and where. Prose only; nothing parses it. +# ci (optional) a command wired into the gate's OWN job, asserted present in that job +# workflow (with `ci`) the workflow file the command must appear in +# +# THIS FILE IS NOT BRANCH PROTECTION AND DOES NOT TOUCH IT. `.github/required-contexts.txt` is +# READ-ONLY here: it mirrors the live server, and a context added to it before the server is exactly +# the lie it exists to prevent. This file only reads it. +# +# WHAT THE SCANNER GATES CAN AND CANNOT BE CONTROLLED FOR, said once rather than repeated per entry. +# bandit, gitleaks, npm-audit, pip-audit and semgrep run third-party binaries the pytest legs do not +# install. Their DETECTION is exercised where the binary lives -- semgrep against annotated fixtures in +# its own CI job, pip-audit's slopsquat half by a stdlib script with its own unit suite. What the pytest +# legs can hold is the property those jobs lose SILENTLY: an enforcement flag removed, a severity floor +# added, an allowlist widened until it swallows the class, a scan re-narrowed to an allow-list. Those +# are the controls registered below, and the distinction is recorded rather than blurred. + +# --- ci.yml ---------------------------------------------------------------------------------------- + +[[control]] +context = "CI gate" +plants = """ +One gated leg reports `failure`, then `cancelled`, in a synthetic needs-results vector read through the +roll-up's own `if:` condition. `CI gate` is required BECAUSE the six legs behind it cannot be -- a +path-gated job does not report on a PR that touches none of its paths -- so this roll-up is the only +thing that turns a red sqlserver-store, postgres-store, load-test, load-test-sqlserver or +windows-service-smoke into a blocked merge. +""" +red = [ + "tests/test_merge_gate_controls.py::test_the_ci_gate_rollup_fires_on_a_failed_or_cancelled_leg", + "tests/test_merge_gate_controls.py::test_the_ci_gate_rollup_still_covers_every_leg_it_is_required_for", +] +holds = """ +An all-`skipped` vector must stay GREEN, and that is the case that actually happens: almost every PR +touches none of the six gated paths. A control that reddened on `skipped` would block every ordinary PR +while looking stronger. The reader itself is also controlled -- a condition with `cancelled` removed +must be reported as no longer firing on it, otherwise "the shipped condition is fine" and "the reader +matches nothing" are the same green. +""" +green = [ + "tests/test_merge_gate_controls.py::test_the_ci_gate_rollup_stays_green_when_every_gated_leg_skipped", + "tests/test_merge_gate_controls.py::test_the_rollup_reader_reports_a_condition_that_dropped_cancelled", +] +observed = """ +Live record, quoted in .github/required-contexts.txt: 18 success / 2 failure over 20 runs. One failure +was a nightly `schedule` run whose sqlserver load-test legs failed; one was a superseded re-push where +every job was CANCELLED -- both terminal states this control asserts are still named. Restored to +branch protection 2026-07-30 after checking exactly that record. + +RUN AGAINST A NEUTERED GATE, 2026-08-10: with `contains(needs.*.result, 'cancelled')` deleted from +ci.yml, 2 of the 4 roll-up controls went red and 2 stayed green -- the specific result, not a uniform +one. ci.yml was restored from the same run. +""" + +[[control]] +context = "test (ubuntu-latest, py3.14)" +plants = """ +A test that cannot pass, run by a child pytest outside this repository's rootdir. The assertion is on +the EXIT CODE alone. +""" +red = ["tests/test_merge_gate_controls.py::test_a_failing_test_makes_the_pytest_leg_exit_nonzero"] +holds = """ +A passing fixture must leave the runner at exit 0 -- a runner that failed unconditionally would satisfy +the planted case while blocking every PR, and nothing would say which of the two it was. The exit codes +are additionally re-measured with the child pinned to a HOSTILE encoding, because a control that only +ever ran under a favourable ambient value proves nothing about the Windows legs. +""" +green = [ + "tests/test_merge_gate_controls.py::test_a_passing_fixture_leaves_the_pytest_leg_green", + "tests/test_merge_gate_controls.py::test_the_pytest_exit_code_does_not_depend_on_the_ambient_encoding", +] +observed = """ +The planted failure exits non-zero and the passing fixture exits 0, unchanged under +PYTHONIOENCODING=ascii / PYTHONUTF8=0. This is not hypothetical plumbing: BACKLOG #1000 records the +measured case one layer over, where `pwsh -File script.ps1` returned 0 although the script inside died +at parameter binding, making every execution assertion built on that return code vacuously green. +""" + +[[control]] +context = "test (windows-2022, py3.14)" +plants = "As the ubuntu leg: a test that cannot pass, asserted on the child's exit code." +red = ["tests/test_merge_gate_controls.py::test_a_failing_test_makes_the_pytest_leg_exit_nonzero"] +holds = """ +Same asymmetry, and the encoding half is load-bearing HERE specifically: the Windows legs are where an +ambient code page differs from the author's shell, which is how a wave-2 lane shipped a test that +passed ubuntu and would have reddened these two contexts. +""" +green = [ + "tests/test_merge_gate_controls.py::test_a_passing_fixture_leaves_the_pytest_leg_green", + "tests/test_merge_gate_controls.py::test_the_pytest_exit_code_does_not_depend_on_the_ambient_encoding", +] +observed = "Run on this Windows worktree; exit codes as above, unchanged under a hostile encoding." + +[[control]] +context = "test (windows-2025, py3.14)" +plants = "As the ubuntu leg: a test that cannot pass, asserted on the child's exit code." +red = ["tests/test_merge_gate_controls.py::test_a_failing_test_makes_the_pytest_leg_exit_nonzero"] +holds = "Same asymmetry as the windows-2022 leg; the two differ only in runner image." +green = [ + "tests/test_merge_gate_controls.py::test_a_passing_fixture_leaves_the_pytest_leg_green", + "tests/test_merge_gate_controls.py::test_the_pytest_exit_code_does_not_depend_on_the_ambient_encoding", +] +observed = "Run on this Windows worktree; exit codes as above, unchanged under a hostile encoding." + +# --- security.yml ---------------------------------------------------------------------------------- + +[[control]] +context = "bandit (Python SAST)" +plants = """ +`bandit -ll` against the shipped invocation's muting detector. A severity floor keeps the scanner +running and the exit code intact while discarding a whole severity band -- so the required context goes +on reporting success, and a neutering scan looking for ADDED idioms (`|| true`, `--exit-zero`) cannot +see it. The sibling scope control plants the other half: a scan re-narrowed to an allow-list, which is +BACKLOG #334's shape. +""" +red = [ + "tests/test_merge_gate_controls.py::test_the_bandit_invocation_carries_no_severity_or_confidence_floor", + "tests/test_lint_scope_parity.py::test_ci_bandit_scans_the_repo_not_an_allow_list", +] +holds = """ +The reviewed `--skip B101,B110,B311,B404,B608` list must NOT be flagged. Each entry there is a per-check +exclusion with a stated reason, not a severity floor, and a detector that flagged it would be "fixed" by +deleting a correct annotation. The excludes must also keep matching semgrep's, so widening one gate's +blind spot cannot pass as tightening the other's. +""" +green = [ + "tests/test_merge_gate_controls.py::test_the_muting_detector_fires_on_a_synthetic_floor", + "tests/test_lint_scope_parity.py::test_semgrep_and_bandit_exclude_the_same_paths", +] +observed = """ +The detector fires on a synthetic `bandit -r . -ll --skip B101` and stays clean on the shipped command +in the same call. Detection by bandit itself runs in its own CI job against the pinned scanner lock. + +RUN AGAINST A NEUTERED GATE, 2026-08-10: adding `-ll` to security.yml's real bandit invocation turned +1 of the 4 selected controls red and left 3 green; security.yml was restored from the same run. +""" + +[[control]] +context = "pip-audit (dependency vulnerabilities)" +plants = """ +A hallucinated distribution name, a registered-but-empty project, and a freshly registered one, fed to +`scripts/security/new_dependency_check.py` -- the step in this job that answers what pip-audit +structurally cannot: a hallucinated name has no advisory, so it resolves, locks, hashes and installs +through every other DEP-1 control clean. +""" +red = [ + "tests/test_new_dependency_check.py::test_a_nonexistent_name_is_caught", + "tests/test_new_dependency_check.py::test_main_exits_1_on_a_hallucinated_name", + "tests/test_new_dependency_check.py::test_main_exits_2_when_the_sweep_examines_nothing", +] +holds = """ +A clean tree must produce no findings AND a non-zero examined count -- "found nothing" and "did not +look" are different answers and only one of them is clean. An allowlisted name must be skipped without +being counted, so the sweep's own liveness number cannot be inflated by entries it never checked. +""" +green = [ + "tests/test_new_dependency_check.py::test_a_clean_tree_produces_no_findings_and_a_nonzero_count", + "tests/test_new_dependency_check.py::test_the_allowlist_skips_a_name_and_does_not_count_it", +] +observed = """ +The planted names are refused and the clean tree passes with a non-zero count, in this suite. The +audit half (pip-audit over three locks) needs the network and the scanner lock and runs in its own job; +its documented blind spot -- a real package that is not the INTENDED one -- is recorded in the script. +""" + +[[control]] +context = "npm-audit (ide dependency vulnerabilities)" +plants = """ +`npm audit --audit-level=high` against the shipped invocation's muting detector. That flag exits 0 on +moderate advisories, which silently contradicts the job's own comment that the default level "fails on +ANY severity, matching pip-audit's strict posture". The second plant is the quieter one: a missing +`working-directory`, which leaves `--package-lock-only` auditing a tree with no lockfile. +""" +red = [ + "tests/test_merge_gate_controls.py::test_the_npm_audit_invocation_carries_no_severity_floor", + "tests/test_merge_gate_controls.py::test_the_npm_audit_target_lockfile_exists", +] +holds = """ +The shipped `npm audit --package-lock-only` must stay clean under the same detector, so the control is +not simply refusing every flag. `--package-lock-only` is itself an install-free NARROWING of what npm +does, and it is deliberate -- the control must distinguish it from a severity floor. +""" +green = ["tests/test_merge_gate_controls.py::test_the_muting_detector_fires_on_a_synthetic_floor"] +observed = """ +The detector fires on the synthetic floor and stays clean on the shipped command; ide/package-lock.json +resolves from the declared working-directory. Detection by npm itself runs in its own CI job. +""" + +[[control]] +context = "gitleaks (secret scan)" +plants = """ +Six credential shapes the default ruleset exists to catch -- an AWS access key id, a GitHub PAT, a Slack +bot token, a PEM private-key header, a 40-hex token and a URL with an inline database password -- all +ASSEMBLED AT RUNTIME, never committed as literals, and matched against every allowlist regex in +`.gitleaks.toml`. That allowlist is the one place this required context can be disabled without +touching a workflow at all. +""" +red = [ + "tests/test_merge_gate_controls.py::test_no_gitleaks_allowlist_regex_swallows_a_fabricated_secret", + "tests/test_merge_gate_controls.py::test_the_gitleaks_config_still_extends_the_default_ruleset", +] +holds = """ +The shipped allowlist must stay GREEN in the same call that flags two planted broad patterns. The +control is not demanding an empty allowlist -- the entries there are documented non-secret fixtures (a +CI container password, a SOAP placeholder class, a pinned audit-chain digest) and deleting them would +red the gate on correct content, which is how an allowlist gets widened in the first place. +""" +green = [ + "tests/test_merge_gate_controls.py::test_the_allowlist_narrowness_detector_fires_on_a_planted_broad_regex" +] +observed = """ +Planted `.{8,}` and `[A-Za-z0-9_/+-]{20,}` are both caught; the shipped entries match none of the six +shapes. This gate has also been seen red in ordinary use: the redaction suite's first JWT fixture was a +literal token and the gitleaks pre-commit hook rejected the commit, which is why that fixture is now +assembled from parts. + +RUN AGAINST A NEUTERED GATE, 2026-08-10: replacing one literal allowlist entry in the real +.gitleaks.toml with `[A-Za-z0-9_/+-]{16,}` turned 2 of the 4 selected controls red and left 2 green; +.gitleaks.toml was restored from the same run. +""" + +[[control]] +context = "semgrep (project SAST rules)" +plants = """ +Annotated handler fixtures carrying inter-statement PHI taint, an aliased import, an ambient subprocess +call, ambient deserialization, ambient eval/exec, an unparameterized db_lookup and a PHI-to-file write. +Each rule must fire EXACTLY as annotated. +""" +ci = "python scripts/ci/assert_semgrep_handler_taint.py" +workflow = "security.yml" +red = [ + "tests/test_lint_scope_parity.py::test_ci_semgrep_still_fails_the_build_on_a_finding", + "tests/test_lint_scope_parity.py::test_ci_semgrep_scans_the_repo_not_an_allow_list", +] +holds = """ +The `# ok` cases in the same fixture -- log.debug, msg.control_id, a parameterized db_lookup -- must +contribute ZERO findings, and the count equality is what asserts it. A rule set that fired on those +would be indistinguishable from one that fires on everything. On the workflow side, the excludes must +keep matching bandit's rather than growing independently. +""" +green = ["tests/test_lint_scope_parity.py::test_semgrep_and_bandit_exclude_the_same_paths"] +observed = """ +The fixture asserter is the gate's own step and fails the job on any count mismatch in either +direction. `--error` is separately asserted here because without it semgrep prints every match across +the whole scanned tree and exits 0 -- a required context that cannot fail, which is strictly worse than +the narrow scope it replaced. +""" + +[[control]] +context = "crypto-inventory (ASVS 11.1.3 discovery gate)" +plants = """ +A seam-only delegator with no inventory row, a Python file planted under the ide/ TypeScript tree, and +a provider row deleted from the documented inventory. Each must be reported. +""" +red = [ + "tests/test_crypto_inventory_scanner.py::test_seam_only_delegator_is_reported_undocumented", + "tests/test_crypto_inventory_scanner.py::test_ide_invariant_flags_a_planted_python_file", + "tests/test_crypto_inventory_doc.py::test_guard_detects_a_removed_provider_row", +] +holds = """ +A plain non-crypto store import must NOT be reported, and the real tree must stay clean -- a discovery +gate that flagged ordinary imports would be silenced by the first person it inconvenienced, which is a +slower version of switching it off. +""" +green = [ + "tests/test_crypto_inventory_scanner.py::test_a_plain_non_crypto_store_import_is_not_a_false_positive", + "tests/test_crypto_inventory_scanner.py::test_gate_is_clean_on_the_real_tree", +] +observed = "Each planted shape is reported and the real tree is clean, in this suite." + +[[control]] +context = "forbidden-content (customer/PHI leak guard)" +plants = """ +A customer name, a case-sensitive site code, a routable host IP, and a file under docs/security/ caught +by its PATH alone; plus a run that examines zero files, which must refuse rather than report clean. +""" +red = [ + "tests/test_scan_forbidden.py::test_scan_file_flags_customer_name", + "tests/test_scan_forbidden.py::test_scan_file_flags_site_code", + "tests/test_scan_forbidden.py::test_a_file_under_docs_security_is_flagged_by_its_path_alone", + "tests/test_scan_forbidden.py::test_zero_examined_still_refuses_rather_than_reporting_clean", +] +holds = """ +Private and documentation IP ranges, ordinary docs paths, and clean text must all pass. This is the +asymmetry with the most history behind it: BACKLOG #321 and #325 are both this gate failing to fire, +and a control that reddened on 10.0.0.1 or on docs/CI.md would have been relaxed rather than trusted. +""" +green = [ + "tests/test_scan_forbidden.py::test_scan_file_allows_private_and_doc_ips", + "tests/test_scan_forbidden.py::test_the_path_detector_does_not_flag_ordinary_paths", + "tests/test_scan_forbidden.py::test_scan_file_clean_text_has_no_hits", +] +observed = """ +Every planted shape is flagged and every benign one passes, in this suite. The gate additionally fails +CLOSED in CI on a per-section detector floor, because a partially-mangled token list once loaded as few +as 1 of 21 detectors and passed with a green tick. +""" + +# --- cla.yml --------------------------------------------------------------------------------------- + +[[control]] +context = "cla" +plants = """ +The context STRING, which is the only part of this gate the repository controls -- its detection lives +entirely in a third-party action. Planted historically rather than synthetically: docs/CI.md and cla.yml +both told a reader to require "CLA Assistant", the WORKFLOW name, which matches no status check. Adding +it to branch protection would have wedged every PR forever. A `name:` on the job would do the same +thing quietly, by moving the context off `cla`. +""" +red = [ + "tests/test_merge_gate_controls.py::test_the_cla_job_still_reports_under_the_job_key_and_not_the_workflow_name", + "tests/test_required_contexts.py::test_no_claim_file_names_the_cla_context_by_its_workflow_name", +] +holds = """ +The maintainer/bot allowlist must keep WORKING -- emptying it would demand a signature on every +maintainer push. What is asserted is its SHAPE: an enumeration, never a glob, because `bot*` would let +any human whose username begins with "bot" skip signing (review low-28). And the context must still +resolve to a real job, which is the opposite failure from the one planted above. +""" +green = [ + "tests/test_merge_gate_controls.py::test_the_cla_allowlist_is_an_enumeration_and_not_a_glob", + "tests/test_required_contexts.py::test_every_required_context_matches_a_real_job", +] +observed = """ +The job key is `cla`, it declares no `name:`, it runs on pull_request_target, and the allowlist is a +three-name enumeration. The wrong string was live in two files for months and is now refused by a test. +""" + +# --- backlog-hygiene.yml --------------------------------------------------------------------------- + +[[control]] +context = "a PR that implements BACKLOG #N must update BACKLOG.md" +plants = """ +A synthetic repository shaped like the PR this gate exists to police: a branch that claims +`BACKLOG #42` and changes engine code and nothing else, while `main` has separately moved +docs/BACKLOG.md since the branch point. The gate's OWN shell is lifted out of the workflow and run +against it -- not a re-implementation, which would be a second copy free to agree with itself. +""" +red = [ + "tests/test_merge_gate_controls.py::test_the_backlog_hygiene_gate_fails_a_code_pr_that_leaves_the_ledger_alone", + "tests/test_merge_gate_controls.py::test_the_two_dot_form_of_the_gate_passes_the_same_planted_pull_request", +] +holds = """ +Four benign shapes must stay green: no claim at all, a claim honoured in the live ledger, a claim +honoured in the ARCHIVE (an item retired between the claim and the PR), and a bare `#42`, which is a PR +number in this repo rather than an item. A gate that failed all four would satisfy the planted case and +block every pull request in the repository. +""" +green = [ + "tests/test_merge_gate_controls.py::test_the_backlog_hygiene_gate_leaves_the_benign_shapes_alone" +] +observed = """ +RUN AGAINST THE PRE-FIX GATE, which is what makes this evidence rather than a claim. The identical +fixture goes RED (exit 1) through the shipped three-dot diff and GREEN (exit 0) with only the diff form +reverted to two-dot. That reproduces the recorded defect exactly: the two-dot form reports main-side +changes as reverse deltas, so any main-side edit to docs/BACKLOG.md credited every PR with an older +base, and the gate went green while enforcing nothing -- on precisely the population it polices. +""" diff --git a/tests/test_ci_docs_only_detector.py b/tests/test_ci_docs_only_detector.py index 61218bbc..8edb0e1c 100644 --- a/tests/test_ci_docs_only_detector.py +++ b/tests/test_ci_docs_only_detector.py @@ -161,6 +161,13 @@ def test_extensionless_config_is_code_and_gitattributes_was_not_before( ".github/workflows/ci.yml", "pyproject.toml", ".gitignore", # BACKLOG #327 — deliberately code, and it has no code-ish extension + # BACKLOG #1000. The negative-control reconciliation runs under pytest, which is gated on + # `code == 'true'` — the same coupling that let a #320 banner merge without the suite + # compiling. Its decay mode is a context arriving in branch protection and being mirrored + # into `.github/required-contexts.txt`, so these two paths MUST classify as code or the gate + # is skipped on exactly the pull request shape it exists for. The first is extensionless. + ".github/required-contexts.txt", + "tests/negative_controls.toml", ], ) def test_code_paths_run_the_suite(path: str, pats: tuple[str, str, str]) -> None: diff --git a/tests/test_merge_gate_controls.py b/tests/test_merge_gate_controls.py new file mode 100644 index 00000000..ab7399a0 --- /dev/null +++ b/tests/test_merge_gate_controls.py @@ -0,0 +1,623 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Negative controls for the required merge contexts that had none (BACKLOG #1000). + +A GATE NOBODY HAS WATCHED FAIL IS AN ASSUMPTION WEARING A GREEN TICK. Thirteen contexts are the entire +merge gate, and several of them were guarded only by the property that they exist. This file supplies +the missing plant-and-observe controls; ``tests/negative_controls.toml`` records which control belongs +to which context and ``tests/test_negative_controls.py`` fails when a context has none. + +EVERY CONTROL HERE IS ASYMMETRIC, and that is the part most likely to be skipped. It is not enough that +neutering a rule turns a control red -- the control must fail for exactly the shapes that rule covers +and KEEP PASSING for the shapes some other layer catches, or it cannot tell you which layer does the +work. Measured 2026-08-05 on a different guard: a fix believed to cover two NTFS alternate-data-stream +spellings turned out to be load-bearing for exactly one, and the eight-case control that reddened on +only one of them is what said so. A uniform red would have flattered the code and taught nothing. + +So each control below is paired: a planted violation the gate must see, and a benign neighbour it must +leave alone. Where the gate's own detector is what is being checked, the detector is additionally run +against a synthetic NEUTERED form of the shipped command and observed firing -- otherwise "the shipped +command is clean" is indistinguishable from "the detector matches nothing". + +WHAT IS NOT HERE, said plainly. The scanner gates (bandit, gitleaks, npm-audit, pip-audit, semgrep) run +third-party binaries that this suite does not install, so what is asserted here is the property those +jobs can lose SILENTLY: an enforcement flag removed, a severity floor added, an allowlist widened until +it swallows the class. The scanners' detection itself is exercised inside their own CI jobs -- semgrep +by ``scripts/ci/assert_semgrep_handler_taint.py`` over annotated fixtures, pip-audit's slopsquat half by +``tests/test_new_dependency_check.py`` -- and the registry records which is which rather than letting +the two read as one. +""" + +from __future__ import annotations + +import os +import re +import shutil +import subprocess +import sys +import tomllib +from pathlib import Path +from typing import Any + +import pytest + +from tests._workflow_contexts import ROOT, jobs_of, load_workflow + +_TIMEOUT = 300 + + +# =================================================================================================== +# Child processes. PIN THE CHILD'S ENVIRONMENT -- do not inherit it. +# =================================================================================================== +def _child_env(**extra: str) -> dict[str, str]: + """A minimal, EXPLICIT environment for a child process. + + Measured in wave 2 of this backlog pass: a lane's new test passed in its author's shell and failed + at integration, because that shell happened to export ``PYTHONIOENCODING=utf-8`` while the test + pinned only the PARENT's decoding. It would have passed ubuntu and reddened the Windows legs. So + nothing is inherited here except what a child genuinely cannot run without, and the two variables + that incident turned on are set EXPLICITLY rather than passed through. + + ``LC_ALL=C`` and the ``GIT_CONFIG_*`` overrides exist for the same reason one level over: a global + ``core.autocrlf``, a global hooks path, or a locale that reorders ``grep`` output would otherwise + make the result a fact about this machine. + """ + env = { + "PATH": os.environ.get("PATH", ""), + "LC_ALL": "C", + "LANG": "C", + "PYTHONIOENCODING": "utf-8", + "PYTHONUTF8": "1", + "GIT_AUTHOR_NAME": "negative control", + "GIT_AUTHOR_EMAIL": "control@example.invalid", + "GIT_COMMITTER_NAME": "negative control", + "GIT_COMMITTER_EMAIL": "control@example.invalid", + "GIT_TERMINAL_PROMPT": "0", + } + # Windows: a child python cannot start without these, and they carry no behaviour of their own. + for name in ("SYSTEMROOT", "SystemRoot", "COMSPEC", "TEMP", "TMP", "WINDIR", "PATHEXT"): + if name in os.environ: + env[name] = os.environ[name] + env.update(extra) + return env + + +def _run(argv: list[str], cwd: Path, env: dict[str, str]) -> subprocess.CompletedProcess[bytes]: + """Run a child and return RAW BYTES. + + Decoding is done by the caller with ``errors="replace"``. A child whose output cannot be decoded + under the ambient code page must not be able to turn an assertion about an EXIT CODE into a + ``UnicodeDecodeError`` -- that failure mode is a property of the console, not of the gate. + """ + return subprocess.run( # noqa: S603 # nosec B603 - fixed argv, no shell, test-local paths + argv, cwd=str(cwd), env=env, capture_output=True, timeout=_TIMEOUT, check=False + ) + + +def _text(proc: subprocess.CompletedProcess[bytes]) -> str: + return (proc.stdout + proc.stderr).decode("utf-8", errors="replace") + + +# =================================================================================================== +# `CI gate` -- the roll-up that is the ONLY way six path-gated legs reach branch protection. +# =================================================================================================== +def _ci_gate_job() -> dict[str, Any]: + return jobs_of("ci.yml")["ci-gate"] + + +def _rollup_fail_condition() -> str: + for step in _ci_gate_job().get("steps", []): + if "if" in step and "needs" in str(step.get("if", "")): + return str(step["if"]) + raise AssertionError( + "ci.yml's `ci-gate` job has no step conditioned on `needs.*.result`. The roll-up is the only " + "path by which six path-gated legs reach branch protection; without that condition it reports " + "success unconditionally." + ) + + +def _terminal_states(condition: str) -> set[str]: + """The `needs.*.result` values the roll-up's condition fires on, read off the workflow.""" + return set(re.findall(r"contains\(\s*needs\.\*\.result\s*,\s*'([a-z]+)'\s*\)", condition)) + + +def _rollup_fires(condition: str, results: list[str]) -> bool: + """`contains(needs.*.result, X) || contains(needs.*.result, Y)` over a synthetic results vector.""" + return any(r in _terminal_states(condition) for r in results) + + +def test_the_ci_gate_rollup_fires_on_a_failed_or_cancelled_leg() -> None: + """PLANTED: one gated leg reports `failure`, then `cancelled`. The roll-up must fire on both. + + `CI gate` is required BECAUSE the six legs behind it cannot be: a path-gated job does not report on + a PR that touches none of its paths, which wedges every such PR forever. So the roll-up is the only + thing that turns a red sqlserver-store, postgres-store, load-test, load-test-sqlserver or + windows-service-smoke into a blocked merge. + """ + condition = _rollup_fail_condition() + states = _terminal_states(condition) + assert states == {"failure", "cancelled"}, ( + f"the roll-up fires on {sorted(states)}. `failure` alone lets a CANCELLED leg -- which is what a " + f"timed-out or infrastructure-killed run reports -- pass the merge gate. Condition: {condition!r}" + ) + for planted in ("failure", "cancelled"): + results = ["success", "success", "skipped", "success", "skipped", planted] + assert _rollup_fires(condition, results), ( + f"a gated leg reporting {planted!r} does not fire the roll-up: {condition!r}" + ) + step = next(s for s in _ci_gate_job()["steps"] if s.get("if") == condition) + assert "exit 1" in str(step.get("run", "")), ( + "the roll-up's failing step does not `exit 1`, so the condition fires and the job still " + "reports success" + ) + + +def test_the_ci_gate_rollup_stays_green_when_every_gated_leg_skipped() -> None: + """THE ASYMMETRY, and it is the case that actually happens. + + Almost every PR touches none of the six gated paths, so all six SKIP. A roll-up that failed on + `skipped` would block every ordinary PR -- .github/required-contexts.txt records the run where all + six skipped and `CI gate` still returned success, which is what made requiring it safe. A control + that reddened on everything would have destroyed that property while looking stronger. + """ + condition = _rollup_fail_condition() + assert not _rollup_fires(condition, ["skipped"] * 6) + assert not _rollup_fires(condition, ["success"] * 6) + assert not _rollup_fires(condition, ["success", "skipped", "skipped", "success", "skipped"]) + + +def test_the_rollup_reader_reports_a_condition_that_dropped_cancelled() -> None: + """NEGATIVE CONTROL OF THE CONTROL. Without it, "the shipped condition is fine" and "the reader + matches nothing" are the same green.""" + neutered = "contains(needs.*.result, 'failure')" + assert _terminal_states(neutered) == {"failure"} + assert not _rollup_fires(neutered, ["cancelled"]), ( + "the reader cannot see a dropped terminal state" + ) + assert _rollup_fires(_rollup_fail_condition(), ["cancelled"]), ( + "...and it does see the shipped one, so the assertion above is about the workflow rather than " + "about the reader" + ) + + +def test_the_ci_gate_rollup_still_covers_every_leg_it_is_required_for() -> None: + """A leg dropped from `needs:` leaves branch protection with no path to it at all -- the roll-up + keeps reporting success and the leg's failures stop mattering, silently.""" + needs = set(_ci_gate_job().get("needs", [])) + gated = { + "sqlserver-store", + "postgres-store", + "load-test", + "load-test-sqlserver", + "windows-service-smoke", + } + missing = sorted(gated - needs) + print(f"[#1000] ci-gate needs: {sorted(needs)}") + assert not missing, ( + f"these gated legs are no longer behind the roll-up: {missing}. They cannot be required " + "directly (a path-gated job does not report on a PR that misses its paths), so nothing on the " + "merge path would notice them going red." + ) + + +# =================================================================================================== +# `test (, py3.14)` -- can the test legs go red at all? +# =================================================================================================== +_FAILING_FIXTURE = "def test_planted_failure():\n assert 1 == 2, 'planted'\n" +_PASSING_FIXTURE = "def test_planted_pass():\n assert 1 == 1\n" + + +def _run_pytest_on(fixture: str, tmp_path: Path, **env_extra: str) -> int: + """Run a child pytest over ONE fixture file, outside this repository's rootdir. + + The assertion is on the EXIT CODE only. Exit codes are encoding-independent, which is the point: + the wave-2 incident this guards against turned on a child's stdout ENCODING, and an assertion that + reads the child's text is exactly the assertion that inherits it. + """ + work = tmp_path / f"probe_{abs(hash(fixture)) % 10000}" + work.mkdir() + (work / "test_probe.py").write_text(fixture, encoding="utf-8") + proc = _run( + [sys.executable, "-m", "pytest", "-q", "-p", "no:cacheprovider", str(work)], + cwd=work, + env=_child_env(**env_extra), + ) + print(f"[#1000] child pytest exit={proc.returncode} env_extra={env_extra}") + return proc.returncode + + +def test_a_failing_test_makes_the_pytest_leg_exit_nonzero(tmp_path: Path) -> None: + """PLANTED: a test that cannot pass. The runner must exit non-zero, or the three `test` contexts -- + the largest block of the merge gate -- are decoration. + + This is not hypothetical plumbing. BACKLOG #1000 records the measured case one layer over: running + an emitted command via `pwsh -File script.ps1` returns 0 even when the script inside died at + parameter binding, so every execution assertion built on that return code was vacuously green. It + was found only by writing a control that had to fail and watching it pass. + """ + assert _run_pytest_on(_FAILING_FIXTURE, tmp_path) != 0, ( + "a deliberately failing test did not make pytest exit non-zero" + ) + + +def test_a_passing_fixture_leaves_the_pytest_leg_green(tmp_path: Path) -> None: + """THE ASYMMETRY. A runner that exited non-zero unconditionally would satisfy the control above + while blocking every PR, and nothing would say which of the two it was.""" + assert _run_pytest_on(_PASSING_FIXTURE, tmp_path) == 0 + + +def test_the_pytest_exit_code_does_not_depend_on_the_ambient_encoding(tmp_path: Path) -> None: + """PROVEN UNDER A HOSTILE AMBIENT VALUE, not merely a favourable one. + + A control that only ever ran under `PYTHONIOENCODING=utf-8` proves nothing about the Windows legs, + where the ambient value is whatever the console code page says. Both probes are re-run with the + child pinned to a HOSTILE encoding; the exit codes must be identical, because they are the only + thing asserted. + """ + assert _run_pytest_on(_FAILING_FIXTURE, tmp_path, PYTHONIOENCODING="ascii", PYTHONUTF8="0") != 0 + assert _run_pytest_on(_PASSING_FIXTURE, tmp_path, PYTHONIOENCODING="ascii", PYTHONUTF8="0") == 0 + + +# =================================================================================================== +# `a PR that implements BACKLOG #N must update BACKLOG.md` -- the gate that went green enforcing +# nothing, run as the SHIPPED SHELL against a synthetic repository. +# =================================================================================================== +_HYGIENE_JOB = "banner-on-implementation" + + +def _hygiene_script() -> str: + steps = jobs_of("backlog-hygiene.yml")[_HYGIENE_JOB]["steps"] + script = next(str(s["run"]) for s in steps if "run" in s) + assert "BASE_SHA...$HEAD_SHA" in script or "$BASE_SHA...$HEAD_SHA" in script, ( + "backlog-hygiene.yml's diff is no longer three-dot. The two-dot form reports main-side changes " + "as reverse deltas, which credited every PR with an older base for a docs/BACKLOG.md edit it " + "never made -- the gate went green while enforcing nothing." + ) + return script + + +def _require_bash() -> str: + bash = shutil.which("bash") + if bash is None: # pragma: no cover - every CI leg has bash + pytest.fail( + "bash is required to run backlog-hygiene.yml's own script. This is a FAILURE and not a " + "skip on purpose: ci.yml sets `defaults.run.shell: bash` on every OS, so a leg without " + "bash could not run the gate this control exists to exercise, and a skip there would be a " + "green that proves nothing." + ) + return bash + + +def _fixture_repo(tmp_path: Path, env: dict[str, str]) -> tuple[Path, str, str, str]: + """A repository shaped like the PR the gate exists to police. + + ``A`` is the merge base. ``B`` is the PR head: it changes engine code and NOTHING else. ``C`` is + main moving on AFTER the branch point, touching only ``docs/BACKLOG.md`` -- the shape the archive + move produced in bulk, and the shape the two-dot diff mis-credited. + """ + repo = tmp_path / "fixture" + (repo / "messagefoundry").mkdir(parents=True) + (repo / "docs").mkdir() + _run(["git", "init", "-b", "main", "."], repo, env) + (repo / "messagefoundry" / "engine.py").write_text("x = 1\n", encoding="utf-8") + (repo / "docs" / "BACKLOG.md").write_text("# ledger\n", encoding="utf-8") + _run(["git", "add", "."], repo, env) + _run(["git", "commit", "-m", "A"], repo, env) + base_a = _text(_run(["git", "rev-parse", "HEAD"], repo, env)).strip() + + _run(["git", "checkout", "-b", "pr"], repo, env) + (repo / "messagefoundry" / "engine.py").write_text("x = 2\n", encoding="utf-8") + _run(["git", "commit", "-am", "B: engine only"], repo, env) + head_b = _text(_run(["git", "rev-parse", "HEAD"], repo, env)).strip() + + _run(["git", "checkout", "main"], repo, env) + (repo / "docs" / "BACKLOG.md").write_text("# ledger\n\nmain moved on\n", encoding="utf-8") + _run(["git", "commit", "-am", "C: main-side ledger edit"], repo, env) + base_c = _text(_run(["git", "rev-parse", "HEAD"], repo, env)).strip() + _run(["git", "checkout", "pr"], repo, env) + assert base_a and head_b and base_c and len({base_a, head_b, base_c}) == 3 + return repo, base_c, head_b, base_a + + +def _run_hygiene( + script: str, repo: Path, env: dict[str, str], *, title: str, body: str, base: str, head: str +) -> tuple[int, str]: + path = repo / "gate.sh" + path.write_text(script, encoding="utf-8", newline="\n") + proc = _run( + [_require_bash(), str(path)], + repo, + {**env, "PR_TITLE": title, "PR_BODY": body, "BASE_SHA": base, "HEAD_SHA": head}, + ) + return proc.returncode, _text(proc) + + +@pytest.fixture +def hygiene(tmp_path: Path) -> tuple[str, Path, dict[str, str], str, str]: + env = _child_env( + HOME=str(tmp_path), + GIT_CONFIG_GLOBAL=str(tmp_path / "gitconfig"), + GIT_CONFIG_SYSTEM=str(tmp_path / "gitconfig"), + ) + (tmp_path / "gitconfig").write_text("", encoding="utf-8") + repo, base_c, head_b, _base_a = _fixture_repo(tmp_path, env) + return _hygiene_script(), repo, env, base_c, head_b + + +def test_the_backlog_hygiene_gate_fails_a_code_pr_that_leaves_the_ledger_alone( + hygiene: tuple[str, Path, dict[str, str], str, str], +) -> None: + """PLANTED: a PR that claims `BACKLOG #42`, changes engine code, and never touches the ledger -- + while main has separately moved docs/BACKLOG.md since the branch point. + + That last clause is the whole point. The gate computed its changed-file list with a two-dot + `git diff "$BASE_SHA" "$HEAD_SHA"`, which reports main-side changes as REVERSE deltas, so any + main-side edit to docs/BACKLOG.md credited every PR with an older base. It went green while + enforcing nothing, on exactly the population it exists to police. + """ + script, repo, env, base, head = hygiene + code, out = _run_hygiene( + script, repo, env, title="feat: something (BACKLOG #42)", body="", base=base, head=head + ) + print(f"[#1000] shipped three-dot gate exit={code}\n{out}") + assert code == 1, f"the gate passed a PR it exists to fail. exit={code}\n{out}" + assert "does not\ntouch docs/BACKLOG.md" in out or "docs/BACKLOG.md" in out + + +def test_the_two_dot_form_of_the_gate_passes_the_same_planted_pull_request( + hygiene: tuple[str, Path, dict[str, str], str, str], +) -> None: + """RUN AGAINST THE PRE-FIX GATE, which is what makes the control above evidence rather than a + claim. The identical fixture, with only the diff form reverted, must go GREEN. + + If both forms failed, the fixture would be proving something else -- and the recorded defect would + be unreproduced. + """ + script, repo, env, base, head = hygiene + pre_fix = script.replace('"$BASE_SHA...$HEAD_SHA"', '"$BASE_SHA" "$HEAD_SHA"') + assert pre_fix != script, ( + "the two-dot substitution matched nothing, so this test compares the shipped gate with itself" + ) + code, out = _run_hygiene( + pre_fix, repo, env, title="feat: something (BACKLOG #42)", body="", base=base, head=head + ) + print(f"[#1000] pre-fix two-dot gate exit={code}\n{out}") + assert code == 0, ( + "the two-dot form no longer reproduces the recorded defect, so the three-dot assertion above " + f"is not measuring what it says. exit={code}\n{out}" + ) + + +@pytest.mark.parametrize( + ("case", "title", "body", "touch"), + [ + ("no claim at all", "chore: tidy", "", None), + ("claims and updates the ledger", "feat (BACKLOG #42)", "", "docs/BACKLOG.md"), + ( + "claims and updates an ARCHIVED item", + "feat (BACKLOG #42)", + "", + "docs/archive/backlog/x.md", + ), + ("a bare #42 is a PR number, not a claim", "fix for #42", "see #42", None), + ], +) +def test_the_backlog_hygiene_gate_leaves_the_benign_shapes_alone( + hygiene: tuple[str, Path, dict[str, str], str, str], + case: str, + title: str, + body: str, + touch: str | None, +) -> None: + """THE ASYMMETRY, four shapes wide. + + A gate that failed everything would satisfy the planted case and block every PR in the repo. Each + row is a shape the rule deliberately does NOT break: no claim, a claim honoured in the live ledger, + a claim honoured in the ARCHIVE (an item retired between the claim and the PR), and the `#42` + spelling that is a PR number in this repo rather than an item. + """ + script, repo, env, base, head = hygiene + if touch: + target = repo / touch + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("banner\n", encoding="utf-8") + _run(["git", "add", "-A"], repo, env) + _run(["git", "commit", "-m", f"branch-side {touch}"], repo, env) + head = _text(_run(["git", "rev-parse", "HEAD"], repo, env)).strip() + code, out = _run_hygiene(script, repo, env, title=title, body=body, base=base, head=head) + print(f"[#1000] benign case {case!r} exit={code}") + assert code == 0, f"the gate failed a benign PR shape ({case}). exit={code}\n{out}" + + +# =================================================================================================== +# `gitleaks (secret scan)` -- the allowlist is the neutering path a scope test cannot see. +# =================================================================================================== +_GITLEAKS = ROOT / ".gitleaks.toml" + + +def _gitleaks_allowlist_regexes() -> list[str]: + parsed = tomllib.loads(_GITLEAKS.read_text(encoding="utf-8")) + return [str(r) for r in (parsed.get("allowlist") or {}).get("regexes", [])] + + +def _fabricated_secrets() -> list[str]: + """Credential-shaped strings ASSEMBLED AT RUNTIME. + + Never committed as literals: gitleaks scans this repository and cannot tell a well-known test + vector from a live credential -- the sibling redaction suite already had a fixture rejected for + exactly that, correctly. Assembling from parts keeps the scanner useful on this file. + """ + hexish = "0123456789abcdef" * 3 + return [ + "AKIA" + "Q" * 16, + "ghp_" + "z" * 36, + "xoxb-" + "1" * 12 + "-" + "2" * 24, + "-----BEGIN " + "RSA PRIVATE KEY-----", + hexish[:40], + "postgres://svc:" + "P" * 24 + "@db.invalid:5432/x", + ] + + +def _swallowing(regexes: list[str], corpus: list[str]) -> list[str]: + return [r for r in regexes if any(re.search(r, s) for s in corpus)] + + +def test_no_gitleaks_allowlist_regex_swallows_a_fabricated_secret() -> None: + """PLANTED: six credential shapes gitleaks' default ruleset exists to catch. + + ``.gitleaks.toml`` is the one place this required context can be disabled without touching a + workflow: an allowlist entry broad enough to match a real credential turns the gate green while it + keeps scanning. Every shipped entry is deliberately a LITERAL or a tightly bounded pattern, and + this is the assertion that keeps it that way. + """ + regexes = _gitleaks_allowlist_regexes() + print( + f"[#1000] scanned {len(regexes)} gitleaks allowlist regexes against " + f"{len(_fabricated_secrets())} fabricated credential shapes" + ) + assert regexes, ( + ".gitleaks.toml parsed to ZERO allowlist regexes -- the format changed under this" + ) + swallowed = _swallowing(regexes, _fabricated_secrets()) + assert not swallowed, ( + f"these allowlist regexes match a fabricated credential: {swallowed}. NEVER allowlist a real " + "secret; a broad entry here neuters a required context with no workflow edit at all." + ) + + +def test_the_allowlist_narrowness_detector_fires_on_a_planted_broad_regex() -> None: + """NEGATIVE CONTROL OF THE CONTROL, and the ASYMMETRY. + + "The shipped allowlist is narrow" and "the detector matches nothing" are the same green. Two broad + patterns are planted and must be caught; the shipped entries must stay clean in the same call, so + the control is not merely demanding an empty allowlist -- which would delete a legitimate, + documented set of non-secret fixtures. + """ + planted = [r".{8,}", r"[A-Za-z0-9_/+-]{20,}"] + assert _swallowing(planted, _fabricated_secrets()) == planted, ( + "the detector cannot see a broad regex" + ) + assert not _swallowing(_gitleaks_allowlist_regexes(), _fabricated_secrets()) + + +def test_the_gitleaks_config_still_extends_the_default_ruleset() -> None: + """`useDefault = false` empties the ruleset and the job then scans for the project's own rules + only -- of which there are none. It reports success in seconds.""" + parsed = tomllib.loads(_GITLEAKS.read_text(encoding="utf-8")) + assert parsed.get("extend", {}).get("useDefault") is True, ( + ".gitleaks.toml no longer extends the default ruleset; the secret scan has nothing to match" + ) + + +# =================================================================================================== +# `bandit (Python SAST)` and `npm-audit` -- a severity floor mutes a gate without touching its scope. +# =================================================================================================== +def _step_run(workflow: str, job: str, name_fragment: str) -> str: + for step in jobs_of(workflow)[job].get("steps", []): + if name_fragment in str(step.get("name", "")): + return str(step.get("run", "")) + raise AssertionError(f"{workflow}:{job} has no step named like {name_fragment!r}") + + +#: Flags that keep a scanner running while discarding part of what it finds. NOT the `|| true` family +#: (tests/test_security_posture.py owns that) -- these leave the exit code intact and shrink the input +#: to it, which a neutering scan looking for added idioms cannot see. +_MUTING_FLAGS = { + "bandit": ( + r"(? list[str]: + body = "\n".join(line for line in command.splitlines() if not line.lstrip().startswith("#")) + return [p for p in _MUTING_FLAGS[family] if re.search(p, body)] + + +def test_the_bandit_invocation_carries_no_severity_or_confidence_floor() -> None: + """PLANTED via the detector: `bandit -ll` reports only MEDIUM and above and still exits non-zero + on what is left, so the job stays green-looking and blocking while it stops reporting a whole + severity band. The scope test next door asks WHAT it is pointed at; this asks what it keeps.""" + command = _step_run("security.yml", "bandit", "Scan source for insecure patterns") + found = _muted(command, "bandit") + print(f"[#1000] bandit invocation scanned for {len(_MUTING_FLAGS['bandit'])} muting flags") + assert not found, ( + f"the bandit invocation carries {found}, which discards findings while the required context " + f"keeps reporting success. Command: {command!r}" + ) + + +def test_the_npm_audit_invocation_carries_no_severity_floor() -> None: + """`npm audit --audit-level=high` exits 0 on moderate advisories. security.yml's own comment says + the default level "fails on ANY severity, matching pip-audit's strict posture" -- this is the + assertion that keeps that sentence true.""" + command = _step_run("security.yml", "npm-audit", "Audit the locked npm dependencies") + found = _muted(command, "npm") + assert not found, f"the npm audit invocation carries {found}: {command!r}" + + +def test_the_muting_detector_fires_on_a_synthetic_floor() -> None: + """NEGATIVE CONTROL OF THE CONTROL, plus the ASYMMETRY that matters here: the detector must NOT + fire on the reviewed `--skip B101,...` list, which is a per-check exclusion with a stated reason + for each entry -- not a severity floor. A detector that flagged it would be "fixed" by deleting a + correct annotation.""" + assert _muted("bandit -r . -ll --skip B101", "bandit") == [r"(? None: + """`npm audit --package-lock-only` reads the committed lockfile. Without it the job errors or + audits an empty tree, and `working-directory: ide` is the only thing pointing it at one.""" + workflow = load_workflow("security.yml") + job = workflow["jobs"]["npm-audit"] + workdir = str(((job.get("defaults") or {}).get("run") or {}).get("working-directory", "")) + assert workdir, "the npm-audit job lost its working-directory; it would audit the repo root" + lock = ROOT / workdir / "package-lock.json" + assert lock.is_file(), f"{lock} does not exist, so --package-lock-only has nothing to audit" + + +# =================================================================================================== +# `cla` -- the context string IS the control surface, and it has been wrong in this repo before. +# =================================================================================================== +def test_the_cla_job_still_reports_under_the_job_key_and_not_the_workflow_name() -> None: + """PLANTED historically, not synthetically: docs/CI.md and cla.yml both told a reader to require + "CLA Assistant" -- the WORKFLOW name, which matches no status check. Adding it to branch + protection would have wedged every PR forever. + + The detection this gate performs lives entirely in a third-party action, so what is controllable + here is whether the context can report at all: the job must declare no `name:` (making the context + its KEY, `cla`) and must run on a pull-request trigger. + """ + jobs = jobs_of("cla.yml") + assert "cla" in jobs, f"cla.yml's job key is no longer `cla`: {sorted(jobs)}" + assert "name" not in jobs["cla"], ( + "the cla job declared a `name:`, which changes its status-check context string. Branch " + "protection still requires `cla`, so the context would never report and every PR would wedge." + ) + workflow = load_workflow("cla.yml") + triggers = workflow.get("on") or workflow.get(True) or {} + assert "pull_request_target" in triggers or "pull_request" in triggers, ( + f"cla.yml has no pull-request trigger ({sorted(triggers)}), so the required `cla` context can " + "never report -- the required-but-absent trap" + ) + + +def test_the_cla_allowlist_is_an_enumeration_and_not_a_glob() -> None: + """THE ASYMMETRY, and a recorded finding rather than a hypothetical: a `bot*` glob would let any + human whose username begins with "bot" skip signing (review low-28). The allowlist is legitimate + and must keep working -- so this asserts its SHAPE, not its absence.""" + step = next(s for s in jobs_of("cla.yml")["cla"]["steps"] if "with" in s) + allowlist = str(step["with"]["allowlist"]) + assert allowlist.strip(), ( + "the CLA allowlist emptied; every maintainer push would need a signature" + ) + assert "*" not in allowlist, f"the CLA allowlist contains a glob: {allowlist!r}" diff --git a/tests/test_negative_controls.py b/tests/test_negative_controls.py new file mode 100644 index 00000000..34f49f46 --- /dev/null +++ b/tests/test_negative_controls.py @@ -0,0 +1,163 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Every required merge context must have a negative control (BACKLOG #1000). + +THE DEFECT THIS EXISTS FOR. Thirteen contexts are the entire merge gate and, before this, not one of +them was proven able to go red. The class has fired at least four times here with no CI signal, each +found by hand: a required backlog gate computing a two-dot diff and crediting every PR with an older +base; a required SAST gate scanning a two-directory allow-list; a leak gate exiting 0 on content +carrying a real site code; the same gate matching one of four spellings of a Windows path. Each was +filed as its own defect, which is right. None of them established the property that would have caught +all four -- a green run is evidence only if the gate has been shown it can go red on that class. + +THIS IS THE CI JOB THE ITEM ASKS FOR, and it deliberately is NOT a new workflow. Running the +reconciliation inside the ``test`` legs makes it BLOCKING today through contexts that are already +required, so it adds no new required context and needs no branch-protection change. A new advisory +workflow would have been weaker (advisory jobs do not stop auto-merge) and a new required one is an +owner decision that also has a live incident history: on 2026-07-29 protection was briefly cut to a +single required context with auto-merge armed and zero approvals, and PRs merged in that window. + +THE INSTRUMENT HAS TO ANSWER THE QUESTION ASKED OF IT, and "it runs in CI" is not the same sentence as +"it runs on the pull request shape it exists for". ``ci.yml`` gates the ``test`` legs' expensive steps +on ``needs.changes.outputs.code == 'true'``, and that coupling is exactly how a #320 banner merged +without the suite ever compiling. The decay mode this registry targets is a context arriving in branch +protection and being mirrored into ``.github/required-contexts.txt`` -- so both that file and this +registry must classify as CODE, not docs. Measured, and then pinned: they are now two rows in +``tests/test_ci_docs_only_detector.py::test_code_paths_run_the_suite`` rather than a sentence here. + +``.github/required-contexts.txt`` IS READ-ONLY TO THIS FILE. It mirrors the live server, and its own +header states the ordering rule -- branch protection first, then the file. Everything here only reads +it, so the registry can never be the reason a context looks required when it is not. + +NOT VACUOUS BY CONSTRUCTION, which is the failure mode of every coverage register. The reconciliation +is driven by the LIVE required set rather than by the registry, so a context added to branch protection +and mirrored into that file arrives here with zero controls and fails -- rather than simply not being +looked at. ``test_the_reconciliation_fails_when_a_context_loses_its_control`` proves the gate can say so +by removing a control and watching it. +""" + +from __future__ import annotations + +from tests import _negative_controls as reg +from tests._workflow_contexts import required_contexts + + +def test_the_registry_covers_every_required_context() -> None: + """THE GATE. It reports WHAT it scanned, not merely a count -- "no gaps" and "nothing was read" + are otherwise the same green, which is the shape of half the defects this registry indexes.""" + problems, coverage = reg.reconcile() + print(f"[#1000] reconciling {len(coverage)} required contexts against {reg.REGISTRY.name}") + for ctx in sorted(coverage): + print(f"[#1000] {coverage[ctx]} control(s): {ctx}") + assert coverage, ( + ".github/required-contexts.txt parsed to ZERO contexts, so this reconciliation compared " + "nothing. That is a broken check, not a clean sweep." + ) + assert not problems, "negative-control registry problems:\n " + "\n ".join(problems) + + +def test_every_required_context_is_covered_by_a_context_specific_control() -> None: + """A universal control (`no required job carries continue-on-error`) is real and valuable, and it + is NOT what this item asks for. It proves a gate cannot be switched off; it says nothing about + whether the gate can see the violation it exists for. Each context needs at least one control + planted at its own subject matter, which is what the per-context registry entries are.""" + _, coverage = reg.reconcile() + uncovered = sorted(ctx for ctx, n in coverage.items() if n < 1) + assert not uncovered, f"contexts with no control of their own: {uncovered}" + + +def test_the_registered_control_count_is_reported_and_has_not_collapsed() -> None: + """A liveness floor. The registry shrinking is how this decays -- not by anyone deciding a gate no + longer needs proving, but by a control being deleted alongside the test it names.""" + controls = reg.load() + red = sum(len(c.red) for c in controls) + green = sum(len(c.green) for c in controls) + print( + f"[#1000] {len(controls)} controls: {red} planted-violation nodes, {green} asymmetry nodes" + ) + assert len(controls) >= len(required_contexts()) + assert red >= 20 and green >= 15, ( + f"the registry collapsed to {red} red / {green} green nodes. Raise this floor when it " + "legitimately grows; never lower it to make the suite pass." + ) + + +# --- The gate's own negative controls. A gate that has never been red is a claim. ------------------- + + +def _problems_for(controls: list[reg.Control]) -> list[str]: + """Run the same rules over a synthetic control list, without touching the file on disk.""" + coverage = dict.fromkeys(required_contexts(), 0) + problems: list[str] = [] + for control in controls: + if control.context in coverage: + coverage[control.context] += 1 + problems.extend( + f"required context {ctx!r} has NO negative control" for ctx, n in coverage.items() if n == 0 + ) + problems.extend(f"dangling control node: {m}" for m in reg.unresolved_nodes(controls)) + return problems + + +def test_the_reconciliation_fails_when_a_context_loses_its_control() -> None: + """PLANTED: the `gitleaks (secret scan)` entry is dropped. The gate must name that context. + + Without this, "every context is covered" and "the reconciliation compares nothing" produce the + same green -- which is the defect this whole registry exists to make visible, occurring inside the + verification of its own fix. + """ + controls = [c for c in reg.load() if c.context != "gitleaks (secret scan)"] + problems = _problems_for(controls) + assert any("gitleaks (secret scan)" in p for p in problems), problems + print(f"[#1000] negative control: dropping one entry produced {len(problems)} problem(s)") + + +def test_the_reconciliation_stays_green_on_the_shipped_registry() -> None: + """THE ASYMMETRY. A reconciliation that reported a problem for everything would satisfy the test + above while being useless, and the two are indistinguishable from a red alone.""" + assert _problems_for(reg.load()) == [] + + +def test_the_node_resolver_reports_a_test_that_does_not_exist() -> None: + """PLANTED: a control naming a test function nobody wrote, and one naming a missing file. + + A registry of dangling node ids satisfies every count-based assertion above. Both spellings of the + failure are planted because they take different branches of the resolver, and a resolver that only + caught the missing FILE would pass a renamed TEST -- which is the likelier accident. + """ + ghost = reg.Control( + context="CI gate", + plants="x" * 50, + holds="x" * 50, + observed="x" * 50, + red=("tests/test_merge_gate_controls.py::test_this_function_does_not_exist",), + green=("tests/test_no_such_file_at_all.py::test_whatever",), + ci=None, + workflow=None, + ) + missing = reg.unresolved_nodes([ghost]) + assert len(missing) == 2, missing + assert any("no test function of that name" in m for m in missing), missing + assert any("no such file" in m for m in missing), missing + # ...and it stays silent on the real ones, so the assertion above is about the ghost. + assert reg.unresolved_nodes(reg.load()) == [] + + +def test_the_ci_wiring_check_reports_a_command_nobody_invokes() -> None: + """PLANTED: a `ci` control naming a command that appears in no step of the workflow it claims. + + This is the quiet way a fixture-based control dies: the asserter script stays in the tree, the + registry keeps pointing at it, and the step that ran it is deleted. Nothing else here would notice. + """ + orphan = reg.Control( + context="semgrep (project SAST rules)", + plants="x" * 50, + holds="x" * 50, + observed="x" * 50, + red=(), + green=(), + ci="python scripts/ci/this_asserter_is_not_wired.py", + workflow="security.yml", + ) + assert reg.unwired_ci_commands([orphan]), "the wiring check cannot see an uninvoked command" + assert reg.unwired_ci_commands(reg.load()) == [], "a shipped ci control is not actually wired" From 53ae7cc109febb226dc59b3ae6326f0dcab7d766 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Mon, 10 Aug 2026 17:34:45 -0500 Subject: [PATCH 3/6] fix(test): the backlog-hygiene control resolved bash from PATH, so its green was a fact about PATH order (BACKLOG #1000) Caught before integration by running the lane under a deliberately hostile ambient environment rather than only the shell it was written in. The control ran backlog-hygiene.yml's own script via shutil.which("bash") with an ABSOLUTE Windows path for the script. Under a Git Bash parent that passed. From PowerShell, where PATH resolves `bash` to C:\Windows\System32\bash.exe -- the WSL launcher, which runs in a different filesystem namespace -- all six hygiene assertions failed with exit 127 and the backslashes eaten from the path. The verdict was a fact about which shell happened to be first on PATH. Three changes, each asserted rather than described: - bash is derived from `git` (Git for Windows ships bash beside it) and only then falls back to PATH. git is already required by these tests, so it is the deterministic anchor. - the chosen candidate must READ A TOKEN this process just wrote before any verdict from it is believed -- a live positive control for the namespace, not a pattern match on the word "system32". - the script is invoked by a RELATIVE path, so no namespace or backslash conversion is involved at all, and 126/127 are now a hard failure rather than a gate verdict. A broken invocation read as "the gate refused this PR" is the probe defect #1000 itself records. test_the_bash_namespace_probe_rejects_an_interpreter_that_cannot_see_the_fixture is the negative control for the resolver: sys.executable is a real runnable program that is not a shell, so the probe must refuse it while accepting the resolved bash in the same call. Verified green under PYTHONIOENCODING=cp1252, PYTHONUTF8=0, LC_ALL=tr_TR.UTF-8, a global core.autocrlf, and a PATH ordered so the WSL bash comes first -- and unchanged under the ordinary Git Bash ambient. --- tests/negative_controls.toml | 12 +- tests/test_merge_gate_controls.py | 186 +++++++++++++++++++++++++----- 2 files changed, 165 insertions(+), 33 deletions(-) diff --git a/tests/negative_controls.toml b/tests/negative_controls.toml index 63d8e389..902fb930 100644 --- a/tests/negative_controls.toml +++ b/tests/negative_controls.toml @@ -375,7 +375,8 @@ number in this repo rather than an item. A gate that failed all four would satis block every pull request in the repository. """ green = [ - "tests/test_merge_gate_controls.py::test_the_backlog_hygiene_gate_leaves_the_benign_shapes_alone" + "tests/test_merge_gate_controls.py::test_the_backlog_hygiene_gate_leaves_the_benign_shapes_alone", + "tests/test_merge_gate_controls.py::test_the_bash_namespace_probe_rejects_an_interpreter_that_cannot_see_the_fixture", ] observed = """ RUN AGAINST THE PRE-FIX GATE, which is what makes this evidence rather than a claim. The identical @@ -383,4 +384,13 @@ fixture goes RED (exit 1) through the shipped three-dot diff and GREEN (exit 0) reverted to two-dot. That reproduces the recorded defect exactly: the two-dot form reports main-side changes as reverse deltas, so any main-side edit to docs/BACKLOG.md credited every PR with an older base, and the gate went green while enforcing nothing -- on precisely the population it polices. + +AND THE CONTROL ITSELF WAS CAUGHT DEPENDING ON ITS AMBIENT ENVIRONMENT, 2026-08-10, before it shipped. +Its first version resolved bash from PATH and passed an absolute Windows path; under a Git Bash parent +it passed, and from PowerShell -- where PATH resolves `bash` to the WSL launcher, a different +filesystem namespace -- all six hygiene assertions failed with exit 127. bash is now derived from +`git`, made to read a file this process wrote before any verdict is believed, and handed a RELATIVE +script path; 126/127 are a hard failure rather than a gate verdict. Re-measured green under a +deliberately hostile ambient (PYTHONIOENCODING=cp1252, PYTHONUTF8=0, LC_ALL=tr_TR.UTF-8, a global +core.autocrlf, and PATH ordered so the WSL bash comes first). """ diff --git a/tests/test_merge_gate_controls.py b/tests/test_merge_gate_controls.py index ab7399a0..8f7aee5e 100644 --- a/tests/test_merge_gate_controls.py +++ b/tests/test_merge_gate_controls.py @@ -276,16 +276,64 @@ def _hygiene_script() -> str: return script -def _require_bash() -> str: - bash = shutil.which("bash") - if bash is None: # pragma: no cover - every CI leg has bash - pytest.fail( - "bash is required to run backlog-hygiene.yml's own script. This is a FAILURE and not a " - "skip on purpose: ci.yml sets `defaults.run.shell: bash` on every OS, so a leg without " - "bash could not run the gate this control exists to exercise, and a skip there would be a " - "green that proves nothing." - ) - return bash +def _bash_candidates() -> list[Path]: + """Every plausible bash, GIT-DERIVED FIRST. + + ``shutil.which("bash")`` alone is a fact about PATH, and on Windows PATH order decides WHICH + OPERATING SYSTEM answers: ``C:\\Windows\\System32\\bash.exe`` is the WSL launcher, whose filesystem + namespace is not the one this process just wrote a fixture into. Git for Windows always ships bash + beside git, so git -- which every test here already requires -- is the deterministic anchor. + """ + found: list[Path] = [] + git = shutil.which("git") + if git: + # `/cmd/git.exe`, `/bin/git.exe` and `/mingw64/bin/git.exe` are all shipped + # layouts, so walk up and try both bash homes from each level. + for parent in Path(git).resolve().parents: + for rel in ("bin/bash.exe", "usr/bin/bash.exe", "bin/bash"): + found.append(parent / rel) + on_path = shutil.which("bash") + if on_path: + found.append(Path(on_path)) + return found + + +def _bash_sees(bash: Path, tmp_path: Path) -> bool: + """LIVE POSITIVE CONTROL for the namespace, not a guess from the path string. + + Rejecting ``system32`` by name would be a pattern match on a spelling. This writes a token into + the directory the fixture will live in and requires the candidate to read it back -- if it cannot, + it is looking at a different filesystem and every verdict it returns would be about nothing. + """ + probe = tmp_path / "mf_bash_probe.txt" + probe.write_text("MFPROBE-OK\n", encoding="utf-8") + try: + out = _run([str(bash), "-c", "cat mf_bash_probe.txt"], tmp_path, _child_env()) + except OSError: + return False + return out.returncode == 0 and b"MFPROBE-OK" in out.stdout + + +def _require_bash(tmp_path: Path) -> str: + """A bash that can see this process's files, or a loud failure -- never a skip. + + ci.yml sets ``defaults.run.shell: bash`` on every OS, so a leg without a usable bash could not run + the gate this control exercises, and a skip there would be a green that proves nothing. + """ + tried: list[str] = [] + for candidate in _bash_candidates(): + if not candidate.is_file(): + continue + tried.append(str(candidate)) + if _bash_sees(candidate, tmp_path): + print(f"[#1000] bash resolved to {candidate} (namespace probe passed)") + return str(candidate) + pytest.fail( + "no bash on this machine can read a file this process just wrote. Tried: " + f"{tried or '(none found)'}. On Windows, `bash` on PATH is often " + "C:\\Windows\\System32\\bash.exe -- the WSL launcher, which runs in a different filesystem " + "namespace, and a control that ran there would be measuring nothing." + ) def _fixture_repo(tmp_path: Path, env: dict[str, str]) -> tuple[Path, str, str, str]: @@ -320,32 +368,92 @@ def _fixture_repo(tmp_path: Path, env: dict[str, str]) -> tuple[Path, str, str, def _run_hygiene( - script: str, repo: Path, env: dict[str, str], *, title: str, body: str, base: str, head: str + bash: str, + script: str, + repo: Path, + env: dict[str, str], + *, + title: str, + body: str, + base: str, + head: str, ) -> tuple[int, str]: - path = repo / "gate.sh" - path.write_text(script, encoding="utf-8", newline="\n") + """Run the workflow's own script, and VALIDATE THE SHAPE OF THE RESULT before returning it. + + The script path is passed RELATIVE to ``cwd``. An absolute Windows path is not portable across + bash builds -- backslashes are escape characters and a drive letter means nothing outside the + Windows namespace -- and the mangling presents as "no such file", i.e. as exit 127, which a caller + comparing `code != 0` would happily read as "the gate refused this PR". + + So 126/127 are a hard failure here rather than a verdict. That is the generalisable half of the + probe defect recorded in BACKLOG #1000: a probe must validate its own output rather than treating + a broken invocation as an answer. + """ + (repo / "gate.sh").write_text(script, encoding="utf-8", newline="\n") proc = _run( - [_require_bash(), str(path)], + [bash, "gate.sh"], repo, {**env, "PR_TITLE": title, "PR_BODY": body, "BASE_SHA": base, "HEAD_SHA": head}, ) - return proc.returncode, _text(proc) + out = _text(proc) + assert proc.returncode not in (126, 127), ( + f"bash could not execute the gate script (exit {proc.returncode}): {out.strip()[:300]}. That " + "is not a gate verdict -- it is a broken invocation, and reading it as one would make every " + "assertion here vacuous." + ) + # Printed ASCII-safe: the gate's own error text carries a status glyph, and a Windows console + # under cp1252 would turn printing it into a UnicodeEncodeError -- an assertion about the gate + # lost to a property of the terminal. + return proc.returncode, out @pytest.fixture -def hygiene(tmp_path: Path) -> tuple[str, Path, dict[str, str], str, str]: +def hygiene(tmp_path: Path) -> tuple[str, str, Path, dict[str, str], str, str]: env = _child_env( HOME=str(tmp_path), GIT_CONFIG_GLOBAL=str(tmp_path / "gitconfig"), GIT_CONFIG_SYSTEM=str(tmp_path / "gitconfig"), ) (tmp_path / "gitconfig").write_text("", encoding="utf-8") + bash = _require_bash(tmp_path) repo, base_c, head_b, _base_a = _fixture_repo(tmp_path, env) - return _hygiene_script(), repo, env, base_c, head_b + return bash, _hygiene_script(), repo, env, base_c, head_b + + +def _ascii(text: str) -> str: + """Child output, safe to print under any console code page.""" + return text.encode("ascii", "backslashreplace").decode("ascii") + + +def test_the_bash_namespace_probe_rejects_an_interpreter_that_cannot_see_the_fixture( + tmp_path: Path, +) -> None: + """NEGATIVE CONTROL OF THE RESOLVER, and it is here because this file already shipped the bug once. + + MEASURED 2026-08-10. The first version resolved bash with ``shutil.which("bash")`` and passed it an + ABSOLUTE Windows path. Under a Git Bash parent it passed; run from PowerShell, where PATH resolves + ``bash`` to ``C:\\Windows\\System32\\bash.exe`` -- the WSL launcher, a different filesystem + namespace -- every hygiene control failed with exit 127 and the backslashes eaten. So the control's + verdict was a fact about PATH ORDER, which is precisely the ambient-environment green the wave-2 + incident warned about, one variable over. + + Two things fixed it and both are asserted here rather than described: the candidate is derived from + ``git`` (which ships bash beside it) and then made to READ A FILE this process wrote, and the + script is invoked by a RELATIVE path so no namespace conversion is involved at all. + + A candidate that cannot read the token must be rejected. ``sys.executable`` stands in for one: it + is a real, runnable program that is not a shell, so the probe must refuse it while accepting the + resolved bash in the same call. + """ + assert not _bash_sees(Path(sys.executable), tmp_path), ( + "the namespace probe accepted a non-shell interpreter, so it cannot reject a bash that is " + "looking at the wrong filesystem either" + ) + assert _bash_sees(Path(_require_bash(tmp_path)), tmp_path) def test_the_backlog_hygiene_gate_fails_a_code_pr_that_leaves_the_ledger_alone( - hygiene: tuple[str, Path, dict[str, str], str, str], + hygiene: tuple[str, str, Path, dict[str, str], str, str], ) -> None: """PLANTED: a PR that claims `BACKLOG #42`, changes engine code, and never touches the ledger -- while main has separately moved docs/BACKLOG.md since the branch point. @@ -355,17 +463,24 @@ def test_the_backlog_hygiene_gate_fails_a_code_pr_that_leaves_the_ledger_alone( main-side edit to docs/BACKLOG.md credited every PR with an older base. It went green while enforcing nothing, on exactly the population it exists to police. """ - script, repo, env, base, head = hygiene + bash, script, repo, env, base, head = hygiene code, out = _run_hygiene( - script, repo, env, title="feat: something (BACKLOG #42)", body="", base=base, head=head + bash, + script, + repo, + env, + title="feat: something (BACKLOG #42)", + body="", + base=base, + head=head, ) - print(f"[#1000] shipped three-dot gate exit={code}\n{out}") - assert code == 1, f"the gate passed a PR it exists to fail. exit={code}\n{out}" - assert "does not\ntouch docs/BACKLOG.md" in out or "docs/BACKLOG.md" in out + print(f"[#1000] shipped three-dot gate exit={code}\n{_ascii(out)}") + assert code == 1, f"the gate passed a PR it exists to fail. exit={code}\n{_ascii(out)}" + assert "docs/BACKLOG.md" in out def test_the_two_dot_form_of_the_gate_passes_the_same_planted_pull_request( - hygiene: tuple[str, Path, dict[str, str], str, str], + hygiene: tuple[str, str, Path, dict[str, str], str, str], ) -> None: """RUN AGAINST THE PRE-FIX GATE, which is what makes the control above evidence rather than a claim. The identical fixture, with only the diff form reverted, must go GREEN. @@ -373,18 +488,25 @@ def test_the_two_dot_form_of_the_gate_passes_the_same_planted_pull_request( If both forms failed, the fixture would be proving something else -- and the recorded defect would be unreproduced. """ - script, repo, env, base, head = hygiene + bash, script, repo, env, base, head = hygiene pre_fix = script.replace('"$BASE_SHA...$HEAD_SHA"', '"$BASE_SHA" "$HEAD_SHA"') assert pre_fix != script, ( "the two-dot substitution matched nothing, so this test compares the shipped gate with itself" ) code, out = _run_hygiene( - pre_fix, repo, env, title="feat: something (BACKLOG #42)", body="", base=base, head=head + bash, + pre_fix, + repo, + env, + title="feat: something (BACKLOG #42)", + body="", + base=base, + head=head, ) - print(f"[#1000] pre-fix two-dot gate exit={code}\n{out}") + print(f"[#1000] pre-fix two-dot gate exit={code}\n{_ascii(out)}") assert code == 0, ( "the two-dot form no longer reproduces the recorded defect, so the three-dot assertion above " - f"is not measuring what it says. exit={code}\n{out}" + f"is not measuring what it says. exit={code}\n{_ascii(out)}" ) @@ -403,7 +525,7 @@ def test_the_two_dot_form_of_the_gate_passes_the_same_planted_pull_request( ], ) def test_the_backlog_hygiene_gate_leaves_the_benign_shapes_alone( - hygiene: tuple[str, Path, dict[str, str], str, str], + hygiene: tuple[str, str, Path, dict[str, str], str, str], case: str, title: str, body: str, @@ -416,7 +538,7 @@ def test_the_backlog_hygiene_gate_leaves_the_benign_shapes_alone( a claim honoured in the ARCHIVE (an item retired between the claim and the PR), and the `#42` spelling that is a PR number in this repo rather than an item. """ - script, repo, env, base, head = hygiene + bash, script, repo, env, base, head = hygiene if touch: target = repo / touch target.parent.mkdir(parents=True, exist_ok=True) @@ -424,9 +546,9 @@ def test_the_backlog_hygiene_gate_leaves_the_benign_shapes_alone( _run(["git", "add", "-A"], repo, env) _run(["git", "commit", "-m", f"branch-side {touch}"], repo, env) head = _text(_run(["git", "rev-parse", "HEAD"], repo, env)).strip() - code, out = _run_hygiene(script, repo, env, title=title, body=body, base=base, head=head) + code, out = _run_hygiene(bash, script, repo, env, title=title, body=body, base=base, head=head) print(f"[#1000] benign case {case!r} exit={code}") - assert code == 0, f"the gate failed a benign PR shape ({case}). exit={code}\n{out}" + assert code == 0, f"the gate failed a benign PR shape ({case}). exit={code}\n{_ascii(out)}" # =================================================================================================== From 6c47eedb7f2028be71e5c7dd897d0bc8b9859d16 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Mon, 10 Aug 2026 17:44:02 -0500 Subject: [PATCH 4/6] fix(coord): record every claim release, -Force included, in claims/.history (BACKLOG #1068) `claim.ps1 -Release` was `Remove-Item` and nothing else, so a `-Force` takeover left no record of who released whose claim, when, or why. The escape hatch is NECESSARY and stays: a claim whose holder worktree is gone would otherwise be stuck forever, and the alternative people reach for is hand-deleting the file, which leaves less evidence still. The item is auditability, not prevention. Every release now appends one JSON line to /mefor-coord/claims/.history -- key, releasing worktree and branch, prior holder, its branch and note, when the claim was taken, and force true/false. RECORD FIRST, then remove, and refuse the release if the record cannot be written. Both orders can lie once and only one lie is recoverable: recording after the removal reproduces this defect exactly (a completed release with nothing left to write the record from), whereas recording first can at worst claim a release that then failed -- which a `release-failed` correction line handles. Refusing is safe because a release is always retryable; the claim simply stays where it was. JSON Lines, LF-only, written in a single Write to a handle opened FileMode::Append + FileShare::Read, so two worktrees releasing in the same instant cannot interleave a record. It sits inside the claims directory safely: every reader there keys on a file NAME (-List and prune-merged.ps1 glob *.json, claim_check.py opens .json) and ConvertTo-KeyFile always appends ".json", so no key can fold onto ".history". tests/test_coord_claim_release_history.py was red first -- 8 of 10 against the unchanged script. The two that passed pre-fix are guards against a WRONG fix (a refused release and an unclaimed key must record nothing). Per BACKLOG #1000 it carries the negatives a "somebody released something" record would pass: a -Force takeover must name the prior holder and the releaser as different paths, and force must record false when not passed. --- docs/SESSION-DRIFT-CONTROLS.md | 7 + scripts/coord/claim.ps1 | 112 ++++++++- tests/test_coord_claim_release_history.py | 283 ++++++++++++++++++++++ 3 files changed, 397 insertions(+), 5 deletions(-) create mode 100644 tests/test_coord_claim_release_history.py diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md index b49ed080..2b759586 100644 --- a/docs/SESSION-DRIFT-CONTROLS.md +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -103,6 +103,13 @@ Frequently forgotten in discussions of "the gate", but it is the same problem cl `-Release` then `-Take` — which drops the claim in between and re-opens the race it exists to close. The note is what `announce-session.ps1` broadcasts to every joining session *in preference to the worktree name*, so a note that cannot be corrected is announced as current intent indefinitely. + Every `-Release` — **`-Force` included** — appends one JSON line to `claims/.history` naming the key, + the releasing worktree and branch, the prior holder, its branch and note, and whether `-Force` was + used. The flag stays: a claim whose holder's worktree is gone would otherwise be stuck, and the + alternative people reach for is hand-deleting the file, which leaves less evidence still. What + changed is that the override is no longer invisible. The record is written *before* the claim file + is removed, and a release that cannot be recorded is refused rather than performed silently + (BACKLOG #1068). - **[`scripts/hooks/claim_check.py`](../scripts/hooks/claim_check.py)** — `commit-msg` gate: a commit whose *subject* declares `BACKLOG #N` with a code-touching diff must hold a claim on N **for this worktree**. Motivated by a recorded incident: three sessions independently fixed one npm advisory; two PRs were diff --git a/scripts/coord/claim.ps1 b/scripts/coord/claim.ps1 index 308a2172..59337734 100644 --- a/scripts/coord/claim.ps1 +++ b/scripts/coord/claim.ps1 @@ -29,6 +29,12 @@ original signal and it was actively misleading: a 21h claim whose holder had committed two minutes earlier was labelled STALE and recommended for release. + EVERY RELEASE IS RECORDED, `-Force` included, as one JSON line appended to + /mefor-coord/claims/.history: the key, the releasing worktree and branch, the + prior holder, its branch and note, when the claim was taken, and whether -Force was used. The + record is written BEFORE the claim file is removed and the release is refused if it cannot be + written -- a release nobody can trace is the outcome this will not produce (BACKLOG #1068). + .EXAMPLE pwsh -NoProfile -File scripts\coord\claim.ps1 -Take 105 -Note "corepoint xml importer" pwsh -NoProfile -File scripts\coord\claim.ps1 -List @@ -65,6 +71,24 @@ $common = (& git -C $repo rev-parse --path-format=absolute --git-common-dir).Tri $claims = Join-Path $common "mefor-coord/claims" New-Item -ItemType Directory -Force -Path $claims | Out-Null +# THE RELEASE LEDGER (BACKLOG #1068). A release used to be `Remove-Item` and nothing else, so a +# `-Force` takeover -- one session releasing a claim held by another -- left no record of who released +# whose claim, when, or why. `-Force` has a real job and stays: a claim whose holder's worktree is +# gone would otherwise be stuck forever, and hand-deleting the file leaves even less evidence. The +# answer is a RECORD, not a refusal. +# +# Measured 2026-08-10. A coordinator force-released a claim after establishing on evidence that the +# holder's worktree was gone and that the work its note guarded had already merged, while the note +# still read "UNPUSHED, NO PR, GitHub finds NOTHING". That release was CORRECT and it left nothing +# behind to find, which is the whole gap: the registry is a shared coordination artifact, and a stale +# note in it has already blocked a lane from claiming an item. +# +# JSON Lines, LF-terminated, one object per release. It sits INSIDE the claims directory, which is +# safe because every reader of that directory keys on a file NAME -- `-List` and prune-merged.ps1 glob +# `*.json`, scripts/hooks/claim_check.py opens `.json` -- and ConvertTo-KeyFile always appends +# ".json", so no key can ever fold onto this name. +$history = Join-Path $claims ".history" + # A key is free text but becomes a FILENAME, so fold it to a safe, case-insensitive form. The original is # kept inside the json so `-List` can show what the human actually typed. function ConvertTo-KeyFile([string]$Key) { @@ -90,6 +114,32 @@ function Get-Mine([string]$Path) { [pscustomobject]@{ Claim = $c; IsMine = ($held -ieq $me) } } +# ONE record, appended exclusively, retried. FileMode::Append + FileShare::Read excludes a second +# WRITER -- two worktrees can release in the same instant -- while leaving readers alone, and the whole +# line goes out in a SINGLE Write to a handle already positioned at end-of-file, so a concurrent +# release cannot interleave half a record into another's. +# +# LF, not the platform newline: this is a machine-readable ledger read from PowerShell, python and +# git-bash on the same clone, and a mixed-newline JSONL file is one of those things nothing complains +# about until a parser splits differently from the writer. +# +# The catch is deliberately UNTYPED, for the reason written out at the claim-refresh Move below: +# PowerShell wraps an exception thrown by a .NET METHOD in a MethodInvocationException, so a typed +# `catch [System.IO.IOException]` here would never match and the failure would escape to +# $ErrorActionPreference = "Stop" instead of being retried. +function Add-HistoryLine([string]$Line) { + $bytes = [System.Text.Encoding]::UTF8.GetBytes($Line + "`n") + foreach ($attempt in 1..5) { + try { + $fs = [System.IO.File]::Open($history, [System.IO.FileMode]::Append, [System.IO.FileAccess]::Write, [System.IO.FileShare]::Read) + try { $fs.Write($bytes, 0, $bytes.Length) } finally { $fs.Dispose() } + return $true + } + catch { Start-Sleep -Milliseconds (20 * $attempt) } + } + return $false +} + # ONE liveness rule, three call sites (BACKLOG #345 Half B). # # `-List` learned this first, and that was the wrong half to fix alone: -List is where you BROWSE, and @@ -165,6 +215,12 @@ function Show-List { Write-Host "" } +# Resolved for BOTH paths, not just -Take. A release record that names who released the claim is only +# half an answer without the branch they were standing on -- the same question -Take records. +$branch = & git -C $repo branch --show-current +if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git -C $repo rev-parse --short HEAD) } +$branch = $branch.Trim() + if ($Release) { $file = Join-Path $claims ((ConvertTo-KeyFile $Release) + ".json") if (-not (Test-Path $file)) { Write-Host "No claim on '$Release' -- nothing to release."; exit 0 } @@ -199,8 +255,58 @@ if ($Release) { } exit 1 } - Remove-Item -LiteralPath $file -Force + # RECORD FIRST, then act. Both orders can lie once and only one lie is recoverable: removing first + # and recording after reproduces this item exactly (a completed release with no trace, and nothing + # left to write the record from), whereas recording first can at worst claim a release that then + # failed -- which the catch below corrects in the same ledger. Refusing when the record cannot be + # written is safe because a release is always retryable: the claim stays where it was. + # + # ConvertTo-Stamp on every field carried over from the claim file, not just the timestamp: + # ConvertFrom-Json date-coerces ANY ISO-8601-shaped string, and note/branch/worktree are free text. + $record = [ordered]@{ + ts = (Get-Date).ToString("o") + event = "release" + key = $Release + released_by = $repo + released_branch = $branch + prior_holder = ConvertTo-Stamp $info.Claim.worktree + prior_branch = ConvertTo-Stamp $info.Claim.branch + # The note is what a later reader judges the release BY -- it is the field that was stale and + # wrong in the incident above -- so the record keeps it rather than pointing at a deleted file. + prior_note = ConvertTo-Stamp $info.Claim.note + claimed = ConvertTo-Stamp $info.Claim.claimed + # The flag AS PASSED. Whether this was a takeover is already readable from prior_holder against + # released_by, and a record should not restate a fact it already carries. + force = [bool]$Force + } | ConvertTo-Json -Compress + if (-not (Add-HistoryLine $record)) { + Write-Host "" + Write-Host "REFUSING to release '$Release': the release record could not be written." -ForegroundColor Yellow + Write-Host " ledger : $history" + Write-Host " Your claim is UNCHANGED and still yours. Retry in a moment -- an untraceable" + Write-Host " release is the one outcome this refuses to produce." + exit 1 + } + try { + Remove-Item -LiteralPath $file -Force + } + catch { + # The line above says a release happened; it did not. Correct it in the same ledger rather than + # leave a record that is now false. + Add-HistoryLine ([ordered]@{ + ts = (Get-Date).ToString("o") + event = "release-failed" + key = $Release + released_by = $repo + reason = $_.Exception.Message + } | ConvertTo-Json -Compress) | Out-Null + throw + } Write-Host "Released claim on '$Release'." -ForegroundColor Green + if (-not $info.IsMine) { + Write-Host " TOOK OVER a claim held by $($info.Claim.worktree) [$($info.Claim.branch)]." -ForegroundColor Yellow + } + Write-Host " recorded in $history" exit 0 } @@ -210,10 +316,6 @@ if ($List) { Show-List; exit 0 } $safe = ConvertTo-KeyFile $Take $file = Join-Path $claims "$safe.json" -$branch = & git -C $repo branch --show-current -if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git -C $repo rev-parse --short HEAD) } -$branch = $branch.Trim() - try { # ATOMIC test-and-set -- identical to alloc.ps1. 'CreateNew' + FileShare::None throws IOException if a # sibling session got here first, and that throw IS the mutual exclusion. diff --git a/tests/test_coord_claim_release_history.py b/tests/test_coord_claim_release_history.py new file mode 100644 index 00000000..0e41a498 --- /dev/null +++ b/tests/test_coord_claim_release_history.py @@ -0,0 +1,283 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Every release must leave a record -- including a ``-Force`` one (BACKLOG #1068). + +``claim.ps1 -Release`` was ``Remove-Item`` and nothing else. With ``-Force`` that meant any session +could release a claim it does not hold and leave no trace of who released whose claim, when, or why. +The escape hatch is NECESSARY -- a claim whose holder's worktree is gone would otherwise be stuck +forever, and the script prints that recipe itself -- so the fix is a RECORD, not a refusal. Every +test below therefore asserts a line was written; none asserts a release was blocked. + +**Measured 2026-08-10, which is why the record has to name the PRIOR holder.** A coordinator +force-released claim #344 after establishing on evidence that the holder's worktree was gone and that +the work the claim's note guarded had already merged, while that note still read "UNPUSHED, NO PR, +GitHub finds NOTHING". The release was correct and it left nothing behind to find. Nothing is +deployed and there is no user to mislead (CLAUDE.md 0), but the registry is a shared coordination +artifact today and a stale note in it has already blocked a lane from claiming an item. + +**The load-bearing negatives, per BACKLOG #1000.** A record that logged only "somebody released +something" satisfies "a line was appended" and answers none of the question, so: + +* ``test_a_force_takeover_names_the_PRIOR_holder_not_the_releaser`` -- the two paths must differ, and + the record must carry both. +* ``test_force_false_is_recorded_when_the_flag_was_not_passed`` -- a hardcoded ``force: true`` passes + the takeover test and fails this one. +* ``test_a_REFUSED_release_writes_nothing`` -- the record is written before the removal, so the case + that can produce a FALSE line (a release that never happened) is checked directly. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +CLAIM = ROOT / "scripts" / "coord" / "claim.ps1" +TIMEOUT = 60 + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None or os.name != "nt", + reason="claim.ps1 needs pwsh on Windows", +) + + +def git(repo: Path, *args: str) -> str: + proc = subprocess.run( + ["git", "-C", str(repo), *args], capture_output=True, text=True, timeout=TIMEOUT, check=True + ) + return proc.stdout + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + """A throwaway checkout carrying its OWN copy of claim.ps1. + + The copy is the sandbox: the script anchors on ``$PSScriptRoot`` (BACKLOG #1060), so it writes to + THIS repository's registry and never to the real one. Same fixture shape as + ``test_coord_claim_liveness.py`` for the same reason. + """ + r = tmp_path / "repo" + (r / "scripts" / "coord").mkdir(parents=True) + subprocess.run(["git", "init", "-q", "-b", "main", str(r)], check=True, capture_output=True) + git(r, "config", "user.email", "t@example.invalid") + git(r, "config", "user.name", "t") + shutil.copy2(CLAIM, r / "scripts" / "coord" / "claim.ps1") + (r / "f.txt").write_text("x", encoding="utf-8") + git(r, "add", "f.txt", "scripts/coord/claim.ps1") + git(r, "commit", "-qm", "base") + return r + + +def claim(cwd: Path, *args: str) -> subprocess.CompletedProcess[str]: + """Run the checkout's OWN copy, with the child's environment PINNED. + + ``env`` is passed explicitly rather than inherited. claim.ps1 reads nothing from the environment + today, so nothing here is load-bearing for the assertions -- but a test that shells out and + inherits whatever the developer's shell exports is measuring the shell as much as the code, which + is how a lane's green survived locally and reddened at integration in wave 2. Pinning costs one + line and removes the whole class. + """ + env = dict(os.environ) + for var in ("PYTHONIOENCODING", "PYTHONUTF8"): + env.pop(var, None) + return subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(cwd / "scripts" / "coord" / "claim.ps1"), + *args, + ], + cwd=str(cwd), + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=TIMEOUT, + check=False, + env=env, + ) + + +def history_path(repo: Path) -> Path: + return repo / ".git" / "mefor-coord" / "claims" / ".history" + + +def records(repo: Path) -> list[dict[str, object]]: + """Every record in the ledger, parsed. + + Asserts the on-disk shape as it goes: JSON Lines, LF-terminated, no CR. A ledger that is only + ever read back by the code that wrote it can drift into any shape at all; this is the file an + operator greps months later, so the format is part of the contract. + """ + p = history_path(repo) + if not p.exists(): + return [] + text = p.read_text(encoding="utf-8") + assert "\r" not in text, f"the ledger must be LF-only JSON Lines, not CRLF: {text!r}" + assert text.endswith("\n"), f"every record must be newline-terminated: {text!r}" + return [json.loads(line) for line in text.splitlines() if line.strip()] + + +def peer_holding(repo: Path, tmp_path: Path, key: str, note: str = "the peer's work") -> Path: + """A second worktree that holds ``key``, sharing this repo's object store and registry.""" + peer = tmp_path / "peer-wt" + git(repo, "worktree", "add", "-q", "-b", "peer-branch", str(peer)) + assert claim(peer, "-Take", key, "-Note", note).returncode == 0 + return peer + + +def norm(p: object) -> str: + return str(p).replace("\\", "/").rstrip("/").casefold() + + +# -------------------------------------------------------------------------------------------------- +# the ordinary release +# -------------------------------------------------------------------------------------------------- + + +def test_releasing_your_own_claim_appends_a_record(repo: Path) -> None: + assert claim(repo, "-Take", "k", "-Note", "my work").returncode == 0 + proc = claim(repo, "-Release", "k") + + assert proc.returncode == 0, proc.stdout + proc.stderr + rows = records(repo) + assert len(rows) == 1, rows + row = rows[0] + assert row["event"] == "release" + assert row["key"] == "k" + assert norm(row["released_by"]) == norm(repo) + assert norm(row["prior_holder"]) == norm(repo) + assert row["prior_note"] == "my work" + assert row["force"] is False + assert row["ts"], "a record with no timestamp cannot be ordered against anything" + assert row["claimed"], "the record must carry when the claim was taken, not only when it ended" + + +def test_the_release_output_names_the_ledger_it_wrote_to(repo: Path) -> None: + """A record nobody can find is barely better than no record. + + The path is not guessable -- it lives beside the SHARED object store, which for a linked worktree + is a different directory from the one the operator is standing in. + """ + assert claim(repo, "-Take", "k", "-Note", "x").returncode == 0 + out = claim(repo, "-Release", "k").stdout + assert ".history" in out, out + + +# -------------------------------------------------------------------------------------------------- +# -Force: the case the item is about +# -------------------------------------------------------------------------------------------------- + + +def test_a_force_takeover_is_recorded(repo: Path, tmp_path: Path) -> None: + peer = peer_holding(repo, tmp_path, "k", note="the peer is mid-flight") + proc = claim(repo, "-Release", "k", "-Force") + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert not (repo / ".git" / "mefor-coord" / "claims" / "k.json").exists(), ( + "-Force must still release -- this item is about auditability, not prevention" + ) + rows = records(repo) + assert len(rows) == 1, rows + assert rows[0]["force"] is True + assert norm(rows[0]["prior_holder"]) == norm(peer) + + +def test_a_force_takeover_names_the_PRIOR_holder_not_the_releaser( + repo: Path, tmp_path: Path +) -> None: + """THE load-bearing case. "who released whose claim" needs both halves, and they differ here. + + A record carrying only the releasing worktree passes every other test in this file and answers + none of the question #1068 asks. + """ + peer = peer_holding(repo, tmp_path, "k", note="UNPUSHED, NO PR") + assert claim(repo, "-Release", "k", "-Force").returncode == 0 + + row = records(repo)[0] + assert norm(row["prior_holder"]) == norm(peer) + assert norm(row["released_by"]) == norm(repo) + assert norm(row["prior_holder"]) != norm(row["released_by"]), ( + "the fixture must make the two paths differ, or this test cannot fail" + ) + # The note is what a reader needs to judge whether the release was right -- it is the field that + # was stale and wrong in the 2026-08-10 incident. + assert row["prior_note"] == "UNPUSHED, NO PR" + assert row["prior_branch"] == "peer-branch" + + +def test_force_false_is_recorded_when_the_flag_was_not_passed(repo: Path) -> None: + """The negative that a hardcoded ``force: true`` cannot pass.""" + assert claim(repo, "-Take", "k", "-Note", "x").returncode == 0 + assert claim(repo, "-Release", "k").returncode == 0 + assert records(repo)[0]["force"] is False + + +def test_a_force_takeover_says_so_on_stdout_too(repo: Path, tmp_path: Path) -> None: + """The releasing session should be told it took someone else's key, not just that it succeeded.""" + peer_holding(repo, tmp_path, "k") + out = claim(repo, "-Release", "k", "-Force").stdout + assert "TOOK OVER" in out, out + assert "peer-branch" in out, out + + +# -------------------------------------------------------------------------------------------------- +# what must NOT be recorded +# -------------------------------------------------------------------------------------------------- + + +def test_a_REFUSED_release_writes_nothing(repo: Path, tmp_path: Path) -> None: + """The record is written BEFORE the removal, so a false line is the failure mode to check. + + A refused release did not happen. A ledger claiming it did is worse than the silence it replaced. + """ + peer_holding(repo, tmp_path, "k") + proc = claim(repo, "-Release", "k") # no -Force: refused + + assert proc.returncode == 1 + assert records(repo) == [], "a refusal is not a release and must not be recorded as one" + + +def test_releasing_an_unclaimed_key_writes_nothing(repo: Path) -> None: + proc = claim(repo, "-Release", "never-claimed") + assert proc.returncode == 0 + assert records(repo) == [] + + +# -------------------------------------------------------------------------------------------------- +# the ledger's own properties +# -------------------------------------------------------------------------------------------------- + + +def test_records_APPEND_rather_than_replace(repo: Path, tmp_path: Path) -> None: + """A ledger that keeps only the last release is a status field, not a history.""" + assert claim(repo, "-Take", "a", "-Note", "first").returncode == 0 + assert claim(repo, "-Release", "a").returncode == 0 + peer_holding(repo, tmp_path, "b", note="second") + assert claim(repo, "-Release", "b", "-Force").returncode == 0 + + rows = records(repo) + assert [r["key"] for r in rows] == ["a", "b"], rows + assert [r["force"] for r in rows] == [False, True], rows + + +def test_the_ledger_is_not_mistaken_for_a_claim(repo: Path) -> None: + """It lives INSIDE the claims directory, so the readers of that directory must ignore it. + + ``-List`` globs ``*.json`` and scripts/hooks/claim_check.py opens ``.json``; a dotfile named + ``.history`` is invisible to both. Asserted rather than argued, because "the glob will not match + it" is exactly the kind of premise that is true until someone widens the glob. + """ + assert claim(repo, "-Take", "k", "-Note", "x").returncode == 0 + assert claim(repo, "-Release", "k").returncode == 0 + assert history_path(repo).is_file(), "the ledger must exist for this test to mean anything" + + out = claim(repo, "-List").stdout + assert "No active claims." in out, out From d60e71e2c64035a8f585d166c03ce7f116cc94e2 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Mon, 10 Aug 2026 17:44:22 -0500 Subject: [PATCH 5/6] fix(dev): setup-leak-gate names the token source it actually loaded (BACKLOG #1080) `-Synthetic` printed "Installed the SYNTHETIC template" and, three lines later, "CONFIGURED with the real token set." Both were true whenever MEFOR_FORBIDDEN_TOKENS was set -- the script wrote the file, and the scanner loaded the environment's list, which wins over it -- but nothing said the environment had OVERRIDDEN what had just been written. Reproduced on a throwaway checkout before the change, in that exact shape. The verify step now prints a `token source:` line naming what the scanner actually loaded (MEFOR_FORBIDDEN_TOKENS -> , that variable carrying the list inline, or scripts/security/scan-tokens.local.txt) and an OVERRIDDEN banner when an install ran and the environment won. An INLINE value is named but never PRINTED: a non-path value IS the token list, and echoing it would publish what the gate protects into whatever log was being captured. An explicitly EMPTY MEFOR_FORBIDDEN_TOKENS is named as the CAUSE of NOT CONFIGURED, because that state does not fall back to the file and the ordinary advice loops forever. A second reporting defect in the same file goes with it: the scanner's exit code was discarded, so a refusal came back out as CONFIGURED and exit 0 -- contradicting this file's own header promise to "exit non-zero if the sections are empty". Measured with an impossible MEFOR_MIN_DETECTORS floor: scanner exit 2, script exit 0. It now exits with the scanner's code under VERIFY FAILED, and deliberately does not reprint the scanner's output, because a hit line can quote matched content. scan_forbidden.py is unchanged. Precedence is defined by its _resolve_token_text and this script necessarily re-expresses it, so tests/test_setup_leak_gate_reports_source.py measures every branch against the scanner's own detector counts in the SAME run and asserts the two sources yield DIFFERENT counts -- a fixture that could not tell them apart would pass while measuring nothing. Red first, 7 of 8; the one green was the paired positive control. Its child environment is pinned explicitly (MEFOR_* and PYTHONIOENCODING/PYTHONUTF8 removed, python resolved through a shim), so the result depends on the code rather than the shell. --- CONTRIBUTING.md | 8 + scripts/dev/setup-leak-gate.ps1 | 83 ++++- tests/test_setup_leak_gate_reports_source.py | 335 +++++++++++++++++++ 3 files changed, 422 insertions(+), 4 deletions(-) create mode 100644 tests/test_setup_leak_gate_reports_source.py diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7b750545..3af78f0a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -106,6 +106,14 @@ Only the first is a real local scan. Note that the synthetic template is **below floor** (`names=7, estate=13, site_prefixes=1`) by design — passing locally with it does not mean you would pass CI's gate, only that nothing structural was found. +It also prints a `token source:` line naming **where those counts came from** — the +`MEFOR_FORBIDDEN_TOKENS` path, that variable carrying the list inline (named, never printed), or +`scripts/security/scan-tokens.local.txt` — and says `OVERRIDDEN` when the environment won over the +file the run just installed. That variable takes precedence, so without the line `-Synthetic` could +install the template and then truthfully report the *real* set as configured, which reads as a +contradiction. The scanner's exit code is propagated too: a refusal is reported as `VERIFY FAILED`, +not as `CONFIGURED`. + ## Finding something to work on Browse issues labeled **`good first issue`** (small, self-contained) and **`help wanted`**. For diff --git a/scripts/dev/setup-leak-gate.ps1 b/scripts/dev/setup-leak-gate.ps1 index 9747f843..879624cd 100644 --- a/scripts/dev/setup-leak-gate.ps1 +++ b/scripts/dev/setup-leak-gate.ps1 @@ -22,6 +22,15 @@ never ran, or ran with nothing loaded. So this script always finishes by invoking the scanner and printing the per-section detector counts, and exits non-zero if the sections are empty. + IT ALSO NAMES THE SOURCE THOSE COUNTS CAME FROM, and says so when the environment overrode the + file just written (BACKLOG #1080). MEFOR_FORBIDDEN_TOKENS wins over + scripts/security/scan-tokens.local.txt, so -Synthetic could install the template and then report + the REAL set as configured -- two true lines that together read as a contradiction. Counts without + a provenance leave exactly the ambiguity this step exists to close. Two consequences worth knowing: + an inline (non-path) MEFOR_FORBIDDEN_TOKENS value is named but never PRINTED, because that value + is the token list; and the scanner's exit code is now propagated, so a refusal is reported as + VERIFY FAILED instead of coming back out as CONFIGURED. + .EXAMPLE pwsh -NoProfile -File scripts/dev/setup-leak-gate.ps1 -From C:\path\to\tokens.txt .EXAMPLE @@ -83,27 +92,93 @@ if (Test-Path -LiteralPath $local) { } } +# --- which source will the scanner ACTUALLY load from? --------------------------------------------- +# The verify step below reports the LOADED token set. Reporting only that is what let -Synthetic print +# "Installed the SYNTHETIC template" and then, three lines later, "CONFIGURED with the real token set" +# (BACKLOG #1080). Both were true -- the file was installed, and MEFOR_FORBIDDEN_TOKENS won over it -- +# but nothing said the environment had OVERRIDDEN what had just been written, so the pair read as a +# contradiction or, worse, as confirmation that a synthetic install had produced a real-token gate. +# +# PRECEDENCE IS DEFINED BY scan_forbidden._resolve_token_text, NOT HERE. This is a second expression of +# it and would drift silently, so tests/test_setup_leak_gate_reports_source.py measures every branch +# below against the scanner's own detector counts in the SAME run rather than trusting either alone. +$envTokens = [Environment]::GetEnvironmentVariable('MEFOR_FORBIDDEN_TOKENS') +$envEmpty = $false +if ($null -ne $envTokens) { + $trimmed = $envTokens.Trim() + if (-not $trimmed) { + # An explicitly-EMPTY value means "no source" and does NOT fall back to the file. Worth naming, + # because in this one state installing a token list changes nothing and the ordinary advice + # ("-From " / "-Synthetic") sends the operator round the same loop indefinitely. + $envEmpty = $true + $source = 'NONE -- MEFOR_FORBIDDEN_TOKENS is set but EMPTY, which the scanner reads as "no source" (it does NOT fall back to the file)' + } + else { + $isFile = $false + try { $isFile = Test-Path -LiteralPath $trimmed -PathType Leaf } catch { $isFile = $false } + if ($isFile) { $source = "MEFOR_FORBIDDEN_TOKENS -> $trimmed" } + # NEVER echo the value. A non-path value IS the token list, carried inline; naming the variable + # is the whole diagnostic, and printing its content would publish exactly what this protects + # into whatever log the operator happened to be capturing. + else { $source = 'MEFOR_FORBIDDEN_TOKENS (inline token content -- value deliberately not printed)' } + } + $fromFile = $false +} +elseif (Test-Path -LiteralPath $local) { + $source = 'scripts/security/scan-tokens.local.txt' + $fromFile = $true +} +else { + $source = 'NONE -- no MEFOR_FORBIDDEN_TOKENS, and scripts/security/scan-tokens.local.txt is absent' + $fromFile = $false +} + # --- verify: what can the gate actually SEE? ------------------------------------------------------- $py = if (Test-Path (Join-Path $repo '.venv/Scripts/python.exe')) { Join-Path $repo '.venv/Scripts/python.exe' } else { 'python' } Write-Host '' Write-Host 'Verifying the gate can see:' -ForegroundColor Cyan $out = & $py $scanner --require-tokens 2>&1 +# Captured on the very next line, before any other native call can overwrite it. The scanner's verdict +# used to be discarded entirely, so a refusal came back out of here as CONFIGURED and exit 0 -- which +# contradicts this file's own header promise to "exit non-zero if the sections are empty". +$scanExit = $LASTEXITCODE $loaded = $out | Where-Object { $_ -match 'loaded names=' } | Select-Object -First 1 if (-not $loaded) { Write-Host ($out | Select-Object -Last 5); throw 'Scanner produced no detector-count line.' } Write-Host " $loaded" +Write-Host " token source: $source" +if ($PSCmdlet.ParameterSetName -ne 'Status' -and -not $fromFile) { + Write-Host ' OVERRIDDEN: the file this run just installed is NOT what the gate loaded -- MEFOR_FORBIDDEN_TOKENS takes precedence over scripts/security/scan-tokens.local.txt.' -ForegroundColor Yellow + Write-Host ' Unset that variable to use the file you just installed.' -ForegroundColor Yellow +} if ($loaded -match 'STRUCTURAL-ONLY') { Write-Host '' - Write-Host 'NOT CONFIGURED — the pre-commit hook will fail closed on every commit.' -ForegroundColor Red - Write-Host ' Maintainers: -From Contributors: -Synthetic' + Write-Host 'NOT CONFIGURED -- the pre-commit hook will fail closed on every commit.' -ForegroundColor Red + if ($envEmpty) { + Write-Host ' CAUSE: MEFOR_FORBIDDEN_TOKENS is set to an EMPTY value, which the scanner reads as "no source".' -ForegroundColor Red + Write-Host ' Installing a token list will NOT fix this -- unset that variable first.' -ForegroundColor Red + } + else { + Write-Host ' Maintainers: -From Contributors: -Synthetic' + } exit 1 } +if ($scanExit -ne 0) { + Write-Host '' + Write-Host "VERIFY FAILED -- a token source is loaded but the scanner EXITED $scanExit." -ForegroundColor Red + Write-Host " token source: $source" + # Its output is NOT reprinted here: a hit line can quote matched content, and copying that into a + # terminal scrollback, a ticket or a CI log is the disclosure this whole gate exists to prevent. + Write-Host ' Re-run the scanner yourself to see why, and do not paste its output anywhere:' -ForegroundColor Red + Write-Host " $py scripts/security/scan_forbidden.py --require-tokens" + exit $scanExit +} if ($loaded -match 'SYNTHETIC') { Write-Host '' - Write-Host 'CONFIGURED (synthetic). Real customer tokens are NOT detected locally.' -ForegroundColor Yellow + Write-Host "CONFIGURED (synthetic), loaded from: $source. Real customer tokens are NOT detected locally." -ForegroundColor Yellow Write-Host ' It also flags the fictional customer/partner names this project uses in its own docs, so a hit here is not by itself a leak. CI is authoritative.' -ForegroundColor Yellow exit 0 } Write-Host '' -Write-Host 'CONFIGURED with the real token set.' -ForegroundColor Green +Write-Host "CONFIGURED with the real token set, loaded from: $source." -ForegroundColor Green exit 0 diff --git a/tests/test_setup_leak_gate_reports_source.py b/tests/test_setup_leak_gate_reports_source.py new file mode 100644 index 00000000..1a0a3745 --- /dev/null +++ b/tests/test_setup_leak_gate_reports_source.py @@ -0,0 +1,335 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The verify step must name the source it LOADED, not the file it installed (BACKLOG #1080). + +``setup-leak-gate.ps1 -Synthetic`` printed *"Installed the SYNTHETIC template"* and, three lines +later, *"CONFIGURED with the real token set."* Both were individually true whenever +``MEFOR_FORBIDDEN_TOKENS`` was set -- the script wrote the synthetic file, and the scanner loaded the +real list from the environment, which wins over the file -- but nothing said the environment had +OVERRIDDEN what had just been written. The pair reads as a contradiction or, worse, as confirmation +that a synthetic install produced a real-token gate. + +The script's own header is what makes that a defect rather than a nit: *"A green gate is evidence +only if you confirmed it can see the class it is meant to catch"*. Printing the detector counts +without printing WHERE they came from leaves exactly the ambiguity the step exists to close. + +**These tests pin a duplicated rule against its definition.** Precedence lives in +``scan_forbidden._resolve_token_text``; the script necessarily re-expresses it in order to name the +source, and a second expression drifts silently. So every case here asserts the script's claim about +the source AND the scanner's own ``loaded names=`` line from the SAME run, and the two-source case +asserts those lines DIFFER -- a fixture whose two sources were indistinguishable would pass while +measuring nothing. + +**The token fixtures are DERIVED from the shipped example, never hand-written.** Two reasons, both +load-bearing. Every floor section must be non-empty or the scanner refuses (see +``token_floor_failure``), and one of those sections is a numeric site prefix -- a value this repo's +own gate scans tracked files for. Deriving keeps the numeric prefix out of this file entirely and +keeps the fixture valid if the example's sections are ever renamed. + +**Assertions are ASCII-only, deliberately.** The output crosses two encoding boundaries (python's +stderr into pwsh, pwsh's stdout into pytest) and the em dash in the scanner's mode line arrives +mojibake on a stock Windows code page. Asserting on it would be testing the code pages. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] +_SETUP = _ROOT / "scripts" / "dev" / "setup-leak-gate.ps1" +_SECURITY = _ROOT / "scripts" / "security" +_EXAMPLE = _SECURITY / "scan-tokens.local.txt.example" +_TIMEOUT = 55 # under pyproject's per-test --timeout=60 + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None, reason="setup-leak-gate.ps1 needs pwsh (PowerShell 7)" +) + +#: Not echoed anywhere by a correct implementation -- see the inline-content test. +_INLINE_MARKER = "ZzInlineOnlyMarkerZz" +_EXTERNAL_MARKER = "ZzExternalCorpZz" + + +def _derived_tokens(marker: str) -> str: + """The shipped synthetic example plus one extra name detector. + + Real-SHAPED without being the example: ``is_synthetic_token_set()`` compares the parsed name + patterns, so one extra entry is enough for the scanner to stop labelling it SYNTHETIC, while the + detector counts move by exactly one -- which is what makes "which source loaded" observable. + """ + return ( + _EXAMPLE.read_text(encoding="utf-8") + + f"\n[names]\n\\b{marker}\\b | synthetic external token | i\n" + ) + + +def _git(*args: str, cwd: Path) -> None: + subprocess.run(["git", *args], cwd=str(cwd), check=True, capture_output=True) + + +def _checkout(path: Path) -> Path: + """A minimal checkout carrying the script and the three files it reaches for. + + Only those files are copied, NOT the whole of scripts/security: a maintainer running this suite + has the real ``scan-tokens.local.txt`` sitting in that directory, and a copytree would sweep the + private token list into a temp dir -- the one mistake the script itself refuses to make. + """ + (path / "scripts" / "dev").mkdir(parents=True) + (path / "scripts" / "security").mkdir(parents=True) + shutil.copy2(_SETUP, path / "scripts" / "dev" / "setup-leak-gate.ps1") + for name in ("scan_forbidden.py", "scan-allowlist.txt", "scan-tokens.local.txt.example"): + shutil.copy2(_SECURITY / name, path / "scripts" / "security" / name) + # check-ignore reads the working-tree .gitignore, so no commit is needed -- but the repo must + # exist, or the script's own not-git-ignored guard deletes what it wrote and throws. + (path / ".gitignore").write_text("scripts/security/scan-tokens.local.txt\n", encoding="utf-8") + _git("init", "-b", "main", ".", cwd=path) + return path + + +def _python_shim(path: Path) -> Path: + """A directory holding a ``python`` that forwards to THIS interpreter. + + The script resolves ``/.venv/Scripts/python.exe`` or falls back to bare ``python`` on PATH. + A temp checkout has no venv, so the verify step would otherwise depend on whichever ``python`` the + runner happens to expose -- which is why the neighbouring anchoring test declines to assert the + verify step at all. The shim makes the interpreter an INPUT of the test rather than an accident of + the leg, so the emitted strings and the exit code are assertable wherever pwsh runs. + """ + path.mkdir(parents=True, exist_ok=True) + if os.name == "nt": + (path / "python.cmd").write_text( + f'@echo off\r\n"{sys.executable}" %*\r\nexit /b %ERRORLEVEL%\r\n', encoding="ascii" + ) + else: + shim = path / "python" + shim.write_text(f'#!/bin/sh\nexec "{sys.executable}" "$@"\n', encoding="ascii") + shim.chmod(0o755) + return path + + +def _env(shim: Path, **over: str) -> dict[str, str]: + """The child's environment, PINNED -- never merely inherited. + + Every variable that steers the scanner is removed first, then set explicitly by the caller. A + maintainer with ``MEFOR_FORBIDDEN_TOKENS`` exported would otherwise run these tests against their + own real list and get greens that mean something else entirely. ``PYTHONIOENCODING`` / ``PYTHONUTF8`` + go too: they change the grandchild's stderr encoding, so leaving them ambient makes the result + depend on the developer's shell rather than on the code -- measured in this repo, on a test that + passed locally and reddened at integration for exactly that. + """ + env = dict(os.environ) + for var in ( + "MEFOR_FORBIDDEN_TOKENS", + "MEFOR_REQUIRE_TOKENS", + "MEFOR_MIN_DETECTORS", + "PYTHONIOENCODING", + "PYTHONUTF8", + ): + env.pop(var, None) + env["PATH"] = str(shim) + os.pathsep + env.get("PATH", "") + env.update(over) + return env + + +def _run(root: Path, *args: str, env: dict[str, str]) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(root / "scripts" / "dev" / "setup-leak-gate.ps1"), + *args, + ], + cwd=str(root), + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=_TIMEOUT, + env=env, + ) + + +def _loaded(proc: subprocess.CompletedProcess[str]) -> str: + """The scanner's own detector-count line, as echoed by the script.""" + line = next((ln for ln in proc.stdout.splitlines() if "loaded names=" in ln), "") + assert line, f"no detector-count line in the output:\n{proc.stdout}\n{proc.stderr}" + return line.strip() + + +def _source_line(proc: subprocess.CompletedProcess[str]) -> str: + """The line that names the RESOLVED source, isolated from the rest of the run. + + Extracted rather than searched for as a substring of the whole output, and that is not fussiness. + ``-Synthetic`` already prints ``Installed the SYNTHETIC template -> scripts/security/ + scan-tokens.local.txt``, so a whole-output search for that path passes with the defect fully + present -- measured: the first draft of the file-source test below was green before the fix, for + exactly that reason. The claim under test is about the line that says what was LOADED. + """ + line = next((ln for ln in proc.stdout.splitlines() if "token source" in ln.lower()), "") + assert line, f"no line naming the resolved token source:\n{proc.stdout}\n{proc.stderr}" + return line.strip() + + +@pytest.fixture +def rig(tmp_path: Path) -> tuple[Path, Path, Path]: + """``(checkout, shim dir, an external token file)``.""" + root = _checkout(tmp_path / "repo") + shim = _python_shim(tmp_path / "shim") + ext = tmp_path / "external-tokens.txt" + ext.write_text(_derived_tokens(_EXTERNAL_MARKER), encoding="utf-8") + return root, shim, ext + + +# -------------------------------------------------------------------------------------------------- +# the source is named -- and the negative control that it is not named unconditionally +# -------------------------------------------------------------------------------------------------- + + +def test_the_installed_file_is_named_as_the_source_when_nothing_overrides_it( + rig: tuple[Path, Path, Path], +) -> None: + root, shim, _ = rig + proc = _run(root, "-Synthetic", env=_env(shim)) + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "scripts/security/scan-tokens.local.txt" in _source_line(proc) + assert "MEFOR_FORBIDDEN_TOKENS" not in _source_line(proc) + assert "SYNTHETIC" in _loaded(proc), "the scanner did not load the file that was just installed" + # THE NEGATIVE CONTROL. An override banner printed unconditionally would satisfy the override test + # below while telling the operator something false here. + assert "OVERRIDDEN" not in proc.stdout, proc.stdout + + +def test_an_env_file_override_is_named_and_announced(rig: tuple[Path, Path, Path]) -> None: + """The exact shape #1080 was filed for: install the synthetic file, load the environment's list.""" + root, shim, ext = rig + proc = _run(root, "-Synthetic", env=_env(shim, MEFOR_FORBIDDEN_TOKENS=str(ext))) + + assert proc.returncode == 0, proc.stdout + proc.stderr + # The two lines that used to read as a contradiction are both still there ... + assert "Installed the SYNTHETIC template" in proc.stdout + assert "CONFIGURED with the real token set" in proc.stdout + # ... and are now reconciled, by name. + assert "OVERRIDDEN" in proc.stdout, proc.stdout + assert "MEFOR_FORBIDDEN_TOKENS" in _source_line(proc) + assert str(ext) in _source_line(proc), ( + "the resolved path must be named, not merely the variable" + ) + + +def test_the_source_the_script_names_is_the_source_the_scanner_USED( + rig: tuple[Path, Path, Path], +) -> None: + """Pins the script's copy of the precedence rule against the scanner's actual behaviour. + + The script re-expresses ``scan_forbidden._resolve_token_text``; nothing but this stops the two + drifting. The detector-count lines must DIFFER between the two runs -- if the fixture's two token + sets were indistinguishable, every other assertion in this file would pass while measuring + nothing. + """ + root, shim, ext = rig + from_file = _run(root, "-Synthetic", env=_env(shim)) + from_env = _run(root, "-Synthetic", env=_env(shim, MEFOR_FORBIDDEN_TOKENS=str(ext))) + + assert _loaded(from_file) != _loaded(from_env), ( + f"the fixture cannot distinguish the two sources: {_loaded(from_file)!r}" + ) + assert "scripts/security/scan-tokens.local.txt" in _source_line(from_file) + assert str(ext) in _source_line(from_env) + # Direction, not merely difference: the env run must not be reported as the file it just wrote. + assert "SYNTHETIC" in _loaded(from_file) + assert "SYNTHETIC" not in _loaded(from_env) + + +def test_INLINE_token_content_is_named_but_never_echoed(rig: tuple[Path, Path, Path]) -> None: + """A non-path value IS the token list. Naming the variable is the diagnostic; printing it is a leak. + + The load-bearing negative for the whole change: the obvious way to "name the resolved source" is + to print the variable's value, which passes every other test here and publishes the private list + into whatever log the operator was capturing. + """ + root, shim, _ = rig + tokens = _derived_tokens(_INLINE_MARKER) + proc = _run(root, "-Synthetic", env=_env(shim, MEFOR_FORBIDDEN_TOKENS=tokens)) + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "OVERRIDDEN" in proc.stdout + assert "MEFOR_FORBIDDEN_TOKENS" in _source_line(proc) + assert "inline" in _source_line(proc).lower(), _source_line(proc) + assert _INLINE_MARKER not in proc.stdout, "the inline token content was echoed to stdout" + assert _INLINE_MARKER not in proc.stderr, "the inline token content was echoed to stderr" + + +def test_an_EMPTY_env_value_is_reported_as_the_cause_rather_than_a_missing_file( + rig: tuple[Path, Path, Path], +) -> None: + """The state where the ordinary advice is WRONG advice. + + An explicitly-empty ``MEFOR_FORBIDDEN_TOKENS`` means "no source" and does NOT fall back to the + file, so the script can install a token list, correctly report NOT CONFIGURED, and send the + operator round the same loop forever. Naming the cause is the difference between a diagnosis and a + restatement of the symptom. + """ + root, shim, _ = rig + proc = _run(root, "-Synthetic", env=_env(shim, MEFOR_FORBIDDEN_TOKENS="")) + + assert proc.returncode != 0, proc.stdout + assert "STRUCTURAL-ONLY" in _loaded(proc), "the scanner loaded a source after all" + assert "NOT CONFIGURED" in proc.stdout + assert "MEFOR_FORBIDDEN_TOKENS" in proc.stdout, proc.stdout + assert "EMPTY" in proc.stdout, proc.stdout + + +def test_status_mode_names_the_source_without_claiming_an_override( + rig: tuple[Path, Path, Path], +) -> None: + """No switch installs nothing, so nothing can have been overridden -- but the operator still needs + to know which source armed the gate, which is the one question the verify step exists to answer.""" + root, shim, _ = rig + assert _run(root, "-Synthetic", env=_env(shim)).returncode == 0 # arm it first + proc = _run(root, env=_env(shim)) + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "Installed" not in proc.stdout, "status mode must not install anything" + assert "scripts/security/scan-tokens.local.txt" in _source_line(proc) + assert "OVERRIDDEN" not in proc.stdout + + +# -------------------------------------------------------------------------------------------------- +# the verdict must follow the scanner, not the happy path +# -------------------------------------------------------------------------------------------------- + + +def test_a_nonzero_scanner_exit_is_not_reported_as_CONFIGURED( + rig: tuple[Path, Path, Path], +) -> None: + """The script's header promises it "exits non-zero if the sections are empty"; it discarded the + scanner's exit code entirely, so a refusal came back out as CONFIGURED and exit 0. + + An impossible ``MEFOR_MIN_DETECTORS`` floor is the cheapest way to make the scanner refuse with a + source genuinely loaded -- the state the old code could not tell apart from success. + """ + root, shim, _ = rig + proc = _run(root, "-Synthetic", env=_env(shim, MEFOR_MIN_DETECTORS="99999")) + + assert proc.returncode != 0, ( + f"the scanner refused and the script reported success:\n{proc.stdout}" + ) + assert "VERIFY FAILED" in proc.stdout, proc.stdout + assert "CONFIGURED (synthetic)" not in proc.stdout, proc.stdout + + +def test_a_healthy_run_still_exits_zero(rig: tuple[Path, Path, Path]) -> None: + """The paired positive. Propagating an exit code is only a fix if the ordinary path stays green.""" + root, shim, _ = rig + proc = _run(root, "-Synthetic", env=_env(shim)) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "VERIFY FAILED" not in proc.stdout From 813a595f2d30a88bd17cc6479c31c3f0ac03e009 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Mon, 10 Aug 2026 18:12:56 -0500 Subject: [PATCH 6/6] backlog: flip the four banners wave 3a landed Coordinator integration commit for the ungated half of wave 3, and the single point where the "a PR that implements BACKLOG #N must update BACKLOG.md" context is satisfied for the train. Banner text is each lane's own, carried verbatim. CLOSED: 1000 1208 1068 1080 Both lanes are TEST-ONLY or script-only; neither changes engine behaviour. #1000 registers a negative control per required context and a gate that fails when one is missing -- inside the ALREADY-REQUIRED test legs, so it blocks today and adds NO new required context. Adding one is an owner decision and was deliberately not taken. .github/required-contexts.txt was treated as read-only, per its own header: branch protection changes first. #1208 follows the VALUE across the factory rename boundary with a sentinel, rather than adding the fourth name list the item forbids. #1068 records every claim release, -Force included, in claims/.history. Its motivating instance is real: a force-release earlier today was correct on evidence and left no trace. #1080 makes setup-leak-gate name the token source it actually loaded. ONE FINDING FROM THIS WAVE IS WORTH MORE THAN THE FOUR ITEMS. W3-L5 caught a defect in its OWN control before trusting it: the backlog-hygiene negative control resolved bash from PATH, so its green was a fact about PATH order rather than about the gate. That is the third instance tonight of one class -- a green that is really a statement about the environment -- after the WSL-bash baseline and the PYTHONIOENCODING child-encoding defect. A control that has never been red is a claim; a control that is green for an environmental reason is worse, because it looks like evidence. Ledger, re-derived with parse_items: live 242, open 188, closed-in-live 54, archive 236, namespace 478 conserved. All three ledger gates pass. --- docs/BACKLOG.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index b180a1eb..3b1f886d 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -3054,7 +3054,7 @@ The distinction matters because these two paths do not look like the case AV cov ## 1000. Prove each required merge context can fail: negative controls for the gates that block merge -> 🔢 **Filed 2026-08-03 — not started.** Value **7/10** · Difficulty **3/10** · _quick win_. Thirteen contexts are the entire merge gate and not one of them is proven able to go red — a gate nobody has watched fail is an assumption wearing a green tick, and the class has now fired at least four times in this repo with no CI signal; the build is a negative-control fixture per context plus a job that fails if a context has none, no new dependency and no change to what the gates check. +> ✅ **SHIPPED (test-only; no branch-protection change, no new workflow).** All 13 required contexts now carry a registered negative control - the count READ from `.github/required-contexts.txt` at run time, never from memory. `tests/negative_controls.toml` records per context what the control plants, the pytest nodes that must go red without it, the shapes it deliberately does NOT break, and the nodes for that half; `tests/_negative_controls.py` + `tests/test_negative_controls.py` reconcile it against the LIVE required set and fail when a context has no control, when a control names a non-required context, when a node id resolves to no test, when the asymmetry half is empty, or when a `ci` control names a command no step invokes. The reconciliation runs inside the already-required `test` legs rather than in a new workflow, so it blocks today and adds NO new required context; two rows in `tests/test_ci_docs_only_detector.py` pin that the required-contexts file and the registry classify as CODE, because pytest is gated on `code == 'true'` and a docs-only classification would skip the gate on exactly the PR shape it exists for. `tests/test_merge_gate_controls.py` supplies the controls the contexts lacked, including the item's motivating instance RUN AGAINST THE PRE-FIX GATE: backlog-hygiene's own shell, lifted from the workflow, exits 1 through the shipped three-dot diff and 0 through the two-dot form on the identical synthetic PR. Watched fail, each restored in the same run: deleting `cancelled` from ci.yml's roll-up reddened 2 of 4 and left 2 green; widening one `.gitleaks.toml` allowlist entry reddened 2 of 4; adding `-ll` to the real bandit invocation reddened 1 of 4. **Residual, stated rather than closed:** the scanner binaries' own detection (bandit, gitleaks, npm-audit, pip-audit's audit half) runs in their own CI jobs, not in the pytest legs - what the legs hold is the property those jobs lose silently. Whether to add an ADVISORY workflow re-running the registered controls is an owner decision, deliberately not taken. **Cluster:** Security / CI gates. **Priority:** P1. **Verdict:** build. **Severity:** medium. @@ -4924,7 +4924,7 @@ The second step's arithmetic is measured: `GetFullPath('.git', )` r ## 1068. A `-Force` claim release leaves no record of who released whose claim -> 🔢 **Filed 2026-08-06 — not started.** Value **5/10** · Difficulty **2/10** · _quick win_. `scripts/coord/claim.ps1:164` gates a non-holder release behind `-not $Force`, so `-Force` lets any session release a claim it does not hold. That escape hatch is **necessary and must stay** — a claim whose holder's worktree is gone would otherwise be permanently stuck. The defect is that the release is `Remove-Item -LiteralPath $file -Force` and **nothing else**: no log, no audit line, no record of who released whose claim, when, or why. The guardrails are on the *advice*, not the *action*. +> ✅ **Shipped 2026-08-10.** Every `claim.ps1 -Release` now appends one JSON line to `/mefor-coord/claims/.history` — the key, the releasing worktree and branch, the prior holder, its branch and its note, when the claim was taken, and `force` true/false. **`-Force` is KEPT**: a claim whose holder worktree is gone would otherwise be stuck forever, and the alternative people reach for is hand-deleting the file, which leaves less evidence still. The item was auditability, not prevention. **And it is not hypothetical** — on 2026-08-10 a coordinator force-released claim #344 after establishing on evidence that the holder worktree was gone and that the work its note guarded had in fact merged as PR #153, while the note still read *"UNPUSHED, NO PR, GitHub finds NOTHING"*. That release was **correct**, and it left no trace, which is exactly the gap. Nothing is deployed, so on a first deployment there would be nobody to mislead (§0) — but the claim registry is a shared coordination artifact **today**, and a stale note in it has already blocked a lane from claiming an item. **The record is written BEFORE the claim file is removed, and the release is refused if it cannot be written.** Both orders can lie once and only one lie is recoverable: recording after the removal reproduces this defect exactly (a completed release with nothing left to write the record from), whereas recording first can at worst claim a release that then failed — and a failed removal appends a `release-failed` correction rather than leaving the first line false. Refusing costs nothing because a release is always retryable; the claim simply stays where it was. JSON Lines, LF-only, one line per release written in a SINGLE write to a handle opened `FileMode::Append` + `FileShare::Read`, so two worktrees releasing in the same instant cannot interleave a record. Safe inside the claims directory because every reader there keys on a file NAME (`-List` and `prune-merged.ps1` glob `*.json`, `claim_check.py` opens `.json`) and `ConvertTo-KeyFile` always appends `.json`, so no key can fold onto `.history`. `tests/test_coord_claim_release_history.py` (10 tests) was **red first, 8 of 10** against the unchanged script; the two that passed pre-fix are the guards against a WRONG fix — a refused release and an unclaimed key must record NOTHING. Per #1000 it carries the negatives that a bare "somebody released something" record would sail through: a `-Force` takeover must name the PRIOR holder and the releaser as DIFFERENT paths, and `force` must record **false** when the flag was not passed. Also asserts the on-disk shape (LF, no CR, newline-terminated, every line parseable) and that `-List` still reports "No active claims." with the ledger present. **Not covered, deliberately:** `prune-merged.ps1` clears a stranded claim with its own `Remove-Item` and writes no `.history` line — reported rather than folded in, because its releases are proven-own-worktree-only and already surface in its `claimsReleased` receipt, so the ledger is a complete record of `claim.ps1` releases and not of every release. **Cluster:** Coordination tooling / claim integrity. **Priority:** P3. **Verdict:** build (add a record; do **not** remove `-Force`). **Severity:** moderate — nothing is silently mis-authorised, but a coordination primitive can be overridden by the party it constrains with no trace, which makes it a convention rather than a control. @@ -5211,7 +5211,7 @@ Both readings reach the same operational conclusion, which is the whole point of ## 1080. `setup-leak-gate.ps1` reports the LOADED token set as if it were the one it just installed -> 🔢 **Filed 2026-08-06 — not started.** Value **3/10** · Difficulty **1/10** · _quick win_. `-Synthetic` prints *"Installed the SYNTHETIC template"* and then, three lines later, *"CONFIGURED with the real token set."* Both are individually true when `MEFOR_FORBIDDEN_TOKENS` is set: the script installed the synthetic file, and the scanner loaded the real list from the environment, which wins over the file. Nothing in the output says the env source **overrode** what was just installed, so the two lines read as a contradiction or, worse, as confirmation that the synthetic install produced a real-token gate. +> ✅ **Shipped 2026-08-10.** `setup-leak-gate.ps1`'s verify step now prints a `token source:` line naming what the scanner **actually loaded** — `MEFOR_FORBIDDEN_TOKENS -> `, that variable carrying the list inline, or `scripts/security/scan-tokens.local.txt` — plus an `OVERRIDDEN` banner when an install ran and the environment won over the file just written. Reproduced before the fix on a throwaway checkout, in exactly the filed shape: `-Synthetic` with `MEFOR_FORBIDDEN_TOKENS` pointing at another list printed *"Installed the SYNTHETIC template"* and then *"CONFIGURED with the real token set"*, with nothing between them to reconcile the two. **An inline value is named but never PRINTED** — a non-path value IS the token list, and echoing it would publish what the gate protects into whatever log the operator was capturing. That is the load-bearing negative in the test file, because the obvious implementation of "name the resolved source" prints the variable's value and passes everything else. An explicitly **EMPTY** `MEFOR_FORBIDDEN_TOKENS` is now named as the CAUSE of `NOT CONFIGURED`, because that state means "no source" and does **not** fall back to the file, so the ordinary advice (`-From ` / `-Synthetic`) sends the operator round the same loop forever. **A second reporting defect in the same file is closed in the same commit:** the scanner's exit code was discarded entirely, so a refusal came back out as `CONFIGURED` and exit 0 — contradicting this script's own header promise to *"exit non-zero if the sections are empty"*. Measured with an impossible `MEFOR_MIN_DETECTORS` floor: scanner exit **2**, script exit **0**. It now exits with the scanner's code under `VERIFY FAILED`, and deliberately does **not** reprint the scanner's output, since a hit line can quote matched content. `scan_forbidden.py` was **not** modified. Precedence is defined by its `_resolve_token_text` and this script necessarily re-expresses it to name the source, so `tests/test_setup_leak_gate_reports_source.py` (8 tests, **7 red first**; the one green is the paired positive control) pins the copy against the scanner's own `loaded names=` counts **in the same run**, and asserts the two sources yield **different** counts — a fixture that could not tell them apart would pass while measuring nothing. Its token fixtures are DERIVED from the shipped example rather than hand-written, so no numeric site prefix is written into tracked test source and every floor section stays non-empty. Its child environment is pinned explicitly (`MEFOR_*` and `PYTHONIOENCODING`/`PYTHONUTF8` removed, `python` resolved through a shim), and that pinning is proven in both directions: the suite passes under a hostile ambient shell whose values make the same invocation exit 1 when inherited. **Related:** #1063 (same script, anchoring rather than reporting) — this closes the reporting half of that pair. **Cluster:** Developer tooling / reporting accuracy. **Priority:** P4. **Verdict:** build (trivial). **Severity:** low, and the direction is safe — the gate really is loaded with the real set, so the operator is better protected than the message implies, not worse. The cost is that a reader cannot tell which source armed the gate, which is the one question the script's own docstring says the verify step exists to answer. @@ -7415,7 +7415,7 @@ filing. ## 1208. no guard asserts that a credential factory PARAMETER maps to a SETTING name the redactor covers -> 🔢 **Filed 2026-08-09 - not started. THREE MEASURED INSTANCES of one shape, not a hypothesis.** Value **7/10** · Difficulty **4/10**. A connector factory takes a credential parameter and emits it under a DIFFERENT setting name. Every redaction control operates on the SETTING name. Nothing asserts the two agree, so a rename silently moves a credential outside the control's domain. +> ✅ **SHIPPED (test-only; `messagefoundry/config/wiring.py` unchanged).** `tests/test_credential_parameter_mapping.py` follows the VALUE across the rename boundary: a unique sentinel is injected into ONE factory parameter at a time, the factory is called, and the destination setting is read off the emitted settings - so no rename has to be taught to it, which is the fourth name list this item forbids. The real redactor is then asked about whatever key the sentinel was found under. A parameter that reaches no setting, or that cannot be built at all, is a FAILURE with a declared reason rather than a silent pass, and every declared exemption is asserted REACHED so the tables cannot go stale. Domain re-derived rather than inherited: 23 spec-returning factories (asserted `>= 23`), 58 credential- and URL-bearing parameters probed, 3 followed through an `env()`-only refusal. Watched fail: pre-#1106 reddens exactly the 2 renamed `with_signing` parameters and leaves 56 green; pre-#1207 reddens exactly the 10 URL-bearing ones and leaves 48 green; shipped code reddens none. It reaches surfaces the outcome-level sibling cannot: de-classifying `intake_api_key`, `intake_api_key_next`, `credential_password`, `ws_password` and `client_key_password` reddens this file on all five and `test_connection_factory_redaction_domain.py` on none, because that file drops a connector's credential arguments when the connector refuses to be built with them. `proxy` -> `proxy_url` is now covered BY DESIGN rather than by luck: URL-bearing parameters are selected by a suffix rule whose own coverage is checked against the destinations the redactor's URL rule owns. **Cluster:** Security / secret disclosure - prevention. **Priority:** P2. **Verdict:** build. **Severity:** no live defect at filing - the three known instances are closed. This is the guard that