codeant-ai-for-open-source[bot] commented on code in PR #41430:
URL: https://github.com/apache/superset/pull/41430#discussion_r3609214999
##########
superset/semantic_layers/api.py:
##########
@@ -863,6 +866,8 @@ def put(self, uuid: str) -> FlaskResponse:
return self.response(200, result={"uuid": str(changed_model.uuid)})
except SemanticLayerNotFoundError:
return self.response_404()
+ except SemanticLayerForbiddenError:
+ return self.response_403()
Review Comment:
**Suggestion:** This adds a new 403 response path for update requests, but
the endpoint contract/docs for this handler were not updated to advertise 403.
Clients generated from the OpenAPI contract can mis-handle these responses as
unexpected failures; add the 403 response definition to keep runtime behavior
and API contract aligned. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
❌ Update requests forbidden but 403 absent from OpenAPI.
⚠️ Generated clients treat 403 as unexpected runtime error.
⚠️ API documentation inconsistent with actual response behavior.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. As a user without `datasource_access` to a semantic layer, issue an
authenticated HTTP
PUT to `/api/v1/semantic_layer/<uuid>` with a valid JSON body matching
`SemanticLayerPutSchema` (used via `edit_model_schema =
SemanticLayerPutSchema()` at
`superset/semantic_layers/api.py:553-554`).
2. The request is handled by `SemanticLayerRestApi.put` at
`superset/semantic_layers/api.py:817-880`, which loads the body with
`self.edit_model_schema.load(request.json)` and then calls
`UpdateSemanticLayerCommand(uuid, item).run()`.
3. Inside `UpdateSemanticLayerCommand.validate` at
`superset/commands/semantic_layer/update.py:115-123`,
`self._model.raise_for_access()`
invokes `SemanticLayer.raise_for_access` at
`superset/semantic_layers/models.py:151-169`;
because the user lacks required `datasource_access`,
`SupersetSecurityException` is raised
and caught, then re-raised as `SemanticLayerForbiddenError`.
4. Control returns to the API handler and hits the new `except
SemanticLayerForbiddenError:` block at
`superset/semantic_layers/api.py:869-870`,
returning `self.response_403()`, while the OpenAPI responses block for this
method at
`superset/semantic_layers/api.py:847-857` documents only 200, 400, 401, 404,
and 422,
leaving 403 undocumented for API consumers and generated clients.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5e731b1ab4a040f688904a27fab0a36a&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=5e731b1ab4a040f688904a27fab0a36a&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/semantic_layers/api.py
**Line:** 869:870
**Comment:**
*Api Mismatch: This adds a new 403 response path for update requests,
but the endpoint contract/docs for this handler were not updated to advertise
403. Clients generated from the OpenAPI contract can mis-handle these responses
as unexpected failures; add the 403 response definition to keep runtime
behavior and API contract aligned.
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%2F41430&comment_hash=537ee5cb9a17b3d1e7b846e32cd5097d8fba1c0288f1a2dd841c9c048eebe511&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41430&comment_hash=537ee5cb9a17b3d1e7b846e32cd5097d8fba1c0288f1a2dd841c9c048eebe511&reaction=dislike'>👎</a>
##########
superset/semantic_layers/api.py:
##########
@@ -902,6 +907,8 @@ def delete(self, uuid: str) -> FlaskResponse:
return self.response(200, message="OK")
except SemanticLayerNotFoundError:
return self.response_404()
+ except SemanticLayerForbiddenError:
+ return self.response_403()
Review Comment:
**Suggestion:** This adds a new 403 response path for delete requests, but
the endpoint contract/docs for this handler were not updated to include 403.
That creates a behavior-contract mismatch for API consumers; document the
forbidden response in the route spec to match actual runtime behavior. [api
mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
❌ Delete requests forbidden yet 403 undocumented in spec.
⚠️ Client libraries mis-handle forbidden deletes on semantic layers.
⚠️ Contract drift complicates troubleshooting authorization failures.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. As a user who lacks `datasource_access` to a particular semantic layer,
send an
authenticated HTTP DELETE request to `/api/v1/semantic_layer/<uuid>`
targeting an existing
layer.
2. The request is processed by `SemanticLayerRestApi.delete` at
`superset/semantic_layers/api.py:881-918`, which calls
`DeleteSemanticLayerCommand(uuid).run()` inside a try block after the route
docstring that
documents responses (200, 401, 404, 422) at
`superset/semantic_layers/api.py:888-903`.
3. In `DeleteSemanticLayerCommand.validate` at
`superset/commands/semantic_layer/delete.py:59-67`,
`self._model.raise_for_access()` calls
`SemanticLayer.raise_for_access` at
`superset/semantic_layers/models.py:151-169`; lacking
access triggers `SupersetSecurityException`, which is caught and re-thrown as
`SemanticLayerForbiddenError`.
4. Back in the API handler, execution enters the new `except
SemanticLayerForbiddenError:`
block at `superset/semantic_layers/api.py:910-911`, returning
`self.response_403()`, even
though the delete endpoint’s OpenAPI responses list does not include 403,
causing a
mismatch between documented and actual behavior for clients and tooling.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=72f88b6a7c3c4c3a8fd16b091a81ac44&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=72f88b6a7c3c4c3a8fd16b091a81ac44&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/semantic_layers/api.py
**Line:** 910:911
**Comment:**
*Api Mismatch: This adds a new 403 response path for delete requests,
but the endpoint contract/docs for this handler were not updated to include
403. That creates a behavior-contract mismatch for API consumers; document the
forbidden response in the route spec to match actual runtime 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%2F41430&comment_hash=6261339e88dec7f26b4a2e84ba492aee631eee2dc408a3c192fb324c47e9d939&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41430&comment_hash=6261339e88dec7f26b4a2e84ba492aee631eee2dc408a3c192fb324c47e9d939&reaction=dislike'>👎</a>
##########
superset/semantic_layers/api.py:
##########
@@ -1148,7 +1155,15 @@ def get(self, uuid: str) -> FlaskResponse:
404:
$ref: '#/components/responses/404'
"""
+ if not is_feature_enabled("SEMANTIC_LAYERS"):
Review Comment:
**Suggestion:** This new feature-flag guard is only applied on the
single-object GET endpoint, while the list endpoint in the same API still
serves semantic layers without the same guard. That creates an inconsistent API
state when the flag is off (objects can be listed but not fetched
individually), which breaks normal list→detail flows. Apply the same flag
behavior consistently across related layer endpoints. [incomplete
implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
⚠️ Semantic layer list endpoint accessible when flag disabled.
⚠️ Detail endpoint returns 404 despite list succeeding.
⚠️ Breaks list-detail navigation flows in semantic layers UI.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure Superset with the `SEMANTIC_LAYERS` feature flag disabled so
`is_feature_enabled("SEMANTIC_LAYERS")` (imported at
`superset/semantic_layers/api.py:31`)
returns False for the current request context.
2. Send an authenticated GET request to `/api/v1/semantic_layer/` which is
implemented by
`SemanticLayerRestApi.get_list` at
`superset/semantic_layers/api.py:1117-1134`; this
handler calls `SemanticLayerDAO.find_all()` and returns `self.response(200,
result=...)`
without checking the `SEMANTIC_LAYERS` feature flag.
3. Send an authenticated GET request to `/api/v1/semantic_layer/<uuid>`
which is
implemented by `SemanticLayerRestApi.get` at
`superset/semantic_layers/api.py:1136-1169`;
execution reaches lines `1157-1158` where `if not
is_feature_enabled("SEMANTIC_LAYERS"):`
immediately returns `self.response_404()`.
4. Observe that with the feature flag off, the list endpoint still responds
with HTTP 200
(even if the list is empty) while the detail endpoint always returns 404,
creating
inconsistent feature-gating and breaking normal list→detail flows for
semantic layers.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2009518bdbd04c05aa07d7674442102f&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=2009518bdbd04c05aa07d7674442102f&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/semantic_layers/api.py
**Line:** 1157:1158
**Comment:**
*Incomplete Implementation: This new feature-flag guard is only applied
on the single-object GET endpoint, while the list endpoint in the same API
still serves semantic layers without the same guard. That creates an
inconsistent API state when the flag is off (objects can be listed but not
fetched individually), which breaks normal list→detail flows. Apply the same
flag behavior consistently across related layer endpoints.
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%2F41430&comment_hash=5962110de0ec1305bd3fe2d9dbcb3a09a16e5de22f3a73a5ee8c3005711de182&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41430&comment_hash=5962110de0ec1305bd3fe2d9dbcb3a09a16e5de22f3a73a5ee8c3005711de182&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]
