fix(nextjs): add auto proxy opt-out#9159
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: cf141a6 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 |
📝 WalkthroughWalkthroughAdds ChangesAutomatic proxy opt-out
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@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/vue
commit: |
f5ba17d to
c6bf6d3
Compare
API Changes Report
Summary
@clerk/sharedCurrent version: 4.25.3 Subpath
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.changeset/disable-vercel-auto-proxy.md (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new environment variable outside the changeset.
Add user-facing documentation explaining where to set
CLERK_DISABLE_AUTO_PROXY, its accepted values, and that it disables only automatic proxying—not explicit proxy or domain configuration. As per coding guidelines, documentation should be updated for API changes.🤖 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 @.changeset/disable-vercel-auto-proxy.md at line 7, Document CLERK_DISABLE_AUTO_PROXY in the user-facing configuration or deployment documentation, including where to set it, its accepted values, and that it disables only automatic Vercel proxying while preserving explicit proxy or domain configuration. Keep the changeset focused on release notes and place the new explanation in the established API/environment-variable documentation section.Source: Coding guidelines
🤖 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.
Inline comments:
In `@packages/nextjs/src/server/clerkMiddleware.ts`:
- Around line 171-176: Add middleware test coverage for the auto-proxy opt-out
branch in clerkMiddleware: set CLERK_DISABLE_AUTO_PROXY=true, use a live
publishable key and eligible Vercel hostname, then assert the proxy handler is
not invoked. Keep the test focused on the condition guarded by
isAutoProxyDisabledFromEnvironment and preserve existing proxy behavior for
other configurations.
In `@packages/nextjs/src/utils/__tests__/mergeNextClerkPropsWithEnv.test.ts`:
- Around line 52-61: Make the environment-sensitive tests deterministic by
clearing or explicitly disabling CLERK_DISABLE_AUTO_PROXY in shared setup for
auto-proxy-enabled cases, while setting it to true only in opt-out cases. Apply
this in packages/nextjs/src/utils/__tests__/mergeNextClerkPropsWithEnv.test.ts
at lines 52-61 and
packages/backend/src/tokens/__tests__/authenticateContext.test.ts at lines
305-313, preserving the existing expectations for /__clerk and automatic proxy
derivation.
---
Nitpick comments:
In @.changeset/disable-vercel-auto-proxy.md:
- Line 7: Document CLERK_DISABLE_AUTO_PROXY in the user-facing configuration or
deployment documentation, including where to set it, its accepted values, and
that it disables only automatic Vercel proxying while preserving explicit proxy
or domain configuration. Keep the changeset focused on release notes and place
the new explanation in the established API/environment-variable documentation
section.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 41c855cf-7afb-4be3-8fca-fe1339e62e98
📒 Files selected for processing (6)
.changeset/disable-vercel-auto-proxy.mdpackages/backend/src/tokens/__tests__/authenticateContext.test.tspackages/nextjs/src/server/clerkMiddleware.tspackages/nextjs/src/utils/__tests__/mergeNextClerkPropsWithEnv.test.tspackages/shared/src/__tests__/proxy.spec.tspackages/shared/src/proxy.ts
| if ( | ||
| !frontendApiProxyConfig && | ||
| !hasExplicitProxyOrDomain && | ||
| !isAutoProxyDisabledFromEnvironment() && | ||
| isProductionFromPublishableKey(publishableKey) | ||
| ) { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== clerkMiddleware excerpt =="
sed -n '130,220p' packages/nextjs/src/server/clerkMiddleware.ts
echo
echo "== search for auto proxy / disable auto proxy / middleware tests =="
rg -n "CLERK_DISABLE_AUTO_PROXY|isAutoProxyDisabledFromEnvironment|proxy handler|auto proxy|frontendApiProxyConfig|hasExplicitProxyOrDomain|isProductionFromPublishableKey|vercel" packages/nextjs -g '*.{ts,tsx,test.ts,test.tsx,spec.ts,spec.tsx}'
echo
echo "== list test files near nextjs server middleware =="
fd -a "clerkMiddleware" packages/nextjs
fd -a "middleware" packages/nextjs/src packages/nextjs/test packages/nextjs/__tests__ 2>/dev/null || trueRepository: clerk/javascript
Length of output: 9457
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files of interest =="
git ls-files 'packages/nextjs/**/*' | rg 'clerkMiddleware|middleware|proxy|vercel|test|spec'
echo
echo "== text search for disable auto proxy across repo =="
rg -n "CLERK_DISABLE_AUTO_PROXY|isAutoProxyDisabledFromEnvironment|auto proxy|proxy handler|frontendApiProxyConfig|hasExplicitProxyOrDomain|isProductionFromPublishableKey" .
echo
echo "== if tests exist, print filenames =="
rg -l "CLERK_DISABLE_AUTO_PROXY|isAutoProxyDisabledFromEnvironment|isProductionFromPublishableKey" packages/nextjsRepository: clerk/javascript
Length of output: 4975
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate test files =="
fd -a -e ts -e tsx -e test.ts -e spec.ts packages/nextjs | rg 'clerkMiddleware|middleware|proxy|vercel'
echo
echo "== read smallest likely test files =="
for f in $(fd -a -e ts -e tsx -e test.ts -e spec.ts packages/nextjs | rg 'clerkMiddleware|middleware|proxy|vercel' | head -n 10); do
echo "---- $f ----"
wc -l "$f"
sed -n '1,260p' "$f"
doneRepository: clerk/javascript
Length of output: 547
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TEST=packages/nextjs/src/server/__tests__/clerkMiddleware.test.ts
echo "== file size =="
wc -l "$TEST"
echo
echo "== outline =="
ast-grep outline "$TEST" --view expanded | sed -n '1,260p'
echo
echo "== relevant ranges around vercel/proxy tests =="
sed -n '1160,1395p' "$TEST"
echo
echo "== search for CLERK_DISABLE_AUTO_PROXY in clerkMiddleware.test.ts =="
rg -n "CLERK_DISABLE_AUTO_PROXY|shouldAutoProxy|proxy handler|frontendApiProxy|vercel" "$TEST"Repository: clerk/javascript
Length of output: 11562
Add coverage for the auto-proxy opt-out branch. clerkMiddleware makes this decision independently, so add a middleware test with CLERK_DISABLE_AUTO_PROXY=true, a live key, and an eligible Vercel hostname, and assert the proxy handler is skipped.
🤖 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/nextjs/src/server/clerkMiddleware.ts` around lines 171 - 176, Add
middleware test coverage for the auto-proxy opt-out branch in clerkMiddleware:
set CLERK_DISABLE_AUTO_PROXY=true, use a live publishable key and eligible
Vercel hostname, then assert the proxy handler is not invoked. Keep the test
focused on the condition guarded by isAutoProxyDisabledFromEnvironment and
preserve existing proxy behavior for other configurations.
Source: Coding guidelines
| it('does not auto-derive proxyUrl when auto-proxy is disabled', () => { | ||
| process.env.CLERK_DISABLE_AUTO_PROXY = 'true'; | ||
| process.env.NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY = 'pk_live_Zm9vLmNsZXJrLmNvbSQ='; | ||
| process.env.VERCEL_TARGET_ENV = 'production'; | ||
| process.env.VERCEL_PROJECT_PRODUCTION_URL = 'myapp.vercel.app'; | ||
|
|
||
| const result = mergeNextClerkPropsWithEnv({}); | ||
|
|
||
| expect(result.proxyUrl).toBe(''); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Keep environment-sensitive tests deterministic.
Both suites can inherit CLERK_DISABLE_AUTO_PROXY=true from the test runner, causing auto-proxy-enabled cases to fail. Clear or explicitly disable the variable in shared setup, and set it to true only in opt-out cases.
packages/nextjs/src/utils/__tests__/mergeNextClerkPropsWithEnv.test.ts#L52-L61: isolate the flag before tests expecting/__clerk.packages/backend/src/tokens/__tests__/authenticateContext.test.ts#L305-L313: isolate the flag before tests expecting automatic proxy derivation.
🧰 Tools
🪛 Betterleaks (1.6.1)
[high] 54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
📍 Affects 2 files
packages/nextjs/src/utils/__tests__/mergeNextClerkPropsWithEnv.test.ts#L52-L61(this comment)packages/backend/src/tokens/__tests__/authenticateContext.test.ts#L305-L313
🤖 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/nextjs/src/utils/__tests__/mergeNextClerkPropsWithEnv.test.ts`
around lines 52 - 61, Make the environment-sensitive tests deterministic by
clearing or explicitly disabling CLERK_DISABLE_AUTO_PROXY in shared setup for
auto-proxy-enabled cases, while setting it to true only in opt-out cases. Apply
this in packages/nextjs/src/utils/__tests__/mergeNextClerkPropsWithEnv.test.ts
at lines 52-61 and
packages/backend/src/tokens/__tests__/authenticateContext.test.ts at lines
305-313, preserving the existing expectations for /__clerk and automatic proxy
derivation.
Allow opting out of auto proxy behavior by setting the
CLERK_DISABLE_AUTO_PROXYenv var. Used for AP to ensure the proxied setup doesn't attempt to proxy FAPI whenvercel.appis detected as the domain.