SEPURI-SAI-KRISHNA opened a new issue, #44952:
URL: https://github.com/apache/superset/issues/44952

   ### Bug description
   
   A malformed `post_processing` option returns **HTTP 500** instead of 400. 
Fuzzing the options of every operation found 39 distinct sites that reach 
`app.errorhandler(Exception)`; 38 of them are bad requests that should be 400, 
and 1 (a missing scipy) is correctly a 500.
   
   The intended contract is already pinned in the test suite — 
`tests/integration_tests/charts/data/api_tests.py::test_chart_data_invalid_post_processing`
 asserts `status_code == 400` for a bad `pivot` option. #44463 (merged 
2026-10-02, `models/helpers.py`) and #44502 (merged 2026-09-21, 
`semantic_layers/models.py`) widened the guard at both `exec_post_processing` 
call sites so that raw pandas `TypeError` and `pandas.errors.DataError` become 
`QueryObjectValidationError`:
   
   ```python
   # superset/models/helpers.py:2684 and superset/semantic_layers/models.py:377
   except InvalidPostProcessingError as ex:
       raise QueryObjectValidationError(ex.message) from ex
   except (TypeError, pd.errors.DataError) as ex:
       raise QueryObjectValidationError(str(ex)) from ex
   ```
   
   **`ValueError`, `KeyError` and `AttributeError` escape the same way.** They 
are not caught here; `QueryContextProcessor.get_df_payload` catches only 
`QueryObjectValidationError`; `ChartDataRestApi.data` has no `@safe` decorator; 
so they reach `app.errorhandler(Exception)`, whose `json_error_response` 
defaults to `status=500`.
   
   ### Why these reach pandas at all
   
   `ChartDataPostProcessingOperationSchema.options` is a bare `fields.Dict` 
(`superset/charts/schemas.py:1123`) — no nested schema, so **no option value is 
validated before it reaches pandas**. The 11 `ChartData*OptionsSchema` classes 
(schemas.py:588-1054) are referenced only from `CHART_SCHEMAS`, which is 
OpenAPI output; they validate nothing. Eight operations have no options schema 
at all: `compare`, `cum`, `diff`, `flatten`, `histogram`, `rank`, `rename`, 
`resample`.
   
   ### Measured
   
   I fuzzed each operation's options one at a time away from a known-good 
baseline (910 calls over 19 operations), classifying by the oracle above. **39 
distinct unhandled-exception sites**, spread over 14 of the 19 operations. The 
sweep mutates one option at a time and is not exhaustive over every possible 
value, so this is a lower bound rather than a complete census:
   
   | exception | distinct sites |
   |---|---|
   | `ValueError` | 21 |
   | `KeyError` | 10 |
   | `AttributeError` | 7 |
   | `ImportError` | 1 |
   
   `rolling` 9, `pivot` 5, `histogram` 5, `rank` 3, `diff` 3, `aggregate` 3, 
`sort` 2, `geohash_encode` 2, `boxplot` 2, and one each for `resample`, 
`geohash_decode`, `geodetic_parse`, `flatten`, `contribution`.
   
   Representative cases, each verified to pass 
`ChartDataQueryObjectSchema.load()` first and then raise:
   
   | `post_processing` options | raises | should be |
   |---|---|---|
   | `boxplot` `percentiles: [10, 200]` | `ValueError: Percentiles must be in 
the range [0, 100]` | 400 |
   | `rolling` `window: -1` | `ValueError: min_periods 0 must be <= window -1` 
| 400 |
   | `rolling` `center: "yes"` | `ValueError: center must be a boolean` | 400 |
   | `diff` `periods: 1.5` | `ValueError: periods must be an integer` | 400 |
   | `diff` `axis: "nope"` | `ValueError: No axis named nope` | 400 |
   | `sort` `ascending: null` | `ValueError: expected type bool` | 400 |
   | `pivot` `marginal_distributions: 1` | `ValueError: margins_name argument 
must be a string` | 400 |
   | `rank` `metric: "nope"` | `KeyError: 'nope'` | 400 |
   | `boxplot` `metrics: ["nope"]` | `KeyError: 'nope'` | 400 |
   | `aggregate` `aggregates: "x"` | `AttributeError: 'str' object has no 
attribute 'items'` | 400 |
   | `pivot` `aggregates: {"y": 0}` | `AttributeError: 'int' object has no 
attribute 'get'` | 400 |
   
   Note the `KeyError` family: a post-processing option naming a column that 
does not exist is a 500. The `invalid_columns` check in `get_df_payload` 
validates `query_obj.columns` and `metrics`, but not column names referenced 
from post-processing options.
   
   ### How to reproduce the bug
   
   1. `POST /api/v1/chart/data` with a query whose `post_processing` is
      `[{"operation": "rank", "options": {"metric": "does_not_exist"}}]`.
   2. The response is 500, not the 400 that 
`test_chart_data_invalid_post_processing` establishes as the contract.
   
   ### Suggested fix
   
   Handle `ValueError`, `KeyError` and `AttributeError` at both call sites, 
following the pattern #44463 established. That converts 38 of the 39 sites the 
fuzz found.
   
   `KeyError` wants its own arm rather than the shared `str(ex)` message: 
`str(KeyError("x"))` is `"'x'"`, so it would answer with `Error: 
'does_not_exist'`. All ten `KeyError` sites name a column or MultiIndex level, 
so a specific message is accurate for each.
   
   Because `ValueError` and `AttributeError` are broad enough to also cover a 
genuine fault inside an operation, and `get_df_payload` records the message 
without logging a traceback, the handler should log the exception before 
re-raising so operators keep the stack trace.
   
   `ImportError` should be left out: the one case is `rolling` with a 
`win_type`, which needs scipy. A missing optional dependency is a deployment 
matter, not a bad request, so 500 is the right answer there.
   
   ### Not a security issue
   
   This is a robustness and API-contract bug, not a boundary violation under 
`SECURITY.md`: it requires a principal already entitled to run chart-data 
queries, grants no capability the role matrix withholds, and exposes no data. 
Filing it as a bug for that reason.
   
   ### Screenshots/recordings
   
   _No response_
   
   ### Superset version
   
   master / latest-dev
   
   ### Python version
   
   3.11
   
   ### Node version
   
   Not applicable
   
   ### Browser
   
   Not applicable
   
   ### Additional context
   
   The guard is duplicated verbatim in `superset/models/helpers.py` and 
`superset/semantic_layers/models.py`. That duplication is why the two were 
fixed eleven days apart (#44502 on 09-21, #44463 on 10-02); #44463's own 
description flags the second site as a follow-up. Consolidating it would 
prevent the next drift, but that is a separate change.
   
   ### Checklist
   
   - [x] I have searched Superset docs and Slack and didn't find a solution to 
my problem.
   - [x] I have searched the GitHub issue tracker and didn't find a similar bug 
report.
   - [x] I have checked Superset's logs for errors and if I found a relevant 
Python stacktrace, I included it here as text in the "additional context" 
section.
   


-- 
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]

Reply via email to