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

fix: harden repository key projections

Browse Source
This commit is contained in:
Hare
2026-09-02 19:53:18 +09:00
parent 96349721cb
commit 0fd1193b6b
5 changed files with 223 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/protocol/src/subscription.rs
+101 -4
View File
@@ -170,6 +170,23 @@ fn validate_identifier(
Ok(())
}
fn validate_repository_key(value: &str) -> Result<(), SubscriptionValidationError> {
let bytes = value.as_bytes();
if bytes.is_empty()
|| bytes.len() > 64
|| bytes.first() == Some(&b'-')
|| bytes.last() == Some(&b'-')
|| !bytes
.iter()
.all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit() || *byte == b'-')
{
return Err(SubscriptionValidationError::InvalidIdentifier {
field: "repository_key",
});
}
Ok(())
}
fn validate_rejection_message(message: &str) -> Result<(), SubscriptionValidationError> {
if message.is_empty() {
return Err(SubscriptionValidationError::EmptyRejectionMessage);
@@ -567,6 +584,7 @@ pub struct SubscriptionWorker {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub profile: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[cfg_attr(feature = "typescript", ts(skip))]
pub repository_id: Option<String>,
/// Workspace-facing Repository key. Runtime producers leave this unset and
/// Workspace Server projections replace `repository_id` with this field.
@@ -588,6 +606,14 @@ impl SubscriptionWorker {
if let Some(repository_id) = &self.repository_id {
validate_identifier("repository_id", repository_id, MAX_RESOURCE_ID_BYTES)?;
}
if let Some(repository_key) = &self.repository_key {
validate_repository_key(repository_key)?;
}
if self.repository_id.is_some() && self.repository_key.is_some() {
return Err(SubscriptionValidationError::InvalidIdentifier {
field: "repository_authority",
});
}
if let Some(working_directory_id) = &self.working_directory_id {
working_directory_id.validate()?;
}
@@ -599,7 +625,13 @@ impl SubscriptionWorker {
#[cfg_attr(feature = "typescript", derive(ts_rs::TS))]
pub struct SubscriptionWorkdir {
pub working_directory_id: SubscriptionWorkdirId,
pub repository_id: String,
/// Runtime-internal Repository id. Workspace-facing TypeScript contracts
/// omit this field and require `repository_key` from the Server projection.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[cfg_attr(feature = "typescript", ts(skip))]
pub repository_id: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub repository_key: Option<String>,
pub state: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub primary_worker_id: Option<SubscriptionWorkerId>,
@@ -608,7 +640,41 @@ pub struct SubscriptionWorkdir {
impl SubscriptionWorkdir {
pub fn validate(&self) -> Result<(), SubscriptionValidationError> {
self.working_directory_id.validate()?;
validate_identifier("repository_id", &self.repository_id, MAX_RESOURCE_ID_BYTES)?;
match (&self.repository_id, &self.repository_key) {
(Some(repository_id), None) => {
validate_identifier("repository_id", repository_id, MAX_RESOURCE_ID_BYTES)?;
}
(None, Some(repository_key)) => validate_repository_key(repository_key)?,
_ => {
return Err(SubscriptionValidationError::InvalidIdentifier {
field: "repository_authority",
});
}
}
validate_identifier("workdir_state", &self.state, MAX_RESOURCE_ID_BYTES)?;
if let Some(worker_id) = &self.primary_worker_id {
worker_id.validate()?;
}
Ok(())
}
}
/// Workspace-facing Workdir summary. Backend-generated Repository UUIDs never
/// enter this DTO; Workspace Server must resolve the required Repository key.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
#[cfg_attr(feature = "typescript", derive(ts_rs::TS))]
pub struct WorkspaceSubscriptionWorkdir {
pub working_directory_id: SubscriptionWorkdirId,
pub repository_key: String,
pub state: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub primary_worker_id: Option<SubscriptionWorkerId>,
}
impl WorkspaceSubscriptionWorkdir {
pub fn validate(&self) -> Result<(), SubscriptionValidationError> {
self.working_directory_id.validate()?;
validate_repository_key(&self.repository_key)?;
validate_identifier("workdir_state", &self.state, MAX_RESOURCE_ID_BYTES)?;
if let Some(worker_id) = &self.primary_worker_id {
worker_id.validate()?;
@@ -629,7 +695,7 @@ pub enum SubscriptionSnapshot {
events: Vec<WorkerProtocolEvent>,
},
WorkspaceWorkdirs {
workdirs: Vec<SubscriptionWorkdir>,
workdirs: Vec<WorkspaceSubscriptionWorkdir>,
},
}
@@ -697,7 +763,7 @@ pub enum SubscriptionEventPayload {
event: WorkerProtocolEvent,
},
WorkdirUpserted {
workdir: SubscriptionWorkdir,
workdir: WorkspaceSubscriptionWorkdir,
},
WorkdirRemoved {
working_directory_id: SubscriptionWorkdirId,
@@ -820,6 +886,37 @@ mod tests {
}
}
#[test]
fn runtime_and_workspace_repository_identity_projections_do_not_alias() {
let mut runtime_worker = worker("worker-1");
runtime_worker.repository_id = Some("01890f47-3c22-7cc0-98c4-dc0c0c07398f".to_string());
runtime_worker.validate().unwrap();
let runtime_json = serde_json::to_value(&runtime_worker).unwrap();
assert_eq!(
runtime_json["repository_id"],
"01890f47-3c22-7cc0-98c4-dc0c0c07398f"
);
assert!(runtime_json.get("repository_key").is_none());
let mut workspace_worker = worker("worker-1");
workspace_worker.repository_key = Some("main".to_string());
workspace_worker.validate().unwrap();
let workspace_json = serde_json::to_value(&workspace_worker).unwrap();
assert_eq!(workspace_json["repository_key"], "main");
assert!(workspace_json.get("repository_id").is_none());
let workspace_workdir = WorkspaceSubscriptionWorkdir {
working_directory_id: SubscriptionWorkdirId::new("workdir-1").unwrap(),
repository_key: "main".to_string(),
state: "active".to_string(),
primary_worker_id: Some(worker_id("worker-1")),
};
workspace_workdir.validate().unwrap();
let workdir_json = serde_json::to_value(&workspace_workdir).unwrap();
assert_eq!(workdir_json["repository_key"], "main");
assert!(workdir_json.get("repository_id").is_none());
}
#[test]
fn subscribe_frame_has_stable_versioned_json_shape() {
let frame = SubscriptionFrame::new(SubscriptionFramePayload::Request(
crates/protocol/src/typescript.rs
+12 -4
View File
@@ -16,9 +16,9 @@ use crate::{
EventSubscriptionSelector, SubscriptionEvent, SubscriptionEventPayload, SubscriptionFrame,
SubscriptionFramePayload, SubscriptionId, SubscriptionRejectionCode, SubscriptionRequest,
SubscriptionRequestId, SubscriptionResponse, SubscriptionSnapshot,
SubscriptionTerminationCode, SubscriptionWorkdir, SubscriptionWorkdirId,
SubscriptionWorker, SubscriptionWorkerId, SubscriptionWorkerIds,
SubscriptionWorkerProtocolMethod, SubscriptionWorkerState,
SubscriptionTerminationCode, SubscriptionWorkdirId, SubscriptionWorker,
SubscriptionWorkerId, SubscriptionWorkerIds, SubscriptionWorkerProtocolMethod,
SubscriptionWorkerState, WorkspaceSubscriptionWorkdir,
},
};
@@ -92,7 +92,7 @@ pub fn generated_protocol_types() -> String {
push_decl::<SubscriptionWorkerState>(&cfg, &mut output);
push_decl::<EventSubscriptionSelector>(&cfg, &mut output);
push_decl::<SubscriptionWorker>(&cfg, &mut output);
push_decl::<SubscriptionWorkdir>(&cfg, &mut output);
push_decl::<WorkspaceSubscriptionWorkdir>(&cfg, &mut output);
push_decl::<SubscriptionSnapshot>(&cfg, &mut output);
push_decl::<SubscriptionEventPayload>(&cfg, &mut output);
push_decl::<SubscriptionRejectionCode>(&cfg, &mut output);
@@ -136,6 +136,14 @@ fn export_decl(decl: &str) -> String {
mod tests {
use super::*;
#[test]
fn workspace_typescript_omits_runtime_repository_ids() {
let generated = generated_protocol_types();
assert!(!generated.contains("repository_id?:"), "{generated}");
assert!(!generated.contains("repository_id:"), "{generated}");
assert!(generated.contains("repository_key"), "{generated}");
}
#[test]
fn generated_protocol_types_are_current() {
let expected = generated_protocol_types();
crates/workspace-server/src/server.rs
+1 -2
View File
@@ -2054,8 +2054,7 @@ impl WorkspaceApi {
})?;
if selected_repository_id.as_deref() != Some(repository_id) {
return Err(ApiError::from(Error::Config(format!(
"Ticket `{ticket_id}` targets repository `{repository_key}`, but the Worker launch resolves `{}`",
selected_repository_id.as_deref().unwrap_or("none")
"Ticket `{ticket_id}` targets Repository `{repository_key}`, but the Worker launch resolves a different Repository"
))));
}
if selected_ref_selector.as_deref() != Some(ref_selector) {
crates/workspace-server/src/store.rs
+105 -32
View File
@@ -1903,6 +1903,7 @@ impl ControlPlaneStore for SqliteWorkspaceStore {
&self,
record: &WorkspaceBootstrapRecord,
) -> Result<WorkspaceBootstrapResult> {
validate_repository_record_identity(&record.repository)?;
self.with_conn_mut(|conn| {
let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?;
let owner_kind = tx
@@ -1959,43 +1960,18 @@ impl ControlPlaneStore for SqliteWorkspaceStore {
});
}
if let Some(existing) = tx
if tx
.query_row(
r#"SELECT workspace_id, owner_account_id, display_name, state, created_at, updated_at
FROM workspaces WHERE workspace_id = ?1"#,
"SELECT 1 FROM workspaces WHERE workspace_id = ?1",
params![record.workspace.workspace_id],
read_workspace_record,
|row| row.get::<_, i64>(0),
)
.optional()?
.is_some()
{
if existing.owner_account_id != record.workspace.owner_account_id
|| existing.display_name != record.workspace.display_name
|| existing.state != record.workspace.state
{
return Err(Error::WorkspaceConfigConflict(
"Workspace identity already exists with different metadata".to_string(),
));
}
let existing_repository = tx
.query_row(
r#"SELECT workspace_id, repository_id, repository_key, kind, provider,
source_kind, source_uri, default_ref, source_revision,
source_fingerprint, observed_status, observed_at, created_at, updated_at
FROM repositories WHERE workspace_id = ?1 AND repository_key = ?2"#,
params![record.repository.workspace_id, record.repository.repository_key],
read_repository_record,
)
.optional()?;
if existing_repository.as_ref().is_none_or(|existing| {
let mut requested = record.repository.clone();
requested.repository_id.clone_from(&existing.repository_id);
existing != &requested
}) {
return Err(Error::WorkspaceConfigConflict(
"Workspace initial repository already exists with different metadata"
.to_string(),
));
}
return Err(Error::WorkspaceConfigConflict(
"Workspace identity already exists".to_string(),
));
} else {
tx.execute(
r#"INSERT INTO workspaces (
@@ -13786,6 +13762,103 @@ CREATE TABLE ticket_assignment_operations (
);
}
#[tokio::test]
async fn workspace_bootstrap_validates_key_and_rejects_duplicate_workspace() {
let store = SqliteWorkspaceStore::in_memory().unwrap();
store
.upsert_account(&AccountRecord {
account_id: "owner-account".to_string(),
kind: "user".to_string(),
handle: "owner".to_string(),
display_name: "Owner".to_string(),
created_at: "1".to_string(),
updated_at: "1".to_string(),
})
.unwrap();
let workspace = WorkspaceRecord {
workspace_id: "workspace-invalid-key".to_string(),
owner_account_id: "owner-account".to_string(),
display_name: "Invalid key".to_string(),
state: "active".to_string(),
created_at: "1".to_string(),
updated_at: "1".to_string(),
};
let repository = RepositoryRecord {
workspace_id: workspace.workspace_id.clone(),
repository_id: Uuid::now_v7().to_string(),
repository_key: "Invalid_Key".to_string(),
kind: "git".to_string(),
provider: Some("git".to_string()),
source: workspace_api::RepositorySource {
kind: workspace_api::RepositorySourceKind::LocalPath,
uri: "/repo".to_string(),
},
default_ref: Some("develop".to_string()),
source_revision: 1,
source_fingerprint: "sha256:test".to_string(),
observed_status: RepositoryObservedStatus::Unverified,
observed_at: None,
created_at: "1".to_string(),
updated_at: "1".to_string(),
};
let error = store
.create_workspace_bootstrap(&WorkspaceBootstrapRecord {
operation_key: "invalid-key".to_string(),
request_fingerprint: "sha256:invalid-key".to_string(),
workspace: workspace.clone(),
repository,
})
.unwrap_err()
.to_string();
assert!(error.contains("invalid Repository key"), "{error}");
assert!(
store
.get_workspace(&workspace.workspace_id)
.await
.unwrap()
.is_none()
);
let valid_repository = RepositoryRecord {
workspace_id: workspace.workspace_id.clone(),
repository_id: Uuid::now_v7().to_string(),
repository_key: "main".to_string(),
kind: "git".to_string(),
provider: Some("git".to_string()),
source: workspace_api::RepositorySource {
kind: workspace_api::RepositorySourceKind::LocalPath,
uri: "/repo".to_string(),
},
default_ref: Some("develop".to_string()),
source_revision: 1,
source_fingerprint: "sha256:test".to_string(),
observed_status: RepositoryObservedStatus::Unverified,
observed_at: None,
created_at: "1".to_string(),
updated_at: "1".to_string(),
};
let first = WorkspaceBootstrapRecord {
operation_key: "create-workspace".to_string(),
request_fingerprint: "sha256:create-workspace".to_string(),
workspace,
repository: valid_repository,
};
assert!(!store.create_workspace_bootstrap(&first).unwrap().replayed);
let mut duplicate = first;
duplicate.operation_key = "duplicate-workspace".to_string();
duplicate.repository.repository_id = Uuid::now_v7().to_string();
let error = store
.create_workspace_bootstrap(&duplicate)
.unwrap_err()
.to_string();
assert!(
error.contains("Workspace identity already exists"),
"{error}"
);
}
#[tokio::test]
async fn repository_records_round_trip() {
let store = SqliteWorkspaceStore::in_memory().unwrap();