From eb64644c9b7dae4af70d4e6db5ab9ad59e509cf4 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 4 Nov 2026 20:05:09 +0900 Subject: [PATCH] fix: validate repository access within owner adoption transaction --- .../workspace-server/src/config_adoption.rs | 1 + .../workspace-server/src/repository_access.rs | 87 ++++-- .../server_configuration_adoption_tests.rs | 264 ++++++++++++++++++ crates/workspace-server/src/store.rs | 32 ++- 4 files changed, 346 insertions(+), 38 deletions(-) diff --git a/crates/workspace-server/src/config_adoption.rs b/crates/workspace-server/src/config_adoption.rs index 018bec136..ee003bb50 100644 --- a/crates/workspace-server/src/config_adoption.rs +++ b/crates/workspace-server/src/config_adoption.rs @@ -529,6 +529,7 @@ fn prepare( schema, )?; crate::config_source::validate_workspace_config_candidate_projections(workspace, &candidate)?; + crate::repository_access::validate_repository_access_in_transaction(tx, workspace, &candidate)?; let source = policy_config::source(&candidate.evaluation)?; policy_config::validate_source(host, &source, require_subject)?; let state = WorkspaceConfigState { diff --git a/crates/workspace-server/src/repository_access.rs b/crates/workspace-server/src/repository_access.rs index c8a13c3f4..460f61ff5 100644 --- a/crates/workspace-server/src/repository_access.rs +++ b/crates/workspace-server/src/repository_access.rs @@ -189,24 +189,65 @@ fn project_repository_access_evaluation( projection_digest: &str, evaluation: &config_source::EvaluationResult, ) -> Result { - let projection = evaluation.projections.first().ok_or_else(|| { - Error::InvalidInput("Workspace config produced no active projection".to_string()) - })?; - let config: VirtualWorkspaceConfig = serde_json::from_value(projection.data_json.clone()) - .map_err(|error| { - Error::InvalidInput(format!("invalid Repository access config: {error}")) - })?; + let config = repository_access_source(evaluation)?; if !config.repository_access.is_empty() { secrets.ensure_workspace_default_credential(workspace_id)?; } + project_repository_access_bindings( + workspace_id, + projection_digest, + config, + |key| store.get_repository_by_key(workspace_id, key), + |id| secrets.get_credential(workspace_id, id, &[]), + |id| secrets.get_host_trust(workspace_id, id, &[]), + ) +} + +/// Validate against the same catalog transaction that will adopt the source. +/// This reads only public credential/trust metadata: no default key generation, +/// unsealing, filesystem writes or second connection under the writer lock. +/// Both preview and adoption use the owning projection's semantic checks. +pub(crate) fn validate_repository_access_in_transaction( + tx: &rusqlite::Transaction<'_>, + workspace_id: &str, + candidate: &EvaluatedConfigCandidate, +) -> Result<()> { + project_repository_access_bindings( + workspace_id, + &candidate.evaluation.projection_digest, + repository_access_source(&candidate.evaluation)?, + |key| crate::store::read_repository_by_key(tx, workspace_id, key), + |id| read_credential(tx, workspace_id, id), + |id| read_host_trust(tx, workspace_id, id), + ) + .map(|_| ()) +} + +fn repository_access_source( + evaluation: &config_source::EvaluationResult, +) -> Result { + let projection = evaluation.projections.first().ok_or_else(|| { + Error::InvalidInput("Workspace config produced no active projection".to_string()) + })?; + serde_json::from_value(projection.data_json.clone()) + .map_err(|error| Error::InvalidInput(format!("invalid Repository access config: {error}"))) +} + +fn project_repository_access_bindings( + workspace_id: &str, + projection_digest: &str, + config: VirtualWorkspaceConfig, + repository: impl Fn(&str) -> Result>, + credential: impl Fn(&str) -> Result>, + host_trust: impl Fn(&str) -> Result>, +) -> Result { let mut bindings = Vec::with_capacity(config.repository_access.len()); for (repository_key, access) in config.repository_access { server_api::validate_repository_key(&repository_key) .map_err(|error| Error::InvalidInput(format!("invalid Repository key: {error}")))?; validate_identifier("credential_id", &access.ssh.credential)?; validate_identifier("host_trust_id", &access.ssh.host_trust)?; - let repository = store - .get_repository_by_key(workspace_id, &repository_key)? + let repository = repository(&repository_key)? .ok_or_else(|| Error::InvalidInput(format!("unknown Repository `{repository_key}`")))?; validate_repository_access_source(&repository_key, &repository.source)?; if repository.source.kind != server_api::RepositorySourceKind::Ssh { @@ -214,28 +255,24 @@ fn project_repository_access_evaluation( "Repository `{repository_key}` is not an ssh:// Repository" ))); } - let credential = secrets - .get_credential(workspace_id, &access.ssh.credential, &[])? - .ok_or_else(|| { - Error::InvalidInput(format!( - "unknown Repository SSH credential `{}`", - access.ssh.credential - )) - })?; + let credential = credential(&access.ssh.credential)?.ok_or_else(|| { + Error::InvalidInput(format!( + "unknown Repository SSH credential `{}`", + access.ssh.credential + )) + })?; if credential.status != "active" { return Err(Error::InvalidInput(format!( "Repository SSH credential `{}` is not active", access.ssh.credential ))); } - let host_trust = secrets - .get_host_trust(workspace_id, &access.ssh.host_trust, &[])? - .ok_or_else(|| { - Error::InvalidInput(format!( - "unknown Repository SSH host trust `{}`", - access.ssh.host_trust - )) - })?; + let host_trust = host_trust(&access.ssh.host_trust)?.ok_or_else(|| { + Error::InvalidInput(format!( + "unknown Repository SSH host trust `{}`", + access.ssh.host_trust + )) + })?; let (hostname, port) = repository_ssh_endpoint(repository_key.as_str(), &repository.source.uri)?.ok_or_else( || { diff --git a/crates/workspace-server/src/server_configuration_adoption_tests.rs b/crates/workspace-server/src/server_configuration_adoption_tests.rs index 12ab1c5dc..96c0cc58f 100644 --- a/crates/workspace-server/src/server_configuration_adoption_tests.rs +++ b/crates/workspace-server/src/server_configuration_adoption_tests.rs @@ -355,6 +355,270 @@ mod configuration_adoption_http_integration { ) } + #[tokio::test] + async fn owner_http_repository_validation_is_read_only_and_precedes_seal() { + let f = Fixture::new().await; + let source = server_api::RepositorySource { + kind: server_api::RepositorySourceKind::Ssh, + uri: "ssh://git@repository.example.test:2222/team/repo.git".into(), + }; + f.api + .store + .upsert_repository(&crate::store::RepositoryRecord { + workspace_id: TEST_WORKSPACE_ID.into(), + repository_id: "ssh-repository".into(), + repository_key: "remote".into(), + kind: "git".into(), + provider: None, + source_fingerprint: crate::repository_source::repository_source_fingerprint( + &source, + ), + source, + default_ref: None, + observed_status: server_api::RepositoryObservedStatus::Unverified, + observed_at: None, + created_at: "1".into(), + updated_at: "1".into(), + }) + .unwrap(); + let mut non_ssh = f + .api + .store + .get_repository_by_key(TEST_WORKSPACE_ID, "remote") + .unwrap() + .unwrap(); + non_ssh.repository_id = "https-repository".into(); + non_ssh.repository_key = "nonssh".into(); + non_ssh.source = server_api::RepositorySource { + kind: server_api::RepositorySourceKind::Https, + uri: "https://repository.example.test/team/repo.git".into(), + }; + non_ssh.source_fingerprint = + crate::repository_source::repository_source_fingerprint(&non_ssh.source); + f.api.store.upsert_repository(&non_ssh).unwrap(); + for id in ["deploy", "revoked"] { + f.api + .repository_secrets + .generate_credential( + TEST_WORKSPACE_ID, + GenerateRepositorySshCredentialRequest { + operation_id: format!("generate-{id}"), + credential_id: id.into(), + name: id.into(), + }, + &f.owner_account, + ) + .unwrap(); + } + f.api.config_store.with_conn(|conn| { + conn.execute("UPDATE repository_ssh_credentials SET status='revoked' WHERE workspace_id=?1 AND credential_id='revoked'", [TEST_WORKSPACE_ID])?; + Ok(()) + }).unwrap(); + let public_key = + ssh_key::PrivateKey::from(ssh_key::private::Ed25519Keypair::from_seed(&[37; 32])) + .public_key() + .to_openssh() + .unwrap(); + for (id, hostname, port) in [ + ("matching", "repository.example.test", 2222), + ("wrong-host", "other.example.test", 2222), + ("wrong-port", "repository.example.test", 22), + ] { + f.api + .repository_secrets + .put_host_trust( + TEST_WORKSPACE_ID, + PutRepositorySshHostTrustRequest { + operation_id: format!("trust-{id}"), + host_trust_id: id.into(), + hostname: hostname.into(), + port, + host_key: public_key.clone(), + expected_fingerprint: None, + }, + &f.owner_account, + ) + .unwrap(); + } + let secret_counts = || { + f.api + .config_store + .with_conn(|conn| { + [ + "repository_ssh_credentials", + "repository_ssh_credential_keys", + "repository_ssh_host_trusts", + "repository_ssh_host_trust_keys", + "repository_secret_operations", + "repository_secret_audit_events", + "server_secret_objects", + ] + .into_iter() + .map(|table| { + conn.query_row( + &format!("SELECT COUNT(*) FROM {table} WHERE workspace_id=?1"), + [TEST_WORKSPACE_ID], + |r| r.get::<_, i64>(0), + ) + .map_err(Error::from) + }) + .collect::>>() + }) + .unwrap() + }; + let secrets_before = secret_counts(); + let content = |repo: &str, credential: &str, trust: &str, access: &str| { + format!( + "{{ repository_access = {{ {repo} = {{ ssh = {{ credential = {credential:?}; host_trust = {trust:?}; access = {access:?}; }}; }}; }}; }}" + ) + }; + for canonical in [false, true] { + let current = f.source().await; + assert_eq!(current.source_stored, canonical); + let before = f.durable(); + for (case, repo, credential, trust, access, diagnostic) in [ + ( + "mode", + "remote", + "deploy", + "matching", + "not-a-mode", + "invalid Repository access config", + ), + ( + "repository", + "absent", + "deploy", + "matching", + "read_only", + "unknown Repository", + ), + ( + "non-ssh", + "nonssh", + "deploy", + "matching", + "read_only", + "not an ssh", + ), + ( + "credential", + "remote", + "absent", + "matching", + "read_only", + "unknown Repository SSH credential", + ), + ( + "inactive", + "remote", + "revoked", + "matching", + "read_only", + "is not active", + ), + ( + "trust", + "remote", + "deploy", + "absent", + "read_only", + "unknown Repository SSH host trust", + ), + ( + "host", + "remote", + "deploy", + "wrong-host", + "read_only", + "does not match host trust", + ), + ( + "port", + "remote", + "deploy", + "wrong-port", + "read_only", + "does not match host trust", + ), + ] { + let adoption = request( + ¤t, + &format!("repository-{canonical}-{case}"), + vec![update( + ¤t, + "main.dcdl", + &content(repo, credential, trust, access), + )], + ); + for endpoint in ["/preview", "/commit"] { + let rejected = f + .owner_call(endpoint, &adoption, StatusCode::BAD_REQUEST) + .await; + assert!( + rejected.to_string().contains(diagnostic), + "{case} {endpoint}: {rejected}" + ); + assert_eq!( + f.durable(), + before, + "{case} {endpoint}: all old authorities must remain unchanged" + ); + assert_eq!( + secret_counts(), + secrets_before, + "validation must not generate keys or secret receipts" + ); + if canonical { + f.assert_legacy_sealed(); + } else { + f.assert_legacy_writable(); + } + } + } + let valid = request( + ¤t, + &format!("valid-repository-{canonical}"), + vec![update( + ¤t, + "main.dcdl", + &content( + "remote", + "deploy", + "matching", + if canonical { "read_write" } else { "read_only" }, + ), + )], + ); + let preview = f.owner_call("/preview", &valid, StatusCode::OK).await; + assert_eq!( + f.durable(), + before, + "valid preview must remain a read, before and after cutover" + ); + assert_eq!(secret_counts(), secrets_before); + let saved = f.commit(&valid).await; + assert_eq!( + serde_json::to_value(&saved.state).unwrap(), + preview["state"] + ); + assert_eq!(f.source().await.state, saved.state); + f.assert_legacy_sealed(); + assert_eq!( + secret_counts(), + secrets_before, + "adoption validates metadata without secret side effects" + ); + } + assert!( + f.api + .repository_secrets + .get_credential(TEST_WORKSPACE_ID, "workspace-default", &[]) + .unwrap() + .is_none() + ); + } + #[tokio::test] async fn owner_http_edits_migration_draft_and_adopts_source_policy_head_and_receipt() { let fixture = Fixture::new().await; diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index c3dac5456..9869e3ed0 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -6586,19 +6586,7 @@ impl ControlPlaneStore for SqliteWorkspaceStore { workspace_id: &str, repository_key: &str, ) -> Result> { - self.with_conn(|conn| { - conn.query_row( - r#"SELECT workspace_id, repository_id, repository_key, kind, provider, - source_kind, source_uri, default_ref, - source_fingerprint, observed_status, observed_at, created_at, updated_at - FROM repositories - WHERE workspace_id = ?1 AND repository_key = ?2"#, - params![workspace_id, repository_key], - read_repository_record, - ) - .optional() - .map_err(Error::from) - }) + self.with_conn(|conn| read_repository_by_key(conn, workspace_id, repository_key)) } fn list_repositories(&self, workspace_id: &str) -> Result> { @@ -12225,6 +12213,24 @@ fn validate_repository_record_identity(record: &RepositoryRecord) -> Result<()> Ok(()) } +pub(crate) fn read_repository_by_key( + conn: &Connection, + workspace_id: &str, + repository_key: &str, +) -> Result> { + conn.query_row( + r#"SELECT workspace_id, repository_id, repository_key, kind, provider, + source_kind, source_uri, default_ref, + source_fingerprint, observed_status, observed_at, created_at, updated_at + FROM repositories + WHERE workspace_id = ?1 AND repository_key = ?2"#, + params![workspace_id, repository_key], + read_repository_record, + ) + .optional() + .map_err(Error::from) +} + fn read_repository_record(row: &rusqlite::Row<'_>) -> rusqlite::Result { let source_kind_value = row.get::<_, String>(5)?; let source_kind = server_api::RepositorySourceKind::parse(&source_kind_value)