From 285a78752a23aa467f9a82868d784599793d3a1f Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 19 May 2026 16:37:08 +0000 Subject: [PATCH 01/29] feat(indexer): observability for unavailable search index (WIN-1956) (#9239) * [ee] feat(indexer): observability for unavailable search index A user hit `Not found: There is no index reader to search from` when searching service logs and could not tell whether it was a config error or a bug, and asked for visibility into the indexer status (WIN-1956). Backend (EE companion PR): - Replace the opaque error with an actionable message explaining the likely causes (indexer disabled, still starting, or blocked acquiring the indexer lock) and pointing to the status panel. - Add a coarse `state` (running | stale | never_started) to `/indexer/status`, derived from the lock row, distinguishing a never-configured indexer from a stale/blocked one. Frontend: - Instance Settings > Indexer now shows Running / Stale / Not started with a tooltip explaining what to check for each. - Service logs search now catches failures and shows an inline, actionable Alert instead of an unhandled rejection. Fixes WIN-1956 Co-Authored-By: Claude Opus 4.7 (1M context) * chore: update ee-repo-ref to 017d36418a65ce5c840c502e3174df0c393612ba This commit updates the EE repository reference after PR #580 was merged in windmill-ee-private. Previous ee-repo-ref: 18b7e1b30a1ff582c4a072580bbb8aec34e22cdc New ee-repo-ref: 017d36418a65ce5c840c502e3174df0c393612ba Automated by sync-ee-ref workflow. * fix(indexer): address review nits - IndexerMemorySettings: older backends without `state` reporting `is_alive: false` now show "Stopped" (red) again instead of falling through to "Unknown" (codex/cubic P2). - ServiceLogsInner: clear stale logs/counts on a failed search so the error isn't shown alongside results from a previous query (codex P2). Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) Co-authored-by: windmill-internal-app[bot] --- backend/ee-repo-ref.txt | 2 +- backend/windmill-api/openapi.yaml | 12 +++ .../lib/components/ServiceLogsInner.svelte | 90 ++++++++++++------- .../IndexerMemorySettings.svelte | 55 +++++++++--- 4 files changed, 113 insertions(+), 46 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 7cda45d507..c64abf81cd 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -ec3cd353245e1cdf6a290528dbd7f2ac2498386c +017d36418a65ce5c840c502e3174df0c393612ba diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index bbe8143d63..00fd942598 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -19838,6 +19838,12 @@ paths: properties: is_alive: type: boolean + state: + type: string + enum: + - running + - stale + - never_started last_locked_at: type: string format: date-time @@ -19859,6 +19865,12 @@ paths: properties: is_alive: type: boolean + state: + type: string + enum: + - running + - stale + - never_started last_locked_at: type: string format: date-time diff --git a/frontend/src/lib/components/ServiceLogsInner.svelte b/frontend/src/lib/components/ServiceLogsInner.svelte index df0298f579..bad18661b6 100644 --- a/frontend/src/lib/components/ServiceLogsInner.svelte +++ b/frontend/src/lib/components/ServiceLogsInner.svelte @@ -16,7 +16,7 @@ import { Loader2 } from 'lucide-svelte' import { copyToClipboard, scroll_into_view_if_needed_polyfill, truncateRev } from '$lib/utils' import LogSnippetViewer from './LogSnippetViewer.svelte' - import { Button, Drawer, DrawerContent } from './common' + import { Alert, Button, Drawer, DrawerContent } from './common' import ClipboardCopy from 'lucide-svelte/icons/clipboard-copy' import { AnsiUp } from 'ansi_up' import SplitPanesOrColumnOnMobile from './splitPanes/SplitPanesOrColumnOnMobile.svelte' @@ -303,6 +303,7 @@ let countsPerHost: any = $state() let sumOtherDocCount: number = $state(0) + let searchError: string | undefined = $state(undefined) async function searchLogs( searchTerm: string, @@ -324,6 +325,7 @@ sumOtherDocCount = 0 loadingLogs = false loadingLogCounts = false + searchError = undefined return } timeout && clearTimeout(timeout) @@ -332,39 +334,52 @@ loadingLogs = true debounceTimeout && clearTimeout(debounceTimeout) debounceTimeout = setTimeout(async () => { - if (allLogs) { - const countLogsResponse = await IndexSearchService.countSearchLogsIndex({ - searchQuery: searchTerm, - minTs, - maxTs - }) - const res = (countLogsResponse.count_per_host as any)['count_per_host'] - const buckets = res['buckets'] - sumOtherDocCount = res['sum_other_doc_count'] - countsPerHost = new Map(buckets.map(({ key, doc_count }) => [key, doc_count])) - countsPerHost = buckets.reduce( - (acc: any, { key, doc_count }) => { - acc[key] = { doc_count } - return acc - }, - {} as Record - ) - queryParseErrors = countLogsResponse.query_parse_errors ?? [] + searchError = undefined + try { + if (allLogs) { + const countLogsResponse = await IndexSearchService.countSearchLogsIndex({ + searchQuery: searchTerm, + minTs, + maxTs + }) + const res = (countLogsResponse.count_per_host as any)['count_per_host'] + const buckets = res['buckets'] + sumOtherDocCount = res['sum_other_doc_count'] + countsPerHost = new Map(buckets.map(({ key, doc_count }) => [key, doc_count])) + countsPerHost = buckets.reduce( + (acc: any, { key, doc_count }) => { + acc[key] = { doc_count } + return acc + }, + {} as Record + ) + queryParseErrors = countLogsResponse.query_parse_errors ?? [] + } + + if (selected) { + logs = await IndexSearchService.searchLogsIndex({ + searchQuery: searchTerm, + mode: selected.mode, + workerGroup: selected.workerGroup != '' ? selected.workerGroup : undefined, + hostname: selected.hostname, + minTs, + maxTs + }) + } + } catch (e) { + const message = e?.body ?? e?.message ?? 'Unknown error' + searchError = message + // Drop any results from a previous successful search so the error + // isn't shown alongside stale matches/counts for the old query. + logs = undefined + countsPerHost = undefined + sumOtherDocCount = 0 + sendUserToast('Service logs search failed: ' + message, true) + console.error(e) + } finally { + loadingLogs = false loadingLogCounts = false } - - if (selected) { - logs = await IndexSearchService.searchLogsIndex({ - searchQuery: searchTerm, - mode: selected.mode, - workerGroup: selected.workerGroup != '' ? selected.workerGroup : undefined, - hostname: selected.hostname, - minTs, - maxTs - }) - } - - loadingLogs = false }, debouncePeriod) } @@ -520,14 +535,21 @@ options={{ right: 'auto-refresh' }} /> + {#if searchError} +
+ + {searchError} + +
+ {/if} {#if allLogs == undefined}
{:else if Object.keys(allLogs).length == 0}
No logs Search only covers a recent time window, configurable in instance settings - under Indexer.Search only covers a recent time window, configurable in instance settings under + Indexer.
{:else if minTs && maxTs} diff --git a/frontend/src/lib/components/instanceSettings/IndexerMemorySettings.svelte b/frontend/src/lib/components/instanceSettings/IndexerMemorySettings.svelte index 077c1c2a12..73f356c8c1 100644 --- a/frontend/src/lib/components/instanceSettings/IndexerMemorySettings.svelte +++ b/frontend/src/lib/components/instanceSettings/IndexerMemorySettings.svelte @@ -53,6 +53,43 @@ return `${diffHours}h ago` } + type IndexerEntry = GetIndexerStatusResponse['job_indexer'] + + function statusDescriptor(entry: IndexerEntry): { + label: string + dot: string + text: string + hint?: string + } { + // Backends that predate the `state` field only report `is_alive`: keep the + // prior Running / Stopped signal instead of falling through to "Unknown". + if (entry?.state == null) { + return entry?.is_alive + ? { label: 'Running', dot: 'bg-green-500', text: 'text-green-600 dark:text-green-400' } + : { label: 'Stopped', dot: 'bg-red-500', text: 'text-red-600 dark:text-red-400' } + } + switch (entry.state) { + case 'running': + return { label: 'Running', dot: 'bg-green-500', text: 'text-green-600 dark:text-green-400' } + case 'stale': + return { + label: 'Stale', + dot: 'bg-yellow-500', + text: 'text-yellow-600 dark:text-yellow-400', + hint: 'The indexer acquired its lock before but has not refreshed it recently. It may have crashed, be blocked, or be handing over during a deployment. Check the indexer container logs.' + } + case 'never_started': + return { + label: 'Not started', + dot: 'bg-red-500', + text: 'text-red-600 dark:text-red-400', + hint: 'No instance has ever run this indexer. Make sure an instance is running in "indexer" mode (or a server with the indexer addon enabled) and that you are on Enterprise.' + } + default: + return { label: 'Unknown', dot: 'bg-gray-400', text: 'text-tertiary' } + } + } + async function loadStatus() { statusLoading = true statusError = false @@ -167,20 +204,16 @@ {#if status}
{#each [{ label: 'Job indexer', entry: status.job_indexer }, { label: 'Service log indexer', entry: status.log_indexer }] as { label, entry } (label)} + {@const d = statusDescriptor(entry)}
- + {label}: - - {entry?.is_alive ? 'Running' : 'Stopped'} + + {d.label} + {#if d.hint} + {d.hint} + {/if} {#if entry?.last_locked_at} Last active: {formatTimeAgo(entry.last_locked_at)} From 355c8379440eb67cd215354dee86e1c63ac3155b Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 19 May 2026 16:52:08 +0000 Subject: [PATCH 02/29] test: provision migrated db for mutual-resource recursion test (WIN-1958) (#9247) Co-authored-by: Claude Opus 4.7 (1M context) --- backend/windmill-store/src/resources.rs | 26 ++++++++++--------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/backend/windmill-store/src/resources.rs b/backend/windmill-store/src/resources.rs index b8d0bdcad9..5c17308bbf 100644 --- a/backend/windmill-store/src/resources.rs +++ b/backend/windmill-store/src/resources.rs @@ -2634,12 +2634,14 @@ mod tests { // Regression test for WIN-1957: two resources whose values reference each // other via `$res:` must NOT recurse forever (stack overflow / process // crash). With the depth guard the resolution terminates with an error. - #[tokio::test] - async fn test_transform_json_value_mutual_resource_recursion_terminates() { - let db_url = std::env::var("DATABASE_URL") - .unwrap_or("postgres://postgres:changeme@localhost:5432/windmill".to_string()); - let pool = sqlx::PgPool::connect(&db_url).await.unwrap(); - + // + // This test needs the real `workspace`/`resource` schema, so it uses + // `#[sqlx::test]` which provisions a migrated ephemeral database per test + // (the bare `DATABASE_URL` database in CI has no migrations applied, which + // previously made the workspace INSERT panic with `relation "workspace" + // does not exist` — WIN-1958). + #[sqlx::test(migrations = "../migrations")] + async fn test_transform_json_value_mutual_resource_recursion_terminates(pool: DB) { let w_id = format!("dostest{}", Uuid::new_v4().simple()); sqlx::query("INSERT INTO workspace (id, name, owner) VALUES ($1, $1, 'test@windmill.dev')") @@ -2671,16 +2673,8 @@ mod tests { ) .await; - // Clean up before asserting so a failed assertion doesn't leave rows. - let _ = sqlx::query("DELETE FROM resource WHERE workspace_id = $1") - .bind(&w_id) - .execute(&pool) - .await; - let _ = sqlx::query("DELETE FROM workspace WHERE id = $1") - .bind(&w_id) - .execute(&pool) - .await; - + // The ephemeral test database is dropped automatically, so no manual + // row cleanup is required. let err = result.expect_err("mutually recursive resources should error, not crash"); assert!( err.to_string().contains("interpolation depth"), From 01bad16c0cc40fa64b2a72ccb8ded487c729cf35 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 19 May 2026 17:40:34 +0000 Subject: [PATCH 03/29] feat: add wmill protection-rules pull/push CLI commands (#9240) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add wmill protection-rules pull/push CLI commands Co-Authored-By: Claude Opus 4.7 (1M context) * refactor: use directional keys for protection-rules pull --json diff Co-Authored-By: Claude Opus 4.7 (1M context) * fix: address review — exit non-zero on failure, resolve override workspace key - failure paths in pull/push now exit 1 so CI/scripts detect failed reconciles - --override writes under the resolved workspace key (findWorkspaceByGitBranch), not the raw branch, so gitBranch-mapped entries aren't left inert - pull --replace clears a shadowing protectionRules override so top-level takes effect (was an infinite pull --diff loop) - push reports applied create/update/delete counts on partial failure and warns loudly when an empty list would wipe all backend rules Co-Authored-By: Claude Opus 4.7 (1M context) * fix: address review — dry-run pull --diff no longer writes; --promotion coherent - pull --diff returns before the no-wmill.yaml bootstrap, so a dry run never creates/mutates wmill.yaml - pull --promotion now writes/clears the promotion target's promotionOverrides (the same block getEffectiveSettings reads), instead of the current branch's regular overrides — read and write are now coherent Co-Authored-By: Claude Opus 4.7 (1M context) * refactor: move protection rules to a per-workspace protection-rules.yaml Replaces the wmill.yaml/SyncOptions integration (top-level + overrides + promotionOverrides) with a dedicated protection-rules.yaml keyed by workspace name. This removes the getEffectiveSettings layering that caused the override shadowing / promotion-coherence / dry-run bugs entirely. - protection-rules.yaml: { : ProtectionRuleEntry[] }, keys must match wmill.yaml 'workspaces' (source of truth for backend id/baseUrl/token) - commands reduced to: pull/push [workspace] | --all, with --dry-run - per-workspace auth resolved via tryResolveBranchWorkspace + setClient - push remains a full reconcile (create/update/delete) with delete confirm, empty-list wipe warning, partial-failure reporting, non-zero exit on failure - conf.ts reverted to main; SyncOptions no longer carries protectionRules Co-Authored-By: Claude Opus 4.7 (1M context) * fix: address review — honor explicit --base-url/--token in protection-rules configureClientForWorkspace bypassed the credential precedence other commands use: explicit --base-url/--token now work for stateless CI (no stored profile or wmill.yaml baseUrl needed), and an explicit --token overrides a stored profile's token. The backend workspace id still derives from the wmill.yaml mapping (feature invariant). Co-Authored-By: Claude Opus 4.7 (1M context) * fix: address cubic review — consistent status on partial --all failure cubic found that pull/push reported success:true while exiting non-zero on partial --all failures, and that the push command description was missing from the generated CLI docs. - pull/push now report success:false + partialFailure:true (and exit 1) when any --all workspace fails; success:true only on full success - .description() calls use single string literals (not + concatenation) so system_prompts/generate.py parses them; regenerated CLI docs Co-Authored-By: Claude Opus 4.7 (1M context) * fix: address review — --json-output must emit only JSON on stdout Codex flagged that workspace resolution (tryResolveBranchWorkspace's log.info) and push's empty-list delete warning print to stdout before the JSON payload, breaking machine callers. Silence human logs via log.setSilent(true) as the first action when --json-output is set (before readConfigFile / resolution); log.error still goes to stderr. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../commands/protection-rules/converter.ts | 132 +++++++++ cli/src/commands/protection-rules/file.ts | 129 +++++++++ cli/src/commands/protection-rules/index.ts | 2 + .../protection-rules/protection-rules.ts | 30 ++ cli/src/commands/protection-rules/pull.ts | 143 ++++++++++ cli/src/commands/protection-rules/push.ts | 262 ++++++++++++++++++ cli/src/commands/protection-rules/types.ts | 18 ++ cli/src/commands/protection-rules/utils.ts | 73 +++++ cli/src/guidance/skills.gen.ts | 14 + cli/src/main.ts | 3 + .../protection_rules_converter_unit.test.ts | 240 ++++++++++++++++ .../auto-generated/cli/cli-commands.md | 14 + system_prompts/auto-generated/prompts.ts | 14 + .../skills/cli-commands/SKILL.md | 14 + 14 files changed, 1088 insertions(+) create mode 100644 cli/src/commands/protection-rules/converter.ts create mode 100644 cli/src/commands/protection-rules/file.ts create mode 100644 cli/src/commands/protection-rules/index.ts create mode 100644 cli/src/commands/protection-rules/protection-rules.ts create mode 100644 cli/src/commands/protection-rules/pull.ts create mode 100644 cli/src/commands/protection-rules/push.ts create mode 100644 cli/src/commands/protection-rules/types.ts create mode 100644 cli/src/commands/protection-rules/utils.ts create mode 100644 cli/test/protection_rules_converter_unit.test.ts diff --git a/cli/src/commands/protection-rules/converter.ts b/cli/src/commands/protection-rules/converter.ts new file mode 100644 index 0000000000..0d3ece5e36 --- /dev/null +++ b/cli/src/commands/protection-rules/converter.ts @@ -0,0 +1,132 @@ +import { ProtectionRuleEntry } from "./types.ts"; +import { ProtectionRuleset } from "../../../gen/types.gen.ts"; + +// Reconciliation plan produced by diffing the local protection rules +// against the backend list. `toDelete` holds names present on the backend but +// absent from wmill.yaml (full-reconcile semantics). +export interface ProtectionRulesPlan { + toCreate: ProtectionRuleEntry[]; + toUpdate: ProtectionRuleEntry[]; + toDelete: string[]; + unchanged: string[]; +} + +function sortedUnique(arr: readonly string[]): string[] { + return [...new Set(arr)].sort(); +} + +export class ProtectionRulesConverter { + // Canonicalize a single rule so comparisons are insensitive to array order + // and duplicates. + static normalizeEntry(entry: ProtectionRuleEntry): ProtectionRuleEntry { + return { + name: entry.name, + rules: sortedUnique(entry.rules ?? []) as ProtectionRuleEntry["rules"], + bypass_groups: sortedUnique(entry.bypass_groups ?? []), + bypass_users: sortedUnique(entry.bypass_users ?? []), + }; + } + + // Canonicalize and sort a list of rules by name. + static normalizeList( + entries: ProtectionRuleEntry[] | undefined, + ): ProtectionRuleEntry[] { + return (entries ?? []) + .map((e) => ProtectionRulesConverter.normalizeEntry(e)) + .sort((a, b) => a.name.localeCompare(b.name)); + } + + // Convert a backend ProtectionRuleset response into the wmill.yaml shape + // (drops workspace_id, which is implied by the synced workspace). + static fromBackend(rulesets: ProtectionRuleset[]): ProtectionRuleEntry[] { + return ProtectionRulesConverter.normalizeList( + rulesets.map((r) => ({ + name: r.name, + rules: [...(r.rules ?? [])], + bypass_groups: [...(r.bypass_groups ?? [])], + bypass_users: [...(r.bypass_users ?? [])], + })), + ); + } + + static entriesEqual( + a: ProtectionRuleEntry, + b: ProtectionRuleEntry, + ): boolean { + const na = ProtectionRulesConverter.normalizeEntry(a); + const nb = ProtectionRulesConverter.normalizeEntry(b); + return ( + na.name === nb.name && + na.rules.length === nb.rules.length && + na.rules.every((v, i) => v === nb.rules[i]) && + na.bypass_groups.length === nb.bypass_groups.length && + na.bypass_groups.every((v, i) => v === nb.bypass_groups[i]) && + na.bypass_users.length === nb.bypass_users.length && + na.bypass_users.every((v, i) => v === nb.bypass_users[i]) + ); + } + + static listsEqual( + a: ProtectionRuleEntry[] | undefined, + b: ProtectionRuleEntry[] | undefined, + ): boolean { + const na = ProtectionRulesConverter.normalizeList(a); + const nb = ProtectionRulesConverter.normalizeList(b); + if (na.length !== nb.length) return false; + return na.every((entry, i) => + ProtectionRulesConverter.entriesEqual(entry, nb[i]) + ); + } + + // Compute the create/update/delete plan to make `backend` match `local`. + static computePlan( + local: ProtectionRuleEntry[] | undefined, + backend: ProtectionRuleEntry[] | undefined, + ): ProtectionRulesPlan { + const localByName = new Map( + ProtectionRulesConverter.normalizeList(local).map((e) => [e.name, e]), + ); + const backendByName = new Map( + ProtectionRulesConverter.normalizeList(backend).map((e) => [e.name, e]), + ); + + const plan: ProtectionRulesPlan = { + toCreate: [], + toUpdate: [], + toDelete: [], + unchanged: [], + }; + + for (const [name, entry] of localByName) { + const existing = backendByName.get(name); + if (!existing) { + plan.toCreate.push(entry); + } else if (!ProtectionRulesConverter.entriesEqual(entry, existing)) { + plan.toUpdate.push(entry); + } else { + plan.unchanged.push(name); + } + } + + for (const name of backendByName.keys()) { + if (!localByName.has(name)) { + plan.toDelete.push(name); + } + } + + plan.toCreate.sort((a, b) => a.name.localeCompare(b.name)); + plan.toUpdate.sort((a, b) => a.name.localeCompare(b.name)); + plan.toDelete.sort(); + plan.unchanged.sort(); + + return plan; + } + + static planHasChanges(plan: ProtectionRulesPlan): boolean { + return ( + plan.toCreate.length > 0 || + plan.toUpdate.length > 0 || + plan.toDelete.length > 0 + ); + } +} diff --git a/cli/src/commands/protection-rules/file.ts b/cli/src/commands/protection-rules/file.ts new file mode 100644 index 0000000000..4b2742d4f6 --- /dev/null +++ b/cli/src/commands/protection-rules/file.ts @@ -0,0 +1,129 @@ +import { existsSync } from "node:fs"; +import { writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { stringify as yamlStringify } from "yaml"; + +import { yamlParseFile } from "../../utils/yaml.ts"; +import { yamlOptions } from "../sync/sync.ts"; +import { + SyncOptions, + getWmillYamlPath, + getWorkspaceNames, + getEffectiveWorkspaceId, + WorkspaceEntryConfig, +} from "../../core/conf.ts"; +import { GlobalOptions } from "../../types.ts"; +import { tryResolveBranchWorkspace } from "../../core/context.ts"; +import { setClient } from "../../core/client.ts"; +import { ProtectionRulesFile } from "./types.ts"; + +export const PROTECTION_RULES_FILENAME = "protection-rules.yaml"; + +// protection-rules.yaml lives next to wmill.yaml. wmill.yaml is required: it is +// the single source of truth for which workspaces exist and how to reach them. +export function getProtectionRulesPath(): string | null { + const wmillPath = getWmillYamlPath(); + if (!wmillPath) return null; + return join(dirname(wmillPath), PROTECTION_RULES_FILENAME); +} + +export async function readProtectionRulesFile( + path: string, +): Promise { + if (!existsSync(path)) return {}; + const parsed = (await yamlParseFile(path)) as ProtectionRulesFile | null; + return parsed ?? {}; +} + +export async function writeProtectionRulesFile( + path: string, + data: ProtectionRulesFile, +): Promise { + // Deterministic key order so diffs/commits stay stable. + const sorted: ProtectionRulesFile = {}; + for (const k of Object.keys(data).sort()) sorted[k] = data[k]; + await writeFile(path, yamlStringify(sorted, yamlOptions), "utf-8"); +} + +// Maps a protection-rules.yaml workspace key to its backend workspace id via +// wmill.yaml's `workspaces` block. A key with no matching entry is rejected — +// without it we don't know which backend to talk to. +export class WorkspaceResolver { + private constructor( + private readonly workspaces: Record, + ) {} + + static fromConfig(config: SyncOptions): WorkspaceResolver { + const ws = (config.workspaces ?? {}) as Record< + string, + WorkspaceEntryConfig + >; + return new WorkspaceResolver(ws); + } + + /** Workspace keys declared in wmill.yaml (excludes reserved keys). */ + knownNames(): string[] { + return getWorkspaceNames(this.workspaces as any); + } + + has(name: string): boolean { + return this.knownNames().includes(name); + } + + /** Backend workspace id (path param) for a key, or throw if unknown. */ + backendId(name: string): string { + if (!this.has(name)) { + throw new Error( + `Workspace '${name}' is not defined in wmill.yaml 'workspaces'. ` + + `Add it there (its keys must match protection-rules.yaml).`, + ); + } + return getEffectiveWorkspaceId(name, this.workspaces[name]); + } +} + +// Point the API client at the backend for a single wmill.yaml workspace key, +// then return the backend workspace id to use as the path param. The backend +// id always comes from the wmill.yaml mapping (the feature's invariant); +// credentials are resolved with the same precedence as every other command: +// +// 1. explicit --base-url + --token -> used as-is (stateless CI; no profile +// or wmill.yaml baseUrl required) +// 2. otherwise, the stored profile matching wmill.yaml workspaces. +// (its baseUrl + token), with an explicit --token overriding the +// stored token +// +// Throws a clean error if the key is unknown or nothing resolves it — callers +// decide whether to skip (--all) or fail (named arg). +export async function configureClientForWorkspace( + opts: GlobalOptions, + ws: string, + resolver: WorkspaceResolver, +): Promise { + const wsId = resolver.backendId(ws); // throws if not in wmill.yaml + + // 1. Explicit credentials — honor them directly, like other commands do. + if (opts.baseUrl) { + if (!opts.token) { + throw new Error( + "When --base-url is set, --token is required for protection-rules.", + ); + } + setClient(opts.token, opts.baseUrl.replace(/\/+$/, "")); + return wsId; + } + + // 2. Stored-profile resolution. Fresh opts so resolveWorkspace's per-call + // cache can't bleed across keys. + const resolved = await tryResolveBranchWorkspace({ ...opts }, ws); + if (!resolved) { + throw new Error( + `Could not resolve credentials for workspace '${ws}'. Either pass ` + + `--base-url and --token, or ensure wmill.yaml workspaces.${ws} has a ` + + `baseUrl and you've run 'wmill workspace add' for it.`, + ); + } + // An explicit --token overrides the stored profile's token. + setClient(opts.token ?? resolved.token, resolved.remote.replace(/\/+$/, "")); + return wsId; +} diff --git a/cli/src/commands/protection-rules/index.ts b/cli/src/commands/protection-rules/index.ts new file mode 100644 index 0000000000..8ea8c47a1c --- /dev/null +++ b/cli/src/commands/protection-rules/index.ts @@ -0,0 +1,2 @@ +export { pullProtectionRules, pushProtectionRules } from "./protection-rules.ts"; +export { default } from "./protection-rules.ts"; diff --git a/cli/src/commands/protection-rules/protection-rules.ts b/cli/src/commands/protection-rules/protection-rules.ts new file mode 100644 index 0000000000..932a956333 --- /dev/null +++ b/cli/src/commands/protection-rules/protection-rules.ts @@ -0,0 +1,30 @@ +import { Command } from "@cliffy/command"; +import { pullProtectionRules } from "./pull.ts"; +import { pushProtectionRules } from "./push.ts"; + +const command = new Command() + .description( + "Sync workspace protection rules between protection-rules.yaml and Windmill. The file is keyed by workspace name; keys must match wmill.yaml 'workspaces'.", + ) + .command("pull") + .description( + "Pull protection rules from Windmill into protection-rules.yaml for a workspace", + ) + .arguments("[workspace:string]") + .option("--all", "Pull every workspace defined in wmill.yaml") + .option("--dry-run", "Show what would change without writing the file") + .option("--json-output", "Output in JSON format") + .action(pullProtectionRules as any) + .command("push") + .description( + "Push protection rules from protection-rules.yaml to Windmill for a workspace (full reconcile: creates, updates, and deletes)", + ) + .arguments("[workspace:string]") + .option("--all", "Push every workspace defined in protection-rules.yaml") + .option("--dry-run", "Show what would change without applying") + .option("--json-output", "Output in JSON format") + .option("--yes", "Skip the confirmation prompt (including deletions)") + .action(pushProtectionRules as any); + +export { pullProtectionRules, pushProtectionRules }; +export default command; diff --git a/cli/src/commands/protection-rules/pull.ts b/cli/src/commands/protection-rules/pull.ts new file mode 100644 index 0000000000..9086800422 --- /dev/null +++ b/cli/src/commands/protection-rules/pull.ts @@ -0,0 +1,143 @@ +import { colors } from "@cliffy/ansi/colors"; + +import * as log from "../../core/log.ts"; +import { GlobalOptions } from "../../types.ts"; +import * as wmill from "../../../gen/services.gen.ts"; +import { readConfigFile } from "../../core/conf.ts"; + +import { ProtectionRulesConverter } from "./converter.ts"; +import { ProtectionRulesFile } from "./types.ts"; +import { + PROTECTION_RULES_FILENAME, + getProtectionRulesPath, + readProtectionRulesFile, + writeProtectionRulesFile, + WorkspaceResolver, + configureClientForWorkspace, +} from "./file.ts"; +import { outputResult, fail, displayPlan, structuredPlan } from "./utils.ts"; + +type PullOpts = GlobalOptions & { + all?: boolean; + dryRun?: boolean; + jsonOutput?: boolean; +}; + +export async function pullProtectionRules( + opts: PullOpts, + workspaceArg?: string, +) { + // In JSON mode stdout must be exactly one JSON payload. Silence human logs + // (log.info/warn → stdout) here, before anything that logs (readConfigFile, + // workspace resolution). log.error still goes to stderr. + if (opts.jsonOutput) log.setSilent(true); + + const prPath = getProtectionRulesPath(); + if (!prPath) { + fail(opts, { + error: + "No wmill.yaml found. Run 'wmill init' first — protection-rules.yaml lives next to it.", + }); + } + + const config = await readConfigFile(); + const resolver = WorkspaceResolver.fromConfig(config); + + let targets: string[]; + if (opts.all) { + targets = resolver.knownNames(); + if (targets.length === 0) { + fail(opts, { + error: "No workspaces defined in wmill.yaml 'workspaces' block.", + }); + } + } else if (workspaceArg) { + targets = [workspaceArg]; + } else { + fail(opts, { error: "Specify a workspace name or use --all." }); + } + + const file = await readProtectionRulesFile(prPath!); + const perWs: Record = {}; + let hadError = false; + let anyChange = false; + + for (const ws of targets) { + let wsId: string; + try { + wsId = await configureClientForWorkspace(opts, ws, resolver); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + if (opts.all) { + log.error(colors.red(msg)); + hadError = true; + continue; + } + fail(opts, { error: msg }); + } + + let backend; + try { + backend = ProtectionRulesConverter.fromBackend( + await wmill.listProtectionRules({ workspace: wsId }), + ); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + if (opts.all) { + log.error(colors.red(`[${ws}] failed to fetch: ${msg}`)); + hadError = true; + continue; + } + fail(opts, { error: `Failed to fetch protection rules: ${msg}` }); + } + + const current = ProtectionRulesConverter.normalizeList(file[ws]); + // plan describes how the local file would change to match the backend + const plan = ProtectionRulesConverter.computePlan(backend, current); + if (ProtectionRulesConverter.planHasChanges(plan)) anyChange = true; + perWs[ws] = structuredPlan(plan); + + if (!opts.dryRun) { + file[ws] = backend; + } else if (!opts.jsonOutput) { + displayPlan(ws, plan); + } + } + + if (opts.dryRun) { + if (opts.jsonOutput) { + console.log( + JSON.stringify({ + success: !hadError, + dryRun: true, + partialFailure: hadError, + hasChanges: anyChange, + workspaces: perWs, + }), + ); + } else if (!hadError && !anyChange) { + log.info(colors.green("All targeted workspaces are in sync")); + } + if (hadError) process.exit(1); + return; + } + + await writeProtectionRulesFile(prPath!, file as ProtectionRulesFile); + const n = Object.keys(perWs).length; + if (hadError) { + // Some --all workspaces failed: status must not say success while we + // exit non-zero. + outputResult(opts, { + success: false, + error: `Pulled ${n} workspace(s) into ${PROTECTION_RULES_FILENAME}, but one or more workspaces failed (see errors above)`, + partialFailure: true, + workspaces: perWs, + }); + process.exit(1); + } + outputResult(opts, { + success: true, + message: `Pulled protection rules for ${n} workspace(s) into ${PROTECTION_RULES_FILENAME}`, + workspaces: perWs, + }); +} diff --git a/cli/src/commands/protection-rules/push.ts b/cli/src/commands/protection-rules/push.ts new file mode 100644 index 0000000000..2ee253eaab --- /dev/null +++ b/cli/src/commands/protection-rules/push.ts @@ -0,0 +1,262 @@ +import process from "node:process"; +import { existsSync } from "node:fs"; + +import { colors } from "@cliffy/ansi/colors"; +import { Confirm } from "@cliffy/prompt/confirm"; + +import * as log from "../../core/log.ts"; +import { GlobalOptions } from "../../types.ts"; +import * as wmill from "../../../gen/services.gen.ts"; +import { readConfigFile } from "../../core/conf.ts"; + +import { ProtectionRulesConverter, ProtectionRulesPlan } from "./converter.ts"; +import { + getProtectionRulesPath, + readProtectionRulesFile, + WorkspaceResolver, + configureClientForWorkspace, +} from "./file.ts"; +import { outputResult, fail, displayPlan, structuredPlan } from "./utils.ts"; + +type PushOpts = GlobalOptions & { + all?: boolean; + dryRun?: boolean; + jsonOutput?: boolean; + yes?: boolean; +}; + +interface WsPlan { + ws: string; + wsId: string; + plan: ProtectionRulesPlan; + wipesAll: boolean; +} + +export async function pushProtectionRules( + opts: PushOpts, + workspaceArg?: string, +) { + // In JSON mode stdout must be exactly one JSON payload. Silence human logs + // (log.info/warn → stdout, incl. workspace resolution + the empty-list + // delete warning) before anything logs. log.error still goes to stderr. + if (opts.jsonOutput) log.setSilent(true); + + const prPath = getProtectionRulesPath(); + if (!prPath) { + fail(opts, { + error: + "No wmill.yaml found. Run 'wmill init' first — protection-rules.yaml lives next to it.", + }); + } + if (!existsSync(prPath!)) { + fail(opts, { + error: + "No protection-rules.yaml found. Run 'wmill protection-rules pull' first.", + }); + } + + const config = await readConfigFile(); + const resolver = WorkspaceResolver.fromConfig(config); + const file = await readProtectionRulesFile(prPath!); + + let targets: string[]; + if (opts.all) { + targets = Object.keys(file).sort(); + if (targets.length === 0) { + fail(opts, { error: "protection-rules.yaml defines no workspaces." }); + } + } else if (workspaceArg) { + if (!(workspaceArg in file)) { + fail(opts, { + error: `Workspace '${workspaceArg}' is not defined in protection-rules.yaml.`, + }); + } + targets = [workspaceArg]; + } else { + fail(opts, { error: "Specify a workspace name or use --all." }); + } + + // Phase 1: resolve + diff every target before mutating anything. + const wsPlans: WsPlan[] = []; + let hadError = false; + for (const ws of targets) { + let wsId: string; + try { + wsId = await configureClientForWorkspace(opts, ws, resolver); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + if (opts.all) { + log.error(colors.red(msg)); + hadError = true; + continue; + } + fail(opts, { error: msg }); + } + + let backend; + try { + backend = ProtectionRulesConverter.fromBackend( + await wmill.listProtectionRules({ workspace: wsId }), + ); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + if (opts.all) { + log.error(colors.red(`[${ws}] failed to fetch: ${msg}`)); + hadError = true; + continue; + } + fail(opts, { error: `Failed to fetch protection rules: ${msg}` }); + } + + const local = ProtectionRulesConverter.normalizeList(file[ws]); + const plan = ProtectionRulesConverter.computePlan(local, backend); + wsPlans.push({ + ws, + wsId, + plan, + wipesAll: local.length === 0 && plan.toDelete.length > 0, + }); + } + + const changed = wsPlans.filter((w) => + ProtectionRulesConverter.planHasChanges(w.plan) + ); + + if (opts.jsonOutput && opts.dryRun) { + console.log( + JSON.stringify({ + success: !hadError, + dryRun: true, + partialFailure: hadError, + hasChanges: changed.length > 0, + workspaces: Object.fromEntries( + wsPlans.map((w) => [w.ws, structuredPlan(w.plan)]), + ), + }), + ); + if (hadError) process.exit(1); + return; + } + + if (!opts.jsonOutput) { + for (const w of wsPlans) displayPlan(w.ws, w.plan); + } + + if (changed.length === 0) { + if (hadError) { + // A workspace failed to resolve/fetch — don't claim success while + // exiting non-zero. + outputResult(opts, { + success: false, + error: + "One or more workspaces failed (see errors above); the rest are in sync", + partialFailure: true, + }); + process.exit(1); + } + if (!opts.dryRun) { + outputResult(opts, { + success: true, + message: "No changes to push - all targeted workspaces are in sync", + }); + } + return; + } + + if (opts.dryRun) { + if (hadError) process.exit(1); + return; + } + + // Pushing an empty list wipes a workspace's rules — be loud even with --yes. + for (const w of wsPlans) { + if (w.wipesAll) { + log.warn( + colors.red( + `WARNING: '${w.ws}' has an empty rule list — this DELETES ALL ${w.plan.toDelete.length} backend rule(s) for that workspace.`, + ), + ); + } + } + + const totalDeletes = changed.reduce((n, w) => n + w.plan.toDelete.length, 0); + if (!opts.yes && !!process.stdin.isTTY) { + const confirmed = await Confirm.prompt({ + message: totalDeletes > 0 + ? `Apply these changes? This DELETES ${totalDeletes} protection rule(s) across ${changed.length} workspace(s).` + : `Apply these changes to ${changed.length} workspace(s)?`, + default: totalDeletes === 0, + }); + if (!confirmed) { + log.info("Operation cancelled"); + return; + } + } + + // Phase 2: apply. Track progress so a mid-run failure reports how far it got. + const applied = { created: 0, updated: 0, deleted: 0 }; + try { + for (const w of changed) { + // Re-point the client at this workspace (phase 1 left it on the last one). + await configureClientForWorkspace(opts, w.ws, resolver); + for (const entry of w.plan.toCreate) { + const n = ProtectionRulesConverter.normalizeEntry(entry); + await wmill.createProtectionRule({ + workspace: w.wsId, + requestBody: { + name: n.name, + rules: n.rules, + bypass_groups: n.bypass_groups, + bypass_users: n.bypass_users, + }, + }); + applied.created++; + } + for (const entry of w.plan.toUpdate) { + const n = ProtectionRulesConverter.normalizeEntry(entry); + await wmill.updateProtectionRule({ + workspace: w.wsId, + ruleName: n.name, + requestBody: { + rules: n.rules, + bypass_groups: n.bypass_groups, + bypass_users: n.bypass_users, + }, + }); + applied.updated++; + } + for (const name of w.plan.toDelete) { + await wmill.deleteProtectionRule({ + workspace: w.wsId, + ruleName: name, + }); + applied.deleted++; + } + } + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + fail(opts, { + error: + `Push partially failed after ${applied.created} create, ${applied.updated} update, ` + + `${applied.deleted} delete: ${msg}. Backend is partially reconciled; re-run push to converge.`, + applied, + }); + } + + if (hadError) { + // Reconcile of resolvable workspaces succeeded, but some --all targets + // failed earlier. Status must reflect the non-zero exit. + outputResult(opts, { + success: false, + error: `Pushed (created ${applied.created}, updated ${applied.updated}, deleted ${applied.deleted}) across ${changed.length} workspace(s), but one or more workspaces failed (see errors above)`, + partialFailure: true, + ...applied, + }); + process.exit(1); + } + outputResult(opts, { + success: true, + message: `Pushed protection rules (created ${applied.created}, updated ${applied.updated}, deleted ${applied.deleted}) across ${changed.length} workspace(s)`, + ...applied, + }); +} diff --git a/cli/src/commands/protection-rules/types.ts b/cli/src/commands/protection-rules/types.ts new file mode 100644 index 0000000000..0256f24e8f --- /dev/null +++ b/cli/src/commands/protection-rules/types.ts @@ -0,0 +1,18 @@ +import { ProtectionRuleKind } from "../../../gen/types.gen.ts"; + +export type { ProtectionRuleKind }; + +// A single workspace protection ruleset as stored in protection-rules.yaml. +// Mirrors the backend ProtectionRuleset shape minus workspace_id (the workspace +// is the map key). +export interface ProtectionRuleEntry { + name: string; + rules: ProtectionRuleKind[]; + bypass_groups: string[]; + bypass_users: string[]; +} + +// protection-rules.yaml is a flat map: workspace name -> its protection rules. +// Workspace names MUST match keys in wmill.yaml's `workspaces` block, which is +// where the backend workspaceId/remote is resolved from. +export type ProtectionRulesFile = Record; diff --git a/cli/src/commands/protection-rules/utils.ts b/cli/src/commands/protection-rules/utils.ts new file mode 100644 index 0000000000..9809e5808c --- /dev/null +++ b/cli/src/commands/protection-rules/utils.ts @@ -0,0 +1,73 @@ +import process from "node:process"; + +import { colors } from "@cliffy/ansi/colors"; +import * as log from "../../core/log.ts"; +import { ProtectionRuleEntry } from "./types.ts"; +import { ProtectionRulesConverter, ProtectionRulesPlan } from "./converter.ts"; + +export function outputResult( + opts: { jsonOutput?: boolean }, + result: { + success: boolean; + message?: string; + error?: string; + [key: string]: any; + }, +): void { + if (opts.jsonOutput) { + console.log(JSON.stringify(result)); + } else if (result.success && result.message) { + log.info(colors.green(result.message)); + } else if (!result.success && result.error) { + log.error(colors.red(result.error)); + } +} + +// Report a genuine failure and exit non-zero so CI / scripted callers detect +// it. outputResult alone only logs, which would let a failed (possibly +// partial) reconcile hide behind exit code 0. +export function fail( + opts: { jsonOutput?: boolean }, + result: { error: string; [key: string]: any }, +): never { + outputResult(opts, { ...result, success: false }); + process.exit(1); +} + +function describeEntry(entry: ProtectionRuleEntry): string { + const n = ProtectionRulesConverter.normalizeEntry(entry); + const parts = [`rules=[${n.rules.join(", ")}]`]; + if (n.bypass_groups.length > 0) { + parts.push(`bypass_groups=[${n.bypass_groups.join(", ")}]`); + } + if (n.bypass_users.length > 0) { + parts.push(`bypass_users=[${n.bypass_users.join(", ")}]`); + } + return parts.join(" "); +} + +// Render a reconciliation plan with a per-workspace heading. +export function displayPlan(ws: string, plan: ProtectionRulesPlan): void { + log.info(colors.bold(`workspace ${ws}:`)); + for (const e of plan.toCreate) { + log.info(colors.green(` + ${e.name} (${describeEntry(e)})`)); + } + for (const e of plan.toUpdate) { + log.info(colors.yellow(` ~ ${e.name} (${describeEntry(e)})`)); + } + for (const name of plan.toDelete) { + log.info(colors.red(` - ${name}`)); + } + if (!ProtectionRulesConverter.planHasChanges(plan)) { + log.info(colors.green(" in sync")); + } +} + +export function structuredPlan(plan: ProtectionRulesPlan) { + return { + create: plan.toCreate.map((e) => e.name), + update: plan.toUpdate.map((e) => e.name), + delete: plan.toDelete, + unchanged: plan.unchanged, + }; +} diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index 2b0962e09d..d2227b5f94 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -6944,6 +6944,20 @@ Validate Windmill flow, schedule, and trigger YAML files in a directory - \`--locks-required\` - Fail if scripts or flow inline scripts that need locks have no locks - \`-w, --watch\` - Watch for file changes and re-lint automatically +### protection-rules + +**Subcommands:** + +- \`protection-rules pull [workspace:string]\` - Pull protection rules from Windmill into protection-rules.yaml for a workspace + - \`--all\` - Pull every workspace defined in wmill.yaml + - \`--dry-run\` - Show what would change without writing the file + - \`--json-output\` - Output in JSON format +- \`protection-rules push [workspace:string]\` - Push protection rules from protection-rules.yaml to Windmill for a workspace (full reconcile: creates, updates, and deletes) + - \`--all\` - Push every workspace defined in protection-rules.yaml + - \`--dry-run\` - Show what would change without applying + - \`--json-output\` - Output in JSON format + - \`--yes\` - Skip the confirmation prompt (including deletions) + ### queues List all queues with their metrics diff --git a/cli/src/main.ts b/cli/src/main.ts index dd078c6c9c..90dbf1164b 100755 --- a/cli/src/main.ts +++ b/cli/src/main.ts @@ -21,6 +21,7 @@ import schedule from "./commands/schedule/schedule.ts"; import trigger from "./commands/trigger/trigger.ts"; import sync from "./commands/sync/sync.ts"; import gitsyncSettings from "./commands/gitsync-settings/gitsync-settings.ts"; +import protectionRules from "./commands/protection-rules/protection-rules.ts"; import instance from "./commands/instance/instance.ts"; import workerGroups from "./commands/worker-groups/worker-groups.ts"; import lint from "./commands/lint/lint.ts"; @@ -65,6 +66,7 @@ export { sync, lint, gitsyncSettings, + protectionRules, instance, dev, docs, @@ -185,6 +187,7 @@ const command = new Command() .command("sync", sync) .command("lint", lint) .command("gitsync-settings", gitsyncSettings) + .command("protection-rules", protectionRules) .command("instance", instance) .command("worker-groups", workerGroups) .command("workers", workers) diff --git a/cli/test/protection_rules_converter_unit.test.ts b/cli/test/protection_rules_converter_unit.test.ts new file mode 100644 index 0000000000..c024cdf502 --- /dev/null +++ b/cli/test/protection_rules_converter_unit.test.ts @@ -0,0 +1,240 @@ +/** + * Unit tests for the protection-rules feature: the reconciliation converter, + * the WorkspaceResolver (protection-rules.yaml key -> backend id via + * wmill.yaml), and protection-rules.yaml read/write round-tripping. + */ + +import { expect, test, describe } from "bun:test"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { mkdtempSync, rmSync } from "node:fs"; +import { ProtectionRulesConverter } from "../src/commands/protection-rules/converter.ts"; +import { + WorkspaceResolver, + readProtectionRulesFile, + writeProtectionRulesFile, +} from "../src/commands/protection-rules/file.ts"; +import { ProtectionRuleEntry } from "../src/commands/protection-rules/types.ts"; +import { SyncOptions } from "../src/core/conf.ts"; + +const rule = ( + name: string, + rules: ProtectionRuleEntry["rules"], + groups: string[] = [], + users: string[] = [], +): ProtectionRuleEntry => ({ + name, + rules, + bypass_groups: groups, + bypass_users: users, +}); + +describe("normalizeEntry", () => { + test("sorts and dedupes rules, groups, users", () => { + const r = rule( + "prod", + ["RestrictDeployToDeployers", "DisableDirectDeployment", "DisableDirectDeployment"], + ["g/b", "g/a"], + ["u/y", "u/x", "u/x"], + ); + const n = ProtectionRulesConverter.normalizeEntry(r); + expect(n.rules).toEqual([ + "DisableDirectDeployment", + "RestrictDeployToDeployers", + ]); + expect(n.bypass_groups).toEqual(["g/a", "g/b"]); + expect(n.bypass_users).toEqual(["u/x", "u/y"]); + }); + + test("handles missing arrays", () => { + const n = ProtectionRulesConverter.normalizeEntry({ + name: "x", + } as unknown as ProtectionRuleEntry); + expect(n.rules).toEqual([]); + expect(n.bypass_groups).toEqual([]); + expect(n.bypass_users).toEqual([]); + }); +}); + +describe("entriesEqual", () => { + test("equal regardless of array order", () => { + const a = rule("p", ["DisableDirectDeployment", "DisableWorkspaceForking"], ["g/a", "g/b"]); + const b = rule("p", ["DisableWorkspaceForking", "DisableDirectDeployment"], ["g/b", "g/a"]); + expect(ProtectionRulesConverter.entriesEqual(a, b)).toBe(true); + }); + + test("different rules are not equal", () => { + const a = rule("p", ["DisableDirectDeployment"]); + const b = rule("p", ["DisableWorkspaceForking"]); + expect(ProtectionRulesConverter.entriesEqual(a, b)).toBe(false); + }); + + test("different bypass users are not equal", () => { + const a = rule("p", ["DisableDirectDeployment"], [], ["u/a"]); + const b = rule("p", ["DisableDirectDeployment"], [], ["u/b"]); + expect(ProtectionRulesConverter.entriesEqual(a, b)).toBe(false); + }); +}); + +describe("fromBackend", () => { + test("strips workspace_id and normalizes", () => { + const out = ProtectionRulesConverter.fromBackend([ + { + name: "p", + workspace_id: "ws1", + rules: ["DisableWorkspaceForking", "DisableDirectDeployment"], + bypass_groups: ["g/b", "g/a"], + bypass_users: [], + }, + ]); + expect(out).toEqual([ + { + name: "p", + rules: ["DisableDirectDeployment", "DisableWorkspaceForking"], + bypass_groups: ["g/a", "g/b"], + bypass_users: [], + }, + ]); + }); +}); + +describe("listsEqual", () => { + test("equal regardless of list order", () => { + const a = [rule("a", ["DisableDirectDeployment"]), rule("b", ["DisableWorkspaceForking"])]; + const b = [rule("b", ["DisableWorkspaceForking"]), rule("a", ["DisableDirectDeployment"])]; + expect(ProtectionRulesConverter.listsEqual(a, b)).toBe(true); + }); + + test("undefined equals empty", () => { + expect(ProtectionRulesConverter.listsEqual(undefined, [])).toBe(true); + }); + + test("different length not equal", () => { + expect( + ProtectionRulesConverter.listsEqual([rule("a", [])], []), + ).toBe(false); + }); +}); + +describe("computePlan (full reconcile)", () => { + test("creates rules present locally but not on backend", () => { + const plan = ProtectionRulesConverter.computePlan( + [rule("new", ["DisableDirectDeployment"])], + [], + ); + expect(plan.toCreate.map((e) => e.name)).toEqual(["new"]); + expect(plan.toUpdate).toEqual([]); + expect(plan.toDelete).toEqual([]); + }); + + test("deletes backend rules not present locally", () => { + const plan = ProtectionRulesConverter.computePlan( + [], + [rule("stale", ["DisableDirectDeployment"])], + ); + expect(plan.toDelete).toEqual(["stale"]); + expect(plan.toCreate).toEqual([]); + }); + + test("updates rules whose content changed", () => { + const plan = ProtectionRulesConverter.computePlan( + [rule("p", ["DisableDirectDeployment", "DisableWorkspaceForking"])], + [rule("p", ["DisableDirectDeployment"])], + ); + expect(plan.toUpdate.map((e) => e.name)).toEqual(["p"]); + expect(plan.toCreate).toEqual([]); + expect(plan.toDelete).toEqual([]); + }); + + test("unchanged rules are not in create/update/delete", () => { + const same = [rule("p", ["DisableDirectDeployment"], ["g/a"])]; + const plan = ProtectionRulesConverter.computePlan(same, [ + rule("p", ["DisableDirectDeployment"], ["g/a"]), + ]); + expect(ProtectionRulesConverter.planHasChanges(plan)).toBe(false); + expect(plan.unchanged).toEqual(["p"]); + }); + + test("mixed plan: create + update + delete + unchanged", () => { + const local = [ + rule("keep", ["DisableDirectDeployment"]), + rule("change", ["DisableWorkspaceForking"]), + rule("brand-new", ["RestrictDeployToDeployers"]), + ]; + const backend = [ + rule("keep", ["DisableDirectDeployment"]), + rule("change", ["DisableDirectDeployment"]), + rule("gone", ["DisableDirectDeployment"]), + ]; + const plan = ProtectionRulesConverter.computePlan(local, backend); + expect(plan.toCreate.map((e) => e.name)).toEqual(["brand-new"]); + expect(plan.toUpdate.map((e) => e.name)).toEqual(["change"]); + expect(plan.toDelete).toEqual(["gone"]); + expect(plan.unchanged).toEqual(["keep"]); + expect(ProtectionRulesConverter.planHasChanges(plan)).toBe(true); + }); + + test("reordered arrays do not produce spurious updates", () => { + const plan = ProtectionRulesConverter.computePlan( + [rule("p", ["DisableWorkspaceForking", "DisableDirectDeployment"], ["g/b", "g/a"])], + [rule("p", ["DisableDirectDeployment", "DisableWorkspaceForking"], ["g/a", "g/b"])], + ); + expect(ProtectionRulesConverter.planHasChanges(plan)).toBe(false); + }); +}); + +describe("WorkspaceResolver", () => { + const config: SyncOptions = { + workspaces: { + prod: { workspaceId: "acme-prod" }, + dev: {}, + commonSpecificItems: { settings: true }, + } as any, + }; + const r = WorkspaceResolver.fromConfig(config); + + test("knownNames excludes reserved keys", () => { + expect(r.knownNames().sort()).toEqual(["dev", "prod"]); + }); + + test("backendId uses workspaceId when set, else the key name", () => { + expect(r.backendId("prod")).toBe("acme-prod"); + expect(r.backendId("dev")).toBe("dev"); + }); + + test("backendId throws for a key absent from wmill.yaml", () => { + expect(() => r.backendId("ghost")).toThrow(/not defined in wmill\.yaml/); + }); + + test("has reflects membership", () => { + expect(r.has("prod")).toBe(true); + expect(r.has("ghost")).toBe(false); + }); + + test("empty config resolves to no workspaces", () => { + expect(WorkspaceResolver.fromConfig({}).knownNames()).toEqual([]); + }); +}); + +describe("protection-rules.yaml read/write", () => { + test("round-trips and sorts workspace keys deterministically", async () => { + const dir = mkdtempSync(join(tmpdir(), "prfile-")); + const path = join(dir, "protection-rules.yaml"); + try { + expect(await readProtectionRulesFile(path)).toEqual({}); + + await writeProtectionRulesFile(path, { + prod: [rule("p", ["DisableDirectDeployment"], ["g/a"])], + dev: [], + }); + const back = await readProtectionRulesFile(path); + expect(Object.keys(back)).toEqual(["dev", "prod"]); + expect(back.prod).toEqual([ + rule("p", ["DisableDirectDeployment"], ["g/a"]), + ]); + expect(back.dev).toEqual([]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/system_prompts/auto-generated/cli/cli-commands.md b/system_prompts/auto-generated/cli/cli-commands.md index dc0eb66640..eefa587666 100644 --- a/system_prompts/auto-generated/cli/cli-commands.md +++ b/system_prompts/auto-generated/cli/cli-commands.md @@ -330,6 +330,20 @@ Validate Windmill flow, schedule, and trigger YAML files in a directory - `--locks-required` - Fail if scripts or flow inline scripts that need locks have no locks - `-w, --watch` - Watch for file changes and re-lint automatically +### protection-rules + +**Subcommands:** + +- `protection-rules pull [workspace:string]` - Pull protection rules from Windmill into protection-rules.yaml for a workspace + - `--all` - Pull every workspace defined in wmill.yaml + - `--dry-run` - Show what would change without writing the file + - `--json-output` - Output in JSON format +- `protection-rules push [workspace:string]` - Push protection rules from protection-rules.yaml to Windmill for a workspace (full reconcile: creates, updates, and deletes) + - `--all` - Push every workspace defined in protection-rules.yaml + - `--dry-run` - Show what would change without applying + - `--json-output` - Output in JSON format + - `--yes` - Skip the confirmation prompt (including deletions) + ### queues List all queues with their metrics diff --git a/system_prompts/auto-generated/prompts.ts b/system_prompts/auto-generated/prompts.ts index f1c00d5763..a47b56aeaa 100644 --- a/system_prompts/auto-generated/prompts.ts +++ b/system_prompts/auto-generated/prompts.ts @@ -2861,6 +2861,20 @@ Validate Windmill flow, schedule, and trigger YAML files in a directory - \`--locks-required\` - Fail if scripts or flow inline scripts that need locks have no locks - \`-w, --watch\` - Watch for file changes and re-lint automatically +### protection-rules + +**Subcommands:** + +- \`protection-rules pull [workspace:string]\` - Pull protection rules from Windmill into protection-rules.yaml for a workspace + - \`--all\` - Pull every workspace defined in wmill.yaml + - \`--dry-run\` - Show what would change without writing the file + - \`--json-output\` - Output in JSON format +- \`protection-rules push [workspace:string]\` - Push protection rules from protection-rules.yaml to Windmill for a workspace (full reconcile: creates, updates, and deletes) + - \`--all\` - Push every workspace defined in protection-rules.yaml + - \`--dry-run\` - Show what would change without applying + - \`--json-output\` - Output in JSON format + - \`--yes\` - Skip the confirmation prompt (including deletions) + ### queues List all queues with their metrics diff --git a/system_prompts/auto-generated/skills/cli-commands/SKILL.md b/system_prompts/auto-generated/skills/cli-commands/SKILL.md index 98ce52efde..a729025eeb 100644 --- a/system_prompts/auto-generated/skills/cli-commands/SKILL.md +++ b/system_prompts/auto-generated/skills/cli-commands/SKILL.md @@ -335,6 +335,20 @@ Validate Windmill flow, schedule, and trigger YAML files in a directory - `--locks-required` - Fail if scripts or flow inline scripts that need locks have no locks - `-w, --watch` - Watch for file changes and re-lint automatically +### protection-rules + +**Subcommands:** + +- `protection-rules pull [workspace:string]` - Pull protection rules from Windmill into protection-rules.yaml for a workspace + - `--all` - Pull every workspace defined in wmill.yaml + - `--dry-run` - Show what would change without writing the file + - `--json-output` - Output in JSON format +- `protection-rules push [workspace:string]` - Push protection rules from protection-rules.yaml to Windmill for a workspace (full reconcile: creates, updates, and deletes) + - `--all` - Push every workspace defined in protection-rules.yaml + - `--dry-run` - Show what would change without applying + - `--json-output` - Output in JSON format + - `--yes` - Skip the confirmation prompt (including deletions) + ### queues List all queues with their metrics From 4b1bea8aed51eb9e24940d89d984ce32f375ab0c Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 19 May 2026 17:40:55 +0000 Subject: [PATCH 04/29] fix: enforce auth guards on app component preview execution (#9235) * fix: enforce auth guards on app component preview execution Co-Authored-By: Claude Opus 4.7 (1M context) * fix: guard previewed runnable path and worker tag in app preview Co-Authored-By: Claude Opus 4.7 (1M context) * fix: validate app_script id ownership and keep root push isolation Co-Authored-By: Claude Opus 4.7 (1M context) * refactor: scope app preview guards to operator check + referenced runnables Co-Authored-By: Claude Opus 4.7 (1M context) * fix: require jobs:run scope and tag check on app preview (apps:run escalation) Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- ...349fa8bd7d23e4589b6936e0d000745cb3f34.json | 23 ++ backend/tests/app_preview_auth.rs | 258 ++++++++++++++++++ backend/tests/fixtures/app_preview_auth.sql | 34 +++ backend/windmill-api/src/apps.rs | 64 ++++- 4 files changed, 378 insertions(+), 1 deletion(-) create mode 100644 backend/.sqlx/query-8816bbe1ea9ea4f359e7a95857d349fa8bd7d23e4589b6936e0d000745cb3f34.json create mode 100644 backend/tests/app_preview_auth.rs create mode 100644 backend/tests/fixtures/app_preview_auth.sql diff --git a/backend/.sqlx/query-8816bbe1ea9ea4f359e7a95857d349fa8bd7d23e4589b6936e0d000745cb3f34.json b/backend/.sqlx/query-8816bbe1ea9ea4f359e7a95857d349fa8bd7d23e4589b6936e0d000745cb3f34.json new file mode 100644 index 0000000000..53fe03ce5f --- /dev/null +++ b/backend/.sqlx/query-8816bbe1ea9ea4f359e7a95857d349fa8bd7d23e4589b6936e0d000745cb3f34.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT a.path FROM app_script s JOIN app a ON a.id = s.app\n WHERE s.id = $1 AND a.workspace_id = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "path", + "type_info": "Varchar" + } + ], + "parameters": { + "Left": [ + "Int8", + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "8816bbe1ea9ea4f359e7a95857d349fa8bd7d23e4589b6936e0d000745cb3f34" +} diff --git a/backend/tests/app_preview_auth.rs b/backend/tests/app_preview_auth.rs new file mode 100644 index 0000000000..4653303392 --- /dev/null +++ b/backend/tests/app_preview_auth.rs @@ -0,0 +1,258 @@ +//! Regression test for the app component preview authorization bypass. +//! +//! `POST /api/w/:workspace/apps_u/execute_component/:path` runs in "preview" +//! mode whenever the client supplies `force_viewer_static_fields`. In that +//! mode it accepts request-supplied `raw_code` and enqueues it as a +//! `Viewer`-mode job — i.e. it is the app-editor equivalent of +//! `/jobs/run/preview`. The bug was that this branch did not re-apply the +//! guards `/jobs/run/preview` enforces for arbitrary code execution, so an +//! authenticated Operator (a run-only user who must not be able to create +//! scripts/apps or run preview jobs) could enqueue arbitrary worker code with +//! a single request, escaping the Operator restriction entirely. +//! +//! This test pins down: +//! - an Operator is rejected from preview mode (the core fix; pre-fix this +//! enqueued a job and returned 200), +//! - a regular non-operator member can still run an editor preview (the fix +//! must not over-block the legitimate editor flow), +//! - preview is confined to paths the caller can read (defense-in-depth +//! against scoped tokens / cross-namespace preview), and +//! - run mode (no `force_viewer_static_fields`) is unaffected by the guard. + +use serde_json::json; +use sqlx::{Pool, Postgres}; +use windmill_test_utils::*; + +fn client() -> reqwest::Client { + reqwest::Client::new() +} + +fn authed(builder: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuilder { + builder.header("Authorization", format!("Bearer {}", token)) +} + +/// A preview request: `force_viewer_static_fields` present + inline `raw_code`. +/// This is the exact shape an attacker (or the editor) sends. +fn preview_body(app_path: &str) -> serde_json::Value { + json!({ + "args": {}, + "component": "comp", + "raw_code": { + "language": "deno", + "content": "export function main() { return \"pwned\"; }", + "path": format!("{}/comp", app_path) + }, + "force_viewer_static_fields": {} + }) +} + +#[sqlx::test(fixtures("base", "app_preview_auth"))] +async fn test_app_preview_authorization(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/w/test-workspace/apps_u/execute_component"); + + // 1. CORE REGRESSION: an Operator sends a preview request in their own + // namespace (so the *only* thing that can reject them is the Operator + // check itself). Pre-fix this returned 200 with an enqueued job UUID; + // post-fix it must be rejected. + let resp = authed( + client().post(format!("{base}/u/operator-user/myapp")), + "OPERATOR_TOKEN", + ) + .json(&preview_body("u/operator-user/myapp")) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status, 401, + "Operator must be rejected from app preview (got {status}): {body}" + ); + assert!( + body.contains("Operators cannot run preview jobs"), + "rejection must be the operator guard, got: {body}" + ); + + // 2. The fix must NOT over-block the legitimate editor flow: a regular + // non-operator member previewing in their own namespace still works + // (the endpoint returns the enqueued job UUID before any worker runs). + let resp = authed( + client().post(format!("{base}/u/test-user-2/myapp")), + "SECRET_TOKEN_2", + ) + .json(&preview_body("u/test-user-2/myapp")) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert!( + status.is_success(), + "non-operator editor preview must still succeed (got {status}): {body}" + ); + assert!( + uuid::Uuid::parse_str(body.trim()).is_ok(), + "successful preview must return a job UUID, got: {body}" + ); + + // 3. Inline `raw_code` preview is deliberately NOT path-gated: a + // non-operator can already run arbitrary inline code via + // `/jobs/run/preview`, so the app URL path string is irrelevant for the + // inline case. This pins that decision so an over-restrictive path check + // is not re-added for inline previews. + let resp = authed( + client().post(format!("{base}/u/test-user/secretapp")), + "SECRET_TOKEN_2", + ) + .json(&preview_body("u/test-user/secretapp")) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert!( + status.is_success(), + "inline raw_code preview must not be path-gated (got {status}): {body}" + ); + assert!( + uuid::Uuid::parse_str(body.trim()).is_ok(), + "inline preview should enqueue a job UUID, got: {body}" + ); + + // 4. Run mode (no `force_viewer_static_fields`) is unaffected by the new + // preview guard: an Operator hitting a deployed-app path still follows + // the pre-existing policy lookup (here: the app does not exist -> 404), + // proving the guard only gates preview mode. + let resp = authed( + client().post(format!("{base}/u/operator-user/nonexistent")), + "OPERATOR_TOKEN", + ) + .json(&json!({ + "args": {}, + "component": "comp", + "raw_code": { + "language": "deno", + "content": "export function main() { return 1; }", + "path": "u/operator-user/nonexistent/comp" + } + })) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status, 404, + "run mode must be unchanged (deployed app lookup -> 404, not the preview guard); got {status}: {body}" + ); + + // 5. Defense-in-depth: the guard must check the *runnable* being previewed, + // not just the app URL path. A caller pairs an allowed app path + // (`u/test-user-2/myapp`, own namespace) with a `path` pointing at a + // deployed runnable in another user's namespace. Without checking the + // runnable path this would resolve `script/u/test-user/private` with the + // root DB handle and enqueue it; it must be rejected by the path check. + let resp = authed( + client().post(format!("{base}/u/test-user-2/myapp")), + "SECRET_TOKEN_2", + ) + .json(&json!({ + "args": {}, + "component": "comp", + "path": "script/u/test-user/private", + "force_viewer_static_fields": {} + })) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status, 400, + "preview targeting a runnable outside the caller's namespace must be rejected even with an allowed app path (got {status}): {body}" + ); + + // 6. Defense-in-depth: a persisted inline-script preview selects code by the + // caller-controlled `app_script` id. Pairing an allowed app path with an + // id owned by another (private) app must be rejected — without the + // id-ownership check the worker would fetch and run that app's code. + let resp = authed( + client().post(format!("{base}/u/test-user-2/myapp")), + "SECRET_TOKEN_2", + ) + .json(&json!({ + "args": {}, + "component": "comp", + "id": 999777, + "raw_code": { + "language": "deno", + "content": "export function main() { return 1; }", + "path": "u/test-user-2/myapp/comp" + }, + "force_viewer_static_fields": {} + })) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status, 400, + "preview with an app_script id owned by another app must be rejected (got {status}): {body}" + ); + + // 7. The id-ownership check must NOT over-block a legitimate persisted + // inline-script preview: an id owned by an app in the caller's own + // namespace passes the guard and enqueues (returns a job UUID). + let resp = authed( + client().post(format!("{base}/u/test-user-2/ownapp")), + "SECRET_TOKEN_2", + ) + .json(&json!({ + "args": {}, + "component": "comp", + "id": 999778, + "raw_code": { + "language": "deno", + "content": "export function main() { return 1; }", + "path": "u/test-user-2/ownapp/comp" + }, + "force_viewer_static_fields": {} + })) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert!( + status.is_success(), + "persisted preview for an app the caller owns must still succeed (got {status}): {body}" + ); + assert!( + uuid::Uuid::parse_str(body.trim()).is_ok(), + "successful persisted preview must return a job UUID, got: {body}" + ); + + // 8. Scope escalation: a token scoped to `apps:run` (but not `jobs:run`) + // can reach this route (it maps to the `apps` scope domain) and is not an + // Operator, but must NOT be able to enqueue arbitrary preview `raw_code`. + // `/jobs/run/preview` requires `jobs:run` for exactly this reason; the + // app preview path must enforce the same. Without the `jobs:run` check + // this enqueues a job (returns a UUID); with it, it is rejected (403). + let resp = authed( + client().post(format!("{base}/u/test-user-2/myapp")), + "APPS_RUN_TOKEN", + ) + .json(&preview_body("u/test-user-2/myapp")) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!( + status, 403, + "apps:run-scoped token must not escalate to arbitrary preview code (got {status}): {body}" + ); + assert!( + body.contains("jobs:run"), + "rejection must be the jobs:run scope gate, got: {body}" + ); + + Ok(()) +} diff --git a/backend/tests/fixtures/app_preview_auth.sql b/backend/tests/fixtures/app_preview_auth.sql new file mode 100644 index 0000000000..9fcda61c51 --- /dev/null +++ b/backend/tests/fixtures/app_preview_auth.sql @@ -0,0 +1,34 @@ +-- Fixture for the app component preview authorization regression test. +-- Layered on top of `base` (which provides test-workspace, the admin +-- `test-user`/SECRET_TOKEN, and the non-operator `test-user-2`/SECRET_TOKEN_2). +-- Adds an Operator member so we can assert that Operators cannot reach the +-- arbitrary-code app preview path (`force_viewer_static_fields` + `raw_code`). + +INSERT INTO password(email, password_hash, login_type, super_admin, verified, name) + VALUES ('operator@windmill.dev', 'not-a-real-hash', 'password', false, true, 'Operator User'); + +INSERT INTO usr(workspace_id, email, username, is_admin, operator, role) VALUES + ('test-workspace', 'operator@windmill.dev', 'operator-user', false, true, 'Operator'); + +INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin) VALUES + (encode(sha256('OPERATOR_TOKEN'::bytea), 'hex'), 'OPERATOR_T', 'OPERATOR_TOKEN', 'operator@windmill.dev', 'operator token', false); + +-- A non-operator token scoped to `apps:run` but NOT `jobs:run`. It can reach +-- the `apps_u/execute_component` route (route maps to the `apps` scope domain) +-- but must not be able to enqueue arbitrary preview `raw_code`. +INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin, scopes) VALUES + (encode(sha256('APPS_RUN_TOKEN'::bytea), 'hex'), 'APPS_RUN_T', 'APPS_RUN_TOKEN', 'test2@windmill.dev', 'apps:run scoped token', false, '{apps:run}'); + +-- A private app owned by `test-user` with a persisted inline script. Used to +-- assert that `test-user-2` cannot preview-execute another app's app_script id. +INSERT INTO app (id, workspace_id, path, summary, policy, versions) VALUES + (999001, 'test-workspace', 'u/test-user/private', 'private app', '{}'::jsonb, '{}'); +INSERT INTO app_script (id, app, hash, code, code_sha256) VALUES + (999777, 999001, repeat('a', 64), 'export function main(){ return "secret" }', repeat('b', 64)); + +-- An app owned by `test-user-2` with its own persisted inline script, to assert +-- the id-ownership check does not over-block a legitimate persisted preview. +INSERT INTO app (id, workspace_id, path, summary, policy, versions) VALUES + (999002, 'test-workspace', 'u/test-user-2/ownapp', 'own app', '{}'::jsonb, '{}'); +INSERT INTO app_script (id, app, hash, code, code_sha256) VALUES + (999778, 999002, repeat('c', 64), 'export function main(){ return "ok" }', repeat('d', 64)); diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 9c64bb0da4..4ecbae879a 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -11,7 +11,7 @@ use crate::{ auth::{get_end_user_email, OptTokened}, db::{ApiAuthed, DB}, jobs::RunJobQuery, - users::{require_owner_of_path, OptAuthed}, + users::{require_owner_of_path, require_path_read_access_for_preview, OptAuthed}, utils::{check_scopes, WithStarredInfoQuery}, webhook_util::{WebhookMessage, WebhookShared}, HTTP_CLIENT, @@ -2138,6 +2138,54 @@ async fn execute_component( // tag from the deployed policy and ignore the request body. let is_preview = payload.force_viewer_static_fields.is_some(); + // Preview mode runs request-supplied code as a `Viewer`-mode job (the + // app-editor equivalent of `/jobs/run/preview`), so it enforces the same + // guards. Operators must never run preview jobs. `jobs:run` is required + // because this route is reachable with an `apps:run`-scoped token (the + // route maps to the `apps` scope domain), which must not be able to escalate + // to arbitrary code execution. The client-supplied inline `raw_code.tag` + // must stay within the caller's allowed worker tags. A preview can also + // *reference* an existing runnable the caller may not be allowed to read — a + // deployed script/flow via `payload.path` or a persisted `app_script` via + // `payload.id`, both resolved with the root DB handle — so those (and only + // those) are confined to paths the caller can read. Inline `raw_code` is not + // path-gated: a non-operator member can already run arbitrary inline code + // via `/jobs/run/preview`. + if is_preview { + let authed = opt_authed.as_ref().ok_or_else(|| { + Error::NotAuthorized("App component preview requires authentication".to_string()) + })?; + if authed.is_operator { + return Err(Error::NotAuthorized( + "Operators cannot run preview jobs for security reasons".to_string(), + )); + } + check_scopes(authed, || format!("jobs:run"))?; + if let Some(p) = payload.path.as_deref() { + let runnable_path = p + .strip_prefix("script/") + .or_else(|| p.strip_prefix("flow/")) + .unwrap_or(p); + require_path_read_access_for_preview(authed, &Some(runnable_path.to_string()))?; + } + if let Some(id) = payload.id { + let owner_path = sqlx::query_scalar!( + "SELECT a.path FROM app_script s JOIN app a ON a.id = s.app + WHERE s.id = $1 AND a.workspace_id = $2", + id, + &w_id, + ) + .fetch_optional(&db) + .await? + .ok_or_else(|| { + Error::NotAuthorized(format!( + "App script {id} does not belong to an app in this workspace" + )) + })?; + require_path_read_access_for_preview(authed, &Some(owner_path))?; + } + } + // Two cases here: // 1. The component is executed from the editor (i.e. in "preview" mode), then: // - The policy is set to default (in `Viewer` execution mode). @@ -2341,6 +2389,20 @@ async fn execute_component( ), _ => unreachable!(), }; + // Preview honors the client-supplied inline tag (`resolved_inline_tag`), so + // — like `/jobs/run/preview` — confine it to worker tags the caller may use + // (a `if_jobs:filter_tags`-restricted token must not escape its filter). + // `is_preview` implies an authed caller (the guard above returns otherwise). + if is_preview { + if let Some(authed) = opt_authed.as_ref() { + crate::jobs::check_tag_available_for_workspace(&db, &w_id, &tag, authed).await?; + } + } + // Identity is already resolved to the requesting user in preview mode (the + // policy is forced to `ExecutionMode::Viewer`, so the job runs as the + // caller). The enqueue stays root-isolated as before — switching the insert + // to user-RLS is not what contains the bypass (the auth guards above are) + // and would add unnecessary breakage risk to the legitimate editor flow. let tx = PushIsolationLevel::IsolatedRoot(db.clone()); let (email, permissioned_as) = if let Some(on_behalf_of) = on_behalf_of.as_ref() { From 457a78cc6a5742afac1ad6d5cd51e9f37e77ab6a Mon Sep 17 00:00:00 2001 From: hugocasa Date: Wed, 20 May 2026 08:02:53 +0200 Subject: [PATCH 05/29] update webmux config for oneshot (#9250) --- .webmux.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.webmux.yaml b/.webmux.yaml index 19a0ea9c30..14f16180a0 100644 --- a/.webmux.yaml +++ b/.webmux.yaml @@ -100,10 +100,12 @@ profiles: integrations: github: + autoRemoveOnMerge: true linkedRepos: - repo: windmill-labs/windmill-ee-private alias: ee-private dir: ../windmill-ee-private__worktrees linear: enabled: true + autoCreateWorktrees: true watchTeams: [WIN,GIT] From d08f72b3e1ef194b5d656cafa15bc88e8b6ba731 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 20 May 2026 06:09:26 +0000 Subject: [PATCH 06/29] feat(vault): optional KV secret path prefix setting (WIN-1960) (#9249) * feat(vault): add optional KV secret path prefix setting (WIN-1960) Co-Authored-By: Claude Opus 4.7 (1M context) * chore: update ee-repo-ref to 0189ba6504fd70eb4929e4881d624d48efd14aee This commit updates the EE repository reference after PR #581 was merged in windmill-ee-private. Previous ee-repo-ref: e32e8d6483550c67897e09b6f900dff1034bdae8 New ee-repo-ref: 0189ba6504fd70eb4929e4881d624d48efd14aee Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 4.7 (1M context) Co-authored-by: windmill-internal-app[bot] --- backend/ee-repo-ref.txt | 2 +- backend/windmill-api/openapi.yaml | 3 ++ .../windmill-common/src/secret_backend/mod.rs | 7 ++++ .../src/secret_backend/tests.rs | 1 + .../tests/secret_backend_integration.rs | 2 ++ .../tests/secret_backend_migration.rs | 1 + .../SecretBackendConfig.svelte | 32 +++++++++++++++++-- 7 files changed, 45 insertions(+), 3 deletions(-) diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index c64abf81cd..45b6cdc229 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -017d36418a65ce5c840c502e3174df0c393612ba +0189ba6504fd70eb4929e4881d624d48efd14aee diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 00fd942598..4b3da914f7 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -21076,6 +21076,9 @@ components: mount_path: type: string description: KV v2 secrets engine mount path (e.g., windmill) + kv_secret_path_prefix: + type: string + description: Optional path prefix inserted between the KV data/metadata segment and the workspace id (e.g., "apps/windmill"). When set, secrets are stored at `/data///`, allowing a Vault policy scoped to exactly `/data//*`. jwt_role: type: string description: Vault JWT auth role name for Windmill (optional, if not provided token auth is used) diff --git a/backend/windmill-common/src/secret_backend/mod.rs b/backend/windmill-common/src/secret_backend/mod.rs index 36f35f0cf8..71fa2ea999 100644 --- a/backend/windmill-common/src/secret_backend/mod.rs +++ b/backend/windmill-common/src/secret_backend/mod.rs @@ -118,6 +118,13 @@ pub struct VaultSettings { pub address: String, /// KV v2 mount path (e.g., "windmill") pub mount_path: String, + /// Optional path prefix inserted between the KV `data`/`metadata` segment + /// and the workspace id, e.g. "apps/windmill". When set, secrets live at + /// `/data///`, so a Vault policy can be + /// scoped to exactly `/data//*`. Surrounding slashes are + /// trimmed. + #[serde(skip_serializing_if = "Option::is_none")] + pub kv_secret_path_prefix: Option, /// JWT auth role name configured in Vault (used for JWT/OIDC auth) /// Optional - if not provided, token auth is used #[serde(skip_serializing_if = "Option::is_none")] diff --git a/backend/windmill-common/src/secret_backend/tests.rs b/backend/windmill-common/src/secret_backend/tests.rs index 3e2a12382a..8f04a755b7 100644 --- a/backend/windmill-common/src/secret_backend/tests.rs +++ b/backend/windmill-common/src/secret_backend/tests.rs @@ -25,6 +25,7 @@ mod tests { VaultSettings { address: "http://127.0.0.1:8200".to_string(), mount_path: "windmill".to_string(), + kv_secret_path_prefix: None, jwt_role: Some("windmill-secrets".to_string()), jwt_mount_path: None, namespace: None, diff --git a/backend/windmill-common/tests/secret_backend_integration.rs b/backend/windmill-common/tests/secret_backend_integration.rs index 350fc297af..ff41f3acf7 100644 --- a/backend/windmill-common/tests/secret_backend_integration.rs +++ b/backend/windmill-common/tests/secret_backend_integration.rs @@ -90,6 +90,7 @@ mod tests { address: std::env::var("VAULT_ADDR") .unwrap_or_else(|_| "http://127.0.0.1:8200".to_string()), mount_path: "windmill".to_string(), + kv_secret_path_prefix: None, jwt_role: None, // Static token mode jwt_mount_path: None, namespace: None, @@ -106,6 +107,7 @@ mod tests { address: std::env::var("VAULT_ADDR") .unwrap_or_else(|_| "http://127.0.0.1:8200".to_string()), mount_path: "windmill".to_string(), + kv_secret_path_prefix: None, jwt_role: Some("windmill-secrets".to_string()), // JWT mode jwt_mount_path: None, namespace: None, diff --git a/backend/windmill-common/tests/secret_backend_migration.rs b/backend/windmill-common/tests/secret_backend_migration.rs index fba27ee260..5a4086fa1b 100644 --- a/backend/windmill-common/tests/secret_backend_migration.rs +++ b/backend/windmill-common/tests/secret_backend_migration.rs @@ -34,6 +34,7 @@ fn test_vault_settings() -> VaultSettings { address: std::env::var("VAULT_ADDR") .unwrap_or_else(|_| "http://127.0.0.1:8200".to_string()), mount_path: "windmill".to_string(), + kv_secret_path_prefix: None, jwt_role: Some("windmill-secrets".to_string()), jwt_mount_path: None, namespace: None, diff --git a/frontend/src/lib/components/instanceSettings/SecretBackendConfig.svelte b/frontend/src/lib/components/instanceSettings/SecretBackendConfig.svelte index b14dec9846..7ecec72dfd 100644 --- a/frontend/src/lib/components/instanceSettings/SecretBackendConfig.svelte +++ b/frontend/src/lib/components/instanceSettings/SecretBackendConfig.svelte @@ -69,6 +69,7 @@ type: 'HashiCorpVault', address: $values['secret_backend']?.address ?? '', mount_path: $values['secret_backend']?.mount_path ?? 'windmill', + kv_secret_path_prefix: $values['secret_backend']?.kv_secret_path_prefix ?? null, jwt_role: $values['secret_backend']?.jwt_role ?? 'windmill-secrets', jwt_mount_path: $values['secret_backend']?.jwt_mount_path ?? null, namespace: $values['secret_backend']?.namespace ?? null, @@ -122,6 +123,7 @@ return { address: $values['secret_backend'].address, mount_path: $values['secret_backend'].mount_path, + kv_secret_path_prefix: $values['secret_backend'].kv_secret_path_prefix || undefined, jwt_role: $values['secret_backend'].jwt_role, jwt_mount_path: $values['secret_backend'].jwt_mount_path || undefined, namespace: $values['secret_backend'].namespace || undefined, @@ -355,6 +357,11 @@ let baseUrl = $derived($values['base_url'] ?? 'https://your-windmill-instance.com') let jwtMount = $derived(($values['secret_backend']?.jwt_mount_path?.trim() || 'jwt') as string) + let kvPrefix = $derived( + ($values['secret_backend']?.kv_secret_path_prefix?.trim().replace(/^\/+|\/+$/g, '') || + '') as string + ) + let kvPolicyPath = $derived(kvPrefix ? `${kvPrefix}/*` : '*') let vaultAudience = $derived( ($values['secret_backend']?.address?.trim() || 'https://vault.example.com:8200') as string ) @@ -453,6 +460,27 @@ bind:value={$values['secret_backend'].mount_path} />
+
+ + Optional prefix inserted before the workspace id. When set, secrets are stored at + <mount>/data/<prefix>/<workspace>/<secret>, so you + can keep an existing layout and scope a Vault policy to exactly + {$values['secret_backend']?.mount_path ?? 'windmill'}/data/{kvPolicyPath}. + +
Authentication Method setAuthMethod(v)}> @@ -541,10 +569,10 @@ vault write auth/{jwtMount}/config \ # Create a policy for Windmill secrets vault policy write windmill-secrets - <<EOF -path "{$values['secret_backend']?.mount_path ?? 'windmill'}/data/*" { +path "{$values['secret_backend']?.mount_path ?? 'windmill'}/data/{kvPolicyPath}" { capabilities = ["create", "read", "update", "delete"] } -path "{$values['secret_backend']?.mount_path ?? 'windmill'}/metadata/*" { +path "{$values['secret_backend']?.mount_path ?? 'windmill'}/metadata/{kvPolicyPath}" { capabilities = ["list", "delete"] } EOF From ef0cb49f7403a8e8dc234345a8cfd9a664faba28 Mon Sep 17 00:00:00 2001 From: hugocasa Date: Wed, 20 May 2026 08:47:02 +0200 Subject: [PATCH 07/29] chore(claude): harden main-branch guard and gate claude.ai MCP tools (#9248) - guard-main-branch.sh: exit 2 on block (was advisory echo), and block force-push to main from any branch (--force, -f, --force-with-lease, +ref) - settings.json: gate claude.ai MCP connectors (Stripe, Gmail, Calendar, Drive, Slack, Linear) behind permissions.ask Co-authored-by: Claude Opus 4.7 (1M context) --- .claude/hooks/guard-main-branch.sh | 19 ++++++++++++++++++- .claude/settings.json | 8 +++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/.claude/hooks/guard-main-branch.sh b/.claude/hooks/guard-main-branch.sh index c7eeea9475..7a3a8189a0 100755 --- a/.claude/hooks/guard-main-branch.sh +++ b/.claude/hooks/guard-main-branch.sh @@ -16,6 +16,23 @@ command="$(echo "$input" | jq -r '.tool_input.command // empty')" if [[ "$command" =~ ^git\ (push|reset|revert|checkout|merge|rebase|commit|add) ]]; then branch="$(git rev-parse --abbrev-ref HEAD 2>/dev/null || true)" if [[ "$branch" == "main" ]]; then - echo "BLOCK: You are on the main branch. Create or switch to a feature branch first." + echo "BLOCK: You are on the main branch. Create or switch to a feature branch first." >&2 + exit 2 + fi +fi + +# Block force-push targeting main from any branch. +if [[ "$command" =~ ^git[[:space:]]+push([[:space:]]|$) ]]; then + has_force=false + if [[ "$command" =~ (--force([[:space:]]|=|$)|--force-with-lease|[[:space:]]-f([[:space:]]|$)) ]]; then + has_force=true + fi + # `+ref` refspec syntax is also a force push. + if [[ "$command" =~ [[:space:]]\+[A-Za-z] ]]; then + has_force=true + fi + if $has_force && [[ "$command" =~ (^|[[:space:]:])\+?main([[:space:]]|$) ]]; then + echo "BLOCK: Force-push to main is not allowed via Claude. Run it yourself if you really mean to." >&2 + exit 2 fi fi diff --git a/.claude/settings.json b/.claude/settings.json index 1ef3704831..24575147e4 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -72,7 +72,13 @@ "Bash(chown:*)", "Bash(truncate:*)", "Bash(shred:*)", - "Bash(unlink:*)" + "Bash(unlink:*)", + "mcp__claude_ai_Stripe", + "mcp__claude_ai_Gmail", + "mcp__claude_ai_Google_Calendar", + "mcp__claude_ai_Google_Drive", + "mcp__claude_ai_Slack", + "mcp__claude_ai_Linear" ] }, "enableAllProjectMcpServers": true, From aa12c66c25e68eefec22c213e8f228fd0699d8ce Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 20 May 2026 06:48:10 +0000 Subject: [PATCH 08/29] feat(snowflake): derive public key from private key when omitted (WIN-1959) (#9251) * feat(snowflake): derive public key from private key when omitted (WIN-1959) Snowflake key-pair auth needs a SHA256 fingerprint of the public key for the JWT iss claim, but the public key is mathematically derivable from the RSA private key. Other tools (e.g. Power BI) only require the private key, so requiring users to supply both is redundant. When public_key is missing, fall back to deriving it from private_key (PKCS#8 or PKCS#1 PEM) instead of erroring out. Fixes WIN-1959 Co-Authored-By: Claude Opus 4.7 (1M context) * fix(snowflake): treat empty public_key/private_key as missing --------- Co-authored-by: Claude Opus 4.7 (1M context) --- backend/Cargo.lock | 1 + backend/windmill-worker/Cargo.toml | 3 +- .../windmill-worker/src/snowflake_executor.rs | 48 ++++++++++++++++--- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 1a3efc3e11..b8b0ab8625 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -15667,6 +15667,7 @@ dependencies = [ "regex", "reqwest 0.13.1", "reqwest-middleware", + "rsa", "rust_decimal", "serde", "serde_json", diff --git a/backend/windmill-worker/Cargo.toml b/backend/windmill-worker/Cargo.toml index 5ebcb0b370..2e75ac3632 100644 --- a/backend/windmill-worker/Cargo.toml +++ b/backend/windmill-worker/Cargo.toml @@ -13,7 +13,7 @@ default = [] private = ["windmill-worker-volumes/private", "windmill-queue/private", "windmill-common/private", "windmill-dep-map/private", "windmill-runtime-nativets?/private"] mcp = ["windmill-ai/mcp", "dep:windmill-mcp"] prometheus = ["dep:prometheus", "windmill-common/prometheus"] -enterprise = ["windmill-queue/enterprise", "windmill-git-sync/enterprise", "windmill-common/enterprise", "windmill-worker-volumes/enterprise", "windmill-runtime-nativets?/enterprise", "dep:pem", "dep:tokio-util", "dep:opentelemetry-proto", "dep:prost", "dep:hudsucker", "dep:rcgen", "dep:hyper-http-proxy", "dep:hyper-tls", "dep:hyper-util"] +enterprise = ["windmill-queue/enterprise", "windmill-git-sync/enterprise", "windmill-common/enterprise", "windmill-worker-volumes/enterprise", "windmill-runtime-nativets?/enterprise", "dep:pem", "dep:rsa", "dep:tokio-util", "dep:opentelemetry-proto", "dep:prost", "dep:hudsucker", "dep:rcgen", "dep:hyper-http-proxy", "dep:hyper-tls", "dep:hyper-util"] mssql = ["dep:tiberius"] mssql-kerberos = ["mssql", "tiberius/integrated-auth-gssapi"] # Linux/Unix integrated auth mssql-winauth = ["mssql", "tiberius/winauth"] # Windows integrated auth @@ -112,6 +112,7 @@ jsonwebtoken.workspace = true sha2.workspace = true hmac.workspace = true pem = { workspace = true, optional = true } +rsa = { workspace = true, optional = true } urlencoding.workspace = true nix.workspace = true bytes.workspace = true diff --git a/backend/windmill-worker/src/snowflake_executor.rs b/backend/windmill-worker/src/snowflake_executor.rs index 839609d9da..35051aee98 100644 --- a/backend/windmill-worker/src/snowflake_executor.rs +++ b/backend/windmill-worker/src/snowflake_executor.rs @@ -630,14 +630,50 @@ pub async fn do_snowflake( ) .to_uppercase(); - let public_key = match database.public_key.as_deref() { - Some(key) => pem::parse(key.as_bytes()).map_err(|e| { - Error::ExecutionErr(format!("Failed to parse public key: {}", e.to_string())) - })?, - None => return Err(Error::ExecutionErr("Public key is missing".to_string())), + let public_key_der: Vec = match database + .public_key + .as_deref() + .map(str::trim) + .filter(|s| !s.is_empty()) + { + Some(key) => pem::parse(key.as_bytes()) + .map_err(|e| Error::ExecutionErr(format!("Failed to parse public key: {e}")))? + .into_contents(), + None => { + // Derive the public key from the private key — RSA private keys + // contain the public components (n, e). + use rsa::pkcs8::{DecodePrivateKey, EncodePublicKey}; + let pk_pem = database + .private_key + .as_deref() + .map(str::trim) + .filter(|s| !s.is_empty()) + .ok_or_else(|| { + Error::ExecutionErr( + "Either public_key or private_key must be provided".to_string(), + ) + })?; + let rsa_priv = rsa::RsaPrivateKey::from_pkcs8_pem(pk_pem) + .or_else(|_| { + use rsa::pkcs1::DecodeRsaPrivateKey; + rsa::RsaPrivateKey::from_pkcs1_pem(pk_pem) + }) + .map_err(|e| { + Error::ExecutionErr(format!( + "Failed to parse private key to derive public key: {e}" + )) + })?; + let rsa_pub = rsa::RsaPublicKey::from(&rsa_priv); + rsa_pub + .to_public_key_der() + .map_err(|e| { + Error::ExecutionErr(format!("Failed to encode derived public key: {e}")) + })? + .to_vec() + } }; let mut public_key_hash = Sha256::new(); - public_key_hash.update(public_key.contents()); + public_key_hash.update(&public_key_der); let public_key_fp = engine::general_purpose::STANDARD.encode(public_key_hash.finalize()); From f066c3df1fbf4849b2f56001ef3185e83b56969e Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Wed, 20 May 2026 07:07:28 +0000 Subject: [PATCH 09/29] chore: allow claude to do stuff in /tmp --- .claude/settings.json | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/.claude/settings.json b/.claude/settings.json index 24575147e4..ca8d9a898d 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -44,7 +44,25 @@ "Bash(git merge:*)", "Bash(git rebase:*)", "Bash(git add:*)", - "Bash(git commit:*)" + "Bash(git commit:*)", + "Read(/tmp/**)", + "Write(/tmp/**)", + "Edit(/tmp/**)", + "Bash(rm:/tmp/*)", + "Bash(rm:/tmp/**)", + "Bash(rmdir:/tmp/*)", + "Bash(mkdir:/tmp/*)", + "Bash(mkdir:/tmp/**)", + "Bash(cp:/tmp/*)", + "Bash(cp:/tmp/**)", + "Bash(mv:/tmp/*)", + "Bash(mv:/tmp/**)", + "Bash(touch:/tmp/*)", + "Bash(touch:/tmp/**)", + "Bash(chmod:/tmp/*)", + "Bash(chmod:/tmp/**)", + "Bash(tar * /tmp/*)", + "Bash(unzip * /tmp/*)" ], "deny": [ "Read(.env)", From 31a046973af960764ee4e153b68c020cbd4690ce Mon Sep 17 00:00:00 2001 From: Guilhem Date: Wed, 20 May 2026 09:10:25 +0200 Subject: [PATCH 10/29] =?UTF-8?q?feat(chat):=20visual=20redesign=20?= =?UTF-8?q?=E2=80=94=20input,=20streaming=20indicator,=20scroll=20polish?= =?UTF-8?q?=20(#9232)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(chat): visual redesign — input, streaming indicator, scroll polish Visual refresh of the AI chat surface used in both the global right-side panel (Cmd+L) and inline editor panels. No new features, no system-prompt or tool changes, no sessions code. Input redesign - Default textarea to `rows={1}` and autosize as the user types. - Drop the separate Send button row in favour of a single `
- - {/snippet} - -
- + {/if} {#if messages.length === 0} - You can use {getModifierKey()}L to open or close this chat, and {getModifierKey()}K in the - script editor to modify selected lines. + {#if emptyHint} + {@render emptyHint()} + {:else} + You can use {getModifierKey()}L to open or close this chat, and {getModifierKey()}K in the + script editor to modify selected lines. + {/if} {/if} {#if messages.length > 0} -
{ - aiChatManager.disableAutomaticScroll() - }} - > -
- {#each messages as message, messageIndex (messageIndex)} - - {/each} - {#if aiChatManager.loading && !aiChatManager.currentReply && !isLastMessageTool} -
- -
- {/if} +
+
+
+ {#each messages as message, messageIndex (messageIndex)} + + {/each} + {#if showTypingIndicator} +
+ +
+ {/if} +
+ {#if showScrollToLatest} +
+
+ {/if}
{/if} -
0} class="relative"> - {#if aiChatManager.loading} -
- -
- {:else if aiChatManager.flowAiChatHelpers?.hasPendingChanges()} +
+ {#if aiChatManager.flowAiChatHelpers?.hasPendingChanges()}
{/if} -
+
+ {#if inputPreface} + {@render inputPreface()} + {/if} {:else}
- + {#if !hideModeSelector} + + {/if} {#if aiChatManager.mode === AIMode.APP} {/if} @@ -344,8 +427,8 @@ {#each suggestions as suggestion (suggestion)}
From 790987831380611b5bd19a760b0a5433492d7796 Mon Sep 17 00:00:00 2001 From: Guilhem Date: Wed, 20 May 2026 14:54:23 +0200 Subject: [PATCH 19/29] feat(chat): waiting-for-user indicator + scroll-to-latest polish (#9252) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(chat): waiting-for-user indicator and arrow polish - Show "Waiting for your input" (text-accent + flipping Hourglass) instead of the typing dots when the latest tool is staged for confirmation (Run/Cancel) or has an active askUserQuestion. The dots imply the AI is working, which is misleading when the loop is paused on the user. - Scroll-to-latest arrow: - Move up to bottom-12 when the flow Accept/Reject row is visible so they no longer overlap. - Wrap in a solid bg-surface + shadow + border badge so the icon doesn't bleed into messages behind it. - Bump unifiedSize xs → sm for a slightly larger target. - Hourglass uses a custom CSS keyframe (:global so the rule reaches the Lucide SVG root) with 4 s period and cubic-bezier(0.65, 0, 0.35, 1) easing — feels like flipping the hourglass rather than spinning. * fix(chat): raise waiting indicator above accept/reject row * fix(chat): solid background behind reject all button * feat(chat): @ picker in controls row, badges above input, polish --- .../copilot/chat/AIChatDisplay.svelte | 165 +++++++++++++++--- .../copilot/chat/AIChatInput.svelte | 75 +++----- 2 files changed, 160 insertions(+), 80 deletions(-) diff --git a/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte b/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte index 3e57542dbf..5967a75d30 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte @@ -1,10 +1,13 @@ -{#key $tutorialsToDo} - - {#snippet buttonReplacement()} -
diff --git a/frontend/src/lib/components/apps/editor/settingsPanel/common/PanelSection.svelte b/frontend/src/lib/components/apps/editor/settingsPanel/common/PanelSection.svelte index 4fa88a021b..eda78badd8 100644 --- a/frontend/src/lib/components/apps/editor/settingsPanel/common/PanelSection.svelte +++ b/frontend/src/lib/components/apps/editor/settingsPanel/common/PanelSection.svelte @@ -1,6 +1,12 @@ + + @@ -932,6 +969,8 @@ onUndo={handleUndo} onRedo={handleRedo} onOpenYamlEditor={() => yamlEditorDrawer?.openDrawer()} + sidebarCollapsed={sidebarCollapsed.val} + onToggleSidebar={() => (sidebarCollapsed.val = !sidebarCollapsed.val)} /> - - files, - (newFiles) => { - files = newFiles - setFilesInIframe(newFiles ?? {}) + {#if !sidebarCollapsed.val} + + files, + (newFiles) => { + files = newFiles + setFilesInIframe(newFiles ?? {}) + } } - } - onSelectFile={handleSelectFile} - bind:selectedRunnable - bind:selectedDocument - dataTableRefs={dataTableRefsObjects} - onDataTableRefsChange={(newRefs) => { - data.tables = newRefs.map(formatDataTableRef) - saveFrontendDraft() - }} - defaultDatatable={data.datatable} - defaultSchema={data.schema} - onDefaultChange={(datatable, schema) => { - data.datatable = datatable - data.schema = schema - // Also sync to aiChatManager - aiChatManager.datatableCreationPolicy = { - ...aiChatManager.datatableCreationPolicy, - datatable, - schema - } - saveFrontendDraft() - }} - {runnables} - {modules} - {historyManager} - historySelectedId={historyManager.selectedEntryId} - onHistorySelect={handleHistorySelect} - onHistorySelectCurrent={() => { - // Restore the temporary current state if it exists - const tempState = historyManager.getAndClearTemporaryState() - if (tempState) { - applyEntry(tempState) - } - // Clear selection to indicate we're at current state - historyManager.clearSelection() - }} - onManualSnapshot={() => { - historyManager.manualSnapshot(files ?? {}, runnables, summary, data, true) - }} - > - + onSelectFile={handleSelectFile} + bind:selectedRunnable + bind:selectedDocument + dataTableRefs={dataTableRefsObjects} + onDataTableRefsChange={(newRefs) => { + data.tables = newRefs.map(formatDataTableRef) + saveFrontendDraft() + }} + defaultDatatable={data.datatable} + defaultSchema={data.schema} + onDefaultChange={(datatable, schema) => { + data.datatable = datatable + data.schema = schema + // Also sync to aiChatManager + aiChatManager.datatableCreationPolicy = { + ...aiChatManager.datatableCreationPolicy, + datatable, + schema + } + saveFrontendDraft() + }} + {runnables} + {modules} + {historyManager} + historySelectedId={historyManager.selectedEntryId} + onHistorySelect={handleHistorySelect} + onHistorySelectCurrent={() => { + // Restore the temporary current state if it exists + const tempState = historyManager.getAndClearTemporaryState() + if (tempState) { + applyEntry(tempState) + } + // Clear selection to indicate we're at current state + historyManager.clearSelection() + }} + onManualSnapshot={() => { + historyManager.manualSnapshot(files ?? {}, runnables, summary, data, true) + }} + > + + {/if} -
+