Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5746424
feat(core): enhance shell command validation and add core tools allow…
galz10 Apr 20, 2026
a56f3f6
Merge branch 'main' into galzahavi/fix/coretools
DavidAPierce Apr 21, 2026
8e4882e
Merge branch 'main' into galzahavi/fix/coretools
galz10 Apr 21, 2026
8c7b837
addressed the failure in shell-substitution.test.ts
DavidAPierce Apr 21, 2026
cfbdbe6
Merge branch 'main' into galzahavi/fix/coretools
DavidAPierce Apr 21, 2026
d0391b3
Merge branch 'main' into galzahavi/fix/coretools
DavidAPierce Apr 21, 2026
ffd9f9f
test(core): mock getShellConfiguration in shell substitution tests
DavidAPierce Apr 21, 2026
d115821
Merge branch 'main' into galzahavi/fix/coretools
DavidAPierce Apr 22, 2026
579ebe0
fix failing unit test for windows.
DavidAPierce Apr 22, 2026
c2984f9
Merge branch 'main' into galzahavi/fix/coretools
DavidAPierce Apr 22, 2026
add751e
Modifying strategy around planning modes and fixing ask handling
kschaab Apr 23, 2026
ad6b710
Merge branch 'main' into galzahavi/fix/coretools
kschaab Apr 23, 2026
cc6d7a4
Merge branch 'main' into galzahavi/fix/coretools
DavidAPierce Apr 23, 2026
5331853
Add a deny policy for tool calls looser than explicetly allowed tool …
DavidAPierce Apr 23, 2026
1734a03
set aggregateDecision to ASK_USER if any result dictates asking the u…
DavidAPierce Apr 23, 2026
7bb4fab
Merge branch 'main' into galzahavi/fix/coretools
DavidAPierce Apr 23, 2026
ad56ab5
respect confirmation required setting when set by users in updated po…
DavidAPierce Apr 23, 2026
a5bd5f4
Merge remote-tracking branch 'refs/remotes/origin/galzahavi/fix/coret…
DavidAPierce Apr 23, 2026
4c6ec40
regenerate settings schema and update configuration.md to include too…
DavidAPierce Apr 23, 2026
2e21f85
add trusted workspace setting to evals
DavidAPierce Apr 23, 2026
bf768cb
set GEMINI_CLI_TRUST_WORKSPACE in evals test helper and revert yml ch…
ehedlund Apr 23, 2026
173d4d6
disable the memoryV2 experiment in eval tests since they don't play n…
DavidAPierce Apr 23, 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
6 changes: 6 additions & 0 deletions docs/reference/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -1467,6 +1467,12 @@ their corresponding top-level category object in your `settings.json` file.
- **Default:** `undefined`
- **Requires restart:** Yes

- **`tools.confirmationRequired`** (array):
- **Description:** Tool names that always require user confirmation. Takes
precedence over allowed tools and core tool allowlists.
- **Default:** `undefined`
- **Requires restart:** Yes

- **`tools.exclude`** (array):
- **Description:** Tool names to exclude from discovery.
- **Default:** `undefined`
Expand Down
55 changes: 55 additions & 0 deletions evals/save_memory.eval.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: rememberingFavoriteColor,
params: {
settings: {
experimental: { memoryV2: false },
},
},

prompt: `remember that my favorite color is blue.

Expand All @@ -40,6 +45,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: rememberingCommandRestrictions,
params: {
settings: {
experimental: { memoryV2: false },
},
},

prompt: `I don't want you to ever run npm commands.`,
assert: async (rig, result) => {
Expand All @@ -61,6 +71,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: rememberingWorkflow,
params: {
settings: {
experimental: { memoryV2: false },
},
},

prompt: `I want you to always lint after building.`,
assert: async (rig, result) => {
Expand All @@ -83,6 +98,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: ignoringTemporaryInformation,
params: {
settings: {
experimental: { memoryV2: false },
},
},

prompt: `I'm going to get a coffee.`,
assert: async (rig, result) => {
Expand All @@ -108,6 +128,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: rememberingPetName,
params: {
settings: {
experimental: { memoryV2: false },
},
},

prompt: `Please remember that my dog's name is Buddy.`,
assert: async (rig, result) => {
Expand All @@ -129,6 +154,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: rememberingCommandAlias,
params: {
settings: {
experimental: { memoryV2: false },
},
},

prompt: `When I say 'start server', you should run 'npm run dev'.`,
assert: async (rig, result) => {
Expand All @@ -151,6 +181,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: savingDbSchemaLocationAsProjectMemory,
params: {
settings: {
experimental: { memoryV2: false },
},
},
prompt: `The database schema for this workspace is located in \`db/schema.sql\`.`,
assert: async (rig, result) => {
const wasToolCalled = await rig.waitForToolCall(
Expand Down Expand Up @@ -180,6 +215,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: rememberingCodingStyle,
params: {
settings: {
experimental: { memoryV2: false },
},
},

prompt: `I prefer to use tabs instead of spaces for indentation.`,
assert: async (rig, result) => {
Expand All @@ -202,6 +242,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: savingBuildArtifactLocationAsProjectMemory,
params: {
settings: {
experimental: { memoryV2: false },
},
},
prompt: `In this workspace, build artifacts are stored in the \`dist/artifacts\` directory.`,
assert: async (rig, result) => {
const wasToolCalled = await rig.waitForToolCall(
Expand Down Expand Up @@ -231,6 +276,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: savingMainEntryPointAsProjectMemory,
params: {
settings: {
experimental: { memoryV2: false },
},
},
prompt: `The main entry point for this workspace is \`src/index.js\`.`,
assert: async (rig, result) => {
const wasToolCalled = await rig.waitForToolCall(
Expand Down Expand Up @@ -259,6 +309,11 @@ describe('save_memory', () => {
suiteName: 'default',
suiteType: 'behavioral',
name: rememberingBirthday,
params: {
settings: {
experimental: { memoryV2: false },
},
},

prompt: `My birthday is on June 15th.`,
assert: async (rig, result) => {
Expand Down
1 change: 1 addition & 0 deletions evals/test-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ export async function internalEvalTest(evalCase: EvalCase) {
timeout: evalCase.timeout,
env: {
GEMINI_CLI_ACTIVITY_LOG_TARGET: activityLogFile,
GEMINI_CLI_TRUST_WORKSPACE: 'true',
},
});

Expand Down
13 changes: 13 additions & 0 deletions packages/cli/src/config/settingsSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1667,6 +1667,19 @@ const SETTINGS_SCHEMA = {
showInDialog: false,
items: { type: 'string' },
},
confirmationRequired: {
type: 'array',
label: 'Confirmation Required',
category: 'Advanced',
requiresRestart: true,
default: undefined as string[] | undefined,
description: oneLine`
Tool names that always require user confirmation.
Takes precedence over allowed tools and core tool allowlists.
`,
showInDialog: false,
items: { type: 'string' },
},
exclude: {
type: 'array',
label: 'Exclude Tools',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ exports[`InputPrompt > mouse interaction > should toggle paste expansion on doub
"
`;

exports[`InputPrompt > mouse interaction > should toggle paste expansion on double-click 4`] = `
"▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
> [Pasted Text: 10 lines]
▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
"
`;

exports[`InputPrompt > multiline rendering > should correctly render multiline input including blank lines 1`] = `
"────────────────────────────────────────────────────────────────────────────────────────────────────
> hello
Expand Down
97 changes: 87 additions & 10 deletions packages/core/src/policy/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ export const ADMIN_POLICY_TIER = 5;

export const MCP_EXCLUDED_PRIORITY = USER_POLICY_TIER + 0.9;
export const EXCLUDE_TOOLS_FLAG_PRIORITY = USER_POLICY_TIER + 0.4;
export const CONFIRMATION_REQUIRED_PRIORITY = USER_POLICY_TIER + 0.35;
export const ALLOWED_TOOLS_FLAG_PRIORITY = USER_POLICY_TIER + 0.3;
export const CORE_TOOLS_FLAG_PRIORITY = USER_POLICY_TIER + 0.25;
export const TRUSTED_MCP_SERVER_PRIORITY = USER_POLICY_TIER + 0.2;
export const ALLOWED_MCP_SERVER_PRIORITY = USER_POLICY_TIER + 0.1;

Expand Down Expand Up @@ -434,10 +436,21 @@ export async function createPolicyEngineConfig(
}
}

// Tools that are explicitly allowed in the settings.
// Priority: ALLOWED_TOOLS_FLAG_PRIORITY (user tier - explicit temporary allows)
if (settings.tools?.allowed) {
for (const tool of settings.tools.allowed) {
const nonPlanModes = [
ApprovalMode.DEFAULT,
ApprovalMode.AUTO_EDIT,
ApprovalMode.YOLO,
];

const mapToolsToRules = (
tools: string[],
priority: number,
source: string,
modes?: ApprovalMode[],
addDefaultDenyForTools = false,
) => {
const toolsWithNarrowing = new Set<string>();
for (const tool of tools) {
// Check for legacy format: toolName(args)
const match = tool.match(/^([a-zA-Z0-9_-]+)\((.*)\)$/);
if (match) {
Expand All @@ -449,15 +462,17 @@ export async function createPolicyEngineConfig(

// Treat args as a command prefix for shell tool
if (toolName === SHELL_TOOL_NAME) {
toolsWithNarrowing.add(toolName);
const patterns = buildArgsPatterns(undefined, args);
for (const pattern of patterns) {
if (pattern) {
rules.push({
toolName,
decision: PolicyDecision.ALLOW,
priority: ALLOWED_TOOLS_FLAG_PRIORITY,
priority,
argsPattern: new RegExp(pattern),
source: 'Settings (Tools Allowed)',
source,
modes,
});
}
}
Expand All @@ -467,8 +482,9 @@ export async function createPolicyEngineConfig(
rules.push({
toolName,
decision: PolicyDecision.ALLOW,
priority: ALLOWED_TOOLS_FLAG_PRIORITY,
source: 'Settings (Tools Allowed)',
priority,
source,
modes,
});
}
} else {
Expand All @@ -479,11 +495,70 @@ export async function createPolicyEngineConfig(
rules.push({
toolName,
decision: PolicyDecision.ALLOW,
priority: ALLOWED_TOOLS_FLAG_PRIORITY,
source: 'Settings (Tools Allowed)',
priority,
source,
modes,
});
}
}

if (addDefaultDenyForTools) {
for (const toolName of toolsWithNarrowing) {
rules.push({
toolName,
decision: PolicyDecision.DENY,
priority: priority - 0.01,
source: `${source} (Narrowing Enforcement)`,
modes,
});
}
}
};

// Tools that are explicitly allowed in the settings.
// Priority: ALLOWED_TOOLS_FLAG_PRIORITY (user tier - explicit temporary allows)
if (settings.tools?.allowed) {
mapToolsToRules(
settings.tools.allowed,
ALLOWED_TOOLS_FLAG_PRIORITY,
'Settings (Tools Allowed)',
undefined,
true,
);
}

// Tools that explicitly require confirmation in the settings.
// Priority: CONFIRMATION_REQUIRED_PRIORITY (overrides allowed and core)
if (settings.tools?.confirmationRequired) {
for (const tool of settings.tools.confirmationRequired) {
rules.push({
toolName: SHELL_TOOL_NAMES.includes(tool) ? SHELL_TOOL_NAME : tool,
decision: PolicyDecision.ASK_USER,
priority: CONFIRMATION_REQUIRED_PRIORITY,
source: 'Settings (Confirmation Required)',
});
}
}

// Core tools that are restricted in the settings.
// Priority: CORE_TOOLS_FLAG_PRIORITY (user tier - core tool allowlist)
if (settings.tools?.core) {
mapToolsToRules(
settings.tools.core,
CORE_TOOLS_FLAG_PRIORITY,
'Settings (Core Tools)',
nonPlanModes,
);

// If core tools are restricted, we should add a default DENY rule for everything else
// at a slightly lower priority than the explicit allows.
rules.push({
toolName: '*',
decision: PolicyDecision.DENY,
priority: CORE_TOOLS_FLAG_PRIORITY - 0.01,
source: 'Settings (Core Tools Allowlist Enforcement)',
modes: nonPlanModes,
});
Comment on lines +555 to +561
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The implementation currently defaults to a DENY decision when tools.core is configured. Based on the repository rules, the default policy decision for tool calls should be ALLOW until an approval mechanism is in place, as a DENY default is considered too restrictive. Please update the decision to ALLOW and ensure the priority is correctly set to avoid unintentionally overriding other policies.

Suggested change
rules.push({
toolName: '*',
decision: PolicyDecision.DENY,
priority: CORE_TOOLS_FLAG_PRIORITY - 0.01,
source: 'Settings (Core Tools Allowlist Enforcement)',
});
rules.push({
toolName: '*',
decision: PolicyDecision.ALLOW,
priority: USER_POLICY_TIER + 0.05,
source: 'Settings (Core Tools Allowlist Enforcement)',
});
References
  1. The default policy decision for tool calls should be ALLOW until a mechanism for wiring up approvals is integrated, as a DENY default would be too restrictive without an approval flow.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems to be ignoring the secure by default methodology being implemented when core tools are restricted. I don't think that this is a valid concern from the bot's part, but if you think this is correct, feel free to address the above comment.

}

// MCP servers that are trusted in the settings.
Expand All @@ -501,6 +576,7 @@ export async function createPolicyEngineConfig(
decision: PolicyDecision.ALLOW,
priority: TRUSTED_MCP_SERVER_PRIORITY,
source: 'Settings (MCP Trusted)',
modes: nonPlanModes,
});
}
}
Expand All @@ -519,6 +595,7 @@ export async function createPolicyEngineConfig(
decision: PolicyDecision.ALLOW,
priority: ALLOWED_MCP_SERVER_PRIORITY,
source: 'Settings (MCP Allowed)',
modes: nonPlanModes,
});
}
}
Expand Down
Loading
Loading