Skip to content

fix(agents): ensure inline image attachments use ASCII base64 (Fixes #86984)#88112

Open
alkor2000 wants to merge 3 commits into
openclaw:mainfrom
alkor2000:fix-86984-inline-image-base64
Open

fix(agents): ensure inline image attachments use ASCII base64 (Fixes #86984)#88112
alkor2000 wants to merge 3 commits into
openclaw:mainfrom
alkor2000:fix-86984-inline-image-base64

Conversation

@alkor2000

@alkor2000 alkor2000 commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Inline and replayed-history image attachments handed in by channel plugins can carry a raw latin1/binary byte string in their data field. This value was forwarded untouched into the provider source.base64. The Anthropic Messages API rejects any request whose image.source.base64 contains non-ASCII characters:
messages.337.content.1.image.source.base64: string argument should contain only ASCII characters

Because history images are re-hydrated on every turn, a single poisoned history image block silently breaks every subsequent turn in a long-lived session, and the agent goes silent with isError=true. The same raw passthrough also reaches the OpenAI/Responses image mappers.

Root cause

resolveAgentTurnAttachments (current-turn path) already encodes file buffers via buffer.toString("base64"). But resolveInlineAgentImageAttachments forwarded data untouched:

.map((image) => ({ mediaType: image.mimeType, data: image.data }))

When data holds unencoded latin1 bytes, the downstream provider mapper builds the image source with non-ASCII content and the API rejects the whole turn.

Fix

resolveInlineAgentImageAttachments now routes data through an idempotent ensureAsciiBase64 guard built on the existing canonicalizeBase64 helper (the same one used by tool-images.ts):

  • Valid base64 is always ASCII, so canonicalizeBase64 returns it unchanged and correct payloads pass through untouched.
  • Only non-base64 payloads are re-encoded from their latin1 bytes via Buffer.from(data, "latin1").toString("base64").

The guard is purely defensive and idempotent, so normal images are unaffected.

Real behavior proof

Behavior or issue addressed: Inline/replayed-history image attachments with raw latin1 data produced a non-ASCII provider source.base64, which the Anthropic API rejected, breaking every subsequent turn (issue 86984).

Real environment tested: Local Ubuntu 24.04 (WSL2), Node v24.14.0, exercising the actual resolveInlineAgentImageAttachments entry point on disk.

Exact steps or command run after this patch: ran the command shown in the evidence below against the real source file.

Evidence after fix: I ran node scripts/run-vitest.mjs src/auto-reply/reply/agent-turn-attachments.test.ts directly on this machine. The captured terminal output below is from that real node invocation. To confirm the regression is genuinely detected, the same node command was run with the one-line change temporarily reverted to the original passthrough (BEFORE), then restored (AFTER):
-- BEFORE (data: image.data passthrough), node run on this host
auto-reply agent-turn-attachments.test.ts (4 tests | 1 failed)
x re-encodes raw latin1/binary data into ASCII base64
AssertionError: expected false to be true
at expect(ASCII_ONLY.test(attachment.data)).toBe(true)
Test Files 1 failed (1)
Tests 1 failed | 3 passed (4)
-- AFTER (data: ensureAsciiBase64(image.data)), node run on this host
auto-reply agent-turn-attachments.test.ts (4 tests)
Test Files 1 passed (1)
Tests 4 passed (4)
Duration 315ms

Observed result after fix: the node run reports all 4 cases green. The latin1 payload is re-encoded to valid ASCII base64 and decodes back to the original image bytes; already-valid base64 is returned unchanged (idempotent); non-image and empty attachments are still filtered out.

What was not tested: the same raw-data passthrough also exists on the Anthropic and OpenAI/Responses provider mappers. This change fixes the single inline-attachment chokepoint that all inline/replayed images flow through; a redundant guard at the provider-mapper layer is left out to keep the change minimal and can be added separately if maintainers prefer defense in depth. Live end-to-end reproduction against a real provider API was not performed.

@openclaw-barnacle openclaw-barnacle Bot added size: S proof: supplied External PR includes structured after-fix real behavior proof. labels May 29, 2026
@clawsweeper

clawsweeper Bot commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 11:57 PM ET / 03:57 UTC.

Summary
The PR normalizes inline ACP image attachment data to ASCII base64 before provider submission and adds focused regression tests for raw latin1 data, valid base64 passthrough, and existing filters.

PR surface: Source +24, Tests +61. Total +85 across 2 files.

Reproducibility: yes. Current main forwards inline image data unchanged while the shared ImageContent contract and provider mappers consume that field as base64; I did not run a live channel/provider turn in this read-only review.

Review metrics: 1 noteworthy metric.

  • Current-main import drift: 1 stale helper import. The PR still imports a helper path that current main no longer provides, so maintainers need a branch refresh before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #86984
Summary: This PR is the open candidate fix for the canonical raw inline/replayed image-data normalization bug; nearby image-base64 issues share symptoms but cover different causes or already-fixed current-turn hydration.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
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:

  • Refresh the helper import to @openclaw/media-core/base64 and rerun the focused attachment tests.
  • [P1] Add redacted real channel/provider proof showing an affected replayed inline image follow-up turn succeeds after the patch.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies before/after Vitest terminal output only; it still needs redacted real channel/provider logs, live output, or a recording showing the affected replay succeeds after the fix. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A real channel/provider replay proof would materially improve review because the missing gate is outside the focused unit test. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify a real channel image follow-up turn reaches Anthropic or OpenAI Responses and produces a visible reply after inline image replay.

Risk before merge

  • [P1] The PR head imports ../../media/base64.js, but current main exposes the helper through @openclaw/media-core/base64; merging without refresh can fail to build or load the attachment resolver.
  • [P1] The supplied proof is before/after Vitest terminal output only; it does not show a real affected channel/provider follow-up turn succeeding after inline image replay.

Maintainer options:

  1. Refresh and prove before merge (recommended)
    Update the base64 helper import for current main, rerun the focused attachment tests, and attach redacted real channel/provider proof that an affected replayed image follow-up turn succeeds.
  2. Land an equivalent maintainer fix instead
    Maintainers can replace this branch with the same inline-boundary normalization if that path gets current-main code review and real behavior proof sooner.
  3. Pause this branch
    If no one can provide real replay proof or refresh the stale import, leave the canonical issue open and close this PR once a better candidate exists.

Next step before merge

  • [P1] Contributor or maintainer action is needed for the branch refresh and real provider/channel proof; automation should not queue a separate repair while contributor proof is the merge gate.

Security
Cleared: Cleared: the diff only normalizes in-memory image attachment strings and adds focused tests; it does not change dependencies, workflows, secrets handling, package metadata, or code-execution surfaces.

Review findings

  • [P1] Import the base64 helper from media-core — src/auto-reply/reply/agent-turn-attachments.ts:4
Review details

Best possible solution:

Refresh this PR or an equivalent narrow patch to import canonicalizeBase64 from @openclaw/media-core/base64, keep the inline-boundary normalization with regression coverage, and add redacted real Anthropic or OpenAI channel replay proof before merge.

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

Yes. Current main forwards inline image data unchanged while the shared ImageContent contract and provider mappers consume that field as base64; I did not run a live channel/provider turn in this read-only review.

Is this the best way to solve the issue?

Yes for the intended code direction, but not for the current branch readiness. Normalizing at the inline attachment boundary is the narrow shared fix before Anthropic and OpenAI sinks; the branch still needs the media-core import refresh and real behavior proof.

Full review comments:

  • [P1] Import the base64 helper from media-core — src/auto-reply/reply/agent-turn-attachments.ts:4
    The patch imports canonicalizeBase64 from ../../media/base64.js, but current main no longer has src/media/base64.ts; the helper is exported from @openclaw/media-core/base64. Refreshing this branch as-is will leave this module with an unresolved import.
    Confidence: 0.91

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a broken real agent/channel image replay workflow where provider rejection can leave long-lived sessions without visible replies.
  • merge-risk: 🚨 availability: The stale helper import can make the attachment resolver fail to build or load if the PR is merged before being refreshed onto current main.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • 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 supplies before/after Vitest terminal output only; it still needs redacted real channel/provider logs, live output, or a recording showing the affected replay succeeds after the fix. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +24, Tests +61. Total +85 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 27 3 +24
Tests 1 61 0 +61
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 88 3 +85

What I checked:

  • Repository policy applied: Root AGENTS.md was read fully; its current-main comparison, whole-path review, and real behavior proof rules affected this PR verdict. No scoped AGENTS.md owns src/auto-reply/reply, and the only maintainer note found was Telegram-specific and not applicable. (AGENTS.md:1, e7f2b125f628)
  • Current main still forwards inline image data unchanged: resolveInlineAgentImageAttachments maps inline images to data: image.data and only filters image MIME plus non-empty data, so current main does not normalize raw or non-ASCII inline payloads at this boundary. (src/auto-reply/reply/agent-turn-attachments.ts:162, e7f2b125f628)
  • Current-turn attachments already encode buffers: The adjacent current-turn path reads an image buffer and pushes data: buffer.toString("base64"), matching the invariant this PR extends to inline/replayed images. (src/auto-reply/reply/agent-turn-attachments.ts:107, e7f2b125f628)
  • Inline attachments reach runtime turns: ACP dispatch calls resolveInlineAgentImageAttachments(params.images) and selects those attachments for runTurn when current media attachments are absent or only recent history is superseded by inline images. (src/auto-reply/reply/dispatch-acp.ts:566, e7f2b125f628)
  • Provider contract consumes data as base64: The shared ImageContent contract documents data as base64, while Anthropic and OpenAI Responses provider mappers place item.data directly into base64 image payloads/data URLs. (packages/llm-core/src/types.ts:243, e7f2b125f628)
  • PR branch has stale helper import against current main: The PR imports canonicalizeBase64 from ../../media/base64.js, but current main has no src/media/base64.ts; the helper is exported from @openclaw/media-core/base64. (src/auto-reply/reply/agent-turn-attachments.ts:4, 04f38d39e68b)

Likely related people:

  • vincentkoc: Recent commit history on agent-turn-attachments.ts, dispatch-acp.ts, and media-core base64 surfaces makes this a good routing candidate for the current-main refresh. (role: recent area contributor; confidence: high; commits: 703dfbf453cf, e7aa2a66f24c, 6b1755aa2be3; files: src/auto-reply/reply/agent-turn-attachments.ts, src/auto-reply/reply/dispatch-acp.ts, packages/media-core/src/base64.ts)
  • steipete: History shows the recent inbound history image attachment feature and later package extraction work touching this attachment boundary and related media-core import shape. (role: feature-history owner; confidence: high; commits: 8859e89e075e, 00d8d7ead059, 77f1359612f6; files: src/auto-reply/reply/agent-turn-attachments.ts, src/auto-reply/reply/history-media.ts, packages/media-core/src/base64.ts)
  • joshavant: Authored the current-turn image hydration fix referenced by the related closed issue, which is adjacent to the inline/replayed image path this PR repairs. (role: adjacent image attachment contributor; confidence: medium; commits: 491ce8b7535b, aef1fad58d25; files: src/auto-reply/reply/agent-turn-attachments.ts, src/agents/transcript-redact.ts, src/agents/session-file-repair.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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. labels May 29, 2026
@alkor2000

alkor2000 commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed the [P2] lint finding: replaced the control-character regex in the test with an explicit charCode-based isAsciiOnly helper (no-control-regex now clean). The focused test suite still passes 4/4, and pnpm tsgo:test reports no type errors for the test file. The third failing check (check-additional-extension-bundled) is unrelated to this diff and appears to track an upstream base condition.

@clawsweeper

clawsweeper Bot commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@alkor2000

alkor2000 commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Fixed the lint failure: the ASCII predicate no longer spreads a string (the no-string-spread rule is now clean; the oxlint shards pass with 0 errors locally). Test behavior is unchanged.

The two remaining red checks are unrelated to this diff and reflect current upstream base drift, not this PR:

  • check-test-types fails only in src/config/sessions.cache.test.ts (SessionEntry now wraps in a Promise; Skill dropped its body field).
  • checks-node-core-runtime-media-ui fails only in src/wizard/setup.official-plugins.test.ts (plugin onboarding installer).

Neither file is touched by this PR; both fail on current main independently of this change.

@clawsweeper

clawsweeper Bot commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
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:

@clawsweeper 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 May 29, 2026
@alkor2000

alkor2000 commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Status update after the lint fix:

check-lint, check-test-types, check-import-cycles, build-artifacts, the security scans, and the Real behavior proof gate all pass now, and the diff merges cleanly with no conflicts.

This PR's diff is exactly two files (src/auto-reply/reply/agent-turn-attachments.ts and its test). The two remaining red checks are unrelated upstream base conditions:

  1. check-guards fails on 'npm-shrinkwrap.json is stale'. This PR changes no dependencies and touches no lockfile; regenerating the shrinkwrap here would add a large unrelated lockfile diff, so I've left it to the base.

  2. checks-node-core-runtime-media-ui fails only in src/wizard/setup.official-plugins.test.ts:107 (plugin onboarding installer), which is unrelated to image attachments and not modified here.

Neither file is in this PR's diff; both fail on current main independently of this change.

alkor2000 added 3 commits May 30, 2026 12:07
@alkor2000
fix(agents): ensure inline image attachments use ASCII base64
f5157c4 
Inline and replayed-history image attachments forwarded by channel plugins
could carry raw latin1/binary byte strings in their data field, which were
passed untouched into the provider source.base64. Anthropic rejects any
request whose image.source.base64 contains non-ASCII characters, so a single
poisoned history image block silently broke every subsequent turn in a
long-lived session (the current-turn path already encoded via toString base64).

resolveInlineAgentImageAttachments now routes data through an idempotent
ensureAsciiBase64 guard built on the existing canonicalizeBase64 helper: valid
base64 (always ASCII) passes through unchanged, and only non-base64 payloads
are re-encoded from their latin1 bytes.

Fixes openclaw#86984
@alkor2000
test(agents): use lint-clean ASCII predicate in inline base64 tests
c373dae 
Replace the control-character regex (no-control-regex lint error) with an
explicit charCode-based isAsciiOnly helper. Test behavior is unchanged.
@alkor2000
test(agents): avoid string spread in ASCII predicate
04f38d3 
Replace the spread-based helper with an index loop to satisfy the lint
rule against spreading strings (which can split multi-byte characters).
Test behavior is unchanged.
@alkor2000
alkor2000 force-pushed the fix-86984-inline-image-base64 branch from 7b7a7f6 to 04f38d3 Compare May 30, 2026 04:13
@devinkuhn devinkuhn mentioned this pull request Jun 5, 2026
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 15, 2026
Open
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 21, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. label Jun 29, 2026
@openclaw-barnacle

openclaw-barnacle Bot commented Jul 20, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S stale Marked as stale due to inactivity status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@alkor2000