fix(security): add baseline HTTP security headers via Netlify configu… #4769
Conversation
KariraLakshya
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve,
derberg,
princerajpoot20 and
sambhavgupta0705
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughReplaced two response headers with multiple security-focused headers in Netlify config and added a Netlify Function that accepts POSTed CSP violation reports, attempts to parse and log them, and always returns HTTP 204 No Content. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Browser
participant CDN as "Site / CDN"
participant CSPFn as "Netlify Function\n/netlify/functions/csp-report"
participant Logs as "Server Console / Logs"
Browser->>CDN: Request page (receives security headers)
Note right of CDN: CSP-Report-Only header present
Browser->>CSPFn: POST CSP violation (JSON)
activate CSPFn
CSPFn->>CSPFn: try parse event.body
alt parse success
CSPFn->>Logs: log timestamp + parsed report
else parse error
CSPFn->>Logs: log timestamp + parse error
end
CSPFn-->>Browser: HTTP 204 No Content
deactivate CSPFn
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4769 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 798 798
Branches 146 146
=========================================
Hits 798 798 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 (2)
netlify.toml (2)
7-7: Remove trailing semicolon from HSTS header.The trailing semicolon after
includeSubDomains;is unnecessary and non-standard, though it shouldn't cause issues with most browsers.🔎 Suggested fix
- Strict-Transport-Security = "max-age=63072000; includeSubDomains;" + Strict-Transport-Security = "max-age=63072000; includeSubDomains"
4-8: Consider adding X-Frame-Options as a fallback for older browsers.While
frame-ancestors 'none'in your CSP provides clickjacking protection, older browsers that don't support CSP Level 2 (including some legacy enterprise browsers) won't enforce it. AddingX-Frame-Options = "DENY"alongside the CSP directive ensures broader compatibility.🔎 Suggested addition
X-Content-Type-Options = "nosniff" + X-Frame-Options = "DENY" Referrer-Policy = "strict-origin-when-cross-origin"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
netlify.toml
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - windows-latest
- GitHub Check: Lighthouse CI
🔇 Additional comments (2)
netlify.toml (2)
4-6: LGTM! Strong baseline security headers.These three headers are correctly configured:
X-Content-Type-Optionsprevents MIME-sniffing attacksReferrer-Policyprovides good privacy/security balancePermissions-Policyblocks risky features by default
1-9: No changes needed; the removal ofAccess-Control-Allow-Originis appropriate.The AsyncAPI website repository contains no CORS-related code, cross-origin API calls, or references to
Access-Controlheaders. The architecture uses static site generation (SSG) with Netlify serverless functions for backend operations, which does not require CORS headers for client-side cross-origin requests. The header removal is intentional security hardening.Likely an incorrect or invalid review comment.
netlify.toml
Outdated
| Referrer-Policy = "strict-origin-when-cross-origin" | ||
| Permissions-Policy = "camera=(), microphone=(), geolocation=()" | ||
| Strict-Transport-Security = "max-age=63072000; includeSubDomains;" | ||
| Content-Security-Policy-Report-Only = "default-src 'self'; frame-ancestors 'none'; script-src 'self' https: 'unsafe-inline' 'unsafe-eval'; style-src 'self' https: 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' https: data:; connect-src 'self' https:;" |
There was a problem hiding this comment.
CSP baseline is too permissive to be effective, even in report-only mode.
The current Content-Security-Policy-Report-Only contains several directives that significantly weaken protection against XSS and other attacks:
'unsafe-inline'inscript-srcandstyle-srcallows inline scripts/styles (primary XSS vector)'unsafe-eval'inscript-srcpermitseval()and similar constructs (another XSS vector)- Broad
https:sources allow loading from any HTTPS origin
While report-only mode is appropriate for initial deployment, these directives are so permissive they won't generate meaningful violation reports to help you harden the policy later. Consider tightening the baseline to at least remove 'unsafe-eval' and restrict sources to specific trusted domains, then use the violation reports to iteratively refine.
Additionally, missing commonly recommended directives:
object-src 'none'- blocks pluginsbase-uri 'self'- prevents base tag injectionform-action 'self'- restricts form submissions
🔎 Suggested improved baseline (still permissive but more useful for monitoring)
- Content-Security-Policy-Report-Only = "default-src 'self'; frame-ancestors 'none'; script-src 'self' https: 'unsafe-inline' 'unsafe-eval'; style-src 'self' https: 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' https: data:; connect-src 'self' https:;"
+ Content-Security-Policy-Report-Only = "default-src 'self'; script-src 'self' 'unsafe-inline' https://trusted-cdn.example.com; style-src 'self' 'unsafe-inline' https://trusted-cdn.example.com; img-src 'self' data: https:; font-src 'self' data: https:; connect-src 'self' https:; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none';"Replace https://trusted-cdn.example.com with your actual CDN/third-party script domains. This removes 'unsafe-eval' and tightens sources while keeping 'unsafe-inline' for compatibility during the monitoring phase.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Content-Security-Policy-Report-Only = "default-src 'self'; frame-ancestors 'none'; script-src 'self' https: 'unsafe-inline' 'unsafe-eval'; style-src 'self' https: 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' https: data:; connect-src 'self' https:;" | |
| Content-Security-Policy-Report-Only = "default-src 'self'; script-src 'self' 'unsafe-inline' https://trusted-cdn.example.com; style-src 'self' 'unsafe-inline' https://trusted-cdn.example.com; img-src 'self' data: https:; font-src 'self' data: https:; connect-src 'self' https:; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none';" |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
netlify.toml
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (2)
netlify.toml (2)
4-7: Excellent baseline security header configuration!The security headers are well-configured:
X-Content-Type-Optionsprevents MIME type sniffing attacksReferrer-Policyprovides a good balance between privacy and functionalityPermissions-Policyappropriately restricts sensitive browser featuresStrict-Transport-Securityenforces HTTPS with a 2-year max-age and subdomain coverage
8-8: Good progress addressing previous feedback on CSP directives!The CSP has been significantly improved from the previous version:
- Removed
'unsafe-eval'fromscript-src(eliminates a major XSS vector)- Added
object-src 'none'(blocks plugins)- Added
base-uri 'self'(prevents base tag injection)- Added
form-action 'self'(restricts form submissions)These changes directly address the concerns raised in the previous review.
|
Thanks for the review feedback. I agree that CSP report-only mode is most effective when paired with a reporting endpoint. Happy to extend this with a reporting endpoint in a follow-up PR once maintainers confirm the preferred approach. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
netlify/functions/csp-report.ts (2)
3-7: Consider adding request validation.The handler accepts any request method and content-type. CSP reports are sent as POST requests with
application/csp-reportorapplication/jsoncontent-type. Adding validation improves security and debugging.🔎 Add basic request validation
export const handler: Handler = async (event) => { + // Validate request method + if (event.httpMethod !== 'POST') { + return { + statusCode: 405, + body: 'Method Not Allowed' + }; + } + + // Validate content-type (CSP reports can be application/csp-report or application/json) + const contentType = event.headers['content-type'] || ''; + if (!contentType.includes('application/csp-report') && !contentType.includes('application/json')) { + return { + statusCode: 415, + body: 'Unsupported Media Type' + }; + } + // Log CSP violation reports console.log('[CSP Violation Report]', { body: event.body, timestamp: new Date().toISOString() }); return { statusCode: 204, }; };
3-3: Remove unnecessaryasynckeyword.The function doesn't perform any asynchronous operations. The
asynckeyword is unnecessary unless you plan to add async logging/storage later.🔎 Remove async if not needed
-export const handler: Handler = async () => { +export const handler: Handler = () => { return { statusCode: 204, }; };Or keep it if you expect to add async operations based on the previous suggestions.
netlify.toml (2)
8-8: CSP policy remains very permissive for eventual enforcement.While report-only mode is appropriate for initial deployment, the current policy is extremely permissive:
https:in multiple directives allows loading from any HTTPS origin'unsafe-inline'inscript-srcandstyle-srcpermits inline scripts/styles (primary XSS vector)These directives are so broad that the reports may not provide useful insights for tightening the policy later. Once you've collected initial reports, consider progressively restricting:
- Replace
https:with specific trusted domains (CDNs, analytics, etc.)- Remove
'unsafe-inline'in favor of nonces or hashes for inline scripts/styles🔎 Example of moderately tightened policy for the next iteration
- Content-Security-Policy-Report-Only = "default-src 'self'; script-src 'self' https: 'unsafe-inline'; style-src 'self' https: 'unsafe-inline'; img-src 'self' data: https:; font-src 'self' data: https:; connect-src 'self' https:; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'; report-uri /.netlify/functions/csp-report;" + Content-Security-Policy-Report-Only = "default-src 'self'; script-src 'self' 'unsafe-inline' https://cdn.example.com https://analytics.example.com; style-src 'self' 'unsafe-inline' https://cdn.example.com; img-src 'self' data: https:; font-src 'self' data: https:; connect-src 'self' https://api.example.com; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'; report-uri /.netlify/functions/csp-report;"Replace placeholder domains with actual third-party services used by the site. This keeps
'unsafe-inline'during monitoring but restricts source origins to generate more actionable violation reports.
7-7: Trailing semicolon in HSTS header is unusual but should work.The trailing semicolon after
includeSubDomains;is syntactically valid but non-standard (typically omitted). Most implementations will handle it correctly, but you might consider removing it for consistency with common formatting.🔎 Remove trailing semicolon
- Strict-Transport-Security = "max-age=63072000; includeSubDomains;" + Strict-Transport-Security = "max-age=63072000; includeSubDomains"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
netlify.tomlnetlify/functions/csp-report.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (2)
netlify.toml (2)
4-8: Security headers look good overall.The baseline security headers are well-configured:
- ✓
X-Content-Type-Options: nosniffprevents MIME sniffing- ✓
Referrer-Policy: strict-origin-when-cross-originbalances privacy and functionality- ✓
Permissions-Policyrestricts powerful features- ✓
Strict-Transport-Securityenforces HTTPS with 2-year max-age and subdomain coverage- ✓
frame-ancestors 'none'in CSP prevents clickjacking- ✓
report-urinow present (addresses previous critical issue)
8-8: Verify the CSP reporting endpoint functions correctly after deployment.Ensure that:
- The Netlify function deploys successfully at
/.netlify/functions/csp-report- Browsers can reach the endpoint (check for CORS or authentication issues)
- CSP violation reports are being generated and logged (once you implement logging per the suggestion on
csp-report.ts)Test by visiting the site and checking browser DevTools Console for CSP violations, then verifying the function logs in Netlify dashboard.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
netlify/functions/csp-report.ts (1)
5-6: Consider adding Content-Type validation and size limits.For improved reliability and to prevent potential abuse:
- Validate the Content-Type header (CSP reports typically use
application/csp-reportorapplication/json)- Add a size limit check to prevent processing excessively large payloads
🔎 Suggested defensive checks
+ const contentType = event.headers['content-type'] || ''; + if (!contentType.includes('application/csp-report') && !contentType.includes('application/json')) { + return { + statusCode: 415, + body: 'Unsupported Media Type', + }; + } + try { if (event.body) { + // Reject overly large payloads (e.g., > 10KB) + if (event.body.length > 10240) { + console.warn('[CSP Report] Rejected oversized payload'); + return { statusCode: 413 }; + } + const report = JSON.parse(event.body);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
netlify/functions/csp-report.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (1)
netlify/functions/csp-report.ts (1)
3-20: Good implementation of CSP report logging.The function now properly logs CSP violation reports with timestamps and handles parsing errors gracefully. This addresses the previous review feedback and provides the necessary observability for refining the CSP policy.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
netlify/functions/csp-report.ts (3)
5-10: Fix indentation inconsistency.Lines 5 and 8-10 have inconsistent indentation that doesn't match the rest of the file. This should be caught and fixed by your linter/formatter.
🔎 Proposed fix for indentation
// CSP reports are sent via POST - if (event.httpMethod !== 'POST') { + if (event.httpMethod !== 'POST') { return { statusCode: 405, headers: { - Allow: 'POST', - }, + Allow: 'POST', + }, body: 'Method Not Allowed', }; }
5-13: Consider validating Content-Type header.Browsers typically send CSP reports with
Content-Type: application/csp-reportorapplication/json. Validating the Content-Type would prevent processing of unexpected payloads and improve security posture.🔎 Proposed fix to add Content-Type validation
// CSP reports are sent via POST if (event.httpMethod !== 'POST') { return { statusCode: 405, headers: { Allow: 'POST', }, body: 'Method Not Allowed', }; } + + const contentType = event.headers['content-type'] || ''; + if (!contentType.includes('application/csp-report') && !contentType.includes('application/json')) { + return { + statusCode: 415, + body: 'Unsupported Media Type', + }; + }
14-25: Excellent implementation of CSP report logging.The code successfully addresses the previous review feedback by logging CSP violations and handling parse errors. Using
console.warnfor violations andconsole.errorfor parse errors provides good log-level differentiation.💡 Optional: Log when body is missing
For easier debugging, consider logging when
event.bodyis missing:try { if (event.body) { const report = JSON.parse(event.body); console.warn('[CSP Violation Report]', { report, timestamp: new Date().toISOString(), }); + } else { + console.warn('[CSP Report] Received POST with empty body'); } } catch (error) { console.error('[CSP Report Parse Error]', error); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
netlify/functions/csp-report.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (2)
netlify/functions/csp-report.ts (2)
1-2: LGTM!The import is correct and uses the appropriate type from
@netlify/functions.
27-29: LGTM!Returning 204 No Content is the correct response for CSP report endpoints.
|
The remaining CI failure appears to be related to the Lighthouse Best Practices Further improving the Lighthouse score would likely require one of the following:
Please let me know which approach you’d prefer:
Happy to proceed based on your guidance. |
|
@anshgoyalevil Should I add isolation headers (COOP/COEP), to fix the remaining CI failure?? |
1 participant
Description
This PR adds a baseline set of HTTP security headers to improve the security posture of the AsyncAPI website.
Changes
netlify.tomlContent-Security-Policyin report-only mode to safely observe violations without breaking the siteNotes
preloadis intentionally omitted from HSTS and can be considered in a follow-up PR.Fixes #4724
Summary by CodeRabbit
Chores
New Features
✏️ Tip: You can customize this high-level summary in your review settings.