From 5f2798458e2d2e78952371d24b8fb36f3a8cda98 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 19 Aug 2026 11:22:56 +0900 Subject: [PATCH] fix: align delegated paths with provider resolution --- crates/manifest/src/scope.rs | 6 ++ crates/workdir/src/delegation.rs | 83 +++++++++++++++---- crates/workdir/src/http.rs | 4 + crates/workdir/src/lib.rs | 6 ++ crates/workdir/src/local.rs | 23 ++++- crates/worker-runtime/src/http_server.rs | 32 +++++++ .../src/feature/builtin/manage_workdir.rs | 6 +- 7 files changed, 144 insertions(+), 16 deletions(-) diff --git a/crates/manifest/src/scope.rs b/crates/manifest/src/scope.rs index 29bec5c8..21fab0d4 100644 --- a/crates/manifest/src/scope.rs +++ b/crates/manifest/src/scope.rs @@ -214,6 +214,12 @@ impl Scope { }) } + /// Resolve one rule target with the same symlink and missing-tail semantics + /// used by scope matching. + pub fn resolved_target(rule: &ScopeRule) -> Result { + Ok(resolve_rule(rule)?.target) + } + /// Return whether this effective scope fully contains a requested rule. /// This is used when attenuating provider authority without mutating the /// parent scope. diff --git a/crates/workdir/src/delegation.rs b/crates/workdir/src/delegation.rs index 774c27f3..892e4084 100644 --- a/crates/workdir/src/delegation.rs +++ b/crates/workdir/src/delegation.rs @@ -360,6 +360,10 @@ impl WorkdirSession for DelegatingWorkdirSession { true } + fn transports_delegation_context(&self) -> bool { + self.source.transports_delegation_context() + } + async fn capture_delegation_source( &self, request: &WorkdirDelegationRequest, @@ -431,44 +435,65 @@ impl WorkdirSession for DelegatingWorkdirSession { } async fn stat(&self, mut request: StatRequest) -> Result { - request.path = self.resolve_path(&request.path)?; - self.ensure_read(&request.path, WorkdirSessionCapability::Read)?; + let path = self.resolve_path(&request.path)?; + self.ensure_read(&path, WorkdirSessionCapability::Read)?; + if !self.source.transports_delegation_context() { + request.path = path; + } self.source.stat(request).await } async fn read(&self, mut request: ReadRequest) -> Result { - request.path = self.resolve_path(&request.path)?; - self.ensure_read(&request.path, WorkdirSessionCapability::Read)?; + let path = self.resolve_path(&request.path)?; + self.ensure_read(&path, WorkdirSessionCapability::Read)?; + if !self.source.transports_delegation_context() { + request.path = path; + } self.source.read(request).await } async fn write(&self, mut request: WriteRequest) -> Result { - request.path = self.resolve_path(&request.path)?; - self.ensure_write(&request.path, WorkdirSessionCapability::Write)?; + let path = self.resolve_path(&request.path)?; + self.ensure_write(&path, WorkdirSessionCapability::Write)?; + if !self.source.transports_delegation_context() { + request.path = path; + } self.source.write(request).await } async fn edit(&self, mut request: EditRequest) -> Result { - request.path = self.resolve_path(&request.path)?; - self.ensure_write(&request.path, WorkdirSessionCapability::Edit)?; + let path = self.resolve_path(&request.path)?; + self.ensure_write(&path, WorkdirSessionCapability::Edit)?; + if !self.source.transports_delegation_context() { + request.path = path; + } self.source.edit(request).await } async fn list(&self, mut request: ListRequest) -> Result { - request.path = self.resolve_path(&request.path)?; - self.ensure_read(&request.path, WorkdirSessionCapability::Read)?; + let path = self.resolve_path(&request.path)?; + self.ensure_read(&path, WorkdirSessionCapability::Read)?; + if !self.source.transports_delegation_context() { + request.path = path; + } self.source.list(request).await } async fn glob(&self, mut request: GlobRequest) -> Result { - request.path = self.resolve_path(&request.path)?; - self.ensure_read(&request.path, WorkdirSessionCapability::Glob)?; + let path = self.resolve_path(&request.path)?; + self.ensure_read(&path, WorkdirSessionCapability::Glob)?; + if !self.source.transports_delegation_context() { + request.path = path; + } self.source.glob(request).await } async fn grep(&self, mut request: GrepRequest) -> Result { - request.path = self.resolve_path(&request.path)?; - self.ensure_read(&request.path, WorkdirSessionCapability::Grep)?; + let path = self.resolve_path(&request.path)?; + self.ensure_read(&path, WorkdirSessionCapability::Grep)?; + if !self.source.transports_delegation_context() { + request.path = path; + } self.source.grep(request).await } @@ -531,6 +556,10 @@ impl WorkdirSession for ReadOnlyWorkdirSession { true } + fn transports_delegation_context(&self) -> bool { + self.inner.transports_delegation_context() + } + async fn delegate( &self, request: WorkdirDelegationRequest, @@ -798,6 +827,32 @@ mod tests { assert!(!root.path().join("secret/new").exists()); } + #[cfg(unix)] + #[tokio::test] + async fn write_delegation_rejects_symlink_target_before_lease() { + use std::os::unix::fs::symlink; + + let root = TempDir::new().unwrap(); + fs::create_dir_all(root.path().join("granted")).unwrap(); + fs::create_dir_all(root.path().join("secret")).unwrap(); + symlink("../secret", root.path().join("granted/outside")).unwrap(); + let parent = session(root.path()); + + assert!(matches!( + parent + .delegate(request( + "granted/outside", + WorkdirDelegationPermission::Write + )) + .await, + Err(WorkdirError::Denied(_)) + )); + parent + .write(write("secret/parent", "still-authoritative")) + .await + .unwrap(); + } + #[tokio::test] async fn write_lease_blocks_parent_region_until_release() { let root = TempDir::new().unwrap(); diff --git a/crates/workdir/src/http.rs b/crates/workdir/src/http.rs index df3f3327..2a80235d 100644 --- a/crates/workdir/src/http.rs +++ b/crates/workdir/src/http.rs @@ -328,6 +328,10 @@ mod client { self.capabilities } + fn transports_delegation_context(&self) -> bool { + true + } + async fn capture_delegation_source( &self, request: &crate::WorkdirDelegationRequest, diff --git a/crates/workdir/src/lib.rs b/crates/workdir/src/lib.rs index 590a8982..fdd05175 100644 --- a/crates/workdir/src/lib.rs +++ b/crates/workdir/src/lib.rs @@ -149,6 +149,12 @@ pub trait WorkdirSession: std::fmt::Debug + Send + Sync { false } + /// Whether this session transports the delegation chain to another + /// provider boundary that will apply logical cwd/path resolution there. + fn transports_delegation_context(&self) -> bool { + false + } + /// Capture a provider-specific source for a delegated child session. /// Remote providers use this boundary to pin attachment identity without /// exposing transport handles or host paths. diff --git a/crates/workdir/src/local.rs b/crates/workdir/src/local.rs index c644b1fd..51d3cc7a 100644 --- a/crates/workdir/src/local.rs +++ b/crates/workdir/src/local.rs @@ -388,6 +388,18 @@ impl WorkdirSession for LocalWorkdirSession { recursive: rule.recursive, }) .collect::>(); + for (logical, host) in request.rules.iter().zip(&host_rules) { + if logical.permission == WorkdirDelegationPermission::Write { + let resolved = Scope::resolved_target(host) + .map_err(|error| WorkdirError::Denied(error.to_string()))?; + if resolved != host.target { + return Err(WorkdirError::Denied(format!( + "write delegation target `{}` traverses a symlink", + logical.target + ))); + } + } + } let parent_scope = self.inner.scope.snapshot(); for rule in &host_rules { if !parent_scope @@ -405,10 +417,19 @@ impl WorkdirSession for LocalWorkdirSession { deny: Vec::new(), }) .map_err(|error| WorkdirError::Denied(error.to_string()))?; + let child_cwd = self.inner.root.join(request.cwd.as_str()); + if !child_scope.is_readable(&child_cwd) + || !std::fs::metadata(&child_cwd).is_ok_and(|metadata| metadata.is_dir()) + { + return Err(WorkdirError::Denied(format!( + "delegated cwd `{}` is not a readable Workdir directory", + request.cwd + ))); + } Ok(Arc::new(LocalWorkdirSession::materialized_bound( self.inner.workdir.clone(), self.inner.root.clone(), - self.inner.cwd.clone(), + self.inner.root.clone(), SharedScope::new(child_scope), self.inner.capabilities, ))) diff --git a/crates/worker-runtime/src/http_server.rs b/crates/worker-runtime/src/http_server.rs index 41e67859..d04e9d42 100644 --- a/crates/worker-runtime/src/http_server.rs +++ b/crates/worker-runtime/src/http_server.rs @@ -2232,6 +2232,8 @@ mod tests { { use std::os::unix::fs::symlink; std::fs::create_dir(temp.path().join("granted")).expect("granted directory"); + std::fs::write(temp.path().join("granted/visible"), "visible") + .expect("visible fixture"); std::fs::create_dir(temp.path().join("secret")).expect("secret directory"); std::fs::write(temp.path().join("secret/key"), "hidden").expect("secret fixture"); symlink("../secret/key", temp.path().join("granted/link")).expect("symlink fixture"); @@ -2287,6 +2289,36 @@ mod tests { #[cfg(unix)] { + let delegated_visible = WorkdirSessionOperationRequest { + delegations: vec![workdir::WorkdirDelegationRequest { + rules: vec![workdir::WorkdirDelegationRule { + target: WorkdirPath::new("granted").unwrap(), + permission: workdir::WorkdirDelegationPermission::Read, + recursive: true, + }], + cwd: WorkdirPath::new("granted").unwrap(), + }], + operation: WorkdirSessionOperation::Read(ReadRequest { + path: WorkdirPath::new("visible").unwrap(), + offset: 0, + limit: 20, + max_bytes: 1024, + }), + }; + let visible = run_workdir_session_operation( + State(state.clone()), + Path("session-1".to_string()), + Some(Extension(auth.clone())), + Ok(Json(delegated_visible)), + ) + .await + .expect("non-root delegated cwd should resolve once") + .0; + assert!(matches!( + visible, + WorkdirSessionOperationResult::Read(result) if result.bytes == b"visible" + )); + let delegated_read = WorkdirSessionOperationRequest { delegations: vec![workdir::WorkdirDelegationRequest { rules: vec![workdir::WorkdirDelegationRule { diff --git a/crates/worker/src/feature/builtin/manage_workdir.rs b/crates/worker/src/feature/builtin/manage_workdir.rs index c35365ff..6ddbc3d2 100644 --- a/crates/worker/src/feature/builtin/manage_workdir.rs +++ b/crates/worker/src/feature/builtin/manage_workdir.rs @@ -234,6 +234,10 @@ impl WorkdirSession for WorkspaceAttachedWorkdirSession { WorkdirSessionCapabilities::ALL } + fn transports_delegation_context(&self) -> bool { + true + } + async fn capture_delegation_source( &self, request: &workdir::WorkdirDelegationRequest, @@ -1155,7 +1159,7 @@ mod tests { assert_eq!(body["delegations"].as_array().unwrap().len(), 2); assert_eq!(body["delegations"][0]["rules"][0]["target"], ""); assert_eq!(body["delegations"][1]["rules"][0]["target"], "nested"); - assert_eq!(body["operation"]["request"]["path"], "nested/file"); + assert_eq!(body["operation"]["request"]["path"], "file"); } #[test]