rusackas opened a new pull request, #42295: URL: https://github.com/apache/superset/pull/42295
### SUMMARY Embedded guests get a generic 403 (`Guest user cannot modify chart payload`) on any chart persisted without a stored `query_context` — charts created via the REST API and every chart loaded by `load_examples`. The guest tamper check (`_columns_metrics_modified`) compares the requested metrics/columns against only the same-named chart params keys, but chart types store what they query under control-specific keys: `metric` (big number, world map), `entity`/`series`/`x`/`y`/`size` (bubble), `granularity_sqla` (temporal axis), `secondary_metric` (sunburst), plain `groupby` vs the payload's `columns` (even table hits this). The chart's own generated payload reads as tampering. This is also why `playwright-tests-experimental`'s embedded suite is flaky: the embedded project runs with admin `storageState`, and a cookie race (the `/embedded/<uuid>` response rotates the iframe to an anonymous session while the SDK's parallel csrf/guest-token fetches rewrite the admin cookie) decides whether chart data requests are evaluated as admin (tamper check skipped, 200) or as a real guest (403). The suite only passes when admin wins. The fix reads the stored side across the control names that can hold a metric or column, keeping exact `freeze_value` matching — only values the chart already references become requestable. No label or expression equivalence is introduced: novel saved metrics, novel columns, adhoc SQL expressions, and label-spoofed adhoc columns replaying a stored column's label are all still rejected (covered by new unit tests). Verification beyond unit tests: replayed the actual network payloads of all nine `world_health` example charts (captured from the embedded Playwright trace artifacts) through the check — every one was rejected before this change and passes after, while tampered variants of the same payloads stay rejected. Security model note per `SECURITY.md`: this does not grant any principal a new capability. An embedded guest token holder is entitled to view the data of charts on the dashboard shared with them; the values unlocked here are exactly the ones the stored chart already renders to that guest. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Before: embedded `world_health` charts show "Data error: Forbidden" for guest-evaluated requests. After: they render. ### TESTING INSTRUCTIONS `pytest tests/unit_tests/security/manager_test.py -k query_context_modified` — includes the new singular-metric, control-specific-columns, and still-tampered cases. Or embed the `world_health` dashboard via the SDK and confirm charts load for a guest token. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
