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: a draft-only app move refuses the other app kind; a session keeps an unknown base unknown

Browse Source 
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-09-14 20:51:14 +02:00
co-authored by Claude Opus 5
parent ee2c2abcb9
commit 68cd7a4420
6 changed files with 81 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/.sqlx/query-25ce9aff8844fbfc1a9e16ac7c4b9d0c37c27da9f9768f6520ac3c96b5765840.json → backend/.sqlx/query-9b574bf93759d822066ac1629744a8df58840c463d8aa412871bd9774d978bae.json
Generated
+4 -3
View File
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "SELECT\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $3\n AND typ = $2 AND email = $4) as \"at_target!\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email = $4\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0\n ) as \"poisoned!\" ",
"query": "SELECT\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $3\n AND typ::text = ANY($6::text[]) AND email = $4) as \"at_target!\",\n EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5\n AND typ = $2 AND email = $4\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0\n ) as \"poisoned!\" ",
"describe": {
"columns": [
{
@@ -54,7 +54,8 @@
},
"Text",
"Text",
"Text"
"Text",
"TextArray"
]
},
"nullable": [
@@ -62,5 +63,5 @@
null
]
},
"hash": "25ce9aff8844fbfc1a9e16ac7c4b9d0c37c27da9f9768f6520ac3c96b5765840"
"hash": "9b574bf93759d822066ac1629744a8df58840c463d8aa412871bd9774d978bae"
}
backend/.sqlx/query-0b247ac75331db89c41629d11aee5f13f78e2ac82265f84ea3f67925eb542140.json → backend/.sqlx/query-e27e447e4d9607fe280807e396049bbe31e4e938d9f53eeb659cdc49f9be8f2c.json
Generated
+4 -3
View File
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE draft\n SET path = $3,\n -- Both path keys, not just the typed one: the editors mirror the\n -- typed path into the other while it differs from the row's path,\n -- and the loaders prefer the mirror — left naming the old location\n -- it un-does this move on the next save. `create_missing = false`\n -- on both, so a draft carrying only one keeps only one.\n value = to_json(\n jsonb_set(\n jsonb_set(\n CASE WHEN $7::text IS NULL THEN to_jsonb(value)\n ELSE jsonb_set(to_jsonb(value), ARRAY['summary'], to_jsonb($7::text))\n END,\n ARRAY[$5::text], to_jsonb($3::text), false\n ),\n ARRAY[$8::text], to_jsonb($3::text), false\n )\n )\n WHERE workspace_id = $1\n AND path = $2\n AND typ = $4\n AND email = $6\n -- A pre-sanitizer NUL escape makes `to_jsonb` raise 22P05. Excluded\n -- here so the statement can't 500; reported below instead. Unlike the\n -- passive carry, rewriting the value IS this operation, so skipping it\n -- silently would move the row and leave its typed path stale.\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) = 0\n -- Skipped on a summary-only edit, where the \"target\" row is this\n -- row and the guard would refuse the update against itself.\n AND ($2 = $3 OR NOT EXISTS (\n SELECT 1 FROM draft o\n WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ = $4 AND o.email = $6\n ))\n RETURNING id",
"query": "UPDATE draft\n SET path = $3,\n -- Both path keys, not just the typed one: the editors mirror the\n -- typed path into the other while it differs from the row's path,\n -- and the loaders prefer the mirror — left naming the old location\n -- it un-does this move on the next save. `create_missing = false`\n -- on both, so a draft carrying only one keeps only one.\n value = to_json(\n jsonb_set(\n jsonb_set(\n CASE WHEN $7::text IS NULL THEN to_jsonb(value)\n ELSE jsonb_set(to_jsonb(value), ARRAY['summary'], to_jsonb($7::text))\n END,\n ARRAY[$5::text], to_jsonb($3::text), false\n ),\n ARRAY[$8::text], to_jsonb($3::text), false\n )\n )\n WHERE workspace_id = $1\n AND path = $2\n AND typ = $4\n AND email = $6\n -- A pre-sanitizer NUL escape makes `to_jsonb` raise 22P05. Excluded\n -- here so the statement can't 500; reported below instead. Unlike the\n -- passive carry, rewriting the value IS this operation, so skipping it\n -- silently would move the row and leave its typed path stale.\n AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) = 0\n -- Skipped on a summary-only edit, where the \"target\" row is this\n -- row and the guard would refuse the update against itself.\n AND ($2 = $3 OR NOT EXISTS (\n SELECT 1 FROM draft o\n WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ::text = ANY($9::text[])\n AND o.email = $6\n ))\n RETURNING id",
"describe": {
"columns": [
{
@@ -52,12 +52,13 @@
"Text",
"Text",
"Text",
"Text"
"Text",
"TextArray"
]
},
"nullable": [
false
]
},
"hash": "0b247ac75331db89c41629d11aee5f13f78e2ac82265f84ea3f67925eb542140"
"hash": "e27e447e4d9607fe280807e396049bbe31e4e938d9f53eeb659cdc49f9be8f2c"
}
backend/tests/drafts_move_taken.rs
+42
View File
@@ -73,3 +73,45 @@ async fn test_rename_onto_a_draft_is_refused(db: Pool<Postgres>) -> anyhow::Resu
Ok(())
}
/// A classic app and a raw app deploy into the same table, so a draft-only move onto
/// the other kind's draft must be refused: deploying either path afterwards deletes
/// the caller's drafts of both kinds, taking the loser's item with it.
#[sqlx::test(fixtures("base", "drafts_move_taken"))]
async fn test_draft_move_refuses_the_other_app_kind(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
let resp = reqwest::Client::new()
.post(format!(
"http://localhost:{port}/api/w/test-workspace/drafts/move/raw_app/u/test-user/mvtaken_raw"
))
.header("Authorization", "Bearer SECRET_TOKEN")
.json(&json!({ "new_path": "u/test-user/mvtaken_app" }))
.send()
.await?;
let status = resp.status();
let body = resp.text().await?;
assert_eq!(status, 400, "move onto a classic app draft was allowed: {body}");
assert!(body.contains("already have a draft"), "unexpected refusal: {body}");
// Both drafts are untouched.
let list: Vec<Value> = reqwest::Client::new()
.get(format!(
"http://localhost:{port}/api/w/test-workspace/drafts/list"
))
.header("Authorization", "Bearer SECRET_TOKEN")
.send()
.await?
.json()
.await?;
let mut kinds = list
.iter()
.filter(|d| matches!(d["kind"].as_str(), Some("app") | Some("raw_app")))
.filter_map(|d| d["kind"].as_str())
.collect::<Vec<_>>();
kinds.sort();
assert_eq!(kinds, vec!["app", "raw_app"], "{list:?}");
Ok(())
}
backend/tests/fixtures/drafts_move_taken.sql
Vendored
+8
View File
@@ -15,3 +15,11 @@ INSERT INTO draft (workspace_id, path, typ, value, email)
VALUES ('test-workspace', 'u/test-user/mvtaken_b', 'script',
'{"path": "u/test-user/mvtaken_b", "summary": "B", "content": ""}',
'test@windmill.dev');
-- A draft-only classic app and a draft-only raw app of the same owner. They share
-- the `app` table, so one occupies the other's path.
INSERT INTO draft (workspace_id, path, typ, value, email) VALUES
('test-workspace', 'u/test-user/mvtaken_app', 'app',
'{"summary": "classic", "value": {}}', 'test@windmill.dev'),
('test-workspace', 'u/test-user/mvtaken_raw', 'raw_app',
'{"summary": "raw", "files": {}}', 'test@windmill.dev');
backend/windmill-api/src/drafts.rs
+16 -2
View File
@@ -679,6 +679,17 @@ async fn move_draft(
}
}
// A classic app and a raw app share the `app` table, so a draft of either kind
// occupies the destination for both: deploying there deletes the caller's drafts
// of both kinds, taking the item that lost the collision with it.
let collision_typs: Vec<&str> = match kind {
UserDraftItemKind::App | UserDraftItemKind::RawApp => vec![
UserDraftItemKind::App.as_str(),
UserDraftItemKind::RawApp.as_str(),
],
_ => vec![kind.as_str()],
};
// One transaction with the move record, so a save addressed to the old path
// never sees the row gone without knowing where it went.
let mut tx = db.begin().await?;
@@ -714,7 +725,8 @@ async fn move_draft(
-- row and the guard would refuse the update against itself.
AND ($2 = $3 OR NOT EXISTS (
SELECT 1 FROM draft o
WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ = $4 AND o.email = $6
WHERE o.workspace_id = $1 AND o.path = $3 AND o.typ::text = ANY($9::text[])
AND o.email = $6
))
RETURNING id"#,
&w_id,
@@ -725,6 +737,7 @@ async fn move_draft(
&authed.email,
req.summary,
mirror_field,
&collision_typs as &[&str],
)
.fetch_optional(&mut *tx)
.await?;
@@ -745,7 +758,7 @@ async fn move_draft(
let row = sqlx::query!(
r#"SELECT
EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $3
AND typ = $2 AND email = $4) as "at_target!",
AND typ::text = ANY($6::text[]) AND email = $4) as "at_target!",
EXISTS(SELECT 1 FROM draft WHERE workspace_id = $1 AND path = $5
AND typ = $2 AND email = $4
AND position(chr(92) || 'u0000' in replace(value::text, chr(92) || chr(92), '')) > 0
@@ -755,6 +768,7 @@ async fn move_draft(
new_path,
&authed.email,
path,
&collision_typs as &[&str],
)
.fetch_one(&db)
.await?;
frontend/src/lib/components/sessions/sessionRuntime.svelte.ts
+7 -5
View File
@@ -849,11 +849,13 @@ function createRuntime(session: Session): SessionRuntime {
path: result.path,
custom_path: draftValue?.custom_path ?? result.custom_path,
draft_path: draftValue?.draft_path,
// The draft's own base, else the head this checkout forks from (the
// standalone editor's loader does the same).
parent_version:
draftValue?.parent_version ??
(Array.isArray(result.versions) ? result.versions[result.versions.length - 1] : undefined)
// Only a fresh checkout forks from the head; a draft keeps its own base,
// unknown included, or it would read as up to date. See loadScript.
parent_version: draftValue
? draftValue.parent_version
: Array.isArray(result.versions)
? result.versions[result.versions.length - 1]
: undefined
}
// Seed the per-tab last_sync from the server draft's timestamp so
// later saves attach a matching last_sync and the server can reject