feat(security): LLM-suggested policy scoping for tool approvals#24722
feat(security): LLM-suggested policy scoping for tool approvals#24722genneth wants to merge 1 commit intogoogle-gemini:mainfrom
Conversation
When users approve a tool for session or permanent use, fire a background Gemini Flash Lite call to suggest a meaningful scoped policy rule (e.g., "read-only git commands") instead of the current heuristic that only captures the root command name. The suggested scope is shown directly in the approval question: Allow execution of [git diff, git log, git status] (read-only git)? Key design decisions: - Non-blocking: 5s timeout, falls back to heuristic if user confirms before the suggestion arrives - Bracket shows the exact scope (commandPrefix list or argsPattern regex), parentheses show the LLM's human-readable description - Skips edit confirmations (already scoped to specific files) - Gated behind security.enableSmartPolicyScoping (default: false) Implementation: - suggestion-generator.ts: calls Flash Lite with tool context, returns PolicySuggestion (commandPrefix, argsPattern, or toolName) - confirmation.ts: fires suggestion in background, publishes on message bus for UI, captures result for updatePolicy - policy.ts: prefers LLM suggestion over heuristic in handleStandardPolicyUpdate and handleMcpPolicyUpdate - ToolConfirmationMessage.tsx: subscribes to POLICY_SUGGESTION, updates bracket text when suggestion arrives - Telemetry: PolicySuggestionEvent logged via clearcut and OTel Closes google-gemini#21641.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the security tool approval flow by integrating LLM-based policy scoping. Instead of relying solely on heuristic root commands, the system now asynchronously generates context-aware policy suggestions that are displayed directly in the approval UI. This provides users with more transparent and granular control over tool permissions while maintaining a non-blocking, performant user experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces 'Smart Policy Scoping,' a feature that uses Gemini Flash Lite to suggest broader, reusable security policy rules when a user approves a tool invocation. The changes include documentation, configuration schema updates, and UI modifications to display suggestions. Core logic was implemented to generate suggestions in the background and apply them to the security policy. Feedback focuses on security vulnerabilities, specifically the risk of prompt injection from unsanitized tool context and the necessity of validating LLM-suggested command prefixes before they are applied to the system's policy.
| function buildToolContext(details: SerializableConfirmationDetails): string { | ||
| switch (details.type) { | ||
| case 'exec': { | ||
| const lines = [`Command: ${details.command}`]; | ||
| if (details.rootCommand) { | ||
| lines.push(`Root command: ${details.rootCommand}`); | ||
| } | ||
| if (details.rootCommands && details.rootCommands.length > 0) { | ||
| lines.push( | ||
| `Root commands (parsed): ${details.rootCommands.join(', ')}`, | ||
| ); | ||
| } | ||
| if (details.commands && details.commands.length > 1) { | ||
| lines.push(`All commands:\n${details.commands.join('\n')}`); | ||
| } | ||
| return lines.join('\n'); | ||
| } | ||
| case 'edit': | ||
| return `File: ${details.fileName}\nPath: ${details.filePath}`; | ||
| case 'mcp': { | ||
| const lines = [ | ||
| `Server: ${details.serverName}`, | ||
| `Tool: ${details.toolName}`, | ||
| ]; | ||
| if (details.toolDescription) { | ||
| lines.push(`Description: ${details.toolDescription}`); | ||
| } | ||
| if (details.toolArgs) { | ||
| try { | ||
| lines.push(`Arguments: ${JSON.stringify(details.toolArgs)}`); | ||
| } catch { | ||
| // Skip unserializable args | ||
| } | ||
| } | ||
| return lines.join('\n'); | ||
| } | ||
| case 'info': | ||
| return `Prompt: ${details.prompt}${details.urls ? `\nURLs: ${details.urls.join(', ')}` : ''}`; | ||
| default: | ||
| return ''; | ||
| } | ||
| } |
There was a problem hiding this comment.
The buildToolContext function constructs a prompt segment from SerializableConfirmationDetails. The data in details can originate from an LLM-generated tool call, making it untrusted input for the policy suggestion LLM call. As per the general rules, data from LLM-driven tools should be sanitized before being injected into a system prompt. Please sanitize all externally-controlled strings used to build the tool_context by removing newlines and context-breaking characters like ']'.
function buildToolContext(details: SerializableConfirmationDetails): string {
const sanitize = (str: string) => str.replace(/[\n\r\[\]]/g, ' ');
switch (details.type) {
case 'exec': {
const lines = [`Command: ${sanitize(details.command)}`];
if (details.rootCommand) {
lines.push(`Root command: ${sanitize(details.rootCommand)}`);
}
if (details.rootCommands && details.rootCommands.length > 0) {
lines.push(
`Root commands (parsed): ${details.rootCommands.map(sanitize).join(', ')}`,
);
}
if (details.commands && details.commands.length > 1) {
lines.push(`All commands:\n${details.commands.map(sanitize).join('\n')}`);
}
return lines.join('\n');
}
case 'edit':
return `File: ${sanitize(details.fileName)}\nPath: ${sanitize(details.filePath)}`;
case 'mcp': {
const lines = [
`Server: ${sanitize(details.serverName)}`,
`Tool: ${sanitize(details.toolName)}`,
];
if (details.toolDescription) {
lines.push(`Description: ${sanitize(details.toolDescription)}`);
}
if (details.toolArgs) {
try {
lines.push(`Arguments: ${sanitize(JSON.stringify(details.toolArgs))}`);
} catch {
// Skip unserializable args
}
}
return lines.join('\n');
}
case 'info':
return `Prompt: ${sanitize(details.prompt)}${details.urls ? `\nURLs: ${details.urls.map(sanitize).join(', ')}` : ''}`;
default:
return '';
}
}References
- Sanitize data from LLM-driven tools before injecting it into a system prompt to prevent prompt injection. At a minimum, remove newlines and context-breaking characters (e.g., ']').
| text: safeTemplateReplace(SUGGESTION_PROMPT, { | ||
| tool_type: confirmationDetails.type, | ||
| tool_context: toolContext, | ||
| }), |
There was a problem hiding this comment.
The suggestPolicyScope function is vulnerable to prompt injection. It constructs an LLM prompt by directly embedding tool invocation context (commands, arguments, etc.) using safeTemplateReplace. Since this context originates from untrusted tool calls, an attacker can craft a malicious command to manipulate the LLM's behavior. To remediate this, implement sanitization for the toolContext before embedding it in the prompt. As per repository rules, at a minimum, remove newlines and context-breaking characters (e.g., ']'). Additionally, validate the LLM's output against the original tool invocation to ensure suggested prefixes are valid and restrictive.
References
- Sanitize data from LLM-driven tools before injecting it into a system prompt to prevent prompt injection. At a minimum, remove newlines and context-breaking characters (e.g., ']').
| if (policySuggestion?.commandPrefix) { | ||
| options.commandPrefix = policySuggestion.commandPrefix; | ||
| } else if ( | ||
| policySuggestion?.argsPattern && | ||
| isSafeRegExp(policySuggestion.argsPattern) | ||
| ) { | ||
| options.argsPattern = policySuggestion.argsPattern; |
There was a problem hiding this comment.
This code applies LLM-suggested commandPrefix and argsPattern to the system's security policy without sufficient validation. While argsPattern is checked for ReDoS safety, the commandPrefix is used directly. If the LLM is manipulated via prompt injection or hallucinations, it can suggest rules that significantly broaden the user's security policy. Validate the suggested policy rules before applying them. Ensure that commandPrefix values are restrictive and directly related to the current tool call.
Summary
meaningful scope (e.g.,
["git diff", "git log", "git status"]) instead ofthe heuristic root command
Allow execution of [git diff, git log, git status] (read-only git commands)?security.enableSmartPolicyScoping(default: false)Builds on the recent tool confirmation UI rework (#24376) which simplified the
shell/edit approval layout and made the bracket scope display a natural
integration point.
Addresses #18268.
Design
user confirms before suggestion arrives
parentheses show human-readable description from the LLM
telemetry infrastructure
Test plan
parse errors, ReDoS rejection, actionable scope validation
(commandPrefix, argsPattern, toolName, server-wide scope bypass)
main, unrelated)