fix(clawrouter): sanitize attribution headers to ByteString#106454
Conversation
openclaw-barnacle
Bot
added
extensions: clawrouter
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Codex review: needs real behavior proof before merge. Reviewed July 17, 2026, 9:07 PM ET / July 18, 2026, 01:07 UTC. Summary PR surface: Source +28, Tests +79. Total +107 across 2 files. Reproducibility: yes. from source and the supplied regression test: pass Unicode automatic agent/session/request identifiers through the ClawRouter stream wrapper and construct transport headers. A fresh exact-head live run has not been attached to the PR body. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Attach a redacted exact-head production-wrapper transcript showing distinct Unicode agent/session headers, preserved request-ID model suffixes, and successful Do we have a high-confidence way to reproduce the issue? Yes, from source and the supplied regression test: pass Unicode automatic agent/session/request identifiers through the ClawRouter stream wrapper and construct transport headers. A fresh exact-head live run has not been attached to the PR body. Is this the best way to solve the issue? Yes, provisionally: one shared bounded-ID policy avoids duplicate sanitizers while preserving request-ID model suffixes and collision resistance. Exact-head runtime proof is still needed to confirm the proposed behavior. AGENTS.md: unclear because the file could not be read completely. Codex review notes: model internal, reasoning high; reviewed against bcc44ba1b121. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +28, Tests +79. Total +107 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (6 earlier review cycles)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
removed
rating: 🧂 unranked krab
|
@clawsweeper re-review |
clawsweeper
Bot
added
proof: sufficient
fc2b856 to
ec0b35f
Compare
clawsweeper
Bot
added
rating: 🦐 gold shrimp
ec0b35f to
c672496
Compare
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Maintainer review complete. Thanks @ZOOWH for the report and initial fix. Head reviewed: I rewrote the patch around one local policy-driven bounded-ID sanitizer shared by automatic attribution and request IDs. The final path keeps ordinary safe IDs unchanged, preserves request model-call suffixes, reserves generated suffix forms, and hashes original UTF-16 code units so encoded-looking literals and lone surrogates cannot deterministically alias rewritten attribution. Proof:
No config, UI, capability, docs, or public API change. |
|
Merged via squash.
|
…#106454) * fix(clawrouter): sanitize bounded header ids Co-authored-by: ZOOWH <[email protected]> * fix(clawrouter): keep sanitized ids collision-resistant --------- Co-authored-by: Peter Steinberger <[email protected]>
2 participants
What Problem This Solves
ClawRouter automatic agent/session attribution can contain emoji, CJK characters, or other non-ByteString values. Node/Undici converts
Headersvalues to ByteString, so those IDs throw before the model request is sent.Why This Change Was Made
One local policy-driven sanitizer now serves automatic agent, session, and request IDs. Ordinary safe values pass unchanged. Unsafe, encoded-looking, or oversized values become readable printable-ASCII prefixes plus a bounded SHA-256 suffix; request IDs retain their model-call suffix. Hashing uses the original UTF-16 code units so lone surrogates remain distinct. Control-character rejection and explicit operator headers remain unchanged.
This replaces the PR's initial duplicate attribution sanitizer with the existing request-ID strategy generalized behind one path, then closes encoded-namespace and lone-surrogate collision cases found during maintainer review.
User Impact
Before: Unicode automatic attribution could crash request construction with a ByteString
TypeError.After: Automatic IDs are bounded, printable ASCII, collision-resistant after rewriting, and accepted by
Headers.Evidence
Head:
c67249675d9a64cd0d4551189c883ac978850fbbLive production-wrapper proof on Node v26.5.0 with bundled Undici 8.7.0 constructed
Headersfor distinct Unicode agent/session pairs and a Unicode request ID. Rewritten agent/session values remained distinct; the request model suffix remained intact. Regression coverage also proves encoded-looking literal IDs and distinct lone surrogates do not alias generated values.Real behavior proof
Behavior addressed: ClawRouter automatic attribution and request identifiers are sanitized before Node constructs transport headers.
Environment tested: macOS, Node v26.5.0, bundled Undici 8.7.0, production
wrapClawRouterProviderStreamimported from source.Observed result: Unicode agent/session pairs produced distinct printable-ASCII headers; Unicode request ID remained bounded with
:model:3;new Headers(...)succeeded for every captured request.Not tested: Live ClawRouter server-side display. Explicit operator-supplied headers remain untouched by design.