mirror of
https://github.com/apache/superset.git
synced 2026-08-12 11:11:01 +00:00
fix(export): sanitize user-supplied CSV export filename (charts + SQL Lab) (#40632)
Co-authored-by: Claude Code <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Code
parent
fa41769a08
commit
b9dc9d722e
@@ -616,6 +616,13 @@ class ChartDataRestApi(ChartRestApi):
|
||||
def _extract_export_params_from_request(self) -> tuple[str | None, int | None]:
|
||||
"""Extract filename and expected_rows from request for streaming exports."""
|
||||
filename = request.form.get("filename")
|
||||
if filename:
|
||||
# Sanitize the user-supplied filename before it is used in the
|
||||
# Content-Disposition header (consistent with the generated-name
|
||||
# path). secure_filename may reduce a name consisting entirely of
|
||||
# unsupported characters to an empty string, in which case fall back
|
||||
# to the generated default downstream.
|
||||
filename = secure_filename(filename) or None
|
||||
if filename:
|
||||
logger.info("FRONTEND PROVIDED FILENAME: %s", filename)
|
||||
|
||||
|
||||
@@ -419,6 +419,14 @@ class SqlLabRestApi(BaseSupersetApi):
|
||||
command = StreamingSqlResultExportCommand(client_id, chunk_size)
|
||||
command.validate()
|
||||
|
||||
if filename:
|
||||
# Sanitize the user-supplied filename before it is used in the
|
||||
# Content-Disposition header (consistent with the generated-name
|
||||
# path below). secure_filename may reduce a name consisting entirely
|
||||
# of unsafe characters to an empty string, in which case we fall
|
||||
# back to the generated default.
|
||||
filename = secure_filename(filename) or None
|
||||
|
||||
if not filename:
|
||||
timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
|
||||
filename = secure_filename(f"sqllab_{client_id}_{timestamp}.csv")
|
||||
|
||||
@@ -16,6 +16,8 @@
|
||||
# under the License.
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from flask import Flask, g
|
||||
|
||||
from superset.utils import json
|
||||
@@ -69,3 +71,32 @@ def test_get_data_sets_g_form_data_without_dashboard_filter() -> None:
|
||||
assert hasattr(g, "form_data")
|
||||
assert g.form_data["datasource"] == {"id": 42, "type": "table"}
|
||||
assert g.form_data["queries"][0]["columns"] == ["col1"]
|
||||
|
||||
|
||||
def _extract_filename(form_value: str) -> str | None:
|
||||
"""Run _extract_export_params_from_request with a form filename value."""
|
||||
from superset.charts.data.api import ChartDataRestApi
|
||||
|
||||
app = Flask(__name__)
|
||||
with app.test_request_context("/", method="POST", data={"filename": form_value}):
|
||||
filename, _ = ChartDataRestApi._extract_export_params_from_request(MagicMock())
|
||||
return filename
|
||||
|
||||
|
||||
def test_extract_export_filename_sanitizes_special_characters() -> None:
|
||||
"""A malicious/path-y filename is sanitized before header/disk use."""
|
||||
filename = _extract_filename('../../etc/pa"ss\r\nSet-Cookie: x')
|
||||
|
||||
assert filename is not None
|
||||
for bad in ("/", "\\", '"', "\r", "\n", ".."):
|
||||
assert bad not in filename
|
||||
|
||||
|
||||
def test_extract_export_filename_preserves_normal_name() -> None:
|
||||
"""A normal filename passes through unchanged."""
|
||||
assert _extract_filename("my_export.csv") == "my_export.csv"
|
||||
|
||||
|
||||
def test_extract_export_filename_all_special_falls_back_to_none() -> None:
|
||||
"""A name with no usable characters becomes None (generated downstream)."""
|
||||
assert _extract_filename("***") is None
|
||||
|
||||
@@ -0,0 +1,16 @@
|
||||
# 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.
|
||||
@@ -0,0 +1,64 @@
|
||||
# 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 __future__ import annotations
|
||||
|
||||
import re
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from flask import Flask
|
||||
|
||||
|
||||
def _disposition_filename(form_filename: str | None) -> str:
|
||||
"""Return the filename rendered into a streaming CSV Content-Disposition."""
|
||||
from superset.sqllab.api import SqlLabRestApi
|
||||
|
||||
app = Flask(__name__)
|
||||
app.config["CSV_EXPORT"] = {"encoding": "utf-8"}
|
||||
with (
|
||||
app.app_context(),
|
||||
patch("superset.sqllab.api.StreamingSqlResultExportCommand") as command_cls,
|
||||
):
|
||||
command = command_cls.return_value
|
||||
command.run.return_value = lambda: iter([b""])
|
||||
response = SqlLabRestApi._create_streaming_csv_response(
|
||||
MagicMock(), client_id="abc123", filename=form_filename
|
||||
)
|
||||
disposition = response.headers["Content-Disposition"]
|
||||
match = re.search(r'filename="([^"]*)"', disposition)
|
||||
assert match is not None, disposition
|
||||
return match.group(1)
|
||||
|
||||
|
||||
def test_streaming_csv_sanitizes_user_filename() -> None:
|
||||
"""A path-y / header-injecting filename is sanitized before the header."""
|
||||
filename = _disposition_filename('../../etc/pa"ss\r\nSet-Cookie: x.csv')
|
||||
|
||||
for bad in ("/", "\\", '"', "\r", "\n", ".."):
|
||||
assert bad not in filename
|
||||
|
||||
|
||||
def test_streaming_csv_preserves_normal_filename() -> None:
|
||||
"""A normal filename passes through unchanged."""
|
||||
assert _disposition_filename("my_results.csv") == "my_results.csv"
|
||||
|
||||
|
||||
def test_streaming_csv_falls_back_when_filename_empty() -> None:
|
||||
"""An all-unsafe filename collapses to the generated default, not empty."""
|
||||
filename = _disposition_filename("///")
|
||||
|
||||
assert filename.startswith("sqllab_abc123_")
|
||||
assert filename.endswith(".csv")
|
||||
Reference in New Issue
Block a user