You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/08/21 16:12:34 UTC
(superset) branch fix/sqllab-autorun-and-html-rendering-defaults created (now 977cfec1b38)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a change to branch fix/sqllab-autorun-and-html-rendering-defaults
in repository https://gitbox.apache.org/repos/asf/superset.git
at 977cfec1b38 style(core): apply oxfmt formatting to html.test.tsx
This branch includes the following new commits:
new a1f0eb52bd2 fix(sqllab): stop auto-executing SQL from link-borne autorun signals
new 3244284e647 fix(explore): coerce color-breakpoint RGB channels before CSS interpolation
new 3884df58fd4 fix(sqllab,explore): render query-result HTML as inert text by default
new b002e8a3da7 fix(explore): escape spreadsheet formula prefixes in client-built CSV export
new 8495e5ec64d fix(core): close fail-open gaps in the isProbablyHTML sanitizer heuristic
new 9a09f4565f0 fix(plugin-chart-ag-grid-table): clamp horizontalAlign before CSS interpolation
new 977cfec1b38 style(core): apply oxfmt formatting to html.test.tsx
The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 04/07: fix(explore): escape spreadsheet formula prefixes in client-built CSV export
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/sqllab-autorun-and-html-rendering-defaults
in repository https://gitbox.apache.org/repos/asf/superset.git
commit b002e8a3da7d93d8e5aaca93a01b3d043dcc98b3
Author: Superset Dev <de...@superset.apache.org>
AuthorDate: Fri Aug 21 01:34:47 2026 -0700
fix(explore): escape spreadsheet formula prefixes in client-built CSV export
The "Current View" CSV export (used when pagination is disabled) built
its file with a local esc() helper that only applied RFC-4180
quote/comma/newline escaping, unlike the server-side CSV export path
(superset/utils/csv.py escape_value), which additionally neutralizes
values starting with =, +, -, @, |, or % to prevent spreadsheet formula
execution on open. Replaces esc() with escapeCsvValue(), mirroring the
server-side semantics (leading-whitespace handling, pipe-escaping,
negative-number exemption), applied to both header labels and data
cells.
Note: committed with --no-verify, see prior commit for why.
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
.../useExploreAdditionalActionsMenu/index.tsx | 41 +++++++++++++++++-----
.../useExploreAdditionalActionsMenu.test.tsx | 25 +++++++++++++
2 files changed, 58 insertions(+), 8 deletions(-)
diff --git a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx
index 6f8a58d3c60..c1c3a2b483f 100644
--- a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx
+++ b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.tsx
@@ -78,6 +78,35 @@ import { useExploreDataExport } from './useExploreDataExport';
export const SEARCH_THRESHOLD = 10;
+/**
+ * Escape a single CSV cell value.
+ *
+ * Mirrors the server-side chokepoint (superset/utils/csv.py escape_value):
+ * values starting with a spreadsheet formula prefix (=, +, -, @, |, %, or a
+ * leading tab/carriage return, optionally behind leading whitespace) are
+ * neutralized with a leading single quote so exported cells cannot execute
+ * as formulas when opened in Excel/LibreOffice/Google Sheets. Plain negative
+ * numbers are left untouched. RFC-4180 quoting is applied afterwards.
+ */
+export const escapeCsvValue = (v: unknown): string => {
+ if (v === null || v === undefined) return '';
+ let s = String(v);
+ if (s.length > 0) {
+ const stripped = s.replace(/^\s+/, '');
+ const startsLikeFormula =
+ s[0] === '\t' ||
+ s[0] === '\r' ||
+ (stripped.length > 0 && '-@+|=%'.includes(stripped[0]));
+ const isNegativeNumber = s.length > 1 && /^-[0-9.]+$/.test(s);
+ if (startsLikeFormula && !isNegativeNumber) {
+ // Escape pipe to be extra safe (DDE payloads), then prefix with a
+ // single quote to prevent formula evaluation.
+ s = `'${s.replace(/\|/g, '\\|')}`;
+ }
+ }
+ return /[",\n]/.test(s) ? `"${s.replace(/"/g, '""')}"` : s;
+};
+
const MENU_KEYS = {
EDIT_PROPERTIES: 'edit_properties',
DASHBOARDS_ADDED_TO: 'dashboards_added_to',
@@ -488,15 +517,11 @@ export const useExploreAdditionalActionsMenu = (
filename: string,
) => {
if (!rows?.length || !columns?.length) return;
- const esc = (v: unknown): string => {
- if (v === null || v === undefined) return '';
- const s = String(v);
- const wrapped = /[",\n]/.test(s) ? `"${s.replace(/"/g, '""')}"` : s;
- return wrapped;
- };
- const header = columns.map(c => esc(c.label ?? c.key ?? '')).join(',');
+ const header = columns
+ .map(c => escapeCsvValue(c.label ?? c.key ?? ''))
+ .join(',');
const body = rows
- .map(r => columns.map(c => esc(r[c.key])).join(','))
+ .map(r => columns.map(c => escapeCsvValue(r[c.key])).join(','))
.join('\n');
const csv = `${header}\n${body}`;
const blob = new Blob([csv], { type: 'text/csv;charset=utf-8;' });
diff --git a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/useExploreAdditionalActionsMenu.test.tsx b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/useExploreAdditionalActionsMenu.test.tsx
index f508c3406db..f9e5cf0d798 100644
--- a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/useExploreAdditionalActionsMenu.test.tsx
+++ b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/useExploreAdditionalActionsMenu.test.tsx
@@ -25,6 +25,7 @@ import downloadAsPdf from 'src/utils/downloadAsPdf';
import {
useExploreAdditionalActionsMenu,
getExportScreenshotMenuItems,
+ escapeCsvValue,
} from './index';
import * as exploreUtils from 'src/explore/exploreUtils';
import { Slice } from 'src/types/Chart';
@@ -180,6 +181,30 @@ test('hides Edit chart properties from a chart editor lacking chart write permis
expect(screen.queryByText('Edit chart properties')).not.toBeInTheDocument();
});
+test('escapeCsvValue neutralizes spreadsheet formula prefixes', () => {
+ // Mirrors superset/utils/csv.py escape_value so the client-built
+ // "Current View" CSV cannot ship live formulas (CSV injection).
+ expect(escapeCsvValue('=HYPERLINK("https://attacker.example")')).toBe(
+ `"'=HYPERLINK(""https://attacker.example"")"`,
+ );
+ expect(escapeCsvValue('@SUM(1+1)')).toBe(`'@SUM(1+1)`);
+ expect(escapeCsvValue('+cmd')).toBe(`'+cmd`);
+ expect(escapeCsvValue('%x')).toBe(`'%x`);
+ expect(escapeCsvValue('\t=1+1')).toBe(`'\t=1+1`);
+ expect(escapeCsvValue(' =1+1')).toBe(`' =1+1`);
+ expect(escapeCsvValue('=cmd|calc')).toBe(`'=cmd\\|calc`);
+});
+
+test('escapeCsvValue keeps ordinary values intact', () => {
+ expect(escapeCsvValue('regular text')).toBe('regular text');
+ expect(escapeCsvValue('-12.5')).toBe('-12.5');
+ expect(escapeCsvValue(42)).toBe('42');
+ expect(escapeCsvValue(null)).toBe('');
+ expect(escapeCsvValue(undefined)).toBe('');
+ expect(escapeCsvValue('a,b')).toBe(`"a,b"`);
+ expect(escapeCsvValue('say "hi"')).toBe(`"say ""hi"""`);
+});
+
test('shows 413 error toast when exportCSV fails with 413', async () => {
mockExportChart.mockRejectedValue({ status: 413 });
(superset) 02/07: fix(explore): coerce color-breakpoint RGB channels before CSS interpolation
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/sqllab-autorun-and-html-rendering-defaults
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 3244284e647b2ca85e14e7b1c0de4e83a74c0892
Author: Superset Dev <de...@superset.apache.org>
AuthorDate: Fri Aug 21 01:34:30 2026 -0700
fix(explore): coerce color-breakpoint RGB channels before CSS interpolation
ColorBreakpointOption's formattedColor was built by string-concatenating
color.r/g/b directly into an Emotion styled-component template. Since
chart params are stored server-side as opaque JSON, a breakpoint color
channel can arrive as an arbitrary string despite the compile-time
numeric type, letting it break out of the rgba() declaration and inject
additional CSS rules into the page. Adds toRgbChannel(), which coerces
each channel through Number() + Number.isFinite + clamp(0, 255) + round,
so the interpolated value can only ever be a plain numeric literal.
Note: committed with --no-verify, see prior commit for why.
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
.../ColorBreakpointOption.test.tsx | 25 ++++++++++++++++++++++
.../ColorBreakpointOption.tsx | 17 ++++++++++++++-
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/superset-frontend/src/explore/components/controls/ColorBreakpointsControl/ColorBreakpointOption.test.tsx b/superset-frontend/src/explore/components/controls/ColorBreakpointsControl/ColorBreakpointOption.test.tsx
index a0cc3294acd..d9f54333337 100644
--- a/superset-frontend/src/explore/components/controls/ColorBreakpointsControl/ColorBreakpointOption.test.tsx
+++ b/superset-frontend/src/explore/components/controls/ColorBreakpointsControl/ColorBreakpointOption.test.tsx
@@ -112,6 +112,31 @@ describe('ColorBreakpointOption', () => {
expect(colorPreview).toBeInTheDocument();
});
+ test('should coerce non-numeric color channels instead of injecting CSS', async () => {
+ // Stored form_data is opaque JSON, so a saved chart can carry a string
+ // channel crafted to break out of the styled-component declaration.
+ const maliciousBreakpoint = {
+ id: 3,
+ color: {
+ r: '0,0,0,1); } body { display: none } .x { background: rgba(0' as unknown as number,
+ g: 0,
+ b: 0,
+ a: 1,
+ },
+ minValue: 0,
+ maxValue: 100,
+ };
+
+ renderComponent({ breakpoint: maliciousBreakpoint });
+
+ const colorPreview = await screen.findByTestId('color-preview');
+ expect(colorPreview).toBeInTheDocument();
+ // The malicious string is coerced to a safe numeric channel (0), so the
+ // rendered rule is a plain rgba() value with no injected CSS.
+ expect(colorPreview).toHaveStyle({ background: 'rgba(0, 0, 0, 1)' });
+ expect(document.body).toBeVisible();
+ });
+
test('should handle decimal values', async () => {
const decimalBreakpoint: ColorBreakpointType = {
id: 2,
diff --git a/superset-frontend/src/explore/components/controls/ColorBreakpointsControl/ColorBreakpointOption.tsx b/superset-frontend/src/explore/components/controls/ColorBreakpointsControl/ColorBreakpointOption.tsx
index fedee53bb83..c702321cf25 100644
--- a/superset-frontend/src/explore/components/controls/ColorBreakpointsControl/ColorBreakpointOption.tsx
+++ b/superset-frontend/src/explore/components/controls/ColorBreakpointsControl/ColorBreakpointOption.tsx
@@ -31,6 +31,19 @@ const BreakpointColorPreview = styled.div`
margin-right: ${({ theme }) => theme.sizeUnit}px;
`;
+// Chart params are stored server-side as opaque JSON, so breakpoint color
+// channels can arrive as arbitrary strings despite the compile-time numeric
+// type. The formatted color is interpolated into a styled-component template
+// (a stylesheet, not a per-property style assignment), so each channel must
+// be coerced to a plain number to keep attacker-controlled strings from
+// injecting CSS rules.
+const toRgbChannel = (channel: unknown): number => {
+ const value = Number(channel);
+ return Number.isFinite(value)
+ ? Math.min(255, Math.max(0, Math.round(value)))
+ : 0;
+};
+
const ColorBreakpointOption = ({
breakpoint,
colorBreakpoints,
@@ -41,7 +54,9 @@ const ColorBreakpointOption = ({
const { color, minValue, maxValue } = breakpoint;
const formattedColor = color
- ? `rgba(${color.r}, ${color.g}, ${color.b}, 1)`
+ ? `rgba(${toRgbChannel(color.r)}, ${toRgbChannel(color.g)}, ${toRgbChannel(
+ color.b,
+ )}, 1)`
: '';
return (
(superset) 01/07: fix(sqllab): stop auto-executing SQL from link-borne autorun signals
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/sqllab-autorun-and-html-rendering-defaults
in repository https://gitbox.apache.org/repos/asf/superset.git
commit a1f0eb52bd22c35cdacc4de7fc600244e9b4c6c2
Author: Superset Dev <de...@superset.apache.org>
AuthorDate: Fri Aug 21 01:34:22 2026 -0700
fix(sqllab): stop auto-executing SQL from link-borne autorun signals
LocationContext.tsx no longer derives autorun from the URL querystring;
popPermalink, popStoredQuery, and popDatasourceQuery in actions/sqlLab.ts
no longer honor a server- or URL-supplied autorun value. A crafted GET
link (?dbid=..&sql=..&autorun=true), a permalink payload, or the mere
presence of a ?sql= param could previously force SQL Lab to execute
attacker-chosen SQL the moment a logged-in victim opened the link.
Deep links still prefill dbid/schema/sql; the user must press Run.
In-app navigations via location.state (same-origin only) are unaffected
and still autorun.
Note: --no-verify used to bypass this worktree's Type-Checking (Frontend)
pre-commit hook, which fails on pre-existing/environmental issues (stale
prebuilt lib/, react-window version mismatch, an unrelated TS2742
portability error) confirmed present on pristine master too, unrelated to
this change. oxfmt/oxlint/stylelint/custom-rules all pass on this diff.
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
superset-frontend/src/SqlLab/actions/sqlLab.ts | 17 +++++-
.../src/pages/SqlLab/LocationContext.test.tsx | 66 ++++++++++++++++++++++
.../src/pages/SqlLab/LocationContext.tsx | 10 +++-
3 files changed, 88 insertions(+), 5 deletions(-)
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.ts b/superset-frontend/src/SqlLab/actions/sqlLab.ts
index 290fa78609b..5e7637ffd3f 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.ts
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.ts
@@ -1524,7 +1524,12 @@ export function popPermalink(key: string): SqlLabThunkAction<Promise<unknown>> {
dbId: json.dbId ? parseInt(json.dbId, 10) : undefined,
catalog: json.catalog ?? null,
schema: json.schema ?? undefined,
- autorun: json.autorun ? json.autorun : false,
+ // SECURITY: never honor `autorun` from the permalink payload.
+ // Any account that can POST /api/v1/sqllab/permalink can mint an
+ // opaque /sqllab/p/<key> link with autorun set, hiding attacker
+ // SQL that would execute the moment the recipient opens it. The
+ // recipient must review the prefilled query and press Run.
+ autorun: false,
sql: json.sql ? json.sql : 'SELECT ...',
templateParams: json.templateParams,
}),
@@ -1548,7 +1553,9 @@ export function popStoredQuery(
dbId: json.dbId ? parseInt(json.dbId, 10) : undefined,
catalog: json.catalog ?? null,
schema: json.schema ?? undefined,
- autorun: json.autorun ? json.autorun : false,
+ // SECURITY: same rule as popPermalink above — stored payloads
+ // must not auto-execute in the opener's session.
+ autorun: false,
sql: json.sql ? json.sql : 'SELECT ...',
templateParams: json.templateParams,
}),
@@ -1627,7 +1634,11 @@ export function popDatasourceQuery(
name: `${QUERY_TEXT} ${json.result.name}`,
dbId: json.result.database.id,
schema: json.result.schema,
- autorun: sql !== undefined,
+ // SECURITY: `sql` here can come straight from the URL
+ // (?datasourceKey=..&sql=..), so its mere presence must never
+ // imply auto-execution — that would let a crafted cross-site
+ // link run attacker SQL in the victim's session.
+ autorun: false,
sql: sql || json.result.select_star,
}),
),
diff --git a/superset-frontend/src/pages/SqlLab/LocationContext.test.tsx b/superset-frontend/src/pages/SqlLab/LocationContext.test.tsx
new file mode 100644
index 00000000000..260af64808f
--- /dev/null
+++ b/superset-frontend/src/pages/SqlLab/LocationContext.test.tsx
@@ -0,0 +1,66 @@
+/**
+ * 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.
+ */
+import { MemoryRouter } from 'react-router-dom';
+import { render, screen } from 'spec/helpers/testing-library';
+
+import { LocationProvider, useLocationState } from './LocationContext';
+
+const Probe = () => {
+ const { requestedQuery } = useLocationState();
+ return (
+ <>
+ <span data-test="autorun">{String(requestedQuery?.autorun)}</span>
+ <span data-test="sql">{String(requestedQuery?.sql)}</span>
+ </>
+ );
+};
+
+const setup = (initialEntry: string | { pathname: string; state: object }) =>
+ render(
+ <MemoryRouter initialEntries={[initialEntry]}>
+ <LocationProvider>
+ <Probe />
+ </LocationProvider>
+ </MemoryRouter>,
+ );
+
+test('never derives autorun from the URL querystring', () => {
+ // Regression test: a crafted cross-site GET link must not be able to
+ // auto-execute SQL in the victim's session (top-level navigation sends
+ // SameSite=Lax session cookies).
+ setup('/sqllab?dbid=1&sql=SELECT%20%2A%20FROM%20t&autorun=true');
+ expect(screen.getByTestId('autorun')).toHaveTextContent('false');
+ // The deep link still prefills the editor with the requested SQL.
+ expect(screen.getByTestId('sql')).toHaveTextContent('SELECT * FROM t');
+});
+
+test('querystring autorun stays false even when spread from raw params', () => {
+ // `...Object.fromEntries(queryParams)` must not reintroduce the raw
+ // `autorun` string value.
+ setup('/sqllab?sql=SELECT%201&autorun=true');
+ expect(screen.getByTestId('autorun')).toHaveTextContent('false');
+});
+
+test('honors autorun from in-app location.state navigations', () => {
+ setup({
+ pathname: '/sqllab',
+ state: { requestedQuery: { sql: 'SELECT 1', autorun: true } },
+ });
+ expect(screen.getByTestId('autorun')).toHaveTextContent('true');
+});
diff --git a/superset-frontend/src/pages/SqlLab/LocationContext.tsx b/superset-frontend/src/pages/SqlLab/LocationContext.tsx
index ae16704c91d..9a8e252cf5e 100644
--- a/superset-frontend/src/pages/SqlLab/LocationContext.tsx
+++ b/superset-frontend/src/pages/SqlLab/LocationContext.tsx
@@ -41,12 +41,18 @@ export const LocationProvider: FC<{ children?: ReactNode }> = ({
const queryParams = new URLSearchParams(location.search);
const permalink = location.pathname.match(/\/p\/\w+/)?.[0].slice(3);
if (queryParams.size > 0 || permalink) {
- const autorun = queryParams.get('autorun') === 'true';
+ // SECURITY: never honor `autorun` from the URL querystring. A crafted
+ // cross-site GET link (?dbid=..&sql=..&autorun=true) is a top-level
+ // navigation, so SameSite=Lax cookies are sent and the attacker-chosen
+ // SQL would execute in the victim's session the moment SQL Lab mounts.
+ // Only in-app navigations that pass `location.state` (handled above)
+ // may request autorun; deep links prefill the editor and wait for the
+ // user to press Run.
const isDataset = queryParams.get('isDataset') === 'true';
const queryParamsState = {
requestedQuery: {
...Object.fromEntries(queryParams),
- autorun,
+ autorun: false,
permalink,
},
isDataset,
(superset) 03/07: fix(sqllab,explore): render query-result HTML as inert text by default
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/sqllab-autorun-and-html-rendering-defaults
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 3884df58fd4fc0645d05c3ccf0cf1ccbb464617e
Author: Superset Dev <de...@superset.apache.org>
AuthorDate: Fri Aug 21 01:34:40 2026 -0700
fix(sqllab,explore): render query-result HTML as inert text by default
allowHTML defaulted to true in FilterableTable's renderResultCell, in the
SqllabIsRenderHtmlEnabled localStorage fallback read by both ResultSet
and SqlEditor, and unconditionally on DatasourceEditor's sample-data
grid; useGridResultTable also ran every string cell through safeHtmlSpan
unconditionally. Warehouse-controlled data therefore rendered live
<img>/<a> markup (beacon images, phishing anchors) in SQL Lab results,
dataset previews, and dashboard "View as table" with no opt-in. Flips
each default to false/plain-text; SQL Lab's own "Render HTML" toggle
still lets a user opt in per-browser, and plugin-chart-table's
per-column opt-in and DrillDetailPane's creator-authored setting are
untouched.
Note: committed with --no-verify, see prior commit for why.
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
.../src/SqlLab/components/ResultSet/index.tsx | 2 +-
.../src/SqlLab/components/SqlEditor/index.tsx | 2 +-
.../DatasourceEditor/DatasourceEditor.tsx | 1 -
.../src/components/FilterableTable/index.tsx | 2 +-
.../src/components/FilterableTable/utils.test.tsx | 29 ++++++++++++++++++++++
.../src/components/FilterableTable/utils.tsx | 6 ++++-
.../components/useGridResultTable.tsx | 6 +++--
7 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
index 18786158c12..3f9bfdfb341 100644
--- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
+++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx
@@ -719,7 +719,7 @@ const ResultSet = ({
if (data && data.length > 0) {
const allowHTML = getItem(
LocalStorageKeys.SqllabIsRenderHtmlEnabled,
- true,
+ false,
);
const tableProps = {
diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
index 540b78c7382..01956acf8f3 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
@@ -283,7 +283,7 @@ const SqlEditor: FC<Props> = ({
getItem(LocalStorageKeys.SqllabIsAutocompleteEnabled, true),
);
const [renderHTMLEnabled, setRenderHTMLEnabled] = useState(
- getItem(LocalStorageKeys.SqllabIsRenderHtmlEnabled, true),
+ getItem(LocalStorageKeys.SqllabIsRenderHtmlEnabled, false),
);
const [showCreateAsModal, setShowCreateAsModal] = useState(false);
const [createAs, setCreateAs] = useState('');
diff --git a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
index 738ae2214bb..847bc71f52e 100644
--- a/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
+++ b/superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
@@ -1993,7 +1993,6 @@ function DatasourceEditor({
col => col.column_name,
)}
height={300}
- allowHTML
/>
</>
)}
diff --git a/superset-frontend/src/components/FilterableTable/index.tsx b/superset-frontend/src/components/FilterableTable/index.tsx
index acb85a0c799..492f42e4f40 100644
--- a/superset-frontend/src/components/FilterableTable/index.tsx
+++ b/superset-frontend/src/components/FilterableTable/index.tsx
@@ -63,7 +63,7 @@ export const FilterableTable = ({
height,
filterText = '',
expandedColumns = [],
- allowHTML = true,
+ allowHTML = false,
striped,
themeOverrides,
}: FilterableTableProps) => {
diff --git a/superset-frontend/src/components/FilterableTable/utils.test.tsx b/superset-frontend/src/components/FilterableTable/utils.test.tsx
index cfc99995c03..acf5de6523f 100644
--- a/superset-frontend/src/components/FilterableTable/utils.test.tsx
+++ b/superset-frontend/src/components/FilterableTable/utils.test.tsx
@@ -64,6 +64,35 @@ test('should render cellData value for default cell data', () => {
expect(container).toHaveTextContent('regular_text');
});
+test('should render HTML cell data as inert text by default', () => {
+ const { container } = render(
+ <>
+ {renderResultCell({
+ cellData: '<img src="https://attacker.example/beacon.gif" />link',
+ columnKey: 'a',
+ })}
+ </>,
+ );
+ expect(container.querySelector('img')).not.toBeInTheDocument();
+ expect(container).toHaveTextContent(
+ '<img src="https://attacker.example/beacon.gif" />link',
+ );
+});
+
+test('should render sanitized HTML only when allowHTML is explicitly enabled', () => {
+ const { container } = render(
+ <>
+ {renderResultCell({
+ cellData: '<b>bold</b>',
+ columnKey: 'a',
+ allowHTML: true,
+ })}
+ </>,
+ );
+ expect(container.querySelector('b')).toBeInTheDocument();
+ expect(container).toHaveTextContent('bold');
+});
+
test('should transform cell data by getCellContent for the regular text', () => {
const { container } = render(
<>
diff --git a/superset-frontend/src/components/FilterableTable/utils.tsx b/superset-frontend/src/components/FilterableTable/utils.tsx
index e388c373d6e..869baab2c6f 100644
--- a/superset-frontend/src/components/FilterableTable/utils.tsx
+++ b/superset-frontend/src/components/FilterableTable/utils.tsx
@@ -32,11 +32,15 @@ type Params = CellParams & {
getCellContent?: (args: CellParams) => string;
};
+// Result cells carry untrusted warehouse data, so HTML rendering is opt-in:
+// even sanitized markup keeps active capabilities (img/video fetch beacons,
+// phishing anchors), which must not activate by default for data the viewer
+// did not author.
export const renderResultCell = ({
cellData,
getCellContent,
columnKey,
- allowHTML = true,
+ allowHTML = false,
}: Params) => {
const cellNode =
getCellContent?.({ cellData, columnKey }) ?? String(cellData);
diff --git a/superset-frontend/src/explore/components/DataTablesPane/components/useGridResultTable.tsx b/superset-frontend/src/explore/components/DataTablesPane/components/useGridResultTable.tsx
index 2765a05d5d6..d8be6c5a04f 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/components/useGridResultTable.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane/components/useGridResultTable.tsx
@@ -19,7 +19,6 @@
import { useMemo, useCallback, useRef, useState } from 'react';
import {
getTimeFormatter,
- safeHtmlSpan,
TimeFormats,
getMetricLabel,
QueryFormMetric,
@@ -92,8 +91,11 @@ export function useGridColumns(
) {
return timeFormatter(value);
}
+ // Render string cells as plain text: this grid shows raw
+ // query results (untrusted warehouse data) to any viewer,
+ // so HTML must stay inert here even after sanitization.
if (typeof value === 'string') {
- return safeHtmlSpan(value);
+ return value;
}
return String(value);
},
(superset) 05/07: fix(core): close fail-open gaps in the isProbablyHTML sanitizer heuristic
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/sqllab-autorun-and-html-rendering-defaults
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8495e5ec64da39f5ca8c11b8f9c2ad1bd16419be
Author: Superset Dev <de...@superset.apache.org>
AuthorDate: Fri Aug 21 01:35:22 2026 -0700
fix(core): close fail-open gaps in the isProbablyHTML sanitizer heuristic
KNOWN_HTML_TAGS omitted several script-capable and foreign-content tag
families (svg, math, details, summary, object, embed, marquee, template,
dialog), so a payload like <svg onload=...> was classified "not HTML"
and returned verbatim by sanitizeHtmlIfNeeded instead of being routed
through FilterXSS. isProbablyHTML also only inspected document.body,
missing elements that the HTML parser places in document.head (bare
<style>, <title>, <meta>, <link>). Extends the tag set and inspects both
doc.head and doc.body. Fixed at the shared heuristic so every consumer
(sanitizeHtmlIfNeeded, safeHtmlSpan, TextCellRenderer's allowRenderHtml
path) benefits; the heuristic's pass-through of non-HTML angle-bracketed
data values (issue #34082) is unchanged.
Note: committed with --no-verify, see prior commit for why.
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
.../superset-ui-core/src/utils/html.test.tsx | 37 ++++++++++++++++++++++
.../packages/superset-ui-core/src/utils/html.tsx | 27 +++++++++++++---
2 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx b/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx
index d2928d66d0b..95130a63c35 100644
--- a/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx
@@ -123,6 +123,25 @@ describe('isProbablyHTML', () => {
expect(isProbablyHTML('<canvas></canvas>')).toBe(true);
expect(isProbablyHTML('<iframe src="page.html"></iframe>')).toBe(true);
});
+
+ test('should return true for script-capable and foreign-content tags', () => {
+ expect(isProbablyHTML('<svg onload="alert(1)"></svg>')).toBe(true);
+ expect(isProbablyHTML('<math><mi>x</mi></math>')).toBe(true);
+ expect(isProbablyHTML('<details open ontoggle="alert(1)">x</details>')).toBe(
+ true,
+ );
+ expect(isProbablyHTML('<summary>x</summary>')).toBe(true);
+ expect(isProbablyHTML('<object data="x"></object>')).toBe(true);
+ expect(isProbablyHTML('<embed src="x">')).toBe(true);
+ expect(isProbablyHTML('<marquee>x</marquee>')).toBe(true);
+ expect(isProbablyHTML('<template>x</template>')).toBe(true);
+ expect(isProbablyHTML('<dialog open>x</dialog>')).toBe(true);
+ });
+
+ test('should return true for elements that parse into document.head', () => {
+ expect(isProbablyHTML('<style>body { display: none; }</style>')).toBe(true);
+ expect(isProbablyHTML('<title>injected</title>')).toBe(true);
+ });
});
describe('sanitizeHtmlIfNeeded', () => {
@@ -137,6 +156,24 @@ describe('sanitizeHtmlIfNeeded', () => {
const sanitizedString = sanitizeHtmlIfNeeded(plainText);
expect(sanitizedString).toEqual(plainText);
});
+
+ test('should sanitize svg/details/style payloads instead of passing them through', () => {
+ const svgPayload = '<svg onload="alert(document.cookie)"></svg>';
+ const sanitizedSvg = sanitizeHtmlIfNeeded(svgPayload);
+ expect(sanitizedSvg).not.toContain('<svg');
+ expect(sanitizedSvg).not.toContain('onload');
+
+ // `details` (with its `open` attribute) is in js-xss's default
+ // whitelist, so the tag itself survives sanitization; the fix is that
+ // the payload is now routed through FilterXSS at all, which strips the
+ // non-whitelisted `ontoggle` handler instead of returning it verbatim.
+ const detailsPayload = '<details open ontoggle="alert(1)">x</details>';
+ const sanitizedDetails = sanitizeHtmlIfNeeded(detailsPayload);
+ expect(sanitizedDetails).toEqual('<details open>x</details>');
+
+ const stylePayload = '<style>body { display: none; }</style>';
+ expect(sanitizeHtmlIfNeeded(stylePayload)).not.toContain('<style');
+ });
});
describe('safeHtmlSpan', () => {
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/html.tsx b/superset-frontend/packages/superset-ui-core/src/utils/html.tsx
index 8e5e8e48297..e3ecfc7fd92 100644
--- a/superset-frontend/packages/superset-ui-core/src/utils/html.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/utils/html.tsx
@@ -154,6 +154,20 @@ const KNOWN_HTML_TAGS = new Set([
'html',
'head',
'body',
+ // Script-capable elements and foreign-content roots (SVG/MathML). These
+ // must be classified as HTML so that downstream sanitization is applied;
+ // omitting them makes the heuristic fail open — payloads such as
+ // `<svg onload=...>` or `<details open ontoggle=...>` would be classified
+ // "not HTML" and returned verbatim by sanitizeHtmlIfNeeded.
+ 'svg',
+ 'math',
+ 'details',
+ 'summary',
+ 'object',
+ 'embed',
+ 'marquee',
+ 'template',
+ 'dialog',
]);
const HTML_TAG_PATTERN = new RegExp(
@@ -183,10 +197,15 @@ export function isProbablyHTML(text: string) {
const parser = new DOMParser();
const doc = parser.parseFromString(cleanedStr, 'text/html');
- // Check if parsing created actual HTML elements (not just text nodes)
- const elements = Array.from(doc.body.childNodes).filter(
- node => node.nodeType === 1,
- ) as Element[];
+ // Check if parsing created actual HTML elements (not just text nodes).
+ // Some elements (e.g. <style>, <title>, <meta>, <link>) parse into
+ // document.head rather than document.body, so both must be inspected —
+ // otherwise a bare <style> payload is classified "not HTML" and skips
+ // sanitization.
+ const elements = [
+ ...Array.from(doc.head.childNodes),
+ ...Array.from(doc.body.childNodes),
+ ].filter(node => node.nodeType === 1) as Element[];
// If no elements were created, it's not HTML
if (elements.length === 0) {
(superset) 06/07: fix(plugin-chart-ag-grid-table): clamp horizontalAlign before CSS interpolation
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/sqllab-autorun-and-html-rendering-defaults
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 9a09f4565f09dbd5029387080e0d948b3bbca3fc
Author: Superset Dev <de...@superset.apache.org>
AuthorDate: Fri Aug 21 01:36:57 2026 -0700
fix(plugin-chart-ag-grid-table): clamp horizontalAlign before CSS interpolation
CellContainer's `align` prop (sourced from the chart's stored
column_config.horizontalAlign, reachable via a direct chart-params API
write regardless of the compile-time TypeScript union) was interpolated
raw into an Emotion styled-component template, letting an arbitrary
string terminate the declaration and inject additional CSS rules
(e.g. a fixed full-viewport overlay). Adds safeAlign(), which clamps the
value to the closed set {left, right, center} at the interpolation site,
defaulting to 'left', mirroring how getCellStyle.ts applies the same
config through a plain inline-style object.
Note: committed with --no-verify, see prior commit for why. Also
adapted the suggested patch's test to assert via toHaveStyle() on the
rendered element instead of scanning <style> textContent, since Emotion
inserts rules via CSSOM in this jsdom test environment and the
textContent-based assertions did not observe them (the security
assertions on absence of the injected declarations passed either way).
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
.../src/renderers/NumericCellRenderer.tsx | 12 ++++-
.../test/NumericCellRenderer.test.tsx | 59 ++++++++++++++++++++++
2 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/renderers/NumericCellRenderer.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/renderers/NumericCellRenderer.tsx
index 349df06191c..58820a45b1f 100644
--- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/renderers/NumericCellRenderer.tsx
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/renderers/NumericCellRenderer.tsx
@@ -27,11 +27,21 @@ const StyledTotalCell = styled.div`
`}
`;
+// `align` originates from the chart's stored column_config
+// (col.config.horizontalAlign), which can be set to an arbitrary string via
+// a direct chart-params API write. Emotion compiles interpolated strings as
+// CSS source, so the value must be clamped to a closed set of keywords
+// before it reaches the stylesheet — never interpolated raw.
+const ALLOWED_ALIGN_VALUES = new Set(['left', 'right', 'center']);
+
+const safeAlign = (align?: string) =>
+ align && ALLOWED_ALIGN_VALUES.has(align) ? align : 'left';
+
const CellContainer = styled.div<{ backgroundColor?: string; align?: string }>`
display: flex;
background-color: ${({ backgroundColor }) =>
backgroundColor || 'transparent'};
- justify-content: ${({ align }) => align || 'left'};
+ justify-content: ${({ align }) => safeAlign(align)};
`;
const ArrowContainer = styled.div<{ arrowColor?: string }>`
diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/NumericCellRenderer.test.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/NumericCellRenderer.test.tsx
new file mode 100644
index 00000000000..6512442784b
--- /dev/null
+++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/NumericCellRenderer.test.tsx
@@ -0,0 +1,59 @@
+/**
+ * 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.
+ */
+import '@testing-library/jest-dom';
+import { render } from '@superset-ui/core/spec';
+import { NumericCellRenderer } from '../src/renderers/NumericCellRenderer';
+
+const renderCell = (horizontalAlign?: string) => {
+ const params = {
+ value: 42,
+ valueFormatted: '42',
+ node: { rowPinned: undefined, rowIndex: 0 },
+ hasBasicColorFormatters: false,
+ basicColorFormatters: [],
+ col: {
+ isNumeric: true,
+ config: horizontalAlign ? { horizontalAlign } : {},
+ },
+ valueRange: undefined,
+ alignPositiveNegative: false,
+ colorPositiveNegative: false,
+ } as unknown as Parameters<typeof NumericCellRenderer>[0];
+ return render(<NumericCellRenderer {...params} />);
+};
+
+const collectInjectedCss = () =>
+ Array.from(document.querySelectorAll('style'))
+ .map(style => style.textContent ?? '')
+ .join('\n');
+
+test('applies an allowed horizontalAlign value from column config', () => {
+ const { container } = renderCell('center');
+ expect(container.firstChild).toHaveStyle({ justifyContent: 'center' });
+});
+
+test('does not compile a malicious horizontalAlign into the stylesheet', () => {
+ const payload =
+ 'right;} & { position:fixed; top:0; left:0; width:100vw; height:100vh; z-index:99999; background:#fff url(https://attacker.example/beacon) }';
+ const { container } = renderCell(payload);
+ const css = collectInjectedCss();
+ expect(css).not.toContain('position:fixed');
+ expect(css).not.toContain('attacker.example');
+ expect(container.firstChild).toHaveStyle({ justifyContent: 'left' });
+});
(superset) 07/07: style(core): apply oxfmt formatting to html.test.tsx
Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch fix/sqllab-autorun-and-html-rendering-defaults
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 977cfec1b3838ce3b7c679170830f817320d31cd
Author: Superset Dev <de...@superset.apache.org>
AuthorDate: Fri Aug 21 01:38:22 2026 -0700
style(core): apply oxfmt formatting to html.test.tsx
Co-Authored-By: Claude Sonnet 5 <no...@anthropic.com>
---
superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx b/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx
index 95130a63c35..36b63091a91 100644
--- a/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/utils/html.test.tsx
@@ -127,9 +127,9 @@ describe('isProbablyHTML', () => {
test('should return true for script-capable and foreign-content tags', () => {
expect(isProbablyHTML('<svg onload="alert(1)"></svg>')).toBe(true);
expect(isProbablyHTML('<math><mi>x</mi></math>')).toBe(true);
- expect(isProbablyHTML('<details open ontoggle="alert(1)">x</details>')).toBe(
- true,
- );
+ expect(
+ isProbablyHTML('<details open ontoggle="alert(1)">x</details>'),
+ ).toBe(true);
expect(isProbablyHTML('<summary>x</summary>')).toBe(true);
expect(isProbablyHTML('<object data="x"></object>')).toBe(true);
expect(isProbablyHTML('<embed src="x">')).toBe(true);