fix: retain external workdir results independently of audit failures
This commit is contained in:
@@ -291,6 +291,11 @@ impl WorkspaceAttachedWorkdirSession {
|
||||
.execute(request)
|
||||
.map_err(workspace_workdir_error)?;
|
||||
if !response.is_success() {
|
||||
if let Ok(error) =
|
||||
serde_json::from_str::<server_api::WorkdirOperationApiError>(&response.body)
|
||||
{
|
||||
notify_workdir_audit_failures(&error.audit_failures);
|
||||
}
|
||||
return Err(
|
||||
serde_json::from_str::<workdir::http::WorkdirTransportError>(&response.body)
|
||||
.map(workdir::http::WorkdirTransportError::into_workdir_error)
|
||||
@@ -302,11 +307,14 @@ impl WorkspaceAttachedWorkdirSession {
|
||||
}),
|
||||
);
|
||||
}
|
||||
serde_json::from_str(&response.body).map_err(|error| {
|
||||
WorkdirError::Transport(format!(
|
||||
"failed to decode Workspace Workdir operation result: {error}"
|
||||
))
|
||||
})
|
||||
let response: workdir::http::WorkdirSessionOperationResponse =
|
||||
serde_json::from_str(&response.body).map_err(|_| {
|
||||
WorkdirError::Transport(
|
||||
"failed to decode Workspace Workdir operation result".into(),
|
||||
)
|
||||
})?;
|
||||
notify_workdir_audit_failures(&response.audit_failures);
|
||||
Ok(response.result)
|
||||
}
|
||||
|
||||
fn mismatch(expected: &str) -> WorkdirError {
|
||||
@@ -316,6 +324,27 @@ impl WorkspaceAttachedWorkdirSession {
|
||||
}
|
||||
}
|
||||
|
||||
// Notification is separate from the operation return: callers retain successful command
|
||||
// handles, checkout observations and original failures, and must not resend for audit loss.
|
||||
fn notify_workdir_audit_failures(failures: &[workdir::http::WorkdirAuditFailure]) {
|
||||
use sha2::Digest;
|
||||
for failure in failures {
|
||||
// Response strings are untrusted. Never copy them into the receiving process log.
|
||||
let operation_id_hash = sha2::Sha256::digest(failure.operation_id.as_bytes())
|
||||
.iter()
|
||||
.map(|byte| format!("{byte:02x}"))
|
||||
.collect::<String>();
|
||||
tracing::error!(
|
||||
event = "external_workdir_audit_failed",
|
||||
operation_id_hash,
|
||||
audit_stage = ?failure.stage,
|
||||
audit_reason = ?failure.reason,
|
||||
operation_outcome = ?failure.operation_outcome,
|
||||
"External Workdir audit persistence failed; operation result is independent; do not resend"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn workspace_workdir_error(error: WorkspaceClientError) -> WorkdirError {
|
||||
match error {
|
||||
WorkspaceClientError::Unavailable(message) => WorkdirError::Unavailable(message),
|
||||
@@ -1653,6 +1682,67 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn attached_session_keeps_operation_results_when_audit_persistence_failed() {
|
||||
let failure = json!({
|
||||
"operation_id":"op-safe-id", "stage":"response_received",
|
||||
"reason":"audit_store_unavailable", "operation_outcome":{"kind":"succeeded"}
|
||||
});
|
||||
let mut responses = vec![response(json!({
|
||||
"operation":"command_start", "result":"retained-command",
|
||||
"audit_failures":[failure]
|
||||
}))];
|
||||
for (code, status, outcome) in [
|
||||
("denied", 403, json!({"kind":"failed","code":"denied"})),
|
||||
("outcome_unknown", 500, json!({"kind":"unknown"})),
|
||||
("not_dispatched", 503, json!({"kind":"not_dispatched"})),
|
||||
] {
|
||||
let mut failure = failure.clone();
|
||||
failure["operation_outcome"] = outcome;
|
||||
responses.push(WorkspaceResponse { status, body: json!({
|
||||
"code":code, "message":"safe provider classification", "audit_failures":[failure]
|
||||
}).to_string() });
|
||||
}
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(responses));
|
||||
let session = WorkspaceAttachedWorkdirSession::handle(client.clone(), "checkout");
|
||||
let handle = session
|
||||
.start_command(CommandRequest {
|
||||
command: "true".into(),
|
||||
timeout_secs: 1,
|
||||
transport_byte_budget: workdir::MAX_COMMAND_TRANSPORT_BYTES,
|
||||
cwd: workdir::WorkdirPath::root(),
|
||||
tool_call_id: None,
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(handle.0, "retained-command");
|
||||
let request = workdir::CheckoutRequest {
|
||||
target: workdir::WorkdirPath::new("file.txt").unwrap(),
|
||||
validator: vec![1],
|
||||
operation: workdir::CheckoutOperation::Write {
|
||||
content: b"new".to_vec(),
|
||||
expected_hash: [0; 32],
|
||||
},
|
||||
};
|
||||
assert!(matches!(
|
||||
session.checkout_execute(request.clone()).await,
|
||||
Err(WorkdirError::Denied(_))
|
||||
));
|
||||
assert!(matches!(
|
||||
session.checkout_execute(request.clone()).await,
|
||||
Err(WorkdirError::OutcomeUnknown(_))
|
||||
));
|
||||
assert!(matches!(
|
||||
session.checkout_execute(request).await,
|
||||
Err(WorkdirError::NotDispatched)
|
||||
));
|
||||
assert_eq!(
|
||||
client.requests().len(),
|
||||
4,
|
||||
"audit loss must not trigger a resend"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn attached_session_does_not_send_worker_local_bash_spill_path() {
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![response(json!({
|
||||
|
||||
Reference in New Issue
Block a user