mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
fix(cli): keep folder trust out of workspace reach
The project-hook gate reads security.folderTrust.enabled from merged settings, so a repository could enable the very flag that authorizes its own hooks and then reach any pre-existing TRUST_FOLDER or TRUST_PARENT rule for the directory. Restrict the key to the operator scopes, beside allowPrivateNetworkHooks, and pin both directions: a workspace can neither enable the gate nor disarm a user's setting. Also close the smaller review notes. The startup warning no longer blames trust when disableAllHooks removed the hook system entirely, and it now has coverage for the warned and both silent branches. Every hook fixture ran pre-trusted, so add an interactive integration case that declines trust and asserts the hooks never fire, the warning names the remedy, and the tool result still reaches the model unmarked. Drop the duplicated node:fs import and correct the docs claim that IDE trust decisions apply live to settings-file hooks. Refs #14746 Co-authored-by: Qwen-Coder <[email protected]>
This commit is contained in:
1 parent
53cee2c0e4
commit
a89935fa1e
8 files changed
+188
-11
No files matched your search
@@ -36,7 +36,7 @@ The browser never edits your configuration: to add, change or remove a hook, edi
|
||||
|
||||
Reloading requires this explicit menu-open action: saving a file, pulling changes or switching branches does not automatically arm new hook commands. The non-interactive `/hooks list` only displays the registry currently loaded by that process; it does not reload settings. In an interactive terminal, `/hooks list` opens the same menu as `/hooks`.
|
||||
|
||||
This reload covers hook definitions, not hook controls or HTTP security settings. Changes to `disableAllHooks`, `stopHookBlockingCap`, `security.allowedHttpHookUrls`, `security.allowPrivateNetworkHooks` and `security.folderTrust` (including trust-store entries) still require a restart; IDE workspace-trust decisions apply live. Hooks registered at runtime by skills or the SDK are not affected. In interactive sessions, project hooks from a project's `.qwen/settings.json` load only with **explicit folder trust**: enable `security.folderTrust.enabled` in user settings and trust the directory, or grant workspace trust through your IDE. A disabled folder-trust feature does not authorize them; when they are withheld the session shows a startup warning. In non-interactive sessions (`--prompt`/`-p`, piped stdin, ACP/SDK embeddings) folder trust is implicit — the caller chose the directory and there is no prompt surface — so project hooks load as before; only run non-interactive sessions in directories you trust. When hooks are turned off by `disableAllHooks`, `--safe-mode` or `--bare`, none load and the browser says so at the top.
|
||||
This reload covers hook definitions, not hook controls or HTTP security settings. Changes to `disableAllHooks`, `stopHookBlockingCap`, `security.allowedHttpHookUrls`, `security.allowPrivateNetworkHooks` and `security.folderTrust` (including trust-store entries) still require a restart. `security.folderTrust` is read from user, system and system-default settings only; a workspace file cannot enable it. Hooks registered at runtime by skills or the SDK are not affected. In interactive sessions, project hooks from a project's `.qwen/settings.json` load only with **explicit folder trust**: enable `security.folderTrust.enabled` in user settings and trust the directory, or grant workspace trust through your IDE. A disabled folder-trust feature does not authorize them; when they are withheld the session shows a startup warning. Settings-file hooks are registered once at startup, so an IDE trust change does not retroactively arm or silence them — reopen the `/hooks` menu or restart to apply it. In non-interactive sessions (`--prompt`/`-p`, piped stdin, ACP/SDK embeddings) folder trust is implicit — the caller chose the directory and there is no prompt surface — so project hooks load as before; only run non-interactive sessions in directories you trust. When hooks are turned off by `disableAllHooks`, `--safe-mode` or `--bare`, none load and the browser says so at the top.
|
||||
|
||||
### Agent frontmatter scope
|
||||
|
||||
@@ -1800,7 +1800,7 @@ sys.exit(0)
|
||||
|
||||
Prompt-hook inputs can be written to the session debug log, so apply appropriate access and retention controls.
|
||||
|
||||
6. **Edited during the session.** Hooks are read when the session starts. After editing hook definitions, open the interactive `/hooks` menu once to reload them, or restart Qwen Code. Changes to hook controls, HTTP security settings and `security.folderTrust` (including trust-store entries) require a restart; IDE workspace-trust decisions apply live.
|
||||
6. **Edited during the session.** Hooks are read when the session starts. After editing hook definitions, open the interactive `/hooks` menu once to reload them, or restart Qwen Code. Changes to hook controls, HTTP security settings and `security.folderTrust` (including trust-store entries) require a restart. Settings-file project hooks are also registered once, so a later IDE trust grant or revocation applies only after that reload or restart.
|
||||
|
||||
### Other checks
|
||||
|
||||
|
||||
@@ -270,6 +270,63 @@ describe('tool hook additionalContext delivery', () => {
|
||||
).toBe(1);
|
||||
});
|
||||
|
||||
it('interactive: withholds project hooks in an untrusted folder and says why', async () => {
|
||||
const server = await setup(
|
||||
'hook context interactive untrusted',
|
||||
'read_file',
|
||||
'note.txt',
|
||||
'call_untrusted',
|
||||
);
|
||||
// Every other fixture runs pre-trusted, so drop the rig's trust grant to
|
||||
// reach the withheld branch the gate exists for.
|
||||
writeFileSync(
|
||||
join(rig.testDir!, '.qwen-home', 'settings.json'),
|
||||
JSON.stringify({ security: { folderTrust: { enabled: true } } }),
|
||||
);
|
||||
writeFileSync(
|
||||
join(rig.testDir!, '.qwen-home', 'trustedFolders.json'),
|
||||
JSON.stringify({}),
|
||||
);
|
||||
const { ptyProcess } = rig.runInteractive(...fakeModelLaunchArgs(server));
|
||||
let output = '';
|
||||
ptyProcess.onData((data) => {
|
||||
output += data;
|
||||
});
|
||||
const waitFor = makeWaitFor(rig, () => output);
|
||||
// The pane wraps the warning, so match on a fragment inside one line.
|
||||
await waitFor('the folder-trust dialog', () =>
|
||||
output.includes('Do you trust this folder?'),
|
||||
);
|
||||
// The third option declines trust; esc alone leaves the dialog open.
|
||||
ptyProcess.write('\u001b[B');
|
||||
ptyProcess.write('\u001b[B');
|
||||
ptyProcess.write('\r');
|
||||
await waitFor('the composer after declining trust', () =>
|
||||
output.includes('Type your message'),
|
||||
);
|
||||
await waitFor('the withheld-hooks warning', () =>
|
||||
output.includes('Project hooks in .qwen/settings.json were not loaded'),
|
||||
);
|
||||
|
||||
const prompt = 'read the note';
|
||||
ptyProcess.write(prompt);
|
||||
await waitFor('the prompt to echo', () => output.includes(prompt));
|
||||
ptyProcess.write('\r');
|
||||
await waitFor('the follow-up model request', () =>
|
||||
server.requests.some(
|
||||
(request) =>
|
||||
request.body['stream'] === true &&
|
||||
JSON.stringify(request.body['messages']).includes('"role":"tool"'),
|
||||
),
|
||||
);
|
||||
await exitInteractive(ptyProcess, waitFor, () => output);
|
||||
|
||||
expect(hookHits()).toEqual([]);
|
||||
const result = toolResultFor(server, 'call_untrusted');
|
||||
expect(result).toContain('hello from note');
|
||||
expect(count(result, marker('PreToolUse', 'call_untrusted'))).toBe(0);
|
||||
}, 360_000);
|
||||
|
||||
it('interactive: delivers the first PreToolUse ask context once after approval', async () => {
|
||||
const server = await setup(
|
||||
'hook context interactive ask',
|
||||
|
||||
@@ -5,10 +5,9 @@
|
||||
*/
|
||||
|
||||
import { spawn } from 'node:child_process';
|
||||
import { mkdirSync, writeFileSync, readFileSync } from 'node:fs';
|
||||
import { mkdirSync, writeFileSync, readFileSync, realpathSync } from 'node:fs';
|
||||
import { rm } from 'node:fs/promises';
|
||||
import { join, dirname } from 'node:path';
|
||||
import { realpathSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
import { env } from 'node:process';
|
||||
import fs from 'node:fs';
|
||||
|
||||
@@ -1499,7 +1499,7 @@ describe('Settings Loading and Merging', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('should use folderTrust from workspace settings when trusted', () => {
|
||||
it('should ignore workspace folderTrust and keep the user value', () => {
|
||||
(mockFsExistsSync as Mock).mockReturnValue(true);
|
||||
const userSettingsContent = {
|
||||
security: {
|
||||
@@ -1511,7 +1511,7 @@ describe('Settings Loading and Merging', () => {
|
||||
const workspaceSettingsContent = {
|
||||
security: {
|
||||
folderTrust: {
|
||||
enabled: false, // This should be used
|
||||
enabled: false, // A repository must not disarm the gate.
|
||||
},
|
||||
},
|
||||
};
|
||||
@@ -1532,7 +1532,27 @@ describe('Settings Loading and Merging', () => {
|
||||
);
|
||||
|
||||
const settings = loadSettings(MOCK_WORKSPACE_DIR);
|
||||
expect(settings.merged.security?.folderTrust?.enabled).toBe(false); // Workspace setting should be used
|
||||
expect(settings.merged.security?.folderTrust?.enabled).toBe(true);
|
||||
});
|
||||
|
||||
it('should not let a workspace enable folderTrust for itself', () => {
|
||||
(mockFsExistsSync as Mock).mockReturnValue(true);
|
||||
(fs.readFileSync as Mock).mockImplementation(
|
||||
(p: fs.PathOrFileDescriptor) => {
|
||||
if (p === MOCK_WORKSPACE_SETTINGS_PATH)
|
||||
return JSON.stringify({
|
||||
security: { folderTrust: { enabled: true } },
|
||||
context: { fileName: 'WORKSPACE.md' },
|
||||
});
|
||||
return '{}';
|
||||
},
|
||||
);
|
||||
|
||||
const settings = loadSettings(MOCK_WORKSPACE_DIR);
|
||||
// Otherwise a repo could arm the gate that authorizes its own hooks and
|
||||
// reach any pre-existing TRUST_FOLDER/TRUST_PARENT rule for the directory.
|
||||
expect(settings.merged.security?.folderTrust?.enabled).toBeUndefined();
|
||||
expect(settings.merged.context?.fileName).toBe('WORKSPACE.md');
|
||||
});
|
||||
|
||||
it('should use system folderTrust over user setting', () => {
|
||||
@@ -4101,14 +4121,14 @@ describe('Settings Loading and Merging', () => {
|
||||
[MOCK_WORKSPACE_SETTINGS_PATH]: {
|
||||
security: {
|
||||
allowedHttpHookUrls: ['*'],
|
||||
folderTrust: { enabled: true },
|
||||
dnsResolutionOrder: 'ipv4first',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const settings = loadSettings(MOCK_WORKSPACE_DIR);
|
||||
expect(settings.merged.security?.allowedHttpHookUrls).toEqual(USER_LIST);
|
||||
expect(settings.merged.security?.folderTrust?.enabled).toBe(true);
|
||||
expect(settings.merged.security?.dnsResolutionOrder).toBe('ipv4first');
|
||||
});
|
||||
|
||||
it('drops the workspace whitelist entirely when the folder is untrusted', () => {
|
||||
|
||||
@@ -283,6 +283,8 @@ export const WORKSPACE_RESTRICTED_SETTINGS = [
|
||||
{ section: 'tools', key: 'executionSandbox' },
|
||||
{ section: 'tools', key: 'workflowsEnabled' },
|
||||
{ section: 'security', key: 'allowPrivateNetworkHooks' },
|
||||
// A repository must not switch on the gate that authorizes its own hooks.
|
||||
{ section: 'security', key: 'folderTrust' },
|
||||
{ section: 'security', key: 'allowedInsecureVoiceBaseUrls' },
|
||||
{ section: 'goals', key: 'modelProposed' },
|
||||
{ section: 'outboundCorrelation', key: 'allowDynamicHeaderValues' },
|
||||
|
||||
@@ -1558,6 +1558,101 @@ describe('AppContainer State Management', () => {
|
||||
}).not.toThrow();
|
||||
});
|
||||
|
||||
it.each([
|
||||
['the folder is explicitly trusted', { trusted: true }],
|
||||
['every hook is disabled', { trusted: false, disableAllHooks: true }],
|
||||
] as Array<[string, { trusted: boolean; disableAllHooks?: boolean }]>)(
|
||||
'does not warn about withheld project hooks when %s',
|
||||
async (_, state) => {
|
||||
const addItem = vi.fn();
|
||||
mockedUseHistory.mockReturnValue({
|
||||
history: [],
|
||||
addItem,
|
||||
updateItem: vi.fn(),
|
||||
clearItems: vi.fn(),
|
||||
loadHistory: vi.fn(),
|
||||
truncateToItem: vi.fn(),
|
||||
});
|
||||
vi.spyOn(mockConfig, 'isInteractive').mockReturnValue(true);
|
||||
vi.spyOn(mockConfig, 'isExplicitlyTrustedFolder').mockReturnValue(
|
||||
state.trusted,
|
||||
);
|
||||
vi.spyOn(mockConfig, 'getDisableAllHooks').mockReturnValue(
|
||||
state.disableAllHooks ?? false,
|
||||
);
|
||||
const settings = {
|
||||
...mockSettings,
|
||||
workspace: { settings: { hooks: { SessionStart: [] } } },
|
||||
} as unknown as LoadedSettings;
|
||||
|
||||
const { unmount } = render(
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
settings={settings}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>,
|
||||
);
|
||||
await act(async () => {});
|
||||
|
||||
expect(addItem).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
text: expect.stringContaining(
|
||||
'Project hooks in .qwen/settings.json',
|
||||
),
|
||||
}),
|
||||
expect.any(Number),
|
||||
);
|
||||
unmount();
|
||||
},
|
||||
);
|
||||
|
||||
it('warns once when an untrusted interactive session withholds project hooks', async () => {
|
||||
const addItem = vi.fn();
|
||||
mockedUseHistory.mockReturnValue({
|
||||
history: [],
|
||||
addItem,
|
||||
updateItem: vi.fn(),
|
||||
clearItems: vi.fn(),
|
||||
loadHistory: vi.fn(),
|
||||
truncateToItem: vi.fn(),
|
||||
});
|
||||
vi.spyOn(mockConfig, 'isInteractive').mockReturnValue(true);
|
||||
vi.spyOn(mockConfig, 'isExplicitlyTrustedFolder').mockReturnValue(false);
|
||||
vi.spyOn(mockConfig, 'getDisableAllHooks').mockReturnValue(false);
|
||||
const settings = {
|
||||
...mockSettings,
|
||||
workspace: {
|
||||
settings: {
|
||||
hooks: {
|
||||
SessionStart: [
|
||||
{ hooks: [{ type: 'command', command: 'echo x' }] },
|
||||
],
|
||||
},
|
||||
},
|
||||
},
|
||||
} as unknown as LoadedSettings;
|
||||
|
||||
const { unmount } = render(
|
||||
<AppContainer
|
||||
config={mockConfig}
|
||||
settings={settings}
|
||||
version="1.0.0"
|
||||
initializationResult={mockInitResult}
|
||||
/>,
|
||||
);
|
||||
await act(async () => {});
|
||||
|
||||
expect(addItem).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
type: MessageType.WARNING,
|
||||
text: expect.stringContaining('Project hooks in .qwen/settings.json'),
|
||||
}),
|
||||
expect.any(Number),
|
||||
);
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('renders with startup warnings', () => {
|
||||
const startupWarnings = ['Warning 1', 'Warning 2'];
|
||||
|
||||
|
||||
@@ -1984,7 +1984,9 @@ export const AppContainer = (props: AppContainerProps) => {
|
||||
!config.isInteractive() ||
|
||||
config.isExplicitlyTrustedFolder() ||
|
||||
config.getBareMode() ||
|
||||
config.isSafeMode()
|
||||
config.isSafeMode() ||
|
||||
// With every hook disabled the trust gate is not why they are absent.
|
||||
config.getDisableAllHooks()
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -11771,7 +11771,9 @@ export class Config {
|
||||
// disabled folder-trust feature must not silently authorize them.
|
||||
// Non-interactive invocations (`-p`, piped stdin, ACP/SDK embeddings)
|
||||
// treat trust as implicit — the caller chose the directory and there is
|
||||
// no prompt surface; only run them in directories you trust.
|
||||
// no prompt surface; only run them in directories you trust. An explicit
|
||||
// refusal is already enforced upstream: an untrusted workspace
|
||||
// contributes no settings, so it supplies no project hooks.
|
||||
if (this.isInteractive() && !this.isExplicitlyTrustedFolder()) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user