Suppress non-actionable ZAP baseline warnings for Next.js SPA#3284
Suppress non-actionable ZAP baseline warnings for Next.js SPA#3284nios-x wants to merge 21 commits intoOWASP:mainfrom
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR addresses ZAP security scan failures by suppressing non-actionable warnings in the baseline configuration, adds security headers to Next.js configuration, and implements CSRF cookie security settings for the Django backend with SPA compatibility. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
nios-x
changed the title
nios-x
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.zapconfig:
- Around line 19-20: The .zapconfig suppression for Spectre protections must be
removed and replaced by setting proper COOP/COEP headers in Next.js: delete the
IGNORE rule and add a headers() export in next.config.ts that sets
"Cross-Origin-Opener-Policy" (e.g., "same-origin" or "same-origin-allow-popups")
and "Cross-Origin-Embedder-Policy" (e.g., "require-corp"); ensure any externally
loaded resources (GitHub avatars, owasp.org, OpenStreetMap tiles) are served
with compatible Cross-Origin-Resource-Policy/Cross-Origin-Resource headers (or
proxied) so COEP does not break them; update next.config.ts's headers() function
accordingly and remove the global zapconfig suppression entry.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.zapconfig
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-18T20:00:23.899Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1634
File: frontend/src/app/api/auth/[...nextauth]/route.ts:30-55
Timestamp: 2025-06-18T20:00:23.899Z
Learning: The OWASP Nest application has logging disabled, so avoid suggesting console.log, console.error, or any other logging statements in code review suggestions.
Applied to files:
.zapconfig
📚 Learning: 2025-07-28T14:51:14.736Z
Learnt from: adithya-naik
Repo: OWASP/Nest PR: 1894
File: frontend/src/components/TopContributorsList.tsx:74-74
Timestamp: 2025-07-28T14:51:14.736Z
Learning: In the OWASP/Nest project, the maintainer adithya-naik prefers not to create separate components for code that's only used in two specific cases, following the YAGNI principle to avoid over-engineering when the duplication is limited and manageable.
Applied to files:
.zapconfig
🔇 Additional comments (7)
.zapconfig (7)
22-23: LGTM - Browser-controlled header.The Sec-Fetch-Dest header is part of the browser's Fetch Metadata Request Headers and is controlled by the browser, not the server. This suppression is appropriate.
25-26: LGTM - Expected Next.js behavior.Base64-encoded data in Next.js chunk files is expected for hydration data and inlined assets. The suppression is appropriately scoped to specific chunk files.
28-29: LGTM - Informational alert.The Non-Storable Content alert is informational about caching behavior and not a security vulnerability. This suppression is appropriate.
31-32: LGTM - Framework-managed caching.For a Next.js SPA with framework-managed caching, this informational alert about Cache-Control directives is appropriate to suppress.
41-43: LGTM - Informational detection with appropriate scope.The suppression is appropriately scoped to the /csrf/ endpoint where session management detection is expected. The informational alert doesn't indicate insecure behavior.
37-39: CSP is properly configured with fallback directive; however, address 'unsafe-inline' usage.The suppression comment is accurate—CSP is configured at the proxy level (proxy/headers.conf) with
default-src 'self'as a fallback, satisfying rule 10055's requirement. However, the CSP uses'unsafe-inline'for bothscript-srcandstyle-srcwithout nonces, which is a separate security concern that should be addressed. Consider replacing'unsafe-inline'with nonces or refactoring inline styles/scripts to external resources.
16-17: Clarify the SRI support limitation for Next.js chunks.Next.js has experimental SRI support available via
experimental.sriconfiguration in next.config.js (available for App Router + webpack). The current comment stating that "Next.js internal chunks do not support SRI" is inaccurate. The suppression should instead explain the actual reason for ignoring this issue:
- If SRI is not configured in the project, state that explicitly.
- If the project uses Pages Router or Turbopack (where SRI is unavailable), specify that constraint.
- If the issue persists despite SRI configuration due to certain Next.js-generated runtime chunks not receiving integrity attributes, document that specific limitation.
Update the comment to reflect the actual constraint rather than a blanket claim about SRI support.
Likely an incorrect or invalid review comment.
|
@arkid15r I added all the non vulnerable warnings in this . You can see the file change |
arkid15r
left a comment
arkid15r
left a comment
There was a problem hiding this comment.
The issue is about addressing all type of problems, not just suppressing them -- #3276 (comment)
|
Okay @arkid15r, Soon i will fix it i just need some time. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @backend/settings/base.py:
- Around line 34-39: Update CSRF_COOKIE_SAMESITE to be environment-aware: detect
whether running in local development (e.g., DJANGO_ENV, DEBUG, or a boolean like
IS_LOCAL) and set CSRF_COOKIE_SAMESITE = "Lax" for local/dev, otherwise set
CSRF_COOKIE_SAMESITE = "Strict" for production/staging; modify the assignment of
CSRF_COOKIE_SAMESITE in settings (reference symbol CSRF_COOKIE_SAMESITE in
backend/settings/base.py) to read the env flag and choose "Lax" or "Strict"
accordingly and ensure any necessary env variables are documented or defaulted
for dev.
In @frontend/next.config.ts:
- Around line 8-39: The CSP currently set in headers() includes "script-src
'unsafe-inline'", which disables XSS protection; remove 'unsafe-inline' and
implement per-request nonces instead by moving CSP generation into a middleware
(create middleware.ts with a middleware function that generates a base64 nonce,
builds the Content-Security-Policy header using "script-src 'self'
'nonce-<nonce>' 'strict-dynamic'" and sets it on the response along with a
custom x-nonce header), then consume that x-nonce in your app/layout.tsx to set
the nonce attribute on your <head> and any server-rendered inline scripts so
inline scripts are allowed only with the matching nonce.
- Around line 14-24: The Cross-Origin-Embedder-Policy header is set to
'require-corp' which will block OpenStreetMap tiles used by the ChapterMap
component (see frontend/src/components/ChapterMap.tsx:178); change the COEP
header in next.config.ts (the Cross-Origin-Embedder-Policy entry) to a less
restrictive value (e.g., 'credentialless') or remove it, or alternatively switch
to a tile provider that serves CORP headers; after changing, verify any other
external image domains referenced in remotePatterns (GitHub avatars, owasp.org,
etc.) support the chosen policy.
🧹 Nitpick comments (1)
frontend/next.config.ts (1)
27-34: Consider restricting overly permissive CSP directives.The CSP allows any HTTPS origin for images (line 31) and connections (line 33), which provides minimal security benefit. Consider restricting these to specific trusted domains:
img-src: Limit to known image CDNs and the domains in yourremotePatterns(avatars.githubusercontent.com, owasp.org, etc.)connect-src: Limit to your backend API domain and any known third-party APIsExample of more restrictive CSP directives
- "img-src 'self' data: https:", + "img-src 'self' data: https://avatars.githubusercontent.com https://owasp.org https://raw.githubusercontent.com https://*.tile.openstreetmap.org", - "connect-src 'self' https:", + "connect-src 'self' https://your-api-domain.com",
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/settings/base.pyfrontend/next.config.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/settings/base.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/settings/base.py
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/settings/production.py (1)
58-58: Remove redundantCSRF_COOKIE_SAMESITEsetting.
backend/settings/base.pyalready definesCSRF_COOKIE_SAMESITE = "Strict"at line 36, so this override in production.py is redundant. Since the production configuration inherits from the base class, this line can be safely removed.backend/settings/staging.py (1)
55-55: Remove redundant CSRF_COOKIE_SAMESITE setting.
backend/settings/base.py(line 36) already definesCSRF_COOKIE_SAMESITE = "Strict". This override in staging.py is redundant since the staging configuration inherits the same value. Consider removing this line.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/settings/local.pybackend/settings/production.pybackend/settings/staging.pycspell/custom-dict.txt
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-31T05:17:39.659Z
Learnt from: kart-u
Repo: OWASP/Nest PR: 3101
File: backend/apps/common/extensions.py:92-98
Timestamp: 2025-12-31T05:17:39.659Z
Learning: In this codebase, import OperationType for GraphQL operations from the graphql-core package rather than from strawberry. Use 'from graphql import OperationType'. Strawberry re-exports via graphql-core internally, so relying on strawberry's API may be brittle. Apply this rule to all Python files that deal with GraphQL operation types; ensure imports come from graphql (graphql-core) and not from strawberry packages. This improves compatibility and avoids coupling to strawberry's internals.
Applied to files:
backend/settings/staging.pybackend/settings/production.pybackend/settings/local.py
📚 Learning: 2026-01-01T17:48:23.963Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2948
File: backend/apps/owasp/management/commands/owasp_generate_community_snapshot_video.py:41-47
Timestamp: 2026-01-01T17:48:23.963Z
Learning: In Django code, be aware that a QuerySet's boolean evaluation (e.g., if not queryset) runs a database query to determine emptiness. While it is technically valid to use the queryset in a boolean context, use queryset.exists() for existence checks to avoid unnecessary queries and improve performance. Applicable broadly to Python/Django files rather than just this specific path.
Applied to files:
backend/settings/staging.pybackend/settings/production.pybackend/settings/local.py
📚 Learning: 2025-08-31T13:48:09.830Z
Learnt from: rudransh-shrivastava
Repo: OWASP/Nest PR: 2155
File: frontend/graphql-codegen.ts:0-0
Timestamp: 2025-08-31T13:48:09.830Z
Learning: In the OWASP/Nest project, Django's GraphQL endpoint accepts CSRF tokens via 'x-csrftoken' header (lowercase) without requiring a Referer header, working fine in their configuration for GraphQL codegen introspection.
Applied to files:
cspell/custom-dict.txt
🔇 Additional comments (2)
cspell/custom-dict.txt (1)
54-54: LGTM!The addition of "credentialless" aligns with the COOP/COEP/CORP security header implementations mentioned in the PR objectives for addressing ZAP alert 90004.
backend/settings/local.py (1)
27-27: LGTM!Setting
CSRF_COOKIE_SAMESITE = "Lax"for local development is appropriate. It provides a more permissive policy than production's "Strict" setting, allowing cookies on top-level navigation which facilitates local testing scenarios while still maintaining CSRF protection.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/next.config.ts:
- Around line 8-28: The headers() route matcher uses an invalid pattern
('/(.*)') so update the source in the headers() return to the Next.js
path-to-regexp form ('/:path*') to ensure headers apply; also avoid applying
Cross-Origin-Opener-Policy (Cross-Origin-Opener-Policy: same-origin),
Cross-Origin-Embedder-Policy (Cross-Origin-Embedder-Policy: credentialless) and
Cross-Origin-Resource-Policy (Cross-Origin-Resource-Policy: same-origin)
globally—limit them to specific routes (e.g., isolated pages like a
sandbox/playground) or remove them where you rely on OAuth/popups, credentialed
cross-origin resources, third-party embeds or analytics; verify changes by
testing headers() behavior and these three header keys across Chromium, Firefox
and Safari and validate popup-based auth flows and third-party resources still
work.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/next.config.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-04T05:23:43.562Z
Learnt from: M-ayank2005
Repo: OWASP/Nest PR: 1282
File: frontend/src/pages/About.tsx:94-98
Timestamp: 2025-04-04T05:23:43.562Z
Learning: In the OWASP/Nest frontend project, DOMPurify is used to sanitize HTML content before rendering it with dangerouslySetInnerHTML, following security best practices to prevent XSS vulnerabilities.
Applied to files:
frontend/next.config.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @frontend/next.config.ts:
- Around line 8-28: The global headers() function currently sets strict
Cross-Origin Isolation headers (Cross-Origin-Opener-Policy: same-origin,
Cross-Origin-Embedder-Policy: credentialless, Cross-Origin-Resource-Policy:
same-origin) for source '/:path*', which may break third-party resources,
NextAuth, and Sentry; update this by narrowing the header scope (exclude
API/auth routes and any known external resource paths), or make COEP conditional
(use 'credentialless' only when target browsers support it or omit for legacy
Firefox), and verify remotePatterns, withSentryConfig telemetry, and NextAuth
flows via functional tests to ensure images and reporting still work; adjust the
headers() implementation accordingly to whitelist or skip paths that require
cross-origin access.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/next.config.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-04T05:23:43.562Z
Learnt from: M-ayank2005
Repo: OWASP/Nest PR: 1282
File: frontend/src/pages/About.tsx:94-98
Timestamp: 2025-04-04T05:23:43.562Z
Learning: In the OWASP/Nest frontend project, DOMPurify is used to sanitize HTML content before rendering it with dangerouslySetInnerHTML, following security best practices to prevent XSS vulnerabilities.
Applied to files:
frontend/next.config.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @frontend/src/proxy.ts:
- Around line 17-24: The CSP currently builds cspHeader with `script-src 'self'
'strict-dynamic' 'nonce-${nonce}' https:` which undermines nonce-based
protection; remove the `https:` token from the `script-src` entry in the
cspHeader so that only nonce-based scripts (and permitted dynamic loads) are
allowed; if you must support older browsers, replace the removed https: fallback
with `'unsafe-inline'` as a deliberate backwards-compatibility option, and
ensure the `nonce` variable used in the `script-src` string (and anywhere nonces
are applied) is generated per-request and injected into both the header and
script tags.
🧹 Nitpick comments (1)
frontend/src/proxy.ts (1)
45-48: Consider performance impact of global middleware execution.The matcher now applies to all paths (
'/:path*') instead of just mentorship routes, meaning this middleware executes on every request (except excluded paths). This includes:
- Nonce generation (
crypto.randomBytes) on every request- CSP header construction string concatenation
- JWT token checking even for public pages
For high-traffic applications, consider:
- Moving CSP to the Next.js
headers()config if the nonce isn't needed for all pages- Or making the nonce generation conditional based on page requirements
- Benchmarking the performance impact
Based on learnings, the frontend uses Next.js 15+ which has improved middleware performance, but global middleware should still be evaluated for production traffic patterns.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/next.config.tsfrontend/src/proxy.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-04-04T05:23:43.562Z
Learnt from: M-ayank2005
Repo: OWASP/Nest PR: 1282
File: frontend/src/pages/About.tsx:94-98
Timestamp: 2025-04-04T05:23:43.562Z
Learning: In the OWASP/Nest frontend project, DOMPurify is used to sanitize HTML content before rendering it with dangerouslySetInnerHTML, following security best practices to prevent XSS vulnerabilities.
Applied to files:
frontend/next.config.ts
📚 Learning: 2025-08-10T11:08:47.258Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/src/app/api/auth/[...nextauth]/route.ts:13-25
Timestamp: 2025-08-10T11:08:47.258Z
Learning: In the OWASP Nest codebase (frontend/src/app/api/auth/[...nextauth]/route.ts), input validation and string trimming for authentication-related queries like `isProjectLeader` and `isMentor` are handled in the backend rather than the frontend. The backend is responsible for sanitizing and validating input parameters.
Applied to files:
frontend/next.config.ts
🔇 Additional comments (4)
frontend/src/proxy.ts (3)
6-11: LGTM: Excluded paths logic is correct.The early exclusion of API and auth callback routes prevents middleware from interfering with API responses and authentication flows, which is appropriate.
13-14: LGTM: Nonce generation is secure.Using
crypto.randomBytes(16).toString('base64')for CSP nonce generation follows security best practices and provides sufficient entropy.
32-40: LGTM: JWT authentication logic is correct.The token validation and redirect flow for protected mentorship routes is properly implemented.
frontend/next.config.ts (1)
8-28: Correct browser support matrix and test credential-less behavior with external resources.COEP "credentialless" is supported in Next.js 15, but the browser support claim needs correction:
- Chrome 96+: ✓ Supported
- Firefox 119+ (not 109+): Firefox earlier than v119 does not support COEP credentialless
- Safari: Not supported in 2024; Safari users won't benefit from cross-origin isolation via this approach
Note that "credentialless" omits cookies from cross-origin requests, which may affect loading of external images (GitHub avatars, OWASP.org resources) if those endpoints require authentication. Test external resources under this policy to confirm they load correctly.
Additionally, verify that the combination of COOP: same-origin, COEP: credentialless, and CORP: same-origin successfully enables
crossOriginIsolatedstate in supported browsers and that the Vercel platform doesn't override these headers (platform-level injection can interfere with next.config.js settings).These headers do address ZAP alert 90004 for Spectre mitigation, but coverage will be limited to Chrome 96+ and Firefox 119+.
|
Now You can merge it |
|
@arkid15r All remaining ZAP WARN findings are non-actionable and fall into one of the following categories: |
|
I tried a lot but when i fixed error 90004 as per coderabbit it breaks whole frontend each time so i have left it unchanged |
|
@arkid15r |
|
2 participants
Proposed change
Resolves #3276
This PR updates the ZAP baseline configuration to suppress non-actionable warnings
caused by Next.js framework internals, browser-controlled headers, and purely
informational findings.
The suppressed alerts do not represent actionable security risks for a modern
Next.js SPA. Real security issues are intentionally not suppressed and remain
enforced by the baseline scan to ensure meaningful CI security checks.
Checklist
make check-testlocally and all tests passedDetails
ZAP WARN Summary (Non-Actionable)
10015 | Cache headers on public, non-sensitive endpoints | Ignored (informational)
10019 | Missing Content-Type on 308 redirects | Ignored (framework behavior)
10049 | Non-storable dynamic/SSR responses | Ignored (Next.js behavior)
10055 | CSP fallback directive warning | Ignored (informational)
10094 | Base64 in static assets (CSS/fonts) | Ignored (false positive)
10112 | Session cookie detection | Ignored (informational)
90003 | Missing SRI on dynamic chunks | Ignored (SPA limitation)
90004 | Spectre mitigation | Fixed via COOP/COEP headers
90005 | Sec-Fetch headers (browser-controlled) | Ignored (non-actionable)