diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cdcbb440..58afc2fea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,20 @@ Changelog tracking starts with 0.2.0. Prior versions were not tracked. definitions, deterministic BLAKE3 semantic digests and canonical registry manifests. Existing profile persistence, wildcard evaluation, bootstrap, socket and wire behavior remain unchanged. Closes #1233. Refs #1228. +- **Distro init can grant exactly installed capsules to an explicit target.** + `astrid --principal init --target-principal + --grant-capsules` ensures the runtime daemon, verifies the operator has + `agent:modify` authority over an existing target before provisioning, and + applies the installed set through the shared `admin.agent.modify` path. + Distro capsules require a concrete released version or tag; identity and + declared version are checked before install mutation, and locks record the + version and WASM hash that actually landed. Fresh-lock reuse rehashes the + installed content blob before names become grants. Concurrent provisioning + of one target is rejected, and recovery commands preserve the operator + identity. The target defaults to the process principal when omitted; no + principal name receives special treatment. + Signed `.shuttle` grant composition remains deferred and fails explicitly. + Closes #1195. ### Changed diff --git a/Cargo.lock b/Cargo.lock index febb4da1f..516e9f987 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -339,6 +339,7 @@ dependencies = [ "directories", "ed25519-dalek 3.0.0", "flate2", + "fs2", "futures", "hex", "indicatif", diff --git a/crates/astrid-capsule-install/src/archive.rs b/crates/astrid-capsule-install/src/archive.rs index ef1053ae2..eae99f350 100644 --- a/crates/astrid-capsule-install/src/archive.rs +++ b/crates/astrid-capsule-install/src/archive.rs @@ -10,11 +10,15 @@ use std::path::Path; use anyhow::{Context, bail}; +use astrid_capsule::capsule::CapsuleId; use astrid_core::dirs::AstridHome; use astrid_core::PrincipalId; -use crate::local::{InstallOptions, InstallOutput, install_from_local_path_for_principal}; +use crate::local::{ + InstallOptions, InstallOutput, install_from_local_path_checked_for_principal, + install_from_local_path_for_principal, +}; /// Unpack `archive_path` (a gzipped tar) into a tempdir, then install /// from there. @@ -45,6 +49,43 @@ pub fn unpack_and_install_for_principal( home: &AstridHome, options: InstallOptions, target_principal: &PrincipalId, +) -> anyhow::Result { + unpack_and_install_internal(archive_path, home, options, target_principal, None, None) +} + +/// Unpack and install only when the archive manifest identity equals +/// `expected`. When `expected_version` is present, the manifest version must +/// match it too. The staged archive is inspected before install mutation. +/// +/// # Errors +/// +/// Returns an error for an unsafe or malformed archive, an identity or version +/// mismatch, or any failure propagated by the local installer. +pub fn unpack_and_install_checked_for_principal( + archive_path: &Path, + home: &AstridHome, + options: InstallOptions, + target_principal: &PrincipalId, + expected: &CapsuleId, + expected_version: Option<&str>, +) -> anyhow::Result { + unpack_and_install_internal( + archive_path, + home, + options, + target_principal, + Some(expected), + expected_version, + ) +} + +fn unpack_and_install_internal( + archive_path: &Path, + home: &AstridHome, + options: InstallOptions, + target_principal: &PrincipalId, + expected: Option<&CapsuleId>, + expected_version: Option<&str>, ) -> anyhow::Result { let tmp_dir = tempfile::tempdir().context("failed to create temp dir for unpacking")?; let unpack_dir = tmp_dir.path(); @@ -90,5 +131,15 @@ pub fn unpack_and_install_for_principal( .with_context(|| format!("Failed to unpack file: {}", out_path.display()))?; } - install_from_local_path_for_principal(unpack_dir, home, options, target_principal) + match expected { + Some(expected) => install_from_local_path_checked_for_principal( + unpack_dir, + home, + options, + target_principal, + expected, + expected_version, + ), + None => install_from_local_path_for_principal(unpack_dir, home, options, target_principal), + } } diff --git a/crates/astrid-capsule-install/src/checked_tests.rs b/crates/astrid-capsule-install/src/checked_tests.rs new file mode 100644 index 000000000..9cf325d0f --- /dev/null +++ b/crates/astrid-capsule-install/src/checked_tests.rs @@ -0,0 +1,90 @@ +use std::fs::File; + +use astrid_capsule::capsule::CapsuleId; +use astrid_core::PrincipalId; +use astrid_core::dirs::AstridHome; + +use crate::{ + InstallOptions, install_from_local_path_checked_for_principal, resolve_target_dir_for, + unpack_and_install_checked_for_principal, +}; + +fn write_manifest(dir: &std::path::Path, name: &str, version: &str) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write( + dir.join("Capsule.toml"), + format!("[package]\nname = \"{name}\"\nversion = \"{version}\"\n"), + ) + .unwrap(); +} + +fn existing_target(home: &AstridHome, principal: &PrincipalId, name: &str) -> std::path::PathBuf { + let target = resolve_target_dir_for(home, principal, name, false).unwrap(); + std::fs::create_dir_all(&target).unwrap(); + std::fs::write(target.join("marker"), b"original").unwrap(); + target +} + +#[test] +fn checked_local_rejects_identity_before_target_mutation() { + let temp = tempfile::tempdir().unwrap(); + let home = AstridHome::from_path(temp.path().join("home")); + let principal = PrincipalId::new("alice").unwrap(); + let expected = CapsuleId::new("expected").unwrap(); + let source = temp.path().join("source"); + write_manifest(&source, "unexpected", "1.0.0"); + let target = existing_target(&home, &principal, expected.as_str()); + + let err = install_from_local_path_checked_for_principal( + &source, + &home, + InstallOptions::default(), + &principal, + &expected, + Some("1.0.0"), + ) + .unwrap_err(); + + assert!(err.to_string().contains("identity mismatch")); + assert_eq!(std::fs::read(target.join("marker")).unwrap(), b"original"); + assert!( + !resolve_target_dir_for(&home, &principal, "unexpected", false) + .unwrap() + .exists() + ); +} + +#[test] +fn checked_archive_rejects_version_before_replacing_existing_install() { + let temp = tempfile::tempdir().unwrap(); + let home = AstridHome::from_path(temp.path().join("home")); + let principal = PrincipalId::new("alice").unwrap(); + let expected = CapsuleId::new("expected").unwrap(); + let source = temp.path().join("source"); + write_manifest(&source, expected.as_str(), "2.0.0"); + let target = existing_target(&home, &principal, expected.as_str()); + + let archive_path = temp.path().join("expected.capsule"); + let encoder = flate2::write::GzEncoder::new( + File::create(&archive_path).unwrap(), + flate2::Compression::default(), + ); + let mut archive = tar::Builder::new(encoder); + archive + .append_path_with_name(source.join("Capsule.toml"), "Capsule.toml") + .unwrap(); + archive.into_inner().unwrap().finish().unwrap(); + + let err = unpack_and_install_checked_for_principal( + &archive_path, + &home, + InstallOptions::default(), + &principal, + &expected, + Some("1.0.0"), + ) + .unwrap_err(); + + assert!(err.to_string().contains("version mismatch")); + assert_eq!(std::fs::read(target.join("marker")).unwrap(), b"original"); +} diff --git a/crates/astrid-capsule-install/src/lib.rs b/crates/astrid-capsule-install/src/lib.rs index 68f2a434c..e5d8404c6 100644 --- a/crates/astrid-capsule-install/src/lib.rs +++ b/crates/astrid-capsule-install/src/lib.rs @@ -72,7 +72,9 @@ pub mod principal_introspection; pub mod wasm; pub mod wit; -pub use archive::{unpack_and_install, unpack_and_install_for_principal}; +pub use archive::{ + unpack_and_install, unpack_and_install_checked_for_principal, unpack_and_install_for_principal, +}; pub use contracts::{ CONTRACTS_WIT_BASENAME, ContractsSkew, canonical_contracts_b3, canonical_contracts_path, contracts_pin, contracts_skew, mismatching_contracts, refresh_canonical_contracts, @@ -81,7 +83,7 @@ pub use contracts::{ pub use copy::copy_capsule_dir; pub use local::{ InstallOptions, InstallOutput, InstallPhase, install_from_local_path, - install_from_local_path_for_principal, + install_from_local_path_checked_for_principal, install_from_local_path_for_principal, }; pub use manifest_check::{ExportConflict, MissingImport, check_export_conflicts, validate_imports}; pub use meta::{ @@ -94,3 +96,6 @@ pub use paths::{ }; pub use principal_introspection::materialize_principal_introspection; pub use wit::{content_address_wit, materialize_wit_mirror}; + +#[cfg(test)] +mod checked_tests; diff --git a/crates/astrid-capsule-install/src/local.rs b/crates/astrid-capsule-install/src/local.rs index 750cdc457..5d814dc37 100644 --- a/crates/astrid-capsule-install/src/local.rs +++ b/crates/astrid-capsule-install/src/local.rs @@ -32,6 +32,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, bail}; +use astrid_capsule::capsule::CapsuleId; use astrid_capsule::discovery::load_manifest; use astrid_capsule::engine::wasm::host_state::LifecyclePhase; use astrid_core::PrincipalId; @@ -160,21 +161,73 @@ pub fn install_from_local_path_for_principal( home: &AstridHome, options: InstallOptions, target_principal: &PrincipalId, +) -> anyhow::Result { + install_from_local_path_internal(source_dir, home, options, target_principal, None, None) +} + +/// Install for an explicit principal only when the loaded manifest identity +/// equals `expected`. When `expected_version` is present, the manifest version +/// must match it too. Both comparisons happen before any install mutation. +/// +/// # Errors +/// +/// Returns an error when the manifest identity or expected version differs, +/// or when any ordinary install validation or filesystem operation fails. +#[allow(clippy::needless_pass_by_value)] +pub fn install_from_local_path_checked_for_principal( + source_dir: &Path, + home: &AstridHome, + options: InstallOptions, + target_principal: &PrincipalId, + expected: &CapsuleId, + expected_version: Option<&str>, +) -> anyhow::Result { + install_from_local_path_internal( + source_dir, + home, + options, + target_principal, + Some(expected), + expected_version, + ) +} + +#[allow(clippy::needless_pass_by_value, clippy::too_many_lines)] +fn install_from_local_path_internal( + source_dir: &Path, + home: &AstridHome, + options: InstallOptions, + target_principal: &PrincipalId, + expected: Option<&CapsuleId>, + expected_version: Option<&str>, ) -> anyhow::Result { let manifest_path = source_dir.join("Capsule.toml"); if !manifest_path.exists() { bail!("No Capsule.toml found in {}", source_dir.display()); } let manifest = load_manifest(&manifest_path).context("failed to load Capsule manifest")?; - let id = manifest.package.name.clone(); + let id = CapsuleId::new(manifest.package.name.clone())?; + if let Some(expected) = expected + && id != *expected + { + bail!("capsule identity mismatch: expected '{expected}', manifest declares '{id}'"); + } let installed_version = manifest.package.version.clone(); + if let Some(expected_version) = expected_version + && installed_version != expected_version + { + bail!( + "capsule version mismatch for '{id}': expected '{expected_version}', manifest declares '{installed_version}'" + ); + } // Pre-flight checks — pure reads, no target mutation. let export_conflicts = check_export_conflicts(&manifest)?; // Resolve target. The parent must exist before we attempt the // backup-rename later; create it now. - let target_dir = resolve_target_dir_for(home, target_principal, &id, options.workspace)?; + let target_dir = + resolve_target_dir_for(home, target_principal, id.as_str(), options.workspace)?; let parent = target_dir.parent().context("target dir has no parent")?; std::fs::create_dir_all(parent) .with_context(|| format!("failed to create {}", parent.display()))?; @@ -223,7 +276,7 @@ pub fn install_from_local_path_for_principal( // Preserve existing .env.json (user configuration survives reinstall). if let Some(ref backup) = backup_dir { - restore_env_from_backup_for(home, target_principal, backup, &id); + restore_env_from_backup_for(home, target_principal, backup, id.as_str()); } // Lifecycle hook — bytes from the content store, not the target. @@ -295,7 +348,7 @@ pub fn install_from_local_path_for_principal( } // Determine env-prompt signal for the caller. - let env_path = resolve_env_path_for(home, target_principal, &id)?; + let env_path = resolve_env_path_for(home, target_principal, id.as_str())?; let env_needs_prompt = !manifest.env.is_empty() && !env_path.exists(); let missing_imports = if options.skip_import_check { diff --git a/crates/astrid-cli/Cargo.toml b/crates/astrid-cli/Cargo.toml index 3dab63e41..0897a00da 100644 --- a/crates/astrid-cli/Cargo.toml +++ b/crates/astrid-cli/Cargo.toml @@ -54,6 +54,7 @@ colored = { workspace = true } crossterm = { workspace = true } directories = { workspace = true } flate2 = { workspace = true } +fs2 = { workspace = true } futures = { workspace = true } indicatif = { workspace = true } libc = { workspace = true } diff --git a/crates/astrid-cli/src/cli.rs b/crates/astrid-cli/src/cli.rs index c91be6c8a..9ae24858c 100644 --- a/crates/astrid-cli/src/cli.rs +++ b/crates/astrid-cli/src/cli.rs @@ -243,6 +243,15 @@ pub(crate) enum Commands { /// Set a variable (repeatable): KEY=VALUE. #[arg(long = "var", value_name = "KEY=VALUE")] vars: Vec, + /// Principal whose home and capsule access this init provisions. + /// The global `--principal` remains the authenticated operator. + #[arg(long = "target-principal", value_name = "PRINCIPAL")] + target_principal: Option, + /// Grant the target principal access to every capsule the distro + /// installs (same mechanism as `agent modify --add-capsule`). + /// A distro source must resolve before initialization runs. + #[arg(long = "grant-capsules")] + grant_capsules: bool, }, /// View resolved configuration, edit it in `$EDITOR`, or print paths. @@ -645,6 +654,47 @@ mod tests { assert_eq!(cli.principal.as_deref(), Some("operator-1")); } + #[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/commands/agent/mod.rs b/crates/astrid-cli/src/commands/agent/mod.rs index 84e9110e6..81903d1af 100644 --- a/crates/astrid-cli/src/commands/agent/mod.rs +++ b/crates/astrid-cli/src/commands/agent/mod.rs @@ -730,18 +730,80 @@ async fn run_modify(args: ModifyArgs) -> Result { return Ok(ExitCode::from(1)); } let mut client = crate::admin_client::connect_as_active_agent().await?; + let outcome = apply_agent_modify( + &mut client, + &principal, + &args.add_group, + &args.remove_group, + &args.add_capsule, + &args.remove_capsule, + ) + .await?; + if outcome.changed { + println!( + "{}", + Theme::success(&format!( + "Updated agent '{principal}' groups: [{}] capsules: [{}]", + outcome.groups.join(", "), + outcome.capsules.join(", ") + )) + ); + } else { + println!( + "{}", + Theme::info(&format!( + "agent '{principal}' already has the requested groups and capsules (no change)" + )) + ); + } + Ok(ExitCode::SUCCESS) +} + +/// Parsed result of an `admin.agent.modify` round-trip: the principal's +/// resulting group and capsule sets, plus whether the kernel actually +/// changed the profile (`false` on an idempotent no-op re-apply). +pub(crate) struct AgentModifyOutcome { + /// The principal's group memberships after the delta. + pub(crate) groups: Vec, + /// The principal's capsule-access grant set after the delta. + pub(crate) capsules: Vec, + /// Whether the profile changed (set-wise). `false` means every + /// requested add/remove was already reflected — an idempotent re-run. + pub(crate) changed: bool, +} + +/// Issue an `admin.agent.modify` request and parse the kernel's reply. +/// +/// The single seam through which group and capsule-access grants reach +/// the kernel. Shared by the `agent modify` verb and `init +/// --grant-capsules` so both provision through the *exact same* +/// idempotent kernel path (`apply_set_delta`) — never a divergent copy of +/// the grant logic. The kernel applies removes-then-adds atomically for +/// the whole request and reports `changed = false` when the resulting set +/// is unchanged, which is what makes a re-apply safe. +/// +/// # Errors +/// Propagates transport errors and any `AdminResponseBody::Error` the +/// kernel returns (e.g. the caller lacks `agent:modify`, or the target +/// principal has no profile). +pub(crate) async fn apply_agent_modify( + client: &mut AdminClient, + principal: &PrincipalId, + add_groups: &[String], + remove_groups: &[String], + add_capsules: &[String], + remove_capsules: &[String], +) -> Result { let body = client .request(AdminRequestKind::AgentModify { principal: principal.clone(), - add_groups: args.add_group.clone(), - remove_groups: args.remove_group.clone(), - add_capsules: args.add_capsule.clone(), - remove_capsules: args.remove_capsule.clone(), + add_groups: add_groups.to_vec(), + remove_groups: remove_groups.to_vec(), + add_capsules: add_capsules.to_vec(), + remove_capsules: remove_capsules.to_vec(), }) .await?; - let body = into_result(body)?; - - let value = match body { + let value = match into_result(body)? { AdminResponseBody::Success(v) => v, other => anyhow::bail!("unexpected response from kernel: {other:?}"), }; @@ -757,30 +819,14 @@ async fn run_modify(args: ModifyArgs) -> Result { }) .unwrap_or_default() }; - let groups = string_array("groups"); - let capsules = string_array("capsules"); - let changed = value - .get("changed") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false); - if changed { - println!( - "{}", - Theme::success(&format!( - "Updated agent '{principal}' groups: [{}] capsules: [{}]", - groups.join(", "), - capsules.join(", ") - )) - ); - } else { - println!( - "{}", - Theme::info(&format!( - "agent '{principal}' already has the requested groups and capsules (no change)" - )) - ); - } - Ok(ExitCode::SUCCESS) + Ok(AgentModifyOutcome { + groups: string_array("groups"), + capsules: string_array("capsules"), + changed: value + .get("changed") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false), + }) } fn run_link(_args: LinkArgs) -> ExitCode { diff --git a/crates/astrid-cli/src/commands/capsule/install.rs b/crates/astrid-cli/src/commands/capsule/install.rs index 7b6ebcff8..81fdb418b 100644 --- a/crates/astrid-cli/src/commands/capsule/install.rs +++ b/crates/astrid-cli/src/commands/capsule/install.rs @@ -28,6 +28,7 @@ use std::path::Path; use std::sync::atomic::{AtomicBool, Ordering}; use anyhow::{Context, bail}; +use astrid_capsule::capsule::CapsuleId; use astrid_capsule_install::github_source::{ capsule_assets, extract_github_org_repo, parse_github_source, pick_capsule, }; @@ -37,6 +38,34 @@ use astrid_events::EventBus; use super::install_prompts::{cli_elicit_handler, prompt_env_fields}; +pub(crate) use super::install_batch::{ + BatchInstallOutcome, InstalledCapsuleOutcome, RefSpec, install_capsule_batch, +}; +use super::install_github::{github_api_client, release_tag_url, resolve_github_ref}; + +#[derive(Clone, Copy)] +struct ExpectedCapsule<'a> { + id: &'a CapsuleId, + version: Option<&'a str>, +} + +#[derive(Clone, Copy)] +struct InstallContext<'a> { + workspace: bool, + home: &'a AstridHome, + original_source: Option<&'a str>, + principal: &'a astrid_core::PrincipalId, + expected: Option>, +} + +#[derive(Clone, Copy)] +pub(crate) struct OfflineCapsuleProvenance<'a> { + pub(crate) original_source: &'a str, + pub(crate) resolved_ref: Option<&'a str>, + pub(crate) signer: Option<&'a str>, + pub(crate) signature: Option<&'a str>, +} + /// Re-exported so sibling CLI modules (`init.rs`, `shuttle_install.rs`) /// keep the `super::install::resolve_target_dir_for` import path. The /// `_for` variant scopes the target to a specific principal — the @@ -51,7 +80,7 @@ pub(crate) use super::install_update::update_capsule; /// When true, import validation and env prompting are suppressed. /// Set by `install_capsule_batch` (called from distro init) where the /// distro handles env config and all capsules are installed together. -static BATCH_MODE: AtomicBool = AtomicBool::new(false); +pub(super) static BATCH_MODE: AtomicBool = AtomicBool::new(false); // --------------------------------------------------------------------------- // Top-level install dispatch @@ -91,65 +120,28 @@ pub(crate) async fn install_capsule( capsule: Option<&str>, workspace: bool, ) -> anyhow::Result<()> { - let (installed, _resolved) = - install_capsule_inner(source, capsule, workspace, &RefSpec::default()).await?; + let principal = crate::principal::current(); + let (installed, _resolved) = install_capsule_inner( + source, + capsule, + workspace, + &RefSpec::default(), + &principal, + None, + ) + .await?; + let installed_ids: Vec = installed + .iter() + .map(|capsule| capsule.id.as_str().to_string()) + .collect(); // Live-load: if a daemon is running, hot-load (or upgrade) each just-installed // capsule so it's usable without a restart. Best-effort and non-fatal — the // on-disk install above already succeeded standalone. The `update` and TUI // install paths route through here too, so they inherit live hot-swap. - super::live_load::nudge_daemon_reload(&installed).await; + super::live_load::nudge_daemon_reload(&installed_ids).await; Ok(()) } -/// Which concrete git ref a GitHub install should resolve. -/// -/// Mirrors the manifest's `tag`/`version` selectors. When everything is -/// `None`, the installer falls back to the latest release (documented, -/// not silent — see [`resolve_github_ref`]). -#[derive(Debug, Clone, Default)] -pub(crate) struct RefSpec { - /// Semver version (resolved to a `v`-prefixed or bare release tag). - pub(crate) version: Option, - /// Explicit git tag (highest priority). - pub(crate) tag: Option, -} - -impl RefSpec { - /// Build a [`RefSpec`] from a distro capsule's pinning fields. - pub(crate) fn from_capsule(cap: &super::super::distro::manifest::DistroCapsule) -> Self { - Self { - // An empty `version` string carries no pin. - version: (!cap.version.is_empty()).then(|| cap.version.clone()), - tag: cap.tag.clone(), - } - } -} - -/// Install a capsule in batch mode (from distro init) — skips import -/// validation and env prompting. `name_hint` is the distro capsule `name`, -/// used to pick the right archive when one source ships several (a monorepo -/// builds/releases one `.capsule` per capsule crate). Honors an explicit -/// version/tag pin from the distro manifest. -/// -/// Returns the concrete git ref that was actually resolved and fetched -/// (`Some` for GitHub-backed sources, `None` for local-path sources), -/// so the caller can record the *installed* ref in the lock rather than -/// an optimistic guess from the manifest's declared fields. -pub(crate) async fn install_capsule_batch( - source: &str, - name_hint: Option<&str>, - workspace: bool, - refspec: &RefSpec, -) -> anyhow::Result> { - BATCH_MODE.store(true, Ordering::Relaxed); - let result = install_capsule_inner(source, name_hint, workspace, refspec).await; - BATCH_MODE.store(false, Ordering::Relaxed); - // Distro batch install does not nudge a live reload (init manages its own - // load, and the daemon is typically down during init) — keep only the - // resolved ref so the caller can record it in the lock. - result.map(|(_ids, resolved_ref)| resolved_ref) -} - /// Install dispatch shared by the CLI and distro-batch paths. /// /// `name_hint` is the `--capsule ` / distro capsule `name` selector @@ -157,12 +149,14 @@ pub(crate) async fn install_capsule_batch( /// `(installed_capsule_ids, resolved_ref)`: the ids of every capsule /// installed, and the resolved git ref for GitHub-backed sources (`Some`), /// or `None` for local-path sources, which have no remote ref to resolve. -async fn install_capsule_inner( +pub(super) async fn install_capsule_inner( source: &str, name_hint: Option<&str>, workspace: bool, refspec: &RefSpec, -) -> anyhow::Result<(Vec, Option)> { + principal: &astrid_core::PrincipalId, + expected: Option<&CapsuleId>, +) -> anyhow::Result<(Vec, Option)> { let home = AstridHome::resolve()?; // Recover any `@org/repo@version` CLI suffix and fold it into the @@ -173,13 +167,17 @@ async fn install_capsule_inner( .clone() .or_else(|| suffix_version.map(str::to_string)); let tag = refspec.tag.clone(); + let expected = expected.map(|id| ExpectedCapsule { + id, + version: version.as_deref(), + }); // 1. Explicit local path — record the path as the source so a // later `astrid distro update` can re-resolve from it (it's the // canonical reference for a locally-sourced capsule). No remote // ref to resolve. if base.starts_with('.') || base.starts_with('/') { - let ids = install_from_local(base, workspace, &home, Some(base))?; + let ids = install_from_local(base, workspace, &home, Some(base), principal, expected)?; return Ok((ids, None)); } @@ -188,12 +186,16 @@ async fn install_capsule_inner( let url = format!("https://github.com/{repo}"); return install_from_github( &url, - workspace, - &home, - Some(base), name_hint, version.as_deref(), tag.as_deref(), + InstallContext { + workspace, + home: &home, + original_source: Some(base), + principal, + expected, + }, ) .await; } @@ -202,18 +204,22 @@ async fn install_capsule_inner( if base.starts_with("github.com/") || base.starts_with("https://github.com/") { return install_from_github( base, - workspace, - &home, - Some(base), name_hint, version.as_deref(), tag.as_deref(), + InstallContext { + workspace, + home: &home, + original_source: Some(base), + principal, + expected, + }, ) .await; } // 4. Fallback: assume local folder. No remote ref to resolve. - let ids = install_from_local(base, workspace, &home, Some(base))?; + let ids = install_from_local(base, workspace, &home, Some(base), principal, expected)?; Ok((ids, None)) } @@ -221,157 +227,6 @@ async fn install_capsule_inner( // GitHub installs — release-artifact download with clone-and-build fallback. // --------------------------------------------------------------------------- -/// A GitHub token from the environment, if present. -/// -/// Checks `GH_TOKEN` then `GITHUB_TOKEN` — the conventions the `gh` CLI and -/// CI both honour. An empty or whitespace value counts as absent. -fn github_token() -> Option { - ["GH_TOKEN", "GITHUB_TOKEN"].into_iter().find_map(|key| { - std::env::var(key) - .ok() - .map(|v| v.trim().to_string()) - .filter(|v| !v.is_empty()) - }) -} - -/// Build an HTTP client for GitHub release/source resolution, authenticated -/// when a token is available. -/// -/// Anonymous GitHub API access is capped at 60 requests/hour, which a full -/// distro (~9 capsules, each costing one or more release-resolution calls) -/// can exhaust mid-provision. A token lifts the ceiling to 5000/hour. The -/// token is attached as a default `Authorization` header; reqwest strips -/// sensitive headers on cross-host redirects, so it never leaks to the -/// release-asset CDN the API redirects downloads to. Absence of a token is -/// NOT an error — resolution simply proceeds anonymously. -fn github_api_client() -> anyhow::Result { - let mut headers = reqwest::header::HeaderMap::new(); - if let Some(token) = github_token() { - match reqwest::header::HeaderValue::from_str(&format!("Bearer {token}")) { - Ok(mut value) => { - value.set_sensitive(true); - headers.insert(reqwest::header::AUTHORIZATION, value); - }, - // A PRESENT-but-malformed token is surfaced rather than silently - // dropped, but does NOT hard-fail: anonymous access still works - // for public repos, and aborting init over an unrelated bad env - // var is worse than the 60/hr ceiling. The token value is never - // echoed. An ABSENT token stays silent — the normal case. - Err(_) => { - eprintln!( - "warning: ignoring malformed GH_TOKEN/GITHUB_TOKEN \ - (not a valid HTTP header value); proceeding with \ - anonymous GitHub API access" - ); - }, - } - } - reqwest::Client::builder() - .user_agent("astrid-cli") - .timeout(std::time::Duration::from_secs(30)) - .default_headers(headers) - .build() - .context("failed to build GitHub HTTP client") -} - -/// Build the GitHub "release by tag" API URL with the tag as a single, -/// percent-encoded path segment. -/// -/// Interpolating the tag into the path (`releases/tags/{tag}`) is unsafe: a -/// tag legitimately containing `/` (e.g. `release/1.0`) would change the URL -/// path structure. Parsing the base and pushing the tag through -/// `path_segments_mut` encodes it as one segment (`release%2F1.0`). -fn release_tag_url(org: &str, repo: &str, tag: &str) -> anyhow::Result { - // Parse the base WITHOUT a trailing slash: a trailing `/` leaves an empty - // final path segment, so pushing onto it yields `releases//tags`. Pushing - // `tags` then `tag` onto the un-slashed base gives the right path, with the - // tag percent-encoded as a single segment. - let mut url = reqwest::Url::parse(&format!( - "https://api.github.com/repos/{org}/{repo}/releases" - )) - .context("failed to build GitHub releases URL")?; - url.path_segments_mut() - .map_err(|()| anyhow::anyhow!("GitHub releases URL cannot be a base"))? - .push("tags") - .push(tag); - Ok(url.to_string()) -} - -/// Resolve which GitHub release tag to install for `org/repo`. -/// -/// Resolution priority: -/// 1. An explicit `tag` is used verbatim — the caller asked for it. -/// 2. A `version` is matched against a release tag: `v{version}` first -/// (the convention), then the bare `{version}`. A version with no -/// matching release is a hard error (we never silently fall through -/// to "latest" when the caller pinned a version). -/// 3. Neither set → the `latest` release. This fallback is explicit and -/// logged, replacing the previous behaviour where `releases/latest` -/// was fetched unconditionally and any `version` field was ignored. -async fn resolve_github_ref( - client: &reqwest::Client, - org: &str, - repo: &str, - version: Option<&str>, - tag: Option<&str>, -) -> anyhow::Result { - if let Some(t) = tag { - return Ok(t.to_string()); - } - - if let Some(v) = version { - for candidate in [format!("v{v}"), v.to_string()] { - let tag_url = release_tag_url(org, repo, &candidate)?; - let r = client.get(&tag_url).send().await.with_context(|| { - format!("failed to query release tag {candidate} for {org}/{repo}") - })?; - // 404 → this candidate tag simply doesn't exist; try the next. - if r.status() == reqwest::StatusCode::NOT_FOUND { - continue; - } - // Any other non-success (5xx, rate-limit, auth) is a real failure - // that must surface — not be misreported as "no release found". - if !r.status().is_success() { - bail!( - "GitHub API error querying release tag {candidate} for {org}/{repo}: HTTP {}", - r.status() - ); - } - let json = r - .json::() - .await - .with_context(|| format!("invalid GitHub API response for tag {candidate}"))?; - return Ok(json - .get("tag_name") - .and_then(serde_json::Value::as_str) - .unwrap_or(&candidate) - .to_string()); - } - bail!("no GitHub release found for version {v} in {org}/{repo}"); - } - - // Explicit, documented fallback to the latest release. - tracing::debug!(%org, %repo, "no version/tag pin — resolving latest release"); - let api_url = format!("https://api.github.com/repos/{org}/{repo}/releases/latest"); - let r = client - .get(&api_url) - .send() - .await - .context("failed to reach GitHub API for latest release")?; - if !r.status().is_success() { - bail!( - "GitHub API returned {} for {org}/{repo} latest release", - r.status() - ); - } - let json: serde_json::Value = r.json().await.context("invalid GitHub API response")?; - Ok(json - .get("tag_name") - .and_then(serde_json::Value::as_str) - .unwrap_or("latest") - .to_string()) -} - /// Stream a `.capsule` asset to `dest`, enforcing a 50 MB ceiling. async fn download_capsule_asset( client: &reqwest::Client, @@ -401,13 +256,11 @@ async fn download_capsule_asset( /// tag it resolved (it builds from whatever `--depth 1` HEAD it cloned). async fn install_from_github( url: &str, - workspace: bool, - home: &AstridHome, - original_source: Option<&str>, name_hint: Option<&str>, version: Option<&str>, tag: Option<&str>, -) -> anyhow::Result<(Vec, Option)> { + context: InstallContext<'_>, +) -> anyhow::Result<(Vec, Option)> { // Authenticated when a token is present so release resolution isn't // throttled at the anonymous 60/hr limit mid-distro (see // `github_api_client`). @@ -455,24 +308,13 @@ async fn install_from_github( let idx = pick_capsule(&names, Some(hint))? .expect("non-empty candidates always select an index"); let (name, download_url) = &candidates[idx]; - let id = download_and_unpack( - &client, - name, - download_url, - workspace, - home, - original_source, - ) - .await?; + let id = download_and_unpack(&client, name, download_url, context).await?; vec![id] }, // Manual install with no `--capsule`: install EVERY capsule // the release ships. Best-effort — report which assets fail // but keep going, then fail if any did. - None => { - install_all_capsules(&client, &candidates, workspace, home, original_source) - .await? - }, + None => install_all_capsules(&client, &candidates, context).await?, }; return Ok((ids, Some(resolved_ref))); } @@ -500,7 +342,7 @@ async fn install_from_github( // Priority 2: clone + build from source via astrid-build — reached only // when nothing was pinned (a pin would have bailed above). - let id = clone_and_build(url, repo, workspace, home, original_source, name_hint)?; + let id = clone_and_build(url, repo, name_hint, context)?; Ok((vec![id], None)) } @@ -584,10 +426,8 @@ async fn download_and_unpack( client: &reqwest::Client, name: &str, download_url: &str, - workspace: bool, - home: &AstridHome, - original_source: Option<&str>, -) -> anyhow::Result { + context: InstallContext<'_>, +) -> anyhow::Result { let tmp_dir = tempfile::tempdir()?; let sanitized_name = Path::new(name).file_name().unwrap_or_default(); let download_path = tmp_dir.path().join(sanitized_name); @@ -602,7 +442,14 @@ async fn download_and_unpack( ); } std::fs::write(&download_path, &bytes)?; - unpack_via_lib(&download_path, workspace, home, original_source) + unpack_via_lib( + &download_path, + context.workspace, + context.home, + context.original_source, + context.principal, + context.expected, + ) } /// Install every `.capsule` asset in a release (the manual-install default). @@ -614,18 +461,14 @@ async fn download_and_unpack( async fn install_all_capsules( client: &reqwest::Client, candidates: &[(String, String)], - workspace: bool, - home: &AstridHome, - original_source: Option<&str>, -) -> anyhow::Result> { + context: InstallContext<'_>, +) -> anyhow::Result> { eprintln!("Release ships {} capsule(s):", candidates.len()); - let mut installed: Vec = Vec::new(); + let mut installed: Vec = Vec::new(); let mut failed: Vec<(&str, String)> = Vec::new(); for (name, download_url) in candidates { eprintln!("Installing {name}..."); - match download_and_unpack(client, name, download_url, workspace, home, original_source) - .await - { + match download_and_unpack(client, name, download_url, context).await { Ok(id) => installed.push(id), Err(e) => { eprintln!(" Failed to install {name}: {e}"); @@ -655,11 +498,9 @@ async fn install_all_capsules( fn clone_and_build( url: &str, repo: &str, - workspace: bool, - home: &AstridHome, - original_source: Option<&str>, name_hint: Option<&str>, -) -> anyhow::Result { + context: InstallContext<'_>, +) -> anyhow::Result { let tmp_dir = tempfile::tempdir().context("failed to create temp dir for cloning")?; let clone_dir = tmp_dir.path().join(repo); @@ -712,7 +553,14 @@ fn clone_and_build( .map(|p| p.file_name().and_then(|n| n.to_str()).unwrap_or("")) .collect(); if let Some(idx) = pick_capsule(&names, name_hint)? { - return unpack_via_lib(&produced[idx], workspace, home, original_source); + return unpack_via_lib( + &produced[idx], + context.workspace, + context.home, + context.original_source, + context.principal, + context.expected, + ); } bail!("astrid-build produced no .capsule archive."); @@ -727,7 +575,9 @@ fn install_from_local( workspace: bool, home: &AstridHome, original_source: Option<&str>, -) -> anyhow::Result> { + principal: &astrid_core::PrincipalId, + expected: Option>, +) -> anyhow::Result> { let source_path = Path::new(source); if !source_path.exists() { bail!("Source path does not exist: {source}"); @@ -735,7 +585,15 @@ fn install_from_local( // Unpack `.capsule` archive when source is a file. if source_path.is_file() && source.ends_with(".capsule") { - return unpack_via_lib(source_path, workspace, home, original_source).map(|id| vec![id]); + return unpack_via_lib( + source_path, + workspace, + home, + original_source, + principal, + expected, + ) + .map(|installed| vec![installed]); } // Auto-build Rust capsules when source is a directory with a Cargo.toml. @@ -762,14 +620,29 @@ fn install_from_local( for entry in std::fs::read_dir(&output_dir)? { let entry = entry?; if entry.path().extension().and_then(|s| s.to_str()) == Some("capsule") { - return unpack_via_lib(&entry.path(), workspace, home, original_source) - .map(|id| vec![id]); + return unpack_via_lib( + &entry.path(), + workspace, + home, + original_source, + principal, + expected, + ) + .map(|installed| vec![installed]); } } bail!("Failed to auto-build capsule from Cargo project."); } - install_from_local_path(source_path, workspace, home, original_source).map(|id| vec![id]) + install_from_local_path_for_principal( + source_path, + workspace, + home, + original_source, + principal, + expected, + ) + .map(|installed| vec![installed]) } // --------------------------------------------------------------------------- @@ -789,23 +662,52 @@ pub(crate) fn install_from_local_path( home: &AstridHome, original_source: Option<&str>, ) -> anyhow::Result { + let principal = crate::principal::current(); + install_from_local_path_for_principal( + source_dir, + workspace, + home, + original_source, + &principal, + None, + ) + .map(|installed| installed.id.as_str().to_string()) +} + +fn install_from_local_path_for_principal( + source_dir: &Path, + workspace: bool, + home: &AstridHome, + original_source: Option<&str>, + principal: &astrid_core::PrincipalId, + expected: Option>, +) -> anyhow::Result { let opts = InstallOptions { workspace, original_source: original_source.map(String::from), skip_import_check: BATCH_MODE.load(Ordering::Relaxed), lifecycle_bus: None, }; - let principal = crate::principal::current(); let output = run_with_elicit(opts, |opts, bus| { - astrid_capsule_install::install_from_local_path_for_principal( - source_dir, - home, - InstallOptions { - lifecycle_bus: Some(bus), - ..opts + let opts = InstallOptions { + lifecycle_bus: Some(bus), + ..opts + }; + match expected { + Some(expected) => { + astrid_capsule_install::install_from_local_path_checked_for_principal( + source_dir, + home, + opts, + principal, + expected.id, + expected.version, + ) }, - &principal, - ) + None => astrid_capsule_install::install_from_local_path_for_principal( + source_dir, home, opts, principal, + ), + } })?; finish_install(&output, home) } @@ -823,27 +725,35 @@ pub(crate) fn install_from_local_path( pub(crate) fn install_offline_capsule( archive: &Path, home: &AstridHome, - name: &str, - original_source: &str, - resolved_ref: Option<&str>, - signer: Option<&str>, - signature: Option<&str>, -) -> anyhow::Result<()> { + expected: &CapsuleId, + expected_version: Option<&str>, + provenance: OfflineCapsuleProvenance<'_>, + principal: &astrid_core::PrincipalId, +) -> anyhow::Result { BATCH_MODE.store(true, Ordering::Relaxed); let result = (|| { - unpack_via_lib(archive, false, home, Some(original_source))?; + let installed = unpack_via_lib( + archive, + false, + home, + Some(provenance.original_source), + principal, + Some(ExpectedCapsule { + id: expected, + version: expected_version, + }), + )?; // Post-stamp provenance into the freshly-written meta.json. The - // unpack above installs under the process principal's home, so read - // it back from there rather than the legacy `default` resolver — a - // scoped principal's offline install would otherwise not be found. - let target_dir = resolve_target_dir_for(home, &crate::principal::current(), name, false)?; + // unpack above installs under the explicit target principal, so + // read metadata back from that same home. + let target_dir = resolve_target_dir_for(home, principal, expected.as_str(), false)?; if let Some(mut meta) = super::meta::read_meta(&target_dir) { - meta.resolved_ref = resolved_ref.map(String::from); - meta.signer = signer.map(String::from); - meta.signature = signature.map(String::from); + meta.resolved_ref = provenance.resolved_ref.map(String::from); + meta.signer = provenance.signer.map(String::from); + meta.signature = provenance.signature.map(String::from); super::meta::write_meta(&target_dir, &meta)?; } - Ok(()) + Ok(installed) })(); BATCH_MODE.store(false, Ordering::Relaxed); result @@ -856,24 +766,33 @@ fn unpack_via_lib( workspace: bool, home: &AstridHome, original_source: Option<&str>, -) -> anyhow::Result { + principal: &astrid_core::PrincipalId, + expected: Option>, +) -> anyhow::Result { let opts = InstallOptions { workspace, original_source: original_source.map(String::from), skip_import_check: BATCH_MODE.load(Ordering::Relaxed), lifecycle_bus: None, }; - let principal = crate::principal::current(); let output = run_with_elicit(opts, |opts, bus| { - astrid_capsule_install::unpack_and_install_for_principal( - archive, - home, - InstallOptions { - lifecycle_bus: Some(bus), - ..opts - }, - &principal, - ) + let opts = InstallOptions { + lifecycle_bus: Some(bus), + ..opts + }; + match expected { + Some(expected) => astrid_capsule_install::unpack_and_install_checked_for_principal( + archive, + home, + opts, + principal, + expected.id, + expected.version, + ), + None => astrid_capsule_install::unpack_and_install_for_principal( + archive, home, opts, principal, + ), + } })?; finish_install(&output, home) } @@ -904,7 +823,10 @@ where /// Render post-install diagnostics and prompt for unset env fields. Returns the /// installed capsule id (its directory name), so the manual-install path can /// nudge a running daemon to hot-load exactly that capsule. -fn finish_install(output: &InstallOutput, home: &AstridHome) -> anyhow::Result { +fn finish_install( + output: &InstallOutput, + home: &AstridHome, +) -> anyhow::Result { let batch = BATCH_MODE.load(Ordering::Relaxed); // Load the manifest once (always present post-install) — used both for @@ -917,10 +839,26 @@ fn finish_install(output: &InstallOutput, home: &AstridHome) -> anyhow::Result` verbs it adds so the operator knows what just became // invocable. Printed adjacent to the other manifest-derived notices. - let capsule_id = output.target_dir.file_name().map_or_else( - || "capsule".to_string(), - |n| n.to_string_lossy().into_owned(), - ); + let capsule_id = CapsuleId::new(manifest.package.name.clone())?; + let meta = super::meta::read_meta(&output.target_dir) + .context("installed capsule has no readable meta.json")?; + if manifest.package.version != meta.version || output.installed_version != meta.version { + bail!( + "installed capsule '{}' version disagreement: manifest={}, meta={}, installer={}", + capsule_id, + manifest.package.version, + meta.version, + output.installed_version + ); + } + if output.wasm_hash != meta.wasm_hash { + bail!( + "installed capsule '{}' hash disagreement: installer={:?}, meta={:?}", + capsule_id, + output.wasm_hash, + meta.wasm_hash + ); + } let cli_commands: Vec<&astrid_capsule::manifest::CommandDef> = manifest .commands .iter() @@ -939,16 +877,13 @@ fn finish_install(output: &InstallOutput, home: &AstridHome) -> anyhow::Result anyhow::Result, + pub(crate) tag: Option, +} + +impl RefSpec { + pub(crate) fn from_capsule(cap: &super::super::distro::manifest::DistroCapsule) -> Self { + Self { + version: (!cap.version.trim().is_empty()).then(|| cap.version.trim().to_string()), + tag: cap + .tag + .as_deref() + .map(str::trim) + .filter(|tag| !tag.is_empty()) + .map(str::to_string), + } + } +} + +#[derive(Debug)] +pub(crate) struct InstalledCapsuleOutcome { + pub(crate) id: CapsuleId, + pub(crate) version: String, + pub(crate) wasm_hash: Option, +} + +#[derive(Debug)] +pub(crate) struct BatchInstallOutcome { + pub(crate) installed: Vec, + pub(crate) resolved_ref: Option, +} + +/// Install without prompting, returning the identities that actually landed. +pub(crate) async fn install_capsule_batch( + source: &str, + expected: &CapsuleId, + workspace: bool, + refspec: &RefSpec, + principal: &astrid_core::PrincipalId, +) -> anyhow::Result { + anyhow::ensure!( + refspec.version.is_some() || refspec.tag.is_some(), + "distro capsule '{expected}' has no concrete released version or tag" + ); + super::install::BATCH_MODE.store(true, Ordering::Relaxed); + let result = super::install::install_capsule_inner( + source, + Some(expected.as_str()), + workspace, + refspec, + principal, + Some(expected), + ) + .await; + super::install::BATCH_MODE.store(false, Ordering::Relaxed); + result.map(|(installed, resolved_ref)| BatchInstallOutcome { + installed, + resolved_ref, + }) +} diff --git a/crates/astrid-cli/src/commands/capsule/install_github.rs b/crates/astrid-cli/src/commands/capsule/install_github.rs new file mode 100644 index 000000000..fd4022972 --- /dev/null +++ b/crates/astrid-cli/src/commands/capsule/install_github.rs @@ -0,0 +1,108 @@ +//! GitHub API client and concrete release-ref resolution for capsule installs. + +use anyhow::{Context, bail}; + +fn github_token() -> Option { + ["GH_TOKEN", "GITHUB_TOKEN"].into_iter().find_map(|key| { + std::env::var(key) + .ok() + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) + }) +} + +pub(super) fn github_api_client() -> anyhow::Result { + let mut headers = reqwest::header::HeaderMap::new(); + if let Some(token) = github_token() { + match reqwest::header::HeaderValue::from_str(&format!("Bearer {token}")) { + Ok(mut value) => { + value.set_sensitive(true); + headers.insert(reqwest::header::AUTHORIZATION, value); + }, + Err(_) => eprintln!( + "warning: ignoring malformed GH_TOKEN/GITHUB_TOKEN \ + (not a valid HTTP header value); proceeding with anonymous GitHub API access" + ), + } + } + reqwest::Client::builder() + .user_agent("astrid-cli") + .timeout(std::time::Duration::from_secs(30)) + .default_headers(headers) + .build() + .context("failed to build GitHub HTTP client") +} + +pub(super) fn release_tag_url(org: &str, repo: &str, tag: &str) -> anyhow::Result { + let mut url = reqwest::Url::parse(&format!( + "https://api.github.com/repos/{org}/{repo}/releases" + )) + .context("failed to build GitHub releases URL")?; + url.path_segments_mut() + .map_err(|()| anyhow::anyhow!("GitHub releases URL cannot be a base"))? + .push("tags") + .push(tag); + Ok(url.to_string()) +} + +pub(super) async fn resolve_github_ref( + client: &reqwest::Client, + org: &str, + repo: &str, + version: Option<&str>, + tag: Option<&str>, +) -> anyhow::Result { + if let Some(tag) = tag { + return Ok(tag.to_string()); + } + if let Some(version) = version { + for candidate in [format!("v{version}"), version.to_string()] { + let tag_url = release_tag_url(org, repo, &candidate)?; + let response = client.get(&tag_url).send().await.with_context(|| { + format!("failed to query release tag {candidate} for {org}/{repo}") + })?; + if response.status() == reqwest::StatusCode::NOT_FOUND { + continue; + } + if !response.status().is_success() { + bail!( + "GitHub API error querying release tag {candidate} for {org}/{repo}: HTTP {}", + response.status() + ); + } + let json = response + .json::() + .await + .with_context(|| format!("invalid GitHub API response for tag {candidate}"))?; + return Ok(json + .get("tag_name") + .and_then(serde_json::Value::as_str) + .unwrap_or(&candidate) + .to_string()); + } + bail!("no GitHub release found for version {version} in {org}/{repo}"); + } + + tracing::debug!(%org, %repo, "no version/tag pin — resolving latest release"); + let api_url = format!("https://api.github.com/repos/{org}/{repo}/releases/latest"); + let response = client + .get(&api_url) + .send() + .await + .context("failed to reach GitHub API for latest release")?; + if !response.status().is_success() { + bail!( + "GitHub API returned {} for {org}/{repo} latest release", + response.status() + ); + } + let json: serde_json::Value = response + .json() + .await + .context("invalid GitHub API response")?; + Ok(json + .get("tag_name") + .and_then(serde_json::Value::as_str) + .unwrap_or("latest") + .to_string()) +} diff --git a/crates/astrid-cli/src/commands/capsule/mod.rs b/crates/astrid-cli/src/commands/capsule/mod.rs index 7f5b2f606..0920fe3b5 100644 --- a/crates/astrid-cli/src/commands/capsule/mod.rs +++ b/crates/astrid-cli/src/commands/capsule/mod.rs @@ -5,6 +5,8 @@ pub(crate) mod check; pub(crate) mod config; pub(crate) mod deps; pub(crate) mod install; +mod install_batch; +mod install_github; pub(crate) mod install_prompts; pub(crate) mod install_update; pub(crate) mod list; diff --git a/crates/astrid-cli/src/commands/distro/shuttle_install.rs b/crates/astrid-cli/src/commands/distro/shuttle_install.rs index 025e08cae..ae62af29a 100644 --- a/crates/astrid-cli/src/commands/distro/shuttle_install.rs +++ b/crates/astrid-cli/src/commands/distro/shuttle_install.rs @@ -21,6 +21,7 @@ use std::path::Path; use anyhow::{Context, bail}; +use astrid_capsule::capsule::CapsuleId; use astrid_core::dirs::AstridHome; use super::lock::{DistroLock, DistroLockMeta, LockedCapsule, manifest_hash, write_lock}; @@ -133,10 +134,9 @@ pub(crate) fn install_from_shuttle(shuttle_path: &Path, opts: &InitOpts) -> anyh // any install side effect. verify_capsule_hashes(mirror, &lock)?; - // The process-wide principal (global `--principal` flag). The offline - // install scopes env config, capsule files, and the Distro.lock to this - // principal's home, matching the online `astrid init` path. - let principal = crate::principal::current(); + // The explicit provisioning target. The authenticated runtime operator is + // independent and does not select where product state is installed. + let principal = opts.target_principal.clone(); // 5. Select capsules + collect variables (headless-aware). let variables = manifest.variables.clone(); @@ -189,11 +189,9 @@ pub(crate) fn install_from_shuttle(shuttle_path: &Path, opts: &InitOpts) -> anyh /// Install each selected capsule from the verified mirror and return /// the resolved [`LockedCapsule`] entries for the user's lock. /// -/// Capsule blake3 was already validated against the lock up front by -/// [`verify_capsule_hashes`] (file bytes proven == lock hash), so this -/// does NOT re-read or re-hash the archive: it reads the installed -/// `meta.json` for the content-addressed WASM hash and falls back to the -/// sealed lock's already-verified blake3 if meta is absent. +/// Capsule archive bytes were already validated against the sealed lock up +/// front by [`verify_capsule_hashes`]. The user's lock records the version and +/// content-addressed WASM hash reported by the checked install itself. fn install_selected_capsules( home: &AstridHome, principal: &astrid_core::PrincipalId, @@ -213,42 +211,35 @@ fn install_selected_capsules( ); } - // The sealed lock entry: carries both the truly-resolved ref (sealed - // online; nothing is resolved or guessed offline) and the - // already-verified blake3 used as the hash fallback below. + // The sealed lock entry carries the truly-resolved ref. Nothing is + // resolved or guessed during offline installation. let sealed = sealed_capsules.get(cap.name.as_str()); let resolved_ref = sealed.and_then(|c| c.resolved_ref.clone()); - crate::commands::capsule::install::install_offline_capsule( + let expected = CapsuleId::new(cap.name.clone())?; + let expected_version = (!cap.version.trim().is_empty()).then_some(cap.version.trim()); + let installed = crate::commands::capsule::install::install_offline_capsule( &file, home, - &cap.name, - &cap.source, - resolved_ref.as_deref(), - signer, - signature, + &expected, + expected_version, + crate::commands::capsule::install::OfflineCapsuleProvenance { + original_source: &cap.source, + resolved_ref: resolved_ref.as_deref(), + signer, + signature, + }, + principal, ) .with_context(|| format!("failed to install capsule {}", cap.name))?; - // Record the installed content-addressed WASM hash from meta, - // falling back to the sealed lock's already-verified archive blake3 - // (no re-read: `verify_capsule_hashes` proved file bytes == this). - // Read back from the scoped principal's home — the offline install - // (via `install_offline_capsule`) wrote it there. - let target_dir = crate::commands::capsule::install::resolve_target_dir_for( - home, principal, &cap.name, false, - )?; - let installed_hash = crate::commands::capsule::meta::read_meta(&target_dir) - .and_then(|m| m.wasm_hash) - .map_or_else( - || sealed.map(|c| c.hash.clone()).unwrap_or_default(), - |h| format!("blake3:{h}"), - ); - locked.push(LockedCapsule { name: cap.name.clone(), - version: cap.version.clone(), + version: installed.version, source: cap.source.clone(), - hash: installed_hash, + hash: installed + .wasm_hash + .map(|hash| format!("blake3:{hash}")) + .unwrap_or_default(), resolved_ref, }); eprintln!(" installed {}", cap.name); diff --git a/crates/astrid-cli/src/commands/distro/validate.rs b/crates/astrid-cli/src/commands/distro/validate.rs index 0182b5d43..c1a9bb5c2 100644 --- a/crates/astrid-cli/src/commands/distro/validate.rs +++ b/crates/astrid-cli/src/commands/distro/validate.rs @@ -152,6 +152,38 @@ pub(crate) fn validate_manifest(manifest: &DistroManifest) -> anyhow::Result<()> cap.name, ); } + let version = cap.version.trim(); + if cap.version != version { + anyhow::bail!( + "capsule '{}': version must not contain surrounding whitespace", + cap.name + ); + } + if !version.is_empty() && Version::parse(version).is_err() { + anyhow::bail!( + "capsule '{}': version '{}' is not valid semver", + cap.name, + cap.version + ); + } + let tag = cap.tag.as_deref().map(str::trim); + if let Some(raw_tag) = cap.tag.as_deref() + && raw_tag != raw_tag.trim() + { + anyhow::bail!( + "capsule '{}': tag must not contain surrounding whitespace", + cap.name + ); + } + if matches!(tag, Some("")) { + anyhow::bail!("capsule '{}': tag must not be empty", cap.name); + } + if version.is_empty() && tag.is_none() { + anyhow::bail!( + "capsule '{}': distro capsules require a concrete released version or tag", + cap.name + ); + } } // At least one uplink. @@ -535,6 +567,53 @@ tag = "v0.2.0-rc1" validate_manifest(&manifest).expect("version/tag release selectors are allowed"); } + #[test] + fn accepts_tag_only_release_selector() { + let toml_src = r#" +schema-version = 1 + +[distro] +id = "test" +name = "Test" +version = "0.1.0" + +[[capsule]] +name = "astrid-capsule-cli" +source = "@org/cli" +version = "" +tag = "v0.2.0-rc1" +role = "uplink" +"#; + let manifest: DistroManifest = toml::from_str(toml_src).unwrap(); + validate_manifest(&manifest).expect("a non-empty tag is a concrete release selector"); + } + + #[test] + fn rejects_missing_or_malformed_release_selector() { + let base = r#" +schema-version = 1 + +[distro] +id = "test" +name = "Test" +version = "0.1.0" + +[[capsule]] +name = "astrid-capsule-cli" +source = "@org/cli" +role = "uplink" +"#; + for (selector, needle) in [ + ("version = \"\"", "concrete released version or tag"), + ("version = \"latest\"", "not valid semver"), + ("version = \"\"\ntag = \"\"", "tag must not be empty"), + ] { + let manifest: DistroManifest = toml::from_str(&format!("{base}{selector}\n")).unwrap(); + let err = validate_manifest(&manifest).unwrap_err(); + assert!(err.to_string().contains(needle), "got: {err:#}"); + } + } + #[test] fn astrid_version_rejects_running_below_floor() { // Running CLI is older than the distro's floor — must reject with the diff --git a/crates/astrid-cli/src/commands/init.rs b/crates/astrid-cli/src/commands/init.rs index b18213cf1..b9c2ecf24 100644 --- a/crates/astrid-cli/src/commands/init.rs +++ b/crates/astrid-cli/src/commands/init.rs @@ -8,6 +8,7 @@ use std::collections::HashMap; use std::io::Write; use anyhow::{Context, bail}; +use astrid_capsule::capsule::CapsuleId; use astrid_core::dirs::AstridHome; use indicatif::{ProgressBar, ProgressStyle}; @@ -38,6 +39,16 @@ pub(crate) struct InitOpts { pub(crate) accept_new_key: bool, /// Pre-supplied variable values from `--var KEY=VALUE`. pub(crate) vars: HashMap, + /// Principal whose local home, capsule installs, env, lock, and grants + /// this invocation provisions. The process principal is separately used + /// to authenticate admin requests. + pub(crate) target_principal: astrid_core::PrincipalId, + /// Grant the target principal capsule-access for every capsule the + /// distro installs, via the same kernel path as `astrid agent modify + /// --add-capsule`. Only valid with a resolved distro. Opt-in: without it, + /// `init` installs capsules but attaches no grants and + /// prints the manual `agent modify` command for discoverability. + pub(crate) grant_capsules: bool, } /// Parse `--var KEY=VALUE` strings into a map. @@ -60,31 +71,49 @@ pub(crate) fn parse_cli_vars(raw: &[String]) -> anyhow::Result anyhow::Result<()> { let home = AstridHome::resolve()?; - home.ensure()?; - - // The process-wide principal resolved from the global `--principal` flag - // (falling back to ASTRID_PRINCIPAL / active-agent context / `default`). - // Every install target below — capsule files, per-capsule env config, and - // the Distro.lock — is scoped to THIS principal so `astrid init --principal - // ` provisions 's home rather than always landing under `default`. - // This is the same source `astrid capsule install` reads, so init and the - // manual installer agree on where a principal's capsules live. - let principal = crate::principal::current(); - - // Workspace init (existing behaviour). - init_workspace()?; - - // Offline, signed, self-contained install path. + let operator = crate::principal::current(); + let target = opts.target_principal.clone(); + + // `--grant-capsules` is only meaningful when a distro install is + // resolving the capsule set to grant. Reject an empty source up front so + // the flag can never be silently honoured without a distro. + grant::validate_grant_capsules(opts.grant_capsules, !distro_source.is_empty())?; + + // Offline, signed, self-contained install path. `--grant-capsules` is not + // wired for `.shuttle` archives (the installed set isn't threaded back + // here); fail loud rather than silently skip the grant the operator asked + // for, pointing at the manual path. if distro_source.ends_with(".shuttle") { + if opts.grant_capsules { + bail!( + "--grant-capsules is not supported for .shuttle installs yet — \ + install first, then grant with `astrid --principal {operator} \ + agent modify {target} \ + --add-capsule ` for each installed capsule." + ); + } + home.ensure()?; + let _provisioning_lock = grant::ProvisioningLock::acquire(&home, &target)?; + init_workspace()?; return run_init_from_shuttle(distro_source, opts); } + // Refuse an unauthorized or nonexistent grant target before creating any + // local workspace, principal-home, env, capsule, or lock state. + if opts.grant_capsules { + grant::preflight_grants(&operator, &target).await?; + } + + home.ensure()?; + let _provisioning_lock = grant::ProvisioningLock::acquire(&home, &target)?; + init_workspace()?; + // Check lockfile — if fresh, we're already initialized. The lock lives // under the resolved principal's home, matching bootstrap's auto-init // freshness check (`should_auto_init`) so a scoped principal isn't // re-provisioned on every run. let lock_path = home - .principal_home(&principal) + .principal_home(&target) .config_dir() .join("distro.lock"); @@ -101,6 +130,8 @@ pub(crate) async fn run_init(distro_source: &str, opts: &InitOpts) -> anyhow::Re // Check lock freshness AFTER parsing manifest (need manifest to compare). if let Some(existing_lock) = load_lock(&lock_path)? && is_lock_fresh(&existing_lock, &manifest) + && let Some(installed) = + grant::validated_grant_set_for_reuse(&home, &target, &existing_lock.capsules) { eprintln!( "{}", @@ -113,6 +144,13 @@ pub(crate) async fn run_init(distro_source: &str, opts: &InitOpts) -> anyhow::Re .unwrap_or(&manifest.distro.name), )) ); + // Idempotent grant path: a re-run with `--grant-capsules` on an + // already-installed principal still (re-)applies grants for the + // locked capsule set. `apply_set_delta` dedups kernel-side, so a + // principal that already holds them reports "no change" rather than + // erroring or duplicating — and this is also how a first run whose + // grant step failed (daemon was down) recovers on re-run. + grant::apply_or_hint_grants(&operator, &target, &installed, opts.grant_capsules).await?; return Ok(()); } @@ -143,14 +181,14 @@ pub(crate) async fn run_init(distro_source: &str, opts: &InitOpts) -> anyhow::Re // Write per-capsule env files BEFORE installing capsules so that // install_capsule's onboarding check finds existing values and // doesn't re-prompt for fields the distro already configured. - write_env_files(&home, &principal, &selected, &vars)?; + write_env_files(&home, &target, &selected, &vars)?; // Install each capsule with progress. `install_capsules` writes the // capsule files under `principal`'s home and returns one LockedCapsule // per capsule that actually installed — failures are reported and // dropped, so `locked.len()` is the true success count. let total = selected.len(); - let locked = install_capsules(&selected, opts.offline, &principal).await?; + let locked = install_capsules(&selected, opts.offline, &target).await?; let succeeded = locked.len(); // Provisioning honesty: a run where every selected install FAILED must @@ -174,7 +212,7 @@ pub(crate) async fn run_init(distro_source: &str, opts: &InitOpts) -> anyhow::Re // shared free-text `[variables]`. Shared values already written above // are preserved (the prompt skips set keys). if should_write_lock(total, succeeded) { - onboard_llm_providers(&home, &principal, &selected); + onboard_llm_providers(&home, &target, &selected); } // Persist Distro.lock iff the run earned it (full success or empty @@ -182,12 +220,23 @@ pub(crate) async fn run_init(distro_source: &str, opts: &InitOpts) -> anyhow::Re // actually retries the missing capsules instead of short-circuiting at // the freshness gate (`is_lock_fresh` diffs only distro id+version, not // the capsule set) — see `should_write_lock`. + // Capture the names that actually installed BEFORE `locked` is consumed + // into the lock — the grant set is EXACTLY this locally-resolved set, never + // a manifest-declared string the installer didn't land (security stance: + // grants derive from what was installed, on explicit `--grant-capsules`). + let installed_names: Vec = locked.iter().map(|c| c.name.clone()).collect(); let lock = create_lock_from_parts(schema_version, &distro_id, &distro_version, locked); let wrote_lock = persist_lock_if_earned(&lock_path, total, succeeded, &lock)?; eprintln!(); if wrote_lock { eprintln!("{}", Theme::success("Installation complete.")); + // Apply capsule grants (opt-in) or print the discoverability hint. + // On a grant failure the capsules are already installed and the lock + // is written; this returns Err so init exits non-zero with the exact + // manual command to finish. + grant::apply_or_hint_grants(&operator, &target, &installed_names, opts.grant_capsules) + .await?; eprintln!(" Run {} to start.", Theme::prompt("astrid")); Ok(()) } else { @@ -670,11 +719,10 @@ async fn install_capsules( let mut locked = Vec::with_capacity(total); let mut failed = Vec::new(); - let home = AstridHome::resolve()?; - for cap in selected { pb.set_message(cap.name.clone()); + let expected = CapsuleId::new(cap.name.clone())?; let refspec = super::capsule::install::RefSpec::from_capsule(cap); // The installer returns the ref it ACTUALLY resolved and fetched // (`Some` for GitHub sources, `None` for local paths). Record @@ -682,15 +730,25 @@ async fn install_capsules( // lock attests what was truly installed. `Some(&cap.name)` is the // name hint used to pick the right archive from a multi-asset // release. - let resolved_ref = match super::capsule::install::install_capsule_batch( + let outcome = match super::capsule::install::install_capsule_batch( &cap.source, - Some(&cap.name), + &expected, false, &refspec, + principal, ) .await { - Ok(resolved_ref) => resolved_ref, + Ok(outcome) => outcome, + Err(e) => { + eprintln!("\n Failed to install {}: {e}", cap.name); + failed.push(cap.name.clone()); + pb.inc(1); + continue; + }, + }; + let verified = match validate_batch_install(&expected, &cap.version, outcome) { + Ok(verified) => verified, Err(e) => { eprintln!("\n Failed to install {}: {e}", cap.name); failed.push(cap.name.clone()); @@ -698,25 +756,16 @@ async fn install_capsules( continue; }, }; - - // Read the installed meta to get the wasm_hash for the lock. The - // capsule was installed under `principal`'s home (via the - // `*_for_principal` install lib), so read it back from there — not - // from the legacy `default`-principal resolver, which would miss a - // scoped principal's install and record an empty hash. - let target_dir = - super::capsule::install::resolve_target_dir_for(&home, principal, &cap.name, false)?; - let meta = super::capsule::meta::read_meta(&target_dir); locked.push(LockedCapsule { name: cap.name.clone(), - version: cap.version.clone(), + version: verified.version, source: cap.source.clone(), - hash: meta - .and_then(|m| m.wasm_hash) + hash: verified + .wasm_hash .map(|h| format!("blake3:{h}")) .unwrap_or_default(), - resolved_ref, + resolved_ref: verified.resolved_ref, }); pb.inc(1); @@ -738,6 +787,55 @@ async fn install_capsules( Ok(locked) } +#[derive(Debug)] +struct VerifiedBatchInstall { + version: String, + wasm_hash: Option, + resolved_ref: Option, +} + +/// Require the checked installer to report one exact identity and an actual +/// version consistent with the distro's released selector. +fn validate_batch_install( + expected: &CapsuleId, + declared_version: &str, + outcome: super::capsule::install::BatchInstallOutcome, +) -> anyhow::Result { + if outcome.installed.len() != 1 { + let actual = outcome + .installed + .iter() + .map(|installed| installed.id.as_str()) + .collect::>() + .join(", "); + bail!( + "distro declared capsule '{expected}', but the checked installer reported [{actual}]" + ); + } + let installed = outcome + .installed + .into_iter() + .next() + .expect("length checked"); + if installed.id != *expected { + bail!( + "distro declared capsule '{expected}', but the checked installer reported '{}'", + installed.id + ); + } + if !declared_version.is_empty() && installed.version != declared_version { + bail!( + "capsule '{expected}' release selector declared version {declared_version}, but the installed manifest reports {}", + installed.version + ); + } + Ok(VerifiedBatchInstall { + version: installed.version, + wasm_hash: installed.wasm_hash, + resolved_ref: outcome.resolved_ref, + }) +} + /// Write per-capsule .env.json files with resolved variable templates. pub(crate) fn write_env_files( home: &AstridHome, @@ -843,6 +941,11 @@ fn onboard_llm_providers( } } +/// `--grant-capsules` post-install grant logic, split out to keep this +/// file under the per-file size cap. +#[path = "init_grant.rs"] +mod grant; + #[cfg(test)] #[path = "init_tests.rs"] mod tests; diff --git a/crates/astrid-cli/src/commands/init_grant.rs b/crates/astrid-cli/src/commands/init_grant.rs new file mode 100644 index 000000000..1ac941389 --- /dev/null +++ b/crates/astrid-cli/src/commands/init_grant.rs @@ -0,0 +1,738 @@ +//! `--grant-capsules`: attach capsule-access grants for the capsules a +//! distro just installed to the target principal, via the same +//! `admin.agent.modify` kernel path as `astrid agent modify --add-capsule`. +//! +//! Split out of `init.rs` (referenced via `#[path]`) so that file stays +//! under the per-file CI line cap. The two entry points `run_init` calls +//! are [`validate_grant_capsules`] (a pure up-front guard) and +//! [`apply_or_hint_grants`] (the post-install grant / hint dispatcher). + +use std::fs::{File, OpenOptions}; +use std::future::Future; + +use anyhow::{Context, bail}; +use astrid_capsule::capsule::CapsuleId; +use astrid_capsule::manifest::CapsuleManifest; +use astrid_core::PrincipalId; +use astrid_core::dirs::AstridHome; +use astrid_core::kernel_api::{AdminRequestKind, AdminResponseBody}; +use fs2::FileExt; + +use crate::theme::Theme; + +/// Guard: `--grant-capsules` may only be honoured alongside a distro +/// install, because the grant set is exactly the capsules that distro +/// installs. `distro_present` is whether a non-empty distro source +/// resolved. Pure so the invariant is unit-testable without a network +/// install. +/// +/// # Errors +/// Returns an error when `grant_capsules` is set but no distro source is +/// present. +pub(super) fn validate_grant_capsules( + grant_capsules: bool, + distro_present: bool, +) -> anyhow::Result<()> { + if grant_capsules && !distro_present { + bail!( + "--grant-capsules requires a resolved distro: grants apply to the capsules a distro installs" + ); + } + Ok(()) +} + +/// What the post-install grant step should do, given the flag and how many +/// capsules installed. Explicit requests always exercise the kernel grant +/// path; the CLI never infers privilege from a principal name. +#[derive(Debug, PartialEq, Eq)] +enum GrantAction { + /// Nothing installed this run — no grants, no hint. + Nothing, + /// Flag omitted — print the manual hint. + Hint, + /// Flag set — apply the grants. + Grant, +} + +fn grant_action(grant_capsules: bool, installed_count: usize) -> GrantAction { + if installed_count == 0 { + return GrantAction::Nothing; + } + if grant_capsules { + GrantAction::Grant + } else { + GrantAction::Hint + } +} + +/// Render the exact `astrid agent modify … --add-capsule …` command that +/// grants `capsules` to `principal`. Shared by the discoverability hint +/// (flag omitted) and the grant-failure recovery message so both print an +/// identical, copy-pasteable command. +fn agent_modify_grant_command( + operator: &PrincipalId, + target: &PrincipalId, + capsules: &[String], +) -> String { + let flags = capsules + .iter() + .map(|c| format!("--add-capsule {c}")) + .collect::>() + .join(" "); + format!("astrid --principal {operator} agent modify {target} {flags}") +} + +/// Read-only authorization and target-existence check for the exact mutation +/// `--grant-capsules` will perform. This runs before init creates local state. +pub(super) async fn preflight_grants( + operator: &PrincipalId, + target: &PrincipalId, +) -> anyhow::Result<()> { + preflight_sequence( + crate::commands::daemon::ensure_daemon("init grant preflight"), + || async move { + let mut client = crate::admin_client::AdminClient::connect(operator.clone()) + .await + .context("grant preflight could not connect to the daemon")?; + let body = client + .request(AdminRequestKind::AgentModify { + principal: target.clone(), + add_groups: Vec::new(), + remove_groups: Vec::new(), + add_capsules: Vec::new(), + remove_capsules: Vec::new(), + }) + .await + .context("grant preflight request failed")?; + match crate::admin_client::into_result(body)? { + AdminResponseBody::Success(_) => Ok(()), + other => bail!("grant preflight returned an unexpected response: {other:?}"), + } + }, + ) + .await +} + +async fn preflight_sequence(ensure_daemon: E, check: C) -> anyhow::Result<()> +where + E: Future>, + C: FnOnce() -> F, + F: Future>, +{ + ensure_daemon + .await + .context("grant preflight could not ensure the runtime daemon")?; + check().await +} + +/// Owner-private, non-blocking lock serializing distro provisioning for one +/// `(AstridHome, target principal)` pair. The file remains in place after +/// unlock so concurrent processes always contend on the same inode. +pub(super) struct ProvisioningLock { + _file: File, +} + +impl ProvisioningLock { + pub(super) fn acquire(home: &AstridHome, target: &PrincipalId) -> anyhow::Result { + let config_dir = home.principal_home(target).config_dir(); + std::fs::create_dir_all(&config_dir) + .with_context(|| format!("failed to create {}", config_dir.display()))?; + set_owner_private_dir(&config_dir)?; + + let path = config_dir.join("distro.init.lock"); + let mut options = OpenOptions::new(); + options.read(true).write(true).create(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let file = options + .open(&path) + .with_context(|| format!("failed to open {}", path.display()))?; + set_owner_private_file(&path)?; + FileExt::try_lock_exclusive(&file).with_context(|| { + format!("another distro provision is already running for target principal '{target}'") + })?; + Ok(Self { _file: file }) + } +} + +/// Prove that every fresh-lock entry still describes an installed capsule for +/// the target before those names become an authorization grant set. +pub(super) fn validate_locked_capsules( + home: &AstridHome, + target: &PrincipalId, + locked: &[super::LockedCapsule], +) -> anyhow::Result> { + let mut installed = Vec::with_capacity(locked.len()); + for capsule in locked { + let expected = CapsuleId::new(capsule.name.clone())?; + let target_dir = super::super::capsule::install::resolve_target_dir_for( + home, + target, + expected.as_str(), + false, + )?; + let manifest_path = target_dir.join("Capsule.toml"); + let manifest = astrid_capsule::discovery::load_manifest(&manifest_path).with_context(|| { + format!( + "Distro.lock is fresh but capsule '{}' is not installed correctly for '{}'; rerun init after removing the stale lock", + capsule.name, target + ) + })?; + let actual = CapsuleId::new(manifest.package.name.clone())?; + if actual != expected { + bail!( + "Distro.lock capsule '{expected}' resolves to installed manifest '{actual}'; refusing to grant stale identity" + ); + } + let meta = super::super::capsule::meta::read_meta(&target_dir).ok_or_else(|| { + anyhow::anyhow!( + "Distro.lock capsule '{}' has no readable install metadata for target '{}'", + capsule.name, + target + ) + })?; + if manifest.package.version != meta.version { + bail!( + "installed capsule '{}' version disagrees between Capsule.toml ({}) and meta.json ({})", + expected, + manifest.package.version, + meta.version + ); + } + if !capsule.version.is_empty() && meta.version != capsule.version { + bail!( + "Distro.lock capsule '{}' expects version {}, but meta.json reports {}", + capsule.name, + capsule.version, + meta.version + ); + } + validate_locked_wasm( + home, + &expected, + &manifest, + meta.wasm_hash.as_deref(), + &capsule.hash, + )?; + installed.push(expected.as_str().to_string()); + } + Ok(installed) +} + +/// Reuse a fresh lock only when its installed state still verifies. A current +/// distro id/version with stale or incomplete install provenance falls through +/// to the normal checked install path so init can regenerate the lock. +pub(super) fn validated_grant_set_for_reuse( + home: &AstridHome, + target: &PrincipalId, + locked: &[super::LockedCapsule], +) -> Option> { + match validate_locked_capsules(home, target, locked) { + Ok(installed) => Some(installed), + Err(error) => { + eprintln!( + "{}", + Theme::warning(&format!( + "Distro.lock is current but installed state failed verification ({error:#}); reinstalling" + )) + ); + None + }, + } +} + +fn validate_locked_wasm( + home: &AstridHome, + capsule: &CapsuleId, + manifest: &CapsuleManifest, + meta_hash: Option<&str>, + locked_hash: &str, +) -> anyhow::Result<()> { + let declares_wasm = manifest_declares_wasm(manifest); + let Some(meta_hash) = meta_hash else { + if declares_wasm { + bail!("Distro.lock capsule '{capsule}' declares WASM but has no installed WASM hash"); + } + if !locked_hash.is_empty() { + bail!("Distro.lock non-WASM capsule '{capsule}' must not carry a WASM hash"); + } + return Ok(()); + }; + + if !declares_wasm { + bail!( + "Distro.lock capsule '{capsule}' does not declare WASM but installed metadata carries a WASM hash" + ); + } + let locked = parse_locked_blake3(capsule, locked_hash)?; + let locked_hex = locked.to_hex().to_string(); + if meta_hash != locked_hex { + bail!("Distro.lock capsule '{capsule}' hash disagrees with installed metadata"); + } + let blob_path = home.bin_dir().join(format!("{locked_hex}.wasm")); + let bytes = std::fs::read(&blob_path).with_context(|| { + format!( + "Distro.lock capsule '{}' content blob is missing or unreadable at {}", + capsule, + blob_path.display() + ) + })?; + let actual = blake3::hash(&bytes); + if actual != locked { + bail!("Distro.lock capsule '{capsule}' content blob bytes do not match hash {locked_hash}"); + } + Ok(()) +} + +fn parse_locked_blake3(capsule: &CapsuleId, value: &str) -> anyhow::Result { + let Some(hex) = value.strip_prefix("blake3:") else { + bail!("Distro.lock capsule '{capsule}' requires a canonical blake3: WASM hash"); + }; + let hash = blake3::Hash::from_hex(hex).map_err(|_| { + anyhow::anyhow!("Distro.lock capsule '{capsule}' has an invalid BLAKE3 hash") + })?; + if hex.len() != 64 || hash.to_hex().as_str() != hex { + bail!("Distro.lock capsule '{capsule}' requires a canonical lowercase BLAKE3 hash"); + } + Ok(hash) +} + +fn manifest_declares_wasm(manifest: &CapsuleManifest) -> bool { + manifest + .components + .iter() + .any(|component| component.path.extension().and_then(|ext| ext.to_str()) == Some("wasm")) +} + +#[cfg(unix)] +fn set_owner_private_dir(path: &std::path::Path) -> anyhow::Result<()> { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700))?; + Ok(()) +} + +#[cfg(not(unix))] +fn set_owner_private_dir(_path: &std::path::Path) -> anyhow::Result<()> { + Ok(()) +} + +#[cfg(unix)] +fn set_owner_private_file(path: &std::path::Path) -> anyhow::Result<()> { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; + Ok(()) +} + +#[cfg(not(unix))] +fn set_owner_private_file(_path: &std::path::Path) -> anyhow::Result<()> { + Ok(()) +} + +/// Apply capsule-access grants for the installed set (opt-in), or print +/// the discoverability hint when the flag was omitted. +/// +/// On the grant path the capsules are already installed and the lock is +/// written; a failure here (daemon unreachable, caller lacks `agent:modify`) +/// returns `Err` so `init` exits non-zero, but always prints the exact +/// manual command to finish. The kernel applies the whole `add_capsules` +/// set atomically, so grants are all-or-nothing rather than partial. +pub(super) async fn apply_or_hint_grants( + operator: &PrincipalId, + target: &PrincipalId, + installed: &[String], + grant_capsules: bool, +) -> anyhow::Result<()> { + match grant_action(grant_capsules, installed.len()) { + GrantAction::Nothing => Ok(()), + GrantAction::Hint => { + eprintln!(); + eprintln!( + "{}", + Theme::info(&format!( + "Capsules were installed for '{target}' but not granted. To let it invoke them:" + )) + ); + eprintln!( + " {}", + agent_modify_grant_command(operator, target, installed) + ); + eprintln!(" (or re-run `astrid init` with --grant-capsules)"); + Ok(()) + }, + GrantAction::Grant => grant_installed_capsules(operator, target, installed).await, + } +} + +/// Grant the installed capsule set to `principal` via the shared +/// `admin.agent.modify` path (the same one `astrid agent modify +/// --add-capsule` uses). Idempotent: a re-run over an already-granted +/// principal reports "no change" instead of erroring or duplicating. +async fn grant_installed_capsules( + operator: &PrincipalId, + target: &PrincipalId, + installed: &[String], +) -> anyhow::Result<()> { + eprintln!(); + eprintln!( + "{}", + Theme::info(&format!( + "Granting {} capsule(s) to '{target}'...", + installed.len() + )) + ); + + let mut client = match crate::admin_client::AdminClient::connect(operator.clone()).await { + Ok(c) => c, + Err(e) => { + bail!( + "capsules are installed, but connecting to the daemon to grant access failed: {e}\n \ + Grant them once the daemon is running:\n {}", + agent_modify_grant_command(operator, target, installed) + ); + }, + }; + + match crate::commands::agent::apply_agent_modify(&mut client, target, &[], &[], installed, &[]) + .await + { + Ok(outcome) if outcome.changed => { + eprintln!( + "{}", + Theme::success(&format!( + "Granted capsule access to '{target}': [{}]", + outcome.capsules.join(", ") + )) + ); + Ok(()) + }, + Ok(_) => { + eprintln!( + "{}", + Theme::info(&format!( + "'{target}' already had access to every installed capsule (no change)." + )) + ); + Ok(()) + }, + Err(e) => { + bail!( + "capsules are installed, but granting capsule access failed: {e}\n \ + Finish manually:\n {}", + agent_modify_grant_command(operator, target, installed) + ); + }, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::commands::capsule::{install, meta}; + use crate::commands::init::LockedCapsule; + + /// (c) The flag is only meaningful with a distro to resolve the grant + /// set: setting it without a distro source is a hard error. + #[test] + fn grant_capsules_requires_a_distro() { + // Flag set, no distro → error. + let err = validate_grant_capsules(true, false).unwrap_err(); + assert!(err.to_string().contains("--grant-capsules"), "got: {err}"); + assert!(err.to_string().contains("resolved distro"), "got: {err}"); + + // Flag set with a distro → allowed. + assert!(validate_grant_capsules(true, true).is_ok()); + // Flag unset → always allowed (distro present or not). + assert!(validate_grant_capsules(false, false).is_ok()); + assert!(validate_grant_capsules(false, true).is_ok()); + } + + /// With the flag set, an installed set takes the grant path. + #[test] + fn grant_action_grants_for_non_default_with_flag() { + assert_eq!(grant_action(true, 3), GrantAction::Grant); + } + + /// Omitting the flag leaves grants absent and prints the hint. + #[test] + fn grant_action_hints_when_flag_omitted() { + assert_eq!(grant_action(false, 3), GrantAction::Hint); + } + + #[test] + fn explicit_grant_does_not_special_case_reserved_names() { + assert_eq!(grant_action(true, 3), GrantAction::Grant); + } + + /// Nothing installed → no grant attempt and no hint, whatever the flag. + #[test] + fn grant_action_does_nothing_with_empty_install_set() { + assert_eq!(grant_action(true, 0), GrantAction::Nothing); + assert_eq!(grant_action(false, 0), GrantAction::Nothing); + } + + /// (d) Idempotency: a re-run always re-derives the same Grant action + /// (the CLI unconditionally re-issues the grant), and the kernel's + /// `apply_set_delta` dedups so an already-granted principal reports "no + /// change" rather than erroring or duplicating. The decision function + /// is pure over its inputs, so two identical runs plan identically. + #[test] + fn grant_action_is_stable_across_reruns() { + let first = grant_action(true, 2); + let second = grant_action(true, 2); + assert_eq!(first, second, "a re-run must plan the same grant"); + assert_eq!(first, GrantAction::Grant); + } + + /// Both the no-flag hint and the failure-recovery message print an + /// identical, copy-pasteable `agent modify` command carrying every + /// installed capsule as a repeated `--add-capsule`. + #[test] + fn agent_modify_grant_command_lists_every_capsule() { + let operator = PrincipalId::new("operator").unwrap(); + let alice = PrincipalId::new("alice").unwrap(); + let caps = vec!["cli".to_string(), "openai".to_string()]; + let cmd = agent_modify_grant_command(&operator, &alice, &caps); + assert_eq!( + cmd, + "astrid --principal operator agent modify alice --add-capsule cli --add-capsule openai" + ); + } + + #[test] + fn fresh_lock_without_flag_plans_operator_aware_hint_only() { + let operator = PrincipalId::new("operator").unwrap(); + let target = PrincipalId::new("agent-1").unwrap(); + let installed = vec!["cli".to_string()]; + + assert_eq!(grant_action(false, installed.len()), GrantAction::Hint); + assert_eq!( + agent_modify_grant_command(&operator, &target, &installed), + "astrid --principal operator agent modify agent-1 --add-capsule cli" + ); + } + + #[test] + fn provisioning_lock_rejects_contention_and_can_be_reacquired() { + let dir = tempfile::tempdir().unwrap(); + let home = AstridHome::from_path(dir.path()); + let target = PrincipalId::new("alice").unwrap(); + let first = ProvisioningLock::acquire(&home, &target).unwrap(); + let err = ProvisioningLock::acquire(&home, &target) + .err() + .expect("a concurrent provision must not acquire the same lock"); + assert!(err.to_string().contains("already running"), "got: {err:#}"); + drop(first); + ProvisioningLock::acquire(&home, &target).unwrap(); + } + + #[tokio::test] + async fn preflight_propagates_daemon_start_failure_without_target_state() { + let dir = tempfile::tempdir().unwrap(); + let target_state = dir.path().join("home/target"); + let check_called = std::cell::Cell::new(false); + + let err = preflight_sequence(async { anyhow::bail!("daemon boot failed") }, || async { + check_called.set(true); + Ok(()) + }) + .await + .unwrap_err(); + + assert!(err.to_string().contains("could not ensure"), "got: {err:#}"); + assert!(!check_called.get()); + assert!(!target_state.exists()); + } + + #[tokio::test] + async fn preflight_propagates_authorization_failure_without_target_state() { + let dir = tempfile::tempdir().unwrap(); + let target_state = dir.path().join("home/target"); + + let err = preflight_sequence(async { Ok(()) }, || async { + anyhow::bail!("agent:modify denied") + }) + .await + .unwrap_err(); + + assert!(err.to_string().contains("agent:modify denied")); + assert!(!target_state.exists()); + } + + #[test] + fn fresh_lock_requires_matching_installed_manifest_and_metadata() { + let dir = tempfile::tempdir().unwrap(); + let home = AstridHome::from_path(dir.path()); + let target = PrincipalId::new("alice").unwrap(); + let wasm = b"real wasm bytes"; + let hash = blake3::hash(wasm).to_hex().to_string(); + let install_dir = install::resolve_target_dir_for(&home, &target, "cli", false).unwrap(); + std::fs::create_dir_all(&install_dir).unwrap(); + std::fs::write( + install_dir.join("Capsule.toml"), + "[package]\nname = \"cli\"\nversion = \"1.0.0\"\n\n[[component]]\nid = \"main\"\nfile = \"cli.wasm\"\n", + ) + .unwrap(); + std::fs::create_dir_all(home.bin_dir()).unwrap(); + std::fs::write(home.bin_dir().join(format!("{hash}.wasm")), wasm).unwrap(); + let meta = meta::CapsuleMeta { + version: "1.0.0".to_string(), + wasm_hash: Some(hash.clone()), + ..Default::default() + }; + meta::write_meta(&install_dir, &meta).unwrap(); + let locked = vec![LockedCapsule { + name: "cli".to_string(), + version: "1.0.0".to_string(), + source: "@example/cli".to_string(), + hash: format!("blake3:{hash}"), + resolved_ref: Some("v1.0.0".to_string()), + }]; + + assert_eq!( + validate_locked_capsules(&home, &target, &locked).unwrap(), + vec!["cli"] + ); + + let mismatched = meta::CapsuleMeta { + version: "2.0.0".to_string(), + wasm_hash: Some(hash), + ..Default::default() + }; + meta::write_meta(&install_dir, &mismatched).unwrap(); + let err = validate_locked_capsules(&home, &target, &locked).unwrap_err(); + assert!(err.to_string().contains("disagrees between Capsule.toml")); + } + + #[test] + fn fresh_lock_rehashes_content_blob_bytes() { + let dir = tempfile::tempdir().unwrap(); + let home = AstridHome::from_path(dir.path()); + let target = PrincipalId::new("alice").unwrap(); + let install_dir = install::resolve_target_dir_for(&home, &target, "cli", false).unwrap(); + std::fs::create_dir_all(&install_dir).unwrap(); + std::fs::write( + install_dir.join("Capsule.toml"), + "[package]\nname = \"cli\"\nversion = \"1.0.0\"\n\n[[component]]\nfile = \"cli.wasm\"\n", + ) + .unwrap(); + let hash = blake3::hash(b"original").to_hex().to_string(); + meta::write_meta( + &install_dir, + &meta::CapsuleMeta { + version: "1.0.0".to_string(), + wasm_hash: Some(hash.clone()), + ..Default::default() + }, + ) + .unwrap(); + std::fs::create_dir_all(home.bin_dir()).unwrap(); + let blob = home.bin_dir().join(format!("{hash}.wasm")); + std::fs::write(&blob, b"tampered").unwrap(); + let locked = vec![LockedCapsule { + name: "cli".to_string(), + version: "1.0.0".to_string(), + source: "@example/cli".to_string(), + hash: format!("blake3:{hash}"), + resolved_ref: Some("v1.0.0".to_string()), + }]; + + let err = validate_locked_capsules(&home, &target, &locked).unwrap_err(); + assert!(err.to_string().contains("blob bytes do not match")); + assert!(validated_grant_set_for_reuse(&home, &target, &locked).is_none()); + std::fs::remove_file(blob).unwrap(); + let err = validate_locked_capsules(&home, &target, &locked).unwrap_err(); + assert!(err.to_string().contains("missing or unreadable")); + } + + #[test] + fn fresh_lock_allows_empty_hash_only_for_non_wasm_capsule() { + let dir = tempfile::tempdir().unwrap(); + let home = AstridHome::from_path(dir.path()); + let target = PrincipalId::new("alice").unwrap(); + let install_dir = install::resolve_target_dir_for(&home, &target, "mcp", false).unwrap(); + std::fs::create_dir_all(&install_dir).unwrap(); + let manifest_path = install_dir.join("Capsule.toml"); + std::fs::write( + &manifest_path, + "[package]\nname = \"mcp\"\nversion = \"1.0.0\"\n", + ) + .unwrap(); + meta::write_meta( + &install_dir, + &meta::CapsuleMeta { + version: "1.0.0".to_string(), + wasm_hash: None, + ..Default::default() + }, + ) + .unwrap(); + let locked = vec![LockedCapsule { + name: "mcp".to_string(), + version: "1.0.0".to_string(), + source: "@example/mcp".to_string(), + hash: String::new(), + resolved_ref: Some("v1.0.0".to_string()), + }]; + assert_eq!( + validate_locked_capsules(&home, &target, &locked).unwrap(), + vec!["mcp"] + ); + + let stray_hash = blake3::hash(b"stray").to_hex().to_string(); + meta::write_meta( + &install_dir, + &meta::CapsuleMeta { + version: "1.0.0".to_string(), + wasm_hash: Some(stray_hash.clone()), + ..Default::default() + }, + ) + .unwrap(); + let mut hashed_non_wasm = locked.clone(); + hashed_non_wasm[0].hash = format!("blake3:{stray_hash}"); + let err = validate_locked_capsules(&home, &target, &hashed_non_wasm).unwrap_err(); + assert!(err.to_string().contains("does not declare WASM")); + + meta::write_meta( + &install_dir, + &meta::CapsuleMeta { + version: "1.0.0".to_string(), + wasm_hash: None, + ..Default::default() + }, + ) + .unwrap(); + + std::fs::write( + manifest_path, + "[package]\nname = \"mcp\"\nversion = \"1.0.0\"\n\n[[component]]\nfile = \"helper.js\"\n\n[[component]]\nfile = \"mcp.wasm\"\n", + ) + .unwrap(); + let err = validate_locked_capsules(&home, &target, &locked).unwrap_err(); + assert!(err.to_string().contains("declares WASM")); + } + + #[cfg(unix)] + #[test] + fn provisioning_lock_is_owner_private() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let home = AstridHome::from_path(dir.path()); + let target = PrincipalId::new("alice").unwrap(); + let _lock = ProvisioningLock::acquire(&home, &target).unwrap(); + let config_dir = home.principal_home(&target).config_dir(); + let lock_path = config_dir.join("distro.init.lock"); + assert_eq!( + std::fs::metadata(config_dir).unwrap().permissions().mode() & 0o777, + 0o700 + ); + assert_eq!( + std::fs::metadata(lock_path).unwrap().permissions().mode() & 0o777, + 0o600 + ); + } +} diff --git a/crates/astrid-cli/src/commands/init_tests.rs b/crates/astrid-cli/src/commands/init_tests.rs index 7be148dec..c7e9c486a 100644 --- a/crates/astrid-cli/src/commands/init_tests.rs +++ b/crates/astrid-cli/src/commands/init_tests.rs @@ -4,6 +4,92 @@ use super::*; +#[test] +fn batch_install_rejects_reported_identity_mismatch() { + let expected = astrid_capsule::capsule::CapsuleId::new("expected-capsule").unwrap(); + let err = validate_batch_install( + &expected, + "1.0.0", + super::super::capsule::install::BatchInstallOutcome { + installed: vec![super::super::capsule::install::InstalledCapsuleOutcome { + id: astrid_capsule::capsule::CapsuleId::new("wrong-capsule").unwrap(), + version: "1.0.0".to_string(), + wasm_hash: Some("abcd".to_string()), + }], + resolved_ref: Some("v1.0.0".to_string()), + }, + ) + .unwrap_err(); + + assert!(err.to_string().contains("expected-capsule"), "got: {err:#}"); +} + +#[test] +fn batch_install_accepts_actual_version_hash_and_ref() { + let expected = astrid_capsule::capsule::CapsuleId::new("expected-capsule").unwrap(); + let verified = validate_batch_install( + &expected, + "1.0.0", + super::super::capsule::install::BatchInstallOutcome { + installed: vec![super::super::capsule::install::InstalledCapsuleOutcome { + id: expected.clone(), + version: "1.0.0".to_string(), + wasm_hash: Some("abcd".to_string()), + }], + resolved_ref: Some("v1.0.0".to_string()), + }, + ) + .unwrap(); + assert_eq!(verified.version, "1.0.0"); + assert_eq!(verified.wasm_hash.as_deref(), Some("abcd")); + assert_eq!(verified.resolved_ref.as_deref(), Some("v1.0.0")); +} + +#[test] +fn batch_install_rejects_multiple_reported_capsules() { + let expected = astrid_capsule::capsule::CapsuleId::new("expected-capsule").unwrap(); + let err = validate_batch_install( + &expected, + "1.0.0", + super::super::capsule::install::BatchInstallOutcome { + installed: vec![ + super::super::capsule::install::InstalledCapsuleOutcome { + id: expected.clone(), + version: "1.0.0".to_string(), + wasm_hash: None, + }, + super::super::capsule::install::InstalledCapsuleOutcome { + id: astrid_capsule::capsule::CapsuleId::new("wrong-capsule").unwrap(), + version: "1.0.0".to_string(), + wasm_hash: None, + }, + ], + resolved_ref: None, + }, + ) + .unwrap_err(); + assert!(err.to_string().contains("checked installer reported")); +} + +#[test] +fn batch_install_rejects_declared_version_mismatch() { + let expected = astrid_capsule::capsule::CapsuleId::new("expected-capsule").unwrap(); + let err = validate_batch_install( + &expected, + "1.0.0", + super::super::capsule::install::BatchInstallOutcome { + installed: vec![super::super::capsule::install::InstalledCapsuleOutcome { + id: expected.clone(), + version: "2.0.0".to_string(), + wasm_hash: None, + }], + resolved_ref: None, + }, + ) + .unwrap_err(); + assert!(err.to_string().contains("installed manifest reports 2.0.0")); +} + #[test] fn provider_selection_parses_multi_select() { assert_eq!(parse_provider_selection("1,2", 3), vec![1, 2]); diff --git a/crates/astrid-cli/src/dispatch.rs b/crates/astrid-cli/src/dispatch.rs index c975e5818..49deb9728 100644 --- a/crates/astrid-cli/src/dispatch.rs +++ b/crates/astrid-cli/src/dispatch.rs @@ -172,6 +172,8 @@ async fn dispatch_subcommand( allow_unsigned, accept_new_key, vars, + target_principal, + grant_capsules, }) => { let distro = distro.ok_or_else(|| { anyhow::anyhow!( @@ -184,6 +186,11 @@ async fn dispatch_subcommand( allow_unsigned, accept_new_key, vars: commands::init::parse_cli_vars(&vars)?, + target_principal: target_principal + .map(astrid_core::PrincipalId::new) + .transpose()? + .unwrap_or_else(crate::principal::current), + grant_capsules, }; commands::init::run_init(&distro, &opts).await?; commands::self_update::ensure_path_setup()?; @@ -386,6 +393,10 @@ async fn dispatch_distro(command: DistroCommands) -> Result { allow_unsigned, accept_new_key, vars: commands::init::parse_cli_vars(&vars)?, + target_principal: crate::principal::current(), + // `distro apply` has no `--grant-capsules` surface; granting + // stays on `astrid init`. Capsules install without grants here. + grant_capsules: false, }; commands::init::run_init(&distro, &opts).await?; Ok(ExitCode::SUCCESS) @@ -552,6 +563,8 @@ mod tests { allow_unsigned: false, accept_new_key: false, vars: Vec::new(), + target_principal: None, + grant_capsules: false, }), OutputFormat::Pretty, ) diff --git a/crates/astrid-core/src/kernel_api/mod.rs b/crates/astrid-core/src/kernel_api/mod.rs index 3882d4a4f..d1c96720c 100644 --- a/crates/astrid-core/src/kernel_api/mod.rs +++ b/crates/astrid-core/src/kernel_api/mod.rs @@ -466,7 +466,9 @@ pub enum AdminRequestKind { /// loaded from `groups.toml` are both accepted as identifiers; /// validation that the named groups exist happens at the new /// profile's `validate` step. Mutations are idempotent — adding an - /// already-present group or removing an absent one is a no-op. + /// already-present group or removing an absent one is a no-op. An empty + /// delta performs the same authorized target-existence check without + /// rewriting the profile. AgentModify { /// Principal to modify. principal: PrincipalId, diff --git a/crates/astrid-integration-tests/tests/management_api_admin_topics.rs b/crates/astrid-integration-tests/tests/management_api_admin_topics.rs index 27bbbf086..7e67db173 100644 --- a/crates/astrid-integration-tests/tests/management_api_admin_topics.rs +++ b/crates/astrid-integration-tests/tests/management_api_admin_topics.rs @@ -82,6 +82,13 @@ fn all_admin_variants() -> Vec { AdminRequestKind::AgentDisable { principal: pid("target"), }, + AdminRequestKind::AgentModify { + principal: pid("target"), + add_groups: vec!["agent".into()], + remove_groups: Vec::new(), + add_capsules: Vec::new(), + remove_capsules: Vec::new(), + }, AdminRequestKind::AgentList, AdminRequestKind::QuotaSet { principal: pid("target"), diff --git a/crates/astrid-kernel/src/kernel_router/admin/handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/handlers.rs index 0182888ee..128d8a988 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/handlers.rs @@ -452,18 +452,6 @@ async fn agent_modify_from_req( "agent_modify_from_req received a non-AgentModify variant".to_string(), ); }; - if add_groups.is_empty() - && remove_groups.is_empty() - && add_capsules.is_empty() - && remove_capsules.is_empty() - { - return err_bad_input( - "agent.modify: at least one of `add_groups`, `remove_groups`, `add_capsules`, or \ - `remove_capsules` must be non-empty" - .to_string(), - ); - } - let _guard = kernel.admin_write_lock.lock().await; let path = principal_profile_path(kernel, &principal); if let Err(msg) = require_principal_exists(&principal, &path) { diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_modify.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_modify.rs index c74972168..2c94e2256 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_modify.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_modify.rs @@ -134,13 +134,13 @@ async fn agent_modify_adds_and_removes_groups_idempotently() { } #[tokio::test(flavor = "multi_thread")] -async fn agent_modify_rejects_empty_changes() { +async fn agent_modify_empty_delta_verifies_target_without_writing_profile() { let (_dir, kernel) = fixture().await; handlers::dispatch( &kernel, &astrid_core::PrincipalId::default(), AdminRequestKind::AgentCreate { - name: "nina".into(), + name: "preflight-target".into(), groups: Vec::new(), grants: Vec::new(), inherit_from: None, @@ -149,11 +149,33 @@ async fn agent_modify_rejects_empty_changes() { }, ) .await; - let res = handlers::dispatch( + let target = pid("preflight-target"); + let path = PrincipalProfile::path_for(&kernel.astrid_home, &target); + let before = std::fs::read(&path).unwrap(); + + let response = handlers::dispatch( &kernel, - &astrid_core::PrincipalId::default(), + &PrincipalId::default(), + AdminRequestKind::AgentModify { + principal: target, + add_groups: Vec::new(), + remove_groups: Vec::new(), + add_capsules: Vec::new(), + remove_capsules: Vec::new(), + }, + ) + .await; + let AdminResponseBody::Success(body) = response else { + panic!("expected success, got {response:?}"); + }; + assert_eq!(body["changed"], false); + assert_eq!(std::fs::read(&path).unwrap(), before); + + let missing = handlers::dispatch( + &kernel, + &PrincipalId::default(), AdminRequestKind::AgentModify { - principal: pid("nina"), + principal: pid("missing-target"), add_groups: Vec::new(), remove_groups: Vec::new(), add_capsules: Vec::new(), @@ -161,7 +183,7 @@ async fn agent_modify_rejects_empty_changes() { }, ) .await; - assert_error_contains(&res, "must be non-empty"); + assert_error_contains(&missing, "missing-target"); } #[tokio::test(flavor = "multi_thread")] diff --git a/crates/astrid-kernel/src/kernel_router/admin/tests.rs b/crates/astrid-kernel/src/kernel_router/admin/tests.rs index 9781cb34c..884a94188 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/tests.rs @@ -47,6 +47,13 @@ fn all_admin_variants() -> Vec { AdminRequestKind::AgentDisable { principal: pid("a"), }, + AdminRequestKind::AgentModify { + principal: pid("a"), + add_groups: vec!["agent".into()], + remove_groups: Vec::new(), + add_capsules: Vec::new(), + remove_capsules: Vec::new(), + }, AdminRequestKind::AgentList, AdminRequestKind::QuotaSet { principal: pid("a"), @@ -219,6 +226,28 @@ fn every_variant_has_a_method_label() { } } +#[test] +fn empty_agent_modify_keeps_existing_wire_and_authority_mapping() { + let target = pid("target"); + let req = AdminRequestKind::AgentModify { + principal: target.clone(), + add_groups: Vec::new(), + remove_groups: Vec::new(), + add_capsules: Vec::new(), + remove_capsules: Vec::new(), + }; + assert_eq!( + resolve_admin_scope(&req, &pid("operator")), + AuthorityScope::Global + ); + assert_eq!( + required_capability_for_admin_request(&req, AuthorityScope::Global), + "agent:modify" + ); + assert_eq!(admin_request_method(&req), "admin.agent.modify"); + assert_eq!(admin_target_principal(&req), Some(&target)); +} + #[test] fn resolve_admin_scope_self_when_target_is_caller() { let caller = pid("alice"); diff --git a/scripts/e2e/runtime-cli-smoke.sh b/scripts/e2e/runtime-cli-smoke.sh index ca3a4eb17..ed30d6784 100644 --- a/scripts/e2e/runtime-cli-smoke.sh +++ b/scripts/e2e/runtime-cli-smoke.sh @@ -246,8 +246,38 @@ PY run_cli_daemon_lifecycle_smoke } +capsule_archive_version() { + local archive=$1 + "$PYTHON" - "$archive" <<'PY' +import sys +import tarfile +import re + +with tarfile.open(sys.argv[1], "r:gz") as capsule: + manifest = capsule.extractfile("Capsule.toml") + if manifest is None: + raise SystemExit("capsule archive is missing Capsule.toml") + section = None + for raw_line in manifest.read().decode("utf-8").splitlines(): + line = raw_line.split("#", 1)[0].strip() + header = re.fullmatch(r"\[([^]]+)]", line) + if header: + section = header.group(1) + continue + if section == "package": + version = re.fullmatch(r'version\s*=\s*"([^"]+)"', line) + if version: + print(version.group(1)) + break + else: + raise SystemExit("capsule manifest is missing package.version") +PY +} + run_cli_offline_init_smoke() { local registry_archive=$1 + local registry_version + registry_version="$(capsule_archive_version "$registry_archive")" local home="$ARTIFACTS/cli-init-home" local cwd="$ARTIFACTS/cli-init-cwd" local distro="$ARTIFACTS/cli-init-distro.toml" @@ -263,7 +293,7 @@ version = "0.1.0" [[capsule]] name = "astrid-capsule-registry" source = "$registry_archive" -version = "0.8.0" +version = "$registry_version" role = "uplink" EOF run_isolated_cli "$home" "$cwd" init --distro "$distro" --offline --yes --allow-unsigned \ @@ -277,6 +307,8 @@ EOF run_cli_distro_seal_smoke() { local registry_archive=$1 + local registry_version + registry_version="$(capsule_archive_version "$registry_archive")" local distro_dir="$ARTIFACTS/cli-distro-seal" local key="$distro_dir/signing.key" mkdir -p "$distro_dir" @@ -291,7 +323,7 @@ version = "0.1.0" [[capsule]] name = "astrid-capsule-registry" source = "$registry_archive" -version = "0.8.0" +version = "$registry_version" role = "uplink" EOF "$PYTHON" - "$key" <<'PY'