fix: skip NUL-poisoned rows in every draft-value rewrite, not just the first

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Guilhem Lemouel
2026-09-08 12:40:08 +02:00
co-authored by Claude Opus 5
parent c6bf9ca965
commit e895cdd4db
4 changed files with 69 additions and 12 deletions
@@ -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"
}
+4 -2
View File
@@ -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
+27 -8
View File
@@ -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::<Vec<_>>();
let clean_ids = moved
.iter()
.filter(|r| !r.poisoned)
.map(|r| r.id)
.collect::<Vec<_>>();
// 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
@@ -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<UserDraftItemKind, 'script' | 'flow' | 'app' | 'raw_app'> {
return item.kind === 'app' && item.rawApp ? 'raw_app' : item.kind
}