From 5121a9354f68f5bdf864308ddeadb17701fa685a Mon Sep 17 00:00:00 2001 From: Divyansh Garg Date: Fri, 16 Oct 2026 16:59:18 +0530 Subject: [PATCH 1/2] Validate GitHub merger config --- service/runway/server/config.go | 18 ++++++++++++++- service/runway/server/config_test.go | 33 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/service/runway/server/config.go b/service/runway/server/config.go index a08f9e09b..e241e47a6 100644 --- a/service/runway/server/config.go +++ b/service/runway/server/config.go @@ -283,6 +283,14 @@ func (m *mergerConfig) normalizeAndValidateGitHub(where string) error { if m.Host == "" { m.Host = defaultGitHubHost } + if m.Host != strings.ToLower(m.Host) || strings.ContainsAny(m.Host, "/ \t") { + return fmt.Errorf( + "%s: github merger host %q must be a lowercase host[:port] such as %q, without scheme or path", + where, + m.Host, + defaultGitHubHost, + ) + } if m.Target == "" { m.Target = "main" } @@ -306,7 +314,15 @@ func parseOptionalDuration(v string) (time.Duration, error) { if v == "" { return 0, nil } - return time.ParseDuration(v) + d, err := time.ParseDuration(v) + if err != nil { + return 0, err + } + if d < 0 { + return 0, fmt.Errorf("duration %q must not be negative", v) + } + + return d, nil } // firstDifference reports the first field in which two git mergers disagree, diff --git a/service/runway/server/config_test.go b/service/runway/server/config_test.go index 0f62aa9cf..3f0e891b7 100644 --- a/service/runway/server/config_test.go +++ b/service/runway/server/config_test.go @@ -592,3 +592,36 @@ func TestMergerConfig_Provisions(t *testing.T) { owned := mergerConfig{Type: mergerTypeGit, CheckoutPath: "/var/checkouts/r", RemoteURL: "https://example.com/o/r.git"} assert.True(t, owned.provisions()) } + +func TestLoadMergeConfig_GitHubDurationAndHostValidation(t *testing.T) { + tests := []struct { + name string + extra string + wantErr bool + }{ + {name: "defaults", extra: ""}, + {name: "valid durations", extra: " httpTimeout: 30s\n pollInterval: 2s\n maxPollDuration: 5m\n"}, + {name: "host with port", extra: " host: ghe.example.com:8443\n"}, + {name: "negative httpTimeout", extra: " httpTimeout: -1s\n", wantErr: true}, + {name: "negative pollInterval", extra: " pollInterval: -1s\n", wantErr: true}, + {name: "negative maxPollDuration", extra: " maxPollDuration: -1m\n", wantErr: true}, + {name: "uppercase host", extra: " host: GitHub.com\n", wantErr: true}, + {name: "host with scheme", extra: " host: https://github.com\n", wantErr: true}, + {name: "host with path", extra: " host: github.com/path\n", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "merge.yaml") + yaml := "defaults:\n merger:\n type: github\n owner: uber\n repo: submitqueue\n" + tt.extra + require.NoError(t, os.WriteFile(path, []byte(yaml), 0o600)) + + _, err := loadMergeConfig(path) + + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + }) + } +} From 2a2843211945555207dc151e3e816be928b34a77 Mon Sep 17 00:00:00 2001 From: Divyansh Garg Date: Fri, 16 Oct 2026 18:49:06 +0530 Subject: [PATCH 2/2] removing uppercase check --- service/runway/server/config.go | 4 ++-- service/runway/server/config_test.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/service/runway/server/config.go b/service/runway/server/config.go index e241e47a6..c5eaeeb39 100644 --- a/service/runway/server/config.go +++ b/service/runway/server/config.go @@ -283,9 +283,9 @@ func (m *mergerConfig) normalizeAndValidateGitHub(where string) error { if m.Host == "" { m.Host = defaultGitHubHost } - if m.Host != strings.ToLower(m.Host) || strings.ContainsAny(m.Host, "/ \t") { + if strings.ContainsAny(m.Host, "/ \t") { return fmt.Errorf( - "%s: github merger host %q must be a lowercase host[:port] such as %q, without scheme or path", + "%s: github merger host %q must be a host[:port] such as %q, without scheme or path", where, m.Host, defaultGitHubHost, diff --git a/service/runway/server/config_test.go b/service/runway/server/config_test.go index 3f0e891b7..7951c9956 100644 --- a/service/runway/server/config_test.go +++ b/service/runway/server/config_test.go @@ -605,7 +605,6 @@ func TestLoadMergeConfig_GitHubDurationAndHostValidation(t *testing.T) { {name: "negative httpTimeout", extra: " httpTimeout: -1s\n", wantErr: true}, {name: "negative pollInterval", extra: " pollInterval: -1s\n", wantErr: true}, {name: "negative maxPollDuration", extra: " maxPollDuration: -1m\n", wantErr: true}, - {name: "uppercase host", extra: " host: GitHub.com\n", wantErr: true}, {name: "host with scheme", extra: " host: https://github.com\n", wantErr: true}, {name: "host with path", extra: " host: github.com/path\n", wantErr: true}, }