fix: harden external workdir lifecycle
This commit is contained in:
@@ -15,6 +15,10 @@ pub fn run_glob(
|
||||
if !root.is_absolute() {
|
||||
return Err(FsError::RelativePath(root.to_path_buf()));
|
||||
}
|
||||
access.check_cancelled().map_err(|source| FsError::Io {
|
||||
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,
|
||||
@@ -32,6 +36,10 @@ pub fn run_glob(
|
||||
walker.hidden(false).follow_links(false);
|
||||
let mut visited = 0_usize;
|
||||
for entry in walker.build().flatten() {
|
||||
access.check_cancelled().map_err(|source| FsError::Io {
|
||||
path: PathBuf::from(request.path.as_str()),
|
||||
source,
|
||||
})?;
|
||||
visited = visited.saturating_add(1);
|
||||
if visited > crate::MAX_TRAVERSAL_ENTRIES {
|
||||
return Err(FsError::InvalidArgument(format!(
|
||||
|
||||
@@ -152,6 +152,12 @@ pub trait FsAccessPolicy: Send + Sync {
|
||||
fn is_readable(&self, path: &Path) -> bool;
|
||||
fn is_writable(&self, path: &Path) -> bool;
|
||||
|
||||
/// Cooperatively stop provider-owned filesystem work. Implementations may
|
||||
/// use this for per-operation cancellation and deadlines.
|
||||
fn check_cancelled(&self) -> std::io::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// 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> {
|
||||
@@ -338,6 +344,87 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cancellation_stops_a_read_already_in_progress() {
|
||||
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
|
||||
use std::sync::{Arc, Condvar, Mutex};
|
||||
|
||||
struct CancellableAccess {
|
||||
root: PathBuf,
|
||||
checks: AtomicUsize,
|
||||
cancelled: AtomicBool,
|
||||
state: Mutex<()>,
|
||||
changed: Condvar,
|
||||
}
|
||||
|
||||
impl FsAccessPolicy for CancellableAccess {
|
||||
fn is_readable(&self, path: &Path) -> bool {
|
||||
path.starts_with(&self.root)
|
||||
}
|
||||
|
||||
fn is_writable(&self, _path: &Path) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
fn check_cancelled(&self) -> std::io::Result<()> {
|
||||
if self.checks.fetch_add(1, Ordering::SeqCst) == 1 {
|
||||
self.changed.notify_all();
|
||||
let mut state = self.state.lock().unwrap();
|
||||
while !self.cancelled.load(Ordering::Acquire) {
|
||||
state = self.changed.wait(state).unwrap();
|
||||
}
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::Interrupted,
|
||||
"cancelled",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
let directory = tempfile::tempdir().unwrap();
|
||||
let root = directory.path().canonicalize().unwrap();
|
||||
std::fs::write(root.join("large.txt"), vec![b'x'; 128 * 1024]).unwrap();
|
||||
let access = Arc::new(CancellableAccess {
|
||||
root: root.clone(),
|
||||
checks: AtomicUsize::new(0),
|
||||
cancelled: AtomicBool::new(false),
|
||||
state: Mutex::new(()),
|
||||
changed: Condvar::new(),
|
||||
});
|
||||
let task_access = access.clone();
|
||||
let task_root = root.clone();
|
||||
let task = std::thread::spawn(move || {
|
||||
run_read_bounded(
|
||||
&task_root,
|
||||
ReadRequest {
|
||||
path: FsPath::new("large.txt").unwrap(),
|
||||
offset: 0,
|
||||
limit: usize::MAX,
|
||||
max_bytes: 1024,
|
||||
},
|
||||
task_access.as_ref(),
|
||||
BoundedReadLimits::new(256 * 1024, 1024).unwrap(),
|
||||
)
|
||||
});
|
||||
|
||||
let mut state = access.state.lock().unwrap();
|
||||
while access.checks.load(Ordering::SeqCst) < 2 {
|
||||
state = access.changed.wait(state).unwrap();
|
||||
}
|
||||
access.cancelled.store(true, Ordering::Release);
|
||||
access.changed.notify_all();
|
||||
drop(state);
|
||||
|
||||
let error = task
|
||||
.join()
|
||||
.unwrap()
|
||||
.expect_err("read must stop on cancellation");
|
||||
assert!(
|
||||
matches!(error, FsError::Io { source, .. } if source.kind() == std::io::ErrorKind::Interrupted)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn logical_paths_reject_absolute_parent_and_backslash_forms() {
|
||||
assert!(FsPath::new("src/lib.rs").is_ok());
|
||||
|
||||
@@ -20,6 +20,9 @@ pub fn run_stat(
|
||||
access: &dyn FsAccessPolicy,
|
||||
) -> Result<StatResult, FsError> {
|
||||
let logical = request.path;
|
||||
access
|
||||
.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))?;
|
||||
if !access.is_readable_paths(&path, &resolved) {
|
||||
@@ -73,6 +76,9 @@ fn run_read_with_limits(
|
||||
limits: Option<BoundedReadLimits>,
|
||||
) -> Result<ReadResult, FsError> {
|
||||
let logical = request.path;
|
||||
access
|
||||
.check_cancelled()
|
||||
.map_err(|error| map_io(&logical, error))?;
|
||||
let path = resolve(root, &logical)?;
|
||||
let target = require_access(&path, &logical, access, false, false)?;
|
||||
let file = access
|
||||
@@ -111,6 +117,9 @@ fn run_read_with_limits(
|
||||
let mut reader = BufReader::with_capacity(READ_BUFFER_BYTES, file);
|
||||
let mut buffer = [0u8; READ_BUFFER_BYTES];
|
||||
loop {
|
||||
access
|
||||
.check_cancelled()
|
||||
.map_err(|error| map_io(&logical, error))?;
|
||||
let read_limit = limits
|
||||
.map(|limits| {
|
||||
limits
|
||||
@@ -190,6 +199,9 @@ pub fn run_write(
|
||||
access: &dyn FsAccessPolicy,
|
||||
) -> Result<WriteResult, FsError> {
|
||||
let logical = request.path;
|
||||
access
|
||||
.check_cancelled()
|
||||
.map_err(|error| map_io(&logical, error))?;
|
||||
let path = resolve(root, &logical)?;
|
||||
let created = !path.exists();
|
||||
if path.exists() {
|
||||
@@ -222,6 +234,9 @@ pub fn run_edit(
|
||||
access: &dyn FsAccessPolicy,
|
||||
) -> Result<EditResult, FsError> {
|
||||
let logical = request.path;
|
||||
access
|
||||
.check_cancelled()
|
||||
.map_err(|error| map_io(&logical, error))?;
|
||||
let path = resolve(root, &logical)?;
|
||||
let target = require_access(&path, &logical, access, true, false)?;
|
||||
let bytes = fs::read(&target).map_err(|error| map_io(&logical, error))?;
|
||||
@@ -262,6 +277,9 @@ pub fn run_list(
|
||||
access: &dyn FsAccessPolicy,
|
||||
) -> Result<ListResult, FsError> {
|
||||
let logical = request.path;
|
||||
access
|
||||
.check_cancelled()
|
||||
.map_err(|error| map_io(&logical, error))?;
|
||||
let path = resolve(root, &logical)?;
|
||||
let logical_base = path.clone();
|
||||
let path = require_access(&path, &logical, access, false, true)?;
|
||||
@@ -279,6 +297,9 @@ pub fn run_list(
|
||||
.open_read_dir(&logical_base, &path)
|
||||
.map_err(|error| map_io(&logical, error))?;
|
||||
for (entry_index, entry) in read_dir.enumerate() {
|
||||
access
|
||||
.check_cancelled()
|
||||
.map_err(|error| map_io(&logical, error))?;
|
||||
if entry_index >= crate::MAX_TRAVERSAL_ENTRIES {
|
||||
return Err(FsError::InvalidArgument(format!(
|
||||
"directory traversal exceeds provider limit {}",
|
||||
|
||||
@@ -18,10 +18,12 @@ use crate::{
|
||||
struct SourceBoundedReader<'a, R> {
|
||||
inner: R,
|
||||
remaining: &'a mut u64,
|
||||
access: &'a dyn FsAccessPolicy,
|
||||
}
|
||||
|
||||
impl<R: Read> Read for SourceBoundedReader<'_, R> {
|
||||
fn read(&mut self, buffer: &mut [u8]) -> std::io::Result<usize> {
|
||||
self.access.check_cancelled()?;
|
||||
if buffer.is_empty() {
|
||||
return Ok(0);
|
||||
}
|
||||
@@ -228,6 +230,9 @@ pub fn run_grep(
|
||||
request: GrepRequest,
|
||||
access: &dyn FsAccessPolicy,
|
||||
) -> Result<GrepResult, FsError> {
|
||||
access
|
||||
.check_cancelled()
|
||||
.map_err(|error| FsError::io(&base, error))?;
|
||||
let p = GrepParams {
|
||||
pattern: request.pattern,
|
||||
path: Some(base.clone()),
|
||||
@@ -372,6 +377,9 @@ pub fn run_grep(
|
||||
let mut visited = 0_usize;
|
||||
let mut source_bytes_remaining = crate::MAX_GREP_SOURCE_BYTES;
|
||||
for entry in walker.build().flatten() {
|
||||
access
|
||||
.check_cancelled()
|
||||
.map_err(|error| FsError::io(&base, error))?;
|
||||
visited = visited.saturating_add(1);
|
||||
if visited > crate::MAX_TRAVERSAL_ENTRIES {
|
||||
return Err(FsError::InvalidArgument(format!(
|
||||
@@ -445,6 +453,7 @@ fn scan_path(
|
||||
let mut reader = SourceBoundedReader {
|
||||
inner: reader,
|
||||
remaining: source_bytes_remaining,
|
||||
access,
|
||||
};
|
||||
match mode {
|
||||
GrepOutputMode::FilesWithMatches => {
|
||||
@@ -635,8 +644,22 @@ impl Sink for ContentSink<'_> {
|
||||
#[cfg(test)]
|
||||
mod bounded_reader_tests {
|
||||
use std::io::{Cursor, Read as _};
|
||||
use std::path::Path;
|
||||
|
||||
use super::SourceBoundedReader;
|
||||
use crate::FsAccessPolicy;
|
||||
|
||||
struct Permit;
|
||||
|
||||
impl FsAccessPolicy for Permit {
|
||||
fn is_readable(&self, _path: &Path) -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
fn is_writable(&self, _path: &Path) -> bool {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn source_budget_is_shared_across_candidate_files() {
|
||||
@@ -644,6 +667,7 @@ mod bounded_reader_tests {
|
||||
let mut first = SourceBoundedReader {
|
||||
inner: Cursor::new(b"ab"),
|
||||
remaining: &mut remaining,
|
||||
access: &Permit,
|
||||
};
|
||||
let mut output = Vec::new();
|
||||
first.read_to_end(&mut output).unwrap();
|
||||
@@ -653,6 +677,7 @@ mod bounded_reader_tests {
|
||||
let mut second = SourceBoundedReader {
|
||||
inner: Cursor::new(b"cd"),
|
||||
remaining: &mut remaining,
|
||||
access: &Permit,
|
||||
};
|
||||
let error = second.read_to_end(&mut output).unwrap_err();
|
||||
assert_eq!(error.kind(), std::io::ErrorKind::InvalidData);
|
||||
|
||||
Reference in New Issue
Block a user