Media: sanitize Content-Disposition filenames from remote fetches by afurm · Pull Request #27818 · openclaw/openclaw

afurm · 2026-02-26T18:07:12Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem: fetchRemoteMedia parsed Content-Disposition filenames with path.basename(), which does not treat Windows \ separators as path separators on POSIX hosts.
Why it matters: a remote server can return filenames like ..\\..\\secret.pdf, and traversal-like segments can propagate into downstream attachment metadata/labels instead of being reduced to a safe basename.
What changed: added a filename sanitizer for Content-Disposition parsing that normalizes \ to /, strips control characters, and then takes a basename; applied to both filename= and RFC5987 filename*= paths; added regression tests.
What did NOT change (scope boundary): no changes to SSRF/network policy, file-write destinations, storage layout, or media fetch request behavior.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes #
Related #

User-visible / Behavior Changes

Remote media downloads now sanitize malicious/invalid Content-Disposition filenames that use Windows-style traversal separators (for example ..\\..\\file.pdf becomes file.pdf).
No change for normal filenames.

Security Impact (required)

New permissions/capabilities? (Yes/No) No
Secrets/tokens handling changed? (Yes/No) No
New/changed network calls? (Yes/No) No
Command/tool execution surface changed? (Yes/No) No
Data access scope changed? (Yes/No) No
If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

OS: macOS (local dev machine)
Runtime/container: Node + pnpm + Vitest (no container)
Model/provider: N/A
Integration/channel (if any): N/A (unit test coverage on shared media helper)
Relevant config (redacted): None

Steps

Mock fetchRemoteMedia() to receive Content-Disposition: attachment; filename="..\\..\\secret.pdf".
Call fetchRemoteMedia() and inspect result.fileName.
Repeat with filename*=UTF-8''..%5C..%5Creport-final.pdf.

Expected

Returned filename is reduced to a safe basename (secret.pdf / report-final.pdf), without traversal-like segments.

Actual

Before fix (pre-patch): POSIX path.basename() preserved backslash-delimited segments, so ..\\..\\secret.pdf could survive as-is.
After fix: both header forms are sanitized to the basename.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Test snippet (after patch):

pnpm test src/media/fetch.test.ts → ✓ src/media/fetch.test.ts (5 tests)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios: ran targeted unit tests for /Users/afurm/Development/openclaw/src/media/fetch.test.ts; confirmed new cases pass for both filename= and filename*= handling.
Edge cases checked: Windows-style backslash separators, percent-encoded backslashes in RFC5987 filename*.
What you did not verify: full repo test suite, live channel/integration end-to-end flows, Windows runtime behavior.

Compatibility / Migration

Backward compatible? (Yes/No) Yes
Config/env changes? (Yes/No) No
Migration needed? (Yes/No) No
If yes, exact upgrade steps: N/A

Failure Recovery (if this breaks)

How to disable/revert this change quickly: revert commit 521f3e428 (or restore the two touched files and redeploy).
Files/config to restore: /Users/afurm/Development/openclaw/src/media/fetch.ts, /Users/afurm/Development/openclaw/src/media/fetch.test.ts
Known bad symptoms reviewers should watch for: unexpected filename truncation/empty filenames from unusual servers (fallback naming should still apply downstream).

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

Risk: Some non-malicious servers may intentionally include backslashes/control characters in display filenames, and these will now be normalized/stripped.
Mitigation: Sanitization is limited to untrusted Content-Disposition filename parsing only; network behavior and storage paths are unchanged, and regression tests cover both standard and RFC5987 header formats.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 521f3e4281

ℹ️ 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".

greptile-apps · 2026-02-26T18:10:12Z

Greptile Summary

Adds filename sanitization to prevent Windows-style path traversal in Content-Disposition headers parsed on POSIX hosts.

Introduced sanitizeContentDispositionFileName helper that normalizes backslashes to forward slashes, strips control characters, and extracts basename using path.posix.basename
Applied to both standard filename= and RFC5987 filename*= header formats
Added regression tests covering Windows-style traversal (..\\..\\secret.pdf) and percent-encoded backslashes (..%5C..%5C)
No changes to network behavior, storage layout, or file-write destinations
Normal filenames without traversal segments or control characters pass through unchanged

Confidence Score: 5/5

This PR is safe to merge with minimal risk
Focused security hardening fix with clear implementation, comprehensive test coverage for both standard and RFC5987 Content-Disposition formats, proper Unicode handling, and no breaking changes to existing functionality
No files require special attention

_{Last reviewed commit: 521f3e4}

afurm · 2026-02-26T18:19:25Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47c9f247cd

ℹ️ 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".

afurm · 2026-02-26T18:47:01Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa9f075748

ℹ️ 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".

chatgpt-codex-connector · 2026-02-26T18:51:52Z

+    if (char === "/") {
+      segment = "";
+      continue;


Preserve last segment when value ends with a separator

The new basename helper clears segment whenever it sees /, so inputs ending in a slash now sanitize to an empty string (for example filename="foo/bar/" or filename*=...foo%2Fbar%2F). Before this commit, path.basename returned bar, so this is a regression that can silently discard the server-provided filename and force fallback naming in fetchRemoteMedia.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2dca557b79

ℹ️ 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".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb03c0f298

ℹ️ 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".

chatgpt-codex-connector · 2026-03-02T19:53:54Z

+      if (next !== '"' && next !== "'") {
+        segment = "";
+        continue;


Preserve multiple escaped backslashes before quotes

basenameWithWindowsSeparatorHeuristic only keeps a backslash when the immediate next character is a quote, so after unescapeHttpQuotedPair a decoded sequence like \\\" (two literal backslashes before a quote) is mishandled: the first backslash is treated as a separator and clears the accumulated basename. In a valid quoted-string filename that contains two backslashes before a quote, this truncates the filename stem (e.g. foo is dropped and only \"bar.txt remains), which is a behavioral regression for compliant Content-Disposition values.

Useful? React with 👍 / 👎.

openclaw-barnacle · 2026-04-06T04:20:50Z

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

openclaw-barnacle · 2026-04-13T04:27:10Z

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

openclaw-barnacle · 2026-04-17T04:22:15Z

Closing due to inactivity.
If you believe this PR should be revived, post in #pr-thunderdome-dangerzone on Discord to talk to a maintainer.
That channel is the escape hatch for high-quality PRs that get auto-closed.

Media: sanitize content-disposition filenames

521f3e4

afurm marked this pull request as ready for review February 26, 2026 18:07

openclaw-barnacle Bot added the size: S label Feb 26, 2026

chatgpt-codex-connector Bot reviewed Feb 26, 2026

View reviewed changes

Comment thread src/media/fetch.ts Outdated

Media: preserve quoted-pair escapes in filenames

076d6a5

afurm added 2 commits February 26, 2026 20:19

Merge branch 'main' into fix/media-content-disposition-filename-sanitize

5362a13

Merge branch 'main' into fix/media-content-disposition-filename-sanitize

47c9f24

chatgpt-codex-connector Bot reviewed Feb 26, 2026

View reviewed changes

Comment thread src/media/fetch.ts Outdated

afurm added 2 commits February 26, 2026 20:46

Media: preserve quoted backslashes before quotes

802acd1

Merge branch 'main' into fix/media-content-disposition-filename-sanitize

aa9f075

chatgpt-codex-connector Bot reviewed Feb 26, 2026

View reviewed changes

Merge branch 'main' into fix/media-content-disposition-filename-sanitize

2dca557

chatgpt-codex-connector Bot reviewed Feb 26, 2026

View reviewed changes

Comment thread src/media/fetch.ts

Merge branch 'main' into fix/media-content-disposition-filename-sanitize

cb03c0f

chatgpt-codex-connector Bot reviewed Mar 2, 2026

View reviewed changes

Merge branch 'main' into fix/media-content-disposition-filename-sanitize

d2da2d3

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Apr 6, 2026

Merge branch 'main' into fix/media-content-disposition-filename-sanitize

0f6517a

openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Apr 8, 2026

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Apr 13, 2026

openclaw-barnacle Bot closed this Apr 17, 2026

masatohoshino mentioned this pull request Apr 27, 2026

fix(media): strip control and bidi characters from outbound filenames #72973

Closed

23 tasks

Uh oh!

Conversation

afurm commented Feb 26, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

greptile-apps Bot commented Feb 26, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

afurm commented Feb 26, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

afurm commented Feb 26, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot Feb 26, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 2, 2026

Choose a reason for hiding this comment

Uh oh!

openclaw-barnacle Bot commented Apr 6, 2026

Uh oh!

openclaw-barnacle Bot commented Apr 13, 2026

Uh oh!

openclaw-barnacle Bot commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant