From 9d7ddcc04ad7b2fd15f44a627fe2162785a8db9a Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 9 Sep 2026 13:20:00 +0900 Subject: [PATCH] fix: enforce Workspace Runtime binding lifecycle --- crates/workspace-server/src/server.rs | 153 ++++++++++++++++----- crates/workspace-server/src/store.rs | 187 +++++++++++++++++++++++++- 2 files changed, 306 insertions(+), 34 deletions(-) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 522d23b3..136ae4ce 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -10331,11 +10331,27 @@ fn working_directory_diagnostics( .collect() } +async fn require_active_workspace_runtime_binding( + api: &WorkspaceApi, + runtime_id: &str, +) -> ApiResult<()> { + let binding = api + .store + .get_workspace_runtime_binding(&api.config.workspace_id, runtime_id) + .await? + .ok_or_else(|| Error::UnknownRuntime(runtime_id.to_string()))?; + if binding.revoked_at.is_some() { + return Err(Error::UnknownRuntime(runtime_id.to_string()).into()); + } + Ok(()) +} + async fn scoped_list_runtime_working_directories( State(api): State, AxumPath(path): AxumPath, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; + require_active_workspace_runtime_binding(&api, &path.runtime_id).await?; let (items, diagnostics) = runtime_working_directory_summaries(&api, &path.runtime_id)?; Ok(Json(BrowserWorkingDirectoryListResponse { workspace_id: api.config.workspace_id.clone(), @@ -10349,6 +10365,7 @@ async fn scoped_create_runtime_working_directory( AxumPath(path): AxumPath, Json(request): Json, ) -> ApiResult<(StatusCode, Json)> { + require_active_workspace_runtime_binding(&api, &path.runtime_id).await?; create_workspace_working_directory( &api, &path.workspace_id, @@ -10363,6 +10380,7 @@ async fn scoped_runtime_working_directory_detail( AxumPath(path): AxumPath, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; + require_active_workspace_runtime_binding(&api, &path.runtime_id).await?; working_directory_detail_for_runtime(api, &path.runtime_id, &path.working_directory_id) } @@ -10374,6 +10392,7 @@ async fn scoped_cleanup_runtime_working_directory( Json(request): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; + require_active_workspace_runtime_binding(&api, &path.runtime_id).await?; let registered_runtime = registered_workdir_runtime_id(&api, &path.working_directory_id)?; if registered_runtime != path.runtime_id { return Err(ApiError::from(Error::WorkspacePermissionDenied( @@ -13587,34 +13606,48 @@ async fn delete_remote_runtime( ) .into()); } - match api - .runtime - .unregister_if_idle(&runtime_id, api.config.max_records.min(200)) - .map_err(|err| err.into_error())? - { - RuntimeRegistryUnregisterResult::Removed | RuntimeRegistryUnregisterResult::NotFound => {} - RuntimeRegistryUnregisterResult::BlockedByWorkers { - worker_count, - diagnostics, - } => { - let mut diagnostics = diagnostics; - diagnostics.push(settings_diagnostic( - "remote_runtime_delete_blocked", - DiagnosticSeverity::Error, - format!( - "Remote Runtime '{runtime_id}' has {worker_count} active worker(s); stop or move them before deleting it." - ), - )); - return Err(ApiError::with_diagnostics( - Error::RuntimeOperationFailed { - runtime_id, - code: "remote_runtime_delete_blocked".to_string(), - message: "Remote Runtime has active workers".to_string(), - }, + let has_other_active_binding = api + .store + .has_other_active_workspace_runtime_binding(&api.config.workspace_id, &runtime_id) + .await?; + if !has_other_active_binding { + match api + .runtime + .unregister_if_idle(&runtime_id, api.config.max_records.min(200)) + .map_err(|err| err.into_error())? + { + RuntimeRegistryUnregisterResult::Removed + | RuntimeRegistryUnregisterResult::NotFound => {} + RuntimeRegistryUnregisterResult::BlockedByWorkers { + worker_count, diagnostics, - )); + } => { + let mut diagnostics = diagnostics; + diagnostics.push(settings_diagnostic( + "remote_runtime_delete_blocked", + DiagnosticSeverity::Error, + format!( + "Remote Runtime '{runtime_id}' has {worker_count} active worker(s); stop or move them before deleting its final Workspace registration." + ), + )); + return Err(ApiError::with_diagnostics( + Error::RuntimeOperationFailed { + runtime_id, + code: "remote_runtime_delete_blocked".to_string(), + message: "Remote Runtime has active workers".to_string(), + }, + diagnostics, + )); + } } } + if !api + .store + .delete_workspace_runtime_binding(&api.config.workspace_id, &runtime_id) + .await? + { + return Err(Error::UnknownRuntime(runtime_id).into()); + } Ok(StatusCode::NO_CONTENT) } @@ -16130,16 +16163,20 @@ fn runtime_binding_summary( binding: &WorkspaceRuntimeBinding, verification: Option<&crate::store::WorkspaceRuntimeVerificationEvidence>, ) -> WorkspaceRuntimeBindingSummary { - let valid_verification = verification.filter(|verification| { - verification.state == "verified" - && verification.binding_revision == binding.binding_revision + let current_verification = verification.filter(|verification| { + verification.binding_revision == binding.binding_revision && verification.runtime_public_key_fingerprint == binding.public_key_fingerprint && verification.workspace_key_id == binding.workspace_key_id.as_deref().unwrap_or_default() }); + let valid_verification = current_verification.filter(|verification| { + verification.state == "verified" && verification.last_outcome == "verified" + }); let connection_state = match binding.state { StoredRuntimeBindingState::Revoked => RuntimeConnectionDisplayState::Revoked, - _ if verification.is_some_and(|verification| verification.last_outcome != "verified") => { + _ if current_verification + .is_some_and(|verification| verification.last_outcome != "verified") => + { RuntimeConnectionDisplayState::Unavailable } StoredRuntimeBindingState::Verified @@ -16161,7 +16198,7 @@ fn runtime_binding_summary( revision: binding.binding_revision, workspace_key_id: binding.workspace_key_id.clone(), workspace_key_generation: binding.workspace_key_generation, - verification: verification.and_then(runtime_verification_summary), + verification: current_verification.and_then(runtime_verification_summary), } } @@ -20957,6 +20994,59 @@ mod tests { config } + #[tokio::test] + async fn scoped_runtime_workdir_access_requires_workspace_binding() { + let dir = tempfile::tempdir().unwrap(); + let api = test_api(dir.path()).await; + + let response = require_active_workspace_runtime_binding(&api, "unregistered-runtime") + .await + .unwrap_err() + .into_response(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); + } + + #[test] + fn runtime_binding_summary_omits_stale_verification_revision() { + let binding = WorkspaceRuntimeBinding { + workspace_id: "workspace-a".to_string(), + runtime_id: "runtime-a".to_string(), + display_name: "Runtime A".to_string(), + base_url: "https://runtime.example.test".to_string(), + public_key: "runtime-public-key".to_string(), + public_key_fingerprint: "sha256:runtime".to_string(), + binding_revision: 2, + state: crate::store::WorkspaceRuntimeBindingState::Revoked, + authentication_mode: + crate::store::WorkspaceRuntimeAuthenticationMode::WorkspaceIdentity, + workspace_key_id: Some("WK-a".to_string()), + workspace_key_generation: Some(1), + created_at: "1".to_string(), + updated_at: "2".to_string(), + revoked_at: Some("2".to_string()), + }; + let stale = crate::store::WorkspaceRuntimeVerificationEvidence { + workspace_id: "workspace-a".to_string(), + runtime_id: "runtime-a".to_string(), + binding_revision: 1, + workspace_key_id: "WK-a".to_string(), + workspace_identity_revision: 1, + workspace_trust_generation: 1, + runtime_public_key_fingerprint: "sha256:runtime".to_string(), + runtime_identity_revision: 1, + challenge_id: "challenge-a".to_string(), + state: "failed".to_string(), + last_outcome: "connectivity_failed".to_string(), + verified_at: None, + checked_at: "1".to_string(), + }; + + let summary = runtime_binding_summary(&binding, Some(&stale)); + + assert_eq!(summary.verification, None); + } + #[test] fn embedded_runtime_request_source_uses_backend_public_url_audience() { let temp = tempfile::tempdir().unwrap(); @@ -27954,9 +28044,8 @@ mod tests { let persisted = store .get_workspace_runtime_binding(TEST_WORKSPACE_ID, "team-runtime") .await - .unwrap() .unwrap(); - assert!(persisted.revoked_at.is_some()); + assert!(persisted.is_none()); } #[tokio::test(flavor = "multi_thread")] diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index ab5c537d..3133cc8b 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -799,6 +799,16 @@ pub trait ControlPlaneStore: Send + Sync + WorkspaceDeletionStore { workspace_id: &str, include_revoked: bool, ) -> Result>; + async fn has_other_active_workspace_runtime_binding( + &self, + workspace_id: &str, + runtime_id: &str, + ) -> Result; + async fn delete_workspace_runtime_binding( + &self, + workspace_id: &str, + runtime_id: &str, + ) -> Result; async fn upsert_workspace_runtime_binding_record( &self, record: WorkspaceRuntimeBinding, @@ -1860,6 +1870,88 @@ impl SqliteWorkspaceStore { }) } + pub fn has_other_active_workspace_runtime_binding( + &self, + workspace_id: &str, + runtime_id: &str, + ) -> Result { + validate_identifier("workspace_id", workspace_id)?; + validate_identifier("runtime_id", runtime_id)?; + self.with_conn(|conn| { + conn.query_row( + "SELECT EXISTS( + SELECT 1 FROM workspace_runtime_bindings + WHERE runtime_id = ?1 AND workspace_id <> ?2 AND revoked_at IS NULL + )", + params![runtime_id, workspace_id], + |row| row.get(0), + ) + .map_err(Into::into) + }) + } + + pub fn delete_workspace_runtime_binding( + &self, + workspace_id: &str, + runtime_id: &str, + ) -> Result { + validate_identifier("workspace_id", workspace_id)?; + validate_identifier("runtime_id", runtime_id)?; + self.with_conn_mut(|conn| { + let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + let revoked_at = tx + .query_row( + "SELECT revoked_at FROM workspace_runtime_bindings WHERE workspace_id = ?1 AND runtime_id = ?2", + params![workspace_id, runtime_id], + |row| row.get::<_, Option>(0), + ) + .optional()?; + let Some(revoked_at) = revoked_at else { + tx.commit()?; + return Ok(false); + }; + if revoked_at.is_none() { + return Err(crate::Error::RuntimeBindingConflict(format!( + "Runtime binding `{runtime_id}` must be revoked before deletion" + ))); + } + let worker_count: i64 = tx.query_row( + "SELECT COUNT(*) FROM worker_registry WHERE workspace_id = ?1 AND runtime_id = ?2", + params![workspace_id, runtime_id], + |row| row.get(0), + )?; + if worker_count > 0 { + return Err(crate::Error::RuntimeBindingConflict(format!( + "Runtime binding `{runtime_id}` still has {worker_count} registered Worker(s) in Workspace `{workspace_id}`" + ))); + } + let workdir_count: i64 = tx.query_row( + "SELECT COUNT(*) FROM workdir_registry WHERE workspace_id = ?1 AND runtime_id = ?2", + params![workspace_id, runtime_id], + |row| row.get(0), + )?; + if workdir_count > 0 { + return Err(crate::Error::RuntimeBindingConflict(format!( + "Runtime binding `{runtime_id}` still has {workdir_count} registered Workdir(s) in Workspace `{workspace_id}`" + ))); + } + tx.execute( + "DELETE FROM worker_mutation_source_proof_jtis WHERE workspace_id = ?1 AND runtime_id = ?2", + params![workspace_id, runtime_id], + )?; + tx.execute( + "DELETE FROM workspace_runtime_binding_audit WHERE workspace_id = ?1 AND runtime_id = ?2", + params![workspace_id, runtime_id], + )?; + let deleted = tx.execute( + "DELETE FROM workspace_runtime_bindings WHERE workspace_id = ?1 AND runtime_id = ?2", + params![workspace_id, runtime_id], + )?; + tx.commit()?; + Ok(deleted == 1) + }) + } + pub fn upsert_workspace_runtime_binding( &self, mut record: WorkspaceRuntimeBinding, @@ -1928,6 +2020,10 @@ impl SqliteWorkspaceStore { ], ) .map_err(map_runtime_binding_write_error)?; + tx.execute( + "DELETE FROM workspace_runtime_verifications WHERE workspace_id = ?1 AND runtime_id = ?2", + params![record.workspace_id, record.runtime_id], + )?; tx.commit()?; return Ok(WorkspaceRuntimeBindingUpsert::Replaced); } @@ -1968,14 +2064,22 @@ impl SqliteWorkspaceStore { validate_identifier("workspace_id", workspace_id)?; validate_identifier("runtime_id", runtime_id)?; validate_non_empty("revoked_at", revoked_at)?; - self.with_conn(|conn| { - let changed = conn.execute( + self.with_conn_mut(|conn| { + let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + let changed = tx.execute( r#"UPDATE workspace_runtime_bindings SET state = 'revoked', revoked_at = ?3, updated_at = ?3, binding_revision = binding_revision + 1 WHERE workspace_id = ?1 AND runtime_id = ?2 AND revoked_at IS NULL"#, params![workspace_id, runtime_id, revoked_at], )?; + if changed > 0 { + tx.execute( + "DELETE FROM workspace_runtime_verifications WHERE workspace_id = ?1 AND runtime_id = ?2", + params![workspace_id, runtime_id], + )?; + } + tx.commit()?; Ok(changed > 0) }) } @@ -2081,6 +2185,10 @@ impl SqliteWorkspaceStore { record.updated_at, ], )?; + tx.execute( + "DELETE FROM workspace_runtime_verifications WHERE workspace_id = ?1 AND runtime_id = ?2", + params![record.workspace_id, record.runtime_id], + )?; insert_workspace_runtime_binding_audit( &tx, &record.workspace_id, @@ -2211,6 +2319,10 @@ impl SqliteWorkspaceStore { WHERE workspace_id = ?1 AND runtime_id = ?2"#, params![workspace_id, runtime_id, revoked_at, next_revision], )?; + tx.execute( + "DELETE FROM workspace_runtime_verifications WHERE workspace_id = ?1 AND runtime_id = ?2", + params![workspace_id, runtime_id], + )?; insert_workspace_runtime_binding_audit( &tx, workspace_id, @@ -3331,6 +3443,26 @@ impl ControlPlaneStore for SqliteWorkspaceStore { SqliteWorkspaceStore::list_workspace_runtime_bindings(self, workspace_id, include_revoked) } + async fn has_other_active_workspace_runtime_binding( + &self, + workspace_id: &str, + runtime_id: &str, + ) -> Result { + SqliteWorkspaceStore::has_other_active_workspace_runtime_binding( + self, + workspace_id, + runtime_id, + ) + } + + async fn delete_workspace_runtime_binding( + &self, + workspace_id: &str, + runtime_id: &str, + ) -> Result { + SqliteWorkspaceStore::delete_workspace_runtime_binding(self, workspace_id, runtime_id) + } + async fn upsert_workspace_runtime_binding_record( &self, record: WorkspaceRuntimeBinding, @@ -10044,6 +10176,28 @@ mod tests { .revoked_at .is_some() ); + assert!( + reopened + .has_other_active_workspace_runtime_binding("workspace-a", "shared") + .unwrap() + ); + assert!( + reopened + .delete_workspace_runtime_binding("workspace-a", "shared") + .unwrap() + ); + assert!( + reopened + .get_workspace_runtime_binding("workspace-a", "shared") + .unwrap() + .is_none() + ); + assert!( + reopened + .get_workspace_runtime_binding("workspace-b", "shared") + .unwrap() + .is_some() + ); } #[test] @@ -10485,6 +10639,29 @@ mod tests { .unwrap(); assert_eq!(replaced, WorkspaceRuntimeBindingMutation::Replaced); assert_eq!(replaced_binding.binding_revision, 2); + store + .record_workspace_runtime_verification_attempt(&WorkspaceRuntimeVerificationEvidence { + workspace_id: "workspace-a".to_string(), + runtime_id: "runtime-a".to_string(), + binding_revision: 2, + workspace_key_id: "WK-a".to_string(), + workspace_identity_revision: 1, + workspace_trust_generation: 1, + runtime_public_key_fingerprint: replaced_binding.public_key_fingerprint.clone(), + runtime_identity_revision: 1, + challenge_id: "challenge-a".to_string(), + state: "failed".to_string(), + last_outcome: "connectivity_failed".to_string(), + verified_at: None, + checked_at: "3".to_string(), + }) + .unwrap(); + assert!( + store + .get_workspace_runtime_verification("workspace-a", "runtime-a") + .unwrap() + .is_some() + ); let (revoked, revoked_binding) = store .revoke_workspace_runtime_binding_key("workspace-a", "runtime-a", 2, "owner", "4") .unwrap(); @@ -10492,6 +10669,12 @@ mod tests { assert_eq!(revoked_binding.binding_revision, 3); assert_eq!(revoked_binding.revoked_at.as_deref(), Some("4")); assert_eq!(revoked_binding.state, WorkspaceRuntimeBindingState::Revoked); + assert_eq!( + store + .get_workspace_runtime_verification("workspace-a", "runtime-a") + .unwrap(), + None + ); let (reactivated, reactivated_binding) = store .put_workspace_runtime_binding_key( binding(second.public_key.clone(), "5"),