chore: merge current develop into T-556
This commit is contained in:
@@ -107,6 +107,31 @@ pub enum WorkdirTransportErrorCode {
|
||||
Internal,
|
||||
}
|
||||
|
||||
impl WorkdirTransportErrorCode {
|
||||
pub const fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::NotFound => "not_found",
|
||||
Self::Conflict => "conflict",
|
||||
Self::Unsupported => "unsupported",
|
||||
Self::InvalidRequest => "invalid_request",
|
||||
Self::UnknownCommand => "unknown_command",
|
||||
Self::Unavailable => "unavailable",
|
||||
Self::Internal => "internal",
|
||||
}
|
||||
}
|
||||
|
||||
/// Shared public HTTP classification for Runtime and Workspace Workdir operation boundaries.
|
||||
pub const fn http_status(self) -> u16 {
|
||||
match self {
|
||||
Self::NotFound | Self::UnknownCommand => 404,
|
||||
Self::Conflict => 409,
|
||||
Self::Unsupported | Self::InvalidRequest => 400,
|
||||
Self::Unavailable => 503,
|
||||
Self::Internal => 500,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct WorkdirTransportError {
|
||||
pub code: WorkdirTransportErrorCode,
|
||||
@@ -126,6 +151,9 @@ impl WorkdirTransportError {
|
||||
message: format!("Workdir capability {capability:?} is not available"),
|
||||
};
|
||||
}
|
||||
WorkdirError::UnsupportedOperation(_) => {
|
||||
(Code::Unsupported, "Workdir operation is not supported")
|
||||
}
|
||||
WorkdirError::UnknownCommand(_) => {
|
||||
(Code::UnknownCommand, "Workdir command was not found")
|
||||
}
|
||||
@@ -161,7 +189,7 @@ impl WorkdirTransportError {
|
||||
match self.code {
|
||||
Code::NotFound => WorkdirError::NotFound("<remote>".into()),
|
||||
Code::Conflict => WorkdirError::Conflict(self.message),
|
||||
Code::Unsupported => WorkdirError::Unavailable(self.message),
|
||||
Code::Unsupported => WorkdirError::UnsupportedOperation(self.message),
|
||||
Code::UnknownCommand => WorkdirError::UnknownCommand("<remote>".to_string()),
|
||||
Code::InvalidRequest => WorkdirError::InvalidArgument(self.message),
|
||||
Code::Unavailable => WorkdirError::Unavailable(self.message),
|
||||
@@ -517,13 +545,13 @@ mod client {
|
||||
.json::<WorkdirTransportError>()
|
||||
.await
|
||||
.map(WorkdirTransportError::into_workdir_error)
|
||||
.unwrap_or_else(|error| {
|
||||
WorkdirError::Unavailable(format!("Runtime HTTP error: {error}"))
|
||||
.unwrap_or_else(|_| {
|
||||
WorkdirError::Transport("Runtime Workdir error response was invalid".to_string())
|
||||
})
|
||||
}
|
||||
|
||||
fn http_unavailable(error: reqwest::Error) -> WorkdirError {
|
||||
WorkdirError::Unavailable(format!("Runtime Workdir HTTP request failed: {error}"))
|
||||
fn http_unavailable(_error: reqwest::Error) -> WorkdirError {
|
||||
WorkdirError::Transport("Runtime Workdir HTTP request failed".to_string())
|
||||
}
|
||||
|
||||
pub use self::RemoteWorkdirSession as ClientSession;
|
||||
@@ -536,6 +564,57 @@ pub use client::{ClientSession as RemoteWorkdirSession, WorkdirHttpAuthorization
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn transport_error_round_trip_keeps_public_classification() {
|
||||
for (code, expected_status, expected_error) in [
|
||||
(
|
||||
WorkdirTransportErrorCode::InvalidRequest,
|
||||
400,
|
||||
"invalid argument",
|
||||
),
|
||||
(WorkdirTransportErrorCode::NotFound, 404, "file not found"),
|
||||
(
|
||||
WorkdirTransportErrorCode::UnknownCommand,
|
||||
404,
|
||||
"unknown Workdir session command",
|
||||
),
|
||||
(
|
||||
WorkdirTransportErrorCode::Conflict,
|
||||
409,
|
||||
"modified externally",
|
||||
),
|
||||
(WorkdirTransportErrorCode::Unsupported, 400, "unsupported"),
|
||||
(WorkdirTransportErrorCode::Unavailable, 503, "unavailable"),
|
||||
(WorkdirTransportErrorCode::Internal, 500, "transport failed"),
|
||||
] {
|
||||
let transport = WorkdirTransportError {
|
||||
code,
|
||||
message: "safe provider message".to_string(),
|
||||
};
|
||||
assert_eq!(code.http_status(), expected_status);
|
||||
let workdir_error = transport.clone().into_workdir_error();
|
||||
assert!(workdir_error.to_string().contains(expected_error));
|
||||
assert_eq!(
|
||||
WorkdirTransportError::from_workdir_error(&workdir_error).code,
|
||||
code
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn local_validation_errors_share_invalid_request_classification() {
|
||||
for error in [
|
||||
WorkdirError::InvalidGlob("[".to_string()),
|
||||
WorkdirError::InvalidRegex("(".to_string()),
|
||||
WorkdirError::InvalidArgument("limit must be positive".to_string()),
|
||||
] {
|
||||
let transport = WorkdirTransportError::from_workdir_error(&error);
|
||||
assert_eq!(transport.code, WorkdirTransportErrorCode::InvalidRequest);
|
||||
assert_eq!(transport.code.http_status(), 400);
|
||||
assert_eq!(transport.message, "Workdir operation request is invalid");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn transport_failure_remains_distinct_from_session_unavailable() {
|
||||
let transport = WorkdirTransportError::from_workdir_error(&WorkdirError::Transport(
|
||||
|
||||
@@ -225,6 +225,9 @@ pub enum WorkdirError {
|
||||
#[error("Workdir session does not support {0:?}")]
|
||||
Unsupported(WorkdirSessionCapability),
|
||||
|
||||
#[error("Workdir operation is unsupported: {0}")]
|
||||
UnsupportedOperation(String),
|
||||
|
||||
#[error("invalid Workdir path: {0}")]
|
||||
InvalidPath(String),
|
||||
|
||||
|
||||
@@ -1721,17 +1721,8 @@ impl RuntimeHttpWorkdirError {
|
||||
impl From<workdir::WorkdirError> for RuntimeHttpWorkdirError {
|
||||
fn from(error: workdir::WorkdirError) -> Self {
|
||||
let payload = WorkdirTransportError::from_workdir_error(&error);
|
||||
let status = match payload.code {
|
||||
WorkdirTransportErrorCode::NotFound | WorkdirTransportErrorCode::UnknownCommand => {
|
||||
StatusCode::NOT_FOUND
|
||||
}
|
||||
WorkdirTransportErrorCode::Conflict => StatusCode::CONFLICT,
|
||||
WorkdirTransportErrorCode::Unsupported | WorkdirTransportErrorCode::InvalidRequest => {
|
||||
StatusCode::BAD_REQUEST
|
||||
}
|
||||
WorkdirTransportErrorCode::Unavailable => StatusCode::SERVICE_UNAVAILABLE,
|
||||
WorkdirTransportErrorCode::Internal => StatusCode::INTERNAL_SERVER_ERROR,
|
||||
};
|
||||
let status = StatusCode::from_u16(payload.code.http_status())
|
||||
.expect("Workdir transport error status is valid");
|
||||
Self { status, payload }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -195,11 +195,16 @@ impl WorkspaceAttachedWorkdirSession {
|
||||
.execute(request)
|
||||
.map_err(workspace_workdir_error)?;
|
||||
if !response.is_success() {
|
||||
return Err(WorkdirError::Transport(format!(
|
||||
"Workspace Workdir API returned HTTP {}: {}",
|
||||
response.status,
|
||||
bounded_error_body(&response.body)
|
||||
)));
|
||||
return Err(
|
||||
serde_json::from_str::<workdir::http::WorkdirTransportError>(&response.body)
|
||||
.map(workdir::http::WorkdirTransportError::into_workdir_error)
|
||||
.unwrap_or_else(|_| {
|
||||
WorkdirError::Transport(format!(
|
||||
"Workspace Workdir operation failed with HTTP {}",
|
||||
response.status
|
||||
))
|
||||
}),
|
||||
);
|
||||
}
|
||||
serde_json::from_str(&response.body).map_err(|error| {
|
||||
WorkdirError::Transport(format!(
|
||||
@@ -795,6 +800,21 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
fn error_response(
|
||||
status: u16,
|
||||
code: workdir::http::WorkdirTransportErrorCode,
|
||||
message: &str,
|
||||
) -> WorkspaceResponse {
|
||||
WorkspaceResponse {
|
||||
status,
|
||||
body: serde_json::to_string(&workdir::http::WorkdirTransportError {
|
||||
code,
|
||||
message: message.to_string(),
|
||||
})
|
||||
.unwrap(),
|
||||
}
|
||||
}
|
||||
|
||||
fn workdir_json(id: &str) -> serde_json::Value {
|
||||
json!({
|
||||
"working_directory_id": id,
|
||||
@@ -1175,6 +1195,48 @@ mod tests {
|
||||
assert_eq!(validation["delegations"].as_array().unwrap().len(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn attached_session_preserves_typed_provider_validation_error() {
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![error_response(
|
||||
400,
|
||||
workdir::http::WorkdirTransportErrorCode::InvalidRequest,
|
||||
"Workdir operation request is invalid",
|
||||
)]));
|
||||
let session = WorkspaceAttachedWorkdirSession::handle(client);
|
||||
|
||||
let error = session
|
||||
.glob(workdir::GlobRequest {
|
||||
pattern: "[".to_string(),
|
||||
path: workdir::WorkdirPath::root(),
|
||||
limit: 10,
|
||||
})
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert!(matches!(error, WorkdirError::InvalidArgument(_)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn attached_session_does_not_expose_untyped_workspace_error_body() {
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![WorkspaceResponse {
|
||||
status: 502,
|
||||
body: "secret token and /host/private/path".to_string(),
|
||||
}]));
|
||||
let session = WorkspaceAttachedWorkdirSession::handle(client);
|
||||
|
||||
let error = session
|
||||
.stat(StatRequest {
|
||||
path: workdir::WorkdirPath::root(),
|
||||
})
|
||||
.await
|
||||
.unwrap_err();
|
||||
let message = error.to_string();
|
||||
assert!(matches!(error, WorkdirError::Transport(_)));
|
||||
assert!(!message.contains("secret token"));
|
||||
assert!(!message.contains("/host/private/path"));
|
||||
assert!(message.contains("HTTP 502"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn nested_attached_session_preserves_full_delegation_chain() {
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![
|
||||
|
||||
@@ -44,7 +44,9 @@ use webauthn_rs::prelude::{
|
||||
PublicKeyCredential, RegisterPublicKeyCredential, RequestChallengeResponse, Webauthn,
|
||||
WebauthnBuilder,
|
||||
};
|
||||
use workdir::http::{WorkdirSessionOperation, WorkdirSessionOperationResult};
|
||||
use workdir::http::{
|
||||
WorkdirSessionOperation, WorkdirSessionOperationResult, WorkdirTransportError,
|
||||
};
|
||||
use workdir::workspace::{
|
||||
MaterializerKind, WorkingDirectoryCleanupTarget,
|
||||
WorkingDirectoryDetailResponse as BrowserWorkingDirectoryDetailResponse,
|
||||
@@ -6932,12 +6934,56 @@ fn validated_current_worker_attachment(
|
||||
Ok(link)
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
enum WorkdirOperationApiError {
|
||||
Api(ApiError),
|
||||
Provider(WorkdirTransportError),
|
||||
}
|
||||
|
||||
impl From<ApiError> for WorkdirOperationApiError {
|
||||
fn from(error: ApiError) -> Self {
|
||||
Self::Api(error)
|
||||
}
|
||||
}
|
||||
|
||||
impl From<Error> for WorkdirOperationApiError {
|
||||
fn from(error: Error) -> Self {
|
||||
Self::Api(error.into())
|
||||
}
|
||||
}
|
||||
|
||||
impl From<WorkdirTransportError> for WorkdirOperationApiError {
|
||||
fn from(error: WorkdirTransportError) -> Self {
|
||||
Self::Provider(error)
|
||||
}
|
||||
}
|
||||
|
||||
impl IntoResponse for WorkdirOperationApiError {
|
||||
fn into_response(self) -> Response {
|
||||
match self {
|
||||
Self::Api(error) => error.into_response(),
|
||||
Self::Provider(error) => {
|
||||
let status = StatusCode::from_u16(error.code.http_status())
|
||||
.expect("Workdir transport error status is valid");
|
||||
let log = ApiErrorLog {
|
||||
kind: format!("workdir_session_operation_{}", error.code.as_str()),
|
||||
message: error.message.clone(),
|
||||
diagnostics: Vec::new(),
|
||||
};
|
||||
let mut response = (status, Json(error)).into_response();
|
||||
response.extensions_mut().insert(log);
|
||||
response
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn scoped_execute_current_worker_workdir_operation(
|
||||
State(api): State<WorkspaceApi>,
|
||||
AxumPath(path): AxumPath<ScopedWorkspacePath>,
|
||||
headers: HeaderMap,
|
||||
Json(request): Json<WorkspaceWorkdirSessionOperationRequest>,
|
||||
) -> ApiResult<Json<WorkdirSessionOperationResult>> {
|
||||
) -> std::result::Result<Json<WorkdirSessionOperationResult>, WorkdirOperationApiError> {
|
||||
validate_workspace_scope(&api, &path.workspace_id)?;
|
||||
let worker = current_worker_identity(&api, &path.workspace_id, &headers)?;
|
||||
let expected_session_fence = request.expected_session_fence;
|
||||
@@ -7076,7 +7122,8 @@ async fn current_worker_command_session(
|
||||
external_handle: &CommandHandle,
|
||||
delegations: &[workdir::WorkdirDelegationRequest],
|
||||
expected_session_fence: Option<&str>,
|
||||
) -> ApiResult<(workdir::AppliedWorkdirDelegation, CommandHandle)> {
|
||||
) -> std::result::Result<(workdir::AppliedWorkdirDelegation, CommandHandle), WorkdirOperationApiError>
|
||||
{
|
||||
let _link = validated_current_worker_attachment(api, worker, expected_session_fence)?;
|
||||
let command = api
|
||||
.workdir_sessions
|
||||
@@ -7084,7 +7131,7 @@ async fn current_worker_command_session(
|
||||
.expect("Workdir session registry lock poisoned")
|
||||
.command(worker, external_handle)
|
||||
.ok_or_else(|| {
|
||||
ApiError::from(current_worker_workdir_operation_error(
|
||||
WorkdirOperationApiError::Provider(current_worker_workdir_operation_error(
|
||||
worker,
|
||||
workdir::WorkdirError::UnknownCommand(external_handle.0.clone()),
|
||||
))
|
||||
@@ -7101,14 +7148,10 @@ async fn current_worker_command_session(
|
||||
}
|
||||
|
||||
fn current_worker_workdir_operation_error(
|
||||
worker: &RuntimeWorkerRef,
|
||||
_worker: &RuntimeWorkerRef,
|
||||
error: workdir::WorkdirError,
|
||||
) -> Error {
|
||||
Error::RuntimeOperationFailed {
|
||||
runtime_id: worker.runtime_id.clone(),
|
||||
code: "workdir_session_operation_failed".to_string(),
|
||||
message: error.to_string(),
|
||||
}
|
||||
) -> WorkdirTransportError {
|
||||
WorkdirTransportError::from_workdir_error(&error)
|
||||
}
|
||||
|
||||
async fn execute_workdir_session_operation(
|
||||
@@ -15737,6 +15780,107 @@ mod tests {
|
||||
assert_eq!(conflict.status(), StatusCode::CONFLICT);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn workdir_operation_errors_preserve_typed_public_status_and_code() {
|
||||
for (code, expected_status) in [
|
||||
(
|
||||
workdir::http::WorkdirTransportErrorCode::InvalidRequest,
|
||||
StatusCode::BAD_REQUEST,
|
||||
),
|
||||
(
|
||||
workdir::http::WorkdirTransportErrorCode::NotFound,
|
||||
StatusCode::NOT_FOUND,
|
||||
),
|
||||
(
|
||||
workdir::http::WorkdirTransportErrorCode::UnknownCommand,
|
||||
StatusCode::NOT_FOUND,
|
||||
),
|
||||
(
|
||||
workdir::http::WorkdirTransportErrorCode::Conflict,
|
||||
StatusCode::CONFLICT,
|
||||
),
|
||||
(
|
||||
workdir::http::WorkdirTransportErrorCode::Unsupported,
|
||||
StatusCode::BAD_REQUEST,
|
||||
),
|
||||
(
|
||||
workdir::http::WorkdirTransportErrorCode::Unavailable,
|
||||
StatusCode::SERVICE_UNAVAILABLE,
|
||||
),
|
||||
(
|
||||
workdir::http::WorkdirTransportErrorCode::Internal,
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
),
|
||||
] {
|
||||
let response = WorkdirOperationApiError::Provider(WorkdirTransportError {
|
||||
code,
|
||||
message: "safe provider message".to_string(),
|
||||
})
|
||||
.into_response();
|
||||
assert_eq!(response.status(), expected_status);
|
||||
let log = response.extensions().get::<ApiErrorLog>().unwrap();
|
||||
assert_eq!(
|
||||
log.kind,
|
||||
format!("workdir_session_operation_{}", code.as_str())
|
||||
);
|
||||
let body = to_bytes(response.into_body(), usize::MAX).await.unwrap();
|
||||
let decoded: WorkdirTransportError = serde_json::from_slice(&body).unwrap();
|
||||
assert_eq!(decoded.code, code);
|
||||
assert_eq!(decoded.message, "safe provider message");
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn local_and_remote_validation_errors_share_workspace_classification() {
|
||||
let worker = RuntimeWorkerRef::new("runtime", "worker");
|
||||
let local = current_worker_workdir_operation_error(
|
||||
&worker,
|
||||
workdir::WorkdirError::InvalidGlob("[".to_string()),
|
||||
);
|
||||
let remote = current_worker_workdir_operation_error(
|
||||
&worker,
|
||||
WorkdirTransportError {
|
||||
code: workdir::http::WorkdirTransportErrorCode::InvalidRequest,
|
||||
message: "Workdir operation request is invalid".to_string(),
|
||||
}
|
||||
.into_workdir_error(),
|
||||
);
|
||||
assert_eq!(local, remote);
|
||||
|
||||
for public in [local, remote] {
|
||||
let response = WorkdirOperationApiError::Provider(public).into_response();
|
||||
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
|
||||
let body = to_bytes(response.into_body(), usize::MAX).await.unwrap();
|
||||
let decoded: WorkdirTransportError = serde_json::from_slice(&body).unwrap();
|
||||
assert_eq!(
|
||||
decoded.code,
|
||||
workdir::http::WorkdirTransportErrorCode::InvalidRequest
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn workdir_operation_error_response_redacts_provider_internal_details() {
|
||||
let error = workdir::WorkdirError::Io {
|
||||
path: PathBuf::from("/host/private/worktree/secret.txt"),
|
||||
source: std::io::Error::new(std::io::ErrorKind::PermissionDenied, "token=secret"),
|
||||
};
|
||||
let public = current_worker_workdir_operation_error(
|
||||
&RuntimeWorkerRef::new("runtime", "worker"),
|
||||
error,
|
||||
);
|
||||
let response = WorkdirOperationApiError::Provider(public).into_response();
|
||||
let body = to_bytes(response.into_body(), usize::MAX).await.unwrap();
|
||||
let text = String::from_utf8(body.to_vec()).unwrap();
|
||||
assert!(!text.contains("/host/private"));
|
||||
assert!(!text.contains("token=secret"));
|
||||
let decoded: WorkdirTransportError = serde_json::from_str(&text).unwrap();
|
||||
assert_eq!(
|
||||
decoded.code,
|
||||
workdir::http::WorkdirTransportErrorCode::Internal
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn backend_worker_projection_preserves_missing_rows_links_and_redacts_paths() {
|
||||
let worker = WorkerRegistryRecord {
|
||||
|
||||
Reference in New Issue
Block a user