codeant-ai-for-open-source[bot] commented on code in PR #44849:
URL: https://github.com/apache/superset/pull/44849#discussion_r4158417315
##########
superset/common/query_context_factory.py:
##########
@@ -114,6 +124,12 @@ def create( # pylint: disable=too-many-arguments
custom_cache_timeout=custom_cache_timeout,
cache_values=cache_values,
)
+ if defer_discovery:
+ security_manager.raise_for_access(query_context=context)
Review Comment:
**Suggestion:** Authorization runs before `_process_query_object` adds
`tooltip_contents` columns, so a guest can request an unapproved tooltip column
and receive it after the payload check.
**Assessment:** ๐ด `Critical` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Security`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1957e6dd00af4cf2bb85cd57db75b634&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=1957e6dd00af4cf2bb85cd57db75b634&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/common/query_context_factory.py
**Line:** 128:128
**Comment:**
*Security: Authorization runs before `_process_query_object` adds
`tooltip_contents` columns, so a guest can request an unapproved tooltip column
and receive it after the payload check.
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%2F44849&comment_hash=0d450f74961ad20b34dce4b0a0669bd62c11e7f3d7d10b04519d58114c14dc56&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44849&comment_hash=0d450f74961ad20b34dce4b0a0669bd62c11e7f3d7d10b04519d58114c14dc56&reaction=dislike'>๐</a>
##########
superset/semantic_layers/metadata_binding.py:
##########
@@ -0,0 +1,227 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Host construction and operation lifetime for optional shared metadata."""
+
+from __future__ import annotations
+
+import math
+import time
+from collections.abc import Iterator
+from contextlib import contextmanager
+from contextvars import ContextVar, Token
+from dataclasses import dataclass, field
+from typing import Any, TYPE_CHECKING
+
+from flask import current_app, has_app_context, has_request_context, request
+from sqlalchemy.orm import Session
+from superset_core.semantic_layers.layer import SemanticLayer as LayerABC
+from superset_core.semantic_layers.metadata import (
+ MetadataRefreshError,
+ remaining_budget,
+)
+from superset_core.semantic_layers.view import SemanticView as ViewABC
+
+from superset import db, is_feature_enabled
+from superset.coordination.deadline_backend import DeadlineRedisBackend
+from superset.semantic_layers.metadata import (
+ FETCH_DEADLINE_SECONDS,
+ metadata_scope,
+ ScopedMetadataStore,
+)
+from superset.semantic_layers.registry import registry
+from superset.utils import json
+
+if TYPE_CHECKING:
+ from superset.semantic_layers.models import SemanticLayer, SemanticView
+
+
+@dataclass
+class MetadataOperation:
+ """One request or worker operation; nested discovery shares its
deadline."""
+
+ deadline: float
+ layers: dict[str, LayerABC[Any, ViewABC]] = field(default_factory=dict)
+ views: dict[tuple[str, str, str], ViewABC] = field(default_factory=dict)
+ stores: dict[str, ScopedMetadataStore] = field(default_factory=dict)
+
+
+_OPERATION_KEY: str = "superset.semantic_metadata.operation"
+_worker_operation: ContextVar[MetadataOperation | None] = ContextVar(
+ _OPERATION_KEY, default=None
+)
+
+
+def request_metadata_budget() -> None:
+ """Register before authentication hooks; this performs no provider or
cache I/O."""
+ if current_app.config.get("SEMANTIC_LAYER_METADATA_REFRESH_ENABLED") is
True:
+ request.environ.setdefault(
+ _OPERATION_KEY, MetadataOperation(time.monotonic() +
FETCH_DEADLINE_SECONDS)
+ )
+
+
+def _current_operation() -> MetadataOperation | None:
+ if has_request_context():
+ return request.environ.get(_OPERATION_KEY) or _worker_operation.get()
+ return _worker_operation.get()
+
+
+def _operation(*, require_budget: bool = True) -> MetadataOperation:
+ state: MetadataOperation | None = _current_operation()
+ if state is None or not math.isfinite(state.deadline):
+ raise MetadataRefreshError("configuration")
+ if require_budget:
+ remaining_budget(state.deadline, now=time.monotonic())
+ return state
+
+
+def operation_deadline() -> float:
+ return _operation().deadline
+
+
+@contextmanager
+def metadata_operation(*, deadline: float | None = None) -> Iterator[None]:
+ """Workers opt in before access checks; nested calls never replenish the
budget."""
+ if deadline is not None and not math.isfinite(deadline):
+ raise MetadataRefreshError("configuration")
+ if deadline is not None:
+ remaining_budget(deadline, now=time.monotonic())
+ if _current_operation() is not None:
+ _operation(require_budget=False)
+ yield
+ return
+ if has_request_context():
+ # HTTP requests must enter through the registered early request hook.
+ raise MetadataRefreshError("configuration")
+ ceiling: float = time.monotonic() + FETCH_DEADLINE_SECONDS
+ state: MetadataOperation = MetadataOperation(
+ min(deadline, ceiling) if deadline is not None else ceiling
+ )
+ token: Token[MetadataOperation | None] = _worker_operation.set(state)
+ try:
+ operation_deadline()
+ yield
+ finally:
+ _worker_operation.reset(token)
+
+
+def metadata_refresh_enabled() -> bool:
+ return (
+ has_app_context()
+ and current_app.config.get("SEMANTIC_LAYER_METADATA_REFRESH_ENABLED")
is True
+ and is_feature_enabled("SEMANTIC_LAYERS")
+ )
+
+
+def participates(layer: SemanticLayer) -> bool:
+ return metadata_refresh_enabled() and registry[
+ layer.type
+ ].supports_metadata_refresh(json.loads(layer.configuration))
Review Comment:
**Suggestion:** `participates` lets `KeyError` and JSON parsing errors
escape for unknown providers or malformed configurations, producing 500 errors
instead of stable metadata configuration responses.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Api mismatch`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=76d5b3878a6c4e69b9f0b3edfa1a8f0b&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=76d5b3878a6c4e69b9f0b3edfa1a8f0b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/semantic_layers/metadata_binding.py
**Line:** 131:133
**Comment:**
*Api Mismatch: `participates` lets `KeyError` and JSON parsing errors
escape for unknown providers or malformed configurations, producing 500 errors
instead of stable metadata configuration responses.
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%2F44849&comment_hash=930d417fdaefb4f1ccc99ab73fd1809906207313f342a06ea7f68c9327e9fa8f&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44849&comment_hash=930d417fdaefb4f1ccc99ab73fd1809906207313f342a06ea7f68c9327e9fa8f&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]
