Uh oh!
There was an error while loading. Please reload this page.
fix(astro): Do not consume the request body in middleware - #9220
Conversation
🦋 Changeset detectedLatest commit: 18f0ebf The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 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 |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe change adds a shared Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/electron@clerk/electron-passkeys@clerk/eslint-plugin@clerk/expo@clerk/expo-google-signin@clerk/expo-passkeys@clerk/express@clerk/fastify@clerk/hono@clerk/localizations@clerk/nextjs@clerk/nuxt@clerk/react@clerk/react-router@clerk/shared@clerk/tanstack-react-start@clerk/testing@clerk/ui@clerk/upgrade@clerk/vuecommit: |
API Changes Report
Summary
@clerk/sharedCurrent version: 4.25.6 Subpath |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/astro/src/server/utils.ts (1)
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit return type to
patchRequest.Use
: Requeston this exported utility to make its API contract explicit.As per coding guidelines, exported TypeScript functions must declare explicit return types. Based on learnings, exported functions in this repository should enforce the same annotation standard.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/astro/src/server/utils.ts` at line 30, Update the exported patchRequest function signature to explicitly declare a Request return type, preserving its existing implementation and behavior.Sources: Coding guidelines, Learnings
packages/astro/src/server/clerk-middleware.ts (1)
86-87: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a middleware regression test for body preservation.
Exercise a body-bearing request through
clerkMiddleware(), then assert that the downstream Astro handler can still readcontext.request.json(). This should also verify that Clerk receives the cloned body rather than an empty request.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/astro/src/server/clerk-middleware.ts` around lines 86 - 87, Add a regression test around clerkMiddleware() that sends a request with a JSON body, verifies Clerk receives the cloned body, and confirms the downstream Astro handler can still read the same payload through context.request.json().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/astro/src/server/clerk-middleware.ts`:
- Around line 86-87: Add a regression test around clerkMiddleware() that sends a
request with a JSON body, verifies Clerk receives the cloned body, and confirms
the downstream Astro handler can still read the same payload through
context.request.json().
In `@packages/astro/src/server/utils.ts`:
- Line 30: Update the exported patchRequest function signature to explicitly
declare a Request return type, preserving its existing implementation and
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e2dd58c6-906c-4253-9883-4f8b41f19016
📒 Files selected for processing (3)
.changeset/cyan-needles-listen.mdpackages/astro/src/server/clerk-middleware.tspackages/astro/src/server/utils.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/__tests__/patchRequest.spec.ts (1)
45-54: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that the original body remains readable.
bodyUsed === falseconfirms the stream was not disturbed, but the test does not exercise the downstream operation this PR protects. Read the original body afterpatchRequestand assert its contents.As per coding guidelines, unit tests for new functionality must verify behavior and edge cases.
Suggested test addition
- it('clones POST requests without forwarding the body', () => {+ it('clones POST requests without forwarding the body', async () => { ... expect(cloned.body).toBeNull(); expect(original.bodyUsed).toBe(false); + expect(await original.text()).toBe('payload');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/__tests__/patchRequest.spec.ts` around lines 45 - 54, Update the POST request test around patchRequest to read the original request body after patchRequest(original) and assert that it remains readable with the expected “payload” contents, while preserving the existing method, cloned-body, and bodyUsed assertions.Source: Coding guidelines
🧹 Nitpick comments (1)
packages/shared/src/patchRequest.ts (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an explicit return type to
patchRequest.
Use: Requesthere so the exported helper’s contract is obvious.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/patchRequest.ts` at line 6, Update the exported patchRequest function signature to explicitly declare the Request return type, preserving its existing implementation and behavior.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/shared/src/__tests__/patchRequest.spec.ts`:
- Around line 45-54: Update the POST request test around patchRequest to read
the original request body after patchRequest(original) and assert that it
remains readable with the expected “payload” contents, while preserving the
existing method, cloned-body, and bodyUsed assertions.
---
Nitpick comments:
In `@packages/shared/src/patchRequest.ts`:
- Line 6: Update the exported patchRequest function signature to explicitly
declare the Request return type, preserving its existing implementation and
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 8994fc8a-ec3b-4a71-b58d-3e09ae8c29ff
📒 Files selected for processing (9)
.changeset/cyan-needles-listen.mdpackages/astro/src/server/clerk-middleware.tspackages/react-router/src/server/clerkMiddleware.tspackages/react-router/src/server/loadOptions.tspackages/react-router/src/server/utils.tspackages/shared/src/__tests__/patchRequest.spec.tspackages/shared/src/patchRequest.tspackages/tanstack-react-start/src/server/clerkMiddleware.tspackages/tanstack-react-start/src/server/utils/index.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)
💤 Files with no reviewable changes (2)
- packages/react-router/src/server/utils.ts
- packages/tanstack-react-start/src/server/utils/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/cyan-needles-listen.md
- packages/astro/src/server/clerk-middleware.ts
Description
This PR fixes#9210 by ensuring Astro’s
clerkMiddleware()does not consume the original request body during authentication.It also moves the existing
patchRequest()implementations from React Router and TanStack React Start into@clerk/shared. Astro now uses the same shared helper, removing the duplicated implementations across the three SDKs.In the future, we could consider applying this behavior inside
authenticateRequest()itself.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change