From 392888d11311d570dc55f30efdee989f866582b0 Mon Sep 17 00:00:00 2001
From: Ruben Fiszel <ruben@windmill.dev>
Date: Sun, 3 May 2026 07:14:14 +0000
Subject: [PATCH] docs: add SAFETY comments to all dynamic SQL call sites
 (#9009)

* docs: add SAFETY comments to all dynamic SQL call sites

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

* docs: address review feedback on SAFETY comments

- Fix missed comment for obo_triggers loop in offboarding.rs
- Fix variable name in comment (table -> table_name) in offboarding.rs
- Fix api-settings comment to reference inline VALID_NAME regex, not validate_dbname()
- Add SAFETY comments to batch_execute calls in api-settings
- Fix db.rs comment: PG_SCHEMA is env var, not compile-time constant
- Add doc comments on RunnableSettingsTraitInternal constants

* docs: remove misleading SAFETY comment on static SQL

---------

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
---
 backend/windmill-api-groups/src/granular_acls.rs     |  3 +++
 backend/windmill-api-settings/src/lib.rs             |  2 ++
 backend/windmill-api-users/src/users.rs              |  1 +
 backend/windmill-api/src/offboarding.rs              |  4 ++++
 backend/windmill-api/src/trash.rs                    |  2 ++
 backend/windmill-common/src/db.rs                    |  1 +
 backend/windmill-common/src/lib.rs                   |  3 +++
 backend/windmill-common/src/runnable_settings/mod.rs |  5 +++++
 backend/windmill-trigger-http/src/handler.rs         |  1 +
 backend/windmill-trigger/src/handler.rs              | 10 ++++++++++
 backend/windmill-trigger/src/listener.rs             |  4 ++++
 11 files changed, 36 insertions(+)

diff --git a/backend/windmill-api-groups/src/granular_acls.rs b/backend/windmill-api-groups/src/granular_acls.rs
index eac532dc78..95243b6626 100644
--- a/backend/windmill-api-groups/src/granular_acls.rs
+++ b/backend/windmill-api-groups/src/granular_acls.rs
@@ -131,6 +131,7 @@ async fn add_granular_acl(
         }
     }
 
+    // SAFETY: `kind` has been validated against the `KINDS` allowlist before reaching this function.
     let obj_o = sqlx::query_scalar::<_, serde_json::Value>(&format!(
         "UPDATE {kind} SET extra_perms = jsonb_set(extra_perms, $1, to_jsonb($2), \
          true) WHERE {identifier} = $3 AND workspace_id = $4 RETURNING extra_perms"
@@ -294,6 +295,7 @@ async fn remove_granular_acl(
         require_owner_of_path(&authed, path)?;
     }
 
+    // SAFETY: `kind` has been validated against the `KINDS` allowlist before reaching this function.
     let obj_o = sqlx::query_scalar::<_, bool>(&format!(
         "WITH old AS (
             SELECT extra_perms->$1 as old_write FROM {kind}
@@ -419,6 +421,7 @@ async fn get_granular_acls(
     } else {
         "path"
     };
+    // SAFETY: `kind` has been validated against the `KINDS` allowlist before reaching this function.
     let obj_o = sqlx::query_scalar::<_, serde_json::Value>(&format!(
         "SELECT extra_perms from {kind} WHERE {identifier} = $1 AND workspace_id = $2"
     ))
diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs
index 2251246f5d..f027046808 100644
--- a/backend/windmill-api-settings/src/lib.rs
+++ b/backend/windmill-api-settings/src/lib.rs
@@ -1132,6 +1132,7 @@ async fn setup_custom_instance_pg_database_inner(
 
     logs.created_database = "SKIP".to_string();
     if !db_exists {
+        // SAFETY: `dbname` has been validated by the VALID_NAME regex and length checks above (lines 1088–1120).
         sqlx::query(&format!("CREATE DATABASE \"{dbname}\""))
             .execute(db)
             .await?;
@@ -1144,6 +1145,7 @@ async fn setup_custom_instance_pg_database_inner(
 
     logs.db_connect = "OK".to_string();
 
+    // SAFETY: `dbname` has been validated by the VALID_NAME regex and length checks above.
     client
         .batch_execute(&format!(
             "GRANT CONNECT ON DATABASE \"{dbname}\" TO custom_instance_user;
diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs
index 951060b531..f664ab09b8 100644
--- a/backend/windmill-api-users/src/users.rs
+++ b/backend/windmill-api-users/src/users.rs
@@ -1698,6 +1698,7 @@ pub async fn delete_workspace_user_internal(
         "azure_trigger",
         "email_trigger",
     ];
+    // SAFETY: `table` comes from a hardcoded allowlist `extra_perms_tables`, not user input.
     for table in &extra_perms_tables {
         sqlx::query(&format!(
             "UPDATE {table} SET extra_perms = extra_perms - ('u/' || $1) \
diff --git a/backend/windmill-api/src/offboarding.rs b/backend/windmill-api/src/offboarding.rs
index a5d15165a2..c929a85a84 100644
--- a/backend/windmill-api/src/offboarding.rs
+++ b/backend/windmill-api/src/offboarding.rs
@@ -194,6 +194,7 @@ async fn get_offboard_preview(
     ];
     let mut triggers = HashMap::new();
     for table in &trigger_tables {
+        // SAFETY: `table` comes from a hardcoded allowlist `trigger_tables`, not user input.
         let paths: Vec<String> = sqlx::query_scalar(&format!(
             "SELECT path FROM {table} WHERE path LIKE $1 AND workspace_id = $2"
         ))
@@ -246,6 +247,7 @@ async fn get_offboard_preview(
     ).fetch_all(db).await?;
 
     let mut obo_triggers = HashMap::new();
+    // SAFETY: `table` comes from a hardcoded allowlist `trigger_tables`, not user input.
     for table in &trigger_tables {
         let paths: Vec<String> = sqlx::query_scalar(&format!(
             "SELECT path FROM {table} WHERE permissioned_as = $1 AND NOT path LIKE $2 AND workspace_id = $3"
@@ -759,6 +761,7 @@ async fn check_path_conflicts(
             "flow" => " AND NOT t1.archived",
             _ => "",
         };
+        // SAFETY: `table_name` comes from a hardcoded allowlist `tables`, not user input.
         let rows: Vec<String> = sqlx::query_scalar(&format!(
             "SELECT REGEXP_REPLACE(t1.path, '^u/' || $1 || '/', $3) \
              FROM {table} t1 \
@@ -1027,6 +1030,7 @@ async fn offboard_user_from_workspace<'c>(
     ];
 
     let mut triggers_reassigned: i64 = 0;
+    // SAFETY: `table` comes from a hardcoded allowlist `trigger_tables`, not user input.
     for table in &trigger_tables {
         let count: i64 = sqlx::query_scalar(&format!(
             "WITH updated AS ( \
diff --git a/backend/windmill-api/src/trash.rs b/backend/windmill-api/src/trash.rs
index 83e607a7dc..d0d7ddc6c2 100644
--- a/backend/windmill-api/src/trash.rs
+++ b/backend/windmill-api/src/trash.rs
@@ -480,6 +480,7 @@ async fn restore_trigger(tx: &mut sqlx::PgConnection, item: &TrashItemWithData)
         )));
     }
 
+    // SAFETY: `table_name` has been validated against the `valid_tables` allowlist above.
     let exists: bool = sqlx::query_scalar(&format!(
         "SELECT EXISTS(SELECT 1 FROM {} WHERE path = $1 AND workspace_id = $2)",
         table_name
@@ -500,6 +501,7 @@ async fn restore_trigger(tx: &mut sqlx::PgConnection, item: &TrashItemWithData)
         .get("row")
         .ok_or_else(|| Error::internal_err("Invalid trash data for trigger"))?;
 
+    // SAFETY: `table_name` has been validated against the `valid_tables` allowlist above.
     sqlx::query(&format!(
         "INSERT INTO {} SELECT * FROM jsonb_populate_record(null::{}, $1)",
         table_name, table_name
diff --git a/backend/windmill-common/src/db.rs b/backend/windmill-common/src/db.rs
index e122c8453e..0048abf438 100644
--- a/backend/windmill-common/src/db.rs
+++ b/backend/windmill-common/src/db.rs
@@ -233,6 +233,7 @@ impl UserDB {
         let mut tx = self.db.begin().await?;
 
         if let Some(schema) = PG_SCHEMA.as_ref() {
+            // SAFETY: `schema` is an operator-controlled environment variable (PG_SCHEMA), set at deploy time and never user-supplied.
             sqlx::query(&format!("SET LOCAL search_path TO {}", schema))
                 .execute(&mut *tx)
                 .await?;
diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs
index ef0b2dc5bf..7c7e3f79ed 100644
--- a/backend/windmill-common/src/lib.rs
+++ b/backend/windmill-common/src/lib.rs
@@ -778,6 +778,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu
 
     if db_exists {
         // Terminate active connections
+        // SAFETY: `dbname` has been validated via validate_dbname() before reaching this point.
         if let Err(e) = sqlx::query(&format!(
             "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '{}' AND pid <> pg_backend_pid()",
             dbname.replace('\'', "''")
@@ -789,6 +790,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu
         }
 
         // Drop the database
+        // SAFETY: `dbname` has been validated via validate_dbname() before reaching this point.
         sqlx::query(&format!("DROP DATABASE IF EXISTS \"{}\"", dbname))
             .execute(db)
             .await
@@ -837,6 +839,7 @@ pub async fn create_custom_instance_database(
         )));
     }
 
+    // SAFETY: `dbname` has been validated via validate_dbname() before reaching this point.
     sqlx::query(&format!("CREATE DATABASE \"{}\"", dbname))
         .execute(db)
         .await
diff --git a/backend/windmill-common/src/runnable_settings/mod.rs b/backend/windmill-common/src/runnable_settings/mod.rs
index 09bca46bbc..c5692d40bd 100644
--- a/backend/windmill-common/src/runnable_settings/mod.rs
+++ b/backend/windmill-common/src/runnable_settings/mod.rs
@@ -62,6 +62,7 @@ pub trait RunnableSettingsTrait:
         async move {
             let v = RUNNABLE_INDIVIDUAL_SETTINGS
                 .get_or_insert_async(hash, async {
+                    // SAFETY: INCLUDE_FIELDS and SETTINGS_NAME are compile-time constants, not user input.
                     let r = sqlx::query_as::<Postgres, Self>(&format!(
                         "SELECT {} FROM {} WHERE hash = $1",
                         Self::INCLUDE_FIELDS.iter().join(","),
@@ -138,7 +139,11 @@ mod private_mod {
     pub type Q<'a> = Query<'a, Postgres, <Postgres as Database>::Arguments<'a>>;
 
     pub trait RunnableSettingsTraitInternal {
+        /// Table name used in dynamic SQL via `format!()`. This is a compile-time
+        /// constant set by each settings impl — it is never user-controllable.
         const SETTINGS_NAME: &'static str;
+        /// Column list used in dynamic SQL SELECT via `format!()`. This is a
+        /// compile-time constant — never user-controllable.
         const INCLUDE_FIELDS: &'static [&'static str];
 
         fn bind_arguments<'a>(&'a self, q: Q<'a>) -> Q<'a>;
diff --git a/backend/windmill-trigger-http/src/handler.rs b/backend/windmill-trigger-http/src/handler.rs
index 9aaad6e7c8..74fd748f55 100644
--- a/backend/windmill-trigger-http/src/handler.rs
+++ b/backend/windmill-trigger-http/src/handler.rs
@@ -628,6 +628,7 @@ impl TriggerCrud for HttpTrigger {
         workspace_id: &str,
         path: &str,
     ) -> Result<bool> {
+        // SAFETY: Self::TABLE_NAME is a compile-time constant, not user input.
         let deleted = sqlx::query(&format!(
             "DELETE FROM {} WHERE workspace_id = $1 AND path = $2",
             Self::TABLE_NAME
diff --git a/backend/windmill-trigger/src/handler.rs b/backend/windmill-trigger/src/handler.rs
index 855961cf5e..f425f97b53 100644
--- a/backend/windmill-trigger/src/handler.rs
+++ b/backend/windmill-trigger/src/handler.rs
@@ -52,6 +52,8 @@ pub trait TriggerCrud: Send + Sync + 'static {
     type TriggerConfigRequest: Debug + DeserializeOwned + Serialize + Send + Sync;
     type TestConnectionConfig: Debug + DeserializeOwned + Serialize + Send + Sync;
 
+    /// Table name used in dynamic SQL queries via `format!()`. This is a compile-time
+    /// constant set by each trigger impl — it is never user-controllable.
     const TABLE_NAME: &'static str;
     const TRIGGER_TYPE: &'static str;
     const SUPPORTS_SERVER_STATE: bool;
@@ -189,6 +191,7 @@ pub trait TriggerCrud: Send + Sync + 'static {
     }
 
     async fn exists(&self, db: &DB, workspace_id: &str, path: &str) -> Result<bool> {
+        // SAFETY: Self::TABLE_NAME is a compile-time constant, not user input.
         let exists = sqlx::query_scalar(&format!(
             "SELECT EXISTS(SELECT 1 FROM {} WHERE workspace_id = $1 AND path = $2)",
             Self::TABLE_NAME
@@ -207,6 +210,7 @@ pub trait TriggerCrud: Send + Sync + 'static {
         workspace_id: &str,
         path: &str,
     ) -> Result<bool> {
+        // SAFETY: Self::TABLE_NAME is a compile-time constant, not user input.
         let deleted = sqlx::query(&format!(
             "DELETE FROM {} WHERE workspace_id = $1 AND path = $2",
             Self::TABLE_NAME
@@ -234,6 +238,7 @@ pub trait TriggerCrud: Send + Sync + 'static {
     ) -> Result<bool> {
         let permissioned_as = windmill_common::users::username_to_permissioned_as(&authed.username);
         let updated = if Self::SUPPORTS_SERVER_STATE {
+            // SAFETY: Self::TABLE_NAME is a compile-time constant.
             sqlx::query(&format!(
                 r#"
                 UPDATE
@@ -260,6 +265,7 @@ pub trait TriggerCrud: Send + Sync + 'static {
             .await?
             .rows_affected()
         } else {
+            // SAFETY: Self::TABLE_NAME is a compile-time constant.
             sqlx::query(&format!(
                 r#"
                 UPDATE
@@ -298,6 +304,7 @@ pub trait TriggerCrud: Send + Sync + 'static {
         is_flow: bool,
         script_path: &str,
     ) -> i64 {
+        // SAFETY: Self::TABLE_NAME is a compile-time constant.
         let count = sqlx::query_scalar(&format!(
             r#"
                 SELECT
@@ -466,6 +473,7 @@ async fn create_trigger<T: TriggerCrud>(
         .await?;
 
     if let Some(ref labels) = labels {
+        // SAFETY: T::TABLE_NAME is a compile-time constant.
         sqlx::query(&format!(
             "UPDATE {} SET labels = $1 WHERE workspace_id = $2 AND path = $3",
             T::TABLE_NAME
@@ -615,6 +623,7 @@ async fn update_trigger<T: TriggerCrud>(
         .await?;
 
     if let Some(ref labels) = labels {
+        // SAFETY: T::TABLE_NAME is a compile-time constant.
         sqlx::query(&format!(
             "UPDATE {} SET labels = $1 WHERE workspace_id = $2 AND path = $3",
             T::TABLE_NAME
@@ -692,6 +701,7 @@ async fn delete_trigger<T: TriggerCrud>(
     let mut tx = user_db.begin(&authed).await?;
 
     // Capture trigger data for trashbin before deleting
+    // SAFETY: T::TABLE_NAME is a compile-time constant.
     let trash_data: Option<serde_json::Value> = sqlx::query_scalar(&format!(
         "SELECT jsonb_build_object('row', to_jsonb(t), 'table_name', '{table}') FROM {table} t WHERE path = $1 AND workspace_id = $2",
         table = T::TABLE_NAME
diff --git a/backend/windmill-trigger/src/listener.rs b/backend/windmill-trigger/src/listener.rs
index 8344b71867..e327ed3155 100644
--- a/backend/windmill-trigger/src/listener.rs
+++ b/backend/windmill-trigger/src/listener.rs
@@ -234,6 +234,7 @@ pub trait Listener: TriggerCrud + TriggerJobArgs {
         listening_trigger: &ListeningTrigger<Self::TriggerConfig>,
         error: Option<&str>,
     ) -> Option<()> {
+        // SAFETY: Self::TABLE_NAME is a compile-time constant, not user input.
         let updated = sqlx::query_scalar::<_, i32>(&format!(
             r#"
                 UPDATE
@@ -336,6 +337,7 @@ pub trait Listener: TriggerCrud + TriggerJobArgs {
         listening_trigger: &ListeningTrigger<Self::TriggerConfig>,
     ) {
         if listening_trigger.trigger_mode {
+            // SAFETY: Self::TABLE_NAME is a compile-time constant.
             let _ = sqlx::query(&format!(
                 r#"
                 UPDATE
@@ -384,6 +386,7 @@ pub trait Listener: TriggerCrud + TriggerJobArgs {
         error: String,
     ) {
         if listening_trigger.trigger_mode {
+            // SAFETY: Self::TABLE_NAME is a compile-time constant.
             let report_status = sqlx::query(&format!(
                 r#"
                     UPDATE
@@ -660,6 +663,7 @@ pub async fn listen_to_unlistened_events<T: Copy + Listener>(
         Ok(mut unlistend_enabled_triggers) => {
             unlistend_enabled_triggers.shuffle(&mut rand::rng());
             for trigger in unlistend_enabled_triggers {
+                // SAFETY: T::TABLE_NAME is a compile-time constant.
                 let has_lock = sqlx::query_scalar(&format!(
                     r#"
                         UPDATE