diff --git a/crates/tools/src/checkout.rs b/crates/tools/src/checkout.rs index 622dd1b3d..8a697a67d 100644 --- a/crates/tools/src/checkout.rs +++ b/crates/tools/src/checkout.rs @@ -404,8 +404,8 @@ fn check_pattern(pattern: &str) -> Result<(), ToolError> { #[path = "checkout_tests.rs"] mod tests; -/// Read a bounded native text window through the same checked target and attachment tracker. -/// Search callers do not establish the prior-Read precondition for mutations. +/// Read a bounded native text window through the same checked target. +/// The returned one-shot action records prior-Read only after the caller's presentation succeeds. #[allow(clippy::too_many_arguments)] pub async fn read_checkout_text( router: Arc, @@ -417,8 +417,7 @@ pub async fn read_checkout_text( offset: usize, limit: usize, max_bytes: usize, - record_read: bool, -) -> Result<(workdir::ReadResult, Option>), ToolError> { +) -> Result<(workdir::ReadResult, Option>, impl FnOnce()), ToolError> { let selected = crate::routing::resolve_session(&router, Some(alias), WorkdirSessionCapability::Read)?; if selected.generation != generation { @@ -431,10 +430,11 @@ pub async fn read_checkout_text( validator: Some(validator), }; let (result, validator) = target.read(offset, limit, max_bytes).await?; - if record_read { - tracker.record_workdir_observation(&path, result.content_hash.clone(), result.total_lines); - } - Ok((result, validator)) + let hash = result.content_hash.clone(); + let total_lines = result.total_lines; + Ok((result, validator, move || { + tracker.record_workdir_observation(&path, hash, total_lines); + })) } /// Binary-safe checked byte window. This does not establish a text prior-Read observation. diff --git a/crates/worker/src/checkout.rs b/crates/worker/src/checkout.rs index 74666982a..b57415103 100644 --- a/crates/worker/src/checkout.rs +++ b/crates/worker/src/checkout.rs @@ -625,7 +625,6 @@ impl WipOperationHandler for FileHandler { offset, limit, crate::file_operation::MAX_BYTES, - operation == "read_lines", ), ) .await @@ -636,7 +635,7 @@ impl WipOperationHandler for FileHandler { ) })? .map_err(map_tool_error)?; - let (read, validator) = result; + let (read, validator, record_read) = result; self.check_connection()?; let text = String::from_utf8_lossy(&read.bytes); let value = crate::file_operation::page_result( @@ -646,6 +645,11 @@ impl WipOperationHandler for FileHandler { offset, read.truncated, )?; + // Only a successfully rendered Read can establish mutation authority. + // Search and failed result budgets must leave prior-Read history unchanged. + if operation == "read_lines" { + record_read(); + } return Ok(match validator { Some(raw) => WipOperationOutput::native_with_validator( value, diff --git a/crates/worker/src/checkout_http_tests.rs b/crates/worker/src/checkout_http_tests.rs index a7871239b..7436dc525 100644 --- a/crates/worker/src/checkout_http_tests.rs +++ b/crates/worker/src/checkout_http_tests.rs @@ -841,3 +841,41 @@ async fn remote_checked_byte_read_uses_common_contract_and_keeps_binary_boundari source.bytes ); } + +#[tokio::test] +async fn http_failed_text_read_does_not_authorize_mutations() { + for operation in ["write", "edit"] { + for oversized in ["\"".repeat(50_000), "x".repeat(65_530)] { + let fixture = HttpFixture::new(WorkdirSessionCapabilities::READ_WRITE).await; + let remote = fixture.open(TOKEN).await.unwrap(); + checkout_wip_tests::failed_text_read_cannot_authorize_mutation( + Arc::new(remote), + &fixture.dir.path().join("text"), + &oversized, + operation, + ) + .await; + let mutations = fixture + .state + .operations + .lock() + .unwrap() + .iter() + .filter(|op| { + matches!( + op, + WorkdirSessionOperation::CheckoutExecute(CheckoutRequest { + operation: CheckoutOperation::Write { .. } + | CheckoutOperation::Edit { .. }, + .. + }) + ) + }) + .count(); + assert_eq!( + mutations, 1, + "only the mutation after a successful Read may reach the provider" + ); + } + } +} diff --git a/crates/worker/src/checkout_wip_tests.rs b/crates/worker/src/checkout_wip_tests.rs index 9460924f1..199b2b32d 100644 --- a/crates/worker/src/checkout_wip_tests.rs +++ b/crates/worker/src/checkout_wip_tests.rs @@ -1536,3 +1536,82 @@ fn common_contract_registration_rejects_descriptor_redefinition_and_domain_opera Err(WipMountError::InterfaceCollision { .. }) )); } + +pub(super) async fn failed_text_read_cannot_authorize_mutation( + source: WorkdirSessionHandle, + disk_file: &std::path::Path, + oversized: &str, + operation: &str, +) { + let original = format!("{oversized}\nknown\n"); + std::fs::write(disk_file, &original).unwrap(); + let router = Arc::new(WorkdirSessionRouter::new()); + router + .attach(WorkdirAttachmentAlias::new("main").unwrap(), source) + .unwrap(); + let r = Arc::new(runtime(router, None)); + let inspected = model_inspect(r.clone(), "/checkouts/main/text").await; + let arguments = if operation == "write" { + json!({"content":"changed"}) + } else { + json!({"old_string":"known","new_string":"changed"}) + }; + let failure = model_invoke(r.clone(), &inspected, "read", json!({"limit":1})) + .await + .unwrap_err(); + assert!( + failure.to_string().contains("ResourceLimitExceeded"), + "{failure}" + ); + let denied = model_invoke(r.clone(), &inspected, operation, arguments.clone()).await; + assert!(denied.is_err(), "failed read authorized {operation}"); + let denied = denied.unwrap_err(); + assert!(denied.to_string().contains("InvalidArguments"), "{denied}"); + assert_eq!(std::fs::read_to_string(disk_file).unwrap(), original); + model_invoke(r.clone(), &inspected, "read", json!({"offset":1,"limit":1})) + .await + .unwrap(); + // A failed read must neither grant new authority nor erase an earlier successful Read. + assert!( + model_invoke(r.clone(), &inspected, "read", json!({"limit":1})) + .await + .is_err() + ); + model_invoke(r, &inspected, operation, arguments) + .await + .unwrap(); + let expected = if operation == "write" { + "changed".to_string() + } else { + original.replace("known", "changed") + }; + assert_eq!(std::fs::read_to_string(disk_file).unwrap(), expected); +} + +#[tokio::test] +async fn failed_text_read_does_not_authorize_write() { + for oversized in ["\"".repeat(50_000), "x".repeat(65_530)] { + let dir = TempDir::new().unwrap(); + failed_text_read_cannot_authorize_mutation( + session(&dir, false), + &dir.path().join("text"), + &oversized, + "write", + ) + .await; + } +} + +#[tokio::test] +async fn failed_text_read_does_not_authorize_edit() { + for oversized in ["\"".repeat(50_000), "x".repeat(65_530)] { + let dir = TempDir::new().unwrap(); + failed_text_read_cannot_authorize_mutation( + session(&dir, false), + &dir.path().join("text"), + &oversized, + "edit", + ) + .await; + } +}