codeant-ai-for-open-source[bot] commented on code in PR #45752:
URL: https://github.com/apache/superset/pull/45752#discussion_r4383896822
##########
superset/commands/utils.py:
##########
@@ -123,28 +127,40 @@ def populate_subject_list(
return [user_subject] if user_subject else []
return []
+ # Resolve typed references before applying lockout checks. A user ID must
+ # never fall back to the Subject with the same numeric primary key.
+ for reference in subject_ids:
+ if not isinstance(reference, dict):
+ subject = get_subject(reference)
+ else:
+ resolver = {
+ "user": get_user_subject,
+ "role": get_or_create_role_subject,
Review Comment:
**Suggestion:** When two writes resolve the same unsynced role concurrently,
both can pass the lookup and insert; the unique `role_id` then makes one
request fail with an uncaught integrity error.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Race condition`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=01525cc9367849d7b38d56727e7ea0dc&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=01525cc9367849d7b38d56727e7ea0dc&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/utils.py
**Line:** 138:138
**Comment:**
*Race Condition: When two writes resolve the same unsynced role
concurrently, both can pass the lookup and insert; the unique `role_id` then
makes one request fail with an uncaught integrity error.
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%2F45752&comment_hash=e42dc8f5f8e4aacb82ccfacabb935fe8ef680b5e3fd565cc9039708a3e65b23e&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45752&comment_hash=e42dc8f5f8e4aacb82ccfacabb935fe8ef680b5e3fd565cc9039708a3e65b23e&reaction=dislike'>๐</a>
##########
superset/commands/utils.py:
##########
@@ -123,28 +127,40 @@ def populate_subject_list(
return [user_subject] if user_subject else []
return []
+ # Resolve typed references before applying lockout checks. A user ID must
+ # never fall back to the Subject with the same numeric primary key.
+ for reference in subject_ids:
+ if not isinstance(reference, dict):
+ subject = get_subject(reference)
+ else:
+ resolver = {
+ "user": get_user_subject,
+ "role": get_or_create_role_subject,
+ "group": get_or_create_group_subject,
+ "subject": get_subject,
+ }.get(reference["type"])
+ subject = resolver(reference["id"]) if resolver else None
+ if not subject:
+ raise SubjectsNotFoundValidationError(field_name)
+ subjects.append(subject)
Review Comment:
**Suggestion:** A bare Subject ID and an equivalent explicit `subject`
reference resolve to the same row, but both are appended; the unique asset
association then makes the write fail.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Logic error`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=7cd8e0d5a9ea4c209ba86096f869df72&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=7cd8e0d5a9ea4c209ba86096f869df72&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/utils.py
**Line:** 145:145
**Comment:**
*Logic Error: A bare Subject ID and an equivalent explicit `subject`
reference resolve to the same row, but both are appended; the unique asset
association then makes the write fail.
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%2F45752&comment_hash=d214476dac07de8ccf89ab31e583f1a6a3e0be881b22587c17d0219ccda71533&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45752&comment_hash=d214476dac07de8ccf89ab31e583f1a6a3e0be881b22587c17d0219ccda71533&reaction=dislike'>๐</a>
##########
superset/commands/utils.py:
##########
@@ -123,28 +127,40 @@ def populate_subject_list(
return [user_subject] if user_subject else []
return []
+ # Resolve typed references before applying lockout checks. A user ID must
+ # never fall back to the Subject with the same numeric primary key.
+ for reference in subject_ids:
+ if not isinstance(reference, dict):
+ subject = get_subject(reference)
+ else:
+ resolver = {
+ "user": get_user_subject,
+ "role": get_or_create_role_subject,
+ "group": get_or_create_group_subject,
+ "subject": get_subject,
+ }.get(reference["type"])
+ subject = resolver(reference["id"]) if resolver else None
+ if not subject:
+ raise SubjectsNotFoundValidationError(field_name)
+ subjects.append(subject)
+
+ resolved_ids = {subject.id for subject in subjects}
if ensure_no_lockout and not security_manager.is_admin() and user_id:
user_subject = get_user_subject(user_id)
if (
user_subject
- and user_subject.id not in subject_ids
+ and user_subject.id not in resolved_ids
and not _has_extra_editors_resolver()
):
user_subject_ids = set(get_user_subject_ids(user_id))
- if not (user_subject_ids & set(subject_ids)):
- subjects.append(user_subject)
-
- for sid in subject_ids:
- subject = get_subject(sid)
- if not subject:
- raise SubjectsNotFoundValidationError(field_name)
- subjects.append(subject)
+ if not (user_subject_ids & resolved_ids):
+ subjects.insert(0, user_subject)
Review Comment:
**Suggestion:** `get_user_subject_ids` can be cached during edit
authorization before an unsynced role or group is created; this check misses
that grant and adds the caller's user Subject too.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Cache`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f3ae635a9e794d94870927edfbd6b8fb&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=f3ae635a9e794d94870927edfbd6b8fb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/utils.py
**Line:** 156:157
**Comment:**
*Cache: `get_user_subject_ids` can be cached during edit authorization
before an unsynced role or group is created; this check misses that grant and
adds the caller's user Subject too.
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%2F45752&comment_hash=aa07199aea29f13de8bd5237788fcd7cb78795958462861aa028d3b3aa70db29&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45752&comment_hash=aa07199aea29f13de8bd5237788fcd7cb78795958462861aa028d3b3aa70db29&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]
