chore(review): revert invalid auth and enhanced-audio heuristics - #252
Conversation
…dio codec masquerade - Treat on_connect_cb and on_shared_object_cb as implicit auth hooks so publish/play require explicit on_publish_cb/on_play_cb when configured - Reject ambiguous enhanced-audio headers (IsExHeader bit with printable but unrecognized FourCC) instead of falling back to legacy SoundFormat - Add regression tests for all three paths Co-authored-by: Alexander Wagner <info@alexanderwagnerdev.com>
|
@codex review |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b0ad82b2-95f8-44dc-a7e8-f43b024a7439) |
![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: 1a74d5515b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Restore the pre-PR conn.rs behavior after review found that observer callbacks are not authorization gates and printable legacy audio payload bytes cannot safely distinguish enhanced FourCC headers.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a70b5d0c-6ddc-456c-b173-0d3e3a35ff96) |
AlexanderWagnerDev
changed the title
2 participants
Review outcome
Follow-up review found that the proposed fixes introduced regressions and cannot safely implement the reported behavior as written.
Observer callbacks are not authorization hooks
on_connect_cbis a fire-and-forget notification callback and cannot approve or reject a connection.on_shared_object_cbis an event-delivery callback; shared-object authorization already has the separateon_shared_object_auth_cbhook.Printable legacy audio payload is not a safe FourCC discriminator
Legacy audio formats with the high SoundFormat bit set can legitimately have arbitrary printable bytes immediately after the first audio-header byte. Therefore bytes 1..5 cannot safely be classified as an enhanced-audio FourCC slot based on printability alone without rejecting valid G.711 μ-law, Speex, or legacy Opus payloads.
Change in this PR
The original changes were reverted in
7b821cea35e6fd8e73989458699ac0a5eecc0100, restoring the pre-PR behavior. The Codex review threads are resolved.Issues #249, #250, and #251 are intentionally not auto-closed by this PR and should be reassessed separately if a different API/protocol-level remediation is desired.