Skip to content

fix(threads): sanitize object keys, not just values, before storing a part payload - #6401

Open
pedrofrxncx wants to merge 1 commit into
mainfrom
fix/sanitize-json-keys-thread-parts-w1
Open

fix(threads): sanitize object keys, not just values, before storing a part payload#6401
pedrofrxncx wants to merge 1 commit into
mainfrom
fix/sanitize-json-keys-thread-parts-w1

Conversation

@pedrofrxncx

@pedrofrxncx pedrofrxncx commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Follows the thread_message_parts NUL/lone-surrogate hardening (already-merged, see serializePayload in apps/api/src/storage/thread-message-parts.ts) with a gap I found while auditing that path per this tick's focus area.

Bug: serializePayload's JSON.stringify replacer strips NUL bytes and lone UTF-16 surrogates from string values so a part payload never trips Postgres's SQLSTATE 22P05 ("unsupported Unicode escape sequence") on INSERT. But a JSON replacer function can only substitute what a key maps to — it never gets to rename the key itself. So an object key that carries a NUL byte or an unpaired surrogate (e.g. a tool result keyed by attacker/content-derived strings, not just its values) still serializes to a raw \u0000 escape sequence and hits the exact same 22P05 this code exists to prevent. Per the file's own comment, that failure strands the whole run in_progress forever, since the projection step is the sole writer of terminal thread status.

Fix: adds sanitizeKeysForPg, which rebuilds the payload tree with clean keys before stringifying (values still pass through the existing replacer). Normal keys are byte-identical output — both regexes are no-ops on clean input — so ids derived from the payload stay stable, matching the existing guarantee for values.

Test: two new cases in apps/api/src/storage/thread-message-parts.test.ts (co-located unit test, no DB needed) — a NUL byte in a key and a lone surrogate in a key — mirroring the existing value-sanitization tests in the same file.

Reviewer check: cd apps/api && bun test src/storage/thread-message-parts.test.ts

Locally verified: bun run fmt, bunx tsc --noEmit (apps/api), the targeted test file above (10/10 pass), and bunx oxlint on both touched files (0 errors, 1 pre-existing unrelated warning on the NUL regex literal). Full CI validates the rest.


Summary by cubic

Sanitizes JSON object keys in thread part payloads before storage to prevent Postgres SQLSTATE 22P05. Previously only string values were sanitized; keys with NUL bytes or lone surrogates caused INSERT failures that left runs in_progress.

  • Introduces sanitizeKeysForPg to rebuild the payload with cleaned keys; the existing JSON.stringify replacer still sanitizes string values.
  • Clean inputs are byte-identical; payload-derived IDs remain stable.
  • Adds unit tests for NUL and lone surrogate in keys. Reviewer: cd apps/api && bun test src/storage/thread-message-parts.test.ts
  • No migrations or API changes.

Written for commit 14fdad3. Summary will update on new commits.

Review in cubic

… part payload

serializePayload's JSON.stringify replacer strips NUL bytes and lone
surrogates from string VALUES so a part payload never trips Postgres's
SQLSTATE 22P05 on INSERT. But a replacer function can only substitute a
key's mapped value, never the key text itself — an object key carrying a
NUL byte or an unpaired surrogate (e.g. a tool result keyed by
attacker/content-derived strings) still serializes to a raw \u0000 escape
and hits the exact same 22P05 this code exists to prevent, stranding the
run in_progress forever.

Adds sanitizeKeysForPg to rebuild the payload tree with clean keys before
stringifying. Normal keys are byte-identical (both regexes are no-ops on
clean input), so ids derived from the payload stay stable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant