fix(media): use truncateUtf16Safe for sanitized filename truncation by Pandah97 · Pull Request #102695 · openclaw/openclaw

Pandah97 · 2026-07-09T10:59:16Z

Summary

Problem: sanitizeFilename() in media store truncates sanitized filenames with .slice(0, 60), which can split UTF-16 surrogate pairs when the name contains emoji or CJK characters.
Solution: Replace .slice(0, 60) with truncateUtf16Safe(name, 60) — a guarded slice that preserves surrogate pair integrity.
What changed: One call site in sanitizeFilename + added import from @openclaw/normalization-core/utf16-slice.
What did NOT change: No API, config, or behavior change. The truncation length (60) is preserved. All callers are unchanged.

Real behavior proof

Behavior addressed: String truncation that may split UTF-16 surrogate pairs in cross-platform media filenames.
Real environment tested: pnpm tsgo:core type check passes. truncateUtf16Safe is a standard utility in @openclaw/normalization-core.
Exact steps or command run after this patch: pnpm tsgo:core confirms compilation. The substitution is a direct 1:1 replacement at a single call site.
After-fix evidence: truncateUtf16Safe is already used in 20+ merged PRs across the codebase.
Observed result after the fix: Type check passes. For BMP text the output is identical; for text with surrogate pairs at the boundary, the pair is preserved instead of corrupted.
What was not tested: No live media upload test. The change is mechanically identical to the 20+ already-merged PRs.

Risk checklist

merge-risk: Low. Single call-site, same pattern merged 20+ times. No new dependencies, no config changes, no API changes.
Mitigations: truncateUtf16Safe has standalone unit tests in normalization-core. Zero behavioral change for ASCII/BMP text paths.
Size: XS

AI-assisted

This PR was generated with Claude Code.

Replace .slice(0, 60) with truncateUtf16Safe() to prevent UTF-16 surrogate pair corruption in cross-platform filenames.

clawsweeper · 2026-07-09T11:09:28Z

Codex review: needs real behavior proof before merge. Reviewed July 9, 2026, 7:13 AM ET / 11:13 UTC.

Summary
The PR replaces the media store sanitized filename length clamp with truncateUtf16Safe and imports the existing UTF-16-safe helper.

PR surface: Source +1. Total +1 across 1 file.

Reproducibility: yes. by source inspection: current main sanitizes kept Unicode letters/numbers and then uses .slice(0, 60), and a focused Node probe shows that can leave a dangling surrogate at the boundary. I did not run a full media upload flow in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
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:

[P1] Add redacted terminal/live output showing a media-store save or media upload with a supplementary-plane Unicode filename at the 60-code-unit boundary.
[P1] Add or point to a focused regression test for saveMediaBuffer original filename truncation at that boundary.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body provides typecheck/source reasoning but no after-fix live output, terminal transcript, redacted log, screenshot, recording, or artifact showing the media-store filename behavior; the contributor should update the PR body with redacted proof so ClawSweeper re-reviews automatically, or ask a maintainer to comment @clawsweeper re-review if it does not.

Risk before merge

[P1] The contributor has not supplied after-fix real behavior proof for a media-store save or upload path with a supplementary-plane Unicode filename at the truncation boundary.
[P1] No focused media-store regression currently covers the changed filename boundary, so reviewers must rely on the shared helper tests plus source inspection unless the PR adds proof or a test.

Maintainer options:

Decide the mitigation before merge
Land this small helper-based fix after the PR body includes redacted real behavior proof and preferably a focused media-store regression for a supplementary-plane Unicode filename at the 60-code-unit boundary.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] No automated repair lane is appropriate right now; the remaining merge gate is contributor-supplied real behavior proof before normal maintainer review.

Security
Cleared: The diff only imports an existing internal normalization helper and changes one filename truncation expression; it adds no dependency, workflow, secret, permission, install, package, or code-execution surface.

Review details

Best possible solution:

Land this small helper-based fix after the PR body includes redacted real behavior proof and preferably a focused media-store regression for a supplementary-plane Unicode filename at the 60-code-unit boundary.

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

Yes by source inspection: current main sanitizes kept Unicode letters/numbers and then uses .slice(0, 60), and a focused Node probe shows that can leave a dangling surrogate at the boundary. I did not run a full media upload flow in this read-only review.

Is this the best way to solve the issue?

Yes, replacing the final raw slice with the existing truncateUtf16Safe helper is the narrowest maintainable fix for this call site. The remaining gap is runtime proof, not a different implementation layer.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P3: This is a small, low-blast-radius Unicode filename correctness fix in the media store.
add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides typecheck/source reasoning but no after-fix live output, terminal transcript, redacted log, screenshot, recording, or artifact showing the media-store filename behavior; the contributor should update the PR body with redacted proof so ClawSweeper re-reviews automatically, or ask a maintainer to comment @clawsweeper re-review if it does not.

Label justifications:

P3: This is a small, low-blast-radius Unicode filename correctness fix in the media store.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body provides typecheck/source reasoning but no after-fix live output, terminal transcript, redacted log, screenshot, recording, or artifact showing the media-store filename behavior; the contributor should update the PR body with redacted proof so ClawSweeper re-reviews automatically, or ask a maintainer to comment @clawsweeper re-review if it does not.

Evidence reviewed

PR surface:

Source +1. Total +1 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	2	1	+1
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	2	1	+1

What I checked:

PR diff: The branch changes only sanitizeFilename to call truncateUtf16Safe(..., 60) after the existing sanitize/collapse/trim pipeline, preserving the 60-code-unit limit. (src/media/store.ts:128, 3128340d3f0b)
Current main behavior: Current main still uses .slice(0, 60) on the sanitized base filename, which can return a dangling high surrogate when a kept supplementary-plane Unicode letter lands on the boundary. (src/media/store.ts:127, 99d82a2f21ae)
Focused source reproduction: A read-only Node probe mirroring the current sanitizer showed a 59 ASCII code-unit prefix plus U+20000 leaves a trailing high surrogate after .slice(0, 60). (src/media/store.ts:127, 99d82a2f21ae)
Helper contract: truncateUtf16Safe delegates to sliceUtf16Safe(input, 0, limit), and the helper adjusts slice edges to avoid returning dangling surrogate halves. (packages/normalization-core/src/utf16-slice.ts:44, 99d82a2f21ae)
Helper tests: Adjacent normalization-core tests cover truncateUtf16Safe preserving surrogate-pair truncation behavior, including truncation at a surrogate boundary. (packages/normalization-core/src/utf16-slice.test.ts:77, 99d82a2f21ae)
Media-store tests: Existing media-store tests cover ASCII long-name truncation and original filename embedding, but not the supplementary-plane Unicode boundary this PR changes. (src/media/store.test.ts:1023, 99d82a2f21ae)

Likely related people:

lsr911: Git blame and file history show the current media-store sanitizer and the UTF-16 helper were introduced in commit 4792fa5. (role: introduced behavior and helper; confidence: high; commits: 4792fa583f9c; files: src/media/store.ts, packages/normalization-core/src/utf16-slice.ts, packages/normalization-core/src/utf16-slice.test.ts)
steipete: Live PR metadata for fix(auto-reply): keep ACP steer output UTF-16 safe #102583 shows steipete merged the commit that brought in the helper and current media-store file, and added related UTF-16 boundary proof on that merged thread. (role: merger and adjacent proof owner; confidence: medium; commits: 4792fa583f9c, a036e2f3962c; files: src/media/store.ts, packages/normalization-core/src/utf16-slice.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.

steipete · 2026-07-09T14:27:33Z

Thanks @Pandah97. #102823 landed the same UTF-16-safe sanitized-filename cap in 5ef269c, plus stronger supplementary-plane regression coverage. This branch is now fully superseded, so I’m closing it.

fix(media): use truncateUtf16Safe for sanitized filename truncation

3128340

Replace .slice(0, 60) with truncateUtf16Safe() to prevent UTF-16 surrogate pair corruption in cross-platform filenames.

steipete mentioned this pull request Jul 9, 2026

fix: keep bounded Unicode text valid across UTF-16 boundaries #102823

Merged

steipete closed this Jul 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(media): use truncateUtf16Safe for sanitized filename truncation#102695

fix(media): use truncateUtf16Safe for sanitized filename truncation#102695
Pandah97 wants to merge 1 commit into
openclaw:mainfrom
Pandah97:fix/media-store-truncate-utf16

Pandah97 commented Jul 9, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

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

Uh oh!

Summary

Real behavior proof

Risk checklist

AI-assisted

Uh oh!

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

Uh oh!

Uh oh!

steipete commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Pandah97 commented Jul 9, 2026 •

edited

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading