From 4c2a02a717e486e565b954279eee3a1e0e57d201 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 12 Aug 2026 19:05:41 +0400 Subject: [PATCH 1/5] fix(agent): fully retire deleted principals Signed-off-by: Joshua J. Bouw --- CHANGELOG.md | 3 + crates/astrid-cli/src/commands/agent/mod.rs | 17 +- crates/astrid-core/src/kernel_api/mod.rs | 8 +- .../src/kernel_router/admin/agent_delete.rs | 260 ++++++++++++++++++ .../src/kernel_router/admin/handlers.rs | 126 +-------- .../src/kernel_router/admin/mod.rs | 3 + .../admin/state_tests_agent_delete.rs | 142 ++++++++++ crates/astrid-kernel/src/lib.rs | 129 ++++++++- 8 files changed, 555 insertions(+), 133 deletions(-) create mode 100644 crates/astrid-kernel/src/kernel_router/admin/agent_delete.rs create mode 100644 crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_delete.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index cacae0d7e..2183ac3c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ Changelog tracking starts with 0.2.0. Prior versions were not tracked. dependencies in `Cargo.lock`. Removing the unused surface eliminates both RustSec exceptions and keeps the audit gate strict. The native principal store and live legacy-SurrealKV migration reader are unchanged. Closes #1448. +### Changed + +- **`agent delete` now reclaims the deleted principal's full runtime footprint.** Delete closes authz first (unlink + profile removal + cache invalidate), then retires its live capsule views and reclaims capsule KV namespaces, the home tree, signing key (`keys/{principal}.key`), and secrets (`secrets/{principal}/`). Shared runtimes remain available to other principals while principal-scoped work is cancelled before state removal. Reclamation is best-effort; failures are reported in the response's `cleanup_errors`. Replaces the previous "reclamation is an ops concern" leave-behind, and drops the interim `--purge-home` flag. Part of #1217. ### Added diff --git a/crates/astrid-cli/src/commands/agent/mod.rs b/crates/astrid-cli/src/commands/agent/mod.rs index 81903d1af..88923f9ee 100644 --- a/crates/astrid-cli/src/commands/agent/mod.rs +++ b/crates/astrid-cli/src/commands/agent/mod.rs @@ -660,7 +660,10 @@ fn print_agent_detail(agent: &AgentSummary) { async fn run_delete(args: DeleteArgs) -> Result { let principal = PrincipalId::new(&args.name).context("invalid agent name")?; if !args.yes { - eprint!("Delete agent '{principal}' (home directory is NOT removed) [y/N]? "); + eprint!( + "Delete agent '{principal}' \ + (home directory, signing key, and secrets are reclaimed) [y/N]? " + ); std::io::Write::flush(&mut std::io::stderr()).ok(); let mut buf = String::new(); std::io::stdin().read_line(&mut buf).ok(); @@ -673,7 +676,17 @@ async fn run_delete(args: DeleteArgs) -> Result { let body = client .request(AdminRequestKind::AgentDelete { principal }) .await?; - let _ = into_result(body)?; + let outcome = into_result(body)?; + // Surface any footprint-reclamation failures the kernel reported + // (delete closes authz regardless; leftovers are an ops follow-up). + if let AdminResponseBody::Success(v) = &outcome + && let Some(errs) = v.get("cleanup_errors").and_then(|e| e.as_array()) + && !errs.is_empty() + { + for e in errs.iter().filter_map(|e| e.as_str()) { + eprintln!("warning: footprint cleanup: {e}"); + } + } println!( "{}", Theme::success(&format!("Deleted agent '{}'", args.name)) diff --git a/crates/astrid-core/src/kernel_api/mod.rs b/crates/astrid-core/src/kernel_api/mod.rs index 8628804ff..73bc1a8da 100644 --- a/crates/astrid-core/src/kernel_api/mod.rs +++ b/crates/astrid-core/src/kernel_api/mod.rs @@ -448,8 +448,12 @@ pub enum AdminRequestKind { allow_admin_clone: bool, }, /// Delete an existing agent identity. The `default` principal is - /// rejected unconditionally. The principal's home directory is NOT - /// scrubbed — reclamation is an ops concern. + /// rejected unconditionally. Delete closes authz first (unlink + + /// profile removal + cache invalidate), then reclaims the principal's + /// on-disk footprint — home tree (`home/{principal}/`), signing key + /// (`keys/{principal}.key`), and secrets (`secrets/{principal}/`). + /// Reclamation is best-effort; any failures are reported in the + /// response's `cleanup_errors` (#1217). AgentDelete { /// Principal to delete. principal: PrincipalId, diff --git a/crates/astrid-kernel/src/kernel_router/admin/agent_delete.rs b/crates/astrid-kernel/src/kernel_router/admin/agent_delete.rs new file mode 100644 index 000000000..0f4141cc5 --- /dev/null +++ b/crates/astrid-kernel/src/kernel_router/admin/agent_delete.rs @@ -0,0 +1,260 @@ +//! Principal deletion: close authority, retire live capsule views, reclaim state. + +use std::collections::BTreeSet; +use std::path::Path; +use std::sync::Arc; + +use astrid_core::principal::PrincipalId; +use astrid_events::kernel_api::AdminResponseBody; +use tracing::{info, warn}; + +use super::handlers::{ + AGENT_IDENTITY_PLATFORM, err_bad_input, err_internal, principal_profile_path, success_json, +}; + +pub(super) async fn agent_delete( + kernel: &Arc, + principal: PrincipalId, +) -> AdminResponseBody { + if principal == PrincipalId::default() { + return err_bad_input( + "cannot delete the `default` principal — it is the single-tenant bootstrap anchor" + .to_string(), + ); + } + + let _guard = kernel.admin_write_lock.lock().await; + let pending = match prepare_identity_removal(kernel, &principal).await { + Ok(pending) => pending, + Err(response) => return response, + }; + + if let Err(e) = kernel + .identity_store + .unlink(AGENT_IDENTITY_PLATFORM, principal.as_str()) + .await + { + return err_internal(format!("identity store unlink failed: {e}")); + } + + let path = principal_profile_path(kernel, &principal); + if let Err(e) = std::fs::remove_file(&path) + && e.kind() != std::io::ErrorKind::NotFound + { + return err_internal(format!( + "failed to remove profile.toml at {}: {e}", + path.display() + )); + } + kernel.profile_cache.invalidate(&principal); + + let (unloaded_capsules, reclaimed, cleanup_errors) = + match retire_and_reclaim(kernel, &principal).await { + Ok(result) => result, + Err(e) => return err_internal(e), + }; + + if let Err(response) = finish_identity_removal(kernel, pending).await { + return response; + } + + info!(%principal, ?unloaded_capsules, ?reclaimed, ?cleanup_errors, "Layer 6 agent.delete"); + success_json(serde_json::json!({ + "principal": principal.as_str(), + "unloaded_capsules": unloaded_capsules, + "reclaimed": reclaimed, + "cleanup_errors": cleanup_errors, + })) +} + +struct PendingIdentityRemoval { + user: Option, + ownership_guard: Option, +} + +async fn prepare_identity_removal( + kernel: &Arc, + principal: &PrincipalId, +) -> Result { + let linked = kernel + .identity_store + .resolve(AGENT_IDENTITY_PLATFORM, principal.as_str()) + .await + .map_err(|e| err_internal(format!("identity store resolve failed: {e}")))?; + let resolved = if linked.is_some() { + linked + } else { + kernel + .identity_store + .list_users() + .await + .map_err(|e| err_internal(format!("identity store list_users failed: {e}")))? + .into_iter() + .find(|user| user.principal == *principal) + }; + + let ownership_guard = if let Some(user) = resolved.as_ref() { + let identity = kernel + .identity_store + .get_principal_identity(user.id) + .await + .map_err(|e| { + err_internal(format!( + "identity store principal identity lookup failed: {e}" + )) + })?; + if let Some(identity) = identity { + match kernel + .ownership_store + .guard_principal_deletion_for_alias(identity.uid, principal.clone()) + .await + { + Ok(guard) => Some(guard), + Err(astrid_storage::OwnershipError::PrincipalAlreadyOwned { fleet, .. }) => { + return Err(err_bad_input(format!( + "cannot delete principal `{principal}` while it is assigned to fleet {fleet}" + ))); + }, + Err(e) => { + return Err(err_internal(format!( + "ownership store deletion guard failed: {e}" + ))); + }, + } + } else { + None + } + } else { + kernel + .ownership_store + .finish_principal_deletion_by_alias(principal) + .await + .map_err(|e| err_internal(format!("ownership store deletion recovery failed: {e}")))?; + None + }; + + Ok(PendingIdentityRemoval { + user: resolved, + ownership_guard, + }) +} + +async fn finish_identity_removal( + kernel: &Arc, + pending: PendingIdentityRemoval, +) -> Result<(), AdminResponseBody> { + if let Some(user) = pending.user { + match kernel.identity_store.delete_user(user.id).await { + Ok(true) => {}, + Ok(false) => { + return Err(err_internal( + "identity store user disappeared during principal deletion".to_string(), + )); + }, + Err(e) => { + return Err(err_internal(format!( + "identity store delete_user failed: {e}" + ))); + }, + } + } + if let Some(guard) = pending.ownership_guard { + guard.finish().await.map_err(|e| { + err_internal(format!( + "ownership store deletion reservation cleanup failed: {e}" + )) + })?; + } + Ok(()) +} + +type ReclaimOutcome = ( + Vec, + Vec<&'static str>, + Vec, +); + +async fn retire_and_reclaim( + kernel: &Arc, + principal: &PrincipalId, +) -> Result { + let unloaded = kernel + .unload_principal_capsules(principal) + .await + .map_err(|e| format!("failed to retire capsule views for `{principal}`: {e}"))?; + + // KV lives in the kernel store rather than below the principal home, so + // reclaim each capsule namespace explicitly before deleting the install + // tree. The live view covers active capsules; the on-disk set also covers + // installed capsules that failed to load. + let capsule_dir = kernel.astrid_home.principal_home(principal).capsules_dir(); + let mut capsule_ids: BTreeSet = unloaded.iter().map(ToString::to_string).collect(); + if let Ok(entries) = std::fs::read_dir(&capsule_dir) { + capsule_ids.extend(entries.flatten().filter_map(|entry| { + entry + .file_type() + .ok() + .filter(std::fs::FileType::is_dir) + .and_then(|_| entry.file_name().into_string().ok()) + })); + } + let mut kv_errors = Vec::new(); + for capsule in capsule_ids { + let namespace = format!("{principal}:capsule:{capsule}"); + if let Err(error) = kernel.kv.clear_namespace(&namespace).await { + kv_errors.push(format!("kv namespace {namespace}: {error}")); + } + } + + let home = kernel + .astrid_home + .principal_home(principal) + .root() + .to_path_buf(); + let key = kernel + .astrid_home + .keys_dir() + .join(format!("{principal}.key")); + let secrets = kernel.astrid_home.secrets_dir().join(principal.as_str()); + let outcomes = tokio::task::spawn_blocking(move || { + [ + ("home", reclaim_dir_all(&home)), + ("keys", reclaim_file(&key)), + ("secrets", reclaim_dir_all(&secrets)), + ] + }) + .await + .map_err(|e| format!("agent footprint reclamation task failed: {e}"))?; + + let mut reclaimed = Vec::new(); + let mut cleanup_errors = kv_errors; + if cleanup_errors.is_empty() { + reclaimed.push("kv"); + } + for (what, outcome) in outcomes { + match outcome { + Ok(()) => reclaimed.push(what), + Err(msg) => { + warn!(%principal, %what, error = %msg, "agent.delete: footprint reclamation failed"); + cleanup_errors.push(format!("{what}: {msg}")); + }, + } + } + Ok((unloaded, reclaimed, cleanup_errors)) +} + +fn reclaim_dir_all(path: &Path) -> Result<(), String> { + match std::fs::remove_dir_all(path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(format!("{}: {e}", path.display())), + } +} + +fn reclaim_file(path: &Path) -> Result<(), String> { + match std::fs::remove_file(path) { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(format!("{}: {e}", path.display())), + } +} diff --git a/crates/astrid-kernel/src/kernel_router/admin/handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/handlers.rs index ec471285c..85a94f2a7 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/handlers.rs @@ -100,7 +100,9 @@ async fn dispatch_inner( ) -> AdminResponseBody { match req { req @ AdminRequestKind::AgentCreate { .. } => agent_create_from_req(kernel, req).await, - AdminRequestKind::AgentDelete { principal } => agent_delete(kernel, principal).await, + AdminRequestKind::AgentDelete { principal } => { + super::agent_delete::agent_delete(kernel, principal).await + }, AdminRequestKind::AgentEnable { principal } => { agent_set_enabled(kernel, principal, true).await }, @@ -353,128 +355,6 @@ async fn agent_create( .await } -async fn agent_delete(kernel: &Arc, principal: PrincipalId) -> AdminResponseBody { - if principal == PrincipalId::default() { - return err_bad_input( - "cannot delete the `default` principal — it is the single-tenant bootstrap anchor" - .to_string(), - ); - } - - let _guard = kernel.admin_write_lock.lock().await; - - // Prefer the frontend link, then fall back to the durable user record. A - // prior partial deletion may already have removed the link while leaving - // the principal identity and directory admission intact. - let linked = match kernel - .identity_store - .resolve(AGENT_IDENTITY_PLATFORM, principal.as_str()) - .await - { - Ok(user) => user, - Err(e) => return err_internal(format!("identity store resolve failed: {e}")), - }; - let resolved = if linked.is_some() { - linked - } else { - match kernel.identity_store.list_users().await { - Ok(users) => users.into_iter().find(|user| user.principal == principal), - Err(e) => return err_internal(format!("identity store list_users failed: {e}")), - } - }; - let ownership_guard = if let Some(user) = resolved.as_ref() { - let identity = match kernel.identity_store.get_principal_identity(user.id).await { - Ok(identity) => identity, - Err(e) => { - return err_internal(format!( - "identity store principal identity lookup failed: {e}" - )); - }, - }; - if let Some(identity) = identity { - match kernel - .ownership_store - .guard_principal_deletion_for_alias(identity.uid, principal.clone()) - .await - { - Ok(guard) => Some(guard), - Err(astrid_storage::OwnershipError::PrincipalAlreadyOwned { fleet, .. }) => { - return err_bad_input(format!( - "cannot delete principal `{principal}` while it is assigned to fleet {fleet}" - )); - }, - Err(e) => { - return err_internal(format!("ownership store deletion guard failed: {e}")); - }, - } - } else { - None - } - } else { - if let Err(e) = kernel - .ownership_store - .finish_principal_deletion_by_alias(&principal) - .await - { - return err_internal(format!("ownership store deletion recovery failed: {e}")); - } - None - }; - // Unlink before delete_user so a concurrent `resolve` can't return - // a dangling user id in the narrow window between the two calls. - if let Err(e) = kernel - .identity_store - .unlink(AGENT_IDENTITY_PLATFORM, principal.as_str()) - .await - { - return err_internal(format!("identity store unlink failed: {e}")); - } - if let Some(user) = resolved { - match kernel.identity_store.delete_user(user.id).await { - Ok(true) => {}, - Ok(false) => { - return err_internal( - "identity store user disappeared during principal deletion".to_string(), - ); - }, - Err(e) => return err_internal(format!("identity store delete_user failed: {e}")), - } - } - if let Some(guard) = ownership_guard - && let Err(e) = guard.finish().await - { - return err_internal(format!( - "ownership store deletion reservation cleanup failed: {e}" - )); - } - - // Remove the policy file. Without this, traffic claiming this - // principal would re-load the old profile from disk via the - // cache and continue to satisfy authz checks against the old - // grants/groups. The home directory itself (capsule data, KV - // namespace, audit chain) is NOT scrubbed — reclamation is an - // ops concern. Best-effort delete: if the file is already gone - // (concurrent admin or never existed) we proceed. - let path = principal_profile_path(kernel, &principal); - if let Err(e) = std::fs::remove_file(&path) - && e.kind() != std::io::ErrorKind::NotFound - { - return err_internal(format!( - "failed to remove profile.toml at {}: {e}", - path.display() - )); - } - - // Invalidate cache so subsequent authz checks for this principal - // re-resolve from disk and observe the deletion (next resolve - // returns Default, which under the Layer 5 enforcement preamble - // grants no capabilities). - kernel.profile_cache.invalidate(&principal); - - info!(%principal, "Layer 6 agent.delete"); - success_json(serde_json::json!({ "principal": principal.as_str() })) -} - async fn agent_set_enabled( kernel: &Arc, principal: PrincipalId, diff --git a/crates/astrid-kernel/src/kernel_router/admin/mod.rs b/crates/astrid-kernel/src/kernel_router/admin/mod.rs index 4a6c7bc5e..a83b2a818 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/mod.rs @@ -22,6 +22,7 @@ //! `profile.toml` snapshots. mod agent_create_helpers; +mod agent_delete; mod caps_tokens; #[cfg(test)] mod enforcement_tests; @@ -40,6 +41,8 @@ mod state_tests_agent_backfill; #[cfg(test)] mod state_tests_agent_clone; #[cfg(test)] +mod state_tests_agent_delete; +#[cfg(test)] mod state_tests_agent_modify; #[cfg(test)] mod state_tests_caps; diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_delete.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_delete.rs new file mode 100644 index 000000000..88da6ddda --- /dev/null +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_delete.rs @@ -0,0 +1,142 @@ +//! Principal deletion state-reclamation regression tests (#1217). + +use std::path::PathBuf; +use std::sync::Arc; + +use astrid_core::dirs::AstridHome; +use astrid_core::groups::{BUILTIN_ADMIN, BUILTIN_AGENT}; +use astrid_core::principal::PrincipalId; +use astrid_core::profile::PrincipalProfile; +use astrid_events::kernel_api::{AdminRequestKind, AdminResponseBody}; + +use super::handlers; +use crate::Kernel; + +async fn fixture() -> (tempfile::TempDir, Arc) { + let dir = tempfile::tempdir().expect("tempdir"); + let kernel = crate::test_kernel_with_home(AstridHome::from_path(dir.path())).await; + let admin = PrincipalProfile { + groups: vec![BUILTIN_ADMIN.to_string()], + ..Default::default() + }; + admin + .save_to_path(&PrincipalProfile::path_for( + &kernel.astrid_home, + &PrincipalId::default(), + )) + .expect("seed default admin profile"); + kernel.profile_cache.invalidate(&PrincipalId::default()); + (dir, kernel) +} + +fn seed_footprint(kernel: &Kernel, principal: &PrincipalId) -> (PathBuf, PathBuf, PathBuf) { + let home = kernel + .astrid_home + .principal_home(principal) + .root() + .to_path_buf(); + let key = kernel + .astrid_home + .keys_dir() + .join(format!("{principal}.key")); + let secrets = kernel.astrid_home.secrets_dir().join(principal.as_str()); + std::fs::create_dir_all(home.join(".local/kv")).unwrap(); + std::fs::write(home.join(".local/kv/state.db"), b"kv").unwrap(); + std::fs::create_dir_all(key.parent().unwrap()).unwrap(); + std::fs::write(&key, b"signing-key").unwrap(); + std::fs::create_dir_all(&secrets).unwrap(); + std::fs::write(secrets.join("api_key"), b"secret").unwrap(); + (home, key, secrets) +} + +async fn create(kernel: &Arc, principal: &PrincipalId) { + let response = handlers::dispatch( + kernel, + &PrincipalId::default(), + AdminRequestKind::AgentCreate { + name: principal.to_string(), + groups: vec![BUILTIN_AGENT.to_string()], + grants: Vec::new(), + inherit_from: None, + clone_from: None, + allow_admin_clone: false, + }, + ) + .await; + assert!(matches!(response, AdminResponseBody::Success(_))); +} + +#[tokio::test(flavor = "multi_thread")] +async fn agent_delete_reclaims_home_key_and_secrets_and_reports_them() { + let (_dir, kernel) = fixture().await; + let principal = PrincipalId::new("ghost").unwrap(); + create(&kernel, &principal).await; + let (home, key, secrets) = seed_footprint(&kernel, &principal); + std::fs::create_dir_all( + kernel + .astrid_home + .principal_home(&principal) + .capsules_dir() + .join("session"), + ) + .unwrap(); + kernel + .kv + .set("ghost:capsule:session", "history", b"private".to_vec()) + .await + .unwrap(); + + let response = handlers::dispatch( + &kernel, + &PrincipalId::default(), + AdminRequestKind::AgentDelete { + principal: principal.clone(), + }, + ) + .await; + + assert!(!home.exists() && !key.exists() && !secrets.exists()); + assert!( + kernel + .kv + .get("ghost:capsule:session", "history") + .await + .is_err(), + "the deleted durable identity must no longer resolve a KV namespace" + ); + let AdminResponseBody::Success(value) = response else { + panic!("expected Success response"); + }; + assert_eq!( + value["reclaimed"], + serde_json::json!(["kv", "home", "keys", "secrets"]) + ); + assert_eq!(value["unloaded_capsules"], serde_json::json!([])); + assert_eq!(value["cleanup_errors"], serde_json::json!([])); +} + +#[tokio::test(flavor = "multi_thread")] +async fn agent_delete_closes_authz_before_reclaiming() { + let (_dir, kernel) = fixture().await; + let principal = PrincipalId::new("active").unwrap(); + create(&kernel, &principal).await; + let (home, key, secrets) = seed_footprint(&kernel, &principal); + assert_eq!( + kernel.profile_cache.resolve(&principal).unwrap().groups, + vec![BUILTIN_AGENT.to_string()] + ); + + let response = handlers::dispatch( + &kernel, + &PrincipalId::default(), + AdminRequestKind::AgentDelete { + principal: principal.clone(), + }, + ) + .await; + assert!(matches!(response, AdminResponseBody::Success(_))); + + let after = kernel.profile_cache.resolve(&principal).unwrap(); + assert!(after.groups.is_empty() && after.grants.is_empty()); + assert!(!home.exists() && !key.exists() && !secrets.exists()); +} diff --git a/crates/astrid-kernel/src/lib.rs b/crates/astrid-kernel/src/lib.rs index 7b8f2ae77..ebc7e1f73 100644 --- a/crates/astrid-kernel/src/lib.rs +++ b/crates/astrid-kernel/src/lib.rs @@ -1493,6 +1493,16 @@ impl Kernel { #[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))] pub async fn ensure_principal_loaded(&self, principal: &PrincipalId) { let _load_guard = self.capsule_load_lock.lock().await; + // A deleted principal has no policy profile. Re-check under the same + // lock used by deletion/unload so a loader queued before the authz + // fence cannot re-attach capsule views after deletion has retired them. + if *principal != PrincipalId::default() + && !astrid_core::profile::PrincipalProfile::path_for(&self.astrid_home, principal) + .exists() + { + tracing::debug!(%principal, "Skipping capsule load for principal without a profile"); + return; + } let sorted = self.sorted_principal_capsules(principal); validate_principal_capsules(principal, &sorted); @@ -1533,6 +1543,13 @@ impl Kernel { #[cfg(not(all(target_arch = "wasm32", target_os = "unknown")))] async fn ensure_principal_uplinks_loaded(&self, principal: &PrincipalId) { let _load_guard = self.capsule_load_lock.lock().await; + if *principal != PrincipalId::default() + && !astrid_core::profile::PrincipalProfile::path_for(&self.astrid_home, principal) + .exists() + { + tracing::debug!(%principal, "Skipping uplink load for principal without a profile"); + return; + } let sorted = self.sorted_principal_capsules(principal); validate_principal_capsules(principal, &sorted); @@ -2148,6 +2165,35 @@ impl Kernel { Ok(true) } + /// Remove every capsule view owned by `principal` before that principal's + /// persistent state is reclaimed. + /// + /// The load lock closes the race with background warm/install discovery: + /// once the profile/identity fence has closed new authorization, no loader + /// can re-attach a view between the snapshot and the last unload. Each + /// release uses [`Self::unload_one_capsule`], preserving shared-runtime + /// semantics: the last view cancels and unloads the whole runtime, while a + /// non-last view cancels only this principal's in-flight blocking work. + pub(crate) async fn unload_principal_capsules( + &self, + principal: &PrincipalId, + ) -> Result, anyhow::Error> { + let _load_guard = self.capsule_load_lock.lock().await; + let mut ids: Vec<_> = { + let registry = self.capsules.read().await; + registry.list_for(principal).into_iter().cloned().collect() + }; + ids.sort_by(|a, b| a.as_str().cmp(b.as_str())); + + let mut unloaded = Vec::with_capacity(ids.len()); + for id in ids { + if self.unload_one_capsule(&id, principal).await? { + unloaded.push(id); + } + } + Ok(unloaded) + } + /// Promote (`commit == true`) or roll back (`commit == false`) a capsule's /// OS-level copy-on-write workspace changes — the gate's approve/reject for /// a non-git workspace (Fix #2). @@ -2557,24 +2603,29 @@ async fn unload_loaded_capsule_after_source_disappeared( #[cfg(test)] async fn open_test_runtime_kv( home: &astrid_core::dirs::AstridHome, -) -> Arc { +) -> ( + Arc, + astrid_storage::PrincipalDirectory, +) { let quota: Arc> = Arc::new(|_: &astrid_storage::StateOwner| Ok(None)); - astrid_storage::open_runtime_kv(home, quota) + let directory = astrid_storage::PrincipalDirectory::default(); + let kv = astrid_storage::open_runtime_kv_with_directory(home, quota, directory.clone()) .await - .expect("test kernel: open authoritative principal store") + .expect("test kernel: open authoritative principal store"); + (kv, directory) } #[cfg(test)] fn open_test_identity_stores( kv: &Arc, + principal_directory: astrid_storage::PrincipalDirectory, ) -> ( Arc, Arc, ) { let identity_kv = astrid_storage::ScopedKvStore::new(Arc::clone(kv), "system:identity") .expect("test kernel: identity kv scope"); - let principal_directory = astrid_storage::PrincipalDirectory::default(); let identity_store = Arc::new(astrid_storage::KvIdentityStore::with_principal_directory( identity_kv, principal_directory.clone(), @@ -2600,7 +2651,7 @@ pub(crate) async fn test_kernel_with_home(home: astrid_core::dirs::AstridHome) - // Use the same authoritative principal-store composition as native boot. // A test helper opening the legacy import source directly would let kernel // tests pass against a runtime topology that production cannot select. - let kv = open_test_runtime_kv(&home).await; + let (kv, principal_directory) = open_test_runtime_kv(&home).await; let capabilities = Arc::new( CapabilityStore::with_kv_store(Arc::clone(&kv)) .await @@ -2644,7 +2695,7 @@ pub(crate) async fn test_kernel_with_home(home: astrid_core::dirs::AstridHome) - )); let allowance_store = Arc::new(astrid_approval::AllowanceStore::new()); - let (identity_store, ownership_store) = open_test_identity_stores(&kv); + let (identity_store, ownership_store) = open_test_identity_stores(&kv, principal_directory); let groups = Arc::new(ArcSwap::from_pointee( GroupConfig::load(&home).expect("test kernel: load groups"), @@ -4374,6 +4425,72 @@ mod tests { ); } + #[tokio::test(flavor = "multi_thread")] + async fn unload_principal_capsules_retires_every_view_without_harming_shared_peers() { + let (_d, home) = scratch_home(); + let kernel = test_kernel_with_home(home).await; + let alice = PrincipalId::new("alice").unwrap(); + let bob = PrincipalId::new("bob").unwrap(); + let shared_id = CapsuleId::new("shared").unwrap(); + let private_id = CapsuleId::new("private").unwrap(); + let shared_hash = astrid_capsule::registry::WasmHash::from_raw("shared-hash"); + let private_hash = astrid_capsule::registry::WasmHash::from_raw("private-hash"); + + let shared_cancelled = Arc::new(AtomicBool::new(false)); + let shared_unloaded = Arc::new(AtomicBool::new(false)); + let shared_cancelled_for: Arc>> = Arc::default(); + let private_cancelled = Arc::new(AtomicBool::new(false)); + let private_unloaded = Arc::new(AtomicBool::new(false)); + + { + let mut registry = kernel.capsules.write().await; + registry + .register_owned_by_default( + Box::new(CancellableTestCapsule { + id: shared_id.clone(), + manifest: CapsuleManifest::default(), + cancelled: Arc::clone(&shared_cancelled), + unloaded: Arc::clone(&shared_unloaded), + cancelled_for: Arc::clone(&shared_cancelled_for), + }), + shared_hash.clone(), + &alice, + ) + .unwrap(); + registry + .register_existing(&shared_id, &shared_hash, &bob) + .unwrap(); + registry + .register_owned_by_default( + Box::new(CancellableTestCapsule { + id: private_id.clone(), + manifest: CapsuleManifest::default(), + cancelled: Arc::clone(&private_cancelled), + unloaded: Arc::clone(&private_unloaded), + cancelled_for: Arc::default(), + }), + private_hash, + &alice, + ) + .unwrap(); + } + + let retired = kernel.unload_principal_capsules(&alice).await.unwrap(); + assert_eq!(retired, vec![private_id.clone(), shared_id.clone()]); + assert_eq!( + shared_cancelled_for.lock().unwrap().as_slice(), + &[alice.clone()] + ); + assert!(!shared_cancelled.load(Ordering::Relaxed)); + assert!(!shared_unloaded.load(Ordering::Relaxed)); + assert!(private_cancelled.load(Ordering::Relaxed)); + assert!(private_unloaded.load(Ordering::Relaxed)); + + let registry = kernel.capsules.read().await; + assert!(registry.list_for(&alice).is_empty()); + assert!(registry.get_for(&bob, &shared_id).is_some()); + } + /// A test capsule that reports `Failed` from `check_health`, for the health /// monitor dedup test. struct FailingTestCapsule { From 718689a6486aa6572aba58d78312025b301d8100 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 12 Aug 2026 21:06:39 +0400 Subject: [PATCH 2/5] fix(kernel): satisfy Rust 1.95 clippy for agent deletion Signed-off-by: Joshua J. Bouw --- crates/astrid-kernel/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/astrid-kernel/src/lib.rs b/crates/astrid-kernel/src/lib.rs index ebc7e1f73..6c33a6ac6 100644 --- a/crates/astrid-kernel/src/lib.rs +++ b/crates/astrid-kernel/src/lib.rs @@ -4479,7 +4479,7 @@ mod tests { assert_eq!(retired, vec![private_id.clone(), shared_id.clone()]); assert_eq!( shared_cancelled_for.lock().unwrap().as_slice(), - &[alice.clone()] + std::slice::from_ref(&alice) ); assert!(!shared_cancelled.load(Ordering::Relaxed)); assert!(!shared_unloaded.load(Ordering::Relaxed)); From 967193c5ab4428b0714a421c731615d8e082fdbf Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 12 Aug 2026 22:21:45 +0400 Subject: [PATCH 3/5] fix: make principal deletion fail closed Signed-off-by: Joshua J. Bouw --- CHANGELOG.md | 2 +- crates/astrid-approval/src/allowance/store.rs | 72 +++++++++ .../src/allowance/store_tests.rs | 55 +++++++ crates/astrid-capabilities/src/store.rs | 115 ++++++++++++++ crates/astrid-capabilities/src/store_tests.rs | 86 +++++++++++ crates/astrid-cli/src/commands/agent/mod.rs | 12 +- crates/astrid-core/src/kernel_api/mod.rs | 6 +- .../admin/agent_create_helpers.rs | 10 ++ .../src/kernel_router/admin/agent_delete.rs | 141 ++++++++++++++---- .../admin/state_tests_agent_delete.rs | 129 ++++++++++++++++ crates/astrid-storage/src/kv/tree.rs | 29 ++++ crates/astrid-storage/src/ownership.rs | 118 +++++++++++++++ crates/astrid-storage/src/ownership_tests.rs | 58 +++++++ crates/astrid-storage/src/principal_state.rs | 39 ++++- .../src/principal_state/purge_tests.rs | 75 ++++++++++ 15 files changed, 892 insertions(+), 55 deletions(-) create mode 100644 crates/astrid-storage/src/principal_state/purge_tests.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 2183ac3c7..58f9b8aa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ Changelog tracking starts with 0.2.0. Prior versions were not tracked. store and live legacy-SurrealKV migration reader are unchanged. Closes #1448. ### Changed -- **`agent delete` now reclaims the deleted principal's full runtime footprint.** Delete closes authz first (unlink + profile removal + cache invalidate), then retires its live capsule views and reclaims capsule KV namespaces, the home tree, signing key (`keys/{principal}.key`), and secrets (`secrets/{principal}/`). Shared runtimes remain available to other principals while principal-scoped work is cancelled before state removal. Reclamation is best-effort; failures are reported in the response's `cleanup_errors`. Replaces the previous "reclamation is an ops concern" leave-behind, and drops the interim `--purge-home` flag. Part of #1217. +- **`agent delete` now reclaims the deleted principal's full runtime footprint.** Delete fences new token and allowance authority, unlinks authentication, removes the profile, retires live capsule views, purges every immutable-UID KV namespace (including orphaned capsules), and reclaims the home tree, signing key (`keys/{principal}.key`), and secrets (`secrets/{principal}/`). Reclamation fails closed: incomplete cleanup returns an error, retains a durable alias reservation, and is safe to retry without letting a replacement identity inherit residual authority or state. Successful responses retain an empty `cleanup_errors` array for wire compatibility. Shared runtimes remain available to other principals. Replaces the previous "reclamation is an ops concern" leave-behind, and drops the interim `--purge-home` flag. Part of #1217. ### Added diff --git a/crates/astrid-approval/src/allowance/store.rs b/crates/astrid-approval/src/allowance/store.rs index 9c3c970eb..2b2b9c528 100644 --- a/crates/astrid-approval/src/allowance/store.rs +++ b/crates/astrid-approval/src/allowance/store.rs @@ -9,6 +9,7 @@ use crate::error::{ApprovalError, ApprovalResult}; use astrid_core::principal::PrincipalId; use std::collections::HashMap; +use std::collections::HashSet; use std::fmt; use std::path::Path; use std::sync::RwLock; @@ -31,6 +32,7 @@ use crate::action::SensitiveAction; /// assert_eq!(store.count(), 0); /// ``` pub struct AllowanceStore { + retiring_principals: RwLock>, /// Two-level map: `principal → allowance id → allowance`. /// /// Outer key isolates principals; inner map keeps lookups cheap within @@ -44,6 +46,7 @@ impl AllowanceStore { #[must_use] pub fn new() -> Self { Self { + retiring_principals: RwLock::new(HashSet::new()), allowances: RwLock::new(HashMap::new()), } } @@ -58,6 +61,16 @@ impl AllowanceStore { /// /// Returns a storage error if the internal lock is poisoned. pub fn add_allowance(&self, allowance: Allowance) -> ApprovalResult<()> { + let retirement = self + .retiring_principals + .read() + .map_err(|e| ApprovalError::Storage(e.to_string()))?; + if retirement.contains(&allowance.principal) { + return Err(ApprovalError::Storage(format!( + "principal {} is retiring", + allowance.principal + ))); + } let mut store = self .allowances .write() @@ -89,6 +102,13 @@ impl AllowanceStore { action: &SensitiveAction, workspace_root: Option<&Path>, ) -> Option { + if self + .retiring_principals + .read() + .map_or(true, |retiring| retiring.contains(principal)) + { + return None; + } let store = self.allowances.read().unwrap_or_else(|e| { tracing::warn!("AllowanceStore read lock poisoned, recovering"); e.into_inner() @@ -117,6 +137,13 @@ impl AllowanceStore { action: &SensitiveAction, workspace_root: Option<&Path>, ) -> Option { + if self + .retiring_principals + .read() + .map_or(true, |retiring| retiring.contains(principal)) + { + return None; + } let mut store = self.allowances.write().unwrap_or_else(|e| { tracing::warn!("AllowanceStore lock poisoned, recovering"); e.into_inner() @@ -219,6 +246,46 @@ impl AllowanceStore { } } + /// Remove every allowance owned by `principal`, regardless of scope. + /// + /// Identity deletion uses this stronger operation: an alias may later be + /// recreated as a new identity, and neither session nor persistent + /// approvals may cross that generation boundary. + pub fn clear_for_principal(&self, principal: &PrincipalId) { + let mut store = self.allowances.write().unwrap_or_else(|e| { + tracing::warn!("AllowanceStore write lock poisoned in clear_for_principal, recovering"); + e.into_inner() + }); + store.remove(principal); + } + + /// Atomically fence future allowance creation and clear existing grants. + /// + /// # Errors + /// + /// Returns a storage error if either internal lock is poisoned. + pub fn begin_principal_retirement(&self, principal: &PrincipalId) -> ApprovalResult<()> { + let mut retirement = self + .retiring_principals + .write() + .map_err(|e| ApprovalError::Storage(e.to_string()))?; + retirement.insert(principal.clone()); + let mut allowances = self + .allowances + .write() + .map_err(|e| ApprovalError::Storage(e.to_string()))?; + allowances.remove(principal); + Ok(()) + } + + /// Release an in-process retirement fence after durable reclamation. + pub fn finish_principal_retirement(&self, principal: &PrincipalId) { + self.retiring_principals + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .remove(principal); + } + /// Remove every principal's session-only allowances. /// /// Reserved for kernel-initiated global clears (shutdown). The normal @@ -353,9 +420,14 @@ impl fmt::Debug for AllowanceStore { }); (store.len(), store.values().map(HashMap::len).sum::()) }; + let retiring = self + .retiring_principals + .read() + .map_or(0, |principals| principals.len()); f.debug_struct("AllowanceStore") .field("principals", &principals) .field("count", &total) + .field("retiring_principals", &retiring) .finish() } } diff --git a/crates/astrid-approval/src/allowance/store_tests.rs b/crates/astrid-approval/src/allowance/store_tests.rs index 2a1ad12a8..2064c462b 100644 --- a/crates/astrid-approval/src/allowance/store_tests.rs +++ b/crates/astrid-approval/src/allowance/store_tests.rs @@ -462,6 +462,61 @@ fn test_store_export_is_principal_scoped() { assert_eq!(bob_exported[0].principal, bob()); } +#[test] +fn clear_for_principal_removes_every_scope_without_touching_peers() { + let store = AllowanceStore::new(); + for session_only in [true, false] { + store + .add_allowance(make_allowance_for( + alice(), + AllowancePattern::ServerTools { + server: format!("alice-{session_only}"), + }, + session_only, + )) + .unwrap(); + } + store + .add_allowance(make_allowance_for( + bob(), + AllowancePattern::ServerTools { + server: "bob".to_string(), + }, + false, + )) + .unwrap(); + + store.clear_for_principal(&alice()); + + assert_eq!(store.count_for(&alice()), 0); + assert_eq!(store.count_for(&bob()), 1); +} + +#[test] +fn retirement_fences_allowance_creation_and_authorization_until_finished() { + let store = AllowanceStore::new(); + let action = SensitiveAction::McpToolCall { + server: "retired".to_string(), + tool: "run".to_string(), + }; + let allowance = make_allowance_for( + alice(), + AllowancePattern::ServerTools { + server: "retired".to_string(), + }, + false, + ); + store.add_allowance(allowance.clone()).unwrap(); + + store.begin_principal_retirement(&alice()).unwrap(); + assert!(store.find_matching(&alice(), &action, None).is_none()); + assert!(store.add_allowance(allowance.clone()).is_err()); + + store.finish_principal_retirement(&alice()); + store.add_allowance(allowance).unwrap(); + assert!(store.find_matching(&alice(), &action, None).is_some()); +} + #[test] fn test_store_add_trusts_allowance_principal() { // Adversarial case: the Allowance's principal is the only source of diff --git a/crates/astrid-capabilities/src/store.rs b/crates/astrid-capabilities/src/store.rs index df07ca462..4ef26d3e7 100644 --- a/crates/astrid-capabilities/src/store.rs +++ b/crates/astrid-capabilities/src/store.rs @@ -56,6 +56,10 @@ const PRESENCE_MARKER: &[u8] = &[1]; /// are about the token's identity, not the caller): revoking a token /// revokes it for every principal that happened to hold it. pub struct CapabilityStore { + /// Principals undergoing durable deletion. The async lock is held for the + /// full token add/read transaction so retirement is a real authority + /// fence rather than a check-then-act race. + retiring_principals: tokio::sync::RwLock>, /// Session tokens (in-memory, cleared on session end), keyed per-principal. session_tokens: RwLock>>, /// Persistent tokens (`KvStore` backed). @@ -76,6 +80,7 @@ impl CapabilityStore { #[must_use] pub fn in_memory() -> Self { Self { + retiring_principals: tokio::sync::RwLock::new(std::collections::HashSet::new()), session_tokens: RwLock::new(HashMap::new()), persistent_store: None, revoked: RwLock::new(std::collections::HashSet::new()), @@ -98,6 +103,7 @@ impl CapabilityStore { let kv: Arc = Arc::new(store); let mut cap_store = Self { + retiring_principals: tokio::sync::RwLock::new(std::collections::HashSet::new()), session_tokens: RwLock::new(HashMap::new()), persistent_store: Some(kv), revoked: RwLock::new(std::collections::HashSet::new()), @@ -118,6 +124,7 @@ impl CapabilityStore { /// Returns an error if loading existing revoked/used tokens fails. pub async fn with_kv_store(store: Arc) -> CapabilityResult { let mut cap_store = Self { + retiring_principals: tokio::sync::RwLock::new(std::collections::HashSet::new()), session_tokens: RwLock::new(HashMap::new()), persistent_store: Some(store), revoked: RwLock::new(std::collections::HashSet::new()), @@ -188,6 +195,13 @@ impl CapabilityStore { /// /// Returns an error if the token is invalid or storage fails. pub async fn add(&self, token: CapabilityToken) -> CapabilityResult<()> { + let retirement = self.retiring_principals.read().await; + if retirement.contains(&token.principal) { + return Err(CapabilityError::StorageError(format!( + "principal {} is retiring", + token.principal + ))); + } // Validate the token first token.validate()?; @@ -255,6 +269,7 @@ impl CapabilityStore { /// fails verification (including v1 tokens still on disk after upgrade /// to v2 signing), or a storage error if reading fails. pub async fn get(&self, token_id: &TokenId) -> CapabilityResult> { + let retirement = self.retiring_principals.read().await; // Check if revoked { let revoked = self @@ -276,6 +291,9 @@ impl CapabilityStore { .map_err(|e| CapabilityError::StorageError(e.to_string()))?; for principal_map in tokens.values() { if let Some(token) = principal_map.get(token_id) { + if retirement.contains(&token.principal) { + return Ok(None); + } return Ok(Some(token.clone())); } } @@ -288,6 +306,9 @@ impl CapabilityStore { if let Some(store) = &self.persistent_store && let Some(token) = Self::read_persistent_token_any_principal(store, token_id).await? { + if retirement.contains(&token.principal) { + return Ok(None); + } return Ok(Some(token)); } @@ -401,6 +422,10 @@ impl CapabilityStore { resource: &str, permission: Permission, ) -> Option { + let retirement = self.retiring_principals.read().await; + if retirement.contains(principal) { + return None; + } // Check session tokens (this principal's inner map only). Matching // candidates are cloned out first — the `std` read guard must not be // held across the consumed-check await below. @@ -478,6 +503,21 @@ impl CapabilityStore { None } + /// Fence all token creation and authorization for a retiring principal. + /// + /// An add already in progress holds the read side of this lock until its + /// storage transaction completes. Therefore, once this method returns, + /// the subsequent purge observes every earlier add and every later add is + /// rejected. + pub async fn begin_principal_retirement(&self, principal: PrincipalId) { + self.retiring_principals.write().await.insert(principal); + } + + /// Release an in-process retirement fence after durable reclamation. + pub async fn finish_principal_retirement(&self, principal: &PrincipalId) { + self.retiring_principals.write().await.remove(principal); + } + /// Revoke a token (global — all principals). /// /// Revocation is a property of the token's identity, not the caller. @@ -570,6 +610,76 @@ impl CapabilityStore { Ok(()) } + /// Permanently remove every capability token owned by `principal`. + /// + /// This is stronger than session cleanup and is intended for identity + /// deletion. Primary rows and secondary indexes are removed so a later + /// identity that reuses the same human-readable alias cannot inherit the + /// old generation's authority. Revocation and replay tombstones are kept: + /// they are cheap, globally unique, and must remain fail-closed if deleting + /// a primary row fails. + /// + /// # Errors + /// + /// Returns a storage error after attempting every cleanup operation. The + /// in-memory session state is cleared even when durable cleanup fails. + pub async fn purge_principal(&self, principal: &PrincipalId) -> CapabilityResult<()> { + self.clear_session_for(principal)?; + let Some(store) = &self.persistent_store else { + return Ok(()); + }; + + let prefix = token_key_prefix(principal); + let primary_keys = store + .list_keys_with_prefix(NS_TOKENS, &prefix) + .await + .map_err(|e| CapabilityError::StorageError(e.to_string()))?; + let mut token_ids = std::collections::HashSet::new(); + for key in &primary_keys { + if let Some(raw_id) = key.strip_prefix(&prefix) + && let Ok(id) = uuid::Uuid::parse_str(raw_id) + { + token_ids.insert(TokenId::from_uuid(id)); + } + } + + // Include index-only remnants from an interrupted earlier purge. + let index_keys = store + .list_keys(NS_TOKEN_INDEX) + .await + .map_err(|e| CapabilityError::StorageError(e.to_string()))?; + for raw_id in index_keys { + let owner = store + .get(NS_TOKEN_INDEX, &raw_id) + .await + .map_err(|e| CapabilityError::StorageError(e.to_string()))?; + if owner.as_deref() == Some(principal.as_str().as_bytes()) + && let Ok(id) = uuid::Uuid::parse_str(&raw_id) + { + token_ids.insert(TokenId::from_uuid(id)); + } + } + + let mut failures = Vec::new(); + for key in primary_keys { + if let Err(error) = store.delete(NS_TOKENS, &key).await { + failures.push(format!("delete token {key}: {error}")); + } + } + for token_id in &token_ids { + let raw_id = token_id.0.to_string(); + if let Err(error) = store.delete(NS_TOKEN_INDEX, &raw_id).await { + failures.push(format!("delete {NS_TOKEN_INDEX}/{raw_id}: {error}")); + } + } + + if failures.is_empty() { + Ok(()) + } else { + Err(CapabilityError::StorageError(failures.join("; "))) + } + } + /// Mark a single-use token as used. /// /// This should be called after successfully using a single-use token @@ -739,6 +849,10 @@ impl std::fmt::Debug for CapabilityStore { }); let revoked_count = self.revoked.read().map_or(0, |r| r.len()); let used_count = self.used_tokens.try_read().map_or(0, |u| u.len()); + let retiring_count = self + .retiring_principals + .try_read() + .map_or(0, |principals| principals.len()); let has_persistence = self.persistent_store.is_some(); f.debug_struct("CapabilityStore") @@ -746,6 +860,7 @@ impl std::fmt::Debug for CapabilityStore { .field("session_tokens", &session_count) .field("revoked_count", &revoked_count) .field("used_count", &used_count) + .field("retiring_principals", &retiring_count) .field("has_persistence", &has_persistence) .finish() } diff --git a/crates/astrid-capabilities/src/store_tests.rs b/crates/astrid-capabilities/src/store_tests.rs index 1ae643234..62c268116 100644 --- a/crates/astrid-capabilities/src/store_tests.rs +++ b/crates/astrid-capabilities/src/store_tests.rs @@ -115,6 +115,92 @@ async fn test_clear_session() { ); } +#[tokio::test] +async fn purge_principal_removes_session_and_persistent_authority_only_for_target() { + let kv: Arc = Arc::new(MemoryKvStore::new()); + let store = CapabilityStore::with_kv_store(Arc::clone(&kv)) + .await + .unwrap(); + let keypair = test_keypair(); + let make = |principal: PrincipalId, scope: TokenScope, resource: &str| { + CapabilityToken::create( + ResourcePattern::exact(resource).unwrap(), + vec![Permission::Invoke], + scope, + keypair.key_id(), + AuditEntryId::new(), + &keypair, + None, + principal, + ) + }; + let alice_session = make(alice(), TokenScope::Session, "mcp://alice:session"); + let alice_persistent = make(alice(), TokenScope::Persistent, "mcp://alice:persistent"); + let alice_persistent_id = alice_persistent.id.clone(); + let bob_persistent = make(bob(), TokenScope::Persistent, "mcp://bob:persistent"); + let bob_persistent_id = bob_persistent.id.clone(); + store.add(alice_session).await.unwrap(); + store.add(alice_persistent).await.unwrap(); + store.add(bob_persistent).await.unwrap(); + store.revoke(&alice_persistent_id).await.unwrap(); + + store.purge_principal(&alice()).await.unwrap(); + + assert!(matches!( + store.get(&alice_persistent_id).await, + Err(CapabilityError::TokenRevoked { .. }) + )); + assert!(store.get(&bob_persistent_id).await.unwrap().is_some()); + assert!( + !store + .has_capability(&alice(), "mcp://alice:session", Permission::Invoke) + .await + ); + assert!( + store + .has_capability(&bob(), "mcp://bob:persistent", Permission::Invoke) + .await + ); + assert!( + kv.get(NS_TOKEN_INDEX, &alice_persistent_id.0.to_string()) + .await + .unwrap() + .is_none() + ); + assert!( + kv.get(NS_REVOKED, &alice_persistent_id.0.to_string()) + .await + .unwrap() + .is_some(), + "revocation tombstones must outlive identity cleanup" + ); +} + +#[tokio::test] +async fn retirement_fences_token_creation_and_lookup_until_finished() { + let store = CapabilityStore::in_memory(); + let keypair = test_keypair(); + let token = CapabilityToken::create( + ResourcePattern::exact("mcp://alice:retired").unwrap(), + vec![Permission::Invoke], + TokenScope::Session, + keypair.key_id(), + AuditEntryId::new(), + &keypair, + None, + alice(), + ); + let token_id = token.id.clone(); + store.add(token.clone()).await.unwrap(); + + store.begin_principal_retirement(alice()).await; + assert!(store.get(&token_id).await.unwrap().is_none()); + assert!(store.add(token.clone()).await.is_err()); + + store.finish_principal_retirement(&alice()).await; + assert!(store.get(&token_id).await.unwrap().is_some()); +} + #[tokio::test] async fn test_find_capability() { let store = CapabilityStore::in_memory(); diff --git a/crates/astrid-cli/src/commands/agent/mod.rs b/crates/astrid-cli/src/commands/agent/mod.rs index 88923f9ee..e52657cc3 100644 --- a/crates/astrid-cli/src/commands/agent/mod.rs +++ b/crates/astrid-cli/src/commands/agent/mod.rs @@ -676,17 +676,7 @@ async fn run_delete(args: DeleteArgs) -> Result { let body = client .request(AdminRequestKind::AgentDelete { principal }) .await?; - let outcome = into_result(body)?; - // Surface any footprint-reclamation failures the kernel reported - // (delete closes authz regardless; leftovers are an ops follow-up). - if let AdminResponseBody::Success(v) = &outcome - && let Some(errs) = v.get("cleanup_errors").and_then(|e| e.as_array()) - && !errs.is_empty() - { - for e in errs.iter().filter_map(|e| e.as_str()) { - eprintln!("warning: footprint cleanup: {e}"); - } - } + into_result(body)?; println!( "{}", Theme::success(&format!("Deleted agent '{}'", args.name)) diff --git a/crates/astrid-core/src/kernel_api/mod.rs b/crates/astrid-core/src/kernel_api/mod.rs index 73bc1a8da..1257264f6 100644 --- a/crates/astrid-core/src/kernel_api/mod.rs +++ b/crates/astrid-core/src/kernel_api/mod.rs @@ -452,8 +452,10 @@ pub enum AdminRequestKind { /// profile removal + cache invalidate), then reclaims the principal's /// on-disk footprint — home tree (`home/{principal}/`), signing key /// (`keys/{principal}.key`), and secrets (`secrets/{principal}/`). - /// Reclamation is best-effort; any failures are reported in the - /// response's `cleanup_errors` (#1217). + /// Reclamation fails closed: any incomplete authority or filesystem + /// cleanup returns an error, retains a durable alias reservation, and is + /// safe to retry. Successful responses retain an empty `cleanup_errors` + /// array for wire compatibility (#1217). AgentDelete { /// Principal to delete. principal: PrincipalId, diff --git a/crates/astrid-kernel/src/kernel_router/admin/agent_create_helpers.rs b/crates/astrid-kernel/src/kernel_router/admin/agent_create_helpers.rs index 3292143aa..a88fc83a5 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/agent_create_helpers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/agent_create_helpers.rs @@ -42,6 +42,16 @@ pub(super) async fn provision_new_principal( clone_from: Option, allow_admin_clone: bool, ) -> AdminResponseBody { + if let Err(error) = kernel + .ownership_store + .ensure_alias_available(&principal) + .await + { + return err_bad_input(format!( + "principal alias `{principal}` is unavailable: {error}" + )); + } + // Build the profile: a `clone_from` replica (validated + admin-guarded) or // a fresh profile from the supplied groups/grants. Runs under the lock so // the clone source is pinned across the read. diff --git a/crates/astrid-kernel/src/kernel_router/admin/agent_delete.rs b/crates/astrid-kernel/src/kernel_router/admin/agent_delete.rs index 0f4141cc5..429199d97 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/agent_delete.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/agent_delete.rs @@ -28,6 +28,16 @@ pub(super) async fn agent_delete( Ok(pending) => pending, Err(response) => return response, }; + kernel + .capabilities + .begin_principal_retirement(principal.clone()) + .await; + if let Err(error) = kernel + .allowance_store + .begin_principal_retirement(&principal) + { + return err_internal(format!("allowance retirement fence failed: {error}")); + } if let Err(e) = kernel .identity_store @@ -49,12 +59,19 @@ pub(super) async fn agent_delete( kernel.profile_cache.invalidate(&principal); let (unloaded_capsules, reclaimed, cleanup_errors) = - match retire_and_reclaim(kernel, &principal).await { + match retire_and_reclaim(kernel, &principal, pending.principal_uid).await { Ok(result) => result, Err(e) => return err_internal(e), }; - if let Err(response) = finish_identity_removal(kernel, pending).await { + if !cleanup_errors.is_empty() { + return err_internal(format!( + "principal reclamation incomplete; alias remains reserved: {}", + cleanup_errors.join("; ") + )); + } + + if let Err(response) = finish_identity_removal(kernel, &principal, pending).await { return response; } @@ -69,6 +86,7 @@ pub(super) async fn agent_delete( struct PendingIdentityRemoval { user: Option, + principal_uid: Option, ownership_guard: Option, } @@ -93,7 +111,7 @@ async fn prepare_identity_removal( .find(|user| user.principal == *principal) }; - let ownership_guard = if let Some(user) = resolved.as_ref() { + let (principal_uid, ownership_guard) = if let Some(user) = resolved.as_ref() { let identity = kernel .identity_store .get_principal_identity(user.id) @@ -109,7 +127,7 @@ async fn prepare_identity_removal( .guard_principal_deletion_for_alias(identity.uid, principal.clone()) .await { - Ok(guard) => Some(guard), + Ok(guard) => (Some(identity.uid), Some(guard)), Err(astrid_storage::OwnershipError::PrincipalAlreadyOwned { fleet, .. }) => { return Err(err_bad_input(format!( "cannot delete principal `{principal}` while it is assigned to fleet {fleet}" @@ -122,28 +140,51 @@ async fn prepare_identity_removal( }, } } else { - None + let guard = recover_or_reserve_legacy_alias(kernel, principal).await?; + (None, Some(guard)) } } else { - kernel - .ownership_store - .finish_principal_deletion_by_alias(principal) - .await - .map_err(|e| err_internal(format!("ownership store deletion recovery failed: {e}")))?; - None + let guard = recover_or_reserve_legacy_alias(kernel, principal).await?; + (Some(guard.principal_uid()), Some(guard)) }; Ok(PendingIdentityRemoval { user: resolved, + principal_uid, ownership_guard, }) } +async fn recover_or_reserve_legacy_alias( + kernel: &Arc, + principal: &PrincipalId, +) -> Result { + if let Some(guard) = kernel + .ownership_store + .resume_principal_deletion_by_alias(principal) + .await + .map_err(|e| err_internal(format!("ownership store deletion recovery failed: {e}")))? + { + return Ok(guard); + } + kernel + .ownership_store + .guard_legacy_alias_deletion(principal.clone()) + .await + .map_err(|e| err_internal(format!("ownership store legacy deletion guard failed: {e}"))) +} + async fn finish_identity_removal( kernel: &Arc, + principal: &PrincipalId, pending: PendingIdentityRemoval, ) -> Result<(), AdminResponseBody> { - if let Some(user) = pending.user { + let PendingIdentityRemoval { + user, + principal_uid, + ownership_guard, + } = pending; + if let Some(user) = user { match kernel.identity_store.delete_user(user.id).await { Ok(true) => {}, Ok(false) => { @@ -158,13 +199,35 @@ async fn finish_identity_removal( }, } } - if let Some(guard) = pending.ownership_guard { + // Deleting the durable identity removes the alias→UID directory entry, + // fencing every principal-scoped KV resolver. Purge once more after that + // fence so a late write from an invocation dispatched before unload cannot + // recreate state behind the reclaimed root. + kernel.allowance_store.clear_for_principal(principal); + kernel + .capabilities + .purge_principal(principal) + .await + .map_err(|e| err_internal(format!("post-identity capability purge failed: {e}")))?; + if let (Some(store), Some(uid)) = (&kernel.principal_store, principal_uid) { + store.purge_principal_kv(uid).map_err(|e| { + err_internal(format!("post-identity principal state purge failed: {e}")) + })?; + } + if let Some(guard) = ownership_guard { guard.finish().await.map_err(|e| { err_internal(format!( "ownership store deletion reservation cleanup failed: {e}" )) })?; } + kernel + .capabilities + .finish_principal_retirement(principal) + .await; + kernel + .allowance_store + .finish_principal_retirement(principal); Ok(()) } @@ -177,32 +240,44 @@ type ReclaimOutcome = ( async fn retire_and_reclaim( kernel: &Arc, principal: &PrincipalId, + principal_uid: Option, ) -> Result { let unloaded = kernel .unload_principal_capsules(principal) .await .map_err(|e| format!("failed to retire capsule views for `{principal}`: {e}"))?; - // KV lives in the kernel store rather than below the principal home, so - // reclaim each capsule namespace explicitly before deleting the install - // tree. The live view covers active capsules; the on-disk set also covers - // installed capsules that failed to load. - let capsule_dir = kernel.astrid_home.principal_home(principal).capsules_dir(); - let mut capsule_ids: BTreeSet = unloaded.iter().map(ToString::to_string).collect(); - if let Ok(entries) = std::fs::read_dir(&capsule_dir) { - capsule_ids.extend(entries.flatten().filter_map(|entry| { - entry - .file_type() - .ok() - .filter(std::fs::FileType::is_dir) - .and_then(|_| entry.file_name().into_string().ok()) - })); + kernel.allowance_store.clear_for_principal(principal); + let mut authority_errors = Vec::new(); + if let Err(error) = kernel.capabilities.purge_principal(principal).await { + authority_errors.push(format!("capabilities: {error}")); } - let mut kv_errors = Vec::new(); - for capsule in capsule_ids { - let namespace = format!("{principal}:capsule:{capsule}"); - if let Err(error) = kernel.kv.clear_namespace(&namespace).await { - kv_errors.push(format!("kv namespace {namespace}: {error}")); + + // Native storage has an authoritative immutable-UID root. Removing it + // reclaims every capsule namespace, including already-uninstalled or + // corrupt/missing installations. Legacy/test compositions without the + // native store retain the prior best-effort namespace fallback. + if let (Some(store), Some(uid)) = (&kernel.principal_store, principal_uid) { + if let Err(error) = store.purge_principal_kv(uid) { + authority_errors.push(format!("principal state: {error}")); + } + } else { + let capsule_dir = kernel.astrid_home.principal_home(principal).capsules_dir(); + let mut capsule_ids: BTreeSet = unloaded.iter().map(ToString::to_string).collect(); + if let Ok(entries) = std::fs::read_dir(&capsule_dir) { + capsule_ids.extend(entries.flatten().filter_map(|entry| { + entry + .file_type() + .ok() + .filter(std::fs::FileType::is_dir) + .and_then(|_| entry.file_name().into_string().ok()) + })); + } + for capsule in capsule_ids { + let namespace = format!("{principal}:capsule:{capsule}"); + if let Err(error) = kernel.kv.clear_namespace(&namespace).await { + authority_errors.push(format!("kv namespace {namespace}: {error}")); + } } } @@ -227,7 +302,7 @@ async fn retire_and_reclaim( .map_err(|e| format!("agent footprint reclamation task failed: {e}"))?; let mut reclaimed = Vec::new(); - let mut cleanup_errors = kv_errors; + let mut cleanup_errors = authority_errors; if cleanup_errors.is_empty() { reclaimed.push("kv"); } diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_delete.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_delete.rs index 88da6ddda..9b6c93159 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_delete.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests_agent_delete.rs @@ -7,6 +7,8 @@ use astrid_core::dirs::AstridHome; use astrid_core::groups::{BUILTIN_ADMIN, BUILTIN_AGENT}; use astrid_core::principal::PrincipalId; use astrid_core::profile::PrincipalProfile; +use astrid_core::{Permission, types::Timestamp}; +use astrid_crypto::KeyPair; use astrid_events::kernel_api::{AdminRequestKind, AdminResponseBody}; use super::handlers; @@ -140,3 +142,130 @@ async fn agent_delete_closes_authz_before_reclaiming() { assert!(after.groups.is_empty() && after.grants.is_empty()); assert!(!home.exists() && !key.exists() && !secrets.exists()); } + +#[tokio::test(flavor = "multi_thread")] +async fn agent_delete_purges_every_token_and_allowance_scope() { + use astrid_approval::{Allowance, AllowanceId, AllowancePattern}; + use astrid_capabilities::{AuditEntryId, CapabilityToken, ResourcePattern, TokenScope}; + + let (_dir, kernel) = fixture().await; + let principal = PrincipalId::new("authority").unwrap(); + create(&kernel, &principal).await; + let keypair = KeyPair::generate(); + let make_token = |scope| { + CapabilityToken::create( + ResourcePattern::exact("mcp://danger:run").unwrap(), + vec![Permission::Invoke], + scope, + keypair.key_id(), + AuditEntryId::new(), + &keypair, + None, + principal.clone(), + ) + }; + let session = make_token(TokenScope::Session); + let persistent = make_token(TokenScope::Persistent); + let persistent_id = persistent.id.clone(); + kernel.capabilities.add(session).await.unwrap(); + kernel.capabilities.add(persistent).await.unwrap(); + for session_only in [true, false] { + kernel + .allowance_store + .add_allowance(Allowance { + id: AllowanceId::new(), + principal: principal.clone(), + action_pattern: AllowancePattern::ServerTools { + server: format!("danger-{session_only}"), + }, + created_at: Timestamp::now(), + expires_at: None, + max_uses: None, + uses_remaining: None, + session_only, + workspace_root: None, + signature: keypair.sign(b"allowance"), + }) + .unwrap(); + } + + let response = handlers::dispatch( + &kernel, + &PrincipalId::default(), + AdminRequestKind::AgentDelete { + principal: principal.clone(), + }, + ) + .await; + + assert!(matches!(response, AdminResponseBody::Success(_))); + assert_eq!(kernel.allowance_store.count_for(&principal), 0); + assert!( + kernel + .capabilities + .get(&persistent_id) + .await + .unwrap() + .is_none() + ); + assert!( + !kernel + .capabilities + .has_capability(&principal, "mcp://danger:run", Permission::Invoke) + .await + ); +} + +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread")] +async fn failed_reclamation_keeps_alias_reserved_until_retry_succeeds() { + use std::os::unix::fs::PermissionsExt as _; + + let (_dir, kernel) = fixture().await; + let principal = PrincipalId::new("retry-delete").unwrap(); + create(&kernel, &principal).await; + let homes = kernel + .astrid_home + .principal_home(&principal) + .root() + .parent() + .unwrap() + .to_path_buf(); + let original_mode = std::fs::metadata(&homes).unwrap().permissions().mode(); + std::fs::set_permissions(&homes, std::fs::Permissions::from_mode(0o500)).unwrap(); + + let failed = handlers::dispatch( + &kernel, + &PrincipalId::default(), + AdminRequestKind::AgentDelete { + principal: principal.clone(), + }, + ) + .await; + assert!(matches!(failed, AdminResponseBody::Error(_))); + let recreate = handlers::dispatch( + &kernel, + &PrincipalId::default(), + AdminRequestKind::AgentCreate { + name: principal.to_string(), + groups: vec![BUILTIN_AGENT.to_string()], + grants: Vec::new(), + inherit_from: None, + clone_from: None, + allow_admin_clone: false, + }, + ) + .await; + assert!(matches!(recreate, AdminResponseBody::Error(_))); + + std::fs::set_permissions(&homes, std::fs::Permissions::from_mode(original_mode)).unwrap(); + let retried = handlers::dispatch( + &kernel, + &PrincipalId::default(), + AdminRequestKind::AgentDelete { + principal: principal.clone(), + }, + ) + .await; + assert!(matches!(retried, AdminResponseBody::Success(_))); +} diff --git a/crates/astrid-storage/src/kv/tree.rs b/crates/astrid-storage/src/kv/tree.rs index 795301e0d..3bb5d69c7 100644 --- a/crates/astrid-storage/src/kv/tree.rs +++ b/crates/astrid-storage/src/kv/tree.rs @@ -413,6 +413,35 @@ where E: KvProjectionEngine

+ 'static, R: KvPrincipalResolver

, { + /// Atomically remove every visible KV entry owned by `owner`. + /// + /// This privileged lifecycle operation works from the authoritative tree + /// and overlay rather than caller-supplied namespace names, so orphaned + /// namespaces remain reclaimable after their capsule is uninstalled. + /// + /// # Errors + /// + /// Returns a storage error if the owner's authoritative KV state cannot be + /// read or the clearing mutation cannot be committed. + pub fn clear_owner(&self, owner: &P) -> StorageResult { + self.blocking_store().mutate(owner, |context, header| { + let mut entries = BTreeMap::, Option>>::new(); + context.visit_entries(header.tree, |composite, value| { + entries.insert(composite.to_vec(), Some(value.to_vec())); + Ok(()) + })?; + for (key, value) in header.overlay.all() { + entries.insert(key, value); + } + let mutations = entries + .into_iter() + .filter_map(|(key, value)| value.map(|_| (key, None))) + .collect::>(); + let count = u64::try_from(mutations.len()).unwrap_or(u64::MAX); + Ok((count, mutations, count != 0)) + }) + } + #[cfg(all(feature = "legacy-surrealkv", not(target_family = "wasm")))] pub(crate) fn import_entries_for_migration( &self, diff --git a/crates/astrid-storage/src/ownership.rs b/crates/astrid-storage/src/ownership.rs index 1a1c2f7fc..84a5fe884 100644 --- a/crates/astrid-storage/src/ownership.rs +++ b/crates/astrid-storage/src/ownership.rs @@ -230,6 +230,12 @@ pub struct PrincipalDeletionGuard { } impl PrincipalDeletionGuard { + /// Immutable principal generation protected by this reservation. + #[must_use] + pub const fn principal_uid(&self) -> PrincipalUid { + self.principal_uid + } + /// Remove the durable reservation after identity removal completes. /// /// # Errors @@ -313,6 +319,62 @@ impl OwnershipStore { .await } + /// Reserve an alias whose legacy identity generation is already missing. + /// + /// Recovery code uses this before touching alias-keyed files so a failed + /// cleanup cannot make an old key, home, or secret tree available to a new + /// identity. The synthetic UID exists only as the durable map key for this + /// reservation and is derived in a separate domain from real identities. + /// + /// # Errors + /// + /// Fails closed if the alias is already reserved, the synthetic key + /// collides with a live principal, or the ownership graph cannot be saved. + pub async fn guard_legacy_alias_deletion( + &self, + alias: PrincipalId, + ) -> Result { + let mut hasher = blake3::Hasher::new_derive_key( + "astrid legacy alias deletion reservation v1", + ); + hasher.update(alias.as_str().as_bytes()); + let reservation_uid = PrincipalUid::from_bytes(*hasher.finalize().as_bytes()); + let guard = Arc::clone(&self.mutation_lock).lock_owned().await; + self.mutate_unlocked(|graph| { + if self.principals.contains_uid(reservation_uid) { + return Err(OwnershipError::CorruptGraph(format!( + "legacy deletion reservation for alias {alias} collides with live principal {reservation_uid}" + ))); + } + if let Some((principal, _)) = graph + .principal_deletions + .iter() + .find(|(_, reservation)| reservation.alias.as_ref() == Some(&alias)) + { + if *principal != reservation_uid { + return Err(OwnershipError::DeletionAliasReserved { + alias: alias.clone(), + principal: *principal, + }); + } + } else { + graph.principal_deletions.insert( + reservation_uid, + PrincipalDeletionReservation { + alias: Some(alias.clone()), + }, + ); + } + Ok(()) + }) + .await?; + Ok(PrincipalDeletionGuard { + store: self.clone(), + principal_uid: reservation_uid, + _guard: guard, + }) + } + /// Finish a previously interrupted deletion using its durable alias. /// /// Returns `true` when a matching reservation was removed and `false` @@ -346,6 +408,62 @@ impl OwnershipStore { .await } + /// Reacquire an interrupted deletion reservation by its retained alias. + /// + /// Unlike [`finish_principal_deletion_by_alias`](Self::finish_principal_deletion_by_alias), + /// this does not remove the reservation. The caller must first finish all + /// generation-scoped reclamation and then call [`PrincipalDeletionGuard::finish`]. + /// + /// # Errors + /// + /// Returns an ownership error if the graph cannot be loaded or the retired + /// principal is unexpectedly live again. + pub async fn resume_principal_deletion_by_alias( + &self, + alias: &PrincipalId, + ) -> Result, OwnershipError> { + let guard = Arc::clone(&self.mutation_lock).lock_owned().await; + let graph = self.load().await?; + let principal_uid = graph + .principal_deletions + .iter() + .find_map(|(uid, reservation)| { + (reservation.alias.as_ref() == Some(alias)).then_some(*uid) + }); + let Some(principal_uid) = principal_uid else { + return Ok(None); + }; + if self.principals.contains_uid(principal_uid) { + return Err(OwnershipError::PrincipalDeletionStillLive(principal_uid)); + } + Ok(Some(PrincipalDeletionGuard { + store: self.clone(), + principal_uid, + _guard: guard, + })) + } + + /// Reject creation while an interrupted deletion still owns `alias`. + /// + /// # Errors + /// + /// Returns an ownership error if the graph cannot be loaded or `alias` is + /// still reserved by an incomplete deletion. + pub async fn ensure_alias_available(&self, alias: &PrincipalId) -> Result<(), OwnershipError> { + let graph = self.load().await?; + if let Some((principal, _)) = graph + .principal_deletions + .iter() + .find(|(_, reservation)| reservation.alias.as_ref() == Some(alias)) + { + return Err(OwnershipError::DeletionAliasReserved { + alias: alias.clone(), + principal: *principal, + }); + } + Ok(()) + } + async fn guard_principal_deletion_inner( &self, principal_uid: PrincipalUid, diff --git a/crates/astrid-storage/src/ownership_tests.rs b/crates/astrid-storage/src/ownership_tests.rs index 2ec1f1d12..ea67420d8 100644 --- a/crates/astrid-storage/src/ownership_tests.rs +++ b/crates/astrid-storage/src/ownership_tests.rs @@ -520,6 +520,64 @@ async fn deletion_reservation_can_be_finished_by_alias_after_identity_disappears )); } +#[tokio::test] +async fn interrupted_deletion_reserves_alias_until_resumed_guard_finishes() { + let backend = Arc::new(MemoryKvStore::new()); + let principals = PrincipalDirectory::default(); + let store = OwnershipStore::new(backend, principals.clone()).unwrap(); + let principal_uid = principal(20, 2); + let alias = astrid_core::PrincipalId::new("recoverable-deletion").unwrap(); + principals.register(alias.clone(), principal_uid).unwrap(); + let guard = store + .guard_principal_deletion_for_alias(principal_uid, alias.clone()) + .await + .unwrap(); + principals.unregister(&alias, principal_uid); + drop(guard); + + assert!(matches!( + store.ensure_alias_available(&alias).await, + Err(OwnershipError::DeletionAliasReserved { principal, .. }) + if principal == principal_uid + )); + let resumed = store + .resume_principal_deletion_by_alias(&alias) + .await + .unwrap() + .expect("reservation exists"); + assert_eq!(resumed.principal_uid(), principal_uid); + resumed.finish().await.unwrap(); + store.ensure_alias_available(&alias).await.unwrap(); +} + +#[tokio::test] +async fn legacy_alias_reservation_blocks_recreation_without_a_live_identity() { + let backend = Arc::new(MemoryKvStore::new()); + let principals = PrincipalDirectory::default(); + let store = OwnershipStore::new(backend, principals).unwrap(); + let alias = astrid_core::PrincipalId::new("legacy-partial-delete").unwrap(); + + let guard = store + .guard_legacy_alias_deletion(alias.clone()) + .await + .unwrap(); + let reservation_uid = guard.principal_uid(); + drop(guard); + + assert!(matches!( + store.ensure_alias_available(&alias).await, + Err(OwnershipError::DeletionAliasReserved { principal, .. }) + if principal == reservation_uid + )); + let resumed = store + .resume_principal_deletion_by_alias(&alias) + .await + .unwrap() + .expect("legacy reservation exists"); + resumed.finish().await.unwrap(); + store.ensure_alias_available(&alias).await.unwrap(); +} + #[tokio::test] async fn deletion_reservation_rejects_a_second_deletion_for_the_same_alias() { let backend = Arc::new(MemoryKvStore::new()); diff --git a/crates/astrid-storage/src/principal_state.rs b/crates/astrid-storage/src/principal_state.rs index c04fb7d8c..1d410c765 100644 --- a/crates/astrid-storage/src/principal_state.rs +++ b/crates/astrid-storage/src/principal_state.rs @@ -219,6 +219,7 @@ pub type NativePrincipalContentStore = PrincipalContentStore< #[derive(Clone)] pub struct RuntimePrincipalStore { engine: Arc, + runtime_kv: Arc, kv: Arc, content: Arc, staging: Arc, @@ -273,6 +274,25 @@ impl RuntimePrincipalStore { self.principals.clone() } + /// Remove every KV namespace owned by one immutable principal UID. + /// + /// This is the authoritative identity-deletion primitive: it reclaims + /// every capsule KV namespace without guessing capsule IDs from the live + /// registry or installation directory. Other typed state components remain + /// bound to the retired immutable UID and cannot be inherited by a later + /// identity that reuses the alias. + /// + /// # Errors + /// + /// Returns a storage error if the principal's authoritative KV state cannot + /// be read or the clearing mutation cannot be committed. + pub fn purge_principal_kv(&self, principal: PrincipalUid) -> StorageResult { + let owner = StateOwner::Principal(principal); + self.runtime_kv + .clear_owner(&owner) + .map(|removed| removed != 0) + } + /// Inspect one owner's exact catalog names under a target-volume policy. /// /// This is a read-only diagnostic. It never mutates the principal root or @@ -509,14 +529,14 @@ async fn open_runtime_principal_store_with_options( })??; } - let kv: Arc = - Arc::new(RuntimeStore::from_engine_with_quota_and_content_validation( - Arc::clone(&engine), - StateOwnerResolver::new(principals.clone()), - Arc::clone("a), - Arc::clone(&validated_kv), - Arc::clone(&validated_catalogs), - )); + let runtime_kv = Arc::new(RuntimeStore::from_engine_with_quota_and_content_validation( + Arc::clone(&engine), + StateOwnerResolver::new(principals.clone()), + Arc::clone("a), + Arc::clone(&validated_kv), + Arc::clone(&validated_catalogs), + )); + let kv: Arc = runtime_kv.clone(); KvIdentityStore::with_principal_directory( ScopedKvStore::new(Arc::clone(&kv), "system:identity")?, principals.clone(), @@ -539,6 +559,7 @@ async fn open_runtime_principal_store_with_options( let staging = Arc::new(NativeContentStagingArea::open(home.content_staging_path())?); Ok(RuntimePrincipalStore { engine, + runtime_kv, kv, content, staging, @@ -585,3 +606,5 @@ pub async fn open_runtime_kv_with_directory( #[cfg(test)] mod runtime_tests; +#[cfg(test)] +mod purge_tests; diff --git a/crates/astrid-storage/src/principal_state/purge_tests.rs b/crates/astrid-storage/src/principal_state/purge_tests.rs new file mode 100644 index 000000000..df0a0c06c --- /dev/null +++ b/crates/astrid-storage/src/principal_state/purge_tests.rs @@ -0,0 +1,75 @@ +use std::sync::Arc; + +use astrid_core::dirs::AstridHome; +use astrid_core::identity::PrincipalUid; +use astrid_core::principal::PrincipalId; + +use super::{RuntimePrincipalStore, StateOwner, open_runtime_principal_store}; +use crate::KvQuotaResolver; + +fn unlimited_quota() -> Arc> { + Arc::new(|owner: &StateOwner| { + Ok(match owner { + StateOwner::System => None, + StateOwner::Principal(_) => Some(u64::MAX), + }) + }) +} + +fn create_principal(store: &RuntimePrincipalStore, alias: &str) -> PrincipalUid { + let uid = PrincipalUid::from_bytes(*blake3::hash(alias.as_bytes()).as_bytes()); + store + .principal_directory() + .register(PrincipalId::new(alias).unwrap(), uid) + .unwrap(); + uid +} + +#[tokio::test] +async fn principal_kv_purge_removes_orphan_namespaces_without_touching_peers() { + let directory = tempfile::tempdir().unwrap(); + let home = AstridHome::from_path(directory.path()); + let store = open_runtime_principal_store(&home, unlimited_quota()) + .await + .unwrap(); + let alice_uid = create_principal(&store, "alice"); + create_principal(&store, "bob"); + store + .kv() + .set("alice:capsule:removed", "orphan", b"secret".to_vec()) + .await + .unwrap(); + store + .kv() + .set("alice:capsule:live", "state", b"state".to_vec()) + .await + .unwrap(); + store + .kv() + .set("bob:capsule:live", "state", b"bob".to_vec()) + .await + .unwrap(); + + assert!(store.purge_principal_kv(alice_uid).unwrap()); + + assert!( + store + .kv() + .get("alice:capsule:removed", "orphan") + .await + .unwrap() + .is_none() + ); + assert!( + store + .kv() + .get("alice:capsule:live", "state") + .await + .unwrap() + .is_none() + ); + assert_eq!( + store.kv().get("bob:capsule:live", "state").await.unwrap(), + Some(b"bob".to_vec()) + ); +} From 29a59d26a879190c8b936a174598c2fcf401cadb Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 12 Aug 2026 22:35:28 +0400 Subject: [PATCH 4/5] style: apply rustfmt Signed-off-by: Joshua J. Bouw --- crates/astrid-storage/src/ownership.rs | 5 ++--- crates/astrid-storage/src/principal_state.rs | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/astrid-storage/src/ownership.rs b/crates/astrid-storage/src/ownership.rs index 84a5fe884..af702f057 100644 --- a/crates/astrid-storage/src/ownership.rs +++ b/crates/astrid-storage/src/ownership.rs @@ -334,9 +334,8 @@ impl OwnershipStore { &self, alias: PrincipalId, ) -> Result { - let mut hasher = blake3::Hasher::new_derive_key( - "astrid legacy alias deletion reservation v1", - ); + let mut hasher = + blake3::Hasher::new_derive_key("astrid legacy alias deletion reservation v1"); hasher.update(alias.as_str().as_bytes()); let reservation_uid = PrincipalUid::from_bytes(*hasher.finalize().as_bytes()); let guard = Arc::clone(&self.mutation_lock).lock_owned().await; diff --git a/crates/astrid-storage/src/principal_state.rs b/crates/astrid-storage/src/principal_state.rs index 1d410c765..9a9eeb542 100644 --- a/crates/astrid-storage/src/principal_state.rs +++ b/crates/astrid-storage/src/principal_state.rs @@ -604,7 +604,7 @@ pub async fn open_runtime_kv_with_directory( .map(|store| store.kv()) } -#[cfg(test)] -mod runtime_tests; #[cfg(test)] mod purge_tests; +#[cfg(test)] +mod runtime_tests; From e84c8027e315b9afa31e774f984d2bb63b6ca813 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 12 Aug 2026 22:57:10 +0400 Subject: [PATCH 5/5] fix(storage): make deletion reservations WASM-portable Signed-off-by: Joshua J. Bouw --- crates/astrid-storage/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/astrid-storage/Cargo.toml b/crates/astrid-storage/Cargo.toml index 57df51f08..6817a8a55 100644 --- a/crates/astrid-storage/Cargo.toml +++ b/crates/astrid-storage/Cargo.toml @@ -23,6 +23,7 @@ astrid-storage-content = { workspace = true } astrid-storage-engine = { workspace = true } astrid-storage-model = { workspace = true } async-trait = { workspace = true } +blake3 = { workspace = true } chrono = { workspace = true, features = ["clock"] } hex = { workspace = true } keyring = { workspace = true, optional = true } @@ -60,7 +61,6 @@ getrandom = { version = "0.4", features = ["wasm_js"] } [target.'cfg(not(target_family = "wasm"))'.dependencies] astrid-resources = { workspace = true } -blake3 = { workspace = true } cap-std = "4.0.2" caseless = { workspace = true } unicode-normalization = { workspace = true }