From be31d5c16b5b6ce7078f9fd707dd4c84728b0a2d Mon Sep 17 00:00:00 2001 From: Divyansh Garg Date: Wed, 7 Oct 2026 14:54:40 +0530 Subject: [PATCH] fix(change): reject non-canonical git change URIs --- platform/base/change/git/change_id.go | 16 +++++++-- platform/base/change/git/change_id_test.go | 40 ++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/platform/base/change/git/change_id.go b/platform/base/change/git/change_id.go index e354f61c4..3fecbe4a3 100644 --- a/platform/base/change/git/change_id.go +++ b/platform/base/change/git/change_id.go @@ -107,13 +107,25 @@ func ParseChangeID(raw string) (ChangeID, error) { return ChangeID{}, fmt.Errorf("invalid change ID %q: empty repo (expected format: %s)", raw, changeIDFormat) } - return ChangeID{ + for _, seg := range segments[:len(segments)-2] { + if seg == "" { + return ChangeID{}, fmt.Errorf("invalid change ID %q: repo path contains an empty segment (expected format: %s)", raw, changeIDFormat) + } + } + + id := ChangeID{ Scheme: u.Scheme, Remote: u.Host, Repo: repo, Ref: ref, CommitSHA: sha, - }, nil + } + + if canonical := id.String(); canonical != raw { + return ChangeID{}, fmt.Errorf("invalid change ID %q: not in canonical form %q (expected format: %s)", raw, canonical, changeIDFormat) + } + + return id, nil } // String returns the string representation of the change ID. diff --git a/platform/base/change/git/change_id_test.go b/platform/base/change/git/change_id_test.go index c06cea670..1abf769ce 100644 --- a/platform/base/change/git/change_id_test.go +++ b/platform/base/change/git/change_id_test.go @@ -136,6 +136,46 @@ func TestParseChangeID(t *testing.T) { raw: "git://git.example.com//refs%2Fheads%2Fmain/" + sha, wantErr: true, }, + { + name: "uppercase scheme", + raw: "GIT://git.example.com/uber/monorepo/refs%2Fheads%2Fmain/" + sha, + wantErr: true, + }, + { + name: "userinfo", + raw: "git://user@git.example.com/uber/monorepo/refs%2Fheads%2Fmain/" + sha, + wantErr: true, + }, + { + name: "query", + raw: "git://git.example.com/uber/monorepo/refs%2Fheads%2Fmain/" + sha + "?x=1", + wantErr: true, + }, + { + name: "fragment", + raw: "git://git.example.com/uber/monorepo/refs%2Fheads%2Fmain/" + sha + "#frag", + wantErr: true, + }, + { + name: "lowercase percent-encoding", + raw: "git://git.example.com/uber/monorepo/refs%2fheads%2fmain/" + sha, + wantErr: true, + }, + { + name: "unnecessarily encoded ref character", + raw: "git://git.example.com/uber/monorepo/%72efs%2Fheads%2Fmain/" + sha, + wantErr: true, + }, + { + name: "empty interior repo segment", + raw: "git://git.example.com/uber//monorepo/refs%2Fheads%2Fmain/" + sha, + wantErr: true, + }, + { + name: "leading empty repo segment", + raw: "git://git.example.com//uber/monorepo/refs%2Fheads%2Fmain/" + sha, + wantErr: true, + }, } for _, tt := range tests {