Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
17fa6b6
fix(core): confirm read-only git commands when repo config executes p…
yiliang114 Aug 6, 2026
d95d980
Merge remote-tracking branch 'origin/main' into fix/8575-git-config-e…
yiliang114 Aug 6, 2026
3c46d35
fix(core): close two probe gaps from review of #8575
yiliang114 Aug 6, 2026
93c93b6
Merge remote-tracking branch 'origin/main' into fix/8575-git-config-e…
yiliang114 Aug 6, 2026
089c987
fix(core): honor Git worktree config semantics
yiliang114 Aug 6, 2026
ab63f6b
fix(core): fail closed on opaque git config constructs (#8575)
yiliang114 Aug 6, 2026
b1ff1e4
fix(core): track cd in git config probe; close filter/url bypasses (#…
yiliang114 Aug 6, 2026
615e225
Merge branch 'main' into fix/8575-git-config-exec-probe
qwen-code-dev-bot Aug 7, 2026
024689f
fix(core): provide getTargetDir in speculation test mocks (#8575)
qwen-code-dev-bot Aug 7, 2026
4200413
fix(core): harden git-config exec probe against cd-tracking bypasses …
qwen-code-dev-bot Aug 7, 2026
a6b5174
Merge branch 'main' into fix/8575-git-config-exec-probe
yiliang114 Aug 7, 2026
6bf04c4
fix(core): close round-2 review findings for git-config exec probe (#…
qwen-code-dev-bot Aug 7, 2026
6bd8c03
fix(core): flag deprecated dot-form git config sections in exec probe…
qwen-code-dev-bot Aug 7, 2026
874ba25
fix(core): probe core.hooksPath targets in git-config exec probe (#8645)
qwen-code-dev-bot Aug 7, 2026
c2fc8b3
fix(core): probe submodule storage configs in git-config exec probe (…
qwen-code-dev-bot Aug 8, 2026
bb6000c
fix(core): tighten git config safety checks
yiliang114 Aug 8, 2026
c04dd29
fix(core): address verification findings for git-config exec probe (#…
qwen-code-dev-bot Aug 8, 2026
313ef68
refactor(core): reset git config probe to issue scope
yiliang114 Aug 8, 2026
4d7193d
Merge remote-tracking branch 'origin/main' into automation/pr-8645-apply
yiliang114 Aug 8, 2026
8000faf
fix(core): use Git config semantics for read-only probes
yiliang114 Aug 8, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/core/src/memory/memory-scoped-agent-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ async function evaluateScopedDecision(
}
const isReadOnly = await isShellCommandReadOnlyAST(
stripShellWrapper(ctx.command),
ctx.cwd ? { cwd: ctx.cwd } : undefined,
);
return isReadOnly ? 'allow' : 'deny';
}
Expand Down
24 changes: 21 additions & 3 deletions packages/core/src/permissions/permission-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,10 @@ export class PermissionManager {
SHELL_TOOL_NAMES.has(toolName) &&
command !== undefined
) {
bashDecision = await this.resolveDefaultPermission(command);
bashDecision = await this.resolveDefaultPermission(
command,
this.probeCwd(ctx),
);
}
}
} else {
Expand Down Expand Up @@ -461,7 +464,7 @@ export class PermissionManager {
// (same logic as ShellToolInvocation.getDefaultPermission)
const decision: ResolvedDecision =
rawDecision === 'default'
? await this.resolveDefaultPermission(subCmd)
? await this.resolveDefaultPermission(subCmd, this.probeCwd(ctx))
: (rawDecision as ResolvedDecision);

if (PRIORITY[decision] > PRIORITY[mostRestrictive]) {
Expand Down Expand Up @@ -491,13 +494,19 @@ export class PermissionManager {
* "relevant" rules for the surrounding compound command.
*
* @param command - The shell command to analyze.
* @param cwd - Execution directory; lets the classifier downgrade git
* commands whose repository-local config executes programs (#8575).
* @returns 'allow' for read-only, 'ask' otherwise.
*/
private async resolveDefaultPermission(
command: string,
cwd?: string,
): Promise<'allow' | 'ask'> {
try {
const isReadOnly = await isShellCommandReadOnlyAST(command);
const isReadOnly = await isShellCommandReadOnlyAST(
command,
cwd ? { cwd } : undefined,
);
if (isReadOnly) {
return 'allow';
}
Expand All @@ -513,6 +522,15 @@ export class PermissionManager {
return 'ask';
}

/**
* Best-effort execution directory for the git-config probe (#8575).
* Returns `undefined` when unknown — the probe is skipped and the
* classifier keeps its text-only verdict.
*/
private probeCwd(ctx: PermissionCheckContext): string | undefined {
return ctx.cwd ?? this.config.getCwd?.();
}

private normalizePermissionContext(
ctx: PermissionCheckContext,
): PermissionCheckContext {
Expand Down
13 changes: 13 additions & 0 deletions packages/core/src/tools/monitor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,19 @@ describe('MonitorTool', () => {
await expect(invocation.getDefaultPermission()).resolves.toBe('allow');
});

it('passes the execution cwd to the read-only classifier (#8575)', async () => {
mockIsShellCommandReadOnlyAST.mockResolvedValueOnce(true);
const invocation = createInvocation({
command: 'git status',
});

await expect(invocation.getDefaultPermission()).resolves.toBe('allow');
expect(mockIsShellCommandReadOnlyAST).toHaveBeenCalledWith(
expect.any(String),
{ cwd: '/test/dir' },
);
});

it('surfaces a command-substitution warning via getConfirmationDetails (issue #4093)', async () => {
const invocation = createInvocation({
command: 'echo $(cat secret.txt)',
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/tools/monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ class MonitorToolInvocation extends BaseToolInvocation<
// Bash(...) — see comment in getConfirmationDetails); only the
// substitution-deny half is removed.
try {
const isReadOnly = await isShellCommandReadOnlyAST(command);
const cwd = this.params.directory || this.config.getTargetDir();
const isReadOnly = await isShellCommandReadOnlyAST(command, { cwd });
if (isReadOnly) {
return 'allow';
}
Expand All @@ -203,6 +204,7 @@ class MonitorToolInvocation extends BaseToolInvocation<
_abortSignal: AbortSignal,
): Promise<ToolCallConfirmationDetails> {
const normalized = normalizeMonitorShellCommand(this.params.command);
const cwd = this.params.directory || this.config.getTargetDir();
const subCommands = splitCommands(normalized.safetyCommand);
const confirmableSubCommands: string[] = [];

Expand All @@ -216,7 +218,7 @@ class MonitorToolInvocation extends BaseToolInvocation<
// permission boundary.
let isReadOnly = false;
try {
isReadOnly = await isShellCommandReadOnlyAST(sub);
isReadOnly = await isShellCommandReadOnlyAST(sub, { cwd });
} catch (e) {
// Conservative fallback: if AST analysis fails, keep the sub-command
// in the confirmation scope instead of accidentally dropping it.
Expand Down
30 changes: 30 additions & 0 deletions packages/core/src/tools/shell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ vi.mock('../utils/debugLogger.js', () => ({
vi.mock('fs');
vi.mock('os');
vi.mock('crypto');
const mockGitConfigMayExecutePrograms = vi.hoisted(() => vi.fn(() => false));
vi.mock('../utils/git-config-safety.js', () => ({
gitConfigMayExecutePrograms: mockGitConfigMayExecutePrograms,
}));

import { isCommandAllowed } from '../utils/shell-utils.js';
import {
Expand Down Expand Up @@ -6868,6 +6872,32 @@ describe('ShellTool', () => {
expect(permission).toBe('allow');
});

// Regression coverage for issue #8575: whitelisted read-only git
// sub-commands execute programs configured in the repository-local
// `.git/config` (diff.external, core.fsmonitor, pagers, credential/ssh
// helpers). When such keys are present the command must be confirmed
// instead of auto-approved.
it('asks for read-only git commands when repo config executes programs (#8575)', async () => {
mockGitConfigMayExecutePrograms.mockReturnValue(true);
const invocation = shellTool.build({
command: 'git status',
is_background: false,
});

expect(await invocation.getDefaultPermission()).toBe('ask');
expect(mockGitConfigMayExecutePrograms).toHaveBeenCalledWith('/test/dir');
});

it('still allows read-only git commands when repo config is clean', async () => {
mockGitConfigMayExecutePrograms.mockReturnValue(false);
const invocation = shellTool.build({
command: 'git status',
is_background: false,
});

expect(await invocation.getDefaultPermission()).toBe('allow');
});

// Regression coverage for PR #4386 round 6 (cid 3298521039): the
// env-prefix wrapper substitution bypass. `getDefaultPermission`
// calls `stripShellWrapper(this.params.command)` BEFORE the AST
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/tools/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2039,7 +2039,8 @@ export class ShellToolInvocation extends BaseToolInvocation<

// AST-based read-only detection
try {
const isReadOnly = await isShellCommandReadOnlyAST(command);
const cwd = this.params.directory || this.config.getTargetDir();
const isReadOnly = await isShellCommandReadOnlyAST(command, { cwd });
if (isReadOnly) {
return 'allow';
}
Expand Down Expand Up @@ -2113,7 +2114,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
for (const sub of subCommands) {
let isReadOnly = false;
try {
isReadOnly = await isShellCommandReadOnlyAST(sub);
isReadOnly = await isShellCommandReadOnlyAST(sub, { cwd });
} catch {
// conservative: treat unknown commands as requiring confirmation
}
Expand Down
193 changes: 193 additions & 0 deletions packages/core/src/utils/git-config-safety.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
/**
* @license
* Copyright 2026 Qwen
* SPDX-License-Identifier: Apache-2.0
*/

import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { gitConfigMayExecutePrograms } from './git-config-safety.js';

describe('gitConfigMayExecutePrograms', () => {
let root: string;

beforeEach(() => {
root = fs.mkdtempSync(path.join(os.tmpdir(), 'git-config-safety-'));
});

afterEach(() => {
fs.rmSync(root, { recursive: true, force: true });
});

function makeRepo(name: string, config = ''): string {
const repo = path.join(root, name);
fs.mkdirSync(path.join(repo, '.git'), { recursive: true });
if (config) {
fs.writeFileSync(path.join(repo, '.git', 'config'), config);
}
return repo;
}

it('returns false outside a git repository', () => {
expect(gitConfigMayExecutePrograms(root)).toBe(false);
});

it('returns false for an undefined cwd', () => {
expect(gitConfigMayExecutePrograms(undefined)).toBe(false);
});

it('returns false for a clean repo config (including nested cwd)', () => {
const repo = makeRepo(
'clean',
'[core]\n\trepositoryformatversion = 0\n\tbare = false\n[remote "origin"]\n\turl = https://example.com/repo.git\n\tfetch = +refs/heads/*:refs/remotes/origin/*\n',
);
expect(gitConfigMayExecutePrograms(repo)).toBe(false);

const nested = path.join(repo, 'src', 'deep');
fs.mkdirSync(nested, { recursive: true });
expect(gitConfigMayExecutePrograms(nested)).toBe(false);
});

it('returns false when .git/config does not exist', () => {
const repo = makeRepo('no-config');
expect(gitConfigMayExecutePrograms(repo)).toBe(false);
});

it.each([
['[diff]\n\texternal = /tmp/evil\n', 'diff.external'],
['[core]\n\tpager = less -R\n', 'core.pager'],
['[core]\n\tfsmonitor = /tmp/evil\n', 'core.fsmonitor'],
['[core]\n\taskpass = /tmp/evil\n', 'core.askpass'],
['[core]\n\tsshCommand = /tmp/evil\n', 'core.sshCommand'],
['[credential]\n\thelper = !/tmp/evil\n', 'credential.helper'],
['[gpg]\n\tprogram = /tmp/evil\n', 'gpg.program'],
] as Array<[string, string]>)(
'flags program-valued key %s',
(config, label) => {
const repo = makeRepo(label.replace(/\W+/g, '-'), config);
expect(gitConfigMayExecutePrograms(repo)).toBe(true);
},
);

it('is case-insensitive for section and key names', () => {
const repo = makeRepo('case', '[DIFF]\n\tEXTERNAL = /tmp/evil\n');
expect(gitConfigMayExecutePrograms(repo)).toBe(true);
});

it('handles quoted values', () => {
const repo = makeRepo(
'quoted',
'[core]\n\tpager = "delta --paging=never"\n',
);
expect(gitConfigMayExecutePrograms(repo)).toBe(true);
});

it.each([
['[pager]\n\tlog = delta\n', 'pager-cmd-override'],
['[diff "drv"]\n\ttextconv = /tmp/evil\n', 'diff-driver-textconv'],
[
'[credential "https://example.com"]\n\thelper = store\n',
'credential-url-helper',
],
['[gpg "ssh"]\n\tprogram = /tmp/evil\n', 'gpg-format-program'],
[
'[remote "origin"]\n\tproxy = nc -X 5 -x proxy:1080 %h %p\n',
'remote-proxy',
],
['[remote "origin"]\n\turl = ext::sh -c evil%% %S %u\n', 'remote-ext-url'],
] as Array<[string, string]>)('flags subsection key %s', (config, label) => {
const repo = makeRepo(label, config);
expect(gitConfigMayExecutePrograms(repo)).toBe(true);
});

it('does not flag core.fsmonitor booleans (built-in daemon / disabled)', () => {
const enabled = makeRepo('fsm-true', '[core]\n\tfsmonitor = true\n');
expect(gitConfigMayExecutePrograms(enabled)).toBe(false);
const disabled = makeRepo('fsm-false', '[core]\n\tfsmonitor = false\n');
expect(gitConfigMayExecutePrograms(disabled)).toBe(false);
});

it('does not flag empty values or non-executing keys', () => {
const repo = makeRepo(
'benign',
'[diff]\n\texternal =\n[pager]\n\tlog =\n[core]\n\teditor = vim\n[init]\n\tdefaultBranch = main\n',
);
expect(gitConfigMayExecutePrograms(repo)).toBe(false);
});

it('ignores comments', () => {
const repo = makeRepo(
'comments',
'# diff.external = /tmp/evil\n; pager.log = evil\n[core]\n',
);
expect(gitConfigMayExecutePrograms(repo)).toBe(false);
});

describe('linked worktrees and submodules', () => {
it('reads config.worktree and the common config via the .git file', () => {
const main = makeRepo('main-repo', '[core]\n\tbare = false\n');
const commonGitDir = path.join(main, '.git');

// Linked worktree: <wt>/.git is a file pointing into
// <main>/.git/worktrees/<name>.
const wtGitDir = path.join(commonGitDir, 'worktrees', 'wt');
fs.mkdirSync(wtGitDir, { recursive: true });
fs.writeFileSync(path.join(wtGitDir, 'commondir'), '../..\n');
const wt = path.join(root, 'wt-clean');
fs.mkdirSync(wt, { recursive: true });
fs.writeFileSync(path.join(wt, '.git'), `gitdir: ${wtGitDir}\n`);
expect(gitConfigMayExecutePrograms(wt)).toBe(false);

// Planted key in the per-worktree config.
fs.writeFileSync(
path.join(wtGitDir, 'config.worktree'),
'[core]\n\tpager = /tmp/evil\n',
);
expect(gitConfigMayExecutePrograms(wt)).toBe(true);

// Planted key in the common config instead.
fs.rmSync(path.join(wtGitDir, 'config.worktree'));
fs.writeFileSync(
path.join(commonGitDir, 'config'),
'[diff]\n\texternal = /tmp/evil\n',
);
expect(gitConfigMayExecutePrograms(wt)).toBe(true);
});

it('reads the gitdir target config for submodule-style .git files', () => {
const store = path.join(root, 'store', 'modules', 'sub');
fs.mkdirSync(store, { recursive: true });
fs.writeFileSync(
path.join(store, 'config'),
'[core]\n\tfsmonitor = /tmp/evil\n',
);
const sub = path.join(root, 'sub-checkout');
fs.mkdirSync(sub, { recursive: true });
fs.writeFileSync(path.join(sub, '.git'), `gitdir: ${store}\n`);
expect(gitConfigMayExecutePrograms(sub)).toBe(true);
});
});

it('fails closed when the config exists but cannot be read', () => {
const repo = path.join(root, 'unreadable');
fs.mkdirSync(path.join(repo, '.git', 'config'), { recursive: true });
// `.git/config` is a directory → readFileSync throws EISDIR.
expect(gitConfigMayExecutePrograms(repo)).toBe(true);
});

it('fails closed when the .git pointer file cannot be read', () => {
const repo = path.join(root, 'bad-pointer');
fs.mkdirSync(path.join(repo, '.git'), { recursive: true });
fs.rmdirSync(path.join(repo, '.git'));
fs.mkdirSync(path.join(repo, '.git.d'), { recursive: true });
fs.writeFileSync(path.join(repo, '.git'), 'gitdir: .git.d\n');
fs.chmodSync(path.join(repo, '.git'), 0o000);
try {
expect(gitConfigMayExecutePrograms(repo)).toBe(true);
} finally {
fs.chmodSync(path.join(repo, '.git'), 0o644);
}
});
});
Loading
Loading