diff --git a/crates/pod-registry/src/conflict.rs b/crates/pod-registry/src/conflict.rs index ba63054f..2ce8aadc 100644 --- a/crates/pod-registry/src/conflict.rs +++ b/crates/pod-registry/src/conflict.rs @@ -210,6 +210,7 @@ mod tests { std::process::id(), sock("b"), vec![write_rule("/src/core", true)], + &delegation_scope(vec![write_rule("/src", true)]), ) .unwrap(); // A different top-level Pod trying to register /src/core/x diff --git a/crates/pod-registry/src/error.rs b/crates/pod-registry/src/error.rs index 62a6e481..44d79659 100644 --- a/crates/pod-registry/src/error.rs +++ b/crates/pod-registry/src/error.rs @@ -3,7 +3,7 @@ use std::io; use std::path::PathBuf; -use manifest::ScopeRule; +use manifest::{ScopeError, ScopeRule}; use session_store::SegmentId; /// Errors raised by the mutating pod-registry operations. @@ -20,10 +20,12 @@ pub enum ScopeLockError { competitor_rule: ScopeRule, }, #[error( - "requested scope `{}` is not within spawner `{spawner}`'s effective scope", + "requested scope `{}` is not within spawner `{spawner}`'s delegation scope", .rule.target.display() )] NotSubset { spawner: String, rule: ScopeRule }, + #[error("invalid delegation scope: {source}")] + InvalidScope { source: ScopeError }, #[error("pod `{0}` is not registered")] UnknownPod(String), #[error( diff --git a/crates/pod-registry/src/mutate.rs b/crates/pod-registry/src/mutate.rs index 8ca49ce5..2213ca78 100644 --- a/crates/pod-registry/src/mutate.rs +++ b/crates/pod-registry/src/mutate.rs @@ -4,10 +4,10 @@ use std::io; use std::path::PathBuf; -use manifest::{Permission, ScopeRule}; +use manifest::{DelegationScope, Permission, ScopeRule}; use session_store::SegmentId; -use crate::conflict::{find_conflict_owner, find_conflict_owners, is_within_effective_write}; +use crate::conflict::{find_conflict_owner, find_conflict_owners}; use crate::error::ScopeLockError; use crate::table::{Allocation, LockFileGuard}; @@ -106,8 +106,8 @@ pub fn register_pod_with_deny( } /// Register a spawned Pod whose scope is delegated from `spawner`. -/// The requested scope must be within `spawner`'s effective write -/// scope; overlap with any Pod other than `spawner` is a conflict. +/// The requested scope must be within the spawner's delegation authority; +/// overlap with any Pod other than `spawner` is a conflict. pub fn delegate_scope( guard: &mut LockFileGuard, spawner: &str, @@ -115,6 +115,7 @@ pub fn delegate_scope( pid: u32, socket: PathBuf, scope_allow: Vec, + delegation_scope: &DelegationScope, ) -> Result<(), ScopeLockError> { reclaim_stale(guard); if guard.data().find(&spawned).is_some() { @@ -124,7 +125,10 @@ pub fn delegate_scope( return Err(ScopeLockError::UnknownPod(spawner.into())); } for rule in &scope_allow { - if !is_within_effective_write(guard.data(), spawner, rule) { + let allowed = delegation_scope + .allows_rule(rule) + .map_err(|source| ScopeLockError::InvalidScope { source })?; + if !allowed { return Err(ScopeLockError::NotSubset { spawner: spawner.into(), rule: rule.clone(), @@ -363,11 +367,42 @@ mod tests { std::process::id(), sock("b"), vec![write_rule("/docs", true)], + &delegation_scope(vec![write_rule("/src", true)]), ) .unwrap_err(); assert!(matches!(err, ScopeLockError::NotSubset { .. })); } + #[test] + fn delegate_uses_delegation_scope_not_direct_effective_write() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("pods.json"); + let mut g = open_empty(&path); + register_pod( + &mut g, + "orchestrator".into(), + std::process::id(), + sock("orchestrator"), + vec![read_rule("/workspace", true)], + sid(), + ) + .unwrap(); + + delegate_scope( + &mut g, + "orchestrator", + "coder".into(), + std::process::id(), + sock("coder"), + vec![write_rule("/workspace/.worktree/task", true)], + &delegation_scope(vec![write_rule("/workspace", true)]), + ) + .unwrap(); + + let coder = g.data().find("coder").expect("coder allocation"); + assert_eq!(coder.delegated_from.as_deref(), Some("orchestrator")); + } + #[test] fn delegate_succeeds_within_parent_scope() { let dir = TempDir::new().unwrap(); @@ -389,6 +424,7 @@ mod tests { std::process::id(), sock("b"), vec![write_rule("/src/core", true)], + &delegation_scope(vec![write_rule("/src", true)]), ) .unwrap(); assert_eq!(g.data().allocations.len(), 2); @@ -427,6 +463,7 @@ mod tests { std::process::id(), sock("b"), vec![write_rule("/src/core", true)], + &delegation_scope(vec![write_rule("/src", true)]), ) .unwrap(); // Sibling C from A tries to take /src/core/sub — already under B's scope. @@ -437,10 +474,13 @@ mod tests { std::process::id(), sock("c"), vec![write_rule("/src/core/sub", true)], + &delegation_scope(vec![write_rule("/src", true)]), ) .unwrap_err(); - // NotSubset fires first because /src/core is no longer in A's effective. - assert!(matches!(err, ScopeLockError::NotSubset { .. })); + match err { + ScopeLockError::WriteConflict { competitor, .. } => assert_eq!(competitor, "b"), + other => panic!("expected WriteConflict, got {other:?}"), + } } #[test] @@ -464,6 +504,7 @@ mod tests { std::process::id(), sock("b"), vec![write_rule("/src/core", true)], + &delegation_scope(vec![write_rule("/src", true)]), ) .unwrap(); delegate_scope( @@ -473,6 +514,7 @@ mod tests { std::process::id(), sock("d"), vec![write_rule("/src/core/x", true)], + &delegation_scope(vec![write_rule("/src/core", true)]), ) .unwrap(); release_pod(&mut g, "b").unwrap(); @@ -571,6 +613,7 @@ mod tests { std::process::id(), sock("b"), vec![write_rule("/src/core", true)], + &delegation_scope(vec![write_rule("/src", true)]), ) .unwrap(); delegate_scope( @@ -580,6 +623,7 @@ mod tests { std::process::id(), sock("d"), vec![write_rule("/src/core/x", true)], + &delegation_scope(vec![write_rule("/src/core", true)]), ) .unwrap(); // Simulate B crashing by rewriting its pid to one the probe @@ -670,6 +714,7 @@ mod tests { std::process::id(), sock("b"), vec![write_rule("/src/core", true)], + &delegation_scope(vec![write_rule("/src", true)]), ) .unwrap(); assert!(!is_within_effective_write( diff --git a/crates/pod-registry/src/table.rs b/crates/pod-registry/src/table.rs index dce8502b..02f63111 100644 --- a/crates/pod-registry/src/table.rs +++ b/crates/pod-registry/src/table.rs @@ -245,6 +245,7 @@ mod tests { std::process::id(), sock("child"), vec![write_rule("/p/sub", true)], + &delegation_scope(vec![write_rule("/p", true)]), ) .unwrap(); diff --git a/crates/pod-registry/src/test_util.rs b/crates/pod-registry/src/test_util.rs index 30f5b773..2ec9dbcf 100644 --- a/crates/pod-registry/src/test_util.rs +++ b/crates/pod-registry/src/test_util.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; use std::sync::{LazyLock, Mutex, MutexGuard}; -use manifest::{Permission, ScopeRule}; +use manifest::{DelegationScope, Permission, ScopeConfig, ScopeRule}; use session_store::SegmentId; use crate::table::LockFileGuard; @@ -88,6 +88,14 @@ pub(crate) fn read_rule(path: &str, recursive: bool) -> ScopeRule { } } +pub(crate) fn delegation_scope(rules: Vec) -> DelegationScope { + DelegationScope::from_config(&ScopeConfig { + allow: rules, + deny: Vec::new(), + }) + .expect("test delegation scope") +} + pub(crate) fn sock(name: &str) -> PathBuf { PathBuf::from(format!("/tmp/{name}.sock")) } diff --git a/crates/pod/src/spawn/tool.rs b/crates/pod/src/spawn/tool.rs index bc345438..019dd88f 100644 --- a/crates/pod/src/spawn/tool.rs +++ b/crates/pod/src/spawn/tool.rs @@ -352,6 +352,7 @@ impl Tool for SpawnPodTool { std::process::id(), predicted_socket.clone(), scope_allow.clone(), + &self.delegation_scope, ) .map_err(pod_registry_err_to_tool)?; } @@ -868,6 +869,7 @@ fn pod_registry_err_to_tool(e: ScopeLockError) -> ToolError { | ScopeLockError::WriteConflict { .. } | ScopeLockError::DuplicatePodName(_) | ScopeLockError::UnknownPod(_) + | ScopeLockError::InvalidScope { .. } | ScopeLockError::SegmentConflict { .. } => ToolError::InvalidArgument(e.to_string()), ScopeLockError::Io(_) => ToolError::ExecutionFailed(e.to_string()), }