diff --git a/backend/windmill-api-auth/src/scopes.rs b/backend/windmill-api-auth/src/scopes.rs index 191c7006e2..c0b2b581cc 100644 --- a/backend/windmill-api-auth/src/scopes.rs +++ b/backend/windmill-api-auth/src/scopes.rs @@ -470,17 +470,6 @@ pub fn check_route_access( // whole `/users`, `/folders` and `/jobs` routers are CORS-enabled for the // opaque app iframe, so default-deny everything in those domains except the // intended routes — otherwise the token could enumerate/export workspace data. - if has_raw_app_sdk_sentinel(Some(token_scopes)) { - if let Some(suffix) = route_suffix.as_deref() { - if is_request_supplied_code_route(suffix) { - return Err(Error::PermissionDenied( - "Access denied. A raw app frontend SDK token cannot run request-supplied code." - .to_string(), - )); - } - } - } - if has_app_embed_sentinel(Some(token_scopes)) { if let Some(suffix) = route_suffix.as_deref() { if app_embed_route_denied(required_domain, suffix) { @@ -500,6 +489,19 @@ pub fn check_route_access( } } + // Raw-app SDK tokens (sentinel) hold `jobs:run` only to run the deployed + // runnables the viewer already can — never request-supplied code. + if has_raw_app_sdk_sentinel(Some(token_scopes)) { + if let Some(suffix) = route_suffix.as_deref() { + if is_request_supplied_code_route(suffix) { + return Err(Error::PermissionDenied( + "Access denied. A raw app frontend SDK token cannot run request-supplied code." + .to_string(), + )); + } + } + } + // MCP scopes (mcp:all, mcp:favorites, mcp:hub:*, etc.) use a custom format // that doesn't fit the standard domain:action model. Verify the token has at // least one mcp: scope; MCP handlers do their own fine-grained checking. @@ -722,24 +724,26 @@ pub fn has_raw_app_sdk_sentinel(scopes: Option<&[String]>) -> bool { } /// Endpoints that execute code supplied in the request body. `jobs:run` reaches -/// them by design (they are the editor's preview surface), but a raw-app SDK -/// token is handed to untrusted app JS and can be exfiltrated off-origin, so for -/// it "run scripts and flows" must mean the deployed runnables the viewer can -/// already run — not arbitrary code. Without this, a captured SDK token escalates -/// to the viewer's full account: a preview job runs attacker-chosen code whose +/// them by design (the editor's preview and dependency-resolution surface), but a +/// raw-app SDK token is handed to untrusted app JS and can be exfiltrated +/// off-origin, so for it "run scripts and flows" must mean the deployed runnables +/// the viewer can already run — not arbitrary code. Without this, a captured SDK +/// token escalates to the viewer's full account: these jobs run attacker-chosen +/// code (or resolve and install attacker-chosen packages on a worker) and their /// own ephemeral credential is unscoped and permissioned as the viewer. -/// The SDK never calls these (its run helpers are all by path/hash). +/// The SDK never calls these — its run helpers are all by path/hash. fn is_request_supplied_code_route(suffix: &str) -> bool { - const CODE_ROUTES: [&str; 6] = [ + // Prefixes, so the `_async` variants are covered too. + const CODE_ROUTES: [&str; 8] = [ "jobs/run/preview", "jobs/run_inline/preview", "jobs/run_wait_result/preview", "jobs/run/preview_bundle", "jobs/run/preview_flow", "jobs/run_wait_result/preview_flow", + "jobs/run/dependencies", + "jobs/run/flow_dependencies", ]; - // `run/preview_bundle` / `preview_flow` also match the `run/preview` prefix; - // listing them is redundant but keeps the set readable as the route list. CODE_ROUTES.iter().any(|p| suffix.starts_with(p)) } diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 5ff436d8b8..6930e819eb 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -1239,6 +1239,7 @@ pub const FRONTEND_SDK_ALLOWED_SCOPES: [&str; 5] = [ /// Enforced on every policy write AND re-checked at mint time, so a policy /// written by an older/foreign client can't broaden what gets minted. fn validate_frontend_sdk_scopes_list(scopes: &[String]) -> Result<()> { + let mut seen = std::collections::HashSet::new(); for s in scopes { if !FRONTEND_SDK_ALLOWED_SCOPES.contains(&s.as_str()) { return Err(Error::BadRequest(format!( @@ -1247,6 +1248,14 @@ fn validate_frontend_sdk_scopes_list(scopes: &[String]) -> Result<()> { FRONTEND_SDK_ALLOWED_SCOPES.join(", ") ))); } + // The viewer's permission prompt renders these as a keyed list, which a + // repeat would break. The editor can't produce one; the API/CLI can. + if !seen.insert(s.as_str()) { + return Err(Error::BadRequest(format!( + "Duplicate frontend SDK scope '{}'", + s + ))); + } } Ok(()) } @@ -1325,10 +1334,16 @@ pub async fn build_embed_token_response( opt_authed: Option<&ApiAuthed>, sdk_consent: bool, ) -> Result { - // A sandboxed bundle lives on an opaque origin and can't use the token, so - // don't mint one — the backend, not the viewer, decides this (the editor - // shows the author the same rule). - let sdk_scopes = if raw_app && !policy.sandbox && !policy.frontend_sdk_scopes.is_empty() { + // Only advertise scopes where a token could actually be minted, so the viewer + // never shows a permission prompt that can grant nothing: a sandboxed bundle + // lives on an opaque origin and can't use the token, and an anonymous visitor + // has no identity to mint against. The backend, not the viewer, decides this + // (the editor shows the author the same sandbox rule). + let sdk_scopes = if raw_app + && !policy.sandbox + && opt_authed.is_some() + && !policy.frontend_sdk_scopes.is_empty() + { Some(policy.frontend_sdk_scopes.clone()) } else { None @@ -4953,6 +4968,12 @@ mod embed_token_tests { ("/api/w/test/jobs/run/preview_bundle", "POST"), ("/api/w/test/jobs/run/preview_flow", "POST"), ("/api/w/test/jobs/run_wait_result/preview_flow", "POST"), + // Dependency jobs are the same class: they resolve and install + // request-supplied imports on a worker. + ("/api/w/test/jobs/run/dependencies", "POST"), + ("/api/w/test/jobs/run/dependencies_async", "POST"), + ("/api/w/test/jobs/run/flow_dependencies", "POST"), + ("/api/w/test/jobs/run/flow_dependencies_async", "POST"), ]; for (path, method) in denied { assert!( diff --git a/frontend/src/lib/components/apps/editor/PublicAppFrame.svelte b/frontend/src/lib/components/apps/editor/PublicAppFrame.svelte index 841ec20574..5aa9daaee6 100644 --- a/frontend/src/lib/components/apps/editor/PublicAppFrame.svelte +++ b/frontend/src/lib/components/apps/editor/PublicAppFrame.svelte @@ -258,21 +258,22 @@ return url.pathname + url.search + url.hash } - async function initEmbedder(sdkConsent = false) { + async function initEmbedder() { status = 'loading' try { - const resp = await fetchEmbedToken({ sdkConsent }) + const resp = await fetchEmbedToken() embedToken = resp.token ?? null sandboxed = resp.sandbox ?? false isRaw = resp.raw_app ?? false appPath = resp.app_path ?? undefined workspaceId = resp.workspace_id ?? undefined - // The backend only advertises scopes where a token is actually usable - // (raw + unsandboxed); it mints one only when `sdkConsent` was passed. + // The backend only advertises scopes where a token could actually be + // minted (raw, unsandboxed, viewer authenticated), and mints one only on + // the follow-up request carrying the viewer's consent. sdkScopes = resp.sdk_scopes?.length ? resp.sdk_scopes : undefined viewerEmail = resp.viewer_email ?? '' - sdkToken = resp.token ?? undefined - if (sdkScopes && !sdkConsent) { + sdkToken = undefined + if (sdkScopes) { if (!hasStoredSdkConsent(viewerEmail, workspaceId ?? '', appPath ?? '', sdkScopes)) { // Ask before the app's code runs. This is the viewer's decision // point, not a containment boundary — an unsandboxed app runs with @@ -280,8 +281,7 @@ status = 'sdkPrompt' return } - // Consent already stored: re-request, this time minting. - await initEmbedder(true) + await mintWithConsent() return } finishReady() @@ -290,6 +290,29 @@ } } + /** Re-request the token with the viewer's consent. The token is optional by + * construction (that is the "Open without granting" path), so a mint that + * fails for any reason other than lost access must still render the app + * credential-less rather than fail it: `ensure_scopes_within_caller` legitimately + * denies a viewer whose own session is scope-restricted (an external-JWT share + * link), and failing there would make the app permanently unviewable for them. + * Returns whether a token was actually obtained. */ + async function mintWithConsent(): Promise { + try { + const resp = await fetchEmbedToken({ sdkConsent: true }) + sdkToken = resp.token ?? undefined + } catch (e: any) { + if (e?.status === 401) { + status = 'noPermission' + return false + } + console.warn('Failed to mint the raw app frontend SDK token', e) + sdkToken = undefined + } + finishReady() + return sdkToken !== undefined + } + function finishReady() { status = 'ready' if (unsandboxed || isRaw) { @@ -304,10 +327,13 @@ } async function onSdkConsentContinue(dontAskAgain: boolean) { - if (dontAskAgain) { + status = 'loading' + const minted = await mintWithConsent() + // Only remember the choice once it actually produced a token, so a viewer + // whose mint fails keeps being asked instead of silently never getting one. + if (dontAskAgain && minted) { storeSdkConsent(viewerEmail, workspaceId ?? '', appPath ?? '', sdkScopes ?? []) } - await initEmbedder(true) } /** Declined: render the app anyway, with no credential for its frontend code