starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-05 16:03:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(datatables): a data table that keeps its database keeps its roles

Browse Source 
The fork-database drop cleared the permissions block and destroyed the
logins above the guards that refuse the drop, so a refusal — or a DROP
DATABASE that fails on an open session — left a live data table
unpermissioned, and every member of the workspace resolving to the
connection that owns it. The clear now happens only once the drop is
under way, and is put back if the drop does not happen.

The workspace-deletion snapshot takes the settings row first, so a
permissions save cannot add a login between the read and the row's
deletion.

The raw-app data drawer waits for the caller's usable roles before
mounting the content, like its sibling in the DB manager: mounting is
what fires the schema and metadata queries, and a first round sent
without a role runs — and caches — as whatever the server defaults to.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5arH3G2Sa1Qqm32veJQ1n
This commit is contained in:
Diego Imbert
2026-09-04 20:08:14 +02:00
co-authored by Claude Opus 5
parent 95c3059266
commit 1e6d28def2
5 changed files with 218 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/.sqlx/query-161cc8750a9c6f22b28bc0cc565c621cd3e6ee378587620deb0ea2f69f1fcbdb.json
Generated
+16
View File
@@ -0,0 +1,16 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE workspace_settings\n SET datatable = jsonb_set(datatable, ARRAY['datatables', $2, 'permissions'], $3)\n WHERE workspace_id = $1\n AND datatable->'datatables' ? $2",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text",
"Jsonb"
]
},
"nullable": []
},
"hash": "161cc8750a9c6f22b28bc0cc565c621cd3e6ee378587620deb0ea2f69f1fcbdb"
}
backend/.sqlx/query-2daf3786f0b7195a3acfd80e01f2dba171ae5850f22503e846b7355a4f250611.json
Generated
+15
View File
@@ -0,0 +1,15 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE workspace_settings\n SET datatable = datatable #- ARRAY['datatables', $2, 'permissions']\n WHERE workspace_id = $1",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": []
},
"hash": "2daf3786f0b7195a3acfd80e01f2dba171ae5850f22503e846b7355a4f250611"
}
backend/.sqlx/query-7845539644254c5f6394241b2207209d52a54104d1fb6632468f63ba691a3ed0.json
Generated
-15
View File
@@ -1,15 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE workspace_settings\n SET datatable = datatable #- ARRAY['datatables', $2, 'permissions']\n WHERE workspace_id = $1",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": []
},
"hash": "7845539644254c5f6394241b2207209d52a54104d1fb6632468f63ba691a3ed0"
}
backend/windmill-api-workspaces/src/workspaces_extra.rs
+115 -39
View File
@@ -975,14 +975,13 @@ pub(crate) async fn delete_workspace(
// database whose only record is the settings row below, so what to drop is
// resolved while that row is here and the drop itself runs after the commit.
// Nothing is dropped here.
// The first row this transaction locks — see `lock_workspace_settings_unchecked`.
// Without it a permissions save that commits between this read and the row's
// deletion adds a login nothing then drops, and the config that named it is
// gone.
let mut planned_role_drops = Vec::new();
let datatable_config: Option<serde_json::Value> = sqlx::query_scalar!(
"SELECT datatable FROM workspace_settings WHERE workspace_id = $1",
&w_id
)
.fetch_optional(&mut *tx)
.await?
.flatten();
let datatable_config =
windmill_common::workspaces::lock_workspace_settings_unchecked(&mut tx, &w_id).await?;
for name in datatable_config
.as_ref()
.and_then(|c| c.get("datatables"))
@@ -1316,6 +1315,96 @@ pub struct DropForkedDatatableDatabasesRequest {
datatable_names: Vec<String>,
}
/// Take a data table's generated logins away before its database goes.
///
/// A login is cluster-wide, so it outlives the database it was created in and
/// stays adoptable by whatever takes this workspace's id and this data table's
/// name next. The config stops naming them first: a data table whose database is
/// being dropped has no business claiming roles in it, and that is also what the
/// drop reads to know they are nobody's.
///
/// Returns the block it removed, which the caller must put back if the drop it
/// was clearing the way for does not happen: a data table that keeps its
/// database and loses its permissions leaves every member of the workspace
/// resolving to the data table's own connection, which owns everything in it.
async fn drop_datatable_roles_before_its_database(
db: &DB,
w_id: &str,
dt_name: &str,
errors: &mut Vec<String>,
) -> Option<serde_json::Value> {
let planned =
crate::datatable_permissions::plan_drop_of_deleted_datatable(db, w_id, dt_name).await?;
// Read and clear under the settings lock, so what comes back is what was
// taken away and is what putting it back would restore.
let cleared = async {
let mut tx = db.begin().await?;
let removed = windmill_common::workspaces::lock_workspace_settings_unchecked(&mut tx, w_id)
.await?
.and_then(|c| {
c.get("datatables")
.and_then(|d| d.get(dt_name))
.and_then(|d| d.get("permissions"))
.cloned()
});
sqlx::query!(
"UPDATE workspace_settings
SET datatable = datatable #- ARRAY['datatables', $2, 'permissions']
WHERE workspace_id = $1",
w_id,
dt_name,
)
.execute(&mut *tx)
.await?;
tx.commit().await?;
Ok::<_, windmill_common::error::Error>(removed)
}
.await;
match cleared {
Ok(removed) => {
crate::datatable_permissions::run_planned_drop(db, w_id, dt_name, planned).await;
removed
}
Err(e) => {
errors.push(format!(
"Could not clear the permissions of datatable://{}: {}",
dt_name, e
));
None
}
}
}
/// Put back the block a drop that did not happen took away. The logins it names
/// are already gone, so every role now resolves to nothing and is refused — the
/// safe answer, and the one a re-save of the data table's permissions repairs by
/// recreating them.
async fn restore_datatable_permissions(
db: &DB,
w_id: &str,
dt_name: &str,
permissions: serde_json::Value,
errors: &mut Vec<String>,
) {
if let Err(e) = sqlx::query!(
"UPDATE workspace_settings
SET datatable = jsonb_set(datatable, ARRAY['datatables', $2, 'permissions'], $3)
WHERE workspace_id = $1
AND datatable->'datatables' ? $2",
w_id,
dt_name,
permissions,
)
.execute(db)
.await
{
errors.push(format!(
"Could not restore the permissions of datatable://{}, which is now open to every member of the workspace: {}",
dt_name, e
));
}
}
/// Drop forked datatable databases. Returns errors per datatable that failed.
/// Same permission as delete_workspace: fork owner or super admin.
pub async fn drop_forked_datatable_databases(
@@ -1367,38 +1456,6 @@ pub async fn drop_forked_datatable_databases(
_ => continue,
};
// Before the database goes: the logins are in it, and the plan is built by
// connecting to it. Dropping the database first leaves them behind — a
// login is cluster-wide, so it survives its database and stays adoptable by
// whatever takes this workspace's id and this data table's name next.
//
// The config stops naming them first, both because a data table whose
// database is being dropped has no business claiming roles in it, and
// because that is what the drop reads to know they are nobody's.
if let Some(planned) =
crate::datatable_permissions::plan_drop_of_deleted_datatable(&db, &w_id, dt_name).await
{
let cleared = sqlx::query!(
"UPDATE workspace_settings
SET datatable = datatable #- ARRAY['datatables', $2, 'permissions']
WHERE workspace_id = $1",
&w_id,
dt_name,
)
.execute(&db)
.await;
match cleared {
Ok(_) => {
crate::datatable_permissions::run_planned_drop(&db, &w_id, dt_name, planned)
.await
}
Err(e) => errors.push(format!(
"Could not clear the permissions of datatable://{}: {}",
dt_name, e
)),
}
}
if dt.database.resource_type
== windmill_common::workspaces::DataTableCatalogResourceType::Instance
{
@@ -1410,11 +1467,17 @@ pub async fn drop_forked_datatable_databases(
));
continue;
}
let removed =
drop_datatable_roles_before_its_database(&db, &w_id, dt_name, &mut errors).await;
if let Err(e) = windmill_common::drop_custom_instance_database(&db, db_to_drop).await {
errors.push(format!(
"Could not drop instance database '{}' for datatable://{}: {}",
db_to_drop, dt_name, e
));
if let Some(permissions) = removed {
restore_datatable_permissions(&db, &w_id, dt_name, permissions, &mut errors)
.await;
}
}
} else {
let fork_pg = match crate::workspaces::resolve_pg_source_checked(
@@ -1475,6 +1538,9 @@ pub async fn drop_forked_datatable_databases(
match parent_pg.connect(Some(&db)).await {
Ok((client, connection)) => {
let join_handle = tokio::spawn(async move { connection.await });
let removed =
drop_datatable_roles_before_its_database(&db, &w_id, dt_name, &mut errors)
.await;
if let Err(e) = client
.execute(&format!("DROP DATABASE \"{}\"", db_to_drop), &[])
.await
@@ -1483,6 +1549,16 @@ pub async fn drop_forked_datatable_databases(
"Could not drop database '{}' for datatable://{}: {}",
db_to_drop, dt_name, e
));
if let Some(permissions) = removed {
restore_datatable_permissions(
&db,
&w_id,
dt_name,
permissions,
&mut errors,
)
.await;
}
}
drop(client);
let _ = windmill_common::shutdown_pg_connection(join_handle).await;
frontend/src/lib/components/raw_apps/RawAppDataTableDrawer.svelte
+72 -19
View File
@@ -6,6 +6,7 @@
import Button from '../common/button/Button.svelte'
import { sendUserToast } from '$lib/toast'
import type { DataTableRef } from './dataTableRefUtils'
import { untrack } from 'svelte'
import { resource } from 'runed'
import { ArrowLeft, Expand, Minimize, Plus, RefreshCcw } from 'lucide-svelte'
import DBManagerContent from '../DBManagerContent.svelte'
@@ -60,6 +61,56 @@
}
})
// Roles the *caller* may use, so the picker never offers one that would be
// refused. Absent/disabled permissions yield no roles and hide the picker.
const usableRoles = resource(
() => [open, opWs, selectedDatatable] as const,
async ([isOpen, workspace, datatable]) => {
if (!isOpen || !workspace || !datatable) return undefined
try {
return {
datatable,
...(await WorkspaceService.listUsableDatatableRoles({
workspace,
datatableName: datatable
}))
}
} catch (e) {
// Never leave the drawer waiting on this: fall back to the
// unpermissioned shape so it opens and the server picks the role.
console.error('Failed to load datatable roles:', e)
return { datatable, enabled: false, roles: [], default_role: 'admin' }
}
}
)
// Roles are per data table, and a resource keeps its previous value while it
// refetches.
const rolesOfCurrent = $derived(
usableRoles.current?.datatable === selectedDatatable ? usableRoles.current : undefined
)
// The content must not mount until the role is settled: mounting is what fires
// the schema and metadata queries, and a first round sent without a role would
// run — and cache — as whatever the server defaults to.
const roleSettled = $derived(
selectedDatatable === undefined ||
(rolesOfCurrent !== undefined &&
(!rolesOfCurrent.enabled ||
rolesOfCurrent.roles.length === 0 ||
selectedRole !== undefined))
)
// Settle the role before anything queries the data table: leaving it unset
// until the user touches the picker would send the first — and cached — round
// of queries as a role they may not be allowed to use.
$effect(() => {
const roles = rolesOfCurrent
if (!roles?.enabled || selectedRole !== undefined) return
const effective = roles.roles.includes(roles.default_role) ? roles.default_role : roles.roles[0]
if (effective) untrack(() => (selectedRole = effective))
})
// Every data table with its schemas and tables: the tree is the picker, so it
// has to cover the data tables the query editor is not pointed at. Asked for
// when the drawer opens — it reaches every data table's database in turn, and
@@ -186,25 +237,27 @@
noPadding
>
{#if dbInput && opWs}
{#key `${selectedDatatable}~${selectedRole ?? ''}`}
<DBManagerContent
bind:this={dbManagerContent}
input={dbInput}
workspace={opWs}
bind:workerTag={() => workerTag.tag, (v) => (workerTag.tag = v)}
bind:hasReplResult
bind:selectedSchemaKey
bind:selectedTableKey
multiSelectMode={true}
bind:selectedTables
{disabledTables}
datatableTree={datatableTree.current}
datatableTreeLoading={datatableTree.loading}
onSelectDatatable={(dt) => ((selectedDatatable = dt), (selectedRole = undefined))}
onSelectRole={(dt, role) => ((selectedDatatable = dt), (selectedRole = role))}
bind:pendingAction
/>
{/key}
{#if roleSettled}
{#key `${selectedDatatable}~${selectedRole ?? ''}`}
<DBManagerContent
bind:this={dbManagerContent}
input={dbInput}
workspace={opWs}
bind:workerTag={() => workerTag.tag, (v) => (workerTag.tag = v)}
bind:hasReplResult
bind:selectedSchemaKey
bind:selectedTableKey
multiSelectMode={true}
bind:selectedTables
{disabledTables}
datatableTree={datatableTree.current}
datatableTreeLoading={datatableTree.loading}
onSelectDatatable={(dt) => ((selectedDatatable = dt), (selectedRole = undefined))}
onSelectRole={(dt, role) => ((selectedDatatable = dt), (selectedRole = role))}
bind:pendingAction
/>
{/key}
{/if}
{:else}
<div class="flex items-center justify-center h-full text-tertiary">
<span>Select a data table to explore</span>