FhirPeek refuses a repeated JSON key on every parse path (BACKLOG #2812)

[wshallwshall]

Batch 265 (transports), BACKLOG #2812, on its own because it changes a security control. It adds two refusals. One of them, reply-repeats-a-key, is a new permanent refusal after a 2xx. It rests on a Manager decision (batch 265), not on an owner ruling. Its siblings rest on an owner ruling or on ADR 0095 B.1, so the Lander may want the owner to rule on it (see Open findings).

Merge danger

Reversible: yes. A hand revert of b9c0e26 and b1dcfd4 undoes it.

Blast radius: FHIR outbound connections. FhirPeek.parse (parsing/fhir/peek.py) now refuses a JSON object that repeats a key at any depth. A single-resource create or update dead-letters as bad-body. A wrapped update's 2xx transaction-response that repeats a key now dead-letters permanently as reply-repeats-a-key and is not recorded as delivered. fhir_lookup raises FhirLookupError. RepeatedKeyError and refuse_repeated_keys moved from transports/fhir.py to messagefoundry/redaction.py. The Bundle screen from engine PR 2244 behaves as before.

Evidence

• Before: a body or reply that repeats a key is gated and routed on the last value while the server is given every value. With the hook removed from FhirPeek.parse, all 9 body-refusal tests fail and the controls pass. With the reply refusal disabled, all 10 reply-refusal cases fail and both controls pass.
• After: the hook is restored and the reply refusal enabled. Repeated top-level id, the Observation-then-Patient resourceType case, a nested repeat, and the same body as bytes and behind a BOM are all refused. A bad-body dead-letter goes out with nothing sent, across create, update, if-match, path-form, conditional-update and if-none-exist. The measured reply {"status":"200 OK","status":"404 Not Found"}, its reverse order, a repeat inside meta, a reply where both readings are a success, and a reply that is not a transaction-response each dead-letter with capture on and off. A clean body is sent and a clean reply is delivered (controls). Refusal text names no key or value.

Builder report: lane F, BACKLOG #2812

Source branch b265-2812, head b1dcfd4d7cc6b2b85a025e662d602fda3c8663c6. That is 2 commits on origin/main 8ba57ee: b9c0e26b (the build) and b1dcfd4d (the Manager's fail-closed correction).

A Manager correction happened between the two commits. The first brief had a repeated-key reply read as unparseable. Code review flagged in both rounds that this weakened the #2180 control: the measured reply above dead-lettered on origin/main and delivered on b9c0e26b. The Manager reversed it, and the reply now fails closed.

• Checks: ruff clean and all three mypy legs clean. 1082 FHIR, redaction and boundary tests passed (2 xfailed) on b9c0e26b, and 572 passed on b1dcfd4d. The full suite was not run. Some test_redaction.py / test_redaction_glued_credential_label.py cases fail only under xdist load (-n 8 / -n 4) and pass serially. This diff does not touch that code.
• Brief versus tree:
  • The names are public (RepeatedKeyError, refuse_repeated_keys) because they are imported across modules.
  • These files outside the lane were edited: docs/CONNECTIONS.md (the FHIR Interactions paragraph and the update-reply bullet), changelog.d/, tests/test_redaction.py, tests/test_dependency_boundaries.py.
• Open findings, not fixed:
  1. The refusal fires on a repeated key anywhere, including a reply where every reading is a success. An update that most likely applied is therefore dead-lettered, and replaying an if-match update would then hit a 412.
  2. Provenance: reply-repeats-a-key rests on a Manager decision; see the top of this body.
  3. Other unreadable 2xx replies (malformed JSON, XML, too deep) to a wrapped update still count as delivered with no log line, as on origin/main.
  4. _repeats_a_key re-parses the reply and compares against RepeatedKeyError.__name__; a FhirPeekError subclass would remove both.
  5. The fhir_lookup message still says "unparseable body" for a repeated key.
  6. _capture_outcome, _operation_outcome and FhirResource.parse still keep the last of two equal keys.
  7. The peek runs on the event loop, and the hook makes a parse about 1.8x slower (390 KB searchset measured).
  8. Keys that differ only by case or by an escaped NUL are not caught.
  9. Bytes decoded with errors=replace can merge two different invalid keys; that is refused but reported as a repeated key.
• Banner: SHIPPED: FhirPeek.parse refuses a repeated JSON key at any depth; single-resource writes dead-letter as bad-body and a wrapped update's repeated-key reply fails closed as reply-repeats-a-key. Also closes PR 2244 follow-up item 3.

Hosted legs to read

The standard ci.yml test and mypy legs, especially linux. No local full run was made.

[wshallwshall]

QA (lane F, BACKLOG #2812): code-review skill at level xhigh (BUILDER.md step 11), args "xhigh Review this branch's diff against origin/main; read docs/REVIEW-STANDARDS.md and apply its rules." plus "including uncommitted working-tree changes" in later rounds. Tag: none printed; the skill returned a JSON findings list. Head b9c0e26: two rounds; both flagged the fail-open reply path, which the Manager then reversed. Head b1dcfd4: one round; fixed a from-None raise that broke test_every_from_none_is_classified, the bullet scope, the "(FHIR destination)" suffix, the changelog "at least" list, and a branch-state test comment. Shipped open: findings 1-9 in the PR body.

[wshallwshall]

Lander, under the owner's standing driver rule (an adversarial review upheld the Lander's recommendation, so no owner question is needed): SENT BACK, narrowed.
On main an all-success repeated-key reply is delivered (last value wins). This PR dead-letters it, which falsely fails an update that most likely applied, and invites a replay that applies it twice. Vault #2812 asks only for refusing repeated keys in an OUTGOING body.
Fix: parse the reply twice, once keeping the first value of each key and once keeping the last. Deliver only if both parses agree on everything consumed: status, etag, location, lastModified, resourceType and the entry shape. Otherwise raise reply-repeats-a-key. That keeps this PR's real fix: main delivers '404 then 200', which fails open.
Post the new head with a QA line naming it. Its 9 open findings must be filed before it lands.

[wshallwshall]

Manager decision, batch 265, taken under the owner's driver rule: the reply-repeats-a-key permanent refusal stands as built. No owner ruling is needed before merge. Why: it fails closed in the same shape as the existing reply-too-large refusal. The alternative (read the reply as unparseable) was measured to turn a 404 entry that origin/main dead-lettered into a recorded delivery, which weakens the #2180 control. The cost, finding 1 (an applied update whose readings are all success is dead-lettered and needs a manual check before replay), is an operator check on a reply no conformant FHIR server sends. That is the safe direction. This supersedes the "may need an owner ruling" line in my handover. The owner may still overrule at review.

[wshallwshall]

Manager, batch 265: RETRACTED. My 14:47 comment ("reply-repeats-a-key stands as built") was written without reading the Lander's 14:29 send-back, and it is withdrawn. The Lander's narrowed ruling governs: deliver a repeated-key reply only when the first-value and last-value parses agree on everything consumed (status, etag, location, lastModified, resourceType, entry shape); otherwise raise reply-repeats-a-key. The lane F Builder is re-briefed to build exactly that on b265-2812. A new head and a QA line naming it will follow.

[wshallwshall]

Manager, batch 265: DO NOT MERGE head 78dd64a. Its last commit, 78dd64a, adds two defects: an exponential-time compare (a 521-byte reply nested 22 deep took 2.44 s, and depth 32 would take about 40 minutes on a to_thread worker with no timeout), and a RecursionError at about 1200 levels that escapes send(). A git revert of 78dd64a is being pushed now. It restores the tree of cc9d9d0 (reviewed), which applies the three-occurrence rule in linear time. Wait for the revert head before landing. Open after the revert, for a row: a fail-open that needs exotic nesting (a middle response object that reads 404 only under keep-last is delivered); and 1 vs true is not distinguished below the top level. Both are strictly narrower than main, which fails open on a plain 404-then-200. The proper fix (canonical node ids interned bottom-up in the hook) is a follow-up row.

[wshallwshall]

QA (PR 2356, final head e94b9cd). The hold is lifted. This head is a git revert of 78dd64a, so its tree is byte-identical to cc9d9d0 (git diff is empty). That tree was covered by the code-review pass at xhigh over 696d157..cc9d9d0, which applies docs/REVIEW-STANDARDS.md. No pass printed a tag.

What the tree does. The Lander's agreement rule, narrowed:

• A wrapped update's repeated-key reply is delivered only if the first-key and last-key readings reach the same judgment and agree on etag, location and lastModified.
• No occurrence of a repeated key may differ from both its first and its last value.
• Otherwise it fails closed as reply-repeats-a-key. This is recorded in ADR 0095 B.1, 2026-10-17.

Shipped open, for rows:

1. A fail-open in exotic nesting: a middle response object that reads 404 only under keep-last is delivered. This is still narrower than main, which delivers a plain 404-then-200.
2. 1 against true is not distinguished below the top level.
3. A NaN beside a repeat refuses (fail-closed).
4. The changelog fragment does not mention the occurrence rule.
5. The proper fix: canonical node ids, interned bottom-up in the hook.
6. Earlier items:
   • captured replies keep their repeated keys;
   • the refusal says "FHIR destination", not the connection;
   • fhir_lookup still says "unparseable";
   • other reply parsing keeps the last key;
   • the peek runs on the event loop;
   • the hook lives in redaction.py;
   • the depth-band refusal text says a key repeats.

[wshallwshall]

Lander: enqueuing at pinned head e94b9cd. Review proof covers this head; open findings are filed in vault (pass U, vault PR 2506, merged).