Mcp improvements#189
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8599167617
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
# Conflicts: # src/django_smartbase_admin/actions/admin_action_list.py
|
@codex re-review if all fixed |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f065e81fef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| slot = { | ||
| key_name: row[target] | ||
| for key_name, target in zip(group_keys, group_targets) | ||
| } |
There was a problem hiding this comment.
When a renamed group column's public name matches a derived aggregate alias, this initializes the slot with that group key and the later slot[alias] = row[alias] assignment overwrites it. For example, an admin method named count backed by @admin.display(ordering="id") with group_by=["count"] and aggregate=[{"fn": "count"}] now returns only the aggregate count under count, losing the group value entirely; before this change the group value was at least kept under the ORM target. Please detect/reject these collisions or namespace one side before merging rows.
Useful? React with 👍 / 👎.
|
@codex re-review fixes |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f065e81fef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| """ | ||
| requested = urlsplit(redirect_uri) | ||
| if requested.hostname not in LOOPBACK_HOSTS: | ||
| if requested.scheme != "http" or requested.hostname not in LOOPBACK_HOSTS: |
There was a problem hiding this comment.
When an unauthenticated DCR request registers a non-http URI such as javascript:alert(1), this fallback never runs for an exact authorize redirect because validate_redirect_uri() returns after DOT's exact-match path; the bundled register() endpoint only rejects non-loopback http and saves without full_clean(). This new check only blocks port-mismatch fallbacks, so the scheme allowlist remains bypassable unless schemes are validated during registration or before calling super().
Useful? React with 👍 / 👎.
3 participants
No description provided.