mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-05 08:02:18 +00:00
fix: strip NUL characters from app values at save time (#9903)
App values are persisted to a json column, which permits the JSON NUL escape (backslash-u-0000), but are later converted to jsonb (e.g. a workspace fork clone_apps, search indexing), which rejects it with "unsupported Unicode escape sequence" -- silently making the app un-forkable. The usual source is a binary file such as .DS_Store accidentally bundled into a raw app file map. A real NUL is unstorable in jsonb either way, and frontend code that needs the character writes it as the source escape (which JSON-encodes to an escaped backslash + literal u0000 and is left untouched), so rather than hard-failing the save we strip genuine NULs and warn. Add strip_null_chars and apply it at both app_version insert sites (create_app_internal and update_app_internal, covering the regular and raw create/update routes). It removes a genuine NUL escape (odd run of backslashes before u0000) while preserving an even run. Returns a borrowed Cow (no allocation) when the value is already clean. Covered by unit tests. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
5c521d808a
commit
3ec1f164be
@@ -1,4 +1,4 @@
|
||||
use std::{collections::HashMap, sync::Arc};
|
||||
use std::{borrow::Cow, collections::HashMap, sync::Arc};
|
||||
|
||||
/*
|
||||
* Author: Ruben Fiszel
|
||||
@@ -1818,6 +1818,50 @@ fn custom_path_conflict_error(
|
||||
}
|
||||
}
|
||||
|
||||
/// App values live in a `json` column, which — unlike `jsonb` — accepts the
|
||||
/// `\u0000` escape. Any later `json`→`jsonb` conversion (a workspace fork's
|
||||
/// `clone_apps`, search indexing, …) then aborts with "unsupported Unicode
|
||||
/// escape sequence". Strip genuine NULs so the value is jsonb-safe before it
|
||||
/// lands in the DB; the usual source is a binary file such as `.DS_Store`
|
||||
/// accidentally bundled into a raw app's file map. A real NUL is unstorable
|
||||
/// either way, and frontend code that needs the character writes it as the
|
||||
/// source escape `\u0000`, which JSON-encodes to `\\u0000` (an escaped
|
||||
/// backslash — the even-parity case below) and is left untouched.
|
||||
///
|
||||
/// Returns `Cow::Borrowed` (no allocation) when the value is already clean.
|
||||
fn strip_null_chars(raw: &str) -> Cow<'_, str> {
|
||||
let bytes = raw.as_bytes();
|
||||
let mut out: Option<String> = None;
|
||||
let mut copied_to = 0;
|
||||
let mut search_from = 0;
|
||||
// A genuine NUL is `\u0000`: a `u0000` introduced by an *odd* run of
|
||||
// backslashes. An even run (`\\u0000`) is an escaped backslash then the
|
||||
// literal text "u0000" (common in minified JS regexes) and is preserved.
|
||||
while let Some(rel) = raw[search_from..].find("u0000") {
|
||||
let at = search_from + rel;
|
||||
let mut backslashes = 0;
|
||||
let mut j = at;
|
||||
while j > 0 && bytes[j - 1] == b'\\' {
|
||||
backslashes += 1;
|
||||
j -= 1;
|
||||
}
|
||||
if backslashes % 2 == 1 {
|
||||
// Drop the escaping backslash + `u0000` — the 6 chars in [at-1, at+5).
|
||||
let out = out.get_or_insert_with(String::new);
|
||||
out.push_str(&raw[copied_to..at - 1]);
|
||||
copied_to = at + 5;
|
||||
}
|
||||
search_from = at + 5;
|
||||
}
|
||||
match out {
|
||||
Some(mut out) => {
|
||||
out.push_str(&raw[copied_to..]);
|
||||
Cow::Owned(out)
|
||||
}
|
||||
None => Cow::Borrowed(raw),
|
||||
}
|
||||
}
|
||||
|
||||
async fn create_app_internal<'a>(
|
||||
authed: ApiAuthed,
|
||||
db: sqlx::Pool<sqlx::Postgres>,
|
||||
@@ -1961,13 +2005,18 @@ async fn create_app_internal<'a>(
|
||||
)
|
||||
.fetch_one(&mut *tx)
|
||||
.await?;
|
||||
// `.get()` keeps the raw text (and thus key order); strip any NUL so the
|
||||
// `json`→`jsonb` conversion downstream (fork, indexing) can't choke on it.
|
||||
let value = strip_null_chars(app.value.0.get());
|
||||
if matches!(value, Cow::Owned(_)) {
|
||||
tracing::warn!(path = %app.path, "stripped NUL character(s) from app value on create");
|
||||
}
|
||||
let v_id = sqlx::query_scalar!(
|
||||
"INSERT INTO app_version
|
||||
(app_id, value, created_by, raw_app)
|
||||
VALUES ($1, $2::text::json, $3, $4) RETURNING id",
|
||||
id,
|
||||
//to preserve key orders
|
||||
serde_json::to_string(&app.value).unwrap(),
|
||||
value.as_ref(),
|
||||
authed.username,
|
||||
raw_app
|
||||
)
|
||||
@@ -2541,13 +2590,18 @@ async fn update_app_internal<'a>(
|
||||
.fetch_one(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// `.get()` keeps the raw text (and thus key order); strip any NUL so the
|
||||
// `json`→`jsonb` conversion downstream (fork, indexing) can't choke on it.
|
||||
let value = strip_null_chars(nvalue.0.get());
|
||||
if matches!(value, Cow::Owned(_)) {
|
||||
tracing::warn!(path = %npath, "stripped NUL character(s) from app value on update");
|
||||
}
|
||||
let v_id = sqlx::query_scalar!(
|
||||
"INSERT INTO app_version
|
||||
(app_id, value, created_by, raw_app)
|
||||
VALUES ($1, $2::text::json, $3, $4) RETURNING id",
|
||||
app_id,
|
||||
//to preserve key orders
|
||||
serde_json::to_string(&nvalue).unwrap(),
|
||||
value.as_ref(),
|
||||
authed.username,
|
||||
raw_app
|
||||
)
|
||||
@@ -4388,3 +4442,62 @@ mod embed_token_tests {
|
||||
assert!(parse_embed_policy("not json").is_err());
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod strip_null_chars_tests {
|
||||
use super::strip_null_chars;
|
||||
use std::borrow::Cow;
|
||||
|
||||
// Build `{"k":"<n backslashes>u0000"}` without writing the escape literally
|
||||
// (a real NUL can't live in Rust source). Odd n => the trailing `u0000` is a
|
||||
// genuine NUL escape; even n => an escaped backslash then the text "u0000".
|
||||
fn doc(backslashes: usize) -> String {
|
||||
format!(r#"{{"k":"{}u0000"}}"#, "\\".repeat(backslashes))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strips_genuine_null_escape() {
|
||||
// 1 backslash: the NUL escape is dropped, the string value becomes "".
|
||||
assert_eq!(strip_null_chars(&doc(1)).as_ref(), r#"{"k":""}"#);
|
||||
// 3 backslashes: escaped backslash + NUL -> keep the escaped backslash.
|
||||
let three = doc(3);
|
||||
let out = strip_null_chars(&three);
|
||||
assert_eq!(out.as_ref(), r#"{"k":"\\"}"#);
|
||||
// Result is now valid, NUL-free JSON (i.e. jsonb-safe).
|
||||
let v: serde_json::Value = serde_json::from_str(out.as_ref()).unwrap();
|
||||
assert!(!v["k"].as_str().unwrap().as_bytes().contains(&0u8));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_escaped_backslash_then_literal_u0000() {
|
||||
// Even runs are the literal text "u0000" (e.g. a minified JS regex char
|
||||
// class) and must be returned untouched, with no allocation.
|
||||
for n in [2usize, 4] {
|
||||
let s = doc(n);
|
||||
let out = strip_null_chars(&s);
|
||||
assert_eq!(out.as_ref(), s.as_str());
|
||||
assert!(matches!(out, Cow::Borrowed(_)), "n={n} should be borrowed");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_clean_values() {
|
||||
// Plain value, and the bare token "u0000" with no preceding backslash.
|
||||
for s in [r#"{"files":{"/index.tsx":"hello"}}"#, r#"{"k":"u0000"}"#] {
|
||||
let out = strip_null_chars(s);
|
||||
assert_eq!(out.as_ref(), s);
|
||||
assert!(matches!(out, Cow::Borrowed(_)));
|
||||
}
|
||||
// The escape for a literal backslash char (`u005c`) then text "u0000":
|
||||
// the only "u0000" match is preceded by `c` (0 backslashes) -> no NUL.
|
||||
let s = format!(r#"{{"k":"{}u005cu0000"}}"#, "\\");
|
||||
assert!(matches!(strip_null_chars(&s), Cow::Borrowed(_)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strips_multiple_and_preserves_surrounding() {
|
||||
// Mirrors the .DS_Store case: several NULs interleaved with real text.
|
||||
let s = format!(r#"{{"a":"x{b}u0000{b}u0000y","b":"ok"}}"#, b = "\\");
|
||||
assert_eq!(strip_null_chars(&s).as_ref(), r#"{"a":"xy","b":"ok"}"#);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user