From f5d8c94713e3edea40003eaab221b9f6c8643ec8 Mon Sep 17 00:00:00 2001 From: Hare Date: Thu, 1 Oct 2026 15:57:05 +0900 Subject: [PATCH] fix: bind singleton retries to caller intent --- crates/workspace-server/src/hosts.rs | 6 +- crates/workspace-server/src/server.rs | 83 +++++++++++++++++++++++---- crates/workspace-server/src/store.rs | 42 ++++++++++++-- 3 files changed, 115 insertions(+), 16 deletions(-) diff --git a/crates/workspace-server/src/hosts.rs b/crates/workspace-server/src/hosts.rs index ada6f3e8e..839f945d7 100644 --- a/crates/workspace-server/src/hosts.rs +++ b/crates/workspace-server/src/hosts.rs @@ -457,7 +457,11 @@ pub struct WorkerTicketAssignmentRequest { pub(crate) fn worker_spawn_create_fingerprint( request: &WorkerSpawnRequest, ) -> Result { - let encoded = serde_json::to_vec(request) + // WorkerSpawnRequest intentionally omits Backend-resolved fields from its wire encoding. + // Existing Workdir claims are nevertheless caller-visible create intent and must be bound to + // a manual singleton retry. Trusted operation identity and ephemeral Backend launch material + // remain separate from this semantic create fingerprint. + let encoded = serde_json::to_vec(&(request, request.resolved_workdir_attachments.as_slice())) .map_err(|error| format!("serialize Worker create input: {error}"))?; Ok(format!("sha256:{}", digest_hex(&encoded, 64))) } diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 0d82d8560..0371f5a30 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -23040,6 +23040,19 @@ fn browser_worker_console_href(workspace_id: &str, resource_key: &str) -> String ) } +fn validate_caller_worker_singleton_key(singleton_key: Option<&str>) -> Result<()> { + let Some(singleton_key) = singleton_key else { + return Ok(()); + }; + crate::store::validate_worker_singleton_key(singleton_key)?; + if crate::hosts::is_reserved_internal_worker_singleton_key(singleton_key) { + return Err(Error::InvalidInput(format!( + "Worker singleton key {singleton_key:?} is reserved for a Backend-managed Worker" + ))); + } + Ok(()) +} + async fn create_workspace_worker( State(api): State, headers: HeaderMap, @@ -23064,14 +23077,7 @@ async fn create_workspace_worker_inner( workdir_attachments, control_operation_id: _, } = request; - if let Some(singleton_key) = singleton_key.as_deref() - && crate::hosts::is_reserved_internal_worker_singleton_key(singleton_key) - { - return Err(Error::InvalidInput(format!( - "Worker singleton key {singleton_key:?} is reserved for a Backend-managed Worker" - )) - .into()); - } + validate_caller_worker_singleton_key(singleton_key.as_deref())?; let config_state = api .config_store .load_workspace_config(&api.config.workspace_id)? @@ -24164,9 +24170,7 @@ async fn create_runtime_worker( Json(request): Json, ) -> ApiResult> { let mut request = worker_spawn_request_from_api(request)?; - if let Some(singleton_key) = request.singleton_key.as_deref() { - crate::store::validate_worker_singleton_key(singleton_key)?; - } + validate_caller_worker_singleton_key(request.singleton_key.as_deref())?; validate_worker_initial_submit(&request.initial_submit)?; if let Some(assignment) = request.ticket_assignment.as_ref() && let Some(worker) = existing_lifecycle_assignment_worker(&api, assignment, &runtime_id)? @@ -40288,6 +40292,63 @@ mod tests { ); } + #[tokio::test] + async fn runtime_worker_create_rejects_reserved_singleton_key_squatting_and_authority_forgery() + { + let dir = tempfile::tempdir().unwrap(); + init_clean_git_workspace(dir.path()); + let api = test_api(dir.path()).await; + let request = |requested_worker_name: &str, + intent: server_api::RuntimeWorkerSpawnIntent, + profile: &str| server_api::RuntimeWorkerSpawnRequest { + requested_worker_name: Some(requested_worker_name.to_string()), + intent, + singleton_key: Some(crate::hosts::WORKSPACE_ORCHESTRATOR_SINGLETON_KEY.to_string()), + acceptance: server_api::RuntimeWorkerSpawnAcceptanceRequirement::RunAccepted { + expected_segments: 0, + }, + profile: server_api::RuntimeProfileSelector::Builtin(profile.to_string()), + ticket_assignment: None, + initial_submit: Vec::new(), + workdir_attachment_requests: Vec::new(), + }; + + for candidate in [ + request( + "Generic key squatter", + server_api::RuntimeWorkerSpawnIntent::WorkspaceCompanion, + "builtin:companion", + ), + request( + crate::hosts::WORKSPACE_ORCHESTRATOR_SINGLETON_KEY, + server_api::RuntimeWorkerSpawnIntent::WorkspaceOrchestrator, + "builtin:orchestrator", + ), + ] { + let error = scoped_create_runtime_worker( + State(api.clone()), + AxumPath(ScopedRuntimePath { + workspace_id: TEST_WORKSPACE_ID.to_string(), + runtime_id: EMBEDDED_WORKER_RUNTIME_ID.to_string(), + }), + Json(candidate), + ) + .await + .unwrap_err(); + assert_eq!(error.into_response().status(), StatusCode::BAD_REQUEST); + } + assert!( + api.store + .current_worker_singleton_owner( + TEST_WORKSPACE_ID, + crate::hosts::WORKSPACE_ORCHESTRATOR_SINGLETON_KEY, + ) + .unwrap() + .is_none() + ); + assert!(find_workspace_orchestrator(&api).is_none()); + } + #[tokio::test] async fn runtime_worker_ticket_assignment_rejects_missing_target_attachments() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index d8bf0bc46..88c87cd6c 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -18297,7 +18297,7 @@ mod tests { } #[tokio::test] - async fn worker_singleton_reservation_retry_survives_restart_and_new_allocation_key() { + async fn worker_singleton_restart_retry_replays_exact_workdir_intent_and_rejects_changes() { let dir = tempfile::tempdir().unwrap(); let database = dir.path().join("server.db"); let store = SqliteWorkspaceStore::open(&database).unwrap(); @@ -18313,12 +18313,46 @@ mod tests { .await .unwrap(); let memory = store.get_workspace_memory_settings("workspace-a").unwrap(); + let request = |relative_cwd: Option<&str>| crate::hosts::WorkerSpawnRequest { + requested_worker_name: Some("Subject Worker".to_string()), + singleton_key: Some("subjektiv:restart-retry".to_string()), + intent: crate::hosts::WorkerSpawnIntent::WorkspaceCompanion, + acceptance: crate::hosts::WorkerSpawnAcceptanceRequirement::RunAccepted { + expected_segments: 0, + }, + profile: worker_runtime::catalog::ProfileSelector::Builtin( + "builtin:companion".to_string(), + ), + ticket_assignment: None, + initial_submit: Vec::new(), + workdir_attachment_requests: Vec::new(), + resolved_workdir_attachment_requests: Vec::new(), + resolved_workdir_attachments: vec![ + worker_runtime::catalog::WorkingDirectoryAttachmentClaim { + alias: workdir::WorkdirAttachmentAlias::new("checkout").unwrap(), + working_directory_id: "workdir-a".to_string(), + relative_cwd: relative_cwd.map(ToOwned::to_owned), + capabilities: workdir::WorkdirSessionCapabilities::READ_ONLY, + }, + ], + resolved_config_bundle: None, + resolved_worker_observation_enabled: false, + resolved_worker_observation_grants: Vec::new(), + resolved_control_operation: None, + resolved_workspace_api: None, + resolved_memory_settings: None, + }; + let stable_fingerprint = + crate::hosts::worker_spawn_create_fingerprint(&request(Some("src"))).unwrap(); + let changed_workdir_fingerprint = + crate::hosts::worker_spawn_create_fingerprint(&request(Some("tests"))).unwrap(); + assert_ne!(stable_fingerprint, changed_workdir_fingerprint); let reserved = store .reserve_worker_create( "workspace-a", "runtime-a", "manual:lost-before-dispatch", - "sha256:stable-request", + &stable_fingerprint, Some("subjektiv:restart-retry"), &memory, ) @@ -18331,7 +18365,7 @@ mod tests { "workspace-a", "runtime-a", "manual:new-after-restart", - "sha256:stable-request", + &stable_fingerprint, Some("subjektiv:restart-retry"), &memory, ) @@ -18354,7 +18388,7 @@ mod tests { "workspace-a", "runtime-a", "manual:different-request", - "sha256:different-request", + &changed_workdir_fingerprint, Some("subjektiv:restart-retry"), &memory, )