feat(api): mask the message summary on list surfaces, reveal it when one message is opened (BACKLOG #1187) - #514
Merged
Merged
Conversation
…(BACKLOG #1149) ASVS 7.5.2 asks that a user terminate sessions "having authenticated AGAIN" -- an authentication event SUBSEQUENT to the one that established the session. Both terminate routes used require_reauth_only, whose recency test is satisfied by the step-up window the login ceremony itself seeds. So a caller could mass-revoke every other session of the account with no proof beyond the sign-in they already held. Bind both routes to require_reauth_only_action(STEP_UP_ACTION_SESSION_TERMINATE) instead. That factory already exists (ADR 0077) and is the right one on both axes: the grant is single-use and bound to this action, so a login-seeded window no longer satisfies it, and it stays in the reauth-only family, so a require_mfa user who has not enrolled a factor is still able to revoke rather than being deadlocked out (WP-14). THE TWO INVERTED TESTS ARE THE ACCEPTANCE CRITERION, NOT INCIDENTAL CHURN. test_list_and_revoke_own_session and test_revoke_other_sessions_keeps_current previously logged in and terminated with NO intervening authentication of any kind, asserting 200. That pair passing WAS the defect, stated as a test. They now assert 403 with X-Step-Up-Action: session_terminate, then 200 after an action-bound reauth. Measured red-first: all six selected tests failed on the change with the audit log showing purpose=null, before any test was touched. test_cannot_revoke_another_users_session gains a pre-gate 403 and an action reauth so its 404 still measures OWNERSHIP rather than the step-up; without that it would pass on the gate and prove nothing about what it is named for. The three stale-window tests and test_revoke_ownership_404_survives_reauth now name the action on reauth. Scope is the owner's one-liner, and this commit is the JSON API half only. RESIDUAL, AND IT IS A LIVE BYPASS UNTIL THE NEXT COMMIT: the browser console twin is unfixed. messagefoundry_webconsole/routes/account.py:301-302 says so in its own comment -- "a fresh login seeds the step-up window, so an immediate post-login revoke is unaffected" -- and register_ui_action at :93-94 maps both revoke paths with no action. The console is the surface a human actually uses, so this item is NOT closed by this commit. Verification, with scope: ruff check and ruff format --check on the three changed files, clean; mypy --strict on the two engine files, clean; pytest tests/test_api_auth.py in this lane's venv, 63 passed. Full-suite baseline taken at this base BEFORE these edits carries 19 pre-existing failures, all in test_dependabot_automerge_guardrails, test_installed_coord_hooks and test_workflow_shell_syntax -- zero of them reference any file changed here, checked by grep as a second instrument.
…n (BACKLOG #1149) The previous commit bound the JSON terminate routes to a single-use action grant and said in its own message that the browser twin was still open. This closes it. The console is the surface a human actually uses, so shipping only the JSON half would have moved the defect rather than fixed it. Both /ui terminate POSTs now take require_ui_reauth_only_action(STEP_UP_ACTION_SESSION_ TERMINATE), and both register_ui_action entries carry the matching action= tag so /ui/reauth mints the grant the continuation then consumes. They were the LAST /ui write actions whose gate a login-seeded window satisfied -- the factor lanes got this treatment under 7.5.1 (ADR 0077) and the terminate lanes were left behind. A COMMENT THAT DOCUMENTED THE DEFECT AS INTENDED BEHAVIOUR IS CORRECTED, NOT DELETED. The route header read "a fresh login seeds the step-up window, so an immediate post-login revoke is unaffected". That was TRUE of the code and was the defect stated as a design note, which is why it survived review: it reads as a considered carve-out. The retraction is kept in place beside the correction so the next reader sees what changed and why. THREE CONSOLE TESTS FAILED ON THIS CHANGE AND THE THIRD IS THE INTERESTING ONE. test_sessions_revoke_one and test_sessions_revoke_others are the console twins of the JSON pair and are inverted the same way -- they now assert the bounce to /ui/reauth, assert nothing is revoked by it, then mint and retry. test_sessions_posts_reject_cross_site is a DIFFERENT problem and would have passed later for the wrong reason. The action gate runs before assert_same_origin, so a grantless cross-site POST now 303s to a login form instead of reaching the CSRF check -- the test would have been measuring the step-up, not the origin guard it is named for. It mints the grant first, so a cross-site POST is still required to be refused OUTRIGHT rather than bounced. Same shadowing the JSON ownership-404 test had to avoid, one layer over. The five pre-existing stale-window and registration tests were untouched and still pass: a missing action grant and a stale window produce the same 303, which is why only three of the eight failed. Verification, with scope: ruff check and ruff format --check on both changed files, clean; mypy --strict on the console route, clean; pytest on the nine session-terminate tests in packaging/messagefoundry-webconsole/tests/test_webui.py, 9 passed. The full console suite ran 360 passed / 3 failed BEFORE these test edits, and those 3 are exactly the ones fixed here.
…CKLOG #1149) Five rows in SECURITY.md still described the pre-#1149 wiring. Left alone they would be a security document asserting a control that no longer exists in the form it names -- the compensating-control-on-a-false-premise defect, arriving through staleness rather than through error. Four are route rows: the two JSON terminate routes and the two /ui twins now name the action-bound factory and the session_terminate action. THE FIFTH WAS CAUGHT BY A GATE, AND THE GATE WAS RIGHT ABOUT SOMETHING I HAD WRONG. The gate-wrapper tally read "2 / 2" for require_reauth_only / require_reauth_only_action. I measured the live counts as 0 and 4 and wrote "0 / 4", which is arithmetically true and still drift: test_gate_wrapper_table_counts_match_the_route_walk compares the table against a walk of the live app, and require_reauth_only is now ABSENT from that walk rather than present-with-zero. A wrapper with no routes does not belong in a table of wrappers and their route counts. The row now names only require_reauth_only_action at 4, and says in prose that require_reauth_only still exists and still backs the /ui twin -- so a reader who greps for it and finds it is not left thinking the table is wrong. Worth recording because the correct-looking fix was the wrong one: a zero is a claim that something is there and unused, which is a different statement from it not being there. Verification, with scope: pytest tests/ -k "security_doc or doc_drift or route_table or security_md" in this lane's venv -- 112 passed, 91 skipped, and the one failure that gate raised against my first version is what produced this commit. Note the skips are not silent here: test_threat_model_doc_drift reports in its own words that docs/security/ is withheld from public checkouts, so its content assertions are INERT in this run rather than passing.
…y time (BACKLOG #1140) ASVS 6.3.8 asks that valid users not be deducible from failed challenges, including by different response times. The local leg is equalized by a fixed dummy argon2 verify. The AD leg was not: an absent or disabled principal returned before the password-verifying bind, so one case cost a whole Server build, TCP connect and bind round trip that the other did not -- behind an identical response. On a first deployment with AD enabled that is a directory-namespace enumeration oracle the sign-in limiter rate-bounds but never removes. _equalizing_bind does that work anyway and discards it, against a DN that cannot exist under the configured search base. It is the AD analogue of _DUMMY_PASSWORD_HASH. THE OBVIOUS FIX WOULD HAVE BEEN A BYPASS, AND AVOIDING IT IS THE POINT OF THIS COMMIT. The disabled-bit check lives inside _find_user, which has TWO callers: authenticate binds, and the Kerberos/SSO resolve_principal does not. Relocating that check into the bind path would equalize the timing AND LET A DISABLED ACCOUNT AUTHENTICATE OVER SSO -- trading a timing leak for an authentication bypass. So the CALLER is equalized and the check does not move. Git refused my first edit as ambiguous because both call sites share the same four lines, which is the same fact arriving as a tool error. THE SWALLOWED LDAPException IS LOAD-BEARING, NOT LAZY. The caller turns LDAPException into LdapError, so letting a bind against a deliberately-bogus DN raise would turn an ordinary wrong-username login into a CONNECTIVITY ERROR -- a louder oracle than the one being closed. RED-FIRST, PROVEN, NOT ASSERTED. Removing only the _equalizing_bind call reds exactly the two new equalization cases and nothing else; the file was restored and re-hashed byte-identical (SHA-256 equal before and after). The hazard-guard test correctly stays GREEN under that plant, because the plant does not touch the disabled check it tests -- an asymmetric control, so a neutered rule does not simply fail everything. The 2/2 Server+Connection counts are an EQUALITY claim, not a bare number: the success path pins the same 2/2 in the test directly above. WHAT THIS DOES NOT CLAIM: that wall-clock is provably equal. It equalizes the code PATH, which is what the item measured. A directory may still answer invalidCredentials and no-such-object in different times, and the group-resolution search on the success path remains unmatched. No timing measurement has been run, by the item or by me. Verification, with scope: ruff check + ruff format --check on both files, clean; mypy --strict on messagefoundry/auth/ldap.py, clean; pytest tests/test_ldap_timeouts.py + test_auth_service.py + test_auth_hardening.py + test_api_auth.py in this lane's venv, 145 passed.
…DR 0170) ASVS 11.2.4 asks for no short-circuit in cryptographic comparisons or returns. _verify_second_factor returned on the first argon2id match, so the NUMBER of ~64 MiB verifications was a function of which code was presented. TWO LEAKS AND ONLY ONE MATTERS, which is worth stating because it changes what the fix has to be. The matched INDEX is near-worthless: whoever measures it already holds a working code and the response answers them anyway. The real one is on the FAILURE path -- the cost is one verify per REMAINING code, so anyone holding the password can time a wrong-code refusal and learn how many recovery codes an account has left, without ever authenticating to the second factor. THE ITEM RATED THIS DIFFICULTY 7 ON A PREMISE THAT DOES NOT SURVIVE MEASUREMENT. Its re-score says a constant-time loop "converts a timing leak into a memory and CPU amplification target". That is exactly the right objection to raise, and it is false here: THE FAILURE PATH ALREADY VERIFIES EVERY REMAINING HASH. Making the walk unconditional introduces no new cost -- it makes today's WORST CASE the only case. So: always run exactly mfa_recovery_code_count verifies, padding with the same fixed _DUMMY_PASSWORD_HASH the local login leg uses, and select the winner AFTER the loop. The ceiling does not move (default 10, validator-capped at 50), _argon2's semaphore means the concurrent-argon2 footprint cannot widen either, and the path sits behind primary authentication so it is not an unauthenticated flood surface. CLAIMS CONSTANT WORK, NOT CONSTANT TIME, and the difference is written into the test's own docstring so it cannot be over-read later. Not equalized: the store round trip on a match, the TOTP branch that returns earlier, and argon2's own constant-timeness, which is INHERITED from argon2-cffi and has never been measured in this tree -- a gap #1167 names and this does not close. No timing measurement was run, by the item or by me. RED-FIRST, PROVEN. Removing only the padding reds ALL THREE parametrized cases -- first-slot match, last-slot match, non-match -- with the diagnostic naming the short-circuit, and the file restored byte-identical by SHA-256. ADR 0170 records the rejected alternative rather than dropping it: a non-secret lookup index so only ONE verify ever runs is strictly better on both axes, and is out of scope rather than wrong -- it needs a schema change across three backends and a migration. Recorded so the next reader does not re-derive it. Coordinates were re-derived from the SYMBOL, not the item's numbers: _verify_second_factor is at :2223 where the item cites :2076-2080, and AesGcmCipher.decrypt is at :753 where it cites :767. Both had drifted, as the dispatch warned. DOES NOT CLOSE #1167. The item names a second site, the AES-GCM keyring walk in store/crypto.py, which is untouched here. And per the census I ran and reported, the set of sites is NOT asserted complete -- at least these, bounded by a predicate that cannot see a comparison site in a package importing no crypto module. Verification, with scope: ruff check + ruff format --check on both changed code files, clean; mypy --strict on messagefoundry/auth/service.py, clean; pytest test_auth_hardening + test_auth_service + test_api_auth + test_ldap_timeouts, 148 passed; the ADR/doc/link/backlog guard selection, 493 passed after the ADR was staged (the link gate requires its target to be tracked, which is a real check and not a nuisance).
…d correct (BACKLOG #1167) #1167 names two data-dependent early returns. The first, the argon2id recovery-code walk, was a real leak and is fixed (ADR 0170). THIS IS THE OTHER ONE, AND IT IS A NON-DEFECT. Recording that in place, because an unreported non-defect is rediscovered at full cost and the rediscoverer may "fix" it. AesGcmCipher.decrypt tries the named key, then every other key, and returns on the first tag that validates. It is a literal short-circuit return inside a cryptographic loop, so it looks exactly like ASVS 11.2.4's target -- and its sibling twenty lines of reasoning away genuinely was one. Three independent reasons it is not, any one sufficient: 1. THE INPUT IS NOT ATTACKER-SUPPLIED. `stored` comes from the store, not from a request. Submitting chosen ciphertext here already requires database write access. 2. WHAT THE TIMING REVEALS IS ALREADY IN PLAINTEXT. The candidate count varies with which key encrypted the row, and `key_id` is parsed out of `stored` two lines above because the marker publishes it. The leak is a value the same string hands over for free. 3. THE PER-CANDIDATE COST IS A GMAC TAG CHECK -- microseconds, constant-time inside `cryptography`. The recovery walk mattered because each step was a ~64 MiB argon2id. AND THE FIX WOULD COST SOMETHING REAL: forcing all candidates tag-checks every row against every retired key on every read, multiplying store read cost through a rotation, to conceal a value the ciphertext already prints. SCOPED, NOT GENERALISED. #1167 names extending this exact defence to the recovery-code walk as the one move that would NOT be an honest pass. The recovery walk was fixed on its own merits before this was written, so the argument is being used where it holds and nowhere else. #1167 IS NOT CLOSED BY THIS. Its two named sites are now resolved -- one fixed, one examined -- but per the census I ran and reported, THE SET IS NOT ASSERTED COMPLETE: a predicate keyed on secret-shaped names cannot see a comparison site in a package importing no crypto module, and in a tree speaking HL7, X12 and DICOM that heuristic produces false positives rather than coverage. At least these two. The taint-based census that would bound it properly is filed as content with the dispatcher, not absorbed here. Verification, with scope: ruff check + ruff format --check clean; mypy --strict on messagefoundry/store/crypto.py clean; pytest -k "crypto or cipher or encryption", 199 passed, 1 failed. That failure is test_dependabot_automerge_guardrails::test_allowset_holds_everything_not_named[uv-cryptography-false] -- a -k substring hit, "cryptography" contains "crypto" -- and it is PRE-EXISTING, confirmed two ways: exact node-id match against the baseline I recorded at this base before any edit, and zero references to any file I have changed in that module.
…hable pointer (BACKLOG #1151) ASVS 8.1.1 asks that authorization documentation define rules for FUNCTION-level and DATA-specific access. The function axis is exhaustive and CI-pinned. The data axis was not documented at all on the surface that has one. TWO FIXES, both narrow, matching the owner's one-liner rather than the item's full research programme: 1. The three /search/presets rows in SECURITY.md carried EMPTY Extra-constraints cells while the code scopes every read, list and delete to the caller. They now state the rule: the permission grants the FUNCTION, the row's owner grants the DATA, and a preset id belonging to another user is a miss rather than a 403 because ownership is part of the lookup. 2. auth/identity.py's allowed_channels comment ended "See docs/security/PHASE-8C-RBAC.md" -- a path a reader of the PUBLIC repository cannot reach. docs/security/ is gitignored here, so git ls-files docs/security returns ZERO and the directory is absent from a fresh checkout. That is a standing decision, not an oversight, which is exactly why the pointer had to go: a dangling reference to a security document is worse than none, because it tells the reader the rule is written down somewhere they can look. The rule is stated inline instead. I NEARLY REPLACED THE UNREACHABLE POINTER WITH A FALSE SENTENCE, WHICH IS THIS ITEM'S OWN FAILURE MODE, AND THE RETRACTION IS KEPT IN THE COMMENT. My first draft wrote "empty frozenset means every channel". The annotation is frozenset[str] | None = None and NONE means every channel; an empty frozenset would restrict to NOTHING -- the inverse. Caught by re-reading the type rather than by any gate. Verified against enforcement afterwards: eight call sites test `is None` / `is not None`, and api/app.py:1684 says in its own comment that an unscoped caller "sees the full estate". THE ITEM'S OWN CITATION IS STALE TWO WAYS, re-derived from the symbol as the dispatch requires: it cites api/app.py:3960 calling list_search_presets(identity.username); at HEAD it is :4169 and it passes identity.USER_ID, not username. The rule is keyed on the immutable id, which is the stronger property and is what the doc now says. WIDER FINDING, NOT ABSORBED AS SCOPE: the dangling docs/security/ pointer is not one site. At least TEN exist in shipped engine code, and several are RUNTIME MESSAGES rather than comments -- api/app.py:1280 and :5991, client_networks.py:257, trust_anchors.py:233, __main__.py:1762 and :1835 all name docs/security/OFF-LOOPBACK-DEPLOYMENT.md to an operator who cannot open it. config/retention_classification.py:13 already records the gitignore situation in its own words, so the repo knows. That is content for the dispatcher to file, not this item. DOES NOT CLOSE #1151. The item also asks whether a drift GATE over data-scoping claims is constructible -- the function axis is credible precisely because CI fails when the map drifts, and no equivalent instrument exists for this. Untouched here, and the sentences I just wrote are unchecked prose exactly like the ones they replace. Verification, with scope: ruff check + ruff format --check clean; mypy --strict on messagefoundry/auth/identity.py clean; pytest -k "security_doc or doc_drift or link_resolution or route_gates or connection_event_scope", 126 passed, 91 skipped.
…ial" (BACKLOG #1141) HALF OF #1141. The predicate is here; the api/app.py call site is DELIBERATELY NOT in this commit -- see the hold at the bottom. ASVS 6.4.5 wants renewal notice before an expiring credential dies. The API lifespan gates the reminder task on `auth_settings.bootstrap_expiry_hours > 0`, while `AuthService.bootstrap_expiry_warning` warns on the EARLIER OF TWO bounds: WP-3 account retirement AND the ASVS 6.4.1 credential expiry. Those are different questions. THE CONSEQUENCE IS A SILENTLY DEAD ARM, NOT A STYLE DEFECT. At bootstrap_expiry_hours=0 with initial_password_expiry_hours set, the warning method computes a correct deadline and NOTHING EVER CALLS IT -- the reminder task is its only consumer, verified by grep, and it was never created. The operator gets no notice before the credential dies. BACKLOG #1245 corrected the two deadline computations in this file and never reached the gate deciding whether they run. That is the same lesson `_unclaimed_bootstrap` already records one level over, in its own docstring: "two open-coded copies of one lifecycle test is exactly how the warn path silently inherited BACKLOG #1245." This is the third copy, in another module, and it inherited it too. `bootstrap_deadline_configured` is modelled on `directory_reconcile_enabled` directly above it, which documents the identical does-the-lifespan-create-this-task role. THE TEST IS A TRUTH TABLE AND IT IS ASYMMETRIC BY CONSTRUCTION. Four rows: neither bound, WP-3 only, CREDENTIAL only, both. A gate that always returned True passes three and fails the first; the OLD single-bound gate passes three and fails the third. No single wrong answer satisfies the table. RED-FIRST, AND THE FIRST ATTEMPT AT THE PLANT FAILED LOUDLY, WHICH IS THE POINT. My literal here-string replacement did not match, and the script asserted the mutation applied rather than running on regardless -- a no-op mutation scores as a pass and would have proven nothing. Re-run with a whitespace-robust pattern: 91 characters changed, and restoring the pre-#1141 predicate reds EXACTLY ONE row, the credential-only one, whose parametrize label already named it "THE ROW THAT WAS SILENTLY DEAD". File restored byte-identical by SHA-256. HELD, NOT FORGOTTEN: the one-line call-site change in messagefoundry/api/app.py is NOT here. The collision gate refused it and named Builder 2, who holds 9 uncommitted lines in that file at :1116 and :5663. My site is ~:6010 and is disjoint from both by thousands of lines and by function -- and "I measured it as disjoint" is exactly the shape of talking myself past a blocked control, so I asked instead. That request is with them, with three ways out offered including handing them the change. The dispatcher lifted my api/app.py fence on the stated premise that Builder 2 held nothing in it. That premise is false and they have been told, because it was a claim about a LANE and may have cleared other routing too. The likely mechanism is a check computed over COMMITTED state, which cannot see a peer's working tree. Verification, with scope: ruff check + ruff format --check on both changed files, clean (the formatter reformatted the test file and it was re-verified after); mypy --strict on messagefoundry/auth/service.py, clean; pytest test_auth_hardening + test_auth_service + test_api_auth + test_ldap_timeouts in this lane's venv, 152 passed. `git status --porcelain -- messagefoundry/api/app.py` is empty: I did not touch it.
…KLOG #1236, ADR 0171) A deployment with ONE administrator had no recovery from account lockout. Each exit is individually deliberate; the defect is that they close SIMULTANEOUSLY. All re-verified on origin/main: the bootstrap account is literally `admin`, it is created with no email so the ACCOUNT_LOCKED notice never leaves the process, self-reset is refused, an admin reset needs ANOTHER admin, re-bootstrap fires only on an empty users table, and none of 38 CLI subcommands managed users. THE FILED ACCEPTANCE CRITERION COULD NOT DISCRIMINATE, WHICH IS WHY I BUILT AGAINST THE AMENDED ONE. "Recover without hand-editing the database and without a second admin" PASSES ON THE SHIPPED SYSTEM BY WAITING -- the lock self-expires after lockout_minutes. A test a defect-free system and the defective system both pass is not a test. The 2026-08-21 amendment requires recovery on demand, gated, and faster than that window. So the acceptance test locks the account A DAY OUT and never advances a clock. Waiting cannot satisfy it; only the recovery path can. THE GATE IS HOST ACCESS AND IT IS A REAL GATE RATHER THAN AN ABSENT ONE. Reaching this needs the config, the store path and on an encrypted store the key material -- the operator who installed the engine. Anyone holding all three already HAS the database and does not need an unlock to reach an account, so it grants nothing the trust boundary did not already imply. That is why it ships unauthenticated and it is the load-bearing claim: if it is wrong, the design is wrong. CLEARS THE LOCKOUT, DOES NOT RESET THE PASSWORD. Deliberately narrower than the obvious fix -- the holder still needs their credential, where a reset would hand whoever ran it a working account. REUSES record_login_failure(failed_attempts=0, locked_until=None) RATHER THAN ADDING A PROTOCOL METHOD, and a MEASURED cross-lane fact decided that rather than taste: a named clear_lockout would touch base/store/postgres/sqlserver, and all four were uncommitted in a peer lane's worktree at the time. Reuse avoided a four-file collision and a coordination round. No migration, no store change. EXIT CODES FOLLOW THE --json CONVENTION, NOT THE M-31 LINEAGE. The file carries two: _emit_error prints JSON and returns 1; a bare stderr line returns 2. This command has --json, so a stderr line would break a caller's parsing. Verified the discriminator rather than assuming it -- audit-verify, whose M-31 guard this copies, has NO --json flag and correctly uses the other one. RED-FIRST, AND THE ASYMMETRY IS THE FINDING. Neutering the clearing call reds EXACTLY ONE of the four tests -- the acceptance one, with its own message. THE AUDIT-ROW TEST STILL PASSES UNDER THAT PLANT, because the row is written whether or not the clear happened. So it evidences the flow RAN and never that it WORKED, and that is recorded in the ADR so nobody reads a green audit test as proof of the unlock. File restored byte-identical by SHA-256. M-31 carried forward: a typo'd --db is refused rather than creating an empty SQLite store and reporting a false "no such account" -- which reads as a wrong USERNAME when the truth is a wrong DATABASE. The test asserts the file was not created. DOES NOT CLOSE #1236's REPETITION LIMB. An active lock cannot be extended, but lock CYCLES are unbounded and an attacker can re-lock. SECURITY.md already words this as bounding the lock rather than the campaign. Separate control, not claimed here. Verification, with scope: ruff check + ruff format --check on both code files, clean (the formatter reformatted the test file; re-verified after); mypy --strict on messagefoundry/__main__.py, clean; pytest tests/test_cli.py, 94 passed; the link/ADR guard selection, 339 passed. Every file touched here was confirmed clear of all six live worktrees before writing, with a positive control proving the query can see a dirty file.
…141) Completes #1141. The predicate landed at 5c1fbdb; this is its call site, which was HELD on cross-lane coordination rather than forgotten. The ASGI lifespan gated the ASVS 6.4.5 reminder task on `auth_settings.bootstrap_expiry_hours > 0` -- a THIRD open-coded copy of a question `bootstrap_expiry_warning` answers over TWO bounds: WP-3 account retirement AND the ASVS 6.4.1 credential expiry. AT bootstrap_expiry_hours=0 WITH initial_password_expiry_hours SET, the warning method computed a correct deadline and this task -- ITS ONLY CONSUMER, verified by grep -- was never created. The warning arm was silently dead and an operator got no notice before the credential died. BACKLOG #1245 corrected the two deadline computations in auth/service.py and never reached the gate deciding whether they run, which is the same lesson `_unclaimed_bootstrap` records in its own docstring one level over. HOW THIS EDIT WAS SEQUENCED, because the sequencing is most of the work and none of the diff: The collision gate refused it twice -- Builder 2 held 9 uncommitted lines in this file. I measured them as disjoint (pure insertions at 1116 and 5663; mine at 6010) and asked anyway, because "I measured it as safe" is the shape of talking yourself past a blocked control, and COMMON 2.5 records that exact case declined twice on the ground that waiting cost nothing. They cleared it, and THE GATE STILL REFUSED -- it has no coordinated-write path by design: "NOTHING TO OPT INTO... a coordination step you must remember is one you will skip." It asks you to coordinate and then cannot accept the answer. I did not override it. It blocks on live-AND-dirty, so it cleared on its own when they committed at 142b926, and they pinged unprompted as they had committed to. Re-verified their claim before acting on it rather than taking it: app.py clean in their tree, their commit carries the 9 lines, and a positive control confirmed the same query still sees their OTHER dirty files -- so the clean was a real clean and not a broken query. The gate is now advisory and says to check their commits before duplicating; I had. The dispatcher's fence-lift for this item rested on the false premise that Builder 2 held nothing here. That was escalated, they owned it, and they have adopted a standing method: a cross-lane fence is computed from the peer's WORKING TREE, freshly, never from item metadata. Verification, with scope: ruff check + ruff format --check on messagefoundry/api/app.py, clean; mypy --strict on it, clean; pytest test_auth_hardening + test_auth_service + test_api_auth + test_cli + test_ldap_timeouts in this lane's venv, 246 passed. The diff is two hunks at :6010 and :6014 and touches nothing else in the file.
…h corpus (BACKLOG #1134) Owner authorised the fetch. ASVS 6.2.4 asks for a check against "at least the top 3000 passwords WHICH MATCH THE APPLICATION'S PASSWORD POLICY". Measured before building: at the shipped password_min_length=15, only 18 of the bundled 10,000 entries reached that length and only 4 sat in the top 3,000. An entry shorter than the minimum can only reject what the length clause already rejects, so the screen added almost nothing on top of it. Policy-clearing coverage is now 5,274, measured through the SHIPPED LOADER rather than the build script. Corpus 79 KB -> 169 KB. SOURCE chosen by measurement, not guess: SecLists (MIT), the same upstream the NOTICE already credited. I listed the Pwdb series and worked UP from the smallest candidate -- top-100000 yields only 211 entries at length and misses the bar; top-1000000 yields 5,434 by length and 5,256 after the full policy filter, dedupe and hygiene. check_breached IS OFF DURING THE FILTER AND THAT IS LOAD-BEARING. With it on, every candidate already in the bundled corpus is rejected as "a common or breached password" -- by the very corpus being extended. The filter would have asked "is this already in the file" instead of "does this match the policy". check_username is off too: a corpus entry has no user context. THE ORIGINAL 10,000 ARE KEPT, NOT REPLACED. At min_length=15 they are redundant, but password_min_length is an OPERATOR SETTING -- a site that lowers it makes every short entry load-bearing again, and replacing the file would silently delete their protection for exactly the configuration that needs it most. THREE ENTRIES DROPPED THAT THE POLICY WOULD HAVE ACCEPTED, AND BOTH CLASSES WERE FOUND BY THE COMMIT GATES RATHER THAN BY ME. One carried a C0 control byte; two were IPv4-shaped. I filtered the cause rather than allowlisting: the leak gate's own message says a host string must be REMOVED, and allowlisting would train a reviewer to wave through the exact shape that gate exists to catch, to keep three worthless entries. Vendored third-party data carries junk a hand-written corpus never would, and I had not anticipated it. RULED OUT, both named by the item as the dishonest moves: lowering password_min_length so more of the existing corpus becomes reachable, and adopting the permissive reading silently. THREE OF MY OWN MISTAKES, ALL CAUGHT BY CONTROLS RATHER THAN BY REVIEW: - `git checkout --` to undo a red-first plant restored from the INDEX, which held my staged build, so it did NOT reset the corpus and the rebuild found 0 new. The zero-kept assertion refused to write. `git restore --source=HEAD --staged --worktree` is the one that resets both. - An earlier `git checkout --` on an UNCOMMITTED corpus destroyed the build outright; the byte-identical hash assertion reported it. Without that line I would have committed the pre-#1134 corpus with tests written against the new one, all passing. - Writing the hygiene regex through a shell heredoc interpreted the escapes and put a literal NUL in the build script. Rebuilt from code points instead -- an escape sequence in a file about control bytes is one mangling away from becoming the byte it describes. RED-FIRST: reverting the corpus reds both new tests with "only 18 corpus entries clear the shipped policy" -- the item's own figure, reproduced by the test rather than quoted. Verification, with scope: ruff check + ruff format --check clean; pytest test_auth_core + test_auth_hardening + test_auth_service + test_api_auth + test_scan_forbidden + test_settings + test_security_posture_defaults, 333 passed at the pre-hygiene build and test_auth_core 15 passed after; both gate classes independently re-checked at 0 by a byte scan and a regex, having been 1 and 2. Full-suite baseline at the new base 90d6f18: 19 failures, IDENTICAL BY NODE ID to the old-base baseline, so the docs-only commit moved no result.
…g floor (BACKLOG #1148)
FIRST LAYER OF #1148, and it lands alone because it is strictly additive. The route swaps come
next and CANNOT land without it: promoting a route from require_step_up to
require_step_up_action would silently strip the per-actor anti-automation floor, because the
action binding is visible in the diff and the lost pacing is not.
THAT ALREADY HAPPENED ONCE. PATCH /users/{user_id} was promoted to the action gate and lost the
floor; docs/SECURITY.md has filed it under "No limiter of any kind" ever since, with a sentence
explaining why. Charging the floor in the factory fixes that route too rather than only sparing
the two #1148 promotes -- so the exemption set is now EMPTY rather than growing to three, which
is the direction the item's own build condition demanded.
Placement mirrors require_step_up exactly: first inside the auth-enabled block, before the MFA
check, so a throttled write is refused before any further work.
THREE DOC STATEMENTS THIS FALSIFIED, ALL CORRECTED IN THE SAME COMMIT: the require_step_up
bullet's "except PATCH /users/{user_id}" carve-out, the gate table's require_step_up_action row,
and the Route -> limiter map's "No limiter of any kind" row. A code change that leaves the
security doc asserting the opposite is the compensating-control-on-a-false-premise defect.
AND THE RED-FIRST PASS FOUND A DEFECT IN THE GUARD ITSELF, WHICH IS THE PART WORTH KEEPING.
Removing the pacing call reddened only ONE of the two guards. The other,
test_users_manage_write_pacing_exemptions_are_named_exactly, kept passing -- because its
`charging` set was a HAND-MAINTAINED LIST OF FACTORY NAMES. It never compared that list to the
code, so it is a NAME check wearing a behaviour check's clothes: it would stay green over a
factory that stopped charging entirely. I had just updated that list myself, which is precisely
why it could not see my own removal.
Fixed by DERIVING it. The sibling guard already walked the AST for functions that really call
_enforce_admin_write_pacing; that derivation is now a shared helper and both guards use it.
Re-run of the same plant: BOTH guards red where one did before. The fix is proven to have added
coverage rather than merely passing.
Verification, with scope: ruff check + ruff format --check on both code files, clean; mypy
--strict on messagefoundry/api/security.py, clean; pytest test_security_doc_rate_limits +
test_security_doc_drift + test_api_auth + test_auth_hardening + test_security_posture_defaults,
208 passed. Plant restored byte-identical by SHA-256 both times.
NOT YET DONE for #1148: the two action constants, the JSON route swaps, the console registration
split and the console swaps. Next commit.
…CKLOG #1148) SECOND LAYER. The pacing parity landed first at a39fde4 and had to, or this commit would have silently stripped the anti-automation floor from these two routes. ASVS 7.5.1 wants full re-authentication before modifications to attributes that affect authentication, naming MFA configuration verbatim and NOT qualifying whose. The self-service half already satisfied it. Both ADMIN reset lanes rode the plain login-seeded window, so an administrator who signed in under step_up_max_age_seconds ago could act with ZERO fresh proof. What that gate protects is the most complete modification available to the named attribute: admin_reset_mfa disables TOTP and deletes every passkey, and disable_totp NULLs the recovery codes alongside the secret. ONE call clears the second factor, every recovery code and every passkey on someone else's account. I DID NOT BUILD WHAT THE RESEARCH SAID, AND THE DIFFERENCE IS A SECURITY REGRESSION. It specified the action-bound factories "keeping mfa_gate=False so an MFA-required-but-unenrolled operator is not deadlocked out of a reset". mfa_gate=False is require_reauth_only_action, which has NO MFA gate -- so following it would have REMOVED the second factor from the two routes above while the diff read as hardening, because the action binding is the visible half and the lost gate is not. The deadlock reasoning is sound WHERE IT CAME FROM -- the enrolment lanes, where an unenrolled user cannot satisfy a gate standing in front of the only route that enrols them. It does not transfer to an admin acting on a THIRD PARTY: the operator's own enrolment status has nothing to do with the target's. Built with require_step_up_action instead: action-bound AND MFA gate retained, strictly stronger than what shipped. Reported to the dispatcher for relay, because the researcher is repairing 24 more specs and the same carve-out may sit in any of them. THE RESET-MFA LANE SHIPPED WITH NO TEST AT ALL -- the sharper of the two. It has one now, and its SECOND assertion is the one that matters: it pins that the MFA gate is still demanded, with a failure message naming the exact substitution that would remove it. PROVEN WITH TWO PLANTS, not one. (A) reverting both routes to require_step_up reds the inverted test. (B) APPLYING THE RESEARCH'S OWN INSTRUCTION -- swapping to the mfa_gate=False factory -- reds the new MFA-gate assertion with that message. The regression that would have shipped is now pinned by a test that names it. Restored byte-identical by SHA-256. The existing reset-password test is INVERTED the same way #1149's pair was: it used to reset on a fresh login with no further proof, and that passing WAS the defect. Grants are single-use, so each of its four gated calls now mints its own -- without that they would all 403 and the test would stop measuring the 404/400 cases it is named for. Three doc-drift guards reddened on the gate change and are fixed in the same commit: the two route rows still named require_step_up, and the wrapper counts (require_step_up 26 -> 24, require_step_up_action 2 -> 4), re-derived from the guard's own route walk rather than adjusted by hand. BANDIT FLAGGED THE NEW CONSTANT AS A HARDCODED PASSWORD -- a NAME check, not a value check, firing because the identifier ends in PASSWORD. It is a step-up action id echoed publicly in the X-Step-Up-Action header. Suppressed with the house idiom already used twice in auth/notifications.py for the identical shape, with the reason on the line. Verification, with scope: ruff check + ruff format --check on all three code files, clean; mypy --strict on auth_routes.py and service.py, clean; bandit on service.py, clean; pytest test_security_doc_drift + test_security_doc_rate_limits + test_api_auth + test_auth_hardening + test_auth_service, 222 passed before the nosec and 105 on the re-run after. NOT YET DONE for #1148: the console plane. The console calls the JSON handler FUNCTIONS directly through the seam, so the JSON dependency never executes on the browser path -- and the browser console is the only operator surface that ships. Its registry entry is a combined untagged pattern that mints nothing for these actions. Doing the JSON half and stopping is the shape the research explicitly warns produces a completed-looking change that does nothing. Next commit.
…BACKLOG #1148) THIRD AND FINAL LAYER, and the one that actually reaches an operator. The JSON binding at d76673c does NOT reach the browser: the console calls the injected handler FUNCTIONS through the seam, so the JSON route's own dependency never executes on that path -- and the browser console is the only operator surface that ships. Landing the JSON half alone is the shape #1148's research explicitly warns produces a completed-looking change that does nothing. Both /ui reset routes now take require_ui_step_up_action, which keeps the MFA gate exactly as its JSON counterpart does. THE REGISTRY SPLIT IS THE OTHER HALF AND IT IS NOT COSMETIC. The four admin write actions shared ONE combined, untagged pattern, so /ui/reauth minted nothing for these actions and the browser kept riding the login-seeded window. reset-password and reset-mfa are now separate tagged entries; revoke-sessions and delete stay combined and UNTAGGED, deliberately -- they sit outside 7.5.1, which names attributes affecting AUTHENTICATION, and tagging them would be motion without a requirement behind it. The golden was regenerated, 25 -> 27 patterns, as its own failure message instructs. FOUR CONSOLE TESTS REDDENED AND TWO OF THEM WOULD HAVE PASSED LATER FOR THE WRONG REASON. The cross-site sweep and the AD carve-out both assert a refusal that the action gate now pre-empts, so without a grant they would measure the STEP-UP instead of the CSRF defence and the AD carve-out they are named for. Both mint first. The cross-site sweep already carried exactly this reasoning in a comment for the two webauthn lanes, so #1148 joins that pattern rather than inventing it. AND THE RED-FIRST PASS FOUND A COVERAGE HOLE IN MY OWN TEST EDIT, WHICH IS THE PART WORTH KEEPING. Reverting both routes reddened only ONE of the two roundtrip tests. The reset-mfa one kept passing -- because I had written it to mint and then succeed, which proves the FLOW works and NOT that the gate exists. So the sharper of the two lanes, the one that clears TOTP, every recovery code and every passkey, had no console coverage of its own binding. Fixed by asserting the BOUNCE before minting, mirroring the reset-password test. Re-plant: BOTH red where one did. Same lesson as #1236's audit-row test, hit again from a different direction: an assertion placed after the enabling step measures the step, not the gate. Verification, with scope: ruff check + ruff format --check on both changed files, clean; mypy --strict on the console route, clean; the FULL console suite, 363 passed / 3 skipped. Plant restored byte-identical by SHA-256 both times. #1148 IS NOW COMPLETE ACROSS ALL THREE LAYERS: pacing parity (a39fde4), the JSON binding (d76673c), and this. What it does NOT close, named in the research and not absorbed here: the full-factor re-authentication mint, the target-scoped grant, the console write-action tag golden (which compares path patterns only and still cannot see the action field), and the two change-password lanes that consult no second factor at all.
…G #1148)
#1148 moved `POST /ui/users/{id}/reset-password` and `reset-mfa` onto
`require_ui_step_up_action`, and updated the JSON API route map to match. The SEPARATE
/ui route map was missed, so it still claimed `require_ui_step_up` for both.
The drift ran in the direction that misleads: the document stated a WEAKER gate than the
code enforces, so a reader auditing those two admin routes would conclude they carry only a
session-window step-up when they in fact require a single-use action-bound proof.
Split out of the #1137 commit at the Lander's request so it can land on its own -- it is the
sole cause of red on three legs of PR #490, and #1137 layer 1 carries a cross-lane block that
would otherwise couple seventeen commits to work that does not exist yet.
Caught by test_every_ui_route_appears_in_the_ui_route_map, which compares the doc's stated
gate against the live route walk and reports the pair rather than guessing which is right.
…athway (BACKLOG #1137 layer 1) `ad_enabled` answered two unrelated questions on one switch: "can this engine BIND to the directory" -- which Kerberos SSO, federated OIDC and the session reconciler each depend on -- and "may a user present an AD password to our login form". Fusing them meant an operator who wanted federated login had to also expose the AD password surface, because `oidc_enabled` refuses to validate without `ad_enabled`. `ad_enabled` keeps the BIND meaning unchanged, so every dependent validator is untouched. The new `[auth].ad_password_login_enabled` gates the login pathway alone and defaults True, so the split by itself changes no behaviour. Whether that pathway should survive is layer 2 -- a product decision, deliberately not made here. The refusal sits BEFORE the bind: binding first would still probe the directory for a pathway the operator turned off, and the bind's own outcome distinguishes a real account from an absent one. WHAT THIS DELIBERATELY DOES NOT TOUCH, and the test that pins it: `_reauth_ad`. `AuthProvider` has exactly two members, so `_complete_ad_login` stamps Kerberos and OIDC logins `AD` exactly as it stamps a password login, and `reauth` dispatches to `_reauth_ad` on that stamp. Gating the re-bind on the new flag therefore removes step-up from THREE pathways while looking like it touched one. Measured: with that gate planted, the whole rest of the auth suite (58 tests) stays green and only the new guard reds -- so the mistake was invisible to the existing suite. Also corrects two stale /ui rows in SECURITY.md that still claimed `require_ui_step_up` for the admin reset routes after #1148 moved them to `require_ui_step_up_action`. KNOWN RED, blocked cross-lane: test_security_doc_drift.py requires the new field be classified, and that file is held uncommitted by another lane, so the one-line entry is requested rather than written. Not a defect in this change.
…s (BACKLOG #287) The engine charges every non-GET admin write against a per-actor budget (`AuthService.allow_admin_write`, via `api.security._enforce_admin_write_pacing` on `require_paced` / `require_step_up` / `require_step_up_action`). The console's `require_ui_*` twins charged nothing, and the console reaches the handlers IN-PROCESS -- it holds no HTTP client at all -- so a /ui write never passed through the dependency that does the charging. The product's only pacing floor was therefore absent on the one surface a human uses. This was an incomplete application of a principle the file already states rather than an oversight: `require_ui`'s docstring explains that the /ui views call the JSON handlers directly and so must re-apply the equivalent permission and throttle, and `phi=True` duly charges `allow_phi_read`. The same paragraph then declines to charge admin writes against the PHI quota -- correctly, since that quota measures PHI reads -- and no admin-write quota replaced it. PROVENANCE BEFORE SPEND, and the ordering is the security property. A first cut charged the budget in the dependency, which runs BEFORE the routes' inline `assert_same_origin`. That let a cross-origin page spend a victim's budget using the victim's SameSite cookie and throttle their console from off-origin -- and returned 429 rather than 403, announcing the request had been counted. The repo's existing cross-site sweep caught it. The dependency now asserts provenance first, so a cross-site write costs the attacker nothing, with a test naming that failure mode. DOCS CORRECTED IN THE SAME COMMIT, because they are only false BECAUSE of this code, and `test_ui_pacing_gap_wording_flips_with_the_code` is built to demand exactly that: it requires the interim gap sentence while the console lacks the charge, and requires its REMOVAL the moment the charge lands. FOUR sites carried the claim -- the honest-interim paragraph, the two rate-limit tables, and docs/CONFIGURATION.md. The rewrite holds SECURITY.md at 1994 lines so the 69 ASVS anchors below the edit do not shift. One table row was independently stale from #1148 and is fixed here too: the factory list omitted `require_step_up_action`. SCOPE, stated honestly: this closes the RATE floor, not ASVS 2.4.2's flow-timing verb. A per-request budget is not "realistic human timing" across a multi-step flow, so it does not by itself reach a pass -- #1115's body records a five-piece path and I do not speak to the rest of it. Surfaced while researching #1115, which names this gap in its severity paragraph. The parity work is #287's, a live item behind the ledger's publishing boundary -- claimed before building.
…uns publicly (BACKLOG #1124)
The 15-row browser degrade contract ships inside the wheel at `_security.py:79-156`, and that
docstring told every reader a CI guard derives all three sets from the code and fails if any
member is missing. Nothing checked it in any public checkout.
The guard was not un-wired -- it runs on every code PR (`ci.yml:917-927`) and under a bare local
pytest. But all three enforcement tests reached `_runbook_contract()` BEFORE their loops, and that
accessor skips when `docs/security/OFF-LOOPBACK-DEPLOYMENT.md` is absent -- which it is on
origin/main BY DESIGN, deny-listed to the vault. So a skip owed entirely to a private document
took the code-side half down with it. The cookie test was worst: the accessor was its first
statement, so not one line of its body ever executed.
Each guard is now two tests. The code-side one needs no runbook and passes publicly; the
runbook-comparison one skips where the runbook is absent. Splitting rather than reordering is
deliberate: a single test that skips halfway still reports `skipped`, so the code-side result
would have run and stayed invisible.
PROVED THE SPLIT GAVE THE CHECK TEETH, rather than assuming. Planted one mutation -- renamed
`X-Frame-Options` in the contract enumeration -- and ran it against both versions of the guard on
the same tree:
pre-split 19 passed, 3 skipped SILENT
post-split 1 failed caught at :589
Same mutation, opposite verdicts. Restored byte-identical afterwards, verified by SHA-256.
ADOPTS THE REPO'S EXISTING ANSWER TO THIS CLASS rather than inventing one:
`tests/test_threat_model_doc_drift.py` solved it under BACKLOG #1043 for the same stated reason --
a bare skip "is indistinguishable from a pass, a control that cannot report its own inertness"
(ADR 0158's class 2). Three properties taken from it:
* the absence is ANNOUNCED once per run as a `RunbookContractUnenforced` warning naming what
stopped enforcing and what still does. It lands in the warnings summary, which prints even
under `-q` -- and the console CI step runs `-q` with no `-rs`, so the skips were invisible;
* `MEFOR_WEBCONSOLE_RUNBOOK` repoints the comparison at a copy elsewhere;
* `MEFOR_REQUIRE_WEBCONSOLE_RUNBOOK` makes absence a hard FAILURE, so a leg that is supposed to
enforce it is fail-closed rather than best-effort.
All three verified: the warning prints under `-q`; REQUIRE with no runbook gives 3 failed; and
pointed at the real vault copy the suite runs 25 passed with ZERO skips. That last one also
establishes there is no runbook drift today -- the comparison is not merely runnable, it is green
against the actual document.
Suite goes 368 -> 371 passed with the same 3 skips, now only the runbook halves.
The `_security.py` docstring is corrected in the same commit, because it is the artifact that
misled: it now says the code-side check enforces membership HERE, and states plainly that the
runbook mirror skips in every public checkout.
This is the precondition #1124 names for any re-score of ASVS 3.7.5. The finding itself is
recorded under #1116 and is NOT re-filed here; #1124's own proposed-work list names this split.
…CKLOG #1137) `test_contextual_prefixed_settings_force_a_documented_decision` asserts `_CONTEXTUAL_REVIEWED_NON_INPUTS` is exact in BOTH directions: undecided for a candidate nobody classified, stale for a classification with no field. The new `[auth].ad_password_login_enabled` matched a contextual name marker and was unclassified, so the gate has been red on this branch since the field landed. It belongs in this set for the same reason `oidc_enabled` does: it decides which login PATHWAY exists, not whether a given request is allowed, and it is never read as a consumer or environment attribute on a request path. A user refused there is refused because the pathway is closed to everyone, not because anything about them was evaluated. BOTH DIRECTIONS OF THE GATE WERE ALREADY DEMONSTRATED, by two seats, so no fresh mutation is added: the undecided arm red on this branch from the moment the field existed until this commit, and the stale arm was proved by the other builder planting this exact line in a tree WITHOUT the field, where it correctly failed. That is why the line could not live in their commit and had to come here. NOT FOLDED INTO THE COMMIT THAT ADDS THE FIELD, deliberately. The coupling rule says a line only true because of your code belongs with that code, and the field is three commits back. Folding it would mean splitting `messagefoundry_webconsole/routes/core.py`, which carries changes for two different items, across a rewritten history -- real risk of losing content to buy bisect cleanliness on a branch that lands atomically. The ordering the rule exists to prevent, a classification landing where the field is absent, cannot occur here: the field precedes this commit on the same branch. Unblocked by the other builder committing their work, which cleared the PreToolUse collision gate. The gate blocked this edit three times and was correct every time.
…CE (BACKLOG #1167)
The comment added under #1167 gave three reasons the AES-GCM keyring candidate loop is not a
timing leak, and stopped there. Read alone it closes the wrong question: it argues the value is
not secret, when ASVS 11.2.4's verb is ABSOLUTE -- no short-circuit operations in comparisons,
calculations or returns. A short-circuit return is one whether or not what it leaks is secret.
So both statements are true at once, and the comment now says so: no vulnerability here, and no
conformance either. The cell stays partial on this loop BY DESIGN, and its record named closing
both this and the recovery-code walk as what would earn the pass -- written before either commit,
so the arithmetic is not after-the-fact.
Two things the assessor established that this corrects:
* reason 2 was not new. The record already conceded, verbatim and before my commit, that the
key fingerprint "is already carried in cleartext in the mfenc: marker, so nothing secret leaks
there" -- filed as known and correctly disclosed, never an oversight;
* the loop was never unexamined. It had been examined by the assessor before me, and again by
this comment. "Unchanged" was true of the bytes and wrong about the state.
The read-cost trade the comment describes is an OWNER call. An assessor can say what the verb
requires and cannot say the trade is worth making, so the comment no longer implies the question
is settled by the engineering argument alone.
Sequenced by the Lander so docs/SECURITY.md is resolved ONCE rather than twice against a counter moving underneath. #496 merged at 298a2bf; this is that moment. TWO CONFLICTS, resolved per-row rather than per-file, because each row's correct value comes from a different side: * `require_step_up` counter -- took main's 27 and then RE-DERIVED rather than trusting either number. My branch said 24 (#1148 moved two routes off), main said 27 (24 + #1184's three). The drift gates compute from the live route walk, so 83 passing IS the derivation. * PHI-reads row -- took MAIN's "1 further /ui GET". Mine said 3 and is stale; #1184 derived it down and my branch predates that. * Admin-writes row -- took MINE. Main still says "JSON API only" and "no /ui route charges it", both false once #287 is in the tree, and main's version also omits `require_step_up_action` which #1148 already landed. * crypto.py -- kept my #1167 correction (the IMPACT-vs-CONFORMANCE paragraph). Not on main yet; nothing on main contradicts it. Verified after resolving: 83 doc-gate tests, then 163 passed / 3 skipped across the auth, posture, AD-split, pacing and canary suites. ruff clean. Note for whoever reads the diff: most of the 26 both-sides files are my own work returning through #490's squash, not genuine divergence. Only two files actually conflicted.
…ut-open ADR 0165 PAIRING, AUTHORED BY THE LANDER. A builder PR satisfies the ledger gate with a paired ledger commit authored by the Dispatcher or the Lander and carried ON THE PR BRANCH; a builder may not author ledger content. This PR shipped three items' worth of code across 14 files and touched no ledger file. #287 CANNOT BE DISPOSITIONED IN THIS FILE AND THAT IS RECORDED RATHER THAN LEFT BLANK. It is below the public floor, so it belongs to the maintainer-internal ledger this file is a published baseline of -- it appears in neither docs/BACKLOG.md nor the archive. Its work is named inside #1137's note so its absence is not read later as an oversight. The docs/SECURITY.md admin-writes row -- the one merge conflict resolved in the BUILDER's favour rather than main's -- carries its reasoning for the same reason: main described the JSON API alone and stated no /ui route charges the floor, both falsified by this work, and main also omitted require_step_up_action which #1148 had already landed. A reader who finds main's row overwritten deserves to know why without reconstructing it. Both notes are shipped-but-open and neither moves a banner. Closure needs a witness to the code and the Lander is not one. Verified: main 322 items, branch 322, ADDED none, LOST none, status gate reads 558 items each declaring exactly one status.
… (BACKLOG #287) PR #497 went red on both required legs with "webconsole seam drift: the engine/console contract changed but ENGINE_UI_SEAM did not". Real, not a flake -- it reproduced locally first try. CAUSE IS #287. Charging `allow_admin_write` inside `require_ui` changes the contract the console depends on, and the seam is a content hash over that contract. #496 had already moved it to 0beb716d for #1184, so my branch carried a seam that was correct for main and stale for my own tree. 0beb716d882ba6b5 -> ccb7e71700c38031 BOTH SIDES IN THIS COMMIT, which the guard demands and explains why: the console ships as a separate wheel holding exactly ONE accepted seam (BACKLOG #279), so a forgotten console-side update is not a warning -- a deploying site gets a hard startup refusal. Engine side written by `scripts/webconsole_seam_snapshot.py --write`; console side set by hand as the tool instructs. I CHECKED THE WRONG SEAM EARLIER AND SHOULD SAY SO. When the Dispatcher asked whether #1146 would collide with #1184's bump, I measured `_ui_seam.py` for `CoreHandlers` changes, found none, and reported no seam impact. That was true of the question asked and I generalised it: this snapshot hashes more of the contract than the handler protocol, so "my change does not touch the seam" was answered against a narrower artifact than the one that fails. Verified: 116 passed / 3 skipped across the seam, pacing, canary and doc-drift suites; ruff clean.
…aude/builder-1-d8e2c4
… not survive (BACKLOG #1196) The 2026-08-20 research pass on #1196 narrowed an honest ASVS 16.2.2 pass to one buildable control: a default-ON, peer-free startup gate that reads the host's own time-discipline state and refuses. The dispatcher ordered the pass's own eighth subject measured before any gate was written. Measuring it refuted the control. Three findings, each with the control that makes it mean something: - The container worry is real for one candidate probe only. The runtime image installs tini and curl over python:slim, so there is no systemd and no timedatectl; a service-manager probe refuses on every container start. An ntp_adjtime syscall probe needs only glibc and works unprivileged. - The gate's unstated load-bearing assumption holds. A time namespace does not change what ntp_adjtime reports: with a 100000s monotonic offset applied, the namespace demonstrably rewrote CLOCK_MONOTONIC and the discipline probe read identically. The first control failed silently (reading the host's procfs from inside the namespace, because --mount-proc needs privilege) and would have produced a vacuous negative; moving it to a syscall is what made it fire. - The Windows surface, which nobody had measured, is where the control dies. W32Time runs while the clock free-runs off the CMOS clock with no reference, so a default-ON refusing gate would refuse to start the engine on the documented primary deployment surface in the shipped enforcing posture. A probe checking service state rather than sync state calls that box healthy. Two surfaces on the same physical clock disagree, so a gate must name which clock it asserts about. This removes the offered control; it does not assert the item's "cannot honestly reach pass" conclusion, and the memo states that gap rather than closing it. The options the pass ruled out stay ruled out. No engine code changes. Also records a link the pass understates: the validator chain is three deep, not two, so time_sync_fail_closed is not a single-knob flip either.
…ate (BACKLOG #1196) The memo dated its measurements and named no commit. A date cannot be checked; a commit can, and fails loudly when wrong. States the engine tree the readings were taken against, and says in place why the citation style is symbol-for-where plus commit-for-when: a line number rots silently, a symbol does not. Verified with both controls, which is the point of the change: 00cfbc8 resolves, deadbeef1 does not. Without the negative control, "it resolves" is indistinguishable from a command that resolves anything.
…6.2.2 memo (BACKLOG #1196) Seven claims in the memo were verified and then handed to independent skeptics told to refute them. All seven held and none was refuted, but two carried real defects in how the memo cited its sources. The "primary deployment" citation named the wrong files. docs/SERVICE.md and CLAUDE.md section 2 document the NSSM service but neither contains the word "primary" -- measured zero, against a firing control of 17 for NSSM in the same file. The word is in docs/SYSTEM-REQUIREMENTS.md, which grades Windows Server 2022/2025 the primary supported and serviced platform. The claim was true against a source the memo did not name, so the fix is a citation, not a softening. Section 3 also said "the deployment path", exclusive, which overstates the doc set: DEPLOYMENT.md presents the container image as a complement rather than a replacement, and Linux is supported but operator-serviced. Narrowed to primary and only-serviced. This does not rescue the control, which fails on the serviced Windows path and the supported Linux path alike. Restores a dropped qualifier that cuts in the memo's own favour: the 2026-08-10 ruling against a default peer explicitly did NOT rule on the refusing-gate alternative, sending it back for re-specification. Without it a reader could infer the gate measured here had already been declined. It had not, which is why measuring it was legitimate work rather than re-litigation.
… (BACKLOG #1179) make_self_signed wrapped every name in x509.DNSName, including IP literals. Hostname verification for an IP-literal URL matches only against an iPAddress entry; a DNS entry spelling the same characters does not satisfy it. So a certificate minted for the engine's own default bind could never verify. That default is not hypothetical, which is what makes this load-bearing rather than pedantic: [api].host binds 127.0.0.1, and all three shipped first-party clients default to http://127.0.0.1:8765 -- ide/src/cli.ts, tray/config.py and apiclient/client.py. A DNS-only certificate verifies against none of them. Fixes both coupled sites. read_cert_facts collected DNSName values only, so repairing the writer alone would have left `cert inventory` silently under-reporting a certificate whose names are all IP addresses -- the exact case the writer fix creates. Classification uses ipaddress.ip_address rather than a regex because it is the same parser the verifier compares against. Red-first, and each half proven load-bearing by separate mutation: reverting the writer to DNS-only reds the new test alone; reverting the reader to get_values_for_type(DNSName) reds it alone. The mutation asserted its anchor was present before patching, so an anchor miss would raise rather than report a comfortable pass. The test also corrects an assertion of its own that could not have witnessed this: the CLI's --json output echoes the INPUT names, so asserting on it passes whatever the certificate contains. It reads the minted file back instead. Base read against origin/main e03e8e0. Gates: ruff format, ruff check, mypy strict, 43 tests across test_cert_cli and test_cert_expiry.
…lumns only" was a false cannot-clause (BACKLOG #1163) The key-usage scope said this DEK is the confidentiality key for the at-rest store columns "only". The .mfbak DR archive seals its chunks under the SAME resolved store DEK (store/backup_codec.py, which says so in its own header), and an archive carries the SQLite snapshot PLUS the config bundle -- so the real blast radius is wider than the ciphered columns. A wrong negative half is worse than no negative half, because the "cannot" clause is the sentence an operator acts on. Two consequences, and they point in opposite directions, so both are now stated rather than one: a crypto-erase by discarding the DEK reaches the backups too -- MORE complete than the destruction bullet used to promise a surviving archive is NOT a DEK-independent recovery path -- LESS of a safety net than the old wording implied The escrow line said losing a key strands "the rows last written under it"; it strands every archive sealed under it as well. Corrected in place, with the earlier wording named so a reader who acted on it can tell what changed. DELIBERATELY NOT TOUCHED: the Transit-mode row (cipher_provider=vault_transit) makes a similar "only" claim, and I could not establish whether it is wrong. backup_codec requires a raw 32-byte key and neither DR file mentions transit or cipher_provider at all, while Transit mode's premise is that the plaintext DEK never enters engine heap. That interaction is unmeasured, so it is reported rather than edited -- asserting either way would be the unmeasured completeness claim this item exists to remove. Base read against origin/main e03e8e0. Gates: 106 doc-guard tests across test_key_usage_scope_inventory, test_crypto_inventory_doc, test_communications_inventory and test_secret_rotation_inventory.
…age is opened (BACKLOG #1187) ASVS 14.2.6 asks that sensitive data be masked in display and revealed only by a specific act. The field-authorization seam had one decision where it needs two: permission said the caller MAY see summaries, and the caller then got every complete identifier on every row. Authorization is not reveal. A permitted `summary` is now display-masked unless the call names it in `revealed`: MRN 100001 - DOE, JANE -> MRN ****0001 - D**, J** Labels, separator and the name comma survive so a row still reads as a row; an unrecognised part masks whole, because an unknown shape is where guessing wrong leaks. OPENING ONE MESSAGE IS THE REVEAL ACT. The list, dead-letter and content-search surfaces mask, because those are where complete identifiers could be read off a screen opened for another reason. The detail route reveals, because it is deliberate, per-record, and already audited via record_view plus the tamper-evident chain. The item contemplates exactly this reading -- "a per-row click is the specific-view act" -- so it needs no new UI and no query parameter. THE STICKY ANTI-PATTERN IS IMPOSSIBLE, NOT MERELY TESTED AGAINST. `revealed` is a keyword-only parameter with no stored counterpart anywhere, so a reveal cannot outlive the call that passed it. A session-wide or module-level reveal would be a STATUS rather than an ACT and would render every row for as long as it was set -- which is what diagnostics.py's module-global `_reveal` does, so that precedent's mechanism was copied and its scope deliberately was not. A signature test fails if a refactor ever parks it on the identity or a default. Masking does not degrade search: matching happens inside the store over stored columns, before redaction, so a content search still finds a known patient and returns a masked projection. Six existing tests changed contract deliberately, each keeping its own purpose -- the positive controls that prove release works now pass `revealed` so they still observe a complete value and can still tell a working gate from a mask. One was a fail-closed test: its primary assertion (a route that forgets the call returns nulls) is untouched and still passes. Verified by two mutations reddening DISTINCT sets, each asserting its anchor before patching: neutering the mask reds 5 tests, ignoring `revealed` reds 3. Gates: ruff format, ruff check, mypy strict, 129 tests across seven suites. Base read against origin/main e03e8e0. NOTE for whoever re-scores this cell: the item warns that the natural name for a mask helper collides with the cell's own absence pattern, so a re-score must assert on the RESPONSE, never on the pattern firing. `metadata` carries the same ingest-derived PHI and is named in MASKED_UNTIL_REVEALED's docstring as the next candidate, not silently included.
Authored by the lander per ADR 0165. The CI ledger check requires a BACKLOG.md update from a PR citing an item, and a builder may not write ledger content. Not a closure, and the note says so in place. Subject 4 wants its own review per the item, and the seam work beyond subjects 1 and 2 is untouched. The note records WHY the two subjects shipped together rather than leaving it to be re-derived: the mask alone would have regressed the surface whose job is showing the operator the message. It also records the precedent that was deliberately not copied whole -- diagnostics.py's mask-then-reveal mechanism is right, its module-global scope is what the item forecloses. No banner is written here. The item is claimed and its banner is set elsewhere. Read against engine origin/main 66220bb.
wshallwshall
enabled auto-merge (squash)
August 22, 2026 15:30
Follow-on defect from the mask in bb6afb3, found on resuming rather than when it shipped. count_exposed counted any non-empty PHI property, and a masked "MRN ****0001 - D**, J**" is non-empty -- so every list row began registering as a PHI exposure. At list scale that is a false positive per row, and it buries the one event the audit exists for: the record somebody actually opened. The mask is RECORDED by the masker, never inferred at the counting site. A real summary may legitimately contain the mask characters, so "does this look masked" is not a decidable question there. PhiGatedModel gains _phi_masked; an unmarked property still counts as a real exposure, so forgetting to mark one OVER-reports rather than hiding a disclosure. DROPPING THE ROW ENTIRELY WOULD HAVE BEEN WRONG, which is why this is not simply "stop counting masked". The coalescer exists so a scripted bulk fetch cannot harvest the patient census unaudited; if a masked-only list emitted nothing, a 5,000-row scrape would be indistinguishable from no request at all. So the audit now carries BOTH counts and fires on either. "count" keeps its documented meaning -- summaries the caller could READ -- so no existing row is silently re-scoped, and "masked" is additive. Verified by three mutations, each asserting its anchor before patching and each reddening a distinct set: the counter ignoring the mask record, the masker recording nothing, and the audit firing only on readable exposures. A fourth -- the window not accumulating masked -- reds the coalescing pair, which is what proves the accumulation is real and not per-request. One test assertion of mine was wrong and the code was right: opening a message audits [detail, *outbox, *events], and the nested delivery and event rows carry their own last_error / detail PHI that was never masked. The open discloses 3 objects, not 1. Asserted exactly rather than "> 0", because a loose assertion would pass if the summary reveal silently stopped counting and only the nested rows carried it. Gates: ruff format, ruff check, mypy strict 267 files, 132 tests across seven suites. Base read against origin/main at the tip carrying #520.
wshallwshall
added a commit
that referenced
this pull request
Aug 22, 2026
…d only a skipped leg hid it NOT A DEFECT IN THIS BRANCH. #514 (BACKLOG #1187, merged as fdd89b4) added a `masked` key to the summary_access audit detail and updated the coalescer, but not the Postgres or SQL Server twins of the census test. This branch merely ran the legs that hid it. api/app.py _emit, on main: detail=json.dumps({"count": count, "masked": masked, "window_start": hour*3600}) tests/test_postgres_store.py:3409 asserted {"count": 5, "window_start": 0} tests/test_sqlserver_store.py:3435 asserted the same MAIN IS NOT RED, IT IS BLIND -- which is the worse half and the reason this needs saying. Both DB legs were SKIPPED on main's CI at fdd89b4 and at ae72f58 before it, so main reports green while carrying two failing tests. They run on a pull request, so the first PR to touch anything after #514 inherits three red required- adjacent legs it did not cause. This one did: postgres store, sql server 2022 and sql server 2025. WHY THE LEG THAT DID RUN STAYED GREEN, and it is not luck. The SQLite twin at tests/test_api.py:587 asserts `'"count": 5' in detail` -- a SUBSTRING containment check, which cannot see an added key. The two store twins assert DICT EQUALITY, which can. So the weaker assertion survived a change the stronger ones caught, and the stronger ones were the ones not being run. VERIFIED AT RUNTIME, not read off the source. Driving the real _SummaryAuditCoalescer end to end against a live store: ACTUAL detail: {'count': 5, 'masked': 0, 'window_start': 0} OLD assertion holds : False FIXED assertion holds: True The coalescer is backend-agnostic -- it calls store.record_audit and nothing in the shape is per-backend -- so the SQLite run is evidence for the Postgres and SQL Server paths, which is exactly the substitution that makes this checkable here. WHAT THIS RUN DOES NOT PROVE, stated because a local pass would be misleading: this repo SILENTLY SKIPS both DB legs locally. Measured on this tree, 152 skipped and zero run. CI is the only authority for the two lines themselves. LEFT ALONE DELIBERATELY: test_api.py:587's substring assertion. Tightening it to equality is the right change and it belongs to #1187's owner, not to a branch recovering unrelated TLS work.
wshallwshall
added a commit
that referenced
this pull request
Aug 22, 2026
…ot on the sources they assert against Root cause of PR #525's three red DB legs. Found by the liaison; every claim below re-verified against origin/main by this seat before filing. NOT "MAIN SKIPS THE LEGS". That is deliberate and filing it would be filing a billed-minutes decision: ci.yml:1521 is `schedule || workflow_dispatch || serverdb=='true'` and its own comment names the no-per-merge-run guard. The defect is one level down, in the PR arm -- the only arm that can catch a change BEFORE it lands -- whose producer set has a hole. THE HOLE. ci.yml:1358's alternation lists messagefoundry/store/, three pipeline modules, config/(settings|wiring), a transports list, a tests/test_(...) list and ci.yml. It does not list messagefoundry/api/. THE INVARIANT IS WRITTEN ONE LEVEL TOO NARROW. ci.yml:1348 requires the alternation list "every file the sqlserver-store / postgres-store pytest steps below run" -- the TEST files. Not the SOURCE those tests assert against. test_postgres_store.py asserts on a dict built by api/app.py::_emit, so the rule cannot see this class. IT HAS ALREADY FIRED. #514 touched api/app.py, serverdb evaluated false, the legs never ran on its PR, push never runs them, and it merged green leaving two store tests asserting a stale shape. PR #525 selected the legs and inherited three reds it did not cause. SECOND WEAKNESS NAMED ON THE ROW, not folded into the fix: the SQLite twin at test_api.py:587 asserts substring containment and is blind to an added key. The assertion strong enough to catch the change was the one not being run; the one being run was too weak to notice. Tightening it belongs to #1187's owner. Severity bounded in the same line that causes it -- the schedule arm runs these legs nightly, so the blind window is about a day. What it does not do is stop the merge. Number allocated and claimed through scripts/coord. Backlog parser clean at 568 items, each declaring exactly one status.
wshallwshall
added a commit
that referenced
this pull request
Aug 22, 2026
… repair main's census tests, file #1319 and #1322 (#525) * backlog: file #1319 -- the demote-teardown timing assert cannot discriminate its own mutation under load Recovered from a session that was measuring repo health when its Claude account was cancelled mid-command. It had found the CI failure and never reported it. THE OBSERVED FAILURE. tests/test_adr0157_demote_teardown.py:106 asserts elapsed < 1.5 and reported 1.92s on main at 2d1c89e, CI run 32580332076, leg test (ubuntu-latest, py3.14). That commit was docs-only, so nothing in the tree under test moved the number. WHAT MAKES IT MORE THAN A FLAKE, and it is measured rather than argued. Four implementations, 200 sources at 0.4s under a 3.0s budget: shipped, max-shaped 0.40s 200/200 finished timing PASS MUTANT sequential 3.01s 7/200 finished timing FAIL MUTANT semaphore(8) 3.00s 56/200 finished timing FAIL MUTANT semaphore(64) 1.63s 200/200 finished timing FAIL The semaphore(64) row is the item. Every source finishes, so all(stop_finished) passes and _pending_source_stops is empty -- the wall-clock bound is the ONLY assertion that catches it. So the threshold cannot simply be raised: the discriminating band is (0.40, 1.63) and the bound sits at 1.50. CI measured 1.92s for the SHIPPED implementation, which is above the 1.63s the mutant produces. On that run the correct code was slower than the defect the assertion exists to detect, so no threshold in the band could have separated them. That is the same class as #1290 -- an instrument that goes silent on its own subject exactly when it is under stress. LOCAL CONTROL. Seven consecutive runs on Windows while the full suite ran concurrently: 0.40 0.41 0.40 0.41 0.41 0.41 0.41. The floor is the 0.4s sleep plus ~10ms, so CI's 1.92s is runner latency, not the tree. The item records the fix direction (_Source already carries stop_started, and concurrency width separates all four rows with no timing dependence) and states plainly what is NOT established: this is at least one observed failure, with no rate measured and no same-head re-run recorded. Number allocated through scripts/coord/alloc.ps1. Backlog parser clean at 567 items, each declaring exactly one status. * fix(tls): contexts the engine built but never named inherited an unasserted suite list (BACKLOG #1317) Closes the first-party half of the residual #1317 recorded as open. That item covered the operator-facing knob and the shipped context builder; it did not enumerate every SSLContext the engine constructs. RECOVERED, NOT REDONE. This was built by a workflow whose parent session's Claude account was cancelled on 2026-08-22 at 16:56Z while its full-suite run was in flight. The tree was left uncommitted in a locked worktree, three commits behind. It is replayed onto current main -- which had moved two commits, including #1317's own build -- and re-verified there. The claim on #1317 was taken over from that dead seat via claim.ps1 -Force, recorded in claims/.history. THE DEFECT IS INHERITANCE WITHOUT ASSERTION, not a missing context. Every hop below always had a TLS context; the engine simply never held a reference to it, so harden_cipher_suites never ran on it. Sites now asserted: auth/oidc_http.py identity provider token + JWKS hop logging_setup.py syslog TLS forwarder, BOTH arms incl tls_verify=False store/postgres.py pinned-CA and verification-disabled arms transports/soap.py mutual-TLS opener transports/rest.py shared REST/FHIR/DICOMweb opener family transports/fhir.py digest-auth rebuilt opener pipeline/alert_sinks.py webhook opener IT ASSERTS ON URLLIB'S OWN CONTEXT RATHER THAN SUBSTITUTING ONE, and that is load-bearing. Measured on CPython 3.14.6 / OpenSSL 3.5.7: urllib's context adds set_alpn_protocols(["http/1.1"]) and post_handshake_auth=True, which a hand-built ssl.create_default_context() has neither of. Passing a look-alike would silently drop ALPN and TLS 1.3 post-handshake auth from every default HTTP-family hop -- a handshake change on a control whose whole point is to change nothing about the connection. build_asserted_https_handler reads the handler's private _context and FAILS CLOSED if absent, rather than shrugging and reporting success forever. TWO FIRST-PARTY SITES ARE DELIBERATELY NOT ASSERTED, each with its reason in the code so a later reader does not "fix" them. The TLS floor probe builds ALL:@SECLEVEL=0 on purpose -- asserting there would empty the offer and turn a probe that can fail into one that cannot. And the Postgres ssl=True default arm hands asyncpg the job, so no engine object exists to assert on; grading a look-alike would grade an object the connection never uses. THE INSTRUMENT THAT GUARDED THIS RESIDUAL COULD NOT SEE IT, which is why the new test file is the other half rather than more of the same. test_tls_policy.py derives its call-site list from the presence of harden_kex_groups( in a file and its guard is `if kex and assertions < len(kex)`, so a file with ZERO kex-pin sites is skipped entirely. Every site above called neither helper, so the scan passed over all of them in silence. test_tls_cipher_assertion_sites.py names each construction and proves the assertion is REACHED, not merely present: contexts that are built are checked by patching _is_forward_secret to report every suite weak and requiring a ValueError naming that site's connector; contexts handed through an opener are checked by requiring the context the opener will actually use to be the SAME OBJECT the assertion ran on. It carries its own positive control, so a test seeing no raise reports a missing call rather than an inert instrument. VERIFICATION, stated exactly. ruff check, ruff format --check and mypy --strict all clean. 141 passed across test_tls_cipher_assertion_sites, test_tls_policy and test_soap_wssecurity -- run against current main, so the work is confirmed compatible with #1317's newer three-property harden_cipher_suites (forward secrecy plus encryption plus peer authentication), which landed after it was written. THE FULL SUITE IS STILL RUNNING and its result is NOT in this message; it is committed now rather than held because the pool that killed the first attempt is running hot again, and a branch commit is recoverable where an uncommitted worktree is not. CI is the authority. * test(store): the summary_access census assertion is stale on main, and only a skipped leg hid it NOT A DEFECT IN THIS BRANCH. #514 (BACKLOG #1187, merged as fdd89b4) added a `masked` key to the summary_access audit detail and updated the coalescer, but not the Postgres or SQL Server twins of the census test. This branch merely ran the legs that hid it. api/app.py _emit, on main: detail=json.dumps({"count": count, "masked": masked, "window_start": hour*3600}) tests/test_postgres_store.py:3409 asserted {"count": 5, "window_start": 0} tests/test_sqlserver_store.py:3435 asserted the same MAIN IS NOT RED, IT IS BLIND -- which is the worse half and the reason this needs saying. Both DB legs were SKIPPED on main's CI at fdd89b4 and at ae72f58 before it, so main reports green while carrying two failing tests. They run on a pull request, so the first PR to touch anything after #514 inherits three red required- adjacent legs it did not cause. This one did: postgres store, sql server 2022 and sql server 2025. WHY THE LEG THAT DID RUN STAYED GREEN, and it is not luck. The SQLite twin at tests/test_api.py:587 asserts `'"count": 5' in detail` -- a SUBSTRING containment check, which cannot see an added key. The two store twins assert DICT EQUALITY, which can. So the weaker assertion survived a change the stronger ones caught, and the stronger ones were the ones not being run. VERIFIED AT RUNTIME, not read off the source. Driving the real _SummaryAuditCoalescer end to end against a live store: ACTUAL detail: {'count': 5, 'masked': 0, 'window_start': 0} OLD assertion holds : False FIXED assertion holds: True The coalescer is backend-agnostic -- it calls store.record_audit and nothing in the shape is per-backend -- so the SQLite run is evidence for the Postgres and SQL Server paths, which is exactly the substitution that makes this checkable here. WHAT THIS RUN DOES NOT PROVE, stated because a local pass would be misleading: this repo SILENTLY SKIPS both DB legs locally. Measured on this tree, 152 skipped and zero run. CI is the only authority for the two lines themselves. LEFT ALONE DELIBERATELY: test_api.py:587's substring assertion. Tightening it to equality is the right change and it belongs to #1187's owner, not to a branch recovering unrelated TLS work. * backlog: file #1322 -- the serverdb path gate selects on the tests, not on the sources they assert against Root cause of PR #525's three red DB legs. Found by the liaison; every claim below re-verified against origin/main by this seat before filing. NOT "MAIN SKIPS THE LEGS". That is deliberate and filing it would be filing a billed-minutes decision: ci.yml:1521 is `schedule || workflow_dispatch || serverdb=='true'` and its own comment names the no-per-merge-run guard. The defect is one level down, in the PR arm -- the only arm that can catch a change BEFORE it lands -- whose producer set has a hole. THE HOLE. ci.yml:1358's alternation lists messagefoundry/store/, three pipeline modules, config/(settings|wiring), a transports list, a tests/test_(...) list and ci.yml. It does not list messagefoundry/api/. THE INVARIANT IS WRITTEN ONE LEVEL TOO NARROW. ci.yml:1348 requires the alternation list "every file the sqlserver-store / postgres-store pytest steps below run" -- the TEST files. Not the SOURCE those tests assert against. test_postgres_store.py asserts on a dict built by api/app.py::_emit, so the rule cannot see this class. IT HAS ALREADY FIRED. #514 touched api/app.py, serverdb evaluated false, the legs never ran on its PR, push never runs them, and it merged green leaving two store tests asserting a stale shape. PR #525 selected the legs and inherited three reds it did not cause. SECOND WEAKNESS NAMED ON THE ROW, not folded into the fix: the SQLite twin at test_api.py:587 asserts substring containment and is blind to an added key. The assertion strong enough to catch the change was the one not being run; the one being run was too weak to notice. Tightening it belongs to #1187's owner. Severity bounded in the same line that causes it -- the schedule arm runs these legs nightly, so the blind window is about a day. What it does not do is stop the merge. Number allocated and claimed through scripts/coord. Backlog parser clean at 568 items, each declaring exactly one status.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builder 1's work, routed by this seat. The pairing commit is mine per ADR 0165; the feature is theirs.
Why two subjects shipped together
The item's subjects 1 (server mask) and 2 (detail-page reveal) are coupled one way: shipping the mask alone leaves the console showing masked summaries with no way to unmask -- a regression on the surface whose stated job is showing the operator the message. Landing them as a pair ships nothing unusable.
The reveal is per-message and never sticky
A live mask-then-reveal precedent exists in
diagnostics.pyand its mechanism is right:shown = values if _reveal else redacted. Its scope is not -- the reveal is a module-global flag, one switch per process, and the item forecloses exactly that: a session-wide sticky reveal "converts the reveal from an act back into a status".So the application was copied and the scope was not. That distinction is recorded in the ledger note rather than left to be re-derived by whoever reads
diagnostics.pynext and sees a working precedent.What the seam had to change
Before this,
redact_unauthorizedcomputed allowed-by-permission, nulled the rest, then calledrelease_phi(allowed)-- so authorised meant complete. ASVS 14.2.6 wants masked-by-default with reveal on a specific act, which makes authorisation and reveal two different decisions. The seam only had one.Not a closure
Subject 4 -- the console reveal -- the item says wants its own review. The seam work beyond these two subjects is untouched, and the ledger note says so in place.
Verification
api/app.py,api/field_authz.py, and five test modulesdocs/BACKLOG.mdPer CLAUDE.md section 9 this is PHI-surface work: the change is what a console operator sees by default, and the reveal is auditable per message rather than per session.