codeant-ai-for-open-source[bot] commented on code in PR #41133:
URL: https://github.com/apache/superset/pull/41133#discussion_r3486016131
##########
superset/dashboards/schemas.py:
##########
@@ -633,3 +633,21 @@ class CacheScreenshotSchema(Schema):
fields.List(fields.Str(), validate=lambda x: len(x) == 2),
required=False
)
permalinkKey = fields.Str(required=False) # noqa: N815
+
+
+class DashboardExportXlsxPostSchema(Schema):
+ active_data_mask = fields.Dict(
+ keys=fields.Str(),
+ values=fields.Dict(),
+ load_default=dict,
Review Comment:
**Suggestion:** The request schema only validates each `active_data_mask`
entry as a generic dict and does not enforce that `extraFormData` is itself a
dict, so malformed payloads can pass validation and later crash filter-context
merging in the Celery task. Tighten this to a nested schema that requires
`extraFormData` to be a dict so invalid requests are rejected with 400 instead
of failing during export. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Filter context with malformed extraFormData crashes per-chart export.
- ⚠️ Export email omits affected charts; user receives incomplete workbook.
- ⚠️ Logs contain AttributeError from _merge_extra_form_data merging filters.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure Excel export so the endpoint runs end-to-end by setting a
non-empty
`EXCEL_EXPORT_S3_BUCKET` in config (checked at
`superset/dashboards/api.py:58-61` in
`DashboardRestApi.export_xlsx`); start Superset API and Celery workers
normally.
2. Send a POST to `/api/v1/dashboard/<dashboard_id>/export_xlsx/` (handled by
`DashboardRestApi.export_xlsx` at `superset/dashboards/api.py:9-18`) with
JSON body:
`{"active_data_mask": {"NATIVE_FILTER-1": {"extraFormData": ["not", "a",
"dict"]}}}` so
that each mask entry is a dict but its `extraFormData` value is a list.
3. Observe in `export_xlsx` that `DashboardExportXlsxPostSchema().load(...)`
(schema
defined at `superset/dashboards/schemas.py:39-48`) accepts the payload:
`active_data_mask`
is a dict and each value is a dict, and no validation is applied to the
nested
`extraFormData` field; the method then enqueues
`export_dashboard_excel.apply_async(...)`
passing `payload.get("active_data_mask", {})` at
`superset/dashboards/api.py:28-35`.
4. When the Celery worker processes `export_dashboard_excel`
(`superset/tasks/export_dashboard_excel.py:168-214`), `_build_workbook` calls
`_write_chart_sheets` with `active_data_mask` (lines 114-129); inside
`_write_chart_sheets`, `get_dashboard_filter_context(...,
active_data_mask=active_data_mask)` is called at
`superset/tasks/export_dashboard_excel.py:84-88`, which in turn calls
`_resolve_filter_extra_form_data` at
`superset/charts/data/dashboard_filter_context.py:202-224`. For the
malformed filter,
`active_efd = (active_data_mask[flt_id] or {}).get("extraFormData") or {}`
becomes a list,
and `get_dashboard_filter_context` passes this list as `extra_form_data` into
`_merge_extra_form_data` at
`superset/charts/data/dashboard_filter_context.py:120-167`,
where `new.get(...)` raises `AttributeError: 'list' object has no attribute
'get'`. This
exception is caught by `_build_workbook`'s per-chart `except Exception`
block at
`superset/tasks/export_dashboard_excel.py:128-135`, causing the chart to be
skipped and
logged as failed instead of rejecting the request up front with a 400.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b5d253f4b4c242a4a752b12676bfd7fb&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=b5d253f4b4c242a4a752b12676bfd7fb&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/dashboards/schemas.py
**Line:** 639:642
**Comment:**
*Type Error: The request schema only validates each `active_data_mask`
entry as a generic dict and does not enforce that `extraFormData` is itself a
dict, so malformed payloads can pass validation and later crash filter-context
merging in the Celery task. Tighten this to a nested schema that requires
`extraFormData` to be a dict so invalid requests are rejected with 400 instead
of failing during export.
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%2F41133&comment_hash=c13f44ef951daa1cd03a508ae7dbe7bbd2a91949623c138eeb8bf52a41d57abc&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41133&comment_hash=c13f44ef951daa1cd03a508ae7dbe7bbd2a91949623c138eeb8bf52a41d57abc&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]
