diff --git a/backend/.sqlx/query-bc72ca60f647a75b039d2ee0359e177da621325b48e70215e9e40ab431d7499b.json b/backend/.sqlx/query-bc72ca60f647a75b039d2ee0359e177da621325b48e70215e9e40ab431d7499b.json new file mode 100644 index 0000000000..64d0ab9def --- /dev/null +++ b/backend/.sqlx/query-bc72ca60f647a75b039d2ee0359e177da621325b48e70215e9e40ab431d7499b.json @@ -0,0 +1,32 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE draft AS d\n SET path = $3,\n value = CASE\n WHEN position(chr(92) || 'u0000' in replace(d.value::text, chr(92) || chr(92), '')) > 0\n THEN d.value\n WHEN to_jsonb(d.value) -> $4::text = to_jsonb($2::text)\n THEN to_json(jsonb_set(to_jsonb(d.value), ARRAY[$4::text], to_jsonb($3::text), false))\n ELSE d.value\n END\n WHERE d.workspace_id = $1\n AND d.path = $2\n AND d.typ::text = ANY($5::text[])\n AND NOT EXISTS (\n SELECT 1 FROM draft o\n WHERE o.workspace_id = d.workspace_id\n AND o.path = $3\n AND o.typ = d.typ\n AND o.email IS NOT DISTINCT FROM d.email\n )\n RETURNING d.id,\n position(chr(92) || 'u0000' in replace(d.value::text, chr(92) || chr(92), '')) > 0\n as \"poisoned!\" ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "poisoned!", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Text", + "Text", + "Text", + "TextArray" + ] + }, + "nullable": [ + false, + null + ] + }, + "hash": "bc72ca60f647a75b039d2ee0359e177da621325b48e70215e9e40ab431d7499b" +} diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index da0bf467b2..b8954251b0 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -9457,7 +9457,7 @@ paths: /w/{workspace}/drafts/move/{kind}/{path}: post: summary: move the current user's draft-only item to another path - description: Relocates the authed user's own draft row (and the typed path inside its value). Only for draft-only items — a deployed item must be moved through its own deploy endpoint, which carries every draft with it. + description: Relocates the authed user's own draft row (and the typed path inside its value). Only for draft-only items — a deployed item must be moved through its own deploy endpoint, which carries every draft with it. Restricted to script, flow, app and raw_app; any other kind is rejected with 400, because only these keep their deploy target where this endpoint rewrites it. operationId: moveDraft tags: - draft @@ -9466,8 +9466,10 @@ paths: - name: kind in: path required: true + description: script, flow, app or raw_app only. schema: - $ref: "#/components/schemas/UserDraftItemKind" + type: string + enum: [script, flow, app, raw_app] - $ref: "#/components/parameters/ScriptPath" requestBody: required: true diff --git a/backend/windmill-common/src/user_drafts.rs b/backend/windmill-common/src/user_drafts.rs index 3aa80fa338..b58f18b257 100644 --- a/backend/windmill-common/src/user_drafts.rs +++ b/backend/windmill-common/src/user_drafts.rs @@ -631,11 +631,19 @@ pub async fn move_drafts_for_path( // guard (see `backend/tests/drafts_nul.rs`). Such a row is skipped by the // rewrite and carried on its `path` column alone: one teammate's poisoned // draft must not abort an unrelated user's rename mid-transaction. - let moved = sqlx::query_scalar!( + // `poisoned` is computed once here and returned, so the two follow-up + // statements can skip the same rows without re-deriving it. The `replace` + // strips escaped backslashes first: only an ODD-parity backslash-u0000 is a real NUL + // escape, and a draft whose source text legitimately contains those six + // characters (`s.replace("\u0000", "")` in a Python step) serialises as + // `\\u0000` and converts to jsonb perfectly well — flagging it would silently + // skip its rewrite and leave the path stale. + let moved = sqlx::query!( r#"UPDATE draft AS d SET path = $3, value = CASE - WHEN position(chr(92) || 'u0000' in d.value::text) > 0 THEN d.value + WHEN position(chr(92) || 'u0000' in replace(d.value::text, chr(92) || chr(92), '')) > 0 + THEN d.value WHEN to_jsonb(d.value) -> $4::text = to_jsonb($2::text) THEN to_json(jsonb_set(to_jsonb(d.value), ARRAY[$4::text], to_jsonb($3::text), false)) ELSE d.value @@ -650,7 +658,9 @@ pub async fn move_drafts_for_path( AND o.typ = d.typ AND o.email IS NOT DISTINCT FROM d.email ) - RETURNING d.id"#, + RETURNING d.id, + position(chr(92) || 'u0000' in replace(d.value::text, chr(92) || chr(92), '')) > 0 + as "poisoned!" "#, w_id, old_path, new_path, @@ -659,6 +669,15 @@ pub async fn move_drafts_for_path( ) .fetch_all(&mut **tx) .await?; + // Every carried row, for the outcome count; only the convertible ones for the + // value rewrites below — `to_jsonb` on a poisoned row raises 22P05 and would + // abort the whole deploy transaction. + let moved_ids = moved.iter().map(|r| r.id).collect::>(); + let clean_ids = moved + .iter() + .filter(|r| !r.poisoned) + .map(|r| r.id) + .collect::>(); // A flow draft carries the deployed path it forked from in `path`, next to // the staged rename in `draft_path`. The editor layers the draft over the @@ -667,7 +686,7 @@ pub async fn move_drafts_for_path( // triggers following it. Same tri-state rule as the typed path. Scripts need // no second pass (their typed path IS `path`); an app draft has no such key // and `create_missing = false` leaves it untouched. - if typed_path_field != "path" && !moved.is_empty() { + if typed_path_field != "path" && !clean_ids.is_empty() { sqlx::query!( r#"UPDATE draft SET value = to_json( @@ -679,14 +698,14 @@ pub async fn move_drafts_for_path( WHERE id = ANY($3)"#, new_path, old_path, - &moved, + &clean_ids, ) .execute(&mut **tx) .await?; } if let Some((field, version)) = base_version { - if !moved.is_empty() { + if !clean_ids.is_empty() { sqlx::query!( r#"UPDATE draft SET value = to_json( @@ -695,7 +714,7 @@ pub async fn move_drafts_for_path( WHERE id = ANY($3) AND email = $4"#, field, version, - &moved, + &clean_ids, restamp_email, ) .execute(&mut **tx) @@ -716,7 +735,7 @@ pub async fn move_drafts_for_path( .fetch_one(&mut **tx) .await?; - Ok(MoveDraftsOutcome { moved: moved.len(), left_behind: left_behind as usize }) + Ok(MoveDraftsOutcome { moved: moved_ids.len(), left_behind: left_behind as usize }) } /// What `move_drafts_for_path` did. `left_behind` is non-zero only when the diff --git a/frontend/src/lib/components/home/bulkActions.ts b/frontend/src/lib/components/home/bulkActions.ts index 1d7492d5a7..da88e19121 100644 --- a/frontend/src/lib/components/home/bulkActions.ts +++ b/frontend/src/lib/components/home/bulkActions.ts @@ -14,8 +14,12 @@ import { updateItemPathAndSummary } from '$lib/components/moveRenameManager' import { discardDraft } from '$lib/utils_draft_deploy' import type { BulkItem } from './homeSelection.svelte' -/** The draft overlay is the one place a raw app is its own kind. */ -function draftKind(item: BulkItem): UserDraftItemKind { +/** The draft overlay is the one place a raw app is its own kind. Narrowed to the + * four kinds `moveDraft` accepts — a `BulkItem` is never anything else, and + * saying so lets the compiler check that rather than trusting it. */ +function draftKind( + item: BulkItem +): Extract { return item.kind === 'app' && item.rawApp ? 'raw_app' : item.kind }