diff --git a/packages/code/src/native-sandbox.test.ts b/packages/code/src/native-sandbox.test.ts index 2d40db0..a74f372 100644 --- a/packages/code/src/native-sandbox.test.ts +++ b/packages/code/src/native-sandbox.test.ts @@ -3,11 +3,15 @@ import { spawn } from 'node:child_process'; import { EventEmitter } from 'node:events'; import { access, + chmod, mkdtemp, mkdir, + open, realpath, + rename, rm, stat, + symlink, writeFile, } from 'node:fs/promises'; import { tmpdir, homedir } from 'node:os'; @@ -19,6 +23,7 @@ import type { SandboxRuntimeConfig } from '@anthropic-ai/sandbox-runtime'; import type { ChildProcessWithoutNullStreams } from 'node:child_process'; import { NativeSrtWorkspaceCommandSandbox } from './native-sandbox.js'; +import { restoreScratchTraversal } from './native-scratch.js'; import { WorkspaceToolError } from './workspace.js'; const request = { @@ -439,7 +444,7 @@ test('removes scratch storage after a command revokes traversal permissions', as const result = await sandbox.execute({ ...request, command: - 'printf %s "$TMPDIR"; mkdir "$TMPDIR/locked"; touch "$TMPDIR/locked/file"; chmod 000 "$TMPDIR/locked" "$TMPDIR"', + 'printf %s "$TMPDIR"; mkdir -p "$TMPDIR/locked/deeper"; touch "$TMPDIR/locked/deeper/file"; chmod 000 "$TMPDIR/locked/deeper" "$TMPDIR/locked" "$TMPDIR"', }); assert.equal(result.exitCode, 0); @@ -447,6 +452,122 @@ test('removes scratch storage after a command revokes traversal permissions', as await assert.rejects(access(result.stdout)); }); +test('scratch traversal never follows a descendant replaced after inspection', async (t) => { + if (process.platform === 'win32') return; + const root = await mkdtemp(join(tmpdir(), 'librechat-code-scratch-race-')); + const outside = await mkdtemp(join(tmpdir(), 'librechat-code-outside-')); + const descendant = join(root, 'locked'); + const retired = join(root, 'retired'); + const outsideChild = join(outside, 'child'); + t.after(() => rm(root, { recursive: true, force: true })); + t.after(() => rm(outside, { recursive: true, force: true })); + await mkdir(descendant); + await mkdir(outsideChild); + await chmod(outside, 0o711); + await chmod(outsideChild, 0o711); + const rootHandle = await open(root, 'r'); + t.after(() => rootHandle.close()); + let swapped = false; + + await restoreScratchTraversal(rootHandle, { + async afterEntryInspected(_directoryFd, name) { + if (name !== 'locked' || swapped) return; + swapped = true; + await rename(descendant, retired); + await symlink(outside, descendant, 'dir'); + }, + }); + + assert.equal(swapped, true); + assert.equal((await stat(outside)).mode & 0o777, 0o711); + assert.equal((await stat(outsideChild)).mode & 0o777, 0o711); +}); + +test('scratch traversal removes command-created Darwin ACLs', async (t) => { + if (process.platform !== 'darwin') return; + const root = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + t.after(() => rm(root, { recursive: true, force: true })); + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: root, + manager: fakeManager().manager, + }); + const result = await sandbox.execute({ + ...request, + command: + 'printf %s "$TMPDIR"; mkdir -p "$TMPDIR/locked/deeper"; touch "$TMPDIR/locked/deeper/file"; chmod +a "$USER deny list,search,delete_child" "$TMPDIR/locked" "$TMPDIR"; chmod 000 "$TMPDIR/locked" "$TMPDIR"', + }); + + assert.equal(result.exitCode, 0); + await sandbox.close(); + await assert.rejects(access(result.stdout)); +}); + +test('scratch traversal bounds descriptors and work across a deep tree', async (t) => { + if (process.platform === 'win32') return; + const root = await mkdtemp(join(tmpdir(), 'librechat-code-scratch-depth-')); + t.after(() => rm(root, { recursive: true, force: true })); + const directories = [root]; + for (let depth = 0; depth < 100; depth += 1) { + directories.push(join(directories[directories.length - 1], 'd')); + await mkdir(directories[directories.length - 1]); + } + for (const directory of directories.slice(1).reverse()) { + await chmod(directory, 0o000); + } + const rootHandle = await open(root, 'r'); + t.after(() => rootHandle.close()); + + await restoreScratchTraversal(rootHandle); + + assert.equal((await stat(directories[directories.length - 1])).mode & 0o777, 0o700); +}); + +test('scratch traversal rejects trees beyond its recovery depth limit', async (t) => { + if (process.platform === 'win32') return; + const root = await mkdtemp(join(tmpdir(), 'librechat-code-scratch-depth-limit-')); + t.after(() => rm(root, { recursive: true, force: true })); + let directory = root; + for (let depth = 0; depth < 129; depth += 1) { + directory = join(directory, 'd'); + await mkdir(directory); + } + const rootHandle = await open(root, 'r'); + t.after(() => rootHandle.close()); + + await assert.rejects( + restoreScratchTraversal(rootHandle), + /scratch cleanup exceeded its depth limit/, + ); +}); + +test('does not replace scratch state while cleanup remains pending', async (t) => { + if (process.platform === 'win32') return; + const workspace = await mkdtemp(join(tmpdir(), 'librechat-code-native-')); + const retained = await mkdtemp(join(tmpdir(), 'librechat-code-retained-')); + const retainedHandle = await open(retained, 'r'); + t.after(() => retainedHandle.close()); + t.after(() => rm(retained, { recursive: true, force: true })); + t.after(() => rm(workspace, { recursive: true, force: true })); + const sandbox = new NativeSrtWorkspaceCommandSandbox({ + workspaceRoot: workspace, + manager: fakeManager().manager, + }); + const mutable = sandbox as unknown as { + scratchDirectory?: string; + scratchHandle?: typeof retainedHandle; + createScratchDirectory(paths: string[]): Promise; + }; + mutable.scratchDirectory = retained; + mutable.scratchHandle = retainedHandle; + + await assert.rejects( + mutable.createScratchDirectory([]), + /scratch cleanup is still pending/, + ); + assert.equal(mutable.scratchDirectory, retained); + assert.equal(mutable.scratchHandle, retainedHandle); +}); + const proxyEnvironment = { HTTP_PROXY: 'http://upstream.invalid:8080', HTTPS_PROXY: 'http://upstream.invalid:8080', diff --git a/packages/code/src/native-sandbox.ts b/packages/code/src/native-sandbox.ts index bc57044..49d269c 100644 --- a/packages/code/src/native-sandbox.ts +++ b/packages/code/src/native-sandbox.ts @@ -13,15 +13,13 @@ import { import { constants as fsConstants } from 'node:fs'; import { access, - chmod, - lstat, mkdtemp, open, - readdir, realpath, rm, stat, } from 'node:fs/promises'; +import type { FileHandle } from 'node:fs/promises'; import { SandboxManager } from '@anthropic-ai/sandbox-runtime'; @@ -37,6 +35,7 @@ import { removePrivateStorageAcl, } from './private-storage.js'; import { WorkspaceToolError } from './workspace.js'; +import { restoreScratchTraversal } from './native-scratch.js'; import type { ChildProcessWithoutNullStreams, @@ -245,6 +244,7 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox private initialized?: Promise; private canonicalRoot?: string; private scratchDirectory?: string; + private scratchHandle?: FileHandle; private execution?: Promise; private closing?: Promise; private resetFailed = false; @@ -748,6 +748,11 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox ): Promise { // Windows SRT supplies the restricted account's private TEMP directory. if (this.platform === 'win32') return undefined; + if (this.scratchDirectory || this.scratchHandle) { + throw new Error( + 'Native sandbox scratch cleanup is still pending; close the sandbox before reinitializing', + ); + } const canonicalTemporaryRoot = await canonicalPath(HOST_TEMPORARY_ROOT); const sharedScratchRoot = sharedScratchPaths.find((path) => isWithin(path, canonicalTemporaryRoot), @@ -774,12 +779,16 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox if (((await scratchHandle.stat()).mode & 0o777) !== 0o700) { throw new Error('Native sandbox scratch directory is not private'); } - } finally { + this.scratchHandle = scratchHandle; + } catch (error) { await scratchHandle.close(); + throw error; } this.scratchDirectory = await realpath(scratchDirectory); return this.scratchDirectory; } catch (error) { + await this.scratchHandle?.close().catch(() => undefined); + this.scratchHandle = undefined; await rm(scratchDirectory, { recursive: true, force: true }).catch( () => undefined, ); @@ -810,51 +819,23 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox private async removeScratchDirectory(): Promise { const scratchDirectory = this.scratchDirectory; if (!scratchDirectory) return; + const scratchHandle = this.scratchHandle; + if (!scratchHandle) { + throw new Error('Native sandbox scratch descriptor is unavailable'); + } try { await rm(scratchDirectory, { recursive: true, force: true }); } catch { - await this.restoreScratchTraversal(scratchDirectory); + await restoreScratchTraversal(scratchHandle); await rm(scratchDirectory, { recursive: true, force: true }); } + // Retain both the descriptor and path when cleanup fails so close() can + // retry without falling back to an attacker-replaceable ambient path. + await scratchHandle.close(); + this.scratchHandle = undefined; this.scratchDirectory = undefined; } - private async restoreScratchTraversal(root: string): Promise { - const pending = [root]; - for (let index = 0; index < pending.length; index += 1) { - const directory = pending[index]; - const metadata = await lstat(directory).catch( - (error: NodeJS.ErrnoException) => { - if (error.code === 'ENOENT') return undefined; - throw error; - }, - ); - if (!metadata?.isDirectory()) continue; - // Commands own their scratch contents and may remove all directory mode - // bits. Restore traversal before opening the directory with O_NOFOLLOW. - await chmod(directory, 0o700); - let handle; - try { - handle = await open( - directory, - fsConstants.O_RDONLY | fsConstants.O_NOFOLLOW, - ); - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (['ENOENT', 'ELOOP', 'ENOTDIR'].includes(code ?? '')) continue; - throw error; - } - try { - if (!(await handle.stat()).isDirectory()) continue; - } finally { - await handle.close(); - } - for (const entry of await readdir(directory, { withFileTypes: true })) { - if (entry.isDirectory()) pending.push(join(directory, entry.name)); - } - } - } - async close(): Promise { if (this.closing) return this.closing; const closing = this.closeExclusive(); diff --git a/packages/code/src/native-scratch.ts b/packages/code/src/native-scratch.ts new file mode 100644 index 0000000..0ecd330 --- /dev/null +++ b/packages/code/src/native-scratch.ts @@ -0,0 +1,172 @@ +import { constants as fsConstants } from 'node:fs'; +import { opendir } from 'node:fs/promises'; +import type { FileHandle } from 'node:fs/promises'; + +import koffi from 'koffi'; + +import { removePrivateStorageAcl } from './private-storage.js'; + +const POSIX_PLATFORMS = new Set(['darwin', 'linux']); +const lib = POSIX_PLATFORMS.has(process.platform) ? koffi.load(null) : undefined; +const openat = lib?.func('int openat(int dirfd, const char *path, int flags, uint32_t mode)'); +const closeFd = lib?.func('int close(int fd)'); +const fchmod = lib?.func('int fchmod(int fd, uint32_t mode)'); +const fchmodat = lib?.func( + 'int fchmodat(int dirfd, const char *path, uint32_t mode, int flags)', +); +const AT_SYMLINK_NOFOLLOW = process.platform === 'darwin' ? 0x0020 : 0x0100; +const O_EVTONLY = 0x8000; +// Recovery is a last-resort shutdown path over an attacker-controlled tree. +// Keep both its memory use and its descriptor-relative reopen work bounded. +const MAX_SCRATCH_DIRECTORIES = 10_000; +const MAX_SCRATCH_ENTRIES = 100_000; +const MAX_SCRATCH_DEPTH = 128; +const MAX_SCRATCH_COMPONENT_VISITS = 16_384; +const IGNORED_ENTRY_ERRNOS = new Set([ + koffi.os.errno.ENOENT, + koffi.os.errno.ELOOP, + koffi.os.errno.ENOTDIR, + koffi.os.errno.ENOTSUP, +]); + +export interface ScratchTraversalHooks { + /** Test seam for deterministic replacement-race coverage. */ + afterEntryInspected?(directoryFd: number, name: string): Promise; +} + +function descriptorPath(fd: number): string { + return process.platform === 'linux' ? `/proc/self/fd/${fd}` : `/dev/fd/${fd}`; +} + +function requirePosixBindings(): void { + if (!openat || !closeFd || !fchmod || !fchmodat) { + throw new Error('Descriptor-relative scratch cleanup is unavailable'); + } +} + +function ignoredEntryError(): boolean { + return IGNORED_ENTRY_ERRNOS.has(koffi.errno()); +} + +function restoreEntryMode(directoryFd: number, name: string): boolean { + requirePosixBindings(); + if (fchmodat!(directoryFd, name, 0o700, AT_SYMLINK_NOFOLLOW) === 0) return true; + if (ignoredEntryError()) return false; + throw new Error(`Descriptor-relative scratch chmod failed with errno ${koffi.errno()}`); +} + +function openDirectoryAt(directoryFd: number, name: string): number | undefined { + requirePosixBindings(); + const commonFlags = fsConstants.O_DIRECTORY | fsConstants.O_NOFOLLOW; + const repairFlags = process.platform === 'darwin' + ? commonFlags | O_EVTONLY + : commonFlags | fsConstants.O_RDONLY; + const fd = openat!(directoryFd, name, repairFlags, 0); + if (fd >= 0) return fd; + if (ignoredEntryError()) return undefined; + throw new Error(`Descriptor-relative scratch open failed with errno ${koffi.errno()}`); +} + +function closeDirectory(fd: number): void { + requirePosixBindings(); + if (closeFd!(fd) !== 0) { + throw new Error(`Descriptor-relative scratch close failed with errno ${koffi.errno()}`); + } +} + +async function repairDirectory(fd: number, label: string): Promise { + requirePosixBindings(); + if (fchmod!(fd, 0o700) !== 0) { + throw new Error(`Descriptor-relative scratch chmod failed with errno ${koffi.errno()}`); + } + await removePrivateStorageAcl({ fd }, label); +} + +async function openRelativeDirectory( + rootFd: number, + components: string[], + hooks: ScratchTraversalHooks, + consumeComponentVisit: () => void, +): Promise { + let currentFd = rootFd; + try { + for (const component of components) { + consumeComponentVisit(); + await hooks.afterEntryInspected?.(currentFd, component); + if (!restoreEntryMode(currentFd, component)) { + if (currentFd !== rootFd) { + const closingFd = currentFd; + currentFd = rootFd; + closeDirectory(closingFd); + } + return undefined; + } + const childFd = openDirectoryAt(currentFd, component); + if (currentFd !== rootFd) { + const closingFd = currentFd; + currentFd = rootFd; + closeDirectory(closingFd); + } + if (childFd === undefined) return undefined; + currentFd = childFd; + await repairDirectory(currentFd, `scratch directory ${components.join('/')}`); + } + return currentFd; + } catch (error) { + if (currentFd !== rootFd) closeDirectory(currentFd); + throw error; + } +} + +/** + * Restores traversal without resolving worker-controlled descendants through + * ambient paths. Relative component lists retain no descriptors; reopening a + * path holds at most two descriptors and refuses replacement symlinks. + */ +export async function restoreScratchTraversal( + root: FileHandle, + hooks: ScratchTraversalHooks = {}, +): Promise { + await root.chmod(0o700); + await removePrivateStorageAcl(root, 'native sandbox scratch root'); + const pending: string[][] = [[]]; + let componentVisits = 0; + const consumeComponentVisit = () => { + componentVisits += 1; + if (componentVisits > MAX_SCRATCH_COMPONENT_VISITS) { + throw new Error('Native sandbox scratch cleanup exceeded its work limit'); + } + }; + let entriesInspected = 0; + for (let index = 0; index < pending.length; index += 1) { + const components = pending[index]; + const directoryFd = components.length === 0 + ? root.fd + : await openRelativeDirectory( + root.fd, + components, + hooks, + consumeComponentVisit, + ); + if (directoryFd === undefined) continue; + try { + const directory = await opendir(descriptorPath(directoryFd)); + for await (const entry of directory) { + entriesInspected += 1; + if (entriesInspected > MAX_SCRATCH_ENTRIES) { + throw new Error('Native sandbox scratch cleanup exceeded its entry limit'); + } + if (!entry.isDirectory()) continue; + if (components.length >= MAX_SCRATCH_DEPTH) { + throw new Error('Native sandbox scratch cleanup exceeded its depth limit'); + } + if (pending.length >= MAX_SCRATCH_DIRECTORIES) { + throw new Error('Native sandbox scratch cleanup exceeded its directory limit'); + } + pending.push([...components, entry.name]); + } + } finally { + if (directoryFd !== root.fd) closeDirectory(directoryFd); + } + } +} diff --git a/packages/code/src/private-storage.ts b/packages/code/src/private-storage.ts index af4f185..25f23a2 100644 --- a/packages/code/src/private-storage.ts +++ b/packages/code/src/private-storage.ts @@ -25,7 +25,7 @@ async function macOsStorage() { } export async function assertPrivateStorageAcl( - handle: FileHandle, path: string, directory = false, + handle: Pick, path: string, directory = false, ): Promise { if (process.platform === 'darwin') { (await macOsStorage()).verifyMacOsAcl(handle.fd, path, directory); @@ -33,7 +33,10 @@ export async function assertPrivateStorageAcl( } /** Only application-owned files/directories may have their ACLs removed. */ -export async function removePrivateStorageAcl(handle: FileHandle, path: string): Promise { +export async function removePrivateStorageAcl( + handle: Pick, + path: string, +): Promise { if (process.platform === 'darwin') { (await macOsStorage()).removeMacOsAcl(handle.fd, path); }