From 8a3e06bc815ccad3b84c969defc2c918ab6fb591 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 14 Sep 2026 19:09:12 +0900 Subject: [PATCH] feat: add selective Workdir symlink policies --- crates/fs-operation/src/glob.rs | 12 +- crates/fs-operation/src/lib.rs | 15 +- crates/fs-operation/src/local.rs | 95 ++++-- crates/fs-operation/src/search.rs | 37 ++- crates/manifest/src/config.rs | 8 + crates/manifest/src/lib.rs | 2 +- crates/manifest/src/profile.rs | 2 + crates/manifest/src/scope.rs | 287 +++++++++++++++--- crates/memory/src/scope.rs | 1 + crates/protocol/src/lib.rs | 38 +++ crates/protocol/src/typescript.rs | 5 +- crates/session-store/src/worker_metadata.rs | 22 ++ crates/tools/src/bash.rs | 2 + crates/tools/tests/edge_cases.rs | 1 + crates/tools/tests/integration.rs | 1 + crates/workdir/src/http.rs | 17 +- crates/workdir/src/lib.rs | 23 +- crates/workdir/src/local.rs | 228 +++++++++++++- crates/workdir/src/scope.rs | 194 ++++++++++-- crates/worker-runtime/src/http_server.rs | 31 ++ crates/worker/src/controller.rs | 1 + crates/worker/src/discovery.rs | 3 + crates/worker/src/entrypoint.rs | 4 + .../src/feature/builtin/manage_workdir.rs | 30 +- crates/worker/src/fs_view.rs | 4 + crates/worker/src/runtime/dir.rs | 1 + .../runtime/worker_allocation/test_util.rs | 2 + crates/worker/src/spawn/registry.rs | 4 + crates/worker/src/spawn/tool.rs | 80 ++++- crates/worker/src/worker.rs | 3 + .../tests/internal_subworker_restore_test.rs | 2 + crates/worker/tests/worker_events_test.rs | 1 + crates/workspace-server/src/server.rs | 7 +- web/workspace/src/lib/generated/protocol.ts | 9 +- 34 files changed, 1017 insertions(+), 155 deletions(-) diff --git a/crates/fs-operation/src/glob.rs b/crates/fs-operation/src/glob.rs index a3d2ba4f..47af5862 100644 --- a/crates/fs-operation/src/glob.rs +++ b/crates/fs-operation/src/glob.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use globset::Glob; use ignore::WalkBuilder; -use crate::{FsAccessPolicy, FsError, FsPath, GlobRequest, GlobResult}; +use crate::{FsAccessPolicy, FsError, FsPath, GlobRequest, GlobResult, resolve_access_path}; /// Execute a bounded glob entirely inside the provider process. pub fn run_glob( @@ -15,7 +15,11 @@ pub fn run_glob( if !root.is_absolute() { return Err(FsError::RelativePath(root.to_path_buf())); } - if !access.is_readable(base) { + let base_resolved = resolve_access_path(base).map_err(|error| FsError::Io { + path: PathBuf::from(request.path.as_str()), + source: error, + })?; + if !access.is_readable_paths(base, &base_resolved) { return Err(FsError::OutOfScope(PathBuf::from(request.path.as_str()))); } let matcher = Glob::new(&request.pattern) @@ -26,7 +30,9 @@ pub fn run_glob( walker.hidden(false).follow_links(false); for entry in walker.build().flatten() { let path = entry.path(); - if !path.is_file() || !access.is_readable(path) { + let readable = resolve_access_path(path) + .is_ok_and(|resolved| access.is_readable_paths(path, &resolved)); + if !path.is_file() || !readable { continue; } let relative = path.strip_prefix(base).unwrap_or(path); diff --git a/crates/fs-operation/src/lib.rs b/crates/fs-operation/src/lib.rs index 19800ba7..5e58f5a2 100644 --- a/crates/fs-operation/src/lib.rs +++ b/crates/fs-operation/src/lib.rs @@ -14,7 +14,7 @@ use std::path::{Path, PathBuf}; use thiserror::Error; pub use glob::run_glob; -pub use local::{run_edit, run_list, run_read, run_stat, run_write}; +pub use local::{resolve_access_path, run_edit, run_list, run_read, run_stat, run_write}; pub use operation::*; pub use search::run_grep; @@ -22,6 +22,19 @@ pub use search::run_grep; pub trait FsAccessPolicy: Send + Sync { fn is_readable(&self, path: &Path) -> bool; fn is_writable(&self, path: &Path) -> bool; + + /// Authorize both the Workdir-visible path and its provider-resolved + /// target. Implementations that do not distinguish symbolic-link identity + /// retain resolved-target semantics through the defaults. + fn is_readable_paths(&self, logical: &Path, resolved: &Path) -> bool { + let _ = logical; + self.is_readable(resolved) + } + + fn is_writable_paths(&self, logical: &Path, resolved: &Path) -> bool { + let _ = logical; + self.is_writable(resolved) + } } /// First symlink encountered while resolving a provider path. diff --git a/crates/fs-operation/src/local.rs b/crates/fs-operation/src/local.rs index 4f91c9f3..e22b9823 100644 --- a/crates/fs-operation/src/local.rs +++ b/crates/fs-operation/src/local.rs @@ -1,3 +1,4 @@ +use std::ffi::OsString; use std::fs; use std::io::Write; use std::path::{Path, PathBuf}; @@ -18,7 +19,8 @@ pub fn run_stat( ) -> Result { let logical = request.path; let path = resolve(root, &logical)?; - if !access.is_readable(&path) { + let resolved = resolve_access_path(&path).map_err(|error| map_io(&logical, error))?; + if !access.is_readable_paths(&path, &resolved) { return Err(FsError::OutOfScope(PathBuf::from(logical.as_str()))); } let metadata = fs::symlink_metadata(&path).map_err(|error| map_io(&logical, error))?; @@ -113,12 +115,8 @@ pub fn run_write( if request.expected_hash.is_some() { return Err(FsError::Conflict(logical.as_str().to_string())); } - let parent = path.parent().ok_or_else(|| { - FsError::InvalidArgument(format!("{} has no parent", logical.as_str())) - })?; - let parent_logical = logical_parent(&logical); - require_access(parent, &parent_logical, access, true, true)?; - atomic_write(&path, &request.content, &logical)?; + let target = require_access(&path, &logical, access, true, true)?; + atomic_write(&target, &request.content, &logical)?; } Ok(WriteResult { bytes_written: request.content.len(), @@ -173,6 +171,7 @@ pub fn run_list( ) -> Result { let logical = request.path; let path = resolve(root, &logical)?; + let logical_base = path.clone(); let path = require_access(&path, &logical, access, false, true)?; let metadata = fs::metadata(&path).map_err(|error| map_io(&logical, error))?; if !metadata.is_dir() { @@ -183,7 +182,15 @@ pub fn run_list( for entry in read_dir { let entry = entry.map_err(|error| map_io(&logical, error))?; let absolute = entry.path(); - if !access.is_readable(&absolute) { + let relative_to_base = absolute.strip_prefix(&path).map_err(|_| { + FsError::InvalidArgument("provider returned a path outside its list base".to_string()) + })?; + let logical_absolute = logical_base.join(relative_to_base); + let resolved = match resolve_access_path(&absolute) { + Ok(resolved) => resolved, + Err(_) => continue, + }; + if !access.is_readable_paths(&logical_absolute, &resolved) { continue; } let link_metadata = @@ -203,7 +210,7 @@ pub fn run_list( } else { EntryKind::Other }; - let relative = absolute.strip_prefix(root).map_err(|_| { + let relative = logical_absolute.strip_prefix(root).map_err(|_| { FsError::InvalidArgument("provider returned a path outside its root".to_string()) })?; entries.push(ListEntry { @@ -249,18 +256,22 @@ fn require_access( write: bool, allow_symlink_directory: bool, ) -> Result { - if let Some(info) = direct_symlink(path) { - if !info.target_exists { - return Err(FsError::BrokenSymlink { - path: PathBuf::from(logical.as_str()), - link: PathBuf::from(logical.as_str()), - target: PathBuf::from(""), - }); - } + let symlink = direct_symlink(path); + if let Some(info) = symlink.as_ref() + && !info.target_exists + { + return Err(FsError::BrokenSymlink { + path: PathBuf::from(logical.as_str()), + link: PathBuf::from(logical.as_str()), + target: PathBuf::from(""), + }); + } + let resolved = resolve_access_path(path).map_err(|error| map_io(logical, error))?; + if let Some(info) = symlink { let allowed = if write { - access.is_writable(path) + access.is_writable_paths(path, &resolved) } else { - access.is_readable(path) + access.is_readable_paths(path, &resolved) }; if !allowed { return Err(FsError::SymlinkOutOfScope { @@ -275,15 +286,15 @@ fn require_access( target: PathBuf::from(""), }); } - return Ok(info.resolved_path); + return Ok(resolved); } let allowed = if write { - access.is_writable(path) + access.is_writable_paths(path, &resolved) } else { - access.is_readable(path) + access.is_readable_paths(path, &resolved) }; if allowed { - Ok(path.to_path_buf()) + Ok(resolved) } else if write { Err(FsError::ReadOnly(PathBuf::from(logical.as_str()))) } else { @@ -291,12 +302,38 @@ fn require_access( } } -fn logical_parent(path: &FsPath) -> FsPath { - let parent = Path::new(path.as_str()) - .parent() - .unwrap_or_else(|| Path::new("")) - .to_string_lossy(); - FsPath::new(parent).unwrap_or_else(|_| FsPath::root()) +/// Resolve every existing component of an absolute provider path while +/// retaining a missing final tail for create operations. Dangling symlinks are +/// rejected because no resolved authority identity can be established. +pub fn resolve_access_path(path: &Path) -> std::io::Result { + let mut cursor = path; + let mut missing = Vec::::new(); + loop { + match fs::canonicalize(cursor) { + Ok(mut resolved) => { + for component in missing.iter().rev() { + resolved.push(component); + } + return Ok(resolved); + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + if fs::symlink_metadata(cursor) + .is_ok_and(|metadata| metadata.file_type().is_symlink()) + { + return Err(error); + } + let name = cursor.file_name().ok_or(error)?; + missing.push(name.to_os_string()); + cursor = cursor.parent().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::NotFound, + "path has no existing ancestor", + ) + })?; + } + Err(error) => return Err(error), + } + } } fn atomic_write(path: &Path, content: &[u8], logical: &FsPath) -> Result<(), FsError> { diff --git a/crates/fs-operation/src/search.rs b/crates/fs-operation/src/search.rs index 2f219721..57f107f4 100644 --- a/crates/fs-operation/src/search.rs +++ b/crates/fs-operation/src/search.rs @@ -10,7 +10,9 @@ use ignore::WalkBuilder; use ignore::overrides::{Override, OverrideBuilder}; use ignore::types::{Types, TypesBuilder}; -use crate::{FsError, GrepOutputMode, GrepRequest, GrepResult, direct_symlink}; +use crate::{ + FsError, GrepOutputMode, GrepRequest, GrepResult, direct_symlink, resolve_access_path, +}; struct ContentLine { path: PathBuf, @@ -220,14 +222,28 @@ pub fn run_grep( return Err(FsError::RelativePath(base)); } let symlink = direct_symlink(&base); - if !access.is_readable(&base) { + if let Some(info) = symlink.as_ref() + && !info.target_exists + { + return Err(FsError::BrokenSymlink { + path: base.clone(), + link: info.link_path.clone(), + target: info.resolved_path.clone(), + }); + } + let resolved_base = resolve_access_path(&base).map_err(|error| FsError::io(&base, error))?; + if !access.is_readable_paths(&base, &resolved_base) { return Err(if let Some(info) = symlink.as_ref() { let link_parent_readable = info .link_path .parent() - .map(|parent| access.is_readable(parent)) + .and_then(|parent| { + resolve_access_path(parent) + .ok() + .map(|resolved| access.is_readable_paths(parent, &resolved)) + }) .unwrap_or(false); - if info.target_exists && link_parent_readable { + if link_parent_readable { FsError::SymlinkOutOfScope { path: base.clone(), target: info.resolved_path.clone(), @@ -240,15 +256,6 @@ pub fn run_grep( FsError::OutOfScope(base.clone()) }); } - if let Some(info) = symlink.as_ref() { - if !info.target_exists { - return Err(FsError::BrokenSymlink { - path: base.clone(), - link: info.link_path.clone(), - target: info.target_path.clone(), - }); - } - } let base_meta = std::fs::metadata(&base).map_err(|e| match e.kind() { std::io::ErrorKind::NotFound => FsError::NotFound(base.clone()), _ => FsError::io(&base, e), @@ -321,7 +328,9 @@ pub fn run_grep( continue; } let path = entry.path(); - if !access.is_readable(path) { + let readable = resolve_access_path(path) + .is_ok_and(|resolved| access.is_readable_paths(path, &resolved)); + if !readable { continue; } if scan_path( diff --git a/crates/manifest/src/config.rs b/crates/manifest/src/config.rs index d5ae7049..be41aa3e 100644 --- a/crates/manifest/src/config.rs +++ b/crates/manifest/src/config.rs @@ -1329,6 +1329,7 @@ mod tests { target: abs("/worker"), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }], deny: Vec::new(), }, @@ -1575,6 +1576,7 @@ mod tests { target: PathBuf::from("secrets"), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }); let resolved = cfg.resolve_paths(Path::new("/workspace/proj")); assert_eq!(resolved.scope.allow[0].target, Path::new("/workspace/proj")); @@ -1712,6 +1714,7 @@ mod tests { target: abs("/a"), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }], deny: Vec::new(), }, @@ -1723,11 +1726,13 @@ mod tests { target: abs("/b"), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }], deny: vec![ScopeRule { target: abs("/a/secret"), permission: Permission::Read, recursive: false, + symlink_policy: Default::default(), }], }, ..Default::default() @@ -2091,6 +2096,7 @@ enabled = false target: abs("/worker"), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }], deny: Vec::new(), }, @@ -2193,6 +2199,7 @@ enabled = true target: abs("/worker"), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }], deny: Vec::new(), }, @@ -2269,6 +2276,7 @@ permission = "write" target: abs("/worker"), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }], deny: Vec::new(), }, diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index a7908702..91d61091 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -29,7 +29,7 @@ pub use profile::{ WorkspaceAuthorityRequirement, resolve_profile_artifact, resolve_profile_artifact_value, validate_profile_execution_target, }; -pub use protocol::{Permission, ScopeRule}; +pub use protocol::{Permission, ScopeRule, SymlinkPolicy}; pub use scope::{DelegationScope, Scope, ScopeError, SharedScope}; use std::collections::{BTreeMap, HashMap}; diff --git a/crates/manifest/src/profile.rs b/crates/manifest/src/profile.rs index 40bc0e1a..6a0222fe 100644 --- a/crates/manifest/src/profile.rs +++ b/crates/manifest/src/profile.rs @@ -970,6 +970,7 @@ fn profile_scope_intent_to_config( target: workspace_base.join(path), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }); } Ok(ScopeConfig { @@ -977,6 +978,7 @@ fn profile_scope_intent_to_config( target: workspace_base.to_path_buf(), permission, recursive: true, + symlink_policy: Default::default(), }], deny, }) diff --git a/crates/manifest/src/scope.rs b/crates/manifest/src/scope.rs index ad6e5def..270cf61a 100644 --- a/crates/manifest/src/scope.rs +++ b/crates/manifest/src/scope.rs @@ -3,16 +3,17 @@ //! Built from [`crate::ScopeConfig`] via [`Scope::from_config`]. Every //! rule `target` must already be an absolute path — per-layer path //! resolution runs earlier, inside [`crate::WorkerManifestConfig::resolve_paths`]. -//! All rule `target` paths inside the [`Scope`] are normalized lexically so -//! access authority follows the path presented through the Workdir, not a -//! symbolic-link target outside that logical tree. +//! All rule targets retain both their lexically normalized logical identity and +//! their provider-resolved identity. Allow rules select one identity explicitly; +//! deny rules always inspect both so aliases cannot bypass a restriction. +use std::ffi::OsString; use std::path::{Component, Path, PathBuf}; use std::sync::{Arc, Mutex}; use arc_swap::{ArcSwap, Guard}; -use crate::{Permission, ScopeConfig, ScopeRule}; +use crate::{Permission, ScopeConfig, ScopeRule, SymlinkPolicy}; /// Parsed, pwd-resolved set of allow/deny rules for a Worker. /// @@ -26,10 +27,13 @@ pub struct Scope { #[derive(Debug, Clone, PartialEq, Eq)] struct ResolvedRule { - /// Absolute, lexically normalized target directory/file. - target: PathBuf, + /// Absolute, lexically normalized target as presented through the Workdir. + logical_target: PathBuf, + /// Absolute target after provider-side symbolic-link resolution. + resolved_target: PathBuf, permission: Permission, recursive: bool, + symlink_policy: SymlinkPolicy, } /// Parsed filesystem authority this Worker may pass to spawned children. @@ -98,18 +102,46 @@ fn permission_denies_requested(denied: Permission, requested: Permission) -> boo fn rule_covers(available: &ResolvedRule, requested: &ResolvedRule) -> bool { permission_covers(available.permission, requested.permission) - && rule_path_set_contains(available, requested) + && available.symlink_policy >= requested.symlink_policy + && rule_path_set_contains( + available, + requested, + match available.symlink_policy { + SymlinkPolicy::Resolved => RuleIdentity::Resolved, + SymlinkPolicy::Logical => RuleIdentity::Logical, + }, + ) } fn denial_overlaps_requested(deny: &ResolvedRule, requested: &ResolvedRule) -> bool { permission_denies_requested(deny.permission, requested.permission) - && rule_path_sets_overlap(deny, requested) + && (rule_path_sets_overlap(deny, requested, RuleIdentity::Logical) + || rule_path_sets_overlap(deny, requested, RuleIdentity::Resolved)) } -fn rule_path_set_contains(available: &ResolvedRule, requested: &ResolvedRule) -> bool { +#[derive(Clone, Copy)] +enum RuleIdentity { + Logical, + Resolved, +} + +fn rule_target(rule: &ResolvedRule, identity: RuleIdentity) -> &Path { + match identity { + RuleIdentity::Logical => &rule.logical_target, + RuleIdentity::Resolved => &rule.resolved_target, + } +} + +fn rule_path_set_contains( + available: &ResolvedRule, + requested: &ResolvedRule, + identity: RuleIdentity, +) -> bool { + let available_target = rule_target(available, identity); + let requested_target = rule_target(requested, identity); match (available.recursive, requested.recursive) { // A recursive grant contains every possible requested path below its target. - (true, _) => requested.target.starts_with(&available.target), + (true, _) => requested_target.starts_with(available_target), // A non-recursive grant contains only the target and its direct children; // a recursive request always includes descendants beyond that finite-depth // set. @@ -117,36 +149,42 @@ fn rule_path_set_contains(available: &ResolvedRule, requested: &ResolvedRule) -> // Two non-recursive rules have the same finite-depth set only when their // target is identical. A request rooted at a direct child would also grant // that child's children, which are grandchildren of `available.target`. - (false, false) => requested.target == available.target, + (false, false) => requested_target == available_target, } } -fn rule_path_sets_overlap(left: &ResolvedRule, right: &ResolvedRule) -> bool { +fn rule_path_sets_overlap( + left: &ResolvedRule, + right: &ResolvedRule, + identity: RuleIdentity, +) -> bool { + let left_target = rule_target(left, identity); + let right_target = rule_target(right, identity); match (left.recursive, right.recursive) { (true, true) => { - left.target.starts_with(&right.target) || right.target.starts_with(&left.target) + left_target.starts_with(right_target) || right_target.starts_with(left_target) } - (true, false) => recursive_and_non_recursive_sets_overlap(left, right), - (false, true) => recursive_and_non_recursive_sets_overlap(right, left), + (true, false) => recursive_and_non_recursive_sets_overlap(left_target, right_target), + (false, true) => recursive_and_non_recursive_sets_overlap(right_target, left_target), (false, false) => { - left.target == right.target - || direct_child(&left.target, &right.target) - || direct_child(&right.target, &left.target) + left_target == right_target + || direct_child(left_target, right_target) + || direct_child(right_target, left_target) } } } fn recursive_and_non_recursive_sets_overlap( - recursive: &ResolvedRule, - non_recursive: &ResolvedRule, + recursive_target: &Path, + non_recursive_target: &Path, ) -> bool { // The non-recursive set is `{target} + direct children`. It overlaps a // recursive subtree when either the non-recursive target is inside that // subtree, or the recursive subtree begins at the non-recursive target or // one of its direct children. - non_recursive.target.starts_with(&recursive.target) - || recursive.target == non_recursive.target - || direct_child(&recursive.target, &non_recursive.target) + non_recursive_target.starts_with(recursive_target) + || recursive_target == non_recursive_target + || direct_child(recursive_target, non_recursive_target) } fn direct_child(child: &Path, parent: &Path) -> bool { @@ -201,7 +239,8 @@ impl Scope { } /// Convenience constructor for tests and simple setups: a single - /// recursive `allow(Write)` rule rooted at the lexical path `root`. + /// recursive `allow(Write)` rule rooted at `root` with the default + /// resolved-target symlink policy. pub fn writable(root: impl AsRef) -> std::io::Result { let root = normalize_path(root.as_ref()).ok_or_else(|| { std::io::Error::new( @@ -209,19 +248,26 @@ impl Scope { "scope root must be an absolute path without root traversal", ) })?; + let resolved_root = resolve_path(&root)?; Ok(Self { allow: vec![ResolvedRule { - target: root, + logical_target: root, + resolved_target: resolved_root, permission: Permission::Write, recursive: true, + symlink_policy: SymlinkPolicy::Resolved, }], deny: Vec::new(), }) } - /// Return one rule's lexically normalized target without resolving symlinks. + /// Return one rule target in the identity selected by its symlink policy. pub fn resolved_target(rule: &ScopeRule) -> Result { - Ok(resolve_rule(rule)?.target) + let rule = resolve_rule(rule)?; + Ok(match rule.symlink_policy { + SymlinkPolicy::Resolved => rule.resolved_target, + SymlinkPolicy::Logical => rule.logical_target, + }) } /// Return whether this effective scope fully contains a requested rule. @@ -248,10 +294,23 @@ impl Scope { /// Returns `None` when `path` is outside every allow rule, or when /// deny rules have knocked it below `Read`. pub fn permission_at(&self, path: &Path) -> Option { - let resolved = normalize_path(path)?; + let logical = normalize_path(path)?; + let resolved = resolve_path(&logical).ok()?; + self.permission_at_paths(&logical, &resolved) + } + + /// Effective permission for a path whose logical and provider-resolved + /// identities were obtained inside the filesystem provider boundary. + pub fn permission_at_paths(&self, logical: &Path, resolved: &Path) -> Option { + let logical = normalize_path(logical)?; + let resolved = normalize_path(resolved)?; let mut effective: Option = None; for rule in &self.allow { - if rule.matches(&resolved) { + let candidate = match rule.symlink_policy { + SymlinkPolicy::Resolved => &resolved, + SymlinkPolicy::Logical => &logical, + }; + if rule.matches(candidate, rule.symlink_policy) { effective = match effective { None => Some(rule.permission), Some(cur) => Some(cur.max(rule.permission)), @@ -260,11 +319,13 @@ impl Scope { } let mut effective = effective?; - // Deny: min(min_deny) dictates the cap. Effective level is capped - // strictly below that value, so deny(read) wipes access entirely. + // Deny rules always inspect both identities. This prevents a logical + // alias or a second symlink to the same target from bypassing a deny. let mut min_deny: Option = None; for rule in &self.deny { - if rule.matches(&resolved) { + if rule.matches(&logical, SymlinkPolicy::Logical) + || rule.matches(&resolved, SymlinkPolicy::Resolved) + { min_deny = match min_deny { None => Some(rule.permission), Some(cur) => Some(cur.min(rule.permission)), @@ -297,7 +358,7 @@ impl Scope { /// rule, preserving declaration order. Does not account for deny /// rules, which only cap effective permission at query time. pub fn readable_paths(&self) -> impl Iterator { - self.allow.iter().map(|r| r.target.as_path()) + self.allow.iter().map(|r| r.logical_target.as_path()) } /// Allow rules with their targets resolved to absolute paths. @@ -309,9 +370,10 @@ impl Scope { self.allow .iter() .map(|r| ScopeRule { - target: r.target.clone(), + target: r.logical_target.clone(), permission: r.permission, recursive: r.recursive, + symlink_policy: r.symlink_policy, }) .collect() } @@ -326,9 +388,10 @@ impl Scope { self.deny .iter() .map(|r| ScopeRule { - target: r.target.clone(), + target: r.logical_target.clone(), permission: r.permission, recursive: r.recursive, + symlink_policy: r.symlink_policy, }) .collect() } @@ -339,7 +402,7 @@ impl Scope { self.allow .iter() .filter(|r| r.permission == Permission::Write) - .map(|r| r.target.as_path()) + .map(|r| r.logical_target.as_path()) } /// Build a new [`Scope`] equal to `self` with `extra_allow` appended @@ -416,7 +479,10 @@ impl Scope { pub fn summary(&self) -> String { fn push_rule(out: &mut String, rule: &ResolvedRule) { out.push_str(" - "); - out.push_str(&rule.target.display().to_string()); + out.push_str(&rule.logical_target.display().to_string()); + if rule.symlink_policy == SymlinkPolicy::Logical { + out.push_str(" [logical-symlinks]"); + } if !rule.recursive { out.push_str(" [non-recursive]"); } @@ -514,11 +580,15 @@ impl SharedScope { } impl ResolvedRule { - fn matches(&self, path: &Path) -> bool { + fn matches(&self, path: &Path, identity: SymlinkPolicy) -> bool { + let target = match identity { + SymlinkPolicy::Resolved => &self.resolved_target, + SymlinkPolicy::Logical => &self.logical_target, + }; if self.recursive { - path.starts_with(&self.target) + path.starts_with(target) } else { - path == self.target || path.parent() == Some(self.target.as_path()) + path == target || path.parent() == Some(target.as_path()) } } } @@ -527,17 +597,63 @@ fn resolve_rule(rule: &ScopeRule) -> Result { if !rule.target.is_absolute() { return Err(ScopeError::RelativeTarget(rule.target.clone())); } - let target = normalize_path(&rule.target).ok_or_else(|| ScopeError::ResolveTarget { + let logical_target = normalize_path(&rule.target).ok_or_else(|| ScopeError::ResolveTarget { path: rule.target.clone(), source: std::io::Error::new(std::io::ErrorKind::Other, "could not absolutize target"), })?; + let resolved_target = + resolve_path(&logical_target).map_err(|source| ScopeError::ResolveTarget { + path: rule.target.clone(), + source, + })?; Ok(ResolvedRule { - target, + logical_target, + resolved_target, permission: rule.permission, recursive: rule.recursive, + symlink_policy: rule.symlink_policy, }) } +/// Resolve every existing path component while retaining a missing final tail. +/// A dangling symlink is rejected rather than treated as an ordinary missing +/// component because its resolved authority cannot be established. +fn resolve_path(path: &Path) -> std::io::Result { + let mut cursor = path; + let mut missing = Vec::::new(); + loop { + match std::fs::canonicalize(cursor) { + Ok(mut resolved) => { + for component in missing.iter().rev() { + resolved.push(component); + } + return normalize_path(&resolved).ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "resolved target is not an absolute normalized path", + ) + }); + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + if std::fs::symlink_metadata(cursor) + .is_ok_and(|metadata| metadata.file_type().is_symlink()) + { + return Err(error); + } + let name = cursor.file_name().ok_or(error)?; + missing.push(name.to_os_string()); + cursor = cursor.parent().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::NotFound, + "scope target has no existing ancestor", + ) + })?; + } + Err(error) => return Err(error), + } + } +} + /// Normalize an absolute path for lexical scope comparison without consulting /// filesystem metadata or resolving symbolic links. fn normalize_path(path: &Path) -> Option { @@ -571,6 +687,7 @@ mod tests { target: target.to_path_buf(), permission, recursive, + symlink_policy: Default::default(), } } @@ -685,6 +802,7 @@ mod tests { target: dir.path().to_path_buf(), permission: Permission::Write, recursive: false, + symlink_policy: Default::default(), }], deny: Vec::new(), }; @@ -784,6 +902,7 @@ mod tests { target: PathBuf::from("relative/path"), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }], deny: Vec::new(), }; @@ -801,19 +920,84 @@ mod tests { #[cfg(unix)] #[test] - fn scope_authorizes_symlink_paths_lexically_without_authorizing_targets() { + fn scope_defaults_to_resolved_symlink_authority_and_logical_is_explicit() { use std::os::unix::fs::symlink; let dir = TempDir::new().unwrap(); let outside = TempDir::new().unwrap(); std::fs::write(outside.path().join("outside.txt"), "visible through link").unwrap(); symlink(outside.path(), dir.path().join("external")).unwrap(); - let scope = Scope::writable(dir.path()).unwrap(); - assert!(scope.is_readable(&dir.path().join("external/outside.txt"))); - assert!(scope.is_writable(&dir.path().join("external/new.txt"))); - assert!(!scope.is_readable(&outside.path().join("outside.txt"))); - assert!(!scope.is_writable(&outside.path().join("new.txt"))); + let resolved = Scope::writable(dir.path()).unwrap(); + assert!(!resolved.is_readable(&dir.path().join("external/outside.txt"))); + assert!(!resolved.is_writable(&dir.path().join("external/new.txt"))); + + let logical = Scope::from_config(&ScopeConfig { + allow: vec![ScopeRule { + target: dir.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + symlink_policy: SymlinkPolicy::Logical, + }], + deny: Vec::new(), + }) + .unwrap(); + assert!(logical.is_readable(&dir.path().join("external/outside.txt"))); + assert!(logical.is_writable(&dir.path().join("external/new.txt"))); + assert!(!logical.is_readable(&outside.path().join("outside.txt"))); + assert!(!logical.is_writable(&outside.path().join("new.txt"))); + } + + #[cfg(unix)] + #[test] + fn deny_rules_match_both_logical_alias_and_resolved_target() { + use std::os::unix::fs::symlink; + + let root = TempDir::new().unwrap(); + let secret = root.path().join("secret"); + std::fs::create_dir(&secret).unwrap(); + std::fs::write(secret.join("key"), "hidden").unwrap(); + symlink(&secret, root.path().join("alias")).unwrap(); + let scope = Scope::from_config(&ScopeConfig { + allow: vec![ScopeRule { + target: root.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + symlink_policy: SymlinkPolicy::Logical, + }], + deny: vec![ScopeRule { + target: secret, + permission: Permission::Read, + recursive: true, + symlink_policy: SymlinkPolicy::Logical, + }], + }) + .unwrap(); + + assert!(!scope.is_readable(&root.path().join("alias/key"))); + } + + #[test] + fn delegation_symlink_policy_is_monotonically_attenuated() { + let root = TempDir::new().unwrap(); + let mut parent_rule = allow_rule(root.path(), Permission::Write); + parent_rule.symlink_policy = SymlinkPolicy::Logical; + let logical_parent = DelegationScope::from_config(&ScopeConfig { + allow: vec![parent_rule], + deny: Vec::new(), + }) + .unwrap(); + let resolved_child = allow_rule(&root.path().join("child"), Permission::Read); + assert!(logical_parent.allows_rule(&resolved_child).unwrap()); + + let resolved_parent = DelegationScope::from_config(&ScopeConfig { + allow: vec![allow_rule(root.path(), Permission::Write)], + deny: Vec::new(), + }) + .unwrap(); + let mut logical_child = resolved_child; + logical_child.symlink_policy = SymlinkPolicy::Logical; + assert!(!resolved_parent.allows_rule(&logical_child).unwrap()); } #[test] @@ -862,11 +1046,13 @@ mod tests { target: docs.clone(), permission: Permission::Read, recursive: false, + symlink_policy: Default::default(), }, ScopeRule { target: dir.path().to_path_buf(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }, ], deny: Vec::new(), @@ -925,6 +1111,7 @@ mod tests { target: extra.path().to_path_buf(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }]) .unwrap(); assert!(extended.is_readable(&extra.path().join("x"))); @@ -942,6 +1129,7 @@ mod tests { target: sub.clone(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }]) .unwrap(); let f = sub.join("a.txt"); @@ -961,6 +1149,7 @@ mod tests { target: sub.clone(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }; let base = Scope::writable(dir.path()) .unwrap() @@ -1014,6 +1203,7 @@ mod tests { target: sub.clone(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }]) }) .unwrap(); @@ -1032,6 +1222,7 @@ mod tests { target: extra.path().to_path_buf(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }]) }) .unwrap(); diff --git a/crates/memory/src/scope.rs b/crates/memory/src/scope.rs index 4b5326ec..cec6dd45 100644 --- a/crates/memory/src/scope.rs +++ b/crates/memory/src/scope.rs @@ -23,6 +23,7 @@ fn deny_write(target: &Path) -> ScopeRule { target: target.to_path_buf(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), } } diff --git a/crates/protocol/src/lib.rs b/crates/protocol/src/lib.rs index 9cbd089d..d0d6e5a4 100644 --- a/crates/protocol/src/lib.rs +++ b/crates/protocol/src/lib.rs @@ -1599,12 +1599,30 @@ pub struct ScopeRule { /// direct children. Defaults to `true`. #[serde(default = "default_recursive")] pub recursive: bool, + /// Which path identity an allow rule uses when symbolic links are + /// encountered. Deny rules always inspect both identities. + #[serde(default)] + pub symlink_policy: SymlinkPolicy, } fn default_recursive() -> bool { true } +/// Symbolic-link identity used by one filesystem allow rule. +/// +/// `Resolved` is the least authority and the default: access is matched +/// against the provider-resolved target. `Logical` intentionally grants the +/// path as presented through the Workdir, even when it aliases another target. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[cfg_attr(feature = "typescript", derive(ts_rs::TS))] +#[serde(rename_all = "lowercase")] +pub enum SymlinkPolicy { + #[default] + Resolved, + Logical, +} + /// Permission lattice used by [`ScopeRule`]. /// /// The derived `Ord` instance follows declaration order, so @@ -1623,6 +1641,25 @@ pub enum Permission { mod tests { use super::*; + #[test] + fn scope_rule_defaults_to_resolved_symlink_policy() { + let rule: ScopeRule = serde_json::from_value(serde_json::json!({ + "target": "/workspace", + "permission": "read" + })) + .unwrap(); + assert!(rule.recursive); + assert_eq!(rule.symlink_policy, SymlinkPolicy::Resolved); + + let logical: ScopeRule = serde_json::from_value(serde_json::json!({ + "target": "/workspace", + "permission": "read", + "symlink_policy": "logical" + })) + .unwrap(); + assert_eq!(logical.symlink_policy, SymlinkPolicy::Logical); + } + #[test] fn worker_state_snapshot_apply_is_monotonic_and_detects_conflicts() { let mut current = WorkerStateSnapshot::initial(4); @@ -2462,6 +2499,7 @@ mod tests { target: "/tmp/work".into(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }], }); let json = serde_json::to_string(&method).unwrap(); diff --git a/crates/protocol/src/typescript.rs b/crates/protocol/src/typescript.rs index ec5588f9..f3f147a4 100644 --- a/crates/protocol/src/typescript.rs +++ b/crates/protocol/src/typescript.rs @@ -11,8 +11,8 @@ use crate::{ PasteArtifactRef, PendingSubmissionSummary, PendingSubmissionsSnapshot, Permission, RewindSummary, RewindTarget, RewindTargetId, RunResult, ScopeRule, Segment, SessionContentPart, SessionEntryProvenance, SessionMessageRole, SessionSnapshot, SessionSnapshotEntry, - SessionSnapshotEntryData, SessionToolAttachment, SubmissionDisposition, ToolResultDisposition, - TurnResult, UploadedFileAvailability, UploadedFileRef, WorkerBusyState, + SessionSnapshotEntryData, SessionToolAttachment, SubmissionDisposition, SymlinkPolicy, + ToolResultDisposition, TurnResult, UploadedFileAvailability, UploadedFileRef, WorkerBusyState, WorkerCommandAcknowledgement, WorkerCommandDisposition, WorkerCommandEnvelope, WorkerCommandKind, WorkerEvent, WorkerMaintenanceState, WorkerRunState, WorkerState, WorkerStateSnapshot, WorkerStatus, @@ -64,6 +64,7 @@ pub fn generated_protocol_types() -> String { push_decl::(&cfg, &mut output); push_decl::(&cfg, &mut output); push_decl::(&cfg, &mut output); + push_decl::(&cfg, &mut output); push_decl::(&cfg, &mut output); push_decl::(&cfg, &mut output); push_decl::(&cfg, &mut output); diff --git a/crates/session-store/src/worker_metadata.rs b/crates/session-store/src/worker_metadata.rs index ec63f563..387e5f98 100644 --- a/crates/session-store/src/worker_metadata.rs +++ b/crates/session-store/src/worker_metadata.rs @@ -63,6 +63,8 @@ pub struct WorkerSpawnedScopeRule { pub target: PathBuf, pub permission: String, pub recursive: bool, + #[serde(default)] + pub symlink_policy: protocol::SymlinkPolicy, } /// One child Worker spawned by this Worker and persisted with the spawner's @@ -682,6 +684,25 @@ mod tests { assert_eq!(restored, metadata); } + #[test] + fn spawned_scope_rule_defaults_resolved_and_roundtrips_logical_policy() { + let legacy: WorkerSpawnedScopeRule = serde_json::from_value(serde_json::json!({ + "target": "/workspace/src", + "permission": "read", + "recursive": true + })) + .unwrap(); + assert_eq!(legacy.symlink_policy, protocol::SymlinkPolicy::Resolved); + + let logical = WorkerSpawnedScopeRule { + symlink_policy: protocol::SymlinkPolicy::Logical, + ..legacy + }; + let restored: WorkerSpawnedScopeRule = + serde_json::from_value(serde_json::to_value(&logical).unwrap()).unwrap(); + assert_eq!(restored, logical); + } + #[test] fn worker_aggregate_store_writes_one_fixed_metadata_identity() { let tmp = tempfile::tempdir().unwrap(); @@ -835,6 +856,7 @@ mod tests { target: std::path::Path::new("/tmp/delegated").into(), permission: "write".into(), recursive: true, + symlink_policy: Default::default(), }; store .set_spawned_children( diff --git a/crates/tools/src/bash.rs b/crates/tools/src/bash.rs index 433dec0c..34e979b5 100644 --- a/crates/tools/src/bash.rs +++ b/crates/tools/src/bash.rs @@ -300,11 +300,13 @@ mod tests { target: root.path().to_path_buf(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }, ScopeRule { target: output.path().to_path_buf(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }, ], deny: Vec::new(), diff --git a/crates/tools/tests/edge_cases.rs b/crates/tools/tests/edge_cases.rs index e00549aa..252f6001 100644 --- a/crates/tools/tests/edge_cases.rs +++ b/crates/tools/tests/edge_cases.rs @@ -40,6 +40,7 @@ fn setup() -> (TempDir, TempDir, Registry) { target: spill.path().to_path_buf(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }); let scope = Scope::from_config(&config).unwrap(); let fs: WorkdirSessionHandle = diff --git a/crates/tools/tests/integration.rs b/crates/tools/tests/integration.rs index 3888c659..02b8c986 100644 --- a/crates/tools/tests/integration.rs +++ b/crates/tools/tests/integration.rs @@ -27,6 +27,7 @@ fn scope_with_spill(workspace: &Path, spill: &Path) -> Scope { target: spill.to_path_buf(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }); Scope::from_config(&config).unwrap() } diff --git a/crates/workdir/src/http.rs b/crates/workdir/src/http.rs index cd02e19b..a12cc250 100644 --- a/crates/workdir/src/http.rs +++ b/crates/workdir/src/http.rs @@ -11,7 +11,7 @@ use crate::{ CommandHandle, CommandOutput, CommandOutputRequest, CommandRequest, CommandStatus, EditRequest, EditResult, GlobRequest, GlobResult, GrepRequest, GrepResult, ListRequest, ListResult, ReadRequest, ReadResult, StatRequest, StatResult, WorkdirError, WorkdirId, - WorkdirSessionCapabilities, WriteRequest, WriteResult, + WorkdirScopeAuthorizationRequest, WorkdirSessionCapabilities, WriteRequest, WriteResult, }; /// Opaque Runtime-owned identifier for one ephemeral Workdir session. @@ -55,6 +55,7 @@ pub struct OpenWorkdirSessionResponse { #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "operation", content = "request", rename_all = "snake_case")] pub enum WorkdirSessionOperation { + AuthorizeScope(WorkdirScopeAuthorizationRequest), Stat(StatRequest), Read(ReadRequest), Write(WriteRequest), @@ -79,6 +80,7 @@ pub struct WorkdirSessionOperationRequest { #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "operation", content = "result", rename_all = "snake_case")] pub enum WorkdirSessionOperationResult { + AuthorizeScope, Stat(StatResult), Read(ReadResult), Write(WriteResult), @@ -447,6 +449,19 @@ mod client { self.capabilities } + async fn authorize_scope_path( + &self, + request: WorkdirScopeAuthorizationRequest, + ) -> Result<(), WorkdirError> { + match self + .operate(WorkdirSessionOperation::AuthorizeScope(request)) + .await? + { + WorkdirSessionOperationResult::AuthorizeScope => Ok(()), + _ => Err(Self::mismatch("authorize_scope")), + } + } + async fn stat(&self, request: StatRequest) -> Result { match self.operate(WorkdirSessionOperation::Stat(request)).await? { WorkdirSessionOperationResult::Stat(result) => Ok(result), diff --git a/crates/workdir/src/lib.rs b/crates/workdir/src/lib.rs index 4cfa91c1..4abd3c1d 100644 --- a/crates/workdir/src/lib.rs +++ b/crates/workdir/src/lib.rs @@ -28,8 +28,8 @@ pub use local::{ }; pub use operation::*; pub use scope::{ - ReadOnlyWorkdirSession, WorkdirScopeLease, WorkdirToolBroker, WorkdirToolScope, - WorkdirToolScopePermission, WorkdirToolScopeRule, + ReadOnlyWorkdirSession, WorkdirScopeAuthorizationRequest, WorkdirScopeLease, WorkdirToolBroker, + WorkdirToolScope, WorkdirToolScopePermission, WorkdirToolScopeRule, }; /// Persistent, opaque identity of one materialized Workdir. @@ -147,6 +147,25 @@ pub trait WorkdirSession: std::fmt::Debug + Send + Sync { fn workdir(&self) -> &Workdir; fn capabilities(&self) -> WorkdirSessionCapabilities; + /// Validate an attenuated filesystem rule at the provider boundary without + /// exposing the resolved host path. Providers that cannot resolve symbolic + /// links must reject resolved-policy checks rather than downgrade them. + async fn authorize_scope_path( + &self, + request: WorkdirScopeAuthorizationRequest, + ) -> Result<(), WorkdirError> { + if request.rules.iter().any(|rule| { + rule.symlink_policy == manifest::SymlinkPolicy::Logical + && scope::rule_allows_path(rule, &request.path, request.permission) + }) { + Ok(()) + } else { + Err(WorkdirError::Denied( + "Workdir provider cannot establish resolved scope authority".to_string(), + )) + } + } + async fn stat(&self, request: StatRequest) -> Result; async fn read(&self, request: ReadRequest) -> Result; async fn write(&self, request: WriteRequest) -> Result; diff --git a/crates/workdir/src/local.rs b/crates/workdir/src/local.rs index 06ad09ff..4df7c5c8 100644 --- a/crates/workdir/src/local.rs +++ b/crates/workdir/src/local.rs @@ -18,7 +18,7 @@ use std::sync::{Arc, Mutex as StdMutex}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; use async_trait::async_trait; -use manifest::{Scope, SharedScope}; +use manifest::{Permission, Scope, SharedScope, SymlinkPolicy}; use sha2::{Digest, Sha256}; use tokio::process::Command; use tokio::sync::{Mutex, broadcast, watch}; @@ -28,8 +28,9 @@ use crate::{ CommandEvent, CommandHandle, CommandOutput, CommandOutputRequest, CommandRequest, CommandSnapshot, CommandStatus, CommandStream, CommandStreamSlice, EditRequest, EditResult, GlobRequest, GlobResult, GrepRequest, GrepResult, ListRequest, ListResult, ReadRequest, - ReadResult, StatRequest, StatResult, Workdir, WorkdirError, WorkdirPath, WorkdirSession, - WorkdirSessionCapabilities, WorkdirSessionCapability, WriteRequest, WriteResult, + ReadResult, StatRequest, StatResult, Workdir, WorkdirError, WorkdirPath, + WorkdirScopeAuthorizationRequest, WorkdirSession, WorkdirSessionCapabilities, + WorkdirSessionCapability, WorkdirToolScopePermission, WriteRequest, WriteResult, }; #[cfg(test)] use crate::{EntryKind, WriteOutcome}; @@ -211,6 +212,17 @@ impl fs_operation::FsAccessPolicy for ScopeAccess { fn is_writable(&self, path: &Path) -> bool { self.0.is_writable(path) } + + fn is_readable_paths(&self, logical: &Path, resolved: &Path) -> bool { + matches!( + self.0.permission_at_paths(logical, resolved), + Some(Permission::Read | Permission::Write) + ) + } + + fn is_writable_paths(&self, logical: &Path, resolved: &Path) -> bool { + self.0.permission_at_paths(logical, resolved) == Some(Permission::Write) + } } #[derive(Debug)] @@ -397,6 +409,11 @@ impl LocalWorkdirSession { return Err(WorkdirError::RelativePath(path.to_path_buf())); } let symlink = first_symlink(path); + if let Some(info) = symlink.as_ref() + && !info.target_exists + { + return Err(broken_symlink_error(path, info)); + } let scope = self.inner.scope.load(); if !scope.is_readable(path) { return Err(symlink_out_of_scope_or_plain( @@ -406,11 +423,6 @@ impl LocalWorkdirSession { &scope, )); } - if let Some(info) = symlink.as_ref() { - if !info.target_exists { - return Err(broken_symlink_error(path, info)); - } - } let meta = std::fs::metadata(path).map_err(|e| match e.kind() { std::io::ErrorKind::NotFound => WorkdirError::NotFound(path.to_path_buf()), _ => WorkdirError::io(path, e), @@ -556,6 +568,64 @@ impl WorkdirSession for LocalWorkdirSession { self.inner.capabilities } + async fn authorize_scope_path( + &self, + request: WorkdirScopeAuthorizationRequest, + ) -> Result<(), WorkdirError> { + self.ensure_open()?; + let logical = self.inner.root.join(request.path.as_str()); + let resolved = fs_operation::resolve_access_path(&logical) + .map_err(|error| WorkdirError::io(&logical, error))?; + let parent_permission = self + .inner + .scope + .load() + .permission_at_paths(&logical, &resolved); + let parent_allows = match request.permission { + WorkdirToolScopePermission::Read => matches!( + parent_permission, + Some(Permission::Read | Permission::Write) + ), + WorkdirToolScopePermission::Write => parent_permission == Some(Permission::Write), + }; + if !parent_allows { + return Err(WorkdirError::Denied(format!( + "Workdir path `{}` exceeds the provider attachment scope", + request.path + ))); + } + let allowed = request.rules.iter().any(|rule| { + if request.permission == WorkdirToolScopePermission::Write + && rule.permission != WorkdirToolScopePermission::Write + { + return false; + } + let logical_target = self.inner.root.join(rule.target.as_str()); + let (candidate, target) = match rule.symlink_policy { + SymlinkPolicy::Logical => (logical.as_path(), logical_target), + SymlinkPolicy::Resolved => { + let Ok(target) = fs_operation::resolve_access_path(&logical_target) else { + return false; + }; + (resolved.as_path(), target) + } + }; + if rule.recursive { + candidate.starts_with(target) + } else { + candidate == target || candidate.parent() == Some(target.as_path()) + } + }); + if allowed { + Ok(()) + } else { + Err(WorkdirError::Denied(format!( + "Workdir path `{}` is outside the provider-resolved delegated scope", + request.path + ))) + } + } + async fn stat(&self, request: StatRequest) -> Result { self.ensure_capability(WorkdirSessionCapability::Read)?; let logical = request.path.clone(); @@ -1334,6 +1404,22 @@ mod tests { ) } + fn make_logical_fs(dir: &TempDir) -> LocalWorkdirSession { + LocalWorkdirSession::new( + Scope::from_config(&ScopeConfig { + allow: vec![ScopeRule { + target: dir.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + symlink_policy: SymlinkPolicy::Logical, + }], + deny: Vec::new(), + }) + .unwrap(), + dir.path().to_path_buf(), + ) + } + #[tokio::test] async fn logical_provider_operations_cover_read_write_edit_stat_and_list() { let dir = TempDir::new().unwrap(); @@ -1533,6 +1619,102 @@ mod tests { assert_eq!(read.bytes, b"persisted"); } + #[cfg(unix)] + #[tokio::test] + async fn resolved_provider_scope_rejects_read_and_write_through_outside_alias() { + use std::os::unix::fs::symlink; + + let root = TempDir::new().unwrap(); + let outside = TempDir::new().unwrap(); + let target = outside.path().join("target.txt"); + fs::write(&target, "secret").unwrap(); + symlink(&target, root.path().join("alias.txt")).unwrap(); + symlink(outside.path(), root.path().join("alias-dir")).unwrap(); + let workdir = make_fs(&root); + + assert!(matches!( + WorkdirSession::read( + &workdir, + ReadRequest { + path: WorkdirPath::new("alias.txt").unwrap(), + offset: 0, + limit: 10, + max_bytes: 1024, + } + ) + .await, + Err(WorkdirError::SymlinkOutOfScope { .. }) + )); + assert!(matches!( + WorkdirSession::write( + &workdir, + WriteRequest { + path: WorkdirPath::new("alias.txt").unwrap(), + content: b"changed".to_vec(), + expected_hash: None, + } + ) + .await, + Err(WorkdirError::SymlinkOutOfScope { .. }) + )); + assert_eq!(fs::read_to_string(target).unwrap(), "secret"); + assert!(matches!( + WorkdirSession::write( + &workdir, + WriteRequest { + path: WorkdirPath::new("alias-dir/new.txt").unwrap(), + content: b"new".to_vec(), + expected_hash: None, + } + ) + .await, + Err(WorkdirError::ReadOnly(_)) + )); + assert!(!outside.path().join("new.txt").exists()); + } + + #[cfg(unix)] + #[tokio::test] + async fn resolved_deny_blocks_missing_write_through_logical_alias() { + use std::os::unix::fs::symlink; + + let root = TempDir::new().unwrap(); + let outside = TempDir::new().unwrap(); + symlink(outside.path(), root.path().join("alias")).unwrap(); + let workdir = LocalWorkdirSession::new( + Scope::from_config(&ScopeConfig { + allow: vec![ScopeRule { + target: root.path().to_path_buf(), + permission: Permission::Write, + recursive: true, + symlink_policy: SymlinkPolicy::Logical, + }], + deny: vec![ScopeRule { + target: outside.path().join("blocked.txt"), + permission: Permission::Read, + recursive: false, + symlink_policy: SymlinkPolicy::Logical, + }], + }) + .unwrap(), + root.path().to_path_buf(), + ); + + assert!(matches!( + WorkdirSession::write( + &workdir, + WriteRequest { + path: WorkdirPath::new("alias/blocked.txt").unwrap(), + content: b"blocked".to_vec(), + expected_hash: None, + } + ) + .await, + Err(WorkdirError::ReadOnly(_)) + )); + assert!(!outside.path().join("blocked.txt").exists()); + } + #[tokio::test] async fn capability_boundary_rejects_direct_unsupported_operation() { let dir = TempDir::new().unwrap(); @@ -1645,7 +1827,7 @@ mod tests { let link = dir.path().join("outside-repo.txt"); symlink(&target, &link).unwrap(); - let fs = make_fs(&dir); + let fs = make_logical_fs(&dir); assert_eq!(fs.read_bytes(&link).unwrap(), b"secret"); } @@ -1748,7 +1930,7 @@ mod tests { let link = dir.path().join("outside-repo.txt"); symlink(&target, &link).unwrap(); - let fs = make_fs(&dir); + let fs = make_logical_fs(&dir); fs.write(&link, b"new").unwrap(); assert_eq!(fs::read(&target).unwrap(), b"new"); assert!( @@ -1778,11 +1960,13 @@ mod tests { target: dir.path().to_path_buf(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }], deny: vec![ScopeRule { target: sub.clone(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }], }; let scope = Scope::from_config(&cfg).unwrap(); @@ -1846,6 +2030,7 @@ mod tests { target: extra.path().to_path_buf(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }]) }) .unwrap(); @@ -1882,6 +2067,7 @@ mod tests { target: sub.clone(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }]) }) .unwrap(); @@ -1918,6 +2104,7 @@ mod tests { target: dir.path().to_path_buf(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }]) }) .unwrap(); @@ -1935,14 +2122,14 @@ mod tests { #[cfg(unix)] #[tokio::test] - async fn provider_uses_logical_paths_through_symlinked_directories() { + async fn provider_uses_explicit_logical_policy_through_symlinked_directories() { use std::os::unix::fs::symlink; let dir = TempDir::new().unwrap(); let outside = TempDir::new().unwrap(); std::fs::write(outside.path().join("worker.json"), "scope-needle\n").unwrap(); symlink(outside.path(), dir.path().join("yoi.local")).unwrap(); - let workdir = make_fs(&dir); + let workdir = make_logical_fs(&dir); let read = WorkdirSession::read( &workdir, @@ -1956,6 +2143,19 @@ mod tests { .await .unwrap(); assert_eq!(read.bytes, b"scope-needle\n"); + let list = WorkdirSession::list( + &workdir, + ListRequest { + path: WorkdirPath::new("yoi.local").unwrap(), + limit: 10, + }, + ) + .await + .unwrap(); + assert_eq!( + list.entries[0].path, + WorkdirPath::new("yoi.local/worker.json").unwrap() + ); let glob = WorkdirSession::glob( &workdir, GlobRequest { @@ -2084,11 +2284,13 @@ mod tests { target: dir.path().to_path_buf(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }, ScopeRule { target: spill.path().to_path_buf(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }, ], deny: Vec::new(), @@ -2165,11 +2367,13 @@ mod tests { target: dir.path().to_path_buf(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }, ScopeRule { target: spill.path().to_path_buf(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }, ], deny: Vec::new(), diff --git a/crates/workdir/src/scope.rs b/crates/workdir/src/scope.rs index 3137ddba..0a659007 100644 --- a/crates/workdir/src/scope.rs +++ b/crates/workdir/src/scope.rs @@ -8,6 +8,7 @@ use fs_operation::{ EditRequest, EditResult, FsPath, GlobRequest, GlobResult, GrepRequest, GrepResult, ListRequest, ListResult, ReadRequest, ReadResult, StatRequest, StatResult, WriteRequest, WriteResult, }; +use manifest::SymlinkPolicy; use tokio::sync::broadcast; const MAX_SCOPED_COMMANDS: usize = 16; @@ -31,6 +32,17 @@ pub struct WorkdirToolScopeRule { pub target: FsPath, pub permission: WorkdirToolScopePermission, pub recursive: bool, + #[serde(default)] + pub symlink_policy: SymlinkPolicy, +} + +/// Provider-side check for one operation under an attenuated tool scope. +#[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] +#[serde(deny_unknown_fields)] +pub struct WorkdirScopeAuthorizationRequest { + pub rules: Vec, + pub path: FsPath, + pub permission: WorkdirToolScopePermission, } #[derive(Clone, Debug, Eq, PartialEq, serde::Serialize, serde::Deserialize)] @@ -492,9 +504,39 @@ impl ScopedWorkdirSession { } } - fn resolve_operation_path(&self, path: &FsPath) -> Result { + async fn ensure_scope_targets_are_authorized( + &self, + rules: &[WorkdirToolScopeRule], + ) -> Result<(), WorkdirError> { + for rule in rules { + self.source + .authorize_scope_path(WorkdirScopeAuthorizationRequest { + rules: rules.to_vec(), + path: rule.target.clone(), + permission: rule.permission, + }) + .await?; + } + Ok(()) + } + + async fn resolve_operation_path( + &self, + path: &FsPath, + permission: WorkdirToolScopePermission, + ) -> Result { self.ensure_active()?; - self.resolve_path(path) + let resolved = self.resolve_path(path)?; + if let Some(rules) = self.scope.as_ref() { + self.source + .authorize_scope_path(WorkdirScopeAuthorizationRequest { + rules: rules.clone(), + path: resolved.clone(), + permission, + }) + .await?; + } + Ok(resolved) } fn validate_scope( @@ -582,6 +624,8 @@ impl ScopedWorkdirSession { request.cwd ))); } + self.ensure_scope_targets_are_authorized(&request.rules) + .await?; let validity = SessionValidity::child(self.validity.clone()); let cleanup_pending = Arc::new(AtomicBool::new(true)); let id = self.next_lease_id.fetch_add(1, Ordering::Relaxed); @@ -692,49 +736,63 @@ impl WorkdirSession for ScopedWorkdirSession { } async fn stat(&self, mut request: StatRequest) -> Result { - let path = self.resolve_operation_path(&request.path)?; + let path = self + .resolve_operation_path(&request.path, WorkdirToolScopePermission::Read) + .await?; self.ensure_read(&path, WorkdirSessionCapability::Read)?; request.path = path; self.source.stat(request).await } async fn read(&self, mut request: ReadRequest) -> Result { - let path = self.resolve_operation_path(&request.path)?; + let path = self + .resolve_operation_path(&request.path, WorkdirToolScopePermission::Read) + .await?; self.ensure_read(&path, WorkdirSessionCapability::Read)?; request.path = path; self.source.read(request).await } async fn write(&self, mut request: WriteRequest) -> Result { - let path = self.resolve_operation_path(&request.path)?; + let path = self + .resolve_operation_path(&request.path, WorkdirToolScopePermission::Write) + .await?; self.ensure_write(&path, WorkdirSessionCapability::Write)?; request.path = path; self.source.write(request).await } async fn edit(&self, mut request: EditRequest) -> Result { - let path = self.resolve_operation_path(&request.path)?; + let path = self + .resolve_operation_path(&request.path, WorkdirToolScopePermission::Write) + .await?; self.ensure_write(&path, WorkdirSessionCapability::Edit)?; request.path = path; self.source.edit(request).await } async fn list(&self, mut request: ListRequest) -> Result { - let path = self.resolve_operation_path(&request.path)?; + let path = self + .resolve_operation_path(&request.path, WorkdirToolScopePermission::Read) + .await?; self.ensure_read(&path, WorkdirSessionCapability::Read)?; request.path = path; self.source.list(request).await } async fn glob(&self, mut request: GlobRequest) -> Result { - let path = self.resolve_operation_path(&request.path)?; + let path = self + .resolve_operation_path(&request.path, WorkdirToolScopePermission::Read) + .await?; self.ensure_read(&path, WorkdirSessionCapability::Glob)?; request.path = path; self.source.glob(request).await } async fn grep(&self, mut request: GrepRequest) -> Result { - let path = self.resolve_operation_path(&request.path)?; + let path = self + .resolve_operation_path(&request.path, WorkdirToolScopePermission::Read) + .await?; self.ensure_read(&path, WorkdirSessionCapability::Grep)?; request.path = path; self.source.grep(request).await @@ -943,6 +1001,16 @@ impl WorkdirSession for ReadOnlyWorkdirSession { WorkdirSessionCapabilities::READ_ONLY } + async fn authorize_scope_path( + &self, + request: WorkdirScopeAuthorizationRequest, + ) -> Result<(), WorkdirError> { + if request.permission == WorkdirToolScopePermission::Write { + return Err(WorkdirError::Denied("read-only workdir session".into())); + } + self.inner.authorize_scope_path(request).await + } + async fn stat(&self, request: StatRequest) -> Result { self.inner.stat(request).await } @@ -1106,7 +1174,7 @@ fn rules_overlap(left: &WorkdirToolScopeRule, right: &WorkdirToolScopeRule) -> b || rule_allows_path(right, &left.target, WorkdirToolScopePermission::Write)) } -fn rule_allows_path( +pub(crate) fn rule_allows_path( rule: &WorkdirToolScopeRule, path: &FsPath, required: WorkdirToolScopePermission, @@ -1138,6 +1206,11 @@ fn rule_contains_rule(parent: &WorkdirToolScopeRule, child: &WorkdirToolScopeRul { return false; } + // Resolved < Logical: a child may narrow a Logical grant to Resolved, + // but cannot turn a Resolved parent grant into logical-alias authority. + if parent.symlink_policy < child.symlink_policy { + return false; + } if !path_in_rule(parent, &child.target) { return false; } @@ -1168,6 +1241,7 @@ mod tests { target: root.to_path_buf(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }], deny: Vec::new(), }) @@ -1188,6 +1262,7 @@ mod tests { target: fs_path(path), permission, recursive: true, + symlink_policy: Default::default(), }], cwd: fs_path(path), command: permission == WorkdirToolScopePermission::Write, @@ -1298,6 +1373,7 @@ mod tests { target: fs_path("work"), permission: WorkdirToolScopePermission::Write, recursive: true, + symlink_policy: Default::default(), }], cwd: fs_path("work"), command: false, @@ -1385,12 +1461,24 @@ mod tests { ); } + #[test] + fn workdir_rule_defaults_to_resolved_symlink_policy_on_restore() { + let rule: WorkdirToolScopeRule = serde_json::from_value(serde_json::json!({ + "target": "src", + "permission": "read", + "recursive": true + })) + .unwrap(); + assert_eq!(rule.symlink_policy, SymlinkPolicy::Resolved); + } + #[test] fn non_recursive_rule_covers_target_and_direct_children_only() { let rule = WorkdirToolScopeRule { target: fs_path("docs"), permission: WorkdirToolScopePermission::Read, recursive: false, + symlink_policy: Default::default(), }; assert!(path_in_rule(&rule, &fs_path("docs"))); assert!(path_in_rule(&rule, &fs_path("docs/readme.md"))); @@ -1443,7 +1531,7 @@ mod tests { #[cfg(unix)] #[tokio::test] - async fn provider_scope_allows_read_through_its_logical_symlink_path() { + async fn provider_scope_rejects_symlink_aliases_by_default() { use std::os::unix::fs::symlink; let root = TempDir::new().unwrap(); @@ -1457,6 +1545,73 @@ mod tests { .await .unwrap(); + assert!(matches!( + child.read(read("link")).await, + Err(WorkdirError::Denied(message)) + if message.contains("provider-resolved delegated scope") + )); + } + + #[cfg(unix)] + #[tokio::test] + async fn resolved_scope_follows_its_target_but_rejects_nested_escape() { + use std::os::unix::fs::symlink; + + let root = TempDir::new().unwrap(); + fs::create_dir_all(root.path().join("target")).unwrap(); + fs::create_dir_all(root.path().join("secret")).unwrap(); + fs::write(root.path().join("target/visible"), "visible").unwrap(); + fs::write(root.path().join("secret/key"), "hidden").unwrap(); + symlink("target", root.path().join("granted")).unwrap(); + symlink("../secret/key", root.path().join("target/escape")).unwrap(); + let parent = session(root.path()); + let child = parent + .scope(request("granted", WorkdirToolScopePermission::Read)) + .await + .unwrap(); + + assert_eq!(child.read(read("visible")).await.unwrap().bytes, b"visible"); + assert!(matches!( + child.read(read("escape")).await, + Err(WorkdirError::Denied(message)) + if message.contains("provider-resolved delegated scope") + )); + } + + #[tokio::test] + async fn nested_scope_cannot_expand_resolved_policy_to_logical() { + let root = TempDir::new().unwrap(); + fs::create_dir_all(root.path().join("granted")).unwrap(); + let parent = session(root.path()); + let child = parent + .scope(request("granted", WorkdirToolScopePermission::Read)) + .await + .unwrap(); + let mut expanded = request(".", WorkdirToolScopePermission::Read); + expanded.rules[0].symlink_policy = SymlinkPolicy::Logical; + + assert!(matches!( + child.scope(expanded).await, + Err(WorkdirError::Denied(message)) + if message.contains("exceeds the parent tool scope") + )); + } + + #[cfg(unix)] + #[tokio::test] + async fn provider_scope_allows_read_through_its_logical_symlink_path() { + 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(); + fs::write(root.path().join("secret/key"), "hidden").unwrap(); + symlink("../secret/key", root.path().join("granted/link")).unwrap(); + let parent = session(root.path()); + let mut scope = request("granted", WorkdirToolScopePermission::Read); + scope.rules[0].symlink_policy = SymlinkPolicy::Logical; + let child = parent.scope(scope).await.unwrap(); + assert_eq!(child.read(read("link")).await.unwrap().bytes, b"hidden"); } @@ -1470,10 +1625,9 @@ mod tests { fs::create_dir_all(root.path().join("secret")).unwrap(); symlink("../secret", root.path().join("granted/outside")).unwrap(); let parent = session(root.path()); - let child = parent - .scope(request("granted", WorkdirToolScopePermission::Write)) - .await - .unwrap(); + let mut scope = request("granted", WorkdirToolScopePermission::Write); + scope.rules[0].symlink_policy = SymlinkPolicy::Logical; + let child = parent.scope(scope).await.unwrap(); child .write(write("outside/new", "through-logical-path")) @@ -1496,13 +1650,9 @@ mod tests { symlink("../secret", root.path().join("granted/outside")).unwrap(); let parent = session(root.path()); - let child = parent - .scope(request( - "granted/outside", - WorkdirToolScopePermission::Write, - )) - .await - .unwrap(); + let mut scope = request("granted/outside", WorkdirToolScopePermission::Write); + scope.rules[0].symlink_policy = SymlinkPolicy::Logical; + let child = parent.scope(scope).await.unwrap(); child .write(write("from-child", "child-authoritative")) .await diff --git a/crates/worker-runtime/src/http_server.rs b/crates/worker-runtime/src/http_server.rs index b63378d9..298deb72 100644 --- a/crates/worker-runtime/src/http_server.rs +++ b/crates/worker-runtime/src/http_server.rs @@ -1012,6 +1012,10 @@ async fn run_workdir_session_operation( let operation = request.operation; let result = match operation { + WorkdirSessionOperation::AuthorizeScope(request) => { + session.authorize_scope_path(request).await?; + WorkdirSessionOperationResult::AuthorizeScope + } WorkdirSessionOperation::Stat(request) => { WorkdirSessionOperationResult::Stat(session.stat(request).await?) } @@ -2983,6 +2987,33 @@ mod tests { .expect("owned operation"); assert!(matches!(result, WorkdirSessionOperationResult::Stat(_))); + let authorization = WorkdirSessionOperationRequest { + operation: WorkdirSessionOperation::AuthorizeScope( + workdir::WorkdirScopeAuthorizationRequest { + rules: vec![workdir::WorkdirToolScopeRule { + target: WorkdirPath::new("hello.txt").unwrap(), + permission: workdir::WorkdirToolScopePermission::Read, + recursive: false, + symlink_policy: Default::default(), + }], + path: WorkdirPath::new("hello.txt").unwrap(), + permission: workdir::WorkdirToolScopePermission::Read, + }, + ), + }; + let Json(result) = run_workdir_session_operation( + State(state.clone()), + Path("session-1".to_string()), + Some(Extension(auth.clone())), + Ok(Json(authorization)), + ) + .await + .expect("provider-side scope authorization"); + assert!(matches!( + result, + WorkdirSessionOperationResult::AuthorizeScope + )); + let grep = WorkdirSessionOperationRequest { operation: WorkdirSessionOperation::Grep(GrepRequest { pattern: "hello".into(), diff --git a/crates/worker/src/controller.rs b/crates/worker/src/controller.rs index 26e8a77c..a080c678 100644 --- a/crates/worker/src/controller.rs +++ b/crates/worker/src/controller.rs @@ -699,6 +699,7 @@ impl WorkerController { target: bash_output_dir.clone(), permission: manifest::Permission::Read, recursive: true, + symlink_policy: Default::default(), }]) .map_err(std::io::Error::other)?; diff --git a/crates/worker/src/discovery.rs b/crates/worker/src/discovery.rs index 844d8c52..a3864b62 100644 --- a/crates/worker/src/discovery.rs +++ b/crates/worker/src/discovery.rs @@ -743,6 +743,7 @@ fn comm_info_from_spawned_child(child: &session_store::WorkerSpawnedChild) -> Co target: rule.target.clone(), permission, recursive: rule.recursive, + symlink_policy: rule.symlink_policy, }) }) .collect(); @@ -1324,6 +1325,7 @@ mod tests { target: root.path().to_path_buf(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }], active_child_segment, ) @@ -1795,6 +1797,7 @@ mod tests { target: PathBuf::from("/tmp"), permission: "read".into(), recursive: true, + symlink_policy: Default::default(), }], callback_address: PathBuf::from("/tmp/parent.sock"), } diff --git a/crates/worker/src/entrypoint.rs b/crates/worker/src/entrypoint.rs index 562e7b67..6270caf3 100644 --- a/crates/worker/src/entrypoint.rs +++ b/crates/worker/src/entrypoint.rs @@ -286,6 +286,7 @@ fn read_rule(target: PathBuf) -> ScopeRule { target, permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), } } @@ -294,6 +295,7 @@ fn write_rule(target: PathBuf) -> ScopeRule { target, permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), } } @@ -306,6 +308,7 @@ fn workspace_scope( target: workspace_root.to_path_buf(), permission, recursive: true, + symlink_policy: Default::default(), }; let deny = deny_write .iter() @@ -711,6 +714,7 @@ permission = "write" target: target.to_path_buf(), permission, recursive: true, + symlink_policy: Default::default(), } } diff --git a/crates/worker/src/feature/builtin/manage_workdir.rs b/crates/worker/src/feature/builtin/manage_workdir.rs index b481d144..4302ab76 100644 --- a/crates/worker/src/feature/builtin/manage_workdir.rs +++ b/crates/worker/src/feature/builtin/manage_workdir.rs @@ -18,8 +18,9 @@ use workdir::workspace::WorkspaceWorkdirSessionOperationRequest; use workdir::{ CommandHandle, CommandOutput, CommandOutputRequest, CommandRequest, CommandStatus, EditRequest, EditResult, GlobRequest, GlobResult, GrepRequest, GrepResult, ListRequest, ListResult, - ReadRequest, ReadResult, StatRequest, StatResult, Workdir, WorkdirError, WorkdirSession, - WorkdirSessionCapabilities, WorkdirSessionHandle, WriteRequest, WriteResult, + ReadRequest, ReadResult, StatRequest, StatResult, Workdir, WorkdirError, + WorkdirScopeAuthorizationRequest, WorkdirSession, WorkdirSessionCapabilities, + WorkdirSessionHandle, WriteRequest, WriteResult, }; use workspace_api::{ @@ -283,6 +284,16 @@ impl WorkdirSession for WorkspaceAttachedWorkdirSession { WorkdirSessionCapabilities::ALL } + async fn authorize_scope_path( + &self, + request: WorkdirScopeAuthorizationRequest, + ) -> Result<(), WorkdirError> { + match self.operate(WorkdirSessionOperation::AuthorizeScope(request))? { + WorkdirSessionOperationResult::AuthorizeScope => Ok(()), + _ => Err(Self::mismatch("authorize_scope")), + } + } + async fn stat(&self, request: StatRequest) -> Result { match self.operate(WorkdirSessionOperation::Stat(request))? { WorkdirSessionOperationResult::Stat(result) => Ok(result), @@ -1242,10 +1253,14 @@ mod tests { #[tokio::test] async fn scoped_broker_operations_carry_no_child_context() { - let client = Arc::new(RecordingWorkspaceClient::new(vec![response(json!({ - "operation": "stat", - "result": {"path": "visible.txt", "kind": "file", "size": 8} - }))])); + let client = Arc::new(RecordingWorkspaceClient::new(vec![ + response(json!({ "operation": "authorize_scope" })), + response(json!({ "operation": "authorize_scope" })), + response(json!({ + "operation": "stat", + "result": {"path": "visible.txt", "kind": "file", "size": 8} + })), + ])); let broker = workdir::WorkdirToolBroker::new(WorkspaceAttachedWorkdirSession::handle( client.clone(), )); @@ -1255,6 +1270,7 @@ mod tests { target: workdir::WorkdirPath::new("").unwrap(), permission: workdir::WorkdirToolScopePermission::Read, recursive: true, + symlink_policy: Default::default(), }], cwd: workdir::WorkdirPath::new("").unwrap(), command: false, @@ -1269,7 +1285,7 @@ mod tests { .unwrap(); let requests = client.requests(); - assert_eq!(requests.len(), 1); + assert_eq!(requests.len(), 3); for request in requests { assert_eq!( request.path, diff --git a/crates/worker/src/fs_view.rs b/crates/worker/src/fs_view.rs index 623fe074..cca540b0 100644 --- a/crates/worker/src/fs_view.rs +++ b/crates/worker/src/fs_view.rs @@ -411,11 +411,13 @@ mod tests { target: dir.path().to_path_buf(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }], deny: vec![ScopeRule { target: secret.clone(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }], }; let scope = Scope::from_config(&cfg).unwrap(); @@ -574,11 +576,13 @@ mod tests { target: dir.path().to_path_buf(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }], deny: vec![ScopeRule { target: secret.clone(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }], }; let scope = Scope::from_config(&cfg).unwrap(); diff --git a/crates/worker/src/runtime/dir.rs b/crates/worker/src/runtime/dir.rs index 37d8d99e..e812ffe6 100644 --- a/crates/worker/src/runtime/dir.rs +++ b/crates/worker/src/runtime/dir.rs @@ -299,6 +299,7 @@ mod tests { target: "/tmp/work".into(), permission: Permission::Write, recursive: true, + symlink_policy: Default::default(), }], callback_address: "/run/yoi/my-worker/sock".into(), }]; diff --git a/crates/worker/src/runtime/worker_allocation/test_util.rs b/crates/worker/src/runtime/worker_allocation/test_util.rs index d307933c..7744dd7d 100644 --- a/crates/worker/src/runtime/worker_allocation/test_util.rs +++ b/crates/worker/src/runtime/worker_allocation/test_util.rs @@ -77,6 +77,7 @@ pub(crate) fn write_rule(path: &str, recursive: bool) -> ScopeRule { target: PathBuf::from(path), permission: Permission::Write, recursive, + symlink_policy: Default::default(), } } @@ -85,6 +86,7 @@ pub(crate) fn read_rule(path: &str, recursive: bool) -> ScopeRule { target: PathBuf::from(path), permission: Permission::Read, recursive, + symlink_policy: Default::default(), } } diff --git a/crates/worker/src/spawn/registry.rs b/crates/worker/src/spawn/registry.rs index fd946e54..f328838d 100644 --- a/crates/worker/src/spawn/registry.rs +++ b/crates/worker/src/spawn/registry.rs @@ -1030,6 +1030,7 @@ fn record_from_worker_state(child: &WorkerSpawnedChild) -> io::Result>>()?; @@ -1072,6 +1073,7 @@ mod tests { target: std::path::PathBuf::from("/tmp"), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }], deny: Vec::new(), }) @@ -1090,6 +1092,7 @@ mod tests { target: root.clone(), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }], deny: Vec::new(), }) @@ -1109,6 +1112,7 @@ mod tests { target: workdir::WorkdirPath::new("").unwrap(), permission: workdir::WorkdirToolScopePermission::Read, recursive: true, + symlink_policy: Default::default(), }], cwd: workdir::WorkdirPath::new("").unwrap(), command: false, diff --git a/crates/worker/src/spawn/tool.rs b/crates/worker/src/spawn/tool.rs index 6ec7c3cc..37a2e123 100644 --- a/crates/worker/src/spawn/tool.rs +++ b/crates/worker/src/spawn/tool.rs @@ -16,8 +16,8 @@ use manifest::{ CompactionConfigPartial, EngineManifestConfig, FileUploadLimitsPartial, PermissionConfigPartial, ProfileDiscovery, ProfileError, ProfileRegistry, ProfileRegistrySource, ProfileResolveOptions, ProfileResolver, ProfileSelector, ScopeConfig, - ScopeRule, SessionConfigPartial, ToolOutputLimitsPartial, WorkerManifest, WorkerManifestConfig, - WorkerMetaConfig, + ScopeRule, SessionConfigPartial, SymlinkPolicy, ToolOutputLimitsPartial, WorkerManifest, + WorkerManifestConfig, WorkerMetaConfig, }; use serde::Deserialize; use tokio::sync::mpsc; @@ -61,7 +61,9 @@ struct SubWorkerSpawnInput { task: String, /// Allow rules delegated to the spawned SubWorker. Must be a subset of the /// spawner's explicit delegation authority; direct tool scope alone is not - /// sufficient. Omit `recursive` for normal workspace/worktree delegation; it defaults to true. + /// sufficient. Omit `recursive` for normal workspace/worktree delegation; + /// it defaults to true. Omit `symlink_policy` for the least-authority + /// `resolved` policy; `logical` requires matching parent authority. scope: Vec, /// Explicitly grant command execution through the parent-owned Workdir tool broker. #[serde(default)] @@ -88,6 +90,27 @@ struct ScopeRuleInput { /// children only. Defaults to `true`. #[serde(default = "default_true")] recursive: bool, + /// Symbolic-link identity used by this rule. `resolved` is the default + /// and least authority; `logical` requires matching parent authority. + #[serde(default)] + symlink_policy: SymlinkPolicyInput, +} + +#[derive(Debug, Default, Deserialize, schemars::JsonSchema, Clone, Copy)] +#[serde(rename_all = "lowercase")] +enum SymlinkPolicyInput { + #[default] + Resolved, + Logical, +} + +impl From for SymlinkPolicy { + fn from(value: SymlinkPolicyInput) -> Self { + match value { + SymlinkPolicyInput::Resolved => Self::Resolved, + SymlinkPolicyInput::Logical => Self::Logical, + } + } } #[derive(Debug, Deserialize, schemars::JsonSchema, Clone, Copy)] @@ -506,6 +529,7 @@ impl Tool for SubWorkerSpawnTool { target: child_bash_output_dir.clone(), permission: manifest::Permission::Read, recursive: true, + symlink_policy: Default::default(), }]) .map_err(|error| { ToolError::ExecutionFailed(format!( @@ -707,6 +731,7 @@ fn parse_workdir_scope(rules: &[ScopeRuleInput]) -> Result WorkdirToolScopePermission::Write, }, recursive: rule.recursive, + symlink_policy: rule.symlink_policy.into(), }) }) .collect() @@ -1074,21 +1099,26 @@ mod tests { target: ".".to_string(), permission: PermissionInput::Read, recursive: true, + symlink_policy: Default::default(), }, ScopeRuleInput { target: "src".to_string(), permission: PermissionInput::Write, recursive: false, + symlink_policy: SymlinkPolicyInput::Logical, }, ]) .unwrap(); assert_eq!(rules[0].target.as_str(), ""); assert_eq!(rules[1].target.as_str(), "src"); + assert_eq!(rules[0].symlink_policy, SymlinkPolicy::Resolved); + assert_eq!(rules[1].symlink_policy, SymlinkPolicy::Logical); for target in ["/host/path", "../escape"] { let error = parse_workdir_scope(&[ScopeRuleInput { target: target.to_string(), permission: PermissionInput::Read, recursive: true, + symlink_policy: Default::default(), }]) .unwrap_err(); assert!(matches!(error, ToolError::InvalidArgument(_))); @@ -1126,6 +1156,7 @@ mod tests { target: path.to_path_buf(), permission, recursive: true, + symlink_policy: Default::default(), } } @@ -1533,10 +1564,14 @@ enabled = false assert!(record.installed_tools.iter().any(|tool| tool == "Write")); assert!(!record.installed_tools.iter().any(|tool| tool == "Bash")); assert_eq!(calls.load(Ordering::SeqCst), 1); - assert!( - remote_client.requests().is_empty(), - "spawning a child must not open or delegate a provider Workdir session" - ); + let requests = remote_client.requests(); + assert!(!requests.is_empty()); + assert!(requests.iter().all(|request| { + let body = request.body.as_deref().unwrap_or_default(); + body.contains("authorize_scope") + && !body.contains(&bash_output_dir.display().to_string()) + && !body.contains(&workspace_root.display().to_string()) + })); } #[test] @@ -1548,6 +1583,9 @@ enabled = false .expect("schema properties"); assert!(properties.contains_key("cwd"), "schema: {schema}"); assert!(properties.contains_key("command"), "schema: {schema}"); + let schema_text = serde_json::to_string(&schema).unwrap(); + assert!(schema_text.contains("symlink_policy"), "schema: {schema}"); + assert!(schema_text.contains("logical"), "schema: {schema}"); let required = schema .get("required") .and_then(serde_json::Value::as_array) @@ -1708,10 +1746,29 @@ enabled = false self.requests .lock() .expect("remote Workdir request lock") - .push(request); - Err(WorkspaceClientError::Request( - "SubWorker spawn must not call the remote Workdir provider".into(), - )) + .push(request.clone()); + let operation: workdir::workspace::WorkspaceWorkdirSessionOperationRequest = + serde_json::from_str(request.body.as_deref().unwrap_or_default()).map_err( + |error| { + WorkspaceClientError::Request(format!( + "invalid remote Workdir operation: {error}" + )) + }, + )?; + match operation.operation { + workdir::http::WorkdirSessionOperation::AuthorizeScope(_) => { + Ok(WorkspaceResponse { + status: 200, + body: serde_json::to_string( + &workdir::http::WorkdirSessionOperationResult::AuthorizeScope, + ) + .unwrap(), + }) + } + _ => Err(WorkspaceClientError::Request( + "SubWorker spawn may only authorize its provider-side scope".into(), + )), + } } } @@ -1905,6 +1962,7 @@ max_tokens = 3333 target: PathBuf::from("/tmp/child"), permission: Permission::Read, recursive: true, + symlink_policy: Default::default(), }]; let config_json = diff --git a/crates/worker/src/worker.rs b/crates/worker/src/worker.rs index 93ec40ac..b1da9132 100644 --- a/crates/worker/src/worker.rs +++ b/crates/worker/src/worker.rs @@ -7256,6 +7256,7 @@ fn delegated_scope_rule_to_scope_rule(rule: WorkerSpawnedScopeRule) -> Option std::result::Result { match operation { + WorkdirSessionOperation::AuthorizeScope(request) => session + .authorize_scope_path(request) + .await + .map(|()| WorkdirSessionOperationResult::AuthorizeScope), WorkdirSessionOperation::Stat(request) => session .stat(request) .await diff --git a/web/workspace/src/lib/generated/protocol.ts b/web/workspace/src/lib/generated/protocol.ts index b3482873..10b573b0 100644 --- a/web/workspace/src/lib/generated/protocol.ts +++ b/web/workspace/src/lib/generated/protocol.ts @@ -53,6 +53,8 @@ export type ErrorCode = "already_running" | "not_running" | "not_paused" | "prov export type Permission = "read" | "write"; +export type SymlinkPolicy = "resolved" | "logical"; + export type InFlightToolCallState = "pending" | "streaming_args" | "done"; export type CommandStatus = "running" | "completed" | "failed" | "timed_out" | "cancelled"; @@ -94,7 +96,12 @@ permission: Permission, * When `false`, the rule only matches the target itself and its * direct children. Defaults to `true`. */ -recursive: boolean, }; +recursive: boolean, +/** + * Which path identity an allow rule uses when symbolic links are + * encountered. Deny rules always inspect both identities. + */ +symlink_policy: SymlinkPolicy, }; export type CompletionEntry = { value: string, is_dir: boolean, };