fix(whatsapp): canonicalize outbound media delivery#69813
Conversation
🔒 Aisle Security AnalysisWe found 2 potential security issue(s) in this PR:
1. 🟡 Unsanitized filename derived from mediaUrl can enable CRLF/header injection in WhatsApp document upload
Description
A crafted URL such as Vulnerable code: const fileName = path.posix.basename(parsed.pathname);
return fileName ? decodeURIComponent(fileName) : undefined;RecommendationSanitize derived filenames before passing them to messaging libraries. Suggested approach:
Example: function sanitizeFileName(name: string): string {
// remove control chars incl. CR/LF and DEL
let n = name.replace(/[\u0000-\u001F\u007F]/g, "");
// remove path separators
n = n.replace(/[\\/]/g, "_");
// collapse whitespace
n = n.replace(/\s+/g, " ").trim();
// allowlist (tune as needed)
n = n.replace(/[^A-Za-z0-9._ -]/g, "_");
// enforce length
if (!n) return "file";
return n.slice(0, 128);
}
const raw = fileName ? decodeURIComponent(fileName) : undefined;
return raw ? sanitizeFileName(raw) : undefined;Additionally, consider validating 2. 🟡 Non-idempotent retry of WhatsApp auto-reply sends can cause duplicate messages/media
Description
Why this is a security/business risk:
Vulnerable code (auto-reply retries non-idempotent operations): const sendWithRetry = async (fn: () => Promise<unknown>, label: string, maxAttempts = 3) => {
return await sendWhatsAppOutboundWithRetry({ send: fn, maxAttempts, ... });
};
...
await sendWithRetry(() => msg.reply(chunk, quote), "text");
...
await sendWithRetry(() => msg.sendMedia({ ... }, quote), "media:..." );Retry classification is based on error text and can match cases where the send may have succeeded: return /closed|reset|timed\s*out|disconnect/i.test(formatError(error));RecommendationAvoid retrying non-idempotent outbound sends unless you can guarantee idempotency. Options:
Example (no retry for sends): // do not retry WhatsApp sends to avoid duplicates
await msg.reply(chunk, quote);Example (idempotency cache sketch): if (await sentCache.has(key)) return;
await msg.reply(chunk, quote);
await sentCache.set(key, true, { ttlMs: 5 * 60_000 });Analyzed PR: #69813 at commit Last updated on: 2026-04-24T03:46:54Z |
Greptile SummaryThis PR introduces Confidence Score: 5/5Safe to merge; one minor defensive-guard suggestion with no impact on normal callers. All remaining findings are P2 style/robustness suggestions. The canonical contract is correctly wired across all three outbound paths, normalization is idempotent and covered by parity tests, and the wrapped-error retry path is verified by a dedicated test. No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/whatsapp/src/outbound-media-contract.ts
Line: 94-118
Comment:
**`throw lastError` can throw `undefined` when `maxAttempts ≤ 0`**
`lastError` is initialized to `undefined` and only assigned inside the `catch` block. If a caller explicitly passes `maxAttempts: 0` (or negative), the loop body never executes and the fallthrough `throw lastError` silently propagates `undefined`, which produces a cryptic "undefined was thrown" at the call site instead of a meaningful error. A guard at the entry or a sentinel initial value would make the failure clearer.
```suggestion
}): Promise<T> {
const maxAttempts = params.maxAttempts ?? 3;
if (maxAttempts <= 0) {
throw new Error(`sendWhatsAppOutboundWithRetry: maxAttempts must be >= 1, got ${maxAttempts}`);
}
let lastError: unknown;
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(whatsapp): preserve indented channel..." | Re-trigger Greptile |
|
Follow-up on bot feedback:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9480ae0a65
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
c5ed37e to
4188dd5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d6be4895f4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80cd3dbd22
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfc00aec16
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
f4b900e to
7506049
Compare
There was a problem hiding this comment.
💡 Codex Review
https://github.com/openclaw/openclaw/blob/7506049d7af31e0ff1524770c0c66b5616a75469/src/auto-reply/reply/reply-delivery.ts#L147-L150
Deliver media-only block replies when streaming is disabled
When blockStreamingEnabled is false, this handler now never forwards block payloads to onBlockReply, so media-only block replies are dropped. That breaks the lifecycle path where consumePendingToolMediaReply emits attachments at the end of a run: those payloads are not guaranteed to be reconstructible from final assistant text, so they can disappear entirely instead of being sent once. Please restore the media-bearing fallback send path for the non-streaming branch.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
7506049 to
1e5744a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e5744a3a6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
1e5744a to
fd9e485
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd9e485882
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
fd9e485 to
6f20898
Compare
6f20898 to
5ce22bc
Compare
* 'main' of https://github.com/openclaw/openclaw: fix(whatsapp): canonicalize outbound media delivery (#69813)
Fixes openclaw#66053. The WhatsApp adapter previously dropped the audioAsVoice flag from ChannelOutboundContext, so replies carrying the [[audio_as_voice]] directive were delivered as document attachments instead of PTT voice notes. Layers - sendMessageWhatsApp: adds audioAsVoice option. When true and the loaded media is a verified audio source (isVerifiedAudioSource), mediaType is rebuilt from a sanitized MIME with opus codec preservation/fallback so the outbound frame reaches WhatsApp as a voice note. Non-audio-verified payloads stay on the document path. - createWhatsAppOutboundBase (outbound-base.ts): forwards audioAsVoice through the sendMedia factory option so outbound-adapter.ts no longer needs per-adapter wiring (upstream canonicalization landed in openclaw#69813). - deliverWebReply (auto-reply/deliver-reply.ts): routes kind='audio' and audioAsVoice-verified media to PTT with sanitized opus mimetype; image/video/document branches apply sanitized allowlisted mimetypes; document fileName is passed through sanitizeFileName. Security hardening (applied on top of upstream's normalizeWhatsAppLoadedMedia helper) - sanitizeMediaMime rejects control characters (CWE-93) and preserves codecs params only in the audio path. - sanitizeFileName strips ASCII control and bidi/invisible Unicode to prevent filename UI spoofing (CWE-451). - isVerifiedAudioSource gates forceVoiceDelivery so an audioAsVoice reply cannot coerce non-audio bytes into a voice-note payload. Tests (extensions/whatsapp) - send.test.ts: audioAsVoice=true+audio routing, forceVoiceDelivery override guard, mimetype sanitization across kinds, document fileName sanitization. - deliver-reply.test.ts: voice-coercion rejection, control-character fallbacks for audio/image/video/document, fileName sanitization, audioAsVoice-unset document path. - outbound-base.test.ts / outbound-adapter.sendpayload.test.ts: audioAsVoice propagation through the factory. Rebased onto upstream/main after openclaw#69813. Audio mimetype canonicalization ("audio/ogg" -> "audio/ogg; codecs=opus") is now owned by normalizeWhatsAppLoadedMedia; this change layers PTT routing and security hardening on top. Closes openclaw#66053
* fix(whatsapp): normalize outbound media payloads * fix(embedded-runner): preserve final media directives * fix(auto-reply): keep non-streaming media on final path * fix(auto-reply): warn when reply media is dropped * fix(whatsapp): align auto-reply media delivery * docs(changelog): note whatsapp media normalization
* fix(whatsapp): normalize outbound media payloads * fix(embedded-runner): preserve final media directives * fix(auto-reply): keep non-streaming media on final path * fix(auto-reply): warn when reply media is dropped * fix(whatsapp): align auto-reply media delivery * docs(changelog): note whatsapp media normalization
* 'main' of https://github.com/openclaw/openclaw: fix(whatsapp): canonicalize outbound media delivery (openclaw#69813)
* fix(whatsapp): normalize outbound media payloads * fix(embedded-runner): preserve final media directives * fix(auto-reply): keep non-streaming media on final path * fix(auto-reply): warn when reply media is dropped * fix(whatsapp): align auto-reply media delivery * docs(changelog): note whatsapp media normalization
* 'main' of https://github.com/openclaw/openclaw: fix(whatsapp): canonicalize outbound media delivery (openclaw#69813)
* fix(whatsapp): normalize outbound media payloads * fix(embedded-runner): preserve final media directives * fix(auto-reply): keep non-streaming media on final path * fix(auto-reply): warn when reply media is dropped * fix(whatsapp): align auto-reply media delivery * docs(changelog): note whatsapp media normalization
* 'main' of https://github.com/openclaw/openclaw: fix(whatsapp): canonicalize outbound media delivery (openclaw#69813)
* fix(whatsapp): normalize outbound media payloads * fix(embedded-runner): preserve final media directives * fix(auto-reply): keep non-streaming media on final path * fix(auto-reply): warn when reply media is dropped * fix(whatsapp): align auto-reply media delivery * docs(changelog): note whatsapp media normalization
* 'main' of https://github.com/openclaw/openclaw: fix(whatsapp): canonicalize outbound media delivery (openclaw#69813)
* fix(whatsapp): normalize outbound media payloads * fix(embedded-runner): preserve final media directives * fix(auto-reply): keep non-streaming media on final path * fix(auto-reply): warn when reply media is dropped * fix(whatsapp): align auto-reply media delivery * docs(changelog): note whatsapp media normalization
* 'main' of https://github.com/openclaw/openclaw: fix(whatsapp): canonicalize outbound media delivery (openclaw#69813)
* fix(whatsapp): normalize outbound media payloads * fix(embedded-runner): preserve final media directives * fix(auto-reply): keep non-streaming media on final path * fix(auto-reply): warn when reply media is dropped * fix(whatsapp): align auto-reply media delivery * docs(changelog): note whatsapp media normalization
* 'main' of https://github.com/openclaw/openclaw: fix(whatsapp): canonicalize outbound media delivery (openclaw#69813)
Summary
Describe the problem and fix in 2–5 bullets:
extensions/whatsapp/src/channel-outbound.ts,extensions/whatsapp/src/outbound-adapter.ts,extensions/whatsapp/src/send.ts, andextensions/whatsapp/src/auto-reply/deliver-reply.ts, so media fixes kept landing in one path while another path still drifted.extensions/whatsapp/src/outbound-media-contract.tsand routed the outbound paths through it so media normalization, caption-first behavior, retry classification, logging shape, and listener/send resolution stay aligned.NO_REPLY.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
Regression Test Plan (if applicable)
extensions/whatsapp/src/channel-outbound.test.tsextensions/whatsapp/src/send.test.tsextensions/whatsapp/src/outbound-base.test.tsextensions/whatsapp/src/outbound-adapter.sendpayload.test.tsextensions/whatsapp/src/outbound-payload.contract.test.tsextensions/whatsapp/src/auto-reply/deliver-reply.test.tssrc/agents/pi-embedded-runner/run/payloads.test.tssrc/auto-reply/reply/agent-runner-payloads.test.tssrc/auto-reply/reply/reply-delivery.test.tssrc/auto-reply/reply/reply-media-paths.test.tsextensions/whatsapp/src/auto-reply.web-auto-reply.connection-and-logging.e2e.test.tscovers one media reply logging/send path at a higher level.User-visible / Behavior Changes
mediaUrlstays primary when bothmediaUrlandmediaUrlsare present.Diagram (if applicable)
Security Impact (required)
No)No)No)No)No)Yes, explain risk + mitigation: N/ARepro + Verification
Environment
Steps
mediaUrlandmediaUrls, a caption/text body, and a wrapped transient send error case.MEDIA:replies through the embedded/final reply path.Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
mediaUrl+mediaUrlsprimary-source behaviormediaUrlsentriesReview Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes)No)No)Risks and Mitigations