mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
fix(jobs): authorization bypass in only_result job updates (WIN-1980) (#9301)
* fix(jobs): enforce anonymous-only guard on `only_result` job updates
The `jobs_u/getupdate/{id}` and `jobs_u/getupdate_sse/{id}` endpoints
accept `only_result=true`. In that branch, `get_job_update_data` queried
the result solely by (workspace_id, job_id) and skipped the
`created_by == "anonymous"` check that the non-only_result path and
adjacent unauthenticated endpoints apply. An unauthenticated requester
who learned a private job UUID could therefore retrieve that job's
output.
Hoist the guard to the top of `get_job_update_data` so both branches are
covered.
Fixes WIN-1980
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: fold `created_by` check into existing only_result queries
Avoids the extra `SELECT created_by` round-trip per call by joining
`v2_job` once in the two queries that handled the unauth path and
checking inline. Behavior is identical to the prior commit; the SSE
polling loop now does one query per poll instead of two for
unauthenticated callers.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: cache anonymous_verified across SSE polls
Replace the LEFT JOIN approach with an upfront `SELECT created_by`
guarded by a new `&mut bool anonymous_verified` parameter that mirrors
`early_return_suppressed`. The SSE polling loop now performs the auth
check exactly once per stream rather than per poll, and the data SQL
reverts to its original form so authenticated callers pay no extra
cost. `created_by` cannot change after job creation, so caching the
verification across polls is safe.
Cost matrix:
- Authed (any path): 0 extra queries
- Unauthed one-shot: 1 extra query (unavoidable)
- Unauthed SSE: 1 extra query at stream start, 0 per poll
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: scope anonymous check to only_result branch
The non-only_result branch already enforces the `created_by` check via
its main query, so a top-level hoisted check duplicated work for
unauthenticated default-path callers. Move the check inside the
`if only_result.unwrap_or(false)` block — exactly where the bypass
lives — and leave the non-only_result path untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -7217,6 +7217,7 @@ async fn get_job_update(
|
||||
None,
|
||||
false,
|
||||
&mut false,
|
||||
&mut false,
|
||||
)
|
||||
.await?,
|
||||
))
|
||||
@@ -7307,6 +7308,10 @@ pub fn start_job_update_sse_stream(
|
||||
// Latched once the early_return node's failure is observed alongside a
|
||||
// failure_module — subsequent polls then skip the redundant per-node lookup.
|
||||
let mut early_return_suppressed = false;
|
||||
// Latched once we've verified the job was created by "anonymous" — for
|
||||
// unauthenticated SSE streams, this gates access and is checked once per
|
||||
// stream rather than once per poll (created_by cannot change).
|
||||
let mut anonymous_verified = false;
|
||||
|
||||
// Send initial update immediately
|
||||
let mut running = running;
|
||||
@@ -7331,6 +7336,7 @@ pub fn start_job_update_sse_stream(
|
||||
early_return.as_deref(),
|
||||
has_failure_module,
|
||||
&mut early_return_suppressed,
|
||||
&mut anonymous_verified,
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -7451,6 +7457,7 @@ pub fn start_job_update_sse_stream(
|
||||
early_return.as_deref(),
|
||||
has_failure_module,
|
||||
&mut early_return_suppressed,
|
||||
&mut anonymous_verified,
|
||||
)
|
||||
.await
|
||||
{
|
||||
@@ -7584,6 +7591,7 @@ async fn get_job_update_data(
|
||||
early_return: Option<&str>,
|
||||
has_failure_module: bool,
|
||||
early_return_suppressed: &mut bool,
|
||||
anonymous_verified: &mut bool,
|
||||
) -> error::Result<JobUpdate> {
|
||||
let tags = if log_view {
|
||||
log_job_view(
|
||||
@@ -7605,6 +7613,32 @@ async fn get_job_update_data(
|
||||
let ignore_flow_stream_job_id = is_flow.is_some_and(|x| !x) || flow_stream_job_id.is_some();
|
||||
|
||||
if only_result.unwrap_or(false) {
|
||||
// Unauthenticated callers may only read jobs whose creator is "anonymous".
|
||||
// The non-only_result branch enforces this via `record.created_by` from its
|
||||
// main query, but the only_result branch below fetches solely the result by
|
||||
// (workspace_id, job_id), so we guard here to close the gap. The
|
||||
// `anonymous_verified` flag is preserved across SSE poll iterations so the
|
||||
// lookup only happens once per stream — `created_by` cannot change for a
|
||||
// given job once it has been created.
|
||||
if opt_authed.is_none() && !*anonymous_verified {
|
||||
let created_by = sqlx::query_scalar!(
|
||||
"SELECT created_by FROM v2_job WHERE id = $1 AND workspace_id = $2",
|
||||
job_id,
|
||||
w_id,
|
||||
)
|
||||
.fetch_optional(db)
|
||||
.await?
|
||||
.ok_or_else(|| Error::NotFound(format!("Job not found: {}", job_id)))?;
|
||||
|
||||
if created_by != "anonymous" {
|
||||
return Err(Error::BadRequest(
|
||||
"As a non logged in user, you can only see jobs ran by anonymous users"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
*anonymous_verified = true;
|
||||
}
|
||||
|
||||
let (result, running, mut result_stream, mut new_stream_offset, new_flow_stream_job_id) =
|
||||
if let Some(tags) = tags {
|
||||
let r = sqlx::query!(
|
||||
|
||||
Reference in New Issue
Block a user