chore: normalize NUL escapes in job args before name-based checks (#11989)

backend/tests/app_run_mode_lock_strip.rs

@@ -110,10 +110,13 @@ async fn test_run_mode_strips_caller_lock_and_modules(db: Pool<Postgres>) -> any
     .json(&json!({
         // The args map is the other injection channel: an inline run is a
         // `JobKind::Preview` job, so the worker/executors read `_MODULES` and
-        // `_TEMP_SCRIPT_REFS` back out of the job args. Both must be stripped.
+        // `_TEMP_SCRIPT_REFS` back out of the job args. Both must be stripped —
+        // including the NUL spellings, which `push` stores under the plain name.
         "args": {
             "_MODULES": { "m.py": { "content": "print('evil')\n", "language": "python3" } },
-            "_TEMP_SCRIPT_REFS": { "../evil": "deadbeef" }
+            "_MODULES\u{0}": { "m.py": { "content": "print('evil')\n", "language": "python3" } },
+            "_TEMP_SCRIPT_REFS": { "../evil": "deadbeef" },
+            "_TEMP_SCRIPT_REFS\u{0}": { "../evil": "deadbeef" }
         },
         "component": "comp",
         "raw_code": inline_raw_code(Some(CALLER_HASH), true)

backend/tests/caller_modules_strip.rs

@@ -20,10 +20,16 @@ fn modules(v: &str) -> serde_json::Value {
 
 /// Pushes `payload` with a caller `_MODULES` both in `args` and in `extra` (where webhook
 /// query and headers land), and returns the stored `_MODULES`.
+///
+/// Every spelling `push` stores as `_MODULES` is sent, not just the exact name: it writes
+/// the args with their `\u0000` escapes removed, keys included, so `_MODULES\0` lands in
+/// the job as `_MODULES` unless the removal compares the name that will be stored.
 async fn stored_modules(db: &Pool<Postgres>, payload: JobPayload) -> Option<serde_json::Value> {
     let caller: Box<RawValue> = serde_json::value::to_raw_value(&modules("caller")).unwrap();
-    let args = HashMap::from([("_MODULES".to_string(), caller.clone())]);
-    let extra = HashMap::from([("_MODULES".to_string(), caller)]);
+    let spellings =
+        || ["_MODULES", "_MODULES\0", "_MOD\0ULES"].map(|k| (k.to_string(), caller.clone()));
+    let args = HashMap::from(spellings());
+    let extra = HashMap::from(spellings());
     let (id, tx) = windmill_queue::push(
         db,
         PushIsolationLevel::IsolatedRoot(db.clone()),

backend/tests/reserved_control_args.rs

@@ -0,0 +1,436 @@
+//! `_ENTRYPOINT_OVERRIDE`, `_MODULES` and `_TEMP_SCRIPT_REFS` steer *how* a runnable runs:
+//! which function the worker calls instead of `main`, what module files land in the job
+//! directory, where its workspace imports resolve from. They are legitimate for a caller
+//! running its own code under its own identity, and must not be settable by a third party
+//! whose payload runs under somebody else's: an HTTP route body or native trigger message
+//! (the job runs as the trigger's owner) and a public app viewer's inputs (as the
+//! publisher).
+//!
+//! `push` stores the args with their `\u0000` escapes removed, keys included, so every
+//! check also has to hold for a NUL-spelled key: `_ENTRYPOINT_OVERRIDE\0` is read back as
+//! `_ENTRYPOINT_OVERRIDE`. The same rename is what lets a viewer's `tenant\0` land as
+//! `tenant` over an app policy's pinned `tenant`.
+
+use std::collections::HashMap;
+
+use serde_json::{json, value::RawValue, Value};
+use sqlx::{Pool, Postgres};
+use windmill_common::{
+    jobs::JobPayload,
+    runnable_settings::{ConcurrencySettings, DebouncingSettings},
+    scripts::{ScriptHash, ScriptLang},
+    triggers::TriggerMetadata,
+};
+use windmill_queue::{PushArgs, PushArgsOwned, PushIsolationLevel};
+use windmill_test_utils::*;
+
+const SCRIPT_PATH: &str = "f/system/hello";
+const SCRIPT_HASH: i64 = 123412;
+/// Two top-level functions, as in the report: `main` authenticates, `apply` acts.
+const SCRIPT_CONTENT: &str = "export async function main(data: string) { return data }\n\
+                              export async function apply(action: string) { return action }\n";
+
+fn client() -> reqwest::Client {
+    reqwest::Client::new()
+}
+
+fn authed(b: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuilder {
+    b.header("Authorization", format!("Bearer {}", token))
+}
+
+fn raw(v: Value) -> Box<RawValue> {
+    serde_json::value::to_raw_value(&v).unwrap()
+}
+
+fn modules() -> Value {
+    json!({ "bunfig.toml": { "content": "preload = [\"./x.ts\"]", "language": "bun" } })
+}
+
+/// Deploy `SCRIPT_PATH` (a non-null `lock` is what makes a script count as deployed).
+async fn deploy_script(db: &Pool<Postgres>) -> anyhow::Result<()> {
+    sqlx::query(
+        r#"INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by,
+             schema, summary, description, lock)
+           VALUES ('test-workspace', $1, $2, $3, 'bun', 'script', 'test-user', '{}', '', '',
+             E'{}\n//bun.lock\n<empty>')"#,
+    )
+    .bind(SCRIPT_HASH)
+    .bind(SCRIPT_PATH)
+    .bind(SCRIPT_CONTENT)
+    .execute(db)
+    .await?;
+    Ok(())
+}
+
+async fn job_args_and_entrypoint(
+    db: &Pool<Postgres>,
+    id: uuid::Uuid,
+) -> anyhow::Result<(Value, Option<String>)> {
+    Ok(
+        sqlx::query_as("SELECT args, script_entrypoint_override FROM v2_job WHERE id = $1")
+            .bind(id)
+            .fetch_one(db)
+            .await?,
+    )
+}
+
+fn trigger_authed() -> windmill_api_auth::ApiAuthed {
+    windmill_api_auth::ApiAuthed {
+        email: "test@windmill.dev".to_string(),
+        username: "test-user".to_string(),
+        is_admin: true,
+        is_operator: false,
+        groups: vec![],
+        folders: vec![],
+        scopes: None,
+        username_override: None,
+        username_override_is_token_label: false,
+        is_session_token: false,
+        token_prefix: None,
+        read_only: false,
+        job_id: None,
+        credential_expiry: None,
+    }
+}
+
+/// `trigger_runnable_inner` is the one funnel every trigger reaches `push` through, so it
+/// stands in here for the HTTP route whose body keys become the job's top-level args. The
+/// sender is anonymous on a `none`/`custom_script` route; the job runs as the route owner.
+#[sqlx::test(fixtures("base"))]
+async fn trigger_args_lose_every_reserved_control_key(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    deploy_script(&db).await?;
+
+    let args = PushArgsOwned {
+        args: HashMap::from([
+            // Picks `apply` instead of `main`, skipping whatever `main` authenticates.
+            ("_ENTRYPOINT_OVERRIDE".to_string(), raw(json!("apply"))),
+            ("_ENTRYPOINT_OVERRIDE\u{0}".to_string(), raw(json!("apply"))),
+            ("_ENTRY\u{0}POINT_OVERRIDE".to_string(), raw(json!("apply"))),
+            ("_MODULES\u{0}".to_string(), raw(modules())),
+            (
+                "_TEMP_SCRIPT_REFS\u{0}".to_string(),
+                raw(json!({ "f/system/dep": "deadbeef" })),
+            ),
+            // An ordinary input must still get through, NUL-renamed or not.
+            ("data".to_string(), raw(json!("payload"))),
+            ("action\u{0}".to_string(), raw(json!("refund"))),
+        ]),
+        extra: None,
+    };
+
+    let (id, _, _, _, _) = windmill_trigger::trigger_helpers::trigger_runnable_inner(
+        &db,
+        None,
+        None,
+        trigger_authed(),
+        "test-workspace",
+        SCRIPT_PATH,
+        false,
+        args,
+        None,
+        None,
+        None,
+        "u/test-user/route".to_string(),
+        None,
+        TriggerMetadata::new(
+            Some("u/test-user/route".to_string()),
+            windmill_common::jobs::JobTriggerKind::Http,
+        ),
+        None,
+    )
+    .await?;
+
+    let (args, entrypoint) = job_args_and_entrypoint(&db, id).await?;
+    assert_eq!(
+        args,
+        json!({ "data": "payload", "action": "refund" }),
+        "a trigger payload may carry the runnable's inputs and nothing that steers how it runs"
+    );
+    assert_eq!(
+        entrypoint, None,
+        "a trigger payload must not pick the function the worker calls instead of `main`"
+    );
+    Ok(())
+}
+
+/// The counterpart: `_ENTRYPOINT_OVERRIDE` stays for a caller that runs the script under
+/// its own identity with `jobs:run` — the script editor's preprocessor test, dynamic-select
+/// helpers, the CLI and workflow-as-code all send it this way (GHSA-wxjq-w5pj-jqhx bounds
+/// what it may contain, not who may set it). Stripping it in `push` would break them.
+#[sqlx::test(fixtures("base"))]
+async fn an_authenticated_push_keeps_the_entrypoint_override(
+    db: Pool<Postgres>,
+) -> anyhow::Result<()> {
+    deploy_script(&db).await?;
+
+    // Both spellings: `push` normalizes the names to what it stores, so the NUL-spelled one
+    // selects the function exactly like the plain one instead of being silently different.
+    for key in ["_ENTRYPOINT_OVERRIDE", "_ENTRYPOINT_OVERRIDE\u{0}"] {
+        let args = HashMap::from([
+            (key.to_string(), raw(json!("apply"))),
+            ("action".to_string(), raw(json!("refund"))),
+        ]);
+        let (id, tx) = windmill_queue::push(
+            &db,
+            PushIsolationLevel::IsolatedRoot(db.clone()),
+            "test-workspace",
+            JobPayload::ScriptHash {
+                hash: ScriptHash(SCRIPT_HASH),
+                path: SCRIPT_PATH.to_string(),
+                cache_ttl: None,
+                cache_ignore_s3_path: None,
+                dedicated_worker: None,
+                language: ScriptLang::Bun,
+                priority: None,
+                apply_preprocessor: false,
+                concurrency_settings: ConcurrencySettings::default(),
+                debouncing_settings: DebouncingSettings::default(),
+                labels: None,
+                job_token_scopes: None,
+            },
+            PushArgs { args: &args, extra: None },
+            "test-user",
+            "test@windmill.dev",
+            "u/test-user".to_string(),
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+            false,
+            false,
+            None,
+            true,
+            None,
+            None,
+            None,
+            None,
+            None,
+            false,
+            None,
+            None,
+            None,
+            None,
+        )
+        .await?;
+        tx.commit().await?;
+        let (args, entrypoint) = job_args_and_entrypoint(&db, id).await?;
+        assert_eq!(
+            entrypoint.as_deref(),
+            Some("apply"),
+            "an authenticated caller running a script as itself may still pick the entrypoint \
+             (sent as {key:?})"
+        );
+        assert_eq!(
+            args,
+            json!({ "_ENTRYPOINT_OVERRIDE": "apply", "action": "refund" }),
+            "the stored name is the normalized one (sent as {key:?})"
+        );
+    }
+    Ok(())
+}
+
+/// Create a public (anonymous) app whose single component is the deployed script, with the
+/// given policy inputs, and return its path.
+async fn create_public_app(
+    ws: &str,
+    path: &str,
+    static_inputs: Value,
+    one_of_inputs: Value,
+) -> anyhow::Result<()> {
+    let resp = authed(client().post(format!("{ws}/apps/create")), "SECRET_TOKEN")
+        .json(&json!({
+            "path": path,
+            "summary": "",
+            "value": {},
+            "policy": {
+                "execution_mode": "anonymous",
+                "triggerables_v2": {
+                    format!("comp:script/{SCRIPT_PATH}"): {
+                        "static_inputs": static_inputs,
+                        "one_of_inputs": one_of_inputs,
+                    }
+                }
+            }
+        }))
+        .send()
+        .await?;
+    assert_eq!(resp.status(), 201, "create app: {}", resp.text().await?);
+    Ok(())
+}
+
+/// A viewer of a public app runs the publisher's pinned script as the publisher: it may
+/// fill in the inputs the policy leaves free and nothing else.
+#[sqlx::test(fixtures("base"))]
+async fn public_app_viewer_cannot_steer_the_pinned_runnable(
+    db: Pool<Postgres>,
+) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    let server = ApiServer::start(db.clone()).await?;
+    let ws = format!(
+        "http://localhost:{}/api/w/test-workspace",
+        server.addr.port()
+    );
+    deploy_script(&db).await?;
+
+    let app_path = "u/test-user/reserved_args";
+    create_public_app(
+        &ws,
+        app_path,
+        json!({ "tenant": "acme" }),
+        json!({ "greeting": ["Hello,", "Bonjour,"] }),
+    )
+    .await?;
+
+    // No `Authorization` header at all: the anonymous visitor of a public app.
+    let run = |args: &'static str| {
+        client()
+            .post(format!("{ws}/apps_u/execute_component/{app_path}"))
+            .header("content-type", "application/json")
+            .body(format!(
+                r#"{{"component": "comp", "path": "script/{SCRIPT_PATH}", "args": {args}}}"#
+            ))
+            .send()
+    };
+
+    // A name spelled with a NUL is the same name once stored, so the policy's pinned value
+    // must still win and the one-of list must still be enforced.
+    let resp = run(r#"{"tenant\u0000": "globex", "greeting": "Hello,"}"#).await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(status, 200, "run must be accepted: {body}");
+    let (args, _) = job_args_and_entrypoint(&db, uuid::Uuid::parse_str(body.trim())?).await?;
+    assert_eq!(
+        args["tenant"],
+        json!("acme"),
+        "a NUL-renamed arg must not override an input the policy pins: {args}"
+    );
+
+    let resp = run(r#"{"greeting\u0000": "anything"}"#).await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(
+        status, 400,
+        "a NUL-renamed arg must still be held to the policy's one_of list: {body}"
+    );
+    assert!(
+        body.contains("must be one of"),
+        "expected the one_of rejection, got: {body}"
+    );
+
+    // The control keys, in both spellings, in one request.
+    let resp = run(
+        r#"{"_ENTRYPOINT_OVERRIDE": "apply", "_ENTRYPOINT_OVERRIDE\u0000": "apply",
+            "_MODULES": {"bunfig.toml": {"content": "x", "language": "bun"}},
+            "_MODULES\u0000": {"bunfig.toml": {"content": "x", "language": "bun"}},
+            "_TEMP_SCRIPT_REFS": {"f/system/dep": "deadbeef"},
+            "_TEMP_SCRIPT_REFS\u0000": {"f/system/dep": "deadbeef"},
+            "greeting": "Hello,"}"#,
+    )
+    .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(status, 200, "run must be accepted: {body}");
+    let (args, entrypoint) =
+        job_args_and_entrypoint(&db, uuid::Uuid::parse_str(body.trim())?).await?;
+    assert_eq!(
+        entrypoint, None,
+        "a viewer must not pick the function the worker calls instead of `main`"
+    );
+    assert_eq!(
+        args,
+        json!({ "tenant": "acme", "greeting": "Hello," }),
+        "only the policy's pinned input and the viewer's allowed one may reach the job: {args}"
+    );
+    Ok(())
+}
+
+/// A deployed flow's dynamic-select code has no deployed runnable, so it is queued as a
+/// `JobKind::Preview` job — the one kind on which the bun and python executors honor
+/// `_TEMP_SCRIPT_REFS` and resolve the code's workspace imports from the temporary script
+/// store, which any workspace member can write. The caller of this route only fills in that
+/// code's inputs (it may be an operator, who is refused `jobs/run/preview` outright), so the
+/// key must not reach the job, in either spelling.
+#[sqlx::test(fixtures("base"))]
+async fn flow_dynamic_select_strips_caller_control_args(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    let server = ApiServer::start(db.clone()).await?;
+    let ws = format!(
+        "http://localhost:{}/api/w/test-workspace",
+        server.addr.port()
+    );
+
+    // Someone else's flow: `test-user-2` below can run it, not edit it.
+    let flow_path = "u/test-user/dynselect_control_args";
+    let resp = authed(client().post(format!("{ws}/flows/create")), "SECRET_TOKEN")
+        .json(&json!({
+            "path": flow_path,
+            "summary": "",
+            "description": "",
+            "value": { "modules": [] },
+            "schema": {
+                "type": "object",
+                "properties": { "region": { "type": "object", "format": "dynselect-region" } },
+                "x-windmill-dyn-select-lang": "bun",
+                "x-windmill-dyn-select-code":
+                    "import { regions } from \"/f/system/regions\"\n\
+                     export async function region() { return regions() }\n"
+            }
+        }))
+        .send()
+        .await?;
+    assert_eq!(resp.status(), 201, "create flow: {}", resp.text().await?);
+
+    // Read-only for the caller below: running the dyn-select code is all it may do.
+    sqlx::query(
+        "UPDATE flow SET extra_perms = '{\"u/test-user-2\": false}'
+         WHERE path = $1 AND workspace_id = 'test-workspace'",
+    )
+    .bind(flow_path)
+    .execute(&db)
+    .await?;
+
+    let refs = json!({ "f/system/regions": "deadbeef" });
+    let modules = json!({ "regions.ts": { "content": "export const v = 1", "language": "bun" } });
+    let resp = authed(
+        client().post(format!("{ws}/jobs/run/dynamic_select")),
+        "SECRET_TOKEN_2",
+    )
+    .json(&json!({
+        "entrypoint_function": "region",
+        "runnable_ref": { "source": "deployed", "path": flow_path, "runnable_kind": "flow" },
+        "args": {
+            "_TEMP_SCRIPT_REFS": refs,
+            "_TEMP_SCRIPT_REFS\u{0}": refs,
+            "_MODULES": modules,
+            "_MODULES\u{0}": modules,
+            "filterText": "eu"
+        }
+    }))
+    .send()
+    .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(status, 201, "dynamic select must be accepted: {body}");
+
+    let id = uuid::Uuid::parse_str(body.trim())?;
+    let kind: String = sqlx::query_scalar("SELECT kind::text FROM v2_job WHERE id = $1")
+        .bind(id)
+        .fetch_one(&db)
+        .await?;
+    assert_eq!(
+        kind, "preview",
+        "the premise of this test: a deployed flow's dyn-select code runs as a preview job"
+    );
+    let (args, _) = job_args_and_entrypoint(&db, id).await?;
+    assert_eq!(
+        args,
+        json!({ "_ENTRYPOINT_OVERRIDE": "region", "filterText": "eu" }),
+        "only the caller's inputs and the server-set entrypoint may reach the job"
+    );
+    Ok(())
+}

backend/windmill-api/src/apps.rs

@@ -66,8 +66,8 @@ use windmill_common::{
     db::{DbWithOptAuthed, UserDB},
     error::{to_anyhow, Error, JsonResult, Result},
     jobs::{
-        get_payload_tag_from_prefixed_path, resolve_delete_after_secs, schedule_job_deletion,
-        JobPayload, JobTriggerKind, RawCode,
+        get_payload_tag_from_prefixed_path, normalize_nul_in_args, resolve_delete_after_secs,
+        sanitize_untrusted_args, schedule_job_deletion, JobPayload, JobTriggerKind, RawCode,
     },
     scripts::ScriptLang,
     triggers::TriggerMetadata,
@@ -4963,11 +4963,15 @@ async fn execute_component(
     let resolved_delete_secs =
         resolve_delete_after_secs(None, policy_triggerables.delete_after_secs);
 
-    // `_TEMP_SCRIPT_REFS` is a server-injected control key (into `extra`) that the
-    // worker reads back for a `Preview` job, which an inline run is. A caller
-    // supplying it in `args` would redirect relative-import resolution, unpinned, as
-    // the app identity. Drop it; the legitimate value rides in `extra`.
-    payload.args.remove("_TEMP_SCRIPT_REFS");
+    // The caller here is a viewer of the app, down to an anonymous visitor of a public one,
+    // and the job runs as the app's identity on code the policy pins. So the viewer fills in
+    // the runnable's inputs and nothing else: not the function called instead of `main`
+    // (which would reach helpers the app never exposes, as the publisher), not the module
+    // files written into the job directory, and not `_TEMP_SCRIPT_REFS`, which would
+    // redirect relative-import resolution, unpinned, as the app identity. The legitimate
+    // `_TEMP_SCRIPT_REFS` is server-injected into `extra` further below, which this leaves
+    // alone — as it does the policy's own `static_inputs`, which the publisher writes.
+    sanitize_untrusted_args(&mut payload.args);
 
     // An agent runnable takes the message and its files, plus optionally the conversation they
     // belong to, which names the agent's memory rather than an input of its run. Without it the
@@ -6640,6 +6644,16 @@ async fn build_args(
     let mut job_id: Option<Uuid> = None;
     let mut safe_args = HashMap::<String, Box<RawValue>>::new();
 
+    // Every check below matches an argument by name against the policy the publisher wrote,
+    // and `push` stores the args with their `\u0000` escapes removed, names included. So
+    // compare the names that will actually be stored: otherwise a viewer's `tenant\0` is
+    // never recognized as the `tenant` the policy pins in `static_inputs` (and is stored as
+    // `tenant`, winning over the pinned value), and `greeting\0` is never recognized as the
+    // `greeting` restricted by `one_of_inputs`, so neither list is enforced. Values are
+    // normalized along with the names, so the checks that look at a value see what the job
+    // will receive.
+    normalize_nul_in_args(&mut args);
+
     // Before anything reads `args`: a context input the runnable declares is never taken
     // from the request, or a viewer could name someone else as themselves.
     for (k, prop) in ctx_inputs.into_iter().flatten() {

backend/windmill-api/src/jobs.rs

@@ -36,7 +36,7 @@ use windmill_common::error::JsonResult;
 use windmill_common::flow_status::{JobResult, RestartedFrom};
 use windmill_common::jobs::{
     format_completed_job_result, format_result, is_safe_log_file_path, is_valid_entrypoint_name,
-    DynamicInput, ENTRYPOINT_OVERRIDE,
+    sanitize_untrusted_args, DynamicInput, ENTRYPOINT_OVERRIDE,
 };
 #[cfg(feature = "run_inline")]
 use windmill_common::jobs::{
@@ -10488,7 +10488,7 @@ async fn run_dynamic_select(
     Extension(user_db): Extension<UserDB>,
     Path(w_id): Path<String>,
     Query(run_query): Query<RunJobQuery>,
-    Json(request): Json<DynamicSelectRequest>,
+    Json(mut request): Json<DynamicSelectRequest>,
 ) -> error::Result<Response> {
     #[cfg(feature = "enterprise")]
     check_license_key_valid().await?;
@@ -10510,6 +10510,20 @@ async fn run_dynamic_select(
         )));
     }
 
+    // The dyn-select code is the runnable's, not necessarily the caller's: a deployed flow
+    // has no deployed runnable for it, so it runs as a `JobKind::Preview` job, and the
+    // executors honor `_TEMP_SCRIPT_REFS` on every preview — it resolves the code's
+    // workspace imports from the temporary script store, which any workspace member can
+    // write. A caller of this route (an operator, for a deployed flow) only fills in that
+    // code's inputs; the function to call is taken from `entrypoint_function`, validated
+    // above and inserted server-side below, never from the args. So the reserved control
+    // keys are dropped here, in the spellings `push` would store them under. Nothing
+    // server-side puts them in `args`: the request has no `temp_script_refs` field, and the
+    // run form does not send them.
+    if let Some(args) = request.args.as_mut() {
+        sanitize_untrusted_args(args);
+    }
+
     // Deployed scripts keep their normal deployed-run path (a `script` job, resolved below by
     // push_script_job_by_path_into_queue). Deployed flows and inline snippets have no deployed
     // runnable, so they run their dyn-select code as a `preview`; the flow carries its path and

backend/windmill-common/src/jobs.rs

@@ -16,12 +16,77 @@ use crate::{
     flows::get_full_hub_flow_by_path,
     get_latest_deployed_hash_for_path, get_latest_flow_version_info_for_path,
     scripts::{get_full_hub_script_by_path, ScriptHash, ScriptLang},
-    utils::{StripPath, HTTP_CLIENT},
+    utils::{strip_json_nul, StripPath, HTTP_CLIENT},
     worker::{custom_tag_matches, to_raw_value, CUSTOM_TAGS_PER_WORKSPACE, WINDMILL_DIR},
     workspaces::workspace_with_fork_ancestors,
     FlowVersionInfo, ScriptHashInfo, Tag,
 };
 
+/// Whether `args` already reads the way `push` will store it, i.e. holds no NUL escape in a
+/// name or a value. Cheap (one SIMD substring scan per value) so the normalization below can
+/// be a no-op on the overwhelmingly common clean case.
+pub fn args_contain_nul(args: &HashMap<String, Box<RawValue>>) -> bool {
+    args.iter().any(|(k, v)| {
+        k.contains('\0') || memchr::memmem::find(v.get().as_bytes(), b"\\u0000").is_some()
+    })
+}
+
+/// Rewrite `args` the way `push` stores it: `push` runs [`strip_json_nul`] over the
+/// serialized args just before the INSERT, which removes `\u0000` escapes from **names as
+/// well as values** (jsonb rejects them with 22P05). Any decision taken on a name before
+/// that — `push`'s own removal of a caller [`MODULES_ARG`], an app policy's `static_inputs`
+/// and `one_of_inputs`, the `PushArgs` serializer's "`extra` wins this name" rule — would
+/// otherwise compare a name nobody knows against the name that actually lands in the job:
+/// an arg sent as `_MODULES\0` is stored as `_MODULES`, and a viewer's `tenant\0` is stored
+/// as `tenant`, after the publisher's pinned value, which jsonb then discards as the earlier
+/// of two equal keys.
+///
+/// Normalizing first makes every such check see the stored name. Values are normalized too,
+/// so a check on a value sees what the job will receive.
+///
+/// Two names that collide once normalized keep a single value, deterministically: the
+/// spelling without NULs wins, then the lexicographically smallest original name — never the
+/// one that `HashMap` iteration happened to serialize last.
+pub fn normalize_nul_in_args(args: &mut HashMap<String, Box<RawValue>>) {
+    if !args_contain_nul(args) {
+        return;
+    }
+    let mut entries = args.drain().collect::<Vec<_>>();
+    entries.sort_by(|(a, _), (b, _)| {
+        a.contains('\0')
+            .cmp(&b.contains('\0'))
+            .then_with(|| a.cmp(b))
+    });
+    for (key, value) in entries {
+        let key = if key.contains('\0') {
+            key.replace('\0', "")
+        } else {
+            key
+        };
+        let stripped = match strip_json_nul(value.get()) {
+            std::borrow::Cow::Owned(stripped) => RawValue::from_string(stripped).ok(),
+            std::borrow::Cow::Borrowed(_) => None,
+        };
+        args.entry(key).or_insert(stripped.unwrap_or(value));
+    }
+}
+
+/// Drop every [`RESERVED_CONTROL_ARGS`] key from args that a third party supplied for a job
+/// running under somebody else's identity (an HTTP route body or native trigger message run
+/// as the trigger's owner, a public app viewer's inputs run as the app's publisher). Those
+/// keys pick the function the worker calls instead of `main`, the module files written into
+/// the job directory, and where workspace imports resolve from: the third party must not
+/// choose any of them, while a caller running its own code under its own identity still may.
+///
+/// Normalizes first, so a key spelled `_ENTRYPOINT_OVERRIDE\0` — which `push` stores as
+/// `_ENTRYPOINT_OVERRIDE` — cannot slip past the comparison.
+pub fn sanitize_untrusted_args(args: &mut HashMap<String, Box<RawValue>>) {
+    normalize_nul_in_args(args);
+    for key in RESERVED_CONTROL_ARGS {
+        args.remove(key);
+    }
+}
+
 pub fn get_has_preprocessor_from_content_and_lang(
     content: &str,
     language: &ScriptLang,
@@ -633,6 +698,97 @@ pub async fn delete_jobs(conn: &mut sqlx::PgConnection, ids: &[uuid::Uuid]) -> e
     Ok(())
 }
 
+#[cfg(test)]
+mod reserved_args_tests {
+    use super::{normalize_nul_in_args, sanitize_untrusted_args};
+    use serde_json::value::RawValue;
+    use std::collections::HashMap;
+
+    fn raw(json: &str) -> Box<RawValue> {
+        RawValue::from_string(json.to_string()).unwrap()
+    }
+
+    fn args(entries: &[(&str, &str)]) -> HashMap<String, Box<RawValue>> {
+        entries
+            .iter()
+            .map(|(k, v)| (k.to_string(), raw(v)))
+            .collect()
+    }
+
+    fn as_json(args: &HashMap<String, Box<RawValue>>) -> serde_json::Value {
+        serde_json::to_value(args).unwrap()
+    }
+
+    /// A clean map must come out byte for byte the same: this runs on every push.
+    #[test]
+    fn normalization_leaves_clean_args_alone() {
+        let mut map = args(&[
+            ("tenant", r#""acme""#),
+            // A legitimately escaped backslash followed by the literal text `u0000`
+            // (minified JS regexes contain these) is not a NUL and must survive.
+            ("src", r#""[^\\u0000]+""#),
+        ]);
+        let before = as_json(&map);
+        normalize_nul_in_args(&mut map);
+        assert_eq!(as_json(&map), before);
+    }
+
+    /// `push` stores the args with their NUL escapes removed, names included, so a name
+    /// that only differs by NULs is the same name and must be compared as such.
+    #[test]
+    fn normalization_renames_nul_spelled_names_and_cleans_values() {
+        let mut map = args(&[
+            ("_MODULES\u{0}", r#"{"m.py": "evil"}"#),
+            ("_MOD\u{0}ULES", r#"{"m.py": "evil"}"#),
+            ("greeting\u{0}", r#""anything""#),
+            ("note", r#""a\u0000b""#),
+        ]);
+        normalize_nul_in_args(&mut map);
+        let mut names = map.keys().cloned().collect::<Vec<_>>();
+        names.sort();
+        assert_eq!(names, vec!["_MODULES", "greeting", "note"]);
+        assert_eq!(map["note"].get(), r#""ab""#);
+    }
+
+    /// Which of two names that collide once normalized survives may not depend on
+    /// `HashMap` iteration order: the spelling the publisher/server used wins.
+    #[test]
+    fn normalization_keeps_the_nul_free_spelling_on_collision() {
+        for _ in 0..16 {
+            let mut map = args(&[
+                ("tenant", r#""acme""#),
+                ("tenant\u{0}", r#""globex""#),
+                ("\u{0}tenant", r#""initech""#),
+            ]);
+            normalize_nul_in_args(&mut map);
+            assert_eq!(map.len(), 1);
+            assert_eq!(map["tenant"].get(), r#""acme""#);
+        }
+    }
+
+    /// Every reserved control key goes, in every spelling, and nothing else does.
+    #[test]
+    fn untrusted_args_lose_every_reserved_control_key() {
+        let mut map = args(&[
+            ("_ENTRYPOINT_OVERRIDE", r#""apply""#),
+            ("_ENTRYPOINT_OVERRIDE\u{0}", r#""apply""#),
+            ("_ENTRY\u{0}POINT_OVERRIDE", r#""apply""#),
+            ("_MODULES", r#"{"m.py": "evil"}"#),
+            ("_MODULES\u{0}", r#"{"m.py": "evil"}"#),
+            ("_TEMP_SCRIPT_REFS", r#"{"f/a/b": "deadbeef"}"#),
+            ("_TEMP_SCRIPT_REFS\u{0}", r#"{"f/a/b": "deadbeef"}"#),
+            // Neighbours that must not be caught by a prefix/contains match.
+            ("_TEMP_SCRIPT_REFS_", r#"1"#),
+            ("entrypoint", r#""main""#),
+            ("action", r#""refund""#),
+        ]);
+        sanitize_untrusted_args(&mut map);
+        let mut names = map.keys().cloned().collect::<Vec<_>>();
+        names.sort();
+        assert_eq!(names, vec!["_TEMP_SCRIPT_REFS_", "action", "entrypoint"]);
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::is_safe_log_file_path;

backend/windmill-queue/src/jobs.rs

@@ -42,8 +42,8 @@ use windmill_common::auth::JobPerms;
 #[cfg(feature = "benchmark")]
 use windmill_common::bench::BenchmarkIter;
 use windmill_common::jobs::{
-    script_path_to_payload, JobTriggerKind, TriggerKindLabel, EMAIL_ERROR_HANDLER_USER_EMAIL,
-    MODULES_ARG,
+    args_contain_nul, normalize_nul_in_args, sanitize_untrusted_args, script_path_to_payload,
+    JobTriggerKind, TriggerKindLabel, EMAIL_ERROR_HANDLER_USER_EMAIL, MODULES_ARG,
 };
 use windmill_common::min_version::{
     MIN_VERSION_SUPPORTS_DEBOUNCING, MIN_VERSION_SUPPORTS_DEBOUNCING_V2,
@@ -6153,6 +6153,18 @@ impl PushArgsOwned {
     pub fn empty() -> Self {
         PushArgsOwned { extra: None, args: HashMap::new() }
     }
+
+    /// Mark these args as coming from an untrusted third party: see
+    /// [`windmill_common::jobs::sanitize_untrusted_args`]. Applied to both maps, because a
+    /// name in `extra` wins over the same name in `args` when the job is serialized, and no
+    /// source that builds args from a third party's payload sets a reserved control key of
+    /// its own.
+    pub fn sanitize_untrusted(&mut self) {
+        sanitize_untrusted_args(&mut self.args);
+        if let Some(extra) = self.extra.as_mut() {
+            sanitize_untrusted_args(extra);
+        }
+    }
 }
 
 pub fn empty_result() -> Box<RawValue> {
@@ -6398,21 +6410,34 @@ async fn push_inner<'c, 'd>(
     suspended_mode: Option<bool>,
     scope_ceiling: Option<&[String]>,
 ) -> Result<(Uuid, Transaction<'c, Postgres>), Error> {
-    // The worker builds a preview's `_MODULES` arg into the job as its module code. Every
-    // caller-reachable value lands in `args` or `extra` (webhook query and headers go to
-    // `extra`, WAC children copy their parent's args), so it is dropped from both and only
-    // the `JobPayload::Code` arm below sets it, from the server-side `RawCode::modules`.
-    let args_without_modules;
+    // Normalize the names and values to what the INSERT below will actually store before
+    // anything here goes by a name: `strip_json_nul` runs over the serialized args just
+    // before that INSERT and removes `\u0000` escapes from keys as well as values, so a
+    // name compared as sent is not the name that lands in the job. Without this, `_MODULES`
+    // is dropped below while `_MODULES\0` is stored as `_MODULES` and built into a preview's
+    // job directory as module code; likewise the serializer's "`extra` wins this name" rule
+    // (which is how an app policy's `static_inputs` override a viewer's args) compares the
+    // name as sent, so `tenant\0` is written after the pinned `tenant` and jsonb keeps the
+    // viewer's, as the later of two equal keys. The final strip stays, harmless once this
+    // has run.
+    let normalized_args;
     let mut args = {
         let mut extra = args.extra;
         if let Some(extra) = extra.as_mut() {
+            normalize_nul_in_args(extra);
+            // The worker builds a preview's `_MODULES` arg into the job as its module code.
+            // Every caller-reachable value lands in `args` or `extra` (webhook query and
+            // headers go to `extra`, WAC children copy their parent's args), so it is
+            // dropped from both and only the `JobPayload::Code` arm below sets it, from the
+            // server-side `RawCode::modules`.
             extra.remove(MODULES_ARG);
         }
-        let args = if args.args.contains_key(MODULES_ARG) {
+        let args = if args_contain_nul(args.args) || args.args.contains_key(MODULES_ARG) {
             let mut stripped = args.args.clone();
+            normalize_nul_in_args(&mut stripped);
             stripped.remove(MODULES_ARG);
-            args_without_modules = stripped;
-            &args_without_modules
+            normalized_args = stripped;
+            &normalized_args
         } else {
             args.args
         };

backend/windmill-trigger/src/trigger_helpers.rs

@@ -512,7 +512,7 @@ pub async fn trigger_runnable_inner<'c>(
     workspace_id: &str,
     runnable_path: &str,
     is_flow: bool,
-    args: PushArgsOwned,
+    mut args: PushArgsOwned,
     retry: Option<&sqlx::types::Json<Retry>>,
     error_handler_path: Option<&str>,
     error_handler_args: Option<&sqlx::types::Json<HashMap<String, serde_json::Value>>>,
@@ -527,6 +527,18 @@ pub async fn trigger_runnable_inner<'c>(
     bool,
     Option<sqlx::Transaction<'c, sqlx::Postgres>>,
 )> {
+    // A trigger's args are built from a third party's payload — for an HTTP route without
+    // preprocessor the request body's keys *are* the job's top-level args — and the job runs
+    // as the trigger's owner, not as whoever sent that payload. So the sender may not steer
+    // how the runnable runs: pick the function called instead of `main` (and skip whatever
+    // `main` authenticates, e.g. the signature check of a `custom_script` route), plant
+    // module files in the job directory, or redirect where its workspace imports resolve.
+    // Every trigger kind funnels through here on its way to `push`, and none of them sets
+    // one of those keys itself, so this is the single place to do it rather than each args
+    // builder. It does not restrict what the runnable is *given*: ordinary inputs pass
+    // through untouched.
+    args.sanitize_untrusted();
+
     let error_handler_args = error_handler_args.map(|args| {
         let args = args
             .0

backend/windmill-types/src/jobs.rs

@@ -724,6 +724,26 @@ pub const ENTRYPOINT_OVERRIDE: &str = "_ENTRYPOINT_OVERRIDE";
 /// `RawCode::modules`: a caller-supplied one is dropped there.
 pub const MODULES_ARG: &str = "_MODULES";
 
+/// Job-arg key mapping a runnable's workspace imports to contents of the temporary script
+/// store (`scripts/raw_temp/store`, writable by any workspace member). The preview routes
+/// set it server-side from their `temp_script_refs` field; the bun and python executors
+/// honor it on every `JobKind::Preview` job.
+pub const TEMP_SCRIPT_REFS_ARG: &str = "_TEMP_SCRIPT_REFS";
+
+/// Job-arg keys that steer *how* a runnable runs rather than what it is given: which
+/// function the worker calls instead of `main` ([`ENTRYPOINT_OVERRIDE`]), what module files
+/// are written into the job directory ([`MODULES_ARG`]) and where its workspace imports
+/// resolve from ([`TEMP_SCRIPT_REFS_ARG`]).
+///
+/// Each is legitimate for a caller that runs code it authored under its own identity (the
+/// script editor's preprocessor test, dynamic-select helpers, the CLI, workflow-as-code), so
+/// they are not dropped for every job. None of them may come from a third party whose
+/// payload runs under somebody else's identity — an HTTP route body, a native trigger
+/// message, a public app viewer's inputs — which is what `sanitize_untrusted_args` in
+/// `windmill_common::jobs` drops.
+pub const RESERVED_CONTROL_ARGS: [&str; 3] =
+    [ENTRYPOINT_OVERRIDE, MODULES_ARG, TEMP_SCRIPT_REFS_ARG];
+
 /// Reserved job-arg key holding the inbound W3C `traceparent` captured from the
 /// request that enqueued the job (run endpoints). It rides the `args` jsonb like
 /// [`ENTRYPOINT_OVERRIDE`]; normal scripts never see it because args are bound by