From 23555bdd91c15f4813ee41c41b52c0b6a3eb625e Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 28 Jul 2026 20:58:56 -0500 Subject: [PATCH] fix(tests): restore the environment before recomputing the site-code globals test_anon_parity's engine/tee divergence guard failed in any full-suite run on a box with a real token source configured, while passing in isolation. The cause was a teardown ordering bug three modules earlier. synthetic_site_prefix patches MEFOR_FORBIDDEN_TOKENS and recomputes the surrogates module globals from it. Its teardown called delenv and reloaded BEFORE monkeypatch restored the real value, so _SITE_PREFIXES was left derived from an environment that no longer existed -- and nothing recomputed it once monkeypatch put the real value back. The engine's globals then stayed stale for the rest of the session while the vendored tee/anon copy kept its import-time value, so the two diverged on an unrelated MDM^T01 hundreds of tests later. monkeypatch.undo() restores the real environment first, so the reload sees the same source the module saw at import. Confirmed by prediction rather than inspection: the failure disappears when MEFOR_FORBIDDEN_TOKENS is unset, which is exactly the condition that decides whether the stale value differs from the restored one. The regression guard is declared last in test_anon_core.py so it runs after every fixture user, and asserts the live globals still agree with a fresh recomputation. Verified it can fail: reverting monkeypatch.undo() to the old delenv reddens it (with the mutation proven applied by an exact line count first -- a replace that silently matches nothing reads as a pass). Note it also REPAIRS as it detects, since it reloads. That is deliberate: a reintroduction surfaces as one named failure at the true site instead of a distant, confusing parity failure. This never reddened CI because the test job sets no token source, so the guard the anonymization design depends on is weaker there than on a developer box. Worth closing separately; this change does not address it. --- tests/test_anon_core.py | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/test_anon_core.py b/tests/test_anon_core.py index 74b7c1b9..4a3a21e8 100644 --- a/tests/test_anon_core.py +++ b/tests/test_anon_core.py @@ -52,7 +52,15 @@ def synthetic_site_prefix(monkeypatch: pytest.MonkeyPatch) -> Iterator[str]: monkeypatch.setenv("MEFOR_FORBIDDEN_TOKENS", "[site_prefix]\n99\n") surrogates.reload_site_prefixes() yield "99" - monkeypatch.delenv("MEFOR_FORBIDDEN_TOKENS", raising=False) + # Undo the patch BEFORE recomputing, and recompute from the environment that is actually restored. + # `delenv` + reload was wrong in a way that only shows up when a real token source is configured: + # it left the module globals derived from an environment with NO token source, and monkeypatch then + # restored the real value afterwards with nothing to recompute the globals again. The engine's + # `_SITE_PREFIXES` stayed stale for the rest of the session while the vendored `tee/anon` copy kept + # its import-time value, so `test_anon_parity` — the engine/tee divergence guard — failed on an + # unrelated message hundreds of tests later. `monkeypatch.undo()` puts the real environment back + # first, so the reload below sees the same source the module saw at import. + monkeypatch.undo() surrogates.reload_site_prefixes() @@ -291,3 +299,28 @@ def test_anonymize_with_explicit_rules_only_touches_those_fields() -> None: assert "DOE" not in pid.split("|")[5] # PID-5 scrubbed assert "12345" in pid # PID-3 left intact (not in the explicit rule set) assert "DOE^JANE" in out # NK1-2 untouched (only PID-5 was in scope) + + +def test_site_prefix_fixture_leaves_module_globals_consistent_with_the_environment() -> None: + """Regression guard for a cross-module leak that cost a full-suite failure hundreds of tests later. + + ``synthetic_site_prefix`` patches ``MEFOR_FORBIDDEN_TOKENS`` and recomputes the ``surrogates`` + module globals from it. Its teardown used to ``delenv`` and reload BEFORE monkeypatch restored the + real value, leaving ``_SITE_PREFIXES`` derived from an environment that no longer existed. Nothing + recomputed them afterwards, so on any box with a real token source configured the engine's globals + stayed stale for the rest of the session while the vendored ``tee/anon`` copy kept its import-time + value — and ``test_anon_parity`` (the engine/tee divergence guard) failed on an unrelated message. + + Declared last in this module so it runs after every fixture user: it asserts the live globals still + agree with a fresh recomputation from the CURRENT environment. It is only meaningful where a token + source is actually configured, which is exactly the condition the original bug needed — and is why + CI, which does not set one for the test job, never saw the failure. + """ + from messagefoundry.anon import surrogates + + live = surrogates._SITE_PREFIXES + surrogates.reload_site_prefixes() + assert live == surrogates._SITE_PREFIXES, ( + "surrogates._SITE_PREFIXES drifted from what the current environment yields — a fixture " + "recomputed them under a patched environment and did not restore them afterwards" + )