fix: keep ACL grants out of resource and variable permission-denied errors

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

backend/.sqlx/query-108e4c505168381b51ad298b5294aa73e84d35bb68a5911985139bbf94c1d231.json

@@ -1,23 +0,0 @@
-{
-  "db_name": "PostgreSQL",
-  "query": "SELECT extra_perms from resource WHERE path = $1 AND workspace_id = $2",
-  "describe": {
-    "columns": [
-      {
-        "ordinal": 0,
-        "name": "extra_perms",
-        "type_info": "Jsonb"
-      }
-    ],
-    "parameters": {
-      "Left": [
-        "Text",
-        "Text"
-      ]
-    },
-    "nullable": [
-      false
-    ]
-  },
-  "hash": "108e4c505168381b51ad298b5294aa73e84d35bb68a5911985139bbf94c1d231"
-}

backend/.sqlx/query-7813908c080e239fd6b9df3711fd704d6bb5d5686a02b72de2cb61e3c127cb54.json

@@ -1,23 +0,0 @@
-{
-  "db_name": "PostgreSQL",
-  "query": "SELECT extra_perms from folder WHERE name = $1 AND workspace_id = $2",
-  "describe": {
-    "columns": [
-      {
-        "ordinal": 0,
-        "name": "extra_perms",
-        "type_info": "Jsonb"
-      }
-    ],
-    "parameters": {
-      "Left": [
-        "Text",
-        "Text"
-      ]
-    },
-    "nullable": [
-      false
-    ]
-  },
-  "hash": "7813908c080e239fd6b9df3711fd704d6bb5d5686a02b72de2cb61e3c127cb54"
-}

backend/.sqlx/query-a90924854cbd98bb16a2ed02e7966b0726e11ddd7511f1d503b1df29dee71419.json

@@ -1,23 +0,0 @@
-{
-  "db_name": "PostgreSQL",
-  "query": "SELECT extra_perms from variable WHERE path = $1 AND workspace_id = $2",
-  "describe": {
-    "columns": [
-      {
-        "ordinal": 0,
-        "name": "extra_perms",
-        "type_info": "Jsonb"
-      }
-    ],
-    "parameters": {
-      "Left": [
-        "Text",
-        "Text"
-      ]
-    },
-    "nullable": [
-      false
-    ]
-  },
-  "hash": "a90924854cbd98bb16a2ed02e7966b0726e11ddd7511f1d503b1df29dee71419"
-}

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

@@ -566,6 +566,39 @@ async fn test_resource_value_cache_is_identity_scoped(db: Pool<Postgres>) -> any
     Ok(())
 }
 
+/// A denial confirms the resource exists but must not reveal who holds grants on it or on
+/// its folder: test-user-2's grants must never appear in test-user-3's error.
+#[sqlx::test(migrations = "../migrations", fixtures("base", "resource_cache_rls"))]
+async fn test_resource_denial_hides_grants(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    sqlx::query(
+        "UPDATE resource SET extra_perms = '{\"u/test-user-2\": true}'
+         WHERE workspace_id = 'test-workspace' AND path = 'f/secret/cache_target'",
+    )
+    .execute(&db)
+    .await?;
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+
+    for endpoint in ["get_value", "get_value_interpolated"] {
+        let resp = client()
+            .get(resource_url(port, endpoint, "f/secret/cache_target"))
+            .header("Authorization", "Bearer SECRET_TOKEN_3")
+            .send()
+            .await?;
+        assert_eq!(resp.status(), 401);
+        let body = resp.text().await?;
+        assert!(
+            !body.contains("test-user-2"),
+            "{endpoint} leaked grants: {body}"
+        );
+        assert!(body.contains("exists but you don't have access"), "{body}");
+        assert!(body.contains("folder secret"), "{body}");
+    }
+
+    Ok(())
+}
+
 /// A resource whose value contains a `$WM_*` contextual variable (e.g. `$WM_TOKEN`) is
 /// job-dependent and must NEVER be cached — even when first read WITHOUT a `job_id`, where the
 /// placeholder is left unresolved (caching that would serve a stale placeholder to a later job

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

@@ -282,6 +282,39 @@ async fn test_variable_value_cache_is_identity_scoped(db: Pool<Postgres>) -> any
     Ok(())
 }
 
+/// A denial confirms the variable exists but must not reveal who holds grants on it or on
+/// its folder: test-user-2's grants must never appear in test-user-3's error.
+#[sqlx::test(migrations = "../migrations", fixtures("base", "variable_cache_rls"))]
+async fn test_variable_denial_hides_grants(db: Pool<Postgres>) -> anyhow::Result<()> {
+    initialize_tracing().await;
+    sqlx::query(
+        "UPDATE variable SET extra_perms = '{\"u/test-user-2\": true}'
+         WHERE workspace_id = 'test-workspace' AND path = 'f/secret/cache_target_var'",
+    )
+    .execute(&db)
+    .await?;
+    let server = ApiServer::start(db.clone()).await?;
+    let port = server.addr.port();
+
+    for endpoint in ["get", "get_value"] {
+        let resp = client()
+            .get(variable_url(port, endpoint, "f/secret/cache_target_var"))
+            .header("Authorization", "Bearer SECRET_TOKEN_3")
+            .send()
+            .await?;
+        assert_eq!(resp.status(), 401);
+        let body = resp.text().await?;
+        assert!(
+            !body.contains("test-user-2"),
+            "{endpoint} leaked grants: {body}"
+        );
+        assert!(body.contains("exists but you don't have access"), "{body}");
+        assert!(body.contains("folder secret"), "{body}");
+    }
+
+    Ok(())
+}
+
 /// Secret variables ARE cached (with their per-read side effects — the EE
 /// `variables.decrypt_secret` audit and running-job secret registration — re-run on every
 /// hit; that re-emission is not observable in the OSS build since `audit_log` is a no-op).

backend/windmill-store/src/lib.rs

@@ -17,3 +17,34 @@ pub mod variables;
 
 #[cfg(all(test, feature = "oauth2", feature = "private", feature = "enterprise"))]
 mod oauth_refresh_secret_backend_tests;
+
+use windmill_common::{db::Authable, error::Error};
+
+/// Denial message for an item the caller can see the existence of but not read. It must never
+/// describe the grants themselves: `extra_perms` says which users and groups hold read/write,
+/// which the folder ACL model restricts to folder owners, and the caller reaching here is by
+/// definition not one. The acting identity is named because a job started on behalf of another
+/// user is authorized as that user, so a denial that omits it reads as a grant bug.
+pub(crate) fn perm_denied_error(
+    kind: &str,
+    path: &str,
+    authed: Option<&(impl Authable + Sync)>,
+) -> Error {
+    let requester = match authed {
+        Some(authed) => format!("{} ({})", authed.username(), authed.email()),
+        None => "an unauthenticated caller".to_string(),
+    };
+    let folder = path
+        .strip_prefix("f/")
+        .and_then(|rest| rest.split('/').next())
+        .filter(|folder| !folder.is_empty());
+    match folder {
+        Some(folder) => Error::NotAuthorized(format!(
+            "{kind} {path} exists but you don't have access to it, requested as {requester}. \
+             Ask an owner of folder {folder} to grant you access."
+        )),
+        None => Error::NotAuthorized(format!(
+            "{kind} {path} exists but you don't have access to it, requested as {requester}."
+        )),
+    }
+}

backend/windmill-store/src/resources.rs

@@ -651,38 +651,18 @@ pub async fn explain_resource_perm_error(
     db: &sqlx::Pool<Postgres>,
     authed: &ApiAuthed,
 ) -> windmill_common::error::Result<()> {
-    let extra_perms = sqlx::query_scalar!(
-        "SELECT extra_perms from resource WHERE path = $1 AND workspace_id = $2",
+    let exists = sqlx::query_scalar!(
+        "SELECT EXISTS(SELECT 1 FROM resource WHERE path = $1 AND workspace_id = $2)",
         path,
         w_id
     )
-    .fetch_optional(db)
+    .fetch_one(db)
     .await?
-    .ok_or_else(|| Error::NotFound(format!("Resource {} not found", path)))?;
-    if path.starts_with("f/") {
-        let folder = path.split("/").nth(1).ok_or_else(|| {
-            Error::BadRequest(format!(
-                "path {} should have at least 2 components separated by /",
-                path
-            ))
-        })?;
-        let folder_extra_perms = sqlx::query_scalar!(
-            "SELECT extra_perms from folder WHERE name = $1 AND workspace_id = $2",
-            folder,
-            w_id
-        )
-        .fetch_optional(db)
-        .await?;
-        return Err(Error::NotAuthorized(format!(
-            "Resource exists but you don't have access to it:\nresource perms: {}\nfolder perms: {}\nauthed as: {authed:?}",
-            serde_json::to_string_pretty(&extra_perms).unwrap_or_default(), serde_json::to_string_pretty(&folder_extra_perms).unwrap_or_default()
-        )));
-    } else {
-        return Err(Error::NotAuthorized(format!(
-            "Resource exists but you don't have access to it:\nresource perms: {}\nauthed as: {authed:?}",
-            serde_json::to_string_pretty(&extra_perms).unwrap_or_default()
-        )));
+    .unwrap_or(false);
+    if !exists {
+        return Err(Error::NotFound(format!("Resource {} not found", path)));
     }
+    Err(crate::perm_denied_error("Resource", path, Some(authed)))
 }
 
 async fn custom_component(

backend/windmill-store/src/variables.rs

@@ -463,61 +463,24 @@ async fn get_value(
         .map(Json);
 }
 
-/// The grants alone can't explain a denial: a job started on behalf of another
-/// user is authorized as that user, so the error has to name who was actually
-/// asking or it reads as a grant bug.
-fn describe_authed(authed: Option<&(impl Authable + Sync)>) -> String {
-    match authed {
-        Some(authed) => format!(
-            "username: {}, email: {}, groups: {:?}, folders: {:?}",
-            authed.username(),
-            authed.email(),
-            authed.groups(),
-            authed.folders()
-        ),
-        None => "unauthenticated".to_string(),
-    }
-}
-
 async fn explain_variable_perm_error(
     path: &str,
     w_id: &str,
     db: &sqlx::Pool<Postgres>,
     authed: Option<&(impl Authable + Sync)>,
 ) -> windmill_common::error::Result<()> {
-    let extra_perms = sqlx::query_scalar!(
-        "SELECT extra_perms from variable WHERE path = $1 AND workspace_id = $2",
+    let exists = sqlx::query_scalar!(
+        "SELECT EXISTS(SELECT 1 FROM variable WHERE path = $1 AND workspace_id = $2)",
         path,
         w_id
     )
-    .fetch_optional(db)
+    .fetch_one(db)
     .await?
-    .ok_or_else(|| Error::NotFound(format!("Variable {} not found", path)))?;
-    if path.starts_with("f/") {
-        let folder = path.split("/").nth(1).ok_or_else(|| {
-            Error::BadRequest(format!(
-                "path {} should have at least 2 components separated by /",
-                path
-            ))
-        })?;
-        let folder_extra_perms = sqlx::query_scalar!(
-            "SELECT extra_perms from folder WHERE name = $1 AND workspace_id = $2",
-            folder,
-            w_id
-        )
-        .fetch_optional(db)
-        .await?;
-        return Err(Error::NotAuthorized(format!(
-            "Variable exists but you don't have access to it:\nvariable perms: {}\nfolder perms: {}\nauthed as: {}",
-            serde_json::to_string_pretty(&extra_perms).unwrap_or_default(), serde_json::to_string_pretty(&folder_extra_perms).unwrap_or_default(), describe_authed(authed)
-        )));
-    } else {
-        return Err(Error::NotAuthorized(format!(
-            "Variable exists but you don't have access to it:\nvariable perms: {}\nauthed as: {}",
-            serde_json::to_string_pretty(&extra_perms).unwrap_or_default(),
-            describe_authed(authed)
-        )));
+    .unwrap_or(false);
+    if !exists {
+        return Err(Error::NotFound(format!("Variable {} not found", path)));
     }
+    Err(crate::perm_denied_error("Variable", path, authed))
 }
 
 async fn exists_variable(