fix(cli): isolate temporary directory for macOS Seatbelt sandbox (#29171)

Co-authored-by: David Pierce <davidapierce@google.com>

packages/cli/src/utils/sandbox.test.ts

@@ -137,6 +137,10 @@ describe('sandbox', () => {
     vi.mocked(os.tmpdir).mockReturnValue('/tmp');
     vi.mocked(fs.existsSync).mockReturnValue(true);
     vi.mocked(fs.realpathSync).mockImplementation((p) => p as string);
+    vi.mocked(fs.mkdtempSync).mockImplementation(
+      (prefix) => `${prefix}test-tmp`,
+    );
+    vi.mocked(fs.rmSync).mockImplementation(() => {});
     vi.mocked(execSync).mockReturnValue(Buffer.from(''));
   });
 
@@ -182,6 +186,121 @@ describe('sandbox', () => {
       );
     });
 
+    it('should isolate temporary directory for macOS seatbelt (sandbox-exec)', async () => {
+      vi.mocked(os.platform).mockReturnValue('darwin');
+      vi.mocked(os.tmpdir).mockReturnValue('/var/folders/test/T');
+      vi.mocked(fs.realpathSync).mockImplementation((p) => p as string);
+      vi.mocked(fs.mkdtempSync).mockReturnValue(
+        '/var/folders/test/T/gemini-sandbox-tmp-123456',
+      );
+
+      const config: SandboxConfig = createMockSandboxConfig({
+        command: 'sandbox-exec',
+        image: 'some-image',
+      });
+
+      interface MockProcess extends EventEmitter {
+        stdout: EventEmitter;
+        stderr: EventEmitter;
+      }
+      const mockSpawnProcess = new EventEmitter() as MockProcess;
+      mockSpawnProcess.stdout = new EventEmitter();
+      mockSpawnProcess.stderr = new EventEmitter();
+      vi.mocked(spawn).mockReturnValue(
+        mockSpawnProcess as unknown as ReturnType<typeof spawn>,
+      );
+
+      const onSpy = vi.spyOn(process, 'on');
+      const offSpy = vi.spyOn(process, 'off');
+
+      const promise = start_sandbox(config, [], undefined, ['arg1']);
+
+      setTimeout(() => {
+        mockSpawnProcess.emit('close', 0);
+      }, 10);
+
+      await expect(promise).resolves.toBe(0);
+
+      // Verify that an isolated temporary directory was created
+      expect(fs.mkdtempSync).toHaveBeenCalledWith(
+        expect.stringContaining(
+          path.join('/var/folders/test/T', 'gemini-sandbox-'),
+        ),
+      );
+
+      const spawnCalls = vi.mocked(spawn).mock.calls;
+      const spawnArgs = spawnCalls[0]?.[1] as string[];
+
+      // Verify that TMP_DIR argument passed to seatbelt is the isolated temp directory, NOT host tmpdir
+      expect(spawnArgs).toContain(
+        'TMP_DIR=/var/folders/test/T/gemini-sandbox-tmp-123456',
+      );
+      expect(spawnArgs).not.toContain('TMP_DIR=/var/folders/test/T');
+
+      // Verify that TMPDIR environment variable is set in the sandboxed shell execution
+      const shCommand = spawnArgs[spawnArgs.indexOf('sh') + 2];
+      expect(shCommand).toContain('TMPDIR=');
+      expect(shCommand).toContain(
+        '/var/folders/test/T/gemini-sandbox-tmp-123456',
+      );
+
+      // Verify cleanup of the isolated temporary directory
+      expect(fs.rmSync).toHaveBeenCalledWith(
+        '/var/folders/test/T/gemini-sandbox-tmp-123456',
+        expect.objectContaining({ recursive: true, force: true }),
+      );
+
+      // Verify that exit and signal cleanup hooks are registered and unregistered
+      expect(onSpy).toHaveBeenCalledWith('exit', expect.any(Function));
+      expect(onSpy).toHaveBeenCalledWith('SIGINT', expect.any(Function));
+      expect(onSpy).toHaveBeenCalledWith('SIGTERM', expect.any(Function));
+
+      expect(offSpy).toHaveBeenCalledWith('exit', expect.any(Function));
+      expect(offSpy).toHaveBeenCalledWith('SIGINT', expect.any(Function));
+      expect(offSpy).toHaveBeenCalledWith('SIGTERM', expect.any(Function));
+
+      onSpy.mockRestore();
+      offSpy.mockRestore();
+    });
+
+    it('should safely swallow errors if fs.rmSync fails during cleanup', async () => {
+      vi.mocked(os.platform).mockReturnValue('darwin');
+      vi.mocked(os.tmpdir).mockReturnValue('/var/folders/test/T');
+      vi.mocked(fs.realpathSync).mockImplementation((p) => p as string);
+      vi.mocked(fs.mkdtempSync).mockReturnValue(
+        '/var/folders/test/T/gemini-sandbox-tmp-error',
+      );
+      vi.mocked(fs.rmSync).mockImplementation(() => {
+        throw new Error('EPERM: operation not permitted');
+      });
+
+      const config: SandboxConfig = createMockSandboxConfig({
+        command: 'sandbox-exec',
+        image: 'some-image',
+      });
+
+      interface MockProcess extends EventEmitter {
+        stdout: EventEmitter;
+        stderr: EventEmitter;
+      }
+      const mockSpawnProcess = new EventEmitter() as MockProcess;
+      mockSpawnProcess.stdout = new EventEmitter();
+      mockSpawnProcess.stderr = new EventEmitter();
+      vi.mocked(spawn).mockReturnValue(
+        mockSpawnProcess as unknown as ReturnType<typeof spawn>,
+      );
+
+      const promise = start_sandbox(config, [], undefined, ['arg1']);
+
+      setTimeout(() => {
+        mockSpawnProcess.emit('close', 0);
+      }, 10);
+
+      // Even if fs.rmSync throws, start_sandbox should resolve successfully and not crash
+      await expect(promise).resolves.toBe(0);
+      expect(fs.rmSync).toHaveBeenCalled();
+    });
+
     it('should resolve custom seatbelt profile from user home directory', async () => {
       vi.mocked(os.platform).mockReturnValue('darwin');
       vi.stubEnv('SEATBELT_PROFILE', 'custom-test');

packages/cli/src/utils/sandbox.ts

@@ -58,15 +58,30 @@ export async function start_sandbox(
 
   let stopProxy: (() => void) | undefined = undefined;
   let tempProfileFile: string | null = null;
+  let sandboxTmpDir: string | null = null;
 
   const cleanup = () => {
-    if (tempProfileFile && fs.existsSync(tempProfileFile)) {
+    if (sandboxTmpDir) {
+      const dirToDelete = sandboxTmpDir;
+      sandboxTmpDir = null;
       try {
-        fs.unlinkSync(tempProfileFile);
+        if (fs.existsSync(dirToDelete)) {
+          fs.rmSync(dirToDelete, { recursive: true, force: true });
+        }
       } catch {
         // ignore
       }
+    }
+    if (tempProfileFile) {
+      const fileToDelete = tempProfileFile;
       tempProfileFile = null;
+      try {
+        if (fs.existsSync(fileToDelete)) {
+          fs.unlinkSync(fileToDelete);
+        }
+      } catch {
+        // ignore
+      }
     }
     if (stopProxy) {
       try {
@@ -157,11 +172,17 @@ export async function start_sandbox(
           ...nodeArgs,
         ].join(' ');
 
+        const hostTmpDir = fs.realpathSync(os.tmpdir());
+        const resolvedTmpDir = fs.mkdtempSync(
+          path.join(hostTmpDir, 'gemini-sandbox-'),
+        );
+        sandboxTmpDir = resolvedTmpDir;
+
         const args = [
           '-D',
           `TARGET_DIR=${fs.realpathSync(process.cwd())}`,
           '-D',
-          `TMP_DIR=${fs.realpathSync(os.tmpdir())}`,
+          `TMP_DIR=${resolvedTmpDir}`,
           '-D',
           `HOME_DIR=${fs.realpathSync(homedir())}`,
           '-D',
@@ -222,6 +243,9 @@ export async function start_sandbox(
           '-c',
           [
             `SANDBOX=sandbox-exec`,
+            'TMPDIR=' + quote([resolvedTmpDir]),
+            'TMP=' + quote([resolvedTmpDir]),
+            'TEMP=' + quote([resolvedTmpDir]),
             'NODE_OPTIONS=' + quote([nodeOptions]),
             ...finalArgv.map((arg) => quote([arg])),
           ].join(' '),
@@ -231,6 +255,9 @@ export async function start_sandbox(
         let proxyProcess: ChildProcess | undefined = undefined;
         let sandboxProcess: ChildProcess | undefined = undefined;
         const sandboxEnv = { ...process.env };
+        sandboxEnv['TMPDIR'] = resolvedTmpDir;
+        sandboxEnv['TMP'] = resolvedTmpDir;
+        sandboxEnv['TEMP'] = resolvedTmpDir;
         if (proxyCommand) {
           const proxy =
             process.env['HTTPS_PROXY'] ||
@@ -288,6 +315,7 @@ export async function start_sandbox(
         process.stdin.pause();
         sandboxProcess = spawn(config.command, args, {
           stdio: 'inherit',
+          env: sandboxEnv,
         });
         return await new Promise((resolve, reject) => {
           sandboxProcess?.on('error', (err) => {