starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-08-18 16:02:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

docs: add SAFETY comments to all dynamic SQL call sites (#9009)

Browse Source 
* 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>
This commit is contained in:
Ruben Fiszel
2026-05-03 07:14:14 +00:00
committed by GitHub
parent da95588b25
commit 392888d113
11 changed files with 36 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/windmill-api-groups/src/granular_acls.rs
+3
View File
@@ -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"
))
backend/windmill-api-settings/src/lib.rs
+2
View File
@@ -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;
backend/windmill-api-users/src/users.rs
+1
View File
@@ -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) \
backend/windmill-api/src/offboarding.rs
+4
View File
@@ -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 ( \
backend/windmill-api/src/trash.rs
+2
View File
@@ -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
backend/windmill-common/src/db.rs
+1
View File
@@ -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?;
backend/windmill-common/src/lib.rs
+3
View File
@@ -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
backend/windmill-common/src/runnable_settings/mod.rs
+5
View File
@@ -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>;
backend/windmill-trigger-http/src/handler.rs
+1
View File
@@ -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
backend/windmill-trigger/src/handler.rs
+10
View File
@@ -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
backend/windmill-trigger/src/listener.rs
+4
View File
@@ -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