From 475b0c667081851a49464e5454c496333386e2ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20Pr=C3=A9mont?= Date: Wed, 25 Feb 2026 13:46:56 -0500 Subject: [PATCH] fix: strip glob suffixes from allowWrite/denyWrite paths on Linux wrapWithSandbox() and generateFilesystemArgs() did not strip trailing /** from allowWrite and denyWrite paths before generating bubblewrap --bind mounts. fs.existsSync() failed on the literal glob path (e.g. "/home/user/project/**") and silently skipped the mount, leaving the directory read-only under --ro-bind / /. macOS was unaffected because generateWriteRules() converts globs to Seatbelt regex patterns. The same config produced correct results on macOS but broken results on Linux. The fix applies removeTrailingGlobSuffix() and containsGlobChars() filtering to allowWrite and denyWrite in both wrapWithSandbox() (the SandboxManager API path) and generateFilesystemArgs() (defense in depth), matching the existing treatment of denyRead paths and the logic in getFsWriteConfig(). Co-Authored-By: Claude Opus 4.6 (1M context) --- src/sandbox/sandbox-manager.ts | 24 +++++++-- src/sandbox/sandbox-utils.ts | 3 +- test/sandbox/wrap-with-sandbox.test.ts | 67 ++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/src/sandbox/sandbox-manager.ts b/src/sandbox/sandbox-manager.ts index c39579e..87a4add 100644 --- a/src/sandbox/sandbox-manager.ts +++ b/src/sandbox/sandbox-manager.ts @@ -514,12 +514,30 @@ async function wrapWithSandbox( // Get configs - use custom if provided, otherwise fall back to main config // If neither exists, defaults to empty arrays (most restrictive) // Always include default system write paths (like /dev/null, /tmp/claude) - const userAllowWrite = - customConfig?.filesystem?.allowWrite ?? config?.filesystem.allowWrite ?? [] + // + // Strip trailing /** and filter remaining globs on Linux (bwrap needs + // real paths, not globs; macOS subpath matching is also recursive so + // stripping is harmless there). + const stripWriteGlobs = (paths: string[]): string[] => + paths + .map(p => removeTrailingGlobSuffix(p)) + .filter(p => { + if (getPlatform() === 'linux' && containsGlobChars(p)) { + logForDebugging( + `[Sandbox] Skipping glob write pattern on Linux: ${p}`, + ) + return false + } + return true + }) + const userAllowWrite = stripWriteGlobs( + customConfig?.filesystem?.allowWrite ?? config?.filesystem.allowWrite ?? [], + ) const writeConfig = { allowOnly: [...getDefaultWritePaths(), ...userAllowWrite], - denyWithinAllow: + denyWithinAllow: stripWriteGlobs( customConfig?.filesystem?.denyWrite ?? config?.filesystem.denyWrite ?? [], + ), } const rawDenyRead = customConfig?.filesystem?.denyRead ?? config?.filesystem.denyRead ?? [] diff --git a/src/sandbox/sandbox-utils.ts b/src/sandbox/sandbox-utils.ts index a63eb47..93edbc8 100644 --- a/src/sandbox/sandbox-utils.ts +++ b/src/sandbox/sandbox-utils.ts @@ -69,7 +69,8 @@ export function containsGlobChars(pathPattern: string): boolean { * Used to normalize path patterns since /** just means "directory and everything under it" */ export function removeTrailingGlobSuffix(pathPattern: string): string { - return pathPattern.replace(/\/\*\*$/, '') + const stripped = pathPattern.replace(/\/\*\*$/, '') + return stripped || '/' } /** diff --git a/test/sandbox/wrap-with-sandbox.test.ts b/test/sandbox/wrap-with-sandbox.test.ts index 7897645..0c7581b 100644 --- a/test/sandbox/wrap-with-sandbox.test.ts +++ b/test/sandbox/wrap-with-sandbox.test.ts @@ -4,6 +4,9 @@ import type { SandboxRuntimeConfig } from '../../src/sandbox/sandbox-config.js' import { getPlatform } from '../../src/utils/platform.js' import { wrapCommandWithSandboxLinux } from '../../src/sandbox/linux-sandbox-utils.js' import { wrapCommandWithSandboxMacOS } from '../../src/sandbox/macos-sandbox-utils.js' +import { mkdirSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' /** * Create a test configuration with network access @@ -651,3 +654,67 @@ describe('empty allowedDomains network blocking (CVE fix)', () => { }) }) }) + +describe('allowWrite glob suffix handling', () => { + const command = 'echo hello' + + it('allowWrite with /** suffix includes path in sandbox command', async () => { + if (skipIfUnsupportedPlatform()) { + return + } + + const testDir = join(tmpdir(), `srt-test-glob-allow-${Date.now()}`) + mkdirSync(testDir, { recursive: true }) + + try { + await SandboxManager.reset() + await SandboxManager.initialize({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { + denyRead: [], + allowWrite: [`${testDir}/**`], + denyWrite: [], + }, + }) + + const result = await SandboxManager.wrapWithSandbox(command) + + expect(result).not.toBe(command) + expect(result).toContain(testDir) + } finally { + await SandboxManager.reset() + rmSync(testDir, { recursive: true, force: true }) + } + }) + + it('denyWrite with /** suffix within allowed parent includes both paths', async () => { + if (skipIfUnsupportedPlatform()) { + return + } + + const parentDir = join(tmpdir(), `srt-test-glob-deny-${Date.now()}`) + const childDir = join(parentDir, 'denied') + mkdirSync(childDir, { recursive: true }) + + try { + await SandboxManager.reset() + await SandboxManager.initialize({ + network: { allowedDomains: [], deniedDomains: [] }, + filesystem: { + denyRead: [], + allowWrite: [parentDir], + denyWrite: [`${childDir}/**`], + }, + }) + + const result = await SandboxManager.wrapWithSandbox(command) + + expect(result).not.toBe(command) + expect(result).toContain(parentDir) + expect(result).toContain(childDir) + } finally { + await SandboxManager.reset() + rmSync(parentDir, { recursive: true, force: true }) + } + }) +})