Skip to content

Ensure that transfer requests are properly authorized. - #797

Open
alexfish8 wants to merge 1 commit into
mainfrom
afish/authorize-transfers
Open

Ensure that transfer requests are properly authorized.#797
alexfish8 wants to merge 1 commit into
mainfrom
afish/authorize-transfers

Conversation

@alexfish8

@alexfish8 alexfish8 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment thread pkg/sip/service.go
Comment on lines +507 to +511
if req.GetRoomName() == "" && req.GetParticipantIdentity() == "" {
// Skip performing this authorization check against older clients.
// TODO: Remove this branch after clients have been updated to set these fields.
return nil
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟨 Transfer authorization is skipped entirely when the request omits room and participant

The new authorization check in pkg/sip/service.go:507-511 bypasses validation whenever both RoomName and ParticipantIdentity are empty in the transfer request, so any caller able to reach the internal transfer RPC can still transfer an arbitrary call by simply omitting those two fields. This is an intentional backwards-compatibility branch (documented with a TODO), but it means the control provides no protection until it is enforced.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant