diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index d7eeabea73..ce39e2a457 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -3712,7 +3712,13 @@ async fn edit_datatable_config( .and_then(|old| old.permissions.clone()); } - let args_for_audit = format!("{:?}", new_config.settings); + // 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(); audit_log( &mut *tx, &authed, diff --git a/python-client/wmill/wmill/client.py b/python-client/wmill/wmill/client.py index d9bf502c70..713bf6a1e3 100644 --- a/python-client/wmill/wmill/client.py +++ b/python-client/wmill/wmill/client.py @@ -2369,6 +2369,11 @@ def stream_result(stream) -> None: for text in stream: append_to_result_stream(text) +#: Role names the server accepts, so a value carrying a newline cannot close the +#: annotation and append statements of its own. +_ROLE_NAME_RE = re.compile(r"^[A-Za-z0-9_-]{1,63}$") + + class DataTableClient: """Client for executing SQL queries against Windmill DataTables.""" @@ -2381,6 +2386,10 @@ class DataTableClient: role: DataTable role to run as, on a datatable with permissions enabled (default: the data table's default role) """ + if role is not None and not _ROLE_NAME_RE.match(role): + raise ValueError( + f"Invalid data table role '{role}': must be 1-63 characters of letters, digits, '_' or '-'" + ) self.client = client self.role = role self.name, self.schema = parse_sql_client_name(name) diff --git a/typescript-client/sqlUtils.ts b/typescript-client/sqlUtils.ts index 0f04698ba9..4cbc815906 100644 --- a/typescript-client/sqlUtils.ts +++ b/typescript-client/sqlUtils.ts @@ -134,11 +134,20 @@ interface SqlProvider { providerName: string; } +/** Role names the server accepts, so a value carrying a newline cannot close the + * annotation and append statements of its own. */ +const ROLE_NAME_RE = /^[A-Za-z0-9_-]{1,63}$/; + function datatableProvider( name: string, schema?: string, role?: string ): SqlProvider { + if (role !== undefined && !ROLE_NAME_RE.test(role)) { + throw new Error( + `Invalid data table role '${role}': must be 1-63 characters of letters, digits, '_' or '-'` + ); + } return { providerName: "datatable", language: "postgresql", diff --git a/typescript-client/tests/sqlUtils.test.ts b/typescript-client/tests/sqlUtils.test.ts index 2f7197cac8..967d06bb53 100644 --- a/typescript-client/tests/sqlUtils.test.ts +++ b/typescript-client/tests/sqlUtils.test.ts @@ -38,11 +38,18 @@ interface SqlProvider { providerName: string; } +const ROLE_NAME_RE = /^[A-Za-z0-9_-]{1,63}$/; + function datatableProvider( name: string, schema?: string, role?: string ): SqlProvider { + if (role !== undefined && !ROLE_NAME_RE.test(role)) { + throw new Error( + `Invalid data table role '${role}': must be 1-63 characters of letters, digits, '_' or '-'` + ); + } return { providerName: "datatable", language: "postgresql", @@ -545,6 +552,15 @@ describe("datatable() — template tag", () => { const out = datatableQuery("main", "analyst")("SELECT $1", 42); expect(out.content.split("\n")[0]).toBe("-- role analyst"); }); + + test("a role that is not a role name is refused, not annotated", () => { + // The annotation is a comment line: a newline in the value would end it and + // leave the rest as SQL running under whatever role the first line named. + expect(() => dt("main", "admin\nDELETE FROM customers")).toThrow( + /Invalid data table role/ + ); + expect(() => dt("main", "operator-1")).not.toThrow(); + }); }); // =============================================================================