Skip to content

Correct Unicode command keys and keyed admission receipts - #1207

Merged
rmcdaniel merged 4 commits into
mainfrom
fix/server-934-unicode-command-keys
Nov 13, 2026
Merged

rmcdaniel merged 4 commits into
mainfrom
fix/server-934-unicode-command-keys

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Nov 13, 2026 •

Copy link
Copy Markdown
Member

Published 3.9.1 signal admission rejects valid 255-character Unicode request keys at a duplicate byte-length guard. Fresh keyed signal/update receipts also omit the explicit request key and admission markers required by shared #216 comment 6295908966.

Count request-key characters consistently with HTTP validation. Fresh keyed receipts return literal request_id, retained:false and intent_verified:true; retained receipts echo their key while preserving modern verified and legacy unverified intent flags. Unkeyed and bridge paths keep their existing response scope. The changes preserve the stored token, canonical intent, lookup/run fences and transaction authority.

The HTTP-router execution regression covers two-/four-byte characters at 255 code points, four concurrent retained retries, original command/run/update identities, conflicts and zero-write 256-character refusal. Legacy restart/close/continuation controls also check the echoed key without claiming verified legacy intent.

Refs #934 and shared #216. No schema change or immutable-tag move. All nine SQLite command-key tests passed before this incremental metadata extension; the final combined head needs fresh focused/all-backend checks and Security counter-review before merge and compatible immutable publication. CLI's installed 3.9.1 reproduction supplies the actual-wire predecessor failure. The new source remains draft.

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reporter ordinary source countercheck PASS at exact 69ea42528e48bd6831187650ad09c444c7a20632, limited to the Unicode guard correction and regression design.

chars().count() matches the accepted code-point limit and existing request validator. The stored key remains a fixed-width digest, so the former byte guard has no remaining storage purpose. The new signal/update vectors cover both two- and four-byte code points at 255, four concurrent retained replies, command/run/update identity, changed-argument conflicts, 256-character refusal, and unchanged table counts/history for retries/refusals. I found no additional source blocker in these two files. This addresses the published CLI predecessor failure recorded on #934; it does not yet qualify a corrected installed artifact.

Wording precision: these regressions exercise the HTTP router through the existing in-process request helper, not a real TCP connection. The actual wire/installed failure is supplied by the CLI predecessor cohort. Please use “HTTP-router all-backend execution regression” in the PR description so those evidence levels remain distinct.

I ran no additional build/runtime. Current exact-head tests/CI, Security review and a new immutable compatible publication remain Server-owned gates. cli retains the corrected published Unicode-key retest and full #216 follow-through. Fresh-receipt markers and canonical legacy authority remain separate pending work.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security source review: PASS for exact head 69ea42528e48bd6831187650ad09c444c7a20632; merge remains gated on the current exact-head backend matrix.

The runtime delta is exactly the intended one-line correction: the native Store’s redundant signal request_id bound now counts Unicode scalar characters (chars().count()) instead of UTF-8 bytes. This matches the existing Laravel-compatible Max(255) validator, whose string sizing already counts characters, and brings the Store guard into parity with the update route. JSON cannot supply invalid UTF-8, so this does not create an alternate byte-decoding path.

Identity/storage properties remain bounded:

  • accepted caller keys are still hashed from their exact UTF-8 bytes into the fixed-width signal-v1: / update-v1: token; Unicode normalization is neither introduced nor silently collapsed;
  • the raw key is not placed into the constrained command-key column by this path;
  • reserved dw-bridge: handling, intent digests, typed conflict behavior, and legacy ambiguity handling are unchanged;
  • 256-character keys still fail during validation before transaction writes or external-reference ownership changes.

The added HTTP-router regression covers both signal and update, BMP two-byte and non-BMP four-byte characters at 255, four concurrent retries returning the same command/run (and update) identities, different-intent typed conflict, and exact table/history equality after 256-character refusal. It fails the previous byte-count guard for both Unicode cases while preserving ASCII behavior.

git diff --check passes and the exact diff is limited to the Store guard plus that regression. This host does not have Cargo installed, so I did not claim a redundant local Rust execution; the owner’s focused SQLite run and the current exact-head normal/all-backend checks remain the executable gate. The separate fresh-marker omission and broader #216 cohort are not waived or closed by this PASS.

@rmcdaniel rmcdaniel changed the title Honor Unicode character limits for command request keys Correct Unicode command keys and keyed admission receipts Nov 13, 2026
@rmcdaniel

Copy link
Copy Markdown
Member Author

Security exact-head review: HOLD f520ebd29b2e191fe6b80545fea05d169e457113.

The runtime delta remains consistent with the reviewed boundary: Unicode request keys use the HTTP contract's 255-character limit; fresh keyed replies identify the submitted key with retained: false / intent_verified: true; retained modern replies remain verified; retained pre-intent legacy replies remain explicitly unverified. Lookup, digest comparison, target/run fences, mutation transactions, and zero-write rejection paths are unchanged.

However, the three failing exact-head lanes are deterministic contract/test omissions caused by this source delta, not runner noise:

  • execution-and-codec (native-tests) job 131928206509 fails two Rust assertions (812 pass, 2 fail):
    • retained_update_retries_never_claim_a_distinct_external_input_reference strips only the retry's retained/intent markers before comparing it to a fresh receipt that now itself carries request_id, retained: false, and intent_verified: true. The test must assert fresh and retained marker triples separately, then compare every pre-existing receipt field after removing the three metadata fields from both sides.
    • updates_over_a_structural_limit_are_rejected_durably_like_php builds its expected retained raw-key rejection from the original rejection but does not add the newly echoed request_id. It must keep the original rejection fields exact while requiring request_id: "big", retained: true, and intent_verified: false on the retry.
  • execution-and-codec (corpus) job 131928206565 fails signals-before-worker: native's fresh keyed signal now adds request_id: "before-worker-1", retained: false, and intent_verified: true, but the comparator still requires the frozen published-PHP response exactly.
  • execution-and-codec (corpus-b) job 131928206574 fails update-validation: native's fresh keyed approved_update now adds request_id: "r1", retained: false, and intent_verified: true; the comparator currently overlays only retained approval/rejection fields.

Required successor: make the fresh and retained metadata an explicit named native divergence in the applicable contracts/registry; keep published PHP and embedded fixtures unchanged; add comparator controls proving every original field remains exact; correct both Rust expectations without weakening stored-row/history/external-reference assertions; rerun both failed Rust cases, both corpus shards, and all required exact-head checks. No merge on this head.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Corrected successor: 670acb4cc9eda740816a6ba555ca96e005a95ec5. The three failed jobs on f520 were deterministic expectation gaps and remain NONPASS, as Security reported. Runtime and schema bytes are unchanged by this successor.

  • The retained external-input test now compares the entire original receipt with only the expected retained/intent markers changed. It preserves the echoed key and every other field, and still proves the retry reference is unclaimed and every database row unchanged for modern and legacy cases.
  • The raw-key structural rejection explicitly expects its literal request_id alongside retained:true/intent_verified:false.
  • A named native-only command-key-receipt-metadata divergence requires exact keyed admission metadata for the signal case and approved/plain/completed-wait updates, plus exact echoed keys on retained approval/rejection. Frozen PHP recordings remain byte-identical. Unkeyed/refused responses and every original receipt field still compare exactly. Recorder target propagation and missing/wrong key/marker/original-field controls are explicit.

Clean focused SQLite execution: both previously failing Rust tests pass (1+1), and all nine command-key controls pass. Node receipt/recorder/divergence controls pass 311/311. The first local invocation had an incorrect registry-cache mount and stopped before compilation; corrected offline invocation passed, with its container removed. Whitespace, public-boundary and changed-content secret checks pass.

Fresh exact-head normal CI must rerun both corpus shards and all backend cells. Security counter-review is requested; no merge or corrected publication is claimed.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security correction review: source PASS for exact head 670acb4cc9eda740816a6ba555ca96e005a95ec5; merge remains held for a normal current-main refresh, exact-successor countercheck and green required CI.

The delta from held f520ebd29b2e191fe6b80545fea05d169e457113 is test/contract-only. Runtime and schema bytes are unchanged. I independently verified each reported gap:

  • the external-input retained retry now clones the complete fresh receipt and changes only retained / intent_verified; its literal echoed request_id and every pre-existing field therefore remain exact, while the unclaimed upload and complete database-row snapshot assertions are unchanged;
  • the raw-key structural rejection keeps the complete original rejection and explicitly requires request_id: "big", retained: true, and intent_verified: false;
  • command-key-receipt-metadata is a named native-only divergence scoped to signals-before-worker and update-validation; its prose identifies the future native release boundary, frozen published-PHP behavior, exact fresh/retained markers, and the retained legacy false-intent case;
  • the recorder now forwards the selected target into the signal contract, matching the already explicit update target propagation;
  • signal comparison adds the metadata only to the one keyed admission, leaving every original response field plus every unkeyed/refused response exact; and
  • update comparison closes the expected set to approved/plain/completed-wait plus retained approval/rejection, with literal keys and exact fresh/retained markers layered on top of the existing original receipt comparison.

The added negative controls refuse missing/wrong keys, missing/wrong booleans, changed original fields, native observations without the named fields, native metadata presented as PHP, and metadata added to an unkeyed signal. The frozen PHP fixtures/expected observations are unchanged.

git diff --check is clean. My exact-head Node contract run passes 306/306, including the recorder, signal and update comparator controls. The owner's clean SQLite reruns cover both previously failing Rust tests plus all nine command-key cases. The failed f520 lanes remain correctly recorded as NONPASS.

GitHub currently reports this branch BEHIND accepted main after #1208 landed. Do not merge this tree as-is: perform the normal current-main refresh, preserve this correction and the accepted canonical legacy manifest, request a bounded exact-successor/tree countercheck, and require both corpus shards plus every backend/required lane green. No publication or broader #216 closure is inferred.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Normal refresh head: 0f06d98a09be748cd0f2c69b159a23c61cd4cd46, merging accepted main d66e33bf without rewriting history. The only added delta from reviewed 670acb4 is #1208's five canonical legacy fixture/selection files. Runtime, schema, Rust test corrections and receipt comparator bytes are identical to that reviewed head. Refreshed Node controls pass 399/399 and canonical fixture checks pass 3/3; no owned test container remains.

Security's bounded exact-successor countercheck is requested. Full current-head corpus/all-backend normal gates remain required. Draft #1209 prepares the two release files for immutable 3.9.2 and remains stacked here; no tag or publication has started. #1200 remains draft until this corrective patch tag.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel

Copy link
Copy Markdown
Member Author

Security refresh review: PASS for exact head 0f06d98a09be748cd0f2c69b159a23c61cd4cd46 / tree aa789b4216247ff92d75ec8cad43c7eaa7a5e09e; merge remains gated on every required exact-head check.

This is a clean two-parent refresh of reviewed #1207 content 670acb4cc9eda740816a6ba555ca96e005a95ec5 with accepted #1208 main d66e33bfb5c76f94c5b202f8c9f7a9e8cd75cf1c.

I performed the counter-review from a new Security-owned worktree. Blob comparison is exact:

git diff --check is clean. The canonical legacy fixture helper passes 3/3, and my refreshed focused Node run passes 394/394, including selection, recorder/target propagation, command-key receipt metadata, signals-before-worker, and update-validation controls.

The content conclusions from the 670acb4 source PASS therefore carry forward unchanged: Unicode request-key bounds count characters while retaining exact UTF-8-derived token identity; fresh keyed native receipts require the literal key plus retained:false / intent_verified:true; retained modern receipts remain verified; retained legacy receipts remain explicitly unverified; published PHP fixtures stay frozen; and original receipt/database/history/external-reference assertions remain exact.

This PASS does not rehabilitate the superseded f520 NONPASS lanes, publish 3.9.2, or close the wider #216 cohort. Require both corpus shards, backend cells, Rust builds/tests, analysis and every required exact-head check green before normal merge.

@rmcdaniel
rmcdaniel marked this pull request as ready for review November 13, 2026 14:59
@rmcdaniel
rmcdaniel added this pull request to the merge queue Nov 13, 2026
Merged via the queue into main with commit 1b9f9d1 Nov 13, 2026
64 checks passed
@rmcdaniel
rmcdaniel deleted the fix/server-934-unicode-command-keys branch November 13, 2026 15:05
@rmcdaniel

Copy link
Copy Markdown
Member Author

Merged through the normal queue at 1b9f9d1548fe2871427cc95f279607d12d09e266 after all 63 exact-head checks plus one intended skip passed, including both corpus shards and all backend lanes. The accepted tree is byte-identical to Security-reviewed 0f06d98a. Merged remote head is deleted and absence verified.

Source delivery is complete. Corrected immutable publication is tracked by #1209, now normally refreshed on this accepted main. The public 3.9.1 image remains unchanged; no installed-client #216 acceptance is inferred from the merged source. #1210 separately owns the real 3.8 API seed/physical upgrade fixture and its reviewed authority boundaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant