Skip to content

fix(workspace): check the deny list through links; write files atomically - #664

Merged
LinuxDevil merged 1 commit into
mainfrom
fix/workspace-git-link-atomic
Oct 16, 2026
Merged

LinuxDevil merged 1 commit into
mainfrom
fix/workspace-git-link-atomic

Conversation

@LinuxDevil

Copy link
Copy Markdown
Owner

Problem

  • Security: the file tools' deny list (.git by default) only checked the path as typed. A symlink or Windows junction inside the workspace that points at .git (gitj -> .git) got around it. edit_file gitj/config changed the real .git/config (for example core.hooksPath, which runs code on the next git command), and write_file gitj/PWNED worked. NodeWorkspace.resolve() computed the realpath but never ran the deny check on it.
  • NodeWorkspace.writeFile used fs.writeFile (truncate, then write), so a process killed mid-write left a truncated file.

Fix

  • New optional FsProvider.realPath(path) returns the workspace-relative path after following links. For a missing path it returns the nearest existing ancestor's realpath plus the rest. NodeWorkspace implements it.
  • createFsTools wraps a provider that has realPath so every path-taking call (readFile, readFileBytes, writeFile, stat, readdir, getMode, chmod, mkdir, rm) also runs the deny check on the resolved path. This covers read_file, write_file, edit_file, list_dir, and glob/grep given a link as path. write_file/edit_file run the check before checkpoint tracking, so .git contents are never read into a backup. Walks still never follow links. fsOperations.ts is unchanged.
  • NodeWorkspace.writeFile now uses the existing writeFileAtomic (temp file in the same dir, fsync, rename, temp removed on failure), which gained a mode option to keep the replaced file's permission bits. Read-only targets and directories are still refused with the same messages.
  • Docs: workspace-tools.md now describes links into .git (including the explicit grep path case) and atomic writes. CHANGELOG, API reports and llms-full.txt are updated.

Verification

  • New tests in NodeWorkspace.test.ts:
    • A junction to .git and one to .git/hooks (on POSIX, directory symlinks). Every tool path is refused and .git stays unchanged.
    • Directory and file symlinks (skipped on EPERM).
    • On Windows, a junction whose target is spelled .GIT.
    • deny: [] opens the link again; realPath values are checked.
    • Atomic replace: a hard link to the old file keeps the old content and no temp file is left. Mode is preserved (POSIX). Read-only and directory targets are refused.
  • With the guard disabled, the junction and .GIT tests fail: the read returned .git/config, and write_file wrote upper/PWNED.
  • The existing escape-to-outside-the-workspace test still passes.
  • tsc --noEmit, npm run lint, npm run build and api:update are clean. vitest run src/storage src/tools/workspace: 394 passed, 6 skipped (Windows symlink EPERM).

🤖 Generated with Claude Code

…ally

A symlink or junction inside the workspace pointing at .git let edit_file
rewrite .git/config and write_file create files in .git: the deny check
only looked at the typed path. The file tools now also check where a path
really leads, via the new optional FsProvider.realPath (NodeWorkspace has
it), for reads, writes, edits, list_dir and glob/grep link paths.

NodeWorkspace.writeFile now writes a temp file and renames it into place,
keeping the replaced file's mode, so a killed process cannot leave a
truncated file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@LinuxDevil
LinuxDevil merged commit 81046d7 into main Oct 16, 2026
4 of 8 checks passed
@LinuxDevil
LinuxDevil deleted the fix/workspace-git-link-atomic branch October 16, 2026 20:46
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