diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 589a683dba..9826560391 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -2ba6a2a75b6fc97858b306b2c98ada481e363c10 +ddafe452fc04b3ba118b11a0578b6ac85e8e0fa6 diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 14cbb76c91..e7a6ad7c8f 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -448,7 +448,7 @@ async fn get_result_by_id( // Reading a node's result requires being able to read the flow itself (the node // belongs to it). Gate on the flow's visibility (created_by / RLS / root // inheritance) before resolving via the root DB. - require_job_update_read_access( + require_job_read_access_by_id( &db, &user_db, &authed, @@ -481,7 +481,7 @@ async fn get_job_view_token( // link cannot be used to mint further links. `require_job_read_access` also // enforces the caller's `if_jobs:filter_tags` scope, so a tag-scoped token can't // mint a transferable link for a job outside its allowed tags. - require_job_update_read_access(&db, &user_db, &authed, &w_id, &id, None).await?; + require_job_read_access_by_id(&db, &user_db, &authed, &w_id, &id, None).await?; let hmac = generate_view_token(&w_id, id, &db).await?; Ok(format!("{id}.{hmac}")) } @@ -1252,9 +1252,9 @@ fn job_read_access_cache_key(authed: &ApiAuthed, w_id: &str, job_id: &Uuid) -> [ hasher.finalize().into() } -/// [`require_job_read_access`] for callers (job-update poll / SSE) that haven't -/// already loaded `created_by` — fetches it (root DB, by id+workspace) first. -async fn require_job_update_read_access( +/// [`require_job_read_access`] for callers that haven't already loaded +/// `created_by` — fetches it (root DB, by id+workspace) first. +pub(crate) async fn require_job_read_access_by_id( db: &DB, user_db: &UserDB, authed: &ApiAuthed, @@ -4332,7 +4332,11 @@ pub async fn get_resume_urls_internal( /// unknown job); an empty path only matters for path-restricted tokens, which /// would not be running such a flow. Never hard-fails, so it can't break resume /// for unscoped tokens (the in-flow `get_resume_urls()` path). -async fn resume_target_flow_path(db: &DB, w_id: &str, job_id: Uuid) -> error::Result { +pub(crate) async fn resume_target_flow_path( + db: &DB, + w_id: &str, + job_id: Uuid, +) -> error::Result { let job = sqlx::query!( r#"SELECT kind::text as "kind!", parent_job, runnable_path FROM v2_job WHERE id = $1 AND workspace_id = $2"#, @@ -8093,7 +8097,7 @@ async fn get_job_update( }): Query, ) -> JsonResult { if let Some(authed) = opt_authed.as_ref() { - require_job_update_read_access( + require_job_read_access_by_id( &db, &user_db, authed, @@ -8151,7 +8155,7 @@ async fn get_job_update_sse( // Authorize once at connection time; `created_by` cannot change for a given job, // mirroring the per-stream `anonymous_verified` latch in the streaming loop. if let Some(authed) = opt_authed.as_ref() { - require_job_update_read_access( + require_job_read_access_by_id( &db, &user_db, authed, diff --git a/backend/windmill-api/src/slack_approvals.rs b/backend/windmill-api/src/slack_approvals.rs index 593ea3b298..c13486f5a0 100644 --- a/backend/windmill-api/src/slack_approvals.rs +++ b/backend/windmill-api/src/slack_approvals.rs @@ -12,11 +12,15 @@ use serde_json::Value; use sha2::Sha256; use sqlx::types::Uuid; use std::collections::HashMap; +use windmill_common::db::UserDB; use windmill_common::error::{to_anyhow, Error}; use windmill_common::variables::{get_secret_value_as_admin, get_workspace_key}; use crate::db::{ApiAuthed, DB}; -use crate::jobs::{QueryApprover, ResumeUrls}; +use crate::jobs::{ + require_job_read_access_by_id, resume_target_flow_path, QueryApprover, ResumeUrls, +}; +use crate::utils::check_scopes; use crate::{ approvals::{ extract_w_id_from_resume_url, handle_resume_action, ApprovalFormDetails, FieldType, @@ -350,8 +354,9 @@ pub async fn slack_app_callback_handler( } pub async fn request_slack_approval( - _authed: ApiAuthed, + authed: ApiAuthed, Extension(db): Extension, + Extension(user_db): Extension, Path((w_id, job_id)): Path<(String, Uuid)>, Query(approver): Query, Query(message): Query, @@ -366,7 +371,26 @@ pub async fn request_slack_approval( let channel_id = channel_id.channel_id; let flow_step_id = flow_step_id.flow_step_id; - let slack_token = get_slack_token(&db, slack_resource_path.as_str(), &w_id).await?; + // The caller must be able to read the job before this endpoint reads the + // slack resource variable (as admin, below) and posts the approval prompt: + // otherwise any workspace member could mint approvals for jobs they cannot + // see and exfiltrate arbitrary workspace variables to a chosen channel. + require_job_read_access_by_id(&db, &user_db, &authed, &w_id, &job_id, None).await?; + + // The posted message embeds resume/cancel URLs (full resume capability), so a + // scoped token must hold run scope on the suspended job's flow — same rule as + // `get_resume_urls`. No-op for unscoped tokens (incl. the in-flow substep token). + let flow_path = resume_target_flow_path(&db, &w_id, job_id).await?; + check_scopes(&authed, || format!("jobs:run:flows:{}", flow_path))?; + + // Generic error: the raw error echoes the probed variable path back, which + // would be a variable-path existence oracle. Log the detail server-side. + let slack_token = get_slack_token(&db, slack_resource_path.as_str(), &w_id) + .await + .map_err(|e| { + tracing::warn!("Failed to resolve slack token for {w_id}/{slack_resource_path}: {e:#}"); + Error::BadRequest("Invalid Slack approval request".to_string()) + })?; let client = Client::new(); tracing::debug!("Approver: {:?}", approver.approver); @@ -392,7 +416,13 @@ pub async fn request_slack_approval( button_text.cancel_button_text.as_deref(), ) .await - .map_err(|e| Error::BadRequest(e.to_string()))?; + .map_err(|e| { + // Same generic error as the token-resolution failure above: a distinct + // send error (e.g. Slack rejecting a non-token variable) would otherwise + // still disclose whether the probed variable path exists. + tracing::warn!("Failed to send slack approval message for {w_id}/{job_id}: {e:#}"); + Error::BadRequest("Invalid Slack approval request".to_string()) + })?; Ok(StatusCode::OK) }