From 7321d6c7c3f9e13f60559cab4e48ca6863ff62d6 Mon Sep 17 00:00:00 2001 From: Hare Date: Sat, 10 Oct 2026 02:12:29 +0900 Subject: [PATCH] fix: forward scoped Workdir delegation checks Forward provider authorization and overlap checks through ScopedWorkdirSession while preserving capability ceilings, path rebasing, write leases, and revocation. Exercise stacked wrappers, External providers, and remote HTTP sessions without weakening resolved symlink policy. --- crates/workdir/src/checkout_search_tests.rs | 10 +- crates/workdir/src/scope.rs | 47 +++ crates/workdir/src/scope_delegation_tests.rs | 334 ++++++++++++++++++ crates/workspace-server/src/server.rs | 1 + .../server/tests/workdir_delegation_tests.rs | 259 ++++++++++++++ 5 files changed, 646 insertions(+), 5 deletions(-) create mode 100644 crates/workdir/src/scope_delegation_tests.rs create mode 100644 crates/workspace-server/src/server/tests/workdir_delegation_tests.rs diff --git a/crates/workdir/src/checkout_search_tests.rs b/crates/workdir/src/checkout_search_tests.rs index 44c81488e..2777088b7 100644 --- a/crates/workdir/src/checkout_search_tests.rs +++ b/crates/workdir/src/checkout_search_tests.rs @@ -212,17 +212,17 @@ async fn checkout_search_composes_actual_nested_wrappers_and_output_root() { }; assert_eq!(result.entries[0].path, path("keep/yes.txt")); // A second actual scoped wrapper has its own cwd and nonrecursive layer. - // Logical rules avoid requesting resolved authorization from a wrapper - // that conservatively does not expose host-side resolution authority. + // Default resolved authorization must survive every wrapper. std::fs::create_dir(root.path().join("child/keep/deep")).unwrap(); std::fs::write( root.path().join("child/keep/deep/secret"), "needle hidden nested", ) .unwrap(); - let mut own_rule = rule("keep", false); - own_rule.symlink_policy = manifest::SymlinkPolicy::Logical; - let second = stacked.scope(scope(vec![own_rule], "keep")).await.unwrap(); + let second = stacked + .scope(scope(vec![rule("keep", false)], "keep")) + .await + .unwrap(); let result = second .tool_session() .grep(grep(GrepOutputMode::Content)) diff --git a/crates/workdir/src/scope.rs b/crates/workdir/src/scope.rs index e36e3cd88..8bde43c29 100644 --- a/crates/workdir/src/scope.rs +++ b/crates/workdir/src/scope.rs @@ -994,6 +994,51 @@ impl WorkdirSession for ScopedWorkdirSession { self.capabilities } + async fn authorize_scope_path( + &self, + mut request: WorkdirScopeAuthorizationRequest, + ) -> Result<(), WorkdirError> { + let _scope_guard = self.scope_lock.lock().await; + // Like filesystem operations, requests use this session's coordinates. + // Keep every wrapper's attenuation before asking the provider to resolve + // symlinks; forwarding the caller's rules alone would discard our scope. + for rule in &mut request.rules { + rule.target = self.resolve_path(&rule.target)?; + } + self.validate_scope(&request.rules, false)?; + request.path = self + .resolve_operation_path(&request.path, request.permission) + .await?; + match request.permission { + WorkdirToolScopePermission::Read => { + self.ensure_read(&request.path, WorkdirSessionCapability::Read)?; + } + WorkdirToolScopePermission::Write => { + self.ensure_write(&request.path, WorkdirSessionCapability::Write)?; + } + } + self.source.authorize_scope_path(request).await?; + self.ensure_active() + } + + async fn scope_rules_overlap( + &self, + mut request: WorkdirScopeOverlapRequest, + ) -> Result { + let _scope_guard = self.scope_lock.lock().await; + // Comparing write rules is a read-only query, not a write authorization. + // In particular it must work while those paths are leased to children. + for rule in [&mut request.left, &mut request.right] { + rule.target = self + .resolve_operation_path(&rule.target, WorkdirToolScopePermission::Read) + .await?; + self.ensure_read(&rule.target, WorkdirSessionCapability::Read)?; + } + let overlaps = self.source.scope_rules_overlap(request).await?; + self.ensure_active()?; + Ok(overlaps) + } + async fn checkout_search( &self, mut request: crate::CheckoutSearchRequest, @@ -1712,6 +1757,8 @@ mod tests { use super::*; use crate::LocalWorkdirSession; + include!("scope_delegation_tests.rs"); + fn fs_path(path: &str) -> FsPath { FsPath::new(path).unwrap() } diff --git a/crates/workdir/src/scope_delegation_tests.rs b/crates/workdir/src/scope_delegation_tests.rs new file mode 100644 index 000000000..9073c0221 --- /dev/null +++ b/crates/workdir/src/scope_delegation_tests.rs @@ -0,0 +1,334 @@ +// Included in scope::tests; exercise real provider resolution through stacked brokers. + +fn filesystem_scope(path: &str, permission: WorkdirToolScopePermission) -> WorkdirToolScope { + let mut scope = request(path, permission); + scope.command = false; + scope +} + +fn authorization( + path: &str, + permission: WorkdirToolScopePermission, +) -> WorkdirScopeAuthorizationRequest { + WorkdirScopeAuthorizationRequest { + rules: filesystem_scope("", permission).rules, + path: fs_path(path), + permission, + } +} + +#[tokio::test] +async fn stacked_brokers_delegate_all_filesystem_provider_kinds() { + use crate::{BoundedReadLimits, ExternalWorkdirRoot}; + for kind in [ + "repository", + "read-only-wrapper", + "external-read", + "external-write", + "external-read-command", + "external-all", + ] { + let root = TempDir::new().unwrap(); + fs::create_dir(root.path().join("child")).unwrap(); + fs::create_dir(root.path().join("other")).unwrap(); + fs::write(root.path().join("child/input"), "visible").unwrap(); + fs::write(root.path().join("other/secret"), "hidden").unwrap(); + #[cfg(unix)] + std::os::unix::fs::symlink("../other", root.path().join("child/escape")).unwrap(); + let capabilities = match kind { + "read-only-wrapper" | "external-read" => WorkdirSessionCapabilities::READ_ONLY, + "external-write" => WorkdirSessionCapabilities::READ_WRITE, + "external-read-command" => WorkdirSessionCapabilities::READ_ONLY + .union(WorkdirSessionCapabilities::COMMAND_ONLY), + _ => WorkdirSessionCapabilities::ALL, + }; + let source: WorkdirSessionHandle = if kind.starts_with("external-") { + Arc::new( + LocalWorkdirSession::external_with_capabilities_pinned( + Workdir::new(kind), + ExternalWorkdirRoot::pin(root.path()).unwrap(), + BoundedReadLimits::new(4096, 1024).unwrap(), + capabilities, + ) + .unwrap(), + ) + } else { + let source: WorkdirSessionHandle = Arc::new(LocalWorkdirSession::new( + Scope::writable(root.path()).unwrap(), + root.path().to_path_buf(), + )); + if kind == "read-only-wrapper" { + Arc::new(ReadOnlyWorkdirSession::new(source)) + } else { + source + } + }; + // Backend capability wrapper, attachment routing, then the Worker broker. + let backend = WorkdirToolBroker::with_capabilities(source, capabilities); + let router = crate::WorkdirSessionRouter::new(); + let routed = router + .attach( + crate::WorkdirAttachmentAlias::new("selected").unwrap(), + backend.tool_session(), + ) + .unwrap(); + let worker = WorkdirToolBroker::new(routed); + let child = worker + .scope(filesystem_scope("child", WorkdirToolScopePermission::Read)) + .await + .unwrap(); + assert_eq!( + child.read(read("input")).await.unwrap().bytes, + b"visible", + "{kind}" + ); + assert!(child.write(write("denied", "no")).await.is_err(), "{kind}"); + #[cfg(unix)] + assert!(child.read(read("escape/secret")).await.is_err(), "{kind}"); + child.close().await.unwrap(); + let writable = worker + .scope(filesystem_scope("child", WorkdirToolScopePermission::Write)) + .await; + if !capabilities.supports(WorkdirSessionCapability::Write) { + assert!(writable.is_err(), "{kind}"); + continue; + } + let writable = writable.unwrap(); + writable.write(write("created", "child")).await.unwrap(); + assert!( + worker.write(write("child/blocked", "no")).await.is_err(), + "{kind}" + ); + assert!( + worker + .scope(filesystem_scope("child", WorkdirToolScopePermission::Write)) + .await + .is_err(), + "{kind}" + ); + // A non-overlapping write lease requires the second missing provider method. + let sibling = worker + .scope(filesystem_scope("other", WorkdirToolScopePermission::Write)) + .await + .unwrap(); + sibling.write(write("created", "sibling")).await.unwrap(); + assert!( + !writable + .capabilities + .supports(WorkdirSessionCapability::Command) + ); + writable.close().await.unwrap(); + worker + .write(write("child/released", "parent")) + .await + .unwrap(); + sibling.close().await.unwrap(); + let command_child = worker + .scope(request("child", WorkdirToolScopePermission::Write)) + .await; + if capabilities.supports(WorkdirSessionCapability::Command) { + let command_child = command_child.unwrap(); + assert_eq!( + run_command(&command_child.tool_session(), "printf delegated", kind) + .await + .content, + "delegated" + ); + command_child.close().await.unwrap(); + } else { + assert!(command_child.is_err()); + } + } +} + +#[tokio::test] +async fn stacked_authorization_preserves_ceiling_scope_and_revocation() { + let root = TempDir::new().unwrap(); + fs::create_dir_all(root.path().join("allowed/deep")).unwrap(); + let backend = session(root.path()); + let read_only = WorkdirToolBroker::with_capabilities( + backend.tool_session(), + WorkdirSessionCapabilities::READ_ONLY, + ); + read_only + .authorize_scope_path(authorization("allowed", WorkdirToolScopePermission::Read)) + .await + .unwrap(); + assert!( + read_only + .authorize_scope_path(authorization("allowed", WorkdirToolScopePermission::Write)) + .await + .is_err() + ); + let command_only = WorkdirToolBroker::with_capabilities( + backend.tool_session(), + WorkdirSessionCapabilities::COMMAND_ONLY, + ); + assert!( + command_only + .authorize_scope_path(authorization("allowed", WorkdirToolScopePermission::Read)) + .await + .is_err() + ); + assert!( + command_only + .scope(filesystem_scope( + "allowed", + WorkdirToolScopePermission::Read + )) + .await + .is_err() + ); + + let mut limited = filesystem_scope("allowed", WorkdirToolScopePermission::Read); + limited.rules[0].recursive = false; + let parent = backend.scope(limited).await.unwrap(); + let stacked = WorkdirToolBroker::new(parent.tool_session()); + let mut valid = filesystem_scope("", WorkdirToolScopePermission::Read); + valid.rules[0].recursive = false; + let child = stacked.scope(valid.clone()).await.unwrap(); + assert!( + stacked + .scope(filesystem_scope("", WorkdirToolScopePermission::Read)) + .await + .is_err() + ); + let mut logical = valid.clone(); + logical.rules[0].symlink_policy = SymlinkPolicy::Logical; + assert!(stacked.scope(logical).await.is_err()); + assert!( + stacked + .authorize_scope_path(authorization("deep/file", WorkdirToolScopePermission::Read)) + .await + .is_err() + ); + parent.close().await.unwrap(); + assert!(stacked.scope(valid).await.is_err()); + assert!(child.read(read("deep/file")).await.is_err()); + assert!( + stacked + .scope_rules_overlap(WorkdirScopeOverlapRequest { + left: filesystem_scope("", WorkdirToolScopePermission::Read) + .rules + .remove(0), + right: filesystem_scope("", WorkdirToolScopePermission::Read) + .rules + .remove(0), + }) + .await + .is_err() + ); +} + +#[tokio::test] +async fn stacked_scope_rebases_rules_paths_and_overlap_once() { + let root = TempDir::new().unwrap(); + fs::create_dir_all(root.path().join("base/inner/one")).unwrap(); + fs::create_dir_all(root.path().join("base/inner/two")).unwrap(); + fs::write(root.path().join("base/inner/one/input"), "nested").unwrap(); + let backend = session(root.path()); + let first = backend + .scope(filesystem_scope("base", WorkdirToolScopePermission::Write)) + .await + .unwrap(); + let worker = WorkdirToolBroker::new(first.tool_session()); + let second = worker + .scope(filesystem_scope("inner", WorkdirToolScopePermission::Write)) + .await + .unwrap(); + let nested = WorkdirToolBroker::new(second.tool_session()); + let leaf = nested + .scope(filesystem_scope("one", WorkdirToolScopePermission::Write)) + .await + .unwrap(); + assert_eq!(leaf.read(read("input")).await.unwrap().bytes, b"nested"); + leaf.write(write("output", "yes")).await.unwrap(); + assert_eq!( + fs::read(root.path().join("base/inner/one/output")).unwrap(), + b"yes" + ); + assert!(nested.write(write("one/blocked", "no")).await.is_err()); + let sibling = nested + .scope(filesystem_scope("two", WorkdirToolScopePermission::Write)) + .await + .unwrap(); + sibling.write(write("output", "sibling")).await.unwrap(); + assert!( + nested + .scope(filesystem_scope("one", WorkdirToolScopePermission::Write)) + .await + .is_err() + ); + leaf.close().await.unwrap(); + nested.write(write("one/released", "yes")).await.unwrap(); + sibling.close().await.unwrap(); + first.close().await.unwrap(); + assert!( + nested + .authorize_scope_path(authorization("one", WorkdirToolScopePermission::Read)) + .await + .is_err() + ); +} + +#[cfg(unix)] +#[tokio::test] +async fn stacked_scope_keeps_resolved_symlink_and_lease_checks() { + use std::os::unix::fs::symlink; + let root = TempDir::new().unwrap(); + fs::create_dir_all(root.path().join("base/target")).unwrap(); + fs::create_dir(root.path().join("secret")).unwrap(); + fs::write(root.path().join("secret/key"), "hidden").unwrap(); + symlink("target", root.path().join("base/alias-a")).unwrap(); + symlink("target", root.path().join("base/alias-b")).unwrap(); + symlink("../../secret", root.path().join("base/target/escape")).unwrap(); + let backend = session(root.path()); + let parent = backend + .scope(filesystem_scope("base", WorkdirToolScopePermission::Write)) + .await + .unwrap(); + let worker = WorkdirToolBroker::new(parent.tool_session()); + let first = worker + .scope(filesystem_scope( + "alias-a", + WorkdirToolScopePermission::Write, + )) + .await + .unwrap(); + first.write(write("ok", "allowed")).await.unwrap(); + assert!(first.read(read("escape/key")).await.is_err()); + assert!( + worker + .scope(filesystem_scope( + "alias-a/escape", + WorkdirToolScopePermission::Read + )) + .await + .is_err() + ); + assert!( + worker + .scope(filesystem_scope( + "alias-b", + WorkdirToolScopePermission::Write + )) + .await + .is_err() + ); + assert!(worker.write(write("target/blocked", "no")).await.is_err()); + // Direct authorization through the wrapper must not bypass its existing write lease. + assert!( + worker + .authorize_scope_path(authorization( + "target/blocked", + WorkdirToolScopePermission::Write + )) + .await + .is_err() + ); + first.close().await.unwrap(); + worker + .write(write("alias-b/released", "yes")) + .await + .unwrap(); +} diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 9f1edc803..134cf130d 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -32912,6 +32912,7 @@ mod tests { mod ticket_worker_claim_tests; mod ticket_worker_launch_tests; mod value_profiles_tests; + mod workdir_delegation_tests; mod worker_drive_tests; mod worker_operations_tests; include!("server_workspace_config_tests.rs"); diff --git a/crates/workspace-server/src/server/tests/workdir_delegation_tests.rs b/crates/workspace-server/src/server/tests/workdir_delegation_tests.rs new file mode 100644 index 000000000..17240665a --- /dev/null +++ b/crates/workspace-server/src/server/tests/workdir_delegation_tests.rs @@ -0,0 +1,259 @@ +use super::*; +use workdir::{ + BoundedReadLimits, LocalWorkdirSession, Workdir, WorkdirPath, + WorkdirSessionCapabilities as Caps, WorkdirToolBroker, WorkdirToolScope, + WorkdirToolScopePermission as Permission, WorkdirToolScopeRule, +}; + +fn scope(target: &str, permission: Permission) -> WorkdirToolScope { + WorkdirToolScope { + rules: vec![WorkdirToolScopeRule { + target: WorkdirPath::new(target).unwrap(), + permission, + recursive: true, + symlink_policy: Default::default(), + }], + cwd: WorkdirPath::new(target).unwrap(), + command: false, + } +} + +async fn verify_backend_delegation(source: WorkdirSessionHandle, capabilities: Caps) { + // The same capability wrapper returned by both Backend attachment branches. + let backend = WorkdirToolBroker::with_capabilities(source, capabilities); + let session = backend.tool_session(); + let authorization = workdir::WorkdirScopeAuthorizationRequest { + rules: scope("child", Permission::Read).rules, + path: WorkdirPath::new("child").unwrap(), + permission: Permission::Read, + }; + assert!(matches!( + execute_workdir_session_operation( + &session, + WorkdirSessionOperation::AuthorizeScope(authorization) + ) + .await + .unwrap(), + WorkdirSessionOperationResult::AuthorizeScope + )); + let worker = WorkdirToolBroker::new(session.clone()); + let child = worker + .scope(scope("child", Permission::Read)) + .await + .unwrap(); + assert_eq!( + child + .read(workdir::ReadRequest { + path: WorkdirPath::new("input").unwrap(), + offset: 0, + limit: 10, + max_bytes: 1024, + }) + .await + .unwrap() + .bytes, + b"visible" + ); + child.close().await.unwrap(); + + // Comparison is read-only even when the compared rules describe writes. + let overlap = workdir::WorkdirScopeOverlapRequest { + left: scope("child", Permission::Write).rules.remove(0), + right: scope("other", Permission::Write).rules.remove(0), + }; + assert!(matches!( + execute_workdir_session_operation( + &session, + WorkdirSessionOperation::ScopeRulesOverlap(overlap) + ) + .await + .unwrap(), + WorkdirSessionOperationResult::ScopeRulesOverlap { overlaps: false } + )); + let writer = worker.scope(scope("child", Permission::Write)).await; + if !capabilities.supports(workdir::WorkdirSessionCapability::Write) { + assert!(writer.is_err()); + return; + } + let writer = writer.unwrap(); + let sibling = worker + .scope(scope("other", Permission::Write)) + .await + .unwrap(); + let write = |path: &str| workdir::WriteRequest { + path: WorkdirPath::new(path).unwrap(), + content: b"written".to_vec(), + expected_hash: None, + }; + writer.write(write("created")).await.unwrap(); + assert!(worker.write(write("child/blocked")).await.is_err()); + assert!( + worker + .scope(scope("child", Permission::Write)) + .await + .is_err() + ); + sibling.write(write("created")).await.unwrap(); + writer.close().await.unwrap(); + worker.write(write("child/released")).await.unwrap(); + sibling.close().await.unwrap(); +} + +fn fixture() -> tempfile::TempDir { + let root = tempfile::tempdir().unwrap(); + fs::create_dir(root.path().join("child")).unwrap(); + fs::create_dir(root.path().join("other")).unwrap(); + fs::write(root.path().join("child/input"), "visible").unwrap(); + root +} + +#[tokio::test] +async fn backend_delegation_reaches_external_provider_resolution() { + for capabilities in [Caps::READ_ONLY, Caps::READ_WRITE] { + let root = fixture(); + let local = LocalWorkdirSession::external_with_capabilities_pinned( + Workdir::new("external-delegation"), + workdir::ExternalWorkdirRoot::pin(root.path()).unwrap(), + BoundedReadLimits::new(4096, 1024).unwrap(), + capabilities, + ) + .unwrap(); + let (sender, mut commands) = tokio::sync::mpsc::channel(16); + let source = Arc::new(ExternalProviderWorkdirSession::new( + Arc::new(ExternalProviderConnection { + grant_id: "delegation-grant".into(), + workdir_id: "external-delegation".into(), + provider_instance_id: "delegation-provider".into(), + generation: 1, + expires_at: None, + capabilities, + admission: Arc::new(tokio::sync::Semaphore::new(16)), + shutdown_confirmed: Arc::new(std::sync::atomic::AtomicBool::new(false)), + sender, + }), + None, + )); + // Real external session transport and real provider authorization, not + // canned allow responses. Wire serialization also preserves the rules. + let provider = tokio::spawn(async move { + let mut authorizations = 0; + let mut overlaps = 0; + while let Some(command) = commands.recv().await { + let ExternalProviderCommand::Operation { + operation, + response, + .. + } = command + else { + panic!("unexpected provider control command"); + }; + let operation = + serde_json::from_slice(&serde_json::to_vec(&operation).unwrap()).unwrap(); + match &operation { + WorkdirSessionOperation::AuthorizeScope(_) => authorizations += 1, + WorkdirSessionOperation::ScopeRulesOverlap(_) => overlaps += 1, + _ => {} + } + let result = workdir::dispatch_workdir_session_operation(&local, operation) + .await + .map_err(|error| WorkdirTransportError::from_workdir_error(&error)); + response.send(result).unwrap(); + } + (authorizations, overlaps) + }); + tokio::time::timeout( + std::time::Duration::from_secs(10), + verify_backend_delegation(source, capabilities), + ) + .await + .unwrap(); + let (authorizations, overlaps) = + tokio::time::timeout(std::time::Duration::from_secs(5), provider) + .await + .unwrap() + .unwrap(); + assert!(authorizations >= 3); + assert!(overlaps >= 1); + } +} + +#[tokio::test] +async fn backend_delegation_reaches_remote_repository_resolution() { + use workdir::http::{ + OpenWorkdirSessionResponse, RemoteWorkdirSession, WorkdirSessionId, + WorkdirSessionOperationRequest, + }; + let root = fixture(); + let source: WorkdirSessionHandle = Arc::new(LocalWorkdirSession::new( + manifest::Scope::writable(root.path()).unwrap(), + root.path().to_path_buf(), + )); + let workdir_id = source.workdir().id().clone(); + let response_id = workdir_id.clone(); + let app = Router::new() + .route( + "/v1/working-directories/{id}/sessions", + axum::routing::post(move || { + let workdir_id = response_id.clone(); + async move { + Json(OpenWorkdirSessionResponse { + session_id: WorkdirSessionId::new("delegation-session").unwrap(), + workdir_id, + capabilities: Caps::ALL, + }) + } + }), + ) + .route( + "/v1/workdir-sessions/{id}/operations", + axum::routing::post(move |Json(request): Json| { + let source = source.clone(); + async move { + match workdir::dispatch_workdir_session_operation( + source.as_ref(), + request.operation, + ) + .await + { + Ok(result) => Json(result).into_response(), + Err(error) => { + let error = WorkdirTransportError::from_workdir_error(&error); + ( + StatusCode::from_u16(error.code.http_status()).unwrap(), + Json(error), + ) + .into_response() + } + } + } + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let (shutdown, finished) = tokio::sync::oneshot::channel::<()>(); + let server = tokio::spawn(async move { + axum::serve(listener, app) + .with_graceful_shutdown(async { + let _ = finished.await; + }) + .await + .unwrap(); + }); + let remote = RemoteWorkdirSession::open( + reqwest::Client::new(), + format!("http://{address}").parse().unwrap(), + "test-session-token", + workdir_id, + Default::default(), + ) + .await + .unwrap(); + tokio::time::timeout( + std::time::Duration::from_secs(10), + verify_backend_delegation(Arc::new(remote), Caps::ALL), + ) + .await + .unwrap(); + shutdown.send(()).unwrap(); + server.await.unwrap(); +}