rusackas opened a new pull request, #44406:
URL: https://github.com/apache/superset/pull/44406
### SUMMARY
A prior fix (#42930) closed a real cross-user data leak: annotation-layer
payloads are fetched per requesting user and, for chart-backed layers, scoped
by the RLS clauses of the referenced chart's datasource, so they need to be
isolated per viewer. That fix bound the requesting user's identity (and RLS
context) into the cache key for the *entire* chart dataframe, since the
dataframe and the annotation payload shared one cache entry. The side effect:
every distinct viewer of an annotation-layer chart now gets their own full
copy of the (potentially much larger) dataframe cached separately, instead of
sharing one entry.
This PR splits the two apart. The dataframe's cache key goes back to
depending only on its own datasource/RLS/`extra_cache_keys` (unscoped by
requesting user), so distinct viewers of the same chart share one dataframe
cache entry again. Annotation data is now resolved and cached under its own,
separate key that still carries the per-user/per-RLS security context #42930
introduced — so the original leak stays fixed, but only the (much smaller)
annotation payload pays the per-user cost.
As a side effect, this also restores cross-user task dedup for annotated
charts in the async ("get the fresh data") flow: the SHARED task key there
is derived from the same dataframe cache key, so it had the identical
coupling problem — two users viewing the same annotated chart could no
longer join the same in-flight query task.
No cache migration is needed; keys are content-addressed and recomputed on
every request, so stale entries under the old key shape simply age out via
normal TTL.
### TESTING INSTRUCTIONS
- `pytest tests/unit_tests/common/test_query_context_processor.py` — includes
new coverage: the dataframe cache key no longer varies per user for
annotated queries, the annotation cache key still does,
`_get_annotation_data_cached`'s
hit/miss/`force_query`/`force_cached` behavior, and an end-to-end case
proving a dataframe cache hit skips recomputation while annotation data
still resolves through its own path into the payload.
- `pytest tests/unit_tests/tasks/test_async_queries.py` — unaffected/still
green.
- Manually: load a dashboard with an annotation-layer chart as two different
users with different RLS; confirm both see correctly-scoped annotation data
and that the chart's underlying dataframe query only runs once (e.g. via
query count/logging) rather than once per viewer.
### 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]
