fix(serve): Harden multi-workspace ownership guards (#7005)

* fix(serve): harden multi-workspace ownership guards

Co-authored-by: Qwen-Coder <[email protected]>

* codex: address PR review feedback (#7005)

Co-authored-by: Qwen-Coder <[email protected]>

* test(serve): cover empty runtime overlay isolation

Co-authored-by: Qwen-Coder <[email protected]>

* codex: address PR review feedback (#7005)

Co-authored-by: Qwen-Coder <[email protected]>

---------

Co-authored-by: Qwen-Coder <[email protected]>
This commit is contained in:
jinyeandQwen-Coder authored and GitHub committed 2026-07-16 17:29:10 +00:00
1 parent 10a39d15d2
commit 117adc32ee
12 files changed
+584 -81

No files matched your search

+9
View File
@@ -243,6 +243,15 @@ things a reviewer of _this_ codebase must check — not general advice.
set by any caller is a dead switch (`options.foo ?? true` always takes the
default). Decide severity at the read site; never explain an unpopulated field
with author intent you cannot observe.
- **Classify every added or changed daemon route by ownership.** Name whether it
is process-global, legacy-primary, selected-runtime, live-session-owner, or
persisted-workspace scoped, and verify every downstream consumer matches that
scope.
- **Verify workspace-scoped routes stay inside the resolved runtime.** Check the
environment, bridge, service, filesystem, trust boundary, and failure paths.
Each unknown, untrusted, ambiguous, bootstrapping, draining, or removed state
must follow its declared failure semantics and must never fall back to the
primary runtime.
- **Match the house style when judging.** ESM only; no `any`; no relative imports
between packages; `kebab-case.ts` for `.ts` in `packages/core` and `packages/cli`,
`PascalCase.tsx` for React components; tests collocated as `file.test.ts`.
+2 -1
View File
@@ -667,6 +667,7 @@ export class AcpDispatcher {
constructor(
private readonly bridge: HttpAcpBridge,
private readonly boundWorkspace: string,
private readonly env: Readonly<NodeJS.ProcessEnv>,
private readonly workspace: DaemonWorkspaceService,
private readonly workspaceRememberLane: WorkspaceRememberTaskLane,
private readonly fsFactory?: WorkspaceFileSystemFactory,
@@ -2409,7 +2410,7 @@ export class AcpDispatcher {
const result = await setupGithub({
cwd: this.boundWorkspace,
workspaceRoot: this.boundWorkspace,
proxy: resolveSetupGithubProxy(this.boundWorkspace),
proxy: resolveSetupGithubProxy(this.boundWorkspace, this.env),
abortSignal: conn.abortSignal,
fileOps: createSetupGithubFileOps(
this.fsFactory,
+22 -1
View File
@@ -202,6 +202,7 @@ function delay(ms: number): Promise<void> {
function buildChromeDevToolsMcpRuntimeConfig(
localPort: number | undefined,
hostname: string | undefined,
env: Readonly<NodeJS.ProcessEnv>,
): Record<string, unknown> | undefined {
if (
localPort === undefined ||
@@ -210,7 +211,7 @@ function buildChromeDevToolsMcpRuntimeConfig(
) {
return undefined;
}
const command = resolveCdpMcpCommand(process.env);
const command = resolveCdpMcpCommand(env);
if (!command) {
writeStderrLine(
`qwen serve: set ${QWEN_CDP_MCP_COMMAND_ENV} to enable browser automation MCP (no adapter is bundled)`,
@@ -361,6 +362,8 @@ const MAX_INFLIGHT_MCP_DISPATCH = 8;
export interface MountAcpHttpOptions {
boundWorkspace: string;
/** Process-level fallback for embedded mounts and parent-process runtimes. */
daemonEnv?: Readonly<NodeJS.ProcessEnv>;
workspace: DaemonWorkspaceService;
fsFactory?: WorkspaceFileSystemFactory;
deviceFlowRegistry?: DeviceFlowRegistry;
@@ -546,6 +549,17 @@ export interface AcpHttpHandle {
attachServer(server: import('node:http').Server): void;
}
function runtimeEffectiveEnv(
runtime: WorkspaceRuntime,
daemonEnv: Readonly<NodeJS.ProcessEnv>,
): Readonly<NodeJS.ProcessEnv> {
if (runtime.env.mode === 'runtime-overlay') {
// An empty overlay must stay isolated instead of inheriting daemon values.
return runtime.env.effectiveEnv ?? {};
}
return runtime.env.effectiveEnv ?? daemonEnv;
}
/**
* Mount the official ACP Streamable HTTP transport (RFD #721) on an
* existing Express app, backed by the shared `HttpAcpBridge`. Additive:
@@ -566,6 +580,10 @@ export function mountAcpHttp(
const enabled = opts.enabled ?? resolveAcpHttpEnabled();
if (!enabled) return undefined;
const daemonEnv = opts.daemonEnv ?? process.env;
const primaryEnv = opts.workspaceRegistry
? runtimeEffectiveEnv(opts.workspaceRegistry.primary, daemonEnv)
: daemonEnv;
const path = opts.path ?? '/acp';
const dispatcherRef: { current?: AcpDispatcher } = {};
// Lifecycle gate: once `dispose()` runs, late/in-flight HTTP requests get a
@@ -680,6 +698,7 @@ export function mountAcpHttp(
const runtimeConfig = buildChromeDevToolsMcpRuntimeConfig(
localPort,
opts.hostname,
daemonEnv,
);
if (!runtimeConfig) {
cdpMcpTerminalSkipLogged = true;
@@ -747,6 +766,7 @@ export function mountAcpHttp(
const dispatcher = new AcpDispatcher(
bridge,
opts.boundWorkspace,
primaryEnv,
opts.workspace,
opts.workspaceRememberLane,
opts.fsFactory,
@@ -1201,6 +1221,7 @@ export function mountAcpHttp(
const secondaryDispatcher = new AcpDispatcher(
rt.bridge,
rt.workspaceCwd,
runtimeEffectiveEnv(rt, daemonEnv),
rt.workspaceService,
workspaceRememberLane,
rt.routeFileSystemFactory,
@@ -879,6 +879,7 @@ describe('ACP Streamable HTTP transport (over the wire)', () => {
nextBridge?: FakeBridge;
fsFactory?: WorkspaceFileSystemFactory;
boundWorkspace?: string;
daemonEnv?: Readonly<NodeJS.ProcessEnv>;
}): Promise<void> {
server.closeAllConnections?.();
await new Promise<void>((r) => server.close(() => r()));
@@ -890,6 +891,7 @@ describe('ACP Streamable HTTP transport (over the wire)', () => {
boundWorkspace,
workspace: fakeWorkspace,
enabled: true,
daemonEnv: opts.daemonEnv,
fsFactory: opts.fsFactory,
sessionShellCommandEnabled: opts.sessionShellCommandEnabled,
workspaceRememberLane: new WorkspaceRememberTaskLane(
@@ -5252,7 +5254,10 @@ describe('ACP Streamable HTTP transport (over the wire)', () => {
});
it('dispatches _qwen/workspace/setup-github', async () => {
await restartServer({ fsFactory: makeFileFsFactory({}) });
await restartServer({
fsFactory: makeFileFsFactory({}),
daemonEnv: { HTTPS_PROXY: 'http://runtime-proxy.example:8080' },
});
const connId = await initialize();
const connStream = await openStream(connId);
const got = takeFrames(connStream, 1);
@@ -5277,6 +5282,7 @@ describe('ACP Streamable HTTP transport (over the wire)', () => {
expect.objectContaining({
cwd: '/ws',
workspaceRoot: '/ws',
proxy: 'http://runtime-proxy.example:8080',
abortSignal: expect.any(AbortSignal),
fileOps: expect.any(Object),
}),
@@ -7999,6 +8005,7 @@ describe('ACP WebSocket transport security', () => {
allowedOrigins?: { allowAny: boolean; origins: Set<string> };
checkRate?: (key: string, tier: string) => boolean;
cdpTunnelOverWs?: boolean;
daemonEnv?: Readonly<NodeJS.ProcessEnv>;
} = {},
) {
return new Promise<void>((resolve) => {
@@ -8009,6 +8016,7 @@ describe('ACP WebSocket transport security', () => {
boundWorkspace: '/ws',
workspace: fakeWorkspace,
enabled: true,
daemonEnv: opts.daemonEnv,
token: opts.token,
allowedOrigins: opts.allowedOrigins,
workspaceRememberLane: new WorkspaceRememberTaskLane(
@@ -8205,8 +8213,11 @@ describe('ACP WebSocket transport security', () => {
});
it('passes a custom CDP MCP command through to the runtime config', async () => {
process.env['QWEN_CDP_MCP_COMMAND'] = '/opt/custom/cdp-adapter';
await startServer({ cdpTunnelOverWs: true });
process.env['QWEN_CDP_MCP_COMMAND'] = '/opt/process/cdp-adapter';
await startServer({
cdpTunnelOverWs: true,
daemonEnv: { QWEN_CDP_MCP_COMMAND: '/opt/custom/cdp-adapter' },
});
const ws = await wsConnect();
await initializeCdpBridge(ws);
@@ -29,7 +29,13 @@ import type { DaemonWorkspaceService } from '../workspace-service/types.js';
import { writeStderrLine } from '../../utils/stdioHelpers.js';
import { createSessionOrganizationService } from '../session-organization-helpers.js';
const setupGithubMock = vi.hoisted(() => vi.fn());
vi.mock('../../utils/stdioHelpers.js', () => ({ writeStderrLine: vi.fn() }));
vi.mock('../../services/setup-github.js', async (importOriginal) => ({
...(await importOriginal<typeof import('../../services/setup-github.js')>()),
setupGithub: setupGithubMock,
}));
const PARENT_ENV: WorkspaceRuntimeEnvMetadata = {
mode: 'parent-process',
@@ -67,13 +73,14 @@ function makeRuntime(input: {
primary: boolean;
trusted: boolean;
bridge: HttpAcpBridge;
env?: WorkspaceRuntimeEnvMetadata;
}): WorkspaceRuntime {
return {
workspaceId: input.id,
workspaceCwd: input.cwd,
primary: input.primary,
trusted: input.trusted,
env: PARENT_ENV,
env: input.env ?? PARENT_ENV,
bridge: input.bridge,
workspaceService: {} as unknown as DaemonWorkspaceService,
routeFileSystemFactory: {
@@ -140,6 +147,17 @@ describe('workspace-qualified ACP (/workspaces/:workspace/acp)', () => {
let workspaceVoiceConnection: ReturnType<typeof vi.fn>;
beforeEach(async () => {
setupGithubMock.mockReset();
setupGithubMock.mockImplementation(async ({ cwd }: { cwd: string }) => ({
kind: 'github_setup',
workspaceCwd: cwd,
gitRepoRoot: cwd,
releaseTag: 'v1.2.3',
readmeUrl: 'https://example.test/readme',
workflows: [],
gitignore: { path: '.gitignore', status: 'unchanged' },
warnings: [],
}));
primaryBridge = makeBridge();
secondaryBridge = makeBridge();
const untrustedBridge = makeBridge();
@@ -150,6 +168,13 @@ describe('workspace-qualified ACP (/workspaces/:workspace/acp)', () => {
primary: false,
trusted: true,
bridge: secondaryBridge,
env: {
mode: 'runtime-overlay',
overlayKeys: ['HTTPS_PROXY'],
effectiveEnv: {
HTTPS_PROXY: 'http://secondary-proxy.example:8080',
},
},
});
workspaceRegistry = createWorkspaceRegistry([
makeRuntime({
@@ -158,6 +183,7 @@ describe('workspace-qualified ACP (/workspaces/:workspace/acp)', () => {
primary: true,
trusted: true,
bridge: primaryBridge,
env: PARENT_ENV,
}),
secondaryRuntime,
makeRuntime({
@@ -189,7 +215,12 @@ describe('workspace-qualified ACP (/workspaces/:workspace/acp)', () => {
handle = mountAcpHttp(app, primaryBridge, {
boundWorkspace: '/ws',
workspace: {} as unknown as DaemonWorkspaceService,
fsFactory: workspaceRegistry.primary.routeFileSystemFactory,
enabled: true,
daemonEnv: {
...process.env,
HTTPS_PROXY: 'http://primary-proxy.example:8080',
},
workspaceRegistry,
deviceFlowRegistry,
cdpTunnelOverWs: true,
@@ -355,6 +386,61 @@ describe('workspace-qualified ACP (/workspaces/:workspace/acp)', () => {
);
});
it('uses daemon env for parent-process setup-github without leaking into overlays', async () => {
const secondary = await sendWsRequest('/workspaces/secondary-id/acp', {
jsonrpc: '2.0',
id: 2,
method: '_qwen/workspace/setup-github',
params: { consent: true },
});
expect(secondary['result']).toMatchObject({ workspaceCwd: '/ws-b' });
expect(setupGithubMock).toHaveBeenLastCalledWith(
expect.objectContaining({
cwd: '/ws-b',
proxy: 'http://secondary-proxy.example:8080',
}),
);
const primary = await sendWsRequest('/acp', {
jsonrpc: '2.0',
id: 3,
method: '_qwen/workspace/setup-github',
params: { consent: true },
});
expect(primary['result']).toMatchObject({ workspaceCwd: '/ws' });
expect(setupGithubMock).toHaveBeenLastCalledWith(
expect.objectContaining({
cwd: '/ws',
proxy: 'http://primary-proxy.example:8080',
}),
);
});
it('keeps empty runtime overlays isolated from daemon env', async () => {
workspaceRegistry.add(
makeRuntime({
id: 'empty-overlay-id',
cwd: '/ws-empty',
primary: false,
trusted: true,
bridge: makeBridge(),
env: { mode: 'runtime-overlay', overlayKeys: [] },
}),
);
const response = await sendWsRequest('/workspaces/empty-overlay-id/acp', {
jsonrpc: '2.0',
id: 4,
method: '_qwen/workspace/setup-github',
params: { consent: true },
});
expect(response['result']).toMatchObject({ workspaceCwd: '/ws-empty' });
expect(setupGithubMock).toHaveBeenLastCalledWith(
expect.objectContaining({ cwd: '/ws-empty', proxy: undefined }),
);
});
it('routes initialize to a trusted secondary workspace by encoded cwd', async () => {
const res = await postInitialize(
`/workspaces/${encodeURIComponent('/ws-b')}/acp`,
@@ -152,6 +152,10 @@ interface FakeBridge extends AcpSessionBridge {
signal?: AbortSignal;
context?: BridgeClientRequestContext;
}>;
readonly primaryOnlyMutationCalls: Array<{
route: 'branch' | 'fork' | 'cd';
sessionId: string;
}>;
}
function makeSummary(
@@ -284,6 +288,7 @@ function makeBridge(
const rewindSnapshotCalls: string[] = [];
const rewindCalls: FakeBridge['rewindCalls'] = [];
const shellCalls: FakeBridge['shellCalls'] = [];
const primaryOnlyMutationCalls: FakeBridge['primaryOnlyMutationCalls'] = [];
const bridge = {
permissionPolicy: 'first-responder' as const,
spawnCalls,
@@ -314,6 +319,7 @@ function makeBridge(
rewindSnapshotCalls,
rewindCalls,
shellCalls,
primaryOnlyMutationCalls,
get sessionCount() {
return live.size;
},
@@ -607,6 +613,18 @@ function makeBridge(
}
return { exitCode: 0, output: workspaceCwd, aborted: false };
},
async branchSession(sessionId: string) {
primaryOnlyMutationCalls.push({ route: 'branch', sessionId });
throw new Error('Unexpected branchSession call');
},
async launchSessionForkAgent(sessionId: string) {
primaryOnlyMutationCalls.push({ route: 'fork', sessionId });
throw new Error('Unexpected launchSessionForkAgent call');
},
async changeSessionCwd(sessionId: string) {
primaryOnlyMutationCalls.push({ route: 'cd', sessionId });
throw new Error('Unexpected changeSessionCwd call');
},
async cancelSession(sessionId: string) {
cancelCalls.push(sessionId);
},
@@ -1548,20 +1566,55 @@ describe('multi-workspace session dispatch', () => {
);
});
it('returns a clear Phase 2a error for non-primary sessions on primary-only routes', async () => {
const { app, primaryBridge, secondaryBridge } = makeHarness();
it.each([
{
suffix: 'branch',
route: 'POST /session/:id/branch',
body: { name: 'next' },
},
{
suffix: 'fork',
route: 'POST /session/:id/fork',
body: { directive: 'review this' },
},
{
suffix: 'cd',
route: 'POST /session/:id/cd',
body: { path: path.resolve(path.sep, 'work', 'next') },
},
])(
'rejects $route for a non-primary live-session owner',
async ({ suffix, route, body }) => {
const daemonLog = makeDaemonLog();
const { app, primaryBridge, secondaryBridge } = makeHarness({
daemonLog,
});
const res = await request(app)
.post('/session/secondary-session/branch')
.set('Host', host())
.send({ name: 'next' });
const res = await request(app)
.post(`/session/secondary-session/${suffix}`)
.set('Host', host())
.send(body);
expect(res.status).toBe(400);
expect(res.body.code).toBe('non_primary_session_route_not_supported');
expect(res.body.workspaceCwd).toBe(SECONDARY_CWD);
expect(primaryBridge.restoreCalls).toEqual([]);
expect(secondaryBridge.restoreCalls).toEqual([]);
});
expect(res.status).toBe(400);
expect(res.body).toEqual({
error: `Route "${route}" is only available for primary workspace sessions.`,
code: 'non_primary_session_route_not_supported',
sessionId: 'secondary-session',
workspaceId: 'secondary-id',
workspaceCwd: SECONDARY_CWD,
route,
});
expect(primaryBridge.primaryOnlyMutationCalls).toEqual([]);
expect(secondaryBridge.primaryOnlyMutationCalls).toEqual([]);
expect(daemonLog.warn).toHaveBeenCalledWith('session routing failed', {
route,
resolutionKind: 'non_primary_session_route_not_supported',
sessionId: 'secondary-session',
workspaceId: 'secondary-id',
workspaceCwd: SECONDARY_CWD,
});
},
);
it('routes POST /session/:id/model to the owning non-primary workspace bridge', async () => {
const { app, primaryBridge, secondaryBridge } = makeHarness();
+261 -19
View File
@@ -7,6 +7,7 @@
import * as fs from 'node:fs';
import * as path from 'node:path';
import { fileURLToPath } from 'node:url';
import * as ts from 'typescript';
import { describe, expect, it } from 'vitest';
const repoRoot = path.resolve(
@@ -22,26 +23,159 @@ const scannedRoots = [
path.join(repoRoot, 'packages', 'acp-bridge', 'src'),
];
const allowedProcessEnvFiles = new Set(
interface ProcessEnvAllowance {
readonly reason: string;
readonly accesses: Readonly<Record<string, number>>;
}
function normalizeAllowances(
entries: ReadonlyArray<readonly [string, ProcessEnvAllowance]>,
): ReadonlyMap<string, ProcessEnvAllowance> {
return new Map(
entries.map(([file, allowance]) => [path.normalize(file), allowance]),
);
}
const allowedProcessEnvAccesses = normalizeAllowances([
[
'packages/acp-bridge/src/bridge.ts',
'packages/acp-bridge/src/bridgeOptions.ts',
{
reason: 'The ACP bridge debug switch is process-scoped.',
accesses: { 'key:QWEN_SERVE_DEBUG': 1 },
},
],
[
'packages/acp-bridge/src/spawnChannel.ts',
{
reason:
'Standalone channel spawning keeps a process-environment compatibility fallback.',
accesses: { whole: 1 },
},
],
[
'packages/cli/src/serve/acp-http-enabled.ts',
{
reason:
'Embedded callers may omit the daemon-level environment argument.',
accesses: { whole: 1 },
},
],
[
'packages/cli/src/serve/acp-http/index.ts',
{
reason:
'Embedded ACP mounts may omit the daemon-level environment argument.',
accesses: { whole: 1 },
},
],
[
'packages/cli/src/serve/channel-worker-supervisor.ts',
{
reason:
'The process-global channel supervisor needs a base environment for child workers.',
accesses: { whole: 1 },
},
],
[
'packages/cli/src/serve/daemon-logger.ts',
{
reason: 'Daemon log and runtime locations are process-scoped.',
accesses: {
'key:QWEN_DAEMON_LOG_FILE': 1,
'key:QWEN_RUNTIME_DIR': 1,
},
},
],
[
'packages/cli/src/serve/debug-mode.ts',
{
reason: 'Serve debug logging is process-scoped.',
accesses: { 'key:QWEN_SERVE_DEBUG': 1 },
},
],
[
'packages/cli/src/serve/env-snapshot.ts',
{
reason:
'The daemon snapshots its process environment before applying workspace overlays.',
accesses: { whole: 1 },
},
],
[
'packages/cli/src/serve/fast-path-settings.ts',
{
reason:
'Fast-path settings initialize daemon bootstrap compatibility state.',
accesses: {
'computed:key': 4,
'key:CLOUD_SHELL': 1,
'key:GOOGLE_CLOUD_PROJECT': 2,
'key:QWEN_CODE_TRUSTED_FOLDERS_PATH': 1,
'key:QWEN_HOME': 3,
whole: 4,
},
},
],
[
'packages/cli/src/serve/fast-path.ts',
{
reason:
'The fast-path entry point initializes process-level daemon defaults.',
accesses: { whole: 2 },
},
],
[
'packages/cli/src/serve/fs/audit.ts',
'packages/cli/src/serve/routes/workspace-setup-github.ts',
{
reason: 'Filesystem audit redaction is a daemon-wide logging policy.',
accesses: { 'key:QWEN_AUDIT_RAW_PATHS': 1 },
},
],
[
'packages/cli/src/serve/run-qwen-serve.ts',
{
reason:
'The serve entry point owns daemon bootstrap, feature flags, and child-process defaults.',
accesses: {
'computed:QWEN_SERVER_TOKEN_ENV': 1,
'computed:QWEN_SERVE_CDP_TUNNEL_OVER_WS_ENV': 1,
'computed:QWEN_SERVE_CLIENT_MCP_OVER_WS_ENV': 1,
'computed:QWEN_SERVE_PROMPT_DEADLINE_MS_ENV': 1,
'computed:QWEN_SERVE_WRITER_IDLE_TIMEOUT_MS_ENV': 1,
'computed:RUNTIME_STARTUP_TIMEOUT_ENV': 1,
'key:QWEN_CODE_IDE_WORKSPACE_PATH': 1,
'key:QWEN_SERVE_NO_MCP_POOL': 1,
'key:QWEN_SERVE_NO_PERSISTENT_REGISTRATION': 1,
'key:VITEST_WORKER_ID': 1,
whole: 4,
},
},
],
[
'packages/cli/src/serve/server/fs-factory.ts',
{
reason:
'Embedded server construction keeps a process-environment compatibility fallback.',
accesses: { 'computed:IDE_WORKSPACE_PATH_ENV_VAR': 1 },
},
],
[
'packages/cli/src/serve/server.ts',
{
reason:
'Embedded server construction keeps a process-environment compatibility fallback.',
accesses: { whole: 1 },
},
],
[
'packages/cli/src/serve/server/serve-features.ts',
].map((file) => path.normalize(file)),
);
{
reason:
'Embedded feature detection defaults to the daemon process environment.',
accesses: { whole: 1 },
},
],
]);
function listTypeScriptFiles(dir: string): string[] {
const out: string[] = [];
@@ -49,33 +183,141 @@ function listTypeScriptFiles(dir: string): string[] {
const full = path.join(dir, entry.name);
if (entry.isDirectory()) {
listTypeScriptFiles(full).forEach((file) => out.push(file));
} else if (entry.isFile() && full.endsWith('.ts')) {
} else if (entry.isFile() && /\.tsx?$/.test(full)) {
out.push(full);
}
}
return out;
}
function stripComments(source: string): string {
return source.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*\/\/.*$/gm, '');
function findProcessEnvAccesses(
file: string,
source: string,
): Readonly<Record<string, number>> {
const sourceFile = ts.createSourceFile(
file,
source,
ts.ScriptTarget.Latest,
true,
file.endsWith('.tsx') ? ts.ScriptKind.TSX : ts.ScriptKind.TS,
);
const accesses = new Map<string, number>();
const record = (access: string): void => {
accesses.set(access, (accesses.get(access) ?? 0) + 1);
};
const visit = (node: ts.Node): void => {
const isProcessEnv =
(ts.isPropertyAccessExpression(node) &&
ts.isIdentifier(node.expression) &&
node.expression.text === 'process' &&
node.name.text === 'env') ||
(ts.isElementAccessExpression(node) &&
ts.isIdentifier(node.expression) &&
node.expression.text === 'process' &&
ts.isStringLiteralLike(node.argumentExpression) &&
node.argumentExpression.text === 'env');
if (isProcessEnv) {
const parent = node.parent;
if (ts.isElementAccessExpression(parent) && parent.expression === node) {
const argument = parent.argumentExpression;
record(
ts.isStringLiteralLike(argument)
? `key:${argument.text}`
: `computed:${argument.getText(sourceFile)}`,
);
} else if (
ts.isPropertyAccessExpression(parent) &&
parent.expression === node
) {
record(`key:${parent.name.text}`);
} else {
record('whole');
}
}
ts.forEachChild(node, visit);
};
visit(sourceFile);
return Object.fromEntries(
[...accesses].sort(([left], [right]) => left.localeCompare(right)),
);
}
describe('serve process.env guard', () => {
it('keeps workspace-scoped serve and acp-bridge code off direct process.env reads', () => {
const offenders: string[] = [];
it('detects dot and element process.env access expressions', () => {
expect(
findProcessEnvAccesses(
'example.ts',
`
process.env.DOT;
process['env']['ELEMENT'];
process['env'].MIXED;
const whole = process['env'];
`,
),
).toEqual({
'key:DOT': 1,
'key:ELEMENT': 1,
'key:MIXED': 1,
whole: 1,
});
});
it('detects computed process.env access expressions', () => {
expect(
findProcessEnvAccesses(
'example.ts',
['process.env[DYNAMIC_VAR];', 'process.env[`TEMPLATE_${key}`];'].join(
'\n',
),
),
).toEqual({
'computed:`TEMPLATE_${key}`': 1,
'computed:DYNAMIC_VAR': 1,
});
});
it('allows only documented process-scoped process.env expressions', () => {
const actual = new Map<string, Readonly<Record<string, number>>>();
for (const root of scannedRoots) {
for (const file of listTypeScriptFiles(root)) {
if (file.endsWith('.test.ts')) continue;
const relative = path.normalize(path.relative(repoRoot, file));
if (allowedProcessEnvFiles.has(relative)) continue;
if (
stripComments(fs.readFileSync(file, 'utf8')).includes('process.env')
) {
offenders.push(relative);
}
if (/\.(?:test|spec)\.tsx?$/.test(file)) continue;
const accesses = findProcessEnvAccesses(
file,
fs.readFileSync(file, 'utf8'),
);
if (Object.keys(accesses).length === 0) continue;
actual.set(path.normalize(path.relative(repoRoot, file)), accesses);
}
}
expect(offenders).toEqual([]);
const mismatches = [
...new Set([...actual.keys(), ...allowedProcessEnvAccesses.keys()]),
]
.sort()
.flatMap((file) => {
const found = actual.get(file) ?? {};
const allowance = allowedProcessEnvAccesses.get(file);
const expected = allowance?.accesses ?? {};
const sorted = (accesses: Readonly<Record<string, number>>) =>
Object.entries(accesses).sort(([left], [right]) =>
left.localeCompare(right),
);
return JSON.stringify(sorted(found)) ===
JSON.stringify(sorted(expected))
? []
: [
{
file,
found,
expected,
reason: allowance?.reason ?? 'No process-scoped exception',
},
];
});
expect(
mismatches,
'process.env access mismatches — update allowedProcessEnvAccesses with a reason and the new access pattern',
).toEqual([]);
});
});
+67 -37
View File
@@ -105,6 +105,21 @@ const WORKSPACE_TRANSCRIPT_CURSOR_MAX_BYTES = 64 * 1024;
const TRANSCRIPT_CURSOR_TOO_LARGE_REPLAY_ERROR =
'Transcript pagination state exceeds the safe limit';
const GENERATION_HEARTBEAT_MS = 15_000;
const PRIMARY_ONLY_LIVE_SESSION_ROUTES = [
'POST /session/:id/branch',
'POST /session/:id/fork',
'POST /session/:id/cd',
] as const;
type PrimaryOnlyLiveSessionRoute =
(typeof PRIMARY_ONLY_LIVE_SESSION_ROUTES)[number];
function isPrimaryOnlyLiveSessionRoute(
route: string,
): route is PrimaryOnlyLiveSessionRoute {
return (PRIMARY_ONLY_LIVE_SESSION_ROUTES as readonly string[]).includes(
route,
);
}
function formatGenerationSse(
event: string,
@@ -836,12 +851,12 @@ export function registerSessionRoutes(
const sendNonPrimarySessionRouteUnsupported = (
res: Response,
route: string,
route: PrimaryOnlyLiveSessionRoute,
sessionId: string,
runtime: WorkspaceRuntime,
): void => {
res.status(400).json({
error: `Route "${route}" is primary-only for non-primary workspace sessions in Phase 2a.`,
error: `Route "${route}" is only available for primary workspace sessions.`,
code: 'non_primary_session_route_not_supported',
sessionId,
workspaceId: runtime.workspaceId,
@@ -1058,21 +1073,32 @@ export function registerSessionRoutes(
return key;
};
const withMutableSession =
(
route: string,
handler: (
req: Request,
res: Response,
sessionId: string,
) => Promise<void> | void,
): RequestHandler =>
async (req, res) => {
const withPrimaryOnlyMutableSession = (
route: string,
handler: (
req: Request,
res: Response,
sessionId: string,
) => Promise<void> | void,
): RequestHandler => {
if (!isPrimaryOnlyLiveSessionRoute(route)) {
throw new Error(`Unregistered primary-only session route: ${route}`);
}
return async (req, res) => {
const sessionId = requireSessionId(req, res);
if (sessionId === null) return;
const runtime = resolveLiveSessionRuntime(sessionId, res, route);
if (!runtime) return;
if (!runtime.primary) {
logSessionRoutingFailure(
route,
'non_primary_session_route_not_supported',
{
sessionId,
workspaceId: runtime.workspaceId,
workspaceCwd: runtime.workspaceCwd,
},
);
sendNonPrimarySessionRouteUnsupported(res, route, sessionId, runtime);
return;
}
@@ -1084,6 +1110,7 @@ export function registerSessionRoutes(
sendBridgeError(res, err, { route, sessionId });
}
};
};
app.post('/session', mutate(), async (req, res) => {
const body = safeBody(req);
@@ -1328,7 +1355,7 @@ export function registerSessionRoutes(
app.post(
'/session/:id/branch',
mutate(),
withMutableSession(
withPrimaryOnlyMutableSession(
'POST /session/:id/branch',
async (req, res, sessionId) => {
const body = safeBody(req);
@@ -1370,7 +1397,7 @@ export function registerSessionRoutes(
app.post(
'/session/:id/fork',
mutate(),
withMutableSession(
withPrimaryOnlyMutableSession(
'POST /session/:id/fork',
async (req, res, sessionId) => {
const body = safeBody(req);
@@ -1397,29 +1424,32 @@ export function registerSessionRoutes(
app.post(
'/session/:id/cd',
mutate(),
withMutableSession('POST /session/:id/cd', async (req, res, sessionId) => {
const body = safeBody(req);
const targetPath = body['path'];
if (
typeof targetPath !== 'string' ||
targetPath.length === 0 ||
!path.isAbsolute(targetPath)
) {
res.status(400).json({
error: '`path` is required and must be an absolute path',
code: 'invalid_path',
});
return;
}
const clientId = parseClientIdHeader(req, res);
if (clientId === null) return;
const result = await bridge.changeSessionCwd(
sessionId,
{ path: targetPath },
clientId !== undefined ? { clientId } : undefined,
);
res.status(200).json(result);
}),
withPrimaryOnlyMutableSession(
'POST /session/:id/cd',
async (req, res, sessionId) => {
const body = safeBody(req);
const targetPath = body['path'];
if (
typeof targetPath !== 'string' ||
targetPath.length === 0 ||
!path.isAbsolute(targetPath)
) {
res.status(400).json({
error: '`path` is required and must be an absolute path',
code: 'invalid_path',
});
return;
}
const clientId = parseClientIdHeader(req, res);
if (clientId === null) return;
const result = await bridge.changeSessionCwd(
sessionId,
{ path: targetPath },
clientId !== undefined ? { clientId } : undefined,
);
res.status(200).json(result);
},
),
);
app.get('/session/:id/status', (req, res) => {
@@ -89,7 +89,12 @@ function loopbackHost(): string {
}
async function makeHarness(
opts: { token?: string; trusted?: boolean } = {},
opts: {
token?: string;
trusted?: boolean;
runtimeEnv?: Readonly<NodeJS.ProcessEnv>;
daemonEnv?: Readonly<NodeJS.ProcessEnv>;
} = {},
): Promise<Harness> {
const scratch = await fsp.mkdtemp(
path.join(
@@ -125,7 +130,20 @@ async function makeHarness(
const app = createServeApp(
{ ...baseOpts, workspace, token: opts.token },
undefined,
{ bridge, fsFactory },
{
bridge,
fsFactory,
daemonEnv: opts.daemonEnv ?? {},
...(opts.runtimeEnv
? {
primaryRuntimeEnv: {
mode: 'runtime-overlay' as const,
overlayKeys: Object.keys(opts.runtimeEnv),
effectiveEnv: opts.runtimeEnv,
},
}
: {}),
},
);
return { workspace, scratch, bridgeEvents, app };
}
@@ -401,6 +419,29 @@ describe('POST /workspace/setup-github', () => {
);
});
it('uses the owning runtime environment for proxy fallback', async () => {
await teardown(h);
h = await makeHarness({
token: 'secret',
runtimeEnv: { HTTPS_PROXY: 'http://runtime-proxy.example:8080' },
daemonEnv: { HTTPS_PROXY: 'http://daemon-proxy.example:8080' },
});
setupGithubMocks.setupGithub.mockResolvedValueOnce(setupResult());
const res = await request(h.app)
.post('/workspace/setup-github')
.set('Host', loopbackHost())
.set('Authorization', 'Bearer secret')
.send({ consent: true });
expect(res.status).toBe(200);
expect(setupGithubMocks.setupGithub).toHaveBeenCalledWith(
expect.objectContaining({
proxy: 'http://runtime-proxy.example:8080',
}),
);
});
it('redacts setup-github filesystem paths from client errors', async () => {
setupGithubMocks.setupGithub.mockRejectedValueOnce(
new setupGithubMocks.SetupGithubError(
@@ -26,6 +26,7 @@ const ROUTE = 'POST /workspace/setup-github';
interface RegisterDeps {
boundWorkspace: string;
bridge: AcpSessionBridge;
env: Readonly<NodeJS.ProcessEnv>;
mutate: (opts?: { strict?: boolean }) => RequestHandler;
parseClientId: (req: Request, res: Response) => string | undefined | null;
safeBody: (req: Request) => Record<string, unknown>;
@@ -70,7 +71,7 @@ async function handleSetupGithub(
const result = await setupGithub({
cwd: deps.boundWorkspace,
workspaceRoot: deps.boundWorkspace,
proxy: resolveSetupGithubProxy(deps.boundWorkspace),
proxy: resolveSetupGithubProxy(deps.boundWorkspace, deps.env),
abortSignal: requestAbortSignal(req, res),
fileOps: createSetupGithubFileOps(factory, ROUTE, originatorClientId),
});
@@ -339,6 +340,7 @@ export function setupGithubEventData(
export function resolveSetupGithubProxy(
boundWorkspace: string,
env: Readonly<NodeJS.ProcessEnv>,
): string | undefined {
const settings = loadSettings(boundWorkspace, { skipLoadEnvironment: true });
const trustState = getWorkspaceTrustStatus(
@@ -353,10 +355,10 @@ export function resolveSetupGithubProxy(
settings.systemDefaults.settings.proxy;
return (
settingsProxy ||
process.env['HTTPS_PROXY'] ||
process.env['https_proxy'] ||
process.env['HTTP_PROXY'] ||
process.env['http_proxy']
env['HTTPS_PROXY'] ||
env['https_proxy'] ||
env['HTTP_PROXY'] ||
env['http_proxy']
);
}
+1
View File
@@ -4234,6 +4234,7 @@ export async function runQwenServe(
fsFactory: routeFsFactory,
primaryWorkspaceTrusted: trustedWorkspace,
primaryRuntimeEnv,
daemonEnv: daemonRuntimeBaseEnv,
daemonLog,
getChannelWorkerSnapshot,
getChannelWorkerSnapshots,
+6
View File
@@ -471,6 +471,7 @@ export interface ServeAppDeps {
workspaceRuntimeRemoval?: WorkspaceRuntimeRemovalController;
primaryWorkspaceTrusted?: boolean;
primaryRuntimeEnv?: WorkspaceRuntimeEnvMetadata;
daemonEnv?: Readonly<NodeJS.ProcessEnv>;
voiceTranscriber?: WorkspaceVoiceRouteDeps['transcribe'];
voiceCoordinator?: WorkspaceVoiceCoordinator;
}
@@ -890,6 +891,9 @@ export function createServeApp(
(app.locals as { workspaceRegistry?: WorkspaceRegistry }).workspaceRegistry =
workspaceRegistry;
const primaryRuntime = workspaceRegistry.primary;
const daemonEnv = deps.daemonEnv ?? process.env;
const primaryRuntimeEffectiveEnv =
getRuntimeEffectiveEnv(primaryRuntime.env) ?? daemonEnv;
const voiceCoordinator =
deps.voiceCoordinator ?? new WorkspaceVoiceCoordinator();
const primaryBoundWorkspace = primaryRuntime.workspaceCwd;
@@ -1248,6 +1252,7 @@ export function createServeApp(
registerWorkspaceSetupGithubRoutes(app, {
boundWorkspace: primaryBoundWorkspace,
bridge: primaryBridge,
env: primaryRuntimeEffectiveEnv,
mutate,
parseClientId: parseClientIdHeader,
safeBody,
@@ -1676,6 +1681,7 @@ export function createServeApp(
// route through the JSON error contract below.
acpHandleRef.current = mountAcpHttp(app, primaryBridge, {
boundWorkspace: primaryBoundWorkspace,
daemonEnv,
// Phase 4 (issue #6378): pass the registry so `/workspaces/:workspace/acp`
// mounts a per-runtime ACP dispatcher for each registered workspace.
workspaceRegistry,