diff --git a/.claude/settings.json b/.claude/settings.json index a464ca3719..37aaeeb0f5 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -55,22 +55,18 @@ "mcp__claude_ai_Gmail__list_drafts" ], "deny": [ - "Read(.env)", - "Read(.env.*)", - "Read(**/.env)", - "Read(**/.env.*)", - "Read(**/secrets/**)", - "Read(**/*.pem)", - "Read(**/*.key)", - "Read(**/credentials.json)", - "Read(**/.secret*)", - "Read(**/.secrets*)", - "Read(**/*.secret)", - "Read(**/*.secrets)", "Edit(.env)", "Edit(.env.*)", "Edit(**/.env)", - "Edit(**/.env.*)" + "Edit(**/.env.*)", + "Edit(**/secrets/**)", + "Edit(**/*.pem)", + "Edit(**/*.key)", + "Edit(**/credentials.json)", + "Edit(**/.secret*)", + "Edit(**/.secrets*)", + "Edit(**/*.secret)", + "Edit(**/*.secrets)" ], "ask": [ "Bash(rmdir:*)", diff --git a/.github/workflows/backend-test.yml b/.github/workflows/backend-test.yml index cdd0e2a212..9f65c2ae47 100644 --- a/.github/workflows/backend-test.yml +++ b/.github/workflows/backend-test.yml @@ -61,7 +61,7 @@ jobs: bun-version: 1.4.0 - uses: actions/setup-node@v4 with: - node-version: "20" + node-version: "24" - uses: astral-sh/setup-uv@v6.2.1 with: version: "0.11.24" diff --git a/.github/workflows/git-sync-test.yml b/.github/workflows/git-sync-test.yml index ee732569dd..bd15ec8876 100644 --- a/.github/workflows/git-sync-test.yml +++ b/.github/workflows/git-sync-test.yml @@ -9,6 +9,7 @@ on: - "backend/windmill-api-integration-tests/tests/git_sync*" - "backend/ee-repo-ref.txt" - "backend/windmill-common/src/workspaces.rs" + - "frontend/src/lib/hubPaths.json" - "backend/windmill-worker/src/result_processor.rs" - "backend/windmill-api-workspaces/**" - "cli/src/commands/sync/**" @@ -22,6 +23,7 @@ on: - "backend/windmill-api-integration-tests/tests/git_sync*" - "backend/ee-repo-ref.txt" - "backend/windmill-common/src/workspaces.rs" + - "frontend/src/lib/hubPaths.json" - "backend/windmill-worker/src/result_processor.rs" - "backend/windmill-api-workspaces/**" - "cli/src/commands/sync/**" @@ -59,7 +61,7 @@ jobs: echo "$CHANGED_FILES" # Direct git sync file changes — always relevant. - if echo "$CHANGED_FILES" | grep -qE '^(backend/windmill-git-sync/|backend/windmill-worker/src/result_processor\.rs|backend/windmill-api-workspaces/|backend/windmill-api-integration-tests/tests/git_sync|backend/windmill-common/src/workspaces\.rs|cli/src/commands/sync/|cli/src/utils/git\.ts|integration_tests/test/git_sync|\.github/workflows/git-sync-test\.yml)'; then + if echo "$CHANGED_FILES" | grep -qE '^(backend/windmill-git-sync/|backend/windmill-worker/src/result_processor\.rs|backend/windmill-api-workspaces/|backend/windmill-api-integration-tests/tests/git_sync|backend/windmill-common/src/workspaces\.rs|frontend/src/lib/hubPaths\.json|cli/src/commands/sync/|cli/src/utils/git\.ts|integration_tests/test/git_sync|\.github/workflows/git-sync-test\.yml)'; then echo "should_run=true" >> "$GITHUB_OUTPUT" echo "Relevant: direct git sync file changes" exit 0 diff --git a/.release-please-manifest.json b/.release-please-manifest.json index 238a566433..249f8350ae 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "1.801.0" + ".": "1.803.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a7512989f..33e24c0c00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,41 @@ # Changelog +## [1.803.0](https://github.com/windmill-labs/windmill/compare/v1.802.0...v1.803.0) (2026-09-03) + + +### Features + +* expose request headers to scripts invoked via MCP ([#10903](https://github.com/windmill-labs/windmill/issues/10903)) ([e474e88](https://github.com/windmill-labs/windmill/commit/e474e8803ce2ff5c2df09a58dab51d45f5c922ca)) +* reuse an existing workspace resource in the project import wizard ([#10935](https://github.com/windmill-labs/windmill/issues/10935)) ([582761e](https://github.com/windmill-labs/windmill/commit/582761e37c776e92dc1c6ebfee8c4efe7c35d822)) + + +### Bug Fixes + +* bump git sync hub scripts to cli 1.802.1, test the fork ui pull ([#10955](https://github.com/windmill-labs/windmill/issues/10955)) ([ca88009](https://github.com/windmill-labs/windmill/commit/ca8800959aa6a0017cc29bad187c9f49e0d13cc4)) +* **cli:** make a sync push into a fork converge on schedules and inline names ([#10951](https://github.com/windmill-labs/windmill/issues/10951)) ([0f5a1db](https://github.com/windmill-labs/windmill/commit/0f5a1db2abba8df30a2f975f4498e269f13cf93d)) +* fade the home Build with AI placeholder every 10s instead of typing it ([#10953](https://github.com/windmill-labs/windmill/issues/10953)) ([3d089b5](https://github.com/windmill-labs/windmill/commit/3d089b57344f5814086e6176301c5031dc519674)) +* let operators use wmill.datatable() from within running jobs ([#10931](https://github.com/windmill-labs/windmill/issues/10931)) ([9b64a89](https://github.com/windmill-labs/windmill/commit/9b64a89cd46ae718d6c58fa12f925fa041fb1032)) + +## [1.802.0](https://github.com/windmill-labs/windmill/compare/v1.801.0...v1.802.0) (2026-09-02) + + +### Features + +* add retention cleanup for the otel_traces table ([#10949](https://github.com/windmill-labs/windmill/issues/10949)) ([d472193](https://github.com/windmill-labs/windmill/commit/d472193e5bf5f6428e0096a402eb2c9299634fb2)) +* open path links from chat messages in the session preview panel ([#10881](https://github.com/windmill-labs/windmill/issues/10881)) ([f10ac6c](https://github.com/windmill-labs/windmill/commit/f10ac6c2b3644fb16697e650efbc4f7cd3c6944c)) +* restore owner and label filter chips on the homepage ([#10942](https://github.com/windmill-labs/windmill/issues/10942)) ([ccf8476](https://github.com/windmill-labs/windmill/commit/ccf84761dd664b9228dfe2f65867e8c32cd20c21)) +* **sessions:** offer the item you came from when starting a new session ([#10940](https://github.com/windmill-labs/windmill/issues/10940)) ([d3747d6](https://github.com/windmill-labs/windmill/commit/d3747d62555ebcb09c78cfabcaa3b6177758d6ea)) +* workspace setting to hide the AI assistant, agent steps unaffected ([#10941](https://github.com/windmill-labs/windmill/issues/10941)) ([fdd3b36](https://github.com/windmill-labs/windmill/commit/fdd3b36423344a2e1a464674179406581074e926)) + + +### Bug Fixes + +* apply object-storage test SSRF validation to all non-super-admins ([#10933](https://github.com/windmill-labs/windmill/issues/10933)) ([4fef119](https://github.com/windmill-labs/windmill/commit/4fef1195adaa9fa036a219884bd6c996460ca37f)) +* connect to dev server instead of localhost ([#10912](https://github.com/windmill-labs/windmill/issues/10912)) ([337154b](https://github.com/windmill-labs/windmill/commit/337154b8304a5969f35216add627b5c1153c0f6c)) +* preselect first row of AI agent and AI sandbox insert panes ([#10937](https://github.com/windmill-labs/windmill/issues/10937)) ([95b6bbd](https://github.com/windmill-labs/windmill/commit/95b6bbd46ada11d96a914ae5b0e92aba4dd02530)) +* record supplied script lock hashes so importers can skip relocking ([#10915](https://github.com/windmill-labs/windmill/issues/10915)) ([17ba521](https://github.com/windmill-labs/windmill/commit/17ba521c352aec65a8270893752bbadd7f3d6eaa)) +* sandbox script-controlled content types in result_to_response ([#10932](https://github.com/windmill-labs/windmill/issues/10932)) ([419741e](https://github.com/windmill-labs/windmill/commit/419741e5d226c67c51429094fb6ded9474afed99)) + ## [1.801.0](https://github.com/windmill-labs/windmill/compare/v1.800.1...v1.801.0) (2026-09-01) diff --git a/Dockerfile b/Dockerfile index 8ecee96725..6a290c3553 100644 --- a/Dockerfile +++ b/Dockerfile @@ -141,9 +141,9 @@ FROM ${DEBIAN_IMAGE} ARG TARGETPLATFORM ARG POWERSHELL_VERSION=7.5.0 ARG KUBECTL_VERSION=1.36.2 -ARG HELM_VERSION=3.21.2 +ARG HELM_VERSION=3.21.4 # NOTE: If changing, also change go version in workspace dependencies template at WorkspaceDependenciesEditor.svelte -ARG GO_VERSION=1.26.0 +ARG GO_VERSION=1.26.8 ARG APP=/usr/src/app ARG WITH_POWERSHELL=true ARG WITH_KUBECTL=true @@ -250,8 +250,8 @@ RUN UV_CACHE_DIR=/tmp/build_cache/uv UV_PYTHON_INSTALL_DIR=/tmp/build_cache/py_r RUN UV_CACHE_DIR=/tmp/build_cache/uv UV_PYTHON_INSTALL_DIR=/tmp/build_cache/py_runtime uv python install $LATEST_STABLE_PY --compile-bytecode -RUN curl -sL https://deb.nodesource.com/setup_20.x | bash - -RUN apt-get -y update && apt-get install -y curl procps nodejs awscli && apt-get clean \ +RUN curl -sL https://deb.nodesource.com/setup_24.x | bash - +RUN apt-get -y update && apt-get install -y --no-install-recommends curl procps nodejs awscli && apt-get clean \ && rm -rf /var/lib/apt/lists/* # go build is slower the first time it is ran, so we prewarm it in the build @@ -299,7 +299,7 @@ RUN bun install -g windmill-cli \ RUN curl -fsSL https://claude.ai/install.sh | bash \ && cp /root/.local/share/claude/versions/* /usr/bin/claude -COPY --from=php:8.3.30-cli-trixie /usr/local/bin/php /usr/bin/php +COPY --from=php:8.3.33-cli-trixie /usr/local/bin/php /usr/bin/php COPY --from=composer:2.9.5 /usr/bin/composer /usr/bin/composer # add the docker client to call docker from a worker if enabled diff --git a/ai_evals/adapters/frontend/mockBackend.ts b/ai_evals/adapters/frontend/mockBackend.ts index 5c682be5eb..6d80f19197 100644 --- a/ai_evals/adapters/frontend/mockBackend.ts +++ b/ai_evals/adapters/frontend/mockBackend.ts @@ -7,6 +7,7 @@ import type { ListableApp, ListableResource, ListableVariable, + Resource, Script } from '../../../frontend/src/lib/gen' import type { @@ -81,6 +82,15 @@ export interface BenchmarkWorkspaceAiProvider { isDefault?: boolean } +/** A plain (non-AI) resource of the benchmark workspace, for cases about referencing a + * credential — passing one as a run argument, say. `value` is what `get_resource` returns. */ +export interface BenchmarkWorkspaceResource { + path: string + resource_type: string + value?: Record + description?: string +} + export interface BenchmarkWorkspaceJob { /** Stable id so a case prompt can reference a specific run (e.g. for get_job_logs). */ id?: string @@ -98,6 +108,7 @@ export interface BenchmarkWorkspaceRunnables { apps?: BenchmarkWorkspaceApp[] variables?: BenchmarkWorkspaceVariable[] aiProviders?: BenchmarkWorkspaceAiProvider[] + resources?: BenchmarkWorkspaceResource[] datatables?: BenchmarkDatatableSeed[] jobs?: BenchmarkWorkspaceJob[] } @@ -284,15 +295,71 @@ export function listBenchmarkAiProviderResources(workspace: string): ListableRes })) } -/** The value of a seeded AI provider resource. Only the endpoint fields are modelled — a key is - * never needed, because no eval run calls the provider through this resource. */ +/** Plain seeded resources of a benchmark workspace, shaped like `ResourceService.listResource` + * rows. Null when the workspace is not a benchmark one. */ +export function listBenchmarkPlainResources(workspace: string): ListableResource[] | null { + const runnables = benchmarkWorkspaceRunnables.get(workspace) + if (!runnables) { + return null + } + return (runnables.resources ?? []).map((seed) => ({ + workspace_id: workspace, + path: seed.path, + resource_type: seed.resource_type, + description: seed.description, + value: null, + is_oauth: false, + is_linked: false, + is_refreshed: false, + extra_perms: {}, + edited_at: BENCHMARK_TIMESTAMP + })) +} + +/** A seeded resource with its value, as `ResourceService.getResource` returns it. Covers both + * seed kinds, so it agrees with `existsResource` and `listResource` — both of those report AI + * providers too, and a case that lists resources and then reads one by path would otherwise get + * a row it cannot fetch. */ +export function getBenchmarkResource(workspace: string, path: string): Resource | null { + const runnables = benchmarkWorkspaceRunnables.get(workspace) + const seed = runnables?.resources?.find((entry) => entry.path === path) + if (seed) { + return { + workspace_id: workspace, + path: seed.path, + resource_type: seed.resource_type, + description: seed.description, + value: seed.value ?? {}, + is_oauth: false, + extra_perms: {} + } as Resource + } + const provider = runnables?.aiProviders?.find((entry) => entry.path === path) + if (!provider) { + return null + } + return { + workspace_id: workspace, + path: provider.path, + resource_type: provider.kind, + value: getBenchmarkResourceValue(workspace, path) ?? {}, + is_oauth: false, + extra_perms: {} + } as Resource +} + +/** The value of a seeded resource. For an AI provider only the endpoint fields are modelled — a + * key is never needed, because no eval run calls the provider through this resource. */ export function getBenchmarkResourceValue( workspace: string, path: string ): Record | null { - const seed = benchmarkWorkspaceRunnables - .get(workspace) - ?.aiProviders?.find((entry) => entry.path === path) + const runnables = benchmarkWorkspaceRunnables.get(workspace) + const plain = runnables?.resources?.find((entry) => entry.path === path) + if (plain) { + return plain.value ?? {} + } + const seed = runnables?.aiProviders?.find((entry) => entry.path === path) if (!seed) { return null } diff --git a/ai_evals/adapters/frontend/vitestAdapter.test.ts b/ai_evals/adapters/frontend/vitestAdapter.test.ts index e59ec11ad9..338ed8504c 100644 --- a/ai_evals/adapters/frontend/vitestAdapter.test.ts +++ b/ai_evals/adapters/frontend/vitestAdapter.test.ts @@ -70,7 +70,9 @@ vi.mock('$lib/gen', async () => { getBenchmarkResourceValue, getBenchmarkVariableByPath, hasBenchmarkWorkspace, + getBenchmarkResource, listBenchmarkAiProviderResources, + listBenchmarkPlainResources, listBenchmarkApps, listBenchmarkDatatables, listBenchmarkDrafts, @@ -359,18 +361,24 @@ vi.mock('$lib/gen', async () => { hasBenchmarkWorkspace(data.workspace) ? Boolean(getBenchmarkResourceValue(data.workspace, data.path)) : actual.ResourceService.existsResource(data), - // Only AI provider resources are modelled: they are what an AI agent step references. listResource: async (data: { workspace: string; resourceType?: string }) => { if (!hasBenchmarkWorkspace(data.workspace)) { return actual.ResourceService.listResource(data) } - const seeded = listBenchmarkAiProviderResources(data.workspace) ?? [] + const seeded = [ + ...(listBenchmarkAiProviderResources(data.workspace) ?? []), + ...(listBenchmarkPlainResources(data.workspace) ?? []) + ] const wanted = data.resourceType?.split(',') return wanted ? seeded.filter((r) => wanted.includes(r.resource_type)) : seeded }, getResource: async (data: { workspace: string; path: string }) => { if (hasBenchmarkWorkspace(data.workspace)) { - throw new Error(`Resource "${data.path}" not found in benchmark workspace`) + const resource = getBenchmarkResource(data.workspace, data.path) + if (!resource) { + throw new Error(`Resource "${data.path}" not found in benchmark workspace`) + } + return resource } return actual.ResourceService.getResource(data) }, diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index 21f80bef7e..94ecb5c029 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -2365,3 +2365,37 @@ - the step uses the workspace's anthropic resource f/evals/global/anthropic_main - the model is the Opus one the user asked for, taken from the models that resource serves - the diff flow input reaches the agent + +# The failure this pins: passing a resource as `{"$res": ""}` (or as a bare path), which +# reaches the script unresolved because the backend only substitutes a string value that itself +# starts with `$res:`. The mock preview echoes args back and reports success, so nothing in the +# loop corrects a wrong shape — the arg form is the whole test. +- id: global-run-arg-resource-reference + prompt: |- + Run `f/evals/global/github_repo_stats` against the `windmill-labs/windmill` repo, passing our + GitHub credentials at `f/evals/global/github_main` as its `gh_auth` input, and tell me whether + it went through. + initial: ai_evals/fixtures/frontend/global/initial/github_repo_stats_script.json + runtime: + maxTurns: 10 + validate: + draftCountExactly: 0 + toolExpect: + requiredToolsUsed: + - test_run_script + forbiddenToolsUsed: + - write_script + - deploy_workspace_item + toolCallArgs: + # Exact: the mock never resolves the reference, so a near-miss path like + # `$res:f/evals/global/github_main_backup` would otherwise pass. + - tool: test_run_script + field: args.gh_auth + stringEqualsAnyOf: + - "$res:f/evals/global/github_main" + # The judge only sees drafts, and this case makes none — the deliverable is the shape of the + # run argument, checked deterministically above. + skipJudge: true + judgeChecklist: + - runs the existing script rather than rewriting it + - passes the GitHub resource as the bare string $res:f/evals/global/github_main diff --git a/ai_evals/core/types.ts b/ai_evals/core/types.ts index c9a4f7830e..4107dc53da 100644 --- a/ai_evals/core/types.ts +++ b/ai_evals/core/types.ts @@ -160,6 +160,13 @@ export interface ToolCallArgumentRule { field: string; stringStartsWithAnyOf?: string[]; stringMustNotStartWithAnyOf?: string[]; + /** + * Universal over calls: every recorded call to `tool` must carry `field` as + * exactly one of these strings. Use when a near-miss would still satisfy a + * prefix — a resource reference like `$res:f/a/b` shares its prefix with the + * wrong `$res:f/a/b_backup`, and the mock never resolves it to catch that. + */ + stringEqualsAnyOf?: string[]; /** * Case-insensitive "contains", existential over calls: at least one recorded * call to `tool` must have `field` containing one of these substrings. Other diff --git a/ai_evals/core/validators.test.ts b/ai_evals/core/validators.test.ts index 9b606c1c85..4b7891f67e 100644 --- a/ai_evals/core/validators.test.ts +++ b/ai_evals/core/validators.test.ts @@ -228,6 +228,43 @@ describe("validateToolExpectations", () => { }); }); + // A resource reference shares its prefix with a wrong sibling path, and the mock + // never resolves it, so only exact matching separates the two. + it("rejects a resource reference whose path merely shares the prefix", () => { + const checks = validateToolExpectations({ + run: { + success: true, + actual: {}, + assistantMessageCount: 1, + toolCallCount: 1, + toolsUsed: ["test_run_script"], + toolCallDetails: [ + { + name: "test_run_script", + arguments: { args: { gh_auth: "$res:f/evals/global/github_main_backup" } }, + }, + ], + skillsInvoked: [], + }, + toolExpect: { + toolCallArgs: [ + { + tool: "test_run_script", + field: "args.gh_auth", + stringEqualsAnyOf: ["$res:f/evals/global/github_main"], + }, + ], + }, + }); + + expect(checks).toContainEqual({ + name: "test_run_script.args.gh_auth matches an accepted value", + passed: false, + details: + 'accepted values: $res:f/evals/global/github_main; values: "$res:f/evals/global/github_main_backup"', + }); + }); + // The whole point of the same-call rule: the per-field rules are existential over // calls, so two single-filter pages would satisfy them while never opening the // combined view the case asks for. diff --git a/ai_evals/core/validators.ts b/ai_evals/core/validators.ts index 7150dab0c3..132a9ff3d0 100644 --- a/ai_evals/core/validators.ts +++ b/ai_evals/core/validators.ts @@ -278,6 +278,20 @@ export function validateToolExpectations(input: { ); } + if (rule.stringEqualsAnyOf && rule.stringEqualsAnyOf.length > 0) { + const invalidValues = values.filter( + (value) => + typeof value !== "string" || !rule.stringEqualsAnyOf!.includes(value) + ); + checks.push( + check( + `${rule.tool}.${rule.field} matches an accepted value`, + invalidValues.length === 0, + `accepted values: ${rule.stringEqualsAnyOf.join(", ")}; values: ${summarizeToolValues(values)}` + ) + ); + } + if (rule.stringMustNotStartWithAnyOf && rule.stringMustNotStartWithAnyOf.length > 0) { const invalidValues = values.filter( (value) => diff --git a/ai_evals/fixtures/frontend/global/initial/github_repo_stats_script.json b/ai_evals/fixtures/frontend/global/initial/github_repo_stats_script.json new file mode 100644 index 0000000000..80e1b960c5 --- /dev/null +++ b/ai_evals/fixtures/frontend/global/initial/github_repo_stats_script.json @@ -0,0 +1,37 @@ +{ + "workspace": { + "resources": [ + { + "path": "f/evals/global/github_main", + "resource_type": "github", + "description": "GitHub credentials", + "value": { "token": "$var:f/evals/global/github_token" } + } + ], + "scripts": [ + { + "path": "f/evals/global/github_repo_stats", + "summary": "Count open issues on a GitHub repository", + "description": "Reads the open issue count for a repository using GitHub credentials.", + "language": "bun", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { + "gh_auth": { + "type": "object", + "format": "resource-github", + "description": "GitHub credentials" + }, + "repo": { + "type": "string", + "description": "Repository in owner/name form" + } + }, + "required": ["gh_auth", "repo"] + }, + "content": "type Github = { token: string }\n\nexport async function main(gh_auth: Github, repo: string) {\n const res = await fetch(`https://api.github.com/repos/${repo}/issues?state=open`, {\n headers: { Authorization: `Bearer ${gh_auth.token}` }\n })\n const issues = await res.json()\n return { repo, open_issues: issues.length }\n}\n" + } + ] + } +} diff --git a/backend/.sqlx/query-1572b7348a05b7e357031f8d44b5bbee155569488352c10b334ce57d83ce1c0a.json b/backend/.sqlx/query-1572b7348a05b7e357031f8d44b5bbee155569488352c10b334ce57d83ce1c0a.json new file mode 100644 index 0000000000..03f11ef137 --- /dev/null +++ b/backend/.sqlx/query-1572b7348a05b7e357031f8d44b5bbee155569488352c10b334ce57d83ce1c0a.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT lockfile_hash FROM lock_hash WHERE workspace_id = $1 AND path = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "lockfile_hash", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false + ] + }, + "hash": "1572b7348a05b7e357031f8d44b5bbee155569488352c10b334ce57d83ce1c0a" +} diff --git a/backend/.sqlx/query-16b174aaa944fd94458ce3108f0fec23514ae0419f77b632064b75473b5636c3.json b/backend/.sqlx/query-16b174aaa944fd94458ce3108f0fec23514ae0419f77b632064b75473b5636c3.json new file mode 100644 index 0000000000..693d84173a --- /dev/null +++ b/backend/.sqlx/query-16b174aaa944fd94458ce3108f0fec23514ae0419f77b632064b75473b5636c3.json @@ -0,0 +1,20 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT trace_id FROM otel_traces ORDER BY trace_id", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "trace_id", + "type_info": "Bytea" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + false + ] + }, + "hash": "16b174aaa944fd94458ce3108f0fec23514ae0419f77b632064b75473b5636c3" +} diff --git a/backend/.sqlx/query-1b244f65ee6a2607ebc1c333d4359fbbf8be5a81276a3050a42770e4a5b5aa5e.json b/backend/.sqlx/query-1b244f65ee6a2607ebc1c333d4359fbbf8be5a81276a3050a42770e4a5b5aa5e.json new file mode 100644 index 0000000000..f35dbc23e4 --- /dev/null +++ b/backend/.sqlx/query-1b244f65ee6a2607ebc1c333d4359fbbf8be5a81276a3050a42770e4a5b5aa5e.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM otel_traces WHERE ctid IN (\n SELECT ctid FROM otel_traces\n WHERE start_time_unix_nano < EXTRACT(\n EPOCH FROM now() - ($1::bigint::text || ' s')::interval\n )::bigint * 1000000000\n LIMIT $2\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8", + "Int8" + ] + }, + "nullable": [] + }, + "hash": "1b244f65ee6a2607ebc1c333d4359fbbf8be5a81276a3050a42770e4a5b5aa5e" +} diff --git a/backend/.sqlx/query-49b18e987e2dfa3c7ab915757ff3b9c0e6e371136b565f9b0f5a3393ef8d8d57.json b/backend/.sqlx/query-49b18e987e2dfa3c7ab915757ff3b9c0e6e371136b565f9b0f5a3393ef8d8d57.json deleted file mode 100644 index 8d86ff3db6..0000000000 --- a/backend/.sqlx/query-49b18e987e2dfa3c7ab915757ff3b9c0e6e371136b565f9b0f5a3393ef8d8d57.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "WITH update_lock AS (\n UPDATE script SET lock = $1, modules = COALESCE($6, modules) WHERE hash = $2 AND workspace_id = $3\n )\n INSERT INTO lock_hash (workspace_id, path, lockfile_hash)\n VALUES ($3, $4, $5)\n ON CONFLICT (workspace_id, path) DO UPDATE SET lockfile_hash = $5", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Text", - "Int8", - "Text", - "Varchar", - "Int8", - "Jsonb" - ] - }, - "nullable": [] - }, - "hash": "49b18e987e2dfa3c7ab915757ff3b9c0e6e371136b565f9b0f5a3393ef8d8d57" -} diff --git a/backend/.sqlx/query-62ed1fe52bc2c22d199101309cbbadb9842318c4c7a1d2526ac567ce41b7fbdc.json b/backend/.sqlx/query-62ed1fe52bc2c22d199101309cbbadb9842318c4c7a1d2526ac567ce41b7fbdc.json new file mode 100644 index 0000000000..67b672228f --- /dev/null +++ b/backend/.sqlx/query-62ed1fe52bc2c22d199101309cbbadb9842318c4c7a1d2526ac567ce41b7fbdc.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO otel_traces (trace_id, span_id, name, kind, start_time_unix_nano, end_time_unix_nano)\n VALUES ($1, $2, 'GET /', 3, $3, $3)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Bytea", + "Bytea", + "Int8" + ] + }, + "nullable": [] + }, + "hash": "62ed1fe52bc2c22d199101309cbbadb9842318c4c7a1d2526ac567ce41b7fbdc" +} diff --git a/backend/.sqlx/query-8da59f1ace46dc9830cb0fc5a640df68f552e38d4b587839e0e41285a2d55455.json b/backend/.sqlx/query-8da59f1ace46dc9830cb0fc5a640df68f552e38d4b587839e0e41285a2d55455.json new file mode 100644 index 0000000000..610030cffc --- /dev/null +++ b/backend/.sqlx/query-8da59f1ace46dc9830cb0fc5a640df68f552e38d4b587839e0e41285a2d55455.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT DISTINCT ON (path) path, lock FROM script\n WHERE workspace_id = $1 AND NOT archived AND NOT deleted AND lock IS NOT NULL\n ORDER BY path, created_at DESC", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "path", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "lock", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + false, + true + ] + }, + "hash": "8da59f1ace46dc9830cb0fc5a640df68f552e38d4b587839e0e41285a2d55455" +} diff --git a/backend/.sqlx/query-96aa1db2859c80d37ffecc2885442ae6d6d4b6731c3f62e7b85ae7a4f1b666a7.json b/backend/.sqlx/query-96aa1db2859c80d37ffecc2885442ae6d6d4b6731c3f62e7b85ae7a4f1b666a7.json deleted file mode 100644 index 733478e0e9..0000000000 --- a/backend/.sqlx/query-96aa1db2859c80d37ffecc2885442ae6d6d4b6731c3f62e7b85ae7a4f1b666a7.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "\n INSERT INTO script\n (workspace_id, hash, path, parent_hashes, summary, description, content, created_by, schema, is_template, extra_perms, lock, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels)\n\n SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, content, created_by, schema, is_template, extra_perms, NULL, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels\n\n FROM script WHERE hash = $2 AND workspace_id = $3;\n ", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Int8", - "Int8", - "Text" - ] - }, - "nullable": [] - }, - "hash": "96aa1db2859c80d37ffecc2885442ae6d6d4b6731c3f62e7b85ae7a4f1b666a7" -} diff --git a/backend/.sqlx/query-a0ec5048ddb7640b4407013ed45b7a545959c209c550e48bf108734f293e18e4.json b/backend/.sqlx/query-a0ec5048ddb7640b4407013ed45b7a545959c209c550e48bf108734f293e18e4.json new file mode 100644 index 0000000000..f4c4fd31de --- /dev/null +++ b/backend/.sqlx/query-a0ec5048ddb7640b4407013ed45b7a545959c209c550e48bf108734f293e18e4.json @@ -0,0 +1,19 @@ +{ + "db_name": "PostgreSQL", + "query": "WITH update_lock AS (\n UPDATE script SET lock = $1, modules = COALESCE($6, modules) WHERE hash = $2 AND workspace_id = $3\n )\n INSERT INTO lock_hash (workspace_id, path, lockfile_hash)\n VALUES ($3, $4, $5)\n ON CONFLICT (workspace_id, path) DO UPDATE SET lockfile_hash = $5", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Int8", + "Text", + "Varchar", + "Int8", + "Jsonb" + ] + }, + "nullable": [] + }, + "hash": "a0ec5048ddb7640b4407013ed45b7a545959c209c550e48bf108734f293e18e4" +} diff --git a/backend/.sqlx/query-abdf62ef0e4eeb8c3213d2e8837e7032f710fe20dd272b7840c9bfbdb92554db.json b/backend/.sqlx/query-abdf62ef0e4eeb8c3213d2e8837e7032f710fe20dd272b7840c9bfbdb92554db.json new file mode 100644 index 0000000000..c3ef22f973 --- /dev/null +++ b/backend/.sqlx/query-abdf62ef0e4eeb8c3213d2e8837e7032f710fe20dd272b7840c9bfbdb92554db.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT COUNT(*) FROM script WHERE workspace_id = $1 AND path = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "count", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "abdf62ef0e4eeb8c3213d2e8837e7032f710fe20dd272b7840c9bfbdb92554db" +} diff --git a/backend/.sqlx/query-cc507843e00c83a42cc4a463999656ce9a8b0499d6b9282a3ecfae3164b17c2a.json b/backend/.sqlx/query-cc507843e00c83a42cc4a463999656ce9a8b0499d6b9282a3ecfae3164b17c2a.json new file mode 100644 index 0000000000..19dc4781a6 --- /dev/null +++ b/backend/.sqlx/query-cc507843e00c83a42cc4a463999656ce9a8b0499d6b9282a3ecfae3164b17c2a.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO lock_hash (workspace_id, path, lockfile_hash)\n SELECT $1, * FROM UNNEST($2::text[], $3::bigint[])\n ON CONFLICT (workspace_id, path) DO UPDATE SET lockfile_hash = EXCLUDED.lockfile_hash\n WHERE lock_hash.lockfile_hash IS DISTINCT FROM EXCLUDED.lockfile_hash", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Varchar", + "TextArray", + "Int8Array" + ] + }, + "nullable": [] + }, + "hash": "cc507843e00c83a42cc4a463999656ce9a8b0499d6b9282a3ecfae3164b17c2a" +} diff --git a/backend/.sqlx/query-f0070b36f7c4fc84dc9c23bb6c73d8ba80993a28b2c2e5df70968acf6d7cebe4.json b/backend/.sqlx/query-cfdd5ac1dfc7276fc37d49ddfe1b8880eaafb2d3fe71d75b676f1719e26f660f.json similarity index 72% rename from backend/.sqlx/query-f0070b36f7c4fc84dc9c23bb6c73d8ba80993a28b2c2e5df70968acf6d7cebe4.json rename to backend/.sqlx/query-cfdd5ac1dfc7276fc37d49ddfe1b8880eaafb2d3fe71d75b676f1719e26f660f.json index b31e532fc2..2947dd8413 100644 --- a/backend/.sqlx/query-f0070b36f7c4fc84dc9c23bb6c73d8ba80993a28b2c2e5df70968acf6d7cebe4.json +++ b/backend/.sqlx/query-cfdd5ac1dfc7276fc37d49ddfe1b8880eaafb2d3fe71d75b676f1719e26f660f.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "VACUUM v2_job, v2_job_completed, job_result_stream_v2, job_stats, job_logs, job_perms, concurrency_key, log_file, metrics", + "query": "VACUUM v2_job, v2_job_completed, job_result_stream_v2, job_stats, job_logs, job_perms, concurrency_key, log_file, metrics, otel_traces", "describe": { "columns": [], "parameters": { @@ -8,5 +8,5 @@ }, "nullable": [] }, - "hash": "f0070b36f7c4fc84dc9c23bb6c73d8ba80993a28b2c2e5df70968acf6d7cebe4" + "hash": "cfdd5ac1dfc7276fc37d49ddfe1b8880eaafb2d3fe71d75b676f1719e26f660f" } diff --git a/backend/.sqlx/query-dd15827cbac128ec53cf03fa305e4cad34e540a2cc09c92e262491145a0de05a.json b/backend/.sqlx/query-dd15827cbac128ec53cf03fa305e4cad34e540a2cc09c92e262491145a0de05a.json new file mode 100644 index 0000000000..eba1b0da99 --- /dev/null +++ b/backend/.sqlx/query-dd15827cbac128ec53cf03fa305e4cad34e540a2cc09c92e262491145a0de05a.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "DELETE FROM lock_hash WHERE workspace_id = $1 AND path = $2", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "dd15827cbac128ec53cf03fa305e4cad34e540a2cc09c92e262491145a0de05a" +} diff --git a/backend/.sqlx/query-ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637.json b/backend/.sqlx/query-ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637.json new file mode 100644 index 0000000000..bea6be2a7f --- /dev/null +++ b/backend/.sqlx/query-ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637.json @@ -0,0 +1,19 @@ +{ + "db_name": "PostgreSQL", + "query": "\n INSERT INTO script\n (workspace_id, hash, path, parent_hashes, summary, description, content, created_by, schema, is_template, extra_perms, lock, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels, lock_error_logs, created_at)\n\n SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, content, created_by, schema, is_template, extra_perms, $4::text, language, kind, tag, envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, COALESCE($5::jsonb, modules), labels, $6::text, clock_timestamp()\n\n FROM script WHERE hash = $2 AND workspace_id = $3;\n ", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Int8", + "Int8", + "Text", + "Text", + "Jsonb", + "Text" + ] + }, + "nullable": [] + }, + "hash": "ede523c0b0027f7bc1dacd3a5448031783fa7fbbbf86a291b6f8f8f875d45637" +} diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 913e0eda75..d314168b35 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -5177,9 +5177,9 @@ dependencies = [ [[package]] name = "frostem" -version = "1.20260821.3" +version = "1.20260821.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca2e81eab151ba68484704bb3d21b4b4d2747314d3081fa86fa6c7300a4c42e2" +checksum = "481ace7f781f5ae54a5c0a6d6d8edb30adba737cfa1230fbd5632d63ba8dfd80" [[package]] name = "fs3" @@ -7387,9 +7387,9 @@ dependencies = [ [[package]] name = "lru" -version = "0.18.3" +version = "0.18.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d317b4b9eb398e6acce275758ec6125535505e7a146fb1a9b8bda2451b0ff4c" +checksum = "ff9840bcc50b71349309900da0ce7279aa336ae71d73250b07998932c7d97c25" dependencies = [ "hashbrown 0.17.1", ] @@ -7776,9 +7776,9 @@ dependencies = [ [[package]] name = "mio" -version = "1.2.2" +version = "1.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30d65c71f1ce40ab09135ce117d742b9f8a19ff91a41a8b57ed50bc2de59c427" +checksum = "4b18443e9c262bfe8fa82f51666e2642c53393f7e5c27b3e1aeab922cff5b9d8" dependencies = [ "libc", "wasi 0.11.1+wasi-snapshot-preview1", @@ -7883,7 +7883,7 @@ dependencies = [ "futures-sink", "futures-util", "keyed_priority_queue", - "lru 0.18.3", + "lru 0.18.4", "mysql_common", "native-tls", "pem 3.0.6", @@ -12813,7 +12813,7 @@ dependencies = [ "itertools 0.14.0", "levenshtein_automata", "log", - "lru 0.18.3", + "lru 0.18.4", "lz4_flex 0.14.0", "measure_time", "memmap2", @@ -14747,7 +14747,7 @@ dependencies = [ [[package]] name = "windmill" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-nats", @@ -14832,7 +14832,7 @@ dependencies = [ [[package]] name = "windmill-ai" -version = "1.801.0" +version = "1.803.0" dependencies = [ "async-stream", "async-trait", @@ -14865,7 +14865,7 @@ dependencies = [ [[package]] name = "windmill-alerting" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -14878,7 +14878,7 @@ dependencies = [ [[package]] name = "windmill-api" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "argon2", @@ -15018,7 +15018,7 @@ dependencies = [ [[package]] name = "windmill-api-agent-workers" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15041,7 +15041,7 @@ dependencies = [ [[package]] name = "windmill-api-assets" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15058,7 +15058,7 @@ dependencies = [ [[package]] name = "windmill-api-auth" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "axum 0.8.9", @@ -15084,7 +15084,7 @@ dependencies = [ [[package]] name = "windmill-api-client" -version = "1.801.0" +version = "1.803.0" dependencies = [ "reqwest 0.12.28", "serde", @@ -15094,7 +15094,7 @@ dependencies = [ [[package]] name = "windmill-api-configs" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15111,7 +15111,7 @@ dependencies = [ [[package]] name = "windmill-api-debug" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "base64 0.22.1", @@ -15133,7 +15133,7 @@ dependencies = [ [[package]] name = "windmill-api-embeddings" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "axum 0.8.9", @@ -15156,7 +15156,7 @@ dependencies = [ [[package]] name = "windmill-api-flow-conversations" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15172,7 +15172,7 @@ dependencies = [ [[package]] name = "windmill-api-flows" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15194,7 +15194,7 @@ dependencies = [ [[package]] name = "windmill-api-groups" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15215,7 +15215,7 @@ dependencies = [ [[package]] name = "windmill-api-inputs" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15229,7 +15229,7 @@ dependencies = [ [[package]] name = "windmill-api-integration-tests" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-nats", @@ -15264,7 +15264,7 @@ dependencies = [ [[package]] name = "windmill-api-jobs" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "axum 0.8.9", @@ -15289,7 +15289,7 @@ dependencies = [ [[package]] name = "windmill-api-npm-proxy" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15317,7 +15317,7 @@ dependencies = [ [[package]] name = "windmill-api-openapi" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "axum 0.8.9", @@ -15339,7 +15339,7 @@ dependencies = [ [[package]] name = "windmill-api-schedule" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15359,7 +15359,7 @@ dependencies = [ [[package]] name = "windmill-api-scripts" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15397,7 +15397,7 @@ dependencies = [ [[package]] name = "windmill-api-settings" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "axum 0.8.9", @@ -15425,7 +15425,7 @@ dependencies = [ [[package]] name = "windmill-api-sse" -version = "1.801.0" +version = "1.803.0" dependencies = [ "lazy_static", "serde", @@ -15437,7 +15437,7 @@ dependencies = [ [[package]] name = "windmill-api-users" -version = "1.801.0" +version = "1.803.0" dependencies = [ "argon2", "axum 0.8.9", @@ -15461,7 +15461,7 @@ dependencies = [ [[package]] name = "windmill-api-workers" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15475,7 +15475,7 @@ dependencies = [ [[package]] name = "windmill-api-workspaces" -version = "1.801.0" +version = "1.803.0" dependencies = [ "axum 0.8.9", "chrono", @@ -15510,7 +15510,7 @@ dependencies = [ [[package]] name = "windmill-audit" -version = "1.801.0" +version = "1.803.0" dependencies = [ "chrono", "lazy_static", @@ -15524,7 +15524,7 @@ dependencies = [ [[package]] name = "windmill-autoscaling" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "axum 0.8.9", @@ -15543,7 +15543,7 @@ dependencies = [ [[package]] name = "windmill-common" -version = "1.801.0" +version = "1.803.0" dependencies = [ "aes-gcm", "aho-corasick", @@ -15647,9 +15647,10 @@ dependencies = [ [[package]] name = "windmill-dep-map" -version = "1.801.0" +version = "1.803.0" dependencies = [ "chrono", + "futures", "itertools 0.14.0", "lazy_static", "serde", @@ -15666,7 +15667,7 @@ dependencies = [ [[package]] name = "windmill-git-sync" -version = "1.801.0" +version = "1.803.0" dependencies = [ "regex", "serde", @@ -15681,7 +15682,7 @@ dependencies = [ [[package]] name = "windmill-indexer" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "astral-tokio-tar", @@ -15708,7 +15709,7 @@ dependencies = [ [[package]] name = "windmill-jseval" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "futures", @@ -15725,7 +15726,7 @@ dependencies = [ [[package]] name = "windmill-macros" -version = "1.801.0" +version = "1.803.0" dependencies = [ "itertools 0.14.0", "lazy_static", @@ -15741,7 +15742,7 @@ dependencies = [ [[package]] name = "windmill-mcp" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -15762,7 +15763,7 @@ dependencies = [ [[package]] name = "windmill-native-triggers" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -15793,7 +15794,7 @@ dependencies = [ [[package]] name = "windmill-oauth" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "arc-swap", @@ -15818,7 +15819,7 @@ dependencies = [ [[package]] name = "windmill-object-store" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-stream", @@ -15852,7 +15853,7 @@ dependencies = [ [[package]] name = "windmill-operator" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "futures", @@ -15870,7 +15871,7 @@ dependencies = [ [[package]] name = "windmill-parser" -version = "1.801.0" +version = "1.803.0" dependencies = [ "convert_case 0.6.0", "serde", @@ -15879,7 +15880,7 @@ dependencies = [ [[package]] name = "windmill-parser-bash" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -15891,7 +15892,7 @@ dependencies = [ [[package]] name = "windmill-parser-csharp" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "serde_json", @@ -15903,7 +15904,7 @@ dependencies = [ [[package]] name = "windmill-parser-go" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "gosyn", @@ -15915,7 +15916,7 @@ dependencies = [ [[package]] name = "windmill-parser-graphql" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -15927,7 +15928,7 @@ dependencies = [ [[package]] name = "windmill-parser-java" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "serde_json", @@ -15939,7 +15940,7 @@ dependencies = [ [[package]] name = "windmill-parser-nu" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "nu-parser", @@ -15950,7 +15951,7 @@ dependencies = [ [[package]] name = "windmill-parser-php" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "itertools 0.14.0", @@ -15961,7 +15962,7 @@ dependencies = [ [[package]] name = "windmill-parser-py" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "itertools 0.14.0", @@ -15973,7 +15974,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-asset" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "rustpython-ast", @@ -15984,7 +15985,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-imports" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-recursion", @@ -16006,7 +16007,7 @@ dependencies = [ [[package]] name = "windmill-parser-r" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "serde_json", @@ -16018,7 +16019,7 @@ dependencies = [ [[package]] name = "windmill-parser-ruby" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -16032,7 +16033,7 @@ dependencies = [ [[package]] name = "windmill-parser-rust" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "convert_case 0.6.0", @@ -16049,7 +16050,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -16062,7 +16063,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql-asset" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "serde", @@ -16074,7 +16075,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -16092,7 +16093,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts-asset" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "serde-wasm-bindgen", @@ -16108,7 +16109,7 @@ dependencies = [ [[package]] name = "windmill-parser-wac" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "rustpython-ast", @@ -16124,7 +16125,7 @@ dependencies = [ [[package]] name = "windmill-parser-yaml" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -16138,7 +16139,7 @@ dependencies = [ [[package]] name = "windmill-queue" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-recursion", @@ -16177,7 +16178,7 @@ dependencies = [ [[package]] name = "windmill-runtime-nativets" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "const_format", @@ -16217,7 +16218,7 @@ dependencies = [ [[package]] name = "windmill-sql-datatype-parser-wasm" -version = "1.801.0" +version = "1.803.0" dependencies = [ "getrandom 0.3.4", "wasm-bindgen", @@ -16228,7 +16229,7 @@ dependencies = [ [[package]] name = "windmill-store" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-recursion", @@ -16263,7 +16264,7 @@ dependencies = [ [[package]] name = "windmill-test-utils" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16287,7 +16288,7 @@ dependencies = [ [[package]] name = "windmill-trigger" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16320,7 +16321,7 @@ dependencies = [ [[package]] name = "windmill-trigger-amqp" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16347,7 +16348,7 @@ dependencies = [ [[package]] name = "windmill-trigger-azure" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16380,7 +16381,7 @@ dependencies = [ [[package]] name = "windmill-trigger-email" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16400,7 +16401,7 @@ dependencies = [ [[package]] name = "windmill-trigger-gcp" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16434,7 +16435,7 @@ dependencies = [ [[package]] name = "windmill-trigger-http" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16470,7 +16471,7 @@ dependencies = [ [[package]] name = "windmill-trigger-kafka" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16493,7 +16494,7 @@ dependencies = [ [[package]] name = "windmill-trigger-mqtt" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16517,7 +16518,7 @@ dependencies = [ [[package]] name = "windmill-trigger-nats" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-nats", @@ -16541,7 +16542,7 @@ dependencies = [ [[package]] name = "windmill-trigger-postgres" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16576,7 +16577,7 @@ dependencies = [ [[package]] name = "windmill-trigger-sqs" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16604,7 +16605,7 @@ dependencies = [ [[package]] name = "windmill-trigger-websocket" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-trait", @@ -16629,7 +16630,7 @@ dependencies = [ [[package]] name = "windmill-types" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "bitflags 2.13.1", @@ -16648,7 +16649,7 @@ dependencies = [ [[package]] name = "windmill-worker" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-once-cell", @@ -16765,7 +16766,7 @@ dependencies = [ [[package]] name = "windmill-worker-volumes" -version = "1.801.0" +version = "1.803.0" dependencies = [ "bytes", "futures", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index ef62595562..f432f022c5 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "windmill" -version = "1.801.0" +version = "1.803.0" authors.workspace = true edition.workspace = true @@ -88,7 +88,7 @@ members = [ exclude = ["./windmill-duckdb-ffi-internal", "./parsers/windmill-parser-wasm"] [workspace.package] -version = "1.801.0" +version = "1.803.0" authors = ["Ruben Fiszel "] edition = "2021" diff --git a/backend/THREAT_MODEL.md b/backend/THREAT_MODEL.md index 0468ebf721..f5725d65a3 100644 --- a/backend/THREAT_MODEL.md +++ b/backend/THREAT_MODEL.md @@ -85,7 +85,7 @@ published advisory history (73 GHSA advisories, several rated 9.9 critical). | EP9 Worker sandbox | nsjail / unshare / dind / rootless podman isolating user code | user code → host & cross-tenant filesystem/network | Worker host, isolation, downstream | | EP10 Worker code generation / wrappers | Entrypoint override, env-var names, workspace env interpolated into generated wrapper code | user-controlled identifier → executable code | Worker host, isolation | | EP11 OAuth / OIDC / SAML / MCP-OAuth / logout | Login callbacks, MCP OAuth client registration, logout `rd` redirect | untrusted IdP / redirect input → session | Session tokens, accounts | -| EP12 Stored-content rendering | App builder HTML component, markdown, S3 download response headers | stored user content → admin browser (same origin) | Admin session, account takeover | +| EP12 Stored-content rendering | App builder HTML component, markdown, S3 download response headers, script-controlled `wm_content_type`/`wm_headers` on `run_wait_result` and sync HTTP-route responses | stored user content → admin browser (same origin) | Admin session, account takeover | | EP13 Log/file reading & export endpoints | `service_logs`, `jobs_u/getupdate` log file read (symlinks), workspace/tarball export | authed/unauth request → arbitrary file or admin-only config | Arbitrary files, global settings | | EP14 Secret-value & resource-value caches | In-memory caches in `windmill-store` keyed (historically un-keyed) by path | cache lookup crossing identity/folder boundary | Secret variables, resource creds | | EP15 Deployment & runtime config | docker-compose defaults: dind, debugger (`REQUIRE_SIGNED_DEBUG_REQUESTS` now defaults to `true`; can still be overridden to `false`), CORS `Any`, default admin/`changeme`, exposed Postgres, `SUPERADMIN_SECRET`, `ENABLE_NSJAIL=false`, privileged containers | operator/infra default → full instance | All assets | @@ -106,7 +106,7 @@ published advisory history (73 GHSA advisories, several rated 9.9 critical). | T8 | Unauthenticated RCE via the Debugger WebSocket: `/ws_debug/*` exposed by the gateway/ingress with the debugger service as the auth boundary; signature gate was bypassable via `program`-mode launches (read+exec an arbitrary server-side file path, never signed) even with signing on, and the WS handshake had no Origin check (CSWSH) | remote_unauth | EP15 | Worker host, all assets | critical | possible | partially_mitigated | `program`-mode launches now rejected when `REQUIRE_SIGNED_DEBUG_REQUESTS` is on (signing covers every launch, not just inline `code`); shipped `docker-compose` now defaults `REQUIRE_SIGNED_DEBUG_REQUESTS=true`; opt-in `DEBUG_ALLOWED_ORIGINS` allowlist rejects cross-origin handshakes. Residual: code default is secure but operators can still set `=false`; origin allowlist is opt-in | GHSA-725h-99vx-9xr4 | | T9 | Supply-chain compromise via cached hub scripts, GitHub workflow command injection, or vulnerable base-image deps | supply_chain | EP16 | Worker host, build integrity | critical | possible | partially_mitigated | hub-script re-pin to patched versions; HUB_BASE_URL override | GHSA-w2m9-q5f7-3gpq, edf340c4d4, GHSA-8rq7-w7g6-8wvr, GHSA-vch9-39v5-4wg7 (CVE-2024-37371) | | T10 | Unauthenticated disclosure of job results, args, logs, and admin config via missing-authz public endpoints | remote_unauth | EP2, EP13 | Job results/args/logs, global settings, scripts | high | likely | partially_mitigated | anonymous-job checks, log-endpoint authz hardening | GHSA-qfg7-x243-5hg4, GHSA-v448-fmm4-52fp, 108a88a180, bb90f4ce83 | -| T11 | Stored XSS leading to admin/account takeover via app HTML component, markdown, or S3 download content-type | remote_auth | EP12 | Admin session, accounts | high | likely | partially_mitigated | DOMPurify markdown sanitization, `X-Content-Type-Options: nosniff` + CSP sandbox on downloads | GHSA-9c5c-hh3c-r9mc, GHSA-qxj7-hpx3-r892, GHSA-cf2x-rg8c-v63v, bb78b1c06d, 625b67dff0 | +| T11 | Stored XSS leading to admin/account takeover via app HTML component, markdown, S3 download content-type, or a script-chosen `text/html` content type on `run_wait_result` / sync HTTP-route responses (GET-reachable with the `SameSite=Lax` session cookie) | remote_auth | EP12 | Admin session, accounts | high | likely | partially_mitigated | DOMPurify markdown sanitization, `X-Content-Type-Options: nosniff` + CSP sandbox on downloads and on every `result_to_response` composite result (inserted after `wm_headers`; hop-by-hop names such as `Connection` rejected so a proxy cannot strip them) | GHSA-9c5c-hh3c-r9mc, GHSA-qxj7-hpx3-r892, GHSA-cf2x-rg8c-v63v, bb78b1c06d, 625b67dff0, WIN-2471 | | T12 | Webhook authentication bypass / signature replay forges trigger invocations and approvals | remote_unauth | EP3 | Job execution integrity, approvals | high | likely | partially_mitigated | HMAC verification on some triggers; signing-oracle fix | GHSA-jw8c-h45c-xpjw, GHSA-hh9x-rcf8-xjr2, GHSA-q9g3-q6fj-hc2x, GHSA-8jc4-wj2p-2vmp, ab2a15b2a8 | | T13 | Path traversal / arbitrary file read via log-reading and MCP path endpoints (incl. symlink following) | remote_auth | EP13 | Arbitrary files on server, global settings | high | likely | partially_mitigated | traversal checks + no-symlink-follow added | GHSA-4hrf-mgvv-xp9x, bb90f4ce83, df451aa64f, ad5ec293b5, 5f2d3e6812 | | T14 | Privilege escalation via token rescope/refresh, script-issued JWTs, or operator-permission gaps | remote_auth | EP17, EP5 | Tokens, isolation, accounts | high | likely | partially_mitigated | monotonic-privilege enforcement on token lifecycle; SECURITY DEFINER triggers | GHSA-p62p-67xp-v775, GHSA-vv9w-wx3c-q3x2, 2ddf93de96, 865ab70c89, 33fb08cf3d | diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index e6198b836b..e60c4e0fdf 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -1b569b727364e8f3db8513fb41e99f1fff1d1406 +747fbf5a869630d04f0cdabd79f42140ce60adeb diff --git a/backend/parsers/windmill-parser-wasm/Cargo.lock b/backend/parsers/windmill-parser-wasm/Cargo.lock index 15a288cb24..395e692623 100644 --- a/backend/parsers/windmill-parser-wasm/Cargo.lock +++ b/backend/parsers/windmill-parser-wasm/Cargo.lock @@ -6191,7 +6191,7 @@ checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" [[package]] name = "windmill-common" -version = "1.801.0" +version = "1.803.0" dependencies = [ "aho-corasick", "anyhow", @@ -6274,7 +6274,7 @@ dependencies = [ [[package]] name = "windmill-macros" -version = "1.801.0" +version = "1.803.0" dependencies = [ "proc-macro2", "quote", @@ -6286,7 +6286,7 @@ dependencies = [ [[package]] name = "windmill-parser" -version = "1.801.0" +version = "1.803.0" dependencies = [ "convert_case", "serde", @@ -6295,7 +6295,7 @@ dependencies = [ [[package]] name = "windmill-parser-bash" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -6307,7 +6307,7 @@ dependencies = [ [[package]] name = "windmill-parser-csharp" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "serde_json", @@ -6319,7 +6319,7 @@ dependencies = [ [[package]] name = "windmill-parser-go" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "gosyn", @@ -6331,7 +6331,7 @@ dependencies = [ [[package]] name = "windmill-parser-graphql" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -6343,7 +6343,7 @@ dependencies = [ [[package]] name = "windmill-parser-java" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "serde_json", @@ -6355,7 +6355,7 @@ dependencies = [ [[package]] name = "windmill-parser-nu" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "nu-parser", @@ -6366,7 +6366,7 @@ dependencies = [ [[package]] name = "windmill-parser-php" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "itertools 0.14.0", @@ -6377,7 +6377,7 @@ dependencies = [ [[package]] name = "windmill-parser-py" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "itertools 0.14.0", @@ -6389,7 +6389,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-asset" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "rustpython-ast", @@ -6400,7 +6400,7 @@ dependencies = [ [[package]] name = "windmill-parser-py-imports" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "async-recursion", @@ -6422,7 +6422,7 @@ dependencies = [ [[package]] name = "windmill-parser-r" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "serde_json", @@ -6434,7 +6434,7 @@ dependencies = [ [[package]] name = "windmill-parser-ruby" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -6448,7 +6448,7 @@ dependencies = [ [[package]] name = "windmill-parser-rust" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "convert_case", @@ -6465,7 +6465,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -6478,7 +6478,7 @@ dependencies = [ [[package]] name = "windmill-parser-sql-asset" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "serde", @@ -6490,7 +6490,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -6508,7 +6508,7 @@ dependencies = [ [[package]] name = "windmill-parser-ts-asset" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "serde-wasm-bindgen", @@ -6524,7 +6524,7 @@ dependencies = [ [[package]] name = "windmill-parser-wac" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "rustpython-ast", @@ -6540,7 +6540,7 @@ dependencies = [ [[package]] name = "windmill-parser-wasm" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "getrandom 0.2.17", @@ -6572,7 +6572,7 @@ dependencies = [ [[package]] name = "windmill-parser-yaml" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "lazy_static", @@ -6586,7 +6586,7 @@ dependencies = [ [[package]] name = "windmill-types" -version = "1.801.0" +version = "1.803.0" dependencies = [ "anyhow", "bitflags", diff --git a/backend/parsers/windmill-parser-wasm/Cargo.toml b/backend/parsers/windmill-parser-wasm/Cargo.toml index 4aae8cad13..45c8b0d94e 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.801.0" +version = "1.803.0" edition = "2021" authors = ["Ruben Fiszel "] diff --git a/backend/src/main.rs b/backend/src/main.rs index 5ec78b0b06..f84812151d 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -53,9 +53,9 @@ use windmill_common::{ KEEP_JOB_DIR_SETTING, LICENSE_KEY_SETTING, MAVEN_REPOS_SETTING, MAVEN_SETTINGS_XML_SETTING, MONITOR_LOGS_ON_OBJECT_STORE_SETTING, NO_DEFAULT_MAVEN_SETTING, NPM_CONFIG_REGISTRY_SETTING, NSJAIL_TMPFS_SIZE_MB_SETTING, NSJAIL_TMP_BACKING_SETTING, - NUGET_CONFIG_SETTING, OAUTH_SETTING, OTEL_SETTING, OTEL_TRACING_PROXY_SETTING, - PIP_INDEX_URL_SETTING, POWERSHELL_REPO_PAT_SETTING, POWERSHELL_REPO_URL_SETTING, - PREVIEW_TAGS_OVERRIDE_SETTING, REQUEST_SIZE_LIMIT_SETTING, + NUGET_CONFIG_SETTING, OAUTH_SETTING, OTEL_SETTING, OTEL_TRACES_RETENTION_SECS_SETTING, + OTEL_TRACING_PROXY_SETTING, PIP_INDEX_URL_SETTING, POWERSHELL_REPO_PAT_SETTING, + POWERSHELL_REPO_URL_SETTING, PREVIEW_TAGS_OVERRIDE_SETTING, REQUEST_SIZE_LIMIT_SETTING, REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING, RESTART_COORDINATION_SETTING, RETENTION_PERIOD_SECS_OVERRIDES_SETTING, RETENTION_PERIOD_SECS_SETTING, RUBY_REPOS_SETTING, SAML_METADATA_SETTING, SANDBOX_IMAGE_CACHE_MAX_MB_SETTING, @@ -140,12 +140,12 @@ use crate::monitor::{ reload_instance_events_webhook_setting, reload_job_default_timeout_setting, reload_job_isolation_setting, reload_jwt_secret_setting, reload_license_key, reload_npm_config_registry_setting, reload_nsjail_tmp_backing_setting, - reload_nsjail_tmpfs_size_setting, reload_otel_tracing_proxy_setting, - reload_pip_index_url_setting, reload_retention_period_setting, - reload_sandbox_image_cache_max_setting, reload_sandbox_image_default_registry_setting, - reload_sandbox_image_max_size_setting, reload_sandbox_image_pull_policy_setting, - reload_sandbox_registry_auth_setting, reload_scim_token_setting, - reload_service_log_retention_secs_setting, reload_smtp_config, + reload_nsjail_tmpfs_size_setting, reload_otel_traces_retention_secs_setting, + reload_otel_tracing_proxy_setting, reload_pip_index_url_setting, + reload_retention_period_setting, reload_sandbox_image_cache_max_setting, + reload_sandbox_image_default_registry_setting, reload_sandbox_image_max_size_setting, + reload_sandbox_image_pull_policy_setting, reload_sandbox_registry_auth_setting, + reload_scim_token_setting, reload_service_log_retention_secs_setting, reload_smtp_config, reload_store_audit_logs_s3_setting, reload_uv_exclude_newer_setting, reload_uv_index_strategy_setting, reload_uv_python_install_mirror_setting, reload_worker_config, MonitorIteration, @@ -2013,6 +2013,9 @@ async fn process_notify_event( SERVICE_LOG_RETENTION_SECS_SETTING => { reload_service_log_retention_secs_setting(conn).await } + OTEL_TRACES_RETENTION_SECS_SETTING => { + reload_otel_traces_retention_secs_setting(conn).await + } RETENTION_PERIOD_SECS_OVERRIDES_SETTING => { if let Err(e) = load_retention_period_overrides(db).await { tracing::error!("Error loading per-workspace retention overrides: {e:#}"); diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index ce09cebe71..86e75b7de9 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -64,10 +64,11 @@ use windmill_common::{ JOB_ISOLATION_SETTING, JWT_SECRET_SETTING, KEEP_JOB_DIR_SETTING, LICENSE_KEY_SETTING, MONITOR_LOGS_ON_OBJECT_STORE_SETTING, NPMRC_SETTING, NPM_CONFIG_REGISTRY_SETTING, NSJAIL_TMPFS_SIZE_MB_SETTING, NSJAIL_TMP_BACKING_SETTING, NUGET_CONFIG_SETTING, - OTEL_SETTING, OTEL_TRACING_PROXY_SETTING, PIP_INDEX_URL_SETTING, - POWERSHELL_REPO_PAT_SETTING, POWERSHELL_REPO_URL_SETTING, PREVIEW_TAGS_OVERRIDE_SETTING, - REQUEST_SIZE_LIMIT_SETTING, REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING, - RETENTION_PERIOD_SECS_SETTING, SAML_METADATA_SETTING, SANDBOX_IMAGE_CACHE_MAX_MB_SETTING, + OTEL_SETTING, OTEL_TRACES_RETENTION_SECS_SETTING, OTEL_TRACING_PROXY_SETTING, + PIP_INDEX_URL_SETTING, POWERSHELL_REPO_PAT_SETTING, POWERSHELL_REPO_URL_SETTING, + PREVIEW_TAGS_OVERRIDE_SETTING, REQUEST_SIZE_LIMIT_SETTING, + REQUIRE_PREEXISTING_USER_FOR_OAUTH_SETTING, RETENTION_PERIOD_SECS_SETTING, + SAML_METADATA_SETTING, SANDBOX_IMAGE_CACHE_MAX_MB_SETTING, SANDBOX_IMAGE_DEFAULT_REGISTRY_SETTING, SANDBOX_IMAGE_MAX_SIZE_MB_SETTING, SANDBOX_IMAGE_PULL_POLICY_SETTING, SANDBOX_REGISTRY_AUTH_SETTING, SCIM_TOKEN_SETTING, SERVICE_LOG_RETENTION_SECS_SETTING, SMTP_SETTING, STORE_AUDIT_LOGS_S3_SETTING, @@ -97,10 +98,10 @@ use windmill_common::{ KillpillSender, AUDIT_LOG_RETENTION_DAYS, BASE_URL, CRITICAL_ALERTS_ON_DB_OVERSIZE, CRITICAL_ALERTS_ON_TOKEN_EXPIRY, CRITICAL_ALERT_MUTE_UI_ENABLED, CRITICAL_ALERT_MUTE_ZOMBIE_JOB_RESTART, CRITICAL_ERROR_CHANNELS, DB, DEFAULT_HUB_BASE_URL, - DEFAULT_SERVICE_LOG_RETENTION_SECS, HUB_BASE_URL, JOB_RETENTION_SECS, - JOB_RETENTION_SECS_OVERRIDES, JOB_RETENTION_SECS_OVERRIDES_LOADED, METRICS_DEBUG_ENABLED, - METRICS_ENABLED, MONITOR_LOGS_ON_OBJECT_STORE, OTEL_LOGS_ENABLED, OTEL_METRICS_ENABLED, - OTEL_TRACING_ENABLED, STORE_AUDIT_LOGS_S3, + DEFAULT_OTEL_TRACES_RETENTION_SECS, DEFAULT_SERVICE_LOG_RETENTION_SECS, HUB_BASE_URL, + JOB_RETENTION_SECS, JOB_RETENTION_SECS_OVERRIDES, JOB_RETENTION_SECS_OVERRIDES_LOADED, + METRICS_DEBUG_ENABLED, METRICS_ENABLED, MONITOR_LOGS_ON_OBJECT_STORE, OTEL_LOGS_ENABLED, + OTEL_METRICS_ENABLED, OTEL_TRACING_ENABLED, STORE_AUDIT_LOGS_S3, }; use windmill_common::{ client::AuthedClient, @@ -514,6 +515,15 @@ pub async fn initial_load( Ordering::Relaxed, ) }); + pass.setting(OTEL_TRACES_RETENTION_SECS_SETTING, true, |v| async move { + windmill_common::set_otel_traces_retention_secs(parse_setting_value::( + v, + OTEL_TRACES_RETENTION_SECS_SETTING, + "OTEL_TRACES_RETENTION_SECS", + DEFAULT_OTEL_TRACES_RETENTION_SECS, + |x| x, + )) + }); pass.setting(STORE_AUDIT_LOGS_S3_SETTING, true, |v| async move { STORE_AUDIT_LOGS_S3.store( parse_setting_value::( @@ -1682,6 +1692,57 @@ const SERVICE_LOG_DELETE_BATCH: i64 = 2_000; /// across ticks rather than inside one, the way the neighbouring sweeps already do. const SERVICE_LOG_DELETE_MAX_BATCHES: usize = 10; +/// One span per HTTP request made from a job script, so the table grows far faster than the +/// job table it is keyed against; batched for the same reason the service log sweep is. +const OTEL_TRACES_DELETE_BATCH: i64 = 10_000; +const OTEL_TRACES_DELETE_MAX_BATCHES: usize = 10; + +/// Delete HTTP request tracing spans older than `retention_secs`, returning how many went. +/// +/// `retention_secs` is a parameter rather than a read of the process-wide setting so a test can +/// pin a window without writing state the other tests in this binary run against concurrently. +async fn delete_expired_otel_traces(db: &DB, retention_secs: i64) -> u64 { + // `start_time_unix_nano` is the proto field stored verbatim, so the cutoff is built in that + // unit rather than compared against `now()`. Truncating the epoch to whole seconds first + // keeps the multiplication inside `bigint`. + // + // Batched on `ctid`, not on the `(trace_id, span_id)` primary key: with the key the planner + // hashes the LIMITed subquery and Seq Scans the whole table to probe it, which at the size + // this table reaches is the cost the batching exists to avoid. `ctid` plans as a Tid Scan, so + // each batch touches only the rows it deletes. Safe because the subquery and the delete share + // one snapshot, and spans are never updated after insert. + let mut deleted = 0; + for _ in 0..OTEL_TRACES_DELETE_MAX_BATCHES { + let batch = sqlx::query!( + "DELETE FROM otel_traces WHERE ctid IN ( + SELECT ctid FROM otel_traces + WHERE start_time_unix_nano < EXTRACT( + EPOCH FROM now() - ($1::bigint::text || ' s')::interval + )::bigint * 1000000000 + LIMIT $2 + )", + retention_secs, + OTEL_TRACES_DELETE_BATCH, + ) + .execute(db) + .await; + + match batch { + Ok(res) => { + deleted += res.rows_affected(); + if (res.rows_affected() as i64) < OTEL_TRACES_DELETE_BATCH { + break; + } + } + Err(e) => { + tracing::error!("Error deleting expired otel trace spans: {:?}", e); + break; + } + } + } + deleted +} + pub async fn delete_expired_items(db: &DB) -> () { let expired_tokens_r = sqlx::query_as!( TokenRow, @@ -1808,6 +1869,12 @@ pub async fn delete_expired_items(db: &DB) -> () { } } + let deleted_spans = + delete_expired_otel_traces(db, windmill_common::otel_traces_retention_secs()).await; + if deleted_spans > 0 { + tracing::info!("deleted {} expired otel trace spans", deleted_spans); + } + let audit_retention_days = audit_log_retention_days().await; let audit_retention_secs: i64 = audit_retention_days * 60 * 60 * 24; @@ -2927,6 +2994,21 @@ pub async fn reload_service_log_retention_secs_setting(conn: &Connection) { } } +pub async fn reload_otel_traces_retention_secs_setting(conn: &Connection) { + match load_setting_value::( + conn, + OTEL_TRACES_RETENTION_SECS_SETTING, + "OTEL_TRACES_RETENTION_SECS", + DEFAULT_OTEL_TRACES_RETENTION_SECS, + |x| x, + ) + .await + { + Ok(v) => windmill_common::set_otel_traces_retention_secs(v), + Err(e) => tracing::error!("Error reloading otel traces retention period: {:?}", e), + } +} + pub async fn reload_audit_log_retention_days_setting(conn: &Connection) { match load_setting_value::( conn, @@ -4839,7 +4921,7 @@ async fn poll_git_fork_branches( } async fn vacuuming_tables(db: &Pool) -> error::Result<()> { - sqlx::query!("VACUUM v2_job, v2_job_completed, job_result_stream_v2, job_stats, job_logs, job_perms, concurrency_key, log_file, metrics") + sqlx::query!("VACUUM v2_job, v2_job_completed, job_result_stream_v2, job_stats, job_logs, job_perms, concurrency_key, log_file, metrics, otel_traces") .execute(db) .await?; Ok(()) @@ -6975,6 +7057,46 @@ mod zombie_worker_memory_pct_tests { } } +#[cfg(test)] +mod otel_traces_retention_tests { + use super::{delete_expired_otel_traces, DB}; + + async fn insert_span(db: &DB, id: u8, age_secs: i64) { + sqlx::query!( + "INSERT INTO otel_traces (trace_id, span_id, name, kind, start_time_unix_nano, end_time_unix_nano) + VALUES ($1, $2, 'GET /', 3, $3, $3)", + &[id; 16][..], + &[id; 8][..], + (chrono::Utc::now() - chrono::Duration::seconds(age_secs)) + .timestamp_nanos_opt() + .unwrap(), + ) + .execute(db) + .await + .unwrap(); + } + + /// The cutoff crosses two units: a retention configured in seconds against a column holding + /// nanoseconds. Getting that conversion wrong is silent in both directions — a window a + /// billion times too wide never deletes anything, one a billion times too narrow deletes + /// every span on the next tick — so pin it on either side of the boundary. + #[sqlx::test(migrations = "./migrations")] + async fn deletes_only_spans_past_the_window(db: DB) -> anyhow::Result<()> { + let day = 60 * 60 * 24; + insert_span(&db, 1, 60).await; + insert_span(&db, 2, 6 * day).await; + insert_span(&db, 3, 8 * day).await; + + assert_eq!(delete_expired_otel_traces(&db, 7 * day).await, 1); + + let kept = sqlx::query_scalar!("SELECT trace_id FROM otel_traces ORDER BY trace_id") + .fetch_all(&db) + .await?; + assert_eq!(kept, vec![vec![1u8; 16], vec![2u8; 16]]); + Ok(()) + } +} + #[cfg(test)] mod log_file_listing_tests { use super::{rotated_log_files, sorted_log_files}; diff --git a/backend/tests/fixtures/inline_preview_auth.sql b/backend/tests/fixtures/inline_preview_auth.sql index 59fe2dc917..67ca598bda 100644 --- a/backend/tests/fixtures/inline_preview_auth.sql +++ b/backend/tests/fixtures/inline_preview_auth.sql @@ -2,7 +2,9 @@ -- Layered on top of `base` (which provides test-workspace and the non-operator -- `test-user-2`/SECRET_TOKEN_2). Adds an Operator member so we can assert that -- Operators cannot reach the arbitrary-code inline preview path --- (`POST /jobs/run_inline/preview`). +-- (`POST /jobs/run_inline/preview`) with their own token, plus two deployed script +-- jobs of the operator: one running, so we can assert that its WM_TOKEN can, and +-- one queued but not yet pulled, so we can assert that "queued" is not enough. INSERT INTO password(email, password_hash, login_type, super_admin, verified, name) VALUES ('operator@windmill.dev', 'not-a-real-hash', 'password', false, true, 'Operator User'); @@ -12,3 +14,11 @@ INSERT INTO usr(workspace_id, email, username, is_admin, operator, role) VALUES INSERT INTO token(token_hash, token_prefix, token, email, label, super_admin) VALUES (encode(sha256('OPERATOR_TOKEN'::bytea), 'hex'), 'OPERATOR_T', 'OPERATOR_TOKEN', 'operator@windmill.dev', 'operator token', false); + +INSERT INTO v2_job(id, workspace_id, kind, runnable_path, created_by, permissioned_as, permissioned_as_email) VALUES + ('2aa0c0de-0000-4000-8000-000000000001', 'test-workspace', 'script', 'u/test-user/deployed', 'operator-user', 'u/operator-user', 'operator@windmill.dev'), + ('2aa0c0de-0000-4000-8000-000000000002', 'test-workspace', 'script', 'u/test-user/deployed', 'operator-user', 'u/operator-user', 'operator@windmill.dev'); + +INSERT INTO v2_job_queue(id, workspace_id, scheduled_for, running) VALUES + ('2aa0c0de-0000-4000-8000-000000000001', 'test-workspace', now(), true), + ('2aa0c0de-0000-4000-8000-000000000002', 'test-workspace', now(), false); diff --git a/backend/tests/inline_preview_auth.rs b/backend/tests/inline_preview_auth.rs index 97b70fb545..8841719962 100644 --- a/backend/tests/inline_preview_auth.rs +++ b/backend/tests/inline_preview_auth.rs @@ -9,16 +9,33 @@ //! was the incomplete-fix residual of CVE-2026-22683, whose v1.615.0 patch only //! covered the entity-CRUD endpoints and left this direct inline-exec sink open. //! +//! The guard on both routes has one exemption: `wmill.datatable()` called from +//! inside a job the operator is running. Operators can only run deployed code, +//! so a request the job's WM_TOKEN authenticates comes from code a non-operator +//! authored, and the exemption is limited to the request shape the helper sends +//! (PostgreSQL against a `datatable://` database) so a leaked WM_TOKEN cannot +//! be replayed to run anything else. +//! //! This test pins down: -//! - an Operator is rejected by the operator guard (the core fix; pre-fix this -//! reached the inline executor instead of returning 401), and +//! - an Operator's own token is rejected by the operator guard (the core fix; +//! pre-fix this reached the inline executor instead of returning 401), //! - a regular non-operator passes the guard (the fix must not over-block the //! legitimate inline preview flow): in the test harness the worker inline //! utils are not registered, so a caller past the guard gets the distinct -//! "worker inline functions" error rather than the operator rejection. +//! "worker inline functions" error rather than the operator rejection, +//! - an Operator's job token passes the guard for a datatable query while its +//! job is running, on the inline route and on the `/jobs/run/preview` +//! fallback the SDKs use when the worker has no internal server, +//! - the same token is rejected for any other payload (in-process DuckDB, or a +//! `-- database` directive redirecting the query, whether written literally or +//! reached through a `WM_INTERNAL_DB` marker) and for a deferred run, +//! - an Operator's job token for a job that is not running, whether finished or +//! merely queued, is rejected. use serde_json::json; use sqlx::{Pool, Postgres}; +use windmill_common::auth::create_jwt_token; +use windmill_common::db::Authed; use windmill_test_utils::*; fn client() -> reqwest::Client { @@ -38,11 +55,65 @@ fn inline_preview_body() -> serde_json::Value { }) } +/// The request `wmill.datatable("main")` sends: PostgreSQL against `datatable://main`. +fn datatable_query_body() -> serde_json::Value { + json!({ + "language": "postgresql", + "content": "SELECT 1 AS x;", + "args": { "database": "datatable://main" } + }) +} + +/// Mint the WM_TOKEN a job hands its own code: an internally-signed job JWT +/// (note the `job_id` claim) for the fixture's operator, exactly as the worker +/// issues it when the operator runs a deployed script. +async fn operator_job_token(job_id: uuid::Uuid) -> String { + let authed = Authed { + email: "operator@windmill.dev".to_string(), + username: "operator-user".to_string(), + is_admin: false, + is_operator: true, + groups: vec![], + folders: vec![], + scopes: None, + token_prefix: None, + }; + create_jwt_token( + authed, + "test-workspace", + 3600, + Some(job_id), + Some("ephemeral-script".to_string()), + None, + None, + ) + .await + .expect("mint operator job token") +} + const OPERATOR_GUARD_MSG: &str = "Operators cannot run preview jobs"; +/// The fixture's deployed-script jobs of the operator: one running, one queued. +const RUNNING_JOB_ID: &str = "2aa0c0de-0000-4000-8000-000000000001"; +const QUEUED_JOB_ID: &str = "2aa0c0de-0000-4000-8000-000000000002"; + +async fn post(url: &str, token: &str, body: &serde_json::Value) -> (u16, String) { + let resp = authed(client().post(url), token) + .json(body) + .send() + .await + .expect("request"); + let status = resp.status().as_u16(); + let body = resp.text().await.expect("body"); + (status, body) +} + #[sqlx::test(fixtures("base", "inline_preview_auth"))] async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result<()> { initialize_tracing().await; + // The server decodes WM_TOKENs with the same in-process JWT secret, so + // setting it once lets us mint valid ones below. + set_jwt_secret().await; let server = ApiServer::start(db.clone()).await?; let port = server.addr.port(); @@ -51,12 +122,7 @@ async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result // 1. CORE REGRESSION: an Operator must be rejected by the operator guard. // Pre-fix this fell through to the inline executor (arbitrary code // execution); post-fix it returns 401 with the operator guard message. - let resp = authed(client().post(&url), "OPERATOR_TOKEN") - .json(&inline_preview_body()) - .send() - .await?; - let status = resp.status(); - let body = resp.text().await?; + let (status, body) = post(&url, "OPERATOR_TOKEN", &inline_preview_body()).await; assert_eq!( status, 401, "Operator must be rejected from inline preview (got {status}): {body}" @@ -71,12 +137,7 @@ async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result // the worker inline utils, so the request proceeds past the guard and // fails later with the distinct "worker inline functions" error — proving // the operator guard did not reject it. - let resp = authed(client().post(&url), "SECRET_TOKEN_2") - .json(&inline_preview_body()) - .send() - .await?; - let status = resp.status(); - let body = resp.text().await?; + let (status, body) = post(&url, "SECRET_TOKEN_2", &inline_preview_body()).await; assert_ne!( status, 401, "non-operator must not be blocked by the operator guard (got {status}): {body}" @@ -86,5 +147,113 @@ async fn test_inline_preview_authorization(db: Pool) -> anyhow::Result "non-operator must not hit the operator guard, got: {body}" ); + // 3. The WM_TOKEN of a deployed-script job the Operator is running passes the + // guard for a datatable query: this is `wmill.datatable()` called from + // inside that job. As in 2, the harness then fails with the "worker inline + // functions" error. + let running_job_token = + operator_job_token(uuid::Uuid::parse_str(RUNNING_JOB_ID).unwrap()).await; + let (status, body) = post(&url, &running_job_token, &datatable_query_body()).await; + assert_ne!( + status, 401, + "operator job token of a running job must pass the guard for a datatable query (got {status}): {body}" + ); + assert!( + !body.contains(OPERATOR_GUARD_MSG), + "operator job token of a running job must not hit the operator guard, got: {body}" + ); + + // 4. The same token is rejected for any other payload: the exemption covers + // the datatable request shape only, never in-process DuckDB, and never a + // `-- database` directive, which the executor honors over `args.database`. + let mut redirected = datatable_query_body(); + redirected["content"] = json!("-- database u/test-user/other_db\nSELECT 1 AS x;"); + let mut to_s3 = datatable_query_body(); + to_s3["content"] = json!("-- s3\nSELECT 1 AS x;"); + let mut resource_db = datatable_query_body(); + resource_db["args"]["database"] = json!("$res:u/test-user/other_db"); + // A marker is a single line the directive regexes cannot match; the directive only + // appears once the executor expands it, so the guard must check the expansion. + let mut marker = datatable_query_body(); + marker["content"] = json!(concat!( + r#"-- WM_INTERNAL_DB_SELECT {"table":"t","columnDefs":[{"field":"id","datatype":"int4"}],"#, + r#""whereClause":"true\n-- database u/test-user/other_db\n AND true"}"# + )); + for (label, payload) in [ + ("DuckDB", inline_preview_body()), + ("database directive", redirected), + ("s3 directive", to_s3), + ("resource database", resource_db), + ("marker-expanded database directive", marker), + ] { + let (status, body) = post(&url, &running_job_token, &payload).await; + assert_eq!( + status, 401, + "operator job token must be rejected for a {label} payload (got {status}): {body}" + ); + assert!( + body.contains(OPERATOR_GUARD_MSG), + "rejection for a {label} payload must be the operator guard, got: {body}" + ); + } + + // 5. An Operator's job token whose job is not running is rejected like the + // operator's own token, whether the job is over (no queue row) or merely + // queued: a WM_TOKEN that leaked through logs cannot be replayed once the + // job is over. + for (label, job_id) in [ + ("finished", uuid::Uuid::new_v4()), + ("queued", uuid::Uuid::parse_str(QUEUED_JOB_ID).unwrap()), + ] { + let token = operator_job_token(job_id).await; + let (status, body) = post(&url, &token, &datatable_query_body()).await; + assert_eq!( + status, 401, + "operator job token of a {label} job must be rejected (got {status}): {body}" + ); + assert!( + body.contains(OPERATOR_GUARD_MSG), + "rejection for a {label} job must be the operator guard, got: {body}" + ); + } + + // 6. The SDKs fall back to `/jobs/run/preview` when the worker has no internal + // server (agent workers). The same exemption applies there: the running + // job's token queues the datatable query (201 with the job id), the + // operator's own token is still refused. + let fallback_url = format!("http://localhost:{port}/api/w/test-workspace/jobs/run/preview"); + let (status, body) = post(&fallback_url, &running_job_token, &datatable_query_body()).await; + assert_eq!( + status, 201, + "operator job token of a running job must queue a datatable preview (got {status}): {body}" + ); + let (status, body) = post(&fallback_url, "OPERATOR_TOKEN", &datatable_query_body()).await; + assert_eq!( + status, 401, + "Operator must be rejected from the preview fallback (got {status}): {body}" + ); + assert!( + body.contains(OPERATOR_GUARD_MSG), + "rejection must be the operator guard, got: {body}" + ); + + // 7. A deferred run on the fallback would outlive the running job the + // exemption keys off, so the running job's token cannot schedule one. + for deferral in [ + "scheduled_in_secs=86400", + "scheduled_for=2099-01-01T00:00:00Z", + ] { + let deferred_url = format!("{fallback_url}?{deferral}"); + let (status, body) = post(&deferred_url, &running_job_token, &datatable_query_body()).await; + assert_eq!( + status, 401, + "operator job token must not schedule a deferred preview with {deferral} (got {status}): {body}" + ); + assert!( + body.contains(OPERATOR_GUARD_MSG), + "rejection for {deferral} must be the operator guard, got: {body}" + ); + } + Ok(()) } diff --git a/backend/tests/object_storage_test_ssrf.rs b/backend/tests/object_storage_test_ssrf.rs new file mode 100644 index 0000000000..aa9eaaf74c --- /dev/null +++ b/backend/tests/object_storage_test_ssrf.rs @@ -0,0 +1,97 @@ +//! `POST /api/settings/test_object_storage_config` runs the probe on the API server and reflects the +//! upstream response, so every non-super-admin must be rejected for private/loopback endpoints and +//! the Filesystem backend on every deployment (`CLOUD_HOSTED` is unset here), while a super admin's +//! Filesystem probe still round-trips. Requires the `parquet` feature, like the route. +#![cfg(feature = "parquet")] + +use serde_json::json; +use sqlx::{Pool, Postgres}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Arc; +use windmill_test_utils::*; + +const SUPER_ADMIN_TOKEN: &str = "SECRET_TOKEN"; +const USER_TOKEN: &str = "SECRET_TOKEN_2"; + +async fn test_object_storage( + url: &str, + token: &str, + body: serde_json::Value, +) -> anyhow::Result<(u16, String)> { + let resp = reqwest::Client::new() + .post(url) + .header("Authorization", format!("Bearer {token}")) + .json(&body) + .send() + .await?; + Ok((resp.status().as_u16(), resp.text().await?)) +} + +#[sqlx::test(fixtures("base"))] +async fn object_storage_test_is_restricted_for_non_super_admins_off_cloud( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let url = format!( + "http://localhost:{}/api/settings/test_object_storage_config", + server.addr.port() + ); + + // A loopback "S3 endpoint" standing in for an internal service: the probe must be rejected + // before the server opens a connection to it. + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?; + let internal_port = listener.local_addr()?.port(); + let connected = Arc::new(AtomicBool::new(false)); + tokio::spawn({ + let connected = connected.clone(); + async move { + while listener.accept().await.is_ok() { + connected.store(true, Ordering::SeqCst); + } + } + }); + let internal_s3 = json!({ + "type": "S3", + "bucket": "bucket", + "region": "us-east-1", + "access_key": "key", + "secret_key": "secret", + "endpoint": format!("http://127.0.0.1:{internal_port}"), + "allow_http": true, + "path_style": true, + }); + let (status, body) = test_object_storage(&url, USER_TOKEN, internal_s3).await?; + assert_eq!( + status, 401, + "non-super-admin must be rejected for a loopback endpoint (got {status}): {body}" + ); + assert!( + body.contains("requires a super admin"), + "unexpected rejection: {body}" + ); + assert!( + !connected.load(Ordering::SeqCst), + "the server must not connect to the rejected endpoint" + ); + + let tmp = tempfile::tempdir()?; + let filesystem = json!({ "type": "Filesystem", "root_path": tmp.path().to_str().unwrap() }); + let (status, body) = test_object_storage(&url, USER_TOKEN, filesystem.clone()).await?; + assert_eq!( + status, 401, + "non-super-admin must be rejected for a Filesystem backend (got {status}): {body}" + ); + assert!( + body.contains("requires a super admin"), + "unexpected rejection: {body}" + ); + + // Super admins keep the unrestricted path. + let (status, body) = test_object_storage(&url, SUPER_ADMIN_TOKEN, filesystem).await?; + assert_eq!( + status, 200, + "super admin must be able to test a Filesystem backend (got {status}): {body}" + ); + Ok(()) +} diff --git a/backend/tests/relock_noop.rs b/backend/tests/relock_noop.rs new file mode 100644 index 0000000000..5cb8e3f51e --- /dev/null +++ b/backend/tests/relock_noop.rs @@ -0,0 +1,415 @@ +use sqlx::{Pool, Postgres}; +use tokio_stream::StreamExt; +use windmill_api_client::types::NewScript; +use windmill_common::scripts::{deploy_relocked_version, fetch_script_for_update}; +use windmill_test_utils::*; + +const W: &str = "test-workspace"; + +const A: &str = r#"export async function main() { return "a" }"#; +const A_COMMENTED: &str = r#"// same dependencies, different content +export async function main() { return "a" }"#; +const A_WITH_LODASH: &str = r#"import _ from "lodash@4.17.21"; +export async function main() { return _.trim(" a ") }"#; +const B: &str = r#"import { main as a } from "/f/rel/a.ts"; +export async function main() { return "b" + (await a()) }"#; +const C: &str = r#"import { main as b } from "/f/rel/b.ts"; +export async function main() { return "c" + (await b()) }"#; + +fn bun_script(path: &str, content: &str, parent_hash: Option) -> NewScript { + NewScript { + draft_only: None, + content: content.into(), + language: windmill_api_client::types::ScriptLang::Bun, + lock: None, + parent_hash, + path: path.into(), + concurrent_limit: None, + concurrency_time_window_s: None, + cache_ttl: None, + dedicated_worker: None, + description: "".to_string(), + envs: vec![], + is_template: None, + kind: None, + summary: "".to_string(), + tag: None, + schema: std::collections::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, + } +} + +#[derive(sqlx::FromRow, Debug)] +struct Version { + hash: i64, + archived: bool, + lock: Option, + created_at: chrono::DateTime, +} + +/// Every version of `path`, oldest first. +async fn versions(db: &Pool, path: &str) -> Vec { + sqlx::query_as( + "SELECT hash, archived, lock, created_at FROM script + WHERE workspace_id = $1 AND path = $2 ORDER BY created_at", + ) + .bind(W) + .bind(path) + .fetch_all(db) + .await + .unwrap() +} + +fn live(versions: &[Version]) -> &Version { + versions.iter().rev().find(|v| !v.archived).unwrap() +} + +/// `(path, status, logs)` of every dependency job created after `since`, in completion order. +async fn dependency_jobs_since( + db: &Pool, + since: chrono::DateTime, +) -> Vec<(String, String, String)> { + sqlx::query_as( + "SELECT j.runnable_path, c.status::text, COALESCE(l.logs, '') FROM v2_job_completed c + JOIN v2_job j ON j.id = c.id + LEFT JOIN job_logs l ON l.job_id = c.id + WHERE j.kind = 'dependencies' AND j.created_at > $1 + ORDER BY c.started_at", + ) + .bind(since) + .fetch_all(db) + .await + .unwrap() +} + +async fn wait_for_jobs( + completed: &mut (impl futures::Stream + Unpin), + count: usize, +) { + for _ in 0..count { + completed.next().await; + } + // Then let anything else that was queued run out, so a job the assertions say must not + // exist would have shown up here. + while let Ok(Some(_)) = + tokio::time::timeout(std::time::Duration::from_secs(2), completed.next()).await + {} +} + +/// A redeploy of an imported script whose dependencies did not move relocks its importer, +/// and that relock must deploy nothing: no new version, and no dependency job for the +/// importer's own importers. A redeploy that does change the dependencies still walks the +/// whole chain with a new version at each step. +#[sqlx::test(fixtures("base"))] +async fn relative_import_relock_deploys_only_when_the_lock_changed( + db: Pool, +) -> anyhow::Result<()> { + std::env::set_var("DEPENDENCY_JOB_DEBOUNCE_DELAY", "0"); + let (client, port, _s) = init_client(db.clone()).await; + let mut completed = listen_for_completed_jobs(&db).await; + + in_test_worker( + &db, + async { + // One at a time: each deploy's dependency job records the importer's edges, and an + // importer whose edges are recorded is what a later relock of it can skip on. + for (path, content) in [("f/rel/a", A), ("f/rel/b", B), ("f/rel/c", C)] { + client + .create_script(W, &bun_script(path, content, None)) + .await + .unwrap(); + wait_for_jobs(&mut completed, 1).await; + } + let b_before = versions(&db, "f/rel/b").await; + let c_before = versions(&db, "f/rel/c").await; + assert_eq!(b_before.len(), 1); + assert_eq!(c_before.len(), 1); + + // Content-only change on the leaf. + let since = chrono::Utc::now(); + let a_hash = live(&versions(&db, "f/rel/a").await).hash; + client + .create_script( + W, + &bun_script("f/rel/a", A_COMMENTED, Some(format!("{a_hash:016x}"))), + ) + .await + .unwrap(); + wait_for_jobs(&mut completed, 2).await; + + let jobs = dependency_jobs_since(&db, since).await; + let paths: Vec<&str> = jobs.iter().map(|(p, _, _)| p.as_str()).collect(); + assert_eq!( + paths, + ["f/rel/a", "f/rel/b"], + "the leaf's own job and one no-op relock of its importer, and nothing for c" + ); + assert!( + jobs[1] + .2 + .contains("Lock unchanged: no new version deployed"), + "b's relock should have found its lock unchanged: {}", + jobs[1].2 + ); + let b_after = versions(&db, "f/rel/b").await; + let c_after = versions(&db, "f/rel/c").await; + assert_eq!( + b_after.len(), + 1, + "an unchanged relock must not mint a version" + ); + assert_eq!(live(&b_after).hash, live(&b_before).hash); + assert_eq!(c_after.len(), 1); + assert_eq!(live(&c_after).hash, live(&c_before).hash); + + // A dependency change on the leaf. + let since = chrono::Utc::now(); + let a_hash = live(&versions(&db, "f/rel/a").await).hash; + client + .create_script( + W, + &bun_script("f/rel/a", A_WITH_LODASH, Some(format!("{a_hash:016x}"))), + ) + .await + .unwrap(); + wait_for_jobs(&mut completed, 3).await; + + let jobs = dependency_jobs_since(&db, since).await; + let paths: Vec<&str> = jobs.iter().map(|(p, _, _)| p.as_str()).collect(); + assert_eq!(paths, ["f/rel/a", "f/rel/b", "f/rel/c"]); + for path in ["f/rel/b", "f/rel/c"] { + let vs = versions(&db, path).await; + assert_eq!( + vs.len(), + 2, + "{path}: a changed relock deploys a new version" + ); + assert!( + vs[0].archived && !vs[1].archived, + "{path}: parent archived, child live" + ); + assert!(vs[0].created_at < vs[1].created_at, "{path}: lineage order"); + assert!( + vs[1].lock.as_deref().unwrap_or("").contains("lodash"), + "{path}: the new version carries the new lock: {:?}", + vs[1].lock + ); + } + }, + port, + ) + .await; + + Ok(()) +} + +/// A relock that has to wait on its head's row lock, because a deploy of the same path holds +/// it, must find the version that deploy left and requeue itself for it rather than fail. The +/// blocked statement re-checks only the row it selected, which the deploy archived, and comes +/// back empty; the successor is only visible to a fresh read. +#[sqlx::test(fixtures("base"))] +async fn relock_waiting_on_a_deploy_requeues_for_its_successor( + db: Pool, +) -> anyhow::Result<()> { + std::env::set_var("DEPENDENCY_JOB_DEBOUNCE_DELAY", "0"); + let (client, port, _s) = init_client(db.clone()).await; + let mut completed = listen_for_completed_jobs(&db).await; + + in_test_worker( + &db, + async { + for (path, content) in [("f/rel/a", A), ("f/rel/b", B)] { + client + .create_script(W, &bun_script(path, content, None)) + .await + .unwrap(); + wait_for_jobs(&mut completed, 1).await; + } + + // A deploy of b that holds its head's row lock for as long as this transaction lives. + let mut deploy = db.begin().await.unwrap(); + let head = fetch_script_for_update("f/rel/b", W, &mut *deploy) + .await + .unwrap() + .unwrap(); + + let since = chrono::Utc::now(); + let a_hash = live(&versions(&db, "f/rel/a").await).hash; + client + .create_script( + W, + &bun_script("f/rel/a", A_COMMENTED, Some(format!("{a_hash:016x}"))), + ) + .await + .unwrap(); + + // b's relock skips generation and reaches its commit, where it waits on the lock. + let mut waiting = false; + for _ in 0..300 { + waiting = sqlx::query_scalar( + "SELECT EXISTS (SELECT 1 FROM pg_stat_activity + WHERE datname = current_database() AND wait_event_type = 'Lock' + AND query LIKE '%FROM script WHERE path = $1%FOR UPDATE%')", + ) + .fetch_one(&db) + .await + .unwrap(); + if waiting { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + assert!(waiting, "b's relock never reached the row lock"); + + // The deploy lands: the head is archived and a successor with its own lock takes + // its place, while the relock is still waiting. + let lock = head.lock.clone().unwrap(); + let successor = + deploy_relocked_version(&mut deploy, head, None, Some(&lock), None, None) + .await + .unwrap(); + deploy.commit().await.unwrap(); + + // a's own job, the relock that waited, and the relock it queued for the successor. + wait_for_jobs(&mut completed, 3).await; + + let jobs = dependency_jobs_since(&db, since).await; + let paths: Vec<&str> = jobs.iter().map(|(p, _, _)| p.as_str()).collect(); + assert_eq!(paths, ["f/rel/a", "f/rel/b", "f/rel/b"], "{jobs:?}"); + assert!( + jobs.iter().all(|(_, status, _)| status == "success"), + "no relock may fail on the wait: {jobs:?}" + ); + assert!( + jobs[1] + .2 + .contains("was deployed while this lock was generated"), + "the waiting relock should have seen the successor: {}", + jobs[1].2 + ); + assert!( + jobs[2] + .2 + .contains("Lock unchanged: no new version deployed"), + "the requeued relock should find the successor's lock current: {}", + jobs[2].2 + ); + let vs = versions(&db, "f/rel/b").await; + assert_eq!( + vs.len(), + 2, + "the deploy's successor and nothing else: {vs:?}" + ); + assert_eq!(live(&vs).hash, successor); + }, + port, + ) + .await; + + Ok(()) +} + +/// A multi-file importer: on a skipped relock each module gets its own last lock back, not the +/// parent script's, so an import's content-only redeploy leaves the importer alone as well. +#[cfg(feature = "python")] +#[sqlx::test(fixtures("base"))] +async fn multi_file_importer_relock_is_a_no_op_too(db: Pool) -> anyhow::Result<()> { + std::env::set_var("DEPENDENCY_JOB_DEBOUNCE_DELAY", "0"); + let (client, port, _s) = init_client(db.clone()).await; + let mut completed = listen_for_completed_jobs(&db).await; + + let py = |path: &str, content: &str, parent_hash: Option, with_module: bool| { + let mut ns = bun_script(path, content, parent_hash); + ns.language = windmill_api_client::types::ScriptLang::Python3; + if with_module { + ns.modules = Some(std::collections::HashMap::from([( + "helper.py".to_string(), + serde_json::json!({ + "content": "def greet(x):\n return 'hi ' + x\n", + "language": "python3" + }), + )])); + } + ns + }; + async fn module_lock(db: &Pool) -> Option { + sqlx::query_scalar( + "SELECT modules->'helper.py'->>'lock' FROM script + WHERE workspace_id = $1 AND path = 'f/rel/pb' AND archived = false", + ) + .bind(W) + .fetch_one(db) + .await + .unwrap() + } + + in_test_worker( + &db, + async { + client + .create_script(W, &py("f/rel/pa", "def main():\n return 'a'\n", None, false)) + .await + .unwrap(); + wait_for_jobs(&mut completed, 1).await; + client + .create_script( + W, + &py( + "f/rel/pb", + "from f.rel.pa import main as a\nfrom .helper import greet\n\ndef main():\n return greet(a())\n", + None, + true, + ), + ) + .await + .unwrap(); + wait_for_jobs(&mut completed, 1).await; + let lock_before = module_lock(&db).await; + assert!(lock_before.is_some(), "the module got a lock of its own on deploy"); + + let since = chrono::Utc::now(); + let pa_hash = live(&versions(&db, "f/rel/pa").await).hash; + client + .create_script( + W, + &py( + "f/rel/pa", + "# same dependencies\ndef main():\n return 'a'\n", + Some(format!("{pa_hash:016x}")), + false, + ), + ) + .await + .unwrap(); + wait_for_jobs(&mut completed, 2).await; + + let jobs = dependency_jobs_since(&db, since).await; + let paths: Vec<&str> = jobs.iter().map(|(p, _, _)| p.as_str()).collect(); + assert_eq!(paths, ["f/rel/pa", "f/rel/pb"], "{jobs:?}"); + assert!( + jobs[1].2.contains("Lock unchanged: no new version deployed"), + "the multi-file importer's relock should be a no-op: {}", + jobs[1].2 + ); + assert_eq!(versions(&db, "f/rel/pb").await.len(), 1); + assert_eq!(module_lock(&db).await, lock_before, "the module keeps its own lock"); + }, + port, + ) + .await; + + Ok(()) +} diff --git a/backend/tests/relock_skip.rs b/backend/tests/relock_skip.rs index 8262a38cf4..bf5cb24cbf 100644 --- a/backend/tests/relock_skip.rs +++ b/backend/tests/relock_skip.rs @@ -266,7 +266,10 @@ def main(): .await .unwrap(); - in_test_worker(&db, wait_for_jobs_ge(&mut completed, 10), port).await; + // Empty content leaves every importer's lock as it was, so only the five direct + // importers of the default deps run a job: an unchanged script relock deploys no + // version and so queues nothing for its own importers. + in_test_worker(&db, wait_for_jobs_ge(&mut completed, 5), port).await; // Note: within a cascade, the same script may be triggered multiple times. // After the first trigger relocks and stores the hash, subsequent triggers skip. @@ -295,7 +298,7 @@ def main(): .await .unwrap(); - in_test_worker(&db, wait_for_jobs_ge(&mut completed, 10), port).await; + in_test_worker(&db, wait_for_jobs_ge(&mut completed, 5), port).await; let skipping_count = count_pattern_in_job_logs(&db, "Skipping relock", before).await; assert!( diff --git a/backend/windmill-api-integration-tests/tests/mcp_preprocessor_headers.rs b/backend/windmill-api-integration-tests/tests/mcp_preprocessor_headers.rs new file mode 100644 index 0000000000..3e1035ff82 --- /dev/null +++ b/backend/windmill-api-integration-tests/tests/mcp_preprocessor_headers.rs @@ -0,0 +1,186 @@ +//! Request headers reaching a runnable's preprocessor over MCP. +//! +//! The property this pins is structural rather than a filter: the model writes +//! the tool's arguments, which become `event.body`, while the server writes +//! `event.headers`. A model that guesses a header's name can only ever land in +//! `body`, so an identity read from `headers` is one prompt injection cannot +//! forge. Nothing else in the suite exercises MCP argument shaping end to end. +//! +//! Requires: bun runtime, live database (migrations applied by sqlx::test). +#![cfg(feature = "mcp")] + +use serde_json::{json, Value}; +use sqlx::{Pool, Postgres}; +use windmill_test_utils::*; + +const SCRIPT_PATH: &str = "u/test-user/mcp_hdr_probe"; + +/// Echoes the two halves of the event separately, so the assertions can tell +/// which one a value arrived in. +const PREPROCESSOR_SCRIPT: &str = r#" +export async function preprocessor(event: any) { + return { + kind: event.kind, + from_headers: event.headers?.["x-user-id"] ?? "", + from_body: event.body?.x_user_id ?? "", + header_names: Object.keys(event.headers ?? {}).sort(), + }; +} + +export async function main(kind: string, from_headers: string, from_body: string, header_names: string[]) { + return { kind, from_headers, from_body, header_names }; +} +"#; + +async fn insert_mcp_token(db: &Pool) -> anyhow::Result<()> { + sqlx::query( + "INSERT INTO token (token_hash, token_prefix, token, email, label, super_admin, scopes) + VALUES (encode(sha256('MCP_TOKEN'::bytea), 'hex'), 'MCP_TOK', 'MCP_TOKEN', 'test@windmill.dev', 'mcp token', true, ARRAY['mcp:all'])", + ) + .execute(db) + .await?; + Ok(()) +} + +/// POST one JSON-RPC message. The endpoint answers either `application/json` or +/// a single-event SSE stream, so strip the `data: ` framing before parsing. +async fn mcp_post(port: u16, headers: &[(&str, &str)], body: Value) -> anyhow::Result { + let mut req = reqwest::Client::new() + .post(format!( + "http://localhost:{port}/api/mcp/w/test-workspace/mcp" + )) + .header("Authorization", "Bearer MCP_TOKEN") + .header("Accept", "application/json, text/event-stream") + .json(&body); + for (k, v) in headers { + req = req.header(*k, *v); + } + let text = req.send().await?.text().await?; + let payload = text + .lines() + .find_map(|l| l.strip_prefix("data: ")) + .unwrap_or(text.trim()); + serde_json::from_str(payload).map_err(|e| anyhow::anyhow!("unparseable MCP body {text:?}: {e}")) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_mcp_preprocessor_receives_the_callers_headers( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + set_jwt_secret().await; + insert_mcp_token(&db).await?; + let server = ApiServer::start_mcp(db.clone()).await?; + let port = server.addr.port(); + + let resp = reqwest::Client::new() + .post(format!( + "http://localhost:{port}/api/w/test-workspace/scripts/create" + )) + .header("Authorization", "Bearer SECRET_TOKEN") + .json(&json!({ + "path": SCRIPT_PATH, + "summary": "mcp header probe", + "description": "", + "content": PREPROCESSOR_SCRIPT, + "language": "bun", + "lock": "", + "schema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "properties": { "x_user_id": { "type": "string" } }, + "required": [] + } + })) + .send() + .await?; + assert_eq!( + resp.status(), + 201, + "create script: {}", + resp.text().await.unwrap_or_default() + ); + + // A script counts as deployed once it has a lock, which normally arrives from + // a dependency job. Planting an empty one keeps the test to the path under + // test instead of a bun resolution whose timing it does not control. + sqlx::query("UPDATE script SET lock = '' WHERE path = $1 AND workspace_id = 'test-workspace'") + .bind(SCRIPT_PATH) + .execute(&db) + .await?; + + let tools = mcp_post( + port, + &[], + json!({"jsonrpc": "2.0", "id": 1, "method": "tools/list", "params": {}}), + ) + .await?; + let tool_name = tools["result"]["tools"] + .as_array() + .and_then(|list| { + list.iter() + .filter_map(|t| t["name"].as_str()) + .find(|n| n.contains("mcp__hdr__probe")) + }) + .ok_or_else(|| anyhow::anyhow!("the deployed script was not listed as a tool: {tools}"))? + .to_string(); + + let result = in_test_worker( + db.clone(), + async { + mcp_post( + port, + // Every name the withheld list covers has to be on the wire, or + // asserting its absence proves nothing. `Authorization` is already + // set by `mcp_post`, and `extract_token` reads it before the + // cookie, so sending one does not disturb auth. + &[ + ("X-User-Id", "alice@corp.example"), + ("Cookie", "session=secret"), + ("Proxy-Authorization", "Basic Zm9v"), + ], + json!({ + "jsonrpc": "2.0", "id": 2, "method": "tools/call", + // The model names the header it wants to spoof. Its value is an + // argument, so it can only ever reach `event.body`. + "params": { "name": tool_name, "arguments": { "x_user_id": "attacker@evil.test" } } + }), + ) + .await + }, + port, + ) + .await?; + + let text = result["result"]["content"][0]["text"] + .as_str() + .ok_or_else(|| anyhow::anyhow!("tool call returned no text content: {result}"))?; + let out: Value = serde_json::from_str(text)?; + + assert_eq!(out["kind"], "mcp", "preprocessor event kind: {out}"); + assert_eq!( + out["from_headers"], "alice@corp.example", + "the caller's header must reach event.headers: {out}" + ); + assert_eq!( + out["from_body"], "attacker@evil.test", + "the model's argument must land in event.body, not overwrite the header: {out}" + ); + + let names: Vec<&str> = out["header_names"] + .as_array() + .map(|a| a.iter().filter_map(|v| v.as_str()).collect()) + .unwrap_or_default(); + assert!( + names.contains(&"x-user-id"), + "event.headers must carry the request's own headers: {names:?}" + ); + for withheld in ["authorization", "cookie", "proxy-authorization"] { + assert!( + !names.contains(&withheld), + "{withheld} is withheld from a preprocessor: {names:?}" + ); + } + + Ok(()) +} diff --git a/backend/windmill-api-integration-tests/tests/scripts.rs b/backend/windmill-api-integration-tests/tests/scripts.rs index 8c88c53454..3a146add27 100644 --- a/backend/windmill-api-integration-tests/tests/scripts.rs +++ b/backend/windmill-api-integration-tests/tests/scripts.rs @@ -38,6 +38,85 @@ fn new_script(path: &str, summary: &str, content: &str) -> serde_json::Value { }) } +/// A supplied lock queues no dependency job, so if the create does not record its hash nothing +/// ever will, and every importer of this script relocks on each of its deploys forever after. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_create_script_persists_supplied_lock_hash(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let path = "u/test-user/supplied_lock"; + let lock = r#"{"version":"4","remote":{}}"#; + let mut script = new_script( + path, + "Supplied lock", + "export async function main() { return 42; }", + ); + script["lock"] = json!(lock); + + let resp = authed(client().post(format!( + "http://localhost:{port}/api/w/test-workspace/scripts/create" + ))) + .json(&script) + .send() + .await?; + assert_eq!(resp.status(), 201, "create: {}", resp.text().await?); + + let stored_hash = sqlx::query_scalar!( + "SELECT lockfile_hash FROM lock_hash WHERE workspace_id = $1 AND path = $2", + "test-workspace", + path, + ) + .fetch_one(&db) + .await?; + assert_eq!(stored_hash, windmill_common::scripts::hash_script(lock)); + + // A script deployed before the create recorded hashes has no row, and pushing it unchanged + // creates no version to hang one off. Without the write on that path it would keep its + // importers relocking until someone edited it. + sqlx::query!( + "DELETE FROM lock_hash WHERE workspace_id = $1 AND path = $2", + "test-workspace", + path, + ) + .execute(&db) + .await?; + + // The no-op comparison covers every field, so the push has to carry what the first deploy + // filled in by itself; `auto_parent` both resolves the parent and keeps the hash distinct. + script["auto_parent"] = json!(true); + script["ws_error_handler_muted"] = json!(false); + script["assets"] = json!([]); + let resp = authed(client().post(format!( + "http://localhost:{port}/api/w/test-workspace/scripts/create?skip_if_noop=true" + ))) + .json(&script) + .send() + .await?; + assert_eq!(resp.status(), 201, "no-op push: {}", resp.text().await?); + + let versions: i64 = sqlx::query_scalar!( + "SELECT COUNT(*) FROM script WHERE workspace_id = $1 AND path = $2", + "test-workspace", + path, + ) + .fetch_one(&db) + .await? + .unwrap_or_default(); + assert_eq!(versions, 1, "no-op push must not create a version"); + + let repaired_hash = sqlx::query_scalar!( + "SELECT lockfile_hash FROM lock_hash WHERE workspace_id = $1 AND path = $2", + "test-workspace", + path, + ) + .fetch_one(&db) + .await?; + assert_eq!(repaired_hash, windmill_common::scripts::hash_script(lock)); + + Ok(()) +} + #[sqlx::test(migrations = "../migrations", fixtures("base"))] async fn test_script_endpoints(db: Pool) -> anyhow::Result<()> { initialize_tracing().await; @@ -797,10 +876,12 @@ async fn test_update_script_reports_losing_to_a_concurrent_deploy( // What a deploy leaves behind: the old head archived, a new one live at the path. // Copied through a temp table so this does not have to restate every column. - sqlx::query("CREATE TEMP TABLE superseding ON COMMIT DROP AS SELECT * FROM script WHERE hash = $1") - .bind(head) - .execute(&mut *winner) - .await?; + sqlx::query( + "CREATE TEMP TABLE superseding ON COMMIT DROP AS SELECT * FROM script WHERE hash = $1", + ) + .bind(head) + .execute(&mut *winner) + .await?; sqlx::query("UPDATE superseding SET hash = $1, archived = false, parent_hashes = ARRAY[$2]") .bind(head + 1) .bind(head) @@ -818,7 +899,10 @@ async fn test_update_script_reports_losing_to_a_concurrent_deploy( let resp = tokio::time::timeout(std::time::Duration::from_secs(20), update).await??; let status = resp.status(); let body = resp.text().await?; - assert_eq!(status, 400, "losing the race should not read as success: {body}"); + assert_eq!( + status, 400, + "losing the race should not read as success: {body}" + ); assert!( body.contains("deployed to concurrently"), "the loser must say it was superseded, not that the script is missing: {body}" diff --git a/backend/windmill-api-integration-tests/tests/workspaces.rs b/backend/windmill-api-integration-tests/tests/workspaces.rs index 65fe4241cc..25312cae27 100644 --- a/backend/windmill-api-integration-tests/tests/workspaces.rs +++ b/backend/windmill-api-integration-tests/tests/workspaces.rs @@ -889,6 +889,53 @@ async fn test_get_copilot_info_ignores_empty_instance_ai_row( Ok(()) } +/// A workspace with no provider of its own is served the instance config, but the +/// `copilot_disabled` flag must still come from the workspace's own row. +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_get_copilot_info_keeps_workspace_copilot_disabled_over_instance_fallback( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let base = format!("http://localhost:{port}/api/w/test-workspace/workspaces"); + + sqlx::query("UPDATE workspace_settings SET ai_config = $1 WHERE workspace_id = $2") + .bind(json!({ "copilot_disabled": true })) + .bind("test-workspace") + .execute(&db) + .await?; + sqlx::query( + "INSERT INTO global_settings (name, value) VALUES ($1, $2) \ + ON CONFLICT (name) DO UPDATE SET value = EXCLUDED.value", + ) + .bind("ai_config") + .bind(json!({ + "providers": { + "openai": { + "resource_path": "u/test-user/openai_instance", + "models": ["gpt-4o-mini"] + } + } + })) + .execute(&db) + .await?; + + let resp = authed(client().get(format!("{base}/get_copilot_info"))) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 200); + let settings = resp.json::().await?; + assert_eq!( + settings["providers"]["openai"]["models"][0], "gpt-4o-mini", + "instance providers are still served" + ); + assert_eq!(settings["copilot_disabled"], true); + + Ok(()) +} + #[sqlx::test(migrations = "../migrations", fixtures("base"))] async fn test_error_handler_instance_alerts_fallback(db: Pool) -> anyhow::Result<()> { initialize_tracing().await; @@ -941,7 +988,12 @@ async fn test_error_handler_instance_alerts_fallback(db: Pool) -> anyh .send() .await .unwrap(); - assert_eq!(resp.status(), 200, "disable on fork: {}", resp.text().await?); + assert_eq!( + resp.status(), + 200, + "disable on fork: {}", + resp.text().await? + ); assert!(!stored().await?); Ok(()) @@ -1044,9 +1096,11 @@ async fn test_create_service_account_drops_orphaned_group_memberships( .await?; // Same username, different workspace, and very much alive — must not be touched. - sqlx::query("INSERT INTO workspace (id, name, owner) VALUES ('other-workspace', 'other', 'svc_acct')") - .execute(&db) - .await?; + sqlx::query( + "INSERT INTO workspace (id, name, owner) VALUES ('other-workspace', 'other', 'svc_acct')", + ) + .execute(&db) + .await?; sqlx::query( "INSERT INTO group_ (workspace_id, name, summary) VALUES ('other-workspace', 'all', 'All users'), diff --git a/backend/windmill-api-jobs/src/execution.rs b/backend/windmill-api-jobs/src/execution.rs index fbfd656cdf..e897bf5eaf 100644 --- a/backend/windmill-api-jobs/src/execution.rs +++ b/backend/windmill-api-jobs/src/execution.rs @@ -416,11 +416,31 @@ pub fn result_to_response(result: Box, success: bool) -> error::Result let mut headers = HeaderMap::new(); + // A reverse proxy consumes hop-by-hop headers instead of forwarding them and + // drops every header named by `Connection`, so a script could use one to strip + // the sandbox headers this function adds before they reach the browser. + const HOP_BY_HOP_HEADERS: [&str; 9] = [ + "connection", + "keep-alive", + "proxy-authenticate", + "proxy-authorization", + "proxy-connection", + "te", + "trailer", + "transfer-encoding", + "upgrade", + ]; + if let Some(windmill_headers) = windmill_headers { for (k, v) in windmill_headers { let k = HeaderName::from_str(k.as_str()).map_err(|err| { Error::internal_err(format!("Invalid header name {k}: {err}")) })?; + if HOP_BY_HOP_HEADERS.contains(&k.as_str()) { + return Err(Error::ExecutionErr(format!( + "windmill_headers cannot set the hop-by-hop header \"{k}\"" + ))); + } let v = HeaderValue::from_str(v.as_str()).map_err(|err| { Error::internal_err(format!("Invalid header value {v}: {err}")) })?; @@ -428,6 +448,22 @@ pub fn result_to_response(result: Box, success: bool) -> error::Result } } + // The script controls the content type and body, and run_wait_result and sync + // HTTP routes are reachable by top-level GET navigation with the session cookie: + // sandbox the document into an opaque origin so HTML can never run with the + // viewer's session. Inserted after `wm_headers` so a script cannot override it. + headers.insert( + http::header::X_CONTENT_TYPE_OPTIONS, + HeaderValue::from_static("nosniff"), + ); + headers.insert( + http::header::CONTENT_SECURITY_POLICY, + HeaderValue::from_static( + "sandbox allow-scripts allow-forms allow-popups \ + allow-popups-to-escape-sandbox allow-downloads allow-modals", + ), + ); + if let Some(content_type) = windmill_content_type { let serialized_json_result = result_value .map(|val| val.get().to_owned()) @@ -1104,6 +1140,56 @@ mod result_to_response_tests { resp.headers().get(http::header::CONTENT_TYPE).unwrap(), "text/html" ); + assert_sandboxed(resp.headers()); assert_eq!(body_bytes(resp).await, b"

hi

"); } + + fn assert_sandboxed(headers: &HeaderMap) { + assert_eq!( + headers.get(http::header::X_CONTENT_TYPE_OPTIONS).unwrap(), + "nosniff" + ); + let csp = headers + .get(http::header::CONTENT_SECURITY_POLICY) + .expect("content-security-policy") + .to_str() + .unwrap(); + assert!(csp.starts_with("sandbox "), "csp: {csp}"); + assert!(!csp.contains("allow-same-origin"), "csp: {csp}"); + } + + #[tokio::test] + async fn custom_headers_cannot_override_sandbox() { + // wm_headers is script-controlled: a content-type set there replaces the JSON + // one even without wm_content_type, and the sandbox headers must survive an + // attempt to override them. + let resp = result_to_response( + raw( + r#"{"wm_headers":{"content-type":"text/html","content-security-policy":"default-src *","x-content-type-options":"none"},"result":"

hi

"}"#, + ), + true, + ) + .expect("response"); + + assert_eq!( + resp.headers().get(http::header::CONTENT_TYPE).unwrap(), + "text/html" + ); + assert_sandboxed(resp.headers()); + } + + #[tokio::test] + async fn hop_by_hop_custom_headers_are_rejected() { + // A proxy drops every header named by `Connection`, which would strip the + // sandbox headers on the way to the browser. + for name in ["connection", "Connection", "transfer-encoding", "upgrade"] { + let res = result_to_response( + raw(&format!( + r#"{{"wm_content_type":"text/html","wm_headers":{{"{name}":"content-security-policy, x-content-type-options"}},"result":"

hi

"}}"# + )), + true, + ); + assert!(res.is_err(), "hop-by-hop header must be rejected: {name}"); + } + } } diff --git a/backend/windmill-api-scripts/src/scripts.rs b/backend/windmill-api-scripts/src/scripts.rs index 817ff6fc07..ccc5815aa7 100644 --- a/backend/windmill-api-scripts/src/scripts.rs +++ b/backend/windmill-api-scripts/src/scripts.rs @@ -39,7 +39,7 @@ use sqlx::{FromRow, Postgres, Transaction}; use std::{collections::HashMap, sync::Arc}; use windmill_audit::audit_oss::{audit_log, AuditAuthorable}; use windmill_audit::ActionKind; -use windmill_dep_map::process_relative_imports; +use windmill_dep_map::{lock_hash::record_lock_hashes, process_relative_imports}; use windmill_dep_map::scoped_dependency_map::ScopedDependencyMap; use windmill_common::{ @@ -1073,6 +1073,14 @@ fn modules_eq( } } +/// Recorded for the empty lock a codebase or a language with no lock generation carries as well as +/// for a real one: the worker writes `hash_script("")` in the same situation, and a path going from +/// a real lock to an empty one has to stop matching what its importers recorded, or they wrongly +/// skip rather than merely relock too often. +fn lock_hash_entry(path: &str, lock: &str) -> [(String, i64); 1] { + [(path.to_string(), hash_script(lock))] +} + async fn create_script_internal<'c>( mut ns: NewScript, w_id: String, @@ -1340,6 +1348,12 @@ async fn create_script_internal<'c>( parent_hash = %p_hash.0, "Skipping no-op script deploy (identical to parent)" ); + // The version is unchanged, but the row recording its lock's hash may never have + // been written — nothing else writes it for a supplied lock, and a path only ever + // pushed unchanged would otherwise keep its importers relocking forever. + if let Some(lock) = ps.lock.as_deref() { + record_lock_hashes(&mut tx, &w_id, &lock_hash_entry(&ns.path, lock)).await?; + } return Ok((p_hash.clone(), tx, None, Vec::new())); } @@ -1887,6 +1901,13 @@ async fn create_script_internal<'c>( .execute(&mut *tx) .await?; + // A lock that is not left to a dependency job queues none, so this is the only place its hash + // can be recorded. `try_skip_relock` treats a missing hash for an imported script as changed, + // so leaving the row out makes every importer of this path relock on every deploy of it. + if let Some(lock) = lock.as_deref() { + record_lock_hashes(&mut tx, &w_id, &lock_hash_entry(&ns.path, lock)).await?; + } + // Update ci_test_reference table for test scripts // Delete by both new and old path to handle renames let old_path = parent_hashes_and_perms.as_ref().map(|x| x.p_path.as_str()); diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 2cda80f458..18a8d52ac0 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -284,15 +284,28 @@ pub async fn test_s3_bucket( use bytes::Bytes; use futures::StreamExt; - // The probe executes on the API server itself. On multi-tenant Cloud that is a shared control - // plane, so we constrain untrusted callers to remove the SSRF / credential-exfiltration / - // local-filesystem surface (see validate_object_storage_test). On self-hosted instances the - // object store usually lives on the local/private network and all authenticated users are - // trusted, so testing there stays unrestricted. Super admins keep the unrestricted path too. + // The probe executes on the API server itself and reflects the upstream response into the + // error, so any authenticated caller could otherwise use it as an SSRF / port-scan primitive + // against the server's network, exfiltrate its ambient credentials, or write to its local + // disk (see validate_object_storage_test). That holds on self-hosted instances as much as on + // Cloud, so only super admins get the unrestricted path. let is_super_admin = windmill_api_auth::is_super_admin_authed(&db, &authed).await?; - let restrict = !is_super_admin && *CLOUD_HOSTED; + let restrict = !is_super_admin; if restrict { - validate_object_storage_test(&test_s3_bucket).await?; + validate_object_storage_test(&test_s3_bucket) + .await + .map_err(|e| match e { + // A job token never counts as a super admin (it is capped at workspace admin), so + // a super admin calling this route from a script is told why rather than that + // they lack a privilege they hold. + error::Error::NotAuthorized(msg) if authed.job_id.is_some() => { + error::Error::NotAuthorized(format!( + "{msg} A job token ($WM_TOKEN) is never treated as a super admin; call \ + this route with a user token instead." + )) + } + e => e, + })?; } let client = build_object_store_from_settings(test_s3_bucket, Some(&db)) @@ -355,8 +368,8 @@ pub async fn test_s3_bucket( } } -// Hardening for the object-storage connectivity test by an untrusted (non-super-admin) caller on -// Cloud. The probe runs on the shared API server, so without these constraints an authenticated +// Hardening for the object-storage connectivity test by an untrusted (non-super-admin) caller. +// The probe runs on the API server, so without these constraints an authenticated // user could coerce the server into connecting to arbitrary internal endpoints (SSRF), signing // requests with the instance role (credential exfiltration), or reading/writing the server's local // disk (filesystem object store). @@ -366,6 +379,11 @@ async fn validate_object_storage_test(settings: &ObjectSettings) -> error::Resul opt.as_ref().is_some_and(|s| !s.is_empty()) } + // Every refusal names the way out: the resource usually works in jobs (workers reach the + // endpoint directly), so without it the refusal reads as a broken resource. + const ALTERNATIVE: &str = + "Ask a super admin to run it, or test the resource from a script, which runs on a worker."; + // Reject backends that rely on the server's identity or local filesystem, require explicit // credentials for the rest (so the server never falls back to its own ambient credentials), and // resolve the host the client will actually connect to. We derive the *effective* endpoint here @@ -376,20 +394,25 @@ async fn validate_object_storage_test(settings: &ObjectSettings) -> error::Resul let effective_endpoint: Option = match settings { ObjectSettings::Filesystem(_) => { return Err(error::Error::NotAuthorized( - "Testing a local filesystem object store requires a super admin".to_string(), + "Testing a local filesystem object store requires a super admin: it runs on the \ + Windmill server and reads and writes the server's local disk. Ask a super admin \ + to run it." + .to_string(), )); } ObjectSettings::AwsOidc(_) => { - return Err(error::Error::NotAuthorized( - "Testing OIDC-based object storage requires a super admin".to_string(), - )); + return Err(error::Error::NotAuthorized(format!( + "Testing OIDC-based object storage requires a super admin: it runs on the \ + Windmill server with the server's own identity. {ALTERNATIVE}" + ))); } ObjectSettings::S3(s3) => { if !(non_empty(&s3.access_key) && non_empty(&s3.secret_key)) { - return Err(error::Error::NotAuthorized( - "Testing S3 storage without explicit credentials requires a super admin" - .to_string(), - )); + return Err(error::Error::NotAuthorized(format!( + "Testing S3 storage without an explicit access key and secret key requires a \ + super admin: it runs on the Windmill server, which would use its own ambient \ + credentials. {ALTERNATIVE}" + ))); } let region = s3 .region @@ -413,10 +436,11 @@ async fn validate_object_storage_test(settings: &ObjectSettings) -> error::Resul } ObjectSettings::Azure(azure) => { if !non_empty(&azure.access_key) { - return Err(error::Error::NotAuthorized( - "Testing Azure storage without an explicit access key requires a super admin" - .to_string(), - )); + return Err(error::Error::NotAuthorized(format!( + "Testing Azure storage without an explicit access key requires a super admin: \ + it runs on the Windmill server, which would use its own ambient credentials. \ + {ALTERNATIVE}" + ))); } Some( azure @@ -432,10 +456,11 @@ async fn validate_object_storage_test(settings: &ObjectSettings) -> error::Resul // otherwise an untrusted caller could probe with the server's identity (the very // SSRF/credential-exfil this function guards against). if windmill_object_store::gcs_service_account_key_is_blank(&gcs.service_account_key) { - return Err(error::Error::NotAuthorized( - "Testing GCS storage without a service account key requires a super admin" - .to_string(), - )); + return Err(error::Error::NotAuthorized(format!( + "Testing GCS storage without a service account key requires a super admin: \ + it runs on the Windmill server, which would use its own ambient credentials. \ + {ALTERNATIVE}" + ))); } // The service-account-key JSON can override the data-plane URL (`gcs_base_url`) and the // OAuth token endpoint (`token_uri`); the GCS client connects to whatever they point at. @@ -492,10 +517,15 @@ async fn validate_public_endpoint(endpoint: &str) -> error::Result<()> { // attempts (a name resolving to both a public and a private address). for addr in addrs { if is_forbidden_ip(addr.ip()) { - return Err(error::Error::NotAuthorized( - "Testing object storage at a private, loopback, or link-local endpoint requires a super admin" - .to_string(), - )); + // The resolved address stays out of the message: it is the server's resolver's + // answer, and this message is only ever shown to the caller being constrained. + return Err(error::Error::NotAuthorized(format!( + "Testing object storage at '{host}', which resolves to a private, loopback, or \ + link-local address, requires a super admin: this test runs on the Windmill \ + server, which is not allowed to probe internal addresses for non-super-admins. \ + Ask a super admin to run it, or test the resource from a script, which runs on \ + a worker." + ))); } } Ok(()) @@ -2008,7 +2038,10 @@ struct CachedResourceType { /// decodes the on-disk cache, where an absent key means "written before the /// column, leave the stored extension alone" and an explicit null means the hub /// dropped it. Plain serde folds both into `None`. - #[serde(default, deserialize_with = "windmill_common::more_serde::double_option")] + #[serde( + default, + deserialize_with = "windmill_common::more_serde::double_option" + )] format_extension: Option>, } diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 2aa59b20ff..cb5c639d46 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -12,6 +12,7 @@ use windmill_api_auth::{ }; use windmill_api_users::users::WorkspaceInvite; use windmill_common::email_oss::send_email_if_possible; +use windmill_dep_map::lock_hash::record_lock_hashes_for_workspace; use windmill_common::usernames::{get_instance_username_or_create_pending, VALID_USERNAME}; use windmill_common::webhook::WebhookShared; use windmill_common::{BASE_URL, DB}; @@ -7139,7 +7140,16 @@ async fn clone_workspace_runnable_dependencies( .execute(&mut **tx) .await?; - // Clone dependency_map to preserve import relationships + // Recorded so the clone's own relocks have something to match; with no row they record NULL + // and nothing in it ever skips. Hashed from the locks the clone holds rather than copied from + // the source's rows, which are only as current as the last write to them: one left stale by a + // supplied lock deployed before this was recorded names a lock the clone no longer has, and an + // importer that resolved against the real one would then skip a relock it needed. + record_lock_hashes_for_workspace(tx, target_workspace_id).await?; + + // Deliberately without `imported_lockfile_hash`: it records what an importer resolved against + // when it was last locked, which nothing here can establish for the version the clone got. + // Left NULL, every importer relocks once and re-anchors both sides to what the clone holds. sqlx::query!( "INSERT INTO dependency_map (workspace_id, importer_path, importer_kind, imported_path, importer_node_id) SELECT $1, importer_path, importer_kind, imported_path, importer_node_id diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 3830c3f372..a94085e7ce 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -1,7 +1,7 @@ openapi: "3.0.3" info: - version: 1.801.0 + version: 1.803.0 title: Windmill API contact: @@ -10456,7 +10456,7 @@ paths: summary: run script by path operationId: runScriptByPath x-mcp-tool: true - x-mcp-instructions: "You should first use getScriptByPath to retrieve the script's schema and understand what arguments are expected." + x-mcp-instructions: "You should first use getScriptByPath to retrieve the script's schema and understand what arguments are expected. A resource argument (schema format `resource-`) is the bare string `$res:` as that whole argument value -- not a wrapper object, not a plain path. A variable is `$var:`." x-mcp-tool-include-query-params: [] tags: - job @@ -13814,7 +13814,7 @@ paths: summary: run flow by path operationId: runFlowByPath x-mcp-tool: true - x-mcp-instructions: "You should first use getFlowByPath to retrieve the flow's schema and understand what arguments are expected." + x-mcp-instructions: "You should first use getFlowByPath to retrieve the flow's schema and understand what arguments are expected. A resource argument (schema format `resource-`) is the bare string `$res:` as that whole argument value -- not a wrapper object, not a plain path. A variable is `$var:`." x-mcp-tool-include-query-params: [] tags: - job @@ -27156,6 +27156,13 @@ components: type: object additionalProperties: $ref: "#/components/schemas/ModelPriceOverride" + copilot_disabled: + type: boolean + description: >- + Hides the Windmill AI assistant (chat, sessions, code generation, completion, + fixes) from the workspace UI. Read from the workspace's own settings even when + the providers served fall back to the instance config. AI agent steps and the + AI sandbox in flows are unaffected. FreeTierInfo: type: object @@ -29008,6 +29015,8 @@ components: - "igroup.delete" - "igroup.adduser" - "igroup.removeuser" + - "instance_groups.jit_adduser" + - "instance_groups.jit_removeuser" - "variables.decrypt_secret" - "workspaces.read_encryption_key" - "workspaces.edit_command_script" diff --git a/backend/windmill-api/src/ai.rs b/backend/windmill-api/src/ai.rs index c684830b27..e70cc19544 100644 --- a/backend/windmill-api/src/ai.rs +++ b/backend/windmill-api/src/ai.rs @@ -445,6 +445,12 @@ pub struct AIConfig { /// Only models whose rates differ from the built-in table are stored. #[serde(skip_serializing_if = "Option::is_none")] pub model_pricing: Option>, + /// Hides the Windmill AI assistant (chat, sessions, generation, completion, fixes) from + /// the workspace UI. Only the workspace's own row is consulted: the flag holds even when + /// the providers served come from the instance config or the free tier. AI agent steps + /// and the AI sandbox are unaffected, so the providers stay in force. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub copilot_disabled: bool, } /// Negotiated rates in USD per million tokens. An unset cache rate is read as the diff --git a/backend/windmill-api/src/jobs.rs b/backend/windmill-api/src/jobs.rs index a30cffbfbf..53044ee020 100644 --- a/backend/windmill-api/src/jobs.rs +++ b/backend/windmill-api/src/jobs.rs @@ -18,6 +18,7 @@ use quick_cache::sync::Cache; use serde_json::value::RawValue; use serde_json::Value; use sha2::{Digest, Sha256}; +use std::borrow::Cow; use std::collections::HashMap; use std::str::FromStr; use std::sync::Arc; @@ -108,6 +109,7 @@ use windmill_common::{ flows::{add_virtual_items_if_necessary, resolve_maybe_value, FlowValue}, jobs::{script_path_to_payload, CompletedJob, JobKind, JobPayload, QueuedJob, RawCode}, oauth2::HmacSha256, + query_builders, scripts::{ScriptHash, ScriptLang}, users::username_to_permissioned_as, utils::{not_found_if_none, now_from_db, paginate, require_admin, Pagination, StripPath}, @@ -8131,6 +8133,79 @@ pub async fn run_wait_result_flow_by_version( .await } +/// Whether request-supplied SQL from an operator may run. Operators can only run deployed +/// code, so a request their job token (`WM_TOKEN`) authenticates comes from code a +/// non-operator authored. The job must still be running, and the request must have the +/// shape `wmill.datatable()` sends (PostgreSQL against a `datatable://` database), so a +/// WM_TOKEN that leaked into job logs cannot be replayed to reach another target while the +/// job lives, in particular DuckDB, which runs in-process in the worker. +/// +/// What it does permit is any statement against the workspace's data tables, writes and DDL +/// included: the helper's body is an unrestricted SQL template and data tables carry no +/// per-user ACL. Narrowing that is a separate decision from this exemption. +/// +/// The database argument is only half the target: the executor honors a `-- database` +/// directive in the SQL over it, and `-- s3` redirects the result set, so both are refused. +/// Check them against the code the executor runs rather than the request's `content`, which +/// is not the same string once a `WM_INTERNAL_DB` marker expands. +async fn operator_may_run_datatable_query( + db: &DB, + w_id: &str, + job_id: Option, + language: Option<&ScriptLang>, + content: &str, + args: Option<&HashMap>>, +) -> error::Result { + let Some(job_id) = job_id else { + return Ok(false); + }; + if language != Some(&ScriptLang::Postgresql) { + return Ok(false); + } + // Parse the directives out of the code the executor actually runs: it expands a + // `WM_INTERNAL_DB` marker first, and a directive can be embedded in the expansion. + // An expansion that overrides the language would run something other than the SQL the + // language check above cleared, so it is refused along with a malformed marker. + let executed = + match query_builders::try_expand_internal_db_query(content, &ScriptLang::Postgresql) { + Some(Ok(expanded)) if expanded.language_override.is_none() => Cow::Owned(expanded.code), + Some(_) => return Ok(false), + None => Cow::Borrowed(content), + }; + if windmill_parser_sql::parse_db_resource(&executed).is_some() + || !matches!(windmill_parser_sql::parse_s3_mode(&executed), Ok(None)) + { + return Ok(false); + } + let targets_datatable = args + .and_then(|args| args.get("database")) + .and_then(|database| serde_json::from_str::(database.get()).ok()) + .is_some_and(|database| database.starts_with("datatable://")); + if !targets_datatable { + return Ok(false); + } + Ok(sqlx::query_scalar!( + "SELECT running AS \"running!\" FROM v2_job_queue WHERE id = $1 AND workspace_id = $2", + job_id, + w_id + ) + .fetch_optional(db) + .await? + .unwrap_or(false)) +} + +/// The refusal an operator gets from a preview route. Inside a job the caller never ran a +/// preview themselves, so name the one thing the job's token may do. +fn operator_preview_refusal(job_id: Option) -> error::Error { + let reason = if job_id.is_some() { + "Operators cannot run preview jobs for security reasons: from inside a job, an \ + operator may only run a wmill.datatable() query while that job is running" + } else { + "Operators cannot run preview jobs for security reasons" + }; + error::Error::NotAuthorized(reason.to_string()) +} + async fn run_preview_script( authed: ApiAuthed, Extension(db): Extension, @@ -8142,9 +8217,20 @@ async fn run_preview_script( #[cfg(feature = "enterprise")] check_license_key_valid().await?; if authed.is_operator { - return Err(error::Error::NotAuthorized( - "Operators cannot run preview jobs for security reasons".to_string(), - )); + // A deferred run would outlive the running job the exemption keys off. + if run_query.get_scheduled_for(&db).await?.is_some() + || !operator_may_run_datatable_query( + &db, + &w_id, + authed.job_id, + preview.language.as_ref(), + preview.content.as_deref().unwrap_or_default(), + preview.args.as_ref(), + ) + .await? + { + return Err(operator_preview_refusal(authed.job_id)); + } } // Preview runs arbitrary, request-supplied code. require_path_read_access_for_preview // only checks folder/namespace *read* access (and is a no-op when path is null), so a @@ -8239,13 +8325,21 @@ async fn run_inline_preview_script( Path(w_id): Path, Json(preview): Json, ) -> error::Result { - // Same arbitrary-code class as run_preview_script: operators are blocked from - // running request-supplied code, and a narrowly-scoped token must not escape - // its scope through inline preview. - if authed.is_operator { - return Err(error::Error::NotAuthorized( - "Operators cannot run preview jobs for security reasons".to_string(), - )); + // Same arbitrary-code class as run_preview_script, and every worker and standalone + // server exposes this route, so an operator is refused on the same terms. A + // narrowly-scoped token must not escape its scope through inline preview either. + if authed.is_operator + && !operator_may_run_datatable_query( + &db, + &w_id, + job_id, + Some(&preview.language), + &preview.content, + preview.args.as_ref(), + ) + .await? + { + return Err(operator_preview_refusal(job_id)); } check_scopes(&authed, || format!("jobs:run"))?; if let Some(job_id) = job_id { diff --git a/backend/windmill-api/src/lib.rs b/backend/windmill-api/src/lib.rs index b6337351a4..054af0f2fa 100644 --- a/backend/windmill-api/src/lib.rs +++ b/backend/windmill-api/src/lib.rs @@ -452,15 +452,15 @@ pub async fn run_server( // unless they are allowed — hence a separate layer rather than widening the // one every other route shares. (`Mcp-Param-*` is only sent for tool inputs // annotated with `x-mcp-header`, which no tool here declares.) + // + // The request's own header list is mirrored rather than enumerated: a browser + // MCP client may send any custom name for a preprocessor to read, and no fixed + // list could cover them. Nothing is granted by echoing it: the origin is + // `Any`, so browsers never attach credentials, and the endpoint authenticates + // each request on its own. let mcp_cors = CorsLayer::new() .allow_methods([http::Method::GET, http::Method::POST, http::Method::DELETE]) - .allow_headers([ - http::header::CONTENT_TYPE, - http::header::AUTHORIZATION, - http::HeaderName::from_static("mcp-protocol-version"), - http::HeaderName::from_static("mcp-method"), - http::HeaderName::from_static("mcp-name"), - ]) + .allow_headers(tower_http::cors::AllowHeaders::mirror_request()) // The 401 challenge is how a client discovers where to authorize (RFC 9728), // and it is not a safelisted response header, so without this a browser // client sees an empty one and has no way to begin the OAuth flow. diff --git a/backend/windmill-api/src/mcp/auto_generated_endpoints.rs b/backend/windmill-api/src/mcp/auto_generated_endpoints.rs index 568ad854f9..aa2415f664 100644 --- a/backend/windmill-api/src/mcp/auto_generated_endpoints.rs +++ b/backend/windmill-api/src/mcp/auto_generated_endpoints.rs @@ -878,7 +878,7 @@ is, a different one moves it there and archives the old path"), EndpointTool { name: Cow::Borrowed("runScriptByPath"), description: Cow::Borrowed("run script by path"), - instructions: Cow::Borrowed("You should first use getScriptByPath to retrieve the script's schema and understand what arguments are expected."), + instructions: Cow::Borrowed("You should first use getScriptByPath to retrieve the script's schema and understand what arguments are expected. A resource argument (schema format `resource-`) is the bare string `$res:` as that whole argument value -- not a wrapper object, not a plain path. A variable is `$var:`."), path: Cow::Borrowed("/w/{workspace}/jobs/run/p/{path}"), method: Cow::Borrowed("POST"), path_params_schema: Some(serde_json::json!({ @@ -1419,7 +1419,7 @@ is, a different one moves it there and archives the old path"), EndpointTool { name: Cow::Borrowed("runFlowByPath"), description: Cow::Borrowed("run flow by path"), - instructions: Cow::Borrowed("You should first use getFlowByPath to retrieve the flow's schema and understand what arguments are expected."), + instructions: Cow::Borrowed("You should first use getFlowByPath to retrieve the flow's schema and understand what arguments are expected. A resource argument (schema format `resource-`) is the bare string `$res:` as that whole argument value -- not a wrapper object, not a plain path. A variable is `$var:`."), path: Cow::Borrowed("/w/{workspace}/jobs/run/f/{path}"), method: Cow::Borrowed("POST"), path_params_schema: Some(serde_json::json!({ diff --git a/backend/windmill-api/src/mcp/core.rs b/backend/windmill-api/src/mcp/core.rs index 2b4350bcfe..480e3c0841 100644 --- a/backend/windmill-api/src/mcp/core.rs +++ b/backend/windmill-api/src/mcp/core.rs @@ -12,7 +12,9 @@ use windmill_mcp::common::transform::transform_property_keys; use windmill_mcp::common::types::{ FlowInfo, HubScriptInfo, ResourceInfo, ResourceType, SchemaType, ScriptInfo, WorkspaceInfo, }; -use windmill_mcp::server::{BackendResult, EndpointTool, ErrorData, McpBackend, PathFilter}; +use windmill_mcp::server::{ + BackendResult, EndpointTool, ErrorData, McpBackend, McpRequest, PathFilter, +}; use crate::auth::AuthCache; use crate::db::ApiAuthed; @@ -214,8 +216,11 @@ impl McpBackend for WindmillBackend { workspace_id: &str, path: &str, args: Value, + request: &McpRequest<'_>, ) -> BackendResult { - let push_args = prepare_push_args(args); + let push_args = prepare_push_args(&self.db, workspace_id, path, false, args, request) + .await + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; let result = run_wait_result_script_by_path_internal( self.db.clone(), @@ -238,8 +243,11 @@ impl McpBackend for WindmillBackend { workspace_id: &str, path: &str, args: Value, + request: &McpRequest<'_>, ) -> BackendResult { - let push_args = prepare_push_args(args); + let push_args = prepare_push_args(&self.db, workspace_id, path, true, args, request) + .await + .map_err(|e| ErrorData::internal_error(e.to_string(), None))?; let result = run_wait_result_flow_by_path_internal( self.db.clone(), diff --git a/backend/windmill-api/src/mcp/utils.rs b/backend/windmill-api/src/mcp/utils.rs index 94271bb662..65c8069b85 100644 --- a/backend/windmill-api/src/mcp/utils.rs +++ b/backend/windmill-api/src/mcp/utils.rs @@ -11,15 +11,19 @@ use serde_json::Value; use sql_builder::prelude::*; use windmill_common::auth::create_jwt_token; use windmill_common::db::{Authed, UserDB}; +use windmill_common::error::Error; use windmill_common::scripts::{get_full_hub_script_by_path, Schema}; +use windmill_common::triggers::{RunnableFormat, RunnableFormatVersion, TriggerKind}; use windmill_common::utils::{query_elems_from_hub, StripPath}; use windmill_common::worker::to_raw_value; use windmill_common::{DB, HUB_BASE_URL}; use windmill_mcp::server::{ - non_empty_body_fields, BackendResult, EndpointTool, ErrorData, PathFilter, + non_empty_body_fields, BackendResult, EndpointTool, ErrorData, McpRequest, PathFilter, }; use windmill_mcp::{HubResponse, HubScriptInfo, ItemSchema, ResourceInfo, ResourceType}; +use windmill_trigger::trigger_helpers::{get_runnable_format, RunnableId}; +use crate::args::build_headers; use crate::db::ApiAuthed; use crate::HTTP_CLIENT; @@ -641,7 +645,7 @@ fn selects_endpoint_tool(caller_scopes: &[String], tool: &str) -> bool { .is_ok_and(|config| config.endpoints.iter().any(|e| e == tool)) } -/// Create HTTP request with authentication +/// Create HTTP request with authentication. pub async fn create_http_request( method: &str, url: &str, @@ -702,17 +706,113 @@ pub async fn create_http_request( .map_err(|e| ErrorData::internal_error(format!("Failed to execute request: {}", e), None)) } -/// Convert a JSON Value into PushArgsOwned for job execution -pub fn prepare_push_args(args: Value) -> windmill_queue::PushArgsOwned { +/// The `kind` an MCP-invoked runnable sees on its preprocessor event, alongside +/// `webhook`, `http` and the trigger kinds. +const MCP_TRIGGER_KEY: &str = "mcp"; + +/// A preprocessor's view of the MCP request that ran it. Mirrors the HTTP +/// trigger event: `body` is what the model sent, everything else describes the +/// call itself. +#[derive(serde::Serialize)] +struct McpPreprocessorEvent<'a> { + kind: &'a str, + body: Box, + headers: HashMap>, + tool_name: &'a str, +} + +/// Headers withheld from a preprocessor because they authenticate the connection. +/// +/// Not a security boundary: a webhook preprocessor receives all three. Withheld +/// because nothing needs them yet, and releasing one later is additive while +/// withdrawing one after runnables read it is not. +const WITHHELD_FROM_PREPROCESSOR: &[&str] = &["authorization", "cookie", "proxy-authorization"]; + +/// Every header a preprocessor may see. +fn preprocessor_headers( + headers: &http::HeaderMap, +) -> HashMap> { + let mut selected = build_headers(headers, None, true); + selected.retain(|name, _| { + !WITHHELD_FROM_PREPROCESSOR + .iter() + .any(|withheld| withheld.eq_ignore_ascii_case(name)) + }); + selected +} + +/// Build the job arguments for a script or flow run as an MCP tool. +/// +/// Shaped by the runnable's own format: a preprocessor receives the request as +/// an event, and a runnable without one receives only what the model sent. +pub async fn prepare_push_args( + db: &DB, + w_id: &str, + path: &str, + is_flow: bool, + args: Value, + request: &McpRequest<'_>, +) -> Result { + let mut main_args = HashMap::new(); if let Value::Object(map) = args { - let mut args_hash = HashMap::new(); for (k, v) in map { - args_hash.insert(k, to_raw_value(&v)); + main_args.insert(k, to_raw_value(&v)); } - windmill_queue::PushArgsOwned { extra: None, args: args_hash } - } else { - windmill_queue::PushArgsOwned::default() } + + let runnable_id = if is_flow { + RunnableId::from_flow_path(path) + } else { + // Resolves a `hub/` path to the hub script on its own. + RunnableId::from_script_path(path) + }; + + // MCP is not one of the `TRIGGER_KIND` enum values and does not need to be: + // the per-kind arms of the no-preprocessor heuristic are payload-shape + // special cases for message triggers, and `Webhook` reaches the same generic + // arm MCP wants while sharing that kind's format cache. + let runnable_format = get_runnable_format(runnable_id, w_id, db, &TriggerKind::Webhook).await?; + + Ok(match runnable_format { + // Without a preprocessor there is nowhere for a header to go that the + // model does not also write: its arguments *are* the runnable's + // parameters, so a header bound to one of them would be a value the model + // could set. The request is reachable through a preprocessor, where it + // arrives in a key of the event the model never fills. + RunnableFormat { has_preprocessor: false, .. } => { + windmill_queue::PushArgsOwned { args: main_args, extra: None } + } + RunnableFormat { has_preprocessor: true, version } => { + let headers = preprocessor_headers(request.headers); + match version { + RunnableFormatVersion::V2 => { + let event = McpPreprocessorEvent { + kind: MCP_TRIGGER_KEY, + body: to_raw_value(&main_args), + headers, + tool_name: request.tool_name, + }; + windmill_queue::PushArgsOwned { + args: HashMap::from([("event".to_string(), to_raw_value(&event))]), + extra: None, + } + } + RunnableFormatVersion::V1 => windmill_queue::PushArgsOwned { + args: main_args, + extra: Some(HashMap::from([( + "wm_trigger".to_string(), + to_raw_value(&serde_json::json!({ + "kind": MCP_TRIGGER_KEY, + MCP_TRIGGER_KEY: { + "headers": headers, + "tool_name": request.tool_name, + } + })), + )])), + }, + } + } + }) } /// Parse an HTTP response body into a JSON Value diff --git a/backend/windmill-api/src/workspaces.rs b/backend/windmill-api/src/workspaces.rs index 76378e335a..da8532e9d1 100644 --- a/backend/windmill-api/src/workspaces.rs +++ b/backend/windmill-api/src/workspaces.rs @@ -146,6 +146,7 @@ async fn edit_copilot_config( .await?; let workspace_has_config = ai_config.has_providers(); + let copilot_disabled = ai_config.copilot_disabled; let instance_ai_config = sqlx::query_scalar!("SELECT value FROM global_settings WHERE name = 'ai_config'") .fetch_optional(&db) @@ -158,7 +159,7 @@ async fn edit_copilot_config( .as_ref() .and_then(|v| serde_json::from_value::(v.clone()).ok()) .filter(|c| c.has_providers()); - let effective_ai_config = if workspace_has_config { + let mut effective_ai_config = if workspace_has_config { ai_config } else if let Some(instance_config) = instance_config_with_providers { instance_config @@ -172,6 +173,7 @@ async fn edit_copilot_config( } else { AIConfig::default() }; + effective_ai_config.copilot_disabled = copilot_disabled; Ok(Json(EditCopilotConfigResponse { effective_ai_config, @@ -207,6 +209,9 @@ async fn get_copilot_info( )) })?; + let copilot_disabled = workspace_ai_config + .as_ref() + .is_some_and(|c| c.0.copilot_disabled); let instance_config = sqlx::query_scalar!("SELECT value FROM global_settings WHERE name = 'ai_config'") .fetch_optional(&db) @@ -215,20 +220,23 @@ async fn get_copilot_info( // A provider-less instance config (e.g. `{}`) is unconfigured; don't let it shadow the // free-tier fallback, matching the proxy and edit_copilot_config paths. .filter(|c| c.has_providers()); - if let Some(workspace_ai_config) = workspace_ai_config.filter(|c| c.0.has_providers()) { - Ok(Json(workspace_ai_config.0)) - } else if let Some(instance_config) = instance_config { - Ok(Json(instance_config)) - } else if let Some(free_config) = - crate::ai_free_tier_oss::free_tier_copilot_config(&db, &authed.email).await? - { - // Nothing configured: fall back to Windmill's free tier (EE-only). The config - // carries a `free_tier` marker even once the user's grant is spent — with no - // providers, but telling the client *why* AI is off. - Ok(Json(free_config)) - } else { - Ok(Json(AIConfig::default())) - } + let mut effective = + if let Some(workspace_ai_config) = workspace_ai_config.filter(|c| c.0.has_providers()) { + workspace_ai_config.0 + } else if let Some(instance_config) = instance_config { + instance_config + } else if let Some(free_config) = + crate::ai_free_tier_oss::free_tier_copilot_config(&db, &authed.email).await? + { + // Nothing configured: fall back to Windmill's free tier (EE-only). The config + // carries a `free_tier` marker even once the user's grant is spent — with no + // providers, but telling the client *why* AI is off. + free_config + } else { + AIConfig::default() + }; + effective.copilot_disabled = copilot_disabled; + Ok(Json(effective)) } #[cfg(feature = "enterprise")] diff --git a/backend/windmill-common/src/global_settings.rs b/backend/windmill-common/src/global_settings.rs index b1ad2edd01..d38396562b 100644 --- a/backend/windmill-common/src/global_settings.rs +++ b/backend/windmill-common/src/global_settings.rs @@ -94,6 +94,9 @@ pub const AUTOMATE_USERNAME_CREATION_SETTING: &str = "automate_username_creation pub const DISABLE_WORKSPACE_INVITE_EMAILS_SETTING: &str = "disable_workspace_invite_emails"; pub const DISABLE_PASSWORD_LOGIN_SETTING: &str = "disable_password_login"; pub const AUTO_LOGIN_PROVIDER_SETTING: &str = "auto_login_provider"; +/// Name of the SAML attribute or OIDC userinfo claim carrying the user's IdP groups. Unset or +/// empty leaves instance-group membership entirely to SCIM. +pub const SSO_GROUPS_CLAIM_SETTING: &str = "sso_groups_claim"; pub const HUB_BASE_URL_SETTING: &str = "hub_base_url"; pub const HUB_ACCESSIBLE_URL_SETTING: &str = "hub_accessible_url"; pub const DISABLE_HUB_SETTING: &str = "disable_hub"; @@ -108,6 +111,7 @@ pub const JWT_SECRET_SETTING: &str = "jwt_secret"; pub const EMAIL_DOMAIN_SETTING: &str = "email_domain"; pub const OTEL_SETTING: &str = "otel"; pub const OTEL_TRACING_PROXY_SETTING: &str = "otel_tracing_proxy"; +pub const OTEL_TRACES_RETENTION_SECS_SETTING: &str = "otel_traces_retention_secs"; pub const APP_WORKSPACED_ROUTE_SETTING: &str = "app_workspaced_route"; pub const HTTP_ROUTE_WORKSPACED_ROUTE_SETTING: &str = "http_route_workspaced_route"; pub const SECRET_BACKEND_SETTING: &str = "secret_backend"; diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index f1784e9841..b4128f80d2 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -148,43 +148,76 @@ pub const DEFAULT_MAX_CONNECTIONS_INDEXER: u32 = 5; pub const DEFAULT_HUB_BASE_URL: &str = "https://hub.windmill.dev"; pub const PRIVATE_HUB_MIN_VERSION: i32 = 10_000_000; pub const DEFAULT_SERVICE_LOG_RETENTION_SECS: i64 = 60 * 60 * 24 * 14; // 2 weeks retention period for logs +pub const DEFAULT_OTEL_TRACES_RETENTION_SECS: i64 = 60 * 60 * 24 * 7; // 1 week retention period for HTTP request spans pub const WM_DEPLOYERS_GROUP: &str = "wm_deployers"; /// A century. Every consumer has to survive `now - retention`, and the ceilings are much lower /// than an `i64`: `DateTime` subtraction panics past year 262143, and the `( s)::interval` /// the cleanup queries build overflows Postgres' microsecond field. -const MAX_SERVICE_LOG_RETENTION_SECS: i64 = 60 * 60 * 24 * 365 * 100; +const MAX_RETENTION_SECS: i64 = 60 * 60 * 24 * 365 * 100; -/// Apply a configured service log retention, in seconds. +/// Clamp a configured retention window, in seconds, to one a cutoff can be built from. /// -/// The only way into [`SERVICE_LOG_RETENTION_SECS`], so an unusable value can never reach a -/// cutoff. The two unusable directions are not the same mistake and must not share a landing -/// point: too large still says "keep these for a very long time", so it is capped and the -/// intent survives, whereas falling back would delete logs the operator meant to keep. A -/// non-positive value has no such reading — every cutoff is `now - retention`, so it lands at -/// or after `now` and the next sweep expires the entire history, rows and object-storage files -/// alike. Unlike job retention there is no "keep forever" spelling here, so `0` — what an -/// operator types by analogy with it, and what the settings UI writes into a field that was -/// merely focused — falls back to the default. -pub fn set_service_log_retention_secs(configured: i64) { - let effective = if configured > MAX_SERVICE_LOG_RETENTION_SECS { +/// Shared by the retention windows that have no "keep forever" spelling, so that an unusable +/// value can never reach a cutoff. The two unusable directions are not the same mistake and must +/// not share a landing point: too large still says "keep these for a very long time", so it is +/// capped and the intent survives, whereas falling back would delete data the operator meant to +/// keep. A non-positive value has no such reading — every cutoff is `now - retention`, so it +/// lands at or after `now` and the next sweep expires the entire history. `0` is both what an +/// operator types by analogy with job retention, where it does mean keep forever, and what the +/// settings UI writes into a field that was merely focused, so it falls back to the default. +fn clamp_retention_secs(configured: i64, default: i64, what: &str) -> i64 { + if configured > MAX_RETENTION_SECS { tracing::warn!( - "service log retention of {configured}s exceeds the maximum of \ - {MAX_SERVICE_LOG_RETENTION_SECS}s, capping it there" + "{what} retention of {configured}s exceeds the maximum of {MAX_RETENTION_SECS}s, \ + capping it there" ); - MAX_SERVICE_LOG_RETENTION_SECS + MAX_RETENTION_SECS } else if configured >= 1 { configured } else { tracing::warn!( - "service log retention of {configured}s would expire every service log, \ - falling back to the default of {DEFAULT_SERVICE_LOG_RETENTION_SECS}s" + "{what} retention of {configured}s would expire the entire history, \ + falling back to the default of {default}s" ); - DEFAULT_SERVICE_LOG_RETENTION_SECS - }; + default + } +} + +/// Apply a configured service log retention, in seconds. +/// +/// The only way into [`SERVICE_LOG_RETENTION_SECS`]. Expiry reaches every copy of a log line: +/// the row, the file on disk, and the object-storage object. +pub fn set_service_log_retention_secs(configured: i64) { + let effective = clamp_retention_secs( + configured, + DEFAULT_SERVICE_LOG_RETENTION_SECS, + "service log", + ); SERVICE_LOG_RETENTION_SECS.store(effective, std::sync::atomic::Ordering::Relaxed); } +/// Apply a configured OTEL trace retention, in seconds. +/// +/// The only way into [`OTEL_TRACES_RETENTION_SECS`]. +pub fn set_otel_traces_retention_secs(configured: i64) { + let effective = clamp_retention_secs( + configured, + DEFAULT_OTEL_TRACES_RETENTION_SECS, + "otel traces", + ); + OTEL_TRACES_RETENTION_SECS.store(effective, std::sync::atomic::Ordering::Relaxed); +} + +/// How long an HTTP request tracing span stays in `otel_traces`, in seconds. +/// +/// Spans are keyed by the job they were captured for and read back by the job detail view, so +/// this is the outer bound on how far back that view can show a job's HTTP requests. It is +/// independent of job retention: a span can outlive its job, or be swept while the job remains. +pub fn otel_traces_retention_secs() -> i64 { + OTEL_TRACES_RETENTION_SECS.load(std::sync::atomic::Ordering::Relaxed) +} + /// How long a service log line stays retrievable, in seconds. /// /// The outer bound on everything service-log: the `log_file` rows, the raw files in object @@ -423,6 +456,10 @@ lazy_static::lazy_static! { /// would expire every service log cannot reach a cutoff. Read it with /// [`service_log_retention_secs`]. static ref SERVICE_LOG_RETENTION_SECS: AtomicI64 = AtomicI64::new(DEFAULT_SERVICE_LOG_RETENTION_SECS); + /// Private on purpose, same as [`SERVICE_LOG_RETENTION_SECS`]: + /// [`set_otel_traces_retention_secs`] is the only writer, [`otel_traces_retention_secs`] the + /// only reader. + static ref OTEL_TRACES_RETENTION_SECS: AtomicI64 = AtomicI64::new(DEFAULT_OTEL_TRACES_RETENTION_SECS); pub static ref MONITOR_LOGS_ON_OBJECT_STORE: AtomicBool = AtomicBool::new(false); diff --git a/backend/windmill-common/src/scripts.rs b/backend/windmill-common/src/scripts.rs index 31219621e9..d71bc50e60 100644 --- a/backend/windmill-common/src/scripts.rs +++ b/backend/windmill-common/src/scripts.rs @@ -358,31 +358,38 @@ pub async fn fetch_script_for_update<'a>( .map_err(crate::error::Error::from) } -pub struct ClonedScript { - pub old_script: NewScript, - pub new_hash: i64, -} -// TODO: What if dependency job fails, there is script with NULL in the lock -pub async fn clone_script<'c>( - path: &str, - w_id: &str, +/// Deploys the outcome of a relative-import relock as a new version of `head`, the path's live +/// version that the caller holds `FOR UPDATE`, and archives `head`. A `lock` of `None` records +/// a failed generation: the version carries `lock_error_logs` instead and runs keep resolving +/// to the last version that has a lock. A `modules` of `None` keeps the head's module locks. +/// +/// Writes whatever `head` names and checks nothing: callers are responsible for having +/// established access to its workspace and path, as a dependency job's push already has. +/// +/// `created_at` is stamped when the insert runs, not at transaction start. The row lock on +/// `head` is what orders one relock after another, and with `now()` a transaction that began +/// first but locked second commits a live child older than its archived parent, which every +/// "latest version" read then mis-orders. +pub async fn deploy_relocked_version( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + head: Script, deployment_message: Option, - db: &DB, -) -> crate::error::Result { - let mut tx = db.begin().await?; - let s = if let Some(s) = fetch_script_for_update(path, w_id, &mut *tx).await? { - s - } else { - return Err(crate::error::Error::NotFound(format!( - "Non-archived script with path '{}' not found", - path - ))); - }; + lock: Option<&str>, + modules: Option<&std::collections::HashMap>, + lock_error_logs: Option<&str>, +) -> crate::error::Result { + let s = head; + let w_id = s.workspace_id.as_str(); - let rs = runnable_settings::from_handle(s.runnable_settings.runnable_settings_handle, &mut *tx) - .await?; + let rs = + runnable_settings::from_handle(s.runnable_settings.runnable_settings_handle, &mut **tx) + .await?; let (debouncing_settings, concurrency_settings) = - runnable_settings::prefetch_cached_tx(&rs, &mut tx).await?; + runnable_settings::prefetch_cached_tx(&rs, &mut *tx).await?; + + // What the row stores is what the hash covers: the new module locks when there are any. + let modules = modules.cloned().or(s.modules); + let modules_json = modules.as_ref().map(serde_json::to_value).transpose()?; let ns = NewScript { path: s.path.clone(), @@ -392,7 +399,7 @@ pub async fn clone_script<'c>( content: s.content, schema: s.schema, is_template: s.is_template, - lock: None, + lock: lock.map(str::to_string), language: s.language, kind: Some(s.kind), tag: s.tag, @@ -424,7 +431,7 @@ pub async fn clone_script<'c>( on_behalf_of: s.on_behalf_of, preserve_on_behalf_of: None, assets: s.assets, - modules: s.modules, + modules, auto_parent: None, labels: s.labels, skip_draft_deletion: None, @@ -433,7 +440,7 @@ pub async fn clone_script<'c>( let new_hash = hash_script(&ns); tracing::debug!( - "cloning script at path {} from '{}' to '{}'", + "deploying relocked version of script at path {} from '{}' to '{}'", s.path, *s.hash, new_hash @@ -446,17 +453,19 @@ pub async fn clone_script<'c>( envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, \ dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, \ delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, \ - codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels) + codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels, \ + lock_error_logs, created_at) SELECT workspace_id, $1, path, array_prepend($2::bigint, COALESCE(parent_hashes, '{}'::bigint[])), summary, description, \ - content, created_by, schema, is_template, extra_perms, NULL, language, kind, tag, \ + content, created_by, schema, is_template, extra_perms, $4::text, language, kind, tag, \ envs, concurrent_limit, concurrency_time_window_s, cache_ttl, cache_ignore_s3_path, \ dedicated_worker, ws_error_handler_muted, priority, restart_unless_cancelled, \ delete_after_use, delete_after_secs, timeout, concurrency_key, visible_to_runner_only, auto_kind, \ - codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, modules, labels + codebase, has_preprocessor, on_behalf_of, on_behalf_of_email, schema_validation, assets, debounce_key, debounce_delay_s, runnable_settings_handle, COALESCE($5::jsonb, modules), labels, \ + $6::text, clock_timestamp() FROM script WHERE hash = $2 AND workspace_id = $3; - ", new_hash, s.hash.0, w_id).execute(&mut *tx).await?; + ", new_hash, s.hash.0, w_id, lock, modules_json, lock_error_logs).execute(&mut **tx).await?; // Archive base. sqlx::query!( @@ -464,9 +473,8 @@ pub async fn clone_script<'c>( *s.hash, w_id ) - .execute(&mut *tx) + .execute(&mut **tx) .await?; - tx.commit().await?; - Ok(ClonedScript { old_script: ns, new_hash }) + Ok(new_hash) } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index c1d8fc00bc..697fc69e44 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -175,7 +175,7 @@ pub enum ObjectType { DatatableMigration, } -pub const LATEST_GIT_SYNC_SCRIPT_PATH: &str = "hub/28911/sync-script-to-git-repo-windmill"; +pub const LATEST_GIT_SYNC_SCRIPT_PATH: &str = "hub/28931/sync-script-to-git-repo-windmill"; /// Hub script that applies a repository's state back into a workspace /// (the repo → Windmill / "pull" direction). Same script the UI runs from @@ -183,7 +183,7 @@ pub const LATEST_GIT_SYNC_SCRIPT_PATH: &str = "hub/28911/sync-script-to-git-repo /// ignores the slug, so the slug is kept free of characters that would be /// percent-encoded into the run URL (a `:` becomes `%3A`, which some hardened /// reverse proxies reject as double-encoding when the client re-encodes it). -pub const GIT_SYNC_PULL_SCRIPT_PATH: &str = "hub/28910/git-sync-init-repository-windmill"; +pub const GIT_SYNC_PULL_SCRIPT_PATH: &str = "hub/28930/git-sync-init-repository-windmill"; /// Prefix used to identify fork workspaces. A workspace whose id starts with this string is a /// fork of another workspace. diff --git a/backend/windmill-dep-map/Cargo.toml b/backend/windmill-dep-map/Cargo.toml index 28b8552e8e..a93376d167 100644 --- a/backend/windmill-dep-map/Cargo.toml +++ b/backend/windmill-dep-map/Cargo.toml @@ -26,4 +26,5 @@ tracing.workspace = true lazy_static.workspace = true chrono.workspace = true itertools.workspace = true +futures.workspace = true uuid.workspace = true diff --git a/backend/windmill-dep-map/src/lib.rs b/backend/windmill-dep-map/src/lib.rs index ee6a3c3fd4..aab50795a9 100644 --- a/backend/windmill-dep-map/src/lib.rs +++ b/backend/windmill-dep-map/src/lib.rs @@ -1,6 +1,7 @@ pub mod ci_tests; #[cfg(feature = "private")] pub mod ci_tests_ee; +pub mod lock_hash; pub mod scoped_dependency_map; pub mod trigger_dependents; pub mod workspace_dependencies; @@ -127,6 +128,40 @@ pub fn extract_referenced_paths( } } +/// Re-records which paths `script_path` imports and what each one's lock hashes to right now. +/// That snapshot is what a later relock-skip check of this importer compares against, so it +/// has to move whenever the imports may have, whether or not the importer's own lock did. +/// +/// Writes for any path in `w_id` and checks nothing: callers are responsible for having +/// established access to that workspace and script, as a dependency job's push already has. +pub async fn refresh_dependency_map( + db: &sqlx::Pool, + w_id: &str, + script_path: &str, + parent_path: &Option, + code: &str, + script_lang: &Option, +) -> error::Result<()> { + use scoped_dependency_map::ScopedDependencyMap; + + let mut tx = db.begin().await?; + let mut dependency_map = + ScopedDependencyMap::fetch_maybe_rearranged(w_id, script_path, "script", parent_path, db) + .await?; + + tx = dependency_map + .patch( + extract_referenced_paths(code, script_path, *script_lang), + // Ideally should be None, but due to current implementation will use empty string to represent None. + "".into(), + tx, + ) + .await?; + + dependency_map.dissolve(tx).await.commit().await?; + Ok(()) +} + pub async fn process_relative_imports( db: &sqlx::Pool, _job_id: Option, @@ -144,29 +179,7 @@ pub async fn process_relative_imports( use scoped_dependency_map::ScopedDependencyMap; use trigger_dependents::trigger_dependents_to_recompute_dependencies; - // TODO: Should be moved into handle_dependency_job body to be more consistent with how flows and apps are handled - { - let mut tx = db.begin().await?; - let mut dependency_map = ScopedDependencyMap::fetch_maybe_rearranged( - &w_id, - script_path, - "script", - &parent_path, - db, - ) - .await?; - - tx = dependency_map - .patch( - extract_referenced_paths(&code, script_path, *script_lang), - // Ideally should be None, but due to current implementation will use empty string to represent None. - "".into(), - tx, - ) - .await?; - - dependency_map.dissolve(tx).await.commit().await?; - } + refresh_dependency_map(db, w_id, script_path, &parent_path, code, script_lang).await?; { let mut already_visited = args diff --git a/backend/windmill-dep-map/src/lock_hash.rs b/backend/windmill-dep-map/src/lock_hash.rs new file mode 100644 index 0000000000..50bd18a8ee --- /dev/null +++ b/backend/windmill-dep-map/src/lock_hash.rs @@ -0,0 +1,79 @@ +use std::collections::HashMap; + +use futures::TryStreamExt; +use sqlx::{Postgres, Transaction}; +use windmill_common::error::Result; +use windmill_common::scripts::hash_script; + +/// Records what the lock now at each path hashes to, which is one half of the comparison a relock +/// skip makes against what each importer resolved against. +/// +/// Writes any path in `w_id` and checks nothing: callers are responsible for having established +/// the caller's access to that workspace. A path repeated in `entries` keeps its last hash. +/// +/// Callers that write the lock itself in the same statement fold the upsert into that statement +/// instead; this is for the ones with nothing to fold it into. +pub async fn record_lock_hashes( + tx: &mut Transaction<'_, Postgres>, + w_id: &str, + entries: &[(String, i64)], +) -> Result<()> { + // Postgres rejects a whole statement that resolves a conflict on one key twice, so a path + // given more than once keeps its last hash, as it would if the two were written in order. + let mut deduped: HashMap<&str, i64> = HashMap::with_capacity(entries.len()); + for (path, hash) in entries { + deduped.insert(path.as_str(), *hash); + } + if deduped.is_empty() { + return Ok(()); + } + let (paths, hashes): (Vec, Vec) = deduped + .into_iter() + .map(|(path, hash)| (path.to_string(), hash)) + .unzip(); + // Recording a hash a path already has would still cut a row version, and the no-op push this + // is reached from is the mode a git-sync of an unchanged workspace runs in. + sqlx::query!( + "INSERT INTO lock_hash (workspace_id, path, lockfile_hash) + SELECT $1, * FROM UNNEST($2::text[], $3::bigint[]) + ON CONFLICT (workspace_id, path) DO UPDATE SET lockfile_hash = EXCLUDED.lockfile_hash + WHERE lock_hash.lockfile_hash IS DISTINCT FROM EXCLUDED.lockfile_hash", + w_id, + &paths[..], + &hashes[..] + ) + .execute(&mut **tx) + .await?; + Ok(()) +} + +/// Records the hash of every live lock in `w_id`, for a workspace whose scripts arrived without +/// going through a deploy — a clone, which copies their locks verbatim and so would otherwise hold +/// none of the hashes describing them. +/// +/// Carries the same caller obligation as [`record_lock_hashes`]. +/// +/// `script.lock` is unbounded and a workspace holds one per script, so the rows are streamed and +/// each lock is hashed and dropped before the next arrives; only the hashes accumulate. +pub async fn record_lock_hashes_for_workspace( + tx: &mut Transaction<'_, Postgres>, + w_id: &str, +) -> Result<()> { + let mut entries: Vec<(String, i64)> = Vec::new(); + { + let mut rows = sqlx::query!( + "SELECT DISTINCT ON (path) path, lock FROM script + WHERE workspace_id = $1 AND NOT archived AND NOT deleted AND lock IS NOT NULL + ORDER BY path, created_at DESC", + w_id + ) + .fetch(&mut **tx); + + while let Some(row) = rows.try_next().await? { + if let Some(lock) = row.lock { + entries.push((row.path, hash_script(&lock))); + } + } + } + record_lock_hashes(tx, w_id, &entries).await +} diff --git a/backend/windmill-mcp/src/common/schema.rs b/backend/windmill-mcp/src/common/schema.rs index 3f88f7e781..d49fa468ae 100644 --- a/backend/windmill-mcp/src/common/schema.rs +++ b/backend/windmill-mcp/src/common/schema.rs @@ -101,7 +101,7 @@ fn apply_resource_enrichment( 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{}", + "This is a resource named `{}` with the following description: `{}`.\nPass it as the bare string `$res:` — the whole value of this argument, never an object wrapper like {{\"$res\": \"\"}} and never a plain path.\n{}", rt.name, rt.description.as_deref().unwrap_or("No description"), if resources_count == 0 { @@ -138,7 +138,7 @@ fn apply_resource_enrichment( ) }) .collect::>() - .join("\\n"); + .join("\n"); let prior_description = prop_map .get("description") .and_then(Value::as_str) @@ -147,7 +147,7 @@ fn apply_resource_enrichment( 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{}", + "{}\nHere are the available resources, one per line as `title: $res:path`. The title is only a label; pass the `$res:path` part verbatim as this argument's value:\n{}", prior_description, resources_description )), ); @@ -804,6 +804,10 @@ mod tests { let desc = node["description"].as_str().unwrap(); assert!(desc.contains("c_aws_account")); assert!(desc.contains("$res:f/platform/aws_dev")); + // MCP clients render this description verbatim, so the separators must be + // real newlines rather than the two-character escape. + assert!(desc.contains('\n')); + assert!(!desc.contains("\\n")); } #[test] diff --git a/backend/windmill-mcp/src/server/backend.rs b/backend/windmill-mcp/src/server/backend.rs index e42c0dacfc..7ca1eb08a1 100644 --- a/backend/windmill-mcp/src/server/backend.rs +++ b/backend/windmill-mcp/src/server/backend.rs @@ -16,6 +16,14 @@ use crate::server::endpoints::EndpointTool; /// Result type for backend operations using rmcp's ErrorData directly pub type BackendResult = Result; +/// What the backend needs about the HTTP request a tool call arrived on, in order +/// to hand a runnable the headers of the call that triggered it. +pub struct McpRequest<'a> { + pub headers: &'a http::HeaderMap, + /// The MCP tool name the caller invoked, reported to preprocessors. + pub tool_name: &'a str, +} + /// How a script/flow listing is narrowed by path at the SQL layer, *before* the /// `ITEMS_FETCH_MAX_LIMIT` cap applies. /// @@ -157,6 +165,7 @@ pub trait McpBackend: Send + Sync + Clone + 'static { workspace_id: &str, path: &str, args: Value, + request: &McpRequest<'_>, ) -> BackendResult; /// Run a flow and wait for result @@ -166,6 +175,7 @@ pub trait McpBackend: Send + Sync + Clone + 'static { workspace_id: &str, path: &str, args: Value, + request: &McpRequest<'_>, ) -> BackendResult; /// Call an endpoint tool (generated API endpoint) diff --git a/backend/windmill-mcp/src/server/mod.rs b/backend/windmill-mcp/src/server/mod.rs index b6fb7a5b0a..b97e374e98 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 crate::common::types::{McpToken, MultiWorkspaceMcp, WorkspaceInfo}; -pub use backend::{BackendResult, McpAuth, McpBackend, PathFilter}; +pub use backend::{BackendResult, McpAuth, McpBackend, McpRequest, PathFilter}; pub use endpoints::{ endpoint_tool_to_mcp_tool, endpoint_tool_to_mcp_tool_multi, is_endpoint_read_only, list_workspaces_tool, non_empty_body_fields, EndpointTool, diff --git a/backend/windmill-mcp/src/server/runner.rs b/backend/windmill-mcp/src/server/runner.rs index f407504c4b..e27eaa8470 100644 --- a/backend/windmill-mcp/src/server/runner.rs +++ b/backend/windmill-mcp/src/server/runner.rs @@ -9,8 +9,10 @@ use crate::common::transform::{ extract_hub_version_id_from_hashed, extract_path_prefix_from_hashed, parse_tool_prefix, reverse_transform, reverse_transform_key, }; -use crate::common::types::{McpToken, MultiWorkspaceMcp, ResourceInfo, ToolableItem, WorkspaceId}; -use crate::server::backend::{McpAuth, McpBackend, PathFilter}; +use crate::common::types::{ + McpToken, MultiWorkspaceMcp, ResourceInfo, SchemaType, ToolableItem, WorkspaceId, +}; +use crate::server::backend::{McpAuth, McpBackend, McpRequest, PathFilter}; use crate::server::endpoints::{ endpoint_tool_to_mcp_tool, endpoint_tool_to_mcp_tool_multi, list_workspaces_tool, EndpointTool, }; @@ -101,16 +103,24 @@ enum McpMode { Multi(String), } +/// Everything a request carries besides its MCP payload. +struct McpContext { + auth: A, + mode: McpMode, + headers: http::HeaderMap, +} + impl Runner { /// Create a new Runner with the given backend pub fn new(backend: B) -> Self { Self { backend: Arc::new(backend) } } - /// Extract authentication and the workspace mode from request context + /// Extract authentication, the workspace mode and the HTTP request itself + /// from the request context fn extract_context( context: &RequestContext, - ) -> Result<(B::Auth, McpMode), ErrorData> { + ) -> Result, ErrorData> { let http_parts = context.extensions.get::().ok_or_else(|| { tracing::error!("http::request::Parts not found"); ErrorData::internal_error("http::request::Parts not found", None) @@ -148,7 +158,7 @@ impl Runner { McpMode::Single(workspace_id) }; - Ok((auth.clone(), mode)) + Ok(McpContext { auth: auth.clone(), mode, headers: http_parts.headers.clone() }) } } @@ -391,6 +401,18 @@ fn authorize_endpoint_call( Ok(()) } +/// Map the model's argument keys back to the runnable's original parameter names. +fn transform_call_args(args: Value, item_schema: &Option) -> Value { + let Value::Object(map) = args else { + return args; + }; + let mut args_hash = HashMap::new(); + for (k, v) in map { + args_hash.insert(reverse_transform_key(&k, item_schema), v); + } + Value::Object(args_hash.into_iter().collect()) +} + fn find_matching_path(candidates: Vec, request_name: &str) -> Option { candidates .into_iter() @@ -427,7 +449,7 @@ impl ServerHandler for Runner { _request: Option, context: RequestContext, ) -> Result { - let (auth, mode) = Self::extract_context(&context)?; + let McpContext { auth, mode, .. } = Self::extract_context(&context)?; // Parse MCP scopes to determine what to expose let scopes = auth.scopes().unwrap_or(&[]); @@ -455,7 +477,7 @@ impl ServerHandler for Runner { request: CallToolRequestParams, context: RequestContext, ) -> Result { - let (auth, mode) = Self::extract_context(&context)?; + let McpContext { auth, mode, headers } = Self::extract_context(&context)?; // Parse MCP scopes for authorization let scopes = auth.scopes().unwrap_or(&[]); @@ -464,6 +486,7 @@ impl ServerHandler for Runner { let read_only = auth.read_only(); let args = request.arguments.map(Value::Object).unwrap_or(Value::Null); + let mcp_request = McpRequest { headers: &headers, tool_name: request.name.as_ref() }; // Every tool here runs to completion in one round trip: none of them ask the // client for input, so the MRTR variants of `CallToolResponse` are never built. @@ -474,14 +497,22 @@ impl ServerHandler for Runner { &workspace_id, &scope_config, read_only, - request.name, + request.name.clone(), args, + &mcp_request, ) .await } McpMode::Multi(token) => { - self.call_tool_multi(&auth, &token, &scope_config, read_only, request.name, args) - .await + self.call_tool_multi( + &auth, + &token, + &scope_config, + read_only, + request.name.clone(), + args, + ) + .await } }?; Ok(result.into()) @@ -665,6 +696,7 @@ impl Runner { read_only: bool, name: std::borrow::Cow<'static, str>, args: Value, + request: &McpRequest<'_>, ) -> Result { // Check if this is an endpoint tool let endpoint_tools = self.backend.all_endpoint_tools(); @@ -777,17 +809,7 @@ impl Runner { .map_err(|e| ErrorData::internal_error(e.message, None))? }; - // Transform arguments back to original key names - let transformed_args = if let Value::Object(map) = args { - let mut args_hash = HashMap::new(); - for (k, v) in map { - let original_key = reverse_transform_key(&k, &item_schema); - args_hash.insert(original_key, v); - } - Value::Object(args_hash.into_iter().collect()) - } else { - args - }; + let transformed_args = transform_call_args(args, &item_schema); let script_or_flow_path = if is_hub { format!("hub/{}", path) @@ -798,11 +820,23 @@ impl Runner { // Execute script or flow let result = if tool_type == "script" { self.backend - .run_script(auth, workspace_id, &script_or_flow_path, transformed_args) + .run_script( + auth, + workspace_id, + &script_or_flow_path, + transformed_args, + request, + ) .await } else { self.backend - .run_flow(auth, workspace_id, &script_or_flow_path, transformed_args) + .run_flow( + auth, + workspace_id, + &script_or_flow_path, + transformed_args, + request, + ) .await }; diff --git a/backend/windmill-queue/src/jobs.rs b/backend/windmill-queue/src/jobs.rs index c7c119a5a7..8ac4fd390a 100644 --- a/backend/windmill-queue/src/jobs.rs +++ b/backend/windmill-queue/src/jobs.rs @@ -4007,33 +4007,10 @@ async fn clone_runnable(j: &mut PulledJob, db: &DB) -> error::Result<()> { { let maybe_new_id = match j.kind { - JobKind::Dependencies => { - let deployment_message = j - .args - .clone() - .map(|hashmap| { - hashmap - .get("deployment_message") - .map(|map_value| serde_json::from_str::(map_value.get()).ok()) - .flatten() - }) - .flatten(); - - // This way we tell downstream which script we should archive when the resolution is finished. - // (not used at the moment) - j.args - .as_mut() - .map(|args| args.insert("base_hash".to_owned(), to_raw_value(&*base_hash))); - - windmill_common::scripts::clone_script( - j.runnable_path(), - &j.workspace_id, - deployment_message, - db, - ) - .await? - .new_hash - } + // A script gets its new version from the worker, once the generated lock is known + // to differ from the live version's: minting one here would deploy, and walk the + // importers of, a version whose lock turns out byte-identical to its parent's. + JobKind::Dependencies => *base_hash, JobKind::FlowDependencies => { sqlx::query_scalar!( "INSERT INTO flow_version diff --git a/backend/windmill-types/src/scripts.rs b/backend/windmill-types/src/scripts.rs index 414650e846..19f6a06021 100644 --- a/backend/windmill-types/src/scripts.rs +++ b/backend/windmill-types/src/scripts.rs @@ -16,7 +16,7 @@ use crate::{ runnable_settings::{ConcurrencySettings, DebouncingSettings}, }; -#[derive(Serialize, Deserialize, Debug, Clone, Hash)] +#[derive(Serialize, Deserialize, Debug, Clone, Hash, PartialEq, Eq)] pub struct ScriptModule { pub content: String, pub language: ScriptLang, diff --git a/backend/windmill-worker/src/worker_lockfiles.rs b/backend/windmill-worker/src/worker_lockfiles.rs index 57b859075a..9bba9a99e6 100644 --- a/backend/windmill-worker/src/worker_lockfiles.rs +++ b/backend/windmill-worker/src/worker_lockfiles.rs @@ -19,14 +19,17 @@ use windmill_common::error::Result; use windmill_common::flows::{FlowModule, FlowModuleValue, FlowNodeId}; use windmill_common::jobs::JobKind; use windmill_common::min_version::MIN_VERSION_SUPPORTS_DEBOUNCING_V2; -use windmill_common::scripts::ScriptHash; +use windmill_common::scripts::{ + deploy_relocked_version, fetch_script_for_update, hash_script, ScriptHash, ScriptModule, +}; #[cfg(feature = "python")] use windmill_common::worker::PythonAnnotations; use windmill_common::worker::{to_raw_value, to_raw_value_owned, write_file, Connection}; use windmill_common::workspace_dependencies::{ RawWorkspaceDependencies, WorkspaceDependenciesPrefetched, }; -use windmill_dep_map::scoped_dependency_map::ScopedDependencyMap; +use windmill_dep_map::scoped_dependency_map::{DependencyDependent, ScopedDependencyMap}; +use windmill_dep_map::trigger_dependents::trigger_dependents_to_recompute_dependencies; #[cfg(feature = "python")] use windmill_parser_yaml::AnsibleRequirements; @@ -38,8 +41,10 @@ use windmill_common::{ scripts::ScriptLang, DB, }; +use windmill_dep_map::lock_hash::record_lock_hashes; pub use windmill_dep_map::{ extract_referenced_paths, extract_relative_imports, process_relative_imports, + refresh_dependency_map, }; use windmill_git_sync::{ handle_deployment_metadata, tally_deployed_object_changes, DeployedObject, @@ -86,10 +91,13 @@ use crate::{ /// has the toolchain and, since the cache key is per OS/arch, the platform the runtime /// workers use. Deploys that supply their own lock never reach a dependency job at all and /// queue theirs from `create_script_internal` instead. -async fn maybe_queue_binary_prebuild(db: &DB, job: &MiniPulledJob, lock: &str) -> Result<()> { - let (Some(hash), Some(path), Some(lang)) = - (job.runnable_id, job.runnable_path.clone(), job.script_lang) - else { +async fn maybe_queue_binary_prebuild( + db: &DB, + job: &MiniPulledJob, + hash: ScriptHash, + lock: &str, +) -> Result<()> { + let (Some(path), Some(lang)) = (job.runnable_path.clone(), job.script_lang) else { return Ok(()); }; let Some(prebuild) = @@ -283,6 +291,13 @@ pub async fn handle_dependency_job( job.runnable_path() ); let script_path = job.runnable_path(); + let w_id = &job.workspace_id; + + let triggered_by_relative_import = job + .args + .as_ref() + .map(|x| x.get("triggered_by_relative_import").is_some()) + .unwrap_or_default(); // A build pass reads the same script data but writes none of the deploy state below, // including the `lock_error_logs` stamp on a fetch failure: the version it builds is @@ -296,14 +311,48 @@ pub async fn handle_dependency_job( *deployment_tallied = true; } + // A relative-import relock deploys nothing until `commit_relock` says so, while the + // caller's fallback tally assumes a failed dependency job left a deployed version behind. + // Claim the tally here; the failure path hands it back once it has minted the version + // that carries the error. + if triggered_by_relative_import { + *deployment_tallied = true; + } + + // A relative-import relock locks the path's live version as of now, not the hash captured + // when the job was pushed: a deploy can land during the debounce delay, after which that + // hash names an archived version. What it generates is committed against the live version + // re-read under a row lock, so a deploy landing mid-generation is caught there too. + let target_hash = if triggered_by_relative_import { + Some(ScriptHash(live_head_hash(db, w_id, script_path).await?)) + } else { + job.runnable_id + }; + // `JobKind::Dependencies` job store either: // - A saved script `hash` in the `script_hash` column. // - Preview raw lock and code in the `queue` or `job` table. - let script_data = &match job.runnable_id { + let script_data = &match target_hash { + // Read straight from the database: the cache pins a version's data under its hash for + // as long as this worker lives, and the live version may still be waiting on its own + // dependency job's lock, which lands in place. A run resolving to it on this worker + // would then get no lock from the cache at all. + Some(hash) if triggered_by_relative_import => { + let raw = cache::script::fetch_script_from_db(db, hash, std::panic::Location::caller()) + .await?; + Cow::Owned(std::sync::Arc::new(cache::ScriptData { + lock: raw.lock, + code: raw.content, + modules: raw.modules, + })) + } Some(hash) => match cache::script::fetch(&Connection::from(db.clone()), hash).await { Ok(d) => Cow::Owned(d.0), Err(e) => { - if !is_build_job { + // The live version of a relative-import relock is what runs resolve to, and + // `lock_error_logs` on it takes it out of resolution; the job carries the + // error instead, since it deployed nothing. + if !is_build_job && !triggered_by_relative_import { let logs2 = sqlx::query_scalar!( "SELECT logs FROM job_logs WHERE job_id = $1 AND workspace_id = $2", &job.id, @@ -348,12 +397,6 @@ pub async fn handle_dependency_job( .await; } - let triggered_by_relative_import = job - .args - .as_ref() - .map(|x| x.get("triggered_by_relative_import").is_some()) - .unwrap_or_default(); - // Extract temp_script_refs from job args (path -> hash mapping for temp storage) let temp_script_refs: Option> = job .args @@ -391,20 +434,17 @@ pub async fn handle_dependency_job( ) .await; + let (deployment_message, parent_path) = + get_deployment_msg_and_parent_path_from_args(job.args.clone()); + match content { Ok(content) => { - if job.runnable_id.is_none() { + let Some(current_hash) = target_hash else { // it a one-off raw script dependency job, no need to update the db return Ok(to_raw_value_owned( json!({ "status": "Successful lock file generation", "lock": content }), )); - } - - let current_hash = job.runnable_id.unwrap_or(ScriptHash(0)); - let w_id = &job.workspace_id; - - let (deployment_message, parent_path) = - get_deployment_msg_and_parent_path_from_args(job.args.clone()); + }; // Generate lockfiles for module files (if any). // @@ -441,7 +481,9 @@ pub async fn handle_dependency_job( occupancy_metrics, &raw_workspace_dependencies_o, module.lock.as_deref(), - triggered_by_relative_import, + // A module that was never locked has nothing a skip could hand + // back; the path's lock is the parent script's, not its own. + triggered_by_relative_import && module.lock.is_some(), script_path, None, "script", @@ -464,34 +506,83 @@ pub async fn handle_dependency_job( None }; - // We do not create new row for this update - // That means we can keep current hash and just update lock - // Also store lockfile hash for dependency change detection - let lockfile_hash = windmill_common::scripts::hash_script(&content); - let updated_modules_json = updated_modules - .as_ref() - .and_then(|m| serde_json::to_value(m).ok()); - sqlx::query!( - "WITH update_lock AS ( - UPDATE script SET lock = $1, modules = COALESCE($6, modules) WHERE hash = $2 AND workspace_id = $3 + let deployed_hash = if triggered_by_relative_import { + match commit_relock( + db, + w_id, + script_path, + current_hash, + Some(&content), + updated_modules.as_ref(), + None, + deployment_message.clone(), ) - INSERT INTO lock_hash (workspace_id, path, lockfile_hash) - VALUES ($3, $4, $5) - ON CONFLICT (workspace_id, path) DO UPDATE SET lockfile_hash = $5", - &content, - ¤t_hash.0, - w_id, - script_path, - &lockfile_hash, - updated_modules_json - ) - .execute(db) - .await?; + .await? + { + RelockOutcome::Deployed(hash) => hash, + RelockOutcome::Unchanged => { + let log_msg = "\nLock unchanged: no new version deployed"; + tracing::info!(workspace_id = %w_id, job_id = %job.id, "{log_msg}"); + append_logs(&job.id, w_id, log_msg, &db.into()).await; + // The imports may have moved even though the result did not, and the + // map is what this importer's next skip check reads. + refresh_dependency_map( + db, + w_id, + script_path, + &parent_path, + &script_data.code, + &job.script_lang, + ) + .await?; + return Ok(to_raw_value_owned( + json!({ "status": "Lock unchanged, no new version deployed", "lock": content }), + )); + } + RelockOutcome::Superseded(head) => { + let log_msg = format!( + "\nVersion {head} was deployed while this lock was generated; discarding it and queueing a relock of that version" + ); + tracing::info!(workspace_id = %w_id, job_id = %job.id, "{log_msg}"); + append_logs(&job.id, w_id, log_msg, &db.into()).await; + requeue_relock(db, job, script_path, deployment_message, parent_path) + .await?; + return Ok(to_raw_value_owned( + json!({ "status": "Lock generation superseded by a newer version", "lock": content }), + )); + } + } + } else { + // We do not create new row for this update + // That means we can keep current hash and just update lock + // Also store lockfile hash for dependency change detection + let lockfile_hash = windmill_common::scripts::hash_script(&content); + let updated_modules_json = updated_modules + .as_ref() + .and_then(|m| serde_json::to_value(m).ok()); + sqlx::query!( + "WITH update_lock AS ( + UPDATE script SET lock = $1, modules = COALESCE($6, modules) WHERE hash = $2 AND workspace_id = $3 + ) + INSERT INTO lock_hash (workspace_id, path, lockfile_hash) + VALUES ($3, $4, $5) + ON CONFLICT (workspace_id, path) DO UPDATE SET lockfile_hash = $5", + &content, + ¤t_hash.0, + w_id, + script_path, + &lockfile_hash, + updated_modules_json + ) + .execute(db) + .await?; - // `lock` has been updated; invalidate the cache. - // Since only worker that ran this Dependency Job has the cache - // we do not need to think about invalidating cache for other workers. - cache::script::invalidate(current_hash); + // `lock` has been updated; invalidate the cache. + // Since only worker that ran this Dependency Job has the cache + // we do not need to think about invalidating cache for other workers. + cache::script::invalidate(current_hash); + current_hash + }; // The version only became runnable now, so this process still resolves the path to // the one before it. Only the runnable-hash cache: the import-side caches ignore the // lock, so evicting this process' half of that pair here would key a bundle by a @@ -504,7 +595,7 @@ pub async fn handle_dependency_job( &db, &w_id, DeployedObject::Script { - hash: current_hash, + hash: deployed_hash, path: script_path.to_string(), parent_path: parent_path.clone(), }, @@ -565,7 +656,7 @@ pub async fn handle_dependency_job( }); } - if let Err(e) = maybe_queue_binary_prebuild(db, job, &content).await { + if let Err(e) = maybe_queue_binary_prebuild(db, job, deployed_hash, &content).await { tracing::error!(%e, "error queueing the auto-build binary job for {script_path}"); } @@ -583,14 +674,49 @@ pub async fn handle_dependency_job( .await? .flatten() .unwrap_or_else(|| "no logs".to_string()); - sqlx::query!( - "UPDATE script SET lock_error_logs = $1 WHERE hash = $2 AND workspace_id = $3", - &format!("{logs2}\n{error}"), - &job.runnable_id.unwrap_or(ScriptHash(0)).0, - &job.workspace_id - ) - .execute(db) - .await?; + let error_logs = format!("{logs2}\n{error}"); + if let (true, Some(hash)) = (triggered_by_relative_import, target_hash) { + // The same shape a failed deploy leaves: a version without a lock that carries + // the error, so it shows on the script while runs keep resolving to the last + // version that has one. Only that version is the caller's fallback to tally; + // one that landed meanwhile owns its own lock, and a commit that failed left + // nothing. + match commit_relock( + db, + w_id, + script_path, + hash, + None, + None, + Some(&error_logs), + deployment_message.clone(), + ) + .await + { + Ok(RelockOutcome::Deployed(_)) => *deployment_tallied = false, + Ok(RelockOutcome::Superseded(_)) => { + if let Err(e) = + requeue_relock(db, job, script_path, deployment_message, parent_path) + .await + { + tracing::error!(%e, "error queueing a relock of {script_path}") + } + } + Ok(RelockOutcome::Unchanged) => {} + Err(e) => { + tracing::error!(%e, "error recording the failed relock of {script_path}") + } + } + } else { + sqlx::query!( + "UPDATE script SET lock_error_logs = $1 WHERE hash = $2 AND workspace_id = $3", + &error_logs, + &job.runnable_id.unwrap_or(ScriptHash(0)).0, + &job.workspace_id + ) + .execute(db) + .await?; + } Err(Error::ExecutionErr(format!( "Error locking file: {error}\n\nlogs:\n{}", remove_ansi_codes(&logs2) @@ -598,6 +724,136 @@ pub async fn handle_dependency_job( } } } + +/// The version of `script_path` that runs resolve to, which is what a relative-import relock +/// locks. `NotFound` when the path holds none, which a job pushed for a path since archived or +/// deleted reports as its own failure. +async fn live_head_hash(db: &DB, w_id: &str, script_path: &str) -> error::Result { + sqlx::query_scalar!( + "SELECT hash FROM script WHERE path = $1 AND workspace_id = $2 AND deleted = false AND archived = false ORDER BY created_at DESC LIMIT 1", + script_path, + w_id + ) + .fetch_optional(db) + .await? + .ok_or_else(|| { + Error::NotFound(format!( + "Non-archived script with path '{script_path}' not found" + )) + }) +} + +enum RelockOutcome { + /// A new version carrying the result is the live one. + Deployed(ScriptHash), + /// The live version already holds this lock and these module locks; nothing was written. + Unchanged, + /// The live version is no longer the one the lock was generated for; nothing was written. + Superseded(ScriptHash), +} + +/// Commits what a relative-import relock produced against the path's live version, read under +/// a row lock so relocks of one path serialize on it. +/// +/// A result equal to the live version's lock and module locks writes nothing: the importer's +/// dependencies did not move, and a new version would deploy byte-identical content and then +/// walk its own importers for nothing. A `lock` of `None` is a failed generation and always +/// deploys, as the version that carries the error. +async fn commit_relock( + db: &DB, + w_id: &str, + script_path: &str, + generated_for: ScriptHash, + lock: Option<&str>, + modules: Option<&HashMap>, + lock_error_logs: Option<&str>, + deployment_message: Option, +) -> error::Result { + let mut tx = db.begin().await?; + let mut head = None; + for _ in 0..4 { + head = fetch_script_for_update(script_path, w_id, &mut *tx).await?; + if head.is_some() { + break; + } + // Having waited on the live version's row lock, the statement re-checked that row + // once the holder committed, found it archived, and returned nothing: the successor + // the holder inserted is not in the statement's snapshot. A fresh statement sees it, + // unless yet another writer got there first, so this goes around a few times before + // concluding the path holds no live version. + } + let Some(head) = head else { + return Err(Error::NotFound(format!( + "Non-archived script with path '{script_path}' not found" + ))); + }; + if head.hash != generated_for { + // A deploy landed while the lock was generated. It carried its own lock or queued its + // own dependency job, and this lock describes content that is no longer live. + return Ok(RelockOutcome::Superseded(head.hash)); + } + let lock_hash_entry = lock.map(|lock| (script_path.to_string(), hash_script(lock))); + if let Some(lock) = lock { + let modules_unchanged = modules.map_or(true, |m| head.modules.as_ref() == Some(m)); + if head.lock.as_deref() == Some(lock) && modules_unchanged { + // The hash row is still written, and under the same row lock: a version deployed + // before lock hashes were recorded has none, so its importers cannot skip until + // it does, and a deploy that takes the lock next must not have the hash it records + // overwritten by this one. + record_lock_hashes(&mut tx, w_id, lock_hash_entry.as_slice()).await?; + tx.commit().await?; + return Ok(RelockOutcome::Unchanged); + } + } + let new_hash = deploy_relocked_version( + &mut tx, + head, + deployment_message, + lock, + modules, + lock_error_logs, + ) + .await?; + record_lock_hashes(&mut tx, w_id, lock_hash_entry.as_slice()).await?; + tx.commit().await?; + Ok(RelockOutcome::Deployed(ScriptHash(new_hash))) +} + +/// Queues another relative-import relock of `script_path`, through the same push the fan-out +/// uses. The version live now was deployed while a lock was generated for its predecessor; +/// when that deploy was a sibling relock it queued nothing for this path, and the result just +/// discarded may have been the one generated against the current imports. +async fn requeue_relock( + db: &DB, + job: &MiniPulledJob, + script_path: &str, + deployment_message: Option, + parent_path: Option, +) -> error::Result<()> { + let already_visited = job + .args + .as_ref() + .and_then(|x| x.get("already_visited")) + .and_then(|v| serde_json::from_str::>(v.get()).ok()) + .unwrap_or_default(); + trigger_dependents_to_recompute_dependencies( + &job.workspace_id, + vec![DependencyDependent { + importer_path: script_path.to_string(), + importer_kind: "script".to_string(), + importer_node_ids: None, + }], + deployment_message, + parent_path, + &job.permissioned_as_email, + &job.created_by, + &job.permissioned_as, + db, + already_visited, + ) + .await +} + fn remove_ansi_codes(s: &str) -> String { lazy_static::lazy_static! { static ref ANSI_REGEX: regex::Regex = regex::Regex::new(r"\x1b\[[0-9;]*[a-zA-Z]").unwrap(); @@ -2892,8 +3148,12 @@ async fn try_skip_relock( } // Fetch existing lock based on runnable type - let lock = match runnable_type { - "script" => sqlx::query_scalar!( + let lock = match (runnable_type, existing_lock) { + // A script's module asks with the script's own type and hands over the lock it last + // deployed with. The path's lock below is the parent script's, and a module given + // that loses whatever it resolves on its own. + ("script", Some(module_lock)) => Some(module_lock.to_string()), + ("script", None) => sqlx::query_scalar!( "SELECT lock FROM script WHERE path = $1 AND workspace_id = $2 AND lock IS NOT NULL AND deleted = false ORDER BY created_at DESC LIMIT 1", base_path, @@ -2903,7 +3163,7 @@ async fn try_skip_relock( .await? .flatten(), - "flow" | "app" => existing_lock.map(|s| s.to_string()), + ("flow" | "app", existing_lock) => existing_lock.map(|s| s.to_string()), _ => None, }; diff --git a/benchmarks/lib.ts b/benchmarks/lib.ts index db9077b9cc..6a165901e4 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.801.0"; +export const VERSION = "v1.803.0"; export async function login(email: string, password: string): Promise { return await windmill.UserService.login({ diff --git a/cli/src/commands/flow/flow.ts b/cli/src/commands/flow/flow.ts index 6ace1eaf48..7a383fcfeb 100644 --- a/cli/src/commands/flow/flow.ts +++ b/cli/src/commands/flow/flow.ts @@ -1144,7 +1144,7 @@ const command = new Command() .arguments("") .option( "-d --data ", - "Inputs specified as a JSON string or a file using @ or stdin using @-." + "Inputs specified as a JSON string or a file using @ or stdin using @-. A resource argument is the bare string $res: as its whole value, and a variable argument is the bare string $var: — not an object wrapper keyed on $res/$var, and not a plain path." ) .option( "-s --silent", @@ -1162,7 +1162,7 @@ const command = new Command() .arguments("") .option( "-d --data ", - "Inputs specified as a JSON string or a file using @ or stdin using @-." + "Inputs specified as a JSON string or a file using @ or stdin using @-. A resource argument is the bare string $res: as its whole value, and a variable argument is the bare string $var: — not an object wrapper keyed on $res/$var, and not a plain path." ) .option( "-s --silent", diff --git a/cli/src/commands/schedule/schedule.ts b/cli/src/commands/schedule/schedule.ts index edbedc2612..2eb6ba42b8 100644 --- a/cli/src/commands/schedule/schedule.ts +++ b/cli/src/commands/schedule/schedule.ts @@ -106,7 +106,8 @@ export async function pushSchedule( path: string, schedule: Schedule | ScheduleFile | undefined, localSchedule: ScheduleFile, - permissionedAsContext?: PermissionedAsContext + permissionedAsContext?: PermissionedAsContext, + enabledOwnedByParent?: boolean ): Promise { path = removeType(path, "schedule").replaceAll(SEP, "/"); log.debug(`Processing local schedule ${path}`); @@ -123,6 +124,21 @@ export async function pushSchedule( // Strip CLI-only boolean marker before sending to API delete (localSchedule as any).has_permissioned_as; + // In a fork, the file's `enabled` is the parent's for a path the parent + // also has (see sync push's `parentOwnedScheduleEnabled`): the fork's own + // flag stays as it is. + if (enabledOwnedByParent && schedule) { + if ( + localSchedule.enabled !== undefined && + localSchedule.enabled !== schedule.enabled + ) { + log.warnAlways( + `Schedule ${path} stays ${schedule.enabled ? "enabled" : "disabled"}: the file says ${localSchedule.enabled ? "enabled" : "disabled"}, but in a fork that flag is the parent workspace's` + ); + } + delete localSchedule.enabled; + } + const preserveFields: { permissioned_as?: string; preserve_permissioned_as?: boolean } = {}; if (permissionedAsContext?.userIsAdminOrDeployer) { if (schedule) { @@ -153,13 +169,9 @@ export async function pushSchedule( ...preserveFields, }, }); - // Tarball export from a fork strips `enabled` from schedule YAMLs so - // the fork→parent git-sync round-trip can't flip the parent's state. - // Skip the secondary setScheduleEnabled call when the local YAML - // doesn't carry `enabled` — sending `{ enabled: undefined }` would - // serialize to `{}` and the backend (`SetEnabled.enabled` is required) - // would reject the request. Preserving the target's existing flag is - // exactly the round-trip-safe behavior. + // No `enabled` in the file (absent from the YAML, or set aside above) + // leaves the remote flag alone: `SetEnabled.enabled` is required, so + // `{ enabled: undefined }` would be rejected rather than ignored. if ( localSchedule.enabled !== undefined && localSchedule.enabled !== schedule.enabled @@ -167,13 +179,12 @@ export async function pushSchedule( log.info(colors.bold.yellow( `Schedule ${path} is ${localSchedule.enabled ? "enabled" : "disabled"} locally but not on remote, updating remote` )); - await wmill.setScheduleEnabled({ - workspace: workspace, + await setEnabledUnlessParentOwned( + workspace, path, - requestBody: { - enabled: localSchedule.enabled, - }, - }); + localSchedule.enabled, + schedule.enabled + ); } } catch (e) { console.error((e as any).body); @@ -194,6 +205,44 @@ export async function pushSchedule( console.error((e as any).body); throw e; } + // A create in a fork lands disabled whatever the request says. A fork-only + // path the file wants enabled is enabled here, so one push converges; a + // parent-owned one stays disabled. + if (enabledOwnedByParent !== undefined && localSchedule.enabled === true) { + if (enabledOwnedByParent) { + log.warnAlways( + `Schedule ${path} created disabled: the file says enabled, but in a fork that flag is the parent workspace's` + ); + } else { + await setEnabledUnlessParentOwned(workspace, path, true, false); + } + } + } +} + +// The parent listing behind `enabledOwnedByParent` sees only what the pusher +// may read; the backend's `fork-conflict` refusal is the last word, so a path +// it says the parent has keeps the fork's flag rather than failing the push. +async function setEnabledUnlessParentOwned( + workspace: string, + path: string, + enabled: boolean, + remoteEnabled: boolean +): Promise { + try { + await wmill.setScheduleEnabled({ + workspace, + path, + requestBody: { enabled }, + }); + } catch (e) { + const conflict = parseForkConflict(e); + if (!conflict) { + throw e; + } + log.warnAlways( + `Schedule ${path} left ${remoteEnabled ? "enabled" : "disabled"}: the parent workspace '${conflict.parentWorkspaceId}' has the same schedule, so its flag is the parent's to set` + ); } } diff --git a/cli/src/commands/script/script.ts b/cli/src/commands/script/script.ts index 8c93bdb963..fada434e0d 100644 --- a/cli/src/commands/script/script.ts +++ b/cli/src/commands/script/script.ts @@ -2213,7 +2213,7 @@ const command = new Command() .arguments("") .option( "-d --data ", - "Inputs specified as a JSON string or a file using @ or stdin using @-." + "Inputs specified as a JSON string or a file using @ or stdin using @-. A resource argument is the bare string $res: as its whole value, and a variable argument is the bare string $var: — not an object wrapper keyed on $res/$var, and not a plain path." ) .option( "-s --silent", @@ -2231,7 +2231,7 @@ const command = new Command() .arguments("") .option( "-d --data ", - "Inputs specified as a JSON string or a file using @ or stdin using @-." + "Inputs specified as a JSON string or a file using @ or stdin using @-. A resource argument is the bare string $res: as its whole value, and a variable argument is the bare string $var: — not an object wrapper keyed on $res/$var, and not a plain path." ) .option( "-s --silent", diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 6aaa72da0d..bab9967b3c 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -25,7 +25,7 @@ import { } from "yaml"; import JSZip from "jszip"; import { minimatch } from "minimatch"; -import { yamlParseContent } from "../../utils/yaml.ts"; +import { yamlParseContent, yamlParseFile } from "../../utils/yaml.ts"; import * as wmill from "../../../gen/services.gen.ts"; import { @@ -1138,7 +1138,7 @@ export function rawAppPathWithinFolder( return resolved; } -function ZipFSElement( +export function ZipFSElement( zip: JSZip, useYaml: boolean, defaultTs: "bun" | "deno", @@ -1146,6 +1146,14 @@ function ZipFSElement( resourceTypeToIsFileset: Record, ignoreCodebaseChanges: boolean, stripOnBehalfOf: boolean, + // Names a flow's rendered inline-script files after the checkout's own + // `!inline` references (module id -> file). The export carries script + // source, never a reference, so without a checkout to defer to every file + // is named from the step summary, and a file the checkout names otherwise + // reads as a delete + add on every push while the resolved flows are equal. + localFlowInlineMapping?: ( + flowDir: string, + ) => Promise>, ): DynFSElement { // Pre-scan: find zip base paths of scripts that have modules. // These scripts use the folder layout: {basePath}__mod/script.{ext} @@ -1249,59 +1257,71 @@ function ZipFSElement( log.error(`Failed to parse flow.yaml at path: ${p}`); throw error; } - let inlineScripts; + let inlineScripts: InlineScript[]; try { - const assigner = newPathAssigner(defaultTs, { - skipInlineScriptSuffix: getNonDottedPaths(), - }); - // Preserve original !inline filenames from the flow to avoid phantom renames - const inlineMapping = extractCurrentMapping( - flow.value.modules as any, - {}, - flow.value.failure_module, - flow.value.preprocessor_module, - ); - inlineScripts = extractInlineScriptsForFlows( - flow.value.modules as any, - inlineMapping, - SEP, - defaultTs, - assigner, - { + // Extraction rewrites the modules' content into `!inline` refs, + // so each attempt works on its own copy of the flow. + const render = ( + source: OpenFlow, + inlineMapping: Record, + ): [OpenFlow, InlineScript[]] => { + const f: OpenFlow = structuredClone(source); + const assigner = newPathAssigner(defaultTs, { + skipInlineScriptSuffix: getNonDottedPaths(), + }); + const options = { skipInlineScriptSuffix: getNonDottedPaths(), failOnInlineDirective: true, - }, - ); - if (flow.value.failure_module) { - inlineScripts.push( - ...extractInlineScriptsForFlows( - [flow.value.failure_module], - inlineMapping, - SEP, - defaultTs, - assigner, - { - skipInlineScriptSuffix: getNonDottedPaths(), - failOnInlineDirective: true, - }, - ), - ); - } - if (flow.value.preprocessor_module) { - inlineScripts.push( - ...extractInlineScriptsForFlows( - [flow.value.preprocessor_module], - inlineMapping, - SEP, - defaultTs, - assigner, - { - skipInlineScriptSuffix: getNonDottedPaths(), - failOnInlineDirective: true, - }, - ), + }; + const scripts = extractInlineScriptsForFlows( + f.value.modules as any, + inlineMapping, + SEP, + defaultTs, + assigner, + options, ); + if (f.value.failure_module) { + scripts.push( + ...extractInlineScriptsForFlows( + [f.value.failure_module], + inlineMapping, + SEP, + defaultTs, + assigner, + options, + ), + ); + } + if (f.value.preprocessor_module) { + scripts.push( + ...extractInlineScriptsForFlows( + [f.value.preprocessor_module], + inlineMapping, + SEP, + defaultTs, + assigner, + options, + ), + ); + } + return [f, scripts]; + }; + const inlineMapping = localFlowInlineMapping + ? await localFlowInlineMapping(finalPath) + : {}; + let rendered = render(flow, inlineMapping); + // The assigner keeps the names it hands out unique, not the + // checkout's: one of those equal to another step's + // summary-derived name would leave two files at one path, so + // such a flow renders the export's way. + if ( + new Set(rendered[1].map((s) => s.path)).size !== + rendered[1].length + ) { + rendered = render(flow, {}); } + [flow, inlineScripts] = rendered; } catch (error) { log.error( `Failed to extract inline scripts for flow at path: ${p}`, @@ -2581,7 +2601,7 @@ export function preservePendingScriptLocks( } } -async function compareDynFSElement( +export async function compareDynFSElement( els1: DynFSElement, els2: DynFSElement | undefined, ignore: (path: string, isDirectory: boolean) => boolean, @@ -2594,6 +2614,9 @@ async function compareDynFSElement( branchOverride?: string, isEls1Remote?: boolean, caseInsensitiveFs?: boolean, + // Which schedule files carry an `enabled` that is not the target's to set + // (see push's `parentOwnedScheduleEnabled`): those compare without it. + parentOwnsScheduleEnabled?: (scheduleFilePath: string) => boolean, ): Promise<{ changes: Change[]; localMap: Record }> { let [m1, m2] = els2 ? await Promise.all([ @@ -2785,12 +2808,28 @@ async function compareDynFSElement( ); throw error; } + if ( + parentOwnsScheduleEnabled && + getTypeStrFromPath(k) === "schedule" && + parentOwnsScheduleEnabled(k) + ) { + delete parsedV?.enabled; + delete parsedM2?.enabled; + } if (deepEqual(parsedV, parsedM2)) { continue; } } else if (k.endsWith(".yaml")) { const before = parseYaml(k, m2[k]); const after = parseYaml(k, v); + if ( + parentOwnsScheduleEnabled && + getTypeStrFromPath(k) === "schedule" && + parentOwnsScheduleEnabled(k) + ) { + delete before?.enabled; + delete after?.enabled; + } if (deepEqual(before, after)) { continue; } @@ -4543,6 +4582,86 @@ async function checkServerLockJobs( } } +// The checkout's `!inline` references of one flow (module id -> file), as +// `ZipFSElement`'s `localFlowInlineMapping` names the remote render. Empty +// when the flow has no local flow.yaml. +export async function checkoutInlineNames( + flowYamlPath: string, +): Promise> { + let flow: any; + try { + flow = await yamlParseFile(flowYamlPath); + } catch { + return {}; + } + const mapping = extractCurrentMapping( + flow?.value?.modules, + {}, + flow?.value?.failure_module, + flow?.value?.preprocessor_module, + ); + // A reference that leaves the flow folder would render the remote step + // onto another item's path; such a step keeps its summary-derived name. + for (const [id, ref] of Object.entries(mapping)) { + if (path.isAbsolute(ref) || ref.split(/[\\/]/).includes("..")) { + delete mapping[id]; + } + } + return mapping; +} + +// For a path the parent also has, a fork's export writes the parent's +// `enabled` and the backend refuses to enable the fork's copy: the file's flag +// is the parent's. A parent that cannot be listed (a fork-scoped job token) +// may own every path. Undefined when the target is not a fork. +async function parentOwnedScheduleEnabled( + workspaceId: string, +): Promise<((scheduleFilePath: string) => boolean) | undefined> { + let parentWorkspaceId: string | null | undefined; + let known = false; + try { + const { workspaces } = await wmill.listUserWorkspaces(); + const entry = workspaces?.find((w) => w.id === workspaceId); + known = entry !== undefined; + parentWorkspaceId = entry?.parent_workspace_id; + } catch { + // A fork-scoped token cannot list workspaces. + } + // No parent on record (a fork whose parent was deleted keeps its + // `wm-fork-` id): nothing defers to a parent any more. + if (known && !parentWorkspaceId) { + return undefined; + } + // Without the listing only the `wm-fork-` prefix says fork: a dev + // workspace (custom id) reached with a fork-scoped token counts as none. + if (!isForkWorkspace(workspaceId, parentWorkspaceId)) { + return undefined; + } + let parentPaths: Set | undefined; + if (parentWorkspaceId) { + try { + parentPaths = new Set(); + const perPage = 100; + for (let page = 1; ; page++) { + const batch = await wmill.listSchedules({ + workspace: parentWorkspaceId, + page, + perPage, + }); + batch.forEach((s) => parentPaths!.add(s.path)); + if (batch.length < perPage) break; + } + } catch { + parentPaths = undefined; + } + } + return (scheduleFilePath) => + parentPaths === undefined || + parentPaths.has( + removeType(scheduleFilePath, "schedule").replaceAll(SEP, "/"), + ); +} + export async function push( opts: GlobalOptions & SyncOptions & { @@ -4633,6 +4752,9 @@ export async function push( // Merge CLI flags with resolved settings (CLI flags take precedence only for explicit overrides) opts = mergeCliWithEffectiveOptions(originalCliOpts, effectiveOpts); + const parentOwnsScheduleEnabled = opts.includeSchedules + ? await parentOwnedScheduleEnabled(workspace.workspaceId) + : undefined; if (opts.lint) { log.info("Running lint validation before push..."); @@ -4696,6 +4818,10 @@ export async function push( // ignore } + // See ZipFSElement's `localFlowInlineMapping`. + const localFlowInlineMapping = (flowDir: string) => + checkoutInlineNames(path.join(process.cwd(), flowDir, "flow.yaml")); + const remote = ZipFSElement( (await downloadZip( workspace, @@ -4721,6 +4847,7 @@ export async function push( resourceTypeToIsFileset, false, parseSyncBehavior(opts.syncBehavior) >= 1, + localFlowInlineMapping, ); const local = await FSFSElement( @@ -4741,6 +4868,7 @@ export async function push( wsNameForFiles, false, // els1 (local) is not the remote source await isCaseInsensitiveFilesystem(process.cwd()), + parentOwnsScheduleEnabled, ); // Detect resources/variables that the local config flags as ws_specific @@ -5807,6 +5935,9 @@ export async function push( originalLocalPath: originalWorkspaceSpecificPath, permissionedAsContext, wsSpecific: isWsSpecific ? true : undefined, + enabledOwnedByParent: parentOwnsScheduleEnabled?.( + change.path, + ), keyPushOpts: { noninteractive: (opts.yes ?? false) || !process.stdin.isTTY, @@ -5942,6 +6073,9 @@ export async function push( originalLocalPath: localFilePath, permissionedAsContext, wsSpecific: isAddedWsSpecific ? true : undefined, + enabledOwnedByParent: parentOwnsScheduleEnabled?.( + change.path, + ), keyPushOpts: { noninteractive: (opts.yes ?? false) || !process.stdin.isTTY, diff --git a/cli/src/core/constants.ts b/cli/src/core/constants.ts index 3d92b31a36..7017136a66 100644 --- a/cli/src/core/constants.ts +++ b/cli/src/core/constants.ts @@ -10,4 +10,4 @@ export const WM_FORK_PREFIX = "wm-fork"; // (e.g. utils.ts) can read it without importing main.ts and creating a circular // dependency (main → workspace → utils → main) that triggers a TDZ. // Re-exported from main.ts for backwards compatibility. -export const VERSION = "1.801.0"; +export const VERSION = "1.803.0"; diff --git a/cli/src/core/log.ts b/cli/src/core/log.ts index e78450597f..c4cf256e9d 100644 --- a/cli/src/core/log.ts +++ b/cli/src/core/log.ts @@ -41,6 +41,12 @@ export function warnStderr(msg: unknown) { console.error(`\x1b[33m${String(msg)}\x1b[39m`); } +// A notice that must reach a log even in silent (`--json-output`) mode: +// stderr keeps stdout parseable, and a job that runs the CLI records both. +export function warnAlways(msg: unknown) { + console.error(`\x1b[33m${String(msg)}\x1b[39m`); +} + export function error(msg: unknown) { console.error(`\x1b[31m${String(msg)}\x1b[39m`); } diff --git a/cli/src/guidance/core.ts b/cli/src/guidance/core.ts index 100bea6e53..2a9bb34fe0 100644 --- a/cli/src/guidance/core.ts +++ b/cli/src/guidance/core.ts @@ -115,6 +115,8 @@ Local previews exist for every entity type and don't deploy: - \`wmill flow preview -d ''\` — run a local flow.yaml. - \`wmill app dev\` — live-reload dev server for raw apps. +An argument typed as a resource takes the bare string \`"$res:"\` as its whole value (a variable takes \`"$var:"\`) — never an object wrapper like \`{"$res": ""}\`, and never a plain path. See the \`resources\` skill. + Argument shapes and per-language details live in the \`write-script-\`, \`write-flow\`, and \`raw-app\` skills. ## Keeping metadata in sync diff --git a/cli/src/guidance/skills.gen.ts b/cli/src/guidance/skills.gen.ts index 62027ab2ef..5271167a07 100644 --- a/cli/src/guidance/skills.gen.ts +++ b/cli/src/guidance/skills.gen.ts @@ -5116,6 +5116,8 @@ If the user hasn't already told you to run/test the flow, offer it as a one-sent If the user already asked to test/run/try the flow in their original request, skip the offer and just execute \`wmill flow preview -d ''\` directly — pick plausible args from the flow's input schema. +An input typed as a resource (\`format: resource-\` in the schema) takes the bare string \`"$res:"\` as its whole value — \`-d '{"db": "$res:f/databases/postgres_prod"}'\`, not \`{"db": {"$res": "..."}}\` and not a plain path. Same for a variable, with \`"$var:"\`. See the \`resources\` skill. + \`wmill flow preview\` is safe to run yourself (it does not deploy). \`wmill generate-metadata\` does not deploy either (it only writes local lock/hash files) but re-resolves deps — offer it and run on agreement, unless the project's \`AGENTS.md\` opts into automatic metadata. After running it, check the regenerated \`.lock\` diff and tell the user which inline-script dependency versions changed, so they can catch an unwanted bump before deploying. Only \`wmill sync push\` deploys; run it only when the user explicitly asks. ### Visual preview @@ -6188,6 +6190,41 @@ Reference other resources: } \`\`\` +## Passing a Resource or Variable as a Run Argument + +A script or flow argument typed as a resource (schema \`format: resource-\`) is passed as +the **bare string** \`$res:\` — the whole argument value. Same for a variable, with +\`$var:\`. This applies everywhere job arguments are supplied: \`wmill script run/preview\`, +\`wmill flow run/preview\`, the \`runScriptByPath\` / \`runFlowByPath\` API, a schedule's \`args\`, a +trigger's configured static args. + +\`\`\`json +{ + "db": "$res:f/databases/postgres_prod", + "api_token": "$var:g/all/api_token" +} +\`\`\` + +The reference is resolved when the job runs, under the job's run-as identity — the caller for an +ordinary run, but the configured principal for a schedule, a trigger, or a runnable set to run on +behalf of someone else. The run fails if that identity cannot read the referenced resource or +variable. + +**Never wrap it in an object.** The resolver only rewrites a JSON value that *is* a string +starting with \`$res:\` / \`$var:\`; keys are never inspected. These are all wrong and are passed +through to the script unchanged: + +\`\`\`json +{ "db": { "$res": "f/databases/postgres_prod" } } +{ "db": { "resource": "f/databases/postgres_prod" } } +{ "db": "f/databases/postgres_prod" } +\`\`\` + +The string may sit anywhere a string can — a top-level argument, a nested object field +(\`{ "gh_auth": { "token": "$var:g/all/gh_token" } }\`), or an array element (array elements are +walked only while nested at most two levels deep, and only for arrays of at most 1000 items). +The prefix must be on the string itself. + ## Common Resource Types ### PostgreSQL @@ -7074,11 +7111,11 @@ flow related commands - \`flow push \` - push a local flow spec. This overrides any remote versions. - \`--message \` - Deployment message - \`flow run \` - run a flow by path. - - \`-d --data \` - Inputs specified as a JSON string or a file using @ or stdin using @-. + - \`-d --data \` - Inputs specified as a JSON string or a file using @ or stdin using @-. A resource argument is the bare string $res: as its whole value, and a variable argument is the bare string $var: — not an object wrapper keyed on $res/$var, and not a plain path. - \`-s --silent\` - Do not ouput anything other then the final output. Useful for scripting. - \`--tag \` - Override the worker tag the run is dispatched to (e.g. to route it to dev workers instead of the flow's default tag). - \`flow preview \` - preview a local flow without deploying it. Runs the flow definition from local files and uses local PathScripts by default. Pass --step to run only one module in isolation (resolves nested steps inside branchone/branchall/forloopflow/whileloopflow plus the special preprocessor/failure modules; supported step types: rawscript, script, flow). - - \`-d --data \` - Inputs specified as a JSON string or a file using @ or stdin using @-. + - \`-d --data \` - Inputs specified as a JSON string or a file using @ or stdin using @-. A resource argument is the bare string $res: as its whole value, and a variable argument is the bare string $var: — not an object wrapper keyed on $res/$var, and not a plain path. - \`-s --silent\` - Do not output anything other then the final output. Useful for scripting. - \`--remote\` - Use deployed workspace scripts for PathScript steps instead of local files. - \`--step \` - Run only the named step instead of the whole flow. Honors --data as the step's args and --remote / local-PathScript resolution the same way the full-flow preview does. @@ -7474,11 +7511,11 @@ script related commands - \`--json\` - Output as JSON (for piping to jq) - \`script show \` - show a script's content (alias for get) - \`script run \` - run a script by path - - \`-d --data \` - Inputs specified as a JSON string or a file using @ or stdin using @-. + - \`-d --data \` - Inputs specified as a JSON string or a file using @ or stdin using @-. A resource argument is the bare string $res: as its whole value, and a variable argument is the bare string $var: — not an object wrapper keyed on $res/$var, and not a plain path. - \`-s --silent\` - Do not output anything other then the final output. Useful for scripting. - \`--tag \` - Override the worker tag the run is dispatched to (e.g. to route it to dev workers instead of the script's default tag). - \`script preview \` - preview a local script without deploying it. Supports both regular and codebase scripts. - - \`-d --data \` - Inputs specified as a JSON string or a file using @ or stdin using @-. + - \`-d --data \` - Inputs specified as a JSON string or a file using @ or stdin using @-. A resource argument is the bare string $res: as its whole value, and a variable argument is the bare string $var: — not an object wrapper keyed on $res/$var, and not a plain path. - \`-s --silent\` - Do not output anything other than the final output. Useful for scripting. - \`--tag \` - Override the worker tag the preview is dispatched to (e.g. to route it to dev workers instead of the script's default tag). - \`script new \` - create a new script diff --git a/cli/src/types.ts b/cli/src/types.ts index c9244a8f4c..9c517ff3c7 100644 --- a/cli/src/types.ts +++ b/cli/src/types.ts @@ -189,6 +189,8 @@ export interface PushObjOptions { keyPushOpts?: PushWorkspaceKeyOptions; /** TypeScript runtime a bare `.ts` denotes, for raw-app runnables */ defaultTs?: "bun" | "deno"; + /** schedule push into a fork: the file's `enabled` is the parent's */ + enabledOwnedByParent?: boolean; } /** @@ -217,6 +219,7 @@ export async function pushObj( wsSpecific, keyPushOpts, defaultTs, + enabledOwnedByParent, } = opts; const typeEnding = getTypeStrFromPath(p); @@ -250,7 +253,7 @@ export async function pushObj( } else if (typeEnding === "resource-type") { await pushResourceType(workspace, p, befObj, newObj); } else if (typeEnding === "schedule") { - await pushSchedule(workspace, p, befObj, newObj, permissionedAsContext); + await pushSchedule(workspace, p, befObj, newObj, permissionedAsContext, enabledOwnedByParent); } else if (typeEnding === "http_trigger") { await pushTrigger("http", workspace, p, befObj, newObj, permissionedAsContext); } else if (typeEnding === "websocket_trigger") { diff --git a/cli/test/push_diff_convergence_unit.test.ts b/cli/test/push_diff_convergence_unit.test.ts new file mode 100644 index 0000000000..14e248c03e --- /dev/null +++ b/cli/test/push_diff_convergence_unit.test.ts @@ -0,0 +1,298 @@ +import { afterAll, beforeAll, expect, test } from "bun:test"; +import JSZip from "jszip"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, sep } from "node:path"; +import { + checkoutInlineNames, + compareDynFSElement, + ZipFSElement, +} from "../src/commands/sync/sync.ts"; + +// The differ also reads the working tree (shared lockfiles, dependency files); +// an empty one keeps that out of the picture. +const originalCwd = process.cwd(); +beforeAll(() => { + process.chdir(mkdtempSync(join(tmpdir(), "wmill-push-diff-"))); +}); +afterAll(() => { + process.chdir(originalCwd); +}); + +// A push is only useful when a second run of it finds nothing left to do. +// These pin the two shapes that used to be listed on every run of a push into +// a fork while the push itself either applied nothing or aborted. + +type Mock = { + isDirectory: boolean; + path: string; + getContentText(): Promise; + getChildren(): AsyncIterable; +}; + +// Both sides of the differ use the OS separator; fixtures are written with +// "/" and rows are read back the same way. +const osPath = (p: string) => p.split("/").join(sep); +const slashPath = (p: string) => p.split(sep).join("/"); + +function local(files: Record): Mock { + return { + isDirectory: true, + path: "", + async getContentText() { + return ""; + }, + async *getChildren() { + for (const [path, content] of Object.entries(files)) { + yield { + isDirectory: false, + path: osPath(path), + async getContentText() { + return content; + }, + async *getChildren() {}, + }; + } + }, + }; +} + +const noIgnore = () => false; + +async function diff( + localEl: Mock, + remoteEl: Mock, + skips: Record, + parentOwnsScheduleEnabled?: (scheduleFilePath: string) => boolean, +) { + const { changes } = await compareDynFSElement( + localEl as any, + remoteEl as any, + noIgnore, + false, + skips as any, + true, + [], + false, + undefined, + undefined, + false, + false, + parentOwnsScheduleEnabled, + ); + return changes.map((c) => `${c.name} ${slashPath(c.path)}`); +} + +const SCHEDULE = (enabled: string) => + `summary: nightly\nargs: {}\nenabled: ${enabled}\nis_flow: true\nschedule: 0 0 0 * * *\nscript_path: f/mail/flow\ntimezone: UTC\n`; + +test("push into a fork: a schedule the parent also has compares without `enabled`", async () => { + const remote = local({ "f/mail/nightly.schedule.yaml": SCHEDULE("true") }); + const skips = { includeSchedules: true }; + // The parent has `f/mail/nightly`; any other schedule is the fork's own. + const parentHas = (filePath: string) => + slashPath(filePath) === "f/mail/nightly.schedule.yaml"; + + // Not a fork: `enabled` is compared like any other field. + expect( + await diff( + local({ "f/mail/nightly.schedule.yaml": SCHEDULE("false") }), + remote, + skips, + ), + ).toEqual(["edited f/mail/nightly.schedule.yaml"]); + expect( + await diff( + local({ "f/mail/nightly.schedule.yaml": SCHEDULE("false") }), + remote, + skips, + parentHas, + ), + ).toEqual([]); + // The key being absent is the same case as it differing. + expect( + await diff( + local({ + "f/mail/nightly.schedule.yaml": SCHEDULE("false").replace( + "enabled: false\n", + "", + ), + }), + remote, + skips, + parentHas, + ), + ).toEqual([]); + // Only `enabled` is set aside. + expect( + await diff( + local({ + "f/mail/nightly.schedule.yaml": SCHEDULE("false").replace( + "0 0 0 * * *", + "0 0 1 * * *", + ), + }), + remote, + skips, + parentHas, + ), + ).toEqual(["edited f/mail/nightly.schedule.yaml"]); + // A schedule only the fork has keeps toggling from the file. + expect( + await diff( + local({ "f/mail/fork_only.schedule.yaml": SCHEDULE("true") }), + local({ "f/mail/fork_only.schedule.yaml": SCHEDULE("false") }), + skips, + parentHas, + ), + ).toEqual(["edited f/mail/fork_only.schedule.yaml"]); +}); + +const SUMMARY = "process one mail end-to-end (spam check, classify)"; + +function remoteFlow(content: string) { + const zip = new JSZip(); + zip.file( + "f/mail/flow_v2.flow.json", + JSON.stringify({ + summary: "Flow V2", + description: "", + value: { + modules: [ + { + id: "a", + summary: SUMMARY, + value: { + type: "rawscript", + content, + input_transforms: {}, + language: "python3", + }, + }, + ], + }, + schema: { type: "object", properties: {} }, + }), + // The backend's archive carries no directory entries. + { createFolders: false }, + ); + return zip; +} + +function localFlow(content: string) { + return local({ + "f/mail/flow_v2.flow/flow.yaml": `summary: Flow V2\ndescription: ''\nvalue:\n modules:\n - id: a\n summary: ${SUMMARY}\n value:\n type: rawscript\n content: '!inline process_mail.inline_script.py'\n input_transforms: {}\n language: python3\nschema:\n type: object\n properties: {}\n`, + "f/mail/flow_v2.flow/process_mail.inline_script.py": content, + }); +} + +// The checkout's `!inline` references, as `push` reads them from its flow.yaml. +const checkoutNames = async (flowDir: string) => + slashPath(flowDir) === "f/mail/flow_v2.flow" + ? { a: "process_mail.inline_script.py" } + : {}; + +test("push: an inline script the checkout names differently from the step summary is not a rename", async () => { + const skips = { includeSchedules: false }; + const render = (content: string, withCheckout: boolean) => + ZipFSElement( + remoteFlow(content), + true, + "bun", + {}, + {}, + false, + true, + withCheckout ? checkoutNames : undefined, + ) as any; + + // Same content, file named by hand: three rows before, none after. + expect( + await diff( + localFlow("def main():\n return 1\n"), + render("def main():\n return 1\n", false), + skips, + ), + ).toEqual([ + "deleted f/mail/flow_v2.flow/process_one_mail_end-to-end_(spam_check,_classify).inline_script.py", + "edited f/mail/flow_v2.flow/flow.yaml", + "added f/mail/flow_v2.flow/process_mail.inline_script.py", + ]); + expect( + await diff( + localFlow("def main():\n return 1\n"), + render("def main():\n return 1\n", true), + skips, + ), + ).toEqual([]); + + // A real edit is still one. + expect( + await diff( + localFlow("def main():\n return 2\n"), + render("def main():\n return 1\n", true), + skips, + ), + ).toEqual(["edited f/mail/flow_v2.flow/process_mail.inline_script.py"]); +}); + +test("push: a checkout name that collides with another step's summary-derived name keeps two files", async () => { + const zip = new JSZip(); + zip.file( + "f/mail/flow_v2.flow.json", + JSON.stringify({ + summary: "Flow V2", + description: "", + value: { + modules: [ + { + id: "a", + summary: SUMMARY, + value: { + type: "rawscript", + content: "a", + input_transforms: {}, + language: "python3", + }, + }, + { + id: "b", + summary: "process_mail", + value: { + type: "rawscript", + content: "b", + input_transforms: {}, + language: "python3", + }, + }, + ], + }, + schema: { type: "object", properties: {} }, + }), + { createFolders: false }, + ); + // Nothing local: every rendered file is a "deleted" row, one per path. + const rows = await diff( + local({}), + ZipFSElement(zip, true, "bun", {}, {}, false, true, checkoutNames) as any, + { includeSchedules: false }, + ); + expect(rows.filter((r) => r.endsWith(".py")).sort()).toEqual([ + "deleted f/mail/flow_v2.flow/process_mail.inline_script.py", + "deleted f/mail/flow_v2.flow/process_one_mail_end-to-end_(spam_check,_classify).inline_script.py", + ]); +}); + +test("push: checkout inline names stay inside the flow folder", async () => { + const flowYaml = join(process.cwd(), "flow.yaml"); + writeFileSync( + flowYaml, + `summary: x\nvalue:\n modules:\n - id: a\n value:\n type: rawscript\n content: '!inline a.inline_script.py'\n language: python3\n - id: b\n value:\n type: rawscript\n content: '!inline ../shared/b.py'\n language: python3\n - id: c\n value:\n type: rawscript\n content: '!inline /tmp/c.py'\n language: python3\n`, + ); + expect(await checkoutInlineNames(flowYaml)).toEqual({ + a: "a.inline_script.py", + }); + expect( + await checkoutInlineNames(join(process.cwd(), "missing.yaml")), + ).toEqual({}); +}); diff --git a/docs/feature-telemetry.md b/docs/feature-telemetry.md index 5d7f0b2c29..5cef3c9364 100644 --- a/docs/feature-telemetry.md +++ b/docs/feature-telemetry.md @@ -4,9 +4,9 @@ anonymous usage-stats payload. It answers "does anyone use this, and which variant do they pick" without any identifying data leaving the instance. -It currently carries 28 registered actions across fourteen features (`ai_session`, `ai_chat`, +It currently carries 32 registered actions across fifteen features (`ai_session`, `ai_chat`, `ai_fix`, `ai_agent`, `ai_agent_eval`, `flow_editor`, `flow_run`, `flow_step`, `run_form`, -`debugger`, `trigger`, `command_script`, `hub_script`, `usage_meter`). Nearly all of the +`debugger`, `trigger`, `command_script`, `hub_script`, `usage_meter`, `sso_groups_claim`). Nearly all of the product is uninstrumented, so new user-facing work is the opportunity to change that. ## When to instrument diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 872ee5554c..be1dc6c3b5 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1,12 +1,12 @@ { "name": "@windmill-labs/components", - "version": "1.801.0", + "version": "1.803.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@windmill-labs/components", - "version": "1.801.0", + "version": "1.803.0", "hasInstallScript": true, "license": "AGPL-3.0", "dependencies": { diff --git a/frontend/package.json b/frontend/package.json index e9870e2bc9..62c42de5da 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "@windmill-labs/components", - "version": "1.801.0", + "version": "1.803.0", "scripts": { "dev": "vite dev", "dev:ui-builder": "mv static/ui_builder static/ui_builder.dev-disabled 2>/dev/null || true ; trap 'mv static/ui_builder.dev-disabled static/ui_builder 2>/dev/null || true' EXIT ; vite dev", diff --git a/frontend/src/lib/aiStore.test.ts b/frontend/src/lib/aiStore.test.ts index fed6147b62..be8980e93a 100644 --- a/frontend/src/lib/aiStore.test.ts +++ b/frontend/src/lib/aiStore.test.ts @@ -35,6 +35,24 @@ describe('setCopilotInfo legacy /thinking migration', () => { expect(info.aiModels.map((m) => m.model)).toEqual(['claude-sonnet-4-6']) }) + it('keeps the models but turns the assistant off when the workspace disabled it', () => { + setCopilotInfo({ + providers: { + anthropic: { + resource_path: 'u/admin/anthropic', + models: ['claude-sonnet-4-6'] + } + }, + copilot_disabled: true + }) + + const info = get(copilotInfo) + expect(info.enabled).toBe(false) + expect(info.workspaceDisabled).toBe(true) + // The providers still describe what AI agent steps can run on. + expect(info.aiModels.map((m) => m.model)).toEqual(['claude-sonnet-4-6']) + }) + it('defaults provider web search on unless explicitly disabled', () => { setCopilotInfo({ providers: { diff --git a/frontend/src/lib/aiStore.ts b/frontend/src/lib/aiStore.ts index 2d6a98c625..6ac98d69e8 100644 --- a/frontend/src/lib/aiStore.ts +++ b/frontend/src/lib/aiStore.ts @@ -41,6 +41,10 @@ export const copilotSessionModel = writable( export const copilotInfo = writable<{ enabled: boolean + // The workspace hid the assistant (`ai_config.copilot_disabled`). `enabled` is then false + // whatever the providers say, and the AI entry points that nudge "configure AI" when + // `enabled` is off render nothing at all instead. + workspaceDisabled: boolean codeCompletionModel?: AIProviderModel defaultModel?: AIProviderModel metadataModel?: AIProviderModel @@ -56,6 +60,7 @@ export const copilotInfo = writable<{ freeTier?: FreeTierInfo }>({ enabled: false, + workspaceDisabled: false, codeCompletionModel: undefined, defaultModel: undefined, metadataModel: undefined, @@ -71,7 +76,7 @@ export const copilotInfo = writable<{ aiUserDisabled.subscribe((disabled) => { copilotInfo.update((info) => ({ ...info, - enabled: info.aiModels.length > 0 && !disabled + enabled: info.aiModels.length > 0 && !disabled && !info.workspaceDisabled })) }) @@ -126,9 +131,11 @@ export function setCopilotInfo(aiConfig: AIConfig) { return model }) + const workspaceDisabled = aiConfig.copilot_disabled === true copilotInfo.set({ - // Providers are configured; the per-user opt-out is the only thing that can gate it off. - enabled: !get(aiUserDisabled), + // Providers are configured; only the workspace or per-user opt-outs can gate it off. + enabled: !workspaceDisabled && !get(aiUserDisabled), + workspaceDisabled, // Strip the deprecated /thinking suffix from the configured model slots too, // otherwise a workspace whose default still carries it sends an invalid model id. codeCompletionModel: stripModelSuffix(aiConfig.code_completion_model), @@ -146,6 +153,7 @@ export function setCopilotInfo(aiConfig: AIConfig) { copilotInfo.set({ enabled: false, + workspaceDisabled: aiConfig.copilot_disabled === true, codeCompletionModel: undefined, defaultModel: undefined, metadataModel: undefined, diff --git a/frontend/src/lib/components/AppConnectInner.svelte b/frontend/src/lib/components/AppConnectInner.svelte index a9a0837d1f..3d73cea72f 100644 --- a/frontend/src/lib/components/AppConnectInner.svelte +++ b/frontend/src/lib/components/AppConnectInner.svelte @@ -21,9 +21,10 @@ } from '$lib/gen' import { emptyString, truncateRev, urlize } from '$lib/utils' import { registryEntryFor, registryCcCapableFor, stripSandboxSuffix } from './oauthRegistry' - import { createEventDispatcher, onDestroy, tick, untrack } from 'svelte' + import { createEventDispatcher, onDestroy, tick } from 'svelte' import Path from './Path.svelte' - import { Button, RadioCard, Skeleton } from './common' + import { ListRow, RadioCard, Skeleton } from './common' + import { useListHighlight } from './common/listRow/listHighlight.svelte' import ApiConnectForm from './ApiConnectForm.svelte' import SearchItems from './SearchItems.svelte' import WhitelistIp from './WhitelistIp.svelte' @@ -42,7 +43,6 @@ import SyncResourceTypes from './SyncResourceTypes.svelte' import Label from './Label.svelte' import ResourcePathHint from './ResourcePathHint.svelte' - import { twMerge } from 'tailwind-merge' interface Props { step?: number @@ -1027,15 +1027,8 @@ // Both lists start undefined and render skeletons; "nothing found" only means something // once they have landed. let listsLoaded = $derived(rankedConnectsManual !== undefined && rankedConnects !== undefined) - let highlightedIndex = $state(-1) const rowDomId = (index: number) => `resource-type-row-${index}` - // Set at hover time rather than up front, so only the descriptions the row actually cut - // off carry a tooltip. - function titleIfTruncated(e: MouseEvent & { currentTarget: HTMLElement }) { - const el = e.currentTarget - el.title = el.scrollWidth > el.clientWidth ? (el.textContent?.trim() ?? '') : '' - } const oauthRowOffset = $derived(customKeys.length) const otherRowOffset = $derived(customKeys.length + (rankedConnects?.length ?? 0)) @@ -1054,53 +1047,23 @@ return best } - // Filtering reshuffles the rows under the highlight: point it at the best match so Enter - // takes the top hit, and drop it entirely once the filter is cleared. - $effect(() => { - navItems - filter - untrack(() => (highlightedIndex = searching ? bestMatchIndex() : -1)) + const highlight = useListHighlight({ + count: () => navItems.length, + rowId: rowDomId, + // Sections are rendered in a fixed order, so the best match is not necessarily the + // first row; Enter should still take the top hit. + restingIndex: () => (searching ? bestMatchIndex() : -1), + onActivate: (index) => { + const item = navItems[index] + if (!item) return + item.oauth ? connectOauth(item.key) : selectFromOthers(item.key) + }, + activateEnterFrom: [SEARCH_INPUT_ID] }) - // Scrolling rows under a resting pointer makes the browser fire `mouseenter` on each one, - // which would drag the highlight back under the cursor as the arrow keys move it. Only a - // real pointer move hands the highlight back to the mouse. - let pointerOwnsHighlight = $state(true) - - function highlightHovered(index: number) { - if (pointerOwnsHighlight) highlightedIndex = index - } - - function moveHighlight(delta: number) { - const count = navItems.length - if (count === 0) return - pointerOwnsHighlight = false - // Rows are tabbable buttons, so focus can sit on one. Enter then activates whatever is - // focused, which has to stay the highlighted row. - const rowWasFocused = document.activeElement?.id?.startsWith('resource-type-row-') ?? false - highlightedIndex = - highlightedIndex < 0 - ? delta > 0 - ? 0 - : count - 1 - : (highlightedIndex + delta + count) % count - const row = document.getElementById(rowDomId(highlightedIndex)) - row?.scrollIntoView({ block: 'nearest' }) - if (rowWasFocused) row?.focus() - } - function onListKeydown(e: KeyboardEvent) { if (step !== 1) return - if (e.key === 'ArrowDown' || e.key === 'ArrowUp') { - e.preventDefault() - moveHighlight(e.key === 'ArrowDown' ? 1 : -1) - } else if (e.key === 'Enter' && (e.target as HTMLElement)?.id === SEARCH_INPUT_ID) { - // A focused row activates itself on Enter; this covers Enter typed in the search field. - const item = navItems[highlightedIndex] - if (!item) return - e.preventDefault() - item.oauth ? connectOauth(item.key) : selectFromOthers(item.key) - } + highlight.onKeydown(e) } let editScopes = $state(false) @@ -1132,7 +1095,7 @@ - - {/snippet} - + {/snippet} + +{/if} diff --git a/frontend/src/lib/components/copilot/ResourceGen.svelte b/frontend/src/lib/components/copilot/ResourceGen.svelte index 34534285b4..2fc3cfa646 100644 --- a/frontend/src/lib/components/copilot/ResourceGen.svelte +++ b/frontend/src/lib/components/copilot/ResourceGen.svelte @@ -119,69 +119,71 @@ }) - - {#snippet trigger()} - - - {:else} -
-

Enable Windmill AI in the workspace settings

-
- {/if} - - {/snippet} - + }} + disabled={instructions.length == 0} + startIcon={{ icon: Wand2 }} + > + Generate + + + {:else} +
+

Enable Windmill AI in the workspace settings

+
+ {/if} + + {/snippet} + +{/if} diff --git a/frontend/src/lib/components/copilot/ScriptFix.svelte b/frontend/src/lib/components/copilot/ScriptFix.svelte index d6ccb26ab8..f586ad1aaf 100644 --- a/frontend/src/lib/components/copilot/ScriptFix.svelte +++ b/frontend/src/lib/components/copilot/ScriptFix.svelte @@ -77,7 +77,7 @@ const sessionScopedManager = getContext('aiChatManager') -{#if SUPPORTED_LANGUAGES.has(lang)} +{#if SUPPORTED_LANGUAGES.has(lang) && !$copilotInfo.workspaceDisabled} {#if sessionScopedManager} - {:else} + {:else if !$copilotInfo.workspaceDisabled} togglePanel() })} -{:else} +{:else if !$copilotInfo.workspaceDisabled} {#snippet trigger()} {@render button({ onPress: () => togglePanel() })} diff --git a/frontend/src/lib/components/copilot/chat/AIChat.svelte b/frontend/src/lib/components/copilot/chat/AIChat.svelte index 414be0a741..fd95613f2e 100644 --- a/frontend/src/lib/components/copilot/chat/AIChat.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChat.svelte @@ -69,11 +69,13 @@ : freeTierExhausted ? '' : !hasCopilot - ? $aiUserDisabled - ? 'Windmill AI is disabled in your account settings' - : isAdmin - ? `Enable Windmill AI in your [workspace settings](${base}/workspace_settings?tab=ai) to use this chat` - : 'Ask an admin to enable Windmill AI in this workspace to use this chat' + ? $copilotInfo.workspaceDisabled + ? 'Windmill AI is hidden in this workspace' + : $aiUserDisabled + ? 'Windmill AI is disabled in your account settings' + : isAdmin + ? `Enable Windmill AI in your [workspace settings](${base}/workspace_settings?tab=ai) to use this chat` + : 'Ask an admin to enable Windmill AI in this workspace to use this chat' : aiChatManager.mode === AIMode.SCRIPT && aiChatManager.scriptEditorOptions?.lang && !SUPPORTED_CHAT_SCRIPT_LANGUAGES.includes(aiChatManager.scriptEditorOptions.lang) diff --git a/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte b/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte index f610dd5ff4..699098c116 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatDisplay.svelte @@ -35,8 +35,9 @@ import ChatQuickActions from './ChatQuickActions.svelte' import ContextUsageIndicator from './ContextUsageIndicator.svelte' import AIChatModelSettings from './AIChatModelSettings.svelte' - import McpConnections from './McpConnections.svelte' - import SkillsPicker from './SkillsPicker.svelte' + import AssistantSettingsModal from './AssistantSettingsModal.svelte' + import { SkillsMenu } from './skills/skillsMenu.svelte' + import { McpMenu } from '$lib/components/mcp/mcpMenu.svelte' import ChatMode from './ChatMode.svelte' import DatatableCreationPolicy from './DatatableCreationPolicy.svelte' import Tooltip from '$lib/components/meltComponents/Tooltip.svelte' @@ -207,8 +208,11 @@ } = $props() let aiChatInput: AIChatInput | undefined = $state() - let mcpConnections: McpConnections | undefined = $state() - let skillsPicker: SkillsPicker | undefined = $state() + let assistantSettings: AssistantSettingsModal | undefined = $state() + // The "+" menu's skill and MCP rows: enough state to check and flip one, with + // everything else about them behind the assistant settings modal. + const skillsMenu = new SkillsMenu(aiChatManager, () => assistantSettings?.open('skills')) + const mcpMenu = new McpMenu(aiChatManager, () => assistantSettings?.open('mcp')) let plusMenuOpen = $state(false) let editingMessageIndex = $state(null) @@ -959,8 +963,8 @@ the panel, or the Escape-to-stop focus check would wrongly reject them. --> const closeMenu = () => (plusMenuOpen = false) const inGlobal = aiChatManager.mode === AIMode.GLOBAL const [skillItems, mcpItems] = await Promise.all([ - inGlobal ? skillsPicker?.menuItems(closeMenu) : undefined, - inGlobal ? mcpConnections?.menuItems(closeMenu) : undefined + inGlobal ? skillsMenu.items(closeMenu) : undefined, + inGlobal ? mcpMenu.items(closeMenu) : undefined ]) return [ { @@ -1143,10 +1147,12 @@ the panel, or the Escape-to-stop focus check would wrongly reject them. --> {/if} - + + {#if aiChatManager.mode === AIMode.GLOBAL} - - + {/if} {#if aiChatManager.mode === AIMode.APP && appContext && (appContext.inspectorElement || appContext.codeSelection)} diff --git a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts index 61ed493a66..43df13f78c 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/AIChatManager.svelte.ts @@ -2123,7 +2123,10 @@ export class AIChatManager { // pipeline surface when a /pipeline editor has registered helpers. Centralized // so changeMode, refreshGlobalSkills, and setPipelineHelpers stay consistent — // each rebuild would otherwise drop the pipeline augmentation the others added. - private configureGlobalMode = () => { + // + // Public because it is purely local, unlike `changeMode(GLOBAL)`, which also + // fires the three network refreshes. + configureGlobalMode = () => { const systemMessage = prepareGlobalSystemMessage(getCustomPromptParts(AIMode.GLOBAL), { previewTools: this.isSessionChat, user: this.globalIdentity, @@ -2350,6 +2353,10 @@ export class AIChatManager { } openChat = () => { + // Nothing may open the docked pane in a workspace that hid the assistant. + if (get(copilotInfo).workspaceDisabled) { + return + } chatState.size = this.savedSize > 0 ? this.savedSize : DEFAULT_SIZE localStorage.setItem('ai-chat-open', 'true') } @@ -2361,6 +2368,9 @@ export class AIChatManager { } toggleOpen = () => { + if (chatState.size === 0 && get(copilotInfo).workspaceDisabled) { + return + } if (chatState.size > 0) { this.savedSize = chatState.size } @@ -2880,6 +2890,12 @@ export class AIChatManager { sendUserToast('This action needs the AI chat. Start an AI session to continue.', true) return } + // The workspace hid the assistant: every entry point is gone from the UI, so a turn + // reaching here comes from a path that missed the gate and would stream unseen. + if (!this.isSessionChat && get(copilotInfo).workspaceDisabled) { + sendUserToast('Windmill AI is hidden in this workspace.', true) + return + } // Refused before anything mutates, so there is nothing to unwind: the // draft (already taken by the composer) goes back where the user can see // it, and the turn never starts. Only the message's own send restores it diff --git a/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte b/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte index 123768d3f3..9dd94fa4ea 100644 --- a/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte +++ b/frontend/src/lib/components/copilot/chat/AIChatModelSettings.svelte @@ -33,6 +33,13 @@ type ReasoningProviderModel } from '../reasoningRegistry' + let { + /** Whether this dropdown carries the custom-prompt entries. Off where the surface + * has an assistant settings modal — its Instructions section owns them there, and + * two ways in would drift. The home composer has no such modal, so it keeps them. */ + promptSettings = true + }: { promptSettings?: boolean } = $props() + const aiChatManager = getAiChatManager() const AI_SETTINGS_HREF = `${base}/workspace_settings?tab=ai` @@ -335,7 +342,9 @@ class="bg-surface-tertiary dark:border w-64 origin-top-right rounded-lg shadow-lg focus:outline-none py-1 text-xs" > - + {#if promptSettings} + + {/if}
Model
@@ -409,21 +418,24 @@ {/snippet} - + +{#if promptSettings} + +{/if}
(pointerOwnsHighlight = true)} + onpointermove={highlight.pointerMoved} >
@@ -1146,28 +1109,6 @@
- {#snippet resourceRow(key: string)} -
-
- -
-
-
- {resourceTypeDisplayName(key)} - {key} -
- {#if resourceTypeDescriptions[key]} - - {plainDescription(resourceTypeDescriptions[key])} - - {/if} -
-
- {/snippet} - {#snippet sectionHeading(title: string, count: number)}

{title}{#if searching}{count}{/if} @@ -1175,26 +1116,29 @@ {/snippet} {#snippet resourceButton(key: string, index: number, oauth: boolean)} - + {icon} + {title} + subtitle={resourceTypeDescriptions[key] ? subtitle : undefined} + highlighted={index === highlight.index} + onMouseEnter={() => highlight.hovered(index)} + onClick={() => (oauth ? connectOauth(key) : selectFromOthers(key))} + /> {/snippet}
@@ -1213,7 +1157,7 @@ {#if customKeys.length > 0}
{@render sectionHeading('Custom resource types', customKeys.length)} -
+
{#each customKeys as key, i} {@render resourceButton(key, i, false)} {/each} @@ -1227,7 +1171,7 @@ 'Instance-configured OAuth APIs', rankedConnects?.length ?? 0 )} -
+
{#if rankedConnects} {#each rankedConnects as { key }, i} {@render resourceButton(key, oauthRowOffset + i, true)} @@ -1259,7 +1203,7 @@
{/if} -
+
{#if rankedConnectsManual} {#each otherKeys as key, i} {@render resourceButton(key, otherRowOffset + i, false)} diff --git a/frontend/src/lib/components/ImportSetupStep.svelte b/frontend/src/lib/components/ImportSetupStep.svelte index b8c309b1b6..e052ecf9bc 100644 --- a/frontend/src/lib/components/ImportSetupStep.svelte +++ b/frontend/src/lib/components/ImportSetupStep.svelte @@ -14,16 +14,21 @@ import IconedResourceType from '$lib/components/IconedResourceType.svelte' import ImportSetupRow from '$lib/components/ImportSetupRow.svelte' import AppConnectDrawer from '$lib/components/AppConnectDrawer.svelte' + import Modal2 from '$lib/components/common/modal/Modal2.svelte' + import Select from '$lib/components/select/Select.svelte' + import { applyRetarget, seesWholeWorkspace } from '$lib/importWizard/retargetDeployed' import { OauthService } from '$lib/gen' import { registryCcCapableFor } from '$lib/components/oauthRegistry' import { resourceTypeDisplayName } from '$lib/components/resourceTypeDisplay' import { applyOneMigration } from '$lib/components/workspaceSettings/projectInstall' import { probeMigrationsApplied } from '$lib/importWizard/probe' import { + projectReferencesResource, retargetProjectExport, type ProjectExport, type ProjectMigration } from '$lib/components/workspaceSettings/projectBundle' + import { superadmin, userStore } from '$lib/stores' import { sendUserToast } from '$lib/toast' import { escapeHtml } from '$lib/utils' @@ -96,6 +101,17 @@ * is absent, and removing the row reports "all set" over a credential nobody filled. */ unreadable?: boolean + /** + * The workspace resource this row was pointed at. The project's items reference it + * directly now, so this is what the row has to say instead of the path it used to name. + */ + reusedFrom?: string + /** + * The empty placeholder is still at this row's path, because the retarget could not + * account for every item that might read it. Worth saying: the workspace has a resource + * on it that looks unfinished and is not. + */ + stubKept?: boolean } let loading = $state(true) @@ -103,9 +119,38 @@ let rows = $state([]) let blanks = $state([]) let projectResources: { path: string; resource_type: string }[] = [] + /** + * The subset of `projectResources` the checklist asks about: the ones something in the + * project actually points at. The rest are created and left alone — see + * `projectReferencesResource`. Kept apart from `projectResources` because the full list + * is still what a stub may not be replaced by. + */ + let askableResources: { path: string; resource_type: string }[] = [] let working = $state(false) let resourceEditor: ResourceEditorDrawer | undefined = $state(undefined) + /** The folder the import wrote into, which is where every rewritable referrer lives. */ + const targetFolder = $derived(folder?.trim() || slug) + + /** + * Resources the workspace already has, by resource type — what a stub can be replaced + * by. Empty for a workspace this import created, which is why the choice is offered + * rather than imposed: with nothing to choose from the button goes straight to the + * editor, exactly as it did before. + */ + let candidates = $state>({}) + /** + * How many candidates are worth reading back to find the unfilled ones. Past this a + * workspace holds too many resources of these types to be the case worth filtering — + * one project's stub offered as another's credential — and they are all offered rather + * than costing a request each. + */ + const CANDIDATE_READ_CAP = 40 + /** The credential row whose choice dialog is open. */ + let choosing = $state(undefined) + let chosenPath = $state(undefined) + let reusing = $state(false) + const pendingTables = $derived(rows.filter((r) => r.status !== 'done')) // Split because the two say different things to the user: one data table was never // created, the other exists and could not be read. Telling someone to set up what they @@ -262,7 +307,7 @@ // Retargeted the same way the import was, so these are where the stubs actually // landed. `retargetProjectExport` is a no-op when the folder is the slug, which is // every new-workspace import. - const target = folder?.trim() || slug + const target = targetFolder const retargeted = retargetProjectExport(exportData, exportData.project?.slug ?? slug, target) // Contained for the same reason the import contains: a crafted export can name a // path outside the folder, and offering that for editing would reach a resource @@ -270,6 +315,20 @@ projectResources = (retargeted.resources ?? []) .map((r) => ({ path: String(r.path), resource_type: String((r as any).resource_type) })) .filter((r) => r.path.startsWith(`f/${target}/`)) + // Asked against the export as published, not the retargeted copy: a path the project + // spells out in code is not rewritten by the retarget, so only the raw export has + // its references and its resource paths agreeing. `resourceCount` asks the same + // question the same way, and the step and the stepper have to give one answer. + // Paired by position, not by reconstructing the retargeted path: `retargetProjectExport` + // maps `resources` in order, and an external path the bundle pulled in lands at + // `f//` with a `_2` suffix on collision, which no slicing recovers. + const askable = new Set( + (retargeted.resources ?? []) + .map((r, i) => [String(r.path), (exportData.resources ?? [])[i]] as const) + .filter(([, raw]) => raw && projectReferencesResource(exportData, String(raw.path))) + .map(([path]) => path) + ) + askableResources = projectResources.filter((r) => askable.has(r.path)) await refreshBlanks() } catch (e: any) { loadError = e?.body ?? e?.message ?? String(e) @@ -361,13 +420,19 @@ * it only moves a row from outstanding to done. */ async function refreshBlanks(): Promise { - const fresh = await findBlankResources(projectResources) + const fresh = await findBlankResources(askableResources) const stillBlank = new Map(fresh.map((b) => [b.path, b])) if (blanks.length === 0) { blanks = fresh + await loadCandidates() return } blanks = blanks.map((b) => { + // A row pointed at another resource is settled once its own stub is gone: the + // project's items read the chosen resource and nothing is left at this path. A row + // whose stub was kept is not settled, and re-reading it is how filling that stub in + // finally closes the row. + if (b.reusedFrom && !b.stubKept) return b const f = stillBlank.get(b.path) // Every field the fresh read decides is taken from it, not merged selectively: these // describe what is at the path *now*. Keeping a stale `unreadable` leaves a resource @@ -383,12 +448,15 @@ justSaved: false } } - // Gone from the blank list entirely: it was read, and it is filled. + // Gone from the blank list entirely: it was read, and it is filled. `stubKept` goes + // with it — the placeholder the items this run could not move read is a credential + // now, so there is nothing left to tell anyone to fill in. return { ...b, missing: [], unreadable: undefined, occupiedBy: undefined, + stubKept: undefined, done: true, justSaved: !b.done } @@ -401,6 +469,169 @@ if (row) row.justSaved = false }, 1500) } + await loadCandidates() + } + + /** + * Which existing resources each outstanding row could be replaced by. Re-read on every + * refresh rather than once: a resource created from the editor here is a candidate for + * the rows below it. + * + * The project's own resources are never offered — one of this project's stubs standing + * in for another is a reference to something equally unfilled. + */ + async function loadCandidates(): Promise { + const types = [...new Set(blanks.map((b) => b.resourceType))] + if (types.length === 0) { + candidates = {} + return + } + const own = new Set(projectResources.map((r) => r.path)) + const next: Record = Object.fromEntries(types.map((t) => [t, []])) + try { + // One call for every type at once — `resource_type` takes a comma-separated list — + // and every page of it: `perPage` is what bounds the answer, so without the loop a + // workspace past one page would have the rest of its resources silently hidden. + for (let page = 1; page <= 100; page++) { + const rows = await ResourceService.listResource({ + workspace, + resourceType: types.join(','), + page, + perPage: 100 + }) + for (const r of rows) { + if (own.has(r.path)) continue + next[r.resource_type ?? '']?.push(r.path) + } + if (rows.length < 100) break + } + } catch { + // Offer nothing rather than a partial list: every row then behaves as it did before + // this choice existed, which is a working way to fill a credential. + candidates = {} + return + } + // An unfilled resource is never the answer to "which credential should this use" — + // another project's stub above all, which the path filter above cannot recognise. + const paths = Object.values(next).flat() + if (paths.length <= CANDIDATE_READ_CAP) { + const settled = await Promise.all(paths.map(async (p) => [p, await isUnfilled(p)] as const)) + const unfilled = new Set(settled.filter(([, empty]) => empty).map(([p]) => p)) + for (const t of Object.keys(next)) next[t] = next[t].filter((p) => !unfilled.has(p)) + } + candidates = next + } + + /** + * Whether a resource holds nothing. Same test the checklist uses to call one of the + * project's own resources blank, so a resource this drops is exactly one the wizard + * would have asked someone to fill in. + */ + async function isUnfilled(path: string): Promise { + try { + const found = await ResourceService.getResource({ workspace, path }) + const value = found?.value + if (!value || typeof value !== 'object') return true + return !Object.values(value).some((v) => v !== undefined && v !== null && v !== '') + } catch { + // A read that fails says nothing about the value, and offering it is what this did + // before the check existed. + return false + } + } + + /** + * The row's one action. A workspace that already has a resource of this type gets the + * choice first — reusing what is there is usually the answer, and entering the same + * credentials a second time is the thing worth avoiding. With nothing to choose from + * there is no choice to make, so it goes straight where it always went. + */ + function startFilling(b: Blank): void { + // A kept-stub row has already been pointed at a resource; what is left is the empty + // placeholder the items this run could not move still read. Reusing a second resource + // would move nothing — every rewritable referrer is off the stub — and would relabel + // the row after a retarget that did nothing. + if (b.done || b.stubKept || (candidates[b.resourceType] ?? []).length === 0) { + fillDirectly(b) + return + } + chosenPath = undefined + choosing = b + } + + /** Connect where the instance can, hand-fill otherwise. */ + function fillDirectly(b: Blank): void { + if (canConnectType(b.resourceType)) appConnect?.open(b.resourceType, b.path) + else resourceEditor?.initEdit(b.path) + } + + /** + * The chooser's way out: close it and do what the button did before there was a choice. + * The row is read out of the state first — closing the dialog unmounts the block that + * would otherwise be holding it. + */ + function fillNewInstead(): void { + const b = choosing + choosing = undefined + if (b) fillDirectly(b) + } + + /** + * Point the project at an existing resource: every imported item that referenced the stub + * is rewritten to the chosen path. Nothing is copied. The stub is deleted only when + * `applyRetarget` can account for every item that might read it, and kept otherwise — so + * the toast says how many items moved, and whether the placeholder is still there. + */ + async function reuseChosen(): Promise { + const b = choosing + const target = chosenPath + if (!b || !target) return + reusing = true + working = true + try { + const outcome = await applyRetarget({ + workspace, + folder: targetFolder, + from: b.path, + to: target, + // Asked of this workspace, not of whichever one the user record still describes: + // reloading on this step leaves `$userStore` pointing at the previous workspace. + seesWholeWorkspace: seesWholeWorkspace($userStore, !!$superadmin, workspace) + }) + const moved = `${outcome.rewritten.length} item${outcome.rewritten.length === 1 ? '' : 's'}` + if (outcome.error) { + sendUserToast( + `Could not point the project at ${target}: ${outcome.error}. ${moved} had already been updated, and ${b.path} was kept.`, + true + ) + return + } + choosing = undefined + const row = blanks.find((x) => x.path === b.path) + if (row) { + row.reusedFrom = target + row.stubKept = !outcome.stubDeleted + // Settled only when the stub is gone. A kept stub is empty and is still what + // every item the scan could not move reads, so the row stays outstanding and + // keeps its action: filling it in is the thing left to do. + row.done = outcome.stubDeleted + row.justSaved = outcome.stubDeleted + } + await refreshBlanks() + sendUserToast( + outcome.stubDeleted + ? `The project now uses ${target} — ${moved} updated.` + : `The project now uses ${target} — ${moved} updated. ${b.path} was kept, because some items could not be checked.` + ) + } catch (e: any) { + sendUserToast( + `Could not point the project at ${target}: ${e?.body ?? e?.message ?? String(e)}`, + true + ) + } finally { + reusing = false + working = false + } } $effect(() => { @@ -708,7 +939,18 @@
{/snippet} {#snippet detail()} - {#if b.occupiedBy} + {#if b.reusedFrom} + + now uses {b.reusedFrom} + + {#if b.stubKept} + + + some items still read {b.path} — fill it in too + + {/if} + {:else if b.occupiedBy} a {resourceTypeDisplayName(b.occupiedBy)} resource already holds this path — the project did not get this one @@ -735,15 +977,16 @@ {b.occupiedBy ? 'Resolve in the workspace' : 'Check the workspace'} + {:else if b.reusedFrom && !b.stubKept} + + Reused {:else} @@ -859,3 +1102,55 @@ void refreshBlanks()} /> + + + choosing !== undefined, + (v) => { + if (!v && !reusing) choosing = undefined + } + } +> + {#if choosing} + {@const forRow = choosing} + {@const existing = candidates[forRow.resourceType] ?? []} +
+

+ This workspace already has {existing.length} + {resourceTypeDisplayName(forRow.resourceType)} + {existing.length === 1 ? 'resource' : 'resources'}. Use one and this project's apps, flows + and triggers are pointed at it. +

+ - -
- {/if} -
+{#if !$copilotInfo.workspaceDisabled} +
+ { + if (prompt !== undefined) { + prompt = undefined + } else { + prompt = '' + } + }} + options={{ right: `Enable filling ${type} inputs with AI` }} + /> + {#if prompt !== undefined} +
+ +
+ {/if} +
+{/if} diff --git a/frontend/src/lib/components/copilot/CronGen.svelte b/frontend/src/lib/components/copilot/CronGen.svelte index 6a8dc6892e..94715a0f18 100644 --- a/frontend/src/lib/components/copilot/CronGen.svelte +++ b/frontend/src/lib/components/copilot/CronGen.svelte @@ -79,66 +79,68 @@ }) - - {#snippet trigger()} -
- {:else} -
-

Enable Windmill AI in the workspace settings

-
- {/if} -
- {/snippet} - + }} + disabled={instructions.length == 0} + startIcon={{ icon: Wand2 }} + /> +
+ {:else} +
+

Enable Windmill AI in the workspace settings

+
+ {/if} +
+ {/snippet} + +{/if} diff --git a/frontend/src/lib/components/copilot/RegexGen.svelte b/frontend/src/lib/components/copilot/RegexGen.svelte index e61ab5cbf2..f399726988 100644 --- a/frontend/src/lib/components/copilot/RegexGen.svelte +++ b/frontend/src/lib/components/copilot/RegexGen.svelte @@ -1,5 +1,5 @@ - - {#snippet trigger()} - +{#if !$copilotInfo.workspaceDisabled} + + {#snippet trigger()}