mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(datatables): refuse a save that drops a data table's roles through an undeclared rename
A data table's roles follow its entry only through a declared rename. A settings sync sends the whole map and never declares one, so renaming a data table under roles there read as a delete and a new entry on the same database: the new entry carried no roles, and every caller connected as admin. Such a save is now refused, naming both entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb
This commit is contained in:
co-authored by
Claude Opus 5
parent
46dfcc5c49
commit
f8a6347052
@@ -934,3 +934,39 @@ async fn a_settings_save_dropping_a_governing_entry_names_the_forks_it_strands(
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
|
||||
async fn a_save_replacing_an_entry_under_roles_on_its_database_is_refused(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
// A rename as a settings sync sends it: the whole map, no `renames`.
|
||||
let resp = authed(
|
||||
client().post(format!(
|
||||
"http://localhost:{}/api/w/test-workspace/workspaces/edit_datatable_config",
|
||||
server.addr.port()
|
||||
)),
|
||||
"SECRET_TOKEN",
|
||||
)
|
||||
.json(&json!({ "settings": { "datatables": {
|
||||
"main_renamed": { "database": { "resource_type": "instance", "resource_path": "dt_main" } }
|
||||
} } }))
|
||||
.send()
|
||||
.await?;
|
||||
let status = resp.status();
|
||||
let body = resp.text().await?;
|
||||
assert_eq!(
|
||||
status, 400,
|
||||
"a save dropped the roles of the database it kept: {body}"
|
||||
);
|
||||
|
||||
let still_governed: bool = sqlx::query_scalar(
|
||||
"SELECT (datatable->'datatables'->'main') ? 'permissions' FROM workspace_settings
|
||||
WHERE workspace_id = 'test-workspace'",
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
assert!(still_governed, "the refused save still took effect");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -3887,6 +3887,40 @@ async fn edit_datatable_config(
|
||||
.cloned()
|
||||
.collect();
|
||||
|
||||
// Roles follow an entry only through a declared rename. A save that drops an entry under roles
|
||||
// and adds another on the same database without one — which is how a settings sync sends a
|
||||
// rename — would leave that database answering everyone as `admin`.
|
||||
for name in &removed {
|
||||
let Some(old_db) = old_datatables
|
||||
.get(name)
|
||||
.filter(|old| old.permissions.is_some())
|
||||
.and_then(|old| old.database.as_ref())
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
let added_on_same_database =
|
||||
new_config
|
||||
.settings
|
||||
.datatables
|
||||
.iter()
|
||||
.find_map(|(added, dt)| {
|
||||
let db = dt.database.as_ref()?;
|
||||
(!old_datatables.contains_key(added)
|
||||
&& !new_config.renames.iter().any(|r| &r.to == added)
|
||||
&& db.resource_type == old_db.resource_type
|
||||
&& db.resource_path == old_db.resource_path)
|
||||
.then_some(added)
|
||||
});
|
||||
if let Some(added) = added_on_same_database {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table '{name}' is under roles, and this save removes it while adding '{added}' \
|
||||
on the same database. Its roles would not carry over, leaving that database open to \
|
||||
everyone as `admin`. Rename it from the data table settings, which carries its \
|
||||
roles, or turn its roles off first."
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
let config: serde_json::Value = serde_json::to_value(new_config.settings)
|
||||
.map_err(|err| Error::internal_err(err.to_string()))?;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user