diff --git a/crates/fs-operation/src/lib.rs b/crates/fs-operation/src/lib.rs index f8a2ca282..465e3b6b9 100644 --- a/crates/fs-operation/src/lib.rs +++ b/crates/fs-operation/src/lib.rs @@ -337,6 +337,11 @@ pub trait FsAccessPolicy: Send + Sync { None } + /// Bound provider-owned Edit result cardinality before mutation. + fn max_edit_replacements(&self) -> Option { + None + } + /// Atomically replace or create an already-authorized writable file. /// Capability providers override this to keep parent traversal descriptor- /// confined through the final rename. @@ -521,6 +526,33 @@ mod tests { } } + struct CreateRaceAccess { + root: PathBuf, + destination: PathBuf, + } + + impl FsAccessPolicy for CreateRaceAccess { + fn is_readable(&self, path: &Path) -> bool { + path.starts_with(&self.root) + } + + fn is_writable(&self, path: &Path) -> bool { + path.starts_with(&self.root) + } + + fn read_metadata( + &self, + _logical: &Path, + _resolved: &Path, + ) -> std::io::Result { + std::fs::write(&self.destination, b"created concurrently")?; + Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + "simulated absence before create race", + )) + } + } + fn grep_request(path: &str, pattern: &str) -> GrepRequest { GrepRequest { pattern: pattern.to_string(), @@ -1175,6 +1207,34 @@ mod tests { )); } + #[test] + fn no_clobber_create_race_is_reported_as_conflict() { + let root = tempfile::tempdir().unwrap(); + let destination = root.path().join("race.txt"); + let root_path = root.path().canonicalize().unwrap(); + let access = CreateRaceAccess { + root: root_path.clone(), + destination: destination.clone(), + }; + + let error = run_write( + &root_path, + WriteRequest { + path: FsPath::new("race.txt").unwrap(), + content: b"worker content".to_vec(), + expected_hash: None, + }, + &access, + ) + .unwrap_err(); + + assert!(matches!(error, FsError::Conflict(path) if path == "race.txt")); + assert_eq!( + std::fs::read_to_string(destination).unwrap(), + "created concurrently" + ); + } + #[cfg(target_os = "linux")] #[test] fn descriptor_confined_atomic_write_rejects_symlinks_and_survives_root_rename() { diff --git a/crates/fs-operation/src/local.rs b/crates/fs-operation/src/local.rs index 155e0bf4f..aaff61910 100644 --- a/crates/fs-operation/src/local.rs +++ b/crates/fs-operation/src/local.rs @@ -231,7 +231,13 @@ pub fn run_write( } access .atomic_write_file(&path, &target, &request.content, AtomicWriteMode::CreateNew) - .map_err(|error| map_io(&logical, error))?; + .map_err(|error| { + if error.kind() == std::io::ErrorKind::AlreadyExists { + FsError::Conflict(logical.as_str().to_string()) + } else { + map_io(&logical, error) + } + })?; } Ok(WriteResult { bytes_written: request.content.len(), @@ -276,6 +282,14 @@ pub fn run_edit( ))); } let replacement_count = if request.replace_all { occurrences } else { 1 }; + if let Some(limit) = access.max_edit_replacements() + && replacement_count > limit + { + return Err(FsError::InvalidArgument(format!( + "{} exceeds provider Edit replacement limit {limit}", + logical.as_str() + ))); + } let removed_bytes = request .old_string .len() diff --git a/crates/workdir/src/external.rs b/crates/workdir/src/external.rs index 1b48c57ef..22ff45d83 100644 --- a/crates/workdir/src/external.rs +++ b/crates/workdir/src/external.rs @@ -22,7 +22,7 @@ pub const MAX_EXTERNAL_PROVIDER_FRAME_BYTES: usize = 8 * 1024 * 1024; const MAX_PROTOCOL_ID_BYTES: usize = 128; const MAX_EXTERNAL_PATH_BYTES: usize = 4 * 1024; const MAX_EXTERNAL_PATTERN_BYTES: usize = 4 * 1024; -const MAX_EXTERNAL_RESULT_ITEMS: usize = 10_000; +pub const MAX_EXTERNAL_RESULT_ITEMS: usize = 10_000; const MAX_EXTERNAL_GREP_CONTEXT: usize = 100; const MAX_EXTERNAL_SCOPE_RULES: usize = 64; pub const MAX_EXTERNAL_WRITE_BYTES: usize = 512 * 1024; diff --git a/crates/workdir/src/external_local_tests.rs b/crates/workdir/src/external_local_tests.rs index b9aeab480..943eb3dd5 100644 --- a/crates/workdir/src/external_local_tests.rs +++ b/crates/workdir/src/external_local_tests.rs @@ -641,8 +641,15 @@ async fn external_read_write_uses_pinned_root_for_conflict_and_commit() { async fn external_read_write_rejects_oversized_preimages_and_edit_results_before_commit() { let root = tempfile::tempdir().unwrap(); let limit = crate::external::MAX_EXTERNAL_WRITE_BYTES; - std::fs::write(root.path().join("oversized.txt"), vec![b'x'; limit + 1]).unwrap(); + let oversized_content = vec![b'x'; limit + 1]; + std::fs::write(root.path().join("oversized.txt"), &oversized_content).unwrap(); std::fs::write(root.path().join("growth.txt"), vec![b'x'; limit]).unwrap(); + let high_replacement_content = vec![b'x'; crate::external::MAX_EXTERNAL_RESULT_ITEMS + 1]; + std::fs::write( + root.path().join("high-replacements.txt"), + &high_replacement_content, + ) + .unwrap(); let session = LocalWorkdirSession::external_read_write( Workdir::new("external-workdir-bounded-rw"), root.path(), @@ -672,6 +679,10 @@ async fn external_read_write_rejects_oversized_preimages_and_edit_results_before .await .unwrap_err(); assert!(matches!(write_error, WorkdirError::InvalidArgument(_))); + assert_eq!( + std::fs::read(root.path().join("oversized.txt")).unwrap(), + oversized_content + ); let edit_error = WorkdirSession::edit( &session, EditRequest { @@ -686,10 +697,8 @@ async fn external_read_write_rejects_oversized_preimages_and_edit_results_before .unwrap_err(); assert!(matches!(edit_error, WorkdirError::InvalidArgument(_))); assert_eq!( - std::fs::metadata(root.path().join("oversized.txt")) - .unwrap() - .len(), - (limit + 1) as u64 + std::fs::read(root.path().join("oversized.txt")).unwrap(), + oversized_content ); let growth_observed = WorkdirSession::read( @@ -720,6 +729,38 @@ async fn external_read_write_rejects_oversized_preimages_and_edit_results_before std::fs::read(root.path().join("growth.txt")).unwrap(), vec![b'x'; limit] ); + + let high_replacement_observed = WorkdirSession::read( + &session, + ReadRequest { + path: WorkdirPath::new("high-replacements.txt").unwrap(), + offset: 0, + limit: 1, + max_bytes: 1, + }, + ) + .await + .unwrap(); + let high_replacement_error = WorkdirSession::edit( + &session, + EditRequest { + path: WorkdirPath::new("high-replacements.txt").unwrap(), + old_string: "x".to_string(), + new_string: "y".to_string(), + replace_all: true, + expected_hash: high_replacement_observed.content_hash, + }, + ) + .await + .unwrap_err(); + assert!(matches!( + high_replacement_error, + WorkdirError::InvalidArgument(_) + )); + assert_eq!( + std::fs::read(root.path().join("high-replacements.txt")).unwrap(), + high_replacement_content + ); } #[cfg(target_os = "linux")] diff --git a/crates/workdir/src/local.rs b/crates/workdir/src/local.rs index 1d861a35c..2d67a3a12 100644 --- a/crates/workdir/src/local.rs +++ b/crates/workdir/src/local.rs @@ -352,6 +352,11 @@ impl fs_operation::FsAccessPolicy for ScopeAccess { .then_some(crate::external::MAX_EXTERNAL_WRITE_BYTES) } + fn max_edit_replacements(&self) -> Option { + self.reject_symlinks + .then_some(crate::external::MAX_EXTERNAL_RESULT_ITEMS) + } + fn atomic_write_file( &self, _logical: &Path,