fix: enforce complete external edit result contract
This commit is contained in:
@@ -337,6 +337,11 @@ pub trait FsAccessPolicy: Send + Sync {
|
|||||||
None
|
None
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Bound provider-owned Edit result cardinality before mutation.
|
||||||
|
fn max_edit_replacements(&self) -> Option<usize> {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
|
||||||
/// Atomically replace or create an already-authorized writable file.
|
/// Atomically replace or create an already-authorized writable file.
|
||||||
/// Capability providers override this to keep parent traversal descriptor-
|
/// Capability providers override this to keep parent traversal descriptor-
|
||||||
/// confined through the final rename.
|
/// 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::Metadata> {
|
||||||
|
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 {
|
fn grep_request(path: &str, pattern: &str) -> GrepRequest {
|
||||||
GrepRequest {
|
GrepRequest {
|
||||||
pattern: pattern.to_string(),
|
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")]
|
#[cfg(target_os = "linux")]
|
||||||
#[test]
|
#[test]
|
||||||
fn descriptor_confined_atomic_write_rejects_symlinks_and_survives_root_rename() {
|
fn descriptor_confined_atomic_write_rejects_symlinks_and_survives_root_rename() {
|
||||||
|
|||||||
@@ -231,7 +231,13 @@ pub fn run_write(
|
|||||||
}
|
}
|
||||||
access
|
access
|
||||||
.atomic_write_file(&path, &target, &request.content, AtomicWriteMode::CreateNew)
|
.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 {
|
Ok(WriteResult {
|
||||||
bytes_written: request.content.len(),
|
bytes_written: request.content.len(),
|
||||||
@@ -276,6 +282,14 @@ pub fn run_edit(
|
|||||||
)));
|
)));
|
||||||
}
|
}
|
||||||
let replacement_count = if request.replace_all { occurrences } else { 1 };
|
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
|
let removed_bytes = request
|
||||||
.old_string
|
.old_string
|
||||||
.len()
|
.len()
|
||||||
|
|||||||
@@ -22,7 +22,7 @@ pub const MAX_EXTERNAL_PROVIDER_FRAME_BYTES: usize = 8 * 1024 * 1024;
|
|||||||
const MAX_PROTOCOL_ID_BYTES: usize = 128;
|
const MAX_PROTOCOL_ID_BYTES: usize = 128;
|
||||||
const MAX_EXTERNAL_PATH_BYTES: usize = 4 * 1024;
|
const MAX_EXTERNAL_PATH_BYTES: usize = 4 * 1024;
|
||||||
const MAX_EXTERNAL_PATTERN_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_GREP_CONTEXT: usize = 100;
|
||||||
const MAX_EXTERNAL_SCOPE_RULES: usize = 64;
|
const MAX_EXTERNAL_SCOPE_RULES: usize = 64;
|
||||||
pub const MAX_EXTERNAL_WRITE_BYTES: usize = 512 * 1024;
|
pub const MAX_EXTERNAL_WRITE_BYTES: usize = 512 * 1024;
|
||||||
|
|||||||
@@ -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() {
|
async fn external_read_write_rejects_oversized_preimages_and_edit_results_before_commit() {
|
||||||
let root = tempfile::tempdir().unwrap();
|
let root = tempfile::tempdir().unwrap();
|
||||||
let limit = crate::external::MAX_EXTERNAL_WRITE_BYTES;
|
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();
|
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(
|
let session = LocalWorkdirSession::external_read_write(
|
||||||
Workdir::new("external-workdir-bounded-rw"),
|
Workdir::new("external-workdir-bounded-rw"),
|
||||||
root.path(),
|
root.path(),
|
||||||
@@ -672,6 +679,10 @@ async fn external_read_write_rejects_oversized_preimages_and_edit_results_before
|
|||||||
.await
|
.await
|
||||||
.unwrap_err();
|
.unwrap_err();
|
||||||
assert!(matches!(write_error, WorkdirError::InvalidArgument(_)));
|
assert!(matches!(write_error, WorkdirError::InvalidArgument(_)));
|
||||||
|
assert_eq!(
|
||||||
|
std::fs::read(root.path().join("oversized.txt")).unwrap(),
|
||||||
|
oversized_content
|
||||||
|
);
|
||||||
let edit_error = WorkdirSession::edit(
|
let edit_error = WorkdirSession::edit(
|
||||||
&session,
|
&session,
|
||||||
EditRequest {
|
EditRequest {
|
||||||
@@ -686,10 +697,8 @@ async fn external_read_write_rejects_oversized_preimages_and_edit_results_before
|
|||||||
.unwrap_err();
|
.unwrap_err();
|
||||||
assert!(matches!(edit_error, WorkdirError::InvalidArgument(_)));
|
assert!(matches!(edit_error, WorkdirError::InvalidArgument(_)));
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
std::fs::metadata(root.path().join("oversized.txt"))
|
std::fs::read(root.path().join("oversized.txt")).unwrap(),
|
||||||
.unwrap()
|
oversized_content
|
||||||
.len(),
|
|
||||||
(limit + 1) as u64
|
|
||||||
);
|
);
|
||||||
|
|
||||||
let growth_observed = WorkdirSession::read(
|
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(),
|
std::fs::read(root.path().join("growth.txt")).unwrap(),
|
||||||
vec![b'x'; limit]
|
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")]
|
#[cfg(target_os = "linux")]
|
||||||
|
|||||||
@@ -352,6 +352,11 @@ impl fs_operation::FsAccessPolicy for ScopeAccess {
|
|||||||
.then_some(crate::external::MAX_EXTERNAL_WRITE_BYTES)
|
.then_some(crate::external::MAX_EXTERNAL_WRITE_BYTES)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn max_edit_replacements(&self) -> Option<usize> {
|
||||||
|
self.reject_symlinks
|
||||||
|
.then_some(crate::external::MAX_EXTERNAL_RESULT_ITEMS)
|
||||||
|
}
|
||||||
|
|
||||||
fn atomic_write_file(
|
fn atomic_write_file(
|
||||||
&self,
|
&self,
|
||||||
_logical: &Path,
|
_logical: &Path,
|
||||||
|
|||||||
Reference in New Issue
Block a user