From b607c5cb41aee72da09c6336f76a835b740ba6df Mon Sep 17 00:00:00 2001 From: HugoCasa Date: Fri, 13 Mar 2026 15:39:35 +0100 Subject: [PATCH] refactor: rotate webhook tokens instead of recovering plaintext from DB Co-Authored-By: Claude Opus 4.6 --- ...24f96b8c4a5b397ba40792d4f3c05ee7629f4.json | 14 +++++ ...65850d30408d06f6ff77184442df3ede93416.json | 20 ++++++ ...95685b4b7dd6b7e8d610a13fac49368e26846.json | 46 ++++++++++++++ ...3dca862926695cf116843540a56f56a5022cc.json | 22 ------- ...cb7f1c644519db666e9893cc0923ad065de3.json} | 5 +- .../src/google/external.rs | 24 +++++--- .../windmill-native-triggers/src/handler.rs | 47 ++++++-------- backend/windmill-native-triggers/src/lib.rs | 61 +++++++++++++++---- backend/windmill-native-triggers/src/sync.rs | 1 + 9 files changed, 167 insertions(+), 73 deletions(-) create mode 100644 backend/.sqlx/query-0a1a962b4e6172f580904df260e24f96b8c4a5b397ba40792d4f3c05ee7629f4.json create mode 100644 backend/.sqlx/query-509588b81366668241ec5471f8765850d30408d06f6ff77184442df3ede93416.json create mode 100644 backend/.sqlx/query-999a3e795fbf7566b360a45d6f395685b4b7dd6b7e8d610a13fac49368e26846.json delete mode 100644 backend/.sqlx/query-abb79d8397504f48b6c99fed21c3dca862926695cf116843540a56f56a5022cc.json rename backend/.sqlx/{query-2fd22c4ffa2d222bb116260994a748e0639c2f73cbc1d8be66420c70b14c96e1.json => query-fdd84901d5a32fb5b0a3db7c0438cb7f1c644519db666e9893cc0923ad065de3.json} (58%) diff --git a/backend/.sqlx/query-0a1a962b4e6172f580904df260e24f96b8c4a5b397ba40792d4f3c05ee7629f4.json b/backend/.sqlx/query-0a1a962b4e6172f580904df260e24f96b8c4a5b397ba40792d4f3c05ee7629f4.json new file mode 100644 index 0000000000..c541a216fb --- /dev/null +++ b/backend/.sqlx/query-0a1a962b4e6172f580904df260e24f96b8c4a5b397ba40792d4f3c05ee7629f4.json @@ -0,0 +1,14 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM token WHERE token_prefix = $1", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [] + }, + "hash": "0a1a962b4e6172f580904df260e24f96b8c4a5b397ba40792d4f3c05ee7629f4" +} diff --git a/backend/.sqlx/query-509588b81366668241ec5471f8765850d30408d06f6ff77184442df3ede93416.json b/backend/.sqlx/query-509588b81366668241ec5471f8765850d30408d06f6ff77184442df3ede93416.json new file mode 100644 index 0000000000..c0068b434b --- /dev/null +++ b/backend/.sqlx/query-509588b81366668241ec5471f8765850d30408d06f6ff77184442df3ede93416.json @@ -0,0 +1,20 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO token (token_hash, token_prefix, token, email, label, super_admin, scopes, workspace_id)\n VALUES ($1, $2, NULL, $3, $4, $5, $6, $7)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "Varchar", + "Varchar", + "Varchar", + "Bool", + "TextArray", + "Varchar" + ] + }, + "nullable": [] + }, + "hash": "509588b81366668241ec5471f8765850d30408d06f6ff77184442df3ede93416" +} diff --git a/backend/.sqlx/query-999a3e795fbf7566b360a45d6f395685b4b7dd6b7e8d610a13fac49368e26846.json b/backend/.sqlx/query-999a3e795fbf7566b360a45d6f395685b4b7dd6b7e8d610a13fac49368e26846.json new file mode 100644 index 0000000000..e8206f34ba --- /dev/null +++ b/backend/.sqlx/query-999a3e795fbf7566b360a45d6f395685b4b7dd6b7e8d610a13fac49368e26846.json @@ -0,0 +1,46 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT label, email, scopes, workspace_id, super_admin FROM token WHERE token_prefix = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "label", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "email", + "type_info": "Varchar" + }, + { + "ordinal": 2, + "name": "scopes", + "type_info": "TextArray" + }, + { + "ordinal": 3, + "name": "workspace_id", + "type_info": "Varchar" + }, + { + "ordinal": 4, + "name": "super_admin", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + true, + true, + true, + true, + false + ] + }, + "hash": "999a3e795fbf7566b360a45d6f395685b4b7dd6b7e8d610a13fac49368e26846" +} diff --git a/backend/.sqlx/query-abb79d8397504f48b6c99fed21c3dca862926695cf116843540a56f56a5022cc.json b/backend/.sqlx/query-abb79d8397504f48b6c99fed21c3dca862926695cf116843540a56f56a5022cc.json deleted file mode 100644 index 4312fbcc5d..0000000000 --- a/backend/.sqlx/query-abb79d8397504f48b6c99fed21c3dca862926695cf116843540a56f56a5022cc.json +++ /dev/null @@ -1,22 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT token FROM token WHERE token_prefix = $1 AND token IS NOT NULL", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "token", - "type_info": "Varchar" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - true - ] - }, - "hash": "abb79d8397504f48b6c99fed21c3dca862926695cf116843540a56f56a5022cc" -} diff --git a/backend/.sqlx/query-2fd22c4ffa2d222bb116260994a748e0639c2f73cbc1d8be66420c70b14c96e1.json b/backend/.sqlx/query-fdd84901d5a32fb5b0a3db7c0438cb7f1c644519db666e9893cc0923ad065de3.json similarity index 58% rename from backend/.sqlx/query-2fd22c4ffa2d222bb116260994a748e0639c2f73cbc1d8be66420c70b14c96e1.json rename to backend/.sqlx/query-fdd84901d5a32fb5b0a3db7c0438cb7f1c644519db666e9893cc0923ad065de3.json index afd0f503bf..5b89b43a2b 100644 --- a/backend/.sqlx/query-2fd22c4ffa2d222bb116260994a748e0639c2f73cbc1d8be66420c70b14c96e1.json +++ b/backend/.sqlx/query-fdd84901d5a32fb5b0a3db7c0438cb7f1c644519db666e9893cc0923ad065de3.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n UPDATE native_trigger\n SET service_config = $1, updated_at = NOW()\n WHERE\n workspace_id = $2\n AND service_name = $3\n AND external_id = $4\n ", + "query": "\n UPDATE native_trigger\n SET service_config = $1,\n webhook_token_prefix = COALESCE($5, webhook_token_prefix),\n updated_at = NOW()\n WHERE\n workspace_id = $2\n AND service_name = $3\n AND external_id = $4\n ", "describe": { "columns": [], "parameters": { @@ -18,10 +18,11 @@ } } }, + "Text", "Text" ] }, "nullable": [] }, - "hash": "2fd22c4ffa2d222bb116260994a748e0639c2f73cbc1d8be66420c70b14c96e1" + "hash": "fdd84901d5a32fb5b0a3db7c0438cb7f1c644519db666e9893cc0923ad065de3" } diff --git a/backend/windmill-native-triggers/src/google/external.rs b/backend/windmill-native-triggers/src/google/external.rs index 756090fa02..676f987393 100644 --- a/backend/windmill-native-triggers/src/google/external.rs +++ b/backend/windmill-native-triggers/src/google/external.rs @@ -11,7 +11,7 @@ use windmill_common::{ use windmill_queue::PushArgsOwned; use crate::{ - generate_webhook_service_url, get_token_by_prefix, + generate_webhook_service_url, rotate_webhook_token, sync::{SyncAction, SyncError, TriggerSyncInfo}, update_native_trigger_error, update_native_trigger_service_config, External, NativeTrigger, NativeTriggerData, ServiceName, @@ -309,14 +309,17 @@ impl Google { } /// Renew an expiring Google watch channel. - /// Stops the old channel and creates a new one with the same channel ID. - /// Returns the updated service_config with new expiration. + /// Rotates the webhook token (creating a new one with the same label), + /// stops the old channel and creates a new one with the same channel ID. + /// Returns (new_service_config, new_token_prefix). pub async fn renew_channel( &self, w_id: &str, trigger: &NativeTrigger, db: &DB, - ) -> Result { + ) -> Result<(serde_json::Value, String)> { + use windmill_common::auth::TOKEN_PREFIX_LEN; + let config: GoogleServiceConfig = trigger .service_config .as_ref() @@ -324,9 +327,8 @@ impl Google { .transpose()? .ok_or_else(|| Error::InternalErr("Missing service config".to_string()))?; - let webhook_token = get_token_by_prefix(db, &trigger.webhook_token_prefix) - .await? - .ok_or_else(|| Error::InternalErr("Webhook token not found".to_string()))?; + let webhook_token = rotate_webhook_token(db, &trigger.webhook_token_prefix).await?; + let new_prefix = webhook_token[..TOKEN_PREFIX_LEN].to_string(); let base_url = &*BASE_URL.read().await; // Reuse the same channel ID so external_id stays permanent @@ -403,8 +405,9 @@ impl Google { new_config.google_resource_id = Some(resp.resource_id); new_config.expiration = Some(resp.expiration); - serde_json::to_value(&new_config) - .map_err(|e| Error::internal_err(format!("Failed to serialize config: {}", e))) + let config_value = serde_json::to_value(&new_config) + .map_err(|e| Error::internal_err(format!("Failed to serialize config: {}", e)))?; + Ok((config_value, new_prefix)) } } @@ -461,13 +464,14 @@ async fn renew_expiring_channels( ); match handler.renew_channel(workspace_id, trigger, db).await { - Ok(new_config) => { + Ok((new_config, new_prefix)) => { match update_native_trigger_service_config( db, workspace_id, ServiceName::Google, &trigger.external_id, &new_config, + Some(&new_prefix), ) .await { diff --git a/backend/windmill-native-triggers/src/handler.rs b/backend/windmill-native-triggers/src/handler.rs index 9384bde8b6..95ea56b235 100644 --- a/backend/windmill-native-triggers/src/handler.rs +++ b/backend/windmill-native-triggers/src/handler.rs @@ -1,6 +1,6 @@ use crate::{ decrypt_oauth_data, delete_native_trigger, delete_token_by_prefix, get_native_trigger, - get_token_by_prefix, list_native_triggers, store_native_trigger, update_native_trigger_error, + list_native_triggers, rotate_webhook_token, store_native_trigger, update_native_trigger_error, External, NativeTrigger, NativeTriggerConfig, NativeTriggerData, ServiceName, }; use axum::{ @@ -234,32 +234,25 @@ async fn update_native_trigger_handler( let runnable_changed = existing.script_path != data.script_path || existing.is_flow != data.is_flow; - let webhook_token = match get_token_by_prefix(&db, &existing.webhook_token_prefix).await? { - Some(token) if !runnable_changed => token, - existing_token => { - if let Some(_) = existing_token { - delete_token_by_prefix(&db, &existing.webhook_token_prefix).await?; - } else { - tracing::warn!( - "Webhook token not found for trigger {} (prefix: {}), recreating token", - external_id, - existing.webhook_token_prefix - ); - } - let token = new_webhook_token( - &mut *tx, - &db, - &authed, - &data.script_path, - data.is_flow, - &workspace_id, - service_name, - ) - .await?; - tx.commit().await?; - tx = user_db.begin(&authed).await?; - token - } + let webhook_token = if runnable_changed { + // Scopes change when the runnable changes — create a fresh token + delete_token_by_prefix(&db, &existing.webhook_token_prefix).await?; + let token = new_webhook_token( + &mut *tx, + &db, + &authed, + &data.script_path, + data.is_flow, + &workspace_id, + service_name, + ) + .await?; + tx.commit().await?; + tx = user_db.begin(&authed).await?; + token + } else { + // Same runnable — rotate the token keeping the same label + rotate_webhook_token(&db, &existing.webhook_token_prefix).await? }; let service_config = handler diff --git a/backend/windmill-native-triggers/src/lib.rs b/backend/windmill-native-triggers/src/lib.rs index e43dabe014..8573211bcd 100644 --- a/backend/windmill-native-triggers/src/lib.rs +++ b/backend/windmill-native-triggers/src/lib.rs @@ -719,20 +719,53 @@ async fn update_oauth_token_resource( } } -/// Look up the full plaintext token from the token table using its prefix. -/// Only webhook tokens store the plaintext (needed for URL construction). -/// Returns None for tokens that only have a hash stored. -pub async fn get_token_by_prefix<'c, E: sqlx::Executor<'c, Database = Postgres>>( - db: E, - token_prefix: &str, -) -> Result> { - let token: Option> = sqlx::query_scalar!( - "SELECT token FROM token WHERE token_prefix = $1 AND token IS NOT NULL", - token_prefix +/// Create a new webhook token that keeps the same label as the old one, +/// delete the old token, and return the new plaintext token. +/// Used when renewing external service webhooks (e.g. Google channel renewal) +/// so we don't need to store plaintext tokens in the DB. +pub async fn rotate_webhook_token(db: &DB, old_token_prefix: &str) -> Result { + use windmill_common::auth::{hash_token, TOKEN_PREFIX_LEN}; + use windmill_common::utils::rd_string; + + let old = sqlx::query!( + "SELECT label, email, scopes, workspace_id, super_admin FROM token WHERE token_prefix = $1", + old_token_prefix ) .fetch_optional(db) + .await? + .ok_or_else(|| { + Error::NotFound(format!( + "Webhook token not found for prefix: {}", + old_token_prefix + )) + })?; + + let new_token = rd_string(32); + let new_hash = hash_token(&new_token); + let new_prefix = &new_token[..TOKEN_PREFIX_LEN]; + + sqlx::query!( + "INSERT INTO token (token_hash, token_prefix, token, email, label, super_admin, scopes, workspace_id) + VALUES ($1, $2, NULL, $3, $4, $5, $6, $7)", + new_hash, + new_prefix, + old.email, + old.label, + old.super_admin, + old.scopes.as_deref(), + old.workspace_id, + ) + .execute(db) .await?; - Ok(token.flatten()) + + sqlx::query!( + "DELETE FROM token WHERE token_prefix = $1", + old_token_prefix + ) + .execute(db) + .await?; + + Ok(new_token) } /// Delete a token from the token table using its prefix @@ -1029,11 +1062,14 @@ pub async fn update_native_trigger_service_config< service_name: ServiceName, external_id: &str, service_config: &serde_json::Value, + new_webhook_token_prefix: Option<&str>, ) -> Result<()> { sqlx::query!( r#" UPDATE native_trigger - SET service_config = $1, updated_at = NOW() + SET service_config = $1, + webhook_token_prefix = COALESCE($5, webhook_token_prefix), + updated_at = NOW() WHERE workspace_id = $2 AND service_name = $3 @@ -1043,6 +1079,7 @@ pub async fn update_native_trigger_service_config< workspace_id, service_name as ServiceName, external_id, + new_webhook_token_prefix, ) .execute(db) .await?; diff --git a/backend/windmill-native-triggers/src/sync.rs b/backend/windmill-native-triggers/src/sync.rs index 873ba22557..292d00de2a 100644 --- a/backend/windmill-native-triggers/src/sync.rs +++ b/backend/windmill-native-triggers/src/sync.rs @@ -390,6 +390,7 @@ pub async fn reconcile_with_external_state( service_name, &trigger.external_id, external_service_config, + None, ) .await {