codeant-ai-for-open-source[bot] commented on code in PR #40652:
URL: https://github.com/apache/superset/pull/40652#discussion_r3337916975
##########
superset/views/health.py:
##########
@@ -37,9 +37,19 @@ def health() -> FlaskResponse:
@talisman(force_https=False)
def version() -> FlaskResponse:
"""
- Return comprehensive version information including Git SHA
- and branch when available.
+ Return version information for the running Superset instance.
+
+ When ``EXPOSE_VERSION_INFO`` is True (default) this returns the full
+ version metadata, including the Git SHA and branch name when available.
+ When it is False, only the human-readable version string is returned and
+ build-specific details (Git SHA, full SHA, build number, branch name) are
+ omitted so they are not exposed to unauthenticated callers.
"""
from superset.utils.version import get_version_metadata
- return jsonify(get_version_metadata())
+ metadata = get_version_metadata()
+
+ if not app.config.get("EXPOSE_VERSION_INFO", True):
+ metadata = {"version_string": metadata.get("version_string",
"unknown")}
Review Comment:
**Suggestion:** The endpoint always calls `get_version_metadata()` before
checking `EXPOSE_VERSION_INFO`, so even when redaction is disabled it still
performs expensive metadata collection (including potential `git` subprocess
calls via `get_version_metadata`). This defeats the intended hardening behavior
and keeps the unauthenticated `/version` path vulnerable to unnecessary
resource usage. Check `EXPOSE_VERSION_INFO` first, and when it is `False`
return `version_string` directly from config without calling
`get_version_metadata()`. [performance]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Unauthenticated `/version` still invokes Git subprocess calls.
- ⚠️ Extra CPU/IO on frequent health or monitoring probes.
- ⚠️ Increases DoS surface of unauthenticated `/version` endpoint.
- ⚠️ Undermines config intent to reduce exposed build detail.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure Superset with `EXPOSE_VERSION_INFO = False` (overriding the
default `True`
defined in `superset/config.py:10-16`) so that the deployment opts in to
redacting build
details on `/version`.
2. Start the Superset application and issue an unauthenticated HTTP GET
request to
`/version`, which is routed through the `health_blueprint` defined at
`superset/views/health.py:23` and the `version()` view at
`superset/views/health.py:36-38`.
3. Observe that `version()` unconditionally executes `metadata =
get_version_metadata()`
at `superset/views/health.py:50`, which calls `get_version_metadata()` in
`superset/utils/version.py:5-33`; this function reads environment variables
and, if
needed, derives Git metadata including branch name via `_get_local_branch()`
at
`superset/utils/version.py:67-77`, which runs
`subprocess.check_output(["git",
"rev-parse", "--abbrev-ref", "HEAD"], ...)`.
4. Only after all metadata computation completes does `version()` check `if
not
app.config.get("EXPOSE_VERSION_INFO", True):` at
`superset/views/health.py:52-53` and then
discard the extra fields, returning just `{"version_string": ...}`; thus,
even with
`EXPOSE_VERSION_INFO = False`, each unauthenticated `/version` request still
incurs the
cost and potential resource usage of Git subprocess execution instead of
short-circuiting
to `app.config["VERSION_STRING"]`.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8473bf0a54864f869cf06b1e492729ee&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=8473bf0a54864f869cf06b1e492729ee&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/views/health.py
**Line:** 50:53
**Comment:**
*Performance: The endpoint always calls `get_version_metadata()` before
checking `EXPOSE_VERSION_INFO`, so even when redaction is disabled it still
performs expensive metadata collection (including potential `git` subprocess
calls via `get_version_metadata`). This defeats the intended hardening behavior
and keeps the unauthenticated `/version` path vulnerable to unnecessary
resource usage. Check `EXPOSE_VERSION_INFO` first, and when it is `False`
return `version_string` directly from config without calling
`get_version_metadata()`.
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%2F40652&comment_hash=3c46a06ce716515e9cd3bbabc62f5f26aca48f87585a1d6fac4c42dcbefaad7d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40652&comment_hash=3c46a06ce716515e9cd3bbabc62f5f26aca48f87585a1d6fac4c42dcbefaad7d&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]
