fix(a2a-server): enforce workspace trust and task isolation to prevent RCE#28470
fix(a2a-server): enforce workspace trust and task isolation to prevent RCE#28470luisfelipe-alt wants to merge 2 commits into
Conversation
|
📊 PR Size: size/XL
|
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 addresses a critical Remote Code Execution (RCE) vulnerability in the a2a-server backend. It introduces robust task-level isolation for environment variables and working directories, ensuring that untrusted workspaces cannot influence the server's configuration or access sensitive credentials. The changes also harden the startup sequence and improve cross-platform stability by ensuring proper path resolution and environment handling. 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 the 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 counterproductive. 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
|
🛑 Action Required: Evaluation ApprovalSteering changes have been detected in this PR. To prevent regressions, a maintainer must approve the evaluation run before this PR can be merged. Maintainers:
Once approved, the evaluation results will be posted here automatically. |
There was a problem hiding this comment.
Code Review
This pull request implements workspace isolation and secure environment variable handling to mitigate remote code execution vulnerabilities. It introduces task-specific environment and working directory isolation using AsyncLocalStorage and a Proxy on process.env. It also restricts .env loading in untrusted workspaces, passes isolated environments to core services (such as shell execution and content generation), and disables external editor spawning in headless mode. The single review comment was identified as a false positive and removed, as the referenced parameter is present in the code but fell outside the diff context. Consequently, there is no further feedback to provide.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces workspace and environment isolation for tasks in the agent executor to mitigate potential Remote Code Execution (RCE) vulnerabilities. It implements task-specific environment variable isolation using AsyncLocalStorage and a Proxy on process.env, and monkey-patches process.cwd and process.chdir to prevent cross-task interference. It also restricts environment loading and extension loading in untrusted workspaces, disables external editor spawning in headless mode, and updates core services (like shell execution and content generation) to respect the isolated task environments. The review feedback suggests trimming the workspace path string before checking for emptiness in validateWorkspacePath to prevent whitespace-only values from being accepted.
4b4166e to
f0894ec
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces task-level environment and workspace isolation to mitigate potential remote code execution (RCE) vulnerabilities in the agent-to-agent server. It implements an AsyncLocalStorage-backed proxy for process.env and monkey-patches process.cwd and process.chdir to isolate environment variables and working directories per task. Additionally, untrusted workspaces are restricted from loading workspace-level .env files or extensions, and external editor spawning is disabled in headless/server mode. Corresponding unit tests have been added to verify these security mitigations. I have no further feedback to provide as there are no review comments.
Note: Security Review did not run due to the size of the PR.
Summary
This PR resolves a critical security vulnerability in the
a2a-serverbackend that allowed for zero-click Remote Code Execution (RCE) and environment poisoning in untrusted workspaces.By refactoring the startup sequence, environment loading mechanism, and introducing robust task-level environment and process isolation in
a2a-serverusingAsyncLocalStorageand aProxyonprocess.env, we ensure thatworkspace-level environment files (
.envand.gemini/.env) are completely ignored unless the workspace is explicitly trusted by the user. This aligns thea2a-serverbackend's security model with the existing secure implementationin the CLI frontend.
Details
loadEnvironment()inpackages/a2a-server/src/http/app.ts(createApp) andpackages/a2a-server/src/agent/executor.ts(getConfig) until afterworkspace trust is evaluated (
checkPathTrust/setIsTrusted).GEMINI_CLI_TRUST_WORKSPACE=trueinside a malicious
.gemini/.envfile to self-validate their ownuntrusted workspace before trust is checked.
loadEnvironment(isTrusted)inpackages/a2a-server/src/config/config.tsto accept the trust state.isTrustedisfalse, workspace-level environment files (both.envand
.gemini/.env) are completely ignored. Instead, the loader only loadsenvironment variables from the user's trusted home directory (e.g.,
~/.gemini/.envor~/.env). This is a safer and more secure approachthat completely isolates untrusted workspaces from environment loading.
AsyncLocalStorage(
envStorage) and aProxyonprocess.envto intercept reads, writes,deletions, and key enumerations, isolating environment variables per task
and preventing cross-task credential leakages.
process.cwdandprocess.chdirusingAsyncLocalStorageto simulate workspace isolation in a concurrent server.
envProxy:SymbolslikeSymbol.toStringTagor
util.inspect.custom) directly to the originalprocess.envtargetobject. This prevents breaking standard Node.js utilities (like
util.inspect)or third-party libraries that query symbols on
process.env.definePropertyTrap inenvProxy:definePropertytrap inenvProxyto interceptObject.defineProperty(process.env, ...)calls, preventing dependenciesor internal modules (such as test stubs) from bypassing the proxy's
settrap and polluting the global environment.
executeinrunInIsolatedEnvso that the entire agent loop runs with the correct task-isolated environment
and working directory, completely mitigating concurrency race conditions and
cross-task environment pollution.
execute:agentSettings(which includes the synchronouscall to
validateWorkspacePath) in atry-catchblock, logging the errorand notifying the client via the
eventBususingpushTaskStateFailedtoprevent unhandled rejections or server crashes.
.catchblock to therunInIsolatedEnvpromise to handle setuprejections robustly and prevent client hangs.
process.chdir:process.chdirto catchENOENTerrors fromfs.statSyncand throw a new Error with the correct native-like messageformat, ensuring 100% compatibility with Node's native
process.chdirerror behavior.
vi.stubEnvcalls in tests to use an empty string''instead of
undefinedto unset environment variables, and updated theassertions to use
toBeFalsy()to match.process.chdirwas removed fromsetTargetDirto preventconcurrent race conditions in the server,
process.cwd()no longer pointsto the active workspace directory.
packages/core/src/safety/checker-runner.tsto explicitly set thecwdof the spawned safety checker process to the active workspacedirectory (
this.contextBuilder.config.getWorkingDir()), ensuring thatexternal safety checkers run in the correct context.
validateWorkspacePathinpackages/a2a-server/src/agent/executor.tsto be synchronous since itcontains no asynchronous operations (both
process.cwd()andresolveToRealPathare synchronous).awaitkeywords where it is invoked, reducingmicrotask queue overhead and simplifying the code.
loadEnvironmentandfindEnvFileinpackages/a2a-server/src/config/config.tsto be asynchronous, usingfs.promises.accessandfs.promises.readFileto avoid blocking theevent loop in high-concurrency server environments.
executor.ts,config.ts,app.ts, andrce_vulnerability.test.ts) to correctlyawaitthe asynchronousloadEnvironmentfunction.oldPath: stringparameter back to theopenDifffunction signature in
packages/core/src/utils/editor.tsto prevent acritical runtime
ReferenceErrorwhengetDiffCommand(oldPath, newPath, editor)is called.setTargetDirinpackages/a2a-server/src/config/config.tstodefault
allowedRootto the user's home directory (homedir()) insteadof the server's startup directory (
originalCWD) whenCODER_AGENT_ALLOWED_ROOTis not configured.a2a-serverwhen runglobally or as a background service.
GOOGLE_APPLICATION_CREDENTIALS,GOOGLE_CLOUD_PROJECT, andGEMINI_CLI_USE_COMPUTE_ADCto theallowedServerKeysarray inpackages/a2a-server/src/http/app.ts.variables are correctly propagated to
process.envduring server startup,preventing authentication failures.
Related Issues
How to Validate (Cross-Environment Testing)
npm run test:ci)with
LC_ALL=en_US.UTF-8to force the English locale, resolving theyargs translation issue.
@google/gemini-cli-a2a-serverpasssuccessfully.
npm run lint) and TypeScript compilation(
npm run typecheck) pass with 0 errors or warnings.test-check.shrunssuccessfully and all tests pass.
path.parse(resolvedPath).rootinstead of'/'whenisTestEnvis true)to ensure complete compatibility on Windows.
Pre-Merge Checklist