From 4be669f8e248e94965ce66565cf1dc83d39f3d29 Mon Sep 17 00:00:00 2001 From: Alex Fish Date: Thu, 16 Jul 2026 16:37:36 -0700 Subject: [PATCH 1/8] Fix recursive call to validate. --- livekit/sip.go | 2 +- livekit/sip_test.go | 210 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 1 deletion(-) diff --git a/livekit/sip.go b/livekit/sip.go index b461feac4..1c9ef9969 100644 --- a/livekit/sip.go +++ b/livekit/sip.go @@ -441,7 +441,7 @@ func (p *CreateSIPInboundTrunkRequest) ValidateResult() ValidationResult { if p.Trunk.SipTrunkId != "" { return ValidationFailure(errors.New("trunk id must not be set")) } - return p.ValidateResult() + return p.Trunk.ValidateResult() } func (p *UpdateSIPOutboundTrunkRequest) Validate() error { diff --git a/livekit/sip_test.go b/livekit/sip_test.go index 20838ffdb..b4086c22e 100644 --- a/livekit/sip_test.go +++ b/livekit/sip_test.go @@ -2,6 +2,7 @@ package livekit import ( errors "errors" + "fmt" "slices" "testing" "time" @@ -50,11 +51,16 @@ func TestSIPValidate(t *testing.T) { type validateable interface { Validate() error } + type resultValidateable interface { + ValidateResult() ValidationResult + } + type validateTestCase struct { name string req validateable exp bool } + cases := map[string][]validateTestCase{ "SIPInboundTrunkInfo": { { @@ -322,18 +328,222 @@ func TestSIPValidate(t *testing.T) { exp: false, }, }, + "CreateSIPInboundTrunkRequest": { + { + name: "create_inbound_missing_trunk", + req: &CreateSIPInboundTrunkRequest{}, + exp: false, + }, + { + name: "create_inbound_trunk_id_set", + req: &CreateSIPInboundTrunkRequest{ + Trunk: &SIPInboundTrunkInfo{ + SipTrunkId: "id", + Numbers: []string{"+1111"}, + }, + }, + exp: false, + }, + { + name: "create_inbound_valid", + req: &CreateSIPInboundTrunkRequest{ + Trunk: &SIPInboundTrunkInfo{ + Numbers: []string{"+1111"}, + }, + }, + exp: true, + }, + }, + "CreateSIPOutboundTrunkRequest": { + { + name: "create_outbound_missing_trunk", + req: &CreateSIPOutboundTrunkRequest{}, + exp: false, + }, + { + name: "create_outbound_trunk_id_set", + req: &CreateSIPOutboundTrunkRequest{ + Trunk: &SIPOutboundTrunkInfo{ + SipTrunkId: "id", + }, + }, + exp: false, + }, + { + name: "create_outbound_valid", + req: &CreateSIPOutboundTrunkRequest{ + Trunk: &SIPOutboundTrunkInfo{ + Address: "sip.example.com", + Numbers: []string{"+2222"}, + }, + }, + exp: true, + }, + }, + "UpdateSIPInboundTrunkRequest": { + { + name: "update_inbound_missing_id", + req: &UpdateSIPInboundTrunkRequest{}, + exp: false, + }, + { + name: "update_inbound_missing_action", + req: &UpdateSIPInboundTrunkRequest{ + SipTrunkId: "id", + }, + exp: false, + }, + { + name: "update_inbound_update_ok", + req: &UpdateSIPInboundTrunkRequest{ + SipTrunkId: "id", + Action: &UpdateSIPInboundTrunkRequest_Update{ + Update: &SIPInboundTrunkUpdate{}, + }, + }, + exp: true, + }, + { + name: "update_inbound_replace_ok", + req: &UpdateSIPInboundTrunkRequest{ + SipTrunkId: "id", + Action: &UpdateSIPInboundTrunkRequest_Replace{ + Replace: &SIPInboundTrunkInfo{ + Numbers: []string{"+1111"}, + }, + }, + }, + exp: true, + }, + { + name: "update_inbound_replace_id_mismatch", + req: &UpdateSIPInboundTrunkRequest{ + SipTrunkId: "id", + Action: &UpdateSIPInboundTrunkRequest_Replace{ + Replace: &SIPInboundTrunkInfo{ + SipTrunkId: "other", + Numbers: []string{"+1111"}, + }, + }, + }, + exp: false, + }, + }, + "UpdateSIPOutboundTrunkRequest": { + { + name: "update_outbound_missing id", + req: &UpdateSIPOutboundTrunkRequest{}, + exp: false, + }, + { + name: "update_outbound_missing_action", + req: &UpdateSIPOutboundTrunkRequest{ + SipTrunkId: "id", + }, + exp: false, + }, + { + name: "update_outbound_update_ok", + req: &UpdateSIPOutboundTrunkRequest{ + SipTrunkId: "id", + Action: &UpdateSIPOutboundTrunkRequest_Update{ + Update: &SIPOutboundTrunkUpdate{}, + }, + }, + exp: true, + }, + { + name: "update_outbound_replace_ok", + req: &UpdateSIPOutboundTrunkRequest{ + SipTrunkId: "id", + Action: &UpdateSIPOutboundTrunkRequest_Replace{ + Replace: &SIPOutboundTrunkInfo{ + Address: "sip.example.com", + Numbers: []string{"+2222"}, + }, + }, + }, + exp: true, + }, + { + name: "update_outbound_replace_id_mismatch", + req: &UpdateSIPOutboundTrunkRequest{ + SipTrunkId: "id", + Action: &UpdateSIPOutboundTrunkRequest_Replace{ + Replace: &SIPOutboundTrunkInfo{ + SipTrunkId: "other", + Address: "sip.example.com", + Numbers: []string{"+2222"}, + }, + }, + }, + exp: false, + }, + }, + "TransferSIPParticipantRequest": { + { + name: "transfer_valid", + req: &TransferSIPParticipantRequest{ + RoomName: "room1", + ParticipantIdentity: "participant1", + TransferTo: "tel:+15105550100", + }, + exp: true, + }, + { + name: "transfer_missing_room", + req: &TransferSIPParticipantRequest{}, + exp: false, + }, + { + name: "transfer_missing_participant", + req: &TransferSIPParticipantRequest{ + RoomName: "room1", + }, + exp: false, + }, + { + name: "transfer_invalid_uri", + req: &TransferSIPParticipantRequest{ + RoomName: "room1", + ParticipantIdentity: "participant1", + TransferTo: "http://example.com", + }, + exp: false, + }, + }, } + resultValidatableTypes := make(map[string]bool) + for name, class := range cases { t.Run(name, func(t *testing.T) { for _, c := range class { t.Run(c.name, func(t *testing.T) { err := c.req.Validate() require.Equal(t, c.exp, err == nil, "error: %v", err) + + if v, ok := c.req.(resultValidateable); ok { + result := v.ValidateResult() + require.Equal(t, c.exp, result.Error() == nil, "error: %v", err) + resultValidatableTypes[fmt.Sprintf("%T", v)] = true + } }) } }) } + + wantResultValidateableTypes := map[string]bool{ + "*livekit.CreateSIPInboundTrunkRequest": true, + "*livekit.CreateSIPOutboundTrunkRequest": true, + "*livekit.CreateSIPParticipantRequest": true, + "*livekit.SIPInboundTrunkInfo": true, + "*livekit.SIPOutboundTrunkInfo": true, + "*livekit.TransferSIPParticipantRequest": true, + "*livekit.UpdateSIPInboundTrunkRequest": true, + "*livekit.UpdateSIPOutboundTrunkRequest": true, + } + require.Equal(t, wantResultValidateableTypes, resultValidatableTypes) } func TestSIPInboundTrunkFilter(t *testing.T) { From ca822e6da7987becdc4e0fc1b8985a8818e0fd82 Mon Sep 17 00:00:00 2001 From: Alex Fish Date: Thu, 16 Jul 2026 17:26:47 -0700 Subject: [PATCH 2/8] Add changeset. --- .changeset/brave-coats-give.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/brave-coats-give.md diff --git a/.changeset/brave-coats-give.md b/.changeset/brave-coats-give.md new file mode 100644 index 000000000..7def22685 --- /dev/null +++ b/.changeset/brave-coats-give.md @@ -0,0 +1,6 @@ +--- +"github.com/livekit/protocol": patch +"@livekit/protocol": patch +--- + +Fix recursive call to CreateSIPInboundTrunkRequest.ValidateResult. From a0c419829ebe9db4a579b4ef9dd8fdef0c24b690 Mon Sep 17 00:00:00 2001 From: Alex Fish Date: Fri, 17 Jul 2026 10:15:58 -0700 Subject: [PATCH 3/8] Remove newline. --- livekit/sip_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/livekit/sip_test.go b/livekit/sip_test.go index b4086c22e..0ce92d010 100644 --- a/livekit/sip_test.go +++ b/livekit/sip_test.go @@ -54,7 +54,6 @@ func TestSIPValidate(t *testing.T) { type resultValidateable interface { ValidateResult() ValidationResult } - type validateTestCase struct { name string req validateable From 9ad6f4a8c0667f5d533b0103fc2c89ba22103ea8 Mon Sep 17 00:00:00 2001 From: Alex Fish Date: Fri, 17 Jul 2026 10:44:28 -0700 Subject: [PATCH 4/8] Upate SIP validate test to check for soft failures. --- livekit/sip_test.go | 134 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 131 insertions(+), 3 deletions(-) diff --git a/livekit/sip_test.go b/livekit/sip_test.go index 0ce92d010..9f63720e6 100644 --- a/livekit/sip_test.go +++ b/livekit/sip_test.go @@ -55,9 +55,10 @@ func TestSIPValidate(t *testing.T) { ValidateResult() ValidationResult } type validateTestCase struct { - name string - req validateable - exp bool + name string + req validateable + exp bool + wantSoftErrs bool } cases := map[string][]validateTestCase{ @@ -119,6 +120,27 @@ func TestSIPValidate(t *testing.T) { }, exp: false, }, + { + name: "inbound invalid header", + req: &SIPInboundTrunkInfo{ + Numbers: []string{"+1111"}, + HeadersToAttributes: map[string]string{ + "From ": "from", + }, + }, + exp: false, + }, + { + name: "inbound invalid header value", + req: &SIPInboundTrunkInfo{ + Numbers: []string{"+1111"}, + HeadersToAttributes: map[string]string{ + "From": " 0 + if c.wantSoftErrs { + require.Equal(t, c.wantSoftErrs, hasSoftErrs, "got zero soft validation errors; want at least one") + } else { + require.Equal(t, c.wantSoftErrs, hasSoftErrs, "got at least one soft validation error; want zero; %v", result.SoftErrors()) + } } }) } From 3967b68c125f8e0058d10a01fef01551ad1b37e0 Mon Sep 17 00:00:00 2001 From: Alex Fish Date: Fri, 17 Jul 2026 10:44:49 -0700 Subject: [PATCH 5/8] Fix typo. --- livekit/sip_validation.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/livekit/sip_validation.go b/livekit/sip_validation.go index 4b80df171..fdf688e54 100644 --- a/livekit/sip_validation.go +++ b/livekit/sip_validation.go @@ -169,7 +169,7 @@ var RequiredResponseHeaders = map[string]bool{ } // Crucial headers that can't be overridden by the user, and their shorthands -var FrobiddenSipHeaderNames = map[string]bool{ +var ForbiddenSipHeaderNames = map[string]bool{ "accept": true, "accept-encoding": true, "accept-language": true, @@ -232,7 +232,7 @@ func ValidateHeaderName(name string, restrictNames bool) error { // Convert to lowercase for case-insensitive comparison if restrictNames { lowerName := strings.ToLower(name) - if forbidden, exists := FrobiddenSipHeaderNames[lowerName]; exists && forbidden { + if forbidden, exists := ForbiddenSipHeaderNames[lowerName]; exists && forbidden { return fmt.Errorf("header name %s not supported", name) } } From 81ae10cd961439695f5d7b9babfbf7bc30afd569 Mon Sep 17 00:00:00 2001 From: Alex Fish Date: Fri, 17 Jul 2026 10:45:52 -0700 Subject: [PATCH 6/8] Fix typo in sip_validation_test.go. --- livekit/sip_validation_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/livekit/sip_validation_test.go b/livekit/sip_validation_test.go index 424faba7d..efccb6dbe 100644 --- a/livekit/sip_validation_test.go +++ b/livekit/sip_validation_test.go @@ -292,9 +292,9 @@ func TestValidateHeaderValueResult(t *testing.T) { } } -func TestFrobiddenSipHeaderNames(t *testing.T) { +func TestForbiddenSipHeaderNames(t *testing.T) { i := 0 - for name := range FrobiddenSipHeaderNames { + for name := range ForbiddenSipHeaderNames { i++ t.Run(testCaseName(name, 32, i), func(t *testing.T) { err := ValidateHeaderName(name, true) From 22847113ffad3b28c07d69b569bad4a2fa5d677a Mon Sep 17 00:00:00 2001 From: Alex Fish Date: Fri, 17 Jul 2026 11:39:40 -0700 Subject: [PATCH 7/8] Update validateHeaders to propagate failures and add some tests. --- livekit/sip.go | 11 +++-- livekit/sip_test.go | 98 ++++++++++++++++++++++++++++++++------------- 2 files changed, 78 insertions(+), 31 deletions(-) diff --git a/livekit/sip.go b/livekit/sip.go index 1c9ef9969..79f076c95 100644 --- a/livekit/sip.go +++ b/livekit/sip.go @@ -365,15 +365,18 @@ func ValidationFailure(err error) ValidationResult { } func validateHeaders(headers map[string]string) ValidationResult { + ret := ValidationResult{} for headerName, headerValue := range headers { if err := ValidateHeaderName(headerName, true); err != nil { - return ValidationFailure(fmt.Errorf("invalid header name: %w", err)) + return ret.WithError(fmt.Errorf("invalid header name: %w", err)) } - if err := ValidateHeaderValue(headerName, headerValue); err != nil { - return ValidationFailure(fmt.Errorf("invalid header value for %s: %w", headerName, err)) + result := ValidateHeaderValueResult(headerName, headerValue) + if !result.OK() { + return ret.WithError(fmt.Errorf("invalid header value for %s: %w", headerName, result.Error())) } + ret = ret.Combine(result) } - return ValidationResult{} + return ret } // validateHeaderNames Makes sure the values of the given map correspond to valid SIP header names diff --git a/livekit/sip_test.go b/livekit/sip_test.go index 9f63720e6..a83c73814 100644 --- a/livekit/sip_test.go +++ b/livekit/sip_test.go @@ -1,7 +1,7 @@ package livekit import ( - errors "errors" + "errors" "fmt" "slices" "testing" @@ -120,22 +120,12 @@ func TestSIPValidate(t *testing.T) { }, exp: false, }, - { - name: "inbound invalid header", - req: &SIPInboundTrunkInfo{ - Numbers: []string{"+1111"}, - HeadersToAttributes: map[string]string{ - "From ": "from", - }, - }, - exp: false, - }, { name: "inbound invalid header value", req: &SIPInboundTrunkInfo{ Numbers: []string{"+1111"}, - HeadersToAttributes: map[string]string{ - "From": " Date: Fri, 17 Jul 2026 11:44:10 -0700 Subject: [PATCH 8/8] Fix test. --- livekit/sip_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/livekit/sip_test.go b/livekit/sip_test.go index a83c73814..cfda9ac38 100644 --- a/livekit/sip_test.go +++ b/livekit/sip_test.go @@ -228,7 +228,7 @@ func TestSIPValidate(t *testing.T) { }, }, exp: true, - wantSoftErrs: false, + wantSoftErrs: true, }, }, "SIPMediaConfig": {