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 19:37:39 UTC
(superset) branch master updated: fix(export): escape formula-triggering values consistently and bound post-processing inputs (#43396)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 9781254363f fix(export): escape formula-triggering values consistently and bound post-processing inputs (#43396)
9781254363f is described below
commit 9781254363f80c87df4892dd4140e9a141040d5c
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Fri Aug 21 12:37:33 2026 -0700
fix(export): escape formula-triggering values consistently and bound post-processing inputs (#43396)
Co-authored-by: Superset Dev <de...@superset.apache.org>
---
superset/commands/streaming_export/base.py | 34 +++++++++++++----
superset/utils/excel.py | 32 +++++++++++-----
superset/utils/pandas_postprocessing/histogram.py | 21 +++++++++++
superset/utils/pandas_postprocessing/prophet.py | 15 ++++++++
superset/utils/pandas_postprocessing/resample.py | 32 ++++++++++++++++
superset/utils/pandas_postprocessing/rolling.py | 15 ++++++++
.../chart/streaming_export_command_test.py | 36 ++++++++++++++++++
.../pandas_postprocessing/test_histogram.py | 25 ++++++++++++
.../pandas_postprocessing/test_prophet.py | 18 +++++++++
.../pandas_postprocessing/test_resample.py | 39 +++++++++++++++++++
.../pandas_postprocessing/test_rolling.py | 17 +++++++++
tests/unit_tests/utils/excel_tests.py | 44 ++++++++++++++++++++++
12 files changed, 312 insertions(+), 16 deletions(-)
diff --git a/superset/commands/streaming_export/base.py b/superset/commands/streaming_export/base.py
index 9bf58aec9ff..21ae0216cdf 100644
--- a/superset/commands/streaming_export/base.py
+++ b/superset/commands/streaming_export/base.py
@@ -33,6 +33,7 @@ from sqlalchemy import text
from superset import db
from superset.commands.base import BaseCommand
+from superset.utils.csv import escape_value
logger = logging.getLogger(__name__)
@@ -110,7 +111,15 @@ class BaseStreamingCSVExportCommand(BaseCommand):
self, columns: list[str], csv_writer: Any, buffer: io.StringIO
) -> tuple[str, int]:
"""Write CSV header and return header data with byte count."""
- csv_writer.writerow(columns)
+ # Mirror the non-streaming export path (df_to_escaped_csv): header
+ # cells can carry attacker-influenced labels, so neutralize
+ # spreadsheet formula prefixes here too.
+ csv_writer.writerow(
+ [
+ escape_value(column) if isinstance(column, str) else column
+ for column in columns
+ ]
+ )
header_data = buffer.getvalue()
total_bytes = len(header_data.encode("utf-8"))
buffer.seek(0)
@@ -121,7 +130,8 @@ class BaseStreamingCSVExportCommand(BaseCommand):
self, row: tuple[Any, ...], decimal_separator: str | None
) -> list[Any]:
"""
- Format row values, applying custom decimal separator if specified.
+ Format row values: escape string cells against CSV formula injection
+ and apply the custom decimal separator if specified.
Args:
row: Database row as a tuple
@@ -130,20 +140,30 @@ class BaseStreamingCSVExportCommand(BaseCommand):
Returns:
List of formatted values
"""
- if not decimal_separator or decimal_separator == ".":
- return list(row)
+ active_decimal_separator = (
+ decimal_separator
+ if decimal_separator and decimal_separator != "."
+ else None
+ )
formatted: list[Any] = []
for value in row:
+ # Escape string cells so spreadsheet formula prefixes (= + - @ |,
+ # leading tab/CR) are neutralized, mirroring the non-streaming
+ # CSV path (superset.utils.csv.df_to_escaped_csv).
+ if isinstance(value, str):
+ formatted.append(escape_value(value))
# Apply the custom decimal separator to any real numeric value
# (float, decimal.Decimal, numpy numeric types, ...). Booleans are
# technically a numeric type in Python but should never be rewritten
# as numbers in CSV output.
- if isinstance(value, bool):
+ elif isinstance(value, bool):
formatted.append(value)
- elif isinstance(value, (float, Decimal, Real)):
+ elif active_decimal_separator is not None and isinstance(
+ value, (float, Decimal, Real)
+ ):
# Format numeric values with custom decimal separator
- formatted.append(str(value).replace(".", decimal_separator))
+ formatted.append(str(value).replace(".", active_decimal_separator))
else:
formatted.append(value)
return formatted
diff --git a/superset/utils/excel.py b/superset/utils/excel.py
index 1a252de8fe5..b370c5bb2fd 100644
--- a/superset/utils/excel.py
+++ b/superset/utils/excel.py
@@ -47,20 +47,34 @@ NEUTRAL_DOCUMENT_PROPERTIES: dict[str, Any] = {
FORMULA_PREFIXES = {"=", "+", "-", "@"}
+def _quote_formula(value: Any) -> Any:
+ """Prefix a string with a quote when it would parse as a formula."""
+ return (
+ f"'{value}"
+ if isinstance(value, str) and len(value) and value[0] in FORMULA_PREFIXES
+ else value
+ )
+
+
def quote_formulas(df: pd.DataFrame) -> pd.DataFrame:
"""
Make sure to quote any formulas for security reasons.
"""
for col in df.select_dtypes(include="object").columns:
- df[col] = df[col].apply(
- lambda x: (
- f"'{x}"
- if isinstance(x, str) and len(x) and x[0] in FORMULA_PREFIXES
- else x
- )
- )
-
- return df
+ df[col] = df[col].apply(_quote_formula)
+
+ # Column headers and index labels are written to the sheet as well, and
+ # pivot exports promote data values into both (a hostile warehouse string
+ # can become a header or row label), so quote them like the CSV writer
+ # quotes its headers. ``rename`` applies the mapper to every level of a
+ # MultiIndex.
+ df = df.rename(columns=_quote_formula, index=_quote_formula)
+
+ # ``rename`` above only touches axis *labels*. The axis *names* (e.g. a
+ # pivoted group-by column promoted to ``df.index.name``, or per-level
+ # names on a MultiIndex) are a separate attribute that pandas still
+ # writes into the sheet as header cells, so quote those too.
+ return df.rename_axis(index=_quote_formula, columns=_quote_formula)
def df_to_excel(df: pd.DataFrame, **kwargs: Any) -> Any:
diff --git a/superset/utils/pandas_postprocessing/histogram.py b/superset/utils/pandas_postprocessing/histogram.py
index 835448115bb..28375207ae6 100644
--- a/superset/utils/pandas_postprocessing/histogram.py
+++ b/superset/utils/pandas_postprocessing/histogram.py
@@ -17,8 +17,17 @@
from __future__ import annotations
import numpy as np
+from flask_babel import gettext as _
from pandas import DataFrame, Series, to_numeric
+from superset.exceptions import InvalidPostProcessingError
+
+# Upper bound on the number of histogram bins. ``bins`` arrives through the
+# post-processing ``options`` dict, which is not schema-validated, so the cap
+# must be enforced here: numpy allocates a bin-edge array proportional to
+# ``bins`` (e.g. bins=2e9 attempts a ~16 GB allocation in a single request).
+MAX_HISTOGRAM_BINS = 1000
+
# pylint: disable=too-many-arguments
def histogram(
@@ -45,6 +54,18 @@ def histogram(
and each column corresponds to a histogram bin. The values are the counts in each bin.
""" # noqa: E501
+ if (
+ not isinstance(bins, int)
+ or isinstance(bins, bool)
+ or not 1 <= bins <= MAX_HISTOGRAM_BINS
+ ):
+ raise InvalidPostProcessingError(
+ _(
+ "`bins` must be an integer between 1 and %(max)s",
+ max=MAX_HISTOGRAM_BINS,
+ )
+ )
+
if groupby is None:
groupby = []
diff --git a/superset/utils/pandas_postprocessing/prophet.py b/superset/utils/pandas_postprocessing/prophet.py
index 749d40f78c1..474947b8556 100644
--- a/superset/utils/pandas_postprocessing/prophet.py
+++ b/superset/utils/pandas_postprocessing/prophet.py
@@ -136,6 +136,21 @@ def prophet( # pylint: disable=too-many-arguments # noqa: C901
# union types
if not isinstance(periods, int) or periods < 0:
raise InvalidPostProcessingError(_("Periods must be a whole number"))
+ # The schema-declared upper bound is documentation-only for the raw
+ # post-processing ``options`` dict, so enforce it here: every forecast
+ # period adds a future row per series, making unbounded values an
+ # allocation amplifier. Imported locally to avoid a circular import
+ # (charts.schemas imports this package at module load).
+ # pylint: disable=import-outside-toplevel
+ from superset.charts.schemas import get_max_prophet_periods
+
+ if periods > (max_periods := get_max_prophet_periods()):
+ raise InvalidPostProcessingError(
+ _(
+ "Periods must not exceed %(max)s",
+ max=max_periods,
+ )
+ )
if not confidence_interval or confidence_interval <= 0 or confidence_interval >= 1:
raise InvalidPostProcessingError(
_("Confidence interval must be between 0 and 1 (exclusive)")
diff --git a/superset/utils/pandas_postprocessing/resample.py b/superset/utils/pandas_postprocessing/resample.py
index d394ce0f138..02fc9710f72 100644
--- a/superset/utils/pandas_postprocessing/resample.py
+++ b/superset/utils/pandas_postprocessing/resample.py
@@ -22,6 +22,12 @@ from flask_babel import gettext as _
from superset.exceptions import InvalidPostProcessingError
from superset.utils.pandas_postprocessing.utils import RESAMPLE_METHOD
+# Upper bound on the number of rows a resample may project. ``rule`` arrives
+# through the post-processing ``options`` dict, which is not schema-validated;
+# without a cap, upsampling a multi-day span to e.g. ``1ns`` projects ~1e14
+# rows from a single request.
+MAX_RESAMPLE_ROWS = 1_000_000
+
def resample(
df: pd.DataFrame,
@@ -46,6 +52,32 @@ def resample(
_("Resample method should be in ") + ", ".join(RESAMPLE_METHOD) + "."
)
+ if len(df):
+ try:
+ step = pd.Timedelta(pd.tseries.frequencies.to_offset(rule))
+ except ValueError:
+ # Non-fixed frequencies (month, quarter, year) have no fixed
+ # Timedelta; their projected row count is bounded by the span in
+ # days and needs no cap. Invalid rules fail in ``df.resample``.
+ step = None
+ if step is not None and step.value > 0:
+ span = df.index.max() - df.index.min()
+ # pandas snaps the first resample bin to the nearest frequency
+ # multiple at or before the observed span (and may extend the
+ # last bin similarly), so the actual bin count can exceed a
+ # naive span/step projection by one. Add a margin so the check
+ # cannot under-count due to that alignment.
+ projected_rows = span.value // step.value + 2
+ if projected_rows > MAX_RESAMPLE_ROWS:
+ raise InvalidPostProcessingError(
+ _(
+ "Resample rule would project %(rows)s rows, "
+ "exceeding the limit of %(max)s rows",
+ rows=projected_rows,
+ max=MAX_RESAMPLE_ROWS,
+ )
+ )
+
if method == "asfreq" and fill_value is not None:
_df = df.resample(rule).asfreq(fill_value=fill_value)
_df = _df.fillna(fill_value)
diff --git a/superset/utils/pandas_postprocessing/rolling.py b/superset/utils/pandas_postprocessing/rolling.py
index 775acc7aa6d..0b314d65b26 100644
--- a/superset/utils/pandas_postprocessing/rolling.py
+++ b/superset/utils/pandas_postprocessing/rolling.py
@@ -26,6 +26,12 @@ from superset.utils.pandas_postprocessing.utils import (
validate_column_args,
)
+# Upper bound on integer window sizes, matching the documented schema range
+# (1-10000). ``window`` arrives through the post-processing ``options`` dict,
+# which is not schema-validated; a huge integer window combined with
+# ``win_type`` makes scipy allocate a weights array proportional to it.
+MAX_ROLLING_WINDOW = 10_000
+
@validate_column_args("columns")
def rolling( # pylint: disable=too-many-arguments
@@ -67,6 +73,15 @@ def rolling( # pylint: disable=too-many-arguments
raise InvalidPostProcessingError(_("Undefined window for rolling operation"))
if window == 0:
raise InvalidPostProcessingError(_("Window must be > 0"))
+ # Offset-string windows (e.g. "2D") are bounded by the data span and are
+ # left to pandas to validate; only integer windows are capped.
+ if isinstance(window, int) and window > MAX_ROLLING_WINDOW:
+ raise InvalidPostProcessingError(
+ _(
+ "Window must not exceed %(max)s",
+ max=MAX_ROLLING_WINDOW,
+ )
+ )
kwargs["window"] = window
if min_periods is not None:
diff --git a/tests/unit_tests/commands/chart/streaming_export_command_test.py b/tests/unit_tests/commands/chart/streaming_export_command_test.py
index 5f9246eea53..5d354e557b3 100644
--- a/tests/unit_tests/commands/chart/streaming_export_command_test.py
+++ b/tests/unit_tests/commands/chart/streaming_export_command_test.py
@@ -168,6 +168,42 @@ def test_csv_generation_with_special_characters(mocker: MockerFixture) -> None:
assert '"Comma,Value"' in csv_data
+def test_csv_generation_escapes_formula_prefixes(mocker: MockerFixture) -> None:
+ """Streaming CSV must escape formula prefixes like the batch export path."""
+ mock_db, query_context, datasource = _setup_chart_mocks(mocker)
+
+ mock_result = mocker.MagicMock()
+ mock_result.keys.return_value = ["=header", "name"]
+ mock_result.fetchmany.side_effect = [
+ [("=cmd|' /C calc'!A0", "safe"), ("+SUM(A1:A2)", "@evil")],
+ [],
+ ]
+
+ mock_connection = mocker.MagicMock()
+ mock_connection.execution_options.return_value.execute.return_value = mock_result
+ mock_connection.__enter__.return_value = mock_connection
+ mock_connection.__exit__.return_value = None
+
+ mock_engine = mocker.MagicMock()
+ mock_engine.connect.return_value = mock_connection
+ datasource.database.get_sqla_engine.return_value.__enter__.return_value = (
+ mock_engine
+ )
+
+ command = StreamingCSVExportCommand(query_context, chunk_size=10)
+ csv_generator_callable = command.run()
+ csv_data = "".join(csv_generator_callable())
+
+ lines = [line.strip() for line in csv_data.strip().split("\n")]
+ # Header cell is escaped too
+ assert lines[0] == "'=header,name"
+ # Formula-prefixed cells are neutralized ('-prefixed, pipe escaped)
+ assert "'=cmd\\|' /C calc'!A0" in csv_data
+ assert "\n=cmd" not in csv_data
+ assert "'+SUM(A1:A2)" in csv_data
+ assert "'@evil" in csv_data
+
+
def test_streaming_with_null_values(mocker: MockerFixture) -> None:
"""Test CSV generation handles NULL values correctly."""
mock_db, query_context, datasource = _setup_chart_mocks(mocker)
diff --git a/tests/unit_tests/pandas_postprocessing/test_histogram.py b/tests/unit_tests/pandas_postprocessing/test_histogram.py
index 68ffdb0b773..bfc702a71bb 100644
--- a/tests/unit_tests/pandas_postprocessing/test_histogram.py
+++ b/tests/unit_tests/pandas_postprocessing/test_histogram.py
@@ -14,8 +14,10 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+import pytest
from pandas import DataFrame
+from superset.exceptions import InvalidPostProcessingError
from superset.utils.pandas_postprocessing import histogram
data = DataFrame(
@@ -207,3 +209,26 @@ def test_histogram_with_no_groupby_and_all_null_values():
result = histogram(data_with_no_groupby_and_all_nulls, "a", [], bins)
assert result.empty
+
+
+def test_histogram_rejects_unbounded_bins():
+ """
+ ``bins`` comes from the unvalidated post-processing options dict; an
+ unbounded value would make numpy allocate a bin-edge array of that size
+ (bins=2e9 attempts ~16 GB). Non-integer values such as "auto" are also
+ rejected, since data-driven bin estimation is likewise unbounded.
+ """
+ for bad_bins in (0, -1, 2_000_000_000, "auto", None):
+ with pytest.raises(InvalidPostProcessingError):
+ histogram(data, "a", [], bad_bins)
+
+
+def test_histogram_rejects_bool_bins():
+ """
+ ``bool`` is a subclass of ``int`` in Python, so ``isinstance(bins, int)``
+ alone accepts ``True``/``False``. Both must still be rejected since
+ neither is a valid bin count.
+ """
+ for bad_bins in (True, False):
+ with pytest.raises(InvalidPostProcessingError):
+ histogram(data, "a", [], bad_bins)
diff --git a/tests/unit_tests/pandas_postprocessing/test_prophet.py b/tests/unit_tests/pandas_postprocessing/test_prophet.py
index b6f6a97b67e..8e7e1b225eb 100644
--- a/tests/unit_tests/pandas_postprocessing/test_prophet.py
+++ b/tests/unit_tests/pandas_postprocessing/test_prophet.py
@@ -22,6 +22,7 @@ from unittest.mock import patch
import pandas as pd
import pytest
+from superset.charts.schemas import get_max_prophet_periods
from superset.exceptions import InvalidPostProcessingError
from superset.utils.core import DTTM_ALIAS
from superset.utils.pandas_postprocessing import prophet
@@ -180,6 +181,23 @@ def test_prophet_incorrect_periods():
)
+def test_prophet_periods_exceeding_max_raises():
+ """
+ ``periods`` comes from the unvalidated post-processing options dict; the
+ schema-declared upper bound (``MAX_PROPHET_PERIODS``, configurable, default
+ 10000) is documentation-only unless enforced at the point ``periods`` is
+ consumed, since every forecast period adds a future row per series.
+ """
+ assert get_max_prophet_periods() == 10000
+ with pytest.raises(InvalidPostProcessingError, match="must not exceed"):
+ prophet(
+ df=prophet_df,
+ time_grain="P1M",
+ periods=10001,
+ confidence_interval=0.8,
+ )
+
+
def test_prophet_incorrect_time_grain():
with pytest.raises(InvalidPostProcessingError):
prophet(
diff --git a/tests/unit_tests/pandas_postprocessing/test_resample.py b/tests/unit_tests/pandas_postprocessing/test_resample.py
index 7c742de8f36..b0145be8e9e 100644
--- a/tests/unit_tests/pandas_postprocessing/test_resample.py
+++ b/tests/unit_tests/pandas_postprocessing/test_resample.py
@@ -21,6 +21,7 @@ from pandas import to_datetime
from superset.exceptions import InvalidPostProcessingError
from superset.utils import pandas_postprocessing as pp
+from superset.utils.pandas_postprocessing.resample import MAX_RESAMPLE_ROWS
from tests.unit_tests.fixtures.dataframes import (
categories_df,
timeseries_df,
@@ -205,6 +206,44 @@ def test_resample_after_pivot():
)
+def test_resample_rejects_projection_over_row_limit():
+ """
+ ``rule`` comes from the unvalidated post-processing options dict. Without
+ a projected-row-count cap, upsampling a multi-day span to a fine-grained
+ fixed frequency (e.g. nanoseconds) would attempt to allocate an
+ astronomically large DataFrame from a single request.
+ """
+ with pytest.raises(InvalidPostProcessingError, match="exceeding the limit"):
+ pp.resample(df=timeseries_df, rule="1ns", method="ffill")
+
+
+def test_resample_rejects_projection_at_alignment_boundary():
+ """
+ pandas snaps the first resample bin to the nearest frequency multiple at
+ or before the observed span, which can add one bin beyond a naive
+ span/step projection. A span/step whose naive estimate lands exactly at
+ ``MAX_RESAMPLE_ROWS`` must still be rejected once that alignment margin
+ is taken into account, rather than silently allowed through.
+ """
+ span_seconds = MAX_RESAMPLE_ROWS - 1 # naive span/step + 1 == MAX_RESAMPLE_ROWS
+ df = pd.DataFrame(
+ index=to_datetime("2019-01-01") + pd.to_timedelta([0, span_seconds], unit="s"),
+ data={"y": [1.0, 2.0]},
+ )
+ with pytest.raises(InvalidPostProcessingError, match="exceeding the limit"):
+ pp.resample(df=df, rule="1s", method="ffill")
+
+
+def test_resample_allows_legitimate_upsampling_within_limit():
+ """A week upsampled to 1-second granularity (604,800 rows) stays allowed."""
+ df = pd.DataFrame(
+ index=to_datetime(["2019-01-01", "2019-01-08"]),
+ data={"y": [1.0, 2.0]},
+ )
+ post_df = pp.resample(df=df, rule="1s", method="ffill")
+ assert len(post_df) == 604801
+
+
def test_resample_should_raise_ex():
with pytest.raises(InvalidPostProcessingError):
pp.resample(
diff --git a/tests/unit_tests/pandas_postprocessing/test_rolling.py b/tests/unit_tests/pandas_postprocessing/test_rolling.py
index cd162fb2d7b..019002de4de 100644
--- a/tests/unit_tests/pandas_postprocessing/test_rolling.py
+++ b/tests/unit_tests/pandas_postprocessing/test_rolling.py
@@ -107,6 +107,23 @@ def test_rolling():
)
+def test_rolling_rejects_window_over_max():
+ """
+ ``window`` comes from the unvalidated post-processing options dict; the
+ documented schema range (1-10000) is enforced at the point ``window`` is
+ consumed, since a huge integer window combined with ``win_type`` makes
+ scipy allocate a weights array proportional to it.
+ """
+ with pytest.raises(InvalidPostProcessingError, match="must not exceed"):
+ pp.rolling(
+ df=timeseries_df,
+ columns={"y": "y"},
+ rolling_type="sum",
+ window=10001,
+ min_periods=0,
+ )
+
+
def test_rolling_min_periods_trims_correctly():
pivot_df = pp.pivot(
df=single_metric_df,
diff --git a/tests/unit_tests/utils/excel_tests.py b/tests/unit_tests/utils/excel_tests.py
index ccadd70b8e6..4320df19f3d 100644
--- a/tests/unit_tests/utils/excel_tests.py
+++ b/tests/unit_tests/utils/excel_tests.py
@@ -27,6 +27,7 @@ from superset.utils.excel import (
apply_column_types,
df_to_excel,
NEUTRAL_TIMESTAMP,
+ quote_formulas,
)
@@ -53,6 +54,49 @@ def test_quote_formulas() -> None:
]
+def test_quote_formulas_in_headers_and_index() -> None:
+ """
+ Test that formulas in column headers and index labels are quoted too.
+
+ Pivot exports promote data values into the column MultiIndex and row
+ index, so hostile warehouse strings can end up there.
+ """
+ df = pd.DataFrame(
+ {"=SUM(A1:A2)": ["normal"]},
+ index=pd.Index(['=cmd|" /C calc"!A0'], name="label"),
+ )
+ contents = df_to_excel(df)
+ result = pd.read_excel(contents, index_col=0)
+ assert result.columns.tolist() == ["'=SUM(A1:A2)"]
+ assert result.index.tolist() == ['\'=cmd|" /C calc"!A0']
+
+
+def test_quote_formulas_in_axis_names() -> None:
+ """
+ Test that formula-triggering axis *names* are quoted too, not just axis
+ labels/values. Pivot exports can promote a warehouse-controlled column
+ name to ``df.index.name`` (or a MultiIndex level name), and pandas
+ writes those names into the sheet as header cells. ``rename`` alone
+ leaves these untouched since they are a separate attribute from the
+ axis labels/values, so this is asserted directly against the
+ ``quote_formulas`` output rather than round-tripped through
+ ``read_excel``, whose header parsing doesn't reliably preserve the
+ columns-axis name.
+ """
+ df = pd.DataFrame(
+ {"value": ["normal"]},
+ index=pd.Index(["row"], name="=SUM(A1:A2)"),
+ )
+ df.columns.name = "+cmd"
+ result = quote_formulas(df)
+ assert result.index.name == "'=SUM(A1:A2)"
+ assert result.columns.name == "'+cmd"
+
+ # exercised end-to-end to confirm it doesn't error when the axis names
+ # are written out as sheet header cells
+ df_to_excel(df)
+
+
def test_document_properties_are_neutral() -> None:
"""
Test that exported workbooks do not carry identifying document properties.