From fe19387a0f685bde1f14e8dcb22fc2acafae63b4 Mon Sep 17 00:00:00 2001 From: yiliang114 Date: Wed, 4 Nov 2026 21:30:48 +0800 Subject: [PATCH] fix(cli): keep project hook approval env inside the workspace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Approved project hooks were silently withheld in the docker sandbox lane. The workspace trust walk admits an ancestor `.env`, the host hashed it into `hookApprovals.json`, and the container — which mounts only the workspace — recomputed the hash without it, so `getState()` returned `pending`. Narrow `readProjectEnvFiles` to files under the workspace root. `findEnvFiles` still discovers ancestors for the runtime environment, so only the approval hash input changes. Hooks in a workspace with no ancestor `.env` keep their current record, but every hash that covered an ancestor changes once, so those approvals reset to pending a single time. Fixes #16313 Co-authored-by: Qwen-Coder Patrol-Run: qwen-issue-patrol/jmw24pta9wt --- packages/cli/src/config/environment.test.ts | 114 ++++++++++++++++++++ packages/cli/src/config/environment.ts | 16 +++ 2 files changed, 130 insertions(+) diff --git a/packages/cli/src/config/environment.test.ts b/packages/cli/src/config/environment.test.ts index fe4443975b1..39c93fdf682 100644 --- a/packages/cli/src/config/environment.test.ts +++ b/packages/cli/src/config/environment.test.ts @@ -10,13 +10,20 @@ import * as path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { buildRuntimeEnvironment, + findEnvFiles, hasLoadedEnvironmentValues, isFileSourcedEnvKey, loadEnvironment, + readProjectEnvFiles, reloadEnvironment, resetEnvironmentTrackingForTesting, SETTINGS_DIRECTORY_NAME, } from './environment.js'; +import { + hashProjectHooks, + LoadedHookApprovals, + type ProjectHooksApproval, +} from './hook-approvals.js'; import { ENV_ACP_REPEATED_TOOL_FAILURE_GUARD, PRIVATE_RELAUNCH_ENV_PROVENANCE, @@ -2172,3 +2179,110 @@ describe('hasLoadedEnvironmentValues', () => { expect(hasLoadedEnvironmentValues()).toBe(true); }); }); + +describe('project hook approval env files stay inside the workspace', () => { + // #16313: the docker sandbox mounts only the workspace, so an ancestor .env + // the host hashed is absent in the container. Hashing it made the two sides + // disagree and silently withheld approved project hooks in the container. + function trustFolder(workspace: string, folder: string): void { + const trustedFoldersPath = path.join(workspace, 'trustedFolders.json'); + fs.writeFileSync( + trustedFoldersPath, + JSON.stringify({ [folder]: TrustLevel.TRUST_FOLDER }), + ); + process.env['QWEN_CODE_TRUSTED_FOLDERS_PATH'] = trustedFoldersPath; + resetTrustedFoldersForTesting(); + } + + it('keeps an approved record approved when the ancestor .env is absent', async () => { + const parent = makeWorkspace(); + const root = path.join(parent, 'workspace'); + fs.mkdirSync(root); + const ancestorEnv = path.join(parent, '.env'); + fs.writeFileSync(ancestorEnv, 'ANCESTOR_ONLY=host\n'); + const previousTrustedFoldersPath = + process.env['QWEN_CODE_TRUSTED_FOLDERS_PATH']; + trustFolder(parent, root); + + try { + const settings = testSettings({ + security: { folderTrust: { enabled: true } }, + }); + const hooks = { + Stop: [{ hooks: [{ type: 'command', command: 'true' }] }], + }; + const approval = (): ProjectHooksApproval => ({ + root, + hooks, + resolved: hooks, + env: { files: readProjectEnvFiles(settings, root), settings: null }, + }); + + // The host sees the ancestor .env, and approves the hooks with it hashed. + const hostApproval = approval(); + // Positive control: discovery still walks above the workspace for the + // runtime env — only the approval hash input is narrowed. + expect(findEnvFiles(settings, root)).toContain(ancestorEnv); + + const approvalsPath = path.join(parent, 'hookApprovals.json'); + fs.writeFileSync(approvalsPath, '{}'); + const loaded = new LoadedHookApprovals( + { path: approvalsPath, config: {} }, + [], + ); + await loaded.setState(hostApproval, 'approved'); + + // Container view: the mount point stops at the workspace, so the + // ancestor .env the host read does not exist there. + fs.rmSync(ancestorEnv); + const containerApproval = approval(); + expect(containerApproval.env.files).not.toHaveProperty(ancestorEnv); + expect(containerApproval.env.files).toEqual(hostApproval.env.files); + expect(loaded.getState(containerApproval)).toBe('approved'); + } finally { + if (previousTrustedFoldersPath === undefined) { + delete process.env['QWEN_CODE_TRUSTED_FOLDERS_PATH']; + } else { + process.env['QWEN_CODE_TRUSTED_FOLDERS_PATH'] = + previousTrustedFoldersPath; + } + resetTrustedFoldersForTesting(); + } + }); + + it('still hashes a .env inside the root and settings.env', () => { + const root = makeWorkspace(); + const previousTrustedFoldersPath = + process.env['QWEN_CODE_TRUSTED_FOLDERS_PATH']; + trustFolder(root, root); + + try { + const settings = testSettings({ + security: { folderTrust: { enabled: true } }, + }); + const hooks = { Stop: [] }; + const hashOf = ( + files: Record, + settingsEnv: unknown, + ): string => hashProjectHooks({ hooks, env: { files, settings: settingsEnv } }); + + const before = hashOf(readProjectEnvFiles(settings, root), null); + fs.writeFileSync(path.join(root, '.env'), 'INSIDE_ROOT=1\n'); + const after = hashOf(readProjectEnvFiles(settings, root), null); + + expect(Object.keys(readProjectEnvFiles(settings, root))).toContain( + path.join(root, '.env'), + ); + expect(after).not.toBe(before); + expect(hashOf({}, { INSIDE_ROOT: '1' })).not.toBe(hashOf({}, null)); + } finally { + if (previousTrustedFoldersPath === undefined) { + delete process.env['QWEN_CODE_TRUSTED_FOLDERS_PATH']; + } else { + process.env['QWEN_CODE_TRUSTED_FOLDERS_PATH'] = + previousTrustedFoldersPath; + } + resetTrustedFoldersForTesting(); + } + }); +}); diff --git a/packages/cli/src/config/environment.ts b/packages/cli/src/config/environment.ts index e68558f9243..c5ff38e12f7 100644 --- a/packages/cli/src/config/environment.ts +++ b/packages/cli/src/config/environment.ts @@ -552,9 +552,25 @@ export function readProjectEnvFiles( startDir: string, ): Record | null> { const userLevelPaths = getUserLevelEnvPaths(); + // `hashProjectHooks` hashes this map, and the sandbox mounts only the + // workspace: an ancestor `.env` the host can read does not exist inside the + // container, so hashing it made the container-side hash disagree and + // withheld hooks the host had approved. Keep the map to files the workspace + // itself owns; `findEnvFiles` still discovers ancestors for the runtime env. + let workspaceRoot = path.resolve(startDir); + try { + workspaceRoot = fs.realpathSync(workspaceRoot); + } catch { + // Match findEnvFiles(): use the resolved path when realpath is unavailable. + } + const isInsideWorkspace = (filePath: string): boolean => { + const relative = path.relative(workspaceRoot, path.resolve(filePath)); + return !relative.startsWith('..') && !path.isAbsolute(relative); + }; const files: Record | null> = {}; for (const file of findEnvFiles(settings, startDir, userLevelPaths)) { if (userLevelPaths.has(path.normalize(file))) continue; + if (!isInsideWorkspace(file)) continue; try { files[file] = dotenv.parse(fs.readFileSync(file, 'utf-8')); } catch {