fix(agents): strip C1 control characters (0x80-0x9f) in sanitizeForConsole by lsr911 · Pull Request #103226 · openclaw/openclaw

lsr911 · 2026-07-10T01:29:16Z

What Problem This Solves

sanitizeForConsole() filters C0 control characters (0x00-0x1f) and DEL (0x7f) but leaves the C1 control range (0x80-0x9f) unhandled. The C1 range includes 0x9b (CSI), an alternative ANSI escape prefix.

Why This Change Was Made

Add (code >= 0x80 && code <= 0x9f) to the control character filter, covering all 32 C1 bytes. Same pattern as the merged #103274.

Evidence

Standalone proof

$ node --import tsx test/_proof_console_sanitize_c1.mjs
node=24.12.0
input: text<0x9b>[31mred
output: text[31mred
CSI stripped: true
C1 range 0x80-0x9f all stripped: true
PASS

Files Changed (1 file, +2/-1)

🤖 Generated with Claude Code

…nsole sanitizeForConsole filters C0 control characters (0x00-0x1f) and DEL (0x7f) but did not filter C1 controls (0x80-0x9f). The C1 range includes the CSI (Control Sequence Introducer) at 0x9b, which is an alternative prefix for ANSI escape sequences (equivalent to ESC [). An attacker who controls text routed through sanitizeForConsole could inject C1-based ANSI escapes into console/log output. Extend the filter to include the full C1 range (0x80-0x9f). Co-Authored-By: Claude <[email protected]>

…nalBufferText (#103274) renderTerminalBufferText filters C0 control characters (0x00-0x1f) and DEL (0x7f) but did not filter C1 controls (0x80-0x9f). The C1 range includes the CSI (Control Sequence Introducer) at 0x9b, an alternative ANSI escape prefix that stripAnsiSequences() may miss. Add the C1 range to CONTROL_BYTES_REGEX to strip all C1 bytes. Same pattern as the C1 fix in sanitizeForConsole (#103226). Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

…nalBufferText (openclaw#103274) renderTerminalBufferText filters C0 control characters (0x00-0x1f) and DEL (0x7f) but did not filter C1 controls (0x80-0x9f). The C1 range includes the CSI (Control Sequence Introducer) at 0x9b, an alternative ANSI escape prefix that stripAnsiSequences() may miss. Add the C1 range to CONTROL_BYTES_REGEX to strip all C1 bytes. Same pattern as the C1 fix in sanitizeForConsole (openclaw#103226). Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

clawsweeper · 2026-07-10T09:25:03Z

Codex review: needs real behavior proof before merge. Reviewed July 10, 2026, 8:10 AM ET / 12:10 UTC.

Summary
The PR extends sanitizeForConsole to remove U+0080–U+009F, adds focused unit coverage, and adds two standalone proof scripts.

PR surface: Source +3, Tests +64. Total +67 across 4 files.

Reproducibility: yes. at source level with high confidence: current main deterministically preserves U+0080–U+009F in sanitizeForConsole. The review did not establish a failing real-terminal path using a valid C1 sequence.

Review metrics: none identified.

Root-cause cluster
Relationship: canonical
Canonical: #103226
Summary: This PR is the canonical candidate for the sanitizeForConsole gap; the related PRs apply the same general invariant to distinct sinks and validators.

Members:

adjacent_distinct: fix(terminal): strip C1 control characters (0x80-0x9f) in renderTerminalBufferText #103274 - Merged C1 filtering for terminal-buffer rendering at a separate output boundary.
adjacent_distinct: fix(agents): detect C1 control characters in hasControlCharacter #103379 - Targets failover-error control-character classification rather than this output helper.
adjacent_distinct: fix(agents): detect C1 control characters in sanitizeModelWarningValue #103380 - Targets model-warning sanitization rather than compact console output.
adjacent_distinct: fix(sandbox): detect C1 control characters in hasUnsafeControlChars #103381 - Targets sandbox unsafe-character validation.
adjacent_distinct: fix(agents): detect C1 control characters in attachment name validation #103382 - Targets attachment-name validation.
1 more in the report.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Correct the maintained CSI regression and exact-head terminal evidence.
Delete both standalone proof scripts.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The exact-head terminal transcript invokes production code, but its malformed ${csi}[31m input does not prove a valid C1 CSI path; provide redacted escaped-byte or code-point output, update the PR body, and request @clawsweeper re-review if automatic review does not run.

Next step before merge

[P1] The three remaining blockers are concrete mechanical repairs that do not require product, ownership, or security judgment.

Security
Cleared: The functional change tightens a local output-sanitization boundary and adds no dependency, workflow, permission, secret, downloaded-code, or supply-chain exposure.

Review findings

[P2] Exercise a valid C1 CSI sequence — src/agents/console-sanitize.test.ts:23
[P2] Remove the unrelated voice-wake proof script — test/_proof_voice_wake_utf16.mjs:1
[P2] Keep the C1 proof in the maintained test suite — test/_proof_console_sanitize_c1.mjs:1-3

Review details

Best possible solution:

Keep the local helper because its compact-console semantics differ from sanitizeForLog, add C1 removal with a valid U+009B-plus-parameters regression, remove both proof-only scripts, and attach redacted exact-head code-point or escaped-byte output.

Do we have a high-confidence way to reproduce the issue?

Yes at source level with high confidence: current main deterministically preserves U+0080–U+009F in sanitizeForConsole. The review did not establish a failing real-terminal path using a valid C1 sequence.

Is this the best way to solve the issue?

No, not as submitted. The local predicate is the right implementation boundary, but the maintained regression must use a valid U+009B CSI form and the two proof-only files should not be merged.

Full review comments:

[P2] Exercise a valid C1 CSI sequence — src/agents/console-sanitize.test.ts:23
U+009B already represents the complete CSI introducer, so ${csi}[31m inserts an extra [ and does not test the claimed sequence. Use ${csi}31m in the maintained regression; this prior blocker remains unchanged on the current head.
Confidence: 0.99
[P2] Remove the unrelated voice-wake proof script — test/_proof_voice_wake_utf16.mjs:1
This file tests a separate voice-wake UTF-16 behavior and has no connection to console C1 sanitization. It remains unchanged since the prior review and should not ship in this PR.
Confidence: 1
[P2] Keep the C1 proof in the maintained test suite — test/_proof_console_sanitize_c1.mjs:1-3
This standalone script duplicates the adjacent Vitest coverage without becoming part of the maintained suite, adding repository surface without durable CI value. Put the corrected vector in src/agents/console-sanitize.test.ts, retain terminal output as PR evidence, and remove this file; the prior blocker remains after the script-only rewrite.
Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against a13d2c4c9369.

Label changes

Label justifications:

P2: This is a bounded diagnostic-output sanitization bug whose remaining blockers are localized to test correctness, proof quality, and branch scope.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The exact-head terminal transcript invokes production code, but its malformed ${csi}[31m input does not prove a valid C1 CSI path; provide redacted escaped-byte or code-point output, update the PR body, and request @clawsweeper re-review if automatic review does not run.

Evidence reviewed

PR surface:

Source +3, Tests +64. Total +67 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	5	2	+3
Tests	3	64	0	+64
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	4	69	2	+67

Acceptance criteria:

[P1] pnpm test src/agents/console-sanitize.test.ts.
[P1] git diff --check.

What I checked:

Current-main behavior: Current main removes selected C0 characters and DEL but has no predicate for U+0080–U+009F, so the reported source-level gap remains present. (src/agents/console-sanitize.ts:17, a13d2c4c9369)
Actual runtime callers: The helper sanitizes run IDs, provider/model names, session identifiers, tool argument previews, and observed error previews before diagnostic output, confirming that this is a real output boundary rather than unused utility code. (src/agents/embedded-agent-subscribe.handlers.lifecycle.ts:150, a13d2c4c9369)
Prior-finding continuity: Between the previously reviewed SHA and the current head, only the standalone C1 proof script changed; the malformed maintained test vector and unrelated voice-wake proof script are unchanged, while the standalone script itself remains committed. (src/agents/console-sanitize.test.ts:23, 9c90039dd238)
Exact branch scope: The three-commit PR series changes the helper and maintained test, then adds two proof-only scripts, including a script for an unrelated voice-wake behavior. (test/_proof_voice_wake_utf16.mjs:1, 9c90039dd238)
Best-fix comparison: The shared sanitizeForLog already strips ANSI and all C0/C1 controls, but it also deletes tabs/newlines and lacks this helper's whitespace-flattening, optional-value, and length-cap semantics; retaining the small local helper is therefore justified. (packages/terminal-core/src/ansi.ts:97, a13d2c4c9369)
Accepted sibling invariant: Merged commit a3f9f35 applies C1 removal to terminal-buffer rendering at a distinct output boundary, supporting the proposed invariant without superseding this helper. (src/gateway/terminal/buffer-text.ts:11, a3f9f3567f71)

Likely related people:

joshavant: Commit fc60a8e introduced sanitizeForConsole, its current semantics, and the focused test on main. (role: introduced current behavior; confidence: high; commits: fc60a8e66b05; files: src/agents/console-sanitize.ts, src/agents/console-sanitize.test.ts)
lsr911: In addition to proposing this branch, they authored merged commit a3f9f35, which established the same C1-removal invariant at the terminal-buffer output boundary. (role: adjacent sanitizer contributor; confidence: medium; commits: a3f9f3567f71; files: src/gateway/terminal/buffer-text.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (1 earlier review cycle)

reviewed 2026-07-10T09:23:14.338Z sha 7781265 :: needs real behavior proof before merge. :: [P2] Exercise a valid C1 CSI sequence | [P2] Remove the unrelated voice-wake proof script | [P2] Keep the C1 proof in the real test suite

ClawSweeper review: the console-sanitization proof script belongs to openclaw#103226, not this gateway voice-wake fix. Remove it to keep the branch clean. Ref: openclaw#103210 review

lsr911 · 2026-07-10T10:07:03Z

@clawsweeper re-review

clawsweeper · 2026-07-10T10:07:06Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/29091368929
Updated: 2026-07-10T12:12:10.054Z

ClawSweeper review: the console-sanitization proof script belongs to openclaw#103226, not this gateway voice-wake fix. Remove it to keep the branch clean. Ref: openclaw#103210 review

…#103210) * fix(gateway): use truncateUtf16Safe for voice-wake trigger truncation Replace naive .slice(0, 64) with truncateUtf16Safe() in normalizeVoiceWakeTriggers to prevent surrogate pair splitting in user-configured voice wake trigger phrases. Voice wake triggers are user-configurable text strings that may contain emoji or non-BMP characters. A naive .slice(0, 64) at a surrogate pair boundary produces a lone surrogate, which corrupts the trigger text. Co-Authored-By: Claude <[email protected]> * test: add proof scripts for C1 sanitize and voice-wake UTF-16 * chore: remove unrelated proof script from voice-wake PR ClawSweeper review: the console-sanitization proof script belongs to #103226, not this gateway voice-wake fix. Remove it to keep the branch clean. Ref: #103210 review * fix(gateway): harden voice wake unicode boundary --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

vincentkoc · 2026-07-11T02:16:28Z

Closing this branch. It mixes the console change with an unrelated voice-wake proof file, keeps a standalone proof script that should not ship, and the maintained CSI regression is malformed because U+009B is already the complete introducer.

More importantly, terminal-control sanitization needs a deliberate sink-wide review rather than another isolated micro-patch. A clean replacement would need the valid U+009B sequence in maintained tests, no proof-only files, real escaped-byte/code-point evidence, and an audit showing why sibling output sinks are unaffected.

…openclaw#103210) * fix(gateway): use truncateUtf16Safe for voice-wake trigger truncation Replace naive .slice(0, 64) with truncateUtf16Safe() in normalizeVoiceWakeTriggers to prevent surrogate pair splitting in user-configured voice wake trigger phrases. Voice wake triggers are user-configurable text strings that may contain emoji or non-BMP characters. A naive .slice(0, 64) at a surrogate pair boundary produces a lone surrogate, which corrupts the trigger text. Co-Authored-By: Claude <[email protected]> * test: add proof scripts for C1 sanitize and voice-wake UTF-16 * chore: remove unrelated proof script from voice-wake PR ClawSweeper review: the console-sanitization proof script belongs to openclaw#103226, not this gateway voice-wake fix. Remove it to keep the branch clean. Ref: openclaw#103210 review * fix(gateway): harden voice wake unicode boundary --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

lsr911 · 2026-07-11T08:55:54Z

@vincentkoc thanks — that's fair on all three points. Agreed the proof-only scripts and the stray voice-wake file shouldn't have shipped, and the CSI regression should use �31m (U+009B is already the complete introducer).

On the bigger point, I'd rather do the deliberate sink-wide pass than keep sending isolated patches. Before I invest the effort, does this shape sound right to you?

One PR that audits the terminal-control output sinks — the ones I had opened separately (failover reauth command, providerless model-warning log, sandbox session label, attachment-name diagnostic, doctor error message, config lookup-path log, image-generate directive) — applying the C1 range (0x80–0x9f) only where a sink actually reaches a terminal, with a short per-sink note on why it is / isn't affected, and why each can't simply route through the shared sanitizeForLog (its whitespace-flattening, length-cap, and optional-value semantics differ).
Maintained tests use valid U+009B sequences; no proof-only files; evidence provided as redacted code-point / escaped-byte output in the PR body.

If you'd prefer a single shared helper over per-sink predicates, or a different boundary/owner, I'll follow that. I'll hold and close the 7 open siblings (#103379 #103380 #103381 #103382 #103383 #103402 #103405) and fold them into the one PR once you confirm the direction — happy to skip it entirely if you'd rather handle terminal-control sanitization internally.

lsr911 · 2026-07-11T09:54:41Z

@vincentkoc thanks for the steer — I've rebuilt this as the deliberate sink-wide pass you asked for: #104362.

How it maps to your feedback here:

No proof-only scripts, no stray voice-wake file — just source changes plus maintained tests.
Valid U+009B in every regression (CSI + 31m, never ${csi}[…] — U+009B is already the complete introducer, as you noted).
sanitizeForConsole (this PR's fix) is re-landed cleanly inside the consolidated PR.
Sink-wide audit: I enumerated every control-character sink and the PR body tabulates them. Twelve terminal/log output sinks get the full C1 range (each keeps its own strip / escape / replace / truncate / reject action; only the code-point classification is aligned with sanitizeForLog's range). The two already-complete sinks (sanitizeForLog, renderTerminalBufferText from fix(terminal): strip C1 control characters (0x80-0x9f) in renderTerminalBufferText #103274) are noted; one console sink (sanitizeHookConsoleValue) is called out as deferred because it's only reachable through the full gateway hook request handler and deserves integration coverage rather than an untested one-liner; and the input-side / argv / URL / JSON-validity validators are listed with why C1 isn't an ANSI-injection concern there.

It also folds in the seven isolated C1 PRs (now closed in its favour). Happy to reshape it — extract a shared predicate, or land in smaller reviewed batches — if you'd prefer.

…g sinks Terminal-control sanitizers guard untrusted text before it reaches a terminal or log (CWE-117). The canonical range is C0 (0x00-0x1f), DEL (0x7f), and C1 (0x80-0x9f) - the range already used by the shared sanitizeForLog and by renderTerminalBufferText (openclaw#103274). C1 includes the 8-bit CSI introducer U+009B. Deliberate sink-wide pass: closes the C1 gap in twelve terminal/log output sinks, each keeping its own strip/escape/replace/truncate/reject action and gaining a focused valid-U+009B regression test. The PR body audits every other control-char sink (already-covered, deferred, or out-of-scope). Consolidates the isolated PRs openclaw#103379 openclaw#103380 openclaw#103381 openclaw#103382 openclaw#103383 openclaw#103402 Signed-off-by: lsr911 <[email protected]> openclaw#103405 and re-lands the sanitizeForConsole fix from openclaw#103226 cleanly.

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 10, 2026

test: add proof scripts for C1 sanitize and voice-wake UTF-16

7781265

openclaw-barnacle Bot added size: S and removed size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 10, 2026

test: add proof script for C1 control character sanitization

9c90039

vincentkoc self-assigned this Jul 11, 2026

vincentkoc closed this Jul 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): strip C1 control characters (0x80-0x9f) in sanitizeForConsole#103226

fix(agents): strip C1 control characters (0x80-0x9f) in sanitizeForConsole#103226
lsr911 wants to merge 3 commits into
openclaw:mainfrom
lsr911:fix/console-sanitize-c1-control-chars

lsr911 commented Jul 10, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 10, 2026 •

edited

Loading

Uh oh!

lsr911 commented Jul 10, 2026

Uh oh!

clawsweeper Bot commented Jul 10, 2026 •

edited

Loading

Uh oh!

vincentkoc commented Jul 11, 2026

Uh oh!

lsr911 commented Jul 11, 2026

Uh oh!

lsr911 commented Jul 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

lsr911 commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

Evidence

Standalone proof

Files Changed (1 file, +2/-1)

Uh oh!

clawsweeper Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lsr911 commented Jul 10, 2026

Uh oh!

clawsweeper Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincentkoc commented Jul 11, 2026

Uh oh!

lsr911 commented Jul 11, 2026

Uh oh!

lsr911 commented Jul 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lsr911 commented Jul 10, 2026 •

edited

Loading

clawsweeper Bot commented Jul 10, 2026 •

edited

Loading

clawsweeper Bot commented Jul 10, 2026 •

edited

Loading