From c321815e95e9908de96c30ce24645315edcc2982 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:03:51 +0000 Subject: [PATCH 01/33] feat(auth): add configuration profiles for account switching Allows users to seamlessly switch between multiple Google Workspace accounts using the '--profile ' global flag or the 'gws auth switch ' command. This isolates configurations, token caches, and OS keyring storage per profile. --- src/auth_commands.rs | 143 ++++++++++++++++++++++++++++++++++------ src/credential_store.rs | 26 +++++++- src/main.rs | 43 +++++++++++- 3 files changed, 187 insertions(+), 25 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 47d2d4e..32e6177 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -92,29 +92,49 @@ const READONLY_SCOPES: &[&str] = &[ ]; pub fn config_dir() -> PathBuf { - if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { - return PathBuf::from(dir); - } + let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { + PathBuf::from(dir) + } else { + // Use ~/.config/gws on all platforms for a consistent, user-friendly path. + let primary = dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join(".config") + .join("gws"); + + if primary.exists() { + primary + } else { + // Backward compat: fall back to OS-specific config dir for existing installs + // (e.g. ~/Library/Application Support/gws on macOS, %APPDATA%\gws on Windows). + let legacy = dirs::config_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join("gws"); + if legacy.exists() { + legacy + } else { + primary + } + } + }; - // Use ~/.config/gws on all platforms for a consistent, user-friendly path. - let primary = dirs::home_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join(".config") - .join("gws"); - if primary.exists() { - return primary; + // Determine the active profile + let mut profile = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE").ok().filter(|s| !s.is_empty()); + + // Fall back to reading active_profile file + if profile.is_none() { + let active_profile_path = base_dir.join("active_profile"); + if let Ok(content) = std::fs::read_to_string(active_profile_path) { + let trimmed = content.trim(); + if !trimmed.is_empty() { + profile = Some(trimmed.to_string()); + } + } } - // Backward compat: fall back to OS-specific config dir for existing installs - // (e.g. ~/Library/Application Support/gws on macOS, %APPDATA%\gws on Windows). - let legacy = dirs::config_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join("gws"); - if legacy.exists() { - return legacy; + match profile.as_deref() { + Some("default") | None => base_dir, + Some(name) => base_dir.join("profiles").join(name), } - - primary } fn plain_credentials_path() -> PathBuf { @@ -131,7 +151,7 @@ fn token_cache_path() -> PathBuf { /// Handle `gws auth `. pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { const USAGE: &str = concat!( - "Usage: gws auth [options]\n\n", + "Usage: gws auth [options]\n\n", " login Authenticate via OAuth2 (opens browser)\n", " --readonly Request read-only scopes\n", " --full Request all scopes incl. pubsub + cloud-platform\n", @@ -142,6 +162,7 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { " setup Configure GCP project + OAuth client (requires gcloud)\n", " --project Use a specific GCP project\n", " status Show current authentication state\n", + " switch Switch the active configuration profile\n", " export Print decrypted credentials to stdout\n", " logout Clear saved credentials and token cache", ); @@ -156,16 +177,65 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { "login" => handle_login(&args[1..]).await, "setup" => crate::setup::run_setup(&args[1..]).await, "status" => handle_status().await, + "switch" => handle_switch(&args[1..]), "export" => { let unmasked = args.len() > 1 && args[1] == "--unmasked"; handle_export(unmasked).await } "logout" => handle_logout(), other => Err(GwsError::Validation(format!( - "Unknown auth subcommand: '{other}'. Use: login, setup, status, export, logout" + "Unknown auth subcommand: '{other}'. Use: login, setup, status, switch, export, logout" ))), } } + +fn handle_switch(args: &[String]) -> Result<(), GwsError> { + if args.is_empty() { + return Err(GwsError::Validation( + "Missing profile name. Usage: gws auth switch ".to_string(), + )); + } + + let profile_name = &args[0]; + + // Read the base directory without applying the current active profile + let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { + PathBuf::from(dir) + } else { + let primary = dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join(".config") + .join("gws"); + if primary.exists() { + primary + } else { + dirs::config_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join("gws") + } + }; + + if !base_dir.exists() { + std::fs::create_dir_all(&base_dir).map_err(|e| { + GwsError::Validation(format!("Failed to create base config dir: {e}")) + })?; + } + + let active_profile_path = base_dir.join("active_profile"); + + if profile_name == "default" { + let _ = std::fs::remove_file(active_profile_path); + println!("Switched to default profile."); + } else { + std::fs::write(&active_profile_path, profile_name).map_err(|e| { + GwsError::Validation(format!("Failed to write active_profile: {e}")) + })?; + println!("Switched to profile: {profile_name}"); + } + + Ok(()) +} + /// Custom delegate that prints the OAuth URL on its own line for easy copying. /// Optionally includes `login_hint` in the URL for account pre-selection. struct CliFlowDelegate { @@ -1145,6 +1215,37 @@ async fn handle_status() -> Result<(), GwsError> { "{}", serde_json::to_string_pretty(&output).unwrap_or_default() ); + + // Determine the active profile + let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { + PathBuf::from(dir) + } else { + let primary = dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join(".config") + .join("gws"); + if primary.exists() { + primary + } else { + dirs::config_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join("gws") + } + }; + + let profile = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") + .ok() + .filter(|s| !s.is_empty()) + .or_else(|| { + std::fs::read_to_string(base_dir.join("active_profile")) + .ok() + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + }) + .unwrap_or_else(|| "default".to_string()); + + println!("\nActive Profile: {profile}"); + Ok(()) } diff --git a/src/credential_store.rs b/src/credential_store.rs index 867d5bc..7338d56 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -47,7 +47,31 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { let key_file = crate::auth_commands::config_dir().join(".encryption_key"); - let entry = Entry::new("gws-cli", &username); + let profile = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") + .ok() + .filter(|s| !s.is_empty()) + .or_else(|| { + // Attempt to read from the base config dir's active_profile + let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { + PathBuf::from(dir) + } else { + dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join(".config") + .join("gws") + }; + std::fs::read_to_string(base_dir.join("active_profile")) + .ok() + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + }); + + let service_name = match profile.as_deref() { + Some("default") | None => "gws-cli".to_string(), + Some(name) => format!("gws-cli-{}", name), + }; + + let entry = Entry::new(&service_name, &username); if let Ok(entry) = entry { match entry.get_password() { diff --git a/src/main.rs b/src/main.rs index 89d73d9..3b4ed1b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -64,8 +64,9 @@ async fn run() -> Result<(), GwsError> { )); } - // Find the first non-flag arg (skip --api-version and its value) + // Find the first non-flag arg (skip --api-version, --profile and their values) let mut first_arg: Option = None; + let mut filtered_args = vec![args[0].clone()]; { let mut skip_next = false; for a in args.iter().skip(1) { @@ -74,18 +75,52 @@ async fn run() -> Result<(), GwsError> { continue; } if a == "--api-version" { + filtered_args.push(a.clone()); skip_next = true; continue; } if a.starts_with("--api-version=") { + filtered_args.push(a.clone()); continue; } - if !a.starts_with("--") || a.as_str() == "--help" || a.as_str() == "--version" { + if a == "--profile" { + skip_next = true; + continue; + } + if a.starts_with("--profile=") { + continue; + } + filtered_args.push(a.clone()); + + if first_arg.is_none() + && !a.starts_with("--") + && a.as_str() != "--help" + && a.as_str() != "--version" + { first_arg = Some(a.clone()); - break; } } } + + // Extract profile early to set environment variable + let mut profile_name = None; + for i in 1..args.len() { + if args[i] == "--profile" && i + 1 < args.len() { + profile_name = Some(args[i + 1].clone()); + break; + } else if let Some(stripped) = args[i].strip_prefix("--profile=") { + profile_name = Some(stripped.to_string()); + break; + } + } + + if let Some(profile) = profile_name { + std::env::set_var("GOOGLE_WORKSPACE_CLI_PROFILE", profile); + } + + // Use filtered_args for the rest of path resolution (helps `filter_args_for_subcommand`) + let args = filtered_args; + let first_arg = first_arg.ok_or_else(|| { GwsError::Validation( "No service specified. Usage: gws [sub-resource] [flags]" @@ -423,6 +458,7 @@ fn print_usage() { println!(" --upload Local file to upload as media content (multipart)"); println!(" --output Output file path for binary responses"); println!(" --format Output format: json (default), table, yaml, csv"); + println!(" --profile Use a specific configuration profile"); println!(" --api-version Override the API version (e.g., v2, v3)"); println!(" --page-all Auto-paginate, one JSON line per page (NDJSON)"); println!(" --page-limit Max pages to fetch with --page-all (default: 10)"); @@ -449,6 +485,7 @@ fn print_usage() { println!( " GOOGLE_WORKSPACE_CLI_CONFIG_DIR Override config directory (default: ~/.config/gws)" ); + println!(" GOOGLE_WORKSPACE_CLI_PROFILE Configuration profile to use (e.g., 'work', 'personal')"); println!(" GOOGLE_WORKSPACE_CLI_SANITIZE_TEMPLATE Default Model Armor template"); println!( " GOOGLE_WORKSPACE_CLI_SANITIZE_MODE Sanitization mode: warn (default) or block" From ceb95792baffd608203ac209a1ee0caff04e3310 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:03:51 +0000 Subject: [PATCH 02/33] fix(auth): centralize base_dir resolution and fix arg parsing 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. --- src/auth_commands.rs | 42 ++++++--------------------- src/credential_store.rs | 9 +----- src/main.rs | 63 ++++++++++++++++++++--------------------- 3 files changed, 40 insertions(+), 74 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 32e6177..6687ccd 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -91,8 +91,8 @@ const READONLY_SCOPES: &[&str] = &[ "https://www.googleapis.com/auth/tasks.readonly", ]; -pub fn config_dir() -> PathBuf { - let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { +pub fn base_config_dir() -> PathBuf { + if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { PathBuf::from(dir) } else { // Use ~/.config/gws on all platforms for a consistent, user-friendly path. @@ -115,7 +115,11 @@ pub fn config_dir() -> PathBuf { primary } } - }; + } +} + +pub fn config_dir() -> PathBuf { + let base_dir = base_config_dir(); // Determine the active profile let mut profile = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE").ok().filter(|s| !s.is_empty()); @@ -199,21 +203,7 @@ fn handle_switch(args: &[String]) -> Result<(), GwsError> { let profile_name = &args[0]; // Read the base directory without applying the current active profile - let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { - PathBuf::from(dir) - } else { - let primary = dirs::home_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join(".config") - .join("gws"); - if primary.exists() { - primary - } else { - dirs::config_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join("gws") - } - }; + let base_dir = base_config_dir(); if !base_dir.exists() { std::fs::create_dir_all(&base_dir).map_err(|e| { @@ -1217,21 +1207,7 @@ async fn handle_status() -> Result<(), GwsError> { ); // Determine the active profile - let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { - PathBuf::from(dir) - } else { - let primary = dirs::home_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join(".config") - .join("gws"); - if primary.exists() { - primary - } else { - dirs::config_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join("gws") - } - }; + let base_dir = base_config_dir(); let profile = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") .ok() diff --git a/src/credential_store.rs b/src/credential_store.rs index 7338d56..f8cd68b 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -52,14 +52,7 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { .filter(|s| !s.is_empty()) .or_else(|| { // Attempt to read from the base config dir's active_profile - let base_dir = if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { - PathBuf::from(dir) - } else { - dirs::home_dir() - .unwrap_or_else(|| PathBuf::from(".")) - .join(".config") - .join("gws") - }; + let base_dir = crate::auth_commands::base_config_dir(); std::fs::read_to_string(base_dir.join("active_profile")) .ok() .map(|s| s.trim().to_string()) diff --git a/src/main.rs b/src/main.rs index 3b4ed1b..7d1b5c8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -64,42 +64,39 @@ async fn run() -> Result<(), GwsError> { )); } - // Find the first non-flag arg (skip --api-version, --profile and their values) - let mut first_arg: Option = None; let mut filtered_args = vec![args[0].clone()]; - { - let mut skip_next = false; - for a in args.iter().skip(1) { - if skip_next { - skip_next = false; - continue; - } - if a == "--api-version" { - filtered_args.push(a.clone()); - skip_next = true; - continue; - } - if a.starts_with("--api-version=") { - filtered_args.push(a.clone()); - continue; - } - if a == "--profile" { - skip_next = true; - continue; - } - if a.starts_with("--profile=") { - continue; - } - filtered_args.push(a.clone()); - - if first_arg.is_none() - && !a.starts_with("--") - && a.as_str() != "--help" - && a.as_str() != "--version" - { - first_arg = Some(a.clone()); + let mut first_arg: Option = None; + let mut i = 1; + while i < args.len() { + let a = &args[i]; + + if a == "--profile" { + i += 2; // Skip --profile and its value + continue; + } + if a.starts_with("--profile=") { + i += 1; + continue; + } + + filtered_args.push(a.clone()); + + if a == "--api-version" { + if i + 1 < args.len() { + filtered_args.push(args[i+1].clone()); } + i += 2; // Skip both the flag and the value + continue; + } + if a.starts_with("--api-version=") { + i += 1; + continue; + } + + if first_arg.is_none() && !a.starts_with("--") && a != "--help" && a != "--version" { + first_arg = Some(a.clone()); } + i += 1; } // Extract profile early to set environment variable From 60bb1aa7fbe120252924d8608f7935830ecdf812 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:07:52 +0000 Subject: [PATCH 03/33] fix(auth): prevent path traversal and centralize profile resolution 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'. --- src/auth_commands.rs | 55 ++++++++++++++++++++++---------------------- src/main.rs | 32 +++++++++++++++----------- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 6687ccd..a4b0b48 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -118,24 +118,29 @@ pub fn base_config_dir() -> PathBuf { } } +pub fn get_active_profile() -> Option { + std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") + .ok() + .filter(|s| !s.is_empty()) + .or_else(|| { + let base_dir = base_config_dir(); + std::fs::read_to_string(base_dir.join("active_profile")) + .ok() + .and_then(|s| { + let trimmed = s.trim(); + if trimmed.is_empty() { + None + } else { + Some(trimmed.to_string()) + } + }) + }) +} + pub fn config_dir() -> PathBuf { let base_dir = base_config_dir(); - // Determine the active profile - let mut profile = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE").ok().filter(|s| !s.is_empty()); - - // Fall back to reading active_profile file - if profile.is_none() { - let active_profile_path = base_dir.join("active_profile"); - if let Ok(content) = std::fs::read_to_string(active_profile_path) { - let trimmed = content.trim(); - if !trimmed.is_empty() { - profile = Some(trimmed.to_string()); - } - } - } - - match profile.as_deref() { + match get_active_profile().as_deref() { Some("default") | None => base_dir, Some(name) => base_dir.join("profiles").join(name), } @@ -202,6 +207,12 @@ fn handle_switch(args: &[String]) -> Result<(), GwsError> { let profile_name = &args[0]; + if profile_name.contains('/') || profile_name.contains('\\') || profile_name == "." || profile_name == ".." { + return Err(GwsError::Validation( + "Invalid profile name. It cannot contain path separators or be '.' or '..'".to_string(), + )); + } + // Read the base directory without applying the current active profile let base_dir = base_config_dir(); @@ -1205,20 +1216,8 @@ async fn handle_status() -> Result<(), GwsError> { "{}", serde_json::to_string_pretty(&output).unwrap_or_default() ); - // Determine the active profile - let base_dir = base_config_dir(); - - let profile = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") - .ok() - .filter(|s| !s.is_empty()) - .or_else(|| { - std::fs::read_to_string(base_dir.join("active_profile")) - .ok() - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - }) - .unwrap_or_else(|| "default".to_string()); + let profile = get_active_profile().unwrap_or_else(|| "default".to_string()); println!("\nActive Profile: {profile}"); diff --git a/src/main.rs b/src/main.rs index 7d1b5c8..a83fd47 100644 --- a/src/main.rs +++ b/src/main.rs @@ -66,15 +66,26 @@ async fn run() -> Result<(), GwsError> { let mut filtered_args = vec![args[0].clone()]; let mut first_arg: Option = None; + let mut profile_name: Option = None; let mut i = 1; while i < args.len() { let a = &args[i]; if a == "--profile" { - i += 2; // Skip --profile and its value + if i + 1 < args.len() { + if profile_name.is_none() { + profile_name = Some(args[i + 1].clone()); + } + i += 2; // Skip --profile and its value + } else { + i += 1; // Skip dangling --profile flag + } continue; } - if a.starts_with("--profile=") { + if let Some(stripped) = a.strip_prefix("--profile=") { + if profile_name.is_none() { + profile_name = Some(stripped.to_string()); + } i += 1; continue; } @@ -99,19 +110,12 @@ async fn run() -> Result<(), GwsError> { i += 1; } - // Extract profile early to set environment variable - let mut profile_name = None; - for i in 1..args.len() { - if args[i] == "--profile" && i + 1 < args.len() { - profile_name = Some(args[i + 1].clone()); - break; - } else if let Some(stripped) = args[i].strip_prefix("--profile=") { - profile_name = Some(stripped.to_string()); - break; - } - } - if let Some(profile) = profile_name { + if profile.contains('/') || profile.contains('\\') || profile == "." || profile == ".." { + return Err(GwsError::Validation( + "Invalid profile name. It cannot contain path separators or be '.' or '..'".to_string(), + )); + } std::env::set_var("GOOGLE_WORKSPACE_CLI_PROFILE", profile); } From 1cbd74bdaf4c431f8a2adaa0e1416e4492ceed58 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:09:32 +0000 Subject: [PATCH 04/33] fix(auth): handle file removal errors during profile switch Resolves code review feedback: - Correctly handles and surfacing errors when attempting to delete the active_profile file, while ignoring harmless 'NotFound' errors. --- src/auth_commands.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index a4b0b48..4b53129 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -225,7 +225,11 @@ fn handle_switch(args: &[String]) -> Result<(), GwsError> { let active_profile_path = base_dir.join("active_profile"); if profile_name == "default" { - let _ = std::fs::remove_file(active_profile_path); + if let Err(e) = std::fs::remove_file(active_profile_path) { + if e.kind() != std::io::ErrorKind::NotFound { + return Err(GwsError::Validation(format!("Failed to remove active_profile file: {e}"))); + } + } println!("Switched to default profile."); } else { std::fs::write(&active_profile_path, profile_name).map_err(|e| { From 4147995e68182b806a9bb9fca7f41a6be03187b3 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:14:21 +0000 Subject: [PATCH 05/33] refactor(auth): deduplicate profile logic and paths 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. --- src/auth_commands.rs | 15 ++++++++++----- src/credential_store.rs | 12 +----------- src/main.rs | 6 +----- 3 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 4b53129..ccdb6d3 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -137,6 +137,15 @@ pub fn get_active_profile() -> Option { }) } +pub fn validate_profile_name(profile_name: &str) -> Result<(), GwsError> { + if profile_name.contains('/') || profile_name.contains('\\') || profile_name == "." || profile_name == ".." { + return Err(GwsError::Validation( + "Invalid profile name. It cannot contain path separators or be '.' or '..'".to_string(), + )); + } + Ok(()) +} + pub fn config_dir() -> PathBuf { let base_dir = base_config_dir(); @@ -207,11 +216,7 @@ fn handle_switch(args: &[String]) -> Result<(), GwsError> { let profile_name = &args[0]; - if profile_name.contains('/') || profile_name.contains('\\') || profile_name == "." || profile_name == ".." { - return Err(GwsError::Validation( - "Invalid profile name. It cannot contain path separators or be '.' or '..'".to_string(), - )); - } + validate_profile_name(profile_name)?; // Read the base directory without applying the current active profile let base_dir = base_config_dir(); diff --git a/src/credential_store.rs b/src/credential_store.rs index f8cd68b..a3fa86a 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -47,17 +47,7 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { let key_file = crate::auth_commands::config_dir().join(".encryption_key"); - let profile = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") - .ok() - .filter(|s| !s.is_empty()) - .or_else(|| { - // Attempt to read from the base config dir's active_profile - let base_dir = crate::auth_commands::base_config_dir(); - std::fs::read_to_string(base_dir.join("active_profile")) - .ok() - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - }); + let profile = crate::auth_commands::get_active_profile(); let service_name = match profile.as_deref() { Some("default") | None => "gws-cli".to_string(), diff --git a/src/main.rs b/src/main.rs index a83fd47..a218431 100644 --- a/src/main.rs +++ b/src/main.rs @@ -111,11 +111,7 @@ async fn run() -> Result<(), GwsError> { } if let Some(profile) = profile_name { - if profile.contains('/') || profile.contains('\\') || profile == "." || profile == ".." { - return Err(GwsError::Validation( - "Invalid profile name. It cannot contain path separators or be '.' or '..'".to_string(), - )); - } + crate::auth_commands::validate_profile_name(&profile)?; std::env::set_var("GOOGLE_WORKSPACE_CLI_PROFILE", profile); } From 1ea183ca4ecf3f18f97904b217cbaf3788d6bdc1 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:19:51 +0000 Subject: [PATCH 06/33] fix(auth): prevent path traversal from disk active_profile 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. --- src/auth_commands.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index ccdb6d3..783eaed 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -130,6 +130,12 @@ pub fn get_active_profile() -> Option { let trimmed = s.trim(); if trimmed.is_empty() { None + } else if validate_profile_name(trimmed).is_err() { + eprintln!( + "Warning: invalid profile name '{}' found in active_profile file. Ignoring and using default.", + trimmed + ); + None } else { Some(trimmed.to_string()) } From fba08990800493f703195f478ff747dac53f167c Mon Sep 17 00:00:00 2001 From: JoeVenner Date: Sat, 7 Mar 2026 05:23:38 +0000 Subject: [PATCH 07/33] Update src/auth_commands.rs Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- src/auth_commands.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 783eaed..dcedfb0 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -130,13 +130,13 @@ pub fn get_active_profile() -> Option { let trimmed = s.trim(); if trimmed.is_empty() { None - } else if validate_profile_name(trimmed).is_err() { + } else if let Err(e) = validate_profile_name(trimmed) { eprintln!( - "Warning: invalid profile name '{}' found in active_profile file. Ignoring and using default.", - trimmed + "Error: Invalid profile name '{}' found in active_profile file: {}. Please fix it or remove the file.", + trimmed, e ); - None - } else { + std::process::exit(1); + Some(trimmed.to_string()) } }) From 32f5525dcbe17631afa093ec077ff733fb6666fe Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:29:19 +0000 Subject: [PATCH 08/33] fix(auth): resolve profile parsing compiler error and simplify iteration 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. --- src/auth_commands.rs | 20 +++++++++++--------- src/main.rs | 12 ------------ 2 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index dcedfb0..03a669c 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -129,15 +129,17 @@ pub fn get_active_profile() -> Option { .and_then(|s| { let trimmed = s.trim(); if trimmed.is_empty() { - None - } else if let Err(e) = validate_profile_name(trimmed) { - eprintln!( - "Error: Invalid profile name '{}' found in active_profile file: {}. Please fix it or remove the file.", - trimmed, e - ); - std::process::exit(1); - - Some(trimmed.to_string()) + return None; + } + match validate_profile_name(trimmed) { + Ok(_) => Some(trimmed.to_string()), + Err(e) => { + eprintln!( + "Error: Invalid profile name '{}' found in active_profile file: {}. Please fix it or remove the file. Defaulting to 'default' profile.", + trimmed, e + ); + None + } } }) }) diff --git a/src/main.rs b/src/main.rs index a218431..fd493aa 100644 --- a/src/main.rs +++ b/src/main.rs @@ -92,18 +92,6 @@ async fn run() -> Result<(), GwsError> { filtered_args.push(a.clone()); - if a == "--api-version" { - if i + 1 < args.len() { - filtered_args.push(args[i+1].clone()); - } - i += 2; // Skip both the flag and the value - continue; - } - if a.starts_with("--api-version=") { - i += 1; - continue; - } - if first_arg.is_none() && !a.starts_with("--") && a != "--help" && a != "--version" { first_arg = Some(a.clone()); } From 2a07ceb72d1ef7d64cf234a135fbfb1fcd75ed53 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:36:48 +0000 Subject: [PATCH 09/33] fix(auth): prevent empty profile names in validation Resolves code review feedback: - Adds a '.is_empty()' check to 'validate_profile_name()' to prevent silent fallbacks or writing to raw directories like 'profiles/'. --- src/auth_commands.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 03a669c..a5ba01c 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -146,9 +146,9 @@ pub fn get_active_profile() -> Option { } pub fn validate_profile_name(profile_name: &str) -> Result<(), GwsError> { - if profile_name.contains('/') || profile_name.contains('\\') || profile_name == "." || profile_name == ".." { + if profile_name.is_empty() || profile_name.contains('/') || profile_name.contains('\\') || profile_name == "." || profile_name == ".." { return Err(GwsError::Validation( - "Invalid profile name. It cannot contain path separators or be '.' or '..'".to_string(), + "Invalid profile name. It cannot be empty, contain path separators, or be '.' or '..'".to_string(), )); } Ok(()) From ed48063e1f2c35efa1b247385e498fba08a9ba23 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:42:34 +0000 Subject: [PATCH 10/33] fix(auth): secure cross-platform profile names and parsing 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. --- src/auth_commands.rs | 8 ++++++-- src/main.rs | 15 +++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index a5ba01c..70bac62 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -146,9 +146,13 @@ pub fn get_active_profile() -> Option { } pub fn validate_profile_name(profile_name: &str) -> Result<(), GwsError> { - if profile_name.is_empty() || profile_name.contains('/') || profile_name.contains('\\') || profile_name == "." || profile_name == ".." { + if profile_name.is_empty() + || profile_name == "." + || profile_name == ".." + || profile_name.chars().any(|c| !(c.is_alphanumeric() || c == '-' || c == '_')) + { return Err(GwsError::Validation( - "Invalid profile name. It cannot be empty, contain path separators, or be '.' or '..'".to_string(), + "Invalid profile name. It must not be empty, '.', or '..', and can only contain alphanumeric characters, dashes (-), and underscores (_).".to_string(), )); } Ok(()) diff --git a/src/main.rs b/src/main.rs index fd493aa..25cee62 100644 --- a/src/main.rs +++ b/src/main.rs @@ -72,14 +72,17 @@ async fn run() -> Result<(), GwsError> { let a = &args[i]; if a == "--profile" { - if i + 1 < args.len() { - if profile_name.is_none() { - profile_name = Some(args[i + 1].clone()); + if let Some(val) = args.get(i + 1) { + if !val.starts_with('-') { + if profile_name.is_none() { + profile_name = Some(val.clone()); + } + i += 2; // Skip --profile and its value + continue; } - i += 2; // Skip --profile and its value - } else { - i += 1; // Skip dangling --profile flag } + // Dangling --profile or followed by another flag. + i += 1; continue; } if let Some(stripped) = a.strip_prefix("--profile=") { From 2f62a9d54eeadabcf3e19aa9204ff6743c58e3a2 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:49:36 +0000 Subject: [PATCH 11/33] fix(auth): secure configuration config env vars from path traversal and 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. --- src/auth_commands.rs | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 70bac62..47821b0 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -93,11 +93,27 @@ const READONLY_SCOPES: &[&str] = &[ pub fn base_config_dir() -> PathBuf { if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { - PathBuf::from(dir) - } else { - // Use ~/.config/gws on all platforms for a consistent, user-friendly path. - let primary = dirs::home_dir() - .unwrap_or_else(|| PathBuf::from(".")) + let path = PathBuf::from(&dir); + let path_str = path.to_string_lossy(); + + // Check for suspicious paths like root, system directories, or .ssh + if path_str == "/" + || path_str.starts_with("/etc") + || path_str.starts_with("/usr") + || path_str.starts_with("/var") + || path_str.starts_with("/bin") + || path_str.starts_with("/sbin") + || path_str.contains(".ssh") + { + eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path_str); + } else { + return path; + } + } + + // Use ~/.config/gws on all platforms for a consistent, user-friendly path. + let primary = dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) .join(".config") .join("gws"); @@ -115,13 +131,22 @@ pub fn base_config_dir() -> PathBuf { primary } } - } } pub fn get_active_profile() -> Option { std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") .ok() .filter(|s| !s.is_empty()) + .and_then(|s| match validate_profile_name(&s) { + Ok(_) => Some(s), + Err(e) => { + eprintln!( + "Error: Invalid profile name '{}' from GOOGLE_WORKSPACE_CLI_PROFILE: {}. Using default profile.", + s, e + ); + None + } + }) .or_else(|| { let base_dir = base_config_dir(); std::fs::read_to_string(base_dir.join("active_profile")) From e7bf8d90f50d8d1e0a0aa1def7a459ec93894118 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:53:17 +0000 Subject: [PATCH 12/33] fix(cli): prevent subcommands from being parsed as profile names 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. --- src/main.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 25cee62..0f034d9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -73,7 +73,11 @@ async fn run() -> Result<(), GwsError> { if a == "--profile" { if let Some(val) = args.get(i + 1) { - if !val.starts_with('-') { + // A profile name cannot be a known service or built-in command. + let is_command = crate::services::SERVICES.iter().any(|s| s.aliases.contains(&val.as_str())) + || matches!(val.as_str(), "auth" | "schema" | "generate-skills"); + + if !val.starts_with('-') && !is_command { if profile_name.is_none() { profile_name = Some(val.clone()); } From 2cdcb8088ee9adea0d4b5e8805994b5b6543e384 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 05:57:47 +0000 Subject: [PATCH 13/33] fix(cli): allow profile names to match service names 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. --- src/main.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/main.rs b/src/main.rs index 0f034d9..25cee62 100644 --- a/src/main.rs +++ b/src/main.rs @@ -73,11 +73,7 @@ async fn run() -> Result<(), GwsError> { if a == "--profile" { if let Some(val) = args.get(i + 1) { - // A profile name cannot be a known service or built-in command. - let is_command = crate::services::SERVICES.iter().any(|s| s.aliases.contains(&val.as_str())) - || matches!(val.as_str(), "auth" | "schema" | "generate-skills"); - - if !val.starts_with('-') && !is_command { + if !val.starts_with('-') { if profile_name.is_none() { profile_name = Some(val.clone()); } From 311351a7e2556bb6328bcf0cd4f1315054546b53 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 06:04:25 +0000 Subject: [PATCH 14/33] fix(cli): resolve --help regression and async auth file system I/O 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. --- src/auth_commands.rs | 12 ++++++------ src/main.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 47821b0..901b2c9 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -103,7 +103,7 @@ pub fn base_config_dir() -> PathBuf { || path_str.starts_with("/var") || path_str.starts_with("/bin") || path_str.starts_with("/sbin") - || path_str.contains(".ssh") + || path.components().any(|c| c.as_os_str() == ".ssh") { eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path_str); } else { @@ -232,7 +232,7 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { "login" => handle_login(&args[1..]).await, "setup" => crate::setup::run_setup(&args[1..]).await, "status" => handle_status().await, - "switch" => handle_switch(&args[1..]), + "switch" => handle_switch(&args[1..]).await, "export" => { let unmasked = args.len() > 1 && args[1] == "--unmasked"; handle_export(unmasked).await @@ -244,7 +244,7 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { } } -fn handle_switch(args: &[String]) -> Result<(), GwsError> { +async fn handle_switch(args: &[String]) -> Result<(), GwsError> { if args.is_empty() { return Err(GwsError::Validation( "Missing profile name. Usage: gws auth switch ".to_string(), @@ -259,7 +259,7 @@ fn handle_switch(args: &[String]) -> Result<(), GwsError> { let base_dir = base_config_dir(); if !base_dir.exists() { - std::fs::create_dir_all(&base_dir).map_err(|e| { + tokio::fs::create_dir_all(&base_dir).await.map_err(|e| { GwsError::Validation(format!("Failed to create base config dir: {e}")) })?; } @@ -267,14 +267,14 @@ fn handle_switch(args: &[String]) -> Result<(), GwsError> { let active_profile_path = base_dir.join("active_profile"); if profile_name == "default" { - if let Err(e) = std::fs::remove_file(active_profile_path) { + if let Err(e) = tokio::fs::remove_file(&active_profile_path).await { if e.kind() != std::io::ErrorKind::NotFound { return Err(GwsError::Validation(format!("Failed to remove active_profile file: {e}"))); } } println!("Switched to default profile."); } else { - std::fs::write(&active_profile_path, profile_name).map_err(|e| { + tokio::fs::write(&active_profile_path, profile_name).await.map_err(|e| { GwsError::Validation(format!("Failed to write active_profile: {e}")) })?; println!("Switched to profile: {profile_name}"); diff --git a/src/main.rs b/src/main.rs index 25cee62..c1714f1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -95,7 +95,7 @@ async fn run() -> Result<(), GwsError> { filtered_args.push(a.clone()); - if first_arg.is_none() && !a.starts_with("--") && a != "--help" && a != "--version" { + if first_arg.is_none() && (!a.starts_with("--") || a == "--help" || a == "--version") { first_arg = Some(a.clone()); } i += 1; From 1218c18bc529141ed6e077f5d698d7ebb89a1670 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 06:10:18 +0000 Subject: [PATCH 15/33] fix(auth): make config path security check cross-platform 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. --- src/auth_commands.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 901b2c9..c025645 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -94,18 +94,19 @@ const READONLY_SCOPES: &[&str] = &[ pub fn base_config_dir() -> PathBuf { if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { let path = PathBuf::from(&dir); - let path_str = path.to_string_lossy(); // Check for suspicious paths like root, system directories, or .ssh - if path_str == "/" - || path_str.starts_with("/etc") - || path_str.starts_with("/usr") - || path_str.starts_with("/var") - || path_str.starts_with("/bin") - || path_str.starts_with("/sbin") - || path.components().any(|c| c.as_os_str() == ".ssh") - { - eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path_str); + let is_suspicious = path.parent().is_none() + || (cfg!(unix) + && (path.starts_with("/etc") + || path.starts_with("/usr") + || path.starts_with("/var") + || path.starts_with("/bin") + || path.starts_with("/sbin"))) + || path.components().any(|c| c.as_os_str() == ".ssh"); + + if is_suspicious { + eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); } else { return path; } From a440634334563625467152eca8acce2cc24f8736 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 06:17:14 +0000 Subject: [PATCH 16/33] fix(cli): resolve --api-version parsing regression 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. --- src/main.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/main.rs b/src/main.rs index c1714f1..42038d2 100644 --- a/src/main.rs +++ b/src/main.rs @@ -94,13 +94,30 @@ async fn run() -> Result<(), GwsError> { } filtered_args.push(a.clone()); - - if first_arg.is_none() && (!a.starts_with("--") || a == "--help" || a == "--version") { - first_arg = Some(a.clone()); - } i += 1; } + // Now find the first non-flag argument from the filtered list, skipping over --api-version. + { + let mut skip_next = false; + for a in filtered_args.iter().skip(1) { + if skip_next { + skip_next = false; + continue; + } + if a == "--api-version" { + skip_next = true; + continue; + } + if a.starts_with("--api-version=") { + continue; + } + if first_arg.is_none() && (!a.starts_with("--") || a == "--help" || a == "--version") { + first_arg = Some(a.clone()); + } + } + } + if let Some(profile) = profile_name { crate::auth_commands::validate_profile_name(&profile)?; std::env::set_var("GOOGLE_WORKSPACE_CLI_PROFILE", profile); From fb44e06efef1e7956c403d0f1aa9d47be5dbcebb Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 06:21:54 +0000 Subject: [PATCH 17/33] fix(auth): prevent symlink traversal and enforce lowercase profiles 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. --- src/auth_commands.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index c025645..81814f8 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -95,15 +95,18 @@ pub fn base_config_dir() -> PathBuf { if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { let path = PathBuf::from(&dir); + // Resolve symlinks to check the real path. If path doesn't exist, check the given path. + let path_to_check = std::fs::canonicalize(&path).unwrap_or_else(|_| path.clone()); + // Check for suspicious paths like root, system directories, or .ssh - let is_suspicious = path.parent().is_none() + let is_suspicious = path_to_check.parent().is_none() || (cfg!(unix) - && (path.starts_with("/etc") - || path.starts_with("/usr") - || path.starts_with("/var") - || path.starts_with("/bin") - || path.starts_with("/sbin"))) - || path.components().any(|c| c.as_os_str() == ".ssh"); + && (path_to_check.starts_with("/etc") + || path_to_check.starts_with("/usr") + || path_to_check.starts_with("/var") + || path_to_check.starts_with("/bin") + || path_to_check.starts_with("/sbin"))) + || path_to_check.components().any(|c| c.as_os_str() == ".ssh"); if is_suspicious { eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); @@ -175,10 +178,12 @@ pub fn validate_profile_name(profile_name: &str) -> Result<(), GwsError> { if profile_name.is_empty() || profile_name == "." || profile_name == ".." - || profile_name.chars().any(|c| !(c.is_alphanumeric() || c == '-' || c == '_')) + || profile_name.chars().any(|c| { + !c.is_ascii_lowercase() && !c.is_ascii_digit() && c != '-' && c != '_' + }) { return Err(GwsError::Validation( - "Invalid profile name. It must not be empty, '.', or '..', and can only contain alphanumeric characters, dashes (-), and underscores (_).".to_string(), + "Invalid profile name. It must not be empty, '.', or '..', and can only contain lowercase alphanumeric characters, dashes (-), and underscores (_).".to_string(), )); } Ok(()) From 014a653e00213423a59158ed4bc9ba59e9746a70 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 06:27:45 +0000 Subject: [PATCH 18/33] fix(auth): secure config symlink TOCTOU vulnerability 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. --- src/auth_commands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 81814f8..3172065 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -111,7 +111,7 @@ pub fn base_config_dir() -> PathBuf { if is_suspicious { eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); } else { - return path; + return path_to_check; } } From 257e08574d3be2f46b8acae1966abc36c4f22740 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 06:32:25 +0000 Subject: [PATCH 19/33] fix(auth): secure path configuration fallback from dot-dot traversals 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. --- src/auth_commands.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 3172065..f33915f 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -100,13 +100,13 @@ pub fn base_config_dir() -> PathBuf { // Check for suspicious paths like root, system directories, or .ssh let is_suspicious = path_to_check.parent().is_none() + || path_to_check.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") || (cfg!(unix) && (path_to_check.starts_with("/etc") || path_to_check.starts_with("/usr") || path_to_check.starts_with("/var") || path_to_check.starts_with("/bin") - || path_to_check.starts_with("/sbin"))) - || path_to_check.components().any(|c| c.as_os_str() == ".ssh"); + || path_to_check.starts_with("/sbin"))); if is_suspicious { eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); From 62656fa670ebe2a117e366a46fc2ca7a4f5d946e Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 06:36:42 +0000 Subject: [PATCH 20/33] fix(auth): reject profile names starting with a hyphen 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. --- src/auth_commands.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index f33915f..f445f29 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -178,12 +178,13 @@ pub fn validate_profile_name(profile_name: &str) -> Result<(), GwsError> { if profile_name.is_empty() || profile_name == "." || profile_name == ".." + || profile_name.starts_with('-') || profile_name.chars().any(|c| { !c.is_ascii_lowercase() && !c.is_ascii_digit() && c != '-' && c != '_' }) { return Err(GwsError::Validation( - "Invalid profile name. It must not be empty, '.', or '..', and can only contain lowercase alphanumeric characters, dashes (-), and underscores (_).".to_string(), + "Invalid profile name. It must not be empty, '.', or '..', start with a dash (-), and can only contain lowercase alphanumeric characters, dashes, and underscores (_).".to_string(), )); } Ok(()) From 8cea73174302fd292afc53d4103ba885138e6553 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 07:08:09 +0000 Subject: [PATCH 21/33] refactor: tokio async migration and clap --profile integration --- src/auth.rs | 9 +- src/auth_commands.rs | 211 +++++++++++++++++++++------------------- src/commands.rs | 7 ++ src/credential_store.rs | 131 +++++++++++++------------ src/discovery.rs | 2 +- src/main.rs | 120 ++++++++--------------- src/oauth_config.rs | 39 ++++---- src/setup.rs | 16 +-- src/token_storage.rs | 4 +- 9 files changed, 263 insertions(+), 276 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index ec57ac2..d9f76dd 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -78,8 +78,8 @@ pub async fn get_token(scopes: &[&str]) -> anyhow::Result { } let creds_file = std::env::var("GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE").ok(); - let config_dir = crate::auth_commands::config_dir(); - let enc_path = credential_store::encrypted_credentials_path(); + let config_dir = crate::auth_commands::config_dir().await; + let enc_path = credential_store::encrypted_credentials_path().await; let default_path = config_dir.join("credentials.json"); let token_cache = config_dir.join("token_cache.json"); @@ -189,6 +189,7 @@ async fn load_credentials_inner( // 2. Encrypted credentials (always AuthorizedUser for now) if enc_path.exists() { let json_str = credential_store::load_encrypted_from_path(enc_path) + .await .context("Failed to decrypt credentials")?; let creds: serde_json::Value = @@ -539,7 +540,7 @@ mod tests { let enc_path = dir.path().join("credentials.enc"); // Encrypt and write - let encrypted = crate::credential_store::encrypt(json.as_bytes()).unwrap(); + let encrypted = crate::credential_store::encrypt(json.as_bytes()).await.unwrap(); std::fs::write(&enc_path, &encrypted).unwrap(); let res = load_credentials_inner(None, &enc_path, &PathBuf::from("/does/not/exist")) @@ -576,7 +577,7 @@ mod tests { let enc_path = dir.path().join("credentials.enc"); let plain_path = dir.path().join("credentials.json"); - let encrypted = crate::credential_store::encrypt(enc_json.as_bytes()).unwrap(); + let encrypted = crate::credential_store::encrypt(enc_json.as_bytes()).await.unwrap(); std::fs::write(&enc_path, &encrypted).unwrap(); std::fs::write(&plain_path, plain_json).unwrap(); diff --git a/src/auth_commands.rs b/src/auth_commands.rs index f445f29..efd90c7 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -91,12 +91,12 @@ const READONLY_SCOPES: &[&str] = &[ "https://www.googleapis.com/auth/tasks.readonly", ]; -pub fn base_config_dir() -> PathBuf { +pub async fn base_config_dir() -> PathBuf { if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { let path = PathBuf::from(&dir); // Resolve symlinks to check the real path. If path doesn't exist, check the given path. - let path_to_check = std::fs::canonicalize(&path).unwrap_or_else(|_| path.clone()); + let path_to_check = tokio::fs::canonicalize(&path).await.unwrap_or_else(|_| path.clone()); // Check for suspicious paths like root, system directories, or .ssh let is_suspicious = path_to_check.parent().is_none() @@ -137,41 +137,43 @@ pub fn base_config_dir() -> PathBuf { } } -pub fn get_active_profile() -> Option { - std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") +pub async fn get_active_profile() -> Option { + if let Some(s) = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") .ok() .filter(|s| !s.is_empty()) - .and_then(|s| match validate_profile_name(&s) { - Ok(_) => Some(s), + { + match validate_profile_name(&s) { + Ok(_) => return Some(s), Err(e) => { eprintln!( "Error: Invalid profile name '{}' from GOOGLE_WORKSPACE_CLI_PROFILE: {}. Using default profile.", s, e ); - None + return None; } - }) - .or_else(|| { - let base_dir = base_config_dir(); - std::fs::read_to_string(base_dir.join("active_profile")) - .ok() - .and_then(|s| { - let trimmed = s.trim(); - if trimmed.is_empty() { - return None; - } - match validate_profile_name(trimmed) { - Ok(_) => Some(trimmed.to_string()), - Err(e) => { - eprintln!( - "Error: Invalid profile name '{}' found in active_profile file: {}. Please fix it or remove the file. Defaulting to 'default' profile.", - trimmed, e - ); - None - } - } - }) - }) + } + } + + let base_dir = base_config_dir().await; + match tokio::fs::read_to_string(base_dir.join("active_profile")).await { + Ok(s) => { + let trimmed = s.trim(); + if trimmed.is_empty() { + return None; + } + match validate_profile_name(trimmed) { + Ok(_) => Some(trimmed.to_string()), + Err(e) => { + eprintln!( + "Error: Invalid profile name '{}' found in active_profile file: {}. Please fix it or remove the file. Defaulting to 'default' profile.", + trimmed, e + ); + None + } + } + } + Err(_) => None, + } } pub fn validate_profile_name(profile_name: &str) -> Result<(), GwsError> { @@ -190,24 +192,24 @@ pub fn validate_profile_name(profile_name: &str) -> Result<(), GwsError> { Ok(()) } -pub fn config_dir() -> PathBuf { - let base_dir = base_config_dir(); +pub async fn config_dir() -> PathBuf { + let base_dir = base_config_dir().await; - match get_active_profile().as_deref() { + match get_active_profile().await.as_deref() { Some("default") | None => base_dir, Some(name) => base_dir.join("profiles").join(name), } } -fn plain_credentials_path() -> PathBuf { +pub async fn plain_credentials_path() -> PathBuf { if let Ok(path) = std::env::var("GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE") { return PathBuf::from(path); } - config_dir().join("credentials.json") + config_dir().await.join("credentials.json") } -fn token_cache_path() -> PathBuf { - config_dir().join("token_cache.json") +pub async fn token_cache_path() -> PathBuf { + config_dir().await.join("token_cache.json") } /// Handle `gws auth `. @@ -244,7 +246,7 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { let unmasked = args.len() > 1 && args[1] == "--unmasked"; handle_export(unmasked).await } - "logout" => handle_logout(), + "logout" => handle_logout().await, other => Err(GwsError::Validation(format!( "Unknown auth subcommand: '{other}'. Use: login, setup, status, switch, export, logout" ))), @@ -263,7 +265,7 @@ async fn handle_switch(args: &[String]) -> Result<(), GwsError> { validate_profile_name(profile_name)?; // Read the base directory without applying the current active profile - let base_dir = base_config_dir(); + let base_dir = base_config_dir().await; if !base_dir.exists() { tokio::fs::create_dir_all(&base_dir).await.map_err(|e| { @@ -359,11 +361,11 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { // Resolve client_id and client_secret: // 1. Env vars (highest priority) // 2. Saved client_secret.json from `gws auth setup` or manual download - let (client_id, client_secret, project_id) = resolve_client_credentials()?; + let (client_id, client_secret, project_id) = resolve_client_credentials().await?; // Persist credentials to client_secret.json if not already saved, // so they survive env var removal or shell session changes. - if !crate::oauth_config::client_config_path().exists() { + if !crate::oauth_config::client_config_path().await.exists() { let _ = crate::oauth_config::save_client_config( &client_id, &client_secret, @@ -401,7 +403,7 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { } // Use a temp file for yup-oauth2's token persistence, then encrypt it - let temp_path = config_dir().join("credentials.tmp"); + let temp_path = config_dir().await.join("credentials.tmp"); // Always start fresh — delete any stale temp cache from prior login attempts. let _ = std::fs::remove_file(&temp_path); @@ -434,12 +436,14 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { if token.token().is_some() { // Read yup-oauth2's token cache to extract the refresh_token. - // EncryptedTokenStorage stores data encrypted, so we must decrypt first. - let token_data = std::fs::read(&temp_path) - .ok() - .and_then(|bytes| crate::credential_store::decrypt(&bytes).ok()) - .and_then(|decrypted| String::from_utf8(decrypted).ok()) - .unwrap_or_default(); + let mut token_data = String::new(); + if let Ok(bytes) = tokio::fs::read(&temp_path).await { + if let Ok(decrypted) = crate::credential_store::decrypt(&bytes).await { + if let Ok(s) = String::from_utf8(decrypted) { + token_data = s; + } + } + } let refresh_token = extract_refresh_token(&token_data).ok_or_else(|| { GwsError::Auth( "OAuth flow completed but no refresh token was returned. \ @@ -465,6 +469,7 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { // Save encrypted credentials let enc_path = credential_store::save_encrypted(&creds_str) + .await .map_err(|e| GwsError::Auth(format!("Failed to encrypt credentials: {e}")))?; // Clean up temp file @@ -514,14 +519,14 @@ async fn fetch_userinfo_email(access_token: &str) -> Option { } async fn handle_export(unmasked: bool) -> Result<(), GwsError> { - let enc_path = credential_store::encrypted_credentials_path(); + let enc_path = credential_store::encrypted_credentials_path().await; if !enc_path.exists() { return Err(GwsError::Auth( "No encrypted credentials found. Run 'gws auth login' first.".to_string(), )); } - match credential_store::load_encrypted() { + match credential_store::load_encrypted().await { Ok(contents) => { if unmasked { println!("{contents}"); @@ -548,7 +553,7 @@ async fn handle_export(unmasked: bool) -> Result<(), GwsError> { } /// Resolve OAuth client credentials from env vars or saved config file. -fn resolve_client_credentials() -> Result<(String, String, Option), GwsError> { +async fn resolve_client_credentials() -> Result<(String, String, Option), GwsError> { // 1. Try env vars first let env_id = std::env::var("GOOGLE_WORKSPACE_CLI_CLIENT_ID").ok(); let env_secret = std::env::var("GOOGLE_WORKSPACE_CLI_CLIENT_SECRET").ok(); @@ -556,13 +561,14 @@ fn resolve_client_credentials() -> Result<(String, String, Option), GwsE if let (Some(id), Some(secret)) = (env_id, env_secret) { // Still try to load project_id from config file for the scope picker let project_id = crate::oauth_config::load_client_config() + .await .ok() .map(|c| c.project_id); return Ok((id, secret, project_id)); } // 2. Try saved client_secret.json - match crate::oauth_config::load_client_config() { + match crate::oauth_config::load_client_config().await { Ok(config) => Ok(( config.client_id, config.client_secret, @@ -576,7 +582,7 @@ fn resolve_client_credentials() -> Result<(String, String, Option), GwsE 2. Download client_secret.json from Google Cloud Console and save it to:\n \ {}\n \ 3. Set env vars: GOOGLE_WORKSPACE_CLI_CLIENT_ID and GOOGLE_WORKSPACE_CLI_CLIENT_SECRET", - crate::oauth_config::client_config_path().display() + crate::oauth_config::client_config_path().await.display() ), )), } @@ -1023,9 +1029,9 @@ fn run_simple_scope_picker(services_filter: Option<&HashSet>) -> Option< } async fn handle_status() -> Result<(), GwsError> { - let plain_path = plain_credentials_path(); - let enc_path = credential_store::encrypted_credentials_path(); - 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; let has_encrypted = enc_path.exists(); let has_plain = plain_path.exists(); @@ -1056,13 +1062,13 @@ async fn handle_status() -> Result<(), GwsError> { }); // 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; let has_config = config_path.exists(); output["client_config"] = json!(config_path.display().to_string()); output["client_config_exists"] = json!(has_config); if has_config { - match crate::oauth_config::load_client_config() { + match crate::oauth_config::load_client_config().await { Ok(config) => { output["project_id"] = json!(config.project_id); let masked_id = if config.client_id.len() > 12 { @@ -1092,7 +1098,7 @@ async fn handle_status() -> Result<(), GwsError> { output["token_env_var"] = json!(true); "token_env_var" } else { - match resolve_client_credentials() { + match resolve_client_credentials().await { Ok((_, _, _)) => { let has_env_id = std::env::var("GOOGLE_WORKSPACE_CLI_CLIENT_ID").is_ok(); let has_env_secret = std::env::var("GOOGLE_WORKSPACE_CLI_CLIENT_SECRET").is_ok(); @@ -1111,7 +1117,7 @@ async fn handle_status() -> Result<(), GwsError> { // Skip real credential/network access in test builds if !cfg!(test) { if has_encrypted { - match credential_store::load_encrypted() { + match credential_store::load_encrypted().await { Ok(contents) => { if let Ok(creds) = serde_json::from_str::(&contents) { if let Some(client_id) = creds.get("client_id").and_then(|v| v.as_str()) { @@ -1169,7 +1175,7 @@ async fn handle_status() -> Result<(), GwsError> { // Skip all network calls and subprocess spawning in test builds if !cfg!(test) { let creds_json_str = if has_encrypted { - credential_store::load_encrypted().ok() + credential_store::load_encrypted().await.ok() } else if has_plain { tokio::fs::read_to_string(&plain_path).await.ok() } else { @@ -1270,18 +1276,18 @@ async fn handle_status() -> Result<(), GwsError> { serde_json::to_string_pretty(&output).unwrap_or_default() ); // Determine the active profile - let profile = get_active_profile().unwrap_or_else(|| "default".to_string()); + let profile = get_active_profile().await.unwrap_or_else(|| "default".to_string()); println!("\nActive Profile: {profile}"); Ok(()) } -fn handle_logout() -> Result<(), GwsError> { - let plain_path = plain_credentials_path(); - let enc_path = credential_store::encrypted_credentials_path(); - let token_cache = token_cache_path(); - let sa_token_cache = config_dir().join("sa_token_cache.json"); +async fn handle_logout() -> Result<(), GwsError> { + let plain_path = plain_credentials_path().await; + let enc_path = credential_store::encrypted_credentials_path().await; + let token_cache = token_cache_path().await; + let sa_token_cache = config_dir().await.join("sa_token_cache.json"); let mut removed = Vec::new(); @@ -1521,14 +1527,14 @@ mod tests { assert_eq!(scopes[0], "https://www.googleapis.com/auth/drive"); } - #[test] + #[tokio::test] #[serial_test::serial] - fn resolve_client_credentials_from_env_vars() { + async fn resolve_client_credentials_from_env_vars() { unsafe { std::env::set_var("GOOGLE_WORKSPACE_CLI_CLIENT_ID", "test-id"); std::env::set_var("GOOGLE_WORKSPACE_CLI_CLIENT_SECRET", "test-secret"); } - let result = resolve_client_credentials(); + let result = resolve_client_credentials().await; unsafe { std::env::remove_var("GOOGLE_WORKSPACE_CLI_CLIENT_ID"); std::env::remove_var("GOOGLE_WORKSPACE_CLI_CLIENT_SECRET"); @@ -1539,16 +1545,16 @@ mod tests { // project_id may be Some if client_secret.json exists on the machine } - #[test] + #[tokio::test] #[serial_test::serial] - fn resolve_client_credentials_missing_env_vars_uses_config() { + async fn resolve_client_credentials_missing_env_vars_uses_config() { unsafe { std::env::remove_var("GOOGLE_WORKSPACE_CLI_CLIENT_ID"); std::env::remove_var("GOOGLE_WORKSPACE_CLI_CLIENT_SECRET"); } // Result depends on whether client_secret.json exists on the machine - let result = resolve_client_credentials(); - if crate::oauth_config::client_config_path().exists() { + let result = resolve_client_credentials().await; + if crate::oauth_config::client_config_path().await.exists() { assert!( result.is_ok(), "Should succeed when client_secret.json exists" @@ -1560,9 +1566,9 @@ mod tests { } } - #[test] - fn config_dir_returns_gws_subdir() { - let path = config_dir(); + #[tokio::test] + async fn config_dir_returns_gws_subdir() { + let path = config_dir().await; assert!(path.ends_with("gws")); } @@ -1575,9 +1581,9 @@ mod tests { assert!(primary.ends_with(".config/gws") || primary.ends_with(r".config\gws")); } - #[test] + #[tokio::test] #[serial_test::serial] - fn config_dir_fallback_to_legacy() { + async fn config_dir_fallback_to_legacy() { // When GOOGLE_WORKSPACE_CLI_CONFIG_DIR points to a legacy-style dir, // config_dir() should return it (simulating the test env override). let dir = tempfile::tempdir().unwrap(); @@ -1587,46 +1593,47 @@ mod tests { unsafe { std::env::set_var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR", legacy.to_str().unwrap()); } - let path = config_dir(); - assert_eq!(path, legacy); + let path = config_dir().await; + let expected = tokio::fs::canonicalize(&legacy).await.unwrap_or(legacy); + assert_eq!(path, expected); unsafe { std::env::remove_var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR"); } } - #[test] + #[tokio::test] #[serial_test::serial] - fn plain_credentials_path_defaults_to_config_dir() { + async fn plain_credentials_path_defaults_to_config_dir() { // Without env var, should be in config dir unsafe { std::env::remove_var("GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE"); } - let path = plain_credentials_path(); + let path = plain_credentials_path().await; assert!(path.ends_with("credentials.json")); - assert!(path.starts_with(config_dir())); + assert!(path.starts_with(config_dir().await)); } - #[test] + #[tokio::test] #[serial_test::serial] - fn plain_credentials_path_respects_env_var() { + async fn plain_credentials_path_respects_env_var() { unsafe { std::env::set_var( "GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE", "/tmp/test-creds.json", ); } - let path = plain_credentials_path(); + let path = plain_credentials_path().await; assert_eq!(path, PathBuf::from("/tmp/test-creds.json")); unsafe { std::env::remove_var("GOOGLE_WORKSPACE_CLI_CREDENTIALS_FILE"); } } - #[test] - fn token_cache_path_is_in_config_dir() { - let path = token_cache_path(); + #[tokio::test] + async fn token_cache_path_is_in_config_dir() { + let path = token_cache_path().await; assert!(path.ends_with("token_cache.json")); - assert!(path.starts_with(config_dir())); + assert!(path.starts_with(config_dir().await)); } #[tokio::test] @@ -1662,16 +1669,16 @@ mod tests { } } - #[test] + #[tokio::test] #[serial_test::serial] - fn resolve_credentials_fails_without_env_vars_or_config() { + async fn resolve_credentials_fails_without_env_vars_or_config() { unsafe { std::env::remove_var("GOOGLE_WORKSPACE_CLI_CLIENT_ID"); std::env::remove_var("GOOGLE_WORKSPACE_CLI_CLIENT_SECRET"); } // When no env vars AND no client_secret.json on disk, should fail - let result = resolve_client_credentials(); - if !crate::oauth_config::client_config_path().exists() { + let result = resolve_client_credentials().await; + if !crate::oauth_config::client_config_path().await.exists() { assert!(result.is_err()); match result.unwrap_err() { GwsError::Auth(msg) => assert!(msg.contains("No OAuth client configured")), @@ -1682,15 +1689,15 @@ mod tests { // successfully — that's correct behavior, not a test failure. } - #[test] + #[tokio::test] #[serial_test::serial] - fn resolve_credentials_uses_env_vars_when_present() { + async fn resolve_credentials_uses_env_vars_when_present() { unsafe { std::env::set_var("GOOGLE_WORKSPACE_CLI_CLIENT_ID", "test-id"); std::env::set_var("GOOGLE_WORKSPACE_CLI_CLIENT_SECRET", "test-secret"); } - let result = resolve_client_credentials(); + let result = resolve_client_credentials().await; // Clean up immediately unsafe { @@ -1711,12 +1718,12 @@ mod tests { assert!(result.is_ok()); } - #[test] - fn credential_store_save_load_round_trip() { + #[tokio::test] + async fn credential_store_save_load_round_trip() { // Use encrypt/decrypt directly to avoid writing to the real config dir let json = r#"{"client_id":"test","client_secret":"secret","refresh_token":"tok"}"#; - let encrypted = credential_store::encrypt(json.as_bytes()).expect("encrypt should succeed"); - let decrypted = credential_store::decrypt(&encrypted).expect("decrypt should succeed"); + let encrypted = credential_store::encrypt(json.as_bytes()).await.expect("encrypt should succeed"); + let decrypted = credential_store::decrypt(&encrypted).await.expect("decrypt should succeed"); assert_eq!(String::from_utf8(decrypted).unwrap(), json); } diff --git a/src/commands.rs b/src/commands.rs index ecfe991..eddfa53 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -46,6 +46,13 @@ pub fn build_cli(doc: &RestDescription) -> Command { .help("Output format: json (default), table, yaml, csv") .value_name("FORMAT") .global(true), + ) + .arg( + clap::Arg::new("profile") + .long("profile") + .help("The configuration profile to use (e.g. default, work, personal)") + .value_name("PROFILE") + .global(true), ); // Inject helper commands diff --git a/src/credential_store.rs b/src/credential_store.rs index a3fa86a..2b96749 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -19,36 +19,34 @@ use aes_gcm::{AeadCore, Aes256Gcm, Nonce}; use keyring::Entry; use rand::RngCore; -use std::sync::OnceLock; + /// Returns the encryption key derived from the OS keyring, or falls back to a local file. /// Generates a random 256-bit key and stores it securely if it doesn't exist. -fn get_or_create_key() -> anyhow::Result<[u8; 32]> { - static KEY: OnceLock<[u8; 32]> = OnceLock::new(); +async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { + static KEY: tokio::sync::RwLock> = tokio::sync::RwLock::const_new(None); - if let Some(key) = KEY.get() { - return Ok(*key); + if let Some(key) = *KEY.read().await { + return Ok(key); } - let cache_key = |candidate: [u8; 32]| -> [u8; 32] { - if KEY.set(candidate).is_ok() { - candidate - } else { - // If set() fails, another thread already initialized the key. .get() is - // guaranteed to return Some at this point. - *KEY.get() - .expect("key must be initialized if OnceLock::set() failed") - } + let mut lock = KEY.write().await; + if let Some(key) = *lock { + return Ok(key); + } + + let mut cache_key = |candidate: [u8; 32]| -> [u8; 32] { + *lock = Some(candidate); + candidate }; let username = std::env::var("USER") .or_else(|_| std::env::var("USERNAME")) .unwrap_or_else(|_| "unknown-user".to_string()); - let key_file = crate::auth_commands::config_dir().join(".encryption_key"); - - let profile = crate::auth_commands::get_active_profile(); + let key_file = crate::auth_commands::config_dir().await.join(".encryption_key"); + let profile = crate::auth_commands::get_active_profile().await; let service_name = match profile.as_deref() { Some("default") | None => "gws-cli".to_string(), Some(name) => format!("gws-cli-{}", name), @@ -178,7 +176,7 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { } #[cfg(not(unix))] { - let _ = std::fs::write(&key_file, b64_key); + let _ = tokio::fs::write(&key_file, b64_key).await; } Ok(cache_key(key)) @@ -186,8 +184,8 @@ fn get_or_create_key() -> anyhow::Result<[u8; 32]> { /// Encrypts plaintext bytes using AES-256-GCM with a machine-derived key. /// Returns nonce (12 bytes) || ciphertext. -pub fn encrypt(plaintext: &[u8]) -> anyhow::Result> { - let key = get_or_create_key()?; +pub async fn encrypt(plaintext: &[u8]) -> anyhow::Result> { + let key = get_or_create_key().await?; let cipher = Aes256Gcm::new_from_slice(&key) .map_err(|e| anyhow::anyhow!("Failed to create cipher: {e}"))?; @@ -203,12 +201,12 @@ pub fn encrypt(plaintext: &[u8]) -> anyhow::Result> { } /// Decrypts data produced by `encrypt()`. -pub fn decrypt(data: &[u8]) -> anyhow::Result> { +pub async fn decrypt(data: &[u8]) -> anyhow::Result> { if data.len() < 12 { anyhow::bail!("Encrypted data too short"); } - let key = get_or_create_key()?; + let key = get_or_create_key().await?; let cipher = Aes256Gcm::new_from_slice(&key) .map_err(|e| anyhow::anyhow!("Failed to create cipher: {e}"))?; @@ -224,19 +222,21 @@ pub fn decrypt(data: &[u8]) -> anyhow::Result> { } /// Returns the path for encrypted credentials. -pub fn encrypted_credentials_path() -> PathBuf { - crate::auth_commands::config_dir().join("credentials.enc") +pub async fn encrypted_credentials_path() -> PathBuf { + crate::auth_commands::config_dir().await.join("credentials.enc") } /// Saves credentials JSON to an encrypted file. -pub fn save_encrypted(json: &str) -> anyhow::Result { - let path = encrypted_credentials_path(); +pub async fn save_encrypted(json: &str) -> anyhow::Result { + let path = encrypted_credentials_path().await; if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; + tokio::fs::create_dir_all(parent).await?; #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - if let Err(e) = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)) + let mut perms = tokio::fs::metadata(parent).await?.permissions(); + perms.set_mode(0o700); + if let Err(e) = tokio::fs::set_permissions(parent, perms).await { eprintln!( "Warning: failed to set directory permissions on {}: {e}", @@ -246,18 +246,20 @@ pub fn save_encrypted(json: &str) -> anyhow::Result { } } - let encrypted = encrypt(json.as_bytes())?; + let encrypted = encrypt(json.as_bytes()).await?; // Write atomically via a sibling .tmp file + rename so the credentials // file is never left in a corrupt partial-write state on crash/Ctrl-C. - crate::fs_util::atomic_write(&path, &encrypted) + crate::fs_util::atomic_write_async(&path, &encrypted).await .map_err(|e| anyhow::anyhow!("Failed to write credentials: {e}"))?; // Set permissions to 600 on Unix (contains secrets) #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - if let Err(e) = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)) { + let mut perms = tokio::fs::metadata(&path).await?.permissions(); + perms.set_mode(0o600); + if let Err(e) = tokio::fs::set_permissions(&path, perms).await { eprintln!( "Warning: failed to set file permissions on {}: {e}", path.display() @@ -269,78 +271,79 @@ pub fn save_encrypted(json: &str) -> anyhow::Result { } /// Loads and decrypts credentials JSON from a specific path. -pub fn load_encrypted_from_path(path: &std::path::Path) -> anyhow::Result { - let data = std::fs::read(path)?; - let plaintext = decrypt(&data)?; +pub async fn load_encrypted_from_path(path: &std::path::Path) -> anyhow::Result { + let data = tokio::fs::read(path).await?; + let plaintext = decrypt(&data).await?; Ok(String::from_utf8(plaintext)?) } /// Loads and decrypts credentials JSON from the default encrypted file. -pub fn load_encrypted() -> anyhow::Result { - load_encrypted_from_path(&encrypted_credentials_path()) +pub async fn load_encrypted() -> anyhow::Result { + let path = encrypted_credentials_path().await; + load_encrypted_from_path(&path).await } #[cfg(test)] mod tests { use super::*; - #[test] - fn get_or_create_key_is_deterministic() { - let key1 = get_or_create_key().unwrap(); - let key2 = get_or_create_key().unwrap(); + #[tokio::test] + async fn get_or_create_key_is_deterministic() { + let key1 = get_or_create_key().await.unwrap(); + let key2 = get_or_create_key().await.unwrap(); assert_eq!(key1, key2); } - #[test] - fn get_or_create_key_produces_256_bits() { - let key = get_or_create_key().unwrap(); + #[tokio::test] + async fn get_or_create_key_produces_256_bits() { + let key = get_or_create_key().await.unwrap(); assert_eq!(key.len(), 32); } - #[test] - fn encrypt_decrypt_round_trip() { + #[tokio::test] + async fn encrypt_decrypt_round_trip() { let plaintext = b"hello, world!"; - let encrypted = encrypt(plaintext).expect("encryption should succeed"); + let encrypted = encrypt(plaintext).await.expect("encryption should succeed"); assert_ne!(&encrypted, plaintext); assert_eq!(encrypted.len(), 12 + plaintext.len() + 16); - let decrypted = decrypt(&encrypted).expect("decryption should succeed"); + let decrypted = decrypt(&encrypted).await.expect("decryption should succeed"); assert_eq!(decrypted, plaintext); } - #[test] - fn encrypt_decrypt_empty() { + #[tokio::test] + async fn encrypt_decrypt_empty() { let plaintext = b""; - let encrypted = encrypt(plaintext).expect("encryption should succeed"); - let decrypted = decrypt(&encrypted).expect("decryption should succeed"); + let encrypted = encrypt(plaintext).await.expect("encryption should succeed"); + let decrypted = decrypt(&encrypted).await.expect("decryption should succeed"); assert_eq!(decrypted, plaintext); } - #[test] - fn decrypt_rejects_short_data() { - let result = decrypt(&[0u8; 11]); + #[tokio::test] + async fn decrypt_rejects_short_data() { + let result = decrypt(&[0u8; 11]).await; assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("too short")); } - #[test] - fn decrypt_rejects_tampered_ciphertext() { - let encrypted = encrypt(b"secret data").expect("encryption should succeed"); + #[tokio::test] + async fn decrypt_rejects_tampered_ciphertext() { + let encrypted = encrypt(b"secret data").await.expect("encryption should succeed"); let mut tampered = encrypted.clone(); if tampered.len() > 12 { tampered[12] ^= 0xFF; } - let result = decrypt(&tampered); + let result = decrypt(&tampered).await; assert!(result.is_err()); } - #[test] - fn each_encryption_produces_different_output() { + #[tokio::test] + async fn each_encryption_produces_different_output() { let plaintext = b"same input"; - let enc1 = encrypt(plaintext).expect("encryption should succeed"); - let enc2 = encrypt(plaintext).expect("encryption should succeed"); + let enc1 = encrypt(plaintext).await.expect("encryption should succeed"); + let enc2 = encrypt(plaintext).await.expect("encryption should succeed"); assert_ne!(enc1, enc2); - let dec1 = decrypt(&enc1).unwrap(); - let dec2 = decrypt(&enc2).unwrap(); + let dec1 = decrypt(&enc1).await.unwrap(); + let dec2 = decrypt(&enc2).await.unwrap(); assert_eq!(dec1, dec2); assert_eq!(dec1, plaintext); } diff --git a/src/discovery.rs b/src/discovery.rs index b4fa9ff..e8dd021 100644 --- a/src/discovery.rs +++ b/src/discovery.rs @@ -195,7 +195,7 @@ pub async fn fetch_discovery_document( let version = crate::validate::validate_api_identifier(version).map_err(|e| anyhow::anyhow!("{e}"))?; - let cache_dir = crate::auth_commands::config_dir().join("cache"); + let cache_dir = crate::auth_commands::config_dir().await.join("cache"); std::fs::create_dir_all(&cache_dir)?; let cache_file = cache_dir.join(format!("{service}_{version}.json")); diff --git a/src/main.rs b/src/main.rs index 42038d2..492c08b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -64,116 +64,81 @@ async fn run() -> Result<(), GwsError> { )); } - let mut filtered_args = vec![args[0].clone()]; - let mut first_arg: Option = None; - let mut profile_name: Option = None; - let mut i = 1; - while i < args.len() { - let a = &args[i]; - - if a == "--profile" { - if let Some(val) = args.get(i + 1) { - if !val.starts_with('-') { - if profile_name.is_none() { - profile_name = Some(val.clone()); - } - i += 2; // Skip --profile and its value - continue; - } - } - // Dangling --profile or followed by another flag. - i += 1; - continue; - } - if let Some(stripped) = a.strip_prefix("--profile=") { - if profile_name.is_none() { - profile_name = Some(stripped.to_string()); - } - i += 1; - continue; - } - - filtered_args.push(a.clone()); - i += 1; - } - - // Now find the first non-flag argument from the filtered list, skipping over --api-version. - { - let mut skip_next = false; - for a in filtered_args.iter().skip(1) { - if skip_next { - skip_next = false; - continue; - } - if a == "--api-version" { - skip_next = true; - continue; - } - if a.starts_with("--api-version=") { - continue; - } - if first_arg.is_none() && (!a.starts_with("--") || a == "--help" || a == "--version") { - first_arg = Some(a.clone()); - } - } - } - - if let Some(profile) = profile_name { - crate::auth_commands::validate_profile_name(&profile)?; + let globals_cli = clap::Command::new("gws") + .ignore_errors(true) + .allow_external_subcommands(true) + .arg(clap::Arg::new("profile").long("profile").num_args(1).global(true)) + .arg(clap::Arg::new("api-version").long("api-version").num_args(1).global(true)) + .arg(clap::Arg::new("format").long("format").num_args(1).global(true)) + .arg(clap::Arg::new("sanitize").long("sanitize").num_args(1).global(true)); + + let global_matches = globals_cli.try_get_matches_from(&args).unwrap_or_default(); + + if let Some(profile) = global_matches.get_one::("profile") { + crate::auth_commands::validate_profile_name(profile)?; std::env::set_var("GOOGLE_WORKSPACE_CLI_PROFILE", profile); } - // Use filtered_args for the rest of path resolution (helps `filter_args_for_subcommand`) - let args = filtered_args; + let first_arg = if let Some((cmd, _)) = global_matches.subcommand() { + Some(cmd.to_string()) + } else { + args.iter() + .skip(1) + .find(|a| *a == "--help" || *a == "--version" || *a == "-h" || *a == "-V") + .cloned() + }; - let first_arg = first_arg.ok_or_else(|| { + let first_arg_val = first_arg.ok_or_else(|| { GwsError::Validation( "No service specified. Usage: gws [sub-resource] [flags]" .to_string(), ) })?; - // Handle --help and --version at top level - if is_help_flag(&first_arg) { + if is_help_flag(&first_arg_val) { print_usage(); return Ok(()); } - if is_version_flag(&first_arg) { + if is_version_flag(&first_arg_val) { println!("gws {}", env!("CARGO_PKG_VERSION")); println!("This is not an officially supported Google product."); return Ok(()); } - // Handle the `schema` command - if first_arg == "schema" { - if args.len() < 3 { + let parse_external_args = || -> Vec { + global_matches + .subcommand() + .and_then(|(_, sub)| sub.get_many::("")) + .map(|vals| vals.map(|s| s.to_string_lossy().to_string()).collect()) + .unwrap_or_default() + }; + + if first_arg_val == "schema" { + let sub_args = parse_external_args(); + if sub_args.is_empty() { return Err(GwsError::Validation( "Usage: gws schema (e.g., gws schema drive.files.list) [--resolve-refs]" .to_string(), )); } - let resolve_refs = args.iter().any(|arg| arg == "--resolve-refs"); - // Remove the flag if it exists so it doesn't mess up path parsing, or just pass the path - // The path is args[2], flags might follow. - let path = &args[2]; + let resolve_refs = sub_args.iter().any(|arg| arg == "--resolve-refs"); + let path = &sub_args[0]; return schema::handle_schema_command(path, resolve_refs).await; } - // Handle the `generate-skills` command - if first_arg == "generate-skills" { - let gen_args: Vec = args.iter().skip(2).cloned().collect(); + if first_arg_val == "generate-skills" { + let gen_args = parse_external_args(); return generate_skills::handle_generate_skills(&gen_args).await; } - // Handle the `auth` command - if first_arg == "auth" { - let auth_args: Vec = args.iter().skip(2).cloned().collect(); + if first_arg_val == "auth" { + let auth_args = parse_external_args(); return auth_commands::handle_auth_command(&auth_args).await; } // Parse service name and optional version override - let (api_name, version) = parse_service_and_version(&args, &first_arg)?; + let (api_name, version) = parse_service_and_version(&args, &first_arg_val)?; // For synthetic services (no Discovery doc), use an empty RestDescription let doc = if api_name == "workflow" { @@ -195,7 +160,7 @@ async fn run() -> Result<(), GwsError> { // Re-parse args (skip argv[0] which is the binary, and argv[1] which is the service name) // Filter out --api-version and its value // Prepend "gws" as the program name since try_get_matches_from expects argv[0] - let sub_args = filter_args_for_subcommand(&args, &first_arg); + let sub_args = filter_args_for_subcommand(&args, &first_arg_val); let matches = cli.try_get_matches_from(&sub_args).map_err(|e| { // If it's a help or version display, print it and exit cleanly @@ -731,3 +696,4 @@ mod tests { assert_eq!(select_scope(&scopes), None); } } + diff --git a/src/oauth_config.rs b/src/oauth_config.rs index 02154b5..e442554 100644 --- a/src/oauth_config.rs +++ b/src/oauth_config.rs @@ -53,12 +53,12 @@ pub struct ClientSecretFile { } /// Returns the path for the client secret config file. -pub fn client_config_path() -> PathBuf { - crate::auth_commands::config_dir().join("client_secret.json") +pub async fn client_config_path() -> PathBuf { + crate::auth_commands::config_dir().await.join("client_secret.json") } /// Saves OAuth client configuration in the standard Google Cloud Console format. -pub fn save_client_config( +pub async fn save_client_config( client_id: &str, client_secret: &str, project_id: &str, @@ -75,29 +75,32 @@ pub fn save_client_config( }, }; - let path = client_config_path(); + let path = client_config_path().await; if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; + tokio::fs::create_dir_all(parent).await?; } let json = serde_json::to_string_pretty(&config)?; - crate::fs_util::atomic_write(&path, json.as_bytes()) + crate::fs_util::atomic_write_async(&path, json.as_bytes()).await .map_err(|e| anyhow::anyhow!("Failed to write client config: {e}"))?; // Set file permissions to 600 on Unix (contains secrets) #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600))?; + let mut perms = tokio::fs::metadata(&path).await?.permissions(); + perms.set_mode(0o600); + tokio::fs::set_permissions(&path, perms).await?; } Ok(path) } /// Loads OAuth client configuration from the standard Google Cloud Console format. -pub fn load_client_config() -> anyhow::Result { - let path = client_config_path(); - let data = std::fs::read_to_string(&path) +pub async fn load_client_config() -> anyhow::Result { + let path = client_config_path().await; + let data = tokio::fs::read_to_string(&path) + .await .map_err(|e| anyhow::anyhow!("Cannot read {}: {e}", path.display()))?; let file: ClientSecretFile = serde_json::from_str(&data) .map_err(|e| anyhow::anyhow!("Invalid client_secret.json format: {e}"))?; @@ -228,9 +231,9 @@ mod tests { } } - #[test] + #[tokio::test] #[serial_test::serial] - fn test_load_client_config() { + async fn test_load_client_config() { let dir = tempfile::tempdir().unwrap(); let _env_guard = EnvGuard::new( "GOOGLE_WORKSPACE_CLI_CONFIG_DIR", @@ -238,24 +241,24 @@ mod tests { ); // Initially no config file exists - let result = load_client_config(); + let result = load_client_config().await; let err = result.unwrap_err(); assert!(err.to_string().contains("Cannot read")); // Create a valid config file - save_client_config("test-id", "test-secret", "test-project").unwrap(); + save_client_config("test-id", "test-secret", "test-project").await.unwrap(); // Now loading should succeed - let config = load_client_config().unwrap(); + let config = load_client_config().await.unwrap(); assert_eq!(config.client_id, "test-id"); assert_eq!(config.client_secret, "test-secret"); assert_eq!(config.project_id, "test-project"); // Create an invalid config file - let path = client_config_path(); - std::fs::write(&path, "invalid json").unwrap(); + let path = client_config_path().await; + tokio::fs::write(&path, "invalid json").await.unwrap(); - let result = load_client_config(); + let result = load_client_config().await; let err = result.unwrap_err(); assert!(err .to_string() diff --git a/src/setup.rs b/src/setup.rs index a2730ec..755bf10 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -1265,7 +1265,7 @@ async fn stage_enable_apis(ctx: &mut SetupContext) -> Result String { +async fn manual_oauth_instructions(project_id: &str) -> String { let consent_url = if project_id.is_empty() { "https://console.cloud.google.com/apis/credentials/consent".to_string() } else { @@ -1316,7 +1316,7 @@ fn manual_oauth_instructions(project_id: &str) -> String { ), consent_url = consent_url, creds_url = creds_url, - config_path = crate::oauth_config::client_config_path().display() + config_path = crate::oauth_config::client_config_path().await.display().to_string() ) } @@ -1332,14 +1332,14 @@ async fn stage_configure_oauth(ctx: &mut SetupContext) -> Result = crate::credential_store::load_encrypted() - .ok() + .await.ok() .and_then(|s| serde_json::from_str(&s).ok()); w.show_message(&format!( @@ -1424,7 +1424,7 @@ async fn stage_configure_oauth(ctx: &mut SetupContext) -> Result Result<(), GwsError> { "apis_enabled": ctx.enabled.len(), "apis_skipped": ctx.skipped.len(), "apis_failed": ctx.failed.iter().map(|(api, err)| json!({"api": api, "error": err})).collect::>(), - "client_config": crate::oauth_config::client_config_path().display().to_string(), + "client_config": crate::oauth_config::client_config_path().await.display().to_string(), }); println!( "{}", diff --git a/src/token_storage.rs b/src/token_storage.rs index d6c5c47..520c2aa 100644 --- a/src/token_storage.rs +++ b/src/token_storage.rs @@ -40,7 +40,7 @@ impl EncryptedTokenStorage { Err(_) => return HashMap::new(), // File doesn't exist yet — normal on first run }; - let decrypted = match crate::credential_store::decrypt(&data) { + let decrypted = match crate::credential_store::decrypt(&data).await { Ok(d) => d, Err(e) => { eprintln!( @@ -71,7 +71,7 @@ impl EncryptedTokenStorage { async fn save_to_disk(&self, map: &HashMap) -> anyhow::Result<()> { let json = serde_json::to_string(map)?; - let encrypted = crate::credential_store::encrypt(json.as_bytes())?; + let encrypted = crate::credential_store::encrypt(json.as_bytes()).await?; if let Some(parent) = self.file_path.parent() { let _ = tokio::fs::create_dir_all(parent).await; From 576b0358101a728cded6aad8739a4584efe1ea20 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 07:15:03 +0000 Subject: [PATCH 22/33] fix(auth): secure TOCTOU vulnerability and replace blocking exists calls --- src/auth_commands.rs | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index efd90c7..a1710b5 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -96,22 +96,27 @@ pub async fn base_config_dir() -> PathBuf { let path = PathBuf::from(&dir); // Resolve symlinks to check the real path. If path doesn't exist, check the given path. - let path_to_check = tokio::fs::canonicalize(&path).await.unwrap_or_else(|_| path.clone()); - - // Check for suspicious paths like root, system directories, or .ssh - let is_suspicious = path_to_check.parent().is_none() - || path_to_check.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") - || (cfg!(unix) - && (path_to_check.starts_with("/etc") - || path_to_check.starts_with("/usr") - || path_to_check.starts_with("/var") - || path_to_check.starts_with("/bin") - || path_to_check.starts_with("/sbin"))); - - if is_suspicious { - eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); - } else { - return path_to_check; + match tokio::fs::canonicalize(&path).await { + Ok(path_to_check) => { + // Check for suspicious paths like root, system directories, or .ssh + let is_suspicious = path_to_check.parent().is_none() + || path_to_check.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") + || (cfg!(unix) + && (path_to_check.starts_with("/etc") + || path_to_check.starts_with("/usr") + || path_to_check.starts_with("/var") + || path_to_check.starts_with("/bin") + || path_to_check.starts_with("/sbin"))); + + if is_suspicious { + eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); + } else { + return path_to_check; + } + } + Err(e) => { + eprintln!("Warning: Could not resolve GOOGLE_WORKSPACE_CLI_CONFIG_DIR path '{}': {}. Using default.", path.display(), e); + } } } @@ -121,7 +126,7 @@ pub async fn base_config_dir() -> PathBuf { .join(".config") .join("gws"); - if primary.exists() { + if tokio::fs::metadata(&primary).await.is_ok() { primary } else { // Backward compat: fall back to OS-specific config dir for existing installs @@ -129,7 +134,7 @@ pub async fn base_config_dir() -> PathBuf { let legacy = dirs::config_dir() .unwrap_or_else(|| PathBuf::from(".")) .join("gws"); - if legacy.exists() { + if tokio::fs::metadata(&legacy).await.is_ok() { legacy } else { primary @@ -267,7 +272,7 @@ async fn handle_switch(args: &[String]) -> Result<(), GwsError> { // Read the base directory without applying the current active profile let base_dir = base_config_dir().await; - if !base_dir.exists() { + if tokio::fs::metadata(&base_dir).await.is_err() { tokio::fs::create_dir_all(&base_dir).await.map_err(|e| { GwsError::Validation(format!("Failed to create base config dir: {e}")) })?; From 9a9791610455e1a71280e974702d8afe0cc3985b Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 07:22:11 +0000 Subject: [PATCH 23/33] fix(auth): replace remaining std::fs blocking operations with tokio::fs in keyring access --- src/credential_store.rs | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/credential_store.rs b/src/credential_store.rs index 2b96749..05c7b0e 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -70,8 +70,8 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { use base64::{engine::general_purpose::STANDARD, Engine as _}; // If keyring is empty, prefer a persisted local key first. - if key_file.exists() { - if let Ok(b64_key) = std::fs::read_to_string(&key_file) { + if tokio::fs::metadata(&key_file).await.is_ok() { + if let Ok(b64_key) = tokio::fs::read_to_string(&key_file).await { if let Ok(decoded) = STANDARD.decode(b64_key.trim()) { if decoded.len() == 32 { let mut arr = [0u8; 32]; @@ -90,12 +90,12 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { let b64_key = STANDARD.encode(key); if let Some(parent) = key_file.parent() { - let _ = std::fs::create_dir_all(parent); + let _ = tokio::fs::create_dir_all(parent).await; #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; if let Err(e) = - std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)) + tokio::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)).await { eprintln!( "Warning: failed to set secure permissions on key directory: {e}" @@ -107,16 +107,17 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { #[cfg(unix)] { use std::os::unix::fs::OpenOptionsExt; - let mut options = std::fs::OpenOptions::new(); - options.write(true).create(true).truncate(true).mode(0o600); - if let Ok(mut file) = options.open(&key_file) { - use std::io::Write; - let _ = file.write_all(b64_key.as_bytes()); + let mut std_options = std::fs::OpenOptions::new(); + std_options.write(true).create(true).truncate(true).mode(0o600); + let options: tokio::fs::OpenOptions = std_options.into(); + if let Ok(mut file) = options.open(&key_file).await { + use tokio::io::AsyncWriteExt; + let _ = file.write_all(b64_key.as_bytes()).await; } } #[cfg(not(unix))] { - let _ = std::fs::write(&key_file, &b64_key); + let _ = tokio::fs::write(&key_file, &b64_key).await; } // Best effort: also store in keyring when available. @@ -132,8 +133,8 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { // Fallback: Local file `.encryption_key` - if key_file.exists() { - if let Ok(b64_key) = std::fs::read_to_string(&key_file) { + if tokio::fs::metadata(&key_file).await.is_ok() { + if let Ok(b64_key) = tokio::fs::read_to_string(&key_file).await { use base64::{engine::general_purpose::STANDARD, Engine as _}; if let Ok(decoded) = STANDARD.decode(b64_key.trim()) { if decoded.len() == 32 { @@ -153,11 +154,11 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { let b64_key = STANDARD.encode(key); if let Some(parent) = key_file.parent() { - let _ = std::fs::create_dir_all(parent); + let _ = tokio::fs::create_dir_all(parent).await; #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - if let Err(e) = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)) + if let Err(e) = tokio::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)).await { eprintln!("Warning: failed to set secure permissions on key directory: {e}"); } @@ -167,11 +168,12 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { #[cfg(unix)] { use std::os::unix::fs::OpenOptionsExt; - let mut options = std::fs::OpenOptions::new(); - options.write(true).create(true).truncate(true).mode(0o600); - if let Ok(mut file) = options.open(&key_file) { - use std::io::Write; - let _ = file.write_all(b64_key.as_bytes()); + let mut std_options = std::fs::OpenOptions::new(); + std_options.write(true).create(true).truncate(true).mode(0o600); + let options: tokio::fs::OpenOptions = std_options.into(); + if let Ok(mut file) = options.open(&key_file).await { + use tokio::io::AsyncWriteExt; + let _ = file.write_all(b64_key.as_bytes()).await; } } #[cfg(not(unix))] From d6d7e711462b0a8bc06e084b79a7de241865b6ce Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 07:26:44 +0000 Subject: [PATCH 24/33] fix(auth): handle non-existent GOOGLE_WORKSPACE_CLI_CONFIG_DIR gracefully --- src/auth_commands.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index a1710b5..5b1bcee 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -114,6 +114,23 @@ pub async fn base_config_dir() -> PathBuf { return path_to_check; } } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + // The directory doesn't exist yet, which is a valid use case. + // We can't canonicalize, so we'll run security checks on the provided path. + let is_suspicious = path.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") + || (cfg!(unix) + && (path.starts_with("/etc") + || path.starts_with("/usr") + || path.starts_with("/var") + || path.starts_with("/bin") + || path.starts_with("/sbin"))); + + if is_suspicious { + eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); + } else { + return path; // Return the user-provided path + } + } Err(e) => { eprintln!("Warning: Could not resolve GOOGLE_WORKSPACE_CLI_CONFIG_DIR path '{}': {}. Using default.", path.display(), e); } From 558af1d00d33e7246dcd356e29bcb57edf71cc06 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 07:34:31 +0000 Subject: [PATCH 25/33] fix(auth): safely modify unix permissions reading metadata prior to replacing them --- src/credential_store.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/credential_store.rs b/src/credential_store.rs index 05c7b0e..780c763 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -94,9 +94,13 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - if let Err(e) = - tokio::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)).await - { + let perms_result = async { + let mut perms = tokio::fs::metadata(parent).await?.permissions(); + perms.set_mode(0o700); + tokio::fs::set_permissions(parent, perms).await + } + .await; + if let Err(e) = perms_result { eprintln!( "Warning: failed to set secure permissions on key directory: {e}" ); @@ -158,8 +162,13 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - if let Err(e) = tokio::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)).await - { + let perms_result = async { + let mut perms = tokio::fs::metadata(parent).await?.permissions(); + perms.set_mode(0o700); + tokio::fs::set_permissions(parent, perms).await + } + .await; + if let Err(e) = perms_result { eprintln!("Warning: failed to set secure permissions on key directory: {e}"); } } From 87f738ecccd8db1a86de31ea17e87dced1286e71 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 07:39:58 +0000 Subject: [PATCH 26/33] fix(async): replace remaining std::fs blocking I/O calls with tokio::fs in discovery and token storage --- src/discovery.rs | 18 ++++++++---------- src/token_storage.rs | 10 +++++++++- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/discovery.rs b/src/discovery.rs index e8dd021..9832bcc 100644 --- a/src/discovery.rs +++ b/src/discovery.rs @@ -196,19 +196,17 @@ pub async fn fetch_discovery_document( crate::validate::validate_api_identifier(version).map_err(|e| anyhow::anyhow!("{e}"))?; let cache_dir = crate::auth_commands::config_dir().await.join("cache"); - std::fs::create_dir_all(&cache_dir)?; + tokio::fs::create_dir_all(&cache_dir).await?; let cache_file = cache_dir.join(format!("{service}_{version}.json")); // Check cache (24hr TTL) - if cache_file.exists() { - if let Ok(metadata) = std::fs::metadata(&cache_file) { - if let Ok(modified) = metadata.modified() { - if modified.elapsed().unwrap_or_default() < std::time::Duration::from_secs(86400) { - let data = std::fs::read_to_string(&cache_file)?; - let doc: RestDescription = serde_json::from_str(&data)?; - return Ok(doc); - } + if let Ok(metadata) = tokio::fs::metadata(&cache_file).await { + if let Ok(modified) = metadata.modified() { + if modified.elapsed().unwrap_or_default() < std::time::Duration::from_secs(86400) { + let data = tokio::fs::read_to_string(&cache_file).await?; + let doc: RestDescription = serde_json::from_str(&data)?; + return Ok(doc); } } } @@ -242,7 +240,7 @@ pub async fn fetch_discovery_document( }; // Write to cache - if let Err(e) = std::fs::write(&cache_file, &body) { + if let Err(e) = tokio::fs::write(&cache_file, &body).await { // Non-fatal: just warn via stderr-safe approach let _ = e; } diff --git a/src/token_storage.rs b/src/token_storage.rs index 520c2aa..c8c17e3 100644 --- a/src/token_storage.rs +++ b/src/token_storage.rs @@ -78,7 +78,15 @@ impl EncryptedTokenStorage { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - let _ = std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o700)); + let perms_result = async { + let mut perms = tokio::fs::metadata(parent).await?.permissions(); + perms.set_mode(0o700); + tokio::fs::set_permissions(parent, perms).await + } + .await; + if let Err(e) = perms_result { + eprintln!("warning: failed to set secure permissions on token storage directory: {e}"); + } } } From 0a4f5a2798ff8ec69a696aad465b09b9568ced47 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 07:45:15 +0000 Subject: [PATCH 27/33] fix(auth): add missing parent check to config path validation and standardize status JSON output --- src/auth_commands.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 5b1bcee..dac5dc2 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -117,7 +117,8 @@ pub async fn base_config_dir() -> PathBuf { Err(e) if e.kind() == std::io::ErrorKind::NotFound => { // The directory doesn't exist yet, which is a valid use case. // We can't canonicalize, so we'll run security checks on the provided path. - let is_suspicious = path.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") + let is_suspicious = path.parent().is_none() + || path.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") || (cfg!(unix) && (path.starts_with("/etc") || path.starts_with("/usr") @@ -1293,14 +1294,14 @@ async fn handle_status() -> Result<(), GwsError> { } } // end !cfg!(test) + // Determine the active profile + let profile = get_active_profile().await.unwrap_or_else(|| "default".to_string()); + output["active_profile"] = json!(profile); + println!( "{}", serde_json::to_string_pretty(&output).unwrap_or_default() ); - // Determine the active profile - let profile = get_active_profile().await.unwrap_or_else(|| "default".to_string()); - - println!("\nActive Profile: {profile}"); Ok(()) } From ea2e60052993b323452d5d88d8807cc0f7a87e34 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 07:53:30 +0000 Subject: [PATCH 28/33] fix(security): address TOCTOU canonical parent symlink vulnerability and prevent keyring IO lock contention --- src/auth_commands.rs | 27 +++++++++++++++-------- src/credential_store.rs | 48 +++++++++++++++++++++++++++++------------ 2 files changed, 52 insertions(+), 23 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index dac5dc2..ae19f42 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -116,20 +116,29 @@ pub async fn base_config_dir() -> PathBuf { } Err(e) if e.kind() == std::io::ErrorKind::NotFound => { // The directory doesn't exist yet, which is a valid use case. - // We can't canonicalize, so we'll run security checks on the provided path. - let is_suspicious = path.parent().is_none() - || path.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") + // To prevent TOCTOU symlink injection, we try to canonicalize the parent directory. + let mut path_to_check = path.clone(); + if let Some(parent) = path.parent() { + if let Ok(canon_parent) = tokio::fs::canonicalize(parent).await { + if let Some(file_name) = path.file_name() { + path_to_check = canon_parent.join(file_name); + } + } + } + + let is_suspicious = path_to_check.parent().is_none() + || path_to_check.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") || (cfg!(unix) - && (path.starts_with("/etc") - || path.starts_with("/usr") - || path.starts_with("/var") - || path.starts_with("/bin") - || path.starts_with("/sbin"))); + && (path_to_check.starts_with("/etc") + || path_to_check.starts_with("/usr") + || path_to_check.starts_with("/var") + || path_to_check.starts_with("/bin") + || path_to_check.starts_with("/sbin"))); if is_suspicious { eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); } else { - return path; // Return the user-provided path + return path_to_check; // Return the explicitly resolved path } } Err(e) => { diff --git a/src/credential_store.rs b/src/credential_store.rs index 780c763..ea43b68 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -30,15 +30,10 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { return Ok(key); } - let mut lock = KEY.write().await; - if let Some(key) = *lock { - return Ok(key); - } - - let mut cache_key = |candidate: [u8; 32]| -> [u8; 32] { - *lock = Some(candidate); - candidate - }; + // We do NOT acquire the write lock here because the subsequent file I/O operations + // could block other threads attempting concurrent profile accesses. + // Concurrency is resolved securely just before returning by checking whether + // another runtime resolved the key while we were generating ours. let username = std::env::var("USER") .or_else(|_| std::env::var("USERNAME")) @@ -62,7 +57,12 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); - return Ok(cache_key(arr)); + let mut write_lock = KEY.write().await; + if let Some(existing) = *write_lock { + return Ok(existing); + } + *write_lock = Some(arr); + return Ok(arr); } } } @@ -78,7 +78,12 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { arr.copy_from_slice(&decoded); // Best effort: repopulate keyring for future runs. let _ = entry.set_password(&b64_key); - return Ok(cache_key(arr)); + let mut write_lock = KEY.write().await; + if let Some(existing) = *write_lock { + return Ok(existing); + } + *write_lock = Some(arr); + return Ok(arr); } } } @@ -127,7 +132,12 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { // Best effort: also store in keyring when available. let _ = entry.set_password(&b64_key); - return Ok(cache_key(key)); + let mut write_lock = KEY.write().await; + if let Some(existing) = *write_lock { + return Ok(existing); + } + *write_lock = Some(key); + return Ok(key); } Err(e) => { eprintln!("Warning: keyring access failed, falling back to file storage: {e}"); @@ -144,7 +154,12 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); - return Ok(cache_key(arr)); + let mut write_lock = KEY.write().await; + if let Some(existing) = *write_lock { + return Ok(existing); + } + *write_lock = Some(arr); + return Ok(arr); } } } @@ -190,7 +205,12 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { let _ = tokio::fs::write(&key_file, b64_key).await; } - Ok(cache_key(key)) + let mut write_lock = KEY.write().await; + if let Some(existing) = *write_lock { + return Ok(existing); + } + *write_lock = Some(key); + Ok(key) } /// Encrypts plaintext bytes using AES-256-GCM with a machine-derived key. From dd458385e5061ba3ebb284de638896cae8dec38f Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 16:44:20 +0000 Subject: [PATCH 29/33] refactor: extract duplicated security checks, simplify async I/O token blocks, and consolidate keyring cache write-locks --- src/auth_commands.rs | 46 +++++++++++++++++----------------------- src/credential_store.rs | 47 +++++++++++++++-------------------------- 2 files changed, 36 insertions(+), 57 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index ae19f42..7728b8b 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -99,16 +99,7 @@ pub async fn base_config_dir() -> PathBuf { match tokio::fs::canonicalize(&path).await { Ok(path_to_check) => { // Check for suspicious paths like root, system directories, or .ssh - let is_suspicious = path_to_check.parent().is_none() - || path_to_check.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") - || (cfg!(unix) - && (path_to_check.starts_with("/etc") - || path_to_check.starts_with("/usr") - || path_to_check.starts_with("/var") - || path_to_check.starts_with("/bin") - || path_to_check.starts_with("/sbin"))); - - if is_suspicious { + if is_suspicious_path(&path_to_check) { eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); } else { return path_to_check; @@ -126,16 +117,7 @@ pub async fn base_config_dir() -> PathBuf { } } - let is_suspicious = path_to_check.parent().is_none() - || path_to_check.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") - || (cfg!(unix) - && (path_to_check.starts_with("/etc") - || path_to_check.starts_with("/usr") - || path_to_check.starts_with("/var") - || path_to_check.starts_with("/bin") - || path_to_check.starts_with("/sbin"))); - - if is_suspicious { + if is_suspicious_path(&path_to_check) { eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); } else { return path_to_check; // Return the explicitly resolved path @@ -169,6 +151,17 @@ pub async fn base_config_dir() -> PathBuf { } } +fn is_suspicious_path(path_to_check: &std::path::Path) -> bool { + path_to_check.parent().is_none() + || path_to_check.components().any(|c| c.as_os_str() == ".." || c.as_os_str() == ".ssh") + || (cfg!(unix) + && (path_to_check.starts_with("/etc") + || path_to_check.starts_with("/usr") + || path_to_check.starts_with("/var") + || path_to_check.starts_with("/bin") + || path_to_check.starts_with("/sbin"))) +} + pub async fn get_active_profile() -> Option { if let Some(s) = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") .ok() @@ -468,14 +461,13 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { if token.token().is_some() { // Read yup-oauth2's token cache to extract the refresh_token. - let mut token_data = String::new(); - if let Ok(bytes) = tokio::fs::read(&temp_path).await { - if let Ok(decrypted) = crate::credential_store::decrypt(&bytes).await { - if let Ok(s) = String::from_utf8(decrypted) { - token_data = s; - } - } + let token_data = async { + let bytes = tokio::fs::read(&temp_path).await.ok()?; + let decrypted = crate::credential_store::decrypt(&bytes).await.ok()?; + String::from_utf8(decrypted).ok() } + .await + .unwrap_or_default(); let refresh_token = extract_refresh_token(&token_data).ok_or_else(|| { GwsError::Auth( "OAuth flow completed but no refresh token was returned. \ diff --git a/src/credential_store.rs b/src/credential_store.rs index ea43b68..818bb50 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -35,6 +35,18 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { // Concurrency is resolved securely just before returning by checking whether // another runtime resolved the key while we were generating ours. + macro_rules! cache_and_return { + ($candidate:expr) => {{ + let mut write_lock = KEY.write().await; + if let Some(existing) = *write_lock { + existing + } else { + *write_lock = Some($candidate); + $candidate + } + }}; + } + let username = std::env::var("USER") .or_else(|_| std::env::var("USERNAME")) .unwrap_or_else(|_| "unknown-user".to_string()); @@ -57,12 +69,7 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); - let mut write_lock = KEY.write().await; - if let Some(existing) = *write_lock { - return Ok(existing); - } - *write_lock = Some(arr); - return Ok(arr); + return Ok(cache_and_return!(arr)); } } } @@ -78,12 +85,7 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { arr.copy_from_slice(&decoded); // Best effort: repopulate keyring for future runs. let _ = entry.set_password(&b64_key); - let mut write_lock = KEY.write().await; - if let Some(existing) = *write_lock { - return Ok(existing); - } - *write_lock = Some(arr); - return Ok(arr); + return Ok(cache_and_return!(arr)); } } } @@ -132,12 +134,7 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { // Best effort: also store in keyring when available. let _ = entry.set_password(&b64_key); - let mut write_lock = KEY.write().await; - if let Some(existing) = *write_lock { - return Ok(existing); - } - *write_lock = Some(key); - return Ok(key); + return Ok(cache_and_return!(key)); } Err(e) => { eprintln!("Warning: keyring access failed, falling back to file storage: {e}"); @@ -154,12 +151,7 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); - let mut write_lock = KEY.write().await; - if let Some(existing) = *write_lock { - return Ok(existing); - } - *write_lock = Some(arr); - return Ok(arr); + return Ok(cache_and_return!(arr)); } } } @@ -205,12 +197,7 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { let _ = tokio::fs::write(&key_file, b64_key).await; } - let mut write_lock = KEY.write().await; - if let Some(existing) = *write_lock { - return Ok(existing); - } - *write_lock = Some(key); - Ok(key) + Ok(cache_and_return!(key)) } /// Encrypts plaintext bytes using AES-256-GCM with a machine-derived key. From a8a46207e0988a8fa7327db9a39c983a007994f4 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 16:49:53 +0000 Subject: [PATCH 30/33] fix(security): resolve TOCTOU config directory race condition via proactive creation --- src/auth_commands.rs | 45 ++++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 7728b8b..a258be9 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -95,36 +95,27 @@ pub async fn base_config_dir() -> PathBuf { if let Ok(dir) = std::env::var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR") { let path = PathBuf::from(&dir); - // Resolve symlinks to check the real path. If path doesn't exist, check the given path. - match tokio::fs::canonicalize(&path).await { - Ok(path_to_check) => { - // Check for suspicious paths like root, system directories, or .ssh - if is_suspicious_path(&path_to_check) { - eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); - } else { - return path_to_check; - } - } - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - // The directory doesn't exist yet, which is a valid use case. - // To prevent TOCTOU symlink injection, we try to canonicalize the parent directory. - let mut path_to_check = path.clone(); - if let Some(parent) = path.parent() { - if let Ok(canon_parent) = tokio::fs::canonicalize(parent).await { - if let Some(file_name) = path.file_name() { - path_to_check = canon_parent.join(file_name); + // First, do a basic check for ".." to prevent trivial traversal. + if path.components().any(|c| c.as_os_str() == "..") { + eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains '..'. Using default."); + } else { + // Create the directory if it doesn't exist. This is the key to preventing TOCTOU. + if let Err(e) = tokio::fs::create_dir_all(&path).await { + eprintln!("Warning: Could not create GOOGLE_WORKSPACE_CLI_CONFIG_DIR '{}': {}. Using default.", path.display(), e); + } else { + // Now that the path is guaranteed to exist, we can safely canonicalize it. + match tokio::fs::canonicalize(&path).await { + Ok(canonical_path) => { + if is_suspicious_path(&canonical_path) { + eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR resolves to a restricted or sensitive path ({}). Using default.", canonical_path.display()); + } else { + return canonical_path; } } + Err(e) => { + eprintln!("Warning: Could not resolve GOOGLE_WORKSPACE_CLI_CONFIG_DIR path '{}': {}. Using default.", path.display(), e); + } } - - if is_suspicious_path(&path_to_check) { - eprintln!("Warning: GOOGLE_WORKSPACE_CLI_CONFIG_DIR contains a restricted or sensitive path ({}). Using default.", path.display()); - } else { - return path_to_check; // Return the explicitly resolved path - } - } - Err(e) => { - eprintln!("Warning: Could not resolve GOOGLE_WORKSPACE_CLI_CONFIG_DIR path '{}': {}. Using default.", path.display(), e); } } } From fca3f35e9e0413e5186b553964295b925e7368b5 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 16:58:28 +0000 Subject: [PATCH 31/33] fix(cli): replace thread-unsafe std::env::set_var with OnceLock for profile argument propagation --- src/auth_commands.rs | 8 ++++++++ src/main.rs | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/auth_commands.rs b/src/auth_commands.rs index a258be9..6e60304 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -153,7 +153,15 @@ fn is_suspicious_path(path_to_check: &std::path::Path) -> bool { || path_to_check.starts_with("/sbin"))) } +pub static OVERRIDE_PROFILE: std::sync::OnceLock = std::sync::OnceLock::new(); + pub async fn get_active_profile() -> Option { + // 1. Check globally injected CLI argument (thread-safe, avoids env data races) + if let Some(cli_profile) = OVERRIDE_PROFILE.get().cloned() { + return Some(cli_profile); + } + + // 2. Fallback to reading the environment variable natively without local mutations if let Some(s) = std::env::var("GOOGLE_WORKSPACE_CLI_PROFILE") .ok() .filter(|s| !s.is_empty()) diff --git a/src/main.rs b/src/main.rs index 492c08b..b19f501 100644 --- a/src/main.rs +++ b/src/main.rs @@ -76,7 +76,7 @@ async fn run() -> Result<(), GwsError> { if let Some(profile) = global_matches.get_one::("profile") { crate::auth_commands::validate_profile_name(profile)?; - std::env::set_var("GOOGLE_WORKSPACE_CLI_PROFILE", profile); + let _ = crate::auth_commands::OVERRIDE_PROFILE.set(profile.clone()); } let first_arg = if let Some((cmd, _)) = global_matches.subcommand() { From d8f2ab1784a75fcfa7c1fbe7e533f7b550a63f00 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 17:06:42 +0000 Subject: [PATCH 32/33] refactor: replace static RwLock cache with idiomatic tokio::sync::OnceCell for credential key initialization --- src/credential_store.rs | 38 +++++++++++--------------------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/src/credential_store.rs b/src/credential_store.rs index 818bb50..7480b9f 100644 --- a/src/credential_store.rs +++ b/src/credential_store.rs @@ -21,32 +21,16 @@ use keyring::Entry; use rand::RngCore; +static KEY: tokio::sync::OnceCell<[u8; 32]> = tokio::sync::OnceCell::const_new(); + /// Returns the encryption key derived from the OS keyring, or falls back to a local file. /// Generates a random 256-bit key and stores it securely if it doesn't exist. async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { - static KEY: tokio::sync::RwLock> = tokio::sync::RwLock::const_new(None); - - if let Some(key) = *KEY.read().await { - return Ok(key); - } - - // We do NOT acquire the write lock here because the subsequent file I/O operations - // could block other threads attempting concurrent profile accesses. - // Concurrency is resolved securely just before returning by checking whether - // another runtime resolved the key while we were generating ours. - - macro_rules! cache_and_return { - ($candidate:expr) => {{ - let mut write_lock = KEY.write().await; - if let Some(existing) = *write_lock { - existing - } else { - *write_lock = Some($candidate); - $candidate - } - }}; - } + let key = KEY.get_or_try_init(generate_key_logic).await?; + Ok(*key) +} +async fn generate_key_logic() -> anyhow::Result<[u8; 32]> { let username = std::env::var("USER") .or_else(|_| std::env::var("USERNAME")) .unwrap_or_else(|_| "unknown-user".to_string()); @@ -69,7 +53,7 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); - return Ok(cache_and_return!(arr)); + return Ok(arr); } } } @@ -85,7 +69,7 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { arr.copy_from_slice(&decoded); // Best effort: repopulate keyring for future runs. let _ = entry.set_password(&b64_key); - return Ok(cache_and_return!(arr)); + return Ok(arr); } } } @@ -134,7 +118,7 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { // Best effort: also store in keyring when available. let _ = entry.set_password(&b64_key); - return Ok(cache_and_return!(key)); + return Ok(key); } Err(e) => { eprintln!("Warning: keyring access failed, falling back to file storage: {e}"); @@ -151,7 +135,7 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { if decoded.len() == 32 { let mut arr = [0u8; 32]; arr.copy_from_slice(&decoded); - return Ok(cache_and_return!(arr)); + return Ok(arr); } } } @@ -197,7 +181,7 @@ async fn get_or_create_key() -> anyhow::Result<[u8; 32]> { let _ = tokio::fs::write(&key_file, b64_key).await; } - Ok(cache_and_return!(key)) + Ok(key) } /// Encrypts plaintext bytes using AES-256-GCM with a machine-derived key. From 7238c0da9a0a80f69b2bd315147f1c94a315e983 Mon Sep 17 00:00:00 2001 From: joeVenner Date: Sat, 7 Mar 2026 17:14:38 +0000 Subject: [PATCH 33/33] fix(security): resolve final sync I/O traces across async credential scopes and harden test isolation --- src/auth.rs | 10 +++++----- src/auth_commands.rs | 23 +++++++++++++++-------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index d9f76dd..0594d6e 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -175,7 +175,7 @@ async fn load_credentials_inner( // 1. Explicit env var — plaintext file (User or Service Account) if let Some(path) = env_file { let p = PathBuf::from(path); - if p.exists() { + if tokio::fs::metadata(&p).await.is_ok() { let content = tokio::fs::read_to_string(&p) .await .with_context(|| format!("Failed to read credentials from {path}"))?; @@ -187,7 +187,7 @@ async fn load_credentials_inner( } // 2. Encrypted credentials (always AuthorizedUser for now) - if enc_path.exists() { + if tokio::fs::metadata(enc_path).await.is_ok() { let json_str = credential_store::load_encrypted_from_path(enc_path) .await .context("Failed to decrypt credentials")?; @@ -217,7 +217,7 @@ async fn load_credentials_inner( } // 3. Plaintext credentials at default path (Default to AuthorizedUser) - if default_path.exists() { + if tokio::fs::metadata(default_path).await.is_ok() { return Ok(Credential::AuthorizedUser( yup_oauth2::read_authorized_user_secret(default_path) .await @@ -230,7 +230,7 @@ async fn load_credentials_inner( // 4a. GOOGLE_APPLICATION_CREDENTIALS env var (explicit path — hard error if missing) if let Ok(adc_env) = std::env::var("GOOGLE_APPLICATION_CREDENTIALS") { let adc_path = PathBuf::from(&adc_env); - if adc_path.exists() { + if tokio::fs::metadata(&adc_path).await.is_ok() { let content = tokio::fs::read_to_string(&adc_path) .await .with_context(|| format!("Failed to read ADC from {adc_env}"))?; @@ -244,7 +244,7 @@ async fn load_credentials_inner( // 4b. Well-known ADC path: ~/.config/gcloud/application_default_credentials.json // (populated by `gcloud auth application-default login`). Silent if absent. if let Some(well_known) = adc_well_known_path() { - if well_known.exists() { + if tokio::fs::metadata(&well_known).await.is_ok() { let content = tokio::fs::read_to_string(&well_known) .await .with_context(|| format!("Failed to read ADC from {}", well_known.display()))?; diff --git a/src/auth_commands.rs b/src/auth_commands.rs index 6e60304..2c44e24 100644 --- a/src/auth_commands.rs +++ b/src/auth_commands.rs @@ -430,11 +430,11 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { let temp_path = config_dir().await.join("credentials.tmp"); // Always start fresh — delete any stale temp cache from prior login attempts. - let _ = std::fs::remove_file(&temp_path); + let _ = tokio::fs::remove_file(&temp_path).await; // Ensure config directory exists if let Some(parent) = temp_path.parent() { - std::fs::create_dir_all(parent) + tokio::fs::create_dir_all(parent).await .map_err(|e| GwsError::Validation(format!("Failed to create config directory: {e}")))?; } @@ -496,7 +496,7 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { .map_err(|e| GwsError::Auth(format!("Failed to encrypt credentials: {e}")))?; // Clean up temp file - let _ = std::fs::remove_file(&temp_path); + let _ = tokio::fs::remove_file(&temp_path).await; let output = json!({ "status": "success", @@ -513,7 +513,7 @@ async fn handle_login(args: &[String]) -> Result<(), GwsError> { Ok(()) } else { // Clean up temp file on failure - let _ = std::fs::remove_file(&temp_path); + let _ = tokio::fs::remove_file(&temp_path).await; Err(GwsError::Auth( "OAuth flow completed but no token was returned.".to_string(), )) @@ -543,7 +543,7 @@ async fn fetch_userinfo_email(access_token: &str) -> Option { async fn handle_export(unmasked: bool) -> Result<(), GwsError> { let enc_path = credential_store::encrypted_credentials_path().await; - if !enc_path.exists() { + if tokio::fs::metadata(&enc_path).await.is_err() { return Err(GwsError::Auth( "No encrypted credentials found. Run 'gws auth login' first.".to_string(), )); @@ -1315,8 +1315,8 @@ async fn handle_logout() -> Result<(), GwsError> { let mut removed = Vec::new(); for path in [&enc_path, &plain_path, &token_cache, &sa_token_cache] { - if path.exists() { - std::fs::remove_file(path).map_err(|e| { + if tokio::fs::metadata(path).await.is_ok() { + tokio::fs::remove_file(path).await.map_err(|e| { GwsError::Validation(format!("Failed to remove {}: {e}", path.display())) })?; removed.push(path.display().to_string()); @@ -1653,10 +1653,17 @@ mod tests { } #[tokio::test] + #[serial_test::serial] async fn token_cache_path_is_in_config_dir() { + let temp = tempfile::tempdir().unwrap(); + unsafe { std::env::set_var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR", temp.path()); } + + let c_dir = config_dir().await; let path = token_cache_path().await; assert!(path.ends_with("token_cache.json")); - assert!(path.starts_with(config_dir().await)); + assert!(path.starts_with(&c_dir)); + + unsafe { std::env::remove_var("GOOGLE_WORKSPACE_CLI_CONFIG_DIR"); } } #[tokio::test]