Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-782-auth-subcommand-help.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

fix(auth): prevent --help on auth subcommands from triggering OAuth flow
102 changes: 102 additions & 0 deletions crates/google-workspace-cli/src/auth_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,16 @@ fn auth_command() -> clap::Command {
.subcommand(clap::Command::new("logout").about("Clear saved credentials and token cache"))
}

/// Returns true if the args slice contains `--help` or `-h`.
///
/// Used to detect help requests for subcommands before entering any
/// OAuth/setup flow, guarding against cases where clap's `DisplayHelp`
/// error is not propagated (e.g. `disable_help_flag`) or where the
/// subcommand dispatch would otherwise proceed to network I/O.
fn args_request_help(args: &[String]) -> bool {
args.iter().any(|a| a == "--help" || a == "-h")
}

/// Handle `gws auth <subcommand>`.
pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> {
let matches = match auth_command()
Expand All @@ -447,19 +457,37 @@ pub async fn handle_auth_command(args: &[String]) -> Result<(), GwsError> {
};

match matches.subcommand() {
Some(("login", _)) if args_request_help(args) => {
// Print login-specific help without starting any OAuth flow.
build_login_subcommand()
.print_help()
.map_err(|e| GwsError::Validation(format!("Failed to print help: {e}")))?;
Ok(())
}
Some(("login", sub_m)) => {
let (scope_mode, services_filter) = parse_login_args(sub_m);

handle_login_inner(scope_mode, services_filter).await
}
Some(("setup", sub_m)) => {
// Collect remaining args and delegate to setup's own clap parser.
// setup uses disable_help_flag(true) + trailing_var_arg so --help/-h
// lands in the captured args; parse_setup_args handles it internally.
let setup_args: Vec<String> = sub_m
.get_many::<String>("args")
.map(|vals| vals.cloned().collect())
.unwrap_or_default();
crate::setup::run_setup(&setup_args).await
}
Some(("status", _)) if args_request_help(args) => {
// Print status-specific help without querying credentials or the network.
auth_command()
.find_subcommand_mut("status")
.unwrap()
.print_help()
.map_err(|e| GwsError::Validation(format!("Failed to print help: {e}")))?;
Ok(())
}
Some(("status", _)) => handle_status().await,
Comment on lines +460 to 491
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Redundant and Unreachable Dead Code

The new guard match arms for login and status subcommands are completely redundant and unreachable dead code.

Why this is redundant:

  1. When a user runs gws auth login --help or gws auth status --help, handle_auth_command is called with args containing "--help" or "-h".
  2. auth_command().try_get_matches_from(...) is called with these arguments.
  3. Since the login and status subcommands do not have .disable_help_flag(true) set, clap's built-in parser automatically intercepts the --help/-h flag and returns a clap::error::Error with ErrorKind::DisplayHelp.
  4. This error is immediately caught by the Err(e) if e.kind() == ErrorKind::DisplayHelp match arm at lines 447-455 (which prints the help message and returns Ok(()) directly).
  5. As a result, the execution never reaches the match matches.subcommand() block for these help requests.

Recommendation:

Remove these redundant match arms. The existing integration tests added in this PR will still pass perfectly, proving that clap's built-in help handling is already fully sufficient.

        Some(("login", sub_m)) => {
            let (scope_mode, services_filter) = parse_login_args(sub_m);

            handle_login_inner(scope_mode, services_filter).await
        }
        Some(("setup", sub_m)) => {
            // Collect remaining args and delegate to setup's own clap parser.
            // setup uses disable_help_flag(true) + trailing_var_arg so --help/-h
            // lands in the captured args; parse_setup_args handles it internally.
            let setup_args: Vec<String> = sub_m
                .get_many::<String>("args")
                .map(|vals| vals.cloned().collect())
                .unwrap_or_default();
            crate::setup::run_setup(&setup_args).await
        }
        Some(("status", _)) => handle_status().await,

Some(("export", sub_m)) => {
let unmasked = sub_m.get_flag("unmasked");
Expand Down Expand Up @@ -1933,6 +1961,80 @@ mod tests {
}
}

// ── help flag on subcommands does not trigger OAuth/setup flow ─────────

#[tokio::test]
async fn handle_auth_command_login_help_returns_ok_without_oauth() {
// `gws auth login --help` must print help and return Ok, never touch OAuth.
let args = vec!["login".to_string(), "--help".to_string()];
let result = handle_auth_command(&args).await;
assert!(result.is_ok(), "login --help should return Ok, got: {result:?}");
}

#[tokio::test]
async fn handle_auth_command_login_help_short_returns_ok_without_oauth() {
// `gws auth login -h` must print help and return Ok, never touch OAuth.
let args = vec!["login".to_string(), "-h".to_string()];
let result = handle_auth_command(&args).await;
assert!(result.is_ok(), "login -h should return Ok, got: {result:?}");
}

#[tokio::test]
async fn handle_auth_command_status_help_returns_ok_without_network() {
// `gws auth status --help` must print help and return Ok, never call the network.
let args = vec!["status".to_string(), "--help".to_string()];
let result = handle_auth_command(&args).await;
assert!(result.is_ok(), "status --help should return Ok, got: {result:?}");
}

#[tokio::test]
async fn handle_auth_command_status_help_short_returns_ok_without_network() {
// `gws auth status -h` must print help and return Ok, never call the network.
let args = vec!["status".to_string(), "-h".to_string()];
let result = handle_auth_command(&args).await;
assert!(result.is_ok(), "status -h should return Ok, got: {result:?}");
}

#[tokio::test]
async fn handle_auth_command_setup_help_returns_ok_without_gcloud() {
// `gws auth setup --help` must print help and return Ok, never invoke gcloud.
let args = vec!["setup".to_string(), "--help".to_string()];
let result = handle_auth_command(&args).await;
assert!(result.is_ok(), "setup --help should return Ok, got: {result:?}");
}

#[tokio::test]
async fn handle_auth_command_setup_help_short_returns_ok_without_gcloud() {
// `gws auth setup -h` must print help and return Ok, never invoke gcloud.
let args = vec!["setup".to_string(), "-h".to_string()];
let result = handle_auth_command(&args).await;
assert!(result.is_ok(), "setup -h should return Ok, got: {result:?}");
}

#[test]
fn args_request_help_detects_long_flag() {
assert!(args_request_help(&["--help".to_string()]));
}

#[test]
fn args_request_help_detects_short_flag() {
assert!(args_request_help(&["-h".to_string()]));
}

#[test]
fn args_request_help_returns_false_for_unrelated_args() {
assert!(!args_request_help(&["--scopes".to_string(), "drive".to_string()]));
assert!(!args_request_help(&[]));
}

#[test]
fn args_request_help_detects_flag_among_others() {
assert!(args_request_help(&[
"--readonly".to_string(),
"--help".to_string()
]));
}

#[test]
#[serial_test::serial]
fn resolve_credentials_fails_without_env_vars_or_config() {
Expand Down
Loading