Sign up
View stargazersSign up

fix(server): preserve filename whitespace when reading and saving files#29314

open1
main
GitHub fork: voltcrash/preserve-file-path-whitespace
opened9d agobyVvoltcrash
5 files changed
View on GitHubTwo-way sync enabledPR last imported 22h ago

Closes #29313

Reading "report.txt " currently returns report.txt; saving it overwrites that other file. Leading whitespace fails the same way. Preserve filename whitespace through the read/write RPC inputs, results, errors, and server path resolution. Empty paths and the existing length limits, workspace confinement, symlink checks, and Git-metadata write guards still apply.

This fits the focused obvious-bug exception: one valid filename silently targets another file, risking data loss. It fixes read/save targeting, separately from the duplicate file-tree entries in closed #22847. No UI or workspace-root schema changes.

Proof on Linux / Node v26.12.1:

  • Existing read/save cases extended with RPC codec round trips failed first: wrong contents returned, ordinary file overwritten. The absolute-read extension failed at its separate trim. All pass after the fix, including leading/trailing spaces, tabs, and a whitespace-only filename; traversal is still rejected.
  • Local review found the same trimming in error payloads. An assertion added to the existing structured-error case failed first on the whitespace-only filename; all 6 contract tests pass after correction.
  • Focused service/consumer suites: 107 tests passed across file operations, path resolution, contracts, directory listing, assets, and favicons. vp run check:changed: all stages passed (format, lint, knip, generated GraphQL, typechecks for 35 affected/dependent packages, 37 neighboring tests).
  • Actual service runtime after the fix (before: both reads returned ordinary file, and saving overwrote it):
json
1{"requested":"report.txt ","directRead":"whitespace file","rpcRead":"whitespace file","ordinaryAfterSave":"ordinary file","spacedAfterSave":"edited"}
2{"requested":" report.txt","directRead":"whitespace file","rpcRead":"whitespace file","ordinaryAfterSave":"ordinary file","spacedAfterSave":"edited"}

No native Windows/macOS run or UI claim; unsupported whitespace filename forms are excluded from Windows test rows.

Reviewed with review-loop: 2 rounds with Claude Opus 5.5 on xhigh. Reviewer verdict: approved.

Created with GPT-6.1 Sol in Codex, running in T3 Code.

Fixes pingdotgg/t3code#29313

Checks

22 successful · 6 skipped
All checks passed
ef73ea0
MmacroscopeappBot9d ago
Synced from GitHub

Approvability

Verdict: Approved at 9e3b8c1

Macroscope's review found this PR approvable — File reads and saves now preserve filename whitespace instead of targeting a trimmed name. Existing path-containment and file-access safeguards remain unchanged, keeping the change narrowly scoped.

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

CcoderabbitaiBot9d ago
Synced from GitHub

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Read and write path schemas now accept non-empty paths with surrounding whitespace. Workspace path handling preserves the original path during read targeting and relative-path resolution. Tests cover whitespace-containing filenames, schema round trips, and traversal rejection.

Changes: Whitespace-preserving file paths

Layer / File(s)Summary
Read and write path contracts
packages/contracts/src/project.ts, packages/contracts/src/project.test.ts		Read and write input, result, and error schemas accept non-empty path values without trimming whitespace. Tests verify that error paths with spaces survive encoding and decoding.
Workspace path preservation
apps/server/src/workspace/WorkspacePaths.ts, apps/server/src/workspace/WorkspaceFileSystem.ts, apps/server/src/workspace/WorkspaceFileSystem.test.ts		Workspace path resolution and read targeting use the original path. Tests check whitespace-containing reads and writes, confirm that report.txt remains unchanged during a whitespace-path write, and verify that traversal paths fail with WorkspacePathOutsideRootError.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related PRs

  • pingdotgg/t3code#26254: Both changes involve resolveReadTarget; this change preserves whitespace in paths handled by that resolver.

Suggested reviewers: t3dotgg, juliusmarminge

Merge Risk: External image Minimal · up to ef73e

No actionable issue remains in the reviewed changes; the PR is mergeable after normal checks.

Pre-merge checks: Passed 4

Check nameStatusExplanation
Title checkPassedThe title describes the main change: preserving filename whitespace during reads and saves.
Linked Issues checkPassed[#29313] Read and write schemas preserve path whitespace through RPC inputs and results. Server path resolution uses the original path. Tests cover leading and trailing whitespace, codec round trips, …
Out of Scope Changes checkPassed[#29313] The contract and server changes support correct read and write targeting. The error-path changes preserve the same paths in error payloads. The tests cover this behavior and path confinement.…
Description checkPassedThe description explains the problem and expected behavior, describes the focused fix and scope, and gives specific verification results and limitations. It also explains why the obvious-bug exception…
Full details: Linked Issues check

Explanation

[#29313] Read and write schemas preserve path whitespace through RPC inputs and results. Server path resolution uses the original path. Tests cover leading and trailing whitespace, codec round trips, and writes that leave the ordinary file unchanged. Closed #22847 is background only.

Full details: Out of Scope Changes check

Explanation

[#29313] The contract and server changes support correct read and write targeting. The error-path changes preserve the same paths in error payloads. The tests cover this behavior and path confinement. The summary shows no unrelated changes.

Full details: Description check

Explanation

The description explains the problem and expected behavior, describes the focused fix and scope, and gives specific verification results and limitations. It also explains why the obvious-bug exception applies and identifies the model and harness used.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
External image Recent review info

External image Run configuration

External image Commits

Reviewing files that changed from the base of the PR and between 9e3b8c1d942e6bc9a83e71d054790b8ca3368659 and ef73ea0662ffcba00af23ed213140e729a511eb3.

External image Files selected for processing: 2

  • apps/server/src/workspace/WorkspaceFileSystem.test.ts
  • packages/contracts/src/project.ts

Limit details: You’ve used all 10 included reviews currently available.

Mmaria8d ago

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head 9e3b8c1d942e6bc9a83e71d054790b8ca3368659 on linux with the real dev server, repository Playwright, /usr/bin/google-chrome, and real codex gpt-6.1-sol turns. added a project and threads through the app; codex created the files in a separate real git repository. ran one dev server at a time.

  • reproduced on github/main (a036489016) first: whitespace-path reads returned the ordinary file; saving report.txt overwrote report.txt.
  • on this head, chat file links opened the distinct leading-space, trailing-space, and tab-wrapped files. edited and autosaved both space variants in the real editor, switched files, and reopened them. disk contents and websocket requests confirmed the exact paths, while report.txt stayed unchanged.
  • authenticated websocket read/write round trips also passed for a whitespace-only path (" "), absolute reads, optional missing files, and whitespace-preserving error payloads.
  • nearby behavior passed: normal markdown source edits and rendered task-checkbox saves persisted; traversal and .git/config writes were rejected.
  • blacksmith ran vp test run apps/server/src/workspace/WorkspaceFileSystem.test.ts packages/contracts/src/project.test.ts: 2 files, 33 tests passed. all 33 reported ci checks were green. no source changes or pushes were needed.

runtime transcript

main: clicking the trailing-space file link shows the ordinary file.

main reads the ordinary file for the trailing-space filename

pr: the same filename opens its distinct contents.

pr reads the correct trailing-space file

recording: edit the leading-space file, switch to the saved trailing-space file, check the unchanged ordinary file, then reopen the leading-space edit.

real editor saves whitespace filenames and preserves the ordinary file

Not verified

native desktop, ios/android, macos/windows filesystem behavior, other providers, and relay/tunnel connections were not exercised on this linux vm.

the existing file-tree whitespace normalization is outside this pr's scope: variants collapse into one tree entry, and a whitespace-only basename caused a folder-list encoding failure on main. this verification used actual chat file links for editor access and websocket calls for the whitespace-only path; it does not claim that tree selection is fixed. main also hit a whitespace-only checkpoint encoding failure; after moving that case into a subfolder through the app terminal, the pr's real codex turn and checkpoint completed.

Added 1 commit

  • fix(server): preserve filename whitespace when reading and saving filesef73ea0