From 5f59b199b1163f26dfaa1bf124370e711d883d85 Mon Sep 17 00:00:00 2001 From: Diego Imbert Date: Tue, 1 Sep 2026 04:14:29 +0200 Subject: [PATCH] test(datatables): pin the audit parameter's redaction --- .../windmill-api-workspaces/src/workspaces.rs | 10 +++------- backend/windmill-common/src/workspaces.rs | 20 ++++++++++++++++--- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index ce39e2a457..2df63cd0d2 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -3712,13 +3712,9 @@ async fn edit_datatable_config( .and_then(|old| old.permissions.clone()); } - // The settings carry each role's generated login password, and an audit - // parameter is stored in the clear and traced. - let args_for_audit = serde_json::to_value(&new_config.settings) - .ok() - .and_then(|v| redact_datatable_settings_for_export(Some(v))) - .map(|v| v.to_string()) - .unwrap_or_default(); + // The settings carry each role's generated login password. + let args_for_audit = + windmill_common::workspaces::datatable_settings_for_audit(&new_config.settings); audit_log( &mut *tx, &authed, diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 398b13ee93..d12334d6f7 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1111,6 +1111,16 @@ pub fn redact_datatable_settings_for_export( Some(datatable) } +/// The data table settings as one audit parameter, which is stored and traced +/// in the clear — so it goes through the same redaction as any other export. +pub fn datatable_settings_for_audit(settings: &impl Serialize) -> String { + serde_json::to_value(settings) + .ok() + .and_then(|v| redact_datatable_settings_for_export(Some(v))) + .map(|v| v.to_string()) + .unwrap_or_default() +} + /// Postgres caps identifiers at 63 bytes (NAMEDATALEN - 1) and silently /// truncates past it, which would collapse two distinct roles onto one. const PG_IDENTIFIER_MAX_LEN: usize = 63; @@ -2846,9 +2856,11 @@ mod tests { assert!(datatable_role_entry(&dt, "main", Some("analyst")).is_err()); // admin and "no role" both mean the existing connection, so they are fine. assert!(datatable_role_entry(&dt, "main", None).unwrap().is_none()); - assert!(datatable_role_entry(&dt, "main", Some(ADMIN_DATATABLE_ROLE)) - .unwrap() - .is_none()); + assert!( + datatable_role_entry(&dt, "main", Some(ADMIN_DATATABLE_ROLE)) + .unwrap() + .is_none() + ); } #[test] @@ -2865,6 +2877,8 @@ mod tests { "other": { "database": { "resource_type": "instance", "resource_path": "db2" } } } }); + // The audit parameter is that same redaction, not a Debug of the settings. + assert!(!datatable_settings_for_audit(&settings).contains("s3cret")); let redacted = redact_datatable_settings_for_export(Some(settings)).unwrap(); let analyst = &redacted["datatables"]["main"]["permissions"]["roles"]["analyst"]; assert!(analyst.get("pg_password").is_none());