Keep the workers.dev trigger when publishing custom domains - #1016
Conversation
Package apps execute author-supplied code. Serving them on the app origin made that code same-site with kody_session: the owner's cookie was forwarded into the package worker (and into the realtime connect hook), and package browser code could call first-party endpoints with the owner's credentials. - PACKAGE_APP_BASE_URL selects a separate registrable domain for hosted package apps; unset keeps today's same-origin path-based behavior for local dev, preview, and tests. - Requests handed to package code are stripped of Cookie, Authorization, Proxy-Authorization, and X-Kody-* headers unconditionally. - The app origin mints a 60s single-use signed handoff token; the package-app origin exchanges it for a host-scoped kody_pkg_session cookie (distinct name, distinct derived secret, distinct payload) and serves nothing else.
Verified against a real wrangler dev server with a second local origin: - Swapping origins by assigning URL.protocol/host keeps the original port, so the app origin redirected to https://kodyapps.dev:8787. Build the target with the URL constructor instead, and give the workers test different ports so the regression cannot hide. - npm run dev runs the production wrangler env, so the committed PACKAGE_APP_BASE_URL reached local dev and would have sent local package-app traffic to the real domain. Local dev now honors only an origin it can answer on (localhost / *.localhost), so PACKAGE_APP_BASE_URL=http://packages.localhost:<port> in .env still exercises the two-origin flow. - Drop the custom_domain route from wrangler.jsonc: with a route configured, wrangler dev resolves every local request as http://kodyapps.dev/..., which broke canonical URLs, OAuth resource metadata, and redirects locally. The domain is attached to the Worker out-of-band like heykody.dev; docs say how.
The deploy now provisions the kodyapps.dev Workers custom domain (DNS +
edge cert) instead of needing a manual attach: the generated deploy
config derives routes: [{ pattern: <PACKAGE_APP_BASE_URL host>,
custom_domain: true }] from that environment's var, so the host the
Worker routes on and the domain the deploy attaches cannot drift.
The route stays out of wrangler.jsonc on purpose. Wrangler resolves local
dev request URLs against the first configured route, so a committed route
made every npm run dev request arrive as http://kodyapps.dev/... —
canonical URLs, OAuth resource metadata, and login redirects all left
localhost. dev.host can override that host but drops (or hardcodes) the
port, and pinning it to localhost would break the Playwright suite, which
runs on 127.0.0.1.
Preview and test have no PACKAGE_APP_BASE_URL, so they get no route.
… gate - Extract isSessionInvalidatedByStoredPasswordChange so browser and package-app session resolution cannot drift on a security-critical fail-closed check. - Gate the local-dev package-app-origin override on WRANGLER_IS_LOCAL_DEV === 'true' (matching isNonProductionRuntime), so no other value can pull package apps back onto the app origin in a real deployment. - Rewrite a request that still carries __kody_handoff before serving, so a stale or forged token never reaches package code. - Docs: stop claiming the redirect makes the token unusable from history or a referrer; expiry and single-use are what bound a leak.
…e-app-domain-isolation-102f # Conflicts: # packages/worker/src/app/request-auth-cache.ts # packages/worker/src/app/username.ts
A signature-valid token aimed at another package path was consumed and
then rejected, so a mistyped URL cost the owner a handoff they still
held. consumePackageAppHandoffToken now takes the expected
{username, kodyId} and verifies the binding before the replay burn.
Also corrects the shared password-change docstring: the missing-issuedAt
fail-closed only applies once an account has a stored password_changed_at.
Accounts that never changed a password keep pre-issuedAt cookies, which is
the documented tradeoff in security.md, not an oversight.
Wrangler treats a configured routes list as the Worker's complete custom-domain set, not an addition to it. The previous deploy published only kodyapps.dev, which detached heykody.dev and deleted its DNS record — production returned Cloudflare 530 until this deploy republishes both. The generator now derives one custom_domain route per base-URL var (APP_BASE_URL and PACKAGE_APP_BASE_URL), and fails the deploy when PACKAGE_APP_BASE_URL is set without APP_BASE_URL instead of shipping a partial set. Covered by a test asserting both routes and the fail-closed guard.
…e-app-domain-isolation-102f # Conflicts: # docs/contributing/setup-manifest.md # tools/ci/resource-utils.node.test.ts # tools/ci/resource-utils.ts
…e-app-domain-isolation-102f Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Publishing routes flips workers_dev to false, so the previous deploy silently dropped kody-production.kody-a99.workers.dev (Cloudflare now answers it with error 1042). That hostname is a documented access path — MCP clients can be pointed at it and the deploy's URL fallback looks for it — so the generator now asks for it explicitly alongside the routes.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughCustom-domain route generation now preserves the ChangesCustom route access
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔎 Preview deployed: https://kody-pr-1016.kody-a99.workers.dev Worker: Mocks:
|
Second-order fallout from the custom-domain work in #1013, found while verifying the recovered production deploy.
What broke
Publishing
routesflips Wrangler'sworkers_devtofalse. Comparing deploy trigger output:https://kody-production.kody-a99.workers.dev/healthanswered with Cloudflare error 1042 (workers.dev disabled for this Worker). That hostname is a documented access path, not incidental:docs/contributing/architecture/request-lifecycle.mdnotes MCP clients may connect through a workers.dev host and that resource metadata is derived from the inbound origin for exactly that reason, anddeploy.ymlfalls back to scraping a*.workers.devURL out of the deploy log whenAPP_BASE_URLis unset.The fix
writeGeneratedWranglerConfigsetsworkers_dev: truein the same place it publishes the custom-domain routes, so attaching a domain cannot silently remove the workers.dev trigger. Preview and test are untouched (they publish no routes, so they keep whatever triggers they had).tools/ci/resource-utils.node.test.tsassertsworkers_dev: truefor production and absent for preview;docs/contributing/setup-manifest.mdrecords the interaction.Verified in production (deploy of
f661e3f)The deploy published all three triggers and every post-deploy step passed, including the healthcheck, execute smoke check, and Vectorize reindex that the previous failed deploy had skipped:
Live check of all three:
Conductor Report
f661e3f), deployed, and verified. Production was healthy throughout this one; the outage was fixed earlier by Hotfix: publish both custom domains so a deploy cannot detach the app origin #1015.npm run validategreen locally; all CI checks green; Bugbot "no issues found", CodeRabbit "no actionable comments".setup-manifest.md: the generatedrouteslist is the Worker's complete custom-domain set, so any domain attached out-of-band must be added there before the next deploy.Summary by CodeRabbit