Skip to content

fix(sandbox): auto-deny dangerous paths and .git/hooks across all allowedWrite roots - #518

Open
connerkup wants to merge 1 commit into
anthropics:mainfrom
connerkup:feat/deny-git-hooks-write
Open

fix(sandbox): auto-deny dangerous paths and .git/hooks across all allowedWrite roots#518
connerkup wants to merge 1 commit into
anthropics:mainfrom
connerkup:feat/deny-git-hooks-write

Conversation

@connerkup

Copy link
Copy Markdown

Summary

Ensures mandatory write-denies (such as `.git/hooks`, `.git/config`, `.vscode`, and shell dotfiles) are automatically discovered and denied across all directories specified in `allowedWritePaths` / `allowOnly`, rather than solely `process.cwd()`.

Problem

Previously, `linuxGetMandatoryDenyPaths` and `macGetMandatoryDenyPatterns` only scanned `process.cwd()`. When a sandboxed command is executed with `allowOnly` or `--allow-write` targeting an external workspace directory outside CWD (common in multi-repo workflows or orchestrator harnesses), mandatory deny rules were not generated for that target path. This allowed sandboxed processes to write into `.git/hooks/` of the allowed repository, enabling potential host-level code execution on subsequent git operations outside the sandbox.

Additionally, containment checks in `linux-sandbox-utils.ts` used string concatenation (`candidatePath.startsWith(allowedPath + '/')`), which evaluates to `'//'` when `/` is an allowed write root, inadvertently bypassing deny-bind generation.

Solution

  • Linux: Updated `linuxGetMandatoryDenyPaths` to accept `scanRoots: string[]` and scan all directories present in `allowedWritePaths` alongside `process.cwd()`.
  • Linux: Replaced string-prefix concatenation with `isAtOrUnder` in `isWithinAnyAllowedWritePath` and `findSymlinkInPath` to prevent root `/` containment bypass.
  • macOS: Canonicalized and normalized `config.allowOnly` paths before populating `scanRoots` to ensure tilde paths and symlinks (`/var` -> `/private/var`) are preserved.
  • macOS: Updated `macGetMandatoryDenyPatterns` to emit root-specific static paths and dual directory/subtree globs for `.git/hooks`.
  • Added end-to-end integration test verifying that writes to `.git/hooks` in allowed paths outside CWD fail with non-zero status and leave original hooks intact.
  • Added unit tests verifying multi-root pattern emission on macOS.

Test Plan

  • Integration test in `test/sandbox/mandatory-deny-paths.test.ts`: spawns an external repository outside `process.cwd()`, configures `allowOnly`, and confirms writes to `.git/hooks/pre-commit` fail (`EROFS` on Linux, `EPERM` on macOS).
  • Full CI matrix verification:
    • `Tests (linux / x86-64)`: Passed
    • `Tests (linux / arm64)`: Passed
    • `Tests (macos / arm64)` (Apple Silicon `macos-14` runner): Passed
    • `Tests (docker / x86-64)` & `Tests (docker / arm64)`: Passed
    • `npm run typecheck` & `npm run lint:check`: Passed cleanly with zero errors.

…owedWrite roots

* linux: scan all allowedWritePaths alongside process.cwd() for mandatory deny targets (.git/hooks, .git/config, dotfiles)
* linux: use isAtOrUnder for isWithinAnyAllowedWritePath and findSymlinkInPath to prevent root '/' containment bypass
* macos: normalize allowOnly roots before scanRoots filtering to avoid dropping tildes or symlinked paths
* macos: emit root-specific static paths and dual directory/subtree globs for .git/hooks
* test: add integration test for external allowedWrite repo and unit test for multi-root pattern emission
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