diff --git a/.github/workflows/publish-cli-docs.yml b/.github/workflows/publish-cli-docs.yml new file mode 100644 index 0000000000..9e76117eb5 --- /dev/null +++ b/.github/workflows/publish-cli-docs.yml @@ -0,0 +1,84 @@ +name: Publish CLI docs repo + +# Regenerates the windmill-cli-docs repo (consumed by context7) from the +# canonical sources in this repo on every Windmill release. +# +# Required secret: +# CLI_DOCS_DEPLOY_KEY — ed25519 private key whose public half is registered +# as a write-access deploy key on +# windmill-labs/windmill-cli-docs. + +on: + push: + tags: + - "v*" + workflow_dispatch: + +# Serialize pushes to windmill-cli-docs so two release tags landing close +# together (e.g. a release-please bump + a hotfix) can't race to force-push +# the docs repo. +concurrency: + group: publish-cli-docs + cancel-in-progress: false + +jobs: + publish: + runs-on: ubuntu-latest + steps: + - name: Checkout windmill (source of truth) + uses: actions/checkout@v4 + with: + path: windmill + + - name: Checkout windmill-cli-docs (publish target) + uses: actions/checkout@v4 + with: + repository: windmill-labs/windmill-cli-docs + path: windmill-cli-docs + ssh-key: ${{ secrets.CLI_DOCS_DEPLOY_KEY }} + fetch-depth: 0 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install dependencies + run: pip install pyyaml + + - name: Regenerate docs + run: | + python3 windmill/system_prompts/generate.py \ + --context7-dir "$GITHUB_WORKSPACE/windmill-cli-docs" + + - name: Commit and push if changed + working-directory: windmill-cli-docs + env: + REF_NAME: ${{ github.ref_name }} + REF_TYPE: ${{ github.ref_type }} + run: | + git config user.name "windmill-bot" + git config user.email "bot@windmill.dev" + git add -A + if git diff --cached --quiet; then + echo "No doc changes for ${REF_NAME}." + committed=false + else + committed=true + if [ "${REF_TYPE}" = "tag" ]; then + git commit -m "chore: sync from windmill ${REF_NAME}" + else + git commit -m "chore: sync from windmill (manual dispatch from ${REF_NAME})" + fi + git push origin HEAD + fi + # Always mirror the version tag on tag pushes, even when content + # didn't change — downstream consumers tie snapshots to releases by + # tag, and skipping it would leave the docs repo without a tag for + # the new Windmill release. + # workflow_dispatch from a non-tag ref skips this so we don't + # create a junk tag named after a branch. + if [ "${REF_TYPE}" = "tag" ]; then + git tag -f "${REF_NAME}" + git push origin "${REF_NAME}" --force + echo "Mirrored tag ${REF_NAME} to windmill-cli-docs (content changed: ${committed})." + fi diff --git a/.gitignore b/.gitignore index 10889080ad..5f733611de 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,7 @@ rust-client/Cargo.toml # Worktree-specific Claude Code settings (generated by scripts/worktree-env) .claude/settings.local.json +.claude/worktrees/ # Symlinked cache directories (for git worktrees) backend/target diff --git a/CHANGELOG.md b/CHANGELOG.md index e297309df2..c3524f552c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,50 @@ # Changelog +## [1.702.1](https://github.com/windmill-labs/windmill/compare/v1.702.0...v1.702.1) (2026-05-14) + + +### Bug Fixes + +* **nativets:** pass tracing-enabled OtelConfig to deno_telemetry::init ([#9163](https://github.com/windmill-labs/windmill/issues/9163)) ([bf99283](https://github.com/windmill-labs/windmill/commit/bf99283c3333bcdbc7679f4aea04ba29e41a48a5)) + +## [1.702.0](https://github.com/windmill-labs/windmill/compare/v1.701.0...v1.702.0) (2026-05-14) + + +### Features + +* **git-sync:** sync extra_perms for flows/scripts/apps ([#9162](https://github.com/windmill-labs/windmill/issues/9162)) ([5e909b2](https://github.com/windmill-labs/windmill/commit/5e909b2b4f2819f19deaf06d9e78e6458b324683)) +* include service accounts in instance settings users list ([#9157](https://github.com/windmill-labs/windmill/issues/9157)) ([e5286f4](https://github.com/windmill-labs/windmill/commit/e5286f46074cf2893e6ccd26175f929f16011c8f)) + + +### Bug Fixes + +* **mcp:** sanitize and enrich nested resource schemas ([#9158](https://github.com/windmill-labs/windmill/issues/9158)) ([d870edc](https://github.com/windmill-labs/windmill/commit/d870edc959481a06c894b4eda5e2be1a0269d7d0)) + +## [1.701.0](https://github.com/windmill-labs/windmill/compare/v1.700.2...v1.701.0) (2026-05-13) + + +### Features + +* **frontend:** unified EditorHeader with file picker for flow/script/app editors ([#9047](https://github.com/windmill-labs/windmill/issues/9047)) ([d0f23cc](https://github.com/windmill-labs/windmill/commit/d0f23cc5238b025208c61e983701894de28536d5)) +* read-only flag on API tokens ([#9144](https://github.com/windmill-labs/windmill/issues/9144)) ([d666e84](https://github.com/windmill-labs/windmill/commit/d666e8431cdbf14d9373d9ef625b5aafc50ac50a)) + + +### Bug Fixes + +* align script path existence check with deploy logic; hide Delete for non-admin ([#9152](https://github.com/windmill-labs/windmill/issues/9152)) ([c509206](https://github.com/windmill-labs/windmill/commit/c5092069cbeda2c4c18bea80dd629c7c087b30bf)) +* Allow devops role to use all_workspaces runs filter in admins workspace ([#9153](https://github.com/windmill-labs/windmill/issues/9153)) ([110bef0](https://github.com/windmill-labs/windmill/commit/110bef0a6e76615c7b371c5c0f5bc1f4e7a73a64)) +* **bun:** pass --preserve-symlinks on unbundled execution ([#9147](https://github.com/windmill-labs/windmill/issues/9147)) ([4d0f2c2](https://github.com/windmill-labs/windmill/commit/4d0f2c26a116a0f8a89a64231dc824eabda0a8c3)) +* **cli:** prevent !inline-corruption in flow push/pull ([#9142](https://github.com/windmill-labs/windmill/issues/9142)) ([79c5b7b](https://github.com/windmill-labs/windmill/commit/79c5b7b8b7676b0a06fa6480dd04b7105d39d250)) +* **operator:** refresh IAM RDS / Entra ID tokens in operator process ([#9141](https://github.com/windmill-labs/windmill/issues/9141)) ([7ebb081](https://github.com/windmill-labs/windmill/commit/7ebb08133cd4027bc00bacc4a0fc5865cd5709ec)) +* **python:** preserve strings containing Infinity/NaN in result JSON ([#9149](https://github.com/windmill-labs/windmill/issues/9149)) ([33bf01b](https://github.com/windmill-labs/windmill/commit/33bf01b627c8ea430c03dfc27a97a8f2d770582f)) +* scope promotion-mode debounce key per repo ([#9145](https://github.com/windmill-labs/windmill/issues/9145)) ([2ec1863](https://github.com/windmill-labs/windmill/commit/2ec1863340e759bba3408dbc4f41b16912b959ea)) +* send flow push-loop ping outside transaction so zombie monitor sees it ([#9136](https://github.com/windmill-labs/windmill/issues/9136)) ([818cb31](https://github.com/windmill-labs/windmill/commit/818cb31fbc731fa5c70ddf5942bb37bc4bc56e4d)) + + +### Performance Improvements + +* **dynselect:** only retrigger when helper args actually change ([#9148](https://github.com/windmill-labs/windmill/issues/9148)) ([dd19e52](https://github.com/windmill-labs/windmill/commit/dd19e52a84fb9a9f48e3ad061b084841c2ee7464)) + ## [1.700.2](https://github.com/windmill-labs/windmill/compare/v1.700.1...v1.700.2) (2026-05-12) diff --git a/ai_evals/AGENTS.md b/ai_evals/AGENTS.md index 096baf5b58..d26e6d60ea 100644 --- a/ai_evals/AGENTS.md +++ b/ai_evals/AGENTS.md @@ -6,6 +6,7 @@ This folder contains black-box benchmark cases for: - `app` - `script` - `cli` +- `global` The goal is to test the current production prompts and guidance with realistic user requests, not to test one exact implementation shape. @@ -75,6 +76,16 @@ Still, avoid benchmark phrasing. The prompt should read like a repo task, not a When relevant, ask the assistant to tell the user which `wmill` commands to run next. That is part of the benchmarked behavior. +## Global-specific rules + +Global prompts should exercise workspace-level drafting behavior: + +- inspecting existing scripts, flows, apps, schedules, triggers, resources, and variables when relevant +- writing AI drafts rather than saving or deploying by default +- producing coherent multi-artifact changes when the request crosses artifact boundaries + +Keep deterministic validation focused on the draft contract: required draft type/path, required content snippets, forbidden draft paths, and forbidden mutating tools such as deploy/delete unless the case explicitly asks for them. + ## Deterministic validation Use deterministic validation only for hard failures such as: diff --git a/ai_evals/README.md b/ai_evals/README.md index 2e1f3210f8..6982d70da9 100644 --- a/ai_evals/README.md +++ b/ai_evals/README.md @@ -1,11 +1,12 @@ # AI Evals -Small benchmark runner for the four Windmill AI generation modes: +Small benchmark runner for the Windmill AI generation modes: - `cli` - `flow` - `script` - `app` +- `global` The benchmark always tests the current production prompts, tools, and guidance in this checkout. @@ -57,6 +58,7 @@ bun run cli -- run flow flow-test0-sum-two-numbers --runs 3 --verbose bun run cli -- run flow --record GEMINI_API_KEY=... bun run cli -- run app app-test1-counter-create --model gemini-pro WMILL_AI_EVAL_BACKEND_URL=http://127.0.0.1:8000 bun run cli -- run flow --backend-validation preview +bun run cli -- run global global-test1-script-create bun run cli -- run cli bun-hello-script ``` @@ -94,7 +96,7 @@ Today: Notes: - the command also prints accepted alias spellings such as `gpt-4o`, `claude-opus-4.6`, and `claude-haiku-4.5` -- frontend modes (`flow`, `script`, `app`) can use Anthropic, OpenAI, and Gemini-backed aliases +- frontend modes (`flow`, `script`, `app`, `global`) can use Anthropic, OpenAI, and Gemini-backed aliases - `cli` mode always uses the Anthropic agent SDK, so only Anthropic aliases are valid there - the judge model is separate and currently defaults to `claude-sonnet-4-6` @@ -133,6 +135,13 @@ For `app` mode, `validate` can express narrow hard requirements such as: - minimum datatable / datatable-table counts - specific required datatable tables +For `global` mode, `validate` can express draft-level requirements such as: + +- required draft type/path/language +- required or forbidden snippets in draft values +- required or forbidden draft counts +- forbidden draft paths + App fixtures can also include an optional `datatables.json` file at the fixture root. For `flow` mode, an `initial` fixture can also include a benchmark workspace catalog of @@ -174,6 +183,7 @@ If `--record` is used, the CLI also appends one compact JSON line to: - `ai_evals/history/flow.jsonl` - `ai_evals/history/script.jsonl` - `ai_evals/history/app.jsonl` +- `ai_evals/history/global.jsonl` - `ai_evals/history/cli.jsonl` Each recorded line contains: @@ -194,6 +204,7 @@ Typical artifacts by mode: - `flow`: `flow.json` - `script`: `script.json` plus the generated script file - `app`: `app.json` plus frontend/backend files +- `global`: `global-drafts.json` - `cli`: `assistant-output.txt`, `trace.json`, `wmill-invocations.jsonl`, plus generated workspace files - backend-validated attempts also include `backend-preview.json` @@ -209,6 +220,7 @@ Typical artifacts by mode: ## Notes - Frontend modes reuse the production frontend chat code through the Vitest bridge. +- Global mode evaluates the production global AI tools and validates the resulting AI draft store. - CLI mode creates an isolated workspace, writes the current checkout guidance into it, and benchmarks the real skills / `AGENTS.md` flow. - CLI mode now also records a structured trace of invoked skills, tool calls, proposed `wmill` commands, and any attempted `wmill` executions. - Frontend progress streams live while the benchmark is running. diff --git a/ai_evals/adapters/frontend/benchmarkRunner.ts b/ai_evals/adapters/frontend/benchmarkRunner.ts index 50a0d10c3c..14be108a10 100644 --- a/ai_evals/adapters/frontend/benchmarkRunner.ts +++ b/ai_evals/adapters/frontend/benchmarkRunner.ts @@ -12,10 +12,11 @@ import { resolveWindmillBackendSettings } from "../../core/windmillBackendSettin import { emitFrontendBenchmarkProgress } from "./progress"; import { createAppModeRunner } from "../../modes/app"; import { createFlowModeRunner } from "../../modes/flow"; +import { createGlobalModeRunner } from "../../modes/global"; import { createScriptModeRunner } from "../../modes/script"; import { DEFAULT_JUDGE_MODEL } from "../../core/judge"; -export type FrontendBenchmarkMode = "flow" | "app" | "script"; +export type FrontendBenchmarkMode = "flow" | "app" | "script" | "global"; export async function runFrontendBenchmarkFromEnv(): Promise { const mode = parseMode(process.env.WMILL_FRONTEND_AI_EVAL_MODE); @@ -85,11 +86,13 @@ function getModeRunner( backendValidation, backendSettings, ); + case "global": + return createGlobalModeRunner(model, backendSettings); } } function parseMode(value: string | undefined): FrontendBenchmarkMode { - if (value === "flow" || value === "app" || value === "script") { + if (value === "flow" || value === "app" || value === "script" || value === "global") { return value; } throw new Error(`Unsupported frontend benchmark mode: ${String(value)}`); diff --git a/ai_evals/adapters/frontend/core/global/globalEvalRunner.ts b/ai_evals/adapters/frontend/core/global/globalEvalRunner.ts new file mode 100644 index 0000000000..5e00dd6f34 --- /dev/null +++ b/ai_evals/adapters/frontend/core/global/globalEvalRunner.ts @@ -0,0 +1,127 @@ +import { mkdtemp, rm } from "fs/promises"; +import { tmpdir } from "os"; +import { join } from "path"; +import type { AIProvider } from "$lib/gen/types.gen"; +import { + globalTools, + prepareGlobalSystemMessage, + prepareGlobalUserMessage, +} from "../../../../../frontend/src/lib/components/copilot/chat/global/core"; +import { globalDraftStore } from "../../../../../frontend/src/lib/components/copilot/chat/global/draftStore.svelte"; +import type { Tool as ProductionTool } from "../../../../../frontend/src/lib/components/copilot/chat/shared"; +import type { ModeRunContext } from "../../../../core/types"; +import type { GlobalDraftState } from "../../../../core/validators"; +import type { WindmillBackendSettings } from "../../../../core/windmillBackendSettings"; +import { + registerBenchmarkWorkspaceRunnables, + unregisterBenchmarkWorkspaceRunnables, + type BenchmarkWorkspaceRunnables, +} from "../../mockBackend"; +import { runEval } from "../shared"; +import type { TokenUsage, ToolCallDetail } from "../shared/types"; + +const MUTATING_GLOBAL_TOOLS = new Set([ + "deploy_workspace_item", + "delete_workspace_item", +]); + +export interface GlobalEvalResult { + success: boolean; + state: GlobalDraftState; + error?: string; + assistantMessageCount: number; + toolCallCount: number; + toolsUsed: string[]; + toolCallDetails: ToolCallDetail[]; + tokenUsage: TokenUsage; +} + +export interface GlobalEvalOptions { + workspaceFixtures?: BenchmarkWorkspaceRunnables; + model?: string; + maxIterations?: number; + provider?: AIProvider; + backend: WindmillBackendSettings; + workspaceRoot?: string; + runContext?: ModeRunContext; +} + +export async function runGlobalEval( + userPrompt: string, + apiKey: string, + options: GlobalEvalOptions, +): Promise { + const workspaceRoot = + options.workspaceRoot ?? + (await mkdtemp(join(tmpdir(), "wmill-frontend-global-benchmark-"))); + + globalDraftStore.clearDrafts(workspaceRoot); + registerBenchmarkWorkspaceRunnables(workspaceRoot, options.workspaceFixtures ?? {}); + + try { + const model = options.model ?? "claude-haiku-4-5-20251001"; + const rawResult = await runEval({ + userPrompt, + systemMessage: prepareGlobalSystemMessage(), + userMessage: prepareGlobalUserMessage(userPrompt), + tools: getGlobalEvalTools(), + helpers: {}, + apiKey, + getOutput: () => ({ drafts: globalDraftStore.listDrafts(workspaceRoot) }), + onAssistantMessageStart: options.runContext?.onAssistantMessageStart, + onAssistantToken: options.runContext?.onAssistantChunk, + onAssistantMessageEnd: options.runContext?.onAssistantMessageEnd, + onToolCall: options.runContext?.onToolCall, + options: { + maxIterations: options.maxIterations, + model, + workspace: workspaceRoot, + provider: options.provider, + backend: options.backend, + caseId: options.runContext?.caseId, + attempt: options.runContext?.attempt, + }, + }); + + return { + state: rawResult.output, + success: rawResult.success, + error: rawResult.error, + assistantMessageCount: rawResult.iterations, + toolCallCount: rawResult.toolCallsCount, + toolsUsed: rawResult.toolsCalled, + toolCallDetails: rawResult.toolCallDetails, + tokenUsage: rawResult.tokenUsage, + }; + } finally { + globalDraftStore.clearDrafts(workspaceRoot); + unregisterBenchmarkWorkspaceRunnables(workspaceRoot); + if (!options.workspaceRoot) { + await rm(workspaceRoot, { recursive: true, force: true }); + } + } +} + +function getGlobalEvalTools(): ProductionTool<{}>[] { + return (globalTools as ProductionTool<{}>[]).map((tool) => { + if (!MUTATING_GLOBAL_TOOLS.has(tool.def.function.name)) { + return tool; + } + + return { + ...tool, + requiresConfirmation: false, + validateBeforeConfirmation: undefined, + fn: async () => + JSON.stringify( + { + success: false, + error: + "This mutating workspace tool is disabled during ai_evals global mode.", + }, + null, + 2, + ), + }; + }); +} diff --git a/ai_evals/adapters/frontend/progress.ts b/ai_evals/adapters/frontend/progress.ts index b5b8f12c83..3a4810c4a3 100644 --- a/ai_evals/adapters/frontend/progress.ts +++ b/ai_evals/adapters/frontend/progress.ts @@ -1,4 +1,4 @@ -export type FrontendBenchmarkProgressSurface = 'flow' | 'app' | 'script' +export type FrontendBenchmarkProgressSurface = 'flow' | 'app' | 'script' | 'global' export type FrontendBenchmarkProgressEvent = | { diff --git a/ai_evals/adapters/frontend/runtime.ts b/ai_evals/adapters/frontend/runtime.ts index 180c7a2993..347e15191c 100644 --- a/ai_evals/adapters/frontend/runtime.ts +++ b/ai_evals/adapters/frontend/runtime.ts @@ -16,7 +16,7 @@ const FRONTEND_BENCHMARK_TEST = const FRONTEND_BENCHMARK_CONFIG = "../ai_evals/adapters/frontend/vitest.config.ts"; -export type FrontendMode = "flow" | "app" | "script"; +export type FrontendMode = "flow" | "app" | "script" | "global"; export async function runFrontendBenchmarkAdapter(input: { mode: FrontendMode; diff --git a/ai_evals/adapters/frontend/vitestAdapter.test.ts b/ai_evals/adapters/frontend/vitestAdapter.test.ts index 542feaf89b..1275acf0b4 100644 --- a/ai_evals/adapters/frontend/vitestAdapter.test.ts +++ b/ai_evals/adapters/frontend/vitestAdapter.test.ts @@ -65,6 +65,10 @@ vi.mock('$lib/gen', async () => { hasBenchmarkWorkspace(data.workspace) ? (listBenchmarkScripts(data.workspace) ?? []) : actual.ScriptService.listScripts(data), + existsScriptByPath: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? Boolean(getBenchmarkScriptByPath(data.workspace, data.path)) + : actual.ScriptService.existsScriptByPath(data), getScriptByPath: async (data: { workspace: string; path: string }) => { if (hasBenchmarkWorkspace(data.workspace)) { const script = getBenchmarkScriptByPath(data.workspace, data.path) @@ -91,6 +95,10 @@ vi.mock('$lib/gen', async () => { hasBenchmarkWorkspace(data.workspace) ? (listBenchmarkFlows(data.workspace) ?? []) : actual.FlowService.listFlows(data), + existsFlowByPath: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? Boolean(getBenchmarkFlowByPath(data.workspace, data.path)) + : actual.FlowService.existsFlowByPath(data), getFlowByPath: async (data: { workspace: string; path: string }) => { if (hasBenchmarkWorkspace(data.workspace)) { const flow = getBenchmarkFlowByPath(data.workspace, data.path) @@ -142,6 +150,16 @@ vi.mock('$lib/gen', async () => { } }), ScheduleService: wrapService(actual.ScheduleService, { + existsSchedule: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.ScheduleService.existsSchedule(data), + listSchedules: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.ScheduleService.listSchedules(data), + getSchedule: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Schedule "${data.path}" not found in benchmark workspace`) + } + return actual.ScheduleService.getSchedule(data) + }, previewSchedule: async (data: { requestBody?: Record }) => previewBenchmarkSchedule(data), createSchedule: async (data: { workspace: string; requestBody: Record }) => @@ -149,11 +167,167 @@ vi.mock('$lib/gen', async () => { ? createBenchmarkSchedule(data) : actual.ScheduleService.createSchedule(data) }), + ResourceService: wrapService(actual.ResourceService, { + existsResource: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.ResourceService.existsResource(data), + listResource: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.ResourceService.listResource(data), + getResource: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Resource "${data.path}" not found in benchmark workspace`) + } + return actual.ResourceService.getResource(data) + }, + queryResourceTypes: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.ResourceService.queryResourceTypes(data) + }), + VariableService: wrapService(actual.VariableService, { + existsVariable: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.VariableService.existsVariable(data), + listVariable: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.VariableService.listVariable(data), + getVariable: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Variable "${data.path}" not found in benchmark workspace`) + } + return actual.VariableService.getVariable(data) + } + }), + AppService: wrapService(actual.AppService, { + existsApp: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.AppService.existsApp(data), + listApps: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.AppService.listApps(data), + getAppByPath: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`App "${data.path}" not found in benchmark workspace`) + } + return actual.AppService.getAppByPath(data) + } + }), HttpTriggerService: wrapService(actual.HttpTriggerService, { + existsHttpTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.HttpTriggerService.existsHttpTrigger(data), + listHttpTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.HttpTriggerService.listHttpTriggers(data), + getHttpTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`HTTP trigger "${data.path}" not found in benchmark workspace`) + } + return actual.HttpTriggerService.getHttpTrigger(data) + }, createHttpTrigger: async (data: { workspace: string; requestBody: Record }) => hasBenchmarkWorkspace(data.workspace) ? createBenchmarkHttpTrigger(data) : actual.HttpTriggerService.createHttpTrigger(data) + }), + WebsocketTriggerService: wrapService(actual.WebsocketTriggerService, { + existsWebsocketTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? false + : actual.WebsocketTriggerService.existsWebsocketTrigger(data), + listWebsocketTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) + ? [] + : actual.WebsocketTriggerService.listWebsocketTriggers(data), + getWebsocketTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Websocket trigger "${data.path}" not found in benchmark workspace`) + } + return actual.WebsocketTriggerService.getWebsocketTrigger(data) + } + }), + KafkaTriggerService: wrapService(actual.KafkaTriggerService, { + existsKafkaTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? false + : actual.KafkaTriggerService.existsKafkaTrigger(data), + listKafkaTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.KafkaTriggerService.listKafkaTriggers(data), + getKafkaTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Kafka trigger "${data.path}" not found in benchmark workspace`) + } + return actual.KafkaTriggerService.getKafkaTrigger(data) + } + }), + NatsTriggerService: wrapService(actual.NatsTriggerService, { + existsNatsTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.NatsTriggerService.existsNatsTrigger(data), + listNatsTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.NatsTriggerService.listNatsTriggers(data), + getNatsTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`NATS trigger "${data.path}" not found in benchmark workspace`) + } + return actual.NatsTriggerService.getNatsTrigger(data) + } + }), + PostgresTriggerService: wrapService(actual.PostgresTriggerService, { + existsPostgresTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? false + : actual.PostgresTriggerService.existsPostgresTrigger(data), + listPostgresTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) + ? [] + : actual.PostgresTriggerService.listPostgresTriggers(data), + getPostgresTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Postgres trigger "${data.path}" not found in benchmark workspace`) + } + return actual.PostgresTriggerService.getPostgresTrigger(data) + } + }), + MqttTriggerService: wrapService(actual.MqttTriggerService, { + existsMqttTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.MqttTriggerService.existsMqttTrigger(data), + listMqttTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.MqttTriggerService.listMqttTriggers(data), + getMqttTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`MQTT trigger "${data.path}" not found in benchmark workspace`) + } + return actual.MqttTriggerService.getMqttTrigger(data) + } + }), + SqsTriggerService: wrapService(actual.SqsTriggerService, { + existsSqsTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.SqsTriggerService.existsSqsTrigger(data), + listSqsTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.SqsTriggerService.listSqsTriggers(data), + getSqsTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`SQS trigger "${data.path}" not found in benchmark workspace`) + } + return actual.SqsTriggerService.getSqsTrigger(data) + } + }), + GcpTriggerService: wrapService(actual.GcpTriggerService, { + existsGcpTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) ? false : actual.GcpTriggerService.existsGcpTrigger(data), + listGcpTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.GcpTriggerService.listGcpTriggers(data), + getGcpTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`GCP trigger "${data.path}" not found in benchmark workspace`) + } + return actual.GcpTriggerService.getGcpTrigger(data) + } + }), + AzureTriggerService: wrapService(actual.AzureTriggerService, { + existsAzureTrigger: async (data: { workspace: string; path: string }) => + hasBenchmarkWorkspace(data.workspace) + ? false + : actual.AzureTriggerService.existsAzureTrigger(data), + listAzureTriggers: async (data: { workspace: string }) => + hasBenchmarkWorkspace(data.workspace) ? [] : actual.AzureTriggerService.listAzureTriggers(data), + getAzureTrigger: async (data: { workspace: string; path: string }) => { + if (hasBenchmarkWorkspace(data.workspace)) { + throw new Error(`Azure trigger "${data.path}" not found in benchmark workspace`) + } + return actual.AzureTriggerService.getAzureTrigger(data) + } }) } }) diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml new file mode 100644 index 0000000000..b4526f8a85 --- /dev/null +++ b/ai_evals/cases/global.yaml @@ -0,0 +1,89 @@ +- id: global-test1-script-create + prompt: |- + Create a draft Bun script at `f/evals/global/greet_user`. + It should take a string `name` input and return `Hello, ${name}!`. + Leave it as an AI draft only; do not deploy or save it. + runtime: + maxTurns: 8 + validate: + draftCountExactly: 1 + requiredDrafts: + - type: script + path: f/evals/global/greet_user + language: bun + valueIncludes: + - name + - Hello + toolExpect: + requiredToolsUsed: + - write_script + forbiddenToolsUsed: + - deploy_workspace_item + - delete_workspace_item + judgeChecklist: + - creates a Bun script draft at f/evals/global/greet_user + - the script accepts a name input + - the script returns a greeting containing Hello, the provided name, and an exclamation mark + - the result stays as an AI draft and is not deployed or saved to the workspace + +- id: global-test2-script-edit-existing + prompt: |- + Update the existing workspace script at `f/evals/global/format_greeting`. + Keep it as a Bun script, but change the greeting so the provided name is uppercased and the returned message ends with an exclamation mark. + Leave the result as an AI draft only; do not deploy or save it. + initial: ai_evals/fixtures/frontend/global/initial/format_greeting_script.json + runtime: + maxTurns: 8 + validate: + draftCountExactly: 1 + requiredDrafts: + - type: script + path: f/evals/global/format_greeting + language: bun + valueIncludes: + - toUpperCase + - "!" + toolExpect: + forbiddenToolsUsed: + - deploy_workspace_item + - delete_workspace_item + judgeChecklist: + - creates an AI draft for the existing f/evals/global/format_greeting script + - preserves the script as Bun + - uppercases the provided name in the greeting + - returns a message ending with an exclamation mark + - does not deploy or save the draft to the workspace + +- id: global-test3-flow-create + prompt: |- + Create a draft flow at `f/evals/global/sum_numbers`. + It should take two numeric inputs, `a` and `b`, and return their sum. + Leave it as an AI draft only; do not deploy or save it. + runtime: + maxTurns: 8 + validate: + draftCountExactly: 1 + requiredDrafts: + - type: flow + path: f/evals/global/sum_numbers + valueIncludes: + - modules + - rawscript + - flow_input.a + - flow_input.b + toolExpect: + requiredToolsUsed: + - write_flow + forbiddenToolsUsed: + - deploy_workspace_item + - delete_workspace_item + toolCallArgs: + - tool: write_flow + field: modules + stringStartsWithAnyOf: + - "[" + judgeChecklist: + - creates a flow draft at f/evals/global/sum_numbers + - the flow accepts numeric inputs a and b + - the flow returns the sum of a and b + - the result stays as an AI draft and is not deployed or saved to the workspace diff --git a/ai_evals/cli/index.ts b/ai_evals/cli/index.ts index 259b055ff5..8ed61740c8 100644 --- a/ai_evals/cli/index.ts +++ b/ai_evals/cli/index.ts @@ -53,6 +53,7 @@ async function main() { " bun run cli -- run flow --record", " bun run cli -- run flow --backend-validation preview", " bun run cli -- run flow flow-test5-simple-modification --runs 3", + " bun run cli -- run global global-test1-script-create", " bun run cli -- run cli bun-hello-script", "", "Models:", @@ -70,7 +71,7 @@ async function main() { program .command("cases") .description("List available cases") - .argument("[mode]", "cli, flow, script, or app", parseOptionalMode) + .argument("[mode]", "cli, flow, script, app, or global", parseOptionalMode) .action(async (mode?: EvalMode) => { await handleCases(mode); }); @@ -78,7 +79,7 @@ async function main() { program .command("run") .description("Run one benchmark mode") - .argument("", "cli, flow, script, or app", parseMode) + .argument("", "cli, flow, script, app, or global", parseMode) .argument("[caseIds...]", "specific case ids to run") .option( "--runs ", @@ -152,7 +153,7 @@ function handleModels() { process.stdout.write("Available models\n"); for (const model of EVAL_MODELS) { const supports = [ - ...(model.frontend ? ["flow", "script", "app"] : []), + ...(model.frontend ? ["flow", "script", "app", "global"] : []), ...(model.cli ? ["cli"] : []), ]; const aliases = [ diff --git a/ai_evals/core/cases.test.ts b/ai_evals/core/cases.test.ts index 977bb71390..733d34ddd2 100644 --- a/ai_evals/core/cases.test.ts +++ b/ai_evals/core/cases.test.ts @@ -183,6 +183,26 @@ describe("loadCases", () => { }); }); + it("loads global draft validation and forbidden tool expectations", async () => { + const globalCases = await loadCases("global"); + const caseEntry = globalCases.find((entry) => entry.id === "global-test1-script-create"); + + expect(caseEntry?.validate).toMatchObject({ + draftCountExactly: 1, + requiredDrafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + }, + ], + }); + expect(caseEntry?.toolExpect).toMatchObject({ + requiredToolsUsed: ["write_script"], + forbiddenToolsUsed: ["deploy_workspace_item", "delete_workspace_item"], + }); + }); + it("loads tool expectations for workspace mutation cases", async () => { const scriptCases = await loadCases("script"); const caseEntry = scriptCases.find( diff --git a/ai_evals/core/models.ts b/ai_evals/core/models.ts index 9cc0ab0597..82f3b3f69b 100644 --- a/ai_evals/core/models.ts +++ b/ai_evals/core/models.ts @@ -145,7 +145,7 @@ export function resolveEvalModel(mode: EvalMode, alias?: string): EvalModelSpec export function getEvalModelHelpText(): string { return EVAL_MODELS.map((model) => { const modes = [ - ...(model.frontend ? ["flow", "script", "app"] : []), + ...(model.frontend ? ["flow", "script", "app", "global"] : []), ...(model.cli ? ["cli"] : []), ]; return ` ${model.id.padEnd(8)} ${model.label} (${modes.join(", ")})`; diff --git a/ai_evals/core/types.ts b/ai_evals/core/types.ts index dfc6882f84..2b42a0dfc5 100644 --- a/ai_evals/core/types.ts +++ b/ai_evals/core/types.ts @@ -1,4 +1,4 @@ -export const EVAL_MODES = ["cli", "flow", "script", "app"] as const; +export const EVAL_MODES = ["cli", "flow", "script", "app", "global"] as const; export type EvalMode = (typeof EVAL_MODES)[number]; @@ -108,6 +108,27 @@ export interface AppValidationSpec { forbiddenAppContent?: string[]; } +export interface GlobalDraftRequirement { + type: string; + path: string; + triggerKind?: string; + language?: string; + summaryIncludes?: string[]; + valueIncludes?: string[]; + valueExcludes?: string[]; +} + +export interface GlobalValidationSpec { + draftCountAtLeast?: number; + draftCountExactly?: number; + requiredDrafts?: GlobalDraftRequirement[]; + forbiddenDrafts?: Array<{ + type: string; + path: string; + triggerKind?: string; + }>; +} + export interface CliValidationSpec { requiredSkills?: string[]; forbiddenSkills?: string[]; @@ -136,10 +157,11 @@ export interface ToolCallArgumentRule { export interface ToolValidationSpec { requiredToolsUsed?: string[]; + forbiddenToolsUsed?: string[]; toolCallArgs?: ToolCallArgumentRule[]; } -export type EvalValidationSpec = FlowValidationSpec | AppValidationSpec; +export type EvalValidationSpec = FlowValidationSpec | AppValidationSpec | GlobalValidationSpec; export interface EvalCase { id: string; diff --git a/ai_evals/core/validators.test.ts b/ai_evals/core/validators.test.ts index 406172b955..d2a6e954bb 100644 --- a/ai_evals/core/validators.test.ts +++ b/ai_evals/core/validators.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "bun:test"; import { validateAppState, validateCliWorkspace, + validateGlobalState, validateScriptState, validateToolExpectations, } from "./validators"; @@ -117,6 +118,230 @@ describe("validateToolExpectations", () => { details: 'rejected prefixes: schedules/; values: "schedules/greet_user_daily"', }); }); + + it("rejects forbidden tool usage", () => { + const checks = validateToolExpectations({ + run: { + success: true, + actual: {}, + assistantMessageCount: 1, + toolCallCount: 1, + toolsUsed: ["write_script", "deploy_workspace_item"], + skillsInvoked: [], + }, + toolExpect: { + forbiddenToolsUsed: ["deploy_workspace_item"], + }, + }); + + expect(checks).toContainEqual({ + name: "does not use deploy_workspace_item", + passed: false, + details: "tools used: write_script, deploy_workspace_item", + }); + }); +}); + +describe("validateGlobalState", () => { + it("accepts a required script draft", () => { + const checks = validateGlobalState({ + actual: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\n return `Hello, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + validate: { + draftCountExactly: 1, + requiredDrafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + valueIncludes: ["Hello"], + }, + ], + }, + }); + + expect(checks.every((check) => check.passed)).toBe(true); + }); + + it("fails when a required draft is missing", () => { + const checks = validateGlobalState({ + actual: { + drafts: [], + }, + validate: { + requiredDrafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + }, + ], + }, + }); + + expect(checks).toContainEqual({ + name: "global includes script draft f/evals/global/greet_user", + passed: false, + details: "drafts: none", + }); + }); + + it("does not require a TypeScript entrypoint for non-TypeScript script drafts", () => { + const checks = validateGlobalState({ + actual: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_python", + language: "python3", + value: "def main(name: str):\n return f'Hello, {name}!'\n", + isDraft: true, + }, + ], + }, + }); + + expect(checks.some((check) => check.name.includes("exports entrypoint"))).toBe( + false + ); + expect(checks.every((check) => check.passed)).toBe(true); + }); + + it("allows read-only global cases without draft expectations", () => { + const checks = validateGlobalState({ + actual: { + drafts: [], + }, + }); + + expect( + checks.some( + (check) => check.name === "global produced at least one draft" + ) + ).toBe(false); + expect(checks.every((check) => check.passed)).toBe(true); + }); + + it("matches expected global draft fixtures", () => { + const checks = validateGlobalState({ + actual: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\r\n return `Hello, ${name}!`\r\n}\r\n", + isDraft: true, + }, + ], + }, + expected: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\n return `Hello, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + }); + + expect(checks).toContainEqual({ + name: "global drafts match expected", + passed: true, + }); + }); + + it("fails when expected global draft fixtures differ", () => { + const checks = validateGlobalState({ + actual: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\n return `Hello, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + expected: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\n return `Bonjour, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + }); + + const expectedMatchCheck = checks.find( + (check) => check.name === "global drafts match expected" + ); + expect(expectedMatchCheck?.passed).toBe(false); + expect(expectedMatchCheck?.details).toContain( + "script:f/evals/global/greet_user value differs" + ); + expect(expectedMatchCheck?.details).toContain("Hello"); + expect(expectedMatchCheck?.details).toContain("Bonjour"); + }); + + it("explains expected global draft metadata mismatches", () => { + const checks = validateGlobalState({ + actual: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "bun", + value: + "export async function main(name: string) {\n return `Hello, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + expected: { + drafts: [ + { + type: "script", + path: "f/evals/global/greet_user", + language: "python3", + value: + "export async function main(name: string) {\n return `Hello, ${name}!`\n}\n", + isDraft: true, + }, + ], + }, + }); + + const expectedMatchCheck = checks.find( + (check) => check.name === "global drafts match expected" + ); + expect(expectedMatchCheck?.passed).toBe(false); + expect(expectedMatchCheck?.details).toContain( + "script:f/evals/global/greet_user language differs" + ); + expect(expectedMatchCheck?.details).toContain('actual="bun"'); + expect(expectedMatchCheck?.details).toContain('expected="python3"'); + }); }); describe("validateAppState", () => { diff --git a/ai_evals/core/validators.ts b/ai_evals/core/validators.ts index e690b3d7eb..4f59368113 100644 --- a/ai_evals/core/validators.ts +++ b/ai_evals/core/validators.ts @@ -6,6 +6,7 @@ import type { CliTrace, CliValidationSpec, FlowValidationSpec, + GlobalValidationSpec, ModeRunOutput, ToolValidationSpec, } from "./types"; @@ -51,6 +52,20 @@ export interface AppDatatableState { error?: string; } +export interface GlobalDraftState { + drafts: GlobalDraft[]; +} + +export interface GlobalDraft { + type: string; + path: string; + triggerKind?: string; + summary?: string; + language?: string; + value?: unknown; + isDraft?: boolean; +} + const TS_LIKE_LANGUAGES = new Set(["bun", "deno", "nativets", "bunnative", "ts", "typescript"]); const CONTROL_FLOW_MODULE_TYPES = new Set(["branchone", "branchall", "forloopflow", "whileloopflow"]); @@ -154,6 +169,16 @@ export function validateToolExpectations(input: { ); } + for (const toolName of expect.forbiddenToolsUsed ?? []) { + checks.push( + check( + `does not use ${toolName}`, + !input.run.toolsUsed.includes(toolName), + `tools used: ${input.run.toolsUsed.join(", ") || "none"}` + ) + ); + } + for (const rule of expect.toolCallArgs ?? []) { const calls = toolCallDetails.filter((call) => call.name === rule.tool); checks.push( @@ -202,6 +227,161 @@ export function validateToolExpectations(input: { return checks; } +export function validateGlobalState(input: { + actual: GlobalDraftState; + expected?: GlobalDraftState; + validate?: GlobalValidationSpec; +}): BenchmarkCheck[] { + const drafts = input.actual.drafts ?? []; + const checks: BenchmarkCheck[] = []; + + // Read-only global cases are valid; only enforce draft production when the + // case explicitly asks for draft output. + if (globalValidationExpectsDrafts(input)) { + checks.push( + check( + "global produced at least one draft", + drafts.length > 0, + `drafts=${drafts.length}` + ) + ); + } + + checks.push( + check( + "all global outputs are drafts", + drafts.every((draft) => draft.isDraft === true), + summarizeGlobalDrafts(drafts) + ) + ); + + for (const draft of drafts) { + if (draft.type !== "script" || typeof draft.value !== "string") { + continue; + } + + const language = (draft.language ?? "bun").toLowerCase(); + const syntaxErrors = getScriptSyntaxErrors(draft.value, language); + if (TS_LIKE_LANGUAGES.has(language)) { + checks.push( + check( + `script draft ${draft.path} exports entrypoint`, + hasSupportedEntrypoint(draft.value) + ) + ); + } + checks.push( + check( + `script draft ${draft.path} has no syntax errors`, + syntaxErrors.length === 0, + summarizeProblems(syntaxErrors) + ) + ); + } + + if (input.expected) { + checks.push( + check( + "global drafts match expected", + globalDraftStatesEqual(input.actual, input.expected), + describeGlobalDraftStateMismatch(input.actual, input.expected) + ) + ); + } + + const validate = input.validate; + if (!validate) { + return checks; + } + + if (validate.draftCountAtLeast !== undefined) { + checks.push( + check( + `global includes at least ${validate.draftCountAtLeast} draft(s)`, + drafts.length >= validate.draftCountAtLeast, + `drafts=${drafts.length}` + ) + ); + } + + if (validate.draftCountExactly !== undefined) { + checks.push( + check( + `global includes exactly ${validate.draftCountExactly} draft(s)`, + drafts.length === validate.draftCountExactly, + `drafts=${drafts.length}` + ) + ); + } + + for (const required of validate.requiredDrafts ?? []) { + const draft = findGlobalDraft(drafts, required.type, required.path, required.triggerKind); + checks.push( + check( + `global includes ${required.type} draft ${required.path}`, + Boolean(draft), + summarizeGlobalDrafts(drafts) + ) + ); + if (!draft) { + continue; + } + + if (required.language !== undefined) { + checks.push( + check( + `${required.type} draft ${required.path} uses ${required.language}`, + draft.language === required.language, + `language=${draft.language ?? "(none)"}` + ) + ); + } + + for (const snippet of required.summaryIncludes ?? []) { + checks.push( + check( + `${required.type} draft ${required.path} summary includes '${snippet}'`, + normalizeText(draft.summary ?? "").includes(normalizeText(snippet)), + `summary=${draft.summary ?? ""}` + ) + ); + } + + const valueText = stringifyGlobalDraftValue(draft.value); + for (const snippet of required.valueIncludes ?? []) { + checks.push( + check( + `${required.type} draft ${required.path} value includes '${snippet}'`, + normalizeText(valueText).includes(normalizeText(snippet)), + truncateForDetails(valueText) + ) + ); + } + + for (const snippet of required.valueExcludes ?? []) { + checks.push( + check( + `${required.type} draft ${required.path} value excludes '${snippet}'`, + !normalizeText(valueText).includes(normalizeText(snippet)), + truncateForDetails(valueText) + ) + ); + } + } + + for (const forbidden of validate.forbiddenDrafts ?? []) { + checks.push( + check( + `global does not include ${forbidden.type} draft ${forbidden.path}`, + !findGlobalDraft(drafts, forbidden.type, forbidden.path, forbidden.triggerKind), + summarizeGlobalDrafts(drafts) + ) + ); + } + + return checks; +} + export function validateAppState(input: { actual: AppFilesState; initial?: AppFilesState; @@ -433,6 +613,202 @@ function summarizeProblems(problems: string[], limit = 5): string | undefined { return `${problems.slice(0, limit).join("; ")}; ...and ${problems.length - limit} more`; } +function findGlobalDraft( + drafts: GlobalDraft[], + type: string, + path: string, + triggerKind?: string +): GlobalDraft | undefined { + return drafts.find( + (draft) => + draft.type === type && + draft.path === path && + (triggerKind === undefined || draft.triggerKind === triggerKind) + ); +} + +function summarizeGlobalDrafts(drafts: GlobalDraft[]): string { + const summary = drafts + .map((draft) => formatGlobalDraftKey(draft)) + .join(", "); + return `drafts: ${summary || "none"}`; +} + +function formatGlobalDraftKey(draft: GlobalDraft): string { + return `${draft.type}${draft.triggerKind ? `:${draft.triggerKind}` : ""}:${draft.path}`; +} + +function globalValidationExpectsDrafts(input: { + expected?: GlobalDraftState; + validate?: GlobalValidationSpec; +}): boolean { + const validate = input.validate; + return ( + (input.expected?.drafts?.length ?? 0) > 0 || + (validate?.requiredDrafts?.length ?? 0) > 0 || + (validate?.draftCountAtLeast ?? 0) > 0 || + (validate?.draftCountExactly ?? 0) > 0 + ); +} + +function globalDraftStatesEqual(left: GlobalDraftState, right: GlobalDraftState): boolean { + return ( + JSON.stringify(canonicalizeGlobalDrafts(left.drafts ?? [])) === + JSON.stringify(canonicalizeGlobalDrafts(right.drafts ?? [])) + ); +} + +function describeGlobalDraftStateMismatch( + actual: GlobalDraftState, + expected: GlobalDraftState +): string { + const actualDrafts = actual.drafts ?? []; + const expectedDrafts = expected.drafts ?? []; + const actualByKey = new Map( + actualDrafts.map((draft) => [globalDraftSortKey(draft), draft] as const) + ); + const expectedByKey = new Map( + expectedDrafts.map((draft) => [globalDraftSortKey(draft), draft] as const) + ); + + for (const key of Array.from(expectedByKey.keys()).sort()) { + const expectedDraft = expectedByKey.get(key); + if (expectedDraft && !actualByKey.has(key)) { + return `missing expected draft ${formatGlobalDraftKey(expectedDraft)}; actual=${summarizeGlobalDrafts(actualDrafts)}`; + } + } + + for (const key of Array.from(actualByKey.keys()).sort()) { + const actualDraft = actualByKey.get(key); + if (actualDraft && !expectedByKey.has(key)) { + return `unexpected draft ${formatGlobalDraftKey(actualDraft)}; expected=${summarizeGlobalDrafts(expectedDrafts)}`; + } + } + + for (const key of Array.from(expectedByKey.keys()).sort()) { + const actualDraft = actualByKey.get(key); + const expectedDraft = expectedByKey.get(key); + if (!actualDraft || !expectedDraft) { + continue; + } + + const fieldMismatch = describeGlobalDraftFieldMismatch( + formatGlobalDraftKey(expectedDraft), + actualDraft, + expectedDraft + ); + if (fieldMismatch) { + return fieldMismatch; + } + } + + return `actual=${summarizeGlobalDrafts(actualDrafts)}; expected=${summarizeGlobalDrafts(expectedDrafts)}`; +} + +function describeGlobalDraftFieldMismatch( + key: string, + actual: GlobalDraft, + expected: GlobalDraft +): string | undefined { + const fields: Array<"language" | "summary" | "value" | "isDraft"> = [ + "language", + "summary", + "value", + "isDraft", + ]; + + for (const field of fields) { + const actualValue = comparableGlobalDraftFieldValue(actual, field); + const expectedValue = comparableGlobalDraftFieldValue(expected, field); + if (JSON.stringify(actualValue) === JSON.stringify(expectedValue)) { + continue; + } + + return `${key} ${field} differs: actual=${formatGlobalDraftFieldValue( + actualValue + )}; expected=${formatGlobalDraftFieldValue(expectedValue)}`; + } + + return undefined; +} + +function comparableGlobalDraftFieldValue( + draft: GlobalDraft, + field: "language" | "summary" | "value" | "isDraft" +): unknown { + if (field === "summary" && typeof draft.summary === "string") { + return normalizeText(draft.summary); + } + if (field === "value" && typeof draft.value === "string") { + return normalizeText(draft.value); + } + if (field === "value") { + return canonicalizeJsonValue(draft.value); + } + return draft[field]; +} + +function formatGlobalDraftFieldValue(value: unknown): string { + if (value === undefined) { + return "(missing)"; + } + return truncateForDetails(JSON.stringify(value), 300); +} + +function canonicalizeGlobalDrafts(drafts: GlobalDraft[]): unknown[] { + return drafts + .slice() + .sort((left, right) => globalDraftSortKey(left).localeCompare(globalDraftSortKey(right))) + .map((draft) => + canonicalizeJsonValue({ + type: draft.type, + path: draft.path, + triggerKind: draft.triggerKind, + language: draft.language, + summary: + typeof draft.summary === "string" ? normalizeText(draft.summary) : draft.summary, + value: + typeof draft.value === "string" + ? normalizeText(draft.value) + : canonicalizeJsonValue(draft.value), + isDraft: draft.isDraft, + }) + ); +} + +function globalDraftSortKey(draft: GlobalDraft): string { + return `${draft.type}:${draft.triggerKind ?? ""}:${draft.path}`; +} + +function canonicalizeJsonValue(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map(canonicalizeJsonValue); + } + if (value && typeof value === "object") { + return Object.fromEntries( + Object.entries(value as Record) + .sort(([left], [right]) => left.localeCompare(right)) + .map(([key, nested]) => [key, canonicalizeJsonValue(nested)]) + ); + } + return value; +} + +function stringifyGlobalDraftValue(value: unknown): string { + if (typeof value === "string") { + return value; + } + return JSON.stringify(value ?? null, null, 2); +} + +function truncateForDetails(value: string, maxLength = 500): string { + const normalized = value.replace(/\s+/g, " ").trim(); + if (normalized.length <= maxLength) { + return normalized; + } + return `${normalized.slice(0, Math.max(0, maxLength - 3))}...`; +} + function validateCliExpectations( assistantOutput: string, trace: CliTrace | undefined, diff --git a/ai_evals/fixtures/frontend/global/initial/format_greeting_script.json b/ai_evals/fixtures/frontend/global/initial/format_greeting_script.json new file mode 100644 index 0000000000..e66eee2ed2 --- /dev/null +++ b/ai_evals/fixtures/frontend/global/initial/format_greeting_script.json @@ -0,0 +1,23 @@ +{ + "workspace": { + "scripts": [ + { + "path": "f/evals/global/format_greeting", + "summary": "Format a greeting for a provided name", + "description": "Returns a plain greeting for the provided name.", + "language": "bun", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "name": { + "type": "string" + } + }, + "required": ["name"] + }, + "content": "export async function main(name: string) {\n return `Hello, ${name}`\n}\n" + } + ] + } +} diff --git a/ai_evals/modes/global.ts b/ai_evals/modes/global.ts new file mode 100644 index 0000000000..d68df9f5f8 --- /dev/null +++ b/ai_evals/modes/global.ts @@ -0,0 +1,81 @@ +import { readFile } from "node:fs/promises"; +import { runGlobalEval } from "../adapters/frontend/core/global/globalEvalRunner"; +import type { BenchmarkWorkspaceRunnables } from "../adapters/frontend/mockBackend"; +import type { FrontendEvalModelConfig } from "../core/models"; +import type { BenchmarkArtifactFile, GlobalValidationSpec, ModeRunner } from "../core/types"; +import { validateGlobalState, type GlobalDraftState } from "../core/validators"; +import type { WindmillBackendSettings } from "../core/windmillBackendSettings"; +import { getFrontendApiKey } from "./frontendCommon"; + +export interface GlobalInitialFixture { + workspace?: BenchmarkWorkspaceRunnables; +} + +export function createGlobalModeRunner( + modelConfig: FrontendEvalModelConfig, + backendSettings: WindmillBackendSettings, +): ModeRunner { + return { + mode: "global", + concurrency: 3, + judgeThreshold: 80, + async loadInitial(path) { + return path ? await loadGlobalInitialFixture(path) : undefined; + }, + async loadExpected(path) { + return path ? await loadGlobalExpectedFixture(path) : undefined; + }, + async run(prompt, initial, context) { + const result = await runGlobalEval( + prompt, + getFrontendApiKey(modelConfig.provider), + { + workspaceFixtures: initial?.workspace, + maxIterations: context.evalCase?.runtime?.maxTurns, + provider: modelConfig.provider, + model: modelConfig.model, + backend: backendSettings, + runContext: context, + }, + ); + + return { + success: result.success, + actual: result.state, + error: result.error, + assistantMessageCount: result.assistantMessageCount, + toolCallCount: result.toolCallCount, + toolsUsed: result.toolsUsed, + toolCallDetails: result.toolCallDetails, + skillsInvoked: [], + tokenUsage: result.tokenUsage, + }; + }, + validate({ evalCase, actual, expected }) { + return validateGlobalState({ + actual, + expected, + validate: evalCase.validate as GlobalValidationSpec | undefined, + }); + }, + buildArtifacts(actual): BenchmarkArtifactFile[] { + return [ + { + path: "global-drafts.json", + content: JSON.stringify(actual, null, 2) + "\n", + }, + ]; + }, + }; +} + +async function loadGlobalInitialFixture(path: string): Promise { + const parsed = JSON.parse(await readFile(path, "utf8")) as GlobalInitialFixture; + return { + workspace: parsed.workspace ?? {}, + }; +} + +async function loadGlobalExpectedFixture(path: string): Promise { + return JSON.parse(await readFile(path, "utf8")) as GlobalDraftState; +} diff --git a/backend/.sqlx/query-2a49e5b5486b650d96f3e9038cba8a5f2e75d3b12ee4718452e82c7318b1bcf4.json b/backend/.sqlx/query-2a49e5b5486b650d96f3e9038cba8a5f2e75d3b12ee4718452e82c7318b1bcf4.json deleted file mode 100644 index 77f61ccc47..0000000000 --- a/backend/.sqlx/query-2a49e5b5486b650d96f3e9038cba8a5f2e75d3b12ee4718452e82c7318b1bcf4.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT EXISTS(SELECT 1 FROM script WHERE path = $1 AND workspace_id = $2 ORDER BY created_at DESC LIMIT 1)", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "exists", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "2a49e5b5486b650d96f3e9038cba8a5f2e75d3b12ee4718452e82c7318b1bcf4" -} diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 00e8959f72..185cb538c9 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -775,9 +775,9 @@ dependencies = [ [[package]] name = "aws-lc-rs" -version = "1.16.3" +version = "1.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ec6fb3fe69024a75fa7e1bfb48aa6cf59706a101658ea01bfd33b2b248a038f" +checksum = "5ec2f1fc3ec205783a5da9a7e6c1509cc69dedf09a1949e412c1e18469326d00" dependencies = [ "aws-lc-sys", "zeroize", @@ -785,9 +785,9 @@ dependencies = [ [[package]] name = "aws-lc-sys" -version = "0.40.0" +version = "0.41.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f50037ee5e1e41e7b8f9d161680a725bd1626cb6f8c7e901f91f942850852fe7" +checksum = "1a2f9779ce85b93ab6170dd940ad0169b5766ff848247aff13bb788b832fe3f4" dependencies = [ "cc", "cmake", @@ -4559,9 +4559,9 @@ checksum = "28dea519a9695b9977216879a3ebfddf92f1c08c05d984f8996aecd6ecdc811d" [[package]] name = "filetime" -version = "0.2.28" +version = "0.2.29" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2d5b2eef6fafbf69f877e55509ce5b11a760690ac9700a2921be067aa6afaef6" +checksum = "5c287a33c7f0a620c38e641e7f60827713987b3c0f26e8ddc9462cc69cf75759" dependencies = [ "cfg-if", "libc", @@ -8492,18 +8492,18 @@ dependencies = [ [[package]] name = "pin-project" -version = "1.1.12" +version = "1.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cbf0d9e68100b3a7989b4901972f265cd542e560a3a8a724e1e20322f4d06ce9" +checksum = "2466b2336ed02bcdca6b294417127b90ec92038d1d5c4fbeac971a922e0e0924" dependencies = [ "pin-project-internal", ] [[package]] name = "pin-project-internal" -version = "1.1.12" +version = "1.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a990e22f43e84855daf260dded30524ef4a9021cc7541c26540500a50b624389" +checksum = "c96395f0a926bc13b1c17622aaddda1ecb55d49c8f1bf9777e4d877800a43f8b" dependencies = [ "proc-macro2", "quote", @@ -13788,7 +13788,7 @@ dependencies = [ [[package]] name = "windmill" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-nats", @@ -13869,7 +13869,7 @@ dependencies = [ [[package]] name = "windmill-ai" -version = "1.700.2" +version = "1.702.1" dependencies = [ "async-trait", "aws-config", @@ -13899,7 +13899,7 @@ dependencies = [ [[package]] name = "windmill-alerting" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -13912,7 +13912,7 @@ dependencies = [ [[package]] name = "windmill-api" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "argon2", @@ -14055,7 +14055,7 @@ dependencies = [ [[package]] name = "windmill-api-agent-workers" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14078,7 +14078,7 @@ dependencies = [ [[package]] name = "windmill-api-assets" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14091,7 +14091,7 @@ dependencies = [ [[package]] name = "windmill-api-auth" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -14117,7 +14117,7 @@ dependencies = [ [[package]] name = "windmill-api-client" -version = "1.700.2" +version = "1.702.1" dependencies = [ "reqwest 0.12.28", "serde", @@ -14127,7 +14127,7 @@ dependencies = [ [[package]] name = "windmill-api-configs" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14144,7 +14144,7 @@ dependencies = [ [[package]] name = "windmill-api-debug" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "base64 0.22.1", @@ -14166,7 +14166,7 @@ dependencies = [ [[package]] name = "windmill-api-embeddings" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -14189,7 +14189,7 @@ dependencies = [ [[package]] name = "windmill-api-flow-conversations" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14205,7 +14205,7 @@ dependencies = [ [[package]] name = "windmill-api-flows" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14226,7 +14226,7 @@ dependencies = [ [[package]] name = "windmill-api-groups" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14247,7 +14247,7 @@ dependencies = [ [[package]] name = "windmill-api-inputs" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14261,7 +14261,7 @@ dependencies = [ [[package]] name = "windmill-api-integration-tests" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-nats", @@ -14293,7 +14293,7 @@ dependencies = [ [[package]] name = "windmill-api-jobs" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -14318,7 +14318,7 @@ dependencies = [ [[package]] name = "windmill-api-npm-proxy" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "flate2", @@ -14336,7 +14336,7 @@ dependencies = [ [[package]] name = "windmill-api-openapi" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -14358,7 +14358,7 @@ dependencies = [ [[package]] name = "windmill-api-schedule" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14378,7 +14378,7 @@ dependencies = [ [[package]] name = "windmill-api-scripts" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14408,7 +14408,7 @@ dependencies = [ [[package]] name = "windmill-api-settings" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -14436,7 +14436,7 @@ dependencies = [ [[package]] name = "windmill-api-sse" -version = "1.700.2" +version = "1.702.1" dependencies = [ "lazy_static", "serde", @@ -14448,7 +14448,7 @@ dependencies = [ [[package]] name = "windmill-api-users" -version = "1.700.2" +version = "1.702.1" dependencies = [ "argon2", "axum 0.8.9", @@ -14473,7 +14473,7 @@ dependencies = [ [[package]] name = "windmill-api-workers" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14487,7 +14487,7 @@ dependencies = [ [[package]] name = "windmill-api-workspaces" -version = "1.700.2" +version = "1.702.1" dependencies = [ "axum 0.8.9", "chrono", @@ -14520,7 +14520,7 @@ dependencies = [ [[package]] name = "windmill-audit" -version = "1.700.2" +version = "1.702.1" dependencies = [ "chrono", "lazy_static", @@ -14534,7 +14534,7 @@ dependencies = [ [[package]] name = "windmill-autoscaling" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "axum 0.8.9", @@ -14553,7 +14553,7 @@ dependencies = [ [[package]] name = "windmill-common" -version = "1.700.2" +version = "1.702.1" dependencies = [ "aes-gcm", "aho-corasick", @@ -14654,7 +14654,7 @@ dependencies = [ [[package]] name = "windmill-dep-map" -version = "1.700.2" +version = "1.702.1" dependencies = [ "chrono", "itertools 0.14.0", @@ -14673,7 +14673,7 @@ dependencies = [ [[package]] name = "windmill-git-sync" -version = "1.700.2" +version = "1.702.1" dependencies = [ "regex", "serde", @@ -14688,7 +14688,7 @@ dependencies = [ [[package]] name = "windmill-indexer" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "astral-tokio-tar", @@ -14712,7 +14712,7 @@ dependencies = [ [[package]] name = "windmill-jseval" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "futures", @@ -14729,7 +14729,7 @@ dependencies = [ [[package]] name = "windmill-macros" -version = "1.700.2" +version = "1.702.1" dependencies = [ "itertools 0.14.0", "lazy_static", @@ -14745,7 +14745,7 @@ dependencies = [ [[package]] name = "windmill-mcp" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -14766,7 +14766,7 @@ dependencies = [ [[package]] name = "windmill-native-triggers" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -14797,7 +14797,7 @@ dependencies = [ [[package]] name = "windmill-oauth" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "arc-swap", @@ -14822,7 +14822,7 @@ dependencies = [ [[package]] name = "windmill-object-store" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-stream", @@ -14856,7 +14856,7 @@ dependencies = [ [[package]] name = "windmill-operator" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "futures", @@ -14874,7 +14874,7 @@ dependencies = [ [[package]] name = "windmill-parser" -version = "1.700.2" +version = "1.702.1" dependencies = [ "convert_case 0.6.0", "serde", @@ -14883,7 +14883,7 @@ dependencies = [ [[package]] name = "windmill-parser-bash" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "lazy_static", @@ -14895,7 +14895,7 @@ dependencies = [ [[package]] name = "windmill-parser-csharp" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde_json", @@ -14907,7 +14907,7 @@ dependencies = [ [[package]] name = "windmill-parser-go" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "gosyn", @@ -14919,7 +14919,7 @@ dependencies = [ [[package]] name = "windmill-parser-graphql" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "lazy_static", @@ -14931,7 +14931,7 @@ dependencies = [ [[package]] name = "windmill-parser-java" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde_json", @@ -14943,7 +14943,7 @@ dependencies = [ [[package]] name = "windmill-parser-nu" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "nu-parser", @@ -14954,7 +14954,7 @@ dependencies = [ [[package]] name = "windmill-parser-php" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "itertools 0.14.0", @@ -14965,7 +14965,7 @@ dependencies = [ [[package]] name = "windmill-parser-py" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "itertools 0.14.0", @@ -14977,7 +14977,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-asset" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "rustpython-ast", @@ -14988,7 +14988,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-imports" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-recursion", @@ -15010,7 +15010,7 @@ dependencies = [ [[package]] name = "windmill-parser-r" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde_json", @@ -15022,7 +15022,7 @@ dependencies = [ [[package]] name = "windmill-parser-ruby" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "lazy_static", @@ -15036,7 +15036,7 @@ dependencies = [ [[package]] name = "windmill-parser-rust" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "convert_case 0.6.0", @@ -15053,7 +15053,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "lazy_static", @@ -15066,7 +15066,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql-asset" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde", @@ -15078,7 +15078,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "lazy_static", @@ -15096,7 +15096,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts-asset" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde-wasm-bindgen", @@ -15112,7 +15112,7 @@ dependencies = [ [[package]] name = "windmill-parser-wac" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "rustpython-ast", @@ -15128,7 +15128,7 @@ dependencies = [ [[package]] name = "windmill-parser-yaml" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde", @@ -15139,7 +15139,7 @@ dependencies = [ [[package]] name = "windmill-queue" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-recursion", @@ -15176,7 +15176,7 @@ dependencies = [ [[package]] name = "windmill-runtime-nativets" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "const_format", @@ -15214,7 +15214,7 @@ dependencies = [ [[package]] name = "windmill-sql-datatype-parser-wasm" -version = "1.700.2" +version = "1.702.1" dependencies = [ "getrandom 0.3.4", "wasm-bindgen", @@ -15225,7 +15225,7 @@ dependencies = [ [[package]] name = "windmill-store" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-recursion", @@ -15255,7 +15255,7 @@ dependencies = [ [[package]] name = "windmill-test-utils" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15279,7 +15279,7 @@ dependencies = [ [[package]] name = "windmill-trigger" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15312,7 +15312,7 @@ dependencies = [ [[package]] name = "windmill-trigger-azure" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15345,7 +15345,7 @@ dependencies = [ [[package]] name = "windmill-trigger-email" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15365,7 +15365,7 @@ dependencies = [ [[package]] name = "windmill-trigger-gcp" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15399,7 +15399,7 @@ dependencies = [ [[package]] name = "windmill-trigger-http" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15435,7 +15435,7 @@ dependencies = [ [[package]] name = "windmill-trigger-kafka" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15458,7 +15458,7 @@ dependencies = [ [[package]] name = "windmill-trigger-mqtt" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15482,7 +15482,7 @@ dependencies = [ [[package]] name = "windmill-trigger-nats" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-nats", @@ -15506,7 +15506,7 @@ dependencies = [ [[package]] name = "windmill-trigger-postgres" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15541,7 +15541,7 @@ dependencies = [ [[package]] name = "windmill-trigger-sqs" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15569,7 +15569,7 @@ dependencies = [ [[package]] name = "windmill-trigger-websocket" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-trait", @@ -15592,7 +15592,7 @@ dependencies = [ [[package]] name = "windmill-types" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "bitflags 2.11.1", @@ -15611,7 +15611,7 @@ dependencies = [ [[package]] name = "windmill-worker" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-once-cell", @@ -15720,7 +15720,7 @@ dependencies = [ [[package]] name = "windmill-worker-volumes" -version = "1.700.2" +version = "1.702.1" dependencies = [ "bytes", "futures", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 0babfe3810..ccc6467e67 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "windmill" -version = "1.700.2" +version = "1.702.1" authors.workspace = true edition.workspace = true @@ -87,7 +87,7 @@ members = [ exclude = ["./windmill-duckdb-ffi-internal", "./parsers/windmill-parser-wasm"] [workspace.package] -version = "1.700.2" +version = "1.702.1" authors = ["Ruben Fiszel "] edition = "2021" diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 8a34b69a0e..919919c4c6 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -f9494c6320bb5fd07c1e9e09734b7fd5fbe7aa38 +19a76a09ffb43649ee19e62d07e8b8a42d78757b diff --git a/backend/migrations/20260513095235_token_read_only.down.sql b/backend/migrations/20260513095235_token_read_only.down.sql new file mode 100644 index 0000000000..e5380a3b02 --- /dev/null +++ b/backend/migrations/20260513095235_token_read_only.down.sql @@ -0,0 +1 @@ +ALTER TABLE token DROP COLUMN IF EXISTS read_only; diff --git a/backend/migrations/20260513095235_token_read_only.up.sql b/backend/migrations/20260513095235_token_read_only.up.sql new file mode 100644 index 0000000000..4fdeb9db3a --- /dev/null +++ b/backend/migrations/20260513095235_token_read_only.up.sql @@ -0,0 +1,4 @@ +-- Add a flag to restrict a token to read-only HTTP endpoints. +-- Orthogonal to `scopes`: even if scopes grant write/run, this flag denies +-- mutating methods (POST/PUT/PATCH/DELETE) and Run actions. +ALTER TABLE token ADD COLUMN read_only BOOLEAN NOT NULL DEFAULT false; diff --git a/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.down.sql b/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.down.sql new file mode 100644 index 0000000000..bb9b57ac0b --- /dev/null +++ b/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.down.sql @@ -0,0 +1,3 @@ +-- No-op: clearing a stray `auto_kind = 'lib'` value on failure/trigger/approval +-- scripts is not reversible (the original NULL/'lib' distinction is lost), and +-- restoring `'lib'` here would re-hide these scripts from their pickers. diff --git a/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.up.sql b/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.up.sql new file mode 100644 index 0000000000..e2038bb78c --- /dev/null +++ b/backend/migrations/20260514130526_fix_auto_kind_lib_for_runnable_script_kinds.up.sql @@ -0,0 +1,9 @@ +-- Failure, Trigger, and Approval scripts are runnable entrypoints by +-- definition. A prior parser regression occasionally classified them as +-- `auto_kind = 'lib'`, which hid them from the flow error-handler / +-- trigger / approval pickers. Clear those stray values so existing +-- affected scripts re-appear without requiring a redeploy. +UPDATE script +SET auto_kind = NULL +WHERE auto_kind = 'lib' + AND kind IN ('failure', 'trigger', 'approval'); diff --git a/backend/parsers/windmill-parser-wasm/Cargo.lock b/backend/parsers/windmill-parser-wasm/Cargo.lock index 954303108f..b595cfeca1 100644 --- a/backend/parsers/windmill-parser-wasm/Cargo.lock +++ b/backend/parsers/windmill-parser-wasm/Cargo.lock @@ -6183,7 +6183,7 @@ checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" [[package]] name = "windmill-common" -version = "1.700.2" +version = "1.702.1" dependencies = [ "aho-corasick", "anyhow", @@ -6263,7 +6263,7 @@ dependencies = [ [[package]] name = "windmill-macros" -version = "1.700.2" +version = "1.702.1" dependencies = [ "proc-macro2", "quote", @@ -6275,7 +6275,7 @@ dependencies = [ [[package]] name = "windmill-parser" -version = "1.700.2" +version = "1.702.1" dependencies = [ "convert_case", "serde", @@ -6284,7 +6284,7 @@ dependencies = [ [[package]] name = "windmill-parser-bash" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "lazy_static", @@ -6296,7 +6296,7 @@ dependencies = [ [[package]] name = "windmill-parser-csharp" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde_json", @@ -6308,7 +6308,7 @@ dependencies = [ [[package]] name = "windmill-parser-go" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "gosyn", @@ -6320,7 +6320,7 @@ dependencies = [ [[package]] name = "windmill-parser-graphql" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "lazy_static", @@ -6332,7 +6332,7 @@ dependencies = [ [[package]] name = "windmill-parser-java" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde_json", @@ -6344,7 +6344,7 @@ dependencies = [ [[package]] name = "windmill-parser-nu" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "nu-parser", @@ -6355,7 +6355,7 @@ dependencies = [ [[package]] name = "windmill-parser-php" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "itertools 0.14.0", @@ -6366,7 +6366,7 @@ dependencies = [ [[package]] name = "windmill-parser-py" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "itertools 0.14.0", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-asset" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "rustpython-ast", @@ -6389,7 +6389,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-imports" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "async-recursion", @@ -6411,7 +6411,7 @@ dependencies = [ [[package]] name = "windmill-parser-r" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde_json", @@ -6423,7 +6423,7 @@ dependencies = [ [[package]] name = "windmill-parser-ruby" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "lazy_static", @@ -6437,7 +6437,7 @@ dependencies = [ [[package]] name = "windmill-parser-rust" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "convert_case", @@ -6454,7 +6454,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "lazy_static", @@ -6467,7 +6467,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql-asset" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "lazy_static", @@ -6497,7 +6497,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts-asset" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde-wasm-bindgen", @@ -6513,7 +6513,7 @@ dependencies = [ [[package]] name = "windmill-parser-wac" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "rustpython-ast", @@ -6529,7 +6529,7 @@ dependencies = [ [[package]] name = "windmill-parser-wasm" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "getrandom 0.2.17", @@ -6561,7 +6561,7 @@ dependencies = [ [[package]] name = "windmill-parser-yaml" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "serde", @@ -6572,7 +6572,7 @@ dependencies = [ [[package]] name = "windmill-types" -version = "1.700.2" +version = "1.702.1" dependencies = [ "anyhow", "bitflags", diff --git a/backend/parsers/windmill-parser-wasm/Cargo.toml b/backend/parsers/windmill-parser-wasm/Cargo.toml index 0131c16089..3be5965444 100644 --- a/backend/parsers/windmill-parser-wasm/Cargo.toml +++ b/backend/parsers/windmill-parser-wasm/Cargo.toml @@ -12,7 +12,7 @@ resolver = "2" members = ["."] [workspace.package] -version = "1.700.2" +version = "1.702.1" edition = "2021" authors = ["Ruben Fiszel "] diff --git a/backend/src/db_connect.rs b/backend/src/db_connect.rs index 3860c18909..deda032058 100644 --- a/backend/src/db_connect.rs +++ b/backend/src/db_connect.rs @@ -6,6 +6,8 @@ use windmill_common::{ pub const DEFAULT_MAX_CONNECTIONS_SERVER: u32 = 50; pub const DEFAULT_MAX_CONNECTIONS_WORKER: u32 = 5; pub const DEFAULT_MAX_CONNECTIONS_INDEXER: u32 = 5; +#[cfg(feature = "operator")] +pub const DEFAULT_MAX_CONNECTIONS_OPERATOR: u32 = 2; pub async fn initial_connection() -> Result, error::Error> { let connect_options = get_database_url().await?.connect_options().await?; @@ -16,12 +18,35 @@ pub async fn initial_connection() -> Result, error::E .map_err(|err| Error::ConnectingToDatabase(err.to_string())) } +/// Connect to the database for the Kubernetes operator process. +/// +/// Long-running operator pods need IAM RDS / Entra ID token refresh just like the server, +/// otherwise new pool connections start failing once the initial token expires (~15 min). +#[cfg(feature = "operator")] +pub async fn operator_connection( + #[cfg(all(feature = "enterprise", feature = "private"))] + killpill_rx: tokio::sync::broadcast::Receiver<()>, +) -> anyhow::Result> { + let database_url = get_database_url().await?; + let pool = connect( + database_url.clone(), + DEFAULT_MAX_CONNECTIONS_OPERATOR, + false, + ) + .await?; + + #[cfg(all(feature = "enterprise", feature = "private"))] + spawn_token_refresh_task(pool.clone(), database_url, killpill_rx); + + Ok(pool) +} + pub async fn connect_db( server_mode: bool, indexer_mode: bool, worker_mode: bool, num_workers: i32, - #[cfg(feature = "private")] mut killpill_rx: tokio::sync::broadcast::Receiver<()>, + #[cfg(feature = "private")] killpill_rx: tokio::sync::broadcast::Receiver<()>, ) -> anyhow::Result> { use anyhow::Context; @@ -43,70 +68,72 @@ pub async fn connect_db( let pool = connect(database_url.clone(), max_connections, worker_mode).await?; #[cfg(all(feature = "enterprise", feature = "private"))] - { - let needs_token_refresh = matches!( - database_url, - DatabaseUrl::IamRds(_) | DatabaseUrl::EntraId(_) - ); - let label = match &database_url { - DatabaseUrl::IamRds(_) => "IAM RDS", - DatabaseUrl::EntraId(_) => "Entra ID", - DatabaseUrl::Static(_) => "", - }; - if needs_token_refresh { - let pool2 = pool.clone(); - let database_url2 = database_url.clone(); - tokio::spawn(async move { - loop { - tokio::select! { - _ = killpill_rx.recv() => { - break; - } - _ = tokio::time::sleep(std::time::Duration::from_secs(10)) => { - if !database_url2.needs_refresh().await { - continue; - } - let new_url = tokio::time::timeout( - std::time::Duration::from_secs(10), - get_database_url(), - ) - .await; - match new_url { - Ok(Ok(new_url)) => { - match new_url.connect_options().await { - Ok(connect_options) => { - pool2.set_connect_options(connect_options); - tracing::info!("Refreshed {label} URL successfully"); - } - Err(e) => { - tracing::error!( - "Error getting {label} connect options, retrying in 10s: {e}" - ); - continue; - } - } - } - Ok(Err(e)) => { - tracing::error!( - "Error refreshing {label} URL, trying again in 10s: {e}" - ); - continue; + spawn_token_refresh_task(pool.clone(), database_url, killpill_rx); + + Ok(pool) +} + +/// Spawn a background task that refreshes IAM RDS / Entra ID tokens before they expire +/// and updates the pool's connect options so new connections use the fresh token. +/// No-op for static (password-based) database URLs. +#[cfg(all(feature = "enterprise", feature = "private"))] +pub fn spawn_token_refresh_task( + pool: sqlx::Pool, + database_url: DatabaseUrl, + mut killpill_rx: tokio::sync::broadcast::Receiver<()>, +) { + let label = match &database_url { + DatabaseUrl::IamRds(_) => "IAM RDS", + DatabaseUrl::EntraId(_) => "Entra ID", + DatabaseUrl::Static(_) => return, + }; + tokio::spawn(async move { + loop { + tokio::select! { + _ = killpill_rx.recv() => { + break; + } + _ = tokio::time::sleep(std::time::Duration::from_secs(10)) => { + if !database_url.needs_refresh().await { + continue; + } + let new_url = tokio::time::timeout( + std::time::Duration::from_secs(10), + get_database_url(), + ) + .await; + match new_url { + Ok(Ok(new_url)) => { + match new_url.connect_options().await { + Ok(connect_options) => { + pool.set_connect_options(connect_options); + tracing::info!("Refreshed {label} URL successfully"); } Err(e) => { tracing::error!( - "Timeout after 10s refreshing {label} URL, trying again in 10s: {e}" + "Error getting {label} connect options, retrying in 10s: {e}" ); continue; } } } + Ok(Err(e)) => { + tracing::error!( + "Error refreshing {label} URL, trying again in 10s: {e}" + ); + continue; + } + Err(e) => { + tracing::error!( + "Timeout after 10s refreshing {label} URL, trying again in 10s: {e}" + ); + continue; + } } } - }); + } } - } - - Ok(pool) + }); } pub async fn connect( diff --git a/backend/src/main.rs b/backend/src/main.rs index 9a2709e9a9..74e2c240ba 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -670,7 +670,24 @@ async fn windmill_main() -> anyhow::Result<()> { tracing_subscriber::fmt::init(); tracing::info!("Starting Windmill Kubernetes operator..."); tracing::info!("Connecting to database..."); - let db = crate::db_connect::initial_connection().await?; + + #[cfg(all(feature = "enterprise", feature = "private"))] + let (operator_killpill_tx, operator_killpill_rx) = + tokio::sync::broadcast::channel::<()>(2); + + let db = crate::db_connect::operator_connection( + #[cfg(all(feature = "enterprise", feature = "private"))] + operator_killpill_rx, + ) + .await?; + + #[cfg(all(feature = "enterprise", feature = "private"))] + tokio::spawn(async move { + if let Ok(()) = tokio::signal::ctrl_c().await { + let _ = operator_killpill_tx.send(()); + } + }); + tracing::info!("Database connected. Starting ConfigMap watcher..."); windmill_operator::run(db).await?; return Ok(()); diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 0ddc02b256..d5ea145cfc 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -886,11 +886,13 @@ pub async fn reload_otel_tracing_proxy_setting(conn: &Connection) { let mut current = OTEL_TRACING_PROXY_SETTINGS.write().await; if current.enabled != new_settings.enabled || current.enabled_languages != new_settings.enabled_languages + || current.no_proxy_hosts != new_settings.no_proxy_hosts { tracing::info!( - "OTEL tracing proxy settings changed: enabled={}, languages={:?}", + "OTEL tracing proxy settings changed: enabled={}, languages={:?}, no_proxy_hosts={:?}", new_settings.enabled, - new_settings.enabled_languages + new_settings.enabled_languages, + new_settings.no_proxy_hosts, ); *current = new_settings; } diff --git a/backend/tests/bun_jobs.rs b/backend/tests/bun_jobs.rs index fa15866bca..d51e51f58b 100644 --- a/backend/tests/bun_jobs.rs +++ b/backend/tests/bun_jobs.rs @@ -577,6 +577,63 @@ export function main() { Ok(()) } +/// Regression test: a `//nobundling` script that pulls a package whose CJS +/// internals do bare-specifier `require()` of a sibling dependency. +/// +/// Before the `--preserve-symlinks` fix, Bun 1.2/1.3+ would follow the +/// directory symlink in `node_modules/@langchain/core` to its global cache +/// entry, walk parent dirs from the cache realpath, and fail to find +/// `node_modules/zod` — producing: +/// ENOENT while resolving package 'zod/v3' from +/// '.../cache_nomount/bun/@langchain/core@@@@1/dist/runnables/base.js' +/// +/// The fix passes `--preserve-symlinks` so Bun resolves from the +/// symlink path under `/node_modules/`, where `zod` is a sibling. +#[sqlx::test(fixtures("base"))] +async fn test_bun_nobundling_transitive_require(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + let content = r#"//nobundling +import { ChatPromptTemplate } from "@langchain/core/prompts"; + +export async function main() { + const tpl = ChatPromptTemplate.fromMessages([ + ["system", "you are a {role}"], + ["human", "{input}"], + ]); + const out = await tpl.formatMessages({ role: "tester", input: "ping" }); + return out.length; +} +"# + .to_owned(); + + let job = JobPayload::Code(RawCode { + hash: None, + content, + path: None, + language: ScriptLang::Bun, + lock: None, + concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default() + .into(), + debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(), + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + modules: None, + tag: None, + }); + + let result = run_job_in_new_worker_until_complete(&db, false, job, port) + .await + .json_result() + .unwrap(); + + assert_eq!(result, serde_json::json!(2)); + Ok(()) +} + // ============================================================================ // Native Mode Tests (requires deno_core feature) // ============================================================================ diff --git a/backend/tests/python_jobs.rs b/backend/tests/python_jobs.rs index 3f6d682c5a..3a1550d395 100644 --- a/backend/tests/python_jobs.rs +++ b/backend/tests/python_jobs.rs @@ -760,6 +760,112 @@ def main(): Ok(()) } +#[cfg(feature = "python")] +#[sqlx::test(fixtures("base"))] +async fn test_python_result_preserves_infinity_in_string(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + let content = r#" +def main(): + return { + "plain": "Infinity", + "embedded": "value=-Infinity end", + "nan_word": "this is NaN inside text", + "nested": [{"k": "Infinity"}], + } + "# + .to_owned(); + + let job = JobPayload::Code(RawCode { + hash: None, + content, + path: None, + language: ScriptLang::Python3, + lock: None, + concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default() + .into(), + debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(), + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + modules: None, + tag: None, + }); + + let result = run_job_in_new_worker_until_complete(&db, false, job, port) + .await + .json_result() + .unwrap(); + + assert_eq!( + result, + serde_json::json!({ + "plain": "Infinity", + "embedded": "value=-Infinity end", + "nan_word": "this is NaN inside text", + "nested": [{"k": "Infinity"}], + }) + ); + Ok(()) +} + +#[cfg(feature = "python")] +#[sqlx::test(fixtures("base"))] +async fn test_python_result_non_finite_floats_become_null( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + let content = r#" +def main(): + return { + "inf": float("inf"), + "neg_inf": float("-inf"), + "nan": float("nan"), + "finite": 1.5, + "nested": [float("inf"), {"x": float("nan")}], + } + "# + .to_owned(); + + let job = JobPayload::Code(RawCode { + hash: None, + content, + path: None, + language: ScriptLang::Python3, + lock: None, + concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default() + .into(), + debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(), + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + modules: None, + tag: None, + }); + + let result = run_job_in_new_worker_until_complete(&db, false, job, port) + .await + .json_result() + .unwrap(); + + assert_eq!( + result, + serde_json::json!({ + "inf": null, + "neg_inf": null, + "nan": null, + "finite": 1.5, + "nested": [null, {"x": null}], + }) + ); + Ok(()) +} + #[cfg(feature = "python")] #[sqlx::test(fixtures("base"))] async fn test_python_global_site_packages(db: Pool) -> anyhow::Result<()> { diff --git a/backend/tests/script_auto_kind_failure.rs b/backend/tests/script_auto_kind_failure.rs new file mode 100644 index 0000000000..4110bd4ba5 --- /dev/null +++ b/backend/tests/script_auto_kind_failure.rs @@ -0,0 +1,122 @@ +use std::collections::HashMap; + +use sqlx::{Pool, Postgres}; +use windmill_api_client::types::{NewScript, ScriptLang}; +use windmill_test_utils::init_client; + +fn quick_ns(content: &str, path: &str, kind: Option<&str>) -> NewScript { + NewScript { + content: content.into(), + language: ScriptLang::Bun, + lock: None, + parent_hash: None, + path: path.into(), + concurrent_limit: None, + concurrency_time_window_s: None, + cache_ttl: None, + dedicated_worker: None, + description: "".to_string(), + draft_only: None, + envs: vec![], + is_template: None, + kind: kind.map(|s| s.to_string()), + summary: "".to_string(), + tag: None, + schema: HashMap::new(), + ws_error_handler_muted: Some(false), + priority: None, + delete_after_secs: None, + timeout: None, + restart_unless_cancelled: None, + deployment_message: None, + concurrency_key: None, + visible_to_runner_only: None, + auto_kind: None, + codebase: None, + has_preprocessor: None, + on_behalf_of_email: None, + assets: vec![], + modules: None, + } +} + +/// Regression: a `failure`-kind script must never be marked `auto_kind = 'lib'` +/// even if the parser fails to detect a `main` function, because the flow +/// error-handler picker filters out lib scripts and would otherwise hide it. +#[sqlx::test(fixtures("base"))] +async fn failure_kind_script_without_main_is_not_marked_lib( + db: Pool, +) -> anyhow::Result<()> { + let (client, _port, _s) = init_client(db.clone()).await; + + // Content with no `main` — TS parser would normally set auto_kind = 'lib'. + client + .create_script( + "test-workspace", + &quick_ns( + "export function notMain() { return 42 }", + "u/test-user/failure_no_main", + Some("failure"), + ), + ) + .await + .unwrap(); + + let auto_kind: Option = sqlx::query_scalar( + "SELECT auto_kind FROM script \ + WHERE workspace_id = $1 AND path = $2", + ) + .bind("test-workspace") + .bind("u/test-user/failure_no_main") + .fetch_one(&db) + .await?; + + assert_ne!( + auto_kind.as_deref(), + Some("lib"), + "failure-kind script must not be marked as 'lib' auto_kind, got {:?}", + auto_kind + ); + + Ok(()) +} + +/// Sibling: a normal `script` kind WITHOUT main should still be marked `lib` +/// (so it stays hidden from the regular script picker). Guards against an +/// over-broad sanitizer accidentally clearing the value for plain scripts. +#[sqlx::test(fixtures("base"))] +async fn regular_script_without_main_is_still_marked_lib( + db: Pool, +) -> anyhow::Result<()> { + let (client, _port, _s) = init_client(db.clone()).await; + + client + .create_script( + "test-workspace", + &quick_ns( + "export function notMain() { return 42 }", + "u/test-user/script_no_main", + Some("script"), + ), + ) + .await + .unwrap(); + + let auto_kind: Option = sqlx::query_scalar( + "SELECT auto_kind FROM script \ + WHERE workspace_id = $1 AND path = $2", + ) + .bind("test-workspace") + .bind("u/test-user/script_no_main") + .fetch_one(&db) + .await?; + + assert_eq!( + auto_kind.as_deref(), + Some("lib"), + "regular script without main should be marked 'lib', got {:?}", + auto_kind + ); + + Ok(()) +} diff --git a/backend/tests/trigger_listener_queries.rs b/backend/tests/trigger_listener_queries.rs index c169d5289a..d1c20da407 100644 --- a/backend/tests/trigger_listener_queries.rs +++ b/backend/tests/trigger_listener_queries.rs @@ -174,6 +174,7 @@ fn make_authed() -> windmill_api_auth::ApiAuthed { scopes: None, username_override: None, token_prefix: None, + read_only: false, } } diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index ac4017b81c..bda9f54bdd 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -194,6 +194,7 @@ impl AuthCache { scopes: None, username_override, token_prefix: claims.audit_span, + read_only: false, }; let job_id = claims.job_id.and_then(|j| uuid::Uuid::from_str(&j).ok()); AUTH_CACHE.insert( @@ -221,11 +222,20 @@ impl AuthCache { token_hash = $1 AND (expiration > NOW() OR expiration IS NULL) AND (workspace_id IS NULL OR workspace_id = $2) - RETURNING owner, email, super_admin, scopes, label", + RETURNING owner, email, super_admin, scopes, label, read_only", t_hash, w_id.as_ref(), ) - .map(|x| (x.owner, x.email, x.super_admin, x.scopes, x.label)) + .map(|x| { + ( + x.owner, + x.email, + x.super_admin, + x.scopes, + x.label, + x.read_only, + ) + }) .fetch_optional(&self.db) .await .ok() @@ -234,7 +244,9 @@ impl AuthCache { if let Some(user) = user_o { let authed_o = { match user { - (Some(owner), Some(email), super_admin, _, label) if w_id.is_some() => { + (Some(owner), Some(email), super_admin, _, label, read_only) + if w_id.is_some() => + { let username_override = username_override_from_label(label); if let Some((prefix, name)) = owner.split_once('/') { if prefix == "u" { @@ -280,6 +292,7 @@ impl AuthCache { scopes: None, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }) } else { let groups = vec![name.to_string()]; @@ -305,6 +318,7 @@ impl AuthCache { scopes: None, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }) } } else { @@ -320,10 +334,11 @@ impl AuthCache { scopes: None, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }) } } - (_, Some(email), super_admin, scopes, label) => { + (_, Some(email), super_admin, scopes, label, read_only) => { let username_override = username_override_from_label(label); if w_id.is_some() { let row_o = sqlx::query!( @@ -368,6 +383,7 @@ impl AuthCache { scopes, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }) } None if super_admin => Some(ApiAuthed { @@ -380,6 +396,7 @@ impl AuthCache { scopes, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }), None => None, } @@ -394,6 +411,7 @@ impl AuthCache { scopes, username_override, token_prefix: Some(safe_token_prefix(token)), + read_only, }) } } @@ -428,6 +446,7 @@ impl AuthCache { scopes: None, username_override: None, token_prefix: Some(safe_token_prefix(token)), + read_only: false, }; Some(OptJobAuthed { authed, job_id: None }) } else { @@ -630,6 +649,7 @@ pub async fn resolve_opt_job_authed( scopes: None, username_override: None, token_prefix: None, + read_only: false, }; return Ok((OptJobAuthed { authed, job_id: None }, parts)); } @@ -667,12 +687,11 @@ pub async fn resolve_opt_job_authed( cache.get_opt_job_authed(workspace_id.clone(), &token).await { let authed = &mut opt_job_authed.authed; + let path = original_uri.path(); + let method = parts.method.as_str(); if authed.scopes.is_some() { transform_old_scope_to_new_scope(authed.scopes.as_mut()); - let path = original_uri.path(); - let method = parts.method.as_str(); - if let Err(err) = crate::scopes::check_scopes_for_route( authed.scopes.as_deref(), path, @@ -681,6 +700,27 @@ pub async fn resolve_opt_job_authed( return Err((err, parts)); } } + if authed.read_only { + // MCP transport runs over POST (streamable HTTP / SSE handshake), + // so the middleware can't safely reject mutating methods on it — + // the MCP runner itself filters out write tools and rejects + // mutating tool calls for read-only tokens. Narrow to the actual + // transport endpoints: anything else under `/api/mcp/*` (OAuth + // approve, token exchange, client registration) must still go + // through the read-only check, otherwise a read-only token + // could approve an OAuth flow that mints a new non-read-only + // token. + let is_mcp_transport = path == "/api/mcp/gateway" + || (path.starts_with("/api/mcp/w/") + && (path.ends_with("/mcp") + || path.ends_with("/sse") + || path.ends_with("/list_tools"))); + if !is_mcp_transport { + if let Err(err) = crate::scopes::check_read_only_for_route(path, method) { + return Err((err, parts)); + } + } + } parts.extensions.insert(authed.clone()); Span::current().record("username", &authed.username.as_str()); diff --git a/backend/windmill-api-auth/src/lib.rs b/backend/windmill-api-auth/src/lib.rs index c5b9b5adc7..b9bc2e2417 100644 --- a/backend/windmill-api-auth/src/lib.rs +++ b/backend/windmill-api-auth/src/lib.rs @@ -55,6 +55,7 @@ pub struct ApiAuthed { pub scopes: Option>, pub username_override: Option, pub token_prefix: Option, + pub read_only: bool, } impl ApiAuthed { @@ -103,6 +104,7 @@ impl From for ApiAuthed { scopes: value.scopes, username_override: None, token_prefix: value.token_prefix, + read_only: false, } } } @@ -183,6 +185,10 @@ impl windmill_mcp::server::McpAuth for ApiAuthed { fn scopes(&self) -> Option<&[String]> { self.scopes.as_deref() } + + fn read_only(&self) -> bool { + self.read_only + } } // ------------ Utility functions ------------ @@ -478,6 +484,7 @@ pub async fn fetch_api_authed_from_permissioned_as( scopes: authed.scopes, username_override: None, token_prefix: authed.token_prefix, + read_only: false, }; API_AUTHED_CACHE.insert( @@ -506,6 +513,8 @@ pub struct NewToken { pub impersonate_email: Option, pub scopes: Option>, pub workspace_id: Option, + #[serde(default)] + pub read_only: Option, } impl NewToken { @@ -515,8 +524,9 @@ impl NewToken { impersonate_email: Option, scopes: Option>, workspace_id: Option, + read_only: Option, ) -> Self { - Self { label, expiration, impersonate_email, scopes, workspace_id } + Self { label, expiration, impersonate_email, scopes, workspace_id, read_only } } } @@ -564,8 +574,8 @@ pub async fn create_token_internal( } let rows = sqlx::query!( "INSERT INTO token - (token_hash, token_prefix, token, email, label, expiration, super_admin, scopes, workspace_id) - SELECT $1, $2, $3, $4, $5, $6, $7, $8, $9 + (token_hash, token_prefix, token, email, label, expiration, super_admin, scopes, workspace_id, read_only) + SELECT $1, $2, $3, $4, $5, $6, $7, $8, $9, $10 WHERE $9::varchar IS NULL OR NOT EXISTS( SELECT 1 FROM workspace WHERE id = $9 AND deleted = true )", @@ -578,6 +588,7 @@ pub async fn create_token_internal( is_super_admin, token_config.scopes.as_ref().map(|x| x.as_slice()), token_config.workspace_id, + token_config.read_only.unwrap_or(false), ) .execute(&mut *tx) .await?; diff --git a/backend/windmill-api-auth/src/scopes.rs b/backend/windmill-api-auth/src/scopes.rs index 6df2f74ae8..87ca3a8862 100644 --- a/backend/windmill-api-auth/src/scopes.rs +++ b/backend/windmill-api-auth/src/scopes.rs @@ -686,6 +686,19 @@ fn scope_grants_access( Ok(true) } +/// Enforces a token's `read_only` flag: only methods classified as `Read` +/// (GET/HEAD/OPTIONS) are allowed. Run actions and mutating methods are +/// rejected. Independent of `scopes`. +pub fn check_read_only_for_route(route_path: &str, http_method: &str) -> Result<()> { + if map_http_method_to_action(http_method, route_path) == ScopeAction::Read { + Ok(()) + } else { + Err(Error::PermissionDenied( + "Token is read-only. Mutating endpoints are not allowed.".to_string(), + )) + } +} + /// Helper function to check if scopes allow access to a route pub fn check_scopes_for_route( token_scopes: Option<&[String]>, @@ -778,6 +791,34 @@ mod tests { assert_eq!(route_suffix, Some("flow_conversations/list".to_string())); } + #[test] + fn test_check_read_only_for_route() { + // Plain GETs pass. + assert!(check_read_only_for_route("/api/w/x/scripts/list", "GET").is_ok()); + assert!(check_read_only_for_route("/api/w/x/scripts/get/foo", "HEAD").is_ok()); + assert!(check_read_only_for_route("/api/w/x/anything", "OPTIONS").is_ok()); + + // Mutating methods are rejected. + assert!(check_read_only_for_route("/api/w/x/scripts/create", "POST").is_err()); + assert!(check_read_only_for_route("/api/w/x/scripts/update", "PUT").is_err()); + assert!(check_read_only_for_route("/api/w/x/scripts/delete", "DELETE").is_err()); + assert!(check_read_only_for_route("/api/w/x/scripts/patch", "PATCH").is_err()); + + // Run paths are rejected even on GET (map_http_method_to_action elevates + // them to Run via RUN_PATH_ACTIONS). + assert!(check_read_only_for_route("/api/w/x/jobs/run/p/f/foo", "GET").is_err()); + assert!(check_read_only_for_route("/api/w/x/jobs/run/p/f/foo", "POST").is_err()); + + // OAuth/registration endpoints under /api/mcp/* must NOT be exempted by + // the auth middleware — they go through this check on the gateway side + // because they can mint non-read-only tokens. The middleware decides + // which paths to exempt; this helper is method-only, so we just assert + // that mutating methods still fail. + assert!( + check_read_only_for_route("/api/mcp/gateway/oauth/server/approve", "POST").is_err() + ); + } + #[test] fn test_specific_scope_access() { let scopes = vec!["jobs:read".to_string()]; diff --git a/backend/windmill-api-groups/src/granular_acls.rs b/backend/windmill-api-groups/src/granular_acls.rs index 95243b6626..3f0549f4e8 100644 --- a/backend/windmill-api-groups/src/granular_acls.rs +++ b/backend/windmill-api-groups/src/granular_acls.rs @@ -12,6 +12,9 @@ use axum::{ Json, Router, }; use windmill_api_auth::require_owner_of_path; +use windmill_audit::audit_oss::audit_log; +use windmill_audit::ActionKind; +use windmill_common::scripts::ScriptHash; use windmill_common::DB; use windmill_git_sync::{handle_deployment_metadata, DeployedObject}; @@ -24,6 +27,31 @@ use windmill_common::{ utils::{not_found_if_none, StripPath}, }; +/// Map a granular-ACL kind segment to the audit-log action prefix used by the +/// per-kind CRUD endpoints (e.g. `flows.update`, `scripts.update`). The +/// resulting action is suffixed with `.grant_acl` / `.revoke_acl` so the +/// audit log keeps a per-resource record of every `/acls/*` mutation — +/// folder/group already log via their dedicated permission-history tables. +fn audit_action_prefix_for_acl_kind(kind: &str) -> Option<&'static str> { + match kind { + "script" => Some("scripts"), + "flow" => Some("flows"), + "app" => Some("apps"), + // Distinct prefix so dashboards aggregating on `action` can separate + // raw_app ACL mutations from regular app ones without parsing the + // `kind` parameters field. (The granular_acls SQL routes raw_app + // writes to the same `app` table; audit log identity is separate.) + "raw_app" => Some("raw_apps"), + "resource" => Some("resources"), + "variable" => Some("variables"), + "schedule" => Some("schedules"), + "http_trigger" | "websocket_trigger" | "kafka_trigger" | "nats_trigger" + | "postgres_trigger" | "mqtt_trigger" | "gcp_trigger" | "azure_trigger" | "sqs_trigger" + | "email_trigger" => Some("triggers"), + _ => None, + } +} + const KINDS: [&str; 20] = [ "script", "group_", @@ -131,9 +159,15 @@ async fn add_granular_acl( } } + // v2 raw apps are stored in the `app` table (with `app_version.raw_app = true` + // distinguishing them from regular apps); the legacy `raw_app` table no longer + // backs the workspace export, so granting/revoking on `raw_app` must hit `app` + // for the change to be visible. Git-sync dispatch still uses + // DeployedObject::RawApp so the worker writes back `.raw_app.json`. + let table = if kind == "raw_app" { "app" } else { kind }; // SAFETY: `kind` has been validated against the `KINDS` allowlist before reaching this function. let obj_o = sqlx::query_scalar::<_, serde_json::Value>(&format!( - "UPDATE {kind} SET extra_perms = jsonb_set(extra_perms, $1, to_jsonb($2), \ + "UPDATE {table} SET extra_perms = jsonb_set(extra_perms, $1, to_jsonb($2), \ true) WHERE {identifier} = $3 AND workspace_id = $4 RETURNING extra_perms" )) .bind(vec![owner.clone()]) @@ -175,6 +209,34 @@ async fn add_granular_acl( Some(&owner), ) .await?; + } else if let Some(prefix) = audit_action_prefix_for_acl_kind(kind) { + // Mirror the folder/group permission-history coverage for every other + // ACLable kind. Folder/group already wrote a dedicated history row + // above; everything else (script/flow/app/raw_app/resource/...) lands + // in the general audit_log table here. + let access = if write.unwrap_or(false) { + "write" + } else { + "read" + }; + let action = format!("{}.grant_acl", prefix); + audit_log( + &mut *tx, + &authed, + action.as_str(), + ActionKind::Update, + &w_id, + Some(path), + Some( + [ + ("kind", kind), + ("owner", owner.as_str()), + ("access", access), + ] + .into(), + ), + ) + .await?; } tx.commit().await?; @@ -193,46 +255,67 @@ async fn add_granular_acl( ) .await? } - // "app" => { - // handle_deployment_metadata( - // &authed.email, - // &authed.username, - // &db, - // &w_id, - // DeployedObject::App { path: path.to_string(), parent_path: None, version: 0 }, - // Some(format!("App '{}' changed permissions", path)), - // // true, - // ) - // .await? - // } - // "script" => { - // handle_deployment_metadata( - // &authed.email, - // &authed.username, - // &db, - // &w_id, - // DeployedObject::Script { - // path: path.to_string(), - // parent_path: None, - // hash: ScriptHash(0), - // }, - // Some(format!("Script '{}' changed permissions", path)), - // // true, - // ) - // .await? - // } - // "flow" => { - // handle_deployment_metadata( - // &authed.email, - // &authed.username, - // &db, - // &w_id, - // DeployedObject::Flow { path: path.to_string(), parent_path: None }, - // Some(format!("Flow '{}' changed permissions", path)), - // // true, - // ) - // .await? - // } + "app" => { + handle_deployment_metadata( + &authed.email, + &authed.username, + &db, + &w_id, + DeployedObject::App { path: path.to_string(), parent_path: None, version: 0 }, + Some(format!("App '{}' changed permissions", path)), + true, + None, + ) + .await? + } + "raw_app" => { + // RawApp deliberately uses its own DeployedObject variant: the + // git-sync worker reads `path_type` ("app" vs "raw_app") to decide + // whether to write `.app.json` or `.raw_app.json`. + // Collapsing this into `App` would dispatch raw_app perm changes + // against the wrong file in the repo. + handle_deployment_metadata( + &authed.email, + &authed.username, + &db, + &w_id, + DeployedObject::RawApp { path: path.to_string(), parent_path: None, version: 0 }, + Some(format!("Raw App '{}' changed permissions", path)), + true, + None, + ) + .await? + } + "script" => { + handle_deployment_metadata( + &authed.email, + &authed.username, + &db, + &w_id, + DeployedObject::Script { + path: path.to_string(), + parent_path: None, + hash: ScriptHash(0), + }, + Some(format!("Script '{}' changed permissions", path)), + true, + None, + ) + .await? + } + "flow" => { + handle_deployment_metadata( + &authed.email, + &authed.username, + &db, + &w_id, + DeployedObject::Flow { path: path.to_string(), parent_path: None, version: 0 }, + Some(format!("Flow '{}' changed permissions", path)), + true, + None, + ) + .await? + } _ => (), } @@ -295,13 +378,17 @@ async fn remove_granular_acl( require_owner_of_path(&authed, path)?; } + // See add_granular_acl: kind="raw_app" must hit the `app` table because v2 + // raw apps live there and the legacy `raw_app` table no longer backs the + // workspace export. + let table = if kind == "raw_app" { "app" } else { kind }; // SAFETY: `kind` has been validated against the `KINDS` allowlist before reaching this function. let obj_o = sqlx::query_scalar::<_, bool>(&format!( "WITH old AS ( - SELECT extra_perms->$1 as old_write FROM {kind} + SELECT extra_perms->$1 as old_write FROM {table} WHERE {identifier} = $2 AND workspace_id = $3 AND extra_perms ? $1 ) - UPDATE {kind} SET extra_perms = extra_perms - $1 + UPDATE {table} SET extra_perms = extra_perms - $1 WHERE {identifier} = $2 AND workspace_id = $3 AND extra_perms ? $1 RETURNING (SELECT old_write FROM old)::bool" )) @@ -335,6 +422,28 @@ async fn remove_granular_acl( Some(&owner), ) .await?; + } else if let Some(prefix) = audit_action_prefix_for_acl_kind(kind) { + // Mirror the add path: standard audit_log row for every kind that + // doesn't have a dedicated permission-history table. + let access = if write { "write" } else { "read" }; + let action = format!("{}.revoke_acl", prefix); + audit_log( + &mut *tx, + &authed, + action.as_str(), + ActionKind::Update, + &w_id, + Some(path), + Some( + [ + ("kind", kind), + ("owner", owner.as_str()), + ("access", access), + ] + .into(), + ), + ) + .await?; } tx.commit().await?; @@ -353,46 +462,68 @@ async fn remove_granular_acl( ) .await? } - // "app" => { - // handle_deployment_metadata( - // &authed.email, - // &authed.username, - // &db, - // &w_id, - // DeployedObject::App { path: path.to_string(), parent_path: None, version: 0 }, - // Some(format!("App '{}' changed permissions", path)), - // // true, - // ) - // .await? - // } - // "script" => { - // handle_deployment_metadata( - // &authed.email, - // &authed.username, - // &db, - // &w_id, - // DeployedObject::Script { - // path: path.to_string(), - // parent_path: None, - // hash: ScriptHash(0), - // }, - // Some(format!("Script '{}' changed permissions", path)), - // // true, - // ) - // .await? - // } - // "flow" => { - // handle_deployment_metadata( - // &authed.email, - // &authed.username, - // &db, - // &w_id, - // DeployedObject::Flow { path: path.to_string(), parent_path: None }, - // Some(format!("Flow '{}' changed permissions", path)), - // // true, - // ) - // .await? - // } + "app" => { + handle_deployment_metadata( + &authed.email, + &authed.username, + &db, + &w_id, + DeployedObject::App { path: path.to_string(), parent_path: None, version: 0 }, + Some(format!("App '{}' changed permissions", path)), + true, + None, + ) + .await? + } + "raw_app" => { + // See add_granular_acl: raw_app must use its own DeployedObject + // variant so git-sync writes `.raw_app.json`. + handle_deployment_metadata( + &authed.email, + &authed.username, + &db, + &w_id, + DeployedObject::RawApp { + path: path.to_string(), + parent_path: None, + version: 0, + }, + Some(format!("Raw App '{}' changed permissions", path)), + true, + None, + ) + .await? + } + "script" => { + handle_deployment_metadata( + &authed.email, + &authed.username, + &db, + &w_id, + DeployedObject::Script { + path: path.to_string(), + parent_path: None, + hash: ScriptHash(0), + }, + Some(format!("Script '{}' changed permissions", path)), + true, + None, + ) + .await? + } + "flow" => { + handle_deployment_metadata( + &authed.email, + &authed.username, + &db, + &w_id, + DeployedObject::Flow { path: path.to_string(), parent_path: None, version: 0 }, + Some(format!("Flow '{}' changed permissions", path)), + true, + None, + ) + .await? + } _ => (), } } @@ -421,9 +552,13 @@ async fn get_granular_acls( } else { "path" }; + // See add_granular_acl: raw_app rows live in the `app` table now, so the + // read path must also target `app` — otherwise GET would return stale or + // 404 state while POST /acls/add and /acls/remove write to `app`. + let table = if kind == "raw_app" { "app" } else { kind }; // SAFETY: `kind` has been validated against the `KINDS` allowlist before reaching this function. let obj_o = sqlx::query_scalar::<_, serde_json::Value>(&format!( - "SELECT extra_perms from {kind} WHERE {identifier} = $1 AND workspace_id = $2" + "SELECT extra_perms from {table} WHERE {identifier} = $1 AND workspace_id = $2" )) .bind(path) .bind(w_id) diff --git a/backend/windmill-api-integration-tests/tests/native_triggers.rs b/backend/windmill-api-integration-tests/tests/native_triggers.rs index 0d1530a7d9..159236ed4b 100644 --- a/backend/windmill-api-integration-tests/tests/native_triggers.rs +++ b/backend/windmill-api-integration-tests/tests/native_triggers.rs @@ -51,6 +51,7 @@ fn test_authed() -> ApiAuthed { scopes: None, username_override: None, token_prefix: None, + read_only: false, } } diff --git a/backend/windmill-api-integration-tests/tests/workspace_dependencies_git_sync.rs b/backend/windmill-api-integration-tests/tests/workspace_dependencies_git_sync.rs index 0049280aec..a5af874f8c 100644 --- a/backend/windmill-api-integration-tests/tests/workspace_dependencies_git_sync.rs +++ b/backend/windmill-api-integration-tests/tests/workspace_dependencies_git_sync.rs @@ -573,8 +573,10 @@ async fn test_promotion_individual_branch_debounces_per_path( // Wait for both deployment callbacks to resolve a debounce key. The // alpha key is polled first as a warm-up, then we also wait for beta // so the assertions don't race the second spawned callback. - let expected_alpha = "git_sync:script:f/target/alpha"; - let expected_beta = "git_sync:script:f/target/beta"; + // Keys are namespaced by the repo's resource path so multiple promotion + // repos don't collide on the same key. + let expected_alpha = "git_sync:$res:u/test-user/test_git_repo:script:f/target/alpha"; + let expected_beta = "git_sync:$res:u/test-user/test_git_repo:script:f/target/beta"; let _ = wait_for_debounce_key(&db, expected_alpha, Duration::from_secs(5)).await?; let keys = wait_for_debounce_key(&db, expected_beta, Duration::from_secs(5)).await?; assert!( @@ -589,6 +591,162 @@ async fn test_promotion_individual_branch_debounces_per_path( Ok(()) } +/// Create a second git repository resource for multi-repo tests. +#[allow(dead_code)] +async fn create_second_git_repo_resource(db: &Pool) -> anyhow::Result<()> { + sqlx::query( + r#" + INSERT INTO resource (workspace_id, path, value, resource_type, extra_perms, created_by) + VALUES ('test-workspace', 'u/test-user/test_git_repo_2', $1::jsonb, 'git_repository', '{}'::jsonb, 'test-user') + ON CONFLICT (workspace_id, path) DO NOTHING + "#, + ) + .bind(json!({ + "url": "https://github.com/test/test2.git", + "branch": "main", + "token": "test-token-2" + })) + .execute(db) + .await?; + Ok(()) +} + +/// Configure git sync with TWO promotion-mode repositories pointing at distinct +/// git repo resources. Both repos use the same sync script and the same item +/// filters — they only differ in the repo they target. +#[allow(dead_code)] +async fn setup_two_promotion_repos_config( + db: &Pool, + sync_script_path: &str, + group_by_folder: bool, +) -> anyhow::Result<()> { + let git_sync_config = json!({ + "include_type": ["script"], + "include_path": ["**"], + "repositories": [ + { + "script_path": sync_script_path, + "git_repo_resource_path": "$res:u/test-user/test_git_repo", + "use_individual_branch": true, + "group_by_folder": group_by_folder + }, + { + "script_path": sync_script_path, + "git_repo_resource_path": "$res:u/test-user/test_git_repo_2", + "use_individual_branch": true, + "group_by_folder": group_by_folder + } + ] + }); + + sqlx::query!( + "UPDATE workspace_settings SET git_sync = $1 WHERE workspace_id = $2", + git_sync_config, + "test-workspace" + ) + .execute(db) + .await?; + + Ok(()) +} + +/// Regression test for: when two promotion-mode repos are configured with the +/// same `use_individual_branch=true` settings (i.e. a primary and a secondary +/// promotion repo), deploying a single script must enqueue ONE deployment +/// callback per repo. Both callbacks must remain in the queue — neither may +/// be debounced into oblivion by the other. +/// +/// The bug this guards against: the debounce key for promotion mode was +/// derived only from (path_type, path) and omitted any per-repo identifier, +/// so the second repo's push hit ON CONFLICT in `upsert_debounce_key` and +/// `complete_debounced_job` flagged the first repo's job as `status='skipped'` +/// — silently dropping one of the two pushes. +#[cfg(all(feature = "enterprise", feature = "private"))] +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_two_promotion_repos_both_enqueue_callback(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + + create_folder(&db, "28103").await?; + create_folder(&db, "target").await?; + create_git_repo_resource(&db).await?; + create_second_git_repo_resource(&db).await?; + let sync_script_path = "f/28103/test_sync_two_promotion_repos"; + create_sync_script(&db, sync_script_path).await?; + setup_two_promotion_repos_config(&db, sync_script_path, false).await?; + + let (client, _port, _server) = init_client(db.clone()).await; + + // Deploy a single script — handle_deployment_metadata should iterate + // both repos and create one callback job per repo. + create_test_script(&client, "f/target/alpha").await?; + + // Both callbacks should reach the queue. With the bug, only one survives + // (the other is moved to v2_job_completed with status='skipped'). + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); + let mut last_jobs: Vec = vec![]; + loop { + last_jobs = + get_deployment_callback_jobs(&db, sync_script_path, Duration::from_millis(200)).await?; + if last_jobs.len() >= 2 { + break; + } + if tokio::time::Instant::now() >= deadline { + break; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + + // Inspect what landed in v2_job_completed so failure messages explain why. + let skipped: Vec<(uuid::Uuid, String)> = sqlx::query_as( + r#" + SELECT c.id, c.status::text + FROM v2_job_completed c + JOIN v2_job j ON j.id = c.id + WHERE j.runnable_path = $1 AND j.kind = 'deploymentcallback' + "#, + ) + .bind(sync_script_path) + .fetch_all(&db) + .await?; + + assert_eq!( + last_jobs.len(), + 2, + "expected 2 deployment callback jobs in v2_job_queue (one per promotion repo), got {} queued + {:?} completed", + last_jobs.len(), + skipped, + ); + + // Per-repo args sanity check: the two jobs must target different repos. + let mut repo_paths: Vec = last_jobs + .iter() + .filter_map(|j| { + j.args + .as_ref() + .and_then(|a| a.get("repo_url_resource_path")) + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + }) + .collect(); + repo_paths.sort(); + repo_paths.dedup(); + assert_eq!( + repo_paths.len(), + 2, + "expected callbacks to target two distinct repos, got: {:?}", + last_jobs.iter().map(|j| &j.args).collect::>() + ); + + // No callback should have been silently skipped via debouncing collision. + assert!( + skipped.iter().all(|(_, s)| s != "skipped"), + "no deployment callback should be marked skipped, got: {:?}", + skipped, + ); + + Ok(()) +} + /// Promotion mode with group_by_folder: items destined for the same per-folder /// branch must share one debounce key so they accumulate into a single sync /// job; scripts in different folders must get distinct keys. @@ -615,8 +773,9 @@ async fn test_promotion_group_by_folder_debounces_per_folder( // One in a different folder — should get its own key. create_test_script(&client, "f/other/gamma").await?; - let expected_grouped = "git_sync:folder:f/grouped"; - let expected_other = "git_sync:folder:f/other"; + // Keys are namespaced by the repo's resource path. + let expected_grouped = "git_sync:$res:u/test-user/test_git_repo:folder:f/grouped"; + let expected_other = "git_sync:$res:u/test-user/test_git_repo:folder:f/other"; // Wait for BOTH folder keys to appear, not just the first one. let keys = wait_for_debounce_key(&db, expected_other, Duration::from_secs(5)).await?; assert!( @@ -629,7 +788,9 @@ async fn test_promotion_group_by_folder_debounces_per_folder( ); // Paths within the same folder must NOT leak as their own keys. assert!( - !keys.iter().any(|k| k.starts_with("git_sync:script:")), + !keys + .iter() + .any(|k| k.contains(":script:f/grouped/") || k.contains(":script:f/other/")), "group_by_folder mode should not emit per-path keys, got: {keys:?}" ); diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index 3237126bba..bf94756e76 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -1218,6 +1218,22 @@ async fn create_script_internal<'c>( } }; + // Failure, Trigger, and Approval scripts are runnable entrypoints by + // definition. They must never be marked `auto_kind = 'lib'`, or they + // disappear from the flow error-handler / trigger / approval pickers + // (which filter out lib scripts). Strip a stray `lib` here so a parser + // misclassification — e.g. failing to detect `main` after a deno_ast + // bump — cannot orphan these scripts in the UI. + let auto_kind = if matches!( + ns.kind, + Some(ScriptKind::Failure) | Some(ScriptKind::Trigger) | Some(ScriptKind::Approval) + ) && auto_kind.as_deref() == Some("lib") + { + None + } else { + auto_kind + }; + let ci_test_refs = windmill_common::schema::parse_ci_test_annotation(&ns.content, &lang.as_comment_lit()); let auto_kind = if ci_test_refs.is_some() { @@ -2250,7 +2266,7 @@ async fn exists_script_by_path( let path = path.to_path(); let exists = sqlx::query_scalar!( - "SELECT EXISTS(SELECT 1 FROM script WHERE path = $1 AND workspace_id = $2 ORDER BY created_at DESC LIMIT 1)", + "SELECT EXISTS(SELECT 1 FROM script WHERE path = $1 AND workspace_id = $2 AND archived = false ORDER BY created_at DESC LIMIT 1)", path, w_id ) diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index e4540ca903..0eb9d26b5d 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -210,6 +210,8 @@ pub struct GlobalUserInfo { first_time_user: bool, role_source: String, disabled: bool, + #[serde(skip_serializing_if = "Option::is_none")] + workspace_id: Option, } #[derive(Serialize, Debug)] @@ -297,6 +299,7 @@ pub struct TruncatedToken { pub last_used_at: chrono::DateTime, pub scopes: Option>, pub workspace_id: Option, + pub read_only: bool, } // NewToken is re-exported from windmill-api-auth above @@ -450,13 +453,17 @@ async fn list_users_as_super_admin( let rows = if active_only.is_some_and(|x| x) { sqlx::query_as!( GlobalUserInfo, - "WITH active_users AS (SELECT distinct username as email FROM (SELECT username, timestamp, operation FROM audit_partitioned UNION ALL SELECT username, timestamp, operation FROM audit) AS a WHERE timestamp > NOW() - INTERVAL '1 month' AND (operation = 'users.login' OR operation = 'oauth.login' OR operation = 'users.token.refresh')), + r#"WITH active_users AS (SELECT distinct username as email FROM (SELECT username, timestamp, operation FROM audit_partitioned UNION ALL SELECT username, timestamp, operation FROM audit) AS a WHERE timestamp > NOW() - INTERVAL '1 month' AND (operation = 'users.login' OR operation = 'oauth.login' OR operation = 'users.token.refresh')), authors as (SELECT distinct email FROM usr WHERE usr.operator IS false) - SELECT email, email NOT IN (SELECT email FROM authors) as operator_only, login_type::text, verified, super_admin, devops, name, company, username, first_time_user, role_source, disabled + SELECT email as "email!", (email NOT IN (SELECT email FROM authors)) as operator_only, login_type::text, verified as "verified!", super_admin as "super_admin!", devops as "devops!", name, company, username, first_time_user as "first_time_user!", role_source as "role_source!", disabled as "disabled!", NULL::text as workspace_id FROM password WHERE email IN (SELECT email FROM active_users) - ORDER BY super_admin DESC, devops DESC - LIMIT $1 OFFSET $2", + UNION ALL + SELECT email as "email!", true as operator_only, 'service_account'::text as login_type, true as "verified!", false as "super_admin!", false as "devops!", NULL::text as name, NULL::text as company, username, false as "first_time_user!", 'service_account'::text as "role_source!", disabled as "disabled!", workspace_id + FROM usr + WHERE is_service_account IS true + ORDER BY "super_admin!" DESC, "devops!" DESC + LIMIT $1 OFFSET $2"#, per_page as i32, offset as i32 ) @@ -465,8 +472,13 @@ async fn list_users_as_super_admin( } else { sqlx::query_as!( GlobalUserInfo, - "SELECT email, login_type::text, verified, super_admin, devops, name, company, username, NULL::bool as operator_only, first_time_user, role_source, disabled FROM password ORDER BY super_admin DESC, devops DESC, email LIMIT \ - $1 OFFSET $2", + r#"SELECT email as "email!", login_type::text, verified as "verified!", super_admin as "super_admin!", devops as "devops!", name, company, username, NULL::bool as operator_only, first_time_user as "first_time_user!", role_source as "role_source!", disabled as "disabled!", NULL::text as workspace_id FROM password + UNION ALL + SELECT email as "email!", 'service_account'::text as login_type, true as "verified!", false as "super_admin!", false as "devops!", NULL::text as name, NULL::text as company, username, true as operator_only, false as "first_time_user!", 'service_account'::text as "role_source!", disabled as "disabled!", workspace_id + FROM usr + WHERE is_service_account IS true + ORDER BY "super_admin!" DESC, "devops!" DESC, "email!" + LIMIT $1 OFFSET $2"#, per_page as i32, offset as i32 ) @@ -715,7 +727,7 @@ async fn global_whoami( ) -> JsonResult { let user = sqlx::query_as!( GlobalUserInfo, - "SELECT email, login_type::TEXT, super_admin, devops, verified, name, company, username, NULL::bool as operator_only, first_time_user, role_source, disabled FROM password WHERE \ + "SELECT email, login_type::TEXT, super_admin, devops, verified, name, company, username, NULL::bool as operator_only, first_time_user, role_source, disabled, NULL::text as workspace_id FROM password WHERE \ email = $1", email ) @@ -739,6 +751,7 @@ async fn global_whoami( first_time_user: false, role_source: "manual".to_string(), disabled: false, + workspace_id: None, })) } else { // Service accounts don't have a password row @@ -755,6 +768,7 @@ async fn global_whoami( first_time_user: false, role_source: "service_account".to_string(), disabled: false, + workspace_id: None, })) } } @@ -2249,7 +2263,7 @@ async fn list_tokens( sqlx::query_as!( TruncatedToken, "SELECT label, token_prefix, expiration, created_at, \ - last_used_at, scopes, workspace_id FROM token WHERE email = $1 AND (label != 'ephemeral-script' OR label IS NULL) + last_used_at, scopes, workspace_id, read_only FROM token WHERE email = $1 AND (label != 'ephemeral-script' OR label IS NULL) ORDER BY created_at DESC LIMIT $2 OFFSET $3", email, per_page as i64, @@ -2261,7 +2275,7 @@ async fn list_tokens( sqlx::query_as!( TruncatedToken, "SELECT label, token_prefix, expiration, created_at, \ - last_used_at, scopes, workspace_id FROM token WHERE email = $1 + last_used_at, scopes, workspace_id, read_only FROM token WHERE email = $1 ORDER BY created_at DESC LIMIT $2 OFFSET $3", email, per_page as i64, diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 945da4a614..975d697627 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -1,7 +1,7 @@ openapi: "3.0.3" info: - version: 1.700.2 + version: 1.702.1 title: Windmill API contact: @@ -22639,10 +22639,13 @@ components: type: string workspace_id: type: string + read_only: + type: boolean required: - token_prefix - created_at - last_used_at + - read_only ExternalJwtToken: type: object @@ -22691,6 +22694,12 @@ components: type: string workspace_id: type: string + read_only: + type: boolean + description: | + If true, the token is restricted to read-only HTTP methods + (GET/HEAD/OPTIONS). Mutating endpoints and job-run actions are + rejected with 403, regardless of the scopes attached. NewTokenImpersonate: type: object @@ -26367,7 +26376,7 @@ components: type: string login_type: type: string - enum: ["password", "github"] + enum: ["password", "github", "service_account"] super_admin: type: boolean devops: @@ -26386,9 +26395,11 @@ components: type: boolean role_source: type: string - enum: ["manual", "instance_group"] + enum: ["manual", "instance_group", "service_account"] disabled: type: boolean + workspace_id: + type: string required: - email diff --git a/backend/windmill-api/src/lib.rs b/backend/windmill-api/src/lib.rs index 0fde5cacd1..75b3469c41 100644 --- a/backend/windmill-api/src/lib.rs +++ b/backend/windmill-api/src/lib.rs @@ -330,6 +330,7 @@ async fn inject_agent_authed( scopes: None, username_override: None, token_prefix: None, + read_only: false, }, job_id: None, }); diff --git a/backend/windmill-api/src/mcp/core.rs b/backend/windmill-api/src/mcp/core.rs index 07ce233278..52fec66096 100644 --- a/backend/windmill-api/src/mcp/core.rs +++ b/backend/windmill-api/src/mcp/core.rs @@ -7,6 +7,7 @@ use async_trait::async_trait; use serde_json::Value; use std::collections::HashMap; use windmill_common::{db::UserDB, utils::StripPath, DB}; +use windmill_mcp::common::schema::enrich_resource_schemas; use windmill_mcp::common::transform::apply_key_transformation; use windmill_mcp::common::types::{ FlowInfo, HubScriptInfo, ResourceInfo, ResourceType, SchemaType, ScriptInfo, @@ -203,71 +204,13 @@ impl McpBackend for WindmillBackend { schema_obj.properties.insert(new_key, value); } - for (_key, prop_value) in schema_obj.properties.iter_mut() { - if let Value::Object(prop_map) = prop_value { - if let Some(format_value) = prop_map.get("format") { - if let Value::String(format_str) = format_value { - if format_str.starts_with("resource-") { - let resource_type_key = - format_str.split("-").last().unwrap_or_default().to_string(); - let resource_type = resources_types - .iter() - .find(|rt| rt.name == resource_type_key); - let resource_type_obj = resource_type.cloned(); - - if let Some(resource_cache) = resources_cache.get(&resource_type_key) { - let resources_count = resource_cache.len(); - let description = match resource_type_obj { - Some(resource_type_obj) => format!( - "This is a resource named `{}` with the following description: `{}`.\\nThe path of the resource should be used to specify the resource.\\n{}", - resource_type_obj.name, - resource_type_obj.description.as_deref().unwrap_or("No description"), - if resources_count == 0 { - "This resource does not have any available instances, you should create one from your windmill workspace." - } else if resources_count > 1 { - "This resource has multiple available instances, you should precisely select the one you want to use." - } else { - "There is 1 resource available." - } - ), - None => "An object parameter.".to_string(), - }; - prop_map.insert( - "type".to_string(), - Value::String("string".to_string()), - ); - prop_map - .insert("description".to_string(), Value::String(description)); - if resources_count > 0 { - let resources_description = resource_cache - .iter() - .map(|resource| { - format!( - "{}: $res:{}", - resource - .description - .as_deref() - .unwrap_or("No title"), - resource.path - ) - }) - .collect::>() - .join("\\n"); - - prop_map.insert( - "description".to_string(), - Value::String(format!( - "{}\\nHere are the available resources, in the format title:path. Title can be empty. Path should be used to specify the resource:\\n{}", - prop_map.get("description").unwrap_or(&Value::String("No description".to_string())), - resources_description - )), - ); - } - } - } - } - } - } + // Enrich every resource reference in the schema — including those + // inside `items`, nested `properties`, etc. — with a description + // listing the available resources. Both shapes are handled: + // { type: "object", format: "resource-" } (top-level scalar) + // { type: "resource", resourceType: "" } (inside list items) + for prop_value in schema_obj.properties.values_mut() { + enrich_resource_schemas(prop_value, resources_cache, resources_types); } schema_obj diff --git a/backend/windmill-api/src/workspaces_export.rs b/backend/windmill-api/src/workspaces_export.rs index e6b3b9ee0f..3f0cd3c646 100644 --- a/backend/windmill-api/src/workspaces_export.rs +++ b/backend/windmill-api/src/workspaces_export.rs @@ -110,6 +110,12 @@ struct ScriptMetadata { pub debouncing_settings: DebouncingSettings, #[serde(skip_serializing_if = "Option::is_none")] pub labels: Option>, + #[serde(skip_serializing_if = "is_empty_extra_perms")] + pub extra_perms: serde_json::Value, +} + +fn is_empty_extra_perms(value: &serde_json::Value) -> bool { + value.as_object().is_some_and(|o| o.is_empty()) || value.is_null() } pub fn is_none_or_false(val: &Option) -> bool { @@ -224,12 +230,34 @@ pub(crate) struct ArchiveQueryParams { default_ts: Option, /// Settings format version: "v1" (default) returns legacy flat format, "v2" returns grouped format settings_version: Option, + /// Opt-in: include `extra_perms` on flow / script / app rows. Default `false` + /// so cross-workspace tarball imports do not carry over ACLs referring to + /// identities that may not exist in the target workspace. `wmill sync pull` + /// passes `true` to surface ACLs in the git-tracked yaml. + preserve_extra_perms: Option, +} + +/// How to handle `extra_perms` in the serialized output. +/// +/// * `Drop` — strip the field unconditionally (legacy behavior for +/// types that have never carried ACLs in source). +/// * `KeepEvenEmpty` — always keep the field, even when `{}`. Matches the +/// pre-existing serialization for folders and groups so +/// no customer sees a one-time noisy diff on upgrade. +/// * `KeepIfNonEmpty` — keep when there is at least one entry, drop when `{}` +/// or null. New surface for flow / script / app, which +/// never carried ACLs in source before this change. +#[derive(Clone, Copy)] +pub enum ExtraPermsBehavior { + Drop, + KeepEvenEmpty, + KeepIfNonEmpty, } #[inline] pub fn to_string_without_metadata( value: &T, - preserve_extra_perms: bool, + extra_perms: ExtraPermsBehavior, ignore_keys: Option>, ) -> Result where @@ -274,8 +302,19 @@ where o2.remove("on_behalf_of"); o2.remove("on_behalf_of_email"); } - if !preserve_extra_perms && obj.contains_key("extra_perms") { - obj.remove("extra_perms"); + if obj.contains_key("extra_perms") { + let is_empty_extra_perms = obj + .get("extra_perms") + .map(|v| v.as_object().is_some_and(|o| o.is_empty()) || v.is_null()) + .unwrap_or(true); + let drop = match extra_perms { + ExtraPermsBehavior::Drop => true, + ExtraPermsBehavior::KeepEvenEmpty => false, + ExtraPermsBehavior::KeepIfNonEmpty => is_empty_extra_perms, + }; + if drop { + obj.remove("extra_perms"); + } } if obj .get("default_permissioned_as") @@ -442,6 +481,7 @@ pub(crate) async fn tarball_workspace( include_workspace_dependencies, default_ts, settings_version, + preserve_extra_perms, }): Query, ) -> Result<([(HeaderName, String); 2], impl IntoResponse)> { tracing::info!( @@ -452,6 +492,16 @@ pub(crate) async fn tarball_workspace( skip_resources ); + // Opt-in behavior for surfacing per-resource ACLs on flow/app rows. + // Folder and group rows have always carried `extra_perms` in source and + // continue to do so unconditionally (`KeepEvenEmpty`) so existing + // customer git repos see no one-time noisy diff. + let new_kinds_extra_perms = if preserve_extra_perms.unwrap_or(false) { + ExtraPermsBehavior::KeepIfNonEmpty + } else { + ExtraPermsBehavior::Drop + }; + let mut tx = user_db.begin(&authed).await?; // Source-of-truth check for fork-ness: the workspace's parent_workspace_id @@ -498,7 +548,8 @@ pub(crate) async fn tarball_workspace( for folder in folders { archive .write_to_archive( - &to_string_without_metadata(&folder, true, None).unwrap(), + &to_string_without_metadata(&folder, ExtraPermsBehavior::KeepEvenEmpty, None) + .unwrap(), &format!("f/{}/folder.meta.json", folder.name), ) .await?; @@ -506,15 +557,13 @@ pub(crate) async fn tarball_workspace( } { - let scripts = sqlx::query_as::<_, Script>( - &format!( - "SELECT {} FROM script as o WHERE workspace_id = $1 AND archived = false + let scripts = sqlx::query_as::<_, Script>(&format!( + "SELECT {} FROM script as o WHERE workspace_id = $1 AND archived = false AND (draft_only IS NULL OR draft_only = false) AND created_at = (select max(created_at) from script where path = o.path AND \ workspace_id = $1)", - windmill_common::scripts::SCRIPT_COLUMNS, - ), - ) + windmill_common::scripts::SCRIPT_COLUMNS, + )) .bind(&w_id) .fetch_all(&mut *tx) .await?; @@ -587,6 +636,15 @@ pub(crate) async fn tarball_workspace( on_behalf_of_email: script.on_behalf_of_email, modules: script.modules, labels: script.labels, + // Same opt-in contract as flow/app: the tarball only surfaces + // ACLs when `?preserve_extra_perms=true`. Passing `Null` lets the + // `is_empty_extra_perms` skip-serializer drop the field entirely. + extra_perms: if matches!(new_kinds_extra_perms, ExtraPermsBehavior::KeepIfNonEmpty) + { + script.extra_perms + } else { + serde_json::Value::Null + }, }; let metadata_str = serde_json::to_string_pretty(&metadata).unwrap(); archive @@ -605,7 +663,8 @@ pub(crate) async fn tarball_workspace( .await?; for resource in resources { - let resource_str = &to_string_without_metadata(&resource, false, None).unwrap(); + let resource_str = + &to_string_without_metadata(&resource, ExtraPermsBehavior::Drop, None).unwrap(); archive .write_to_archive(&resource_str, &format!("{}.resource.json", resource.path)) .await?; @@ -622,7 +681,9 @@ pub(crate) async fn tarball_workspace( .await?; for resource_type in resource_types { - let resource_str = &to_string_without_metadata(&resource_type, false, None).unwrap(); + let resource_str = + &to_string_without_metadata(&resource_type, ExtraPermsBehavior::Drop, None) + .unwrap(); archive .write_to_archive( &resource_str, @@ -644,7 +705,7 @@ pub(crate) async fn tarball_workspace( .await?; for flow in flows { - let flow_str = &to_string_without_metadata(&flow, false, None).unwrap(); + let flow_str = &to_string_without_metadata(&flow, new_kinds_extra_perms, None).unwrap(); archive .write_to_archive(&flow_str, &format!("{}.flow.json", flow.path)) .await?; @@ -673,7 +734,8 @@ pub(crate) async fn tarball_workspace( Error::internal_err(format!("Error decrypting variable {}: {}", var.path, e)) })?); } - let var_str = &to_string_without_metadata(&var, false, None).unwrap(); + let var_str = + &to_string_without_metadata(&var, ExtraPermsBehavior::Drop, None).unwrap(); archive .write_to_archive(&var_str, &format!("{}.variable.json", var.path)) .await?; @@ -693,7 +755,7 @@ pub(crate) async fn tarball_workspace( .await?; for app in apps { - let app_str = &to_string_without_metadata(&app, false, None).unwrap(); + let app_str = &to_string_without_metadata(&app, new_kinds_extra_perms, None).unwrap(); let kind = if app.raw_app { "raw_app" } else { "app" }; archive .write_to_archive(&app_str, &format!("{}.{}.json", app.path, kind)) @@ -711,7 +773,7 @@ pub(crate) async fn tarball_workspace( workspace_dependencies.len() ); for dep in workspace_dependencies { - // let dep_str = &to_string_without_metadata(&dep, false, None).unwrap(); + // let dep_str = &to_string_without_metadata(&dep, ExtraPermsBehavior::Drop, None).unwrap(); let filename = WorkspaceDependencies::to_path(&dep.name, dep.language)?; tracing::info!( "Adding workspace dependency: name={:?}, language={:?}, filename={}", @@ -739,9 +801,12 @@ pub(crate) async fn tarball_workspace( let schedule_ignore_keys = fork_schedule_ignore_keys(is_fork); for schedule in schedules { - let app_str = - &to_string_without_metadata(&schedule, false, schedule_ignore_keys.clone()) - .unwrap(); + let app_str = &to_string_without_metadata( + &schedule, + ExtraPermsBehavior::Drop, + schedule_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive(&app_str, &format!("{}.schedule.json", schedule.path)) .await?; @@ -777,9 +842,12 @@ pub(crate) async fn tarball_workspace( let http_triggers = handler.list_triggers(&mut *tx, &w_id, None).await?; for trigger in http_triggers { - let trigger_str = - &to_string_without_metadata(&trigger, false, trigger_ignore_keys.clone()) - .unwrap(); + let trigger_str = &to_string_without_metadata( + &trigger, + ExtraPermsBehavior::Drop, + trigger_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive( &trigger_str, @@ -796,9 +864,12 @@ pub(crate) async fn tarball_workspace( let websocket_triggers = handler.list_triggers(&mut *tx, &w_id, None).await?; for trigger in websocket_triggers { - let trigger_str = - &to_string_without_metadata(&trigger, false, trigger_ignore_keys.clone()) - .unwrap(); + let trigger_str = &to_string_without_metadata( + &trigger, + ExtraPermsBehavior::Drop, + trigger_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive( &trigger_str, @@ -815,9 +886,12 @@ pub(crate) async fn tarball_workspace( let kafka_triggers = handler.list_triggers(&mut *tx, &w_id, None).await?; for trigger in kafka_triggers { - let trigger_str = - &to_string_without_metadata(&trigger, false, trigger_ignore_keys.clone()) - .unwrap(); + let trigger_str = &to_string_without_metadata( + &trigger, + ExtraPermsBehavior::Drop, + trigger_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive( &trigger_str, @@ -834,9 +908,12 @@ pub(crate) async fn tarball_workspace( let sqs_triggers = handler.list_triggers(&mut *tx, &w_id, None).await?; for trigger in sqs_triggers { - let trigger_str = - &to_string_without_metadata(&trigger, false, trigger_ignore_keys.clone()) - .unwrap(); + let trigger_str = &to_string_without_metadata( + &trigger, + ExtraPermsBehavior::Drop, + trigger_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive( &trigger_str, @@ -853,9 +930,12 @@ pub(crate) async fn tarball_workspace( let gcp_triggers = handler.list_triggers(&mut *tx, &w_id, None).await?; for trigger in gcp_triggers { - let trigger_str = - &to_string_without_metadata(&trigger, false, trigger_ignore_keys.clone()) - .unwrap(); + let trigger_str = &to_string_without_metadata( + &trigger, + ExtraPermsBehavior::Drop, + trigger_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive( &trigger_str, @@ -872,9 +952,12 @@ pub(crate) async fn tarball_workspace( let azure_triggers = handler.list_triggers(&mut *tx, &w_id, None).await?; for trigger in azure_triggers { - let trigger_str = - &to_string_without_metadata(&trigger, false, trigger_ignore_keys.clone()) - .unwrap(); + let trigger_str = &to_string_without_metadata( + &trigger, + ExtraPermsBehavior::Drop, + trigger_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive( &trigger_str, @@ -891,9 +974,12 @@ pub(crate) async fn tarball_workspace( let nats_triggers = handler.list_triggers(&mut *tx, &w_id, None).await?; for trigger in nats_triggers { - let trigger_str: &String = - &to_string_without_metadata(&trigger, false, trigger_ignore_keys.clone()) - .unwrap(); + let trigger_str: &String = &to_string_without_metadata( + &trigger, + ExtraPermsBehavior::Drop, + trigger_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive( &trigger_str, @@ -910,9 +996,12 @@ pub(crate) async fn tarball_workspace( let postgres_triggers = handler.list_triggers(&mut *tx, &w_id, None).await?; for trigger in postgres_triggers { - let trigger_str = - &to_string_without_metadata(&trigger, false, trigger_ignore_keys.clone()) - .unwrap(); + let trigger_str = &to_string_without_metadata( + &trigger, + ExtraPermsBehavior::Drop, + trigger_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive( &trigger_str, @@ -929,9 +1018,12 @@ pub(crate) async fn tarball_workspace( let mqtt_triggers = handler.list_triggers(&mut *tx, &w_id, None).await?; for trigger in mqtt_triggers { - let trigger_str = - &to_string_without_metadata(&trigger, false, trigger_ignore_keys.clone()) - .unwrap(); + let trigger_str = &to_string_without_metadata( + &trigger, + ExtraPermsBehavior::Drop, + trigger_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive( &trigger_str, @@ -948,9 +1040,12 @@ pub(crate) async fn tarball_workspace( let email_triggers = handler.list_triggers(&mut *tx, &w_id, None).await?; for trigger in email_triggers { - let trigger_str = - &to_string_without_metadata(&trigger, false, trigger_ignore_keys.clone()) - .unwrap(); + let trigger_str = &to_string_without_metadata( + &trigger, + ExtraPermsBehavior::Drop, + trigger_ignore_keys.clone(), + ) + .unwrap(); archive .write_to_archive( &trigger_str, @@ -978,7 +1073,7 @@ pub(crate) async fn tarball_workspace( for trigger in native_triggers { let trigger_str = &to_string_without_metadata( &trigger, - false, + ExtraPermsBehavior::Drop, Some(native_ignore_keys.clone()), ) .unwrap(); @@ -1021,7 +1116,9 @@ pub(crate) async fn tarball_workspace( disabled: user.disabled, email: user.email, }; - let user_str = &to_string_without_metadata(&user, false, Some(vec!["email"])).unwrap(); + let user_str = + &to_string_without_metadata(&user, ExtraPermsBehavior::Drop, Some(vec!["email"])) + .unwrap(); archive .write_to_archive(&user_str, &format!("users/{}.user.json", user.email)) .await?; @@ -1081,7 +1178,9 @@ pub(crate) async fn tarball_workspace( admins, }; - let group_str = &to_string_without_metadata(&group, true, None).unwrap(); + let group_str = + &to_string_without_metadata(&group, ExtraPermsBehavior::KeepEvenEmpty, None) + .unwrap(); archive .write_to_archive(&group_str, &format!("groups/{}.group.json", group.name)) .await?; diff --git a/backend/windmill-common/src/instance_config.rs b/backend/windmill-common/src/instance_config.rs index e19182be30..3b168761e1 100644 --- a/backend/windmill-common/src/instance_config.rs +++ b/backend/windmill-common/src/instance_config.rs @@ -616,6 +616,12 @@ pub struct OtelTracingProxySettings { pub enabled: bool, #[serde(default, skip_serializing_if = "Vec::is_empty")] pub enabled_languages: Vec, + /// Comma-separated list of host patterns injected as NO_PROXY into jobs so their HTTP + /// clients bypass the local MITM tracing proxy. Independent of the worker's own + /// NO_PROXY env (which governs the proxy's upstream relay). Use this for clients that + /// pin their own CA (kubectl, helm, terraform providers, aws cli for EKS, etc.). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub no_proxy_hosts: Option, } /// Script language identifier (for instance config use). diff --git a/backend/windmill-mcp/src/common/schema.rs b/backend/windmill-mcp/src/common/schema.rs index 054b0e999a..de90248689 100644 --- a/backend/windmill-mcp/src/common/schema.rs +++ b/backend/windmill-mcp/src/common/schema.rs @@ -2,11 +2,11 @@ //! //! Contains functions for converting Windmill schemas into MCP-compatible formats. -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; -use serde_json::Value; +use serde_json::{Map, Value}; -use super::types::SchemaType; +use super::types::{ResourceInfo, ResourceType, SchemaType}; use windmill_common::scripts::Schema; /// Convert a Windmill Schema to a SchemaType @@ -22,38 +22,195 @@ pub fn convert_schema_to_schema_type(schema: Option) -> SchemaType { schema_obj } -/// Extract resource type keys from a schema -/// -/// Scans the schema properties for fields with format "resource-{type}" -/// and returns a set of all unique resource type names found. -pub fn extract_resource_types_from_schema(schema: &SchemaType) -> HashSet { - let mut resource_types = HashSet::new(); - for (_key, prop_value) in schema.properties.iter() { - if let Value::Object(prop_map) = prop_value { - if let Some(Value::String(format_str)) = prop_map.get("format") { - if let Some(rt) = format_str.strip_prefix("resource-") { - resource_types.insert(rt.to_string()); - } +/// If `node` is a schema describing a Windmill resource reference, return the +/// resource type name. Recognizes both on-disk shapes: +/// - Form A (top-level scalar): `{ type: "object", format: "resource-" }` +/// - Form B (inside `items`): `{ type: "resource", resourceType: "" }` +fn resource_type_of_schema_node(node: &Value) -> Option { + let obj = node.as_object()?; + if let Some(Value::String(fmt)) = obj.get("format") { + if let Some(name) = fmt.strip_prefix("resource-") { + return Some(name.to_string()); + } + } + if obj.get("type").and_then(Value::as_str) == Some("resource") { + if let Some(Value::String(name)) = obj.get("resourceType") { + return Some(name.clone()); + } + } + None +} + +/// Recursively collect resource type names referenced at any depth in `node`. +fn collect_resource_types(node: &Value, out: &mut HashSet) { + if let Some(rt) = resource_type_of_schema_node(node) { + out.insert(rt); + } + let Some(obj) = node.as_object() else { return }; + if let Some(Value::Object(props)) = obj.get("properties") { + for v in props.values() { + collect_resource_types(v, out); + } + } + if let Some(items) = obj.get("items") { + collect_resource_types(items, out); + } + if let Some(additional) = obj.get("additionalProperties") { + if additional.is_object() { + collect_resource_types(additional, out); + } + } + for kw in ["allOf", "oneOf", "anyOf"] { + if let Some(Value::Array(arr)) = obj.get(kw) { + for s in arr { + collect_resource_types(s, out); } } } +} + +/// Extract resource type keys referenced anywhere in a schema (top-level +/// properties, nested objects, array items, additionalProperties, and +/// allOf/oneOf/anyOf subschemas). Recognizes both the `format: resource-` +/// and `type: resource` + `resourceType` shapes. +pub fn extract_resource_types_from_schema(schema: &SchemaType) -> HashSet { + let mut resource_types = HashSet::new(); + for prop_value in schema.properties.values() { + collect_resource_types(prop_value, &mut resource_types); + } resource_types } +/// Rewrite a single schema node that points to a Windmill resource: set +/// `type: "string"` and inject a description listing the available resources. +/// Mirrors the top-level behavior that used to live in +/// `transform_schema_for_resources`. No-op if the resource type isn't in the +/// pre-fetched cache. +fn apply_resource_enrichment( + prop_map: &mut Map, + resource_type_key: &str, + resources_cache: &HashMap>, + resources_types: &[ResourceType], +) { + let Some(resource_cache) = resources_cache.get(resource_type_key) else { + return; + }; + let resource_type = resources_types + .iter() + .find(|rt| rt.name == resource_type_key); + let resources_count = resource_cache.len(); + let description = match resource_type { + Some(rt) => format!( + "This is a resource named `{}` with the following description: `{}`.\\nThe path of the resource should be used to specify the resource.\\n{}", + rt.name, + rt.description.as_deref().unwrap_or("No description"), + if resources_count == 0 { + "This resource does not have any available instances, you should create one from your windmill workspace." + } else if resources_count > 1 { + "This resource has multiple available instances, you should precisely select the one you want to use." + } else { + "There is 1 resource available." + } + ), + None => "An object parameter.".to_string(), + }; + prop_map.insert("type".to_string(), Value::String("string".to_string())); + prop_map.insert("description".to_string(), Value::String(description)); + // Drop the Windmill-internal keys we just consumed so the node is clean + // regardless of whether `make_schema_compatible` runs after us. (Its strip + // only fires while `type == "resource"`, which is no longer true here.) + prop_map.remove("resourceType"); + if prop_map + .get("format") + .and_then(Value::as_str) + .is_some_and(|s| s.starts_with("resource-")) + { + prop_map.remove("format"); + } + if resources_count > 0 { + let resources_description = resource_cache + .iter() + .map(|resource| { + format!( + "{}: $res:{}", + resource.description.as_deref().unwrap_or("No title"), + resource.path + ) + }) + .collect::>() + .join("\\n"); + let prior_description = prop_map + .get("description") + .and_then(Value::as_str) + .unwrap_or("No description") + .to_string(); + prop_map.insert( + "description".to_string(), + Value::String(format!( + "{}\\nHere are the available resources, in the format title:path. Title can be empty. Path should be used to specify the resource:\\n{}", + prior_description, resources_description + )), + ); + } +} + +/// Walk a schema and enrich every Windmill-resource reference (in either shape, +/// at any nesting depth) with `type: "string"` and a description listing +/// available resources. The non-standard keys (`format: resource-*`, +/// `resourceType`) consumed by the enrichment are stripped in place. +pub fn enrich_resource_schemas( + node: &mut Value, + resources_cache: &HashMap>, + resources_types: &[ResourceType], +) { + if let Some(rt_key) = resource_type_of_schema_node(node) { + if let Value::Object(obj) = node { + apply_resource_enrichment(obj, &rt_key, resources_cache, resources_types); + } + } + let Some(obj) = node.as_object_mut() else { + return; + }; + if let Some(Value::Object(props)) = obj.get_mut("properties") { + for v in props.values_mut() { + enrich_resource_schemas(v, resources_cache, resources_types); + } + } + if let Some(items) = obj.get_mut("items") { + enrich_resource_schemas(items, resources_cache, resources_types); + } + if let Some(additional) = obj.get_mut("additionalProperties") { + if additional.is_object() { + enrich_resource_schemas(additional, resources_cache, resources_types); + } + } + for kw in ["allOf", "oneOf", "anyOf"] { + if let Some(Value::Array(arr)) = obj.get_mut(kw) { + for s in arr.iter_mut() { + enrich_resource_schemas(s, resources_cache, resources_types); + } + } + } +} + /// Transform a JSON schema for maximum MCP client compatibility. /// /// Ensures schemas conform to JSON Schema draft 2020-12 by: /// - Converting `integer` type to `number` (some clients don't support integer) /// - Removing invalid non-array `enum` values /// - Stripping non-standard keywords (`originalType`, `format` with `resource-*` prefix) +/// - Rewriting the Windmill pseudo-type `type: "resource"` to `type: "string"` /// - Fixing contradictory schemas (`type: "string"` with `properties` → `type: "object"`) /// - Removing `default: null` when the type doesn't include `null` /// - Adding `type: "object"` to empty schemas that have no type pub fn make_schema_compatible(schema: &mut Value) { let Value::Object(obj) = schema else { return }; - // 1. Strip non-standard keywords that aren't part of JSON Schema + // 1. Strip non-standard keywords that aren't part of JSON Schema. The + // Windmill-internal `resourceType` is dropped unconditionally so it can't + // leak through even on enrichment cache-miss paths. obj.remove("originalType"); + obj.remove("resourceType"); // 2. Strip non-standard format values (resource-* is Windmill-internal) if obj @@ -64,6 +221,14 @@ pub fn make_schema_compatible(schema: &mut Value) { obj.remove("format"); } + // 2b. Rewrite Windmill pseudo-type `resource` to `string`. The parser emits + // this shape (with a sibling `resourceType` key) for `list[ResourceType]` + // params; "resource" is not in the JSON Schema 2020-12 type enum and is + // rejected by strict validators (e.g. Anthropic's tool registration). + if obj.get("type").and_then(|v| v.as_str()) == Some("resource") { + obj.insert("type".to_string(), Value::String("string".to_string())); + } + // 3. Fix contradictory type: if `properties` is present, type must be "object" if obj.contains_key("properties") { match obj.get("type").and_then(|v| v.as_str()) { @@ -151,8 +316,27 @@ pub fn make_schema_compatible(schema: &mut Value) { #[cfg(test)] mod tests { - use super::make_schema_compatible; + use super::*; + use crate::common::types::{ResourceInfo, ResourceType}; use serde_json::json; + use std::collections::HashMap; + + fn aws_resources() -> (HashMap>, Vec) { + let mut cache = HashMap::new(); + cache.insert( + "c_aws_account".to_string(), + vec![ResourceInfo { + path: "f/platform/aws_dev".to_string(), + description: Some("Dev account".to_string()), + resource_type: "c_aws_account".to_string(), + }], + ); + let types = vec![ResourceType { + name: "c_aws_account".to_string(), + description: Some("AWS account".to_string()), + }]; + (cache, types) + } #[test] fn converts_nested_integer_types() { @@ -356,6 +540,96 @@ mod tests { assert_eq!(schema, json!({})); } + #[test] + fn rewrites_resource_pseudo_type_to_string() { + let mut schema = json!({ + "type": "resource", + "resourceType": "c_aws_account" + }); + + make_schema_compatible(&mut schema); + + assert_eq!(schema["type"], json!("string")); + assert!(schema.get("resourceType").is_none()); + } + + #[test] + fn rewrites_resource_pseudo_type_inside_array_items() { + // Phocas repro: list[ResourceType] parameter. Anthropic rejected this + // with "tools..custom.input_schema: JSON schema is invalid" because + // "resource" is not in the draft 2020-12 type enum. + let mut schema = json!({ + "type": "object", + "properties": { + "accounts": { + "type": "array", + "items": { + "type": "resource", + "resourceType": "c_aws_account" + } + } + } + }); + + make_schema_compatible(&mut schema); + + assert_eq!( + schema["properties"]["accounts"]["items"]["type"], + json!("string") + ); + assert!(schema["properties"]["accounts"]["items"] + .get("resourceType") + .is_none()); + } + + #[test] + fn rewrites_resource_pseudo_type_inside_nested_properties() { + let mut schema = json!({ + "type": "object", + "properties": { + "config": { + "type": "object", + "properties": { + "db": { + "type": "resource", + "resourceType": "postgresql" + } + } + } + } + }); + + make_schema_compatible(&mut schema); + + assert_eq!( + schema["properties"]["config"]["properties"]["db"]["type"], + json!("string") + ); + assert!(schema["properties"]["config"]["properties"]["db"] + .get("resourceType") + .is_none()); + } + + #[test] + fn strips_nested_resource_format_inside_array_items() { + let mut schema = json!({ + "type": "object", + "properties": { + "dbs": { + "type": "array", + "items": { + "type": "object", + "format": "resource-postgresql" + } + } + } + }); + + make_schema_compatible(&mut schema); + + assert!(schema["properties"]["dbs"]["items"].get("format").is_none()); + } + #[test] fn adds_type_to_schema_with_properties_but_no_type() { let mut schema = json!({ @@ -368,4 +642,144 @@ mod tests { assert_eq!(schema["type"], json!("object")); } + + #[test] + fn extract_resource_types_top_level_form_a() { + let schema: SchemaType = serde_json::from_value(json!({ + "type": "object", + "properties": { + "db": { "type": "object", "format": "resource-postgresql" } + }, + "required": [] + })) + .unwrap(); + + let types = extract_resource_types_from_schema(&schema); + assert!(types.contains("postgresql")); + assert_eq!(types.len(), 1); + } + + #[test] + fn extract_resource_types_inside_array_items_form_b() { + let schema: SchemaType = serde_json::from_value(json!({ + "type": "object", + "properties": { + "accounts": { + "type": "array", + "items": { "type": "resource", "resourceType": "c_aws_account" } + } + }, + "required": [] + })) + .unwrap(); + + let types = extract_resource_types_from_schema(&schema); + assert!(types.contains("c_aws_account")); + } + + #[test] + fn extract_resource_types_inside_nested_properties_and_one_of() { + let schema: SchemaType = serde_json::from_value(json!({ + "type": "object", + "properties": { + "config": { + "type": "object", + "properties": { + "db": { "type": "resource", "resourceType": "postgresql" } + } + }, + "either": { + "oneOf": [ + { "type": "object", "format": "resource-mysql" }, + { "type": "string" } + ] + } + }, + "required": [] + })) + .unwrap(); + + let types = extract_resource_types_from_schema(&schema); + assert!(types.contains("postgresql")); + assert!(types.contains("mysql")); + } + + #[test] + fn enrich_top_level_form_a_resource() { + let (cache, types) = aws_resources(); + let mut node = json!({ + "type": "object", + "format": "resource-c_aws_account" + }); + + enrich_resource_schemas(&mut node, &cache, &types); + + assert_eq!(node["type"], json!("string")); + assert!(node.get("format").is_none()); + let desc = node["description"].as_str().unwrap(); + assert!(desc.contains("c_aws_account")); + assert!(desc.contains("$res:f/platform/aws_dev")); + } + + #[test] + fn enrich_inside_array_items_form_b() { + // Phocas repro: items use the parser-style `type: resource` form. + let (cache, types) = aws_resources(); + let mut node = json!({ + "type": "array", + "items": { "type": "resource", "resourceType": "c_aws_account" } + }); + + enrich_resource_schemas(&mut node, &cache, &types); + + // The items schema should be rewritten to string with a description + // listing the available resources, and the Windmill-internal + // resourceType key should be stripped. + assert_eq!(node["items"]["type"], json!("string")); + assert!(node["items"].get("resourceType").is_none()); + let desc = node["items"]["description"].as_str().unwrap(); + assert!(desc.contains("$res:f/platform/aws_dev")); + } + + #[test] + fn enrich_is_noop_when_resource_type_not_in_cache() { + let mut node = json!({ + "type": "object", + "format": "resource-unknown_type" + }); + let before = node.clone(); + + enrich_resource_schemas(&mut node, &HashMap::new(), &[]); + + assert_eq!(node, before); + } + + #[test] + fn enrich_deeply_nested_resource() { + let (cache, types) = aws_resources(); + let mut node = json!({ + "type": "object", + "properties": { + "outer": { + "type": "object", + "properties": { + "inner": { + "type": "array", + "items": { + "type": "resource", + "resourceType": "c_aws_account" + } + } + } + } + } + }); + + enrich_resource_schemas(&mut node, &cache, &types); + + assert_eq!( + node["properties"]["outer"]["properties"]["inner"]["items"]["type"], + json!("string") + ); + } } diff --git a/backend/windmill-mcp/src/server/backend.rs b/backend/windmill-mcp/src/server/backend.rs index 0b942353b3..7a6e007c4d 100644 --- a/backend/windmill-mcp/src/server/backend.rs +++ b/backend/windmill-mcp/src/server/backend.rs @@ -33,6 +33,13 @@ pub trait McpAuth: Send + Sync + Clone + 'static { /// Get token scopes fn scopes(&self) -> Option<&[String]>; + /// True if the token was created with the `read_only` flag. + /// When set, write-capable tools must be hidden from `list_tools` and + /// rejected by `call_tool`. Defaults to false so existing impls compile. + fn read_only(&self) -> bool { + false + } + /// Check if the user has an MCP scope fn has_mcp_scope(&self) -> bool { self.scopes() diff --git a/backend/windmill-mcp/src/server/endpoints.rs b/backend/windmill-mcp/src/server/endpoints.rs index 2401b10757..373db36eb1 100644 --- a/backend/windmill-mcp/src/server/endpoints.rs +++ b/backend/windmill-mcp/src/server/endpoints.rs @@ -26,6 +26,12 @@ pub struct EndpointTool { pub body_field_renames: Option, } +/// True if this endpoint is safe to expose to a read-only token. Mirrors the +/// `read_only_hint` computed by `create_endpoint_annotations`: only `GET`. +pub fn is_endpoint_read_only(tool: &EndpointTool) -> bool { + tool.method.as_ref() == "GET" +} + /// Convert a single endpoint tool to MCP tool pub fn endpoint_tool_to_mcp_tool(tool: &EndpointTool) -> Tool { let mut combined_properties = serde_json::Map::new(); diff --git a/backend/windmill-mcp/src/server/mod.rs b/backend/windmill-mcp/src/server/mod.rs index d489b3f429..688c12b827 100644 --- a/backend/windmill-mcp/src/server/mod.rs +++ b/backend/windmill-mcp/src/server/mod.rs @@ -12,7 +12,7 @@ pub mod tools; // Re-export main types pub use backend::{BackendResult, McpAuth, McpBackend}; -pub use endpoints::{endpoint_tool_to_mcp_tool, EndpointTool}; +pub use endpoints::{endpoint_tool_to_mcp_tool, is_endpoint_read_only, EndpointTool}; pub use runner::Runner; pub use tools::create_tool_from_item; diff --git a/backend/windmill-mcp/src/server/runner.rs b/backend/windmill-mcp/src/server/runner.rs index 0cb2962c55..6d33573d95 100644 --- a/backend/windmill-mcp/src/server/runner.rs +++ b/backend/windmill-mcp/src/server/runner.rs @@ -145,94 +145,100 @@ impl ServerHandler for Runner { parse_mcp_scopes(scopes).map_err(|e| ErrorData::internal_error(e, None))?; let favorites_only = scope_config.favorites; - - // Fetch all items concurrently - let (scripts, flows, resource_types, hub_scripts) = tokio::try_join!( - self.backend - .list_scripts(&auth, &workspace_id, favorites_only, None), - self.backend - .list_flows(&auth, &workspace_id, favorites_only, None), - self.backend.list_resource_types(&auth, &workspace_id), - async { - if let Some(ref apps) = scope_config.hub_apps { - self.backend.list_hub_scripts(Some(apps)).await - } else { - Ok(vec![]) - } - } - )?; - - // Filter items based on scope - let filtered_scripts: Vec<_> = scripts - .into_iter() - .filter(|s| !scope_config.granular || scope_config.is_allowed("script", &s.path)) - .collect(); - - let filtered_flows: Vec<_> = flows - .into_iter() - .filter(|f| !scope_config.granular || scope_config.is_allowed("flow", &f.path)) - .collect(); - - // Collect all needed resource types from all schemas - let mut needed_resource_types: HashSet = HashSet::new(); - for script in &filtered_scripts { - needed_resource_types.extend(extract_resource_types_from_schema(&script.get_schema())); - } - for flow in &filtered_flows { - needed_resource_types.extend(extract_resource_types_from_schema(&flow.get_schema())); - } - for hub_script in &hub_scripts { - needed_resource_types - .extend(extract_resource_types_from_schema(&hub_script.get_schema())); - } - - // Pre-fetch all resources - let resource_futures: Vec<_> = needed_resource_types - .into_iter() - .map(|rt| { - let backend = self.backend.clone(); - let auth = auth.clone(); - let workspace_id = workspace_id.clone(); - async move { - backend - .list_resources(&auth, &workspace_id, &rt) - .await - .map(|resources| (rt, resources)) - } - }) - .collect(); - - let resource_results = futures::future::try_join_all(resource_futures).await?; - let resources_cache: HashMap> = - resource_results.into_iter().collect(); + let read_only = auth.read_only(); let mut tools = Vec::new(); - for script in &filtered_scripts { - tools.push(create_tool_from_item( - script, - self.backend.as_ref(), - &resources_cache, - &resource_types, - )); - } + // Read-only tokens cannot run scripts/flows/hub-scripts (running is a + // mutating action), so skip the script/flow/hub/resource fetches + // entirely — they would only be discarded below. + if !read_only { + let (scripts, flows, resource_types, hub_scripts) = tokio::try_join!( + self.backend + .list_scripts(&auth, &workspace_id, favorites_only, None), + self.backend + .list_flows(&auth, &workspace_id, favorites_only, None), + self.backend.list_resource_types(&auth, &workspace_id), + async { + if let Some(ref apps) = scope_config.hub_apps { + self.backend.list_hub_scripts(Some(apps)).await + } else { + Ok(vec![]) + } + } + )?; - for flow in &filtered_flows { - tools.push(create_tool_from_item( - flow, - self.backend.as_ref(), - &resources_cache, - &resource_types, - )); - } + let filtered_scripts: Vec<_> = scripts + .into_iter() + .filter(|s| !scope_config.granular || scope_config.is_allowed("script", &s.path)) + .collect(); - for hub_script in &hub_scripts { - tools.push(create_tool_from_item( - hub_script, - self.backend.as_ref(), - &resources_cache, - &resource_types, - )); + let filtered_flows: Vec<_> = flows + .into_iter() + .filter(|f| !scope_config.granular || scope_config.is_allowed("flow", &f.path)) + .collect(); + + // Collect all needed resource types from all schemas + let mut needed_resource_types: HashSet = HashSet::new(); + for script in &filtered_scripts { + needed_resource_types + .extend(extract_resource_types_from_schema(&script.get_schema())); + } + for flow in &filtered_flows { + needed_resource_types + .extend(extract_resource_types_from_schema(&flow.get_schema())); + } + for hub_script in &hub_scripts { + needed_resource_types + .extend(extract_resource_types_from_schema(&hub_script.get_schema())); + } + + // Pre-fetch all resources + let resource_futures: Vec<_> = needed_resource_types + .into_iter() + .map(|rt| { + let backend = self.backend.clone(); + let auth = auth.clone(); + let workspace_id = workspace_id.clone(); + async move { + backend + .list_resources(&auth, &workspace_id, &rt) + .await + .map(|resources| (rt, resources)) + } + }) + .collect(); + + let resource_results = futures::future::try_join_all(resource_futures).await?; + let resources_cache: HashMap> = + resource_results.into_iter().collect(); + + for script in &filtered_scripts { + tools.push(create_tool_from_item( + script, + self.backend.as_ref(), + &resources_cache, + &resource_types, + )); + } + + for flow in &filtered_flows { + tools.push(create_tool_from_item( + flow, + self.backend.as_ref(), + &resources_cache, + &resource_types, + )); + } + + for hub_script in &hub_scripts { + tools.push(create_tool_from_item( + hub_script, + self.backend.as_ref(), + &resources_cache, + &resource_types, + )); + } } // Add endpoint tools from the generated MCP tools, filtered by scope @@ -241,6 +247,9 @@ impl ServerHandler for Runner { if scope_config.granular && !scope_config.is_allowed("endpoint", &endpoint_tool.name) { continue; } + if read_only && !crate::server::is_endpoint_read_only(&endpoint_tool) { + continue; + } tools.push(endpoint_tool_to_mcp_tool(&endpoint_tool)); } @@ -259,6 +268,7 @@ impl ServerHandler for Runner { let scopes = auth.scopes().unwrap_or(&[]); let scope_config = parse_mcp_scopes(scopes).map_err(|e| ErrorData::internal_error(e, None))?; + let read_only = auth.read_only(); let args = request.arguments.map(Value::Object).unwrap_or(Value::Null); @@ -278,6 +288,15 @@ impl ServerHandler for Runner { None, )); } + if read_only && !crate::server::is_endpoint_read_only(endpoint_tool) { + return Err(ErrorData::internal_error( + format!( + "Access denied: endpoint '{}' is not read-only and this token is restricted to read-only operations", + endpoint_tool.name + ), + None, + )); + } // This is an endpoint tool, call via backend let result = self @@ -294,6 +313,18 @@ impl ServerHandler for Runner { } } + // Anything below this point runs a script or flow, which is a mutating + // action and must be denied for read-only tokens. + if read_only { + return Err(ErrorData::internal_error( + format!( + "Access denied: tool '{}' runs a script/flow and this token is restricted to read-only operations", + request.name + ), + None, + )); + } + // Resolve the tool name to (type, path, is_hub) let (type_str, is_hub, is_hashed) = parse_tool_prefix(&request.name).map_err(|e| { ErrorData::internal_error(format!("Failed to parse tool name: {}", e), None) diff --git a/backend/windmill-native-triggers/src/handler.rs b/backend/windmill-native-triggers/src/handler.rs index 1b6aa83b92..fda28af407 100644 --- a/backend/windmill-native-triggers/src/handler.rs +++ b/backend/windmill-native-triggers/src/handler.rs @@ -94,6 +94,7 @@ async fn new_webhook_token( None, Some(scopes), Some(workspace_id.to_owned()), + None, ); let token = create_token_internal(&mut *tx, &db, &authed, token_config).await?; diff --git a/backend/windmill-runtime-nativets/tests/otel_e2e.rs b/backend/windmill-runtime-nativets/tests/otel_e2e.rs new file mode 100644 index 0000000000..cadddb2fcf --- /dev/null +++ b/backend/windmill-runtime-nativets/tests/otel_e2e.rs @@ -0,0 +1,206 @@ +//! End-to-end coverage for the EE HTTP-tracing path on nativets. +//! +//! Pairs with `otel_init.rs` (which pins only the `OTEL_GLOBALS` +//! population contract). This test exercises the full chain that +//! actually delivers a span to a collector: +//! +//! `deno_telemetry::init` with EE config (Rust) +//! → `globalThis.__bootstrapOtel()` (JS, flips TRACING_ENABLED) +//! → user `fetch()` → deno_fetch's `builtinTracer().startSpan` +//! → `BatchSpanProcessor` → `HttpExporter` (OTLP/HTTP-binary) +//! → our mock OTLP listener captures the request bytes +//! +//! Without the v1.702.0 fix (#573 EE / #9163 OSS), the third arrow +//! panics in a tokio worker. With the fix in place, the span is +//! emitted and shows up at the listener — which is what the customer +//! is paying for when they enable HTTP tracing on nativets. +//! +//! `#[ignore]`'d: spins a V8 isolate (~seconds) and binds two TCP +//! listeners. Run with `cargo test -p windmill-runtime-nativets +//! --test otel_e2e -- --ignored`. +//! +//! Owns its own test binary so `OTEL_GLOBALS`'s `OnceCell` doesn't +//! race with `otel_init.rs`. + +use std::sync::Arc; +use std::time::Duration; + +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::TcpListener; +use tokio::sync::Mutex; + +use windmill_runtime_nativets::{deno_telemetry, transpile_ts, NativeAnnotation, PrewarmedIsolate}; + +/// Bind 127.0.0.1:0 and spawn an accept loop. Each connection is +/// read until idle/EOF, the body is appended to `captured`, then we +/// respond with HTTP 200. Returns the bound port. +async fn spawn_capturing_http(captured: Arc>>>) -> u16 { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + tokio::spawn(async move { + loop { + let Ok((mut sock, _)) = listener.accept().await else { + break; + }; + let captured = captured.clone(); + tokio::spawn(async move { + let mut buf = Vec::new(); + let mut tmp = [0u8; 8192]; + let _ = tokio::time::timeout(Duration::from_millis(300), async { + loop { + match sock.read(&mut tmp).await { + Ok(0) | Err(_) => break, + Ok(n) => buf.extend_from_slice(&tmp[..n]), + } + } + }) + .await; + let _ = sock + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n") + .await; + captured.lock().await.push(buf); + }); + } + }); + port +} + +/// Initialize `deno_telemetry` with the exact `OtelConfig` shape that +/// the EE `load_internal_otel_exporter` ships in production. Keeps +/// this test in lockstep with the actual call site: if production +/// drifts away from `tracing_enabled + Capture`, this test breaks +/// before the customer-facing panic does. +fn init_with_ee_otel_config() { + deno_telemetry::init( + deno_telemetry::OtelRuntimeConfig { + runtime_name: "windmill-nativets".into(), + runtime_version: "0".into(), + }, + deno_telemetry::OtelConfig { + tracing_enabled: true, + console: deno_telemetry::OtelConsoleConfig::Capture, + ..Default::default() + }, + ) + .expect("deno_telemetry init failed"); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +#[ignore = "spins V8 + tcp listeners; run with --ignored"] +async fn fetch_after_init_otel_emits_span_to_collector() { + let _ = rustls::crypto::ring::default_provider().install_default(); + + // 1. Stand up two listeners: one that pretends to be the user's + // fetch target, one that pretends to be the OTLP collector. + let fetch_hits: Arc>>> = Arc::new(Mutex::new(Vec::new())); + let otlp_hits: Arc>>> = Arc::new(Mutex::new(Vec::new())); + let fetch_port = spawn_capturing_http(fetch_hits.clone()).await; + let otlp_port = spawn_capturing_http(otlp_hits.clone()).await; + + // 2. Point the deno_telemetry exporter at the mock collector and + // initialize. Mirrors `load_internal_otel_exporter` in EE. + // + // SAFETY: test runs in its own process binary; no other thread + // reads OTEL_EXPORTER_OTLP_ENDPOINT before init returns. + unsafe { + std::env::set_var( + "OTEL_EXPORTER_OTLP_ENDPOINT", + format!("http://127.0.0.1:{otlp_port}"), + ); + } + init_with_ee_otel_config(); + assert!( + deno_telemetry::OTEL_GLOBALS.get().is_some(), + "OTEL_GLOBALS missing — load_internal_otel_exporter's config regressed?" + ); + + // 3. Run user TS that bootstraps OTel and then issues a fetch + // at the mock target. `__bootstrapOtel` is fire-and-forget + // (resolves a dynamic import on the microtask queue); the + // `setTimeout` loop yields a few times so the import resolves + // and the `TRACING_ENABLED` flag is set before fetch runs. + let ts = format!( + r#" +declare const globalThis: any; +export async function main(): Promise {{ + globalThis.__bootstrapOtel(); + // Yield multiple microtask + timer turns so the dynamic import + // in __bootstrapOtel resolves and TRACING_ENABLED flips before + // fetch runs (otherwise deno_fetch skips the span entirely). + for (let i = 0; i < 5; i++) {{ + await new Promise(r => setTimeout(r, 10)); + }} + const resp = await fetch("http://127.0.0.1:{fetch_port}/probe"); + return resp.status; +}} +"# + ); + let js = transpile_ts(ts).expect("transpile failed"); + let ann = NativeAnnotation { useragent: None, proxy: None }; + + let mut iso = PrewarmedIsolate::spawn(String::new(), js, ann, vec![], None); + iso.wait_ready().await.expect("isolate failed to pre-warm"); + let res = iso + .start_execution("{}".to_string()) + .wait() + .await + .expect("isolate panicked"); + + // The exact bug: pre-fix, fetch panics this isolate. Post-fix, + // we get back the mock target's 200. + let raw = res.result.expect("user script returned an error"); + assert_eq!(raw.get(), "200", "fetch should return mock target status"); + + assert_eq!( + fetch_hits.lock().await.len(), + 1, + "fetch target should have been hit exactly once" + ); + + // 4. Force the BatchSpanProcessor to flush so the exporter posts + // to our mock collector synchronously (default flush interval + // is ~5s; tests can't wait that long). + deno_telemetry::flush(); + // Exporter is async over the OTel runtime; give it a beat to + // actually send the HTTP request. + tokio::time::sleep(Duration::from_millis(500)).await; + + let otlp_captured = otlp_hits.lock().await; + assert!( + !otlp_captured.is_empty(), + "OTLP collector should have received at least one export — \ + spans aren't reaching the collector after init" + ); + + // Verify the export carries our fetch span. OTLP is protobuf, so + // grep the raw bytes for OTel HTTP semantic-convention markers + // that deno_fetch's auto-instrumentation attaches: + // - the target URL ("url.full" attribute) + // - "http.request.method" attribute + let combined: Vec = otlp_captured.iter().flatten().copied().collect(); + let bytes_contain = |needle: &[u8]| combined.windows(needle.len()).any(|w| w == needle); + + let url_marker = format!("http://127.0.0.1:{fetch_port}/probe"); + let has_url = bytes_contain(url_marker.as_bytes()); + let has_method_attr = bytes_contain(b"http.request.method"); + + if !(has_url && has_method_attr) { + eprintln!( + "OTLP bytes ({}): {:?}", + combined.len(), + String::from_utf8_lossy(&combined) + ); + } + + assert!( + has_url, + "exported OTLP body should reference the fetched URL ({}); got {} bytes", + url_marker, + combined.len() + ); + assert!( + has_method_attr, + "exported OTLP body should carry HTTP semconv attributes (http.request.method); got {} bytes", + combined.len() + ); +} diff --git a/backend/windmill-runtime-nativets/tests/otel_init.rs b/backend/windmill-runtime-nativets/tests/otel_init.rs new file mode 100644 index 0000000000..fdf1ac944d --- /dev/null +++ b/backend/windmill-runtime-nativets/tests/otel_init.rs @@ -0,0 +1,88 @@ +//! Regression test for the deno_telemetry 0.31 nativets-fetch panic. +//! +//! Setup: when EE's `load_internal_otel_exporter` runs (HTTP tracing +//! enabled), it calls `deno_telemetry::init` and then flips +//! `DENO_OTEL_INITIALIZED=true` so `js_eval` will run +//! `globalThis.__bootstrapOtel()` inside the nativets JsRuntime. That +//! bootstrap unconditionally sets JS-side `TRACING_ENABLED=true`, which +//! makes `deno_fetch`'s `fetch()` call `builtinTracer().startSpan(...)` +//! — and `OtelTracer::builtin()` does `OTEL_GLOBALS.get().unwrap()`. +//! +//! In deno_telemetry 0.31, `init` was given an early-return guard: if +//! `tracing_enabled`, `metrics_enabled`, and `console` are all +//! off/Ignore, it returns `Ok(())` *without populating OTEL_GLOBALS*. +//! v1.700.0 was passing `OtelConfig::default()` (all off) — so the JS +//! bootstrap proceeded but the Rust-side OnceCell was empty, and the +//! first `fetch()` panicked the tokio worker in a context that cannot +//! unwind. Fixed in v1.702.0 by passing `tracing_enabled: true` + +//! `console: Capture` from `load_internal_otel_exporter` (PRs #573 EE +//! / #9163 OSS), matching the JS bootstrap shape `[1, 0, 1, 0]`. +//! +//! This test pins that contract directly against `deno_telemetry:: +//! init` — the function the EE call site invokes — so the next time +//! the dep is bumped and someone is tempted to "simplify" the config +//! back to `OtelConfig::default()`, CI catches it. +//! +//! Lives in `tests/` (not `src/`) so it gets its own test binary and +//! its own process — `OTEL_GLOBALS` is a `OnceCell`, so sharing it +//! with the smoke tests in `src/smoke_tests.rs` would race. + +use windmill_runtime_nativets::deno_telemetry; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn ee_otel_init_config_populates_otel_globals() { + // deno_telemetry::init builds an HttpExporter (uses rustls) — it + // needs a process-wide CryptoProvider, which the real binary + // installs in setup_deno_runtime / main. Mirror that here. + let _ = rustls::crypto::ring::default_provider().install_default(); + + // Pre-condition: nothing else in this test binary has touched the + // OnceCell, so it must be empty. + assert!( + deno_telemetry::OTEL_GLOBALS.get().is_none(), + "OTEL_GLOBALS must start empty in a fresh test process" + ); + + // Step 1: reproduce the footgun. `OtelConfig::default()` has + // tracing/metrics off and `console = Ignore`, which trips the + // 0.31 early-return — Ok(()) but OnceCell stays empty. + deno_telemetry::init( + deno_telemetry::OtelRuntimeConfig { + runtime_name: "windmill-nativets-test".into(), + runtime_version: "0".into(), + }, + deno_telemetry::OtelConfig::default(), + ) + .expect("init with default config returns Ok (early-return)"); + + assert!( + deno_telemetry::OTEL_GLOBALS.get().is_none(), + "deno_telemetry 0.31 contract: init with all-disabled config \ + must NOT populate OTEL_GLOBALS — if this changes on a future \ + bump, load_internal_otel_exporter can drop the explicit \ + tracing_enabled/console fields." + ); + + // Step 2: re-call init with the exact config shape that the EE + // `load_internal_otel_exporter` ships — this is what production + // hits, so the test exercises the actual prod call path. + deno_telemetry::init( + deno_telemetry::OtelRuntimeConfig { + runtime_name: "windmill-nativets".into(), + runtime_version: "0".into(), + }, + deno_telemetry::OtelConfig { + tracing_enabled: true, + console: deno_telemetry::OtelConsoleConfig::Capture, + ..Default::default() + }, + ) + .expect("init with tracing_enabled config must succeed on a fresh OnceCell"); + + assert!( + deno_telemetry::OTEL_GLOBALS.get().is_some(), + "load_internal_otel_exporter's OtelConfig must populate \ + OTEL_GLOBALS so OtelTracer::builtin() does not panic on \ + .unwrap() once __bootstrapOtel flips JS-side TRACING_ENABLED" + ); +} diff --git a/backend/windmill-worker/src/bun_executor.rs b/backend/windmill-worker/src/bun_executor.rs index 5e640c1fde..97ff28cae1 100644 --- a/backend/windmill-worker/src/bun_executor.rs +++ b/backend/windmill-worker/src/bun_executor.rs @@ -2172,6 +2172,7 @@ try {{ "--", &BUN_PATH, "run", + "--preserve-symlinks", "-i", "--prefer-offline", "-r", @@ -2238,6 +2239,7 @@ try {{ } else { vec![ "run", + "--preserve-symlinks", "-i", "--prefer-offline", "-r", @@ -3895,6 +3897,7 @@ pub async fn start_worker( common_bun_proc_envs, vec![ "run", + "--preserve-symlinks", "-i", "--prefer-offline", "-r", diff --git a/backend/windmill-worker/src/python_executor.rs b/backend/windmill-worker/src/python_executor.rs index 4db7b20fd5..1f20148841 100644 --- a/backend/windmill-worker/src/python_executor.rs +++ b/backend/windmill-worker/src/python_executor.rs @@ -841,7 +841,11 @@ def to_b_64(v: bytes): b64 = base64.b64encode(v) return b64.decode('ascii') -replace_invalid_fields = re.compile(r'(?:\bNaN\b|\\*\\u0000|Infinity|\-Infinity)') +_u=re.compile(r'\\\\|\\u0000') +_us=lambda m:' null ' if m.group(0)[1]=='u' else m.group(0) +_r=lambda m,s='':(_u.sub(_us,s) if '\\u0000' in s else s) if (s:=m.group(0))[0]=='"' else ' null ' +replace_invalid_fields=re.compile(r'"(?:\\.|[^"\\])*"|\bNaN\b|-?Infinity') +_fix=lambda s:s if 'Infinity' not in s and 'NaN' not in s and '\\u0000' not in s else re.sub(replace_invalid_fields,_r,s) result_json = os.path.join(os.path.abspath(os.path.dirname(__file__)), "result.json") @@ -1367,7 +1371,11 @@ def to_b_64(v: bytes): b64 = base64.b64encode(v) return b64.decode('ascii') -replace_invalid_fields = re.compile(r'(?:\bNaN\b|\\u0000|Infinity|\-Infinity)') +_u=re.compile(r'\\\\|\\u0000') +_us=lambda m:' null ' if m.group(0)[1]=='u' else m.group(0) +_r=lambda m,s='':(_u.sub(_us,s) if '\\u0000' in s else s) if (s:=m.group(0))[0]=='"' else ' null ' +replace_invalid_fields=re.compile(r'"(?:\\.|[^"\\])*"|\bNaN\b|-?Infinity') +_fix=lambda s:s if 'Infinity' not in s and 'NaN' not in s and '\\u0000' not in s else re.sub(replace_invalid_fields,_r,s) def res_to_json(res, typ): {res_to_json_body} @@ -2951,7 +2959,7 @@ fn get_result_postprocessor<'a>(skip: bool) -> &'a str { if skip { "unprocessed" } else { - "re.sub(replace_invalid_fields, ' null ', unprocessed)" + "_fix(unprocessed)" } } diff --git a/backend/windmill-worker/src/worker.rs b/backend/windmill-worker/src/worker.rs index 15405800be..169c4526f9 100644 --- a/backend/windmill-worker/src/worker.rs +++ b/backend/windmill-worker/src/worker.rs @@ -277,6 +277,8 @@ pub struct OtelTracingProxySettings { pub enabled: bool, #[serde(default)] pub enabled_languages: HashSet, + #[serde(default)] + pub no_proxy_hosts: Option, } #[cfg(feature = "prometheus")] @@ -969,14 +971,15 @@ async fn get_otel_tracing_proxy_envs( } }; let proxy_url = format!("http://127.0.0.1:{}", port); + let no_proxy = build_tracing_proxy_no_proxy().await; Ok(vec![ ("HTTP_PROXY", proxy_url.clone()), ("HTTPS_PROXY", proxy_url.clone()), // Lowercase variants for Ruby and other runtimes that check lowercase first ("http_proxy", proxy_url.clone()), ("https_proxy", proxy_url), - ("NO_PROXY", "".to_string()), - ("no_proxy", "".to_string()), + ("NO_PROXY", no_proxy.clone()), + ("no_proxy", no_proxy), // CA cert for various runtimes to trust the tracing proxy ("SSL_CERT_FILE", TRACING_PROXY_CA_CERT_PATH.to_string()), ("REQUESTS_CA_BUNDLE", TRACING_PROXY_CA_CERT_PATH.to_string()), @@ -990,6 +993,70 @@ async fn get_otel_tracing_proxy_envs( ]) } +/// NO_PROXY value injected into jobs so their HTTP clients bypass the local MITM proxy for +/// the configured hosts. This is distinct from the worker's own NO_PROXY env, which governs +/// what the MITM proxy bypasses when relaying upstream (e.g. through a corporate proxy) and +/// is honored automatically by the in-process MITM. The configured hosts are tunneled +/// through the proxy without TLS interception, so clients that pin their own CA (kubectl, +/// helm, terraform, etc.) keep working. Empty when unset, matching the prior behavior of +/// intercepting all destinations including loopback. +#[cfg(all(feature = "private", feature = "enterprise"))] +async fn build_tracing_proxy_no_proxy() -> String { + let configured = OTEL_TRACING_PROXY_SETTINGS + .read() + .await + .no_proxy_hosts + .clone(); + normalize_no_proxy_hosts(configured.as_deref()) +} + +/// Split a comma-separated NO_PROXY value, trim whitespace, drop empty entries, and +/// deduplicate while preserving order. `None` returns an empty string. +#[cfg(all(feature = "private", feature = "enterprise"))] +fn normalize_no_proxy_hosts(configured: Option<&str>) -> String { + let Some(configured) = configured else { + return String::new(); + }; + let mut seen = std::collections::HashSet::new(); + let mut out: Vec<&str> = Vec::new(); + for entry in configured.split(',') { + let trimmed = entry.trim(); + if !trimmed.is_empty() && seen.insert(trimmed) { + out.push(trimmed); + } + } + out.join(",") +} + +#[cfg(all(test, feature = "private", feature = "enterprise"))] +mod no_proxy_tests { + use super::normalize_no_proxy_hosts; + + #[test] + fn unset_returns_empty() { + assert_eq!(normalize_no_proxy_hosts(None), ""); + } + + #[test] + fn empty_and_whitespace_only_returns_empty() { + assert_eq!(normalize_no_proxy_hosts(Some("")), ""); + assert_eq!(normalize_no_proxy_hosts(Some(" , ,\t")), ""); + } + + #[test] + fn trims_and_skips_empties() { + assert_eq!( + normalize_no_proxy_hosts(Some(" *.eks.amazonaws.com ,, *.internal ")), + "*.eks.amazonaws.com,*.internal" + ); + } + + #[test] + fn dedupes_preserving_first_occurrence_order() { + assert_eq!(normalize_no_proxy_hosts(Some("a,b,a,c,b,d")), "a,b,c,d"); + } +} + #[cfg(windows)] lazy_static::lazy_static! { pub static ref SYSTEM_ROOT: String = std::env::var("SystemRoot").unwrap_or_else(|_| "C:\\Windows".to_string()); diff --git a/backend/windmill-worker/src/worker_flow.rs b/backend/windmill-worker/src/worker_flow.rs index 53c94a7458..68ca83c270 100644 --- a/backend/windmill-worker/src/worker_flow.rs +++ b/backend/windmill-worker/src/worker_flow.rs @@ -3919,11 +3919,14 @@ async fn push_next_flow_job( for (i, payload_tag) in job_payloads.into_iter().enumerate() { if i % 100 == 0 && i != 0 { tracing::info!(id = %flow_job.id, root_id = %job_root, "pushed (non-commited yet) first {i} subflows of {len}"); + // Ping on the pool, outside `tx`, so the zombie flow monitor sees it before the + // push transaction commits — otherwise large parallel pushes can be flagged as + // zombie and trigger a cancel/push deadlock. sqlx::query!( - "UPDATE v2_job_runtime SET ping = now() WHERE id = $1 AND ping < now()", + "UPDATE v2_job_runtime SET ping = now() WHERE id = $1", flow_job.id, ) - .execute(&mut *tx) + .execute(db) .warn_after_seconds(3) .await?; } diff --git a/benchmarks/lib.ts b/benchmarks/lib.ts index 22cc1f75eb..4df4d4ca7d 100644 --- a/benchmarks/lib.ts +++ b/benchmarks/lib.ts @@ -2,7 +2,7 @@ import { sleep } from "https://deno.land/x/sleep@v1.2.1/mod.ts"; import * as windmill from "https://deno.land/x/windmill@v1.174.0/mod.ts"; import * as api from "https://deno.land/x/windmill@v1.174.0/windmill-api/index.ts"; -export const VERSION = "v1.700.2"; +export const VERSION = "v1.702.1"; export async function login(email: string, password: string): Promise { return await windmill.UserService.login({ diff --git a/cli/src/commands/app/app.ts b/cli/src/commands/app/app.ts index 2749c0269c..a6f85fde1c 100644 --- a/cli/src/commands/app/app.ts +++ b/cli/src/commands/app/app.ts @@ -20,12 +20,18 @@ import newCommand from "./new.ts"; import generateAgentsCommand from "./generate_agents.ts"; import { isVersionsGeq1585 } from "../sync/global.ts"; import type { PermissionedAsContext } from "../../core/permissioned_as.ts"; +import { applyExtraPermsDiff } from "../../core/extra_perms.ts"; export interface AppFile { value: any; public?: boolean; summary: string; policy: Policy; + // Mirrors granular ACLs on the app path. Omitted from app.yaml when no + // perms are set. The CLI applies diffs through /acls/add and /acls/remove + // (see applyExtraPermsDiff) — never through update_app — so a perm-only + // change never bumps the app version. + extra_perms?: Record; } const alreadySynced: string[] = []; @@ -168,21 +174,27 @@ export async function pushApp( // On create: backend applies folder defaults } + // extra_perms goes through /acls/* — strip from the body so a perms-only + // edit never bumps the app version (see applyExtraPermsDiff for details). + const { extra_perms: localPerms, ...localAppBody } = localApp as AppFile & { + extra_perms?: Record; + }; + if (app) { - if (isSuperset(localApp, app)) { + if (isSuperset(localAppBody, app)) { log.info(colors.green(`App ${remotePath} is up to date`)); - return; + } else { + log.info(colors.bold.yellow(`Updating app ${remotePath}...`)); + await wmill.updateApp({ + workspace, + path: remotePath, + requestBody: { + deployment_message: message, + ...localAppBody, + ...preserveFields, + }, + }); } - log.info(colors.bold.yellow(`Updating app ${remotePath}...`)); - await wmill.updateApp({ - workspace, - path: remotePath, - requestBody: { - deployment_message: message, - ...localApp, - ...preserveFields, - }, - }); } else { log.info(colors.yellow.bold("Creating new app...")); @@ -191,11 +203,23 @@ export async function pushApp( requestBody: { path: remotePath, deployment_message: message, - ...localApp, + ...localAppBody, ...preserveFields, }, }); } + + // Independent perms sync via /acls/* — self-contained log + non-fatal errors. + // No refetch: extra_perms is item-specific and folder perms are never merged + // onto item.extra_perms, and the body sent to update_app / create_app omits + // the field — so the value we already have from getAppByPath is authoritative. + await applyExtraPermsDiff( + workspace, + "app", + remotePath, + localPerms, + (app as any)?.extra_perms, + ); } export async function generatingPolicy( diff --git a/cli/src/commands/app/raw_apps.ts b/cli/src/commands/app/raw_apps.ts index 43c39ec9e4..5b9e880adc 100644 --- a/cli/src/commands/app/raw_apps.ts +++ b/cli/src/commands/app/raw_apps.ts @@ -19,6 +19,7 @@ import { createBundle, detectFrameworks } from "./bundle.ts"; import { APP_BACKEND_FOLDER } from "./app_metadata.ts"; import { writeIfChanged } from "../../utils/utils.ts"; import { yamlOptions } from "../sync/sync.ts"; +import { applyExtraPermsDiff } from "../../core/extra_perms.ts"; import { EXTENSION_TO_LANGUAGE, getLanguageFromExtension, @@ -35,6 +36,10 @@ export interface AppFile { datatable?: string; schema?: string; }; + // Mirrors granular ACLs on the raw_app path. Synced via /acls/* by + // applyExtraPermsDiff — never through update_app_raw — so a perm-only + // change never bumps the app version. Stripped from the yaml when empty. + extra_perms?: Record; } // Match siblings of a YAML metadata file case-insensitively. A buggy CLI @@ -431,35 +436,45 @@ export async function pushRawApp( value.data = localApp.data; } + // extra_perms is synced independently via /acls/* — strip from the + // up-to-date comparison so a perm-only edit doesn't trigger a rebuild + + // new app_version. The kind segment is "raw_app" so git-sync writes back + // to `.raw_app.json`, not `.app.json`. The backend granular_acls + // handler routes "raw_app" to the `app` table (where v2 raw apps actually + // live) while still dispatching DeployedObject::RawApp for git-sync. + const { extra_perms: localPerms, ...localAppNoPerms } = localApp as AppFile & { + extra_perms?: Record; + }; + if (app) { // Check both metadata/runnables AND files for changes // Files need separate comparison because isSuperset only checks if local keys exist in remote - const metadataUpToDate = isSuperset({ ...localApp, runnables }, app); + const metadataUpToDate = isSuperset({ ...localAppNoPerms, runnables }, app); const filesUpToDate = deepEqual(files, app.value?.files); if (metadataUpToDate && filesUpToDate) { log.info(colors.green(`App ${remotePath} is up to date`)); - return; - } - const { js, css } = await createBundleRaw(); - log.info(colors.bold.yellow(`Updating app ${remotePath}...`)); - await wmill.updateAppRaw({ - workspace, - path: remotePath, - formData: { - app: { - value, - path: remotePath, - summary: localApp.summary, - policy: appForPolicy.policy, - deployment_message: message, - ...(localApp.custom_path - ? { custom_path: localApp.custom_path } - : {}), + } else { + const { js, css } = await createBundleRaw(); + log.info(colors.bold.yellow(`Updating app ${remotePath}...`)); + await wmill.updateAppRaw({ + workspace, + path: remotePath, + formData: { + app: { + value, + path: remotePath, + summary: localApp.summary, + policy: appForPolicy.policy, + deployment_message: message, + ...(localApp.custom_path + ? { custom_path: localApp.custom_path } + : {}), + }, + js, + css, }, - js, - css, - }, - }); + }); + } } else { const { js, css } = await createBundleRaw(); await wmill.createAppRaw({ @@ -480,6 +495,16 @@ export async function pushRawApp( }, }); } + + // No refetch needed: folder perms are never merged into item.extra_perms, + // and the body sent to update_app_raw / create_app_raw omits the field. + await applyExtraPermsDiff( + workspace, + "raw_app", + remotePath, + localPerms, + (app as any)?.extra_perms, + ); } export async function generatingPolicy( diff --git a/cli/src/commands/flow/flow.ts b/cli/src/commands/flow/flow.ts index efd05c376a..7f437b8005 100644 --- a/cli/src/commands/flow/flow.ts +++ b/cli/src/commands/flow/flow.ts @@ -19,6 +19,7 @@ import { defaultFlowDefinition } from "../../../bootstrap/flow_bootstrap.ts"; import { SyncOptions, mergeConfigWithConfigFile } from "../../core/conf.ts"; import { FSFSElement, elementsToMap, ignoreF } from "../sync/sync.ts"; import { Flow } from "../../../gen/types.gen.ts"; +import { applyExtraPermsDiff } from "../../core/extra_perms.ts"; import type { PermissionedAsContext } from "../../core/permissioned_as.ts"; import { collectPathScriptPaths, @@ -41,6 +42,11 @@ export interface FlowFile { schema?: any; on_behalf_of_email?: string; has_on_behalf_of?: boolean; + // Mirrors granular ACLs on the flow path. Omitted from flow.yaml when no + // perms are set. The CLI applies diffs through /acls/add and /acls/remove + // (see applyExtraPermsDiff) — never through update_flow — so a perm-only + // change never bumps the flow version. + extra_perms?: Record; } function normalizeOptionalString(value: string | null | undefined): string | undefined { @@ -174,10 +180,14 @@ export async function pushFlow( await replaceInlineScripts([localFlow.value.preprocessor_module], fileReader, log, localPath, SEP, undefined, missingFiles); } if (missingFiles.length > 0) { - log.warn(colors.yellow( - `Warning: missing inline script file(s): ${missingFiles.join(", ")}. ` + - `The flow will be pushed with unresolved !inline references.` - )); + // Hard-fail rather than push the literal `!inline path` text as + // rawscript.content. That string would be persisted in flow_version.value + // and round-trip as the script body on the next pull, overwriting the + // user's local handler with the directive — see GIT-871 / #9140. + throw new Error( + `Cannot push flow ${remotePath}: missing inline script file(s): ${missingFiles.join(", ")}. ` + + `Either restore the file(s) or remove the !inline reference(s) from flow.yaml before pushing.` + ); } const hasOnBehalfOf = (localFlow as any).has_on_behalf_of ?? !!localFlow.on_behalf_of_email; @@ -193,22 +203,30 @@ export async function pushFlow( // On create: backend applies folder defaults — no client-side resolution needed } + // extra_perms is synced independently via /acls/* (see applyExtraPermsDiff) + // so a perm-only edit never bumps the flow version. Strip the field from the + // body that goes to update_flow / create_flow and treat it as a separate + // step both for the up-to-date short-circuit and after the deploy. + const { extra_perms: localPerms, ...localFlowBody } = localFlow as FlowFile & { + extra_perms?: Record; + }; + if (flow) { - if (isSuperset(localFlow, flow)) { + if (isSuperset(localFlowBody, flow)) { log.info(colors.green(`Flow ${remotePath} is up to date`)); - return; - } - log.info(colors.bold.yellow(`Updating flow ${remotePath}...`)); - await wmill.updateFlow({ - workspace: workspace, - path: remotePath.replaceAll(SEP, "/"), - requestBody: { + } else { + log.info(colors.bold.yellow(`Updating flow ${remotePath}...`)); + await wmill.updateFlow({ + workspace: workspace, path: remotePath.replaceAll(SEP, "/"), - deployment_message: message, - ...localFlow, - ...preserveFields, - }, - }); + requestBody: { + path: remotePath.replaceAll(SEP, "/"), + deployment_message: message, + ...localFlowBody, + ...preserveFields, + }, + }); + } } else { log.info(colors.bold.yellow("Creating new flow...")); try { @@ -217,7 +235,7 @@ export async function pushFlow( requestBody: { path: remotePath.replaceAll(SEP, "/"), deployment_message: message, - ...localFlow, + ...localFlowBody, ...preserveFields, }, }); @@ -228,6 +246,22 @@ export async function pushFlow( ); } } + + // Independent of whether the flow body changed, sync extra_perms via /acls/*. + // Self-contained log line + non-fatal failures. + // + // No refetch is needed: extra_perms is item-specific and additive on top of + // folder perms — folder perms are never merged onto item.extra_perms. And + // since the request body sent to update_flow / create_flow doesn't carry + // extra_perms, the value we read in the initial getFlowByPath above is + // also the post-write value (a no-op deploy can't drift it). + await applyExtraPermsDiff( + workspace, + "flow", + remotePath.replaceAll(SEP, "/"), + localPerms, + (flow as any)?.extra_perms, + ); } type Options = GlobalOptions; diff --git a/cli/src/commands/flow/flow_metadata.ts b/cli/src/commands/flow/flow_metadata.ts index f33ef07bcc..5c3ca578c7 100644 --- a/cli/src/commands/flow/flow_metadata.ts +++ b/cli/src/commands/flow/flow_metadata.ts @@ -266,19 +266,32 @@ export async function generateFlowLockInternal( return tree.isStale(treePath); }) : changedScripts; + const missingFiles: string[] = []; await replaceInlineScripts( flowValue.value.modules, fileReader, log, folder + SEP!, SEP, - locksToRemove + locksToRemove, + missingFiles ); if (flowValue.value.failure_module) { - await replaceInlineScripts([flowValue.value.failure_module], fileReader, log, folder + SEP!, SEP, locksToRemove); + await replaceInlineScripts([flowValue.value.failure_module], fileReader, log, folder + SEP!, SEP, locksToRemove, missingFiles); } if (flowValue.value.preprocessor_module) { - await replaceInlineScripts([flowValue.value.preprocessor_module], fileReader, log, folder + SEP!, SEP, locksToRemove); + await replaceInlineScripts([flowValue.value.preprocessor_module], fileReader, log, folder + SEP!, SEP, locksToRemove, missingFiles); + } + if (missingFiles.length > 0) { + // Abort before updateFlow rather than push the literal `!inline path` + // string as rawscript.content (GIT-871 / #9140). Note: at this point + // replaceInlineScripts has already mutated `flowValue.value` in place + // for the modules that *did* resolve. All current callers re-throw on + // this error; do not catch and reuse `flowValue` without re-parsing. + throw new Error( + `Cannot regenerate lock for flow ${remote_path}: missing inline script file(s): ${missingFiles.join(", ")}. ` + + `Either restore the file(s) or remove the !inline reference(s) from flow.yaml before retrying.` + ); } //removeChangedLocks @@ -304,18 +317,23 @@ export async function generateFlowLockInternal( const lockAssigner = newPathAssigner(opts.defaultTs ?? "bun", { skipInlineScriptSuffix: getNonDottedPaths(), }); + // flowValue.value here is the backend's response from updateFlow, so a + // rawscript whose content is `!inline ...` is corruption (GIT-871) — fail + // fast rather than writing the literal directive back to a script file. + const extractOpts = { skipInlineScriptSuffix: getNonDottedPaths(), failOnInlineDirective: true }; const inlineScripts = extractInlineScriptsForFlows( flowValue.value.modules, currentMapping, SEP, opts.defaultTs, - lockAssigner + lockAssigner, + extractOpts ); if (flowValue.value.failure_module) { - inlineScripts.push(...extractInlineScriptsForFlows([flowValue.value.failure_module], currentMapping, SEP, opts.defaultTs, lockAssigner)); + inlineScripts.push(...extractInlineScriptsForFlows([flowValue.value.failure_module], currentMapping, SEP, opts.defaultTs, lockAssigner, extractOpts)); } if (flowValue.value.preprocessor_module) { - inlineScripts.push(...extractInlineScriptsForFlows([flowValue.value.preprocessor_module], currentMapping, SEP, opts.defaultTs, lockAssigner)); + inlineScripts.push(...extractInlineScriptsForFlows([flowValue.value.preprocessor_module], currentMapping, SEP, opts.defaultTs, lockAssigner, extractOpts)); } inlineScripts.forEach((s) => { writeIfChanged(process.cwd() + SEP + folder + SEP + s.path, s.content); diff --git a/cli/src/commands/script/script.ts b/cli/src/commands/script/script.ts index f1e605d8ff..b9d9c6ee60 100644 --- a/cli/src/commands/script/script.ts +++ b/cli/src/commands/script/script.ts @@ -2,6 +2,7 @@ import { GlobalOptions } from "../../types.ts"; import { requireLogin } from "../../core/auth.ts"; import { resolveWorkspace, validatePath } from "../../core/context.ts"; import type { PermissionedAsContext } from "../../core/permissioned_as.ts"; +import { applyExtraPermsDiff } from "../../core/extra_perms.ts"; import { writeFile, stat, mkdir } from "node:fs/promises"; import { Buffer } from "node:buffer"; import { colors } from "@cliffy/ansi/colors"; @@ -83,6 +84,11 @@ export interface ScriptFile { is_template?: boolean; lock?: Array; kind?: "script" | "failure" | "trigger" | "command" | "approval"; + // Mirrors granular ACLs on the script path. Omitted from .script.yaml when + // no perms are set. The CLI applies diffs through /acls/add and /acls/remove + // (see applyExtraPermsDiff) — never through create_script — so a perm-only + // change never bumps the script hash/version. + extra_perms?: Record; } /** @@ -542,6 +548,15 @@ export async function handleFile( deepEqual(modules ?? null, remote.modules ?? null)) ) { log.info(colors.green(`Script ${remotePath} is up to date`)); + // Even when the body is unchanged, perms may still drift — sync them + // independently before returning. + await applyExtraPermsDiff( + workspaceId, + "script", + remotePath.replaceAll(SEP, "/"), + (typed as any)?.extra_perms, + (remote as any)?.extra_perms, + ); return true; } } @@ -581,6 +596,27 @@ export async function handleFile( ) ); } + + // Sync granular ACLs as an independent step — perm-only edits never reach + // create_script (which would bump the script hash) and instead route + // through /acls/* via applyExtraPermsDiff. + // + // No refetch is needed: + // - folder perms are additive at auth time, never merged onto item rows; + // - the body sent to create_script doesn't carry extra_perms, so a fresh + // deploy of an existing path inherits the previous version's perms + // unchanged. The diff against `remote` (captured before the deploy) + // therefore matches what `wmill acl remove` would do — and the granular + // ACL endpoint updates every matching row, so the inheritance on the + // new version doesn't leave ghost entries. + await applyExtraPermsDiff( + workspaceId, + "script", + remotePath.replaceAll(SEP, "/"), + (typed as any)?.extra_perms, + (remote as any)?.extra_perms, + ); + return true; } return false; diff --git a/cli/src/commands/sync/pull.ts b/cli/src/commands/sync/pull.ts index b89d81b91c..adfd486294 100644 --- a/cli/src/commands/sync/pull.ts +++ b/cli/src/commands/sync/pull.ts @@ -98,12 +98,16 @@ export async function downloadZip( } const includeWorkspaceDependenciesValue = !(skipWorkspaceDependencies ?? false); + // `preserve_extra_perms=true` opts the tarball into surfacing granular ACLs + // on flow / script / app rows. Default-off on the server protects cross- + // workspace tarball imports from carrying ACLs that reference identities + // missing in the target workspace; the CLI sync flow explicitly wants them. const baseParams = `&plain_secret=${plainSecrets ?? false }&skip_variables=${skipVariables ?? false}&skip_resources=${skipResources ?? false }&skip_secrets=${skipSecrets ?? false}&include_schedules=${includeSchedules ?? false }&include_triggers=${includeTriggers ?? false}&include_users=${includeUsers ?? false }&include_groups=${includeGroups ?? false}&include_settings=${includeSettings ?? false - }&include_key=${includeKey ?? false}&include_workspace_dependencies=${includeWorkspaceDependenciesValue}&default_ts=${defaultTs ?? "bun"}&skip_resource_types=${skipResourceTypes ?? false}&settings_version=v2`; + }&include_key=${includeKey ?? false}&include_workspace_dependencies=${includeWorkspaceDependenciesValue}&default_ts=${defaultTs ?? "bun"}&skip_resource_types=${skipResourceTypes ?? false}&settings_version=v2&preserve_extra_perms=true`; const baseUrl = workspace.remote + "api/w/" + workspace.workspaceId + "/workspaces/tarball?"; diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 6cd3545c5d..215a0d3120 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -950,7 +950,7 @@ function ZipFSElement( SEP, defaultTs, assigner, - { skipInlineScriptSuffix: getNonDottedPaths() }, + { skipInlineScriptSuffix: getNonDottedPaths(), failOnInlineDirective: true }, ); if (flow.value.failure_module) { inlineScripts.push(...extractInlineScriptsForFlows( @@ -959,7 +959,7 @@ function ZipFSElement( SEP, defaultTs, assigner, - { skipInlineScriptSuffix: getNonDottedPaths() }, + { skipInlineScriptSuffix: getNonDottedPaths(), failOnInlineDirective: true }, )); } if (flow.value.preprocessor_module) { @@ -969,7 +969,7 @@ function ZipFSElement( SEP, defaultTs, assigner, - { skipInlineScriptSuffix: getNonDottedPaths() }, + { skipInlineScriptSuffix: getNonDottedPaths(), failOnInlineDirective: true }, )); } } catch (error) { diff --git a/cli/src/core/extra_perms.ts b/cli/src/core/extra_perms.ts new file mode 100644 index 0000000000..7b50d34c8d --- /dev/null +++ b/cli/src/core/extra_perms.ts @@ -0,0 +1,192 @@ +import * as wmill from "../../gen/services.gen.ts"; +import * as log from "./log.ts"; +import { colors } from "@cliffy/ansi/colors"; +import type { AddGranularAclsData } from "../../gen/types.gen.ts"; + +export type ExtraPermsKind = AddGranularAclsData["kind"]; + +type PermsMap = Record; + +type Normalized = { + perms: PermsMap; + /** Keys with non-boolean values: never revoked, never granted — treat as "no opinion". */ + invalidOwners: Set; + /** Top-level value is not a plain object (array, primitive, etc.). The + * entire map is rejected and the caller must treat it like "no opinion" + * to avoid silently revoking every remote ACL. */ + malformedTop: boolean; +}; + +function normalize(value: unknown, source: string): Normalized { + const perms: PermsMap = {}; + const invalidOwners = new Set(); + + if ( + value === null || + value === undefined || + typeof value !== "object" || + Array.isArray(value) + ) { + if (value !== undefined && value !== null) { + log.error( + colors.red( + `extra_perms: ${source} is not a {owner: boolean} map — skipping ACL sync to avoid clobbering remote ACLs`, + ), + ); + return { perms, invalidOwners, malformedTop: true }; + } + return { perms, invalidOwners, malformedTop: false }; + } + + const invalidList: string[] = []; + for (const [k, v] of Object.entries(value as Record)) { + if (typeof v === "boolean") { + perms[k] = v; + } else { + invalidOwners.add(k); + invalidList.push(k); + } + } + if (invalidList.length > 0) { + log.error( + colors.red( + `extra_perms: ${invalidList.length} invalid entry/entries in ${source} (non-boolean value, treating as "no opinion"): ${invalidList.join(", ")}`, + ), + ); + } + return { perms, invalidOwners, malformedTop: false }; +} + +function formatError(e: unknown): string { + if (!e || typeof e !== "object") return String(e); + const anyE = e as { body?: unknown; message?: unknown }; + if (anyE.body !== undefined) { + if (typeof anyE.body === "string") return anyE.body; + try { + return JSON.stringify(anyE.body); + } catch { + // fall through + } + } + if (typeof anyE.message === "string") return anyE.message; + try { + return JSON.stringify(e); + } catch { + return String(e); + } +} + +/** + * Apply the diff between `local` and `remote` granular ACL maps as a sequence + * of `/acls/add` and `/acls/remove` calls. Used by every CLI push path so a + * yaml change that *only* touches `extra_perms` never bumps the script/flow/app + * version — perm mutations route through the dedicated granular-ACL endpoints + * exactly as if the user had clicked through the UI. + * + * **`local === undefined` means "no opinion".** If the yaml does not carry an + * `extra_perms` field at all, this function is a no-op — the remote ACLs are + * left untouched. This is what prevents a stale local checkout from racing + * a concurrent UI grant: only users who explicitly track perms in source (by + * writing `extra_perms:` in the yaml, even as `{}`) get destructive sync. + * + * The function is intentionally independent of the surrounding push logic: + * it has its own log lines and its own non-fatal failure mode. Each grant / + * revoke is logged as a separate line on success; failures are logged in red + * but never throw — a stale yaml referencing a deleted user/group surfaces as + * a red error, not a hard error that would block the surrounding deploy. + * + * @returns number of /acls/* calls actually issued (0 means perms in sync, or + * the local yaml had no `extra_perms` field). + */ +export async function applyExtraPermsDiff( + workspace: string, + kind: ExtraPermsKind, + path: string, + local: unknown, + remote: unknown, +): Promise { + // Absent local field = "no opinion" — never call /acls/* in this case. + // Crucially, this protects users who don't track ACLs in source from having + // their UI-managed perms silently revoked by a stale CLI push. + if (local === undefined || local === null) { + return 0; + } + + const localN = normalize(local, "local yaml"); + // Top-level malformed (array, primitive, etc.) → treat as "no opinion" so a + // typo in yaml can never silently revoke every remote ACL. + if (localN.malformedTop) { + return 0; + } + const remoteN = normalize(remote, "remote response"); + + const localPerms = localN.perms; + const remotePerms = remoteN.perms; + + const toGrant: Array<[string, boolean]> = []; + for (const [owner, write] of Object.entries(localPerms)) { + if (!(owner in remotePerms) || remotePerms[owner] !== write) { + toGrant.push([owner, write]); + } + } + + // Owners with a malformed value in local yaml are treated as "no opinion": + // they are excluded from the revoke set so a typo (`g/devs: "write"`) never + // silently strips an existing ACL. + const toRevoke: string[] = Object.keys(remotePerms).filter( + (owner) => + !(owner in localPerms) && !localN.invalidOwners.has(owner), + ); + + if (toGrant.length === 0 && toRevoke.length === 0) { + return 0; + } + + let calls = 0; + for (const [owner, write] of toGrant) { + const access = write ? "write" : "read"; + try { + await wmill.addGranularAcls({ + workspace, + kind, + path, + requestBody: { owner, write }, + }); + log.info( + colors.green( + ` extra_perms: granted ${access} to ${owner} on ${kind}/${path}`, + ), + ); + calls += 1; + } catch (e: any) { + log.error( + colors.red( + ` extra_perms: failed to grant ${access} to ${owner} on ${kind}/${path}: ${formatError(e)}`, + ), + ); + } + } + + for (const owner of toRevoke) { + try { + await wmill.removeGranularAcls({ + workspace, + kind, + path, + requestBody: { owner }, + }); + log.info( + colors.green(` extra_perms: revoked ${owner} on ${kind}/${path}`), + ); + calls += 1; + } catch (e: any) { + log.error( + colors.red( + ` extra_perms: failed to revoke ${owner} on ${kind}/${path}: ${formatError(e)}`, + ), + ); + } + } + + return calls; +} diff --git a/cli/src/main.ts b/cli/src/main.ts index a755c16901..24772ad250 100755 --- a/cli/src/main.ts +++ b/cli/src/main.ts @@ -79,7 +79,7 @@ export { token, }; -export const VERSION = "1.700.2"; +export const VERSION = "1.702.1"; // Re-exported from constants.ts to maintain backwards compatibility export { WM_FORK_PREFIX } from "./core/constants.ts"; diff --git a/cli/test/inline_scripts_failure_preprocessor_unit.test.ts b/cli/test/inline_scripts_failure_preprocessor_unit.test.ts index 78af37a02e..a5d1298f15 100644 --- a/cli/test/inline_scripts_failure_preprocessor_unit.test.ts +++ b/cli/test/inline_scripts_failure_preprocessor_unit.test.ts @@ -574,3 +574,68 @@ describe("extractInlineScripts with mapping preserves file paths", () => { expect(lockScript!.path).toBe("my.inline_script.lock"); }); }); + +// --------------------------------------------------------------------------- +// failOnInlineDirective option (GIT-871 / #9140) +// --------------------------------------------------------------------------- + +describe("failOnInlineDirective option", () => { + test("default behavior: yaml-parsed module with !inline content extracts without throwing", () => { + // Simulates flow_metadata / dev callers: yaml-parsed local flow whose + // rawscript.content is the literal `!inline foo.ts` directive (the + // legitimate on-disk shape after extraction). + const mod = makeRawscriptModule("a", "!inline a.inline_script.ts", "bun"); + expect(() => + extractInlineScripts([mod], {}, "/", "bun"), + ).not.toThrow(); + }); + + test("yaml-parsed !inline content round-trips as the script's body", () => { + const mod = makeRawscriptModule("a", "!inline a.inline_script.ts", "bun"); + const scripts = extractInlineScripts([mod], {}, "/", "bun"); + const script = scripts.find((s) => !s.is_lock); + expect(script).toBeDefined(); + expect(script!.content).toBe("!inline a.inline_script.ts"); + }); + + test("opt-in: failOnInlineDirective=true throws on !inline content", () => { + // Simulates the sync-pull call site: rawscript came from the backend's + // flow_version.value, so `!inline ...` content means the row is corrupt. + const mod = makeRawscriptModule("failure", "!inline Handle_error.ts", "bun"); + expect(() => + extractInlineScripts([mod], {}, "/", "bun", undefined, { + failOnInlineDirective: true, + }), + ).toThrow(/corrupted inline script/); + }); + + test("opt-in: real script content still extracts cleanly", () => { + const mod = makeRawscriptModule( + "failure", + 'export function main() { return 1; }', + "bun", + ); + expect(() => + extractInlineScripts([mod], {}, "/", "bun", undefined, { + failOnInlineDirective: true, + }), + ).not.toThrow(); + }); + + test("opt-in: throws for nested rawscript inside branchall", () => { + const inner = makeRawscriptModule("inner", "!inline poisoned.ts", "bun"); + const outer: FlowModule = { + id: "branch", + value: { + type: "branchall" as const, + branches: [{ summary: "b1", expr: "true", modules: [inner], skip_failure: false, parallel: false }], + parallel: false, + }, + }; + expect(() => + extractInlineScripts([outer], {}, "/", "bun", undefined, { + failOnInlineDirective: true, + }), + ).toThrow(/corrupted inline script/); + }); +}); diff --git a/cli/test/sync_pull_push.test.ts b/cli/test/sync_pull_push.test.ts index e31c4e1084..4b5b8e5389 100644 --- a/cli/test/sync_pull_push.test.ts +++ b/cli/test/sync_pull_push.test.ts @@ -111,6 +111,10 @@ kind: script function createFlowFixture(name: string): Record { const flowSuffix = getFolderSuffix("flow"); const metadataFile = getMetadataFileName("flow", "yaml"); + // !inline paths are resolved relative to the flow folder (see + // pushFlow's fileReader in cli/src/commands/flow/flow.ts), so the + // path inside the directive must NOT include the flow folder prefix. + const scriptFile = "a.ts"; return { metadata: { @@ -122,7 +126,7 @@ value: - id: a value: type: rawscript - content: "!inline ${name}${flowSuffix}/a.ts" + content: "!inline ${scriptFile}" language: bun input_transforms: {} schema: @@ -133,7 +137,7 @@ schema: `, }, inlineScript: { - path: `${name}${flowSuffix}/a.ts`, + path: `${name}${flowSuffix}/${scriptFile}`, content: `export async function main() {\n return "Hello from flow ${name}";\n}`, }, }; diff --git a/cli/windmill-utils-internal/src/inline-scripts/extractor.ts b/cli/windmill-utils-internal/src/inline-scripts/extractor.ts index e472372a99..f556b32c57 100644 --- a/cli/windmill-utils-internal/src/inline-scripts/extractor.ts +++ b/cli/windmill-utils-internal/src/inline-scripts/extractor.ts @@ -20,13 +20,28 @@ function extractRawscriptInline( rawscript: RawScript, mapping: Record, separator: string, - assigner: PathAssigner + assigner: PathAssigner, + failOnInlineDirective: boolean ): InlineScript[] { const [basePath, ext] = assigner.assignPath(summary ?? id, rawscript.language); const mappedPath = mapping[id]; const path = mappedPath ?? basePath + ext; const language = rawscript.language; const content = rawscript.content; + // Opt-in defensive guard: when extracting from backend-shaped data (i.e. + // sync pull), a rawscript whose content is itself an `!inline ...` directive + // means the backend was poisoned by a prior push that sent the unresolved + // directive as the script body (GIT-871 / #9140). Refuse to write it back + // to disk. Off by default because callers that operate on YAML-parsed local + // flows (flow_metadata, dev) legitimately see `!inline foo.ts` as content. + if (failOnInlineDirective && typeof content === "string" && content.startsWith("!inline ")) { + throw new Error( + `Refusing to extract corrupted inline script for module '${id}': ` + + `rawscript.content is the literal string \`${content.split("\n")[0]}\` ` + + `instead of script source. The backend's flow_version.value is corrupt — ` + + `re-push from a known-good local copy to repair it.` + ); + } const r = [{ path: path, content: content, language, is_lock: false}]; rawscript.content = "!inline " + path.replaceAll(separator, "/"); const lock = rawscript.lock; @@ -50,6 +65,15 @@ function extractRawscriptInline( export interface ExtractInlineScriptsOptions { /** When true, skip the .inline_script. suffix in file names */ skipInlineScriptSuffix?: boolean; + /** + * When true, throw if a `rawscript.content` is itself an `!inline ...` + * directive. Set this only at the sync-pull call site, where the input + * comes from the backend's `flow_version.value` and `!inline ...` content + * means the row is corrupt (GIT-871 / #9140). Leave off for callers that + * pass YAML-parsed local flows — the directive is the legitimate on-disk + * shape there. + */ + failOnInlineDirective?: boolean; } /** @@ -74,6 +98,7 @@ export function extractInlineScripts( ): InlineScript[] { // Create pathAssigner only if not provided (top-level call), but reuse it for nested calls const assigner = pathAssigner ?? newPathAssigner(defaultTs ?? "bun", { skipInlineScriptSuffix: options?.skipInlineScriptSuffix }); + const failOnInlineDirective = options?.failOnInlineDirective ?? false; return modules.flatMap((m) => { if (m.value.type == "rawscript") { @@ -83,7 +108,8 @@ export function extractInlineScripts( m.value, mapping, separator, - assigner + assigner, + failOnInlineDirective ); } else if (m.value.type == "forloopflow") { return extractInlineScripts( @@ -91,11 +117,12 @@ export function extractInlineScripts( mapping, separator, defaultTs, - assigner + assigner, + options ); } else if (m.value.type == "branchall") { return m.value.branches.flatMap((b) => - extractInlineScripts(b.modules, mapping, separator, defaultTs, assigner) + extractInlineScripts(b.modules, mapping, separator, defaultTs, assigner, options) ); } else if (m.value.type == "whileloopflow") { return extractInlineScripts( @@ -103,7 +130,8 @@ export function extractInlineScripts( mapping, separator, defaultTs, - assigner + assigner, + options ); } else if (m.value.type == "branchone") { return [ @@ -113,7 +141,8 @@ export function extractInlineScripts( mapping, separator, defaultTs, - assigner + assigner, + options ) ), ...extractInlineScripts( @@ -121,7 +150,8 @@ export function extractInlineScripts( mapping, separator, defaultTs, - assigner + assigner, + options ), ]; } else if (m.value.type == "aiagent") { @@ -138,7 +168,8 @@ export function extractInlineScripts( toolValue, mapping, separator, - assigner + assigner, + failOnInlineDirective ); }); } else { diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 2483fa4d97..efb4fd9529 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1,12 +1,12 @@ { "name": "windmill-components", - "version": "1.700.2", + "version": "1.702.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "windmill-components", - "version": "1.700.2", + "version": "1.702.1", "hasInstallScript": true, "license": "AGPL-3.0", "dependencies": { diff --git a/frontend/package.json b/frontend/package.json index 32bdc019f0..96b2bf4953 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "windmill-components", - "version": "1.700.2", + "version": "1.702.1", "scripts": { "dev": "vite dev", "build": "vite build", diff --git a/frontend/src/lib/components/DynamicInput.svelte b/frontend/src/lib/components/DynamicInput.svelte index 692272d50e..f4e2ba9c75 100644 --- a/frontend/src/lib/components/DynamicInput.svelte +++ b/frontend/src/lib/components/DynamicInput.svelte @@ -25,6 +25,7 @@ import { type DynamicInput } from '$lib/utils' import { deepEqual } from 'fast-equals' import { untrack } from 'svelte' + import { getHelperEntrypointArgs } from '$lib/infer' interface Props { value?: any @@ -48,7 +49,9 @@ }) let resultJobLoader: JobLoader | undefined = $state() - let _items = usePromise(getItemsFromOptions, { clearValueOnRefresh: false }) + // loadInit:false — the $effect below owns the first refresh once + // resultJobLoader is bound; without this the promise is kicked off twice. + let _items = usePromise(getItemsFromOptions, { clearValueOnRefresh: false, loadInit: false }) let items = $derived(_items.value) let filterText: string = $state('') @@ -125,9 +128,43 @@ }, 1000) }) + // Parameter names declared by the helper function. When known, we restrict + // the change-detection to only those keys so typing in unrelated form fields + // no longer retriggers the dynselect job. `undefined` means we couldn't + // determine the signature → fall back to a full-args comparison. + let helperParams = $state | undefined>(undefined) + + $effect(() => { + const script = helperScript + const ep = entrypoint + if (!script) { + helperParams = undefined + return + } + let cancelled = false + void getHelperEntrypointArgs(script, ep || undefined).then((params) => { + if (!cancelled) helperParams = params + }) + return () => { + cancelled = true + } + }) + + function filterArgs(args: Record | undefined) { + if (!args || !helperParams) return args + const filtered: Record = {} + for (const k of helperParams) { + if (k in args) filtered[k] = args[k] + } + return filtered + } + $effect(() => { ;[filterText, entrypoint, helperScript] - if (resultJobLoader && (open || neverLoaded || !deepEqual(lastArgs, nargs))) { + if ( + resultJobLoader && + (open || neverLoaded || !deepEqual(filterArgs(lastArgs), filterArgs(nargs))) + ) { neverLoaded = false lastArgs = $state.snapshot(otherArgs) _items.refresh() diff --git a/frontend/src/lib/components/InstanceSetting.svelte b/frontend/src/lib/components/InstanceSetting.svelte index 4964fb9c1d..703b833a2f 100644 --- a/frontend/src/lib/components/InstanceSetting.svelte +++ b/frontend/src/lib/components/InstanceSetting.svelte @@ -699,6 +699,31 @@ {/each} +
+ + +

+ Comma-separated host patterns that job HTTP clients should bypass the tracing + proxy for — those hosts will not be traced. Use this for clients that pin their + own CA (kubectl, helm, terraform providers, aws cli for EKS, etc.) which would + otherwise fail with x509: certificate signed by unknown authority. + Independent of the worker's own NO_PROXY env, which governs the proxy's + upstream relay (e.g. through a corporate proxy). +

+
{/if} {:else if setting.fieldType == 'object_store_config'} diff --git a/frontend/src/lib/components/RunsPage.svelte b/frontend/src/lib/components/RunsPage.svelte index 86b7588099..360eb9276d 100644 --- a/frontend/src/lib/components/RunsPage.svelte +++ b/frontend/src/lib/components/RunsPage.svelte @@ -10,7 +10,7 @@ } from '$lib/gen' import { sendUserToast } from '$lib/toast' - import { userStore, workspaceStore, userWorkspaces, superadmin } from '$lib/stores' + import { userStore, workspaceStore, userWorkspaces, superadmin, devopsRole } from '$lib/stores' import { Button, ButtonType, @@ -82,7 +82,7 @@ usernames, folders, jobTriggerKinds, - isSuperAdmin: !!$superadmin, + isSuperAdminOrDevops: !!$superadmin || !!$devopsRole, isAdminsWorkspace: $workspaceStore === 'admins' }) ) @@ -750,7 +750,7 @@ )} schema={runsFilterSearchbarSchema} presets={buildRunsFilterPresets({ - isSuperadmin: !!$superadmin, + isSuperAdminOrDevops: !!$superadmin || !!$devopsRole, isAdminsWorkspace: $workspaceStore === 'admins' })} bind:value={filters.val} diff --git a/frontend/src/lib/components/SuperadminSettingsInner.svelte b/frontend/src/lib/components/SuperadminSettingsInner.svelte index c652293fc0..a9824de414 100644 --- a/frontend/src/lib/components/SuperadminSettingsInner.svelte +++ b/frontend/src/lib/components/SuperadminSettingsInner.svelte @@ -21,12 +21,15 @@ import { ArrowRightLeft, Ban, + Bot, CheckCircle2, ExternalLink, Pencil, UserMinus, UserPlus } from 'lucide-svelte' + import Badge from './common/badge/Badge.svelte' + import Tooltip from './Tooltip.svelte' import DropdownV2 from './DropdownV2.svelte' import Popover from './meltComponents/Popover.svelte' import ConfirmationModal from './common/confirmationModal/ConfirmationModal.svelte' @@ -219,7 +222,16 @@ {filter} items={users} bind:filteredItems={filteredUsers} - f={(x) => x.email + ' ' + x.name + ' ' + x.company} + f={(x) => + (x.email ?? '') + + ' ' + + (x.name ?? '') + + ' ' + + (x.company ?? '') + + ' ' + + (x.username ?? '') + + ' ' + + (x.workspace_id ?? '')} />
@@ -393,7 +405,8 @@ {#if filteredUsers && users} - {#each filteredUsers.slice(0, nbDisplayed) as { email, super_admin, devops, login_type, name, username, operator_only, role_source, disabled }, i (email)} + {#each filteredUsers.slice(0, nbDisplayed) as { email, super_admin, devops, login_type, name, username, operator_only, role_source, disabled, workspace_id }, i (email + '::' + (workspace_id ?? ''))} + {@const isServiceAccount = login_type === 'service_account'}
- {email} + {#if isServiceAccount} + + {email} + {:else} + {email} + {/if} + {#if workspace_id} + + {truncate(workspace_id, 20)} + + {/if} {#if disabled} {/if} -
- {#key `${super_admin}_${devops}_${role_source}`} - { - if (email == $userStore?.email) { - sendUserToast('You cannot demote yourself', true) + {#if isServiceAccount} +
+ + Operator + + Service accounts are always operators. +
+ {:else} +
+ {#key `${super_admin}_${devops}_${role_source}`} + { + if (email == $userStore?.email) { + sendUserToast('You cannot demote yourself', true) + listUsers(activeOnly) + return + } + + let role = e.detail + + if (role === 'super_admin') { + await UserService.globalUserUpdate({ + email, + requestBody: { + is_super_admin: true, + is_devops: false + } + }) + } + if (role === 'devops') { + await UserService.globalUserUpdate({ + email, + requestBody: { + is_super_admin: false, + is_devops: true + } + }) + } + if (role === 'user') { + await UserService.globalUserUpdate({ + email, + requestBody: { + is_super_admin: false, + is_devops: false + } + }) + } + sendUserToast('User updated') listUsers(activeOnly) - return - } - - let role = e.detail - - if (role === 'super_admin') { - await UserService.globalUserUpdate({ - email, - requestBody: { - is_super_admin: true, - is_devops: false - } - }) - } - if (role === 'devops') { - await UserService.globalUserUpdate({ - email, - requestBody: { - is_super_admin: false, - is_devops: true - } - }) - } - if (role === 'user') { - await UserService.globalUserUpdate({ - email, - requestBody: { - is_super_admin: false, - is_devops: false - } - }) - } - sendUserToast('User updated') - listUsers(activeOnly) - }} - > - {#snippet children({ item })} - - - - {/snippet} - - {/key} - {#if role_source === 'instance_group' && (super_admin || devops)} - closeDrawer?.()} - > - Set by instance group - - {/if} -
+ }} + > + {#snippet children({ item })} + + + + {/snippet} + + {/key} + {#if role_source === 'instance_group' && (super_admin || devops)} + closeDrawer?.()} + > + Set by instance group + + {/if} +
+ {/if}
- - { - const btn = editWrappers[email]?.querySelector( - '[aria-label="Popup button"]' - ) - if (btn instanceof HTMLElement) btn.click() - } - }, - { - displayName: disabled ? 'Enable' : 'Disable', - icon: disabled ? CheckCircle2 : Ban, - action: () => { - if (!disabled) { - disableUserEmail = email - disableConfirmedCallback = async () => { - try { - await UserService.globalUserUpdate({ - email, - requestBody: { disabled: true } - }) - sendUserToast('User disabled') - listUsers(activeOnly) - } catch (e) { - sendUserToast('Failed to disable user', true) + {#if isServiceAccount} + {#if workspace_id} + Manage in workspace + {/if} + {:else} + + { + const btn = editWrappers[email]?.querySelector( + '[aria-label="Popup button"]' + ) + if (btn instanceof HTMLElement) btn.click() + } + }, + { + displayName: disabled ? 'Enable' : 'Disable', + icon: disabled ? CheckCircle2 : Ban, + action: () => { + if (!disabled) { + disableUserEmail = email + disableConfirmedCallback = async () => { + try { + await UserService.globalUserUpdate({ + email, + requestBody: { disabled: true } + }) + sendUserToast('User disabled') + listUsers(activeOnly) + } catch (e) { + sendUserToast('Failed to disable user', true) + } } + } else { + UserService.globalUserUpdate({ + email, + requestBody: { disabled: false } + }) + .then(() => { + sendUserToast('User enabled') + listUsers(activeOnly) + }) + .catch(() => { + sendUserToast('Failed to enable user', true) + }) } - } else { - UserService.globalUserUpdate({ - email, - requestBody: { disabled: false } - }) - .then(() => { - sendUserToast('User enabled') - listUsers(activeOnly) - }) - .catch(() => { - sendUserToast('Failed to enable user', true) - }) + } + }, + { + displayName: 'Reassign', + icon: ArrowRightLeft, + action: () => { + offboardingEmail = email + offboardingReassignOnly = true + } + }, + { + displayName: 'Remove', + icon: UserMinus, + type: 'delete', + action: () => { + offboardingEmail = email + offboardingReassignOnly = false } } - }, - { - displayName: 'Reassign', - icon: ArrowRightLeft, - action: () => { - offboardingEmail = email - offboardingReassignOnly = true - } - }, - { - displayName: 'Remove', - icon: UserMinus, - type: 'delete', - action: () => { - offboardingEmail = email - offboardingReassignOnly = false - } - } - ]} - /> + ]} + /> + {/if}
diff --git a/frontend/src/lib/components/copilot/chat/global/core.test.ts b/frontend/src/lib/components/copilot/chat/global/core.test.ts index 2664c1495d..2732db4dac 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.test.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.test.ts @@ -113,19 +113,17 @@ describe('global AI tools', () => { await callGlobalTool('write_flow', { path: 'f/flows/empty-module', summary: 'Flow with empty module', - value: JSON.stringify({ - modules: [ - { - id: 'empty_step', - value: { - type: 'rawscript', - language: 'bun', - content: '', - input_transforms: {} - } + modules: JSON.stringify([ + { + id: 'empty_step', + value: { + type: 'rawscript', + language: 'bun', + content: '', + input_transforms: {} } - ] - }) + } + ]) }) const code = 'export async function main() {\n\treturn 42\n}' @@ -145,4 +143,59 @@ describe('global AI tools', () => { }) ).resolves.toBe(code) }) + + it('writes flows with flow-mode arguments and reads compact flow value', async () => { + const writeResult = JSON.parse( + await callGlobalTool('write_flow', { + path: 'f/flows/with-schema-and-groups', + summary: 'Flow with schema and groups', + modules: JSON.stringify([ + { + id: 'start', + summary: 'Start', + value: { + type: 'identity' + } + } + ]), + schema: JSON.stringify({ + type: 'object', + properties: { + name: { type: 'string' } + }, + required: ['name'] + }), + groups: JSON.stringify([{ summary: 'Main', start_id: 'start', end_id: 'start' }]) + }) + ) + + expect(writeResult.item.value.value).toBeUndefined() + + const raw = await callGlobalTool('read_workspace_item', { + type: 'flow', + path: 'f/flows/with-schema-and-groups' + }) + const item = JSON.parse(raw) + + expect(item.value).toMatchObject({ + modules: [ + { + id: 'start', + summary: 'Start', + value: { type: 'identity' } + } + ], + schema: { + type: 'object', + properties: { + name: { type: 'string' } + }, + required: ['name'] + }, + preprocessor_module: null, + failure_module: null, + groups: [{ summary: 'Main', start_id: 'start', end_id: 'start' }] + }) + expect(item.value.value).toBeUndefined() + }) }) diff --git a/frontend/src/lib/components/copilot/chat/global/core.ts b/frontend/src/lib/components/copilot/chat/global/core.ts index de18426a3c..210636e43d 100644 --- a/frontend/src/lib/components/copilot/chat/global/core.ts +++ b/frontend/src/lib/components/copilot/chat/global/core.ts @@ -37,6 +37,7 @@ import { import { applyEditableFlowJsonToFlow, buildEditableFlowJson, + type EditableFlowJson, validateEditableFlowJson } from '../flow/editableFlowJson' import { createInlineScriptSession } from '../flow/inlineScriptsUtils' @@ -59,7 +60,6 @@ import { type ToolCallbacks, type ToolDisplayAction } from '../shared' -import { flowModuleSchema, flowModulesSchema } from '../flow/openFlowZod.gen' import { resourceRequestSchema, scheduleRequestSchema, @@ -149,22 +149,6 @@ const writeScriptSchema = z.object({ content: z.string().describe('Full script source code.') }) -const flowValueSchema = z - .looseObject({ - modules: flowModulesSchema.describe('Sequential flow modules.'), - preprocessor_module: flowModuleSchema - .nullable() - .optional() - .describe( - "Optional preprocessor module with id 'preprocessor'. Runs before normal modules; cannot reference results.*." - ), - failure_module: flowModuleSchema - .nullable() - .optional() - .describe("Optional failure handler module with id 'failure'.") - }) - .describe('OpenFlow value: modules plus optional preprocessor_module and failure_module.') - const readFlowModuleCodeSchema = z.object({ path: z.string().describe('Workspace path of the flow.'), module_id: z @@ -184,24 +168,81 @@ const setFlowModuleCodeSchema = z.object({ code: z.string().describe('New script source. Replaces the module\'s value.content entirely.') }) -// `value` is taken as a JSON string rather than a typed object because the -// underlying flowValueSchema is recursive (modules can contain modules), which -// makes z.toJSONSchema emit $defs/$ref. Gemini's tools API rejects those -// keywords ("Unknown name $ref/$defs"). The string is parsed and validated -// against flowValueSchema inside the handler. Same trick as set_flow_json in -// chat/flow/core.ts (see comment on its schema). +// Flow structure fields are taken as JSON strings rather than typed objects +// because the underlying flow module schema is recursive (modules can contain +// modules), which makes z.toJSONSchema emit $defs/$ref. Gemini's tools API +// rejects those keywords ("Unknown name $ref/$defs"). Same trick as +// set_flow_json in chat/flow/core.ts. const writeFlowSchema = z.object({ path: z .string() .describe('Workspace path of the flow, e.g. f/folder/name or u/user/name.'), summary: z.string().optional().describe('Short human-readable summary.'), - value: z + modules: z.string().describe('JSON string containing the complete flow modules array.'), + schema: z .string() + .optional() + .nullable() + .describe('JSON string containing the flow input schema.'), + preprocessor_module: z + .string() + .optional() + .nullable() + .describe('JSON string containing the optional preprocessor module.'), + failure_module: z + .string() + .optional() + .nullable() + .describe('JSON string containing the optional failure module.'), + groups: z + .string() + .optional() + .nullable() .describe( - 'JSON string of the OpenFlow value object: { modules, preprocessor_module?, failure_module? }. Pass it as a JSON-encoded string, not a nested object.' + 'JSON string containing the optional array of semantic flow groups. Pass null to clear groups.' ) }) +function parseOptionalJsonArg(value: unknown, field: string): unknown { + if (value === undefined || value === null) { + return value + } + + try { + return typeof value === 'string' ? JSON.parse(value) : value + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + throw new Error(`Invalid JSON for ${field}: ${message}`) + } +} + +function editableFlowToDraftValue(editable: EditableFlowJson): FlowDraftValue { + const value: FlowValue = { + modules: editable.modules, + preprocessor_module: editable.preprocessor_module ?? undefined, + failure_module: editable.failure_module ?? undefined, + groups: editable.groups ?? undefined + } + return { + value, + schema: editable.schema, + groups: editable.groups + } +} + +function flowDraftAsEditableInput(flowDraft: FlowDraftValue): { + value: FlowValue + schema?: Record | null | undefined +} { + return { + value: + flowDraft.groups === undefined + ? flowDraft.value + : { ...flowDraft.value, groups: flowDraft.groups ?? undefined }, + schema: flowDraft.schema + } +} + const writeScheduleSchema = scheduleRequestSchema const writeTriggerSchema = z.object({ @@ -428,7 +469,7 @@ Important rules: - Use search_resource_types before write_resource to discover the resource_type name and the JSON Schema its value must match. - Use get_instructions before writing a script, flow, resource, or app. For scripts, pass the target language; when modifying, use the language from the item you read. - Schedules, triggers, and variables do not need get_instructions — their tool schemas describe every field. -- A workspace item is { type, path, summary?, language?, triggerKind?, value, isDraft }. For scripts, value is the source code string. For flows, value is { value: , schema, groups } so the inputs schema and groups round-trip through deploy. For schedules/triggers/resources/variables, value is the full request body for that type. For apps, value is { files, runnables, data?, policy?, custom_path? } with frontend file contents and backend runnable definitions. +- A workspace item is { type, path, summary?, language?, triggerKind?, value, isDraft }. For scripts, value is the source code string. For flows, read_workspace_item returns value as the compact flow object { modules, schema, preprocessor_module, failure_module, groups }; write_flow takes the same flow fields as top-level tool arguments plus path/summary. For schedules/triggers/resources/variables, value is the full request body for that type. For apps, value is { files, runnables, data?, policy?, custom_path? } with frontend file contents and backend runnable definitions. - Apps (raw apps): use list_workspace_items with types: ['app'] to find them, read_workspace_item with type 'app' for a metadata summary (file paths + runnable list, no contents), then read_app_file to read individual files. Edit with write_app_file / patch_app_file / delete_app_file for frontend files and write_app_runnable / delete_app_runnable for backend runnables. Frontend file paths start with "/" (e.g. /index.tsx). Backend inline runnables are addressed as "backend//main.{ts|py}". /wmill.d.ts is generated and cannot be written. - To create a new raw app, use init_app. Before calling it, confirm framework (react19 / react18 / svelte5 / vue), path, and summary with the user — do not silently default to react19, even though it is the recommended choice. - Apps cannot be deployed from chat. The app editor bundles JS/CSS before save; tell the user to open the app editor to deploy app drafts. @@ -496,7 +537,7 @@ function serializeWorkspaceItemForRead(item: WorkspaceItem): unknown { if (item.type !== 'flow' || !item.value) return item const flowDraft = item.value as FlowDraftValue const session = createInlineScriptSession() - const editable = buildEditableFlowJson(flowDraft, session) + const editable = buildEditableFlowJson(flowDraftAsEditableInput(flowDraft), session) return { type: 'flow', path: item.path, @@ -1074,8 +1115,9 @@ function getFlowInstructions(): string { return `# Global draft flow instructions - Global mode writes complete draft payloads only; it does not save, deploy, run, scaffold local files, or generate metadata. -- A flow draft is a workspace item: \`{ type: 'flow', path, summary?, value, isDraft }\` where \`value\` is \`{ value: , schema, groups }\`. The inputs schema and groups are kept alongside the OpenFlow value so deploy round-trips them. -- \`value.modules\` contains normal sequential modules. Use top-level \`value.preprocessor_module\` and \`value.failure_module\` for special modules; do not put \`preprocessor\` or \`failure\` in \`value.modules\`. +- \`write_flow\` mirrors flow mode's \`set_flow_json\`: pass \`path\`, optional \`summary\`, required \`modules\`, and optional \`schema\`, \`preprocessor_module\`, \`failure_module\`, and \`groups\`. The flow-structure arguments are JSON strings, matching the tool schema descriptions. +- \`read_workspace_item\` returns a compact flow \`value\` object with \`modules\`, \`schema\`, \`preprocessor_module\`, \`failure_module\`, and \`groups\`. +- \`modules\` contains normal sequential modules. Use top-level \`preprocessor_module\` and \`failure_module\` for special modules; do not put \`preprocessor\` or \`failure\` in \`modules\`. - Every module needs a stable unique \`id\` and a useful \`summary\` when the schema supports it. - Prefer path/script/flow modules when composing existing workspace logic. Use rawscript modules only when new inline code is needed. - When writing rawscript module code, call \`get_instructions\` with \`subject: "script"\` and the rawscript language first. @@ -1087,7 +1129,7 @@ function getFlowInstructions(): string { - \`read_flow_module_code(path, module_id)\` — returns the raw inline script content for one module. - \`set_flow_module_code(path, module_id, code)\` — overwrites that module's inline script content; saves to the AI draft. - Use \`patch_flow_json\` for *structural* edits: module ids, paths, input_transforms, branch arrangement, summaries, preprocessor/failure swaps, schema/groups. Use \`set_flow_module_code\` for changes inside a specific rawscript body. -- \`write_flow\` is for full overwrites / create-from-scratch. Its \`value\` argument is the **non-compact** OpenFlow value (rawscript content is the actual code, not a placeholder). +- \`write_flow\` is for full overwrites / create-from-scratch. Its \`modules\`, \`preprocessor_module\`, and \`failure_module\` arguments use **non-compact** flow modules (rawscript content is the actual code, not a placeholder). # Windmill flow authoring reference @@ -1271,35 +1313,29 @@ export const globalTools: Tool<{}>[] = [ def: createToolDef( writeFlowSchema, 'write_flow', - 'Create or overwrite an AI draft flow. Does not save or deploy. Read the existing flow first when overwriting. value must be a JSON-encoded string of the OpenFlow value object.' + 'Create or overwrite an AI draft flow. Does not save or deploy. Read the existing flow first when overwriting. Uses the same flow-structure arguments as set_flow_json plus path and summary.' ), showDetails: true, streamArguments: true, showFade: true, fn: async (ctx) => { const parsed = writeFlowSchema.parse(ctx.args) - let value: unknown - try { - value = JSON.parse(parsed.value) - } catch (error) { - const message = error instanceof Error ? error.message : String(error) - throw new Error(`Invalid JSON for value: ${message}`) - } - const validated = flowValueSchema.safeParse(value) - if (!validated.success) { - throw new Error( - `Invalid flow value: ${validated.error.issues - .slice(0, 5) - .map((i) => `${i.path.join('.')}: ${i.message}`) - .join('; ')}` - ) - } + const editable = validateEditableFlowJson({ + modules: parseOptionalJsonArg(parsed.modules, 'modules'), + schema: parseOptionalJsonArg(parsed.schema, 'schema'), + preprocessor_module: parseOptionalJsonArg( + parsed.preprocessor_module, + 'preprocessor_module' + ), + failure_module: parseOptionalJsonArg(parsed.failure_module, 'failure_module'), + groups: parseOptionalJsonArg(parsed.groups, 'groups') + }) return writeDraft( { type: 'flow', path: parsed.path, summary: parsed.summary, - value: { value: validated.data as FlowValue, schema: null, groups: null }, + value: editableFlowToDraftValue(editable), isDraft: true }, ctx @@ -1683,7 +1719,7 @@ async function patchFlowJson( // model uses set_flow_module_code to change inline script bodies. const base = await loadFlowDraftValue(path, ctx.workspace) const session = createInlineScriptSession() - const editable = buildEditableFlowJson(base.flow, session) + const editable = buildEditableFlowJson(flowDraftAsEditableInput(base.flow), session) const currentJson = JSON.stringify(editable) const updatedJson = findAndReplace( currentJson, @@ -1730,7 +1766,7 @@ async function readFlowModuleCode( }) const base = await loadFlowDraftValue(args.path, workspace) const session = createInlineScriptSession() - buildEditableFlowJson(base.flow, session) + buildEditableFlowJson(flowDraftAsEditableInput(base.flow), session) const content = session.get(args.module_id) if (content === undefined) { throw new Error( @@ -1753,7 +1789,7 @@ async function setFlowModuleCode( }) const base = await loadFlowDraftValue(args.path, workspace) const session = createInlineScriptSession() - const editable = buildEditableFlowJson(base.flow, session) + const editable = buildEditableFlowJson(flowDraftAsEditableInput(base.flow), session) if (!session.has(args.module_id)) { throw new Error( `Module "${args.module_id}" is not an inline rawscript in flow "${args.path}". Use patch_flow_json or write_flow for structural changes.` @@ -2342,6 +2378,10 @@ async function writeDraft(item: WorkspaceItem, ctx: WriteDraftCtx): Promise e.method === 'GET') : mcpEndpointTools + ) + + // When read-only flips on, prune already-selected non-GET endpoints so the + // scope string doesn't keep references to tools the server will reject. + $effect(() => { + if (!readOnly || selectedEndpoints.length === 0) return + const allowed = new Set(visibleEndpointTools.map((e) => e.name)) + const filtered = selectedEndpoints.filter((n) => allowed.has(n)) + if (filtered.length !== selectedEndpoints.length) { + selectedEndpoints = filtered + } + }) const parsedInitial = parseInitialScope(initialScope) @@ -410,7 +428,7 @@ selectedFlows = [] } function selectAllEndpoints() { - selectedEndpoints = [...mcpEndpointTools.map((e) => e.name)] + selectedEndpoints = [...visibleEndpointTools.map((e) => e.name)] } function clearAllEndpoints() { selectedEndpoints = [] @@ -529,7 +547,7 @@
{@render sectionHeader('API Endpoints', selectAllEndpoints, clearAllEndpoints)} e.name))} + items={safeSelectItems(visibleEndpointTools.map((e) => e.name))} placeholder="Select endpoints" bind:value={selectedEndpoints} /> @@ -594,29 +612,35 @@
{:else}
- Scripts & Flows that will be available via MCP -
- {#if includedRunnables.length > 0 && includedRunnables.length <= 5} - {#each includedRunnables as scriptOrFlow (scriptOrFlow)} - {scriptOrFlow} - {/each} - {:else if includedRunnables.length > 0} - {#each includedRunnables.slice(0, 3) as scriptOrFlow (scriptOrFlow)} - {scriptOrFlow} - {/each} - - +{includedRunnables.length - 3} more - - {:else} -

- {warning} -

- {/if} -
+ {#if !readOnly} + Scripts & Flows that will be available via MCP +
+ {#if includedRunnables.length > 0 && includedRunnables.length <= 5} + {#each includedRunnables as scriptOrFlow (scriptOrFlow)} + {scriptOrFlow} + {/each} + {:else if includedRunnables.length > 0} + {#each includedRunnables.slice(0, 3) as scriptOrFlow (scriptOrFlow)} + {scriptOrFlow} + {/each} + + +{includedRunnables.length - 3} more + + {:else} +

+ {warning} +

+ {/if} +
+ {:else} +

+ Scripts and flows are hidden because this token is read-only. +

+ {/if} API endpoint tools that will be available via MCP
- {#each mcpEndpointTools as endpoint (endpoint.name)} + {#each visibleEndpointTools as endpoint (endpoint.name)} {#snippet text()}
diff --git a/frontend/src/lib/components/runs/runsFilter.ts b/frontend/src/lib/components/runs/runsFilter.ts index cbece7aefb..9d0b85dd75 100644 --- a/frontend/src/lib/components/runs/runsFilter.ts +++ b/frontend/src/lib/components/runs/runsFilter.ts @@ -23,14 +23,14 @@ export function buildRunsFilterSearchbarSchema({ usernames, folders, jobTriggerKinds, - isSuperAdmin, + isSuperAdminOrDevops, isAdminsWorkspace }: { paths: string[] usernames: string[] folders: string[] jobTriggerKinds: JobTriggerKind[] - isSuperAdmin: boolean + isSuperAdminOrDevops: boolean isAdminsWorkspace: boolean }) { return { @@ -206,12 +206,12 @@ export function buildRunsFilterSearchbarSchema({ label: 'Show future jobs (Default: true)', description: 'Include jobs that are planned later' }, - ...(isSuperAdmin && + ...(isSuperAdminOrDevops && isAdminsWorkspace && { all_workspaces: { type: 'boolean' as const, label: 'All workspaces', - description: 'Show jobs of all workspaces (superadmin only)' + description: 'Show jobs of all workspaces (superadmin or devops only)' } }) } satisfies FilterSchemaRec @@ -230,16 +230,16 @@ export function allowWildcards(filters: Partial | undefined) } export const buildRunsFilterPresets = ({ - isSuperadmin, + isSuperAdminOrDevops, isAdminsWorkspace }: { - isSuperadmin: boolean + isSuperAdminOrDevops: boolean isAdminsWorkspace: boolean }) => [ { name: 'Hide schedules', value: 'job_trigger_kind:\\ !schedule' }, { name: 'Hide future jobs', value: 'show_future_jobs:\\ false' }, { name: 'Show skipped', value: 'show_skipped:\\ true' }, - ...(isSuperadmin && isAdminsWorkspace + ...(isSuperAdminOrDevops && isAdminsWorkspace ? [{ name: 'All workspaces', value: 'all_workspaces:\\ true' }] : []) ] diff --git a/frontend/src/lib/components/settings/CreateToken.svelte b/frontend/src/lib/components/settings/CreateToken.svelte index 6ba2461725..80c6cb34ef 100644 --- a/frontend/src/lib/components/settings/CreateToken.svelte +++ b/frontend/src/lib/components/settings/CreateToken.svelte @@ -46,6 +46,7 @@ let mcpLabelAutofilled = $state(false) let pickedScopes = $state(null) + let readOnly = $state(false) function ensureCurrentWorkspaceIncluded( workspacesList: UserWorkspace[], @@ -67,6 +68,7 @@ newTokenWorkspace = defaultNewTokenWorkspace ?? $workspaceStore newToken = undefined newMcpToken = undefined + readOnly = false if (!newTokenLabel) { newTokenLabel = 'MCP token' mcpLabelAutofilled = true @@ -80,6 +82,7 @@ newTokenExpiration = undefined newTokenWorkspace = defaultNewTokenWorkspace newMcpToken = undefined + readOnly = false if (mcpLabelAutofilled) { newTokenLabel = undefined } @@ -100,7 +103,8 @@ label: newTokenLabel, expiration: date?.toISOString(), scopes: tokenScopes, - workspace_id: mcpMode ? newTokenWorkspace || $workspaceStore : newTokenWorkspace + workspace_id: mcpMode ? newTokenWorkspace || $workspaceStore : newTokenWorkspace, + read_only: readOnly } as NewToken }) @@ -184,6 +188,17 @@ {#each scopes as scope (scope)} {/each} +
+ +
{/if} @@ -192,6 +207,7 @@ mode={mcpCreationMode ? 'mcp' : 'standard'} workspaceId={newTokenWorkspace || $workspaceStore || ''} bind:value={pickedScopes} + bind:readOnly /> {/if} diff --git a/frontend/src/lib/components/settings/ScopeSelector.svelte b/frontend/src/lib/components/settings/ScopeSelector.svelte index da9d4949a6..91a62e65f7 100644 --- a/frontend/src/lib/components/settings/ScopeSelector.svelte +++ b/frontend/src/lib/components/settings/ScopeSelector.svelte @@ -7,10 +7,14 @@ import Tooltip from '../Tooltip.svelte' import { twMerge } from 'tailwind-merge' + import type { Snippet } from 'svelte' + interface Props { selectedScopes?: string[] disabled?: boolean class?: string + /** Renders above the scope-list card, below the Selected Scopes summary. */ + topSlot?: Snippet } interface ScopeState { @@ -30,7 +34,12 @@ domains: Record } - let { selectedScopes = $bindable([]), disabled = false, class: className = '' }: Props = $props() + let { + selectedScopes = $bindable([]), + disabled = false, + class: className = '', + topSlot + }: Props = $props() let scopeDomains = $state(null) let loading = $state(false) @@ -535,6 +544,12 @@ {/if}
+ {#if topSlot} +
+ {@render topSlot()} +
+ {/if} +
{#each scopeDomains as domain} {@const domainState = getDomainState(domain.name)} diff --git a/frontend/src/lib/components/settings/ScopesPicker.svelte b/frontend/src/lib/components/settings/ScopesPicker.svelte index 2e96269a00..e8cdccf40c 100644 --- a/frontend/src/lib/components/settings/ScopesPicker.svelte +++ b/frontend/src/lib/components/settings/ScopesPicker.svelte @@ -10,9 +10,28 @@ initialScopes?: string[] /** Final scope value: null = unrestricted/full access, array = explicit list */ value: string[] | null + /** Read-only flag; also forwarded to McpScopeSelector to filter incompatible + * endpoints/runnables. Two-way bound so the inline toggle below the + * "Limit token permissions" switch (and the MCP variant) writes back. */ + readOnly?: boolean } - let { mode, workspaceId = '', initialScopes, value = $bindable() }: Props = $props() + let { + mode, + workspaceId = '', + initialScopes, + value = $bindable(), + readOnly = $bindable(false) + }: Props = $props() + + // In standard mode, only meaningful when the user has turned "Limit token + // permissions" on. Reset when they un-limit so the flag doesn't quietly + // stick if they re-enable later. + $effect(() => { + if (mode === 'standard' && !limited && readOnly) { + readOnly = false + } + }) const initialMcpScope = $derived( (initialScopes ?? []).length > 0 ? (initialScopes ?? []).join(' ') : undefined @@ -51,9 +70,41 @@ size="xs" /> {#if limited} - + + {#snippet topSlot()} +
+ +
+ {/snippet} + {/if}
{:else} - +
+
+ +
+ +
{/if} diff --git a/frontend/src/lib/components/settings/TokensTable.svelte b/frontend/src/lib/components/settings/TokensTable.svelte index 816d710e3c..5907c808a6 100644 --- a/frontend/src/lib/components/settings/TokensTable.svelte +++ b/frontend/src/lib/components/settings/TokensTable.svelte @@ -170,7 +170,7 @@ {#snippet body()} {#if tokens && tokens.length > 0} - {#each tokens as { token_prefix, expiration, label, scopes, workspace_id } (token_prefix)} + {#each tokens as { token_prefix, expiration, label, scopes, workspace_id, read_only } (token_prefix)} {@const badge = expirationBadge(expiration, label)} {token_prefix}**** @@ -185,8 +185,15 @@ {scopes?.join(', ') ?? ''} +
+ {#if read_only} + Read-only + {/if} + {scopes?.join(', ') ?? ''} +
+