codeant-ai-for-open-source[bot] commented on code in PR #42438:
URL: https://github.com/apache/superset/pull/42438#discussion_r3670981118
##########
tests/unit_tests/views/test_base.py:
##########
@@ -315,3 +318,35 @@ def endpoint(self: BaseSupersetView) -> None:
formatted = template % tuple(args)
assert "5.0.0. Use the following API endpoint instead" in formatted
assert "5.0.0 . Use" not in formatted
+
+
+def test_language_pack_endpoint_is_public(app: "SupersetApp") -> None:
+ """The language pack endpoint must be accessible without authentication.
+
+ Translation data is non-sensitive and the embedded dashboard SPA needs to
+ fetch it with a bare ``fetch()`` (no guest-token header). Previously the
+ endpoint was protected by ``@has_access`` which caused a 302 redirect to
+ ``/login/`` for unauthenticated requests, silently breaking i18n in
+ embedded dashboards (issue #42433).
+
+ When the compiled catalog exists the endpoint returns 200 with JSON.
+ When it is missing the endpoint returns 404 (never a 302 to /login/).
+ """
+ with app.test_client() as client:
+ resp = client.get("/language_pack/en/")
+ # The endpoint must be reachable without authentication.
+ # In the test environment compiled catalogs may not exist, so
+ # a 404 is acceptable; a 302 to /login/ would indicate the old
+ # @has_access guard is still in place.
+ assert resp.status_code in (200, 404)
Review Comment:
**Suggestion:** The successful branch only checks the status code and never
verifies that the response is JSON containing a language catalog. An unrelated
200 response, HTML response, or empty payload would satisfy this test while
still breaking the frontend's `fetch()` and translation loading contract. [api
mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Invalid 200 responses can pass the endpoint test.
- ⚠️ `preamble.ts` translation loading contract remains unverified.
- ⚠️ JSON catalog regressions are detected only by embedded users.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Call `/language_pack/en/` through the unauthenticated client at
`tests/unit_tests/views/test_base.py:335-336`.
2. When the catalog exists, `Superset.language_pack()` at
`superset/views/core.py:926-930`
returns the file with `mimetype="application/json"`.
3. The test at `tests/unit_tests/views/test_base.py:341` checks only that
the status is
not 302; any 200 response, including an unrelated HTML response or an empty
body,
satisfies the assertion.
4. Change the handler to return an invalid 200 payload or wrong content
type; the test
still passes even though the frontend fetch in
`superset-frontend/src/preamble.ts:91-96`
cannot load a usable language catalog. The test should assert the successful
branch, JSON
content type, and expected catalog structure, while separately handling
environments where
the catalog is mocked or unavailable.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9dbdb15b8a744578aceb66fc460d0ee1&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=9dbdb15b8a744578aceb66fc460d0ee1&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:** tests/unit_tests/views/test_base.py
**Line:** 336:341
**Comment:**
*Api Mismatch: The successful branch only checks the status code and
never verifies that the response is JSON containing a language catalog. An
unrelated 200 response, HTML response, or empty payload would satisfy this test
while still breaking the frontend's `fetch()` and translation loading contract.
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%2F42438&comment_hash=53a7405d06d405799a1038256f2e1e0b9455f65b66580590061f42dbfac59f03&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42438&comment_hash=53a7405d06d405799a1038256f2e1e0b9455f65b66580590061f42dbfac59f03&reaction=dislike'>👎</a>
##########
tests/unit_tests/views/test_base.py:
##########
@@ -315,3 +318,35 @@ def endpoint(self: BaseSupersetView) -> None:
formatted = template % tuple(args)
assert "5.0.0. Use the following API endpoint instead" in formatted
assert "5.0.0 . Use" not in formatted
+
+
+def test_language_pack_endpoint_is_public(app: "SupersetApp") -> None:
+ """The language pack endpoint must be accessible without authentication.
+
+ Translation data is non-sensitive and the embedded dashboard SPA needs to
+ fetch it with a bare ``fetch()`` (no guest-token header). Previously the
+ endpoint was protected by ``@has_access`` which caused a 302 redirect to
+ ``/login/`` for unauthenticated requests, silently breaking i18n in
+ embedded dashboards (issue #42433).
+
+ When the compiled catalog exists the endpoint returns 200 with JSON.
+ When it is missing the endpoint returns 404 (never a 302 to /login/).
+ """
+ with app.test_client() as client:
+ resp = client.get("/language_pack/en/")
+ # The endpoint must be reachable without authentication.
+ # In the test environment compiled catalogs may not exist, so
+ # a 404 is acceptable; a 302 to /login/ would indicate the old
+ # @has_access guard is still in place.
+ assert resp.status_code in (200, 404)
+ assert resp.status_code != 302
+
+
+def test_language_pack_endpoint_rejects_invalid_lang(app: "SupersetApp") ->
None:
+ """Invalid language codes are rejected with 400."""
+ with app.test_client() as client:
+ resp = client.get("/language_pack/../../../etc/passwd/")
+ assert resp.status_code in (400, 404)
+
+ resp = client.get("/language_pack/en-US/injection/")
+ assert resp.status_code in (400, 404)
Review Comment:
**Suggestion:** These URLs contain extra path segments, so Flask can return
404 during routing before `language_pack` executes; because 404 is accepted,
the test does not verify that the handler rejects malformed language codes with
400. Pass malformed values within the single `<lang>` path segment and assert
the handler's intended status. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Invalid language-code handling is not actually tested.
- ⚠️ Route-level regressions can be mistaken for validation success.
- ⚠️ Path-validation coverage remains incomplete for the public endpoint.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. The route is declared as `/language_pack/<lang>/` at
`superset/views/core.py:919`, so
`<lang>` accepts one path segment only.
2. Request `/language_pack/../../../etc/passwd/` from
`tests/unit_tests/views/test_base.py:348`; the additional slash-separated
segments prevent
Flask routing from invoking `Superset.language_pack()`, so a 404 can occur
before the
validation at `superset/views/core.py:921-923`.
3. Request `/language_pack/en-US/injection/` from
`tests/unit_tests/views/test_base.py:351`; the extra `/injection/` segment
likewise
exercises routing rather than the handler's regular-expression validation.
4. Replace those requests with single-segment invalid values such as
`/language_pack/en-US/` and `/language_pack/en_US_extra/`; these reach
`language_pack()`
and should produce the handler's 400 response at
`superset/views/core.py:922-923`, whereas
the current assertions accept 404 and therefore cannot detect a validation
regression.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b1b92f96a3c544bda4990be87c602269&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=b1b92f96a3c544bda4990be87c602269&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:** tests/unit_tests/views/test_base.py
**Line:** 348:352
**Comment:**
*Api Mismatch: These URLs contain extra path segments, so Flask can
return 404 during routing before `language_pack` executes; because 404 is
accepted, the test does not verify that the handler rejects malformed language
codes with 400. Pass malformed values within the single `<lang>` path segment
and assert the handler's intended status.
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%2F42438&comment_hash=d9de72aecf0d9f7d79edfc82bef6226215533f65dfde100d62545f49ade34d8d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42438&comment_hash=d9de72aecf0d9f7d79edfc82bef6226215533f65dfde100d62545f49ade34d8d&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]
