codeant-ai-for-open-source[bot] commented on code in PR #39255:
URL: https://github.com/apache/superset/pull/39255#discussion_r3591340571
##########
superset/charts/data/api.py:
##########
@@ -554,6 +563,30 @@ def _log_is_cached(
elif is_cached_values:
add_extra_log_payload(is_cached=is_cached_values)
+ def _build_export_filename(self, form_data: dict[str, Any] | None) -> str:
+ """Build a sanitized filename for CSV/Excel chart exports.
+
+ Tries slice_name from form_data first, then falls back to a DB lookup
+ by slice_id, then viz_type, then a generic "export" default.
+ """
+ slice_name = (form_data or {}).get("slice_name")
+ if not slice_name:
+ slice_id = (form_data or {}).get("slice_id")
+ if slice_id:
+ try:
+ slice_obj = db.session.get(Slice, int(slice_id))
+ if slice_obj:
+ slice_name = slice_obj.slice_name
Review Comment:
**Suggestion:** The chart lookup by `slice_id` reads `Slice` directly from
the DB without any chart access check, so a user can submit arbitrary
`slice_id` values and infer names of charts they are not authorized to view via
the download filename. Gate this lookup behind chart-permission validation (for
example, verify access before using `slice_name`) or skip DB fallback when
access cannot be proven. [security]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ CSV/Excel chart data export leaks unauthorized chart names.
- ⚠️ Filename-based metadata disclosure affects Explore download endpoints.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call the chart data export API via `ChartDataRestApi.data`
(superset/charts/data/api.py:266) with a POST body containing `form_data`
for a dataset
the user can access, setting `form_data["slice_id"]` to the numeric ID of a
chart the user
is not allowed to view.
2. Inside `data()` (superset/charts/data/api.py:266-355) the server builds a
`QueryContext` from `json_body` using `_create_query_context_from_form()`
and constructs a
`ChartDataCommand`; `ChartDataCommand.validate()` at
superset/commands/chart/data/get_data_command.py:72 calls
`QueryContext.raise_for_access()` to enforce datasource access but does not
validate the
`slice_id` or check chart-level permissions.
3. `_get_data_response()` (superset/charts/data/api.py:591) is invoked with
the
attacker-controlled `form_data`, calls `command.run()` to execute the query,
and then
forwards `result` and `form_data` into `_send_chart_response()`
(superset/charts/data/api.py:449) when the requested `result_format` is CSV
or XLSX and
the query succeeds.
4. In `_send_chart_response()` the single-query export path calls
`export_filename =
self._build_export_filename(form_data)` at superset/charts/data/api.py:492;
`_build_export_filename()` performs `slice_obj = db.session.get(Slice,
int(slice_id))` at
lines 577-579 with no `security_manager` permission check, then uses
`slice_obj.slice_name` in the CSV/XLSX `Content-Disposition` headers,
allowing a user to
iterate `slice_id` values and infer names of charts they are not authorized
to view.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b1200b6ee5134238bbe1df02c01daf58&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b1200b6ee5134238bbe1df02c01daf58&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/charts/data/api.py
**Line:** 577:579
**Comment:**
*Security: The chart lookup by `slice_id` reads `Slice` directly from
the DB without any chart access check, so a user can submit arbitrary
`slice_id` values and infer names of charts they are not authorized to view via
the download filename. Gate this lookup behind chart-permission validation (for
example, verify access before using `slice_name`) or skip DB fallback when
access cannot be proven.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39255&comment_hash=b5f9b064cff140485233d88013e9f0876827933827ecd0572eaa2c3d0228468a&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39255&comment_hash=b5f9b064cff140485233d88013e9f0876827933827ecd0572eaa2c3d0228468a&reaction=dislike'>👎</a>
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
