Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
9 changes: 9 additions & 0 deletions docs/design/2026-08-08-read-only-git-config-safety.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Read-only Git config safety

Issue #8575 proves two repository-local configuration paths that turn an otherwise read-only command into program execution: `diff.external` for `git diff`, and `core.fsmonitor` for `git status`.

The classifier will ask only for those reproduced command/config pairs. It will query effective local and worktree values through `git config --includes --show-scope`, so Git owns config syntax, include handling, precedence, and worktree behavior. Git probe and parse errors fail closed; a cwd that cannot be entered has no config execution path.

Commands that change directory before the relevant Git command also ask. The classifier will not simulate shell cwd state or resolve `git -C`; the latter is already outside the read-only allowlist.

Other execution-bearing Git settings are follow-up work only after an independent reproduction identifies the affected read-only subcommand.
9 changes: 8 additions & 1 deletion packages/core/src/core/plan-mode-shell-policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {
import { ToolConfirmationOutcome } from '../tools/tools.js';
import {
classifyShellCommandSafety,
classifyShellCommandSafetyInDirectory,
type ShellCommandSafety,
} from '../utils/shellAstParser.js';
import { normalizeMonitorCommand } from '../utils/shell-utils.js';
Expand Down Expand Up @@ -164,7 +165,13 @@ export async function evaluatePlanModeShellPolicy(input: {
let classification: ShellCommandSafety;
try {
classification = await raceWithAbort(
() => classifyShellCommandSafety(safetyCommand),
() =>
permissionContext.cwd
? classifyShellCommandSafetyInDirectory(
safetyCommand,
permissionContext.cwd,
)
: classifyShellCommandSafety(safetyCommand),
input.signal,
);
} catch (error) {
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/followup/speculation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ async function runSpeculativeLoop(
args,
state.overlayFs!,
approvalMode,
config.getTargetDir?.(),
);
Comment thread
yiliang114 marked this conversation as resolved.

if (gate.action === 'boundary') {
Expand Down
15 changes: 13 additions & 2 deletions packages/core/src/followup/speculationToolGate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@
*/

import { ToolNames } from '../tools/tool-names.js';
import { classifyShellCommandSafety } from '../utils/shellAstParser.js';
import {
classifyShellCommandSafety,
classifyShellCommandSafetyInDirectory,
} from '../utils/shellAstParser.js';
import { ApprovalMode } from '../config/config.js';
import { unescapePath, PATH_ARG_KEYS } from '../utils/paths.js';
import type { OverlayFs } from './overlayFs.js';
Expand Down Expand Up @@ -61,13 +64,15 @@ const BOUNDARY_TOOLS = new Set<string>([
* @param args - The tool call arguments
* @param overlayFs - The overlay filesystem for path rewriting
* @param approvalMode - The user's current approval mode
* @param cwd - Default execution directory for shell commands
* @returns Gate result: allow, redirect, or boundary
*/
export async function evaluateToolCall(
toolName: string,
args: Record<string, unknown>,
overlayFs: OverlayFs,
approvalMode: ApprovalMode,
cwd?: string,
): Promise<ToolGateResult> {
// Safe read-only tools — allow, but resolve paths through overlay
if (SAFE_READ_ONLY_TOOLS.has(toolName)) {
Expand Down Expand Up @@ -95,9 +100,15 @@ export async function evaluateToolCall(
// Shell — use AST parser for accurate read-only detection
if (toolName === ToolNames.SHELL) {
const command = typeof args['command'] === 'string' ? args['command'] : '';
const directory =
typeof args['directory'] === 'string' && args['directory']
? args['directory']
: cwd;
if (
command &&
(await classifyShellCommandSafety(command)) === 'read-only'
(await (directory
? classifyShellCommandSafetyInDirectory(command, directory)
: classifyShellCommandSafety(command))) === 'read-only'
) {
return { action: 'allow' };
}
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/memory/memory-scoped-agent-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type {
PermissionDecision,
} from '../permissions/types.js';
import { ToolNames } from '../tools/tool-names.js';
import { isShellCommandReadOnlyAST } from '../utils/shellAstParser.js';
import { isShellCommandReadOnlyASTInDirectory } from '../utils/shellAstParser.js';
import { stripShellWrapper } from '../utils/shell-utils.js';
import {
AUTO_MEMORY_PINNED_DIRNAME,
Expand Down Expand Up @@ -250,8 +250,9 @@ async function evaluateScopedDecision(
if (!opts.allowShell || !ctx.command) {
return 'deny';
}
const isReadOnly = await isShellCommandReadOnlyAST(
const isReadOnly = await isShellCommandReadOnlyASTInDirectory(
stripShellWrapper(ctx.command),
ctx.cwd ?? projectRoot,
);
return isReadOnly ? 'allow' : 'deny';
}
Expand Down
14 changes: 14 additions & 0 deletions packages/core/src/permissions/permission-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2083,6 +2083,20 @@ describe('PermissionManager', () => {
});

describe('compound command evaluation', () => {
it('keeps Git after a directory change in the confirmation boundary', async () => {
pm = new PermissionManager(
makeConfig({ permissionsAllow: ['Bash(cd *)'] }),
);
pm.initialize();
expect(
await pm.evaluate({
toolName: 'run_shell_command',
command: 'cd /tmp && git status',
cwd: process.cwd(),
}),
).toBe('ask');
});

it('all sub-commands allowed → allow', async () => {
pm = new PermissionManager(
makeConfig({
Expand Down
23 changes: 19 additions & 4 deletions packages/core/src/permissions/permission-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ import {
import type { PathMatchContext } from './rule-parser.js';
import { extractShellOperationsAcrossCommand } from './shell-semantics.js';
import type { ShellOperation } from './shell-semantics.js';
import { isShellCommandReadOnlyAST } from '../utils/shellAstParser.js';
import {
isShellCommandReadOnlyAST,
isShellCommandReadOnlyASTInDirectory,
} from '../utils/shellAstParser.js';
import { normalizeMonitorCommand } from '../utils/shell-utils.js';
import { createDebugLogger } from '../utils/debugLogger.js';
import {
Expand Down Expand Up @@ -233,7 +236,10 @@ export class PermissionManager {
SHELL_TOOL_NAMES.has(toolName) &&
command !== undefined
) {
bashDecision = await this.resolveDefaultPermission(command);
bashDecision = await this.resolveDefaultPermission(
command,
ctx.cwd ?? this.config.getCwd?.(),
);
}
}
} else {
Expand Down Expand Up @@ -449,6 +455,9 @@ export class PermissionManager {
};

let mostRestrictive: ResolvedDecision = 'allow';
const changesDirectory = subCommands.some((command) =>
/^\s*(?:cd|pushd)(?:\s|$)/.test(command),
);

for (const subCmd of subCommands) {
const subCtx: PermissionCheckContext = {
Expand All @@ -461,7 +470,10 @@ export class PermissionManager {
// (same logic as ShellToolInvocation.getDefaultPermission)
const decision: ResolvedDecision =
rawDecision === 'default'
? await this.resolveDefaultPermission(subCmd)
? await this.resolveDefaultPermission(
changesDirectory ? ctx.command! : subCmd,
ctx.cwd ?? this.config.getCwd?.(),
)
: (rawDecision as ResolvedDecision);

if (PRIORITY[decision] > PRIORITY[mostRestrictive]) {
Expand Down Expand Up @@ -495,9 +507,12 @@ export class PermissionManager {
*/
private async resolveDefaultPermission(
command: string,
cwd?: string,
): Promise<'allow' | 'ask'> {
try {
const isReadOnly = await isShellCommandReadOnlyAST(command);
const isReadOnly = cwd
? await isShellCommandReadOnlyASTInDirectory(command, cwd)
: await isShellCommandReadOnlyAST(command);
if (isReadOnly) {
return 'allow';
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/tools/monitor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ vi.mock('../utils/shell-utils.js', async (importOriginal) => {
const mockIsShellCommandReadOnlyAST = vi.hoisted(() => vi.fn());
const mockExtractCommandRules = vi.hoisted(() => vi.fn());
vi.mock('../utils/shellAstParser.js', () => ({
isShellCommandReadOnlyAST: mockIsShellCommandReadOnlyAST,
isShellCommandReadOnlyASTInDirectory: mockIsShellCommandReadOnlyAST,
extractCommandRules: mockExtractCommandRules,
}));

Expand Down
18 changes: 12 additions & 6 deletions packages/core/src/tools/monitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import {
import { MAX_CONCURRENT_MONITORS } from '../services/monitorRegistry.js';
import {
extractCommandRules,
isShellCommandReadOnlyAST,
isShellCommandReadOnlyASTInDirectory,
} from '../utils/shellAstParser.js';
import { getCurrentAgentId } from '../agents/runtime/agent-context.js';
import { getShellContextEnvVars } from '../utils/shellContextEnv.js';
Expand Down Expand Up @@ -171,9 +171,10 @@ class MonitorToolInvocation extends BaseToolInvocation<
}

override async getDefaultPermission(): Promise<PermissionDecision> {
const command = normalizeMonitorShellCommand(
this.params.command,
).safetyCommand;
const normalized = normalizeMonitorShellCommand(this.params.command);
const command = normalized.safetyCommand;
const cwd =
this.params.directory || this.config.getTargetDir?.() || process.cwd();

// Command substitution ($(), ``, <(), >()) is NOT a hard deny here —
// it falls through to 'ask' along with every other non-read-only
Expand All @@ -188,7 +189,10 @@ class MonitorToolInvocation extends BaseToolInvocation<
// Bash(...) — see comment in getConfirmationDetails); only the
// substitution-deny half is removed.
try {
const isReadOnly = await isShellCommandReadOnlyAST(command);
const isReadOnly = await isShellCommandReadOnlyASTInDirectory(
command,
cwd,
);
if (isReadOnly) {
return 'allow';
}
Expand All @@ -203,6 +207,8 @@ class MonitorToolInvocation extends BaseToolInvocation<
_abortSignal: AbortSignal,
): Promise<ToolCallConfirmationDetails> {
const normalized = normalizeMonitorShellCommand(this.params.command);
const cwd =
this.params.directory || this.config.getTargetDir?.() || process.cwd();
const subCommands = splitCommands(normalized.safetyCommand);
const confirmableSubCommands: string[] = [];

Expand All @@ -216,7 +222,7 @@ class MonitorToolInvocation extends BaseToolInvocation<
// permission boundary.
let isReadOnly = false;
try {
isReadOnly = await isShellCommandReadOnlyAST(sub);
isReadOnly = await isShellCommandReadOnlyASTInDirectory(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
9 changes: 6 additions & 3 deletions packages/core/src/tools/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ import { parse, type ControlOperator } from 'shell-quote';
import { createDebugLogger } from '../utils/debugLogger.js';
import { checkPriorRead, StructuredToolError } from './priorReadEnforcement.js';
import {
isShellCommandReadOnlyAST,
isShellCommandReadOnlyASTInDirectory,
extractCommandRules,
} from '../utils/shellAstParser.js';
import {
Expand Down Expand Up @@ -2040,7 +2040,10 @@ export class ShellToolInvocation extends BaseToolInvocation<

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

import { spawnSync } from 'node:child_process';
import { statSync } from 'node:fs';

interface LocalGitConfigRisk {
diffExternal: boolean;
fsmonitor: boolean;
}

const NO_RISK: LocalGitConfigRisk = {
diffExternal: false,
fsmonitor: false,
};
const PROBE_FAILED: LocalGitConfigRisk = {
diffExternal: true,
fsmonitor: true,
};

export function getLocalGitConfigRisk(cwd: string): LocalGitConfigRisk {
try {
if (!statSync(cwd).isDirectory()) return NO_RISK;
} catch {
return NO_RISK;
}

const result = spawnSync(
'git',
[
'-C',
cwd,
'config',
'--includes',
'--show-scope',
'--null',
'--get-regexp',
'^diff\\.external$|^core\\.fsmonitor$',
],
{
encoding: 'utf8',
maxBuffer: 64 * 1024,
timeout: 1000,
windowsHide: true,
},
);

if (result.status === 1) return NO_RISK;
if (result.status !== 0 || typeof result.stdout !== 'string') {
return PROBE_FAILED;
}

const effective = new Map<string, { scope: string; value: string }>();
const fields = result.stdout.split('\0');
for (let i = 0; i + 1 < fields.length; i += 2) {
const entry = fields[i + 1]!;
const newline = entry.indexOf('\n');
if (newline < 0) return PROBE_FAILED;
effective.set(entry.slice(0, newline), {
scope: fields[i]!,
value: entry.slice(newline + 1),
});
}

const localValue = (key: string): string | undefined => {
const entry = effective.get(key);
return entry && (entry.scope === 'local' || entry.scope === 'worktree')
? entry.value.trim()
: undefined;
};
const diffExternal = localValue('diff.external');
const fsmonitor = localValue('core.fsmonitor');

return {
diffExternal: diffExternal !== undefined && diffExternal !== '',
fsmonitor:
fsmonitor !== undefined &&
fsmonitor !== '' &&
!/^(?:true|false|yes|no|on|off|0|1)$/i.test(fsmonitor),
};
}
Loading
Loading