Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions api/base/change/proto/change.proto
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,15 @@ option java_package = "com.uber.submitqueue.base.change";
// Stovepipe, and other repo-local domains — the proto-level analog of the
// platform/base/change Go entity. Domains import it rather than redefining their own.
message Change {
// URIs identifying the change(s) (RFC 3986 compliant). The scheme identifies the
// change provider, and the path contains provider-specific resource identifiers.
// URIs identifying the change(s) (RFC 3986 compliant): scheme://<host[:port]>/<path>.
// The scheme identifies the change provider, the authority is the provider instance
// the change lives on, and the path contains provider-specific resource identifiers.
//
// Supported by default (other providers can be added):
// GitHub PR: "github://<org>/<repo>/pull/<pr>/<head_commit_sha>"
// ("ghe"/"ghes" schemes for GitHub Enterprise)
// git commit: "git://<remote>/<repo>/<ref>/<commit_sha>"
// (<ref> is a fully-qualified, percent-encoded git ref)
// GitHub PR: "github://<host[:port]>/<org>/<repo>/pull/<pr>/<head_commit_sha>"
// Phabricator Diff: "phab://<host[:port]>/D<revision>/<diff>"
// git commit: "git://<host[:port]>/<repo>/<ref>/<commit_sha>"
// (<ref> is a fully-qualified, percent-encoded git ref)
//
// The commit SHA must be the full 40-character lowercase hex SHA; abbreviated
// SHAs are rejected because downstream staleness checks compare by strict equality.
Expand Down
13 changes: 7 additions & 6 deletions api/base/change/protopb/change.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions api/runway/messagequeue/merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ func TestMergeRequestRoundTrip(t *testing.T) {
Steps: []*MergeStep{
{
StepId: "queue-a/1",
Changes: []*changepb.Change{{Uris: []string{"github://uber/repo/pull/1/0123456789abcdef0123456789abcdef01234567"}}},
Changes: []*changepb.Change{{Uris: []string{"github://github.example.com/uber/repo/pull/1/0123456789abcdef0123456789abcdef01234567"}}},
Strategy: strategypb.Strategy_REBASE,
},
{
StepId: "queue-a/2",
Changes: []*changepb.Change{{Uris: []string{"github://uber/repo/pull/2/89abcdef0123456789abcdef0123456789abcdef"}}},
Changes: []*changepb.Change{{Uris: []string{"github://github.example.com/uber/repo/pull/2/89abcdef0123456789abcdef0123456789abcdef"}}},
Strategy: strategypb.Strategy_MERGE,
},
},
Expand Down
16 changes: 10 additions & 6 deletions platform/base/change/change.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,17 @@ package change
// Change represents a code change identified by URIs from a code change provider (e.g., GitHub Pull Request, Phabricator Diff).
// The provider is extracted from the URI scheme. The object is immutable after creation.
type Change struct {
// URIs identifies the change(s) to land (RFC 3986 compliant).
// The scheme identifies the change provider, and the path contains provider-specific resource identifiers.
// URIs identifies the change(s) to land (RFC 3986 compliant): scheme://<host[:port]>/<path>.
// The scheme identifies the change provider, the authority is the provider instance the
// change lives on, and the path contains provider-specific resource identifiers.
//
// GitHub is supported by default (though other providers can be added):
// Template: "<scheme>://<org>/<repo>/pull/<pr>/<head_commit_sha>"
// Example: "github://uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"
// Schemes: "github", "ghe", "ghes". Head commit SHA must be full 40-char lowercase hex.
// Supported formats:
// GitHub PR: "github://<host[:port]>/<org>/<repo>/pull/<pr>/<head_commit_sha>"
// Phabricator Diff: "phab://<host[:port]>/D<revision>/<diff>"
// git ref/commit: "git://<host[:port]>/<repo>/<ref>/<sha>"
// Example: "github://github.example.com/uber/submitqueue/pull/123/c3a4d5e6f7890123456789abcdef0123456789ab"
//
// Head/commit SHAs must be the full 40-char lowercase hex form.
//
URIs []string `json:"uris"`
}
10 changes: 8 additions & 2 deletions platform/base/change/changeutil/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,20 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "go_default_library",
srcs = ["hex.go"],
srcs = [
"hex.go",
"host.go",
],
importpath = "github.com/uber/submitqueue/platform/base/change/changeutil",
visibility = ["//visibility:public"],
)

go_test(
name = "go_default_test",
srcs = ["hex_test.go"],
srcs = [
"hex_test.go",
"host_test.go",
],
embed = [":go_default_library"],
deps = ["@com_github_stretchr_testify//assert:go_default_library"],
)
30 changes: 30 additions & 0 deletions platform/base/change/changeutil/host.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright (c) 2026 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package changeutil

// IsLowercaseASCII reports whether s contains no uppercase ASCII letters (A-Z).
// Providers validate the host segment of a change URI with this: DNS is
// case-insensitive, so an uppercase variant of a host would alias one
// provider instance into many identities if let through. Canonical form
// rejects uppercase hosts rather than folding them.
func IsLowercaseASCII(s string) bool {
for i := 0; i < len(s); i++ {
c := s[i]
if c >= 'A' && c <= 'Z' {
return false
}
}
return true
}
42 changes: 42 additions & 0 deletions platform/base/change/changeutil/host_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) 2026 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package changeutil

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestIsLowercaseASCII(t *testing.T) {
tests := []struct {
name string
s string
want bool
}{
{name: "plain lowercase host", s: "github.example.com", want: true},
{name: "lowercase host with port", s: "github.example.com:8443", want: true},
{name: "empty string", s: "", want: true},
{name: "digits and hyphens", s: "git-01.example-corp.com", want: true},
{name: "uppercase letter rejected", s: "GitHub.example.com", want: false},
{name: "all uppercase rejected", s: "GITHUB.EXAMPLE.COM", want: false},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, IsLowercaseASCII(tt.s))
})
}
}
3 changes: 3 additions & 0 deletions platform/base/change/git/change_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ func ParseChangeID(raw string) (ChangeID, error) {
if u.Host == "" {
return ChangeID{}, fmt.Errorf("invalid change ID %q: missing remote (expected format: %s)", raw, changeIDFormat)
}
if !changeutil.IsLowercaseASCII(u.Hostname()) {
Comment thread
JamyDev marked this conversation as resolved.
return ChangeID{}, fmt.Errorf("invalid change ID %q: remote %q must be lowercase (expected format: %s)", raw, u.Hostname(), changeIDFormat)
}

// Split on the escaped path so the percent-encoded ref stays a single
// segment (url.URL.Path decodes %2F to "/", which would split it apart).
Expand Down
5 changes: 5 additions & 0 deletions platform/base/change/git/change_id_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ func TestParseChangeID(t *testing.T) {
raw: "git:///uber/monorepo/refs%2Fheads%2Fmain/" + sha,
wantErr: true,
},
{
name: "uppercase host",
raw: "git://Git.example.com/uber/monorepo/refs%2Fheads%2Fmain/" + sha,
wantErr: true,
},
{
name: "missing commit SHA",
raw: "git://git.example.com/uber/monorepo/refs%2Fheads%2Fmain",
Expand Down
58 changes: 35 additions & 23 deletions platform/base/change/github/change_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,21 @@ package github

import (
"fmt"
"net/url"
"strconv"
"strings"

"github.com/uber/submitqueue/platform/base/change/changeutil"
)

// scheme is the canonical URI scheme for GitHub change identifiers. Deployment
// flavors (github.com, GitHub Enterprise, GitHub Enterprise Server) share this
// one scheme; the flavor is derivable from the host in the authority, so it
// does not get its own scheme.
const scheme = "github"

// changeIDFormat is the expected format for change IDs, included in error messages.
const changeIDFormat = "{scheme}://{owner}/{repo}/pull/{pr_number}/{head_commit_sha}"
const changeIDFormat = "github://{host[:port]}/{owner}/{repo}/pull/{pr_number}/{head_commit_sha}"

// pullSegment is the literal segment separating the repo path from the pull
// request number. Mirrors the path layout of an actual GitHub PR URL
Expand All @@ -38,13 +45,16 @@ const pullSegment = "pull"
// Validate up-front to fail fast at the gateway with a clearer error.
const shaLength = 40

// ChangeID represents a parsed GitHub-family change identifier.
// Covers GitHub.com, GitHub Enterprise (GHE), and GitHub Enterprise Server (GHES)
// since they share the same pull request model.
// Format: {scheme}://{owner}/{repo}/pull/{pr_number}/{head_commit_sha}
// ChangeID represents a parsed GitHub change identifier. The authority names
// the GitHub instance the change lives on, so github.com, GitHub Enterprise,
// and GitHub Enterprise Server all parse under the single "github" scheme.
// Format: github://{host[:port]}/{owner}/{repo}/pull/{pr_number}/{head_commit_sha}
type ChangeID struct {
// Scheme captures the source variant (e.g., "github", "ghe", "ghes").
// Scheme captures the URI scheme (always "github" in current implementation).
Scheme string
// Host is the host or host:port of the GitHub instance the change lives
// on, e.g. "github.example.com" or "github.example.com:8443".
Host string
// Org is the organization or owner of the repository.
Org string
// Repo is the repository name.
Expand All @@ -56,29 +66,30 @@ type ChangeID struct {
}

// ParseChangeID parses a raw change ID string into a ChangeID.
// Expected format: {scheme}://{owner}/{repo}/pull/{pr_number}/{head_commit_sha}
// Expected format: github://{host[:port]}/{owner}/{repo}/pull/{pr_number}/{head_commit_sha}
// The parser works from the end: SHA (last), PR number (second-to-last),
// the literal "pull" segment (third-to-last), and everything before is the
// repo path (split into owner and repo).
func ParseChangeID(raw string) (ChangeID, error) {
// Split on "://" to get scheme and path
schemeSplit := strings.SplitN(raw, "://", 2)
if len(schemeSplit) != 2 {
return ChangeID{}, fmt.Errorf("invalid change ID %q: missing '://' separator (expected format: %s)", raw, changeIDFormat)
u, err := url.Parse(raw)
if err != nil {
return ChangeID{}, fmt.Errorf("invalid change ID %q: %w (expected format: %s)", raw, err, changeIDFormat)
}

scheme := schemeSplit[0]
if scheme == "" {
return ChangeID{}, fmt.Errorf("invalid change ID %q: empty scheme (expected format: %s)", raw, changeIDFormat)
if u.Scheme != scheme {
return ChangeID{}, fmt.Errorf("invalid change ID %q: scheme must be %q, got %q (expected format: %s)", raw, scheme, u.Scheme, changeIDFormat)
}
if u.Host == "" {
return ChangeID{}, fmt.Errorf("invalid change ID %q: missing host (expected format: %s)", raw, changeIDFormat)
}
if !changeutil.IsLowercaseASCII(u.Hostname()) {
return ChangeID{}, fmt.Errorf("invalid change ID %q: host %q must be lowercase (expected format: %s)", raw, u.Hostname(), changeIDFormat)
}

path := schemeSplit[1]

// Split the path into segments and parse from the end.
segments := strings.Split(path, "/")
// Need at least 5 segments: {owner}/{repo}/pull/{pr_number}/{sha}
// Split on the escaped path so any percent-encoded segments stay intact.
segments := strings.Split(strings.TrimPrefix(u.EscapedPath(), "/"), "/")
// Need at least 5 segments: {owner}/{repo}/pull/{pr_number}/{sha}.
if len(segments) < 5 {
return ChangeID{}, fmt.Errorf("invalid change ID %q: need at least owner/repo/pull/pr/sha, got %d segments (expected format: %s)", raw, len(segments), changeIDFormat)
return ChangeID{}, fmt.Errorf("invalid change ID %q: need at least owner/repo/pull/pr/sha, got %d path segments (expected format: %s)", raw, len(segments), changeIDFormat)
}

sha := segments[len(segments)-1]
Expand Down Expand Up @@ -118,7 +129,8 @@ func ParseChangeID(raw string) (ChangeID, error) {
}

return ChangeID{
Scheme: scheme,
Scheme: u.Scheme,
Host: u.Host,
Org: org,
Repo: repo,
PRNumber: prNumber,
Expand All @@ -128,7 +140,7 @@ func ParseChangeID(raw string) (ChangeID, error) {

// String returns the string representation of the change ID.
func (c ChangeID) String() string {
return fmt.Sprintf("%s://%s/%s/%s/%d/%s", c.Scheme, c.Org, c.Repo, pullSegment, c.PRNumber, c.HeadCommitSHA)
return fmt.Sprintf("%s://%s/%s/%s/%s/%d/%s", c.Scheme, c.Host, c.Org, c.Repo, pullSegment, c.PRNumber, c.HeadCommitSHA)
}

// OwnerRepo returns the "{org}/{repo}" string.
Expand Down
Loading
Loading