diff --git a/.agents/skills/pr/SKILL.md b/.agents/skills/pr/SKILL.md index 6a72949c96..f2a9ec8dad 100644 --- a/.agents/skills/pr/SKILL.md +++ b/.agents/skills/pr/SKILL.md @@ -210,7 +210,7 @@ the diff first — `git diff --name-only main...HEAD` answers most of these. **Flip without asking** when it is self-contained: a single-file fix, test-only, docs-only, one call site, no new public surface. -Unattended (webmux oneshot) there is nobody to ask, so the judgement holds and the action +Unattended (a one-shot run) there is nobody to ask, so the judgement holds and the action degrades: flip the self-contained ones, and leave the rest at a clean draft with a line in the PR description saying why — `left in draft: adds a migration, wants a human look before ready`. Don't flip a wide-blast-radius change just because the round came back clean, and don't ask a diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 80970d1e76..0d174a548f 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,4 +1,13 @@ -# Basic set up for three package managers +# Weekly, grouped version updates for every shipped manifest, plus one grouped +# security-update PR per manifest. Security-update PRs have no open-PR limit, so +# without an `applies-to: security-updates` group each alert would get its own PR. +# Bun has no security updates: /cli alerts are fixed through its version group. +# +# Deliberately NOT covered here: +# - /rust-client: Cargo.toml is generated and gitignored +# - /backend/parsers/windmill-parser-wasm: excluded from the workspace, needs nightly cargo +# - /benchmarks/pulumi: one-off load-test infrastructure, not shipped +# - /integration_tests: CI-only version: 2 updates: @@ -13,12 +22,32 @@ updates: directory: "/frontend" schedule: interval: "weekly" + groups: + # minor + patch bumps in one PR; majors in their own PR so they can be + # reviewed separately without blocking the routine bumps + frontend-deps: + patterns: ["*"] + update-types: ["minor", "patch"] + frontend-majors: + patterns: ["*"] + update-types: ["major"] + frontend-security: + applies-to: security-updates + patterns: ["*"] # Maintain dependencies for cargo - package-ecosystem: "cargo" directory: "/backend" schedule: interval: "weekly" + groups: + # majors stay as individual PRs: they usually need code changes + cargo-deps: + patterns: ["*"] + update-types: ["minor", "patch"] + cargo-security: + applies-to: security-updates + patterns: ["*"] # Maintain dependencies for Docker - package-ecosystem: "docker" @@ -31,3 +60,52 @@ updates: directory: "/python-client/wmill" schedule: interval: "weekly" + groups: + python-client-security: + applies-to: security-updates + patterns: ["*"] + + # Maintain dependencies for the CLI (bun.lock) + - package-ecosystem: "bun" + directory: "/cli" + schedule: + interval: "weekly" + groups: + cli-deps: + patterns: ["*"] + + # Maintain dependencies for the YAML validator + - package-ecosystem: "npm" + directory: "/windmill-yaml-validator" + schedule: + interval: "weekly" + groups: + yaml-validator-deps: + patterns: ["*"] + yaml-validator-security: + applies-to: security-updates + patterns: ["*"] + + # Maintain dependencies for the TypeScript client (devDependencies only) + - package-ecosystem: "npm" + directory: "/typescript-client" + schedule: + interval: "weekly" + groups: + typescript-client-deps: + patterns: ["*"] + typescript-client-security: + applies-to: security-updates + patterns: ["*"] + + # Maintain dependencies for multiplayer + - package-ecosystem: "npm" + directory: "/multiplayer" + schedule: + interval: "weekly" + groups: + multiplayer-deps: + patterns: ["*"] + multiplayer-security: + applies-to: security-updates + patterns: ["*"] diff --git a/.github/workflows/ai-agent-tests.yml b/.github/workflows/ai-agent-tests.yml index cd07602109..8a3e9e6dbc 100644 --- a/.github/workflows/ai-agent-tests.yml +++ b/.github/workflows/ai-agent-tests.yml @@ -63,11 +63,11 @@ jobs: with: bun-version: 1.4.0 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v7 with: node-version: "20" - - uses: actions/setup-python@v5 + - uses: actions/setup-python@v7 with: python-version: "3.11" diff --git a/.github/workflows/ai-evals-test.yml b/.github/workflows/ai-evals-test.yml index 87eefab0f3..573bc77df9 100644 --- a/.github/workflows/ai-evals-test.yml +++ b/.github/workflows/ai-evals-test.yml @@ -77,7 +77,7 @@ jobs: with: bun-version: 1.4.0 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v7 with: # Node 24 ships npm 11, which frontend/package-lock.json is authored # with; npm 10 rejects it ("Missing: picomatch@4.0.5 from lock file"). diff --git a/.github/workflows/backend-test-windows.yml b/.github/workflows/backend-test-windows.yml index f9153c0ce0..54a62ec495 100644 --- a/.github/workflows/backend-test-windows.yml +++ b/.github/workflows/backend-test-windows.yml @@ -67,7 +67,7 @@ jobs: # tests what production runs, instead of floating on the latest v2.x. deno-version: 2.2.1 - - uses: actions/setup-go@v2 + - uses: actions/setup-go@v7 with: go-version: 1.21.5 @@ -75,11 +75,11 @@ jobs: with: bun-version: 1.4.0 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v7 with: node-version: "20" - - uses: astral-sh/setup-uv@v6.2.1 + - uses: astral-sh/setup-uv@v7 with: version: "0.11.24" diff --git a/.github/workflows/backend-test.yml b/.github/workflows/backend-test.yml index 9f65c2ae47..fea84e7781 100644 --- a/.github/workflows/backend-test.yml +++ b/.github/workflows/backend-test.yml @@ -53,16 +53,16 @@ jobs: # Pin to the Deno version shipped in the runtime image (Dockerfile) so CI # tests what production runs, instead of floating on the latest v2.x. deno-version: 2.2.1 - - uses: actions/setup-go@v2 + - uses: actions/setup-go@v7 with: go-version: 1.21.5 - uses: oven-sh/setup-bun@v2 with: bun-version: 1.4.0 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v7 with: node-version: "24" - - uses: astral-sh/setup-uv@v6.2.1 + - uses: astral-sh/setup-uv@v7 with: version: "0.11.24" - uses: shivammathur/setup-php@v2 diff --git a/.github/workflows/check-docs-links.yml b/.github/workflows/check-docs-links.yml index ec17c06771..e32e976f0c 100644 --- a/.github/workflows/check-docs-links.yml +++ b/.github/workflows/check-docs-links.yml @@ -16,7 +16,7 @@ jobs: sparse-checkout: | frontend/src .github/scripts - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v7 with: node-version: "22.x" - name: Verify docs links are not 404 diff --git a/.github/workflows/check-system-prompts.yml b/.github/workflows/check-system-prompts.yml index 3e684c82f9..370a177716 100644 --- a/.github/workflows/check-system-prompts.yml +++ b/.github/workflows/check-system-prompts.yml @@ -28,7 +28,7 @@ jobs: steps: - uses: actions/checkout@v4 - - uses: actions/setup-python@v5 + - uses: actions/setup-python@v7 with: python-version: "3.12" diff --git a/.github/workflows/cli-tests.yml b/.github/workflows/cli-tests.yml index e26231eda4..19afa777ae 100644 --- a/.github/workflows/cli-tests.yml +++ b/.github/workflows/cli-tests.yml @@ -43,7 +43,7 @@ jobs: uses: actions/checkout@v4 - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version: "20" @@ -89,7 +89,7 @@ jobs: cache-workspaces: backend - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version: "20" @@ -143,7 +143,7 @@ jobs: cache-workspaces: backend - name: Setup Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version: "20" diff --git a/.github/workflows/codex-pr-review.yml b/.github/workflows/codex-pr-review.yml index 7a3882df7c..354e8a60ca 100644 --- a/.github/workflows/codex-pr-review.yml +++ b/.github/workflows/codex-pr-review.yml @@ -213,7 +213,7 @@ jobs: - name: Set up Node.js if: steps.codex_config.outputs.enabled == 'true' && steps.pr.outputs.skip != 'true' - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version: 22 diff --git a/.github/workflows/frontend-check.yml b/.github/workflows/frontend-check.yml index 178ed53dc5..6347614e16 100644 --- a/.github/workflows/frontend-check.yml +++ b/.github/workflows/frontend-check.yml @@ -18,7 +18,7 @@ jobs: runs-on: ubicloud-standard-8 steps: - uses: actions/checkout@v4 - - uses: actions/setup-node@v5 + - uses: actions/setup-node@v7 with: node-version: 24 cache: "npm" diff --git a/.github/workflows/git-sync-test.yml b/.github/workflows/git-sync-test.yml index bd15ec8876..b19f899a20 100644 --- a/.github/workflows/git-sync-test.yml +++ b/.github/workflows/git-sync-test.yml @@ -141,7 +141,7 @@ jobs: with: deno-version: v2.x - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v7 with: node-version: "20" diff --git a/.github/workflows/go_on_release.yml b/.github/workflows/go_on_release.yml index 78ce12c6c3..6e53db09e7 100644 --- a/.github/workflows/go_on_release.yml +++ b/.github/workflows/go_on_release.yml @@ -13,7 +13,7 @@ jobs: runs-on: ubicloud-standard-8 steps: - uses: actions/checkout@v4 - - uses: actions/setup-go@v4 + - uses: actions/setup-go@v7 - name: install_jq run: | sudo apt-get update diff --git a/.github/workflows/npm_on_release.yml b/.github/workflows/npm_on_release.yml index 7d30265c74..631ab53153 100644 --- a/.github/workflows/npm_on_release.yml +++ b/.github/workflows/npm_on_release.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubicloud-standard-8 steps: - uses: actions/checkout@v4 - - uses: actions/setup-node@v3 + - uses: actions/setup-node@v7 with: node-version: "20.x" registry-url: "https://registry.npmjs.org" @@ -21,7 +21,7 @@ jobs: runs-on: ubicloud-standard-8 steps: - uses: actions/checkout@v4 - - uses: actions/setup-node@v3 + - uses: actions/setup-node@v7 with: node-version: "20.x" registry-url: "https://registry.npmjs.org" @@ -32,7 +32,7 @@ jobs: runs-on: ubicloud-standard-8 steps: - uses: actions/checkout@v4 - - uses: actions/setup-node@v3 + - uses: actions/setup-node@v7 with: node-version: "20.x" registry-url: "https://registry.npmjs.org" diff --git a/.github/workflows/pi-pr-review.yml b/.github/workflows/pi-pr-review.yml index 9b536b6ed5..a89275cfb5 100644 --- a/.github/workflows/pi-pr-review.yml +++ b/.github/workflows/pi-pr-review.yml @@ -206,7 +206,7 @@ jobs: - name: Set up Node.js if: steps.pi_config.outputs.enabled == 'true' && steps.pr.outputs.skip != 'true' - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version: 22 diff --git a/.github/workflows/publish-cli-docs.yml b/.github/workflows/publish-cli-docs.yml index 9e76117eb5..d6a2bdb332 100644 --- a/.github/workflows/publish-cli-docs.yml +++ b/.github/workflows/publish-cli-docs.yml @@ -38,7 +38,7 @@ jobs: ssh-key: ${{ secrets.CLI_DOCS_DEPLOY_KEY }} fetch-depth: 0 - - uses: actions/setup-python@v5 + - uses: actions/setup-python@v7 with: python-version: "3.12" diff --git a/.github/workflows/sdk-tests.yml b/.github/workflows/sdk-tests.yml index 6f7d79c6af..260fbb3a02 100644 --- a/.github/workflows/sdk-tests.yml +++ b/.github/workflows/sdk-tests.yml @@ -43,7 +43,7 @@ jobs: with: bun-version: latest - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v7 with: node-version: "20.x" @@ -58,7 +58,7 @@ jobs: uses: actions/checkout@v4 - name: Setup uv - uses: astral-sh/setup-uv@v5 + uses: astral-sh/setup-uv@v7 # The interpreter is named explicitly: on a clean checkout uv picks the # runner's system python and stops with "not compatible with the locked diff --git a/.github/workflows/validate-openapi.yml b/.github/workflows/validate-openapi.yml index 5bc59ebc4c..846621b503 100644 --- a/.github/workflows/validate-openapi.yml +++ b/.github/workflows/validate-openapi.yml @@ -15,7 +15,7 @@ jobs: uses: actions/checkout@v4 - name: Set up Node.js - uses: actions/setup-node@v4 + uses: actions/setup-node@v7 with: node-version: '20' diff --git a/.github/workflows/yaml-validator-tests.yml b/.github/workflows/yaml-validator-tests.yml index 04d31451d4..c5524312cb 100644 --- a/.github/workflows/yaml-validator-tests.yml +++ b/.github/workflows/yaml-validator-tests.yml @@ -24,7 +24,7 @@ jobs: steps: - uses: actions/checkout@v4 - - uses: actions/setup-node@v4 + - uses: actions/setup-node@v7 with: node-version: "20" diff --git a/AGENTS.md b/AGENTS.md index a315f1a67c..77b38c3267 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,12 +55,23 @@ Open-source platform for internal tools, workflows, API integrations, background > defaults in this section apply only to a plain single checkout. **Discover the real > values before running anything** — see "Per-worktree ports and database" below. -**Check whether they are already running before starting anything.** In a webmux worktree -(`$WEBMUX_WORKTREE_PATH` is set) the backend and frontend are already up in sibling tmux panes — -use those, don't spawn your own. `tmux list-panes -t "$(tmux display-message -p -t "$TMUX_PANE" -'#{window_id}')" -F '#{pane_index} #{pane_current_command}'` shows what is running; read its log -with `tmux capture-pane`, and see `backend/AGENTS.md` to restart it with different cargo features. -A second server started in your own shell fights the first one for the port. The commands below +**Check whether they are already running before starting anything.** In a managed worktree the +backend and frontend are already up in sibling panes — use those, don't spawn your own. A second +server started in your own shell fights the first one for the port. + +**herdr** is the worktree manager; `$HERDR_ENV=1` marks one of its panes. `herdr pane list +--workspace "$HERDR_WORKSPACE_ID"` lists this worktree's panes — match on `cwd`, since the servers +run from `backend/` and `frontend/` — and `herdr pane read --source recent-unwrapped +--lines 50` reads one's log; `herdr --skill` prints the full reference for +inspecting and driving panes and agents. The plugins that provision these worktrees live in the +private `windmill-labs/windmill-herdr` — clone it and run `./setup.sh` to install them and the +keybindings they need. + +A worktree from the older **webmux** setup sets `$WEBMUX_WORKTREE_PATH` instead and is driven +through tmux: `tmux list-panes -t "$(tmux display-message -p -t "$TMUX_PANE" '#{window_id}')" -F +'#{pane_index} #{pane_current_command}'` for what is running, `tmux capture-pane` for a pane's log. + +See `backend/AGENTS.md` to restart the backend with different cargo features. The commands below are for a plain checkout with nothing running. - **Backend**: `cargo run` from `backend/` (API at http://localhost:8000) @@ -72,10 +83,16 @@ are for a plain checkout with nothing running. ### Per-worktree ports and database -In a webmux worktree the authoritative values live in -`$(git rev-parse --git-dir)/webmux/runtime.env` — `BACKEND_PORT`, `FRONTEND_PORT`, -`DATABASE_URL`, `CARGO_FEATURES`, `WM_DB_NAME`. Every pane sources it at startup. Read that -first: it is not a `.env*` file, so the repo's secret-file read rules don't stand in the way. +**`.env.local` in the worktree root holds the real values** — `BACKEND_PORT`, `FRONTEND_PORT`, +`REMOTE`, `DATABASE_URL`, `CARGO_FEATURES`, `WM_DB_NAME`. Every manager writes those fields, so it +is correct whichever one you are in. Read it with `cat .env.local` from a shell: the file-read +tool denies every `.env.*` path, and `DATABASE_URL` is written with an `export` prefix that a +`^DATABASE_URL=` grep misses. + +herdr keeps nothing besides that file; its hooks write it and read it back. A webmux worktree +additionally has `$(git rev-parse --git-dir)/webmux/runtime.env`, which every pane sources at +startup and which carries the extras `.env.local` lacks: `WEBMUX_*`, `WM_CLONE_DB`, +`USE_RUST_PLUGIN`. In a plain checkout, fall back to `.env` / `.env.local` (repo root) and `backend/.env`. @@ -84,19 +101,25 @@ hook. It is not a copy of the main dev instance: you get the `admins` workspace, `admin@windmill.dev` superadmin, the license key copied from the base database, and whatever the migrations seed — and none of your own workspaces, scripts, flows or apps. Create whatever a test needs. Cloning the base `windmill` database instead is -opt-in per project via `WM_CLONE_DB` in `.webmux.yaml`; read the note there before turning it on. +opt-in via `WM_CLONE_DB` (the `windmill.worktree` plugin's `config.env` under herdr, or +`.webmux.yaml` under webmux); read the note in `.webmux.yaml` before turning it on. The database is named after the **worktree directory, not the branch** (`scripts/worktree-common.sh`): `windmill_` + the directory basename with `-` → `_`, which Postgres then truncates at 63 -characters. Branch `hugo/win-2340-ai-agent-evals-standalone-agent-runs-and-eval-datasets` sits in -a worktree directory named `win-2340-…`, so its database is -`windmill_win_2340_ai_agent_evals_standalone_agent_runs_and_eval` — no `hugo_`, and the tail -chopped. Take `WM_DB_NAME` from `runtime.env` instead of reconstructing the name. Read those, or -discover from what is already running: +characters. herdr creates the directory under `~/.herdr/worktrees//` from the branch name +with each `/` turned into `-`, so branch +`hugo/win-2544-add-options-field-to-the-postgresql-resource-type` gets the database +`windmill_hugo_win_2544_add_options_field_to_the_postgresql_reso` — the whole branch name, cut +mid-word at the limit. The two drift apart as soon as the branch is renamed, and webmux named its +directories after the ticket alone, so reconstructing the name from the branch you are on is +wrong in both. Take `WM_DB_NAME` from `.env.local` instead. Read that, or discover from what is +already running: ```bash +# match on the worktree directory, truncated the way Postgres truncates it (63 - len('windmill_')): psql postgres://postgres:changeme@localhost:5432/postgres -tAc \ - "select datname from pg_database where datname like 'windmill%'" | grep "$(git branch --show-current | tr - _)" + "select datname from pg_database where datname like 'windmill%'" \ + | grep -F "$(basename "$(git rev-parse --show-toplevel)" | tr '/-' '_' | cut -c1-54)" # the port the frontend actually proxies to (REMOTE of this worktree's vite): for p in $(pgrep -f vite); do case "$(readlink /proc/$p/cwd)" in *"$(basename "$(git rev-parse --show-toplevel)")"*) tr '\0' '\n' < /proc/$p/environ | grep -E '^REMOTE=|^PORT=';; esac; done diff --git a/ai_evals/adapters/frontend/mockBackend.ts b/ai_evals/adapters/frontend/mockBackend.ts index 950cc2e1b5..90df0cf870 100644 --- a/ai_evals/adapters/frontend/mockBackend.ts +++ b/ai_evals/adapters/frontend/mockBackend.ts @@ -14,7 +14,6 @@ import type { DataMetric, DataTableTables, DataTableTableSchema, - EndpointTool, GetDraftForUserResponse, GetOwnDraftResponse, ListDraftsResponse, @@ -1020,119 +1019,6 @@ function buildBenchmarkApp(app: BenchmarkWorkspaceApp): AppWithLastVersion { } } -// ============= API endpoint catalog (McpService.listMcpTools + raw fetch) ============= -// The global chat's API catalog tools list endpoints via McpService and execute -// them with a plain relative fetch('/api/...'), which has no meaning in the -// vitest environment. A representative slice of the real catalog is served here, -// and `handleBenchmarkApiFetch` answers the executed calls. - -const BENCHMARK_MCP_TOOLS: EndpointTool[] = [ - { - name: 'listWorkers', - description: 'List workers', - instructions: 'List all workers with their last ping and job counts.', - path: '/workers/list', - method: 'GET', - query_params_schema: { - type: 'object', - properties: { page: { type: 'integer' }, per_page: { type: 'integer' } } - } - }, - { - name: 'listQueue', - description: 'List queued jobs', - instructions: '', - path: '/w/{workspace}/jobs/queue/list', - method: 'GET', - path_params_schema: { - type: 'object', - properties: { workspace: { type: 'string' } }, - required: ['workspace'] - } - }, - { - name: 'getJob', - description: 'get job', - instructions: '', - path: '/w/{workspace}/jobs_u/get/{id}', - method: 'GET', - path_params_schema: { - type: 'object', - properties: { workspace: { type: 'string' }, id: { type: 'string', format: 'uuid' } }, - required: ['workspace', 'id'] - }, - query_params_schema: { - type: 'object', - properties: { - no_logs: { type: 'boolean' }, - no_code: { type: 'boolean' }, - approval_token: { type: 'string' } - }, - required: [] - } - }, - { - name: 'runScriptByPath', - description: 'Run the deployed version of a script by path', - instructions: '', - path: '/w/{workspace}/jobs/run/p/{path}', - method: 'POST', - path_params_schema: { - type: 'object', - properties: { workspace: { type: 'string' }, path: { type: 'string' } }, - required: ['workspace', 'path'] - }, - body_schema: { type: 'object', properties: {} } - }, - { - name: 'runFlowByPath', - description: 'Run the deployed version of a flow by path', - instructions: '', - path: '/w/{workspace}/jobs/run/f/{path}', - method: 'POST', - path_params_schema: { - type: 'object', - properties: { workspace: { type: 'string' }, path: { type: 'string' } }, - required: ['workspace', 'path'] - }, - body_schema: { type: 'object', properties: {} } - }, - // Draft-covered endpoints, present so steering cases exercise the guard the - // way production does (hidden from search, refused at call time). - { - name: 'getScriptByPath', - description: 'Get a script by path', - instructions: '', - path: '/w/{workspace}/scripts/get/p/{path}', - method: 'GET' - }, - { - name: 'createFlow', - description: 'Create a flow', - instructions: '', - path: '/w/{workspace}/flows/create', - method: 'POST' - }, - { - name: 'deleteSchedule', - description: 'Delete a schedule', - instructions: '', - path: '/w/{workspace}/schedules/delete/{path}', - method: 'DELETE' - }, - { - name: 'getVariable', - description: 'Get a variable', - instructions: '', - path: '/w/{workspace}/variables/get/{path}', - method: 'GET' - } -] - -export function listBenchmarkMcpTools(): EndpointTool[] { - return BENCHMARK_MCP_TOOLS -} - /** A stand-in Windmill hub. `search_hub_scripts` and a `hub/` read go out over * relative `/api/...` fetches, which have no origin here, so without these the * hub tools throw and no case can exercise hub reuse. Serving fixtures rather @@ -1292,26 +1178,6 @@ const BENCHMARK_WORKERS = [ } ] -const BENCHMARK_JOB_GET_PATH = /^\/api\/w\/([^/]+)\/jobs_u\/get\/([^/]+)$/ -const BENCHMARK_RUN_BY_PATH = /^\/api\/w\/([^/]+)\/jobs\/run\/(p|f)\/([^/]+)$/ - -/** `executeEndpoint` sends a JSON string; anything else means no args were supplied. */ -function parseBenchmarkRequestBody( - body: BodyInit | null | undefined -): Record | undefined { - if (typeof body !== 'string') { - return undefined - } - try { - const parsed = JSON.parse(body) - return typeof parsed === 'object' && parsed !== null - ? (parsed as Record) - : undefined - } catch { - return undefined - } -} - /** True when `handleBenchmarkApiFetch` has an answer for this `/api/...` url. * Any other relative fetch must keep its normal (non-benchmark) behavior — * intercepting it with a synthetic 404 sends the model into retry loops. */ @@ -1323,17 +1189,13 @@ export function hasBenchmarkApiHandler(url: string): boolean { const path = url.split('?')[0] return ( path === '/api/workers/list' || - BENCHMARK_JOB_GET_PATH.test(path) || - BENCHMARK_RUN_BY_PATH.test(path) || - /^\/api\/w\/[^/]+\/jobs\/queue\/list$/.test(path) || path === '/api/embeddings/query_hub_scripts' || path.startsWith('/api/scripts/hub/get_full/') || BENCHMARK_AI_MODELS_PATH.test(path) ) } -/** Answer a relative `/api/...` fetch — from the API catalog executor, or from the - * chat's hub tools. */ +/** Answer the relative `/api/...` fetches no mocked service covers. */ export function handleBenchmarkApiFetch(url: string, init?: RequestInit): Response { const path = url.split('?')[0] if (path === '/api/workers/list') { @@ -1350,39 +1212,6 @@ export function handleBenchmarkApiFetch(url: string, init?: RequestInit): Respon ?.aiProviders?.find((entry) => entry.path === resourcePath) return Response.json({ data: (seed?.models ?? []).map((id) => ({ id })) }) } - if (/^\/api\/w\/[^/]+\/jobs\/queue\/list$/.test(path)) { - return Response.json([]) - } - const jobGet = BENCHMARK_JOB_GET_PATH.exec(path) - if (jobGet) { - const id = decodeURIComponent(jobGet[2]) - const job = getBenchmarkCompletedJob(decodeURIComponent(jobGet[1]), id) - if (!job) { - return Response.json({ error: `Job not found for "${id}"` }, { status: 404 }) - } - // The real endpoint lets a caller drop the bulky fields. Ignoring that here would - // size the model's context off a payload it explicitly asked to shrink. - const query = new URLSearchParams(url.split('?')[1] ?? '') - if (query.get('no_logs') === 'true') { - delete job.logs - } - if (query.get('no_code') === 'true') { - delete job.raw_code - } - return Response.json(job) - } - const runByPath = BENCHMARK_RUN_BY_PATH.exec(path) - if (runByPath) { - const workspace = decodeURIComponent(runByPath[1]) - const runnablePath = decodeURIComponent(runByPath[3]) - const args = parseBenchmarkRequestBody(init?.body) - // The real endpoint answers with the bare job id as text, not JSON. - return new Response( - runByPath[2] === 'f' - ? runBenchmarkFlowByPath({ workspace, path: runnablePath, args }) - : runBenchmarkScriptByPath({ workspace, path: runnablePath, args }) - ) - } if (path === '/api/embeddings/query_hub_scripts') { const text = new URLSearchParams(url.split('?')[1] ?? '').get('text') ?? '' return Response.json(searchBenchmarkHubScripts(text)) diff --git a/ai_evals/adapters/frontend/mockBackendApi.test.ts b/ai_evals/adapters/frontend/mockBackendApi.test.ts index d9cfa08bf4..85bd1f3320 100644 --- a/ai_evals/adapters/frontend/mockBackendApi.test.ts +++ b/ai_evals/adapters/frontend/mockBackendApi.test.ts @@ -1,83 +1,18 @@ -import { afterEach, beforeEach, describe, expect, it } from 'bun:test' -import { - createBenchmarkCompletedJob, - getBenchmarkCompletedJob, - handleBenchmarkApiFetch, - hasBenchmarkApiHandler, - listBenchmarkMcpTools, - resetBenchmarkMockBackend, - registerBenchmarkWorkspaceRunnables -} from './mockBackend' +import { describe, expect, it } from 'bun:test' +import { handleBenchmarkApiFetch, hasBenchmarkApiHandler } from './mockBackend' -const WORKSPACE = 'benchmark-api-ws' +// `list_workers`'s client call is not faked, so its request really does leave as a relative +// `/api/...` url that node's fetch cannot resolve. Without this route the tool throws instead +// of answering. +describe('benchmark API fetch handlers', () => { + it('lists workers, the way list_workers reaches them', async () => { + const url = '/api/workers/list?per_page=100' -// A catalog entry with no fetch handler is a dead end: the catalog executor builds a -// relative `/api/...` url, the stub declines it, and node's fetch throws on the relative -// url instead of returning a result the model can act on. Mutating entries are reachable -// too — the eval runners define no `requestConfirmation`, so `call_api_endpoint` executes -// unconfirmed. -describe('benchmark API catalog', () => { - beforeEach(() => resetBenchmarkMockBackend()) - afterEach(() => resetBenchmarkMockBackend()) + expect(hasBenchmarkApiHandler(url)).toBe(true) + const body = await handleBenchmarkApiFetch(url).json() - it('answers every endpoint it advertises', () => { - const unanswered = listBenchmarkMcpTools() - .map((tool) => - `/api${tool.path.replace('{workspace}', WORKSPACE)}`.replace(/\{[^}]+\}/g, 'x') - ) - .filter((url) => !hasBenchmarkApiHandler(url)) - - // The draft-covered entries are refused by name before any fetch, so they are - // advertised without a handler on purpose. - expect(unanswered).toEqual([ - `/api/w/${WORKSPACE}/scripts/get/p/x`, - `/api/w/${WORKSPACE}/flows/create`, - `/api/w/${WORKSPACE}/schedules/delete/x`, - `/api/w/${WORKSPACE}/variables/get/x` - ]) - }) - - it('runs a deployed script by path, the way call_api_endpoint reaches it', async () => { - registerBenchmarkWorkspaceRunnables(WORKSPACE, { - scripts: [ - { - path: 'f/evals/greet', - summary: 'Greet', - language: 'bun', - content: 'export async function main() {}' - } - ] - }) - - const res = handleBenchmarkApiFetch( - `/api/w/${WORKSPACE}/jobs/run/p/${encodeURIComponent('f/evals/greet')}`, - { method: 'POST', body: JSON.stringify({ name: 'ada' }) } - ) - - expect(res.status).toBe(200) - const job = getBenchmarkCompletedJob(WORKSPACE, (await res.text()).trim()) - expect(job).toMatchObject({ success: true, args: { name: 'ada' } }) - }) - - it('serves a recorded job so a model can check the run it just started', async () => { - const id = createBenchmarkCompletedJob({ - workspace: WORKSPACE, - jobKind: 'preview', - result: 'Hello, World!' - }) - - const res = handleBenchmarkApiFetch(`/api/w/${WORKSPACE}/jobs_u/get/${id}`) - - expect(res.status).toBe(200) - expect(await res.json()).toMatchObject({ - id, - success: true, - result: 'Hello, World!' - }) - }) - - it('404s an unknown job id instead of letting the fetch fall through', () => { - expect(hasBenchmarkApiHandler(`/api/w/${WORKSPACE}/jobs_u/get/missing`)).toBe(true) - expect(handleBenchmarkApiFetch(`/api/w/${WORKSPACE}/jobs_u/get/missing`).status).toBe(404) + // The tool counts the list and maps over it, so it needs a bare array, not an envelope. + expect(Array.isArray(body)).toBe(true) + expect(body[0]).toMatchObject({ worker: expect.any(String), worker_group: expect.any(String) }) }) }) diff --git a/ai_evals/adapters/frontend/vitestAdapter.test.ts b/ai_evals/adapters/frontend/vitestAdapter.test.ts index c9e889b9a0..0c8a22a987 100644 --- a/ai_evals/adapters/frontend/vitestAdapter.test.ts +++ b/ai_evals/adapters/frontend/vitestAdapter.test.ts @@ -5,8 +5,8 @@ import { mkdir, writeFile } from 'fs/promises' import { dirname, resolve } from 'path' import { handleBenchmarkApiFetch, hasBenchmarkApiHandler } from './mockBackend' -// The API catalog executor issues relative fetch('/api/...') calls, which have -// no meaning in the vitest environment — serve the ones the benchmark handles. +// Some tools reach the backend by relative fetch('/api/...'), which has no meaning +// in the vitest environment — serve the ones the benchmark handles. // Every other relative fetch keeps its normal behavior (it fails the same way // it does without this stub) so unrelated tools see an unchanged environment. // The frontend builds API URLs from location.origin (fetchAvailableModels does), and node has no @@ -92,8 +92,7 @@ vi.mock('$lib/gen', async () => { runBenchmarkFlowPreview, runBenchmarkScriptByPath, runBenchmarkScriptPreview, - updateBenchmarkDraft, - listBenchmarkMcpTools + updateBenchmarkDraft } = await import('./mockBackend') function wrapService(target: T, overrides: Record): T { @@ -434,12 +433,6 @@ vi.mock('$lib/gen', async () => { queryResourceTypes: async (data: { workspace: string }) => hasBenchmarkWorkspace(data.workspace) ? [] : actual.ResourceService.queryResourceTypes(data) }), - McpService: wrapService(actual.McpService, { - listMcpTools: async (data: { workspace: string }) => - hasBenchmarkWorkspace(data.workspace) - ? listBenchmarkMcpTools() - : actual.McpService.listMcpTools(data) - }), VariableService: wrapService(actual.VariableService, { existsVariable: async (data: { workspace: string; path: string }) => hasBenchmarkWorkspace(data.workspace) diff --git a/ai_evals/cases/global.yaml b/ai_evals/cases/global.yaml index fdb2442729..b17d37cb29 100644 --- a/ai_evals/cases/global.yaml +++ b/ai_evals/cases/global.yaml @@ -1919,11 +1919,10 @@ - when the lookup fails, tells the user instead of inventing table names - does not write scripts or resources to answer a read-only question -# --- Dedicated tools preferred over the API catalog --- -# The harness serves worker/queue reads itself (benchmark fetch handlers in -# adapters/frontend), so these cases do not require an mcp-enabled eval backend. -# The stale `api-catalog` in the id below is kept so results stay comparable -# across benchmark runs. +# --- Dedicated tools for workspace, run and instance state --- +# The harness serves the worker read itself (a benchmark fetch handler in +# adapters/frontend). The stale `api-catalog` in the id below is kept so +# results stay comparable across benchmark runs. - id: global-test30-api-catalog-workers prompt: |- @@ -1937,8 +1936,6 @@ requiredToolsUsed: - list_workers forbiddenToolsUsed: - - call_api_get - - call_api_endpoint - write_script - deploy_workspace_item # Read-only workspace inspection produces no draft; validate via tool use. @@ -1990,13 +1987,11 @@ requiredToolsUsed: - test_run_script forbiddenToolsUsed: - - call_api_endpoint - deploy_workspace_item - delete_workspace_item # The judge only sees the drafts artifact and cannot observe runs, so it always # docks the prompt's "run it" requirement — validate deterministically instead: - # draft content via valueIncludes, the test run via toolExpect (test_run_script - # required, call_api_endpoint forbidden). + # draft content via valueIncludes, the test run via toolExpect. skipJudge: true judgeChecklist: - creates an AI draft of f/evals/global/format_greeting with the name uppercased in the greeting @@ -2014,13 +2009,11 @@ requiredToolsUsed: - delete_workspace_item forbiddenToolsUsed: - - call_api_endpoint - - search_api_endpoints - write_script # Deletion produces no draft; validate via tool use. skipJudge: true judgeChecklist: - - deletes the deployed script via delete_workspace_item rather than a raw API endpoint + - deletes the deployed script via delete_workspace_item - id: global-test33-run-deployed-script-with-form prompt: |- @@ -2041,7 +2034,6 @@ - read_workspace_item forbiddenToolsUsed: - test_run_script - - call_api_endpoint - write_script - deploy_workspace_item # An empty form pushes the work back onto the user, so the prefill is part of @@ -2054,7 +2046,7 @@ # Running produces no draft, and the judge cannot observe runs; validate via tool use. skipJudge: true judgeChecklist: - - runs the deployed script through run_script rather than a preview test run or a raw API endpoint + - runs the deployed script through run_script rather than a preview test run - passes the name "ada" so the confirmation form comes up prefilled - id: global-test34-run-with-secret-from-variable @@ -2113,7 +2105,6 @@ - read_workspace_item forbiddenToolsUsed: - test_run_flow - - call_api_endpoint - write_flow - deploy_workspace_item # An empty form pushes the work back onto the user, so the prefill is part of @@ -2126,7 +2117,7 @@ # Running produces no draft, and the judge cannot observe runs; validate via tool use. skipJudge: true judgeChecklist: - - runs the deployed flow through run_flow rather than a preview test run or a raw API endpoint + - runs the deployed flow through run_flow rather than a preview test run - passes the customer "acme" so the confirmation form comes up prefilled - id: global-test36-draft-flow-test-run-not-deployed @@ -2151,7 +2142,6 @@ # version instead — the edit would not be in what ran. forbiddenToolsUsed: - run_flow - - call_api_endpoint - deploy_workspace_item # The judge cannot observe runs, and the edit's content is already pinned by # global-test5 on this fixture; what this case guards is where the run went. diff --git a/backend/.sqlx/query-2b93351b9c2c91272c3f69c9c8eba192d2e4d087c7d7db8d3b257bb864c8265d.json b/backend/.sqlx/query-0f374865faa5bdf884345808a004ce891a85daa00e037aba4b65b60f87243bd3.json similarity index 65% rename from backend/.sqlx/query-2b93351b9c2c91272c3f69c9c8eba192d2e4d087c7d7db8d3b257bb864c8265d.json rename to backend/.sqlx/query-0f374865faa5bdf884345808a004ce891a85daa00e037aba4b65b60f87243bd3.json index b82fcf2e17..2ff7820d86 100644 --- a/backend/.sqlx/query-2b93351b9c2c91272c3f69c9c8eba192d2e4d087c7d7db8d3b257bb864c8265d.json +++ b/backend/.sqlx/query-0f374865faa5bdf884345808a004ce891a85daa00e037aba4b65b60f87243bd3.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "INSERT INTO datatable_migrations (workspace_id, datatable, timestamp, name, code_up, code_down) VALUES ($1, $2, $3, $4, $5, $6) ON CONFLICT (workspace_id, datatable, timestamp) DO UPDATE SET name = EXCLUDED.name, code_up = EXCLUDED.code_up, code_down = EXCLUDED.code_down WHERE NOT $7 OR (datatable_migrations.name = $8::text AND datatable_migrations.code_up = $9::text AND datatable_migrations.code_down IS NOT DISTINCT FROM $10::text)", + "query": "INSERT INTO datatable_migrations (workspace_id, datatable, timestamp, name, code_up, code_down) VALUES ($1, $2, $3, $4, $5, $6) ON CONFLICT (workspace_id, datatable, timestamp) DO UPDATE SET name = EXCLUDED.name, code_up = EXCLUDED.code_up, code_down = EXCLUDED.code_down WHERE datatable_migrations.name = $7::text AND datatable_migrations.code_up = $8::text AND datatable_migrations.code_down IS NOT DISTINCT FROM $9::text", "describe": { "columns": [], "parameters": { @@ -11,7 +11,6 @@ "Varchar", "Text", "Text", - "Bool", "Text", "Text", "Text" @@ -19,5 +18,5 @@ }, "nullable": [] }, - "hash": "2b93351b9c2c91272c3f69c9c8eba192d2e4d087c7d7db8d3b257bb864c8265d" + "hash": "0f374865faa5bdf884345808a004ce891a85daa00e037aba4b65b60f87243bd3" } diff --git a/backend/.sqlx/query-3c057e0284f1219865b8f7dc6eb3c1293c4b72d0b52abeefd1e954617984a2d8.json b/backend/.sqlx/query-91e2456814ac29534f5a1f65a3bb97462276930f0a83ffb458f7cb26035c8c3d.json similarity index 59% rename from backend/.sqlx/query-3c057e0284f1219865b8f7dc6eb3c1293c4b72d0b52abeefd1e954617984a2d8.json rename to backend/.sqlx/query-91e2456814ac29534f5a1f65a3bb97462276930f0a83ffb458f7cb26035c8c3d.json index 3206ba4433..57dbc41a9f 100644 --- a/backend/.sqlx/query-3c057e0284f1219865b8f7dc6eb3c1293c4b72d0b52abeefd1e954617984a2d8.json +++ b/backend/.sqlx/query-91e2456814ac29534f5a1f65a3bb97462276930f0a83ffb458f7cb26035c8c3d.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "DELETE FROM datatable_migrations WHERE workspace_id = $1 AND datatable = $2 AND timestamp = $3 RETURNING name", + "query": "DELETE FROM datatable_migrations WHERE workspace_id = $1 AND datatable = $2 AND timestamp = $3 AND code_up = $4 AND code_down IS NOT DISTINCT FROM $5 RETURNING name", "describe": { "columns": [ { @@ -13,12 +13,14 @@ "Left": [ "Text", "Text", - "Int8" + "Int8", + "Text", + "Text" ] }, "nullable": [ false ] }, - "hash": "3c057e0284f1219865b8f7dc6eb3c1293c4b72d0b52abeefd1e954617984a2d8" + "hash": "91e2456814ac29534f5a1f65a3bb97462276930f0a83ffb458f7cb26035c8c3d" } diff --git a/backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json b/backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json new file mode 100644 index 0000000000..2ff2722f77 --- /dev/null +++ b/backend/.sqlx/query-d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a.json @@ -0,0 +1,15 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings ws\n SET datatable = (\n SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(\n dt.key,\n (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $2\n THEN jsonb_set(r.v, '{governed_by,workspace_id}', to_jsonb($1::text))\n ELSE r.v END\n FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $2\n THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text))\n ELSE dt.value END AS v) r)\n ))\n FROM jsonb_each(ws.datatable->'datatables') dt\n )\n WHERE jsonb_typeof(ws.datatable->'datatables') = 'object'\n AND (ws.datatable::text LIKE '%\"reference\"%'\n OR ws.datatable::text LIKE '%\"governed_by\"%')", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "d0548225d92e6e7d0eb9a0227a6522398d2a3ae99d0568b785e63749d6f3225a" +} diff --git a/backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json b/backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json new file mode 100644 index 0000000000..af8d0eb1ab --- /dev/null +++ b/backend/.sqlx/query-e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT ws.workspace_id AS \"workspace_id!\", dt.key AS \"datatable!\"\n FROM workspace_settings ws\n CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt\n WHERE EXISTS (\n SELECT 1 FROM (VALUES ('reference'), ('governed_by')) k(link)\n WHERE dt.value->k.link->>'workspace_id' = $1\n AND dt.value->k.link->>'datatable' = $2\n )", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "workspace_id!", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "datatable!", + "type_info": "Text" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false, + null + ] + }, + "hash": "e524e89440004953cc0b1cb57b8df6bc09f19cf53b141fac5aefa981463d39fa" +} diff --git a/backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.json b/backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.json new file mode 100644 index 0000000000..1be51feb85 --- /dev/null +++ b/backend/.sqlx/query-f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE workspace_settings ws\n SET datatable = (\n SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg(\n dt.key,\n (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $1\n AND r.v->'governed_by'->>'datatable' = $2\n THEN jsonb_set(r.v, '{governed_by,datatable}', to_jsonb($3::text))\n ELSE r.v END\n FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $1\n AND dt.value->'reference'->>'datatable' = $2\n THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text))\n ELSE dt.value END AS v) r)\n ))\n FROM jsonb_each(ws.datatable->'datatables') dt\n )\n WHERE EXISTS (\n SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d,\n LATERAL (VALUES ('reference'), ('governed_by')) k(link)\n WHERE d.value->k.link->>'workspace_id' = $1\n AND d.value->k.link->>'datatable' = $2\n )", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Text" + ] + }, + "nullable": [] + }, + "hash": "f7ece5036ad92e485b5e15a70e5052b42aaf5c87e5be6333b2df67a81990c8a6" +} diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 7bd84dfca3..750e0594e1 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -23,7 +23,7 @@ ## Cargo features & running the dev backend The dev backend runs under `cargo watch` and is launched by default with **only -`--features quickjs`** (see the tmux backend pane). That baseline compiles fast but +`--features quickjs`** (see the backend pane). That baseline compiles fast but **deliberately omits most functionality** — notably S3/object storage, the S3 proxy, all EE code, MCP, and every non-JS language runtime. A running server never gains a feature you didn't compile in: feature-gated routes 404 or return a `"requires "` stub. So if @@ -33,22 +33,30 @@ MUST **restart the backend with the appropriate features** for what you're worki ### Restarting the dev backend with the right features Restart in the **same pane**, so the relaunch inherits that pane's `DATABASE_URL`, `BACKEND_PORT` -and the rest of `runtime.env`. Scope every kill to this worktree — **never** +and the rest of the worktree environment. Scope every kill to this worktree — **never** `pkill -f target/debug/windmill`, which kills every sibling worktree's backend. -1. Find the backend pane by what it is running, not by index. The index depends on the webmux - profile: pane 1 is the backend under `full`, but the *frontend* under `frontendOnly`. +1. Find the backend pane by where it is running, not by index. Under herdr the two server panes + sit in the worktree's `backend/` and `frontend/`, so match on `cwd`: + + ```bash + herdr pane list --workspace "$HERDR_WORKSPACE_ID" + ``` + + Under webmux they are tmux panes, and the index depends on the profile: pane 1 is the backend + under `full`, but the *frontend* under `frontendOnly`. ```bash WIN=$(tmux display-message -p -t "$TMUX_PANE" '#{window_id}') tmux list-panes -t "$WIN" -F '#{pane_index} #{pane_current_command} #{pane_pid}' ``` -2. Read the feature set it is **actually** running. `CARGO_FEATURES` in `runtime.env` is only what - the pane started with, and goes stale the first time anyone restarts by hand: +2. Read the feature set it is **actually** running. The `CARGO_FEATURES` the worktree was created + with is only what the pane started with, and goes stale the first time anyone restarts by hand: ```bash - ps --ppid -o args= + herdr pane process-info --pane # foreground_processes[].argv + ps --ppid -o args= # under webmux # /home/hugo/.cargo/bin/cargo-watch watch -x run --features quickjs ``` @@ -56,22 +64,25 @@ and the rest of `runtime.env`. Scope every kill to this worktree — **never** explicitly: ```bash - tmux send-keys -t "$WIN." C-c - tmux send-keys -t "$WIN." 'PORT=$BACKEND_PORT cargo watch -x "run --features quickjs,private,parquet"' Enter + herdr pane send-keys C-c + herdr pane run 'PORT=$BACKEND_PORT cargo watch -x "run --features quickjs,private,parquet"' + # under webmux: tmux send-keys -t "$WIN." C-c, then the same command line with Enter ``` Carry over every feature the old command had unless you mean to drop one — rebuilding the list from memory is how a backend silently loses `quickjs`. -4. Persist the new set so a recreated pane starts with it: set `CARGO_FEATURES` in the - worktree's `.env.local`, which `scripts/post-create.sh` writes and webmux reads. Do **not** - edit `runtime.env` for this — webmux regenerates it from metadata and `.env.local` every time - the worktree is opened, so an edit there is lost on the next reopen. Either way the change - only affects a future pane; step 3 is what takes effect now. +4. Persist the new set so a recreated pane starts with it: set `CARGO_FEATURES` in the worktree's + `.env.local`, which the worktree hooks write. A herdr pane sources that file directly, so the + next pane picks the value up. A webmux pane sources `webmux/runtime.env` instead, which webmux + regenerates from metadata and `.env.local` when the worktree is *opened* — so the value lands + on the next reopen, not the next pane, and editing `runtime.env` by hand is lost at that same + reopen. Either way the change only affects a future pane; step 3 is what takes effect now. -5. Re-capture the pane until `health check completed` appears before hitting the API. A cold - rebuild takes ~60s, and the previous run's success line is still in the scrollback, so a - capture taken too early reads as ready when it isn't. +5. Re-read the pane (`herdr pane read --source recent-unwrapped`, or `tmux + capture-pane`) until `health check completed` appears before hitting the API. A cold rebuild + takes ~60s, and the previous run's success line is still in the scrollback, so a capture taken + too early reads as ready when it isn't. cargo-watch only re-runs on a file change, so after an idle/failed run `touch README.md` (from `backend/`, where the watch runs) is a cheap retrigger (touching a `.rs` forces a full rebuild). diff --git a/backend/Cargo.lock b/backend/Cargo.lock index d653e6cd5e..e74be23c47 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -6949,13 +6949,14 @@ dependencies = [ [[package]] name = "jsonwebtoken" -version = "8.3.0" +version = "9.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6971da4d9c3aa03c3d8f3ff0f4155b534aad021292003895a469716b2a230378" +checksum = "5a87cc7a48537badeae96744432de36f4be2b4a34a05a5ef32e9dd8a1c169dde" dependencies = [ - "base64 0.21.7", - "pem 1.1.1", - "ring 0.16.20", + "base64 0.22.1", + "js-sys", + "pem", + "ring 0.17.14", "serde", "serde_json", "simple_asn1", @@ -6963,17 +6964,20 @@ dependencies = [ [[package]] name = "jsonwebtoken" -version = "9.3.1" +version = "10.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a87cc7a48537badeae96744432de36f4be2b4a34a05a5ef32e9dd8a1c169dde" +checksum = "eba32bfb4ffdeaca3e34431072faf01745c9b26d25504aa7a6cf5684334fc4fc" dependencies = [ + "aws-lc-rs", "base64 0.22.1", + "getrandom 0.2.17", "js-sys", - "pem 3.0.6", - "ring 0.17.14", + "pem", "serde", "serde_json", + "signature", "simple_asn1", + "zeroize", ] [[package]] @@ -7068,7 +7072,7 @@ dependencies = [ "jsonpath-rust", "k8s-openapi", "kube-core", - "pem 3.0.6", + "pem", "rustls 0.23.35", "secrecy", "serde", @@ -7932,7 +7936,7 @@ dependencies = [ "lru 0.18.4", "mysql_common", "native-tls", - "pem 3.0.6", + "pem", "percent-encoding", "rand 0.10.2", "serde", @@ -9075,15 +9079,6 @@ dependencies = [ "hmac", ] -[[package]] -name = "pem" -version = "1.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8835c273a76a90455d7344889b0964598e3316e2a79ede8e36f16bdcf2228b8" -dependencies = [ - "base64 0.13.1", -] - [[package]] name = "pem" version = "3.0.6" @@ -10125,7 +10120,7 @@ version = "0.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "75e669e5202259b5314d1ea5397316ad400819437857b90861765f24c4cf80a2" dependencies = [ - "pem 3.0.6", + "pem", "ring 0.17.14", "rustls-pki-types", "time", @@ -14810,7 +14805,7 @@ dependencies = [ "git-version", "hex", "hmac", - "jsonwebtoken 8.3.0", + "jsonwebtoken 10.4.0", "lazy_static", "once_cell", "opentelemetry 0.30.0", @@ -14961,7 +14956,7 @@ dependencies = [ "hyper 1.11.1", "indexmap 2.14.2", "itertools 0.14.0", - "jsonwebtoken 8.3.0", + "jsonwebtoken 10.4.0", "lazy_static", "libxml", "magic-crypt", @@ -15115,7 +15110,7 @@ dependencies = [ "chrono", "http 1.5.0", "itertools 0.14.0", - "jsonwebtoken 8.3.0", + "jsonwebtoken 10.4.0", "lazy_static", "quick_cache", "reqwest 0.13.5", @@ -15635,7 +15630,7 @@ dependencies = [ "hyper 1.11.1", "indexmap 2.14.2", "itertools 0.14.0", - "jsonwebtoken 8.3.0", + "jsonwebtoken 10.4.0", "lazy_static", "magic-crypt", "mail-send", @@ -16472,7 +16467,7 @@ dependencies = [ "google-cloud-pubsub", "http 1.5.0", "itertools 0.14.0", - "jsonwebtoken 8.3.0", + "jsonwebtoken 10.4.0", "lazy_static", "quick_cache", "reqwest 0.13.5", @@ -16740,7 +16735,7 @@ dependencies = [ "hyper-rustls 0.27.9", "hyper-util", "itertools 0.14.0", - "jsonwebtoken 8.3.0", + "jsonwebtoken 10.4.0", "lazy_static", "libffi-sys", "libloading", @@ -16754,7 +16749,7 @@ dependencies = [ "opentelemetry-proto 0.30.0", "oracle", "parquet", - "pem 3.0.6", + "pem", "pep440_rs", "postgres-native-tls 0.5.3", "process-wrap", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 1ce51d9f51..b1762bc291 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -638,7 +638,9 @@ libxml = { version = "=0.3.3" } samael = { git="https://github.com/njaremko/samael", rev="f879f1942ec1b34b6d3027ce7e4724ad95d15dfa", features = ["xmlsec"] } gcp_auth = "0.9.0" rust_decimal = { version = "^1", features = ["db-postgres", "serde-float"]} -jsonwebtoken = "8.3.0" +# aws_lc_rs, not rust_crypto: the RustCrypto backend refuses RSA keys above 4096 bits, and +# external/guest JWT issuers may sign with 8192-bit keys. +jsonwebtoken = { version = "^10.3", features = ["aws_lc_rs"] } pem = "3.0.1" nix = { version = "0.27.1", features = ["process", "signal"] } fs4 = "0.13" diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 66137d09db..c90f32cf6d 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -92a72d522c3934f46f146ac35f565cf4af97928d +5585dae0957838fd7bd2ae8c58059431f2cea4d9 diff --git a/backend/parsers/windmill-parser-wasm/Cargo.lock b/backend/parsers/windmill-parser-wasm/Cargo.lock index fea3a11063..bed1d70cf8 100644 --- a/backend/parsers/windmill-parser-wasm/Cargo.lock +++ b/backend/parsers/windmill-parser-wasm/Cargo.lock @@ -267,6 +267,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ec6fb3fe69024a75fa7e1bfb48aa6cf59706a101658ea01bfd33b2b248a038f" dependencies = [ "aws-lc-sys", + "untrusted 0.7.1", "zeroize", ] @@ -374,12 +375,6 @@ dependencies = [ "windows-link", ] -[[package]] -name = "base64" -version = "0.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e1b586273c5702936fe7b7d6896644d8be71e6314cfe09d3167c95f712589e8" - [[package]] name = "base64" version = "0.21.7" @@ -1337,7 +1332,7 @@ checksum = "da0e4dd2a88388a1f4ccc7c9ce104604dab68d9f408dc34cd45823d5a9069095" dependencies = [ "futures-core", "futures-sink", - "spin 0.9.8", + "spin", ] [[package]] @@ -2186,16 +2181,20 @@ dependencies = [ [[package]] name = "jsonwebtoken" -version = "8.3.0" +version = "10.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6971da4d9c3aa03c3d8f3ff0f4155b534aad021292003895a469716b2a230378" +checksum = "eba32bfb4ffdeaca3e34431072faf01745c9b26d25504aa7a6cf5684334fc4fc" dependencies = [ - "base64 0.21.7", + "aws-lc-rs", + "base64 0.22.1", + "getrandom 0.2.17", + "js-sys", "pem", - "ring 0.16.20", "serde", "serde_json", + "signature", "simple_asn1", + "zeroize", ] [[package]] @@ -2225,7 +2224,7 @@ version = "1.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" dependencies = [ - "spin 0.9.8", + "spin", ] [[package]] @@ -2598,7 +2597,7 @@ dependencies = [ "httparse", "memchr", "mime", - "spin 0.9.8", + "spin", "version_check", ] @@ -2994,11 +2993,12 @@ checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a" [[package]] name = "pem" -version = "1.1.1" +version = "3.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8835c273a76a90455d7344889b0964598e3316e2a79ede8e36f16bdcf2228b8" +checksum = "1d30c53c26bc5b31a98cd02d20f25a7c8567146caf63ed593a9d87b2775291be" dependencies = [ - "base64 0.13.1", + "base64 0.22.1", + "serde_core", ] [[package]] @@ -3403,7 +3403,7 @@ dependencies = [ "getrandom 0.3.4", "lru-slab", "rand 0.9.0", - "ring 0.17.14", + "ring", "rustc-hash 2.1.1", "rustls", "rustls-pki-types", @@ -3755,21 +3755,6 @@ dependencies = [ "rand 0.8.5", ] -[[package]] -name = "ring" -version = "0.16.20" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3053cf52e236a3ed746dfc745aa9cacf1b791d846bdaf412f60a8d7d6e17c8fc" -dependencies = [ - "cc", - "libc", - "once_cell", - "spin 0.5.2", - "untrusted 0.7.1", - "web-sys", - "winapi", -] - [[package]] name = "ring" version = "0.17.14" @@ -3901,7 +3886,7 @@ checksum = "758025cb5fccfd3bc2fd74708fd4682be41d99e5dff73c377c0646c6012c73a4" dependencies = [ "aws-lc-rs", "once_cell", - "ring 0.17.14", + "ring", "rustls-pki-types", "rustls-webpki", "subtle", @@ -3964,7 +3949,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7df23109aa6c1567d1c575b9952556388da57401e4ace1d15f79eedad0d8f53" dependencies = [ "aws-lc-rs", - "ring 0.17.14", + "ring", "rustls-pki-types", "untrusted 0.9.0", ] @@ -4199,15 +4184,16 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.143" +version = "1.0.151" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d401abef1d108fbd9cbaebc3e46611f4b1021f714a0597a71f41ee463f5f4a5a" +checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" dependencies = [ "indexmap", "itoa", "memchr", - "ryu", "serde", + "serde_core", + "zmij", ] [[package]] @@ -4426,12 +4412,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "spin" -version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" - [[package]] name = "spin" version = "0.9.8" @@ -6233,6 +6213,7 @@ dependencies = [ "pep440_rs", "phf 0.11.3", "pin-project-lite", + "pkcs1", "postgres-native-tls", "quick_cache", "rand 0.9.0", @@ -6244,8 +6225,10 @@ dependencies = [ "serde", "serde_json", "serde_yml", + "sha1", "sha2 0.10.9", "size", + "spki", "sqlx", "strum 0.27.2", "strum_macros 0.27.2", @@ -7282,6 +7265,20 @@ name = "zeroize" version = "1.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b97154e67e32c85465826e8bcc1c59429aaaf107c1e4a9e53c8d8ccd5eff88d0" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c50655cbb0fe3fc43170059e702f1ce5e19b84cec58dc87b037a09935c2f328" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] [[package]] name = "zerotrie" @@ -7315,3 +7312,9 @@ dependencies = [ "quote", "syn 2.0.117", ] + +[[package]] +name = "zmij" +version = "1.0.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29666d0abbfad1e3dc4dcf6144730dd3a3ab225bbbdac83319345b1b44ccfc1b" diff --git a/backend/tests/datatable_migration_add_down.rs b/backend/tests/datatable_migration_add_down.rs index 3cd39e0fa3..47ea4c253b 100644 --- a/backend/tests/datatable_migration_add_down.rs +++ b/backend/tests/datatable_migration_add_down.rs @@ -142,3 +142,35 @@ async fn test_add_down_to_an_applied_migration(db: Pool) -> anyhow::Re Ok(()) } + +/// A definition saved before `-- role` annotations were parsed can open with a comment that now +/// reads as a malformed one. It never gets through a role check again, so an admin must still be +/// able to finish it rather than be refused on its annotation. +#[sqlx::test(fixtures("datatable_migrations_grants"))] +async fn test_add_down_to_a_migration_whose_comment_predates_role_annotations( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + setup_applied_migration_without_down(&db).await?; + let code_up = format!("-- Role table for widgets\n{CODE_UP}"); + sqlx::query("UPDATE datatable_migrations SET code_up = $1 WHERE workspace_id = 'dtmig-ws'") + .bind(&code_up) + .execute(&db) + .await?; + + let server = ApiServer::start(db.clone()).await?; + let resp = authed(reqwest::Client::new().post(format!( + "http://localhost:{}/api/w/dtmig-ws/workspaces/upsert_datatable_migration/main", + server.addr.port() + ))) + .json(&json!({ + "timestamp": VERSION, + "name": "create_widgets", + "code_up": code_up, + "code_down": CODE_DOWN, + })) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + Ok(()) +} diff --git a/backend/windmill-ai/src/ai_types.rs b/backend/windmill-ai/src/ai_types.rs index 69c8ac4538..599cf0cca3 100644 --- a/backend/windmill-ai/src/ai_types.rs +++ b/backend/windmill-ai/src/ai_types.rs @@ -210,7 +210,9 @@ pub fn validate_model_pricing_json(ai_config: &serde_json::Value) -> Result<(), return Err(format!("Price override for {} is not an object", key)); }; for field in ["input", "output", "cache_read", "cache_write"] { - let Some(rate) = price.get(field) else { continue }; + let Some(rate) = price.get(field) else { + continue; + }; let rate = rate .as_f64() .filter(|r| r.is_finite() && *r >= 0.0 && *r <= MAX_MODEL_RATE); @@ -223,9 +225,98 @@ pub fn validate_model_pricing_json(ai_config: &serde_json::Value) -> Result<(), } for required in ["input", "output"] { if !price.contains_key(required) { - return Err(format!("Price override for {} is missing {}", key, required)); + return Err(format!( + "Price override for {} is missing {}", + key, required + )); } } } Ok(()) } + +// ============================================================================ +// Per-model token maps +// ============================================================================ + +/// One `provider:model` -> tokens map of the AI config, with the bounds the API +/// documents for it. Both maps are validated the same way, so the bounds live here +/// rather than at each call site. +pub struct TokenMap { + pub field: &'static str, + pub label: &'static str, + pub min: i64, + pub max: i64, +} + +/// The chat compacts its history as it nears the window, so a value below any real +/// model's window would compact every turn; the ceiling is far above any shipped +/// model and only catches typos. +pub const CONTEXT_WINDOWS: TokenMap = TokenMap { + field: "context_window_per_model", + label: "Context window", + min: 1024, + max: 10_000_000, +}; + +/// The completion cap the chat asks the provider for. +pub const OUTPUT_LIMITS: TokenMap = + TokenMap { field: "max_tokens_per_model", label: "Output limit", min: 1, max: 2_000_000 }; + +pub const TOKEN_MAPS: [TokenMap; 2] = [CONTEXT_WINDOWS, OUTPUT_LIMITS]; + +pub fn validate_token_limit(map: &TokenMap, key: &str, tokens: i64) -> Result<(), String> { + if (map.min..=map.max).contains(&tokens) { + Ok(()) + } else { + Err(format!( + "{} for {} must be between {} and {} tokens", + map.label, key, map.min, map.max + )) + } +} + +/// Bound these maps in the untyped instance AI config, for the reason given on +/// `validate_model_pricing_json`. +pub fn validate_token_maps_json(ai_config: &serde_json::Value) -> Result<(), String> { + for map in TOKEN_MAPS.iter() { + let entries = match ai_config.get(map.field) { + None | Some(serde_json::Value::Null) => continue, + Some(v) => v + .as_object() + .ok_or_else(|| format!("{} must be an object", map.field))?, + }; + for (key, tokens) in entries { + let tokens = tokens + .as_i64() + .ok_or_else(|| format!("{} for {} must be an integer", map.label, key))?; + validate_token_limit(map, key, tokens)?; + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn instance_token_maps_are_bounded_integers() { + assert!(validate_token_maps_json(&json!({})).is_ok()); + for map in TOKEN_MAPS.iter() { + let entry = |value| json!({ map.field: { "customai:m": value } }); + assert!(validate_token_maps_json(&entry(json!(map.min))).is_ok()); + assert!(validate_token_maps_json(&entry(json!(map.max))).is_ok()); + for bad in [ + json!(map.min - 1), + json!(map.max + 1), + json!(4096.5), + json!("4096"), + ] { + assert!(validate_token_maps_json(&entry(bad)).is_err()); + } + assert!(validate_token_maps_json(&json!({ map.field: [4096] })).is_err()); + } + } +} diff --git a/backend/windmill-api-integration-tests/tests/datatable_roles.rs b/backend/windmill-api-integration-tests/tests/datatable_roles.rs index d2f6ca325e..4948a0098a 100644 --- a/backend/windmill-api-integration-tests/tests/datatable_roles.rs +++ b/backend/windmill-api-integration-tests/tests/datatable_roles.rs @@ -627,21 +627,234 @@ async fn a_rename_has_to_match_the_save_it_claims_to_describe( Ok(()) } +async fn database_exists(db: &Pool, name: &str) -> anyhow::Result { + Ok( + sqlx::query_scalar("SELECT EXISTS (SELECT 1 FROM pg_database WHERE datname = $1)") + .bind(name) + .fetch_one(db) + .await?, + ) +} + +async fn workspace_exists(db: &Pool, id: &str) -> anyhow::Result { + Ok( + sqlx::query_scalar("SELECT EXISTS (SELECT 1 FROM workspace WHERE id = $1)") + .bind(id) + .fetch_one(db) + .await?, + ) +} + #[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] -async fn a_data_table_under_roles_is_not_copied_into_a_fork( +async fn a_data_table_under_roles_is_cloned_only_with_its_grants( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + // The database a fork's copy of `main` goes into is named after the fork. Every request below + // is refused before it is created, so the cluster-wide name never collides with a sibling run. + let target = "wm_fork_copy__main"; + let fork = |token: &str, forked: Value| { + authed( + client().post(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/create_fork" + )), + token, + ) + .json(&json!({"id": "wm-fork-copy", "name": "copy", "forked_datatables": [forked]})) + }; + + // Rows are a workspace admin's to copy, as they were before roles. + let resp = fork( + "SECRET_TOKEN_2", + json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_and_data"}), + ) + .send() + .await?; + assert_eq!(resp.status(), 403, "{}", resp.text().await?); + assert!(!database_exists(&db, target).await?); + + // Even the schema alone lists every table, which a member covered by no role cannot read in + // the parent. + let resp = fork( + "SECRET_TOKEN_3", + json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_only"}), + ) + .send() + .await?; + assert_eq!(resp.status(), 401, "{}", resp.text().await?); + assert!(!database_exists(&db, target).await?); + + // Refused in the first phase too, before a git branch is created for a fork that cannot be. + let resp = authed( + client().post(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/create_workspace_fork_branch" + )), + "SECRET_TOKEN_3", + ) + .json( + &json!({"id": "wm-fork-copy", "name": "copy", "forked_datatables": [ + {"name": "main", "new_dbname": target, "fork_behavior": "schema_only"} + ]}), + ) + .send() + .await?; + assert_eq!(resp.status(), 401, "{}", resp.text().await?); + + // A database the request did not create — whatever the data table, under roles or not — may be + // a copy left behind by a deleted fork, which the entry would reach without its governance. + let resp = fork( + "SECRET_TOKEN", + json!({"name": "other", "new_dbname": "wm_fork_copy__other"}), + ) + .send() + .await?; + assert_eq!(resp.status(), 400); + assert!( + resp.text().await?.contains("fork_behavior"), + "a database the fork did not create was taken" + ); + assert!(!workspace_exists(&db, "wm-fork-copy").await?); + + // The copy an admin asks for goes ahead in an edition that replays grants, and is refused + // before any database exists in one that does not. The fixture's database does not exist, so + // the dump fails either way — and leaves neither a database nor a fork behind. + let resp = fork( + "SECRET_TOKEN", + json!({"name": "main", "new_dbname": target, "fork_behavior": "schema_only"}), + ) + .send() + .await?; + assert_ne!(resp.status(), 200); + let body = resp.text().await?; + #[cfg(all(feature = "private", feature = "enterprise"))] + assert!(body.contains("pg_dump"), "{body}"); + #[cfg(not(all(feature = "private", feature = "enterprise")))] + assert!(body.contains("Enterprise Edition"), "{body}"); + assert!(!database_exists(&db, target).await?); + assert!(!workspace_exists(&db, "wm-fork-copy").await?); + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_clone_takes_its_roles_from_the_data_table_it_was_cloned_from( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + sqlx::query( + r#"UPDATE workspace_settings SET datatable = jsonb_set(datatable, '{datatables,copy}', '{ + "database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"}, + "governed_by": {"workspace_id": "test-workspace", "datatable": "main"}, + "forked_from": {} + }'::jsonb) WHERE workspace_id = 'wm-fork-dt'"#, + ) + .execute(&db) + .await?; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let fork = format!("http://localhost:{port}/api/w/wm-fork-dt/workspaces"); + + // `test-user-2` administers the fork, and is only a tenant of `analytics` in the parent. + let resp = authed( + client().get(format!("{fork}/datatable_usable_roles/copy")), + "SECRET_TOKEN_2", + ) + .send() + .await?; + let body: Value = resp.json().await?; + assert_eq!(body["roles"], json!(["analytics"]), "{body}"); + + let resp = authed( + client().get(format!("{fork}/datatable_permissions/copy")), + "SECRET_TOKEN_2", + ) + .send() + .await?; + let body: Value = resp.json().await?; + assert_eq!(body["editable"], false, "{body}"); + assert_eq!(body["clone_of"]["workspace_id"], "test-workspace", "{body}"); + + // Nobody changes a clone's roles, a superadmin included: they are the source's. + for token in ["SECRET_TOKEN_2", "SECRET_TOKEN"] { + let resp = authed( + client().post(format!("{fork}/datatable_permissions/copy")), + token, + ) + .json(&json!({"permissioned": false})) + .send() + .await?; + assert_eq!(resp.status(), 400, "{token} changed a clone's roles"); + } + + // A settings save cannot clear the link. + let resp = authed( + client().post(format!("{fork}/edit_datatable_config")), + "SECRET_TOKEN_2", + ) + .json(&json!({ + "settings": {"datatables": {"main": {}, "copy": { + "database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"} + }}}, + "renames": [], "deleted_datatables": [] + })) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + let governed_by: Option = sqlx::query_scalar( + "SELECT datatable->'datatables'->'copy'->'governed_by' FROM workspace_settings + WHERE workspace_id = 'wm-fork-dt'", + ) + .fetch_one(&db) + .await?; + assert_eq!(governed_by.unwrap()["datatable"], "main"); + + // Nor move it, a superadmin included: its grants were replayed into that database alone. + let resp = authed( + client().post(format!("{fork}/edit_datatable_config")), + "SECRET_TOKEN", + ) + .json(&json!({ + "settings": {"datatables": {"main": {}, "copy": { + "database": {"resource_type": "instance", "resource_path": "wm_fork_dt__elsewhere"} + }}}, + "renames": [], "deleted_datatables": [] + })) + .send() + .await?; + assert_eq!(resp.status(), 400, "{}", resp.text().await?); + + // Nor reach its database through a second entry without roles, which would connect as `admin`. + let resp = authed( + client().post(format!( + "http://localhost:{port}/api/w/test-workspace/workspaces/edit_datatable_config" + )), + "SECRET_TOKEN", + ) + .json(&json!({ + "settings": {"datatables": { + "main": {"database": {"resource_type": "instance", "resource_path": "dt_main"}}, + "alias": {"database": {"resource_type": "instance", "resource_path": "wm_fork_dt__copy"}} + }}, + "renames": [], "deleted_datatables": [] + })) + .send() + .await?; + let status = resp.status(); + let body = resp.text().await?; + assert_eq!(status, 400, "{body}"); + assert!(body.contains("without carrying those roles"), "{body}"); + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_data_table_under_roles_is_not_copied_without_its_grants( db: Pool, ) -> anyhow::Result<()> { initialize_tracing().await; - // `pg_dump` carries no roles and the restore drops ACLs, so a copy would arrive with the - // parent's tenants and none of the grants behind them: every role but admin denied by - // Postgres in a data table that reads as configured. Refuse the copy rather than ship that, - // and refuse it before any data moves. let server = ApiServer::start(db.clone()).await?; let port = server.addr.port(); - // Both halves of the clone: the database the copy would land in, then the copy itself. The - // first has to refuse too, or a permissioned fork leaves an empty registered database that - // no data table entry names and nothing collects. let resp = authed( client().post(format!( "http://localhost:{port}/api/w/test-workspace/workspaces/create_pg_database" @@ -1201,6 +1414,158 @@ async fn a_fork_waits_for_a_rename_of_the_data_table_it_keeps( Ok(()) } +async fn enable_migrations_on_main(db: &Pool) -> anyhow::Result<()> { + sqlx::query( + "UPDATE workspace_settings + SET datatable = jsonb_set(datatable, '{datatables,main,migrations_enabled}', 'true') + WHERE workspace_id = 'test-workspace'", + ) + .execute(db) + .await?; + Ok(()) +} + +#[cfg(all(feature = "private", feature = "enterprise"))] +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn a_migration_is_written_only_by_who_may_run_it(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + enable_migrations_on_main(&db).await?; + let server = ApiServer::start(db.clone()).await?; + let base = format!( + "http://localhost:{}/api/w/test-workspace/workspaces", + server.addr.port() + ); + let create = |token: &str, body: Value| { + authed( + client() + .post(format!("{base}/create_datatable_migration/main")) + .json(&body), + token, + ) + .send() + }; + let upsert = |token: &str, code_up: &str| { + authed( + client() + .post(format!("{base}/upsert_datatable_migration/main")) + .json(&json!({"timestamp": 20260101000000i64, "name": "m", "code_up": code_up})), + token, + ) + .send() + }; + + // `test-user-2` holds `analytics` and nothing else, so of a migration they may store only + // what they could run: SQL that declares no role runs as admin, in the up or the down. + for body in [ + json!({"name": "m", "code_up": "DROP TABLE t"}), + json!({"name": "m", "code_up": "-- role analytics\nSELECT 1", "code_down": "DROP TABLE t"}), + ] { + let resp = create("SECRET_TOKEN_2", body.clone()).await?; + assert_eq!(resp.status(), 401, "{body}: {}", resp.text().await?); + } + let resp = create( + "SECRET_TOKEN_2", + json!({"name": "m", "code_up": "-- role analytics\nSELECT 1"}), + ) + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + + // An admin's migration re-pushed as stored writes nothing, so it passes. Replacing it removes + // SQL they could not run, even with SQL they could, so it is refused like deleting it. + let resp = upsert("SECRET_TOKEN", "DROP TABLE t").await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + let resp = upsert("SECRET_TOKEN_2", "DROP TABLE t").await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + for code_up in ["DROP TABLE u", "-- role analytics\nSELECT 1"] { + let resp = upsert("SECRET_TOKEN_2", code_up).await?; + assert_eq!(resp.status(), 401, "{code_up}: {}", resp.text().await?); + } + let resp = authed( + client().delete(format!( + "{base}/delete_datatable_migration/main/20260101000000" + )), + "SECRET_TOKEN_2", + ) + .send() + .await?; + assert_eq!(resp.status(), 401, "{}", resp.text().await?); + + let code_up: String = sqlx::query_scalar( + "SELECT code_up FROM datatable_migrations + WHERE workspace_id = 'test-workspace' AND timestamp = 20260101000000", + ) + .fetch_one(&db) + .await?; + assert_eq!(code_up, "DROP TABLE t"); + Ok(()) +} + +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn an_operator_neither_writes_nor_runs_a_migration(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + enable_migrations_on_main(&db).await?; + // Off roles, where every role check passes everyone, so only the operator guard is left to + // refuse. + sqlx::query( + "UPDATE workspace_settings SET datatable = datatable #- '{datatables,main,permissions}' + WHERE workspace_id = 'test-workspace'", + ) + .execute(&db) + .await?; + // A token of its own: the auth cache is process-wide, and the other tests here authenticate + // `SECRET_TOKEN_3` in this workspace as a non-operator. + sqlx::raw_sql( + "UPDATE usr SET operator = true, role = 'Operator' + WHERE workspace_id = 'test-workspace' AND username = 'test-user-3'; + INSERT INTO token (token_hash, token_prefix, token, email, label, super_admin) + VALUES (encode(sha256('DT_OPERATOR_TOKEN'::bytea), 'hex'), 'DT_OPERATO', + 'DT_OPERATOR_TOKEN', 'test3@windmill.dev', 'operator', false);", + ) + .execute(&db) + .await?; + let server = ApiServer::start(db.clone()).await?; + let base = format!( + "http://localhost:{}/api/w/test-workspace/workspaces", + server.addr.port() + ); + let migration = json!({"timestamp": 20260101000000i64, "name": "m", "code_up": "SELECT 1"}); + + for request in [ + client() + .post(format!("{base}/create_datatable_migration/main")) + .json(&migration), + client() + .post(format!("{base}/upsert_datatable_migration/main")) + .json(&migration), + client().delete(format!( + "{base}/delete_datatable_migration/main/20260101000000" + )), + client().post(format!("{base}/run_datatable_migrations/main")), + client().post(format!("{base}/rollback_datatable_migrations/main")), + client().post(format!("{base}/generate_initial_datatable_migration/main")), + ] { + let resp = authed(request, "DT_OPERATOR_TOKEN").send().await?; + let status = resp.status(); + let text = resp.text().await?; + assert!( + status == 401 && text.contains("Operators cannot"), + "{status}: {text}" + ); + } + + // The same write from a developer is accepted: what refused the operator is theirs alone. + let resp = authed( + client() + .post(format!("{base}/upsert_datatable_migration/main")) + .json(&migration), + "SECRET_TOKEN_2", + ) + .send() + .await?; + assert_eq!(resp.status(), 200, "{}", resp.text().await?); + Ok(()) +} + #[cfg(not(all(feature = "private", feature = "enterprise")))] const ENTERPRISE_REFUSAL: &str = "Data table roles are a Windmill Enterprise Edition feature"; @@ -1321,3 +1686,73 @@ async fn without_the_enterprise_edition_a_data_table_under_roles_is_refused_a_co assert!(err.to_string().contains(ENTERPRISE_REFUSAL), "{err}"); Ok(()) } + +/// A save blocked on an instance database's lock is not a user of it until it commits one: the +/// cleanup for a database whose setup failed lets that save through and reads what it left. +#[sqlx::test(migrations = "../migrations", fixtures("base", "datatable_roles"))] +async fn cleanup_waits_out_a_save_racing_it_for_an_instance_database( + db: Pool, +) -> anyhow::Result<()> { + initialize_tracing().await; + let key = "instance_database:dt_probe"; + + // Queues cleanup behind `holder`, then a save behind cleanup, so cleanup takes the lock with + // the save already waiting on it — the ordering the waiter check is for. + let race = |commit: bool| { + let db = db.clone(); + async move { + let mut holder = db.acquire().await?; + sqlx::query("SELECT pg_advisory_lock(hashtext($1))") + .bind(key) + .execute(&mut *holder) + .await?; + + let cleanup = tokio::spawn({ + let db = db.clone(); + async move { windmill_common::drop_unused_instance_database(&db, "dt_probe").await } + }); + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + + let save = tokio::spawn({ + let db = db.clone(); + async move { + let mut tx = db.begin().await?; + windmill_common::lock_instance_databases(&mut tx, ["dt_probe"]).await?; + sqlx::query( + r#"UPDATE workspace_settings SET datatable = jsonb_set(datatable, + '{datatables,probe}', + '{"database": {"resource_type": "instance", "resource_path": "dt_probe"}}') + WHERE workspace_id = 'test-workspace'"#, + ) + .execute(&mut *tx) + .await?; + if commit { + tx.commit().await?; + } else { + tx.rollback().await?; + } + Ok::<_, anyhow::Error>(()) + } + }); + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + + sqlx::query("SELECT pg_advisory_unlock(hashtext($1))") + .bind(key) + .execute(&mut *holder) + .await?; + save.await??; + Ok::<_, anyhow::Error>(cleanup.await??) + } + }; + + assert!( + matches!(race(false).await?, windmill_common::Cleanup::Dropped), + "a save that rolled back kept the database, whose name then blocks every retry" + ); + let kept = race(true).await?; + let windmill_common::Cleanup::InUse(users) = kept else { + anyhow::bail!("the database was dropped under a save that committed a reference to it") + }; + assert_eq!(users, vec!["test-workspace".to_string()]); + Ok(()) +} diff --git a/backend/windmill-api-settings/src/lib.rs b/backend/windmill-api-settings/src/lib.rs index 9818c3a9c7..53f538861a 100644 --- a/backend/windmill-api-settings/src/lib.rs +++ b/backend/windmill-api-settings/src/lib.rs @@ -968,6 +968,8 @@ async fn run_setting_pre_write_hook( AI_CONFIG_SETTING => { windmill_ai::ai_types::validate_model_pricing_json(value) .map_err(error::Error::BadRequest)?; + windmill_ai::ai_types::validate_token_maps_json(value) + .map_err(error::Error::BadRequest)?; } AUTOMATE_USERNAME_CREATION_SETTING => { if value.as_bool().unwrap_or(false) { @@ -1712,6 +1714,8 @@ struct CustomInstanceDbLogs { replication_user: String, #[serde(skip_serializing_if = "Option::is_none")] replication_user_error: Option, + #[serde(skip_serializing_if = "String::is_empty")] + user_connect: String, } async fn list_custom_instance_pg_databases( @@ -2125,9 +2129,57 @@ async fn setup_custom_instance_pg_database_inner( } } + // Everything above logged in as the DATABASE_URL user, but whatever uses the database logs + // in as custom_instance_user. A proxy that routes on the login name can accept one and + // refuse the other, which would otherwise surface only once a data table first connects. + let user_creds = PgDatabase { + user: Some(windmill_common::datatable_roles::CUSTOM_INSTANCE_USER.to_string()), + password: Some(windmill_common::utils::get_custom_pg_instance_password(db).await?), + ..pg_creds + }; + let (client, connection) = user_creds + .connect(Some(db)) + .await + .map_err(|e| custom_instance_user_connect_error(&user_creds.host, dbname, e))?; + let join_handle = tokio::spawn(async move { connection.await }); + logs.user_connect = "OK".to_string(); + drop(client); // /!\ Drop before joining to avoid deadlock + windmill_common::shutdown_pg_connection(join_handle).await?; + Ok(()) } +fn custom_instance_user_connect_error(host: &str, dbname: &str, e: error::Error) -> error::Error { + let cause = match &e { + error::Error::Anyhow { error, .. } => format!("{error:#}"), + e => e.to_string(), + }; + // Supavisor, Supabase's pooler, reads the tenant to route to from the login + // (`.`), and only knows the logins configured for that tenant. + let lower = cause.to_lowercase(); + let routing_refused = [ + "enoidentifier", + "tenant identifier", + "tenant or user", + "tenant/user", + ] + .iter() + .any(|signature| lower.contains(signature)); + if routing_refused { + error::Error::BadConfig(format!( + "DATABASE_URL reaches Postgres through a connection pooler ({host}) that picks the \ + server to route to from the login name, and it refused custom_instance_user, the \ + role Windmill uses for instance databases ({cause}). Instance databases cannot be \ + used through this pooler: use your own Postgres database instead, or point \ + DATABASE_URL at the Postgres server directly rather than at the pooler." + )) + } else { + error::Error::ExecutionErr(format!( + "Could not connect to {dbname} as custom_instance_user: {cause}" + )) + } +} + async fn drop_custom_instance_pg_database( authed: ApiAuthed, Extension(db): Extension, @@ -2530,6 +2582,34 @@ mod tests { use std::collections::BTreeMap; use windmill_common::instance_config::{GlobalSettings, InstanceConfig, WorkerGroupConfig}; + #[test] + fn supavisor_refusing_custom_instance_user_is_named() { + use windmill_common::error::{to_anyhow, Error}; + let connect_error = |message: &str| { + let e = Error::from(to_anyhow(std::io::Error::other(message.to_string()))); + super::custom_instance_user_connect_error( + "aws-0-eu-west-1.pooler.supabase.com", + "dt", + e, + ) + }; + for supavisor in [ + "db error: FATAL: (ENOIDENTIFIER) no tenant identifier provided", + "db error: FATAL: Tenant or user not found", + ] { + assert!( + matches!(connect_error(supavisor), Error::BadConfig(m) if m.contains("connection pooler")), + "{supavisor}" + ); + } + assert!(matches!( + connect_error( + "db error: FATAL: password authentication failed for user \"custom_instance_user\"" + ), + Error::ExecutionErr(_) + )); + } + #[test] fn instance_config_yaml_round_trip() { let config = InstanceConfig { diff --git a/backend/windmill-api-workspaces/src/datatable_acl.rs b/backend/windmill-api-workspaces/src/datatable_acl.rs index 6bd14316fc..04dc3d4e80 100644 --- a/backend/windmill-api-workspaces/src/datatable_acl.rs +++ b/backend/windmill-api-workspaces/src/datatable_acl.rs @@ -245,6 +245,8 @@ pub struct DatatableAclInfo { /// Whether this caller may plan and apply changes: they administer the data table, on an /// edition that has the planner. pub editable: bool, + /// Whether the data table is a clone, whose grants stay as they were copied from its source. + pub clone: bool, /// Whether the server is Postgres 17 or later, which added the `MAINTAIN` table privilege. pub supports_maintain: bool, /// The database the target lives in, which no target carries itself. @@ -335,13 +337,40 @@ async fn connect_as_admin_unchecked( pg.user = Some(CUSTOM_INSTANCE_USER.to_string()); pg.password = Some(windmill_common::utils::get_custom_pg_instance_password(db).await?); let dbname = pg.dbname.clone(); - let (client, mut connection) = pg.connect(Some(db)).await?; + let (client, notices) = connect_with_notices(db, &pg).await?; + Ok((client, notices, dbname)) +} + +/// A connection to `pg`, and the notices Postgres sends on it — which is where a grant or revoke +/// that changed nothing is reported ([`execute_acl_statements`]). +pub(crate) async fn connect_with_notices( + db: &DB, + pg: &PgDatabase, +) -> Result<(tokio_postgres::Client, mpsc::UnboundedReceiver)> { + let (client, connection) = pg.connect(Some(db)).await?; + Ok(( + client, + drive_with_notices(connection, windmill_common::TokioPgConnection::poll_message), + )) +} + +type PollMessage = + fn( + &mut C, + &mut std::task::Context<'_>, + ) -> std::task::Poll>>; + +/// Drive `connection` in the background with `poll`, forwarding its notices. +pub(crate) fn drive_with_notices( + mut connection: C, + poll: PollMessage, +) -> mpsc::UnboundedReceiver { // Unbounded: the driver must never wait on the receiver, which only drains once the statement // the driver is carrying has completed. let (notices_tx, notices) = mpsc::unbounded_channel(); tokio::spawn(async move { loop { - match std::future::poll_fn(|cx| connection.poll_message(cx)).await { + match std::future::poll_fn(|cx| poll(&mut connection, cx)).await { Some(Ok(AsyncMessage::Notice(notice))) => { let _ = notices_tx.send(notice); } @@ -354,7 +383,41 @@ async fn connect_as_admin_unchecked( } } }); - Ok((client, notices, dbname)) + notices +} + +/// Run `statements` in order on `tx`, failing on the first that errors or that Postgres only warns +/// about. A privilege the connection cannot pass on is a warning to Postgres (`01007` / `01006`), +/// which then carries on having changed nothing; returning drops the transaction, rolling back +/// everything before it. +/// +/// Authorization: none. Runs `statements` as the connection `tx` is on; callers MUST have authorized +/// changing that database's access, and built the statements themselves. +pub(crate) async fn execute_acl_statements( + tx: &tokio_postgres::Transaction<'_>, + notices: &mut mpsc::UnboundedReceiver, + statements: &[String], +) -> Result<()> { + while notices.try_recv().is_ok() {} + for statement in statements { + tx.batch_execute(statement).await.map_err(|e| { + Error::ExecutionErr(format!( + "Failed to run `{statement}`: {}", + pg_error_message(&e) + )) + })?; + while let Ok(notice) = notices.try_recv() { + if *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_GRANTED + || *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_REVOKED + { + return Err(Error::ExecutionErr(format!( + "`{statement}` did not take effect ({}), so nothing was applied", + notice.message() + ))); + } + } + } + Ok(()) } /// An object whose ownership follows the schema's. @@ -399,10 +462,12 @@ macro_rules! schema_owned_objects { AND x.refobjsubid <> 0)))" }; } +#[allow(unused_imports)] +pub(crate) use schema_owned_objects; /// The keyword `ALTER ... OWNER TO` takes for a kind of object, as `pg_identify_object` names the /// kind. A kind missing here is refused rather than skipped, which would leave it behind. -fn owned_keyword(kind: &str) -> Option<&'static str> { +pub(crate) fn owned_keyword(kind: &str) -> Option<&'static str> { Some(match kind { "table" => "TABLE", "view" => "VIEW", @@ -1054,6 +1119,7 @@ async fn get_datatable_acl( owner: role_name_of(&owner), roles, editable, + clone: governing.governor.is_some(), supports_maintain, dbname, grants, @@ -1625,27 +1691,7 @@ async fn apply_datatable_acl( pg_error_message(&e) )) })?; - for statement in &plan.statements { - pg_tx.batch_execute(statement).await.map_err(|e| { - Error::ExecutionErr(format!( - "Failed to run `{statement}`: {}", - pg_error_message(&e) - )) - })?; - // A privilege the connection cannot pass on is only a warning to Postgres, which then - // carries on having changed nothing. Returning drops the transaction, rolling back - // everything before it. - while let Ok(notice) = notices.try_recv() { - if *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_GRANTED - || *notice.code() == SqlState::WARNING_PRIVILEGE_NOT_REVOKED - { - return Err(Error::ExecutionErr(format!( - "`{statement}` did not take effect ({}), so nothing was applied", - notice.message() - ))); - } - } - } + execute_acl_statements(&pg_tx, &mut notices, &plan.statements).await?; // A schema's objects were listed before the transaction opened; one committed since would stay // with its old owner. One created while this transaction is still open can still slip past, as diff --git a/backend/windmill-api-workspaces/src/datatable_clone.rs b/backend/windmill-api-workspaces/src/datatable_clone.rs new file mode 100644 index 0000000000..1c61baff5c --- /dev/null +++ b/backend/windmill-api-workspaces/src/datatable_clone.rs @@ -0,0 +1,572 @@ +/* + * Author: Ruben Fiszel + * Copyright: Windmill Labs, Inc 2022 + * This file and its contents are licensed under the AGPLv3 License. + * Please see the included NOTICE for copyright information and + * LICENSE-AGPL for a copy of the license. + */ + +//! Copying a data table's database for the fork being created. +//! +//! The fork request makes its copies before it writes the fork: each is created, filled and — for a +//! data table under roles — given the source's owners and grants. What each copy was made from +//! stays in the request ([`MadeCopy`]), so the fork's transaction checks it against the source as it +//! is then, and a fork that fails drops the instance copies it made ([`drop_copies_after`]) and names +//! the others, which live on servers of the workspace's own. + +use std::collections::BTreeSet; + +use windmill_api_auth::ApiAuthed; +use windmill_common::datatable_roles::{lock_role_catalog, read_role_catalog_tx}; +use windmill_common::error::{pg_error_message, Error, Result}; +use windmill_common::utils::require_admin; +use windmill_common::worker::CLOUD_HOSTED; +use windmill_common::workspaces::{ + get_datatable_resource_from_db_unchecked, DataTableDatabase, DataTableForkBehavior, + GoverningDatatable, +}; +use windmill_common::{PgDatabase, DB}; + +use crate::datatable_acl::connect_with_notices; +use crate::datatable_permissions::ensure_reaches_governing_datatable; +use crate::workspaces::{ + create_database_on_server, ensure_datatable_is_clonable, pg_dump_database, pg_import_dump, + DumpFile, PgDumpOptions, +}; + +/// A database this request created and filled for one data table of the fork. +pub(crate) struct MadeCopy { + /// The data table's name, in the parent and in the fork. + pub(crate) name: String, + pub(crate) dbname: String, + pub(crate) behavior: DataTableForkBehavior, + /// The database the source resolved to when it was copied. + pub(crate) source_database: DataTableDatabase, + /// Whether the source's owners and grants were replayed into the copy: it was under roles. + pub(crate) replayed: bool, + /// For a resource-backed copy, the resource and variables its connection was resolved from. + pub(crate) connection: Option, +} + +/// What a resource-backed data table's connection is resolved from: its resource, and every +/// resource and variable that one references, as stored — and, for a secret kept in an external +/// backend, the value that backend holds. The connection is resolved from the snapshot itself +/// ([`ConnectionSnapshot::resolve`]), so it is exactly the one these rows describe. +#[derive(PartialEq)] +pub(crate) struct ConnectionSnapshot { + root: String, + resources: std::collections::BTreeMap>, + /// Stored value and whether it is secret. + variables: std::collections::BTreeMap>, + external_secrets: std::collections::BTreeMap, +} + +/// Read the [`ConnectionSnapshot`] of resource `resource_path` in workspace `w_id`. +/// +/// Authorization: none, and it holds secret values. Callers MUST have authorized using that +/// resource, and never disclose the snapshot or what it resolves to. +pub(crate) async fn connection_snapshot( + db: &DB, + conn: &mut sqlx::PgConnection, + w_id: &str, + resource_path: &str, +) -> Result { + let root = resource_path.trim_start_matches("$res:").to_string(); + let mut snapshot = ConnectionSnapshot { + root: root.clone(), + resources: Default::default(), + variables: Default::default(), + external_secrets: Default::default(), + }; + let mut pending = vec![root]; + while let Some(path) = pending.pop() { + if snapshot.resources.contains_key(&path) { + continue; + } + let value: Option = + sqlx::query_scalar("SELECT value FROM resource WHERE workspace_id = $1 AND path = $2") + .bind(w_id) + .bind(&path) + .fetch_optional(&mut *conn) + .await? + .flatten(); + let mut strings = vec![]; + collect_strings(value.as_ref(), &mut strings); + for reference in strings { + if let Some(var) = reference.strip_prefix("$var:") { + if snapshot.variables.contains_key(var) { + continue; + } + let row: Option<(String, bool)> = sqlx::query_as( + "SELECT value, is_secret FROM variable WHERE workspace_id = $1 AND path = $2", + ) + .bind(w_id) + .bind(var) + .fetch_optional(&mut *conn) + .await?; + if let Some((stored, true)) = &row { + if windmill_common::secret_backend::is_external_stored_value(stored) { + let secret = windmill_common::secret_backend::get_secret_value( + db, w_id, var, stored, + ) + .await?; + snapshot.external_secrets.insert(var.to_string(), secret); + } + } + snapshot.variables.insert(var.to_string(), row); + } else if let Some(res) = reference.strip_prefix("$res:") { + pending.push(res.to_string()); + } + } + snapshot.resources.insert(path, value); + } + Ok(snapshot) +} + +impl ConnectionSnapshot { + /// The connection these rows resolve to, as the data table's own resolution would: references + /// substituted, secrets decrypted. + pub(crate) async fn resolve(&self, db: &DB, w_id: &str) -> Result { + let root = self.resource(&self.root)?; + self.substitute(db, w_id, root, 0).await + } + + /// Whether the resource itself holds the connection's fields, which the fork repoints by setting + /// its `dbname`, rather than being a reference to another resource. + pub(crate) fn root_holds_connection(&self) -> bool { + self.resource(&self.root).is_ok_and(|v| v.is_object()) + } + + fn resource(&self, path: &str) -> Result<&serde_json::Value> { + self.resources + .get(path) + .and_then(|v| v.as_ref()) + .ok_or_else(|| Error::NotFound(format!("resource {path} does not exist"))) + } + + fn substitute<'a>( + &'a self, + db: &'a DB, + w_id: &'a str, + value: &'a serde_json::Value, + depth: usize, + ) -> std::pin::Pin> + Send + 'a>> + { + Box::pin(async move { + if depth > 32 { + return Err(Error::BadRequest( + "resource references nest too deeply".to_string(), + )); + } + Ok(match value { + serde_json::Value::Object(map) => { + let mut out = serde_json::Map::new(); + for (key, val) in map { + out.insert(key.clone(), self.substitute(db, w_id, val, depth).await?); + } + serde_json::Value::Object(out) + } + serde_json::Value::Array(items) => { + let mut out = vec![]; + for val in items { + out.push(self.substitute(db, w_id, val, depth).await?); + } + serde_json::Value::Array(out) + } + serde_json::Value::String(s) if s.starts_with("$res:") => { + let path = &s[5..]; + self.substitute(db, w_id, self.resource(path)?, depth + 1) + .await? + } + serde_json::Value::String(s) if s.starts_with("$var:") => { + let path = &s[5..]; + let (stored, secret) = self + .variables + .get(path) + .and_then(|v| v.as_ref()) + .ok_or_else(|| { + Error::NotFound(format!("variable {path} does not exist")) + })?; + serde_json::Value::String(match (secret, self.external_secrets.get(path)) { + (false, _) => stored.clone(), + (true, Some(external)) => external.clone(), + (true, None) => windmill_common::variables::decrypt( + &windmill_common::variables::build_crypt(db, w_id).await?, + stored.clone(), + ) + .map_err(|e| { + Error::internal_err(format!("Error decrypting variable {s}: {e}")) + })?, + }) + } + other => other.clone(), + }) + }) + } +} + +fn collect_strings<'a>(value: Option<&'a serde_json::Value>, out: &mut Vec<&'a str>) { + match value { + Some(serde_json::Value::String(s)) => out.push(s), + Some(serde_json::Value::Array(items)) => { + items.iter().for_each(|v| collect_strings(Some(v), out)) + } + Some(serde_json::Value::Object(map)) => { + map.values().for_each(|v| collect_strings(Some(v), out)) + } + _ => {} + } +} + +/// What a data table of the fork should be copied as. +pub(crate) struct CopyRequest<'a> { + pub(crate) name: &'a str, + pub(crate) dbname: &'a str, + pub(crate) behavior: DataTableForkBehavior, +} + +/// Check that `authed` may copy each data table of `parent_w_id` as asked, before anything is +/// created. +/// +/// Who may copy is what it was before data table roles — anyone for the schema, an admin of the +/// workspace for the rows — narrowed under roles to whoever may connect as one of them: even the +/// schema alone lists every table, which under roles only role holders can read in the parent. +pub(crate) async fn authorize_copies( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + requests: &[CopyRequest<'_>], +) -> Result<()> { + for request in requests { + match request.behavior { + DataTableForkBehavior::KeepOriginal => { + return Err(Error::BadRequest(format!( + "Data table '{}' is kept, which copies nothing", + request.name + ))) + } + DataTableForkBehavior::SchemaOnly => {} + DataTableForkBehavior::SchemaAndData => { + require_admin(authed.is_admin, &authed.username)?; + if *CLOUD_HOSTED { + return Err(Error::BadRequest( + "Cloning schema and data is not available on cloud".to_string(), + )); + } + } + } + windmill_common::validate_dbname(request.dbname)?; + if !request.dbname.starts_with("wm_fork_") { + return Err(Error::BadRequest(format!( + "Forked datatable database name '{}' must start with 'wm_fork_'", + request.dbname + ))); + } + let governing = ensure_datatable_is_clonable(db, parent_w_id, request.name).await?; + ensure_reaches_governing_datatable(db, parent_w_id, request.name, &governing, authed) + .await?; + if governing.datatable.permissions.is_some() { + crate::datatable_replay_oss::ensure_replay()?; + } + } + Ok(()) +} + +/// Make every copy in `requests`, in order, once [`authorize_copies`] allows them all. When one +/// fails, the copies already made are dropped before the error is returned. +pub(crate) async fn make_copies( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + requests: &[CopyRequest<'_>], +) -> Result> { + authorize_copies(db, authed, parent_w_id, requests).await?; + let mut copies = Vec::with_capacity(requests.len()); + for request in requests { + match make_copy(db, authed, parent_w_id, request).await { + Ok(copy) => copies.push(copy), + Err(e) => return Err(drop_copies_after(db, copies, e).await), + } + } + Ok(copies) +} + +/// Drop every copy, returning `error` — with what could not be dropped appended, since nothing +/// else will name those databases again. +pub(crate) async fn drop_copies_after(db: &DB, copies: Vec, error: Error) -> Error { + let mut stranded = Vec::new(); + for copy in copies { + if let Err(e) = drop_copy(db, ©.source_database, ©.dbname).await { + tracing::error!("Could not drop '{}' after a failed fork: {e}", copy.dbname); + stranded.push(format!("'{}' ({e})", copy.dbname)); + } + } + if stranded.is_empty() { + error + } else { + Error::ExecutionErr(format!( + "{error}. These databases created for the fork could not be dropped: {}", + stranded.join(", ") + )) + } +} + +async fn drop_copy(db: &DB, source_database: &DataTableDatabase, dbname: &str) -> Result<()> { + if source_database.resource_type + == windmill_common::workspaces::DataTableCatalogResourceType::Instance + { + match windmill_common::drop_unused_instance_database(db, dbname).await? { + windmill_common::Cleanup::Dropped => Ok(()), + windmill_common::Cleanup::InUse(users) => Err(Error::BadRequest(format!( + "kept, since workspaces {} now use it", + users.join(", ") + ))), + windmill_common::Cleanup::Waiter(pid) => Err(Error::BadRequest(format!( + "kept, since a request (pid {pid}) is still waiting to name it" + ))), + } + } else { + // On a server of the workspace's own, where a resource edited meanwhile can already name it + // and nothing locks such an edit: dropping it could take someone's data. + Err(Error::BadRequest( + "kept on its PostgreSQL server, where it may already be in use; drop it there once it \ + is not, before forking the same data table under this id again" + .to_string(), + )) + } +} + +async fn make_copy( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + request: &CopyRequest<'_>, +) -> Result { + let governing = ensure_datatable_is_clonable(db, parent_w_id, request.name).await?; + let source_database = governing.datatable.database.clone().ok_or_else(|| { + Error::internal_err(format!( + "Data table '{}' resolves to an entry that owns no database", + request.name + )) + })?; + let is_instance = governing.is_instance(); + // Resolved from the snapshot, not read again: the connection the copy is made on is then + // exactly what these rows describe, which the fork's own clone of them is checked against. + let (server, connection): (PgDatabase, Option) = if is_instance { + let server = serde_json::from_value( + get_datatable_resource_from_db_unchecked(db, parent_w_id, request.name).await?, + ) + .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {e}")))?; + (server, None) + } else { + let snapshot = connection_snapshot( + db, + &mut *db.acquire().await?, + &governing.workspace_id, + &source_database.resource_path, + ) + .await?; + if !snapshot.root_holds_connection() { + return Err(Error::BadRequest(format!( + "Data table '{}' uses resource '{}', which only refers to another resource: the \ + fork's copy of it could not be pointed at the copied database. Point the data \ + table at the resource holding the connection, then fork again.", + request.name, source_database.resource_path + ))); + } + let server = serde_json::from_value(snapshot.resolve(db, &governing.workspace_id).await?) + .map_err(|e| { + Error::internal_err(format!("Failed to parse database credentials: {e}")) + })?; + (server, Some(snapshot)) + }; + + // Dumped before the database exists, so a source that cannot be read leaves nothing behind. + // Ownership never carries over: the restore runs as the target's connection user. Grants do, + // except on the instance, where the replay below is what reproduces them. + let dump = pg_dump_database( + &server, + PgDumpOptions { + schema_only: request.behavior == DataTableForkBehavior::SchemaOnly, + no_owner: true, + no_acl: is_instance, + ..Default::default() + }, + ) + .await?; + + if is_instance { + windmill_common::create_custom_instance_database( + db, + request.dbname, + "datatable", + Some(parent_w_id), + ) + .await?; + } else { + create_database_on_server(db, &server, request.dbname).await?; + } + + let target = PgDatabase { dbname: request.dbname.to_string(), ..server.clone() }; + let filled = fill( + db, + authed, + parent_w_id, + request.name, + &governing, + &server, + &target, + &dump, + ) + .await; + match filled { + Ok(replayed) => Ok(MadeCopy { + name: request.name.to_string(), + dbname: request.dbname.to_string(), + behavior: request.behavior, + source_database, + replayed, + connection, + }), + Err(e) => { + if let Err(drop_err) = drop_copy(db, &source_database, request.dbname).await { + tracing::error!( + "Could not drop '{}' after a failed copy of data table '{}': {drop_err}", + request.dbname, + request.name + ); + return Err(Error::ExecutionErr(format!( + "{e}. The database '{}' created for the copy could not be dropped: {drop_err}", + request.dbname + ))); + } + Err(e) + } + } +} + +/// Restore the dump into the new database and, for a data table under roles, replay the source's +/// owners and grants into it. Returns whether it replayed. +#[allow(clippy::too_many_arguments)] +async fn fill( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + name: &str, + governing: &GoverningDatatable, + source: &PgDatabase, + target: &PgDatabase, + dump: &DumpFile, +) -> Result { + pg_import_dump(target, dump).await?; + + // Held until the replay commits: a role renamed or dropped meanwhile would change what the + // replay names, and a settings save could move the source onto another database or put it under + // roles. Taken in the same order as the permissions save and the ACL apply. + // On a connection of its own: the checks below take theirs from the pool, and a lock holder + // drawn from the pool too could leave a small one with nothing to give them. + let database_url = windmill_common::get_database_url().await?; + let mut lock_holder = ::connect_with( + &database_url.connect_options().await?, + ) + .await + .map_err(|e| Error::internal_err(format!("Failed to connect to the database: {e}")))?; + let mut tx = sqlx::Connection::begin(&mut lock_holder).await?; + lock_role_catalog(&mut tx).await?; + lock_settings_rows(&mut tx, parent_w_id, name).await?; + + // Everything so far was decided before the locks. + let now = ensure_datatable_is_clonable(db, parent_w_id, name).await?; + ensure_reaches_governing_datatable(db, parent_w_id, name, &now, authed).await?; + if !same_database(&now.datatable.database, &governing.datatable.database) { + return Err(Error::BadRequest(format!( + "Data table '{name}' moved to another database while it was being copied; fork again" + ))); + } + + let replayed = now.datatable.permissions.is_some(); + if replayed { + crate::datatable_replay_oss::ensure_replay()?; + let catalog = read_role_catalog_tx(&mut tx).await?; + // The replay leaves `CONNECT` to the catalog, and creating the database only tried to set + // it: a copy `PUBLIC` could still connect to would admit logins the source turns away. + windmill_common::datatable_roles::converge_connect_grants_with( + db, + &target.dbname, + &catalog, + ) + .await?; + let catalog_roles: BTreeSet = catalog.values().map(|r| r.name.clone()).collect(); + let (source, _source_notices) = connect_with_notices(db, source).await?; + let (mut target, mut notices) = connect_with_notices(db, target).await?; + // One transaction on the copy: a replay that stops halfway leaves objects owned by one role + // and granted as another. + let pg_tx = target.transaction().await.map_err(|e| { + Error::internal_err(format!( + "Failed to open a transaction on the copy: {}", + pg_error_message(&e) + )) + })?; + crate::datatable_replay_oss::replay_owners_and_grants( + &source, + &pg_tx, + &mut notices, + &catalog_roles, + ) + .await?; + pg_tx.commit().await.map_err(|e| { + Error::internal_err(format!( + "Failed to commit the replayed grants: {}", + pg_error_message(&e) + )) + })?; + } + tx.commit().await?; + Ok(replayed) +} + +/// Lock every settings row the resolution of data table `name` of `start_w_id` passes through, in +/// the order it passes them — each pointer, the entry that owns the database, and each workspace its +/// roles come from — and return them. Deleting or repointing any of them would leave a copy made +/// for the resolution answering for another one. +pub(crate) async fn lock_settings_rows( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + start_w_id: &str, + name: &str, +) -> Result> { + let mut path: Vec<(String, String)> = vec![]; + let (mut w_id, mut datatable) = (start_w_id.to_string(), name.to_string()); + loop { + if path.contains(&(w_id.clone(), datatable.clone())) || path.len() > 32 { + return Err(Error::BadRequest(format!( + "Data table '{name}' of workspace '{start_w_id}' resolves through a cycle" + ))); + } + let entry: Option = sqlx::query_scalar( + "SELECT datatable->'datatables'->$2 FROM workspace_settings + WHERE workspace_id = $1 FOR UPDATE", + ) + .bind(&w_id) + .bind(&datatable) + .fetch_optional(&mut **tx) + .await? + .flatten(); + path.push((w_id.clone(), datatable.clone())); + let entry: Option = + entry.and_then(|e| serde_json::from_value(e).ok()); + match entry.and_then(|e| e.reference.or(e.governed_by)) { + Some(next) => (w_id, datatable) = (next.workspace_id, next.datatable), + None => break, + } + } + Ok(path.into_iter().map(|(w_id, _)| w_id).collect()) +} + +pub(crate) fn same_database(a: &Option, b: &Option) -> bool { + match (a, b) { + (Some(a), Some(b)) => { + a.resource_type == b.resource_type && a.resource_path == b.resource_path + } + _ => false, + } +} diff --git a/backend/windmill-api-workspaces/src/datatable_migrations.rs b/backend/windmill-api-workspaces/src/datatable_migrations.rs index 7ce6d17e07..f762fda175 100644 --- a/backend/windmill-api-workspaces/src/datatable_migrations.rs +++ b/backend/windmill-api-workspaces/src/datatable_migrations.rs @@ -107,26 +107,63 @@ async fn ensure_migration_role_allowed( datatable_name: &str, authed: &ApiAuthed, sql: &str, - timestamp: i64, - name: &str, + migration: &str, ) -> Result<()> { - let context = format!("Migration {timestamp} ({name})"); let access = DatatableAccess::Authed(authed.to_authed_ref()); match SqlAnnotations::datatable_role(sql)? { Some(role) => { - ensure_can_use_datatable_role(db, w_id, datatable_name, Some(&role), &access, &context) + ensure_can_use_datatable_role(db, w_id, datatable_name, Some(&role), &access, migration) .await } None => ensure_datatable_admin_access(db, w_id, datatable_name, &access) .await .map_err(|e| { Error::NotAuthorized(format!( - "{context} declares no role, so it would run as admin. {e}" + "{migration} declares no role, so it would run as admin. {e}" )) }), } } +/// Refuse writing or deleting a migration definition unless this caller may run both its up and its +/// down. A definition is run later by whoever applies or rolls back pending migrations, as the role +/// it declares, so gating only the run would let anyone plant SQL for a data table's admin to run. +async fn ensure_may_edit_migration( + db: &DB, + w_id: &str, + datatable_name: &str, + authed: &ApiAuthed, + migration: &str, + code_up: &str, + code_down: Option<&str>, +) -> Result<()> { + // Admin access reaches everything a role can, so it edits any definition without reading its + // annotation, which one saved before annotations were parsed may no longer pass. A refusal + // there would leave such a definition impossible to delete or fix. + let access = DatatableAccess::Authed(authed.to_authed_ref()); + if ensure_datatable_admin_access(db, w_id, datatable_name, &access) + .await + .is_ok() + { + return Ok(()); + } + for sql in std::iter::once(code_up).chain(code_down) { + ensure_migration_role_allowed(db, w_id, datatable_name, authed, sql, migration).await?; + } + Ok(()) +} + +/// Operators run deployed code only, and a migration is SQL written or run by hand. Checked before +/// anything else, including the role checks: on a data table not under roles those pass everyone. +fn ensure_not_operator(authed: &ApiAuthed) -> Result<()> { + if authed.is_operator { + return Err(Error::NotAuthorized( + "Operators cannot manage or run data table migrations".to_string(), + )); + } + Ok(()) +} + #[derive(Serialize)] struct AppliedMigration { version: i64, @@ -436,6 +473,7 @@ async fn run_datatable_migrations( Path((w_id, datatable_name)): Path<(String, String)>, Query(query): Query, ) -> JsonResult { + ensure_not_operator(&authed)?; // Before the admin connection is opened at all: the bookkeeping below is created and read // through it, so a caller no role covers must be refused here rather than after the fact. crate::datatable_permissions::ensure_reaches_datatable(&db, &w_id, &datatable_name, &authed) @@ -503,8 +541,7 @@ async fn run_datatable_migrations( &datatable_name, &authed, &m.code_up, - m.timestamp, - &m.name, + &format!("Migration {} ({})", m.timestamp, m.name), ) .await?; run_datatable_migration_job(&db, &user_db, &authed, &w_id, &database_arg, &m.code_up) @@ -573,6 +610,7 @@ async fn rollback_datatable_migrations( Path((w_id, datatable_name)): Path<(String, String)>, Query(query): Query, ) -> JsonResult { + ensure_not_operator(&authed)?; // Before the admin connection is opened at all: the bookkeeping below is created and read // through it, so a caller no role covers must be refused here rather than after the fact. crate::datatable_permissions::ensure_reaches_datatable(&db, &w_id, &datatable_name, &authed) @@ -666,8 +704,7 @@ async fn rollback_datatable_migrations( &datatable_name, &authed, &code_down, - version, - &definition.name, + &format!("Migration {} ({})", version, definition.name), ) .await?; @@ -1213,9 +1250,20 @@ async fn create_datatable_migration( Path((w_id, datatable_name)): Path<(String, String)>, Json(payload): Json, ) -> JsonResult { + ensure_not_operator(&authed)?; validate_datatable_path_segment(&datatable_name)?; validate_migration_name(&payload.name)?; ensure_datatable_migrations_enabled(&db, &w_id, &datatable_name).await?; + ensure_may_edit_migration( + &db, + &w_id, + &datatable_name, + &authed, + &format!("Migration '{}'", payload.name), + &payload.code_up, + payload.code_down.as_deref(), + ) + .await?; let mut tx = db.begin().await?; let timestamp = insert_datatable_migration_def( @@ -1267,6 +1315,34 @@ async fn delete_datatable_migration( Extension(db): Extension, Path((w_id, datatable_name, timestamp)): Path<(String, String, i64)>, ) -> Result { + ensure_not_operator(&authed)?; + let deleted_message = format!("Deleted migration {} from {}", timestamp, datatable_name); + + // A definition already gone is not an error: `wmill sync push` deletes whatever the repo + // dropped, which may no longer exist here. + let Some(definition) = sqlx::query!( + "SELECT name, code_up, code_down FROM datatable_migrations \ + WHERE workspace_id = $1 AND datatable = $2 AND timestamp = $3", + &w_id, + &datatable_name, + timestamp, + ) + .fetch_optional(&db) + .await? + else { + return Ok(deleted_message); + }; + ensure_may_edit_migration( + &db, + &w_id, + &datatable_name, + &authed, + &format!("Migration {} ({})", timestamp, definition.name), + &definition.code_up, + definition.code_down.as_deref(), + ) + .await?; + // Hold the run-serialization lock across the applied-check and the delete: a // run snapshots a migration's SQL before recording its version, so an // unserialized delete could race it and leave `_wm_migrations` pointing at a @@ -1294,19 +1370,46 @@ async fn delete_datatable_migration( ))); } + // Delete only the SQL the role check above cleared: it read the definition without the lock, + // and a rewrite that landed since was never checked. let deleted_name = sqlx::query_scalar!( "DELETE FROM datatable_migrations \ WHERE workspace_id = $1 AND datatable = $2 AND timestamp = $3 \ + AND code_up = $4 AND code_down IS NOT DISTINCT FROM $5 \ RETURNING name", &w_id, &datatable_name, timestamp, + &definition.code_up, + definition.code_down.as_deref(), ) .fetch_optional(&db) .await?; // The definition is removed; runs may resume (audit/deploy metadata below // don't need the lock). drop(lock_client); + let Some(name) = deleted_name else { + // Nothing matched either because another delete got there first, which is this one's + // outcome too, or because the definition was rewritten. + let still_defined = sqlx::query_scalar!( + "SELECT EXISTS(SELECT 1 FROM datatable_migrations \ + WHERE workspace_id = $1 AND datatable = $2 AND timestamp = $3)", + &w_id, + &datatable_name, + timestamp, + ) + .fetch_one(&db) + .await? + .unwrap_or(false); + if !still_defined { + return Ok(deleted_message); + } + return Err(Error::BadRequest(format!( + "Migration {} on data table '{}' changed while it was being deleted. Reload it and \ + retry.", + timestamp, datatable_name + ))); + }; audit_log( &db, @@ -1319,23 +1422,10 @@ async fn delete_datatable_migration( ) .await?; - // Only tally a change if a migration was actually deleted. - if let Some(name) = deleted_name { - record_datatable_migration_deployment( - &authed, - &db, - &w_id, - &datatable_name, - timestamp, - &name, - ) + record_datatable_migration_deployment(&authed, &db, &w_id, &datatable_name, timestamp, &name) .await?; - } - Ok(format!( - "Deleted migration {} from {}", - timestamp, datatable_name - )) + Ok(deleted_message) } #[derive(Deserialize)] @@ -1356,6 +1446,7 @@ async fn upsert_datatable_migration( Path((w_id, datatable_name)): Path<(String, String)>, Json(payload): Json, ) -> Result { + ensure_not_operator(&authed)?; validate_datatable_path_segment(&datatable_name)?; validate_migration_name(&payload.name)?; ensure_datatable_migrations_enabled(&db, &w_id, &datatable_name).await?; @@ -1381,9 +1472,9 @@ async fn upsert_datatable_migration( // run-serialization lock across the applied-check and the write below so an // in-flight run can't record a version for the SQL we're about to overwrite. // Held until the end of the handler (well past the write). The exempt - // upserts need no lock: a new or unchanged one overwrites nothing, and one - // that only adds a down leaves the `code_up` a concurrent run is recording - // a version for untouched. + // upserts need no lock: a new one overwrites nothing, and one that only adds + // a down leaves the `code_up` a concurrent run is recording a version for + // untouched. let only_adds_down = existing.as_ref().is_some_and(|existing| { existing.name == payload.name && existing.code_up == payload.code_up @@ -1395,8 +1486,40 @@ async fn upsert_datatable_migration( && existing.code_up == payload.code_up && existing.code_down == payload.code_down }); + let upserted_message = format!( + "Upserted migration {} in {}", + payload.timestamp, datatable_name + ); + // A re-push of the definition as stored changes nothing, so it is not checked, written, audited, + // synced or counted. Writing it anyway would re-create a definition deleted since the read above. + if unchanged { + return Ok(upserted_message); + } + ensure_may_edit_migration( + &db, + &w_id, + &datatable_name, + &authed, + &format!("Migration {} ({})", payload.timestamp, payload.name), + &payload.code_up, + payload.code_down.as_deref(), + ) + .await?; + // Replacing a definition removes the one stored, so it answers to the same check as a delete. + if let Some(existing) = existing.as_ref() { + ensure_may_edit_migration( + &db, + &w_id, + &datatable_name, + &authed, + &format!("Migration {} ({})", payload.timestamp, existing.name), + &existing.code_up, + existing.code_down.as_deref(), + ) + .await?; + } let _run_lock = match existing.as_ref() { - Some(_) if !only_adds_down && !unchanged => { + Some(_) if !only_adds_down => { // Fail closed: if we can't lock/read the applied set (e.g. the // data-table database is temporarily unreachable), refuse the change // rather than risk overwriting a migration that has already run. @@ -1426,31 +1549,27 @@ async fn upsert_datatable_migration( _ => None, }; - // An exempt upsert judged the row from an unlocked read and then writes - // without the lock, so that whole read is re-tested here, where `ON CONFLICT - // DO UPDATE` re-reads the row under a row lock. Otherwise a request working - // from a stale definition silently reverts whatever changed in between — a - // second addition's down, or a locked rewrite of the up whose new SQL a run - // may already have recorded a version for. Reading no row at all is part of - // the premise: the equality is NULL when `$8` is, so a version created in the - // meantime is refused rather than overwritten. - let recheck_observed = _run_lock.is_none(); + // The role checks, and the exemptions from the lock, all judged the row from the unlocked read + // above, so that whole read is re-tested here, where `ON CONFLICT DO UPDATE` re-reads the row + // under a row lock. Otherwise a request working from a stale definition silently replaces + // whatever changed in between: SQL the role checks never saw, or a locked rewrite of the up + // whose new SQL a run may already have recorded a version for. Reading no row at all is part of + // the premise: the equality is NULL when `$7` is, so a version created in the meantime is + // refused rather than overwritten. let written = sqlx::query!( "INSERT INTO datatable_migrations (workspace_id, datatable, timestamp, name, code_up, code_down) \ VALUES ($1, $2, $3, $4, $5, $6) \ ON CONFLICT (workspace_id, datatable, timestamp) DO UPDATE \ SET name = EXCLUDED.name, code_up = EXCLUDED.code_up, code_down = EXCLUDED.code_down \ - WHERE NOT $7 \ - OR (datatable_migrations.name = $8::text \ - AND datatable_migrations.code_up = $9::text \ - AND datatable_migrations.code_down IS NOT DISTINCT FROM $10::text)", + WHERE datatable_migrations.name = $7::text \ + AND datatable_migrations.code_up = $8::text \ + AND datatable_migrations.code_down IS NOT DISTINCT FROM $9::text", &w_id, &datatable_name, payload.timestamp, &payload.name, &payload.code_up, payload.code_down.as_deref(), - recheck_observed, existing.as_ref().map(|e| e.name.as_str()), existing.as_ref().map(|e| e.code_up.as_str()), existing.as_ref().and_then(|e| e.code_down.as_deref()), @@ -1489,24 +1608,17 @@ async fn upsert_datatable_migration( ) .await?; - // An unchanged re-push is not counted: `wmill sync push` sends every migration - // on every sync, so counting those would swamp the definitions people write. - if !unchanged { - windmill_common::feature_usage::log_feature_usage( - "datatable", - "migration_created", - if existing.is_none() { - "synced" - } else { - "edited" - }, - ); - } + windmill_common::feature_usage::log_feature_usage( + "datatable", + "migration_created", + if existing.is_none() { + "synced" + } else { + "edited" + }, + ); - Ok(format!( - "Upserted migration {} in {}", - payload.timestamp, datatable_name - )) + Ok(upserted_message) } /// Generate the first migration for a data table by snapshotting its current @@ -1519,6 +1631,7 @@ async fn generate_initial_datatable_migration( Extension(db): Extension, Path((w_id, datatable_name)): Path<(String, String)>, ) -> JsonResult { + ensure_not_operator(&authed)?; // Returns a `pg_dump` of the whole schema and writes into the data table's own bookkeeping, so // it answers to the workspace that governs it rather than to whoever is asking. ensure_datatable_admin_access( diff --git a/backend/windmill-api-workspaces/src/datatable_replay_oss.rs b/backend/windmill-api-workspaces/src/datatable_replay_oss.rs new file mode 100644 index 0000000000..edcf8e402b --- /dev/null +++ b/backend/windmill-api-workspaces/src/datatable_replay_oss.rs @@ -0,0 +1,48 @@ +/* + * Author: Ruben Fiszel + * Copyright: Windmill Labs, Inc 2022 + * This file and its contents are licensed under the AGPLv3 License. + * Please see the included NOTICE for copyright information and + * LICENSE-AGPL for a copy of the license. + */ + +//! Where the replay of a data table's owners and grants into its copy comes from: the enterprise +//! one, or a refusal. Without it a data table under roles is not copied at all: its rows would +//! arrive owned by the admin connection with no grant for any role. + +#[cfg(all(feature = "private", feature = "enterprise"))] +pub(crate) use crate::datatable_replay_ee::replay_owners_and_grants; + +#[cfg(all(feature = "private", feature = "enterprise"))] +pub(crate) fn ensure_replay() -> windmill_common::error::Result<()> { + Ok(()) +} + +#[cfg(not(all(feature = "private", feature = "enterprise")))] +use { + std::collections::BTreeSet, + tokio::sync::mpsc, + tokio_postgres::error::DbError, + windmill_common::error::{Error, Result}, +}; + +/// Checked before anything is created. +#[cfg(not(all(feature = "private", feature = "enterprise")))] +pub(crate) fn ensure_replay() -> Result<()> { + Err(Error::BadRequest( + "Cloning a data table under roles is a Windmill Enterprise Edition feature: the copy \ + needs the source's owners and grants replayed. Fork it keeping the original database \ + instead." + .to_string(), + )) +} + +#[cfg(not(all(feature = "private", feature = "enterprise")))] +pub(crate) async fn replay_owners_and_grants( + _source: &tokio_postgres::Client, + _target: &tokio_postgres::Transaction<'_>, + _notices: &mut mpsc::UnboundedReceiver, + _catalog_roles: &BTreeSet, +) -> Result<()> { + ensure_replay() +} diff --git a/backend/windmill-api-workspaces/src/lib.rs b/backend/windmill-api-workspaces/src/lib.rs index 5dfdece644..e7da2b8496 100644 --- a/backend/windmill-api-workspaces/src/lib.rs +++ b/backend/windmill-api-workspaces/src/lib.rs @@ -3,9 +3,11 @@ pub mod ai_session_backups; pub mod data_metrics; pub mod datatable_acl; pub mod datatable_acl_oss; +pub mod datatable_clone; pub mod datatable_migrations; pub mod datatable_permissions; pub mod datatable_permissions_oss; +pub mod datatable_replay_oss; pub mod deployment_requests; pub mod workspaces; pub mod workspaces_extra; @@ -16,6 +18,8 @@ pub mod workspaces_ee; #[cfg(all(feature = "private", feature = "enterprise"))] pub mod datatable_acl_ee; +#[cfg(all(feature = "private", feature = "enterprise"))] +pub mod datatable_replay_ee; #[cfg(all(feature = "private", feature = "enterprise"))] pub mod datatable_permissions_ee; diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index b5ae865df6..d764f72948 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -501,8 +501,8 @@ struct CreateWorkspaceFork { id: String, name: String, color: Option, - /// Datatable names that were forked. For each, the backend will update the - /// forked workspace's datatable config to point to the new database. + /// Data tables the fork gets a copy of rather than a pointer at the parent's. For each, the + /// fork's entry is pointed at the new database. #[serde(default)] forked_datatables: Vec, /// Lakes the user explicitly chose to SHARE with the parent (the fork then reads and @@ -534,6 +534,11 @@ struct CreateWorkspaceFork { struct ForkedDatatableInfo { name: String, new_dbname: String, + /// What this request copies into `new_dbname`, which it creates. Optional only so that the + /// entry older CLIs send — naming a database they created and filled themselves — is refused + /// with a message rather than a parse error. + #[serde(default)] + fork_behavior: Option, } #[derive(Deserialize)] @@ -2233,8 +2238,8 @@ async fn list_datatables( name, resource_type: database.resource_type.as_ref().to_string(), resource_path: database.resource_path.clone(), - governing_workspace_id: (governing.workspace_id != w_id) - .then(|| governing.workspace_id.clone()), + governing_workspace_id: (governing.governing_workspace_id() != w_id) + .then(|| governing.governing_workspace_id().to_string()), permissioned: governing.datatable.permissions.is_some(), }); } @@ -2916,6 +2921,18 @@ fn truncate_column_default(default: String) -> String { mod tests { use super::*; + #[test] + fn a_dev_workspace_copies_into_the_fork_database_namespace() { + assert_eq!( + forked_datatable_dbname("wm-fork-my-fork", "main"), + "wm_fork_my_fork__main" + ); + assert_eq!( + forked_datatable_dbname("my-dev", "main"), + "wm_fork_my_dev__main" + ); + } + /// The header of a pg_dump, followed by an object whose body also holds a `SET`. const DUMP: &str = "--\n\ -- PostgreSQL database dump\n\ @@ -3043,6 +3060,25 @@ pub(crate) async fn resolve_pg_source_checked( w_id: &str, source: &str, ) -> Result { + Ok( + resolve_pg_source_checked_with_kind(db, user_db, authed, w_id, source) + .await? + .0, + ) +} + +/// [`resolve_pg_source_checked`], also reporting the kind of Windmill-managed database behind the +/// source, `None` for a user resource. Callers that guard a managed connection MUST take the kind +/// from here rather than ask separately: between two reads a save can flip the entry, leaving the guards of one kind +/// applied to the connection of the other. +pub(crate) async fn resolve_pg_source_checked_with_kind( + db: &DB, + user_db: &UserDB, + authed: &ApiAuthed, + w_id: &str, + source: &str, +) -> Result<(PgDatabase, Option)> { + let mut managed_kind = None; let db_resource = if let Some(name) = source.strip_prefix("datatable://") { windmill_common::workspaces::ensure_datatable_admin_access( db, @@ -3051,7 +3087,13 @@ pub(crate) async fn resolve_pg_source_checked( &DatatableAccess::Authed(authed.to_authed_ref()), ) .await?; - get_datatable_resource_from_db_unchecked(db, w_id, name).await? + let (connection, kind) = + windmill_common::workspaces::get_datatable_connection_and_kind_unchecked( + db, w_id, name, + ) + .await?; + managed_kind = kind; + connection } else if let Some(path) = source.strip_prefix("$res:") { let db_with_authed = windmill_common::db::DbWithOptAuthed::from_authed( authed, @@ -3084,8 +3126,9 @@ pub(crate) async fn resolve_pg_source_checked( ))); }; - serde_json::from_value(db_resource) - .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e))) + let pg: PgDatabase = serde_json::from_value(db_resource) + .map_err(|e| Error::internal_err(format!("Failed to parse database credentials: {}", e)))?; + Ok((pg, managed_kind)) } /// The kind of the database backing the data table `name` when Windmill manages it (on its own @@ -3484,7 +3527,10 @@ async fn server_setting_names(pg_db: &PgDatabase) -> Result> { /// so a dump that breaks partway through imports partially and reads as a success. /// ON_ERROR_STOP surfaces the failure and --single-transaction makes the restore /// all-or-nothing, leaving the target as it was and the import retryable. -async fn pg_import_dump(target_db: &PgDatabase, dump_file: &DumpFile) -> Result<()> { +/// +/// Authorization: none. Writes into `target_db` with the credentials it carries; callers MUST have +/// authorized writing to that database. +pub(crate) async fn pg_import_dump(target_db: &PgDatabase, dump_file: &DumpFile) -> Result<()> { let supported_settings = server_setting_names(target_db).await?; comment_out_unsupported_settings(dump_file, &supported_settings).await?; @@ -3534,7 +3580,7 @@ async fn create_pg_database( // exists rather than leaving an empty registered `wm_fork_…` behind. if let Some(reference) = req.source.strip_prefix("datatable://") { let (name, _) = parse_datatable_ref_for(&db, &w_id, reference).await?; - ensure_datatable_is_clonable(&db, &w_id, &name).await?; + ensure_copied_without_roles(&ensure_datatable_is_clonable(&db, &w_id, &name).await?)?; } // Non-superadmin: restrict dbname to wm_fork_ prefix @@ -3582,50 +3628,62 @@ async fn create_pg_database( } else { let source_pg = resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.source).await?; - let (client, connection) = source_pg.connect(Some(&db)).await?; - let join_handle = tokio::spawn(async move { connection.await }); - - let row = client - .query_one( - "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_database WHERE datname = $1)", - &[&req.target_dbname], - ) - .await - .map_err(|e| { - Error::internal_err(format!( - "Failed to check database existence: {}", - pg_error_message(&e) - )) - })?; - let db_exists: bool = row.get(0); - - if db_exists { - drop(client); - let _ = windmill_common::shutdown_pg_connection(join_handle).await; - return Err(Error::BadRequest(format!( - "Database '{}' already exists on the resource server", - req.target_dbname - ))); - } - - client - .execute(&format!("CREATE DATABASE \"{}\"", &req.target_dbname), &[]) - .await - .map_err(|e| { - Error::internal_err(format!( - "Failed to create database '{}': {}", - req.target_dbname, - pg_error_message(&e) - )) - })?; - - drop(client); - windmill_common::shutdown_pg_connection(join_handle).await?; + create_database_on_server(&db, &source_pg, &req.target_dbname).await?; } Ok(format!("Created database '{}'", req.target_dbname)) } +/// `CREATE DATABASE` on the server `server` connects to, refusing a name already taken there. +/// +/// Authorization: none. Callers MUST have authorized creating a database on that server — resolved +/// from a data table or resource the caller may administer. +pub(crate) async fn create_database_on_server( + db: &DB, + server: &PgDatabase, + dbname: &str, +) -> Result<()> { + windmill_common::validate_dbname(dbname)?; + let (client, connection) = server.connect(Some(db)).await?; + let join_handle = tokio::spawn(async move { connection.await }); + + let row = client + .query_one( + "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_database WHERE datname = $1)", + &[&dbname], + ) + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to check database existence: {}", + pg_error_message(&e) + )) + })?; + let db_exists: bool = row.get(0); + + if db_exists { + drop(client); + let _ = windmill_common::shutdown_pg_connection(join_handle).await; + return Err(Error::BadRequest(format!( + "Database '{dbname}' already exists on the resource server" + ))); + } + + client + .execute(&format!("CREATE DATABASE \"{dbname}\""), &[]) + .await + .map_err(|e| { + Error::internal_err(format!( + "Failed to create database '{dbname}': {}", + pg_error_message(&e) + )) + })?; + + drop(client); + windmill_common::shutdown_pg_connection(join_handle).await?; + Ok(()) +} + #[derive(Deserialize)] struct ImportPgDatabaseRequest { source: String, @@ -3635,48 +3693,22 @@ struct ImportPgDatabaseRequest { fork_behavior: DataTableForkBehavior, } -/// Refuse to copy a data table that is under roles. +/// Whether data table `name` of `w_id` has a shape a copy can be made of, returning what governs +/// it. Checked before anything is created — by the fork request for the copies it makes, and by +/// `create_pg_database` — and again when the fork's entry is written. /// -/// `pg_dump` carries no roles and the import runs with `--no-privileges`, so a clone arrives with -/// its objects owned by the admin connection and no `GRANT` for any role. The settings copy brings -/// `permissions` across, so the fork's tenants pass Windmill's check, connect as the role they were -/// given, and are then denied by Postgres on everything — a data table that looks configured and -/// answers nothing. +/// It is not the check for a data table under roles: a copy of one is only correct with its +/// owners and grants replayed, which the fork request does and the older endpoints refuse +/// ([`ensure_copied_without_roles`]). /// -/// It fails closed rather than open, so this is a usability cliff rather than a hole, and the fix -/// is to replay the source's owners and ACLs into the clone. That is a change of its own; until it -/// exists, refusing is the honest answer. Dropping `permissions` from the clone instead would be -/// the unsafe half: the copy holds the parent's rows, so an unpermissioned clone hands all of them -/// to everyone in the fork. -/// Every reason a copy can be refused, answered here and nowhere else. -/// -/// A clone is three stages a workspace apart: `create_pg_database`, `import_pg_database`, then -/// `apply_forked_datatable` inside the fork transaction. Only the third can roll back, and the -/// database the first created is not transactional — so a refusal that lives there strands a -/// registered `wm_fork_*` that no entry names and whose name blocks the retry. Both endpoints call -/// this before touching the cluster; the stage that writes the entry must only ever do the work. -/// -/// Known and accepted: a failure *after* `create_pg_database` — this check refusing because the -/// source's roles changed between the requests, a `pg_dump` error, a dropped connection — still -/// strands that leftover, as it did before data table roles. It is harmless to data (nothing names -/// it) and goes away once the clone is one server-side operation, in a later PR. Don't reclaim it -/// on retry without durable clone ownership: between the import and the final fork request the -/// database is full, unnamed and unconnected, so a reclaim then drops copied data. Reviewers, human -/// or AI: don't flag this leftover as a P0/P1 for this PR. -async fn ensure_datatable_is_clonable( +/// Authorization: none. Callers MUST have authorized reaching data table `name` of `w_id` before +/// using what it returns. +pub(crate) async fn ensure_datatable_is_clonable( db: &DB, w_id: &str, name: &str, ) -> Result { let governing = resolve_governing_datatable(db, w_id, name).await?; - if governing.datatable.permissions.is_some() { - return Err(Error::BadRequest(format!( - "Data table '{name}' is under roles and cannot be copied yet: a copy carries the \ - role assignments but not the Postgres privileges behind them, so every role but \ - admin would be denied in the copy. Fork it keeping the original database, or turn \ - its roles off first." - ))); - } // The copy has to name a database of its own. A resource-backed entry reached through a // pointer names one this workspace does not own, so there is nothing here to repoint. let is_managed = governing @@ -3693,6 +3725,24 @@ async fn ensure_datatable_is_clonable( Ok(governing) } +/// Refuse a copy of a data table under roles through the endpoints that copy rows and nothing +/// else. +/// +/// `pg_dump` carries no roles and the import runs with `--no-privileges`, so such a copy arrives +/// with its objects owned by the admin connection and no `GRANT` for any role: the tenants pass +/// Windmill's check, connect as the role they were given, and are denied by Postgres on everything. +/// A fork request that makes the copy itself replays the owners and grants instead. +fn ensure_copied_without_roles(governing: &GoverningDatatable) -> Result<()> { + if governing.datatable.permissions.is_some() { + return Err(Error::BadRequest(format!( + "Data table '{}' is under roles, so a copy has to carry its owners and grants: \ + clone it through the fork wizard or `wmill workspace fork`, which do.", + governing.name + ))); + } + Ok(()) +} + /// Import (pg_dump/pg_import) from source to target async fn import_pg_database( authed: ApiAuthed, @@ -3707,7 +3757,7 @@ async fn import_pg_database( if let Some(reference) = req.source.strip_prefix("datatable://") { let (name, _) = parse_datatable_ref_for(&db, &w_id, reference).await?; - ensure_datatable_is_clonable(&db, &w_id, &name).await?; + ensure_copied_without_roles(&ensure_datatable_is_clonable(&db, &w_id, &name).await?)?; } if req.fork_behavior == DataTableForkBehavior::SchemaAndData { @@ -3721,9 +3771,10 @@ async fn import_pg_database( let schema_only = req.fork_behavior == DataTableForkBehavior::SchemaOnly; let mut fork_lock: Option> = None; - let source_pg = resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.source).await?; - let mut target_pg = - resolve_pg_source_checked(&db, &user_db, &authed, &w_id, &req.target).await?; + let (source_pg, source_kind) = + resolve_pg_source_checked_with_kind(&db, &user_db, &authed, &w_id, &req.source).await?; + let (mut target_pg, target_kind) = + resolve_pg_source_checked_with_kind(&db, &user_db, &authed, &w_id, &req.target).await?; if let Some(ref override_dbname) = req.target_dbname_override { if !windmill_api_auth::is_super_admin_authed(&db, &authed).await? { @@ -3733,7 +3784,10 @@ async fn import_pg_database( .to_string(), )); } - if let Some(kind) = managed_datatable_source_kind(&db, &w_id, &req.target).await? { + // The kind the connection above was built from, never a second read: an entry flipped + // to a resource between the two would keep the managed connection here and lose the + // check that decides which copy it may fill. + if let Some(kind) = target_kind { // Held until the restore is done: fork finalization takes the first, and every // save newly naming a database, in any workspace, the second. Nothing may start // using this database while `psql` is still filling it. @@ -3762,12 +3816,7 @@ async fn import_pg_database( // what it creates it owns. Grants do, except around an instance data table — Windmill // plants `custom_instance_user` grants in one, which nothing else can replay. Elsewhere // the ACLs are user intent (`REVOKE ... FROM PUBLIC`) and dropping them widens access. - let no_acl = managed_datatable_source_kind(&db, &w_id, &req.target) - .await? - .is_some() - || managed_datatable_source_kind(&db, &w_id, &req.source) - .await? - .is_some(); + let no_acl = target_kind.is_some() || source_kind.is_some(); let dump_file = pg_dump_database( &source_pg, @@ -3856,6 +3905,15 @@ async fn edit_ducklake_config( } let mut tx = db.begin().await?; + windmill_common::lock_instance_databases( + &mut tx, + new_config.settings.ducklakes.values().filter_map(|lake| { + (lake.catalog.resource_type + == windmill_common::workspaces::DucklakeCatalogResourceType::Instance) + .then_some(lake.catalog.resource_path.as_str()) + }), + ) + .await?; let args_for_audit = format!("{:?}", new_config.settings); audit_log( @@ -3977,6 +4035,16 @@ async fn edit_datatable_config( let mut tx = db.begin().await?; // Ahead of the settings row, as fork cleanup of this workspace takes the two. windmill_common::workspaces::lock_fork_datatables(&mut tx, &w_id).await?; + windmill_common::lock_instance_databases( + &mut tx, + new_config.settings.datatables.values().filter_map(|dt| { + dt.database + .as_ref() + .filter(|d| d.resource_type == DataTableCatalogResourceType::Instance) + .map(|d| d.resource_path.as_str()) + }), + ) + .await?; // Read under the row lock this transaction will write with. `permissions`, `reference` and // `forked_from` are carried across from what this read returns, so a permissions save @@ -4103,12 +4171,28 @@ async fn edit_datatable_config( // hand the fork the database outright. `forked_from` is the clone stamp the fork flow // writes: whether an entry has one is carried the same way, since it is what marks the // database droppable, but the schema baseline inside it is the diff view's to advance. + // `governed_by` is a clone's `reference` for its roles, and clearing it would hand the + // fork the copied rows the same way. dt.permissions = old.and_then(|old| old.permissions.clone()); dt.reference = old.and_then(|old| old.reference.clone()); + dt.governed_by = old.and_then(|old| old.governed_by.clone()); dt.forked_from = match old.and_then(|old| old.forked_from.as_ref()) { Some(stored) => Some(dt.forked_from.take().unwrap_or_else(|| stored.clone())), None => None, }; + // A clone's roles and grants were replayed into the database it was copied into, and hold + // for that database alone: whoever saves, it stays where it is. + if dt.governed_by.is_some() + && !crate::datatable_clone::same_database( + &dt.database, + &old.and_then(|old| old.database.clone()), + ) + { + return Err(Error::BadRequest(format!( + "Data table '{name}' is a clone taking its roles from the data table it was copied \ + from, and its grants hold for its own database only: it cannot be moved." + ))); + } // Carrying the block onto a resource-backed entry would produce a data table the chokepoint // refuses on every job — a save that succeeds and breaks everything afterwards. Refuse it // instead: turning roles off first is one step, and it keeps discarding an access decision @@ -4148,6 +4232,10 @@ async fn edit_datatable_config( // here because it cannot be requested at all: `reference` is overwritten from the stored entry // above, for every caller. An unchanged entry is left alone either way, so a downgraded // instance can still save settings that already name an external instance database. + // + // Compared against the entry the carried fields came from, not the one stored under the same + // name: otherwise swapping two names keeps each database in place while moving a clone's + // `governed_by` off the copy it governs. for (name, dt) in new_config.settings.datatables.iter() { let Some(database) = dt .database @@ -4157,7 +4245,12 @@ async fn edit_datatable_config( continue; }; let unchanged = old_datatables - .get(name) + .get( + rename_src + .get(name.as_str()) + .copied() + .unwrap_or(name.as_str()), + ) .and_then(|o| o.database.as_ref()) .is_some_and(|o| { o.resource_type == database.resource_type @@ -4204,7 +4297,8 @@ async fn edit_datatable_config( .settings .datatables .iter() - .filter(|(_, dt)| dt.permissions.is_none()) + // A clone carries its roles through `governed_by` rather than `permissions`. + .filter(|(_, dt)| dt.permissions.is_none() && dt.governed_by.is_none()) .filter_map(|(name, dt)| { let db = dt .database @@ -4265,7 +4359,7 @@ async fn edit_datatable_config( sqlx::query_scalar( "SELECT DISTINCT dt.value->'database'->>'resource_path' FROM workspace_settings ws CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt - WHERE ws.workspace_id <> $1 AND dt.value ? 'permissions' + WHERE ws.workspace_id <> $1 AND (dt.value ? 'permissions' OR dt.value ? 'governed_by') AND dt.value->'database'->>'resource_type' = 'instance'", ) .bind(&w_id) @@ -4274,7 +4368,7 @@ async fn edit_datatable_config( }; for (name, dbname) in newly_pointed { let governed_here = old_datatables.values().any(|old| { - old.permissions.is_some() + (old.permissions.is_some() || old.governed_by.is_some()) && old.database.as_ref().is_some_and(|d| { d.resource_type == DataTableCatalogResourceType::Instance && d.resource_path == dbname @@ -4334,8 +4428,11 @@ async fn edit_datatable_config( r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!" FROM workspace_settings ws CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt - WHERE dt.value->'reference'->>'workspace_id' = $1 - AND dt.value->'reference'->>'datatable' = $2"#, + WHERE EXISTS ( + SELECT 1 FROM (VALUES ('reference'), ('governed_by')) k(link) + WHERE dt.value->k.link->>'workspace_id' = $1 + AND dt.value->k.link->>'datatable' = $2 + )"#, &w_id, name, ) @@ -8463,6 +8560,7 @@ async fn create_workspace_fork_branch( // dangling branch on the synced repos. check_fork_w_id_conflict(&db, &nw.id).await?; purge_stale_fork_diff_state(&db, &nw.id).await?; + validate_forked_datatables(&db, &authed, &w_id, &nw).await?; Ok(Json( handle_fork_branch_creation(&authed.email, &authed.username, &db, &w_id, &nw.id).await?, @@ -8502,8 +8600,9 @@ async fn snapshot_datatable_schema( /// a fork admin could then edit to widen their own access to it. A pointer has nothing local to /// edit: the parent's entry stays the only place the decision lives. /// -/// The cloned data tables are skipped: they own a fresh database of their own, and they keep the -/// copied `permissions` as their starting point, which they then govern. +/// The cloned data tables are skipped: they own a fresh database of their own, and +/// `apply_forked_datatable` has already dropped their copied `permissions` — for a clone of a data +/// table under roles, in favor of `governed_by`. async fn point_kept_datatables_at_parent( tx: &mut Transaction<'_, Postgres>, parent_w_id: &str, @@ -8559,6 +8658,7 @@ async fn point_kept_datatables_at_parent( workspace_id: parent_w_id.to_string(), datatable: name.clone(), }), + governed_by: None, forked_from: None, migrations_enabled: dt.migrations_enabled, permissions: None, @@ -8579,7 +8679,8 @@ async fn point_kept_datatables_at_parent( Ok(()) } -/// Move every pointer in any workspace that names `(w_id, from)` to `(w_id, to)`. +/// Move every pointer, and every clone's `governed_by`, in any workspace that names `(w_id, from)` +/// to `(w_id, to)`. /// /// `EXISTS` rather than a `LIKE` over the whole document: the update rewrites the row, so matching /// every workspace that holds any pointer would rewrite rows to a byte-identical value and hold an @@ -8595,17 +8696,22 @@ async fn repoint_datatable_references( SET datatable = ( SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg( dt.key, - CASE WHEN dt.value->'reference'->>'workspace_id' = $1 - AND dt.value->'reference'->>'datatable' = $2 - THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text)) - ELSE dt.value END + (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $1 + AND r.v->'governed_by'->>'datatable' = $2 + THEN jsonb_set(r.v, '{governed_by,datatable}', to_jsonb($3::text)) + ELSE r.v END + FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $1 + AND dt.value->'reference'->>'datatable' = $2 + THEN jsonb_set(dt.value, '{reference,datatable}', to_jsonb($3::text)) + ELSE dt.value END AS v) r) )) FROM jsonb_each(ws.datatable->'datatables') dt ) WHERE EXISTS ( - SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d - WHERE d.value->'reference'->>'workspace_id' = $1 - AND d.value->'reference'->>'datatable' = $2 + SELECT 1 FROM jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) d, + LATERAL (VALUES ('reference'), ('governed_by')) k(link) + WHERE d.value->k.link->>'workspace_id' = $1 + AND d.value->k.link->>'datatable' = $2 )"#, w_id, from, @@ -8623,17 +8729,8 @@ async fn apply_forked_datatable( parent_w_id: &str, forked_w_id: &str, fdt: &ForkedDatatableInfo, + copy: &crate::datatable_clone::MadeCopy, ) -> Result<()> { - // Cloning reads the parent's whole schema as admin and hands the copy to the fork, so it is - // for the workspace that governs the data table — a fork can use one, never duplicate it. - windmill_common::workspaces::ensure_datatable_admin_access( - db, - parent_w_id, - &fdt.name, - &DatatableAccess::Authed(authed.to_authed_ref()), - ) - .await?; - let governing = ensure_datatable_is_clonable(db, parent_w_id, &fdt.name).await?; windmill_common::validate_dbname(&fdt.new_dbname)?; if !fdt.new_dbname.starts_with("wm_fork_") { return Err(Error::BadRequest(format!( @@ -8641,6 +8738,52 @@ async fn apply_forked_datatable( fdt.new_dbname ))); } + // Held until the fork commits, as the permissions save holds them: the source moved onto + // another database, or put under roles or taken off them, since its copy was made would link + // the copy to roles its grants were not replayed for. + let locked = crate::datatable_clone::lock_settings_rows(tx, parent_w_id, &fdt.name).await?; + let governing = ensure_datatable_is_clonable(db, parent_w_id, &fdt.name).await?; + let under_roles = governing.datatable.permissions.is_some(); + let unchanged = crate::datatable_clone::same_database( + &governing.datatable.database, + &Some(copy.source_database.clone()), + ) && copy.replayed == under_roles + && crate::datatable_clone::lock_settings_rows(tx, parent_w_id, &fdt.name).await? == locked; + if !unchanged { + return Err(Error::BadRequest(format!( + "Data table '{}' changed while it was being copied for this fork — its database, or \ + whether it is under roles. Create the fork again.", + fdt.name + ))); + } + // The fork keeps a snapshot of the source's schema, which under roles is only for those who may + // connect as one of them — as the copy itself was. + crate::datatable_permissions::ensure_reaches_governing_datatable( + db, + parent_w_id, + &fdt.name, + &governing, + authed, + ) + .await?; + // Under roles, the copy holds rows the source's roles decide who reaches, so that entry keeps + // deciding. Settled from the source as it resolves now: the settings clone may have handed the + // fork a pointer, or a clone of its own. A copy of a data table without roles is the fork's, + // as it was before roles existed: everyone reached all of it already. + let governed_by = governing + .datatable + .permissions + .is_some() + .then(|| { + serde_json::to_value(governing.governor.clone().unwrap_or_else(|| { + windmill_common::workspaces::DataTableReference { + workspace_id: governing.workspace_id.clone(), + datatable: governing.name.clone(), + } + })) + }) + .transpose() + .map_err(|e| Error::internal_err(format!("serializing a clone's governor: {e}")))?; // Snapshot the schema from the source (parent) datatable let schema = snapshot_datatable_schema(db, parent_w_id, &fdt.name).await?; @@ -8714,33 +8857,42 @@ async fn apply_forked_datatable( "resource_type": database.resource_type, "resource_path": &fdt.new_dbname, }); - sqlx::query!( + sqlx::query( r#"UPDATE workspace_settings SET datatable = jsonb_set( - jsonb_set( - datatable #- ARRAY['datatables', $2, 'reference'], - ARRAY['datatables', $2, 'database'], $3::jsonb), - ARRAY['datatables', $2, 'forked_from'], $4::jsonb - ) + datatable #- ARRAY['datatables', $2, 'reference'], + ARRAY['datatables', $2, 'database'], $3::jsonb) WHERE workspace_id = $1"#, - forked_w_id, - &fdt.name, - new_database, - forked_from, ) + .bind(forked_w_id) + .bind(&fdt.name) + .bind(new_database) .execute(&mut **tx) .await?; } else { - // Resource: update the resource's dbname and mark as ws_specific + // Resource: point it at the copy and mark it ws_specific. What the settings clone carried is + // the source's resource and variables as they are now, which an edit made while the copy + // was being made can have changed, and changed back: the clone has to be what the copy's + // connection was resolved from. let resource_path = &database.resource_path; - sqlx::query!( + let cloned = + crate::datatable_clone::connection_snapshot(db, &mut **tx, forked_w_id, resource_path) + .await?; + if copy.connection.as_ref() != Some(&cloned) { + return Err(Error::BadRequest(format!( + "The resource of data table '{}' changed while it was being copied for this \ + fork. Create the fork again.", + fdt.name + ))); + } + sqlx::query( r#"UPDATE resource SET value = jsonb_set(value, '{dbname}', to_jsonb($3::text)) WHERE workspace_id = $1 AND path = $2"#, - forked_w_id, - resource_path, - &fdt.new_dbname, ) + .bind(forked_w_id) + .bind(resource_path) + .bind(&fdt.new_dbname) .execute(&mut **tx) .await?; @@ -8751,20 +8903,31 @@ async fn apply_forked_datatable( ) .execute(&mut **tx) .await?; - - // Set forked_from on the datatable config - sqlx::query!( - r#"UPDATE workspace_settings - SET datatable = jsonb_set(datatable, ARRAY['datatables', $2, 'forked_from'], $3::jsonb) - WHERE workspace_id = $1"#, - forked_w_id, - &fdt.name, - forked_from, - ) - .execute(&mut **tx) - .await?; } + // The settings clone copied the source's `permissions` along, and a clone of a clone its + // `governed_by`: a clone keeps no `permissions` of its own, and a link only under roles. + sqlx::query( + r#"UPDATE workspace_settings + SET datatable = jsonb_set( + CASE WHEN $3::jsonb IS NULL + THEN datatable #- ARRAY['datatables', $2, 'permissions'] + #- ARRAY['datatables', $2, 'governed_by'] + ELSE jsonb_set( + datatable #- ARRAY['datatables', $2, 'permissions'], + ARRAY['datatables', $2, 'governed_by'], $3::jsonb) + END, + ARRAY['datatables', $2, 'forked_from'], $4::jsonb + ) + WHERE workspace_id = $1"#, + ) + .bind(forked_w_id) + .bind(&fdt.name) + .bind(governed_by) + .bind(forked_from) + .execute(&mut **tx) + .await?; + Ok(()) } @@ -9099,7 +9262,169 @@ async fn create_workspace_fork( ensure_no_existing_dev_workspace(&db, &parent_workspace_id).await?; } + // Refused here, before any database exists; `make_copies` checks again under the locks. + validate_forked_datatables(&db, &authed, &parent_workspace_id, &nw).await?; + + // Detached from the request: a client that goes away while the copies are made must still end + // with the fork created or no copy left behind, and dropping the handler's future would skip + // that cleanup. + tokio::spawn(async move { + let fork_id = nw.id.clone(); + let copies = crate::datatable_clone::make_copies( + &db, + &authed, + &parent_workspace_id, + ©_requests(&nw.forked_datatables), + ) + .await?; + let replayed: Vec = copies + .iter() + .filter(|c| c.replayed) + .map(|c| c.behavior) + .collect(); + match write_workspace_fork( + db.clone(), + authed, + parent_workspace_id, + nw, + dev_workspace_label, + &copies, + ) + .await + { + Ok(message) => { + for behavior in replayed { + windmill_common::feature_usage::log_feature_usage( + "datatable", + "clone_replayed", + match behavior { + DataTableForkBehavior::SchemaOnly => "schema_only", + _ => "schema_and_data", + }, + ); + } + Ok(message) + } + // A commit whose acknowledgement was lost can still have committed, and a concurrent + // request for the same id can have: once the write has settled, the cleanup keeps + // whichever copies a committed fork names, and drops the rest. + Err(e) => match wait_for_fork_write(&db, &fork_id).await { + Ok(()) => Err(crate::datatable_clone::drop_copies_after(&db, copies, e).await), + Err(settle) => { + tracing::error!( + "Could not tell whether fork '{fork_id}' was created, so its copies were \ + kept: {settle}" + ); + Err(e) + } + }, + } + }) + .await + .map_err(|e| Error::internal_err(format!("Creating the fork stopped unexpectedly: {e}")))? +} + +/// The database a data table of the fork or dev workspace `fork_id` is copied into, as the wizard +/// and the CLI derive it. A dev workspace's id has no `wm-fork-` prefix, and its copies are dropped +/// by the same `wm_fork_` rule as a fork's. Dev workspace `x` and fork `wm-fork-x` share a name, as +/// their branches do: the second to copy a data table of that name is refused at `CREATE`. +fn forked_datatable_dbname(fork_id: &str, datatable: &str) -> String { + let suffix = fork_id + .strip_prefix(windmill_common::workspaces::WM_FORK_PREFIX) + .unwrap_or(fork_id); + format!("wm_fork_{}__{datatable}", suffix.replace('-', "_")) +} + +/// Wait until no transaction writing fork `fork_id` is still running: the lock +/// `write_workspace_fork` holds until its transaction ends is taken and released. +async fn wait_for_fork_write(db: &DB, fork_id: &str) -> Result<()> { + let mut tx = db.begin().await?; + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('fork:' || $1))") + .bind(fork_id) + .execute(&mut *tx) + .await?; + tx.commit().await?; + Ok(()) +} + +/// Everything about a fork's data tables that can be refused before a git branch or a database is +/// created, and that the fork request re-checks. +/// +/// Every data table is copied by the fork request, into a database it creates: an entry naming a +/// database that already exists could reach a copy made for another fork — one left behind by a +/// deleted fork included — without that copy's governance. +async fn validate_forked_datatables( + db: &DB, + authed: &ApiAuthed, + parent_w_id: &str, + nw: &CreateWorkspaceFork, +) -> Result<()> { + let mut names = HashSet::new(); + for fdt in &nw.forked_datatables { + if fdt.fork_behavior.is_none() { + return Err(Error::BadRequest(format!( + "Data table '{}' names no `fork_behavior`: the fork request makes the copy of each \ + data table itself. Update the Windmill CLI.", + fdt.name + ))); + } + let expected = forked_datatable_dbname(&nw.id, &fdt.name); + if fdt.new_dbname != expected { + return Err(Error::BadRequest(format!( + "Data table '{}' of fork '{}' is copied into database '{expected}', not '{}'", + fdt.name, nw.id, fdt.new_dbname + ))); + } + if !names.insert(fdt.name.as_str()) { + return Err(Error::BadRequest(format!( + "Data table '{}' is named more than once in this fork", + fdt.name + ))); + } + } + crate::datatable_clone::authorize_copies( + db, + authed, + parent_w_id, + ©_requests(&nw.forked_datatables), + ) + .await +} + +/// The copies a fork request asks this request to make. [`validate_forked_datatables`] refuses an +/// entry without a `fork_behavior`. +fn copy_requests( + forked_datatables: &[ForkedDatatableInfo], +) -> Vec> { + forked_datatables + .iter() + .filter_map(|fdt| { + fdt.fork_behavior + .map(|behavior| crate::datatable_clone::CopyRequest { + name: &fdt.name, + dbname: &fdt.new_dbname, + behavior, + }) + }) + .collect() +} + +/// Write the fork, with its data tables pointed at `copies`. +async fn write_workspace_fork( + db: DB, + authed: ApiAuthed, + parent_workspace_id: String, + nw: CreateWorkspaceFork, + dev_workspace_label: Option, + copies: &[crate::datatable_clone::MadeCopy], +) -> Result { let mut tx: Transaction<'_, Postgres> = db.begin().await?; + // Held until this transaction ends: after an error, the copies' cleanup waits on it, so it reads + // whether the fork exists only once a commit still being resolved has settled. + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('fork:' || $1))") + .bind(&nw.id) + .execute(&mut *tx) + .await?; // Before the settings clone reads the parent's data tables: a pointer this fork ends up with // must not be written after cleanup of the parent decided that nothing points at its copies. // Also before the external cluster's lifecycle lock, which finalizing an external copy takes: @@ -9230,10 +9555,41 @@ async fn create_workspace_fork( repoint_unresolvable_cloned_identities(&mut tx, &forked_id, &authed).await?; + // Held until the fork commits, as settings saves naming these databases hold it: an entry + // saved elsewhere while a copy was being made would reach it without the governance written + // below, and the alias check on that save saw no governed entry yet. + let instance_copies: Vec<&str> = copies + .iter() + .filter(|c| c.source_database.resource_type == DataTableCatalogResourceType::Instance) + .map(|c| c.dbname.as_str()) + .collect(); + windmill_common::lock_instance_databases(&mut tx, instance_copies.iter().copied()).await?; + for dbname in &instance_copies { + let users = windmill_common::instance_database_users(&mut *tx, dbname).await?; + if !users.is_empty() { + return Err(Error::BadRequest(format!( + "Database '{dbname}' copied for this fork is already used by workspaces {}; \ + fork again", + users.join(", ") + ))); + } + } + // Update forked datatable settings to point to new databases for fdt in &nw.forked_datatables { - apply_forked_datatable(&db, &mut tx, &authed, &parent_workspace_id, &forked_id, fdt) - .await?; + let copy = copies.iter().find(|c| c.name == fdt.name).ok_or_else(|| { + Error::internal_err(format!("No copy was made of data table '{}'", fdt.name)) + })?; + apply_forked_datatable( + &db, + &mut tx, + &authed, + &parent_workspace_id, + &forked_id, + fdt, + copy, + ) + .await?; } point_kept_datatables_at_parent( @@ -9292,6 +9648,20 @@ async fn create_workspace_fork( .await?; } + let copied = copies + .iter() + .map(|c| { + format!( + "{} ({})", + c.name, + match c.behavior { + DataTableForkBehavior::SchemaOnly => "schema_only", + _ => "schema_and_data", + } + ) + }) + .collect::>() + .join(", "); audit_log( &mut *tx, &authed, @@ -9299,7 +9669,7 @@ async fn create_workspace_fork( ActionKind::Create, &forked_id, Some(nw.name.as_str()), - None, + (!copied.is_empty()).then(|| [("copied_datatables", copied.as_str())].into()), ) .await?; tx.commit().await?; diff --git a/backend/windmill-api-workspaces/src/workspaces_extra.rs b/backend/windmill-api-workspaces/src/workspaces_extra.rs index e17eeab08f..1dd2a4b169 100644 --- a/backend/windmill-api-workspaces/src/workspaces_extra.rs +++ b/backend/windmill-api-workspaces/src/workspaces_extra.rs @@ -507,21 +507,25 @@ pub(crate) async fn change_workspace_id( // A fork's data table entry names the workspace that governs it by id, so the rename has to // follow there too — anywhere, not just in the reparented children: a detached workspace can // point at this one without being its fork. Left behind, the pointer resolves to the archived - // shell and every job through it stops. + // shell and every job through it stops. A clone's `governed_by` names it the same way. info!("Re-pointing data table references to the new workspace id"); sqlx::query!( r#"UPDATE workspace_settings ws SET datatable = ( SELECT jsonb_set(ws.datatable, '{datatables}', jsonb_object_agg( dt.key, - CASE WHEN dt.value->'reference'->>'workspace_id' = $2 - THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text)) - ELSE dt.value END + (SELECT CASE WHEN r.v->'governed_by'->>'workspace_id' = $2 + THEN jsonb_set(r.v, '{governed_by,workspace_id}', to_jsonb($1::text)) + ELSE r.v END + FROM (SELECT CASE WHEN dt.value->'reference'->>'workspace_id' = $2 + THEN jsonb_set(dt.value, '{reference,workspace_id}', to_jsonb($1::text)) + ELSE dt.value END AS v) r) )) FROM jsonb_each(ws.datatable->'datatables') dt ) WHERE jsonb_typeof(ws.datatable->'datatables') = 'object' - AND ws.datatable::text LIKE '%"reference"%'"#, + AND (ws.datatable::text LIKE '%"reference"%' + OR ws.datatable::text LIKE '%"governed_by"%')"#, &rw.new_id, &old_id, ) @@ -1042,17 +1046,19 @@ pub(crate) async fn delete_workspace( // fails mid-way must never leave a live workspace with its fork data destroyed and no // registry row to retry from. Read-only: nothing is dropped here. // Read before the delete: another workspace's data table entry can point at one of this - // workspace's, and deleting the workspace it names leaves that pointer resolving to nothing. - // Nothing sweeps them — turning them back into copies would hand each fork the database - // outright — so the deleter is told which data tables they just stranded. - let stranded_pointers = sqlx::query!( - r#"SELECT ws.workspace_id AS "workspace_id!", dt.key AS "datatable!" + // workspace's, or be a clone taking its roles from one, and deleting the workspace it names + // leaves it resolving to nothing. Nothing sweeps them — turning them back into copies would + // hand each fork the database outright — so the deleter is told which data tables they just + // stranded. + let stranded_pointers = sqlx::query_as::<_, (String, String)>( + r#"SELECT ws.workspace_id, dt.key FROM workspace_settings ws CROSS JOIN LATERAL jsonb_each(COALESCE(ws.datatable->'datatables', '{}'::jsonb)) dt WHERE dt.value->'reference'->>'workspace_id' = $1 + OR dt.value->'governed_by'->>'workspace_id' = $1 ORDER BY ws.workspace_id, dt.key"#, - &w_id, ) + .bind(&w_id) .fetch_all(&db) .await .unwrap_or_default(); @@ -1380,7 +1386,7 @@ pub(crate) async fn delete_workspace( } else { let stranded = stranded_pointers .iter() - .map(|r| format!("{}/{}", r.workspace_id, r.datatable)) + .map(|(workspace_id, datatable)| format!("{workspace_id}/{datatable}")) .collect::>() .join(", "); Ok(format!( diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 551382b437..3d55b8d104 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -29094,6 +29094,15 @@ components: type: object additionalProperties: $ref: "#/components/schemas/ModelPriceOverride" + context_window_per_model: + type: object + description: >- + Context window in tokens per `provider:model`, overriding the built-in + table the AI chat uses to decide when to compact its history. + additionalProperties: + type: integer + minimum: 1024 + maximum: 10000000 copilot_disabled: type: boolean description: >- @@ -33920,6 +33929,8 @@ components: $ref: "#/components/schemas/LoggedWizardStatus" replication_user_error: type: string + user_connect: + $ref: "#/components/schemas/LoggedWizardStatus" CustomInstanceDbTag: type: string @@ -34008,6 +34019,15 @@ components: $ref: "#/components/schemas/DatatableRoleTenants" governing_workspace_id: type: string + clone_of: + type: object + description: for a clone, the data table whose roles it takes + required: [workspace_id, datatable] + properties: + workspace_id: + type: string + datatable: + type: string editable: type: boolean available_roles: @@ -34246,7 +34266,7 @@ components: DatatableAclInfo: type: object - required: [owner, roles, editable, supports_maintain, dbname, grants, children] + required: [owner, roles, editable, clone, supports_maintain, dbname, grants, children] properties: owner: type: string @@ -34258,6 +34278,9 @@ components: editable: type: boolean description: whether the caller may plan and apply changes + clone: + type: boolean + description: whether this is a clone, whose grants stay as they were copied supports_maintain: type: boolean description: whether the server is Postgres 17+, which added the MAINTAIN table privilege @@ -35413,6 +35436,16 @@ components: new_dbname: type: string description: "New database name for the fork" + fork_behavior: + type: string + enum: + - schema_only + - schema_and_data + description: >- + What the fork request copies into `new_dbname`, which it creates — with the + owners and grants of a data table under roles. This server refuses an entry + without it; servers predating it expect `new_dbname` created and filled + beforehand. shared_ducklakes: type: array items: @@ -36377,6 +36410,17 @@ components: type: string datatable: type: string + governed_by: + description: >- + On a clone, the data table it was copied from, whose roles it takes. Server-owned + like `reference`. + type: object + required: [workspace_id, datatable] + properties: + workspace_id: + type: string + datatable: + type: string migrations_enabled: type: boolean description: Whether the SQL migrations feature is opted in for this data table diff --git a/backend/windmill-api/src/ai.rs b/backend/windmill-api/src/ai.rs index 1710db9cec..16b4b17141 100644 --- a/backend/windmill-api/src/ai.rs +++ b/backend/windmill-api/src/ai.rs @@ -23,7 +23,7 @@ use windmill_ai::ai_cache::current_instance_ai_config_revision; use windmill_ai::ai_providers::{ empty_string_as_none, AIPlatform, AIProvider, ProviderConfig, ProviderModel, }; -use windmill_ai::ai_types::MAX_MODEL_RATE; +use windmill_ai::ai_types::{validate_token_limit, CONTEXT_WINDOWS, MAX_MODEL_RATE, OUTPUT_LIMITS}; use windmill_ai::credentials::ProviderCredentials; #[cfg(feature = "bedrock")] use windmill_ai::providers::bedrock::{ @@ -445,6 +445,11 @@ 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>, + /// Per-model context windows the chat budgets its history against, keyed + /// `provider:model` like `max_tokens_per_model`. Overrides the client's built-in + /// table, which a model served through a custom endpoint is usually missing from. + #[serde(skip_serializing_if = "Option::is_none")] + pub context_window_per_model: 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 @@ -508,6 +513,18 @@ impl AIConfig { Ok(()) } + pub fn validate_token_limits(&self) -> Result<()> { + for (entries, bounds) in [ + (&self.context_window_per_model, &CONTEXT_WINDOWS), + (&self.max_tokens_per_model, &OUTPUT_LIMITS), + ] { + for (key, tokens) in entries.iter().flatten() { + validate_token_limit(bounds, key, i64::from(*tokens)).map_err(Error::BadRequest)?; + } + } + Ok(()) + } + pub fn validate_sessions_retention(&self) -> Result<()> { match self.sessions_retention_days { Some(days) if !(1..=MAX_SESSIONS_RETENTION_DAYS).contains(&days) => { diff --git a/backend/windmill-api/src/workspaces.rs b/backend/windmill-api/src/workspaces.rs index a0a27eab4c..186cfbc291 100644 --- a/backend/windmill-api/src/workspaces.rs +++ b/backend/windmill-api/src/workspaces.rs @@ -108,6 +108,7 @@ async fn edit_copilot_config( } ai_config.validate_model_pricing()?; + ai_config.validate_token_limits()?; ai_config.validate_sessions_retention()?; let mut tx = db.begin().await?; diff --git a/backend/windmill-common/src/guest_jwt.rs b/backend/windmill-common/src/guest_jwt.rs index 9f3e47d708..dc0407163d 100644 --- a/backend/windmill-common/src/guest_jwt.rs +++ b/backend/windmill-common/src/guest_jwt.rs @@ -167,7 +167,13 @@ pub fn jwk_algorithms(jwk: &Jwk) -> Option> { { return None; } - match (&jwk.algorithm, jwk.common.algorithm) { + let alg = crate::jwt::jwk_algorithm(jwk); + // A key that names an `alg` we cannot verify with (RSA-OAEP, ...) is not an alg-less + // key to be tried against the whole family: it is published for something else. + if alg.is_none() && jwk.common.key_algorithm.is_some() { + return None; + } + match (&jwk.algorithm, alg) { (AlgorithmParameters::RSA(_), Some(alg)) if RSA_ALGORITHMS.contains(&alg) => { Some(vec![alg]) } @@ -197,6 +203,9 @@ pub fn verify( let mut validation = Validation::new(algorithms[0]); validation.algorithms = algorithms.to_vec(); validation.validate_nbf = true; + // Audience is not part of the guest contract: issuers commonly stamp their own `aud`, + // and no accepted audience is configured, so an `aud` claim must not fail the token. + validation.validate_aud = false; let claims = jsonwebtoken::decode::(token, key, &validation) .map_err(|e| Error::NotAuthorized(format!("guest JWT refused: {e}")))? .claims; @@ -648,6 +657,12 @@ mod tests { assert_eq!(jwk_algorithms(&k), None); } + #[test] + fn a_key_naming_an_encryption_alg_is_refused() { + let k = jwk(serde_json::json!({"kty":"RSA","alg":"RSA-OAEP","n":"aa","e":"AQAB"})); + assert_eq!(jwk_algorithms(&k), None); + } + #[test] fn a_key_marked_for_encryption_is_refused() { let k = jwk(serde_json::json!({"kty":"RSA","use":"enc","n":"aa","e":"AQAB"})); @@ -789,6 +804,50 @@ mod tests { .is_err()); } + #[test] + fn a_token_carrying_an_audience_still_verifies() { + // jsonwebtoken validates `aud` by default when the token has one, which would refuse + // every issuer that stamps its own audience. + let key = jsonwebtoken::DecodingKey::from_jwk(&jwk(serde_json::json!({ + "kty": "EC", "crv": "P-256", "kid": "k1", "x": PUB1_X, "y": PUB1_Y + }))) + .unwrap(); + let token = jsonwebtoken::encode( + &jsonwebtoken::Header::new(Algorithm::ES256), + &serde_json::json!({ + "email": "g@example.com", + "workspace_id": "ws", + "app_path": "u/a/app", + "aud": "https://issuer.example/api", + "exp": jsonwebtoken::get_current_timestamp() + 600, + }), + &jsonwebtoken::EncodingKey::from_ec_pem(PRIV1.as_bytes()).unwrap(), + ) + .unwrap(); + verify(&token, &key, &[Algorithm::ES256], "ws").expect("aud is not validated"); + } + + #[test] + fn an_8192_bit_rsa_key_verifies() { + // The largest modulus real issuers use; a crypto backend capped at 4096 bits rejects + // every token signed with it while the key itself still loads. + const PRIVATE: &str = include_str!("../tests/fixtures/guest_jwt_rsa8192_private.pem"); + const PUBLIC: &str = include_str!("../tests/fixtures/guest_jwt_rsa8192_public.pem"); + let (key, algs) = decoding_key_from_pem(PUBLIC).expect("8192-bit SPKI loads"); + let token = jsonwebtoken::encode( + &jsonwebtoken::Header::new(Algorithm::RS256), + &serde_json::json!({ + "email": "g@example.com", + "workspace_id": "ws", + "app_path": "u/a/app", + "exp": jsonwebtoken::get_current_timestamp() + 600, + }), + &jsonwebtoken::EncodingKey::from_rsa_pem(PRIVATE.as_bytes()).unwrap(), + ) + .unwrap(); + verify(&token, &key, algs, "ws").expect("8192-bit RSA signature verifies"); + } + // A real RSA public key (SPKI). Its private counterpart is RSA_PKCS1_PRIVATE below. const RSA_PUBLIC: &str = "-----BEGIN PUBLIC KEY-----\n\ MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAx3J0fQcHp2ZlMI4rCVsY\n\ diff --git a/backend/windmill-common/src/instance_config.rs b/backend/windmill-common/src/instance_config.rs index dd2c0ee713..02b175f090 100644 --- a/backend/windmill-common/src/instance_config.rs +++ b/backend/windmill-common/src/instance_config.rs @@ -836,6 +836,8 @@ pub struct CustomInstanceDbLogs { pub replication_user: String, #[serde(skip_serializing_if = "Option::is_none")] pub replication_user_error: Option, + #[serde(skip_serializing_if = "String::is_empty")] + pub user_connect: String, } // --------------------------------------------------------------------------- diff --git a/backend/windmill-common/src/jwt.rs b/backend/windmill-common/src/jwt.rs index cc2f1102bf..76df13b518 100644 --- a/backend/windmill-common/src/jwt.rs +++ b/backend/windmill-common/src/jwt.rs @@ -3,7 +3,6 @@ use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; use hmac::{Hmac, Mac}; use serde::{de::DeserializeOwned, Serialize}; use sha2::Sha256; -use std::collections::HashSet; lazy_static::lazy_static! { pub static ref JWT_SECRET: arc_swap::ArcSwap = arc_swap::ArcSwap::from_pointee("".to_string()); @@ -44,18 +43,8 @@ pub async fn decode_with_internal_secret(token: &str) -> er } pub fn decode_without_verify(token: &str) -> anyhow::Result { - // Create a validation that skips all checks - let mut validation = jsonwebtoken::Validation::default(); - validation.insecure_disable_signature_validation(); - validation.validate_exp = false; - validation.validate_nbf = false; - validation.required_spec_claims = HashSet::new(); - - // Use an empty key since we're not verifying - let key = jsonwebtoken::DecodingKey::from_secret(&[]); - - // Decode the token - let token_data = jsonwebtoken::decode::(token, &key, &validation)?; + // No signature, expiry, nbf or required-claim checks: the caller wants the raw claims. + let token_data = jsonwebtoken::dangerous::insecure_decode::(token)?; Ok(token_data.claims) } @@ -74,3 +63,36 @@ pub async fn generate_signature(header_and_payload: &str) -> anyhow::Result Option { + // `alg` is a `KeyAlgorithm` (signing and encryption algorithms alike); only the + // signing subset maps onto `Algorithm`. + jwk.common + .key_algorithm + .and_then(|alg| alg.to_string().parse::().ok()) +} + +#[cfg(test)] +mod tests { + use super::jwk_algorithm; + use jsonwebtoken::{jwk::Jwk, Algorithm}; + + #[test] + fn jwk_algorithm_reads_alg_when_present() { + let with: Jwk = + serde_json::from_value(serde_json::json!({"kty":"RSA","alg":"RS256","n":"aa","e":"AQAB"})) + .unwrap(); + assert_eq!(jwk_algorithm(&with), Some(Algorithm::RS256)); + let without: Jwk = + serde_json::from_value(serde_json::json!({"kty":"RSA","n":"aa","e":"AQAB"})).unwrap(); + assert_eq!(jwk_algorithm(&without), None); + let encryption: Jwk = serde_json::from_value( + serde_json::json!({"kty":"RSA","alg":"RSA-OAEP","n":"aa","e":"AQAB"}), + ) + .unwrap(); + assert_eq!(jwk_algorithm(&encryption), None); + } +} diff --git a/backend/windmill-common/src/lib.rs b/backend/windmill-common/src/lib.rs index 2974df5300..6aee64a7dd 100644 --- a/backend/windmill-common/src/lib.rs +++ b/backend/windmill-common/src/lib.rs @@ -1484,20 +1484,161 @@ pub fn validate_dbname(dbname: &str) -> error::Result<()> { Ok(()) } +/// Lock the instance databases among `names` until `tx` ends, in a stable order. Taken by every +/// settings save naming an instance database, and by [`drop_unused_instance_database`]: a save +/// cannot start using a database between that drop's check that nothing does and the drop. +pub async fn lock_instance_databases<'a>( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + names: impl IntoIterator, +) -> error::Result<()> { + let names: std::collections::BTreeSet<&str> = names.into_iter().collect(); + for name in names { + sqlx::query("SELECT pg_advisory_xact_lock(hashtext('instance_database:' || $1))") + .bind(name) + .execute(&mut **tx) + .await?; + } + Ok(()) +} + +/// Drop instance database `dbname`, which a request just created, unless a workspace names it as a +/// data table or a ducklake catalog — then it is kept, and who keeps it is what comes back. A +/// database is registered on the instance as soon as it is created, so a superadmin can point a +/// workspace at it before the request that created it gives up on it. +/// +/// Authorization: none. Callers MUST pass only a database the same request created and has not +/// handed to anything yet. +/// +/// The lock, the check and the drop share one connection: a second one taken from the pool while +/// the first is held could wait forever on a small pool. That connection is closed rather than +/// returned, since a session lock outlives the future holding it: a cancellation between taking +/// the lock and releasing it would otherwise hand a locked session back to the pool, where every +/// later settings save waits on it. Closing on drop covers the cancellation and still counts the +/// connection against the pool, which detaching it would not. +pub async fn drop_unused_instance_database(db: &DB, dbname: &str) -> error::Result { + let mut conn = db.acquire().await?; + conn.close_on_drop(); + let key = format!("instance_database:{dbname}"); + // A save blocked on this lock is about to name the database, but it may also roll back — a + // later validation of its own, a superadmin check, a cancelled request. So it is let through + // and what it committed is read, rather than taken as a user: trusting it would strand the + // database, whose name then blocks every retry. A fresh waiter can always arrive, so after a + // few rounds the database is kept instead; the name is then a superadmin's to drop from + // instance settings, since a retry fails on the name before reaching this cleanup. + let mut rounds = 0; + let dropped = loop { + if let Err(e) = sqlx::query("SELECT pg_advisory_lock(hashtext($1))") + .bind(&key) + .execute(&mut *conn) + .await + { + break Err(e.into()); + } + match drop_if_unused_on(&mut conn, dbname).await { + Ok(Cleanup::Waiter(_)) if rounds < 2 => { + rounds += 1; + // Releasing hands the lock to the waiter; re-taking it above then waits for that + // transaction to end, so the next round reads what it actually committed. + if let Err(e) = unlock_instance_database(&mut conn, &key).await { + break Err(e); + } + } + Ok(outcome) => break Ok(outcome), + Err(e) => break Err(e), + } + }; + // Dropping closes the connection, and the server releases the lock with the session, so + // nothing here depends on an unlock landing. + dropped +} + +async fn unlock_instance_database(conn: &mut sqlx::PgConnection, key: &str) -> error::Result<()> { + sqlx::query("SELECT pg_advisory_unlock(hashtext($1))") + .bind(key) + .execute(&mut *conn) + .await?; + Ok(()) +} + +/// What [`drop_unused_instance_database`] did, and for whom when it kept the database. +pub enum Cleanup { + Dropped, + /// Workspaces that name the database, as [`instance_database_users`] reports them. + InUse(Vec), + /// A transaction still blocked on the lock after the rounds above, so what it will commit + /// stays unknown, named as its `pid`. + Waiter(i32), +} + +async fn drop_if_unused_on(conn: &mut sqlx::PgConnection, dbname: &str) -> error::Result { + let users = instance_database_users(conn, dbname).await?; + if !users.is_empty() { + return Ok(Cleanup::InUse(users)); + } + // A settings save naming this database takes the same lock, so one waiting on it would read + // its own users after the drop and commit a reference to nothing. + if let Some(waiter) = waiting_for_instance_database(conn, dbname).await? { + return Ok(Cleanup::Waiter(waiter)); + } + drop_custom_instance_database_on(conn, dbname).await?; + Ok(Cleanup::Dropped) +} + +/// The `pid` of a transaction blocked on `dbname`'s instance-database lock. The lock is +/// taken by key, so `pg_locks` reports it split across `classid` and `objid`, and it lists every +/// database on the cluster — another Windmill on the same one holds its own locks under the same +/// key. +async fn waiting_for_instance_database( + conn: &mut sqlx::PgConnection, + dbname: &str, +) -> error::Result> { + let pid: Option = sqlx::query_scalar( + "SELECT l.pid FROM pg_locks l + WHERE l.locktype = 'advisory' AND NOT l.granted + AND l.database = (SELECT oid FROM pg_database WHERE datname = current_database()) + AND l.classid = ((hashtext('instance_database:' || $1)::bigint >> 32) & 4294967295)::oid + AND l.objid = (hashtext('instance_database:' || $1)::bigint & 4294967295)::oid + LIMIT 1", + ) + .bind(dbname) + .fetch_optional(&mut *conn) + .await?; + Ok(pid) +} + +/// The workspaces naming instance database `dbname` as a data table or a ducklake catalog. Taken +/// under [`lock_instance_databases`] for `dbname`, the answer holds until that lock is released. +/// +/// Authorization: none, and it reads every workspace's settings. Callers MUST pass only a database +/// their own request created, and may name the workspaces returned only to a caller allowed to +/// create or drop instance databases. +pub async fn instance_database_users( + conn: &mut sqlx::PgConnection, + dbname: &str, +) -> error::Result> { + Ok(sqlx::query_scalar( + "SELECT DISTINCT ws.workspace_id FROM workspace_settings ws + WHERE EXISTS (SELECT 1 FROM jsonb_each(CASE WHEN jsonb_typeof(ws.datatable->'datatables') = 'object' + THEN ws.datatable->'datatables' ELSE '{}'::jsonb END) dt + WHERE dt.value->'database'->>'resource_type' = 'instance' + AND dt.value->'database'->>'resource_path' = $1) + OR EXISTS (SELECT 1 FROM jsonb_each(CASE WHEN jsonb_typeof(ws.ducklake->'ducklakes') = 'object' + THEN ws.ducklake->'ducklakes' ELSE '{}'::jsonb END) dl + WHERE dl.value->'catalog'->>'resource_type' = 'instance' + AND dl.value->'catalog'->>'resource_path' = $1)", + ) + .bind(dbname) + .fetch_all(conn) + .await?) +} + /// Drop a custom instance database: validate, terminate connections, DROP DATABASE, remove from global_settings. /// /// Authorization: drops any instance database but Windmill's own and checks nothing. Callers MUST /// be superadmin, or have established the caller may drop this one — a fork's owner cleaning up /// its own copy that nothing else uses. pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Result<()> { - drop_custom_instance_database_keep_entry(db, dbname).await?; - sqlx::query!( - r#"UPDATE global_settings SET value = value #- ARRAY['databases', $1] WHERE name = 'custom_instance_pg_databases'"#, - dbname.trim() - ) - .execute(db) - .await?; - Ok(()) + drop_custom_instance_database_on(&mut *db.acquire().await?, dbname).await } /// [`drop_custom_instance_database`] leaving its registry entry, for a caller holding row locks in @@ -1505,6 +1646,29 @@ pub async fn drop_custom_instance_database(db: &DB, dbname: &str) -> error::Resu /// connection for a lock the transaction's own peers hold is a deadlock Postgres cannot see. Same /// authorization contract. pub async fn drop_custom_instance_database_keep_entry(db: &DB, dbname: &str) -> error::Result<()> { + drop_instance_database_keep_entry_on(&mut *db.acquire().await?, dbname).await +} + +async fn drop_custom_instance_database_on( + conn: &mut sqlx::PgConnection, + dbname: &str, +) -> error::Result<()> { + let dbname = dbname.trim(); + drop_instance_database_keep_entry_on(&mut *conn, dbname).await?; + // Always remove from global_settings + sqlx::query!( + r#"UPDATE global_settings SET value = value #- ARRAY['databases', $1] WHERE name = 'custom_instance_pg_databases'"#, + dbname + ) + .execute(&mut *conn) + .await?; + Ok(()) +} + +async fn drop_instance_database_keep_entry_on( + conn: &mut sqlx::PgConnection, + dbname: &str, +) -> error::Result<()> { let dbname = dbname.trim(); validate_dbname(dbname)?; @@ -1519,7 +1683,7 @@ pub async fn drop_custom_instance_database_keep_entry(db: &DB, dbname: &str) -> "SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_database WHERE datname = $1)", dbname ) - .fetch_one(db) + .fetch_one(&mut *conn) .await? .unwrap_or(false); @@ -1530,7 +1694,7 @@ pub async fn drop_custom_instance_database_keep_entry(db: &DB, dbname: &str) -> "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '{}' AND pid <> pg_backend_pid()", dbname.replace('\'', "''") )) - .execute(db) + .execute(&mut *conn) .await { tracing::warn!("Failed to terminate connections to '{}': {}", dbname, e); @@ -1539,7 +1703,7 @@ pub async fn drop_custom_instance_database_keep_entry(db: &DB, dbname: &str) -> // Drop the database // SAFETY: `dbname` has been validated via validate_dbname() before reaching this point. sqlx::query(&format!("DROP DATABASE IF EXISTS \"{}\"", dbname)) - .execute(db) + .execute(&mut *conn) .await .map_err(|e| { error::Error::internal_err(format!("Failed to drop database '{}': {}", dbname, e)) @@ -1623,6 +1787,44 @@ pub async fn create_custom_instance_database( error::Error::internal_err(format!("Failed to create database '{}': {}", dbname, e)) })?; + // Nothing names a database that failed past this point, and its name blocks the retry: drop it + // rather than leave it behind. + if let Err(e) = finish_custom_instance_database(db, dbname, tag, for_workspace).await { + match drop_unused_instance_database(db, dbname).await { + Ok(Cleanup::InUse(users)) => tracing::warn!( + "Kept '{dbname}' after failing to set it up: workspaces {} use it", + users.join(", ") + ), + Ok(Cleanup::Waiter(pid)) => tracing::warn!( + "Kept '{dbname}' after failing to set it up: a request (pid {pid}) is still \ + waiting to name it. Drop it from instance settings once it is unused." + ), + Ok(Cleanup::Dropped) => {} + Err(drop_err) => { + tracing::error!("Could not drop '{dbname}' after failing to set it up: {drop_err}") + } + } + return Err(e); + } + + // A data table role can only reach a database it may CONNECT to, and PUBLIC's default CONNECT + // would otherwise let every role in regardless of what this instance defines. Best-effort: a + // failure here leaves the database usable as `admin`, and the next role change repairs it. + if let Err(e) = crate::datatable_roles::converge_connect_grants(db, dbname).await { + tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}"); + } + + tracing::info!("Created custom instance database '{}'", dbname); + Ok(()) +} + +/// Grant `custom_instance_user` its privileges on a database just created, and register it. +async fn finish_custom_instance_database( + db: &DB, + dbname: &str, + tag: &str, + for_workspace: Option<&str>, +) -> error::Result<()> { // Grant permissions to custom_instance_user let wmill_pg_creds = PgDatabase::parse_uri(&get_database_url().await?.as_str().await)?; let new_pg_creds = PgDatabase { dbname: dbname.to_string(), ..wmill_pg_creds }; @@ -1658,15 +1860,6 @@ pub async fn create_custom_instance_database( ) .execute(db) .await?; - - // A data table role can only reach a database it may CONNECT to, and PUBLIC's default CONNECT - // would otherwise let every role in regardless of what this instance defines. Best-effort: a - // failure here leaves the database usable as `admin`, and the next role change repairs it. - if let Err(e) = crate::datatable_roles::converge_connect_grants(db, dbname).await { - tracing::warn!("Could not set CONNECT grants on instance database '{dbname}': {e}"); - } - - tracing::info!("Created custom instance database '{}'", dbname); Ok(()) } diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 7af411ade5..03441796cb 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -1304,6 +1304,12 @@ pub struct DataTable { /// nothing local for a fork admin to widen. #[serde(default, skip_serializing_if = "Option::is_none")] pub reference: Option, + /// Set on a *clone* — a terminal entry whose database was copied from the entry this names. The + /// copy holds that entry's rows, so who may connect as which role stays that entry's decision: + /// the clone carries no `permissions` of its own and is governed like a pointer, while + /// connecting to its own database. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub governed_by: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub forked_from: Option, /// Whether the SQL-migrations feature is opted in for this data table. @@ -1362,9 +1368,16 @@ pub const DATATABLE_TENANT_WILDCARD: &str = "*"; /// enough to survive a fork of a fork. const DATATABLE_REFERENCE_MAX_DEPTH: usize = 20; -/// Exactly one of `database` and `reference` must be set. Called wherever an entry is persisted, -/// so nothing downstream has to handle an entry that is both or neither. +/// Exactly one of `database` and `reference` must be set, and a clone's `governed_by` leaves no +/// `permissions` beside it. Called wherever an entry is persisted, so nothing downstream has to +/// handle an entry that is both or neither, or a clone with a decision of its own. pub fn validate_datatable_shape(name: &str, dt: &DataTable) -> Result<()> { + if dt.governed_by.is_some() && (dt.database.is_none() || dt.permissions.is_some()) { + return Err(Error::BadRequest(format!( + "Data table '{name}' is a clone, which owns a database and takes its roles from the \ + data table it was cloned from" + ))); + } match (&dt.database, &dt.reference) { (Some(_), None) | (None, Some(_)) => Ok(()), (Some(_), Some(_)) => Err(Error::BadRequest(format!( @@ -1464,23 +1477,28 @@ pub async fn read_datatable_entry(db: &DB, w_id: &str, name: &str) -> Result(datatable.clone())?) } -/// The terminal entry a reference chain lands on: the workspace that governs the data table, the -/// entry name there, and the entry itself. A terminal entry resolves to itself. +/// The terminal entry a reference chain lands on, and what governs it: the workspace and name of +/// the entry that owns the database, the entry itself, and — for a clone — the entry its +/// `governed_by` chain lands on. A terminal entry that is not a clone resolves to itself and +/// governs itself. /// -/// Every decision downstream — which database to connect to, whose `permissions` apply, whose -/// members tenants are evaluated against, who may administer it — is taken on this, never on the -/// entry the caller named. +/// Every decision downstream is taken on this, never on the entry the caller named. Which database +/// to connect to comes from `workspace_id` / `name` / `datatable.database`; whose `permissions` +/// apply (already in `datatable.permissions`), whose members tenants are evaluated against and who +/// may administer it come from [`GoverningDatatable::governing_workspace_id`]. /// /// Authorization: resolving deliberately crosses into the governing workspace, so it answers for a /// workspace the caller may not belong to and checks nothing itself. It is the input to the /// checks, not one of them: callers MUST pass what it returns to /// [`can_use_datatable_role_in_governing_workspace`] or [`ensure_datatable_admin_access`] before -/// acting on it, and MUST NOT return its `permissions` or `workspace_id` to a caller from +/// acting on it, and MUST NOT return its `permissions` or workspace ids to a caller from /// elsewhere without gating on the answer. pub struct GoverningDatatable { pub workspace_id: String, pub name: String, pub datatable: DataTable, + /// For a clone, the entry whose `permissions` govern it. `None` when the entry governs itself. + pub governor: Option, } /// Everything still using the Windmill-managed database `dbname`, one description per use: data @@ -1559,6 +1577,13 @@ impl GoverningDatatable { .as_ref() .is_some_and(|d| d.resource_type == DataTableCatalogResourceType::Instance) } + + /// The workspace whose admins administer the data table and whose members its tenants are. + pub fn governing_workspace_id(&self) -> &str { + self.governor + .as_ref() + .map_or(&self.workspace_id, |g| &g.workspace_id) + } } pub async fn resolve_governing_datatable( @@ -1569,6 +1594,9 @@ pub async fn resolve_governing_datatable( let mut workspace_id = w_id.to_string(); let mut name = name.to_string(); let mut hops = 0; + // The clone a `governed_by` chain started from: it keeps its database, and takes the + // `permissions` of wherever the chain lands. + let mut clone: Option<(String, String, DataTable)> = None; for _ in 0..DATATABLE_REFERENCE_MAX_DEPTH { let datatable = read_datatable_entry(db, &workspace_id, &name) .await @@ -1579,21 +1607,48 @@ pub async fn resolve_governing_datatable( // A pointer outlives the workspace it names: deleting one only nulls the fork // lineage, it does not sweep the entries that pointed at it. Say which one is // gone rather than reporting a data table this workspace never had. - Error::NotFound(format!( - "Data table '{name}' of workspace '{workspace_id}' governs this one and no \ - longer exists. A superadmin can point this data table somewhere else." - )) + if clone.is_some() { + Error::NotFound(format!( + "Data table '{name}' of workspace '{workspace_id}', which this clone \ + takes its roles from, no longer exists, so nobody is let into the copy." + )) + } else { + Error::NotFound(format!( + "Data table '{name}' of workspace '{workspace_id}' governs this one and \ + no longer exists. A superadmin can point this data table somewhere \ + else." + )) + } } })?; hops += 1; validate_datatable_shape(&name, &datatable)?; - match &datatable.reference { - None => return Ok(GoverningDatatable { workspace_id, name, datatable }), - Some(reference) => { - workspace_id = reference.workspace_id.clone(); - name = reference.datatable.clone(); + let next = match (&datatable.reference, &datatable.governed_by) { + (Some(reference), _) => reference.clone(), + (None, Some(governed_by)) => { + let governed_by = governed_by.clone(); + if clone.is_none() { + clone = Some((workspace_id.clone(), name.clone(), datatable)); + } + governed_by } - } + (None, None) => { + return Ok(match clone { + None => GoverningDatatable { workspace_id, name, datatable, governor: None }, + Some((clone_w_id, clone_name, mut clone_datatable)) => { + clone_datatable.permissions = datatable.permissions; + GoverningDatatable { + workspace_id: clone_w_id, + name: clone_name, + datatable: clone_datatable, + governor: Some(DataTableReference { workspace_id, datatable: name }), + } + } + }); + } + }; + workspace_id = next.workspace_id; + name = next.datatable; } Err(Error::BadRequest(format!( "Data table '{name}' points at another data table through more than \ @@ -1633,17 +1688,20 @@ pub async fn resolve_workspace_governing_datatables( let mut entries = Entries::new(); let listed = load(db, &[w_id.to_string()], &mut entries).await?; - // (index into `listed`, workspace, entry name) still to be followed. - let mut cursors: Vec<(usize, String, String)> = listed + // (index into `listed`, workspace, entry name, the clone the chain started from) still to be + // followed. A clone keeps its own database and takes the `permissions` of wherever its + // `governed_by` chain lands, as the single resolution does. + type Clone = Option<(String, String, DataTable)>; + let mut cursors: Vec<(usize, String, String, Clone)> = listed .iter() .enumerate() - .map(|(i, name)| (i, w_id.to_string(), name.clone())) + .map(|(i, name)| (i, w_id.to_string(), name.clone(), None)) .collect(); let mut resolved: Vec<(usize, GoverningDatatable)> = vec![]; for _ in 0..DATATABLE_REFERENCE_MAX_DEPTH { let mut next = vec![]; - for (i, ws, name) in cursors.drain(..) { + for (i, ws, name, clone) in cursors.drain(..) { let Some(value) = entries .get(&ws) .and_then(|m| m.get(&name)) @@ -1657,14 +1715,41 @@ pub async fn resolve_workspace_governing_datatables( if validate_datatable_shape(&name, &datatable).is_err() { continue; } - match &datatable.reference { - None => { - resolved.push((i, GoverningDatatable { workspace_id: ws, name, datatable })) - } - Some(reference) => next.push(( + match (&datatable.reference, &datatable.governed_by) { + (Some(reference), _) => next.push(( i, reference.workspace_id.clone(), reference.datatable.clone(), + clone, + )), + (None, Some(governed_by)) => { + let (governor_ws, governor_name) = + (governed_by.workspace_id.clone(), governed_by.datatable.clone()); + let clone = clone.or(Some((ws, name, datatable))); + next.push((i, governor_ws, governor_name, clone)); + } + (None, None) => resolved.push(( + i, + match clone { + None => GoverningDatatable { + workspace_id: ws, + name, + datatable, + governor: None, + }, + Some((clone_ws, clone_name, mut clone_datatable)) => { + clone_datatable.permissions = datatable.permissions; + GoverningDatatable { + workspace_id: clone_ws, + name: clone_name, + datatable: clone_datatable, + governor: Some(DataTableReference { + workspace_id: ws, + datatable: name, + }), + } + } + }, )), } } @@ -1673,7 +1758,7 @@ pub async fn resolve_workspace_governing_datatables( } let to_load: Vec = next .iter() - .map(|(_, ws, _)| ws.clone()) + .map(|(_, ws, _, _)| ws.clone()) .filter(|ws| !entries.contains_key(ws)) .collect::>() .into_iter() @@ -1751,8 +1836,31 @@ pub async fn get_datatable_resource_from_db_unchecked( w_id: &str, name: &str, ) -> Result { + Ok(get_datatable_connection_and_kind_unchecked(db, w_id, name) + .await? + .0) +} + +/// As [`get_datatable_resource_from_db_unchecked`], also reporting the kind of database Windmill +/// manages behind it, `None` for a user resource. One resolution answers both: a caller reading the +/// kind separately can be handed one kind's connection and the other kind's checks by a save +/// landing between the two, and the entry a pointer lands on is another workspace's to change. +/// +/// Same authorization contract: the connection reaches every role. +pub async fn get_datatable_connection_and_kind_unchecked( + db: &DB, + w_id: &str, + name: &str, +) -> Result<(serde_json::Value, Option)> { let governing = resolve_governing_datatable(db, w_id, name).await?; - resolve_datatable_connection_unchecked(db, &governing, false).await + let kind = governing + .datatable + .database + .as_ref() + .map(|d| d.resource_type) + .filter(|kind| kind.is_windmill_managed()); + let connection = resolve_datatable_connection_unchecked(db, &governing, false).await?; + Ok((connection, kind)) } /// Same as [`get_datatable_resource_from_db_unchecked`] but for postgres trigger @@ -3445,6 +3553,7 @@ mod tests { resource_path: "dt_main".to_string(), }), reference: None, + governed_by: None, forked_from: None, migrations_enabled: None, permissions: None, diff --git a/backend/windmill-common/tests/fixtures/guest_jwt_rsa8192_private.pem b/backend/windmill-common/tests/fixtures/guest_jwt_rsa8192_private.pem new file mode 100644 index 0000000000..bb443cf6bf --- /dev/null +++ b/backend/windmill-common/tests/fixtures/guest_jwt_rsa8192_private.pem @@ -0,0 +1,100 @@ +-----BEGIN PRIVATE KEY----- +MIISQgIBADANBgkqhkiG9w0BAQEFAASCEiwwghIoAgEAAoIEAQC6deMC6KWwkWV2 +9xb4bXLt6ceNGQD3gJH3sYqpulWsZB0L3TdqXwnzL3C/8Uf3K8hLZ4Tw+llHCxaS +Z7aTEYyeLDJuYuJPVS8DAgVnXDluoDg7B1UtvoUa1/MjLQm4f/Qp322i7l0SH9kx +NRb7UZp/xx1tL/kiq9NIv1inK4ZC0DbleybBOoLlwP9Zqhazw3MJdZAyey4FAXTx +bZcjG3UtiZ2COXVP1qdtqhLrQQQsicoLqZuxAfXdIU6xgcYgGapVQNTVbo7w8pHn ++6RzlPb9rqyBs2IiKLFSb467mtZeM9dfbyD4DICy42O/JKAbTZw/f3trbs+tu+sK +rj7ysk+lGzuXj2/irOuvOSbzpxERdZuDyLOhlMej5+aJXkrmVhVPf/3wt/6oKCzo +7Ta24OvGaz6pxK1iDOQvzkCqJpaTjSfpcscHyU3ztXCUINtsq7s78XoOxJZkerfq +LDxuutYviFiB9pQDzYdErL8XMUkRXXY22hpnLHt8GV+lNCJs2KxcECzkgwfjIn9X +Y66U4iO+k9EvY/FHyI25ur0eKWDnJAPL6PypVhx9NOuBkot6nwUugzWzM/MKJ9Hf +1n9pwVNlMam8zgMQ5Ivu3+RiOV3P0iuAaw7d1sZRSSX11l/EyjGFDpwmnFNjleEK +vnop6rCftWAsd2XJHXiD1/80QIgKJHdflpHOcy6Su5w/CRYc/jC2rJjUr9W+hBwg +fnayip8MTMa4cAXzbFmSc5NCzSsqHkVq/4RdfdMephubGmWcPDQnkUf/n/RvoZNi +VktW34lX3YIe97600vSWPfWahj/EUnfI8m+4Qzuz2o6N9basCNftKPDIc5ej3PMS +RVIgBhEXUh1wVFdEUs8RKHg/QV0xrN2qbKcS8XvV3fFfLuFZo4a66nZ5zgGGJUgL +ED41cEqf/sQObfhGgIw25v/315ARiv9thRF3tUMLGQZNQ8tSs2VMuU6szBQciplx +y2y4JzXdv+G0PvheE7tVdsGsMipSHTZDdNZo51MOBSgsNTyNmWMZlsfoZTTZ2o2y +XtKv8xUP01KnlwR6Z8hY1CAPxM1QKDU5nKp9T/df6GEhohps9gSt6cs6SHlH8piq +pyR0xiDvS+vcPN/IbOaRgE9WmfM5py72SNZbetiUHdUTmAC1dd0IH3opte1uVpQQ +sauKVWZuzV8Bcd2O06QFOVJDwoxm00YaF0rqhaisppah5eQMN/ctlqWVDHXR8laV +CNkzWYE/jxAU9dGuCMCIZ7BSs5U9mZ7pWstagXWEFTI9IugjvNnaZDijZQJSPmAZ +gQtAxH/z8R9e/PihW9dDQc/nKBC9Lpta3oFGq74fc7Y+Wu/Uskjf8qtFOOVrvfZx +kwlRR0s5AgMBAAECggQAEEeerHBXlxT6wOpN7R7RPAh/TchzFyj4laHAs5T7SfPr +vU9xOS5XFg58T6XnXKNL95lwOAIDrXpAXEQF4s1Tc7P0K0Mi8z5CvoZ8pkudEBWZ +Ez5Op5HM2U0lg1q5UIyLauSBFAqvwZdaRTuXaBX14VYnMZNQs0BBUEa0nNPmAwte +36Zi+df7+cdiBBgfp05XSuoizuWGXRWFelbRLVWGY5VyVR5jKbdaIu8Ie9XPYXwe +UepbF5dOOiJgVos4GzFid9MTQhdHD65+Ir0L3LOS5Q9NLH3XAZh/8uyej66zLMJl ++YUgYcflrLjByryN5hReogSlZ/ni1A5mNSaFhHchEQ9Roqf5YtcqNMBOsFNT4Cxe +iaEKXf85dwwWSsQa0gWOEsTgDRZje5cWZLfhYRI+Q7K1Y+kyIi9x9OKTLsfzuapH +Y9CLpBhlX/Plgmlj/xb3YAJzX7q2TsuQJzf5hsl1Te0fnSlEPxjAyLQ0cvfwoz1/ +xsZAK+XsWid0/F2S6+H3GoqnAMSX+bLXX2/qnEEqUcdiKjsVo+jUxvSQ8oM2XLf+ +RN1wwfAsyJnIPiXnW6tqrmbyV1Uf0TEs19hIg5lHiLxPT5bFGcJ6nT6bGCBeGnkF +J08xmxjXc91pHQ+8iNgGftGJvK1FcppxbZi33dUJ2sfhP8Ck+GxES26eb5d6yTfD +oZRQmESvikHPU5shm+YbjPrYhEbEgLZKO3wUhCAvoFLN2TG0i2v//aaCSI/XfNWt +1XyE+uN/o6kQEPvaUy/0uHBuzoC4pZ8or22ywmeftCOHuT9YE9LIJuU81qMJvjP7 +BJ6YkcTO6G89iD0g6HHYRiD9I+pgih8B1NPq4VmFuJrvaqgAX9srmmMt7FXBrcqa +JSyLS+WeHTLlOiHN1DqOgpgq0gq6vC1i53xPqSUkFzAKpDOWgGNMVAABFN9EOVmM +zvPnJThz5PHDAuud9eaLjZ0V76IVXmRTz22warkdKhBW5BzDzy+zNp8mF5SMH9mZ ++JQvCGTc2itiKGKMDzvY99pBzHRlrlFL7n5SHLqxclBfnEJcU9wPvFyZ+6k8MGim +0kFrK63NqiceNWp5vdU/6V+boCYoFG8PDbABUxCbb+lhtnYx5AUcKfKX74Uv4gpX +AM0gsaY8U+4ATJbgoCTqplTKvXtyw4zcpJAXCMnhygKyDKNOk+Lckfquwx9r/TQN +hbbjjvwgBkzJtAlylCVYYEJ/03lazRpGR5osCksECxNU4TMDRD2Hd+zwUxdeRunm +7EusRSrKq8swi8LFyV7E8ISwwXegqVarHqXjCHhcn/5Ozw6PkVk39Ug2UE4ibesC +MzVP3lfP67gXgGCzYrImIUuqHGTyLFs8/B8+2OmdyQKCAgEA6PCozdeeQS0P/+ah +E015KECxB0aGbrvdpczXX5szHYb8KhejvHmVV2OLFisL7GyMeBhEZXAeKtMZwd66 +FMORgdFACpckR/a/aVEnebHx5fOKtAQfhLLH/3YE5dFIWxu9SKNZBrX0lIBFXfoK +lZYM7cGm7vN55AK99bYfCAqFs+HI9sDQLjbaxR41UOfZwPjA9VjBQ1vBr0N3UoOT +xf8Rw6hlAd+ED7iJavn6KUb64D5eB4eGSZNXM5HCp2ZtUFN/4bU5lbHPCSK29+Wd +mM5flZtIKQ7PzGyXqv4Kbtz9o1hPOQPeH0faDEcYJLHV/LAxHc1AkqJDXU8HANhI +m/XI44RlJesNntOdJuCumHtTAsreWAYoydXurOWHkDIpx6OLi0F3eJrOV6dhwCCG +v3bX4PIBjYZKyfHukcoCzGetXpu5bDKC1uwwZLJmF/QTO3AeVpWNrbWkBfj/PCio +8RodjVlPkDaoCrDmpca22fNo+93IM2m1SWdSnTlXvgg58cLUrlMUbg24Lo1cyySF +1AnRw0uvaFLdH8iMtKZIA3zgCZyUKV1DcGUKsdcL/9IYD6EO5bCcA6p5gcxxBOan +6MEHIuJKycmZMGutN5rm3uDHQgmNCs25ma3fWQF0s/dRJ2lg947qKUKevqBkd4gt +BuB8VXUGsHXGDWHfz/eLjWYA3k8CggIBAMzrTqnmT27U1SXPwEXI0kc6A44D8skD +a4rqxOMBTGvu+HeVdEBMcxYnGeJV2QtZY96KXwadiWuqdEr4Pc7JWnAa2f9yWNCb +QI0YJSWSNcDuPsSV1K15MM7uxKk/oNcXdbjrAfVLzCmEYDg085QV80rl7Q6WRaYv +acvipeZY82LuGhtAJAN6S8VWmDqZ5vBDd61F09AlBrpIFoSBRnYVkEwaXXwFJauz +S+kEmCIb97tBf7nr08ldoNvDdecRtm+s9odfUhm+ONroZf7OEyTCOXB666oDogpI +hktuliNUVpXeZ5QtkzpxP2SBbHNWZ9LiAGpTv2wa7z7k1PqCVR6ABGMkTkge41iy ++v82KYi4qRnUfVlFcn9xLyBDaaCWJYrsux8OmAW48cVN1Og0tKu/T+bFUvHs0etZ +oijYoYZaf9ERO0X0tuMXKIuT0QcPnkcT+KXFY6uzOC1UmF5ITCeKogGwzkFbvInA +w8OiFYBkgXZ41tE/oOARm1AhuaNc/ZfWujLECjuJgvw0uhdUyihGw8ZJv91WiIjX +Jqq1BNrhvDQqTPSnDvhcBBDGdQA6HJRGAerIx7PgCKzS2WbrfmsFzjfZ3zbrUSNf +e8CM37aDzte25g7HmvFuwOLmE9DXNnE+cWGpb/S1l4ACahnqKF9zj3uKVpN1kG+l +xFH+ADFgNCP3AoICAQDW+fuf9QVdezwP3IX9bXuKVaHLaB/7JDBbqt99oZsb/g7M +vgJjmAXmn7ymGdYoPMmqsCK3RzDgLvDPhkFPlmm8WCR1YaWfMK7YgIgg/MOViB0T +I9b+vYxsTDwa6PmQ89nNpssrsVNmlbbvEysfgFqwNg9c6aEteAZeNNUQs71PFOpG +KA45CHEAiG+GGmNRf4F9LIs4If9vQ6Fkqn//s/ZuDwQhJWoYpCNWchuYaqRvvb9D +zaR2VJ+evT/bBGGQR293G6xoZa//62TXBbdbZeK7DJMdKUlHjVSv/0IbrKH8VSzQ +HqypS8OFnzs96k3lpam+qkWUzmHDRILfGbxxCvV6XB6JTxPD5+JaFfOsT+x9YEKx +FAYit+hM8owY3OLIFzxaqXGcAaq7JIPHL32MSB7VwCMEvNJa64dUTw4kHO7YqHkI +zNkJhvLmi+pC4wO40DF/kQgvL7qPtIvBtnisAoERiUIvgvG05CwJBIqOPOE/yR/R +qxYfB1csONasT6VCZJ+pvozVQJ4hgF8KL8u+s80+IqRgcFEY2xpYQjml1mJhHHt0 +bVkm6mljVWcCkp+YFPjNrB9gFr2Kt+X6MvD1kueGHzgMYJybHlS/Q5scSHA8ltnv +BYvrs5az6u8MhAwGhm9yKRgcOhqNLTNrSWbcuOKmb5lb02WTxicrJHdLaL8WawKC +AgAvXHdK51MWj5RP7+tFdbu8AJmPujPj56l0avxkZwltrMa6hM1giwrSZOkjmanX +aKqfZaW+1qxLORM1vJg0ce84F8yosJn8ZNWPhrQXMerAk4/oSaTlDyyLtz22zGD1 +NZPc0FwcgI9ITTMb6okdgdFAT1Hx3CNu3Gh8Wf7z9T0TcWqRP8rTDvHyQyhd6vIo +N7+ySYBCDDy40y55I5PQeP1nyXQv1NS3EXKLAafTwlkIa4QEyY+ia8Rg5pUaecqL +lxNHiG1mS8PCPJJOSPy9szIpYA+VtM8/xzFav81tjvGQI94Wrk3n5QK8zPju01qd +r+RersqARt1i0Oq0l2jq6rWY4GxDWPg8qOlAERmaYuuab7Uk69shv1wf/dSs6+bz +a/F3pRzXvb4LVwjNPlk/lLD36kVYat69oXLMM/1pGwONo4jqizu10sVpNRnLov12 ++hOu5Kc7qOlj+LhamiUXwn/dGishif1SbT4RuiV0DkapxhYIOMeK4ibeZ10Ld9OY +w2U3lH208vxWJC2adNtgYbKf4q24j2ZxEiD6tWnQ2PlxqREuPmpEVqgN5IBoNAYk +8uVAiXU2iqIWp4jds7H9NFNML0va2ppfLZNhlm2iNmaZDsmvQsz53howlvKfgw4v +MDpuUwXwLgQ87tHs+WSpYwEXkYqe/D4Mv+hW9vi+cLi8UwKCAgAsehP6DOrt9bA3 +pVdtHJG1a9fm/rXC9TQS2oSE2paXRZpxhbk917Gdl2njo7tnoGXXBpQOb51/HUXX +QTARdp2LP1VFn9RJP3CwaK+Sxs1IBa9r1/p2ePDR85QR1HxN+Gj98EcEfKl1RSe6 +RmLOkLIg4pkNrDhwWyYJ1H5//d/yUQlZ015fQAKaHCdaoz7L3+0jOyFXDGrDr+Gc +680SyVqPfRx5dAm8TrfyXW6Q2uZEhIqxbcxTdG98Pxi0qLLAS/nD16IHC1iljHsI +QrfUvFvGg5v7kxeXrP0VPMRM0/Y8vno3Bbi04I0Pk3Wko8UhWuiUMjs+TVbEPKox +8zG/G+R5EpHKDxNyfUUwZ9+lyjFmBZOcVd3dze23VQYCzV/W04yiQdEl2C/sED2m +NycJ3GN/ByqpJJZY3u+B/WAiHPLe1ivIEeIjwdBt2w/3L/rKoRBR/P8LSRXLIIh4 +Xk1BMNygJxHTwsvFGhflx89+1BO/n8HrR0s/2G1QanxGdgso9bwmXMDrmJ+XxrUY +qBmDCU8RkBdUHKOuJlWWn2JaIswsEE5kAk7MgNNaBTv91VBYCDs50NW8VBaKnJjN +j+hXI80qKi7WqeoonFKFLDgUYbKnmzKsX+T0x/H99v+7Or5PmAPN+hRwj54/LEVR +MsjrTlnoCGXKgPQQ5ItWviGjqoCTiQ== +-----END PRIVATE KEY----- diff --git a/backend/windmill-common/tests/fixtures/guest_jwt_rsa8192_public.pem b/backend/windmill-common/tests/fixtures/guest_jwt_rsa8192_public.pem new file mode 100644 index 0000000000..44645f5af2 --- /dev/null +++ b/backend/windmill-common/tests/fixtures/guest_jwt_rsa8192_public.pem @@ -0,0 +1,25 @@ +-----BEGIN PUBLIC KEY----- +MIIEIjANBgkqhkiG9w0BAQEFAAOCBA8AMIIECgKCBAEAunXjAuilsJFldvcW+G1y +7enHjRkA94CR97GKqbpVrGQdC903al8J8y9wv/FH9yvIS2eE8PpZRwsWkme2kxGM +niwybmLiT1UvAwIFZ1w5bqA4OwdVLb6FGtfzIy0JuH/0Kd9tou5dEh/ZMTUW+1Ga +f8cdbS/5IqvTSL9YpyuGQtA25XsmwTqC5cD/WaoWs8NzCXWQMnsuBQF08W2XIxt1 +LYmdgjl1T9anbaoS60EELInKC6mbsQH13SFOsYHGIBmqVUDU1W6O8PKR5/ukc5T2 +/a6sgbNiIiixUm+Ou5rWXjPXX28g+AyAsuNjvySgG02cP397a27PrbvrCq4+8rJP +pRs7l49v4qzrrzkm86cREXWbg8izoZTHo+fmiV5K5lYVT3/98Lf+qCgs6O02tuDr +xms+qcStYgzkL85AqiaWk40n6XLHB8lN87VwlCDbbKu7O/F6DsSWZHq36iw8brrW +L4hYgfaUA82HRKy/FzFJEV12NtoaZyx7fBlfpTQibNisXBAs5IMH4yJ/V2OulOIj +vpPRL2PxR8iNubq9Hilg5yQDy+j8qVYcfTTrgZKLep8FLoM1szPzCifR39Z/acFT +ZTGpvM4DEOSL7t/kYjldz9IrgGsO3dbGUUkl9dZfxMoxhQ6cJpxTY5XhCr56Keqw +n7VgLHdlyR14g9f/NECICiR3X5aRznMukrucPwkWHP4wtqyY1K/VvoQcIH52soqf +DEzGuHAF82xZknOTQs0rKh5Fav+EXX3THqYbmxplnDw0J5FH/5/0b6GTYlZLVt+J +V92CHve+tNL0lj31moY/xFJ3yPJvuEM7s9qOjfW2rAjX7SjwyHOXo9zzEkVSIAYR +F1IdcFRXRFLPESh4P0FdMazdqmynEvF71d3xXy7hWaOGuup2ec4BhiVICxA+NXBK +n/7EDm34RoCMNub/99eQEYr/bYURd7VDCxkGTUPLUrNlTLlOrMwUHIqZcctsuCc1 +3b/htD74XhO7VXbBrDIqUh02Q3TWaOdTDgUoLDU8jZljGZbH6GU02dqNsl7Sr/MV +D9NSp5cEemfIWNQgD8TNUCg1OZyqfU/3X+hhIaIabPYErenLOkh5R/KYqqckdMYg +70vr3DzfyGzmkYBPVpnzOacu9kjWW3rYlB3VE5gAtXXdCB96KbXtblaUELGrilVm +bs1fAXHdjtOkBTlSQ8KMZtNGGhdK6oWorKaWoeXkDDf3LZallQx10fJWlQjZM1mB +P48QFPXRrgjAiGewUrOVPZme6VrLWoF1hBUyPSLoI7zZ2mQ4o2UCUj5gGYELQMR/ +8/EfXvz4oVvXQ0HP5ygQvS6bWt6BRqu+H3O2Plrv1LJI3/KrRTjla732cZMJUUdL +OQIDAQAB +-----END PUBLIC KEY----- diff --git a/cli/src/commands/workspace/fork.ts b/cli/src/commands/workspace/fork.ts index 219252bb7a..0aca0984ff 100644 --- a/cli/src/commands/workspace/fork.ts +++ b/cli/src/commands/workspace/fork.ts @@ -239,6 +239,7 @@ async function createWorkspaceFork( interface ForkedDatatableInfo { name: string; new_dbname: string; + fork_behavior?: "schema_only" | "schema_and_data"; } const forkedDatatables: ForkedDatatableInfo[] = []; @@ -285,6 +286,23 @@ async function createWorkspaceFork( const newDbName = `${trueWorkspaceId.replace(/-/g, "_")}__${dt.name}`; + const forkBehavior = dtBehavior as "schema_only" | "schema_and_data"; + // A server that reports `permissioned` copies each data table in the fork request itself, + // and refuses a database copied beforehand. An older one only takes a database copied here. + if (typeof dt.permissioned === "boolean") { + log.info( + colors.blue( + ` Datatable "${dt.name}" will be cloned (${forkBehavior === "schema_only" ? "schema" : "schema + data"}) into "${newDbName}" when the fork is created.` + ) + ); + forkedDatatables.push({ + name: dt.name, + new_dbname: newDbName, + fork_behavior: forkBehavior, + }); + continue; + } + try { log.info( colors.blue(` Creating database "${newDbName}" for datatable "${dt.name}"...`) @@ -300,7 +318,7 @@ async function createWorkspaceFork( log.info( colors.blue( - ` Importing ${dtBehavior === "schema_only" ? "schema" : "schema + data"}...` + ` Importing ${forkBehavior === "schema_only" ? "schema" : "schema + data"}...` ) ); @@ -310,7 +328,7 @@ async function createWorkspaceFork( source: `datatable://${dt.name}`, target: `datatable://${dt.name}`, target_dbname_override: newDbName, - fork_behavior: dtBehavior as "schema_only" | "schema_and_data", + fork_behavior: forkBehavior, }, }); @@ -336,6 +354,8 @@ async function createWorkspaceFork( id: trueWorkspaceId, name: opts.createWorkspaceName ?? workspaceName ?? trueWorkspaceId, color: forkColor, + // So a clone the fork would refuse is refused before any branch is created. + forked_datatables: forkedDatatables, }, }); if (gitSyncJobIds && gitSyncJobIds.length > 0) { diff --git a/frontend/package-lock.json b/frontend/package-lock.json index f0c3ca61c5..0427688011 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -19,8 +19,10 @@ "@codingame/monaco-vscode-standalone-json-language-features": "=25.0.0", "@codingame/monaco-vscode-standalone-languages": "=25.0.0", "@codingame/monaco-vscode-standalone-typescript-language-features": "=25.0.0", + "@floating-ui/core": "^1.3.1", "@json2csv/plainjs": "^7.0.6", "@leeoniya/ufuzzy": "^1.0.8", + "@melt-ui/svelte": "^0.86.2", "@popperjs/core": "^2.11.8", "@redocly/json-to-json-schema": "^0.0.1", "@scalar/openapi-parser": "^0.15.0", @@ -29,6 +31,7 @@ "@windmill-labs/svelte-dnd-action": "^0.9.44", "@xterm/addon-fit": "^0.10.0", "@xyflow/svelte": "^1.0.0", + "@zerodevx/svelte-toast": "^0.9.6", "acorn": "^8.15.0", "ag-charts-community": "^9.0.1", "ag-charts-enterprise": "^9.0.1", @@ -39,6 +42,7 @@ "chartjs-adapter-date-fns": "^3.0.0", "chartjs-plugin-zoom": "^2.0.0", "clone": "^2.1.2", + "d3-dag": "^0.11.5", "d3-zoom": "^3.0.0", "date-fns": "^2.30.0", "diff": "^8.0.3", @@ -73,9 +77,17 @@ "rehype-sanitize": "^6.0.0", "rfc4648": "^1.5.3", "runed": "^0.36.0", + "style-to-object": "^0.4.1", + "svelte": "^5.55.7", + "svelte-awesome-color-picker": "^3.0.4", "svelte-carousel": "^1.0.25", "svelte-exmarkdown": "^5.0.0", + "svelte-floating-ui": "^1.5.8", + "svelte-highlight": "^7.6.0", "svelte-infinite-loading": "^1.4.0", + "svelte-popperjs": "^1.3.2", + "svelte-range-slider-pips": "^2.3.1", + "svelte-splitpanes": "^8.0.9", "tailwind-merge": "^1.13.2", "unist-util-visit": "^5.0.0", "vscode": "npm:@codingame/monaco-vscode-extension-api@=25.0.0", @@ -107,10 +119,8 @@ "zod": "^4.1.10" }, "devDependencies": { - "@floating-ui/core": "^1.3.1", "@hey-api/openapi-ts": "^0.43.0", "@melt-ui/pp": "^0.3.2", - "@melt-ui/svelte": "^0.86.2", "@playwright/test": "^1.57.0", "@sveltejs/adapter-static": "^3.0.6", "@sveltejs/kit": "^2.69.1", @@ -124,10 +134,8 @@ "@types/vscode": "^1.83.5", "@typescript-eslint/eslint-plugin": "^5.59.8", "@typescript-eslint/parser": "^5.60.0", - "@zerodevx/svelte-toast": "^0.9.6", "autoprefixer": "^10.4.13", "cssnano": "^6.0.1", - "d3-dag": "^0.11.5", "dts-bundle-generator": "^9.5.1", "eslint": "^8.47.0", "eslint-config-prettier": "^8.6.0", @@ -141,18 +149,10 @@ "postcss-load-config": "^4.0.1", "prettier": "^3.1.0", "prettier-plugin-svelte": "^3.3.3", - "style-to-object": "^0.4.1", "stylelint-config-recommended": "^13.0.0", - "svelte": "^5.55.7", - "svelte-awesome-color-picker": "^3.0.4", "svelte-check": "^4.4.3", "svelte-fast-check": "^0.4.5", - "svelte-floating-ui": "^1.5.8", - "svelte-highlight": "^7.6.0", - "svelte-popperjs": "^1.3.2", "svelte-preprocess": "^6.0.0", - "svelte-range-slider-pips": "^2.3.1", - "svelte-splitpanes": "^8.0.9", "tailwindcss": "^3.4.1", "tar": "^7.5.18", "tslib": "^2.6.1", @@ -1219,7 +1219,6 @@ "version": "1.7.3", "resolved": "https://registry.npmjs.org/@floating-ui/core/-/core-1.7.3.tgz", "integrity": "sha512-sGnvb5dmrJaKEZ+LDIpguvdX3bDlEllmv4/ClQ9awcmCZrlx5jQyyMWFM5kBI+EyNOCDDiKk8il0zeuX3Zlg/w==", - "dev": true, "license": "MIT", "dependencies": { "@floating-ui/utils": "^0.2.10" @@ -1229,7 +1228,6 @@ "version": "1.7.4", "resolved": "https://registry.npmjs.org/@floating-ui/dom/-/dom-1.7.4.tgz", "integrity": "sha512-OOchDgh4F2CchOX94cRVqhvy7b3AFb+/rQXyswmzmGakRfkMgoWVjfnLWkRirfLEfuD4ysVW16eXzwt3jHIzKA==", - "dev": true, "license": "MIT", "dependencies": { "@floating-ui/core": "^1.7.3", @@ -1240,7 +1238,6 @@ "version": "0.2.10", "resolved": "https://registry.npmjs.org/@floating-ui/utils/-/utils-0.2.10.tgz", "integrity": "sha512-aGTxbpbg8/b5JfU1HXSrbH3wXZuLPJcNEcZQFMxLs3oSzgtVu6nFPkbbGGUvBcUjKV2YyB9Wxxabo+HEH9tcRQ==", - "dev": true, "license": "MIT" }, "node_modules/@glideapps/ts-necessities": { @@ -1344,7 +1341,6 @@ "version": "3.10.0", "resolved": "https://registry.npmjs.org/@internationalized/date/-/date-3.10.0.tgz", "integrity": "sha512-oxDR/NTEJ1k+UFVQElaNIk65E/Z83HK1z1WI3lQyhTtnNg4R5oVXaPzK3jcpKG8UHKDVuDQHzn+wsxSz8RP3aw==", - "dev": true, "license": "Apache-2.0", "dependencies": { "@swc/helpers": "^0.5.0" @@ -1576,7 +1572,6 @@ "version": "0.86.6", "resolved": "https://registry.npmjs.org/@melt-ui/svelte/-/svelte-0.86.6.tgz", "integrity": "sha512-Jer+M7DgIwT5IHfTayb4Iw/fkkxWNmC/mqn/nMh9JrbPbkxmyabfLQnhJ+JDn5HK77f84j34lubO3iqFtYAfMg==", - "dev": true, "license": "MIT", "dependencies": { "@floating-ui/core": "^1.3.1", @@ -2160,7 +2155,6 @@ "version": "0.5.17", "resolved": "https://registry.npmjs.org/@swc/helpers/-/helpers-0.5.17.tgz", "integrity": "sha512-5IKx/Y13RsYd+sauPb2x+U/xZikHjolzfuDgTAl/Tdf3Q8rslRvC19NKDLgAJQ6wsqADk10ntlv08nPFw/gO/A==", - "dev": true, "license": "Apache-2.0", "dependencies": { "tslib": "^2.8.0" @@ -3146,7 +3140,6 @@ "version": "0.9.6", "resolved": "https://registry.npmjs.org/@zerodevx/svelte-toast/-/svelte-toast-0.9.6.tgz", "integrity": "sha512-nHlTrCjverlPK9yukK6fqbG3e/R+f10ldrc4nJHOe2qNDScuPTuYVSFEk2dDDtzWAwTN5pmdEXgA3M2RbT8jiw==", - "dev": true, "license": "ISC", "peerDependencies": { "svelte": "^3.57.0 || ^4.0.0 || ^5.0.0" @@ -4134,7 +4127,6 @@ "version": "2.9.3", "resolved": "https://registry.npmjs.org/colord/-/colord-2.9.3.tgz", "integrity": "sha512-jeC1axXpnb0/2nn/Y1LPuLdgXBLH7aDcHu4KEKfqw3CUhX7ZpfBSlPKyqXE6btIgEzfWtrX3/tyBCaCvXvMkOw==", - "dev": true, "license": "MIT" }, "node_modules/combined-stream": { @@ -4660,7 +4652,6 @@ "version": "0.11.5", "resolved": "https://registry.npmjs.org/d3-dag/-/d3-dag-0.11.5.tgz", "integrity": "sha512-sNHvYqjzDlvV2fyEkoOCSuLs2GeWliIg7pJcAiKXgtUSxl0kIX0C2q1J8JzzA9CQWptKxYtzxFCXiKptTW8qsQ==", - "dev": true, "license": "MIT", "dependencies": { "d3-array": "^3.1.6", @@ -5986,7 +5977,6 @@ "version": "0.0.3", "resolved": "https://registry.npmjs.org/esm-env-robust/-/esm-env-robust-0.0.3.tgz", "integrity": "sha512-90Gnuw2DALOqlL1581VxP3GHPUNHX9U+fQ+8FNcTTFClhY5gEggAAnJ3q1b2Oq23knRsjv8YpNeMRPaMLUymOA==", - "dev": true, "license": "MIT", "dependencies": { "esm-env": "^1.0.0" @@ -6272,7 +6262,6 @@ "version": "0.7.2", "resolved": "https://registry.npmjs.org/fastpriorityqueue/-/fastpriorityqueue-0.7.2.tgz", "integrity": "sha512-5DtIKh6vtOmEGkYdEPNNb+mxeYCnBiKbK3s4gq52l6cX8I5QaTDWWw0Wx/iYo80fVOblSycHu1/iJeqeNxG8Jw==", - "dev": true, "license": "Apache-2.0" }, "node_modules/fastq": { @@ -6354,7 +6343,6 @@ "version": "7.6.5", "resolved": "https://registry.npmjs.org/focus-trap/-/focus-trap-7.6.5.tgz", "integrity": "sha512-7Ke1jyybbbPZyZXFxEftUtxFGLMpE2n6A+z//m4CRDlj0hW+o3iYSmh8nFlYMurOiJVDmJRilUQtJr08KfIxlg==", - "dev": true, "license": "MIT", "dependencies": { "tabbable": "^6.2.0" @@ -7223,7 +7211,6 @@ "version": "0.1.1", "resolved": "https://registry.npmjs.org/inline-style-parser/-/inline-style-parser-0.1.1.tgz", "integrity": "sha512-7NXolsK4CAS5+xvdj5OMMbI962hU/wvwoxk+LWR9Ek9bVtyuuYScDN6eS0rUm6TxApFpw7CX1o4uJzcd4AyD3Q==", - "dev": true, "license": "MIT" }, "node_modules/internmap": { @@ -7411,7 +7398,6 @@ "version": "0.4.24", "resolved": "https://registry.npmjs.org/javascript-lp-solver/-/javascript-lp-solver-0.4.24.tgz", "integrity": "sha512-5edoDKnMrt/u3M6GnZKDDIPxOyFOg+WrwDv8mjNiMC2DePhy2H9/FFQgf4ggywaXT1utvkxusJcjQUER72cZmA==", - "dev": true, "license": "Unlicense" }, "node_modules/jiti": { @@ -9767,7 +9753,6 @@ "version": "5.1.16", "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-5.1.16.tgz", "integrity": "sha512-kVrnsrJqMR8+oLJnGEmSWw9BivK5mt7H3FZatVRjrc5wGqFYuBxX1yG7+A7Gi5AefkX6t/oCkizcQgpu0cY1dQ==", - "dev": true, "funding": [ { "type": "github", @@ -11409,7 +11394,6 @@ "version": "1.6.1", "resolved": "https://registry.npmjs.org/quadprog/-/quadprog-1.6.1.tgz", "integrity": "sha512-fN5Jkcjlln/b3pJkseDKREf89JkKIyu6cKIVXisgL6ocKPQ0yTp9n6NZUAq3otEPPw78WZMG9K0o9WsfKyMWJw==", - "dev": true, "license": "MIT", "engines": { "node": ">=8.x" @@ -12562,7 +12546,6 @@ "version": "0.4.4", "resolved": "https://registry.npmjs.org/style-to-object/-/style-to-object-0.4.4.tgz", "integrity": "sha512-HYNoHZa2GorYNyqiCaBgsxvcJIn7OHq6inEga+E6Ke3m5JkoqpQbnFssk4jwe+K7AhGa2fcha4wSOf1Kn01dMg==", - "dev": true, "license": "MIT", "dependencies": { "inline-style-parser": "0.1.1" @@ -12936,7 +12919,6 @@ "version": "3.1.4", "resolved": "https://registry.npmjs.org/svelte-awesome-color-picker/-/svelte-awesome-color-picker-3.1.4.tgz", "integrity": "sha512-tiFakxvSpCwodOSFW6CflnLVSldSlDY77rDipHfW7hvB+4gTP3y6uO+Dm9O9e7P5N7K+CNyNjjdjbZOHYkMyNQ==", - "dev": true, "license": "MIT", "dependencies": { "colord": "^2.9.3", @@ -12950,7 +12932,6 @@ "version": "1.1.2", "resolved": "https://registry.npmjs.org/svelte-awesome-slider/-/svelte-awesome-slider-1.1.2.tgz", "integrity": "sha512-HFIWwq6rtX6aXyc6ns2R0P6pDLYIr6nRjTPyAddAiTAgZqsohAoEfOEwKKqMmZ6Q+CH48zzcFdyo6lFkBJvyQw==", - "dev": true, "license": "MIT", "peerDependencies": { "svelte": "^3.54.0 || ^4.0.0 || ^5.0.0" @@ -13123,7 +13104,6 @@ "version": "1.6.2", "resolved": "https://registry.npmjs.org/svelte-floating-ui/-/svelte-floating-ui-1.6.2.tgz", "integrity": "sha512-EC+DZtBey50P6l3NSzNQWon3cip8a1bzwdpmCdc45kymqEWL4BKhPemAq7SQ9QLebDPaMECW6YodxFbs2d+O/w==", - "dev": true, "license": "MIT", "dependencies": { "@floating-ui/dom": "^1.6.13" @@ -13133,7 +13113,6 @@ "version": "7.8.4", "resolved": "https://registry.npmjs.org/svelte-highlight/-/svelte-highlight-7.8.4.tgz", "integrity": "sha512-aVp+Q0hH9kI7PlSDrklmFTF4Uj7wYj7UGuqkREnkXlqpEffxr2g6esZcMMaTgECdg5rb2mJqM88+nwS10ecoTg==", - "dev": true, "license": "MIT", "dependencies": { "highlight.js": "11.11.1" @@ -13149,7 +13128,6 @@ "version": "1.3.2", "resolved": "https://registry.npmjs.org/svelte-popperjs/-/svelte-popperjs-1.3.2.tgz", "integrity": "sha512-fwrErlkvngL876WXRnL3OLlfk/n9YkZwwLxuKRpZOYCJLt1zrwhoKTXS+/sRgDveD/zd6GQ35hV89EOip+NBGA==", - "dev": true, "license": "MIT", "peerDependencies": { "@popperjs/core": ">=2", @@ -13216,14 +13194,12 @@ "version": "2.3.1", "resolved": "https://registry.npmjs.org/svelte-range-slider-pips/-/svelte-range-slider-pips-2.3.1.tgz", "integrity": "sha512-P29PNqHld+SiaDuHzf98rLvhSYWXb3TVL9p7U5RG9UX8emUgypZgp9zuIIwpmIXysGQC6JG8duMc5FuaPnSVdg==", - "dev": true, "license": "MPL-2.0" }, "node_modules/svelte-splitpanes": { "version": "8.0.9", "resolved": "https://registry.npmjs.org/svelte-splitpanes/-/svelte-splitpanes-8.0.9.tgz", "integrity": "sha512-L3oLXTC99M191FInTXJ/f/2i0welRql1QuVbPaU8iy6nvCR6X9VyjHCsCpLqKGWHwqkWo/AM9CQ1c0nzlb+MkA==", - "dev": true, "license": "MIT", "dependencies": { "esm-env-robust": "0.0.3" @@ -13333,7 +13309,6 @@ "version": "6.2.0", "resolved": "https://registry.npmjs.org/tabbable/-/tabbable-6.2.0.tgz", "integrity": "sha512-Cat63mxsVJlzYvN51JmVXIgNoUokrIaT2zLclCXjRd8boZ0004U4KCs/sToJ75C6sdlByWxpYnb5Boif1VSFew==", - "dev": true, "license": "MIT" }, "node_modules/table": { diff --git a/frontend/package.json b/frontend/package.json index c81465a19a..96d4862677 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -22,10 +22,8 @@ "test:e2e": "playwright test" }, "devDependencies": { - "@floating-ui/core": "^1.3.1", "@hey-api/openapi-ts": "^0.43.0", "@melt-ui/pp": "^0.3.2", - "@melt-ui/svelte": "^0.86.2", "@playwright/test": "^1.57.0", "@sveltejs/adapter-static": "^3.0.6", "@sveltejs/kit": "^2.69.1", @@ -39,10 +37,8 @@ "@types/vscode": "^1.83.5", "@typescript-eslint/eslint-plugin": "^5.59.8", "@typescript-eslint/parser": "^5.60.0", - "@zerodevx/svelte-toast": "^0.9.6", "autoprefixer": "^10.4.13", "cssnano": "^6.0.1", - "d3-dag": "^0.11.5", "dts-bundle-generator": "^9.5.1", "eslint": "^8.47.0", "eslint-config-prettier": "^8.6.0", @@ -56,18 +52,10 @@ "postcss-load-config": "^4.0.1", "prettier": "^3.1.0", "prettier-plugin-svelte": "^3.3.3", - "style-to-object": "^0.4.1", "stylelint-config-recommended": "^13.0.0", - "svelte": "^5.55.7", - "svelte-awesome-color-picker": "^3.0.4", "svelte-check": "^4.4.3", "svelte-fast-check": "^0.4.5", - "svelte-floating-ui": "^1.5.8", - "svelte-highlight": "^7.6.0", - "svelte-popperjs": "^1.3.2", "svelte-preprocess": "^6.0.0", - "svelte-range-slider-pips": "^2.3.1", - "svelte-splitpanes": "^8.0.9", "tailwindcss": "^3.4.1", "tar": "^7.5.18", "tslib": "^2.6.1", @@ -97,8 +85,10 @@ "@codingame/monaco-vscode-standalone-json-language-features": "=25.0.0", "@codingame/monaco-vscode-standalone-languages": "=25.0.0", "@codingame/monaco-vscode-standalone-typescript-language-features": "=25.0.0", + "@floating-ui/core": "^1.3.1", "@json2csv/plainjs": "^7.0.6", "@leeoniya/ufuzzy": "^1.0.8", + "@melt-ui/svelte": "^0.86.2", "@popperjs/core": "^2.11.8", "@redocly/json-to-json-schema": "^0.0.1", "@scalar/openapi-parser": "^0.15.0", @@ -107,6 +97,7 @@ "@windmill-labs/svelte-dnd-action": "^0.9.44", "@xterm/addon-fit": "^0.10.0", "@xyflow/svelte": "^1.0.0", + "@zerodevx/svelte-toast": "^0.9.6", "acorn": "^8.15.0", "ag-charts-community": "^9.0.1", "ag-charts-enterprise": "^9.0.1", @@ -117,6 +108,7 @@ "chartjs-adapter-date-fns": "^3.0.0", "chartjs-plugin-zoom": "^2.0.0", "clone": "^2.1.2", + "d3-dag": "^0.11.5", "d3-zoom": "^3.0.0", "date-fns": "^2.30.0", "diff": "^8.0.3", @@ -151,9 +143,17 @@ "rehype-sanitize": "^6.0.0", "rfc4648": "^1.5.3", "runed": "^0.36.0", + "style-to-object": "^0.4.1", + "svelte": "^5.55.7", + "svelte-awesome-color-picker": "^3.0.4", "svelte-carousel": "^1.0.25", "svelte-exmarkdown": "^5.0.0", + "svelte-floating-ui": "^1.5.8", + "svelte-highlight": "^7.6.0", "svelte-infinite-loading": "^1.4.0", + "svelte-popperjs": "^1.3.2", + "svelte-range-slider-pips": "^2.3.1", + "svelte-splitpanes": "^8.0.9", "tailwind-merge": "^1.13.2", "unist-util-visit": "^5.0.0", "vscode": "npm:@codingame/monaco-vscode-extension-api@=25.0.0", diff --git a/frontend/src/lib/aiStore.ts b/frontend/src/lib/aiStore.ts index 6ac98d69e8..ff02157e4a 100644 --- a/frontend/src/lib/aiStore.ts +++ b/frontend/src/lib/aiStore.ts @@ -51,6 +51,8 @@ export const copilotInfo = writable<{ aiModels: AIProviderModel[] customPrompts?: Record maxTokensPerModel?: Record + /** Context windows per `provider:model`, overriding the built-in window table. */ + contextWindowPerModel?: Record /** Negotiated rates per `provider:model`, overriding the built-in price table. */ modelPricing?: Record webSearchEnabledProviders?: Partial> @@ -67,6 +69,7 @@ export const copilotInfo = writable<{ aiModels: [], customPrompts: {}, maxTokensPerModel: {}, + contextWindowPerModel: {}, modelPricing: {}, webSearchEnabledProviders: {} }) @@ -144,6 +147,7 @@ export function setCopilotInfo(aiConfig: AIConfig) { aiModels: aiModels, customPrompts: aiConfig.custom_prompts ?? {}, maxTokensPerModel: aiConfig.max_tokens_per_model ?? {}, + contextWindowPerModel: aiConfig.context_window_per_model ?? {}, webSearchEnabledProviders, modelPricing: aiConfig.model_pricing ?? {}, freeTier: aiConfig.free_tier @@ -160,6 +164,7 @@ export function setCopilotInfo(aiConfig: AIConfig) { aiModels: [], customPrompts: {}, maxTokensPerModel: {}, + contextWindowPerModel: {}, webSearchEnabledProviders: {}, modelPricing: {}, // An exhausted free grant lands here — no providers, but the reason AI is off diff --git a/frontend/src/lib/components/DBManagerDrawer.svelte b/frontend/src/lib/components/DBManagerDrawer.svelte index 71dda5e1d3..797c06b64f 100644 --- a/frontend/src/lib/components/DBManagerDrawer.svelte +++ b/frontend/src/lib/components/DBManagerDrawer.svelte @@ -58,6 +58,7 @@ if (!workspace || !datatable) return undefined try { return { + workspace, datatable, ...(await listUsableDatatableRoles(workspace, datatable)) } @@ -65,16 +66,24 @@ // Never leave the drawer waiting on this: fall back to the // unpermissioned shape so it opens and the server picks the role. console.error('Failed to load datatable roles:', e) - return { datatable, permissioned: false, roles: [], default_role: ADMIN_DATATABLE_ROLE } + return { + workspace, + datatable, + permissioned: false, + roles: [], + default_role: ADMIN_DATATABLE_ROLE + } } } ) - // A resource keeps its previous value while refetching, and roles are per data - // table: settling from the last one's answer would connect to the new data - // table as a role it may not even have. + // A resource keeps its previous value while refetching, and roles are per data table of one + // workspace: settling from another answer would connect to this data table as a role it may + // not even have. const rolesOfCurrent = $derived( - usableRoles.current?.datatable === selectedDatatable ? usableRoles.current : undefined + usableRoles.current?.datatable === selectedDatatable && usableRoles.current?.workspace === ws + ? usableRoles.current + : undefined ) // Nothing that connects runs until the role is settled: a first round sent without a role diff --git a/frontend/src/lib/components/DraftConflictAlert.svelte b/frontend/src/lib/components/DraftConflictAlert.svelte new file mode 100644 index 0000000000..0e0fd1fb97 --- /dev/null +++ b/frontend/src/lib/components/DraftConflictAlert.svelte @@ -0,0 +1,31 @@ + + + +
+
+ It was saved from another tab or session since this one read it, so your changes here are no + longer being saved. +
+
+ + +
+
+ diff --git a/frontend/src/lib/components/InstanceSettings.svelte b/frontend/src/lib/components/InstanceSettings.svelte index abe37c3537..d87de125a9 100644 --- a/frontend/src/lib/components/InstanceSettings.svelte +++ b/frontend/src/lib/components/InstanceSettings.svelte @@ -1094,9 +1094,10 @@ the home page’s create menu and hub-project picker are opened and from which entry point, the name of any public hub project imported from the home page and how far that import got, whether a pre-approved trial offer was opened, whether data tables are put - under roles and whether callers name a role or take the default, and which kinds of - access change (grant, revoke, ownership, default privileges) are applied to data - tables, last 30 days)
  • feature adoption (counts of which flow, script, trigger, worker and data table @@ -1163,9 +1164,10 @@ the home page’s create menu and hub-project picker are opened and from which entry point, the name of any public hub project imported from the home page and how far that import got, whether a pre-approved trial offer was opened, whether data tables are put - under roles and whether callers name a role or take the default, and which kinds of - access change (grant, revoke, ownership, default privileges) are applied to data - tables, last 30 days)
  • feature adoption (counts of which flow, script, trigger, worker and data table diff --git a/frontend/src/lib/components/ResourceEditor.svelte b/frontend/src/lib/components/ResourceEditor.svelte index d33b31b332..9847454aca 100644 --- a/frontend/src/lib/components/ResourceEditor.svelte +++ b/frontend/src/lib/components/ResourceEditor.svelte @@ -19,6 +19,8 @@ import { useActingUser } from '$lib/actingUser.svelte' import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte' import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' + import { useDraftConflictSession } from '$lib/draftConflictSession.svelte' + import DraftConflictAlert from './DraftConflictAlert.svelte' import { setLocalDraftHint } from '$lib/localDraftHints.svelte' import { onUserInput } from '$lib/userDraftEditGate' import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte' @@ -48,6 +50,9 @@ * so it can hide the banner's Discard button in read-only mode (matches * the trigger editors' `disabled={!can_write}` wiring). */ onCanWriteChange?: (canWrite: boolean) => void + /** The drawer renders the conflict alert: inside this scrollable form it can land above the + * viewport on a long resource, leaving the fixed banner claiming the edits are saved. */ + onDraftConflictChange?: (state: { conflicted: boolean; busy: boolean }) => void } let { @@ -61,7 +66,8 @@ selected: selectedProp = $bindable(), viewJsonSchema = $bindable(), onDraftStateChange, - onCanWriteChange + onCanWriteChange, + onDraftConflictChange }: Props = $props() type ResourceState = { @@ -263,6 +269,132 @@ ) const anyDirty = $derived(dirtyWorkspaces.length > 0) + /** Scoped by `selected` so switching workspace-specific versions is a new session. Ending is + * exported on top of that because the drawer, not this component, knows when its own session + * is over: this component outlives it. */ + const conflictSession = useDraftConflictSession(() => selected) + export function endEditingSession(): void { + conflictSession.end() + } + onDestroy(endEditingSession) + const resolvingConflict = $derived(conflictSession.busy) + /** The server refused this tab's autosave because the row moved under it: another tab, or the + * AI chat, which writes these drafts too. Nothing typed here reaches the server until the user + * picks a version, and the unsaved-changes banner says the opposite — that the edits are held + * as a draft — so without this they are told their work is safe while it is being dropped. */ + const draftConflict = $derived( + selected && initialPath + ? UserDraftDbSyncer.getConflict({ + workspace: selected, + itemKind: 'resource', + path: initialPath + }).conflict + : undefined + ) + + async function resolveDraftConflict(keepMine: boolean): Promise { + const ws = selected + const p = initialPath + if (!ws || !p || resolvingConflict) return + const query = { workspace: ws, itemKind: 'resource' as const, path: p } + const token = conflictSession.start() + const stillOurs = () => conflictSession.holds(token) && selected === ws + try { + if (keepMine) { + // Settle the key first: an ordinary autosave still queued would displace the forced + // write below, and being conditional it would be refused — so "Keep mine" would + // finish without keeping anything and leave the alert standing. + await UserDraftDbSyncer.quiesce(query) + // A resolution belongs to the session that started it. One that outlives its editor + // stops here rather than writing on: whatever replaced it — another session on the + // same draft, or its own resolution — owns the key now, and the edit this one was + // keeping is still parked for a later flush either way. + if (!stillOurs()) return + // A parked `null` is this tab's "no draft any more" — a discard, or an edit that + // landed back on the deployed value. Keeping that means removing the row, not + // writing the baseline back as a draft with no dirty banner to discard it through. + const parked = UserDraftDbSyncer.peekPending(query) + const mine = parked?.value === null ? null : $state.snapshot(states[ws]?.draft) + // Forced, so it goes over the row that refused us, and its response reseeds + // `last_sync` so the next ordinary save is conditional again. + if (mine !== undefined) await UserDraftDbSyncer.overwrite({ ...query, value: mine }) + // Say so rather than leave the alert up with no explanation: a write displaced by + // something typed meanwhile can still lose the race. + if (UserDraftDbSyncer.getConflict(query).conflict) { + sendUserToast('Could not keep your version — try again', true) + } + return + } + // Settle the key BEFORE reading, so what comes back is the version the server is left + // holding: a write this tab started can still be in flight — a forced one it walked + // away from included — and a response fetched past it describes a version about to be + // replaced, which would then be seeded along with its already-stale `last_sync`. + // Nothing is given up by waiting; `quiesce` only stops the pipeline. + await UserDraftDbSyncer.quiesce(query) + if (!stillOurs()) return + // Read BEFORE giving anything up: until the server has answered, the refused payload is + // still the only copy of this tab's edit, and the conflict is still true. + const r = await ResourceService.getResource({ workspace: ws, path: p, getDraft: true }) + const deployedState: ResourceState = { + path: r.path, + args: (r.value ?? {}) as Record, + description: r.description ?? '', + labels: r.labels ?? undefined, + wsSpecific: r.ws_specific ?? false + } + // Everything below writes shared editor state, so first make sure it is still this + // resource's: the drawer stays closable while the read is out, and another resource + // opened meanwhile would otherwise get this one's baseline — and with it this one's + // path as its save target. + if (!stillOurs()) return + // Again, because the form stayed editable while the read was out: `quiesce` settles what + // is running when it is called, not the key for the rest of the resolution, so a + // keystroke since can have started a save of its own. Left running, its rejection lands + // after the baseline below and raises the conflict this just resolved. + await UserDraftDbSyncer.quiesce(query) + if (!stillOurs()) return + // Now, and not in `quiesce`: the refused payload belongs to the version being replaced, + // but until this point it was still the only copy of the edit, and a resolution that + // gave up before here has to leave it behind. + UserDraftDbSyncer.dropPending(query) + UserDraftDbSyncer.clearConflict(query) + initialStates[ws] = structuredClone(deployedState) + // Everything else the load path takes from this same response. The item can have been + // deleted, recreated under another type, or had its permissions changed while the + // conflict stood, and the fields below decide create-vs-update, the schema and write + // access — so refreshing only what is displayed would leave those deciding on the + // version the user just replaced. + fetchedResources[ws] = r + existedInitially[ws] = !(r as any).no_deployed + if (ws === effectiveWorkspace) resource_type = r.resource_type + UserDraftDbSyncer.recordRemoteSync(query, (r as any).draft_saved_at) + const loadedDraft = (r as any).draft as ResourceState | undefined + // Loading a draft makes this workspace one that opened with a draft, whatever it opened + // with before — a refused deletion opens gated, and gated the settling absorber would + // fold the version just loaded into the deployed baseline, leaving it silently clean + // with Save disabled. Ungate here rather than leaving it to the effect, so no write + // between the two is absorbed. + openedOnDraft[ws] = !!loadedDraft + if (loadedDraft) { + setGated(ws, false) + } else { + // Accepting "there is no draft" has to shut the gate, the way discarding one does. + // The edit that raised the conflict set `userEdited`, and left open, the form's + // settling writes — schema defaults materializing over the loaded value — read as + // the user's and recreate the draft just accepted away. + userEdited[ws] = false + setGated(ws, true) + } + UserDraft.seed('resource', p, loadedDraft ?? deployedState, { workspace: ws }) + } catch (e) { + // Nothing was given up above, so the conflict stands and the edit is still here to + // resolve again — which is the whole point of reading first. + sendUserToast(`Could not load the other version: ${e}`, true) + } finally { + conflictSession.finish(token) + } + } + // The syncer owns the list-page `*` hint; the editor only CLEARS it when a // workspace is at the deployed baseline (so a draft discarded elsewhere // vanishes on reopen). Never SET here. See VariableEditor for the full note. @@ -338,15 +470,33 @@ labels: r.labels ?? undefined, wsSpecific: r.ws_specific ?? false } - // Open with the saved draft if present, else the deployed. - const s: ResourceState = savedDraftState ?? deployedState - openedOnDraft[ws] = !!savedDraftState + // A refused save leaves this tab's own version parked. `.draft` is the + // version that refused it, so opening on that would quietly drop the edit + // the alert is about and leave "Keep mine" offering to keep the other one. + const conflictQuery = { + workspace: ws, + itemKind: 'resource' as const, + path: initialPath + } + const refused = UserDraftDbSyncer.getConflict(conflictQuery).conflict + ? UserDraftDbSyncer.peekPending(conflictQuery) + : undefined + // A parked `null` is this tab's "no draft any more", which on screen is the + // deployed value — so only a payload with content counts as a local draft. + const refusedDraft = (refused?.value ?? undefined) as ResourceState | undefined + const hasLocalDraft = refused ? refused.value !== null : !!savedDraftState + // Open with this tab's refused version if there is one, else the saved + // draft, else the deployed. + const s: ResourceState = refused + ? (refusedDraft ?? deployedState) + : (savedDraftState ?? deployedState) + openedOnDraft[ws] = hasLocalDraft // Gate BEFORE the handle is acquired: `stopSync` queues on a // not-yet-live entry, and the form can settle before the effect // above gets a chance to run. Only worth doing when no draft exists // yet — where one does, there is no phantom to prevent and // suspending could only drop a write. - if (!savedDraftState) setGated(ws, true) + if (!hasLocalDraft) setGated(ws, true) ensureHandle(ws, s) initialStates[ws] = structuredClone(deployedState) // Draft-only paths (`no_deployed`) have no row — saving must @@ -426,6 +576,13 @@ $effect(() => { onCanWriteChange?.(can_write === true) }) + $effect(() => { + onDraftConflictChange?.({ conflicted: !!draftConflict, busy: resolvingConflict }) + }) + + export function resolveDraftConflictFromBanner(keepMine: boolean): void { + void resolveDraftConflict(keepMine) + } export function localDraftDeployed(): ResourceState | undefined { return selected ? initialStates[selected] : undefined @@ -539,6 +696,18 @@
    + + {#if draftConflict && !onDraftConflictChange} + void resolveDraftConflict(false)} + onOverwrite={() => void resolveDraftConflict(true)} + /> + {/if} + {#if otherDirty.length > 0} You are going to edit the value in: {otherDirty.join(', ')} diff --git a/frontend/src/lib/components/ResourceEditorDrawer.svelte b/frontend/src/lib/components/ResourceEditorDrawer.svelte index 7259fff633..ce4d1eea5a 100644 --- a/frontend/src/lib/components/ResourceEditorDrawer.svelte +++ b/frontend/src/lib/components/ResourceEditorDrawer.svelte @@ -8,6 +8,7 @@ import { isOwner } from '$lib/utils' import { useActingUser } from '$lib/actingUser.svelte' import LocalDraftBanner from './LocalDraftBanner.svelte' + import DraftConflictAlert from './DraftConflictAlert.svelte' import OpenInSessionButton from './sessions/OpenInSessionButton.svelte' import { clearPageDrawerAnchor, @@ -62,9 +63,12 @@ localDraftDeployed: () => unknown localDraftCurrent: () => unknown discardLocalDraft: () => void + endEditingSession: () => void + resolveDraftConflictFromBanner: (keepMine: boolean) => void } | undefined = $state(undefined) let hasLocalDraft = $state(false) + let draftConflict = $state({ conflicted: false, busy: false }) let canWriteSelected = $state(true) let path: string | undefined = $state(undefined) @@ -151,6 +155,9 @@ size="50rem" {disableChatOffset} on:close={() => { + // The editor outlives this drawer, so tell it the session is over: a conflict resolution + // still in flight must not land on whatever the next opening shows. + resourceEditor?.endEditingSession?.() if (keepAnchorOnClose) { keepAnchorOnClose = false return @@ -168,7 +175,15 @@ bannerReserved={mode == 'edit'} hideClose={inline && !onClose} fullScreen={!inline} - on:close={() => (inline ? onClose?.() : drawer?.closeDrawer())} + on:close={() => { + // Inline has no drawer to emit a close, so the session ends here instead. + if (inline) { + resourceEditor?.endEditingSession?.() + onClose?.() + } else { + drawer?.closeDrawer() + } + }} > {#snippet titleExtra()} {#if mode == 'new' && resource_type} @@ -189,10 +204,18 @@ bind:selected bind:viewJsonSchema onDraftStateChange={(v) => (hasLocalDraft = v)} + onDraftConflictChange={(v) => (draftConflict = v)} onCanWriteChange={(v) => (canWriteSelected = v)} /> {/await} {#snippet banner()} + {#if draftConflict.conflicted} + resourceEditor?.resolveDraftConflictFromBanner?.(false)} + onOverwrite={() => resourceEditor?.resolveDraftConflictFromBanner?.(true)} + /> + {/if} import { VariableService, WorkspaceService } from '$lib/gen' - import { createEventDispatcher, untrack } from 'svelte' + import { createEventDispatcher, onDestroy, untrack } from 'svelte' import { Button } from './common' import Drawer from './common/drawer/Drawer.svelte' import DrawerContent from './common/drawer/DrawerContent.svelte' @@ -23,6 +23,9 @@ import { useActingUser } from '$lib/actingUser.svelte' import { UserDraft, draftValuesEqual, type UserDraftHandle } from '$lib/userDraft.svelte' import LocalDraftBanner from './LocalDraftBanner.svelte' + import DraftConflictAlert from './DraftConflictAlert.svelte' + import { UserDraftDbSyncer } from '$lib/userDraftDbSyncer.svelte' + import { useDraftConflictSession } from '$lib/draftConflictSession.svelte' import { isEncryptedDraftValue } from '$lib/encryptedDraft' import { setLocalDraftHint } from '$lib/localDraftHints.svelte' import { useOperatingWorkspace } from '$lib/components/operatingWorkspace.svelte' @@ -141,6 +144,126 @@ Object.keys(states).filter((ws) => !draftValuesEqual(states[ws].draft, initialStates[ws])) ) + /** Scoped by `selected` so switching workspace-specific versions is a new session. Ended on top + * of that by every entry point and by the teardown, since reopening the same variable reuses + * this component. */ + const conflictSession = useDraftConflictSession(() => selected) + function endEditingSession(): void { + conflictSession.end() + } + onDestroy(endEditingSession) + const resolvingConflict = $derived(conflictSession.busy) + /** The server refused this tab's autosave because the row moved under it: another tab, or the + * AI chat, which writes these drafts too. Nothing typed here reaches the server until the user + * picks a version, and the unsaved-changes banner says the opposite — that the edits are held + * as a draft — so without this they are told their work is safe while it is being dropped. */ + const draftConflict = $derived( + edit && selected && editPath + ? UserDraftDbSyncer.getConflict({ + workspace: selected, + itemKind: 'variable', + path: editPath + }).conflict + : undefined + ) + + async function resolveDraftConflict(keepMine: boolean): Promise { + const ws = selected + const p = editPath + if (!ws || !p || resolvingConflict) return + const query = { workspace: ws, itemKind: 'variable' as const, path: p } + const token = conflictSession.start() + const stillOurs = () => conflictSession.holds(token) && selected === ws + try { + if (keepMine) { + // Settle the key first: an ordinary autosave still queued would displace the forced + // write below, and being conditional it would be refused — so "Keep mine" would + // finish without keeping anything and leave the alert standing. + await UserDraftDbSyncer.quiesce(query) + // A resolution belongs to the session that started it. One that outlives its editor + // stops here rather than writing on: whatever replaced it — another session on the + // same draft, or its own resolution — owns the key now, and the edit this one was + // keeping is still parked for a later flush either way. + if (!stillOurs()) return + // A parked `null` is this tab's "no draft any more" — a discard, or an edit that + // landed back on the deployed value. Keeping that means removing the row, not + // writing the baseline back as a draft with no dirty banner to discard it through. + const parked = UserDraftDbSyncer.peekPending(query) + const mine = parked?.value === null ? null : $state.snapshot(states[ws]?.draft) + // Forced, so it goes over the row that refused us, and its response reseeds + // `last_sync` so the next ordinary save is conditional again. + if (mine !== undefined) await UserDraftDbSyncer.overwrite({ ...query, value: mine }) + // Say so rather than leave the alert up with no explanation: a write displaced by + // something typed meanwhile can still lose the race. + if (UserDraftDbSyncer.getConflict(query).conflict) { + sendUserToast('Could not keep your version — try again', true) + } + return + } + // Settle the key BEFORE reading, so what comes back is the version the server is left + // holding: a write this tab started can still be in flight — a forced one it walked + // away from included — and a response fetched past it describes a version about to be + // replaced, which would then be seeded along with its already-stale `last_sync`. + // Nothing is given up by waiting; `quiesce` only stops the pipeline. + await UserDraftDbSyncer.quiesce(query) + if (!stillOurs()) return + // Read BEFORE giving anything up: until the server has answered, the refused payload is + // still the only copy of this tab's edit, and the conflict is still true. + const v = await VariableService.getVariable({ + workspace: ws, + path: p, + decryptSecret: false, + getDraft: true + }) + const deployedState: VariableState = { + path: v.path, + variable: { + value: v.value ?? '', + is_secret: v.is_secret, + description: v.description ?? '' + }, + labels: v.labels ?? undefined, + wsSpecific: v.ws_specific ?? false + } + // Everything below writes shared editor state, so first make sure it is still this + // variable's: the drawer stays closable while the read is out, and another variable + // opened meanwhile would otherwise get this one's baseline — and with it this one's + // path as its save target. + if (!stillOurs()) return + // Again, because the form stayed editable while the read was out: `quiesce` settles what + // is running when it is called, not the key for the rest of the resolution, so a + // keystroke since can have started a save of its own. Left running, its rejection lands + // after the baseline below and raises the conflict this just resolved. + await UserDraftDbSyncer.quiesce(query) + if (!stillOurs()) return + // Now, and not in `quiesce`: the refused payload belongs to the version being replaced, + // but until this point it was still the only copy of the edit, and a resolution that + // gave up before here has to leave it behind. + UserDraftDbSyncer.dropPending(query) + UserDraftDbSyncer.clearConflict(query) + initialStates[ws] = structuredClone(deployedState) + // Everything else the load path takes from this same response. The variable can have + // been deleted, recreated, or had its permissions changed while the conflict stood, + // and these decide create-vs-update and write access — so refreshing only what is + // displayed would leave those deciding on the version the user just replaced. + existedInitially[ws] = !(v as any).no_deployed + extraPerms[ws] = v.extra_perms ?? {} + UserDraftDbSyncer.recordRemoteSync(query, (v as any).draft_saved_at) + UserDraft.seed( + 'variable', + p, + ((v as any).draft as VariableState | undefined) ?? deployedState, + { workspace: ws } + ) + } catch (e) { + // Nothing was given up above, so the conflict stands and the edit is still here to + // resolve again — which is the whole point of reading first. + sendUserToast(`Could not load the other version: ${e}`, true) + } finally { + conflictSession.finish(token) + } + } + // The list-page `*` hint is owned by UserDraftDbSyncer (set on save, cleared // on delete). The editor only CLEARS it — a workspace at the deployed // baseline has no draft, so drop any stale hint (this is how a draft @@ -210,8 +333,21 @@ labels: v.labels ?? undefined, wsSpecific: v.ws_specific ?? false } - // Open with the saved draft if present, else the deployed. - const s: VariableState = savedDraftState ?? deployedState + // A refused save leaves this tab's own version parked. `.draft` is the version + // that refused it, so opening on that would quietly drop the edit the alert is + // about and leave "Keep mine" offering to keep the other one. + const conflictQuery = { workspace: ws, itemKind: 'variable' as const, path: p } + const refused = UserDraftDbSyncer.getConflict(conflictQuery).conflict + ? UserDraftDbSyncer.peekPending(conflictQuery) + : undefined + // A parked `null` is this tab's "no draft any more", which on screen is the + // deployed value. + const refusedDraft = (refused?.value ?? undefined) as VariableState | undefined + // Open with this tab's refused version if there is one, else the saved draft, + // else the deployed. + const s: VariableState = refused + ? (refusedDraft ?? deployedState) + : (savedDraftState ?? deployedState) ensureHandle(ws, s) initialStates[ws] = structuredClone(deployedState) // Draft-only paths (`no_deployed`) have no row — saving must @@ -223,6 +359,8 @@ }) function reset() { + // A new session starts here, so anything still running for the last one is spent. + endEditingSession() // Clearing workspaceSpecs triggers useMany's reconcile to release // every acquired entry. The $derived `states` then collapses to {}. workspaceSpecs = [] @@ -334,7 +472,14 @@ {#if inline} {@render content()} {:else} - clearPageDrawerAnchor(VARIABLES_PATH)}> + { + endEditingSession() + clearPageDrawerAnchor(VARIABLES_PATH) + }} + > {@render content()} {/if} @@ -345,9 +490,24 @@ bannerReserved={edit} hideClose={inline && !onClose} fullScreen={!inline} - on:close={() => (inline ? onClose?.() : drawer?.closeDrawer())} + on:close={() => { + // Inline has no drawer to emit a close, so the session ends here instead. + if (inline) { + endEditingSession() + onClose?.() + } else { + drawer?.closeDrawer() + } + }} > {#snippet banner()} + {#if draftConflict} + void resolveDraftConflict(false)} + onOverwrite={() => void resolveDraftConflict(true)} + /> + {/if} = contextWindow * COMPACTION_TRIGGER_RATIO diff --git a/frontend/src/lib/components/copilot/chat/ContextUsageIndicator.svelte b/frontend/src/lib/components/copilot/chat/ContextUsageIndicator.svelte index cd6c58b2a3..b49bc582ef 100644 --- a/frontend/src/lib/components/copilot/chat/ContextUsageIndicator.svelte +++ b/frontend/src/lib/components/copilot/chat/ContextUsageIndicator.svelte @@ -1,6 +1,6 @@ {#if Object.keys(aiProviders).length > 0} - +
    - {#each Object.entries(modelsByProvider).filter(([provider, models]) => models.length > 0) as [provider, models]} + {#each Object.entries(modelsByProvider).filter(([_, models]) => models.length > 0) as [provider, models] (provider)} {@const isExpanded = !collapsedProviders[provider]} - {@const hasCustom = hasCustomSettings(provider, models)}
    - + {#if isExpanded}
    - {#each models as { model }} - {@const currentTokens = getCurrentTokensForModel(provider as AIProvider, model)} - {@const defaultTokens = getDefaultTokensForModel(provider as AIProvider, model)} - {@const isAtDefault = isModelAtDefault(provider as AIProvider, model)} + {#each models as { model } (model)} + {@const key = modelKey(provider as AIProvider, model)} + {@const fallback = getDefault(provider as AIProvider, model)} + {@const overridden = isOverridden(provider as AIProvider, model)}
    {model}
    - { - const value = parseInt(e.currentTarget.value) - if (!isNaN(value)) { - updateTokensForModel(provider as AIProvider, model, value) - } - }} - class="w-20 px-2 py-1 text-xs text-center border border-gray-200 dark:border-gray-700 rounded bg-surface focus:outline-none focus:ring-2 focus:ring-blue-500 focus:border-transparent" - /> +
    + { + if (e.currentTarget.value === '') { + resetModel(provider as AIProvider, model) + return + } + const value = Number(e.currentTarget.value) + if (!isNaN(value)) { + updateTokens(provider as AIProvider, model, value) + } + }, + onblur: (e: Event & { currentTarget: HTMLInputElement }) => { + // Resync a value the state refused, so what is shown is what is stored. + e.currentTarget.value = String(limits[key] ?? '') + errors[key] = '' + } + }} + /> +
    tokens
    - {#if !isAtDefault} -
    - Default: {defaultTokens} tokens + {#if overridden} +
    + + {fallback.assumed ? 'Assumed' : 'Default'}: {fallback.tokens.toLocaleString()} + tokens +
    - {#if errors[getModelKey(provider as AIProvider, model)]} -
    {errors[getModelKey(provider as AIProvider, model)]}
    - {/if} + {/if} + {#if errors[key]} +
    {errors[key]}
    {/if}
    {/each} diff --git a/frontend/src/lib/components/workspaceSettings/instanceDbSteps.ts b/frontend/src/lib/components/workspaceSettings/instanceDbSteps.ts index dd465626f4..c11a205116 100644 --- a/frontend/src/lib/components/workspaceSettings/instanceDbSteps.ts +++ b/frontend/src/lib/components/workspaceSettings/instanceDbSteps.ts @@ -84,6 +84,11 @@ export function instanceDbSteps(dbname: string, status: CustomInstanceDb | undef (status?.logs.replication_user_error ? `\n\nError: ${status.logs.replication_user_error}` : '') + }, + { + title: 'Connect as custom_instance_user', + status: status?.logs.user_connect, + description: `The steps above run as the user in DATABASE_URL, but data tables and DuckLake catalogs on ${dbname} log in as custom_instance_user. This checks that login reaches the database too. A connection pooler that routes on the login name, such as Supabase's shared pooler, refuses it: use your own Postgres database instead, or point DATABASE_URL at the Postgres server directly.` } ] } diff --git a/frontend/src/lib/components/workspaceSettings/perModelSettings.ts b/frontend/src/lib/components/workspaceSettings/perModelSettings.ts new file mode 100644 index 0000000000..a83da75540 --- /dev/null +++ b/frontend/src/lib/components/workspaceSettings/perModelSettings.ts @@ -0,0 +1,24 @@ +import type { AIConfig, AIProvider } from '$lib/gen' +import { stripLegacyThinkingSuffix } from '../copilot/reasoningRegistry' + +/** + * The rows of a per-model setting, grouped by provider. Settings are keyed by the id + * the chat reads them under, and `setCopilotInfo` strips the deprecated `/thinking` + * suffix before the chat ever sees a model. A row built from the raw config would + * save a value under a key nothing reads, so it would sit in settings looking applied + * and never take effect. Stripping can collapse two configured slots onto one model, + * hence the dedupe. + */ +export function chatModelsByProvider( + aiProviders: Exclude +): Record> { + return Object.fromEntries( + Object.entries(aiProviders).map(([provider, config]) => [ + provider, + [...new Set(config.models.map(stripLegacyThinkingSuffix))].map((model) => ({ + provider: provider as AIProvider, + model + })) + ]) + ) +} diff --git a/frontend/src/lib/draftConflictSession.svelte.ts b/frontend/src/lib/draftConflictSession.svelte.ts new file mode 100644 index 0000000000..d377b92dfd --- /dev/null +++ b/frontend/src/lib/draftConflictSession.svelte.ts @@ -0,0 +1,65 @@ +import { untrack } from 'svelte' + +/** + * Identity for one editing session's conflict resolution. + * + * An editor outlives what it shows, so an awaited step cannot tell from the workspace and path + * alone whether it still speaks for what is on screen: both come back to the values it started + * with when the user returns to where they were. Every resolution takes a token and checks it + * before touching shared editor state. + */ +export function createDraftConflictSession() { + let generation = $state(0) + /** The token of the resolution in flight, or 0. A token rather than a flag: one left over from + * a closed session must not clear the busy state of the session that replaced it. */ + let inFlight = $state(0) + + return { + /** A resolution started in the current session is in flight. Ending a session clears it, so + * a reopened editor never shows buttons that only a stale request settling can re-enable. */ + get busy(): boolean { + return inFlight !== 0 + }, + /** Claim the session for a resolution about to start. */ + start(): number { + inFlight = ++generation + return inFlight + }, + /** Whether `token` still speaks for the editor. */ + holds(token: number): boolean { + return token === generation + }, + /** Release `token`'s claim. A stale token is ignored. */ + finish(token: number): void { + if (inFlight === token) inFlight = 0 + }, + /** Nothing outstanding speaks for this editor any more: a different item, a different + * session on the same one, or the component going away. */ + end(): void { + generation++ + inFlight = 0 + } + } +} + +/** + * A session for an editor that swaps what it shows in place, `scopeOf` naming what that currently + * is. Leaving a scope and coming back is a new session, so the change itself ends the old one: + * afterwards every value a resolution captured before the switch reads as current again, and no + * later comparison could tell it apart from one that never left. + */ +export function useDraftConflictSession(scopeOf: () => unknown) { + const session = createDraftConflictSession() + + let lastScope = untrack(scopeOf) + $effect(() => { + const next = scopeOf() + untrack(() => { + if (next === lastScope) return + lastScope = next + session.end() + }) + }) + + return session +} diff --git a/frontend/src/lib/draftConflictSession.test.ts b/frontend/src/lib/draftConflictSession.test.ts new file mode 100644 index 0000000000..88e62f44de --- /dev/null +++ b/frontend/src/lib/draftConflictSession.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect } from 'vitest' + +import { createDraftConflictSession } from './draftConflictSession.svelte' + +/** + * A resolution the user has walked away from must neither speak for what replaced it nor hold its + * buttons disabled until it settles. + */ +describe('createDraftConflictSession', () => { + it('frees the next session from a resolution the last one left in flight', () => { + const session = createDraftConflictSession() + + const stale = session.start() + expect(session.busy).toBe(true) + + session.end() + expect(session.busy).toBe(false) + expect(session.holds(stale)).toBe(false) + + const fresh = session.start() + expect(session.busy).toBe(true) + expect(session.holds(fresh)).toBe(true) + + // The stale request settles last: it must not release the live session's claim. + session.finish(stale) + expect(session.busy).toBe(true) + + session.finish(fresh) + expect(session.busy).toBe(false) + }) +}) diff --git a/frontend/src/lib/userDraftDbSyncer.svelte.ts b/frontend/src/lib/userDraftDbSyncer.svelte.ts index 25125a74e4..e383213cfe 100644 --- a/frontend/src/lib/userDraftDbSyncer.svelte.ts +++ b/frontend/src/lib/userDraftDbSyncer.svelte.ts @@ -723,5 +723,35 @@ export const UserDraftDbSyncer = { const key = draftKey(query.workspace, query.itemKind, query.path) pendingSaveOpts.delete(key) debouncer.cancel(key) + }, + + /** + * The value parked for this key, wrapped so a parked delete (`null`) stays distinguishable + * from nothing parked at all. While a conflict stands this is the version the server refused, + * which is to say this tab's own: an editor opening on the key takes it over the server's + * draft, which is the version that did the refusing. + */ + peekPending(query: UserDraftLastSyncQuery): { value: unknown } | undefined { + const parked = pendingSaveOpts.get(draftKey(query.workspace, query.itemKind, query.path)) + return parked ? { value: parked.value } : undefined + }, + + /** + * Stop scheduling saves for this key and wait until nothing for it is still in flight. + * Cancelling alone cannot stop a POST the runner already started, and such a POST settles + * *after* the caller has moved on — a rejected one re-raising the conflict it was told to + * resolve. Await this before installing a baseline that would make a stale payload acceptable. + * + * What is parked is deliberately left alone: a refused save keeps its payload here, and while + * the conflict stands that is the only copy of the edit outside the editor's own memory. A + * caller that gives up half way must leave it recoverable, so dropping it is the committing + * caller's job, via `dropPending`, once it has something to replace it with. + */ + async quiesce(query: UserDraftLastSyncQuery): Promise { + const key = draftKey(query.workspace, query.itemKind, query.path) + debouncer.cancel(key) + await runner.settled(key) + // A save that landed while we waited can have scheduled the next one. + debouncer.cancel(key) } } diff --git a/frontend/src/lib/userDraftQuiesce.test.ts b/frontend/src/lib/userDraftQuiesce.test.ts new file mode 100644 index 0000000000..b6c38740da --- /dev/null +++ b/frontend/src/lib/userDraftQuiesce.test.ts @@ -0,0 +1,126 @@ +import { describe, it, expect, afterEach, vi } from 'vitest' + +// Mocked so a test can hold a POST in flight: that window is the whole point of `quiesce`, +// which exists to wait one out rather than return while it is still going. +const updateDraft = vi.fn(async (..._args: any[]) => ({ + status: 'saved' as const, + current_timestamp: '2020-01-01T00:00:00Z' +})) + +vi.mock('./gen', () => ({ + DraftService: { updateDraft: (...a: unknown[]) => updateDraft(...(a as [])) } +})) +vi.mock('./gen/core/OpenAPI', () => ({ OpenAPI: { BASE: '' } })) +vi.mock('./localDraftHints.svelte', () => ({ setLocalDraftHint: vi.fn() })) + +import { UserDraftDbSyncer } from './userDraftDbSyncer.svelte' + +function deferred() { + let resolve!: (v: T) => void + const promise = new Promise((res) => (resolve = res)) + return { promise, resolve } +} + +afterEach(() => { + vi.clearAllMocks() + updateDraft.mockResolvedValue({ status: 'saved', current_timestamp: '2020-01-01T00:00:00Z' }) +}) + +/** + * Resolving a draft conflict installs a fresh baseline. Anything of the old version still in the + * air when that lands would be made acceptable by it, and a rejected one re-raises the conflict + * just resolved — so the resolution waits for the key to go quiet first. What it must NOT do is + * discard the payload while waiting: a refused save keeps it here, and until the resolution + * commits it is the only copy of the edit outside the editor's own memory. + */ +describe('UserDraftDbSyncer.quiesce', () => { + it('waits for a save already in flight', async () => { + const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_a' } + const inFlight = deferred() + let settledBeforeQuiesceReturned = false + updateDraft.mockImplementationOnce(async () => { + await inFlight.promise + settledBeforeQuiesceReturned = true + return { status: 'saved', current_timestamp: '2020-01-01T00:00:00Z' } + }) + + // In flight, not awaited: the POST is held open by the mock above. + const saving = UserDraftDbSyncer.save({ ...q, value: { v: 'in flight' }, immediate: true }) + + let quiesced = false + const quiescing = UserDraftDbSyncer.quiesce(q).then(() => (quiesced = true)) + // A full macrotask, not a microtask: anything that merely yields would have resolved by + // now, so this is what tells "waited for the POST" apart from "waited for nothing". + await new Promise((r) => setTimeout(r, 0)) + expect(quiesced).toBe(false) + + inFlight.resolve() + await quiescing + await saving + expect(settledBeforeQuiesceReturned).toBe(true) + }) + + it('keeps a refused payload recoverable, until the resolution that replaces it says otherwise', async () => { + const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_b' } + // Refused, so the payload stays parked — the state a conflict resolution starts from. + updateDraft.mockResolvedValueOnce({ + status: 'conflict', + current_timestamp: '2020-01-02T00:00:00Z' + }) + await UserDraftDbSyncer.save({ ...q, value: { v: 'refused' }, immediate: true }) + expect(UserDraftDbSyncer.getConflict(q).conflict).toBeTruthy() + + await UserDraftDbSyncer.quiesce(q) + + // A resolution abandoned here (the editor closed while quiescing) must leave the edit + // somewhere it can still be sent. Flushing is how that payload is observed, since the + // parked opts themselves are private to the syncer. + updateDraft.mockClear() + await UserDraftDbSyncer.flush(q) + expect(updateDraft).toHaveBeenCalledTimes(1) + expect(updateDraft.mock.calls[0][0]).toMatchObject({ requestBody: { value: { v: 'refused' } } }) + + // Only the caller that has something to put in its place drops it. + UserDraftDbSyncer.dropPending(q) + updateDraft.mockClear() + await UserDraftDbSyncer.flush(q) + expect(updateDraft).not.toHaveBeenCalled() + }) + + it('keeps a parked delete distinguishable from nothing parked', async () => { + const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_delete' } + + expect(UserDraftDbSyncer.peekPending(q)).toBeUndefined() + + // Reverting to the deployed value, or discarding, parks a delete. Refused, it stays + // parked — and a resolution reading it has to keep meaning "remove the row", not fall + // back to whatever the server holds. + updateDraft.mockResolvedValueOnce({ + status: 'conflict', + current_timestamp: '2020-01-02T00:00:00Z' + }) + await UserDraftDbSyncer.save({ ...q, value: null, immediate: true }) + + expect(UserDraftDbSyncer.peekPending(q)).toEqual({ value: null }) + }) + + it('cancels a debounced autosave so it cannot displace the write that follows', async () => { + const q = { workspace: 'w', itemKind: 'variable' as const, path: 'u/me/quiesce_c' } + vi.useFakeTimers() + try { + // Debounced rather than immediate: this is the autosave a resolution has to call off, or + // it fires mid-resolution and, being conditional, is refused. + void UserDraftDbSyncer.save({ ...q, value: { v: 'queued' } }) + + await UserDraftDbSyncer.quiesce(q) + updateDraft.mockClear() + + // Past the debouncer's 10s ceiling, so a schedule that survived has certainly fired. + // Real time would not reach it: the test would pass with the cancelling removed. + await vi.advanceTimersByTimeAsync(11000) + expect(updateDraft).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/frontend/src/routes/(root)/(logged)/+layout.svelte b/frontend/src/routes/(root)/(logged)/+layout.svelte index 9489384405..bfae9bb5d2 100644 --- a/frontend/src/routes/(root)/(logged)/+layout.svelte +++ b/frontend/src/routes/(root)/(logged)/+layout.svelte @@ -143,10 +143,11 @@ // same way the old `w-52`/`w-12` classes did — `:root` jumps to 18px on screens // ≥1760px (app.css), which grows the rem-based button content; a fixed-px rail would // not grow with it and the content would overflow. SIDEBAR_MIN_REM is the default - // expanded width (the old w-52); the handle only resizes when expanded and only - // widens from there — collapsing is the toggle button's job, not the drag's. + // expanded width (the old w-52); the handle only widens from there. Dragging the + // pointer into the snap zone at the screen's left edge collapses the rail. const SIDEBAR_MIN_REM = 13 const SIDEBAR_COLLAPSED_REM = 3 + const SIDEBAR_SNAP_COLLAPSE_REM = 6 // Root font-size in px, used to convert the pointer's clientX (px) into rem. function rootFontPx(): number { if (!BROWSER) return 16 @@ -169,11 +170,14 @@ // Width (in rem) the content offset must track: the icon strip when collapsed, // the user-chosen width otherwise. let railWidth = $derived(isCollapsed ? SIDEBAR_COLLAPSED_REM : sidebarWidth) - // Width transition shared by the rail and the content offset: none for the whole - // drag (the rail tracks the pointer 1:1 and hits the min as a hard wall, no - // friction), a plain ease only for the collapse/expand toggle. + // True for one transition's length after a drag crosses the snap zone's edge, so + // the collapse/expand eases before the rail goes back to tracking the pointer. + let sidebarSnapping = $state(false) + let sidebarSnapTimer: ReturnType | undefined + // Width transition shared by the rail and the content offset: none while a drag + // tracks the pointer 1:1, a plain ease for the toggle and the snap. let sidebarTransitionClass = $derived( - resizingSidebar ? '' : 'transition-all duration-200 ease-in-out' + resizingSidebar && !sidebarSnapping ? '' : 'transition-all duration-200 ease-in-out' ) // Set while a drag is live so it can be torn down if the layout unmounts // mid-drag (otherwise the window listeners would leak). @@ -192,12 +196,22 @@ handle.setPointerCapture(e.pointerId) } catch {} resizingSidebar = true + // Re-expanding a snap-collapsed rail restores the width it had before the drag. + const widthAtStart = sidebarWidth + const collapsedAtStart = isCollapsed // The rail is fixed at left:0, so the pointer's clientX is the width — in px. - // Convert to rem (the unit the rail is sized in) via the root font-size. Pure - // resize: clamp at the min so the rail stops there like a wall (dragging left - // never collapses — that's the toggle button's job). + // Convert to rem (the unit the rail is sized in) via the root font-size. The + // rail clamps at the min like a wall, until the pointer reaches the snap zone. const onMove = (ev: PointerEvent) => { - sidebarWidth = Math.max(SIDEBAR_MIN_REM, ev.clientX / rootFontPx()) + const x = ev.clientX / rootFontPx() + const collapse = x < SIDEBAR_SNAP_COLLAPSE_REM + if (collapse !== isCollapsed) { + isCollapsed = collapse + sidebarSnapping = true + clearTimeout(sidebarSnapTimer) + sidebarSnapTimer = setTimeout(() => (sidebarSnapping = false), 200) + } + sidebarWidth = collapse ? widthAtStart : Math.max(SIDEBAR_MIN_REM, x) } // pointercancel (and unmount, via onDestroy) must clear the state too, or // `resizingSidebar` sticks true — the overlay and handle highlight stay up @@ -205,7 +219,10 @@ const stop = () => { if (!resizingSidebar) return resizingSidebar = false + clearTimeout(sidebarSnapTimer) + sidebarSnapping = false widthPref.val = sidebarWidth + if (isCollapsed !== collapsedAtStart) collapsePref.val = isCollapsed window.removeEventListener('pointermove', onMove) window.removeEventListener('pointerup', stop) window.removeEventListener('pointercancel', stop) @@ -1158,22 +1175,19 @@ class="flex-1 flex flex-col min-h-0 h-screen shadow-[inset_-1px_0_0_0_rgb(var(--color-border-light))] dark:shadow-[inset_-1px_0_0_0_#374151] [html.github-dark_&]:shadow-[inset_-1px_0_0_0_rgb(var(--color-border-light))]" style:background-color={darkMode ? SIDEBAR_BG_DARK : SIDEBAR_BG} > - {#if !isCollapsed} - - - {/if} + +