aminghadersohi opened a new pull request, #45485:
URL: https://github.com/apache/superset/pull/45485
### SUMMARY
Accept dashboard titles and chart names containing prose such as `[DEMO]
Service Desk - Executive Summary (substitute data: Jira support tickets)`
rather than treating every occurrence of `data:` as a URL.
Title/name warning detection runs before field validation. Attach the
canonical field location to its errors so the existing bounded MCP diagnostic
formatter can report `request.dashboard_title` or `request.chart_name`, without
exposing the supplied value. Chart-update aliases also use canonical error
locations.
Pattern changes in `superset/mcp_service/utils/sanitization.py`:
- Replace `\b(javascript|vbscript|data):` with an unchanged script-scheme
alternative, `\b(javascript|vbscript):`, plus data-specific alternatives.
- `^\s*data:` rejects a data scheme at the start of the value, including
whitespace before its payload.
- `\bdata:(?=\S|$)` rejects an adjacent payload or a bare scheme at the end,
including URL-like tokens following whitespace, quotes, or parentheses.
- `\bdata:\s+[^,]*,` conservatively rejects comma-delimited data URLs after
whitespace, including encoded or whitespace-obfuscated MIME types. Prose
containing `data:` and a later comma remains conservatively rejected.
- No event-handler, HTML-stripping, Unicode-normalization, or SQL-rejection
patterns change.
`test_url_scheme_payloads_remain_rejected` in
`tests/unit_tests/mcp_service/utils/test_sanitization.py` pins
JavaScript/VBScript payloads (including spaces), data URLs with
MIME/base64/empty MIME, whitespace and percent-encoded MIME, token boundaries,
case folding, HTML entities, and zero-width encoding. It exercises both text
and filter sanitizers. Existing HTML and Unicode rejection tests also remain
green.
Sibling paths checked: dashboard generation/update/duplication; chart
generation/update and aliases; chart
column/label/filter/time-range/comparison-period/bullet-label sanitizers;
dashboard certification fields. The latter field validators already attach
field locations and share the updated sanitizer. Dataset names and dashboard
descriptions do not call this sanitizer. SQL-expression validation,
error-message filtering, log sanitization, and result serialization use
separate policies and are unchanged. No gateway change is needed.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable: MCP argument validation, no UI change.
Before: the title above fails validation; warning-detection errors point
only to `request`.
After: the title is accepted, and rejected title/name values identify their
field without echoing the input.
### TESTING INSTRUCTIONS
Local results:
- With the new regressions and the base implementation restored, the two
changed test modules produced **26 failed, 146 passed**; failures include
`test_title_accepts_data_in_prose`, `test_sanitizer_accepts_data_in_prose`,
`test_title_sanitization_error_names_field`, and
`test_title_sanitization_wire_error_names_field`.
- Targeted head run below: **791 passed**. The wire tests use a real
in-process FastMCP client/server and canonical/alias arguments; no external
model is called.
- `pre-commit run --files <the seven changed files>`: exit 0, including
mypy, ruff, and pylint.
```bash
PYTHONPATH="$PWD/superset-core/src:$PWD" pytest -q \
tests/unit_tests/mcp_service/test_sanitization_warning_inputs.py \
tests/unit_tests/mcp_service/utils/test_sanitization.py \
tests/unit_tests/mcp_service/utils/test_sanitization_unicode_ordering.py \
tests/unit_tests/mcp_service/test_validation_contract.py \
tests/unit_tests/mcp_service/chart/test_chart_schemas.py \
tests/unit_tests/mcp_service/dashboard/test_dashboard_schemas.py \
tests/unit_tests/mcp_service/utils/test_schema_utils.py \
-m 'not manual and not llm_compliance'
```
Manual: call `generate_dashboard` or `update_dashboard` with the prose title
above and otherwise valid arguments. Repeat with `javascript:alert(1)` and
`data:text/html,payload`; those requests should be rejected with
`request.dashboard_title` in the error.
The local Superset HTTP health endpoint was unavailable; a deployed gateway
end-to-end run was not performed. Real-LLM checks were not run.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
