Repository navigation
fix(workspace): check the deny list through links; write files atomically - #664
Merged
Merged
Conversation
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
denylist (.gitby 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/configchanged the real.git/config(for examplecore.hooksPath, which runs code on the next git command), andwrite_file gitj/PWNEDworked.NodeWorkspace.resolve()computed the realpath but never ran the deny check on it.NodeWorkspace.writeFileusedfs.writeFile(truncate, then write), so a process killed mid-write left a truncated file.Fix
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.NodeWorkspaceimplements it.createFsToolswraps a provider that hasrealPathso 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 aspath.write_file/edit_filerun the check before checkpoint tracking, so.gitcontents are never read into a backup. Walks still never follow links.fsOperations.tsis unchanged.NodeWorkspace.writeFilenow uses the existingwriteFileAtomic(temp file in the same dir, fsync, rename, temp removed on failure), which gained amodeoption to keep the replaced file's permission bits. Read-only targets and directories are still refused with the same messages..git(including the explicit grep path case) and atomic writes. CHANGELOG, API reports and llms-full.txt are updated.Verification
NodeWorkspace.test.ts:.gitand one to.git/hooks(on POSIX, directory symlinks). Every tool path is refused and.gitstays unchanged..GIT.deny: []opens the link again;realPathvalues are checked..GITtests fail: the read returned.git/config, andwrite_filewroteupper/PWNED.tsc --noEmit,npm run lint,npm run buildandapi:updateare clean.vitest run src/storage src/tools/workspace: 394 passed, 6 skipped (Windows symlink EPERM).🤖 Generated with Claude Code