Accept hosted package-app URLs as package identity - #1017
Merged
kody-bot merged 14 commits intoJul 29, 2026
Merged
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.
A URL copied out of a running package app is on the package-app origin (kodyapps.dev), so package search resolved it as invalid-package-identity. The origin check now accepts either of this deployment's own origins, derived from APP_BASE_URL and PACKAGE_APP_BASE_URL rather than hardcoded. /account/* stays app-origin only, because the package-app origin does not serve those pages. Public-origin resolution moves to a neutral #worker/app-base-url.ts so the MCP layer can read it without importing #app/*, which retires the allowlisted mcp-to-app edge and its TODO. #app/app-base-url.ts stays as a compatibility re-export.
|
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 (15)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe worker now centralizes app and package-app URL resolution, validates hosted package identity URLs against both authorized origins, preserves ChangesHosted package origin handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant SearchExecution
participant BaseUrlResolver
participant IdentitySearch
participant PackageUrlParser
SearchExecution->>BaseUrlResolver: resolve package-app origin from env
SearchExecution->>IdentitySearch: pass packageAppBaseUrl
IdentitySearch->>PackageUrlParser: parse package identity URL
PackageUrlParser-->>IdentitySearch: accept authorized origin or reject invalid URL
IdentitySearch-->>SearchExecution: return identity search result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
Contributor
|
🔎 Preview deployed: https://kody-pr-1017.kody-a99.workers.dev Worker: Mocks:
|
This was referenced Jul 29, 2026
kody-bot
pushed a commit
that referenced
this pull request
Jul 29, 2026
* Remove import-boundary compatibility re-export shims The boundary-extraction PRs (#1014, #1017) moved shared primitives to neutral #worker/* modules but left pure re-export shims in #app/* to keep their diffs small. Repoint the 30 remaining importers (including four vi.mock specifiers that would otherwise stop applying) at the canonical modules and delete the six shims; #app/username.ts already had zero importers. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> * Fix stale relative and tsconfig references to moved identity modules e2e/playwright-utils.ts imported username.ts through a relative path that the specifier-based sweep missed, and two tsconfig include lists still pointed at app/username.ts, app/reserved-usernames.ts (already gone since #1014), and app/permissions.ts. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> * Drop duplicate identity/permissions tsconfig include ./src/identity/permissions.ts was already listed further down the include list, so the repointed entry was redundant. Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
This was referenced Aug 7, 2026
Merged
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
3 participants
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.
The follow-up flagged as out-of-scope in #1013. It does belong to this track: it is a direct consequence of moving package apps to
kodyapps.dev.The problem
parsePackageSearchIdentitycompared a URL's origin against the app origin only, so the URL a user copies out of a running package app resolved asinvalid-package-identity:The fix
The origin check now accepts either of this deployment's own origins, derived from the vars (never hardcoded):
APP_BASE_URL→ the app origin, via the caller context'sbaseUrl(unchanged)PACKAGE_APP_BASE_URL→ the package-app origin, viagetPackageAppBaseUrl, whichsearch-execution.tsreads fromenvand passes downTwo deliberate boundaries kept tight:
/account/packages/:packageIdstays app-origin only, because the package-app origin does not serve account pages — accepting it there would validate a URL that 404s in a browser.PACKAGE_APP_BASE_URLunset — preview, test, local dev) do not start accepting that host, and an unparseable var falls back to app-origin-only rather than opening up.Deep links inside a running app (
/@user/packages/x/report?tab=1) remainnot-package-identity, exactly as they already were on the app origin. That is asserted rather than changed.Import boundary
#mcp/*may not import#app/*, and#app/app-base-url.tssat in the allowlist withTODO: move the base-URL resolver into a neutral #worker/* module. Rather than add a second file to that exception, this does the move the TODO asked for:packages/worker/src/app-base-url.ts(neutral#worker/app-base-url.ts)#app/app-base-url.ts→ compatibility re-export, so the 27 app-layer importers are untouched#mcp/*importers point at the neutral module, and the allowlist entry is deletedtools/oxlint/import-boundaries.node.test.tsnow asserts#app/app-base-url.tsis forbidden from#mcp/*and#worker/app-base-url.tsis fine.Checked a sibling that enforces the same rule
getSafeMarkdownLinkHref(packages/worker/client/markdown-view.tsx) refuses any/@...user-scope path regardless of host, so an untrusted README still cannot link into either origin's package surface. It needs no change, andsecurity.mdnow records why the two rules point in opposite directions on purpose.Tests
packages/worker/src/mcp/tools/package-search-identity.node.test.tscovers: hosted URL on the package-app origin resolves, app-origin and relative URLs keep working, and these stay invalid — another user's package on the package-app origin,/account/*on the package-app origin,evil-kodyapps.dev,kodyapps.dev.attacker.example, embedded credentials, and the package-app host when no package-app origin is configured.npm run validategreen locally (all 9 lanes); all CI checks green; Bugbot "no issues found", CodeRabbit "no actionable comments".Conductor Report
STATUS: done — merged (
60edb3d), deployed, and verified in production. This did belong to my track (it is a direct consequence of the origin split), so I handled it rather than routing it back.Functional verification against the live production MCP (the behavior this PR changes, not just endpoint health):
https://kodyapps.dev/@kentcdodds/packages/github@kentcdodds/github— the fix (previouslyinvalid-package-identity)https://heykody.dev/@kentcdodds/packages/githubhttps://kodyapps.dev.attacker.example/@kentcdodds/packages/githubEndpoint health after deploy (
60edb3d3), immediately and on a delayed re-check ~7 minutes later, including DNS read straight from two public resolvers to catch a flap rather than a pinned connection:No flap: the A records are intact on both resolvers and all three triggers stayed healthy. The deploy published all three (
Custom domain routes: heykody.dev (APP_BASE_URL), kodyapps.dev (PACKAGE_APP_BASE_URL); workers.dev trigger kept) and its own healthcheck, execute smoke check, and Vectorize reindex all passed.Incident-history correction for the record, per the conductor: the durable restoration of
heykody.devafter the #1013 deploy detached it was the operator's manual re-attach via the Cloudflare dashboard. #1015 briefly restored it and then it flapped back down (Cloudflare eventual consistency after the rapid detach/re-attach), so #1015's deploy alone was not what fixed it;kodyapps.devhad been attached earlier by the conductor via the Cloudflare API. I have corrected the same overstatement in #1013 and #1015, where I had credited the recovery to #1015's deploy.Optional follow-up, not done here:
hostedUrlin search results is still built from the app origin (buildPackageHostedUrl/buildPackageAppUrlwithcallerContext.baseUrl), so agents are handedheykody.dev/@user/packages/x, which 302s to the package origin. Pointing generation directly at the package-app origin would remove that hop; it needs this PR's acceptance change first, which is now in. Happy to take it if you want it.Summary by CodeRabbit
New Features
workers.devaccess path.Bug Fixes
Documentation