Hare/yoi
1
1
Fork 0
Code Actions Packages Releases Activity

feat: authorize scoped symlink paths lexically

Browse Source
This commit is contained in:
Hare
2026-09-09 00:51:21 +09:00
parent 18fd6a1f5e
commit fcc7d79d80
7 changed files with 194 additions and 167 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/fs-operation/src/glob.rs
+4 -12
View File
@@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
use globset::Glob;
use ignore::WalkBuilder;
use crate::{FsAccessPolicy, FsError, FsPath, GlobRequest, GlobResult, direct_symlink};
use crate::{FsAccessPolicy, FsError, FsPath, GlobRequest, GlobResult};
/// Execute a bounded glob entirely inside the provider process.
pub fn run_glob(
@@ -18,21 +18,13 @@ pub fn run_glob(
if !access.is_readable(base) {
return Err(FsError::OutOfScope(PathBuf::from(request.path.as_str())));
}
if let Some(info) = direct_symlink(base)
&& info.target_exists
&& info.resolved_path.is_dir()
{
return Err(FsError::SymlinkDirectoryNotTraversed {
tool: "Glob",
path: PathBuf::from(request.path.as_str()),
target: PathBuf::from("<provider-internal target>"),
});
}
let matcher = Glob::new(&request.pattern)
.map_err(|error| FsError::InvalidGlob(error.to_string()))?
.compile_matcher();
let mut matches = Vec::new();
for entry in WalkBuilder::new(base).hidden(false).build().flatten() {
let mut walker = WalkBuilder::new(base);
walker.hidden(false).follow_links(false);
for entry in walker.build().flatten() {
let path = entry.path();
if !path.is_file() || !access.is_readable(path) {
continue;
crates/fs-operation/src/lib.rs
+26 -8
View File
@@ -477,13 +477,14 @@ mod tests {
#[cfg(unix)]
#[test]
fn grep_keeps_direct_symlink_directory_and_broken_path_guards() {
fn grep_traverses_a_direct_symlink_directory_and_rejects_a_broken_path() {
use std::os::unix::fs::symlink;
let temp = tempfile::tempdir().unwrap();
let root = temp.path().canonicalize().unwrap();
let readable = RootAccess(root.clone());
std::fs::create_dir(root.join("target-dir")).unwrap();
std::fs::write(root.join("target-dir/nested.rs"), "needle nested\n").unwrap();
std::fs::write(root.join("target-file.rs"), "needle file\n").unwrap();
symlink(root.join("target-file.rs"), root.join("file-link.rs")).unwrap();
symlink(root.join("target-dir"), root.join("directory-link")).unwrap();
@@ -501,18 +502,35 @@ mod tests {
assert_eq!(file_result.match_count, 1);
assert!(file_result.output.starts_with("file-link.rs\n"));
let directory_error = run_grep(
let directory_result = run_grep(
&root,
root.join("directory-link"),
request("directory-link"),
&readable,
)
.unwrap_err();
assert!(matches!(
directory_error,
FsError::SymlinkDirectoryNotTraversed { tool: "Grep", path, .. }
if path == root.join("directory-link")
));
.unwrap();
assert_eq!(directory_result.match_count, 1);
assert!(
directory_result
.output
.starts_with("directory-link/nested.rs\n")
);
let glob_result = run_glob(
&root,
&root.join("directory-link"),
GlobRequest {
pattern: "**/*.rs".to_string(),
path: FsPath::new("directory-link").unwrap(),
limit: 10,
},
&readable,
)
.unwrap();
assert_eq!(
glob_result.paths,
vec![FsPath::new("directory-link/nested.rs").unwrap()]
);
let broken_error = run_grep(
&root,
crates/fs-operation/src/local.rs
+9 -8
View File
@@ -45,7 +45,7 @@ pub fn run_read(
) -> Result<ReadResult, FsError> {
let logical = request.path;
let path = resolve(root, &logical)?;
let path = require_access(&path, &logical, access, false)?;
let path = require_access(&path, &logical, access, false, false)?;
let metadata = fs::metadata(&path).map_err(|error| map_io(&logical, error))?;
if metadata.is_dir() {
return Err(FsError::IsDirectory(PathBuf::from(logical.as_str())));
@@ -99,7 +99,7 @@ pub fn run_write(
let path = resolve(root, &logical)?;
let created = !path.exists();
if path.exists() {
let target = require_access(&path, &logical, access, true)?;
let target = require_access(&path, &logical, access, true, false)?;
let metadata = fs::metadata(&target).map_err(|error| map_io(&logical, error))?;
if metadata.is_dir() {
return Err(FsError::IsDirectory(PathBuf::from(logical.as_str())));
@@ -117,7 +117,7 @@ pub fn run_write(
FsError::InvalidArgument(format!("{} has no parent", logical.as_str()))
})?;
let parent_logical = logical_parent(&logical);
require_access(parent, &parent_logical, access, true)?;
require_access(parent, &parent_logical, access, true, true)?;
atomic_write(&path, &request.content, &logical)?;
}
Ok(WriteResult {
@@ -133,7 +133,7 @@ pub fn run_edit(
) -> Result<EditResult, FsError> {
let logical = request.path;
let path = resolve(root, &logical)?;
let target = require_access(&path, &logical, access, true)?;
let target = require_access(&path, &logical, access, true, false)?;
let bytes = fs::read(&target).map_err(|error| map_io(&logical, error))?;
let actual_hash = hash_bytes(&bytes);
if actual_hash != request.expected_hash {
@@ -173,7 +173,7 @@ pub fn run_list(
) -> Result<ListResult, FsError> {
let logical = request.path;
let path = resolve(root, &logical)?;
let path = require_access(&path, &logical, access, false)?;
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() {
return Err(FsError::NotDirectory(PathBuf::from(logical.as_str())));
@@ -247,6 +247,7 @@ fn require_access(
logical: &FsPath,
access: &dyn FsAccessPolicy,
write: bool,
allow_symlink_directory: bool,
) -> Result<PathBuf, FsError> {
if let Some(info) = direct_symlink(path) {
if !info.target_exists {
@@ -257,9 +258,9 @@ fn require_access(
});
}
let allowed = if write {
access.is_writable(&info.resolved_path)
access.is_writable(path)
} else {
access.is_readable(&info.resolved_path)
access.is_readable(path)
};
if !allowed {
return Err(FsError::SymlinkOutOfScope {
@@ -268,7 +269,7 @@ fn require_access(
required_permission: if write { "write" } else { "read" },
});
}
if write && info.resolved_path.is_dir() {
if !allow_symlink_directory && info.resolved_path.is_dir() {
return Err(FsError::SymlinkTargetIsDirectory {
path: PathBuf::from(logical.as_str()),
target: PathBuf::from("<provider-internal target>"),
crates/fs-operation/src/search.rs
-10
View File
@@ -259,16 +259,6 @@ pub fn run_grep(
base.display()
)));
}
if base_meta.is_dir()
&& let Some(info) = symlink.as_ref()
{
return Err(FsError::SymlinkDirectoryNotTraversed {
tool: "Grep",
path: base.clone(),
target: info.resolved_path.clone(),
});
}
let filter_base = if base_meta.is_file() { root } else { &base };
let types = build_types(p.file_type.as_deref())?;
let overrides = build_overrides(filter_base, p.glob.as_deref())?;
crates/manifest/src/scope.rs
+47 -36
View File
@@ -3,11 +3,11 @@
//! 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 canonicalised (where
//! possible) so access checks are pure path comparisons.
//! 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.
use std::ffi::OsString;
use std::path::{Path, PathBuf};
use std::path::{Component, Path, PathBuf};
use std::sync::{Arc, Mutex};
use arc_swap::{ArcSwap, Guard};
@@ -26,7 +26,7 @@ pub struct Scope {
#[derive(Debug, Clone, PartialEq, Eq)]
struct ResolvedRule {
/// Absolute, canonicalized-or-normalized target directory/file.
/// Absolute, lexically normalized target directory/file.
target: PathBuf,
permission: Permission,
recursive: bool,
@@ -201,9 +201,14 @@ impl Scope {
}
/// Convenience constructor for tests and simple setups: a single
/// recursive `allow(Write)` rule rooted at `root`.
/// recursive `allow(Write)` rule rooted at the lexical path `root`.
pub fn writable(root: impl AsRef<Path>) -> std::io::Result<Self> {
let root = root.as_ref().canonicalize()?;
let root = normalize_path(root.as_ref()).ok_or_else(|| {
std::io::Error::new(
std::io::ErrorKind::InvalidInput,
"scope root must be an absolute path without root traversal",
)
})?;
Ok(Self {
allow: vec![ResolvedRule {
target: root,
@@ -214,8 +219,7 @@ impl Scope {
})
}
/// Resolve one rule target with the same symlink and missing-tail semantics
/// used by scope matching.
/// Return one rule's lexically normalized target without resolving symlinks.
pub fn resolved_target(rule: &ScopeRule) -> Result<PathBuf, ScopeError> {
Ok(resolve_rule(rule)?.target)
}
@@ -244,7 +248,7 @@ 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<Permission> {
let resolved = resolve_path(path)?;
let resolved = normalize_path(path)?;
let mut effective: Option<Permission> = None;
for rule in &self.allow {
if rule.matches(&resolved) {
@@ -523,7 +527,7 @@ fn resolve_rule(rule: &ScopeRule) -> Result<ResolvedRule, ScopeError> {
if !rule.target.is_absolute() {
return Err(ScopeError::RelativeTarget(rule.target.clone()));
}
let target = resolve_path(&rule.target).ok_or_else(|| ScopeError::ResolveTarget {
let 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"),
})?;
@@ -534,37 +538,27 @@ fn resolve_rule(rule: &ScopeRule) -> Result<ResolvedRule, ScopeError> {
})
}
/// Convert `path` to an absolute form suitable for prefix comparison.
///
/// Tries `canonicalize` on the full path first (resolves symlinks). If
/// the path doesn't exist yet, climbs to the closest existing ancestor,
/// canonicalizes it, then rejoins the missing tail. Returns `None` for
/// relative inputs that have no existing ancestor to anchor against.
fn resolve_path(path: &Path) -> Option<PathBuf> {
/// Normalize an absolute path for lexical scope comparison without consulting
/// filesystem metadata or resolving symbolic links.
fn normalize_path(path: &Path) -> Option<PathBuf> {
if !path.is_absolute() {
return None;
}
if let Ok(canonical) = path.canonicalize() {
return Some(canonical);
}
let mut tail: Vec<OsString> = Vec::new();
let mut cur = path.to_path_buf();
loop {
if let Ok(canonical) = cur.canonicalize() {
let mut out = canonical;
for segment in tail.iter().rev() {
out.push(segment);
let mut normalized = PathBuf::new();
for component in path.components() {
match component {
Component::Prefix(prefix) => normalized.push(prefix.as_os_str()),
Component::RootDir => normalized.push(component.as_os_str()),
Component::CurDir => {}
Component::ParentDir => {
if !normalized.pop() {
return None;
}
}
return Some(out);
Component::Normal(part) => normalized.push(part),
}
let name = cur.file_name()?.to_os_string();
tail.push(name);
let parent = cur.parent()?.to_path_buf();
if parent == cur {
return None;
}
cur = parent;
}
normalized.is_absolute().then_some(normalized)
}
#[cfg(test)]
@@ -805,6 +799,23 @@ mod tests {
assert!(!scope.is_readable(&traversal));
}
#[cfg(unix)]
#[test]
fn scope_authorizes_symlink_paths_lexically_without_authorizing_targets() {
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")));
}
#[test]
fn summary_lists_readable_and_writable() {
let dir = TempDir::new().unwrap();
crates/workdir/src/local.rs
+73 -18
View File
@@ -1635,7 +1635,7 @@ mod tests {
#[cfg(unix)]
#[test]
fn read_bytes_reports_symlink_target_outside_scope() {
fn read_bytes_allows_logical_symlink_path_with_target_outside_scope() {
use std::os::unix::fs::symlink;
let dir = TempDir::new().unwrap();
@@ -1646,15 +1646,7 @@ mod tests {
symlink(&target, &link).unwrap();
let fs = make_fs(&dir);
let err = fs.read_bytes(&link).unwrap_err();
assert!(
matches!(
err,
WorkdirError::SymlinkOutOfScope { ref path, target: ref err_target, required_permission: "read" }
if path == &link && err_target == &target.canonicalize().unwrap()
),
"expected symlink out-of-scope diagnostic, got {err:?}"
);
assert_eq!(fs.read_bytes(&link).unwrap(), b"secret");
}
#[cfg(unix)]
@@ -1746,7 +1738,7 @@ mod tests {
#[cfg(unix)]
#[test]
fn write_reports_symlink_target_outside_scope() {
fn write_allows_logical_symlink_path_with_target_outside_scope() {
use std::os::unix::fs::symlink;
let dir = TempDir::new().unwrap();
@@ -1757,14 +1749,13 @@ mod tests {
symlink(&target, &link).unwrap();
let fs = make_fs(&dir);
let err = fs.write(&link, b"new").unwrap_err();
fs.write(&link, b"new").unwrap();
assert_eq!(fs::read(&target).unwrap(), b"new");
assert!(
matches!(
err,
WorkdirError::SymlinkOutOfScope { ref path, target: ref err_target, required_permission: "write" }
if path == &link && err_target == &target.canonicalize().unwrap()
),
"expected write symlink out-of-scope diagnostic, got {err:?}"
fs::symlink_metadata(&link)
.unwrap()
.file_type()
.is_symlink()
);
}
@@ -1942,6 +1933,70 @@ mod tests {
));
}
#[cfg(unix)]
#[tokio::test]
async fn provider_uses_logical_paths_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 read = WorkdirSession::read(
&workdir,
ReadRequest {
path: WorkdirPath::new("yoi.local/worker.json").unwrap(),
offset: 0,
limit: 100,
max_bytes: 1024,
},
)
.await
.unwrap();
assert_eq!(read.bytes, b"scope-needle\n");
let glob = WorkdirSession::glob(
&workdir,
GlobRequest {
pattern: "**/*.json".into(),
path: WorkdirPath::new("yoi.local").unwrap(),
limit: 10,
},
)
.await
.unwrap();
assert_eq!(
glob.paths,
[WorkdirPath::new("yoi.local/worker.json").unwrap()]
);
let grep = WorkdirSession::grep(
&workdir,
GrepRequest {
pattern: "scope-needle".into(),
path: WorkdirPath::new("yoi.local").unwrap(),
glob: Some("*.json".into()),
file_type: None,
case_insensitive: false,
before_context: 0,
after_context: 0,
multiline: false,
output_mode: crate::GrepOutputMode::Content,
limit: 10,
offset: 0,
},
)
.await
.unwrap();
assert_eq!(grep.match_count, 1);
assert!(grep.output.contains("yoi.local/worker.json"));
assert!(
!workdir
.scope()
.is_readable(&outside.path().join("worker.json"))
);
}
#[tokio::test]
async fn provider_executes_glob_grep_and_command_at_the_materialization() {
let dir = TempDir::new().unwrap();
crates/workdir/src/scope.rs
+35 -75
View File
@@ -492,51 +492,9 @@ impl ScopedWorkdirSession {
}
}
async fn ensure_source_path_has_no_symlink(&self, path: &FsPath) -> Result<(), WorkdirError> {
let mut current = String::new();
for component in Path::new(path.as_str()).components() {
let component = component.as_os_str().to_string_lossy();
if component.is_empty() || component == "." {
continue;
}
if !current.is_empty() {
current.push('/');
}
current.push_str(&component);
let current = FsPath::new(&current).map_err(|error| {
WorkdirError::Denied(format!("invalid scoped Workdir path: {error}"))
})?;
match self.source.stat(StatRequest { path: current }).await {
Ok(result) if result.kind == fs_operation::EntryKind::Symlink => {
return Err(WorkdirError::Denied(format!(
"scoped Workdir path `{path}` traverses a symlink"
)));
}
Ok(_) => {}
Err(WorkdirError::NotFound(_)) => break,
Err(error) => return Err(error),
}
}
Ok(())
}
async fn ensure_scope_targets_do_not_traverse_symlinks(
&self,
rules: &[WorkdirToolScopeRule],
) -> Result<(), WorkdirError> {
for rule in rules {
self.ensure_source_path_has_no_symlink(&rule.target).await?;
}
Ok(())
}
async fn resolve_operation_path(&self, path: &FsPath) -> Result<FsPath, WorkdirError> {
fn resolve_operation_path(&self, path: &FsPath) -> Result<FsPath, WorkdirError> {
self.ensure_active()?;
let resolved = self.resolve_path(path)?;
if self.scope.is_some() {
self.ensure_source_path_has_no_symlink(&resolved).await?;
}
Ok(resolved)
self.resolve_path(path)
}
fn validate_scope(
@@ -624,8 +582,6 @@ impl ScopedWorkdirSession {
request.cwd
)));
}
self.ensure_scope_targets_do_not_traverse_symlinks(&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);
@@ -736,49 +692,49 @@ impl WorkdirSession for ScopedWorkdirSession {
}
async fn stat(&self, mut request: StatRequest) -> Result<StatResult, WorkdirError> {
let path = self.resolve_operation_path(&request.path).await?;
let path = self.resolve_operation_path(&request.path)?;
self.ensure_read(&path, WorkdirSessionCapability::Read)?;
request.path = path;
self.source.stat(request).await
}
async fn read(&self, mut request: ReadRequest) -> Result<ReadResult, WorkdirError> {
let path = self.resolve_operation_path(&request.path).await?;
let path = self.resolve_operation_path(&request.path)?;
self.ensure_read(&path, WorkdirSessionCapability::Read)?;
request.path = path;
self.source.read(request).await
}
async fn write(&self, mut request: WriteRequest) -> Result<WriteResult, WorkdirError> {
let path = self.resolve_operation_path(&request.path).await?;
let path = self.resolve_operation_path(&request.path)?;
self.ensure_write(&path, WorkdirSessionCapability::Write)?;
request.path = path;
self.source.write(request).await
}
async fn edit(&self, mut request: EditRequest) -> Result<EditResult, WorkdirError> {
let path = self.resolve_operation_path(&request.path).await?;
let path = self.resolve_operation_path(&request.path)?;
self.ensure_write(&path, WorkdirSessionCapability::Edit)?;
request.path = path;
self.source.edit(request).await
}
async fn list(&self, mut request: ListRequest) -> Result<ListResult, WorkdirError> {
let path = self.resolve_operation_path(&request.path).await?;
let path = self.resolve_operation_path(&request.path)?;
self.ensure_read(&path, WorkdirSessionCapability::Read)?;
request.path = path;
self.source.list(request).await
}
async fn glob(&self, mut request: GlobRequest) -> Result<GlobResult, WorkdirError> {
let path = self.resolve_operation_path(&request.path).await?;
let path = self.resolve_operation_path(&request.path)?;
self.ensure_read(&path, WorkdirSessionCapability::Glob)?;
request.path = path;
self.source.glob(request).await
}
async fn grep(&self, mut request: GrepRequest) -> Result<GrepResult, WorkdirError> {
let path = self.resolve_operation_path(&request.path).await?;
let path = self.resolve_operation_path(&request.path)?;
self.ensure_read(&path, WorkdirSessionCapability::Grep)?;
request.path = path;
self.source.grep(request).await
@@ -1487,7 +1443,7 @@ mod tests {
#[cfg(unix)]
#[tokio::test]
async fn provider_scope_denies_read_through_symlink_outside_grant() {
async fn provider_scope_allows_read_through_its_logical_symlink_path() {
use std::os::unix::fs::symlink;
let root = TempDir::new().unwrap();
@@ -1501,16 +1457,12 @@ mod tests {
.await
.unwrap();
let result = child.read(read("link")).await;
assert!(
result.is_err(),
"symlink read escaped provider scope: {result:?}"
);
assert_eq!(child.read(read("link")).await.unwrap().bytes, b"hidden");
}
#[cfg(unix)]
#[tokio::test]
async fn provider_scope_denies_write_through_symlink_outside_grant() {
async fn provider_scope_allows_write_through_its_logical_symlink_path() {
use std::os::unix::fs::symlink;
let root = TempDir::new().unwrap();
@@ -1523,17 +1475,19 @@ mod tests {
.await
.unwrap();
let result = child.write(write("outside/new", "forbidden")).await;
assert!(
result.is_err(),
"symlink write escaped provider scope: {result:?}"
child
.write(write("outside/new", "through-logical-path"))
.await
.unwrap();
assert_eq!(
fs::read_to_string(root.path().join("secret/new")).unwrap(),
"through-logical-path"
);
assert!(!root.path().join("secret/new").exists());
}
#[cfg(unix)]
#[tokio::test]
async fn write_delegation_rejects_symlink_target_before_lease() {
async fn write_delegation_leases_the_logical_symlink_path() {
use std::os::unix::fs::symlink;
let root = TempDir::new().unwrap();
@@ -1542,19 +1496,25 @@ mod tests {
symlink("../secret", root.path().join("granted/outside")).unwrap();
let parent = session(root.path());
assert!(matches!(
parent
.scope(request(
"granted/outside",
WorkdirToolScopePermission::Write
))
.await,
Err(WorkdirError::Denied(_))
));
let child = parent
.scope(request(
"granted/outside",
WorkdirToolScopePermission::Write,
))
.await
.unwrap();
child
.write(write("from-child", "child-authoritative"))
.await
.unwrap();
parent
.write(write("secret/parent", "still-authoritative"))
.await
.unwrap();
assert_eq!(
fs::read_to_string(root.path().join("secret/from-child")).unwrap(),
"child-authoritative"
);
}
#[tokio::test]