mirror of
https://github.com/anthropic-experimental/sandbox-runtime.git
synced 2026-05-07 22:20:26 +08:00
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>
111 lines
3.4 KiB
TypeScript
111 lines
3.4 KiB
TypeScript
import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest'
|
|
import * as fs from 'fs'
|
|
import * as os from 'os'
|
|
import * as path from 'path'
|
|
import { loadConfig, loadConfigFromString } from '../src/utils/config-loader.js'
|
|
|
|
describe('loadConfig', () => {
|
|
let tmpDir: string
|
|
let configPath: string
|
|
|
|
beforeEach(() => {
|
|
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'config-test-'))
|
|
configPath = path.join(tmpDir, 'config.json')
|
|
})
|
|
|
|
afterEach(() => {
|
|
fs.rmSync(tmpDir, { recursive: true, force: true })
|
|
})
|
|
|
|
it('should return null when file does not exist', () => {
|
|
const result = loadConfig('/nonexistent/path/config.json')
|
|
expect(result).toBeNull()
|
|
})
|
|
|
|
it('should return null for empty file', () => {
|
|
fs.writeFileSync(configPath, '')
|
|
const result = loadConfig(configPath)
|
|
expect(result).toBeNull()
|
|
})
|
|
|
|
it('should return null for whitespace-only file', () => {
|
|
fs.writeFileSync(configPath, ' \n\t ')
|
|
const result = loadConfig(configPath)
|
|
expect(result).toBeNull()
|
|
})
|
|
|
|
it('should return null and log error for invalid JSON', () => {
|
|
fs.writeFileSync(configPath, '{ invalid json }')
|
|
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
|
|
|
const result = loadConfig(configPath)
|
|
|
|
expect(result).toBeNull()
|
|
expect(consoleSpy).toHaveBeenCalledWith(
|
|
expect.stringContaining('Invalid JSON'),
|
|
)
|
|
consoleSpy.mockRestore()
|
|
})
|
|
|
|
it('should return null and log Zod errors for invalid schema', () => {
|
|
// Valid JSON but missing required fields
|
|
fs.writeFileSync(configPath, JSON.stringify({ network: {} }))
|
|
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
|
|
|
|
const result = loadConfig(configPath)
|
|
|
|
expect(result).toBeNull()
|
|
expect(consoleSpy).toHaveBeenCalledWith(
|
|
expect.stringContaining('Invalid configuration'),
|
|
)
|
|
consoleSpy.mockRestore()
|
|
})
|
|
|
|
it('should return valid config for valid file', () => {
|
|
const validConfig = {
|
|
network: { allowedDomains: ['example.com'], deniedDomains: [] },
|
|
filesystem: { denyRead: [], allowWrite: [], denyWrite: [] },
|
|
}
|
|
fs.writeFileSync(configPath, JSON.stringify(validConfig))
|
|
|
|
const result = loadConfig(configPath)
|
|
|
|
expect(result).not.toBeNull()
|
|
expect(result?.network.allowedDomains).toContain('example.com')
|
|
})
|
|
})
|
|
|
|
describe('loadConfigFromString', () => {
|
|
it('should return null for empty string', () => {
|
|
const result = loadConfigFromString('')
|
|
expect(result).toBeNull()
|
|
})
|
|
|
|
it('should return null for whitespace-only string', () => {
|
|
const result = loadConfigFromString(' \n\t ')
|
|
expect(result).toBeNull()
|
|
})
|
|
|
|
it('should return null for invalid JSON', () => {
|
|
const result = loadConfigFromString('{ invalid json }')
|
|
expect(result).toBeNull()
|
|
})
|
|
|
|
it('should return null for valid JSON with invalid schema', () => {
|
|
// Valid JSON but missing required fields
|
|
const result = loadConfigFromString(JSON.stringify({ network: {} }))
|
|
expect(result).toBeNull()
|
|
})
|
|
|
|
it('should return valid config for valid JSON', () => {
|
|
const validConfig = {
|
|
network: { allowedDomains: ['example.com'], deniedDomains: [] },
|
|
filesystem: { denyRead: [], allowWrite: [], denyWrite: [] },
|
|
}
|
|
const result = loadConfigFromString(JSON.stringify(validConfig))
|
|
|
|
expect(result).not.toBeNull()
|
|
expect(result?.network.allowedDomains).toContain('example.com')
|
|
})
|
|
})
|