diff --git a/AGENTS.md b/AGENTS.md index fd4b60a64b..a57215cde5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,6 +33,9 @@ Open-source platform for internal tools, workflows, API integrations, background administers behind `external_instance` data tables and Ducklake catalogs: its invariants (one lifecycle lock, managed-object markers, the setup gate, per-cluster roles, fork copy ownership) and how to run one locally +- **Data table viewer roles**: `docs/datatable-viewer-roles.md`, on what a connection naming no + role holds: the default role as a floor, where the viewer comes from, why an admin's roles are + not the viewer's, the per-role-set logins and the order their rows and logins are written in - **Enterprise**: `docs/enterprise.md` — EE file conventions and PR workflow - **Operator write rights**: `docs/operator-write-rights.md` — which `operator_settings` flags are enforced rather than cosmetic, and why a right that is granted-unless-withdrawn needs `Option` diff --git a/ai_evals/README.md b/ai_evals/README.md index 44870ba2a2..9b8850c435 100644 --- a/ai_evals/README.md +++ b/ai_evals/README.md @@ -196,7 +196,10 @@ not have read is itself the failure — e.g. `write_variable.value` on a secret Global (and flow) initial fixtures can seed `workspace.datatables` so the `list_datatables`, `get_datatable_table_schema`, and `exec_datatable_sql` tools return seeded data during evals. Each entry is -`{ datatable_name, schemas: { : { : { columns, rows? } } } }`. +`{ datatable_name, schemas: { : {
: { columns, rows? } } } }`, +plus `migrations_enabled: true` and `migrations: [{ timestamp, name, code_up, code_down?, applied? }]` +for a datatable that keeps its schema as migrations, which the migration tools then +read and write in memory. SQL runs through a small in-memory engine (`datatableSqlEngine.ts`), not a real database. Writes are **stateful within a case**: `CREATE`/`DROP`/`INSERT`/`UPDATE`/ `DELETE` mutate the seeded datatable in place, so a later `list_datatables`, diff --git a/ai_evals/adapters/frontend/datatableSqlEngine.ts b/ai_evals/adapters/frontend/datatableSqlEngine.ts index 453eca286e..96a62f6804 100644 --- a/ai_evals/adapters/frontend/datatableSqlEngine.ts +++ b/ai_evals/adapters/frontend/datatableSqlEngine.ts @@ -30,6 +30,17 @@ export interface BenchmarkDatatableSeed { [table: string]: BenchmarkDatatableTableSeed } } + /** Whether the datatable keeps its schema as migrations. */ + migrations_enabled?: boolean + migrations?: BenchmarkDatatableMigrationSeed[] +} + +export interface BenchmarkDatatableMigrationSeed { + timestamp: number + name: string + code_up: string + code_down?: string + applied?: boolean } export interface DatatableSqlResult { diff --git a/ai_evals/adapters/frontend/mockBackend.ts b/ai_evals/adapters/frontend/mockBackend.ts index 2f47e85be6..d8284039b9 100644 --- a/ai_evals/adapters/frontend/mockBackend.ts +++ b/ai_evals/adapters/frontend/mockBackend.ts @@ -692,10 +692,138 @@ export function listBenchmarkDatatables(workspace: string): DataTableTables[] | datatable_name: datatable.datatable_name, schemas: Object.fromEntries( Object.entries(datatable.schemas).map(([schema, tables]) => [schema, Object.keys(tables)]) - ) + ), + ...(datatable.migrations_enabled && { migrations_enabled: true }) })) } +// ============= Datatable migrations ============= + +function benchmarkDatatable(workspace: string, datatableName: string): BenchmarkDatatableSeed { + const datatable = (benchmarkWorkspaceRunnables.get(workspace)?.datatables ?? []).find( + (entry) => entry.datatable_name === datatableName + ) + if (!datatable) { + throw new Error(`datatable "${datatableName}" not found`) + } + return datatable +} + +/** Run a migration body through the in-memory engine, one statement at a time and without the + * transaction frame, so the tables it creates show up in later reads. */ +function applyBenchmarkMigrationSql(datatable: BenchmarkDatatableSeed, sql: string): void { + // Split on the semicolons outside single-quoted literals. + for (const statement of sql.match(/(?:'(?:[^']|'')*'|[^;'])+/g) ?? []) { + const trimmed = statement.replace(/^\s*--.*$/gm, '').trim() + if (trimmed !== '' && !/^(begin|end|commit)$/i.test(trimmed)) { + applyDatatableSql(datatable, trimmed) + } + } +} + +export function getBenchmarkDatatableMigrationsStatus(input: { + workspace: string + datatableName: string +}) { + const datatable = benchmarkDatatable(input.workspace, input.datatableName) + if (!datatable.migrations_enabled) { + return { enabled: false, migrations: [] } + } + return { + enabled: true, + migrations: (datatable.migrations ?? []).map(({ applied, ...migration }) => ({ + ...migration, + status: applied ? ('ran' as const) : ('not_run' as const) + })) + } +} + +export function createBenchmarkDatatableMigration(input: { + workspace: string + datatableName: string + requestBody: { name: string; code_up: string; code_down?: string } +}) { + const datatable = benchmarkDatatable(input.workspace, input.datatableName) + if (!datatable.migrations_enabled) { + throw new Error( + `Migrations are not enabled for data table '${input.datatableName}'. Enable them first.` + ) + } + const migrations = (datatable.migrations ??= []) + const timestamp = Math.max(20260101000000, ...migrations.map((m) => m.timestamp)) + 1 + migrations.push({ timestamp, ...input.requestBody }) + return { datatable: input.datatableName, timestamp, ...input.requestBody } +} + +export function upsertBenchmarkDatatableMigration(input: { + workspace: string + datatableName: string + requestBody: { timestamp: number; name: string; code_up: string; code_down?: string } +}): string { + const datatable = benchmarkDatatable(input.workspace, input.datatableName) + const migrations = (datatable.migrations ??= []) + const existing = migrations.find((m) => m.timestamp === input.requestBody.timestamp) + if (existing?.applied) { + throw new Error( + `Migration ${existing.timestamp} on data table '${input.datatableName}' has already been applied and cannot be modified` + ) + } + if (existing) { + // Replaced whole, as the server does: a down left out is stored as none. + delete existing.code_down + Object.assign(existing, input.requestBody) + } else { + migrations.push({ ...input.requestBody }) + migrations.sort((a, b) => a.timestamp - b.timestamp) + } + return `Upserted migration ${input.requestBody.timestamp} in ${input.datatableName}` +} + +export function runBenchmarkDatatableMigrations(input: { + workspace: string + datatableName: string + upTo?: number + only?: number +}) { + const datatable = benchmarkDatatable(input.workspace, input.datatableName) + const applied: { version: number; name: string }[] = [] + for (const migration of datatable.migrations ?? []) { + if ( + migration.applied || + (input.only !== undefined && migration.timestamp !== input.only) || + (input.only === undefined && input.upTo !== undefined && migration.timestamp > input.upTo) + ) { + continue + } + applyBenchmarkMigrationSql(datatable, migration.code_up) + migration.applied = true + applied.push({ version: migration.timestamp, name: migration.name }) + } + return { applied } +} + +export function rollbackBenchmarkDatatableMigration(input: { + workspace: string + datatableName: string + only?: number +}) { + const datatable = benchmarkDatatable(input.workspace, input.datatableName) + const last = (datatable.migrations ?? []) + .filter((m) => m.applied && (input.only === undefined || m.timestamp === input.only)) + .at(-1) + if (!last) { + return { rolled_back: [] } + } + if (last.code_down === undefined) { + throw new Error( + `Cannot roll back migration ${last.timestamp} (${last.name}): it has no down migration` + ) + } + applyBenchmarkMigrationSql(datatable, last.code_down) + last.applied = false + return { rolled_back: [{ version: last.timestamp, name: last.name }] } +} + // ============= DuckLake catalogs and declared metrics ============= /** Seeded DuckLake names, or `null` for a non-benchmark workspace. */ diff --git a/ai_evals/adapters/frontend/vitestAdapter.test.ts b/ai_evals/adapters/frontend/vitestAdapter.test.ts index 288c67e3eb..cb7469b45e 100644 --- a/ai_evals/adapters/frontend/vitestAdapter.test.ts +++ b/ai_evals/adapters/frontend/vitestAdapter.test.ts @@ -92,6 +92,11 @@ vi.mock('$lib/gen', async () => { getBenchmarkCompletedJob, getBenchmarkCompletedJobResultMaybe, getBenchmarkDatatableSchema, + getBenchmarkDatatableMigrationsStatus, + createBenchmarkDatatableMigration, + upsertBenchmarkDatatableMigration, + runBenchmarkDatatableMigrations, + rollbackBenchmarkDatatableMigration, getBenchmarkDraftForUser, getBenchmarkFlowByPath, getBenchmarkFlowAllResults, @@ -400,7 +405,27 @@ vi.mock('$lib/gen', async () => { schemaName: data.schemaName, tableName: data.tableName }) - : actual.WorkspaceService.getDataTableTableSchema(data) + : actual.WorkspaceService.getDataTableTableSchema(data), + getDatatableMigrationsStatus: async (data: any) => + hasBenchmarkWorkspace(data.workspace) + ? getBenchmarkDatatableMigrationsStatus(data) + : actual.WorkspaceService.getDatatableMigrationsStatus(data), + createDatatableMigration: async (data: any) => + hasBenchmarkWorkspace(data.workspace) + ? createBenchmarkDatatableMigration(data) + : actual.WorkspaceService.createDatatableMigration(data), + upsertDatatableMigration: async (data: any) => + hasBenchmarkWorkspace(data.workspace) + ? upsertBenchmarkDatatableMigration(data) + : actual.WorkspaceService.upsertDatatableMigration(data), + runDatatableMigrations: async (data: any) => + hasBenchmarkWorkspace(data.workspace) + ? runBenchmarkDatatableMigrations(data) + : actual.WorkspaceService.runDatatableMigrations(data), + rollbackDatatableMigrations: async (data: any) => + hasBenchmarkWorkspace(data.workspace) + ? rollbackBenchmarkDatatableMigration(data) + : actual.WorkspaceService.rollbackDatatableMigrations(data) }), DataMetricService: wrapService(actual.DataMetricService, { listDataMetrics: async (data: { workspace: string }) => diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index b66af3e8dd..4b6704b229 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -901,6 +901,57 @@ - treats creating the table as a normal SQL statement and does not claim a separate registration step is needed - does not write a script to create the table +# The same request on a data table that keeps its schema as migrations: the table is +# created by a migration, since plain SQL would leave it out of the history. +- id: global-datatable-schema-change-uses-migration + prompt: |- + Add a new table called refunds to our workspace data table, with an order id and a refund amount. + initial: ai_evals/fixtures/frontend/global/initial/datatable_orders_migrations_seed.json + runtime: + maxTurns: 8 + validate: + draftCountExactly: 0 + toolExpect: + requiredToolsUsed: + - create_datatable_migration + forbiddenToolsUsed: + - write_script + - deploy_workspace_item + - delete_workspace_item + toolCallArgs: + - tool: create_datatable_migration + field: code_up + stringIncludesAnyOf: + - create table + skipJudge: true + judgeChecklist: + - notices that the data table keeps its schema as migrations + - creates the refunds table through a migration rather than a plain CREATE TABLE statement + - includes an order id and a refund amount column + - gives the migration a down that drops the table + +# Row data is not schema: it is written with plain SQL even where migrations are enabled. +- id: global-datatable-row-change-skips-migration + prompt: |- + Mark order number 2 as shipped in our workspace data table. + initial: ai_evals/fixtures/frontend/global/initial/datatable_orders_migrations_seed.json + runtime: + maxTurns: 12 + validate: + draftCountExactly: 0 + toolExpect: + requiredToolsUsed: + - exec_datatable_sql + forbiddenToolsUsed: + - create_datatable_migration + - write_script + - deploy_workspace_item + - delete_workspace_item + skipJudge: true + judgeChecklist: + - runs an UPDATE on the orders table setting the status of order id 2 to shipped + - does not create a migration for a change to row data + - id: global-test25-datatable-mutate-rows prompt: |- Mark order number 2 as shipped in our workspace data table. diff --git a/ai_evals/fixtures/frontend/global/initial/datatable_orders_migrations_seed.json b/ai_evals/fixtures/frontend/global/initial/datatable_orders_migrations_seed.json new file mode 100644 index 0000000000..d5227d9af8 --- /dev/null +++ b/ai_evals/fixtures/frontend/global/initial/datatable_orders_migrations_seed.json @@ -0,0 +1,36 @@ +{ + "workspace": { + "datatables": [ + { + "datatable_name": "main", + "migrations_enabled": true, + "migrations": [ + { + "timestamp": 20260501090000, + "name": "create_orders", + "code_up": "BEGIN;\n\nCREATE TABLE orders (id serial PRIMARY KEY, customer_id int, total numeric, status text, created_at timestamptz DEFAULT now());\n\nEND;", + "code_down": "BEGIN;\n\nDROP TABLE orders;\n\nEND;", + "applied": true + } + ], + "schemas": { + "public": { + "orders": { + "columns": { + "id": "int4", + "customer_id": "int4", + "total": "numeric", + "status": "text", + "created_at": "timestamptz" + }, + "rows": [ + { "id": 1, "customer_id": 1, "total": 42.5, "status": "shipped", "created_at": "2026-05-01T10:00:00Z" }, + { "id": 2, "customer_id": 2, "total": 19.99, "status": "pending", "created_at": "2026-05-02T11:30:00Z" } + ] + } + } + } + } + ] + } +} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 4c3f6abd98..695f6d248b 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -1e538620a9b2f08b044997d032e71d8720ecbd5c +40ab35596e6fcb82a4dfc56e186befb807f8e2af diff --git a/backend/migrations/20261012180101_datatable_role_set.down.sql b/backend/migrations/20261012180101_datatable_role_set.down.sql new file mode 100644 index 0000000000..15384edae8 --- /dev/null +++ b/backend/migrations/20261012180101_datatable_role_set.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS datatable_role_set; diff --git a/backend/migrations/20261012180101_datatable_role_set.up.sql b/backend/migrations/20261012180101_datatable_role_set.up.sql new file mode 100644 index 0000000000..0093837d3b --- /dev/null +++ b/backend/migrations/20261012180101_datatable_role_set.up.sql @@ -0,0 +1,21 @@ +-- The logins behind a data table connection made as several roles at once: one Postgres login per +-- distinct set of data table roles, a member of each. Which roles a login holds is its +-- memberships on the cluster, and its name is derived from their catalog ids, so a row only has to +-- carry the credential. +-- +-- A row is written after its login exists and removed before the login is dropped, so a row always +-- names a login that can authenticate with `pwd`. +CREATE TABLE datatable_role_set ( + cluster VARCHAR(20) NOT NULL CHECK (cluster IN ('instance', 'external_instance')), + name VARCHAR(63) NOT NULL, + -- Generated by Windmill, never entered by anyone, and never leaves the server. + pwd TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + -- Recorded at most once a day. A login unused for long enough is dropped, and made again if + -- its set of roles is asked for later. + last_used_at TIMESTAMPTZ NOT NULL DEFAULT now(), + PRIMARY KEY (cluster, name) +); + +GRANT ALL ON datatable_role_set TO windmill_user; +GRANT ALL ON datatable_role_set TO windmill_admin; diff --git a/backend/parsers/windmill-parser-wasm/Cargo.lock b/backend/parsers/windmill-parser-wasm/Cargo.lock index 75c10528df..51878c6dfb 100644 --- a/backend/parsers/windmill-parser-wasm/Cargo.lock +++ b/backend/parsers/windmill-parser-wasm/Cargo.lock @@ -2,21 +2,22 @@ # It is not intended for manual editing. version = 4 -[[package]] -name = "addr2line" -version = "0.25.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b5d307320b3181d6d7954e663bd7c774a838b8220fe0593c86d9fb09f498b4b" -dependencies = [ - "gimli", -] - [[package]] name = "adler2" version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" +[[package]] +name = "aead" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d122413f284cf2d62fb1b7db97e02edb8cda96d769b16e443a4f6195e35662b0" +dependencies = [ + "crypto-common 0.1.7", + "generic-array", +] + [[package]] name = "aes" version = "0.7.5" @@ -24,11 +25,36 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9e8b47f52ea9bae42228d07ec09eb676433d7c4ed1ebdf0f1d1c29ed446f1ab8" dependencies = [ "cfg-if", - "cipher", + "cipher 0.3.0", "cpufeatures 0.2.17", "opaque-debug", ] +[[package]] +name = "aes" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b169f7a6d4742236a0a00c541b845991d0ac43e546831af1249753ab4c3aa3a0" +dependencies = [ + "cfg-if", + "cipher 0.4.4", + "cpufeatures 0.2.17", +] + +[[package]] +name = "aes-gcm" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "831010a0f742e1209b3bcea8fab6a8e149051ba6099432c8cb2cc117dec3ead1" +dependencies = [ + "aead", + "aes 0.8.4", + "cipher 0.4.4", + "ctr", + "ghash", + "subtle", +] + [[package]] name = "ahash" version = "0.7.8" @@ -389,21 +415,6 @@ dependencies = [ "tokio", ] -[[package]] -name = "backtrace" -version = "0.3.76" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb531853791a215d7c62a30daf0dde835f381ab5de4589cfe7c649d2cbe92bd6" -dependencies = [ - "addr2line", - "cfg-if", - "libc", - "miniz_oxide", - "object", - "rustc-demangle", - "windows-link", -] - [[package]] name = "base64" version = "0.21.7" @@ -539,7 +550,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2cb03d1bed155d89dce0f845b7899b18a9a163e148fd004e1c28421a783e2d8e" dependencies = [ "block-padding", - "cipher", + "cipher 0.3.0", ] [[package]] @@ -763,6 +774,16 @@ dependencies = [ "generic-array", ] +[[package]] +name = "cipher" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "773f3b9af64447d2ce9850330c473515014aa235e6a783b02db81ff39e4a3dad" +dependencies = [ + "crypto-common 0.1.7", + "inout", +] + [[package]] name = "clang-sys" version = "1.8.1" @@ -1095,6 +1116,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78c8292055d1c1df0cce5d180393dc8cce0abec0a7102adb6c7b1eef6016d60a" dependencies = [ "generic-array", + "rand_core 0.6.4", "typenum", ] @@ -1107,6 +1129,15 @@ dependencies = [ "hybrid-array", ] +[[package]] +name = "ctr" +version = "0.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0369ee1ad671834580515889b80f2ea915f23b8be8d0daa4bbaf2ac5c7590835" +dependencies = [ + "cipher 0.4.4", +] + [[package]] name = "ctutils" version = "0.4.2" @@ -1163,7 +1194,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac41dd49fb554432020d52c875fc290e110113f864c6b1b525cd62c7e7747a5d" dependencies = [ "byteorder", - "cipher", + "cipher 0.3.0", "opaque-debug", ] @@ -1636,10 +1667,14 @@ dependencies = [ ] [[package]] -name = "gimli" -version = "0.32.3" +name = "ghash" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e629b9b98ef3dd8afe6ca2bd0f89306cec16d43d907889945bc5d6687f2f13c7" +checksum = "f0d8a4362ccb29cb0b265253fb0a2728f592895ee6854fd9bc13f2ffda266ff1" +dependencies = [ + "opaque-debug", + "polyval", +] [[package]] name = "git-version" @@ -2101,6 +2136,15 @@ dependencies = [ "serde_core", ] +[[package]] +name = "inout" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "879f10e63c20629ecabbb64a8010319738c66a5cd0c29b02d63d272b03751d01" +dependencies = [ + "generic-array", +] + [[package]] name = "interpolator" version = "0.5.0" @@ -2116,17 +2160,6 @@ dependencies = [ "rustversion", ] -[[package]] -name = "io-uring" -version = "0.7.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4d09b98f7eace8982db770e4408e7470b028ce513ac28fecdc6bf4c30fe92b62" -dependencies = [ - "bitflags", - "cfg-if", - "libc", -] - [[package]] name = "ipnet" version = "2.12.0" @@ -2455,7 +2488,7 @@ version = "3.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c42f95f9d296f2dcb50665f507ed5a68a171453142663ce44d77a4eb217b053" dependencies = [ - "aes", + "aes 0.7.5", "base64 0.21.7", "block-modes", "crc-any", @@ -2619,9 +2652,9 @@ dependencies = [ [[package]] name = "mio" -version = "1.1.1" +version = "1.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a69bcab0ad47271a0234d9422b131806bf3968021e5dc9328caf2d4cd58557fc" +checksum = "1788edb87fdc09c7e26304471e2f5be8cdefb1b6930d6e3985fc02ff53bf86ee" dependencies = [ "libc", "wasi 0.11.1+wasi-snapshot-preview1", @@ -3220,6 +3253,18 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4596b6d070b27117e987119b4dac604f3c58cfb0b191112e24771b2faeac1a6" +[[package]] +name = "polyval" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d1fe60d06143b2430aa532c94cfe9e29783047f06c0d7fd359a9a51b729fa25" +dependencies = [ + "cfg-if", + "cpufeatures 0.2.17", + "opaque-debug", + "universal-hash", +] + [[package]] name = "postgres-native-tls" version = "0.5.1" @@ -4031,12 +4076,6 @@ dependencies = [ "serde_json", ] -[[package]] -name = "rustc-demangle" -version = "0.1.27" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b50b8869d9fc858ce7266cce0194bd74df58b9d0e3f6df3a9fc8eb470d95c09d" - [[package]] name = "rustc-hash" version = "2.1.1" @@ -5350,34 +5389,31 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.47.1" +version = "1.53.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89e49afdadebb872d3145a5638b59eb0691ea23e46ca484037cfab3b76b95038" +checksum = "e95f91fcc7a621e8b030f6aa23c71fe9838ae2fb4d8118b75602a328f5144044" dependencies = [ - "backtrace", "bytes", - "io-uring", "libc", "mio", "parking_lot", "pin-project-lite", "signal-hook-registry", - "slab", "socket2", "tokio-macros", "tracing", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] name = "tokio-macros" -version = "2.5.0" +version = "2.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e06d43f1345a3bcd39f6a56dbb7dcab2ba47e68e8ac134855e7e2bdbaf8cab8" +checksum = "78773a2a397f451582ce068015985c33193cf6dea8b74d2a639fe457b2f07b0e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.117", + "syn 3.0.7", ] [[package]] @@ -5875,6 +5911,16 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853" +[[package]] +name = "universal-hash" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc1de2c688dc15305988b563c3854064043356019f97a4b46276fe734c4f07ea" +dependencies = [ + "crypto-common 0.1.7", + "subtle", +] + [[package]] name = "unscanny" version = "0.1.0" @@ -6296,6 +6342,7 @@ checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" name = "windmill-common" version = "1.830.0" dependencies = [ + "aes-gcm", "aho-corasick", "anyhow", "arc-swap", @@ -6323,6 +6370,7 @@ dependencies = [ "git-version", "globset", "hex", + "hkdf", "hmac 0.12.1", "hyper", "indexmap", diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 24f9d8221c..23ab2eaed1 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -4449,6 +4449,24 @@ pub async fn monitor_db( } }; + // Drop the data table role set logins nobody has connected with for a month. Every ~40 min, + // spawned for the same reason as the credential maintenance above: it opens a connection per + // managed database for each login it drops. + let datatable_role_set_sweep_f = async { + if server_mode && iteration.is_some() && iteration.as_ref().unwrap().should_run(240) { + if let Some(db) = conn.as_sql() { + let db = db.clone(); + tokio::spawn(async move { + if let Err(e) = + windmill_common::datatable_roles::sweep_unused_role_set_logins(&db).await + { + tracing::error!("Error sweeping unused data table role set logins: {e}"); + } + }); + } + } + }; + // Re-encrypt the legacy-format secret variables once new values are written in AES-256-GCM. // Every ~5 min until one pass has gone through every workspace; spawned so a large instance // does not hold up the join below. @@ -4538,6 +4556,7 @@ pub async fn monitor_db( cleanup_scheduled_job_deletions_f, git_auto_pull_f, git_credential_maintenance_f, + datatable_role_set_sweep_f, ai_session_retention_f, legacy_secret_upgrade_f, pipeline_freshness_watchdog_f, diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs index 9800d97841..023d9f20ed 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_roles.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -1847,3 +1847,222 @@ async fn cleanup_waits_out_a_save_racing_it_for_an_instance_database( assert_eq!(users, vec!["test-workspace".to_string()]); Ok(()) } + +#[cfg(all(feature = "private", feature = "enterprise"))] +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_connection_naming_no_role_holds_the_default_one_and_the_viewers( + db: Pool, +) -> anyhow::Result<()> { + use windmill_common::workspaces::{get_datatable_resource_from_db, DatatableAccess}; + + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let suffix: String = uuid::Uuid::new_v4().simple().to_string()[..8].to_string(); + let names = [ + format!("wmtest_floor_{suffix}"), + format!("wmtest_emea_{suffix}"), + format!("wmtest_amer_{suffix}"), + ]; + let roles_url = format!("http://localhost:{port}/api/settings/datatable_roles"); + const MEMBER_LOGINS: &str = "SELECT m.rolname::text FROM pg_auth_members am + JOIN pg_roles r ON r.oid = am.roleid JOIN pg_roles m ON m.oid = am.member + WHERE r.rolname = $1 AND m.rolname LIKE 'wm\\_set\\_%'"; + + // The job is the app publisher's in every case: only the viewer the server recorded differs. + let connect = |role: Option<&'static str>, viewer: Option<&'static str>| { + get_datatable_resource_from_db( + &db, + "test-workspace", + "main", + role, + DatatableAccess::PermissionedAs { + permissioned_as: "u/test-user", + email: "test@windmill.dev", + end_user_email: viewer, + }, + ) + }; + let set_permissions = |default_role: String, roles: Value| { + sqlx::query( + "UPDATE workspace_settings + SET datatable = jsonb_set(datatable, '{datatables,main,permissions}', $1) + WHERE workspace_id = 'test-workspace'", + ) + .bind(json!({ "default_role": default_role, "roles": roles })) + .execute(&db) + }; + + let outcome = async { + let mut ids = vec![]; + for name in &names { + let created: Value = authed(client().post(&roles_url), "SECRET_TOKEN") + .json(&json!({ "name": name })) + .send() + .await? + .error_for_status()? + .json() + .await?; + ids.push(created["id"].as_str().unwrap().to_string()); + } + // `test-user-2` is named by both regional roles, one of them through a group, and by + // `admin`; `test-user-3` by one; the workspace admin `test-user` by neither. + let roles = json!({ + "admin": {"tenants": ["u/test-user-2"]}, + (ids[0].as_str()): {"tenants": ["*"]}, + (ids[1].as_str()): {"tenants": ["u/test-user-2", "u/test-user-3"]}, + (ids[2].as_str()): {"tenants": ["g/analysts"]}, + }); + set_permissions(ids[0].clone(), roles.clone()).await?; + sqlx::query( + "INSERT INTO usr_to_group (workspace_id, usr, group_) + VALUES ('test-workspace', 'test-user-2', 'analysts')", + ) + .execute(&db) + .await?; + let member_of = |login: String| async { + let mut roles: Vec = sqlx::query_scalar( + "SELECT r.rolname::text FROM pg_auth_members am + JOIN pg_roles r ON r.oid = am.roleid JOIN pg_roles m ON m.oid = am.member + WHERE m.rolname = $1", + ) + .bind(login) + .fetch_all(&db) + .await?; + roles.sort(); + Ok::<_, anyhow::Error>(roles) + }; + + let all = connect(None, Some("test2@windmill.dev")).await?; + let login = all["user"].as_str().unwrap().to_string(); + assert!(login.starts_with("wm_set_"), "{login}"); + let mut expected = names.to_vec(); + expected.sort(); + assert_eq!( + member_of(login.clone()).await?, + expected, + "the login holds the default role and the viewer's, and not `admin`" + ); + assert_eq!(connect(None, Some("test2@windmill.dev")).await?, all); + + let two = connect(None, Some("test3@windmill.dev")).await?; + let mut expected = names[..2].to_vec(); + expected.sort(); + assert_eq!( + member_of(two["user"].as_str().unwrap().to_string()).await?, + expected + ); + + // An admin may use every role, the one owning the tables included. As a viewer they add + // what names them, which here is nothing; nor does a job with no viewer recorded. + for viewer in [Some("test@windmill.dev"), None] { + assert_eq!(connect(None, viewer).await?["user"], names[0].as_str()); + } + // A named role is that role alone, whoever is viewing. + let named: &'static str = Box::leak(names[1].clone().into_boxed_str()); + assert_eq!( + connect(Some(named), Some("test2@windmill.dev")).await?["user"], + names[1].as_str() + ); + // A default of `admin` is the connection it always was: the viewer's roles are not added. + set_permissions("admin".to_string(), roles.clone()).await?; + let admin = connect(None, Some("test2@windmill.dev")).await?; + assert_eq!(admin, connect(Some("admin"), None).await?); + assert!(!admin["user"].as_str().unwrap().starts_with("wm_")); + + // With no default role the viewer's roles are all there is, the one open to everyone + // included, and a connection left with none is refused rather than resolved as anything. + set_permissions("$none".to_string(), roles.clone()).await?; + assert_eq!( + connect(None, Some("test3@windmill.dev")).await?, + two, + "no default changes nothing for a viewer the roles name" + ); + assert!(connect(None, None).await.is_err()); + // The same holds for a default the job's own identity is not a tenant of. + let mut closed = roles.clone(); + closed[ids[0].as_str()] = json!({"tenants": []}); + set_permissions(ids[0].clone(), closed).await?; + let as_member = |viewer: Option<&'static str>| { + get_datatable_resource_from_db( + &db, + "test-workspace", + "main", + None, + DatatableAccess::PermissionedAs { + permissioned_as: "u/test-user-3", + email: "test3@windmill.dev", + end_user_email: viewer, + }, + ) + }; + assert_eq!( + as_member(Some("test3@windmill.dev")).await?["user"], + names[1].as_str() + ); + assert!(as_member(None).await.is_err()); + set_permissions(ids[0].clone(), roles).await?; + + // A set login nobody connects with any more is dropped, a recent one is kept, and a + // dropped one is made again for the next viewer who needs it. + let exists = || async { + sqlx::query_scalar::<_, bool>( + "SELECT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = $1)", + ) + .bind(&login) + .fetch_one(&db) + .await + }; + windmill_common::datatable_roles::sweep_unused_role_set_logins(&db).await?; + assert!(exists().await?, "a set login in use was swept"); + sqlx::query("UPDATE datatable_role_set SET last_used_at = now() - interval '31 days'") + .execute(&db) + .await?; + windmill_common::datatable_roles::sweep_unused_role_set_logins(&db).await?; + assert!(!exists().await?, "an unused set login was kept"); + assert_eq!( + connect(None, Some("test2@windmill.dev")).await?["user"], + login.as_str() + ); + + // A disabled role must not stay reachable through a login that is a member of it. + authed( + client().post(format!("{roles_url}/{}", ids[2])), + "SECRET_TOKEN", + ) + .json(&json!({ "enabled": false })) + .send() + .await? + .error_for_status()?; + let can_login: bool = + sqlx::query_scalar("SELECT rolcanlogin FROM pg_roles WHERE rolname = $1") + .bind(&login) + .fetch_one(&db) + .await?; + assert!(!can_login, "{login} still logs in with a disabled role"); + assert_eq!( + connect(None, Some("test2@windmill.dev")).await?["user"], + two["user"] + ); + Ok::<_, anyhow::Error>(()) + } + .await; + + // Roles are cluster-wide, so they outlive this test's throwaway database. + for name in &names { + let logins: Vec = sqlx::query_scalar(MEMBER_LOGINS) + .bind(name) + .fetch_all(&db) + .await + .unwrap_or_default(); + for login in logins { + let _ = sqlx::query(&format!("DROP ROLE IF EXISTS \"{login}\"")) + .execute(&db) + .await; + } + let _ = sqlx::query(&format!("DROP ROLE IF EXISTS \"{name}\"")) + .execute(&db) + .await; + } + outcome +} diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index e63e69d4d5..21e460359e 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -880,7 +880,11 @@ struct DatatableMigrationsStatusResult { /// Whether the SQL-migrations feature is enabled for a data table. Honors the /// explicit `migrations_enabled` flag; when unset (data tables predating the /// feature) it is considered enabled only if migrations already exist. -async fn datatable_migrations_enabled(db: &DB, w_id: &str, datatable_name: &str) -> Result { +pub(crate) async fn datatable_migrations_enabled( + db: &DB, + w_id: &str, + datatable_name: &str, +) -> Result { let flag: Option = sqlx::query_scalar!( "SELECT (ws.datatable->'datatables'->$2->>'migrations_enabled')::boolean \ FROM workspace_settings ws WHERE ws.workspace_id = $1", diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 3bfcef5c96..bdb3b59e6e 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -2325,6 +2325,9 @@ struct DataTableTables { /// What the role the listing connected as may create. can_create_schema: bool, creatable_schemas: Vec, + /// Whether its schema is kept as versioned migrations, which a change made with plain SQL + /// would bypass. + migrations_enabled: bool, } #[derive(Deserialize)] @@ -2570,8 +2573,17 @@ async fn list_one_datatable_tables( default_role: windmill_common::datatable_roles::ADMIN_DATATABLE_ROLE.to_string(), can_create_schema: false, creatable_schemas: vec![], + migrations_enabled: false, }; let result: Result<()> = async { + // Read from the workspace settings alone, so it is known even when the database is not + // reachable. + entry.migrations_enabled = crate::datatable_migrations::datatable_migrations_enabled( + db, + w_id, + &entry.datatable_name, + ) + .await?; let governing = resolve_governing_datatable(db, w_id, &entry.datatable_name).await?; entry.instance = governing.role_cluster().is_some(); let usable = diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index ae01930182..53996db1dc 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -5568,6 +5568,9 @@ paths: type: boolean default_role: type: string + description: >- + The id of one of `roles`, `admin` (the default), or `$none` for no default + role: a connection that names no role then holds its viewer's roles only. roles: type: array items: @@ -34782,6 +34785,7 @@ components: type: boolean default_role: type: string + description: A role id, `admin`, or `$none` when the data table has no default role. roles: type: array items: @@ -37332,6 +37336,7 @@ components: default_role, can_create_schema, creatable_schemas, + migrations_enabled, ] properties: datatable_name: @@ -37365,6 +37370,9 @@ components: description: the schemas the role the listing connected as may create in items: type: string + migrations_enabled: + type: boolean + description: whether its schema is kept as versioned migrations, which a change made with plain SQL would bypass DataTableTableSchema: type: object diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index 29268d3cb2..2b93c6daeb 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -8982,6 +8982,7 @@ async fn run_preview_script( check_tag_available_for_workspace(&db, &w_id, &tag, &push_args, &authed).await?; let scope_ceiling = windmill_api_auth::caller_scope_ceiling(&db, &authed).await?; + let end_user_email = run_end_user_email(&db, &w_id, &authed, None).await?; let (uuid, tx) = push( &db, tx, @@ -9029,7 +9030,7 @@ async fn run_preview_script( None, Some(&authed.clone().into()), false, - None, + end_user_email, authed.trigger_or_fallback(None), None, scope_ceiling.as_deref(), @@ -9068,6 +9069,7 @@ async fn run_inline_preview_script( if let Some(job_id) = job_id { register_potential_assets_on_inline_execution(job_id, &w_id, &preview); } + let end_user_email = run_end_user_email(&db, &w_id, &authed, None).await?; let utils = get_worker_internal_server_inline_utils()?; let result = utils.run_inline_preview_script.as_ref()(RunInlinePreviewScriptFnParams { content: preview.content, @@ -9078,6 +9080,7 @@ async fn run_inline_preview_script( created_by: authed.display_username().to_string(), permissioned_as: username_to_permissioned_as(&authed.username), permissioned_as_email: authed.email.clone(), + end_user_email, lang: preview.language, job_dir: "".to_string(), worker_name: "".to_string(), @@ -9217,6 +9220,7 @@ async fn run_inline_script_inner( .to_string(), )); } + let end_user_email = run_end_user_email(&db, &w_id, &authed, None).await?; let utils = get_worker_internal_server_inline_utils()?; let authed_owned: windmill_common::db::Authed = authed.clone().into(); let result = utils.run_inline_script.as_ref()(RunInlineScriptFnParams { @@ -9228,6 +9232,7 @@ async fn run_inline_script_inner( created_by: authed.display_username().to_string(), permissioned_as: username_to_permissioned_as(&authed.username), permissioned_as_email: authed.email.clone(), + end_user_email, job_dir: "".to_string(), worker_name: "".to_string(), worker_dir: "".to_string(), @@ -9404,6 +9409,7 @@ async fn run_bundle_preview_script( // tracing::info!("is_tar 1: {is_tar}"); // hmap.insert("") let scope_ceiling = windmill_api_auth::caller_scope_ceiling(&db, &authed).await?; + let end_user_email = run_end_user_email(&db, &w_id, &authed, None).await?; let (uuid, ntx) = push( &db, ltx, @@ -9447,7 +9453,7 @@ async fn run_bundle_preview_script( None, Some(&authed.clone().into()), false, - None, + end_user_email, authed.trigger_or_fallback(None), None, scope_ceiling.as_deref(), @@ -10151,6 +10157,7 @@ async fn run_preview_flow_job( .await?; let scope_ceiling = windmill_api_auth::caller_scope_ceiling(&db, &authed).await?; + let end_user_email = run_end_user_email(&db, &w_id, &authed, None).await?; let (uuid, mut tx) = push( &db, tx, @@ -10182,7 +10189,7 @@ async fn run_preview_flow_job( None, Some(&authed.clone().into()), false, - None, + end_user_email, authed.trigger_or_fallback(None), None, scope_ceiling.as_deref(), diff --git a/backend/windmill-common/src/datatable_roles.rs b/backend/windmill-common/src/datatable_roles.rs index e7e490a6a4..522327040d 100644 --- a/backend/windmill-common/src/datatable_roles.rs +++ b/backend/windmill-common/src/datatable_roles.rs @@ -72,6 +72,11 @@ impl DatatableRoleCluster { /// created, renamed or dropped. pub const ADMIN_DATATABLE_ROLE: &str = "admin"; +/// Stands in a data table's `default_role` for "none": a connection that names no role then holds +/// its viewer's roles only, and is refused without any. `$` is outside both the role-name charset +/// and the catalog ids, and a server predating it refuses it as a role the data table lacks. +pub const NO_DEFAULT_DATATABLE_ROLE: &str = "$none"; + /// 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"; @@ -121,8 +126,13 @@ fn is_reserved_role_name(name: &str) -> bool { || lower.starts_with("pg_") || lower.starts_with("windmill_") || lower.starts_with("custom_instance_") + || lower.starts_with(ROLE_SET_LOGIN_PREFIX) } +/// Prefix of the logins Windmill creates for a connection made as several roles at once. Reserved, +/// so no data table role can be created under, renamed to or taken over at such a name. +pub const ROLE_SET_LOGIN_PREFIX: &str = "wm_set_"; + /// The charset is what makes every downstream interpolation safe: the name reaches Postgres as a /// quoted identifier, a `-- role ` annotation, and a `?role=` query parameter. pub fn validate_role_name(name: &str) -> Result<()> { @@ -414,6 +424,13 @@ pub async fn drop_datatable_role( crate::datatable_roles_oss::drop_datatable_role(db, tx, cluster, name).await } +/// Drop the logins made for a connection as several roles at once that nobody has connected with +/// for a month, on every cluster. They are made again on demand, so this only bounds how many +/// exist. Safe to call from any server at any time: it serializes on the role catalog lock. +pub async fn sweep_unused_role_set_logins(db: &DB) -> Result<()> { + crate::datatable_roles_oss::sweep_unused_role_set_logins(db).await +} + #[cfg(test)] mod tests { use super::*; diff --git a/backend/windmill-common/src/datatable_roles_oss.rs b/backend/windmill-common/src/datatable_roles_oss.rs index 33d711e7f9..739f08d60b 100644 --- a/backend/windmill-common/src/datatable_roles_oss.rs +++ b/backend/windmill-common/src/datatable_roles_oss.rs @@ -29,7 +29,8 @@ pub(crate) use crate::datatable_roles_ee::{ ensure_instance_db_grant_options_unchecked, forget_datatable_role_everywhere, insert_role_catalog_entry, read_role_catalog, read_role_catalog_tx, registered_instance_databases, rename_datatable_role, resolve_datatable_role_connection, - role_cluster, set_datatable_role_login, update_role_catalog_entry, + role_cluster, set_datatable_role_login, sweep_unused_role_set_logins, + update_role_catalog_entry, }; #[cfg(not(all(feature = "private", feature = "enterprise")))] @@ -235,6 +236,11 @@ mod ce { } } + /// No roles, so no set login to sweep. + pub(crate) async fn sweep_unused_role_set_logins(_db: &DB) -> Result<()> { + Ok(()) + } + pub(crate) async fn forget_datatable_role_everywhere( _tx: &mut Tx<'_>, _role_id: &str, diff --git a/backend/windmill-common/src/jobs.rs b/backend/windmill-common/src/jobs.rs index 1a314d34c0..75670fe874 100644 --- a/backend/windmill-common/src/jobs.rs +++ b/backend/windmill-common/src/jobs.rs @@ -474,6 +474,8 @@ pub struct RunInlinePreviewScriptFnParams { pub created_by: String, pub permissioned_as: String, pub permissioned_as_email: String, + /// The caller's recorded end user: `job_perms.end_user_email` of the calling job. + pub end_user_email: Option, pub base_internal_url: String, pub worker_name: String, pub conn: crate::worker::Connection, @@ -496,6 +498,8 @@ pub struct RunInlineScriptFnParams { pub created_by: String, pub permissioned_as: String, pub permissioned_as_email: String, + /// The caller's recorded end user: `job_perms.end_user_email` of the calling job. + pub end_user_email: Option, pub base_internal_url: String, pub worker_name: String, pub conn: crate::worker::Connection, diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 2de1af7317..eae89c6e27 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1582,7 +1582,8 @@ pub struct DataTableReference { /// The access decision for one data table: which role a caller gets, and who may ask for each. #[derive(Deserialize, Serialize, Debug, Clone, Default)] pub struct DataTablePermissions { - /// A role id from the instance catalog, or `admin`. Absent = `admin`. + /// A role id from the instance catalog, `admin`, or + /// [`crate::datatable_roles::NO_DEFAULT_DATATABLE_ROLE`]. Absent = `admin`. #[serde(default, skip_serializing_if = "Option::is_none")] pub default_role: Option, /// Keyed by instance role id, plus the reserved `admin` key. A role absent from this map @@ -2179,9 +2180,13 @@ pub enum DatatableAccess<'a> { PermissionedAs { permissioned_as: &'a str, email: &'a str, + /// The job's viewer: MUST be what the server recorded in `job_perms.end_user_email`, + /// never an argument or anything else the job or its caller can set. + end_user_email: Option<&'a str>, }, - /// A job identified by id; its owner is read from `v2_job`. For agent workers and anything - /// else that authenticates as infrastructure rather than as the job's user. + /// A job identified by id; its owner is read from `v2_job` and its viewer from `job_perms`. + /// For agent workers and anything else that authenticates as infrastructure rather than as + /// the job's user. Job(uuid::Uuid), /// No identity established. Unpermissioned data tables resolve as before; permissioned ones /// are refused, so a caller predating this feature fails closed. @@ -2229,6 +2234,13 @@ pub async fn can_use_datatable_role_in_governing_workspace( /// /// The resolved role **logs in as itself**. Never `SET ROLE`: a script could `RESET ROLE` its way /// back to admin. +/// +/// A default role other than `admin` is a floor rather than the whole connection: a resolution +/// naming no role holds it when the job's own identity may use it, plus the roles whose tenant +/// lists name the job's viewer, as one login that is a member of each. With no role at all it is +/// refused. Being an admin adds none of the viewer's and `admin` is never among them, so a viewer +/// cannot be handed the connection that owns the tables and bypasses row-level security. A named +/// role is that role alone. pub async fn get_datatable_resource_from_db( db: &DB, w_id: &str, diff --git a/backend/windmill-worker/src/common.rs b/backend/windmill-worker/src/common.rs index 6f6c48baec..6088fb4960 100644 --- a/backend/windmill-worker/src/common.rs +++ b/backend/windmill-worker/src/common.rs @@ -1726,6 +1726,23 @@ pub async fn cached_result_path( hasher.update(b"wac_step:"); hasher.update(step_key.as_bytes()); } + // What a run returns can depend on who it ran for (`WM_END_USER_EMAIL`, a data table used + // as the viewer), so a result cached for one viewer is never served to another. Read from + // `job_perms`: a flow job re-fetched between steps does not carry it. + let end_user: Option> = sqlx::query_scalar( + "SELECT end_user_email FROM job_perms WHERE job_id = $1 AND workspace_id = $2", + ) + .bind(job.id) + .bind(&job.workspace_id) + .fetch_optional(db) + .await?; + // Length-prefixed: the arguments follow undelimited, and the end of one viewer's address + // must not read as the start of another run's first argument name. + if let Some(end_user) = end_user.flatten() { + hasher.update(b"end_user:"); + hasher.update((end_user.len() as u64).to_le_bytes()); + hasher.update(end_user.as_bytes()); + } hash_args( db, client, @@ -2475,6 +2492,52 @@ mod tests { } } + // A cache hit returns before the job runs, so the key is all that keeps a result computed + // for one viewer (a data table used as the viewer, `WM_END_USER_EMAIL`) from another. + #[sqlx::test(migrations = "../migrations")] + async fn a_cached_result_is_keyed_by_the_recorded_viewer(pool: sqlx::PgPool) { + let client = test_client(); + + // The last two differ only in where the viewer's address ends and the first argument + // name starts. + let mut paths = vec![]; + for (viewer, arg) in [ + (Some("a@windmill.dev"), "mode"), + (Some("a@windmill.dev"), "mode"), + (Some("b@windmill.dev"), "mode"), + (None, "mode"), + (Some("a@windmill.co"), "mode"), + (Some("a@windmill.com"), "ode"), + ] { + let args = HashMap::from([(arg.to_string(), to_raw_value(&"all"))]); + let job = MiniPulledJob { + id: uuid::Uuid::new_v4(), + args: Some(sqlx::types::Json(args)), + ..test_job() + }; + sqlx::query( + "INSERT INTO job_perms (job_id, email, username, is_admin, is_operator, + workspace_id, groups, folders, end_user_email) + VALUES ($1, 'publisher@windmill.dev', 'publisher', false, false, 'test', '{}', + '{}', $2)", + ) + .bind(job.id) + .bind(viewer) + .execute(&pool) + .await + .unwrap(); + paths.push(cached_result_path(&pool, &client, &job, None).await.unwrap()); + } + assert_eq!( + paths[0], paths[1], + "one viewer shares their own cached result" + ); + assert_ne!(paths[0], paths[2], "two viewers share a cached result"); + assert_ne!(paths[0], paths[3]); + assert_ne!(paths[2], paths[3]); + assert_ne!(paths[4], paths[5], "an address runs into the argument names"); + } + #[tokio::test] async fn test_transform_array_over_1000_passthrough() { let db_url = std::env::var("DATABASE_URL") diff --git a/backend/windmill-worker/src/duckdb_executor.rs b/backend/windmill-worker/src/duckdb_executor.rs index b8c850a2f4..58e47b9332 100644 --- a/backend/windmill-worker/src/duckdb_executor.rs +++ b/backend/windmill-worker/src/duckdb_executor.rs @@ -2758,6 +2758,7 @@ async fn transform_attach_datatable( DatatableAccess::PermissionedAs { permissioned_as: &job.permissioned_as, email: &job.permissioned_as_email, + end_user_email: job.permissioned_as_end_user_email.as_deref(), }, ) .await? diff --git a/backend/windmill-worker/src/pg_executor.rs b/backend/windmill-worker/src/pg_executor.rs index aac7d549db..6309ba8c95 100644 --- a/backend/windmill-worker/src/pg_executor.rs +++ b/backend/windmill-worker/src/pg_executor.rs @@ -1229,6 +1229,7 @@ pub async fn do_postgresql( DatatableAccess::PermissionedAs { permissioned_as: &job.permissioned_as, email: &job.permissioned_as_email, + end_user_email: job.permissioned_as_end_user_email.as_deref(), }, ) .await? diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index 7c3c3971fa..3aa377c904 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -7253,7 +7253,7 @@ pub fn init_worker_internal_server_inline_utils( base_internal_url, killpill_rx: Arc::new(killpill_rx), run_inline_preview_script: Arc::new(|params| { - let job = MiniPulledJob::new_inline( + let mut job = MiniPulledJob::new_inline( params.workspace_id, params.args, params.created_by, @@ -7265,6 +7265,7 @@ pub fn init_worker_internal_server_inline_utils( "inline_preview".to_string(), Some(params.lang), ); + job.permissioned_as_end_user_email = params.end_user_email; Box::pin(async move { let mut mem_peak: i32 = -1; let mut canceled_by: Option = None; @@ -7314,7 +7315,7 @@ pub fn init_worker_internal_server_inline_utils( let content_info = get_script_content_by_hash(&script_hash, ¶ms.workspace_id, ¶ms.conn) .await?; - let job = MiniPulledJob::new_inline( + let mut job = MiniPulledJob::new_inline( params.workspace_id, params.args, params.created_by, @@ -7326,6 +7327,7 @@ pub fn init_worker_internal_server_inline_utils( "inline_run".to_string(), content_info.language, ); + job.permissioned_as_end_user_email = params.end_user_email; let mut mem_peak: i32 = -1; let mut canceled_by: Option = None; let mut column_order: Option> = None; diff --git a/cli/bun.lock b/cli/bun.lock index 0e0885f563..db6fd253bf 100644 --- a/cli/bun.lock +++ b/cli/bun.lock @@ -19,14 +19,14 @@ "pg-gateway": "0.3.0-beta.4", "svelte": "^5.56.8", "tar-stream": "^3.1.7", - "windmill-parser-wasm-asset": "1.753.0", + "windmill-parser-wasm-asset": "1.830.0", "windmill-parser-wasm-csharp": "1.510.1", "windmill-parser-wasm-go": "1.761.0", "windmill-parser-wasm-java": "1.510.1", "windmill-parser-wasm-nu": "1.510.1", "windmill-parser-wasm-php": "1.647.1", - "windmill-parser-wasm-py": "1.782.0", - "windmill-parser-wasm-py-imports": "1.693.1", + "windmill-parser-wasm-py": "1.830.0", + "windmill-parser-wasm-py-imports": "1.830.0", "windmill-parser-wasm-r": "1.668.1", "windmill-parser-wasm-regex": "1.764.0", "windmill-parser-wasm-ruby": "1.526.1", @@ -291,7 +291,7 @@ "utility-types": ["utility-types@3.11.0", "", {}, "sha512-6Z7Ma2aVEWisaL6TvBCy7P8rm2LQoPv6dJ7ecIaIixHcwfbJ0x7mWdbcwlIM5IGQxPZSFYeqRCqlOOeKoJYMkw=="], - "windmill-parser-wasm-asset": ["windmill-parser-wasm-asset@1.753.0", "", {}, "sha512-zpJhjvcU8EWRoOJzas/nRGKjGdQnvzeB9GOxP+Mdmnk8BFk3uehsmHS2Krxyjo36fUrCHqobbnffEqf0g3LIGg=="], + "windmill-parser-wasm-asset": ["windmill-parser-wasm-asset@1.830.0", "", {}, "sha512-q4HSaMOd5cF8pmfPcEGiGeQbO8vhtv89us7zbO12IlgpxpWT8ojvw66J75y4yUt1PaEuaoztSaX8Gkxxn/6MXw=="], "windmill-parser-wasm-csharp": ["windmill-parser-wasm-csharp@1.510.1", "", {}, "sha512-qm09YmnbeYHLwYn1jUnObVzPhYO9NZKMlIO7nlo7zPJBXqksgG5fK/KCtwGw9rChrnz+DsvM9wP5FhrwRLMtwQ=="], @@ -303,9 +303,9 @@ "windmill-parser-wasm-php": ["windmill-parser-wasm-php@1.647.1", "", {}, "sha512-u2qaMkupSdhJibxvkLh3r/y36IARvnYNTLXWvOKxcQ0G/BPUB4+yF5o/yf47vv9zUV5WZv4mrdsKDt/pZDYeDg=="], - "windmill-parser-wasm-py": ["windmill-parser-wasm-py@1.782.0", "", {}, "sha512-ZnpEvhbffaJI+Uq7HH1mVO6kUgR4uiAIIxaLa30Pqb1/oz0h4abT6W7bFOApIE5X9SDOuycoaDBEr9Ogm6ZLzQ=="], + "windmill-parser-wasm-py": ["windmill-parser-wasm-py@1.830.0", "", {}, "sha512-+P4mIyp3lXkgjsr3/20Oc31ank8orQDd4K/v8TnjTQnCyxyy+VB9rynLESnkH6OlcEVSk2qf9pDM9c367E8DoQ=="], - "windmill-parser-wasm-py-imports": ["windmill-parser-wasm-py-imports@1.693.1", "", {}, "sha512-FC0KbREe2G/sa/9kYIR930wmWw+VL6PvEIqg12J3dsJes3A+0x5JIUPT/jeD+c24DrG0ko/Ub7yDnYs56Bem7g=="], + "windmill-parser-wasm-py-imports": ["windmill-parser-wasm-py-imports@1.830.0", "", {}, "sha512-Pi9gHStNFTEqfcDrjSxK5SnI/1dTG4xKcVUD3BHlR1aQ7E/VxkTD/D0INJm7mNC2usKb2bO21NMdKmGh+RAXEw=="], "windmill-parser-wasm-r": ["windmill-parser-wasm-r@1.668.1", "", {}, "sha512-5YNeUibxpNBvYrxCgQcz1PxGhTFx2CyEpg2udtIhq7bx0d4gF/KDZVupMeQmAObmrEtTSFGUWNRJ4zXSWNrSpQ=="], diff --git a/cli/package.json b/cli/package.json index 5e7623a7ab..5efe8a2499 100644 --- a/cli/package.json +++ b/cli/package.json @@ -31,14 +31,14 @@ "pg-gateway": "0.3.0-beta.4", "svelte": "^5.56.8", "tar-stream": "^3.1.7", - "windmill-parser-wasm-asset": "1.753.0", + "windmill-parser-wasm-asset": "1.830.0", "windmill-parser-wasm-csharp": "1.510.1", "windmill-parser-wasm-go": "1.761.0", "windmill-parser-wasm-java": "1.510.1", "windmill-parser-wasm-nu": "1.510.1", "windmill-parser-wasm-php": "1.647.1", - "windmill-parser-wasm-py": "1.782.0", - "windmill-parser-wasm-py-imports": "1.693.1", + "windmill-parser-wasm-py": "1.830.0", + "windmill-parser-wasm-py-imports": "1.830.0", "windmill-parser-wasm-r": "1.668.1", "windmill-parser-wasm-regex": "1.764.0", "windmill-parser-wasm-ruby": "1.526.1", diff --git a/cli/src/commands/app/app.ts b/cli/src/commands/app/app.ts index 470f0f4b11..0060c2c422 100644 --- a/cli/src/commands/app/app.ts +++ b/cli/src/commands/app/app.ts @@ -115,6 +115,14 @@ export function replaceInlineScripts( } return []; } +/** `parent_version` is the editor's stale-draft base: a version id that changes on + * every deploy and means nothing in another workspace, so it is kept out of the + * tracked file and out of what a push deploys. */ +export function stripEditorState(app: any) { + if (app?.value && typeof app.value == "object") { + delete app.value.parent_version; + } +} export function isExecutionModeAnonymous(app: any) { return app?.["policy"]?.["execution_mode"] == "anonymous"; } @@ -294,6 +302,7 @@ export async function pushApp( } const path = localPath + "app.yaml"; const localApp = (await yamlParseFile(path)) as AppFile; + stripEditorState(localApp); const isolation = statedIsolation(localApp, false); replaceInlineScripts(localApp.value, localPath, true); diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index b9e373abad..5ba8888332 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -149,7 +149,11 @@ import { extractCurrentMapping, } from "../../../windmill-utils-internal/src/inline-scripts/extractor.ts"; import { generateFlowLockInternal } from "../flow/flow_metadata.ts"; -import { markAccessFromPolicy, markIsolationFromPolicy } from "../app/app.ts"; +import { + markAccessFromPolicy, + markIsolationFromPolicy, + stripEditorState, +} from "../app/app.ts"; import { APP_BACKEND_FOLDER, generateAppLocksInternal, @@ -1329,6 +1333,7 @@ export function ZipFSElement( log.error(`Failed to parse app.yaml at path: ${p}`); throw error; } + stripEditorState(app); let inlineScripts; try { inlineScripts = extractInlineScriptsForApps( @@ -2749,12 +2754,18 @@ export async function compareDynFSElement( } return o; } else { + let o: any; try { - return yamlParseContent(k, v); + o = yamlParseContent(k, v); } catch (error) { log.error(`Failed to parse YAML content at path: ${k}`); throw error; } + // A file tracked before the field was dropped still carries it. + if (path.basename(k) == "app.yaml") { + stripEditorState(o); + } + return o; } } diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index ab4ef31c3e..0a880b05cf 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -34,6 +34,7 @@ export const SKILLS: SkillMetadata[] = [ { name: "resources", description: "MUST use when managing resources." }, { name: "write-workflow-as-code", description: "MUST use when writing or modifying Windmill Workflow-as-Code scripts using workflow, task, step, sleep, approvals, taskScript, taskFlow, task_script, or task_flow." }, { name: "write-pipeline", description: "MUST use when creating or modifying a data pipeline, a set of scripts marked `pipeline` and wired together by `on` / `materialize` annotations." }, + { name: "datatables", description: "MUST use when changing a data table's schema, writing or running its migrations, or working with its roles and permissions." }, { name: "cli-commands", description: "MUST use when using the CLI, including debugging job failures and inspecting run history via `wmill job`." }, { name: "preview", description: "MUST use when opening the Windmill dev page / visual preview of a flow, script, or app. Triggers on words like preview, open, navigate to, visualize, see the flow/app/script, and after writing a flow/script/app for visual verification." }, ]; @@ -6557,7 +6558,7 @@ data: - \`/
\` — Specific table in public schema - \`/:
\` — Table in specific schema -**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. \`roles\` records the role the app uses each datatable through; the app's code must pass the same role: \`wmill.datatable('main', { role: 'analyst' })\` in TypeScript, \`wmill.datatable('main', role='analyst')\` in Python. A datatable without an entry is used as its default role. +**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. \`roles\` records the role the app uses each datatable through; the app's code must pass the same role: \`wmill.datatable('main', { role: 'analyst' })\` in TypeScript, \`wmill.datatable('main', role='analyst')\` in Python. A datatable without an entry is used as its default role, if it has one, plus the roles of whoever is using the app unless that default is \`admin\`. ## SQL Migrations (sql_to_apply/) @@ -6863,6 +6864,7 @@ Data tables are PostgreSQL databases managed by Windmill. Backend runnables quer 3. **No DDL inside runnables**: runnables only read and write rows (SELECT, INSERT, UPDATE, DELETE) on existing tables. Never CREATE, ALTER or DROP a table from a runnable. 4. **Qualify table names**: an unqualified name means the \`public\` schema, so write every other table as \`schema.table\`, in table creation and queries alike. The app's \`data\` config sets the default datatable and schema its tables go in; use them consistently across runnables. 5. **Pass the role**: when the app's \`data.roles\` gives a data table a role, every \`wmill.datatable\` call on it passes that role (\`wmill.datatable('main', { role: 'analyst' })\` in TypeScript, \`wmill.datatable('main', role='analyst')\` in Python). The role only reaches what it was granted, so a query outside it fails with \`permission denied\`. +6. **Per-viewer rows**: a data table with no entry in \`data.roles\` is called with no role. Unless its default role is \`admin\`, the connection then holds the default role, if it has one, plus every role the person using the app is a tenant of, and is refused when that is none. Row-level security policies written as \`pg_has_role(current_user, '', 'member')\` return that viewer's rows with nothing added to the runnable. A role pins the connection to that one role for every viewer, so where rows should differ per viewer give the data table no \`data.roles\` entry. \`wmill datatable list\` lists the workspace's data tables. Create or change tables with a migration in \`sql_to_apply/\` (see "SQL Migrations" above), then add them to \`data.tables\` in \`raw_app.yaml\`. @@ -8122,6 +8124,53 @@ Node \`f/sales/orders_daily\` (runs when \`orders\` is produced, writes a rollup SELECT date_trunc('day', ts) AS day, count(*) AS n FROM ducklake.main.orders GROUP BY 1 \`\`\` +`, + "datatables": `--- +name: datatables +description: MUST use when changing a data table's schema, writing or running its migrations, or working with its roles and permissions. +--- + +# Data tables: schema changes, migrations and permissions + +A data table is a PostgreSQL database managed by Windmill and shared by the whole workspace: scripts, flows and apps reach it by name, \`main\` being the default. It is configured in the workspace settings (Data Tables), never created by SQL. + +## Changing the schema + +A data table with migrations enabled keeps its schema as an ordered list of versioned SQL migrations, each an \`up\` and an optional \`down\`. That list is what git sync and a workspace merge carry, and what rebuilds the schema in another workspace. So first find out whether migrations are enabled on the data table: + +- **Enabled**: make every schema change (CREATE, ALTER, DROP, anything but row data) as a new migration. A change made with plain SQL exists in that one database only, and the history no longer describes it. +- **Not enabled**: schema changes are plain SQL. Enabling migrations is a workspace admin's decision, made in the Data Tables settings, not a step to take on the way to something else. + +Writing a migration: + +- Wrap the statements in \`BEGIN; ... END;\` so it applies atomically. Leave the frame out only for a statement that cannot run in a transaction, such as \`CREATE INDEX CONCURRENTLY\`: such a migration can fail half applied. +- Give it a \`down\` that undoes the \`up\` whenever the change is reversible. Without one it cannot be rolled back. +- Migrations apply oldest first. One that is applied is not edited: correct it with a new migration. +- Row data is not schema. Insert, update and delete rows with plain SQL, migrations or not. + +\`wmill datatable migrate new \` scaffolds \`migrations/datatable//_.up.sql\` and \`.down.sql\`, \`wmill datatable migrate up\` pushes the local files and applies what is pending, \`wmill datatable migrate down\` rolls back the most recent one and \`wmill datatable migrate status\` lists them. All take \`-d \` and default to \`main\`. + +## A migration is not a draft + +A migration is saved to the workspace and changes the database as soon as it is applied, for everything that reads those tables. To try a schema out without touching tables already in use: + +- put the new tables in a schema of their own, created by the same migration (\`CREATE SCHEMA IF NOT EXISTS\`), and point what you are building at it. A raw app names its schema in its \`data\` config, which is why a new app gets a new schema by default; +- or work in a workspace fork whose data table was cloned when the fork was created ("Clone schema only" or "Clone schema and data"): migrations applied there change that copy alone, and reach the parent workspace when the fork is merged. A fork that kept the original data table points at the parent's database, so a migration applied in it changes the parent's tables at once. + +Altering or dropping a table other scripts or apps use is neither of those: say what depends on it before doing it. + +## Roles and permissions + +Data table roles are a self-hosted Enterprise Edition feature, for data tables on managed Postgres (the instance database or the external managed instance). Without them, and always on a data table backed by a Postgres resource, everyone who can use the data table reaches all of it: there is no per-user, per-schema or per-row access to promise. + +A data table under roles is reached as a Postgres role, and Postgres enforces what that role was granted: + +- roles are assigned, per data table, to users, groups or folders. \`admin\` is the connection the data table had before roles: it owns the existing objects and ignores grants; +- a connection either names a role (\`wmill.datatable('main', { role: 'analyst' })\`, \`role='analyst'\` in Python, \`datatable://main?role=analyst\`) and holds that role alone, or names none and is resolved from the data table's default role; +- \`permission denied\` means the role in use lacks a grant. Say which role and which object: the grant is a data table admin's to give, in the roles drawer of the Database Manager. Do not switch to \`admin\` to get past it; +- a migration runs as the role its SQL declares on its first line, \`-- role \`, the same one in its \`down\`. One that declares none runs as \`admin\`, which only those who may use \`admin\` can do. + +Read the Permissions section of https://www.windmill.dev/docs/core_concepts/persistent_storage/data_tables before designing who reaches what: it covers tenants, grants, the default role and the roles a connection that names none ends up holding, rows that differ per viewer through row-level security, and how forks and raw apps behave under roles. `, "cli-commands": `--- name: cli-commands diff --git a/docs/datatable-viewer-roles.md b/docs/datatable-viewer-roles.md new file mode 100644 index 0000000000..9855e4f97a --- /dev/null +++ b/docs/datatable-viewer-roles.md @@ -0,0 +1,93 @@ +# Data table connections and the viewer's roles + +A data table under roles (Enterprise Edition) resolves a connection that names a role to that +role alone. A connection that names none is resolved from the data table's default role: + +| Default role | A connection naming no role holds | +|---|---| +| `admin` (until changed) | `admin`, as before viewer roles existed | +| another role | that role when the job's own identity may use it, plus **every role the job's viewer is a tenant of** | +| none (`NO_DEFAULT_DATATABLE_ROLE`) | the viewer's roles only | + +A connection left with no role at all is refused. Postgres row-level security can then answer +per viewer from a backend runnable that runs as the app's publisher, with nothing written in the +runnable. + +Policies test membership, which is true for a role's own login too, so one policy serves a named +connection and an unnamed one: + +```sql +CREATE POLICY emea ON orders FOR SELECT + USING (region = 'emea' AND pg_has_role(current_user, 'sales_emea', 'member')); +``` + +A policy names its role as text, so renaming the role breaks it. + +## Code + +| Where | What | +|---|---| +| `windmill-common/src/workspaces.rs` | `DatatableAccess`, `get_datatable_resource_from_db` | +| `windmill-common/src/datatable_roles_ee.rs` | `resolve_datatable_role_connection`, `viewer_role_ids`, `ensure_role_set_login`, `retire_role_set_logins_of`, `sweep_unused_role_set_logins` | +| `windmill-api-jobs/src/execution.rs` | `run_end_user_email`, where a run's viewer is recorded | +| `frontend/.../workspaceSettings/DataTablePermissionsButton.svelte` | the roles drawer, which warns while the default is `admin` | + +## What holds it together + +- **A default of `admin` is not a floor.** `admin` owns the tables and bypasses row-level + security, so nothing could be added to it, and it is the default until someone changes it. + Such a data table resolves exactly as it did before viewer roles existed. Nothing ever sets a + default back to `admin`: removing or deleting the default role leaves none. +- **The default role is tenant-checked against the job's own identity**, the publisher for a + publisher-mode app, and held only when that passes. The viewer's roles are added either way. +- **No role at all is a refusal, never an empty answer.** There is no login that would read + nothing: what a role sees is up to each table's grants and policies. A job without a recorded + viewer (anonymous app viewer, schedule, trigger, a tool job started by an AI agent step), or + whose viewer is not a member of the governing workspace, therefore needs a default it may use + or a named role. +- **A set login owns what it creates.** Postgres gives a new object to `current_user`, and + default privileges go by the creating role, so a table created over a connection that holds + several roles belongs to that set's login: no other role or viewer reaches it, and it passes to + `custom_instance_user` when the login is swept. Unnamed connections are for rows. To create an + object, name a role or run `SET ROLE ` first, which a set login may do for any role it + holds. Migrations are not affected: one that declares no role runs as `admin`. +- **The viewer is `job_perms.end_user_email`, and nothing else.** The server records it when the + run starts: the session behind an app request, the caller of a direct run or preview, and for a + job started with another job's token, that job's viewer. No argument reaches it. +- **A cached result belongs to one viewer.** `cached_result_path` hashes the run's recorded viewer + into the key, since a cache hit returns before any connection is resolved. Runs with no viewer + recorded keep sharing one entry. +- **Only a tenant list naming the viewer counts.** A workspace admin may *use* every role, + `admin` included. The viewer's roles are those whose tenants name them (user, group, folder, + `*`), and never `admin`. +- **One role is that role's login. Several are a login per distinct set.** `wm_set_` is a + member of exactly those roles, with `INHERIT`; the hash is over their catalog ids, so a rename + moves nothing and a change in someone's groups resolves them to another login instead of + editing one. Its privileges are its memberships, so `SET ROLE` only narrows and `RESET ROLE` + comes back to it. The prefix is reserved in `validate_role_name`. +- **Resolving stays cheap where no viewer could add anything.** The viewer is only looked up when + some role other than the default and `admin` has tenants on that data table. +- **A `datatable_role_set` row with a password names a login that can authenticate with it.** The + row is written after the login exists and removed, in its own commit, before the login is + dropped or made `NOLOGIN`. A login without a row is taken back with a new password on next use, + and so is one whose row has an empty password: that is what a sweep leaves when it could + neither drop a login nor let it back in, so the next sweep still finds it. +- **Set logins are bounded by use, not by the number of role combinations.** One exists only for + a set some viewer actually connected with, so there are at most as many as (viewer, data table) + pairs. `last_used_at` is recorded at most once a day, and `sweep_unused_role_set_logins`, run + from the monitor loop, drops those unused for 30 days; a dropped one is made again on demand. + The sweep goes by this instance's rows, never by the cluster's `wm_set_*` roles: a cluster can + serve several Windmill instances. +- **Disabling or dropping a role retires every set login holding it.** `NOLOGIN` on a role does + not reach a login that is a member of it. +- **A worker or server that predates this connects as the default role alone**, so a mixed fleet + shows a viewer fewer rows, never more. It refuses a data table with no default role, whose + marker it reads as a role the data table lacks. + +## Trust + +A job that names no role runs its author's code with the viewer's reach into the data table, the +same trust as an app in viewer execution mode or a script someone else wrote and you run. Whoever +can deploy a publisher-mode app, or a script that others run, can therefore read through the +non-`admin` roles of the people who use it. Naming a role in the code is how an author pins the +connection to one role instead. diff --git a/frontend/package-lock.json b/frontend/package-lock.json index c8250f9eca..6130364ff9 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -95,19 +95,19 @@ "vscode-languageclient": "~9.0.1", "vscode-uri": "~3.1.0", "vscode-ws-jsonrpc": "~3.5.0", - "windmill-parser-wasm-asset": "1.753.0", + "windmill-parser-wasm-asset": "1.830.0", "windmill-parser-wasm-csharp": "1.510.1", "windmill-parser-wasm-go": "1.761.0", "windmill-parser-wasm-java": "1.510.1", "windmill-parser-wasm-nu": "1.510.1", "windmill-parser-wasm-php": "1.647.1", - "windmill-parser-wasm-py": "1.782.0", + "windmill-parser-wasm-py": "1.830.0", "windmill-parser-wasm-r": "1.668.1", "windmill-parser-wasm-regex": "1.764.0", "windmill-parser-wasm-ruby": "1.526.1", "windmill-parser-wasm-rust": "1.647.1", "windmill-parser-wasm-ts": "1.695.0", - "windmill-parser-wasm-wac": "1.668.6", + "windmill-parser-wasm-wac": "1.830.0", "windmill-parser-wasm-yaml": "1.770.0", "windmill-sql-datatype-parser-wasm": "1.512.0", "windmill-utils-internal": "1.8.2", @@ -14644,9 +14644,9 @@ } }, "node_modules/windmill-parser-wasm-asset": { - "version": "1.753.0", - "resolved": "https://registry.npmjs.org/windmill-parser-wasm-asset/-/windmill-parser-wasm-asset-1.753.0.tgz", - "integrity": "sha512-zpJhjvcU8EWRoOJzas/nRGKjGdQnvzeB9GOxP+Mdmnk8BFk3uehsmHS2Krxyjo36fUrCHqobbnffEqf0g3LIGg==" + "version": "1.830.0", + "resolved": "https://registry.npmjs.org/windmill-parser-wasm-asset/-/windmill-parser-wasm-asset-1.830.0.tgz", + "integrity": "sha512-q4HSaMOd5cF8pmfPcEGiGeQbO8vhtv89us7zbO12IlgpxpWT8ojvw66J75y4yUt1PaEuaoztSaX8Gkxxn/6MXw==" }, "node_modules/windmill-parser-wasm-csharp": { "version": "1.510.1", @@ -14674,9 +14674,9 @@ "integrity": "sha512-u2qaMkupSdhJibxvkLh3r/y36IARvnYNTLXWvOKxcQ0G/BPUB4+yF5o/yf47vv9zUV5WZv4mrdsKDt/pZDYeDg==" }, "node_modules/windmill-parser-wasm-py": { - "version": "1.782.0", - "resolved": "https://registry.npmjs.org/windmill-parser-wasm-py/-/windmill-parser-wasm-py-1.782.0.tgz", - "integrity": "sha512-ZnpEvhbffaJI+Uq7HH1mVO6kUgR4uiAIIxaLa30Pqb1/oz0h4abT6W7bFOApIE5X9SDOuycoaDBEr9Ogm6ZLzQ==" + "version": "1.830.0", + "resolved": "https://registry.npmjs.org/windmill-parser-wasm-py/-/windmill-parser-wasm-py-1.830.0.tgz", + "integrity": "sha512-+P4mIyp3lXkgjsr3/20Oc31ank8orQDd4K/v8TnjTQnCyxyy+VB9rynLESnkH6OlcEVSk2qf9pDM9c367E8DoQ==" }, "node_modules/windmill-parser-wasm-r": { "version": "1.668.1", @@ -14704,9 +14704,9 @@ "integrity": "sha512-9EFxeRZWmfb7EyhSlcG7dzTTKETPRYAvpRlxxLkhhtI5I219wFgI7kwrMpz4stXHJj/aqBknVv66NQHRstSJmw==" }, "node_modules/windmill-parser-wasm-wac": { - "version": "1.668.6", - "resolved": "https://registry.npmjs.org/windmill-parser-wasm-wac/-/windmill-parser-wasm-wac-1.668.6.tgz", - "integrity": "sha512-/ovcLWlIO+TQMrnWwcWoXquJI6hTZ+Zpo4POhV8z6e+pb4cVModGysASI9mERn2e9uj8/xSRmYqk54TLG8oUbQ==" + "version": "1.830.0", + "resolved": "https://registry.npmjs.org/windmill-parser-wasm-wac/-/windmill-parser-wasm-wac-1.830.0.tgz", + "integrity": "sha512-M67qPkzDnuMNWGrZTj9V4ajDoCQrTsQJ89UMHq308t5CtCdtiX8Z3freL+8SoRAG/dVlqQ8roK6dzncHIEM0hQ==" }, "node_modules/windmill-parser-wasm-yaml": { "version": "1.770.0", diff --git a/frontend/package.json b/frontend/package.json index ddfc6b410b..7522af8675 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -161,19 +161,19 @@ "vscode-languageclient": "~9.0.1", "vscode-uri": "~3.1.0", "vscode-ws-jsonrpc": "~3.5.0", - "windmill-parser-wasm-asset": "1.753.0", + "windmill-parser-wasm-asset": "1.830.0", "windmill-parser-wasm-csharp": "1.510.1", "windmill-parser-wasm-go": "1.761.0", "windmill-parser-wasm-java": "1.510.1", "windmill-parser-wasm-nu": "1.510.1", "windmill-parser-wasm-php": "1.647.1", - "windmill-parser-wasm-py": "1.782.0", + "windmill-parser-wasm-py": "1.830.0", "windmill-parser-wasm-r": "1.668.1", "windmill-parser-wasm-regex": "1.764.0", "windmill-parser-wasm-ruby": "1.526.1", "windmill-parser-wasm-rust": "1.647.1", "windmill-parser-wasm-ts": "1.695.0", - "windmill-parser-wasm-wac": "1.668.6", + "windmill-parser-wasm-wac": "1.830.0", "windmill-parser-wasm-yaml": "1.770.0", "windmill-sql-datatype-parser-wasm": "1.512.0", "windmill-utils-internal": "1.8.2", diff --git a/frontend/src/lib/components/DBManager.svelte b/frontend/src/lib/components/DBManager.svelte index 837569c575..5b34c66bc5 100644 --- a/frontend/src/lib/components/DBManager.svelte +++ b/frontend/src/lib/components/DBManager.svelte @@ -45,7 +45,12 @@ import { favoriteManager } from './sidebar/FavoriteMenu.svelte' import DatatableRoleBadge from './DatatableRoleBadge.svelte' import type { AclTarget, Asset, DataTableTables } from '$lib/gen' - import { ADMIN_DATATABLE_ROLE, datatableNameTakesRole, type DatatableRowAction } from './dbTypes' + import { + ADMIN_DATATABLE_ROLE, + datatableNameTakesRole, + namedDefaultRole, + type DatatableRowAction + } from './dbTypes' import TextInput from './text_input/TextInput.svelte' import Checkbox from './common/checkbox/Checkbox.svelte' import DbSchemaDiagram from './dbdiagram/DbSchemaDiagram.svelte' @@ -417,7 +422,10 @@ if (!entry?.permissioned || !datatableNameTakesRole(datatable)) return undefined const roles = entry.usable_roles if (roles.length === 0) return { kind: 'none' } - const role = (datatable === currentDatatable ? currentRole : undefined) ?? entry.default_role + const role = + (datatable === currentDatatable ? currentRole : undefined) ?? + namedDefaultRole(entry.default_role) ?? + roles[0] if (roles.length === 1 && roles[0] === ADMIN_DATATABLE_ROLE && role === ADMIN_DATATABLE_ROLE) { return undefined } diff --git a/frontend/src/lib/components/MemoryFootprintViewer.svelte b/frontend/src/lib/components/MemoryFootprintViewer.svelte index 9547861bc3..14ad1a2b06 100644 --- a/frontend/src/lib/components/MemoryFootprintViewer.svelte +++ b/frontend/src/lib/components/MemoryFootprintViewer.svelte @@ -3,7 +3,7 @@ import { type MetricDataPoint, MetricsService } from '$lib/gen' import { displayTime } from '$lib/utils' - import { workspaceStore } from '$lib/stores' + import { useOperatingWorkspace } from './operatingWorkspace.svelte' import { CategoryScale, Chart as ChartJS, @@ -26,6 +26,8 @@ let { jobId, jobUpdateLastFetch }: Props = $props(); + const operatingWorkspace = useOperatingWorkspace() + let jobMetricsLastFetch: Date | undefined = undefined let jobMemoryStats: MetricDataPoint[] | undefined = $state(undefined) @@ -38,7 +40,7 @@ async function loadMetricsData() { try { let jobStatsPromise = MetricsService.getJobMetrics({ - workspace: $workspaceStore!, + workspace: $operatingWorkspace!, id: jobId, requestBody: { from_timestamp: jobMetricsLastFetch?.toISOString(), diff --git a/frontend/src/lib/components/RunsPage.svelte b/frontend/src/lib/components/RunsPage.svelte index 7a130869b5..3cc3bb2ffd 100644 --- a/frontend/src/lib/components/RunsPage.svelte +++ b/frontend/src/lib/components/RunsPage.svelte @@ -11,14 +11,12 @@ import { sendUserToast } from '$lib/toast' import { displayDate } from '$lib/utils' + import { userWorkspaces, superadmin, devopsRole, enterpriseLicense } from '$lib/stores' + import { useHostedPage } from '$lib/components/hostedPage' import { - userStore, - workspaceStore, - userWorkspaces, - superadmin, - devopsRole, - enterpriseLicense - } from '$lib/stores' + useOperatingUser, + useOperatingWorkspace + } from '$lib/components/operatingWorkspace.svelte' import { Button, Drawer, DrawerContent, Skeleton, Tab, Tabs } from '$lib/components/common' import TextInput from '$lib/components/text_input/TextInput.svelte' import RunChart from '$lib/components/RunChart.svelte' @@ -81,6 +79,13 @@ let { initialPath }: Props = $props() + // Under a host — an AI session's preview tab — the document URL, the page header band and + // the navigation workspace all belong to the host: the filters live where it keeps them, + // the filter row renders in the page, and requests go to the workspace it names. + const hosted = useHostedPage() + const operatingWorkspace = useOperatingWorkspace() + const operatingUser = useOperatingUser() + let paths: string[] = $state([]) let usernames: string[] = $state([]) let folders: string[] = $state([]) @@ -91,7 +96,7 @@ folders, jobTriggerKinds, isSuperAdminOrDevops: !!$superadmin || !!$devopsRole, - isAdminsWorkspace: $workspaceStore === 'admins' + isAdminsWorkspace: $operatingWorkspace === 'admins' }) ) let perPage = useLocalStorageValue('runs_per_page', 1000, 'number') @@ -115,6 +120,8 @@ // what was seeded at mount is gone. Re-apply it. Editing a filter writes with `replaceState`, // which never reaches `page.url`, so a filter the user clears stays cleared. $effect(() => { + // A host remounts the page when it re-points it, and its document URL is not this page's. + if (hosted) return page.url.href untrack(() => filters.seed(filterSeeds())) }) @@ -171,7 +178,9 @@ let manualTimeframe = $derived(timeframe.type === 'manual' ? timeframe : undefined) let graph: 'RunChart' | 'ConcurrencyChart' = $state( - typeOfChart(page.url.searchParams.get('graph')) + typeOfChart( + (hosted ? new URLSearchParams(hosted.search) : page.url.searchParams).get('graph') + ) ) let innerWidth = $state(window.innerWidth) let jobsLoader = useJobsLoader(() => ({ @@ -183,7 +192,7 @@ resultError, perPage: perPage.val, lookback: graph === 'RunChart' ? 0 : lookback, - currentWorkspace: $workspaceStore ?? '' + currentWorkspace: $operatingWorkspace ?? '' })) let batchProgress = $derived(jobsLoader.batchProgress) let currentBatchSize = $derived(jobsLoader.currentBatchSize) @@ -211,18 +220,18 @@ } async function loadUsernames(): Promise { - usernames = await UserService.listUsernames({ workspace: $workspaceStore! }) + usernames = await UserService.listUsernames({ workspace: $operatingWorkspace! }) } async function loadFolders(): Promise { folders = await FolderService.listFolders({ - workspace: $workspaceStore! + workspace: $operatingWorkspace! }).then((x) => x.map((y) => y.name)) } async function loadPaths() { - const npaths_scripts = await ScriptService.listScriptPaths({ workspace: $workspaceStore ?? '' }) - const npaths_flows = await FlowService.listFlowPaths({ workspace: $workspaceStore ?? '' }) + const npaths_scripts = await ScriptService.listScriptPaths({ workspace: $operatingWorkspace ?? '' }) + const npaths_flows = await FlowService.listFlowPaths({ workspace: $operatingWorkspace ?? '' }) paths = npaths_scripts.concat(npaths_flows).sort() } @@ -265,7 +274,7 @@ const resultFilter = filters.val.result && JSON.parse(filters.val.result) const { minTs, maxTs } = timeframe.computeMinMax() return { - workspace: $workspaceStore ?? '', + workspace: $operatingWorkspace ?? '', startedBefore: maxTs ?? undefined, startedAfter: minTs ?? undefined, schedulePath: filters.val.schedule_path ?? undefined, @@ -323,7 +332,7 @@ } async function cancelJobs(uuidsToCancel: string[], forceCancel: boolean = false) { const uuids = await JobService.cancelSelection({ - workspace: $workspaceStore ?? '', + workspace: $operatingWorkspace ?? '', requestBody: uuidsToCancel, forceCancel: forceCancel, allWorkspaces: filters.val.all_workspaces || undefined @@ -350,7 +359,7 @@ for (const j of jobs ?? []) if (j.workspace_id) workspaceById.set(j.id, j.workspace_id) const byWorkspace = new Map() for (const id of jobIds) { - const ws = workspaceById.get(id) ?? $workspaceStore ?? '' + const ws = workspaceById.get(id) ?? $operatingWorkspace ?? '' const bucket = byWorkspace.get(ws) if (bucket) bucket.push(id) else byWorkspace.set(ws, [id]) @@ -428,7 +437,7 @@ } async function reRunJobs(jobIdsToReRun: string[], batchReRunOptions: BatchReRunOptions) { - if (!$workspaceStore) return + if (!$operatingWorkspace) return if (askingForConfirmation) { askingForConfirmation.loading = true @@ -442,7 +451,7 @@ // workaround because EventSource does not support POST requests // https://medium.com/@david.richards.tech/sse-server-sent-events-using-a-post-request-without-eventsource-1c0bd6f14425 - const response = await fetch(`${OpenAPI.BASE}/w/${$workspaceStore}/jobs/run/batch_rerun_jobs`, { + const response = await fetch(`${OpenAPI.BASE}/w/${$operatingWorkspace}/jobs/run/batch_rerun_jobs`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body) @@ -555,7 +564,7 @@ } $effect(() => { - if ($workspaceStore) { + if ($operatingWorkspace) { untrack(() => { loadUsernames() loadFolders() @@ -603,7 +612,7 @@ openedRunId = undefined } let resolutionNote = $state('') - const phone = $derived(pageHeader.barWidth > 0 && pageHeader.barWidth < PHONE_BAR) + const phone = $derived(!hosted && pageHeader.barWidth > 0 && pageHeader.barWidth < PHONE_BAR) {#snippet runsHint()} @@ -620,7 +629,7 @@ two or three buttons, so they take a wider gap than the band's own. `flex-1 min-w-0` passes the band's flexibility down to the search field, the one control here that gives width back; the rest keep theirs or drop out of the row entirely at a width. --> -
+
-
+
RUNS_PATH + window.location.search, - workspaceId: $workspaceStore ?? undefined + workspaceId: $operatingWorkspace ?? undefined }} btnProps={{ unifiedSize: 'sm' }} /> @@ -900,13 +909,14 @@ + { - reset() + if (!hosted) reset() }} /> -{#if $userStore?.operator && $workspaceStore && !$userWorkspaces.find((_) => _.id === $workspaceStore)?.operator_settings?.runs} +{#if operatingUser.current?.operator && $operatingWorkspace && !$userWorkspaces.find((_) => _.id === $operatingWorkspace)?.operator_settings?.runs} +{:else if slot.kind === 'run' && mounted && runtime} +
+ {#if overlayHostEl} + {#await import('./RunViewerView.svelte')} + {@render editorLoading()} + {:then Module} + + + {/await} + {/if} +
{:else if slot.kind === 'artifact' && mounted}
+ /* + * A run in a session preview tab, rendered by the component the workspace `/run/` page + * renders. Everything it reaches is scoped to the run's workspace through `workspace`, and + * every navigation it attempts is caught here and turned into a move inside the preview + * panel, so nothing takes the browser out of the session. + */ + import { setContext, untrack } from 'svelte' + import RunDetail from '$lib/components/runs/RunDetail.svelte' + import type { SessionRuntime } from './sessionRuntime.svelte' + import { + parseHistoricalScriptEdit, + parsePreviewItemRoute, + parseRunRoute, + previewLocationLabel, + barePath + } from './previewRouter' + + let { + runtime, + jobId, + workspaceId, + tabId, + container, + reloadNonce = 0 + }: { + runtime: SessionRuntime + jobId: string + workspaceId: string + /** The preview tab hosting this run, which a move to another run re-points. */ + tabId: string + /** The tab's host element, which also holds the overlays the page portals out. */ + container: HTMLElement | undefined + /** Bumped to load the run again from the server. */ + reloadNonce?: number + } = $props() + + // Mounted in the preview panel, outside the chat's own subtree, as ItemViewerView is: the + // run page's AI entry points would otherwise offer a session from inside one. + setContext( + 'aiChatManager', + untrack(() => runtime.manager) + ) + + // Everything the run page navigates to, resolved against the preview panel. Another run — + // a re-run, a retry, the parent job — is still this tab's subject, so it re-points in + // place; an item or any other page opens beside it. Nothing reaches the top-level router. + function onNavigate(url: string): void { + const parsed = new URL(url, window.location.origin) + const href = `${parsed.pathname}${parsed.search}${parsed.hash}` + if (parseRunRoute(href)) { + runtime.previewTabs.navigate({ type: 'page', href, label: previewLocationLabel(href) }, tabId) + return + } + const route = parsePreviewItemRoute(href) + // Legacy drag-and-drop apps and historical script edits have no in-panel host, so they + // fall through to a page tab. So does `Run again`, whose args ride in the fragment, and + // a link to one version: opening an item focuses a tab already on it as it stands, + // where a page tab's URL is written whole, pin and fragment included. + if ( + route && + (route.kind !== 'app' || route.raw_app) && + !parseHistoricalScriptEdit(href) && + !parsed.hash && + !parsed.searchParams.has('version') + ) { + runtime.previewTabs.open({ + type: 'item', + item: { path: route.itemPath, summary: '', kind: route.kind, raw_app: route.raw_app }, + mode: route.mode + }) + return + } + runtime.previewTabs.open({ type: 'page', href, label: previewLocationLabel(barePath(href)) }) + } + + // Every in-app link under the tab, not only the ones that call `onNavigate`: a plain anchor + // followed by the browser would take it out of the session. Listened for on `body`, as + // ItemViewerView does and for the same reason. + $effect(() => { + const el = container + if (!el) return + function onClick(e: MouseEvent) { + if (e.defaultPrevented || e.button !== 0) return + if (e.metaKey || e.ctrlKey || e.shiftKey || e.altKey) return + const target = e.target as Element | null + if (!target || !el!.contains(target)) return + const a = target.closest?.('a[href]') as HTMLAnchorElement | null + if (!a || (a.target && a.target !== '_self') || a.hasAttribute('download')) return + const url = new URL(a.href, window.location.href) + if (url.origin !== window.location.origin) return + e.preventDefault() + if (a.getAttribute('href')?.startsWith('#')) return + onNavigate(`${url.pathname}${url.search}${url.hash}`) + } + document.body.addEventListener('click', onClick) + return () => document.body.removeEventListener('click', onClick) + }) + + +
+ + {#key `${workspaceId}/${jobId}/${reloadNonce}`} + + {/key} +
diff --git a/frontend/src/lib/components/sessions/previewPaths.ts b/frontend/src/lib/components/sessions/previewPaths.ts index 535cb1a272..6c462ff322 100644 --- a/frontend/src/lib/components/sessions/previewPaths.ts +++ b/frontend/src/lib/components/sessions/previewPaths.ts @@ -84,6 +84,17 @@ export function isPageItemListPath(path: string): boolean { return pageItemForListPath(path, '') !== undefined } +/** Whether a path is the runs list: every run, or `/runs/` for one item's. */ +export function isRunsListPath(path: string): boolean { + const clean = barePath(path) + return clean === RUNS_PATH || clean.startsWith(`${RUNS_PATH}/`) +} + +/** Whether a path is a list page a session preview mounts in process rather than frames. */ +export function isPreviewListPath(path: string): boolean { + return isPageItemListPath(path) || isRunsListPath(path) +} + /** The page item a list page's row names, or undefined for a page that lists none. */ export function pageItemForListPath(pagePath: string, path: string): PageItemRef | undefined { const clean = barePath(pagePath) @@ -145,7 +156,13 @@ export const pageKey = (path: string) => `page:${path}` /** Strip the query and hash from a preview path so it can be matched against * `PREVIEW_PAGES` / parsed as an item route. */ export function barePath(path: string): string { - return path.split('?')[0].split('#')[0] || '/' + // The router serves `/runs/` as `/runs`, and links are written both ways. + return ( + path + .split('?')[0] + .split('#')[0] + .replace(/(.)\/+$/, '$1') || '/' + ) } /** Which side of an item a route addresses: `/edit/` builds it, `/get/` shows the diff --git a/frontend/src/lib/components/sessions/previewRouter.test.ts b/frontend/src/lib/components/sessions/previewRouter.test.ts index 038691da55..18d287769e 100644 --- a/frontend/src/lib/components/sessions/previewRouter.test.ts +++ b/frontend/src/lib/components/sessions/previewRouter.test.ts @@ -313,7 +313,30 @@ describe('itemDisplayName', () => { describe('resolvePreviewTab', () => { it('routes a static page to the iframe fallback', () => { - expect(resolvePreviewTab('/runs')).toEqual({ kind: 'iframe' }) + expect(resolvePreviewTab('/audit_logs')).toEqual({ kind: 'iframe' }) + }) + + it('mounts a run in process, in the workspace its URL names', () => { + expect(resolvePreviewTab('/run/01a0?workspace=fork')).toEqual({ + kind: 'run', + jobId: '01a0', + workspace: 'fork' + }) + expect(resolvePreviewTab('/run/01a0')).toEqual({ kind: 'run', jobId: '01a0' }) + expect(resolvePreviewTab('/run/01a0?workspace=fork&view_token=t')).toEqual({ kind: 'iframe' }) + }) + + it('mounts the runs list in process, for every run or for one item', () => { + expect(resolvePreviewTab('/runs?status=failure')).toEqual({ kind: 'pagelist', path: '/runs' }) + expect(resolvePreviewTab('/runs/f/a/b')).toEqual({ kind: 'pagelist', path: '/runs/f/a/b' }) + expect(resolvePreviewTab('/runsheet')).toEqual({ kind: 'iframe' }) + }) + + it('tells two runs lists apart by their filters, trailing slash or not', () => { + expect(resolvePreviewTab('/runs/?status=failure')).toEqual({ kind: 'pagelist', path: '/runs' }) + expect(showsView('/runs/?schedule_path=a', '/runs/?schedule_path=b')).toBe(false) + expect(showsView('/runs?schedule_path=a', '/runs/?schedule_path=a')).toBe(true) + expect(showsView('/runs/f/a/b?status=failure', '/runs/f/a/b')).toBe(false) }) it('mounts the list page of a page item kind in process, whatever its filters', () => { diff --git a/frontend/src/lib/components/sessions/previewRouter.ts b/frontend/src/lib/components/sessions/previewRouter.ts index 2de5354c4d..637d6a6052 100644 --- a/frontend/src/lib/components/sessions/previewRouter.ts +++ b/frontend/src/lib/components/sessions/previewRouter.ts @@ -3,7 +3,8 @@ import { AUDIT_LOGS_PATH, FOLDERS_PATH, GROUPS_PATH, - isPageItemListPath, + isPreviewListPath, + isRunsListPath, pageItemForListPath, pageItemListPath, pageItemPageHref, @@ -247,9 +248,14 @@ function pageRequestParamTable(): Record { }) } +// The page whose filters a path takes: one item's runs filter as every run does. +function filterPageOf(path: string): string { + return isRunsListPath(path) ? RUNS_PATH : path +} + /** The query params a request can set on `path` — empty for a page that takes none. */ export function pageRequestParams(path: string): readonly string[] { - return pageRequestParamTable()[barePath(path)] ?? [] + return pageRequestParamTable()[filterPageOf(barePath(path))] ?? [] } /** What a preview location means, read against the page it points at. */ @@ -286,7 +292,7 @@ export function describeLocation(loc: string): PreviewLocation { const query = bare.includes('?') ? bare.slice(bare.indexOf('?') + 1) : '' return { identity: path, - view: requestedParams(query, pageRequestParamTable()[path]), + view: requestedParams(query, pageRequestParamTable()[filterPageOf(path)]), anchor: drawerAnchorFor(canonical) ?? '' } } @@ -319,7 +325,7 @@ export function showsView(observed: string, commanded: string): boolean { const y = describeLocation(commanded) if (x.identity !== y.identity || x.anchor !== y.anchor) return false if (x.view === y.view) return true - const seeded = PAGE_SEEDED_PARAMS[x.identity] + const seeded = PAGE_SEEDED_PARAMS[filterPageOf(x.identity)] if (!seeded?.length) return false // Both views come out of `requestedParams` already sorted and encoded, so they compare // as strings once the seeded params are dropped. One direction only: the page may add @@ -464,9 +470,9 @@ export function previewLocationLabel(url: string): string { if (page) return page.label const trigger = triggerLabelForPath(url) if (trigger) return trigger - const run = barePath(url).match(/^\/run\/([^/?#]+)/) + const run = parseRunRoute(url) // Job ids are time-ordered, so runs started moments apart share their head. - if (run) return `Run ${decodeURIComponent(run[1]).slice(-8)}` + if (run) return `Run ${run.jobId.slice(-8)}` // The runs of one item, as a detail page's Runs button links them. const itemRuns = barePath(url).match(/^\/runs\/(.+)$/) if (itemRuns) return `Runs · ${decodeURIComponent(itemRuns[1]).split('/').pop()}` @@ -550,6 +556,25 @@ export function parseRunFormRoute(url: string): { toolCallId: string; label: str } } +/** The run a `/run/` location shows, in the workspace its query names (the opener's, + * when it names none). */ +export function parseRunRoute( + url: string +): { jobId: string; workspace?: string; viewToken: boolean } | null { + const m = barePath(url).match(/^\/run\/([^/?#]+)$/) + if (!m) return null + try { + const query = new URL(url, 'http://_').searchParams + return { + jobId: decodeURIComponent(m[1]), + workspace: query.get('workspace') || undefined, + viewToken: query.has('view_token') + } + } catch { + return null + } +} + export function runFormUrl(toolCallId: string, label: string): string { return `runform:${encodeURIComponent(toolCallId)}#${encodeURIComponent(label)}` } @@ -575,7 +600,8 @@ export const isArtifactKey = (key: string) => key.startsWith('artifact:') // flow, raw app) mounts its per-(kind,path) cell editor on `/edit/` and its viewer // on `/get/`; a `/pipeline/` route mounts the data-pipeline graph editor of // that folder (`path` is the folder); the list page of a page item kind mounts that -// list; everything +// list, and so does the runs list; a `/run/` route without a share token mounts that +// run's page; everything // else (static pages, regular drag-and-drop apps, any other route) stays an iframe. export type PreviewSlot = | { kind: 'editor'; editorKind: SessionTargetKind | 'pipeline'; path: string } @@ -584,6 +610,7 @@ export type PreviewSlot = | { kind: 'runform'; toolCallId: string } | { kind: 'pageitem'; ref: PageItemRef } | { kind: 'pagelist'; path: string } + | { kind: 'run'; jobId: string; workspace?: string } | { kind: 'iframe' } export function resolvePreviewTab(url: string): PreviewSlot { @@ -594,7 +621,13 @@ export function resolvePreviewTab(url: string): PreviewSlot { const pageItem = parsePageItemRoute(url) if (pageItem) return { kind: 'pageitem', ref: pageItem } const route = barePath(url) - if (isPageItemListPath(route)) return { kind: 'pagelist', path: route } + if (isPreviewListPath(route)) return { kind: 'pagelist', path: route } + const run = parseRunRoute(url) + // A share link's token authorizes every request of the window that installs it, so a run + // opened through one keeps a frame of its own rather than lending the token to the session. + if (run && !run.viewToken) { + return { kind: 'run', jobId: run.jobId, ...(run.workspace && { workspace: run.workspace }) } + } const pipelineFolder = parsePipelineRoute(url) if (pipelineFolder) { return { kind: 'editor', editorKind: 'pipeline', path: pipelineFolder } @@ -628,6 +661,7 @@ export function workspacePageHref(location: string): string | undefined { case 'editor': case 'viewer': case 'pagelist': + case 'run': case 'iframe': return location default: { diff --git a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts index 2995366ae0..8d242912dc 100644 --- a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts @@ -471,6 +471,7 @@ function createRuntime(session: Session): SessionRuntime { slotKind !== 'iframe' && slotKind !== 'pageitem' && slotKind !== 'pagelist' && + slotKind !== 'run' && slotKind !== 'viewer' ) return undefined @@ -593,8 +594,8 @@ function createRuntime(session: Session): SessionRuntime { onTabOpened: logTabUsage }) - // Let the jobs tray open a run in this session's preview panel (as an iframe - // tab over the run page). The global side-panel chat leaves this unset and + // Let the jobs tray open a run in this session's preview panel (as a tab on + // the run's page). The global side-panel chat leaves this unset and // falls back to a new browser tab. manager.openRunInPreview = ({ jobId, workspace, label }) => { previewTabs.open({ diff --git a/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte b/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte index 680421450d..496c967d7f 100644 --- a/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte +++ b/frontend/src/lib/components/workspaceSettings/DataTablePermissionsButton.svelte @@ -23,7 +23,7 @@ import { superadmin } from '$lib/stores' import { sendUserToast } from '$lib/toast' import { deepEqual } from 'fast-equals' - import { ADMIN_DATATABLE_ROLE, isDatatableRoleName } from '../dbTypes' + import { ADMIN_DATATABLE_ROLE, NO_DEFAULT_DATATABLE_ROLE, isDatatableRoleName } from '../dbTypes' import PgAclEditor from '../datatableAcl/PgAclEditor.svelte' import ConfirmationModal from '../common/confirmationModal/ConfirmationModal.svelte' import { createAsyncConfirmationModal } from '../common/confirmationModal/asyncConfirmationModal.svelte' @@ -87,6 +87,8 @@ const pgInstanceName = $derived(pgInstanceNameOf(info?.cluster)) const unusedRoles = $derived(availableRoles.filter((r) => !roles.some((row) => row.id === r.id))) const confirmationModal = createAsyncConfirmationModal() + // Every member of the governing workspace, as the tenant lists spell it. + const EVERYONE_TENANT = '*' let aclEditor = $state(undefined) const hasUnsavedChanges = $derived( @@ -111,9 +113,13 @@ (a, b) => Number(b.id === ADMIN_DATATABLE_ROLE) - Number(a.id === ADMIN_DATATABLE_ROLE) ) // A data table never put under roles comes back with none; admin is what turning the toggle - // on starts from. + // on starts from, open to everyone so that turning it on changes nothing by itself. if (!loaded.some((r) => r.id === ADMIN_DATATABLE_ROLE)) { - loaded.unshift({ id: ADMIN_DATATABLE_ROLE, name: ADMIN_DATATABLE_ROLE, tenants: [] }) + loaded.unshift({ + id: ADMIN_DATATABLE_ROLE, + name: ADMIN_DATATABLE_ROLE, + tenants: [EVERYONE_TENANT] + }) } info = res catalog = undefined @@ -137,7 +143,7 @@ FolderService.listFolderNames({ workspace: ws }) ]) tenantItems = [ - { value: '*', label: 'Everyone', group: 'Anyone in the workspace' }, + { value: EVERYONE_TENANT, label: 'Everyone', group: 'Anyone in the workspace' }, ...users.map((u) => ({ value: `u/${u}`, label: u, group: 'Users' })), ...groups.map((g) => ({ value: `g/${g}`, label: g, group: 'Groups' })), ...folders.map((f) => ({ value: `f/${f}`, label: f, group: 'Folders' })) @@ -218,11 +224,28 @@ } } - function removeRole(role: EditedRole) { + async function removeRole(role: EditedRole) { + if (defaultRoleId === role.id) { + const confirmed = await confirmationModal.ask({ + title: 'Remove the default role?', + children: `Jobs that name no role connect as ${role.name ?? role.id}. Without it the data table has no default role: they connect with the roles of the person running them, and are refused when that person has none.`, + confirmationText: 'Remove', + type: 'danger' + }) + if (!confirmed) return + defaultRoleId = NO_DEFAULT_DATATABLE_ROLE + } roles = roles.filter((r) => r.id !== role.id) - if (defaultRoleId === role.id) defaultRoleId = ADMIN_DATATABLE_ROLE } + const defaultRoleName = $derived(roles.find((r) => r.id === defaultRoleId)?.name ?? defaultRoleId) + const defaultsToAdmin = $derived(defaultRoleId === ADMIN_DATATABLE_ROLE) + const hasNoDefault = $derived(defaultRoleId === NO_DEFAULT_DATATABLE_ROLE) + const hasOtherRoles = $derived(roles.some((r) => r.id !== ADMIN_DATATABLE_ROLE)) + const adminOpenToEveryone = $derived( + !!roles.find((r) => r.id === ADMIN_DATATABLE_ROLE)?.tenants.includes(EVERYONE_TENANT) + ) + async function save() { saving = true // Turning roles off clears the whole block, so every role it named is removed. @@ -320,7 +343,7 @@ options={{ right: 'Put this data table under roles', rightTooltip: - 'Off, every job connects as admin — the connection that owns every table. On, every job resolves to a role, and a caller no tenant covers is refused.' + 'Off, every job connects as admin — the connection that owns every table. On, every job resolves to a role, and a caller no tenant covers is refused. It starts with admin open to everyone, so turning it on changes nothing until the roles are edited.' }} /> @@ -391,8 +414,10 @@ Default - The role a job gets when it names none — no `-- role` annotation, no `?role=` in - the reference. Callers still have to be one of its tenants. + The role a job gets when it names none: no `-- role` annotation, no `?role=` in + the reference. There is always one choice: admin until changed, another role, or + none. Unless it is admin, the roles the person running the job is a tenant of + are added to it. @@ -440,6 +465,29 @@ {/each} + + + No default role + + + + Jobs that name no role get only the roles of the person running them. + + + +
+ { + defaultRoleId = NO_DEFAULT_DATATABLE_ROLE + }} + /> +
+
+ +
{#if editable} @@ -466,6 +514,47 @@ {/if} + + {#if adminOpenToEveryone} + + Every member of the workspace may use admin{defaultsToAdmin + ? ', and a job that names no role connects as it, exactly as with roles off' + : ' by naming it'}. admin owns every table: it reads every row and ignores grants + and row-level security. Once the other roles are set up, remove Everyone from admin's + tenants{defaultsToAdmin ? ' and choose another default, or none' : ''}. + + {/if} + {#if defaultsToAdmin && hasOtherRoles && !adminOpenToEveryone} + + A job that names no role connects as admin when it runs as a workspace admin or as one + of admin's tenants, which covers every viewer of an app an admin published. admin owns + every table: it reads every row and ignores grants and row-level security. Everyone + else's jobs that name no role are refused. Choose another default, or none, to have + the roles above apply to them. + + {:else if !defaultsToAdmin} +
+ {#if hasNoDefault} + + A job that names no role connects with the roles here that the person running it + is a tenant of. It is refused when they have none, and so is a job with nobody + behind it: a schedule, a trigger, an anonymous app viewer. + + {:else} + + A job that names no role connects as + {defaultRoleName} when it may use it, plus every other + role here that the person running it is a tenant of. With neither, it is refused. + + {/if} + A job that names a role connects as that role alone. + + A table created over a connection that holds several roles belongs to that + combination of roles alone. Run SET ROLE to one role before + creating it, or name a role. + +
+ {/if} {/if}
diff --git a/frontend/src/lib/utils_draft_deploy.ts b/frontend/src/lib/utils_draft_deploy.ts index df04bb383a..611d5b96e8 100644 --- a/frontend/src/lib/utils_draft_deploy.ts +++ b/frontend/src/lib/utils_draft_deploy.ts @@ -538,8 +538,14 @@ export async function deployDraft( // no draft. `draft_path` and `summary` are draft-only fields mirrored onto // the App value (the editor drops them on deploy), so strip them from the // value and apply them as the deploy path / summary column. + // `parent_version` is the draft's stale-check base, never deployed. const draft = r.draft as Record | undefined - const { draft_path: draftPath, summary: draftSummary, ...appValue } = draft ?? r.value ?? {} + const { + draft_path: draftPath, + summary: draftSummary, + parent_version: _pv, + ...appValue + } = draft ?? r.value ?? {} // Policy isn't carried in the app draft, so it comes from the deployed app // (or a default). custom_path requires admin on update; non-admins send // undefined so the backend preserves the existing route. The draft has no diff --git a/frontend/src/routes/(root)/(logged)/+layout.svelte b/frontend/src/routes/(root)/(logged)/+layout.svelte index 5edf490220..868d78c73b 100644 --- a/frontend/src/routes/(root)/(logged)/+layout.svelte +++ b/frontend/src/routes/(root)/(logged)/+layout.svelte @@ -110,7 +110,7 @@ import { isGlobalAiEnabled, sessionsAllowedFor } from '$lib/components/copilot/chat/global/gate' import { copilotInfo } from '$lib/aiStore' import { - isPageItemListPath, + isPreviewListPath, parsePreviewItemRoute, barePath } from '$lib/components/sessions/previewPaths' @@ -467,7 +467,7 @@ } // A list page mounts in process too, where its rows open as tabs of their own. const listUrl = navigation.to?.url - if (listUrl && isPageItemListPath(barePath(listUrl.pathname))) { + if (listUrl && isPreviewListPath(barePath(listUrl.pathname))) { navigation.cancel() const u = new URL(listUrl.href) u.searchParams.delete('nomenubar') diff --git a/frontend/src/routes/(root)/(logged)/run/[...run]/+page.svelte b/frontend/src/routes/(root)/(logged)/run/[...run]/+page.svelte index 48931c7871..ca5d8c83ad 100644 --- a/frontend/src/routes/(root)/(logged)/run/[...run]/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/run/[...run]/+page.svelte @@ -1,1373 +1,13 @@ - - - - -{#if (job?.job_kind == 'flow' || isFlowPreview(job?.job_kind)) && job?.['running'] && job?.parent_job == undefined} - - - {#snippet actions()} -
-
- -
- -
- {/snippet} -

-					
-			
-
-
-{/if} -{#if !job || (job?.job_kind != 'flow' && job?.job_kind != 'flownode' && job?.job_kind != 'flowpreview')} - -{/if} - -{#snippet flowRestartButton( - state: ReturnType, - step: string, - triggerStyle: 'button' | 'link' -)} - {#if job} - { - goto('/run/' + newJobId + '?workspace=' + $workspaceStore) - }} - flowPath={job.script_path} - flowVersionId={job.script_hash ? parseInt(job.script_hash, 16) : undefined} - disabled={!$enterpriseLicense} - enterpriseOnly={!$enterpriseLicense} - {triggerStyle} - unifiedSize="sm" - /> - {/if} -{/snippet} - - - - - -{#if loadError?.status === 403} -
-
- -
-

- This run exists in {$workspaceStore}, but you don't - have permission to view it. -

-

- Ask a colleague who can see it to open the run and use the - Share button to send you a read-only link. Opening that - link will grant you access to this run (and its steps). -

-
-
-
- -
-
-
-{:else if notfound || (job?.workspace_id != undefined && $workspaceStore != undefined && job?.workspace_id != $workspaceStore)} - {@const currentWorkspace = $userWorkspaces.find((w) => w.id === $workspaceStore)} - {@const otherWorkspaces = $userWorkspaces.filter((w) => w.id !== $workspaceStore)} -
-
-
- -
-

Run not found

-

- No run with ID - {page.params.run} - exists in - {currentWorkspace?.name ?? $workspaceStore}. It may belong to another workspace, or it may have been deleted. -

-
- - {#if otherWorkspaces.length > 0} -
-

Look in another workspace

-
- {#each otherWorkspaces as workspace (workspace.id)} - - {/each} -
-
- {/if} - -
- -
-
-{:else} - - - - - {#snippet runActions()} - {@const isScript = job?.job_kind === 'script'} - {@const isHubFlowPreview = isFlowPreview(job?.job_kind) && isHubFlowPath(job?.script_path)} - {@const runsHref = `/runs/${job?.script_path}${!isScript ? '?jobKind=flow' : ''}`} - {#if job && 'deleted' in job && !job?.deleted && ($superadmin || ($userStore?.is_admin ?? false))} - { - job?.id && deleteCompletedJob(job.id) - }, - type: 'delete' - } - ]} - > - {#snippet buttonReplacement()} - - {/if} - {/if} - {#if job} - job && shareReadLink(job.id) - }, - { - displayName: 'Copy public link', - icon: Globe, - disabled: !canSharePublicly, - tooltip: canSharePublicly - ? "Read-only link that anyone on the internet can open, without logging in. It shows a minimal version of this page: this run's inputs, result and logs, and for a flow its graph plus every step's inputs, result, logs and code. The link cannot be revoked." - : 'Sharing a run publicly is restricted in this workspace. Ask an admin to share it, or to grant you a bypass on the ruleset.', - action: () => job && sharePublicLink(job.id) - } - ]} - > - {#snippet buttonReplacement()} - - {/snippet} - - {/if} - {@const stem = job?.job_kind === 'script_hub' ? '/scripts' : `/${job?.job_kind}s`} - {@const viewHref = `${stem}/get/${isScript ? job?.script_hash : job?.script_path}`} - {#if (job?.job_kind == 'flow' || isFlowPreview(job?.job_kind)) && job?.['running'] && job?.parent_job == undefined} -
- { - debugInfo() - } - } - ]} - class="h-auto" - > - {#snippet buttonReplacement()} - - {/snippet} - -
- {/if} - {#if isFlowPreview(job?.job_kind) || isScriptPreview(job?.job_kind)} - - {/if} - {#if persistentScriptDefinition !== undefined} - - {/if} - {#if canRunNow} - - {/if} - {#if job && job?.type != 'CompletedJob' && (!job?.schedule_path || job?.['running'] == true)} - {#if !forceCancel} - - {:else} - - {/if} - {/if} - {#if job?.schedule_path} - - {/if} - {#if restartStep !== undefined && canRestart(restart)} - {@render flowRestartButton(restart, restartStep, 'button')} - {/if} - {#if job?.job_kind === 'script' || job?.job_kind === 'script_hub' || job?.job_kind === 'flow'} - - {/if} - {#if job?.job_kind === 'script' || job?.job_kind === 'flow'} - {#if !$userStore?.operator} - {#if editRights.canEditItem(isScript ? 'script' : 'flow', job?.script_path ?? '', {})} - - {#if showEditButton} - - - {/if} - {/if} - {#if !showEditButton && !isCloudHosted() && editInForkAllowed($workspaceStore, $userWorkspaces)} - - {/if} - {/if} - {/if} - {#if job?.job_kind === 'script' || job?.job_kind === 'script_hub' || job?.job_kind === 'flow'} - - {/if} - {/snippet} -
- -
- - {#if job} - { - persistentScriptDrawer?.open?.(persistentScriptDefinition) - }} - {concurrencyKey} - /> - {/if} -
- {#if job?.['deleted']} -
- - The content of this run was deleted (by an admin, no less) - -
-
- {/if} - - - {#if job?.job_kind === 'flow' || job?.job_kind === 'flowpreview'} -
- - {#snippet errorAction()} - - {#if failedTopLevelStep && failedTopLevelStep !== AGENT_STEP_ID} - at step {failedTopLevelStep} - {#if $enterpriseLicense && canRestart(failedRestart)} - · - {@render flowRestartButton(failedRestart, failedTopLevelStep, 'link')} - on this flow version, or on a new one after you fix it - {/if} - {/if} - {/snippet} - - {#if suspendStatus} - - {/if} -
- {/if} - - -
-
Inputs
-
- - {#if job && currentJobIsLongRunning && showExplicitProgressTip && !scriptProgress && 'running' in job} - - - - {/if} -
-
- - {#if job} - - {/if} - - {#if isNotFlow(job?.job_kind)} - {#if ['python3', 'bun', 'deno'].includes(job?.language ?? '') && (job?.job_kind == 'script' || isScriptPreview(job?.job_kind))} - - {/if} -
- {#if job?.workflow_as_code_status && job.job_kind !== 'aiagent'} -
-

Workflow Timeline

-
- -
-
- {/if} - {#if scriptProgress} - - {/if} - - - {#if job?.language === 'dbt' && job?.script_path} -
-

Models

- -
- {/if} - - - {#if job} -
-

Result

-
- {#if job.result_stream || (job.type == 'CompletedJob' && job.result !== undefined)} - - {:else} -
No output is available yet
- {/if} -
-
- {#if job.id && job.workspace_id} - - - {/if} - {/if} - - -
- - - - - - {#if isScriptPreview(job?.job_kind)} - - {/if} - - - - {#if job} -
- {#if viewTab == 'logs'} -
- -
- {:else if viewTab == 'assets'} -
- -
- {:else if viewTab == 'traces'} -
- -
- {:else if viewTab == 'code'} - {#if job && 'raw_code' in job && job.raw_code} -
- -
- {:else if job} - No code available - {:else} - - {/if} - {:else if viewTab == 'stats'} -
- -
- {:else} -
Select a tab to view content
- {/if} -
- {/if} -
-
- {:else if !job?.['deleted']} -
- -
- {#if job?.id} - { - job = newJob - }} - onDone={({ job: newJob }) => { - job = newJob - }} - initialJob={job} - workspaceId={$workspaceStore} - bind:selectedJobStep - bind:suspendStatus - bind:isOwner - bind:localModuleStates={graphModuleStates} - bind:expandedSubflows - /> - {:else} - - {/if} -
- {/if} -
-{/if} - - diff --git a/frontend/src/routes/(root)/(logged)/sessions/+page.svelte b/frontend/src/routes/(root)/(logged)/sessions/+page.svelte index 114c6554aa..a1b2856323 100644 --- a/frontend/src/routes/(root)/(logged)/sessions/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/sessions/+page.svelte @@ -80,7 +80,7 @@ tabsToReload, viewerTabsToReload } from '$lib/components/sessions/previewReload' - import { isPageItemListPath, barePath } from '$lib/components/sessions/previewPaths' + import { isPreviewListPath, barePath } from '$lib/components/sessions/previewPaths' import { leafKeyFor, loadKind, @@ -928,7 +928,7 @@ // A preview frame navigating to a list page: re-point the active tab to the list // mounted in process, whose rows open as tabs of their own. if (d.type === 'wm.session.openList') { - if (typeof d.href !== 'string' || !isPageItemListPath(barePath(d.href))) return + if (typeof d.href !== 'string' || !isPreviewListPath(barePath(d.href))) return owner?.navigate({ type: 'page', href: d.href, label: previewLocationLabel(d.href) }) return } diff --git a/system_prompts/README.md b/system_prompts/README.md index cc16473c6c..a651c48692 100644 --- a/system_prompts/README.md +++ b/system_prompts/README.md @@ -46,6 +46,7 @@ a CLI-only workflow file (`flow-cli.md`, `script-cli.md`, `raw-app-cli.md`). | resources | `getResourcePrompt()` | `resources` | `resources.md` | | workflow-as-code | `getWorkflowAsCodePrompt(lang)` | `write-workflow-as-code` | `workflow-as-code.md`, the WAC SDK | | pipeline | `getPipelinePrompt()` | `write-pipeline` | `pipeline-base.md` | +| data tables | `getDatatablePrompt()` | `datatables` | `datatable.md` | | triggers, schedules, preview, CLI commands | — | same names | CLI only | The app chat embeds `RAW_APP_BASE` directly (`chat/app/core.ts`). diff --git a/system_prompts/auto-generated/index.d.ts b/system_prompts/auto-generated/index.d.ts index d77f699a33..8ba83f7f74 100644 --- a/system_prompts/auto-generated/index.d.ts +++ b/system_prompts/auto-generated/index.d.ts @@ -4,5 +4,6 @@ export declare function getFlowPrompt(): string; export declare function getResourcePrompt(): string; export declare function getRawAppPrompt(language?: string): string; export declare function getPipelinePrompt(): string; +export declare function getDatatablePrompt(): string; export declare function getDatatableSdkReference(language?: string): string; export declare function getWorkflowAsCodePrompt(language?: string): string; diff --git a/system_prompts/auto-generated/index.ts b/system_prompts/auto-generated/index.ts index ade95efe9e..60bb79b031 100644 --- a/system_prompts/auto-generated/index.ts +++ b/system_prompts/auto-generated/index.ts @@ -73,6 +73,13 @@ export function getPipelinePrompt(): string { ].filter(Boolean).join('\n\n'); } +// Helper for data table schema changes, migrations and permissions (chat consumers) +export function getDatatablePrompt(): string { + return [ + prompts.DATATABLE_BASE + ].filter(Boolean).join('\n\n'); +} + // Helper to get the datatable SQL SDK reference (wmill.datatable()). // Pass a language to get only that SDK; omit it to get both. export function getDatatableSdkReference(language?: string): string { diff --git a/system_prompts/auto-generated/prompts.d.ts b/system_prompts/auto-generated/prompts.d.ts index e74555504e..cfdde52495 100644 --- a/system_prompts/auto-generated/prompts.d.ts +++ b/system_prompts/auto-generated/prompts.d.ts @@ -5,6 +5,7 @@ export declare const FLOW_BASE: string; export declare const RESOURCES_BASE: string; export declare const RAW_APP_BASE: string; export declare const PIPELINE_BASE: string; +export declare const DATATABLE_BASE: string; export declare const WORKFLOW_AS_CODE_BASE: string; export declare const FLOW_CHAT_SPECIAL_MODULES: string; export declare const SDK_TYPESCRIPT: string; diff --git a/system_prompts/auto-generated/prompts.ts b/system_prompts/auto-generated/prompts.ts index 097a7af895..54d869f727 100644 --- a/system_prompts/auto-generated/prompts.ts +++ b/system_prompts/auto-generated/prompts.ts @@ -1131,6 +1131,7 @@ Data tables are PostgreSQL databases managed by Windmill. Backend runnables quer 3. **No DDL inside runnables**: runnables only read and write rows (SELECT, INSERT, UPDATE, DELETE) on existing tables. Never CREATE, ALTER or DROP a table from a runnable. 4. **Qualify table names**: an unqualified name means the \`public\` schema, so write every other table as \`schema.table\`, in table creation and queries alike. The app's \`data\` config sets the default datatable and schema its tables go in; use them consistently across runnables. 5. **Pass the role**: when the app's \`data.roles\` gives a data table a role, every \`wmill.datatable\` call on it passes that role (\`wmill.datatable('main', { role: 'analyst' })\` in TypeScript, \`wmill.datatable('main', role='analyst')\` in Python). The role only reaches what it was granted, so a query outside it fails with \`permission denied\`. +6. **Per-viewer rows**: a data table with no entry in \`data.roles\` is called with no role. Unless its default role is \`admin\`, the connection then holds the default role, if it has one, plus every role the person using the app is a tenant of, and is refused when that is none. Row-level security policies written as \`pg_has_role(current_user, '', 'member')\` return that viewer's rows with nothing added to the runnable. A role pins the connection to that one role for every viewer, so where rows should differ per viewer give the data table no \`data.roles\` entry. ### Querying in TypeScript (Bun/Deno) @@ -1337,6 +1338,47 @@ FROM ducklake.main.orders GROUP BY 1 \`\`\` `; +export const DATATABLE_BASE = `# Data tables: schema changes, migrations and permissions + +A data table is a PostgreSQL database managed by Windmill and shared by the whole workspace: scripts, flows and apps reach it by name, \`main\` being the default. It is configured in the workspace settings (Data Tables), never created by SQL. + +## Changing the schema + +A data table with migrations enabled keeps its schema as an ordered list of versioned SQL migrations, each an \`up\` and an optional \`down\`. That list is what git sync and a workspace merge carry, and what rebuilds the schema in another workspace. So first find out whether migrations are enabled on the data table: + +- **Enabled**: make every schema change (CREATE, ALTER, DROP, anything but row data) as a new migration. A change made with plain SQL exists in that one database only, and the history no longer describes it. +- **Not enabled**: schema changes are plain SQL. Enabling migrations is a workspace admin's decision, made in the Data Tables settings, not a step to take on the way to something else. + +Writing a migration: + +- Wrap the statements in \`BEGIN; ... END;\` so it applies atomically. Leave the frame out only for a statement that cannot run in a transaction, such as \`CREATE INDEX CONCURRENTLY\`: such a migration can fail half applied. +- Give it a \`down\` that undoes the \`up\` whenever the change is reversible. Without one it cannot be rolled back. +- Migrations apply oldest first. One that is applied is not edited: correct it with a new migration. +- Row data is not schema. Insert, update and delete rows with plain SQL, migrations or not. + +## A migration is not a draft + +A migration is saved to the workspace and changes the database as soon as it is applied, for everything that reads those tables. To try a schema out without touching tables already in use: + +- put the new tables in a schema of their own, created by the same migration (\`CREATE SCHEMA IF NOT EXISTS\`), and point what you are building at it. A raw app names its schema in its \`data\` config, which is why a new app gets a new schema by default; +- or work in a workspace fork whose data table was cloned when the fork was created ("Clone schema only" or "Clone schema and data"): migrations applied there change that copy alone, and reach the parent workspace when the fork is merged. A fork that kept the original data table points at the parent's database, so a migration applied in it changes the parent's tables at once. + +Altering or dropping a table other scripts or apps use is neither of those: say what depends on it before doing it. + +## Roles and permissions + +Data table roles are a self-hosted Enterprise Edition feature, for data tables on managed Postgres (the instance database or the external managed instance). Without them, and always on a data table backed by a Postgres resource, everyone who can use the data table reaches all of it: there is no per-user, per-schema or per-row access to promise. + +A data table under roles is reached as a Postgres role, and Postgres enforces what that role was granted: + +- roles are assigned, per data table, to users, groups or folders. \`admin\` is the connection the data table had before roles: it owns the existing objects and ignores grants; +- a connection either names a role (\`wmill.datatable('main', { role: 'analyst' })\`, \`role='analyst'\` in Python, \`datatable://main?role=analyst\`) and holds that role alone, or names none and is resolved from the data table's default role; +- \`permission denied\` means the role in use lacks a grant. Say which role and which object: the grant is a data table admin's to give, in the roles drawer of the Database Manager. Do not switch to \`admin\` to get past it; +- a migration runs as the role its SQL declares on its first line, \`-- role \`, the same one in its \`down\`. One that declares none runs as \`admin\`, which only those who may use \`admin\` can do. + +Read the Permissions section of https://www.windmill.dev/docs/core_concepts/persistent_storage/data_tables before designing who reaches what: it covers tenants, grants, the default role and the roles a connection that names none ends up holding, rows that differ per viewer through row-level security, and how forks and raw apps behave under roles. +`; + export const WORKFLOW_AS_CODE_BASE = `# Windmill Workflow-as-Code Writing Guide ## Scope diff --git a/system_prompts/auto-generated/skills/datatables/SKILL.md b/system_prompts/auto-generated/skills/datatables/SKILL.md new file mode 100644 index 0000000000..1c1cb9eede --- /dev/null +++ b/system_prompts/auto-generated/skills/datatables/SKILL.md @@ -0,0 +1,46 @@ +--- +name: datatables +description: MUST use when changing a data table's schema, writing or running its migrations, or working with its roles and permissions. +--- + +# Data tables: schema changes, migrations and permissions + +A data table is a PostgreSQL database managed by Windmill and shared by the whole workspace: scripts, flows and apps reach it by name, `main` being the default. It is configured in the workspace settings (Data Tables), never created by SQL. + +## Changing the schema + +A data table with migrations enabled keeps its schema as an ordered list of versioned SQL migrations, each an `up` and an optional `down`. That list is what git sync and a workspace merge carry, and what rebuilds the schema in another workspace. So first find out whether migrations are enabled on the data table: + +- **Enabled**: make every schema change (CREATE, ALTER, DROP, anything but row data) as a new migration. A change made with plain SQL exists in that one database only, and the history no longer describes it. +- **Not enabled**: schema changes are plain SQL. Enabling migrations is a workspace admin's decision, made in the Data Tables settings, not a step to take on the way to something else. + +Writing a migration: + +- Wrap the statements in `BEGIN; ... END;` so it applies atomically. Leave the frame out only for a statement that cannot run in a transaction, such as `CREATE INDEX CONCURRENTLY`: such a migration can fail half applied. +- Give it a `down` that undoes the `up` whenever the change is reversible. Without one it cannot be rolled back. +- Migrations apply oldest first. One that is applied is not edited: correct it with a new migration. +- Row data is not schema. Insert, update and delete rows with plain SQL, migrations or not. + +`wmill datatable migrate new ` scaffolds `migrations/datatable//_.up.sql` and `.down.sql`, `wmill datatable migrate up` pushes the local files and applies what is pending, `wmill datatable migrate down` rolls back the most recent one and `wmill datatable migrate status` lists them. All take `-d ` and default to `main`. + +## A migration is not a draft + +A migration is saved to the workspace and changes the database as soon as it is applied, for everything that reads those tables. To try a schema out without touching tables already in use: + +- put the new tables in a schema of their own, created by the same migration (`CREATE SCHEMA IF NOT EXISTS`), and point what you are building at it. A raw app names its schema in its `data` config, which is why a new app gets a new schema by default; +- or work in a workspace fork whose data table was cloned when the fork was created ("Clone schema only" or "Clone schema and data"): migrations applied there change that copy alone, and reach the parent workspace when the fork is merged. A fork that kept the original data table points at the parent's database, so a migration applied in it changes the parent's tables at once. + +Altering or dropping a table other scripts or apps use is neither of those: say what depends on it before doing it. + +## Roles and permissions + +Data table roles are a self-hosted Enterprise Edition feature, for data tables on managed Postgres (the instance database or the external managed instance). Without them, and always on a data table backed by a Postgres resource, everyone who can use the data table reaches all of it: there is no per-user, per-schema or per-row access to promise. + +A data table under roles is reached as a Postgres role, and Postgres enforces what that role was granted: + +- roles are assigned, per data table, to users, groups or folders. `admin` is the connection the data table had before roles: it owns the existing objects and ignores grants; +- a connection either names a role (`wmill.datatable('main', { role: 'analyst' })`, `role='analyst'` in Python, `datatable://main?role=analyst`) and holds that role alone, or names none and is resolved from the data table's default role; +- `permission denied` means the role in use lacks a grant. Say which role and which object: the grant is a data table admin's to give, in the roles drawer of the Database Manager. Do not switch to `admin` to get past it; +- a migration runs as the role its SQL declares on its first line, `-- role `, the same one in its `down`. One that declares none runs as `admin`, which only those who may use `admin` can do. + +Read the Permissions section of https://www.windmill.dev/docs/core_concepts/persistent_storage/data_tables before designing who reaches what: it covers tenants, grants, the default role and the roles a connection that names none ends up holding, rows that differ per viewer through row-level security, and how forks and raw apps behave under roles. diff --git a/system_prompts/auto-generated/skills/raw-app/SKILL.md b/system_prompts/auto-generated/skills/raw-app/SKILL.md index 7afcbd2969..bd21352995 100644 --- a/system_prompts/auto-generated/skills/raw-app/SKILL.md +++ b/system_prompts/auto-generated/skills/raw-app/SKILL.md @@ -188,7 +188,7 @@ data: - `/
` — Specific table in public schema - `/:
` — Table in specific schema -**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. `roles` records the role the app uses each datatable through; the app's code must pass the same role: `wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python. A datatable without an entry is used as its default role. +**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. `roles` records the role the app uses each datatable through; the app's code must pass the same role: `wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python. A datatable without an entry is used as its default role, if it has one, plus the roles of whoever is using the app unless that default is `admin`. ## SQL Migrations (sql_to_apply/) @@ -494,6 +494,7 @@ Data tables are PostgreSQL databases managed by Windmill. Backend runnables quer 3. **No DDL inside runnables**: runnables only read and write rows (SELECT, INSERT, UPDATE, DELETE) on existing tables. Never CREATE, ALTER or DROP a table from a runnable. 4. **Qualify table names**: an unqualified name means the `public` schema, so write every other table as `schema.table`, in table creation and queries alike. The app's `data` config sets the default datatable and schema its tables go in; use them consistently across runnables. 5. **Pass the role**: when the app's `data.roles` gives a data table a role, every `wmill.datatable` call on it passes that role (`wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python). The role only reaches what it was granted, so a query outside it fails with `permission denied`. +6. **Per-viewer rows**: a data table with no entry in `data.roles` is called with no role. Unless its default role is `admin`, the connection then holds the default role, if it has one, plus every role the person using the app is a tenant of, and is refused when that is none. Row-level security policies written as `pg_has_role(current_user, '', 'member')` return that viewer's rows with nothing added to the runnable. A role pins the connection to that one role for every viewer, so where rows should differ per viewer give the data table no `data.roles` entry. `wmill datatable list` lists the workspace's data tables. Create or change tables with a migration in `sql_to_apply/` (see "SQL Migrations" above), then add them to `data.tables` in `raw_app.yaml`. diff --git a/system_prompts/base/datatable.md b/system_prompts/base/datatable.md new file mode 100644 index 0000000000..c90fb5e349 --- /dev/null +++ b/system_prompts/base/datatable.md @@ -0,0 +1,43 @@ +# Data tables: schema changes, migrations and permissions + +A data table is a PostgreSQL database managed by Windmill and shared by the whole workspace: scripts, flows and apps reach it by name, `main` being the default. It is configured in the workspace settings (Data Tables), never created by SQL. + +## Changing the schema + +A data table with migrations enabled keeps its schema as an ordered list of versioned SQL migrations, each an `up` and an optional `down`. That list is what git sync and a workspace merge carry, and what rebuilds the schema in another workspace. So first find out whether migrations are enabled on the data table: + +- **Enabled**: make every schema change (CREATE, ALTER, DROP, anything but row data) as a new migration. A change made with plain SQL exists in that one database only, and the history no longer describes it. +- **Not enabled**: schema changes are plain SQL. Enabling migrations is a workspace admin's decision, made in the Data Tables settings, not a step to take on the way to something else. + +Writing a migration: + +- Wrap the statements in `BEGIN; ... END;` so it applies atomically. Leave the frame out only for a statement that cannot run in a transaction, such as `CREATE INDEX CONCURRENTLY`: such a migration can fail half applied. +- Give it a `down` that undoes the `up` whenever the change is reversible. Without one it cannot be rolled back. +- Migrations apply oldest first. One that is applied is not edited: correct it with a new migration. +- Row data is not schema. Insert, update and delete rows with plain SQL, migrations or not. + + +`wmill datatable migrate new ` scaffolds `migrations/datatable//_.up.sql` and `.down.sql`, `wmill datatable migrate up` pushes the local files and applies what is pending, `wmill datatable migrate down` rolls back the most recent one and `wmill datatable migrate status` lists them. All take `-d ` and default to `main`. + + +## A migration is not a draft + +A migration is saved to the workspace and changes the database as soon as it is applied, for everything that reads those tables. To try a schema out without touching tables already in use: + +- put the new tables in a schema of their own, created by the same migration (`CREATE SCHEMA IF NOT EXISTS`), and point what you are building at it. A raw app names its schema in its `data` config, which is why a new app gets a new schema by default; +- or work in a workspace fork whose data table was cloned when the fork was created ("Clone schema only" or "Clone schema and data"): migrations applied there change that copy alone, and reach the parent workspace when the fork is merged. A fork that kept the original data table points at the parent's database, so a migration applied in it changes the parent's tables at once. + +Altering or dropping a table other scripts or apps use is neither of those: say what depends on it before doing it. + +## Roles and permissions + +Data table roles are a self-hosted Enterprise Edition feature, for data tables on managed Postgres (the instance database or the external managed instance). Without them, and always on a data table backed by a Postgres resource, everyone who can use the data table reaches all of it: there is no per-user, per-schema or per-row access to promise. + +A data table under roles is reached as a Postgres role, and Postgres enforces what that role was granted: + +- roles are assigned, per data table, to users, groups or folders. `admin` is the connection the data table had before roles: it owns the existing objects and ignores grants; +- a connection either names a role (`wmill.datatable('main', { role: 'analyst' })`, `role='analyst'` in Python, `datatable://main?role=analyst`) and holds that role alone, or names none and is resolved from the data table's default role; +- `permission denied` means the role in use lacks a grant. Say which role and which object: the grant is a data table admin's to give, in the roles drawer of the Database Manager. Do not switch to `admin` to get past it; +- a migration runs as the role its SQL declares on its first line, `-- role `, the same one in its `down`. One that declares none runs as `admin`, which only those who may use `admin` can do. + +Read the Permissions section of https://www.windmill.dev/docs/core_concepts/persistent_storage/data_tables before designing who reaches what: it covers tenants, grants, the default role and the roles a connection that names none ends up holding, rows that differ per viewer through row-level security, and how forks and raw apps behave under roles. diff --git a/system_prompts/base/raw-app-cli.md b/system_prompts/base/raw-app-cli.md index f12762ff5f..e22de102f6 100644 --- a/system_prompts/base/raw-app-cli.md +++ b/system_prompts/base/raw-app-cli.md @@ -183,7 +183,7 @@ data: - `/
` — Specific table in public schema - `/:
` — Table in specific schema -**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. `roles` records the role the app uses each datatable through; the app's code must pass the same role: `wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python. A datatable without an entry is used as its default role. +**Roles:** when a datatable is under roles, its queries run as a role, which only reaches what it was granted. `roles` records the role the app uses each datatable through; the app's code must pass the same role: `wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python. A datatable without an entry is used as its default role, if it has one, plus the roles of whoever is using the app unless that default is `admin`. ## SQL Migrations (sql_to_apply/) diff --git a/system_prompts/base/raw-app.md b/system_prompts/base/raw-app.md index 23b09f1996..0979000b2a 100644 --- a/system_prompts/base/raw-app.md +++ b/system_prompts/base/raw-app.md @@ -264,6 +264,7 @@ Data tables are PostgreSQL databases managed by Windmill. Backend runnables quer 3. **No DDL inside runnables**: runnables only read and write rows (SELECT, INSERT, UPDATE, DELETE) on existing tables. Never CREATE, ALTER or DROP a table from a runnable. 4. **Qualify table names**: an unqualified name means the `public` schema, so write every other table as `schema.table`, in table creation and queries alike. The app's `data` config sets the default datatable and schema its tables go in; use them consistently across runnables. 5. **Pass the role**: when the app's `data.roles` gives a data table a role, every `wmill.datatable` call on it passes that role (`wmill.datatable('main', { role: 'analyst' })` in TypeScript, `wmill.datatable('main', role='analyst')` in Python). The role only reaches what it was granted, so a query outside it fails with `permission denied`. +6. **Per-viewer rows**: a data table with no entry in `data.roles` is called with no role. Unless its default role is `admin`, the connection then holds the default role, if it has one, plus every role the person using the app is a tenant of, and is refused when that is none. Row-level security policies written as `pg_has_role(current_user, '', 'member')` return that viewer's rows with nothing added to the runnable. A role pins the connection to that one role for every viewer, so where rows should differ per viewer give the data table no `data.roles` entry. `wmill datatable list` lists the workspace's data tables. Create or change tables with a migration in `sql_to_apply/` (see "SQL Migrations" above), then add them to `data.tables` in `raw_app.yaml`. diff --git a/system_prompts/generate.py b/system_prompts/generate.py index 918deef0fb..055f03d543 100644 --- a/system_prompts/generate.py +++ b/system_prompts/generate.py @@ -1696,6 +1696,12 @@ TOPICS: dict[str, dict] = { 'chat_helper': 'getPipelinePrompt', 'parts': ['pipeline-base.md'], }, + 'datatable': { + 'skill': 'datatables', + 'description': "MUST use when changing a data table's schema, writing or running its migrations, or working with its roles and permissions.", + 'chat_helper': 'getDatatablePrompt', + 'parts': ['datatable.md'], + }, 'cli_commands': { 'skill': 'cli-commands', 'description': 'MUST use when using the CLI, including debugging job failures and inspecting run history via `wmill job`.', @@ -1715,6 +1721,7 @@ CHAT_CONSTANTS = { 'resources.md': 'RESOURCES_BASE', 'raw-app.md': 'RAW_APP_BASE', 'pipeline-base.md': 'PIPELINE_BASE', + 'datatable.md': 'DATATABLE_BASE', 'workflow-as-code.md': 'WORKFLOW_AS_CODE_BASE', 'flow-chat-special-modules.md': 'FLOW_CHAT_SPECIAL_MODULES', } @@ -2650,6 +2657,11 @@ export function getPipelinePrompt(): string { __RETURN_pipeline__ } +// Helper for data table schema changes, migrations and permissions (chat consumers) +export function getDatatablePrompt(): string { + __RETURN_datatable__ +} + // Helper to get the datatable SQL SDK reference (wmill.datatable()). // Pass a language to get only that SDK; omit it to get both. export function getDatatableSdkReference(language?: string): string { @@ -2711,6 +2723,7 @@ export declare function getFlowPrompt(): string; export declare function getResourcePrompt(): string; export declare function getRawAppPrompt(language?: string): string; export declare function getPipelinePrompt(): string; +export declare function getDatatablePrompt(): string; export declare function getDatatableSdkReference(language?: string): string; export declare function getWorkflowAsCodePrompt(language?: string): string; """