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

fix: close external workdir review gaps

Browse Source
This commit is contained in:
Hare
2026-09-22 18:57:47 +09:00
parent b5ee6b5804
commit 757432dac4
9 changed files with 1046 additions and 174 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/fs-operation/src/glob.rs
+50 -14
View File
@@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
use globset::Glob;
use ignore::WalkBuilder;
use crate::{FsAccessPolicy, FsError, FsPath, GlobRequest, GlobResult, resolve_access_path};
use crate::{FsAccessPolicy, FsError, FsPath, GlobRequest, GlobResult};
/// Execute a bounded glob entirely inside the provider process.
pub fn run_glob(
@@ -19,10 +19,12 @@ pub fn run_glob(
path: PathBuf::from(request.path.as_str()),
source,
})?;
let base_resolved = resolve_access_path(base).map_err(|error| FsError::Io {
path: PathBuf::from(request.path.as_str()),
source: error,
})?;
let base_resolved = access
.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())));
}
@@ -32,8 +34,33 @@ pub fn run_glob(
let mut matches = Vec::new();
let mut retained_path_bytes = 0_usize;
let mut provider_truncated = false;
let mut walker = WalkBuilder::new(base);
let traversal = access
.open_traversal_root(base, &base_resolved)
.map_err(|source| FsError::Io {
path: PathBuf::from(request.path.as_str()),
source,
})?;
let walker_root = traversal
.as_ref()
.map(|traversal| traversal.path())
.unwrap_or(base);
let mut walker = WalkBuilder::new(walker_root);
walker.hidden(false).follow_links(false);
if traversal.is_some() {
let search_relative = base.strip_prefix(root).map_err(|_| {
FsError::InvalidArgument("glob base is outside its provider root".to_string())
})?;
let search_relative = search_relative.to_path_buf();
let filter_root = walker_root.to_path_buf();
walker.filter_entry(move |entry| {
entry
.path()
.strip_prefix(&filter_root)
.is_ok_and(|relative| {
relative.starts_with(&search_relative) || search_relative.starts_with(relative)
})
});
}
let mut visited = 0_usize;
for entry in walker.build().flatten() {
access.check_cancelled().map_err(|source| FsError::Io {
@@ -47,19 +74,28 @@ pub fn run_glob(
crate::MAX_TRAVERSAL_ENTRIES
)));
}
let path = entry.path();
let readable = resolve_access_path(path)
.is_ok_and(|resolved| access.is_readable_paths(path, &resolved));
if !path.is_file() || !readable {
continue;
}
let Ok(file) = access.open_read_file(path, path) else {
let walked_path = entry.path();
let path = if traversal.is_some() {
let relative = walked_path.strip_prefix(walker_root).map_err(|_| {
FsError::InvalidArgument(
"descriptor traversal returned a path outside its root".to_string(),
)
})?;
root.join(relative)
} else {
walked_path.to_path_buf()
};
let resolved = match access.resolve_access_path(&path) {
Ok(resolved) if access.is_readable_paths(&path, &resolved) => resolved,
_ => continue,
};
let Ok(file) = access.open_read_file(&path, &resolved) else {
continue;
};
if !file.metadata().is_ok_and(|metadata| metadata.is_file()) {
continue;
}
let relative = path.strip_prefix(base).unwrap_or(path);
let relative = path.strip_prefix(base).unwrap_or(&path);
if !matcher.is_match(relative) {
continue;
}
crates/fs-operation/src/lib.rs
+48 -4
View File
@@ -110,7 +110,14 @@ pub fn open_beneath_no_symlinks_at(
) as RawFd
};
if fd < 0 {
return Err(std::io::Error::last_os_error());
let error = std::io::Error::last_os_error();
if error.raw_os_error() == Some(libc::ELOOP) {
return Err(std::io::Error::new(
std::io::ErrorKind::PermissionDenied,
"symbolic links are not permitted by this provider",
));
}
return Err(error);
}
// SAFETY: fd is a new owned descriptor returned by openat2.
Ok(unsafe { std::fs::File::from_raw_fd(fd) })
@@ -147,6 +154,26 @@ pub fn open_beneath_no_symlinks(_root: &Path, _path: &Path) -> std::io::Result<s
))
}
/// Keeps a descriptor-backed traversal path alive for the duration of a walk.
#[derive(Debug)]
pub struct FsTraversalRoot {
path: PathBuf,
_directory: std::fs::File,
}
impl FsTraversalRoot {
pub fn new(path: PathBuf, directory: std::fs::File) -> Self {
Self {
path,
_directory: directory,
}
}
pub fn path(&self) -> &Path {
&self.path
}
}
/// Provider-owned access policy used by local filesystem operations.
pub trait FsAccessPolicy: Send + Sync {
fn is_readable(&self, path: &Path) -> bool;
@@ -158,6 +185,23 @@ pub trait FsAccessPolicy: Send + Sync {
Ok(())
}
/// Resolve a logical path for authorization. Descriptor-confined providers
/// may retain the lexical path because the subsequent open is the authority.
fn resolve_access_path(&self, logical: &Path) -> std::io::Result<PathBuf> {
resolve_access_path(logical)
}
/// Return a descriptor-backed traversal root when pathname walking would
/// violate provider confinement. The retained directory keeps procfs-style
/// descriptor paths valid for the complete walk.
fn open_traversal_root(
&self,
_logical: &Path,
_resolved: &Path,
) -> std::io::Result<Option<FsTraversalRoot>> {
Ok(None)
}
/// Open an already-authorized readable file. Capability providers override
/// this to bind path resolution and open into one root-confined operation.
fn open_read_file(&self, _logical: &Path, resolved: &Path) -> std::io::Result<std::fs::File> {
@@ -168,10 +212,10 @@ pub trait FsAccessPolicy: Send + Sync {
/// override this to prevent a path swap between authorization and stat.
fn read_metadata(
&self,
logical: &Path,
_resolved: &Path,
_logical: &Path,
resolved: &Path,
) -> std::io::Result<std::fs::Metadata> {
std::fs::symlink_metadata(logical)
std::fs::metadata(resolved)
}
/// Open an already-authorized directory for bounded enumeration.
crates/fs-operation/src/local.rs
+8 -4
View File
@@ -24,7 +24,9 @@ pub fn run_stat(
.check_cancelled()
.map_err(|error| map_io(&logical, error))?;
let path = resolve(root, &logical)?;
let resolved = resolve_access_path(&path).map_err(|error| map_io(&logical, error))?;
let resolved = access
.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())));
}
@@ -308,7 +310,7 @@ pub fn run_list(
}
let entry = entry.map_err(|error| map_io(&logical, error))?;
let logical_absolute = logical_base.join(entry.file_name());
let resolved = match resolve_access_path(&logical_absolute) {
let resolved = match access.resolve_access_path(&logical_absolute) {
Ok(resolved) => resolved,
Err(_) => continue,
};
@@ -380,7 +382,10 @@ fn require_access(
write: bool,
allow_symlink_directory: bool,
) -> Result<PathBuf, FsError> {
let symlink = direct_symlink(path);
let resolved = access
.resolve_access_path(path)
.map_err(|error| map_io(logical, error))?;
let symlink = (resolved != path).then(|| direct_symlink(path)).flatten();
if let Some(info) = symlink.as_ref()
&& !info.target_exists
{
@@ -390,7 +395,6 @@ fn require_access(
target: PathBuf::from("<provider-internal target>"),
});
}
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_paths(path, &resolved)
crates/fs-operation/src/search.rs
+252 -36
View File
@@ -1,6 +1,6 @@
use std::collections::BTreeMap;
use std::collections::{BTreeMap, HashSet};
use std::fmt::Write as _;
use std::io::Read;
use std::io::{BufRead, BufReader, Read};
use std::path::{Path, PathBuf};
use crate::FsAccessPolicy;
@@ -8,12 +8,11 @@ use grep_regex::RegexMatcherBuilder;
use grep_searcher::sinks::UTF8 as UTF8Sink;
use grep_searcher::{BinaryDetection, Searcher, SearcherBuilder, Sink, SinkContext, SinkMatch};
use ignore::WalkBuilder;
use ignore::gitignore::{Gitignore, GitignoreBuilder};
use ignore::overrides::{Override, OverrideBuilder};
use ignore::types::{Types, TypesBuilder};
use crate::{
FsError, GrepOutputMode, GrepRequest, GrepResult, direct_symlink, resolve_access_path,
};
use crate::{FsError, GrepOutputMode, GrepRequest, GrepResult, direct_symlink};
struct SourceBoundedReader<'a, R> {
inner: R,
@@ -208,6 +207,118 @@ fn direct_file_selected(path: &Path, overrides: Option<&Override>, types: Option
&& !types.is_some_and(|filter| filter.matched(path, false).is_ignore())
}
#[derive(Default)]
struct DescriptorIgnoreMatchers {
loaded_directories: HashSet<PathBuf>,
by_directory: BTreeMap<PathBuf, Gitignore>,
}
impl DescriptorIgnoreMatchers {
fn load_directory(
&mut self,
directory: &Path,
explicit_base: &Path,
source_bytes_remaining: &mut u64,
access: &dyn FsAccessPolicy,
) -> Result<(), FsError> {
if !self.loaded_directories.insert(directory.to_path_buf()) {
return Ok(());
}
let mut builder = GitignoreBuilder::new(directory);
let mut has_patterns = false;
for name in [".gitignore", ".ignore"] {
let path = directory.join(name);
let resolved = access
.resolve_access_path(&path)
.map_err(|error| FsError::io(&path, error))?;
if !access.is_readable_paths(&path, &resolved) {
continue;
}
let file = match access.open_read_file(&path, &resolved) {
Ok(file) => file,
Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue,
Err(error) => return Err(FsError::io(&path, error)),
};
if !file
.metadata()
.map_err(|error| FsError::io(&path, error))?
.is_file()
{
continue;
}
let bounded = SourceBoundedReader {
inner: file,
remaining: source_bytes_remaining,
access,
};
let mut reader = BufReader::new(bounded);
let mut line = String::new();
let mut line_number = 0_u64;
loop {
line.clear();
let read = reader
.read_line(&mut line)
.map_err(|error| FsError::io(&path, error))?;
if read == 0 {
break;
}
line_number = line_number.saturating_add(1);
if line.len() > 1024 * 1024 {
return Err(FsError::InvalidArgument(format!(
"ignore pattern line {line_number} exceeds provider limit"
)));
}
let line = line.trim_end_matches(['\r', '\n']);
// Match the ignore crate's partial-error behavior: one invalid
// pattern does not discard the remaining valid lines.
if builder.add_line(Some(path.clone()), line).is_ok() {
has_patterns = true;
}
}
}
if !has_patterns {
return Ok(());
}
let matcher = builder.build().map_err(|_| {
FsError::InvalidArgument("ignore patterns could not be compiled".to_string())
})?;
// An explicitly selected search root is traversed even when an ancestor
// ignores that directory, matching path-backed Grep behavior.
if directory != explicit_base
&& matcher
.matched_path_or_any_parents(explicit_base, true)
.is_ignore()
{
return Ok(());
}
self.by_directory.insert(directory.to_path_buf(), matcher);
Ok(())
}
fn is_ignored(&self, root: &Path, path: &Path) -> bool {
let mut directories = path
.parent()
.into_iter()
.flat_map(Path::ancestors)
.take_while(|directory| directory.starts_with(root))
.collect::<Vec<_>>();
directories.reverse();
let mut ignored = false;
for directory in directories {
let Some(matcher) = self.by_directory.get(directory) else {
continue;
};
let matched = matcher.matched_path_or_any_parents(path, false);
if matched.is_ignore() {
ignored = true;
} else if matched.is_whitelist() {
ignored = false;
}
}
ignored
}
}
struct GrepParams {
pattern: String,
path: Option<PathBuf>,
@@ -272,7 +383,22 @@ pub fn run_grep(
if !base.is_absolute() {
return Err(FsError::RelativePath(base));
}
let symlink = direct_symlink(&base);
let resolved_base = match access.resolve_access_path(&base) {
Ok(resolved) => resolved,
Err(error) => {
if let Some(info) = direct_symlink(&base).filter(|info| !info.target_exists) {
return Err(FsError::BrokenSymlink {
path: base.clone(),
link: info.link_path,
target: info.resolved_path,
});
}
return Err(FsError::io(&base, error));
}
};
let symlink = (resolved_base != base)
.then(|| direct_symlink(&base))
.flatten();
if let Some(info) = symlink.as_ref()
&& !info.target_exists
{
@@ -282,14 +408,14 @@ pub fn run_grep(
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()
.and_then(|parent| {
resolve_access_path(parent)
access
.resolve_access_path(parent)
.ok()
.map(|resolved| access.is_readable_paths(parent, &resolved))
})
@@ -307,10 +433,12 @@ pub fn run_grep(
FsError::OutOfScope(base.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),
})?;
let base_meta = access
.read_metadata(&base, &resolved_base)
.map_err(|e| match e.kind() {
std::io::ErrorKind::NotFound => FsError::NotFound(base.clone()),
_ => FsError::io(&base, e),
})?;
if !base_meta.is_file() && !base_meta.is_dir() {
return Err(FsError::InvalidArgument(format!(
"grep search path must be a regular file or directory: {}",
@@ -358,24 +486,82 @@ pub fn run_grep(
return Ok(report.into_result(root));
}
let mut walker = WalkBuilder::new(&base);
walker
.hidden(true)
.git_ignore(true)
.git_global(true)
.git_exclude(true)
.ignore(true)
.parents(true)
.follow_links(false);
if let Some(types) = types {
walker.types(types);
}
if let Some(overrides) = overrides {
walker.overrides(overrides);
let traversal = access
.open_traversal_root(&base, &resolved_base)
.map_err(|error| FsError::io(&base, error))?;
let walker_root = traversal
.as_ref()
.map(|traversal| traversal.path())
.unwrap_or(&base);
let mut walker = WalkBuilder::new(walker_root);
if traversal.is_some() {
walker
.hidden(false)
// Path-backed ignore discovery may follow an ignore-file symlink.
// Descriptor providers load ignore files through `access` below.
.git_ignore(false)
.git_global(false)
.git_exclude(false)
.ignore(false)
.parents(false)
.follow_links(false);
let search_relative = base.strip_prefix(root).map_err(|_| {
FsError::InvalidArgument("grep base is outside its provider root".to_string())
})?;
let search_relative = search_relative.to_path_buf();
let filter_root = walker_root.to_path_buf();
walker.filter_entry(move |entry| {
entry
.path()
.strip_prefix(&filter_root)
.is_ok_and(|relative| {
relative.starts_with(&search_relative) || search_relative.starts_with(relative)
})
});
} else {
walker
.hidden(true)
.git_ignore(true)
.git_global(true)
.git_exclude(true)
.ignore(true)
.parents(true)
.follow_links(false);
if let Some(types) = types.clone() {
walker.types(types);
}
if let Some(overrides) = overrides.clone() {
walker.overrides(overrides);
}
}
let mut visited = 0_usize;
let mut source_bytes_remaining = crate::MAX_GREP_SOURCE_BYTES;
let mut descriptor_ignores = DescriptorIgnoreMatchers::default();
if traversal.is_some() {
let mut directory = root.to_path_buf();
descriptor_ignores.load_directory(
&directory,
&base,
&mut source_bytes_remaining,
access,
)?;
for component in base
.strip_prefix(root)
.map_err(|_| {
FsError::InvalidArgument("grep base is outside its provider root".to_string())
})?
.components()
{
directory.push(component);
descriptor_ignores.load_directory(
&directory,
&base,
&mut source_bytes_remaining,
access,
)?;
}
}
for entry in walker.build().flatten() {
access
.check_cancelled()
@@ -387,23 +573,50 @@ pub fn run_grep(
crate::MAX_TRAVERSAL_ENTRIES
)));
}
if !entry
.file_type()
.map(|kind| kind.is_file())
.unwrap_or(false)
{
let walked_path = entry.path();
let path = if traversal.is_some() {
let relative = walked_path.strip_prefix(walker_root).map_err(|_| {
FsError::InvalidArgument(
"descriptor traversal returned a path outside its root".to_string(),
)
})?;
root.join(relative)
} else {
walked_path.to_path_buf()
};
let file_type = entry.file_type();
if traversal.is_some() && file_type.as_ref().is_some_and(|kind| kind.is_dir()) {
descriptor_ignores.load_directory(&path, &base, &mut source_bytes_remaining, access)?;
continue;
}
let path = entry.path();
let readable = resolve_access_path(path)
.is_ok_and(|resolved| access.is_readable_paths(path, &resolved));
if !file_type.map(|kind| kind.is_file()).unwrap_or(false) {
continue;
}
if traversal.is_some() {
let relative = path.strip_prefix(&base).map_err(|_| {
FsError::InvalidArgument(
"descriptor traversal returned a path outside its search base".to_string(),
)
})?;
if relative
.components()
.any(|component| component.as_os_str().to_string_lossy().starts_with('.'))
|| descriptor_ignores.is_ignored(root, &path)
|| !direct_file_selected(&path, overrides.as_ref(), types.as_ref())
{
continue;
}
}
let readable = access
.resolve_access_path(&path)
.is_ok_and(|resolved| access.is_readable_paths(&path, &resolved));
if !readable {
continue;
}
if scan_path(
&mut searcher,
&matcher,
path,
&path,
mode,
&mut report,
&mut matching_files_seen,
@@ -434,8 +647,11 @@ fn scan_path(
source_bytes_remaining: &mut u64,
access: &dyn FsAccessPolicy,
) -> Result<bool, FsError> {
let resolved = access
.resolve_access_path(path)
.map_err(|error| FsError::io(path, error))?;
let reader = access
.open_read_file(path, path)
.open_read_file(path, &resolved)
.map_err(|error| FsError::io(path, error))?;
let metadata = reader
.metadata()
crates/workdir/src/external_local_tests.rs
+182 -11
View File
@@ -2,9 +2,10 @@ use crate::http::{
WorkdirSessionOperation, WorkdirSessionOperationResult, dispatch_workdir_session_operation,
};
use crate::{
BoundedReadLimits, CommandRequest, EditRequest, ExternalWorkdirRoot, LocalWorkdirSession,
ReadRequest, Workdir, WorkdirError, WorkdirPath, WorkdirSession, WorkdirSessionCapabilities,
WorkdirSessionCapability, WriteRequest,
BoundedReadLimits, CommandRequest, EditRequest, ExternalWorkdirRoot, GlobRequest,
GrepOutputMode, GrepRequest, LocalWorkdirSession, ReadRequest, Workdir, WorkdirError,
WorkdirPath, WorkdirSession, WorkdirSessionCapabilities, WorkdirSessionCapability,
WriteRequest,
};
fn external_session(root: &tempfile::TempDir, limits: BoundedReadLimits) -> LocalWorkdirSession {
@@ -206,10 +207,13 @@ async fn external_local_provider_rejects_symlink_roots_and_traversal() {
)
.await
.unwrap_err();
assert!(matches!(
error,
WorkdirError::SymlinkOutOfScope { .. } | WorkdirError::OutOfScope(_)
));
assert!(
matches!(
error,
WorkdirError::SymlinkOutOfScope { .. } | WorkdirError::OutOfScope(_)
),
"unexpected error for {path}: {error:?}"
);
}
let parent = tempfile::tempdir().unwrap();
@@ -224,20 +228,103 @@ async fn external_local_provider_rejects_symlink_roots_and_traversal() {
assert!(matches!(error, WorkdirError::Denied(_)));
}
#[cfg(target_os = "linux")]
#[tokio::test]
async fn external_grep_honors_gitignore_above_a_nested_search_root() {
let root = tempfile::tempdir().unwrap();
std::fs::create_dir(root.path().join("nested")).unwrap();
std::fs::write(root.path().join(".gitignore"), "nested/ignored.txt\n").unwrap();
std::fs::write(root.path().join("nested/ignored.txt"), "needle ignored\n").unwrap();
std::fs::write(root.path().join("nested/visible.txt"), "needle visible\n").unwrap();
let session = external_session(&root, BoundedReadLimits::new(4096, 1024).unwrap());
let result = WorkdirSession::grep(
&session,
GrepRequest {
pattern: "needle".to_string(),
path: WorkdirPath::new("nested").unwrap(),
glob: Some("*.txt".to_string()),
file_type: None,
case_insensitive: false,
before_context: 0,
after_context: 0,
multiline: false,
output_mode: GrepOutputMode::FilesWithMatches,
limit: 10,
offset: 0,
},
)
.await
.unwrap();
assert!(result.output.contains("nested/visible.txt"));
assert!(!result.output.contains("nested/ignored.txt"));
std::fs::write(root.path().join(".gitignore"), "nested/\n").unwrap();
let explicit_ignored_root = WorkdirSession::grep(
&session,
GrepRequest {
pattern: "needle".to_string(),
path: WorkdirPath::new("nested").unwrap(),
glob: Some("*.txt".to_string()),
file_type: None,
case_insensitive: false,
before_context: 0,
after_context: 0,
multiline: false,
output_mode: GrepOutputMode::FilesWithMatches,
limit: 10,
offset: 0,
},
)
.await
.unwrap();
assert!(explicit_ignored_root.output.contains("nested/visible.txt"));
assert!(explicit_ignored_root.output.contains("nested/ignored.txt"));
let outside = tempfile::tempdir().unwrap();
std::fs::write(outside.path().join("ignore-rules"), "nested/visible.txt\n").unwrap();
std::fs::remove_file(root.path().join(".gitignore")).unwrap();
std::os::unix::fs::symlink(
outside.path().join("ignore-rules"),
root.path().join(".gitignore"),
)
.unwrap();
let symlink_error = WorkdirSession::grep(
&session,
GrepRequest {
pattern: "needle".to_string(),
path: WorkdirPath::new("nested").unwrap(),
glob: Some("*.txt".to_string()),
file_type: None,
case_insensitive: false,
before_context: 0,
after_context: 0,
multiline: false,
output_mode: GrepOutputMode::FilesWithMatches,
limit: 10,
offset: 0,
},
)
.await
.unwrap_err();
assert!(matches!(symlink_error, WorkdirError::OutOfScope(_)));
}
#[cfg(target_os = "linux")]
#[tokio::test]
async fn external_local_provider_keeps_pre_grant_root_when_path_changes_before_session_creation() {
let parent = tempfile::tempdir().unwrap();
let approved = parent.path().join("shared");
std::fs::create_dir(&approved).unwrap();
std::fs::write(approved.join("item.txt"), "approved").unwrap();
std::fs::write(approved.join("approved.txt"), "approved needle").unwrap();
// This is the CLI's approval/open boundary. Remote grant creation happens
// only after this descriptor has been pinned.
let pinned = ExternalWorkdirRoot::pin(&approved).unwrap();
std::fs::rename(&approved, parent.path().join("approved-original")).unwrap();
std::fs::create_dir(&approved).unwrap();
std::fs::write(approved.join("item.txt"), "replacement").unwrap();
std::fs::write(approved.join("replacement.txt"), "replacement needle").unwrap();
let session = LocalWorkdirSession::external_read_only_pinned(
Workdir::new("external-workdir-pre-grant-pin"),
@@ -248,7 +335,7 @@ async fn external_local_provider_keeps_pre_grant_root_when_path_changes_before_s
let read = WorkdirSession::read(
&session,
ReadRequest {
path: WorkdirPath::new("item.txt").unwrap(),
path: WorkdirPath::new("approved.txt").unwrap(),
offset: 0,
limit: 10,
max_bytes: 1024,
@@ -256,7 +343,91 @@ async fn external_local_provider_keeps_pre_grant_root_when_path_changes_before_s
)
.await
.unwrap();
assert_eq!(read.bytes, b"approved");
assert_eq!(read.bytes, b"approved needle");
let glob = WorkdirSession::glob(
&session,
GlobRequest {
pattern: "*.txt".to_string(),
path: WorkdirPath::new("").unwrap(),
limit: 10,
},
)
.await
.unwrap();
assert_eq!(
glob.paths
.iter()
.map(|path| path.as_str())
.collect::<Vec<_>>(),
vec!["approved.txt"]
);
let grep = WorkdirSession::grep(
&session,
GrepRequest {
pattern: "needle".to_string(),
path: WorkdirPath::new("").unwrap(),
glob: Some("*.txt".to_string()),
file_type: None,
case_insensitive: false,
before_context: 0,
after_context: 0,
multiline: false,
output_mode: GrepOutputMode::FilesWithMatches,
limit: 10,
offset: 0,
},
)
.await
.unwrap();
assert!(grep.output.contains("approved.txt"));
assert!(!grep.output.contains("replacement.txt"));
let outside = tempfile::tempdir().unwrap();
std::fs::write(outside.path().join("secret.txt"), "secret needle").unwrap();
std::fs::remove_file(approved.join("replacement.txt")).unwrap();
std::fs::remove_dir(&approved).unwrap();
std::os::unix::fs::symlink(outside.path(), &approved).unwrap();
let glob_after_symlink_swap = WorkdirSession::glob(
&session,
GlobRequest {
pattern: "*.txt".to_string(),
path: WorkdirPath::new("").unwrap(),
limit: 10,
},
)
.await
.unwrap();
assert_eq!(
glob_after_symlink_swap
.paths
.iter()
.map(|path| path.as_str())
.collect::<Vec<_>>(),
vec!["approved.txt"]
);
let grep_after_symlink_swap = WorkdirSession::grep(
&session,
GrepRequest {
pattern: "needle".to_string(),
path: WorkdirPath::new("").unwrap(),
glob: Some("*.txt".to_string()),
file_type: None,
case_insensitive: false,
before_context: 0,
after_context: 0,
multiline: false,
output_mode: GrepOutputMode::FilesWithMatches,
limit: 10,
offset: 0,
},
)
.await
.unwrap();
assert!(grep_after_symlink_swap.output.contains("approved.txt"));
assert!(!grep_after_symlink_swap.output.contains("secret.txt"));
}
#[cfg(target_os = "linux")]
crates/workdir/src/local.rs
+60 -5
View File
@@ -246,9 +246,7 @@ impl ScopeAccess {
fn path_is_confined(&self, logical: &Path, resolved: &Path) -> bool {
!self.reject_symlinks
|| (logical.starts_with(&self.root)
&& resolved.starts_with(&self.root)
&& fs_operation::first_symlink(logical).is_none())
|| (logical.starts_with(&self.root) && resolved.starts_with(&self.root))
}
}
@@ -281,6 +279,54 @@ impl fs_operation::FsAccessPolicy for ScopeAccess {
Ok(())
}
fn resolve_access_path(&self, logical: &Path) -> std::io::Result<PathBuf> {
if !self.reject_symlinks {
return fs_operation::resolve_access_path(logical);
}
if logical.starts_with(&self.root) {
Ok(logical.to_path_buf())
} else {
Err(std::io::Error::new(
std::io::ErrorKind::PermissionDenied,
"path is outside provider root",
))
}
}
fn open_traversal_root(
&self,
_logical: &Path,
resolved: &Path,
) -> std::io::Result<Option<fs_operation::FsTraversalRoot>> {
if !self.reject_symlinks {
return Ok(None);
}
#[cfg(target_os = "linux")]
{
use std::os::fd::AsRawFd;
let directory = self.open_confined(resolved)?;
if !directory.metadata()?.is_dir() {
return Err(std::io::Error::new(
std::io::ErrorKind::NotADirectory,
"path is not a directory",
));
}
let root = self
.pinned_root
.as_ref()
.expect("confined traversal requires a pinned root")
.try_clone()?;
let path = PathBuf::from(format!("/proc/self/fd/{}", root.as_raw_fd()));
return Ok(Some(fs_operation::FsTraversalRoot::new(path, root)));
}
#[cfg(not(target_os = "linux"))]
Err(std::io::Error::new(
std::io::ErrorKind::Unsupported,
"descriptor-rooted traversal is unavailable on this platform",
))
}
fn open_read_file(&self, _logical: &Path, resolved: &Path) -> std::io::Result<std::fs::File> {
if self.reject_symlinks {
self.open_confined(resolved)
@@ -289,11 +335,15 @@ impl fs_operation::FsAccessPolicy for ScopeAccess {
}
}
fn read_metadata(&self, logical: &Path, resolved: &Path) -> std::io::Result<std::fs::Metadata> {
fn read_metadata(
&self,
_logical: &Path,
resolved: &Path,
) -> std::io::Result<std::fs::Metadata> {
if self.reject_symlinks {
self.open_confined(resolved)?.metadata()
} else {
std::fs::symlink_metadata(logical)
std::fs::metadata(resolved)
}
}
@@ -1367,6 +1417,11 @@ fn sanitize_error(error: WorkdirError, logical: &WorkdirPath) -> WorkdirError {
WorkdirError::ReadOnly(_) => WorkdirError::ReadOnly(path),
WorkdirError::IsDirectory(_) => WorkdirError::IsDirectory(path),
WorkdirError::NotFound(_) => WorkdirError::NotFound(path),
WorkdirError::Io { source, .. }
if source.kind() == std::io::ErrorKind::PermissionDenied =>
{
WorkdirError::OutOfScope(path)
}
WorkdirError::Io { source, .. } => WorkdirError::Unavailable(format!(
"I/O operation failed for {logical}: {}",
source.kind()
crates/workspace-server/src/server.rs
+392 -99
View File
@@ -723,6 +723,7 @@ struct ExternalProviderConnection {
}
struct ExternalWorkdirExpiryTask {
task_id: Uuid,
handle: tokio::task::JoinHandle<()>,
}
@@ -8991,7 +8992,7 @@ async fn open_current_worker_workdir_session_locked(
})?;
if let WorkdirRegistrySource::ExternalGrant { grant_id } = &workdir.source {
close_current_worker_attachment_session_locked(api, worker).await?;
let grant = api
let mut grant = api
.store
.get_external_workdir_grant(&api.config.workspace_id, grant_id)?
.ok_or_else(|| Error::RuntimeOperationFailed {
@@ -9003,45 +9004,15 @@ async fn open_current_worker_workdir_session_locked(
.map(|expires_at| expires_at.with_timezone(&Utc) <= Utc::now())
.unwrap_or(true);
if expired {
let updated_at = Utc::now().to_rfc3339_opts(SecondsFormat::Nanos, true);
api.store.update_external_workdir_grant_state(
&api.config.workspace_id,
grant_id,
&grant.provider_instance_id,
grant.generation,
"expired",
&updated_at,
)?;
grant = fence_external_workdir_grant_expired(api, grant_id)
.await
.map_err(|error| error.error)?;
}
if matches!(grant.status.as_str(), "revoked" | "expired") {
cleanup_external_workdir_attachments_with_lock(api, &grant.workdir_id, Some(worker))
.await
.map_err(|error| error.error)?;
cancel_external_workdir_expiry(api, grant_id);
let connection = {
api.external_workdir_providers
.lock()
.expect("External Workdir provider registry poisoned")
.disconnect(grant_id, grant.generation)
};
if let Some(connection) = connection {
let _ = connection
.sender
.send(ExternalProviderCommand::Revoke {
reason: workdir::external::ExternalWorkdirRevokeReason::Expired,
})
.await;
}
close_current_worker_command_sessions_for_alias_locked(api, worker, &link.alias)
.await?;
api.store.detach_worker_workdir(
&api.config.workspace_id,
worker,
Some(&link.workdir_id),
&updated_at,
)?;
sync_runtime_worker_workdir_attachments(api, worker).map_err(|error| error.error)?;
api.worker_projection.refresh(worker)?;
return Err(Error::RuntimeOperationFailed {
runtime_id: "workspace-backend".to_string(),
code: "external_workdir_provider_expired".to_string(),
message: "External Workdir grant has expired".to_string(),
});
}
if grant.status != "online" {
return Err(Error::RuntimeOperationFailed {
@@ -11950,22 +11921,30 @@ fn schedule_external_workdir_expiry(
let grant_id = grant.grant_id.clone();
let task_api = api.clone();
let task_grant_id = grant_id.clone();
let task_id = Uuid::now_v7();
let task_instance_id = task_id;
let (start_sender, start_receiver) = tokio::sync::oneshot::channel();
let handle = tokio::spawn(async move {
if start_receiver.await.is_err() {
return;
}
let delay = (expires_at - Utc::now()).to_std().unwrap_or_default();
tokio::time::sleep(delay).await;
loop {
match expire_external_workdir_grant(&task_api, &task_grant_id).await {
Ok(()) => break,
Ok(true) => break,
Ok(false) => {
// Wall time may have moved backwards after the monotonic
// sleep was calculated. Keep ownership and recheck without
// detaching an active grant.
tokio::time::sleep(std::time::Duration::from_secs(1)).await;
}
Err(error) => {
tracing::error!(grant_id = %task_grant_id, error = ?error, "Backend External Workdir expiry cleanup requires retry");
let retryable = task_api
.store
.get_external_workdir_grant(&task_api.config.workspace_id, &task_grant_id)
.ok()
.flatten()
.is_some_and(|grant| {
matches!(grant.status.as_str(), "revoked" | "expired")
});
.map_or(true, |grant| grant.is_some());
if !retryable {
break;
}
@@ -11973,14 +11952,26 @@ fn schedule_external_workdir_expiry(
}
}
}
let mut tasks = task_api
.external_workdir_expiry_tasks
.lock()
.expect("External Workdir expiry task registry poisoned");
if tasks
.get(&task_grant_id)
.is_some_and(|task| task.task_id == task_instance_id)
{
tasks.remove(&task_grant_id);
}
});
let mut tasks = api
.external_workdir_expiry_tasks
.lock()
.expect("External Workdir expiry task registry poisoned");
if let Some(existing) = tasks.insert(grant_id, ExternalWorkdirExpiryTask { handle }) {
if let Some(existing) = tasks.insert(grant_id, ExternalWorkdirExpiryTask { task_id, handle }) {
existing.handle.abort();
}
drop(tasks);
let _ = start_sender.send(());
Ok(())
}
@@ -11995,20 +11986,23 @@ fn cancel_external_workdir_expiry(api: &WorkspaceApi, grant_id: &str) {
}
}
async fn expire_external_workdir_grant(api: &WorkspaceApi, grant_id: &str) -> ApiResult<()> {
async fn fence_external_workdir_grant_expired(
api: &WorkspaceApi,
grant_id: &str,
) -> ApiResult<ExternalWorkdirGrantRecord> {
loop {
let grant = api
let mut grant = api
.store
.get_external_workdir_grant(&api.config.workspace_id, grant_id)?
.ok_or_else(|| Error::InvalidInput("Unknown External Workdir grant".to_string()))?;
if matches!(grant.status.as_str(), "revoked" | "expired") {
break;
return Ok(grant);
}
let expires_at = chrono::DateTime::parse_from_rfc3339(&grant.expires_at)
.map(|value| value.with_timezone(&Utc))
.map_err(|_| Error::Store("External Workdir grant has invalid expiry".to_string()))?;
if expires_at > Utc::now() {
return Ok(());
return Ok(grant);
}
let updated_at = Utc::now().to_rfc3339_opts(SecondsFormat::Nanos, true);
if !api.store.update_external_workdir_grant_state(
@@ -12050,20 +12044,19 @@ async fn expire_external_workdir_grant(api: &WorkspaceApi, grant_id: &str) -> Ap
),
&updated_at,
)?;
break;
grant.status = "expired".to_string();
grant.updated_at = updated_at;
return Ok(grant);
}
cleanup_external_workdir_attachments_for_grant(api, grant_id).await
}
async fn cleanup_external_workdir_attachments_for_grant(
api: &WorkspaceApi,
grant_id: &str,
) -> ApiResult<()> {
let grant = api
.store
.get_external_workdir_grant(&api.config.workspace_id, grant_id)?
.ok_or_else(|| Error::InvalidInput("Unknown External Workdir grant".to_string()))?;
cleanup_external_workdir_attachments(api, &grant.workdir_id).await
async fn expire_external_workdir_grant(api: &WorkspaceApi, grant_id: &str) -> ApiResult<bool> {
let grant = fence_external_workdir_grant_expired(api, grant_id).await?;
if !matches!(grant.status.as_str(), "revoked" | "expired") {
return Ok(false);
}
cleanup_external_workdir_attachments(api, &grant.workdir_id).await?;
Ok(true)
}
async fn scoped_create_external_workdir_grant(
@@ -12166,7 +12159,7 @@ async fn scoped_external_workdir_provider_ws(
ws: WebSocketUpgrade,
) -> ApiResult<Response> {
validate_workspace_scope(&api, &workspace_id)?;
let grant = api
let mut grant = api
.store
.get_external_workdir_grant(&workspace_id, &grant_id)?
.ok_or_else(|| Error::InvalidInput("Unknown External Workdir grant".to_string()))?;
@@ -12180,17 +12173,14 @@ async fn scoped_external_workdir_provider_ws(
.map(|expires_at| expires_at.with_timezone(&Utc) <= now)
.unwrap_or(true);
if expired {
api.store.update_external_workdir_grant_state(
&workspace_id,
&grant_id,
&grant.provider_instance_id,
grant.generation,
"expired",
&now.to_rfc3339_opts(SecondsFormat::Nanos, true),
)?;
cancel_external_workdir_expiry(&api, &grant_id);
grant = fence_external_workdir_grant_expired(&api, &grant_id).await?;
}
if matches!(grant.status.as_str(), "revoked" | "expired") {
cleanup_external_workdir_attachments(&api, &grant.workdir_id).await?;
return Err(Error::InvalidInput("External Workdir grant has expired".to_string()).into());
cancel_external_workdir_expiry(&api, &grant_id);
return Err(
Error::InvalidInput(format!("External Workdir grant is {}", grant.status)).into(),
);
}
Ok(ws
.max_message_size(workdir::external::MAX_EXTERNAL_PROVIDER_FRAME_BYTES)
@@ -12551,24 +12541,13 @@ async fn scoped_get_external_workdir_grant(
.map(|expires_at| expires_at.with_timezone(&Utc) <= now)
.unwrap_or(true);
if expired {
let updated_at = now.to_rfc3339_opts(SecondsFormat::Nanos, true);
if api.store.update_external_workdir_grant_state(
&workspace_id,
&grant_id,
&grant.provider_instance_id,
grant.generation,
"expired",
&updated_at,
)? {
grant.status = "expired".to_string();
grant.updated_at = updated_at;
}
cancel_external_workdir_expiry(&api, &grant_id);
grant = fence_external_workdir_grant_expired(&api, &grant_id).await?;
}
// Terminal cleanup is retryable: a prior expiry/revoke may have committed
// its fence before Runtime/session projection completed.
if matches!(grant.status.as_str(), "expired" | "revoked") {
cleanup_external_workdir_attachments(&api, &grant.workdir_id).await?;
cancel_external_workdir_expiry(&api, &grant_id);
}
Ok(Json(external_workdir_grant_response(&grant)))
}
@@ -12615,7 +12594,6 @@ async fn scoped_revoke_external_workdir_grant(
}
grant.status = "revoked".to_string();
grant.updated_at = updated_at;
cancel_external_workdir_expiry(&api, &grant_id);
api.store.record_external_workdir_audit(
&workspace_id,
"account",
@@ -12634,12 +12612,21 @@ async fn scoped_revoke_external_workdir_grant(
// Cleanup is deliberately retried for terminal grants. A prior revoke may
// have committed its fence before a Runtime/session projection failure.
cleanup_external_workdir_attachments(&api, &grant.workdir_id).await?;
cancel_external_workdir_expiry(&api, &grant_id);
Ok(Json(external_workdir_grant_response(&grant)))
}
async fn cleanup_external_workdir_attachments(
api: &WorkspaceApi,
workdir_id: &str,
) -> ApiResult<()> {
cleanup_external_workdir_attachments_with_lock(api, workdir_id, None).await
}
async fn cleanup_external_workdir_attachments_with_lock(
api: &WorkspaceApi,
workdir_id: &str,
already_locked_worker: Option<&RuntimeWorkerRef>,
) -> ApiResult<()> {
let workdir = api
.store
@@ -12657,7 +12644,7 @@ async fn cleanup_external_workdir_attachments(
&now,
)?;
match release_external_workdir_attachments(api, workdir_id).await {
match release_external_workdir_attachments(api, workdir_id, already_locked_worker).await {
Ok(()) => {
let completed_at = Utc::now().to_rfc3339_opts(SecondsFormat::Nanos, true);
api.store.update_external_workdir_cleanup_state(
@@ -12705,16 +12692,66 @@ async fn cleanup_external_workdir_attachments(
}
}
async fn release_external_workdir_attachments(
#[cfg(test)]
fn external_workdir_cleanup_failure_slot() -> &'static Mutex<Option<String>> {
use std::sync::OnceLock;
static INJECTED_WORKDIR: OnceLock<Mutex<Option<String>>> = OnceLock::new();
INJECTED_WORKDIR.get_or_init(|| Mutex::new(None))
}
#[cfg(test)]
fn inject_external_workdir_cleanup_failure(workdir_id: &str) {
*external_workdir_cleanup_failure_slot()
.lock()
.expect("External cleanup injection lock poisoned") = Some(workdir_id.to_string());
}
#[cfg(test)]
fn take_external_workdir_cleanup_failure(workdir_id: &str) -> bool {
let mut injected = external_workdir_cleanup_failure_slot()
.lock()
.expect("External cleanup injection lock poisoned");
if injected.as_deref() == Some(workdir_id) {
*injected = None;
true
} else {
false
}
}
#[cfg(test)]
fn external_workdir_cleanup_reconciliations() -> &'static Mutex<HashMap<String, usize>> {
use std::sync::OnceLock;
static RECONCILIATIONS: OnceLock<Mutex<HashMap<String, usize>>> = OnceLock::new();
RECONCILIATIONS.get_or_init(|| Mutex::new(HashMap::new()))
}
#[cfg(test)]
fn record_external_workdir_cleanup_reconciliation(workdir_id: &str) {
let mut reconciliations = external_workdir_cleanup_reconciliations()
.lock()
.expect("External cleanup reconciliation lock poisoned");
*reconciliations.entry(workdir_id.to_string()).or_default() += 1;
}
#[cfg(test)]
fn external_workdir_cleanup_reconciliation_count(workdir_id: &str) -> usize {
external_workdir_cleanup_reconciliations()
.lock()
.expect("External cleanup reconciliation lock poisoned")
.get(workdir_id)
.copied()
.unwrap_or_default()
}
async fn release_external_workdir_attachment_locked(
api: &WorkspaceApi,
workdir_id: &str,
link: &WorkerWorkdirLinkRecord,
) -> ApiResult<()> {
let links = api
.store
.list_workdir_worker_links(&api.config.workspace_id, workdir_id)?;
for link in links.into_iter().filter(|link| link.unlinked_at.is_none()) {
let lock = current_worker_session_lock(api, &link.worker);
let _guard = lock.lock().await;
if link.unlinked_at.is_none() {
close_current_worker_attachment_session_locked(api, &link.worker).await?;
close_current_worker_command_sessions_for_alias_locked(api, &link.worker, &link.alias)
.await?;
@@ -12724,10 +12761,47 @@ async fn release_external_workdir_attachments(
Some(workdir_id),
&Utc::now().to_rfc3339_opts(SecondsFormat::Nanos, true),
)?;
sync_runtime_worker_workdir_attachments(api, &link.worker)?;
api.worker_projection
.refresh(&link.worker)
.map_err(ApiError::from)?;
#[cfg(test)]
if take_external_workdir_cleanup_failure(workdir_id) {
return Err(Error::RuntimeOperationFailed {
runtime_id: "workspace-backend".to_string(),
code: "injected_external_workdir_cleanup_failure".to_string(),
message: "injected External Workdir attachment cleanup failure".to_string(),
}
.into());
}
}
// Reconcile even an already-unlinked durable link. A previous cleanup may
// have committed detach before Runtime persistence or projection failed.
#[cfg(test)]
record_external_workdir_cleanup_reconciliation(workdir_id);
sync_runtime_worker_workdir_attachments(api, &link.worker)?;
api.worker_projection
.refresh(&link.worker)
.map_err(ApiError::from)?;
Ok(())
}
async fn release_external_workdir_attachments(
api: &WorkspaceApi,
workdir_id: &str,
already_locked_worker: Option<&RuntimeWorkerRef>,
) -> ApiResult<()> {
let Some(link) = api
.store
.latest_workdir_worker_link(&api.config.workspace_id, workdir_id)?
else {
return Ok(());
};
// The newest link is the only possible active attachment and remains the
// durable reconciliation target if detach committed before Runtime update.
// Missing/ambiguous Runtime state remains a retryable cleanup failure.
if already_locked_worker == Some(&link.worker) {
release_external_workdir_attachment_locked(api, workdir_id, &link).await?;
} else {
let lock = current_worker_session_lock(api, &link.worker);
let _guard = lock.lock().await;
release_external_workdir_attachment_locked(api, workdir_id, &link).await?;
}
Ok(())
}
@@ -21226,7 +21300,13 @@ mod tests {
.map_err(Error::from)
})
.unwrap();
if cleanup_state == "completed" {
if cleanup_state == "completed"
&& !api
.external_workdir_expiry_tasks
.lock()
.unwrap()
.contains_key(&grant.grant_id)
{
break;
}
}
@@ -21237,6 +21317,219 @@ mod tests {
.expect("Backend-owned expiry must run without a provider socket");
}
#[tokio::test]
async fn session_open_expiry_persists_cleanup_retry_without_deadlocking() {
let dir = tempfile::tempdir().unwrap();
init_clean_git_workspace(dir.path());
let api = test_api(dir.path()).await;
let Json(created_worker) = create_workspace_worker(
State(api.clone()),
HeaderMap::new(),
Json(CreateWorkspaceWorkerRequest {
runtime_id: EMBEDDED_WORKER_RUNTIME_ID.to_string(),
display_name: "External expiry Worker".to_string(),
profile: Some("builtin:coder".to_string()),
ticket_assignment: None,
initial_submit: Vec::new(),
workdir_attachments: Vec::new(),
control_operation_id: None,
}),
)
.await
.unwrap();
let worker = RuntimeWorkerRef::new(&created_worker.runtime_id, &created_worker.worker_id);
let now = Utc::now();
let created_at = now.to_rfc3339_opts(SecondsFormat::Nanos, true);
let grant = ExternalWorkdirGrantRecord {
grant_id: "session-open-expiry-grant".to_string(),
workspace_id: TEST_WORKSPACE_ID.to_string(),
workdir_id: "session-open-expiry-workdir".to_string(),
provider_instance_id: "session-open-expiry-provider".to_string(),
display_name: "Session-open expiry".to_string(),
permissions: "read_only".to_string(),
created_by: test_browser_request_actor().account_id,
created_at: created_at.clone(),
expires_at: (now + Duration::minutes(5)).to_rfc3339_opts(SecondsFormat::Nanos, true),
generation: 1,
status: "online".to_string(),
updated_at: created_at.clone(),
};
api.store
.create_external_workdir_grant(
&grant,
&WorkdirRegistryRecord {
workspace_id: TEST_WORKSPACE_ID.to_string(),
workdir_id: grant.workdir_id.clone(),
display_name: Some(grant.display_name.clone()),
source: WorkdirRegistrySource::ExternalGrant {
grant_id: grant.grant_id.clone(),
},
creation_selector: None,
creation_ref: None,
creation_tree: None,
current_selector: None,
current_ref: None,
current_tree: None,
observed_at_epoch_seconds: None,
materialization_status: "present".to_string(),
cleanliness: "unknown".to_string(),
created_at: created_at.clone(),
updated_at: created_at.clone(),
},
)
.unwrap();
schedule_external_workdir_expiry(&api, &grant).unwrap();
let link = WorkerWorkdirLinkRecord {
workspace_id: TEST_WORKSPACE_ID.to_string(),
worker: worker.clone(),
workdir_id: grant.workdir_id.clone(),
alias: "expired-external".to_string(),
linked_at: created_at,
unlinked_at: None,
};
api.store.attach_worker_workdir(&link).unwrap();
sync_runtime_worker_workdir_attachments(&api, &worker).unwrap();
assert!(
!expire_external_workdir_grant(&api, &grant.grant_id)
.await
.unwrap(),
"an early scheduler wake must retain ownership without cleanup"
);
assert!(
api.store
.list_workdir_worker_links(TEST_WORKSPACE_ID, &grant.workdir_id)
.unwrap()
.iter()
.any(|current| current.unlinked_at.is_none())
);
api.config_store
.with_conn(|conn| {
conn.execute(
"UPDATE external_workdir_grants SET expires_at = ?1 WHERE workspace_id = ?2 AND grant_id = ?3",
rusqlite::params![
(now - Duration::seconds(1))
.to_rfc3339_opts(SecondsFormat::Nanos, true),
TEST_WORKSPACE_ID,
grant.grant_id,
],
)?;
Ok(())
})
.unwrap();
inject_external_workdir_cleanup_failure(&grant.workdir_id);
let session_lock = current_worker_session_lock(&api, &worker);
let session_guard = session_lock.lock().await;
let error = tokio::time::timeout(
std::time::Duration::from_secs(1),
open_current_worker_workdir_session_locked(&api, &worker, &link),
)
.await
.expect("session-open expiry cleanup must not reacquire its Worker lock")
.expect_err("injected attachment cleanup must surface");
assert!(matches!(
error,
Error::RuntimeOperationFailed { ref code, .. }
if code == "injected_external_workdir_cleanup_failure"
));
let (status, cleanup_state, cleanup_error): (String, String, Option<String>) = api
.config_store
.with_conn(|conn| {
conn.query_row(
"SELECT status, cleanup_state, cleanup_error FROM external_workdir_grants WHERE workspace_id = ?1 AND grant_id = ?2",
rusqlite::params![TEST_WORKSPACE_ID, grant.grant_id],
|row| Ok((row.get(0)?, row.get(1)?, row.get(2)?)),
)
.map_err(Error::from)
})
.unwrap();
assert_eq!(status, "expired");
assert_eq!(cleanup_state, "retry_required");
assert_eq!(cleanup_error.as_deref(), Some("attachment_release_failed"));
assert!(
api.external_workdir_expiry_tasks
.lock()
.unwrap()
.contains_key(&grant.grant_id),
"retry ownership must remain until cleanup completes"
);
assert!(
api.store
.list_workdir_worker_links(TEST_WORKSPACE_ID, &grant.workdir_id)
.unwrap()
.iter()
.all(|current| current.unlinked_at.is_some()),
"durable detach commits before the injected Runtime reconciliation failure"
);
assert_eq!(
external_workdir_cleanup_reconciliation_count(&grant.workdir_id),
0,
"injected failure occurs after durable detach but before Runtime reconciliation"
);
drop(session_guard);
assert!(
expire_external_workdir_grant(&api, &grant.grant_id)
.await
.unwrap()
);
cancel_external_workdir_expiry(&api, &grant.grant_id);
let (cleanup_state, cleanup_error): (String, Option<String>) = api
.config_store
.with_conn(|conn| {
conn.query_row(
"SELECT cleanup_state, cleanup_error FROM external_workdir_grants WHERE workspace_id = ?1 AND grant_id = ?2",
rusqlite::params![TEST_WORKSPACE_ID, grant.grant_id],
|row| Ok((row.get(0)?, row.get(1)?)),
)
.map_err(Error::from)
})
.unwrap();
assert_eq!(cleanup_state, "completed");
assert_eq!(cleanup_error, None);
assert!(
api.store
.list_workdir_worker_links(TEST_WORKSPACE_ID, &grant.workdir_id)
.unwrap()
.iter()
.all(|current| current.unlinked_at.is_some())
);
assert_eq!(
external_workdir_cleanup_reconciliation_count(&grant.workdir_id),
1,
"retry must reconcile an already-unlinked durable attachment"
);
assert!(
api.runtime
.worker(&worker)
.unwrap()
.workdir_attachments
.iter()
.all(
|attachment| attachment.working_directory.summary.working_directory_id
!= grant.workdir_id
),
"retry must reconcile Runtime state for an already-unlinked durable attachment"
);
let audit_actions: Vec<String> = api
.config_store
.with_conn(|conn| {
let mut statement = conn.prepare(
"SELECT action FROM audit_events WHERE workspace_id = ?1 AND target_kind = 'external_workdir_grant' AND target_id = ?2 ORDER BY created_at",
)?;
statement
.query_map(rusqlite::params![TEST_WORKSPACE_ID, grant.grant_id], |row| {
row.get(0)
})?
.collect::<rusqlite::Result<Vec<_>>>()
.map_err(Error::from)
})
.unwrap();
assert!(audit_actions.contains(&"external_workdir_grant_expired".to_string()));
assert!(audit_actions.contains(&"external_workdir_cleanup_attention_required".to_string()));
assert!(audit_actions.contains(&"external_workdir_cleanup_completed".to_string()));
}
#[tokio::test]
async fn external_workdirs_are_launchable_with_real_multiple_attachments() {
let dir = tempfile::tempdir().unwrap();
crates/workspace-server/src/store.rs
+25
View File
@@ -1743,6 +1743,11 @@ pub trait ControlPlaneStore: Send + Sync + WorkspaceDeletionStore {
workspace_id: &str,
workdir_id: &str,
) -> Result<Vec<WorkerWorkdirLinkRecord>>;
fn latest_workdir_worker_link(
&self,
workspace_id: &str,
workdir_id: &str,
) -> Result<Option<WorkerWorkdirLinkRecord>>;
}
#[derive(Clone)]
@@ -8293,6 +8298,26 @@ impl ControlPlaneStore for SqliteWorkspaceStore {
.map_err(Error::from)
})
}
fn latest_workdir_worker_link(
&self,
workspace_id: &str,
workdir_id: &str,
) -> Result<Option<WorkerWorkdirLinkRecord>> {
self.with_conn(|conn| {
conn.query_row(
r#"SELECT workspace_id, runtime_id, worker_id, workdir_id, alias, linked_at, unlinked_at
FROM worker_workdir_links
WHERE workspace_id = ?1 AND workdir_id = ?2
ORDER BY linked_at DESC, rowid DESC
LIMIT 1"#,
params![workspace_id, workdir_id],
read_worker_workdir_link_record,
)
.optional()
.map_err(Error::from)
})
}
}
fn flow_source_name(path: &str) -> Result<String> {
crates/yoi/src/workdir_share.rs
+29 -1
View File
@@ -77,11 +77,29 @@ impl Drop for OperationTask {
}
}
fn pin_selected_root(path: &std::path::Path) -> Result<ExternalWorkdirRoot, String> {
let cwd = std::env::current_dir()
.map_err(|error| format!("failed to resolve current directory: {error}"))?;
pin_selected_root_from(path, &cwd)
}
fn pin_selected_root_from(
path: &std::path::Path,
cwd: &std::path::Path,
) -> Result<ExternalWorkdirRoot, String> {
let absolute = if path.is_absolute() {
path.to_path_buf()
} else {
cwd.join(path)
};
ExternalWorkdirRoot::pin(absolute).map_err(|error| error.to_string())
}
pub(crate) async fn run(options: WorkdirShareOptions) -> Result<(), String> {
// Pin before creating remote authority: the Backend grant can never outlive
// a failed local approval/open race, and subsequent path replacement cannot
// redirect the provider.
let pinned_root = ExternalWorkdirRoot::pin(&options.path).map_err(|error| error.to_string())?;
let pinned_root = pin_selected_root(&options.path)?;
let client = BackendApiClient::from_stored_token(&options.backend_url)
.map_err(|error| error.to_string())?;
@@ -619,6 +637,16 @@ fn encode_path_segment(value: &str) -> String {
mod tests {
use super::*;
#[test]
fn relative_share_path_is_resolved_and_pinned_before_backend_access() {
let directory = tempfile::tempdir().unwrap();
let pinned = pin_selected_root_from(std::path::Path::new("."), directory.path()).unwrap();
assert_eq!(
pinned.canonical_path(),
directory.path().canonicalize().unwrap()
);
}
#[tokio::test]
async fn operation_shutdown_waits_until_executor_has_stopped() {
let operation_id = ExternalWorkdirOperationId::new("op-cancel-test").unwrap();