-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Fix/pending tools and trust overrides #27854
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
04f8be5
fd0daae
16c6618
b1a1e14
32f9334
66e16be
f316070
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -34,6 +34,8 @@ import { logger } from '../utils/logger.js'; | |||||||||||||||||||||||||||||||||
| import type { Settings } from './settings.js'; | ||||||||||||||||||||||||||||||||||
| import { type AgentSettings, CoderAgentEvent } from '../types.js'; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const INITIAL_FOLDER_TRUST = process.env['GEMINI_FOLDER_TRUST']; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export async function loadConfig( | ||||||||||||||||||||||||||||||||||
| settings: Settings, | ||||||||||||||||||||||||||||||||||
| extensionLoader: ExtensionLoader, | ||||||||||||||||||||||||||||||||||
|
|
@@ -182,6 +184,15 @@ export async function loadConfig( | |||||||||||||||||||||||||||||||||
| return config; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export function setIsTrusted( | ||||||||||||||||||||||||||||||||||
| agentSettings: AgentSettings | undefined, | ||||||||||||||||||||||||||||||||||
| ): boolean { | ||||||||||||||||||||||||||||||||||
| if (INITIAL_FOLDER_TRUST !== undefined) { | ||||||||||||||||||||||||||||||||||
| return INITIAL_FOLDER_TRUST === 'true'; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| return !!agentSettings?.isTrusted; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+187
to
+194
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
References
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| export function setTargetDir(agentSettings: AgentSettings | undefined): string { | ||||||||||||||||||||||||||||||||||
| const originalCWD = process.cwd(); | ||||||||||||||||||||||||||||||||||
| const targetDir = | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,10 @@ import { | |
| type ScheduledToolCall, | ||
| } from './types.js'; | ||
| import { ToolErrorType } from '../tools/tool-error.js'; | ||
| import { UPDATE_TOPIC_TOOL_NAME } from '../tools/tool-names.js'; | ||
| import { | ||
| UPDATE_TOPIC_TOOL_NAME, | ||
| EDIT_TOOL_NAMES, | ||
| } from '../tools/tool-names.js'; | ||
|
Comment on lines
+29
to
+32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| import { PolicyDecision, type ApprovalMode } from '../policy/types.js'; | ||
| import { | ||
| ToolConfirmationOutcome, | ||
|
|
@@ -548,7 +551,10 @@ export class Scheduler { | |
|
|
||
| private _isParallelizable(request: ToolCallRequestInfo): boolean { | ||
| // update_topic tool is forced as sequential call | ||
| if (request.name === UPDATE_TOPIC_TOOL_NAME) { | ||
| if ( | ||
| request.name === UPDATE_TOPIC_TOOL_NAME || | ||
| EDIT_TOOL_NAMES.has(request.name) | ||
| ) { | ||
| return false; | ||
| } | ||
|
Comment on lines
+554
to
559
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of hardcoding specific file-modifying tool names like if (
request.name === UPDATE_TOPIC_TOOL_NAME ||
EDIT_TOOL_NAMES.has(request.name)
) {
return false;
} |
||
| if (request.args) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Folder Trust Bypass via Workspace
.envEnvironment PollutionSeverity: critical
Sub-category: Broken Access Control / Environment Pollution
Description:
The
setIsTrustedfunction determines if a workspace folder is trusted by checkingprocess.env['GEMINI_FOLDER_TRUST']and falling back toagentSettings.isTrusted.However, in
CoderAgentExecutor.getConfig(inpackages/a2a-server/src/agent/executor.ts),loadEnvironment()is called immediately aftersetIsTrusted.loadEnvironment()loads environment variables from the workspace's.envfile and overridesprocess.envglobally usingdotenv.config({ override: true }).If an untrusted workspace contains a
.envfile withGEMINI_FOLDER_TRUST=true, this value will be loaded intoprocess.envglobally. On any subsequent task execution or configuration resolution,setIsTrustedwill read this polluted environment variable and returntrue, completely bypassing the folder trust security mechanism.Since trusted folders allow the agent to execute powerful tools (such as running arbitrary shell commands) without user confirmation, this bypass directly enables Remote Code Execution (RCE) when an untrusted folder is opened.
Remediation:
Do not allow workspace-level
.envfiles to override system-level security environment variables likeGEMINI_FOLDER_TRUST. Alternatively, resolve the folder trust status using only the system's initial environment variables or the explicitagentSettings.isTrustedpassed by the client, and preventloadEnvironment()from overriding security-critical environment variables.References
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix this.