feat: drop data table role set logins unused for 30 days

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-10-12 22:21:29 +02:00
co-authored by Claude Opus 5.5
parent d563cc2687
commit f7f8741b36
7 changed files with 66 additions and 6 deletions
+1 -1
View File
@@ -1 +1 @@
6dedb4687f591a88aba1503d55a001fcb0ab128a
de85414a7dc5ccddbe9c2c7bb390396b9bf1742f
@@ -11,6 +11,9 @@ CREATE TABLE datatable_role_set (
-- Generated by Windmill, never entered by anyone, and never leaves the server.
pwd TEXT NOT NULL,
created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
-- Recorded at most once a day. A login unused for long enough is dropped, and made again if
-- its set of roles is asked for later.
last_used_at TIMESTAMPTZ NOT NULL DEFAULT now(),
PRIMARY KEY (cluster, name)
);
+19
View File
@@ -4449,6 +4449,24 @@ pub async fn monitor_db(
}
};
// Drop the data table role set logins nobody has connected with for a month. Every ~40 min,
// spawned for the same reason as the credential maintenance above: it opens a connection per
// managed database for each login it drops.
let datatable_role_set_sweep_f = async {
if server_mode && iteration.is_some() && iteration.as_ref().unwrap().should_run(240) {
if let Some(db) = conn.as_sql() {
let db = db.clone();
tokio::spawn(async move {
if let Err(e) =
windmill_common::datatable_roles::sweep_unused_role_set_logins(&db).await
{
tracing::error!("Error sweeping unused data table role set logins: {e}");
}
});
}
}
};
// Delete the AI session backups older than their workspace's retention. Every ~40 min
// (240 iterations at the default 10 s, the most a u8 `should_run` counts): the retention
// counts in days. Spawned for the same reason as the credential maintenance above, a
@@ -4520,6 +4538,7 @@ pub async fn monitor_db(
cleanup_scheduled_job_deletions_f,
git_auto_pull_f,
git_credential_maintenance_f,
datatable_role_set_sweep_f,
ai_session_retention_f,
pipeline_freshness_watchdog_f,
reconcile_unarmed_schedules_f,
@@ -1947,6 +1947,28 @@ async fn connecting_as_the_viewer_takes_the_roles_that_name_them_and_no_other(
// No recorded viewer must not fall back to the job's owner.
assert!(as_viewer(None).await.is_err());
// A set login nobody connects with any more is dropped, a recent one is kept, and a
// dropped one is made again for the next viewer who needs it.
let exists = || async {
sqlx::query_scalar::<_, bool>(
"SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = $1)",
)
.bind(&login)
.fetch_one(&db)
.await
};
windmill_common::datatable_roles::sweep_unused_role_set_logins(&db).await?;
assert!(exists().await?, "a set login in use was swept");
sqlx::query("UPDATE datatable_role_set SET last_used_at = now() - interval '31 days'")
.execute(&db)
.await?;
windmill_common::datatable_roles::sweep_unused_role_set_logins(&db).await?;
assert!(!exists().await?, "an unused set login was kept");
assert_eq!(
as_viewer(Some("test2@windmill.dev")).await?["user"],
login.as_str()
);
// A disabled role must not stay reachable through a login that is a member of it.
authed(
client().post(format!("{roles_url}/{}", ids[1])),
@@ -419,6 +419,13 @@ pub async fn drop_datatable_role(
crate::datatable_roles_oss::drop_datatable_role(db, tx, cluster, name).await
}
/// Drop the logins made for a connection as several roles at once that nobody has connected with
/// for a month, on every cluster. They are made again on demand, so this only bounds how many
/// exist. Safe to call from any server at any time: it serializes on the role catalog lock.
pub async fn sweep_unused_role_set_logins(db: &DB) -> Result<()> {
crate::datatable_roles_oss::sweep_unused_role_set_logins(db).await
}
#[cfg(test)]
mod tests {
use super::*;
@@ -29,7 +29,8 @@ pub(crate) use crate::datatable_roles_ee::{
ensure_instance_db_grant_options_unchecked, forget_datatable_role_everywhere,
insert_role_catalog_entry, read_role_catalog, read_role_catalog_tx,
registered_instance_databases, rename_datatable_role, resolve_datatable_role_connection,
role_cluster, set_datatable_role_login, update_role_catalog_entry,
role_cluster, set_datatable_role_login, sweep_unused_role_set_logins,
update_role_catalog_entry,
};
#[cfg(not(all(feature = "private", feature = "enterprise")))]
@@ -235,6 +236,11 @@ mod ce {
}
}
/// No roles, so no set login to sweep.
pub(crate) async fn sweep_unused_role_set_logins(_db: &DB) -> Result<()> {
Ok(())
}
pub(crate) async fn forget_datatable_role_everywhere(
_tx: &mut Tx<'_>,
_role_id: &str,
+7 -4
View File
@@ -27,7 +27,7 @@ A policy names its role as text, so renaming the role breaks it.
| Where | What |
|---|---|
| `windmill-common/src/workspaces.rs` | `DATATABLE_VIEWER_ROLE`, `parse_datatable_ref`, `DatatableAccess` |
| `windmill-common/src/datatable_roles_ee.rs` | `resolve_viewer_login`, `ensure_role_set_login`, `retire_role_set_logins_of` |
| `windmill-common/src/datatable_roles_ee.rs` | `resolve_viewer_login`, `ensure_role_set_login`, `retire_role_set_logins_of`, `sweep_unused_role_set_logins` |
| `windmill-api-jobs/src/execution.rs` | `run_end_user_email`, where a run's viewer is recorded |
## What holds it together
@@ -52,6 +52,12 @@ A policy names its role as text, so renaming the role breaks it.
- **A `datatable_role_set` row always names a login that can authenticate.** The row is written
after the login exists and removed, in its own commit, before the login is dropped or made
`NOLOGIN`. A login without a row is taken back with a new password on next use.
- **Set logins are bounded by use, not by the number of role combinations.** One exists only for
a set some viewer actually connected with, so there are at most as many as (viewer, data table)
pairs. `last_used_at` is recorded at most once a day, and `sweep_unused_role_set_logins`, run
from the monitor loop, drops those unused for 30 days; a dropped one is made again on demand.
The sweep goes by this instance's rows, never by the cluster's `wm_set_*` roles: a cluster can
serve several Windmill instances.
- **Disabling or dropping a role retires every set login holding it.** `NOLOGIN` on a role does
not reach a login that is a member of it.
- **It travels in the reference, not in an annotation.** A worker that predates `?as=` refuses the
@@ -66,6 +72,3 @@ table, the same trust as an app in viewer execution mode or a script someone els
run. Whoever can deploy a publisher-mode app, or a script that others run, can therefore read
through the non-`admin` roles of the people who use it; nothing on the data table opts in to
that.
A set login is dropped only with one of its roles. One that no viewer resolves to any more stays
on the cluster, holding only roles that still exist.