fix(agents): detect full C1 range in image-generate inline directive check by lsr911 · Pull Request #103405 · openclaw/openclaw

lsr911 · 2026-07-10T05:07:36Z

What Problem This Solves

sanitizeInlineDirectiveText() in src/agents/tools/image-generate-tool.ts escapes control characters in provider/model/override text before generated-media directives are appended to the image-tool summary (so untrusted values can't inject MEDIA: directives or terminal escapes). Its isInlineDirectiveControlCharacter() predicate covered C0 (0x00-0x1f), DEL (0x7f), LS (0x2028) and PS (0x2029), but not the C1 control range (0x80-0x9f) — which includes the CSI introducer U+009B, an alternative ANSI escape prefix equivalent to ESC [.

Change

Add (code >= 0x80 && code <= 0x9f) to isInlineDirectiveControlCharacter, so C1 bytes are escaped to a visible backslash-u sequence alongside the existing ranges.

Testing / Proof

Extended the existing summary-escaping regression in src/agents/tools/image-generate-tool.test.ts ("escapes image-generation summary text before appending tool MEDIA output") so an ignoredOverrides value now carries a U+009B control byte (followed by [2J). The test asserts the emitted summary contains the escaped backslash-u form of U+009B rather than the raw byte, while the structured details object still round-trips the original raw value unchanged.

$ node scripts/run-vitest.mjs run src/agents/tools/image-generate-tool.test.ts

 RUN  v4.1.8 D:/IdeaWork/openclaw-repo

 Test Files  1 passed (1)
      Tests  57 passed (57)
   Duration  62.17s

The test drives the real image-tool execute path through sanitizeInlineDirectiveText, so the passing run is the before/after proof: on current main the U+009B passes through the escaping boundary raw; with this change it is escaped to its visible backslash-u form before the media directive is appended.

…check Same C1 gap pattern as openclaw#103274. Add full C1 range (0x80-0x9f). Co-Authored-By: Claude <[email protected]>

clawsweeper · 2026-07-10T10:45:57Z

Codex review: needs real behavior proof before merge. Reviewed July 11, 2026, 4:37 AM ET / 08:37 UTC.

Summary
The PR expands image-generation inline summary sanitization to escape U+0080–U+009F controls and adds a U+009B execute-path regression assertion.

PR surface: Source +6, Tests 0. Total +6 across 2 files.

Reproducibility: yes. from source at high confidence: current main omits U+0080–U+009F at the image-summary sanitizer, and the added execute-path regression exercises U+009B. A real provider-backed reproduction was not supplied.

Review metrics: none identified.

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

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

Rank-up moves:

Apply the sanitizer consistently to the video and music summary siblings with focused regression coverage.
Post redacted real image-generation terminal output or runtime logs showing U+009B escaped while structured details retain the raw value.

Proof guidance:

[P1] Needs real behavior proof before merge: Only focused Vitest output is provided; add redacted after-fix terminal output or runtime logs from a real image-generation setup, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

[P1] Merging the image-only change leaves parallel video and music summaries accepting raw control characters, creating inconsistent protection at equivalent generated-media output boundaries.
[P1] The contributor has not shown after-fix output from a real image-generation setup, so the runtime behavior and unchanged structured details are not independently demonstrated outside mocks.

Maintainer options:

Decide the mitigation before merge
Use one shared generated-media summary sanitizer for image, video, and music provider/model text and string override values, preserve the original structured details, and demonstrate the result with redacted real runtime output.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] The contributor needs to resolve the concrete sibling defect and provide real setup proof; automation cannot manufacture the required external-environment evidence.

Security
Cleared: The diff strengthens a text-sanitization boundary and introduces no dependency, workflow, permission, secret-handling, or supply-chain regression.

Review findings

[P2] Cover all generated-media summary siblings — src/agents/tools/image-generate-tool.ts:541

Review details

Best possible solution:

Use one shared generated-media summary sanitizer for image, video, and music provider/model text and string override values, preserve the original structured details, and demonstrate the result with redacted real runtime output.

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

Yes, from source at high confidence: current main omits U+0080–U+009F at the image-summary sanitizer, and the added execute-path regression exercises U+009B. A real provider-backed reproduction was not supplied.

Is this the best way to solve the issue?

No. The predicate addition is correct for image generation, but the narrowest maintainable solution is a shared generated-media output boundary that also covers equivalent video and music interpolation paths.

Full review comments:

[P2] Cover all generated-media summary siblings — src/agents/tools/image-generate-tool.ts:541
The prior video-boundary finding remains unresolved: this predicate hardens only image_generate, while video_generate still interpolates string overrides/provider/model and music_generate can interpolate string lyrics/provider/model before generated attachment lines. Move the sanitizer to a shared boundary and add focused sibling regressions, otherwise the same C1/directive injection remains reachable through equivalent tools.
Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

P2: This is a bounded terminal-visible output-sanitization defect with limited blast radius and no demonstrated active exploit, data loss, or outage.
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 real behavior proof before merge: Only focused Vitest output is provided; add redacted after-fix terminal output or runtime logs from a real image-generation setup, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Evidence reviewed

PR surface:

Source +6, Tests 0. Total +6 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	7	1	+6
Tests	1	3	3	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	10	4	+6

What I checked:

Changed image boundary: The branch adds the C1 range to the image-only control-character predicate while retaining raw values in structured details. (src/agents/tools/image-generate-tool.ts:541, 19cf27b309aa)
Video sibling remains raw: Current main directly interpolates provider, model, and string-capable ignored override values into video summary lines immediately before generated attachment directives. (src/agents/tools/video-generate-tool.ts:790, 585914fcd510)
Music sibling remains raw: Current main directly interpolates provider, model, and string-capable ignored overrides, including lyrics, into music summary lines before generated attachment directives. (src/agents/tools/music-generate-tool.ts:491, 585914fcd510)
Prior finding remains: The latest completed ClawSweeper cycle already required coverage of the parallel video summary boundary; the subsequent commit changed only the image regression test. (src/agents/tools/image-generate-tool.test.ts:2448, 19cf27b309aa)
Proof is test-only: The PR body supplies a focused Vitest transcript, but no redacted terminal output or runtime logs from a real image-generation setup. (src/agents/tools/image-generate-tool.test.ts:2474, 19cf27b309aa)
Related C1 precedent: A separate C1 sanitizer fix for terminal buffer rendering was merged in fix(terminal): strip C1 control characters (0x80-0x9f) in renderTerminalBufferText #103274, confirming the omitted range pattern while leaving generated-media summaries as a distinct boundary. (src/gateway/terminal/buffer-text.ts, a3f9f3567f71)

Likely related people:

steipete: Authored the shared media-generation helper, normalization, provider-fallback, and ignored-override history central to this output boundary. (role: feature owner; confidence: high; commits: 5f370149f311, 425592cf9ceb, a463a33eee69; files: src/agents/tools/image-generate-tool.ts, src/agents/tools/video-generate-tool.ts, src/agents/tools/music-generate-tool.ts)
Vincent Koc: Recently carried cross-tool type and leaf-surface refactoring in the image, video, and music generation area. (role: recent area contributor; confidence: medium; commits: 74e7b8d47b18; files: src/agents/tools/image-generate-tool.ts, src/agents/tools/video-generate-tool.ts, src/agents/tools/music-generate-tool.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 (2 earlier review cycles)

reviewed 2026-07-10T10:44:19.032Z sha abe4841 :: needs real behavior proof before merge. :: none
reviewed 2026-07-11T08:29:37.621Z sha 19cf27b :: needs real behavior proof before merge. :: [P2] Cover the parallel video summary boundary

Extend the existing summary-escaping regression so an ignored-override value carrying the C1 CSI introducer U+009B (an alt ANSI escape prefix) is escaped to \u009b before generated-media directives are appended, matching the existing C0/DEL/LS/PS coverage. Signed-off-by: lsr911 <[email protected]>

lsr911 · 2026-07-11T09:44:44Z

Superseded by #104362 — a single sink-wide C1 (0x80–0x9f) pass that folds this sink's fix in with a valid U+009B regression, alongside an audit of every other control-character sink (already-covered / deferred / out-of-scope). This consolidates the isolated C1 PRs per the direction on #103226. Closing in favor of the consolidated review.

…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.

fix(agents): detect full C1 range in image-generate inline directive …

abe4841

…check Same C1 gap pattern as openclaw#103274. Add full C1 range (0x80-0x9f). Co-Authored-By: Claude <[email protected]>

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. and removed triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 10, 2026

openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 11, 2026

clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 11, 2026

This was referenced Jul 11, 2026

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

Closed

fix: complete C1 control-character (0x80-0x9f) coverage across terminal/log sinks #104362

Closed

lsr911 closed this Jul 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(agents): detect full C1 range in image-generate inline directive check#103405

fix(agents): detect full C1 range in image-generate inline directive check#103405
lsr911 wants to merge 2 commits into
openclaw:mainfrom
lsr911:fix/image-generate-tool-c1

lsr911 commented Jul 10, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 10, 2026 •

edited

Loading

Uh oh!

lsr911 commented Jul 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

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

Change

Testing / Proof

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 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lsr911 commented Jul 10, 2026 •

edited

Loading

clawsweeper Bot commented Jul 10, 2026 •

edited

Loading