mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-12 16:02:56 +00:00
fix: confine restricted job tokens to their own run lineage; drop remaining extra lookups
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FHGE3ynoAu6yrgeg4kLwL
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
3636374f46
commit
d1fef7ceb3
-23
@@ -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"
|
||||
}
|
||||
+16
-17
@@ -6167,6 +6167,21 @@ async fn handle_zombie_jobs(db: &Pool<Postgres>, 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<Postgres>, 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<Postgres>, 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(
|
||||
|
||||
@@ -163,7 +163,7 @@ async fn test_restricted_job_token_is_confined(db: Pool<Postgres>) -> 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<Postgres>) -> 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?;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
};
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user