From de7e7557dc1a3dbb2676901674cfbcff3c537fae Mon Sep 17 00:00:00 2001 From: tt-a1i <53142663+tt-a1i@users.noreply.github.com> Date: Fri, 19 Jun 2026 12:31:30 +0800 Subject: [PATCH] fix(cli): respect sandbox path boundaries (#5375) --- packages/cli/src/utils/sandbox-path.ts | 32 ++++++++++++++++++++++++ packages/cli/src/utils/sandbox.test.ts | 34 ++++++++++++++++++++++++++ packages/cli/src/utils/sandbox.ts | 26 +++++++++----------- 3 files changed, 77 insertions(+), 15 deletions(-) create mode 100644 packages/cli/src/utils/sandbox-path.ts diff --git a/packages/cli/src/utils/sandbox-path.ts b/packages/cli/src/utils/sandbox-path.ts new file mode 100644 index 0000000000..3195038565 --- /dev/null +++ b/packages/cli/src/utils/sandbox-path.ts @@ -0,0 +1,32 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import path from 'node:path'; + +export function isContainerPathWithinWorkdir( + containerWorkdir: string, + containerPath: string, +): boolean { + const normalize = (value: string) => { + const normalized = path.posix + .normalize(value.replace(/\\/g, '/')) + .replace(/\/+$/, '') + .toLowerCase(); + return normalized || '/'; + }; + + const normalizedWorkdir = normalize(containerWorkdir); + const normalizedPath = normalize(containerPath); + + if (normalizedWorkdir === '/') { + return normalizedPath.startsWith('/'); + } + + return ( + normalizedPath === normalizedWorkdir || + normalizedPath.startsWith(`${normalizedWorkdir}/`) + ); +} diff --git a/packages/cli/src/utils/sandbox.test.ts b/packages/cli/src/utils/sandbox.test.ts index 060e8267cb..2183405174 100644 --- a/packages/cli/src/utils/sandbox.test.ts +++ b/packages/cli/src/utils/sandbox.test.ts @@ -5,8 +5,42 @@ */ import { describe, expect, it } from 'vitest'; +import { isContainerPathWithinWorkdir } from './sandbox-path.js'; import { parseSandboxImageName } from './sandboxImageName.js'; +describe('isContainerPathWithinWorkdir', () => { + it('allows the workdir itself', () => { + expect(isContainerPathWithinWorkdir('/repo/app', '/repo/app')).toBe(true); + }); + + it('allows paths under the workdir', () => { + expect(isContainerPathWithinWorkdir('/repo/app', '/repo/app/bin')).toBe( + true, + ); + }); + + it('rejects sibling paths with the same prefix', () => { + expect( + isContainerPathWithinWorkdir('/repo/app', '/repo/app-tools/bin'), + ).toBe(false); + }); + + it('allows absolute paths under the filesystem root workdir', () => { + expect(isContainerPathWithinWorkdir('/', '/bin')).toBe(true); + }); + + it('normalizes trailing slashes and case for container paths', () => { + expect( + isContainerPathWithinWorkdir('/C/Repo/App/', '/c/repo/app/bin'), + ).toBe(true); + }); + + it('handles converted Windows drive roots without matching sibling drives', () => { + expect(isContainerPathWithinWorkdir('/c', '/c/tools')).toBe(true); + expect(isContainerPathWithinWorkdir('/c', '/c2/tools')).toBe(false); + }); +}); + describe('parseSandboxImageName', () => { it('uses the image basename and tag for container names', () => { expect(parseSandboxImageName('ghcr.io/qwenlm/qwen-code:0.18.3')).toBe( diff --git a/packages/cli/src/utils/sandbox.ts b/packages/cli/src/utils/sandbox.ts index cb8df524e2..7c0ab552ee 100644 --- a/packages/cli/src/utils/sandbox.ts +++ b/packages/cli/src/utils/sandbox.ts @@ -24,6 +24,7 @@ import { import { randomBytes } from 'node:crypto'; import { writeStderrLine } from './stdioHelpers.js'; import { parseSandboxImageName } from './sandboxImageName.js'; +import { isContainerPathWithinWorkdir } from './sandbox-path.js'; const execAsync = promisify(exec); @@ -121,9 +122,7 @@ function entrypoint(workdir: string, cliArgs: string[]): string[] { const paths = process.env['PATH'].split(pathSeparator); for (const p of paths) { const containerPath = getContainerPath(p); - if ( - containerPath.toLowerCase().startsWith(containerWorkdir.toLowerCase()) - ) { + if (isContainerPathWithinWorkdir(containerWorkdir, containerPath)) { pathSuffix += `:${containerPath}`; } } @@ -137,9 +136,7 @@ function entrypoint(workdir: string, cliArgs: string[]): string[] { const paths = process.env['PYTHONPATH'].split(pathSeparator); for (const p of paths) { const containerPath = getContainerPath(p); - if ( - containerPath.toLowerCase().startsWith(containerWorkdir.toLowerCase()) - ) { + if (isContainerPathWithinWorkdir(containerWorkdir, containerPath)) { pythonPathSuffix += `:${containerPath}`; } } @@ -723,8 +720,13 @@ export async function start_sandbox( // also mount-replace VIRTUAL_ENV directory with /sandbox.venv // sandbox can then set up this new VIRTUAL_ENV directory using sandbox.bashrc (see below) // directory will be empty if not set up, which is still preferable to having host binaries + const virtualEnv = process.env['VIRTUAL_ENV']; if ( - process.env['VIRTUAL_ENV']?.toLowerCase().startsWith(workdir.toLowerCase()) + virtualEnv && + isContainerPathWithinWorkdir( + getContainerPath(workdir), + getContainerPath(virtualEnv), + ) ) { const sandboxVenvPath = path.resolve( SETTINGS_DIRECTORY_NAME, @@ -733,14 +735,8 @@ export async function start_sandbox( if (!fs.existsSync(sandboxVenvPath)) { fs.mkdirSync(sandboxVenvPath, { recursive: true }); } - args.push( - '--volume', - `${sandboxVenvPath}:${getContainerPath(process.env['VIRTUAL_ENV'])}`, - ); - args.push( - '--env', - `VIRTUAL_ENV=${getContainerPath(process.env['VIRTUAL_ENV'])}`, - ); + args.push('--volume', `${sandboxVenvPath}:${getContainerPath(virtualEnv)}`); + args.push('--env', `VIRTUAL_ENV=${getContainerPath(virtualEnv)}`); } // copy additional environment variables from SANDBOX_ENV