codeant-ai-for-open-source[bot] commented on code in PR #40239:
URL: https://github.com/apache/superset/pull/40239#discussion_r3264840074
##########
superset/initialization/__init__.py:
##########
@@ -987,7 +987,7 @@ def enable_profiling(self) -> None:
class SupersetIndexView(IndexView):
@expose("/")
def index(self) -> FlaskResponse:
- return redirect(url_for("Superset.welcome"))
+ return redirect("/dashboard/list/")
Review Comment:
**Suggestion:** The root index now hard-redirects to an absolute
`"/dashboard/list/"`, which bypasses `APPLICATION_ROOT`/`SUPERSET_APP_ROOT`
handling. In subdirectory deployments (for example `/superset`), this sends
users to a URL outside the mounted prefix and can produce 404s via
`AppRootMiddleware`. Build the redirect with a route helper
(`url_for`/AppBuilder URL helper) so the app root prefix is preserved. [logic
error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Root URL 404s when APP_ROOT prefix is configured.
- ⚠️ Breaks navigation for subdirectory deployments using AppRootMiddleware.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure Superset to run under a non-root app root by calling
`create_app(superset_app_root="/superset")`, as exercised in
`tests/unit_tests/initialization_test.py:61-69` where `AppRootMiddleware` is
attached with
`app_root` and `PATH_INFO` must start with that prefix.
2. Start the app and browse to the external root URL `/superset/`;
`AppRootMiddleware.__call__` in `superset/app.py:80-102` strips the
`/superset` prefix
from `PATH_INFO` and forwards the request with `PATH_INFO="/"` into the
Flask app.
3. The root route is handled by `SupersetIndexView.index` in
`superset/initialization/__init__.py:987-991`, which for `@expose("/")`
executes `return
redirect("/dashboard/list/")`, emitting a 302 Location header pointing to
the absolute
path `/dashboard/list/` (no `/superset` prefix).
4. The browser follows the redirect to `/dashboard/list/`;
`AppRootMiddleware.__call__` in
`superset/app.py:95-104` now receives `PATH_INFO="/dashboard/list/"`, which
does not start
with `app_root="/superset"`, so it returns `NotFound()`, producing an HTTP
404 instead of
the dashboard list page for any subdirectory deployment.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eb5c84ac2ab14742829ff901289dafb9&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=eb5c84ac2ab14742829ff901289dafb9&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:** 990:990
**Comment:**
*Logic Error: The root index now hard-redirects to an absolute
`"/dashboard/list/"`, which bypasses `APPLICATION_ROOT`/`SUPERSET_APP_ROOT`
handling. In subdirectory deployments (for example `/superset`), this sends
users to a URL outside the mounted prefix and can produce 404s via
`AppRootMiddleware`. Build the redirect with a route helper
(`url_for`/AppBuilder URL helper) so the app root prefix is preserved.
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%2F40239&comment_hash=4e5eda188642193190cbb7ca3340161653200c9442fe0aa0c7c00b07a514d443&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40239&comment_hash=4e5eda188642193190cbb7ca3340161653200c9442fe0aa0c7c00b07a514d443&reaction=dislike'>👎</a>
##########
superset/views/base.py:
##########
@@ -282,7 +281,7 @@ def menu_data(user: User) -> dict[str, Any]:
return {
"menu": appbuilder.menu.get_data(),
"brand": {
- "path": app.config["LOGO_TARGET_PATH"] or
url_for("Superset.welcome"),
+ "path": app.config["LOGO_TARGET_PATH"] or "/dashboard/list/",
Review Comment:
**Suggestion:** Forcing a fallback to `"/dashboard/list/"` changes the
documented contract of `LOGO_TARGET_PATH` (default `None` should route to
welcome) and bypasses the existing welcome-page flow. This can send users
without dashboard-list permissions to an unauthorized landing path and removes
the expected default behavior; keep the fallback aligned with the configured
welcome target instead of dashboard list. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Logo default target conflicts with documented LOGO_TARGET_PATH behavior.
- ❌ Logo link breaks under subdirectory AppRootMiddleware deployments.
- ⚠️ Users lose personalized welcome landing page experience.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run Superset with the default configuration where `LOGO_TARGET_PATH =
None` as defined
and documented in `superset/config.py:22-26`, which states "Default value of
None will
take you to '/superset/welcome'".
2. Render any SPA page that uses `common_bootstrap_payload`, such as the
personalized
welcome page `Superset.welcome` exposed at `/welcome/` in
`superset/views/core.py:932-943`, which calls `common_bootstrap_payload()`
from
`superset/views/base.py:31-35`.
3. Inside `common_bootstrap_payload`, `cached_common_bootstrap_data` is
invoked and builds
`bootstrap_data` including `"menu_data": menu_data(g.user)` at
`superset/views/base.py:520-545`; `menu_data` in
`superset/views/base.py:269-36` computes
`"brand": {"path": app.config["LOGO_TARGET_PATH"] or "/dashboard/list/",
...}`, so with
`LOGO_TARGET_PATH` set to `None` the effective logo target becomes
`"/dashboard/list/"`
instead of the documented `/superset/welcome`.
4. The SPA receives this `menu_data` via the `bootstrap_data` payload
(`"menu_data":
menu_data(g.user)` at `superset/views/base.py:540-545`) and uses
`brand["path"]` for the
application logo link; with default config the logo now navigates to
`/dashboard/list/`,
breaking the documented contract in `superset/config.py:22-24`, and in
subdirectory
deployments the absolute `/dashboard/list/` path further bypasses the
`AppRootMiddleware`
prefix in `superset/app.py:80-102`, leading to a 404 for logo clicks.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0820637282044edda09c3c0bd7c097f7&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=0820637282044edda09c3c0bd7c097f7&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/base.py
**Line:** 284:284
**Comment:**
*Api Mismatch: Forcing a fallback to `"/dashboard/list/"` changes the
documented contract of `LOGO_TARGET_PATH` (default `None` should route to
welcome) and bypasses the existing welcome-page flow. This can send users
without dashboard-list permissions to an unauthorized landing path and removes
the expected default behavior; keep the fallback aligned with the configured
welcome target instead of dashboard list.
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%2F40239&comment_hash=fa169a0fec3a85d99ae8079b1d2a4d8bc08a8d0b15e28cec11f59e0a0cd8d96d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40239&comment_hash=fa169a0fec3a85d99ae8079b1d2a4d8bc08a8d0b15e28cec11f59e0a0cd8d96d&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]
