From fb97edfe95f0fc679f124d507816550d6f07dd8d Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 1 Sep 2026 12:41:14 +0900 Subject: [PATCH] fix: validate workspace schema before owner migration --- crates/workspace-server/src/store.rs | 116 +++++++++++------- .../workspace-server/src/workspace_catalog.rs | 1 - 2 files changed, 73 insertions(+), 44 deletions(-) diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index 579837a3..8114d56c 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -341,9 +341,6 @@ pub enum RepositoryInsertOutcome { pub struct WorkspaceBootstrapRecord { pub operation_key: String, pub request_fingerprint: String, - /// When true, the transaction must prove that no Workspace exists before - /// it inserts this ownerless local-bootstrap Workspace. - pub require_empty_catalog: bool, pub workspace: WorkspaceRecord, pub repository: RepositoryRecord, } @@ -1932,20 +1929,6 @@ impl ControlPlaneStore for SqliteWorkspaceStore { }); } - if record.require_empty_catalog { - let workspace_exists = tx.query_row( - "SELECT EXISTS(SELECT 1 FROM workspaces LIMIT 1)", - [], - |row| row.get::<_, bool>(0), - )?; - if workspace_exists { - return Err(Error::WorkspaceConfigConflict( - "ownerless local bootstrap is available only while the Workspace catalog is empty" - .to_string(), - )); - } - } - if let Some(existing) = tx .query_row( r#"SELECT workspace_id, owner_account_id, display_name, state, created_at, updated_at @@ -7004,33 +6987,64 @@ fn bind_workdir_create_repository_access_evidence(conn: &Connection) -> Result<( Ok(()) } +fn normalize_schema_sql(sql: &str) -> String { + sql.split_ascii_whitespace() + .collect::>() + .join(" ") + .to_ascii_lowercase() +} + fn require_workspace_account_owner(conn: &Connection) -> Result<()> { - let expected_columns = [ - "workspace_id", - "display_name", - "state", - "created_at", - "updated_at", - "owner_account_id", + let actual_columns = { + let mut statement = conn.prepare("PRAGMA table_info(workspaces)")?; + statement + .query_map([], |row| { + Ok(( + row.get::<_, String>(1)?, + row.get::<_, String>(2)?, + row.get::<_, i64>(3)?, + row.get::<_, Option>(4)?, + row.get::<_, i64>(5)?, + )) + })? + .collect::>>()? + }; + let expected_columns = vec![ + ("workspace_id".to_string(), "TEXT".to_string(), 0, None, 1), + ("display_name".to_string(), "TEXT".to_string(), 1, None, 0), + ("state".to_string(), "TEXT".to_string(), 1, None, 0), + ("created_at".to_string(), "TEXT".to_string(), 1, None, 0), + ("updated_at".to_string(), "TEXT".to_string(), 1, None, 0), + ( + "owner_account_id".to_string(), + "TEXT".to_string(), + 0, + None, + 0, + ), ]; - let actual_columns = table_columns(conn, "workspaces")?; - if actual_columns - != expected_columns - .iter() - .map(|column| (*column).to_string()) - .collect::>() - { + if actual_columns != expected_columns { return Err(Error::Store(format!( - "Workspace owner migration rejected workspaces schema drift: expected columns [{}], found [{}]", - expected_columns.join(", "), - actual_columns.join(", ") + "Workspace owner migration rejected workspaces column schema drift: expected {expected_columns:?}, found {actual_columns:?}" ))); } - let owner_not_null = conn.query_row( - "SELECT \"notnull\" FROM pragma_table_info('workspaces') WHERE name = 'owner_account_id'", + let table_sql = conn.query_row( + "SELECT sql FROM sqlite_schema WHERE type = 'table' AND name = 'workspaces'", [], - |row| row.get::<_, i64>(0), + |row| row.get::<_, String>(0), )?; + let expected_table_sql = r#"CREATE TABLE workspaces ( + workspace_id TEXT PRIMARY KEY, + display_name TEXT NOT NULL, + state TEXT NOT NULL, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL + , owner_account_id TEXT REFERENCES accounts(account_id) ON DELETE SET NULL)"#; + if normalize_schema_sql(&table_sql) != normalize_schema_sql(expected_table_sql) { + return Err(Error::Store(format!( + "Workspace owner migration rejected workspaces table SQL drift: found {table_sql}" + ))); + } let owner_foreign_key = conn .query_row( "SELECT \"table\", \"to\", on_delete \ @@ -7046,11 +7060,10 @@ fn require_workspace_account_owner(conn: &Connection) -> Result<()> { }, ) .optional()?; - if owner_not_null != 0 - || owner_foreign_key - .as_ref() - .map(|(table, column, on_delete)| (table.as_str(), column.as_str(), on_delete.as_str())) - != Some(("accounts", "account_id", "SET NULL")) + if owner_foreign_key + .as_ref() + .map(|(table, column, on_delete)| (table.as_str(), column.as_str(), on_delete.as_str())) + != Some(("accounts", "account_id", "SET NULL")) { return Err(Error::Store( "Workspace owner migration rejected owner_account_id schema drift; expected nullable accounts(account_id) with ON DELETE SET NULL" @@ -13988,7 +14001,24 @@ CREATE TABLE ticket_assignment_operations ( fn workspace_owner_migration_rejects_schema_drift_before_rebuild() { let conn = workspace_owner_schema_47(); conn.execute_batch( - "ALTER TABLE workspaces RENAME COLUMN owner_account_id TO legacy_owner_account_id;", + r#" + PRAGMA foreign_keys = OFF; + PRAGMA legacy_alter_table = ON; + BEGIN EXCLUSIVE; + CREATE TABLE workspaces_drift ( + workspace_id TEXT PRIMARY KEY, + display_name TEXT, + state TEXT NOT NULL, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + owner_account_id TEXT REFERENCES accounts(account_id) ON DELETE SET NULL + ); + DROP TABLE workspaces; + ALTER TABLE workspaces_drift RENAME TO workspaces; + COMMIT; + PRAGMA legacy_alter_table = OFF; + PRAGMA foreign_keys = ON; + "#, ) .unwrap(); diff --git a/crates/workspace-server/src/workspace_catalog.rs b/crates/workspace-server/src/workspace_catalog.rs index 58dc6273..531eb6d0 100644 --- a/crates/workspace-server/src/workspace_catalog.rs +++ b/crates/workspace-server/src/workspace_catalog.rs @@ -142,7 +142,6 @@ impl WorkspaceCatalogService { .create_workspace_bootstrap(&WorkspaceBootstrapRecord { operation_key, request_fingerprint: fingerprint.clone(), - require_empty_catalog: false, workspace: WorkspaceRecord { workspace_id: workspace_id.clone(), owner_account_id,