fix(datatables): an archived fork still reaches the database, and raw-app AI tools run as the app's role

This commit is contained in:
Diego Imbert
2026-09-06 00:56:55 +02:00
parent 1aac111b80
commit 73628dea4b
13 changed files with 266 additions and 145 deletions
@@ -1,22 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "\n WITH RECURSIVE tree AS (\n SELECT id, deleted, 0 AS depth FROM workspace WHERE id = $1\n UNION ALL\n SELECT w.id, w.deleted, tree.depth + 1 FROM workspace w\n JOIN tree ON w.parent_workspace_id = tree.id\n WHERE tree.depth < 20\n )\n SELECT id AS \"id!\" FROM tree WHERE id != $1 AND NOT deleted ORDER BY id\n ",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "id!",
"type_info": "Varchar"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
null
]
},
"hash": "3c76304619fba1222eec7fcfa7057e488a4ba601be7062d3c79097513cb93723"
}
@@ -1,29 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"name!\"\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id AND NOT w.deleted,\n jsonb_each(ws.datatable->'datatables') dt\n WHERE ws.workspace_id <> $1\n AND dt.value->'database'->>'resource_type' = 'instance'\n AND dt.value->'database'->>'resource_path' = $2",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "workspace_id!",
"type_info": "Varchar"
},
{
"ordinal": 1,
"name": "name!",
"type_info": "Text"
}
],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": [
false,
null
]
},
"hash": "a1b9097dc53acafe21256f093633b89f944ddd5ac9d0bafe4bcbbbd934518535"
}
@@ -0,0 +1,14 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE workspace_settings\n SET datatable = jsonb_set(datatable, '{datatables}', '{}'::jsonb)\n WHERE workspace_id = $1 AND jsonb_typeof(datatable->'datatables') = 'object'",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text"
]
},
"nullable": []
},
"hash": "a673eb72797bbdab342b86fe9156dd9c68dc10deedd93cb04a8c6636d736f30b"
}
@@ -0,0 +1,35 @@
{
"db_name": "PostgreSQL",
"query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"name!\", w.deleted AS \"deleted!\"\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id,\n jsonb_each(ws.datatable->'datatables') dt\n WHERE ws.workspace_id = ANY($1)\n AND dt.value->'database' = $2\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": "deleted!",
"type_info": "Bool"
}
],
"parameters": {
"Left": [
"TextArray",
"Jsonb"
]
},
"nullable": [
false,
null,
false
]
},
"hash": "aab1b338605f4fb9dd0d19e78d32849ccc783d295de38508d20ec30f99f518ca"
}
@@ -0,0 +1,35 @@
{
"db_name": "PostgreSQL",
"query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"name!\", w.deleted AS \"deleted!\"\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id,\n jsonb_each(ws.datatable->'datatables') dt\n WHERE ws.workspace_id <> $1\n AND dt.value->'database'->>'resource_type' = 'instance'\n AND dt.value->'database'->>'resource_path' = $2\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": "deleted!",
"type_info": "Bool"
}
],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": [
false,
null,
false
]
},
"hash": "eaed04b0b2f2736a6ec7f3bf446922910b7ff505584ef11c84a2f77f41e7deed"
}
@@ -1,14 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE workspace_settings\n SET datatable = jsonb_set(datatable, '{datatables}', COALESCE((\n SELECT jsonb_object_agg(key, value - 'permissions')\n FROM jsonb_each(datatable->'datatables')\n WHERE COALESCE((value->'permissions'->>'enabled')::boolean, false) = false\n ), '{}'::jsonb))\n WHERE workspace_id = $1 AND jsonb_typeof(datatable->'datatables') = 'object'",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text"
]
},
"nullable": []
},
"hash": "f117e4fd909fe5f2fe821e176011e838bd797c023123735ed52eb0507455165a"
}
@@ -136,6 +136,16 @@ async fn enabling_permissions_is_refused_while_a_fork_exists(
)
.execute(&db)
.await?;
sqlx::query(
"INSERT INTO workspace_settings (workspace_id, datatable) VALUES ('wm-fork-t', $1)",
)
.bind(json!({
"datatables": {
"main": { "database": { "resource_type": "instance", "resource_path": "dt_main" } }
}
}))
.execute(&db)
.await?;
let body = json!({ "enabled": true, "roles": [] });
for endpoint in [
@@ -149,7 +159,10 @@ async fn enabling_permissions_is_refused_while_a_fork_exists(
let status = resp.status();
let text = resp.text().await?;
assert_eq!(status, 400, "{endpoint}: {text}");
assert!(text.contains("wm-fork-t"), "{endpoint}: {text}");
assert!(
text.contains("wm-fork-t (data table 'main')"),
"{endpoint}: {text}"
);
}
// Only the opt-in is gated: once permissions are on, forks made afterwards
@@ -186,7 +199,8 @@ async fn enabling_permissions_is_refused_while_a_fork_exists(
.execute(&db)
.await?;
// An archived fork has no members left to reach anything.
// Archiving keeps the fork's members and its copy, so it still counts; a fork
// whose copy is a clone of its own does not.
sqlx::query("UPDATE workspace SET deleted = true WHERE id = 'wm-fork-t'")
.execute(&db)
.await?;
@@ -199,11 +213,38 @@ async fn enabling_permissions_is_refused_while_a_fork_exists(
.json(&body)
.send()
.await?;
assert_eq!(resp.status(), 400);
let text = resp.text().await?;
assert!(!text.contains("has forks"), "{text}");
assert!(
text.contains("wm-fork-t, archived (data table 'main')"),
"{text}"
);
sqlx::query(
r#"UPDATE workspace_settings
SET datatable = jsonb_set(
jsonb_set(datatable, '{datatables,main,forked_from}', '{"schema": {}}'),
'{datatables,main,database,resource_path}', '"wm_fork_dt_main"')
WHERE workspace_id = 'wm-fork-t'"#,
)
.execute(&db)
.await?;
let resp = authed(
client().post(format!(
"{ws}/workspaces/datatable_permissions/main/preview"
)),
"SECRET_TOKEN",
)
.json(&body)
.send()
.await?;
let text = resp.text().await?;
assert!(!text.contains("cannot be enabled"), "{text}");
// A workspace that is no longer a fork — a detached dev workspace — keeps its
// copy of the data table, pointing at the same instance database.
sqlx::query("DELETE FROM workspace_settings WHERE workspace_id = 'wm-fork-t'")
.execute(&db)
.await?;
sqlx::query("DELETE FROM workspace WHERE id = 'wm-fork-t'")
.execute(&db)
.await?;
@@ -233,8 +274,25 @@ async fn enabling_permissions_is_refused_while_a_fork_exists(
let text = resp.text().await?;
assert!(text.contains("detached (data table 'copy')"), "{text}");
// With both gone the refusal lifts: the preview then gets as far as the
// database, which this test does not have.
// Archived, it still counts; with the copy gone the refusal lifts, and the
// preview then gets as far as the database, which this test does not have.
sqlx::query("UPDATE workspace SET deleted = true WHERE id = 'detached'")
.execute(&db)
.await?;
let resp = authed(
client().post(format!(
"{ws}/workspaces/datatable_permissions/main/preview"
)),
"SECRET_TOKEN",
)
.json(&body)
.send()
.await?;
let text = resp.text().await?;
assert!(
text.contains("detached, archived (data table 'copy')"),
"{text}"
);
sqlx::query("DELETE FROM workspace_settings WHERE workspace_id = 'detached'")
.execute(&db)
.await?;
@@ -254,13 +312,12 @@ async fn enabling_permissions_is_refused_while_a_fork_exists(
}
/// The rename keeps a copy of the settings under the archived id, and commits it
/// before the old id is archived. A permissioned data table must not be in that
/// copy: without its `permissions` block it would resolve, for anyone still
/// using the old id, to the owner connection.
/// before the old id is archived. No data table may be in that copy: a
/// permissioned one without its `permissions` block would resolve, for anyone
/// still using the old id, to the owner connection, and any one naming the same
/// instance database would keep the renamed workspace from opting in.
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
async fn a_rename_leaves_no_permissioned_datatable_under_the_old_id(
db: Pool<Postgres>,
) -> anyhow::Result<()> {
async fn a_rename_leaves_no_datatable_under_the_old_id(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
@@ -310,7 +367,7 @@ async fn a_rename_leaves_no_permissioned_datatable_under_the_old_id(
}
};
let (old_names, _) = names("test-workspace").await;
assert_eq!(old_names, vec!["open".to_string()]);
assert!(old_names.is_empty(), "{old_names:?}");
let (new_names, new_value) = names("renamed-ws").await;
assert_eq!(new_names, vec!["main".to_string(), "open".to_string()]);
assert_eq!(new_value["main"]["permissions"]["enabled"], json!(true));
@@ -724,8 +724,8 @@ pub(crate) async fn ensure_can_use_datatable_role(
}
/// Permissions are turned on where this workspace is the only one reaching the
/// database: no fork above it, none below it, and no other workspace's data
/// table naming the same instance database.
/// database: it is not a fork, no fork below it holds a copy of the data table,
/// and no other workspace's data table names the same instance database.
///
/// A fork's data table is either a copy pointing at the database of the workspace
/// it was forked from, where roles created in the fork would hold grants that
@@ -740,6 +740,12 @@ pub(crate) async fn ensure_can_use_datatable_role(
/// tables naming it, and meaningless for a resource-backed one, where whoever
/// holds the resource's credentials reaches the database regardless.
///
/// Archived workspaces count. Archiving keeps the members, their session tokens
/// and the settings, and nothing on the job path checks the flag, so an archived
/// fork reaches the database exactly as a live one does. Only a permanent
/// deletion, or removing the copy, takes that away. The shell a rename archives
/// is left with no data tables at all, so it never counts.
///
/// All three are properties of the opt-in, so only the save that turns
/// permissions on is checked. Forks made afterwards never receive a permissioned
/// data table (see the strip in the fork creation), and a save that edits the
@@ -772,25 +778,55 @@ async fn refuse_enabling_permissions_over_shared_access(
.to_string(),
));
}
let forks = windmill_common::workspaces::list_live_fork_descendants(db, w_id).await?;
let database = serde_json::to_value(&datatable.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 forks = windmill_common::workspaces::list_fork_descendants(db, w_id).await?;
if !forks.is_empty() {
return Err(Error::BadRequest(format!(
"Data table permissions cannot be enabled while this workspace has forks ({}): a \
fork holds a copy of the data table pointing at the same database, and its members \
would keep reaching it through the data table's own connection, as every role at \
once. Delete the forks first.",
forks.join(", ")
)));
// A clone points at a database of its own and does not count.
let copies = sqlx::query!(
r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "name!", w.deleted AS "deleted!"
FROM workspace_settings ws
JOIN workspace w ON w.id = ws.workspace_id,
jsonb_each(ws.datatable->'datatables') dt
WHERE ws.workspace_id = ANY($1)
AND dt.value->'database' = $2
ORDER BY ws.workspace_id, dt.key"#,
&forks[..],
database,
)
.fetch_all(db)
.await?;
if !copies.is_empty() {
let copies: Vec<String> = copies
.into_iter()
.map(|c| describe(&c.workspace_id, &c.name, c.deleted))
.collect();
return Err(Error::BadRequest(format!(
"Data table permissions cannot be enabled while a fork of this workspace holds a \
copy of the data table pointing at the same database: {}. Its members would \
keep reaching it through the copy's own connection, as every role at once — an \
archived fork included, since archiving keeps its members. Remove the data \
table from the fork, or delete the fork permanently, first.",
copies.join(", ")
)));
}
}
if datatable.database.resource_type == DataTableCatalogResourceType::Instance {
let others = sqlx::query!(
r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "name!"
r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "name!", w.deleted AS "deleted!"
FROM workspace_settings ws
JOIN workspace w ON w.id = ws.workspace_id AND NOT w.deleted,
JOIN workspace w ON w.id = ws.workspace_id,
jsonb_each(ws.datatable->'datatables') dt
WHERE ws.workspace_id <> $1
AND dt.value->'database'->>'resource_type' = 'instance'
AND dt.value->'database'->>'resource_path' = $2"#,
AND dt.value->'database'->>'resource_path' = $2
ORDER BY ws.workspace_id, dt.key"#,
w_id,
&datatable.database.resource_path,
)
@@ -799,12 +835,14 @@ async fn refuse_enabling_permissions_over_shared_access(
if !others.is_empty() {
let others: Vec<String> = others
.into_iter()
.map(|o| format!("{} (data table '{}')", o.workspace_id, o.name))
.map(|o| describe(&o.workspace_id, &o.name, o.deleted))
.collect();
return Err(Error::BadRequest(format!(
"Data table permissions cannot be enabled while another workspace reaches the \
same database: {}. Its members would keep reaching it through that data \
table's own connection, as every role at once. Remove that data table first.",
table's own connection, as every role at once — an archived workspace \
included, since archiving keeps its members. Remove that data table, or delete \
the workspace permanently, first.",
others.join(", ")
)));
}
@@ -2233,6 +2233,8 @@ struct GetDataTableSchemaQuery {
datatable_name: String,
schema_name: String,
table_name: String,
/// Read the columns as this role rather than the data table's default one.
role: Option<String>,
}
#[derive(Serialize, Debug)]
@@ -2501,6 +2503,7 @@ async fn get_datatable_table_schema(
&query.datatable_name,
&query.schema_name,
&query.table_name,
query.role.as_deref(),
)
.await?;
@@ -2797,6 +2800,7 @@ async fn get_datatable_table_columns(
datatable_name: &str,
schema_name: &str,
table_name: &str,
role: Option<&str>,
) -> Result<ColumnMap> {
if is_system_pg_schema(schema_name) {
return Err(Error::BadRequest(format!(
@@ -2805,8 +2809,16 @@ async fn get_datatable_table_columns(
)));
}
let db_resource =
get_datatable_resource_as_default_role(db, authed, w_id, datatable_name).await?;
// Columns are what the connected role may see, so a caller on a role reads
// them as that role.
let db_resource = get_datatable_resource_from_db(
db,
w_id,
datatable_name,
role,
DatatableAccess::Authed(authed.to_authed_ref()),
)
.await?;
let pg_db: PgDatabase = serde_json::from_value(db_resource)
.map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e)))?;
let (client, connection) = pg_db.connect(Some(db)).await?;
@@ -8244,11 +8256,11 @@ async fn create_workspace_fork(
.await?;
}
// Enabling a data table's permissions is refused while the workspace has forks, and it
// checks for them under this same row: a fork still being created has either committed,
// Enabling a data table's permissions is refused while a fork holds a copy of it, and
// that check runs under this same row: a fork still being created has either committed,
// and is found, or copies the parent's settings only after the opt-in landed, so the
// permissioned data table is stripped from it below. After the pairing lock, which is
// the order the workspace rename takes the two in.
// permissioned data table is stripped from it below. Pairing lock first, as
// `lock_workspace_settings_unchecked` states.
windmill_common::workspaces::lock_workspace_settings_unchecked(&mut tx, &parent_workspace_id)
.await?;
@@ -65,7 +65,7 @@ pub(crate) async fn change_workspace_id(
// The settings are copied below, and a permissions save holds this row while it changes
// the roles in the database and then the config: copied without it, the new workspace
// could carry the config from before that save while the database has the roles from
// after it. Same order as fork creation: pairing lock, then the settings row.
// after it. Pairing lock first, as `lock_workspace_settings_unchecked` states.
windmill_common::workspaces::lock_workspace_settings_unchecked(&mut tx, &old_id).await?;
check_w_id_conflict(&mut tx, &rw.new_id).await?;
@@ -126,13 +126,14 @@ pub(crate) async fn change_workspace_id(
.execute(&mut *tx)
.await?;
// Two configs now name the same Postgres logins, and only one of them owns them:
// deleting the archived id would plan drops for logins the renamed workspace is still
// using. A permissioned data table leaves the archived copy entirely rather than losing
// its `permissions` block: this transaction commits before the old id is archived, and
// a copy that still named the database without the block would hand every caller of the
// old id the owner connection in between — and again if the shell were ever unarchived.
// A missing data table fails closed. The unpermissioned ones stay, for reference.
// The archived copy keeps no data tables. Two configs would otherwise name the same
// databases and, for a permissioned one, the same Postgres logins, which only the renamed
// workspace owns: deleting the archived id would plan drops for logins still in use, and
// — since this transaction commits before the old id is archived — a copy without its
// `permissions` block would hand every caller of the old id the owner connection in
// between. An archived shell that still named an instance database would also keep the
// renamed workspace from ever opting in, as another workspace reaching the same database.
// A missing data table fails closed.
//
// The renamed workspace keeps the roles and keeps working: a role's `pg_rolename` is
// what resolution uses, and the generated name only decides what a *new* role is called.
@@ -141,11 +142,7 @@ pub(crate) async fn change_workspace_id(
// rename already carries.
sqlx::query!(
"UPDATE workspace_settings
SET datatable = jsonb_set(datatable, '{datatables}', COALESCE((
SELECT jsonb_object_agg(key, value - 'permissions')
FROM jsonb_each(datatable->'datatables')
WHERE COALESCE((value->'permissions'->>'enabled')::boolean, false) = false
), '{}'::jsonb))
SET datatable = jsonb_set(datatable, '{datatables}', '{}'::jsonb)
WHERE workspace_id = $1 AND jsonb_typeof(datatable->'datatables') = 'object'",
&old_id,
)
+6
View File
@@ -4983,6 +4983,12 @@ paths:
required: true
schema:
type: string
- name: role
in: query
required: false
description: read the columns as this role rather than the data table's default one
schema:
type: string
responses:
"200":
description: schema of one datatable table
+9 -31
View File
@@ -737,31 +737,6 @@ pub async fn list_fork_descendants(db: &crate::DB, w_id: &str) -> Result<Vec<Str
Ok(ids)
}
/// Same, without the soft-deleted ones: the descendants that still have members. An archived
/// fork, or the shell a fork's rename archives, keeps its `parent_workspace_id` and would
/// otherwise count as a workspace that can still reach what the parent owns.
///
/// Unauthenticated helper, like [`list_fork_descendants`].
pub async fn list_live_fork_descendants(db: &crate::DB, w_id: &str) -> Result<Vec<String>> {
let ids = sqlx::query_scalar!(
r#"
WITH RECURSIVE tree AS (
SELECT id, deleted, 0 AS depth FROM workspace WHERE id = $1
UNION ALL
SELECT w.id, w.deleted, tree.depth + 1 FROM workspace w
JOIN tree ON w.parent_workspace_id = tree.id
WHERE tree.depth < 20
)
SELECT id AS "id!" FROM tree WHERE id != $1 AND NOT deleted ORDER BY id
"#,
w_id
)
.fetch_all(db)
.await
.map_err(|e| Error::internal_err(format!("listing live fork descendants of {w_id}: {e:#}")))?;
Ok(ids)
}
/// Count non-deleted fork/dev workspaces anywhere under `root` (excludes `root` itself).
///
/// Unauthenticated metering helper: it reads workspace hierarchy for any `root` id, so callers must
@@ -1372,12 +1347,15 @@ pub fn remove_datatable_tenant(datatable: &mut serde_json::Value, tenant: &str)
/// planned with `g/devs` would otherwise put the tenant back after the group's
/// deletion took it away.
///
/// **Take it before the transaction locks anything else.** One lock, always
/// acquired first, cannot deadlock; a caller that writes `usr` or `group_` and
/// then reaches for this one holds two in an order some other path holds the
/// other way round. A transaction spanning workspaces takes them in
/// `workspace_id` order, for the same reason. That is the whole ordering rule:
/// what a handler writes after taking it, and in what order, does not matter.
/// **Take it before the transaction locks anything else**, with one exception:
/// the dev-pairing advisory lock (`lock_dev_pairing`) comes first where a path
/// needs both — fork creation and the workspace rename — and nothing takes this
/// row and then reaches for that one. One order, held everywhere, cannot
/// deadlock; a caller that writes `usr` or `group_` and then reaches for this
/// one holds two in an order some other path holds the other way round. A
/// transaction spanning workspaces takes them in `workspace_id` order, for the
/// same reason. That is the whole ordering rule: what a handler writes after
/// taking it, and in what order, does not matter.
///
/// Authorization: performs none, for any workspace it is handed. What it returns
/// is the config as stored, generated role passwords included, so callers MUST
@@ -283,6 +283,12 @@
}
}
// The role the app is saved with applies to its own data table; another data
// table named by a tool is read as its default role, like everywhere else.
function appRoleFor(datatableName: string): string | undefined {
return datatableName === data.datatable ? data.role : undefined
}
function isDatatableTableWhitelisted(
datatableName: string,
schemaName: string,
@@ -1086,8 +1092,12 @@
return []
}
// The app's data table is read as the role the app runs as, so the AI
// sees the tables that role reaches, not the default role's.
const tables = await WorkspaceService.listDataTableTables({
workspace: opWorkspace
workspace: opWorkspace,
roleFor: data.role ? data.datatable : undefined,
role: data.role
})
return filterDatatableTables(tables)
},
@@ -1113,7 +1123,8 @@
workspace: opWorkspace,
datatableName,
schemaName,
tableName
tableName,
role: appRoleFor(datatableName)
})
return schema.columns
},
@@ -1131,13 +1142,16 @@
}
try {
// Same reference the generated runnables use: the role rides in it, so
// a table the AI creates belongs to the role the app will connect as.
const role = appRoleFor(datatableName)
const result = await runScriptAndPollResult(
{
workspace: opWorkspace,
requestBody: {
language: 'postgresql',
content: sql,
args: { database: `datatable://${datatableName}` }
args: { database: `datatable://${datatableName}${role ? `?role=${role}` : ''}` }
}
},
writingJobOptions