codeant-ai-for-open-source[bot] commented on code in PR #41897:
URL: https://github.com/apache/superset/pull/41897#discussion_r3554822296
##########
superset/subjects/filters.py:
##########
@@ -176,6 +176,28 @@ def apply(self, query: Query, value: Optional[Any]) ->
Query:
return _apply_subject_list_filters(query)
+class SubjectAllTextFilter(BaseFilter): # pylint:
disable=too-few-public-methods
+ """ILIKE search across a subject's textual columns.
+
+ Used by the Subject REST API ``search_filters`` for free-text search.
+ """
+
+ name = _("All Text")
+ arg_name = "subject_all_text"
+
+ def apply(self, query: Query, value: Any) -> Query:
+ if not value:
+ return query
+ ilike_value = f"%{value}%"
+ return query.filter(
+ or_(
+ Subject.label.ilike(ilike_value),
+ Subject.secondary_label.ilike(ilike_value),
+ Subject.extra_search.ilike(ilike_value),
+ )
+ )
Review Comment:
**Suggestion:** `SubjectAllTextFilter` skips `_apply_subject_list_filters`,
so requests using this search path bypass `EXCLUDE_USERS_FROM_LISTS` and
`EXTRA_RELATED_QUERY_FILTERS`. That creates inconsistent behavior with other
subject-list filters and can expose subjects that should be filtered out. Apply
`_apply_subject_list_filters(...)` to both the empty-value and filtered query
paths. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Subject REST API can bypass EXTRA_RELATED_QUERY_FILTERS tenant
restrictions.
- ❌ EXCLUDE_USERS_FROM_LISTS subjects visible despite global exclusion
config.
- ⚠️ Inconsistent subject filtering between pickers and Subject API.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure EXCLUDE_USERS_FROM_LISTS or EXTRA_RELATED_QUERY_FILTERS so
certain subjects
should be hidden; these configs are read in _apply_excluded_users() and
_apply_extra_related_query_filters() at superset/subjects/filters.py:37–97.
2. Start Superset with this PR so SubjectRestApi is registered, as wired in
superset/initialization/__init__.py:196 importing
superset.subjects.api.SubjectRestApi.
3. Log in as an admin and call GET /api/v1/security/subject/ with a q
payload containing
filters like {"filters": [{"col": "label", "opr": "subject_all_text",
"value":
"subject_label1"}]}, matching test_get_list_subject_filter in
superset/tests/integration_tests/subjects/api_tests.py:11–29.
4. The request is handled by SubjectRestApi.get_list(), which applies
SubjectAllTextFilter
via search_filters = {"label": [SubjectAllTextFilter]} in
superset/subjects/api.py:113–123; inside SubjectAllTextFilter.apply() at
superset/subjects/filters.py:188–198 the code returns query.filter(...)
directly without
wrapping in _apply_subject_list_filters(...), so the
EXCLUDE_USERS_FROM_LISTS /
EXTRA_RELATED_QUERY_FILTERS predicates are never applied, unlike
FilterRelatedSubjects.apply() at superset/subjects/filters.py:165–176, and
subjects that
global filters are meant to hide remain visible in this search path.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=85a13f341ddd433b9a7fb67b1e90ad48&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=85a13f341ddd433b9a7fb67b1e90ad48&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/subjects/filters.py
**Line:** 189:198
**Comment:**
*Logic Error: `SubjectAllTextFilter` skips
`_apply_subject_list_filters`, so requests using this search path bypass
`EXCLUDE_USERS_FROM_LISTS` and `EXTRA_RELATED_QUERY_FILTERS`. That creates
inconsistent behavior with other subject-list filters and can expose subjects
that should be filtered out. Apply `_apply_subject_list_filters(...)` to both
the empty-value and filtered query paths.
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%2F41897&comment_hash=4c7b30af1ffc2eaac666d752a50b60d04a6de377fbb90b448d603458e97a21e5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41897&comment_hash=4c7b30af1ffc2eaac666d752a50b60d04a6de377fbb90b448d603458e97a21e5&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]
