From ddadb3941da9105e08ccd5b08f4deb7da4db9180 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 9 Jun 2026 09:26:07 -0700 Subject: [PATCH 1/3] fix: adopt cli-engine 0.2.0 fail-closed auth; mark local commands no_auth Upgrade the engine dependency to 0.2.0, whose authentication is now fail-closed by default (`AuthRequirement::Required`): the engine resolves a credential before a command runs unless the command opts out. Annotate every local-only command `no_auth(true)` so they keep working without authentication: - env: list, get, set, info (read/write ~/.gdenv) - actions: list, describe (local action-contract catalog) - api: list, describe, search (local API catalog; `api call` stays Required) - application: validate and the `add` subcommands (edit local godaddy.toml) Backend commands (application list/info/init/update/enable/disable/archive/ release/deploy, api call, webhook events) keep the default Required policy, so a forgotten annotation now over-prompts rather than running unauthenticated. Credential reads use the lazy `ctx.credential().await?` accessor. Regression tests: `env list` runs with no provider registered (local path), and `application list` fails closed with no provider (backend path). Co-Authored-By: Claude Opus 4.8 --- rust/Cargo.lock | 10 ++-- rust/Cargo.toml | 2 +- rust/src/actions_catalog/mod.rs | 4 +- rust/src/api_explorer/mod.rs | 9 ++-- rust/src/application/commands/mod.rs | 68 ++++++++++++++++++++-------- rust/src/env/mod.rs | 41 +++++++++++++++-- rust/src/webhook/mod.rs | 6 +-- 7 files changed, 101 insertions(+), 39 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 8e85480..0f0a8d9 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -536,9 +536,9 @@ checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" [[package]] name = "cli-engine" -version = "0.1.3" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f3ac303c124ed8284bbc3eb99f99e970509d3e1d4030896ab19663d09911fba" +checksum = "d3c13c34cb4bce62d50adef63dff8b16abc53ac4ae5ddb4d24670307b0291390" dependencies = [ "async-trait", "base64 0.22.1", @@ -1290,7 +1290,7 @@ dependencies = [ "libc", "percent-encoding", "pin-project-lite", - "socket2 0.6.3", + "socket2 0.5.10", "tokio", "tower-service", "tracing", @@ -2020,7 +2020,7 @@ dependencies = [ "quinn-udp", "rustc-hash", "rustls", - "socket2 0.6.3", + "socket2 0.5.10", "thiserror 2.0.18", "tokio", "tracing", @@ -2057,7 +2057,7 @@ dependencies = [ "cfg_aliases", "libc", "once_cell", - "socket2 0.6.3", + "socket2 0.5.10", "tracing", "windows-sys 0.60.2", ] diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 294e46f..bdca7b1 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -16,7 +16,7 @@ path = "src/main.rs" async-trait = "0.1" chrono = { version = "0.4", default-features = false, features = ["clock", "serde"] } clap = { version = "4.5", features = ["std", "string"] } -cli-engine = { features = ["pkce-auth"], version = "0.1.3" } +cli-engine = { features = ["pkce-auth"], version = "0.2" } dirs = "6" fancy-regex = "0.14" regex = { version = "1", features = ["std"] } diff --git a/rust/src/actions_catalog/mod.rs b/rust/src/actions_catalog/mod.rs index 31ab261..0527585 100644 --- a/rust/src/actions_catalog/mod.rs +++ b/rust/src/actions_catalog/mod.rs @@ -66,7 +66,8 @@ pub fn module() -> Module { .with_command(RuntimeCommandSpec::new( CommandSpec::new("list", "List all available action contracts") .with_system("actions") - .with_tier(Tier::Read), + .with_tier(Tier::Read) + .no_auth(true), |_cred, _args| async move { let actions: Vec<_> = ACTIONS .iter() @@ -79,6 +80,7 @@ pub fn module() -> Module { CommandSpec::new("describe", "Show detailed schema for an action contract") .with_system("actions") .with_tier(Tier::Read) + .no_auth(true) .with_arg( clap::Arg::new("action") .value_name("ACTION") diff --git a/rust/src/api_explorer/mod.rs b/rust/src/api_explorer/mod.rs index 01eeb1d..23227a2 100644 --- a/rust/src/api_explorer/mod.rs +++ b/rust/src/api_explorer/mod.rs @@ -182,6 +182,7 @@ fn list_command() -> RuntimeCommandSpec { CommandSpec::new("list", "List all API domains") .with_system("api") .with_tier(Tier::Read) + .no_auth(true) .with_default_fields("domain,title,endpoints,baseUrl") .with_arg( clap::Arg::new("domain") @@ -253,6 +254,7 @@ fn describe_command() -> RuntimeCommandSpec { ) .with_system("api") .with_tier(Tier::Read) + .no_auth(true) .with_arg( clap::Arg::new("endpoint") .value_name("ENDPOINT") @@ -298,6 +300,7 @@ fn search_command() -> RuntimeCommandSpec { CommandSpec::new("search", "Search API endpoints by keyword") .with_system("api") .with_tier(Tier::Read) + .no_auth(true) .with_default_fields("domain,method,path,summary") .with_arg( clap::Arg::new("query") @@ -443,11 +446,7 @@ fn call_command() -> RuntimeCommandSpec { .get("method") .and_then(|v| v.as_str()) .unwrap_or("GET"); - let token = ctx - .credential - .as_ref() - .map(|c| c.token.clone()) - .unwrap_or_default(); + let token = ctx.credential().await?.token; let base_url = crate::application::client::api_url_for_env(&ctx.middleware.env); let url = format!("{base_url}{endpoint}"); diff --git a/rust/src/application/commands/mod.rs b/rust/src/application/commands/mod.rs index 9c51beb..f30e02e 100644 --- a/rust/src/application/commands/mod.rs +++ b/rust/src/application/commands/mod.rs @@ -6,12 +6,10 @@ use serde_json::json; use crate::application::client::{ApplicationClient, api_url_for_env}; -fn make_client(ctx: &cli_engine::CommandContext) -> cli_engine::Result { - let token = ctx - .credential - .as_ref() - .map(|c| c.token.clone()) - .unwrap_or_default(); +async fn make_client(ctx: &cli_engine::CommandContext) -> cli_engine::Result { + // Lazily resolve the credential; this triggers the auth flow only for + // commands that actually call the API. + let token = ctx.credential().await?.token; let base_url = api_url_for_env(&ctx.middleware.env); Ok(ApplicationClient::new(base_url, token)) } @@ -48,7 +46,7 @@ fn list_command() -> RuntimeCommandSpec { .with_tier(Tier::Read) .with_default_fields("name,label,status"), |ctx| async move { - let client = make_client(&ctx)?; + let client = make_client(&ctx).await?; let data = client.list_applications().await.map_err(client_err)?; Ok(CommandResult::new(data).with_next_actions(vec![ NextAction::new( @@ -80,7 +78,7 @@ fn info_command() -> RuntimeCommandSpec { ), |ctx| async move { let name = arg_str(&ctx, "name").to_owned(); - let client = make_client(&ctx)?; + let client = make_client(&ctx).await?; let data = client.get_application(&name).await.map_err(client_err)?; Ok( CommandResult::new(data["application"].clone()).with_next_actions(vec![ @@ -178,7 +176,7 @@ fn init_command() -> RuntimeCommandSpec { .map(|s| s.split(',').map(|p| p.trim().to_owned()).collect()) .unwrap_or_else(|| vec!["apps.app-registry:read".to_owned()]); - let client = make_client(&ctx)?; + let client = make_client(&ctx).await?; let data = client .create_application(json!({ "name": name, @@ -235,6 +233,7 @@ fn validate_command() -> RuntimeCommandSpec { CommandSpec::new("validate", "Validate godaddy.toml config") .with_system("applications") .with_tier(Tier::Read) + .no_auth(true) .with_arg( clap::Arg::new("config") .long("config") @@ -294,7 +293,7 @@ fn update_command() -> RuntimeCommandSpec { if let Some(desc) = ctx.args.get("description").and_then(|v| v.as_str()) { input.insert("description".to_owned(), json!(desc)); } - let client = make_client(&ctx)?; + let client = make_client(&ctx).await?; let data = client .update_application(&id, json!(input)) .await @@ -356,7 +355,7 @@ fn enable_command() -> RuntimeCommandSpec { |ctx| async move { let name = arg_str(&ctx, "name").to_owned(); let store_id = arg_str(&ctx, "store-id").to_owned(); - let client = make_client(&ctx)?; + let client = make_client(&ctx).await?; let data = client .enable_application(json!({ "applicationName": name, "storeId": store_id })) .await @@ -422,7 +421,7 @@ fn disable_command() -> RuntimeCommandSpec { |ctx| async move { let name = arg_str(&ctx, "name").to_owned(); let store_id = arg_str(&ctx, "store-id").to_owned(); - let client = make_client(&ctx)?; + let client = make_client(&ctx).await?; let data = client .disable_application(json!({ "applicationName": name, "storeId": store_id })) .await @@ -482,7 +481,7 @@ fn archive_command() -> RuntimeCommandSpec { ), |ctx| async move { let name = arg_str(&ctx, "name").to_owned(); - let client = make_client(&ctx)?; + let client = make_client(&ctx).await?; let app_data = client.get_application(&name).await.map_err(client_err)?; let app_id = app_data["application"]["id"] .as_str() @@ -540,7 +539,7 @@ fn release_command() -> RuntimeCommandSpec { if let Some(desc) = description { input["description"] = json!(desc); } - let client = make_client(&ctx)?; + let client = make_client(&ctx).await?; let data = client.create_release(input).await.map_err(client_err)?; Ok( CommandResult::new(data["createRelease"].clone()).with_next_actions(vec![ @@ -573,11 +572,7 @@ fn deploy_command() -> RuntimeCommandSpec { .unwrap_or("") .to_owned(); let env = ctx.middleware.env.clone(); - let token = ctx - .credential - .as_ref() - .map(|c| c.token.clone()) - .unwrap_or_default(); + let token = ctx.credential().await?.token; let base_url = api_url_for_env(&env); let client = ApplicationClient::new(base_url, token); @@ -831,6 +826,7 @@ pub fn add_group() -> RuntimeGroupSpec { CommandSpec::new("action", "Add an action to godaddy.toml") .with_system("applications") .with_tier(Tier::Mutate) + .no_auth(true) .with_arg( clap::Arg::new("name") .long("name") @@ -862,6 +858,7 @@ pub fn add_group() -> RuntimeGroupSpec { CommandSpec::new("subscription", "Add a webhook subscription to godaddy.toml") .with_system("applications") .with_tier(Tier::Mutate) + .no_auth(true) .with_arg( clap::Arg::new("name") .long("name") @@ -925,6 +922,7 @@ pub fn add_extension_group() -> RuntimeGroupSpec { CommandSpec::new("embed", "Add an embed extension") .with_system("applications") .with_tier(Tier::Mutate) + .no_auth(true) .with_arg( clap::Arg::new("name") .long("name") @@ -974,6 +972,7 @@ pub fn add_extension_group() -> RuntimeGroupSpec { CommandSpec::new("checkout", "Add a checkout extension") .with_system("applications") .with_tier(Tier::Mutate) + .no_auth(true) .with_arg( clap::Arg::new("name") .long("name") @@ -1023,6 +1022,7 @@ pub fn add_extension_group() -> RuntimeGroupSpec { CommandSpec::new("blocks", "Add a blocks extension") .with_system("applications") .with_tier(Tier::Mutate) + .no_auth(true) .with_arg( clap::Arg::new("source") .long("source") @@ -1052,3 +1052,33 @@ pub fn add_extension_group() -> RuntimeGroupSpec { }, )) } + +#[cfg(test)] +mod tests { + use cli_engine::{Cli, CliConfig}; + + /// API commands must stay fail-closed: `application list` calls the backend, + /// so it must require authentication. Built with **no auth provider + /// registered**, the engine's default `AuthRequirement::Required` must reject + /// it before the handler runs (no network call). This guards against someone + /// mistakenly marking an API command `no_auth(true)`, which would let it run + /// unauthenticated. + #[tokio::test] + async fn application_list_requires_auth() { + let cli = Cli::new( + CliConfig::new("gddy", "GoDaddy developer CLI", "gddy") + .with_default_auth_provider("godaddy") + .with_module(super::super::module()), + ); + + let output = cli + .run(["gddy", "application", "list", "--output", "json"]) + .await; + + assert_ne!( + output.exit_code, 0, + "application list must fail closed without a credential, got: {}", + output.rendered + ); + } +} diff --git a/rust/src/env/mod.rs b/rust/src/env/mod.rs index 28fc09a..cdb36d8 100644 --- a/rust/src/env/mod.rs +++ b/rust/src/env/mod.rs @@ -57,7 +57,8 @@ pub fn module() -> Module { .with_command(RuntimeCommandSpec::new( CommandSpec::new("list", "List available environments") .with_system("env") - .with_tier(Tier::Read), + .with_tier(Tier::Read) + .no_auth(true), |_cred, _args| async move { let current = get_env().unwrap_or_else(|| DEFAULT_ENV.to_owned()); let envs: Vec<_> = KNOWN_ENVS @@ -76,7 +77,8 @@ pub fn module() -> Module { .with_command(RuntimeCommandSpec::new( CommandSpec::new("get", "Get the active environment") .with_system("env") - .with_tier(Tier::Read), + .with_tier(Tier::Read) + .no_auth(true), |_cred, _args| async move { let env = get_env().unwrap_or_else(|| DEFAULT_ENV.to_owned()); Ok(CommandResult::new(json!({ @@ -89,6 +91,7 @@ pub fn module() -> Module { CommandSpec::new("set", "Set the active environment") .with_system("env") .with_tier(Tier::Mutate) + .no_auth(true) .with_arg( // Distinct id from the global `--env` flag (also id "env"); // a shared id makes the positional collide with the flag's @@ -125,7 +128,8 @@ pub fn module() -> Module { .with_command(RuntimeCommandSpec::new( CommandSpec::new("info", "Show details for the active environment") .with_system("env") - .with_tier(Tier::Read), + .with_tier(Tier::Read) + .no_auth(true), |_cred, _args| async move { let env = get_env().unwrap_or_else(|| DEFAULT_ENV.to_owned()); Ok(CommandResult::new(json!({ @@ -137,3 +141,34 @@ pub fn module() -> Module { )) }) } + +#[cfg(test)] +mod tests { + use cli_engine::{Cli, CliConfig}; + + /// `env list` is a local-only command: it must run without any auth flow. + /// + /// The CLI is built with **no auth provider registered**. Because the engine + /// authenticates fail-closed by default (`AuthRequirement::Required`), a + /// command that forgot to opt out would fail here with "no provider + /// registered". This guards that the `env` commands stay `no_auth(true)`. + #[tokio::test] + async fn env_list_runs_without_auth() { + let cli = Cli::new( + CliConfig::new("gddy", "GoDaddy developer CLI", "gddy").with_module(super::module()), + ); + + let output = cli.run(["gddy", "env", "list", "--output", "json"]).await; + + assert_eq!(output.exit_code, 0, "stderr: {}", output.rendered); + let json: serde_json::Value = + serde_json::from_str(&output.rendered).expect("valid json output"); + let envs = json["data"].as_array().expect("data array"); + assert!( + envs.iter() + .any(|e| e["name"] == "ote" || e["name"] == "prod"), + "expected known environments in output, got: {}", + output.rendered + ); + } +} diff --git a/rust/src/webhook/mod.rs b/rust/src/webhook/mod.rs index da37322..9a02cc4 100644 --- a/rust/src/webhook/mod.rs +++ b/rust/src/webhook/mod.rs @@ -12,11 +12,7 @@ pub fn module() -> Module { .with_system("webhooks") .with_tier(Tier::Read), |ctx| async move { - let token = ctx - .credential - .as_ref() - .map(|c| c.token.clone()) - .unwrap_or_default(); + let token = ctx.credential().await?.token; let base_url = api_url_for_env(&ctx.middleware.env); let url = format!("{base_url}/v1/apis/webhook-event-types"); let resp = crate::application::client::make_http_client() From fe8cfe7d373a2af038533623e588845e871898aa Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 9 Jun 2026 09:32:18 -0700 Subject: [PATCH 2/3] test: assert application list fails closed at auth resolution Address Copilot review on PR #56: the fail-closed test asserted only a non-zero exit, which could mask a later handler/network failure. Assert the auth-failure exit code (2) and that the rendered error names the missing provider, proving rejection happens at credential resolution before the handler runs. Co-Authored-By: Claude Opus 4.8 --- rust/src/application/commands/mod.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/rust/src/application/commands/mod.rs b/rust/src/application/commands/mod.rs index f30e02e..9bda4dc 100644 --- a/rust/src/application/commands/mod.rs +++ b/rust/src/application/commands/mod.rs @@ -1075,9 +1075,20 @@ mod tests { .run(["gddy", "application", "list", "--output", "json"]) .await; - assert_ne!( - output.exit_code, 0, - "application list must fail closed without a credential, got: {}", + // Exit code 2 is the engine's auth-failure code, and the rendered error + // names the missing provider — proving the command was rejected at + // credential resolution, not by the handler hitting the network. + assert_eq!( + output.exit_code, 2, + "application list must fail closed at auth resolution, got: {}", + output.rendered + ); + let json: serde_json::Value = + serde_json::from_str(&output.rendered).expect("valid json output"); + let message = json["error"]["message"].as_str().unwrap_or_default(); + assert!( + message.contains("provider"), + "expected an auth-provider resolution error, got: {}", output.rendered ); } From f7a4bbbcee45afa0b7b6b8f60908a38d21d068c7 Mon Sep 17 00:00:00 2001 From: Jacob Page Date: Tue, 9 Jun 2026 09:38:25 -0700 Subject: [PATCH 3/3] test: clarify fail-closed test assertions Address Copilot re-review on PR #56: relabel the misleading "stderr" panic message on the env test (the value is the rendered output), and name the hard-coded auth-failure exit code as AUTH_FAILURE_EXIT for clarity. Co-Authored-By: Claude Opus 4.8 --- rust/src/application/commands/mod.rs | 7 ++++--- rust/src/env/mod.rs | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/rust/src/application/commands/mod.rs b/rust/src/application/commands/mod.rs index 9bda4dc..3262571 100644 --- a/rust/src/application/commands/mod.rs +++ b/rust/src/application/commands/mod.rs @@ -1075,11 +1075,12 @@ mod tests { .run(["gddy", "application", "list", "--output", "json"]) .await; - // Exit code 2 is the engine's auth-failure code, and the rendered error - // names the missing provider — proving the command was rejected at + // The engine maps auth-resolution failures to exit code 2; together with + // the provider-named error below this proves the command was rejected at // credential resolution, not by the handler hitting the network. + const AUTH_FAILURE_EXIT: i32 = 2; assert_eq!( - output.exit_code, 2, + output.exit_code, AUTH_FAILURE_EXIT, "application list must fail closed at auth resolution, got: {}", output.rendered ); diff --git a/rust/src/env/mod.rs b/rust/src/env/mod.rs index cdb36d8..6d7407b 100644 --- a/rust/src/env/mod.rs +++ b/rust/src/env/mod.rs @@ -160,7 +160,7 @@ mod tests { let output = cli.run(["gddy", "env", "list", "--output", "json"]).await; - assert_eq!(output.exit_code, 0, "stderr: {}", output.rendered); + assert_eq!(output.exit_code, 0, "rendered output: {}", output.rendered); let json: serde_json::Value = serde_json::from_str(&output.rendered).expect("valid json output"); let envs = json["data"].as_array().expect("data array");