Compare commits

..
Author SHA1 Message Date
Diego ImbertandClaude Opus 5 dbe0230f48 fix(datatables): never grant CREATEROLE to custom_instance_user on the external cluster
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 11:49:20 +02:00
Diego Imbert b3a3f1cf5a Merge remote-tracking branch 'origin/datatable-roles-redesign-part-4' into HEAD
# Conflicts:
#	backend/ee-repo-ref.txt
2026-09-17 11:34:28 +02:00
Diego Imbert 92e51f19aa Merge remote-tracking branch 'origin/datatable-roles-redesign-part-2' into datatable-roles-redesign-part-4
# Conflicts:
#	backend/ee-repo-ref.txt
#	backend/windmill-api-integration-tests/tests/datatable_roles.rs
#	backend/windmill-api-workspaces/src/datatable_permissions_oss.rs
#	backend/windmill-api-workspaces/src/workspaces.rs
#	backend/windmill-common/src/datatable_roles_oss.rs
#	backend/windmill-common/src/workspaces.rs
#	frontend/src/lib/components/datatableAcl/PgAclEditor.svelte
#	frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte
#	frontend/src/lib/components/workspaceSettings/DataTableRolesSection.svelte
#	frontend/src/lib/components/workspaceSettings/DataTableSettings.svelte
#	frontend/src/lib/components/workspaceSettings/InstanceRolesButton.svelte
2026-09-17 10:03:05 +02:00
Diego ImbertandClaude Opus 5 36532c35d8 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
2026-09-17 10:01:19 +02:00
Diego ImbertandClaude Opus 5 79481a2f02 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
2026-09-17 10:01:19 +02:00
Diego ImbertandClaude Opus 5 c884597e56 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
2026-09-17 10:01:19 +02:00
Diego ImbertandClaude Opus 5 115448d3d1 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
2026-09-17 10:01:19 +02:00
Diego ImbertandClaude Opus 5 1ece278fd2 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
2026-09-17 10:01:19 +02:00
Diego ImbertandClaude Opus 5 7819d48df9 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
2026-09-17 10:01:19 +02:00
Diego ImbertandClaude Opus 5 ce1f720d85 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
2026-09-17 10:01:19 +02:00
Diego ImbertandClaude Opus 5 93d346e54b feat(datatables): add an ACL editor for data table roles
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 10:01:19 +02:00
Diego ImbertandClaude Opus 5 9378828472 docs(datatables): note that legacy names with ? cannot be migrated
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 79bb761c5d fix(datatables): serialize roles going on with aliases saved from other workspaces
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 d191cfe4eb fix(datatables): let CE migrations connect as an explicitly named admin
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 31b5b0a062 fix(datatables): refuse roles while a same-workspace alias reaches the database
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 10:01:15 +02:00
windmill-internal-app[bot] d1c2f801f4 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.
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 d3ee67c9bc 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 eafc551651 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 db05c8b7c9 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 11ba3cd477 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 7ae3335f03 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 c7269e975f 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 8a0babd7a6 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 6d9ef5f6b7 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 1dbc1e7759 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 df471e37d8 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>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 122a3bd164 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>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 2f95c22242 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>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 7279ee15e2 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>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 fa1e8bd3aa 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>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 dc3ebfbe74 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>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 8255f2ad2b 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>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 ebe076cbe6 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>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 643bc1b51a Revert "fix(datatables): let a retried clone reclaim its own leftover database"
This reverts commit 7dd3275a10.

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>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 29fccb0978 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>
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 432ccc0fbd 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 e6794984d2 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 2abcd122a8 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 30f039d292 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 5fd9ab24b8 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 f7812187cc 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 b5785e8e7a 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 ac8587e452 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
2026-09-17 10:01:15 +02:00
Diego Imbert 3ae1688c44 fix pg_dump stuck on version 17 on nix 2026-09-17 10:01:15 +02:00
Diego Imbert d69da0bbd6 nit worker error msg 2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 eeed332591 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 78a12fdfcc 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 189bc3297b 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 0efddb94cf 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 1db77dda87 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 d0ea1ce598 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 c400469e67 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 a74f73ccb1 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 a7bff8de97 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 0d5106f618 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 b8d6b3c953 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 c2c24598a2 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 21d97f3a4c 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
2026-09-17 10:01:15 +02:00
Diego ImbertandClaude Opus 5 34ed0c4230 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
2026-09-17 10:01:15 +02:00
Diego Imbert 13162656bf Merge remote-tracking branch 'origin/datatable-roles-redesign-part-2' into datatable-roles-redesign-part-4
# Conflicts:
#	backend/ee-repo-ref.txt
#	backend/windmill-api-workspaces/src/datatable_permissions_oss.rs
2026-09-17 09:50:52 +02:00
Diego ImbertandClaude Opus 5 e79bc14903 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
2026-09-17 09:00:04 +02:00
Diego ImbertandClaude Opus 5 b7623f12eb 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
2026-09-17 00:09:29 +02:00
Diego ImbertandClaude Opus 5 631c01d20d fix(datatables): compare the external cluster settings under a row lock before storing setup
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-17 00:07:50 +02:00
Diego ImbertandClaude Opus 5 0a2ebfb3db 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
2026-09-16 23:55:43 +02:00
Diego Imbert 0489cb1630 Merge remote-tracking branch 'origin/datatable-roles-redesign-part-2' into datatable-roles-redesign-part-4
# Conflicts:
#	backend/ee-repo-ref.txt
#	backend/windmill-api-integration-tests/tests/datatable_roles.rs
2026-09-16 23:45:48 +02:00
Diego ImbertandClaude Opus 5 d7417f5cc2 Merge branch 'datatable-roles-redesign' into datatable-roles-redesign-part-2
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BRoYE5ZeAVvrDYdfhDAYXb
2026-09-16 23:40:04 +02:00
Diego ImbertandClaude Opus 5 5ef5f58eeb fix(datatables): scope external cluster credential readers to the crate
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 23:38:06 +02:00
Diego ImbertandClaude Opus 5 4533ec460e fix(datatables): send external cluster passwords as SCRAM verifiers
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 22:44:11 +02:00
Diego ImbertandClaude Opus 5 b3cda2f548 feat(datatables): set up an external instance cluster for data tables
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 22:35:00 +02:00
Diego ImbertandClaude Opus 5 8d0d51e99d 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
2026-09-16 22:25:51 +02:00
Diego ImbertandClaude Opus 5 110876d0c9 docs(datatables): note that legacy names with ? cannot be migrated
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 22:25:27 +02:00
Diego ImbertandClaude Opus 5 e5ec5217d4 fix(datatables): serialize roles going on with aliases saved from other workspaces
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 18:09:07 +02:00
Diego ImbertandClaude Opus 5 6a89ba0179 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
2026-09-16 17:31:06 +02:00
Diego ImbertandClaude Opus 5 707b0dc8ee fix(datatables): let CE migrations connect as an explicitly named admin
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 17:25:42 +02:00
Diego ImbertandClaude Opus 5 dcc37bf6f2 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>
2026-09-16 17:20:19 +02:00
Diego ImbertandClaude Opus 5 10861bb529 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>
2026-09-16 17:03:32 +02:00
Diego ImbertandClaude Opus 5 d459b371d8 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
2026-09-16 16:53:13 +02:00
Diego ImbertandClaude Opus 5 41e1101a2d 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
2026-09-16 16:39:14 +02:00
Diego ImbertandClaude Opus 5 4416283b17 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>
2026-09-16 16:32:36 +02:00
Diego ImbertandClaude Opus 5 e0360bce5c 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>
2026-09-16 16:07:00 +02:00
Diego ImbertandClaude Opus 5 f36aa69fc3 feat(datatables): data table roles in the DB manager and raw apps
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 15:23:20 +02:00
Diego ImbertandClaude Opus 5 a1b91690fd feat(datatables): add an ACL editor for data table roles
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 15:22:05 +02:00
Diego ImbertandClaude Opus 5 0676f55856 fix(datatables): refuse roles while a same-workspace alias reaches the database
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-16 15:14:29 +02:00
windmill-internal-app[bot] aa6a52271b 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.
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 64d5f762dc 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 4c2f1a7b60 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 16fdba4d82 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 1986ec2535 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 a7bac22c13 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 1d6da0084e 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 3d4a2133ed 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 f8a6347052 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 46dfcc5c49 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 e9c27d63db 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>
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 65a6cc3eb5 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>
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 dd2ca5513e 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>
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 6a3a2ed13c 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>
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 791bb8d843 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>
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 b68b97f5ea 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>
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 92c723f735 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>
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 c7f28ba8ed 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>
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 bea9667516 Revert "fix(datatables): let a retried clone reclaim its own leftover database"
This reverts commit 7dd3275a10.

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>
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 bb8c8ed735 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>
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 4610f8a35c 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 284d1ecb43 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
2026-09-16 15:14:29 +02:00
Diego ImbertandClaude Opus 5 306a35e3b0 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 5a98030fb7 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 d558b6508a 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 fce20faeae 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 da3babccc7 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 69742c0b48 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
2026-09-16 15:14:28 +02:00
Diego Imbert 3dfbf64d9d fix pg_dump stuck on version 17 on nix 2026-09-16 15:14:28 +02:00
Diego Imbert d32ff92eac nit worker error msg 2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 01eba26582 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 cafcf3afc3 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 030254af47 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 9872bff69f 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 e400daabb8 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 7f3c7a19af 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 1d9ee09b31 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 715d8a0e6d 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 ea14c01a3c 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 54f4467027 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 8276ae09fa 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 6f9457fc8e 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 6ea682d741 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
2026-09-16 15:14:28 +02:00
Diego ImbertandClaude Opus 5 22b3e69c07 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
2026-09-16 15:14:28 +02:00
474 changed files with 15251 additions and 8638 deletions
-4
View File
@@ -175,10 +175,6 @@ the decrypted value, exactly as against a real backend. The chat's read path pas
Seed a recognizable secret (the existing fixture uses `sk_live_do_not_leak_me`) and
assert it via `valueExcludes` to catch a leak.
`toolExpect.toolCallArgs` entries support `sharedByAtLeast: <n>`: at least `n` recorded
calls to that tool must carry the same non-blank string in the field. Use it for calls that
have to share an identifier, like two test runs of one chat conversation.
`toolExpect.toolCallArgs` entries additionally support `fieldMustBeAbsent: true`: no
recorded call to that tool may pass the field at all (an explicit `null` counts as
passing it). Use it for partial-update tools, where supplying a field the model could
+12 -1
View File
@@ -1,5 +1,5 @@
import { randomUUID } from 'node:crypto'
import type { BackendValidationSettings } from '../../core/backendValidation'
import { buildWorkspaceId } from './workspaceId'
interface CompletedJobResultMaybe {
completed: boolean
@@ -24,6 +24,7 @@ export interface CompletedPreviewJob {
const tokenCache = new Map<string, Promise<string>>()
const sharedWorkspaceQueue = new Map<string, Promise<void>>()
const managedSharedWorkspacePrefixes = ['f/evals/']
const DEFAULT_WORKSPACE_PREFIX = 'ai-evals'
export class BackendPreviewClient {
constructor(private readonly settings: BackendValidationSettings) {}
@@ -440,6 +441,16 @@ async function withSharedWorkspaceLock<T>(workspaceId: string, body: () => Promi
}
}
function buildWorkspaceId(caseId: string, attempt: number): string {
const caseSlug = caseId
.toLowerCase()
.replace(/[^a-z0-9-]+/g, '-')
.replace(/^-+|-+$/g, '')
.slice(0, 30)
const suffix = randomUUID().slice(0, 8)
return `${DEFAULT_WORKSPACE_PREFIX}-${caseSlug || 'case'}-a${attempt}-${suffix}`
}
function extractFolderName(path: string): string | null {
if (!path.startsWith('f/')) {
return null
-48
View File
@@ -11,7 +11,6 @@ import type {
Script
} from '../../../frontend/src/lib/gen'
import type {
DataMetric,
DataTableTables,
DataTableTableSchema,
EndpointTool,
@@ -113,9 +112,6 @@ export interface BenchmarkWorkspaceRunnables {
aiProviders?: BenchmarkWorkspaceAiProvider[]
resources?: BenchmarkWorkspaceResource[]
datatables?: BenchmarkDatatableSeed[]
/** DuckLake catalog names, as `list_ducklakes` reports them. */
ducklakes?: string[]
dataMetrics?: DataMetric[]
jobs?: BenchmarkWorkspaceJob[]
}
@@ -677,27 +673,6 @@ export function listBenchmarkDatatables(workspace: string): DataTableTables[] |
}))
}
// ============= DuckLake catalogs and declared metrics =============
/** Seeded DuckLake names, or `null` for a non-benchmark workspace. */
export function listBenchmarkDucklakes(workspace: string): string[] | null {
const runnables = benchmarkWorkspaceRunnables.get(workspace)
return runnables ? (runnables.ducklakes ?? []) : null
}
/**
* Seeded metric declarations, or `null` for a non-benchmark workspace.
*
* The `table` / `path_prefix` filters are ignored: which rows a filter selects is
* `canonical_table_path`'s business and is pinned by `ducklakeTools.test.ts`.
* Re-deriving it here would give the eval its own copy of that spec to drift from,
* and the case this serves measures whether the model reaches for the tool at all.
*/
export function listBenchmarkDataMetrics(workspace: string): DataMetric[] | null {
const runnables = benchmarkWorkspaceRunnables.get(workspace)
return runnables ? (runnables.dataMetrics ?? []) : null
}
export function getBenchmarkDatatableSchema(input: {
workspace: string
datatableName: string
@@ -865,29 +840,6 @@ export function runBenchmarkFlowByPath(input: {
})
}
/**
* Mirror `JobService.runFlowPreview` for benchmark workspaces, including the server's
* refusal of a chat-enabled flow run that names no conversation (`memory_id`).
*/
export function runBenchmarkFlowPreview(input: {
workspace: string
memoryId?: string
requestBody?: { path?: string; value?: { chat_input_enabled?: boolean }; args?: unknown }
}): string {
if (input.requestBody?.value?.chat_input_enabled && !input.memoryId) {
throw new Error('Bad request: memory_id is required for chat-enabled flows')
}
const args = (input.requestBody?.args ?? {}) as Record<string, unknown>
return createBenchmarkCompletedJob({
workspace: input.workspace,
jobKind: 'flowpreview',
success: true,
args,
result: { path: input.requestBody?.path, args, mocked: true },
logs: 'Mock benchmark flow preview completed successfully.'
})
}
export function previewBenchmarkSchedule(input: {
requestBody?: Record<string, unknown>
}): Record<string, unknown> {
@@ -76,9 +76,7 @@ vi.mock('$lib/gen', async () => {
listBenchmarkPlainResources,
listBenchmarkApps,
listBenchmarkDatatables,
listBenchmarkDataMetrics,
listBenchmarkDrafts,
listBenchmarkDucklakes,
listBenchmarkFlows,
listBenchmarkJobs,
listBenchmarkScripts,
@@ -89,7 +87,6 @@ vi.mock('$lib/gen', async () => {
previewBenchmarkSchedule,
runBenchmarkDatatableSql,
runBenchmarkFlowByPath,
runBenchmarkFlowPreview,
runBenchmarkScriptByPath,
runBenchmarkScriptPreview,
updateBenchmarkDraft,
@@ -296,14 +293,6 @@ vi.mock('$lib/gen', async () => {
args: data.requestBody
})
: actual.JobService.runScriptByPath(data),
runFlowPreview: async (data: {
workspace: string
memoryId?: string
requestBody?: { path?: string; value?: { chat_input_enabled?: boolean }; args?: unknown }
}) =>
hasBenchmarkWorkspace(data.workspace)
? runBenchmarkFlowPreview(data)
: actual.JobService.runFlowPreview(data as any),
runFlowByPath: async (data: {
workspace: string
path: string
@@ -352,10 +341,6 @@ vi.mock('$lib/gen', async () => {
hasBenchmarkWorkspace(data.workspace)
? (listBenchmarkDatatables(data.workspace) ?? [])
: actual.WorkspaceService.listDataTableTables(data),
listDucklakes: async (data: { workspace: string }) =>
hasBenchmarkWorkspace(data.workspace)
? (listBenchmarkDucklakes(data.workspace) ?? [])
: actual.WorkspaceService.listDucklakes(data),
getDataTableTableSchema: async (data: {
workspace: string
datatableName: string
@@ -371,12 +356,6 @@ vi.mock('$lib/gen', async () => {
})
: actual.WorkspaceService.getDataTableTableSchema(data)
}),
DataMetricService: wrapService(actual.DataMetricService, {
listDataMetrics: async (data: { workspace: string }) =>
hasBenchmarkWorkspace(data.workspace)
? { metrics: listBenchmarkDataMetrics(data.workspace) ?? [] }
: actual.DataMetricService.listDataMetrics(data)
}),
ScheduleService: wrapService(actual.ScheduleService, {
existsSchedule: async (data: { workspace: string; path: string }) =>
hasBenchmarkWorkspace(data.workspace) ? false : actual.ScheduleService.existsSchedule(data),
+12 -1
View File
@@ -1,8 +1,9 @@
import { randomUUID } from "node:crypto";
import type { WindmillBackendSettings } from "../../core/windmillBackendSettings";
import { buildWorkspaceId } from "./workspaceId";
const tokenCache = new Map<string, Promise<string>>();
const sharedWorkspaceQueue = new Map<string, Promise<void>>();
const DEFAULT_WORKSPACE_PREFIX = "ai-evals";
export class WindmillBackendClient {
constructor(private readonly settings: WindmillBackendSettings) {}
@@ -178,6 +179,16 @@ async function withSharedWorkspaceLock<T>(
}
}
function buildWorkspaceId(caseId: string, attempt: number): string {
const caseSlug = caseId
.toLowerCase()
.replace(/[^a-z0-9-]+/g, "-")
.replace(/^-+|-+$/g, "")
.slice(0, 30);
const suffix = randomUUID().slice(0, 8);
return `${DEFAULT_WORKSPACE_PREFIX}-${caseSlug || "case"}-a${attempt}-${suffix}`;
}
async function expectOk(response: Response, context: string): Promise<void> {
if (response.ok) {
return;
@@ -1,21 +0,0 @@
import { describe, expect, it } from "bun:test";
import { buildWorkspaceId } from "./workspaceId";
describe("buildWorkspaceId", () => {
// `workspace.proper_id` rejects `--`, which a case id can carry itself and
// which truncating a slug on a hyphen produces once the suffix adds its own.
// One id per shape: cut landing on a hyphen, cut landing mid-word, no cut, and
// a doubled hyphen no cut ever reaches.
it("stays within the id length cap and the proper_id format", () => {
for (const caseId of [
"global-test6-secret-variable-draft",
"global-test23-datatable-query-select",
"short",
"global--test-foo",
]) {
const id = buildWorkspaceId(caseId, 1);
expect(id.length).toBeLessThanOrEqual(50);
expect(id).toMatch(/^\w+(-\w+)*$/);
}
});
});
-22
View File
@@ -1,22 +0,0 @@
import { randomUUID } from "node:crypto";
const DEFAULT_WORKSPACE_PREFIX = "ai-evals";
// A workspace id must be at most 50 characters AND match `^\w+(-\w+)*$`
// (`workspace.proper_id`), so the case slug yields to the random suffix that
// makes the id unique, and no hyphen may end up doubled — neither one already in
// the case id nor one a truncation leaves for the suffix to follow.
const MAX_WORKSPACE_ID_LENGTH = 50;
export function buildWorkspaceId(caseId: string, attempt: number): string {
const caseSlug = caseId
.toLowerCase()
.replace(/[^a-z0-9-]+/g, "-")
.replace(/-{2,}/g, "-")
.replace(/^-+|-+$/g, "");
const suffix = `-a${attempt}-${randomUUID().slice(0, 8)}`;
const head = `${DEFAULT_WORKSPACE_PREFIX}-${caseSlug || "case"}`;
return `${head
.slice(0, MAX_WORKSPACE_ID_LENGTH - suffix.length)
.replace(/-+$/, "")}${suffix}`;
}
+12 -58
View File
@@ -1919,11 +1919,10 @@
- when the lookup fails, tells the user instead of inventing table names
- does not write scripts or resources to answer a read-only question
# --- Dedicated tools preferred over the API catalog ---
# The harness serves worker/queue reads itself (benchmark fetch handlers in
# adapters/frontend), so these cases do not require an mcp-enabled eval backend.
# The stale `api-catalog` in the id below is kept so results stay comparable
# across benchmark runs.
# --- API catalog (search_api_endpoints / call_api_get / call_api_endpoint) ---
# The harness serves the catalog and the executed calls itself (mock
# listMcpTools + benchmark fetch handlers in adapters/frontend), so these cases
# do not require an mcp-enabled eval backend.
- id: global-test30-api-catalog-workers
prompt: |-
@@ -1935,42 +1934,23 @@
draftCountExactly: 0
toolExpect:
requiredToolsUsed:
- list_workers
forbiddenToolsUsed:
- search_api_endpoints
- call_api_get
forbiddenToolsUsed:
- call_api_endpoint
- write_script
- deploy_workspace_item
toolCallArgs:
- tool: call_api_get
field: name
stringIncludesAnyOf:
- listWorkers
# Read-only workspace inspection produces no draft; validate via tool use.
skipJudge: true
judgeChecklist:
- reads worker state through list_workers instead of guessing or fabricating
- discovers the workers endpoint through the API catalog instead of guessing or fabricating
- reports worker status from the returned data
- id: global-test37-ducklake-declared-measure
prompt: |-
We track orders in the main ducklake. Write me a duckdb script that reports total
revenue by month. Keep it as a draft, don't deploy it.
initial: ai_evals/fixtures/frontend/global/initial/ducklake_orders_metrics.json
runtime:
maxTurns: 8
validate:
draftCountExactly: 1
toolExpect:
requiredToolsUsed:
- list_data_metrics
forbiddenToolsUsed:
- deploy_workspace_item
- delete_workspace_item
# The judge runs: the point is not that the tool was called but that the number it
# describes is the declared one. `revenue` excludes test rows, so an aggregate that
# reproduces it without the filter is plausible, runnable and wrong.
judgeChecklist:
- totals revenue with the declared sum over the amount column rather than an invented aggregate over a guessed column
- excludes test orders from the total, as the declared revenue measure does
- groups by month using the declared order_month expression over order_date
- does not introduce column names absent from the declarations
- id: global-test31-draft-test-run-not-deployed
prompt: |-
Update `f/evals/global/format_greeting` so the provided name is uppercased in the greeting, then run it with name "ada" to check it works.
@@ -2160,32 +2140,6 @@
- creates an AI draft of f/evals/global/process_invoice applying 8% tax
- does not deploy or save the draft
- id: global-test38-chat-flow-follow-up-same-conversation
prompt: |-
I want to check that my support chat flow `f/evals/global/support_chat` remembers what was said.
Test it: first send "My name is Ada", then send "What is my name?" as a follow-up in the same chat.
initial: ai_evals/fixtures/frontend/global/initial/support_chat_flow.json
runtime:
maxTurns: 8
validate:
draftCountExactly: 0
toolExpect:
requiredToolsUsed:
- test_run_flow
# A chat flow's memory lives in its conversation, so a follow-up only reaches the first
# turn's history when both test runs name the same conversation.
toolCallArgs:
- tool: test_run_flow
field: memory_id
sharedByAtLeast: 2
forbiddenToolsUsed:
- run_flow
- deploy_workspace_item
# The judge cannot observe runs; what this case guards is the conversation the runs share.
skipJudge: true
judgeChecklist:
- test-runs the chat flow twice, the second message as a follow-up in the first run's conversation
- id: global-undo-created-draft
prompt: |-
Create a draft Postgres resource at `u/admin/scratch_db` for host db.example.com port 5432, database `orders`, user `app`, and tell me what fields it ended up with.
-7
View File
@@ -182,13 +182,6 @@ export interface ToolCallArgumentRule {
* the point is that the model filled it in at all rather than what it said.
*/
nonEmpty?: boolean;
/**
* Existential over calls: at least this many recorded calls to `tool` carry the
* same non-blank string in `field`. Use when calls have to share an identifier —
* e.g. test runs that continue one conversation — while a retry with a rejected
* value in between is still acceptable.
*/
sharedByAtLeast?: number;
/**
* Universal over calls: no recorded call to `tool` may pass `field` at all.
* For partial-update tools, where supplying a field the model could not have
-26
View File
@@ -396,32 +396,6 @@ describe("validateToolExpectations", () => {
expect(nonEmptyCheck?.details).toContain("blank on 1 of 2");
});
it("requires sharedByAtLeast calls to carry one value, not merely a value each", () => {
const run = (ids: (string | undefined)[]) =>
validateToolExpectations({
run: {
success: true,
actual: {},
assistantMessageCount: 1,
toolCallCount: ids.length,
toolsUsed: ["test_run_flow"],
toolCallDetails: ids.map((memory_id) => ({
name: "test_run_flow",
arguments: { path: "f/chat", memory_id },
})),
skillsInvoked: [],
},
toolExpect: {
toolCallArgs: [{ tool: "test_run_flow", field: "memory_id", sharedByAtLeast: 2 }],
},
}).find((c) => c.name.includes("is shared by at least 2 calls"))?.passed;
expect(run(["a", "b"])).toBe(false);
expect(run(["a"])).toBe(false);
expect(run([undefined, undefined])).toBe(false);
expect(run(["rejected", "a", "a"])).toBe(true);
});
it("passes nonEmpty when every call filled the field", () => {
const checks = validateToolExpectations({
run: {
-17
View File
@@ -320,23 +320,6 @@ export function validateToolExpectations(input: {
);
}
if (rule.sharedByAtLeast !== undefined) {
const counts = new Map<string, number>();
for (const value of values) {
if (typeof value === "string" && value.trim().length > 0) {
counts.set(value, (counts.get(value) ?? 0) + 1);
}
}
const mostShared = Math.max(0, ...counts.values());
checks.push(
check(
`${rule.tool}.${rule.field} is shared by at least ${rule.sharedByAtLeast} calls`,
mostShared >= rule.sharedByAtLeast,
`most calls sharing one value: ${mostShared}; values: ${summarizeToolValues(values)}`
)
);
}
if (rule.fieldMustBeAbsent) {
// Anything other than `undefined` was supplied — an explicit `null` is the
// model passing the field, not omitting it.
@@ -1,36 +0,0 @@
{
"workspace": {
"ducklakes": ["main"],
"dataMetrics": [
{
"script_path": "f/analytics/orders_pipeline",
"table_path": "main/main.orders",
"kind": "measure",
"name": "revenue",
"expr": "sum(amount)",
"filter": "not is_test"
},
{
"script_path": "f/analytics/orders_pipeline",
"table_path": "main/main.orders",
"kind": "measure",
"name": "order_count",
"expr": "count(*)"
},
{
"script_path": "f/analytics/orders_pipeline",
"table_path": "main/main.orders",
"kind": "dimension",
"name": "order_month",
"expr": "date_trunc('month', order_date)"
},
{
"script_path": "f/analytics/orders_pipeline",
"table_path": "main/main.orders",
"kind": "dimension",
"name": "region",
"expr": "region"
}
]
}
}
@@ -1,59 +0,0 @@
{
"workspace": {
"flows": [
{
"path": "f/evals/global/support_chat",
"summary": "Support chat",
"description": "Answers customer questions in a chat, remembering earlier messages.",
"schema": {
"$schema": "https://json-schema.org/draft/2020-12/schema",
"type": "object",
"properties": {
"user_message": {
"type": "string",
"description": "Message from user"
}
},
"required": ["user_message"]
},
"value": {
"chat_input_enabled": true,
"modules": [
{
"id": "assistant",
"summary": "Support assistant",
"value": {
"type": "aiagent",
"tools": [],
"input_transforms": {
"provider": {
"type": "static",
"value": {
"kind": "anthropic",
"model": "claude-haiku-4-5-20251001",
"resource": "$res:f/evals/ai/anthropic"
}
},
"user_message": {
"type": "javascript",
"expr": "flow_input.user_message"
},
"system_prompt": {
"type": "static",
"value": "You are a friendly support assistant. Keep answers short."
},
"memory": {
"type": "static",
"value": { "kind": "auto", "context_length": 10 }
},
"streaming": { "type": "static", "value": true },
"output_type": { "type": "static", "value": "text" }
}
}
}
]
}
}
]
}
}
@@ -0,0 +1,22 @@
{
"db_name": "PostgreSQL",
"query": "SELECT 1 AS one FROM workspace_settings WHERE workspace_id = $1 FOR UPDATE",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "one",
"type_info": "Int4"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
null
]
},
"hash": "06abbf945bee93349ff88f64906b96ea1e853ef202510281427cfa9beeff81b3"
}
@@ -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 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\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 WHERE d.value->'reference'->>'workspace_id' = $1\n AND d.value->'reference'->>'datatable' = $2\n )",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text",
"Text"
]
},
"nullable": []
},
"hash": "06ce02cd7ce2f5a57355153edb573c242f9ba758db66e9a5e16f30e3e1494201"
}
@@ -0,0 +1,29 @@
{
"db_name": "PostgreSQL",
"query": "SELECT permissioned_as, permissioned_as_email FROM v2_job\n WHERE id = $1 AND workspace_id = $2",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "permissioned_as",
"type_info": "Varchar"
},
{
"ordinal": 1,
"name": "permissioned_as_email",
"type_info": "Varchar"
}
],
"parameters": {
"Left": [
"Uuid",
"Text"
]
},
"nullable": [
false,
false
]
},
"hash": "0d10e0fa5cf4033c7d93c9ed56be8209046007917f44da954eccf2188e5bff1f"
}
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "SELECT id, conversation_id, message_type as \"message_type: MessageType\", content, job_id, created_at, created_seq, step_name, success, tool_arguments, tool_result, reasoning, attachments\n FROM flow_conversation_message\n WHERE conversation_id = $1\n AND created_seq > $2\n ORDER BY created_seq ASC\n LIMIT $3\n ",
"query": "SELECT id, conversation_id, message_type as \"message_type: MessageType\", content, job_id, created_at, created_seq, step_name, success\n FROM (\n SELECT id, conversation_id, message_type, content, job_id, created_at, created_seq, step_name, success\n FROM flow_conversation_message\n WHERE conversation_id = $1\n ORDER BY created_seq DESC\n LIMIT $2 OFFSET $3\n ) AS messages\n ORDER BY created_seq ASC\n ",
"describe": {
"columns": [
{
@@ -58,26 +58,6 @@
"ordinal": 8,
"name": "success",
"type_info": "Bool"
},
{
"ordinal": 9,
"name": "tool_arguments",
"type_info": "Text"
},
{
"ordinal": 10,
"name": "tool_result",
"type_info": "Text"
},
{
"ordinal": 11,
"name": "reasoning",
"type_info": "Text"
},
{
"ordinal": 12,
"name": "attachments",
"type_info": "Jsonb"
}
],
"parameters": {
@@ -96,12 +76,8 @@
false,
false,
true,
false,
true,
true,
true,
true
false
]
},
"hash": "d6fa78c43b6c5f8040d7bccb29ad8627be1dac6fbe0097735a52f47c173f51c9"
"hash": "1c3473a0f9f6b6148b2c975f9f05bdefedf8a51c4e6ddf0eca367b9cc778d051"
}
@@ -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 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\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\"%'",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": []
},
"hash": "297c7a40dfce729d44aa37bc7c65560517bd25e40c0752a00467829191e2eb98"
}
@@ -0,0 +1,16 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE capture_config SET server_id = NULL, last_server_ping = NULL\n WHERE workspace_id = $1 AND trigger_kind = 'postgres'\n AND (trigger_config->>'postgres_resource_path' = $2\n OR trigger_config->>'postgres_resource_path' LIKE $3)",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text",
"Text"
]
},
"nullable": []
},
"hash": "2a391cc1bfcd2f75b46144a394c01237e09c3060da88170f1f6e06468309d213"
}
@@ -0,0 +1,22 @@
{
"db_name": "PostgreSQL",
"query": "SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = $1)",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "exists",
"type_info": "Bool"
}
],
"parameters": {
"Left": [
"Name"
]
},
"nullable": [
null
]
},
"hash": "334dbcd48fb59c96c62c2705ab2d1ce716cd52417f487cc1a8dd376017b2db7d"
}
@@ -0,0 +1,16 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE postgres_trigger SET server_id = NULL, last_server_ping = NULL\n WHERE workspace_id = $1\n AND (postgres_resource_path = $2 OR postgres_resource_path LIKE $3)",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text",
"Text"
]
},
"nullable": []
},
"hash": "4692810d2be817bbb5de9b476d68d695941bd4fb5ccef393e4da522ed479d601"
}
@@ -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 dt.value->'reference'->>'workspace_id' = $1\n AND dt.value->'reference'->>'datatable' = $2",
"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": "5048e21546f9710697100100e1255ab103979433bc386d7c89d0e30db12bfd57"
}
@@ -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 dt.value->'reference'->>'workspace_id' = $1\n AND dt.value->'reference'->>'datatable' = $2",
"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": "538dd1779874e4003932d7f17750239c625f85e25b3364bf2edf566f518c8ee2"
}
@@ -0,0 +1,23 @@
{
"db_name": "PostgreSQL",
"query": "SELECT username FROM usr WHERE workspace_id = $1 AND email = $2 AND disabled = false",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "username",
"type_info": "Varchar"
}
],
"parameters": {
"Left": [
"Text",
"Text"
]
},
"nullable": [
false
]
},
"hash": "58e5cfe9eb87bda9f7de87c403861b6e7b9d35a41594681e2a92a87359e6a018"
}
@@ -1,24 +0,0 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE flow_conversation SET title = $1, updated_at = updated_at\n WHERE id = $2 AND workspace_id = $3\n RETURNING id",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "id",
"type_info": "Uuid"
}
],
"parameters": {
"Left": [
"Varchar",
"Uuid",
"Text"
]
},
"nullable": [
false
]
},
"hash": "5b9c9eb64051f291fed4be9bc0b0cc0aef2e7bde732899976eddac36a2da7658"
}
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "SELECT id, workspace_id, flow_path, title, created_at, updated_at, created_by, is_test\n FROM flow_conversation\n WHERE id = $1 AND workspace_id = $2",
"query": "SELECT id, workspace_id, flow_path, title, created_at, updated_at, created_by\n FROM flow_conversation\n WHERE id = $1 AND workspace_id = $2\n FOR UPDATE",
"describe": {
"columns": [
{
@@ -37,11 +37,6 @@
"ordinal": 6,
"name": "created_by",
"type_info": "Varchar"
},
{
"ordinal": 7,
"name": "is_test",
"type_info": "Bool"
}
],
"parameters": {
@@ -57,9 +52,8 @@
true,
false,
false,
false,
false
]
},
"hash": "48c8522a4fed219c5011f4ba63c81cfe028a8b2a32bd790840cef65c452a8c31"
"hash": "6f32c1feed096ff706ae359ad6a3ca33b3f82ca38289dfa4a69aa95041027d57"
}
@@ -0,0 +1,15 @@
{
"db_name": "PostgreSQL",
"query": "INSERT INTO global_settings (name, value) VALUES ($1, $2)\n ON CONFLICT (name) DO UPDATE SET value = $2",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Varchar",
"Jsonb"
]
},
"nullable": []
},
"hash": "6f9fb5d72f486358fa25d6887bd69b93910e028f140c07048f2c1c8d63ee6909"
}
@@ -0,0 +1,38 @@
{
"db_name": "PostgreSQL",
"query": "SELECT id, name, enabled, pwd FROM datatable_role",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "id",
"type_info": "Varchar"
},
{
"ordinal": 1,
"name": "name",
"type_info": "Varchar"
},
{
"ordinal": 2,
"name": "enabled",
"type_info": "Bool"
},
{
"ordinal": 3,
"name": "pwd",
"type_info": "Text"
}
],
"parameters": {
"Left": []
},
"nullable": [
false,
false,
false,
true
]
},
"hash": "71ee2cb6661cca1fa4d8874a7f6d368347c59f36fd87df6dc7996152ccb84af0"
}
@@ -0,0 +1,29 @@
{
"db_name": "PostgreSQL",
"query": "\n SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"datatable!\"\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id AND w.deleted = false\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE ws.workspace_id <> $1\n AND dt.value->'database'->>'resource_type' = 'instance'\n AND dt.value->'database'->>'resource_path' = $2\n ORDER BY ws.workspace_id, dt.key\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": "79799b5a2e499df6c28e286c42b9ad2db940c2455ab19cc95e5198baf96d5629"
}
@@ -0,0 +1,17 @@
{
"db_name": "PostgreSQL",
"query": "INSERT INTO datatable_role (id, name, enabled, pwd) VALUES ($1, $2, $3, $4)",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Varchar",
"Varchar",
"Bool",
"Text"
]
},
"nullable": []
},
"hash": "86af9d51a158ea5cb6161461ecddf2a63695f8cbf8af648da5a0a77a5b9d02ba"
}
@@ -0,0 +1,15 @@
{
"db_name": "PostgreSQL",
"query": "INSERT INTO global_settings (name, value) VALUES ($1, $2)\n ON CONFLICT (name) DO UPDATE SET value = $2, updated_at = now()",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Varchar",
"Jsonb"
]
},
"nullable": []
},
"hash": "975e404ac3a6258bb8220e122e3de094c7ab23330fdbc74d6e4ad472ddd3c820"
}
@@ -0,0 +1,20 @@
{
"db_name": "PostgreSQL",
"query": "SELECT pg_advisory_xact_lock(hashtext('datatable_role_catalog'))",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "pg_advisory_xact_lock",
"type_info": "Void"
}
],
"parameters": {
"Left": []
},
"nullable": [
null
]
},
"hash": "9f663180166f53d117e794f3f3a5723a0a43db163ecca7d5a63d4e74ab1d3be1"
}
@@ -0,0 +1,16 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE capture_config SET server_id = NULL, last_server_ping = NULL\n WHERE workspace_id = $1 AND trigger_kind = 'postgres'\n AND (trigger_config->>'postgres_resource_path' = $2\n OR trigger_config->>'postgres_resource_path' LIKE $3)",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text",
"Text"
]
},
"nullable": []
},
"hash": "a57de2bb0442a5ee8a607cd63cfcf675de175796184f620cb4b09670c8b0b19f"
}
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "INSERT INTO flow_conversation_message (conversation_id, message_type, content, job_id, step_name, success, tool_arguments, tool_result, reasoning, attachments)\n VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)",
"query": "INSERT INTO flow_conversation_message (conversation_id, message_type, content, job_id, step_name, success)\n VALUES ($1, $2, $3, $4, $5, $6)",
"describe": {
"columns": [],
"parameters": {
@@ -21,14 +21,10 @@
"Text",
"Uuid",
"Varchar",
"Bool",
"Text",
"Text",
"Text",
"Jsonb"
"Bool"
]
},
"nullable": []
},
"hash": "12329c3359a7944ab5fa3aa27ddca1b26f340ccf574b9fa07641fe88b2d2987c"
"hash": "b1a9a433e577133869c067b2ce383fc6ce4e9df307feb5fd3edc0d1276d61ff1"
}
@@ -0,0 +1,22 @@
{
"db_name": "PostgreSQL",
"query": "SELECT ws.datatable->'datatables' FROM workspace_settings ws\n WHERE ws.workspace_id = $1 FOR UPDATE",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "?column?",
"type_info": "Jsonb"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
null
]
},
"hash": "b42af37fb474bea4c5419b0a46d9eadfe384013ab970ccf9c5effd1c78321b7c"
}
@@ -0,0 +1,20 @@
{
"db_name": "PostgreSQL",
"query": "SELECT jsonb_object_keys(value->'databases') FROM global_settings\n WHERE name = 'custom_instance_pg_databases'",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "jsonb_object_keys",
"type_info": "Text"
}
],
"parameters": {
"Left": []
},
"nullable": [
null
]
},
"hash": "b9842d2d8abf382bd82d8fa1de012373638be391f884f81dc387ffc465badac6"
}
@@ -0,0 +1,28 @@
{
"db_name": "PostgreSQL",
"query": "DELETE FROM usr WHERE email = $1 RETURNING username, workspace_id",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "username",
"type_info": "Varchar"
},
{
"ordinal": 1,
"name": "workspace_id",
"type_info": "Varchar"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
false,
false
]
},
"hash": "c1d026c886799dabc39ce73e1fe09ccb175c7271df75d67aa9c72ad6f825a992"
}
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "INSERT INTO flow_conversation (id, workspace_id, flow_path, created_by, title, is_test)\n VALUES ($1, $2, $3, $4, $5, $6)\n ON CONFLICT (id) DO NOTHING\n RETURNING id, workspace_id, flow_path, title, created_at, updated_at, created_by, is_test",
"query": "INSERT INTO flow_conversation (id, workspace_id, flow_path, created_by, title)\n VALUES ($1, $2, $3, $4, $5)\n ON CONFLICT (id) DO NOTHING\n RETURNING id, workspace_id, flow_path, title, created_at, updated_at, created_by",
"describe": {
"columns": [
{
@@ -37,11 +37,6 @@
"ordinal": 6,
"name": "created_by",
"type_info": "Varchar"
},
{
"ordinal": 7,
"name": "is_test",
"type_info": "Bool"
}
],
"parameters": {
@@ -50,8 +45,7 @@
"Varchar",
"Varchar",
"Varchar",
"Varchar",
"Bool"
"Varchar"
]
},
"nullable": [
@@ -61,9 +55,8 @@
true,
false,
false,
false,
false
]
},
"hash": "6d259b8cce5da5fecefe4ce322789b6b2cc43b51f2056c677d58f39c31fb26cb"
"hash": "c1e3ed3ecc3bcb98f60ba8196d33fee4a74f61b061e5025ecb75882208b3ba8f"
}
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "SELECT id, workspace_id, flow_path, title, created_at, updated_at, created_by, is_test\n FROM flow_conversation\n WHERE id = $1 AND workspace_id = $2\n FOR UPDATE",
"query": "SELECT id, workspace_id, flow_path, title, created_at, updated_at, created_by\n FROM flow_conversation\n WHERE id = $1 AND workspace_id = $2",
"describe": {
"columns": [
{
@@ -37,11 +37,6 @@
"ordinal": 6,
"name": "created_by",
"type_info": "Varchar"
},
{
"ordinal": 7,
"name": "is_test",
"type_info": "Bool"
}
],
"parameters": {
@@ -57,9 +52,8 @@
true,
false,
false,
false,
false
]
},
"hash": "dd84f9dfb238d18cb74f9e43228345427131bf8008eba6920021d6a449791534"
"hash": "c383cc023714b361d10c10e8fef1fc148ab1da942951ee9ffdddaecee76a6be9"
}
@@ -0,0 +1,28 @@
{
"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 dt.value->'reference'->>'workspace_id' = $1\n ORDER BY ws.workspace_id, dt.key",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "workspace_id!",
"type_info": "Varchar"
},
{
"ordinal": 1,
"name": "datatable!",
"type_info": "Text"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
false,
null
]
},
"hash": "c5451ea9d9fa5146af242d1ee8c19ebd65b80e7ed9f29b9fb2e03767c2aa94ba"
}
@@ -0,0 +1,14 @@
{
"db_name": "PostgreSQL",
"query": "DELETE FROM datatable_role WHERE id = $1",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text"
]
},
"nullable": []
},
"hash": "c85d362fe2e652d4ac01a35bf470e80b993020a2ff5dcb5849dc570d52798587"
}
@@ -0,0 +1,24 @@
{
"db_name": "PostgreSQL",
"query": "SELECT dt.key AS \"datatable!\"\n FROM workspace_settings ws\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE ws.workspace_id = $1\n AND dt.key <> $2\n AND NOT dt.value ? 'permissions'\n AND dt.value->'database'->>'resource_type' = 'instance'\n AND dt.value->'database'->>'resource_path' = $3\n ORDER BY dt.key",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "datatable!",
"type_info": "Text"
}
],
"parameters": {
"Left": [
"Text",
"Text",
"Text"
]
},
"nullable": [
null
]
},
"hash": "d48ca62c86b1af7a9dd2450c1c28dc45020a2a553d8874c49f9eafedea5a9d40"
}
@@ -0,0 +1,22 @@
{
"db_name": "PostgreSQL",
"query": "SELECT datatable FROM workspace_settings WHERE workspace_id = $1",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "datatable",
"type_info": "Jsonb"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
true
]
},
"hash": "d5fb5dde6300862f978739a3d9249fc2b3e7697c0da7d3195398933d3d81aadf"
}
@@ -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 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\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\"%'",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text",
"Text"
]
},
"nullable": []
},
"hash": "da5057c86607327bafc2942c218025ca9181a0c396405984d87e422e129521c1"
}
@@ -0,0 +1,20 @@
{
"db_name": "PostgreSQL",
"query": "SELECT value->'roles' FROM global_settings WHERE name = 'custom_instance_pg_databases'",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "?column?",
"type_info": "Jsonb"
}
],
"parameters": {
"Left": []
},
"nullable": [
null
]
},
"hash": "dc8dfc37559e9b6713bde48155f48b5a2c7b8199eace1508e102b60d1ff40c04"
}
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "\n SELECT\n j.args as \"args: Json<HashMap<String, Box<RawValue>>>\",\n js.flow_status as \"flow_status: Json<windmill_common::flow_status::FlowStatus>\",\n j.runnable_path\n FROM v2_job_status js\n INNER JOIN v2_job j ON j.id = js.id\n WHERE js.id = $1\n ",
"query": "\n SELECT\n j.args as \"args: Json<HashMap<String, Box<RawValue>>>\",\n js.flow_status as \"flow_status: Json<windmill_common::flow_status::FlowStatus>\"\n FROM v2_job_status js\n INNER JOIN v2_job j ON j.id = js.id\n WHERE js.id = $1\n ",
"describe": {
"columns": [
{
@@ -12,11 +12,6 @@
"ordinal": 1,
"name": "flow_status: Json<windmill_common::flow_status::FlowStatus>",
"type_info": "Jsonb"
},
{
"ordinal": 2,
"name": "runnable_path",
"type_info": "Varchar"
}
],
"parameters": {
@@ -25,10 +20,9 @@
]
},
"nullable": [
true,
true,
true
]
},
"hash": "9008f9abb70a9a07e38acb20bea6a710d0efd77dac4aedeb88d72240e816530b"
"hash": "dd89d652154748d6d7e625e31778f6885d0ee62d29a4b8894a4b459dd215a103"
}
@@ -0,0 +1,16 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE postgres_trigger SET server_id = NULL, last_server_ping = NULL\n WHERE workspace_id = $1\n AND (postgres_resource_path = $2 OR postgres_resource_path LIKE $3)",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text",
"Text"
]
},
"nullable": []
},
"hash": "e159b2ff15633f85e839ee4fe1ec2ecd11caf228ea8d0f52ad66def595644250"
}
@@ -0,0 +1,22 @@
{
"db_name": "PostgreSQL",
"query": "SELECT datatable FROM workspace_settings WHERE workspace_id = $1 FOR UPDATE",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "datatable",
"type_info": "Jsonb"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
true
]
},
"hash": "e2061df65ffd4a72146c4ca316829265289c8d6f625ac272655c88e1ad0b1745"
}
@@ -1,6 +1,6 @@
{
"db_name": "PostgreSQL",
"query": "SELECT id, conversation_id, message_type as \"message_type: MessageType\", content, job_id, created_at, created_seq, step_name, success, tool_arguments, tool_result, reasoning, attachments\n FROM (\n SELECT id, conversation_id, message_type, content, job_id, created_at, created_seq, step_name, success, tool_arguments, tool_result, reasoning, attachments\n FROM flow_conversation_message\n WHERE conversation_id = $1\n ORDER BY created_seq DESC\n LIMIT $2 OFFSET $3\n ) AS messages\n ORDER BY created_seq ASC\n ",
"query": "SELECT id, conversation_id, message_type as \"message_type: MessageType\", content, job_id, created_at, created_seq, step_name, success\n FROM flow_conversation_message\n WHERE conversation_id = $1\n AND created_seq > $2\n ORDER BY created_seq ASC\n LIMIT $3\n ",
"describe": {
"columns": [
{
@@ -58,26 +58,6 @@
"ordinal": 8,
"name": "success",
"type_info": "Bool"
},
{
"ordinal": 9,
"name": "tool_arguments",
"type_info": "Text"
},
{
"ordinal": 10,
"name": "tool_result",
"type_info": "Text"
},
{
"ordinal": 11,
"name": "reasoning",
"type_info": "Text"
},
{
"ordinal": 12,
"name": "attachments",
"type_info": "Jsonb"
}
],
"parameters": {
@@ -96,12 +76,8 @@
false,
false,
true,
false,
true,
true,
true,
true
false
]
},
"hash": "a4a823f70b3dbe6aaf4a61c98345e94c5042fd5e6351fea139a66ecb1fb812ab"
"hash": "e8802be9203c1e88a06e337260ccca029380139f89a01a89033e36a6ed9ac082"
}
@@ -0,0 +1,17 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE workspace_settings\n SET datatable = jsonb_set(\n jsonb_set(\n datatable #- ARRAY['datatables', $2, 'reference'],\n ARRAY['datatables', $2, 'database'], $3::jsonb),\n ARRAY['datatables', $2, 'forked_from'], $4::jsonb\n )\n WHERE workspace_id = $1",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text",
"Jsonb",
"Jsonb"
]
},
"nullable": []
},
"hash": "ebaf3ed3097621da59dd201b5a4b9d1f440692f183c7c378f59e4b73f1c6e241"
}
@@ -0,0 +1,22 @@
{
"db_name": "PostgreSQL",
"query": "SELECT workspace_id FROM workspace_settings WHERE datatable::text LIKE $1",
"describe": {
"columns": [
{
"ordinal": 0,
"name": "workspace_id",
"type_info": "Varchar"
}
],
"parameters": {
"Left": [
"Text"
]
},
"nullable": [
false
]
},
"hash": "f3ee09fb17955ca8d886f446d397063c4094546a7807343b570b823796372cef"
}
@@ -0,0 +1,16 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE workspace_settings\n SET datatable = CASE WHEN $3::jsonb = 'null'::jsonb\n THEN datatable #- ARRAY['datatables', $2, 'permissions']\n ELSE jsonb_set(datatable, ARRAY['datatables', $2, 'permissions'], $3::jsonb)\n END\n WHERE workspace_id = $1",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Text",
"Jsonb"
]
},
"nullable": []
},
"hash": "f4adc9e26ebfebce18a29fb2c21bf06394cacb8a9699a608327b097e0ac1363e"
}
@@ -0,0 +1,17 @@
{
"db_name": "PostgreSQL",
"query": "UPDATE datatable_role SET name = $2, enabled = $3, pwd = $4 WHERE id = $1",
"describe": {
"columns": [],
"parameters": {
"Left": [
"Text",
"Varchar",
"Bool",
"Text"
]
},
"nullable": []
},
"hash": "fcb34e643b888122766e115a01394ab31ac856252aaa76c75ea27a447009c363"
}
+2 -2
View File
@@ -14896,7 +14896,6 @@ dependencies = [
"eventsource-stream",
"futures",
"http 1.5.0",
"indexmap 2.14.2",
"lazy_static",
"mime_guess",
"reqwest 0.13.5",
@@ -15471,6 +15470,7 @@ dependencies = [
"windmill-ai",
"windmill-alerting",
"windmill-api-auth",
"windmill-audit",
"windmill-common",
"windmill-object-store",
]
@@ -15653,6 +15653,7 @@ dependencies = [
"pin-project-lite",
"pkcs1",
"postgres-native-tls 0.5.3",
"postgres-protocol",
"prometheus",
"quick_cache",
"rand 0.9.0",
@@ -15666,7 +15667,6 @@ dependencies = [
"serde",
"serde_json",
"serde_yml",
"sha1",
"sha2 0.10.9",
"size",
"spki",
+1
View File
@@ -624,6 +624,7 @@ wasm-bindgen-test = "^0"
convert_case = "0.6.0"
getrandom = "0.2"
tokio-postgres = {version = "^0.7", features = ["array-impls", "with-serde_json-1", "with-chrono-0_4", "with-uuid-1", "with-bit-vec-0_6"]}
postgres-protocol = "0.6"
rust-postgres = { package = "tokio-postgres", git = "https://github.com/MaterializeInc/rust-postgres", rev = "78c1222577bb091d69bc22b1bc7ad01c14675abe"}
rust-postgres-native-tls = { package = "postgres-native-tls", git = "https://github.com/MaterializeInc/rust-postgres", features = ["runtime"], rev = "78c1222577bb091d69bc22b1bc7ad01c14675abe" }
bit-vec = "=0.6.3"
+1 -1
View File
@@ -1 +1 @@
d252afcc80e77fcc4f9a2a346b80908c8605a6c0
2de95863062cc0b00933afd86800aaf953af47d5
@@ -0,0 +1,18 @@
-- Refuse while the catalog holds anything. Each row is a live Postgres login with a password
-- only this table carries, so dropping it would leave credentials on the cluster that Windmill can
-- no longer disable, delete or even name — and re-applying could not recreate them, because the
-- role names would already be taken. Cleaning them up here is not an option either: dropping a
-- role means reassigning what it owns in *every* instance database, and a migration runs in one.
--
-- Delete the roles through instance settings first; that path does the cluster work.
LOCK TABLE datatable_role IN ACCESS EXCLUSIVE MODE;
DO $$
BEGIN
IF EXISTS (SELECT 1 FROM datatable_role) THEN
RAISE EXCEPTION 'Cannot roll back: % data table role(s) still exist as Postgres logins. Delete them in instance settings first, which drops them from the cluster.',
(SELECT count(*) FROM datatable_role);
END IF;
END $$;
DROP TABLE IF EXISTS datatable_role;
@@ -0,0 +1,21 @@
-- The instance's data table role catalog: one row per Postgres login Windmill created for data
-- table access.
--
-- A table rather than a `global_settings` key, because the value is a set of live cluster
-- credentials and that table has generic read, list, write and CLI round-trip paths that know
-- nothing about what they are carrying. Every one of them is a way to leak the passwords or to
-- overwrite the catalog with a copy that has none, and a row nothing generic touches has none of
-- those. One row per role also makes two concurrent creates two inserts rather than a
-- read-modify-write over one document.
CREATE TABLE datatable_role (
id VARCHAR(50) PRIMARY KEY,
-- The Postgres role name, verbatim. Unique because it is the cluster's own key.
name VARCHAR(63) NOT NULL UNIQUE,
enabled BOOLEAN NOT NULL DEFAULT true,
-- Generated by Windmill, never entered by anyone, and never leaves the server.
pwd TEXT,
created_at TIMESTAMPTZ NOT NULL DEFAULT now()
);
GRANT ALL ON datatable_role TO windmill_user;
GRANT ALL ON datatable_role TO windmill_admin;
@@ -1,3 +0,0 @@
ALTER TABLE flow_conversation_message DROP COLUMN tool_arguments;
ALTER TABLE flow_conversation_message DROP COLUMN tool_result;
ALTER TABLE flow_conversation_message DROP COLUMN reasoning;
@@ -1,12 +0,0 @@
-- A chat is rebuilt from its rows without reading jobs, so every tool row carries its call:
-- the arguments the model wrote and the text the model got back, or what the call failed
-- with. A script or flow tool's job holds the args its input transforms produced, not the
-- model's; an MCP tool runs inside the agent's job, whose result lists every call of the
-- turn with nothing tying one to a row. A provider-native web search carries only its
-- citations, the provider never returning the query.
ALTER TABLE flow_conversation_message ADD COLUMN tool_arguments TEXT;
ALTER TABLE flow_conversation_message ADD COLUMN tool_result TEXT;
-- The thinking behind this row. The agent job keeps the turn's thinking as one string;
-- the rows keep it per iteration, next to the answer or tool call it led to.
ALTER TABLE flow_conversation_message ADD COLUMN reasoning TEXT;
@@ -1 +0,0 @@
ALTER TABLE flow_conversation DROP COLUMN is_test;
@@ -1,26 +0,0 @@
-- A chat run from the flow editor's test panel is stored exactly like one from the
-- deployed flow, so the two were indistinguishable once written. Marking them lets the
-- lists tell a trial apart from a real conversation.
ALTER TABLE flow_conversation ADD COLUMN is_test BOOLEAN NOT NULL DEFAULT false;
-- Existing rows: a conversation whose messages came from a flowpreview run was a test.
-- Derived once here because the job is purged on retention, after which the origin of an
-- old conversation is unknowable.
--
-- Walked to the root job rather than matched directly: an existing message row never holds
-- the flow job itself. The rows point at the step that produced them — the AI agent's job
-- for an answer, the tool's own job for a tool call — whose kind is never 'flowpreview'.
--
-- `root_job` first, matching `get_root_job_id` (windmill-worker/src/common.rs): only it
-- reaches the top of the run. `flow_innermost_root_job` stops at the closest flow scope by
-- design, so an agent inside a subflow would land on that subflow's 'flow' row and the
-- conversation would read as deployed.
UPDATE flow_conversation c
SET is_test = true
WHERE EXISTS (
SELECT 1 FROM flow_conversation_message m
JOIN v2_job j ON j.id = m.job_id
JOIN v2_job root
ON root.id = coalesce(j.root_job, j.flow_innermost_root_job, j.parent_job, j.id)
WHERE m.conversation_id = c.id AND root.kind = 'flowpreview'
);
@@ -1 +0,0 @@
ALTER TABLE flow_conversation_message DROP COLUMN attachments;
@@ -1,4 +0,0 @@
-- The files a user message carried, as object-storage references: `[{input, s3, storage?,
-- filename?}]`. Only references, never file bytes and never a presigned URL, so a
-- transcript can show a message's files without reading its run's args.
ALTER TABLE flow_conversation_message ADD COLUMN attachments JSONB;
@@ -730,7 +730,12 @@ pub fn parse_asset_syntax(
s: &str,
enable_default_syntax: bool,
) -> Option<(AssetKind, Cow<'_, str>)> {
if enable_default_syntax && s == "datatable" {
// `datatable` and `datatable?role=analyst` both name the default data table: the role picks
// which Postgres login the connection is made as, not which data table is read.
if enable_default_syntax
&& s.strip_prefix("datatable")
.is_some_and(|rest| rest.is_empty() || rest.starts_with('?'))
{
return Some((AssetKind::DataTable, Cow::Borrowed("main")));
} else if enable_default_syntax && s == "ducklake" {
return Some((AssetKind::Ducklake, Cow::Borrowed("main")));
@@ -741,6 +746,14 @@ pub fn parse_asset_syntax(
if *kind == AssetKind::Dbt {
return Some((*kind, Cow::Owned(canonicalize_table_asset_path(suffix))));
}
// Same reasoning as above, for the explicit form. Specific to data tables: a
// `Resource`'s `?table=` is part of what it names, and stripping it would merge two
// different assets.
if *kind == AssetKind::DataTable {
if let Some((path, _role)) = suffix.split_once('?') {
return Some((*kind, Cow::Borrowed(path)));
}
}
// The suffix is kept verbatim. For S3 the path encodes the storage:
// `s3://<storage>/<key>`, with an EMPTY storage segment for the
// workspace default — so `s3:///key` yields `/key` (leading slash
@@ -1692,6 +1705,25 @@ fn parse_trigger_spec(s: &str) -> Option<TriggerSpec> {
mod pipeline_annotation_tests {
use super::*;
#[test]
fn a_datatable_role_is_not_part_of_the_asset_it_names() {
// The role picks which Postgres login the connection is made as, so two references that
// differ only by role are the same asset and must land on one graph node.
assert_eq!(
parse_asset_syntax("datatable://sales?role=analytics", false),
Some((AssetKind::DataTable, Cow::Borrowed("sales")))
);
assert_eq!(
parse_asset_syntax("datatable?role=analytics", true),
Some((AssetKind::DataTable, Cow::Borrowed("main")))
);
// A resource's `?table=` is part of what it names, so it is kept.
assert_eq!(
parse_asset_syntax("$res:f/db/pg?table=users", false),
Some((AssetKind::Resource, Cow::Borrowed("f/db/pg?table=users")))
);
}
#[test]
fn s3_path_keeps_storage_distinction() {
// An S3 asset path is `<storage>/<key>` with an empty storage segment
+2 -2
View File
@@ -99,9 +99,9 @@ email_trigger: path(char), local_part(char), workspaced_local_part(bool), script
favorite: usr(char), workspace_id(char), path(char), favorite_kind(favorite_kind)
flow: workspace_id(char), path(char), summary(text), description(text), value(jsonb), edited_by(char), edited_at(ts), archived(bool), schema(json), extra_perms(jsonb), dependency_job(uuid), draft_only(bool), tag(char), ws_error_handler_muted(bool), dedicated_worker(bool), timeout(int), visible_to_runner_only(bool), concurrency_key(char), versions(bigint[]), on_behalf_of(varchar), on_behalf_of_email(text), lock_error_logs(text), labels(text[])
FK: (workspace_id) -> workspace(id)
flow_conversation: id(uuid), workspace_id(char), flow_path(char), title(char), created_at(ts), updated_at(ts), created_by(char), is_test(bool)
flow_conversation: id(uuid), workspace_id(char), flow_path(char), title(char), created_at(ts), updated_at(ts), created_by(char)
FK: (workspace_id) -> workspace(id)
flow_conversation_message: id(uuid), conversation_id(uuid), message_type(message_type), content(text), job_id(uuid), created_at(ts), created_seq(int8), step_name(char), success(bool), tool_arguments(text), tool_result(text), reasoning(text), attachments(jsonb)
flow_conversation_message: id(uuid), conversation_id(uuid), message_type(message_type), content(text), job_id(uuid), created_at(ts), created_seq(int8), step_name(char), success(bool)
FK: (conversation_id) -> flow_conversation(id) | (job_id) -> v2_job(id)
flow_iterator_data: job_id(uuid), itered(jsonb)
flow_node: id(bigint), workspace_id(char), hash(bigint), path(char), lock(text), code(text), flow(jsonb), hash_v2(char(64))
-2
View File
@@ -258,7 +258,6 @@ async fn test_new_turns_wait_for_conversation_cleanup_and_recreate(
"test-user",
"hi again",
conv_id,
false,
)
.await?;
windmill_common::flow_conversations::add_message_to_conversation_tx(
@@ -269,7 +268,6 @@ async fn test_new_turns_wait_for_conversation_cleanup_and_recreate(
windmill_common::flow_conversations::MessageType::User,
None,
true,
None,
)
.await?;
tx.commit().await?;
-1
View File
@@ -23,7 +23,6 @@ async-trait.workspace = true
async-stream.workspace = true
base64.workspace = true
bytes.workspace = true
indexmap.workspace = true
eventsource-stream.workspace = true
futures.workspace = true
http.workspace = true
+2 -6
View File
@@ -1074,8 +1074,7 @@ impl BedrockQueryBuilder {
let mut accumulated_text = String::new();
let mut events_str = String::new();
let mut accumulated_tool_calls: indexmap::IndexMap<String, StreamingToolCall> =
indexmap::IndexMap::new();
let mut accumulated_tool_calls: HashMap<String, StreamingToolCall> = HashMap::new();
let mut current_tool_use_id: Option<String> = None;
let mut usage: Option<TokenUsage> = None;
// Claude reasoning block for the turn (only populated when thinking is on),
@@ -1264,10 +1263,7 @@ mod tests {
// recovers the uncached share by subtracting the details back out.
assert_eq!(usage["usage"]["prompt_tokens"], 1010);
assert_eq!(usage["usage"]["completion_tokens"], 7);
assert_eq!(
usage["usage"]["prompt_tokens_details"]["cached_tokens"],
900
);
assert_eq!(usage["usage"]["prompt_tokens_details"]["cached_tokens"], 900);
assert_eq!(
usage["usage"]["prompt_tokens_details"]["cache_write_tokens"],
100
+8 -11
View File
@@ -1,7 +1,6 @@
use std::collections::HashMap;
use eventsource_stream::Eventsource;
use indexmap::IndexMap;
use reqwest::Response;
use serde::Deserialize;
use tokio_stream::StreamExt;
@@ -138,9 +137,7 @@ pub struct OpenAISSEParser {
pub accumulated_content: String,
/// The thinking streamed before the answer, kept so it can be stored with it.
pub accumulated_reasoning: String,
// Insertion-ordered in every parser: tool calls run and are persisted in the order the
// stream showed them, and a chat attaches a round's thinking to its first call.
pub accumulated_tool_calls: IndexMap<i64, OpenAIToolCall>,
pub accumulated_tool_calls: HashMap<i64, OpenAIToolCall>,
pub events_str: String,
pub stream_event_processor: Box<dyn StreamEventSink>,
/// Token usage from final chunk (when stream_options.include_usage is true)
@@ -152,7 +149,7 @@ impl OpenAISSEParser {
Self {
accumulated_content: String::new(),
accumulated_reasoning: String::new(),
accumulated_tool_calls: IndexMap::new(),
accumulated_tool_calls: HashMap::new(),
events_str: String::new(),
stream_event_processor,
usage: None,
@@ -362,7 +359,7 @@ pub struct AnthropicSSEParser {
pub accumulated_content: String,
/// The thinking streamed before the answer, kept so it can be stored with it.
pub accumulated_reasoning: String,
pub accumulated_tool_calls: IndexMap<i64, OpenAIToolCall>,
pub accumulated_tool_calls: HashMap<i64, OpenAIToolCall>,
pub events_str: String,
pub stream_event_processor: Box<dyn StreamEventSink>,
/// Track content block types by index
@@ -385,7 +382,7 @@ impl AnthropicSSEParser {
Self {
accumulated_content: String::new(),
accumulated_reasoning: String::new(),
accumulated_tool_calls: IndexMap::new(),
accumulated_tool_calls: HashMap::new(),
events_str: String::new(),
stream_event_processor,
content_blocks: HashMap::new(),
@@ -604,7 +601,7 @@ pub struct GeminiSSEParser {
pub accumulated_content: String,
/// The thinking streamed before the answer, kept so it can be stored with it.
pub accumulated_reasoning: String,
pub accumulated_tool_calls: IndexMap<i64, OpenAIToolCall>,
pub accumulated_tool_calls: HashMap<i64, OpenAIToolCall>,
pub events_str: String,
pub stream_event_processor: Box<dyn StreamEventSink>,
tool_call_index: i64,
@@ -618,7 +615,7 @@ impl GeminiSSEParser {
Self {
accumulated_content: String::new(),
accumulated_reasoning: String::new(),
accumulated_tool_calls: IndexMap::new(),
accumulated_tool_calls: HashMap::new(),
events_str: String::new(),
stream_event_processor,
tool_call_index: 0,
@@ -836,7 +833,7 @@ pub struct OpenAIResponsesSSEParser {
pub accumulated_content: String,
/// The reasoning summary streamed before the answer, kept so it can be stored with it.
pub accumulated_reasoning: String,
pub accumulated_tool_calls: IndexMap<String, OpenAIToolCall>,
pub accumulated_tool_calls: HashMap<String, OpenAIToolCall>,
/// Maps item_id -> (name, call_id) for function calls
tool_call_metadata: HashMap<String, (String, String)>,
/// Maps item_id -> accumulated arguments
@@ -858,7 +855,7 @@ impl OpenAIResponsesSSEParser {
Self {
accumulated_content: String::new(),
accumulated_reasoning: String::new(),
accumulated_tool_calls: IndexMap::new(),
accumulated_tool_calls: HashMap::new(),
tool_call_metadata: HashMap::new(),
tool_call_arguments: HashMap::new(),
events_str: String::new(),
+6 -56
View File
@@ -78,50 +78,17 @@ impl Default for OutputType {
#[serde(tag = "kind", rename_all = "lowercase")]
pub enum Memory {
Off,
Window {
#[serde(default, deserialize_with = "deserialize_null_as_zero")]
context_length: usize,
},
/// Written before `window`. Its `memory_id` stays a fallback behind the run's memory id.
Auto {
#[serde(default, deserialize_with = "deserialize_null_as_zero")]
#[serde(default)]
context_length: usize,
#[serde(default, deserialize_with = "deserialize_blank_as_none")]
#[serde(default)]
memory_id: Option<Uuid>,
},
/// Written before a step had history inputs of its own, and read on its own where it remains.
Manual {
messages: Vec<OpenAIMessage>,
},
}
// An editor form can leave `""` in a legacy baked id it never filled; it means no id rather than
// failing every run of the step.
fn deserialize_blank_as_none<'de, D: serde::Deserializer<'de>>(
deserializer: D,
) -> Result<Option<Uuid>, D::Error> {
match <Option<String> as serde::Deserialize>::deserialize(deserializer)? {
Some(id) if !id.trim().is_empty() => Uuid::parse_str(id.trim())
.map(Some)
.map_err(serde::de::Error::custom),
_ => Ok(None),
}
}
// A count the editor's number field was cleared of is stored as `null`, which `default` does not
// cover; it reads as 0, memory off, rather than failing every run of the step.
fn deserialize_null_as_zero<'de, D: serde::Deserializer<'de>>(
deserializer: D,
) -> Result<usize, D::Error> {
<Option<usize> as serde::Deserialize>::deserialize(deserializer).map(Option::unwrap_or_default)
}
fn deserialize_present<'de, D: serde::Deserializer<'de>>(
deserializer: D,
) -> Result<Option<serde_json::Value>, D::Error> {
<serde_json::Value as serde::Deserialize>::deserialize(deserializer).map(Some)
}
#[derive(Deserialize)]
struct AIAgentArgsRaw {
provider: ProviderWithResource,
@@ -136,12 +103,6 @@ struct AIAgentArgsRaw {
streaming: Option<bool>,
max_iterations: Option<usize>,
memory: Option<Memory>,
// A null must stay distinguishable from an absent key: a step whose own memory id evaluates to
// nothing runs stateless instead of falling back to the run's memory id.
#[serde(default, deserialize_with = "deserialize_present")]
memory_id: Option<serde_json::Value>,
#[serde(default)]
previous_messages: Option<Vec<OpenAIMessage>>,
enabled_tools: Option<Vec<String>>,
// Legacy field for backward compatibility
messages_context_length: Option<usize>,
@@ -163,10 +124,6 @@ pub struct AIAgentArgs {
pub streaming: Option<bool>,
pub max_iterations: Option<usize>,
pub memory: Option<Memory>,
/// Memory id set on the step, overriding the run's. Empty when its expression produced none.
pub memory_id: Option<String>,
/// History supplied by the flow, replayed without reading or writing memory.
pub previous_messages: Option<Vec<OpenAIMessage>>,
/// Which of the agent's tools this run may call; `narrow_roster` holds what the names are and
/// what `None` means.
pub enabled_tools: Option<Vec<String>>,
@@ -182,17 +139,12 @@ impl From<AIAgentArgsRaw> for AIAgentArgs {
});
// Backward compatibility: if context_length is 0, use off mode
let memory = memory.map(|memory| match memory {
Memory::Auto { context_length: 0, .. } | Memory::Window { context_length: 0 } => {
let memory = memory.map(|memory| {
if let Memory::Auto { context_length: 0, .. } = memory {
Memory::Off
} else {
memory
}
memory => memory,
});
let memory_id = raw.memory_id.map(|value| match value {
serde_json::Value::Null => String::new(),
serde_json::Value::String(s) => s.trim().to_string(),
value => value.to_string(),
});
AIAgentArgs {
@@ -207,8 +159,6 @@ impl From<AIAgentArgsRaw> for AIAgentArgs {
streaming: raw.streaming,
max_iterations: raw.max_iterations,
memory,
memory_id,
previous_messages: raw.previous_messages,
enabled_tools: raw.enabled_tools,
credentials_check: raw.credentials_check.unwrap_or(false),
}
@@ -1,6 +1,6 @@
use axum::{
extract::{Path, Query},
routing::{delete, get, post},
routing::{delete, get},
Extension, Json, Router,
};
use chrono::{DateTime, Utc};
@@ -15,14 +15,13 @@ use windmill_common::{
db::{UserDB, DB},
error::{JsonResult, Result},
flow_conversations::MessageType,
utils::{not_found_if_none, paginate, truncate_with_ellipsis, Pagination},
utils::{not_found_if_none, paginate, Pagination},
};
pub fn workspaced_service() -> Router {
Router::new()
.route("/list", get(list_conversations))
.route("/delete/{conversation_id}", delete(delete_conversation))
.route("/update/{conversation_id}", post(update_conversation))
.route("/{conversation_id}/messages", get(list_messages))
}
@@ -37,37 +36,11 @@ pub struct FlowConversationMessage {
pub created_seq: i64,
pub step_name: Option<String>,
pub success: bool,
/// On a tool row, the arguments the model wrote. For a Windmill tool these exclude the
/// inputs its step wires in. Null for a web search, whose query the provider does not
/// return.
pub tool_arguments: Option<String>,
/// On a tool row, the text the model got back, or what the call failed with; a web
/// search's citations.
pub tool_result: Option<String>,
/// On an answer, the thinking that produced it; on a tool row, the thinking that led to
/// the call. The agent job keeps the turn's thinking as one string.
pub reasoning: Option<String>,
/// The files a user message carried, as object-storage references
/// (`[{input, s3, storage?, filename?}]`).
pub attachments: Option<sqlx::types::JsonValue>,
}
/// Which conversations a listing holds. A test chat was started from the editor's test
/// panel; a deployed one from the flow itself.
#[derive(Deserialize, Default, Clone, Copy)]
#[serde(rename_all = "lowercase")]
pub enum ConversationKind {
Test,
/// The default: a deployed flow's chat should not surface someone's trial runs.
#[default]
Deployed,
All,
}
#[derive(Deserialize)]
pub struct ListConversationsQuery {
pub flow_path: Option<String>,
pub kind: Option<ConversationKind>,
}
#[derive(Deserialize)]
@@ -94,7 +67,6 @@ async fn list_conversations(
"created_at",
"updated_at",
"created_by",
"is_test",
])
.and_where_eq("workspace_id", "?".bind(&w_id));
@@ -102,16 +74,6 @@ async fn list_conversations(
sqlb.and_where_eq("flow_path", "?".bind(flow_path));
}
match query.kind.unwrap_or_default() {
ConversationKind::Test => {
sqlb.and_where_eq("is_test", "true");
}
ConversationKind::Deployed => {
sqlb.and_where_eq("is_test", "false");
}
ConversationKind::All => {}
}
sqlb.order_by("updated_at", true)
.limit(per_page as i64)
.offset(offset as i64);
@@ -139,7 +101,7 @@ async fn delete_conversation(
// Verify the conversation exists and belongs to the user
let conversation = sqlx::query_as!(
FlowConversation,
"SELECT id, workspace_id, flow_path, title, created_at, updated_at, created_by, is_test
"SELECT id, workspace_id, flow_path, title, created_at, updated_at, created_by
FROM flow_conversation
WHERE id = $1 AND workspace_id = $2",
conversation_id,
@@ -186,50 +148,6 @@ async fn delete_conversation(
Ok(format!("Conversation {} deleted", conversation_id))
}
#[derive(Deserialize)]
pub struct UpdateConversation {
pub title: String,
}
async fn update_conversation(
authed: ApiAuthed,
Extension(user_db): Extension<UserDB>,
Path((w_id, conversation_id)): Path<(String, Uuid)>,
Json(update): Json<UpdateConversation>,
) -> Result<String> {
// Postgres refuses a NUL in a text column, so it must not reach the query as a 500.
if update.title.contains('\0') {
return Err(windmill_common::error::Error::BadRequest(
"title cannot contain a NUL character".to_string(),
));
}
// The column is VARCHAR(255) and the helper appends an ellipsis to what it cuts, so the
// bound it takes is three short of the column's. A longer title would otherwise reach
// Postgres as a 22001 and come back a 500.
let title = truncate_with_ellipsis(update.title.trim(), 252);
let mut tx = user_db.clone().begin(&authed).await?;
// `updated_at` is kept: the list is ordered by it, and a rename must not move the
// chat to the top the way a new turn does.
let updated = sqlx::query_scalar!(
"UPDATE flow_conversation SET title = $1, updated_at = updated_at
WHERE id = $2 AND workspace_id = $3
RETURNING id",
title,
conversation_id,
&w_id
)
.fetch_optional(&mut *tx)
.await?;
not_found_if_none(updated, "Conversation", conversation_id.to_string())?;
tx.commit().await?;
Ok(format!("Conversation {} updated", conversation_id))
}
async fn list_messages(
authed: ApiAuthed,
Extension(user_db): Extension<UserDB>,
@@ -260,7 +178,7 @@ async fn list_messages(
let messages = if let Some(after_seq) = query.after_seq {
sqlx::query_as!(
FlowConversationMessage,
r#"SELECT id, conversation_id, message_type as "message_type: MessageType", content, job_id, created_at, created_seq, step_name, success, tool_arguments, tool_result, reasoning, attachments
r#"SELECT id, conversation_id, message_type as "message_type: MessageType", content, job_id, created_at, created_seq, step_name, success
FROM flow_conversation_message
WHERE conversation_id = $1
AND created_seq > $2
@@ -277,9 +195,9 @@ async fn list_messages(
// Fetch messages for this conversation, oldest first, but reverse the order of the messages for easy rendering on the frontend
sqlx::query_as!(
FlowConversationMessage,
r#"SELECT id, conversation_id, message_type as "message_type: MessageType", content, job_id, created_at, created_seq, step_name, success, tool_arguments, tool_result, reasoning, attachments
r#"SELECT id, conversation_id, message_type as "message_type: MessageType", content, job_id, created_at, created_seq, step_name, success
FROM (
SELECT id, conversation_id, message_type, content, job_id, created_at, created_seq, step_name, success, tool_arguments, tool_result, reasoning, attachments
SELECT id, conversation_id, message_type, content, job_id, created_at, created_seq, step_name, success
FROM flow_conversation_message
WHERE conversation_id = $1
ORDER BY created_seq DESC
@@ -800,6 +800,14 @@ async fn delete_folder(
not_found_if_none(get_folderopt(&mut tx, &w_id, &name).await?, "Folder", &name)?;
// See the same call in `delete_group`: a freed name must not stay in a tenant list.
windmill_common::workspaces::remove_datatable_tenant_in_workspace(
&mut tx,
&w_id,
&format!("f/{name}"),
)
.await?;
let del = sqlx::query_scalar!(
"DELETE FROM folder WHERE name = $1 AND workspace_id = $2 RETURNING 1",
name,
@@ -797,6 +797,15 @@ async fn delete_group(
}
not_found_if_none(get_group_opt(&mut tx, &w_id, &name).await?, "Group", &name)?;
// A tenant list names a principal, so a freed name must not linger in one: a later group
// reusing it would silently inherit the data table access this one had.
windmill_common::workspaces::remove_datatable_tenant_in_workspace(
&mut tx,
&w_id,
&format!("g/{name}"),
)
.await?;
sqlx::query!(
"DELETE FROM usr_to_group WHERE group_ = $1 AND workspace_id = $2",
name,
@@ -0,0 +1,108 @@
//! Who may read and change a data table's grants and owners. On the Enterprise Edition: its
//! administrators, from the workspace that governs it. Without it: nobody. Each refusal is decided
//! before anything connects to the data table, so the fixture's database never has to exist.
use serde_json::{json, Value};
use sqlx::{Pool, Postgres};
use windmill_test_utils::*;
fn grant_select_on_public() -> Value {
json!({
"target": {"kind": "schema", "schema": "public"},
"change": {"type": "grant", "role": "analytics", "privileges": ["SELECT"],
"scope": "all_tables"},
"statements": [r#"GRANT SELECT ON ALL TABLES IN SCHEMA "public" TO "analytics""#]
})
}
async fn post_acl(
port: u16,
w_id: &str,
action: &str,
token: &str,
) -> anyhow::Result<reqwest::Response> {
Ok(reqwest::Client::new()
.post(format!(
"http://localhost:{port}/api/w/{w_id}/workspaces/datatable_acl/main/{action}"
))
.header("Authorization", format!("Bearer {token}"))
.json(&grant_select_on_public())
.send()
.await?)
}
/// A fork reaches the data table through a pointer: it may use it, never change what each role may
/// touch on it — not even as an admin of the fork.
#[cfg(all(feature = "private", feature = "enterprise"))]
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
async fn a_fork_cannot_change_access_on_the_data_table_it_points_at(
db: Pool<Postgres>,
) -> anyhow::Result<()> {
initialize_tracing().await;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
for action in ["plan", "apply"] {
let resp = post_acl(port, "wm-fork-dt", action, "SECRET_TOKEN_2").await?;
assert_eq!(resp.status(), 401, "{action}: {}", resp.text().await?);
}
Ok(())
}
#[cfg(all(feature = "private", feature = "enterprise"))]
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
async fn a_member_who_is_not_an_admin_cannot_change_access(
db: Pool<Postgres>,
) -> anyhow::Result<()> {
initialize_tracing().await;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
for action in ["plan", "apply"] {
let resp = post_acl(port, "test-workspace", action, "SECRET_TOKEN_2").await?;
assert_eq!(resp.status(), 401, "{action}: {}", resp.text().await?);
}
Ok(())
}
/// Not even reading, and not even on a data table that is not under roles — which any member
/// reaches, so only the edition stands between them and the instance's credentials.
#[cfg(not(all(feature = "private", feature = "enterprise")))]
#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))]
async fn only_the_enterprise_edition_has_the_access_editor(
db: Pool<Postgres>,
) -> anyhow::Result<()> {
initialize_tracing().await;
sqlx::query(
"UPDATE workspace_settings
SET datatable = datatable #- '{datatables,main,permissions}'
WHERE workspace_id = 'test-workspace'",
)
.execute(&db)
.await?;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
let read = reqwest::Client::new()
.get(format!(
"http://localhost:{port}/api/w/test-workspace/workspaces/datatable_acl/main?kind=database"
))
.header("Authorization", "Bearer SECRET_TOKEN")
.send()
.await?;
let mut responses = vec![("read", read)];
for action in ["plan", "apply"] {
responses.push((
action,
post_acl(port, "test-workspace", action, "SECRET_TOKEN").await?,
));
}
for (action, resp) in responses {
assert_eq!(resp.status(), 400, "{action}");
let body = resp.text().await?;
assert!(
body.contains("Data table roles are a Windmill Enterprise Edition feature"),
"{action}: {body}"
);
}
Ok(())
}
File diff suppressed because it is too large Load Diff
@@ -0,0 +1,45 @@
-- A data table under roles in `test-workspace`, and a fork whose entry points at it rather than
-- carrying a copy. `test-user-2` is a non-admin of the parent and an admin of the fork: the shape
-- the pointer exists for.
-- Empty registry: role provisioning grants CONNECT on every database named here, and the data
-- table's `dt_main` is a name in workspace settings, not a database that exists.
INSERT INTO global_settings (name, value) VALUES
('custom_instance_pg_databases', '{"user_pwd": "pw", "databases": {}}'::jsonb)
ON CONFLICT (name) DO UPDATE SET value = EXCLUDED.value;
INSERT INTO datatable_role (id, name, enabled, pwd) VALUES ('role1', 'analytics', true, 'pw');
UPDATE workspace_settings SET datatable = '{
"datatables": {
"main": {
"database": {"resource_type": "instance", "resource_path": "dt_main"},
"permissions": {
"default_role": "role1",
"roles": {
"admin": {"tenants": []},
"role1": {"tenants": ["u/test-user-2", "g/analysts", "f/finance"]}
}
}
}
}
}'::jsonb WHERE workspace_id = 'test-workspace';
INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES
('test-workspace', 'analysts', 'Analysts', '{}');
INSERT INTO folder (workspace_id, name, display_name, owners, extra_perms) VALUES
('test-workspace', 'finance', 'finance', '{}', '{}');
INSERT INTO workspace (id, name, owner, parent_workspace_id) VALUES
('wm-fork-dt', 'fork of test-workspace', 'test2@windmill.dev', 'test-workspace');
INSERT INTO workspace_key (workspace_id, kind, key) VALUES ('wm-fork-dt', 'cloud', 'test-key');
INSERT INTO group_ (workspace_id, name, summary, extra_perms) VALUES
('wm-fork-dt', 'all', 'All users', '{}');
INSERT INTO usr (workspace_id, email, username, is_admin, role) VALUES
('wm-fork-dt', 'test2@windmill.dev', 'test-user-2', true, 'Admin');
INSERT INTO workspace_settings (workspace_id, datatable) VALUES ('wm-fork-dt', '{
"datatables": {
"main": {"reference": {"workspace_id": "test-workspace", "datatable": "main"}}
}
}'::jsonb);
+6 -17
View File
@@ -26,9 +26,7 @@ use windmill_api_auth::{check_scopes, get_scope_tags, ApiAuthed};
use windmill_common::{
db::{UserDB, UserDbWithAuthed},
error::{self, Error},
flow_conversations::{
add_message_to_conversation_tx, message_attachments, MessageExtras, MessageType,
},
flow_conversations::{add_message_to_conversation_tx, MessageType},
get_latest_flow_version_info_for_path,
jobs::{
check_tag_available_for_workspace_internal, format_result, script_path_to_payload,
@@ -655,11 +653,9 @@ pub async fn set_flow_memory_id(
pub async fn process_flow_run_query_params(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
job_id: Uuid,
w_id: &str,
flow_path: &str,
run_query: &RunJobQuery,
) -> error::Result<()> {
if let Some(memory_id) = run_query.memory_key(w_id, flow_path) {
if let Some(memory_id) = run_query.memory_id {
set_flow_memory_id(tx, job_id, memory_id).await?;
}
Ok(())
@@ -673,13 +669,10 @@ pub async fn handle_chat_conversation_messages(
run_query: &RunJobQuery,
user_message_raw: Option<&Box<serde_json::value::RawValue>>,
job_id: Uuid,
is_test: bool,
// The run's args, for the files the message carried.
args: &HashMap<String, Box<serde_json::value::RawValue>>,
) -> error::Result<()> {
// Names the query parameter rather than the field: it is not a flow argument, and
// supplying it as one is the first thing tried on reading `memory_id is required`.
let memory_id = run_query.memory_key(w_id, flow_path).ok_or_else(|| {
let memory_id = run_query.memory_id.ok_or_else(|| {
windmill_common::error::Error::BadRequest(
"memory_id is required for chat-enabled flows. Pass it as the `memory_id` query \
parameter, not as a flow argument: it names the conversation the turn belongs to, \
@@ -708,12 +701,11 @@ pub async fn handle_chat_conversation_messages(
&authed.username,
&user_message,
memory_id,
is_test,
)
.await?;
// The run this message started. The row keeps the files the message carried as
// references; its args are the only record of every other flow input, and nothing
// The run this message started. Its args are the only record of what the message
// carried besides its text — attachments and every other flow input and nothing
// written later points at them: an assistant row holds the AI agent step's job.
add_message_to_conversation_tx(
tx,
@@ -723,7 +715,6 @@ pub async fn handle_chat_conversation_messages(
MessageType::User,
None,
true,
Some(&MessageExtras { attachments: message_attachments(args), ..Default::default() }),
)
.await?;
@@ -831,7 +822,7 @@ pub async fn run_flow<'c>(
.await?;
// Set memory_id if provided (for agent memory)
if let Some(memory_id) = run_query.memory_key(w_id, flow_path) {
if let Some(memory_id) = run_query.memory_id {
set_flow_memory_id(&mut tx, uuid, memory_id).await?;
}
@@ -845,8 +836,6 @@ pub async fn run_flow<'c>(
&run_query,
args.args.get("user_message"),
uuid,
false,
&args.args,
)
.await?;
}
+1 -13
View File
@@ -47,25 +47,13 @@ pub struct RunJobQuery {
pub cache_ignore_s3_path: Option<bool>,
pub skip_preprocessor: Option<bool>,
pub poll_delay_ms: Option<u64>,
/// Any string; see [`RunJobQuery::memory_key`].
pub memory_id: Option<String>,
pub memory_id: Option<Uuid>,
pub trigger_external_id: Option<String>,
pub service_name: Option<String>,
pub suspended_mode: Option<bool>,
}
impl RunJobQuery {
/// The memory id as stored in `flow_status.memory_id`: a uuid is kept, any other string hashed
/// within the workspace and the flow being run.
pub fn memory_key(&self, workspace_id: &str, flow_path: &str) -> Option<Uuid> {
self.memory_id
.as_deref()
.filter(|memory_id| !memory_id.trim().is_empty())
.map(|memory_id| {
windmill_common::flow_conversations::memory_key(workspace_id, flow_path, memory_id)
})
}
pub async fn get_scheduled_for(
&self,
db: &DB,
+2 -1
View File
@@ -11,7 +11,7 @@ path = "src/lib.rs"
[features]
default = []
enterprise = ["license"]
private = ["windmill-common/private"]
private = ["windmill-common/private", "windmill-audit/private"]
parquet = ["windmill-common/parquet", "windmill-object-store/parquet"]
license = ["dep:rsa"]
@@ -19,6 +19,7 @@ license = ["dep:rsa"]
windmill-ai = { workspace = true, default-features = false }
windmill-alerting.workspace = true
windmill-api-auth.workspace = true
windmill-audit.workspace = true
windmill-common = { workspace = true, default-features = false }
axum.workspace = true
anyhow.workspace = true
@@ -0,0 +1,44 @@
/*
* 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 data table role catalog endpoints come from: the enterprise implementation, or a
//! refusal. Roles are an Enterprise Edition feature; see `windmill_common::datatable_roles_oss`.
#[cfg(all(feature = "private", feature = "enterprise"))]
pub(crate) use crate::datatable_roles_ee::{
create_datatable_role, delete_datatable_role, list_datatable_roles, update_datatable_role,
};
#[cfg(not(all(feature = "private", feature = "enterprise")))]
pub(crate) use ce::*;
// The routes stay registered so the API has one shape; each answers after authentication, before
// anything is read.
#[cfg(not(all(feature = "private", feature = "enterprise")))]
mod ce {
use windmill_api_auth::ApiAuthed;
use windmill_common::{
datatable_roles_oss::datatable_roles_unavailable as unavailable, error::Result,
};
pub(crate) async fn list_datatable_roles(_authed: ApiAuthed) -> Result<String> {
Err(unavailable())
}
pub(crate) async fn create_datatable_role(_authed: ApiAuthed) -> Result<String> {
Err(unavailable())
}
pub(crate) async fn update_datatable_role(_authed: ApiAuthed) -> Result<String> {
Err(unavailable())
}
pub(crate) async fn delete_datatable_role(_authed: ApiAuthed) -> Result<String> {
Err(unavailable())
}
}
+85 -1
View File
@@ -17,6 +17,9 @@ mod audit_logs_s3;
mod audit_logs_s3_backfill;
#[cfg(feature = "parquet")]
mod background_task;
#[cfg(all(feature = "private", feature = "enterprise"))]
mod datatable_roles_ee;
mod datatable_roles_oss;
#[cfg(feature = "private")]
mod ee;
pub mod ee_oss;
@@ -57,7 +60,7 @@ use windmill_common::{
global_settings::{
AI_CONFIG_SETTING, APP_WORKSPACED_ROUTE_SETTING, AUTOMATE_USERNAME_CREATION_SETTING,
CRITICAL_ALERT_MUTE_UI_SETTING, CUSTOM_TAGS_SETTING, DEFAULT_TAGS_WORKSPACES_SETTING,
DISABLE_HUB_SETTING, EMAIL_DOMAIN_SETTING, ENV_SETTINGS,
DISABLE_HUB_SETTING, EMAIL_DOMAIN_SETTING, ENV_SETTINGS, EXTERNAL_INSTANCE_PG_SETTING,
GITHUB_APP_WEBHOOK_BASE_URL_SETTING, HTTP_ROUTE_DEFAULT_ALLOWED_ORIGINS_SETTING,
HTTP_ROUTE_WORKSPACED_ROUTE_SETTING, HUB_ACCESSIBLE_URL_SETTING, HUB_BASE_URL_SETTING,
INSTANCE_BANNER_SETTING, MAX_RETENTION_OVERRIDE_WORKSPACES,
@@ -151,10 +154,28 @@ pub fn global_service() -> Router {
"/list_custom_instance_pg_databases",
post(list_custom_instance_pg_databases),
)
.route(
"/datatable_roles",
get(datatable_roles_oss::list_datatable_roles)
.post(datatable_roles_oss::create_datatable_role),
)
.route(
"/datatable_roles/{id}",
post(datatable_roles_oss::update_datatable_role)
.delete(datatable_roles_oss::delete_datatable_role),
)
.route(
"/refresh_custom_instance_user_pwd",
post(refresh_custom_instance_user_pwd),
)
.route(
"/external_instance_pg/status",
get(get_external_instance_pg_status),
)
.route(
"/external_instance_pg/setup",
post(setup_external_instance_pg),
)
.route(
"/setup_custom_instance_pg_database/{name}",
post(setup_custom_instance_pg_database),
@@ -925,6 +946,13 @@ async fn run_setting_pre_write_hook(
value: &serde_json::Value,
) -> error::Result<()> {
match key {
EXTERNAL_INSTANCE_PG_SETTING => {
windmill_common::external_instance_pg::check_external_instance_pg_write(
db,
Some(value),
)
.await?;
}
// The instance AI config is written as an untyped blob through this generic
// endpoint, so it never passes the typed check the workspace handler applies.
// Rates that reach a cost total unbounded would make it negative or infinite.
@@ -1276,6 +1304,14 @@ async fn set_instance_config(
for (key, value) in &settings_diff.upserts {
run_setting_pre_write_hook(&db, key, value).await?;
}
if settings_diff
.deletes
.iter()
.any(|k| k == EXTERNAL_INSTANCE_PG_SETTING)
{
windmill_common::external_instance_pg::check_external_instance_pg_write(&db, None)
.await?;
}
instance_config::apply_settings_diff(&db, &settings_diff)
.await
@@ -1730,6 +1766,54 @@ async fn refresh_custom_instance_user_pwd(
Ok(Json(()))
}
async fn get_external_instance_pg_status(
authed: ApiAuthed,
Extension(db): Extension<DB>,
) -> JsonResult<windmill_common::external_instance_pg::ExternalInstancePgStatus> {
require_super_admin(&db, &authed).await?;
Ok(Json(
windmill_common::external_instance_pg::external_instance_pg_status(&db).await?,
))
}
#[derive(Deserialize)]
struct SetupExternalInstancePgBody {
#[serde(default)]
rotate_passwords: bool,
}
async fn setup_external_instance_pg(
authed: ApiAuthed,
Extension(db): Extension<DB>,
Json(body): Json<SetupExternalInstancePgBody>,
) -> JsonResult<windmill_common::external_instance_pg::ExternalInstancePgSetupReport> {
require_super_admin(&db, &authed).await?;
let report = windmill_common::external_instance_pg::setup_external_instance_pg_unchecked(
&db,
body.rotate_passwords,
)
.await?;
let rotated = body.rotate_passwords.to_string();
let success = report.success.to_string();
windmill_audit::audit_oss::audit_log(
&db,
&authed,
"settings.setup_external_instance_pg",
windmill_audit::ActionKind::Update,
"global",
Some(&authed.email),
Some(
[
("rotate_passwords", rotated.as_str()),
("success", success.as_str()),
]
.into(),
),
)
.await?;
Ok(Json(report))
}
#[derive(Deserialize)]
struct SetupCustomInstanceDbBody {
tag: Option<String>,
+29 -3
View File
@@ -1703,14 +1703,25 @@ async fn delete_user(
.await?;
windmill_common::user_drafts::delete_drafts_of_email(&mut *tx, &email_to_delete).await?;
let usernames = sqlx::query_scalar!(
"DELETE FROM usr WHERE email = $1 RETURNING username",
let memberships = sqlx::query!(
"DELETE FROM usr WHERE email = $1 RETURNING username, workspace_id",
&email_to_delete
)
.fetch_all(&mut *tx)
.await?;
for username in usernames {
for row in memberships {
let username = row.username;
// A tenant list names a principal of its workspace, so the name has to be freed in every
// workspace this account belonged to: a later account taking the username would otherwise
// inherit the data table access it had.
windmill_common::workspaces::remove_datatable_tenant_in_workspace(
&mut tx,
&row.workspace_id,
&format!("u/{username}"),
)
.await?;
sqlx::query!("DELETE FROM password WHERE email = $1", &email_to_delete)
.execute(&mut *tx)
.await?;
@@ -2456,6 +2467,15 @@ pub async fn delete_workspace_user_internal(
tx: &mut Transaction<'_, Postgres>,
authed: Option<&ApiAuthed>, // None for system operations
) -> Result<()> {
// Same reasoning as the `extra_perms` sweep below: a freed username must not stay named
// anywhere that grants access, tenant lists included.
windmill_common::workspaces::remove_datatable_tenant_in_workspace(
tx,
w_id,
&format!("u/{username_to_delete}"),
)
.await?;
// ---- Clean up extra_perms referencing this user ----
let extra_perms_tables = [
"script",
@@ -3965,6 +3985,12 @@ async fn leave_workspace(
) -> Result<String> {
forbid_job_token_account_destruction(&authed)?;
let mut tx = db.begin().await?;
windmill_common::workspaces::remove_datatable_tenant_in_workspace(
&mut tx,
&w_id,
&format!("u/{}", authed.username),
)
.await?;
sqlx::query!(
"DELETE FROM usr WHERE workspace_id = $1 AND username = $2",
&w_id,
File diff suppressed because it is too large Load Diff
@@ -0,0 +1,44 @@
/*
* 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 ACL planner comes from: the enterprise one, or a refusal.
//!
//! Data table roles are an Enterprise Edition feature, and so is everything here — reading who
//! owns what included. `private` alone is not that edition — community builds carry it — so the
//! planner is behind `enterprise` as well.
#[cfg(all(feature = "private", feature = "enterprise"))]
pub(crate) use crate::datatable_acl_ee::plan_statements;
#[cfg(all(feature = "private", feature = "enterprise"))]
pub(crate) fn ensure_datatable_acl_available() -> windmill_common::error::Result<()> {
Ok(())
}
#[cfg(not(all(feature = "private", feature = "enterprise")))]
use {
crate::datatable_acl::{AclChange, AclPlan, AclTarget, CatalogFacts},
windmill_common::{datatable_roles_oss::datatable_roles_unavailable, error::Result},
};
/// Checked first by every ACL route, before anything is read or connected to.
#[cfg(not(all(feature = "private", feature = "enterprise")))]
pub(crate) fn ensure_datatable_acl_available() -> Result<()> {
Err(datatable_roles_unavailable())
}
#[cfg(not(all(feature = "private", feature = "enterprise")))]
pub(crate) fn plan_statements(
_target: &AclTarget,
_change: &AclChange,
_dbname: &str,
_pg_role: &str,
_facts: &CatalogFacts,
) -> Result<AclPlan> {
Err(datatable_roles_unavailable())
}
@@ -30,6 +30,7 @@ use windmill_api_auth::{require_super_admin, ApiAuthed};
use windmill_api_jobs::run_wait_result_internal;
use windmill_audit::audit_oss::audit_log;
use windmill_audit::ActionKind;
use windmill_common::datatable_roles::ADMIN_DATATABLE_ROLE;
use windmill_common::db::UserDB;
use windmill_common::error::{pg_error_message, Error, JsonResult, Result};
use windmill_common::jobs::{JobPayload, RawCode};
@@ -38,7 +39,11 @@ use windmill_common::runnable_settings::{ConcurrencySettingsWithCustom, Debounci
use windmill_common::scripts::ScriptLang;
use windmill_common::users::username_to_permissioned_as;
use windmill_common::worker::to_raw_value;
use windmill_common::workspaces::get_datatable_resource_from_db_unchecked;
use windmill_common::worker::SqlAnnotations;
use windmill_common::workspaces::{
ensure_can_use_datatable_role, ensure_datatable_admin_access,
get_datatable_resource_from_db_unchecked, resolve_governing_datatable, DatatableAccess,
};
use windmill_common::{PgDatabase, DB};
use windmill_git_sync::{
handle_deployment_metadata, handle_deployment_metadata_batch, DeployedObject,
@@ -86,6 +91,42 @@ pub(crate) fn routes() -> Router {
)
}
/// Refuse a migration whose role this caller may not use, before a job is pushed or a version
/// recorded.
///
/// A migration that declares `-- role <name>` runs as that role, so the caller has to be one of its
/// tenants. One that declares none runs as `admin` and reaches every object in the database
/// whatever the roles grant, so it is for the admins of the workspace that governs the data table
/// — a fork can run a migration under a role it holds, never a migration under `admin`.
///
/// The executor re-checks the role when it resolves the connection, so this is not the boundary. It
/// is what makes the refusal legible: which migration, and which role.
async fn ensure_migration_role_allowed(
db: &DB,
w_id: &str,
datatable_name: &str,
authed: &ApiAuthed,
sql: &str,
timestamp: i64,
name: &str,
) -> Result<()> {
let context = format!("Migration {timestamp} ({name})");
let access = DatatableAccess::Authed(authed.to_authed_ref());
match SqlAnnotations::datatable_role(sql)? {
Some(role) => {
ensure_can_use_datatable_role(db, w_id, datatable_name, Some(&role), &access, &context)
.await
}
None => ensure_datatable_admin_access(db, w_id, datatable_name, &access)
.await
.map_err(|e| {
Error::NotAuthorized(format!(
"{context} declares no role, so it would run as admin. {e}"
))
}),
}
}
#[derive(Serialize)]
struct AppliedMigration {
version: i64,
@@ -128,7 +169,18 @@ async fn datatable_database_arg(
.await?
.ok_or_else(|| Error::internal_err(format!("datatable {datatable_name} not found")))?;
Ok(to_raw_value(&format!("datatable://{datatable_name}")))
// `?role=admin` rather than a bare reference, so a migration that declares no `-- role` runs
// as the connection that owns the schema instead of falling through to the data table's
// default role — which is what `ensure_migration_role_allowed` gated it as, and which is the
// only role a DDL statement can be expected to succeed under. A migration that does declare a
// role overrides this: the annotation wins over the reference.
//
// A legacy name containing `?` cannot be migrated through this reference: the appended query
// makes it neither an exact name nor a parseable one. Accepted on purpose, since such names can
// no longer be created and none are expected to carry migrations.
Ok(to_raw_value(&format!(
"datatable://{datatable_name}?role={ADMIN_DATATABLE_ROLE}"
)))
}
/// Run a migration's SQL as a normal Windmill `postgresql` job, permissioned as
@@ -384,6 +436,11 @@ async fn run_datatable_migrations(
Path((w_id, datatable_name)): Path<(String, String)>,
Query(query): Query<RunDatatableMigrationsQuery>,
) -> JsonResult<RunDatatableMigrationsResult> {
// Before the admin connection is opened at all: the bookkeeping below is created and read
// through it, so a caller no role covers must be refused here rather than after the fact.
crate::datatable_permissions::ensure_reaches_datatable(&db, &w_id, &datatable_name, &authed)
.await?;
audit_log(
&db,
&authed,
@@ -440,6 +497,16 @@ async fn run_datatable_migrations(
if applied_versions.contains(&m.timestamp) {
continue;
}
ensure_migration_role_allowed(
&db,
&w_id,
&datatable_name,
&authed,
&m.code_up,
m.timestamp,
&m.name,
)
.await?;
run_datatable_migration_job(&db, &user_db, &authed, &w_id, &database_arg, &m.code_up)
.await
.map_err(|e| {
@@ -506,6 +573,11 @@ async fn rollback_datatable_migrations(
Path((w_id, datatable_name)): Path<(String, String)>,
Query(query): Query<RollbackDatatableMigrationsQuery>,
) -> JsonResult<RollbackDatatableMigrationsResult> {
// Before the admin connection is opened at all: the bookkeeping below is created and read
// through it, so a caller no role covers must be refused here rather than after the fact.
crate::datatable_permissions::ensure_reaches_datatable(&db, &w_id, &datatable_name, &authed)
.await?;
audit_log(
&db,
&authed,
@@ -588,6 +660,17 @@ async fn rollback_datatable_migrations(
))
})?;
ensure_migration_role_allowed(
&db,
&w_id,
&datatable_name,
&authed,
&code_down,
version,
&definition.name,
)
.await?;
let database_arg = datatable_database_arg(&db, &w_id, &datatable_name).await?;
run_datatable_migration_job(&db, &user_db, &authed, &w_id, &database_arg, &code_down)
.await
@@ -748,10 +831,15 @@ async fn read_applied_datatable_versions(
/// List a data table's migrations annotated with whether each has been applied.
async fn datatable_migrations_status(
_authed: ApiAuthed,
authed: ApiAuthed,
Extension(db): Extension<DB>,
Path((w_id, datatable_name)): Path<(String, String)>,
) -> JsonResult<DatatableMigrationsStatusResult> {
// Reads `_wm_migrations` through the data table's admin connection, so it answers to the same
// question as running one: may you reach this data table at all.
crate::datatable_permissions::ensure_reaches_datatable(&db, &w_id, &datatable_name, &authed)
.await?;
let enabled = datatable_migrations_enabled(&db, &w_id, &datatable_name).await?;
if !enabled {
return Ok(Json(DatatableMigrationsStatusResult {
@@ -1431,6 +1519,15 @@ async fn generate_initial_datatable_migration(
Extension(db): Extension<DB>,
Path((w_id, datatable_name)): Path<(String, String)>,
) -> JsonResult<DatatableMigration> {
// Returns a `pg_dump` of the whole schema and writes into the data table's own bookkeeping, so
// it answers to the workspace that governs it rather than to whoever is asking.
ensure_datatable_admin_access(
&db,
&w_id,
&datatable_name,
&DatatableAccess::Authed(authed.to_authed_ref()),
)
.await?;
validate_datatable_path_segment(&datatable_name)?;
ensure_datatable_migrations_enabled(&db, &w_id, &datatable_name).await?;
@@ -1601,9 +1698,21 @@ pub(crate) struct DatatableRename {
pub(crate) to: String,
}
async fn resolve_datatable_pg(db: &DB, w_id: &str, datatable: &str) -> Result<PgDatabase> {
/// The database whose `_wm_migrations` a rename or delete of `datatable` in `w_id` should touch —
/// `None` when that is somebody else's.
///
/// A fork's entry points at the workspace that governs the data table, so renaming or removing it
/// changes what the fork calls the data table and nothing more. Following the pointer here would
/// let a fork admin relabel or wipe the *governing* workspace's migration bookkeeping through
/// their own settings form, and the parent would then re-run every migration from zero.
async fn resolve_datatable_pg(db: &DB, w_id: &str, datatable: &str) -> Result<Option<PgDatabase>> {
let governing = resolve_governing_datatable(db, w_id, datatable).await?;
if governing.workspace_id != w_id {
return Ok(None);
}
let db_resource = get_datatable_resource_from_db_unchecked(db, w_id, datatable).await?;
serde_json::from_value(db_resource)
.map(Some)
.map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e)))
}
@@ -1621,7 +1730,9 @@ fn ignore_missing_wm_migrations(e: tokio_postgres::Error) -> Result<()> {
/// Drop a data table's rows from its own database's `_wm_migrations`.
async fn remote_forget_datatable_migrations(db: &DB, w_id: &str, datatable: &str) -> Result<()> {
let pg_db = resolve_datatable_pg(db, w_id, datatable).await?;
let Some(pg_db) = resolve_datatable_pg(db, w_id, datatable).await? else {
return Ok(());
};
let (client, connection) = pg_db.connect(Some(db)).await?;
tokio::spawn(async move {
let _ = connection.await;
@@ -1646,7 +1757,9 @@ async fn remote_rename_datatable_migrations(
from: &str,
to: &str,
) -> Result<()> {
let pg_db = resolve_datatable_pg(db, w_id, resolve_by).await?;
let Some(pg_db) = resolve_datatable_pg(db, w_id, resolve_by).await? else {
return Ok(());
};
let (client, connection) = pg_db.connect(Some(db)).await?;
tokio::spawn(async move {
let _ = connection.await;
@@ -0,0 +1,79 @@
/*
* 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.
*/
//! Who may connect to a data table as which role.
//!
//! The decision lives on the data table entry of the workspace that governs it, which is not
//! necessarily the workspace asking: a fork's entry points at its parent's, and everything here
//! resolves through that pointer first. Nothing in this module runs SQL against the data table —
//! a save is tenant lists and a default, and the Postgres roles themselves are the instance
//! catalog's business.
use axum::{routing::get, Router};
use windmill_api_auth::ApiAuthed;
use windmill_common::error::Result;
use windmill_common::workspaces::GoverningDatatable;
use windmill_common::DB;
use crate::datatable_permissions_oss as roles;
pub(crate) fn routes() -> Router {
Router::new()
.route(
"/datatable_permissions/{datatable_name}",
get(roles::get_datatable_permissions).post(roles::set_datatable_permissions),
)
.route(
"/datatable_usable_roles/{datatable_name}",
get(roles::list_usable_datatable_roles),
)
}
/// Administering a data table — its permissions, its migrations that declare no role, its exports
/// — is for the admins of the workspace that governs it. A fork can use the data table; it never
/// administers it.
// The gate for whatever administers a data table under roles, which the routes of this module alone
// do not always reach.
#[allow(dead_code)]
pub(crate) async fn ensure_governs_datatable(
db: &DB,
authed: &ApiAuthed,
w_id: &str,
governing: &GoverningDatatable,
) -> Result<()> {
roles::ensure_governs_datatable(db, authed, w_id, governing).await
}
/// Refuse a caller that no tenant of this data table covers.
///
/// The bookkeeping endpoints below open the data table's `admin` connection to read or create
/// `_wm_migrations` before they know which migration will run — so without this, someone covered
/// by no role at all can still force admin-backed reads and writes on a database they may not
/// touch. It asks only "may you reach this data table as anything"; which role a given migration
/// runs as is still decided per migration, and by the executor after that.
pub(crate) async fn ensure_reaches_datatable(
db: &DB,
w_id: &str,
datatable_name: &str,
authed: &ApiAuthed,
) -> Result<()> {
roles::ensure_reaches_datatable(db, w_id, datatable_name, authed).await
}
/// [`ensure_reaches_datatable`] against an entry already resolved, for a caller that goes on to
/// connect from that same entry.
pub(crate) async fn ensure_reaches_governing_datatable(
db: &DB,
w_id: &str,
datatable_name: &str,
governing: &GoverningDatatable,
authed: &ApiAuthed,
) -> Result<()> {
roles::ensure_reaches_governing_datatable(db, w_id, datatable_name, governing, authed).await
}
@@ -0,0 +1,112 @@
/*
* 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 data table permissions endpoints and their gates come from: the enterprise
//! implementation, or a refusal. Roles are an Enterprise Edition feature; see
//! `windmill_common::datatable_roles_oss`.
#[cfg(all(feature = "private", feature = "enterprise"))]
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")))]
pub(crate) use ce::*;
#[cfg(not(all(feature = "private", feature = "enterprise")))]
mod ce {
use windmill_api_auth::ApiAuthed;
use windmill_common::{
datatable_roles_oss::datatable_roles_unavailable as unavailable,
error::Result,
workspaces::{resolve_governing_datatable, GoverningDatatable},
DB,
};
/// Nobody administers a data table's roles without them.
#[allow(dead_code)]
pub(crate) async fn ensure_governs_datatable(
_db: &DB,
_authed: &ApiAuthed,
_w_id: &str,
_governing: &GoverningDatatable,
) -> Result<()> {
Err(unavailable())
}
/// A data table not under roles is reached as it was before roles existed. One under roles is
/// refused: no role of it can be connected as.
pub(crate) async fn ensure_reaches_datatable(
db: &DB,
w_id: &str,
datatable_name: &str,
_authed: &ApiAuthed,
) -> Result<()> {
let governing = resolve_governing_datatable(db, w_id, datatable_name).await?;
if governing.datatable.permissions.is_none() {
Ok(())
} else {
Err(unavailable())
}
}
pub(crate) async fn ensure_reaches_governing_datatable(
_db: &DB,
_w_id: &str,
_datatable_name: &str,
governing: &GoverningDatatable,
_authed: &ApiAuthed,
) -> Result<()> {
if governing.datatable.permissions.is_none() {
Ok(())
} else {
Err(unavailable())
}
}
// The routes stay registered so the API has one shape; each answers after authentication,
// before anything is read.
pub(crate) async fn get_datatable_permissions(_authed: ApiAuthed) -> Result<String> {
Err(unavailable())
}
pub(crate) async fn set_datatable_permissions(_authed: ApiAuthed) -> Result<String> {
Err(unavailable())
}
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(),
})
}
}
@@ -1,7 +1,11 @@
#[cfg(feature = "parquet")]
pub mod ai_session_backups;
pub mod data_metrics;
pub mod datatable_acl;
pub mod datatable_acl_oss;
pub mod datatable_migrations;
pub mod datatable_permissions;
pub mod datatable_permissions_oss;
pub mod deployment_requests;
pub mod workspaces;
pub mod workspaces_extra;
@@ -9,3 +13,9 @@ pub mod workspaces_oss;
#[cfg(feature = "private")]
pub mod workspaces_ee;
#[cfg(all(feature = "private", feature = "enterprise"))]
pub mod datatable_acl_ee;
#[cfg(all(feature = "private", feature = "enterprise"))]
pub mod datatable_permissions_ee;
File diff suppressed because it is too large Load Diff
@@ -492,6 +492,30 @@ pub(crate) async fn change_workspace_id(
.fetch_all(&mut *tx)
.await?;
// 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.
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
))
FROM jsonb_each(ws.datatable->'datatables') dt
)
WHERE jsonb_typeof(ws.datatable->'datatables') = 'object'
AND ws.datatable::text LIKE '%"reference"%'"#,
&rw.new_id,
&old_id,
)
.execute(&mut *tx)
.await?;
info!("Updating workspace_protection_rule table");
sqlx::query!(
"UPDATE workspace_protection_rule SET workspace_id = $1 WHERE workspace_id = $2",
@@ -971,6 +995,22 @@ pub(crate) async fn delete_workspace(
// but the destructive cleanup itself runs only after the commit below: a delete that
// 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!"
FROM workspace_settings ws
CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt
WHERE dt.value->'reference'->>'workspace_id' = $1
ORDER BY ws.workspace_id, dt.key"#,
&w_id,
)
.fetch_all(&db)
.await
.unwrap_or_default();
let fork_ducklake_cleanups = prepare_fork_ducklake_cleanups(&db, &w_id, None)
.await
.unwrap_or_else(|e| {
@@ -1289,7 +1329,23 @@ pub(crate) async fn delete_workspace(
tracing::warn!("failed to broadcast fork lineage change: {e:#}");
}
Ok(format!("Deleted workspace {}", &w_id))
if stranded_pointers.is_empty() {
Ok(format!("Deleted workspace {}", &w_id))
} else {
let stranded = stranded_pointers
.iter()
.map(|r| format!("{}/{}", r.workspace_id, r.datatable))
.collect::<Vec<_>>()
.join(", ");
Ok(format!(
concat!(
"Deleted workspace {}. These data tables were governed by it and no longer ",
"resolve: {}. Their databases still exist; a superadmin can point them at ",
"another workspace's data table."
),
&w_id, stranded
))
}
}
#[derive(Deserialize)]
@@ -1343,15 +1399,20 @@ pub async fn drop_forked_datatable_databases(
let mut errors: Vec<String> = Vec::new();
for dt_name in &req.datatable_names {
let dt = match datatables.get(dt_name) {
Some(dt) if dt.forked_from.is_some() => dt,
// Only a clone is droppable, and a clone is terminal by construction: a kept data table is
// a pointer at the parent's database, which this fork does not own.
let database = match datatables.get(dt_name) {
Some(dt) if dt.forked_from.is_some() => match dt.database.as_ref() {
Some(database) => database,
None => continue,
},
_ => continue,
};
if dt.database.resource_type
if database.resource_type
== windmill_common::workspaces::DataTableCatalogResourceType::Instance
{
let db_to_drop = &dt.database.resource_path;
let db_to_drop = &database.resource_path;
if !db_to_drop.starts_with("wm_fork_") {
errors.push(format!(
"Refusing to drop instance database '{}' for datatable://{}: name does not start with 'wm_fork_'",
File diff suppressed because it is too large Load Diff
+6 -7
View File
@@ -4310,11 +4310,11 @@ async fn execute_component(
}
}
let flow_path = payload
let is_flow = payload
.path
.as_deref()
.and_then(|path| path.strip_prefix("flow/"))
.map(str::to_string);
.as_ref()
.map(|p| p.starts_with("flow/"))
.unwrap_or(false);
// Tag for inline-script jobs is read from the deployed policy in run mode;
// only preview mode (editor) honors the client-supplied tag. This applies to
@@ -4444,9 +4444,8 @@ async fn execute_component(
// Apply runnable query parameters if provided
if let Some(ref run_query) = payload.run_query_params {
if let Some(flow_path) = flow_path.as_deref() {
crate::jobs::process_flow_run_query_params(&mut tx, uuid, &w_id, flow_path, run_query)
.await?;
if is_flow {
crate::jobs::process_flow_run_query_params(&mut tx, uuid, run_query).await?;
}
}
+6
View File
@@ -567,6 +567,9 @@ async fn set_config(
};
let mut tx = user_db.begin(&authed).await?;
if matches!(nc.trigger_kind, TriggerKind::Postgres) {
windmill_common::datatable_roles::lock_datatable_streams(&mut *tx, false).await?;
}
sqlx::query!(
r#"
@@ -614,6 +617,9 @@ async fn ping_config(
)>,
) -> Result<()> {
let mut tx = user_db.begin(&authed).await?;
if matches!(trigger_kind, TriggerKind::Postgres) {
windmill_common::datatable_roles::lock_datatable_streams(&mut *tx, false).await?;
}
sqlx::query!(
r#"
+11 -8
View File
@@ -8343,8 +8343,9 @@ pub async fn run_wait_result_flow_by_version(
/// job lives, in particular DuckDB, which runs in-process in the worker.
///
/// What it does permit is any statement against the workspace's data tables, writes and DDL
/// included: the helper's body is an unrestricted SQL template and data tables carry no
/// per-user ACL. Narrowing that is a separate decision from this exemption.
/// included: the helper's body is an unrestricted SQL template. What that reaches is the
/// operator's own data table role — the preview job is permissioned as them, so the executor
/// resolves it under their tenancy like any other job.
///
/// The database argument is only half the target: the executor honors a `-- database`
/// directive in the SQL over it, and `-- s3` redirects the result set, so both are refused.
@@ -8576,7 +8577,7 @@ async fn run_inline_preview_script(
#[cfg(not(feature = "run_inline"))]
async fn run_inline_preview_script() -> error::Result<Response> {
Err(error::Error::InternalErr(
"inline preview requires the worker feature".to_string(),
"inline preview requires the run_inline feature on the worker".to_string(),
))
}
@@ -8700,7 +8701,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) {
@@ -9540,7 +9546,7 @@ async fn run_preview_flow_job(
.await?;
// Set memory_id if provided (for agent memory)
if let Some(memory_id) = run_query.memory_key(&w_id, &flow_path) {
if let Some(memory_id) = run_query.memory_id {
set_flow_memory_id(&mut tx, uuid, memory_id).await?;
}
@@ -9554,9 +9560,6 @@ async fn run_preview_flow_job(
&run_query,
user_message.as_ref(),
uuid,
// Run from the editor's test panel: a trial, not a real conversation.
true,
&flow_args,
)
.await?;
}
+11
View File
@@ -352,6 +352,17 @@ async fn update_username_in_workpsace<'c>(
new_username: &str,
w_id: &str,
) -> error::Result<()> {
// ---- data table tenants ----
// Tenants name the user, so the rename has to follow here too; a list left naming the old
// username silently drops the access instead of moving it.
windmill_common::workspaces::rename_datatable_tenant_in_workspace(
tx,
w_id,
&format!("u/{old_username}"),
&format!("u/{new_username}"),
)
.await?;
// ---- instance and workspace users ----
sqlx::query!(
"UPDATE usr SET username = $1 WHERE email = $2",
@@ -1639,7 +1639,7 @@ pub(crate) async fn tarball_workspace(
mute_critical_alerts: row.mute_critical_alerts,
color: row.color.clone(),
operator_settings: row.operator_settings.clone(),
datatable: row.datatable.clone(),
datatable: windmill_common::workspaces::strip_datatable_permissions(row.datatable.clone()),
slack_team_id: row.slack_team_id.clone(),
slack_name: row.slack_name.clone(),
slack_command_script: row.slack_command_script.clone(),
@@ -1703,7 +1703,7 @@ pub(crate) async fn tarball_workspace(
mute_critical_alerts: row.mute_critical_alerts,
color: row.color,
operator_settings: row.operator_settings,
datatable: row.datatable,
datatable: windmill_common::workspaces::strip_datatable_permissions(row.datatable),
slack_team_id: row.slack_team_id,
slack_name: row.slack_name,
slack_command_script: row.slack_command_script,
+1 -1
View File
@@ -33,7 +33,6 @@ path = "src/lib.rs"
tar.workspace = true
hmac.workspace = true
sha2.workspace = true
sha1.workspace = true
thiserror.workspace = true
anyhow.workspace = true
serde.workspace = true
@@ -76,6 +75,7 @@ bitflags.workspace = true
once_cell.workspace = true
phf.workspace = true
tokio-postgres.workspace = true
postgres-protocol.workspace = true
postgres-native-tls.workspace = true
native-tls.workspace = true
@@ -0,0 +1,359 @@
/*
* 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.
*/
//! The instance's data table role catalog.
//!
//! A data table role is a real Postgres login role on the Windmill cluster, named exactly as the
//! user named it, shared by every instance database. Windmill decides who may ask for a role (the
//! per-data-table tenant lists in [`crate::workspaces`]); Postgres decides what the role may then
//! touch. The catalog here is only the first half's vocabulary plus the cluster provisioning.
//!
//! Entries are keyed by a generated id so a rename moves nothing else: tenants name the id.
use std::collections::BTreeMap;
use crate::{
error::{Error, Result},
DB,
};
/// The connection every data table resolved to before roles existed (`custom_instance_user`). It
/// owns every pre-existing object, so it is a reserved name rather than a catalog entry: never
/// created, renamed or dropped.
pub const ADMIN_DATATABLE_ROLE: &str = "admin";
/// The login the admin connection uses, and the role every created role is granted to — that
/// membership is what later lets it `ALTER ... OWNER TO` a role and drop it.
pub const CUSTOM_INSTANCE_USER: &str = "custom_instance_user";
/// One catalog entry, as stored in `datatable_role`. The password is per role and instance-wide;
/// it belongs to the instance, not to any workspace's settings.
/// No `Serialize`/`Deserialize`: the catalog is rows now, and a derived `Serialize` would emit
/// `pwd` — the same way out for a credential that the hand-written `Debug` below closes on the log
/// side.
#[derive(Clone)]
pub struct InstanceDatatableRole {
/// The Postgres role name, verbatim.
pub name: String,
pub enabled: bool,
/// Absent only for a role whose provisioning did not finish; resolving as it then errors
/// rather than falling back to admin.
///
/// A plain string rather than a `StringOrSecretRef` like the instance user's password: that
/// one is a secret ref because an operator supplies it and may want it to come from their own
/// backend, while this one is 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.
pub pwd: Option<String>,
}
/// Hand-written so `{:?}` on a catalog cannot put a live Postgres password in a log line or an
/// audit record. Everything else about the entry is safe to print.
impl std::fmt::Debug for InstanceDatatableRole {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("InstanceDatatableRole")
.field("name", &self.name)
.field("enabled", &self.enabled)
.field("pwd", &self.pwd.as_ref().map(|_| "<redacted>"))
.finish()
}
}
pub type DatatableRoleCatalog = BTreeMap<String, InstanceDatatableRole>;
/// Names Postgres or Windmill already owns. `admin` is excluded because it never reaches the
/// cluster as a role name at all — it resolves to `custom_instance_user`.
fn is_reserved_role_name(name: &str) -> bool {
let lower = name.to_ascii_lowercase();
lower == ADMIN_DATATABLE_ROLE
|| lower == "postgres"
|| lower == "public"
|| lower.starts_with("pg_")
|| lower.starts_with("windmill_")
|| lower.starts_with("custom_instance_")
}
/// The charset is what makes every downstream interpolation safe: the name reaches Postgres as a
/// quoted identifier, a `-- role <name>` annotation, and a `?role=` query parameter.
pub fn validate_role_name(name: &str) -> Result<()> {
if name.is_empty() || name.len() > 63 {
return Err(Error::BadRequest(format!(
"Invalid data table role name '{name}': it must be between 1 and 63 characters"
)));
}
if !name
.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
{
return Err(Error::BadRequest(format!(
"Invalid data table role name '{name}': only letters, digits, '_' and '-' are allowed"
)));
}
if is_reserved_role_name(name) {
return Err(Error::BadRequest(format!(
"'{name}' is reserved and cannot be used as a data table role name"
)));
}
Ok(())
}
/// A double-quoted Postgres identifier. Doubling `"` is Postgres's own escaping inside one, so this
/// quotes any name — schema, table or role. Role names are validated as well
/// ([`validate_role_name`]) because they also travel unquoted, in `-- role <name>` and `?role=`.
pub fn quote_ident(name: &str) -> String {
format!("\"{}\"", name.replace('"', "\"\""))
}
/// Serialize the mutations that are not already serialized by the row itself.
///
/// A create is an insert and a delete is a delete, which Postgres orders for us — the unique index
/// on `name` is what makes two concurrent creates of the same name one winner and one error. What
/// still needs it is the window between the cluster DDL and the row: `CREATE ROLE` is not visible
/// to another transaction's `pg_roles` check until commit, so without this two creates of the same
/// name both pass their existence check and one fails on the index having already made the login.
/// Held for the transaction, so the DDL has to run on that same transaction to be covered.
pub async fn lock_role_catalog(tx: &mut sqlx::Transaction<'_, sqlx::Postgres>) -> Result<()> {
sqlx::query!("SELECT pg_advisory_xact_lock(hashtext('datatable_role_catalog'))")
.execute(&mut **tx)
.await?;
Ok(())
}
/// A replication stream reads every row whatever a data table's roles grant. Turning roles on looks
/// for streams holding this exclusive; whatever can start a Postgres trigger or capture streaming
/// holds it shared on the transaction that commits it. So either the look sees the stream, or the
/// stream's listener connects after roles are committed and refuses. Held for the transaction.
pub async fn lock_datatable_streams(conn: &mut sqlx::PgConnection, exclusive: bool) -> Result<()> {
let lock = if exclusive {
"pg_advisory_xact_lock"
} else {
"pg_advisory_xact_lock_shared"
};
sqlx::query(&format!("SELECT {lock}(hashtext('datatable_streams'))"))
.execute(conn)
.await?;
Ok(())
}
/// Whether an instance database is reached only through entries under roles is decided by two
/// writes that lock different workspaces' settings rows: turning roles on for one entry, and a
/// settings save pointing an entry without roles at the database. Each holds this for every
/// database it decides on, so neither reads past the other's uncommitted write. Held for the
/// transaction; the names are locked in sorted order so two holders cannot deadlock.
pub async fn lock_instance_databases_governance<'a>(
conn: &mut sqlx::PgConnection,
dbnames: impl IntoIterator<Item = &'a str>,
) -> Result<()> {
let dbnames: std::collections::BTreeSet<&str> = dbnames.into_iter().collect();
for dbname in dbnames {
sqlx::query("SELECT pg_advisory_xact_lock(hashtext('datatable_instance_database:' || $1))")
.bind(dbname)
.execute(&mut *conn)
.await?;
}
Ok(())
}
/// Disclosure: returns every role's stored Postgres password in plaintext. Any server path that
/// has to resolve or name a role may call it — including handlers open to a workspace member, who
/// need the names — but callers MUST NOT let `pwd` reach a response, a log line, an audit record
/// or an export. Nothing about who may call it: the credential is the whole risk, and `Debug` is
/// hand-written to redact it for the same reason.
pub async fn read_role_catalog(db: &DB) -> Result<DatatableRoleCatalog> {
crate::datatable_roles_oss::read_role_catalog(db).await
}
/// As [`read_role_catalog`], reading inside the caller's transaction so the value is the one
/// [`lock_role_catalog`] is protecting. Same disclosure contract.
pub async fn read_role_catalog_tx(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
) -> Result<DatatableRoleCatalog> {
crate::datatable_roles_oss::read_role_catalog_tx(tx).await
}
/// Record a role, in the caller's transaction so it commits with the `CREATE ROLE` it describes.
///
/// Authorization: writes a generated Postgres credential. Callers MUST restrict this to superadmin
/// paths and MUST hold [`lock_role_catalog`] on `tx`.
pub async fn insert_role_catalog_entry(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
id: &str,
role: &InstanceDatatableRole,
) -> Result<()> {
crate::datatable_roles_oss::insert_role_catalog_entry(tx, id, role).await
}
/// Update a role's recorded name, login flag and password. Same contract as
/// [`insert_role_catalog_entry`].
pub async fn update_role_catalog_entry(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
id: &str,
role: &InstanceDatatableRole,
) -> Result<()> {
crate::datatable_roles_oss::update_role_catalog_entry(tx, id, role).await
}
/// Forget a role. Same contract as [`insert_role_catalog_entry`]; run it in the transaction that
/// drops the cluster login, so the two cannot disagree.
pub async fn delete_role_catalog_entry(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
id: &str,
) -> Result<()> {
crate::datatable_roles_oss::delete_role_catalog_entry(tx, id).await
}
/// Resolve the role a caller named to its catalog id. A disabled role is an error rather than a
/// silent fallback: the caller asked for something the instance deliberately turned off.
pub fn role_id_by_name<'a>(catalog: &'a DatatableRoleCatalog, name: &str) -> Result<&'a str> {
let entry = catalog
.iter()
.find(|(_, role)| role.name == name)
.ok_or_else(|| {
Error::NotFound(format!(
"'{name}' is not a data table role of this instance. Defined roles: {}.",
catalog
.values()
.map(|r| r.name.as_str())
.collect::<Vec<_>>()
.join(", ")
))
})?;
if !entry.1.enabled {
return Err(Error::BadRequest(format!(
"Data table role '{name}' is disabled on this instance"
)));
}
Ok(entry.0.as_str())
}
/// Every instance database the registry knows about. Role provisioning has to reach all of them:
/// a role that cannot `CONNECT` to a database is refused by Postgres before any grant matters.
pub async fn registered_instance_databases(db: &DB) -> Result<Vec<String>> {
crate::datatable_roles_oss::registered_instance_databases(db).await
}
/// `CONNECT` on `dbname` for every enabled role, and none for `PUBLIC`. Run at role creation, at
/// database creation, and lazily whenever an instance data table is administered, so a database
/// provisioned before a role existed is repaired rather than left silently unreachable.
///
/// Authorization: rewrites a database's ACL with the server's own credentials and checks nothing.
/// Callers MUST have authorized administration of `dbname` — superadmin, or an admin of the
/// workspace governing a data table on it.
pub async fn converge_connect_grants(db: &DB, dbname: &str) -> Result<()> {
crate::datatable_roles_oss::converge_connect_grants(db, dbname).await
}
/// As [`converge_connect_grants`], with a catalog the caller already read. Same contract.
pub async fn converge_connect_grants_with(
db: &DB,
dbname: &str,
catalog: &DatatableRoleCatalog,
) -> Result<()> {
crate::datatable_roles_oss::converge_connect_grants_with(db, dbname, catalog).await
}
/// `CREATE ROLE <name> LOGIN PASSWORD ...; GRANT <name> TO custom_instance_user`, and `CONNECT` on
/// every registered database. No privileges beyond that — an admin grants them through SQL or the
/// ACL editor.
///
/// Authorization: creates a cluster-wide Postgres login. Callers MUST restrict this to superadmin
/// paths, and MUST hold [`lock_role_catalog`] on the same transaction.
pub async fn create_instance_role(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
name: &str,
password: &str,
) -> Result<()> {
crate::datatable_roles_oss::create_instance_role(tx, name, password).await
}
/// Authorization: alters a cluster-wide Postgres login. Callers MUST restrict this to superadmin
/// paths, and MUST hold [`lock_role_catalog`] on the same transaction.
pub async fn set_instance_role_login(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
name: &str,
enabled: bool,
) -> Result<()> {
crate::datatable_roles_oss::set_instance_role_login(tx, name, enabled).await
}
/// A rename discards an md5-hashed password, so the caller has to hand over a fresh one.
///
/// Authorization: renames a cluster-wide Postgres login. Callers MUST restrict this to superadmin
/// paths, and MUST hold [`lock_role_catalog`] on the same transaction.
pub async fn rename_instance_role(
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
from: &str,
to: &str,
password: &str,
) -> Result<()> {
crate::datatable_roles_oss::rename_instance_role(tx, from, to, password).await
}
/// A role owning anything in any database blocks its own `DROP ROLE`, and both its objects and the
/// privileges granted to it are only visible from inside each database — hence the pass over the
/// registry. An unreachable database aborts the whole delete: dropping the role while one database
/// still holds objects owned by it leaves those objects owned by a numeric OID nobody can name.
///
/// Each pass runs as the instance's own Postgres user rather than `custom_instance_user`, which
/// owns the databases and can therefore revoke a grant whoever made it. `custom_instance_user`
/// could only undo what it granted itself, so a privilege planted by an operator in psql — the
/// ordinary way privileges reach a role — would survive and block the drop.
///
/// Authorization: drops a cluster-wide Postgres login and reassigns everything it owns. Callers
/// MUST restrict this to superadmin paths, and MUST hold [`lock_role_catalog`] on `tx`.
///
/// The per-database passes open their own connections and cannot join `tx`; the lock is what keeps
/// a concurrent mutation out while they run. Only the final `DROP ROLE` is on `tx`, so it commits
/// or rolls back with the catalog write that forgets the role. Those passes commit as they go, so
/// callers MUST have disabled the role in an earlier committed transaction: a failure part-way
/// then leaves a disabled role to retry, not an enabled one already stripped in some databases.
pub async fn drop_instance_role(
db: &DB,
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
name: &str,
) -> Result<()> {
crate::datatable_roles_oss::drop_instance_role(db, tx, name).await
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn role_names_are_validated() {
assert!(validate_role_name("analytics").is_ok());
assert!(validate_role_name("read-only_2").is_ok());
assert!(validate_role_name("").is_err());
assert!(validate_role_name(&"a".repeat(64)).is_err());
assert!(validate_role_name("has space").is_err());
assert!(validate_role_name("quote\"injection").is_err());
// Reserved, case-insensitively.
assert!(validate_role_name("admin").is_err());
assert!(validate_role_name("Postgres").is_err());
assert!(validate_role_name("pg_read_all_data").is_err());
assert!(validate_role_name("windmill_user").is_err());
assert!(validate_role_name("custom_instance_user").is_err());
}
#[test]
fn a_disabled_role_is_an_error_not_a_fallback() {
let mut catalog = DatatableRoleCatalog::new();
catalog.insert(
"id1".to_string(),
InstanceDatatableRole {
name: "analytics".to_string(),
enabled: false,
pwd: Some("x".to_string()),
},
);
assert!(role_id_by_name(&catalog, "analytics").is_err());
assert!(role_id_by_name(&catalog, "nope").is_err());
catalog.get_mut("id1").unwrap().enabled = true;
assert_eq!(role_id_by_name(&catalog, "analytics").unwrap(), "id1");
}
}
@@ -0,0 +1,209 @@
/*
* 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 data table roles come from: the enterprise implementation, or a refusal.
//!
//! Roles are an Enterprise Edition feature. An edition without them creates, grants and connects
//! as none, and a data table saved under roles — by an enterprise build, before a downgrade — is
//! refused rather than resolved as `admin`. A data table not under roles, asked for no role,
//! resolves as it always has. `private` alone is not that edition: community builds carry it.
use crate::error::Error;
/// 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())
}
#[cfg(all(feature = "private", feature = "enterprise"))]
pub(crate) use crate::datatable_roles_ee::{
can_use_datatable_role, can_use_datatable_role_in_governing_workspace, converge_connect_grants,
converge_connect_grants_with, create_instance_role, delete_role_catalog_entry,
drop_instance_role, ensure_can_use_datatable_role, ensure_datatable_admin_access,
ensure_instance_db_grant_options_unchecked, forget_datatable_role_everywhere,
insert_role_catalog_entry, read_role_catalog, read_role_catalog_tx,
registered_instance_databases, rename_instance_role, resolve_datatable_role_connection,
set_instance_role_login, update_role_catalog_entry,
};
#[cfg(not(all(feature = "private", feature = "enterprise")))]
pub(crate) use ce::*;
#[cfg(not(all(feature = "private", feature = "enterprise")))]
mod ce {
use super::datatable_roles_unavailable as unavailable;
use crate::{
datatable_roles::{DatatableRoleCatalog, InstanceDatatableRole},
db::AuthedRef,
error::Result,
workspaces::{
resolve_governing_datatable, DataTableRoleTenants, DatatableAccess, GoverningDatatable,
},
DB,
};
type Tx<'a> = sqlx::Transaction<'a, sqlx::Postgres>;
pub(crate) async fn read_role_catalog(_db: &DB) -> Result<DatatableRoleCatalog> {
Err(unavailable())
}
pub(crate) async fn read_role_catalog_tx(_tx: &mut Tx<'_>) -> Result<DatatableRoleCatalog> {
Err(unavailable())
}
pub(crate) async fn insert_role_catalog_entry(
_tx: &mut Tx<'_>,
_id: &str,
_role: &InstanceDatatableRole,
) -> Result<()> {
Err(unavailable())
}
pub(crate) async fn update_role_catalog_entry(
_tx: &mut Tx<'_>,
_id: &str,
_role: &InstanceDatatableRole,
) -> Result<()> {
Err(unavailable())
}
pub(crate) async fn delete_role_catalog_entry(_tx: &mut Tx<'_>, _id: &str) -> Result<()> {
Err(unavailable())
}
pub(crate) async fn registered_instance_databases(_db: &DB) -> Result<Vec<String>> {
Err(unavailable())
}
/// Nothing to converge: with no roles to admit, an instance database keeps the `CONNECT`
/// grants it was created with, `PUBLIC`'s included, as it did before roles existed.
pub(crate) async fn converge_connect_grants(_db: &DB, _dbname: &str) -> Result<()> {
Ok(())
}
/// As [`converge_connect_grants`].
pub(crate) async fn converge_connect_grants_with(
_db: &DB,
_dbname: &str,
_catalog: &DatatableRoleCatalog,
) -> Result<()> {
Ok(())
}
pub(crate) async fn create_instance_role(
_tx: &mut Tx<'_>,
_name: &str,
_password: &str,
) -> Result<()> {
Err(unavailable())
}
pub(crate) async fn set_instance_role_login(
_tx: &mut Tx<'_>,
_name: &str,
_enabled: bool,
) -> Result<()> {
Err(unavailable())
}
pub(crate) async fn rename_instance_role(
_tx: &mut Tx<'_>,
_from: &str,
_to: &str,
_password: &str,
) -> Result<()> {
Err(unavailable())
}
pub(crate) async fn drop_instance_role(_db: &DB, _tx: &mut Tx<'_>, _name: &str) -> Result<()> {
Err(unavailable())
}
pub(crate) async fn ensure_instance_db_grant_options_unchecked(
_db: &DB,
_dbname: &str,
) -> Result<()> {
Err(unavailable())
}
/// No tenant list covers anyone: there is no role to connect as.
pub(crate) fn can_use_datatable_role(
_tenants: &DataTableRoleTenants,
_authed: &AuthedRef<'_>,
) -> bool {
false
}
pub(crate) async fn can_use_datatable_role_in_governing_workspace(
_db: &DB,
_governing_w_id: &str,
_w_id: &str,
_tenants: &DataTableRoleTenants,
_access: &DatatableAccess<'_>,
) -> Result<bool> {
Err(unavailable())
}
/// Reached only for a data table under roles or a caller naming a role: both are refused.
pub(crate) async fn resolve_datatable_role_connection(
_db: &DB,
_w_id: &str,
_name: &str,
_governing: &GoverningDatatable,
_db_resource: serde_json::Value,
_role: Option<&str>,
_access: DatatableAccess<'_>,
) -> Result<serde_json::Value> {
Err(unavailable())
}
/// A data table not under roles, asked for no role or for `admin`, is not a role decision and
/// passes, as it did before roles existed. Anything else is refused.
pub(crate) async fn ensure_can_use_datatable_role(
db: &DB,
w_id: &str,
name: &str,
role: Option<&str>,
_access: &DatatableAccess<'_>,
_context: &str,
) -> Result<()> {
let governing = resolve_governing_datatable(db, w_id, name).await?;
if governing.datatable.permissions.is_none()
&& role.is_none_or(|r| r == crate::datatable_roles::ADMIN_DATATABLE_ROLE)
{
Ok(())
} else {
Err(unavailable())
}
}
/// A data table not under roles is the `admin` connection for anyone who reaches it, as before
/// roles existed. One under roles is refused.
pub(crate) async fn ensure_datatable_admin_access(
db: &DB,
w_id: &str,
name: &str,
_access: &DatatableAccess<'_>,
) -> Result<()> {
let governing = resolve_governing_datatable(db, w_id, name).await?;
if governing.datatable.permissions.is_none() {
Ok(())
} else {
Err(unavailable())
}
}
pub(crate) async fn forget_datatable_role_everywhere(
_tx: &mut Tx<'_>,
_role_id: &str,
) -> Result<()> {
Err(unavailable())
}
}

Some files were not shown because too many files have changed in this diff Show More