Skip to content

Commit e49d720

Browse files
wshallwshallclaude
andcommitted
fix(store): ADR 0114 sub-lever A — the claim-proc startup gate has never passed
OBJECT_DEFINITION() does not return a CREATE OR ALTER module verbatim: SQL Server deletes the OR and ALTER keyword TOKENS and keeps their separators, so a head submitted as `CREATE OR ALTER PROCEDURE dbo.x` comes back as `CREATE` + three spaces + `PROCEDURE dbo.x` (delta exactly 7; everything after the head is byte-identical). The gate hashed the SUBMITTED text, so it could never match. Sub-lever A has therefore been INERT in every deployment since it shipped — the gate degraded to the ad-hoc batch on every open, and no test noticed because the offline fixture fed the submitted body back as the "deployed" body, modelling the server as an identity function. Both sides of the comparison were the same function of the same argument. Measured on SQL Server 2022 16.0.4255.1 and 2025 17.0.4055.5, compat 130/160/170, across five deploy paths. Live legs go 5-failed/1-passed -> 6 passed. THE FIX IS SHIPPED-SIDE ONLY. The gate compares the deployed hash against a small set of code-controlled constants — the head forms a server may store for a module THIS build deployed (_CLAIM_PROC_STORED_HEADS: the measured `rewritten` form, plus `verbatim` for a hypothetical non-rewriting engine). _claim_proc_body() renders byte-identically, so _claim_proc_ddl, _SCHEMA, _schema_hash() and both golden body pins are untouched: no re-pin, no forced DDL re-apply on any live database. The expected map is keyed PER PROC. A single flat accepted-set would take the cid body served under the dst name — reachable with no tampering intent via sp_rename, which does not rewrite sys.sql_modules.definition — and silently swap the lane predicate, claiming zero rows forever. Head spellings this deploy path cannot emit (CREATE PROC, differing case) keep failing the gate: each is affirmative evidence of an out-of-band hand deploy, which is the AC-7 event. A two-sided canonicalization would launder exactly that signal, and its correctness rests on regex minutiae no finite assertion can pin (mutation-scored 0/7 against this suite; the shipped-side form scores 3/3). ALSO IN THIS COMMIT, because the gate fix makes them reachable: * The proc CALL's 9 parameter pins are PERSISTENT cursor state and were never cleared. At OUTBOUND the same pooled cursor runs the H2 delivery probe, binding an NVARCHAR id against the stale SQL_DOUBLE descriptor for @now FLOAT -> client 22018 -> rollback -> outbound delivery collapse. Fixing the gate alone would arm this for the first operator who sets fifo_claim_proc=true. Cleared the moment the CALL's result is drained. Verified on pyodbc 5.3.0 / ODBC Driver 18 that a ZERO-parameter execute tolerates surplus descriptors, so the shielded `SET LOCK_TIMEOUT -1;` reset in the finally-guard is unaffected and the exposure is exactly the H2 bind chain. * AC-11 no-match parity was honoured only on the two flagged branches. The ad-hoc batch bound the raw lane list into (VALUES (?),...) feeding a `DECLARE @heads TABLE (lane NVARCHAR(256) NOT NULL` — SQL Server evaluates that narrowing conversion on the outer constant scan before the CROSS APPLY filters it, so with ANSI_WARNINGS ON an oversized lane raises 2628 even when zero rows would match. 2628 is not 1222, so it is not translated to EMPTY-all: it rolls back and re-raises. The skip is hoisted ahead of the dispatch-path split so all three branches agree. Clamp still runs before the skip (the tested contract). TESTS — the fix's only evidence. The offline suite could not previously tell a working gate from a broken one; three contradictory implementations all scored 51/51. Both fixture doors are closed (the `_gate_rows` default AND the whitespace test that passed cid_body= explicitly), the "deployed" body now comes from an INDEPENDENT model of the server rewrite with a liveness receipt, and new tests pin the accepted set by set-equality, reject cross-proc substitution, reject un-emittable head spellings, and require the anchor break to degrade diagnosably. The live tamper leg gains the in-test positive control it lacked — under the old defect the gate rejected everything, so its degrade assertions were vacuous. Mutation check: bug 10 failed, flattened-key 3 failed, over-wide 8 failed, pin-clear no-op 1 failed; correct implementation 64 passed. DOCS — three docstrings asserted the false premise ("OBJECT_DEFINITION() preserves the definition text as executed"). The operator-facing degraded reason prescribed `DELETE FROM schema_meta`, which could not work: the re-apply submits the same text and mismatches again, forever. ADR 0114 amended; AC-7b added, because AC-7 is one-directional and was LITERALLY SATISFIED by the defect — nothing anywhere required a correctly deployed proc to pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7128f5b commit e49d720

6 files changed

Lines changed: 564 additions & 88 deletions

docs/CONFIGURATION.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ backend-limited.
7070
| `group_commit_max_batch` | int | `64` | **SQLite only.** Flush threshold for the group-commit committer: once this many members are enrolled in the open batch it commits immediately rather than waiting out the rest of `group_commit_window_ms`, bounding batch size + latency under load. Ignored when group-commit is off (`group_commit_window_ms = 0`). |
7171
| `fifo_claim_batch` | int | `1` | all backends (ADR 0058). Max rows the **INGRESS/ROUTED** FIFO claim takes per commit. `1` = **OFF** (the workers claim one row per commit — byte-identical to before). `> 1` (clamped `1..64`) claims the **contiguous due head-prefix** in one commit and then processes each row in strict FIFO order with its own off-loop route/transform + separate handoff, amortizing the standalone claim commit toward 1/N. A not-due or producer-locked head still blocks the lane (strict per-lane FIFO, #285). The **outbound/delivery** claim is never batched. Opt-in throughput tuning (recommend `8``16`); size against worst-case message size, since N decrypted bodies are resident per lane between the claim and the N handoffs. |
7272
| `fifo_claim_fold_reset` | bool | `false` | **SQL Server only** ([ADR 0114](adr/0114-phase-4-claim-path-call-complexity-reduction-driver-interface-redesign-ingress-routed-reset-fold.md) sub-lever C). Folds the pooled claim's session `LOCK_TIMEOUT` reset into the claim batch on the **clean success path at INGRESS/ROUTED** (the write-less commit#2 disappears; the shielded finally-guard still runs on every non-clean exit — 1222, kept≠claimed, cancellation, any error). OUTBOUND/RESPONSE are never folded. `false` = **byte-identical** shipped batch + guard. Flip only after its own ADR 0114 §8 bench gate (AC-14). |
73-
| `fifo_claim_proc` | bool | `false` | **SQL Server only** (ADR 0114 sub-lever A). Executes the pooled claim via the two lane-family versioned procs `dbo.mefor_claim_fifo_heads_cid_v1` / `_dst_v1` (fixed-arity `{CALL}`, one JSON lanes parameter) instead of the ~3 KB ad-hoc batch. Needs database `COMPATIBILITY_LEVEL >= 130` (SQL Server 2016); **fails safe to the batch, loudly**, when the procs are missing, hand-edited (body-hash mismatch), or compat < 130 — never a lane outage. A hardened split-principal deployment must `GRANT EXECUTE` on both procs to the runtime principal (the bootstrap principal owns them). `false` = byte-identical. Flip only after its own §8 gate (AC-14). |
73+
| `fifo_claim_proc` | bool | `false` | **SQL Server only** (ADR 0114 sub-lever A). Executes the pooled claim via the two lane-family versioned procs `dbo.mefor_claim_fifo_heads_cid_v1` / `_dst_v1` (fixed-arity `{CALL}`, one JSON lanes parameter) instead of the ~3 KB ad-hoc batch. Needs database `COMPATIBILITY_LEVEL >= 130` (SQL Server 2016); **fails safe to the batch, loudly**, whenever the startup gate cannot verify both deployed bodies against this build — at least: a missing proc, a body matching no form this build deploys (hand edit, hand deploy, or a body changed without bumping the `_v1` proc name), a definition this principal cannot read (no `VIEW DEFINITION`, or `WITH ENCRYPTION`), or compat < 130 — never a lane outage. A hardened split-principal deployment must `GRANT EXECUTE` on both procs to the runtime principal (the bootstrap principal owns them), and `GRANT VIEW DEFINITION` so the gate can read the bodies it verifies. `false` = byte-identical. Flip only after its own §8 gate (AC-14). |
7474
| `fifo_claim_prepared` | bool | `false` | **SQL Server only** (ADR 0114 sub-lever B). Stabilizes the pooled claim's statement text (one JSON lanes parameter) and retains a prepared claim cursor on store-owned dedicated connections (INGRESS/ROUTED; the non-DDL fallback lane to `fifo_claim_proc`). **Logs + no-ops unless `fifo_claim_fold_reset` is on** (without the fold the finally-guard's reset would evict the one-slot prepare cache every call). `false` = byte-identical. Flip only after its own §8 gate (AC-14). |
7575
| `encryption_key` | secret || **env only** (`MEFOR_STORE_ENCRYPTION_KEY`); base64 32-byte **active** key — when set, PHI columns (`raw`/`payload` + `error`/`last_error`/`detail`) are AES-256-GCM-encrypted at rest. Mint one with `messagefoundry gen-key`. Empty = off. See [PHI.md §3](PHI.md#3-encryption-at-rest). |
7676
| `encryption_keys_retired` | secret || **env only** (`MEFOR_STORE_ENCRYPTION_KEYS_RETIRED`); comma-separated base64 **decrypt-only** keys kept available during a rotation until `messagefoundry rotate-key` finishes re-encrypting under the active key (ASVS 11.2.2). |

docs/adr/0114-phase-4-claim-path-call-complexity-reduction-driver-interface-redesign-ingress-routed-reset-fold.md

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -395,12 +395,47 @@ per-database property, not a server version; `CREATE OR ALTER` needs 2016 SP1. A
395395
compat-120 database and under a DDL-denied principal.
396396

397397
**Startup gate (fail-safe to the batch, loudly).** With `fifo_claim_proc` ON, `open()` probes: (a)
398-
`OBJECT_ID` of **both** procs; (b) a SHA-256 of each deployed body via `OBJECT_DEFINITION()` against the shipped
399-
DDL text (normalized) — **existence alone cannot catch a hand-edited body**, and the ADR 0064 marker covers only
400-
in-repo edits, while the proc *is* the claim logic; (c) `compatibility_level ≥ 130`. Any failure → the store
401-
records `claim_proc_effective = False`, logs a **WARNING naming the reason**, sets a degraded gauge, and runs
402-
the shipped batch — never a lane outage; the hot path contains **no error-2812 handling**. Out-of-band drift is
403-
caught at the next open; `DELETE FROM schema_meta` forces a full re-create.
398+
`OBJECT_ID` of **both** procs; (b) a SHA-256 of each deployed body via `OBJECT_DEFINITION()` against the
399+
**stored forms** of the shipped DDL text (normalized) — **existence alone cannot catch a hand-edited body**, and
400+
the ADR 0064 marker covers only in-repo edits, while the proc *is* the claim logic; (c) `compatibility_level ≥
401+
130`. Any failure → the store records `claim_proc_effective = False`, logs a **WARNING naming the reason** and
402+
runs the shipped batch — never a lane outage; the hot path contains **no error-2812 handling**. Out-of-band
403+
drift is caught at the next open.
404+
405+
> **AMENDMENT (2026-07-30) — `OBJECT_DEFINITION()` does not return the submitted text, and this gate was
406+
> inert until it was fixed.**
407+
>
408+
> SQL Server does not store a `CREATE OR ALTER` module verbatim: it **deletes the `OR` and `ALTER` keyword
409+
> tokens and keeps their separators**, so a head submitted as `CREATE OR ALTER PROCEDURE dbo.x` is returned by
410+
> `OBJECT_DEFINITION()` as `CREATE` + three spaces + `PROCEDURE dbo.x` (character delta exactly 7; everything
411+
> after the head byte-identical). MEASURED on SQL Server 2022 16.0.4255.1 and 2025 17.0.4055.5, compat
412+
> 130/160/170, across five deploy paths (fresh `CREATE`, the `OR ALTER` re-apply, a plain batch, inside the
413+
> shipped guarded `EXEC(N'…')`, and an out-of-band `ALTER PROCEDURE` — which the engine also rewrites, to a
414+
> single-spaced `CREATE PROCEDURE`). Case is preserved, not folded; `PROC` survives as `CREATE PROC`.
415+
>
416+
> Because the gate as originally implemented hashed the **submitted** text, it **could never pass for a proc
417+
> deployed by `_claim_proc_ddl`, on any engine that function can deploy to** — sub-lever A degraded to the batch
418+
> on every open, in every deployment, from the feature shipping until this amendment. The lever was inert, not
419+
> merely unused. (Scope note: this is a statement about *this* deploy path, not about every conceivable module.)
420+
>
421+
> The fix is **shipped-side only**: the gate now compares the deployed hash against a small set of
422+
> code-controlled constants — the head forms a server may store for a module *this build* deployed
423+
> (`_CLAIM_PROC_STORED_HEADS`: the measured `rewritten` form, plus the `verbatim` form for a hypothetical
424+
> non-rewriting engine). `_claim_proc_body()` renders byte-identically, so `_claim_proc_ddl`, `_SCHEMA`,
425+
> `_schema_hash()` and the golden body pins are untouched: **no re-pin and no forced DDL re-apply on any live
426+
> database.** The expected map is keyed **per proc**, so the cid body served under the dst name (reachable via
427+
> `sp_rename`, which does not rewrite `sys.sql_modules.definition`) degrades rather than silently swapping the
428+
> lane predicate. Head spellings this deploy path cannot emit (`CREATE PROC`, differing case) keep failing the
429+
> gate: each is affirmative evidence of an out-of-band hand deploy, which is the AC-7 event.
430+
>
431+
> The accepted set is exactly two constants over **normalized** text. It is *not* two byte strings —
432+
> `_normalize_tsql` still applies to the deployed side, so its whitespace collapse remains semantically lossy
433+
> inside comments and string literals. That is contained by the AC-8 body lint (no quotes, no `--`, no `/*`,
434+
> ASCII-only), which is now load-bearing rather than defensive.
435+
>
436+
> **`DELETE FROM schema_meta` was previously prescribed here as the remedy for a body mismatch. It could not
437+
> work** — the re-apply submits the same text, the engine rewrites it the same way, and the hash mismatches
438+
> again — so the advice has been removed from the ADR and from the operator-facing degraded reason.
404439
405440
**Versioning, mixed vintages, downgrade.** Procs are **name-versioned** (`_v1`, `_v2`, …): engine sharding runs
406441
N processes against ONE unified store (ADR 0037/0063), so a rolling upgrade briefly runs two builds against one
@@ -413,6 +448,9 @@ repeated "schema DDL batch applied" lines mid-rollback knows it is expected. **G
413448
owns the procs (EXECUTE implicit via ownership); a hardened split-principal deployment must `GRANT EXECUTE` — an
414449
ops-doc line, not a code path. **Two-copies drift** (batch vs proc bodies) is contained by the content hash +
415450
the body-definition probe + a lint test diffing the proc DDL's statement sequence against the batch construction.
451+
(Until the 2026-07-30 amendment the body-definition probe was **not** a real compensating control: it compared
452+
against text no server could return, so its verdict was constant and a genuine tamper was indistinguishable from
453+
baseline. The content hash and the DDL-vs-batch lint were carrying that containment alone.)
416454

417455
### 5. Sub-lever B — stable statement text + a retained prepared claim cursor (the non-DDL fallback lane)
418456

@@ -641,10 +679,18 @@ states, including the mismatch and 1222 translations). **Any miss = the flag sta
641679
injected-row test.
642680
- **AC-6** — The three flags SHALL be provable no-ops on SQLite and Postgres (neither backend references them).
643681
→ sentinel test (the ADR 0075 precedent).
644-
- **AC-7** — WHEN `fifo_claim_proc` is ON and a proc is missing, its `OBJECT_DEFINITION` hash mismatches the
645-
shipped body, or compat < 130, the store SHALL degrade loudly to the shipped batch (WARNING naming the reason
646-
+ degraded gauge), never a lane outage; the hot path SHALL contain no error-2812 handling. → startup-gate
647-
tests incl. a hand-edited-body leg.
682+
- **AC-7** — WHEN `fifo_claim_proc` is ON and a proc is missing, its `OBJECT_DEFINITION` hash mismatches every
683+
form this build deploys, or compat < 130, the store SHALL degrade loudly to the shipped batch (WARNING naming
684+
the reason + degraded gauge), never a lane outage; the hot path SHALL contain no error-2812 handling. →
685+
startup-gate tests incl. a hand-edited-body leg.
686+
- **AC-7b** — WHEN `fifo_claim_proc` is ON and both procs are deployed **by this build's own DDL**, the gate
687+
SHALL **PASS** and `claim_proc_effective` SHALL be True, verified against a **real SQL Server** (not a stub
688+
that echoes the submitted text back as the deployed body). → `test_adr0114_claim_proc_live.py`, plus an
689+
offline round-trip whose "deployed" fixture independently models the engine's module rewrite.
690+
> Added by the 2026-07-30 amendment. AC-7 as originally written is **one-directional** — it requires the gate
691+
> to degrade when the body mismatches, and nothing anywhere required a *correctly deployed* proc to pass. The
692+
> shipped defect therefore **satisfied AC-7 literally** while leaving the lever inert, and AC review could not
693+
> have caught it. Any future gate-shaped AC needs both directions or it is not a gate.
648694
- **AC-8** — The proc bodies SHALL contain no `BEGIN/COMMIT/ROLLBACK`, no `TRY/CATCH`, no `SET XACT_ABORT`, and
649695
no `LOCK_TIMEOUT` reset outside the `@fold_reset` tail; `@@TRANCOUNT` on exit SHALL equal entry. → DDL lint
650696
test + a trancount probe test.

messagefoundry/config/settings.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,10 @@ class StoreSettings(_Section):
327327
description=(
328328
"Execute the pooled claim via the two lane-family versioned procs "
329329
"(dbo.mefor_claim_fifo_heads_cid_v1/_dst_v1; fixed-arity CALL) instead of the ~3KB ad-hoc "
330-
"batch. Fails safe to the batch (loud) if the procs are missing/stale or compat < 130. "
331-
"SQL Server only; OFF = byte-identical."
330+
"batch. Fails safe to the batch (loud) whenever the startup gate cannot verify both "
331+
"deployed bodies against this build — at least: a missing proc, a body matching no form "
332+
"this build deploys, a definition this principal cannot read (no VIEW DEFINITION, or "
333+
"WITH ENCRYPTION), or compat < 130. SQL Server only; OFF = byte-identical."
332334
),
333335
)
334336
fifo_claim_prepared: bool = Field(

0 commit comments

Comments
 (0)