From 1dc4ed5e7fb86c9658f1f877e7b92cd9dab5e283 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Fri, 9 Oct 2026 19:13:48 +0200 Subject: [PATCH] feat: add a jobs:run:dependencies token scope for lock generation (#11643) * feat: add a jobs:run:dependencies token scope for lock generation Co-Authored-By: Claude Opus 5.5 * test: pin lock-job reads to the dependencies scope Co-Authored-By: Claude Opus 5.5 * fix: align scope labels with and without a restrict paths button Co-Authored-By: Claude Opus 5.5 * fix: match a lock job's stored path literally on job reads Co-Authored-By: Claude Opus 5.5 * fix: drop the dependency-lock warning from the jobs scope heading Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KNiYsRRDyvkRYwkHda6uyf --------- Co-authored-by: Claude Opus 5.5 --- ...f378e9fac4986cd29f69c1f2b8a83ba515c3.json} | 4 +- backend/tests/fixtures/jobs_read_auth.sql | 26 ++++++++++ backend/tests/jobs_read_auth.rs | 38 ++++++++++++++ backend/windmill-api-auth/src/scopes.rs | 52 ++++++++++++++++--- backend/windmill-api/src/jobs.rs | 33 +++++++----- backend/windmill-api/src/token.rs | 5 ++ .../components/settings/ScopeSelector.svelte | 6 +-- 7 files changed, 139 insertions(+), 25 deletions(-) rename backend/.sqlx/{query-7904ac35748b1de2c7ff764f22787436d0fe74f99f298e5c399233f0d247f2ec.json => query-8aef4f9d27ec6df07cd671bcf90bf378e9fac4986cd29f69c1f2b8a83ba515c3.json} (54%) diff --git a/backend/.sqlx/query-7904ac35748b1de2c7ff764f22787436d0fe74f99f298e5c399233f0d247f2ec.json b/backend/.sqlx/query-8aef4f9d27ec6df07cd671bcf90bf378e9fac4986cd29f69c1f2b8a83ba515c3.json similarity index 54% rename from backend/.sqlx/query-7904ac35748b1de2c7ff764f22787436d0fe74f99f298e5c399233f0d247f2ec.json rename to backend/.sqlx/query-8aef4f9d27ec6df07cd671bcf90bf378e9fac4986cd29f69c1f2b8a83ba515c3.json index e103a91c1e..53a9daaf9a 100644 --- a/backend/.sqlx/query-7904ac35748b1de2c7ff764f22787436d0fe74f99f298e5c399233f0d247f2ec.json +++ b/backend/.sqlx/query-8aef4f9d27ec6df07cd671bcf90bf378e9fac4986cd29f69c1f2b8a83ba515c3.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "WITH RECURSIVE chain(id, parent_job) AS (\n SELECT id, parent_job FROM v2_job WHERE id = $1 AND workspace_id = $2\n UNION ALL\n SELECT j.id, j.parent_job FROM v2_job j\n JOIN chain c ON j.id = c.parent_job AND j.workspace_id = $2\n )\n SELECT\n CASE WHEN a.agent THEN regexp_replace(j.runnable_path, '\\.chat$', '')\n ELSE j.runnable_path END AS runnable_path,\n CASE\n WHEN a.agent THEN 'agents'\n WHEN j.kind IN ('script', 'script_hub', 'unassigned_script') THEN 'scripts'\n WHEN j.kind IN ('flow', 'unassigned_flow') THEN 'flows'\n WHEN j.kind IN ('singlestepflow', 'unassigned_singlestepflow') THEN\n CASE WHEN COALESCE(\n (SELECT m->'value'->>'type'\n FROM jsonb_array_elements(j.raw_flow->'modules') m\n WHERE m->>'id' IN ('a', 'main')\n LIMIT 1),\n 'script'\n ) = 'flow' THEN 'flows' ELSE 'scripts' END\n END AS scope_kind,\n CASE WHEN j.trigger_kind = 'app' THEN j.trigger END AS launched_by_app\n FROM v2_job j JOIN chain c ON c.id = j.id,\n LATERAL (SELECT j.kind = 'flowpreview'\n AND j.raw_flow->'modules'->1 IS NULL\n AND j.raw_flow->'modules'->0->>'id' = '__wm_agent_root' AS agent) a\n WHERE j.workspace_id = $2", + "query": "WITH RECURSIVE chain(id, parent_job) AS (\n SELECT id, parent_job FROM v2_job WHERE id = $1 AND workspace_id = $2\n UNION ALL\n SELECT j.id, j.parent_job FROM v2_job j\n JOIN chain c ON j.id = c.parent_job AND j.workspace_id = $2\n )\n SELECT\n CASE WHEN a.agent THEN regexp_replace(j.runnable_path, '\\.chat$', '')\n ELSE j.runnable_path END AS runnable_path,\n CASE\n WHEN a.agent THEN 'agents'\n WHEN j.kind IN ('script', 'script_hub', 'unassigned_script') THEN 'scripts'\n WHEN j.kind IN ('flow', 'unassigned_flow') THEN 'flows'\n WHEN j.kind IN ('dependencies', 'flowdependencies') THEN 'dependencies'\n WHEN j.kind IN ('singlestepflow', 'unassigned_singlestepflow') THEN\n CASE WHEN COALESCE(\n (SELECT m->'value'->>'type'\n FROM jsonb_array_elements(j.raw_flow->'modules') m\n WHERE m->>'id' IN ('a', 'main')\n LIMIT 1),\n 'script'\n ) = 'flow' THEN 'flows' ELSE 'scripts' END\n END AS scope_kind,\n CASE WHEN j.trigger_kind = 'app' THEN j.trigger END AS launched_by_app\n FROM v2_job j JOIN chain c ON c.id = j.id,\n LATERAL (SELECT j.kind = 'flowpreview'\n AND j.raw_flow->'modules'->1 IS NULL\n AND j.raw_flow->'modules'->0->>'id' = '__wm_agent_root' AS agent) a\n WHERE j.workspace_id = $2", "describe": { "columns": [ { @@ -31,5 +31,5 @@ null ] }, - "hash": "7904ac35748b1de2c7ff764f22787436d0fe74f99f298e5c399233f0d247f2ec" + "hash": "8aef4f9d27ec6df07cd671bcf90bf378e9fac4986cd29f69c1f2b8a83ba515c3" } diff --git a/backend/tests/fixtures/jobs_read_auth.sql b/backend/tests/fixtures/jobs_read_auth.sql index e32b777a74..e175a9c065 100644 --- a/backend/tests/fixtures/jobs_read_auth.sql +++ b/backend/tests/fixtures/jobs_read_auth.sql @@ -98,6 +98,32 @@ INSERT INTO public.v2_job_completed (id, workspace_id, duration_ms, status, resu ('18181818-1818-1818-1818-181818181818', 'test-workspace', 1000, 'success'::job_status, '{"agent": "AGENT_CHAT_RESULT"}'); +-- A lock job for flow `f/shared/flow1`, as `wmill generate-metadata` queues it, and the +-- token CI would lock with. +INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin, scopes) VALUES ( + encode(sha256('RUN_SCOPED_DEPS_TOKEN'::bytea), 'hex'), 'RUN_DEPS_', 'RUN_SCOPED_DEPS_TOKEN', + 'test2@windmill.dev', 'lock token', false, + ARRAY['jobs:run:dependencies:f/shared/*'] +); +INSERT INTO public.v2_job ( + id, workspace_id, created_by, created_at, permissioned_as, permissioned_as_email, + kind, runnable_path, tag, visible_to_owner +) VALUES ( + '19191919-1919-1919-1919-191919191919', 'test-workspace', 'test-user-2', + '2023-01-01 00:00:00', 'u/test-user-2', 'test2@windmill.dev', + 'flowdependencies', 'f/shared/flow1', 'dependency', true +), ( + -- Queued by a broader token with `*` as its path, which the request body allows. + '20202020-2020-2020-2020-202020202020', 'test-workspace', 'test-user-2', + '2023-01-01 00:00:00', 'u/test-user-2', 'test2@windmill.dev', + 'dependencies', '*', 'dependency', true +); +INSERT INTO public.v2_job_completed (id, workspace_id, duration_ms, status, result) VALUES + ('19191919-1919-1919-1919-191919191919', 'test-workspace', 1000, 'success'::job_status, + '{"lock": "DEPS_RESULT"}'), + ('20202020-2020-2020-2020-202020202020', 'test-workspace', 1000, 'success'::job_status, + '{"lock": "WILDCARD_DEPS_RESULT"}'); + -- A token pairing an app scope with a run scope, as someone driving an app's components -- programmatically would build. `APP_INLINE_JOB` is an inline-script component run: no -- `jobs:run` scope can name its kind, so only the `apps:run` half puts it in reach. diff --git a/backend/tests/jobs_read_auth.rs b/backend/tests/jobs_read_auth.rs index 6a101200a1..bffacfbaf0 100644 --- a/backend/tests/jobs_read_auth.rs +++ b/backend/tests/jobs_read_auth.rs @@ -476,6 +476,44 @@ async fn test_single_job_read_authorization(db: Pool) -> anyhow::Resul ); } + // A lock job belongs to `jobs:run:dependencies`: its path is whatever the caller sent, + // so a token scoped to run the flow at that path must not read it. + let path = "completed/get_result/19191919-1919-1919-1919-191919191919"; + let (status, body) = get(&base, path, Some("RUN_SCOPED_DEPS_TOKEN")).await; + assert!( + status.is_success() && body.contains("DEPS_RESULT"), + "dependencies-scoped token must read its lock job (got {status}): {body}" + ); + let (status, body) = get(&base, path, Some("RUN_SCOPED_TOKEN")).await; + assert_eq!( + status, + reqwest::StatusCode::NOT_FOUND, + "a flow-scoped token must not read a lock job (got {status}): {body}" + ); + let (status, body) = get( + &base, + &format!("completed/get_result/{FLOW_JOB}"), + Some("RUN_SCOPED_DEPS_TOKEN"), + ) + .await; + assert_eq!( + status, + reqwest::StatusCode::NOT_FOUND, + "dependencies-scoped token must not read a flow run at its path (got {status}): {body}" + ); + // A lock job filed at `*` matches no path-scoped grant: the stored path is not a pattern. + let (status, body) = get( + &base, + "completed/get_result/20202020-2020-2020-2020-202020202020", + Some("RUN_SCOPED_DEPS_TOKEN"), + ) + .await; + assert_eq!( + status, + reqwest::StatusCode::NOT_FOUND, + "a path-scoped token must not read a lock job filed at `*` (got {status}): {body}" + ); + // An `apps:run:` scope is a start grant too: the inline-script component run it // launched — a kind no `jobs:run` scope can name — stays readable to a token scoped // to that app, and stays out of reach for one that is only scoped to run jobs. diff --git a/backend/windmill-api-auth/src/scopes.rs b/backend/windmill-api-auth/src/scopes.rs index d586b6ba63..21f916bf94 100644 --- a/backend/windmill-api-auth/src/scopes.rs +++ b/backend/windmill-api-auth/src/scopes.rs @@ -170,10 +170,16 @@ const FLOW_JOBS: [&'static str; 6] = [ /// scoped by the agent's path like a deployed runnable's, as `jobs:run:agents:`. const AGENT_JOBS: [&'static str; 1] = ["jobs/run/agent"]; +/// Lock resolution for scripts, apps and flows, scoped as `jobs:run:dependencies:`. +/// Resolving a package can run its build or install scripts on the worker, so this is +/// still code execution: the scope only takes away the direct ways to run code. +const DEPENDENCY_JOBS: [&'static str; 2] = ["jobs/run/dependencies", "jobs/run/flow_dependencies"]; + lazy_static::lazy_static! { static ref RUN_PATH_ACTIONS: Vec<&'static str> = { - let mut v = vec!["jobs/resume/", "jobs/run/batch_rerun_jobs", "jobs/run/workflow_as_code", "jobs/run/dependencies","jobs/run/flow_dependencies", "apps_u/execute_component", "apps_u/upload_s3_file"]; + let mut v = vec!["jobs/resume/", "jobs/run/batch_rerun_jobs", "jobs/run/workflow_as_code", "apps_u/execute_component", "apps_u/upload_s3_file"]; + v.extend(DEPENDENCY_JOBS); v.extend(SCRIPT_JOBS); v.extend(FLOW_JOBS); v.extend(AGENT_JOBS); @@ -227,11 +233,13 @@ fn map_http_method_to_action(method: &str, route_path: &str) -> ScopeAction { } } -/// Checks the route path to determine the runnable kind ("agents", "flows" or "scripts"). +/// Checks the route path to determine the runnable kind ("dependencies", "agents", "flows" +/// or "scripts"). /// /// The order of checks is important: -/// - Flow-related paths are checked first to avoid false positives, as some flow paths -/// (e.g., `/run_preview_flow`) share prefixes with script paths (e.g., `/run_preview`). +/// - Dependency paths are checked before flow paths: `flow_dependencies` starts with `f`. +/// - Flow-related paths are checked before script paths to avoid false positives, as some +/// flow paths (e.g., `/run_preview_flow`) share prefixes with script paths (e.g., `/run_preview`). /// /// Returns the kind based on the match, or `None` if no match is found. fn determine_kind_from_route(route_path: &str) -> Option { @@ -247,6 +255,12 @@ fn determine_kind_from_route(route_path: &str) -> Option { { return None; } + if DEPENDENCY_JOBS + .iter() + .any(|path| route_path.starts_with(path)) + { + return Some("dependencies".to_string()); + } if AGENT_JOBS.iter().any(|path| route_path.starts_with(path)) { return Some("agents".to_string()); } @@ -586,17 +600,23 @@ pub struct JobReadConfinement { } impl JobReadConfinement { - /// Whether a job that ran `runnable_path` as `kind` (`scripts`, `flows` or `agents`) - /// is inside the confinement. A read path admits every kind; a run scope only its own. + /// Whether a job that ran `runnable_path` as `kind` (`scripts`, `flows`, `agents` or + /// `dependencies`) is inside the confinement. A read path admits every kind; a run + /// scope only its own. pub fn admits(&self, kind: &str, runnable_path: &str) -> bool { if self.read_paths.allows(runnable_path) { return true; } + // Lock jobs and previews store the path their caller sent. The matcher reads `*` + // in it as a pattern, and `,`/`:` are scope syntax: such a path needs a grant + // covering every path, as enqueuing it did. + let resource = windmill_common::auth::is_scope_literal_path(runnable_path) + .then(|| vec![runnable_path.to_string()]); let required = ScopeDefinition::new( ScopeDomain::Jobs.as_str(), ScopeAction::Run.as_str(), Some(kind), - Some(vec![runnable_path.to_string()]), + resource, ); self.run.iter().any(|scope| scope.includes(&required)) } @@ -1715,6 +1735,24 @@ mod tests { scope_for_route("POST", "/api/w/ws/jobs/run/agent/u/x/y").as_deref(), Some("jobs:run:agents") ); + for path in [ + "/api/w/ws/jobs/run/dependencies", + "/api/w/ws/jobs/run/dependencies_async", + "/api/w/ws/jobs/run/flow_dependencies", + "/api/w/ws/jobs/run/flow_dependencies_async", + ] { + assert_eq!( + scope_for_route("POST", path).as_deref(), + Some("jobs:run:dependencies"), + "{path}" + ); + assert!(check_route_access(&["jobs:run:dependencies".into()], path, "POST").is_ok()); + assert!(check_route_access(&["jobs:run:flows".into()], path, "POST").is_err()); + } + // A dependencies grant reaches no other run route. + for path in ["/api/w/ws/jobs/run/preview", "/api/w/ws/jobs/run/p/u/x/y"] { + assert!(check_route_access(&["jobs:run:dependencies".into()], path, "POST").is_err()); + } // Preview/bundle runs have no deployed path and their handlers require the // broad `jobs:run` scope, so the derived scope must not carry a kind. diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index adbbda0abc..f1e4120e8c 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -2010,9 +2010,11 @@ async fn require_job_within_read_scope( } } // `scope_kind` is the runnable kind a `jobs:run::` scope can name, or - // NULL for a job no such scope reaches directly (previews, dependency jobs, - // flow-inlined scripts) — those are still readable as a step of a matching flow, - // through their ancestors. A `singlestepflow` wraps either a script or a flow, so it + // NULL for a job no such scope reaches directly (previews, flow-inlined scripts) — + // those are still readable as a step of a matching flow, through their ancestors. + // A dependency job's path is the one its `jobs:run:dependencies:` check + // admitted, but it is filed under `dependencies`, so it never passes for a run of + // the deployed runnable at that path. A `singlestepflow` wraps either a script or a flow, so it // projects onto the wrapped runnable the same way the batch-rerun query does. An agent // run is a preview of the one-step flow `agent_runs::agent_step_flow` builds, filed under the // agent's path (`.chat` for a chat turn); the editor's own runs of it look the same, @@ -2031,6 +2033,7 @@ async fn require_job_within_read_scope( WHEN a.agent THEN 'agents' WHEN j.kind IN ('script', 'script_hub', 'unassigned_script') THEN 'scripts' WHEN j.kind IN ('flow', 'unassigned_flow') THEN 'flows' + WHEN j.kind IN ('dependencies', 'flowdependencies') THEN 'dependencies' WHEN j.kind IN ('singlestepflow', 'unassigned_singlestepflow') THEN CASE WHEN COALESCE( (SELECT m->'value'->>'type' @@ -4628,14 +4631,8 @@ async fn cancel_selection( } else { ids }; - let Json(mut w_cancelled) = cancel_jobs( - ids, - &db, - &authed, - workspace_id.as_str(), - force_cancel, - ) - .await?; + let Json(mut w_cancelled) = + cancel_jobs(ids, &db, &authed, workspace_id.as_str(), force_cancel).await?; cancelled.append(&mut w_cancelled); } @@ -9529,13 +9526,23 @@ pub struct RunDependenciesResponse { pub dependencies: String, } +/// The path comes from the request body: one carrying scope syntax (`,`, `:`, `*`) could +/// put a granted path beside an ungranted one, so it needs a grant covering every path. +fn check_dependencies_scope(authed: &ApiAuthed, path: &str) -> error::Result<()> { + if windmill_common::auth::is_scope_literal_path(path) { + check_scopes(authed, || format!("jobs:run:dependencies:{path}")) + } else { + check_scopes(authed, || "jobs:run:dependencies".to_string()) + } +} + async fn push_dependencies_job( authed: &ApiAuthed, db: &DB, w_id: &str, req: RunDependenciesRequest, ) -> error::Result { - check_scopes(authed, || format!("jobs:run"))?; + check_dependencies_scope(authed, &req.entrypoint)?; if authed.is_operator { return Err(error::Error::NotAuthorized( "Operators cannot run dependencies jobs for security reasons".to_string(), @@ -9658,7 +9665,7 @@ async fn push_flow_dependencies_job( w_id: &str, req: RunFlowDependenciesRequest, ) -> error::Result { - check_scopes(authed, || format!("jobs:run"))?; + check_dependencies_scope(authed, &req.path)?; check_operator_can_build_flows(db, w_id, authed.is_operator, "run dependencies jobs").await?; // The dependency job locks whatever inline code this request carries, on a worker. A // composition-only flow has none, so validating here costs a builder nothing and keeps the diff --git a/backend/windmill-api/src/token.rs b/backend/windmill-api/src/token.rs index b7fbe96404..ab22f2d15b 100644 --- a/backend/windmill-api/src/token.rs +++ b/backend/windmill-api/src/token.rs @@ -215,6 +215,11 @@ lazy_static! { label: "Run flows".to_string(), requires_resource_path: true, }, + ScopeOption { + value: "jobs:run:dependencies".to_string(), + label: "Generate dependency locks".to_string(), + requires_resource_path: true, + }, ], }]; diff --git a/frontend/src/lib/components/settings/ScopeSelector.svelte b/frontend/src/lib/components/settings/ScopeSelector.svelte index b3293ab36e..663e043035 100644 --- a/frontend/src/lib/components/settings/ScopeSelector.svelte +++ b/frontend/src/lib/components/settings/ScopeSelector.svelte @@ -643,7 +643,7 @@ ? 'bg-surface-disabled opacity-60' : 'bg-surface-tertiary'}" > -
+