bito-code-review[bot] commented on code in PR #43827:
URL: https://github.com/apache/superset/pull/43827#discussion_r4042866374
##########
superset/commands/semantic_layer/update.py:
##########
@@ -173,4 +160,4 @@ def validate(self) -> None:
if configuration := self._properties.get("configuration"):
sl_type = self._model.type
cls = registry[sl_type]
- cls.from_configuration(configuration)
+ validate_configuration(cls, configuration)
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Unguarded registry KeyError</b></div>
<div id="fix">
The new `validate_configuration(cls, configuration)` call executes `cls =
registry[sl_type]` (line 162) with no membership guard. `registry`
(superset/semantic_layers/registry.py:24) only holds registered plugin classes;
a layer whose stored `type` is stale (plugin uninstalled/renamed) raises
KeyError, which the PUT handler (api.py:924-932) does not catch, so clients get
a 500. `CreateSemanticLayerCommand.validate` guards this via `sl_type not in
registry` (create.py:66-67); the update path should match.
</div>
</div>
<div id="suggestion">
<div id="issue"><b>Unsanitized non-ValidationError leak</b></div>
<div id="fix">
`validate_configuration` (utils.py:33-44) sanitizes only pydantic
`ValidationError`; any other exception from `layer_class.from_configuration`
propagates raw. The abstract contract (superset-core layer.py:40-49) does not
limit implementations to ValidationError, and this configuration comes straight
from the request body (api.py PUT -> UpdateSemanticLayerCommand). A plugin
raising ValueError/ConnectionError with config values in the message bypasses
the sanitization this change introduces and surfaces as a 500.
</div>
</div>
<small><i>Code Review Run #5aa19b</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
