Skip to content

Commit 8c747e9

Browse files
rubenfiszelclaude
andcommitted
fix: scope-check dynamic_select flow branch and inline preview
Address CI review: the dynamic_select Deployed{Flow} branch ran a deployed flow's dynamic-select code without any scope check (only the Script branch delegated to a scope-checked handler), and run_inline_preview_script executed request-supplied code with no in-handler scope check. Add jobs:run:flows:{path} to the flow branch and jobs:run to inline preview; correct the misleading comment. Expand regression tests (preview_flow case, assert success for the broad-token case). Advisory GHSA-vxc5-w28p-m9xw. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent f1a7cc8 commit 8c747e9

2 files changed

Lines changed: 55 additions & 3 deletions

File tree

backend/tests/preview_scope_enforcement.rs

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,53 @@ async fn broad_jobs_run_token_not_blocked_on_preview(db: Pool<Postgres>) -> anyh
108108
.send()
109109
.await?;
110110

111-
assert_ne!(
111+
assert!(
112+
resp.status().is_success(),
113+
"a token with the broad jobs:run scope must be allowed to run preview \
114+
(expected 2xx), got {}",
115+
resp.status()
116+
);
117+
118+
Ok(())
119+
}
120+
121+
/// A token scoped to a specific flow clears the route middleware for the flow
122+
/// preview route (route kind = flows) but must still be denied by the in-handler
123+
/// `check_scopes("jobs:run")` because flow preview runs an arbitrary flow.
124+
#[sqlx::test(fixtures("base"))]
125+
async fn flow_scoped_token_cannot_run_arbitrary_preview_flow(
126+
db: Pool<Postgres>,
127+
) -> anyhow::Result<()> {
128+
initialize_tracing().await;
129+
let server = ApiServer::start(db.clone()).await?;
130+
let port = server.addr.port();
131+
132+
insert_scoped_token(&db, "POC_FLOW_TOKEN", &["jobs:run:flows:f/locked/onlyflow"]).await;
133+
let client = windmill_api_client::create_client(
134+
&format!("http://localhost:{port}"),
135+
"POC_FLOW_TOKEN".to_string(),
136+
);
137+
138+
let resp = client
139+
.client()
140+
.post(format!(
141+
"{}/w/test-workspace/jobs/run/preview_flow",
142+
client.baseurl()
143+
))
144+
.json(&json!({ "value": { "modules": [] }, "path": null, "args": {} }))
145+
.send()
146+
.await?;
147+
148+
assert_eq!(
112149
resp.status(),
113150
reqwest::StatusCode::FORBIDDEN,
114-
"a token with the broad jobs:run scope must not be blocked on /jobs/run/preview"
151+
"a flow-scoped token must be denied (403) on /jobs/run/preview_flow, got {}",
152+
resp.status()
153+
);
154+
let body = resp.text().await?;
155+
assert!(
156+
body.contains("jobs:run"),
157+
"403 should reference the required jobs:run scope, body: {body}"
115158
);
116159

117160
Ok(())

backend/windmill-api/src/jobs.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5752,6 +5752,9 @@ async fn run_inline_preview_script(
57525752
Path(w_id): Path<String>,
57535753
Json(preview): Json<PreviewInline>,
57545754
) -> error::Result<Response> {
5755+
// Same arbitrary-code class as run_preview_script: a narrowly-scoped token
5756+
// must not be able to run request-supplied code through inline preview.
5757+
check_scopes(&authed, || format!("jobs:run"))?;
57555758
if let Some(job_id) = job_id {
57565759
register_potential_assets_on_inline_execution(job_id, &w_id, &preview);
57575760
}
@@ -6821,6 +6824,11 @@ async fn run_dynamic_select(
68216824
return Ok((StatusCode::CREATED, uuid.to_string()).into_response());
68226825
}
68236826
RunnableKind::Flow => {
6827+
// Runs the deployed flow's dynamic-select code. Enforce the same
6828+
// path-scoped check the script branch gets via
6829+
// push_script_job_by_path_into_queue, so a token not scoped to this
6830+
// flow cannot trigger its code through dynamic select.
6831+
check_scopes(&authed, || format!("jobs:run:flows:{path}"))?;
68246832
let mut conn = user_db.clone().begin(&authed).await?;
68256833

68266834
let dynamic_input_res = match DYNAMIC_INPUT_CACHE.get(&format!("{}:{}", w_id, path))
@@ -6870,7 +6878,8 @@ async fn run_dynamic_select(
68706878
DynamicSelectRunnableRef::Inline { code, lang: language } => {
68716879
// Inline dynamic select runs arbitrary, request-supplied code; require the broad
68726880
// jobs:run scope so a narrowly-scoped token cannot escape its scope. The Deployed
6873-
// branches resolve a path and are scope-checked by their own runnable handlers.
6881+
// branches are path-scoped instead (scripts via push_script_job_by_path_into_queue,
6882+
// flows via the check_scopes above).
68746883
check_scopes(&authed, || format!("jobs:run"))?;
68756884
dynamic_input = DynamicInput {
68766885
x_windmill_dyn_select_code: code,

0 commit comments

Comments
 (0)