When .git is a file (worktrees) or doesn't exist, the mandatory deny for
.git/hooks caused bwrap failures or turned .git into a /dev/null file.
Three fixes: (1) skip denies when a path ancestor is a file, (2) mount
empty directories instead of /dev/null for intermediate non-existent
components, (3) only add .git/hooks and .git/config denies when .git is
a directory. Also updates cleanup to handle empty directory mount points.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PR #80 hardened the sandbox by mounting /dev/null over non-existent deny
paths to prevent their creation, but this caused bwrap to leave empty
"ghost dotfiles" on the host (issue #85), which PR #91 reverted. This
re-introduces the protection with proper cleanup: mount points are
tracked and removed via cleanupBwrapMountPoints(). A new lightweight
cleanupAfterCommand() API is exposed on SandboxManager for callers to
invoke after each command, and the srt CLI calls it on child exit.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add test/utils/which.test.ts for Bun environment testing
- Add test/utils/which-node-test.mjs for Node.js fallback testing
- Update test/sandbox/linux-dependency-error.test.ts to mock
globalThis.Bun.which directly instead of using mock.module
- Update test/sandbox/seccomp-filter.test.ts to use whichSync
- Add src/utils/which.ts with whichSync function that uses Bun.which
in Bun runtime and falls back to spawnSync('which', ...) in Node.js
- Replace spawnSync('which', ...) calls with whichSync in:
- src/sandbox/linux-sandbox-utils.ts (dependency checks, shell lookup)
- src/sandbox/macos-sandbox-utils.ts (shell lookup)
- src/sandbox/sandbox-manager.ts (ripgrep check)
- src/utils/ripgrep.ts (hasRipgrepSync)
This avoids spawning a new process for 'which' lookups when running
in Bun, as Bun.which is a native built-in function.
The previous example called reset() immediately after spawning the
child process, which would shut down proxy servers before the child
process completes. This caused the sandboxed command to fail.
Move reset() inside the 'exit' event callback to ensure cleanup
happens after the child process terminates.
This function runs `npm root -g` which spawns a subprocess. Since the result
is stable for the process lifetime and the function is called from multiple
fallback paths, caching avoids redundant process spawns.
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
WSL2+ uses the same bubblewrap sandboxing as Linux, so it should be
supported. WSL1 does not support bubblewrap properly.
- Add getWslVersion() to detect WSL version from /proc/version
- Update isSupportedPlatform() to check for WSL1 and reject it
- Remove 'wsl' from Platform type (getPlatform returns 'linux' for WSL)
- Update comments to clarify Linux/WSL glob pattern handling
Security improvement: The file watcher approach had a privilege escalation risk
where any process could write to ~/.srt-settings.json to relax sandbox restrictions.
The new fd 3 control channel is secure because only the parent process can write
to a child's file descriptor.
Changes:
- Add --control-fd <fd> flag to read JSON lines config updates from a file descriptor
- Add loadConfigFromString() for parsing config from the control channel
- Remove watchConfigFile() and CONFIG_FILE_SETTLE_DELAY_MS (security risk)
- Add integration tests for --control-fd functionality
- stdin still passes through to child (interactive commands work)
- --settings flag still works for initial config loading
Claude-Generated-By: Claude Code (cli/claude-opus-4-5=100%)
Claude-Steers: 3
Claude-Permission-Prompts: 0
Claude-Escapes: 0
Claude-Plan:
<claude-plan>
# Plan: Replace File Watcher with fd 3 Control Channel (TDD)
## Problem
The current file watcher approach (`~/.srt-settings.json`) has a privilege escalation risk:
- Any process that can write to the config file can relax sandbox restrictions
- A compromised process or sandbox escape could modify the file
## Solution
Replace file watching with fd 3 (file descriptor 3) based config updates. This is secure because:
- Only the parent process can write to the child's fd 3
- A sandboxed process cannot write to srt's fd 3 from outside
- Clean privilege separation tied to process hierarchy
- stdin remains available for the sandboxed command (interactive use still works)
## Design
### Protocol: JSON Lines on fd 3
- Each line is a complete `SandboxRuntimeConfig` JSON object
- Full replacement semantics (not patches)
- Invalid JSON/schema is logged and ignored (keeps old config)
- Empty lines are ignored
Example - Claude Code sends config updates:
```typescript
const srt = spawn('srt', ['--control-fd', '3', '--', 'bash'], {
stdio: ['inherit', 'inherit', 'inherit', 'pipe'] // fd 3 is a writable pipe
})
// Send config update
srt.stdio[3].write('{"network":{"allowedDomains":["example.com"],"deniedDomains":[]},"filesystem":{"denyRead":[],"allowWrite":[],"denyWrite":[]}}\n')
```
### CLI Changes
Add `--control-fd <fd>` flag:
```bash
# Old (file watching - will be removed)
srt --settings ~/.srt-settings.json -- my-command
# New (fd 3 control) - parent must set up fd 3 as a pipe
srt --control-fd 3 -- my-command
# Interactive use still works (no control channel)
srt --debug bash
```
When `--control-fd` is enabled:
1. srt reads from the specified fd for config updates
2. stdin passes through to child normally (interactive commands work)
3. Each valid config line calls `SandboxManager.updateConfig()`
### stdio Configuration
Current:
```typescript
spawn(sandboxedCommand, { shell: true, stdio: 'inherit' })
```
With `--control-fd 3`:
```typescript
// stdin still inherited - interactive commands work!
spawn(sandboxedCommand, { shell: true, stdio: 'inherit' })
// Additionally, srt reads from fd 3 for control messages
const controlStream = fs.createReadStream('', { fd: 3 })
```
### Backward Compatibility
- Remove file watcher entirely (it's a security risk)
- Keep `--settings` flag for initial config loading from file
- Dynamic updates only via `--control-fd`
- Without `--control-fd`, srt works exactly as before (stdin passes through)
## Files to Modify
| File | Changes |
|------|---------|
| `src/cli.ts` | Add `--control-fd` flag, read from fd for config updates, remove file watcher setup |
| `src/utils/config-loader.ts` | Remove `watchConfigFile`, add `loadConfigFromString` |
| `test/cli-config-loading.test.ts` | Remove `watchConfigFile` tests, add `loadConfigFromString` tests |
| `test/control-fd.test.ts` | New file: integration tests for --control-fd |
## TDD Implementation Steps
### Step 1: Write tests for `loadConfigFromString` (RED)
Add to `test/cli-config-loading.test.ts`:
```typescript
describe('loadConfigFromString', () => {
it('should return null for empty string')
it('should return null for invalid JSON')
it('should return null for valid JSON with invalid schema')
it('should return valid config for valid JSON')
})
```
### Step 2: Implement `loadConfigFromString` (GREEN)
In `src/utils/config-loader.ts`:
```typescript
export function loadConfigFromString(content: string): SandboxRuntimeConfig | null {
if (!content.trim()) return null
try {
const parsed = JSON.parse(content)
const result = SandboxRuntimeConfigSchema.safeParse(parsed)
if (!result.success) return null
return result.data
} catch {
return null
}
}
```
### Step 3: Write integration tests for `--control-fd` (RED)
New file `test/control-fd.test.ts`:
```typescript
describe('--control-fd', () => {
it('should update config when receiving valid JSON on control fd')
it('should ignore invalid JSON on control fd')
it('should allow stdin to pass through to child process')
it('should work without --control-fd (backward compat)')
})
```
### Step 4: Implement `--control-fd` in CLI (GREEN)
In `src/cli.ts`:
- Add `.option('--control-fd <fd>', 'read config updates from fd (JSON lines)', parseInt)`
- Add readline listener on the control fd
- Call `loadConfigFromString` and `SandboxManager.updateConfig()` for each line
### Step 5: Remove file watcher (REFACTOR)
- Delete `watchConfigFile` and `CONFIG_FILE_SETTLE_DELAY_MS` from `src/utils/config-loader.ts`
- Delete watchConfigFile tests from `test/cli-config-loading.test.ts`
- Remove file watcher setup from `src/cli.ts`
## Verification
1. Run tests after each step: `yarn test`
2. **Integration test** spawns srt with fd 3 pipe and verifies config updates work
3. **Manual test**: `srt --debug bash` still works (interactive stdin)
4. **Manual test**: `srt --control-fd 3` with pipe from parent works
## Security Considerations
- fd 3 control is secure: only parent process can write to it (pipe is unidirectional)
- FD_CLOEXEC ensures sandboxed child never inherits the control fd
- No file system control channel eliminates privilege escalation risk
- stdin still passes through to child - interactive commands work
- If `--control-fd` is not specified, no control channel exists (safe default)
</claude-plan>
- Extract loadConfig/watchConfigFile to src/utils/config-loader.ts for testability
- Add CONFIG_FILE_SETTLE_DELAY_MS constant (100ms, increased from 50ms)
- Add test for updateConfig called before initialize
- Add tests for loadConfig edge cases: missing file, empty file, invalid JSON, Zod failures
- Add tests for watchConfigFile: file deletion, invalid updates, rename handling
Claude-Generated-By: Claude Code (cli/claude-opus-4-5=100%)
Claude-Steers: 3
Claude-Permission-Prompts: 0
Claude-Escapes: 0
Claude-Plan:
<claude-plan>
# Plan: Fix PR #90 Issues
## Issues to Address
1. Missing tests for config watcher edge cases (file deleted, invalid JSON, Zod failure)
2. Missing test for `updateConfig` before `initialize`
3. Magic number 50ms delay in file watcher
4. Integration tests depend on `example.com`
---
## 1. Remove `example.com` Dependency from Integration Tests
**Problem**: Tests in `test/sandbox/update-config.test.ts` use `curl http://example.com` which requires external network.
**Solution**: The PR already has `proxyRequest()` helper that tests proxy filtering without external network. Replace curl-based integration tests with proxy-based tests.
**File**: `test/sandbox/update-config.test.ts`
**Changes**:
- Remove/refactor tests that spawn `curl http://example.com`
- Keep the `proxyRequest()` tests which already test the same behavior without external network
- The curl tests are redundant since proxy tests verify the same filtering logic
**Tests to modify/remove** (lines ~260-400):
- `should block then allow domain after updateConfig with sandboxed curl` - redundant with proxy test
- `should allow then block domain after updateConfig with sandboxed curl` - redundant with proxy test
- `should handle multiple config updates with sandboxed commands` - redundant with `should handle rapid config updates`
- `should allow network via curl after updateConfig when started with empty allowlist` - keep but convert to proxy-based
---
## 2. Extract and Name the Magic Number
**Problem**: 50ms delay is unexplained and potentially fragile.
**File**: `src/cli.ts`
**Changes**:
```typescript
// At top of file or near watchConfigFile
const CONFIG_FILE_SETTLE_DELAY_MS = 100 // Increased for reliability
// In watchConfigFile, line ~107
setTimeout(() => setupWatcher(), CONFIG_FILE_SETTLE_DELAY_MS)
```
Increase to 100ms for more reliability on slower filesystems.
---
## 3. Add Config Watcher Edge Case Tests
**Problem**: `watchConfigFile` and `loadConfig` are not exported.
**Solution**: Export them for testing, or create a new test file that tests via the CLI.
**Approach**: Export `loadConfig` from cli.ts (it's already useful standalone), create unit tests.
**File**: `src/cli.ts` - add export
**New file**: `test/cli-config-loading.test.ts`
**Tests to add**:
```typescript
describe('loadConfig', () => {
it('should return null when file does not exist')
it('should return null for empty file')
it('should return null and log error for invalid JSON')
it('should return null and log Zod errors for invalid schema')
it('should return valid config for valid file')
})
describe('watchConfigFile', () => {
it('should not call callback when file is deleted')
it('should not call callback for invalid JSON')
it('should not call callback for Zod validation failure')
it('should call callback with new config on valid change')
it('should re-establish watcher after file replacement (rename)')
})
```
---
## 4. Add Test for `updateConfig` Before `initialize`
**File**: `test/sandbox/update-config.test.ts`
**Test to add**:
```typescript
it('should handle updateConfig called before initialize', async () => {
// Ensure clean state
await SandboxManager.reset()
// updateConfig before initialize - should not throw
SandboxManager.updateConfig({
network: { allowedDomains: ['example.com'], deniedDomains: [] },
filesystem: { denyRead: [], allowWrite: [], denyWrite: [] },
})
// Config should be set
expect(SandboxManager.getConfig()).toBeDefined()
// But network infrastructure not ready
expect(SandboxManager.getProxyPort()).toBeUndefined()
// Initialize should still work and respect the pre-set config
await SandboxManager.initialize({
network: { allowedDomains: ['other.com'], deniedDomains: [] },
filesystem: { denyRead: [], allowWrite: [], denyWrite: [] },
})
// initialize() overwrites config
const config = SandboxManager.getConfig()
expect(config?.network.allowedDomains).toContain('other.com')
})
```
---
## Files to Modify
| File | Changes |
|------|---------|
| `src/cli.ts` | Export `loadConfig`, `watchConfigFile`; name magic number constant |
| `test/sandbox/update-config.test.ts` | Remove curl-based tests, add pre-init test |
| `test/cli-config-loading.test.ts` | New file with loadConfig and watchConfigFile tests |
---
## Verification
1. Run full test suite: `yarn test`
2. Verify no tests use external network (grep for `example.com` in test assertions)
3. Verify new edge case tests pass
4. Manual test: modify `~/.srt-settings.json` while srt is running with `--debug` to verify watcher logs
</claude-plan>
