feat: add per-route CORS origin allowlist for HTTP triggers - #10833
Draft
hugocasa wants to merge 6 commits into
Draft
feat: add per-route CORS origin allowlist for HTTP triggers#10833hugocasa wants to merge 6 commits into
hugocasa wants to merge 6 commits into
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deploying windmill with
|
| Latest commit: |
8df2b2c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d4768778.windmill.pages.dev |
| Branch Preview URL: | https://http-trigger-cors-config.windmill.pages.dev |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #10826.
Every response from
/api/r/*is stampedAccess-Control-Allow-Origin: *, and a route owner has no way to narrow it. The documented escape hatch — returningwm_headersfrom the runnable — is narrower than it looks: it is applied byresult_to_response, which is only reached from the sync path, soasync,sync_sseand static-asset routes never had it either.The structural gap is the preflight.
OPTIONSwas answered by the router with an empty body before any trigger lookup happened, and the middleware then stamped the permissive defaults onto it because its skip-if-present logic only skips headers the response already carries. No amount ofwm_headerscan reach that — by the time the runnable returns, a route whose purpose is a side effect has already run.This adds an origin allowlist at two levels, enforced in the middleware for both the preflight and the response.
Resolution order
route
allowed_origins→http_route_default_allowed_originsinstance setting →*ACAO: *, andwm_headersstill wins on sync routesNULL["*"]wm_headersescape hatch[]NULL, which inheritsBecause the preflight is answered before any code runs, config is the only thing it can consult. So whenever an allowlist is in effect it also bounds
wm_headers: letting the response widen what the preflight advertised would make the two disagree and leave the allowlist bounding nothing. A route that genuinely computes its own origin opts out with["*"].The instance setting is a default, not a ceiling — a workspace user can still set
["*"]on their own route. It is a house-style convenience for self-hosted admins, not a tenant boundary; against an untrusted tenant the control isauthentication_method, since CORS only stops a browser reading a response, not the request itself. It is an instance setting rather than an env var so a malformed origin is rejected at write time — a typo matches no request, so via env it would silently block the very app it names.Access-Control-Allow-Credentialsis still never set, so this is not a cookie-CSRF change. The exposure it closes is unauthenticated routes, and routes whose credential is a header the calling page already holds.Workspace-level defaults are deliberately left out — a tenant can already do this per route, and inserting a workspace step into the resolution order later is non-breaking.
Changes
allowed_origins TEXT[]tohttp_triggerallowed_originsonTriggerRoute,HttpConfigandHttpConfigRequest, threaded through create, update, therefresh_routersquery, and the workspace-fork cloneHTTP_ROUTE_DEFAULT_ALLOWED_ORIGINSinstance setting, validated on write and hot-reloaded (no restart, and no router-cache bump — it is read per request)match_origin(exact, case-insensitive, echoes the request origin only on a match — never reflects unchecked) andvalidate_allowed_origins(rejects paths, queries, userinfo, whitespace, non-ASCII andnull, since a sandboxed iframe sendsOrigin: null), shared by the route field and the settingconditional_cors_middlewareresolves the trigger fromHTTP_ROUTERS_CACHEand is now the single place CORS headers are decided, covering the preflight without a second code path that could driftRouteCorsOption.svelte: a "Restrict origins" toggle and comma-separated field in Advanced → Request Options, with inline validation mirroring the backend, and the inherited instance default shown when the route sets noneScreenshots
An inherited restriction is visible without expanding the section, and the field names the origins it inherited:
Test plan
Exercised against a running instance built with
--features quickjs,http_trigger(/api/r/*is not mounted in the default dev build).ACAO: *with the full method list on both preflight and response, andwm_headersstill overrides itVary: origin; disallowed → noACAOon both preflight and responseNULL, applies without a restart, and boundswm_headers["*"]on a route opts out of the instance default entirely —wm_headerswins again, full method list, noVarylocalhost:3140fetching the route onlocalhost:8140reads the body from an allowed origin; from a disallowed one Chromium reports "Response to preflight request doesn't pass access control check" and the job count is unchanged — the runnable never executes/api/r/corspr%6fbe) resolve to the same trigger the handler serves, so the allowlist cannot be stepped around by re-encoding a characterNULLCORSbadge on the Advanced header tracks the same rule the backend applies: on for a route list or an inherited default, off when nothing is configured or the route opted out with["*"]http_route_default_allowed_origins(so the inherited hint renders for the people who did not set it), whilelicense_keystays superadmin-onlycargo check --features http_triggerand--features http_trigger,enterprise,private; 112 unit tests inwindmill-trigger-http;npm run check0 errors; sqlx cache regenerated (+5 entries, 0 lost)Not exercised at runtime: the branch where the routers are genuinely unloadable because the database is unreachable. Reaching it needs a failed load and a cold cache at once, which is only reproducible by disrupting the Postgres shared with other worktrees. It fails closed by omitting the header.
🤖 Generated with Claude Code