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

fix(datatables): the same-database scan reads resource rows, the settings form refuses a second door, and a fork keeps no orphan migrations

Browse Source
This commit is contained in:
Diego Imbert
2026-09-06 13:37:19 +02:00
parent c08cbc98f7
commit e6c91aebeb
7 changed files with 364 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/.sqlx/query-7918594d0fbbc2dac2397cba0bb7bcd5a35bc618ef43d54130be373c7ec8b7da.json
Generated
+41
View File
@@ -0,0 +1,41 @@
{
"db_name": "PostgreSQL",
"query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"name!\",\n dt.value->'database' AS \"database!\",\n dt.value->'permissions'->>'database_identity' AS identity\n FROM workspace_settings ws, jsonb_each(ws.datatable->'datatables') dt\n WHERE NOT (ws.workspace_id = $1 AND dt.key = $2)\n AND COALESCE((dt.value->'permissions'->>'enabled')::boolean, false)\n ORDER BY ws.workspace_id, dt.key",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "workspace_id!",
"type_info": "Varchar"
},
{
"ordinal": 1,
"name": "name!",
"type_info": "Text"
},
{
"ordinal": 2,
"name": "database!",
"type_info": "Jsonb"
},
{
"ordinal": 3,
"name": "identity",
"type_info": "Text"
}
],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": [
false,
null,
null,
null
]
},
"hash": "7918594d0fbbc2dac2397cba0bb7bcd5a35bc618ef43d54130be373c7ec8b7da"
}
backend/.sqlx/query-bf7d6e52d23cd0aed238ec1a883baeb468330b51edfb690921c67828c4c0bdf3.json
Generated
+14
View File
@@ -0,0 +1,14 @@
{
"db_name": "PostgreSQL",
"query": "DELETE FROM datatable_migrations m\n WHERE m.workspace_id = $1\n AND NOT EXISTS (\n SELECT 1 FROM workspace_settings ws\n WHERE ws.workspace_id = $1 AND ws.datatable->'datatables' ? m.datatable\n )",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text"
]
},
"nullable": []
},
"hash": "bf7d6e52d23cd0aed238ec1a883baeb468330b51edfb690921c67828c4c0bdf3"
}
backend/.sqlx/query-41030ce05776505da194798e418e634cade1f712b8a6dd4ab50eb8e2207a4c62.json → backend/.sqlx/query-e53586dfce4275479a353b3bd0ff7100f39a7a7704a8ad31dba82042f6fd0b63.json
Generated
+9 -3
View File
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"name!\", w.deleted AS \"deleted!\",\n dt.value->'database' AS \"database!\"\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id,\n jsonb_each(ws.datatable->'datatables') dt\n WHERE NOT (ws.workspace_id = $1 AND dt.key = $2)\n AND jsonb_typeof(dt.value->'database') = 'object'\n ORDER BY ws.workspace_id, dt.key",
"query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"name!\", w.deleted AS \"deleted!\",\n dt.value->'database' AS \"database!\", r.value AS \"resource?\"\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id\n CROSS JOIN LATERAL jsonb_each(ws.datatable->'datatables') dt\n LEFT JOIN resource r ON r.workspace_id = ws.workspace_id\n AND dt.value->'database'->>'resource_type' <> 'instance'\n AND r.path = dt.value->'database'->>'resource_path'\n WHERE NOT (ws.workspace_id = $1 AND dt.key = $2)\n AND jsonb_typeof(dt.value->'database') = 'object'\n ORDER BY ws.workspace_id, dt.key",
"describe": {
"columns": [
{
@@ -22,6 +22,11 @@
"ordinal": 3,
"name": "database!",
"type_info": "Jsonb"
},
{
"ordinal": 4,
"name": "resource?",
"type_info": "Jsonb"
}
],
"parameters": {
@@ -34,8 +39,9 @@
false,
null,
false,
null
null,
true
]
},
"hash": "41030ce05776505da194798e418e634cade1f712b8a6dd4ab50eb8e2207a4c62"
"hash": "e53586dfce4275479a353b3bd0ff7100f39a7a7704a8ad31dba82042f6fd0b63"
}
backend/ee-repo-ref.txt
+1 -1
View File
@@ -1 +1 @@
f89f0cd16ec24f7beb146ba8373d688e1fe126e8
1e70ab9734682756988f184e850a3b2c5a0595fe
backend/windmill-api-integration-tests/tests/datatable_tenants.rs
+88
View File
@@ -428,11 +428,22 @@ async fn a_kept_original_does_not_inherit_the_clone_stamp(
"byo": {
"database": { "resource_type": "postgresql", "resource_path": "u/test-user/pg" },
"forked_from": { "schema": {} }
},
"governed": {
"database": { "resource_type": "instance", "resource_path": "dt_governed" },
"permissions": { "enabled": true, "roles": { "admin": { "tenants": [] } } }
}
}
}))
.execute(&db)
.await?;
sqlx::query(
"INSERT INTO datatable_migrations (workspace_id, datatable, timestamp, name, code_up)
VALUES ('test-workspace', 'governed', 1, 'init', 'SELECT 1'),
('test-workspace', 'byo', 1, 'init', 'SELECT 1')",
)
.execute(&db)
.await?;
let resp = authed(
client().post(format!("{ws}/workspaces/create_fork")),
@@ -450,6 +461,13 @@ async fn a_kept_original_does_not_inherit_the_clone_stamp(
.await?;
assert_eq!(copy["database"]["resource_path"], json!("u/test-user/pg"));
assert!(copy.get("forked_from").is_none(), "{copy}");
// The permissioned data table stayed out of the fork, its migrations with it.
let migrated: Vec<String> = sqlx::query_scalar(
"SELECT datatable FROM datatable_migrations WHERE workspace_id = 'wm-fork-kept' ORDER BY 1",
)
.fetch_all(&db)
.await?;
assert_eq!(migrated, vec!["byo".to_string()]);
let resp = authed(
client().post(format!("{ws}/workspaces/datatable_permissions/byo/preview")),
@@ -735,6 +753,76 @@ async fn a_same_named_resource_counts_only_when_it_reaches_the_same_database(
"{text}"
);
// The same rule from the other side: once `byo` is governed, the settings
// form cannot add an entry that reaches its database, in this workspace or
// another, and the pointer alone does not decide it.
let identity: String = {
// Stamped by the opt-in in the real flow; here from the same resource.
let resp = authed(
client().get(format!(
"http://localhost:{port}/api/w/test-workspace/resources/get_value_interpolated/u/test-user/pg"
)),
"SECRET_TOKEN",
)
.send()
.await?;
assert_eq!(resp.status(), 200);
let value: serde_json::Value = resp.json().await?;
use sha2::{Digest, Sha256};
let mut hasher = Sha256::new();
for field in ["host", "port", "dbname", "user"] {
hasher.update(value.get(field).map(|v| v.to_string()).unwrap_or_default());
hasher.update([0u8]);
}
format!("{:x}", hasher.finalize())
};
sqlx::query(
r#"UPDATE workspace_settings
SET datatable = jsonb_set(datatable, '{datatables,byo,permissions}',
jsonb_build_object('enabled', true, 'roles', '{"admin": {"tenants": []}}'::jsonb,
'database_identity', $1::text))
WHERE workspace_id = 'test-workspace'"#,
)
.bind(&identity)
.execute(&db)
.await?;
let save = |w: &'static str, path: &'static str| async move {
let resp = authed(
client().post(format!(
"http://localhost:{port}/api/w/{w}/workspaces/edit_datatable_config"
)),
"SECRET_TOKEN",
)
.json(&json!({ "settings": { "datatables": {
"door": { "database": { "resource_type": "postgresql", "resource_path": path } }
}}}))
.send()
.await
.unwrap();
(resp.status().as_u16(), resp.text().await.unwrap())
};
let (status, text) = save("detached", "f/moved/pg").await;
assert_eq!(status, 400, "{text}");
assert!(
text.contains("data table 'byo' of workspace test-workspace"),
"{text}"
);
sqlx::query(
"UPDATE resource SET value = jsonb_set(value, '{dbname}', '\"other\"')
WHERE workspace_id = 'detached' AND path = 'f/moved/pg'",
)
.execute(&db)
.await?;
let (status, text) = save("detached", "f/moved/pg").await;
assert_eq!(status, 200, "{text}");
sqlx::query(
r#"UPDATE workspace_settings
SET datatable = datatable #- '{datatables,byo,permissions}'
WHERE workspace_id = 'test-workspace'"#,
)
.execute(&db)
.await?;
// A second entry of this workspace on the same resource is a second door.
sqlx::query("DELETE FROM workspace_settings WHERE workspace_id = 'detached'")
.execute(&db)
backend/windmill-api-workspaces/src/datatable_permissions.rs
+182 -60
View File
@@ -499,6 +499,7 @@ async fn build_plan(
.map(|p| p.roles.keys().map(String::as_str))
.into_iter()
.flatten()
.filter(|name| *name != ADMIN_DATATABLE_ROLE)
.collect();
let stranded = migrations_naming(db, w_id, datatable_name, &roles).await?;
if !stranded.is_empty() {
@@ -849,65 +850,7 @@ async fn refuse_enabling_permissions_over_shared_access(
)));
}
}
// Every other data table entry on the instance, this workspace's other
// entries included: a second entry on the same resource is a second door.
let entries = sqlx::query!(
r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "name!", w.deleted AS "deleted!",
dt.value->'database' AS "database!"
FROM workspace_settings ws
JOIN workspace w ON w.id = ws.workspace_id,
jsonb_each(ws.datatable->'datatables') dt
WHERE NOT (ws.workspace_id = $1 AND dt.key = $2)
AND jsonb_typeof(dt.value->'database') = 'object'
ORDER BY ws.workspace_id, dt.key"#,
w_id,
datatable_name,
)
.fetch_all(db)
.await?;
let mut others = Vec::new();
match datatable.database.resource_type {
DataTableCatalogResourceType::Instance => {
others.extend(
entries
.iter()
.filter(|o| o.database == database)
.map(|o| describe(&o.workspace_id, &o.name, o.deleted)),
);
}
DataTableCatalogResourceType::Postgresql => {
// A resource-backed entry reaches wherever its resource points,
// whatever path it names: a copy of the resource under another path,
// in another workspace or this one, is the same database.
let identity = datatable_database_identity(
&get_datatable_resource_from_db_unchecked(db, w_id, datatable_name).await?,
);
for o in entries.iter().filter(|o| {
o.database.get("resource_type").and_then(|t| t.as_str()) != Some("instance")
}) {
let resolved =
match get_datatable_resource_from_db_unchecked(db, &o.workspace_id, &o.name)
.await
{
Ok(resolved) => resolved,
// A resource that is gone reaches nothing. Anything else is
// not proof of not reaching: refused, and named.
Err(Error::NotFound(_)) => continue,
Err(e) => {
return Err(Error::BadRequest(format!(
"Data table permissions cannot be enabled: whether {} reaches the \
same database could not be checked ({e}). Remove that data \
table first.",
describe(&o.workspace_id, &o.name, o.deleted)
)))
}
};
if datatable_database_identity(&resolved) == identity {
others.push(describe(&o.workspace_id, &o.name, o.deleted));
}
}
}
}
let others = entries_reaching(db, w_id, datatable_name, &datatable.database).await?;
if !others.is_empty() {
return Err(Error::BadRequest(format!(
"Data table permissions cannot be enabled while another data table reaches the \
@@ -921,6 +864,183 @@ async fn refuse_enabling_permissions_over_shared_access(
Ok(())
}
/// `<workspace>[, archived] (data table '<name>')` for every data table entry on
/// the instance, other than `(w_id, datatable_name)`, that reaches `database` —
/// this workspace's other entries included: a second entry on the same
/// resource is a second door.
///
/// An instance database is matched by name. A resource-backed entry reaches
/// wherever its resource points, whatever path it names, so it is matched by the
/// resource's host, port, database and user. Read from the stored resource rows
/// in one query: a field that is a `$var:` / `$res:` reference is only resolved
/// — a per-row read, and a secret backend call where the workspace uses one —
/// when every plain field already agrees, so the loop that decrypts runs for
/// candidates that can match and not for every data table on the instance. A
/// resource that no longer exists reaches nothing.
async fn entries_reaching(
db: &DB,
w_id: &str,
datatable_name: &str,
database: &windmill_common::workspaces::DataTableDatabase,
) -> Result<Vec<String>> {
let pointer = serde_json::to_value(database)
.map_err(|e| Error::internal_err(format!("Failed to serialize the database: {e}")))?;
let describe = |workspace_id: &str, name: &str, deleted: bool| {
format!(
"{workspace_id}{} (data table '{name}')",
if deleted { ", archived" } else { "" }
)
};
let entries = sqlx::query!(
r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "name!", w.deleted AS "deleted!",
dt.value->'database' AS "database!", r.value AS "resource?"
FROM workspace_settings ws
JOIN workspace w ON w.id = ws.workspace_id
CROSS JOIN LATERAL jsonb_each(ws.datatable->'datatables') dt
LEFT JOIN resource r ON r.workspace_id = ws.workspace_id
AND dt.value->'database'->>'resource_type' <> 'instance'
AND r.path = dt.value->'database'->>'resource_path'
WHERE NOT (ws.workspace_id = $1 AND dt.key = $2)
AND jsonb_typeof(dt.value->'database') = 'object'
ORDER BY ws.workspace_id, dt.key"#,
w_id,
datatable_name,
)
.fetch_all(db)
.await?;
let mut reaching = Vec::new();
match database.resource_type {
DataTableCatalogResourceType::Instance => {
reaching.extend(
entries
.iter()
.filter(|o| o.database == pointer)
.map(|o| describe(&o.workspace_id, &o.name, o.deleted)),
);
}
DataTableCatalogResourceType::Postgresql => {
let ours = get_datatable_resource_from_db_unchecked(db, w_id, datatable_name).await?;
let identity = datatable_database_identity(&ours);
let is_reference = |v: &serde_json::Value| {
v.as_str()
.is_some_and(|s| s.starts_with("$var:") || s.starts_with("$res:"))
};
for o in entries.iter() {
let Some(raw) = o.resource.as_ref() else {
continue;
};
let Some(fields) = raw.as_object() else {
continue;
};
let mut references = false;
let mut plain_fields_agree = true;
for field in DATABASE_IDENTITY_FIELDS {
let theirs = fields.get(field).unwrap_or(&serde_json::Value::Null);
if is_reference(theirs) {
references = true;
} else if theirs != ours.get(field).unwrap_or(&serde_json::Value::Null) {
plain_fields_agree = false;
}
}
if !plain_fields_agree {
continue;
}
if !references {
reaching.push(describe(&o.workspace_id, &o.name, o.deleted));
continue;
}
let resolved =
match get_datatable_resource_from_db_unchecked(db, &o.workspace_id, &o.name)
.await
{
Ok(resolved) => resolved,
Err(Error::NotFound(_)) => continue,
// Not resolving is not proof of not reaching: refused, and
// named.
Err(e) => {
return Err(Error::BadRequest(format!(
"Whether {} reaches the same database could not be checked \
({e}). Remove that data table first.",
describe(&o.workspace_id, &o.name, o.deleted)
)))
}
};
if datatable_database_identity(&resolved) == identity {
reaching.push(describe(&o.workspace_id, &o.name, o.deleted));
}
}
}
}
Ok(reaching)
}
/// The fields [`datatable_database_identity`] hashes.
const DATABASE_IDENTITY_FIELDS: [&str; 4] = ["host", "port", "dbname", "user"];
/// Refuse a data table entry that reaches a database another data table governs.
///
/// The opt-in refuses while any other entry reaches the database; this is the
/// same rule from the other side, for the settings form that adds an entry or
/// points one elsewhere. Without it a second entry on a governed database is a
/// second door, open to every member as the owning connection. Governed entries
/// are matched by the identity their opt-in stamped, so nothing of theirs is
/// resolved; the entry being saved is resolved once, and must resolve.
pub(crate) async fn refuse_reaching_a_governed_database(
db: &DB,
w_id: &str,
datatable_name: &str,
database: &windmill_common::workspaces::DataTableDatabase,
) -> Result<()> {
let pointer = serde_json::to_value(database)
.map_err(|e| Error::internal_err(format!("Failed to serialize the database: {e}")))?;
let governed = sqlx::query!(
r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "name!",
dt.value->'database' AS "database!",
dt.value->'permissions'->>'database_identity' AS identity
FROM workspace_settings ws, jsonb_each(ws.datatable->'datatables') dt
WHERE NOT (ws.workspace_id = $1 AND dt.key = $2)
AND COALESCE((dt.value->'permissions'->>'enabled')::boolean, false)
ORDER BY ws.workspace_id, dt.key"#,
w_id,
datatable_name,
)
.fetch_all(db)
.await?;
if governed.is_empty() {
return Ok(());
}
let hit = match database.resource_type {
DataTableCatalogResourceType::Instance => governed.iter().find(|g| g.database == pointer),
DataTableCatalogResourceType::Postgresql => {
// The stored config is what the form is about to replace, so the entry
// is resolved from the database it names rather than from the config.
let resource = windmill_common::workspaces::transform_json_value_unchecked(
&serde_json::Value::String(format!("$res:{}", database.resource_path)),
w_id,
db,
)
.await?;
let identity = datatable_database_identity(&resource);
governed
.iter()
.find(|g| g.identity.as_deref() == Some(identity.as_str()))
}
};
if let Some(g) = hit {
return Err(Error::BadRequest(format!(
"Data table '{datatable_name}' would reach the database of {}, whose role \
permissions are enabled: every member would reach it through this data table's \
own connection, as every role at once.",
if g.workspace_id == w_id {
format!("data table '{}'", g.name)
} else {
format!("data table '{}' of workspace {}", g.name, g.workspace_id)
}
)));
}
Ok(())
}
/// Refuse a save that names something that no longer exists: a tenant whose
/// user, group or folder is gone, or a role that stored migrations still name
/// and the save no longer defines.
@@ -1016,11 +1136,13 @@ async fn ensure_save_names_what_exists(
// Renamed away or removed: every old name the save no longer defines.
let kept: HashSet<&str> = req.roles.iter().map(|r| r.name.as_str()).collect();
// `admin` resolves to the data table's own connection whether or not it is
// named, so a migration naming it never strands.
let gone: HashSet<&str> = old
.map(|old| old.roles.keys().map(String::as_str))
.into_iter()
.flatten()
.filter(|name| !kept.contains(name))
.filter(|name| !kept.contains(name) && *name != ADMIN_DATATABLE_ROLE)
.collect();
let blocking = migrations_naming(db, w_id, datatable_name, &gone).await?;
if !blocking.is_empty() {
backend/windmill-api-workspaces/src/workspaces.rs
+29
View File
@@ -3876,6 +3876,21 @@ async fn edit_datatable_config(
schema: old.schema.clone(),
}),
};
// An entry that is new, or points somewhere new, must not reach a database
// another data table governs.
let points_elsewhere = old.is_none_or(|old| {
old.database.resource_path != dt.database.resource_path
|| old.database.resource_type != dt.database.resource_type
});
if points_elsewhere {
crate::datatable_permissions::refuse_reaching_a_governed_database(
&db,
&w_id,
name,
&dt.database,
)
.await?;
}
// The roles live in the database this data table points at: their logins
// were created there and every grant they hold is recorded there. Carried
// onto another database they authenticate against a cluster that never
@@ -8411,6 +8426,20 @@ async fn create_workspace_fork(
.execute(&mut *tx)
.await?;
// The migrations were cloned for every data table; the ones left out above
// would otherwise keep a history for a data table the fork does not have.
sqlx::query!(
r#"DELETE FROM datatable_migrations m
WHERE m.workspace_id = $1
AND NOT EXISTS (
SELECT 1 FROM workspace_settings ws
WHERE ws.workspace_id = $1 AND ws.datatable->'datatables' ? m.datatable
)"#,
&forked_id,
)
.execute(&mut *tx)
.await?;
// The settings clone copies the source's ducklake config verbatim — including a parent
// fork's own `fork_behavior` stamps. Sharing is a per-fork-creation choice, never
// inherited: reset any cloned stamps first, then apply this fork's requested list.