docs: document all environment variables and enable CONFIG_DIR override#222
docs: document all environment variables and enable CONFIG_DIR override#222
Conversation
🦋 Changeset detectedLatest commit: 04e31e9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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 significantly improves the discoverability and configurability of the Highlights
Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request primarily focuses on improving documentation for environment variables and enabling the GOOGLE_WORKSPACE_CLI_CONFIG_DIR override in release builds. The documentation changes are thorough and well-structured. The code changes correctly enable the configuration override and fix a pre-existing mutability issue. However, I've identified a critical security vulnerability related to the new configuration override. The path from the environment variable is not validated, which could lead to path traversal attacks. My review includes a comment with a suggested fix to address this vulnerability.
jpoehnelt
left a comment
There was a problem hiding this comment.
Re: path traversal concern on GOOGLE_WORKSPACE_CLI_CONFIG_DIR — environment variables are trusted inputs set by the user (shell profile, .env, deployment config), not untrusted CLI arguments from AI agents. Validating against traversal/absolute paths would make the env var unusable (users need to set absolute paths like /home/user/.gws). This is consistent with XDG_CONFIG_HOME, CARGO_HOME, npm_config_cache, etc., none of which validate their values.
Updated AGENTS.md to explicitly document this trust boundary distinction.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request does a great job of documenting all the environment variables used by the CLI, which significantly improves usability and discoverability. Enabling the GOOGLE_WORKSPACE_CLI_CONFIG_DIR override in release builds is a useful feature for users who need to customize their configuration directory. The accompanying code cleanup to fix variable mutability is also a nice improvement.
I have one security-related suggestion in src/auth_commands.rs to add basic sanitization for the path provided via the environment variable to harden the CLI against potential null-byte injection issues. Otherwise, the changes look solid.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly improves the documentation for environment variables across .env.example, README.md, and AGENTS.md, and adds them to the CLI's help output. It also enables the GOOGLE_WORKSPACE_CLI_CONFIG_DIR override in release builds. While the documentation changes are excellent, enabling the configuration directory override without validation introduces a critical security vulnerability. My review includes a comment to address this issue.
| if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { | ||
| return PathBuf::from(dir); | ||
| } |
There was a problem hiding this comment.
While the AGENTS.md file was updated to state that environment variables are trusted inputs, using the GOOGLE_WORKSPACE_CLI_CONFIG_DIR variable without validation introduces a significant security risk. An attacker who can control the environment variables of the process running gws could set this variable to an arbitrary path. This could lead to:
- Credential Exfiltration: Pointing the config directory to a location the attacker can read (e.g.,
/tmp/gws-config) to steal credentials and tokens. - Impersonation/Privilege Escalation: Pointing to a directory containing attacker-controlled credentials.
Although other environment variables like LD_PRELOAD can be more dangerous, defense-in-depth is crucial, especially for a security-sensitive tool that handles credentials. The project already has path validation logic in src/validate.rs (as mentioned in AGENTS.md). This logic should be applied to the path provided by GOOGLE_WORKSPACE_CLI_CONFIG_DIR to prevent path traversal attacks (CWE-22).
| if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { | |
| return PathBuf::from(dir); | |
| } | |
| if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { | |
| if crate::validate::validate_safe_dir_path(&dir).is_ok() { | |
| return PathBuf::from(dir); | |
| } else { | |
| eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR value ('{}') is invalid or points to a restricted path. Falling back to default config directory.", dir); | |
| } | |
| } |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #222 +/- ##
==========================================
+ Coverage 57.56% 57.61% +0.04%
==========================================
Files 38 38
Lines 14213 14301 +88
==========================================
+ Hits 8182 8239 +57
- Misses 6031 6062 +31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Closes #171
Summary
Documents all 11 environment variables used by
gwsand enablesGOOGLE_WORKSPACE_CLI_CONFIG_DIRin release builds so users can override the config directory.Changes
.env.example— Expanded from 6 to 11 vars, organized by category with descriptionsREADME.md— Added consolidated "Environment Variables" reference section with link to.env.example; added to TOCAGENTS.md— Replaced bullet list with categorized tables covering all varssrc/main.rs— Added 5 missing vars togws --helpENVIRONMENT outputsrc/auth_commands.rs— Removed#[cfg(test)]guard fromGOOGLE_WORKSPACE_CLI_CONFIG_DIRso users can override the config directory in production; fixed pre-existingscopesmutability errorEnvironment Variables
GOOGLE_WORKSPACE_CLI_TOKEN,CREDENTIALS_FILE,ACCOUNT,IMPERSONATED_USERGOOGLE_WORKSPACE_CLI_CONFIG_DIRGOOGLE_WORKSPACE_CLI_CLIENT_ID,CLIENT_SECRETGOOGLE_WORKSPACE_CLI_SANITIZE_TEMPLATE,SANITIZE_MODEGOOGLE_WORKSPACE_PROJECT_IDGOOGLE_APPLICATION_CREDENTIALS(ADC fallback)