Skip to content

fix(server): canonicalise media paths the same way their callers do - #9575

Merged
juliusmarminge merged 1 commit into
windows-tests/antigravity-hostfrom
windows-tests/media-realpath
Sep 4, 2026
Merged

fix(server): canonicalise media paths the same way their callers do#9575
juliusmarminge merged 1 commit into
windows-tests/antigravity-hostfrom
windows-tests/media-realpath

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 4, 2026

Copy link
Copy Markdown
Member

openMediaFile rejected a file whenever fs/promises.realpath disagreed with
the path it was handed. Callers canonicalise through Effect's
FileSystem.realPath, which is Node's JS realpath; the promises variant is the
native binding, and on Windows it also expands 8.3 short names, so every
media file under a short-named temp or profile directory was refused as a
symlink swap. Use the same realpath as the callers, so the check still
catches a planted link but agrees with a path that is already canonical.

The favicon and asset tests now build their relative-path expectations with
Path.join rather than a literal '/'.

Part of the Windows test-suite stack rooted at #9564; the manual Windows lane comes from #9538.

Model: Claude Fable 5 in Claude Code.

🤖 Generated with Claude Code


Note

Medium Risk
Touches security-sensitive media open acceptance logic; behavior can differ only where the two Node realpath APIs disagree (notably Windows short paths).

Overview
openMediaFile no longer uses fs/promises.realpath for its “path must already be canonical” guard. It now calls callback fs.realpath via realpathLikeFileSystem, matching how upstream code canonicalises with Effect FileSystem.realPath. That fixes false rejections on Windows when the promises API expands 8.3 short names while the caller’s path is already canonical; symlink-swap protection is unchanged in intent.

AssetAccess and ProjectFaviconResolver tests now assert sourcePath / resolved favicon paths with Path.join instead of hard-coded / segments, so expectations match platform-native separators.

Reviewed by Cursor Bugbot for commit 2c64068. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Use callback-based Node realpath in openMediaFile descriptor checks

  • Adds realpathLikeFileSystem, a Promise wrapper around the callback-based Node fs.realpath, and replaces the fs/promises.realpath call in openMediaFile's acceptance checks with it.
  • This aligns the canonicalization with how callers produce paths, fixing mismatches where the two Node realpath APIs return different representations (e.g. Windows 8.3 short names).
  • Updates AssetAccess and ProjectFaviconResolver tests to assert exact platform-native joined paths instead of slash-separated substring checks.
  • Behavioral Change: openMediaFile may now accept or reject files differently for paths whose fs/promises.realpath and callback-based fs.realpath disagree; reviewers should check the descriptor acceptance logic in MediaFile.ts.

Macroscope summarized 2c64068.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.6 KiB 15.1 KiB
Codex Thread snapshot wire 7.0 KiB 7.3 KiB
Codex Live turn WebSocket wire 6.5 KiB 7.8 KiB
Codex Live turn WebSocket decoded 57.0 KiB 66.4 KiB
Codex Live turn messages 9 21
Claude Total thread wire 13.6 KiB 15.1 KiB
Claude Thread snapshot wire 7.0 KiB 7.3 KiB
Claude Live turn WebSocket wire 6.5 KiB 7.8 KiB
Claude Live turn WebSocket decoded 57.8 KiB 66.4 KiB
Claude Live turn messages 9 21

Baseline: unavailable · PR result: 2c64068 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.8 KiB
  • Claude decoded thread snapshot: 114.5 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@juliusmarminge
juliusmarminge force-pushed the windows-tests/media-realpath branch from d948799 to b039eb7 Compare September 4, 2026 03:20
@juliusmarminge
juliusmarminge force-pushed the windows-tests/media-realpath branch 2 times, most recently from ea4c4e0 to a2b013d Compare September 4, 2026 04:23
@juliusmarminge
juliusmarminge force-pushed the windows-tests/media-realpath branch from a2b013d to f92bec2 Compare September 4, 2026 04:54
@juliusmarminge
juliusmarminge force-pushed the windows-tests/media-realpath branch from f92bec2 to f03a2e4 Compare September 4, 2026 07:20
@juliusmarminge
juliusmarminge force-pushed the windows-tests/media-realpath branch from f03a2e4 to 0d2c372 Compare September 4, 2026 07:20
@juliusmarminge
juliusmarminge force-pushed the windows-tests/media-realpath branch from 0d2c372 to 696fd95 Compare September 4, 2026 07:28
@juliusmarminge
juliusmarminge force-pushed the windows-tests/media-realpath branch from 696fd95 to 529f71e Compare September 4, 2026 07:32
@juliusmarminge
juliusmarminge marked this pull request as ready for review September 4, 2026 07:35
@macroscopeapp

macroscopeapp Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 529f71e

Macroscope's review found this PR approvable — This is a small, self-contained server bug fix that aligns media-path validation with the canonicalization already used by callers, primarily correcting Windows path handling. Existing file identity and replacement protections remain in place, and the other changes are platform-portable test assertions.

No code changes detected at 2c64068. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge
juliusmarminge force-pushed the windows-tests/media-realpath branch from 529f71e to f7c7159 Compare September 4, 2026 17:42
@juliusmarminge
juliusmarminge force-pushed the windows-tests/media-realpath branch 3 times, most recently from f8e87a6 to 617d458 Compare September 4, 2026 18:07
openMediaFile rejected a file whenever fs/promises.realpath disagreed with
the path it was handed. Callers canonicalise through Effect's
FileSystem.realPath, which is Node's JS realpath; the promises variant is the
native binding, and on Windows it also expands 8.3 short names, so every
media file under a short-named temp or profile directory was refused as a
symlink swap. Use the same realpath as the callers, so the check still
catches a planted link but agrees with a path that is already canonical.

The favicon and asset tests now build their relative-path expectations with
Path.join rather than a literal '/'.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@juliusmarminge
juliusmarminge force-pushed the windows-tests/media-realpath branch from 617d458 to 2c64068 Compare September 4, 2026 20:26
@juliusmarminge
juliusmarminge merged commit 5f4c716 into main Sep 4, 2026
35 of 44 checks passed
@juliusmarminge
juliusmarminge deleted the windows-tests/media-realpath branch September 4, 2026 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant