refactor(authorization): enforce tenant isolation via ScopedFilesystem in FilesystemCapabilityLeaseStore

Per the systemic finding tracked in
`docs/plans/2026-05-16-scoped-filesystem-tenant-isolation.md` (HIGH on
PR #3679, mirror of the engine FilesystemStore fix in commit
`ac8e677f9`): `FilesystemCapabilityLeaseStore` was taking `&'a F: RootFilesystem`
and hand-formatting every lease path with
`/engine/tenants/<tenant_id>/users/<user_id>/...` prefixes. Any composition
layer that wrapped the backend without remembering to re-prefix would
leak across tenants, and the type system stayed silent.

Migrated the store so tenant isolation is structural, not a convention
the path builders have to remember:

- `FilesystemCapabilityLeaseStore::new` now takes
  `&'a ScopedFilesystem<F>` (preserves the existing `'a`-borrow shape;
  no `Arc` wrapping at the call site).
- Path helpers return `ScopedPath` rooted at the `/authorization`
  mount alias. `lease_invocation_root` / `lease_tenant_user_root` /
  `scoped_owner_root` are gone; the leading
  `/engine/tenants/{tenant_id}/users/{user_id}` prefix is now the
  MountView's responsibility, not this crate's. Within-tenant scope
  (agent/project/mission/thread/invocation) stays in the path.
- `CapabilityLeaseIndex` now stores `Vec<ScopedPath>` (was
  `Vec<VirtualPath>`), so the indexed-listing fast path stays
  alias-relative end-to-end.
- The `list_dir` → child-`get` flow strips the leaf name from the
  returned `VirtualPath` and rebuilds a `ScopedPath` under the owner
  prefix (same shape as `FilesystemStore::list_subdir_names` in
  `ironclaw_engine`), so the follow-up `get` still re-runs the per-op
  ACL check.

Preserved verbatim:

- The CAS-Version retry pattern in `update_lease_cas` (H4 fix from
  commit `4eccad56d`) — `revoke` / `claim` / `consume` still re-read
  + retry on `FilesystemError::VersionMismatch`.
- The `Unsupported → CasExpectation::Any` fallback in
  `write_lease_raw` for byte-only backends (also H4) — adapted to
  operate on `ScopedFilesystem`, same logic.
- `read_lease_index(...).await?.unwrap_or_default()` (H5 fix) and the
  rest of the idempotent revoke / claim-consume race rejection
  semantics.
- The `CapabilityLeaseStore` trait surface is unchanged.

Tests:

- `capability_lease_contract.rs` now builds a `ScopedFilesystem` with
  `MountPermissions::read_write_list_delete()` on alias
  `/authorization` → tenant-scoped target.
  `CountingFilesystem` keeps its role in
  `filesystem_lease_store_lists_from_owner_index_without_scanning_invocation_roots`
  by wrapping `LocalFilesystem` *inside* the `ScopedFilesystem`.
- New regression test
  `filesystem_capability_lease_store_isolates_two_tenants_with_same_user_project_ids`:
  two stores share one `InMemoryBackend` but have different
  `MountView`s; issuing a lease under tenant A's `(user_id,
  project_id, invocation_id)` triple must NOT make it visible from
  tenant B even when tenant B queries with tenant A's scope.

All 26 contract tests + 5 DB-backed store tests pass; `cargo clippy
-p ironclaw_authorization -p ironclaw_capabilities --all-features
--tests -- -D warnings` is clean.
This commit is contained in:
ilblackdragon@gmail.com
2026-05-16 17:07:01 -07:00
parent 6ecca195d6
commit 5e7688d3b5
2 changed files with 249 additions and 74 deletions

View File

@@ -22,6 +22,7 @@ use async_trait::async_trait;
use chrono::Utc;
use ironclaw_filesystem::{
CasExpectation, ContentType, Entry, FileType, FilesystemError, RecordVersion, RootFilesystem,
ScopedFilesystem,
};
/// Bounded retry budget for compare-and-swap loops on lease writes.
@@ -34,7 +35,7 @@ use ironclaw_host_api::{
AgentId, CapabilityDescriptor, CapabilityGrant, CapabilityGrantId, Decision, DenyReason,
EffectKind, ExecutionContext, HostApiError, InvocationFingerprint, InvocationId, MissionId,
NetworkPolicy, Obligation, Obligations, Principal, ProjectId, ResourceCeiling,
ResourceEstimate, ResourceScope, SandboxQuota, TenantId, ThreadId, UserId, VirtualPath,
ResourceEstimate, ResourceScope, SandboxQuota, ScopedPath, TenantId, ThreadId, UserId,
};
use ironclaw_trust::{AuthorityCeiling, TrustDecision};
use serde::{Deserialize, Serialize};
@@ -400,12 +401,28 @@ impl CapabilityLeaseStore for InMemoryCapabilityLeaseStore {
}
}
/// Filesystem-backed capability lease store under resource-owner/invocation-scoped `/engine` paths.
/// Filesystem-backed capability lease store under the `/authorization` mount
/// alias.
///
/// Construct with a [`ScopedFilesystem`] over any
/// [`RootFilesystem`] (typically a
/// [`CompositeRootFilesystem`](ironclaw_filesystem::CompositeRootFilesystem)
/// or the in-memory backend for tests). The [`ScopedFilesystem`] resolves
/// the `/authorization` alias to a tenant/user-scoped
/// [`VirtualPath`](ironclaw_host_api::VirtualPath) per its
/// [`MountView`](ironclaw_host_api::MountView) and enforces per-op ACL
/// before any backend dispatch — so tenant isolation is structural, not a
/// convention this crate has to remember in its path builders.
///
/// The store keeps a `'a` borrow on the [`ScopedFilesystem`] (mirroring the
/// pre-refactor `&'a F` shape) rather than reaching for `Arc`, so existing
/// callers don't have to wrap the per-invocation view in an `Arc` just to
/// hand it here.
pub struct FilesystemCapabilityLeaseStore<'a, F>
where
F: RootFilesystem,
{
filesystem: &'a F,
filesystem: &'a ScopedFilesystem<F>,
mutation_locks: Mutex<HashMap<CapabilityLeaseOwnerKey, Arc<tokio::sync::Mutex<()>>>>,
}
@@ -413,7 +430,7 @@ impl<'a, F> FilesystemCapabilityLeaseStore<'a, F>
where
F: RootFilesystem,
{
pub fn new(filesystem: &'a F) -> Self {
pub fn new(filesystem: &'a ScopedFilesystem<F>) -> Self {
Self {
filesystem,
mutation_locks: Mutex::new(HashMap::new()),
@@ -556,7 +573,7 @@ where
async fn read_lease_index(
&self,
scope: &ResourceScope,
) -> Result<Option<Vec<VirtualPath>>, CapabilityLeaseError> {
) -> Result<Option<Vec<ScopedPath>>, CapabilityLeaseError> {
let path = lease_index_path(scope)?;
let Some(versioned) = self
.filesystem
@@ -573,7 +590,7 @@ where
async fn write_lease_index(
&self,
scope: &ResourceScope,
mut paths: Vec<VirtualPath>,
mut paths: Vec<ScopedPath>,
) -> Result<(), CapabilityLeaseError> {
paths.sort_by(|left, right| left.as_str().cmp(right.as_str()));
paths.dedup_by(|left, right| left.as_str() == right.as_str());
@@ -590,7 +607,7 @@ where
async fn index_lease_path(
&self,
scope: &ResourceScope,
path: VirtualPath,
path: ScopedPath,
) -> Result<(), CapabilityLeaseError> {
let mut paths = self.read_lease_index(scope).await?.unwrap_or_default();
if !paths.iter().any(|existing| existing == &path) {
@@ -602,7 +619,7 @@ where
async fn list_lease_paths_from_index_or_scan(
&self,
scope: &ResourceScope,
) -> Result<Vec<VirtualPath>, CapabilityLeaseError> {
) -> Result<Vec<ScopedPath>, CapabilityLeaseError> {
if let Some(paths) = self.read_lease_index(scope).await? {
return Ok(paths);
}
@@ -612,62 +629,72 @@ where
async fn scan_lease_paths(
&self,
scope: &ResourceScope,
) -> Result<Vec<VirtualPath>, CapabilityLeaseError> {
let roots = self.list_invocation_roots(scope).await?;
) -> Result<Vec<ScopedPath>, CapabilityLeaseError> {
let owner_prefix = lease_owner_prefix(scope)?;
let invocation_subdirs = self.list_subdir_names(&owner_prefix).await?;
let mut paths = Vec::new();
for root in roots {
paths.extend(self.list_lease_files(&root).await?);
for subdir in invocation_subdirs {
let invocation_root = join_scoped(&owner_prefix, &subdir)?;
paths.extend(self.list_lease_files(&invocation_root).await?);
}
Ok(paths)
}
async fn list_invocation_roots(
/// List the immediate child subdirectories of `prefix`, returning each
/// child's leaf name. Mirrors `FilesystemStore::list_subdir_names` in
/// `ironclaw_engine`: `list_dir` returns
/// [`VirtualPath`](ironclaw_host_api::VirtualPath) results because
/// resolution has already happened — we strip the leaf so callers can
/// rebuild a [`ScopedPath`] and let the per-op ACL fire again on the
/// follow-up read.
async fn list_subdir_names(
&self,
scope: &ResourceScope,
) -> Result<Vec<VirtualPath>, CapabilityLeaseError> {
let root = lease_tenant_user_root(scope)?;
let entries = match self.filesystem.list_dir(&root).await {
Ok(entries) => entries,
Err(error) if is_not_found(&error) => return Ok(Vec::new()),
Err(error) => return Err(lease_persistence_error(error)),
};
Ok(entries
.into_iter()
.filter(|entry| entry.file_type == FileType::Directory)
.map(|entry| entry.path)
.collect())
prefix: &ScopedPath,
) -> Result<Vec<String>, CapabilityLeaseError> {
match self.filesystem.list_dir(prefix).await {
Ok(entries) => Ok(entries
.into_iter()
.filter(|entry| entry.file_type == FileType::Directory)
.map(|entry| entry.name)
.collect()),
Err(error) if is_not_found(&error) => Ok(Vec::new()),
Err(error) => Err(lease_persistence_error(error)),
}
}
async fn list_lease_files(
&self,
root: &VirtualPath,
) -> Result<Vec<VirtualPath>, CapabilityLeaseError> {
root: &ScopedPath,
) -> Result<Vec<ScopedPath>, CapabilityLeaseError> {
let entries = match self.filesystem.list_dir(root).await {
Ok(entries) => entries,
Err(error) if is_not_found(&error) => return Ok(Vec::new()),
Err(error) => return Err(lease_persistence_error(error)),
};
Ok(entries
.into_iter()
.filter(|entry| entry.file_type == FileType::File)
.map(|entry| entry.path)
.collect())
let mut out = Vec::new();
for entry in entries {
if entry.file_type != FileType::File {
continue;
}
// `list_dir` returned a `VirtualPath`; rebuild the equivalent
// `ScopedPath` under our prefix so the follow-up `get` re-runs
// the per-op ACL check.
out.push(join_scoped(root, &entry.name)?);
}
Ok(out)
}
async fn read_lease_file(
&self,
path: &VirtualPath,
path: &ScopedPath,
) -> Result<CapabilityLease, CapabilityLeaseError> {
let versioned = self
.filesystem
.get(path)
.await
.map_err(lease_persistence_error)?
.ok_or_else(|| {
lease_persistence_error(FilesystemError::NotFound {
path: path.clone(),
operation: ironclaw_filesystem::FilesystemOperation::ReadFile,
})
.ok_or_else(|| CapabilityLeaseError::Persistence {
reason: format!("filesystem capability lease store: lease file missing: {path}"),
})?;
deserialize(&versioned.entry.body)
}
@@ -845,7 +872,7 @@ impl CapabilityLeaseOwnerKey {
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
struct CapabilityLeaseIndex {
paths: Vec<VirtualPath>,
paths: Vec<ScopedPath>,
}
/// Authorizer that combines request-scoped grants with active capability leases.
@@ -1395,57 +1422,86 @@ pub(crate) fn same_scope_owner(left: &ResourceScope, right: &ResourceScope) -> b
&& left.thread_id == right.thread_id
}
// ── Lease path helpers ────────────────────────────────────────
//
// All helpers return [`ScopedPath`] strings under the `/authorization`
// mount alias. The [`MountView`](ironclaw_host_api::MountView) granted by
// composition resolves the alias to a tenant/user-scoped
// [`VirtualPath`](ironclaw_host_api::VirtualPath) before any backend op —
// so the within-tenant scope (agent/project/mission/thread/invocation)
// stays in the path while the leading `tenants/<tenant_id>/users/<user_id>`
// prefix is the MountView's responsibility, not this crate's.
//
// Layout:
//
// ```text
// /authorization/leases/<within-tenant-scope>/<invocation_id>/<lease_id>.json
// /authorization/leases/<within-tenant-scope>/_lease_index.json
// ```
//
// where `<within-tenant-scope>` is `[agents/<agent_id>/][projects/<project_id>/][missions/<mission_id>/][threads/<thread_id>]`.
const LEASES_PREFIX: &str = "/authorization/leases";
fn lease_path(
scope: &ResourceScope,
lease_id: CapabilityGrantId,
) -> Result<VirtualPath, CapabilityLeaseError> {
VirtualPath::new(format!(
"{}/{lease_id}.json",
lease_invocation_root(scope)?.as_str()
) -> Result<ScopedPath, CapabilityLeaseError> {
ScopedPath::new(format!(
"{}/{}/{}/{lease_id}.json",
LEASES_PREFIX,
within_tenant_scope(scope),
scope.invocation_id,
))
.map_err(lease_host_api_error)
}
fn lease_index_path(scope: &ResourceScope) -> Result<VirtualPath, CapabilityLeaseError> {
VirtualPath::new(format!(
"{}/_lease_index.json",
lease_tenant_user_root(scope)?.as_str()
fn lease_index_path(scope: &ResourceScope) -> Result<ScopedPath, CapabilityLeaseError> {
ScopedPath::new(format!(
"{}/{}/_lease_index.json",
LEASES_PREFIX,
within_tenant_scope(scope),
))
.map_err(lease_host_api_error)
}
fn lease_invocation_root(scope: &ResourceScope) -> Result<VirtualPath, CapabilityLeaseError> {
VirtualPath::new(format!(
"{}/{}",
lease_tenant_user_root(scope)?.as_str(),
scope.invocation_id
))
.map_err(lease_host_api_error)
}
fn lease_tenant_user_root(scope: &ResourceScope) -> Result<VirtualPath, CapabilityLeaseError> {
VirtualPath::new(format!("{}/capability-leases", scoped_owner_root(scope)))
fn lease_owner_prefix(scope: &ResourceScope) -> Result<ScopedPath, CapabilityLeaseError> {
ScopedPath::new(format!("{}/{}", LEASES_PREFIX, within_tenant_scope(scope),))
.map_err(lease_host_api_error)
}
fn scoped_owner_root(scope: &ResourceScope) -> String {
let mut base = format!(
"/engine/tenants/{}/users/{}",
scope.tenant_id, scope.user_id
);
/// Within-tenant path segment carrying the parts of the resource scope that
/// are *not* the tenant/user identity (those move to the MountView). Always
/// renders at least one segment (`scope`) so the lease prefix stays a
/// non-empty directory the backend can `list_dir`.
fn within_tenant_scope(scope: &ResourceScope) -> String {
let mut segments = Vec::new();
if let Some(agent_id) = &scope.agent_id {
base = format!("{base}/agents/{agent_id}");
segments.push(format!("agents/{agent_id}"));
}
if let Some(project_id) = &scope.project_id {
base = format!("{base}/projects/{project_id}");
segments.push(format!("projects/{project_id}"));
}
if let Some(mission_id) = &scope.mission_id {
base = format!("{base}/missions/{mission_id}");
segments.push(format!("missions/{mission_id}"));
}
if let Some(thread_id) = &scope.thread_id {
base = format!("{base}/threads/{thread_id}");
segments.push(format!("threads/{thread_id}"));
}
base
if segments.is_empty() {
"scope".to_string()
} else {
segments.join("/")
}
}
/// Join a leaf segment onto a [`ScopedPath`] prefix. Used when reconstructing
/// a child path after `list_dir` (which returns
/// [`VirtualPath`](ironclaw_host_api::VirtualPath)s) so the per-op ACL
/// enforced by [`ScopedFilesystem`] still runs on the follow-up `get`.
fn join_scoped(prefix: &ScopedPath, leaf: &str) -> Result<ScopedPath, CapabilityLeaseError> {
ScopedPath::new(format!("{}/{leaf}", prefix.as_str().trim_end_matches('/'),))
.map_err(lease_host_api_error)
}
fn serialize_pretty<T>(value: &T) -> Result<Vec<u8>, CapabilityLeaseError>

View File

@@ -6,7 +6,10 @@ use std::sync::{
use async_trait::async_trait;
use chrono::Utc;
use ironclaw_authorization::*;
use ironclaw_filesystem::{DirEntry, FileStat, FilesystemError, LocalFilesystem, RootFilesystem};
use ironclaw_filesystem::{
DirEntry, FileStat, FilesystemError, InMemoryBackend, LocalFilesystem, RootFilesystem,
ScopedFilesystem,
};
use ironclaw_host_api::*;
use ironclaw_trust::{AuthorityCeiling, EffectiveTrustClass, TrustDecision, TrustProvenance};
@@ -705,7 +708,16 @@ async fn filesystem_lease_store_persists_and_reloads_issued_leases() {
#[tokio::test]
async fn filesystem_lease_store_lists_from_owner_index_without_scanning_invocation_roots() {
let fs = CountingFilesystem::new(engine_filesystem());
// Wrap the underlying [`LocalFilesystem`] in a [`CountingFilesystem`]
// so we can assert that `leases_for_scope` reads the owner index
// rather than fanning out to `list_dir` per invocation. The
// [`ScopedFilesystem`] layer on top binds the `/authorization` alias
// to a tenant/user-scoped target — same as `engine_filesystem()`.
let counting = Arc::new(CountingFilesystem::new(local_filesystem_with_engine_mount()));
let fs = build_scoped_fs(
Arc::clone(&counting),
"/engine/tenants/test/users/test/authorization",
);
let context = execution_context(CapabilitySet::default());
let descriptor = descriptor(CapabilityId::new("echo.say").unwrap());
let store = FilesystemCapabilityLeaseStore::new(&fs);
@@ -725,7 +737,7 @@ async fn filesystem_lease_store_lists_from_owner_index_without_scanning_invocati
store.issue(lease).await.unwrap();
}
fs.reset_list_dir_calls();
counting.reset_list_dir_calls();
let leases = store.leases_for_scope(&context.resource_scope).await;
let mut actual = leases
@@ -736,7 +748,7 @@ async fn filesystem_lease_store_lists_from_owner_index_without_scanning_invocati
expected.sort_by_key(|lease_id| lease_id.as_uuid());
assert_eq!(actual, expected);
assert_eq!(
fs.list_dir_calls(),
counting.list_dir_calls(),
0,
"indexed lease listing should not scan every invocation directory"
);
@@ -923,6 +935,85 @@ async fn filesystem_lease_store_is_tenant_user_invocation_scoped() {
);
}
/// Regression test for the systemic finding tracked in
/// `docs/plans/2026-05-16-scoped-filesystem-tenant-isolation.md`:
/// `FilesystemCapabilityLeaseStore` must enforce tenant isolation through
/// the [`ScopedFilesystem`] mount permission boundary, not by hand-rolling
/// `/engine/tenants/<tenant_id>/users/<user_id>/...` prefixes inside its
/// path builders.
///
/// Two stores share one [`InMemoryBackend`] but are constructed with
/// different [`MountView`]s — each resolves the `/authorization` alias to
/// a distinct tenant-scoped [`VirtualPath`] subtree. Issuing a lease on
/// tenant A under a `(user_id, project_id, invocation_id)` scope that
/// tenant B *also* uses must NOT make that lease visible from tenant B.
/// Before the migration to `&ScopedFilesystem<F>`, the store hand-coded
/// the tenant + user prefix into every path string, so any composition
/// layer that wrapped the backend without remembering to re-prefix would
/// leak across tenants with the type system saying nothing — this test
/// fails closed if that ever regresses.
#[tokio::test]
async fn filesystem_capability_lease_store_isolates_two_tenants_with_same_user_project_ids() {
let backend = Arc::new(InMemoryBackend::new());
let scoped_a = build_scoped_fs(
Arc::clone(&backend),
"/engine/tenants/a/users/alice/authorization",
);
let scoped_b = build_scoped_fs(
Arc::clone(&backend),
"/engine/tenants/b/users/alice/authorization",
);
let store_a = FilesystemCapabilityLeaseStore::new(&scoped_a);
let store_b = FilesystemCapabilityLeaseStore::new(&scoped_b);
// Build a context whose `(user_id, project_id, invocation_id)` triple
// is reused verbatim across tenants. After the mount-view migration
// these identifiers no longer appear in the in-store path string
// (modulo project_id for the within-tenant subtree); cross-tenant
// routing is the MountView's responsibility.
let context_a = execution_context(CapabilitySet::default());
let mut context_b = context_a.clone();
// Same user/project/invocation; the only thing that should distinguish
// the two stores is the mount-time tenant prefix wired by composition.
context_b.tenant_id = TenantId::new("tenant-b-unused").unwrap();
context_b.resource_scope.tenant_id = context_b.tenant_id.clone();
let descriptor = descriptor(CapabilityId::new("echo.say").unwrap());
let lease = CapabilityLease::new(
context_a.resource_scope.clone(),
grant_for(
descriptor.id.clone(),
Principal::Extension(context_a.extension_id.clone()),
vec![EffectKind::DispatchCapability],
),
);
let lease_id = lease.grant.id;
store_a.issue(lease.clone()).await.unwrap();
// Tenant A sees its own lease.
assert_eq!(
store_a.get(&context_a.resource_scope, lease_id).await,
Some(lease),
"tenant A must see the lease it just issued",
);
// Tenant B must NOT see tenant A's lease, even when looking up the
// identical `(user_id, project_id, invocation_id)` scope from
// tenant A's request.
assert_eq!(
store_b.get(&context_a.resource_scope, lease_id).await,
None,
"tenant B must NOT see tenant A's lease (cross-tenant leak)",
);
assert!(
store_b
.leases_for_scope(&context_a.resource_scope)
.await
.is_empty(),
"tenant B leases_for_scope under (user, project, invocation) shared with tenant A must be empty",
);
}
#[tokio::test]
async fn revoked_lease_no_longer_authorizes_dispatch() {
let leases = InMemoryCapabilityLeaseStore::new();
@@ -1086,7 +1177,16 @@ fn timestamp(value: &str) -> Timestamp {
serde_json::from_value(serde_json::Value::String(value.to_string())).unwrap()
}
fn engine_filesystem() -> LocalFilesystem {
fn engine_filesystem() -> ScopedFilesystem<LocalFilesystem> {
build_scoped_fs(
Arc::new(local_filesystem_with_engine_mount()),
"/engine/tenants/test/users/test/authorization",
)
}
/// Build a [`LocalFilesystem`] with a temp directory mounted at `/engine`
/// so tests can target paths beneath it via `ScopedFilesystem`.
fn local_filesystem_with_engine_mount() -> LocalFilesystem {
let storage = tempfile::tempdir().unwrap().keep();
let engine_root = storage.join("engine");
std::fs::create_dir_all(&engine_root).unwrap();
@@ -1099,6 +1199,25 @@ fn engine_filesystem() -> LocalFilesystem {
fs
}
/// Build a [`ScopedFilesystem`] that mounts the `/authorization` alias onto
/// `target_root` (a tenant/user-scoped subtree of the underlying backend)
/// with full read/write/list/delete permissions. Multiple stores can share
/// one backend by passing different `target_root` values — that's how the
/// tenant-isolation regression test below constructs two disjoint
/// `FilesystemCapabilityLeaseStore`s over a single `InMemoryBackend`.
fn build_scoped_fs<F>(backend: Arc<F>, target_root: &str) -> ScopedFilesystem<F>
where
F: RootFilesystem,
{
let mounts = MountView::new(vec![MountGrant::new(
MountAlias::new("/authorization").expect("alias"),
VirtualPath::new(target_root).expect("target"),
MountPermissions::read_write_list_delete(),
)])
.expect("mount view");
ScopedFilesystem::new(backend, mounts)
}
fn execution_context(grants: CapabilitySet) -> ExecutionContext {
let invocation_id = InvocationId::new();
let resource_scope = ResourceScope {