diff --git a/backend/.sqlx/query-ac94dccaf3709f178ab5636892c85c4ed3a067e4374517244608c345781dbc5c.json b/backend/.sqlx/query-ac94dccaf3709f178ab5636892c85c4ed3a067e4374517244608c345781dbc5c.json deleted file mode 100644 index c64a4b452f..0000000000 --- a/backend/.sqlx/query-ac94dccaf3709f178ab5636892c85c4ed3a067e4374517244608c345781dbc5c.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT job_token_scopes IS NOT NULL FROM script WHERE hash = $1 AND workspace_id = $2", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "?column?", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Int8", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "ac94dccaf3709f178ab5636892c85c4ed3a067e4374517244608c345781dbc5c" -} diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 4de51ca00a..77963b73c6 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -6167,6 +6167,21 @@ async fn handle_zombie_jobs(db: &Pool, base_internal_url: &str, node_n continue; } if let Some(job) = job.unwrap() { + // Read while the job is still queued: a re-run (perpetual, retry) takes its cap from + // here once the `job_perms` row is swept after completion. A failed read leaves the + // job for the next sweep rather than completing it with no cap. + let perms = + match windmill_common::auth::get_job_perms(db, &job.id, &job.workspace_id).await { + Ok(perms) => perms, + Err(e) => { + tracing::error!( + "Could not read the permissions of zombie job {}: {e:#}", + job.id + ); + continue; + } + }; + let job_token_scopes = perms.as_ref().and_then(|p| p.job_token_scopes.clone()); let label = ephemeral_script_token_label(&job.permissioned_as, &job.created_by); let token = create_token_for_owner( &db, @@ -6176,7 +6191,7 @@ async fn handle_zombie_jobs(db: &Pool, base_internal_url: &str, node_n job_token_expiry_secs(&db, &job.workspace_id).await, &job.permissioned_as_email, &job.id, - None, + perms, Some(format!("handle_zombie_jobs")), ) .await @@ -6198,22 +6213,6 @@ async fn handle_zombie_jobs(db: &Pool, base_internal_url: &str, node_n ); let memory_peak = job.memory_peak.unwrap_or(0); let (_, killpill_rx_never_used) = KillpillSender::new(1); - // Read while the job is still queued: a re-run (perpetual, retry) takes its cap from - // here once the `job_perms` row is swept after completion. A failed read leaves the - // job for the next sweep rather than completing it with no cap. - let job_token_scopes = match sqlx::query_scalar!( - "SELECT job_token_scopes FROM job_perms WHERE job_id = $1", - job.id - ) - .fetch_optional(db) - .await - { - Ok(scopes) => scopes.flatten(), - Err(e) => { - tracing::error!("Could not read the token scopes of zombie job {}: {e:#}", job.id); - continue; - } - }; let mut completed = windmill_queue::MiniCompletedJob::from(job); completed.job_token_scopes = job_token_scopes; let _ = handle_job_error( diff --git a/backend/tests/job_token_scopes.rs b/backend/tests/job_token_scopes.rs index 8c5f082f46..8129fe2b3e 100644 --- a/backend/tests/job_token_scopes.rs +++ b/backend/tests/job_token_scopes.rs @@ -163,7 +163,7 @@ async fn test_restricted_job_token_is_confined(db: Pool) -> anyhow::Re "INSERT INTO job_perms (job_id, email, username, is_admin, is_operator, folders, groups, workspace_id, job_token_scopes) VALUES ($1, 'test@windmill.dev', 'test-user', true, false, '{}', '{}', 'test-workspace', - '{jobs:write}')", + '{jobs:run}')", ) .bind(admin_job) .execute(&db) @@ -187,6 +187,27 @@ async fn test_restricted_job_token_is_confined(db: Pool) -> anyhow::Re .send() .await?; assert_eq!(resp.status(), StatusCode::FORBIDDEN, "{}", resp.text().await?); + // Nor can it, even an admin's, place a child in an unrelated run: the flow-run routes + // trust that lineage. + let resp = client + .post(format!( + "{base}/jobs/run/p/u/test-user-3/open?root_job={RUN_JOB}" + )) + .bearer_auth(&admin_token) + .json(&json!({})) + .send() + .await?; + assert_eq!(resp.status(), StatusCode::CREATED, "{}", resp.text().await?); + let child = Uuid::parse_str(&resp.text().await?)?; + let claimed: bool = sqlx::query_scalar( + "SELECT $2 IN (parent_job, root_job, flow_innermost_root_job) IS TRUE FROM v2_job + WHERE id = $1", + ) + .bind(child) + .bind(Uuid::parse_str(RUN_JOB)?) + .fetch_one(&db) + .await?; + assert!(!claimed); // A job it starts is capped at its own scopes, intersected with the target's setting. let run_token = job_token(&db, RUN_JOB).await?; diff --git a/backend/windmill-api-jobs/src/execution.rs b/backend/windmill-api-jobs/src/execution.rs index 688144eaeb..8c13483e41 100644 --- a/backend/windmill-api-jobs/src/execution.rs +++ b/backend/windmill-api-jobs/src/execution.rs @@ -124,7 +124,9 @@ pub async fn drop_unclaimable_run_lineage( /// The jobs of `referenced` that `authed` cannot claim as its own run lineage: anything but the /// token's own job and that job's `parent_job`, `root_job` and `flow_innermost_root_job`, or for -/// a workspace admin anything outside the workspace. +/// a workspace admin anything outside the workspace. A restricted job token never gets the admin +/// latitude: its flow-run routes trust this lineage (`job_in_same_flow_run`), so a claimed +/// unrelated run would escape its scopes. pub async fn unclaimable_run_lineage( db: &DB, w_id: &str, @@ -157,7 +159,8 @@ pub async fn unclaimable_run_lineage( if referenced.is_empty() { return Ok(referenced); } - let in_workspace = if authed.is_admin { + let restricted_job_token = authed.job_id.is_some() && authed.scopes.is_some(); + let in_workspace = if authed.is_admin && !restricted_job_token { sqlx::query_scalar!( "SELECT id FROM v2_job WHERE id = ANY($1) AND workspace_id = $2", &referenced, diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 2f0c8a3400..b3a5e86b46 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -9042,18 +9042,16 @@ async fn run_inline_script_inner( ) .await?; let restricted = info.job_token_scopes.is_some(); - (InlineScriptTarget::Path { path, hash: info.hash }, restricted) + ( + InlineScriptTarget::Path { path, hash: info.hash }, + restricted, + ) } InlineScriptTarget::Hash(hash) => { - let restricted = sqlx::query_scalar!( - "SELECT job_token_scopes IS NOT NULL FROM script WHERE hash = $1 AND workspace_id = $2", - hash, - &w_id - ) - .fetch_optional(&db) - .await? - .flatten() - .unwrap_or(false); + let restricted = windmill_common::get_script_info_for_hash(None, &db, &w_id, hash) + .await? + .job_token_scopes + .is_some(); (InlineScriptTarget::Hash(hash), restricted) } }; diff --git a/docs/auth-surface.md b/docs/auth-surface.md index be1f35572c..38e5cefbf6 100644 --- a/docs/auth-surface.md +++ b/docs/auth-surface.md @@ -63,7 +63,9 @@ Symbols, not line numbers, are cited: they drift less. triggers and error/success handlers start from the target's own setting. A restricted token keeps only the runtime routes about its own job (`is_own_job_runtime_route`) and the reads of its own flow run (`flow_run_read_route_job`, checked against its lineage: the orchestrator - evaluates a step's `results.x` with the token of the step that just finished); a route that + evaluates a step's `results.x` with the token of the step that just finished). That lineage is + trusted, so a restricted job token, even an admin's, can only place a job it starts in its own + run (`unclaimable_run_lineage`); a route that authenticates a token itself instead of through the route layer must call `check_job_token_scope`. A restricted job never runs on a dedicated worker, which uses its own unscoped token for every job. `$var:`/`$res:` args are resolved through the API with the job's