fix: preserve external workdir session failure diagnostics

Retain allowlisted Backend failure codes in operation logs without exposing diagnostic prose. Distinguish missing provider connections from mismatched grant bindings while preserving rejection and HTTP status behavior.

Validate signed read/command failure logs, no-dispatch and attachment retention, and unknown-code redaction. Targeted logging tests, restore regression, root check and static checks pass. Broad Server validation remains incomplete due to shard timeouts and observed Plugin/restore-guard failures.
This commit is contained in:
2026-11-01 00:56:41 +09:00
parent f22b69bb92
commit bfbddfa443
4 changed files with 290 additions and 11 deletions
+44 -10
View File
@@ -21944,16 +21944,22 @@ async fn open_current_worker_workdir_session_locked(
.lock() .lock()
.expect("External Workdir provider registry poisoned") .expect("External Workdir provider registry poisoned")
.connection(grant_id) .connection(grant_id)
.filter(|connection| {
connection.generation == grant.generation
&& connection.provider_instance_id == grant.provider_instance_id
&& connection.workdir_id == workdir.workdir_id
})
.ok_or_else(|| Error::RuntimeOperationFailed { .ok_or_else(|| Error::RuntimeOperationFailed {
runtime_id: "workspace-backend".to_string(), runtime_id: "workspace-backend".to_string(),
code: "external_workdir_provider_offline".to_string(), code: "external_workdir_provider_connection_missing".to_string(),
message: "External Workdir provider connection is unavailable".to_string(), message: "External Workdir provider connection is unavailable".to_string(),
})?; })?;
if connection.generation != grant.generation
|| connection.provider_instance_id != grant.provider_instance_id
|| connection.workdir_id != workdir.workdir_id
{
return Err(Error::RuntimeOperationFailed {
runtime_id: "workspace-backend".to_string(),
code: "external_workdir_provider_identity_mismatch".to_string(),
message: "External Workdir provider connection does not match the current grant"
.to_string(),
});
}
let source: WorkdirSessionHandle = Arc::new(ExternalProviderWorkdirSession::new( let source: WorkdirSessionHandle = Arc::new(ExternalProviderWorkdirSession::new(
connection, connection,
Some(ExternalProviderAuditContext { Some(ExternalProviderAuditContext {
@@ -39386,10 +39392,38 @@ impl ApiErrorLog {
log.denial_reason = error.denial_reason; log.denial_reason = error.denial_reason;
log.failure_reason = error.failure_reason; log.failure_reason = error.failure_reason;
if log.workdir_log.is_some() { if log.workdir_log.is_some() {
// Public classification and arbitrary diagnostic text are not safe // Keep bounded Backend codes as well as typed provider reasons. Never
// operational causes. Keep only the separately typed reason/stage; // log arbitrary codes/messages or infer causes from diagnostic prose.
// never infer a reason by parsing request- or provider-authored text. const BACKEND_CODES: &[&str] = &[
log.kind = format!("workdir_session_operation_api_{}", status.as_u16()); "db_capacity_exceeded",
"db_executor_unavailable",
"db_operation_outcome_unknown",
"working_directory_not_found",
"external_workdir_grant_not_found",
"external_workdir_provider_pending",
"external_workdir_provider_offline",
"external_workdir_provider_revoked",
"external_workdir_provider_expired",
"external_workdir_provider_unavailable",
"external_workdir_provider_connection_missing",
"external_workdir_provider_identity_mismatch",
"duplicate_workdir_session_close_failed",
"workdir_session_registration_failed",
];
let backend_code = error
.diagnostics
.as_deref()
.and_then(|diagnostics| diagnostics.first())
.and_then(|diagnostic| {
BACKEND_CODES
.iter()
.copied()
.find(|code| diagnostic.code == *code)
});
log.kind = match backend_code {
Some(code) => format!("workdir_session_operation_{code}"),
None => format!("workdir_session_operation_api_{}", status.as_u16()),
};
// This can also fail after a command was dispatched. Do not claim // This can also fail after a command was dispatched. Do not claim
// a pre-dispatch refusal or absence of effects in the safe message. // a pre-dispatch refusal or absence of effects in the safe message.
log.message = "Workdir operation failed".to_string(); log.message = "Workdir operation failed".to_string();
@@ -1,3 +1,5 @@
mod workdir_open;
use super::*; use super::*;
use std::io::Write; use std::io::Write;
use tracing::instrument::WithSubscriber; use tracing::instrument::WithSubscriber;
@@ -0,0 +1,243 @@
//! Signed operations -> real attachment/grant SQLite records -> session opening
//! -> API error logging. Provider connections use an unconsumed channel: rejected
//! opens must not dispatch commands or reads. No live external provider is used.
use super::*;
#[tokio::test]
async fn external_session_open_logs_distinguish_unavailable_and_mismatched_connections() {
for (state, mismatch, code) in [
("pending", None, "external_workdir_provider_pending"),
("offline", None, "external_workdir_provider_offline"),
(
"online",
None,
"external_workdir_provider_connection_missing",
),
(
"online",
Some("generation"),
"external_workdir_provider_identity_mismatch",
),
(
"online",
Some("provider"),
"external_workdir_provider_identity_mismatch",
),
(
"online",
Some("workdir"),
"external_workdir_provider_identity_mismatch",
),
] {
let mut fixture = manual_worker_assignment_fixture().await;
let identity = RuntimeIdentityMaterial::generate(&fixture.worker.runtime_id).unwrap();
configure_runtime_request_auth(&mut fixture.api, &identity, &fixture.worker.runtime_id);
let grant = ExternalWorkdirGrantRecord {
grant_id: "logging-external-grant".into(),
workspace_id: TEST_WORKSPACE_ID.into(),
workdir_id: "logging-external-workdir".into(),
provider_instance_id: "logging-provider".into(),
display_name: "body-secret".into(),
permissions: "read_write_command".into(),
created_by: test_browser_request_actor().account_id,
created_at: TEST_CREATED_AT.into(),
expires_at: None,
generation: 1,
status: state.into(),
updated_at: TEST_CREATED_AT.into(),
};
let mut record = fixture
.api
.store
.list_workdir_registry(TEST_WORKSPACE_ID, 100)
.unwrap()
.into_iter()
.find(|record| record.workdir_id == fixture.main_workdir_id)
.unwrap();
record.workdir_id = grant.workdir_id.clone();
record.source = WorkdirRegistrySource::ExternalGrant {
grant_id: grant.grant_id.clone(),
};
fixture
.api
.store
.create_external_workdir_grant(&grant, &record)
.unwrap();
fixture
.api
.store
.attach_worker_workdir(&WorkerWorkdirLinkRecord {
connection_id: String::new(),
workspace_id: TEST_WORKSPACE_ID.into(),
worker: fixture.worker.clone(),
workdir_id: grant.workdir_id.clone(),
alias: "external".into(),
capabilities: workdir::WorkdirSessionCapabilities::ALL,
linked_at: TEST_CREATED_AT.into(),
unlinked_at: None,
})
.unwrap();
let (sender, mut operations) = tokio::sync::mpsc::channel(1);
if let Some(mismatch) = mismatch {
let connection = ExternalProviderConnection {
grant_id: grant.grant_id.clone(),
workdir_id: if mismatch == "workdir" {
"other-workdir".into()
} else {
grant.workdir_id.clone()
},
provider_instance_id: if mismatch == "provider" {
"other-provider".into()
} else {
grant.provider_instance_id.clone()
},
generation: if mismatch == "generation" {
2
} else {
grant.generation
},
expires_at: None,
capabilities: workdir::WorkdirSessionCapabilities::ALL,
admission: Arc::new(tokio::sync::Semaphore::new(1)),
shutdown_confirmed: Arc::new(std::sync::atomic::AtomicBool::new(false)),
sender,
};
assert!(
fixture
.api
.external_workdir_providers
.lock()
.unwrap()
.connected(Arc::new(connection))
.is_none()
);
}
let links = fixture
.api
.store
.list_worker_workdir_links(TEST_WORKSPACE_ID, &fixture.worker)
.unwrap();
let app = build_router(fixture.api.clone());
let path = format!("/api/w/{TEST_WORKSPACE_ID}/workers/self/workdir-session/operations");
for operation in [
json!({"operation": "command_start", "request": {
"command": "body-secret", "cwd": "", "timeout_secs": 1,
"transport_byte_budget": 1048576, "tool_call_id": "proof-secret"
}}),
json!({"operation": "read", "request": {
"path": "README.md", "offset": 0, "limit": 10, "max_bytes": 1024
}}),
] {
let request = runtime_source_request(
&identity,
Some(&fixture.worker.worker_id),
"POST",
&path,
serde_json::to_vec(&json!({"target_workdir": "external", "operation": operation}))
.unwrap(),
);
let (status, body, events) = request_with_logs(app.clone(), request).await;
assert_eq!(
status,
StatusCode::BAD_GATEWAY,
"{state}/{mismatch:?}: {body}"
);
assert!(
body["message"]
.as_str()
.unwrap()
.starts_with(&format!("{code}:")),
"{body}"
);
assert_eq!(events.len(), 1, "{events:?}");
assert_eq!(
events[0]["kind"],
format!("workdir_session_operation_{code}")
);
assert_eq!(events[0]["workdir_stage"], "session_open");
assert_eq!(events[0]["operation"], operation["operation"]);
assert_eq!(events[0]["message"], "Workdir operation failed");
assert_eq!(events[0]["diagnostics"], "[]");
assert_eq!(events[0]["failure_reason"], Value::Null);
assert_workdir_log_identity(&events[0], &fixture, "external", Some(&grant.workdir_id));
assert!(
operations.try_recv().is_err(),
"failed open dispatched an operation"
);
assert_eq!(
fixture
.api
.store
.list_worker_workdir_links(TEST_WORKSPACE_ID, &fixture.worker)
.unwrap(),
links
);
}
}
}
#[test]
fn workdir_backend_diagnostics_keep_only_known_codes_not_messages_or_unknown_codes() {
for (code, expected) in [
(
"external_workdir_provider_revoked",
"workdir_session_operation_external_workdir_provider_revoked",
),
(
"external_workdir_provider_expired",
"workdir_session_operation_external_workdir_provider_expired",
),
(
"external_workdir_grant_not_found",
"workdir_session_operation_external_workdir_grant_not_found",
),
(
"workdir_session_registration_failed",
"workdir_session_operation_workdir_session_registration_failed",
),
(
"external_workdir_provider_body-secret",
"workdir_session_operation_api_502",
),
(
"unknown_provider_error",
"workdir_session_operation_api_502",
),
] {
let error = InternalWorkdirOperationError::Contextual {
error: Box::new(
Error::RuntimeOperationFailed {
runtime_id: "workspace-backend".into(),
code: code.into(),
message: "external_workdir_provider_offline body-secret /host/private".into(),
}
.into(),
),
context: server_api::WorkdirOperationLogContext {
operation: "command_start",
stage: "session_open",
..Default::default()
},
}
.into_contract_error();
let log = ApiErrorLog::from_workdir_operation_error(&error, StatusCode::BAD_GATEWAY);
assert_eq!(log.kind, expected);
assert_eq!(log.message, "Workdir operation failed");
assert!(log.diagnostics.is_empty());
assert!(log.failure_reason.is_none());
assert!(!format!("{log:?}").contains("secret"));
assert!(!format!("{log:?}").contains("/host/private"));
// A matching message/classification without an actual diagnostic code
// is not evidence of that Backend failure.
let mut without_diagnostics = error;
without_diagnostics.diagnostics = None;
without_diagnostics.error = Some(code.into());
without_diagnostics.message = code.into();
let log = ApiErrorLog::from_workdir_operation_error(
&without_diagnostics,
StatusCode::BAD_GATEWAY,
);
assert_eq!(log.kind, "workdir_session_operation_api_502");
}
}
@@ -167,7 +167,7 @@ async fn restore_keeps_offline_external_and_unmaterialized_repository_attachment
"external" => assert!(matches!( "external" => assert!(matches!(
error, error,
Error::RuntimeOperationFailed { code, .. } Error::RuntimeOperationFailed { code, .. }
if code == "external_workdir_provider_offline" if code == "external_workdir_provider_connection_missing"
)), )),
"repository" => assert!(matches!( "repository" => assert!(matches!(
error, error,