fix: record native prior Read only after result validation
This commit is contained in:
@@ -404,8 +404,8 @@ fn check_pattern(pattern: &str) -> Result<(), ToolError> {
|
|||||||
#[path = "checkout_tests.rs"]
|
#[path = "checkout_tests.rs"]
|
||||||
mod tests;
|
mod tests;
|
||||||
|
|
||||||
/// Read a bounded native text window through the same checked target and attachment tracker.
|
/// Read a bounded native text window through the same checked target.
|
||||||
/// Search callers do not establish the prior-Read precondition for mutations.
|
/// The returned one-shot action records prior-Read only after the caller's presentation succeeds.
|
||||||
#[allow(clippy::too_many_arguments)]
|
#[allow(clippy::too_many_arguments)]
|
||||||
pub async fn read_checkout_text(
|
pub async fn read_checkout_text(
|
||||||
router: Arc<WorkdirSessionRouter>,
|
router: Arc<WorkdirSessionRouter>,
|
||||||
@@ -417,8 +417,7 @@ pub async fn read_checkout_text(
|
|||||||
offset: usize,
|
offset: usize,
|
||||||
limit: usize,
|
limit: usize,
|
||||||
max_bytes: usize,
|
max_bytes: usize,
|
||||||
record_read: bool,
|
) -> Result<(workdir::ReadResult, Option<Vec<u8>>, impl FnOnce()), ToolError> {
|
||||||
) -> Result<(workdir::ReadResult, Option<Vec<u8>>), ToolError> {
|
|
||||||
let selected =
|
let selected =
|
||||||
crate::routing::resolve_session(&router, Some(alias), WorkdirSessionCapability::Read)?;
|
crate::routing::resolve_session(&router, Some(alias), WorkdirSessionCapability::Read)?;
|
||||||
if selected.generation != generation {
|
if selected.generation != generation {
|
||||||
@@ -431,10 +430,11 @@ pub async fn read_checkout_text(
|
|||||||
validator: Some(validator),
|
validator: Some(validator),
|
||||||
};
|
};
|
||||||
let (result, validator) = target.read(offset, limit, max_bytes).await?;
|
let (result, validator) = target.read(offset, limit, max_bytes).await?;
|
||||||
if record_read {
|
let hash = result.content_hash.clone();
|
||||||
tracker.record_workdir_observation(&path, result.content_hash.clone(), result.total_lines);
|
let total_lines = result.total_lines;
|
||||||
}
|
Ok((result, validator, move || {
|
||||||
Ok((result, validator))
|
tracker.record_workdir_observation(&path, hash, total_lines);
|
||||||
|
}))
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Binary-safe checked byte window. This does not establish a text prior-Read observation.
|
/// Binary-safe checked byte window. This does not establish a text prior-Read observation.
|
||||||
|
|||||||
@@ -625,7 +625,6 @@ impl WipOperationHandler for FileHandler {
|
|||||||
offset,
|
offset,
|
||||||
limit,
|
limit,
|
||||||
crate::file_operation::MAX_BYTES,
|
crate::file_operation::MAX_BYTES,
|
||||||
operation == "read_lines",
|
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
@@ -636,7 +635,7 @@ impl WipOperationHandler for FileHandler {
|
|||||||
)
|
)
|
||||||
})?
|
})?
|
||||||
.map_err(map_tool_error)?;
|
.map_err(map_tool_error)?;
|
||||||
let (read, validator) = result;
|
let (read, validator, record_read) = result;
|
||||||
self.check_connection()?;
|
self.check_connection()?;
|
||||||
let text = String::from_utf8_lossy(&read.bytes);
|
let text = String::from_utf8_lossy(&read.bytes);
|
||||||
let value = crate::file_operation::page_result(
|
let value = crate::file_operation::page_result(
|
||||||
@@ -646,6 +645,11 @@ impl WipOperationHandler for FileHandler {
|
|||||||
offset,
|
offset,
|
||||||
read.truncated,
|
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 {
|
return Ok(match validator {
|
||||||
Some(raw) => WipOperationOutput::native_with_validator(
|
Some(raw) => WipOperationOutput::native_with_validator(
|
||||||
value,
|
value,
|
||||||
|
|||||||
@@ -841,3 +841,41 @@ async fn remote_checked_byte_read_uses_common_contract_and_keeps_binary_boundari
|
|||||||
source.bytes
|
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"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -1536,3 +1536,82 @@ fn common_contract_registration_rejects_descriptor_redefinition_and_domain_opera
|
|||||||
Err(WipMountError::InterfaceCollision { .. })
|
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;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user