fix(media): gate markdown image extraction by channel#72718
fix(media): gate markdown image extraction by channel#72718steipete merged 1 commit intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR adds an Confidence Score: 4/5Safe to merge; fix is narrowly scoped and well-tested with only a minor regex style issue. Only P2 finding present: a dead No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/media/parse.ts
Line: 259
Comment:
**Dead `\?` branch in `BADGE_PATH_PATTERN_RE`**
The regex is always tested against `parsed.pathname`, but `URL.pathname` never contains a `?` — query parameters live in `URL.search`. The `\?` alternative in `(?:\?|$)` is therefore unreachable dead code in both positions. The existing `$` anchor already covers the end-of-path case correctly (e.g. `https://github.com/owner/repo/badge.svg?v=1` produces pathname `/owner/repo/badge.svg`, matched by `$`).
```suggestion
const BADGE_PATH_PATTERN_RE = /\/badge\.svg$|\/workflows\/[^/]+\/badge(?:\.|$)/i;
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: exclude badge/shield URLs from mark..." | Re-trigger Greptile |
|
|
||
| // Matches badge paths on generic hosts (e.g. GitHub CI status badges: | ||
| // https://github.com/owner/repo/actions/workflows/ci.yml/badge.svg). | ||
| const BADGE_PATH_PATTERN_RE = /\/badge\.svg(?:\?|$)|\/workflows\/[^/]+\/badge(?:\.|$)/i; |
There was a problem hiding this comment.
\? branch in BADGE_PATH_PATTERN_RE
The regex is always tested against parsed.pathname, but URL.pathname never contains a ? — query parameters live in URL.search. The \? alternative in (?:\?|$) is therefore unreachable dead code in both positions. The existing $ anchor already covers the end-of-path case correctly (e.g. https://github.com/owner/repo/badge.svg?v=1 produces pathname /owner/repo/badge.svg, matched by $).
| const BADGE_PATH_PATTERN_RE = /\/badge\.svg(?:\?|$)|\/workflows\/[^/]+\/badge(?:\.|$)/i; | |
| const BADGE_PATH_PATTERN_RE = /\/badge\.svg$|\/workflows\/[^/]+\/badge(?:\.|$)/i; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/media/parse.ts
Line: 259
Comment:
**Dead `\?` branch in `BADGE_PATH_PATTERN_RE`**
The regex is always tested against `parsed.pathname`, but `URL.pathname` never contains a `?` — query parameters live in `URL.search`. The `\?` alternative in `(?:\?|$)` is therefore unreachable dead code in both positions. The existing `$` anchor already covers the end-of-path case correctly (e.g. `https://github.com/owner/repo/badge.svg?v=1` produces pathname `/owner/repo/badge.svg`, matched by `$`).
```suggestion
const BADGE_PATH_PATTERN_RE = /\/badge\.svg$|\/workflows\/[^/]+\/badge(?:\.|$)/i;
```
How can I resolve this? If you propose a fix, please make it concise.a98e8bb to
e522084
Compare
Closes openclaw#72642 Co-authored-by: Bartok9 <danielrpike9@gmail.com>
e522084 to
7cbc9a8
Compare
|
Landed cleanly after rewriting this to a channel-owned opt-in instead of a badge-host blacklist.
Thanks @Bartok9! |
Closes openclaw#72642 Co-authored-by: Peter Steinberger <steipete@gmail.com>
Closes openclaw#72642 Co-authored-by: Peter Steinberger <steipete@gmail.com>
Problem
Discord final replies can contain incidental Markdown images from README snippets, package summaries, or badge examples. Since #66471, the shared media parser lifted remote Markdown image syntax into
mediaUrls, so Discord delivered those incidental images as file attachments.Example:
That should remain visible reply text on Discord, not become an attachment.
Fix
Make Markdown-image media extraction opt-in:
splitMediaFromOutput()keeps Markdown images as text by default.ChannelOutboundAdapter.extractMarkdownImagesis the generic channel-owned capability.MEDIA:directives remain unchanged, so agents and trusted tools can still intentionally attach any valid public media URL.Tests
src/media/parse.test.tssrc/infra/outbound/payloads.test.tssrc/infra/outbound/deliver.test.tssrc/auto-reply/reply/agent-runner-payloads.test.tspnpm check:changedCloses #72642.