fix: Harden Native Scratch Cleanup

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,14 +444,130 @@ 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);
   await sandbox.close();
   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<string | undefined>;
+  };
+  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',

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<void>;
   private canonicalRoot?: string;
   private scratchDirectory?: string;
+  private scratchHandle?: FileHandle;
   private execution?: Promise<WorkspaceExecuteCommandResult>;
   private closing?: Promise<void>;
   private resetFailed = false;
@@ -748,6 +748,11 @@ export class NativeSrtWorkspaceCommandSandbox implements WorkspaceCommandSandbox
   ): Promise<string | undefined> {
     // 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<void> {
     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<void> {
-    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<void> {
     if (this.closing) return this.closing;
     const closing = this.closeExclusive();