fix: sandbox script-controlled content types in result_to_response (#10932)

* fix: sandbox script-controlled content type in result_to_response Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFhu2MHsJVMqdMfdgbwNkT * fix: reject hop-by-hop wm_headers so a proxy cannot strip the sandbox Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFhu2MHsJVMqdMfdgbwNkT * docs: condense sandbox comments and record the surface in the threat model Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFhu2MHsJVMqdMfdgbwNkT --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-11 16:09:39 +00:00 · 2026-09-02 17:27:18 +02:00
parent fdd3b36423
commit 419741e5d2
2 changed files with 88 additions and 2 deletions
@@ -85,7 +85,7 @@ published advisory history (73 GHSA advisories, several rated 9.9 critical).
 | EP9 Worker sandbox | nsjail / unshare / dind / rootless podman isolating user code | user code → host & cross-tenant filesystem/network | Worker host, isolation, downstream |
 | EP10 Worker code generation / wrappers | Entrypoint override, env-var names, workspace env interpolated into generated wrapper code | user-controlled identifier → executable code | Worker host, isolation |
 | EP11 OAuth / OIDC / SAML / MCP-OAuth / logout | Login callbacks, MCP OAuth client registration, logout `rd` redirect | untrusted IdP / redirect input → session | Session tokens, accounts |
-| EP12 Stored-content rendering | App builder HTML component, markdown, S3 download response headers | stored user content → admin browser (same origin) | Admin session, account takeover |
+| EP12 Stored-content rendering | App builder HTML component, markdown, S3 download response headers, script-controlled `wm_content_type`/`wm_headers` on `run_wait_result` and sync HTTP-route responses | stored user content → admin browser (same origin) | Admin session, account takeover |
 | EP13 Log/file reading & export endpoints | `service_logs`, `jobs_u/getupdate` log file read (symlinks), workspace/tarball export | authed/unauth request → arbitrary file or admin-only config | Arbitrary files, global settings |
 | EP14 Secret-value & resource-value caches | In-memory caches in `windmill-store` keyed (historically un-keyed) by path | cache lookup crossing identity/folder boundary | Secret variables, resource creds |
 | EP15 Deployment & runtime config | docker-compose defaults: dind, debugger (`REQUIRE_SIGNED_DEBUG_REQUESTS` now defaults to `true`; can still be overridden to `false`), CORS `Any`, default admin/`changeme`, exposed Postgres, `SUPERADMIN_SECRET`, `ENABLE_NSJAIL=false`, privileged containers | operator/infra default → full instance | All assets |
@@ -106,7 +106,7 @@ published advisory history (73 GHSA advisories, several rated 9.9 critical).
 | T8 | Unauthenticated RCE via the Debugger WebSocket: `/ws_debug/*` exposed by the gateway/ingress with the debugger service as the auth boundary; signature gate was bypassable via `program`-mode launches (read+exec an arbitrary server-side file path, never signed) even with signing on, and the WS handshake had no Origin check (CSWSH) | remote_unauth | EP15 | Worker host, all assets | critical | possible | partially_mitigated | `program`-mode launches now rejected when `REQUIRE_SIGNED_DEBUG_REQUESTS` is on (signing covers every launch, not just inline `code`); shipped `docker-compose` now defaults `REQUIRE_SIGNED_DEBUG_REQUESTS=true`; opt-in `DEBUG_ALLOWED_ORIGINS` allowlist rejects cross-origin handshakes. Residual: code default is secure but operators can still set `=false`; origin allowlist is opt-in | GHSA-725h-99vx-9xr4 |
 | T9 | Supply-chain compromise via cached hub scripts, GitHub workflow command injection, or vulnerable base-image deps | supply_chain | EP16 | Worker host, build integrity | critical | possible | partially_mitigated | hub-script re-pin to patched versions; HUB_BASE_URL override | GHSA-w2m9-q5f7-3gpq, edf340c4d4, GHSA-8rq7-w7g6-8wvr, GHSA-vch9-39v5-4wg7 (CVE-2024-37371) |
 | T10 | Unauthenticated disclosure of job results, args, logs, and admin config via missing-authz public endpoints | remote_unauth | EP2, EP13 | Job results/args/logs, global settings, scripts | high | likely | partially_mitigated | anonymous-job checks, log-endpoint authz hardening | GHSA-qfg7-x243-5hg4, GHSA-v448-fmm4-52fp, 108a88a180, bb90f4ce83 |
-| T11 | Stored XSS leading to admin/account takeover via app HTML component, markdown, or S3 download content-type | remote_auth | EP12 | Admin session, accounts | high | likely | partially_mitigated | DOMPurify markdown sanitization, `X-Content-Type-Options: nosniff` + CSP sandbox on downloads | GHSA-9c5c-hh3c-r9mc, GHSA-qxj7-hpx3-r892, GHSA-cf2x-rg8c-v63v, bb78b1c06d, 625b67dff0 |
+| T11 | Stored XSS leading to admin/account takeover via app HTML component, markdown, S3 download content-type, or a script-chosen `text/html` content type on `run_wait_result` / sync HTTP-route responses (GET-reachable with the `SameSite=Lax` session cookie) | remote_auth | EP12 | Admin session, accounts | high | likely | partially_mitigated | DOMPurify markdown sanitization, `X-Content-Type-Options: nosniff` + CSP sandbox on downloads and on every `result_to_response` composite result (inserted after `wm_headers`; hop-by-hop names such as `Connection` rejected so a proxy cannot strip them) | GHSA-9c5c-hh3c-r9mc, GHSA-qxj7-hpx3-r892, GHSA-cf2x-rg8c-v63v, bb78b1c06d, 625b67dff0, WIN-2471 |
 | T12 | Webhook authentication bypass / signature replay forges trigger invocations and approvals | remote_unauth | EP3 | Job execution integrity, approvals | high | likely | partially_mitigated | HMAC verification on some triggers; signing-oracle fix | GHSA-jw8c-h45c-xpjw, GHSA-hh9x-rcf8-xjr2, GHSA-q9g3-q6fj-hc2x, GHSA-8jc4-wj2p-2vmp, ab2a15b2a8 |
 | T13 | Path traversal / arbitrary file read via log-reading and MCP path endpoints (incl. symlink following) | remote_auth | EP13 | Arbitrary files on server, global settings | high | likely | partially_mitigated | traversal checks + no-symlink-follow added | GHSA-4hrf-mgvv-xp9x, bb90f4ce83, df451aa64f, ad5ec293b5, 5f2d3e6812 |
 | T14 | Privilege escalation via token rescope/refresh, script-issued JWTs, or operator-permission gaps | remote_auth | EP17, EP5 | Tokens, isolation, accounts | high | likely | partially_mitigated | monotonic-privilege enforcement on token lifecycle; SECURITY DEFINER triggers | GHSA-p62p-67xp-v775, GHSA-vv9w-wx3c-q3x2, 2ddf93de96, 865ab70c89, 33fb08cf3d |
@@ -416,11 +416,31 @@ pub fn result_to_response(result: Box<RawValue>, success: bool) -> error::Result

            let mut headers = HeaderMap::new();

+            // A reverse proxy consumes hop-by-hop headers instead of forwarding them and
+            // drops every header named by `Connection`, so a script could use one to strip
+            // the sandbox headers this function adds before they reach the browser.
+            const HOP_BY_HOP_HEADERS: [&str; 9] = [
+                "connection",
+                "keep-alive",
+                "proxy-authenticate",
+                "proxy-authorization",
+                "proxy-connection",
+                "te",
+                "trailer",
+                "transfer-encoding",
+                "upgrade",
+            ];
+
            if let Some(windmill_headers) = windmill_headers {
                for (k, v) in windmill_headers {
                    let k = HeaderName::from_str(k.as_str()).map_err(|err| {
                        Error::internal_err(format!("Invalid header name {k}: {err}"))
                    })?;
+                    if HOP_BY_HOP_HEADERS.contains(&k.as_str()) {
+                        return Err(Error::ExecutionErr(format!(
+                            "windmill_headers cannot set the hop-by-hop header \"{k}\""
+                        )));
+                    }
                    let v = HeaderValue::from_str(v.as_str()).map_err(|err| {
                        Error::internal_err(format!("Invalid header value {v}: {err}"))
                    })?;
@@ -428,6 +448,22 @@ pub fn result_to_response(result: Box<RawValue>, success: bool) -> error::Result
                }
            }

+            // The script controls the content type and body, and run_wait_result and sync
+            // HTTP routes are reachable by top-level GET navigation with the session cookie:
+            // sandbox the document into an opaque origin so HTML can never run with the
+            // viewer's session. Inserted after `wm_headers` so a script cannot override it.
+            headers.insert(
+                http::header::X_CONTENT_TYPE_OPTIONS,
+                HeaderValue::from_static("nosniff"),
+            );
+            headers.insert(
+                http::header::CONTENT_SECURITY_POLICY,
+                HeaderValue::from_static(
+                    "sandbox allow-scripts allow-forms allow-popups \
+                     allow-popups-to-escape-sandbox allow-downloads allow-modals",
+                ),
+            );
+
            if let Some(content_type) = windmill_content_type {
                let serialized_json_result = result_value
                    .map(|val| val.get().to_owned())
@@ -1104,6 +1140,56 @@ mod result_to_response_tests {
            resp.headers().get(http::header::CONTENT_TYPE).unwrap(),
            "text/html"
        );
+        assert_sandboxed(resp.headers());
        assert_eq!(body_bytes(resp).await, b"<h1>hi</h1>");
    }
+
+    fn assert_sandboxed(headers: &HeaderMap) {
+        assert_eq!(
+            headers.get(http::header::X_CONTENT_TYPE_OPTIONS).unwrap(),
+            "nosniff"
+        );
+        let csp = headers
+            .get(http::header::CONTENT_SECURITY_POLICY)
+            .expect("content-security-policy")
+            .to_str()
+            .unwrap();
+        assert!(csp.starts_with("sandbox "), "csp: {csp}");
+        assert!(!csp.contains("allow-same-origin"), "csp: {csp}");
+    }
+
+    #[tokio::test]
+    async fn custom_headers_cannot_override_sandbox() {
+        // wm_headers is script-controlled: a content-type set there replaces the JSON
+        // one even without wm_content_type, and the sandbox headers must survive an
+        // attempt to override them.
+        let resp = result_to_response(
+            raw(
+                r#"{"wm_headers":{"content-type":"text/html","content-security-policy":"default-src *","x-content-type-options":"none"},"result":"<h1>hi</h1>"}"#,
+            ),
+            true,
+        )
+        .expect("response");
+
+        assert_eq!(
+            resp.headers().get(http::header::CONTENT_TYPE).unwrap(),
+            "text/html"
+        );
+        assert_sandboxed(resp.headers());
+    }
+
+    #[tokio::test]
+    async fn hop_by_hop_custom_headers_are_rejected() {
+        // A proxy drops every header named by `Connection`, which would strip the
+        // sandbox headers on the way to the browser.
+        for name in ["connection", "Connection", "transfer-encoding", "upgrade"] {
+            let res = result_to_response(
+                raw(&format!(
+                    r#"{{"wm_content_type":"text/html","wm_headers":{{"{name}":"content-security-policy, x-content-type-options"}},"result":"<h1>hi</h1>"}}"#
+                )),
+                true,
+            );
+            assert!(res.is_err(), "hop-by-hop header must be rejected: {name}");
+        }
+    }
 }