From d0aaba0f167dc1b4603719884f8048af96ba77bf Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Sun, 14 Jun 2026 23:48:03 +0200 Subject: [PATCH] require pinned sha for inline raw_code in viewer app run mode (#9570) * fix: require pinned sha for inline raw_code in viewer app run mode * fix: gate rd_string import behind parquet feature to fix oss build * fix: also require pin for raw_code with app_script id in viewer run mode --- backend/tests/app_preview_auth.rs | 92 ++++++++++++++++++++- backend/tests/fixtures/app_preview_auth.sql | 8 ++ backend/windmill-api/src/apps.rs | 19 ++++- backend/windmill-object-store/src/lib.rs | 1 + 4 files changed, 118 insertions(+), 2 deletions(-) diff --git a/backend/tests/app_preview_auth.rs b/backend/tests/app_preview_auth.rs index 4653303392..2a69004a7b 100644 --- a/backend/tests/app_preview_auth.rs +++ b/backend/tests/app_preview_auth.rs @@ -17,7 +17,12 @@ //! must not over-block the legitimate editor flow), //! - preview is confined to paths the caller can read (defense-in-depth //! against scoped tokens / cross-namespace preview), and -//! - run mode (no `force_viewer_static_fields`) is unaffected by the guard. +//! - run mode (no `force_viewer_static_fields`) is unaffected by the preview +//! guard, and +//! - run mode against a deployed Viewer app rejects caller-supplied inline +//! `raw_code` whose sha is not publisher-pinned (CVE-2026-22683 residual: +//! the Viewer default-triggerable fallback let any caller / an operator run +//! arbitrary code as themselves, bypassing the content-hash pin). use serde_json::json; use sqlx::{Pool, Postgres}; @@ -254,5 +259,90 @@ async fn test_app_preview_authorization(db: Pool) -> anyhow::Result<() "rejection must be the jobs:run scope gate, got: {body}" ); + // 9. RUN-MODE REGRESSION (CVE-2026-22683 residual): in run mode (no + // `force_viewer_static_fields`) against a deployed Viewer-mode app, + // caller-supplied inline `raw_code` whose `rawscript/` is not pinned + // in the app's `triggerables_v2` must be rejected by the policy — it must + // not resolve via the Viewer default triggerable and run as the caller + // (the same preview-class execution an operator is denied in step 1). + let run_mode_raw_code = json!({ + "args": {}, + "component": "comp", + "raw_code": { + "language": "bash", + "content": "id; echo RCE_$(whoami)", + "path": "x" + } + }); + let resp = authed( + client().post(format!("{base}/u/test-user/vapp")), + "OPERATOR_TOKEN", + ) + .json(&run_mode_raw_code) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status, 400, + "run-mode inline raw_code against a Viewer app must be rejected, not run as the caller (got {status}): {body}" + ); + assert!( + body.contains("forbidden by policy"), + "rejection must be the content-hash pin (unpinned rawscript), got: {body}" + ); + + // 10. The content-pin fix is not operator-specific: even a regular + // non-operator member (who could run their own code via + // `/jobs/run/preview`) must not be able to substitute unpinned code into + // someone else's deployed Viewer app — the deployed-app integrity break. + let resp = authed( + client().post(format!("{base}/u/test-user/vapp")), + "SECRET_TOKEN_2", + ) + .json(&run_mode_raw_code) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status, 400, + "run-mode unpinned raw_code must be rejected for any caller, not just operators (got {status}): {body}" + ); + + // 11. The pin requirement also covers `raw_code` carrying an `app_script` + // `id`. The id resolves to `rawscript/` of any app_script row + // by number (no app scoping), so without the pin a caller could run a + // script belonging to another app against this Viewer app. Here `999777` + // belongs to `u/test-user/private`, not to the targeted `vapp`, and its + // sha is absent from `vapp`'s empty `triggerables_v2` — it must be + // rejected, not fall back to the Viewer default. + let resp = authed( + client().post(format!("{base}/u/test-user/vapp")), + "OPERATOR_TOKEN", + ) + .json(&json!({ + "args": {}, + "component": "comp", + "id": 999777, + "raw_code": { + "language": "deno", + "content": "export function main() { return 1; }", + "path": "x" + } + })) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status, 400, + "run-mode raw_code with an unpinned app_script id must be rejected against a Viewer app (got {status}): {body}" + ); + assert!( + body.contains("forbidden by policy"), + "rejection must be the content-hash pin (unpinned app_script sha), got: {body}" + ); + Ok(()) } diff --git a/backend/tests/fixtures/app_preview_auth.sql b/backend/tests/fixtures/app_preview_auth.sql index 9fcda61c51..b94e08f3fd 100644 --- a/backend/tests/fixtures/app_preview_auth.sql +++ b/backend/tests/fixtures/app_preview_auth.sql @@ -32,3 +32,11 @@ INSERT INTO app (id, workspace_id, path, summary, policy, versions) VALUES (999002, 'test-workspace', 'u/test-user-2/ownapp', 'own app', '{}'::jsonb, '{}'); INSERT INTO app_script (id, app, hash, code, code_sha256) VALUES (999778, 999002, repeat('c', 64), 'export function main(){ return "ok" }', repeat('d', 64)); + +-- A deployed empty Viewer-mode app owned by `test-user` with NO runnables pinned +-- in `triggerables_v2`. Used to assert run mode rejects caller-supplied inline +-- `raw_code` whose sha is not publisher-pinned (the CVE-2026-22683 residual: +-- the Viewer default fallback let any caller / an operator run arbitrary code). +INSERT INTO app (id, workspace_id, path, summary, policy, versions) VALUES + (999003, 'test-workspace', 'u/test-user/vapp', 'empty viewer app', + '{"execution_mode": "viewer", "triggerables_v2": {}}'::jsonb, '{}'); diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index ed75c25c6f..4a245530e5 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -2333,6 +2333,15 @@ async fn execute_component( &policy }; + // Caller-supplied inline code (`raw_code`), with or without an + // `app_script` id. Its resolved `rawscript/` key must be present + // in the policy triggerables below — it must never resolve via the + // Viewer default fallback. Without `id` the caller supplies the code + // verbatim; with `id` it selects any `app_script` row by number (no + // app/workspace scoping), so both let a caller run code the publisher + // never pinned for this app. + let is_inline_raw_code = payload.raw_code.is_some(); + // Compute the path for the triggerables map: // - flow: `flow/` // - script: `script/` @@ -2370,7 +2379,15 @@ async fn execute_component( .get(path) // start with `path` in case we can avoid the next` format!`. .or_else(|| triggerables_v2.get(&format!("{}:{}", payload.component, &path))) .or(match policy.execution_mode { - ExecutionMode::Viewer => Some(&policy_triggerables_default), + // A Viewer app may invoke any deployed `script`/`flow` it + // references (resolved as the caller), but caller-supplied + // inline `raw_code` must match a publisher-pinned + // `rawscript/` entry — otherwise an unauthorized caller + // (e.g. an operator, barred from `/jobs/run/preview`) could + // run code the publisher never pinned for this app. + ExecutionMode::Viewer if !is_inline_raw_code => { + Some(&policy_triggerables_default) + } _ => None, }) .ok_or_else(|| Error::BadRequest(format!("Path {path} forbidden by policy")))?; diff --git a/backend/windmill-object-store/src/lib.rs b/backend/windmill-object-store/src/lib.rs index fe2d8ce347..4b1279609f 100644 --- a/backend/windmill-object-store/src/lib.rs +++ b/backend/windmill-object-store/src/lib.rs @@ -53,6 +53,7 @@ use tokio::task; use windmill_common::error::to_anyhow; #[cfg(feature = "parquet")] use windmill_common::jobs::is_safe_log_file_path; +#[cfg(feature = "parquet")] use windmill_common::utils::rd_string; #[cfg(all(feature = "parquet", feature = "private"))] pub mod job_s3_helpers_ee;