Files
sandbox-runtime/test/cli-config-loading.test.ts
Andy Locascio 868a04af2e Replace file watcher with fd 3 control channel for config updates
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>
2026-01-14 14:35:12 -08:00

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')
})
})