From 8627b649b482d6a87eb8d178fbef33d679237d20 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Fri, 2 Oct 2026 18:15:29 +0200 Subject: [PATCH] fix: keep restricted jobs off every dedicated handoff, confine progress flow id Co-Authored-By: Claude Opus 5.5 (1M context) --- ...51cbd7b8896984115d55318bb6f4100d72d9a.json | 24 +++++++++++++++++++ backend/ee-repo-ref.txt | 2 +- backend/windmill-api-jobs/src/job_metrics.rs | 23 ++++++++++++++++++ backend/windmill-worker/src/worker.rs | 12 ++++++---- 4 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 backend/.sqlx/query-67db536d06c57f195641f178ce651cbd7b8896984115d55318bb6f4100d72d9a.json diff --git a/backend/.sqlx/query-67db536d06c57f195641f178ce651cbd7b8896984115d55318bb6f4100d72d9a.json b/backend/.sqlx/query-67db536d06c57f195641f178ce651cbd7b8896984115d55318bb6f4100d72d9a.json new file mode 100644 index 0000000000..cfd88ed5cb --- /dev/null +++ b/backend/.sqlx/query-67db536d06c57f195641f178ce651cbd7b8896984115d55318bb6f4100d72d9a.json @@ -0,0 +1,24 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT $2 IN (parent_job, root_job, flow_innermost_root_job) FROM v2_job\n WHERE id = $1 AND workspace_id = $3", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "?column?", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Uuid", + "Uuid", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "67db536d06c57f195641f178ce651cbd7b8896984115d55318bb6f4100d72d9a" +} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 2e26e32e9b..38d164ba09 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -7737c32ebed0901cc143ada358654ee4743b02b3 \ No newline at end of file +d5879586211e7116d8198f9a03a0fcbb81c55787 \ No newline at end of file diff --git a/backend/windmill-api-jobs/src/job_metrics.rs b/backend/windmill-api-jobs/src/job_metrics.rs index abb95693f5..ddbe5a047a 100644 --- a/backend/windmill-api-jobs/src/job_metrics.rs +++ b/backend/windmill-api-jobs/src/job_metrics.rs @@ -173,10 +173,33 @@ struct JobProgressSetRequest { } async fn set_job_progress( + authed: windmill_api_auth::ApiAuthed, Extension(db): Extension, Path((w_id, job_id)): Path<(String, Uuid)>, Json(JobProgressSetRequest { percent, flow_job_id }): Json, ) -> error::JsonResult<()> { + // A restricted job token reaches this route without a scope only for its own job + // (`is_own_job_runtime_route`), so the flow it reports progress to must be its own too. + if let (Some(flow_job_id), Some(token_job), Some(_)) = + (flow_job_id, authed.job_id, &authed.scopes) + { + let own_flow = sqlx::query_scalar!( + "SELECT $2 IN (parent_job, root_job, flow_innermost_root_job) FROM v2_job + WHERE id = $1 AND workspace_id = $3", + token_job, + flow_job_id, + &w_id + ) + .fetch_optional(&db) + .await? + .flatten() + .unwrap_or(false); + if !own_flow { + return Err(error::Error::PermissionDenied(format!( + "flow job {flow_job_id} is not this job's flow" + ))); + } + } // If flow_job_id exists, than we should modify flow_status of corresponding module // Individual jobs and flows are handled differently if let Some(flow_job_id) = flow_job_id { diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index 0f6c022edd..15bbfa15a4 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -3595,8 +3595,12 @@ pub async fn run_worker( // dispatch by path and return before that check, so a job sent down them // would run with whatever arguments survived the failure. let fails_before_running = job.pre_run_error.is_some(); + // A dedicated worker or flow runner runs every job it gets with its own + // unscoped worker token, so a job with a restricted token runs here, with the + // token minted for it, whichever tag brought it. + let restricted = job.job_token_scopes.is_some(); - if !dedicated_workers.is_empty() && !fails_before_running { + if !dedicated_workers.is_empty() && !fails_before_running && !restricted { let dedicated_worker_tx = job.runnable_path.as_ref().and_then(|path| { // For flow steps inside branches/loops, runnable_path includes // nesting segments (e.g. f/flow/branchone-0/a) but the dedicated @@ -3641,10 +3645,8 @@ pub async fn run_worker( NextJob::Http(_) => None, }; - // A flow runner runs every step it gets with its own unscoped worker token, so - // a step with a restricted token runs here, with the token minted for it. - if let Some(flow_runners) = flow_runners - .filter(|_| !fails_before_running && job.job_token_scopes.is_none()) + if let Some(flow_runners) = + flow_runners.filter(|_| !fails_before_running && !restricted) { let key_o = job.flow_step_id.as_ref().map(|x| x.to_string()); if let Some(key) = key_o {