mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-03 16:02:12 +00:00
feat: data table roles in the DB manager and raw apps (#11139)
* feat(datatables): put a data table's connection under Postgres roles A data table backed by the instance database resolved to exactly one Postgres connection, `custom_instance_user`, for everyone who could reach it at all. There was no way to say this job reads, that one writes, this one never sees the salaries table. A data table role is now a real Postgres login on the cluster, defined once for the instance by a superadmin and named exactly as they named it. A script that declares `-- role analytics` connects as `analytics`, and Postgres decides what it may touch — grants are ordinary SQL. Windmill answers only "may this caller ask for this role", from the tenant lists on the data table entry: `u/alice`, `g/analysts`, `f/finance` or `*`. A data table with no `permissions` block behaves exactly as before. Everything that opens a connection on someone's behalf goes through one chokepoint, `get_datatable_resource_from_db`, which takes the identity explicitly and fails closed when there is none. The role logs in as itself — never `SET ROLE`, which a script could `RESET ROLE` its way out of. A fork's data table entry becomes a pointer at the workspace that governs it rather than a copy of it. The settings clone used to hand a fork a byte-identical entry naming the parent's database, which a fork admin could edit to grant themselves `admin` there; a pointer has nothing local to edit, and its tenants are evaluated as a member of the governing workspace, by email. `permissions` is stripped from the workspace export and ignored on import: tenants name principals of one workspace, and a settings push is not where an access decision should be made. Operations that see the whole database whatever the roles grant stay with the governing workspace's admins: editing the roles, a migration that declares none, and opening a replication stream for a Postgres trigger or capture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): gate the paths that reach a whole database as admin Auditing what still resolved through the unchecked resolver turned up three that act for a caller and hand back the admin connection: `resolve_pg_source_checked` (behind schema export, the full-schema read, database creation, import and the forked-database drop), the connection test, and the schema snapshot a fork clone takes of its parent. On a data table under roles each let any workspace member — or a fork admin who is nobody in the governing workspace — read or copy the whole database whatever its roles grant. All three now require admin reach on the governing workspace. A dump taken under a restricted role would be a silently truncated copy rather than an error, so refusing is the only right answer for the copy paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): confine roles to the instance database, and stop a fork reaching the parent's bookkeeping A data table role is a login on Windmill's own Postgres. Nothing stopped a workspace admin putting a *resource-backed* data table under roles, at which point the executor dialled the host that resource names — one the admin chose — with the role's real cluster password, and `CONNECT` is granted to every registered instance database. Both ends now refuse: the permissions endpoint rejects the save, and the chokepoint refuses to substitute credentials on a non-instance entry rather than trusting the record it read. Two more places reached the governing database without answering to it. The initial-migration generator returned a `pg_dump` of the whole schema to any member. And the migration rename/delete cascade followed a fork's pointer into the parent, so a fork admin renaming or removing their own local entry relabelled or wiped the parent's `_wm_migrations` — after which the parent re-runs every migration from zero. The remote half is now skipped when the entry resolves into another workspace, which is also just correct: a fork renaming what it calls a data table changes nothing about the data table. Also: revoking a tenant now bounces the replication streams of every workspace holding an entry that resolves here, not only the governing one, so a fork's trigger stops rather than living on inside its open connection; the instance role catalog and the governing workspace's tenant lists are no longer returned to someone who cannot edit them; and the tenant rename dedup collapses non-adjacent duplicates, per role rather than once any role changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): fail loudly where a role or a pointer can be left half-recorded Three ways the feature could end up in a state nobody could see or undo. Creating a role writes the cluster first and the catalog second, but the catalog write was an `UPDATE` that matched nothing when the instance Postgres settings row was absent — leaving a live login with a password nobody recorded: invisible to the catalog, un-recreatable because the name is taken, and un-deletable because there is no entry to delete. It now errors, so the operation is retryable once the row is restored. Deleting a workspace only nulls the fork lineage; the data table entries pointing at it are left resolving to nothing. Sweeping them is not an option — turning a pointer back into a copy would hand each fork the database outright — so the delete now names the data tables it stranded, and resolving one says which workspace is missing rather than reporting a data table this workspace never had. `InstanceDatatableRole` derived `Debug` while holding a Postgres password; it is now hand-written so `{:?}` on the catalog cannot put a live credential in a log line. Adds the two branches the reviews found unpinned: a caller who is not a member of the governing workspace at all, and `NoIdentity` — the compatibility path for an agent worker that predates this and sends no job id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): unbreak two operator messages and two comments that described other code The two strings this branch added for states an operator hits once — the catalog write that matched nothing, and the delete that stranded a pointer — were collapsed from their multi-line form with the indentation left in, so both rendered with a fourteen-space gap mid-sentence. `list_datatables` claimed to report a chain it cannot follow and then dropped it; it does drop it, and the comment now says why that is the right place to stay quiet. The non-superadmin check in `edit_datatable_config` was introduced as also covering references, which it does not and need not: `reference` is overwritten from the stored entry for every caller before the check runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): serialize role catalog mutations, and state each helper's authorization contract The catalog is one JSON document, so create, rename, enable and delete are all read-modify-write. Two concurrent creates read the same snapshot, both succeed in the cluster, and the second write drops the first — leaving a live Postgres login with a password nobody recorded, which is the exact state the delete path exists to prevent. Every mutation now runs in one transaction holding an advisory lock across the read, the cluster DDL and the write, so a lost update cannot happen and a failure rolls the whole thing back. The DDL helpers take that transaction rather than the pool, which is what makes the lock cover them. Their statements moved off `sqlx::raw_sql`: the simple protocol is only needed for genuinely multi-statement SQL, and its future is not `Send`, which an axum handler holding the transaction requires. Each of these is one statement anyway. The new cross-crate surface now says what callers must do. `read_role_catalog` returns plaintext credentials; `create`/`rename`/`set_login`/`drop_instance_role` and `converge_connect_grants` mutate cluster-wide state; `read_datatable_entry` reads a workspace's raw config. All of them are superadmin-gated by their current handlers, but nothing said so at the definition, which is where the next caller looks. Also: the roles table reloads after a failed login toggle instead of leaving it claiming a flip that did not land; the rename affordance is the design-system `Button`, not a raw one; and `resolve_datatable_pg_as_caller` drops a `role` parameter no caller ever filled — browsing resolves as the data table's default until the database manager grows a picker. Why role passwords stay a plain `String` while the instance user's password beside them is a `StringOrSecretRef`, asked three times across reviews: that one is a secret ref because an operator supplies it and may want it from their own backend, while these are minted here and never entered by anyone, so there is nothing for a ref to point at. Encrypting generated secrets at rest is a separate change that would take the replication password with it. Now said at the field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): give the role catalog its own row, out of reach of the config machinery Putting it inside `custom_instance_pg_databases` was the wrong call, and it cost two ways. The catalog serializes a generated Postgres password per role, and that row is the operator-facing instance config, so the passwords reached `get_instance_config` and its YAML editor — a live cluster credential in a response body, a UI field and any log of either. Worse in the other direction: `to_settings_map` strips the catalog, so a full-row upsert of that key writes the row back without it and the catalog is gone, while the cluster keeps every login it described. `custom_instance_replication_pwd` is the precedent and says exactly why — a generated secret, written only by the server, never operator-authored, hidden so the config machinery cannot read, rewrite or drop it. The catalog is the same thing, so it now has the same shape: `datatable_roles`, in `HIDDEN_SETTINGS`, `PROTECTED_SETTINGS` and the agent-worker denylist. No redaction to keep in step with three code paths, and no way for a neighbouring write to take it out. Two races on the same shared documents. `edit_datatable_config` read the stored data tables outside its transaction and then wrote the whole `datatable` document, so a permissions save committing in between was silently rolled back; it now reads under `FOR UPDATE`. And `set_datatable_permissions` validated role ids against the catalog before opening its transaction, so a deletion in between let it write a deleted role back — including as the default, which every later job then fails on; it now holds the catalog lock and the settings row across validation and write. Completes the authorization contracts the previous commit claimed but did not finish: `read_datatable_entry` (which it named and missed), `resolve_governing_datatable`, whose whole job is to answer for a workspace the caller may not belong to, and `converge_connect_grants_with`, which had not inherited its wrapper's. Also the generic Python SDK reference: `_format_py_params` learned the bare `*` last time, but `extract_py_functions` is a second formatter and still rendered `datatable(name, role)`, so code written from that page passed a keyword-only argument positionally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): make the concurrency test pin the handlers, and the contracts describe what is enforced The concurrency test reimplemented the read-modify-write inline, so deleting the lock from all three handlers left it green — it pinned Postgres, not the code it was written for. It now drives `create_datatable_role` twice concurrently and asserts the catalog kept both names. Checked the way the last one should have been: removing the lock from the handler makes it fail with "wmtest_a_… is a live cluster login the catalog forgot". The contracts added last commit were stricter than this PR's own callers, which is worse than none — the next reader sees a rule already broken and learns to ignore it. `read_role_catalog` said superadmin-only while two of its four callers are open to any workspace member, and `converge_connect_grants` said superadmin while `set_datatable_permissions` reaches it as a workspace admin. Both were fine on substance: the rule that actually holds is about the credential never reaching a response, log, audit record or export, not about who may call. They now say that. `read_datatable_entry` gets the same treatment rather than the one the earlier message claimed for it: it is the primitive every resolution goes through, so it is deliberately open, and what must not escape is `permissions` — it names the governing workspace's users, groups and folders. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): close the last ways a role or a pointer can be left pointing at nothing The raw settings readers hand back whatever is in the row, so moving the catalog into its own `global_settings` key protected the config machinery and left `GET /settings/global/datatable_roles` and the settings listing returning every live password. Both now filter that one key. The neighbouring `custom_instance_replication_pwd` has the same shape and is not touched here: it predates this and widening the fix to it is a decision about an operator workflow, not a consequence of this change. Three ways a save could leave something resolving to nothing: A permissioned data table could be moved to a PostgreSQL resource. The block was carried across as a server-owned field, the runtime refuses roles on a resource-backed table, so the save succeeded and every job afterwards failed. Refused instead — turning roles off first is one step, and it keeps discarding an access decision something somebody chose. Renaming a governing data table left every fork pointing at the old name: the data table disappears from their pickers and their jobs stop, with nothing in the renaming workspace to suggest why. The rename now follows into the pointers in the same transaction. Deleting one cannot be followed the same way, so it is reported instead — the response names what it stranded, the way deleting a workspace does, and the fork's own error already says which workspace is gone. Also: `ensure_instance_db_grant_options_unchecked` claimed superadmin while the permissions handler reaches it as a workspace admin (the same class fixed last commit, one instance missed); the role entry kept an `instance_config_schema` derive it no longer needs; `write_role_catalog` was the one writer of that table not stamping `updated_at`; and the concurrency test dropped its roles only on success — a failing run is exactly the one that creates them without recording them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * refactor(datatables): put the role catalog in its own table, not in global_settings Five findings across three rounds were all the same choice. A set of live Postgres credentials was living in `global_settings`, which has generic read, list, write, config-export and CLI round-trip paths that know nothing about what they carry: the passwords reached the instance config and its YAML editor, a full-row upsert of a neighbouring key erased the catalog, `GET /settings/global/{key}` and the settings listing returned them raw, and this round the redaction that fixed the last two turned `wmill instance push` into something that wipes every password — a fix breaking the assumption the previous fix made. `POST /settings/global/datatable_roles` could also empty it outside the lock. The approved plan offered a table or `global_settings`, so this is the other option it already allowed rather than a new design. `datatable_role` is a table: no generic settings path can read it, list it, export it, write it or round-trip it, so none of the five needs a guard. The redaction, the hidden/protected/agent-denylist entries and the JSON document all go with it. One row per role also removes the read-modify-write the concurrency work was about: two concurrent creates are two inserts, and the unique index on `name` is what settles a collision. The advisory lock stays for the one window rows do not cover — `CREATE ROLE` is invisible to another transaction until commit, so without it both creates pass their `pg_roles` check. Also from this round: rename mappings are checked against the configuration they claim to describe, since fork pointers are rewritten from them — a caller could otherwise submit `main -> missing` against an unchanged config and repoint every fork of `main` at a name nothing has, and `A -> B` plus `B -> C` moved what pointed at `A` all the way to `C`. And the warning naming forks a delete stranded reached the response but not the screen: both the data table settings save and the workspace delete now show it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): validate a rename against the save it describes, and re-check under the locks Three from the round, all about deciding on state that could already have moved. A permission save resolved the data table and checked it was instance-backed before taking any lock, then wrote under one. A config save committing in between could move the table onto a PostgreSQL resource — recreating exactly what the transition guard refuses — or rename it, in which case the write targeted a key that no longer existed and reported success having changed nothing. It now re-resolves and re-checks on the locked state. Rename validation checked that the source existed before and the target existed after, which still accepts `main -> decoy` against a save that keeps both: every fork of `main` then follows onto a different data table, silently, because it keeps resolving. The rule is now the actual old-to-new key transition — a source may only survive if another rename took its name, and a target may only pre-exist if another rename freed it. That also stops two sources sharing one target, and it admits a swap, which the previous guard refused: `datatables` is keyed by name, so a swap cannot be done one save at a time, and refusing it was a regression against main. The pointer cascade now runs in two passes through a temporary name, the way the migration cascade one layer down already handles the same shape, so `A -> B` with `B -> C` moves each pointer once from what it named before the save. The tenant mutators say what they are for: they write an access decision for any workspace named, with an arbitrary mutation, and exist for the transaction that frees or renames a principal. Editing a decision on purpose belongs in the permissions endpoint. Carried in the same change: the stranded-fork list is a field rather than a phrase to grep out of a success string; the pointer cascade matches with `EXISTS` instead of a `LIKE` over the whole document, so a workspace whose pointers name something else is not rewritten to a byte-identical value under an exclusive lock; and `InstanceDatatableRole` drops the serde derives left over from the JSON document, one of which would emit `pwd`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): cascade on the leave route that is used, gate migrations before the admin connection, and drop a role atomically The tenant cascade on leaving went onto `/users/leave`. The UI and the generated client call `/workspaces/leave` — a different handler in a different crate with the same name — which deleted the membership and left `u/<username>` in the tenant lists. Leaving and rejoining therefore restored the access the leave was supposed to end, and a later account taking the username would have inherited it. The regression test drives the route the client actually calls; without the fix it fails with "leaving kept the tenant". The migration endpoints authorized too late. `run_datatable_migrations` opened the data table's admin connection, created `_wm_migrations` and read it before reaching the per-migration role check — so with nothing pending, nothing was checked at all. Rollback returned before its check when nothing was applied, and the status endpoint had none. All three now ask, before any connection is opened, whether the caller can reach the data table as any role at all; which role a given migration runs as is still decided per migration, and by the executor after that. Deleting a role committed the cluster drop and the catalog row, then swept the tenant lists in separate transactions. A sweep failing part-way left workspaces naming a role nothing can connect as, while the retry answered `NotFound` because the catalog entry was already gone. The sweep now runs in the same transaction, so the drop, the row and every tenant list commit together. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): refuse to copy a data table that is under roles pg_dump carries no roles and the import runs with --no-privileges, so a copied data table arrives owned by the admin connection with no GRANT for any role. The settings clone brings `permissions` across, so the fork's tenants pass Windmill's check, connect as the role they were given, and are denied by Postgres on everything: an entry that reads as configured and answers nothing. Refuse the copy — in the import endpoint before any data moves, and in the fork path the CLI takes. Replaying the source's owners and ACLs into the clone is what lifts this, and is a change of its own. Dropping `permissions` from the copy instead would be the unsafe half, since the copy holds the parent's rows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): refuse the clone's database too, not only its data A clone is two endpoints: `create_pg_database` then `import_pg_database`. Only the second refused a data table under roles, so a fork asking to clone one created and registered an empty `wm_fork_…` instance database and then failed — and nothing collects it, since `drop_forked_datatable_databases` only drops entries carrying `forked_from` and no entry names this one. Refuse in both, so the clone stops before a database exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * nit worker error msg * fix pg_dump stuck on version 17 on nix * fix(datatables): refuse a malformed role annotation instead of ignoring it `-- Role operator`, `-- role operator;` and `-- role operator -- why` all failed the annotation parser's exact-match rule, so the query fell through to the data table's default role and ran, silently, under a login the author did not choose. Naming a role exists precisely to not do that. A leading comment whose first word is `role` is now an annotation attempt: the keyword matches case-insensitively, one trailing `;` is tolerated, and anything else is an error naming the line. Only callers that already know the target is a `datatable://` reference ever run this, so ordinary SQL keeps its comments. Also bumps the dev shell's postgres client to 18 — it trailed the server the dev database runs, which takes out every data table export, clone and fork-with-data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): refuse a malformed role query string instead of ignoring it `?Role=analytics`, `?role=` and `?x=1&role=…` all fell through the reference parser's exact-match rule, so the connection resolved to the data table's default role and ran under a login the caller never asked for — the URI half of the same trap as a malformed `-- role` annotation. The key now matches case-insensitively, and anything else in the query string is an error naming it; `role` is the only parameter a reference takes. Callers that only need the entry keep a lenient `datatable_ref_name`, since they never act on the role. The DuckDB `ATTACH` parser propagates it rather than attaching under the default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): carry the role annotation into the row_to_json retry The retry rebuilds its SQL from `pruneComments(code)`, so the leading comment block never reached the second attempt — and with it the `-- role <name>` line that decides which login the query runs as. The retry connected as the data table's default role instead, so a query the first attempt was denied could succeed on the second, reported as "recovered with the row_to_json fix". Carry the leading comment block over. The retry itself is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * chore(datatables): don't mount the roles UI until the ACL editor lands Enforcement ships first. The permissions drawer is what turns roles on, and the catalog section is what creates them — both are only useful once there is a way to grant a role the privileges it needs, which arrives with the ACL editor. Left mounted they would offer a feature whose other half does not exist. The two components are complete and reviewed; only their call sites here are commented out, with a note pointing the follow-up PRs at them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * fix(datatables): honour `-- role: x`, and fix the DuckDB attach test Two review findings, both real. `attach_datatable_parses_name_and_role` never compiled: `parse_attach_datatable` returns `Result<Option<_>>` now and one call site kept a single `unwrap`. Its `?Role=analytics` case also asserted a refusal, contradicting the parser in the same commit, which matches the key case-insensitively. Replaced with the cases that are genuinely malformed, and a positive one for the cased key. `-- role: analytics` fell through to the default role — the silent fallback the strict parser exists to remove, for the spelling most likely to be typed. The keyword now accepts an optional colon, attached or spaced, while a word that merely starts with it (`rolebased`) is still not an attempt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * fix(datatables): clone a fork's pointer instead of failing after the copy Forking a fork with cloning left an orphan database. The preflight resolves the pointer and sees the governing entry, so both endpoints ran and filled the new database; `apply_forked_datatable` then refused the inherited pointer and rolled the fork back, stranding a registered `wm_fork_*` that no entry names and whose name blocks the retry. Refusing earlier would have been the smaller change, but forking a fork and cloning worked before pointers existed, so it would trade an orphan for a regression. Resolve what the pointer names and write the terminal entry the clone needs: the whole `database` object rather than a patch of its `resource_path`, since a pointer has none, and `reference` removed with it. Also accepts `-- role=x` and `-- Role = x`, two more spellings that fell through to the default role. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * fix(datatables): refuse to roll back the catalog while roles exist The down migration dropped the table and left every role behind: live Postgres logins whose passwords only that table carried, so after a revert Windmill could neither use, disable nor delete them, and re-applying could not recreate them because the names were taken. Cleaning up here is not possible either — dropping a role means reassigning what it owns in every instance database, and a migration runs in one — so it now refuses while the catalog is non-empty and says to delete the roles through instance settings, which does the cluster work. Also enforces the instance-only invariant the resolved-pointer clone relies on rather than only asserting it in a comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * refactor(datatables): settle clonability in one place, before anything is created A clone is three stages a workspace apart — `create_pg_database`, then `import_pg_database`, then `apply_forked_datatable` inside the fork transaction. Only the third can roll back, and `CREATE DATABASE` is not transactional, so any refusal that lives there strands a registered `wm_fork_*` that no entry names and whose name blocks the retry. That orphan has now been fixed three times, most recently reintroduced by a guard added one commit ago. Patching each new refusal into the first endpoint is not the fix; having two places that can refuse is. `ensure_datatable_is_clonable` now answers every reason a copy can be refused and returns what it resolved, and the stage that writes the entry only does the work. Also takes an ACCESS EXCLUSIVE lock before the rollback guard counts, so a role created concurrently cannot slip between the check and the drop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * fix(datatables): let a retried clone reclaim its own leftover database A clone creates its target database one request before it copies into it, and the fork that would name it is written a request after that. Any failure in between — a pg_dump error, a bad restore, a dropped connection, the source's roles changing mid-flow — left a registered `wm_fork_*` that no entry names, and every retry then failed on its name. This predates data table roles. `create_pg_database` now reclaims such a leftover before creating: only a `wm_fork_*` database Windmill registered as a data table database and that no data table or ducklake entry names, in any workspace, archived ones included. The drop never terminates connections, so a clone still copying into it makes the reclaim fail instead of being cut off. It is limited to callers who administer the source — reaching it is not enough, since on a data table without roles every member reaches it — and anyone else gets the refusal an existing database always got. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Revert "fix(datatables): let a retried clone reclaim its own leftover database" This reverts commit7dd3275a10. The reclaim tied the caller to the source they administer, but not to the database it dropped. Between another workspace's import and its final fork request, that workspace's target is full, registered, unnamed and has no open connection, so an admin of any instance data table could name it and have it dropped and recreated empty. The victim's fork would then commit pointing at the empty copy. Safe reclaim needs durable clone ownership and serialization with the request that names the database; until then the leftover stays, as it did before this PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(datatables): record the stale clone database as a known limitation A clone is three requests and `CREATE DATABASE` is not transactional, so a failure after the first leaves a registered `wm_fork_*` behind, as it did before data table roles. Accepted for this PR: it is harmless to data and goes away once the clone is a single server-side operation. The comment also records why the obvious fix is wrong: reclaiming the leftover on retry, without durable clone ownership, can drop another workspace's fully copied database between its import and its final fork request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): bounce the streams reading a data table when it is deleted Deleting a governing data table, or the workspace that holds it, only collected the fork pointers it stranded, for the warning. A Postgres trigger or capture already streaming through one of those pointers kept the replication connection it opened while the pointer still resolved, so it went on dispatching the governing database's rows after the fork lost access — until its connection happened to restart. The governing workspace's own streams on a deleted entry did the same. Both deletion paths now bounce the affected listeners inside their own transaction, through the helper a permission change already uses, so a listener that reconnects re-resolves the entry and finds it gone. The helper is split so a caller can pass the (workspace, local name) pairs it already holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): keep the fork schema baseline, and bounce streams on every removal Three fixes from review. `edit_datatable_config` took `forked_from` wholesale from the stored entry, so the fork schema diff's save of an advanced baseline was silently discarded and an applied change was offered again. Whether an entry carries a clone stamp is still carried from the store, since that is what marks its database droppable, but the baseline inside it is now taken from the request. The stranded-pointer warning and the stream bounce ran over the optional `deleted_datatables` hint, which the settings-sync CLI never sends, so removing a governing data table through `wmill` bounced nothing. Removals are now derived from the stored configuration against the saved one. `delete_workspace` read the pointers to bounce before its transaction, so a fork committing a pointer during the deletion was missed. The read now happens inside the transaction, after the workspace row is deleted: a fork's insert key-share locks that row through its parent foreign key, so it is either seen or fails on the missing parent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(datatables): keep Postgres triggers and data table roles apart A replication stream reads every row of every table whatever the data table's roles grant, and its listener checks access only when it connects. Rather than chase every way access can change and bounce the streams each one affects, a data table now carries one or the other: - a Postgres trigger or capture cannot be created on, or connect to, a data table under roles; - roles cannot be turned on while an enabled trigger or a live capture reads the data table, its own or a fork's through its pointer. The refusal names each one to disable. This removes the stream bounces on roles edits and on data table and workspace deletion, and the trigger gate that admitted admins. The fork schema baseline fix from the same review round is kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): refuse a Postgres trigger on a data table under roles when it is saved Creating or editing a trigger that points at a data table under roles was accepted, and its listener then retried the refused connection every 30 seconds forever. The save is now refused, and a trigger that reaches such a data table anyway (re-enabled, or cloned into a fork) is disabled by its listener with the reason, as a missing replication slot is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): disable a data table role before deleting it Deleting a role reassigns and drops what it owns in each registered database on its own connection, and each of those passes commits as it goes. A database failing part-way left the role enabled in the catalog and able to log in, but already stripped in the databases reached before it. The role is now disabled in its own commit first, so a failed delete leaves a disabled role to retry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): serialize roles going on with a stream starting Turning roles on looked for enabled triggers and live captures once, without a lock anything starting a stream also took. A trigger enabled in that window could have its listener connect before roles committed, and a healthy listener never checks again. Both transitions now serialize on one advisory lock: roles going on hold it exclusive while they look, and trigger create, edit and enable, and capture setup and ping hold it shared while they commit. Either the look sees the stream, or the listener connects after roles are committed and refuses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): wait out live listeners, and resolve stored names containing `?` Turning roles on counted a trigger as gone once disabled, and a capture once its client stopped pinging, but the listener keeps its replication connection until its next heartbeat notices. A trigger or capture whose listener pinged in the last 15 seconds, the window a server holds a listener for, now still counts as streaming. Data table names could contain `?` before they were restricted, and such entries are still stored. Splitting `?role=` off a reference misread them: `a?b` became `a` with an unknown parameter, and the clone checks looked at a different entry than the one copied. An entry stored under the whole reference is now looked up first, in the Postgres executor, DuckDB ATTACH and the clone checks. Agent workers cannot read the workspace and keep the strict parse, which refuses such a name rather than misreading it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): warn when a settings sync strands fork pointers A settings save reported the fork pointers left resolving to nothing only for the names in `deleted_datatables`, which `wmill sync push` never sends. The save now works out what it removed from the locked entries, and the CLI prints the stranded pointers it returns. Also correct the replication helper's contract: no role or admin check makes a replication connection safe, so a data table under roles is refused outright rather than gated as an admin operation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * 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 * fix(datatables): no entry without roles may newly reach a database under roles The previous guard only caught a new name replacing an entry under roles. A whole-map save could also repoint an existing entry without roles at that database, or another workspace could point one there, and every caller of that entry would connect as admin. The rule is now stated on the saved entries: one that carries no roles and newly points at an instance database any entry under roles uses, in this workspace or another, is refused. A declared rename carries its roles and passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * feat(datatables): move data table role catalog and resolution to the enterprise edition Roles are an Enterprise Edition feature. The catalog, the Postgres logins, CONNECT convergence, tenant evaluation and the role half of connection resolution move to windmill-ee-private. Every public function keeps its path and signature and forwards through datatable_roles_oss, which re-exports the enterprise implementation or, without it, refuses. Without the enterprise edition a data table under roles, or a caller naming a role, is refused a connection rather than resolved as admin, and the reach and admin-access checks refuse one under roles. A data table not under roles resolves as before in every edition, and an instance database keeps the CONNECT grants it was created with. The catalog lock, the stream lock, the tenant cascades and the permissions stripping stay in OSS: they only restrict. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * feat(datatables): move the data table permissions endpoints to the enterprise edition The permissions read, save and usable-roles handlers move to windmill-ee-private; the routes stay registered and, without the enterprise edition, answer that data table roles are an Enterprise Edition feature. ensure_governs_datatable and ensure_reaches_datatable keep their paths: the first refuses, the second passes a data table not under roles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * feat(datatables): move the data table role catalog endpoints to the enterprise edition The superadmin list, create, update and delete handlers move to windmill-ee-private. The routes stay registered and, without the enterprise edition, refuse after authentication. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * test(datatables): run the roles tests on the enterprise edition, refusals without it Each test that exercises roles runs with private and enterprise. Two tests run without them: every roles route answers the Enterprise refusal, and a data table saved under roles, or a named role, is refused a connection while one not under roles resolves as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * feat(datatables): gate the roles UI mount sites on an enterprise license Both mount sites are still commented out; the gate travels with them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * test(datatables): run the tenant matcher test on the enterprise edition The matcher it covers is enterprise code now, so without the enterprise edition the test hit the stub and failed the default windmill-common run. It runs with private and enterprise, and a counterpart without them asserts that no tenant list covers anyone, the wildcard and a workspace admin included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * chore: update ee-repo-ref to a1873dbb67f2302b85ff5362f8387b48eccdb607 This commit updates the EE repository reference after PR #783 was merged in windmill-ee-private. Previous ee-repo-ref: 5c853e2c20eca6b748415fc0d6862a6ebfb5fec4 New ee-repo-ref: a1873dbb67f2302b85ff5362f8387b48eccdb607 Automated by sync-ee-ref workflow. * fix(datatables): refuse roles while a same-workspace alias reaches the database Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(datatables): add an ACL editor for data table roles Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(datatables): data table roles in the DB manager and raw apps Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: never add a role to the reference of a data table whose name contains '?' Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: read the roles of a data table whose name contains '?' The generated client leaves a '?' in a path param unencoded, so the lookup 404'd and the raw-app picker blocked Start on such a data table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: take every pooled connection before the ACL apply locks Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: refresh grant options only after the ACL apply validates its plan Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix(datatables): refuse a reference naming both a legacy data table and a role When a workspace stores both `sales` and a legacy `sales?role=analytics`, the reference resolved to the legacy entry without a role, so browsing `sales` as `analytics` reached another data table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: declare the default role in migrations written for a data table whose name contains '?' Such a data table connects as its default role without naming it, so the migrations the manager wrote for it declared no role and ran as admin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): let CE migrations connect as an explicitly named admin Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: add only missing grant options before an ACL apply, never default privileges Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix(datatables): serialize roles going on with aliases saved from other workspaces Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(datatables): note that legacy names with ? cannot be migrated Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: run one data table ACL apply at a time per server before it connects Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: hold the ACL connection to the database that was authorized Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: build the ACL connection from the authorized data table entry Resolving the settings again could land on a resource with the same database name on another server, which the later entry checks never see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: check ACL read reach against the entry it connects from Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * feat(datatables): put a data table's connection under Postgres roles A data table backed by the instance database resolved to exactly one Postgres connection, `custom_instance_user`, for everyone who could reach it at all. There was no way to say this job reads, that one writes, this one never sees the salaries table. A data table role is now a real Postgres login on the cluster, defined once for the instance by a superadmin and named exactly as they named it. A script that declares `-- role analytics` connects as `analytics`, and Postgres decides what it may touch — grants are ordinary SQL. Windmill answers only "may this caller ask for this role", from the tenant lists on the data table entry: `u/alice`, `g/analysts`, `f/finance` or `*`. A data table with no `permissions` block behaves exactly as before. Everything that opens a connection on someone's behalf goes through one chokepoint, `get_datatable_resource_from_db`, which takes the identity explicitly and fails closed when there is none. The role logs in as itself — never `SET ROLE`, which a script could `RESET ROLE` its way out of. A fork's data table entry becomes a pointer at the workspace that governs it rather than a copy of it. The settings clone used to hand a fork a byte-identical entry naming the parent's database, which a fork admin could edit to grant themselves `admin` there; a pointer has nothing local to edit, and its tenants are evaluated as a member of the governing workspace, by email. `permissions` is stripped from the workspace export and ignored on import: tenants name principals of one workspace, and a settings push is not where an access decision should be made. Operations that see the whole database whatever the roles grant stay with the governing workspace's admins: editing the roles, a migration that declares none, and opening a replication stream for a Postgres trigger or capture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): gate the paths that reach a whole database as admin Auditing what still resolved through the unchecked resolver turned up three that act for a caller and hand back the admin connection: `resolve_pg_source_checked` (behind schema export, the full-schema read, database creation, import and the forked-database drop), the connection test, and the schema snapshot a fork clone takes of its parent. On a data table under roles each let any workspace member — or a fork admin who is nobody in the governing workspace — read or copy the whole database whatever its roles grant. All three now require admin reach on the governing workspace. A dump taken under a restricted role would be a silently truncated copy rather than an error, so refusing is the only right answer for the copy paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): confine roles to the instance database, and stop a fork reaching the parent's bookkeeping A data table role is a login on Windmill's own Postgres. Nothing stopped a workspace admin putting a *resource-backed* data table under roles, at which point the executor dialled the host that resource names — one the admin chose — with the role's real cluster password, and `CONNECT` is granted to every registered instance database. Both ends now refuse: the permissions endpoint rejects the save, and the chokepoint refuses to substitute credentials on a non-instance entry rather than trusting the record it read. Two more places reached the governing database without answering to it. The initial-migration generator returned a `pg_dump` of the whole schema to any member. And the migration rename/delete cascade followed a fork's pointer into the parent, so a fork admin renaming or removing their own local entry relabelled or wiped the parent's `_wm_migrations` — after which the parent re-runs every migration from zero. The remote half is now skipped when the entry resolves into another workspace, which is also just correct: a fork renaming what it calls a data table changes nothing about the data table. Also: revoking a tenant now bounces the replication streams of every workspace holding an entry that resolves here, not only the governing one, so a fork's trigger stops rather than living on inside its open connection; the instance role catalog and the governing workspace's tenant lists are no longer returned to someone who cannot edit them; and the tenant rename dedup collapses non-adjacent duplicates, per role rather than once any role changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): fail loudly where a role or a pointer can be left half-recorded Three ways the feature could end up in a state nobody could see or undo. Creating a role writes the cluster first and the catalog second, but the catalog write was an `UPDATE` that matched nothing when the instance Postgres settings row was absent — leaving a live login with a password nobody recorded: invisible to the catalog, un-recreatable because the name is taken, and un-deletable because there is no entry to delete. It now errors, so the operation is retryable once the row is restored. Deleting a workspace only nulls the fork lineage; the data table entries pointing at it are left resolving to nothing. Sweeping them is not an option — turning a pointer back into a copy would hand each fork the database outright — so the delete now names the data tables it stranded, and resolving one says which workspace is missing rather than reporting a data table this workspace never had. `InstanceDatatableRole` derived `Debug` while holding a Postgres password; it is now hand-written so `{:?}` on the catalog cannot put a live credential in a log line. Adds the two branches the reviews found unpinned: a caller who is not a member of the governing workspace at all, and `NoIdentity` — the compatibility path for an agent worker that predates this and sends no job id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): unbreak two operator messages and two comments that described other code The two strings this branch added for states an operator hits once — the catalog write that matched nothing, and the delete that stranded a pointer — were collapsed from their multi-line form with the indentation left in, so both rendered with a fourteen-space gap mid-sentence. `list_datatables` claimed to report a chain it cannot follow and then dropped it; it does drop it, and the comment now says why that is the right place to stay quiet. The non-superadmin check in `edit_datatable_config` was introduced as also covering references, which it does not and need not: `reference` is overwritten from the stored entry for every caller before the check runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): serialize role catalog mutations, and state each helper's authorization contract The catalog is one JSON document, so create, rename, enable and delete are all read-modify-write. Two concurrent creates read the same snapshot, both succeed in the cluster, and the second write drops the first — leaving a live Postgres login with a password nobody recorded, which is the exact state the delete path exists to prevent. Every mutation now runs in one transaction holding an advisory lock across the read, the cluster DDL and the write, so a lost update cannot happen and a failure rolls the whole thing back. The DDL helpers take that transaction rather than the pool, which is what makes the lock cover them. Their statements moved off `sqlx::raw_sql`: the simple protocol is only needed for genuinely multi-statement SQL, and its future is not `Send`, which an axum handler holding the transaction requires. Each of these is one statement anyway. The new cross-crate surface now says what callers must do. `read_role_catalog` returns plaintext credentials; `create`/`rename`/`set_login`/`drop_instance_role` and `converge_connect_grants` mutate cluster-wide state; `read_datatable_entry` reads a workspace's raw config. All of them are superadmin-gated by their current handlers, but nothing said so at the definition, which is where the next caller looks. Also: the roles table reloads after a failed login toggle instead of leaving it claiming a flip that did not land; the rename affordance is the design-system `Button`, not a raw one; and `resolve_datatable_pg_as_caller` drops a `role` parameter no caller ever filled — browsing resolves as the data table's default until the database manager grows a picker. Why role passwords stay a plain `String` while the instance user's password beside them is a `StringOrSecretRef`, asked three times across reviews: that one is a secret ref because an operator supplies it and may want it from their own backend, while these are minted here and never entered by anyone, so there is nothing for a ref to point at. Encrypting generated secrets at rest is a separate change that would take the replication password with it. Now said at the field. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): give the role catalog its own row, out of reach of the config machinery Putting it inside `custom_instance_pg_databases` was the wrong call, and it cost two ways. The catalog serializes a generated Postgres password per role, and that row is the operator-facing instance config, so the passwords reached `get_instance_config` and its YAML editor — a live cluster credential in a response body, a UI field and any log of either. Worse in the other direction: `to_settings_map` strips the catalog, so a full-row upsert of that key writes the row back without it and the catalog is gone, while the cluster keeps every login it described. `custom_instance_replication_pwd` is the precedent and says exactly why — a generated secret, written only by the server, never operator-authored, hidden so the config machinery cannot read, rewrite or drop it. The catalog is the same thing, so it now has the same shape: `datatable_roles`, in `HIDDEN_SETTINGS`, `PROTECTED_SETTINGS` and the agent-worker denylist. No redaction to keep in step with three code paths, and no way for a neighbouring write to take it out. Two races on the same shared documents. `edit_datatable_config` read the stored data tables outside its transaction and then wrote the whole `datatable` document, so a permissions save committing in between was silently rolled back; it now reads under `FOR UPDATE`. And `set_datatable_permissions` validated role ids against the catalog before opening its transaction, so a deletion in between let it write a deleted role back — including as the default, which every later job then fails on; it now holds the catalog lock and the settings row across validation and write. Completes the authorization contracts the previous commit claimed but did not finish: `read_datatable_entry` (which it named and missed), `resolve_governing_datatable`, whose whole job is to answer for a workspace the caller may not belong to, and `converge_connect_grants_with`, which had not inherited its wrapper's. Also the generic Python SDK reference: `_format_py_params` learned the bare `*` last time, but `extract_py_functions` is a second formatter and still rendered `datatable(name, role)`, so code written from that page passed a keyword-only argument positionally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): make the concurrency test pin the handlers, and the contracts describe what is enforced The concurrency test reimplemented the read-modify-write inline, so deleting the lock from all three handlers left it green — it pinned Postgres, not the code it was written for. It now drives `create_datatable_role` twice concurrently and asserts the catalog kept both names. Checked the way the last one should have been: removing the lock from the handler makes it fail with "wmtest_a_… is a live cluster login the catalog forgot". The contracts added last commit were stricter than this PR's own callers, which is worse than none — the next reader sees a rule already broken and learns to ignore it. `read_role_catalog` said superadmin-only while two of its four callers are open to any workspace member, and `converge_connect_grants` said superadmin while `set_datatable_permissions` reaches it as a workspace admin. Both were fine on substance: the rule that actually holds is about the credential never reaching a response, log, audit record or export, not about who may call. They now say that. `read_datatable_entry` gets the same treatment rather than the one the earlier message claimed for it: it is the primitive every resolution goes through, so it is deliberately open, and what must not escape is `permissions` — it names the governing workspace's users, groups and folders. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): close the last ways a role or a pointer can be left pointing at nothing The raw settings readers hand back whatever is in the row, so moving the catalog into its own `global_settings` key protected the config machinery and left `GET /settings/global/datatable_roles` and the settings listing returning every live password. Both now filter that one key. The neighbouring `custom_instance_replication_pwd` has the same shape and is not touched here: it predates this and widening the fix to it is a decision about an operator workflow, not a consequence of this change. Three ways a save could leave something resolving to nothing: A permissioned data table could be moved to a PostgreSQL resource. The block was carried across as a server-owned field, the runtime refuses roles on a resource-backed table, so the save succeeded and every job afterwards failed. Refused instead — turning roles off first is one step, and it keeps discarding an access decision something somebody chose. Renaming a governing data table left every fork pointing at the old name: the data table disappears from their pickers and their jobs stop, with nothing in the renaming workspace to suggest why. The rename now follows into the pointers in the same transaction. Deleting one cannot be followed the same way, so it is reported instead — the response names what it stranded, the way deleting a workspace does, and the fork's own error already says which workspace is gone. Also: `ensure_instance_db_grant_options_unchecked` claimed superadmin while the permissions handler reaches it as a workspace admin (the same class fixed last commit, one instance missed); the role entry kept an `instance_config_schema` derive it no longer needs; `write_role_catalog` was the one writer of that table not stamping `updated_at`; and the concurrency test dropped its roles only on success — a failing run is exactly the one that creates them without recording them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * refactor(datatables): put the role catalog in its own table, not in global_settings Five findings across three rounds were all the same choice. A set of live Postgres credentials was living in `global_settings`, which has generic read, list, write, config-export and CLI round-trip paths that know nothing about what they carry: the passwords reached the instance config and its YAML editor, a full-row upsert of a neighbouring key erased the catalog, `GET /settings/global/{key}` and the settings listing returned them raw, and this round the redaction that fixed the last two turned `wmill instance push` into something that wipes every password — a fix breaking the assumption the previous fix made. `POST /settings/global/datatable_roles` could also empty it outside the lock. The approved plan offered a table or `global_settings`, so this is the other option it already allowed rather than a new design. `datatable_role` is a table: no generic settings path can read it, list it, export it, write it or round-trip it, so none of the five needs a guard. The redaction, the hidden/protected/agent-denylist entries and the JSON document all go with it. One row per role also removes the read-modify-write the concurrency work was about: two concurrent creates are two inserts, and the unique index on `name` is what settles a collision. The advisory lock stays for the one window rows do not cover — `CREATE ROLE` is invisible to another transaction until commit, so without it both creates pass their `pg_roles` check. Also from this round: rename mappings are checked against the configuration they claim to describe, since fork pointers are rewritten from them — a caller could otherwise submit `main -> missing` against an unchanged config and repoint every fork of `main` at a name nothing has, and `A -> B` plus `B -> C` moved what pointed at `A` all the way to `C`. And the warning naming forks a delete stranded reached the response but not the screen: both the data table settings save and the workspace delete now show it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): validate a rename against the save it describes, and re-check under the locks Three from the round, all about deciding on state that could already have moved. A permission save resolved the data table and checked it was instance-backed before taking any lock, then wrote under one. A config save committing in between could move the table onto a PostgreSQL resource — recreating exactly what the transition guard refuses — or rename it, in which case the write targeted a key that no longer existed and reported success having changed nothing. It now re-resolves and re-checks on the locked state. Rename validation checked that the source existed before and the target existed after, which still accepts `main -> decoy` against a save that keeps both: every fork of `main` then follows onto a different data table, silently, because it keeps resolving. The rule is now the actual old-to-new key transition — a source may only survive if another rename took its name, and a target may only pre-exist if another rename freed it. That also stops two sources sharing one target, and it admits a swap, which the previous guard refused: `datatables` is keyed by name, so a swap cannot be done one save at a time, and refusing it was a regression against main. The pointer cascade now runs in two passes through a temporary name, the way the migration cascade one layer down already handles the same shape, so `A -> B` with `B -> C` moves each pointer once from what it named before the save. The tenant mutators say what they are for: they write an access decision for any workspace named, with an arbitrary mutation, and exist for the transaction that frees or renames a principal. Editing a decision on purpose belongs in the permissions endpoint. Carried in the same change: the stranded-fork list is a field rather than a phrase to grep out of a success string; the pointer cascade matches with `EXISTS` instead of a `LIKE` over the whole document, so a workspace whose pointers name something else is not rewritten to a byte-identical value under an exclusive lock; and `InstanceDatatableRole` drops the serde derives left over from the JSON document, one of which would emit `pwd`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): cascade on the leave route that is used, gate migrations before the admin connection, and drop a role atomically The tenant cascade on leaving went onto `/users/leave`. The UI and the generated client call `/workspaces/leave` — a different handler in a different crate with the same name — which deleted the membership and left `u/<username>` in the tenant lists. Leaving and rejoining therefore restored the access the leave was supposed to end, and a later account taking the username would have inherited it. The regression test drives the route the client actually calls; without the fix it fails with "leaving kept the tenant". The migration endpoints authorized too late. `run_datatable_migrations` opened the data table's admin connection, created `_wm_migrations` and read it before reaching the per-migration role check — so with nothing pending, nothing was checked at all. Rollback returned before its check when nothing was applied, and the status endpoint had none. All three now ask, before any connection is opened, whether the caller can reach the data table as any role at all; which role a given migration runs as is still decided per migration, and by the executor after that. Deleting a role committed the cluster drop and the catalog row, then swept the tenant lists in separate transactions. A sweep failing part-way left workspaces naming a role nothing can connect as, while the retry answered `NotFound` because the catalog entry was already gone. The sweep now runs in the same transaction, so the drop, the row and every tenant list commit together. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): refuse to copy a data table that is under roles pg_dump carries no roles and the import runs with --no-privileges, so a copied data table arrives owned by the admin connection with no GRANT for any role. The settings clone brings `permissions` across, so the fork's tenants pass Windmill's check, connect as the role they were given, and are denied by Postgres on everything: an entry that reads as configured and answers nothing. Refuse the copy — in the import endpoint before any data moves, and in the fork path the CLI takes. Replaying the source's owners and ACLs into the clone is what lifts this, and is a change of its own. Dropping `permissions` from the copy instead would be the unsafe half, since the copy holds the parent's rows. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): refuse the clone's database too, not only its data A clone is two endpoints: `create_pg_database` then `import_pg_database`. Only the second refused a data table under roles, so a fork asking to clone one created and registered an empty `wm_fork_…` instance database and then failed — and nothing collects it, since `drop_forked_datatable_databases` only drops entries carrying `forked_from` and no entry names this one. Refuse in both, so the clone stops before a database exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * nit worker error msg * fix pg_dump stuck on version 17 on nix * fix(datatables): refuse a malformed role annotation instead of ignoring it `-- Role operator`, `-- role operator;` and `-- role operator -- why` all failed the annotation parser's exact-match rule, so the query fell through to the data table's default role and ran, silently, under a login the author did not choose. Naming a role exists precisely to not do that. A leading comment whose first word is `role` is now an annotation attempt: the keyword matches case-insensitively, one trailing `;` is tolerated, and anything else is an error naming the line. Only callers that already know the target is a `datatable://` reference ever run this, so ordinary SQL keeps its comments. Also bumps the dev shell's postgres client to 18 — it trailed the server the dev database runs, which takes out every data table export, clone and fork-with-data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): refuse a malformed role query string instead of ignoring it `?Role=analytics`, `?role=` and `?x=1&role=…` all fell through the reference parser's exact-match rule, so the connection resolved to the data table's default role and ran under a login the caller never asked for — the URI half of the same trap as a malformed `-- role` annotation. The key now matches case-insensitively, and anything else in the query string is an error naming it; `role` is the only parameter a reference takes. Callers that only need the entry keep a lenient `datatable_ref_name`, since they never act on the role. The DuckDB `ATTACH` parser propagates it rather than attaching under the default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ti5HyeTikPMYyW8YSdiHR * fix(datatables): carry the role annotation into the row_to_json retry The retry rebuilds its SQL from `pruneComments(code)`, so the leading comment block never reached the second attempt — and with it the `-- role <name>` line that decides which login the query runs as. The retry connected as the data table's default role instead, so a query the first attempt was denied could succeed on the second, reported as "recovered with the row_to_json fix". Carry the leading comment block over. The retry itself is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * chore(datatables): don't mount the roles UI until the ACL editor lands Enforcement ships first. The permissions drawer is what turns roles on, and the catalog section is what creates them — both are only useful once there is a way to grant a role the privileges it needs, which arrives with the ACL editor. Left mounted they would offer a feature whose other half does not exist. The two components are complete and reviewed; only their call sites here are commented out, with a note pointing the follow-up PRs at them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * fix(datatables): honour `-- role: x`, and fix the DuckDB attach test Two review findings, both real. `attach_datatable_parses_name_and_role` never compiled: `parse_attach_datatable` returns `Result<Option<_>>` now and one call site kept a single `unwrap`. Its `?Role=analytics` case also asserted a refusal, contradicting the parser in the same commit, which matches the key case-insensitively. Replaced with the cases that are genuinely malformed, and a positive one for the cased key. `-- role: analytics` fell through to the default role — the silent fallback the strict parser exists to remove, for the spelling most likely to be typed. The keyword now accepts an optional colon, attached or spaced, while a word that merely starts with it (`rolebased`) is still not an attempt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * fix(datatables): clone a fork's pointer instead of failing after the copy Forking a fork with cloning left an orphan database. The preflight resolves the pointer and sees the governing entry, so both endpoints ran and filled the new database; `apply_forked_datatable` then refused the inherited pointer and rolled the fork back, stranding a registered `wm_fork_*` that no entry names and whose name blocks the retry. Refusing earlier would have been the smaller change, but forking a fork and cloning worked before pointers existed, so it would trade an orphan for a regression. Resolve what the pointer names and write the terminal entry the clone needs: the whole `database` object rather than a patch of its `resource_path`, since a pointer has none, and `reference` removed with it. Also accepts `-- role=x` and `-- Role = x`, two more spellings that fell through to the default role. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * fix(datatables): refuse to roll back the catalog while roles exist The down migration dropped the table and left every role behind: live Postgres logins whose passwords only that table carried, so after a revert Windmill could neither use, disable nor delete them, and re-applying could not recreate them because the names were taken. Cleaning up here is not possible either — dropping a role means reassigning what it owns in every instance database, and a migration runs in one — so it now refuses while the catalog is non-empty and says to delete the roles through instance settings, which does the cluster work. Also enforces the instance-only invariant the resolved-pointer clone relies on rather than only asserting it in a comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * refactor(datatables): settle clonability in one place, before anything is created A clone is three stages a workspace apart — `create_pg_database`, then `import_pg_database`, then `apply_forked_datatable` inside the fork transaction. Only the third can roll back, and `CREATE DATABASE` is not transactional, so any refusal that lives there strands a registered `wm_fork_*` that no entry names and whose name blocks the retry. That orphan has now been fixed three times, most recently reintroduced by a guard added one commit ago. Patching each new refusal into the first endpoint is not the fix; having two places that can refuse is. `ensure_datatable_is_clonable` now answers every reason a copy can be refused and returns what it resolved, and the stage that writes the entry only does the work. Also takes an ACCESS EXCLUSIVE lock before the rollback guard counts, so a role created concurrently cannot slip between the check and the drop. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * fix(datatables): let a retried clone reclaim its own leftover database A clone creates its target database one request before it copies into it, and the fork that would name it is written a request after that. Any failure in between — a pg_dump error, a bad restore, a dropped connection, the source's roles changing mid-flow — left a registered `wm_fork_*` that no entry names, and every retry then failed on its name. This predates data table roles. `create_pg_database` now reclaims such a leftover before creating: only a `wm_fork_*` database Windmill registered as a data table database and that no data table or ducklake entry names, in any workspace, archived ones included. The drop never terminates connections, so a clone still copying into it makes the reclaim fail instead of being cut off. It is limited to callers who administer the source — reaching it is not enough, since on a data table without roles every member reaches it — and anyone else gets the refusal an existing database always got. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Revert "fix(datatables): let a retried clone reclaim its own leftover database" This reverts commit7dd3275a10. The reclaim tied the caller to the source they administer, but not to the database it dropped. Between another workspace's import and its final fork request, that workspace's target is full, registered, unnamed and has no open connection, so an admin of any instance data table could name it and have it dropped and recreated empty. The victim's fork would then commit pointing at the empty copy. Safe reclaim needs durable clone ownership and serialization with the request that names the database; until then the leftover stays, as it did before this PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(datatables): record the stale clone database as a known limitation A clone is three requests and `CREATE DATABASE` is not transactional, so a failure after the first leaves a registered `wm_fork_*` behind, as it did before data table roles. Accepted for this PR: it is harmless to data and goes away once the clone is a single server-side operation. The comment also records why the obvious fix is wrong: reclaiming the leftover on retry, without durable clone ownership, can drop another workspace's fully copied database between its import and its final fork request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): bounce the streams reading a data table when it is deleted Deleting a governing data table, or the workspace that holds it, only collected the fork pointers it stranded, for the warning. A Postgres trigger or capture already streaming through one of those pointers kept the replication connection it opened while the pointer still resolved, so it went on dispatching the governing database's rows after the fork lost access — until its connection happened to restart. The governing workspace's own streams on a deleted entry did the same. Both deletion paths now bounce the affected listeners inside their own transaction, through the helper a permission change already uses, so a listener that reconnects re-resolves the entry and finds it gone. The helper is split so a caller can pass the (workspace, local name) pairs it already holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): keep the fork schema baseline, and bounce streams on every removal Three fixes from review. `edit_datatable_config` took `forked_from` wholesale from the stored entry, so the fork schema diff's save of an advanced baseline was silently discarded and an applied change was offered again. Whether an entry carries a clone stamp is still carried from the store, since that is what marks its database droppable, but the baseline inside it is now taken from the request. The stranded-pointer warning and the stream bounce ran over the optional `deleted_datatables` hint, which the settings-sync CLI never sends, so removing a governing data table through `wmill` bounced nothing. Removals are now derived from the stored configuration against the saved one. `delete_workspace` read the pointers to bounce before its transaction, so a fork committing a pointer during the deletion was missed. The read now happens inside the transaction, after the workspace row is deleted: a fork's insert key-share locks that row through its parent foreign key, so it is either seen or fails on the missing parent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(datatables): keep Postgres triggers and data table roles apart A replication stream reads every row of every table whatever the data table's roles grant, and its listener checks access only when it connects. Rather than chase every way access can change and bounce the streams each one affects, a data table now carries one or the other: - a Postgres trigger or capture cannot be created on, or connect to, a data table under roles; - roles cannot be turned on while an enabled trigger or a live capture reads the data table, its own or a fork's through its pointer. The refusal names each one to disable. This removes the stream bounces on roles edits and on data table and workspace deletion, and the trigger gate that admitted admins. The fork schema baseline fix from the same review round is kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): refuse a Postgres trigger on a data table under roles when it is saved Creating or editing a trigger that points at a data table under roles was accepted, and its listener then retried the refused connection every 30 seconds forever. The save is now refused, and a trigger that reaches such a data table anyway (re-enabled, or cloned into a fork) is disabled by its listener with the reason, as a missing replication slot is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): disable a data table role before deleting it Deleting a role reassigns and drops what it owns in each registered database on its own connection, and each of those passes commits as it goes. A database failing part-way left the role enabled in the catalog and able to log in, but already stripped in the databases reached before it. The role is now disabled in its own commit first, so a failed delete leaves a disabled role to retry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): serialize roles going on with a stream starting Turning roles on looked for enabled triggers and live captures once, without a lock anything starting a stream also took. A trigger enabled in that window could have its listener connect before roles committed, and a healthy listener never checks again. Both transitions now serialize on one advisory lock: roles going on hold it exclusive while they look, and trigger create, edit and enable, and capture setup and ping hold it shared while they commit. Either the look sees the stream, or the listener connects after roles are committed and refuses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): wait out live listeners, and resolve stored names containing `?` Turning roles on counted a trigger as gone once disabled, and a capture once its client stopped pinging, but the listener keeps its replication connection until its next heartbeat notices. A trigger or capture whose listener pinged in the last 15 seconds, the window a server holds a listener for, now still counts as streaming. Data table names could contain `?` before they were restricted, and such entries are still stored. Splitting `?role=` off a reference misread them: `a?b` became `a` with an unknown parameter, and the clone checks looked at a different entry than the one copied. An entry stored under the whole reference is now looked up first, in the Postgres executor, DuckDB ATTACH and the clone checks. Agent workers cannot read the workspace and keep the strict parse, which refuses such a name rather than misreading it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): warn when a settings sync strands fork pointers A settings save reported the fork pointers left resolving to nothing only for the names in `deleted_datatables`, which `wmill sync push` never sends. The save now works out what it removed from the locked entries, and the CLI prints the stranded pointers it returns. Also correct the replication helper's contract: no role or admin check makes a replication connection safe, so a data table under roles is refused outright rather than gated as an admin operation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * 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 * fix(datatables): no entry without roles may newly reach a database under roles The previous guard only caught a new name replacing an entry under roles. A whole-map save could also repoint an existing entry without roles at that database, or another workspace could point one there, and every caller of that entry would connect as admin. The rule is now stated on the saved entries: one that carries no roles and newly points at an instance database any entry under roles uses, in this workspace or another, is refused. A declared rename carries its roles and passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * feat(datatables): move data table role catalog and resolution to the enterprise edition Roles are an Enterprise Edition feature. The catalog, the Postgres logins, CONNECT convergence, tenant evaluation and the role half of connection resolution move to windmill-ee-private. Every public function keeps its path and signature and forwards through datatable_roles_oss, which re-exports the enterprise implementation or, without it, refuses. Without the enterprise edition a data table under roles, or a caller naming a role, is refused a connection rather than resolved as admin, and the reach and admin-access checks refuse one under roles. A data table not under roles resolves as before in every edition, and an instance database keeps the CONNECT grants it was created with. The catalog lock, the stream lock, the tenant cascades and the permissions stripping stay in OSS: they only restrict. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * feat(datatables): move the data table permissions endpoints to the enterprise edition The permissions read, save and usable-roles handlers move to windmill-ee-private; the routes stay registered and, without the enterprise edition, answer that data table roles are an Enterprise Edition feature. ensure_governs_datatable and ensure_reaches_datatable keep their paths: the first refuses, the second passes a data table not under roles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * feat(datatables): move the data table role catalog endpoints to the enterprise edition The superadmin list, create, update and delete handlers move to windmill-ee-private. The routes stay registered and, without the enterprise edition, refuse after authentication. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * test(datatables): run the roles tests on the enterprise edition, refusals without it Each test that exercises roles runs with private and enterprise. Two tests run without them: every roles route answers the Enterprise refusal, and a data table saved under roles, or a named role, is refused a connection while one not under roles resolves as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * feat(datatables): gate the roles UI mount sites on an enterprise license Both mount sites are still commented out; the gate travels with them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * test(datatables): run the tenant matcher test on the enterprise edition The matcher it covers is enterprise code now, so without the enterprise edition the test hit the stub and failed the default windmill-common run. It runs with private and enterprise, and a counterpart without them asserts that no tenant list covers anyone, the wildcard and a workspace admin included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjfMkJyKzodxkobqGZ6Lqb * chore: update ee-repo-ref to a1873dbb67f2302b85ff5362f8387b48eccdb607 This commit updates the EE repository reference after PR #783 was merged in windmill-ee-private. Previous ee-repo-ref: 5c853e2c20eca6b748415fc0d6862a6ebfb5fec4 New ee-repo-ref: a1873dbb67f2302b85ff5362f8387b48eccdb607 Automated by sync-ee-ref workflow. * fix(datatables): refuse roles while a same-workspace alias reaches the database Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): let CE migrations connect as an explicitly named admin Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): serialize roles going on with aliases saved from other workspaces Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(datatables): note that legacy names with ? cannot be migrated Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(datatables): add an ACL editor for data table roles Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: take every pooled connection before the ACL apply locks Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: refresh grant options only after the ACL apply validates its plan Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: add only missing grant options before an ACL apply, never default privileges Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: run one data table ACL apply at a time per server before it connects Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: hold the ACL connection to the database that was authorized Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: build the ACL connection from the authorized data table entry Resolving the settings again could land on a resource with the same database name on another server, which the later entry checks never see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: check ACL read reach against the entry it connects from Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix(datatables): drop a DuckDB data table secret once its ATTACH has used it Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * perf(datatables): resolve a workspace's data tables per pointer hop, not per entry Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): hold the parent's settings while a fork points at its data tables Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(datatables): add an ACL editor for data table roles Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: take every pooled connection before the ACL apply locks Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: refresh grant options only after the ACL apply validates its plan Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: add only missing grant options before an ACL apply, never default privileges Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: run one data table ACL apply at a time per server before it connects Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: hold the ACL connection to the database that was authorized Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: build the ACL connection from the authorized data table entry Resolving the settings again could land on a resource with the same database name on another server, which the later entry checks never see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * fix: check ACL read reach against the entry it connects from Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb * chore: update ee-repo-ref to 7e338e4dabf91689bfd7fb0333c6534040b17b59 This commit updates the EE repository reference after PR #787 was merged in windmill-ee-private. Previous ee-repo-ref: 0edd40979cf36bfba59323f3f6a0811ae1369cf5 New ee-repo-ref: 7e338e4dabf91689bfd7fb0333c6534040b17b59 Automated by sync-ee-ref workflow. * feat(datatables): clone a data table under roles with its owners and grants (#11120) Claude-Session: https://claude.ai/code/session_01UbrtwiYNfayrmqouBJHwGV Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: open the raw app data table drawer when the workspace has none Selecting the first data table of an empty list passed undefined to the name check, which threw instead of opening the drawer on no data table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): offer cloning a data table under roles where its grants can be replayed The server clones such a data table and replays the source's owners and grants, which only the Enterprise Edition does, so the fork wizard hid both clone options everywhere instead of on a build that cannot replay them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: name the placeholder the empty raw app data drawer renders Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: pin the enterprise refusal the role pickers read as 'not under roles' The server's sentence and the frontend's copy of it were coupled by nothing, so rewording either one turned every role picker on a community build into a failed lookup. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): read a roles answer only for the workspace it was asked in A fork and its parent each have their own roles on a data table of the same name, so an answer stamped with the name alone settled the role from the workspace the editor was acting on before. Also derive the AI table creation flag from the data replaced into the editor: data naming no data table left the flag on from before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): keep an instance database a settings save is waiting to name Cleanup for a database whose setup failed took the lock first, read no user, and dropped it while a save blocked on that same lock was about to commit a reference to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): check the workspace stamp in the default database selector too Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): read what a save racing instance-database cleanup committed A transaction blocked on the lock may still roll back, so keeping the database for it stranded one whose name then blocks every retry: it is let through and its outcome read instead. The waiter query also matches this database's locks only, since pg_locks spans the cluster. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): tell a waiting request apart from the workspaces using a database Both callers render what cleanup returns as the workspaces that keep the database, so a waiting request's pid read as one of them. Each now words that case itself, and the give-up comment names where the kept name actually goes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): take the cleanup lock on a connection the pool cannot reclaim A session lock outlives the future holding it, so a cancellation between taking it and releasing it handed a locked session back to the pool, where every later settings save waits on it. Detached, the connection closes when it is dropped and the server releases the lock with the session. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(datatables): close the cleanup connection on drop instead of detaching it Detaching released the pool permit while the session stayed alive, so concurrent cleanups waiting on their locks could open as many connections as they liked. Closing on drop covers the same cancellation and keeps them counted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: update ee-repo-ref to fd5b8af748f2c985b13e18d9ea30894f3bd7e9a3 This commit updates the EE repository reference after PR #798 was merged in windmill-ee-private. Previous ee-repo-ref: 3145e422d61d580f0a82804f075285c112879da0 New ee-repo-ref: fd5b8af748f2c985b13e18d9ea30894f3bd7e9a3 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
Claude Opus 5
windmill-internal-app[bot]
parent
4f7f705f8c
commit
e1e3692fbc
+15
@@ -0,0 +1,15 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE workspace_settings ws\n SET datatable = (\n SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(\n dt.key,\n (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $2\n THEN jsonb_set(r.v, '{governed_by,workspace_id}', to_jsonb($1::text))\n ELSE r.v END\n FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $2\n THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text))\n ELSE dt.value END AS v) r)\n ))\n FROM jsonb_each(ws.datatable->'datatables') dt\n )\n WHERE jsonb_typeof(ws.datatable->'datatables') = 'object'\n AND (ws.datatable::text LIKE '%\"reference\"%'\n OR ws.datatable::text LIKE '%\"governed_by\"%')",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a"
|
||||
}
|
||||
+29
@@ -0,0 +1,29 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"datatable!\"\n FROM workspace_settings ws\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE EXISTS (\n SELECT 1 FROM (VALUES ('reference'), ('governed_by')) k(link)\n WHERE dt.value->k.link->>'workspace_id' = $1\n AND dt.value->k.link->>'datatable' = $2\n )",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "workspace_id!",
|
||||
"type_info": "Varchar"
|
||||
},
|
||||
{
|
||||
"ordinal": 1,
|
||||
"name": "datatable!",
|
||||
"type_info": "Text"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false,
|
||||
null
|
||||
]
|
||||
},
|
||||
"hash": "e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa"
|
||||
}
|
||||
+16
@@ -0,0 +1,16 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE workspace_settings ws\n SET datatable = (\n SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(\n dt.key,\n (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $1\n AND r.v->'governed_by'->>'datatable' = $2\n THEN jsonb_set(r.v, '{governed_by,datatable}', to_jsonb($3::text))\n ELSE r.v END\n FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $1\n AND dt.value->'reference'->>'datatable' = $2\n THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text))\n ELSE dt.value END AS v) r)\n ))\n FROM jsonb_each(ws.datatable->'datatables') dt\n )\n WHERE EXISTS (\n SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d,\n LATERAL (VALUES ('reference'), ('governed_by')) k(link)\n WHERE d.value->k.link->>'workspace_id' = $1\n AND d.value->k.link->>'datatable' = $2\n )",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Text",
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6"
|
||||
}
|
||||
@@ -1 +1 @@
|
||||
bc3ef08c8e4233508c023e6ee847a3cd0b8be43b
|
||||
fd5b8af748f2c985b13e18d9ea30894f3bd7e9a3
|
||||
|
||||
@@ -627,21 +627,234 @@ async fn a_rename_has_to_match_the_save_it_claims_to_describe(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn database_exists(db: &Pool<Postgres>, name: &str) -> anyhow::Result<bool> {
|
||||
Ok(
|
||||
sqlx::query_scalar("SELECT EXISTS (SELECT 1 FROM pg_database WHERE datname = $1)")
|
||||
.bind(name)
|
||||
.fetch_one(db)
|
||||
.await?,
|
||||
)
|
||||
}
|
||||
|
||||
async fn workspace_exists(db: &Pool<Postgres>, id: &str) -> anyhow::Result<bool> {
|
||||
Ok(
|
||||
sqlx::query_scalar("SELECT EXISTS (SELECT 1 FROM workspace WHERE id = $1)")
|
||||
.bind(id)
|
||||
.fetch_one(db)
|
||||
.await?,
|
||||
)
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
|
||||
async fn a_data_table_under_roles_is_not_copied_into_a_fork(
|
||||
async fn a_data_table_under_roles_is_cloned_only_with_its_grants(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
// The database a fork's copy of `main` goes into is named after the fork. Every request below
|
||||
// is refused before it is created, so the cluster-wide name never collides with a sibling run.
|
||||
let target = "wm_fork_copy__main";
|
||||
let fork = |token: &str, forked: Value| {
|
||||
authed(
|
||||
client().post(format!(
|
||||
"http://localhost:{port}/api/w/test-workspace/workspaces/create_fork"
|
||||
)),
|
||||
token,
|
||||
)
|
||||
.json(&json!({"id": "wm-fork-copy", "name": "copy", "forked_datatables": [forked]}))
|
||||
};
|
||||
|
||||
// Rows are a workspace admin's to copy, as they were before roles.
|
||||
let resp = fork(
|
||||
"SECRET_TOKEN_2",
|
||||
json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_and_data"}),
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 403, "{}", resp.text().await?);
|
||||
assert!(!database_exists(&db, target).await?);
|
||||
|
||||
// Even the schema alone lists every table, which a member covered by no role cannot read in
|
||||
// the parent.
|
||||
let resp = fork(
|
||||
"SECRET_TOKEN_3",
|
||||
json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_only"}),
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 401, "{}", resp.text().await?);
|
||||
assert!(!database_exists(&db, target).await?);
|
||||
|
||||
// Refused in the first phase too, before a git branch is created for a fork that cannot be.
|
||||
let resp = authed(
|
||||
client().post(format!(
|
||||
"http://localhost:{port}/api/w/test-workspace/workspaces/create_workspace_fork_branch"
|
||||
)),
|
||||
"SECRET_TOKEN_3",
|
||||
)
|
||||
.json(
|
||||
&json!({"id": "wm-fork-copy", "name": "copy", "forked_datatables": [
|
||||
{"name": "main", "new_dbname": target, "fork_behavior": "schema_only"}
|
||||
]}),
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 401, "{}", resp.text().await?);
|
||||
|
||||
// A database the request did not create — whatever the data table, under roles or not — may be
|
||||
// a copy left behind by a deleted fork, which the entry would reach without its governance.
|
||||
let resp = fork(
|
||||
"SECRET_TOKEN",
|
||||
json!({"name": "other", "new_dbname": "wm_fork_copy__other"}),
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 400);
|
||||
assert!(
|
||||
resp.text().await?.contains("fork_behavior"),
|
||||
"a database the fork did not create was taken"
|
||||
);
|
||||
assert!(!workspace_exists(&db, "wm-fork-copy").await?);
|
||||
|
||||
// The copy an admin asks for goes ahead in an edition that replays grants, and is refused
|
||||
// before any database exists in one that does not. The fixture's database does not exist, so
|
||||
// the dump fails either way — and leaves neither a database nor a fork behind.
|
||||
let resp = fork(
|
||||
"SECRET_TOKEN",
|
||||
json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_only"}),
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
assert_ne!(resp.status(), 200);
|
||||
let body = resp.text().await?;
|
||||
#[cfg(all(feature = "private", feature = "enterprise"))]
|
||||
assert!(body.contains("pg_dump"), "{body}");
|
||||
#[cfg(not(all(feature = "private", feature = "enterprise")))]
|
||||
assert!(body.contains("Enterprise Edition"), "{body}");
|
||||
assert!(!database_exists(&db, target).await?);
|
||||
assert!(!workspace_exists(&db, "wm-fork-copy").await?);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
|
||||
async fn a_clone_takes_its_roles_from_the_data_table_it_was_cloned_from(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
sqlx::query(
|
||||
r#"UPDATE workspace_settings SET datatable = jsonb_set(datatable, '{datatables,copy}', '{
|
||||
"database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"},
|
||||
"governed_by": {"workspace_id": "test-workspace", "datatable": "main"},
|
||||
"forked_from": {}
|
||||
}'::jsonb) WHERE workspace_id = 'wm-fork-dt'"#,
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let fork = format!("http://localhost:{port}/api/w/wm-fork-dt/workspaces");
|
||||
|
||||
// `test-user-2` administers the fork, and is only a tenant of `analytics` in the parent.
|
||||
let resp = authed(
|
||||
client().get(format!("{fork}/datatable_usable_roles/copy")),
|
||||
"SECRET_TOKEN_2",
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
let body: Value = resp.json().await?;
|
||||
assert_eq!(body["roles"], json!(["analytics"]), "{body}");
|
||||
|
||||
let resp = authed(
|
||||
client().get(format!("{fork}/datatable_permissions/copy")),
|
||||
"SECRET_TOKEN_2",
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
let body: Value = resp.json().await?;
|
||||
assert_eq!(body["editable"], false, "{body}");
|
||||
assert_eq!(body["clone_of"]["workspace_id"], "test-workspace", "{body}");
|
||||
|
||||
// Nobody changes a clone's roles, a superadmin included: they are the source's.
|
||||
for token in ["SECRET_TOKEN_2", "SECRET_TOKEN"] {
|
||||
let resp = authed(
|
||||
client().post(format!("{fork}/datatable_permissions/copy")),
|
||||
token,
|
||||
)
|
||||
.json(&json!({"permissioned": false}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 400, "{token} changed a clone's roles");
|
||||
}
|
||||
|
||||
// A settings save cannot clear the link.
|
||||
let resp = authed(
|
||||
client().post(format!("{fork}/edit_datatable_config")),
|
||||
"SECRET_TOKEN_2",
|
||||
)
|
||||
.json(&json!({
|
||||
"settings": {"datatables": {"main": {}, "copy": {
|
||||
"database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"}
|
||||
}}},
|
||||
"renames": [], "deleted_datatables": []
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200, "{}", resp.text().await?);
|
||||
let governed_by: Option<Value> = sqlx::query_scalar(
|
||||
"SELECT datatable->'datatables'->'copy'->'governed_by' FROM workspace_settings
|
||||
WHERE workspace_id = 'wm-fork-dt'",
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
assert_eq!(governed_by.unwrap()["datatable"], "main");
|
||||
|
||||
// Nor move it, a superadmin included: its grants were replayed into that database alone.
|
||||
let resp = authed(
|
||||
client().post(format!("{fork}/edit_datatable_config")),
|
||||
"SECRET_TOKEN",
|
||||
)
|
||||
.json(&json!({
|
||||
"settings": {"datatables": {"main": {}, "copy": {
|
||||
"database": {"resource_type": "instance", "resource_path": "wm_fork_dt__elsewhere"}
|
||||
}}},
|
||||
"renames": [], "deleted_datatables": []
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 400, "{}", resp.text().await?);
|
||||
|
||||
// Nor reach its database through a second entry without roles, which would connect as `admin`.
|
||||
let resp = authed(
|
||||
client().post(format!(
|
||||
"http://localhost:{port}/api/w/test-workspace/workspaces/edit_datatable_config"
|
||||
)),
|
||||
"SECRET_TOKEN",
|
||||
)
|
||||
.json(&json!({
|
||||
"settings": {"datatables": {
|
||||
"main": {"database": {"resource_type": "instance", "resource_path": "dt_main"}},
|
||||
"alias": {"database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"}}
|
||||
}},
|
||||
"renames": [], "deleted_datatables": []
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
let status = resp.status();
|
||||
let body = resp.text().await?;
|
||||
assert_eq!(status, 400, "{body}");
|
||||
assert!(body.contains("without carrying those roles"), "{body}");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
|
||||
async fn a_data_table_under_roles_is_not_copied_without_its_grants(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
// `pg_dump` carries no roles and the restore drops ACLs, so a copy would arrive with the
|
||||
// parent's tenants and none of the grants behind them: every role but admin denied by
|
||||
// Postgres in a data table that reads as configured. Refuse the copy rather than ship that,
|
||||
// and refuse it before any data moves.
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
|
||||
// Both halves of the clone: the database the copy would land in, then the copy itself. The
|
||||
// first has to refuse too, or a permissioned fork leaves an empty registered database that
|
||||
// no data table entry names and nothing collects.
|
||||
let resp = authed(
|
||||
client().post(format!(
|
||||
"http://localhost:{port}/api/w/test-workspace/workspaces/create_pg_database"
|
||||
@@ -912,6 +1125,18 @@ async fn a_stored_name_containing_a_question_mark_resolves_as_itself(
|
||||
resolve("main?dt").await.is_err(),
|
||||
"an unknown parameter was ignored"
|
||||
);
|
||||
|
||||
sqlx::query(
|
||||
"UPDATE workspace_settings
|
||||
SET datatable = jsonb_set(datatable, '{datatables,main?role=analytics}', datatable->'datatables'->'main')
|
||||
WHERE workspace_id = 'test-workspace'",
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
assert!(
|
||||
resolve("main?role=analytics").await.is_err(),
|
||||
"a reference naming both a stored data table and a role on another resolved to one of them"
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -1009,6 +1234,70 @@ async fn an_entry_without_roles_cannot_newly_reach_a_database_under_roles(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Browsing names the role it connects as, and a role the caller may not use is refused rather
|
||||
/// than quietly listed as the default. The refusal is decided before connecting, so the fixture's
|
||||
/// database never has to exist.
|
||||
#[cfg(all(feature = "private", feature = "enterprise"))]
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
|
||||
async fn browsing_as_a_role_the_caller_may_not_use_is_refused(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let base = format!("http://localhost:{port}/api/w/test-workspace/workspaces");
|
||||
|
||||
// `test-user-2` is a tenant of `analytics` only.
|
||||
let resp = authed(
|
||||
client().get(format!(
|
||||
"{base}/list_datatable_tables?role_for=main&role=admin"
|
||||
)),
|
||||
"SECRET_TOKEN_2",
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 200);
|
||||
let body: Value = resp.json().await?;
|
||||
let entry = body
|
||||
.as_array()
|
||||
.and_then(|a| a.iter().find(|e| e["datatable_name"] == "main"))
|
||||
.expect("main is listed");
|
||||
assert_eq!(entry["usable_roles"], json!(["analytics"]), "{entry}");
|
||||
assert_eq!(entry["default_role"], "analytics", "{entry}");
|
||||
assert_eq!(entry["permissioned"], true, "{entry}");
|
||||
assert_eq!(entry["instance"], true, "{entry}");
|
||||
let error = entry["error"].as_str().unwrap_or_default();
|
||||
assert!(
|
||||
error.contains("Not allowed to use role 'admin'"),
|
||||
"listed as another role than the one asked for: {entry}"
|
||||
);
|
||||
|
||||
let resp = authed(
|
||||
client().get(format!(
|
||||
"{base}/get_datatable_table_schema?datatable_name=main&schema_name=public&table_name=t&role=admin"
|
||||
)),
|
||||
"SECRET_TOKEN_2",
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
let status = resp.status();
|
||||
let text = resp.text().await?;
|
||||
assert!(
|
||||
text.contains("Not allowed to use role 'admin'"),
|
||||
"{status}: {text}"
|
||||
);
|
||||
|
||||
// A role means nothing without the data table it belongs to.
|
||||
let resp = authed(
|
||||
client().get(format!("{base}/list_datatable_tables?role=analytics")),
|
||||
"SECRET_TOKEN_2",
|
||||
)
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 400, "{}", resp.text().await?);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
|
||||
async fn an_alias_saved_elsewhere_waits_for_roles_going_on_for_its_database(
|
||||
db: Pool<Postgres>,
|
||||
@@ -1245,3 +1534,73 @@ async fn without_the_enterprise_edition_a_data_table_under_roles_is_refused_a_co
|
||||
assert!(err.to_string().contains(ENTERPRISE_REFUSAL), "{err}");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A save blocked on an instance database's lock is not a user of it until it commits one: the
|
||||
/// cleanup for a database whose setup failed lets that save through and reads what it left.
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
|
||||
async fn cleanup_waits_out_a_save_racing_it_for_an_instance_database(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let key = "instance_database:dt_probe";
|
||||
|
||||
// Queues cleanup behind `holder`, then a save behind cleanup, so cleanup takes the lock with
|
||||
// the save already waiting on it — the ordering the waiter check is for.
|
||||
let race = |commit: bool| {
|
||||
let db = db.clone();
|
||||
async move {
|
||||
let mut holder = db.acquire().await?;
|
||||
sqlx::query("SELECT pg_advisory_lock(hashtext($1))")
|
||||
.bind(key)
|
||||
.execute(&mut *holder)
|
||||
.await?;
|
||||
|
||||
let cleanup = tokio::spawn({
|
||||
let db = db.clone();
|
||||
async move { windmill_common::drop_unused_instance_database(&db, "dt_probe").await }
|
||||
});
|
||||
tokio::time::sleep(std::time::Duration::from_millis(300)).await;
|
||||
|
||||
let save = tokio::spawn({
|
||||
let db = db.clone();
|
||||
async move {
|
||||
let mut tx = db.begin().await?;
|
||||
windmill_common::lock_instance_databases(&mut tx, ["dt_probe"]).await?;
|
||||
sqlx::query(
|
||||
r#"UPDATE workspace_settings SET datatable = jsonb_set(datatable,
|
||||
'{datatables,probe}',
|
||||
'{"database": {"resource_type": "instance", "resource_path": "dt_probe"}}')
|
||||
WHERE workspace_id = 'test-workspace'"#,
|
||||
)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
if commit {
|
||||
tx.commit().await?;
|
||||
} else {
|
||||
tx.rollback().await?;
|
||||
}
|
||||
Ok::<_, anyhow::Error>(())
|
||||
}
|
||||
});
|
||||
tokio::time::sleep(std::time::Duration::from_millis(300)).await;
|
||||
|
||||
sqlx::query("SELECT pg_advisory_unlock(hashtext($1))")
|
||||
.bind(key)
|
||||
.execute(&mut *holder)
|
||||
.await?;
|
||||
save.await??;
|
||||
Ok::<_, anyhow::Error>(cleanup.await??)
|
||||
}
|
||||
};
|
||||
|
||||
assert!(
|
||||
matches!(race(false).await?, windmill_common::Cleanup::Dropped),
|
||||
"a save that rolled back kept the database, whose name then blocks every retry"
|
||||
);
|
||||
let kept = race(true).await?;
|
||||
let windmill_common::Cleanup::InUse(users) = kept else {
|
||||
anyhow::bail!("the database was dropped under a save that committed a reference to it")
|
||||
};
|
||||
assert_eq!(users, vec!["test-workspace".to_string()]);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -245,6 +245,8 @@ pub struct DatatableAclInfo {
|
||||
/// Whether this caller may plan and apply changes: they administer the data table, on an
|
||||
/// edition that has the planner.
|
||||
pub editable: bool,
|
||||
/// Whether the data table is a clone, whose grants stay as they were copied from its source.
|
||||
pub clone: bool,
|
||||
/// Whether the server is Postgres 17 or later, which added the `MAINTAIN` table privilege.
|
||||
pub supports_maintain: bool,
|
||||
/// The database the target lives in, which no target carries itself.
|
||||
@@ -335,13 +337,40 @@ async fn connect_as_admin_unchecked(
|
||||
pg.user = Some(CUSTOM_INSTANCE_USER.to_string());
|
||||
pg.password = Some(windmill_common::utils::get_custom_pg_instance_password(db).await?);
|
||||
let dbname = pg.dbname.clone();
|
||||
let (client, mut connection) = pg.connect(Some(db)).await?;
|
||||
let (client, notices) = connect_with_notices(db, &pg).await?;
|
||||
Ok((client, notices, dbname))
|
||||
}
|
||||
|
||||
/// A connection to `pg`, and the notices Postgres sends on it — which is where a grant or revoke
|
||||
/// that changed nothing is reported ([`execute_acl_statements`]).
|
||||
pub(crate) async fn connect_with_notices(
|
||||
db: &DB,
|
||||
pg: &PgDatabase,
|
||||
) -> Result<(tokio_postgres::Client, mpsc::UnboundedReceiver<DbError>)> {
|
||||
let (client, connection) = pg.connect(Some(db)).await?;
|
||||
Ok((
|
||||
client,
|
||||
drive_with_notices(connection, windmill_common::TokioPgConnection::poll_message),
|
||||
))
|
||||
}
|
||||
|
||||
type PollMessage<C> =
|
||||
fn(
|
||||
&mut C,
|
||||
&mut std::task::Context<'_>,
|
||||
) -> std::task::Poll<Option<std::result::Result<AsyncMessage, tokio_postgres::Error>>>;
|
||||
|
||||
/// Drive `connection` in the background with `poll`, forwarding its notices.
|
||||
pub(crate) fn drive_with_notices<C: Send + 'static>(
|
||||
mut connection: C,
|
||||
poll: PollMessage<C>,
|
||||
) -> mpsc::UnboundedReceiver<DbError> {
|
||||
// Unbounded: the driver must never wait on the receiver, which only drains once the statement
|
||||
// the driver is carrying has completed.
|
||||
let (notices_tx, notices) = mpsc::unbounded_channel();
|
||||
tokio::spawn(async move {
|
||||
loop {
|
||||
match std::future::poll_fn(|cx| connection.poll_message(cx)).await {
|
||||
match std::future::poll_fn(|cx| poll(&mut connection, cx)).await {
|
||||
Some(Ok(AsyncMessage::Notice(notice))) => {
|
||||
let _ = notices_tx.send(notice);
|
||||
}
|
||||
@@ -354,7 +383,41 @@ async fn connect_as_admin_unchecked(
|
||||
}
|
||||
}
|
||||
});
|
||||
Ok((client, notices, dbname))
|
||||
notices
|
||||
}
|
||||
|
||||
/// Run `statements` in order on `tx`, failing on the first that errors or that Postgres only warns
|
||||
/// about. A privilege the connection cannot pass on is a warning to Postgres (`01007` / `01006`),
|
||||
/// which then carries on having changed nothing; returning drops the transaction, rolling back
|
||||
/// everything before it.
|
||||
///
|
||||
/// Authorization: none. Runs `statements` as the connection `tx` is on; callers MUST have authorized
|
||||
/// changing that database's access, and built the statements themselves.
|
||||
pub(crate) async fn execute_acl_statements(
|
||||
tx: &tokio_postgres::Transaction<'_>,
|
||||
notices: &mut mpsc::UnboundedReceiver<DbError>,
|
||||
statements: &[String],
|
||||
) -> Result<()> {
|
||||
while notices.try_recv().is_ok() {}
|
||||
for statement in statements {
|
||||
tx.batch_execute(statement).await.map_err(|e| {
|
||||
Error::ExecutionErr(format!(
|
||||
"Failed to run `{statement}`: {}",
|
||||
pg_error_message(&e)
|
||||
))
|
||||
})?;
|
||||
while let Ok(notice) = notices.try_recv() {
|
||||
if *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_GRANTED
|
||||
|| *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_REVOKED
|
||||
{
|
||||
return Err(Error::ExecutionErr(format!(
|
||||
"`{statement}` did not take effect ({}), so nothing was applied",
|
||||
notice.message()
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// An object whose ownership follows the schema's.
|
||||
@@ -399,10 +462,12 @@ macro_rules! schema_owned_objects {
|
||||
AND x.refobjsubid <> 0)))"
|
||||
};
|
||||
}
|
||||
#[allow(unused_imports)]
|
||||
pub(crate) use schema_owned_objects;
|
||||
|
||||
/// The keyword `ALTER ... OWNER TO` takes for a kind of object, as `pg_identify_object` names the
|
||||
/// kind. A kind missing here is refused rather than skipped, which would leave it behind.
|
||||
fn owned_keyword(kind: &str) -> Option<&'static str> {
|
||||
pub(crate) fn owned_keyword(kind: &str) -> Option<&'static str> {
|
||||
Some(match kind {
|
||||
"table" => "TABLE",
|
||||
"view" => "VIEW",
|
||||
@@ -1054,6 +1119,7 @@ async fn get_datatable_acl(
|
||||
owner: role_name_of(&owner),
|
||||
roles,
|
||||
editable,
|
||||
clone: governing.governor.is_some(),
|
||||
supports_maintain,
|
||||
dbname,
|
||||
grants,
|
||||
@@ -1625,27 +1691,7 @@ async fn apply_datatable_acl(
|
||||
pg_error_message(&e)
|
||||
))
|
||||
})?;
|
||||
for statement in &plan.statements {
|
||||
pg_tx.batch_execute(statement).await.map_err(|e| {
|
||||
Error::ExecutionErr(format!(
|
||||
"Failed to run `{statement}`: {}",
|
||||
pg_error_message(&e)
|
||||
))
|
||||
})?;
|
||||
// A privilege the connection cannot pass on is only a warning to Postgres, which then
|
||||
// carries on having changed nothing. Returning drops the transaction, rolling back
|
||||
// everything before it.
|
||||
while let Ok(notice) = notices.try_recv() {
|
||||
if *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_GRANTED
|
||||
|| *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_REVOKED
|
||||
{
|
||||
return Err(Error::ExecutionErr(format!(
|
||||
"`{statement}` did not take effect ({}), so nothing was applied",
|
||||
notice.message()
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
execute_acl_statements(&pg_tx, &mut notices, &plan.statements).await?;
|
||||
|
||||
// A schema's objects were listed before the transaction opened; one committed since would stay
|
||||
// with its old owner. One created while this transaction is still open can still slip past, as
|
||||
|
||||
@@ -0,0 +1,566 @@
|
||||
/*
|
||||
* Author: Ruben Fiszel
|
||||
* Copyright: Windmill Labs, Inc 2022
|
||||
* This file and its contents are licensed under the AGPLv3 License.
|
||||
* Please see the included NOTICE for copyright information and
|
||||
* LICENSE-AGPL for a copy of the license.
|
||||
*/
|
||||
|
||||
//! Copying a data table's database for the fork being created.
|
||||
//!
|
||||
//! The fork request makes its copies before it writes the fork: each is created, filled and — for a
|
||||
//! data table under roles — given the source's owners and grants. What each copy was made from
|
||||
//! stays in the request ([`MadeCopy`]), so the fork's transaction checks it against the source as it
|
||||
//! is then, and a fork that fails drops the instance copies it made ([`drop_copies_after`]) and names
|
||||
//! the others, which live on servers of the workspace's own.
|
||||
|
||||
use std::collections::BTreeSet;
|
||||
|
||||
use windmill_api_auth::ApiAuthed;
|
||||
use windmill_common::datatable_roles::{lock_role_catalog, read_role_catalog_tx};
|
||||
use windmill_common::error::{pg_error_message, Error, Result};
|
||||
use windmill_common::utils::require_admin;
|
||||
use windmill_common::worker::CLOUD_HOSTED;
|
||||
use windmill_common::workspaces::{
|
||||
get_datatable_resource_from_db_unchecked, DataTableDatabase, DataTableForkBehavior,
|
||||
GoverningDatatable,
|
||||
};
|
||||
use windmill_common::{PgDatabase, DB};
|
||||
|
||||
use crate::datatable_acl::connect_with_notices;
|
||||
use crate::datatable_permissions::ensure_reaches_governing_datatable;
|
||||
use crate::workspaces::{
|
||||
create_database_on_server, ensure_datatable_is_clonable, pg_dump_database, pg_import_dump,
|
||||
DumpFile, PgDumpOptions,
|
||||
};
|
||||
|
||||
/// A database this request created and filled for one data table of the fork.
|
||||
pub(crate) struct MadeCopy {
|
||||
/// The data table's name, in the parent and in the fork.
|
||||
pub(crate) name: String,
|
||||
pub(crate) dbname: String,
|
||||
pub(crate) behavior: DataTableForkBehavior,
|
||||
/// The database the source resolved to when it was copied.
|
||||
pub(crate) source_database: DataTableDatabase,
|
||||
/// Whether the source's owners and grants were replayed into the copy: it was under roles.
|
||||
pub(crate) replayed: bool,
|
||||
/// For a resource-backed copy, the resource and variables its connection was resolved from.
|
||||
pub(crate) connection: Option<ConnectionSnapshot>,
|
||||
}
|
||||
|
||||
/// What a resource-backed data table's connection is resolved from: its resource, and every
|
||||
/// resource and variable that one references, as stored — and, for a secret kept in an external
|
||||
/// backend, the value that backend holds. The connection is resolved from the snapshot itself
|
||||
/// ([`ConnectionSnapshot::resolve`]), so it is exactly the one these rows describe.
|
||||
#[derive(PartialEq)]
|
||||
pub(crate) struct ConnectionSnapshot {
|
||||
root: String,
|
||||
resources: std::collections::BTreeMap<String, Option<serde_json::Value>>,
|
||||
/// Stored value and whether it is secret.
|
||||
variables: std::collections::BTreeMap<String, Option<(String, bool)>>,
|
||||
external_secrets: std::collections::BTreeMap<String, String>,
|
||||
}
|
||||
|
||||
/// Read the [`ConnectionSnapshot`] of resource `resource_path` in workspace `w_id`.
|
||||
///
|
||||
/// Authorization: none, and it holds secret values. Callers MUST have authorized using that
|
||||
/// resource, and never disclose the snapshot or what it resolves to.
|
||||
pub(crate) async fn connection_snapshot(
|
||||
db: &DB,
|
||||
conn: &mut sqlx::PgConnection,
|
||||
w_id: &str,
|
||||
resource_path: &str,
|
||||
) -> Result<ConnectionSnapshot> {
|
||||
let root = resource_path.trim_start_matches("$res:").to_string();
|
||||
let mut snapshot = ConnectionSnapshot {
|
||||
root: root.clone(),
|
||||
resources: Default::default(),
|
||||
variables: Default::default(),
|
||||
external_secrets: Default::default(),
|
||||
};
|
||||
let mut pending = vec![root];
|
||||
while let Some(path) = pending.pop() {
|
||||
if snapshot.resources.contains_key(&path) {
|
||||
continue;
|
||||
}
|
||||
let value: Option<serde_json::Value> =
|
||||
sqlx::query_scalar("SELECT value FROM resource WHERE workspace_id = $1 AND path = $2")
|
||||
.bind(w_id)
|
||||
.bind(&path)
|
||||
.fetch_optional(&mut *conn)
|
||||
.await?
|
||||
.flatten();
|
||||
let mut strings = vec![];
|
||||
collect_strings(value.as_ref(), &mut strings);
|
||||
for reference in strings {
|
||||
if let Some(var) = reference.strip_prefix("$var:") {
|
||||
if snapshot.variables.contains_key(var) {
|
||||
continue;
|
||||
}
|
||||
let row: Option<(String, bool)> = sqlx::query_as(
|
||||
"SELECT value, is_secret FROM variable WHERE workspace_id = $1 AND path = $2",
|
||||
)
|
||||
.bind(w_id)
|
||||
.bind(var)
|
||||
.fetch_optional(&mut *conn)
|
||||
.await?;
|
||||
if let Some((stored, true)) = &row {
|
||||
if windmill_common::secret_backend::is_external_stored_value(stored) {
|
||||
let secret = windmill_common::secret_backend::get_secret_value(
|
||||
db, w_id, var, stored,
|
||||
)
|
||||
.await?;
|
||||
snapshot.external_secrets.insert(var.to_string(), secret);
|
||||
}
|
||||
}
|
||||
snapshot.variables.insert(var.to_string(), row);
|
||||
} else if let Some(res) = reference.strip_prefix("$res:") {
|
||||
pending.push(res.to_string());
|
||||
}
|
||||
}
|
||||
snapshot.resources.insert(path, value);
|
||||
}
|
||||
Ok(snapshot)
|
||||
}
|
||||
|
||||
impl ConnectionSnapshot {
|
||||
/// The connection these rows resolve to, as the data table's own resolution would: references
|
||||
/// substituted, secrets decrypted.
|
||||
pub(crate) async fn resolve(&self, db: &DB, w_id: &str) -> Result<serde_json::Value> {
|
||||
let root = self.resource(&self.root)?;
|
||||
self.substitute(db, w_id, root, 0).await
|
||||
}
|
||||
|
||||
/// Whether the resource itself holds the connection's fields, which the fork repoints by setting
|
||||
/// its `dbname`, rather than being a reference to another resource.
|
||||
pub(crate) fn root_holds_connection(&self) -> bool {
|
||||
self.resource(&self.root).is_ok_and(|v| v.is_object())
|
||||
}
|
||||
|
||||
fn resource(&self, path: &str) -> Result<&serde_json::Value> {
|
||||
self.resources
|
||||
.get(path)
|
||||
.and_then(|v| v.as_ref())
|
||||
.ok_or_else(|| Error::NotFound(format!("resource {path} does not exist")))
|
||||
}
|
||||
|
||||
fn substitute<'a>(
|
||||
&'a self,
|
||||
db: &'a DB,
|
||||
w_id: &'a str,
|
||||
value: &'a serde_json::Value,
|
||||
depth: usize,
|
||||
) -> std::pin::Pin<Box<dyn std::future::Future<Output = Result<serde_json::Value>> + Send + 'a>>
|
||||
{
|
||||
Box::pin(async move {
|
||||
if depth > 32 {
|
||||
return Err(Error::BadRequest(
|
||||
"resource references nest too deeply".to_string(),
|
||||
));
|
||||
}
|
||||
Ok(match value {
|
||||
serde_json::Value::Object(map) => {
|
||||
let mut out = serde_json::Map::new();
|
||||
for (key, val) in map {
|
||||
out.insert(key.clone(), self.substitute(db, w_id, val, depth).await?);
|
||||
}
|
||||
serde_json::Value::Object(out)
|
||||
}
|
||||
serde_json::Value::Array(items) => {
|
||||
let mut out = vec![];
|
||||
for val in items {
|
||||
out.push(self.substitute(db, w_id, val, depth).await?);
|
||||
}
|
||||
serde_json::Value::Array(out)
|
||||
}
|
||||
serde_json::Value::String(s) if s.starts_with("$res:") => {
|
||||
let path = &s[5..];
|
||||
self.substitute(db, w_id, self.resource(path)?, depth + 1)
|
||||
.await?
|
||||
}
|
||||
serde_json::Value::String(s) if s.starts_with("$var:") => {
|
||||
let path = &s[5..];
|
||||
let (stored, secret) = self
|
||||
.variables
|
||||
.get(path)
|
||||
.and_then(|v| v.as_ref())
|
||||
.ok_or_else(|| {
|
||||
Error::NotFound(format!("variable {path} does not exist"))
|
||||
})?;
|
||||
serde_json::Value::String(match (secret, self.external_secrets.get(path)) {
|
||||
(false, _) => stored.clone(),
|
||||
(true, Some(external)) => external.clone(),
|
||||
(true, None) => windmill_common::variables::decrypt(
|
||||
&windmill_common::variables::build_crypt(db, w_id).await?,
|
||||
stored.clone(),
|
||||
)
|
||||
.map_err(|e| {
|
||||
Error::internal_err(format!("Error decrypting variable {s}: {e}"))
|
||||
})?,
|
||||
})
|
||||
}
|
||||
other => other.clone(),
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn collect_strings<'a>(value: Option<&'a serde_json::Value>, out: &mut Vec<&'a str>) {
|
||||
match value {
|
||||
Some(serde_json::Value::String(s)) => out.push(s),
|
||||
Some(serde_json::Value::Array(items)) => {
|
||||
items.iter().for_each(|v| collect_strings(Some(v), out))
|
||||
}
|
||||
Some(serde_json::Value::Object(map)) => {
|
||||
map.values().for_each(|v| collect_strings(Some(v), out))
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
/// What a data table of the fork should be copied as.
|
||||
pub(crate) struct CopyRequest<'a> {
|
||||
pub(crate) name: &'a str,
|
||||
pub(crate) dbname: &'a str,
|
||||
pub(crate) behavior: DataTableForkBehavior,
|
||||
}
|
||||
|
||||
/// Check that `authed` may copy each data table of `parent_w_id` as asked, before anything is
|
||||
/// created.
|
||||
///
|
||||
/// Who may copy is what it was before data table roles — anyone for the schema, an admin of the
|
||||
/// workspace for the rows — narrowed under roles to whoever may connect as one of them: even the
|
||||
/// schema alone lists every table, which under roles only role holders can read in the parent.
|
||||
pub(crate) async fn authorize_copies(
|
||||
db: &DB,
|
||||
authed: &ApiAuthed,
|
||||
parent_w_id: &str,
|
||||
requests: &[CopyRequest<'_>],
|
||||
) -> Result<()> {
|
||||
for request in requests {
|
||||
match request.behavior {
|
||||
DataTableForkBehavior::KeepOriginal => {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table '{}' is kept, which copies nothing",
|
||||
request.name
|
||||
)))
|
||||
}
|
||||
DataTableForkBehavior::SchemaOnly => {}
|
||||
DataTableForkBehavior::SchemaAndData => {
|
||||
require_admin(authed.is_admin, &authed.username)?;
|
||||
if *CLOUD_HOSTED {
|
||||
return Err(Error::BadRequest(
|
||||
"Cloning schema and data is not available on cloud".to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
windmill_common::validate_dbname(request.dbname)?;
|
||||
if !request.dbname.starts_with("wm_fork_") {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Forked datatable database name '{}' must start with 'wm_fork_'",
|
||||
request.dbname
|
||||
)));
|
||||
}
|
||||
let governing = ensure_datatable_is_clonable(db, parent_w_id, request.name).await?;
|
||||
ensure_reaches_governing_datatable(db, parent_w_id, request.name, &governing, authed)
|
||||
.await?;
|
||||
if governing.datatable.permissions.is_some() {
|
||||
crate::datatable_replay_oss::ensure_replay()?;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Make every copy in `requests`, in order, once [`authorize_copies`] allows them all. When one
|
||||
/// fails, the copies already made are dropped before the error is returned.
|
||||
pub(crate) async fn make_copies(
|
||||
db: &DB,
|
||||
authed: &ApiAuthed,
|
||||
parent_w_id: &str,
|
||||
requests: &[CopyRequest<'_>],
|
||||
) -> Result<Vec<MadeCopy>> {
|
||||
authorize_copies(db, authed, parent_w_id, requests).await?;
|
||||
let mut copies = Vec::with_capacity(requests.len());
|
||||
for request in requests {
|
||||
match make_copy(db, authed, parent_w_id, request).await {
|
||||
Ok(copy) => copies.push(copy),
|
||||
Err(e) => return Err(drop_copies_after(db, copies, e).await),
|
||||
}
|
||||
}
|
||||
Ok(copies)
|
||||
}
|
||||
|
||||
/// Drop every copy, returning `error` — with what could not be dropped appended, since nothing
|
||||
/// else will name those databases again.
|
||||
pub(crate) async fn drop_copies_after(db: &DB, copies: Vec<MadeCopy>, error: Error) -> Error {
|
||||
let mut stranded = Vec::new();
|
||||
for copy in copies {
|
||||
if let Err(e) = drop_copy(db, ©.source_database, ©.dbname).await {
|
||||
tracing::error!("Could not drop '{}' after a failed fork: {e}", copy.dbname);
|
||||
stranded.push(format!("'{}' ({e})", copy.dbname));
|
||||
}
|
||||
}
|
||||
if stranded.is_empty() {
|
||||
error
|
||||
} else {
|
||||
Error::ExecutionErr(format!(
|
||||
"{error}. These databases created for the fork could not be dropped: {}",
|
||||
stranded.join(", ")
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
async fn drop_copy(db: &DB, source_database: &DataTableDatabase, dbname: &str) -> Result<()> {
|
||||
if source_database.resource_type
|
||||
== windmill_common::workspaces::DataTableCatalogResourceType::Instance
|
||||
{
|
||||
match windmill_common::drop_unused_instance_database(db, dbname).await? {
|
||||
windmill_common::Cleanup::Dropped => Ok(()),
|
||||
windmill_common::Cleanup::InUse(users) => Err(Error::BadRequest(format!(
|
||||
"kept, since workspaces {} now use it",
|
||||
users.join(", ")
|
||||
))),
|
||||
windmill_common::Cleanup::Waiter(pid) => Err(Error::BadRequest(format!(
|
||||
"kept, since a request (pid {pid}) is still waiting to name it"
|
||||
))),
|
||||
}
|
||||
} else {
|
||||
// On a server of the workspace's own, where a resource edited meanwhile can already name it
|
||||
// and nothing locks such an edit: dropping it could take someone's data.
|
||||
Err(Error::BadRequest(
|
||||
"kept on its PostgreSQL server, where it may already be in use; drop it there once it \
|
||||
is not, before forking the same data table under this id again"
|
||||
.to_string(),
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
async fn make_copy(
|
||||
db: &DB,
|
||||
authed: &ApiAuthed,
|
||||
parent_w_id: &str,
|
||||
request: &CopyRequest<'_>,
|
||||
) -> Result<MadeCopy> {
|
||||
let governing = ensure_datatable_is_clonable(db, parent_w_id, request.name).await?;
|
||||
let source_database = governing.datatable.database.clone().ok_or_else(|| {
|
||||
Error::internal_err(format!(
|
||||
"Data table '{}' resolves to an entry that owns no database",
|
||||
request.name
|
||||
))
|
||||
})?;
|
||||
let is_instance = governing.is_instance();
|
||||
// Resolved from the snapshot, not read again: the connection the copy is made on is then
|
||||
// exactly what these rows describe, which the fork's own clone of them is checked against.
|
||||
let (server, connection): (PgDatabase, Option<ConnectionSnapshot>) = if is_instance {
|
||||
let server = serde_json::from_value(
|
||||
get_datatable_resource_from_db_unchecked(db, parent_w_id, request.name).await?,
|
||||
)
|
||||
.map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {e}")))?;
|
||||
(server, None)
|
||||
} else {
|
||||
let snapshot = connection_snapshot(
|
||||
db,
|
||||
&mut *db.acquire().await?,
|
||||
&governing.workspace_id,
|
||||
&source_database.resource_path,
|
||||
)
|
||||
.await?;
|
||||
if !snapshot.root_holds_connection() {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table '{}' uses resource '{}', which only refers to another resource: the \
|
||||
fork's copy of it could not be pointed at the copied database. Point the data \
|
||||
table at the resource holding the connection, then fork again.",
|
||||
request.name, source_database.resource_path
|
||||
)));
|
||||
}
|
||||
let server = serde_json::from_value(snapshot.resolve(db, &governing.workspace_id).await?)
|
||||
.map_err(|e| {
|
||||
Error::internal_err(format!("Failed to parse database credentials: {e}"))
|
||||
})?;
|
||||
(server, Some(snapshot))
|
||||
};
|
||||
|
||||
// Dumped before the database exists, so a source that cannot be read leaves nothing behind.
|
||||
// Ownership never carries over: the restore runs as the target's connection user. Grants do,
|
||||
// except on the instance, where the replay below is what reproduces them.
|
||||
let dump = pg_dump_database(
|
||||
&server,
|
||||
PgDumpOptions {
|
||||
schema_only: request.behavior == DataTableForkBehavior::SchemaOnly,
|
||||
no_owner: true,
|
||||
no_acl: is_instance,
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
if is_instance {
|
||||
windmill_common::create_custom_instance_database(db, request.dbname, "datatable").await?;
|
||||
} else {
|
||||
create_database_on_server(db, &server, request.dbname).await?;
|
||||
}
|
||||
|
||||
let target = PgDatabase { dbname: request.dbname.to_string(), ..server.clone() };
|
||||
let filled = fill(
|
||||
db,
|
||||
authed,
|
||||
parent_w_id,
|
||||
request.name,
|
||||
&governing,
|
||||
&server,
|
||||
&target,
|
||||
&dump,
|
||||
)
|
||||
.await;
|
||||
match filled {
|
||||
Ok(replayed) => Ok(MadeCopy {
|
||||
name: request.name.to_string(),
|
||||
dbname: request.dbname.to_string(),
|
||||
behavior: request.behavior,
|
||||
source_database,
|
||||
replayed,
|
||||
connection,
|
||||
}),
|
||||
Err(e) => {
|
||||
if let Err(drop_err) = drop_copy(db, &source_database, request.dbname).await {
|
||||
tracing::error!(
|
||||
"Could not drop '{}' after a failed copy of data table '{}': {drop_err}",
|
||||
request.dbname,
|
||||
request.name
|
||||
);
|
||||
return Err(Error::ExecutionErr(format!(
|
||||
"{e}. The database '{}' created for the copy could not be dropped: {drop_err}",
|
||||
request.dbname
|
||||
)));
|
||||
}
|
||||
Err(e)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Restore the dump into the new database and, for a data table under roles, replay the source's
|
||||
/// owners and grants into it. Returns whether it replayed.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn fill(
|
||||
db: &DB,
|
||||
authed: &ApiAuthed,
|
||||
parent_w_id: &str,
|
||||
name: &str,
|
||||
governing: &GoverningDatatable,
|
||||
source: &PgDatabase,
|
||||
target: &PgDatabase,
|
||||
dump: &DumpFile,
|
||||
) -> Result<bool> {
|
||||
pg_import_dump(target, dump).await?;
|
||||
|
||||
// Held until the replay commits: a role renamed or dropped meanwhile would change what the
|
||||
// replay names, and a settings save could move the source onto another database or put it under
|
||||
// roles. Taken in the same order as the permissions save and the ACL apply.
|
||||
// On a connection of its own: the checks below take theirs from the pool, and a lock holder
|
||||
// drawn from the pool too could leave a small one with nothing to give them.
|
||||
let database_url = windmill_common::get_database_url().await?;
|
||||
let mut lock_holder = <sqlx::PgConnection as sqlx::Connection>::connect_with(
|
||||
&database_url.connect_options().await?,
|
||||
)
|
||||
.await
|
||||
.map_err(|e| Error::internal_err(format!("Failed to connect to the database: {e}")))?;
|
||||
let mut tx = sqlx::Connection::begin(&mut lock_holder).await?;
|
||||
lock_role_catalog(&mut tx).await?;
|
||||
lock_settings_rows(&mut tx, parent_w_id, name).await?;
|
||||
|
||||
// Everything so far was decided before the locks.
|
||||
let now = ensure_datatable_is_clonable(db, parent_w_id, name).await?;
|
||||
ensure_reaches_governing_datatable(db, parent_w_id, name, &now, authed).await?;
|
||||
if !same_database(&now.datatable.database, &governing.datatable.database) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table '{name}' moved to another database while it was being copied; fork again"
|
||||
)));
|
||||
}
|
||||
|
||||
let replayed = now.datatable.permissions.is_some();
|
||||
if replayed {
|
||||
crate::datatable_replay_oss::ensure_replay()?;
|
||||
let catalog = read_role_catalog_tx(&mut tx).await?;
|
||||
// The replay leaves `CONNECT` to the catalog, and creating the database only tried to set
|
||||
// it: a copy `PUBLIC` could still connect to would admit logins the source turns away.
|
||||
windmill_common::datatable_roles::converge_connect_grants_with(
|
||||
db,
|
||||
&target.dbname,
|
||||
&catalog,
|
||||
)
|
||||
.await?;
|
||||
let catalog_roles: BTreeSet<String> = catalog.values().map(|r| r.name.clone()).collect();
|
||||
let (source, _source_notices) = connect_with_notices(db, source).await?;
|
||||
let (mut target, mut notices) = connect_with_notices(db, target).await?;
|
||||
// One transaction on the copy: a replay that stops halfway leaves objects owned by one role
|
||||
// and granted as another.
|
||||
let pg_tx = target.transaction().await.map_err(|e| {
|
||||
Error::internal_err(format!(
|
||||
"Failed to open a transaction on the copy: {}",
|
||||
pg_error_message(&e)
|
||||
))
|
||||
})?;
|
||||
crate::datatable_replay_oss::replay_owners_and_grants(
|
||||
&source,
|
||||
&pg_tx,
|
||||
&mut notices,
|
||||
&catalog_roles,
|
||||
)
|
||||
.await?;
|
||||
pg_tx.commit().await.map_err(|e| {
|
||||
Error::internal_err(format!(
|
||||
"Failed to commit the replayed grants: {}",
|
||||
pg_error_message(&e)
|
||||
))
|
||||
})?;
|
||||
}
|
||||
tx.commit().await?;
|
||||
Ok(replayed)
|
||||
}
|
||||
|
||||
/// Lock every settings row the resolution of data table `name` of `start_w_id` passes through, in
|
||||
/// the order it passes them — each pointer, the entry that owns the database, and each workspace its
|
||||
/// roles come from — and return them. Deleting or repointing any of them would leave a copy made
|
||||
/// for the resolution answering for another one.
|
||||
pub(crate) async fn lock_settings_rows(
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
start_w_id: &str,
|
||||
name: &str,
|
||||
) -> Result<Vec<String>> {
|
||||
let mut path: Vec<(String, String)> = vec![];
|
||||
let (mut w_id, mut datatable) = (start_w_id.to_string(), name.to_string());
|
||||
loop {
|
||||
if path.contains(&(w_id.clone(), datatable.clone())) || path.len() > 32 {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table '{name}' of workspace '{start_w_id}' resolves through a cycle"
|
||||
)));
|
||||
}
|
||||
let entry: Option<serde_json::Value> = sqlx::query_scalar(
|
||||
"SELECT datatable->'datatables'->$2 FROM workspace_settings
|
||||
WHERE workspace_id = $1 FOR UPDATE",
|
||||
)
|
||||
.bind(&w_id)
|
||||
.bind(&datatable)
|
||||
.fetch_optional(&mut **tx)
|
||||
.await?
|
||||
.flatten();
|
||||
path.push((w_id.clone(), datatable.clone()));
|
||||
let entry: Option<windmill_common::workspaces::DataTable> =
|
||||
entry.and_then(|e| serde_json::from_value(e).ok());
|
||||
match entry.and_then(|e| e.reference.or(e.governed_by)) {
|
||||
Some(next) => (w_id, datatable) = (next.workspace_id, next.datatable),
|
||||
None => break,
|
||||
}
|
||||
}
|
||||
Ok(path.into_iter().map(|(w_id, _)| w_id).collect())
|
||||
}
|
||||
|
||||
pub(crate) fn same_database(a: &Option<DataTableDatabase>, b: &Option<DataTableDatabase>) -> bool {
|
||||
match (a, b) {
|
||||
(Some(a), Some(b)) => {
|
||||
a.resource_type == b.resource_type && a.resource_path == b.resource_path
|
||||
}
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
@@ -14,6 +14,7 @@
|
||||
pub(crate) use crate::datatable_permissions_ee::{
|
||||
ensure_governs_datatable, ensure_reaches_datatable, ensure_reaches_governing_datatable,
|
||||
get_datatable_permissions, list_usable_datatable_roles, set_datatable_permissions,
|
||||
usable_datatable_roles,
|
||||
};
|
||||
|
||||
#[cfg(not(all(feature = "private", feature = "enterprise")))]
|
||||
@@ -84,4 +85,28 @@ mod ce {
|
||||
pub(crate) async fn list_usable_datatable_roles(_authed: ApiAuthed) -> Result<String> {
|
||||
Err(unavailable())
|
||||
}
|
||||
|
||||
pub(crate) struct UsableDatatableRoles {
|
||||
pub(crate) permissioned: bool,
|
||||
pub(crate) roles: Vec<String>,
|
||||
pub(crate) default_role: String,
|
||||
}
|
||||
|
||||
/// A data table not under roles is used as `admin`, as before roles existed. One under roles
|
||||
/// is refused: no role of it can be connected as.
|
||||
pub(crate) async fn usable_datatable_roles(
|
||||
_db: &DB,
|
||||
_authed: &ApiAuthed,
|
||||
_w_id: &str,
|
||||
governing: &GoverningDatatable,
|
||||
) -> Result<UsableDatatableRoles> {
|
||||
if governing.datatable.permissions.is_some() {
|
||||
return Err(unavailable());
|
||||
}
|
||||
Ok(UsableDatatableRoles {
|
||||
permissioned: false,
|
||||
roles: vec![],
|
||||
default_role: windmill_common::datatable_roles::ADMIN_DATATABLE_ROLE.to_string(),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,48 @@
|
||||
/*
|
||||
* Author: Ruben Fiszel
|
||||
* Copyright: Windmill Labs, Inc 2022
|
||||
* This file and its contents are licensed under the AGPLv3 License.
|
||||
* Please see the included NOTICE for copyright information and
|
||||
* LICENSE-AGPL for a copy of the license.
|
||||
*/
|
||||
|
||||
//! Where the replay of a data table's owners and grants into its copy comes from: the enterprise
|
||||
//! one, or a refusal. Without it a data table under roles is not copied at all: its rows would
|
||||
//! arrive owned by the admin connection with no grant for any role.
|
||||
|
||||
#[cfg(all(feature = "private", feature = "enterprise"))]
|
||||
pub(crate) use crate::datatable_replay_ee::replay_owners_and_grants;
|
||||
|
||||
#[cfg(all(feature = "private", feature = "enterprise"))]
|
||||
pub(crate) fn ensure_replay() -> windmill_common::error::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(not(all(feature = "private", feature = "enterprise")))]
|
||||
use {
|
||||
std::collections::BTreeSet,
|
||||
tokio::sync::mpsc,
|
||||
tokio_postgres::error::DbError,
|
||||
windmill_common::error::{Error, Result},
|
||||
};
|
||||
|
||||
/// Checked before anything is created.
|
||||
#[cfg(not(all(feature = "private", feature = "enterprise")))]
|
||||
pub(crate) fn ensure_replay() -> Result<()> {
|
||||
Err(Error::BadRequest(
|
||||
"Cloning a data table under roles is a Windmill Enterprise Edition feature: the copy \
|
||||
needs the source's owners and grants replayed. Fork it keeping the original database \
|
||||
instead."
|
||||
.to_string(),
|
||||
))
|
||||
}
|
||||
|
||||
#[cfg(not(all(feature = "private", feature = "enterprise")))]
|
||||
pub(crate) async fn replay_owners_and_grants(
|
||||
_source: &tokio_postgres::Client,
|
||||
_target: &tokio_postgres::Transaction<'_>,
|
||||
_notices: &mut mpsc::UnboundedReceiver<DbError>,
|
||||
_catalog_roles: &BTreeSet<String>,
|
||||
) -> Result<()> {
|
||||
ensure_replay()
|
||||
}
|
||||
@@ -3,9 +3,11 @@ pub mod ai_session_backups;
|
||||
pub mod data_metrics;
|
||||
pub mod datatable_acl;
|
||||
pub mod datatable_acl_oss;
|
||||
pub mod datatable_clone;
|
||||
pub mod datatable_migrations;
|
||||
pub mod datatable_permissions;
|
||||
pub mod datatable_permissions_oss;
|
||||
pub mod datatable_replay_oss;
|
||||
pub mod deployment_requests;
|
||||
pub mod workspaces;
|
||||
pub mod workspaces_extra;
|
||||
@@ -16,6 +18,8 @@ pub mod workspaces_ee;
|
||||
|
||||
#[cfg(all(feature = "private", feature = "enterprise"))]
|
||||
pub mod datatable_acl_ee;
|
||||
#[cfg(all(feature = "private", feature = "enterprise"))]
|
||||
pub mod datatable_replay_ee;
|
||||
|
||||
#[cfg(all(feature = "private", feature = "enterprise"))]
|
||||
pub mod datatable_permissions_ee;
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -502,21 +502,25 @@ pub(crate) async fn change_workspace_id(
|
||||
// A fork's data table entry names the workspace that governs it by id, so the rename has to
|
||||
// follow there too — anywhere, not just in the reparented children: a detached workspace can
|
||||
// point at this one without being its fork. Left behind, the pointer resolves to the archived
|
||||
// shell and every job through it stops.
|
||||
// shell and every job through it stops. A clone's `governed_by` names it the same way.
|
||||
info!("Re-pointing data table references to the new workspace id");
|
||||
sqlx::query!(
|
||||
r#"UPDATE workspace_settings ws
|
||||
SET datatable = (
|
||||
SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(
|
||||
dt.key,
|
||||
CASE WHEN dt.value->'reference'->>'workspace_id' = $2
|
||||
THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text))
|
||||
ELSE dt.value END
|
||||
(SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $2
|
||||
THEN jsonb_set(r.v, '{governed_by,workspace_id}', to_jsonb($1::text))
|
||||
ELSE r.v END
|
||||
FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $2
|
||||
THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text))
|
||||
ELSE dt.value END AS v) r)
|
||||
))
|
||||
FROM jsonb_each(ws.datatable->'datatables') dt
|
||||
)
|
||||
WHERE jsonb_typeof(ws.datatable->'datatables') = 'object'
|
||||
AND ws.datatable::text LIKE '%"reference"%'"#,
|
||||
AND (ws.datatable::text LIKE '%"reference"%'
|
||||
OR ws.datatable::text LIKE '%"governed_by"%')"#,
|
||||
&rw.new_id,
|
||||
&old_id,
|
||||
)
|
||||
@@ -1003,17 +1007,19 @@ pub(crate) async fn delete_workspace(
|
||||
// fails mid-way must never leave a live workspace with its fork data destroyed and no
|
||||
// registry row to retry from. Read-only: nothing is dropped here.
|
||||
// Read before the delete: another workspace's data table entry can point at one of this
|
||||
// workspace's, and deleting the workspace it names leaves that pointer resolving to nothing.
|
||||
// Nothing sweeps them — turning them back into copies would hand each fork the database
|
||||
// outright — so the deleter is told which data tables they just stranded.
|
||||
let stranded_pointers = sqlx::query!(
|
||||
r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!"
|
||||
// workspace's, or be a clone taking its roles from one, and deleting the workspace it names
|
||||
// leaves it resolving to nothing. Nothing sweeps them — turning them back into copies would
|
||||
// hand each fork the database outright — so the deleter is told which data tables they just
|
||||
// stranded.
|
||||
let stranded_pointers = sqlx::query_as::<_, (String, String)>(
|
||||
r#"SELECT ws.workspace_id, dt.key
|
||||
FROM workspace_settings ws
|
||||
CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt
|
||||
WHERE dt.value->'reference'->>'workspace_id' = $1
|
||||
OR dt.value->'governed_by'->>'workspace_id' = $1
|
||||
ORDER BY ws.workspace_id, dt.key"#,
|
||||
&w_id,
|
||||
)
|
||||
.bind(&w_id)
|
||||
.fetch_all(&db)
|
||||
.await
|
||||
.unwrap_or_default();
|
||||
@@ -1341,7 +1347,7 @@ pub(crate) async fn delete_workspace(
|
||||
} else {
|
||||
let stranded = stranded_pointers
|
||||
.iter()
|
||||
.map(|r| format!("{}/{}", r.workspace_id, r.datatable))
|
||||
.map(|(workspace_id, datatable)| format!("{workspace_id}/{datatable}"))
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ");
|
||||
Ok(format!(
|
||||
|
||||
@@ -5519,6 +5519,21 @@ paths:
|
||||
- workspace
|
||||
parameters:
|
||||
- $ref: "#/components/parameters/WorkspaceId"
|
||||
- name: datatable_name
|
||||
in: query
|
||||
description: list only this data table; each listed data table opens a connection to its database
|
||||
schema:
|
||||
type: string
|
||||
- name: role_for
|
||||
in: query
|
||||
description: the data table `role` applies to; every other one is listed as its default role
|
||||
schema:
|
||||
type: string
|
||||
- name: role
|
||||
in: query
|
||||
description: the role to list `role_for` as; refused, in that entry's `error`, if the caller may not use it
|
||||
schema:
|
||||
type: string
|
||||
responses:
|
||||
"200":
|
||||
description: table metadata of all datatables
|
||||
@@ -5552,6 +5567,11 @@ paths:
|
||||
required: true
|
||||
schema:
|
||||
type: string
|
||||
- name: role
|
||||
in: query
|
||||
description: the data table role to read the table as; defaults to the data table's default role
|
||||
schema:
|
||||
type: string
|
||||
responses:
|
||||
"200":
|
||||
description: schema of one datatable table
|
||||
@@ -33852,6 +33872,15 @@ components:
|
||||
$ref: "#/components/schemas/DatatableRoleTenants"
|
||||
governing_workspace_id:
|
||||
type: string
|
||||
clone_of:
|
||||
type: object
|
||||
description: for a clone, the data table whose roles it takes
|
||||
required: [workspace_id, datatable]
|
||||
properties:
|
||||
workspace_id:
|
||||
type: string
|
||||
datatable:
|
||||
type: string
|
||||
editable:
|
||||
type: boolean
|
||||
available_roles:
|
||||
@@ -34090,7 +34119,7 @@ components:
|
||||
|
||||
DatatableAclInfo:
|
||||
type: object
|
||||
required: [owner, roles, editable, supports_maintain, dbname, grants, children]
|
||||
required: [owner, roles, editable, clone, supports_maintain, dbname, grants, children]
|
||||
properties:
|
||||
owner:
|
||||
type: string
|
||||
@@ -34102,6 +34131,9 @@ components:
|
||||
editable:
|
||||
type: boolean
|
||||
description: whether the caller may plan and apply changes
|
||||
clone:
|
||||
type: boolean
|
||||
description: whether this is a clone, whose grants stay as they were copied
|
||||
supports_maintain:
|
||||
type: boolean
|
||||
description: whether the server is Postgres 17+, which added the MAINTAIN table privilege
|
||||
@@ -35254,6 +35286,16 @@ components:
|
||||
new_dbname:
|
||||
type: string
|
||||
description: "New database name for the fork"
|
||||
fork_behavior:
|
||||
type: string
|
||||
enum:
|
||||
- schema_only
|
||||
- schema_and_data
|
||||
description: >-
|
||||
What the fork request copies into `new_dbname`, which it creates — with the
|
||||
owners and grants of a data table under roles. This server refuses an entry
|
||||
without it; servers predating it expect `new_dbname` created and filled
|
||||
beforehand.
|
||||
shared_ducklakes:
|
||||
type: array
|
||||
items:
|
||||
@@ -36217,6 +36259,17 @@ components:
|
||||
type: string
|
||||
datatable:
|
||||
type: string
|
||||
governed_by:
|
||||
description: >-
|
||||
On a clone, the data table it was copied from, whose roles it takes. Server-owned
|
||||
like `reference`.
|
||||
type: object
|
||||
required: [workspace_id, datatable]
|
||||
properties:
|
||||
workspace_id:
|
||||
type: string
|
||||
datatable:
|
||||
type: string
|
||||
migrations_enabled:
|
||||
type: boolean
|
||||
description: Whether the SQL migrations feature is opted in for this data table
|
||||
@@ -36285,7 +36338,17 @@ components:
|
||||
|
||||
DataTableTables:
|
||||
type: object
|
||||
required: [datatable_name, schemas]
|
||||
required:
|
||||
[
|
||||
datatable_name,
|
||||
schemas,
|
||||
instance,
|
||||
permissioned,
|
||||
usable_roles,
|
||||
default_role,
|
||||
can_create_schema,
|
||||
creatable_schemas,
|
||||
]
|
||||
properties:
|
||||
datatable_name:
|
||||
type: string
|
||||
@@ -36298,6 +36361,26 @@ components:
|
||||
type: string
|
||||
error:
|
||||
type: string
|
||||
instance:
|
||||
type: boolean
|
||||
description: on the instance database, the only kind that can be under roles or have its access edited
|
||||
permissioned:
|
||||
type: boolean
|
||||
usable_roles:
|
||||
type: array
|
||||
description: the roles the caller may connect as, by name; empty when not under roles
|
||||
items:
|
||||
type: string
|
||||
default_role:
|
||||
type: string
|
||||
can_create_schema:
|
||||
type: boolean
|
||||
description: whether the role the listing connected as may create schemas
|
||||
creatable_schemas:
|
||||
type: array
|
||||
description: the schemas the role the listing connected as may create in
|
||||
items:
|
||||
type: string
|
||||
|
||||
DataTableTableSchema:
|
||||
type: object
|
||||
|
||||
@@ -8723,7 +8723,12 @@ fn register_potential_assets_on_inline_execution(
|
||||
.as_ref()
|
||||
.and_then(|args| args.get("database"))
|
||||
.map(|v| v.get().trim_matches('"'))
|
||||
.and_then(|dt| dt.strip_prefix("datatable://"));
|
||||
.and_then(|dt| dt.strip_prefix("datatable://"))
|
||||
// `?role=` picks the connection, not the data table. Anything else after a `?` may be
|
||||
// part of a name stored before names were restricted, so it stays.
|
||||
.map(|dt| {
|
||||
windmill_common::workspaces::parse_datatable_ref(dt).map_or(dt, |(name, _)| name)
|
||||
});
|
||||
if let Some(datatable) = datatable {
|
||||
let re = regex::Regex::new(r#"SET search_path TO "([^"]+)";"#).unwrap();
|
||||
let (schema, content) = if let Some(captures) = re.captures(&preview.content) {
|
||||
|
||||
@@ -15,7 +15,8 @@
|
||||
|
||||
use crate::error::Error;
|
||||
|
||||
/// What every roles path answers without the Enterprise Edition.
|
||||
/// What every roles path answers without the Enterprise Edition. The frontend matches this exact
|
||||
/// sentence (`datatableUsableRoles.ts`) to read the refusal as "not under roles": reword both.
|
||||
pub fn datatable_roles_unavailable() -> Error {
|
||||
Error::BadRequest("Data table roles are a Windmill Enterprise Edition feature".to_string())
|
||||
}
|
||||
|
||||
@@ -1474,8 +1474,163 @@ pub fn validate_dbname(dbname: &str) -> error::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Lock the instance databases among `names` until `tx` ends, in a stable order. Taken by every
|
||||
/// settings save naming an instance database, and by [`drop_unused_instance_database`]: a save
|
||||
/// cannot start using a database between that drop's check that nothing does and the drop.
|
||||
pub async fn lock_instance_databases<'a>(
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
names: impl IntoIterator<Item = &'a str>,
|
||||
) -> error::Result<()> {
|
||||
let names: std::collections::BTreeSet<&str> = names.into_iter().collect();
|
||||
for name in names {
|
||||
sqlx::query("SELECT pg_advisory_xact_lock(hashtext('instance_database:' || $1))")
|
||||
.bind(name)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Drop instance database `dbname`, which a request just created, unless a workspace names it as a
|
||||
/// data table or a ducklake catalog — then it is kept, and who keeps it is what comes back. A
|
||||
/// database is registered on the instance as soon as it is created, so a superadmin can point a
|
||||
/// workspace at it before the request that created it gives up on it.
|
||||
///
|
||||
/// Authorization: none. Callers MUST pass only a database the same request created and has not
|
||||
/// handed to anything yet.
|
||||
///
|
||||
/// The lock, the check and the drop share one connection: a second one taken from the pool while
|
||||
/// the first is held could wait forever on a small pool. That connection is closed rather than
|
||||
/// returned, since a session lock outlives the future holding it: a cancellation between taking
|
||||
/// the lock and releasing it would otherwise hand a locked session back to the pool, where every
|
||||
/// later settings save waits on it. Closing on drop covers the cancellation and still counts the
|
||||
/// connection against the pool, which detaching it would not.
|
||||
pub async fn drop_unused_instance_database(db: &DB, dbname: &str) -> error::Result<Cleanup> {
|
||||
let mut conn = db.acquire().await?;
|
||||
conn.close_on_drop();
|
||||
let key = format!("instance_database:{dbname}");
|
||||
// A save blocked on this lock is about to name the database, but it may also roll back — a
|
||||
// later validation of its own, a superadmin check, a cancelled request. So it is let through
|
||||
// and what it committed is read, rather than taken as a user: trusting it would strand the
|
||||
// database, whose name then blocks every retry. A fresh waiter can always arrive, so after a
|
||||
// few rounds the database is kept instead; the name is then a superadmin's to drop from
|
||||
// instance settings, since a retry fails on the name before reaching this cleanup.
|
||||
let mut rounds = 0;
|
||||
let dropped = loop {
|
||||
if let Err(e) = sqlx::query("SELECT pg_advisory_lock(hashtext($1))")
|
||||
.bind(&key)
|
||||
.execute(&mut *conn)
|
||||
.await
|
||||
{
|
||||
break Err(e.into());
|
||||
}
|
||||
match drop_if_unused_on(&mut conn, dbname).await {
|
||||
Ok(Cleanup::Waiter(_)) if rounds < 2 => {
|
||||
rounds += 1;
|
||||
// Releasing hands the lock to the waiter; re-taking it above then waits for that
|
||||
// transaction to end, so the next round reads what it actually committed.
|
||||
if let Err(e) = unlock_instance_database(&mut conn, &key).await {
|
||||
break Err(e);
|
||||
}
|
||||
}
|
||||
Ok(outcome) => break Ok(outcome),
|
||||
Err(e) => break Err(e),
|
||||
}
|
||||
};
|
||||
// Dropping closes the connection, and the server releases the lock with the session, so
|
||||
// nothing here depends on an unlock landing.
|
||||
dropped
|
||||
}
|
||||
|
||||
async fn unlock_instance_database(conn: &mut sqlx::PgConnection, key: &str) -> error::Result<()> {
|
||||
sqlx::query("SELECT pg_advisory_unlock(hashtext($1))")
|
||||
.bind(key)
|
||||
.execute(&mut *conn)
|
||||
.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// What [`drop_unused_instance_database`] did, and for whom when it kept the database.
|
||||
pub enum Cleanup {
|
||||
Dropped,
|
||||
/// Workspaces that name the database, as [`instance_database_users`] reports them.
|
||||
InUse(Vec<String>),
|
||||
/// A transaction still blocked on the lock after the rounds above, so what it will commit
|
||||
/// stays unknown, named as its `pid`.
|
||||
Waiter(i32),
|
||||
}
|
||||
|
||||
async fn drop_if_unused_on(conn: &mut sqlx::PgConnection, dbname: &str) -> error::Result<Cleanup> {
|
||||
let users = instance_database_users(conn, dbname).await?;
|
||||
if !users.is_empty() {
|
||||
return Ok(Cleanup::InUse(users));
|
||||
}
|
||||
// A settings save naming this database takes the same lock, so one waiting on it would read
|
||||
// its own users after the drop and commit a reference to nothing.
|
||||
if let Some(waiter) = waiting_for_instance_database(conn, dbname).await? {
|
||||
return Ok(Cleanup::Waiter(waiter));
|
||||
}
|
||||
drop_custom_instance_database_on(conn, dbname).await?;
|
||||
Ok(Cleanup::Dropped)
|
||||
}
|
||||
|
||||
/// The `pid` of a transaction blocked on `dbname`'s instance-database lock. The lock is
|
||||
/// taken by key, so `pg_locks` reports it split across `classid` and `objid`, and it lists every
|
||||
/// database on the cluster — another Windmill on the same one holds its own locks under the same
|
||||
/// key.
|
||||
async fn waiting_for_instance_database(
|
||||
conn: &mut sqlx::PgConnection,
|
||||
dbname: &str,
|
||||
) -> error::Result<Option<i32>> {
|
||||
let pid: Option<i32> = sqlx::query_scalar(
|
||||
"SELECT l.pid FROM pg_locks l
|
||||
WHERE l.locktype = 'advisory' AND NOT l.granted
|
||||
AND l.database = (SELECT oid FROM pg_database WHERE datname = current_database())
|
||||
AND l.classid = ((hashtext('instance_database:' || $1)::bigint >> 32) & 4294967295)::oid
|
||||
AND l.objid = (hashtext('instance_database:' || $1)::bigint & 4294967295)::oid
|
||||
LIMIT 1",
|
||||
)
|
||||
.bind(dbname)
|
||||
.fetch_optional(&mut *conn)
|
||||
.await?;
|
||||
Ok(pid)
|
||||
}
|
||||
|
||||
/// The workspaces naming instance database `dbname` as a data table or a ducklake catalog. Taken
|
||||
/// under [`lock_instance_databases`] for `dbname`, the answer holds until that lock is released.
|
||||
///
|
||||
/// Authorization: none, and it reads every workspace's settings. Callers MUST pass only a database
|
||||
/// their own request created, and may name the workspaces returned only to a caller allowed to
|
||||
/// create or drop instance databases.
|
||||
pub async fn instance_database_users(
|
||||
conn: &mut sqlx::PgConnection,
|
||||
dbname: &str,
|
||||
) -> error::Result<Vec<String>> {
|
||||
Ok(sqlx::query_scalar(
|
||||
"SELECT DISTINCT ws.workspace_id FROM workspace_settings ws
|
||||
WHERE EXISTS (SELECT 1 FROM jsonb_each(CASE WHEN jsonb_typeof(ws.datatable->'datatables') = 'object'
|
||||
THEN ws.datatable->'datatables' ELSE '{}'::jsonb END) dt
|
||||
WHERE dt.value->'database'->>'resource_type' = 'instance'
|
||||
AND dt.value->'database'->>'resource_path' = $1)
|
||||
OR EXISTS (SELECT 1 FROM jsonb_each(CASE WHEN jsonb_typeof(ws.ducklake->'ducklakes') = 'object'
|
||||
THEN ws.ducklake->'ducklakes' ELSE '{}'::jsonb END) dl
|
||||
WHERE dl.value->'catalog'->>'resource_type' = 'instance'
|
||||
AND dl.value->'catalog'->>'resource_path' = $1)",
|
||||
)
|
||||
.bind(dbname)
|
||||
.fetch_all(conn)
|
||||
.await?)
|
||||
}
|
||||
|
||||
/// Drop a custom instance database: validate, terminate connections, DROP DATABASE, remove from global_settings.
|
||||
pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Result<()> {
|
||||
drop_custom_instance_database_on(&mut *db.acquire().await?, dbname).await
|
||||
}
|
||||
|
||||
async fn drop_custom_instance_database_on(
|
||||
conn: &mut sqlx::PgConnection,
|
||||
dbname: &str,
|
||||
) -> error::Result<()> {
|
||||
let dbname = dbname.trim();
|
||||
validate_dbname(dbname)?;
|
||||
|
||||
@@ -1490,7 +1645,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu
|
||||
"SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_database WHERE datname = $1)",
|
||||
dbname
|
||||
)
|
||||
.fetch_one(db)
|
||||
.fetch_one(&mut *conn)
|
||||
.await?
|
||||
.unwrap_or(false);
|
||||
|
||||
@@ -1501,7 +1656,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu
|
||||
"SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '{}' AND pid <> pg_backend_pid()",
|
||||
dbname.replace('\'', "''")
|
||||
))
|
||||
.execute(db)
|
||||
.execute(&mut *conn)
|
||||
.await
|
||||
{
|
||||
tracing::warn!("Failed to terminate connections to '{}': {}", dbname, e);
|
||||
@@ -1510,7 +1665,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu
|
||||
// Drop the database
|
||||
// SAFETY: `dbname` has been validated via validate_dbname() before reaching this point.
|
||||
sqlx::query(&format!("DROP DATABASE IF EXISTS \"{}\"", dbname))
|
||||
.execute(db)
|
||||
.execute(&mut *conn)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
error::Error::internal_err(format!("Failed to drop database '{}': {}", dbname, e))
|
||||
@@ -1526,7 +1681,7 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu
|
||||
r#"UPDATE global_settings SET value = value #- ARRAY['databases', $1] WHERE name = 'custom_instance_pg_databases'"#,
|
||||
dbname
|
||||
)
|
||||
.execute(db)
|
||||
.execute(&mut *conn)
|
||||
.await?;
|
||||
|
||||
Ok(())
|
||||
@@ -1600,6 +1755,39 @@ pub async fn create_custom_instance_database(
|
||||
error::Error::internal_err(format!("Failed to create database '{}': {}", dbname, e))
|
||||
})?;
|
||||
|
||||
// Nothing names a database that failed past this point, and its name blocks the retry: drop it
|
||||
// rather than leave it behind.
|
||||
if let Err(e) = finish_custom_instance_database(db, dbname, tag).await {
|
||||
match drop_unused_instance_database(db, dbname).await {
|
||||
Ok(Cleanup::InUse(users)) => tracing::warn!(
|
||||
"Kept '{dbname}' after failing to set it up: workspaces {} use it",
|
||||
users.join(", ")
|
||||
),
|
||||
Ok(Cleanup::Waiter(pid)) => tracing::warn!(
|
||||
"Kept '{dbname}' after failing to set it up: a request (pid {pid}) is still \
|
||||
waiting to name it. Drop it from instance settings once it is unused."
|
||||
),
|
||||
Ok(Cleanup::Dropped) => {}
|
||||
Err(drop_err) => {
|
||||
tracing::error!("Could not drop '{dbname}' after failing to set it up: {drop_err}")
|
||||
}
|
||||
}
|
||||
return Err(e);
|
||||
}
|
||||
|
||||
// A data table role can only reach a database it may CONNECT to, and PUBLIC's default CONNECT
|
||||
// would otherwise let every role in regardless of what this instance defines. Best-effort: a
|
||||
// failure here leaves the database usable as `admin`, and the next role change repairs it.
|
||||
if let Err(e) = crate::datatable_roles::converge_connect_grants(db, dbname).await {
|
||||
tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}");
|
||||
}
|
||||
|
||||
tracing::info!("Created custom instance database '{}'", dbname);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Grant `custom_instance_user` its privileges on a database just created, and register it.
|
||||
async fn finish_custom_instance_database(db: &DB, dbname: &str, tag: &str) -> error::Result<()> {
|
||||
// Grant permissions to custom_instance_user
|
||||
let wmill_pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?;
|
||||
let new_pg_creds = PgDatabase { dbname: dbname.to_string(), ..wmill_pg_creds };
|
||||
@@ -1634,15 +1822,6 @@ pub async fn create_custom_instance_database(
|
||||
)
|
||||
.execute(db)
|
||||
.await?;
|
||||
|
||||
// A data table role can only reach a database it may CONNECT to, and PUBLIC's default CONNECT
|
||||
// would otherwise let every role in regardless of what this instance defines. Best-effort: a
|
||||
// failure here leaves the database usable as `admin`, and the next role change repairs it.
|
||||
if let Err(e) = crate::datatable_roles::converge_connect_grants(db, dbname).await {
|
||||
tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}");
|
||||
}
|
||||
|
||||
tracing::info!("Created custom instance database '{}'", dbname);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -329,6 +329,7 @@ pub fn try_expand_internal_db_query(
|
||||
"ALTER_TABLE" => expand_alter_table(json_str, db_type).map(ExpandedQuery::sql),
|
||||
"CREATE_SCHEMA" => expand_create_schema(json_str, db_type).map(ExpandedQuery::sql),
|
||||
"DROP_SCHEMA" => expand_drop_schema(json_str, db_type).map(ExpandedQuery::sql),
|
||||
"RENAME_SCHEMA" => expand_rename_schema(json_str, db_type).map(ExpandedQuery::sql),
|
||||
// Metadata queries
|
||||
"LOAD_TABLE_METADATA" => expand_load_table_metadata(json_str, db_type),
|
||||
"FOREIGN_KEYS" => expand_foreign_keys(json_str, db_type).map(ExpandedQuery::sql),
|
||||
@@ -1716,6 +1717,13 @@ struct DropSchemaPayload {
|
||||
ducklake: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct RenameSchemaPayload {
|
||||
schema: String,
|
||||
new_schema: String,
|
||||
ducklake: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize)]
|
||||
struct TableEditorColumn {
|
||||
name: String,
|
||||
@@ -2004,6 +2012,23 @@ fn expand_drop_schema(json_str: &str, db_type: DbType) -> Result<String, String>
|
||||
Ok(maybe_wrap_ducklake(query, p.ducklake.as_deref()))
|
||||
}
|
||||
|
||||
fn expand_rename_schema(json_str: &str, db_type: DbType) -> Result<String, String> {
|
||||
let p: RenameSchemaPayload = serde_json::from_str(json_str)
|
||||
.map_err(|e| format!("Invalid RENAME_SCHEMA payload: {}", e))?;
|
||||
if !matches!(db_type, DbType::Postgresql | DbType::Snowflake) || p.ducklake.is_some() {
|
||||
return Err(format!(
|
||||
"Renaming a schema is not supported on {:?}",
|
||||
db_type
|
||||
));
|
||||
}
|
||||
let query = format!(
|
||||
"ALTER SCHEMA {} RENAME TO {};",
|
||||
qi(&p.schema, db_type),
|
||||
qi(&p.new_schema, db_type)
|
||||
);
|
||||
Ok(query)
|
||||
}
|
||||
|
||||
fn expand_create_table(json_str: &str, db_type: DbType) -> Result<String, String> {
|
||||
let p: CreateTablePayload = serde_json::from_str(json_str)
|
||||
.map_err(|e| format!("Invalid CREATE_TABLE payload: {}", e))?;
|
||||
@@ -2598,7 +2623,9 @@ WHERE table_catalog = current_database()",
|
||||
)
|
||||
} else {
|
||||
(
|
||||
"\nWHERE c.relkind = 'r' AND a.attnum > 0 AND NOT a.attisdropped\n AND ns.nspname != 'pg_catalog' AND ns.nspname != 'information_schema'".to_string(),
|
||||
// pg_catalog is readable by everyone: without the privilege check this lists
|
||||
// tables of schemas the connection's role cannot even enter.
|
||||
"\nWHERE c.relkind = 'r' AND a.attnum > 0 AND NOT a.attisdropped\n AND ns.nspname != 'pg_catalog' AND ns.nspname != 'information_schema'\n AND has_schema_privilege(ns.oid, 'USAGE')".to_string(),
|
||||
",\n ns.nspname AS schema_name,\n c.relname AS table_name".to_string(),
|
||||
"\nJOIN pg_catalog.pg_class c ON a.attrelid = c.oid\nJOIN pg_catalog.pg_namespace ns ON c.relnamespace = ns.oid".to_string(),
|
||||
"ns.nspname, c.relname, a.attnum".to_string(),
|
||||
@@ -4101,6 +4128,13 @@ mod tests {
|
||||
assert_eq!(sql, "DROP SCHEMA \"old_schema\" CASCADE;");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_expand_rename_schema() {
|
||||
let marker = r#"-- WM_INTERNAL_DB_RENAME_SCHEMA {"schema":"old","new_schema":"new"}"#;
|
||||
let sql = expand_code(marker, &ScriptLang::Postgresql);
|
||||
assert_eq!(sql, "ALTER SCHEMA \"old\" RENAME TO \"new\";");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_expand_create_schema_with_ducklake() {
|
||||
let marker = r#"-- WM_INTERNAL_DB_CREATE_SCHEMA {"schema":"s","ducklake":"lake"}"#;
|
||||
@@ -4468,6 +4502,7 @@ mod tests {
|
||||
assert!(sql.contains("schema_name"));
|
||||
assert!(sql.contains("table_name"));
|
||||
assert!(sql.contains("c.relkind = 'r'"));
|
||||
assert!(sql.contains("has_schema_privilege(ns.oid, 'USAGE')"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1304,6 +1304,12 @@ pub struct DataTable {
|
||||
/// nothing local for a fork admin to widen.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub reference: Option<DataTableReference>,
|
||||
/// Set on a *clone* — a terminal entry whose database was copied from the entry this names. The
|
||||
/// copy holds that entry's rows, so who may connect as which role stays that entry's decision:
|
||||
/// the clone carries no `permissions` of its own and is governed like a pointer, while
|
||||
/// connecting to its own database.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub governed_by: Option<DataTableReference>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub forked_from: Option<DataTableForkedFrom>,
|
||||
/// Whether the SQL-migrations feature is opted in for this data table.
|
||||
@@ -1362,9 +1368,16 @@ pub const DATATABLE_TENANT_WILDCARD: &str = "*";
|
||||
/// enough to survive a fork of a fork.
|
||||
const DATATABLE_REFERENCE_MAX_DEPTH: usize = 20;
|
||||
|
||||
/// Exactly one of `database` and `reference` must be set. Called wherever an entry is persisted,
|
||||
/// so nothing downstream has to handle an entry that is both or neither.
|
||||
/// Exactly one of `database` and `reference` must be set, and a clone's `governed_by` leaves no
|
||||
/// `permissions` beside it. Called wherever an entry is persisted, so nothing downstream has to
|
||||
/// handle an entry that is both or neither, or a clone with a decision of its own.
|
||||
pub fn validate_datatable_shape(name: &str, dt: &DataTable) -> Result<()> {
|
||||
if dt.governed_by.is_some() && (dt.database.is_none() || dt.permissions.is_some()) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table '{name}' is a clone, which owns a database and takes its roles from the \
|
||||
data table it was cloned from"
|
||||
)));
|
||||
}
|
||||
match (&dt.database, &dt.reference) {
|
||||
(Some(_), None) | (None, Some(_)) => Ok(()),
|
||||
(Some(_), Some(_)) => Err(Error::BadRequest(format!(
|
||||
@@ -1452,23 +1465,28 @@ pub async fn read_datatable_entry(db: &DB, w_id: &str, name: &str) -> Result<Dat
|
||||
Ok(serde_json::from_value::<DataTable>(datatable.clone())?)
|
||||
}
|
||||
|
||||
/// The terminal entry a reference chain lands on: the workspace that governs the data table, the
|
||||
/// entry name there, and the entry itself. A terminal entry resolves to itself.
|
||||
/// The terminal entry a reference chain lands on, and what governs it: the workspace and name of
|
||||
/// the entry that owns the database, the entry itself, and — for a clone — the entry its
|
||||
/// `governed_by` chain lands on. A terminal entry that is not a clone resolves to itself and
|
||||
/// governs itself.
|
||||
///
|
||||
/// Every decision downstream — which database to connect to, whose `permissions` apply, whose
|
||||
/// members tenants are evaluated against, who may administer it — is taken on this, never on the
|
||||
/// entry the caller named.
|
||||
/// Every decision downstream is taken on this, never on the entry the caller named. Which database
|
||||
/// to connect to comes from `workspace_id` / `name` / `datatable.database`; whose `permissions`
|
||||
/// apply (already in `datatable.permissions`), whose members tenants are evaluated against and who
|
||||
/// may administer it come from [`GoverningDatatable::governing_workspace_id`].
|
||||
///
|
||||
/// Authorization: resolving deliberately crosses into the governing workspace, so it answers for a
|
||||
/// workspace the caller may not belong to and checks nothing itself. It is the input to the
|
||||
/// checks, not one of them: callers MUST pass what it returns to
|
||||
/// [`can_use_datatable_role_in_governing_workspace`] or [`ensure_datatable_admin_access`] before
|
||||
/// acting on it, and MUST NOT return its `permissions` or `workspace_id` to a caller from
|
||||
/// acting on it, and MUST NOT return its `permissions` or workspace ids to a caller from
|
||||
/// elsewhere without gating on the answer.
|
||||
pub struct GoverningDatatable {
|
||||
pub workspace_id: String,
|
||||
pub name: String,
|
||||
pub datatable: DataTable,
|
||||
/// For a clone, the entry whose `permissions` govern it. `None` when the entry governs itself.
|
||||
pub governor: Option<DataTableReference>,
|
||||
}
|
||||
|
||||
impl GoverningDatatable {
|
||||
@@ -1480,6 +1498,13 @@ impl GoverningDatatable {
|
||||
.as_ref()
|
||||
.is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance)
|
||||
}
|
||||
|
||||
/// The workspace whose admins administer the data table and whose members its tenants are.
|
||||
pub fn governing_workspace_id(&self) -> &str {
|
||||
self.governor
|
||||
.as_ref()
|
||||
.map_or(&self.workspace_id, |g| &g.workspace_id)
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn resolve_governing_datatable(
|
||||
@@ -1490,6 +1515,9 @@ pub async fn resolve_governing_datatable(
|
||||
let mut workspace_id = w_id.to_string();
|
||||
let mut name = name.to_string();
|
||||
let mut hops = 0;
|
||||
// The clone a `governed_by` chain started from: it keeps its database, and takes the
|
||||
// `permissions` of wherever the chain lands.
|
||||
let mut clone: Option<(String, String, DataTable)> = None;
|
||||
for _ in 0..DATATABLE_REFERENCE_MAX_DEPTH {
|
||||
let datatable = read_datatable_entry(db, &workspace_id, &name)
|
||||
.await
|
||||
@@ -1500,21 +1528,48 @@ pub async fn resolve_governing_datatable(
|
||||
// A pointer outlives the workspace it names: deleting one only nulls the fork
|
||||
// lineage, it does not sweep the entries that pointed at it. Say which one is
|
||||
// gone rather than reporting a data table this workspace never had.
|
||||
Error::NotFound(format!(
|
||||
"Data table '{name}' of workspace '{workspace_id}' governs this one and no \
|
||||
longer exists. A superadmin can point this data table somewhere else."
|
||||
))
|
||||
if clone.is_some() {
|
||||
Error::NotFound(format!(
|
||||
"Data table '{name}' of workspace '{workspace_id}', which this clone \
|
||||
takes its roles from, no longer exists, so nobody is let into the copy."
|
||||
))
|
||||
} else {
|
||||
Error::NotFound(format!(
|
||||
"Data table '{name}' of workspace '{workspace_id}' governs this one and \
|
||||
no longer exists. A superadmin can point this data table somewhere \
|
||||
else."
|
||||
))
|
||||
}
|
||||
}
|
||||
})?;
|
||||
hops += 1;
|
||||
validate_datatable_shape(&name, &datatable)?;
|
||||
match &datatable.reference {
|
||||
None => return Ok(GoverningDatatable { workspace_id, name, datatable }),
|
||||
Some(reference) => {
|
||||
workspace_id = reference.workspace_id.clone();
|
||||
name = reference.datatable.clone();
|
||||
let next = match (&datatable.reference, &datatable.governed_by) {
|
||||
(Some(reference), _) => reference.clone(),
|
||||
(None, Some(governed_by)) => {
|
||||
let governed_by = governed_by.clone();
|
||||
if clone.is_none() {
|
||||
clone = Some((workspace_id.clone(), name.clone(), datatable));
|
||||
}
|
||||
governed_by
|
||||
}
|
||||
}
|
||||
(None, None) => {
|
||||
return Ok(match clone {
|
||||
None => GoverningDatatable { workspace_id, name, datatable, governor: None },
|
||||
Some((clone_w_id, clone_name, mut clone_datatable)) => {
|
||||
clone_datatable.permissions = datatable.permissions;
|
||||
GoverningDatatable {
|
||||
workspace_id: clone_w_id,
|
||||
name: clone_name,
|
||||
datatable: clone_datatable,
|
||||
governor: Some(DataTableReference { workspace_id, datatable: name }),
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
};
|
||||
workspace_id = next.workspace_id;
|
||||
name = next.datatable;
|
||||
}
|
||||
Err(Error::BadRequest(format!(
|
||||
"Data table '{name}' points at another data table through more than \
|
||||
@@ -1554,17 +1609,20 @@ pub async fn resolve_workspace_governing_datatables(
|
||||
|
||||
let mut entries = Entries::new();
|
||||
let listed = load(db, &[w_id.to_string()], &mut entries).await?;
|
||||
// (index into `listed`, workspace, entry name) still to be followed.
|
||||
let mut cursors: Vec<(usize, String, String)> = listed
|
||||
// (index into `listed`, workspace, entry name, the clone the chain started from) still to be
|
||||
// followed. A clone keeps its own database and takes the `permissions` of wherever its
|
||||
// `governed_by` chain lands, as the single resolution does.
|
||||
type Clone = Option<(String, String, DataTable)>;
|
||||
let mut cursors: Vec<(usize, String, String, Clone)> = listed
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, name)| (i, w_id.to_string(), name.clone()))
|
||||
.map(|(i, name)| (i, w_id.to_string(), name.clone(), None))
|
||||
.collect();
|
||||
let mut resolved: Vec<(usize, GoverningDatatable)> = vec![];
|
||||
|
||||
for _ in 0..DATATABLE_REFERENCE_MAX_DEPTH {
|
||||
let mut next = vec![];
|
||||
for (i, ws, name) in cursors.drain(..) {
|
||||
for (i, ws, name, clone) in cursors.drain(..) {
|
||||
let Some(value) = entries
|
||||
.get(&ws)
|
||||
.and_then(|m| m.get(&name))
|
||||
@@ -1578,14 +1636,41 @@ pub async fn resolve_workspace_governing_datatables(
|
||||
if validate_datatable_shape(&name, &datatable).is_err() {
|
||||
continue;
|
||||
}
|
||||
match &datatable.reference {
|
||||
None => {
|
||||
resolved.push((i, GoverningDatatable { workspace_id: ws, name, datatable }))
|
||||
}
|
||||
Some(reference) => next.push((
|
||||
match (&datatable.reference, &datatable.governed_by) {
|
||||
(Some(reference), _) => next.push((
|
||||
i,
|
||||
reference.workspace_id.clone(),
|
||||
reference.datatable.clone(),
|
||||
clone,
|
||||
)),
|
||||
(None, Some(governed_by)) => {
|
||||
let (governor_ws, governor_name) =
|
||||
(governed_by.workspace_id.clone(), governed_by.datatable.clone());
|
||||
let clone = clone.or(Some((ws, name, datatable)));
|
||||
next.push((i, governor_ws, governor_name, clone));
|
||||
}
|
||||
(None, None) => resolved.push((
|
||||
i,
|
||||
match clone {
|
||||
None => GoverningDatatable {
|
||||
workspace_id: ws,
|
||||
name,
|
||||
datatable,
|
||||
governor: None,
|
||||
},
|
||||
Some((clone_ws, clone_name, mut clone_datatable)) => {
|
||||
clone_datatable.permissions = datatable.permissions;
|
||||
GoverningDatatable {
|
||||
workspace_id: clone_ws,
|
||||
name: clone_name,
|
||||
datatable: clone_datatable,
|
||||
governor: Some(DataTableReference {
|
||||
workspace_id: ws,
|
||||
datatable: name,
|
||||
}),
|
||||
}
|
||||
}
|
||||
},
|
||||
)),
|
||||
}
|
||||
}
|
||||
@@ -1594,7 +1679,7 @@ pub async fn resolve_workspace_governing_datatables(
|
||||
}
|
||||
let to_load: Vec<String> = next
|
||||
.iter()
|
||||
.map(|(_, ws, _)| ws.clone())
|
||||
.map(|(_, ws, _, _)| ws.clone())
|
||||
.filter(|ws| !entries.contains_key(ws))
|
||||
.collect::<std::collections::BTreeSet<_>>()
|
||||
.into_iter()
|
||||
@@ -1967,6 +2052,8 @@ pub fn strip_datatable_permissions(
|
||||
/// As [`parse_datatable_ref`], except that an entry whose stored name itself contains `?` — which
|
||||
/// names could before they were restricted — resolves by that exact name, without a role. It is
|
||||
/// looked up first, so `sales?role=x` never reaches a different entry than the one stored so.
|
||||
/// When `sales` is stored too, the reference means either one, and is refused rather than
|
||||
/// resolved to whichever is looked up first.
|
||||
///
|
||||
/// Authorization: checks nothing, and its answer reveals whether `w_id` stores that exact name.
|
||||
/// Callers MUST already act for `w_id` — a job of it, or a caller authenticated into it — and
|
||||
@@ -1977,16 +2064,26 @@ pub async fn parse_datatable_ref_for(
|
||||
reference: &str,
|
||||
) -> Result<(String, Option<String>)> {
|
||||
if reference.contains('?') {
|
||||
let exists = sqlx::query_scalar::<_, Option<bool>>(
|
||||
"SELECT (datatable->'datatables') ? $2 FROM workspace_settings WHERE workspace_id = $1",
|
||||
let role_target = parse_datatable_ref(reference)
|
||||
.ok()
|
||||
.and_then(|(name, role)| role.map(|_| name));
|
||||
let (exists, target_exists) = sqlx::query_as::<_, (Option<bool>, Option<bool>)>(
|
||||
"SELECT (datatable->'datatables') ? $2, (datatable->'datatables') ? $3
|
||||
FROM workspace_settings WHERE workspace_id = $1",
|
||||
)
|
||||
.bind(w_id)
|
||||
.bind(reference)
|
||||
.bind(role_target)
|
||||
.fetch_optional(db)
|
||||
.await?
|
||||
.flatten()
|
||||
.unwrap_or(false);
|
||||
if exists {
|
||||
.unwrap_or((None, None));
|
||||
if exists.unwrap_or(false) {
|
||||
if let (Some(name), Some(true)) = (role_target, target_exists) {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"Data table reference '{reference}' names both the data table '{reference}' \
|
||||
and a role on the data table '{name}'. Rename '{reference}' to use either."
|
||||
)));
|
||||
}
|
||||
return Ok((reference.to_string(), None));
|
||||
}
|
||||
}
|
||||
@@ -3344,6 +3441,7 @@ mod tests {
|
||||
resource_path: "dt_main".to_string(),
|
||||
}),
|
||||
reference: None,
|
||||
governed_by: None,
|
||||
forked_from: None,
|
||||
migrations_enabled: None,
|
||||
permissions: None,
|
||||
|
||||
Reference in New Issue
Block a user