codeant-ai-for-open-source[bot] commented on code in PR #37516:
URL: https://github.com/apache/superset/pull/37516#discussion_r3601092201
##########
superset/models/sql_lab.py:
##########
@@ -367,7 +366,7 @@ def perm(self) -> str:
def default_endpoint(self) -> str:
return ""
- def get_extra_cache_keys(self, query_obj: QueryObjectDict) ->
list[Hashable]:
+ def get_extra_cache_keys(self, query_obj: QueryObjectDict) -> list[Any]:
return []
Review Comment:
**Suggestion:** Returning an unconditional empty list here makes cache keys
for SQL-query datasources ignore user-dependent Jinja/ExtraCache context. If a
query uses user-scoped macros (for example current-user values) and result
caching is enabled, different users can collide on the same cache key and
receive each other’s cached results. Build extra cache keys from the query
SQL/template context (like SqlaTable does) so user-specific inputs are part of
the cache key. [cache]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Query-based charts with Jinja macros leak user-specific data.
- ❌ Cached /api/v1/chart/data results shared between different users.
- ⚠️ Query datasources ignore ExtraCache user context entirely.
- ⚠️ Per-user caching relies solely on global impersonation flags.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset/common/query_context_processor.py:59-77`,
`QueryContextProcessor.query_cache_key` calls
`datasource.get_extra_cache_keys(query_obj.to_dict())` and passes the result
into
`query_obj.cache_key(...)` as the `extra_cache_keys` argument.
2. When a chart uses a SQL Lab saved query as its datasource (datasource type
`DatasourceType.QUERY`), `self._qc_datasource` in `QueryContextProcessor` is
an instance
of `Query` from `superset/models/sql_lab.py:16-27`.
3. The `Query` class implementation of `get_extra_cache_keys` at
`superset/models/sql_lab.py:369-370` simply returns an empty list, so
`extra_cache_keys`
passed into `QueryObject.cache_key` (see
`superset/common/query_object.py:489-500`) is
always `[]` for query-based datasources.
4. Create a SQL Lab query that uses user-scoped Jinja macros (for example `{{
current_user_id() }}` or `{{ url_param('tenant_id') }}` from
`superset/jinja_context.py:46-60, 240-37`), save it, and build a chart on
top of this
saved query; then enable result caching and hit `/api/v1/chart/data` as two
different
users. Because `Query.get_extra_cache_keys` ignores any user-specific
context, both users
share the same cache key and the second user receives the first user’s
cached results
despite different macro inputs.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f3a81fa91f84432b9634a484a042dc34&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=f3a81fa91f84432b9634a484a042dc34&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/models/sql_lab.py
**Line:** 369:370
**Comment:**
*Cache: Returning an unconditional empty list here makes cache keys for
SQL-query datasources ignore user-dependent Jinja/ExtraCache context. If a
query uses user-scoped macros (for example current-user values) and result
caching is enabled, different users can collide on the same cache key and
receive each other’s cached results. Build extra cache keys from the query
SQL/template context (like SqlaTable does) so user-specific inputs are part of
the cache key.
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%2F37516&comment_hash=35eabfe7286ce567a9653e9633bb907c9f659966e7c7c6d6a0eda58537e7fbfb&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=35eabfe7286ce567a9653e9633bb907c9f659966e7c7c6d6a0eda58537e7fbfb&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]
