[VPAT-50] chore(security): add X-Frame-Options header to nginx configuration to prevent clickjacking attacks #8507
Conversation
…uration to prevent clickjacking attacks
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
📝 WalkthroughWalkthroughAdded four security response headers to the nginx server blocks in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
��� Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Look into this PR for adding more header options. |
![coderabbitai[bot]](https://cdn.statically.io/img/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 @apps/admin/nginx/nginx.conf:
- Around line 23-27: Remove the obsolete X-XSS-Protection header and either
remove or conditionally apply Strict-Transport-Security depending on whether TLS
is terminated here (if HTTPS is terminated upstream, move/ensure HSTS is set at
the TLS terminator; otherwise remove the Strict-Transport-Security line), keep
X-Frame-Options as a fallback, and add a Content-Security-Policy header with at
minimum "default-src 'self'; frame-ancestors 'none';" (adjust additional CSP
directives as needed for your app).
In @apps/web/nginx/nginx.conf:
- Around line 23-27: Remove the ineffective/deprecated headers: delete the
add_header directive for X-XSS-Protection and remove or gate the add_header
Strict-Transport-Security directive (the Strict-Transport-Security header must
only be set when HTTPS is terminated before this server; if HTTPS is terminated
elsewhere, ensure that logic only sets add_header Strict-Transport-Security when
requests are secure). Instead add a Content-Security-Policy header via
add_header Content-Security-Policy with a restrictive policy (for example:
default-src 'self'; frame-ancestors 'none';) to replace
X-Frame-Options/X-XSS-Protection functionality in modern browsers and improve
protection.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/admin/nginx/nginx.confapps/web/nginx/nginx.conf
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: check:types
- GitHub Check: check:lint
- GitHub Check: Analyze (javascript)
| # Security headers | ||
| add_header X-Frame-Options "DENY" always; | ||
| add_header X-Content-Type-Options "nosniff" always; | ||
| add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; | ||
| add_header X-XSS-Protection "1; mode=block" always; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "nginx.conf" | head -20Repository: makeplane/plane
Length of output: 146
🏁 Script executed:
# Check the admin nginx config
if [ -f "apps/admin/nginx/nginx.conf" ]; then
cat -n apps/admin/nginx/nginx.conf
fiRepository: makeplane/plane
Length of output: 1119
🏁 Script executed:
# Check the web nginx config for comparison
if [ -f "apps/web/nginx/nginx.conf" ]; then
cat -n apps/web/nginx/nginx.conf
fiRepository: makeplane/plane
Length of output: 1118
🌐 Web query:
HSTS Strict-Transport-Security HTTP port 3000 browser behavior
💡 Result:
Short answer
- HSTS policies are stored per host name (not per port) and apply to all ports. If a browser has an HSTS entry for example.com and you try to open http://example.com:3000/, the browser will change the scheme to https but keep the explicit port 3000 (i.e. it will try https://example.com:3000/). [1][2]
Key points
- Browser only accepts Strict-Transport-Security over an HTTPS response. HSTS is added to the browser’s HSTS list only after receiving the header on TLS. [1][2]
- When upgrading an http URL: if the URL contains an explicit port other than 80, that port is preserved; if it contains port 80 it is converted to 443. If no port is present the UA does not add one. [2]
- If HTTPS-to-port-3000 fails (e.g., no TLS server or certificate error) and the host is known HSTS, the UA must terminate the connection and not allow a bypass. [2]
Examples
- http://example.com:3000/ → browser rewrites to https://example.com:3000/ (and then attempts TLS). [1][2]
- http://example.com/ (no explicit port) → rewritten to https://example.com/ (port handling per spec: implicit 80→443). [2]
Sources
[1] MDN: Strict-Transport-Security header.
[2] RFC 6797 (HTTP Strict Transport Security).
🌐 Web query:
X-XSS-Protection deprecated obsolete 2024
💡 Result:
Short answer: Yes — X-XSS-Protection is functionally deprecated/obsolete in modern browsers and is no longer recommended (use Content-Security-Policy instead). Set it to 0 or stop sending it. [1][2][3][4]
Key points and sources:
- MDN: X-XSS-Protection is non‑standard, Chrome removed its XSS Auditor, Firefox never implemented it, Edge retired its filter — use CSP instead. [1]
- Chromium / Chrome 78: the XSS Auditor was removed (Chrome no longer honors X-XSS-Protection). [2]
- OWASP Secure Headers / OWASP guidance: header is deprecated; recommendation is to disable it (X-XSS-Protection: 0) and rely on CSP. [3]
- Mozilla infosec guidance shows the header marked obsolete in their 2024 updates and recommends CSP. [4]
If you want, I can show the exact header config snippets (NGINX/Apache/Express) to explicitly disable it and add a CSP example.
🌐 Web query:
Content-Security-Policy frame-ancestors modern browser support
💡 Result:
Short summary — modern support for Content-Security-Policy's frame-ancestors
- What it does: blocks/allows which parent origins may embed a page (applies to frame, iframe, object, embed). Must be sent in the HTTP Content-Security-Policy header (not meta). [1][2]
- Broad modern support: supported in current Chrome, Firefox, Safari, Opera and Chromium-based Edge (desktop and mobile). Use frame-ancestors (CSP Level 2+) instead of the deprecated X-Frame-Options when those browsers are targeted. [1][3]
- Not supported in Internet Explorer (IE11 and older). If you need IE compatibility, include a fallback X-Frame-Options header (DENY or SAMEORIGIN) alongside CSP. [2][3]
- Practical note: nested frames are checked against the policy of the framed resource (all ancestors must match); use both CSP and X-Frame-Options when you must cover older clients. [1][2]
Sources: MDN (frame-ancestors) [1]; Mozilla web-security guidance (recommend X-Frame-Options fallback) [2]; content-security-policy.com compatibility notes [3].
Verify HSTS usage and remove deprecated X-XSS-Protection header.
Good additions for X-Frame-Options and X-Content-Type-Options. However, there are concerns with the other headers:
-
HSTS on port 3000 (line 26): The
Strict-Transport-Securityheader only functions over HTTPS connections. Since this server listens on HTTP port 3000, browsers will ignore this header entirely. If a reverse proxy handles HTTPS termination in front, verify that HSTS is appropriate for your deployment architecture. Otherwise, remove this header. -
X-XSS-Protection is deprecated (line 27): This header is obsolete and no longer honored by modern browsers (Chrome removed the XSS Auditor; Firefox and Edge don't implement it). Remove this header and rely on
Content-Security-Policyinstead. -
Consider adding Content-Security-Policy: Add a CSP header with appropriate directives including
frame-ancestors 'none'(which supersedes X-Frame-Options in modern browsers) anddefault-src 'self'. Keep X-Frame-Options as a fallback for older browsers.
🔒 Proposed security header improvements
# Security headers
add_header X-Frame-Options "DENY" always;
add_header X-Content-Type-Options "nosniff" always;
-add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always;
-add_header X-XSS-Protection "1; mode=block" always;
+# Remove HSTS if serving over HTTP, or verify reverse proxy handles HTTPS
+add_header Content-Security-Policy "default-src 'self'; frame-ancestors 'none';" always;📝 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.
| # Security headers | |
| add_header X-Frame-Options "DENY" always; | |
| add_header X-Content-Type-Options "nosniff" always; | |
| add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; | |
| add_header X-XSS-Protection "1; mode=block" always; | |
| # Security headers | |
| add_header X-Frame-Options "DENY" always; | |
| add_header X-Content-Type-Options "nosniff" always; | |
| # Remove HSTS if serving over HTTP, or verify reverse proxy handles HTTPS | |
| add_header Content-Security-Policy "default-src 'self'; frame-ancestors 'none';" always; |
🤖 Prompt for AI Agents
In @apps/admin/nginx/nginx.conf around lines 23 - 27, Remove the obsolete
X-XSS-Protection header and either remove or conditionally apply
Strict-Transport-Security depending on whether TLS is terminated here (if HTTPS
is terminated upstream, move/ensure HSTS is set at the TLS terminator; otherwise
remove the Strict-Transport-Security line), keep X-Frame-Options as a fallback,
and add a Content-Security-Policy header with at minimum "default-src 'self';
frame-ancestors 'none';" (adjust additional CSP directives as needed for your
app).
| # Security headers | ||
| add_header X-Frame-Options "DENY" always; | ||
| add_header X-Content-Type-Options "nosniff" always; | ||
| add_header Strict-Transport-Security "max-age=31536000; includeSubDomains" always; | ||
| add_header X-XSS-Protection "1; mode=block" always; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the nginx.conf file to understand the actual configuration
cat -n apps/web/nginx/nginx.confRepository: makeplane/plane
Length of output: 1118
🏁 Script executed:
# Search for HTTPS, SSL, and port configurations
rg -n "listen|ssl|https|443" apps/web/nginx/nginx.confRepository: makeplane/plane
Length of output: 78
🏁 Script executed:
# Search for reverse proxy or upstream configurations
rg -n "proxy_pass|upstream" apps/web/nginx/Repository: makeplane/plane
Length of output: 41
🌐 Web query:
X-XSS-Protection header deprecated modern browsers security
💡 Result:
Short answer: X-XSS-Protection is deprecated — modern browsers removed or stopped honoring it because the built‑in XSS auditors were inconsistent, bypassable and could create side‑channel/XS‑Leak issues. Use a strong Content‑Security‑Policy instead; if you must keep the header for very old browsers, explicitly disable the broken auditor with "X-XSS-Protection: 0". (owasp.org)
Citations:
Remove deprecated X-XSS-Protection header and reconsider HSTS on HTTP.
Good additions for X-Frame-Options and X-Content-Type-Options. However:
-
HSTS ineffective on HTTP (line 26): The
Strict-Transport-Securityheader requires HTTPS to function. This server listens on port 3000 over HTTP with no reverse proxy handling HTTPS in the configuration. Browsers ignore HSTS headers on HTTP connections, making this header ineffective. Either remove it or verify that HTTPS is terminated elsewhere before requests reach this server. -
Remove X-XSS-Protection (line 27): This header is deprecated and ignored by modern browsers. It can introduce vulnerabilities in older browsers. Remove it and use
Content-Security-Policyinstead. -
Consider adding Content-Security-Policy: Add a CSP header (e.g.,
"default-src 'self'; frame-ancestors 'none';") for comprehensive security coverage, which supersedes X-Frame-Options in modern browsers.
🤖 Prompt for AI Agents
In @apps/web/nginx/nginx.conf around lines 23 - 27, Remove the
ineffective/deprecated headers: delete the add_header directive for
X-XSS-Protection and remove or gate the add_header Strict-Transport-Security
directive (the Strict-Transport-Security header must only be set when HTTPS is
terminated before this server; if HTTPS is terminated elsewhere, ensure that
logic only sets add_header Strict-Transport-Security when requests are secure).
Instead add a Content-Security-Policy header via add_header
Content-Security-Policy with a restrictive policy (for example: default-src
'self'; frame-ancestors 'none';) to replace X-Frame-Options/X-XSS-Protection
functionality in modern browsers and improve protection.
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.