From c6dd35b65f067a0d1486dce126109d0af790c9cc Mon Sep 17 00:00:00 2001 From: Hoyt Harness <2735828+hoyt-harness@users.noreply.github.com> Date: Wed, 22 Apr 2026 12:10:39 -0500 Subject: [PATCH 1/2] fix(auth): validate --services names and remove automatic cloud-platform injection - `parse_login_args()` now validates each token in `-s`/`--services` against the known service registry (`crate::services::SERVICES`) and returns a clear error listing valid names before opening the browser. Tokens are lowercased before checking so `Drive` and `drive` both work. - `scope_matches_service()` no longer unconditionally returns `true` for `cloud-platform`. The scope is now filtered the same way as any other scope, so `-s drive` no longer silently injects cloud-platform into the token. - `run_discovery_scope_picker()` no longer forces cloud-platform into the result when a services filter is active. It is still auto-added in the unfiltered interactive flow (no `-s` flag). - Fixes pre-existing `clippy::collapsible_match` in `helpers/script.rs`. Closes #741. --- .../fix-741-auth-services-validation.md | 7 ++ .../google-workspace-cli/src/auth_commands.rs | 99 ++++++++++++++----- .../src/helpers/script.rs | 8 +- 3 files changed, 83 insertions(+), 31 deletions(-) create mode 100644 .changeset/fix-741-auth-services-validation.md diff --git a/.changeset/fix-741-auth-services-validation.md b/.changeset/fix-741-auth-services-validation.md new file mode 100644 index 00000000..8e361720 --- /dev/null +++ b/.changeset/fix-741-auth-services-validation.md @@ -0,0 +1,7 @@ +--- +"@googleworkspace/cli": patch +--- + +`auth login -s` / `--services`: reject unknown service names at parse time with a clear error listing valid names. Previously, unrecognised tokens were silently dropped, causing a token to cover fewer services than the user intended. + +**Behavior change:** `cloud-platform` is no longer injected automatically when a `--services` filter is active. It was previously added to every filtered login regardless of the services requested. It still appears when using `--full` (no filter) or the interactive discovery picker (no filter). diff --git a/crates/google-workspace-cli/src/auth_commands.rs b/crates/google-workspace-cli/src/auth_commands.rs index d7571e74..cd0fc8e2 100644 --- a/crates/google-workspace-cli/src/auth_commands.rs +++ b/crates/google-workspace-cli/src/auth_commands.rs @@ -448,7 +448,7 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> { match matches.subcommand() { Some(("login", sub_m)) => { - let (scope_mode, services_filter) = parse_login_args(sub_m); + let (scope_mode, services_filter) = parse_login_args(sub_m)?; handle_login_inner(scope_mode, services_filter).await } @@ -483,7 +483,11 @@ fn login_command() -> clap::Command { } /// Extract `ScopeMode` and optional services filter from parsed login args. -fn parse_login_args(matches: &clap::ArgMatches) -> (ScopeMode, Option>) { +/// +/// Returns `Err` if any token in `--services` is not a known service alias. +fn parse_login_args( + matches: &clap::ArgMatches, +) -> Result<(ScopeMode, Option>), GwsError> { let scope_mode = if let Some(scopes_str) = matches.get_one::("scopes") { ScopeMode::Custom( scopes_str @@ -501,14 +505,42 @@ fn parse_login_args(matches: &clap::ArgMatches) -> (ScopeMode, Option> = matches.get_one::("services").map(|v| { - v.split(',') - .map(|s| s.trim().to_lowercase()) - .filter(|s| !s.is_empty()) - .collect() - }); + let services_filter: Option> = + if let Some(v) = matches.get_one::("services") { + let tokens: Vec = v + .split(',') + .map(|s| s.trim().to_lowercase()) + .filter(|s| !s.is_empty()) + .collect(); - (scope_mode, services_filter) + let unknown: Vec<&str> = tokens + .iter() + .filter(|name| { + !crate::services::SERVICES + .iter() + .any(|e| e.aliases.contains(&name.as_str())) + }) + .map(|s| s.as_str()) + .collect(); + + if !unknown.is_empty() { + let valid: Vec<&str> = crate::services::SERVICES + .iter() + .flat_map(|e| e.aliases.iter().copied()) + .collect(); + return Err(GwsError::Validation(format!( + "Unknown service(s): {}. Valid services: {}.", + unknown.join(", "), + valid.join(", ") + ))); + } + + Some(tokens.into_iter().collect()) + } else { + None + }; + + Ok((scope_mode, services_filter)) } /// Run the `auth login` flow. @@ -532,7 +564,7 @@ pub async fn run_login(args: &[String]) -> Result<(), GwsError> { Err(e) => return Err(GwsError::Validation(e.to_string())), }; - let (scope_mode, services_filter) = parse_login_args(&matches); + let (scope_mode, services_filter) = parse_login_args(&matches)?; handle_login_inner(scope_mode, services_filter).await } @@ -817,11 +849,6 @@ fn scope_matches_service(scope_url: &str, services: &HashSet) -> bool { .strip_prefix("https://www.googleapis.com/auth/") .unwrap_or(scope_url); - // cloud-platform is a cross-service scope, always include - if short == "cloud-platform" { - return true; - } - let prefix = short.split('.').next().unwrap_or(short); services.iter().any(|svc| { @@ -1102,8 +1129,9 @@ fn run_discovery_scope_picker( } } - // Always include cloud-platform scope - if !selected.contains(&PLATFORM_SCOPE.to_string()) { + // Include cloud-platform when no services filter is active. When the + // user passes -s, they get exactly the services they asked for. + if services_filter.is_none() && !selected.contains(&PLATFORM_SCOPE.to_string()) { selected.push(PLATFORM_SCOPE.to_string()); } @@ -2184,9 +2212,9 @@ mod tests { } #[test] - fn scope_matches_service_cloud_platform_always_matches() { + fn scope_matches_service_cloud_platform_does_not_match_unrelated_service() { let services: HashSet = ["drive"].iter().map(|s| s.to_string()).collect(); - assert!(scope_matches_service( + assert!(!scope_matches_service( "https://www.googleapis.com/auth/cloud-platform", &services )); @@ -2248,9 +2276,7 @@ mod tests { .strip_prefix("https://www.googleapis.com/auth/") .unwrap_or(scope); assert!( - short.starts_with("drive") - || short.starts_with("gmail") - || short == "cloud-platform", + short.starts_with("drive") || short.starts_with("gmail"), "Unexpected scope with service filter: {scope}" ); } @@ -2274,7 +2300,7 @@ mod tests { .strip_prefix("https://www.googleapis.com/auth/") .unwrap_or(scope); assert!( - short.starts_with("drive") || short == "cloud-platform", + short.starts_with("drive"), "Unexpected scope with service + readonly filter: {scope}" ); } @@ -2289,7 +2315,7 @@ mod tests { .strip_prefix("https://www.googleapis.com/auth/") .unwrap_or(scope); assert!( - short.starts_with("gmail") || short == "cloud-platform", + short.starts_with("gmail"), "Unexpected scope with service + full filter: {scope}" ); } @@ -2307,6 +2333,31 @@ mod tests { assert_eq!(scopes[0], "https://www.googleapis.com/auth/calendar"); } + #[test] + fn parse_login_args_rejects_unknown_service() { + let cmd = login_command(); + let matches = cmd + .try_get_matches_from(["login", "--services", "drive,typoservice"]) + .unwrap(); + let result = parse_login_args(&matches); + assert!(result.is_err()); + let msg = result.err().expect("expected error").to_string(); + assert!(msg.contains("typoservice"), "error should name the unknown token: {msg}"); + assert!(msg.contains("drive"), "error should list valid services: {msg}"); + } + + #[test] + fn parse_login_args_accepts_known_services() { + let cmd = login_command(); + let matches = cmd + .try_get_matches_from(["login", "--services", "Drive,Gmail"]) + .unwrap(); + let (_, filter) = parse_login_args(&matches).unwrap(); + let services = filter.unwrap(); + assert!(services.contains("drive"), "should lowercase: {services:?}"); + assert!(services.contains("gmail"), "should lowercase: {services:?}"); + } + #[test] fn filter_scopes_by_services_none_returns_all() { let scopes = vec![ diff --git a/crates/google-workspace-cli/src/helpers/script.rs b/crates/google-workspace-cli/src/helpers/script.rs index 11bcdebe..4b31db62 100644 --- a/crates/google-workspace-cli/src/helpers/script.rs +++ b/crates/google-workspace-cli/src/helpers/script.rs @@ -169,13 +169,7 @@ fn process_file(path: &Path) -> Result, GwsError> { filename.trim_end_matches(".js").trim_end_matches(".gs"), ), "html" => ("HTML", filename.trim_end_matches(".html")), - "json" => { - if filename == "appsscript.json" { - ("JSON", "appsscript") - } else { - return Ok(None); - } - } + "json" if filename == "appsscript.json" => ("JSON", "appsscript"), _ => return Ok(None), }; From 966294089b368e45bc4ffe9fff0ed57a4f3c61ab Mon Sep 17 00:00:00 2001 From: Hoyt Harness <2735828+hoyt-harness@users.noreply.github.com> Date: Wed, 22 Apr 2026 16:45:03 -0500 Subject: [PATCH 2/2] fix(auth): allow cloud-platform/pubsub in --services and sanitize error output Two issues from PR review: 1. Validation was too restrictive: cloud-platform and pubsub are valid requestable scopes (present in SCOPE_ENTRIES / FULL_SCOPES) but absent from the Workspace service registry (SERVICES), so they were incorrectly rejected by parse_login_args(). Since cloud-platform is no longer auto-injected when a filter is active, this made it impossible to request that scope via -s. Added PLATFORM_SCOPES allowlist for these two. 2. User-supplied tokens appeared unsanitized in the error message, allowing escape-sequence injection. Apply .escape_debug() on unknown tokens. Add test: parse_login_args_accepts_platform_scopes. --- .../google-workspace-cli/src/auth_commands.rs | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/crates/google-workspace-cli/src/auth_commands.rs b/crates/google-workspace-cli/src/auth_commands.rs index cd0fc8e2..61f97b67 100644 --- a/crates/google-workspace-cli/src/auth_commands.rs +++ b/crates/google-workspace-cli/src/auth_commands.rs @@ -513,24 +513,36 @@ fn parse_login_args( .filter(|s| !s.is_empty()) .collect(); + // Platform scopes recognised by the tool but not registered as + // Workspace API services (they lack an API discovery entry). + const PLATFORM_SCOPES: &[&str] = &["cloud-platform", "pubsub"]; + let unknown: Vec<&str> = tokens .iter() .filter(|name| { - !crate::services::SERVICES - .iter() - .any(|e| e.aliases.contains(&name.as_str())) + let n = name.as_str(); + !PLATFORM_SCOPES.contains(&n) + && !crate::services::SERVICES + .iter() + .any(|e| e.aliases.contains(&n)) }) .map(|s| s.as_str()) .collect(); if !unknown.is_empty() { - let valid: Vec<&str> = crate::services::SERVICES + let mut valid: Vec<&str> = crate::services::SERVICES .iter() .flat_map(|e| e.aliases.iter().copied()) .collect(); + valid.extend(PLATFORM_SCOPES); + valid.sort_unstable(); return Err(GwsError::Validation(format!( "Unknown service(s): {}. Valid services: {}.", - unknown.join(", "), + unknown + .iter() + .map(|s| s.escape_debug().to_string()) + .collect::>() + .join(", "), valid.join(", ") ))); } @@ -2358,6 +2370,24 @@ mod tests { assert!(services.contains("gmail"), "should lowercase: {services:?}"); } + #[test] + fn parse_login_args_accepts_platform_scopes() { + let cmd = login_command(); + let matches = cmd + .try_get_matches_from(["login", "--services", "cloud-platform,pubsub"]) + .unwrap(); + let (_, filter) = parse_login_args(&matches).unwrap(); + let services = filter.unwrap(); + assert!( + services.contains("cloud-platform"), + "cloud-platform should be accepted: {services:?}" + ); + assert!( + services.contains("pubsub"), + "pubsub should be accepted: {services:?}" + ); + } + #[test] fn filter_scopes_by_services_none_returns_all() { let scopes = vec![