diff --git a/backend/.sqlx/query-07bcd445061f34a5d370398ee56d98e0e8f42e9e4fd70f00ee5f120fc3b04ed9.json b/backend/.sqlx/query-07bcd445061f34a5d370398ee56d98e0e8f42e9e4fd70f00ee5f120fc3b04ed9.json new file mode 100644 index 0000000000..7bfbd5aae3 --- /dev/null +++ b/backend/.sqlx/query-07bcd445061f34a5d370398ee56d98e0e8f42e9e4fd70f00ee5f120fc3b04ed9.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM job_resolution WHERE workspace_id = $1 AND job_id = ANY($2)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "UuidArray" + ] + }, + "nullable": [] + }, + "hash": "07bcd445061f34a5d370398ee56d98e0e8f42e9e4fd70f00ee5f120fc3b04ed9" +} diff --git a/backend/.sqlx/query-0dafd0882f28604872a9187a5a9b3e707644fa3b9833328e75cf599df11e22b2.json b/backend/.sqlx/query-0dafd0882f28604872a9187a5a9b3e707644fa3b9833328e75cf599df11e22b2.json new file mode 100644 index 0000000000..8c21761ab0 --- /dev/null +++ b/backend/.sqlx/query-0dafd0882f28604872a9187a5a9b3e707644fa3b9833328e75cf599df11e22b2.json @@ -0,0 +1,12 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM job_resolution jr WHERE NOT EXISTS (SELECT 1 FROM v2_job WHERE id = jr.job_id)", + "describe": { + "columns": [], + "parameters": { + "Left": [] + }, + "nullable": [] + }, + "hash": "0dafd0882f28604872a9187a5a9b3e707644fa3b9833328e75cf599df11e22b2" +} diff --git a/backend/.sqlx/query-13d8e65ffd3ef6bcfb963525fc3ec0859463b1c153950f894a830a4b5ec33fcc.json b/backend/.sqlx/query-13d8e65ffd3ef6bcfb963525fc3ec0859463b1c153950f894a830a4b5ec33fcc.json new file mode 100644 index 0000000000..9cdddc5121 --- /dev/null +++ b/backend/.sqlx/query-13d8e65ffd3ef6bcfb963525fc3ec0859463b1c153950f894a830a4b5ec33fcc.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO job_resolution (job_id, workspace_id, resolved_by, note, automatic)\n SELECT c.id, c.workspace_id, $4, COALESCE($5, $7), false\n FROM v2_job_completed c\n JOIN v2_job j ON j.id = c.id\n WHERE c.id = ANY($1)\n AND c.workspace_id = $2\n AND ($3::TEXT[] IS NULL OR j.tag = ANY($3))\n AND c.status = 'failure'\n -- Resolution is a top-level triage state: a step resolved on its own\n -- would render orange inside a flow whose status is still red.\n AND j.flow_step_id IS NULL\n -- A supersession claim has to be proven, not trusted: a later success of the\n -- same identified runnable, itself visible to the caller. An unproven claim\n -- resolves nothing, so the caller learns it was rejected instead of having\n -- the fiction recorded as provenance.\n AND ($6::UUID IS NULL OR EXISTS (\n SELECT 1 FROM v2_job_completed sc\n JOIN v2_job sj ON sj.id = sc.id\n WHERE sc.id = $6\n AND sc.workspace_id = $2\n -- Tag scope is a read restriction enforced outside RLS, so it has\n -- to bind the evidence as well: otherwise the result reveals\n -- whether an out-of-scope run succeeded.\n AND ($3::TEXT[] IS NULL OR sj.tag = ANY($3))\n AND sc.status = 'success'\n AND sc.completed_at >= c.completed_at\n AND (j.runnable_id IS NOT NULL OR j.runnable_path IS NOT NULL)\n AND sj.runnable_id IS NOT DISTINCT FROM j.runnable_id\n AND sj.runnable_path IS NOT DISTINCT FROM j.runnable_path\n ))\n ON CONFLICT (job_id) DO UPDATE SET\n resolved_at = now(),\n -- Both COALESCEd: `resolution_attribution` returns NULLs outside EE and once the\n -- licence lapses, and bulk selections routinely include already-resolved rows,\n -- so overwriting would erase metadata recorded while it was valid. Clear either\n -- by unresolving first.\n resolved_by = COALESCE($4, job_resolution.resolved_by),\n -- A person's explanation replaces what was there; machine provenance only fills\n -- a blank, so re-running an already-explained failure never erases their words.\n note = COALESCE($5, job_resolution.note, $7),\n -- A human taking over an automatic resolution makes it no longer automatic.\n automatic = false\n RETURNING job_id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "job_id", + "type_info": "Uuid" + } + ], + "parameters": { + "Left": [ + "UuidArray", + "Text", + "TextArray", + "Varchar", + "Text", + "Uuid", + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "13d8e65ffd3ef6bcfb963525fc3ec0859463b1c153950f894a830a4b5ec33fcc" +} diff --git a/backend/.sqlx/query-e5c2eb0b1303e0310d1002562fabcdb68b75a2a86141a6b2911f4c4625f2d075.json b/backend/.sqlx/query-3258241cf28c17d9b5dcf06f6a3e4b182ae4393eb8fb22d8521de538704d63fd.json similarity index 71% rename from backend/.sqlx/query-e5c2eb0b1303e0310d1002562fabcdb68b75a2a86141a6b2911f4c4625f2d075.json rename to backend/.sqlx/query-3258241cf28c17d9b5dcf06f6a3e4b182ae4393eb8fb22d8521de538704d63fd.json index 30a8967790..0afe235d33 100644 --- a/backend/.sqlx/query-e5c2eb0b1303e0310d1002562fabcdb68b75a2a86141a6b2911f4c4625f2d075.json +++ b/backend/.sqlx/query-3258241cf28c17d9b5dcf06f6a3e4b182ae4393eb8fb22d8521de538704d63fd.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT role_name, datatable FROM datatable_ephemeral_role\n WHERE workspace_id = $1 AND ($2::text IS NULL OR datatable = $2)", + "query": "SELECT role_name, datatable FROM datatable_ephemeral_role WHERE workspace_id = $1", "describe": { "columns": [ { @@ -16,7 +16,6 @@ ], "parameters": { "Left": [ - "Text", "Text" ] }, @@ -25,5 +24,5 @@ false ] }, - "hash": "e5c2eb0b1303e0310d1002562fabcdb68b75a2a86141a6b2911f4c4625f2d075" + "hash": "3258241cf28c17d9b5dcf06f6a3e4b182ae4393eb8fb22d8521de538704d63fd" } diff --git a/backend/.sqlx/query-41468bb803b63c190728938616a46409057a16a2723916d973eac5c70c2dc2c7.json b/backend/.sqlx/query-41468bb803b63c190728938616a46409057a16a2723916d973eac5c70c2dc2c7.json new file mode 100644 index 0000000000..a8ddb02aea --- /dev/null +++ b/backend/.sqlx/query-41468bb803b63c190728938616a46409057a16a2723916d973eac5c70c2dc2c7.json @@ -0,0 +1,24 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM job_resolution r\n USING v2_job_completed c\n JOIN v2_job j ON j.id = c.id\n WHERE r.job_id = c.id\n AND c.id = ANY($1)\n AND c.workspace_id = $2\n AND ($3::TEXT[] IS NULL OR j.tag = ANY($3))\n RETURNING r.job_id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "job_id", + "type_info": "Uuid" + } + ], + "parameters": { + "Left": [ + "UuidArray", + "Text", + "TextArray" + ] + }, + "nullable": [ + false + ] + }, + "hash": "41468bb803b63c190728938616a46409057a16a2723916d973eac5c70c2dc2c7" +} diff --git a/backend/.sqlx/query-58bef783a5d85eb114293bd0e43b01013e27017765b3fdade4322ebdf1fe1408.json b/backend/.sqlx/query-58bef783a5d85eb114293bd0e43b01013e27017765b3fdade4322ebdf1fe1408.json new file mode 100644 index 0000000000..a18f3730e6 --- /dev/null +++ b/backend/.sqlx/query-58bef783a5d85eb114293bd0e43b01013e27017765b3fdade4322ebdf1fe1408.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM job_resolution WHERE job_id = ANY($1)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "UuidArray" + ] + }, + "nullable": [] + }, + "hash": "58bef783a5d85eb114293bd0e43b01013e27017765b3fdade4322ebdf1fe1408" +} diff --git a/backend/.sqlx/query-9b7255c12350fa9e493d6dd0d85c3fe5c6884cb18252390ebcfcd5fcad1c345f.json b/backend/.sqlx/query-9b7255c12350fa9e493d6dd0d85c3fe5c6884cb18252390ebcfcd5fcad1c345f.json new file mode 100644 index 0000000000..ffcf505398 --- /dev/null +++ b/backend/.sqlx/query-9b7255c12350fa9e493d6dd0d85c3fe5c6884cb18252390ebcfcd5fcad1c345f.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT owner_creds, perms_hash FROM datatable_ephemeral_role WHERE role_name = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "owner_creds", + "type_info": "Text" + }, + { + "ordinal": 1, + "name": "perms_hash", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + true, + false + ] + }, + "hash": "9b7255c12350fa9e493d6dd0d85c3fe5c6884cb18252390ebcfcd5fcad1c345f" +} diff --git a/backend/.sqlx/query-a99066262acf851bd6f5f05c75d6fd43ebe1e723660cb6c894b1c914842a8f8e.json b/backend/.sqlx/query-a99066262acf851bd6f5f05c75d6fd43ebe1e723660cb6c894b1c914842a8f8e.json new file mode 100644 index 0000000000..c19f95172a --- /dev/null +++ b/backend/.sqlx/query-a99066262acf851bd6f5f05c75d6fd43ebe1e723660cb6c894b1c914842a8f8e.json @@ -0,0 +1,35 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT role_name, datatable, perms_hash FROM datatable_ephemeral_role\n WHERE workspace_id = $1 AND ($2::text IS NULL OR datatable = $2)", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "role_name", + "type_info": "Text" + }, + { + "ordinal": 1, + "name": "datatable", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "perms_hash", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false, + false, + false + ] + }, + "hash": "a99066262acf851bd6f5f05c75d6fd43ebe1e723660cb6c894b1c914842a8f8e" +} diff --git a/backend/.sqlx/query-c65ce3580a6648a7168c0003d9ec4465bc94bf7845f1d38f6d0fd20f1154ff9e.json b/backend/.sqlx/query-c65ce3580a6648a7168c0003d9ec4465bc94bf7845f1d38f6d0fd20f1154ff9e.json new file mode 100644 index 0000000000..d8cb1aff4b --- /dev/null +++ b/backend/.sqlx/query-c65ce3580a6648a7168c0003d9ec4465bc94bf7845f1d38f6d0fd20f1154ff9e.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "WITH ids AS (SELECT id FROM v2_job WHERE workspace_id = $1),\n _de AS (DELETE FROM dispatch_event WHERE workspace_id = $1),\n _jr AS (DELETE FROM job_resolution WHERE workspace_id = $1),\n _fc AS (DELETE FROM flow_conversation_message WHERE job_id IN (SELECT id FROM ids))\n DELETE FROM zombie_job_counter WHERE job_id IN (SELECT id FROM ids)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [] + }, + "hash": "c65ce3580a6648a7168c0003d9ec4465bc94bf7845f1d38f6d0fd20f1154ff9e" +} diff --git a/backend/.sqlx/query-ddb7d633931e71c1620a10711593e323c6dd4eae99d42e098cb0d0f8ea8305fa.json b/backend/.sqlx/query-ddb7d633931e71c1620a10711593e323c6dd4eae99d42e098cb0d0f8ea8305fa.json deleted file mode 100644 index e4e382e61b..0000000000 --- a/backend/.sqlx/query-ddb7d633931e71c1620a10711593e323c6dd4eae99d42e098cb0d0f8ea8305fa.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT owner_creds FROM datatable_ephemeral_role WHERE role_name = $1", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "owner_creds", - "type_info": "Text" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - true - ] - }, - "hash": "ddb7d633931e71c1620a10711593e323c6dd4eae99d42e098cb0d0f8ea8305fa" -} diff --git a/backend/.sqlx/query-e34cb7cd6ff9d80e77a51ae47c3cfb9b0772d5d3d239273fc5cea322cae4e258.json b/backend/.sqlx/query-e34cb7cd6ff9d80e77a51ae47c3cfb9b0772d5d3d239273fc5cea322cae4e258.json new file mode 100644 index 0000000000..4ebad828df --- /dev/null +++ b/backend/.sqlx/query-e34cb7cd6ff9d80e77a51ae47c3cfb9b0772d5d3d239273fc5cea322cae4e258.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO job_resolution (job_id, workspace_id, resolved_by, note, automatic)\n SELECT c.id, c.workspace_id, NULL, NULL, true\n FROM v2_job_completed c\n JOIN v2_job j ON j.id = c.id\n WHERE c.status = 'failure'\n AND c.workspace_id = $2\n AND j.flow_step_id IS NULL\n AND j.runnable_id IS NOT DISTINCT FROM $3\n AND EXISTS (\n SELECT 1 FROM v2_job_completed sc\n JOIN v2_job sj ON sj.id = sc.id\n JOIN native_retry_attempt nra ON nra.job_id = sc.id\n WHERE sc.status = 'success'\n AND sc.workspace_id = $2\n AND sj.parent_job = $1\n AND sj.runnable_id IS NOT DISTINCT FROM $3\n )\n AND (\n (j.parent_job = $1\n AND EXISTS (SELECT 1 FROM native_retry_attempt WHERE job_id = c.id))\n OR (c.id = $1 AND j.parent_job IS NULL\n AND NOT EXISTS (SELECT 1 FROM native_retry_attempt WHERE job_id = c.id))\n )\n ON CONFLICT (job_id) DO NOTHING", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Uuid", + "Text", + "Int8" + ] + }, + "nullable": [] + }, + "hash": "e34cb7cd6ff9d80e77a51ae47c3cfb9b0772d5d3d239273fc5cea322cae4e258" +} diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index adf859d139..b1d5f74ada 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -1259,6 +1259,12 @@ async fn report_token_expiration(db: &DB, token: &TokenRow, expired: bool) { } pub async fn delete_expired_items(db: &DB) -> () { + // Expired data table ephemeral roles must be revoked even when no new role + // creation happens to trigger the opportunistic sweep — a revoked caller + // could otherwise keep reconnecting with a self-set password until some + // unrelated access finally sweeps. + windmill_common::datatable_permissions::cleanup_expired_datatable_roles(db, 20).await; + let expired_tokens_r = sqlx::query_as!( TokenRow, "DELETE FROM token WHERE expiration <= now() diff --git a/backend/windmill-api-workspaces/src/datatable_permissions_api.rs b/backend/windmill-api-workspaces/src/datatable_permissions_api.rs index 71b59d7984..77db4f71e0 100644 --- a/backend/windmill-api-workspaces/src/datatable_permissions_api.rs +++ b/backend/windmill-api-workspaces/src/datatable_permissions_api.rs @@ -12,7 +12,7 @@ use windmill_audit::audit_oss::audit_log; use windmill_audit::ActionKind; use windmill_common::datatable_permissions::{ compute_effective_grants, datatable_license_valid, datatable_permissions_enabled, - drop_datatable_ephemeral_roles_best_effort, validate_grant_identifier, + snapshot_datatable_roles, teardown_snapshot_roles_best_effort, validate_grant_identifier, PERMISSIONED_AS_FOLDER_PREFIX, }; use windmill_common::error::{Error, JsonResult, Result}; @@ -252,6 +252,10 @@ async fn set_datatable_permissions( ), ) .await?; + // Captured before the commit so the post-commit teardown only revokes the + // pre-edit role generation — a role concurrently recreated from the new + // grants must keep working. + let pre_edit_roles = snapshot_datatable_roles(&db, &w_id, Some(&datatable_name)).await?; let perms_json = serde_json::to_value(&perms).map_err(|e| Error::internal_err(e.to_string()))?; sqlx::query!( @@ -266,10 +270,9 @@ async fn set_datatable_permissions( .await?; tx.commit().await?; - // Existing ephemeral roles were built from the previous grants; drop them - // so nothing outlives the edit (new accesses would recreate them via the - // perms-hash check anyway, and disabled data tables stop using them). - drop_datatable_ephemeral_roles_best_effort(&db, &w_id, Some(&datatable_name)).await; + // The pre-edit roles were built from the previous grants; revoke them so + // nothing outlives the edit (active holders get NOLOGIN'd, the rest drop). + teardown_snapshot_roles_best_effort(&db, &w_id, pre_edit_roles).await; Ok(format!( "Edited permissions of data table {datatable_name} in workspace {w_id}" diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 2dd7b75284..3c3639dc3e 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -3205,27 +3205,36 @@ async fn edit_datatable_config( ) .await?; - // Tear down ephemeral roles of deleted/renamed/re-pointed data tables - // while the old config (needed to reach their previous database) is still - // readable outside this transaction — after the switch the cleanup sweep - // would look for the roles on the wrong cluster. Renames and database - // changes get fresh roles on next access. + // Capture the roles of deleted/renamed/re-pointed data tables before the + // commit: the post-commit teardown revokes exactly this pre-edit + // generation (their rows carry the recorded previous target), while any + // role recreated from the new config keeps working. + let mut pre_edit_roles = vec![]; for name in new_config .deleted_datatables .iter() .chain(new_config.renames.iter().map(|r| &r.from)) .chain(database_changed_names.iter()) { - windmill_common::datatable_permissions::drop_datatable_ephemeral_roles_best_effort( - &db, - &w_id, - Some(name), - ) - .await; + pre_edit_roles.extend( + windmill_common::datatable_permissions::snapshot_datatable_roles( + &db, + &w_id, + Some(name), + ) + .await?, + ); } tx.commit().await?; + windmill_common::datatable_permissions::teardown_snapshot_roles_best_effort( + &db, + &w_id, + pre_edit_roles, + ) + .await; + Ok(format!("Edit datatable config for workspace {}", &w_id)) } diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index 9047fe6dc6..0852da6713 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -928,7 +928,13 @@ pub(crate) async fn delete_workspace( // rows and the workspace key (which encrypts their recorded targets) still // exist — both cascade with the workspace row, after which an unrevoked // role could never be reached again. Failure (e.g. an unreachable external - // cluster) aborts the deletion; retry once the database is reachable. + // cluster) aborts the deletion; retry once the database is reachable. The + // workspace-scoped lock (held on this transaction until commit) keeps a + // concurrent resolution from creating a fresh role after the teardown scan. + sqlx::query(windmill_common::datatable_permissions::WORKSPACE_ROLES_LOCK) + .bind(&w_id) + .execute(&mut *tx) + .await?; windmill_common::datatable_permissions::teardown_datatable_roles_strict(&db, &w_id) .await .map_err(|e| { diff --git a/backend/windmill-common/src/datatable_permissions.rs b/backend/windmill-common/src/datatable_permissions.rs index b50a3f96d2..202402ad6b 100644 --- a/backend/windmill-common/src/datatable_permissions.rs +++ b/backend/windmill-common/src/datatable_permissions.rs @@ -27,6 +27,12 @@ use crate::{ /// to touch a role that does not carry it. pub const DATATABLE_EPHEMERAL_ROLE_PREFIX: &str = "wm_dt_"; +/// Transaction-scoped advisory lock serializing a workspace's ephemeral role +/// creation with its deletion-time strict teardown. Bind the workspace id. +/// Lock order where both are taken: workspace lock first, then per-role lock. +pub const WORKSPACE_ROLES_LOCK: &str = + "SELECT pg_advisory_xact_lock(hashtextextended('wm_dt_ws:' || $1::text, 0))"; + pub const PERMISSIONED_AS_FOLDER_PREFIX: &str = "f/"; const EPHEMERAL_ROLE_CONNECTION_LIMIT: u32 = 25; @@ -586,7 +592,7 @@ async fn drop_or_disable_on_target( .map_err(|e| pg_err("checking active sessions", e))? .get(0); if active > 0 { - disable_role_login(&client, role).await; + disable_role_login(&client, role).await?; return Ok(DropOutcome::SkippedActive); } if role_exists(&client, role).await? { @@ -652,8 +658,16 @@ async fn ensure_ephemeral_role( // Slow path: (re)create the role under a per-role advisory lock on the // main DB (advisory locks are per-database — never take them on the - // target cluster). + // target cluster). The workspace-scoped lock (taken first — same order as + // workspace deletion, which holds it from strict teardown through its + // commit) keeps a creation from racing workspace deletion: a role created + // after deletion's teardown scan would be orphaned forever once the + // bookkeeping rows and workspace key cascade away. let mut tx = db.begin().await?; + sqlx::query(WORKSPACE_ROLES_LOCK) + .bind(w_id) + .execute(&mut *tx) + .await?; sqlx::query("SELECT pg_advisory_xact_lock(hashtextextended('wm_dt_role:' || $1, 0))") .bind(&role) .execute(&mut *tx) @@ -910,47 +924,81 @@ async fn cleanup_one_expired_role(db: &DB, role: &str, w_id: &str, datatable: &s Ok(()) } -/// Best-effort teardown of every ephemeral role of a data table (or a whole -/// workspace when `datatable` is `None`), used on data table deletion, -/// database re-points and permission edits. Roles that cannot be dropped keep -/// their bookkeeping row (with the recorded target), so the expiry sweep -/// retries them — failure here loses nothing. -/// -/// Authorization: mutates cluster roles; call only from admin-gated flows. -pub async fn drop_datatable_ephemeral_roles_best_effort( +/// One bookkeeping row captured before a permissions/config edit commits. +/// The post-commit teardown only revokes roles still carrying the captured +/// hash: a role concurrently recreated from the NEW config has a different +/// hash and must keep working. +pub struct RoleSnapshot { + role_name: String, + datatable: String, + perms_hash: String, +} + +/// Capture the current roles of a data table (or whole workspace) with their +/// perms hashes. Take this BEFORE committing the edit that invalidates them +/// (ideally under the same serialization lock), then pass it to +/// [`teardown_snapshot_roles_best_effort`] after the commit. +pub async fn snapshot_datatable_roles( db: &DB, w_id: &str, datatable: Option<&str>, -) { - if let Err(e) = teardown_datatable_roles(db, w_id, datatable).await { - tracing::warn!("tearing down datatable ephemeral roles: {e:#}"); - } -} - -/// Strict variant for workspace deletion: the bookkeeping rows (and the -/// workspace key their targets are encrypted with) are about to cascade away, -/// so every role must be revoked NOW — dropped, or at least stripped of LOGIN -/// when sessions are still active. Any failure (e.g. an unreachable external -/// cluster) must abort the deletion; proceeding would orphan a live LOGIN -/// role with no remaining way to ever revoke it. -/// -/// Authorization: mutates cluster roles; call only from admin-gated flows. -pub async fn teardown_datatable_roles_strict(db: &DB, w_id: &str) -> Result<()> { - teardown_datatable_roles(db, w_id, None).await -} - -async fn teardown_datatable_roles(db: &DB, w_id: &str, datatable: Option<&str>) -> Result<()> { - let rows = sqlx::query!( - "SELECT role_name, datatable FROM datatable_ephemeral_role +) -> Result> { + Ok(sqlx::query!( + "SELECT role_name, datatable, perms_hash FROM datatable_ephemeral_role WHERE workspace_id = $1 AND ($2::text IS NULL OR datatable = $2)", w_id, datatable ) .fetch_all(db) + .await? + .into_iter() + .map(|r| RoleSnapshot { + role_name: r.role_name, + datatable: r.datatable, + perms_hash: r.perms_hash, + }) + .collect()) +} + +/// Best-effort teardown of a pre-edit role generation, used after permission +/// edits, data table deletion/renames and database re-points commit. Roles +/// that cannot be dropped keep their bookkeeping row (with the recorded +/// target), so the expiry sweep retries them — failure here loses nothing. +/// +/// Authorization: mutates cluster roles; call only from admin-gated flows. +pub async fn teardown_snapshot_roles_best_effort(db: &DB, w_id: &str, snapshot: Vec) { + for s in snapshot { + if let Err(e) = + teardown_role(db, w_id, &s.datatable, &s.role_name, Some(&s.perms_hash)).await + { + tracing::warn!( + "tearing down datatable ephemeral role {}: {e:#}", + s.role_name + ); + } + } +} + +/// Strict teardown of every role of a workspace, for workspace deletion: the +/// bookkeeping rows (and the workspace key their targets are encrypted with) +/// are about to cascade away, so every role must be revoked NOW — dropped, or +/// at least stripped of LOGIN when sessions are still active. Any failure +/// (e.g. an unreachable external cluster) must abort the deletion; proceeding +/// would orphan a live LOGIN role with no remaining way to ever revoke it. +/// Callers must hold [`WORKSPACE_ROLES_LOCK`] until the deletion commits so +/// no new role is created after this scan. +/// +/// Authorization: mutates cluster roles; call only from admin-gated flows. +pub async fn teardown_datatable_roles_strict(db: &DB, w_id: &str) -> Result<()> { + let rows = sqlx::query!( + "SELECT role_name, datatable FROM datatable_ephemeral_role WHERE workspace_id = $1", + w_id + ) + .fetch_all(db) .await?; let mut failures: Vec = vec![]; for row in rows { - if let Err(e) = teardown_role(db, w_id, &row.datatable, &row.role_name).await { + if let Err(e) = teardown_role(db, w_id, &row.datatable, &row.role_name, None).await { failures.push(format!("{} ({}): {e:#}", row.role_name, row.datatable)); } } @@ -970,19 +1018,22 @@ async fn teardown_datatable_roles(db: &DB, w_id: &str, datatable: Option<&str>) /// revocation. Only CREATEROLE can restore LOGIN, so this closes the /// reconnect vector while letting in-flight queries finish; the kept /// bookkeeping row makes the expiry sweep finish the drop later. -async fn disable_role_login(client: &tokio_postgres::Client, role: &str) { +/// Failure is propagated: callers (notably strict workspace-deletion +/// teardown) must not treat an active role as revoked when it still holds +/// LOGIN. +async fn disable_role_login(client: &tokio_postgres::Client, role: &str) -> Result<()> { if !role.starts_with(DATATABLE_EPHEMERAL_ROLE_PREFIX) { - return; + return Err(Error::internal_err(format!( + "refusing to alter role '{role}' without the reserved prefix" + ))); } - if let Err(e) = client + client .batch_execute(&format!( "ALTER ROLE {} NOLOGIN CONNECTION LIMIT 0 PASSWORD NULL", quote_ident(role) )) .await - { - tracing::warn!("disabling login of ephemeral role {role}: {e:#}"); - } + .map_err(|e| pg_err(&format!("disabling login of ephemeral role {role}"), e)) } /// Where a role must be revoked: the stored owner target from its bookkeeping @@ -1023,7 +1074,13 @@ async fn resolve_role_target( } } -async fn teardown_role(db: &DB, w_id: &str, datatable: &str, role: &str) -> Result<()> { +async fn teardown_role( + db: &DB, + w_id: &str, + datatable: &str, + role: &str, + only_if_hash: Option<&str>, +) -> Result<()> { // Same per-role lock as role creation and the expiry sweep: without it, // teardown could observe a stale role while a resolver is recreating it // and drop the fresh role right before its bookkeeping row lands, leaving @@ -1033,13 +1090,21 @@ async fn teardown_role(db: &DB, w_id: &str, datatable: &str, role: &str) -> Resu .bind(role) .execute(&mut *tx) .await?; - let owner_creds = sqlx::query_scalar!( - "SELECT owner_creds FROM datatable_ephemeral_role WHERE role_name = $1", + let row = sqlx::query!( + "SELECT owner_creds, perms_hash FROM datatable_ephemeral_role WHERE role_name = $1", role ) .fetch_optional(&mut *tx) - .await? - .flatten(); + .await?; + if let (Some(expected), Some(row)) = (only_if_hash, row.as_ref()) { + // The role was recreated from the post-edit config while we were + // getting here — it is current, not the generation this teardown + // targets. Leave it alone. + if row.perms_hash != expected { + return Ok(()); + } + } + let owner_creds = row.and_then(|r| r.owner_creds); if let Some((owner, is_instance)) = resolve_role_target(db, w_id, datatable, owner_creds).await { if matches!(