Conversation
📝 WalkthroughWalkthroughThis pull request implements Content Security Policy (CSP) nonce support across the application. It introduces server-side nonce generation via middleware, integrates nonce passing through the layout and theme provider components, adds security headers to the Next.js config, and tightens npm audit requirements in CI/CD pipelines. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Pull request overview
This PR improves security headers by migrating from Vercel's header configuration to Next.js middleware with Content Security Policy (CSP) implementation. The changes introduce a nonce-based CSP for script and style protection while maintaining other security headers.
Changes:
- Moved security header configuration from vercel.json to next.config.ts
- Added new middleware (proxy.ts) to implement CSP with nonce generation
- Updated ThemeProvider to support CSP nonces for inline styles
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| vercel.json | Removed header configuration and framework declaration (moved to next.config.ts) |
| proxy.ts | New middleware file implementing CSP with nonce-based script/style protection |
| next.config.ts | Added headers configuration for X-Content-Type-Options, X-Frame-Options, Referrer-Policy, and Permissions-Policy |
| components/theme/theme-provider.tsx | Added nonce prop support for CSP compliance |
| app/layout.tsx | Made RootLayout async to retrieve nonce from headers and pass to ThemeProvider |
| Dockerfile | Added --ignore-scripts flag to npm install for improved security |
| .github/workflows/ci.yml | Changed npm audit level from moderate to high and removed continue-on-error |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| export function proxy(request: NextRequest) { | ||
| const nonce = Buffer.from(crypto.randomUUID()).toString("base64"); |
There was a problem hiding this comment.
The nonce generation is not cryptographically secure. Using Buffer.from(crypto.randomUUID()).toString("base64") generates a nonce by encoding a UUID, but crypto.randomUUID() generates a UUID v4 which is intended for unique identifiers, not cryptographic nonces.
For CSP nonces, use crypto.getRandomValues() or crypto.randomBytes() to generate truly random bytes. For example: Buffer.from(crypto.getRandomValues(new Uint8Array(16))).toString("base64") or in Node.js: crypto.randomBytes(16).toString("base64").
| export function proxy(request: NextRequest) { | |
| const nonce = Buffer.from(crypto.randomUUID()).toString("base64"); | |
| import { randomBytes } from "crypto"; | |
| export function proxy(request: NextRequest) { | |
| const nonce = randomBytes(16).toString("base64"); |
| object-src 'none'; | ||
| base-uri 'self'; | ||
| form-action 'self'; | ||
| frame-ancestors 'none'; |
There was a problem hiding this comment.
The CSP header does not include connect-src directive, which defaults to default-src 'self'. This might be too restrictive if the application needs to make API calls to external services. Consider explicitly setting connect-src to match your application's requirements.
Additionally, consider adding other security directives like worker-src, manifest-src, and media-src if the application uses service workers, web manifests, or media resources.
| frame-ancestors 'none'; | |
| frame-ancestors 'none'; | |
| connect-src 'self'; | |
| worker-src 'self'; | |
| manifest-src 'self'; | |
| media-src 'self'; |
|
|
||
| # Install all dependencies (including dev) for build | ||
| RUN npm install | ||
| RUN npm install --ignore-scripts |
There was a problem hiding this comment.
Adding --ignore-scripts to npm install in the build stage may prevent necessary build scripts from running. While this flag improves security by preventing arbitrary script execution, it can break packages that rely on postinstall scripts for compilation or setup.
Ensure that all dependencies work correctly without their install scripts, or consider using a more targeted approach like npm config set ignore-scripts true for specific packages only. Test the build thoroughly to verify no functionality is broken.
| RUN npm install --ignore-scripts | |
| RUN npm install |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
next.config.ts (1)
6-30: Good security header configuration.The headers cover essential protections: MIME sniffing prevention, clickjacking defense, referrer control, and permissions restrictions.
Consider adding
Strict-Transport-Security(HSTS) if the application is served exclusively over HTTPS:{ key: 'Strict-Transport-Security', value: 'max-age=31536000; includeSubDomains', },Note:
X-XSS-Protectionwas previously invercel.jsonbut is omitted here—this is acceptable since the header is deprecated in modern browsers in favor of CSP.proxy.ts (1)
4-4: Consider usingcrypto.getRandomValues()for nonce generation.While
crypto.randomUUID()provides sufficient entropy (122 bits), a more direct approach for CSP nonces usescrypto.getRandomValues():Alternative nonce generation
- const nonce = Buffer.from(crypto.randomUUID()).toString("base64"); + const nonce = Buffer.from(crypto.getRandomValues(new Uint8Array(16))).toString("base64");This generates 128 bits of cryptographically secure randomness directly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/ci.ymlDockerfileapp/layout.tsxcomponents/theme/theme-provider.tsxnext.config.tsproxy.tsvercel.json
🧰 Additional context used
🧬 Code graph analysis (1)
app/layout.tsx (2)
next.config.ts (1)
headers(6-30)components/theme/theme-provider.tsx (1)
ThemeProvider(9-22)
🔇 Additional comments (8)
Dockerfile (1)
24-24: Good security hardening with--ignore-scripts.Disabling lifecycle scripts during install mitigates supply-chain attacks from malicious postinstall scripts.
Verify the build completes successfully—some packages (e.g.,
sharp,esbuild, native bindings) rely on postinstall scripts to download platform-specific binaries. If the build fails with missing binaries, you may need to selectively allow scripts or use pre-built alternatives.vercel.json (1)
1-7: LGTM!Consolidating security headers in
next.config.tsensures consistent behavior across deployment environments (Vercel and self-hosted via Docker). The migration is complete—X-XSS-Protectionis correctly omitted as it's deprecated..github/workflows/ci.yml (1)
67-68: LGTM!Removing
continue-on-errorensures the pipeline fails on high/critical vulnerabilities, enforcing a stricter security posture. The--audit-level=highthreshold is a reasonable balance—consider periodic reviews of moderate vulnerabilities to prevent accumulation.app/layout.tsx (2)
61-61: LGTM!The nonce is correctly propagated to
ThemeProvider, which forwards it toNextThemesProviderfor nonce-based inline style injection. This enables strict CSP without'unsafe-inline'for theme styles.
5-5: CSP nonce integration correctly implemented.The async conversion and nonce retrieval follow the recommended Next.js 15 server component pattern. The
x-nonceheader is properly generated inproxy.ts(line 23) using a cryptographically secure UUID, applied to CSP directives for both scripts and styles in production, and safely retrieved in the layout with a fallback toundefined. The implementation enables inline style support for theme switching without requiringunsafe-inlinein the CSP policy.Also applies to: 49-54
components/theme/theme-provider.tsx (1)
5-9: Consider simplifying the Props interface.The
Omit<ThemeProviderProps, "nonce">followed by re-declaringnonce?: stringmay be unnecessary ifThemeProviderPropsalready definesnonceasstring | undefined. However, if you're intentionally narrowing or documenting the type, this is fine.The nonce forwarding logic is correct and integrates well with the CSP implementation.
proxy.ts (2)
35-40: Matcher configuration looks good.The matcher correctly excludes static assets and image files from middleware processing, which is the recommended pattern for CSP middleware.
7-20: Noconnect-srcdirective needed.The codebase does not contain any external API calls (no fetch, axios, or external URLs detected). The CSP correctly falls back to
default-src 'self'for theconnect-srcdirective, which is the appropriate configuration for an application that only makes same-origin requests.Likely an incorrect or invalid review comment.
Description
Type of Change
Related Issues
Fixes #
Related to #
Screenshots
Before
After
Additional Context
Deployment Notes
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.