Hare/yoi
1
1
Fork 0
Code Actions Packages Releases Activity

fix: defer restored workdir access until use

Browse Source
This commit is contained in:
Hare
2026-10-20 22:00:58 +09:00
parent 3cfe04be3a
commit 338e39bcdb
8 changed files with 933 additions and 914 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/workspace-server/src/server.rs
+28 -242
View File
@@ -6079,12 +6079,11 @@ impl WorkspaceApi {
/// Binds observation-based Restore intent to a Runtime-owned operation.
///
/// First queries existing owners/receipts without preparation, handling replay and recovery
/// before new-launch eligibility. New intent rereads singleton, Workdir, and External grants
/// to build references. SSH resources are issued only after Runtime admits the same observation
/// token under its operation lock. Replay does not replace saved preparation; incomplete auth
/// preparation remains ReconciliationRequired. Runtime owns lifecycle settlement, while the
/// returned summary comes from current observation.
/// Existing owners reach receipts/recovery before new-launch eligibility.
/// Restore retains attachment references without opening providers, resolving
/// Repository credentials, or requiring materialization. Workdir operations
/// validate current authority and availability when a session is used.
/// Runtime owns lifecycle settlement; summaries come from current observation.
fn restore_workspace_worker(
&self,
worker: &RuntimeWorkerRef,
@@ -6117,82 +6116,11 @@ impl WorkspaceApi {
result.worker = self.current_restore_summary(worker, result.worker.is_some());
return Ok(result);
}
let mut preparation = if let Some(preparation) = owned_preparation {
let preparation = if let Some(preparation) = owned_preparation {
preparation
} else {
self.store
.require_current_worker_singleton_owner(&self.config.workspace_id, worker)?;
let mut repository_access_workdirs = Vec::new();
for link in self
.store
.list_worker_workdir_links(&self.config.workspace_id, worker)?
.into_iter()
.filter(|link| link.unlinked_at.is_none())
{
let record = self
.store
.get_workdir_registry(&self.config.workspace_id, &link.workdir_id)?
.ok_or_else(|| Error::RuntimeOperationFailed {
runtime_id: "workspace-backend".to_string(),
code: "working_directory_not_found".to_string(),
message: format!("Unknown Workdir `{}`", link.workdir_id),
})?;
match record.source {
WorkdirRegistrySource::Repository {
runtime_id,
repository_id,
} => {
if self
.require_configured_workspace_repository(&repository_id)?
.source
.kind
== server_api::RepositorySourceKind::Ssh
{
repository_access_workdirs.push(
runtime_api::RepositoryAccessWorkdirReference {
runtime_id,
working_directory_id: link.workdir_id.clone(),
},
);
}
}
WorkdirRegistrySource::WorkspaceConfig { .. } => continue,
WorkdirRegistrySource::ExternalGrant { grant_id } => {
let grant = self
.store
.get_external_workdir_grant(&self.config.workspace_id, &grant_id)?
.ok_or_else(|| Error::RuntimeOperationFailed {
runtime_id: "external-workdir-provider".to_string(),
code: "external_workdir_unavailable".to_string(),
message: "External Workdir grant is unavailable".to_string(),
})?;
let grant_expired = external_workdir_grant_is_expired(&grant)?;
let connection = self
.external_workdir_providers
.lock()
.expect("External Workdir provider registry poisoned")
.connection(&grant_id);
if grant.status != "online"
|| grant_expired
|| !connection.is_some_and(|connection| {
connection.generation == grant.generation
&& connection.provider_instance_id == grant.provider_instance_id
&& connection.workdir_id == link.workdir_id
&& connection
.expires_at
.is_none_or(|expires_at| expires_at > Utc::now())
})
{
return Err(Error::RuntimeOperationFailed {
runtime_id: "external-workdir-provider".to_string(),
code: "external_workdir_unavailable".to_string(),
message: "External Workdir provider is offline".to_string(),
}
.into());
}
}
}
}
let preparation = runtime_api::WorkerRestorePreparation {
workspace_api: Some(runtime_contract_restore(
self.workspace_api_ref(&worker.runtime_id),
@@ -6201,7 +6129,7 @@ impl WorkspaceApi {
restore_logical_workdir_attachments(self, worker)?,
)?),
repository_access: Vec::new(),
repository_access_workdirs,
repository_access_workdirs: Vec::new(),
};
let (result, preparation) = self
.runtime
@@ -6222,60 +6150,6 @@ impl WorkspaceApi {
Error::Config("Runtime did not retain admitted Restore preparation".into())
})?
};
// Only the admitted operation may issue SSH handles. Reissue from the pinned
// credential snapshot after interruption; never renew candidate bindings.
let completion =
complete_restore_repository_access(self, &request.request_id, &mut preparation);
if let Err(error) = completion {
return Ok(InternalWorkerRestoreResult {
state: server_api::WorkerRestoreState::ReconciliationRequired,
worker: None,
diagnostics: vec![RuntimeDiagnostic::new(
"worker_restore_preparation_pending",
"warning",
sanitize_backend_error(&error.error.to_string()),
)],
});
}
// Persist the first resource snapshot at the admitted owner BEFORE any
// Workdir Runtime authorization effect. Replays renew only its opaque handles.
let (result, retained) = self
.runtime
.coordinate_worker_restore(
worker,
runtime_api::WorkerRestoreCoordinationRequest {
expected_observation_token: request.expected_observation_token.clone(),
request_id: request.request_id.clone(),
preparation: Some(preparation),
},
)
.map_err(RuntimeRegistryError::into_error)?;
if let Some(mut result) = result {
result.worker = self.current_restore_summary(worker, result.worker.is_some());
return Ok(result);
}
let preparation = retained.ok_or_else(|| {
Error::Config("Runtime did not retain Restore resource snapshot".into())
})?;
for access in &preparation.repository_access {
let access: worker_runtime::catalog::WorkingDirectoryRepositoryAccessRequest =
runtime_contract_restore(access.clone())?;
let runtime_id = access.materialization.runtime_id.clone();
if let Err(error) = self
.runtime
.authorize_working_directory_repository_access(&runtime_id, access)
{
return Ok(InternalWorkerRestoreResult {
state: server_api::WorkerRestoreState::ReconciliationRequired,
worker: None,
diagnostics: vec![RuntimeDiagnostic::new(
"worker_restore_repository_access_pending",
"warning",
sanitize_backend_error(&error.message()),
)],
});
}
}
let mut restored = self
.runtime
.restore_worker(
@@ -20315,44 +20189,30 @@ fn restore_logical_workdir_attachments(
api: &WorkspaceApi,
worker: &RuntimeWorkerRef,
) -> ApiResult<Vec<LogicalWorkdirAttachment>> {
api.store
let mut attachments = Vec::new();
for link in api
.store
.list_worker_workdir_links(&api.config.workspace_id, worker)?
.into_iter()
.filter(|link| link.unlinked_at.is_none())
.filter(|link| {
api.store
.get_workdir_registry(&api.config.workspace_id, &link.workdir_id)
.ok()
.flatten()
.is_none_or(|record| {
!matches!(record.source, WorkdirRegistrySource::WorkspaceConfig { .. })
})
})
.map(|link| {
let workdir = api
.store
.get_workdir_registry(&api.config.workspace_id, &link.workdir_id)?
.ok_or_else(|| {
Error::RegistryInconsistency(format!(
"Worker attachment references unknown Workdir {}",
link.workdir_id
))
})?;
let capabilities = link
.capabilities
.intersection(workdir_source_capabilities(api, &workdir)?)
.intersection(ticket_target_capabilities_for_workdir(
api, worker, &workdir,
)?);
Ok(LogicalWorkdirAttachment {
alias: workdir::WorkdirAttachmentAlias::new(link.alias)
.map_err(|error| Error::InvalidInput(error.to_string()))?,
working_directory_id: link.workdir_id,
capabilities,
{
if api
.store
.get_workdir_registry(&api.config.workspace_id, &link.workdir_id)?
.is_some_and(|record| {
matches!(record.source, WorkdirRegistrySource::WorkspaceConfig { .. })
})
})
.collect::<Result<Vec<_>>>()
.map_err(ApiError::from)
{
continue;
}
attachments.push(LogicalWorkdirAttachment {
alias: workdir::WorkdirAttachmentAlias::new(link.alias)
.map_err(|error| Error::InvalidInput(error.to_string()))?,
working_directory_id: link.workdir_id,
capabilities: link.capabilities,
});
}
Ok(attachments)
}
fn prepare_runtime_worker_workdir_attachments(
@@ -36648,81 +36508,6 @@ fn authorize_repository_materialization(
Ok(())
}
fn complete_restore_repository_access(
api: &WorkspaceApi,
request_id: &str,
preparation: &mut runtime_api::WorkerRestorePreparation,
) -> ApiResult<()> {
if preparation.repository_access.is_empty() {
for reference in &preparation.repository_access_workdirs {
if let Some(access) = repository_access_request_for_workdir(
api,
&reference.runtime_id,
&reference.working_directory_id,
&format!("worker-restore:{request_id}"),
)? {
preparation
.repository_access
.push(runtime_contract_restore(access)?);
}
}
return Ok(());
}
for prepared in &mut preparation.repository_access {
let mut access: worker_runtime::catalog::WorkingDirectoryRepositoryAccessRequest =
runtime_contract_restore(prepared.clone())?;
let Some(ssh) = access.materialization.ssh.as_mut() else {
continue;
};
let mut leases = Vec::new();
for candidate in &ssh.credential_candidates {
leases.push(
api.repository_secrets
.lease_ssh_materialization_access_fingerprints(
api.workspace_id(),
&candidate.credential_id,
&candidate.public_key_fingerprint,
&ssh.host_trust_id,
&ssh.host_key_fingerprint,
)?,
);
}
let known_hosts_entry = leases
.first()
.ok_or_else(|| Error::Config("Admitted SSH snapshot has no candidates".into()))?
.known_hosts_entry
.clone();
ssh.expires_at_epoch_seconds = repository_access_expiry();
ssh.secret_resource = api
.resource_broker
.issue_repository_ssh_access_handle(
api.workspace_id(),
&access.materialization.runtime_id,
format!(
"repository-ssh-access:{}",
access.materialization.operation_id
),
i64::try_from(ssh.expires_at_epoch_seconds).unwrap_or(i64::MAX),
worker_runtime::resource::RepositorySshAccessSecret {
credential_candidates: leases
.into_iter()
.map(
|lease| worker_runtime::resource::RepositorySshAccessSecretCandidate {
credential_id: lease.credential_id,
public_key_fingerprint: lease.credential_fingerprint.clone(),
private_key: lease.private_key.as_str().to_owned(),
},
)
.collect(),
known_hosts_entry,
},
)
.map_err(|error| Error::Config(error.to_string()))?;
*prepared = runtime_contract_restore(access)?;
}
Ok(())
}
fn repository_access_request_for_workdir(
api: &WorkspaceApi,
runtime_id: &str,
@@ -37544,6 +37329,7 @@ mod tests {
mod embedded_owned_execution_tests;
mod external_checkout_tests;
mod protocol_backpressure_tests;
mod restore_workdir_tests;
mod restored_parent_control_tests;
mod subject_authorization_tests;
mod subject_spawn_tests;
crates/workspace-server/src/server/tests/restore_workdir_tests.rs
+194
View File
@@ -0,0 +1,194 @@
//! Restore entry -> real SQLite attachment registry -> embedded Runtime restore
//! coordination. Only execution is deterministic; Workdir providers are absent.
//! This suite covers restore preparation, not HTTP authentication or model turns.
use super::*;
#[tokio::test]
async fn restore_keeps_offline_external_and_unmaterialized_repository_attachments() {
let temp = tempfile::tempdir().unwrap();
init_clean_git_workspace(temp.path());
let (api, execution) = test_api_with_recording_backend(temp.path()).await;
execution.accept_restores();
let Json(created) = create_workspace_worker(
State(api.clone()),
HeaderMap::new(),
Json(CreateWorkspaceWorkerRequest {
runtime_id: EMBEDDED_WORKER_RUNTIME_ID.into(),
display_name: "Restore without providers".into(),
singleton_key: None,
profile: Some("builtin:coder".into()),
ticket_assignment: None,
initial_submit: Vec::new(),
workdir_attachments: Vec::new(),
feature_connections: Default::default(),
control_operation_id: None,
execution_grant: None,
}),
)
.await
.unwrap();
let worker = RuntimeWorkerRef::new(&created.runtime_id, &created.worker_id);
let grant = ExternalWorkdirGrantRecord {
grant_id: "restore-external-grant".into(),
workspace_id: TEST_WORKSPACE_ID.into(),
workdir_id: "restore-external".into(),
provider_instance_id: "offline-provider".into(),
display_name: "Offline external".into(),
permissions: "read_only".into(),
created_by: test_browser_request_actor().account_id,
created_at: TEST_CREATED_AT.into(),
expires_at: None,
generation: 1,
status: "pending".into(),
updated_at: TEST_CREATED_AT.into(),
};
for (alias, workdir_id, source) in [
(
"external",
grant.workdir_id.as_str(),
WorkdirRegistrySource::ExternalGrant {
grant_id: grant.grant_id.clone(),
},
),
(
"repository",
"restore-missing-repository",
WorkdirRegistrySource::Repository {
runtime_id: EMBEDDED_WORKER_RUNTIME_ID.into(),
repository_id: test_repository_id(&api),
},
),
] {
let record = WorkdirRegistryRecord {
workspace_id: TEST_WORKSPACE_ID.into(),
workdir_id: workdir_id.into(),
display_name: Some(alias.into()),
source,
creation_selector: None,
creation_ref: None,
creation_tree: None,
current_selector: None,
current_ref: None,
current_tree: None,
observed_at_epoch_seconds: None,
materialization_status: "not_found".into(),
cleanliness: "unknown".into(),
created_at: TEST_CREATED_AT.into(),
updated_at: TEST_CREATED_AT.into(),
};
if alias == "external" {
api.store
.create_external_workdir_grant(&grant, &record)
.unwrap();
assert!(
api.store
.activate_external_workdir_grant(
TEST_WORKSPACE_ID,
&grant.grant_id,
&grant.provider_instance_id,
1,
TEST_CREATED_AT,
)
.unwrap()
);
} else {
api.store.upsert_workdir_registry(&record).unwrap();
}
api.store
.attach_worker_workdir(&WorkerWorkdirLinkRecord {
connection_id: String::new(),
workspace_id: TEST_WORKSPACE_ID.into(),
worker: worker.clone(),
workdir_id: workdir_id.into(),
alias: alias.into(),
capabilities: workdir::WorkdirSessionCapabilities::READ_ONLY,
linked_at: TEST_CREATED_AT.into(),
unlinked_at: None,
})
.unwrap();
}
let links = api
.store
.list_worker_workdir_links(TEST_WORKSPACE_ID, &worker)
.unwrap();
api.runtime
.stop_worker(
&worker,
WorkerLifecycleRequest {
reason: None,
ticket_assignment: None,
},
)
.unwrap();
let result = api
.restore_workspace_worker(
&worker,
server_api::WorkerRestoreRequest {
expected_observation_token: api
.runtime
.worker(&worker)
.unwrap()
.restore_observation_token
.unwrap(),
request_id: uuid::Uuid::now_v7().to_string(),
},
)
.unwrap();
assert_eq!(result.state, server_api::WorkerRestoreState::Accepted);
let attachments = restore_logical_workdir_attachments(&api, &worker).unwrap();
assert_eq!(attachments.len(), 2);
assert!(
attachments
.iter()
.any(|attachment| attachment.alias.as_str() == "external"
&& attachment.working_directory_id == grant.workdir_id)
);
assert!(
attachments
.iter()
.any(|attachment| attachment.alias.as_str() == "repository"
&& attachment.working_directory_id == "restore-missing-repository")
);
assert_eq!(
api.store
.list_worker_workdir_links(TEST_WORKSPACE_ID, &worker)
.unwrap(),
links
);
let lock = current_worker_session_lock(&api, &worker);
let guard = Arc::new(lock.lock_owned().await);
for link in &links {
let error = open_current_worker_workdir_session_locked(&api, &worker, link, &guard)
.await
.unwrap_err();
match link.alias.as_str() {
"external" => assert!(matches!(
error,
Error::RuntimeOperationFailed { code, .. }
if code == "external_workdir_provider_offline"
)),
"repository" => assert!(matches!(
error,
Error::WorkdirSessionOpenFailed { transport, .. }
if transport.code == workdir::http::WorkdirTransportErrorCode::Unavailable
)),
alias => panic!("unexpected attachment: {alias}"),
}
}
assert_eq!(
api.store
.list_worker_workdir_links(TEST_WORKSPACE_ID, &worker)
.unwrap(),
links
);
api.runtime
.stop_worker(
&worker,
WorkerLifecycleRequest {
reason: None,
ticket_assignment: None,
},
)
.unwrap();
}
crates/workspace-server/src/server/tests/worker_operations_tests.rs
+23 -131
View File
@@ -1593,7 +1593,7 @@ async fn restore_guard_workspace_interrupted_preparation_recovers_admitted_snaps
}
#[tokio::test]
async fn restore_guard_cross_runtime_ssh_completion_retries_pinned_snapshot_at_workdir_owner() {
async fn restore_keeps_cross_runtime_ssh_attachment_without_issuing_repository_access() {
let workspace = tempfile::tempdir().unwrap();
let mut config = test_server_config(workspace.path());
let source = server_api::RepositorySource {
@@ -1619,39 +1619,6 @@ async fn restore_guard_cross_runtime_ssh_completion_retries_pinned_snapshot_at_w
.stop(&WorkerOperationContext::Backend, operation_lifecycle())
.await
.unwrap();
let original_key = api
.repository_secrets
.generate_credential(
api.workspace_id(),
GenerateRepositorySshCredentialRequest {
operation_id: "cross-runtime-default-key".into(),
credential_id:
crate::repository_access::WORKSPACE_DEFAULT_REPOSITORY_SSH_CREDENTIAL_ID.into(),
name: "Restore default".into(),
},
"owner-account",
)
.unwrap();
let public_key = api
.repository_secrets
.credential_public_key(api.workspace_id(), &original_key.credential_id)
.unwrap()
.unwrap()
.public_key;
api.repository_secrets
.put_host_trust(
api.workspace_id(),
PutRepositorySshHostTrustRequest {
operation_id: "cross-runtime-trust".into(),
host_trust_id: "cross-runtime-host".into(),
hostname: "example.invalid".into(),
port: 22,
host_key: public_key,
expected_fingerprint: None,
},
"owner-account",
)
.unwrap();
let record = restore_test_workdir(
&api,
"cross-runtime-restore-workdir",
@@ -1663,102 +1630,27 @@ async fn restore_guard_cross_runtime_ssh_completion_retries_pinned_snapshot_at_w
peer.set_workdir_repository(&record.workdir_id, &test_repository_id(&api));
api.store.upsert_workdir_registry(&record).unwrap();
attach_restore_test_workdir(&api, &identity, &record);
let links = api
.store
.list_worker_workdir_links(api.workspace_id(), &identity)
.unwrap();
let request = restore_request_for(&api, &identity, "cross-runtime-restore-request");
assert_eq!(
handle
.restore(&WorkerOperationContext::Backend, request.clone())
.await
.unwrap()
.state,
server_api::WorkerRestoreState::ReconciliationRequired
);
assert!(
execution.restore_operations.lock().unwrap().is_empty(),
"credential completion must precede execution"
);
let first = peer.repository_access_requests.lock().unwrap()[0].clone();
assert_eq!(
first.materialization.runtime_id,
WorkdirlessFixtureRuntime::RUNTIME_ID
);
assert_ne!(first.materialization.runtime_id, identity.runtime_id);
// Rotate current host trust while the accepted operation is uncertain. Its
// original credential/trust fingerprints, source and binding must remain pinned.
let alternate = api
.repository_secrets
.generate_credential(
api.workspace_id(),
GenerateRepositorySshCredentialRequest {
operation_id: "cross-runtime-alternate-key".into(),
credential_id: "cross-runtime-alternate".into(),
name: "Alternate".into(),
},
"owner-account",
)
.unwrap();
let alternate_public = api
.repository_secrets
.credential_public_key(api.workspace_id(), &alternate.credential_id)
.unwrap()
.unwrap()
.public_key;
let original_trust_fingerprint = first
.materialization
.ssh
.as_ref()
.unwrap()
.host_key_fingerprint
.clone();
api.repository_secrets
.put_host_trust(
api.workspace_id(),
PutRepositorySshHostTrustRequest {
operation_id: "cross-runtime-trust-rotate".into(),
host_trust_id: "cross-runtime-host".into(),
hostname: "example.invalid".into(),
port: 22,
host_key: alternate_public,
expected_fingerprint: Some(original_trust_fingerprint),
},
"owner-account",
)
.unwrap();
*peer.reject_repository_access.lock().unwrap() = false;
assert_eq!(
handle
.restore(&WorkerOperationContext::Backend, request.clone())
.await
.unwrap()
.state,
server_api::WorkerRestoreState::Accepted
);
let attempts = peer.repository_access_requests.lock().unwrap();
assert_eq!(attempts.len(), 2);
let second = &attempts[1];
assert_eq!(
second.materialization.operation_id,
first.materialization.operation_id
);
let mut normalized = second.clone();
let old_ssh = first.materialization.ssh.as_ref().unwrap();
let new_ssh = normalized.materialization.ssh.as_mut().unwrap();
assert_ne!(new_ssh.secret_resource.nonce, old_ssh.secret_resource.nonce);
new_ssh.secret_resource = old_ssh.secret_resource.clone();
new_ssh.expires_at_epoch_seconds = old_ssh.expires_at_epoch_seconds;
assert_eq!(normalized, first, "only opaque handle and expiry may renew");
drop(attempts);
assert_eq!(execution.restore_operations.lock().unwrap().len(), 1);
assert_eq!(
handle
.restore(&WorkerOperationContext::Backend, request)
.await
.unwrap()
.state,
server_api::WorkerRestoreState::Accepted
);
assert_eq!(
peer.repository_access_requests.lock().unwrap().len(),
2,
"terminal receipt must not renew access"
);
for _ in 0..2 {
assert_eq!(
handle
.restore(&WorkerOperationContext::Backend, request.clone())
.await
.unwrap()
.state,
server_api::WorkerRestoreState::Accepted
);
assert!(peer.repository_access_requests.lock().unwrap().is_empty());
assert_eq!(execution.restore_operations.lock().unwrap().len(), 1);
assert_eq!(
api.store
.list_worker_workdir_links(api.workspace_id(), &identity)
.unwrap(),
links
);
}
}