fix: read OAuth env from cloudflare workers runtime#1482
Conversation
🦋 Changeset detectedLatest commit: a52df8c The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
review/needs-review
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a shared helper to resolve OAuth provider environment bindings across Astro adapters (Cloudflare Workers vs Node), replacing prior locals.runtime.env usage removed in Astro v6.
Changes:
- Introduces
getOAuthEnv()to read Cloudflare bindings viacloudflare:workerswith fallback toimport.meta.env - Updates OAuth authorize and callback API routes to use
getOAuthEnv()instead oflocals.runtime.env - Removes repeated locals/env-casting logic from route handlers
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/core/src/auth/oauth-env.ts | New helper to resolve env/bindings across Cloudflare Workers and non-Cloudflare runtimes |
| packages/core/src/astro/routes/api/auth/oauth/[provider]/callback.ts | Switches env lookup to getOAuthEnv() for provider config resolution |
| packages/core/src/astro/routes/api/auth/oauth/[provider].ts | Switches env lookup to getOAuthEnv() for provider config resolution |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| // @ts-ignore - runtime-only Cloudflare Workers virtual module | ||
| const { env } = await import("cloudflare:workers"); | ||
| return env as Record<string, unknown>; |
| // @ts-ignore - runtime-only Cloudflare Workers virtual module | ||
| const { env } = await import("cloudflare:workers"); |
| } catch { | ||
| return import.meta.env as Record<string, unknown>; | ||
| } |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
The approach is correct: switching from locals.runtime.env to the cloudflare:workers virtual module with an import.meta.env fallback is exactly what AGENTS.md recommends and matches the pattern already used elsewhere in the repo (e.g., packages/plugins/ai-moderation/src/guard.ts). The change is appropriately narrow.
However, there are two AGENTS.md convention gaps that need addressing before merge:
- Missing changeset —
packages/coreis a published package, and bug fixes require a changeset per CONTRIBUTING.md/AGENTS.md. - Missing test — AGENTS.md requires TDD for bugs (
Failing test -> fix -> verify. A bug without a reproducing test is not fixed.). A unit test forgetOAuthEnvverifying the fallback behavior should be added. Even a minimal Vitest test that mocks thecloudflare:workersmodule (or verifies the catch path when the dynamic import fails) would satisfy the convention.
| @@ -0,0 +1,16 @@ | |||
| /** | |||
There was a problem hiding this comment.
[needs fixing] This file adds a new helper used by OAuth routes, but the PR includes no test for it. AGENTS.md requires TDD for bugs: "Failing test -> fix -> verify. A bug without a reproducing test is not fixed." Add at least a minimal unit test that verifies getOAuthEnv falls back to import.meta.env when the cloudflare:workers dynamic import throws. Vitest supports module-level mocking via vi.mock or temporal import interception for this.
| @@ -0,0 +1,16 @@ | |||
| /** | |||
There was a problem hiding this comment.
[needs fixing] The PR changes a published package (packages/core) but does not include a changeset. Per AGENTS.md: "A changeset is release notes a user reads while upgrading... lead with a present-tense verb (Fixes, Adds, Updates, Removes), describe the observable effect, and leave out internal mechanics." Add a changeset describing the fix for Cloudflare Workers OAuth login.
Overlapping PRsThis PR modifies files that are also changed by other open PRs: This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
github-actions
Bot
added
review/awaiting-author
Astro v6 removed the Cloudflare-specific locals.runtime.env path. Read OAuth provider secrets from cloudflare:workers when available and fall back to import.meta.env elsewhere so GitHub/Google login can start correctly on Workers deployments.
Address the minimal review feedback for the OAuth runtime fix by making the dynamic Cloudflare import bundler-safe, narrowing the fallback path, adding a focused unit test, and adding the required patch changeset.
Keep the Cloudflare runtime import path dynamic while avoiding the last unsafe assertion called out by lint/review feedback.
richardjoo
force-pushed
the
upstream/fix-oauth-env-cloudflare-v6
branch
from
734656b to
9756538
Compare
|
we need a fix for this |
|
Checked the current branch state on this PR: the earlier review asks are now covered.
The current PR checks on the branch are green from what I can see. If there is still a specific issue you are seeing, could you point to the exact failure mode or log? That would make it much easier to verify whether there is another edge case left to fix. |
What does this PR do?
Fixes OAuth login on Cloudflare Workers/Astro v6 by stopping the OAuth routes from reading provider secrets from the removed
Astro.locals.runtime.envpath. Instead, the routes read runtime bindings fromcloudflare:workerswhen available and fall back toimport.meta.envelsewhere.Closes #
Type of change
Checklist
pnpm --filter emdash typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.Notes:
/_emdash/api/auth/oauth/githubfailed at runtime with:Astro.locals.runtime.env has been removed in Astro v6.pnpm --filter emdash typecheck.AI-generated code disclosure
Screenshots / test output
/_emdash/api/auth/oauth/githubnow redirects correctly to GitHub OAuth instead of failing back to the local login page.