mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-07 16:03:21 +00:00
fix(apps): deny dependency jobs and survive a failed SDK mint
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018Gmsk9kAG7p9t2Qy6ADRJz
This commit is contained in:
co-authored by
Claude Fable 5
parent
4b9ed58c3f
commit
96387e312c
@@ -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))
|
||||
}
|
||||
|
||||
|
||||
@@ -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<EmbedTokenResponse> {
|
||||
// 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!(
|
||||
|
||||
@@ -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<boolean> {
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user