You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "kgabryje (via GitHub)" <gi...@apache.org> on 2023/06/01 11:49:15 UTC

[GitHub] [superset] kgabryje opened a new pull request, #24262: chore: Enable CSP by default

kgabryje opened a new pull request, #24262:
URL: https://github.com/apache/superset/pull/24262

   
   ### SUMMARY
   This PR enables `TALISMAN_ENABLED` by default and provides a default content security policy config.
   Please keep in mind that this is a **breaking change**. If your Superset deployment loads additional scripts, loads images from external domains, performs HTTP requests to external domains, you need to adjust the default CSP config by adding external origins to appropriate CSP directives and/or mark the scripts with nonce as described in the updated docs.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] commented on pull request #24262: chore: Enable CSP by default

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #24262:
URL: https://github.com/apache/superset/pull/24262#issuecomment-1571916861

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#24262](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (d093ef9) into [master](https://app.codecov.io/gh/apache/superset/commit/218d1c7405a77435285c0d70eff5b6f0d7c6ac02?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (218d1c7) will **decrease** coverage by `11.22%`.
   > The diff coverage is `42.30%`.
   
   > :exclamation: Current head d093ef9 differs from pull request most recent head 34cb843. Consider uploading reports for the commit 34cb843 to get more accurate results
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #24262       +/-   ##
   ===========================================
   - Coverage   68.30%   57.09%   -11.22%     
   ===========================================
     Files        1957     1957               
     Lines       75590    75596        +6     
     Branches     8223     8222        -1     
   ===========================================
   - Hits        51635    43163     -8472     
   - Misses      21848    30325     +8477     
   - Partials     2107     2108        +1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `?` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `53.29% <100.00%> (+<0.01%)` | :arrow_up: |
   | python | `59.62% <100.00%> (-23.20%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `53.42% <100.00%> (-0.01%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [.../superset-ui-core/src/models/ExtensibleFunction.ts](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvbW9kZWxzL0V4dGVuc2libGVGdW5jdGlvbi50cw==) | `100.00% <ø> (ø)` | |
   | [...set-ui-core/src/ui-overrides/ExtensionsRegistry.ts](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY29yZS9zcmMvdWktb3ZlcnJpZGVzL0V4dGVuc2lvbnNSZWdpc3RyeS50cw==) | `100.00% <ø> (ø)` | |
   | [...hart-echarts/src/BigNumber/BigNumberTotal/index.ts](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQmlnTnVtYmVyL0JpZ051bWJlclRvdGFsL2luZGV4LnRz) | `66.66% <ø> (ø)` | |
   | [...arts/src/BigNumber/BigNumberWithTrendline/index.ts](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQmlnTnVtYmVyL0JpZ051bWJlcldpdGhUcmVuZGxpbmUvaW5kZXgudHM=) | `66.66% <ø> (ø)` | |
   | [...t-frontend/plugins/plugin-chart-table/src/index.ts](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtdGFibGUvc3JjL2luZGV4LnRz) | `66.66% <ø> (ø)` | |
   | [...nd/src/components/ErrorMessage/BasicErrorAlert.tsx](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRXJyb3JNZXNzYWdlL0Jhc2ljRXJyb3JBbGVydC50c3g=) | `93.75% <ø> (ø)` | |
   | [superset-frontend/src/components/Table/index.tsx](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFibGUvaW5kZXgudHN4) | `75.29% <ø> (ø)` | |
   | [superset-frontend/src/components/Tags/Tag.tsx](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvVGFncy9UYWcudHN4) | `62.50% <ø> (ø)` | |
   | [...veFilters/FilterBar/FilterControls/FilterValue.tsx](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyQmFyL0ZpbHRlckNvbnRyb2xzL0ZpbHRlclZhbHVlLnRzeA==) | `5.66% <0.00%> (-0.06%)` | :arrow_down: |
   | [superset-frontend/src/dashboard/types.ts](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC90eXBlcy50cw==) | `100.00% <ø> (ø)` | |
   | ... and [8 more](https://app.codecov.io/gh/apache/superset/pull/24262?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
   
   ... and [305 files with indirect coverage changes](https://app.codecov.io/gh/apache/superset/pull/24262/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] github-code-scanning[bot] commented on a diff in pull request #24262: chore: Enable CSP by default

Posted by "github-code-scanning[bot] (via GitHub)" <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #24262:
URL: https://github.com/apache/superset/pull/24262#discussion_r1213037371


##########
superset/templates/superset/theme.html:
##########
@@ -1340,7 +1340,7 @@
 {% endblock %}
 {% block tail_js %}
   {{ super() }}
-  <script src="https://code.jquery.com/jquery-1.10.2.min.js"></script>
-  <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js"></script>
-  <script src="{{ assets_prefix }}/static/assets/stylesheets/less/cosmo/cosmoTheme.js"></script>
+  <script src="https://code.jquery.com/jquery-1.10.2.min.js" nonce="{{ csp_nonce() }}"></script>

Review Comment:
   ## Inclusion of functionality from an untrusted source
   
   Script loaded from content delivery network with no integrity check.
   
   [Show more details](https://github.com/apache/superset/security/code-scanning/1204)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on a diff in pull request #24262: chore: Enable CSP by default

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #24262:
URL: https://github.com/apache/superset/pull/24262#discussion_r1234138092


##########
superset/config.py:
##########
@@ -1370,13 +1370,43 @@ def EMAIL_HEADER_MUTATOR(  # pylint: disable=invalid-name,unused-argument
 CONTENT_SECURITY_POLICY_WARNING = True
 
 # Do you want Talisman enabled?
-TALISMAN_ENABLED = False
+TALISMAN_ENABLED = True
 # If you want Talisman, how do you want it configured??
 TALISMAN_CONFIG = {
-    "content_security_policy": None,
-    "force_https": True,
+    "content_security_policy": {
+        "default-src": ["'self'"],
+        "img-src": ["'self'", "data:"],
+        "worker-src": ["'self'", "blob:"],
+        "connect-src": [
+            "'self'",
+            "https://api.mapbox.com",
+            "https://events.mapbox.com",
+        ],
+        "object-src": "'none'",
+        "style-src": ["'self'", "'unsafe-inline'"],
+        "script-src": ["'self'", "'strict-dynamic'"],
+    },
+    "content_security_policy_nonce_in": ["script-src"],
+    "force_https": False,

Review Comment:
   I wanted to limit the scope of the changes in this PR just to CSP - and `force_https` was disabled by default so far (since Talisman was disabled by default)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on a diff in pull request #24262: chore: Enable CSP by default

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #24262:
URL: https://github.com/apache/superset/pull/24262#discussion_r1213037154


##########
superset-frontend/packages/superset-ui-core/src/models/ExtensibleFunction.ts:
##########
@@ -22,9 +22,8 @@
  */
 
 export default class ExtensibleFunction extends Function {
+  // @ts-ignore
   constructor(fn: Function) {
-    super();

Review Comment:
   Calling `new Function()` breaks the CSP rule that disallows using `eval` in scripts.
   Extending `Function` is a nice hack that let's us make a class instance (in most cases it's a formatter) callable - for example instead of calling `formatter.format(value)` we call `formatter(value)`. Removing `super()` does not break that behaviour while also letting us avoid calling Function constructor.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on a diff in pull request #24262: chore: Enable CSP by default

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #24262:
URL: https://github.com/apache/superset/pull/24262#discussion_r1235211441


##########
superset/initialization/__init__.py:
##########
@@ -613,7 +613,11 @@ def __call__(
 
         # Talisman
         talisman_enabled = self.config["TALISMAN_ENABLED"]
-        talisman_config = self.config["TALISMAN_CONFIG"]
+        talisman_config = (
+            self.config["TALISMAN_DEV_CONFIG"]
+            if self.superset_app.debug
+            else self.config["TALISMAN_CONFIG"]
+        )

Review Comment:
   We must have "unsafe-eval" in dev mode - React and Webpack use it and there's no way around it



##########
superset/config.py:
##########
@@ -1363,13 +1363,43 @@ def EMAIL_HEADER_MUTATOR(  # pylint: disable=invalid-name,unused-argument
 CONTENT_SECURITY_POLICY_WARNING = True
 
 # Do you want Talisman enabled?
-TALISMAN_ENABLED = False
+TALISMAN_ENABLED = True
 # If you want Talisman, how do you want it configured??
 TALISMAN_CONFIG = {
-    "content_security_policy": None,
-    "force_https": True,
+    "content_security_policy": {
+        "default-src": ["'self'"],
+        "img-src": ["'self'", "data:"],
+        "worker-src": ["'self'", "blob:"],
+        "connect-src": [
+            "'self'",
+            "https://api.mapbox.com",
+            "https://events.mapbox.com",
+        ],
+        "object-src": "'none'",
+        "style-src": ["'self'", "'unsafe-inline'"],
+        "script-src": ["'self'", "'strict-dynamic'"],
+    },
+    "content_security_policy_nonce_in": ["script-src"],
+    "force_https": False,
     "force_https_permanent": False,
 }
+# React requires `eval` to work correctly in dev mode
+TALISMAN_DEV_CONFIG = {
+    "content_security_policy": {
+        "default-src": ["'self'"],
+        "img-src": ["'self'", "data:"],
+        "worker-src": ["'self'", "blob:"],
+        "connect-src": [
+            "'self'",
+            "https://api.mapbox.com",
+            "https://events.mapbox.com",
+        ],
+        "object-src": "'none'",
+        "style-src": ["'self'", "'unsafe-inline'"],
+        "script-src": ["'self'", "'unsafe-inline'", "'unsafe-eval'"],
+    },
+    "content_security_policy_nonce_in": ["script-src"],
+}

Review Comment:
   Yup nice catch!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] eschutho commented on a diff in pull request #24262: chore: Enable CSP by default

Posted by "eschutho (via GitHub)" <gi...@apache.org>.
eschutho commented on code in PR #24262:
URL: https://github.com/apache/superset/pull/24262#discussion_r1231643845


##########
superset/config.py:
##########
@@ -1370,13 +1370,43 @@ def EMAIL_HEADER_MUTATOR(  # pylint: disable=invalid-name,unused-argument
 CONTENT_SECURITY_POLICY_WARNING = True
 
 # Do you want Talisman enabled?
-TALISMAN_ENABLED = False
+TALISMAN_ENABLED = True
 # If you want Talisman, how do you want it configured??
 TALISMAN_CONFIG = {
-    "content_security_policy": None,
-    "force_https": True,
+    "content_security_policy": {
+        "default-src": ["'self'"],
+        "img-src": ["'self'", "data:"],
+        "worker-src": ["'self'", "blob:"],
+        "connect-src": [
+            "'self'",
+            "https://api.mapbox.com",
+            "https://events.mapbox.com",
+        ],
+        "object-src": "'none'",
+        "style-src": ["'self'", "'unsafe-inline'"],
+        "script-src": ["'self'", "'strict-dynamic'"],
+    },
+    "content_security_policy_nonce_in": ["script-src"],
+    "force_https": False,

Review Comment:
   @kgabryje curious why this force_https change?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje commented on pull request #24262: chore: Enable CSP by default

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #24262:
URL: https://github.com/apache/superset/pull/24262#issuecomment-1572428645

   Well spotted @sfirke, thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] sfirke commented on pull request #24262: chore: Enable CSP by default

Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #24262:
URL: https://github.com/apache/superset/pull/24262#issuecomment-1572201368

   Two things re: docs changes:
   1) Typo: "Superset needs **both** `style-src unsafe-inline` CSP directive in order to operate." should be "Superset needs **the** ..."
   2) Changing `force_https` to now default to False seems good to me.  But it means that this section on the Security page "[Other Talisman security considerations](https://superset.apache.org/docs/security/#other-talisman-security-considerations)" needs to be updated.  For instance right now there's text talking about how to override the default value of force_https = True.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] villebro commented on a diff in pull request #24262: chore: Enable CSP by default

Posted by "villebro (via GitHub)" <gi...@apache.org>.
villebro commented on code in PR #24262:
URL: https://github.com/apache/superset/pull/24262#discussion_r1235201986


##########
superset/initialization/__init__.py:
##########
@@ -613,7 +613,11 @@ def __call__(
 
         # Talisman
         talisman_enabled = self.config["TALISMAN_ENABLED"]
-        talisman_config = self.config["TALISMAN_CONFIG"]
+        talisman_config = (
+            self.config["TALISMAN_DEV_CONFIG"]
+            if self.superset_app.debug
+            else self.config["TALISMAN_CONFIG"]
+        )

Review Comment:
   I'm wondering if it's a good idea to have separate dev and non-dev configs. Should these maybe be `TALISMAN_DEFAULT_DEV_CONFIG` and `TALISMAN_DEFAULT_PROD_CONFIG`, and only if `TALISMAN_CONFIG` is undefined would we fall back to the default. Thoughts?



##########
superset/config.py:
##########
@@ -1370,13 +1370,43 @@ def EMAIL_HEADER_MUTATOR(  # pylint: disable=invalid-name,unused-argument
 CONTENT_SECURITY_POLICY_WARNING = True
 
 # Do you want Talisman enabled?
-TALISMAN_ENABLED = False
+TALISMAN_ENABLED = True
 # If you want Talisman, how do you want it configured??
 TALISMAN_CONFIG = {
-    "content_security_policy": None,
-    "force_https": True,
+    "content_security_policy": {
+        "default-src": ["'self'"],
+        "img-src": ["'self'", "data:"],
+        "worker-src": ["'self'", "blob:"],
+        "connect-src": [
+            "'self'",
+            "https://api.mapbox.com",
+            "https://events.mapbox.com",
+        ],
+        "object-src": "'none'",
+        "style-src": ["'self'", "'unsafe-inline'"],
+        "script-src": ["'self'", "'strict-dynamic'"],
+    },
+    "content_security_policy_nonce_in": ["script-src"],
+    "force_https": False,

Review Comment:
   I agree with keeping this disabled by default - I would wager that by far the majority of prod Superset deployments terminate SSL/TLS on the LB.



##########
superset/config.py:
##########
@@ -1363,13 +1363,43 @@ def EMAIL_HEADER_MUTATOR(  # pylint: disable=invalid-name,unused-argument
 CONTENT_SECURITY_POLICY_WARNING = True
 
 # Do you want Talisman enabled?
-TALISMAN_ENABLED = False
+TALISMAN_ENABLED = True
 # If you want Talisman, how do you want it configured??
 TALISMAN_CONFIG = {
-    "content_security_policy": None,
-    "force_https": True,
+    "content_security_policy": {
+        "default-src": ["'self'"],
+        "img-src": ["'self'", "data:"],
+        "worker-src": ["'self'", "blob:"],
+        "connect-src": [
+            "'self'",
+            "https://api.mapbox.com",
+            "https://events.mapbox.com",
+        ],
+        "object-src": "'none'",
+        "style-src": ["'self'", "'unsafe-inline'"],
+        "script-src": ["'self'", "'strict-dynamic'"],
+    },
+    "content_security_policy_nonce_in": ["script-src"],
+    "force_https": False,
     "force_https_permanent": False,
 }
+# React requires `eval` to work correctly in dev mode
+TALISMAN_DEV_CONFIG = {
+    "content_security_policy": {
+        "default-src": ["'self'"],
+        "img-src": ["'self'", "data:"],
+        "worker-src": ["'self'", "blob:"],
+        "connect-src": [
+            "'self'",
+            "https://api.mapbox.com",
+            "https://events.mapbox.com",
+        ],
+        "object-src": "'none'",
+        "style-src": ["'self'", "'unsafe-inline'"],
+        "script-src": ["'self'", "'unsafe-inline'", "'unsafe-eval'"],
+    },
+    "content_security_policy_nonce_in": ["script-src"],
+}

Review Comment:
   shouldn't we set `force_https: False` here, too? AFAIK it defaults to `True`, right?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] kgabryje merged pull request #24262: chore: Enable CSP by default

Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje merged PR #24262:
URL: https://github.com/apache/superset/pull/24262


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org