security: implement hard blocklist for workspace environment variables to prevent RCE

[galdawave]

This PR addresses a critical security vulnerability where malicious .env files in a workspace could override sensitive CLI and IDE environment variables, potentially leading to Remote Code Execution (RCE), addressing issue 25021.

Key Changes:

• Defined a RESTRICTED_WORKSPACE_ENV_VARS hard blocklist in settings.ts containing critical variables such as GEMINI_CLI_IDE_SERVER_STDIO_COMMAND, NODE_OPTIONS, PAGER, SHELL, etc.
• Updated loadEnvironment to strictly filter out these restricted variables when loading from workspace-level .env files, regardless of workspace trust or user settings.
• Implemented a proactive security warning that alerts the user when a workspace attempt to override a restricted variable is blocked.
• Added a regression test case in settings.test.ts to verify that malicious overrides are correctly blocked even in trusted workspaces.

Verification:

• Built the monorepo successfully.
• Verified that all settings and environment loading unit tests pass.
• Confirmed that attempting to set GEMINI_CLI_IDE_SERVER_STDIO_COMMAND in a local .env file results in a security warning and no environment modification.

[galdawave]

Code Review

Scope: Pull Request #33

This PR implements a hard blocklist for workspace-level .env files to prevent Remote Code Execution (RCE) vulnerabilities. It correctly identifies high-risk environment variables like NODE_OPTIONS, SHELL, and LD_PRELOAD, blocking them from being overridden by untrusted or malicious local .env files, even in trusted workspaces. The implementation includes proactive user feedback via security warnings and a regression test suite.

Metadata Review

• PR Title: Follows the established security: prefix convention used in the repository (e.g., google-gemini#19288, google-gemini#19026). It is clear and descriptive.
• PR Description: Excellent detail. It clearly maps the changes to the security requirement (issue 25021), lists the affected variables, and provides a clear verification plan.

Concerns (Action Required)

• [packages/cli/src/config/settings.ts]: Case-Sensitivity Bypass
  The check RESTRICTED_WORKSPACE_ENV_VARS.has(key) is case-sensitive. On case-insensitive operating systems (like Windows) or for processes that normalize environment variables, a malicious user could bypass this blocklist by using lowercase or mixed-case keys (e.g., node_options=--inspect or Pager=evil-script).

  • Suggestion: Normalize the key to uppercase before checking the set, and ensure the set contains only uppercase strings.

  if (RESTRICTED_WORKSPACE_ENV_VARS.has(key.toUpperCase())) { ... }

• [packages/cli/src/config/settings.ts]: Fragile Workspace Detection
  The logic const isProjectEnvFile = !envFilePath.includes(GEMINI_DIR); (where GEMINI_DIR is ".gemini") is fragile. A user could inadvertently bypass security protections by naming their project directory or any parent directory .gemini (e.g., ~/projects/my.gemini.tools/.env). This would cause isProjectEnvFile to be false, skipping the blocklist entirely.

  • Suggestion: Use a more robust check, such as comparing the resolved envFilePath against the resolved GEMINI_DIR path segment, or checking if the file resides within the user's home configuration directory.

• [packages/cli/src/config/settings.ts]: Missing Dangerous Variables
  The current blocklist is a great start, but several other critical variables commonly used for hijacking or RCE are missing:

  • LD_LIBRARY_PATH and DYLD_LIBRARY_PATH: Can be used to hijack shared library loading.
  • PATH: Shadowing system binaries (like git or ls) is a primary RCE vector. While blocking PATH may impact some legitimate use cases (e.g., adding node_modules/.bin), the security risk of allowing a workspace to redefine the CLI's own execution path is significant.
  • Suggestion: Add LD_LIBRARY_PATH and DYLD_LIBRARY_PATH to the blocklist. Consider adding PATH or at least auditing how it is used after environment loading.

• [package-lock.json]: Lockfile Noise/Artifacts
  The PR includes changes to package-lock.json that add "peer": true to various telemetry and gRPC packages. Since package.json was not modified, these appear to be artifacts of the developer's environment (e.g., using a different npm version or flags).

  • Suggestion: Revert these unrelated lockfile changes unless they are a required part of a dependency resolution fix.

Nits (Suggestions)

• [packages/cli/src/config/settings.ts]: Normalization of Blocklist
  Since the blocklist is intended to be static and performant, consider defining it as a Set<string> of uppercase strings and using a small helper function to check it, reducing boilerplate in loadEnvironment.

• [packages/cli/src/config/settings.test.ts]: Test Case Coverage
  The added test is good, but it should also verify the case-sensitivity concern mentioned above (e.g., ensuring node_options is also blocked).

[galdawave]

🤖 Automated Fix Applied

I've processed the latest review and implemented the requested changes in an isolated worktree.

• Changes have been committed and pushed to the PR branch.

[galdawave]

Closing PR as requested. Branch will be preserved.