diff --git a/packages/acp-bridge/src/bridge.sandbox.test.ts b/packages/acp-bridge/src/bridge.sandbox.test.ts new file mode 100644 index 00000000000..84bd611f1b3 --- /dev/null +++ b/packages/acp-bridge/src/bridge.sandbox.test.ts @@ -0,0 +1,61 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { makeBridge, makeChannel } from './internal/testUtils.js'; +import { _setSandboxMountExistsForTest } from './workspacePaths.js'; + +// #7139 wiring for the bridge ingestion site: `resolveWorkspaceKey` guards +// with `path.isAbsolute` before canonicalizing, so a Windows-shaped +// `workspaceCwd` arriving via spawnOrAttach (clients, persisted +// registrations) must be translated to its bind mount first — the sibling +// of the dispatch/request-helpers/route/boot-validator wiring tests. +describe('bridge spawnOrAttach inside a POSIX container sandbox (#7139)', () => { + afterEach(() => { + vi.unstubAllEnvs(); + _setSandboxMountExistsForTest(undefined); + }); + + it.skipIf(process.platform === 'win32')( + 'accepts a Windows-shaped workspaceCwd bound to its mount location', + async () => { + vi.stubEnv('SANDBOX', 'qwen-code-sandbox-0'); + _setSandboxMountExistsForTest((p) => p === '/c/qwen-repro'); + const handle = makeChannel(); + const bridge = makeBridge({ + boundWorkspace: '/c/qwen-repro', + channelFactory: vi.fn().mockResolvedValue(handle.channel), + }); + try { + const session = await bridge.spawnOrAttach({ + workspaceCwd: 'C:\\qwen-repro', + }); + expect(session.sessionId).toBeTruthy(); + } finally { + await bridge.shutdown(); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'still rejects a Windows-shaped workspaceCwd outside a sandbox', + async () => { + vi.stubEnv('SANDBOX', ''); + const handle = makeChannel(); + const bridge = makeBridge({ + boundWorkspace: '/c/qwen-repro', + channelFactory: vi.fn().mockResolvedValue(handle.channel), + }); + try { + await expect( + bridge.spawnOrAttach({ workspaceCwd: 'C:\\qwen-repro' }), + ).rejects.toThrow('workspaceCwd must be an absolute path'); + } finally { + await bridge.shutdown(); + } + }, + ); +}); diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index a0d0133e786..4ea9ba329a3 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -82,7 +82,10 @@ import { SessionBusyError, InvalidRewindTargetError, } from './bridgeErrors.js'; -import { canonicalizeWorkspace } from './workspacePaths.js'; +import { + canonicalizeWorkspace, + translateAndCheckAbsoluteWorkspacePath, +} from './workspacePaths.js'; import { parseSessionSource } from './session-source.js'; import { CHANNEL_STARTUP_PROFILE_META_KEY, @@ -2845,10 +2848,15 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge { return entry.transportClosedReject; }; - const resolveWorkspaceKey = (workspaceCwd: string): string => { - if (!path.isAbsolute(workspaceCwd)) { + const resolveWorkspaceKey = (rawWorkspaceCwd: string): string => { + // #7139: host-shaped Windows paths reach the in-container bridge via + // clients and persisted registrations; the shared helper maps them to + // the bind mount before the absolute-path check. + const workspaceCwd = + translateAndCheckAbsoluteWorkspacePath(rawWorkspaceCwd); + if (workspaceCwd === null) { throw new Error( - `workspaceCwd must be an absolute path; got "${workspaceCwd}"`, + `workspaceCwd must be an absolute path; got "${rawWorkspaceCwd}"`, ); } const workspaceKey = diff --git a/packages/acp-bridge/src/workspacePaths.sandbox.test.ts b/packages/acp-bridge/src/workspacePaths.sandbox.test.ts new file mode 100644 index 00000000000..6cddecd0c6a --- /dev/null +++ b/packages/acp-bridge/src/workspacePaths.sandbox.test.ts @@ -0,0 +1,52 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { canonicalizeWorkspace } from './workspacePaths.js'; + +// Isolated from workspacePaths.test.ts because it mocks node:fs — the other +// file drives real filesystem fixtures. +vi.mock('node:fs', async (importOriginal) => { + const actual = (await importOriginal()) as typeof import('node:fs'); + return { + ...actual, + existsSync: (p: unknown) => + p === '/c/qwen-repro' ? true : actual.existsSync(p as never), + }; +}); + +describe('canonicalizeWorkspace inside a POSIX container sandbox (#7139)', () => { + afterEach(() => { + vi.unstubAllEnvs(); + }); + + it.skipIf(process.platform === 'win32')( + 'resolves a Windows-shaped workspace to its bind-mount location', + () => { + vi.stubEnv('SANDBOX', 'qwen-code-sandbox-0'); + // The mount exists (mocked), realpath on it ENOENTs on this host, so + // the fallback returns the translated absolute path — NOT the cwd + // concatenation `path.resolve` alone would produce. + expect(canonicalizeWorkspace('C:\\qwen-repro')).toBe('/c/qwen-repro'); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'keeps Windows-shaped input untouched outside a sandbox', + () => { + // Explicitly clear SANDBOX: when this suite itself runs inside the + // project's Docker sandbox the launcher sets it, and unstubAllEnvs + // would not remove a genuinely inherited value. + vi.stubEnv('SANDBOX', ''); + const result = canonicalizeWorkspace('C:\\qwen-repro'); + // Unsandboxed POSIX behavior is unchanged: the string resolves + // relative to the cwd (and stays broken — which is what the + // pre-#7139 sandbox path produced too). + expect(result.endsWith('C:\\qwen-repro')).toBe(true); + expect(result.startsWith('/')).toBe(true); + }, + ); +}); diff --git a/packages/acp-bridge/src/workspacePaths.test.ts b/packages/acp-bridge/src/workspacePaths.test.ts index 2c9caee7c42..f483a154e2c 100644 --- a/packages/acp-bridge/src/workspacePaths.test.ts +++ b/packages/acp-bridge/src/workspacePaths.test.ts @@ -7,8 +7,13 @@ import { promises as fsp, realpathSync } from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; -import { afterEach, describe, expect, it } from 'vitest'; -import { canonicalizeWorkspaces } from './workspacePaths.js'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + _setSandboxMountExistsForTest, + canonicalizeWorkspaces, + translateAndCheckAbsoluteWorkspacePath, + translateWindowsWorkspaceForPosixSandbox, +} from './workspacePaths.js'; const scratches: string[] = []; @@ -46,3 +51,128 @@ describe('canonicalizeWorkspaces', () => { expect(canonicalizeWorkspaces([])).toEqual([]); }); }); + +// The single ingestion-ordering enforcement point: translation before the +// absolute-path check, shared by all five workspace-ingestion sites. +describe('translateAndCheckAbsoluteWorkspacePath', () => { + it('returns the translated mount for Windows-shaped input in a sandbox', () => { + vi.stubEnv('SANDBOX', 'qwen-code-sandbox-0'); + _setSandboxMountExistsForTest((p) => p === '/c/qwen-repro'); + try { + expect(translateAndCheckAbsoluteWorkspacePath('C:\\qwen-repro')).toBe( + '/c/qwen-repro', + ); + } finally { + vi.unstubAllEnvs(); + _setSandboxMountExistsForTest(undefined); + } + }); + + it('returns null for non-absolute input (untranslated Windows shape included)', () => { + vi.stubEnv('SANDBOX', ''); + try { + expect(translateAndCheckAbsoluteWorkspacePath('C:\\qwen-repro')).toBe( + null, + ); + expect(translateAndCheckAbsoluteWorkspacePath('relative/dir')).toBe(null); + } finally { + vi.unstubAllEnvs(); + } + }); + + it('passes POSIX absolute paths through unchanged', () => { + expect(translateAndCheckAbsoluteWorkspacePath('/work/a')).toBe('/work/a'); + }); +}); + +// Regression for #7139: a Windows host relaunching `qwen serve` into a Linux +// Docker sandbox forwards `--workspace C:\…` (and client/persisted workspace +// registrations) in host shape; every ACP child then failed with +// `chdir(2) ENOENT`. These tests pin the container-side translation. +describe('translateWindowsWorkspaceForPosixSandbox', () => { + const sandboxOpts = (exists: boolean) => ({ + platform: 'linux' as NodeJS.Platform, + sandboxEnv: 'qwen-code-sandbox-0', + exists: () => exists, + }); + + it('maps a Windows-absolute path to its bind-mount location', () => { + expect( + translateWindowsWorkspaceForPosixSandbox( + 'C:\\qwen-repro', + sandboxOpts(true), + ), + ).toBe('/c/qwen-repro'); + expect( + translateWindowsWorkspaceForPosixSandbox( + 'D:/Work/proj sub', + sandboxOpts(true), + ), + ).toBe('/d/Work/proj sub'); + expect( + translateWindowsWorkspaceForPosixSandbox( + 'C:\\nested\\dir\\leaf', + sandboxOpts(true), + ), + ).toBe('/c/nested/dir/leaf'); + }); + + it('leaves the path alone when the translated mount does not exist', () => { + expect( + translateWindowsWorkspaceForPosixSandbox( + 'C:\\qwen-repro', + sandboxOpts(false), + ), + ).toBe('C:\\qwen-repro'); + }); + + it('refuses ..-laden input that escapes the drive mount', () => { + // existsSync would resolve /c/../../etc to /etc and return true — the + // guard must refuse before the probe can bless an out-of-mount path. + expect( + translateWindowsWorkspaceForPosixSandbox( + 'C:\\..\\..\\etc', + sandboxOpts(true), + ), + ).toBe('C:\\..\\..\\etc'); + // In-mount .. that stays under the drive prefix is still fine. + expect( + translateWindowsWorkspaceForPosixSandbox( + 'C:\\work\\..\\proj', + sandboxOpts(true), + ), + ).toBe('/c/work/../proj'); + }); + + it('leaves non-Windows-shaped paths alone', () => { + for (const p of ['/c/qwen-repro', 'relative/dir', 'C:', 'CC:\\x', '']) { + expect( + translateWindowsWorkspaceForPosixSandbox(p, sandboxOpts(true)), + ).toBe(p); + } + }); + + it('is inert on Windows hosts, outside sandboxes, and under seatbelt', () => { + expect( + translateWindowsWorkspaceForPosixSandbox('C:\\qwen-repro', { + platform: 'win32', + sandboxEnv: 'qwen-code-sandbox-0', + exists: () => true, + }), + ).toBe('C:\\qwen-repro'); + expect( + translateWindowsWorkspaceForPosixSandbox('C:\\qwen-repro', { + platform: 'linux', + sandboxEnv: undefined, + exists: () => true, + }), + ).toBe('C:\\qwen-repro'); + expect( + translateWindowsWorkspaceForPosixSandbox('C:\\qwen-repro', { + platform: 'darwin', + sandboxEnv: 'sandbox-exec', + exists: () => true, + }), + ).toBe('C:\\qwen-repro'); + }); +}); diff --git a/packages/acp-bridge/src/workspacePaths.ts b/packages/acp-bridge/src/workspacePaths.ts index 8f232ef8bfa..4c123883f81 100644 --- a/packages/acp-bridge/src/workspacePaths.ts +++ b/packages/acp-bridge/src/workspacePaths.ts @@ -4,9 +4,78 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { realpathSync } from 'node:fs'; +import { existsSync, realpathSync } from 'node:fs'; import * as path from 'node:path'; +const WINDOWS_ABSOLUTE_PATH_RE = /^([A-Za-z]):[\\/](.*)$/; + +/** + * Maps a Windows-shaped absolute path to the container mount produced by the + * host-side sandbox launcher (`C:\work\proj` → `/c/work/proj`, mirroring + * `getContainerPath` in `cli/src/utils/sandbox.ts`). + * + * A Windows host relaunching `qwen serve` into a Linux Docker/Podman sandbox + * translates the bind mount and `--workdir`, but path-valued CLI arguments + * (`--workspace C:\…`), client-registered workspaces, and persisted + * registrations reach the in-container daemon in host shape. Left alone, + * `path.resolve` on Linux mangles them further (prepends the cwd) and every + * ACP child spawn fails with `chdir(2) ENOENT` before running anything + * (#7139). + * + * Deliberately conservative — the input is returned unchanged unless ALL of: + * - the daemon is running on POSIX inside a container sandbox (`SANDBOX` env + * set by the launcher; macOS `sandbox-exec` does not remap paths and is + * excluded), + * - the path is Windows-absolute (`:\…` or `:/…`), + * - the translated candidate actually exists (i.e. the drive really is + * mounted the way the launcher mounts workspaces). + * + * The `opts` seams exist for tests; production callers use the defaults. + */ +// Test-only override for the mount-existence probe: the translated target +// (`/c/…`) sits at the filesystem root, which tests cannot create, and +// cross-package node:fs mocks don't reach this module's binding. Follows +// the `_reset*ForTest` convention. +let sandboxMountExistsOverrideForTest: ((p: string) => boolean) | undefined; +export function _setSandboxMountExistsForTest( + fn?: (p: string) => boolean, +): void { + sandboxMountExistsOverrideForTest = fn; +} + +export function translateWindowsWorkspaceForPosixSandbox( + p: string, + opts: { + platform?: NodeJS.Platform; + sandboxEnv?: string | undefined; + exists?: (candidate: string) => boolean; + } = {}, +): string { + const platform = opts.platform ?? process.platform; + const sandboxEnv = + 'sandboxEnv' in opts ? opts.sandboxEnv : process.env['SANDBOX']; + const exists = opts.exists ?? sandboxMountExistsOverrideForTest ?? existsSync; + if (platform === 'win32' || !sandboxEnv || sandboxEnv === 'sandbox-exec') { + return p; + } + const match = WINDOWS_ABSOLUTE_PATH_RE.exec(p); + if (!match) return p; + const translated = `/${match[1]!.toLowerCase()}/${match[2]!.replace(/\\/g, '/')}`; + // `..` segments would let the existence probe resolve outside the drive + // mount (`C:\..\..\etc` -> existsSync('/c/../../etc') === true via /etc), + // making the "translated candidate actually exists" contract a lie. Not a + // privilege escalation (downstream workspace binding still rejects it), + // but refuse to translate anything that escapes the drive prefix. + const resolvedTranslated = path.resolve(translated); + if ( + resolvedTranslated !== `/${match[1]!.toLowerCase()}` && + !resolvedTranslated.startsWith(`/${match[1]!.toLowerCase()}/`) + ) { + return p; + } + return exists(resolvedTranslated) ? translated : p; +} + /** * Canonicalize a workspace path so the boot-time bound path and every * request's `workspaceCwd` collapse to the same key. `path.resolve` @@ -39,8 +108,31 @@ import * as path from 'node:path'; * `cli/src/serve/fs/paths.ts` re-exports for callers still pointing * at the original location. */ +/** + * Single enforcement point for the workspace-ingestion ordering (#7139): + * the sandbox translation MUST run before the absolute-path check, because + * `path.isAbsolute('C:\\…')` is false on POSIX and would reject the + * host-shaped input before `canonicalizeWorkspace`'s own translation could + * ever see it. Every ingestion site calls this instead of hand-rolling the + * translate-then-isAbsolute pair, so a future sixth endpoint cannot forget + * the ordering. + * + * Returns the translated path, or null when it is not absolute — callers + * keep their own error surface (HTTP 400, AcpParamError, boot Error). + */ +export function translateAndCheckAbsoluteWorkspacePath( + raw: string, +): string | null { + const translated = translateWindowsWorkspaceForPosixSandbox(raw); + return path.isAbsolute(translated) ? translated : null; +} + export function canonicalizeWorkspace(p: string): string { - const resolved = path.resolve(p); + // #7139: inside a Linux container sandbox, host-shaped Windows workspace + // paths must be mapped to their bind-mount location BEFORE resolution — + // `path.resolve('C:\\x')` on POSIX treats the whole string as relative + // and prepends the cwd. + const resolved = path.resolve(translateWindowsWorkspaceForPosixSandbox(p)); try { // FIXME(stage-2): switch to `fs.promises.realpath` once the // bridge call sites become async-friendly. This sync syscall diff --git a/packages/cli/src/serve/acp-http/dispatch.sandbox.test.ts b/packages/cli/src/serve/acp-http/dispatch.sandbox.test.ts new file mode 100644 index 00000000000..53c820888d8 --- /dev/null +++ b/packages/cli/src/serve/acp-http/dispatch.sandbox.test.ts @@ -0,0 +1,40 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { _setSandboxMountExistsForTest } from '@qwen-code/acp-bridge/workspacePaths'; +import { parseOptionalWorkspaceCwd } from './dispatch.js'; + +// #7139 wiring: the ACP JSON-RPC `cwd` entry point must translate a +// Windows-shaped path to its bind mount BEFORE its absolute-path guard — +// this is the dispatch-side sibling of request-helpers.sandbox.test.ts. +describe('ACP dispatch parseOptionalWorkspaceCwd inside a POSIX container sandbox (#7139)', () => { + afterEach(() => { + vi.unstubAllEnvs(); + _setSandboxMountExistsForTest(undefined); + }); + + it.skipIf(process.platform === 'win32')( + 'accepts a Windows-shaped cwd and returns its bind-mount location', + () => { + vi.stubEnv('SANDBOX', 'qwen-code-sandbox-0'); + _setSandboxMountExistsForTest((p) => p === '/c/qwen-repro'); + expect( + parseOptionalWorkspaceCwd({ cwd: 'C:\\qwen-repro' }, '/c/qwen-repro'), + ).toBe('/c/qwen-repro'); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'still rejects a Windows-shaped cwd outside a sandbox', + () => { + vi.stubEnv('SANDBOX', ''); + expect(() => + parseOptionalWorkspaceCwd({ cwd: 'C:\\qwen-repro' }, '/tmp'), + ).toThrow('`cwd` must be an absolute path when provided'); + }, + ); +}); diff --git a/packages/cli/src/serve/acp-http/dispatch.ts b/packages/cli/src/serve/acp-http/dispatch.ts index 43600a10121..a3c7b97758b 100644 --- a/packages/cli/src/serve/acp-http/dispatch.ts +++ b/packages/cli/src/serve/acp-http/dispatch.ts @@ -4,7 +4,6 @@ * SPDX-License-Identifier: Apache-2.0 */ -import path from 'node:path'; import { APPROVAL_MODES, type ApprovalMode, @@ -42,6 +41,10 @@ import { } from '../auth/device-flow.js'; import type { HttpAcpBridge } from '@qwen-code/acp-bridge/bridgeTypes'; import { parseSessionSource } from '@qwen-code/acp-bridge'; +import { + translateAndCheckAbsoluteWorkspacePath, + canonicalizeWorkspace, +} from '@qwen-code/acp-bridge/workspacePaths'; import type { BridgeEvent } from '@qwen-code/acp-bridge/eventBus'; import { SessionShellClientRequiredError, @@ -52,7 +55,6 @@ import { SessionArtifactAuthorizationError, SessionArtifactValidationError, } from '@qwen-code/acp-bridge/sessionArtifacts'; -import { canonicalizeWorkspace } from '@qwen-code/acp-bridge/workspacePaths'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import { MAX_WORKSPACE_PATH_LENGTH } from '../fs/paths.js'; import { @@ -305,7 +307,9 @@ function parseOptionalSafeIntegerInRange( * Closes the body-amplification DoS the REST code documents. Returns the * bound workspace when omitted. */ -function parseOptionalWorkspaceCwd( +// Exported for the sandbox-translation wiring test — this is the entry +// point for every ACP JSON-RPC `cwd` (#7139). +export function parseOptionalWorkspaceCwd( params: Record, boundWorkspace: string, ): string { @@ -321,13 +325,14 @@ function parseOptionalWorkspaceCwd( `\`cwd\` exceeds the ${MAX_WORKSPACE_PATH_LENGTH}-character limit`, ); } - // `path.isAbsolute` (platform-aware) — same as the REST route. A bare - // `startsWith('/')` would reject valid Windows `C:\…`/UNC paths a client - // gets back from `/capabilities.workspaceCwd`. - if (!path.isAbsolute(cwd)) { + // #7139: the shared helper maps a Windows-shaped cwd to its container + // bind mount before the (platform-aware) absolute-path check — same as + // the REST route. + const sandboxCwd = translateAndCheckAbsoluteWorkspacePath(cwd); + if (sandboxCwd === null) { throw new AcpParamError('`cwd` must be an absolute path when provided'); } - return cwd; + return sandboxCwd; } /** Validate a `session/prompt` body before it reaches the bridge/agent. */ diff --git a/packages/cli/src/serve/fast-path.test.ts b/packages/cli/src/serve/fast-path.test.ts index e6260604a25..7e2c6769f67 100644 --- a/packages/cli/src/serve/fast-path.test.ts +++ b/packages/cli/src/serve/fast-path.test.ts @@ -452,9 +452,13 @@ describe('CLI entry import boundary', () => { expect(requestHelpersSource).toContain( "import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes';", ); - expect(requestHelpersSource).toContain( - "import { MAX_WORKSPACE_PATH_LENGTH } from '@qwen-code/acp-bridge/workspacePaths';", + // MAX_WORKSPACE_PATH_LENGTH (and, since #7139, the sandbox path + // translation) must come from the workspacePaths subpath — never the + // acp-bridge barrel or the compatibility shim. + expect(requestHelpersSource).toMatch( + /import \{[^}]*\bMAX_WORKSPACE_PATH_LENGTH\b[^}]*\} from '@qwen-code\/acp-bridge\/workspacePaths';/, ); + expect(requestHelpersSource).not.toMatch(/from '@qwen-code\/acp-bridge';/); }); it('keeps the runQwenServe static source graph free of ACP runtime modules', () => { diff --git a/packages/cli/src/serve/process-env-guard.test.ts b/packages/cli/src/serve/process-env-guard.test.ts index dda23fde2b2..0739499aaaf 100644 --- a/packages/cli/src/serve/process-env-guard.test.ts +++ b/packages/cli/src/serve/process-env-guard.test.ts @@ -52,6 +52,17 @@ const allowedProcessEnvAccesses = normalizeAllowances([ accesses: { whole: 1 }, }, ], + [ + 'packages/acp-bridge/src/workspacePaths.ts', + { + reason: + 'Whether the daemon runs inside a container sandbox is process-scoped: ' + + 'the sandbox launcher marks the whole process via the SANDBOX env, and ' + + 'workspace canonicalization uses it to map Windows-shaped host paths ' + + 'to their bind-mount location (#7139).', + accesses: { 'key:SANDBOX': 1 }, + }, + ], [ 'packages/cli/src/serve/acp-http-enabled.ts', { diff --git a/packages/cli/src/serve/routes/workspace-management.test.ts b/packages/cli/src/serve/routes/workspace-management.test.ts index ac994922eb0..44d3855aa79 100644 --- a/packages/cli/src/serve/routes/workspace-management.test.ts +++ b/packages/cli/src/serve/routes/workspace-management.test.ts @@ -5,6 +5,7 @@ */ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { _setSandboxMountExistsForTest } from '@qwen-code/acp-bridge/workspacePaths'; import express, { type Request, type Response } from 'express'; import request from 'supertest'; import { @@ -190,6 +191,48 @@ describe('POST /workspaces', () => { expect(res.body.code).toBe('invalid_path'); }); + // #7139 wiring: with a container sandbox active, a Windows-shaped cwd is + // translated to its bind mount BEFORE the absolute-path guard — the 400 + // moves from the isAbsolute rejection to the (deeper) existence check, + // because the root-level translated mount cannot exist in a test. + it.skipIf(process.platform === 'win32')( + 'translates a Windows-shaped cwd past the absolute-path guard in a sandbox', + async () => { + vi.stubEnv('SANDBOX', 'qwen-code-sandbox-0'); + _setSandboxMountExistsForTest((p) => p === '/c/qwen-repro'); + try { + const { app } = createApp(); + const res = await request(app) + .post('/workspaces') + .send({ cwd: 'C:\\qwen-repro' }); + expect(res.status).toBe(400); + // Past the guard: the failure is now the realpath existence check, + // not the absolute-path rejection. + expect(res.body.error).toBe('Path does not exist or is not accessible'); + } finally { + vi.unstubAllEnvs(); + _setSandboxMountExistsForTest(undefined); + } + }, + ); + + it.skipIf(process.platform === 'win32')( + 'still rejects a Windows-shaped cwd outside a sandbox', + async () => { + vi.stubEnv('SANDBOX', ''); + try { + const { app } = createApp(); + const res = await request(app) + .post('/workspaces') + .send({ cwd: 'C:\\qwen-repro' }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('`cwd` must be an absolute path'); + } finally { + vi.unstubAllEnvs(); + } + }, + ); + it('returns 400 when path does not exist', async () => { const { app } = createApp(); const res = await request(app) diff --git a/packages/cli/src/serve/routes/workspace-management.ts b/packages/cli/src/serve/routes/workspace-management.ts index ca4a48a8626..47c5b7d2836 100644 --- a/packages/cli/src/serve/routes/workspace-management.ts +++ b/packages/cli/src/serve/routes/workspace-management.ts @@ -5,11 +5,14 @@ */ import { readdir, stat } from 'node:fs/promises'; +import { + translateAndCheckAbsoluteWorkspacePath, + MAX_WORKSPACE_PATH_LENGTH, +} from '@qwen-code/acp-bridge/workspacePaths'; import { realpathSync } from 'node:fs'; import { basename, dirname, isAbsolute, join, resolve, sep } from 'node:path'; import type { Application, Request, Response } from 'express'; import { isWithinRoot } from '@qwen-code/qwen-code-core'; -import { MAX_WORKSPACE_PATH_LENGTH } from '@qwen-code/acp-bridge/workspacePaths'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import { MAX_REGISTERED_WORKSPACES } from '../workspace-inputs.js'; import type { @@ -240,19 +243,24 @@ export function registerWorkspaceManagementRoutes( return; } - if (!isAbsolute(cwd)) { + // Bound the input before any filesystem work, matching the limit other + // workspace routes enforce (memory-amplification guard). Must run + // before the sandbox translation below — its existence probe is a + // filesystem call. + if (cwd.length > MAX_WORKSPACE_PATH_LENGTH) { res.status(400).json({ - error: '`cwd` must be an absolute path', + error: `\`cwd\` exceeds the ${MAX_WORKSPACE_PATH_LENGTH}-character limit`, code: 'invalid_path', }); return; } - // Bound the input before any filesystem work, matching the limit other - // workspace routes enforce (memory-amplification guard). - if (cwd.length > MAX_WORKSPACE_PATH_LENGTH) { + // #7139: the shared helper maps a Windows-shaped cwd to its container + // bind mount before the absolute-path check. + const sandboxCwd = translateAndCheckAbsoluteWorkspacePath(cwd); + if (sandboxCwd === null) { res.status(400).json({ - error: `\`cwd\` exceeds the ${MAX_WORKSPACE_PATH_LENGTH}-character limit`, + error: '`cwd` must be an absolute path', code: 'invalid_path', }); return; @@ -265,7 +273,7 @@ export function registerWorkspaceManagementRoutes( // two distinct canonical strings and defeat the duplicate check. let canonical: string; try { - canonical = realpathSync.native(resolve(cwd)); + canonical = realpathSync.native(resolve(sandboxCwd)); } catch { res.status(400).json({ error: 'Path does not exist or is not accessible', diff --git a/packages/cli/src/serve/run-qwen-serve.sandbox.test.ts b/packages/cli/src/serve/run-qwen-serve.sandbox.test.ts new file mode 100644 index 00000000000..62ca33dea76 --- /dev/null +++ b/packages/cli/src/serve/run-qwen-serve.sandbox.test.ts @@ -0,0 +1,66 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { _setSandboxMountExistsForTest } from '@qwen-code/acp-bridge/workspacePaths'; +import { validateAndCanonicalizeWorkspaceInput } from './run-qwen-serve.js'; + +// #7139 wiring for the PRIMARY reproduction path: `qwen serve --workspace +// C:\qwen-repro` relaunched into a Linux Docker sandbox. The boot validator +// must translate to the bind mount BEFORE its absolute-path guard. statSync +// is mocked so the (root-level, uncreatable) translated mount stats as a +// directory; acp-bridge's canonicalizeWorkspace then falls back to the +// resolved path on ENOENT, matching a real container where the mount exists. +vi.mock('node:fs', async (importOriginal) => { + const actual = (await importOriginal()) as typeof import('node:fs'); + return { + ...actual, + statSync: ((p: unknown, ...rest: unknown[]) => + p === '/c/qwen-repro' + ? ({ isDirectory: () => true } as ReturnType) + : (actual.statSync as (...a: unknown[]) => unknown)( + p, + ...rest, + )) as typeof actual.statSync, + }; +}); + +describe('validateAndCanonicalizeWorkspaceInput inside a POSIX container sandbox (#7139)', () => { + afterEach(() => { + vi.unstubAllEnvs(); + _setSandboxMountExistsForTest(undefined); + }); + + it.skipIf(process.platform === 'win32')( + 'boots a Windows-shaped --workspace via its bind-mount location', + () => { + vi.stubEnv('SANDBOX', 'qwen-code-sandbox-0'); + _setSandboxMountExistsForTest((p) => p === '/c/qwen-repro'); + expect(validateAndCanonicalizeWorkspaceInput('C:\\qwen-repro')).toBe( + '/c/qwen-repro', + ); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'still rejects a Windows-shaped --workspace outside a sandbox', + () => { + vi.stubEnv('SANDBOX', ''); + expect(() => + validateAndCanonicalizeWorkspaceInput('C:\\qwen-repro'), + ).toThrow('must be an absolute path'); + }, + ); + + it('echoes the operator-typed input in the rejection, not the translation result', () => { + vi.stubEnv('SANDBOX', ''); + // The extraction renamed the parameter; the message must interpolate + // the RAW input ("relative/path"), never the null translation result. + expect(() => + validateAndCanonicalizeWorkspaceInput('relative/path'), + ).toThrow('Invalid --workspace "relative/path": must be an absolute path.'); + }); +}); diff --git a/packages/cli/src/serve/run-qwen-serve.ts b/packages/cli/src/serve/run-qwen-serve.ts index bb7bac9e01b..641e6d40940 100644 --- a/packages/cli/src/serve/run-qwen-serve.ts +++ b/packages/cli/src/serve/run-qwen-serve.ts @@ -37,7 +37,10 @@ import { resolveWorkspaceInputs, } from './workspace-inputs.js'; import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; -import { canonicalizeWorkspace } from '@qwen-code/acp-bridge/workspacePaths'; +import { + canonicalizeWorkspace, + translateAndCheckAbsoluteWorkspacePath, +} from '@qwen-code/acp-bridge/workspacePaths'; import type { AuthType, ProviderSetupInputs, @@ -1695,6 +1698,60 @@ interface DaemonLoggerLifecycleCallbacks { signalOwned(): void; } +/** + * Validates and canonicalizes a `--workspace` boot argument. Extracted to + * module scope (from the runQwenServe closure) so the #7139 sandbox path + * translation ahead of the absolute-path guard is testable — this is the + * primary reproduction path of that issue. + */ +export function validateAndCanonicalizeWorkspaceInput( + rawWorkspace: string, +): string { + // #7139: inside a Linux container sandbox a Windows host forwards + // `--workspace C:\…` in host shape; translate to the bind-mount + // location BEFORE the absolute-path guard, which would otherwise + // reject it (`path.isAbsolute('C:\…')` is false on POSIX). + const workspace = translateAndCheckAbsoluteWorkspacePath(rawWorkspace); + if (workspace === null) { + throw new Error( + `Invalid --workspace "${rawWorkspace}": must be an absolute path.`, + ); + } + try { + const stats = fs.statSync(workspace); + if (!stats.isDirectory()) { + throw new Error( + `Invalid --workspace "${workspace}": exists but is not a directory.`, + ); + } + } catch (err) { + if (err && typeof err === 'object' && 'code' in err) { + const code = (err as { code?: unknown }).code; + if (code === 'ENOENT') { + throw new Error( + `Invalid --workspace "${workspace}": directory does not exist.`, + ); + } + // EACCES / EPERM: the path exists but the current user can't + // stat it (typical for SIP-protected paths on macOS, root-owned + // dirs the daemon's user can't traverse, etc.). The raw Node + // SystemError has the path AND the syscall but no operator- + // facing breadcrumb that this came from `--workspace`. Wrap + // both codes so the boot failure points at the flag the + // operator actually set. + if (code === 'EACCES' || code === 'EPERM') { + throw new Error( + `Invalid --workspace "${workspace}": permission denied ` + + `(${String(code)}). The path exists but cannot be stat'd ` + + `by the current user.`, + ); + } + } + throw err; + } + return canonicalizeWorkspace(workspace); +} + export async function runQwenServe( optsIn: RunQwenServeOptions, deps: RunQwenServeDeps = {}, @@ -2053,46 +2110,8 @@ async function runQwenServeImpl( ); } - const validateAndCanonicalizeWorkspace = (workspace: string): string => { - if (!path.isAbsolute(workspace)) { - throw new Error( - `Invalid --workspace "${workspace}": must be an absolute path.`, - ); - } - try { - const stats = fs.statSync(workspace); - if (!stats.isDirectory()) { - throw new Error( - `Invalid --workspace "${workspace}": exists but is not a directory.`, - ); - } - } catch (err) { - if (err && typeof err === 'object' && 'code' in err) { - const code = (err as { code?: unknown }).code; - if (code === 'ENOENT') { - throw new Error( - `Invalid --workspace "${workspace}": directory does not exist.`, - ); - } - // EACCES / EPERM: the path exists but the current user can't - // stat it (typical for SIP-protected paths on macOS, root-owned - // dirs the daemon's user can't traverse, etc.). The raw Node - // SystemError has the path AND the syscall but no operator- - // facing breadcrumb that this came from `--workspace`. Wrap - // both codes so the boot failure points at the flag the - // operator actually set. - if (code === 'EACCES' || code === 'EPERM') { - throw new Error( - `Invalid --workspace "${workspace}": permission denied ` + - `(${String(code)}). The path exists but cannot be stat'd ` + - `by the current user.`, - ); - } - } - throw err; - } - return canonicalizeWorkspace(workspace); - }; + const validateAndCanonicalizeWorkspace = + validateAndCanonicalizeWorkspaceInput; // Resolve the bound workspace list. The first explicit workspace remains the // primary workspace for legacy APIs; later workspaces are isolated secondary diff --git a/packages/cli/src/serve/server/request-helpers.sandbox.test.ts b/packages/cli/src/serve/server/request-helpers.sandbox.test.ts new file mode 100644 index 00000000000..9b3310e6d64 --- /dev/null +++ b/packages/cli/src/serve/server/request-helpers.sandbox.test.ts @@ -0,0 +1,77 @@ +/** + * @license + * Copyright 2025 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { Response } from 'express'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { _setSandboxMountExistsForTest } from '@qwen-code/acp-bridge/workspacePaths'; +import { parseOptionalWorkspaceCwd } from './request-helpers.js'; + +// Regression for the #7228 review finding: every workspace-ingestion path +// validates with `path.isAbsolute` BEFORE canonicalization, and on POSIX +// `isAbsolute('C:\\…')` is false — so a translation that only lives inside +// canonicalizeWorkspace never runs on the real request path. This test +// drives the real exported REST-route parser (guard included), not the +// canonicalization choke point. The mount-existence probe is stubbed via +// the acp-bridge test seam (the `/c/…` target sits at the filesystem root, +// which tests cannot create). + +function mockRes(): { res: Response; status: ReturnType } { + const status = vi.fn().mockReturnValue({ json: vi.fn() }); + return { res: { status } as unknown as Response, status }; +} + +describe('parseOptionalWorkspaceCwd inside a POSIX container sandbox (#7139)', () => { + afterEach(() => { + vi.unstubAllEnvs(); + _setSandboxMountExistsForTest(undefined); + }); + + it.skipIf(process.platform === 'win32')( + 'accepts a Windows-shaped cwd and returns its bind-mount location', + () => { + vi.stubEnv('SANDBOX', 'qwen-code-sandbox-0'); + _setSandboxMountExistsForTest((p) => p === '/c/qwen-repro'); + const { res, status } = mockRes(); + const cwd = parseOptionalWorkspaceCwd( + { cwd: 'C:\\qwen-repro' }, + '/c/qwen-repro', + res, + ); + expect(cwd).toBe('/c/qwen-repro'); + expect(status).not.toHaveBeenCalled(); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'still rejects a Windows-shaped cwd outside a sandbox', + () => { + vi.stubEnv('SANDBOX', ''); + const { res, status } = mockRes(); + const cwd = parseOptionalWorkspaceCwd( + { cwd: 'C:\\qwen-repro' }, + '/tmp', + res, + ); + expect(cwd).toBeUndefined(); + expect(status).toHaveBeenCalledWith(400); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'rejects when the translated mount does not exist (no invented paths)', + () => { + vi.stubEnv('SANDBOX', 'qwen-code-sandbox-0'); + const { res, status } = mockRes(); + const cwd = parseOptionalWorkspaceCwd( + { cwd: 'D:\\never-mounted' }, + '/tmp', + res, + ); + expect(cwd).toBeUndefined(); + expect(status).toHaveBeenCalledWith(400); + }, + ); +}); diff --git a/packages/cli/src/serve/server/request-helpers.ts b/packages/cli/src/serve/server/request-helpers.ts index 5cee3b8ab6d..11465df493b 100644 --- a/packages/cli/src/serve/server/request-helpers.ts +++ b/packages/cli/src/serve/server/request-helpers.ts @@ -4,10 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as path from 'node:path'; +import { + MAX_WORKSPACE_PATH_LENGTH, + translateAndCheckAbsoluteWorkspacePath, +} from '@qwen-code/acp-bridge/workspacePaths'; import type { Request, Response } from 'express'; import type { AcpSessionBridge } from '@qwen-code/acp-bridge/bridgeTypes'; -import { MAX_WORKSPACE_PATH_LENGTH } from '@qwen-code/acp-bridge/workspacePaths'; import { writeStderrLine } from '../../utils/stdioHelpers.js'; import type { WorkspaceRequestContext } from '../workspace-service/index.js'; @@ -143,8 +145,12 @@ export function parseOptionalWorkspaceCwd( }); return undefined; } - const cwd = hasCwd ? (body['cwd'] as string) : boundWorkspace; - if (!path.isAbsolute(cwd)) { + // #7139: the shared helper maps a Windows-shaped cwd to its container + // bind mount before the absolute-path check. + const cwd = translateAndCheckAbsoluteWorkspacePath( + hasCwd ? (body['cwd'] as string) : boundWorkspace, + ); + if (cwd === null) { res .status(400) .json({ error: '`cwd` must be an absolute path when provided' });