fix: add authorization checks to slack/teams approval-minting endpoints

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UrDsBckALYs4yBguik1Pfa

backend/ee-repo-ref.txt

@@ -1 +1 @@
-2ba6a2a75b6fc97858b306b2c98ada481e363c10
+ddafe452fc04b3ba118b11a0578b6ac85e8e0fa6

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<String> {
+pub(crate) async fn resume_target_flow_path(
+    db: &DB,
+    w_id: &str,
+    job_id: Uuid,
+) -> error::Result<String> {
     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<JobUpdateQuery>,
 ) -> JsonResult<JobUpdate> {
     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,

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<DB>,
+    Extension(user_db): Extension<UserDB>,
     Path((w_id, job_id)): Path<(String, Uuid)>,
     Query(approver): Query<QueryApprover>,
     Query(message): Query<QueryMessage>,
@@ -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)
 }