fix(path): reject Windows reserved basenames in sanitizeUntrustedFileName

[SebTardif]

What Problem This Solves

Fixes an issue where consumers using sanitizeUntrustedFileName() (via staging, external output, and sibling-temp helpers) would accept Windows reserved device basenames such as CON, NUL.txt, or COM1 from untrusted input. On Windows those names open devices rather than ordinary files, which breaks portable name sanitization already used for C0/C1 and Windows-invalid characters on every host.

Why This Change Was Made

Reuse the reserved-name set and normalization already used by the device-path read guards (isWindowsReservedDeviceBaseName) inside sanitizeUntrustedFileName(). Reserved basenames (including extensions and trailing spaces/dots) return the caller-supplied fallback. Non-goals: changing path resolution or read guards; only the untrusted basename sanitizer.

User Impact

Callers that pass reserved basenames now receive the same fallback they already get for empty, ., or .. names. Valid names such as console.txt and null.pdf are unchanged. No public API signature changes.

Evidence

Red / green

• Red: pnpm exec vitest run test/filename.test.ts failed with expected 'CON' to be 'fallback.bin' before the production change.
• Green: same file, 4/4 tests pass after the fix; full pnpm check (598 passed, 22 skipped).

Runtime sample (built dist/)

"CON" -> fallback.bin
"nul.txt" -> fallback.bin
"COM1" -> fallback.bin
"LPT9.doc" -> fallback.bin
"aux " -> fallback.bin
"console.txt" -> console.txt
"report.pdf" -> report.pdf

Checklist

• Tests added or updated when behavior changed
• Security and compatibility impact considered
• CHANGELOG.md updated when release-relevant
• No credentials, private paths, private hosts, or sensitive contents included

Related

• Same-repo reserved-name read guards: #22
• Hardened staging / portable basename sanitization: #62
• Node.js path reserved-name edge cases: nodejs/node#64266

Real behavior proof

• Behavior or issue addressed: Untrusted basenames that are Windows reserved devices must not pass through sanitizeUntrustedFileName; they must fall back.

• Real environment tested: macOS arm64, Node from repo toolchain, @openclaw/fs-safe branch fix/windows-reserved-basenames after pnpm build.

• Exact steps or command run after this patch:

  pnpm install --frozen-lockfile
  pnpm exec vitest run test/filename.test.ts
  pnpm build
  node --input-type=module -e "import { sanitizeUntrustedFileName } from './dist/filename.js';
  for (const c of ['CON','nul.txt','console.txt']) console.log(c, '->', sanitizeUntrustedFileName(c, 'fallback.bin'));"
  pnpm check

• Evidence after fix: reserved cases map to fallback.bin; non-reserved console.txt kept; full check green.

• Observed result after fix: sanitizeUntrustedFileName('CON', 'fallback.bin') === 'fallback.bin' and siblings; staging helpers inherit the safer basename policy.

• What was not tested: Live Windows NTFS create of reserved names (behavior is documented OS semantics; this PR blocks the name before open).

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed July 30, 2026, 7:51 AM ET / 11:51 UTC.

ClawSweeper review

What this changes

The PR makes sanitizeUntrustedFileName() return its fallback for Windows reserved device basenames such as CON, NUL.txt, COM1, and trailing-dot/space variants, with regression tests and a changelog entry.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

This PR still addresses a distinct missing hardening step and has credible after-fix runtime proof. It is not safe to close as implemented because the supplied context does not establish that current main contains the sanitizer change; the branch is also reported dirty against current main, so it needs a merge refresh before final landing.

Priority: P2
Reviewed head: c47b4e59718f1eacd2b26645a3445d02c80b372d

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	This is a focused and well-evidenced hardening patch without a concrete code defect; refreshing the dirty branch is the remaining merge-readiness step.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The PR body supplies after-fix built-package terminal output for rejected and preserved names, supplemented by focused tests and a full repository check.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body supplies after-fix built-package terminal output for rejected and preserved names, supplemented by focused tests and a full repository check.
Evidence reviewed	5 items	Proposed sanitizer guard: The PR imports the existing Windows reserved-device predicate into the filename sanitizer and returns the caller fallback before length truncation when the basename is reserved. Focused regression coverage: The new test covers reserved names with extensions and ignored trailing whitespace, while preserving ordinary names that merely contain similar text. Shared guard history: The reused predicate follows the Windows reserved-device normalization introduced by the merged device-read hardening work, rather than creating a second independent reserved-name list.
Findings	None	None.
Security	None	None.

How this fits together

sanitizeUntrustedFileName() is the portable basename filter used before staging and output helpers create files from untrusted names. It converts an input filename into a safe basename or caller-provided fallback that downstream filesystem operations use.

flowchart LR
  A[Untrusted filename] --> B[Basename extraction]
  B --> C[Portable filename sanitizer]
  C --> D{Reserved Windows device name?}
  D -->|Yes| E[Caller fallback filename]
  D -->|No| F[Sanitized safe basename]
  E --> G[Staging and output helpers]
  F --> G

Loading

Before merge

• Resolve merge risk (P1) - GitHub reports this branch as dirty against newer main; rebase or otherwise refresh the actual merge result before landing it.
• Resolve merge risk (P2) - Existing callers using a Windows reserved device basename will intentionally receive their fallback after upgrade, so maintainers should preserve the documented fallback semantics.
• Complete next step (P2) - No code repair is identified; ordinary maintainer review should refresh the dirty branch against current main and then assess the exact merge result.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch scope	30 added, 0 removed across 4 files	The change is limited to one sanitizer guard, its shared predicate access, targeted tests, and release documentation.
Reserved-name coverage	6 rejected, 2 preserved	The test distinguishes device-name variants from names that merely contain reserved tokens as prefixes or suffixes.

Root-cause cluster

Relationship: canonical
Canonical: #67
Summary: This PR is the focused remaining implementation of Windows reserved-device rejection in the portable filename sanitizer; the related merged work is adjacent rather than a duplicate.

Members:

• adjacent_distinct: Reject unsafe device read paths #22 - It established reserved-name checks for device-path reads but does not add the filename-sanitizer fallback behavior proposed here.
• adjacent_distinct: feat: harden atomic replacement and output staging #62 - It hardened output staging and consumes sanitized fallback names but does not add this missing reserved-basename check.

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

Merge-risk options

Maintainer options:

1. Refresh and validate the merge result (recommended)
   Rebase onto current main, preserve the shared reserved-device predicate, and rerun the focused filename suite plus pnpm check on the refreshed head.
2. Pause if fallback behavior is disputed
   If maintainers do not want reserved device names to follow the existing invalid-basename fallback contract, pause for an explicit public API decision rather than merging a different implicit behavior.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Rebase onto current main; preserve the shared reserved-device predicate; run the focused filename suite and pnpm check on the refreshed head.

Technical review

Best possible solution:

Refresh the focused guard against current main, retain the single shared reserved-device normalization rule, and land it with the reserved and non-reserved basename regression coverage intact.

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

Yes, at source level: the supplied regression test defines the failing reserved-basename path and the PR includes built-package terminal output showing the corrected behavior. Independent local execution was not available because the sandbox command runner failed before it launched a shell.

Is this the best way to solve the issue?

Yes: reusing the existing reserved-device normalization is narrower and less likely to drift than maintaining another reserved-name list in the filename sanitizer.

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

• P2: This is a bounded filesystem-safety correction with concrete cross-platform effects but no demonstrated active outage.
• merge-risk: 🚨 compatibility: After upgrade, callers that pass Windows reserved device basenames receive their configured fallback rather than the original input basename.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body supplies after-fix built-package terminal output for rejected and preserved names, supplemented by focused tests and a full repository check.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies after-fix built-package terminal output for rejected and preserved names, supplemented by focused tests and a full repository check.

Evidence

What I checked:

• Proposed sanitizer guard: The PR imports the existing Windows reserved-device predicate into the filename sanitizer and returns the caller fallback before length truncation when the basename is reserved. (src/filename.ts:1, c47b4e59718f)
• Focused regression coverage: The new test covers reserved names with extensions and ignored trailing whitespace, while preserving ordinary names that merely contain similar text. (test/filename.test.ts:21, c47b4e59718f)
• Shared guard history: The reused predicate follows the Windows reserved-device normalization introduced by the merged device-read hardening work, rather than creating a second independent reserved-name list. (src/device-path.ts:104, 5bf301406851)
• Runtime proof: The PR body reports a built-package terminal run where reserved names resolve to fallback.bin, ordinary names remain unchanged, the focused filename suite passes, and pnpm check passes. (test/filename.test.ts:21, c47b4e59718f)
• Unmerged release status: The PR remains open and the supplied latest release is v0.5.0, so this proposed behavior is not established as shipped implementation. (CHANGELOG.md:1, 66201c1f347a)

Likely related people:

• amknight: Authored the merged device-read hardening work that established the reused reserved-device normalization and guard surface. (role: introduced shared device-name protection; confidence: high; commits: 5bf301406851; files: src/device-path.ts)
• steipete: Authored the merged output-staging hardening work that depends on portable fallback filename handling near this sanitizer boundary. (role: recent adjacent contributor; confidence: medium; commits: 66e59e0ce95e; files: src/filename.ts, src/device-path.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Rebase or refresh against current main and rerun the stated focused and full validation commands.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• 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.

History

Review history (1 earlier review cycle)

• reviewed 2026-07-27T15:36:27.515Z sha c47b4e5 :: needs maintainer review before merge. :: none