You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/06/27 00:09:05 UTC
[PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
rusackas opened a new pull request, #41468:
URL: https://github.com/apache/superset/pull/41468
### SUMMARY
Adds a **first-class `IFRAME` dashboard component** plus a **runtime, permission-gated Content Security Policy (CSP) allowlist**, so trusted Admins can embed external content in dashboards and "punch holes" in the CSP at runtime — without editing `TALISMAN_CONFIG` and restarting every Superset process.
This is a **draft** opened for design discussion. A companion **`SIP.md`** (following the SIP issue template) is included in the diff and tracks the design; it will be kept in sync as the implementation evolves.
**How it works**
Superset's default CSP declares `default-src 'self'` and no `frame-src`, so external iframes are blocked. Rather than reconfigure flask-talisman at runtime, this adds an `after_request` hook registered **before** Talisman — so, because Flask runs `after_request` callbacks in reverse order, it runs **after** Talisman sets the header — that merges operator-curated allowlist origins into the response CSP header. Entries live in a new `csp_allowlist` table and are cached in-process with a short TTL (invalidated on write).
The whole runtime-override path is **inert unless the `CSP_RUNTIME_ALLOWLIST` feature flag is on** (default off), so the static deploy-time policy remains the default and operators opt in explicitly.
**Design decisions (resolved):** global allowlist scope; feature-flag kill-switch so operators retain full control. See `SIP.md` for rationale and rejected alternatives.
**Security posture:** off by default; write permission is Admin-only (`CSPAllowlist` in `ADMIN_ONLY_VIEW_MENUS`); strict server-side origin validation (bare `scheme://host[:port]`, no wildcards/paths/credentials); only a fixed set of directives may be widened — notably **not** `script-src`; iframes are sandboxed; audit trail via the audit mixin.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
_To add — UI is feature-flagged behind `CSP_RUNTIME_ALLOWLIST`._
### TESTING INSTRUCTIONS
1. Enable the feature flag: `FEATURE_FLAGS = {"CSP_RUNTIME_ALLOWLIST": True}` and run the DB migration (`superset db upgrade`).
2. As an Admin, edit a dashboard → drag the new **Embed / Iframe** element from the Layout elements pane → paste an external URL (e.g. `https://example.com`).
3. Observe the **"This domain is not allowed to be embedded"** warning and the **Enable domain in CSP** button. Click it.
4. The origin is added to the allowlist; the iframe now loads without a server restart. Confirm the response `Content-Security-Policy` header now includes `frame-src 'self' https://example.com`.
5. As a Gamma user, confirm the Enable button is absent (ask-admin text instead) and `POST /api/v1/csp_allowlist/` is rejected.
Automated: `pytest tests/unit_tests/security/csp_test.py` (validation/merge/hook), `tests/integration_tests/csp_allowlist/api_tests.py` (API + permission gating), and `npx jest src/dashboard/util/cspAllowlist.test.ts src/dashboard/components/gridComponents/Iframe` (util + component).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: **`CSP_RUNTIME_ALLOWLIST`**
- [x] Changes UI
- [x] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [x] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-4814264020
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-4814293249
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41468?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `91.66667%` with `8 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 64.53%. Comparing base ([`022f66a`](https://app.codecov.io/gh/apache/superset/commit/022f66a6941087db6f90429dfb2a316d8aed29ee?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`7df650e`](https://app.codecov.io/gh/apache/superset/commit/7df650ec04b23e798e5b1208a2a2d012044b2ca7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/41468?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...hboard/components/gridComponents/Iframe/Iframe.tsx](https://app.codecov.io/gh/apache/superset/pull/41468?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fdashboard%2Fcomponents%2FgridComponents%2FIframe%2FIframe.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0lmcmFtZS9JZnJhbWUudHN4) | 90.90% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/41468?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...hboard/components/gridComponents/new/NewIframe.tsx](https://app.codecov.io/gh/apache/superset/pull/41468?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2Fdashboard%2Fcomponents%2FgridComponents%2Fnew%2FNewIframe.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL25ldy9OZXdJZnJhbWUudHN4) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/41468?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #41468 +/- ##
==========================================
+ Coverage 64.52% 64.53% +0.01%
==========================================
Files 2664 2667 +3
Lines 146158 146254 +96
Branches 33715 33749 +34
==========================================
+ Hits 94305 94387 +82
- Misses 50136 50150 +14
Partials 1717 1717
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41468/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/41468/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.98% <91.66%> (+0.02%)` | :arrow_up: |
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.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41468?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-6022170519
### ⚠️ Possible DB migration conflict
@rusackas the base branch `master` picked up a new database migration under `superset/migrations/` after this PR was opened, and this PR adds or modifies migration files too. Alembic migrations form a single linear chain, so if both branch off the same parent revision, merging this PR as-is could leave the migration history with two heads instead of one, which makes `superset db upgrade` fail with `Multiple head revisions are present`.
To check and, if needed, fix this:
1. Rebase this branch onto the latest `master`.
2. Run `superset db heads`. If it lists more than one revision, you have a conflict.
3. In your new migration's script, update `down_revision` (and the `Revises` comment) to point at the other head instead, so the chain has a single head again.
See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-5786548399
### ⚠️ Possible DB migration conflict
@rusackas the base branch `master` picked up a new database migration under `superset/migrations/` after this PR was opened, and this PR adds or modifies migration files too. Alembic migrations form a single linear chain, so if both branch off the same parent revision, merging this PR as-is could leave the migration history with two heads instead of one, which makes `superset db upgrade` fail with `Multiple head revisions are present`.
To check and, if needed, fix this:
1. Rebase this branch onto the latest `master`.
2. Run `superset db heads`. If it lists more than one revision, you have a conflict.
3. In your new migration's script, update `down_revision` (and the `Revises` comment) to point at the other head instead, so the chain has a single head again.
See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-4814269393
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 7df650ec04b23e798e5b1208a2a2d012044b2ca7 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a3f14a2c42fd90008ee9824 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-41468--superset-docs-preview.netlify.app](https://deploy-preview-41468--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpLCoAgEADQu8w6yUxCu0HHmHQkwR9mRER3z91bvBdKzbE0WAEG2Csmc3Q7wjaas7B6peYjMQwh38GfjXlXMVLPlkrIz2Z7X3B2k0RhpHBWc84VkVZCwvcD) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-41468--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-5953113983
### ⚠️ Possible DB migration conflict
@rusackas the base branch `master` picked up a new database migration under `superset/migrations/` after this PR was opened, and this PR adds or modifies migration files too. Alembic migrations form a single linear chain, so if both branch off the same parent revision, merging this PR as-is could leave the migration history with two heads instead of one, which makes `superset db upgrade` fail with `Multiple head revisions are present`.
To check and, if needed, fix this:
1. Rebase this branch onto the latest `master`.
2. Run `superset db heads`. If it lists more than one revision, you have a conflict.
3. In your new migration's script, update `down_revision` (and the `Revises` comment) to point at the other head instead, so the chain has a single head again.
See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-5941610515
### ⚠️ Possible DB migration conflict
@rusackas the base branch `master` picked up a new database migration under `superset/migrations/` after this PR was opened, and this PR adds or modifies migration files too. Alembic migrations form a single linear chain, so if both branch off the same parent revision, merging this PR as-is could leave the migration history with two heads instead of one, which makes `superset db upgrade` fail with `Multiple head revisions are present`.
To check and, if needed, fix this:
1. Rebase this branch onto the latest `master`.
2. Run `superset db heads`. If it lists more than one revision, you have a conflict.
3. In your new migration's script, update `down_revision` (and the `Revises` comment) to point at the other head instead, so the chain has a single head again.
See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-6070753971
### ⚠️ Possible DB migration conflict
@rusackas the base branch `master` picked up a new database migration under `superset/migrations/` after this PR was opened, and this PR adds or modifies migration files too. Alembic migrations form a single linear chain, so if both branch off the same parent revision, merging this PR as-is could leave the migration history with two heads instead of one, which makes `superset db upgrade` fail with `Multiple head revisions are present`.
To check and, if needed, fix this:
1. Rebase this branch onto the latest `master`.
2. Run `superset db heads`. If it lists more than one revision, you have a conflict.
3. In your new migration's script, update `down_revision` (and the `Revises` comment) to point at the other head instead, so the chain has a single head again.
See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-5763325153
# 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️❗ @rusackas Your base branch `master` has also updated `superset/migrations`.
**Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-5719094626
# 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️❗ @rusackas Your base branch `master` has also updated `superset/migrations`.
**Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #41468:
URL: https://github.com/apache/superset/pull/41468#discussion_r3484690980
##########
tests/unit_tests/security/csp_test.py:
##########
@@ -0,0 +1,160 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from unittest.mock import patch
+
+import pytest
+from flask import Response
+
+from superset.security import csp as csp_module
+from superset.security.csp import (
+ apply_runtime_csp_allowlist,
+ is_valid_csp_directive,
+ is_valid_csp_origin,
+ merge_allowlist_into_csp,
+)
+
+
+@pytest.mark.parametrize(
+ "origin",
+ [
+ "https://example.com",
+ "http://example.com",
+ "https://example.com:8443",
+ "http://localhost:9000",
+ ],
+)
+def test_is_valid_csp_origin_accepts_bare_origins(origin: str) -> None:
+ assert is_valid_csp_origin(origin) is True
+
+
+@pytest.mark.parametrize(
+ "origin",
+ [
+ "",
+ "example.com", # missing scheme
+ "ftp://example.com", # disallowed scheme
+ "https://*.example.com", # wildcard
+ "https://example.com/path", # path
+ "https://example.com?q=1", # query
+ "https://example.com#frag", # fragment
+ "https://user:pw@example.com", # credentials
+ "javascript:alert(1)", # non-network scheme
+ "https://example.com ", # whitespace
+ ],
+)
+def test_is_valid_csp_origin_rejects_unsafe_values(origin: str) -> None:
+ assert is_valid_csp_origin(origin) is False
+
+
+def test_is_valid_csp_directive() -> None:
+ assert is_valid_csp_directive("frame-src") is True
+ assert is_valid_csp_directive("img-src") is True
+ # script-src is intentionally not allowlistable
+ assert is_valid_csp_directive("script-src") is False
+ assert is_valid_csp_directive("nonsense") is False
+
+
+def test_merge_seeds_a_missing_directive_with_self() -> None:
+ base = "default-src 'self'; img-src 'self' data:"
+ merged = merge_allowlist_into_csp(base, {"frame-src": ["https://maps.example"]})
+ assert "frame-src 'self' https://maps.example" in merged
+ # existing directives are preserved
+ assert "default-src 'self'" in merged
+ assert "img-src 'self' data:" in merged
+
+
+def test_merge_appends_to_existing_directive_without_duplicates() -> None:
+ base = "default-src 'self'; img-src 'self' data:"
+ merged = merge_allowlist_into_csp(
+ base, {"img-src": ["https://cdn.example", "data:"]}
+ )
+ assert "https://cdn.example" in merged
Review Comment:
## CodeQL / Incomplete URL substring sanitization
The string [https://cdn.example](1) may be at an arbitrary position in the sanitized URL.
[Show more details](https://github.com/apache/superset/security/code-scanning/2554)
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "sha174n (via GitHub)" <gi...@apache.org>.
sha174n commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-5280836264
Two notes on the iframe component:
1. The sandbox combines `allow-scripts` with `allow-same-origin`, which lets the embedded document remove its own sandboxing for its origin. For third-party embeds, could we drop `allow-same-origin` (or gate it behind an explicit opt-in) so an allowlisted-then-compromised origin can't run scripts as same-origin with forms/popups inside the Superset chrome?
2. Minor: allowlisting an origin makes it embeddable by any dashboard editor for all viewers (it's a global `frame-src`), and the client-side allowlist check is UX-only since the real enforcement is the CSP header. Might be worth documenting that trust boundary and adding a regression test that asserts the final CSP header on a real response.
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-6102865026
### ⚠️ Possible DB migration conflict
@rusackas the base branch `master` picked up a new database migration under `superset/migrations/` after this PR was opened, and this PR adds or modifies migration files too. Alembic migrations form a single linear chain, so if both branch off the same parent revision, merging this PR as-is could leave the migration history with two heads instead of one, which makes `superset db upgrade` fail with `Multiple head revisions are present`.
To check and, if needed, fix this:
1. Rebase this branch onto the latest `master`.
2. Run `superset db heads`. If it lists more than one revision, you have a conflict.
3. In your new migration's script, update `down_revision` (and the `Revises` comment) to point at the other head instead, so the chain has a single head again.
See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-6307958593
### ⚠️ Possible DB migration conflict
@rusackas the base branch `master` picked up a new database migration under `superset/migrations/` after this PR was opened, and this PR adds or modifies migration files too. Alembic migrations form a single linear chain, so if both branch off the same parent revision, merging this PR as-is could leave the migration history with two heads instead of one, which makes `superset db upgrade` fail with `Multiple head revisions are present`.
To check and, if needed, fix this:
1. Rebase this branch onto the latest `master`.
2. Run `superset db heads`. If it lists more than one revision, you have a conflict.
3. In your new migration's script, update `down_revision` (and the `Revises` comment) to point at the other head instead, so the chain has a single head again.
See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-4814274750
<!-- Bito Reply -->
The security concern regarding incomplete URL substring sanitization is not applicable to this pull request. The provided PR diff and comments focus on the implementation of a first-class iframe dashboard component and a runtime CSP allowlist mechanism, and do not contain the code or tests referenced in the security report.
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-5351357722
@sha174n good catches. Dropped `allow-same-origin` from the sandbox, no reason to pair it with allow-scripts for third-party embeds.
Also added a test that exercises the CSP merge through a real Flask response rather than a bare `Response()` object. The global/UX-only trust boundary is already called out in `SIP.md` and the docstrings, but say the word if that's not enough.
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-5706356508
# 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️❗ @rusackas Your base branch `master` has also updated `superset/migrations`.
**Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-5704370986
# 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️ 🙅♂️❗ @rusackas Your base branch `master` has also updated `superset/migrations`.
**Please consider rebasing your branch and [resolving potential db migration conflicts](https://superset.apache.org/docs/contributing/development#merging-db-migrations).**
--
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
Re: [PR] feat(dashboard): first-class iframe component with runtime CSP allowlist [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #41468:
URL: https://github.com/apache/superset/pull/41468#issuecomment-6281396087
### ⚠️ Possible DB migration conflict
@rusackas the base branch `master` picked up a new database migration under `superset/migrations/` after this PR was opened, and this PR adds or modifies migration files too. Alembic migrations form a single linear chain, so if both branch off the same parent revision, merging this PR as-is could leave the migration history with two heads instead of one, which makes `superset db upgrade` fail with `Multiple head revisions are present`.
To check and, if needed, fix this:
1. Rebase this branch onto the latest `master`.
2. Run `superset db heads`. If it lists more than one revision, you have a conflict.
3. In your new migration's script, update `down_revision` (and the `Revises` comment) to point at the other head instead, so the chain has a single head again.
See [Merging DB Migrations](https://superset.apache.org/docs/contributing/development#merging-db-migrations) in the contributing guide for a worked example.
--
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