From 70b3dff70e3e06a8dd93dba21e4ee59f39cce381 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 13 Aug 2026 06:28:09 -0500 Subject: [PATCH] backlog: file #1243 -- close ASVS 1.2.2 on the merits, both limbs in one item (#1107) Owner-ruled on both halves of the 1.2.2 encoding question, filed as one item because closing either half alone leaves the cell at partial. Limb A removes the verbatim flat FHIR query path. The default ships fhir_require_structured_params = False, so the shipped path appends the caller's search string with no encoding. The ruling was deletion rather than flipping the default: a flipped default leaves the unencoded sink reachable behind one config edit, which closes the cell on a setting rather than on the absence of the sink. Zero deployments means there is no migration cost to weigh against the simple correct end state. Limb B escapes the FHIR value separators in structured values. Percent-encoding neutralises the URL layer but not the FHIR value layer, so a comma, pipe or dollar inside a datum survives decoding and changes the query's meaning. The item records that limb B must NOT touch conditional_query, which is an operator-authored query string whose separators are deliberate, documented and test-pinned on both sinks. Its containment is the already-filed screen item. It also records that the four-shape screen sitting where encoding belongs is the gap, not a control, so it must not be anchored as evidence for the cell. --- docs/BACKLOG.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index c72af25f..b42bc222 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -8329,3 +8329,21 @@ every worker session's handoff, which is the sentence the next session bases its > ⚠️ **Amendment, same day, recording what the adversarial pass corrected.** The item was first written as "latent, and a clean scalar/sub-table asymmetry." Both halves were wrong. It is **active**, because the measurement was taken one commit before the backfill landed and the parent revision genuinely has zero `sym` keys -- a stale checkout reproduces the wrong answer perfectly and reads as confirmation. And the asymmetry is four-way, not two-way; the clean framing concealed the table-mangling and the payload-only drop entirely. A fifth limb about line-ending rewriting was **investigated and rejected**: the tool does rewrite the working tree to LF, but `core.autocrlf=true` normalizes it back in the index, so the blob is unchanged and the claimed review-diff consequence does not occur. **Cluster:** Security tooling / evidence integrity. **Priority:** P1. **Verdict:** build. **Severity:** no deployment axis -- vault tooling, ships to nobody. P1 rather than P2 because the loss is **pending on the next routine operation**, is **silent in both directions** (the writer reports success, the verifier reports green having checked less), and destroys evidence that cost a dedicated backfill to produce. + +## 1243. close ASVS 1.2.2 on the merits: remove the verbatim flat FHIR query path and escape FHIR value separators + +> 🔢 **Filed 2026-08-13 -- OWNER-RULED, both limbs, and the two limbs are one item on purpose.** Value **7/10** -- Difficulty **5/10**. ASVS 1.2.2 asks that untrusted data be *"encoded according to its context"* when dynamically building URLs. `transports/fhir.py` gets the URL layer right and the **FHIR value layer** wrong, and separately ships an **unencoded path as the default**. Either one alone holds the cell at partial, so **closing one limb without the other does not close 1.2.2** -- that is why this is not two items. + +> **LIMB A -- REMOVE THE VERBATIM FLAT QUERY PATH.** `[egress].fhir_require_structured_params` ships **`False`** (`config/settings.py:2555`), so the default path is `transports/fhir.py:750`, `url = f"{url}?{search_part}"` -- the caller's search string appended **with no encoding at all**. The four screens above it at `:744-748` reject `#`, a second `?`, and a percent-decoded control character, and the comment at `:741` labels them *"Defense-in-depth (ASVS 1.2.2)"*. +> **Do not read that comment as a control. The screen is the gap.** It is a denylist of three shapes standing where contextual encoding belongs, and its own comment says why it stops there: it declines to break *"legitimate multi-param (&/=/|) searches"*. Anchoring `:744-748` as evidence for 1.2.2 would cite the statement of the shortfall as proof of the control. +> **The ruling is deletion, not a flipped default.** Setting `fhir_require_structured_params = True` would leave the unencoded sink reachable behind one config edit, which closes the cell on a default rather than on the absence of the sink -- a compensating control that a single setting removes. With **zero deployments** there is no migration cost to protect, so the simple correct end state wins outright: one code path, encoded by construction. + +> **LIMB B -- ESCAPE THE FHIR VALUE SEPARATORS `,` `|` `$` IN STRUCTURED VALUES.** `_encode_search_params` (`transports/fhir.py:671`) percent-encodes via `urlencode(..., quote_via=quote, safe="")`, which neutralises `&` and `=` at the URL layer. It does **not** neutralise `,`, `|` or `$`, which are **FHIR search-value separators**: they survive percent-decoding and arrive at the server with their separator meaning intact. Verified by execution -- `code=a,b` and `code=sys|val` reach the server as a two-value OR and a system-qualified token respectively, not as literals. A data value that happens to contain one therefore **changes the query's semantics**, which is parameter injection one layer above the URL. FHIR defines backslash escaping for exactly these characters; there is currently **no escaping of any kind** in the file. + +> **LIMB B DOES NOT TOUCH `conditional_query`, AND THAT IS THE WHOLE REASON IT IS SAFE.** The distinction is provenance, and it is easy to get backwards. A **structured value** is a single datum, so a separator inside it is always accidental and escaping it is always right. `conditional_query` is an **operator-authored whole query string** in which separators are the author's own syntax -- `CONNECTIONS.md:1479` documents `identifier=sys|val` as the intended shape and `tests/test_fhir_transport.py:210-221` pins it on both sinks (`If-None-Exist` at `:214`, the conditional-update URL at `:221`). **Escaping `conditional_query` would break a documented feature and fail a shipped test.** Its containment is a construction-time screen, which is **#1241**, already filed. Fixing this item must leave both assertions green. + +> **WHAT IS NOT IN SCOPE HERE, so nobody folds it in.** The `match`-versus-`fullmatch` grammar-gate defect and the missing READ-path control-char screen are **#1240**. The construction-time screen for operator config reaching URL and header sinks is **#1241**. The two identical control-char predicates are **#1239**. This item is the 1.2.2 encoding question only. + +> **RE-SCORE NOTE.** 1.2.2's residual of record currently reads, in full: *"fhir_require_structured_params ships False; on the default the flat search string is appended verbatim."* Limb A retires that sentence. The cell cannot reach an honest pass on Limb A alone, because the structured path it would then mandate still leaks the value-layer separators -- which is the finding that produced Limb B, and which had been filed as safe by everyone who looked at it, including the dispatcher who raised it. + +**Cluster:** Transports / FHIR egress. **Priority:** P2. **Verdict:** build. **Severity:** no deployment axis -- zero instances; on first deployment a Handler passing an unsanitised value into a FHIR search **would** be able to alter the query's meaning.