codeant-ai-for-open-source[bot] commented on code in PR #40649:
URL: https://github.com/apache/superset/pull/40649#discussion_r3337916746
##########
superset/initialization/__init__.py:
##########
@@ -691,6 +695,32 @@ def check_guest_token_secret(self) -> None:
)
sys.exit(1)
+ def check_async_query_secret(self) -> None:
+ """Refuse to start with the default async JWT secret when GAQ is
enabled."""
+ if not feature_flag_manager.is_feature_enabled("GLOBAL_ASYNC_QUERIES"):
+ return
+ if (
+ self.config.get("GLOBAL_ASYNC_QUERIES_JWT_SECRET")
+ != CHANGE_ME_GLOBAL_ASYNC_QUERIES_JWT_SECRET
+ ):
+ return
+ self._log_config_warning(
+ "GLOBAL_ASYNC_QUERIES is enabled but
GLOBAL_ASYNC_QUERIES_JWT_SECRET "
+ "has not been changed from its default value.\n"
+ "The default value is publicly known and must be replaced before "
+ "running in production.\n"
+ "Set a strong random value (at least 32 bytes) in
superset_config.py:\n"
+ " GLOBAL_ASYNC_QUERIES_JWT_SECRET = "
+ "'<output of: openssl rand -base64 42>'"
+ )
+ if self.superset_app.debug or self.superset_app.config["TESTING"] or
is_test():
+ return
Review Comment:
**Suggestion:** Returning early in debug/testing mode does not actually
preserve startup, because the same default secret still reaches
`configure_async_queries()` later in initialization and triggers
`AsyncQueryManager.init_app()`'s length check, which raises and aborts app
startup. Make this check consistent with downstream behavior by either
enforcing a valid non-default secret before continuing when
`GLOBAL_ASYNC_QUERIES` is enabled, or by ensuring async-query initialization is
skipped in these non-production modes when the default secret is present.
[incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Debug deployments with GAQ enabled crash during app initialization.
- ⚠️ Integration tests enabling GAQ cannot rely on warning-only behavior.
- ⚠️ Security check semantics differ between async and guest secrets.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Leave the async JWT secret at its default by using the shipped config
where
`GLOBAL_ASYNC_QUERIES_JWT_SECRET =
CHANGE_ME_GLOBAL_ASYNC_QUERIES_JWT_SECRET` in
`superset/config.py:2320–2339` and
`CHANGE_ME_GLOBAL_ASYNC_QUERIES_JWT_SECRET =
"test-secret-change-me"` in `superset/constants.py:32`, and enable the
`GLOBAL_ASYNC_QUERIES` feature flag so that
`feature_flag_manager.is_feature_enabled("GLOBAL_ASYNC_QUERIES")` returns
`True` (used in
`superset/initialization/__init__.py:31` and `:1043–1045`).
2. Start Superset via `superset.app.create_app()` (`superset/app.py:9–39`)
in a
non-production environment where `app.debug` is `True`,
`app.config["TESTING"]` is
`False`, and `superset.initialization.is_test()` returns `False` (mirroring
the debug-only
behavior tested in
`tests/unit_tests/initialization/check_async_query_secret_test.py:72–86` but
exercising
the full initializer instead of the unit-tested method).
3. During `create_app`, the `SupersetAppInitializer` is constructed and
`init_app()` is
called (`superset/app.py:38–39`), which in turn invokes
`check_async_query_secret()` after
feature flags are configured (`superset/initialization/__init__.py:795–809);
with
`GLOBAL_ASYNC_QUERIES` enabled and the default secret present, the method
logs a warning
and then returns early at the debug/testing guard
(`superset/initialization/__init__.py:698–717`, including the line `716 if
self.superset_app.debug or self.superset_app.config["TESTING"] or
is_test():`), so
initialization continues instead of exiting.
4. Later in the same `init_app()` call, the initializer enters the app
context and calls
`init_app_in_ctx()` (`superset/initialization/__init__.py:820–828`), which
calls
`configure_async_queries()` (`superset/initialization/__init__.py:16–24);
because
`GLOBAL_ASYNC_QUERIES` is enabled, this calls
`async_query_manager_factory.init_app(self.superset_app)`
(`superset/initialization/__init__.py:1043–1045`), which instantiates
`AsyncQueryManager`
and invokes its `init_app`
(`superset/async_events/async_query_manager_factory.py:9–13`);
inside `AsyncQueryManager.init_app`, the line `if
len(app.config["GLOBAL_ASYNC_QUERIES_JWT_SECRET"]) < 32:` raises
`AsyncQueryTokenException` for the default 21-byte secret
(`superset/async_events/async_query_manager.py:20–37`), bubbling up through
`create_app`'s
try/except (`superset/app.py:15–57`) and causing app startup to fail even in
debug/testing, so the early return in `check_async_query_secret` does not
actually
preserve startup when the default async JWT secret is used.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7263f9090ff74c73a0548bd85385d451&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=7263f9090ff74c73a0548bd85385d451&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/initialization/__init__.py
**Line:** 716:717
**Comment:**
*Incomplete Implementation: Returning early in debug/testing mode does
not actually preserve startup, because the same default secret still reaches
`configure_async_queries()` later in initialization and triggers
`AsyncQueryManager.init_app()`'s length check, which raises and aborts app
startup. Make this check consistent with downstream behavior by either
enforcing a valid non-default secret before continuing when
`GLOBAL_ASYNC_QUERIES` is enabled, or by ensuring async-query initialization is
skipped in these non-production modes when the default secret is present.
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%2F40649&comment_hash=18440e33a693d2c2c3b34953ebfecd8bf9451d3bdd81be7d98934327c86f9afd&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40649&comment_hash=18440e33a693d2c2c3b34953ebfecd8bf9451d3bdd81be7d98934327c86f9afd&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]
