fix(backend): strip NUL bytes from draft values on write (#9673)

draft.value is a json column (not jsonb), so a client could store a U+0000
escape in it. Any later text extraction (`->>` / `to_jsonb`) on such a value
raises 22P05 "unsupported Unicode escape sequence" — one poisoned draft 500'd
the whole GET /drafts/list, silently hiding the home-page "This workspace has N
drafts" banner (and breaking the global drafts page).

Prevent it at the source: sanitize the value in update_draft (the only path that
writes client-supplied draft content) so a NUL never reaches the column.
strip_json_nul does a single backslash-parity-aware byte pass that removes real
NUL escapes (values and keys alike) while leaving a legitimate escaped backslash
intact — O(n) with no serde_json::Value tree to allocate, important because the
slow path is also hit by any value legitimately containing the text after a
backslash (e.g. script source). The clean path is a single substring check.

A SQL migration scrubs rows written before this, gated to genuinely-poisoned
rows (a real NUL makes value::jsonb raise, distinguishing it from a legitimately
escaped backslash). With the data clean, no read-side query needs to change.

Tests: unit tests for the strip helper (escaped-backslash no-op, real+literal
collision, odd-backslash-run parity, nested keys/values) and an integration test
that POSTs a NUL-bearing draft and asserts it is stored and listed NUL-free
(fails without the strip).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

backend/migrations/20260619113554_scrub_draft_value_nul.down.sql

@@ -0,0 +1,2 @@
+-- Irreversible: a stripped NUL cannot be restored (and was never meaningful).
+SELECT 1;

backend/migrations/20260619113554_scrub_draft_value_nul.up.sql

@@ -0,0 +1,30 @@
+-- One-time cleanup of drafts whose `json` value carries a real U+0000 (NUL)
+-- escape — storable only because `draft.value` is `json`, not `jsonb`. Such a
+-- value makes any `->>`/`to_jsonb` extraction raise `22P05`, which 500'd
+-- GET /drafts/list. New writes are sanitized in the application layer
+-- (update_draft → strip_json_nul); this fixes rows written before that landed.
+--
+-- Only genuinely-poisoned rows are touched: a real NUL makes `value::jsonb`
+-- raise, which distinguishes it from a legitimately escaped backslash sequence
+-- (which `jsonb` accepts). The text replace handles the real-world shape — a NUL
+-- inside a text field. A contrived value where stripping the escape leaves
+-- invalid JSON is left as-is (and can no longer be created).
+DO $$
+DECLARE r RECORD;
+BEGIN
+    FOR r IN
+        SELECT id, value FROM draft WHERE position(E'\\u0000' in value::text) > 0
+    LOOP
+        BEGIN
+            PERFORM r.value::jsonb; -- not poisoned (legit escaped backslash): skip
+        EXCEPTION WHEN others THEN
+            BEGIN
+                UPDATE draft
+                SET value = replace(r.value::text, E'\\u0000', '')::json
+                WHERE id = r.id;
+            EXCEPTION WHEN others THEN
+                NULL; -- pathological shape; cannot strip in SQL, no longer creatable
+            END;
+        END;
+    END LOOP;
+END $$;

backend/tests/drafts_nul.rs

@@ -0,0 +1,80 @@
+//! Regression test for NUL bytes in draft values.
+//!
+//! `draft.value` is a `json` column (not `jsonb`), so a U+0000 escape can be
+//! stored and then make any `->>`/`to_jsonb` extraction raise `22P05` — one
+//! poisoned draft 500'd `GET /drafts/list` (silently hiding the home-page
+//! "This workspace has N drafts" banner). The fix sanitizes the value on write
+//! (`update_draft` -> `strip_json_nul`) so a NUL never reaches the column; this
+//! drives the real endpoint and asserts the stored + listed value is NUL-free.
+
+use serde_json::{json, Value};
+use sqlx::{Pool, Postgres};
+
+use windmill_test_utils::*;
+
+fn client() -> reqwest::Client {
+    reqwest::Client::new()
+}
+
+fn authed(b: reqwest::RequestBuilder) -> reqwest::RequestBuilder {
+    b.header("Authorization", "Bearer DNUL_ADMIN_TOKEN")
+}
+
+#[sqlx::test(fixtures("drafts_nul"))]
+async fn test_draft_write_strips_nul(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/dnul-ws");
+
+    // Save a draft whose summary and content carry a real NUL.
+    let resp = authed(client().post(format!(
+        "{base}/drafts/update/script/u/dnul-admin/poison"
+    )))
+    .json(&json!({
+        "value": {
+            "summary": "hi\u{0}there",
+            "path": "u/dnul-admin/poison",
+            "content": "x\u{0}y"
+        }
+    }))
+    .send()
+    .await?;
+    assert_eq!(
+        resp.status(),
+        200,
+        "save should succeed: {}",
+        resp.text().await.unwrap_or_default()
+    );
+
+    // The stored value must be NUL-free (sanitized on write).
+    let stored: Value = authed(client().get(format!(
+        "{base}/drafts/get_own/script/u/dnul-admin/poison"
+    )))
+    .send()
+    .await?
+    .json()
+    .await?;
+    let value = stored.get("value").expect("draft should exist");
+    assert_eq!(value["summary"], "hithere");
+    assert_eq!(value["content"], "xy");
+    assert!(
+        !serde_json::to_string(value).unwrap().contains("\\u0000"),
+        "stored value still contains a NUL escape: {value}"
+    );
+
+    // The list endpoint uses raw `->>`; it works (200, no 500) because the
+    // stored data is clean, and the summary comes back stripped.
+    let items: Vec<Value> = authed(client().get(format!("{base}/drafts/list")))
+        .send()
+        .await?
+        .json()
+        .await?;
+    let item = items
+        .iter()
+        .find(|d| d["path"] == "u/dnul-admin/poison")
+        .expect("saved draft should be listed");
+    assert_eq!(item["summary"], "hithere");
+
+    Ok(())
+}

backend/tests/fixtures/drafts_nul.sql

@@ -0,0 +1,24 @@
+-- Fixture for the draft NUL-byte write-sanitization regression test.
+-- Just a workspace + admin user + token; the test itself POSTs a draft whose
+-- value carries a U+0000 and asserts it is stored (and listed) NUL-free.
+
+INSERT INTO workspace (id, name, owner) VALUES
+    ('dnul-ws', 'DNUL WS', 'dnul-admin');
+
+INSERT INTO workspace_key (workspace_id, kind, key) VALUES
+    ('dnul-ws', 'cloud', 'dnul-key');
+
+INSERT INTO workspace_settings (workspace_id) VALUES
+    ('dnul-ws');
+
+INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES
+    ('dnul-ws', 'all', 'All users', '{}');
+
+INSERT INTO password(email, password_hash, login_type, super_admin, verified, name, username)
+    VALUES ('dnul-admin@windmill.dev', 'x', 'password', true, true, 'DNUL Admin', 'dnul-admin');
+
+INSERT INTO usr(workspace_id, email, username, is_admin, role) VALUES
+    ('dnul-ws', 'dnul-admin@windmill.dev', 'dnul-admin', true, 'Admin');
+
+INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin)
+    VALUES (encode(sha256('DNUL_ADMIN_TOKEN'::bytea), 'hex'), 'DNUL_ADMIN', 'DNUL_ADMIN_TOKEN', 'dnul-admin@windmill.dev', 't', true);

backend/windmill-api/src/drafts.rs

@@ -286,6 +286,10 @@ async fn update_draft(
         } else {
             serde_json::to_string(value).unwrap()
         };
+        // `draft.value` is a `json` column, so a U+0000 (NUL) would persist as an
+        // escape and later make any `->>`/`to_jsonb` extraction raise `22P05`.
+        // Strip it here so a NUL never reaches the column.
+        let serialized = strip_json_nul(serialized);
         // Upsert. The conflict check rides on the DO UPDATE WHERE clause —
         // when the row is newer than `last_sync`, RETURNING yields nothing.
         // `created_at` defaults to `now()` but the migration overrides it ($8)
@@ -453,6 +457,53 @@ async fn migrate_legacy_draft(
     }
 }
 
+/// Remove every U+0000 (NUL) from a serialized JSON document so it is safe to
+/// store in the `json`-typed `draft.value` (a NUL there would later make any
+/// `->>`/`to_jsonb` extraction raise `22P05`).
+///
+/// A NUL can only appear in JSON text as a backslash-u0000 escape, and a
+/// backslash only ever occurs inside a string, so one backslash-parity-aware
+/// pass removes every real NUL escape — covering values and keys alike — while
+/// leaving a legitimate `\\u0000` (an escaped backslash followed by the literal
+/// text `u0000`) intact. O(n) over the bytes with no `serde_json::Value` tree to
+/// allocate, and the fast path (no such substring at all) returns the input
+/// untouched. The slow path is reached not only by genuinely poisoned values but
+/// by any value that legitimately contains `u0000` after a backslash (e.g. script
+/// source), so it must stay allocation-light for potentially large drafts.
+fn strip_json_nul(serialized: String) -> String {
+    if !serialized.contains("\\u0000") {
+        return serialized;
+    }
+    let bytes = serialized.as_bytes();
+    let mut out: Vec<u8> = Vec::with_capacity(bytes.len());
+    let mut i = 0;
+    while i < bytes.len() {
+        if bytes[i] != b'\\' {
+            out.push(bytes[i]);
+            i += 1;
+            continue;
+        }
+        // Consume the whole run of backslashes. An even run is N/2 escaped
+        // backslashes and leaves the next char unescaped; an odd run ends in an
+        // escaping backslash, so a following `u0000` is a real NUL escape.
+        let run_start = i;
+        while i < bytes.len() && bytes[i] == b'\\' {
+            i += 1;
+        }
+        let run = i - run_start;
+        if run % 2 == 1 && bytes[i..].starts_with(b"u0000") {
+            // Drop the escaping backslash + `u0000`; keep the leading literal pairs.
+            out.extend(std::iter::repeat(b'\\').take(run - 1));
+            i += 5;
+        } else {
+            out.extend(std::iter::repeat(b'\\').take(run));
+        }
+    }
+    // Only whole ASCII backslash-u0000 escapes were removed, so the bytes remain
+    // valid UTF-8 (and valid JSON).
+    String::from_utf8(out).expect("removing a NUL escape preserves valid UTF-8")
+}
+
 /// For variable-kind drafts with `variable.is_secret == true`, encrypt
 /// `variable.value` with the workspace crypt key and mark it
 /// `$encrypted:<base64>` so the secret never persists in plaintext at rest.
@@ -743,3 +794,64 @@ async fn require_can_read_path(
     }
     Err(Error::NotFound(format!("no draft visible at {path}")))
 }
+
+
+#[cfg(test)]
+mod tests {
+    use super::strip_json_nul;
+
+    // Parse the (NUL-free) result so assertions read clearly.
+    fn parsed(s: String) -> serde_json::Value {
+        serde_json::from_str(&s).expect("strip_json_nul must return valid JSON")
+    }
+
+    #[test]
+    fn clean_value_is_returned_byte_for_byte() {
+        let s = r#"{"summary":"all good","n":1}"#.to_string();
+        assert_eq!(strip_json_nul(s.clone()), s);
+    }
+
+    #[test]
+    fn real_nul_in_value_is_stripped() {
+        let out = strip_json_nul(r#"{"summary":"hi\u0000there"}"#.to_string());
+        assert!(!out.contains(r"\u0000"));
+        assert_eq!(parsed(out)["summary"], "hithere");
+    }
+
+    #[test]
+    fn legit_escaped_backslash_is_a_noop() {
+        // JSON "a\\u0000b" decodes to the 8-char string a,backslash,u,0,0,0,0,b
+        // — not a NUL — so the value is already clean and round-trips byte-for-byte.
+        let s = r#"{"summary":"a\\u0000b"}"#.to_string();
+        assert_eq!(strip_json_nul(s.clone()), s);
+    }
+
+    #[test]
+    fn collision_real_and_literal_both_handled() {
+        // "a" carries a real NUL; "b" carries the literal text backslash-u0000.
+        // The value walk strips the former and leaves the latter intact — the
+        // pathological case that needed a fallback in SQL is trivial in Rust.
+        let v = parsed(strip_json_nul(r#"{"a":"x\u0000y","b":"p\\u0000q"}"#.to_string()));
+        assert_eq!(v["a"], "xy");
+        assert_eq!(v["b"], "p\\u0000q");
+    }
+
+    #[test]
+    fn nested_values_and_keys_are_cleaned() {
+        let out = strip_json_nul(
+            r#"{"o":{"k\u0000":["a\u0000b",{"deep\u0000":"v\u0000"}]}}"#.to_string(),
+        );
+        assert!(!out.contains(r"\u0000"));
+        let v = parsed(out);
+        assert_eq!(v["o"]["k"][0], "ab");
+        assert_eq!(v["o"]["k"][1]["deep"], "v");
+    }
+
+    #[test]
+    fn odd_backslash_run_keeps_literal_drops_nul() {
+        // JSON "a\\\u0000b" is an escaped backslash (kept) immediately followed by
+        // a real NUL escape (dropped) -> decodes to a,backslash,b.
+        let v = parsed(strip_json_nul(r#"{"x":"a\\\u0000b"}"#.to_string()));
+        assert_eq!(v["x"], "a\\b");
+    }
+}