Skip to content

Commit 9573e43

Browse files
committed
fix(provider): authenticate readiness
Keep public liveness separate from sidecar readiness. Reject malformed or duplicate bearer headers before constant-time provider-token authorization.
1 parent f33a6e3 commit 9573e43

2 files changed

Lines changed: 125 additions & 2 deletions

File tree

‎internal/module_runner_provider.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1540,6 +1540,7 @@ func validateEphemeralRunnerJobArgs(args map[string]any) error {
15401540
func (m *githubRunnerProviderModule) HTTPHandler() http.Handler {
15411541
mux := http.NewServeMux()
15421542
mux.HandleFunc("GET /healthz", m.handleHealth)
1543+
mux.HandleFunc("GET /readyz", m.handleReadiness)
15431544
mux.HandleFunc("POST /v1/actions/runners/registration-token", m.handleRegistrationToken)
15441545
mux.HandleFunc("DELETE /v1/actions/runners/{runner_id}", m.handleRemoveRunner)
15451546
mux.HandleFunc("POST /v1/actions/orgs/{organization}/runners/registration-token", m.handleOrgRegistrationToken)
@@ -1559,6 +1560,19 @@ func (m *githubRunnerProviderModule) handleHealth(w http.ResponseWriter, _ *http
15591560
writeProviderResponse(w, http.StatusOK, map[string]any{"status": "ok"})
15601561
}
15611562

1563+
func (m *githubRunnerProviderModule) handleReadiness(w http.ResponseWriter, r *http.Request) {
1564+
if r.Method != http.MethodGet {
1565+
w.Header().Set("Allow", http.MethodGet)
1566+
writeProviderError(w, http.StatusMethodNotAllowed, errors.New("method not allowed"))
1567+
return
1568+
}
1569+
if err := m.authorizeProvider(map[string]any{"provider_token": bearerToken(r)}); err != nil {
1570+
writeProviderError(w, providerErrorStatus(err), err)
1571+
return
1572+
}
1573+
writeProviderResponse(w, http.StatusOK, map[string]any{"status": "ok"})
1574+
}
1575+
15621576
func (m *githubRunnerProviderModule) handleRegistrationToken(w http.ResponseWriter, r *http.Request) {
15631577
var req struct {
15641578
Repository string `json:"repository"`
@@ -1845,9 +1859,16 @@ func parseOptionalRFC3339Query(value, name string) (time.Time, error) {
18451859
}
18461860

18471861
func bearerToken(r *http.Request) string {
1848-
value := r.Header.Get("Authorization")
1862+
values := r.Header.Values("Authorization")
1863+
if len(values) != 1 {
1864+
return ""
1865+
}
1866+
value := values[0]
1867+
if strings.TrimSpace(value) != value {
1868+
return ""
1869+
}
18491870
token, ok := strings.CutPrefix(value, "Bearer ")
1850-
if !ok {
1871+
if !ok || token == "" || strings.ContainsAny(token, " \t\r\n") {
18511872
return ""
18521873
}
18531874
return token

‎internal/runner_provider_test.go‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,108 @@ func TestT44_GitHubRunnerProviderServesHealthz(t *testing.T) {
10351035
}
10361036
}
10371037

1038+
func TestT916GitHubRunnerProviderReadyzRequiresProviderBearer(t *testing.T) {
1039+
module, err := newGitHubRunnerProviderModule("provider", map[string]any{
1040+
"token": "github-token",
1041+
"provider_token": "provider-token",
1042+
"repositories": []any{"example/repository"},
1043+
}, &fakeRunnerClient{})
1044+
if err != nil {
1045+
t.Fatalf("module: %v", err)
1046+
}
1047+
server := httptest.NewServer(module.HTTPHandler())
1048+
defer server.Close()
1049+
1050+
unauthorizedCases := []struct {
1051+
name string
1052+
authorization []string
1053+
}{
1054+
{name: "missing"},
1055+
{name: "bare token", authorization: []string{"provider-token"}},
1056+
{name: "bare bearer", authorization: []string{"Bearer"}},
1057+
{name: "empty bearer", authorization: []string{"Bearer "}},
1058+
{name: "basic", authorization: []string{"Basic provider-token"}},
1059+
{name: "lowercase scheme", authorization: []string{"bearer provider-token"}},
1060+
{name: "extra whitespace", authorization: []string{"Bearer provider-token"}},
1061+
{name: "wrong token", authorization: []string{"Bearer submitted-wrong-token"}},
1062+
{name: "duplicate", authorization: []string{"Bearer provider-token", "Bearer submitted-wrong-token"}},
1063+
}
1064+
for _, tc := range unauthorizedCases {
1065+
t.Run(tc.name, func(t *testing.T) {
1066+
req, requestErr := http.NewRequest(http.MethodGet, server.URL+"/readyz", nil)
1067+
if requestErr != nil {
1068+
t.Fatalf("request: %v", requestErr)
1069+
}
1070+
for _, authorization := range tc.authorization {
1071+
req.Header.Add("Authorization", authorization)
1072+
}
1073+
resp, requestErr := server.Client().Do(req)
1074+
if requestErr != nil {
1075+
t.Fatalf("readiness request: %v", requestErr)
1076+
}
1077+
body, readErr := io.ReadAll(resp.Body)
1078+
_ = resp.Body.Close()
1079+
if readErr != nil {
1080+
t.Fatalf("read unauthorized response: %v", readErr)
1081+
}
1082+
if resp.StatusCode != http.StatusUnauthorized {
1083+
t.Fatalf("readiness status: got %d want %d body=%s", resp.StatusCode, http.StatusUnauthorized, body)
1084+
}
1085+
if string(body) != "{\"error\":\"provider token is invalid\"}\n" {
1086+
t.Fatalf("unauthorized body: got %q", body)
1087+
}
1088+
for _, token := range []string{"provider-token", "submitted-wrong-token"} {
1089+
if strings.Contains(string(body), token) {
1090+
t.Fatalf("unauthorized response exposed token %q", token)
1091+
}
1092+
}
1093+
})
1094+
}
1095+
1096+
directReq := httptest.NewRequest(http.MethodGet, "/readyz", nil)
1097+
directReq.Header["Authorization"] = []string{"Bearer provider-token "}
1098+
directResp := httptest.NewRecorder()
1099+
module.HTTPHandler().ServeHTTP(directResp, directReq)
1100+
if directResp.Code != http.StatusUnauthorized {
1101+
t.Fatalf("direct trailing-whitespace status: got %d want %d", directResp.Code, http.StatusUnauthorized)
1102+
}
1103+
1104+
headReq, err := http.NewRequest(http.MethodHead, server.URL+"/readyz", nil)
1105+
if err != nil {
1106+
t.Fatalf("HEAD request: %v", err)
1107+
}
1108+
headReq.Header.Set("Authorization", "Bearer provider-token")
1109+
headResp, err := server.Client().Do(headReq)
1110+
if err != nil {
1111+
t.Fatalf("HEAD readiness request: %v", err)
1112+
}
1113+
_ = headResp.Body.Close()
1114+
if headResp.StatusCode != http.StatusMethodNotAllowed || headResp.Header.Get("Allow") != http.MethodGet {
1115+
t.Fatalf("HEAD readiness response: status=%d allow=%q", headResp.StatusCode, headResp.Header.Get("Allow"))
1116+
}
1117+
1118+
req, err := http.NewRequest(http.MethodGet, server.URL+"/readyz", nil)
1119+
if err != nil {
1120+
t.Fatalf("request: %v", err)
1121+
}
1122+
req.Header.Set("Authorization", "Bearer provider-token")
1123+
resp, err := server.Client().Do(req)
1124+
if err != nil {
1125+
t.Fatalf("authenticated readiness request: %v", err)
1126+
}
1127+
t.Cleanup(func() { _ = resp.Body.Close() })
1128+
if resp.StatusCode != http.StatusOK {
1129+
t.Fatalf("authenticated readiness status: got %d want %d", resp.StatusCode, http.StatusOK)
1130+
}
1131+
body, err := io.ReadAll(resp.Body)
1132+
if err != nil {
1133+
t.Fatalf("read readiness response: %v", err)
1134+
}
1135+
if string(body) != "{\"status\":\"ok\"}\n" {
1136+
t.Fatalf("readiness response: got %q", body)
1137+
}
1138+
}
1139+
10381140
func TestT916GitHubRunnerProviderHTTPRejectsConcatenatedJSON(t *testing.T) {
10391141
module, err := newGitHubRunnerProviderModule("provider", map[string]any{
10401142
"token": "github-token",

0 commit comments

Comments
 (0)