fix: retain safe Workdir open authorization and response failure causes
Keep bounded reasons and derived failure stages separate from public classification. Reject provider-authored raw error text on transport decoding. Preserve identity mismatch when cleanup fails and retain its handle for explicit close without opening again. Add signed Runtime HTTP, Registry/API logging, remote mismatch, authorization no-dispatch and deferred fault coverage. Focused tests and Workdir suite pass; full-source validation follows T773 audit contract merge.
This commit is contained in:
@@ -375,12 +375,14 @@ impl WorkdirTransportError {
|
||||
use WorkdirTransportErrorCode as Code;
|
||||
let error = match self.code {
|
||||
Code::NotFound => WorkdirError::NotFound("<remote>".into()),
|
||||
Code::Conflict => WorkdirError::Conflict(self.message),
|
||||
Code::Conflict => WorkdirError::Conflict(
|
||||
"The target file's content or existence changed since it was last observed; read the file again before retrying".into(),
|
||||
),
|
||||
Code::OutcomeUnknown => WorkdirError::OutcomeUnknown(
|
||||
"Workdir operation may have effects; inspect before retrying".into(),
|
||||
),
|
||||
Code::Unsupported => WorkdirError::UnsupportedOperation(self.message),
|
||||
Code::InvalidRequest => WorkdirError::InvalidArgument(self.message),
|
||||
Code::Unsupported => WorkdirError::UnsupportedOperation("Workdir operation is not supported".into()),
|
||||
Code::InvalidRequest => WorkdirError::InvalidArgument("Workdir operation request is invalid".into()),
|
||||
Code::Denied => WorkdirError::Denied(crate::WorkdirDenial {
|
||||
reason: self.denial_reason,
|
||||
// Provider-authored text is not trusted diagnostic data.
|
||||
@@ -409,12 +411,12 @@ impl WorkdirTransportError {
|
||||
target: "<remote-target>".into(),
|
||||
},
|
||||
Code::UnknownCommand => WorkdirError::UnknownCommand("<remote>".to_string()),
|
||||
Code::Unavailable => WorkdirError::Unavailable(self.message),
|
||||
Code::Unavailable => WorkdirError::Unavailable("Workdir session is unavailable".into()),
|
||||
Code::Io => WorkdirError::Io {
|
||||
path: "<remote>".into(),
|
||||
source: std::io::Error::other(self.message),
|
||||
source: std::io::Error::other("Workdir I/O operation failed"),
|
||||
},
|
||||
Code::Transport => WorkdirError::Transport(self.message),
|
||||
Code::Transport => WorkdirError::Transport("Workdir transport failed".into()),
|
||||
Code::Internal => WorkdirError::OperationFailed,
|
||||
};
|
||||
let error = if let Some(reason) = self.failure_reason {
|
||||
@@ -549,7 +551,8 @@ mod client {
|
||||
if opened.workdir_id.as_str() != workdir_id.as_str() {
|
||||
return Err(WorkdirError::Unavailable(
|
||||
"Runtime opened a session for a different Workdir".to_string(),
|
||||
));
|
||||
)
|
||||
.with_failure_reason(crate::WorkdirFailureReason::SessionIdentityMismatch));
|
||||
}
|
||||
Ok(Self {
|
||||
client,
|
||||
@@ -608,6 +611,7 @@ mod client {
|
||||
WorkdirError::Unavailable(format!(
|
||||
"Runtime returned a mismatched Workdir operation result; expected {expected}"
|
||||
))
|
||||
.with_failure_reason(crate::WorkdirFailureReason::OperationResultMismatch)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -770,7 +774,8 @@ mod client {
|
||||
}
|
||||
Ok(_) if mutation => Err(WorkdirError::OutcomeUnknown(
|
||||
"mismatched response after checkout mutation".into(),
|
||||
)),
|
||||
)
|
||||
.with_failure_reason(crate::WorkdirFailureReason::OperationResultMismatch)),
|
||||
Ok(_) => Err(Self::mismatch("checkout_execute")),
|
||||
Err(error)
|
||||
if mutation
|
||||
|
||||
@@ -414,6 +414,24 @@ pub enum WorkdirFailureReason {
|
||||
HttpTransfer,
|
||||
HttpResponseInvalid,
|
||||
HttpResponseTooLarge,
|
||||
OperationResultMismatch,
|
||||
SessionIdentityMismatch,
|
||||
SessionOpenTaskFailed,
|
||||
SessionOpenFailed,
|
||||
AuthorizationMissing,
|
||||
AuthorizationStale,
|
||||
AuthorizationUnavailable,
|
||||
SigningIdentityUnavailable,
|
||||
AuthorizationSignFailed,
|
||||
RuntimeStopped,
|
||||
RuntimeStateUnavailable,
|
||||
RuntimeBackendUnavailable,
|
||||
RuntimeOwnerMismatch,
|
||||
RuntimeInvalidRequest,
|
||||
WorkingDirectoryNotFound,
|
||||
WorkingDirectoryNotActive,
|
||||
WorkingDirectoryRecordInvalid,
|
||||
WorkingDirectoryAccessUnavailable,
|
||||
}
|
||||
|
||||
impl WorkdirFailureReason {
|
||||
@@ -427,6 +445,83 @@ impl WorkdirFailureReason {
|
||||
Self::HttpTransfer => "http_transfer",
|
||||
Self::HttpResponseInvalid => "http_response_invalid",
|
||||
Self::HttpResponseTooLarge => "http_response_too_large",
|
||||
Self::OperationResultMismatch => "operation_result_mismatch",
|
||||
Self::SessionIdentityMismatch => "session_identity_mismatch",
|
||||
Self::SessionOpenTaskFailed => "session_open_task_failed",
|
||||
Self::SessionOpenFailed => "session_open_failed",
|
||||
Self::AuthorizationMissing => "authorization_missing",
|
||||
Self::AuthorizationStale => "authorization_stale",
|
||||
Self::AuthorizationUnavailable => "authorization_unavailable",
|
||||
Self::SigningIdentityUnavailable => "signing_identity_unavailable",
|
||||
Self::AuthorizationSignFailed => "authorization_sign_failed",
|
||||
Self::RuntimeStopped => "runtime_stopped",
|
||||
Self::RuntimeStateUnavailable => "runtime_state_unavailable",
|
||||
Self::RuntimeBackendUnavailable => "runtime_backend_unavailable",
|
||||
Self::RuntimeOwnerMismatch => "runtime_owner_mismatch",
|
||||
Self::RuntimeInvalidRequest => "runtime_invalid_request",
|
||||
Self::WorkingDirectoryNotFound => "working_directory_not_found",
|
||||
Self::WorkingDirectoryNotActive => "working_directory_not_active",
|
||||
Self::WorkingDirectoryRecordInvalid => "working_directory_record_invalid",
|
||||
Self::WorkingDirectoryAccessUnavailable => "working_directory_access_unavailable",
|
||||
}
|
||||
}
|
||||
|
||||
/// The observed failing boundary, not a dispatch or effects assertion.
|
||||
pub const fn stage(self) -> WorkdirFailureStage {
|
||||
use WorkdirFailureStage as Stage;
|
||||
match self {
|
||||
Self::SessionClosed => Stage::Session,
|
||||
Self::ProviderUnavailable => Stage::Provider,
|
||||
Self::CommandTerminalMissing => Stage::Command,
|
||||
Self::HttpTimeout | Self::HttpConnect | Self::HttpTransfer => Stage::Http,
|
||||
Self::HttpResponseInvalid
|
||||
| Self::HttpResponseTooLarge
|
||||
| Self::OperationResultMismatch => Stage::ResponseValidation,
|
||||
Self::SessionIdentityMismatch
|
||||
| Self::SessionOpenTaskFailed
|
||||
| Self::SessionOpenFailed
|
||||
| Self::WorkingDirectoryNotFound
|
||||
| Self::WorkingDirectoryNotActive
|
||||
| Self::WorkingDirectoryRecordInvalid
|
||||
| Self::WorkingDirectoryAccessUnavailable => Stage::SessionOpen,
|
||||
Self::AuthorizationMissing
|
||||
| Self::AuthorizationStale
|
||||
| Self::AuthorizationUnavailable
|
||||
| Self::SigningIdentityUnavailable
|
||||
| Self::AuthorizationSignFailed => Stage::AuthorizationPreparation,
|
||||
Self::RuntimeStopped
|
||||
| Self::RuntimeStateUnavailable
|
||||
| Self::RuntimeBackendUnavailable
|
||||
| Self::RuntimeOwnerMismatch
|
||||
| Self::RuntimeInvalidRequest => Stage::Runtime,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Internal operational boundary derived only from a bounded reason.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub enum WorkdirFailureStage {
|
||||
Session,
|
||||
Provider,
|
||||
Command,
|
||||
Http,
|
||||
ResponseValidation,
|
||||
SessionOpen,
|
||||
AuthorizationPreparation,
|
||||
Runtime,
|
||||
}
|
||||
|
||||
impl WorkdirFailureStage {
|
||||
pub const fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::Session => "session",
|
||||
Self::Provider => "provider",
|
||||
Self::Command => "command",
|
||||
Self::Http => "http",
|
||||
Self::ResponseValidation => "response_validation",
|
||||
Self::SessionOpen => "session_open",
|
||||
Self::AuthorizationPreparation => "authorization_preparation",
|
||||
Self::Runtime => "runtime",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -83,3 +83,34 @@ fn provider_cannot_inject_free_text_failure_reason() {
|
||||
.is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_authored_operational_messages_never_enter_error_display_or_debug() {
|
||||
for code in [
|
||||
Code::Conflict,
|
||||
Code::OutcomeUnknown,
|
||||
Code::Unsupported,
|
||||
Code::InvalidRequest,
|
||||
Code::Unavailable,
|
||||
Code::Io,
|
||||
Code::Transport,
|
||||
Code::Internal,
|
||||
] {
|
||||
let wire = serde_json::json!({
|
||||
"code": code, "message":"/host/private command-secret body-secret proof-secret key-secret Authorization: Bearer bearer-secret",
|
||||
"failure_reason": "session_open_task_failed"
|
||||
});
|
||||
let decoded: WorkdirTransportError = serde_json::from_value(wire).unwrap();
|
||||
let error = decoded.into_workdir_error();
|
||||
for text in [
|
||||
format!("{error}"),
|
||||
format!("{error:?}"),
|
||||
serde_json::to_string(&WorkdirTransportError::from_workdir_error(&error)).unwrap(),
|
||||
] {
|
||||
assert!(!text.contains("secret"), "{text}");
|
||||
assert!(!text.contains("/host/private"), "{text}");
|
||||
}
|
||||
assert_eq!(error.failure_reason(), Some(Reason::SessionOpenTaskFailed));
|
||||
assert_eq!(WorkdirTransportError::from_workdir_error(&error).code, code);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -216,3 +216,106 @@ async fn remote_checkout_diagnostics_preserve_mutation_outcome_and_read_failures
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mismatched_http_results_keep_cause_and_mutation_uncertainty() {
|
||||
use workdir::{
|
||||
CheckoutOperation, CheckoutRequest, CommandHandle, CommandOutputRequest,
|
||||
WorkdirFailureReason as Failure,
|
||||
};
|
||||
for case in ["variant", "bytes", "mutation", "identity"] {
|
||||
let listener = TcpListener::bind("127.0.0.1:0").unwrap();
|
||||
let address = listener.local_addr().unwrap();
|
||||
let server = thread::spawn(move || {
|
||||
reply(
|
||||
&listener,
|
||||
"200 OK",
|
||||
if case == "identity" {
|
||||
r#"{"session_id":"s","workdir_id":"wrong-secret","capabilities":{"bits":63}}"#
|
||||
} else {
|
||||
r#"{"session_id":"s","workdir_id":"w","capabilities":{"bits":63}}"#
|
||||
},
|
||||
);
|
||||
if case != "identity" {
|
||||
let result = if case == "bytes" {
|
||||
serde_json::json!({"operation":"command_output","result":{
|
||||
"chunks": [], "next_cursor": 123, "terminal": null
|
||||
}})
|
||||
} else {
|
||||
serde_json::to_value(
|
||||
workdir::http::WorkdirSessionOperationResult::CommandCancel,
|
||||
)
|
||||
.unwrap()
|
||||
};
|
||||
reply(&listener, "200 OK", &result.to_string());
|
||||
}
|
||||
});
|
||||
let opened = RemoteWorkdirSession::open(
|
||||
reqwest::Client::builder()
|
||||
.no_proxy()
|
||||
.timeout(Duration::from_secs(5))
|
||||
.build()
|
||||
.unwrap(),
|
||||
format!("http://{address}").parse().unwrap(),
|
||||
"request-secret",
|
||||
Workdir::new("w").id().clone(),
|
||||
OpenWorkdirSessionRequest::default(),
|
||||
)
|
||||
.await;
|
||||
let error = if case == "identity" {
|
||||
opened.unwrap_err()
|
||||
} else {
|
||||
let remote = opened.unwrap();
|
||||
match case {
|
||||
"variant" => remote
|
||||
.stat(StatRequest {
|
||||
path: WorkdirPath::new("file-secret").unwrap(),
|
||||
})
|
||||
.await
|
||||
.unwrap_err(),
|
||||
"bytes" => remote
|
||||
.command_output(CommandOutputRequest {
|
||||
handle: CommandHandle("command-secret".into()),
|
||||
cursor: 0,
|
||||
limit: 16,
|
||||
wait: false,
|
||||
})
|
||||
.await
|
||||
.unwrap_err(),
|
||||
"mutation" => remote
|
||||
.checkout_execute(CheckoutRequest {
|
||||
target: WorkdirPath::new("file-secret").unwrap(),
|
||||
validator: vec![1],
|
||||
operation: CheckoutOperation::Write {
|
||||
content: b"body-secret".to_vec(),
|
||||
expected_hash: [0; 32],
|
||||
},
|
||||
})
|
||||
.await
|
||||
.unwrap_err(),
|
||||
_ => unreachable!(),
|
||||
}
|
||||
};
|
||||
server.join().unwrap();
|
||||
let expected = if case == "identity" {
|
||||
Failure::SessionIdentityMismatch
|
||||
} else {
|
||||
Failure::OperationResultMismatch
|
||||
};
|
||||
assert_eq!(error.failure_reason(), Some(expected), "{case}: {error:?}");
|
||||
let transport = WorkdirTransportError::from_workdir_error(&error);
|
||||
assert_eq!(
|
||||
transport.code,
|
||||
if case == "mutation" {
|
||||
Code::OutcomeUnknown
|
||||
} else {
|
||||
Code::Unavailable
|
||||
}
|
||||
);
|
||||
let wire = serde_json::to_string(&transport).unwrap();
|
||||
assert!(!wire.contains("secret"), "{wire}");
|
||||
assert!(!format!("{error:?}").contains("secret"));
|
||||
let forwarded = transport.into_workdir_error();
|
||||
assert_eq!(forwarded.failure_reason(), Some(expected));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user