mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix: stop a resource delete from taking variables it does not own (#11102)
* fix: stop a resource delete from taking variables it does not own Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nb6mKJWUACuPKA3wZRuyy7 * fix: key the ws_specific cleanup on what the delete actually removed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nb6mKJWUACuPKA3wZRuyy7 * fix: attribute a cascaded variable to the resource that actually referenced it Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nb6mKJWUACuPKA3wZRuyy7 * docs: state the real constraint behind the pre-transaction referrer scan Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Nb6mKJWUACuPKA3wZRuyy7 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
co-authored by
Claude Opus 5
Claude Opus 5
parent
c90d1d95c2
commit
2a21efa11b
+15
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "DELETE FROM ws_specific\n WHERE workspace_id = $1 AND item_kind = 'variable' AND path = ANY($2)",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"TextArray"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "61caaa5c4ae62f618ba18f36330bbf58ff58e8c448874ffc9ca466165e545878"
|
||||
}
|
||||
+24
@@ -0,0 +1,24 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "WITH survivors AS (\n SELECT value::text AS rendered FROM resource\n WHERE workspace_id = $1 AND NOT (path = ANY($2::text[]))\n )\n SELECT v.path FROM unnest($3::text[]) AS v(path)\n WHERE EXISTS (\n SELECT 1 FROM survivors s\n WHERE strpos(s.rendered, '\"$var:' || v.path || '\"') > 0\n OR strpos(s.rendered, '\"$jsonvar:' || v.path || '\"') > 0\n )",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "path",
|
||||
"type_info": "Text"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"TextArray",
|
||||
"TextArray"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "a7b6731427d51eb34744ca6a39e30d02949f149901ec95323b5725911944df4d"
|
||||
}
|
||||
+23
@@ -0,0 +1,23 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "DELETE FROM variable WHERE workspace_id = $1 AND path = ANY($2) RETURNING path",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "path",
|
||||
"type_info": "Varchar"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"TextArray"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "d88b1c445acc5375e9c543dcec81059d0e7018a6cea79ba743693098d223edf5"
|
||||
}
|
||||
@@ -391,7 +391,8 @@ async fn test_create_resource_upsert_clears_ws_specific(db: Pool<Postgres>) -> a
|
||||
|
||||
/// Regression for GHSA-xmr2-98m6-cjf7: a token scoped only to `resources:write:<r>`
|
||||
/// must NOT use the resource-delete cascade to delete a linked secret variable it has
|
||||
/// no `variables:write` scope for.
|
||||
/// no `variables:write` scope for. The victim sits at a path the resource owns, which is
|
||||
/// the only kind the cascade reaches at all.
|
||||
#[sqlx::test(fixtures("ws_specific"))]
|
||||
async fn test_scoped_token_cannot_cascade_delete_linked_variable(
|
||||
db: Pool<Postgres>,
|
||||
@@ -406,7 +407,7 @@ async fn test_scoped_token_cannot_cascade_delete_linked_variable(
|
||||
"SECRET_TOKEN",
|
||||
)
|
||||
.json(&json!({
|
||||
"path": "u/test-user/victim_secret",
|
||||
"path": "u/test-user/db_victim_secret",
|
||||
"value": "hunter2",
|
||||
"is_secret": true,
|
||||
"description": ""
|
||||
@@ -421,7 +422,7 @@ async fn test_scoped_token_cannot_cascade_delete_linked_variable(
|
||||
)
|
||||
.json(&json!({
|
||||
"path": "u/test-user/db",
|
||||
"value": { "password": "$var:u/test-user/victim_secret" },
|
||||
"value": { "password": "$var:u/test-user/db_victim_secret" },
|
||||
"resource_type": "object"
|
||||
}))
|
||||
.send()
|
||||
@@ -443,9 +444,206 @@ async fn test_scoped_token_cannot_cascade_delete_linked_variable(
|
||||
resp.text().await?
|
||||
);
|
||||
assert!(
|
||||
variable_exists(&db, "test-workspace", "u/test-user/victim_secret").await?,
|
||||
variable_exists(&db, "test-workspace", "u/test-user/db_victim_secret").await?,
|
||||
"victim variable must survive the denied cascade"
|
||||
);
|
||||
// The scope check runs after the resource DELETE, so only the rollback keeps the resource
|
||||
// alive — moving the check out of the transaction would silently delete it on a 403.
|
||||
let resource_left: Option<i64> =
|
||||
sqlx::query_scalar("SELECT COUNT(*) FROM resource WHERE workspace_id = $1 AND path = $2")
|
||||
.bind("test-workspace")
|
||||
.bind("u/test-user/db")
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
resource_left.unwrap_or(0),
|
||||
1,
|
||||
"the denied delete must roll the resource back too"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Deleting a resource must not take a variable other things still need. Two gates, each
|
||||
/// with a way past the other: a variable outside the resource's own path is never its to
|
||||
/// delete, and even one it owns stays if another resource points at it.
|
||||
#[sqlx::test(fixtures("ws_specific"))]
|
||||
async fn test_resource_delete_spares_variables_it_does_not_own(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let base = format!("http://localhost:{port}/api/w/test-workspace");
|
||||
|
||||
let create_var = |path: &'static str| {
|
||||
authed(
|
||||
client().post(format!("{base}/variables/create")),
|
||||
"SECRET_TOKEN",
|
||||
)
|
||||
.json(&json!({ "path": path, "value": "hunter2", "is_secret": true, "description": "" }))
|
||||
.send()
|
||||
};
|
||||
let create_res = |path: &'static str, var: &'static str| {
|
||||
authed(
|
||||
client().post(format!("{base}/resources/create")),
|
||||
"SECRET_TOKEN",
|
||||
)
|
||||
.json(&json!({
|
||||
"path": path,
|
||||
"value": { "password": format!("$var:{var}") },
|
||||
"resource_type": "object"
|
||||
}))
|
||||
.send()
|
||||
};
|
||||
|
||||
// A shared secret at a path of its own, and two resources reading it.
|
||||
assert_eq!(create_var("u/test-user/shared_canary").await?.status(), 201);
|
||||
assert_eq!(
|
||||
create_res("u/test-user/probe_a", "u/test-user/shared_canary")
|
||||
.await?
|
||||
.status(),
|
||||
201
|
||||
);
|
||||
assert_eq!(
|
||||
create_res("u/test-user/probe_b", "u/test-user/shared_canary")
|
||||
.await?
|
||||
.status(),
|
||||
201
|
||||
);
|
||||
|
||||
// A secret the resource at the same path owns, which a second resource also reads.
|
||||
assert_eq!(create_var("u/test-user/owned").await?.status(), 201);
|
||||
assert_eq!(
|
||||
create_res("u/test-user/owned", "u/test-user/owned")
|
||||
.await?
|
||||
.status(),
|
||||
201
|
||||
);
|
||||
assert_eq!(
|
||||
create_res("u/test-user/borrower", "u/test-user/owned")
|
||||
.await?
|
||||
.status(),
|
||||
201
|
||||
);
|
||||
|
||||
for resource in ["u/test-user/probe_a", "u/test-user/owned"] {
|
||||
let resp = authed(
|
||||
client().delete(format!("{base}/resources/delete/{resource}")),
|
||||
"SECRET_TOKEN",
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(
|
||||
resp.status(),
|
||||
200,
|
||||
"delete {resource}: {}",
|
||||
resp.text().await?
|
||||
);
|
||||
}
|
||||
|
||||
assert!(
|
||||
variable_exists(&db, "test-workspace", "u/test-user/shared_canary").await?,
|
||||
"a variable the deleted resource only referenced must survive"
|
||||
);
|
||||
assert!(
|
||||
variable_exists(&db, "test-workspace", "u/test-user/owned").await?,
|
||||
"an owned variable another resource still references must survive"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// The bulk cascade follows what RLS actually deleted, not what the caller asked for: a
|
||||
/// resource the request names but leaves standing neither cascades nor stops counting as a
|
||||
/// referrer. Both halves matter, and neither covers the other.
|
||||
#[sqlx::test(fixtures("ws_specific"))]
|
||||
async fn test_bulk_delete_follows_what_rls_deleted(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let base = format!("http://localhost:{port}/api/w/test-workspace");
|
||||
|
||||
let create_var = |path: &'static str| {
|
||||
authed(
|
||||
client().post(format!("{base}/variables/create")),
|
||||
"SECRET_TOKEN",
|
||||
)
|
||||
.json(&json!({ "path": path, "value": "hunter2", "is_secret": true, "description": "" }))
|
||||
.send()
|
||||
};
|
||||
// ws_specific so the flag assertion at the end has something to check.
|
||||
let create_res = |path: &'static str, var: &'static str| {
|
||||
authed(
|
||||
client().post(format!("{base}/resources/create")),
|
||||
"SECRET_TOKEN",
|
||||
)
|
||||
.json(&json!({
|
||||
"path": path,
|
||||
"value": { "password": format!("$var:{var}") },
|
||||
"resource_type": "object",
|
||||
"ws_specific": true
|
||||
}))
|
||||
.send()
|
||||
};
|
||||
|
||||
// Private to test-user: a resource and the secret it owns.
|
||||
assert_eq!(create_var("u/test-user/hidden_pwd").await?.status(), 201);
|
||||
assert_eq!(
|
||||
create_res("u/test-user/hidden", "u/test-user/hidden_pwd")
|
||||
.await?
|
||||
.status(),
|
||||
201
|
||||
);
|
||||
// test-user-2's own resource and secret, which the private resource above also reads.
|
||||
assert_eq!(create_var("u/test-user-2/own_pwd").await?.status(), 201);
|
||||
assert_eq!(
|
||||
create_res("u/test-user-2/own", "u/test-user-2/own_pwd")
|
||||
.await?
|
||||
.status(),
|
||||
201
|
||||
);
|
||||
assert_eq!(
|
||||
create_res("u/test-user/reader", "u/test-user-2/own_pwd")
|
||||
.await?
|
||||
.status(),
|
||||
201
|
||||
);
|
||||
|
||||
// test-user-2 may write the private secret but has no access to its resource at all.
|
||||
sqlx::query(
|
||||
"UPDATE variable SET extra_perms = '{\"u/test-user-2\": true}'::jsonb
|
||||
WHERE workspace_id = 'test-workspace' AND path = 'u/test-user/hidden_pwd'",
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
|
||||
let resp = authed(
|
||||
client().delete(format!("{base}/resources/delete_bulk")),
|
||||
"SECRET_TOKEN_2",
|
||||
)
|
||||
.json(&json!({
|
||||
"paths": ["u/test-user/hidden", "u/test-user-2/own", "u/test-user/reader"]
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "bulk delete: {}", resp.text().await?);
|
||||
|
||||
assert!(
|
||||
variable_exists(&db, "test-workspace", "u/test-user/hidden_pwd").await?,
|
||||
"the variable of a resource RLS refused to delete must survive"
|
||||
);
|
||||
assert!(
|
||||
variable_exists(&db, "test-workspace", "u/test-user-2/own_pwd").await?,
|
||||
"a requested resource RLS left standing still counts as a referrer"
|
||||
);
|
||||
// ws_specific has no RLS policy of its own, so clearing it by requested path rather than
|
||||
// by deleted path would quietly turn a surviving resource workspace-generic.
|
||||
assert_eq!(
|
||||
ws_specific_row_count(&db, "test-workspace", "resource", "u/test-user/hidden").await?,
|
||||
1,
|
||||
"a resource RLS refused to delete must keep its ws_specific flag"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
*/
|
||||
|
||||
use dashmap::DashMap;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::net::IpAddr;
|
||||
use std::sync::LazyLock;
|
||||
|
||||
@@ -1339,53 +1339,21 @@ async fn delete_resource(
|
||||
return Err(Error::PermissionDenied(msg));
|
||||
}
|
||||
|
||||
let cascade = plan_linked_var_cascade(&db, &w_id, &[path.to_string()]).await?;
|
||||
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
|
||||
// Capture resource data for trashbin before deleting
|
||||
let trash_resource: Option<serde_json::Value> = sqlx::query_scalar(
|
||||
"SELECT to_jsonb(t) FROM resource t WHERE path = $1 AND workspace_id = $2",
|
||||
// The whole row comes back out of the delete, so the trashbin entry below is built from
|
||||
// what RLS actually removed, and the cascade runs only once RLS has allowed the delete.
|
||||
let deleted: Option<(String, serde_json::Value)> = sqlx::query_as(
|
||||
"DELETE FROM resource AS t WHERE t.path = $1 AND t.workspace_id = $2
|
||||
RETURNING t.path, to_jsonb(t)",
|
||||
)
|
||||
.bind(path)
|
||||
.bind(&w_id)
|
||||
.fetch_optional(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// Fetch the resource value before deleting, so we can find linked $var: references
|
||||
let resource_value: Option<Option<serde_json::Value>> =
|
||||
sqlx::query_scalar("SELECT value FROM resource WHERE path = $1 AND workspace_id = $2")
|
||||
.bind(path)
|
||||
.bind(&w_id)
|
||||
.fetch_optional(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// Collect all $var: paths referenced in the resource value
|
||||
let mut linked_var_paths: Vec<String> = Vec::new();
|
||||
if let Some(Some(ref value)) = resource_value {
|
||||
collect_var_refs(value, &mut linked_var_paths);
|
||||
}
|
||||
|
||||
// A scoped token must not delete linked variables it lacks variables:write for.
|
||||
check_linked_var_delete_scopes(&authed, &linked_var_paths)?;
|
||||
|
||||
// Capture linked variables for trashbin before deleting them
|
||||
let trash_linked_vars: Vec<serde_json::Value> = if linked_var_paths.is_empty() {
|
||||
Vec::new()
|
||||
} else {
|
||||
let placeholders: Vec<String> = linked_var_paths
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, _)| format!("${}", i + 2))
|
||||
.collect();
|
||||
let query = format!(
|
||||
"SELECT to_jsonb(t) FROM variable t WHERE workspace_id = $1 AND path IN ({})",
|
||||
placeholders.join(", ")
|
||||
);
|
||||
let mut q = sqlx::query_scalar::<_, serde_json::Value>(&query).bind(&w_id);
|
||||
for var_path in &linked_var_paths {
|
||||
q = q.bind(var_path);
|
||||
}
|
||||
q.fetch_all(&mut *tx).await?
|
||||
};
|
||||
let (deleted_path, res_data) = not_found_if_none(deleted, "Resource", &path)?;
|
||||
|
||||
sqlx::query!(
|
||||
"DELETE FROM ws_specific WHERE workspace_id = $1 AND item_kind = 'resource' AND path = $2",
|
||||
@@ -1395,64 +1363,63 @@ async fn delete_resource(
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
let deleted_path = sqlx::query_scalar!(
|
||||
"DELETE FROM resource WHERE path = $1 AND workspace_id = $2 RETURNING path",
|
||||
path,
|
||||
w_id
|
||||
let linked_var_paths = cascade.resolve(std::slice::from_ref(&deleted_path));
|
||||
|
||||
// A scoped token must not delete linked variables it lacks variables:write for. Erroring
|
||||
// here rolls the resource delete back with it, so nothing is deleted either way.
|
||||
check_linked_var_delete_scopes(&authed, &linked_var_paths)?;
|
||||
|
||||
// Capture linked variables for trashbin before deleting them
|
||||
let trash_linked_vars: Vec<(String, serde_json::Value)> = sqlx::query_as(
|
||||
"SELECT path, to_jsonb(t) FROM variable t WHERE workspace_id = $1 AND path = ANY($2)",
|
||||
)
|
||||
.fetch_optional(&mut *tx)
|
||||
.bind(&w_id)
|
||||
.bind(&linked_var_paths)
|
||||
.fetch_all(&mut *tx)
|
||||
.await?;
|
||||
not_found_if_none(deleted_path, "Resource", &path)?;
|
||||
|
||||
// Delete linked variables that are actually referenced in the resource value
|
||||
let deleted_linked_variables: Vec<String> = if linked_var_paths.is_empty() {
|
||||
Vec::new()
|
||||
} else {
|
||||
// Clean up any ws_specific rows for these variables first
|
||||
// (mark_linked_variables_ws_specific may have auto-inserted them) so
|
||||
// they don't survive the variable deletion as orphans — a variable
|
||||
// later recreated at the same path would otherwise inherit the stale
|
||||
// ws_specific flag.
|
||||
sqlx::query!(
|
||||
"DELETE FROM ws_specific
|
||||
WHERE workspace_id = $1 AND item_kind = 'variable' AND path = ANY($2)",
|
||||
w_id,
|
||||
&linked_var_paths
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
let deleted_linked_variables = sqlx::query_scalar!(
|
||||
"DELETE FROM variable WHERE workspace_id = $1 AND path = ANY($2) RETURNING path",
|
||||
w_id,
|
||||
&linked_var_paths
|
||||
)
|
||||
.fetch_all(&mut *tx)
|
||||
.await?;
|
||||
|
||||
let placeholders: Vec<String> = linked_var_paths
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, _)| format!("${}", i + 2))
|
||||
.collect();
|
||||
let query = format!(
|
||||
"DELETE FROM variable WHERE workspace_id = $1 AND path IN ({}) RETURNING path",
|
||||
placeholders.join(", ")
|
||||
);
|
||||
let mut q = sqlx::query_scalar::<_, String>(&query).bind(&w_id);
|
||||
for var_path in &linked_var_paths {
|
||||
q = q.bind(var_path);
|
||||
}
|
||||
q.fetch_all(&mut *tx).await?
|
||||
};
|
||||
// ws_specific has no FK to variable, so a row mark_linked_variables_ws_specific inserted
|
||||
// would survive as an orphan and a variable later recreated at that path would inherit
|
||||
// the stale flag.
|
||||
sqlx::query!(
|
||||
"DELETE FROM ws_specific
|
||||
WHERE workspace_id = $1 AND item_kind = 'variable' AND path = ANY($2)",
|
||||
w_id,
|
||||
&deleted_linked_variables
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
if let Some(res_data) = trash_resource {
|
||||
let mut trash_data = serde_json::json!({"row": res_data});
|
||||
if !trash_linked_vars.is_empty() {
|
||||
trash_data["linked_variables"] = serde_json::Value::Array(trash_linked_vars);
|
||||
}
|
||||
windmill_common::trashbin::move_to_trash(
|
||||
&mut *tx,
|
||||
&w_id,
|
||||
"resource",
|
||||
path,
|
||||
trash_data,
|
||||
&authed.username,
|
||||
)
|
||||
.await?;
|
||||
// Only the rows that actually went: the snapshot above is what the caller could read,
|
||||
// which is not necessarily what RLS let it delete, and the trashbin must not hold a copy
|
||||
// of a secret that is still live.
|
||||
let trash_linked_vars: Vec<serde_json::Value> = trash_linked_vars
|
||||
.into_iter()
|
||||
.filter(|(var_path, _)| deleted_linked_variables.contains(var_path))
|
||||
.map(|(_, row)| row)
|
||||
.collect();
|
||||
|
||||
let mut trash_data = serde_json::json!({"row": res_data});
|
||||
if !trash_linked_vars.is_empty() {
|
||||
trash_data["linked_variables"] = serde_json::Value::Array(trash_linked_vars);
|
||||
}
|
||||
windmill_common::trashbin::move_to_trash(
|
||||
&mut *tx,
|
||||
&w_id,
|
||||
"resource",
|
||||
path,
|
||||
trash_data,
|
||||
&authed.username,
|
||||
)
|
||||
.await?;
|
||||
|
||||
audit_log(
|
||||
&mut *tx,
|
||||
@@ -1464,6 +1431,24 @@ async fn delete_resource(
|
||||
None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
// The cascade is the one way a variable dies without a variables/delete request of its
|
||||
// own, so give each one the audit row it would have had, stamped with what took it.
|
||||
for var_path in &deleted_linked_variables {
|
||||
let mut params = HashMap::new();
|
||||
params.insert("via_resource", path);
|
||||
audit_log(
|
||||
&mut *tx,
|
||||
&authed,
|
||||
"variables.delete",
|
||||
ActionKind::Delete,
|
||||
&w_id,
|
||||
Some(var_path),
|
||||
Some(params),
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
tx.commit().await?;
|
||||
|
||||
// Resource gone for everyone: wipe ALL users' drafts at this path (and any linked
|
||||
@@ -1515,35 +1500,205 @@ async fn delete_resource(
|
||||
);
|
||||
}
|
||||
|
||||
Ok(format!("resource {} deleted", path))
|
||||
// Name what else went: the cascade is silent from the caller's side otherwise, and a
|
||||
// secret it took is not something to discover later from a failing job.
|
||||
if deleted_linked_variables.is_empty() {
|
||||
Ok(format!("resource {} deleted", path))
|
||||
} else {
|
||||
Ok(format!(
|
||||
"resource {} deleted, along with its linked variables: {}",
|
||||
path,
|
||||
deleted_linked_variables.join(", ")
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
/// Recursively collect all `$var:path` references from a JSON value.
|
||||
fn collect_var_refs(value: &serde_json::Value, out: &mut Vec<String>) {
|
||||
/// The forms that resolve a variable path against `variable`, so a value carrying any of them
|
||||
/// breaks when that variable goes. Only `$var:` is minted by the resource editor, which is why
|
||||
/// `collect_var_refs` stays narrower than this.
|
||||
const REFERRER_PREFIXES: [&str; 2] = ["$var:", "$jsonvar:"];
|
||||
|
||||
/// Recursively collect the variable paths a JSON value references through any of `prefixes`.
|
||||
fn collect_refs_with_prefixes(value: &serde_json::Value, prefixes: &[&str], out: &mut Vec<String>) {
|
||||
match value {
|
||||
serde_json::Value::String(s) => {
|
||||
if let Some(var_path) = s.strip_prefix("$var:") {
|
||||
if let Some(var_path) = prefixes.iter().find_map(|p| s.strip_prefix(p)) {
|
||||
out.push(var_path.to_string());
|
||||
}
|
||||
}
|
||||
serde_json::Value::Object(m) => {
|
||||
for v in m.values() {
|
||||
collect_var_refs(v, out);
|
||||
collect_refs_with_prefixes(v, prefixes, out);
|
||||
}
|
||||
}
|
||||
serde_json::Value::Array(arr) => {
|
||||
for v in arr {
|
||||
collect_var_refs(v, out);
|
||||
collect_refs_with_prefixes(v, prefixes, out);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
/// Deleting a resource cascades into the `$var:` variables its value references. A
|
||||
/// scoped token must not use that cascade to delete variables it could not delete
|
||||
/// directly via `delete_variable` (which gates on `variables:write:<path>`), so require
|
||||
/// `variables:write` for EVERY linked variable and fail the whole delete otherwise.
|
||||
/// Recursively collect all `$var:path` references from a JSON value.
|
||||
fn collect_var_refs(value: &serde_json::Value, out: &mut Vec<String>) {
|
||||
collect_refs_with_prefixes(value, &["$var:"], out)
|
||||
}
|
||||
|
||||
/// Whether the variable at `var_path` is the resource's own secret rather than one its value
|
||||
/// merely points at: the same-path twin `delete_variable` and the `update_resource` rename
|
||||
/// already act on, or `<resource path>_<field>`, which the connect form mints for a resource
|
||||
/// type with several secret fields. Anything else is a standalone workspace variable, and
|
||||
/// deleting one destroys a secret its other referrers still need.
|
||||
///
|
||||
/// A rename moves only the twin, so `<old path>_<field>` secrets stop matching and are left
|
||||
/// behind instead. An orphaned secret can be deleted by hand; a destroyed one cannot.
|
||||
fn is_owned_linked_var(resource_path: &str, var_path: &str) -> bool {
|
||||
var_path == resource_path
|
||||
|| var_path
|
||||
.strip_prefix(resource_path)
|
||||
.is_some_and(|suffix| suffix.starts_with('_'))
|
||||
}
|
||||
|
||||
/// Which of `var_paths` a resource outside `excluded_resource_paths` still references.
|
||||
///
|
||||
/// Must run off the non-RLS pool: a referrer in a folder the caller cannot read is precisely
|
||||
/// the one whose variable has to survive. Nothing from those rows reaches the response.
|
||||
async fn linked_vars_referenced_elsewhere(
|
||||
db: &DB,
|
||||
w_id: &str,
|
||||
var_paths: &[String],
|
||||
excluded_resource_paths: &[String],
|
||||
) -> Result<HashSet<String>> {
|
||||
if var_paths.is_empty() {
|
||||
return Ok(HashSet::new());
|
||||
}
|
||||
// The quotes around the pattern are what make it a whole-JSON-string match rather than a
|
||||
// prefix one, so `f/db` does not match `"$var:f/db_replica"`. Paths are `proper_id`
|
||||
// segments, so no JSON escaping or LIKE wildcard can reach this.
|
||||
let referenced = sqlx::query_scalar!(
|
||||
"WITH survivors AS (
|
||||
SELECT value::text AS rendered FROM resource
|
||||
WHERE workspace_id = $1 AND NOT (path = ANY($2::text[]))
|
||||
)
|
||||
SELECT v.path FROM unnest($3::text[]) AS v(path)
|
||||
WHERE EXISTS (
|
||||
SELECT 1 FROM survivors s
|
||||
WHERE strpos(s.rendered, '\"$var:' || v.path || '\"') > 0
|
||||
OR strpos(s.rendered, '\"$jsonvar:' || v.path || '\"') > 0
|
||||
)",
|
||||
w_id,
|
||||
excluded_resource_paths,
|
||||
var_paths,
|
||||
)
|
||||
.fetch_all(db)
|
||||
.await?;
|
||||
Ok(referenced.into_iter().flatten().collect())
|
||||
}
|
||||
|
||||
/// A resource delete's candidate cascade, gathered before the caller's transaction opens: the
|
||||
/// referrer scan runs on `db` because it has to see resources RLS hides, and a second acquire
|
||||
/// from that same pool under an open `user_db` transaction stalls to the acquire timeout when
|
||||
/// `DATABASE_CONNECTIONS` is small. `resolve` then decides without touching the database.
|
||||
///
|
||||
/// So a resource that starts referencing a candidate between the scan and the delete keeps a
|
||||
/// `$var:` pointing at nothing. Narrowing that window means running the scan on the
|
||||
/// transaction's own connection under a tightly scoped `SET LOCAL ROLE NONE` (the elevation
|
||||
/// `windmill-queue/src/schedule.rs` uses); closing it needs a lock on every resource write.
|
||||
struct LinkedVarCascade {
|
||||
/// Each owned `$var:` path with the requested resource whose value carries it.
|
||||
candidates: Vec<(String, String)>,
|
||||
/// Requested resource paths, each with every variable path its value references.
|
||||
requested: Vec<(String, Vec<String>)>,
|
||||
/// Candidate paths a resource outside the requested set still references.
|
||||
referenced_outside: HashSet<String>,
|
||||
}
|
||||
|
||||
impl LinkedVarCascade {
|
||||
/// The variables to delete, now that RLS has settled which resources went.
|
||||
///
|
||||
/// A requested resource left standing is the one referrer the scan could not account for,
|
||||
/// having had to exclude every requested path before RLS had ruled.
|
||||
fn resolve(&self, deleted_paths: &[String]) -> Vec<String> {
|
||||
let referenced_by_survivors: HashSet<&str> = self
|
||||
.requested
|
||||
.iter()
|
||||
.filter(|(path, _)| !deleted_paths.contains(path))
|
||||
.flat_map(|(_, refs)| refs.iter().map(String::as_str))
|
||||
.collect();
|
||||
|
||||
let mut resolved: Vec<String> = self
|
||||
.candidates
|
||||
.iter()
|
||||
.filter(|(var_path, owner)| {
|
||||
deleted_paths.contains(owner)
|
||||
&& !self.referenced_outside.contains(var_path.as_str())
|
||||
&& !referenced_by_survivors.contains(var_path.as_str())
|
||||
})
|
||||
.map(|(var_path, _)| var_path.clone())
|
||||
.collect();
|
||||
resolved.sort();
|
||||
resolved.dedup();
|
||||
resolved
|
||||
}
|
||||
|
||||
/// Which deleted resource the cascade took `var_path` for, to stamp on its audit row.
|
||||
fn owner_of<'a>(&'a self, var_path: &str, deleted_paths: &[String]) -> Option<&'a str> {
|
||||
self.candidates
|
||||
.iter()
|
||||
.find(|(candidate, owner)| candidate == var_path && deleted_paths.contains(owner))
|
||||
.map(|(_, owner)| owner.as_str())
|
||||
}
|
||||
}
|
||||
|
||||
/// Gather what `LinkedVarCascade::resolve` needs for a delete of `paths`.
|
||||
async fn plan_linked_var_cascade(
|
||||
db: &DB,
|
||||
w_id: &str,
|
||||
paths: &[String],
|
||||
) -> Result<LinkedVarCascade> {
|
||||
let rows: Vec<(String, Option<serde_json::Value>)> = sqlx::query_as(
|
||||
"SELECT path, value FROM resource WHERE workspace_id = $1 AND path = ANY($2)",
|
||||
)
|
||||
.bind(w_id)
|
||||
.bind(paths)
|
||||
.fetch_all(db)
|
||||
.await?;
|
||||
|
||||
let mut candidates: Vec<(String, String)> = Vec::new();
|
||||
let mut requested: Vec<(String, Vec<String>)> = Vec::new();
|
||||
for (path, value) in rows {
|
||||
let mut owned: Vec<String> = Vec::new();
|
||||
let mut refs: Vec<String> = Vec::new();
|
||||
if let Some(value) = &value {
|
||||
collect_var_refs(value, &mut owned);
|
||||
collect_refs_with_prefixes(value, &REFERRER_PREFIXES, &mut refs);
|
||||
}
|
||||
candidates.extend(
|
||||
owned
|
||||
.into_iter()
|
||||
.filter(|var_path| is_owned_linked_var(&path, var_path))
|
||||
.map(|var_path| (var_path, path.clone())),
|
||||
);
|
||||
requested.push((path, refs));
|
||||
}
|
||||
candidates.sort();
|
||||
candidates.dedup();
|
||||
|
||||
let mut candidate_paths: Vec<String> = candidates
|
||||
.iter()
|
||||
.map(|(var_path, _)| var_path.clone())
|
||||
.collect();
|
||||
candidate_paths.dedup();
|
||||
let referenced_outside =
|
||||
linked_vars_referenced_elsewhere(db, w_id, &candidate_paths, paths).await?;
|
||||
Ok(LinkedVarCascade { candidates, requested, referenced_outside })
|
||||
}
|
||||
|
||||
/// Deleting a resource cascades into the `$var:` variables it owns. A scoped token must not
|
||||
/// use that cascade to delete variables it could not delete directly via `delete_variable`
|
||||
/// (which gates on `variables:write:<path>`), so require `variables:write` for EVERY cascaded
|
||||
/// variable and fail the whole delete otherwise.
|
||||
///
|
||||
/// No co-located-path exemption: a resource and a variable may share a path, and a
|
||||
/// resource-write token can create a resource over an existing standalone variable and
|
||||
@@ -1646,111 +1801,91 @@ async fn delete_resources_bulk(
|
||||
return Err(Error::PermissionDenied(msg));
|
||||
}
|
||||
|
||||
let cascade = plan_linked_var_cascade(&db, &w_id, &request.paths).await?;
|
||||
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
|
||||
// Capture resources for trashbin per path before bulk delete, and
|
||||
// collect $var: references so we can cascade-delete the linked variables
|
||||
// (matching single-resource delete semantics).
|
||||
let mut linked_var_paths: Vec<String> = Vec::new();
|
||||
for path in &request.paths {
|
||||
let trash_resource: Option<serde_json::Value> = sqlx::query_scalar(
|
||||
"SELECT to_jsonb(t) FROM resource t WHERE path = $1 AND workspace_id = $2",
|
||||
)
|
||||
.bind(path)
|
||||
.bind(&w_id)
|
||||
.fetch_optional(&mut *tx)
|
||||
.await?;
|
||||
|
||||
if let Some(res_data) = trash_resource {
|
||||
// Per-resource linked vars so each resource's trash entry carries
|
||||
// exactly the variables that vanished with it (matching the
|
||||
// single-delete shape: trash_data["linked_variables"]).
|
||||
let mut this_linked: Vec<String> = Vec::new();
|
||||
if let Some(value) = res_data.get("value") {
|
||||
collect_var_refs(value, &mut this_linked);
|
||||
}
|
||||
this_linked.sort();
|
||||
this_linked.dedup();
|
||||
|
||||
let trash_linked_vars: Vec<serde_json::Value> = if this_linked.is_empty() {
|
||||
Vec::new()
|
||||
} else {
|
||||
let placeholders: Vec<String> = this_linked
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, _)| format!("${}", i + 2))
|
||||
.collect();
|
||||
let query = format!(
|
||||
"SELECT to_jsonb(t) FROM variable t WHERE workspace_id = $1 AND path IN ({})",
|
||||
placeholders.join(", ")
|
||||
);
|
||||
let mut q = sqlx::query_scalar::<_, serde_json::Value>(&query).bind(&w_id);
|
||||
for var_path in &this_linked {
|
||||
q = q.bind(var_path);
|
||||
}
|
||||
q.fetch_all(&mut *tx).await?
|
||||
};
|
||||
|
||||
let mut trash_data = serde_json::json!({"row": res_data});
|
||||
if !trash_linked_vars.is_empty() {
|
||||
trash_data["linked_variables"] = serde_json::Value::Array(trash_linked_vars);
|
||||
}
|
||||
windmill_common::trashbin::move_to_trash(
|
||||
&mut *tx,
|
||||
&w_id,
|
||||
"resource",
|
||||
path,
|
||||
trash_data,
|
||||
&authed.username,
|
||||
)
|
||||
.await?;
|
||||
|
||||
linked_var_paths.extend(this_linked);
|
||||
}
|
||||
}
|
||||
linked_var_paths.sort();
|
||||
linked_var_paths.dedup();
|
||||
|
||||
// A scoped token must not delete linked variables it lacks variables:write for.
|
||||
check_linked_var_delete_scopes(&authed, &linked_var_paths)?;
|
||||
// Whole rows out of the delete; see delete_resource. RLS can leave a requested resource
|
||||
// standing, so everything below is driven by this list rather than by `request.paths`.
|
||||
let deleted: Vec<(String, serde_json::Value)> = sqlx::query_as(
|
||||
"DELETE FROM resource AS t WHERE t.path = ANY($1) AND t.workspace_id = $2
|
||||
RETURNING t.path, to_jsonb(t)",
|
||||
)
|
||||
.bind(&request.paths)
|
||||
.bind(&w_id)
|
||||
.fetch_all(&mut *tx)
|
||||
.await?;
|
||||
let deleted_paths: Vec<String> = deleted.iter().map(|(path, _)| path.clone()).collect();
|
||||
|
||||
sqlx::query!(
|
||||
"DELETE FROM ws_specific WHERE workspace_id = $1 AND item_kind = 'resource' AND path = ANY($2)",
|
||||
w_id,
|
||||
&request.paths
|
||||
&deleted_paths
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
let deleted_paths = sqlx::query_scalar!(
|
||||
"DELETE FROM resource WHERE path = ANY($1) AND workspace_id = $2 RETURNING path",
|
||||
&request.paths,
|
||||
w_id
|
||||
let linked_var_paths = cascade.resolve(&deleted_paths);
|
||||
|
||||
// A scoped token must not delete linked variables it lacks variables:write for. Erroring
|
||||
// here rolls the resource deletes back with it.
|
||||
check_linked_var_delete_scopes(&authed, &linked_var_paths)?;
|
||||
|
||||
// Snapshot before the delete below: the trashbin entries need the rows.
|
||||
let trash_linked_vars: Vec<(String, serde_json::Value)> = sqlx::query_as(
|
||||
"SELECT path, to_jsonb(t) FROM variable t WHERE workspace_id = $1 AND path = ANY($2)",
|
||||
)
|
||||
.bind(&w_id)
|
||||
.bind(&linked_var_paths)
|
||||
.fetch_all(&mut *tx)
|
||||
.await?;
|
||||
|
||||
let deleted_linked_variables = sqlx::query_scalar!(
|
||||
"DELETE FROM variable WHERE workspace_id = $1 AND path = ANY($2) RETURNING path",
|
||||
w_id,
|
||||
&linked_var_paths
|
||||
)
|
||||
.fetch_all(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// Cascade-clean linked variables: delete any ws_specific 'variable' rows
|
||||
// (typically auto-inserted by mark_linked_variables_ws_specific when the
|
||||
// resource was ws_specific) BEFORE deleting the variable rows themselves
|
||||
// — otherwise those ws_specific rows survive as orphans and a later
|
||||
// variable created at the same path would inherit a stale flag.
|
||||
if !linked_var_paths.is_empty() {
|
||||
sqlx::query!(
|
||||
"DELETE FROM ws_specific
|
||||
WHERE workspace_id = $1 AND item_kind = 'variable' AND path = ANY($2)",
|
||||
w_id,
|
||||
&linked_var_paths
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
// See delete_resource: ws_specific has no FK, so the rows would orphan.
|
||||
sqlx::query!(
|
||||
"DELETE FROM ws_specific
|
||||
WHERE workspace_id = $1 AND item_kind = 'variable' AND path = ANY($2)",
|
||||
w_id,
|
||||
&deleted_linked_variables
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
sqlx::query!(
|
||||
"DELETE FROM variable WHERE workspace_id = $1 AND path = ANY($2)",
|
||||
w_id,
|
||||
&linked_var_paths
|
||||
for (path, res_data) in &deleted {
|
||||
// Every cascaded variable this resource's value points at, ownership aside: restoring
|
||||
// it on its own must bring back each secret it needs, and the one it borrowed from a
|
||||
// sibling in the same batch is gone too.
|
||||
let mut refs: Vec<String> = Vec::new();
|
||||
if let Some(value) = res_data.get("value") {
|
||||
collect_var_refs(value, &mut refs);
|
||||
}
|
||||
let this_linked: Vec<serde_json::Value> = trash_linked_vars
|
||||
.iter()
|
||||
.filter(|(var_path, _)| {
|
||||
refs.contains(var_path) && deleted_linked_variables.contains(var_path)
|
||||
})
|
||||
.map(|(_, row)| row.clone())
|
||||
.collect();
|
||||
|
||||
let mut trash_data = serde_json::json!({"row": res_data});
|
||||
if !this_linked.is_empty() {
|
||||
trash_data["linked_variables"] = serde_json::Value::Array(this_linked);
|
||||
}
|
||||
windmill_common::trashbin::move_to_trash(
|
||||
&mut *tx,
|
||||
&w_id,
|
||||
"resource",
|
||||
path,
|
||||
trash_data,
|
||||
&authed.username,
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
}
|
||||
|
||||
@@ -1765,13 +1900,30 @@ async fn delete_resources_bulk(
|
||||
)
|
||||
.await?;
|
||||
|
||||
// See delete_resource: a cascaded variable gets the audit row it would have had.
|
||||
for var_path in &deleted_linked_variables {
|
||||
let params = cascade
|
||||
.owner_of(var_path, &deleted_paths)
|
||||
.map(|resource_path| HashMap::from([("via_resource", resource_path)]));
|
||||
audit_log(
|
||||
&mut *tx,
|
||||
&authed,
|
||||
"variables.delete",
|
||||
ActionKind::Delete,
|
||||
&w_id,
|
||||
Some(var_path),
|
||||
params,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
tx.commit().await?;
|
||||
|
||||
// Wipe ALL users' drafts at these paths (and linked variables); see delete_resource.
|
||||
for path in &deleted_paths {
|
||||
delete_all_drafts_for_path(&db, &w_id, UserDraftItemKind::Resource, path).await?;
|
||||
}
|
||||
for var_path in &linked_var_paths {
|
||||
for var_path in &deleted_linked_variables {
|
||||
delete_all_drafts_for_path(&db, &w_id, UserDraftItemKind::Variable, var_path).await?;
|
||||
}
|
||||
|
||||
|
||||
@@ -343,7 +343,9 @@
|
||||
if (account) {
|
||||
OauthService.disconnectAccount({ workspace: $workspaceStore!, id: account })
|
||||
}
|
||||
await ResourceService.deleteResource({ workspace: $workspaceStore!, path })
|
||||
// The response names the linked variables that went with it, which nothing else on the
|
||||
// page would show.
|
||||
sendUserToast(await ResourceService.deleteResource({ workspace: $workspaceStore!, path }))
|
||||
reload()
|
||||
}
|
||||
|
||||
@@ -762,8 +764,8 @@
|
||||
>
|
||||
{#if deleteIsLinked}
|
||||
<Alert type="warning" title="Linked variable">
|
||||
This resource is linked with a variable of the same path. The linked variable will also be
|
||||
deleted.
|
||||
This resource is linked with a variable of the same path. That variable is deleted with it,
|
||||
unless another resource still references it.
|
||||
</Alert>
|
||||
{/if}
|
||||
<Alert type="info" title="Bypass confirmation">
|
||||
|
||||
Reference in New Issue
Block a user