backlog: file #1097, the worktree gate's interpreter recursion misses PowerShell flag prefixes (live fail-open) - #282
Merged
Conversation
…om every rule
Found by the adversarial verification of a candidate fix for #1086, and separated from
that candidate by measuring both against the committed gate: this half is PRE-EXISTING
and LIVE.
worktree_gate.ps1:285 recurses into an interpreter argument for a FIXED list of
spellings. PowerShell accepts any unambiguous PREFIX, so these run code and are not on
the list:
pwsh -NoProfile -Com "..." -> runs
pwsh -NoProfile -Comm "..." -> runs
pwsh -NoProfile -Cm "..." -> not recognized, so the family is bounded
Their argument is therefore blanked as an ordinary quoted span and its contents are
invisible to EVERY rule, not only 3c. Whatever a session may not do directly, it may do
inside one of these.
THE SINGLE-LINE FORM ALREADY ALLOWS TODAY. The here-string and multi-line forms still
deny, but only BY ACCIDENT -- every line of a multi-line span reaches the scanner raw, so
the disarm is caught as ordinary text rather than because the interpreter was recognised.
That accident is load-bearing and trivially removed: the #1086 candidate blanked message
bodies correctly and in doing so converted the accident into a clean fail-open across all
forms. Which is why the item says the fix must be verified against the gate as it will
ship, not as it is.
AND THE OBVIOUS FIX IS INSUFFICIENT, MEASURED: adding the three spellings to the list
closes every form on the committed gate, but on a message-blanking gate it closes only
the single-line one and leaves the here-string route open behind it.
The item asks for prefix recognition bounded to the shells that actually accept prefixes
-- pwsh and powershell do, bash and sh do not -- and for the test to enumerate the FAMILY
from -C through -Command plus a bounding negative, because a test pinning one spelling
cannot see the next one. That is the same defect as the third verifier accepting the
candidate: its corpus contained no such payload, so its evidence could not see the class.
…es it not re-derives it A candidate was built and put through three adversarial verifiers today. NOT READY, for exactly one defect. Recording the correction in the item rather than leaving it in a workflow synthesis, because the next session should spend its budget verifying a one-line change, not rediscovering which line. WHAT WAS RIGHT, both pinned by killed mutants and not to be undone: key on the CONSUMING FLAG rather than the delimiter, and run the message-blanking AFTER the interpreter recursion. The blank-before-recursion mutant is killed by seven to nine interpreter payloads in all three mutation runs. WHAT WAS WRONG, and it is the whole rejection: the flag classifier matched an OPEN-ENDED letter cluster. PowerShell accepts unambiguous parameter PREFIXES, so -Com and -Comm are working spellings of -Command that contain no lowercase c or e and end in m. They were classified as MESSAGE and their code was blanked -- four rows went from DENY to ALLOW across rules 3, 3c and 3d. THE REPLACEMENT IS MEASURED, not proposed: a closed set of git message flags. Two verifiers independently confirmed it returns those four rows to DENY, keeps all eleven ALLOW cases and every narrowness case, and produces ZERO other verdict movement. TWO THINGS THE CANDIDATE DID NOT DISCLOSE are now in the item. A mutant relaxing the separator survives the entire suite -- found independently by two verifiers, same hash -- so that quantifier is load-bearing and no test says so. And two glued spellings of this item's OWN defect remain refused, which belongs in the disclosure list. AND THE READING RULE THE ROUND EARNED: two of three verifiers found the fail-open; the THIRD ACCEPTED, because its corpus contained no such payload. Its evidence could not see the class it was approving -- #1000's shape occurring inside the verification of a #1000 fix. Do not read "one verifier accepted" as evidence; read what its corpus covered. Also records that this item and #1097 are separable and must both be fixed: #1097 is the pre-existing interpreter-recursion gap, and this candidate would have converted #1097's accidental here-string denial into a clean fail-open. Verify either against the gate as it will SHIP, not as it is.
…doc-message # Conflicts: # docs/BACKLOG.md
wshallwshall
added a commit
that referenced
this pull request
Aug 7, 2026
Plain APPEND conflict in docs/BACKLOG.md: my side adds ## 1096., main's side adds ## 1097. (landed via PR #282). Neither touches the other's text and the overlapping-heading-number count between the sides is ZERO, measured. Kept both, ascending: 1096 then 1097. THIS CONFLICT WAS ENGINEERED TO LAND HERE RATHER THAN ON #282. Both PRs append to the same file tail, so whichever merged second was always going to conflict. #282 was armed FIRST and deliberately: the ledger gate keys on which worktree ALLOCATED a number, #1097 belongs to nice-payne-4dcee0, and that session has now stopped -- so a conflict landing on #282 would have been unresolvable by anyone still running. #1096 is allocated to THIS worktree, so the conflict is resolvable here. Ordering was the whole point; it was not luck. Verified after resolution: ## 1094. x1, ## 1095. x1, ## 1096. x1, ## 1097. x1, conflict markers 0, CRLF preserved. My #1084 reconciliation section survived intact, as did #1096's table row and the census line reading 109. Live table still 109 rows with contiguous ranks. backlog_status_check.py --min-items 150 OK at 368 items, each declaring exactly one status. LEDGER DRIFT NOTE, stated rather than quietly absorbed: #1097 arrived with no table row, so the gap between parse_items' open count and the census table grows by one. That is the pre-existing drift already recorded in #1096's own commit (52 open items with no row, 11 rows naming a closed item, the two sets cancelling on a total). This merge does not fix it and does not make it worse by any mechanism of mine -- it simply carries it forward, which is what a correct filing does. The reconcile pass remains outstanding and needs 52 items scored.
wshallwshall
added a commit
that referenced
this pull request
Aug 12, 2026
…surface (BACKLOG #1220) (#346) * feat(seam): discover the webconsole seam surface instead of enumerating it (BACKLOG #1220) Layer 1 of #1220, and it is INERT: nothing consumes this yet. The generator still reads its five hand-maintained tuples and ENGINE_UI_SEAM is still the hand-chosen integer 20. Separating coverage from derivation means a bisect can tell which one broke something. WHY THIS AND NOT A HASH OVER THE EXISTING LISTS. #1220's ruling is that the hash input must be DISCOVERED, never enumerated, because a content hash over a hand-maintained list stops colliding while still failing to move when an unlisted model changes -- and a hash LOOKS like it covers content, so the next reader trusts it more than the list it replaced. Measured at ebf4882, three of the five tuples had already drifted: _API_MODELS_DTOS 30 curated, 55 reachable. 7 DTOs the console imports directly are absent (UploadedFileList among them); the other 18 are reachable only by NESTING, and _dto_fields records field names ONE level deep with no recursion, so a nested model's field set is absent from the contract entirely. _API_SECURITY_SYMBOLS missing client_ip and enforce_phi_read_pacing, carrying five names the console no longer imports. That is why _ui_seam.py asserts the console imports six symbols directly -- false for five of six. _AUTH_SERVICE_METHODS AuthService.has_action_step_up is called at _auth.py:620 and absent. Six of the seven additions are PROPERTIES, on which inspect.signature raises, so the curated list held methods only because the renderer could not do more -- the instrument's limitation had silently defined what counted as contract. The proof the defect is real rather than theoretical is in the history: 40a4d5d added a REQUIRED scope field to UploadedFileList, which pages/uploaded_logs.py renders unconditionally, and touched no seam file across 17 changed files. The seam moved later, by hand, in review. CALIBRATION. _APP_STATE_ATTRS was the one tuple with zero drift in either direction, and the two-sided rule (console reads intersected with engine writes, plus console writes) reproduces its eight names EXACTLY. A walk that merely returned "more" would be consistent with measuring the wrong thing; reproducing the un-drifted list is the evidence that it measures the contract. The AuthService rule is annotation-driven -- attribute access on a receiver annotated AuthService or assigned from get_auth() -- rather than a bare name intersection. The bare rule yields 38 names, ~11 of which are present only because they collide with AdminHandlers field names, and a gate that moves for methods the console never calls is one people stop reading. Annotation-driven yields 34: the 27 curated plus 7 verified real, nothing dropped. FAIL LOUD, never skip. A star import, an alias escaping attribute position, a dynamic getattr on a DTO module, or an unresolvable model raises SeamDiscoveryError. A silent skip would recreate the enumeration blind spot inside the walk's control flow, which is strictly worse than the tuple it replaces -- a 30-line tuple was at least reviewable. Every loud case measures zero occurrences today, so the guard costs nothing now and its first firing is a genuine new idiom. The closure deliberately crosses module boundaries: a rendered DTO that exposes a model defined elsewhere still ships that model's field set across the seam. Discovery runs in the generator and the test only -- messagefoundry/ must not import the console or scripts/, and a seam computed at import time would make every proof condition pass vacuously. Verified: ruff check + format clean, mypy strict clean, 21 new tests plus the 3 existing seam tests pass. Full-suite baseline at this merge-base was 11882 passed / 861 skipped / 0 failed. NOT verified: no CI leg run; the SQL Server and Postgres legs skip locally. * feat(seam): feed discovery into the snapshot and delete the curated tuples (BACKLOG #1220) Layer 2 of #1220. The generator now reads the DISCOVERED surface; the five hand-maintained tuples are gone. ENGINE_UI_SEAM is STILL the hand-chosen 20 -- deriving the value is layer 3. Isolating coverage from derivation keeps a bisect able to say which one broke something. WHAT THE GOLDEN GAINS. It went from 196 lines to 237, and the additions are the measured blind spot: UploadedFileList appeared ZERO times in the old golden. It now carries its field set AND its Literal values. This is the DTO whose required `scope` field landed in 40a4d5d without moving any seam file -- the gate stayed green through exactly the change it exists to catch. 25 more DTOs 7 the console imports directly, 18 reachable only by nesting. The closure matters because _dto_fields records names one level deep with no recursion, so a nested model's fields were absent entirely. 6 AuthService properties action_step_up_required, ad_enabled, kerberos_available, oidc_enabled, oidc_flow_ttl_seconds, store -- all read by the console across the seam, none recorded. inspect.signature RAISES on a property, which is why the retired tuple held methods only: the renderer's capability had silently defined what counted as the contract. _member() renders them now. has_action_step_up called at _auth.py:620, absent from the golden, and on the ADR 0077 per-action step-up path. client_ip, imported by the console and never hashed. The same correction drops five enforce_phi_read_pacing stale names the console no longer imports -- which is why _ui_seam.py asserted the console imports six api.security symbols directly, false for five of six. That assertion is deleted with the tuple it described. enum + Literal sets Field NAMES are not the contract. pages/uploaded_logs.py renders UploadedFileList.scope as _SCOPE_NOTES[data.scope], so renaming a literal from "own" to "owner" would KeyError at runtime while a field-name-only snapshot stayed byte-identical. NO SEAM BUMP, DELIBERATELY. The snapshot changed because the GENERATOR changed, not because the engine's contract did. A console built against seam 20 is no less compatible than it was an hour ago -- recording more of an unchanged contract cannot make an older console incompatible. Bumping here would spend a hard console-wheel refusal (SUPPORTED_ENGINE_SEAMS holds exactly one value, BACKLOG #279) on a bookkeeping change. The calibration tests now assert against the retired tuples FROZEN AS LITERALS rather than reading them from the generator. Reading them would make the calibration vacuous the moment the generator stopped carrying them, which is this commit. Frozen, they keep proving the two properties that matter: app.state is reproduced EXACTLY (the one tuple with zero drift), and no previously-curated name is ever dropped. Verified: ruff check + format clean, mypy strict clean on both scripts, 24 seam tests pass, plus tests/test_ui_oidc_interstitial_route.py and the full packaging/messagefoundry-webconsole suite -- those are the only three test files in the tree that reference the seam, the golden, or the generator (grep over tests/ and packaging/.../tests/ for ENGINE_UI_SEAM|webconsole_seam| SUPPORTED_ENGINE_SEAMS|engine_seam). NOT verified: no CI leg run; full suite re-run deferred to the end of the layer stack. * feat(seam): derive ENGINE_UI_SEAM from the contract surface (BACKLOG #1220) Layer 3, and the one that closes #1220. ENGINE_UI_SEAM is no longer a number anyone picks: it is a 16-hex SHA-256 of the DISCOVERED contract surface, produced by `python scripts/webconsole_seam_snapshot.py --write`. Value: 494a51230dce5730. WHY THIS FIXES THE COLLISION AND GATING THE INTEGER WOULD NOT. Two branches bumping to 19 for two independent contract changes produced a COSMETIC conflict in a comment block while the golden snapshot auto-merged CLEAN carrying both changes under one seam -- so resolving the visible conflict correctly still shipped the fault. A digest has no such failure mode: two branches changing different surfaces derive different values, and their MERGED surface derives a THIRD matching neither, so the merge reds. Uniqueness stops being something a human maintains. str, NOT a truncated int, and the old skew test is why. It asserted `ENGINE_UI_SEAM - 1` is refused, under a docstring claiming "one seam older AND one newer". Under an int digest that arithmetic still evaluates, the assertion still PASSES, and the docstring silently becomes false -- a passing test making a claim the value no longer supports, which is the same defect class #1220 was filed against. A str makes it a TypeError instead. It also removes the hand-typable affordance: someone can write 21 and it looks legitimate; nobody types a hex digest by hand and believes it. ANTI-CIRCULARITY IS BY CONSTRUCTION. contract_sections() never reads ENGINE_UI_SEAM, so the seam cannot feed its own digest -- there is no "strip the seam line" filter that could quietly stop matching. The test asserts it by MOVING the constant and requiring the digest to hold still, not by checking the seam's text is absent from the input: a substring check is defeated by a short or coincidental value (the header already contains the literal 0065). Mutation-proved before trusting it -- a generator that folds the seam into its own input makes that test FAIL, as it must. Four tests, three of them new: the stored value equals the derived digest (the gate); the digest holds still when the seam moves (anti-circularity); the digest MOVES when a rendered DTO gains a field, with 40a4d5d as the historical control where it did not; and the skew test rewritten to refuse the empty string, a truncation, an extension and an arbitrary digest, preserving the both-directions intent BACKLOG #279 put in the original. The failure message is rewritten and is now pure ASCII. The old one carried U+2014 (a replacement character on a cp1252 console, the developer-facing half of #1221) and told the reader to "bump ENGINE_UI_SEAM" -- an action that no longer exists. A gate whose message names the wrong repair teaches the wrong repair. --write rewrites the constant and the golden, and REFUSES rather than guesses if the constant line does not match exactly once. It deliberately does NOT write the console's SUPPORTED_ENGINE_SEAMS: that is the independent half of a two-wheel handshake, and a tool writing both sides turns it into a self-consistent tautology. The message spells out the one-line manual edit instead. Also fixes tests/test_ui_oidc_interstitial_route.py, which passed engine_seam=0 into a field now typed str. Nothing automated would have caught it -- tests/ is outside mypy's configured scope and a dataclass does not validate at runtime -- so it was found by grepping every consumer deliberately. Verified: ruff check + format clean; mypy strict clean on messagefoundry_webconsole; 40 tests pass across the three seam-touching test files. mypy on messagefoundry reports 21 errors in 4 files (auth/webauthn.py, parsing/dicom/_deps.py, parsing/fhir/_deps.py, transports/dicom.py) -- all pre-existing and all downstream of four optional extras absent from this local venv (pydicom, pynetdicom, webauthn, fhir), which CI installs. None of the four is in this diff and none references the seam. NOT verified in this commit: the two-branch merge proof and the full suite; both run next. * fix(seam): refuse an unresolved ForwardRef instead of walking past it (BACKLOG #1220) Found by running #1220's own acceptance proof, which is the entire reason for running it rather than reasoning about it. Proof (c2) plants a DTO reachable ONLY as a field of one the console imports, renames that nested DTO's field, and requires the gate to red. It stayed GREEN. MECHANISM. Pydantic resolves string annotations at class-build time, but NOT when the referenced class is defined later in the module and nobody calls model_rebuild(). The field's annotation then stays a ForwardRef; typing.get_args() returns () on it; and the closure walked straight past a nested DTO whose field set belongs in the contract. Measured directly: SystemStatus.model_fields['proof'].annotation -> ForwardRef("'ProofNested | None'", is_class=True) typing.get_args(...) -> [] That is a SILENT SKIP inside the walk -- precisely what this module's own docstring forbids, and the defect it was written to remove, reproduced one level down. A skip here is indistinguishable from full coverage at every downstream vantage point: the digest is computed, the gate is green, and the uncovered model looks exactly like a covered one. The fix raises SeamDiscoveryError naming the class and the ref, and says how to resolve it (define the referenced model first, or call model_rebuild). Zero occurrences in the tree today, so this costs nothing now; its first firing is a real hole. With the guard in place the corrected proof (c2) discriminates as it should -- renaming the nested field reds the gate on this tree and leaves origin/main GREEN, which is the defect the closure exists to close. Same edit, opposite outcomes. Verified: ruff check + format clean, mypy strict clean, 28 tests in tests/test_seam_discovery.py pass including a new one that drives the ForwardRef case directly. * docs(seam): record the seam digest in the crypto inventory (BACKLOG #1220) The full suite caught this, not review: three tests failed on the #1220 stack, all one cause -- scripts/webconsole_seam_snapshot.py now calls hashlib.sha256 and the crypto-inventory gate refuses an undocumented crypto call site. The gate is correct and this is the entry it asked for. Recorded in all three places the convention requires, because they are three INDEPENDENT enumerations of the same fact and a fix that updates one leaves the other two lying: scripts/security/crypto_inventory_check.py INVENTORY -- what the gate enforces tests/test_security_static.py _CRYPTO_SITES_OUTSIDE_THE_PACKAGE -- the test-side pin that covers roots the gate does not yet walk (#282) docs/ASVS-L2-PHASE0-CHANGES.md section 4 the human inventory the gate calls itself the machine-readable companion to WHAT THE RATIONALE HAS TO SAY, because "we hash something" is not a justification: it is a CHANGE DETECTOR, not a security control. No secret, no key, no message authentication, and nothing user- or PHI-derived is hashed -- the input is a serialization of public type signatures, field names, enum members and Literal values. The property required is accidental-collision avoidance across the contract surfaces this project will ever produce, which at 64 bits is 2.7e-12 for 10,000 distinct surfaces (about 500x the ~20 seam moves to date). Preimage resistance buys nothing: anyone able to craft a colliding surface already has commit access to _ui_seam.py, where writing the constant directly is strictly easier. SHA-256 rather than BLAKE2 or a truncated non-approved digest only because the engine renders a fips_mode attestation, and a non-approved hash in the shipped surface invites a FIPS question for zero benefit. Verified: tests/test_security_static.py + tests/test_crypto_inventory_scanner.py, 64 passed, 0 failed -- the three that failed on the previous full run. ruff check + format clean. * feat(ai): refuse an unserviceable [ai].provider at config time (BACKLOG #95) Validator ONLY, per the owner's ruling -- no per-provider wire shapes. THE DEMAND GATE ON #95 WAS LIFTED BY THE OWNER IN SESSION on 2026-08-12, in response to a pause-and-ask that named the item, its scope and its cost. Recording that here because the ledger row cannot cite a source otherwise, and this project has already resolved one lane-vs-lane dispute over an unsourced claim that a gate had been lifted. WHAT WAS WRONG. settings.py declared `provider: str = "claude"` with no validation, so any string loaded clean -- azure_openai, gpt-4o-gateway, '', even '../../etc/passwd'. Verified through the real config path (load_settings), not just the constructor. WHY IT MATTERS EVEN WHEN THE BROKER IS NEVER BUILT, which is the part that decided the design: the value is recorded in the PER-USE AUDIT regardless of mode (api/app.py). A config naming a provider the engine cannot service therefore made the audit trail assert something untrue -- so the validator is field-level and refuses in every mode, rather than a model_validator gated on managed_endpoint. The narrower form was the defensible alternative; this is the deliberate choice, not the default. THE ALLOWLIST IS ONE ENTRY, AND ITS SOURCE OF TRUTH IS A HARD-CODED REQUEST BODY. AiBroker.chat builds one wire shape unconditionally -- an Anthropic Messages body with x-api-key and anthropic-version, and an _extract_text that assumes Anthropic's content-block list. There is no provider registry, no dispatch, and AiBroker.provider has ZERO readers, so nothing can derive the serviceable set. That makes the list hand-maintained, which is a real cost and is written down as one. A test pins it to chat()'s body so widening the list without teaching chat() a second shape fails loudly. THE INVERSE FAILURE IS THE TEMPTING ONE. Listing aspirational names (azure_openai, bedrock, ollama) would ACCEPT configurations the broker still cannot service, turning a clean config-time refusal back into the opaque runtime failure this item exists to remove. The list describes what chat() can send, never what the roadmap intends. DOCS. The item names docs/AI.md:22. That line is stale, but docs/CONFIGURATION.md:637 carried a STRONGER false claim the item does not mention -- that provider selects "the broker's request shape", which is not true in the code and directly contradicted AI.md. Fixing only the named line would have left the more actively misleading one in place. Also corrected: AI.md asserted "No model-provider or engine broker integration exists yet" while managed_endpoint has been BUILT since ADR 0135, and the mode tables omitted managed_endpoint entirely (the string appears zero times in that file). The ai_broker module docstring said the Anthropic shape "is used when the provider is claude", implying a branch that does not exist. Verified: ruff check + format clean, mypy strict clean on both changed modules, 94 tests in tests/test_ai_policy.py + tests/test_ai_broker.py pass, and 916 passed / 0 failed across every doc-drift, link and config test in the tree. NOT verified: no CI leg run. * fix(uploads): attribute the upload.prune audit row to the system (BACKLOG #1224) Both sites, because closing one leaves the same false attribution reachable by the other path: the save-time opportunistic sweep in the upload handler, and the background UploadRetentionRunner's audit closure in the API lifespan. Located by the literal "upload.prune", not by line number -- the item's own cited lines had already moved 87 and 121 lines when #1152 landed above them. THE DEFECT IS CROSS-OWNER, AND IT IS STRUCTURAL RATHER THAN INCIDENTAL. prune_expired() is deliberately UNSCOPED -- it has to be, since the per-uploader quota and the sweep both need to see every file -- so the operator whose upload triggers a pass is in general NOT the owner of what it prunes. The row named the pruned file's OWNER as actor while stamping the TRIGGERING operator's address as client, asserting that X deleted their own file from Y's host. That is true of neither of them. The fix applies a decision this repo already made rather than inventing one. actor="system" matches pipeline/retention.py's retention_purge (plus dr, dr_backup, reference_sync and the auth bootstrap). Dropping client is not tidying: _record_reload_audit's own contract is that client is the address OF THE ACTOR NAMED IN THE ROW, and once the actor is the system principal no address is in scope -- ADR 0150 decision 4 rejects exactly this pairing for dual-control config reload, calling a stamped approver address "worse than NULL". Note the engine-internal-writes rule does NOT justify it here, because site 1 genuinely runs inside a request; the actor-address correspondence is what does. The uploader is not lost -- it stays in detail.uploader, which is where a reader can still see whose file went without the row claiming they went and deleted it. RED-FIRST, TWICE, because one mutant was not enough. Reverting the actor reds the test with actor == 'op' while op2 triggered the sweep -- the cross-owner defect in one assertion. And a PARTIAL-FIX mutant (actor corrected, client=client_ip(request) left behind) reds on `assert not '127.0.0.1'`. That second mutant is the point: a test asserting only the actor string would have passed the half-fix, which is the likeliest way this gets "fixed" wrong. Zero tests referenced "upload.prune" before this one, so the defect had no guard at all. Verified: 97 tests across tests/test_upload_api.py, tests/test_uploads.py and tests/test_audit_integrity.py pass -- the last because dropping client changes the ADR 0150 hash-chain payload shape for these rows. ruff check + format clean, mypy strict clean on app.py. NOT verified: no CI leg; SQL Server and Postgres record_audit implementations were read, not executed -- the audit-integrity suite covers the SQLite path only in a local run. * fix(search): key saved presets on the immutable user_id, not the username (BACKLOG #1225) Four sites in api/app.py, and the WRITE went first because it is the setter the other three read -- re-keying only the readers would have made every newly created preset invisible to its own creator. Located by store-method name, not line number: the item's cited lines had all moved +122 when #1152 landed above them. THE ENABLING HALF THE ITEM DOES NOT CITE. A username being reassignable is only half the defect. The other half is that delete_user (store/store.py) removes user_roles, sessions and webauthn_credentials and NEVER preset rows -- so a departed operator's presets outlive the account under a name that is now free to reissue, and a recreated account of that name inherited them, including the PHI-shaped `criteria` that the layered compose returns. Both halves are asserted in the new test. WHY THE EXISTING SUITE WAS NOT EVIDENCE. test_preset_crud_and_owner_scoping compares two operators who are BOTH LIVE, and it passes on the defective code. The defect needs an account to go away and its name to come back; nothing exercised that. The new test does, through the list route, the delete route and the layered compose. RED-FIRST, AND THE FIRST RED WAS THE WRONG ONE. Restoring username-keying at all four sites initially failed on my own PRECONDITION -- which queried by user_id and so could not survive a username-keyed build -- masking the assertion that matters. Made key-agnostic, the mutant now reds exactly where it should: the recycled account sees {'name': 'mine'} where it must see []. A test that goes red for the wrong reason proves nothing, and it looks identical to one that does not. I ALSO TRIED TO CLEAN UP THE STRANDED ROWS AND BACKED IT OUT. The re-key leaves pre-existing rows matching no user_id: unreachable by every route, with their encrypted criteria lingering. I added a one-shot DELETE keyed on `owner NOT IN (SELECT id FROM users)` -- exact, idempotent, and it would have collected delete_user's orphans too. tests/test_retention.py caught it: test_reopening_a_pre_306_db_migrates_last_used_at_in asserts that migration PRESERVES rows, and my statement ran on every open and destroyed data. That is a behaviour change well outside this item, so it is reverted. CONSEQUENCE, stated rather than hidden: a developer with an existing dev store will find their old presets unreachable and should recreate the store. There is no correct username-to-user_id backfill -- a freed name may since have been reissued, which IS the defect. THIS IS A NARROWING, NOT A CLOSURE, and the ADR now says so: _upsert_ad_user re-binds a surviving mirror row's user_id when a directory-side sAMAccountName is recycled without a MessageFoundry delete_user, so a user_id-keyed check is still defeated on that path. BACKLOG #1143 is the real close. Also corrected: the schema comment that read "the owning username" and would have become a lie, the field_authz seed that wrote presets under a username (a row no route can now reach), and ADR 0136, which asserted `owner = identity.username` as the design. NOT swept: identity.username appears 60 times in app.py and 46 are audit actor= fields, where recording a NAME is correct. Only the four ACCESS KEYS moved. Verified: ruff check + format clean, mypy strict clean on both changed modules; 212 passed across every preset/retention/field-authz/schema/PHI-inventory/doc-drift selection, and 447 passed across every doc, ADR and link test. NOT verified: no CI leg. The Postgres and SQL Server preset paths were read, not executed -- they skip locally, and per project experience a blind DB-leg change fails only in CI. Their DDL is unchanged by this commit (the column still holds a string) but their tests seed owners by name. * fix(lint): harden the ASVS tally gate's own stdout against a cp1252 console (BACKLOG #1221) The gate prints DOCUMENT CONTENT, and the corpus it scans is documentation -- so a hit whose line carried a glyph killed the run with UnicodeEncodeError before the finding could be read. THE DEFECT IS INVISIBLE UNTIL THE GATE HAS SOMETHING TO SAY. Against the real corpus it exits 0 and looks perfectly healthy; it fails exactly when it is needed. Reproduced deterministically here before fixing: a one-line doc with a new tally and a U+2192 dies at the findings loop, and the same run is green on the shipped corpus. STREAM-LEVEL, NOT A TARGETED SAFE-PRINT AT THE OBVIOUS SITE -- and that is a correctness choice, not a style one. Python's \d is Unicode-aware, so _AGAINST_TOTAL and _ARITHMETIC take m.group(0) VERBATIM into the token: idioms_for_line() over Arabic-Indic digits yields a token carrying non-ASCII. That token flows into the --print-keys output, the stale-baseline report AND the baseline file, so hardening only the findings loop leaves the same class open in three other places. The two fix shapes are not equally defensible, which is what the item asked to have decided explicitly. Copied rather than imported: this file is on MIRRORED_TOOLS and is contractually stdlib-only, so it cannot reach for a shared helper. The guard mirrors messagefoundry/__main__.py's shape including its reason for being guarded -- some stream wrappers lack reconfigure or reject it, and hardening must never itself crash the tool. THE TEST IS A SUBPROCESS, DELIBERATELY. The existing 45 tests never exercised the print path at all. Forcing the encoding in-process would NOT be the same instrument: pytest's capture wrapper is one of the very objects the hardening guards against, so an in-process test can pass having measured nothing. The child runs under PYTHONIOENCODING=cp1252 with no error handler, so it starts at errors='strict' and only the fix can save it -- a test of the fix rather than of the environment. Mutation-proved: removing the _harden_stdout() call reds the new test with the child's own UnicodeEncodeError traceback in the failure message. Verified: 45 tests in tests/test_asvs_tally_lint.py plus the vault mirror contract pass; ruff check + format clean. The real-corpus run still exits 0. NOT verified: no CI leg. The lint's CI job runs on ubuntu (utf-8), which is why the test forces the encoding rather than relying on the platform -- the defect is not reproducible there by default. * test(seam): classify the seam digest as non-key material (BACKLOG #1220) The full suite caught this, and it is the SECOND gate to fire on the same commit -- the crypto inventory wanted the call site recorded, and this one wants every ASVS section 4 row classified as key material or explicitly not. Both are correct, and between them they are why "we hash something" cannot be waved through here. Classified as NOT key material, with the reason the gate asks for: it is a keyless content hash over PUBLIC type signatures, field names, enum members and Literal values -- a change detector for the engine/console handshake, not a key, a secret, or a message authenticator. Nothing user- or PHI-derived is hashed, and the digest is committed in source on both sides of the seam, so there is no scope to state and nothing to rotate. That places it with the Config fingerprint, the ASVS corpus pin and the engine wheel attestation -- the three existing rows of exactly this shape. Verified: 69 tests across tests/test_key_usage_scope_inventory.py, tests/test_security_static.py and tests/test_crypto_inventory_scanner.py pass. ruff clean. This was the ONLY failure in the full-suite run over the whole lane (1 failed, 11914 passed, 861 skipped). * feat(uploads): record the immutable owner key on the prune audit row (BACKLOG #1224) Owner-approved follow-on to #1224, raised rather than folded in silently because it is beyond that item's stated scope -- #1224 was about the ACTOR field, this is the DETAIL. WHY IT IS WORTH THE KEYSTROKES. The prune row is a permanent record of a deletion whose subject cannot be recovered afterwards: the file is gone, so `detail` is the only surviving description of whose it was. It recorded `uploader`, a username, and a username is reassignable -- which is the entire finding of #1225, shipped two commits ago. A row read a year from now could name a different person than it meant, and nothing would flag it. UploadedFileMeta already carries both fields and its docstring says the split is deliberate (uploads.py:116: "uploader_id is the account's IMMUTABLE..."), so the data was in hand at both sites. This records both: the username because that is what a human reads, the id because that is what still identifies the account after the name moves. FIRST-OF-KIND, AND SAID OUT LOUD. No other audit row in api/app.py carries an id in its detail, so this sets a convention rather than following one. That was the honest argument against doing it, and the counter-argument that won: you fix things where you touch them, and the alternative -- an item proposing the convention for every owner-bearing audit row -- improves nothing today and may sit. If that broader sweep is ever wanted, these two rows are the precedent to point at. No schema change: `detail` is a free-form JSON string, so existing rows still verify against the ADR 0150 hash chain. Ran the audit-integrity suite rather than assuming that. The test asserts uploader_id is present AND that it differs from the username, so a fix that populated it with the name would still red. Verified: 68 tests across tests/test_upload_api.py and tests/test_audit_integrity.py pass; ruff check + format clean; mypy strict clean. * backlog: close BACKLOG #1220 -- the derived seam, and the numbers this item published are now void The fix is the building lane's (claude/instruments-config-handoff-e64adc). This commit adds only the banner, which is the coordinator's to write: allocation and the commit that files it cannot be split across worktrees, and the claim gate requires the banner in the SAME PR as code citing BACKLOG #N. I will not buy that check's green by removing the citation from the PR body -- a green there is evidence nothing looked. The banner records three things beyond "shipped": 1. It shipped the option THIS ITEM PREFERRED. The Scope says derive the seam from a hash so collisions are impossible by construction, and warns the gate-an-integer alternative is "one more thing to remember". The built form is the derived one, and the surface is DISCOVERED rather than enumerated, which was the stated condition. 2. THE ITEM'S OWN NUMBERS ARE VOID, kept rather than deleted. It publishes origin/main = 18 and "the second branch to land re-bumps to 20". Measured 2026-08-12: main reached 20 when #1152 landed, so BOTH held branches sat at 19 -- BELOW main -- and that rule had already inverted before anyone applied it. 3. WHAT IT BREAKS. The seam becomes a str digest, so the two held branches no longer carry a stale integer, they carry a mypy-strict TYPE error. Their bumps are a rewrite, not a renumber. Sequence #1227's class fix after this, never concurrently. Attribution is split in the banner: what the lane reported (the three acceptance criteria, the 25-DTO coverage figure) is labelled as reported, and what was measured here (main at 20, both branches at 19) is labelled as measured.
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.
Authored by the session in worktree
nice-payne-4dcee0; pushed and opened by the coordinator. Docs only-- one BACKLOG item, +28 lines. The fix is deliberately not in this PR.
The defect, and it is LIVE on the installed gate
worktree_gate.ps1recurses one level into an interpreter argument, because that argument is quoted butis code that runs. The recursion keys on a fixed alternation:
PowerShell accepts any unambiguous prefix of a parameter name. So spellings exist that execute and are
not on that list -- and their argument is then blanked as an ordinary quoted span, which makes its contents
invisible to every rule, not only rule 3c.
Coordinator verification -- reproduced independently, not taken on report
1. Which prefixes actually execute (harmless
Write-Output PREFIX_RANpayload):-Command-Comm-Com-Cm-C-c)So the bypassing family is bounded --
-Com,-Comm, and the intermediate spellings up to-Command--which is what makes this fixable rather than open-ended.
2. Regex confirmation,
[regex]::IsMatch(..., 'IgnoreCase')onpwsh -NoProfile <flag> "git reset --hard":-CommandTrue,-CTrue,-CommFalse,-ComFalse.3. It is LIVE, which required checking the INSTALLED copy rather than the source. The gate executes
from
C:\Users\Scott\.claude\hooks\worktree_gate.ps1(per thesettings.jsonhook wiring), 93,581 bytes,last written 2026-08-07 11:19. LF-folded, it is byte-identical to
scripts/hooks/worktree_gate.ps1inthis checkout and carries the same fixed alternation. This repo has measured genuinely stale installed
hooks before with zero instruments watching them, so "the source is wrong" and "the live gate is wrong"
are separate claims; both hold here.
The obvious fix is insufficient, and that is the load-bearing part
Adding the three spellings to the list closes every form on the committed gate, but on a
message-blanking gate it closes only the single-line form and leaves the here-string route open behind it.
The here-string form denies today only by accident -- every line of a multi-line span reaches the
scanner raw -- and that accident is removed by any correct message-blanking change.
So the fix must be verified against the gate as it will SHIP, not as it is today.
Why the #1086 fix was reverted, and the lesson worth carrying
The #1086 work is banked, not abandoned. It was rejected because two of three verifiers independently found
a new fail-open in it; the design was right (key on the consuming flag, blank after interpreter
recursion, both pinned by killed mutants) and only the classifier was wrong -- one open-ended cluster regex
matched
-Com. Two verifiers measured the closed-flag-set replacement as returning the bad rows to DENYwith zero other verdict movement, so it is resumable (
resumeFromRunId: "wf_b4050e2f-77d", re-apply thebanked patch first).
Claim 1086 remains held by
nice-payne-4dcee0.Auto-merge not armed -- queued behind merge train #275. Docs-only, so it costs no CI slot when its turn
comes.