diff --git a/Cargo.lock b/Cargo.lock index 1ca24238..3c3e9fb5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -82,6 +82,15 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "arc-swap" +version = "1.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a3a1fd6f75306b68087b831f025c712524bcb19aad54e557b1129cfa0a2b207" +dependencies = [ + "rustversion", +] + [[package]] name = "assert-json-diff" version = "2.0.2" @@ -1696,6 +1705,7 @@ dependencies = [ name = "manifest" version = "0.1.0" dependencies = [ + "arc-swap", "llm-worker", "protocol", "serde", diff --git a/crates/manifest/Cargo.toml b/crates/manifest/Cargo.toml index 3e4a4114..487de08c 100644 --- a/crates/manifest/Cargo.toml +++ b/crates/manifest/Cargo.toml @@ -5,6 +5,7 @@ edition.workspace = true license.workspace = true [dependencies] +arc-swap = "1" llm-worker = { workspace = true } protocol = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index 5bf83ece..90333720 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -15,7 +15,7 @@ pub use model::{ }; pub use paths::user_manifest_path; pub use protocol::{Permission, ScopeRule}; -pub use scope::{Scope, ScopeError}; +pub use scope::{Scope, ScopeError, SharedScope}; use std::collections::HashMap; use std::num::NonZeroU32; diff --git a/crates/manifest/src/scope.rs b/crates/manifest/src/scope.rs index 753703d8..0a8bae56 100644 --- a/crates/manifest/src/scope.rs +++ b/crates/manifest/src/scope.rs @@ -8,6 +8,9 @@ use std::ffi::OsString; use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex}; + +use arc_swap::{ArcSwap, Guard}; use crate::{Permission, ScopeConfig, ScopeRule}; @@ -182,6 +185,38 @@ impl Scope { .map(|r| r.target.as_path()) } + /// Build a new [`Scope`] equal to `self` with `extra_allow` appended + /// to the allow set. Used by dynamic-scope grow paths + /// (e.g. controller adding the bash-output Read rule, future + /// external `GrantScope`). + pub fn with_added_allow_rules( + &self, + extra_allow: impl IntoIterator, + ) -> Result { + let mut config = ScopeConfig { + allow: self.allow_rules(), + deny: self.deny_rules(), + }; + config.allow.extend(extra_allow); + Self::from_config(&config) + } + + /// Build a new [`Scope`] equal to `self` with `extra_deny` appended + /// to the deny set. Used by dynamic-scope shrink paths + /// (e.g. SpawnPod-style delegation that strips Write from the + /// spawner without touching its allow rules). + pub fn with_added_deny_rules( + &self, + extra_deny: impl IntoIterator, + ) -> Result { + let mut config = ScopeConfig { + allow: self.allow_rules(), + deny: self.deny_rules(), + }; + config.deny.extend(extra_deny); + Self::from_config(&config) + } + /// Human-readable grouping of allow rules, suitable for embedding in /// LLM system prompts. Deny rules are intentionally omitted — they /// only cap effective permission and surface them would mislead the @@ -230,6 +265,81 @@ impl Scope { } } +/// Shared, atomically-swappable view of a [`Scope`]. +/// +/// Built around [`ArcSwap`] so the hot path (permission checks inside +/// `ScopedFs`) reads the current scope lock-free. Mutators are +/// serialised by an internal `Mutex` so concurrent `update` calls do +/// not lose each other's contributions. +/// +/// All clones share the same underlying state — a `SharedScope` cloned +/// out to multiple consumers (Pod, ScopedFs, future grant/revoke +/// callers) sees every update. +#[derive(Debug, Clone)] +pub struct SharedScope { + inner: Arc, +} + +#[derive(Debug)] +struct SharedScopeInner { + scope: ArcSwap, + /// Serialises read-modify-write update transactions so a producer + /// can read the current scope, build a derived one, and store it + /// without losing concurrent updates. + write_lock: Mutex<()>, +} + +impl SharedScope { + /// Wrap an owned [`Scope`] in a shared, atomically-swappable handle. + pub fn new(scope: Scope) -> Self { + Self { + inner: Arc::new(SharedScopeInner { + scope: ArcSwap::from_pointee(scope), + write_lock: Mutex::new(()), + }), + } + } + + /// Snapshot the current scope. Cheap and lock-free; the returned + /// guard borrows the live scope for as long as it is held. + pub fn load(&self) -> Guard> { + self.inner.scope.load() + } + + /// Snapshot the current scope into an owned `Arc`. Useful + /// when the caller needs a value that outlives the load guard + /// (e.g. cloning into another struct). + pub fn snapshot(&self) -> Arc { + self.inner.scope.load_full() + } + + /// Replace the current scope wholesale. Equivalent to building a + /// fresh [`Scope`] from a [`ScopeConfig`] and storing it. Concurrent + /// `update` callers in the middle of a read-modify-write will see + /// this store reflected on their next iteration if their derived + /// scope cannot be built from the now-stale snapshot. + pub fn store(&self, scope: Scope) { + let _guard = self.inner.write_lock.lock().expect("scope mutex poisoned"); + self.inner.scope.store(Arc::new(scope)); + } + + /// Read-modify-write transaction. `f` is called with the current + /// scope and returns a derived one (or an error). The internal + /// write lock ensures that two concurrent `update` calls see each + /// other's results — the second observes the first's output as its + /// input. + pub fn update(&self, f: F) -> Result<(), ScopeError> + where + F: FnOnce(&Scope) -> Result, + { + let _guard = self.inner.write_lock.lock().expect("scope mutex poisoned"); + let current = self.inner.scope.load(); + let new = f(¤t)?; + self.inner.scope.store(Arc::new(new)); + Ok(()) + } +} + impl ResolvedRule { fn matches(&self, path: &Path) -> bool { if self.recursive { @@ -545,4 +655,87 @@ mod tests { let deep = dir.path().join("a/b/c/new.txt"); assert!(scope.is_writable(&deep)); } + + #[test] + fn with_added_allow_rules_grows_readable_set() { + let dir = TempDir::new().unwrap(); + let extra = TempDir::new().unwrap(); + let base = Scope::writable(dir.path()).unwrap(); + assert!(!base.is_readable(&extra.path().join("x"))); + let extended = base + .with_added_allow_rules([ScopeRule { + target: extra.path().to_path_buf(), + permission: Permission::Read, + recursive: true, + }]) + .unwrap(); + assert!(extended.is_readable(&extra.path().join("x"))); + assert!(extended.is_writable(&dir.path().join("y"))); + } + + #[test] + fn with_added_deny_rules_demotes_write_to_read() { + let dir = TempDir::new().unwrap(); + let sub = dir.path().join("sub"); + std::fs::create_dir(&sub).unwrap(); + let base = Scope::writable(dir.path()).unwrap(); + let demoted = base + .with_added_deny_rules([ScopeRule { + target: sub.clone(), + permission: Permission::Write, + recursive: true, + }]) + .unwrap(); + let f = sub.join("a.txt"); + assert_eq!(demoted.permission_at(&f), Some(Permission::Read)); + assert_eq!( + demoted.permission_at(&dir.path().join("top.txt")), + Some(Permission::Write) + ); + } + + #[test] + fn shared_scope_load_returns_current_value() { + let dir = TempDir::new().unwrap(); + let shared = SharedScope::new(Scope::writable(dir.path()).unwrap()); + assert!(shared.load().is_writable(&dir.path().join("a.txt"))); + } + + #[test] + fn shared_scope_update_replaces_view_atomically() { + let dir = TempDir::new().unwrap(); + let sub = dir.path().join("sub"); + std::fs::create_dir(&sub).unwrap(); + let shared = SharedScope::new(Scope::writable(dir.path()).unwrap()); + let target = sub.join("a.txt"); + assert_eq!(shared.load().permission_at(&target), Some(Permission::Write)); + shared + .update(|cur| { + cur.with_added_deny_rules([ScopeRule { + target: sub.clone(), + permission: Permission::Write, + recursive: true, + }]) + }) + .unwrap(); + assert_eq!(shared.load().permission_at(&target), Some(Permission::Read)); + } + + #[test] + fn shared_scope_clones_share_state() { + let dir = TempDir::new().unwrap(); + let extra = TempDir::new().unwrap(); + let a = SharedScope::new(Scope::writable(dir.path()).unwrap()); + let b = a.clone(); + assert!(!b.load().is_readable(&extra.path().join("x"))); + a.update(|cur| { + cur.with_added_allow_rules([ScopeRule { + target: extra.path().to_path_buf(), + permission: Permission::Read, + recursive: true, + }]) + }) + .unwrap(); + assert!(b.load().is_readable(&extra.path().join("x"))); + } } diff --git a/crates/pod/src/controller.rs b/crates/pod/src/controller.rs index fc33a7a8..090d198e 100644 --- a/crates/pod/src/controller.rs +++ b/crates/pod/src/controller.rs @@ -83,7 +83,7 @@ impl PodController { // Snapshot pod-immutable values needed for tool factories so the // mutable worker borrow below doesn't conflict with reads on `pod`. - let scope_for_tools = pod.scope().clone(); + let scope_handle = pod.scope().clone(); let pwd_for_tools = pod.pwd().to_path_buf(); let spawner_name = pod.manifest().pod.name.clone(); let spawner_model = pod.manifest().model.clone(); @@ -230,11 +230,14 @@ impl PodController { // touching. // // Bash spills long outputs to a per-pod subdir under the - // runtime dir. We layer a recursive `allow(Read)` rule for - // that path on top of the user-facing scope so the agent can - // `Read` the saved files without polluting the workspace. - // Same approach memory takes for its deny rules: round-trip - // through `ScopeConfig` and rebuild via `from_config`. + // runtime dir. Push a recursive `allow(Read)` for that path + // into the Pod's runtime scope so the agent can `Read` the + // saved files without polluting the workspace. The Pod's + // SharedScope is the single source of truth — the same + // handle backs every ScopedFs (builtin tools, fs_view, + // compact worker), and any future scope mutation + // (SpawnPod-style revoke, future GrantScope) propagates + // through it. let bash_output_dir = runtime_dir.path().join("bash-output"); std::fs::create_dir_all(&bash_output_dir).map_err(|e| { std::io::Error::other(format!( @@ -242,18 +245,16 @@ impl PodController { bash_output_dir.display() )) })?; - let mut scope_config = manifest::ScopeConfig { - allow: scope_for_tools.allow_rules(), - deny: scope_for_tools.deny_rules(), - }; - scope_config.allow.push(manifest::ScopeRule { - target: bash_output_dir.clone(), - permission: manifest::Permission::Read, - recursive: true, - }); - let scope_with_bash = manifest::Scope::from_config(&scope_config) + scope_handle + .update(|cur| { + cur.with_added_allow_rules([manifest::ScopeRule { + target: bash_output_dir.clone(), + permission: manifest::Permission::Read, + recursive: true, + }]) + }) .map_err(std::io::Error::other)?; - let fs = tools::ScopedFs::new(scope_with_bash, pwd_for_tools.clone()); + let fs = tools::ScopedFs::with_shared_scope(scope_handle.clone(), pwd_for_tools.clone()); let tracker = tools::Tracker::new(); // The same ScopedFs also powers the IPC `ListCompletions` // query — keep a clone for the FS view we attach below, @@ -292,6 +293,7 @@ impl PodController { spawned_registry.clone(), self_parent_socket.clone(), spawner_model.clone(), + scope_handle.clone(), )); worker.register_tool(send_to_pod_tool(spawned_registry.clone())); worker.register_tool(read_pod_output_tool(spawned_registry.clone())); @@ -873,7 +875,7 @@ where cwd: pod.pwd().display().to_string(), provider: provider_name, model: model_id, - scope_summary: pod.scope().summary(), + scope_summary: pod.scope_snapshot().summary(), tools: tool_names, } } diff --git a/crates/pod/src/pod.rs b/crates/pod/src/pod.rs index 87cd6bb8..ba82dfce 100644 --- a/crates/pod/src/pod.rs +++ b/crates/pod/src/pod.rs @@ -10,7 +10,10 @@ use llm_worker::{ToolOutputLimits, UsageRecord, Worker, WorkerError, WorkerResul use session_store::{EntryHash, SessionId, SessionStartState, Store, StoreError}; use tracing::{info, warn}; -use manifest::{PodManifest, PodManifestConfig, ResolveError, Scope, ScopeError, WorkerManifest}; +use manifest::{ + PodManifest, PodManifestConfig, ResolveError, Scope, ScopeError, ScopeRule, SharedScope, + WorkerManifest, +}; use crate::compact::state::CompactState; use crate::compact::usage_tracker::UsageTracker; @@ -60,8 +63,11 @@ pub struct Pod { head_hash: Option, /// Absolute working directory of the Pod. pwd: PathBuf, - /// Resolved scope — always present. - scope: Scope, + /// Shared, atomically-swappable view of the Pod's resolved scope. + /// Cloned out to `ScopedFs` instances (builtin tools, fs_view, + /// compact worker) so scope updates propagate to every consumer + /// at the next permission check. + scope: SharedScope, hook_builder: HookRegistryBuilder, interceptor_installed: bool, /// Shared compaction state (present when compact_threshold is configured). @@ -185,7 +191,7 @@ impl Pod { session_id, head_hash: None, pwd, - scope, + scope: SharedScope::new(scope), hook_builder: HookRegistryBuilder::new(), interceptor_installed: false, compact_state: None, @@ -252,11 +258,46 @@ impl Pod { &self.pwd } - /// The Pod's directory scope. - pub fn scope(&self) -> &Scope { + /// The Pod's directory scope, as a shared atomically-swappable + /// handle. Clone it to share scope state with another consumer + /// (e.g. a tool that needs to mutate scope dynamically). + pub fn scope(&self) -> &SharedScope { &self.scope } + /// Snapshot the current scope as an owned `Arc`. Subsequent + /// scope mutations do not affect the returned snapshot. + pub fn scope_snapshot(&self) -> Arc { + self.scope.snapshot() + } + + /// Apply `extra_allow` to the Pod's runtime scope. Future tool + /// permission checks (read/write/glob/grep) reflect the broadened + /// scope; in-flight tool calls keep the snapshot they captured at + /// invocation time. + pub fn add_scope_rules( + &self, + extra_allow: impl IntoIterator, + ) -> Result<(), ScopeError> { + let extra: Vec = extra_allow.into_iter().collect(); + self.scope + .update(|cur| cur.with_added_allow_rules(extra.clone())) + } + + /// Strip `revoke` rules from the Pod's runtime scope by adding + /// matching deny rules. A `Permission::Write` revoke caps effective + /// access at `Read` (mirroring the pod-registry `effective_write` + /// semantics — Write is the only permission tracked across Pods). + /// A `Permission::Read` revoke removes access entirely. + pub fn revoke_scope_rules( + &self, + revoke: impl IntoIterator, + ) -> Result<(), ScopeError> { + let revoke: Vec = revoke.into_iter().collect(); + self.scope + .update(|cur| cur.with_added_deny_rules(revoke.clone())) + } + /// Direct access to the underlying Worker. pub fn worker(&self) -> &Worker { self.worker.as_ref().expect("worker taken during run") @@ -582,10 +623,11 @@ impl Pod { } else { None }; + let scope_snapshot = self.scope.snapshot(); let ctx = SystemPromptContext { now: chrono::Utc::now(), cwd: &self.pwd, - scope: &self.scope, + scope: &scope_snapshot, tool_names, agents_md: agents_md_read.body, resident_knowledge: resident_slice, @@ -667,7 +709,7 @@ impl Pod { /// are skipped — the unresolved placeholder stays in the flattened /// user message so the LLM still sees the intent. fn resolve_file_refs(&self, segments: &[Segment]) -> Vec { - let view = crate::fs_view::PodFsView::new(tools::ScopedFs::new( + let view = crate::fs_view::PodFsView::new(tools::ScopedFs::with_shared_scope( self.scope.clone(), self.pwd.clone(), )); @@ -1078,7 +1120,7 @@ impl Pod { // with the main Pod (reads go through the same policy) but the // Tracker is fresh — compact-time reads must not pollute the // main session's recency list, which feeds `default_refs` above. - let scoped_fs = tools::ScopedFs::new(self.scope.clone(), self.pwd.clone()); + let scoped_fs = tools::ScopedFs::with_shared_scope(self.scope.clone(), self.pwd.clone()); let summary_tracker = tools::Tracker::new(); let summary_client: Box = self.build_compactor_client()?; let summary_system_prompt = self @@ -1801,7 +1843,7 @@ impl Pod, St> { session_id, head_hash: None, pwd: common.pwd, - scope: common.scope, + scope: SharedScope::new(common.scope), hook_builder: HookRegistryBuilder::new(), interceptor_installed: false, compact_state: None, @@ -1859,7 +1901,7 @@ impl Pod, St> { session_id, head_hash: None, pwd: common.pwd, - scope: common.scope, + scope: SharedScope::new(common.scope), hook_builder: HookRegistryBuilder::new(), interceptor_installed: false, compact_state: None, @@ -1967,7 +2009,7 @@ impl Pod, St> { session_id, head_hash: state.head_hash, pwd: common.pwd, - scope: common.scope, + scope: SharedScope::new(common.scope), hook_builder: HookRegistryBuilder::new(), interceptor_installed: false, compact_state: None, diff --git a/crates/pod/src/spawn/tool.rs b/crates/pod/src/spawn/tool.rs index 9588acc8..1246318e 100644 --- a/crates/pod/src/spawn/tool.rs +++ b/crates/pod/src/spawn/tool.rs @@ -15,7 +15,7 @@ use async_trait::async_trait; use llm_worker::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput}; use manifest::{ ModelManifest, Permission, PodManifestConfig, PodMetaConfig, ScopeConfig, ScopeRule, - WorkerManifestConfig, + SharedScope, WorkerManifestConfig, }; use protocol::Method; use protocol::stream::JsonLineWriter; @@ -119,6 +119,14 @@ pub struct SpawnPodTool { /// configuration in the manifest cascade. Per-spawn override is /// out of scope here (see `tickets/spawn-inherit-provider.md`). spawner_model: ModelManifest, + /// Spawner's runtime scope. After a successful spawn, the + /// `Permission::Write` rules in the delegated scope are revoked + /// from the spawner's in-memory view (a `deny(Write, target)` is + /// pushed on top, downgrading the spawner's effective access on + /// those paths to `Read`). Mirrors the pod-registry's + /// `effective_write` semantics: Write is the only permission + /// tracked across Pods, so revocation only touches Write. + spawner_scope: SharedScope, } impl SpawnPodTool { @@ -130,6 +138,7 @@ impl SpawnPodTool { registry: Arc, parent_socket: Option, spawner_model: ModelManifest, + spawner_scope: SharedScope, ) -> Self { Self { spawner_name, @@ -139,6 +148,7 @@ impl SpawnPodTool { registry, parent_socket, spawner_model, + spawner_scope, } } } @@ -217,6 +227,27 @@ impl Tool for SpawnPodTool { // Child is live. Post-start errors propagate but do not roll // back the scope allocation — the child already owns it. + // + // Mirror that ownership transfer in the spawner's in-memory + // scope: every `Permission::Write` rule in the delegated scope + // is shadowed by a `deny(Write, target)` so subsequent tool + // calls (Edit/Write) on the delegated paths fail with + // `ReadOnly`. Read access is left intact — the registry only + // arbitrates Write, and keeping Read lets the spawner observe + // the child's intermediate output through Read/Glob/Grep. + let revoke_write: Vec = scope_allow + .iter() + .filter(|r| r.permission == Permission::Write) + .cloned() + .collect(); + if !revoke_write.is_empty() { + self.spawner_scope + .update(|cur| cur.with_added_deny_rules(revoke_write.clone())) + .map_err(|e| { + ToolError::ExecutionFailed(format!("revoke spawner scope: {e}")) + })?; + } + send_run(&predicted_socket, &input.task).await?; let record = SpawnedPodRecord { @@ -456,6 +487,7 @@ pub fn spawn_pod_tool( registry: Arc, parent_socket: Option, spawner_model: ModelManifest, + spawner_scope: SharedScope, ) -> ToolDefinition { Arc::new(move || { let schema = schemars::schema_for!(SpawnPodInput); @@ -471,6 +503,7 @@ pub fn spawn_pod_tool( registry.clone(), parent_socket.clone(), spawner_model.clone(), + spawner_scope.clone(), )); (meta, tool) }) diff --git a/crates/pod/tests/spawn_pod_test.rs b/crates/pod/tests/spawn_pod_test.rs index 57141f9d..212aa71e 100644 --- a/crates/pod/tests/spawn_pod_test.rs +++ b/crates/pod/tests/spawn_pod_test.rs @@ -11,7 +11,7 @@ use std::path::{Path, PathBuf}; use std::sync::{LazyLock, Mutex}; use llm_worker::tool::{ToolError, ToolOutput}; -use manifest::{AuthRef, ModelManifest, Permission, SchemeKind, ScopeRule}; +use manifest::{AuthRef, ModelManifest, Permission, SchemeKind, Scope, ScopeRule, SharedScope}; use pod::runtime::dir::{RuntimeDir, SpawnedPodRecord}; use pod::runtime::pod_registry::{self, LockFileGuard}; use pod::spawn::registry::SpawnedPodRegistry; @@ -149,6 +149,14 @@ fn dummy_model() -> ModelManifest { } } +/// Spawner-side `SharedScope` mirroring the `allow_root` granted by +/// `setup_spawner`. The tool revokes Write rules from this scope on +/// successful spawn — tests can `load()` it to assert the +/// revocation took effect. +fn shared_scope_for(allow_root: &Path) -> SharedScope { + SharedScope::new(Scope::writable(allow_root).unwrap()) +} + fn clear_env() { unsafe { std::env::remove_var("INSOMNIA_RUNTIME_DIR"); @@ -169,6 +177,7 @@ async fn spawn_pod_delegates_scope_and_sends_run() { let received = accept_one_method(listener); let registry = SpawnedPodRegistry::new(spawner_rd.clone()); + let spawner_scope = shared_scope_for(allow_root.path()); let def = spawn_pod_tool( "root".into(), spawner_socket.clone(), @@ -177,6 +186,7 @@ async fn spawn_pod_delegates_scope_and_sends_run() { registry, None, dummy_model(), + spawner_scope.clone(), ); let (_meta, tool) = def(); @@ -190,6 +200,13 @@ async fn spawn_pod_delegates_scope_and_sends_run() { }) .to_string(); + // Pre-spawn: the spawner can write to the delegated path. + assert!( + spawner_scope + .load() + .is_writable(&allow_root.path().join("a.txt")) + ); + let output: ToolOutput = tool.execute(&input).await.unwrap(); assert!( output.summary.contains("child"), @@ -225,6 +242,15 @@ async fn spawn_pod_delegates_scope_and_sends_run() { assert_eq!(records[0].pod_name, "child"); assert_eq!(records[0].callback_address, spawner_socket); + // Post-spawn: the spawner's runtime scope has been demoted on the + // delegated path. Write is gone, Read remains. + let post = spawner_scope.load(); + assert_eq!( + post.permission_at(&allow_root.path().join("a.txt")), + Some(Permission::Read), + "spawner should still be able to read delegated path" + ); + clear_env(); } @@ -239,6 +265,7 @@ async fn spawn_pod_rejects_scope_outside_spawner() { point_pod_command_at_true(); let registry = SpawnedPodRegistry::new(spawner_rd); + let spawner_scope = shared_scope_for(allow_root.path()); let def = spawn_pod_tool( "root".into(), spawner_socket, @@ -247,6 +274,7 @@ async fn spawn_pod_rejects_scope_outside_spawner() { registry, None, dummy_model(), + spawner_scope.clone(), ); let (_meta, tool) = def(); @@ -277,6 +305,13 @@ async fn spawn_pod_rejects_scope_outside_spawner() { let guard = LockFileGuard::open(&lock_path).unwrap(); assert!(guard.data().find("child").is_none()); + // Failed spawn must not have demoted the spawner's scope either. + assert!( + spawner_scope + .load() + .is_writable(&allow_root.path().join("a.txt")) + ); + clear_env(); } @@ -301,6 +336,7 @@ async fn spawn_pod_rolls_back_reservation_when_socket_never_appears() { // by running this test alone when iterating. let registry = SpawnedPodRegistry::new(spawner_rd); + let spawner_scope = shared_scope_for(allow_root.path()); let def = spawn_pod_tool( "root".into(), spawner_socket, @@ -309,6 +345,7 @@ async fn spawn_pod_rolls_back_reservation_when_socket_never_appears() { registry, None, dummy_model(), + spawner_scope.clone(), ); let (_meta, tool) = def(); @@ -341,5 +378,13 @@ async fn spawn_pod_rolls_back_reservation_when_socket_never_appears() { "allocation was not rolled back after socket wait timed out" ); + // Spawner's runtime scope must also be untouched — revoke is + // performed only after exec_child succeeds. + assert!( + spawner_scope + .load() + .is_writable(&allow_root.path().join("a.txt")) + ); + clear_env(); } diff --git a/crates/tools/src/scoped_fs.rs b/crates/tools/src/scoped_fs.rs index 76db5bfe..91e204da 100644 --- a/crates/tools/src/scoped_fs.rs +++ b/crates/tools/src/scoped_fs.rs @@ -13,17 +13,23 @@ use std::io::Write as _; use std::path::{Path, PathBuf}; use std::sync::Arc; -use manifest::Scope; +use manifest::{Scope, SharedScope}; use crate::error::ToolsError; #[derive(Debug)] struct ScopedFsInner { - scope: Scope, + scope: SharedScope, pwd: PathBuf, } /// Scope-aware filesystem handle. Clone-cheap (`Arc` inside). +/// +/// The wrapped [`SharedScope`] is shared with every clone of this +/// `ScopedFs` and with whoever else holds the same `SharedScope` +/// handle (typically the owning Pod). Mutations to that `SharedScope` +/// propagate atomically; the next permission check inside any +/// `ScopedFs` reads the new view. #[derive(Debug, Clone)] pub struct ScopedFs { inner: Arc, @@ -37,15 +43,34 @@ pub struct WriteOutcome { } impl ScopedFs { - /// Create a new [`ScopedFs`] wrapping the given [`Scope`] and pwd. + /// Create a new [`ScopedFs`] wrapping `scope` and `pwd` in a fresh + /// [`SharedScope`]. Use [`ScopedFs::with_shared_scope`] when you + /// need the resulting `ScopedFs` to share scope state with another + /// holder of the `SharedScope` (typically the Pod). pub fn new(scope: Scope, pwd: PathBuf) -> Self { + Self::with_shared_scope(SharedScope::new(scope), pwd) + } + + /// Build a [`ScopedFs`] over an existing [`SharedScope`]. The + /// resulting handle and any future updates the caller pushes to + /// `scope` are observed by every clone of this `ScopedFs`. + pub fn with_shared_scope(scope: SharedScope, pwd: PathBuf) -> Self { Self { inner: Arc::new(ScopedFsInner { scope, pwd }), } } - /// The underlying [`Scope`]. - pub fn scope(&self) -> &Scope { + /// Snapshot the current scope. Cheap; the returned `Arc` is + /// a coherent point-in-time view that subsequent mutations do not + /// affect. + pub fn scope(&self) -> Arc { + self.inner.scope.snapshot() + } + + /// Shared scope handle backing this `ScopedFs`. Cloning it lets a + /// caller (usually the Pod) hold the same view and push updates + /// that are immediately reflected in subsequent permission checks. + pub fn shared_scope(&self) -> &SharedScope { &self.inner.scope } @@ -67,7 +92,7 @@ impl ScopedFs { if !path.is_absolute() { return Err(ToolsError::RelativePath(path.to_path_buf())); } - if !self.inner.scope.is_readable(path) { + if !self.inner.scope.load().is_readable(path) { return Err(ToolsError::OutOfScope(path.to_path_buf())); } let meta = std::fs::metadata(path).map_err(|e| match e.kind() { @@ -100,13 +125,15 @@ impl ScopedFs { if !path.is_absolute() { return Err(ToolsError::RelativePath(path.to_path_buf())); } - if !self.inner.scope.is_writable(path) { - return Err(if self.inner.scope.is_readable(path) { + let scope = self.inner.scope.load(); + if !scope.is_writable(path) { + return Err(if scope.is_readable(path) { ToolsError::ReadOnly(path.to_path_buf()) } else { ToolsError::OutOfScope(path.to_path_buf()) }); } + drop(scope); // Reject existing directory targets. match std::fs::metadata(path) { @@ -299,4 +326,118 @@ mod tests { let err = fs.write(dir.path(), b"x").unwrap_err(); assert!(matches!(err, ToolsError::IsDirectory(_))); } + + // ------------------------------------------------------------------------- + // Dynamic scope: SharedScope mutations propagate into ScopedFs decisions + // ------------------------------------------------------------------------- + + #[test] + fn add_allow_rule_through_shared_scope_grows_readable_set() { + use manifest::SharedScope; + + let dir = TempDir::new().unwrap(); + let extra = TempDir::new().unwrap(); + let extra_file = extra.path().join("x.txt"); + fs::write(&extra_file, b"hi").unwrap(); + + let shared = SharedScope::new(Scope::writable(dir.path()).unwrap()); + let fs = ScopedFs::with_shared_scope(shared.clone(), dir.path().to_path_buf()); + + // Before: extra is out of scope. + let err = fs.read_bytes(&extra_file).unwrap_err(); + assert!(matches!(err, ToolsError::OutOfScope(_))); + + // Push an allow(Read) rule. + shared + .update(|cur| { + cur.with_added_allow_rules([ScopeRule { + target: extra.path().to_path_buf(), + permission: Permission::Read, + recursive: true, + }]) + }) + .unwrap(); + + // After: read goes through. + assert_eq!(fs.read_bytes(&extra_file).unwrap(), b"hi"); + // But write still fails — allow only granted Read. + let err = fs.write(&extra.path().join("y.txt"), b"x").unwrap_err(); + assert!( + matches!(err, ToolsError::ReadOnly(_)), + "expected ReadOnly, got {err:?}" + ); + } + + #[test] + fn revoke_write_through_shared_scope_blocks_subsequent_writes() { + use manifest::SharedScope; + + let dir = TempDir::new().unwrap(); + let sub = dir.path().join("sub"); + fs::create_dir(&sub).unwrap(); + let target = sub.join("a.txt"); + + let shared = SharedScope::new(Scope::writable(dir.path()).unwrap()); + let fs = ScopedFs::with_shared_scope(shared.clone(), dir.path().to_path_buf()); + + // Write succeeds initially. + fs.write(&target, b"first").unwrap(); + + // Revoke Write on `sub` (push a deny(Write) rule). + shared + .update(|cur| { + cur.with_added_deny_rules([ScopeRule { + target: sub.clone(), + permission: Permission::Write, + recursive: true, + }]) + }) + .unwrap(); + + // Subsequent write fails with ReadOnly — Read is preserved. + let err = fs.write(&target, b"second").unwrap_err(); + assert!( + matches!(err, ToolsError::ReadOnly(_)), + "expected ReadOnly after revoke, got {err:?}" + ); + // Read still works. + assert_eq!(fs.read_bytes(&target).unwrap(), b"first"); + } + + #[test] + fn shared_scope_changes_propagate_across_clones() { + use manifest::SharedScope; + + let dir = TempDir::new().unwrap(); + let target = dir.path().join("a.txt"); + + let shared = SharedScope::new(Scope::writable(dir.path()).unwrap()); + let fs1 = ScopedFs::with_shared_scope(shared.clone(), dir.path().to_path_buf()); + let fs2 = fs1.clone(); + + // fs1 writes; both clones see the file. + fs1.write(&target, b"hi").unwrap(); + assert_eq!(fs2.read_bytes(&target).unwrap(), b"hi"); + + // Revoke write through the original handle. + shared + .update(|cur| { + cur.with_added_deny_rules([ScopeRule { + target: dir.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + }]) + }) + .unwrap(); + + // Both clones reject writes now — they share the same SharedScope. + assert!(matches!( + fs1.write(&target, b"x").unwrap_err(), + ToolsError::ReadOnly(_) + )); + assert!(matches!( + fs2.write(&target, b"x").unwrap_err(), + ToolsError::ReadOnly(_) + )); + } }