feat: Implement multi-account Configuration Profiles#358
feat: Implement multi-account Configuration Profiles#358joeVenner wants to merge 34 commits intogoogleworkspace:mainfrom
Conversation
Allows users to seamlessly switch between multiple Google Workspace accounts using the '--profile <name>' global flag or the 'gws auth switch <name>' command. This isolates configurations, token caches, and OS keyring storage per profile.
Resolves code review feedback: - Extracted 'base_config_dir()' to a single location to prevent buggy fallback deduplication across 'auth_commands.rs' and 'credential_store.rs'. - Fixed 'main.rs' argument filtering logic which incorrectly skipped the values of the '--api-version' flag.
Resolves code review feedback: - Sanitized profile names during parsing and 'auth switch' to prevent path traversals using '../'. - Added 'get_active_profile()' to deduplicate the logic of loading the active profile environment variable or fallback file. - Optimized arguments iteration in 'main.rs'.
Resolves code review feedback: - Correctly handles and surfacing errors when attempting to delete the active_profile file, while ignoring harmless 'NotFound' errors.
Resolves code review feedback: - Extracted 'validate_profile_name' to a reusable function in 'auth_commands.rs' and applied it consistently. - Updated 'credential_store.rs' to rely on the centralized 'get_active_profile' function instead of duplicating environmental checks.
Resolves code review feedback: - Ensures profile names read from the local 'active_profile' file are validated against path traversal vectors (e.g. '../'). - Fallbacks to the default profile if the user-modified input on disk is flagged as invalid.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Resolves code review feedback: - Fixed a compilation error regarding a missing 'else' return branch in 'get_active_profile', and handled the error without using 'process::exit'. - Simplified the argument iteration loop in 'main.rs' to strip redundant '--api-version' checks since it is handled later by the subcommand filter.
Resolves code review feedback: - Adds a '.is_empty()' check to 'validate_profile_name()' to prevent silent fallbacks or writing to raw directories like 'profiles/'.
Resolves code review feedback: - Fixed a bug in 'main.rs' where a dangling '--profile' flag before a subcommand like 'gws --profile drive files list' would swallow 'drive' as the profile name, breaking the command. It now correctly checks if the next argument is another flag or a command and behaves accordingly. - Strengthened the profile name validation to restrict characters to alphanumerics, hyphens, and underscores, avoiding cross-platform filepath issues on Windows like ':' or '*' being disallowed in file names.
…nd system directories Resolves critical security code review feedback: - Validates 'GOOGLE_WORKSPACE_CLI_CONFIG_DIR' to prevent users from accidentally tricking the application into securely writing tokens or cache maps to protected root directories, e.g. '/', '/etc/', or '~/.ssh'. It now safely falls back to the default config directory if a suspicious location is detected. - Validates 'GOOGLE_WORKSPACE_CLI_PROFILE' by using 'validate_profile_name', ensuring the profile's name cannot path-traverse via '../..' out of the 'profiles/' directory.
Resolves code review feedback:
- Improved the parsing logic for the '--profile' argument in 'main.rs'. Instead of unconditionally consuming the next non-flag argument as a profile name, the parser now checks if the argument matches any known services or built-in commands ('auth', 'schema', 'generate-skills'). This prevents subcommands from being incorrectly interpreted as profiles when a trailing '--profile' flag is omitted.
Resolves code review feedback: - Removed the restriction that prevented profile names from matching service names (e.g., 'drive' or 'gmail'). - Profile names are now only validated against alphanumeric characters, dashes, and underscores, giving users more flexibility when naming their configuration profiles.
Resolves code review feedback: - Corrected a regression where '--help' and '--version' mistakenly threw an invalid service parsing error. They are now whitelisted correctly as acceptable 'first_arg' parameters without requiring a subcommand. - Migrated 'handle_switch' internal file system logic to use asynchronous 'tokio::fs' equivalents instead of 'std::fs', preventing the underlying async tokio thread from arbitrarily blocking during user profile activation. - Tightened the path string security detection algorithm to ensure legitimate routes with '.ssh' inside the path string (e.g. '/home/user/project-ssh/config') aren't falsely flagged, only asserting when '.ssh' is definitively separated as an individual component folder.
Resolves code review feedback:
- Improved the 'GOOGLE_WORKSPACE_CLI_CONFIG_DIR' security validation in 'base_config_dir' to use rust's 'Path' methods instead of string representations, correctly securing Windows platforms against root directory writes.
- Enclosed UNIX specific path directory checks ('/etc', '/bin', etc) in a 'cfg!(unix)' target filter so they do not inadvertently conflict in non-POSIX environments.
Resolves code review feedback: - Fixed an issue where the single-pass argument iterator combined '--profile' and generic argument extraction but skipped over the logic required to bypass the '--api-version' parameter and its subsequent value. - Implemented a two-pass approach that cleanly extracts '--profile' values first into a filtered list, and then loops over the result to locate the 'first_arg' service string while correctly omitting any '--api-version' values.
Resolves code review feedback: - Improved the 'GOOGLE_WORKSPACE_CLI_CONFIG_DIR' security check in 'base_config_dir' by resolving symlinks natively with 'std::fs::canonicalize' before inspecting the payload for restricted system origins. This prevents users from sneaking past the check using malicious directory shortcuts to system-critical routes like '/etc/'. - Tightened 'validate_profile_name' to exclusively allow lowercase alphanumeric characters, dashes, and underscores. This mitigates critical path collisions on case-insensitive filesystems (such as default macOS or Windows) where 'MyProfile' and 'myprofile' resolve to identical folders but could conceptually conflict internally.
Resolves code review feedback: - Fixed a Time-of-check to time-of-use (TOCTOU) vulnerability where the configuration directory was being checked successfully using the canonicalized symlink version, but erroneously returned the original, unchecked symlinked string allowing the symlink destination to be hot-swapped post-verification.
Resolves code review feedback: - Fixed a path traversal security gap where if a 'GOOGLE_WORKSPACE_CLI_CONFIG_DIR' directory did not exist yet (meaning 'canonicalize' falls back to the raw path string), users could theoretically bypass restrictions using dot-dot operators, e.g., '/tmp/nonexistent/../../etc/'. - Explicitly blocked any path containing '..' string components.
Resolves code review feedback:
- Enforced a rule inside 'validate_profile_name' that explicitly forbids profile names from starting with a hyphen ('-').
- This prevents CLI parsing ambiguity where a user passes a profile name like '-x', accidentally tricking the argument parser into interpreting it as a command-line flag rather than a valid configuration directory.
…fs in keyring access
…fs in discovery and token storage
…ndardize status JSON output
…and prevent keyring IO lock contention
…n blocks, and consolidate keyring cache write-locks
…rofile argument propagation
…eCell for credential key initialization
…scopes and harden test isolation
Feat/configuration profiles
|
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 introduces a robust multi-account configuration profiles feature, enabling seamless management and isolation of Google Workspace accounts within Highlights
Changelog
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 is an impressive and comprehensive pull request that successfully introduces the multi-account configuration profiles feature. The massive refactoring to a purely asynchronous tokio runtime is well-executed and significantly hardens the application against TOCTOU vulnerabilities, data races, and I/O blocking. The migration from manual argument parsing to clap's subcommand handling is also a major improvement for robustness.
My review has identified a critical issue where some synchronous file system calls remain in an async function, which could block the tokio runtime. Addressing this will ensure the full benefits of the async migration are realized.
Note: Security Review did not run due to the size of the PR.
| let token_cache = token_cache_path(); | ||
| let plain_path = plain_credentials_path().await; | ||
| let enc_path = credential_store::encrypted_credentials_path().await; | ||
| let token_cache = token_cache_path().await; |
There was a problem hiding this comment.
While the path resolutions are now correctly async, the subsequent checks for file existence on lines 1059-1061 use the synchronous Path::exists(). These blocking I/O calls can starve the tokio runtime, which undermines a primary goal of this refactoring.
Please replace these with the non-blocking equivalent:
let has_encrypted = tokio::fs::metadata(&enc_path).await.is_ok();
let has_plain = tokio::fs::metadata(&plain_path).await.is_ok();
let has_token_cache = tokio::fs::metadata(&token_cache).await.is_ok();|
|
||
| // Show client config (client_secret.json) status | ||
| let config_path = crate::oauth_config::client_config_path(); | ||
| let config_path = crate::oauth_config::client_config_path().await; |
There was a problem hiding this comment.
Similar to the previous comment, the check on the next line (config_path.exists()) is a synchronous, blocking I/O call within an async function. This should be updated to use the non-blocking tokio::fs::metadata to avoid starving the executor.
Please change line 1089 to:
let has_config = tokio::fs::metadata(&config_path).await.is_ok();
Description
This pull request introduces a comprehensive Configuration Profiles feature, allowing users to seamlessly manage, sandbox, and switch between multiple Google Workspace accounts within
gwcli.In addition to the feature work, this PR includes a massive refactoring effort to migrate the authentication and credential management systems into a purely asynchronous
tokioruntime environment, alongside several critical security and concurrency hardening patches.🚀 Key Features
credentials.json,token_cache.json,.encryption_key, andclient_secret.jsonare now safely sandboxed inside~/.config/gws/profiles/<name>/.<profile>label explicitly (e.g.,gws-cli-work), maintaining tight namespace isolation across multi-account deployments.--profileArgument: Migrated the--profileflag resolution toclapnatively viaallow_external_subcommands, replacing fragile manual AST extraction loops and guaranteeing fallback support mapping through theGOOGLE_WORKSPACE_CLI_PROFILEenvironment variable.🛡️ Security & Hardening
GOOGLE_WORKSPACE_CLI_CONFIG_DIRhandlers to forcefully instantiate directories prior to canonicalization. This securely seals a Time-of-Check to Time-of-Use window that could allow symlink substitution attacks on configuration directories./etc,/usr,.ssh, etc.) into a centralis_suspicious_path()helper to eliminate blind spots.perms.set_mode(), which safely maps existing system metadata instead of destructively replacing behaviors on non-Unix platforms.⚡ Async I/O & Concurrency Optimizations
std::fs,Path::exists(), etc.) across the auth and credential stores intotokio::fsasynchronous executions. This explicitly prevents massive thread starvation intervals and deadlock chains from terminating the async runner during critical I/O handoffs (such aslogoutsweeps).KEYinitialization tracking incredential_store.rsdown to an idiomatictokio::sync::OnceCell, cleanly substituting manualtokio::sync::RwLockblocking macros with nativeget_or_try_init()executors.std::env::set_varon explicitly invoked CLI--profilearguments by replacing the unsafe process mutation strategy with a global thread-safestd::sync::OnceLock<String>.Validation
gws auth switch.cargo test --all), representing 426 tests including heavily isolated environment simulation checks.