codeant-ai-for-open-source[bot] commented on code in PR #37516:
URL: https://github.com/apache/superset/pull/37516#discussion_r3610258129
##########
superset/explore/utils.py:
##########
@@ -53,7 +53,7 @@ def check_query_access(query_id: int) -> Optional[bool]:
# Access checks below, no need to validate them twice as they can be
expensive.
query = QueryDAO.find_by_id(query_id, skip_base_filter=True)
if query:
- security_manager.raise_for_access(query=query)
+ query.raise_for_explore_access()
Review Comment:
**Suggestion:** Use the centralized `security_manager.raise_for_access(...)`
authorization check for query resources instead of invoking a resource-specific
access method. [custom_rule_security]
**Severity Level:** Critical 🚨
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Queries are explicitly covered by the custom rule for data-bearing
resources, which requires using security_manager.raise_for_access(...) rather
than a resource-specific access method. This code calls
query.raise_for_explore_access() instead, so the violation is real.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
=== .github/copilot-instructions.md === (line 79)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=affe116dd8e84d3693ebd335a3d339db&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=affe116dd8e84d3693ebd335a3d339db&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/explore/utils.py
**Line:** 56:56
**Comment:**
*Custom Rule Security: Use the centralized
`security_manager.raise_for_access(...)` authorization check for query
resources instead of invoking a resource-specific access method.
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=3a840bc80c8c46c2f906fac384a01e0dbf6f383251e0a4ca29e6f6e49806c635&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=3a840bc80c8c46c2f906fac384a01e0dbf6f383251e0a4ca29e6f6e49806c635&reaction=dislike'>👎</a>
##########
superset/common/query_context.py:
##########
@@ -131,6 +150,21 @@ def get_df_payload(
force_cached=force_cached,
)
+ def get_df_payload_result(
+ self,
+ query_obj: QueryObject,
+ force_cached: bool | None = False,
+ source_kind: SourceKind = SourceKind.PRIMARY,
+ cache_key_extra: Mapping[str, Any] | None = None,
+ ) -> QueryAcquisitionResult:
+ """Acquire a dataframe with timing kept outside the payload."""
+ return self._processor.get_df_payload_result(
+ query_obj=query_obj,
+ force_cached=force_cached,
+ source_kind=source_kind,
+ cache_key_extra=cache_key_extra,
+ )
Review Comment:
**Suggestion:** Perform the required access validation before acquiring
dataframe payload results so this new data path cannot bypass authorization.
[custom_rule_security]
**Severity Level:** Critical 🚨
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This new query-context data path returns a dataframe acquisition result and
does so without an explicit raise_for_access() call in the shown code. Because
query contexts are in scope for the rule, the suggestion points to a real
violation.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
=== .github/copilot-instructions.md === (line 79)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=93fb94abad5c4393b8ead4c04c6c1524&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=93fb94abad5c4393b8ead4c04c6c1524&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/common/query_context.py
**Line:** 161:166
**Comment:**
*Custom Rule Security: Perform the required access validation before
acquiring dataframe payload results so this new data path cannot bypass
authorization.
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=5c061700a4cfe1fb62dbbdf4288cd7d0fc113fa3a9053980a7cecc41da626476&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=5c061700a4cfe1fb62dbbdf4288cd7d0fc113fa3a9053980a7cecc41da626476&reaction=dislike'>👎</a>
##########
superset/commands/explore/get.py:
##########
@@ -125,16 +126,17 @@ def run(self) -> Optional[dict[str, Any]]: # noqa: C901
datasource_name = datasource.name
if slc:
security_manager.raise_for_access(chart=slc)
+ elif isinstance(datasource, Query):
+ datasource.raise_for_explore_access()
Review Comment:
**Suggestion:** Replace the direct per-object access call with
`security_manager.raise_for_access(...)` so query access checks use the
centralized authorization pathway required for data-bearing resources.
[custom_rule_security]
**Severity Level:** Critical 🚨
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Query objects are explicitly covered by the rule as data-bearing resources,
and the code uses a custom per-object access method instead of
security_manager.raise_for_access(...). That is a real violation of the stated
authorization rule.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
=== .github/copilot-instructions.md === (line 79)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6bd0561039f24de28b0d3759ce986bea&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=6bd0561039f24de28b0d3759ce986bea&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/commands/explore/get.py
**Line:** 129:130
**Comment:**
*Custom Rule Security: Replace the direct per-object access call with
`security_manager.raise_for_access(...)` so query access checks use the
centralized authorization pathway required for data-bearing resources.
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=e2eddc97e830358ec540d2690d942f90e127345e0fb95ce100f10dc05ea5846f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=e2eddc97e830358ec540d2690d942f90e127345e0fb95ce100f10dc05ea5846f&reaction=dislike'>👎</a>
##########
superset/common/query_context.py:
##########
@@ -98,6 +104,19 @@ def get_payload(
"""Returns the query results with both metadata and data"""
return self._processor.get_payload(cache_query_context, force_cached)
+ def get_payload_result(
+ self,
+ cache_query_context: bool | None = False,
+ force_cached: bool = False,
+ materialize: bool = True,
+ ) -> QueryContextExecutionResult:
+ """Execute queries with timing retained in a typed sidecar."""
+ return self._processor.get_payload_result(
+ cache_query_context,
+ force_cached,
+ materialize,
+ )
Review Comment:
**Suggestion:** Add an explicit access-control check for the query context
before executing and returning payload results, using the standard
raise-for-access path. [custom_rule_security]
**Severity Level:** Critical 🚨
<details>
<summary><b>Why it matters? ⭐ </b></summary>
QueryContext is a data-bearing resource covered by the access-control rule,
and this new method returns execution results without calling
raise_for_access() first. The suggestion correctly identifies a real missing
access check in the shown code.
</details>
<details>
<summary><b>Rule source 📖 </b></summary>
=== .github/copilot-instructions.md === (line 79)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=98bd16e174204ada8ab8700f8bfe9bb6&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=98bd16e174204ada8ab8700f8bfe9bb6&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/common/query_context.py
**Line:** 114:118
**Comment:**
*Custom Rule Security: Add an explicit access-control check for the
query context before executing and returning payload results, using the
standard raise-for-access path.
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=2da335240ef90839b9c420c57dbc8127541d4a811ef33cda14b2c8cbfb423d15&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=2da335240ef90839b9c420c57dbc8127541d4a811ef33cda14b2c8cbfb423d15&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]
