Fix content security policy #1353
Conversation
shahar1
force-pushed
the
fix-content-security-policy
branch
from
db9a904 to
8aa638d
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds a Content Security Policy (CSP) meta tag to the Sphinx Airflow theme's layout template to enhance web application security by controlling which sources can load content.
Key changes:
- Added CSP meta tag with directives for default sources and fonts to restrict content loading origins
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shahar1
force-pushed
the
fix-content-security-policy
branch
2 times, most recently
from
a60a681 to
637900c
Compare
shahar1
force-pushed
the
fix-content-security-policy
branch
from
637900c to
19690c9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <meta name="viewport" content="width=device-width, initial-scale=1"> | ||
| <meta http-equiv="Content-Security-Policy" content=" | ||
| default-src 'self'; | ||
| script-src 'self' 'unsafe-inline' 'unsafe-eval' | ||
| https://www.apachecon.com | ||
| https://www.communityovercode.org | ||
| https://*.apache.org | ||
| https://*.scarf.sh; | ||
| style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; | ||
| font-src 'self' https://fonts.gstatic.com data:; | ||
| img-src 'self' data: https://*.apache.org https://*.scarf.sh; | ||
| connect-src 'self' https://*.apache.org https://*.scarf.sh; | ||
| frame-ancestors 'none'; | ||
| object-src 'none'; | ||
| base-uri 'self'; | ||
| form-action 'self'; |
There was a problem hiding this comment.
The viewport meta tag has been removed but this is essential for responsive design on mobile devices. The CSP meta tag should be added without removing the viewport meta tag. Add back the line:
<meta name="viewport" content="width=device-width, initial-scale=1">
| <meta name="viewport" content="width=device-width, initial-scale=1"> | ||
| <meta http-equiv="Content-Security-Policy" content=" | ||
| default-src 'self'; | ||
| script-src 'self' 'unsafe-inline' 'unsafe-eval' |
There was a problem hiding this comment.
The use of 'unsafe-inline' and 'unsafe-eval' in the script-src directive significantly weakens the Content Security Policy, as they allow inline scripts and eval() usage which are common XSS attack vectors. Consider using nonces or hashes for inline scripts instead of 'unsafe-inline', and refactor code to eliminate the need for 'unsafe-eval'. If these are required for third-party dependencies or Sphinx functionality, document why they are necessary and consider if they can be scoped more narrowly.
| script-src 'self' 'unsafe-inline' 'unsafe-eval' | |
| script-src 'self' |
| https://www.communityovercode.org | ||
| https://*.apache.org | ||
| https://*.scarf.sh; | ||
| style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; |
There was a problem hiding this comment.
The use of 'unsafe-inline' in the style-src directive weakens the Content Security Policy by allowing inline styles, which can be exploited for certain attacks. Consider using nonces or hashes for inline styles, or moving styles to external stylesheets. If 'unsafe-inline' is required for Sphinx functionality or third-party dependencies, document the necessity.
| style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; | |
| style-src 'self' https://fonts.googleapis.com; |
| <meta http-equiv="Content-Security-Policy" content=" | ||
| default-src 'self'; | ||
| script-src 'self' 'unsafe-inline' 'unsafe-eval' | ||
| https://www.apachecon.com | ||
| https://www.communityovercode.org | ||
| https://*.apache.org | ||
| https://*.scarf.sh; | ||
| style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; | ||
| font-src 'self' https://fonts.gstatic.com data:; | ||
| img-src 'self' data: https://*.apache.org https://*.scarf.sh; | ||
| connect-src 'self' https://*.apache.org https://*.scarf.sh; | ||
| frame-ancestors 'none'; | ||
| object-src 'none'; | ||
| base-uri 'self'; | ||
| form-action 'self'; | ||
| "> |
There was a problem hiding this comment.
The CSP meta tag has inconsistent indentation. The opening line uses 4 spaces, but the policy directives use 6 spaces. For better readability and consistency, align all directives with the same indentation level, or consider placing the entire CSP on a single line to avoid formatting issues.
| <meta http-equiv="Content-Security-Policy" content=" | |
| default-src 'self'; | |
| script-src 'self' 'unsafe-inline' 'unsafe-eval' | |
| https://www.apachecon.com | |
| https://www.communityovercode.org | |
| https://*.apache.org | |
| https://*.scarf.sh; | |
| style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; | |
| font-src 'self' https://fonts.gstatic.com data:; | |
| img-src 'self' data: https://*.apache.org https://*.scarf.sh; | |
| connect-src 'self' https://*.apache.org https://*.scarf.sh; | |
| frame-ancestors 'none'; | |
| object-src 'none'; | |
| base-uri 'self'; | |
| form-action 'self'; | |
| "> | |
| <meta http-equiv="Content-Security-Policy" content="default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval' https://www.apachecon.com https://www.communityovercode.org https://*.apache.org https://*.scarf.sh; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com data:; img-src 'self' data: https://*.apache.org https://*.scarf.sh; connect-src 'self' https://*.apache.org https://*.scarf.sh; frame-ancestors 'none'; object-src 'none'; base-uri 'self'; form-action 'self';"> |
|
I think we do not need that (And should not have it). CSP is added automatically by ASF hosting. |
|
This is what we have today - added by the ASF Apache server serving our pages: 
|
|
Don't believe in everything that Copilot says ;) |
Interesting, wasn't aware of that - thanks! |
I think that it blocks Google originated fonts. |
We should then embed those fonts in our page. The problem is that if we need fonts to be downloaded from somewhere (say google) - in order to render the page - this is Privacy concern that we don't want to deal with. Similarly I embedded screenshots from youtube videos to be served from our page rather than from youtube.com -> user needs to explicitly click on those to be redirected to youtube page, but there is no privacy tracking when they just view our page. |
|
That's precisely why CSP in the ASF is supposed to prevent :) |
|
BTW. Which page generates those font issues? I thought I looked at all pages before and have not seen any remaining CSP issues ? |
One of my browser's extensions caused the main page to raise those errors 🤦♂️ |
2 participants
Copilot Summary
This pull request introduces a significant security improvement to the
sphinx_airflow_themeby adding a Content Security Policy (CSP) header to thelayout.htmltemplate. The CSP restricts the sources for scripts, styles, fonts, images, and other resources, enhancing the protection against cross-site scripting (XSS) and related attacks.Security enhancements:
<meta http-equiv="Content-Security-Policy">tag tolayout.htmlto enforce strict resource loading policies, specifying allowed sources for scripts, styles, fonts, images, connections, and more. This helps prevent XSS and content injection vulnerabilities.