diff --git a/platform/base/change/github/change_id.go b/platform/base/change/github/change_id.go index d6e75f465..b79341066 100644 --- a/platform/base/change/github/change_id.go +++ b/platform/base/change/github/change_id.go @@ -109,15 +109,26 @@ func ParseChangeID(raw string) (ChangeID, error) { } prNumber, err := strconv.Atoi(prStr) + if err != nil { return ChangeID{}, fmt.Errorf("invalid change ID %q: PR number %q is not a valid integer (expected format: %s)", raw, prStr, changeIDFormat) } + if prNumber < 1 || strconv.Itoa(prNumber) != prStr { + return ChangeID{}, fmt.Errorf("invalid change ID %q: PR number %q must be a positive integer without sign or leading zeros (expected format: %s)", raw, prStr, changeIDFormat) + } + // Split repo path: last segment is repo name, everything before is the owner. if len(repoSegments) < 2 { return ChangeID{}, fmt.Errorf("invalid change ID %q: repo path must have at least owner/repo (expected format: %s)", raw, changeIDFormat) } + for _, seg := range repoSegments { + if seg == "" { + return ChangeID{}, fmt.Errorf("invalid change ID %q: repo path contains an empty segment (expected format: %s)", raw, changeIDFormat) + } + } + repo := repoSegments[len(repoSegments)-1] org := strings.Join(repoSegments[:len(repoSegments)-1], "/") diff --git a/platform/base/change/github/change_id_test.go b/platform/base/change/github/change_id_test.go index 58d7a2471..009f9a531 100644 --- a/platform/base/change/github/change_id_test.go +++ b/platform/base/change/github/change_id_test.go @@ -133,6 +133,31 @@ func TestParseChangeID(t *testing.T) { raw: "github://github.example.com/uber/submitqueue/pull/abc/" + shaAFull, wantErr: true, }, + { + name: "zero PR number", + raw: "github://github.example.com/uber/submitqueue/pull/0/" + shaAFull, + wantErr: true, + }, + { + name: "negative PR number", + raw: "github://github.example.com/uber/submitqueue/pull/-3/" + shaAFull, + wantErr: true, + }, + { + name: "plus-signed PR number", + raw: "github://github.example.com/uber/submitqueue/pull/+5/" + shaAFull, + wantErr: true, + }, + { + name: "leading-zero PR number", + raw: "github://github.example.com/uber/submitqueue/pull/007/" + shaAFull, + wantErr: true, + }, + { + name: "empty interior org segment", + raw: "github://github.example.com/uber//frontend/webapp/pull/42/" + shaAFull, + wantErr: true, + }, { name: "empty SHA", raw: "github://github.example.com/uber/submitqueue/pull/123/",