fix: enforce auth guards on app component preview execution (#9235)

* fix: enforce auth guards on app component preview execution Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: guard previewed runnable path and worker tag in app preview Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: validate app_script id ownership and keep root push isolation Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: scope app preview guards to operator check + referenced runnables Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: require jobs:run scope and tag check on app preview (apps:run escalation) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-08-18 16:02:10 +00:00 · 2026-05-19 17:40:55 +00:00
parent 01bad16c0c
commit 4b1bea8aed
4 changed files with 378 additions and 1 deletions
@@ -0,0 +1,23 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "SELECT a.path FROM app_script s JOIN app a ON a.id = s.app\n                 WHERE s.id = $1 AND a.workspace_id = $2",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "path",
+        "type_info": "Varchar"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Int8",
+        "Text"
+      ]
+    },
+    "nullable": [
+      false
+    ]
+  },
+  "hash": "8816bbe1ea9ea4f359e7a95857d349fa8bd7d23e4589b6936e0d000745cb3f34"
+}
@@ -0,0 +1,258 @@
+//! Regression test for the app component preview authorization bypass.
+//!
+//! `POST /api/w/:workspace/apps_u/execute_component/:path` runs in "preview"
+//! mode whenever the client supplies `force_viewer_static_fields`. In that
+//! mode it accepts request-supplied `raw_code` and enqueues it as a
+//! `Viewer`-mode job — i.e. it is the app-editor equivalent of
+//! `/jobs/run/preview`. The bug was that this branch did not re-apply the
+//! guards `/jobs/run/preview` enforces for arbitrary code execution, so an
+//! authenticated Operator (a run-only user who must not be able to create
+//! scripts/apps or run preview jobs) could enqueue arbitrary worker code with
+//! a single request, escaping the Operator restriction entirely.
+//!
+//! This test pins down:
+//!   - an Operator is rejected from preview mode (the core fix; pre-fix this
+//!     enqueued a job and returned 200),
+//!   - a regular non-operator member can still run an editor preview (the fix
+//!     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.
+
+use serde_json::json;
+use sqlx::{Pool, Postgres};
+use windmill_test_utils::*;
+
+fn client() -> reqwest::Client {
+    reqwest::Client::new()
+}
+
+fn authed(builder: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuilder {
+    builder.header("Authorization", format!("Bearer {}", token))
+}
+
+/// A preview request: `force_viewer_static_fields` present + inline `raw_code`.
+/// This is the exact shape an attacker (or the editor) sends.
+fn preview_body(app_path: &str) -> serde_json::Value {
+    json!({
+        "args": {},
+        "component": "comp",
+        "raw_code": {
+            "language": "deno",
+            "content": "export function main() { return \"pwned\"; }",
+            "path": format!("{}/comp", app_path)
+        },
+        "force_viewer_static_fields": {}
+    })
+}
+
+#[sqlx::test(fixtures("base", "app_preview_auth"))]
+async fn test_app_preview_authorization(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/test-workspace/apps_u/execute_component");
+
+    // 1. CORE REGRESSION: an Operator sends a preview request in their own
+    //    namespace (so the *only* thing that can reject them is the Operator
+    //    check itself). Pre-fix this returned 200 with an enqueued job UUID;
+    //    post-fix it must be rejected.
+    let resp = authed(
+        client().post(format!("{base}/u/operator-user/myapp")),
+        "OPERATOR_TOKEN",
+    )
+    .json(&preview_body("u/operator-user/myapp"))
+    .send()
+    .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(
+        status, 401,
+        "Operator must be rejected from app preview (got {status}): {body}"
+    );
+    assert!(
+        body.contains("Operators cannot run preview jobs"),
+        "rejection must be the operator guard, got: {body}"
+    );
+
+    // 2. The fix must NOT over-block the legitimate editor flow: a regular
+    //    non-operator member previewing in their own namespace still works
+    //    (the endpoint returns the enqueued job UUID before any worker runs).
+    let resp = authed(
+        client().post(format!("{base}/u/test-user-2/myapp")),
+        "SECRET_TOKEN_2",
+    )
+    .json(&preview_body("u/test-user-2/myapp"))
+    .send()
+    .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert!(
+        status.is_success(),
+        "non-operator editor preview must still succeed (got {status}): {body}"
+    );
+    assert!(
+        uuid::Uuid::parse_str(body.trim()).is_ok(),
+        "successful preview must return a job UUID, got: {body}"
+    );
+
+    // 3. Inline `raw_code` preview is deliberately NOT path-gated: a
+    //    non-operator can already run arbitrary inline code via
+    //    `/jobs/run/preview`, so the app URL path string is irrelevant for the
+    //    inline case. This pins that decision so an over-restrictive path check
+    //    is not re-added for inline previews.
+    let resp = authed(
+        client().post(format!("{base}/u/test-user/secretapp")),
+        "SECRET_TOKEN_2",
+    )
+    .json(&preview_body("u/test-user/secretapp"))
+    .send()
+    .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert!(
+        status.is_success(),
+        "inline raw_code preview must not be path-gated (got {status}): {body}"
+    );
+    assert!(
+        uuid::Uuid::parse_str(body.trim()).is_ok(),
+        "inline preview should enqueue a job UUID, got: {body}"
+    );
+
+    // 4. Run mode (no `force_viewer_static_fields`) is unaffected by the new
+    //    preview guard: an Operator hitting a deployed-app path still follows
+    //    the pre-existing policy lookup (here: the app does not exist -> 404),
+    //    proving the guard only gates preview mode.
+    let resp = authed(
+        client().post(format!("{base}/u/operator-user/nonexistent")),
+        "OPERATOR_TOKEN",
+    )
+    .json(&json!({
+        "args": {},
+        "component": "comp",
+        "raw_code": {
+            "language": "deno",
+            "content": "export function main() { return 1; }",
+            "path": "u/operator-user/nonexistent/comp"
+        }
+    }))
+    .send()
+    .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(
+        status, 404,
+        "run mode must be unchanged (deployed app lookup -> 404, not the preview guard); got {status}: {body}"
+    );
+
+    // 5. Defense-in-depth: the guard must check the *runnable* being previewed,
+    //    not just the app URL path. A caller pairs an allowed app path
+    //    (`u/test-user-2/myapp`, own namespace) with a `path` pointing at a
+    //    deployed runnable in another user's namespace. Without checking the
+    //    runnable path this would resolve `script/u/test-user/private` with the
+    //    root DB handle and enqueue it; it must be rejected by the path check.
+    let resp = authed(
+        client().post(format!("{base}/u/test-user-2/myapp")),
+        "SECRET_TOKEN_2",
+    )
+    .json(&json!({
+        "args": {},
+        "component": "comp",
+        "path": "script/u/test-user/private",
+        "force_viewer_static_fields": {}
+    }))
+    .send()
+    .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(
+        status, 400,
+        "preview targeting a runnable outside the caller's namespace must be rejected even with an allowed app path (got {status}): {body}"
+    );
+
+    // 6. Defense-in-depth: a persisted inline-script preview selects code by the
+    //    caller-controlled `app_script` id. Pairing an allowed app path with an
+    //    id owned by another (private) app must be rejected — without the
+    //    id-ownership check the worker would fetch and run that app's code.
+    let resp = authed(
+        client().post(format!("{base}/u/test-user-2/myapp")),
+        "SECRET_TOKEN_2",
+    )
+    .json(&json!({
+        "args": {},
+        "component": "comp",
+        "id": 999777,
+        "raw_code": {
+            "language": "deno",
+            "content": "export function main() { return 1; }",
+            "path": "u/test-user-2/myapp/comp"
+        },
+        "force_viewer_static_fields": {}
+    }))
+    .send()
+    .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(
+        status, 400,
+        "preview with an app_script id owned by another app must be rejected (got {status}): {body}"
+    );
+
+    // 7. The id-ownership check must NOT over-block a legitimate persisted
+    //    inline-script preview: an id owned by an app in the caller's own
+    //    namespace passes the guard and enqueues (returns a job UUID).
+    let resp = authed(
+        client().post(format!("{base}/u/test-user-2/ownapp")),
+        "SECRET_TOKEN_2",
+    )
+    .json(&json!({
+        "args": {},
+        "component": "comp",
+        "id": 999778,
+        "raw_code": {
+            "language": "deno",
+            "content": "export function main() { return 1; }",
+            "path": "u/test-user-2/ownapp/comp"
+        },
+        "force_viewer_static_fields": {}
+    }))
+    .send()
+    .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert!(
+        status.is_success(),
+        "persisted preview for an app the caller owns must still succeed (got {status}): {body}"
+    );
+    assert!(
+        uuid::Uuid::parse_str(body.trim()).is_ok(),
+        "successful persisted preview must return a job UUID, got: {body}"
+    );
+
+    // 8. Scope escalation: a token scoped to `apps:run` (but not `jobs:run`)
+    //    can reach this route (it maps to the `apps` scope domain) and is not an
+    //    Operator, but must NOT be able to enqueue arbitrary preview `raw_code`.
+    //    `/jobs/run/preview` requires `jobs:run` for exactly this reason; the
+    //    app preview path must enforce the same. Without the `jobs:run` check
+    //    this enqueues a job (returns a UUID); with it, it is rejected (403).
+    let resp = authed(
+        client().post(format!("{base}/u/test-user-2/myapp")),
+        "APPS_RUN_TOKEN",
+    )
+    .json(&preview_body("u/test-user-2/myapp"))
+    .send()
+    .await?;
+    let status = resp.status();
+    let body = resp.text().await?;
+    assert_eq!(
+        status, 403,
+        "apps:run-scoped token must not escalate to arbitrary preview code (got {status}): {body}"
+    );
+    assert!(
+        body.contains("jobs:run"),
+        "rejection must be the jobs:run scope gate, got: {body}"
+    );
+
+    Ok(())
+}
@@ -0,0 +1,34 @@
+-- Fixture for the app component preview authorization regression test.
+-- Layered on top of `base` (which provides test-workspace, the admin
+-- `test-user`/SECRET_TOKEN, and the non-operator `test-user-2`/SECRET_TOKEN_2).
+-- Adds an Operator member so we can assert that Operators cannot reach the
+-- arbitrary-code app preview path (`force_viewer_static_fields` + `raw_code`).
+
+INSERT INTO password(email, password_hash, login_type, super_admin, verified, name)
+    VALUES ('operator@windmill.dev', 'not-a-real-hash', 'password', false, true, 'Operator User');
+
+INSERT INTO usr(workspace_id, email, username, is_admin, operator, role) VALUES
+	('test-workspace', 'operator@windmill.dev', 'operator-user', false, true, 'Operator');
+
+INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin) VALUES
+	(encode(sha256('OPERATOR_TOKEN'::bytea), 'hex'), 'OPERATOR_T', 'OPERATOR_TOKEN', 'operator@windmill.dev', 'operator token', false);
+
+-- A non-operator token scoped to `apps:run` but NOT `jobs:run`. It can reach
+-- the `apps_u/execute_component` route (route maps to the `apps` scope domain)
+-- but must not be able to enqueue arbitrary preview `raw_code`.
+INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin, scopes) VALUES
+	(encode(sha256('APPS_RUN_TOKEN'::bytea), 'hex'), 'APPS_RUN_T', 'APPS_RUN_TOKEN', 'test2@windmill.dev', 'apps:run scoped token', false, '{apps:run}');
+
+-- A private app owned by `test-user` with a persisted inline script. Used to
+-- assert that `test-user-2` cannot preview-execute another app's app_script id.
+INSERT INTO app (id, workspace_id, path, summary, policy, versions) VALUES
+	(999001, 'test-workspace', 'u/test-user/private', 'private app', '{}'::jsonb, '{}');
+INSERT INTO app_script (id, app, hash, code, code_sha256) VALUES
+	(999777, 999001, repeat('a', 64), 'export function main(){ return "secret" }', repeat('b', 64));
+
+-- An app owned by `test-user-2` with its own persisted inline script, to assert
+-- the id-ownership check does not over-block a legitimate persisted preview.
+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));
@@ -11,7 +11,7 @@ use crate::{
    auth::{get_end_user_email, OptTokened},
    db::{ApiAuthed, DB},
    jobs::RunJobQuery,
-    users::{require_owner_of_path, OptAuthed},
+    users::{require_owner_of_path, require_path_read_access_for_preview, OptAuthed},
    utils::{check_scopes, WithStarredInfoQuery},
    webhook_util::{WebhookMessage, WebhookShared},
    HTTP_CLIENT,
@@ -2138,6 +2138,54 @@ async fn execute_component(
    // tag from the deployed policy and ignore the request body.
    let is_preview = payload.force_viewer_static_fields.is_some();

+    // Preview mode runs request-supplied code as a `Viewer`-mode job (the
+    // app-editor equivalent of `/jobs/run/preview`), so it enforces the same
+    // guards. Operators must never run preview jobs. `jobs:run` is required
+    // because this route is reachable with an `apps:run`-scoped token (the
+    // route maps to the `apps` scope domain), which must not be able to escalate
+    // to arbitrary code execution. The client-supplied inline `raw_code.tag`
+    // must stay within the caller's allowed worker tags. A preview can also
+    // *reference* an existing runnable the caller may not be allowed to read — a
+    // deployed script/flow via `payload.path` or a persisted `app_script` via
+    // `payload.id`, both resolved with the root DB handle — so those (and only
+    // those) are confined to paths the caller can read. Inline `raw_code` is not
+    // path-gated: a non-operator member can already run arbitrary inline code
+    // via `/jobs/run/preview`.
+    if is_preview {
+        let authed = opt_authed.as_ref().ok_or_else(|| {
+            Error::NotAuthorized("App component preview requires authentication".to_string())
+        })?;
+        if authed.is_operator {
+            return Err(Error::NotAuthorized(
+                "Operators cannot run preview jobs for security reasons".to_string(),
+            ));
+        }
+        check_scopes(authed, || format!("jobs:run"))?;
+        if let Some(p) = payload.path.as_deref() {
+            let runnable_path = p
+                .strip_prefix("script/")
+                .or_else(|| p.strip_prefix("flow/"))
+                .unwrap_or(p);
+            require_path_read_access_for_preview(authed, &Some(runnable_path.to_string()))?;
+        }
+        if let Some(id) = payload.id {
+            let owner_path = sqlx::query_scalar!(
+                "SELECT a.path FROM app_script s JOIN app a ON a.id = s.app
+                 WHERE s.id = $1 AND a.workspace_id = $2",
+                id,
+                &w_id,
+            )
+            .fetch_optional(&db)
+            .await?
+            .ok_or_else(|| {
+                Error::NotAuthorized(format!(
+                    "App script {id} does not belong to an app in this workspace"
+                ))
+            })?;
+            require_path_read_access_for_preview(authed, &Some(owner_path))?;
+        }
+    }
+
    // Two cases here:
    // 1. The component is executed from the editor (i.e. in "preview" mode), then:
    //    - The policy is set to default (in `Viewer` execution mode).
@@ -2341,6 +2389,20 @@ async fn execute_component(
        ),
        _ => unreachable!(),
    };
+    // Preview honors the client-supplied inline tag (`resolved_inline_tag`), so
+    // — like `/jobs/run/preview` — confine it to worker tags the caller may use
+    // (a `if_jobs:filter_tags`-restricted token must not escape its filter).
+    // `is_preview` implies an authed caller (the guard above returns otherwise).
+    if is_preview {
+        if let Some(authed) = opt_authed.as_ref() {
+            crate::jobs::check_tag_available_for_workspace(&db, &w_id, &tag, authed).await?;
+        }
+    }
+    // Identity is already resolved to the requesting user in preview mode (the
+    // policy is forced to `ExecutionMode::Viewer`, so the job runs as the
+    // caller). The enqueue stays root-isolated as before — switching the insert
+    // to user-RLS is not what contains the bypass (the auth guards above are)
+    // and would add unnecessary breakage risk to the legitimate editor flow.
    let tx = PushIsolationLevel::IsolatedRoot(db.clone());

    let (email, permissioned_as) = if let Some(on_behalf_of) = on_behalf_of.as_ref() {