Skip to content

ATLAS-5386: Harden CSP headers by removing unsafe directives (unsafe-… - #742

Open
chaitalicod wants to merge 1 commit into
apache:masterfrom
chaitalicod:ATLAS-5386
Open

ATLAS-5386: Harden CSP headers by removing unsafe directives (unsafe-…#742
chaitalicod wants to merge 1 commit into
apache:masterfrom
chaitalicod:ATLAS-5386

Conversation

@chaitalicod

Copy link
Copy Markdown
Contributor

…inline/unsafe-eval) and enabling nonce-based policy

What changes were proposed in this pull request?

(Please fill in changes proposed in this fix. Create an issue in ASF JIRA before opening a pull request and
set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. ATLAS-XXXX: Fix a typo in YYY))

How was this patch tested?

(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

@pawarprasad123 pawarprasad123 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reviewed this against the SEA-3922 requirements and React Dashboard UI changes.

Summary

This PR correctly updates the default CSP policy to remove 'unsafe-inline' / 'unsafe-eval' and introduces per-request nonce generation. That is a necessary first step.

However, this is not sufficient as a complete backend fix on its own. The PR updates the CSP header but does not include HTML nonce injection or consistent nonce reuse across all header-writing code paths. Merging this without follow-up work will likely break HTML pages (/n3/index.html, login, error, classic UI) because inline <script> / <style> tags will not carry a matching nonce.

What looks good

  • Default CSP template removes unsafe directives and adds 'nonce-${nonce}'
  • Nonce generation uses SecureRandom with 16 bytes (meets minimum length requirement)
  • Additional hardening: object-src 'none', base-uri 'self', frame-ancestors 'none'
  • atlas.csp.nonce request attribute is exposed for downstream UI consumption
  • Basic unit test verifies nonce substitution in CSP header

Blocking concerns

  1. Multiple nonces per requestgenerateCspNonce() is called independently in AtlasSecurityConfig, AtlasHeaderFilter, and via setSecurityHeaders(wrapper) in several other filters/servlets. This can cause CSP header and request attribute to diverge.
  2. Missing HTML nonce injection — No CspNonceHtmlFilter / CspNonceHtmlProcessor / web.xml registration to inject nonce="..." into HTML <script> and <style> tags.
  3. Other call sites not updatedAtlasAuthenticationFilter, AtlasCSRFPreventionFilter, AtlasHeaderPreAuthFilter, AtlasKnoxSSOAuthenticationFilter, AtlasHttpServlet still call setSecurityHeaders(wrapper) without a shared request-scoped nonce.
  4. Test coverage gaps — Missing tests for nonce reuse, custom CSP override behavior, and server-common HeadersUtilTest.

Recommendation

Request changes. Please either:

  • Extend this PR with getOrCreateNonce(request), update all call sites, and add the HTML nonce injection pipeline, OR
  • Split explicitly: merge header changes only with a linked follow-up PR for HTML injection + filter wiring, and coordinate with the React Dashboard UI PR before enabling in production.

Happy to re-review once the above is addressed.

@chaitalicod
chaitalicod force-pushed the ATLAS-5386 branch 2 times, most recently from 151089c to 5e7d664 Compare August 27, 2026 07:53
…inline/unsafe-eval) and enabling nonce-based policy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants