fix: align cors headers with production contract#994
Conversation
👷 Deploy request for docmagic-muneer pending review.Visit the deploys page to approve it
|
👷 Deploy request for docmagic1 pending review.Visit the deploys page to approve it
|
|
@saurabhhhcodes is attempting to deploy a commit to the muneerali199's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR centralizes security and CORS header handling by extracting shared definitions and utilities into dedicated modules, then refactoring middleware and Next.js config to consume these shared helpers instead of maintaining duplicate local header logic. ChangesSecurity headers centralization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed due to a network error. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
middleware.ts (1)
6-11: ⚡ Quick win
DEPLOYMENT_ERROR_PATTERNSis defined but never used.The constant is declared with regex patterns for deployment error detection, but
isDeploymentError()only checks HTTP status codes. Either remove this dead code or integrate the patterns into the detection logic if body/header matching was intended.🤖 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 `@middleware.ts` around lines 6 - 11, DEPLOYMENT_ERROR_PATTERNS is declared but unused; either delete the constant or integrate it into isDeploymentError(): update isDeploymentError() to not only check numeric status codes but also accept a response text/headers string (or response object) and test it against each regex in DEPLOYMENT_ERROR_PATTERNS (e.g., loop patterns and return true on a match), or if body/header matching is not needed, remove DEPLOYMENT_ERROR_PATTERNS entirely to eliminate dead code; refer to the symbols DEPLOYMENT_ERROR_PATTERNS and isDeploymentError() when making the change.
🤖 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 `@middleware.ts`:
- Around line 158-161: The deployment-error detection in middleware.ts is
ineffective because isDeploymentError(r) is called with r from
NextResponse.next(), which is a pass-through with status 200; remove or disable
the isDeploymentError checks and related calls (the blocks that call
isDeploymentError(r), logError(pathname, ...), and
r.headers.set("X-Deployment-Error", "true")) from the middleware and instead
implement the detection in code that has access to real server responses—for
example inside your API route handlers, React error boundaries, or an
observability layer where you can call isDeploymentError(response) against the
actual Response object and then call logError and set any headers on that real
response.
---
Nitpick comments:
In `@middleware.ts`:
- Around line 6-11: DEPLOYMENT_ERROR_PATTERNS is declared but unused; either
delete the constant or integrate it into isDeploymentError(): update
isDeploymentError() to not only check numeric status codes but also accept a
response text/headers string (or response object) and test it against each regex
in DEPLOYMENT_ERROR_PATTERNS (e.g., loop patterns and return true on a match),
or if body/header matching is not needed, remove DEPLOYMENT_ERROR_PATTERNS
entirely to eliminate dead code; refer to the symbols DEPLOYMENT_ERROR_PATTERNS
and isDeploymentError() when making the change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9dc57003-47a3-4a4b-8d4f-04e620774661
📒 Files selected for processing (4)
lib/security-headers.mjslib/security-headers.tsmiddleware.tsnext.config.js
| if (isDeploymentError(r)) { | ||
| logError(pathname, 'Deployment error detected', r.status, Date.now()); | ||
| r.headers.set('X-Deployment-Error', 'true'); | ||
| logError(pathname, "Deployment error detected", r.status, Date.now()); | ||
| r.headers.set("X-Deployment-Error", "true"); | ||
| } |
There was a problem hiding this comment.
isDeploymentError(r) cannot detect errors in middleware at both middleware.ts lines 158-161 and 185-188.
Both locations call isDeploymentError(r) where r is the result of NextResponse.next(). This creates a pass-through response object, not the actual server response—middleware executes before the request reaches the server. The status is always 200, so these checks are dead code.
To detect deployment errors, move this logic to API route handlers, error boundaries, or an observability layer with access to actual responses.
🤖 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 `@middleware.ts` around lines 158 - 161, The deployment-error detection in
middleware.ts is ineffective because isDeploymentError(r) is called with r from
NextResponse.next(), which is a pass-through with status 200; remove or disable
the isDeploymentError checks and related calls (the blocks that call
isDeploymentError(r), logError(pathname, ...), and
r.headers.set("X-Deployment-Error", "true")) from the middleware and instead
implement the detection in code that has access to real server responses—for
example inside your API route handlers, React error boundaries, or an
observability layer where you can call isDeploymentError(response) against the
actual Response object and then call logError and set any headers on that real
response.
Muneerali199
added
type:bug
|
@SaurabhhCodes please rebase on latest main to resolve conflicts |
2 participants
Summary\n- align CORS and security header values with the production contract\n- keep both TS and MJS helpers in sync\n\n## Validation\n- reviewed the header config diff\n- no unrelated app changes
Summary by CodeRabbit
Release Notes
New Features
Security