Skip to content

fix: sanitise user-editable names in default screenshot filenames - #119

Merged
onevcat merged 1 commit into
mainfrom
fix/screenshot-name-safety
Aug 27, 2026
Merged

onevcat merged 1 commit into
mainfrom
fix/screenshot-name-safety

Conversation

@onevcat

@onevcat onevcat commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #118, which introduced OutputFilePath.safeFilenameComponent for the physical-device screenshot. The same hazard existed on the simulator side and predates #118: the default filename interpolates the FBSimulator name verbatim, and simctl accepts any free text as a name — so a simulator named My iPhone/Work turned the default output into a directory hierarchy under the current directory, and .. segments could walk out of it, breaking the documented "single file in the current directory" default.

Changes

  • IOSSimScreenshotCommand.prepareOutputURL collapses the simulator name into a single path component (/ → -) before building the default filename, matching ios-device screenshot. With separators gone, .. can never form a standalone path component, which closes the traversal case as well.
  • The top-level Android forwarder's default name embeds the adb serial. The router's accepted serial charset ([A-Za-z0-9._:-]) already excludes separators, so this was structurally safe — it is now sanitised too, as defence in depth should the routing rules ever loosen (commented as such).
  • Video paths need no change: sim-use-video-<ISO8601>.<ext> embeds no user-controlled text. Swept the codebase for other name-interpolating filename sites; the only remaining interpolations are daemon socket/pid/log paths keyed by shape-validated UDIDs.

Verification

  • make build — passed, 0 warnings. make test — 1353 passed, 0 warnings; new regression tests pin the slash case (single component, resolves into cwd) and the traversal-shaped name.
  • Live: created a simulator actually named Slash/Name Probe, booted it, ran default-name sim-use screenshot — produced the single file Simulator Screenshot - Slash-Name Probe - <timestamp>.png in the working directory, no directory hierarchy. Probe simulator deleted afterwards.

Not included

The simulator/video "remove the existing --output file before capture" window (a failed capture after prepareOutputURL loses the old file) is a separate pre-existing behaviour, also noted during #118 review — worth its own follow-up (temp + atomic replace, as ios-device screenshot now does) rather than riding along here.

Follow-up to #118, which fixed this for ios-device screenshot: the
simulator default filename interpolated the FBSimulator name verbatim,
and simctl accepts any free text as a name — "My iPhone/Work" turned
the default output into a directory hierarchy, and ".." segments could
walk out of the current directory. The name now passes through
OutputFilePath.safeFilenameComponent, same as the physical-device verb.

The Android default filename embeds the adb serial, whose accepted
router charset already excludes separators; it is sanitised too as
defence in depth. Video default filenames (sim-use-video-<ISO8601>)
embed no user-controlled text and need no change.

Regression tests pin the slash and traversal cases; verified live with
a simulator actually named "Slash/Name Probe".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Signed-off-by: onevcat <onevcat@gmail.com>
@onevcat
onevcat merged commit f3aaa8a into main Aug 27, 2026
4 checks passed
@onevcat
onevcat deleted the fix/screenshot-name-safety branch August 27, 2026 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant