fix(cli): don't let an unreadable .env (EACCES) break extension loading#28059
fix(cli): don't let an unreadable .env (EACCES) break extension loading#28059manumishra12 wants to merge 3 commits into
Conversation
setUpCloudShellEnvironment read the resolved .env with an unguarded fs.readFileSync. If the file exists but cannot be read (e.g. EACCES under a sandbox), the error propagated and crashed early startup — which cascades into downstream failures such as the extension system failing to load. Wrap the read/parse in try/catch and fall back to the Cloud Shell default, mirroring the resilient handling already used in loadEnvironment. Adds a regression test for the EACCES case (which fails without this change). Related to google-gemini#27894
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 improves the stability of the CLI startup process within Cloud Shell environments. By handling potential file access errors gracefully, it prevents the application from crashing when it encounters an unreadable .env file, ensuring a more resilient startup sequence that aligns with existing error handling patterns in the codebase. 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
|
|
📊 PR Size: size/M
|
There was a problem hiding this comment.
Code Review
This pull request introduces error handling when reading and parsing the .env file in Cloud Shell environments to prevent startup crashes when the file is unreadable (e.g., due to EACCES permissions issues), along with a corresponding unit test. Feedback on this PR highlights a security vulnerability where the GOOGLE_CLOUD_PROJECT environment variable from an untrusted workspace is only sanitized if sandboxing is enabled. It is recommended to always sanitize this variable when the workspace is untrusted to prevent potential command injection.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This addresses the actual root cause of google-gemini#27894 (the extension system failing to load when a workspace .env is unreadable), as identified in the issue's effort analysis: - getScopedEnvContents (extensionSettings.ts) read the extension settings `.env` with an unguarded fsSync.readFileSync. If the file exists but is unreadable (EACCES under a sandbox), the error propagated and aborted the entire extension build. Wrap the stat/read/parse in try/catch and skip the file on error, mirroring the resilient handling in loadEnvironment(). - _buildExtension (extension-manager.ts) now respects `advanced.ignoreLocalEnv` and skips the workspace-local .env read entirely when it is set, honoring the user's intent and avoiding the inaccessible file altogether. Adds a regression test that makes the workspace .env unreadable (chmod 000) and asserts extension settings still resolve without throwing (fails without the getScopedEnvContents fix).
Summary
Fixes #27894 — the extension system failing to load when a workspace
.envis unreadable (EACCES under a sandbox) — at the root cause identified in the issue's effort analysis, plus a related hardening on the Cloud Shell path.Root-cause fix (extension loading)
getScopedEnvContents(extensionSettings.ts) read the extension settings.envwith an unguardedfsSync.readFileSync. If the file exists but is unreadable (EACCES), the error propagated and aborted the entire extension build (Skipping extension … EACCES … No extensions installed). Now the stat/read/parse is wrapped intry/catch, the unreadable file is skipped, and resolution continues (keychain secrets etc.) — mirroring the resilient handling inloadEnvironment()._buildExtension(extension-manager.ts) now respectsadvanced.ignoreLocalEnv: when set, it skips the workspace-local.envread entirely, honoring the user's intent and avoiding the inaccessible file (exactly what the issue asks for).Related hardening (Cloud Shell)
setUpCloudShellEnvironment(settings.ts) had the same unguarded-read pattern; wrapped it intry/catchtoo so an unreadable.envcan't crash Cloud Shell startup.Tests
Added a regression test: makes the workspace
.envunreadable (chmod 000) and asserts extension settings still resolve without throwing — it fails without thegetScopedEnvContentsfix (realEACCES) and passes with it. FullextensionSettings+extension-managersuites pass (49 tests); prettier/eslint/typecheck clean.