codeant-ai-for-open-source[bot] commented on code in PR #42463:
URL: https://github.com/apache/superset/pull/42463#discussion_r3680393795
##########
superset/connectors/sqla/utils.py:
##########
@@ -109,13 +110,13 @@ def get_virtual_table_metadata(dataset: SqlaTable) ->
list[ResultSetColumnType]:
dataset.sql, **dataset.template_params_dict
)
except SupersetSyntaxErrorException as ex:
- raise SupersetGenericDBErrorException(
+ raise SupersetVirtualTableParseException(
message=_("Template processing error: %(error)s", error=str(ex)),
) from ex
Review Comment:
**Suggestion:** `process_template` converts `SecurityError` raised while
compiling a template into `SupersetSyntaxErrorException`, and this catch then
converts it into `SupersetVirtualTableParseException`, which
`RefreshDatasetCommand` deliberately swallows. Consequently, a Jinja template
rejected by the sandbox can make refresh return successfully instead of
surfacing the security failure. Preserve security/template-policy failures
separately and soften only the missing-runtime-context case. [security]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Sandbox-rejected templates can return successful refreshes.
- ⚠️ Template security-policy failures become warning-only events.
- ⚠️ Dataset metadata refresh may silently remain incomplete.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=78318564845c4543ac3deae35f3ff5e4&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=78318564845c4543ac3deae35f3ff5e4&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/connectors/sqla/utils.py
**Line:** 112:115
**Comment:**
*Security: `process_template` converts `SecurityError` raised while
compiling a template into `SupersetSyntaxErrorException`, and this catch then
converts it into `SupersetVirtualTableParseException`, which
`RefreshDatasetCommand` deliberately swallows. Consequently, a Jinja template
rejected by the sandbox can make refresh return successfully instead of
surfacing the security failure. Preserve security/template-policy failures
separately and soften only the missing-runtime-context case.
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%2F42463&comment_hash=01d8e62b3a0fe8cdeca17404c5d43ae3637353481436c6858120614fdbaf078e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42463&comment_hash=01d8e62b3a0fe8cdeca17404c5d43ae3637353481436c6858120614fdbaf078e&reaction=dislike'>👎</a>
##########
superset/connectors/sqla/utils.py:
##########
@@ -109,13 +110,13 @@ def get_virtual_table_metadata(dataset: SqlaTable) ->
list[ResultSetColumnType]:
dataset.sql, **dataset.template_params_dict
)
except SupersetSyntaxErrorException as ex:
- raise SupersetGenericDBErrorException(
+ raise SupersetVirtualTableParseException(
message=_("Template processing error: %(error)s", error=str(ex)),
) from ex
try:
parsed_script = SQLScript(sql, engine=db_engine_spec.engine)
except SupersetParseError as ex:
- raise SupersetGenericDBErrorException(
+ raise SupersetVirtualTableParseException(
message=_("Invalid SQL: %(error)s", error=ex.error.message),
) from ex
Review Comment:
**Suggestion:** The new exception is raised for every `SupersetParseError`,
including ordinary invalid static SQL with no Jinja dependency.
`RefreshDatasetCommand` treats this subclass as a successful best-effort
refresh, so a genuinely invalid virtual dataset can return success and the
direct dataset refresh endpoint can respond with 200 while leaving stale
columns. Only failures caused by unavailable template context should be
softened; static SQL parse failures must retain their previous hard-error
behavior. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Invalid virtual SQL can produce successful refresh responses.
- ⚠️ Dataset columns remain stale after failed parsing.
- ⚠️ Dataset Editor saves can hide static SQL errors.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=de8d1497e2e64119aca7753438027674&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=de8d1497e2e64119aca7753438027674&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/connectors/sqla/utils.py
**Line:** 118:121
**Comment:**
*Api Mismatch: The new exception is raised for every
`SupersetParseError`, including ordinary invalid static SQL with no Jinja
dependency. `RefreshDatasetCommand` treats this subclass as a successful
best-effort refresh, so a genuinely invalid virtual dataset can return success
and the direct dataset refresh endpoint can respond with 200 while leaving
stale columns. Only failures caused by unavailable template context should be
softened; static SQL parse failures must retain their previous hard-error
behavior.
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%2F42463&comment_hash=77bb574ce43afddd0e13c43205c4f182cc537612e0737d1971541db194d0a4ca&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42463&comment_hash=77bb574ce43afddd0e13c43205c4f182cc537612e0737d1971541db194d0a4ca&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]
