lywsvip/ironclaw
1
0
Fork 0
mirror of https://github.com/nearai/ironclaw.git synced 2026-09-03 08:06:01 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor(engine): enforce tenant isolation via ScopedFilesystem in FilesystemStore

Browse Source 
Per-PR-review concern from serrrfirat on PR #3679 (commit 4eccad56d):
FilesystemStore must enforce tenant isolation via ScopedFilesystem instead
of raw RootFilesystem. The engine was taking `Arc<F: RootFilesystem>` and
speaking VirtualPath strings directly to the backend — so any composition
layer that forgot to wrap the backend in a tenant scope would leak across
tenants, with the type system saying nothing.

Migrated the engine's filesystem store surface so tenant isolation is
structural, not a convention engine code has to remember:

- `FilesystemStore::new` now takes `Arc<ScopedFilesystem<F>>`.
- Every path helper in `store/paths.rs` returns `ScopedPath` instead of
  `VirtualPath`. Path strings are unchanged — `/engine/threads/<id>.json`
  is now alias-relative under the `/engine` mount alias, and the
  composition-supplied MountView resolves the alias to a tenant-scoped
  VirtualPath at every op.
- `list_dir`-derived child directory enumeration now strips the leaf
  segment and reconstructs the follow-up read as a `ScopedPath`, so the
  per-op ACL still applies (no `VirtualPath` shortcut).
- The integration tests construct a ScopedFilesystem over InMemoryBackend
  with a tenant-scoped VirtualPath target.
- New regression test `filesystem_store_isolates_two_tenants_with_same_user_project_ids`:
  two FilesystemStores share one InMemoryBackend but have different
  MountViews; writing the same (user_id, project_id, thread_id) on tenant
  A must not be visible from tenant B.

The `Store` trait surface is unchanged. Other consumer crates
(ironclaw_processes, ironclaw_secrets, ironclaw_outbound,
ironclaw_authorization) are tracked separately and not touched here.

Composition wiring (ironclaw_reborn_composition) is updated in a
follow-up commit so the engine's `Arc<ScopedFilesystem<F>>` consumer
gets the matching `/engine` mount with full read/write/list/delete on the
tenant-scoped target.
This commit is contained in:
ilblackdragon@gmail.com
2026-05-16 16:46:28 -07:00
parent 199137b57c
commit ac8e677f94
3 changed files with 252 additions and 130 deletions
Download Patch File Download Diff File Expand all files Collapse all files

169
crates/ironclaw_engine/src/store/filesystem.rs
View File

@@ -34,16 +34,16 @@ use std::sync::{Arc, Mutex, MutexGuard, OnceLock};
use async_trait::async_trait;
use ironclaw_filesystem::{
CasExpectation, Entry, FileType, FilesystemError, Filter, IndexKind, IndexSpec, Page,
RecordKind, RootFilesystem, VersionedEntry,
RecordKind, RootFilesystem, ScopedFilesystem, VersionedEntry,
};
use ironclaw_host_api::VirtualPath;
use ironclaw_host_api::ScopedPath;
use serde::{Serialize, de::DeserializeOwned};
use crate::store::paths::{
conversation_path, conversations_prefix, event_path, events_prefix, host_api_to_engine_error,
index_key_doc_type, index_key_parent_thread_id, index_key_project_id, index_key_revoked,
index_key_status, index_key_thread_id, index_key_user_id, index_name, index_value_bool,
index_value_text, lease_path, leases_prefix, memory_path, memory_prefix_all,
conversation_path, conversations_prefix, event_path, events_prefix, index_key_doc_type,
index_key_parent_thread_id, index_key_project_id, index_key_revoked, index_key_status,
index_key_thread_id, index_key_user_id, index_name, index_value_bool, index_value_text,
lease_path, leases_prefix, leases_root, memory_path, memory_prefix_all,
memory_prefix_for_project, mission_path, missions_prefix, missions_prefix_for_project,
project_path, projects_prefix, step_path, steps_prefix, thread_path, threads_prefix,
};
@@ -91,26 +91,33 @@ fn kind_mission() -> RecordKind {
/// Filesystem-backed [`Store`] implementation.
///
/// Construct with any [`RootFilesystem`] — typically a
/// [`CompositeRootFilesystem`](ironclaw_filesystem::CompositeRootFilesystem)
/// or the in-memory backend for tests. Indexes are declared lazily on the
/// first write that needs them (mirrors the secrets / authorization stores).
/// Construct with a [`ScopedFilesystem`] over any [`RootFilesystem`] —
/// typically a [`CompositeRootFilesystem`](ironclaw_filesystem::CompositeRootFilesystem)
/// or the in-memory backend for tests. The [`ScopedFilesystem`] enforces
/// the caller's [`MountView`](ironclaw_host_api::MountView) per-operation
/// ACL and resolves the `/engine` alias to a tenant-scoped
/// [`VirtualPath`](ironclaw_host_api::VirtualPath) before any backend
/// dispatch — so tenant isolation is structural, not a convention engine
/// code has to remember.
///
/// Indexes are declared lazily on the first write that needs them
/// (mirrors the secrets / authorization stores).
pub struct FilesystemStore<F>
where
F: RootFilesystem,
{
filesystem: Arc<F>,
filesystem: Arc<ScopedFilesystem<F>>,
}
impl<F> FilesystemStore<F>
where
F: RootFilesystem,
{
pub fn new(filesystem: Arc<F>) -> Self {
pub fn new(filesystem: Arc<ScopedFilesystem<F>>) -> Self {
Self { filesystem }
}
pub fn filesystem(&self) -> &Arc<F> {
pub fn filesystem(&self) -> &Arc<ScopedFilesystem<F>> {
&self.filesystem
}
@@ -119,28 +126,28 @@ where
async fn ensure_threads_indexes(&self) -> Result<(), EngineError> {
let prefix = threads_prefix()?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("threads_by_project"),
index_key_project_id(),
)
.await?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("threads_by_user"),
index_key_user_id(),
)
.await?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("threads_by_parent"),
index_key_parent_thread_id(),
)
.await?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("threads_by_status"),
index_key_status(),
@@ -152,7 +159,7 @@ where
async fn ensure_projects_indexes(&self) -> Result<(), EngineError> {
let prefix = projects_prefix()?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("projects_by_user"),
index_key_user_id(),
@@ -163,7 +170,7 @@ where
async fn ensure_conversations_indexes(&self) -> Result<(), EngineError> {
let prefix = conversations_prefix()?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("conversations_by_user"),
index_key_user_id(),
@@ -174,21 +181,21 @@ where
async fn ensure_memory_indexes(&self) -> Result<(), EngineError> {
let prefix = memory_prefix_all()?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("memory_by_project"),
index_key_project_id(),
)
.await?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("memory_by_user"),
index_key_user_id(),
)
.await?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("memory_by_doc_type"),
index_key_doc_type(),
@@ -200,21 +207,21 @@ where
async fn ensure_missions_indexes(&self) -> Result<(), EngineError> {
let prefix = missions_prefix()?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("missions_by_project"),
index_key_project_id(),
)
.await?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("missions_by_user"),
index_key_user_id(),
)
.await?;
ensure_exact_index(
self.filesystem.as_ref(),
&self.filesystem,
&prefix,
index_name("missions_by_status"),
index_key_status(),
@@ -225,7 +232,7 @@ where
// ── Read helpers ───────────────────────────────────────────
async fn read_one<T>(&self, path: &VirtualPath) -> Result<Option<T>, EngineError>
async fn read_one<T>(&self, path: &ScopedPath) -> Result<Option<T>, EngineError>
where
T: DeserializeOwned,
{
@@ -238,14 +245,14 @@ where
async fn read_versioned(
&self,
path: &VirtualPath,
path: &ScopedPath,
) -> Result<Option<VersionedEntry>, EngineError> {
self.filesystem.get(path).await.map_err(fs_to_engine_error)
}
async fn query_all<T>(
&self,
prefix: &VirtualPath,
prefix: &ScopedPath,
filter: &Filter,
) -> Result<Vec<T>, EngineError>
where
@@ -272,22 +279,36 @@ where
Ok(out)
}
async fn list_subdir_paths(
&self,
prefix: &VirtualPath,
) -> Result<Vec<VirtualPath>, EngineError> {
/// List the immediate child subdirectories of `prefix`, returning each
/// child's directory name (the trailing segment).
///
/// The underlying `list_dir` op returns
/// [`VirtualPath`](ironclaw_host_api::VirtualPath) results because
/// resolution has already happened — we only need the leaf name to
/// reconstruct the child as a [`ScopedPath`] under the same prefix, so
/// we strip it here and let callers join it back. This keeps every
/// public path in this module a [`ScopedPath`] enforced by the
/// [`ScopedFilesystem`] ACL.
async fn list_subdir_names(&self, prefix: &ScopedPath) -> Result<Vec<String>, EngineError> {
match self.filesystem.list_dir(prefix).await {
Ok(entries) => Ok(entries
.into_iter()
.filter(|entry| entry.file_type == FileType::Directory)
.map(|entry| entry.path)
.filter_map(|entry| {
entry
.path
.as_str()
.rsplit('/')
.next()
.map(|s| s.to_string())
})
.collect()),
Err(error) if is_not_found(&error) => Ok(Vec::new()),
Err(error) => Err(fs_to_engine_error(error)),
}
}
async fn read_all_files_under<T>(&self, prefix: &VirtualPath) -> Result<Vec<T>, EngineError>
async fn read_all_files_under<T>(&self, prefix: &ScopedPath) -> Result<Vec<T>, EngineError>
where
T: DeserializeOwned,
{
@@ -304,7 +325,11 @@ where
if !entry.name.ends_with(".json") {
continue;
}
if let Some(value) = self.read_one::<T>(&entry.path).await? {
// `list_dir` returned a `VirtualPath`; reconstruct the
// equivalent `ScopedPath` under our prefix so the per-op
// ACL is enforced on the follow-up `get`.
let scoped_child = join_scoped(prefix, &entry.name)?;
if let Some(value) = self.read_one::<T>(&scoped_child).await? {
out.push(value);
}
}
@@ -315,7 +340,7 @@ where
async fn write_record<T>(
&self,
path: &VirtualPath,
path: &ScopedPath,
kind: RecordKind,
value: &T,
indexed: Vec<(
@@ -372,7 +397,7 @@ type FilesystemRecordLock = Arc<tokio::sync::Mutex<()>>;
static FILESYSTEM_RECORD_LOCKS: OnceLock<Mutex<HashMap<String, FilesystemRecordLock>>> =
OnceLock::new();
fn lock_for_path(path: &VirtualPath) -> FilesystemRecordLock {
fn lock_for_path(path: &ScopedPath) -> FilesystemRecordLock {
let locks = FILESYSTEM_RECORD_LOCKS.get_or_init(|| Mutex::new(HashMap::new()));
let mut guard = lock_or_recover(locks);
Arc::clone(
@@ -391,13 +416,13 @@ fn lock_or_recover<T>(mutex: &Mutex<HashMap<String, T>>) -> MutexGuard<'_, HashM
// ── Helpers ────────────────────────────────────────────────────
async fn ensure_exact_index<F>(
filesystem: &F,
prefix: &VirtualPath,
filesystem: &ScopedFilesystem<F>,
prefix: &ScopedPath,
name: ironclaw_filesystem::IndexName,
key: ironclaw_filesystem::IndexKey,
) -> Result<(), EngineError>
where
F: RootFilesystem + ?Sized,
F: RootFilesystem,
{
let spec = IndexSpec::new(name, vec![key], IndexKind::Exact);
match filesystem.ensure_index(prefix, &spec).await {
@@ -410,6 +435,17 @@ where
}
}
/// 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
/// check still runs on the follow-up `get`.
fn join_scoped(prefix: &ScopedPath, leaf: &str) -> Result<ScopedPath, EngineError> {
let joined = format!("{}/{}", prefix.as_str().trim_end_matches('/'), leaf);
ScopedPath::new(joined).map_err(|error| EngineError::Store {
reason: format!("filesystem engine store: invalid scoped path: {error}"),
})
}
fn fs_to_engine_error(error: ironclaw_filesystem::FilesystemError) -> EngineError {
// Tag the typed `Unsupported` variant so `is_engine_unsupported` can
// recognize it discriminator-wise rather than by free-text matching.
@@ -845,13 +881,9 @@ where
// not hit this hot — most consumers call `list_memory_docs` with a
// project scope first.
let memory_root = memory_prefix_all()?;
let project_dirs = self.list_subdir_paths(&memory_root).await?;
for dir in project_dirs {
let candidate = match dir.as_str().rsplit('/').next() {
Some(slug) => slug,
None => continue,
};
let project_id = match uuid::Uuid::parse_str(candidate) {
let project_dirs = self.list_subdir_names(&memory_root).await?;
for slug in project_dirs {
let project_id = match uuid::Uuid::parse_str(&slug) {
Ok(uuid) => ProjectId(uuid),
Err(_) => continue,
};
@@ -906,10 +938,11 @@ where
// over `list_all_projects()`, which on a fresh filesystem is
// empty — the directory scan is the right primitive here.
Err(error) if is_engine_unsupported(&error) => {
let project_dirs = self.list_subdir_paths(&prefix).await?;
let project_dirs = self.list_subdir_names(&prefix).await?;
let mut docs = Vec::new();
for dir in project_dirs {
docs.extend(self.read_all_files_under::<MemoryDoc>(&dir).await?);
for slug in project_dirs {
let child = join_scoped(&prefix, &slug)?;
docs.extend(self.read_all_files_under::<MemoryDoc>(&child).await?);
}
docs.retain(|doc| doc.user_id == user_id);
Ok(docs)
@@ -942,14 +975,10 @@ where
// lease subdirectories until we find it. Lease lookup is rare
// (revoke + grant flows), and the directory cardinality is
// bounded by active threads.
let leases_root = leases_root_path()?;
let thread_dirs = self.list_subdir_paths(&leases_root).await?;
for dir in thread_dirs {
let candidate = match dir.as_str().rsplit('/').next() {
Some(slug) => slug,
None => continue,
};
let thread_id = match uuid::Uuid::parse_str(candidate) {
let leases_root_path = leases_root()?;
let thread_dirs = self.list_subdir_names(&leases_root_path).await?;
for slug in thread_dirs {
let thread_id = match uuid::Uuid::parse_str(&slug) {
Ok(uuid) => ThreadId(uuid),
Err(_) => continue,
};
@@ -1005,13 +1034,9 @@ where
// mission project subdirectories. Same approach as
// `load_memory_doc`.
let mission_root = missions_prefix()?;
let project_dirs = self.list_subdir_paths(&mission_root).await?;
for dir in project_dirs {
let candidate = match dir.as_str().rsplit('/').next() {
Some(slug) => slug,
None => continue,
};
let project_id = match uuid::Uuid::parse_str(candidate) {
let project_dirs = self.list_subdir_names(&mission_root).await?;
for slug in project_dirs {
let project_id = match uuid::Uuid::parse_str(&slug) {
Ok(uuid) => ProjectId(uuid),
Err(_) => continue,
};
@@ -1057,13 +1082,9 @@ where
status: MissionStatus,
) -> Result<(), EngineError> {
let mission_root = missions_prefix()?;
let project_dirs = self.list_subdir_paths(&mission_root).await?;
for dir in project_dirs {
let candidate = match dir.as_str().rsplit('/').next() {
Some(slug) => slug,
None => continue,
};
let project_id = match uuid::Uuid::parse_str(candidate) {
let project_dirs = self.list_subdir_names(&mission_root).await?;
for slug in project_dirs {
let project_id = match uuid::Uuid::parse_str(&slug) {
Ok(uuid) => ProjectId(uuid),
Err(_) => continue,
};
@@ -1144,7 +1165,3 @@ fn is_engine_unsupported(error: &EngineError) -> bool {
_ => false,
}
}
fn leases_root_path() -> Result<VirtualPath, EngineError> {
VirtualPath::new("/engine/leases").map_err(host_api_to_engine_error)
}

112
crates/ironclaw_engine/src/store/paths.rs
View File

@@ -1,14 +1,14 @@
//! Path layout for the filesystem-backed engine store.
//!
//! All engine state lives under `/engine/...` on the unified
//! [`RootFilesystem`](ironclaw_filesystem::RootFilesystem) surface. The path
//! shape is intentionally simple — there is no tenant/user/agent prefix here
//! because the engine's entities already carry `user_id` / `project_id`
//! fields, and tenant/user isolation is enforced at the mount-table layer
//! when the engine's `/engine` mount is composed.
//! All engine state lives under the `/engine` mount alias on a
//! [`ScopedFilesystem`](ironclaw_filesystem::ScopedFilesystem). The paths
//! below are alias-relative [`ScopedPath`] strings, not raw
//! [`VirtualPath`]s — at every op the [`ScopedFilesystem`] resolves the
//! alias against its [`MountView`](ironclaw_host_api::MountView) and
//! enforces per-grant ACL before any backend dispatch.
//!
//! ```text
//! /engine/threads/<thread_id>.json
//! /engine/threads/<thread_id>.json — alias-relative
//! /engine/steps/<thread_id>/<step_id>.json
//! /engine/events/<thread_id>/<event_id>.json
//! /engine/projects/<project_id>.json
@@ -18,11 +18,18 @@
//! /engine/missions/<project_id>/<mission_id>.json
//! ```
//!
//! Indexed projection (rather than path hierarchy) is the queryable surface
//! for `user_id`, `status`, `parent_thread_id`, etc.
//! These are [`ScopedPath`] strings under the `/engine` mount alias. The
//! [`MountView`](ironclaw_host_api::MountView) granted by composition
//! resolves the alias to a tenant-scoped
//! [`VirtualPath`](ironclaw_host_api::VirtualPath) (e.g.
//! `/engine/tenants/<tenant_id>/users/<user_id>/engine`), so the engine
//! code itself is tenant-agnostic.
//!
//! Indexed projection (rather than path hierarchy) is the queryable
//! surface for `user_id`, `status`, `parent_thread_id`, etc.
use ironclaw_filesystem::{IndexKey, IndexName, IndexValue};
use ironclaw_host_api::{HostApiError, VirtualPath};
use ironclaw_host_api::{HostApiError, ScopedPath};
use uuid::Uuid;
use crate::types::capability::LeaseId;
@@ -43,104 +50,105 @@ const MEMORY_PREFIX: &str = "/engine/memory";
const LEASES_PREFIX: &str = "/engine/leases";
const MISSIONS_PREFIX: &str = "/engine/missions";
pub(super) fn threads_prefix() -> Result<VirtualPath, EngineError> {
virtual_path(THREADS_PREFIX)
pub(super) fn threads_prefix() -> Result<ScopedPath, EngineError> {
scoped_path(THREADS_PREFIX)
}
pub(super) fn thread_path(thread_id: ThreadId) -> Result<VirtualPath, EngineError> {
virtual_path(&format!("{THREADS_PREFIX}/{}.json", thread_id.0))
pub(super) fn thread_path(thread_id: ThreadId) -> Result<ScopedPath, EngineError> {
scoped_path(&format!("{THREADS_PREFIX}/{}.json", thread_id.0))
}
pub(super) fn steps_prefix(thread_id: ThreadId) -> Result<VirtualPath, EngineError> {
virtual_path(&format!("{STEPS_PREFIX}/{}", thread_id.0))
pub(super) fn steps_prefix(thread_id: ThreadId) -> Result<ScopedPath, EngineError> {
scoped_path(&format!("{STEPS_PREFIX}/{}", thread_id.0))
}
pub(super) fn step_path(thread_id: ThreadId, step_id: StepId) -> Result<VirtualPath, EngineError> {
virtual_path(&format!(
pub(super) fn step_path(thread_id: ThreadId, step_id: StepId) -> Result<ScopedPath, EngineError> {
scoped_path(&format!(
"{STEPS_PREFIX}/{}/{}.json",
thread_id.0, step_id.0
))
}
pub(super) fn events_prefix(thread_id: ThreadId) -> Result<VirtualPath, EngineError> {
virtual_path(&format!("{EVENTS_PREFIX}/{}", thread_id.0))
pub(super) fn events_prefix(thread_id: ThreadId) -> Result<ScopedPath, EngineError> {
scoped_path(&format!("{EVENTS_PREFIX}/{}", thread_id.0))
}
pub(super) fn event_path(thread_id: ThreadId, event_id: Uuid) -> Result<VirtualPath, EngineError> {
virtual_path(&format!(
pub(super) fn event_path(thread_id: ThreadId, event_id: Uuid) -> Result<ScopedPath, EngineError> {
scoped_path(&format!(
"{EVENTS_PREFIX}/{}/{}.json",
thread_id.0, event_id
))
}
pub(super) fn projects_prefix() -> Result<VirtualPath, EngineError> {
virtual_path(PROJECTS_PREFIX)
pub(super) fn projects_prefix() -> Result<ScopedPath, EngineError> {
scoped_path(PROJECTS_PREFIX)
}
pub(super) fn project_path(project_id: ProjectId) -> Result<VirtualPath, EngineError> {
virtual_path(&format!("{PROJECTS_PREFIX}/{}.json", project_id.0))
pub(super) fn project_path(project_id: ProjectId) -> Result<ScopedPath, EngineError> {
scoped_path(&format!("{PROJECTS_PREFIX}/{}.json", project_id.0))
}
pub(super) fn conversation_path(
conversation_id: ConversationId,
) -> Result<VirtualPath, EngineError> {
virtual_path(&format!(
) -> Result<ScopedPath, EngineError> {
scoped_path(&format!(
"{CONVERSATIONS_PREFIX}/{}.json",
conversation_id.0
))
}
pub(super) fn conversations_prefix() -> Result<VirtualPath, EngineError> {
virtual_path(CONVERSATIONS_PREFIX)
pub(super) fn conversations_prefix() -> Result<ScopedPath, EngineError> {
scoped_path(CONVERSATIONS_PREFIX)
}
pub(super) fn memory_prefix_for_project(project_id: ProjectId) -> Result<VirtualPath, EngineError> {
virtual_path(&format!("{MEMORY_PREFIX}/{}", project_id.0))
pub(super) fn memory_prefix_for_project(project_id: ProjectId) -> Result<ScopedPath, EngineError> {
scoped_path(&format!("{MEMORY_PREFIX}/{}", project_id.0))
}
pub(super) fn memory_prefix_all() -> Result<VirtualPath, EngineError> {
virtual_path(MEMORY_PREFIX)
pub(super) fn memory_prefix_all() -> Result<ScopedPath, EngineError> {
scoped_path(MEMORY_PREFIX)
}
pub(super) fn memory_path(
project_id: ProjectId,
doc_id: DocId,
) -> Result<VirtualPath, EngineError> {
virtual_path(&format!(
pub(super) fn memory_path(project_id: ProjectId, doc_id: DocId) -> Result<ScopedPath, EngineError> {
scoped_path(&format!(
"{MEMORY_PREFIX}/{}/{}.json",
project_id.0, doc_id.0
))
}
pub(super) fn leases_prefix(thread_id: ThreadId) -> Result<VirtualPath, EngineError> {
virtual_path(&format!("{LEASES_PREFIX}/{}", thread_id.0))
pub(super) fn leases_prefix(thread_id: ThreadId) -> Result<ScopedPath, EngineError> {
scoped_path(&format!("{LEASES_PREFIX}/{}", thread_id.0))
}
pub(super) fn lease_path(
thread_id: ThreadId,
lease_id: LeaseId,
) -> Result<VirtualPath, EngineError> {
virtual_path(&format!(
) -> Result<ScopedPath, EngineError> {
scoped_path(&format!(
"{LEASES_PREFIX}/{}/{}.json",
thread_id.0, lease_id.0
))
}
pub(super) fn missions_prefix() -> Result<VirtualPath, EngineError> {
virtual_path(MISSIONS_PREFIX)
pub(super) fn leases_root() -> Result<ScopedPath, EngineError> {
scoped_path(LEASES_PREFIX)
}
pub(super) fn missions_prefix() -> Result<ScopedPath, EngineError> {
scoped_path(MISSIONS_PREFIX)
}
pub(super) fn missions_prefix_for_project(
project_id: ProjectId,
) -> Result<VirtualPath, EngineError> {
virtual_path(&format!("{MISSIONS_PREFIX}/{}", project_id.0))
) -> Result<ScopedPath, EngineError> {
scoped_path(&format!("{MISSIONS_PREFIX}/{}", project_id.0))
}
pub(super) fn mission_path(
project_id: ProjectId,
mission_id: MissionId,
) -> Result<VirtualPath, EngineError> {
virtual_path(&format!(
) -> Result<ScopedPath, EngineError> {
scoped_path(&format!(
"{MISSIONS_PREFIX}/{}/{}.json",
project_id.0, mission_id.0
))
@@ -192,8 +200,8 @@ pub(super) fn index_value_bool(b: bool) -> IndexValue {
// ── Internals ────────────────────────────────────────────────
fn virtual_path(raw: &str) -> Result<VirtualPath, EngineError> {
VirtualPath::new(raw).map_err(host_api_to_engine_error)
fn scoped_path(raw: &str) -> Result<ScopedPath, EngineError> {
ScopedPath::new(raw).map_err(host_api_to_engine_error)
}
fn index_key(key: &str) -> IndexKey {
@@ -204,6 +212,6 @@ fn index_key(key: &str) -> IndexKey {
pub(super) fn host_api_to_engine_error(error: HostApiError) -> EngineError {
EngineError::Store {
reason: format!("filesystem engine store: invalid virtual path: {error}"),
reason: format!("filesystem engine store: invalid scoped path: {error}"),
}
}

101
crates/ironclaw_engine/tests/filesystem_store.rs
View File

@@ -21,10 +21,39 @@ use ironclaw_engine::types::step::Step;
use ironclaw_engine::types::thread::{Thread, ThreadConfig, ThreadId, ThreadState, ThreadType};
use ironclaw_engine::types::{LEGACY_SHARED_OWNER_ID, shared_owner_id};
use ironclaw_engine::{EventKind, FilesystemStore, ProjectId, Store, ThreadEvent};
use ironclaw_filesystem::InMemoryBackend;
use ironclaw_filesystem::{InMemoryBackend, ScopedFilesystem};
use ironclaw_host_api::{MountAlias, MountGrant, MountPermissions, MountView, VirtualPath};
/// Build a `ScopedFilesystem<InMemoryBackend>` with full
/// read/write/list/delete permissions on the `/engine` alias, mapped to a
/// distinct tenant-scoped [`VirtualPath`] subtree. Tests can pass in a
/// different `target_root` to simulate multiple tenants sharing one
/// underlying backend (`filesystem_store_isolates_two_tenants_*` below).
fn build_scoped_fs(
backend: Arc<InMemoryBackend>,
target_root: &str,
) -> Arc<ScopedFilesystem<InMemoryBackend>> {
let mounts = MountView::new(vec![MountGrant::new(
MountAlias::new("/engine").expect("alias"),
VirtualPath::new(target_root).expect("target"),
MountPermissions {
read: true,
write: true,
list: true,
delete: true,
execute: false,
},
)])
.expect("mount view");
Arc::new(ScopedFilesystem::new(backend, mounts))
}
fn make_store() -> FilesystemStore<InMemoryBackend> {
FilesystemStore::new(Arc::new(InMemoryBackend::new()))
let backend = Arc::new(InMemoryBackend::new());
FilesystemStore::new(build_scoped_fs(
backend,
"/engine/tenants/test/users/test/engine",
))
}
fn make_thread(project_id: ProjectId, user_id: &str) -> Thread {
@@ -624,3 +653,71 @@ async fn list_skills_global_returns_only_shared_skills() {
assert_eq!(globals.len(), 1);
assert_eq!(globals[0].id, shared_skill.id);
}
/// Regression test for the HIGH-severity finding flagged in PR #3679
/// review (commit `4eccad56d`): the engine's `FilesystemStore` must
/// enforce tenant isolation through the [`ScopedFilesystem`] mount
/// permission boundary, not assume that path strings inside engine code
/// already encode tenant identity.
///
/// Two stores share one [`InMemoryBackend`] but are constructed with
/// different [`MountView`]s — each one resolves the `/engine` alias to a
/// distinct tenant-scoped [`VirtualPath`] subtree. Writing the same
/// `(user_id, project_id, thread_id)` tuple on store A must NOT make the
/// thread visible from store B. Before the migration to
/// `Arc<ScopedFilesystem<F>>`, the engine spoke raw `VirtualPath`s
/// directly to a `RootFilesystem`, so any composition layer that forgot
/// to wrap the backend in a tenant scope would leak across tenants —
/// this test fails closed if that ever regresses.
#[tokio::test]
async fn filesystem_store_isolates_two_tenants_with_same_user_project_ids() {
let backend = Arc::new(InMemoryBackend::new());
let store_a = FilesystemStore::new(build_scoped_fs(
Arc::clone(&backend),
"/engine/tenants/a/users/alice/engine",
));
let store_b = FilesystemStore::new(build_scoped_fs(
Arc::clone(&backend),
"/engine/tenants/b/users/alice/engine",
));
// Identical `(user_id, project_id)` for both stores — the only thing
// that should keep them apart is the mount-time tenant prefix.
let project_id = ProjectId::new();
let thread = make_thread(project_id, "alice");
let thread_id = thread.id;
store_a.save_thread(&thread).await.unwrap();
// Tenant A sees its own thread.
let from_a = store_a
.load_thread(thread_id)
.await
.expect("store_a load_thread succeeds");
assert!(
from_a.is_some(),
"tenant A must see the thread it just wrote",
);
// Tenant B does NOT see tenant A's thread, despite identical
// (user_id, project_id, thread_id).
let from_b = store_b
.load_thread(thread_id)
.await
.expect("store_b load_thread succeeds");
assert!(
from_b.is_none(),
"tenant B must NOT see tenant A's thread (cross-tenant leak)",
);
// Tenant B's list_threads for the same user/project must be empty.
let b_threads = store_b
.list_threads(project_id, "alice")
.await
.expect("store_b list_threads succeeds");
assert!(
b_threads.is_empty(),
"tenant B list_threads must be empty under (user, project) shared with tenant A; got {} threads",
b_threads.len(),
);
}