From 223e565de8cc7df0a5a83ca722b570694a92f9a8 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 14 Jul 2026 20:10:24 +0400 Subject: [PATCH 01/16] fix(kernel): preserve device attenuation across authority views --- CHANGELOG.md | 6 + .../src/kernel_router/admin/group.rs | 33 +++- .../src/kernel_router/admin/handlers.rs | 51 +++-- .../src/kernel_router/admin/mod.rs | 6 +- .../admin/pair_device_handlers.rs | 62 +++--- .../kernel_router/admin/pair_device_tests.rs | 144 ++++++++++++++ .../src/kernel_router/admin/state_tests.rs | 102 +++++++++- .../kernel_router/admin/state_tests_group.rs | 91 ++++++++- .../src/kernel_router/device_scope.rs | 52 +++++ crates/astrid-kernel/src/kernel_router/mod.rs | 70 +++---- .../astrid-kernel/src/kernel_router/tests.rs | 180 +++++++++++++++++- 11 files changed, 678 insertions(+), 119 deletions(-) create mode 100644 crates/astrid-kernel/src/kernel_router/device_scope.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cdcbb440..4e6de54e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -106,6 +106,12 @@ Changelog tracking starts with 0.2.0. Prior versions were not tracked. ### Security +- **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 + authority. Malformed, unknown, and revoked device IDs fail closed. Closes + #1237. + - Bumped the locked `crossbeam-epoch` transitive dependency to `0.9.20`, clearing `RUSTSEC-2026-0204` (published 2026-07-06 — an invalid pointer dereference in the `fmt::Pointer` / pre-0.9 `fmt::Display` impls for `Atomic` and `Shared` when the underlying pointer is null or invalid) from the RustSec audit. The advisory landed the day after the 0.9.3 release and turned the Security Audit CI check red on `main` and every open PR; nothing in our own code changed. Lockfile-only, same-minor patch — no other dependency moved. Closes #1161. ## [0.9.3] - 2026-07-06 diff --git a/crates/astrid-kernel/src/kernel_router/admin/group.rs b/crates/astrid-kernel/src/kernel_router/admin/group.rs index 7ecde0aa2..981d55c1e 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/group.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/group.rs @@ -68,9 +68,13 @@ pub(super) async fn group_modify( commit_group_config(kernel, next) } -pub(super) fn group_list(kernel: &Arc, caller: &PrincipalId) -> AdminResponseBody { +pub(super) fn group_list( + kernel: &Arc, + caller: &PrincipalId, + device_key_id: Option<&str>, +) -> AdminResponseBody { let cfg = kernel.groups.load_full(); - let visible_groups = if caller_has_global_group_list(kernel, caller) { + let visible_groups = if caller_has_global_group_list(kernel, caller, device_key_id) { None } else { Some(caller_group_names(kernel, caller)) @@ -94,13 +98,32 @@ pub(super) fn group_list(kernel: &Arc, caller: &PrincipalId) -> A AdminResponseBody::GroupList(summaries) } -fn caller_has_global_group_list(kernel: &Arc, caller: &PrincipalId) -> bool { +fn caller_has_global_group_list( + kernel: &Arc, + caller: &PrincipalId, + device_key_id: Option<&str>, +) -> bool { let Ok(profile) = kernel.profile_cache.resolve(caller) else { return false; }; + let Ok(device_scope) = crate::kernel_router::resolve_device_scope( + profile.as_ref(), + caller, + device_key_id, + "group:list", + ) else { + return false; + }; let groups = kernel.groups.load_full(); - astrid_capabilities::CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()) - .has("group:list") + let mut check = astrid_capabilities::CapabilityCheck::new( + profile.as_ref(), + groups.as_ref(), + caller.clone(), + ); + if let Some(scope) = &device_scope { + check = check.with_device_scope(scope); + } + check.has("group:list") } fn caller_group_names(kernel: &Arc, caller: &PrincipalId) -> BTreeSet { diff --git a/crates/astrid-kernel/src/kernel_router/admin/handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/handlers.rs index 0182888ee..3aa067faf 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/handlers.rs @@ -64,18 +64,12 @@ pub(super) async fn dispatch( dispatch_with_device(kernel, caller, None, req).await } -/// Dispatch carrying the issuer's authenticating device key id. -/// -/// `issuer_device_key_id` is the device key that authenticated this request, -/// when one did. Only [`AdminRequestKind::PairDeviceIssue`] consumes it: it -/// resolves the issuer's OWN device scope so the no-escalation subset check -/// and the full-mint gate run against the issuer's *attenuated* effective set -/// (a scoped device cannot mint a child broader than itself). Every other -/// handler ignores it. +/// Dispatch carrying the caller's authenticating device key id. +/// Device scope applies to every authority decision made during dispatch. pub(super) async fn dispatch_with_device( kernel: &Arc, caller: &PrincipalId, - issuer_device_key_id: Option<&str>, + device_key_id: Option<&str>, req: AdminRequestKind, ) -> AdminResponseBody { match req { @@ -87,7 +81,7 @@ pub(super) async fn dispatch_with_device( AdminRequestKind::AgentDisable { principal } => { agent_set_enabled(kernel, principal, false).await }, - AdminRequestKind::AgentList => agent_list(kernel, caller), + AdminRequestKind::AgentList => agent_list(kernel, caller, device_key_id), req @ AdminRequestKind::AgentModify { .. } => agent_modify_from_req(kernel, req).await, AdminRequestKind::QuotaSet { principal, quotas } => { super::quota::quota_set(kernel, principal, quotas).await @@ -111,7 +105,7 @@ pub(super) async fn dispatch_with_device( } => { super::group::group_modify(kernel, name, capabilities, description, unsafe_admin).await }, - AdminRequestKind::GroupList => super::group::group_list(kernel, caller), + AdminRequestKind::GroupList => super::group::group_list(kernel, caller, device_key_id), AdminRequestKind::CapsGrant { principal, capabilities, @@ -156,7 +150,7 @@ pub(super) async fn dispatch_with_device( | AdminRequestKind::PairDeviceRedeem { .. } | AdminRequestKind::PairDeviceList { .. } | AdminRequestKind::PairDeviceRevoke { .. }) => { - pair_device_dispatch(kernel, caller, issuer_device_key_id, req).await + pair_device_dispatch(kernel, caller, device_key_id, req).await }, } } @@ -652,16 +646,39 @@ where /// (segment 1 `self` ≠ `agent`), so this returns `false` for them — they /// are filtered to their own row. Fail-closed: an unresolvable caller /// profile yields `false` (most restrictive, self-only). -fn caller_has_global_agent_list(kernel: &Arc, caller: &PrincipalId) -> bool { +fn caller_has_global_agent_list( + kernel: &Arc, + caller: &PrincipalId, + device_key_id: Option<&str>, +) -> bool { let Ok(profile) = kernel.profile_cache.resolve(caller) else { return false; }; + let Ok(device_scope) = crate::kernel_router::resolve_device_scope( + profile.as_ref(), + caller, + device_key_id, + "agent:list", + ) else { + return false; + }; let groups = kernel.groups.load_full(); - astrid_capabilities::CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()) - .has("agent:list") + let mut check = astrid_capabilities::CapabilityCheck::new( + profile.as_ref(), + groups.as_ref(), + caller.clone(), + ); + if let Some(scope) = &device_scope { + check = check.with_device_scope(scope); + } + check.has("agent:list") } -fn agent_list(kernel: &Arc, caller: &PrincipalId) -> AdminResponseBody { +fn agent_list( + kernel: &Arc, + caller: &PrincipalId, + device_key_id: Option<&str>, +) -> AdminResponseBody { // Source of truth: `etc/profiles/{principal}.toml`. Iterating the // home directory was the pre-#672 approach but stopped working // when profiles moved out — and was always wrong in spirit since @@ -725,7 +742,7 @@ fn agent_list(kernel: &Arc, caller: &PrincipalId) -> AdminRespons // self-scoped form), so in practice only the `admin` group's `*` // (which matches both) sees everyone. This realises the gateway's // documented "the kernel filters server-side" contract. - if !caller_has_global_agent_list(kernel, caller) { + if !caller_has_global_agent_list(kernel, caller, device_key_id) { summaries.retain(|s| s.principal == *caller); } diff --git a/crates/astrid-kernel/src/kernel_router/admin/mod.rs b/crates/astrid-kernel/src/kernel_router/admin/mod.rs index 0044ebf1d..99a354e60 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/mod.rs @@ -602,11 +602,7 @@ async fn handle_admin_request( }, } - // Pass the issuer's authenticating device key id through so - // `pair_device_issue` can resolve the issuer's OWN device scope and - // enforce no-escalation against the issuer's *attenuated* effective set - // (a scoped issuer cannot mint a broader child). Every other handler - // ignores it. + // Secondary views and pair delegation use the authenticated device scope. let body = handlers::dispatch_with_device(kernel, &caller, device_key_id.as_deref(), req.kind).await; publish_response( diff --git a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs index 7a922c5a2..a2006083b 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs @@ -54,15 +54,12 @@ pub(crate) async fn pair_device_issue( )); } - // Resolve the requested scope arg to a concrete DeviceScope. An unknown - // preset name is a bad-input rejection, not a silent fallback. let requested_scope = match resolve_pair_scope(&requested) { Ok(s) => s, Err(e) => return err_bad_input(e), }; - // Caller's profile must already exist. A pair-token tied to a - // missing principal would be a dead grant on redeem. + // Pair tokens must name an existing principal. let profile_path = kernel.astrid_home.profile_path(caller); if !profile_path.exists() { return err_bad_input(format!( @@ -70,42 +67,28 @@ pub(crate) async fn pair_device_issue( )); } - // ── No-escalation enforcement ────────────────────────────────────── - // - // Resolve the issuer's EFFECTIVE capability set: their principal profile - // narrowed by the issuer's OWN authenticating device scope. A - // restricted-but-can-pair device must NOT be able to mint a broader child - // — "device scope ⊆ issuer effective caps" where the issuer's effective - // caps already account for their own device attenuation. - // - // The scope is cloned into a local so it outlives the borrow of `profile` - // for the `CapabilityCheck` below (cheap — a couple of small pattern - // vectors — and avoids fighting the borrow `device_by_key_id` takes). let profile = match kernel.profile_cache.resolve(caller) { Ok(p) => p, Err(e) => return err_internal(format!("issuer profile resolution failed: {e}")), }; - let issuer_scope: Option = issuer_device_key_id - .and_then(|kid| profile.auth.device_by_key_id(kid)) - .map(|dev| dev.scope.clone()); + let issuer_scope = match crate::kernel_router::resolve_device_scope( + profile.as_ref(), + caller, + issuer_device_key_id, + "self:auth:pair", + ) { + Ok(scope) => scope, + Err(e) => return err_unauthorized(e.to_string()), + }; let groups = kernel.groups.load_full(); let mut issuer_check = CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()); - // Apply the issuer's own device floor (Full / None = unattenuated, the - // common admin/agent case — behaviour unchanged there). + // A scoped issuer can grant only its attenuated authority. if let Some(scope @ DeviceScope::Scoped { .. }) = &issuer_scope { issuer_check = issuer_check.with_device_scope(scope); } match &requested_scope { - // FULL-MINT GATE: minting an unattenuated device is the broadest grant - // possible. It requires BOTH: - // 1. that the ISSUER is itself unattenuated. A device under its own - // scope cannot grant "no restrictions" to a child — a Full child - // applies no attenuation, so it would escape the issuer's own - // denies (deny-inheritance below only narrows *Scoped* children, - // not Full ones). A scoped issuer must mint a Scoped child instead. - // 2. that the issuer holds `self:auth:pair:admin` under their - // attenuated effective set. + // Full devices require an unattenuated issuer with pair-admin authority. DeviceScope::Full => { if matches!(&issuer_scope, Some(DeviceScope::Scoped { .. })) { return err_unauthorized(format!( @@ -118,10 +101,14 @@ pub(crate) async fn pair_device_issue( )); } }, - // SUBSET CHECK: every requested `allow` pattern must be held by the - // issuer's attenuated effective set (no escalation). `deny` patterns - // only restrict and need no validation. - DeviceScope::Scoped { .. } => { + DeviceScope::Scoped { allow, .. } => { + // Bare `*` confers the principal's full authority even in a scoped shape. + let requests_universal_scope = allow.iter().any(|pattern| pattern == "*"); + if requests_universal_scope && !issuer_check.has("self:auth:pair:admin") { + return err_unauthorized(format!( + "minting a device scope with universal `*` requires self:auth:pair:admin, which {caller} does not effectively hold" + )); + } if let Err(e) = device_scope_within(&issuer_check, &requested_scope) { return err_unauthorized(format!( "requested device scope exceeds your authority: {e}" @@ -130,14 +117,9 @@ pub(crate) async fn pair_device_issue( }, } - // DENY-INHERITANCE (monotonic narrowing): a Scoped child inherits the - // issuer's own device-scope denies. Without this, a broad child - // `allow:[self:*]` from an issuer that denies e.g. `self:capsule:install` - // would let the child exceed the parent on that cap. Unioning the parent's - // deny list re-blocks those caps in the child — children only ever get - // more restricted. + // A scoped child inherits its issuer's denies. let stored_scope = inherit_issuer_denies(requested_scope, issuer_scope.as_ref()); - // Drop the borrow on `profile`/`groups` before taking the write lock. + // Release authority state before awaiting the write lock. drop(issuer_check); drop(profile); drop(groups); diff --git a/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs b/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs index 1b37fa8bd..6eb5f1601 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs @@ -42,8 +42,19 @@ fn pid(name: &str) -> PrincipalId { /// Seed `principal` holding `grants`, enabled, with the supplied device keys. fn seed(kernel: &Arc, principal: &PrincipalId, grants: &[&str], devices: Vec) { + seed_policy(kernel, principal, grants, &[], devices); +} + +fn seed_policy( + kernel: &Arc, + principal: &PrincipalId, + grants: &[&str], + revokes: &[&str], + devices: Vec, +) { let mut profile = PrincipalProfile { grants: grants.iter().map(|g| (*g).to_string()).collect(), + revokes: revokes.iter().map(|r| (*r).to_string()).collect(), enabled: true, ..Default::default() }; @@ -86,6 +97,27 @@ fn issue(scope: PairScopeArg) -> AdminRequestKind { } } +async fn assert_missing_device_rejects_broad_mints( + kernel: &Arc, + caller: &PrincipalId, + device_key_id: &str, +) { + for scope in [ + PairScopeArg::Full, + PairScopeArg::Explicit { + allow: vec!["*".into()], + deny: vec![], + }, + ] { + let response = + handlers::dispatch_with_device(kernel, caller, Some(device_key_id), issue(scope)).await; + assert!( + matches!(response, AdminResponseBody::Error(_)), + "a missing issuer device must not mint broad authority: {response:?}" + ); + } +} + // ── Criterion 2: a full device (full-principal) can re-issue ────────── #[tokio::test(flavor = "multi_thread")] @@ -182,6 +214,118 @@ async fn full_scope_device_can_mint_full() { ); } +#[tokio::test(flavor = "multi_thread")] +async fn unknown_or_revoked_issuer_device_cannot_mint_broad_authority() { + let (_dir, kernel) = fixture().await; + + let unknown_caller = pid("unknown_issuer_device"); + seed(&kernel, &unknown_caller, &["*"], vec![]); + assert_missing_device_rejects_broad_mints(&kernel, &unknown_caller, "0000000000000000").await; + assert_missing_device_rejects_broad_mints(&kernel, &unknown_caller, "not-a-key-id").await; + + let revoked_caller = pid("revoked_issuer_device"); + let revoked_device = full_device('f'); + let revoked_id = revoked_device.key_id.clone(); + seed(&kernel, &revoked_caller, &["*"], vec![revoked_device]); + let response = handlers::dispatch( + &kernel, + &revoked_caller, + AdminRequestKind::PairDeviceRevoke { + principal: revoked_caller.clone(), + key_id: revoked_id.clone(), + }, + ) + .await; + assert!(matches!( + response, + AdminResponseBody::PairDeviceRevoked { .. } + )); + assert_missing_device_rejects_broad_mints(&kernel, &revoked_caller, &revoked_id).await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn universal_scoped_mint_requires_effective_admin_capability() { + let (_dir, kernel) = fixture().await; + let caller = pid("universal_scope_without_admin"); + seed_policy(&kernel, &caller, &["*"], &["self:auth:pair:admin"], vec![]); + + let req = issue(PairScopeArg::Explicit { + allow: vec!["*".into()], + deny: vec![], + }); + let resp = handlers::dispatch_with_device(&kernel, &caller, None, req).await; + match resp { + AdminResponseBody::Error(msg) => assert!( + msg.contains("universal `*`") && msg.contains("self:auth:pair:admin"), + "universal scoped denial must name both the scope and required capability: {msg}" + ), + other => panic!("universal scoped mint without pair-admin must reject, got: {other:?}"), + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn universal_scoped_mint_is_accepted_with_admin_capability() { + let (_dir, kernel) = fixture().await; + let caller = pid("universal_scope_with_admin"); + let dev = scoped_device('a', &["*"], &[]); + let dev_id = dev.key_id.clone(); + seed(&kernel, &caller, &["*"], vec![dev]); + + let req = issue(PairScopeArg::Explicit { + allow: vec!["*".into()], + deny: vec![], + }); + let resp = handlers::dispatch_with_device(&kernel, &caller, Some(&dev_id), req).await; + assert!( + matches!(resp, AdminResponseBody::PairToken(_)), + "an issuer effectively holding pair-admin may mint a universal scoped device: {resp:?}" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn scoped_issuer_cannot_bypass_admin_gate_with_universal_allow() { + let (_dir, kernel) = fixture().await; + let caller = pid("attenuated_universal_issuer"); + let dev = scoped_device('a', &["*"], &["self:auth:pair:admin"]); + let dev_id = dev.key_id.clone(); + seed(&kernel, &caller, &["*"], vec![dev]); + + let req = issue(PairScopeArg::Explicit { + allow: vec!["*".into()], + deny: vec![], + }); + let resp = handlers::dispatch_with_device(&kernel, &caller, Some(&dev_id), req).await; + match resp { + AdminResponseBody::Error(msg) => assert!( + msg.contains("self:auth:pair:admin"), + "attenuated issuer denial must name the missing effective capability: {msg}" + ), + other => panic!("attenuated scoped issuer must not mint universal scope, got: {other:?}"), + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn use_only_scope_does_not_require_admin_capability() { + let (_dir, kernel) = fixture().await; + let caller = pid("self_scope_without_admin"); + seed_policy( + &kernel, + &caller, + &["self:*"], + &["self:auth:pair:admin"], + vec![], + ); + + let req = issue(PairScopeArg::Preset { + name: "use-only".into(), + }); + let resp = handlers::dispatch_with_device(&kernel, &caller, None, req).await; + assert!( + matches!(resp, AdminResponseBody::PairToken(_)), + "self:* must remain an ordinary subset-checked scoped grant: {resp:?}" + ); +} + // ── Criterion 3: over-broad requested scope rejected at issue time ──── #[tokio::test(flavor = "multi_thread")] diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs index 611e9a2a9..005194671 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs @@ -15,7 +15,7 @@ use std::sync::Arc; use astrid_core::dirs::AstridHome; use astrid_core::groups::{BUILTIN_ADMIN, BUILTIN_AGENT, BUILTIN_RESTRICTED, GroupConfig}; use astrid_core::principal::PrincipalId; -use astrid_core::profile::{PrincipalProfile, Quotas}; +use astrid_core::profile::{AuthMethod, DeviceKey, DeviceScope, PrincipalProfile, Quotas}; use astrid_events::kernel_api::{AdminRequestKind, AdminResponseBody, AgentSummary, GroupSummary}; use tempfile::TempDir; @@ -996,3 +996,103 @@ async fn agent_list_filters_to_self_for_non_admin_caller() { ); assert_eq!(mine[0].principal.as_str(), "alice"); } + +#[tokio::test(flavor = "multi_thread")] +async fn agent_list_global_view_is_attenuated_by_device_scope() { + let (_dir, kernel) = fixture().await; + for name in ["alice", "bob"] { + let res = handlers::dispatch( + &kernel, + &PrincipalId::default(), + AdminRequestKind::AgentCreate { + name: name.into(), + groups: Vec::new(), + grants: Vec::new(), + inherit_from: None, + clone_from: None, + allow_admin_clone: false, + }, + ) + .await; + assert_success(&res); + } + + let full = DeviceKey::new("a".repeat(64), DeviceScope::Full, None, 0); + let self_only = DeviceKey::new( + "b".repeat(64), + DeviceScope::Scoped { + allow: vec!["self:*".to_string()], + deny: Vec::new(), + }, + None, + 0, + ); + let global_list = DeviceKey::new( + "c".repeat(64), + DeviceScope::Scoped { + allow: vec!["self:*".to_string(), "agent:list".to_string()], + deny: Vec::new(), + }, + None, + 0, + ); + let full_id = full.key_id.clone(); + let self_only_id = self_only.key_id.clone(); + let global_list_id = global_list.key_id.clone(); + let mut profile = PrincipalProfile::load_from_path(&PrincipalProfile::path_for( + &kernel.astrid_home, + &PrincipalId::default(), + )) + .expect("load default profile"); + profile.auth.methods = vec![AuthMethod::Keypair]; + profile.auth.public_keys = vec![full, self_only, global_list]; + profile + .save_to_path(&PrincipalProfile::path_for( + &kernel.astrid_home, + &PrincipalId::default(), + )) + .expect("save device-scoped default profile"); + kernel.profile_cache.invalidate(&PrincipalId::default()); + + let list_for = |response| match response { + AdminResponseBody::AgentList(list) => list, + other => panic!("expected AgentList, got {other:?}"), + }; + let full = list_for( + handlers::dispatch_with_device( + &kernel, + &PrincipalId::default(), + Some(&full_id), + AdminRequestKind::AgentList, + ) + .await, + ); + assert!(full.iter().any(|entry| entry.principal == pid("alice"))); + assert!(full.iter().any(|entry| entry.principal == pid("bob"))); + + let global = list_for( + handlers::dispatch_with_device( + &kernel, + &PrincipalId::default(), + Some(&global_list_id), + AdminRequestKind::AgentList, + ) + .await, + ); + assert!(global.iter().any(|entry| entry.principal == pid("alice"))); + assert!(global.iter().any(|entry| entry.principal == pid("bob"))); + + for device_key_id in [self_only_id.as_str(), "0000000000000000"] { + let scoped = list_for( + handlers::dispatch_with_device( + &kernel, + &PrincipalId::default(), + Some(device_key_id), + AdminRequestKind::AgentList, + ) + .await, + ); + assert_eq!(scoped.len(), 1); + assert_eq!(scoped[0].principal, PrincipalId::default()); + } +} diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs index f7656d447..da1710bd6 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs @@ -6,7 +6,7 @@ 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_core::profile::{AuthMethod, DeviceKey, DeviceScope, PrincipalProfile}; use astrid_events::kernel_api::{AdminRequestKind, AdminResponseBody}; use tempfile::TempDir; @@ -66,3 +66,92 @@ async fn group_list_filters_to_callers_profile_groups_without_global_cap() { let names: Vec<_> = list.iter().map(|group| group.name.as_str()).collect(); assert_eq!(names, vec![BUILTIN_AGENT]); } + +#[tokio::test(flavor = "multi_thread")] +async fn group_list_global_view_is_attenuated_by_device_scope() { + let (_dir, kernel) = fixture().await; + handlers::dispatch( + &kernel, + &PrincipalId::default(), + AdminRequestKind::GroupCreate { + name: "ops".into(), + capabilities: vec!["capsule:install".into()], + description: None, + unsafe_admin: false, + }, + ) + .await; + + let full = DeviceKey::new("a".repeat(64), DeviceScope::Full, None, 0); + let self_only = DeviceKey::new( + "b".repeat(64), + DeviceScope::Scoped { + allow: vec!["self:*".to_string()], + deny: Vec::new(), + }, + None, + 0, + ); + let global_list = DeviceKey::new( + "c".repeat(64), + DeviceScope::Scoped { + allow: vec!["self:*".to_string(), "group:list".to_string()], + deny: Vec::new(), + }, + None, + 0, + ); + let full_id = full.key_id.clone(); + let self_only_id = self_only.key_id.clone(); + let global_list_id = global_list.key_id.clone(); + let profile_path = PrincipalProfile::path_for(&kernel.astrid_home, &PrincipalId::default()); + let mut profile = + PrincipalProfile::load_from_path(&profile_path).expect("load default profile"); + profile.auth.methods = vec![AuthMethod::Keypair]; + profile.auth.public_keys = vec![full, self_only, global_list]; + profile + .save_to_path(&profile_path) + .expect("save device-scoped default profile"); + kernel.profile_cache.invalidate(&PrincipalId::default()); + + let names_for = |response| match response { + AdminResponseBody::GroupList(list) => { + list.into_iter().map(|group| group.name).collect::>() + }, + other => panic!("expected GroupList, got {other:?}"), + }; + let full = names_for( + handlers::dispatch_with_device( + &kernel, + &PrincipalId::default(), + Some(&full_id), + AdminRequestKind::GroupList, + ) + .await, + ); + assert!(full.iter().any(|name| name == "ops")); + + let global = names_for( + handlers::dispatch_with_device( + &kernel, + &PrincipalId::default(), + Some(&global_list_id), + AdminRequestKind::GroupList, + ) + .await, + ); + assert!(global.iter().any(|name| name == "ops")); + + for device_key_id in [self_only_id.as_str(), "0000000000000000"] { + let scoped = names_for( + handlers::dispatch_with_device( + &kernel, + &PrincipalId::default(), + Some(device_key_id), + AdminRequestKind::GroupList, + ) + .await, + ); + assert_eq!(scoped, vec![BUILTIN_ADMIN]); + } +} diff --git a/crates/astrid-kernel/src/kernel_router/device_scope.rs b/crates/astrid-kernel/src/kernel_router/device_scope.rs new file mode 100644 index 000000000..2b1e0b95e --- /dev/null +++ b/crates/astrid-kernel/src/kernel_router/device_scope.rs @@ -0,0 +1,52 @@ +//! Shared authenticating-device scope resolution for kernel authority checks. + +use astrid_capabilities::PermissionError; +use astrid_core::principal::PrincipalId; +use astrid_core::profile::{DeviceKeyId, DeviceScope, PrincipalProfile}; +use tracing::warn; + +/// Resolve the authenticating device's attenuation floor. +/// +/// A supplied id must be canonical and registered to `caller`; an invalid, +/// unknown, or revoked id never falls back to full-principal authority. +pub(super) fn resolve_device_scope( + profile: &PrincipalProfile, + caller: &PrincipalId, + device_key_id: Option<&str>, + required_cap: &str, +) -> Result, PermissionError> { + let Some(raw_key_id) = device_key_id else { + return Ok(None); + }; + let key_id = match DeviceKeyId::new(raw_key_id) { + Ok(key_id) => key_id, + Err(reason) => { + warn!( + security_event = true, + principal = %caller, + key_id = %raw_key_id, + required = required_cap, + error = %reason, + "device_key_id is invalid — fail-closed deny" + ); + return Err(PermissionError::DeviceScopeDenied { + principal: caller.clone(), + required: required_cap.to_string(), + }); + }, + }; + let Some(device) = profile.auth.device_by_typed_key_id(&key_id) else { + warn!( + security_event = true, + principal = %caller, + key_id = %raw_key_id, + required = required_cap, + "device_key_id resolves to no registered key — fail-closed deny" + ); + return Err(PermissionError::DeviceScopeDenied { + principal: caller.clone(), + required: required_cap.to_string(), + }); + }; + Ok(Some(device.scope.clone())) +} diff --git a/crates/astrid-kernel/src/kernel_router/mod.rs b/crates/astrid-kernel/src/kernel_router/mod.rs index a449fde34..2d9f13f89 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 device_scope; /// `KernelRequest::InstallCapsule` handler — delegates to the /// `astrid-capsule-install` library so the daemon and the CLI reach /// disk through the same code path. @@ -23,6 +24,8 @@ use astrid_events::ipc::{IpcMessage, IpcPayload, Topic}; use astrid_events::kernel_api::{KernelRequest, KernelResponse}; use tracing::{debug, info, warn}; +use device_scope::resolve_device_scope; + #[cfg(test)] mod capability_catalog_tests; #[cfg(test)] @@ -298,7 +301,7 @@ async fn handle_request( KernelResponse::Error("Approval logic not yet implemented in kernel router".to_string()) }, KernelRequest::ListCapsules => { - let visibility = CapsuleVisibility::new(kernel, &caller); + let visibility = CapsuleVisibility::new(kernel, &caller, device_key_id.as_deref()); let list: Vec<_> = visible_inventory_manifests(kernel, &visibility) .await .into_iter() @@ -307,7 +310,7 @@ async fn handle_request( KernelResponse::Success(serde_json::json!(list)) }, KernelRequest::GetCommands => { - let visibility = CapsuleVisibility::new(kernel, &caller); + let visibility = CapsuleVisibility::new(kernel, &caller, device_key_id.as_deref()); let mut commands = Vec::new(); let manifests = visible_inventory_manifests(kernel, &visibility).await; for manifest in &manifests { @@ -432,7 +435,7 @@ async fn handle_request( KernelResponse::Status(status) }, KernelRequest::GetCapsuleMetadata => { - let visibility = CapsuleVisibility::new(kernel, &caller); + let visibility = CapsuleVisibility::new(kernel, &caller, device_key_id.as_deref()); let mut entries = Vec::new(); for manifest in visible_inventory_manifests(kernel, &visibility).await { entries.push(astrid_events::kernel_api::CapsuleMetadataEntry { @@ -448,7 +451,7 @@ async fn handle_request( KernelResponse::CapsuleMetadata(entries) }, KernelRequest::GetAgentReadiness => { - let visibility = CapsuleVisibility::new(kernel, &caller); + let visibility = CapsuleVisibility::new(kernel, &caller, device_key_id.as_deref()); let manifests = visible_inventory_manifests(kernel, &visibility).await; let readiness = astrid_capsule::readiness::agent_loop_readiness(&manifests); KernelResponse::AgentReadiness(readiness) @@ -560,7 +563,7 @@ struct CapsuleVisibility { } impl CapsuleVisibility { - fn new(kernel: &crate::Kernel, caller: &PrincipalId) -> Self { + fn new(kernel: &crate::Kernel, caller: &PrincipalId, device_key_id: Option<&str>) -> Self { let profile = if caller.as_str() == "anonymous" { None } else { @@ -578,23 +581,35 @@ impl CapsuleVisibility { } }; let Some(profile) = profile else { - return Self { - principal: caller.clone(), - is_admin: false, - capsule_grants: BTreeSet::new(), - }; + return Self::denied(caller); }; + let device_scope = + match resolve_device_scope(profile.as_ref(), caller, device_key_id, "capsule:list") { + Ok(scope) => scope, + Err(_) => return Self::denied(caller), + }; let groups = kernel.groups.load_full(); - let check = CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()); + let mut check = CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()); + if let Some(scope) = &device_scope { + check = check.with_device_scope(scope); + } Self { principal: caller.clone(), - is_admin: check.has("*") || check.has("capsule:list"), + is_admin: check.has("capsule:list"), capsule_grants: profile.capsules.iter().cloned().collect(), } } + fn denied(caller: &PrincipalId) -> Self { + Self { + principal: caller.clone(), + is_admin: false, + capsule_grants: BTreeSet::new(), + } + } + fn allows(&self, capsule_id: &astrid_capsule::capsule::CapsuleId) -> bool { self.is_admin || self.capsule_grants.contains(capsule_id.as_str()) } @@ -826,36 +841,7 @@ fn authorize_request( } let groups = kernel.groups.load_full(); - // Per-device scope attenuation. When the request authenticated with a - // specific registered device, the device's scope is applied as a floor on - // the principal's effective capabilities (deny wins, can only narrow). - // - // Fail-closed on an unresolved key_id: a request that names a device the - // principal no longer has (revoked / unknown) must NOT fall back to the - // principal's full authority — that would let a revoked device keep acting. - // - // The scope is cloned into a local so it outlives the borrow of `profile` - // for the `require` call below (a `DeviceScope` clone is cheap — at most a - // couple of small pattern vectors — and avoids fighting the borrow that - // `device_by_key_id` takes on `profile`). - let device_scope: Option = if let Some(kid) = device_key_id { - let Some(dev) = profile.auth.device_by_key_id(kid) else { - warn!( - security_event = true, - principal = %caller, - key_id = %kid, - required = required_cap, - "device_key_id resolves to no registered key — fail-closed deny" - ); - return Err(PermissionError::DeviceScopeDenied { - principal: caller.clone(), - required: required_cap.to_string(), - }); - }; - Some(dev.scope.clone()) - } else { - None - }; + let device_scope = resolve_device_scope(profile.as_ref(), caller, device_key_id, required_cap)?; let mut check = CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()); if let Some(scope) = &device_scope { diff --git a/crates/astrid-kernel/src/kernel_router/tests.rs b/crates/astrid-kernel/src/kernel_router/tests.rs index 24571bd89..18c5a2933 100644 --- a/crates/astrid-kernel/src/kernel_router/tests.rs +++ b/crates/astrid-kernel/src/kernel_router/tests.rs @@ -9,7 +9,7 @@ use astrid_capsule::error::CapsuleResult; use astrid_capsule::manifest::{CapsuleManifest, CommandDef, PackageDef, SubscribeDef}; use astrid_capsule::registry::WasmHash; use astrid_core::kernel_api::CommandKind; -use astrid_core::profile::PrincipalProfile; +use astrid_core::profile::{AuthMethod, DeviceKey, DeviceScope, PrincipalProfile}; use std::sync::atomic::AtomicBool; struct InventoryCapsule { @@ -658,9 +658,9 @@ async fn capsule_visibility_precomputes_admin_and_capsule_grants() { let allowed = CapsuleId::new("allowed").expect("valid capsule id"); let default_only = CapsuleId::new("default-only").expect("valid capsule id"); - let admin_visibility = CapsuleVisibility::new(&kernel, &admin); - let global_lister_visibility = CapsuleVisibility::new(&kernel, &global_lister); - let limited_visibility = CapsuleVisibility::new(&kernel, &limited); + let admin_visibility = CapsuleVisibility::new(&kernel, &admin, None); + let global_lister_visibility = CapsuleVisibility::new(&kernel, &global_lister, None); + let limited_visibility = CapsuleVisibility::new(&kernel, &limited, None); assert!(admin_visibility.allows(&allowed)); assert!(admin_visibility.allows(&default_only)); @@ -670,6 +670,131 @@ async fn capsule_visibility_precomputes_admin_and_capsule_grants() { assert!(!limited_visibility.allows(&default_only)); } +#[tokio::test(flavor = "multi_thread")] +async fn device_scope_attenuates_every_capsule_inventory_surface() { + let (_dir, kernel) = kernel_with_inventory_capsules().await; + let caller = PrincipalId::new("device-scoped-admin").expect("valid principal"); + seed_capsule_inventory_profile(&kernel, &caller, &["allowed"]).await; + + let full = DeviceKey::new("a".repeat(64), DeviceScope::Full, None, 0); + let self_only = DeviceKey::new( + "b".repeat(64), + DeviceScope::Scoped { + allow: vec!["self:*".to_string()], + deny: Vec::new(), + }, + None, + 0, + ); + let global_list = DeviceKey::new( + "c".repeat(64), + DeviceScope::Scoped { + allow: vec!["self:*".to_string(), "capsule:list".to_string()], + deny: Vec::new(), + }, + None, + 0, + ); + let denied_global_list = DeviceKey::new( + "d".repeat(64), + DeviceScope::Scoped { + allow: vec!["*".to_string()], + deny: vec!["capsule:list".to_string()], + }, + None, + 0, + ); + let full_id = full.key_id.clone(); + let self_only_id = self_only.key_id.clone(); + let global_list_id = global_list.key_id.clone(); + let denied_global_list_id = denied_global_list.key_id.clone(); + let mut profile = PrincipalProfile { + grants: vec!["*".to_string()], + capsules: vec!["allowed".to_string()], + ..Default::default() + }; + profile.auth.methods.push(AuthMethod::Keypair); + profile.auth.public_keys = vec![full, self_only, global_list, denied_global_list]; + seed_profile(&kernel, &caller, &profile); + + let global_capsules = &["allowed", "default-only"]; + let global_commands = &["allowed-cmd", "default-only-cmd"]; + let scoped_capsules = &["allowed"]; + let scoped_commands = &["allowed-cmd"]; + + assert_capsule_inventory_surface_for_device( + &kernel, + &caller, + None, + "unattenuated_admin", + global_capsules, + global_commands, + global_capsules, + global_capsules, + ) + .await; + assert_capsule_inventory_surface_for_device( + &kernel, + &caller, + Some(&full_id), + "full_device_admin", + global_capsules, + global_commands, + global_capsules, + global_capsules, + ) + .await; + assert_capsule_inventory_surface_for_device( + &kernel, + &caller, + Some(&self_only_id), + "self_only_device_admin", + scoped_capsules, + scoped_commands, + scoped_capsules, + scoped_capsules, + ) + .await; + assert_capsule_inventory_surface_for_device( + &kernel, + &caller, + Some(&denied_global_list_id), + "global_list_denied_device_admin", + scoped_capsules, + scoped_commands, + scoped_capsules, + scoped_capsules, + ) + .await; + assert_capsule_inventory_surface_for_device( + &kernel, + &caller, + Some(&global_list_id), + "global_list_device_admin", + global_capsules, + global_commands, + global_capsules, + global_capsules, + ) + .await; + + let allowed = CapsuleId::new("allowed").expect("valid capsule id"); + let unknown = CapsuleVisibility::new(&kernel, &caller, Some("0000000000000000")); + let malformed = CapsuleVisibility::new(&kernel, &caller, Some("not-a-key-id")); + assert!(!unknown.allows(&allowed)); + assert!(!malformed.allows(&allowed)); + + let response = request_kernel_for_device( + &kernel, + &caller, + Some("0000000000000000"), + "unknown_device_inventory", + KernelRequest::ListCapsules, + ) + .await; + assert!(matches!(response, KernelResponse::Error(_))); +} + async fn kernel_with_inventory_capsules() -> (tempfile::TempDir, Arc) { let dir = tempfile::tempdir().expect("tempdir"); let home = astrid_core::dirs::AstridHome::from_path(dir.path()); @@ -776,9 +901,34 @@ async fn assert_capsule_inventory_surface( expected_metadata: &[&str], expected_readiness: &[&str], ) { - let list = request_kernel( + assert_capsule_inventory_surface_for_device( + kernel, + caller, + None, + label, + expected_capsules, + expected_commands, + expected_metadata, + expected_readiness, + ) + .await; +} + +#[allow(clippy::too_many_arguments)] +async fn assert_capsule_inventory_surface_for_device( + kernel: &Arc, + caller: &PrincipalId, + device_key_id: Option<&str>, + label: &str, + expected_capsules: &[&str], + expected_commands: &[&str], + expected_metadata: &[&str], + expected_readiness: &[&str], +) { + let list = request_kernel_for_device( kernel, caller, + device_key_id, &format!("{label}_list_capsules"), KernelRequest::ListCapsules, ) @@ -789,9 +939,10 @@ async fn assert_capsule_inventory_surface( let capsules: Vec = serde_json::from_value(value).expect("capsule list shape"); assert_eq!(capsules, expected_capsules); - let commands = request_kernel( + let commands = request_kernel_for_device( kernel, caller, + device_key_id, &format!("{label}_commands"), KernelRequest::GetCommands, ) @@ -802,9 +953,10 @@ async fn assert_capsule_inventory_surface( let command_names: Vec<_> = commands.iter().map(|cmd| cmd.name.as_str()).collect(); assert_eq!(command_names, expected_commands); - let metadata = request_kernel( + let metadata = request_kernel_for_device( kernel, caller, + device_key_id, &format!("{label}_metadata"), KernelRequest::GetCapsuleMetadata, ) @@ -815,9 +967,10 @@ async fn assert_capsule_inventory_surface( let metadata_names: Vec<_> = metadata.iter().map(|entry| entry.name.as_str()).collect(); assert_eq!(metadata_names, expected_metadata); - let readiness = request_kernel( + let readiness = request_kernel_for_device( kernel, caller, + device_key_id, &format!("{label}_readiness"), KernelRequest::GetAgentReadiness, ) @@ -833,6 +986,16 @@ async fn request_kernel( caller: &PrincipalId, suffix: &str, request: KernelRequest, +) -> KernelResponse { + request_kernel_for_device(kernel, caller, None, suffix, request).await +} + +async fn request_kernel_for_device( + kernel: &Arc, + caller: &PrincipalId, + device_key_id: Option<&str>, + suffix: &str, + request: KernelRequest, ) -> KernelResponse { let request_topic = Topic::kernel_request(format!("{suffix}.{}", uuid::Uuid::new_v4())); let response_topic = response_topic_for(request_topic.as_str()); @@ -844,6 +1007,7 @@ async fn request_kernel( kernel.session_id.0, ); msg.principal = Some(caller.as_str().to_string()); + msg.device_key_id = device_key_id.map(str::to_owned); let _ = kernel.event_bus.publish(astrid_events::AstridEvent::Ipc { metadata: astrid_events::EventMetadata::new("test"), message: msg, From 09139a2374b3f980c718fb50d407682ab4655343 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 14 Jul 2026 20:56:08 +0400 Subject: [PATCH 02/16] fix(core): synchronize profile cache invalidation --- CHANGELOG.md | 5 +- crates/astrid-capsule/src/profile_cache.rs | 101 ++++++++++++++++----- 2 files changed, 81 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e6de54e6..31e4ce7dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,8 +109,9 @@ Changelog tracking starts with 0.2.0. Prior versions were not tracked. - **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 - authority. Malformed, unknown, and revoked device IDs fail closed. Closes - #1237. + authority. Malformed, unknown, and revoked device IDs fail closed, and cache + invalidation prevents stale pre-revocation profiles from being republished. + Closes #1237. - Bumped the locked `crossbeam-epoch` transitive dependency to `0.9.20`, clearing `RUSTSEC-2026-0204` (published 2026-07-06 — an invalid pointer dereference in the `fmt::Pointer` / pre-0.9 `fmt::Display` impls for `Atomic` and `Shared` when the underlying pointer is null or invalid) from the RustSec audit. The advisory landed the day after the 0.9.3 release and turned the Security Audit CI check red on `main` and every open PR; nothing in our own code changed. Lockfile-only, same-minor patch — no other dependency moved. Closes #1161. ## [0.9.3] - 2026-07-06 diff --git a/crates/astrid-capsule/src/profile_cache.rs b/crates/astrid-capsule/src/profile_cache.rs index a22e81b7e..7278fb2e0 100644 --- a/crates/astrid-capsule/src/profile_cache.rs +++ b/crates/astrid-capsule/src/profile_cache.rs @@ -23,6 +23,7 @@ //! defaults or the capsule owner's limits. use std::collections::HashMap; +use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, RwLock}; use astrid_core::dirs::AstridHome; @@ -46,6 +47,7 @@ pub struct PrincipalProfileCache { /// one-shot home resolution at boot. astrid_home: AstridHome, cache: RwLock>>, + generation: AtomicU64, } impl PrincipalProfileCache { @@ -76,6 +78,7 @@ impl PrincipalProfileCache { Self { astrid_home, cache: RwLock::new(HashMap::new()), + generation: AtomicU64::new(0), } } @@ -98,41 +101,52 @@ impl PrincipalProfileCache { /// The caller is expected to deny the invocation on any of these errors /// (see Layer 3 design doc, issue #666). pub fn resolve(&self, principal: &PrincipalId) -> ProfileResult> { - // Fast path: a concurrent reader should never take the write lock. - // RwLock poisoning can happen only if a writer panicked mid-insert; - // recover and continue — the map is a simple key → Arc mapping - // with no partial-write window. - if let Some(profile) = self - .cache - .read() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .get(principal) - { - return Ok(Arc::clone(profile)); + loop { + if let Some(profile) = self + .cache + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .get(principal) + { + return Ok(Arc::clone(profile)); + } + + let generation = self.generation.load(Ordering::Acquire); + let profile = Arc::new(PrincipalProfile::load(&self.astrid_home, principal)?); + if let Some(profile) = self.publish_loaded(principal, profile, generation) { + return Ok(profile); + } } + } - let profile = Arc::new(PrincipalProfile::load(&self.astrid_home, principal)?); - - let mut w = self + fn publish_loaded( + &self, + principal: &PrincipalId, + profile: Arc, + generation: u64, + ) -> Option> { + let mut cache = self .cache .write() .unwrap_or_else(std::sync::PoisonError::into_inner); - // Two threads may race to resolve the same principal; the first - // writer wins and the second returns the already-inserted value. - let entry = w.entry(principal.clone()).or_insert(profile); - Ok(Arc::clone(entry)) + if self.generation.load(Ordering::Acquire) != generation { + return None; + } + let entry = cache.entry(principal.clone()).or_insert(profile); + Some(Arc::clone(entry)) } /// Drop the cached entry for `principal`, forcing a reload on the next /// [`resolve`](Self::resolve) call. /// - /// Reserved for Layer 6 management IPC (`astrid.v1.admin.quota.set`). - /// Unused today — the invalidation model is kernel restart. + /// A concurrent pre-invalidation load cannot repopulate the cache. pub fn invalidate(&self, principal: &PrincipalId) { - self.cache + let mut cache = self + .cache .write() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .remove(principal); + .unwrap_or_else(std::sync::PoisonError::into_inner); + cache.remove(principal); + self.generation.fetch_add(1, Ordering::Release); } /// Persist an operator-consented local-egress endpoint to `principal`'s @@ -201,6 +215,7 @@ impl PrincipalProfileCache { // cache entry so a reload reflects on-disk state, then return // success. guard.remove(principal); + self.generation.fetch_add(1, Ordering::Release); return Ok(()); } @@ -209,6 +224,7 @@ impl PrincipalProfileCache { // profile never reaches disk. profile.save_to_path(&path)?; guard.remove(principal); + self.generation.fetch_add(1, Ordering::Release); Ok(()) } @@ -384,6 +400,38 @@ mod tests { assert_eq!(second.quotas.max_memory_bytes, 8_388_608); } + #[test] + fn invalidation_prevents_stale_load_publication() { + let (dir, cache) = fixture(); + let p = principal("generation-race"); + write_profile( + &dir, + &p, + &format!( + "profile_version = {CURRENT_PROFILE_VERSION}\n\ + enabled = true\n" + ), + ); + let generation = cache.generation.load(Ordering::Acquire); + let stale = Arc::new( + PrincipalProfile::load(&cache.astrid_home, &p).expect("load pre-invalidation profile"), + ); + + write_profile( + &dir, + &p, + &format!( + "profile_version = {CURRENT_PROFILE_VERSION}\n\ + enabled = false\n" + ), + ); + cache.invalidate(&p); + + assert!(cache.publish_loaded(&p, stale, generation).is_none()); + assert_eq!(cache.len(), 0); + assert!(!cache.resolve(&p).expect("resolve current profile").enabled); + } + #[test] fn concurrent_readers_do_not_race() { // Lightweight contention check — not a loom model, just a sanity @@ -488,15 +536,22 @@ mod tests { fn persist_egress_is_idempotent() { let (_dir, cache) = fixture(); let p = principal("bob"); + let initial_generation = cache.generation.load(Ordering::Acquire); // No file on disk → starts from default (empty capsule_egress). cache .persist_egress(&p, "react", "10.0.0.5:8080") .expect("first persist"); + let persisted_generation = cache.generation.load(Ordering::Acquire); + assert_eq!(persisted_generation, initial_generation + 1); // A second persist of the SAME endpoint under the SAME capsule // (case-insensitive) is a no-op success, not a duplicate. cache .persist_egress(&p, "react", "10.0.0.5:8080") .expect("idempotent persist"); + assert_eq!( + cache.generation.load(Ordering::Acquire), + persisted_generation + 1 + ); let profile = cache.resolve(&p).expect("reload"); assert_eq!( From ca7de9166bcdcfc937f18c2669062f852f61a777 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 14 Jul 2026 21:18:36 +0400 Subject: [PATCH 03/16] test(core): keep handler scope cases reachable --- .../src/kernel_router/admin/state_tests.rs | 24 +++++++++---------- .../kernel_router/admin/state_tests_group.rs | 22 ++++++++--------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs index 005194671..d66528ad7 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs @@ -1082,17 +1082,15 @@ async fn agent_list_global_view_is_attenuated_by_device_scope() { assert!(global.iter().any(|entry| entry.principal == pid("alice"))); assert!(global.iter().any(|entry| entry.principal == pid("bob"))); - for device_key_id in [self_only_id.as_str(), "0000000000000000"] { - let scoped = list_for( - handlers::dispatch_with_device( - &kernel, - &PrincipalId::default(), - Some(device_key_id), - AdminRequestKind::AgentList, - ) - .await, - ); - assert_eq!(scoped.len(), 1); - assert_eq!(scoped[0].principal, PrincipalId::default()); - } + let scoped = list_for( + handlers::dispatch_with_device( + &kernel, + &PrincipalId::default(), + Some(&self_only_id), + AdminRequestKind::AgentList, + ) + .await, + ); + assert_eq!(scoped.len(), 1); + assert_eq!(scoped[0].principal, PrincipalId::default()); } diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs index da1710bd6..f844bbbcd 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs @@ -142,16 +142,14 @@ async fn group_list_global_view_is_attenuated_by_device_scope() { ); assert!(global.iter().any(|name| name == "ops")); - for device_key_id in [self_only_id.as_str(), "0000000000000000"] { - let scoped = names_for( - handlers::dispatch_with_device( - &kernel, - &PrincipalId::default(), - Some(device_key_id), - AdminRequestKind::GroupList, - ) - .await, - ); - assert_eq!(scoped, vec![BUILTIN_ADMIN]); - } + let scoped = names_for( + handlers::dispatch_with_device( + &kernel, + &PrincipalId::default(), + Some(&self_only_id), + AdminRequestKind::GroupList, + ) + .await, + ); + assert_eq!(scoped, vec![BUILTIN_ADMIN]); } From ba3747a2caa63ac960533212c07aacacfbe0c32c Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 14 Jul 2026 21:34:38 +0400 Subject: [PATCH 04/16] fix(core): isolate profile cache generations --- crates/astrid-capsule/src/profile_cache.rs | 90 +++++++++++++++------- 1 file changed, 62 insertions(+), 28 deletions(-) diff --git a/crates/astrid-capsule/src/profile_cache.rs b/crates/astrid-capsule/src/profile_cache.rs index 7278fb2e0..ac86b2ae3 100644 --- a/crates/astrid-capsule/src/profile_cache.rs +++ b/crates/astrid-capsule/src/profile_cache.rs @@ -23,7 +23,6 @@ //! defaults or the capsule owner's limits. use std::collections::HashMap; -use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::{Arc, RwLock}; use astrid_core::dirs::AstridHome; @@ -35,7 +34,8 @@ use astrid_core::profile::{PrincipalProfile, ProfileError, ProfileResult}; /// One instance is created per kernel boot and shared (via `Arc`) through /// the capsule load context into every [`WasmEngine`](crate::engine::wasm::WasmEngine). /// Reads vastly outnumber writes (entries are populated on first use and -/// never mutated afterward), so the inner map sits behind a `RwLock`. +/// never mutated afterward), so profiles and their per-principal invalidation +/// generations share one `RwLock`. #[derive(Debug)] pub struct PrincipalProfileCache { /// Root against which principal profile paths are resolved. @@ -46,8 +46,21 @@ pub struct PrincipalProfileCache { /// [`AstridHome::resolve`] once — matching the rest of the kernel's /// one-shot home resolution at boot. astrid_home: AstridHome, - cache: RwLock>>, - generation: AtomicU64, + state: RwLock, +} + +#[derive(Debug, Default)] +struct ProfileCacheState { + profiles: HashMap>, + generations: HashMap, +} + +impl ProfileCacheState { + fn invalidate(&mut self, principal: &PrincipalId) { + self.profiles.remove(principal); + let generation = self.generations.entry(principal.clone()).or_default(); + *generation = generation.wrapping_add(1); + } } impl PrincipalProfileCache { @@ -77,8 +90,7 @@ impl PrincipalProfileCache { pub fn with_home(astrid_home: AstridHome) -> Self { Self { astrid_home, - cache: RwLock::new(HashMap::new()), - generation: AtomicU64::new(0), + state: RwLock::new(ProfileCacheState::default()), } } @@ -102,16 +114,15 @@ impl PrincipalProfileCache { /// (see Layer 3 design doc, issue #666). pub fn resolve(&self, principal: &PrincipalId) -> ProfileResult> { loop { - if let Some(profile) = self - .cache + let state = self + .state .read() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .get(principal) - { + .unwrap_or_else(std::sync::PoisonError::into_inner); + if let Some(profile) = state.profiles.get(principal) { return Ok(Arc::clone(profile)); } - - let generation = self.generation.load(Ordering::Acquire); + let generation = state.generations.get(principal).copied().unwrap_or(0); + drop(state); let profile = Arc::new(PrincipalProfile::load(&self.astrid_home, principal)?); if let Some(profile) = self.publish_loaded(principal, profile, generation) { return Ok(profile); @@ -125,14 +136,14 @@ impl PrincipalProfileCache { profile: Arc, generation: u64, ) -> Option> { - let mut cache = self - .cache + let mut state = self + .state .write() .unwrap_or_else(std::sync::PoisonError::into_inner); - if self.generation.load(Ordering::Acquire) != generation { + if state.generations.get(principal).copied().unwrap_or(0) != generation { return None; } - let entry = cache.entry(principal.clone()).or_insert(profile); + let entry = state.profiles.entry(principal.clone()).or_insert(profile); Some(Arc::clone(entry)) } @@ -141,12 +152,11 @@ impl PrincipalProfileCache { /// /// A concurrent pre-invalidation load cannot repopulate the cache. pub fn invalidate(&self, principal: &PrincipalId) { - let mut cache = self - .cache + let mut state = self + .state .write() .unwrap_or_else(std::sync::PoisonError::into_inner); - cache.remove(principal); - self.generation.fetch_add(1, Ordering::Release); + state.invalidate(principal); } /// Persist an operator-consented local-egress endpoint to `principal`'s @@ -197,7 +207,7 @@ impl PrincipalProfileCache { // the disk write, or snapshot-then-swap) only if this becomes a // measurable `resolve()` latency source. let mut guard = self - .cache + .state .write() .unwrap_or_else(std::sync::PoisonError::into_inner); @@ -214,8 +224,7 @@ impl PrincipalProfileCache { // Already persisted for this capsule — idempotent. Drop the stale // cache entry so a reload reflects on-disk state, then return // success. - guard.remove(principal); - self.generation.fetch_add(1, Ordering::Release); + guard.invalidate(principal); return Ok(()); } @@ -223,8 +232,7 @@ impl PrincipalProfileCache { // `save_to_path` re-runs `validate()` before writing, so a malformed // profile never reaches disk. profile.save_to_path(&path)?; - guard.remove(principal); - self.generation.fetch_add(1, Ordering::Release); + guard.invalidate(principal); Ok(()) } @@ -232,9 +240,10 @@ impl PrincipalProfileCache { #[cfg(test)] #[must_use] pub(crate) fn len(&self) -> usize { - self.cache + self.state .read() .unwrap_or_else(std::sync::PoisonError::into_inner) + .profiles .len() } } @@ -412,7 +421,14 @@ mod tests { enabled = true\n" ), ); - let generation = cache.generation.load(Ordering::Acquire); + let generation = cache + .state + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .generations + .get(&p) + .copied() + .unwrap_or(0); let stale = Arc::new( PrincipalProfile::load(&cache.astrid_home, &p).expect("load pre-invalidation profile"), ); @@ -432,6 +448,24 @@ mod tests { assert!(!cache.resolve(&p).expect("resolve current profile").enabled); } + #[test] + fn invalidating_one_principal_does_not_reject_another_principals_load() { + let (_dir, cache) = fixture(); + let alice = principal("alice-invalidation"); + let bob = principal("bob-load"); + let bob_generation = 0; + let bob_profile = Arc::new(PrincipalProfile::default()); + + cache.invalidate(&alice); + + assert!( + cache + .publish_loaded(&bob, bob_profile, bob_generation) + .is_some() + ); + assert_eq!(cache.len(), 1); + } + #[test] fn concurrent_readers_do_not_race() { // Lightweight contention check — not a loom model, just a sanity From e20fd78bb4724f2a6fb1d5e08ec94532f02c99ac Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 14 Jul 2026 21:43:44 +0400 Subject: [PATCH 05/16] test(core): read per-principal cache generations --- crates/astrid-capsule/src/profile_cache.rs | 29 +++++++++++----------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/crates/astrid-capsule/src/profile_cache.rs b/crates/astrid-capsule/src/profile_cache.rs index ac86b2ae3..72c4cd423 100644 --- a/crates/astrid-capsule/src/profile_cache.rs +++ b/crates/astrid-capsule/src/profile_cache.rs @@ -246,6 +246,17 @@ impl PrincipalProfileCache { .profiles .len() } + + #[cfg(test)] + fn generation_for(&self, principal: &PrincipalId) -> u64 { + self.state + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .generations + .get(principal) + .copied() + .unwrap_or(0) + } } #[cfg(test)] @@ -421,14 +432,7 @@ mod tests { enabled = true\n" ), ); - let generation = cache - .state - .read() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .generations - .get(&p) - .copied() - .unwrap_or(0); + let generation = cache.generation_for(&p); let stale = Arc::new( PrincipalProfile::load(&cache.astrid_home, &p).expect("load pre-invalidation profile"), ); @@ -570,22 +574,19 @@ mod tests { fn persist_egress_is_idempotent() { let (_dir, cache) = fixture(); let p = principal("bob"); - let initial_generation = cache.generation.load(Ordering::Acquire); + let initial_generation = cache.generation_for(&p); // No file on disk → starts from default (empty capsule_egress). cache .persist_egress(&p, "react", "10.0.0.5:8080") .expect("first persist"); - let persisted_generation = cache.generation.load(Ordering::Acquire); + let persisted_generation = cache.generation_for(&p); assert_eq!(persisted_generation, initial_generation + 1); // A second persist of the SAME endpoint under the SAME capsule // (case-insensitive) is a no-op success, not a duplicate. cache .persist_egress(&p, "react", "10.0.0.5:8080") .expect("idempotent persist"); - assert_eq!( - cache.generation.load(Ordering::Acquire), - persisted_generation + 1 - ); + assert_eq!(cache.generation_for(&p), persisted_generation + 1); let profile = cache.resolve(&p).expect("reload"); assert_eq!( From 24e709e152c2b4de1476e807781cf46172ebead0 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 14 Jul 2026 22:40:12 +0400 Subject: [PATCH 06/16] fix(kernel): satisfy attenuation lint gates --- .../admin/pair_device_handlers.rs | 33 +++--- crates/astrid-kernel/src/kernel_router/mod.rs | 10 +- .../astrid-kernel/src/kernel_router/tests.rs | 106 ++++++++++-------- 3 files changed, 86 insertions(+), 63 deletions(-) diff --git a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs index a2006083b..cd8c52ef2 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs @@ -44,19 +44,9 @@ pub(crate) async fn pair_device_issue( label: Option, requested: PairScopeArg, ) -> AdminResponseBody { - let lifetime = expires_secs.unwrap_or(DEFAULT_EXPIRY_SECS); - if lifetime == 0 { - return err_bad_input("expires_secs must be greater than 0".into()); - } - if lifetime > MAX_EXPIRY_SECS { - return err_bad_input(format!( - "expires_secs {lifetime} exceeds the 1-hour cap ({MAX_EXPIRY_SECS}s) — pair-tokens are intended for immediate use" - )); - } - - let requested_scope = match resolve_pair_scope(&requested) { - Ok(s) => s, - Err(e) => return err_bad_input(e), + let (lifetime, requested_scope) = match validate_pair_issue(expires_secs, &requested) { + Ok(values) => values, + Err(error) => return err_bad_input(error), }; // Pair tokens must name an existing principal. @@ -166,6 +156,23 @@ pub(crate) async fn pair_device_issue( }) } +fn validate_pair_issue( + expires_secs: Option, + requested: &PairScopeArg, +) -> Result<(u64, DeviceScope), String> { + let lifetime = expires_secs.unwrap_or(DEFAULT_EXPIRY_SECS); + if lifetime == 0 { + return Err("expires_secs must be greater than 0".into()); + } + if lifetime > MAX_EXPIRY_SECS { + return Err(format!( + "expires_secs {lifetime} exceeds the 1-hour cap ({MAX_EXPIRY_SECS}s) — pair-tokens are intended for immediate use" + )); + } + + resolve_pair_scope(requested).map(|scope| (lifetime, scope)) +} + /// Resolve a [`PairScopeArg`] to a concrete [`DeviceScope`]. An unknown preset /// name is rejected (fail-closed) rather than silently defaulting. fn resolve_pair_scope(arg: &PairScopeArg) -> Result { diff --git a/crates/astrid-kernel/src/kernel_router/mod.rs b/crates/astrid-kernel/src/kernel_router/mod.rs index 2d9f13f89..858823ac8 100644 --- a/crates/astrid-kernel/src/kernel_router/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/mod.rs @@ -584,11 +584,11 @@ impl CapsuleVisibility { return Self::denied(caller); }; - let device_scope = - match resolve_device_scope(profile.as_ref(), caller, device_key_id, "capsule:list") { - Ok(scope) => scope, - Err(_) => return Self::denied(caller), - }; + let Ok(device_scope) = + resolve_device_scope(profile.as_ref(), caller, device_key_id, "capsule:list") + else { + return Self::denied(caller); + }; let groups = kernel.groups.load_full(); let mut check = CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()); if let Some(scope) = &device_scope { diff --git a/crates/astrid-kernel/src/kernel_router/tests.rs b/crates/astrid-kernel/src/kernel_router/tests.rs index 18c5a2933..b65daa35b 100644 --- a/crates/astrid-kernel/src/kernel_router/tests.rs +++ b/crates/astrid-kernel/src/kernel_router/tests.rs @@ -675,47 +675,7 @@ async fn device_scope_attenuates_every_capsule_inventory_surface() { let (_dir, kernel) = kernel_with_inventory_capsules().await; let caller = PrincipalId::new("device-scoped-admin").expect("valid principal"); seed_capsule_inventory_profile(&kernel, &caller, &["allowed"]).await; - - let full = DeviceKey::new("a".repeat(64), DeviceScope::Full, None, 0); - let self_only = DeviceKey::new( - "b".repeat(64), - DeviceScope::Scoped { - allow: vec!["self:*".to_string()], - deny: Vec::new(), - }, - None, - 0, - ); - let global_list = DeviceKey::new( - "c".repeat(64), - DeviceScope::Scoped { - allow: vec!["self:*".to_string(), "capsule:list".to_string()], - deny: Vec::new(), - }, - None, - 0, - ); - let denied_global_list = DeviceKey::new( - "d".repeat(64), - DeviceScope::Scoped { - allow: vec!["*".to_string()], - deny: vec!["capsule:list".to_string()], - }, - None, - 0, - ); - let full_id = full.key_id.clone(); - let self_only_id = self_only.key_id.clone(); - let global_list_id = global_list.key_id.clone(); - let denied_global_list_id = denied_global_list.key_id.clone(); - let mut profile = PrincipalProfile { - grants: vec!["*".to_string()], - capsules: vec!["allowed".to_string()], - ..Default::default() - }; - profile.auth.methods.push(AuthMethod::Keypair); - profile.auth.public_keys = vec![full, self_only, global_list, denied_global_list]; - seed_profile(&kernel, &caller, &profile); + let devices = seed_inventory_device_scopes(&kernel, &caller); let global_capsules = &["allowed", "default-only"]; let global_commands = &["allowed-cmd", "default-only-cmd"]; @@ -736,7 +696,7 @@ async fn device_scope_attenuates_every_capsule_inventory_surface() { assert_capsule_inventory_surface_for_device( &kernel, &caller, - Some(&full_id), + Some(&devices.full), "full_device_admin", global_capsules, global_commands, @@ -747,7 +707,7 @@ async fn device_scope_attenuates_every_capsule_inventory_surface() { assert_capsule_inventory_surface_for_device( &kernel, &caller, - Some(&self_only_id), + Some(&devices.self_only), "self_only_device_admin", scoped_capsules, scoped_commands, @@ -758,7 +718,7 @@ async fn device_scope_attenuates_every_capsule_inventory_surface() { assert_capsule_inventory_surface_for_device( &kernel, &caller, - Some(&denied_global_list_id), + Some(&devices.denied_global_list), "global_list_denied_device_admin", scoped_capsules, scoped_commands, @@ -769,7 +729,7 @@ async fn device_scope_attenuates_every_capsule_inventory_surface() { assert_capsule_inventory_surface_for_device( &kernel, &caller, - Some(&global_list_id), + Some(&devices.global_list), "global_list_device_admin", global_capsules, global_commands, @@ -795,6 +755,62 @@ async fn device_scope_attenuates_every_capsule_inventory_surface() { assert!(matches!(response, KernelResponse::Error(_))); } +struct InventoryDeviceScopes { + full: String, + self_only: String, + global_list: String, + denied_global_list: String, +} + +fn seed_inventory_device_scopes( + kernel: &Arc, + caller: &PrincipalId, +) -> InventoryDeviceScopes { + let full = DeviceKey::new("a".repeat(64), DeviceScope::Full, None, 0); + let self_only = DeviceKey::new( + "b".repeat(64), + DeviceScope::Scoped { + allow: vec!["self:*".to_string()], + deny: Vec::new(), + }, + None, + 0, + ); + let global_list = DeviceKey::new( + "c".repeat(64), + DeviceScope::Scoped { + allow: vec!["self:*".to_string(), "capsule:list".to_string()], + deny: Vec::new(), + }, + None, + 0, + ); + let denied_global_list = DeviceKey::new( + "d".repeat(64), + DeviceScope::Scoped { + allow: vec!["*".to_string()], + deny: vec!["capsule:list".to_string()], + }, + None, + 0, + ); + let devices = InventoryDeviceScopes { + full: full.key_id.clone(), + self_only: self_only.key_id.clone(), + global_list: global_list.key_id.clone(), + denied_global_list: denied_global_list.key_id.clone(), + }; + let mut profile = PrincipalProfile { + grants: vec!["*".to_string()], + capsules: vec!["allowed".to_string()], + ..Default::default() + }; + profile.auth.methods.push(AuthMethod::Keypair); + profile.auth.public_keys = vec![full, self_only, global_list, denied_global_list]; + seed_profile(kernel, caller, &profile); + devices +} + async fn kernel_with_inventory_capsules() -> (tempfile::TempDir, Arc) { let dir = tempfile::tempdir().expect("tempdir"); let home = astrid_core::dirs::AstridHome::from_path(dir.path()); From a728124f90569ef59191687af4183c1df4b1e960 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Tue, 14 Jul 2026 23:13:41 +0400 Subject: [PATCH 07/16] test(kernel): preserve opaque device scope denials --- .../src/kernel_router/device_scope.rs | 3 ++ .../astrid-kernel/src/kernel_router/tests.rs | 28 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/crates/astrid-kernel/src/kernel_router/device_scope.rs b/crates/astrid-kernel/src/kernel_router/device_scope.rs index 2b1e0b95e..8a848f53e 100644 --- a/crates/astrid-kernel/src/kernel_router/device_scope.rs +++ b/crates/astrid-kernel/src/kernel_router/device_scope.rs @@ -9,6 +9,9 @@ use tracing::warn; /// /// A supplied id must be canonical and registered to `caller`; an invalid, /// unknown, or revoked id never falls back to full-principal authority. +/// Resolution failures deliberately share the outward scope-denial reason so +/// authorization responses cannot reveal whether a device key exists. The +/// structured security warning retains the operator-visible cause. pub(super) fn resolve_device_scope( profile: &PrincipalProfile, caller: &PrincipalId, diff --git a/crates/astrid-kernel/src/kernel_router/tests.rs b/crates/astrid-kernel/src/kernel_router/tests.rs index b65daa35b..777d058a8 100644 --- a/crates/astrid-kernel/src/kernel_router/tests.rs +++ b/crates/astrid-kernel/src/kernel_router/tests.rs @@ -755,6 +755,34 @@ async fn device_scope_attenuates_every_capsule_inventory_surface() { assert!(matches!(response, KernelResponse::Error(_))); } +#[tokio::test(flavor = "multi_thread")] +async fn device_scope_denials_do_not_expose_key_resolution() { + let (_dir, kernel) = kernel_with_inventory_capsules().await; + let caller = PrincipalId::new("device-denial-oracle").expect("valid principal"); + let devices = seed_inventory_device_scopes(&kernel, &caller); + let required = "capsule:list"; + + let scoped = authorize_request( + &kernel, + &caller, + Some(&devices.denied_global_list), + required, + ) + .expect_err("known scoped device must be denied") + .to_string(); + let malformed = authorize_request(&kernel, &caller, Some("not-a-key-id"), required) + .expect_err("malformed device id must be denied") + .to_string(); + // Revoked devices are removed from the profile and therefore share the + // same resolution path as a never-registered id. + let unresolved = authorize_request(&kernel, &caller, Some("0000000000000000"), required) + .expect_err("unresolved device id must be denied") + .to_string(); + + assert_eq!(malformed, scoped); + assert_eq!(unresolved, scoped); +} + struct InventoryDeviceScopes { full: String, self_only: String, From 470408c08cfe9b079e485a7b6614bcc199fd42bb Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 15 Jul 2026 00:00:57 +0400 Subject: [PATCH 08/16] test(capsule): clarify cache isolation regression --- crates/astrid-capsule/src/profile_cache.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/astrid-capsule/src/profile_cache.rs b/crates/astrid-capsule/src/profile_cache.rs index 72c4cd423..623b35ffa 100644 --- a/crates/astrid-capsule/src/profile_cache.rs +++ b/crates/astrid-capsule/src/profile_cache.rs @@ -453,7 +453,7 @@ mod tests { } #[test] - fn invalidating_one_principal_does_not_reject_another_principals_load() { + fn invalidating_one_principal_does_not_reject_another_principal_load() { let (_dir, cache) = fixture(); let alice = principal("alice-invalidation"); let bob = principal("bob-load"); From 8946faa615d8824b4e6cc5de6fb791b57c59533f Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 15 Jul 2026 00:43:42 +0400 Subject: [PATCH 09/16] fix(kernel): pin request authorization snapshots --- .../src/kernel_router/admin/group.rs | 32 ++- .../src/kernel_router/admin/handlers.rs | 51 ++++- .../src/kernel_router/admin/mod.rs | 112 +++++------ .../admin/pair_device_handlers.rs | 52 ++--- .../kernel_router/admin/pair_device_tests.rs | 15 +- .../src/kernel_router/admin/state_tests.rs | 29 +++ .../kernel_router/admin/state_tests_group.rs | 25 +++ crates/astrid-kernel/src/kernel_router/mod.rs | 184 +++++++++--------- .../astrid-kernel/src/kernel_router/tests.rs | 78 ++++++-- 9 files changed, 381 insertions(+), 197 deletions(-) diff --git a/crates/astrid-kernel/src/kernel_router/admin/group.rs b/crates/astrid-kernel/src/kernel_router/admin/group.rs index 981d55c1e..6720477c2 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/group.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/group.rs @@ -13,6 +13,8 @@ use astrid_core::groups::{Group, GroupConfig}; use astrid_core::principal::PrincipalId; use astrid_events::kernel_api::{AdminResponseBody, GroupSummary}; +use crate::kernel_router::AuthorizedRequest; + use super::handlers::{err_bad_input, err_internal, success_json}; pub(super) async fn group_create( @@ -71,14 +73,19 @@ pub(super) async fn group_modify( pub(super) fn group_list( kernel: &Arc, caller: &PrincipalId, + authorization: Option<&AuthorizedRequest>, device_key_id: Option<&str>, ) -> AdminResponseBody { - let cfg = kernel.groups.load_full(); - let visible_groups = if caller_has_global_group_list(kernel, caller, device_key_id) { - None - } else { - Some(caller_group_names(kernel, caller)) - }; + let cfg = authorization.map_or_else( + || kernel.groups.load_full(), + |authorization| Arc::clone(&authorization.groups), + ); + let visible_groups = + if caller_has_global_group_list(kernel, caller, authorization, device_key_id) { + None + } else { + Some(caller_group_names(kernel, caller, authorization)) + }; let mut summaries: Vec = cfg .iter() .filter(|(name, _)| { @@ -101,8 +108,12 @@ pub(super) fn group_list( fn caller_has_global_group_list( kernel: &Arc, caller: &PrincipalId, + authorization: Option<&AuthorizedRequest>, device_key_id: Option<&str>, ) -> bool { + if let Some(authorization) = authorization { + return authorization.capability_check().has("group:list"); + } let Ok(profile) = kernel.profile_cache.resolve(caller) else { return false; }; @@ -126,7 +137,14 @@ fn caller_has_global_group_list( check.has("group:list") } -fn caller_group_names(kernel: &Arc, caller: &PrincipalId) -> BTreeSet { +fn caller_group_names( + kernel: &Arc, + caller: &PrincipalId, + authorization: Option<&AuthorizedRequest>, +) -> BTreeSet { + if let Some(authorization) = authorization { + return authorization.profile.groups.iter().cloned().collect(); + } kernel .profile_cache .resolve(caller) diff --git a/crates/astrid-kernel/src/kernel_router/admin/handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/handlers.rs index 3aa067faf..2b026737d 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/handlers.rs @@ -36,6 +36,8 @@ use astrid_core::profile::{ use astrid_events::kernel_api::{AdminRequestKind, AdminResponseBody, AgentSummary}; use tracing::{info, warn}; +use crate::kernel_router::AuthorizedRequest; + /// Platform label used by the identity store for agent principals /// created via [`AdminRequestKind::AgentCreate`]. The per-principal /// `platform_user_id` equals the `PrincipalId` string. @@ -52,10 +54,9 @@ pub(super) const AGENT_IDENTITY_PLATFORM: &str = "cli"; /// token tied to the caller's own principal regardless of any /// wire-level hint) need it. /// -/// Thin wrapper over [`dispatch_with_device`] for callers (tests, any -/// non-device path) that have no authenticating device key id. The -/// production admin path uses [`dispatch_with_device`] so a paired issuer's -/// own device scope is threaded into `PairDeviceIssue`'s no-escalation checks. +/// Thin wrapper over [`dispatch_with_device`] for direct callers and tests that +/// have no pinned authorization snapshot. The production admin path uses +/// [`dispatch_authorized`]. pub(super) async fn dispatch( kernel: &Arc, caller: &PrincipalId, @@ -64,6 +65,21 @@ pub(super) async fn dispatch( dispatch_with_device(kernel, caller, None, req).await } +pub(super) async fn dispatch_authorized( + kernel: &Arc, + authorization: &AuthorizedRequest, + req: AdminRequestKind, +) -> AdminResponseBody { + dispatch_inner( + kernel, + &authorization.principal, + Some(authorization), + None, + req, + ) + .await +} + /// Dispatch carrying the caller's authenticating device key id. /// Device scope applies to every authority decision made during dispatch. pub(super) async fn dispatch_with_device( @@ -71,6 +87,16 @@ pub(super) async fn dispatch_with_device( caller: &PrincipalId, device_key_id: Option<&str>, req: AdminRequestKind, +) -> AdminResponseBody { + dispatch_inner(kernel, caller, None, device_key_id, req).await +} + +async fn dispatch_inner( + kernel: &Arc, + caller: &PrincipalId, + authorization: Option<&AuthorizedRequest>, + device_key_id: Option<&str>, + req: AdminRequestKind, ) -> AdminResponseBody { match req { req @ AdminRequestKind::AgentCreate { .. } => agent_create_from_req(kernel, req).await, @@ -81,7 +107,7 @@ pub(super) async fn dispatch_with_device( AdminRequestKind::AgentDisable { principal } => { agent_set_enabled(kernel, principal, false).await }, - AdminRequestKind::AgentList => agent_list(kernel, caller, device_key_id), + AdminRequestKind::AgentList => agent_list(kernel, caller, authorization, device_key_id), req @ AdminRequestKind::AgentModify { .. } => agent_modify_from_req(kernel, req).await, AdminRequestKind::QuotaSet { principal, quotas } => { super::quota::quota_set(kernel, principal, quotas).await @@ -105,7 +131,9 @@ pub(super) async fn dispatch_with_device( } => { super::group::group_modify(kernel, name, capabilities, description, unsafe_admin).await }, - AdminRequestKind::GroupList => super::group::group_list(kernel, caller, device_key_id), + AdminRequestKind::GroupList => { + super::group::group_list(kernel, caller, authorization, device_key_id) + }, AdminRequestKind::CapsGrant { principal, capabilities, @@ -150,7 +178,7 @@ pub(super) async fn dispatch_with_device( | AdminRequestKind::PairDeviceRedeem { .. } | AdminRequestKind::PairDeviceList { .. } | AdminRequestKind::PairDeviceRevoke { .. }) => { - pair_device_dispatch(kernel, caller, device_key_id, req).await + pair_device_dispatch(kernel, caller, authorization, device_key_id, req).await }, } } @@ -161,6 +189,7 @@ pub(super) async fn dispatch_with_device( async fn pair_device_dispatch( kernel: &Arc, caller: &PrincipalId, + authorization: Option<&AuthorizedRequest>, issuer_device_key_id: Option<&str>, req: AdminRequestKind, ) -> AdminResponseBody { @@ -173,6 +202,7 @@ async fn pair_device_dispatch( super::pair_device_handlers::pair_device_issue( kernel, caller, + authorization, issuer_device_key_id, expires_secs, label, @@ -649,8 +679,12 @@ where fn caller_has_global_agent_list( kernel: &Arc, caller: &PrincipalId, + authorization: Option<&AuthorizedRequest>, device_key_id: Option<&str>, ) -> bool { + if let Some(authorization) = authorization { + return authorization.capability_check().has("agent:list"); + } let Ok(profile) = kernel.profile_cache.resolve(caller) else { return false; }; @@ -677,6 +711,7 @@ fn caller_has_global_agent_list( fn agent_list( kernel: &Arc, caller: &PrincipalId, + authorization: Option<&AuthorizedRequest>, device_key_id: Option<&str>, ) -> AdminResponseBody { // Source of truth: `etc/profiles/{principal}.toml`. Iterating the @@ -742,7 +777,7 @@ fn agent_list( // self-scoped form), so in practice only the `admin` group's `*` // (which matches both) sees everyone. This realises the gateway's // documented "the kernel filters server-side" contract. - if !caller_has_global_agent_list(kernel, caller, device_key_id) { + if !caller_has_global_agent_list(kernel, caller, authorization, device_key_id) { summaries.retain(|s| s.principal == *caller); } diff --git a/crates/astrid-kernel/src/kernel_router/admin/mod.rs b/crates/astrid-kernel/src/kernel_router/admin/mod.rs index 99a354e60..eaff29b32 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/mod.rs @@ -546,65 +546,65 @@ async fn handle_admin_request( return; } - match authorize_request(kernel, &caller, device_key_id.as_deref(), required_cap) { - Ok(()) => { - record_admin_audit( - kernel, - AdminAuditEntry { - caller: &caller, - method, - required_cap, - device_key_id: device_key_id.as_deref(), - target_principal: target.clone(), - params: audit_params.clone(), - authorization: AuthorizationProof::System { - reason: format!("policy allow: {caller} holds {required_cap}"), + let authorization = + match authorize_request(kernel, &caller, device_key_id.as_deref(), required_cap) { + Ok(authorization) => { + record_admin_audit( + kernel, + AdminAuditEntry { + caller: &caller, + method, + required_cap, + device_key_id: device_key_id.as_deref(), + target_principal: target.clone(), + params: audit_params.clone(), + authorization: AuthorizationProof::System { + reason: format!("policy allow: {caller} holds {required_cap}"), + }, + outcome: AuditOutcome::success(), }, - outcome: AuditOutcome::success(), - }, - ) - .await; - }, - Err(e) => { - warn!( - security_event = true, - method = method, - principal = %caller, - required = required_cap, - error = %e, - "Permission check denied admin request" - ); - record_admin_audit( - kernel, - AdminAuditEntry { - caller: &caller, - method, - required_cap, - device_key_id: device_key_id.as_deref(), - target_principal: target, - params: audit_params, - authorization: AuthorizationProof::Denied { - reason: e.to_string(), + ) + .await; + authorization + }, + Err(e) => { + warn!( + security_event = true, + method = method, + principal = %caller, + required = required_cap, + error = %e, + "Permission check denied admin request" + ); + record_admin_audit( + kernel, + AdminAuditEntry { + caller: &caller, + method, + required_cap, + device_key_id: device_key_id.as_deref(), + target_principal: target, + params: audit_params, + authorization: AuthorizationProof::Denied { + reason: e.to_string(), + }, + outcome: AuditOutcome::failure(e.to_string()), }, - outcome: AuditOutcome::failure(e.to_string()), - }, - ) - .await; - publish_response( - kernel, - response_topic, - AdminKernelResponse::for_request( - request_id, - AdminResponseBody::Error(e.to_string()), - ), - ); - return; - }, - } + ) + .await; + publish_response( + kernel, + response_topic, + AdminKernelResponse::for_request( + request_id, + AdminResponseBody::Error(e.to_string()), + ), + ); + return; + }, + }; - // Secondary views and pair delegation use the authenticated device scope. - let body = - handlers::dispatch_with_device(kernel, &caller, device_key_id.as_deref(), req.kind).await; + let body = handlers::dispatch_authorized(kernel, &authorization, req.kind).await; publish_response( kernel, response_topic, diff --git a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs index cd8c52ef2..266f6a8c4 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs @@ -29,6 +29,7 @@ use astrid_core::profile::{ }; use tracing::{info, warn}; +use crate::kernel_router::AuthorizedRequest; use crate::pair_token::{self, MAX_EXPIRY_SECS, PairToken, PairTokenStore}; /// Default token lifetime when the issuer doesn't specify. Matches @@ -36,9 +37,10 @@ use crate::pair_token::{self, MAX_EXPIRY_SECS, PairToken, PairTokenStore}; /// device to be close at hand. const DEFAULT_EXPIRY_SECS: u64 = 5 * 60; -pub(crate) async fn pair_device_issue( +pub(super) async fn pair_device_issue( kernel: &Arc, caller: &PrincipalId, + authorization: Option<&AuthorizedRequest>, issuer_device_key_id: Option<&str>, expires_secs: Option, label: Option, @@ -49,28 +51,34 @@ pub(crate) async fn pair_device_issue( Err(error) => return err_bad_input(error), }; - // Pair tokens must name an existing principal. - let profile_path = kernel.astrid_home.profile_path(caller); - if !profile_path.exists() { - return err_bad_input(format!( - "caller principal {caller} does not exist (no profile.toml)" - )); - } - - let profile = match kernel.profile_cache.resolve(caller) { - Ok(p) => p, - Err(e) => return err_internal(format!("issuer profile resolution failed: {e}")), - }; - let issuer_scope = match crate::kernel_router::resolve_device_scope( - profile.as_ref(), - caller, - issuer_device_key_id, - "self:auth:pair", - ) { - Ok(scope) => scope, - Err(e) => return err_unauthorized(e.to_string()), + let (profile, groups, issuer_scope) = if let Some(authorization) = authorization { + ( + Arc::clone(&authorization.profile), + Arc::clone(&authorization.groups), + authorization.device_scope.clone(), + ) + } else { + let profile_path = kernel.astrid_home.profile_path(caller); + if !profile_path.exists() { + return err_bad_input(format!( + "caller principal {caller} does not exist (no profile.toml)" + )); + } + let profile = match kernel.profile_cache.resolve(caller) { + Ok(profile) => profile, + Err(e) => return err_internal(format!("issuer profile resolution failed: {e}")), + }; + let issuer_scope = match crate::kernel_router::resolve_device_scope( + profile.as_ref(), + caller, + issuer_device_key_id, + "self:auth:pair", + ) { + Ok(scope) => scope, + Err(e) => return err_unauthorized(e.to_string()), + }; + (profile, kernel.groups.load_full(), issuer_scope) }; - let groups = kernel.groups.load_full(); let mut issuer_check = CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()); // A scoped issuer can grant only its attenuated authority. if let Some(scope @ DeviceScope::Scoped { .. }) = &issuer_scope { diff --git a/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs b/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs index 6eb5f1601..286827bd8 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs @@ -1,7 +1,7 @@ //! Acceptance tests for per-device capability scope on pairing. //! //! These drive the kernel/handler path end-to-end (real `Kernel`, profiles on -//! disk, the same `handlers::dispatch_with_device` the IPC router uses) and +//! disk, with the same authorization snapshot the IPC router uses) and //! prove the no-escalation guarantees the feature exists to provide: //! //! * a `use-only` device cannot mint pair-tokens (cap-gate denial), but the @@ -377,6 +377,17 @@ async fn scoped_issuer_child_inherits_issuer_denies() { let dev = scoped_device('a', &["self:*"], &["self:capsule:install"]); let dev_id = dev.key_id.clone(); seed(&kernel, &caller, &["self:*"], vec![dev]); + let authorization = + crate::kernel_router::authorize_request(&kernel, &caller, Some(&dev_id), "self:auth:pair") + .expect("authorize pair issuance"); + let mut revoked = load(&kernel, &caller); + revoked.auth.public_keys.clear(); + revoked + .save_to_path(&PrincipalProfile::path_for(&kernel.astrid_home, &caller)) + .expect("revoke issuer device"); + kernel.profile_cache.invalidate(&caller); + crate::kernel_router::authorize_request(&kernel, &caller, Some(&dev_id), "self:auth:pair") + .expect_err("a later request must observe the revoked issuer"); // The issuer requests a broad child: allow self:*. The stored child scope // must inherit the issuer's deny (self:capsule:install) so the child can @@ -385,7 +396,7 @@ async fn scoped_issuer_child_inherits_issuer_denies() { allow: vec!["self:*".into()], deny: vec![], }); - let resp = handlers::dispatch_with_device(&kernel, &caller, Some(&dev_id), req).await; + let resp = handlers::dispatch_authorized(&kernel, &authorization, req).await; let token = match resp { AdminResponseBody::PairToken(t) => t.token, other => panic!("issue must succeed (allow self:* ⊆ issuer self:*): {other:?}"), diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs index d66528ad7..8a91ddb56 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs @@ -1093,4 +1093,33 @@ async fn agent_list_global_view_is_attenuated_by_device_scope() { ); assert_eq!(scoped.len(), 1); assert_eq!(scoped[0].principal, PrincipalId::default()); + + let authorization = crate::kernel_router::authorize_request( + &kernel, + &PrincipalId::default(), + Some(&global_list_id), + "self:agent:list", + ) + .expect("authorize agent inventory"); + profile.auth.public_keys.clear(); + profile + .save_to_path(&PrincipalProfile::path_for( + &kernel.astrid_home, + &PrincipalId::default(), + )) + .expect("revoke inventory device"); + kernel.profile_cache.invalidate(&PrincipalId::default()); + crate::kernel_router::authorize_request( + &kernel, + &PrincipalId::default(), + Some(&global_list_id), + "self:agent:list", + ) + .expect_err("a later request must observe the revoked device"); + + let pinned = list_for( + handlers::dispatch_authorized(&kernel, &authorization, AdminRequestKind::AgentList).await, + ); + assert!(pinned.iter().any(|entry| entry.principal == pid("alice"))); + assert!(pinned.iter().any(|entry| entry.principal == pid("bob"))); } diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs index f844bbbcd..e760689ec 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs @@ -152,4 +152,29 @@ async fn group_list_global_view_is_attenuated_by_device_scope() { .await, ); assert_eq!(scoped, vec![BUILTIN_ADMIN]); + + let authorization = crate::kernel_router::authorize_request( + &kernel, + &PrincipalId::default(), + Some(&global_list_id), + "self:group:list", + ) + .expect("authorize group inventory"); + profile.auth.public_keys.clear(); + profile + .save_to_path(&profile_path) + .expect("revoke group inventory device"); + kernel.profile_cache.invalidate(&PrincipalId::default()); + crate::kernel_router::authorize_request( + &kernel, + &PrincipalId::default(), + Some(&global_list_id), + "self:group:list", + ) + .expect_err("a later request must observe the revoked device"); + + let pinned = names_for( + handlers::dispatch_authorized(&kernel, &authorization, AdminRequestKind::GroupList).await, + ); + assert!(pinned.iter().any(|name| name == "ops")); } diff --git a/crates/astrid-kernel/src/kernel_router/mod.rs b/crates/astrid-kernel/src/kernel_router/mod.rs index 858823ac8..e36be35ee 100644 --- a/crates/astrid-kernel/src/kernel_router/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/mod.rs @@ -19,7 +19,9 @@ use std::sync::atomic::{AtomicBool, Ordering}; use astrid_audit::{AuditAction, AuditOutcome, AuthorizationProof}; use astrid_capabilities::{CapabilityCheck, PermissionError}; +use astrid_core::groups::GroupConfig; use astrid_core::principal::PrincipalId; +use astrid_core::profile::{DeviceScope, PrincipalProfile}; use astrid_events::ipc::{IpcMessage, IpcPayload, Topic}; use astrid_events::kernel_api::{KernelRequest, KernelResponse}; use tracing::{debug, info, warn}; @@ -229,53 +231,55 @@ async fn handle_request( let method = kernel_request_method(&req); let scope = resolve_scope(&req, &caller); let required_cap = required_capability(&req, scope); - match authorize_request(kernel, &caller, device_key_id.as_deref(), required_cap) { - Ok(()) => { - record_admin_audit( - kernel, - AdminAuditEntry { - caller: &caller, - method, - required_cap, - device_key_id: device_key_id.as_deref(), - target_principal: None, - params: None, - authorization: AuthorizationProof::System { - reason: format!("policy allow: {caller} holds {required_cap}"), + let authorization = + match authorize_request(kernel, &caller, device_key_id.as_deref(), required_cap) { + Ok(authorization) => { + record_admin_audit( + kernel, + AdminAuditEntry { + caller: &caller, + method, + required_cap, + device_key_id: device_key_id.as_deref(), + target_principal: None, + params: None, + authorization: AuthorizationProof::System { + reason: format!("policy allow: {caller} holds {required_cap}"), + }, + outcome: AuditOutcome::success(), }, - outcome: AuditOutcome::success(), - }, - ) - .await; - }, - Err(e) => { - warn!( - security_event = true, - method = method, - principal = %caller, - required = required_cap, - "Permission check denied admin request" - ); - record_admin_audit( - kernel, - AdminAuditEntry { - caller: &caller, - method, - required_cap, - device_key_id: device_key_id.as_deref(), - target_principal: None, - params: None, - authorization: AuthorizationProof::Denied { - reason: e.to_string(), + ) + .await; + authorization + }, + Err(e) => { + warn!( + security_event = true, + method = method, + principal = %caller, + required = required_cap, + "Permission check denied admin request" + ); + record_admin_audit( + kernel, + AdminAuditEntry { + caller: &caller, + method, + required_cap, + device_key_id: device_key_id.as_deref(), + target_principal: None, + params: None, + authorization: AuthorizationProof::Denied { + reason: e.to_string(), + }, + outcome: AuditOutcome::failure(e.to_string()), }, - outcome: AuditOutcome::failure(e.to_string()), - }, - ) - .await; - publish_response(kernel, response_topic, KernelResponse::Error(e.to_string())); - return; - }, - } + ) + .await; + publish_response(kernel, response_topic, KernelResponse::Error(e.to_string())); + return; + }, + }; // Keepalive pinger: from here until the terminal response is published, emit // a `KernelResponse::Working` frame every `KEEPALIVE_INTERVAL` so a waiting @@ -301,7 +305,7 @@ async fn handle_request( KernelResponse::Error("Approval logic not yet implemented in kernel router".to_string()) }, KernelRequest::ListCapsules => { - let visibility = CapsuleVisibility::new(kernel, &caller, device_key_id.as_deref()); + let visibility = CapsuleVisibility::new(&authorization); let list: Vec<_> = visible_inventory_manifests(kernel, &visibility) .await .into_iter() @@ -310,7 +314,7 @@ async fn handle_request( KernelResponse::Success(serde_json::json!(list)) }, KernelRequest::GetCommands => { - let visibility = CapsuleVisibility::new(kernel, &caller, device_key_id.as_deref()); + let visibility = CapsuleVisibility::new(&authorization); let mut commands = Vec::new(); let manifests = visible_inventory_manifests(kernel, &visibility).await; for manifest in &manifests { @@ -435,7 +439,7 @@ async fn handle_request( KernelResponse::Status(status) }, KernelRequest::GetCapsuleMetadata => { - let visibility = CapsuleVisibility::new(kernel, &caller, device_key_id.as_deref()); + let visibility = CapsuleVisibility::new(&authorization); let mut entries = Vec::new(); for manifest in visible_inventory_manifests(kernel, &visibility).await { entries.push(astrid_events::kernel_api::CapsuleMetadataEntry { @@ -451,7 +455,7 @@ async fn handle_request( KernelResponse::CapsuleMetadata(entries) }, KernelRequest::GetAgentReadiness => { - let visibility = CapsuleVisibility::new(kernel, &caller, device_key_id.as_deref()); + let visibility = CapsuleVisibility::new(&authorization); let manifests = visible_inventory_manifests(kernel, &visibility).await; let readiness = astrid_capsule::readiness::agent_loop_readiness(&manifests); KernelResponse::AgentReadiness(readiness) @@ -563,40 +567,15 @@ struct CapsuleVisibility { } impl CapsuleVisibility { - fn new(kernel: &crate::Kernel, caller: &PrincipalId, device_key_id: Option<&str>) -> Self { - let profile = if caller.as_str() == "anonymous" { - None - } else { - match kernel.profile_cache.resolve(caller) { - Ok(profile) => Some(profile), - Err(e) => { - warn!( - security_event = true, - principal = %caller, - error = %e, - "Capsule inventory visibility check: profile resolution failed — deny" - ); - None - }, - } - }; - let Some(profile) = profile else { - return Self::denied(caller); - }; - - let Ok(device_scope) = - resolve_device_scope(profile.as_ref(), caller, device_key_id, "capsule:list") - else { - return Self::denied(caller); - }; - let groups = kernel.groups.load_full(); - let mut check = CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()); - if let Some(scope) = &device_scope { - check = check.with_device_scope(scope); + fn new(authorization: &AuthorizedRequest) -> Self { + if authorization.principal.as_str() == "anonymous" { + return Self::denied(&authorization.principal); } + let profile = authorization.profile.as_ref(); + let check = authorization.capability_check(); Self { - principal: caller.clone(), + principal: authorization.principal.clone(), is_admin: check.has("capsule:list"), capsule_grants: profile.capsules.iter().cloned().collect(), } @@ -794,19 +773,42 @@ fn resolve_device_key_id(message: &IpcMessage) -> Option { message.device_key_id.clone() } -/// Evaluate the capability check for `caller` against the kernel's -/// resolved group config and the caller's profile. +/// Authorization inputs pinned at the request's policy decision point. +#[derive(Debug)] +struct AuthorizedRequest { + principal: PrincipalId, + profile: Arc, + groups: Arc, + device_scope: Option, +} + +impl AuthorizedRequest { + fn capability_check(&self) -> CapabilityCheck<'_> { + let check = CapabilityCheck::new( + self.profile.as_ref(), + self.groups.as_ref(), + self.principal.clone(), + ); + match &self.device_scope { + Some(scope) => check.with_device_scope(scope), + None => check, + } + } +} + +/// Evaluate the capability check for `caller` against the kernel's resolved +/// group config and the caller's profile. /// -/// Returns `Ok(())` on success, or the policy reason on denial. Profile -/// resolution failures (malformed TOML, IO error) are themselves treated -/// as deny — fail-closed — with a synthesized `MissingCapability` so the -/// deny path has a single shape in the audit log. +/// Returns the pinned authorization snapshot on success, or the policy reason +/// on denial. Profile resolution failures (malformed TOML, IO error) are +/// themselves treated as deny — fail-closed — with a synthesized +/// `MissingCapability` so the deny path has a single shape in the audit log. fn authorize_request( kernel: &crate::Kernel, caller: &PrincipalId, device_key_id: Option<&str>, required_cap: &str, -) -> Result<(), PermissionError> { +) -> Result { let profile = match kernel.profile_cache.resolve(caller) { Ok(p) => p, Err(e) => { @@ -847,7 +849,13 @@ fn authorize_request( if let Some(scope) = &device_scope { check = check.with_device_scope(scope); } - check.require(required_cap) + check.require(required_cap)?; + Ok(AuthorizedRequest { + principal: caller.clone(), + profile, + groups, + device_scope, + }) } /// Bundled inputs for [`record_admin_audit`] — keeps the call site diff --git a/crates/astrid-kernel/src/kernel_router/tests.rs b/crates/astrid-kernel/src/kernel_router/tests.rs index 777d058a8..2107f5308 100644 --- a/crates/astrid-kernel/src/kernel_router/tests.rs +++ b/crates/astrid-kernel/src/kernel_router/tests.rs @@ -658,9 +658,16 @@ async fn capsule_visibility_precomputes_admin_and_capsule_grants() { let allowed = CapsuleId::new("allowed").expect("valid capsule id"); let default_only = CapsuleId::new("default-only").expect("valid capsule id"); - let admin_visibility = CapsuleVisibility::new(&kernel, &admin, None); - let global_lister_visibility = CapsuleVisibility::new(&kernel, &global_lister, None); - let limited_visibility = CapsuleVisibility::new(&kernel, &limited, None); + let admin_authorization = + authorize_request(&kernel, &admin, None, "self:capsule:list").expect("authorize admin"); + let global_lister_authorization = + authorize_request(&kernel, &global_lister, None, "self:capsule:list") + .expect("authorize global lister"); + let limited_authorization = + authorize_request(&kernel, &limited, None, "self:capsule:list").expect("authorize limited"); + let admin_visibility = CapsuleVisibility::new(&admin_authorization); + let global_lister_visibility = CapsuleVisibility::new(&global_lister_authorization); + let limited_visibility = CapsuleVisibility::new(&limited_authorization); assert!(admin_visibility.allows(&allowed)); assert!(admin_visibility.allows(&default_only)); @@ -738,12 +745,6 @@ async fn device_scope_attenuates_every_capsule_inventory_surface() { ) .await; - let allowed = CapsuleId::new("allowed").expect("valid capsule id"); - let unknown = CapsuleVisibility::new(&kernel, &caller, Some("0000000000000000")); - let malformed = CapsuleVisibility::new(&kernel, &caller, Some("not-a-key-id")); - assert!(!unknown.allows(&allowed)); - assert!(!malformed.allows(&allowed)); - let response = request_kernel_for_device( &kernel, &caller, @@ -755,6 +756,39 @@ async fn device_scope_attenuates_every_capsule_inventory_surface() { assert!(matches!(response, KernelResponse::Error(_))); } +#[tokio::test(flavor = "multi_thread")] +async fn capsule_visibility_uses_the_authorized_device_scope_snapshot() { + let (_dir, kernel) = kernel_with_inventory_capsules().await; + let caller = PrincipalId::new("device-snapshot-admin").expect("valid principal"); + let devices = seed_inventory_device_scopes(&kernel, &caller); + let authorization = authorize_request( + &kernel, + &caller, + Some(&devices.global_list), + "self:capsule:list", + ) + .expect("authorize inventory request"); + + let allowed = CapsuleId::new("default-only").expect("valid capsule id"); + assert!(CapsuleVisibility::new(&authorization).allows(&allowed)); + + let mut revoked = authorization.profile.as_ref().clone(); + revoked.auth.public_keys.clear(); + seed_profile(&kernel, &caller, &revoked); + authorize_request( + &kernel, + &caller, + Some(&devices.global_list), + "self:capsule:list", + ) + .expect_err("a later request must observe the revoked device"); + + assert!( + CapsuleVisibility::new(&authorization).allows(&allowed), + "the in-flight request must keep the authority snapshot already audited as allowed" + ); +} + #[tokio::test(flavor = "multi_thread")] async fn device_scope_denials_do_not_expose_key_resolution() { let (_dir, kernel) = kernel_with_inventory_capsules().await; @@ -773,14 +807,30 @@ async fn device_scope_denials_do_not_expose_key_resolution() { let malformed = authorize_request(&kernel, &caller, Some("not-a-key-id"), required) .expect_err("malformed device id must be denied") .to_string(); - // Revoked devices are removed from the profile and therefore share the - // same resolution path as a never-registered id. - let unresolved = authorize_request(&kernel, &caller, Some("0000000000000000"), required) - .expect_err("unresolved device id must be denied") + let unknown = authorize_request(&kernel, &caller, Some("0000000000000000"), required) + .expect_err("unknown device id must be denied") .to_string(); + let mut revoked_profile = kernel + .profile_cache + .resolve(&caller) + .expect("resolve device profile") + .as_ref() + .clone(); + revoked_profile.auth.public_keys.clear(); + seed_profile(&kernel, &caller, &revoked_profile); + let revoked = authorize_request( + &kernel, + &caller, + Some(&devices.denied_global_list), + required, + ) + .expect_err("revoked device id must be denied") + .to_string(); + assert_eq!(malformed, scoped); - assert_eq!(unresolved, scoped); + assert_eq!(unknown, scoped); + assert_eq!(revoked, scoped); } struct InventoryDeviceScopes { From d337b77a3b19188d58be3d6584c32d8856a12663 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 15 Jul 2026 00:53:04 +0400 Subject: [PATCH 10/16] fix(kernel): tighten snapshot regressions --- .../admin/pair_device_handlers.rs | 68 +++++++++------- .../src/kernel_router/admin/state_tests.rs | 81 +++++++++++-------- .../kernel_router/admin/state_tests_group.rs | 78 ++++++++++-------- .../astrid-kernel/src/kernel_router/tests.rs | 2 +- 4 files changed, 133 insertions(+), 96 deletions(-) diff --git a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs index 266f6a8c4..e16b77f05 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs @@ -21,6 +21,7 @@ use std::sync::Arc; use astrid_capabilities::{CapabilityCheck, device_scope_within}; use astrid_core::PrincipalId; +use astrid_core::groups::GroupConfig; use astrid_core::kernel_api::{ AdminResponseBody, DeviceKeyInfo, PairScopeArg, PairTokenIssued, PairTokenRedeemed, }; @@ -37,6 +38,42 @@ use crate::pair_token::{self, MAX_EXPIRY_SECS, PairToken, PairTokenStore}; /// device to be close at hand. const DEFAULT_EXPIRY_SECS: u64 = 5 * 60; +type PairIssuerAuthority = (Arc, Arc, Option); + +fn resolve_pair_issuer_authority( + kernel: &Arc, + caller: &PrincipalId, + authorization: Option<&AuthorizedRequest>, + issuer_device_key_id: Option<&str>, +) -> Result { + if let Some(authorization) = authorization { + return Ok(( + Arc::clone(&authorization.profile), + Arc::clone(&authorization.groups), + authorization.device_scope.clone(), + )); + } + + let profile_path = kernel.astrid_home.profile_path(caller); + if !profile_path.exists() { + return Err(err_bad_input(format!( + "caller principal {caller} does not exist (no profile.toml)" + ))); + } + let profile = kernel + .profile_cache + .resolve(caller) + .map_err(|error| err_internal(format!("issuer profile resolution failed: {error}")))?; + let issuer_scope = crate::kernel_router::resolve_device_scope( + profile.as_ref(), + caller, + issuer_device_key_id, + "self:auth:pair", + ) + .map_err(|error| err_unauthorized(error.to_string()))?; + Ok((profile, kernel.groups.load_full(), issuer_scope)) +} + pub(super) async fn pair_device_issue( kernel: &Arc, caller: &PrincipalId, @@ -51,34 +88,11 @@ pub(super) async fn pair_device_issue( Err(error) => return err_bad_input(error), }; - let (profile, groups, issuer_scope) = if let Some(authorization) = authorization { - ( - Arc::clone(&authorization.profile), - Arc::clone(&authorization.groups), - authorization.device_scope.clone(), - ) - } else { - let profile_path = kernel.astrid_home.profile_path(caller); - if !profile_path.exists() { - return err_bad_input(format!( - "caller principal {caller} does not exist (no profile.toml)" - )); - } - let profile = match kernel.profile_cache.resolve(caller) { - Ok(profile) => profile, - Err(e) => return err_internal(format!("issuer profile resolution failed: {e}")), + let (profile, groups, issuer_scope) = + match resolve_pair_issuer_authority(kernel, caller, authorization, issuer_device_key_id) { + Ok(authority) => authority, + Err(response) => return response, }; - let issuer_scope = match crate::kernel_router::resolve_device_scope( - profile.as_ref(), - caller, - issuer_device_key_id, - "self:auth:pair", - ) { - Ok(scope) => scope, - Err(e) => return err_unauthorized(e.to_string()), - }; - (profile, kernel.groups.load_full(), issuer_scope) - }; let mut issuer_check = CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()); // A scoped issuer can grant only its attenuated authority. if let Some(scope @ DeviceScope::Scoped { .. }) = &issuer_scope { diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs index 8a91ddb56..edf64ee72 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests.rs @@ -80,6 +80,48 @@ fn assert_error_contains(res: &AdminResponseBody, needle: &str) { } } +fn agent_list_for(response: AdminResponseBody) -> Vec { + match response { + AdminResponseBody::AgentList(list) => list, + other => panic!("expected AgentList, got {other:?}"), + } +} + +async fn assert_agent_list_authorization_snapshot( + kernel: &Arc, + mut profile: PrincipalProfile, + global_list_id: &str, +) { + let authorization = crate::kernel_router::authorize_request( + kernel, + &PrincipalId::default(), + Some(global_list_id), + "self:agent:list", + ) + .expect("authorize agent inventory"); + profile.auth.public_keys.clear(); + profile + .save_to_path(&PrincipalProfile::path_for( + &kernel.astrid_home, + &PrincipalId::default(), + )) + .expect("revoke inventory device"); + kernel.profile_cache.invalidate(&PrincipalId::default()); + crate::kernel_router::authorize_request( + kernel, + &PrincipalId::default(), + Some(global_list_id), + "self:agent:list", + ) + .expect_err("a later request must observe the revoked device"); + + let pinned = agent_list_for( + handlers::dispatch_authorized(kernel, &authorization, AdminRequestKind::AgentList).await, + ); + assert!(pinned.iter().any(|entry| entry.principal == pid("alice"))); + assert!(pinned.iter().any(|entry| entry.principal == pid("bob"))); +} + // ── agent.create ───────────────────────────────────────────────────── #[tokio::test(flavor = "multi_thread")] @@ -1054,11 +1096,7 @@ async fn agent_list_global_view_is_attenuated_by_device_scope() { .expect("save device-scoped default profile"); kernel.profile_cache.invalidate(&PrincipalId::default()); - let list_for = |response| match response { - AdminResponseBody::AgentList(list) => list, - other => panic!("expected AgentList, got {other:?}"), - }; - let full = list_for( + let full = agent_list_for( handlers::dispatch_with_device( &kernel, &PrincipalId::default(), @@ -1070,7 +1108,7 @@ async fn agent_list_global_view_is_attenuated_by_device_scope() { assert!(full.iter().any(|entry| entry.principal == pid("alice"))); assert!(full.iter().any(|entry| entry.principal == pid("bob"))); - let global = list_for( + let global = agent_list_for( handlers::dispatch_with_device( &kernel, &PrincipalId::default(), @@ -1082,7 +1120,7 @@ async fn agent_list_global_view_is_attenuated_by_device_scope() { assert!(global.iter().any(|entry| entry.principal == pid("alice"))); assert!(global.iter().any(|entry| entry.principal == pid("bob"))); - let scoped = list_for( + let scoped = agent_list_for( handlers::dispatch_with_device( &kernel, &PrincipalId::default(), @@ -1094,32 +1132,5 @@ async fn agent_list_global_view_is_attenuated_by_device_scope() { assert_eq!(scoped.len(), 1); assert_eq!(scoped[0].principal, PrincipalId::default()); - let authorization = crate::kernel_router::authorize_request( - &kernel, - &PrincipalId::default(), - Some(&global_list_id), - "self:agent:list", - ) - .expect("authorize agent inventory"); - profile.auth.public_keys.clear(); - profile - .save_to_path(&PrincipalProfile::path_for( - &kernel.astrid_home, - &PrincipalId::default(), - )) - .expect("revoke inventory device"); - kernel.profile_cache.invalidate(&PrincipalId::default()); - crate::kernel_router::authorize_request( - &kernel, - &PrincipalId::default(), - Some(&global_list_id), - "self:agent:list", - ) - .expect_err("a later request must observe the revoked device"); - - let pinned = list_for( - handlers::dispatch_authorized(&kernel, &authorization, AdminRequestKind::AgentList).await, - ); - assert!(pinned.iter().any(|entry| entry.principal == pid("alice"))); - assert!(pinned.iter().any(|entry| entry.principal == pid("bob"))); + assert_agent_list_authorization_snapshot(&kernel, profile, &global_list_id).await; } diff --git a/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs b/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs index e760689ec..a31efe334 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/state_tests_group.rs @@ -35,6 +35,47 @@ fn pid(name: &str) -> PrincipalId { PrincipalId::new(name).unwrap() } +fn group_names_for(response: AdminResponseBody) -> Vec { + match response { + AdminResponseBody::GroupList(list) => list.into_iter().map(|group| group.name).collect(), + other => panic!("expected GroupList, got {other:?}"), + } +} + +async fn assert_group_list_authorization_snapshot( + kernel: &Arc, + mut profile: PrincipalProfile, + global_list_id: &str, +) { + let authorization = crate::kernel_router::authorize_request( + kernel, + &PrincipalId::default(), + Some(global_list_id), + "self:group:list", + ) + .expect("authorize group inventory"); + profile.auth.public_keys.clear(); + profile + .save_to_path(&PrincipalProfile::path_for( + &kernel.astrid_home, + &PrincipalId::default(), + )) + .expect("revoke group inventory device"); + kernel.profile_cache.invalidate(&PrincipalId::default()); + crate::kernel_router::authorize_request( + kernel, + &PrincipalId::default(), + Some(global_list_id), + "self:group:list", + ) + .expect_err("a later request must observe the revoked device"); + + let pinned = group_names_for( + handlers::dispatch_authorized(kernel, &authorization, AdminRequestKind::GroupList).await, + ); + assert!(pinned.iter().any(|name| name == "ops")); +} + #[tokio::test(flavor = "multi_thread")] async fn group_list_filters_to_callers_profile_groups_without_global_cap() { let (_dir, kernel) = fixture().await; @@ -114,13 +155,7 @@ async fn group_list_global_view_is_attenuated_by_device_scope() { .expect("save device-scoped default profile"); kernel.profile_cache.invalidate(&PrincipalId::default()); - let names_for = |response| match response { - AdminResponseBody::GroupList(list) => { - list.into_iter().map(|group| group.name).collect::>() - }, - other => panic!("expected GroupList, got {other:?}"), - }; - let full = names_for( + let full = group_names_for( handlers::dispatch_with_device( &kernel, &PrincipalId::default(), @@ -131,7 +166,7 @@ async fn group_list_global_view_is_attenuated_by_device_scope() { ); assert!(full.iter().any(|name| name == "ops")); - let global = names_for( + let global = group_names_for( handlers::dispatch_with_device( &kernel, &PrincipalId::default(), @@ -142,7 +177,7 @@ async fn group_list_global_view_is_attenuated_by_device_scope() { ); assert!(global.iter().any(|name| name == "ops")); - let scoped = names_for( + let scoped = group_names_for( handlers::dispatch_with_device( &kernel, &PrincipalId::default(), @@ -153,28 +188,5 @@ async fn group_list_global_view_is_attenuated_by_device_scope() { ); assert_eq!(scoped, vec![BUILTIN_ADMIN]); - let authorization = crate::kernel_router::authorize_request( - &kernel, - &PrincipalId::default(), - Some(&global_list_id), - "self:group:list", - ) - .expect("authorize group inventory"); - profile.auth.public_keys.clear(); - profile - .save_to_path(&profile_path) - .expect("revoke group inventory device"); - kernel.profile_cache.invalidate(&PrincipalId::default()); - crate::kernel_router::authorize_request( - &kernel, - &PrincipalId::default(), - Some(&global_list_id), - "self:group:list", - ) - .expect_err("a later request must observe the revoked device"); - - let pinned = names_for( - handlers::dispatch_authorized(&kernel, &authorization, AdminRequestKind::GroupList).await, - ); - assert!(pinned.iter().any(|name| name == "ops")); + assert_group_list_authorization_snapshot(&kernel, profile, &global_list_id).await; } diff --git a/crates/astrid-kernel/src/kernel_router/tests.rs b/crates/astrid-kernel/src/kernel_router/tests.rs index 2107f5308..2d06e189a 100644 --- a/crates/astrid-kernel/src/kernel_router/tests.rs +++ b/crates/astrid-kernel/src/kernel_router/tests.rs @@ -661,7 +661,7 @@ async fn capsule_visibility_precomputes_admin_and_capsule_grants() { let admin_authorization = authorize_request(&kernel, &admin, None, "self:capsule:list").expect("authorize admin"); let global_lister_authorization = - authorize_request(&kernel, &global_lister, None, "self:capsule:list") + authorize_request(&kernel, &global_lister, None, "capsule:list") .expect("authorize global lister"); let limited_authorization = authorize_request(&kernel, &limited, None, "self:capsule:list").expect("authorize limited"); From b93409cb9a2fe2503fd19b1808b4d3a1cce34cb0 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 15 Jul 2026 01:11:31 +0400 Subject: [PATCH 11/16] docs(test): clarify pairing snapshot coverage --- .../src/kernel_router/admin/pair_device_tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs b/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs index 286827bd8..b22a10af2 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs @@ -1,8 +1,8 @@ //! Acceptance tests for per-device capability scope on pairing. //! -//! These drive the kernel/handler path end-to-end (real `Kernel`, profiles on -//! disk, with the same authorization snapshot the IPC router uses) and -//! prove the no-escalation guarantees the feature exists to provide: +//! These drive the kernel/handler path end-to-end with a real `Kernel`, +//! disk-backed profiles, and the same authorization snapshot as the IPC router. +//! They prove the no-escalation guarantees the feature exists to provide: //! //! * a `use-only` device cannot mint pair-tokens (cap-gate denial), but the //! same principal can from a full device; From 5ca755a5a8fb2f59323e1ac1367d152d2fd4a841 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 15 Jul 2026 10:14:30 +0400 Subject: [PATCH 12/16] fix(kernel): validate pair issuance before audit --- CHANGELOG.md | 8 +- crates/astrid-audit/src/entry.rs | 7 +- crates/astrid-core/src/kernel_api/mod.rs | 26 +-- .../kernel_router/admin/enforcement_tests.rs | 188 +++++++++++++++++- .../src/kernel_router/admin/mod.rs | 89 ++++++--- .../admin/pair_device_handlers.rs | 143 +++++++++---- .../kernel_router/admin/pair_device_tests.rs | 85 ++++++++ .../src/kernel_router/admin/tests.rs | 38 +++- 8 files changed, 498 insertions(+), 86 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31e4ce7dc..d10fb813e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,9 +109,11 @@ Changelog tracking starts with 0.2.0. Prior versions were not tracked. - **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 - authority. Malformed, unknown, and revoked device IDs fail closed, and cache - invalidation prevents stale pre-revocation profiles from being republished. - Closes #1237. + authority. Explicit pair-scope allow and deny patterns pass canonical + validation before persistence, and structural no-escalation denials produce + a failure audit row without a preceding success row. Malformed, unknown, and + revoked device IDs fail closed, and cache invalidation prevents stale + pre-revocation profiles from being republished. Closes #1237. - Bumped the locked `crossbeam-epoch` transitive dependency to `0.9.20`, clearing `RUSTSEC-2026-0204` (published 2026-07-06 — an invalid pointer dereference in the `fmt::Pointer` / pre-0.9 `fmt::Display` impls for `Atomic` and `Shared` when the underlying pointer is null or invalid) from the RustSec audit. The advisory landed the day after the 0.9.3 release and turned the Security Audit CI check red on `main` and every open PR; nothing in our own code changed. Lockfile-only, same-minor patch — no other dependency moved. Closes #1161. ## [0.9.3] - 2026-07-06 diff --git a/crates/astrid-audit/src/entry.rs b/crates/astrid-audit/src/entry.rs index 5a43398c3..dfc2634a8 100644 --- a/crates/astrid-audit/src/entry.rs +++ b/crates/astrid-audit/src/entry.rs @@ -434,9 +434,10 @@ pub enum AuditAction { /// Configuration was reloaded. ConfigReloaded, - /// Kernel management-API request (admin surface) — allowed or denied - /// by the [`CapabilityCheck`](astrid_capabilities::CapabilityCheck) - /// enforcement preamble. Pair this action with + /// Kernel management-API request (admin surface) — allowed or denied by + /// the [`CapabilityCheck`](astrid_capabilities::CapabilityCheck) + /// enforcement preamble and any request-specific no-escalation checks. + /// Pair this action with /// [`AuthorizationProof::Denied`] + [`AuditOutcome::failure`] for the /// deny path, or any of the positive authorization variants + /// [`AuditOutcome::success`] for the allow path. diff --git a/crates/astrid-core/src/kernel_api/mod.rs b/crates/astrid-core/src/kernel_api/mod.rs index 3882d4a4f..cecee947b 100644 --- a/crates/astrid-core/src/kernel_api/mod.rs +++ b/crates/astrid-core/src/kernel_api/mod.rs @@ -325,7 +325,8 @@ impl From for AdminKernelRequest { /// on `PairDeviceIssue` defaults to [`PairScopeArg::Full`] when omitted, so /// pre-scope callers (and single-tenant admin flows) keep their existing /// behaviour — but minting a `Full` device additionally requires the issuer to -/// hold `self:auth:pair:admin`, enforced in the handler. +/// hold `self:auth:pair:admin`, enforced by the authorization preamble and +/// rechecked with the issuer's pinned policy snapshot before persistence. #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case", tag = "kind")] pub enum PairScopeArg { @@ -357,8 +358,8 @@ pub enum PairScopeArg { /// Serde default for [`AdminRequestKind::PairDeviceIssue::scope`] — `Full`, /// for back-compat with callers that predate the `scope` field. A `Full` mint -/// is independently gated on `self:auth:pair:admin` in the handler, so the -/// permissive *default* does not relax the *authority* required to use it. +/// is independently gated on `self:auth:pair:admin`, so the permissive +/// *default* does not relax the *authority* required to use it. fn default_pair_scope() -> PairScopeArg { PairScopeArg::Full } @@ -666,12 +667,13 @@ pub enum AdminRequestKind { /// The opaque token to invalidate. token: String, }, - /// Issue a pair-device token. Gated by `self:auth:pair` (the - /// caller can only mint pair-tokens for their own principal — - /// the kernel ignores any target field on the wire and ties the - /// token to the caller). Used to add a new device's ed25519 - /// public key to an existing principal's `AuthConfig.public_keys` - /// without minting a separate principal. + /// Issue a pair-device token. Scoped issuance is gated by + /// `self:auth:pair`; unattenuated issuance additionally requires + /// `self:auth:pair:admin`. The caller can only mint pair-tokens for their + /// own principal — the kernel ignores any target field on the wire and + /// ties the token to the caller. Used to add a new device's ed25519 public + /// key to an existing principal's `AuthConfig.public_keys` without minting + /// a separate principal. PairDeviceIssue { /// Seconds until the token expires. Capped server-side to /// 1 hour — pair-tokens are intended for immediate use on a @@ -686,9 +688,9 @@ pub enum AdminRequestKind { /// Capability scope the redeemed device will authenticate under. /// Defaults to [`PairScopeArg::Full`] when omitted, for back-compat /// with pre-scope callers; a `Full` mint is independently gated on - /// `self:auth:pair:admin` in the handler, and an `Explicit`/`Preset` - /// scope is validated to be a subset of the issuer's effective - /// capabilities (no escalation). + /// `self:auth:pair:admin`, and an `Explicit`/`Preset` scope is validated + /// to be a subset of the issuer's effective capabilities (no + /// escalation). #[serde(default = "default_pair_scope")] scope: PairScopeArg, }, 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 18590d421..70415002d 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/enforcement_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/enforcement_tests.rs @@ -14,7 +14,7 @@ use std::sync::Arc; -use astrid_audit::AuditAction; +use astrid_audit::{AuditAction, AuditOutcome, AuthorizationProof}; use astrid_core::dirs::AstridHome; use astrid_core::principal::PrincipalId; use astrid_core::profile::PrincipalProfile; @@ -323,6 +323,55 @@ async fn send_admin_scoped( .expect("admin response within 2s") } +async fn assert_pair_issue_audit( + kernel: &Arc, + expected_capability: &str, + expected_success: bool, +) { + let entries = kernel + .audit_log + .get_session_entries(&kernel.session_id) + .await + .expect("read audit chain"); + let pair_entries = entries + .iter() + .filter(|entry| { + matches!( + &entry.action, + AuditAction::AdminRequest { method, .. } + if method == "admin.auth.pair.issue" + ) + }) + .collect::>(); + assert_eq!( + pair_entries.len(), + 1, + "pair issuance must produce exactly one terminal audit row" + ); + let entry = pair_entries[0]; + let AuditAction::AdminRequest { + required_capability, + .. + } = &entry.action + else { + unreachable!("filtered to pair issue audit rows") + }; + assert_eq!(required_capability, expected_capability); + if expected_success { + assert!(matches!( + &entry.authorization, + AuthorizationProof::System { .. } + )); + assert!(matches!(&entry.outcome, AuditOutcome::Success { .. })); + } else { + assert!(matches!( + &entry.authorization, + AuthorizationProof::Denied { .. } + )); + assert!(matches!(&entry.outcome, AuditOutcome::Failure { .. })); + } +} + /// Seed a principal that holds `self:auth:pair` and registers two devices: a /// Full-scope device and a use-only device whose scope denies `self:auth:pair`. /// Returns `(full_key_id, scoped_key_id)`. @@ -362,7 +411,7 @@ async fn device_scope_denies_pair_issue_but_full_device_and_none_allow() { let caller = pid("paired_user"); let (full_id, scoped_id) = seed_principal_with_devices(&kernel, &caller); - // PairDeviceIssue is gated by `self:auth:pair`, which the principal holds. + // Full PairDeviceIssue is gated by pair-admin, which `self:*` admits. let req = || { AdminRequestKind::PairDeviceIssue { expires_secs: Some(300), @@ -431,3 +480,138 @@ async fn unknown_device_key_id_fails_closed() { "an unresolved device_key_id must fail closed: {resp}" ); } + +#[tokio::test(flavor = "multi_thread")] +async fn full_pair_mint_without_admin_capability_audits_denial() { + let (_dir, kernel) = fixture().await; + let caller = pid("pair_issue_only"); + seed_profile( + &kernel, + &caller, + &PrincipalProfile { + grants: vec!["self:auth:pair".into()], + enabled: true, + ..Default::default() + }, + ); + + let response = send_admin( + &kernel, + &caller, + "auth.pair.issue", + AdminRequestKind::PairDeviceIssue { + expires_secs: Some(300), + label: None, + scope: astrid_events::kernel_api::PairScopeArg::Full, + } + .into(), + ) + .await; + assert_eq!(response["status"], "Error", "got: {response}"); + assert_pair_issue_audit(&kernel, "self:auth:pair:admin", false).await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn scoped_issuer_full_mint_structural_denial_is_audited() { + use astrid_core::profile::{AuthMethod, DeviceKey, DeviceScope}; + + let (_dir, kernel) = fixture().await; + let caller = pid("scoped_pair_admin"); + let device = DeviceKey::new( + "c".repeat(64), + DeviceScope::Scoped { + allow: vec!["self:*".into()], + deny: vec![], + }, + None, + 0, + ); + let device_id = device.key_id.clone(); + let mut profile = PrincipalProfile { + grants: vec!["self:*".into()], + enabled: true, + ..Default::default() + }; + profile.auth.methods.push(AuthMethod::Keypair); + profile.auth.public_keys.push(device); + seed_profile(&kernel, &caller, &profile); + + let response = send_admin_scoped( + &kernel, + &caller, + Some(&device_id), + "auth.pair.issue", + AdminRequestKind::PairDeviceIssue { + expires_secs: Some(300), + label: None, + scope: astrid_events::kernel_api::PairScopeArg::Full, + } + .into(), + ) + .await; + assert_eq!(response["status"], "Error", "got: {response}"); + assert_pair_issue_audit(&kernel, "self:auth:pair:admin", false).await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn universal_pair_mint_subset_denial_is_audited() { + let (_dir, kernel) = fixture().await; + let caller = pid("bounded_pair_admin"); + seed_profile( + &kernel, + &caller, + &PrincipalProfile { + grants: vec!["self:auth:pair:admin".into()], + enabled: true, + ..Default::default() + }, + ); + + let response = send_admin( + &kernel, + &caller, + "auth.pair.issue", + AdminRequestKind::PairDeviceIssue { + expires_secs: Some(300), + label: None, + scope: astrid_events::kernel_api::PairScopeArg::Explicit { + allow: vec!["*".into()], + deny: vec![], + }, + } + .into(), + ) + .await; + assert_eq!(response["status"], "Error", "got: {response}"); + assert_pair_issue_audit(&kernel, "self:auth:pair:admin", false).await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn successful_full_pair_mint_audits_pair_admin_allow() { + let (_dir, kernel) = fixture().await; + let caller = pid("unattenuated_pair_admin"); + seed_profile( + &kernel, + &caller, + &PrincipalProfile { + grants: vec!["*".into()], + enabled: true, + ..Default::default() + }, + ); + + let response = send_admin( + &kernel, + &caller, + "auth.pair.issue", + AdminRequestKind::PairDeviceIssue { + expires_secs: Some(300), + label: None, + scope: astrid_events::kernel_api::PairScopeArg::Full, + } + .into(), + ) + .await; + assert_eq!(response["status"], "PairToken", "got: {response}"); + assert_pair_issue_audit(&kernel, "self:auth:pair:admin", true).await; +} diff --git a/crates/astrid-kernel/src/kernel_router/admin/mod.rs b/crates/astrid-kernel/src/kernel_router/admin/mod.rs index eaff29b32..0412f624c 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/mod.rs @@ -279,10 +279,14 @@ pub fn required_capability_for_admin_request( // audit-log readability. (AdminRequestKind::PairDeviceRedeem { .. }, _) => "auth:pair:redeem", // PairDeviceIssue is intrinsically self-scoped (kernel binds the - // token to the caller). Device list / revoke reuse the same pairing - // capability (no new base cap): a principal manages its own devices - // with `self:auth:pair`; an admin manages anyone's via the global - // `auth:pair`. + // token to the caller). Unattenuated scopes are escalation primitives + // and require the pair-admin capability in the common preamble; the + // handler independently enforces scope subset and attenuation rules. + (AdminRequestKind::PairDeviceIssue { scope, .. }, _) + if pair_device_handlers::pair_scope_requires_admin(scope) => + { + "self:auth:pair:admin" + }, (AdminRequestKind::PairDeviceIssue { .. }, _) | ( AdminRequestKind::PairDeviceList { .. } | AdminRequestKind::PairDeviceRevoke { .. }, @@ -548,25 +552,7 @@ async fn handle_admin_request( let authorization = match authorize_request(kernel, &caller, device_key_id.as_deref(), required_cap) { - Ok(authorization) => { - record_admin_audit( - kernel, - AdminAuditEntry { - caller: &caller, - method, - required_cap, - device_key_id: device_key_id.as_deref(), - target_principal: target.clone(), - params: audit_params.clone(), - authorization: AuthorizationProof::System { - reason: format!("policy allow: {caller} holds {required_cap}"), - }, - outcome: AuditOutcome::success(), - }, - ) - .await; - authorization - }, + Ok(authorization) => authorization, Err(e) => { warn!( security_event = true, @@ -604,6 +590,63 @@ async fn handle_admin_request( }, }; + if let AdminRequestKind::PairDeviceIssue { + expires_secs, + scope, + .. + } = &req.kind + && let Err(error) = + pair_device_handlers::preflight_pair_device_issue(&authorization, *expires_secs, scope) + { + warn!( + security_event = true, + method, + principal = %caller, + required = required_cap, + error = %error, + "Pair-device issuance denied by scope validation" + ); + record_admin_audit( + kernel, + AdminAuditEntry { + caller: &caller, + method, + required_cap, + device_key_id: device_key_id.as_deref(), + target_principal: target, + params: audit_params, + authorization: AuthorizationProof::Denied { + reason: error.clone(), + }, + outcome: AuditOutcome::failure(error.clone()), + }, + ) + .await; + publish_response( + kernel, + response_topic, + AdminKernelResponse::for_request(request_id, AdminResponseBody::Error(error)), + ); + return; + } + + record_admin_audit( + kernel, + AdminAuditEntry { + caller: &caller, + method, + required_cap, + device_key_id: device_key_id.as_deref(), + target_principal: target, + params: audit_params, + authorization: AuthorizationProof::System { + reason: format!("policy allow: {caller} holds {required_cap}"), + }, + outcome: AuditOutcome::success(), + }, + ) + .await; + let body = handlers::dispatch_authorized(kernel, &authorization, req.kind).await; publish_response( kernel, diff --git a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs index e16b77f05..99a12f3b1 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs @@ -26,7 +26,8 @@ use astrid_core::kernel_api::{ AdminResponseBody, DeviceKeyInfo, PairScopeArg, PairTokenIssued, PairTokenRedeemed, }; use astrid_core::profile::{ - AuthMethod, DeviceKey, DeviceKeyId, DevicePubkey, DeviceScope, PrincipalProfile, + AuthMethod, CapabilityPattern, DeviceKey, DeviceKeyId, DevicePubkey, DeviceScope, + PrincipalProfile, }; use tracing::{info, warn}; @@ -93,46 +94,17 @@ pub(super) async fn pair_device_issue( Ok(authority) => authority, Err(response) => return response, }; - let mut issuer_check = CapabilityCheck::new(profile.as_ref(), groups.as_ref(), caller.clone()); - // A scoped issuer can grant only its attenuated authority. - if let Some(scope @ DeviceScope::Scoped { .. }) = &issuer_scope { - issuer_check = issuer_check.with_device_scope(scope); - } - - match &requested_scope { - // Full devices require an unattenuated issuer with pair-admin authority. - DeviceScope::Full => { - if matches!(&issuer_scope, Some(DeviceScope::Scoped { .. })) { - return err_unauthorized(format!( - "a scoped device cannot mint a full-scope (unattenuated) device; issue a scoped device instead — {caller}" - )); - } - if !issuer_check.has("self:auth:pair:admin") { - return err_unauthorized(format!( - "minting a full-scope device requires self:auth:pair:admin, which {caller} does not effectively hold" - )); - } - }, - DeviceScope::Scoped { allow, .. } => { - // Bare `*` confers the principal's full authority even in a scoped shape. - let requests_universal_scope = allow.iter().any(|pattern| pattern == "*"); - if requests_universal_scope && !issuer_check.has("self:auth:pair:admin") { - return err_unauthorized(format!( - "minting a device scope with universal `*` requires self:auth:pair:admin, which {caller} does not effectively hold" - )); - } - if let Err(e) = device_scope_within(&issuer_check, &requested_scope) { - return err_unauthorized(format!( - "requested device scope exceeds your authority: {e}" - )); - } - }, - } - - // A scoped child inherits its issuer's denies. - let stored_scope = inherit_issuer_denies(requested_scope, issuer_scope.as_ref()); - // Release authority state before awaiting the write lock. - drop(issuer_check); + let stored_scope = match validate_pair_issue_authority( + caller, + profile.as_ref(), + groups.as_ref(), + issuer_scope.as_ref(), + &requested_scope, + ) { + Ok(scope) => scope, + Err(error) => return err_unauthorized(error), + }; + // Release the pinned authority snapshot before awaiting the write lock. drop(profile); drop(groups); @@ -192,7 +164,94 @@ fn validate_pair_issue( )); } - resolve_pair_scope(requested).map(|scope| (lifetime, scope)) + let scope = resolve_pair_scope(requested)?; + validate_pair_scope_patterns(&scope)?; + Ok((lifetime, scope)) +} + +/// Return whether the requested scope confers unattenuated authority and must +/// therefore pass the pair-admin capability preamble. +pub(super) fn pair_scope_requires_admin(scope: &PairScopeArg) -> bool { + match scope { + PairScopeArg::Full => true, + PairScopeArg::Preset { name } => { + matches!(DeviceScope::preset(name), Some(DeviceScope::Full)) + }, + PairScopeArg::Explicit { allow, .. } => allow.iter().any(|pattern| pattern == "*"), + } +} + +/// Validate pair issuance against the exact policy snapshot pinned by the +/// router before it records an authorization success audit row. +pub(super) fn preflight_pair_device_issue( + authorization: &AuthorizedRequest, + expires_secs: Option, + requested: &PairScopeArg, +) -> Result<(), String> { + let (_, requested_scope) = validate_pair_issue(expires_secs, requested)?; + validate_pair_issue_authority( + &authorization.principal, + authorization.profile.as_ref(), + authorization.groups.as_ref(), + authorization.device_scope.as_ref(), + &requested_scope, + )?; + Ok(()) +} + +fn validate_pair_issue_authority( + caller: &PrincipalId, + profile: &PrincipalProfile, + groups: &GroupConfig, + issuer_scope: Option<&DeviceScope>, + requested_scope: &DeviceScope, +) -> Result { + let mut issuer_check = CapabilityCheck::new(profile, groups, caller.clone()); + if let Some(scope @ DeviceScope::Scoped { .. }) = issuer_scope { + issuer_check = issuer_check.with_device_scope(scope); + } + + match requested_scope { + DeviceScope::Full => { + if matches!(issuer_scope, Some(DeviceScope::Scoped { .. })) { + return Err(format!( + "a scoped device cannot mint a full-scope (unattenuated) device; issue a scoped device instead — {caller}" + )); + } + if !issuer_check.has("self:auth:pair:admin") { + return Err(format!( + "minting a full-scope device requires self:auth:pair:admin, which {caller} does not effectively hold" + )); + } + }, + DeviceScope::Scoped { allow, .. } => { + let requests_universal_scope = allow.iter().any(|pattern| pattern == "*"); + if requests_universal_scope && !issuer_check.has("self:auth:pair:admin") { + return Err(format!( + "minting a device scope with universal `*` requires self:auth:pair:admin, which {caller} does not effectively hold" + )); + } + if let Err(error) = device_scope_within(&issuer_check, requested_scope) { + return Err(format!( + "requested device scope exceeds your authority: {error}" + )); + } + }, + } + + Ok(inherit_issuer_denies(requested_scope.clone(), issuer_scope)) +} + +fn validate_pair_scope_patterns(scope: &DeviceScope) -> Result<(), String> { + let DeviceScope::Scoped { allow, deny } = scope else { + return Ok(()); + }; + for pattern in allow.iter().chain(deny) { + CapabilityPattern::new(pattern.as_str()).map_err(|error| { + format!("invalid device scope capability pattern {pattern:?}: {error}") + })?; + } + Ok(()) } /// Resolve a [`PairScopeArg`] to a concrete [`DeviceScope`]. An unknown preset diff --git a/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs b/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs index b22a10af2..48d477c32 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/pair_device_tests.rs @@ -28,6 +28,7 @@ use tempfile::TempDir; use super::handlers; use crate::Kernel; +use crate::pair_token::PairTokenStore; async fn fixture() -> (TempDir, Arc) { let dir = tempfile::tempdir().expect("tempdir"); @@ -366,6 +367,90 @@ async fn over_broad_scope_rejected_at_issue() { ); } +#[tokio::test(flavor = "multi_thread")] +async fn malformed_explicit_patterns_are_rejected_before_persistence() { + let (_dir, kernel) = fixture().await; + let caller = pid("malformed_scope_issuer"); + seed(&kernel, &caller, &["self:*"], vec![]); + + for scope in [ + PairScopeArg::Explicit { + allow: vec!["self:capsule;install".into()], + deny: vec![], + }, + PairScopeArg::Explicit { + allow: vec!["self:capsule:reload".into()], + deny: vec!["self:capsule;install".into()], + }, + ] { + let response = handlers::dispatch_with_device(&kernel, &caller, None, issue(scope)).await; + match response { + AdminResponseBody::Error(message) => assert!( + message.contains("invalid device scope capability pattern"), + "canonical capability validation must reject the scope: {message}" + ), + other => panic!("malformed explicit scope must reject, got: {other:?}"), + } + } + + let store = PairTokenStore::new(PairTokenStore::path_for(&kernel.astrid_home)); + assert!( + store.load().expect("load pair-token store").is_empty(), + "invalid allow or deny patterns must never be persisted" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn valid_explicit_allow_and_deny_survive_issue_and_redeem() { + let (_dir, kernel) = fixture().await; + let caller = pid("explicit_scope_issuer"); + seed( + &kernel, + &caller, + &["self:auth:pair", "self:capsule:reload"], + vec![], + ); + let response = handlers::dispatch_with_device( + &kernel, + &caller, + None, + issue(PairScopeArg::Explicit { + allow: vec!["self:capsule:reload".into()], + deny: vec!["self:capsule:install".into()], + }), + ) + .await; + let token = match response { + AdminResponseBody::PairToken(token) => token.token, + other => panic!("valid explicit scope must issue, got: {other:?}"), + }; + + let public_key = "d".repeat(64); + let response = handlers::dispatch( + &kernel, + &PrincipalId::default(), + AdminRequestKind::PairDeviceRedeem { + token, + public_key: public_key.clone(), + }, + ) + .await; + assert!(matches!(response, AdminResponseBody::PairTokenRedeemed(_))); + + let profile = load(&kernel, &caller); + let child = profile + .auth + .device_by_key_id(&device_key_id_fingerprint(&public_key)) + .expect("redeemed device"); + assert_eq!( + child.scope, + DeviceScope::Scoped { + allow: vec!["self:capsule:reload".into()], + deny: vec!["self:capsule:install".into()], + } + ); +} + // ── Coordinator correction: deny-inheritance (monotonic narrowing) ──── #[tokio::test(flavor = "multi_thread")] diff --git a/crates/astrid-kernel/src/kernel_router/admin/tests.rs b/crates/astrid-kernel/src/kernel_router/admin/tests.rs index 9781cb34c..8dcd20ff7 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/tests.rs @@ -17,7 +17,7 @@ use std::sync::Arc; use astrid_capabilities::CapabilityCheck; use astrid_core::principal::PrincipalId; use astrid_core::{GroupConfig, PrincipalProfile}; -use astrid_events::kernel_api::AdminRequestKind; +use astrid_events::kernel_api::{AdminRequestKind, PairScopeArg}; use super::{ AuthorityScope, admin_request_method, admin_response_topic, admin_target_principal, @@ -162,6 +162,42 @@ fn agent_list_maps_self_to_self_prefix() { ); } +#[test] +fn pair_issue_mapping_requires_admin_only_for_unattenuated_scopes() { + for scope in [ + PairScopeArg::Full, + PairScopeArg::Preset { + name: "full".into(), + }, + PairScopeArg::Explicit { + allow: vec!["*".into()], + deny: vec![], + }, + ] { + let request = AdminRequestKind::PairDeviceIssue { + expires_secs: None, + label: None, + scope, + }; + assert_eq!( + required_capability_for_admin_request(&request, AuthorityScope::Self_), + "self:auth:pair:admin" + ); + } + + let scoped = AdminRequestKind::PairDeviceIssue { + expires_secs: None, + label: None, + scope: PairScopeArg::Preset { + name: "use-only".into(), + }, + }; + assert_eq!( + required_capability_for_admin_request(&scoped, AuthorityScope::Self_), + "self:auth:pair" + ); +} + #[test] fn agent_create_mapping_uses_granular_clone_and_inherit_caps() { assert_eq!( From 810b3ec26b58de1fc49697118f401678dec74437 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 15 Jul 2026 10:29:29 +0400 Subject: [PATCH 13/16] refactor(kernel): isolate redeem audit flow --- .../src/kernel_router/admin/mod.rs | 92 +++++++++---------- 1 file changed, 44 insertions(+), 48 deletions(-) diff --git a/crates/astrid-kernel/src/kernel_router/admin/mod.rs b/crates/astrid-kernel/src/kernel_router/admin/mod.rs index 0412f624c..88e8c5a05 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/mod.rs @@ -480,6 +480,42 @@ fn redeem_audit_proof(body: &AdminResponseBody) -> (AuthorizationProof, AuditOut } } +/// Redeem requests use the token as authorization, so dispatch must complete +/// before the audit row can record the real allow-or-deny outcome. +async fn handle_redeem_admin_request( + kernel: &Arc, + response_topic: Topic, + request_id: Option, + caller: PrincipalId, + kind: AdminRequestKind, +) { + let method = admin_request_method(&kind); + let required_cap = + required_capability_for_admin_request(&kind, resolve_admin_scope(&kind, &caller)); + let audit_params = sanitize_admin_audit_params(&kind); + let body = handlers::dispatch(kernel, &caller, kind).await; + let (authorization, outcome) = redeem_audit_proof(&body); + record_admin_audit( + kernel, + AdminAuditEntry { + caller: &caller, + method, + required_cap, + device_key_id: None, + target_principal: None, + params: audit_params, + authorization, + outcome, + }, + ) + .await; + publish_response( + kernel, + response_topic, + AdminKernelResponse::for_request(request_id, body), + ); +} + async fn handle_admin_request( kernel: &Arc, topic: Topic, @@ -489,6 +525,14 @@ async fn handle_admin_request( ) { let response_topic = admin_response_topic(&topic); let request_id = req.request_id.clone(); + if matches!( + req.kind, + AdminRequestKind::InviteRedeem { .. } | AdminRequestKind::PairDeviceRedeem { .. } + ) { + handle_redeem_admin_request(kernel, response_topic, request_id, caller, req.kind).await; + return; + } + let method = admin_request_method(&req.kind); let scope = resolve_admin_scope(&req.kind, &caller); let required_cap = required_capability_for_admin_request(&req.kind, scope); @@ -502,54 +546,6 @@ async fn handle_admin_request( // lives on `AuthConfig.public_keys`. let audit_params = sanitize_admin_audit_params(&req.kind); - // `InviteRedeem` / `PairDeviceRedeem` are the two variants that - // bypass the capability preamble: the redeemer's principal does not - // exist yet at the moment the request arrives — the token IS the - // auth. The handler verifies the token internally and either mints a - // principal (success) or rejects it (invalid / expired / consumed / - // forged token, or an internal store error). - // - // Dispatch FIRST, then audit with the REAL outcome. Stamping - // `success` before dispatch (as this used to) meant a rejected token - // still wrote a success row, so brute-force / forged-token attempts - // were invisible in the audit log itself — only in tracing. - // Recording after dispatch makes the row's outcome match what - // actually happened: this is the "allow OR deny" the comment always - // promised. The handler still emits its own `security_event` warn on - // rejection; this adds the missing audit-store signal so the - // security team can detect token brute-forcing from audit rows alone. - if matches!( - req.kind, - AdminRequestKind::InviteRedeem { .. } | AdminRequestKind::PairDeviceRedeem { .. } - ) { - // Redeem variants carry no issuer device scope — the token is the - // auth, not a paired device. - let body = handlers::dispatch(kernel, &caller, req.kind).await; - let (authorization, outcome) = redeem_audit_proof(&body); - record_admin_audit( - kernel, - AdminAuditEntry { - caller: &caller, - method, - required_cap, - // Redeems mint an identity and carry no device scope — the - // token is the auth, not a paired device. - device_key_id: None, - target_principal: None, - params: audit_params, - authorization, - outcome, - }, - ) - .await; - publish_response( - kernel, - response_topic, - AdminKernelResponse::for_request(request_id, body), - ); - return; - } - let authorization = match authorize_request(kernel, &caller, device_key_id.as_deref(), required_cap) { Ok(authorization) => authorization, From d394345ccd491a6d1bca0911701d330a59791fc0 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 15 Jul 2026 10:37:49 +0400 Subject: [PATCH 14/16] refactor(kernel): centralize admin authorization audit --- .../src/kernel_router/admin/mod.rs | 208 ++++++++++-------- 1 file changed, 112 insertions(+), 96 deletions(-) diff --git a/crates/astrid-kernel/src/kernel_router/admin/mod.rs b/crates/astrid-kernel/src/kernel_router/admin/mod.rs index 88e8c5a05..aa3f2ec37 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/mod.rs @@ -516,6 +516,99 @@ async fn handle_redeem_admin_request( ); } +struct AdminAuthorizationContext<'a> { + caller: &'a PrincipalId, + device_key_id: Option<&'a str>, + kind: &'a AdminRequestKind, + method: &'static str, + required_cap: &'static str, + target_principal: Option<&'a PrincipalId>, + audit_params: Option<&'a serde_json::Value>, +} + +async fn record_admin_authorization_failure( + kernel: &Arc, + context: &AdminAuthorizationContext<'_>, + error: &str, +) { + warn!( + security_event = true, + method = context.method, + principal = %context.caller, + required = context.required_cap, + error, + "Permission check denied admin request" + ); + record_admin_audit( + kernel, + AdminAuditEntry { + caller: context.caller, + method: context.method, + required_cap: context.required_cap, + device_key_id: context.device_key_id, + target_principal: context.target_principal.cloned(), + params: context.audit_params.cloned(), + authorization: AuthorizationProof::Denied { + reason: error.to_string(), + }, + outcome: AuditOutcome::failure(error), + }, + ) + .await; +} + +async fn authorize_admin_request( + kernel: &Arc, + context: &AdminAuthorizationContext<'_>, +) -> Result { + let authorization = match authorize_request( + kernel, + context.caller, + context.device_key_id, + context.required_cap, + ) { + Ok(authorization) => authorization, + Err(error) => { + let error = error.to_string(); + record_admin_authorization_failure(kernel, context, &error).await; + return Err(error); + }, + }; + + if let AdminRequestKind::PairDeviceIssue { + expires_secs, + scope, + .. + } = context.kind + && let Err(error) = + pair_device_handlers::preflight_pair_device_issue(&authorization, *expires_secs, scope) + { + record_admin_authorization_failure(kernel, context, &error).await; + return Err(error); + } + + record_admin_audit( + kernel, + AdminAuditEntry { + caller: context.caller, + method: context.method, + required_cap: context.required_cap, + device_key_id: context.device_key_id, + target_principal: context.target_principal.cloned(), + params: context.audit_params.cloned(), + authorization: AuthorizationProof::System { + reason: format!( + "policy allow: {} holds {}", + context.caller, context.required_cap + ), + }, + outcome: AuditOutcome::success(), + }, + ) + .await; + Ok(authorization) +} + async fn handle_admin_request( kernel: &Arc, topic: Topic, @@ -545,103 +638,26 @@ async fn handle_admin_request( // later mistake for a system-of-record entry — the canonical copy // lives on `AuthConfig.public_keys`. let audit_params = sanitize_admin_audit_params(&req.kind); - - let authorization = - match authorize_request(kernel, &caller, device_key_id.as_deref(), required_cap) { - Ok(authorization) => authorization, - Err(e) => { - warn!( - security_event = true, - method = method, - principal = %caller, - required = required_cap, - error = %e, - "Permission check denied admin request" - ); - record_admin_audit( - kernel, - AdminAuditEntry { - caller: &caller, - method, - required_cap, - device_key_id: device_key_id.as_deref(), - target_principal: target, - params: audit_params, - authorization: AuthorizationProof::Denied { - reason: e.to_string(), - }, - outcome: AuditOutcome::failure(e.to_string()), - }, - ) - .await; - publish_response( - kernel, - response_topic, - AdminKernelResponse::for_request( - request_id, - AdminResponseBody::Error(e.to_string()), - ), - ); - return; - }, - }; - - if let AdminRequestKind::PairDeviceIssue { - expires_secs, - scope, - .. - } = &req.kind - && let Err(error) = - pair_device_handlers::preflight_pair_device_issue(&authorization, *expires_secs, scope) - { - warn!( - security_event = true, - method, - principal = %caller, - required = required_cap, - error = %error, - "Pair-device issuance denied by scope validation" - ); - record_admin_audit( - kernel, - AdminAuditEntry { - caller: &caller, - method, - required_cap, - device_key_id: device_key_id.as_deref(), - target_principal: target, - params: audit_params, - authorization: AuthorizationProof::Denied { - reason: error.clone(), - }, - outcome: AuditOutcome::failure(error.clone()), - }, - ) - .await; - publish_response( - kernel, - response_topic, - AdminKernelResponse::for_request(request_id, AdminResponseBody::Error(error)), - ); - return; - } - - record_admin_audit( - kernel, - AdminAuditEntry { - caller: &caller, - method, - required_cap, - device_key_id: device_key_id.as_deref(), - target_principal: target, - params: audit_params, - authorization: AuthorizationProof::System { - reason: format!("policy allow: {caller} holds {required_cap}"), - }, - outcome: AuditOutcome::success(), + let context = AdminAuthorizationContext { + caller: &caller, + device_key_id: device_key_id.as_deref(), + kind: &req.kind, + method, + required_cap, + target_principal: target.as_ref(), + audit_params: audit_params.as_ref(), + }; + let authorization = match authorize_admin_request(kernel, &context).await { + Ok(authorization) => authorization, + Err(error) => { + publish_response( + kernel, + response_topic, + AdminKernelResponse::for_request(request_id, AdminResponseBody::Error(error)), + ); + return; }, - ) - .await; + }; let body = handlers::dispatch_authorized(kernel, &authorization, req.kind).await; publish_response( From 7c42b8356b8dff4910cb8099970c5e31e6dbfb49 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 15 Jul 2026 10:44:11 +0400 Subject: [PATCH 15/16] fix(capsule): refresh idempotent profile cache --- crates/astrid-capsule/src/profile_cache.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/astrid-capsule/src/profile_cache.rs b/crates/astrid-capsule/src/profile_cache.rs index 623b35ffa..b6dcf124a 100644 --- a/crates/astrid-capsule/src/profile_cache.rs +++ b/crates/astrid-capsule/src/profile_cache.rs @@ -221,10 +221,11 @@ impl PrincipalProfileCache { .or_default(); if entries.iter().any(|e| e.eq_ignore_ascii_case(endpoint)) { - // Already persisted for this capsule — idempotent. Drop the stale - // cache entry so a reload reflects on-disk state, then return - // success. - guard.invalidate(principal); + // Already persisted for this capsule — idempotent. The profile was + // just loaded from disk while holding the cache write lock, so it + // is the current system-of-record value and can refresh the cache + // directly without a generation bump or another disk read. + guard.profiles.insert(principal.clone(), Arc::new(profile)); return Ok(()); } @@ -586,9 +587,10 @@ mod tests { cache .persist_egress(&p, "react", "10.0.0.5:8080") .expect("idempotent persist"); - assert_eq!(cache.generation_for(&p), persisted_generation + 1); + assert_eq!(cache.generation_for(&p), persisted_generation); + assert_eq!(cache.len(), 1, "idempotent persist refreshes the cache"); - let profile = cache.resolve(&p).expect("reload"); + let profile = cache.resolve(&p).expect("resolve refreshed profile"); assert_eq!( profile.network.capsule_egress.get("react"), Some(&vec!["10.0.0.5:8080".to_string()]), From b7c5bec4c20cb09cd7df404d29534fb92ac10584 Mon Sep 17 00:00:00 2001 From: "Joshua J. Bouw" Date: Wed, 15 Jul 2026 10:57:44 +0400 Subject: [PATCH 16/16] fix(kernel): classify pair issue audit failures --- crates/astrid-audit/src/entry.rs | 9 +- .../kernel_router/admin/enforcement_tests.rs | 109 +++++++++++++++--- .../src/kernel_router/admin/mod.rs | 57 +++++++-- .../admin/pair_device_handlers.rs | 17 ++- 4 files changed, 157 insertions(+), 35 deletions(-) diff --git a/crates/astrid-audit/src/entry.rs b/crates/astrid-audit/src/entry.rs index dfc2634a8..5ef27fcca 100644 --- a/crates/astrid-audit/src/entry.rs +++ b/crates/astrid-audit/src/entry.rs @@ -437,10 +437,11 @@ pub enum AuditAction { /// Kernel management-API request (admin surface) — allowed or denied by /// the [`CapabilityCheck`](astrid_capabilities::CapabilityCheck) /// enforcement preamble and any request-specific no-escalation checks. - /// Pair this action with - /// [`AuthorizationProof::Denied`] + [`AuditOutcome::failure`] for the - /// deny path, or any of the positive authorization variants + - /// [`AuditOutcome::success`] for the allow path. + /// Pair this action with [`AuthorizationProof::Denied`] plus + /// [`AuditOutcome::failure`] for an authorization denial. A request that + /// passes authorization but fails request-shape validation carries a + /// positive authorization proof plus a failure outcome; a completed allow + /// path carries a positive proof plus [`AuditOutcome::success`]. AdminRequest { /// Request variant name (e.g. `"Shutdown"`, `"ReloadCapsules"`). method: String, 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 70415002d..844eabb76 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/enforcement_tests.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/enforcement_tests.rs @@ -323,10 +323,17 @@ async fn send_admin_scoped( .expect("admin response within 2s") } +#[derive(Clone, Copy)] +enum PairIssueAuditExpectation { + Success, + Denied, + BadInput, +} + async fn assert_pair_issue_audit( kernel: &Arc, expected_capability: &str, - expected_success: bool, + expectation: PairIssueAuditExpectation, ) { let entries = kernel .audit_log @@ -357,18 +364,28 @@ async fn assert_pair_issue_audit( unreachable!("filtered to pair issue audit rows") }; assert_eq!(required_capability, expected_capability); - if expected_success { - assert!(matches!( - &entry.authorization, - AuthorizationProof::System { .. } - )); - assert!(matches!(&entry.outcome, AuditOutcome::Success { .. })); - } else { - assert!(matches!( - &entry.authorization, - AuthorizationProof::Denied { .. } - )); - assert!(matches!(&entry.outcome, AuditOutcome::Failure { .. })); + match expectation { + PairIssueAuditExpectation::Success => { + assert!(matches!( + &entry.authorization, + AuthorizationProof::System { .. } + )); + assert!(matches!(&entry.outcome, AuditOutcome::Success { .. })); + }, + PairIssueAuditExpectation::Denied => { + assert!(matches!( + &entry.authorization, + AuthorizationProof::Denied { .. } + )); + assert!(matches!(&entry.outcome, AuditOutcome::Failure { .. })); + }, + PairIssueAuditExpectation::BadInput => { + assert!(matches!( + &entry.authorization, + AuthorizationProof::System { .. } + )); + assert!(matches!(&entry.outcome, AuditOutcome::Failure { .. })); + }, } } @@ -508,7 +525,12 @@ async fn full_pair_mint_without_admin_capability_audits_denial() { ) .await; assert_eq!(response["status"], "Error", "got: {response}"); - assert_pair_issue_audit(&kernel, "self:auth:pair:admin", false).await; + assert_pair_issue_audit( + &kernel, + "self:auth:pair:admin", + PairIssueAuditExpectation::Denied, + ) + .await; } #[tokio::test(flavor = "multi_thread")] @@ -550,7 +572,12 @@ async fn scoped_issuer_full_mint_structural_denial_is_audited() { ) .await; assert_eq!(response["status"], "Error", "got: {response}"); - assert_pair_issue_audit(&kernel, "self:auth:pair:admin", false).await; + assert_pair_issue_audit( + &kernel, + "self:auth:pair:admin", + PairIssueAuditExpectation::Denied, + ) + .await; } #[tokio::test(flavor = "multi_thread")] @@ -583,7 +610,50 @@ async fn universal_pair_mint_subset_denial_is_audited() { ) .await; assert_eq!(response["status"], "Error", "got: {response}"); - assert_pair_issue_audit(&kernel, "self:auth:pair:admin", false).await; + assert_pair_issue_audit( + &kernel, + "self:auth:pair:admin", + PairIssueAuditExpectation::Denied, + ) + .await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn malformed_pair_scope_audits_bad_input_not_permission_denial() { + let (_dir, kernel) = fixture().await; + let caller = pid("malformed_pair_scope"); + seed_profile( + &kernel, + &caller, + &PrincipalProfile { + grants: vec!["self:auth:pair".into(), "self:capsule:reload".into()], + enabled: true, + ..Default::default() + }, + ); + + let response = send_admin( + &kernel, + &caller, + "auth.pair.issue", + AdminRequestKind::PairDeviceIssue { + expires_secs: Some(300), + label: None, + scope: astrid_events::kernel_api::PairScopeArg::Explicit { + allow: vec!["self:capsule:reload".into()], + deny: vec!["self:capsule;install".into()], + }, + } + .into(), + ) + .await; + assert_eq!(response["status"], "Error", "got: {response}"); + assert_pair_issue_audit( + &kernel, + "self:auth:pair", + PairIssueAuditExpectation::BadInput, + ) + .await; } #[tokio::test(flavor = "multi_thread")] @@ -613,5 +683,10 @@ async fn successful_full_pair_mint_audits_pair_admin_allow() { ) .await; assert_eq!(response["status"], "PairToken", "got: {response}"); - assert_pair_issue_audit(&kernel, "self:auth:pair:admin", true).await; + assert_pair_issue_audit( + &kernel, + "self:auth:pair:admin", + PairIssueAuditExpectation::Success, + ) + .await; } diff --git a/crates/astrid-kernel/src/kernel_router/admin/mod.rs b/crates/astrid-kernel/src/kernel_router/admin/mod.rs index aa3f2ec37..d2ce72f33 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/mod.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/mod.rs @@ -526,6 +526,15 @@ struct AdminAuthorizationContext<'a> { audit_params: Option<&'a serde_json::Value>, } +fn allowed_admin_authorization(context: &AdminAuthorizationContext<'_>) -> AuthorizationProof { + AuthorizationProof::System { + reason: format!( + "policy allow: {} holds {}", + context.caller, context.required_cap + ), + } +} + async fn record_admin_authorization_failure( kernel: &Arc, context: &AdminAuthorizationContext<'_>, @@ -557,6 +566,27 @@ async fn record_admin_authorization_failure( .await; } +async fn record_admin_bad_input_failure( + kernel: &Arc, + context: &AdminAuthorizationContext<'_>, + error: &str, +) { + record_admin_audit( + kernel, + AdminAuditEntry { + caller: context.caller, + method: context.method, + required_cap: context.required_cap, + device_key_id: context.device_key_id, + target_principal: context.target_principal.cloned(), + params: context.audit_params.cloned(), + authorization: allowed_admin_authorization(context), + outcome: AuditOutcome::failure(error), + }, + ) + .await; +} + async fn authorize_admin_request( kernel: &Arc, context: &AdminAuthorizationContext<'_>, @@ -575,16 +605,26 @@ async fn authorize_admin_request( }, }; - if let AdminRequestKind::PairDeviceIssue { + let preflight = if let AdminRequestKind::PairDeviceIssue { expires_secs, scope, .. } = context.kind - && let Err(error) = - pair_device_handlers::preflight_pair_device_issue(&authorization, *expires_secs, scope) { - record_admin_authorization_failure(kernel, context, &error).await; - return Err(error); + pair_device_handlers::preflight_pair_device_issue(&authorization, *expires_secs, scope) + } else { + Ok(()) + }; + match preflight { + Ok(()) => {}, + Err(pair_device_handlers::PairIssuePreflightError::BadInput(error)) => { + record_admin_bad_input_failure(kernel, context, &error).await; + return Err(error); + }, + Err(pair_device_handlers::PairIssuePreflightError::Unauthorized(error)) => { + record_admin_authorization_failure(kernel, context, &error).await; + return Err(error); + }, } record_admin_audit( @@ -596,12 +636,7 @@ async fn authorize_admin_request( device_key_id: context.device_key_id, target_principal: context.target_principal.cloned(), params: context.audit_params.cloned(), - authorization: AuthorizationProof::System { - reason: format!( - "policy allow: {} holds {}", - context.caller, context.required_cap - ), - }, + authorization: allowed_admin_authorization(context), outcome: AuditOutcome::success(), }, ) diff --git a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs index 99a12f3b1..d2c8f3c56 100644 --- a/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs +++ b/crates/astrid-kernel/src/kernel_router/admin/pair_device_handlers.rs @@ -181,21 +181,32 @@ pub(super) fn pair_scope_requires_admin(scope: &PairScopeArg) -> bool { } } +/// Pair-issuance failure class used to keep audit authorization and operation +/// outcomes distinct without changing the public error wire shape. +pub(super) enum PairIssuePreflightError { + /// The caller was authorized, but the requested expiry or scope is invalid. + BadInput(String), + /// The requested scope exceeds the issuer's effective authority. + Unauthorized(String), +} + /// Validate pair issuance against the exact policy snapshot pinned by the /// router before it records an authorization success audit row. pub(super) fn preflight_pair_device_issue( authorization: &AuthorizedRequest, expires_secs: Option, requested: &PairScopeArg, -) -> Result<(), String> { - let (_, requested_scope) = validate_pair_issue(expires_secs, requested)?; +) -> Result<(), PairIssuePreflightError> { + let (_, requested_scope) = + validate_pair_issue(expires_secs, requested).map_err(PairIssuePreflightError::BadInput)?; validate_pair_issue_authority( &authorization.principal, authorization.profile.as_ref(), authorization.groups.as_ref(), authorization.device_scope.as_ref(), &requested_scope, - )?; + ) + .map_err(PairIssuePreflightError::Unauthorized)?; Ok(()) }