diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c6e6805b..f5f6f407f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ Changelog tracking starts with 0.2.0. Prior versions were not tracked. ### Added +- **Operators can enforce a distro for `astrid init`.** + `ASTRID_ENFORCED_DISTRO` supplies the distro source and rejects CLI attempts + to override it. Standalone `astrid init` requires an explicit `--distro`; + Astrid Runtime never chooses a product distro. Closes #1253. + - **Passive content-addressed capability-registry primitives.** Astrid now has exact capability IDs, typed content-bound references, immutable registered definitions, deterministic BLAKE3 semantic digests and canonical registry diff --git a/crates/astrid-cli/src/cli.rs b/crates/astrid-cli/src/cli.rs index be15e5736..74a954a35 100644 --- a/crates/astrid-cli/src/cli.rs +++ b/crates/astrid-cli/src/cli.rs @@ -225,7 +225,8 @@ pub(crate) enum Commands { /// Initialize a workspace and install a distro Init { - /// Required distro to install (name, @org/repo, path to Distro.toml, or .shuttle) + /// Distro source to install. Required unless an embedding launcher sets + /// `ASTRID_ENFORCED_DISTRO` (`@owner/repo`, URL, local Distro.toml, or .shuttle). #[arg(long)] distro: Option, /// Non-interactive: accept all defaults. @@ -512,7 +513,7 @@ pub(crate) enum SessionCommands { pub(crate) enum DistroCommands { /// Apply a distro to the active or specified agent. Apply { - /// Distro identifier (name, `@org/repo`, path, or .shuttle). + /// Distro source (`@owner/repo`, URL, local Distro.toml, or .shuttle). name: Option, /// Target agent (defaults to active context). #[arg(short, long)] @@ -561,6 +562,10 @@ pub(crate) enum DistroCommands { }, } +#[cfg(test)] +#[path = "cli_distro_tests.rs"] +mod distro_tests; + #[cfg(test)] mod tests { use super::{CapsuleCommands, Cli, Commands, InviteCommand}; @@ -684,47 +689,6 @@ mod tests { )); } - #[test] - fn grant_capsules_parsing_stays_open_for_embedding_composition() { - let cli = Cli::try_parse_from(["astrid", "init", "--grant-capsules"]) - .expect("parsing stays open so an embedding layer can resolve the distro source"); - - assert!(matches!( - cli.command, - Some(Commands::Init { - distro: None, - grant_capsules: true, - .. - }) - )); - } - - #[test] - fn init_parses_target_separately_from_operator() { - let cli = Cli::try_parse_from([ - "astrid", - "--principal", - "operator-1", - "init", - "--distro", - "./Distro.toml", - "--target-principal", - "agent-1", - "--grant-capsules", - ]) - .expect("operator and target principal should parse independently"); - - assert_eq!(cli.principal.as_deref(), Some("operator-1")); - assert!(matches!( - cli.command, - Some(Commands::Init { - target_principal: Some(ref target), - grant_capsules: true, - .. - }) if target == "agent-1" - )); - } - #[test] fn global_format_does_not_collide_with_nested_format_enum() { let cli = Cli::try_parse_from(["astrid", "keypair", "pubkey", "e2e-cli-key"]) diff --git a/crates/astrid-cli/src/cli_distro_tests.rs b/crates/astrid-cli/src/cli_distro_tests.rs new file mode 100644 index 000000000..7e43aeae4 --- /dev/null +++ b/crates/astrid-cli/src/cli_distro_tests.rs @@ -0,0 +1,101 @@ +use clap::{CommandFactory, Parser}; + +use super::{Cli, Commands}; + +#[test] +fn grant_capsules_allows_an_operator_enforced_distro() { + let cli = Cli::try_parse_from(["astrid", "init", "--grant-capsules"]) + .expect("the operator may supply the distro outside the CLI argument surface"); + + assert!(matches!( + cli.command, + Some(Commands::Init { + distro: None, + grant_capsules: true, + .. + }) + )); +} + +#[test] +fn distro_help_lists_only_supported_sources_and_the_launcher_exception() { + let mut command = Cli::command(); + let init = command.find_subcommand_mut("init").expect("init command"); + let init_distro_help = init + .get_arguments() + .find(|argument| argument.get_id() == "distro") + .and_then(|argument| argument.get_help()) + .map(ToString::to_string); + + let mut command = Cli::command(); + let apply = command + .find_subcommand_mut("distro") + .expect("distro command") + .find_subcommand_mut("apply") + .expect("distro apply command"); + let apply_name_help = apply + .get_arguments() + .find(|argument| argument.get_id() == "name") + .and_then(|argument| argument.get_help()) + .map(ToString::to_string); + + assert_eq!( + init_distro_help.as_deref(), + Some( + "Distro source to install. Required unless an embedding launcher sets \ + `ASTRID_ENFORCED_DISTRO` (`@owner/repo`, URL, local Distro.toml, or .shuttle)" + ) + ); + assert_eq!( + apply_name_help.as_deref(), + Some("Distro source (`@owner/repo`, URL, local Distro.toml, or .shuttle)") + ); +} + +#[test] +fn init_parses_target_separately_from_operator() { + let cli = Cli::try_parse_from([ + "astrid", + "--principal", + "operator-1", + "init", + "--distro", + "./Distro.toml", + "--target-principal", + "agent-1", + "--grant-capsules", + ]) + .expect("operator and target principal should parse independently"); + + assert_eq!(cli.principal.as_deref(), Some("operator-1")); + assert!(matches!( + cli.command, + Some(Commands::Init { + target_principal: Some(ref target), + grant_capsules: true, + .. + }) if target == "agent-1" + )); +} + +#[test] +fn global_options_before_init_preserve_the_requested_distro() { + let cli = Cli::try_parse_from([ + "astrid", + "--principal", + "operator-1", + "init", + "--distro", + "@example/other", + ]) + .expect("global options before init should parse"); + + assert_eq!(cli.principal.as_deref(), Some("operator-1")); + assert!(matches!( + cli.command, + Some(Commands::Init { + distro: Some(ref distro), + .. + }) if distro == "@example/other" + )); +} diff --git a/crates/astrid-cli/src/dispatch.rs b/crates/astrid-cli/src/dispatch.rs index 49deb9728..8edeb68ee 100644 --- a/crates/astrid-cli/src/dispatch.rs +++ b/crates/astrid-cli/src/dispatch.rs @@ -4,6 +4,7 @@ //! [`crate::cli::Commands`] to its handler. Lives in its own module so //! [`crate::main`] is just `tokio::main` plus error-formatting plumbing. +use std::ffi::OsString; use std::io::IsTerminal; use std::process::ExitCode; @@ -175,11 +176,7 @@ async fn dispatch_subcommand( target_principal, grant_capsules, }) => { - let distro = distro.ok_or_else(|| { - anyhow::anyhow!( - "astrid init requires --distro ; Astrid Runtime does not choose a product distro" - ) - })?; + let distro = resolve_init_distro(distro)?; let opts = commands::init::InitOpts { yes, offline, @@ -233,6 +230,39 @@ async fn dispatch_subcommand( } } +fn resolve_init_distro(requested: Option) -> Result { + resolve_init_distro_with(requested, std::env::var_os("ASTRID_ENFORCED_DISTRO")) +} + +fn resolve_init_distro_with( + requested: Option, + enforced: Option, +) -> Result { + let Some(enforced) = enforced else { + return non_empty_distro_source(requested).ok_or_else(|| { + anyhow::anyhow!( + "astrid init requires --distro <@owner/repo, URL, local Distro.toml, or .shuttle> unless ASTRID_ENFORCED_DISTRO is set by an embedding launcher; Astrid Runtime does not choose a product distro" + ) + }); + }; + let enforced = enforced.into_string().map_err(|_| { + anyhow::anyhow!("ASTRID_ENFORCED_DISTRO must contain a valid UTF-8 distro source") + })?; + if enforced.is_empty() { + anyhow::bail!("ASTRID_ENFORCED_DISTRO must not be empty"); + } + if requested.is_some() { + anyhow::bail!( + "astrid init cannot override the operator-enforced distro in ASTRID_ENFORCED_DISTRO" + ); + } + Ok(enforced) +} + +fn non_empty_distro_source(source: Option) -> Option { + source.filter(|source| !source.is_empty()) +} + /// Route the root capsule-verb shorthand (`astrid [args…]`). /// /// Built-in verbs never reach here — clap matches a declared `Commands` @@ -382,9 +412,9 @@ async fn dispatch_distro(command: DistroCommands) -> Result { &[tracker_657()], )); } - let distro = name.ok_or_else(|| { + let distro = non_empty_distro_source(name).ok_or_else(|| { anyhow::anyhow!( - "astrid distro apply requires a distro name, @org/repo, path, or .shuttle; Astrid Runtime does not choose a product distro" + "astrid distro apply requires an explicit distro source: @owner/repo, URL, local Distro.toml, or .shuttle; Astrid Runtime does not choose a product distro" ) })?; let opts = commands::init::InitOpts { @@ -502,6 +532,8 @@ fn dispatch_session(command: SessionCommands) -> Result { #[cfg(test)] mod tests { + use clap::Parser; + use super::*; /// The production harvest must include invocable **aliases**, not just @@ -555,45 +587,143 @@ mod tests { #[tokio::test] async fn init_without_a_distro_never_selects_a_product_default() { - let error = dispatch_subcommand( - Some(Commands::Init { - distro: None, - yes: false, - offline: false, - allow_unsigned: false, - accept_new_key: false, - vars: Vec::new(), - target_principal: None, - grant_capsules: false, - }), - OutputFormat::Pretty, - ) - .await - .expect_err("standalone init must require an explicit distro"); + for distro in [None, Some(String::new())] { + let error = dispatch_subcommand( + Some(Commands::Init { + distro, + yes: false, + offline: false, + allow_unsigned: false, + accept_new_key: false, + vars: Vec::new(), + target_principal: None, + grant_capsules: false, + }), + OutputFormat::Pretty, + ) + .await + .expect_err("standalone init must require a non-empty explicit distro"); + + assert_eq!( + error.to_string(), + "astrid init requires --distro <@owner/repo, URL, local Distro.toml, or .shuttle> unless ASTRID_ENFORCED_DISTRO is set by an embedding launcher; Astrid Runtime does not choose a product distro" + ); + } + } + + #[test] + fn distro_resolution_without_a_source_never_selects_a_product_default() { + let error = resolve_init_distro_with(None, None) + .expect_err("standalone init must require an explicit distro"); assert_eq!( error.to_string(), - "astrid init requires --distro ; Astrid Runtime does not choose a product distro" + "astrid init requires --distro <@owner/repo, URL, local Distro.toml, or .shuttle> unless ASTRID_ENFORCED_DISTRO is set by an embedding launcher; Astrid Runtime does not choose a product distro" ); } - #[tokio::test] - async fn distro_apply_without_a_name_never_selects_a_product_default() { - let error = dispatch_distro(DistroCommands::Apply { - name: None, - agent: None, - yes: false, - offline: false, - allow_unsigned: false, - accept_new_key: false, - vars: Vec::new(), - }) - .await - .expect_err("standalone distro apply must require an explicit distro"); + #[test] + fn operator_enforced_distro_cannot_be_overridden_by_the_cli() { + assert_eq!( + resolve_init_distro_with(Some("@example/other".to_string()), None) + .expect("standalone explicit distro should remain valid"), + "@example/other" + ); + assert_eq!( + resolve_init_distro_with(None, Some(OsString::from("/opt/product/Distro.toml"))) + .expect("operator distro should satisfy init"), + "/opt/product/Distro.toml" + ); + let error = resolve_init_distro_with( + Some("@example/other".to_string()), + Some(OsString::from("/opt/product/Distro.toml")), + ) + .expect_err("CLI must not override an operator-enforced distro"); assert_eq!( error.to_string(), - "astrid distro apply requires a distro name, @org/repo, path, or .shuttle; Astrid Runtime does not choose a product distro" + "astrid init cannot override the operator-enforced distro in ASTRID_ENFORCED_DISTRO" + ); + } + + #[test] + fn operator_distro_and_targeted_capsule_grants_compose() { + let cli = Cli::try_parse_from([ + "astrid", + "--principal", + "operator-1", + "init", + "--target-principal", + "agent-1", + "--grant-capsules", + ]) + .expect("an operator may supply the distro outside Astrid's CLI arguments"); + + assert_eq!(cli.principal.as_deref(), Some("operator-1")); + let Some(Commands::Init { + distro, + target_principal, + grant_capsules, + .. + }) = cli.command + else { + panic!("expected init command"); + }; + assert_eq!( + resolve_init_distro_with(distro, Some(OsString::from("/opt/product/Distro.toml")),) + .expect("operator-enforced distro should satisfy init"), + "/opt/product/Distro.toml" + ); + assert_eq!(target_principal.as_deref(), Some("agent-1")); + assert!(grant_capsules); + } + + #[test] + fn malformed_operator_enforced_distro_fails_closed() { + let empty = resolve_init_distro_with(None, Some(OsString::new())) + .expect_err("empty enforced distro must fail"); + assert_eq!( + empty.to_string(), + "ASTRID_ENFORCED_DISTRO must not be empty" ); + + #[cfg(unix)] + { + use std::os::unix::ffi::OsStringExt; + + let invalid = resolve_init_distro_with( + None, + Some(OsString::from_vec(vec![ + b'd', b'i', b's', b't', b'r', b'o', 0xff, + ])), + ) + .expect_err("non-UTF-8 enforced distro must fail"); + assert_eq!( + invalid.to_string(), + "ASTRID_ENFORCED_DISTRO must contain a valid UTF-8 distro source" + ); + } + } + + #[tokio::test] + async fn distro_apply_without_a_source_never_selects_a_product_default() { + for name in [None, Some(String::new())] { + let error = dispatch_distro(DistroCommands::Apply { + name, + agent: None, + yes: false, + offline: false, + allow_unsigned: false, + accept_new_key: false, + vars: Vec::new(), + }) + .await + .expect_err("standalone distro apply must require a non-empty explicit distro"); + + assert_eq!( + error.to_string(), + "astrid distro apply requires an explicit distro source: @owner/repo, URL, local Distro.toml, or .shuttle; Astrid Runtime does not choose a product distro" + ); + } } }