diff --git a/service/runway/server/config.go b/service/runway/server/config.go index a08f9e09b..c5eaeeb39 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 strings.ContainsAny(m.Host, "/ \t") { + return fmt.Errorf( + "%s: github merger host %q must be a 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..7951c9956 100644 --- a/service/runway/server/config_test.go +++ b/service/runway/server/config_test.go @@ -592,3 +592,35 @@ 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: "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) + }) + } +}