feat(security): harden CSP and security headers in Next.js web

[aayushprsingh]

Summary

Hardens the Content Security Policy (CSP) and security headers configuration for the UltimateHealth Next.js web app (web/), addressing the issues identified in #1047.

Changes

web/src/middleware.ts (new)

Implements a robust security middleware with:

• CSP with Nonces: Production uses 'nonce-{uuid}' + 'strict-dynamic' for script-src; development allows 'unsafe-eval'/'unsafe-inline' for HMR without breaking DX.
• HSTS: max-age=63072000; includeSubDomains; preload (2-year, preload-list eligible).
• X-Frame-Options: DENY — prevents clickjacking.
• X-Content-Type-Options: nosniff — prevents MIME sniffing.
• Referrer-Policy: strict-origin-when-cross-origin.
• Permissions-Policy: Restricts camera, microphone, geolocation, autoplay, payment, USB, gyroscope, magnetometer.
• Cross-Origin-Opener-Policy and Cross-Origin-Resource-Policy: same-origin for cross-origin isolation.
• CSP report-uri: Violations reported to /api/csp-report.

web/src/app/api/csp-report/route.ts (new)

A lightweight API endpoint that:

• Accepts application/csp-report POST requests.
• Parses and logs violations to the server console for monitoring.
• Returns 204 No Content to avoid response feedback loops.

web/src/app/layout.tsx (modified)

• Imports headers() from next/headers to read the x-nonce header injected by middleware.
• Makes RootLayout async to await the headers.
• Exports dynamic = 'force-dynamic' to ensure a unique nonce is generated per request (prevents nonce reuse from cached responses).

Testing

1. Run npm run dev inside web/ — no console errors, app loads normally.
2. In production build, check browser DevTools → Network → response headers for Content-Security-Policy, Strict-Transport-Security, X-Frame-Options, etc.
3. Intentionally load a cross-origin script to verify CSP blocks it and /api/csp-report receives the violation.

Fixes #1047

[github-actions]

Thank you @, for creating the PR and contributing to our UltimateHealth project 💗.
Our team will review the PR and will reach out to you soon! 😇
Make sure that you have marked all the tasks that you are done with ✅.
Thank you for your patience! 😀

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a security-focused Next.js middleware that sets CSP (with per-request nonces) and other hardening headers, plus a CSP violation reporting endpoint and layout changes to consume the nonce.

Changes:

• Introduces middleware.ts to generate CSP nonces and attach security headers.
• Updates RootLayout to read a nonce from headers and forces dynamic rendering.
• Adds /api/csp-report route to receive CSP violation reports.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.

File	Description
web/src/middleware.ts	Generates CSP nonce and sets CSP + security headers for matched routes.
web/src/app/layout.tsx	Reads nonce from headers and forces dynamic rendering to avoid cached nonces.
web/src/app/api/csp-report/route.ts	Adds endpoint to receive/log CSP violation reports.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[SB2318]

/review

[github-actions]

🤖 Gemini AI Code Review

Summary

This Pull Request introduces a comprehensive set of security enhancements to the UltimateHealth Next.js application, primarily focusing on Content Security Policy (CSP) and various HTTP security headers. The changes include a new middleware for header injection, a dedicated API endpoint for CSP violation reporting, and modifications to the root layout to support CSP nonces.

The PR demonstrates a strong understanding of modern web security best practices, particularly with the implementation of strict-dynamic CSP and a wide array of protective headers. However, there are critical issues regarding the application of the CSP nonce and some areas for further hardening and robustness, especially in the CSP report endpoint.

🔴 High Severity

• Issue: CSP Nonce Not Applied to HTML Element
  The middleware.ts correctly generates a unique nonce and sets it in the x-nonce header. The RootLayout in layout.tsx correctly reads this x-nonce header. However, the nonce is not actually passed to the <html> element (or any <script> tags) in layout.tsx. For Next.js to automatically apply the nonce to its own inline scripts (e.g., for hydration, client-side routing), the nonce attribute must be present on the <html> tag. Without this, the script-src 'nonce-{uuid}' 'strict-dynamic' directive will block all Next.js-generated inline scripts, effectively breaking the application in production.

  • Impact: The application will fail to load or function correctly in a production environment with the hardened CSP, as essential Next.js client-side scripts will be blocked. This completely negates the intended security benefit and introduces a critical functional bug.
  • Fix: Modify web/src/app/layout.tsx to pass the nonce to the <html> element.

  --- a/web/src/app/layout.tsx
  +++ b/web/src/app/layout.tsx
  @@ -36,6 +36,7 @@ export default async function RootLayout({
   <html
     lang="en"
     className={cn(inter.variable, interHeading.variable)}
  +  {...(nonce ? { nonce } : {})} // Pass the nonce attribute here
   >
     <body>
       <TooltipProvider>{children}</TooltipProvider>

• Issue: CSP Report Endpoint Returns 200 OK with Body Instead of 204 No Content
  The PR description explicitly states that the /api/csp-report endpoint "Returns 204 No Content to avoid response feedback loops." However, the implementation in web/src/app/api/csp-report/route.ts returns NextResponse.json({ status: "ok" }, { status: 200 });.

  • Impact: While not a functional blocker, returning a 200 OK with a body for a report endpoint is generally discouraged. 204 No Content is the standard for such endpoints as it signals successful processing without needing to send any data back, reducing network overhead and potential for unintended client-side processing.
  • Fix: Change the successful response to 204 No Content.

  --- a/web/src/app/api/csp-report/route.ts
  +++ b/web/src/app/api/csp-report/route.ts
  @@ -13,7 +13,7 @@
   // In production, consider using a service like Sentry, Datadog, or a custom logger
   console.warn("CSP Violation Reported:", report);
   
  -    return NextResponse.json({ status: "ok" }, { status: 200 });
  +    return new NextResponse(null, { status: 204 });
   } catch (error) {
     console.error("Error processing CSP report:", error);
     return NextResponse.json({ error: "Failed to process report" }, { status: 400 });

• Issue: Insufficient Error Handling and Logging in CSP Report Endpoint
  The try/catch block in web/src/app/api/csp-report/route.ts is too broad. If request.json() fails (e.g., due to a non-JSON body), the error message is generic. More importantly, console.error is not sufficient for production monitoring.

  • Impact: Malformed or malicious CSP reports could lead to unhandled errors or obscure logging, making it difficult to diagnose issues or detect attack attempts. In a production environment, errors should be sent to a dedicated monitoring service (e.g., Sentry, Datadog, or a custom logger) to ensure visibility and alerting.
  • Fix: Implement more granular error handling for JSON parsing and ensure production-grade error reporting.

  --- a/web/src/app/api/csp-report/route.ts
  +++ b/web/src/app/api/csp-report/route.ts
  @@ -8,15 +8,24 @@
    */
   export async function POST(request: Request) {
     try {
  -    const report = await request.json();
  +    const rawBody = await request.text(); // Read as text first for robust error handling
  +    let report;
  +    try {
  +      report = JSON.parse(rawBody);
  +    } catch (jsonError) {
  +      console.error("CSP Report: Error parsing JSON body:", jsonError, "Raw body:", rawBody);
  +      // In production, send to error monitoring service
  +      return NextResponse.json({ error: "Invalid JSON format" }, { status: 400 });
  +    }
   
   // Log the violation report
   // In production, consider using a service like Sentry, Datadog, or a custom logger
  -    console.warn("CSP Violation Reported:", report);
  +    // Ensure sensitive data is redacted before logging to external services
  +    console.warn("CSP Violation Reported:", report); 
   
  -    return NextResponse.json({ status: "ok" }, { status: 200 });
  +    return new NextResponse(null, { status: 204 });
     } catch (error) {

• console.error("Error processing CSP report:", error);

• return NextResponse.json({ error: "Failed to process report" }, { status: 400 });

• console.error("CSP Report: Unexpected error processing request:", error);
• // In production, send to error monitoring service
• return NextResponse.json({ error: "Internal server error" }, { status: 500 }); // Use 500 for unexpected server errors
  }
  }
  ```

• Issue: Performance Impact of export const dynamic = "force-dynamic"; on RootLayout
  Setting dynamic = "force-dynamic" on the RootLayout ensures that a unique nonce is generated per request and prevents caching of the layout. While necessary for the nonce to be effective, applying it to the entire RootLayout means that no part of the layout will be statically optimized or cached by Next.js.
  • Impact: This can significantly increase server load and reduce the performance benefits of Next.js's static rendering capabilities for every page load. It forces the entire layout to be rendered on the server for every request, even for parts that could otherwise be static.
  • Fix: This is a design trade-off. While force-dynamic is required for the nonce, it's important to acknowledge the performance implications. If the application has many static pages, consider if there's a more granular way to apply dynamic rendering or if the performance hit is acceptable for the security benefits. For now, it's a necessary evil, but worth noting as a potential optimization target if performance becomes an issue. No direct code fix, but a conscious decision.

🟡 Medium Severity

• Issue: CSP style-src 'unsafe-inline'
  The CSP includes 'unsafe-inline' for style-src. While often necessary for certain UI libraries or dynamic styling, it's a weaker directive that allows any inline style block or attribute.

  • Impact: An XSS vulnerability that allows an attacker to inject HTML could potentially be escalated to inject arbitrary CSS, which can be used for defacement, phishing, or even data exfiltration (e.g., by styling input fields based on user input).
  • Fix: Review if 'unsafe-inline' is strictly necessary. If possible, consider:
    • Using a CSS-in-JS library that supports nonce or hashing for inline styles (e.g., styled-components with StyleSheetManager nonce={nonce}).
    • Extracting critical CSS to external stylesheets.
    • Using hashes for specific inline styles if they are few and static.
    • If it's unavoidable, document the necessity and ensure all user-controlled inputs are rigorously sanitized to prevent style injection.

• Issue: Logging of Potentially Sensitive Data in CSP Reports
  CSP reports can sometimes contain sensitive information, such as full URLs (including query parameters) or referrer information, especially if the report-uri is triggered by a resource load. Logging the entire report object directly to console.warn (and potentially to a production logging service) might expose sensitive data.

  • Impact: If these logs are stored or accessible, they could inadvertently expose user data, internal paths, or other sensitive information.
  • Fix: When integrating with a production logging service, ensure that the CSP report data is sanitized, redacted, or truncated before storage. For example, only log specific fields like document-uri, blocked-uri, violated-directive, and original-policy, and redact query parameters from URLs.

🟢 Low Severity / Nits

• web/src/app/api/csp-report/route.ts Comment Clarity: The comment "In production, consider using a service like Sentry, Datadog, or a custom logger" is good. However, for a production-grade PR, it might be beneficial to include a placeholder for such integration, even if commented out, to guide future development.
• web/src/middleware.ts Nonce Generation Comment: The comment "Standard UUIDs are less ideal for nonces than random bytes" is technically correct but could be slightly clearer. While UUIDs are strings, crypto.randomUUID() is a modern, simpler API for generating cryptographically strong unique identifiers that are perfectly suitable for nonces. The current implementation using crypto.getRandomValues is also perfectly fine and robust. This is a minor stylistic preference.
• web/src/app/layout.tsx dynamic Comment: The comment "force-dynamic ensures a unique CSP nonce is generated per request (not cached)" is accurate. It might be helpful to add a brief note about the performance implications or link to the Next.js documentation regarding force-dynamic for context.

What's Good ✅

1. Comprehensive Security Headers: The PR implements a robust set of security headers (HSTS, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy, Cross-Origin-Opener-Policy, Cross-Origin-Resource-Policy). This demonstrates a thorough approach to hardening the application's security posture beyond just CSP.
2. Modern CSP Implementation with strict-dynamic: The use of script-src 'nonce-{nonce}' 'strict-dynamic' in production, combined with a separate development configuration, is an excellent and highly recommended modern CSP strategy. It provides strong protection against XSS while maintaining developer experience.
3. Dedicated CSP Report Endpoint: Creating a specific API endpoint (/api/csp-report) for receiving violation reports is a crucial step for monitoring and iteratively improving the CSP. This foresight is commendable.

Verdict

Request Changes

The most critical issue is the CSP Nonce Not Applied to HTML Element. Without this fix, the application will likely be non-functional in a production environment with the enabled CSP. Addressing this, along with the other high and medium severity issues, is essential before merging.

[SB2318]

Thanks for your contribution!

But I would like to suggest some fixes.

• Make sure you applied all suggestions provided by Copilot
• The CSP nonce generated in middleware is read in layout.tsx, but I don't see it being applied to the element or any script tags. With a nonce-based CSP (script-src 'nonce-...' 'strict-dynamic'), Next.js needs access to the nonce for its inline scripts. Can you confirm the nonce is actually propagated to the rendered HTML? Otherwise client-side hydration may fail in production.
• The CSP report endpoint currently relies on request.json() and broad error handling. Consider adding explicit handling for malformed payloads and returning a more specific response for invalid reports. This would make debugging and monitoring easier in production.
• CSP reports may contain URLs, referrers, or other potentially sensitive information. Before integrating these logs into monitoring systems, consider sanitizing or limiting the fields that are recorded.
• Using force-dynamic is a deliberate trade-off when implementing per-request nonces. The performance impact is worth noting, but I don't think it should block this PR unless there is evidence of a measurable regression.