fix: enforce permissions on hash-addressed script reads

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

backend/.sqlx/query-3a85abf71e6b3008f92528bd43b9bb8113b163a635b5480911b7aab625d892b9.json

@@ -1,20 +1,25 @@
 {
   "db_name": "PostgreSQL",
-  "query": "SELECT s.lock, s.lock_error_logs, dm.job_id\n         FROM script s\n         LEFT JOIN deployment_metadata dm ON s.hash = dm.script_hash AND s.workspace_id = dm.workspace_id\n         WHERE s.hash = $1 AND s.workspace_id = $2",
+  "query": "SELECT s.path, s.lock, s.lock_error_logs, dm.job_id\n         FROM script s\n         LEFT JOIN deployment_metadata dm ON s.hash = dm.script_hash AND s.workspace_id = dm.workspace_id\n         WHERE s.hash = $1 AND s.workspace_id = $2",
   "describe": {
     "columns": [
       {
         "ordinal": 0,
+        "name": "path",
+        "type_info": "Varchar"
+      },
+      {
+        "ordinal": 1,
         "name": "lock",
         "type_info": "Text"
       },
       {
-        "ordinal": 1,
+        "ordinal": 2,
         "name": "lock_error_logs",
         "type_info": "Text"
       },
       {
-        "ordinal": 2,
+        "ordinal": 3,
         "name": "job_id",
         "type_info": "Uuid"
       }
@@ -26,10 +31,11 @@
       ]
     },
     "nullable": [
+      false,
       true,
       true,
       true
     ]
   },
-  "hash": "513ed713afdbafb587026d1536c47a9bbaa6967e36777454746b8817d68219a5"
+  "hash": "3a85abf71e6b3008f92528bd43b9bb8113b163a635b5480911b7aab625d892b9"
 }

backend/.sqlx/query-9b35fc71fa5994aa7db3b190c7389648ca885dc87ff1b105a35ba23572598504.json

@@ -0,0 +1,23 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "SELECT EXISTS(SELECT 1 FROM script WHERE hash = $1 AND workspace_id = $2)",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "exists",
+        "type_info": "Bool"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Int8",
+        "Text"
+      ]
+    },
+    "nullable": [
+      null
+    ]
+  },
+  "hash": "9b35fc71fa5994aa7db3b190c7389648ca885dc87ff1b105a35ba23572598504"
+}

backend/windmill-api-flows/src/flows.rs

@@ -1554,11 +1554,13 @@ struct DeploymentStatus {
     job_id: Option<sqlx::types::Uuid>,
 }
 async fn get_deployment_status(
-    Extension(db): Extension<DB>,
+    authed: ApiAuthed,
+    Extension(user_db): Extension<UserDB>,
     Path((w_id, path)): Path<(String, StripPath)>,
 ) -> JsonResult<DeploymentStatus> {
     let path = path.to_path();
-    let mut tx = db.begin().await?;
+    check_scopes(&authed, || format!("flows:read:{}", path))?;
+    let mut tx = user_db.begin(&authed).await?;
     let status_o = sqlx::query!(
         "SELECT f.lock_error_logs, dm.job_id
          FROM flow f

backend/windmill-api-integration-tests/tests/scripts.rs

@@ -1,5 +1,6 @@
 use serde_json::json;
 use sqlx::{Pool, Postgres};
+use std::collections::HashMap;
 
 use windmill_test_utils::*;
 
@@ -561,3 +562,121 @@ async fn test_list_search_scope_filtering(db: Pool<Postgres>) -> anyhow::Result<
 
     Ok(())
 }
+
+/// The hash-addressed script endpoints must enforce the same folder ACLs and token
+/// scopes as their path-addressed counterparts: a hash is an alternate address for a
+/// script, never a way around its permissions.
+#[sqlx::test(migrations = "../migrations", fixtures("base"))]
+async fn test_by_hash_endpoints_enforce_acls(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+    let base = format!("http://localhost:{port}/api/w/test-workspace/scripts");
+
+    // `allowed` grants test-user-2 read; `private` grants it nothing.
+    for (folder, extra_perms) in [
+        ("allowed", json!({ "u/test-user-2": false })),
+        ("private", json!({})),
+    ] {
+        let resp = authed(client().post(format!(
+            "http://localhost:{port}/api/w/test-workspace/folders/create"
+        )))
+        .json(&json!({ "name": folder, "extra_perms": extra_perms }))
+        .send()
+        .await
+        .unwrap();
+        assert_eq!(resp.status(), 200, "create folder: {}", resp.text().await?);
+    }
+
+    let mut hashes = HashMap::new();
+    for (path, content) in [
+        (
+            "f/allowed/foo",
+            "export async function main() { return 'allowed'; }",
+        ),
+        (
+            "f/private/bar",
+            "export async function main() { return 'secret'; }",
+        ),
+    ] {
+        let resp = authed(client().post(format!("{base}/create")))
+            .json(&new_script(path, "summary", content))
+            .send()
+            .await
+            .unwrap();
+        assert_eq!(resp.status(), 201, "create {path}: {}", resp.text().await?);
+        let resp = authed_get(port, "get/p", path).await;
+        assert_eq!(resp.status(), 200);
+        let body = resp.json::<serde_json::Value>().await?;
+        hashes.insert(path, body["hash"].as_str().unwrap().to_string());
+    }
+
+    // A token scoped to `f/allowed/*` for the same super-admin user: RLS lets it through,
+    // only the per-path scope check can stop it.
+    sqlx::query(
+        "INSERT INTO token (token_hash, token_prefix, token, email, label, super_admin, scopes) VALUES
+         (encode(sha256('SCOPED_TOKEN'::bytea), 'hex'), 'SCOPED_TOK', 'SCOPED_TOKEN', 'test@windmill.dev', 'scoped', true, ARRAY['scripts:read:f/allowed/*'])",
+    )
+    .execute(&db)
+    .await?;
+
+    async fn get_as(port: u16, token: &str, endpoint: &str, path: &str) -> reqwest::Response {
+        client()
+            .get(script_url(port, endpoint, path))
+            .header("Authorization", format!("Bearer {token}"))
+            .send()
+            .await
+            .unwrap()
+    }
+
+    let private = &hashes["f/private/bar"];
+    let allowed = &hashes["f/allowed/foo"];
+
+    for token in ["SECRET_TOKEN_2", "SCOPED_TOKEN"] {
+        for (endpoint, hash) in [
+            ("get/h", private.clone()),
+            ("raw/h", format!("{private}.ts")),
+            ("deployment_status/h", private.clone()),
+        ] {
+            let resp = get_as(port, token, endpoint, &hash).await;
+            let status = resp.status();
+            let body = resp.text().await?;
+            assert!(
+                !status.is_success(),
+                "{token} must not reach {endpoint} of f/private/bar (got {status}: {body})"
+            );
+            assert!(
+                !body.contains("return 'secret'"),
+                "{token} must not read f/private/bar source via {endpoint}: {body}"
+            );
+        }
+    }
+
+    // Denying an RLS-invisible script must not read as "this script does not exist":
+    // legitimate viewers reach these endpoints for scripts they cannot read.
+    let resp = get_as(port, "SECRET_TOKEN_2", "raw/h", &format!("{private}.ts")).await;
+    let body = resp.text().await?;
+    assert!(
+        body.contains("does not have permissions to access it"),
+        "denial must name the permission problem, got: {body}"
+    );
+
+    // The grant/scope that does cover `f/allowed/*` must keep working by hash.
+    for token in ["SECRET_TOKEN_2", "SCOPED_TOKEN"] {
+        for (endpoint, hash) in [
+            ("get/h", allowed.clone()),
+            ("raw/h", format!("{allowed}.ts")),
+            ("deployment_status/h", allowed.clone()),
+        ] {
+            let resp = get_as(port, token, endpoint, &hash).await;
+            let status = resp.status();
+            let body = resp.text().await?;
+            assert_eq!(
+                status, 200,
+                "{token} must still reach {endpoint} of f/allowed/foo (got {status}: {body})"
+            );
+        }
+    }
+
+    Ok(())
+}

backend/windmill-api-scripts/src/scripts.rs

@@ -3219,24 +3219,50 @@ async fn get_script_by_hash_internal<'c>(
     Ok(script)
 }
 
-#[derive(Deserialize)]
-struct GetScriptByHashQuery {
-    authed: Option<bool>,
+/// A hash-addressed read that misses under RLS is ambiguous: the hash may name no
+/// script, or one in a folder the caller has no grant on. Legitimate callers land here
+/// (a flow step, an app component job, or a shared run can all reference a script the
+/// viewer cannot read), so report which of the two it is rather than a bare "not found".
+/// Disclosing existence costs nothing a caller who already holds the hash does not have,
+/// and mirrors what `raw/p` reports for paths.
+async fn explain_hash_miss(
+    err: Error,
+    db: &DB,
+    w_id: &str,
+    hash: &ScriptHash,
+    authed: &ApiAuthed,
+) -> Error {
+    if !matches!(err, Error::NotFound(_)) {
+        return err;
+    }
+    let exists = sqlx::query_scalar!(
+        "SELECT EXISTS(SELECT 1 FROM script WHERE hash = $1 AND workspace_id = $2)",
+        hash.0,
+        w_id,
+    )
+    .fetch_one(db)
+    .await;
+    match exists {
+        Ok(Some(true)) => Error::NotFound(format!(
+            "Script with hash {hash} exists but {} does not have permissions to access it",
+            authed.username
+        )),
+        _ => err,
+    }
 }
+
 async fn get_script_by_hash(
     Extension(db): Extension<DB>,
     Extension(user_db): Extension<UserDB>,
     Path((w_id, hash)): Path<(String, ScriptHash)>,
     Query(query): Query<WithStarredInfoQuery>,
-    Query(query_auth): Query<GetScriptByHashQuery>,
     Extension(authed): Extension<ApiAuthed>,
 ) -> JsonResult<ScriptWithStarred<ScriptRunnableSettingsInline>> {
-    let mut tx = if query_auth.authed.is_some_and(|x| x) {
-        user_db.begin(&authed).await?
-    } else {
-        db.begin().await?
-    };
-    let r = get_script_by_hash_internal(
+    // Addressing a script by hash must not be a way around the folder ACLs that
+    // `get/p` enforces: the fetch always goes through RLS, and `scripts:read:<path>`
+    // is checked once the row (hence the path) is known.
+    let mut tx = user_db.begin(&authed).await?;
+    let r = match get_script_by_hash_internal(
         &mut tx,
         &w_id,
         &hash,
@@ -3248,7 +3274,11 @@ async fn get_script_by_hash(
             }
         }),
     )
-    .await?;
+    .await
+    {
+        Ok(r) => r,
+        Err(e) => return Err(explain_hash_miss(e, &db, &w_id, &hash, &authed).await),
+    };
 
     check_scopes(&authed, || format!("scripts:read:{}", &r.script.path))?;
 
@@ -3260,16 +3290,23 @@ async fn get_script_by_hash(
 }
 
 async fn raw_script_by_hash(
+    authed: ApiAuthed,
     Extension(db): Extension<DB>,
+    Extension(user_db): Extension<UserDB>,
     Path((w_id, hash_str)): Path<(String, String)>,
 ) -> Result<String> {
-    let mut tx = db.begin().await?;
     let hash = ScriptHash(to_i64(hash_str.strip_suffix(".ts").ok_or_else(|| {
         Error::BadRequest("Raw script path must end with .ts".to_string())
     })?)?);
-    let r = get_script_by_hash_internal(&mut tx, &w_id, &hash, None).await?;
+    let mut tx = user_db.begin(&authed).await?;
+    let r = match get_script_by_hash_internal(&mut tx, &w_id, &hash, None).await {
+        Ok(r) => r,
+        Err(e) => return Err(explain_hash_miss(e, &db, &w_id, &hash, &authed).await),
+    };
     tx.commit().await?;
 
+    check_scopes(&authed, || format!("scripts:read:{}", &r.script.path))?;
+
     Ok(r.script.content)
 }
 
@@ -3280,12 +3317,14 @@ struct DeploymentStatus {
     job_id: Option<sqlx::types::Uuid>,
 }
 async fn get_deployment_status(
+    authed: ApiAuthed,
     Extension(db): Extension<DB>,
+    Extension(user_db): Extension<UserDB>,
     Path((w_id, hash)): Path<(String, ScriptHash)>,
 ) -> JsonResult<DeploymentStatus> {
-    let mut tx = db.begin().await?;
+    let mut tx = user_db.begin(&authed).await?;
     let status_o = sqlx::query!(
-        "SELECT s.lock, s.lock_error_logs, dm.job_id
+        "SELECT s.path, s.lock, s.lock_error_logs, dm.job_id
          FROM script s
          LEFT JOIN deployment_metadata dm ON s.hash = dm.script_hash AND s.workspace_id = dm.workspace_id
          WHERE s.hash = $1 AND s.workspace_id = $2",
@@ -3295,7 +3334,13 @@ async fn get_deployment_status(
     .fetch_optional(&mut *tx)
     .await?;
 
-    let status = not_found_if_none(status_o, "DeploymentStatus", hash.to_string())?;
+    let status = match not_found_if_none(status_o, "DeploymentStatus", hash.to_string()) {
+        Ok(s) => s,
+        Err(e) => return Err(explain_hash_miss(e, &db, &w_id, &hash, &authed).await),
+    };
+    tx.commit().await?;
+
+    check_scopes(&authed, || format!("scripts:read:{}", &status.path))?;
 
     let deployment_status = DeploymentStatus {
         lock: status.lock,
@@ -3303,7 +3348,6 @@ async fn get_deployment_status(
         job_id: status.job_id,
     };
 
-    tx.commit().await?;
     Ok(Json(deployment_status))
 }
 

backend/windmill-api/openapi.yaml

@@ -9813,10 +9813,6 @@ paths:
           in: query
           schema:
             type: boolean
-        - name: authed
-          in: query
-          schema:
-            type: boolean
       responses:
         "200":
           description: script details

frontend/src/lib/components/flows/content/FlowModuleScript.svelte

@@ -53,6 +53,7 @@
 	let lock: string | undefined = $state(undefined)
 	let date: string | undefined = $state(undefined)
 	let notFound = $state(false)
+	let notFoundReason: string | undefined = $state(undefined)
 
 	function getCachedKey(path: string, hash: string | undefined) {
 		return `${opWs}-${path}-${hash ?? ''}`
@@ -66,6 +67,7 @@
 		previousCode = cachedValues[key]?.previousCode
 		tag = cachedValues[key]?.tag
 		notFound = cachedValues[key]?.notFound ?? false
+		notFoundReason = undefined
 	}
 
 	getCachedValues(
@@ -97,6 +99,7 @@
 	async function loadCode(path: string, hash: string | undefined) {
 		try {
 			notFound = false
+			notFoundReason = undefined
 			const script = path.startsWith('hub/')
 				? await getScriptByPath(path!)
 				: hash
@@ -119,6 +122,9 @@
 			}
 		} catch (e) {
 			notFound = true
+			// A step can reference a script the viewer has no read grant on, so show the
+			// server's reason rather than always claiming the script is missing.
+			notFoundReason = e?.body
 			console.error(e)
 		}
 	}
@@ -140,7 +146,9 @@
 		<div class="text-xs text-primary mb-4">tag: {tag}</div>
 	{/if}
 	{#if notFound}
-		<div class="text-red-400">script not found at {path} in workspace {opWs}</div>
+		<div class="text-red-400">
+			{notFoundReason ?? `script not found at ${path} in workspace ${opWs}`}
+		</div>
 	{:else if showAllCode}
 		{#if showDiff}
 			{#key (previousCode ?? '') + (code ?? '')}

frontend/src/routes/(root)/(logged)/run/[...run]/+page.svelte

@@ -299,11 +299,13 @@
 			job.script_hash &&
 			persistentScriptDefinition === undefined
 		) {
+			// A job can be visible to someone with no read permission on its script, in
+			// which case the persistent-script banner is simply not shown.
 			const script = await ScriptService.getScriptByHash({
 				workspace: $workspaceStore!,
 				hash: job.script_hash
-			})
-			if (script.restart_unless_cancelled ?? false) {
+			}).catch(() => undefined)
+			if (script?.restart_unless_cancelled ?? false) {
 				persistentScriptDefinition = script
 			}
 		}

frontend/src/routes/(root)/(logged)/scripts/get/[...hash]/+page.svelte

@@ -246,8 +246,7 @@
 			script = await ScriptService.getScriptByHash({
 				workspace: $workspaceStore!,
 				hash,
-				withStarredInfo: true,
-				authed: true
+				withStarredInfo: true
 			})
 		} catch {
 			try {