diff --git a/CHANGELOG.md b/CHANGELOG.md index a15be36d5..9cc973d83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -120,6 +120,14 @@ Changelog tracking starts with 0.2.0. Prior versions were not tracked. ### Security +- **Bootstrap authority boundaries now fail closed.** Kernel management requests + require a valid principal instead of falling back to the `default` bootstrap + administrator, malformed connection lifecycle identities cannot move its + counter, bootstrap profile/key and legacy-profile migration failures abort + kernel construction, and agent modification cannot remove `default` from the + built-in `admin` group. The local CLI retains its intentional active-principal + default before requests reach the kernel. Closes #1256. + - **Device attenuation now applies to every kernel authority view.** Capsule, agent, and group inventory checks use the authenticating device scope, and a scoped pair request containing bare `*` requires effective pair-admin diff --git a/crates/astrid-kernel/src/kernel_router/admin/enforcement_tests.rs b/crates/astrid-kernel/src/kernel_router/admin/enforcement_tests.rs index 844eabb76..4024b80da 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/enforcement_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/enforcement_tests.rs @@ -51,6 +51,15 @@ async fn send_admin( caller: &PrincipalId, suffix: &str, req: AdminKernelRequest, +) -> serde_json::Value { + send_admin_with_raw_principal(kernel, Some(caller.as_str()), suffix, req).await +} + +async fn send_admin_with_raw_principal( + kernel: &Arc, + principal: Option<&str>, + suffix: &str, + req: AdminKernelRequest, ) -> serde_json::Value { let topic = Topic::admin_request(suffix); let response_topic = Topic::admin_response(suffix); @@ -58,7 +67,7 @@ async fn send_admin( let payload = serde_json::to_value(&req).expect("serialize admin request"); let mut msg = IpcMessage::new(topic, IpcPayload::RawJson(payload), kernel.session_id.0); - msg.principal = Some(caller.as_str().to_string()); + msg.principal = principal.map(str::to_string); let _ = kernel.event_bus.publish(astrid_events::AstridEvent::Ipc { metadata: astrid_events::EventMetadata::new("test"), message: msg, @@ -283,6 +292,63 @@ async fn admin_request_id_echoed_on_deny_path_too() { assert_eq!(resp["status"], "Error"); } +#[tokio::test(flavor = "multi_thread")] +async fn admin_router_denies_missing_and_invalid_principals_deterministically() { + let (_dir, kernel) = fixture().await; + + for (suffix, principal) in [ + ("caller.missing", None), + ("caller.invalid", Some("alice@evil.example")), + ] { + let request_id = format!("req-{suffix}"); + let response = send_admin_with_raw_principal( + &kernel, + principal, + suffix, + AdminKernelRequest::with_request_id(&request_id, AdminRequestKind::AgentList), + ) + .await; + assert_eq!(response["request_id"], request_id); + assert_eq!(response["status"], "Error"); + assert_eq!(response["data"], super::super::MANAGEMENT_CALLER_REQUIRED); + } + + let entries = kernel + .audit_log + .get_session_entries(&kernel.session_id) + .await + .expect("read audit chain"); + let denials = entries + .iter() + .filter(|entry| { + entry.principal.as_ref() == Some(&PrincipalId::anonymous()) + && matches!( + &entry.action, + AuditAction::AdminRequest { method, .. } + if method == "admin.agent.list" + ) + }) + .collect::>(); + assert_eq!( + denials.len(), + 2, + "missing and malformed callers must each produce one terminal audit row" + ); + for (entry, expected_reason) in denials + .iter() + .zip(["missing principal", "invalid principal"]) + { + let AuthorizationProof::Denied { reason } = &entry.authorization else { + panic!("caller-boundary denial must carry a denied authorization proof"); + }; + assert!(reason.contains(expected_reason), "got: {reason}"); + let AuditOutcome::Failure { error } = &entry.outcome else { + panic!("caller-boundary denial must carry a failure outcome"); + }; + assert!(error.contains(expected_reason), "got: {error}"); + } +} + // ── Per-device scope attenuation at the cap-gate ──────────────────── /// Like [`send_admin`] but stamps a host-derived `device_key_id` on the IPC diff --git a/crates/astrid-kernel/src/kernel_router/admin/handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/handlers.rs index c6ba338fb..ef9333b9d 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/handlers.rs @@ -495,6 +495,18 @@ async fn agent_modify_from_req( Ok(changed) => changed, Err(e) => return err_bad_input(format!("group delta rejected: {e}")), }; + if principal == PrincipalId::default() + && !profile + .groups + .iter() + .any(|group| group == astrid_core::groups::BUILTIN_ADMIN) + { + return err_bad_input( + "cannot remove the built-in `admin` group from the `default` principal — it is the \ + single-tenant bootstrap anchor" + .to_string(), + ); + } let capsules_changed = match apply_set_delta::( &mut profile.capsules, &add_capsules, diff --git a/crates/astrid-kernel/src/kernel_router/admin/mod.rs b/crates/astrid-kernel/src/kernel_router/admin/mod.rs index d2ce72f33..e1cd09c56 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/mod.rs @@ -62,6 +62,7 @@ use astrid_events::kernel_api::{ }; use tracing::warn; +use super::caller::{CallerResolutionError, MANAGEMENT_CALLER_REQUIRED}; use super::{ AdminAuditEntry, AuthorityScope, authorize_request, publish_response, record_admin_audit, resolve_caller, resolve_device_key_id, @@ -109,9 +110,33 @@ pub(crate) fn spawn_admin_router(kernel: Arc) -> astrid_runtime:: // pure-read endpoints. (For an HTTP front that // hosts thousands of agents the serial loop is // unworkable.) + let caller = match resolve_caller(message) { + Ok(caller) => caller, + Err(error) => { + warn!( + security_event = true, + topic = %message.topic, + reason = error.reason(), + "Rejected admin management request without a valid principal" + ); + let kernel = Arc::clone(&kernel); + let response_topic = admin_response_topic(&message.topic); + let device_key_id = resolve_device_key_id(message); + astrid_runtime::spawn(async move { + reject_admin_request_without_caller( + &kernel, + response_topic, + device_key_id, + req, + error, + ) + .await; + }); + continue; + }, + }; let kernel = Arc::clone(&kernel); let topic = message.topic.clone(); - let caller = resolve_caller(message); let device_key_id = resolve_device_key_id(message); astrid_runtime::spawn(async move { handle_admin_request(&kernel, topic, caller, device_key_id, req).await; @@ -129,6 +154,47 @@ pub(crate) fn spawn_admin_router(kernel: Arc) -> astrid_runtime:: }) } +/// Record and reject an admin request that crossed the IPC boundary without a +/// valid authenticated principal. The reserved `anonymous` identity preserves +/// the audit trail without granting the malformed envelope a caller identity. +async fn reject_admin_request_without_caller( + kernel: &Arc, + response_topic: Topic, + device_key_id: Option, + req: AdminKernelRequest, + error: CallerResolutionError, +) { + let caller = PrincipalId::anonymous(); + let method = admin_request_method(&req.kind); + let required_cap = + required_capability_for_admin_request(&req.kind, resolve_admin_scope(&req.kind, &caller)); + let reason = format!("{MANAGEMENT_CALLER_REQUIRED}: {}", error.reason()); + record_admin_audit( + kernel, + AdminAuditEntry { + caller: &caller, + method, + required_cap, + device_key_id: device_key_id.as_deref(), + target_principal: admin_target_principal(&req.kind).cloned(), + params: sanitize_admin_audit_params(&req.kind), + authorization: AuthorizationProof::Denied { + reason: reason.clone(), + }, + outcome: AuditOutcome::failure(reason), + }, + ) + .await; + publish_response( + kernel, + response_topic, + AdminKernelResponse::for_request( + req.request_id, + AdminResponseBody::Error(MANAGEMENT_CALLER_REQUIRED.to_string()), + ), + ); +} + /// Compute the response topic for an incoming admin request topic. fn admin_response_topic(input_topic: &str) -> Topic { input_topic 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 2c94e2256..a2a9c4ba2 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 @@ -9,7 +9,7 @@ use std::sync::Arc; use astrid_core::dirs::AstridHome; -use astrid_core::groups::{BUILTIN_AGENT, BUILTIN_RESTRICTED}; +use astrid_core::groups::{BUILTIN_ADMIN, BUILTIN_AGENT, BUILTIN_RESTRICTED}; use astrid_core::principal::PrincipalId; use astrid_core::profile::PrincipalProfile; use astrid_events::kernel_api::{AdminRequestKind, AdminResponseBody}; @@ -186,6 +186,59 @@ async fn agent_modify_empty_delta_verifies_target_without_writing_profile() { assert_error_contains(&missing, "missing-target"); } +#[tokio::test(flavor = "multi_thread")] +async fn agent_modify_preserves_default_admin_bootstrap_anchor() { + let (_dir, kernel) = fixture().await; + let default = PrincipalId::default(); + let path = PrincipalProfile::path_for(&kernel.astrid_home, &default); + PrincipalProfile { + groups: vec![BUILTIN_ADMIN.to_string()], + ..Default::default() + } + .save_to_path(&path) + .expect("seed default admin profile"); + + let removal = handlers::dispatch( + &kernel, + &default, + AdminRequestKind::AgentModify { + principal: default.clone(), + add_groups: Vec::new(), + remove_groups: vec![BUILTIN_ADMIN.to_string()], + add_capsules: Vec::new(), + remove_capsules: Vec::new(), + }, + ) + .await; + assert_error_contains(&removal, "bootstrap anchor"); + assert_eq!( + PrincipalProfile::load_from_path(&path) + .expect("reload default profile") + .groups, + vec![BUILTIN_ADMIN.to_string()] + ); + + let remove_and_add = handlers::dispatch( + &kernel, + &default, + AdminRequestKind::AgentModify { + principal: default.clone(), + add_groups: vec![BUILTIN_ADMIN.to_string()], + remove_groups: vec![BUILTIN_ADMIN.to_string()], + add_capsules: Vec::new(), + remove_capsules: Vec::new(), + }, + ) + .await; + assert_success(&remove_and_add); + assert_eq!( + PrincipalProfile::load_from_path(&path) + .expect("reload default profile") + .groups, + vec![BUILTIN_ADMIN.to_string()] + ); +} + #[tokio::test(flavor = "multi_thread")] async fn agent_modify_rejects_unknown_principal() { let (_dir, kernel) = fixture().await; diff --git a/crates/astrid-kernel/src/kernel_router/caller.rs b/crates/astrid-kernel/src/kernel_router/caller.rs new file mode 100644 index 000000000..bc56f1e1e --- /dev/null +++ b/crates/astrid-kernel/src/kernel_router/caller.rs @@ -0,0 +1,49 @@ +use astrid_core::principal::PrincipalId; +use astrid_events::ipc::IpcMessage; + +/// Stable outward denial for a management request with no authenticated caller. +pub(super) const MANAGEMENT_CALLER_REQUIRED: &str = + "management request denied: missing or invalid principal"; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum CallerResolutionError { + Missing, + Invalid, +} + +impl CallerResolutionError { + pub(super) const fn reason(self) -> &'static str { + match self { + Self::Missing => "missing principal", + Self::Invalid => "invalid principal", + } + } +} + +/// Resolve the authenticated caller from a management IPC envelope. +/// +/// The kernel never supplies the interactive CLI's active-principal default: +/// that deliberate UX choice is made by the local client before it publishes. +/// Once a request reaches this authority boundary, an absent or malformed +/// principal must not acquire the bootstrap `default` principal's authority. +pub(super) fn resolve_caller(message: &IpcMessage) -> Result { + let raw = message + .principal + .as_deref() + .ok_or(CallerResolutionError::Missing)?; + PrincipalId::new(raw).map_err(|_| CallerResolutionError::Invalid) +} + +/// Resolve one connection-tracking identity without granting bootstrap authority. +/// +/// A missing identity is the explicit no-capability `anonymous` principal used by +/// the legacy handshake. A malformed identity is rejected so a forged lifecycle +/// message cannot move any principal's counter. +pub(super) fn resolve_connection_principal( + message: &IpcMessage, +) -> Result { + match message.principal.as_deref() { + Some(raw) => PrincipalId::new(raw).map_err(|_| CallerResolutionError::Invalid), + None => Ok(PrincipalId::anonymous()), + } +} diff --git a/crates/astrid-kernel/src/kernel_router/connection_tracker_tests.rs b/crates/astrid-kernel/src/kernel_router/connection_tracker_tests.rs index f9c5ab526..d41afe385 100644 --- a/crates/astrid-kernel/src/kernel_router/connection_tracker_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/connection_tracker_tests.rs @@ -9,7 +9,9 @@ use astrid_events::AstridEvent; use astrid_events::EventMetadata; use astrid_events::ipc::{IpcMessage, IpcPayload, Topic}; -use super::{ConnectionSignal, connection_signal}; +use super::{ + CallerResolutionError, ConnectionSignal, connection_signal, resolve_connection_principal, +}; #[test] fn typed_connect_payload_opens() { @@ -96,6 +98,32 @@ fn typed_payload_wins_even_on_an_unrelated_topic() { ); } +#[test] +fn missing_connection_principal_maps_to_anonymous_not_default() { + let message = IpcMessage::new( + Topic::client_connect(), + IpcPayload::Connect, + uuid::Uuid::nil(), + ); + let principal = resolve_connection_principal(&message).expect("anonymous identity"); + assert_eq!(principal, astrid_core::PrincipalId::anonymous()); + assert_ne!(principal, astrid_core::PrincipalId::default()); +} + +#[test] +fn malformed_connection_principal_is_ignored_not_defaulted() { + let message = IpcMessage::new( + Topic::client_connect(), + IpcPayload::Connect, + uuid::Uuid::nil(), + ) + .with_principal("alice@evil.example"); + assert_eq!( + resolve_connection_principal(&message), + Err(CallerResolutionError::Invalid) + ); +} + // ── End-to-end counter balance through the live tracker ────────────────── // // The classifier tests above are pure. These drive the real @@ -199,6 +227,41 @@ async fn matched_cycles_balance_to_baseline_for_verified_principal() { ); } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn missing_principal_lifecycle_balances_under_anonymous() { + let dir = tempfile::tempdir().unwrap(); + let home = astrid_core::dirs::AstridHome::from_path(dir.path()); + let kernel = crate::test_kernel_with_home(home).await; + drop(super::spawn_connection_tracker(Arc::clone(&kernel))); + + for (topic, expected) in [ + (Topic::client_connect(), 1), + (Topic::client_disconnect(), 0), + ] { + let message = IpcMessage::new( + topic, + IpcPayload::RawJson(serde_json::json!({})), + uuid::Uuid::nil(), + ); + let _ = kernel.event_bus.publish(AstridEvent::Ipc { + metadata: EventMetadata::new("test").with_session_id(uuid::Uuid::nil()), + message, + }); + assert!( + wait_until(|| kernel.total_connection_count() == expected).await, + "anonymous lifecycle count should become {expected}" + ); + } + + assert!( + kernel + .connections_by_principal() + .iter() + .all(|(principal, _)| *principal != astrid_core::PrincipalId::default()), + "missing lifecycle identity must never touch the bootstrap principal" + ); +} + /// The historical leak, reproduced as a guard: a connect stamped with the real /// principal but a disconnect stamped `anonymous` (what the proxy produced /// post-close) does NOT balance — the real principal stays at 1 while diff --git a/crates/astrid-kernel/src/kernel_router/mod.rs b/crates/astrid-kernel/src/kernel_router/mod.rs index e36be35ee..2b6346698 100644 --- a/crates/astrid-kernel/src/kernel_router/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/mod.rs @@ -1,5 +1,6 @@ /// Admin management API dispatcher (issue #672, Layer 6). pub mod admin; +mod caller; mod device_scope; /// `KernelRequest::InstallCapsule` handler — delegates to the /// `astrid-capsule-install` library so the daemon and the CLI reach @@ -26,6 +27,9 @@ use astrid_events::ipc::{IpcMessage, IpcPayload, Topic}; use astrid_events::kernel_api::{KernelRequest, KernelResponse}; use tracing::{debug, info, warn}; +#[cfg(test)] +use caller::CallerResolutionError; +use caller::{MANAGEMENT_CALLER_REQUIRED, resolve_caller, resolve_connection_principal}; use device_scope::resolve_device_scope; #[cfg(test)] @@ -76,6 +80,23 @@ pub(crate) fn spawn_kernel_router(kernel: Arc) -> astrid_runtime: match serde_json::from_value::(val.clone()) { Ok(req) => { + let caller = match resolve_caller(message) { + Ok(caller) => caller, + Err(error) => { + warn!( + security_event = true, + topic = %message.topic, + reason = error.reason(), + "Rejected kernel management request without a valid principal" + ); + publish_response( + &kernel, + response_topic_for(&message.topic), + KernelResponse::Error(MANAGEMENT_CALLER_REQUIRED.to_string()), + ); + continue; + }, + }; let (method, limit) = rate_limit_for_request(&req); if let Some(max) = limit && !rate_limiter.check(method, max) @@ -95,7 +116,6 @@ pub(crate) fn spawn_kernel_router(kernel: Arc) -> astrid_runtime: ); continue; } - let caller = resolve_caller(message); let device_key_id = resolve_device_key_id(message); handle_request(&kernel, message.topic.clone(), caller, device_key_id, req) .await; @@ -179,26 +199,34 @@ fn spawn_connection_tracker(kernel: Arc) -> astrid_runtime::JoinH let astrid_events::AstridEvent::Ipc { message, .. } = &*event else { continue; }; - // Derive the connecting principal from the IPC message. Today's - // CLI socket always sets this to the default principal - // (bootstrapped in `bootstrap_cli_root_user`), but as per-agent - // socket auth lands (#658) the same plumbing will carry the - // real invoking principal. - let principal = message - .principal - .as_deref() - .and_then(|p| astrid_core::principal::PrincipalId::new(p).ok()) - .unwrap_or_default(); - match connection_signal(&message.topic, &message.payload) { - Some(ConnectionSignal::Closed { reason }) => { + let Some(signal) = connection_signal(&message.topic, &message.payload) else { + continue; + }; + // Lifecycle messages without a principal belong to the explicit + // no-authority identity. A malformed principal is not a lifecycle + // identity at all, so ignore it rather than crediting the bootstrap + // `default` principal with a connection it never authenticated. + let principal = match resolve_connection_principal(message) { + Ok(principal) => principal, + Err(error) => { + warn!( + security_event = true, + topic = %message.topic, + reason = error.reason(), + "Ignored connection lifecycle event with malformed principal" + ); + continue; + }, + }; + match signal { + ConnectionSignal::Closed { reason } => { kernel.connection_closed(&principal); debug!(%principal, topic = %message.topic, ?reason, "Client disconnected"); }, - Some(ConnectionSignal::Opened) => { + ConnectionSignal::Opened => { kernel.connection_opened(&principal); debug!(%principal, topic = %message.topic, "New client connection accepted"); }, - None => {}, } } }) @@ -748,20 +776,6 @@ pub fn kernel_request_method(req: &KernelRequest) -> &'static str { } } -/// Resolve the caller [`PrincipalId`] from an incoming [`IpcMessage`]. -/// -/// Pre-#658 single-token socket traffic arrives without a principal -/// field set; we fall back to [`PrincipalId::default`] — the default -/// principal is bootstrapped with the built-in `admin` group, matching -/// today's single-tenant behaviour. -fn resolve_caller(message: &IpcMessage) -> PrincipalId { - message - .principal - .as_deref() - .and_then(|p| PrincipalId::new(p).ok()) - .unwrap_or_default() -} - /// Resolve the authenticating device `key_id` from an incoming [`IpcMessage`]. /// /// Host-derived metadata stamped by the socket per-connection registry or the diff --git a/crates/astrid-kernel/src/kernel_router/tests.rs b/crates/astrid-kernel/src/kernel_router/tests.rs index 2d06e189a..0fefa90a1 100644 --- a/crates/astrid-kernel/src/kernel_router/tests.rs +++ b/crates/astrid-kernel/src/kernel_router/tests.rs @@ -457,32 +457,76 @@ fn resolve_caller_uses_ipc_principal_when_present() { uuid::Uuid::nil(), ); msg.principal = Some("alice".to_string()); - let caller = resolve_caller(&msg); + let caller = resolve_caller(&msg).expect("valid caller"); assert_eq!(caller.as_str(), "alice"); } #[test] -fn resolve_caller_falls_back_to_default_when_missing() { +fn resolve_caller_rejects_missing_principal() { let msg = IpcMessage::new( Topic::kernel_request("system"), IpcPayload::RawJson(serde_json::json!({})), uuid::Uuid::nil(), ); - let caller = resolve_caller(&msg); - assert_eq!(caller, PrincipalId::default()); + assert_eq!(resolve_caller(&msg), Err(CallerResolutionError::Missing)); } #[test] -fn resolve_caller_falls_back_to_default_on_invalid_principal() { +fn resolve_caller_rejects_invalid_principal() { let mut msg = IpcMessage::new( Topic::kernel_request("system"), IpcPayload::RawJson(serde_json::json!({})), uuid::Uuid::nil(), ); - // Invalid principal chars → PrincipalId::new fails → fall back. msg.principal = Some("alice@evil.example".to_string()); - let caller = resolve_caller(&msg); - assert_eq!(caller, PrincipalId::default()); + assert_eq!(resolve_caller(&msg), Err(CallerResolutionError::Invalid)); +} + +#[tokio::test(flavor = "multi_thread")] +async fn management_router_denies_missing_and_invalid_principals_deterministically() { + let dir = tempfile::tempdir().expect("tempdir"); + let home = astrid_core::dirs::AstridHome::from_path(dir.path()); + let kernel = crate::test_kernel_with_home(home).await; + drop(spawn_kernel_router(Arc::clone(&kernel))); + + for (suffix, principal) in [ + ("missing_caller", None), + ("invalid_caller", Some("alice@evil.example")), + ] { + let request_topic = Topic::kernel_request(suffix); + let response_topic = Topic::kernel_response(suffix); + let mut receiver = kernel.event_bus.subscribe_topic(response_topic.as_str()); + let payload = serde_json::to_value(KernelRequest::GetStatus).expect("serialize request"); + let mut message = IpcMessage::new( + request_topic, + IpcPayload::RawJson(payload), + kernel.session_id.0, + ); + message.principal = principal.map(str::to_string); + let _ = kernel.event_bus.publish(astrid_events::AstridEvent::Ipc { + metadata: astrid_events::EventMetadata::new("test"), + message, + }); + + let value = astrid_runtime::time::timeout(std::time::Duration::from_secs(2), async { + loop { + let event = receiver.recv().await.expect("response event"); + if let astrid_events::AstridEvent::Ipc { message, .. } = &*event + && let IpcPayload::RawJson(value) = &message.payload + { + return value.clone(); + } + } + }) + .await + .expect("management denial within 2s"); + let response: KernelResponse = serde_json::from_value(value).expect("typed response"); + assert!(matches!( + response, + KernelResponse::Error(ref reason) if reason == MANAGEMENT_CALLER_REQUIRED + )); + assert_eq!(kernel.total_connection_count(), 0); + } } // ── Agent-loop readiness dispatch (roundtrip) ──────────────────── diff --git a/crates/astrid-kernel/src/lib.rs b/crates/astrid-kernel/src/lib.rs index 6c5efd3cb..c678e5871 100644 --- a/crates/astrid-kernel/src/lib.rs +++ b/crates/astrid-kernel/src/lib.rs @@ -3058,9 +3058,11 @@ async fn bootstrap_cli_root_user( // Seed the default principal profile with the admin group. Runs // before the identity-link short-circuit below so a deleted profile // between boots is restored even when the identity record persists. - if let Err(e) = seed_default_principal_admin_profile(home) { - tracing::warn!(error = %e, "Failed to seed default admin profile — continuing boot"); - } + seed_default_principal_admin_profile(home).map_err(|error| { + astrid_storage::IdentityError::Storage(format!( + "default admin profile bootstrap failed: {error}" + )) + })?; // Check if root user already exists by trying to resolve the CLI link. if let Some(_user) = store.resolve("cli", "local").await? { @@ -3106,7 +3108,7 @@ fn migrate_legacy_profile_path( // Operator already migrated, or a prior boot did the rename. // Drop the stale legacy file so capsules can no longer reach // it via `home://.config/profile.toml`. - let _ = std::fs::remove_file(&legacy_path); + remove_legacy_profile_file(&legacy_path)?; return Ok(()); } if let Some(parent) = new_path.parent() { @@ -3123,6 +3125,14 @@ fn migrate_legacy_profile_path( Ok(()) } +fn remove_legacy_profile_file(path: &Path) -> Result<(), std::io::Error> { + match std::fs::remove_file(path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(error), + } +} + /// Idempotently ensure the default principal's profile on disk has the /// built-in `admin` group, so the single-tenant CLI path carries full /// management-API capabilities (issue #670). @@ -3148,9 +3158,7 @@ fn seed_default_principal_admin_profile( // Move any legacy file in front of load — load_from_path on the new // path would otherwise return Default and clobber the operator's // existing groups/grants/revokes. - if let Err(e) = migrate_legacy_profile_path(home, &default_principal) { - tracing::warn!(error = %e, "Failed to migrate legacy profile path — continuing"); - } + migrate_legacy_profile_path(home, &default_principal)?; let path = PrincipalProfile::path_for(home, &default_principal); let mut profile = PrincipalProfile::load_from_path(&path)?; @@ -4169,6 +4177,93 @@ mod tests { (dir, home) } + fn injected_kernel_resources(home: &astrid_core::dirs::AstridHome) -> KernelResources { + home.ensure().expect("ensure test home"); + let kv: Arc = Arc::new(astrid_storage::MemoryKvStore::new()); + let runtime_key = Arc::new(astrid_crypto::KeyPair::generate()); + let principal_home = home.principal_home(&astrid_core::PrincipalId::default()); + principal_home.ensure().expect("ensure default home"); + let audit_log = Arc::new( + AuditLog::open(principal_home.audit_dir(), Arc::clone(&runtime_key)) + .expect("open test audit log"), + ); + KernelResources::new( + home.clone(), + kv, + audit_log, + runtime_key, + Arc::new(astrid_core::session_token::SessionToken::generate()), + home.token_path(), + None, + None, + ) + } + + async fn boot_with_injected_resources( + home: &astrid_core::dirs::AstridHome, + resources: KernelResources, + ) -> std::io::Result> { + Kernel::with_resources( + SessionId::SYSTEM, + home.root().to_path_buf(), + astrid_capsule_types::CapsuleRuntimeLimits::default(), + std::collections::HashMap::new(), + astrid_capsule_types::HttpLimits::default(), + resources, + ) + .await + } + + fn assert_bootstrap_error(error: &std::io::Error) { + let message = error.to_string(); + assert!( + message.contains("Failed to bootstrap CLI root user") + && message.contains("default admin profile bootstrap failed"), + "unexpected boot error: {message}" + ); + } + + #[tokio::test(flavor = "multi_thread")] + async fn with_resources_aborts_when_legacy_profile_migration_fails() { + let (_dir, home) = scratch_home(); + let resources = injected_kernel_resources(&home); + let default = astrid_core::PrincipalId::default(); + let legacy_path = home + .principal_home(&default) + .config_dir() + .join("profile.toml"); + astrid_core::PrincipalProfile { + groups: vec![astrid_core::groups::BUILTIN_ADMIN.to_string()], + ..Default::default() + } + .save_to_path(&legacy_path) + .expect("seed legacy profile"); + std::fs::write(home.profiles_dir(), b"blocks profile directory") + .expect("create deterministic migration obstacle"); + + let Err(error) = boot_with_injected_resources(&home, resources).await else { + panic!("kernel boot must fail when policy migration fails"); + }; + assert_bootstrap_error(&error); + assert!( + legacy_path.exists(), + "failed migration must preserve source policy" + ); + } + + #[tokio::test(flavor = "multi_thread")] + async fn with_resources_aborts_when_default_key_seeding_fails() { + let (_dir, home) = scratch_home(); + let resources = injected_kernel_resources(&home); + std::fs::create_dir(home.keys_dir().join("default.key")) + .expect("create deterministic key-write obstacle"); + + let Err(error) = boot_with_injected_resources(&home, resources).await else { + panic!("kernel boot must fail when bootstrap key seeding fails"); + }; + assert_bootstrap_error(&error); + } + #[test] fn seed_admin_writes_fresh_profile_when_missing() { let (_d, home) = scratch_home(); @@ -4389,4 +4484,10 @@ mod tests { let result = astrid_core::PrincipalProfile::load_from_path(&new_path).unwrap(); assert_eq!(result.groups, vec!["canonical".to_string()]); } + + #[test] + fn missing_legacy_profile_cleanup_is_idempotent() { + let dir = tempfile::tempdir().unwrap(); + remove_legacy_profile_file(&dir.path().join("already-removed.toml")).unwrap(); + } }