fix: authorize every runnable a builder composes, not just pinned versions

CI review found the general case behind the pinned-hash fix. `script_to_payload`
resolves an un-pinned step path with the root DB handle and returns the
referenced runnable's `on_behalf_of`, which the step job then adopts. Composing
a path is therefore enough to run it, and to run it as whoever it runs as, so
the previous commit closed only the narrow variant.

- The walk now reports every referenced workspace runnable, and
  `validate_operator_composed_flow` checks each under the caller's RLS. The
  pinned `(path, hash)` comparison stays on top: the dispatch ignores the path
  beside a hash, so a readable path paired with another script's hash still runs
  that other script.
- Same reasoning for apps: a policy triggerable is what `execute_component` will
  resolve, also with the root DB handle, so an unreadable path there means an
  admin who merely opens the app runs code the builder could not see, as
  themselves. `validate_operator_composed_app` checks every `script/<path>` and
  `flow/<path>` key and refuses hub ones. The value-and-policy half stays a pure
  function so it keeps its unit test.
- `execute_component`'s preview branch refuses a hub path for operators:
  `require_path_read_access_for_preview` admits `hub/` for everyone, and
  `get_payload_tag_from_prefixed_path` then downloads and enqueues it, which is
  exactly the unreviewed code the composition check refuses in a flow.

Also: the app row menu's relaxed entries are gated on the row being a full-code
app, and the notify trigger only fires when `operator_settings` actually
changed, matching its closest sibling.

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

backend/.sqlx/query-3dccc8e745f4a0973541088f66172e74af6828c1ab52cf7a6fd10b305deade85.json

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

backend/.sqlx/query-4bc47050c74a02ab3169c3165898b2af07e995de71564d867b171f97f719fbde.json

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

backend/migrations/20260816212109_notify_operator_settings_change.up.sql

@@ -16,4 +16,5 @@ DROP TRIGGER IF EXISTS operator_settings_change_trigger ON workspace_settings;
 CREATE TRIGGER operator_settings_change_trigger
 AFTER UPDATE OF operator_settings ON workspace_settings
 FOR EACH ROW
+WHEN (OLD.operator_settings IS DISTINCT FROM NEW.operator_settings)
 EXECUTE FUNCTION notify_operator_settings_change();

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

@@ -592,30 +592,56 @@ pub async fn validate_operator_composed_flow(
         .await?;
     }
 
-    // A version-pinned step dispatches on its `hash` alone, reading the row with root permissions
-    // and taking that script's tag and `on_behalf_of` identity; the `path` beside it is never
-    // consulted. So the pair has to be real, and readable by this caller, or a builder pins the
-    // hash of a script it cannot reach and runs that instead.
-    if !refs.pinned_scripts.is_empty() {
-        let mut tx = user_db.clone().begin(authed).await?;
-        for (path, hash) in &refs.pinned_scripts {
-            let exists = sqlx::query_scalar!(
-                "SELECT EXISTS(SELECT 1 FROM script WHERE workspace_id = $1 AND path = $2 AND hash = $3)",
+    if refs.runnables.is_empty() && refs.pinned_scripts.is_empty() {
+        return Ok(());
+    }
+    // Composing a runnable is enough to run it: the worker resolves a step's path with the root DB
+    // handle and adopts that runnable's `on_behalf_of`, so an unreadable path would let a builder
+    // execute code it cannot see, as whoever that code runs as. RLS on this transaction is the
+    // check. A pinned `hash` needs its own comparison on top: the dispatch ignores the path beside
+    // it, so a readable path paired with another script's hash still runs that other script.
+    let mut tx = user_db.clone().begin(authed).await?;
+    for (is_flow, path) in &refs.runnables {
+        let readable = if *is_flow {
+            sqlx::query_scalar!(
+                "SELECT EXISTS(SELECT 1 FROM flow WHERE workspace_id = $1 AND path = $2)",
+                w_id,
+                path,
+            )
+        } else {
+            sqlx::query_scalar!(
+                "SELECT EXISTS(SELECT 1 FROM script WHERE workspace_id = $1 AND path = $2)",
                 w_id,
                 path,
-                hash.0,
             )
-            .fetch_one(&mut *tx)
-            .await?
-            .unwrap_or(false);
-            if !exists {
-                return Err(Error::NotAuthorized(format!(
-                    "Version {hash} is not a readable version of {path}"
-                )));
-            }
         }
-        tx.commit().await?;
+        .fetch_one(&mut *tx)
+        .await?
+        .unwrap_or(false);
+        if !readable {
+            return Err(Error::NotAuthorized(format!(
+                "{} {path} does not exist or is not readable by you",
+                if *is_flow { "Flow" } else { "Script" }
+            )));
+        }
     }
+    for (path, hash) in &refs.pinned_scripts {
+        let exists = sqlx::query_scalar!(
+            "SELECT EXISTS(SELECT 1 FROM script WHERE workspace_id = $1 AND path = $2 AND hash = $3)",
+            w_id,
+            path,
+            hash.0,
+        )
+        .fetch_one(&mut *tx)
+        .await?
+        .unwrap_or(false);
+        if !exists {
+            return Err(Error::NotAuthorized(format!(
+                "Version {hash} is not a readable version of {path}"
+            )));
+        }
+    }
+    tx.commit().await?;
     Ok(())
 }
 

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

@@ -31,6 +31,10 @@ async fn set_builder(db: &Pool<Postgres>, enabled: bool) -> anyhow::Result<()> {
 }
 
 fn composition_flow(path: &str) -> serde_json::Value {
+    composition_flow_at(path, "u/operator/some_script")
+}
+
+fn composition_flow_at(path: &str, step_path: &str) -> serde_json::Value {
     json!({
         "path": path,
         "summary": "",
@@ -38,7 +42,7 @@ fn composition_flow(path: &str) -> serde_json::Value {
         "schema": {},
         "value": {"modules": [{
             "id": "a",
-            "value": {"type": "script", "path": "u/operator/some_script", "input_transforms": {}}
+            "value": {"type": "script", "path": step_path, "input_transforms": {}}
         }]}
     })
 }
@@ -71,6 +75,17 @@ async fn test_operator_builder_rights_boundary(db: Pool<Postgres>) -> anyhow::Re
     let api = format!("http://localhost:{port}/api/w/{WS}");
     let c = operator_client();
 
+    // A composition-only flow references a runnable that exists and the builder can read; the
+    // check now rejects anything else, so the fixture needs one.
+    sqlx::query(
+        "INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, schema,
+             summary, description, lock, extra_perms)
+         VALUES ($1, 4241, 'u/operator/some_script', 'x', 'bun', 'script', 'operator', '{}', '', '', '', '{}')",
+    )
+    .bind(WS)
+    .execute(&db)
+    .await?;
+
     set_builder(&db, false).await?;
     let resp = c
         .post(format!("{api}/flows/create"))
@@ -178,6 +193,27 @@ async fn test_operator_builder_rights_boundary(db: Pool<Postgres>) -> anyhow::Re
         "a builder must not create a low-code app"
     );
 
+    // Composing a runnable is enough to run it: the worker resolves a step's path with the root
+    // DB handle and adopts that runnable's `on_behalf_of`. `permissions_test` gives the operator
+    // fixture no rights on `u/alice/**`.
+    sqlx::query(
+        "INSERT INTO script (workspace_id, hash, path, content, language, kind, created_by, schema,
+             summary, description, lock, extra_perms)
+         VALUES ($1, 4243, 'u/alice/private', 'x', 'bun', 'script', 'alice', '{}', '', '', '', '{}')",
+    )
+    .bind(WS)
+    .execute(&db)
+    .await?;
+    let resp = c
+        .post(format!("{api}/flows/create"))
+        .json(&composition_flow_at("u/operator/f4", "u/alice/private"))
+        .send()
+        .await?;
+    assert!(
+        !resp.status().is_success(),
+        "a builder must not compose a runnable it cannot read"
+    );
+
     // A version-pinned step dispatches on its hash alone, so the pair must be real and readable:
     // otherwise a builder pins the hash of a script it cannot reach and runs that instead.
     sqlx::query(

backend/windmill-api/src/apps.rs

@@ -1901,12 +1901,15 @@ async fn store_raw_app_file<'a>(
 /// so isolation is what makes it safe to let an operator publish one: `sandbox` renders it in an
 /// opaque-origin iframe instead of handing it the viewer's Windmill session. Inline scripts are
 /// the low-code side's way of carrying code and must not appear either.
+/// The value-and-policy half: everything decidable without the DB. Returns the workspace
+/// runnables the policy authorizes the app to invoke, as `(is_flow, path)`, for
+/// [`validate_operator_composed_app`] to authorize against the builder's permissions.
 fn check_operator_composed_app(
     raw_app: bool,
     value: Option<&RawValue>,
     policy: Option<&mut Policy>,
     allow_kind_change: bool,
-) -> Result<()> {
+) -> Result<Vec<(bool, String)>> {
     if !raw_app || allow_kind_change {
         return Err(Error::NotAuthorized(
             "Operators with builder rights can only author full-code apps, and cannot convert an existing app into one".to_string(),
@@ -1948,6 +1951,78 @@ fn check_operator_composed_app(
         ));
     }
     policy.sandbox = Some(true);
+
+    // The triggerables are the deployed app's authorization to invoke a runnable.
+    // `<component>:` prefixes the key when the app scopes it to one component.
+    let mut referenced = policy
+        .triggerables
+        .iter()
+        .flat_map(|t| t.keys())
+        .chain(policy.triggerables_v2.iter().flat_map(|t| t.keys()))
+        .filter_map(|key| {
+            let key = key.split_once(':').map_or(key.as_str(), |(_, rest)| rest);
+            key.strip_prefix("script/")
+                .map(|p| (false, p.to_string()))
+                .or_else(|| key.strip_prefix("flow/").map(|p| (true, p.to_string())))
+        })
+        .collect::<Vec<_>>();
+    referenced.sort();
+    referenced.dedup();
+    for (_, path) in &referenced {
+        if path.starts_with("hub/") {
+            return Err(Error::NotAuthorized(format!(
+                "Operators with builder rights cannot reference the hub runnable {path}. Deploy it to the workspace first."
+            )));
+        }
+    }
+    Ok(referenced)
+}
+
+/// Runs on every app write by an operator with builder rights, from inside the create/update
+/// internals so both raw-app endpoints are covered by one check.
+///
+/// `execute_component` resolves the runnable it picks with the root DB handle, so an unreadable
+/// path in the policy means an admin who merely opens the app runs code the builder could not
+/// see, as themselves. RLS on this transaction is the check.
+async fn validate_operator_composed_app(
+    authed: &ApiAuthed,
+    user_db: &UserDB,
+    w_id: &str,
+    raw_app: bool,
+    value: Option<&RawValue>,
+    policy: Option<&mut Policy>,
+    allow_kind_change: bool,
+) -> Result<()> {
+    let referenced = check_operator_composed_app(raw_app, value, policy, allow_kind_change)?;
+    if referenced.is_empty() {
+        return Ok(());
+    }
+    let mut tx = user_db.clone().begin(authed).await?;
+    for (is_flow, path) in referenced {
+        let readable = if is_flow {
+            sqlx::query_scalar!(
+                "SELECT EXISTS(SELECT 1 FROM flow WHERE workspace_id = $1 AND path = $2)",
+                w_id,
+                path,
+            )
+        } else {
+            sqlx::query_scalar!(
+                "SELECT EXISTS(SELECT 1 FROM script WHERE workspace_id = $1 AND path = $2)",
+                w_id,
+                path,
+            )
+        }
+        .fetch_one(&mut *tx)
+        .await?
+        .unwrap_or(false);
+        if !readable {
+            return Err(Error::NotAuthorized(format!(
+                "{} {path} does not exist or is not readable by you",
+                if is_flow { "Flow" } else { "Script" }
+            )));
+        }
+    }
+    tx.commit().await?;
     Ok(())
 }
 
@@ -2163,7 +2238,16 @@ async fn create_app_internal<'a>(
     check_scopes(&authed, || format!("apps:write:{}", &app.path))?;
     validate_frontend_sdk_scopes(&app.policy)?;
     if authed.is_operator {
-        check_operator_composed_app(raw_app, Some(&app.value.0), Some(&mut app.policy), false)?;
+        validate_operator_composed_app(
+            &authed,
+            &user_db,
+            w_id,
+            raw_app,
+            Some(&app.value.0),
+            Some(&mut app.policy),
+            false,
+        )
+        .await?;
     }
     if *CLOUD_HOSTED {
         let nb_apps =
@@ -3055,12 +3139,16 @@ async fn update_app_internal<'a>(
     }
 
     if authed.is_operator {
-        check_operator_composed_app(
+        validate_operator_composed_app(
+            &authed,
+            &user_db,
+            w_id,
             raw_app,
             ns.value.as_ref().map(|v| v.0.as_ref()),
             ns.policy.as_mut(),
             ns.allow_kind_change.unwrap_or(false),
-        )?;
+        )
+        .await?;
     }
 
     let mut tx = user_db.clone().begin(&authed).await?;
@@ -3569,9 +3657,20 @@ async fn execute_component(
         })?;
         // A builder testing the app it is composing only ever previews a deployed runnable
         // (`path`) or a persisted app script (`id`), both confined below to what it may read.
-        // Inline `raw_code` is authoring code, so it stays closed to every operator.
+        // Inline `raw_code` is authoring code, so it stays closed to every operator. So is a hub
+        // path, which `require_path_read_access_for_preview` admits for everyone and
+        // `get_payload_tag_from_prefixed_path` then downloads and enqueues: it is exactly the
+        // unreviewed code the composition check refuses in a flow.
+        let previews_hub = payload.path.as_deref().is_some_and(|p| {
+            p.strip_prefix("script/")
+                .or_else(|| p.strip_prefix("flow/"))
+                .unwrap_or(p)
+                .starts_with("hub/")
+        });
         if authed.is_operator
-            && (payload.raw_code.is_some() || !operator_builder_enabled(&db, &w_id).await?)
+            && (payload.raw_code.is_some()
+                || previews_hub
+                || !operator_builder_enabled(&db, &w_id).await?)
         {
             return Err(Error::NotAuthorized(
                 "Operators cannot run preview jobs for security reasons".to_string(),
@@ -5607,7 +5706,7 @@ mod operator_app_tests {
     fn composed_app(
         value: serde_json::Value,
         policy: &mut Policy,
-    ) -> Result<()> {
+    ) -> Result<Vec<(bool, String)>> {
         let value = to_raw_value(&value);
         check_operator_composed_app(true, Some(&value), Some(policy), false)
     }
@@ -5620,8 +5719,16 @@ mod operator_app_tests {
 
         // A composition-only app goes through, sandboxed whether or not it asked to be.
         let mut policy = builder_policy(serde_json::json!({"a:script/f/x/s": {"static_inputs": {}, "one_of_inputs": {}}}));
-        composed_app(clean.clone(), &mut policy).unwrap();
+        let referenced = composed_app(clean.clone(), &mut policy).unwrap();
         assert_eq!(policy.sandbox, Some(true));
+        // The runnables the policy authorizes are handed back for the caller to authorize.
+        assert_eq!(referenced, vec![(false, "f/x/s".to_string())]);
+
+        // A hub reference is unreviewed code, refused like it is in a flow.
+        let mut policy = builder_policy(
+            serde_json::json!({"a:script/hub/1/x": {"static_inputs": {}, "one_of_inputs": {}}}),
+        );
+        assert!(composed_app(clean.clone(), &mut policy).is_err());
 
         let mut policy = builder_policy(serde_json::json!({}));
         policy.sandbox = Some(false);

backend/windmill-common/src/flows.rs

@@ -271,9 +271,12 @@ pub fn check_flow_is_composition_only(value: &FlowValue) -> Result<ComposedFlowR
 #[derive(Default)]
 pub struct ComposedFlowRefs {
     pub tags: Vec<String>,
+    /// Every workspace runnable a step references, as `(is_flow, path)`. The worker resolves
+    /// these with the root DB handle and adopts the referenced runnable's `on_behalf_of`, so
+    /// composing a path is enough to run it, and to run it as whoever it runs as.
+    pub runnables: Vec<(bool, String)>,
     /// Version-pinned script steps. A step carrying a `hash` is dispatched by that hash alone,
-    /// with the path ignored and the row read with root permissions, so an unverified pair runs
-    /// some other script's code under some other script's `on_behalf_of` identity.
+    /// with the path beside it ignored, so the pair has to be checked on top of the path.
     pub pinned_scripts: Vec<(String, ScriptHash)>,
 }
 
@@ -322,11 +325,15 @@ fn check_module_value_is_composition_only(
         FlowModuleValue::Script { path, hash, tag_override, .. } => {
             check_composable_path(path, id)?;
             push_tag(tag_override);
+            refs.runnables.push((false, path.clone()));
             if let Some(hash) = hash {
                 refs.pinned_scripts.push((path.clone(), *hash));
             }
         }
-        FlowModuleValue::Flow { path, .. } => check_composable_path(path, id)?,
+        FlowModuleValue::Flow { path, .. } => {
+            check_composable_path(path, id)?;
+            refs.runnables.push((true, path.clone()));
+        }
         FlowModuleValue::ForloopFlow { modules, modules_node, .. }
         | FlowModuleValue::WhileloopFlow { modules, modules_node, .. } => {
             refuse_node(modules_node)?;

docs/operator-builder-rights.md

@@ -31,10 +31,20 @@ It also returns what a value-only walk cannot authorize, for the caller to check
 permissions:
 
 - **the worker tags the steps pin**, or a builder routes a job onto a privileged worker group;
+- **every runnable a step references**. `script_to_payload` resolves a step's path with the root DB
+  handle (`db_authed = None`) and returns the referenced runnable's `on_behalf_of`, which
+  `worker_flow` then applies to the step job. So composing a path is enough to run it, and to run
+  it as whoever it runs as: `validate_operator_composed_flow` re-checks each path under the
+  caller's RLS. This is the general case; the one below is on top of it, not instead of it.
 - **the `(path, hash)` of every version-pinned step**. A step carrying a `hash` is dispatched by
-  that hash alone: `script_to_payload` ignores the path, reads the row with root permissions, and
-  takes that script's tag and `on_behalf_of` identity. An unverified pair therefore runs some other
-  script's code, possibly as some other identity, from behind a path the builder may read.
+  that hash alone, with the path beside it never consulted, so a readable path paired with another
+  script's hash still runs that other script.
+
+The same reasoning applies to a builder-authored app: its policy triggerables are what
+`execute_component` will resolve, also with the root DB handle, so
+`validate_operator_composed_app` checks every `script/<path>` and `flow/<path>` key the same way
+and refuses hub ones. `execute_component`'s preview branch refuses a hub path for operators too:
+`require_path_read_access_for_preview` admits `hub/` for everyone.
 
 Call it on every write **and** every preview: `run_preview_flow_job` and
 `push_flow_dependencies_job` both take a request-supplied flow value, so leaving either out makes

frontend/src/lib/components/common/table/AppRow.svelte

@@ -209,7 +209,7 @@
 							// list endpoint only surfaces own/legacy draft-only rows), so
 							// discarding it never requires write permission on the path.
 							disabled: !showEditButton,
-							hide: $userStore?.operator && !$operatorBuilderRights
+							hide: $userStore?.operator && !($operatorBuilderRights && app.raw_app)
 						},
 						{
 							displayName: $userStore?.operator ? 'View JSON' : 'View/Edit JSON',
@@ -271,7 +271,7 @@
 						displayName: 'Deployments',
 						icon: History,
 						action: () => appDeploymentHistory?.open(),
-						hide: $userStore?.operator && !$operatorBuilderRights
+						hide: $userStore?.operator && !($operatorBuilderRights && app.raw_app)
 					},
 					{
 						displayName: 'Permissions',
@@ -279,7 +279,7 @@
 						action: () => {
 							shareModal.openDrawer && shareModal.openDrawer(path, 'app')
 						},
-						hide: $userStore?.operator && !$operatorBuilderRights
+						hide: $userStore?.operator && !($operatorBuilderRights && app.raw_app)
 					},
 					{
 						displayName: 'Copy path',
@@ -325,7 +325,7 @@
 						},
 						type: 'delete',
 						disabled: !canEdit,
-						hide: $userStore?.operator && !$operatorBuilderRights
+						hide: $userStore?.operator && !($operatorBuilderRights && app.raw_app)
 					}
 				]
 			}}