enhance(runner): add extra HTTP headers (#1269)

Add `runner.extra_headers` for reverse proxies that require request headers on the runner's API requests to Gitea, like registration, polling and log and status reports.

Headers the runner sets itself (authentication, User-Agent, protocol headers) stay authoritative. Extra headers are dropped when a redirect changes the hostname or leaves HTTPS, while the runner's own authentication headers follow redirects as before. Invalid or case-insensitively duplicated header names are rejected at config load.

Fixes: https://gitea.com/gitea/runner/issues/171
Co-authored-by: silverwind <[email protected]>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1269
Co-authored-by: bircni <[email protected]>

internal/pkg/client/http.go

@@ -6,6 +6,7 @@ package client
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"net/http"
 	"strings"
 	"time"
@@ -18,7 +19,7 @@ import (
 	"gitea.dev/actionslib/runner/v1/runnerv1connect"
 )
 
-func getHTTPClient(endpoint string, insecure bool, timeout time.Duration) *http.Client {
+func getHTTPClient(endpoint string, insecure bool, timeout time.Duration, extraHeaders map[string]string) *http.Client {
 	transport := &http.Transport{
 		Proxy:               http.ProxyFromEnvironment,
 		MaxIdleConns:        10,
@@ -30,17 +31,39 @@ func getHTTPClient(endpoint string, insecure bool, timeout time.Duration) *http.
 			InsecureSkipVerify: true,
 		}
 	}
-	return &http.Client{Transport: transport, Timeout: timeout}
+	return &http.Client{
+		Transport: transport,
+		Timeout:   timeout,
+		CheckRedirect: func(req *http.Request, via []*http.Request) error {
+			if len(via) >= 10 { // a custom CheckRedirect replaces net/http's own limit
+				return errors.New("stopped after 10 redirects")
+			}
+			from, to := via[0].URL, req.URL
+			if from.Hostname() != to.Hostname() || from.Scheme == "https" && to.Scheme != "https" {
+				for name, value := range extraHeaders {
+					if req.Header.Get(name) == value {
+						req.Header.Del(name)
+					}
+				}
+			}
+			return nil
+		},
+	}
 }
 
 // New returns a new runner client. timeout bounds every RPC: without it a
 // stalled connection parks the reporter for the whole job context, so logs and
 // heartbeats stop together and the task is reaped as a zombie.
-func New(endpoint string, insecure bool, uuid, token string, timeout time.Duration, opts ...connect.ClientOption) *HTTPClient {
+func New(endpoint string, insecure bool, uuid, token string, timeout time.Duration, extraHeaders map[string]string, opts ...connect.ClientOption) *HTTPClient {
 	baseURL := strings.TrimRight(endpoint, "/") + "/api/actions"
 
 	opts = append(opts, connect.WithInterceptors(connect.UnaryInterceptorFunc(func(next connect.UnaryFunc) connect.UnaryFunc {
 		return func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) {
+			for name, value := range extraHeaders {
+				if req.Header().Get(name) == "" { // keep protocol headers such as Content-Type
+					req.Header().Set(name, value)
+				}
+			}
 			req.Header().Set("User-Agent", "gitea-runner/"+ver.Version())
 			if uuid != "" {
 				req.Header().Set(protocol.UUIDHeader, uuid)
@@ -52,7 +75,7 @@ func New(endpoint string, insecure bool, uuid, token string, timeout time.Durati
 		}
 	})))
 
-	httpClient := getHTTPClient(endpoint, insecure, timeout)
+	httpClient := getHTTPClient(endpoint, insecure, timeout, extraHeaders)
 	return &HTTPClient{
 		PingServiceClient: pingv1connect.NewPingServiceClient(
 			httpClient,

internal/pkg/client/http_test.go

@@ -4,6 +4,7 @@
 package client
 
 import (
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -19,7 +20,7 @@ import (
 func TestGetHTTPClientUsesProxyFromEnvironment(t *testing.T) {
 	t.Setenv("HTTP_PROXY", "http://proxy.example.com:8080")
 
-	client := getHTTPClient("http://gitea.example.com", false, time.Minute)
+	client := getHTTPClient("http://gitea.example.com", false, time.Minute, nil)
 	require.Equal(t, time.Minute, client.Timeout)
 	transport, ok := client.Transport.(*http.Transport)
 	require.True(t, ok)
@@ -35,7 +36,7 @@ func TestGetHTTPClientUsesProxyFromEnvironment(t *testing.T) {
 
 func TestGetHTTPClientInsecureTLS(t *testing.T) {
 	// insecure only takes effect for https endpoints
-	httpsInsecure := getHTTPClient("https://gitea.example.com", true, time.Minute)
+	httpsInsecure := getHTTPClient("https://gitea.example.com", true, time.Minute, nil)
 	transport, ok := httpsInsecure.Transport.(*http.Transport)
 	require.True(t, ok)
 	require.NotNil(t, transport.TLSClientConfig)
@@ -50,7 +51,7 @@ func TestGetHTTPClientInsecureTLS(t *testing.T) {
 		{"http insecure ignored", "http://gitea.example.com", true},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			c := getHTTPClient(tc.endpoint, tc.insecure, time.Minute)
+			c := getHTTPClient(tc.endpoint, tc.insecure, time.Minute, nil)
 			tr, ok := c.Transport.(*http.Transport)
 			require.True(t, ok)
 			require.Nil(t, tr.TLSClientConfig)
@@ -69,7 +70,12 @@ func TestNewSetsBaseURLAndHeaders(t *testing.T) {
 	defer server.Close()
 
 	// trailing slash must be trimmed before "/api/actions" is appended
-	c := New(server.URL+"/", false, "the-uuid", "the-token", time.Minute)
+	c := New(server.URL+"/", false, "the-uuid", "the-token", time.Minute, map[string]string{
+		"X-Proxy-Token":      "proxy-token",
+		protocol.UUIDHeader:  "other-uuid",
+		protocol.TokenHeader: "other-token",
+		"Content-Type":       "text/plain",
+	})
 	// Address returns the endpoint as supplied (untrimmed)
 	require.Equal(t, server.URL+"/", c.Address())
 
@@ -79,6 +85,8 @@ func TestNewSetsBaseURLAndHeaders(t *testing.T) {
 	require.True(t, strings.HasPrefix(gotPath, "/api/actions/"), "unexpected path %q", gotPath)
 	require.Equal(t, "the-uuid", gotHeaders.Get(protocol.UUIDHeader))
 	require.Equal(t, "the-token", gotHeaders.Get(protocol.TokenHeader))
+	require.Equal(t, "proxy-token", gotHeaders.Get("X-Proxy-Token"))
+	require.Equal(t, "application/proto", gotHeaders.Get("Content-Type"))
 }
 
 func TestNewOmitsEmptyHeaders(t *testing.T) {
@@ -89,9 +97,52 @@ func TestNewOmitsEmptyHeaders(t *testing.T) {
 	}))
 	defer server.Close()
 
-	c := New(server.URL, false, "", "", time.Minute)
+	c := New(server.URL, false, "", "", time.Minute, nil)
 	_, _ = c.Ping(t.Context(), connect.NewRequest(&pingv1.PingRequest{Data: "hi"}))
 
 	require.Empty(t, gotHeaders.Get(protocol.UUIDHeader))
 	require.Empty(t, gotHeaders.Get(protocol.TokenHeader))
 }
+
+func TestNewStopsRedirectLoop(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, r.URL.Path, http.StatusTemporaryRedirect)
+	}))
+	defer server.Close()
+
+	c := New(server.URL, false, "", "", time.Minute, nil)
+	_, err := c.Ping(t.Context(), connect.NewRequest(&pingv1.PingRequest{Data: "hi"}))
+	require.ErrorContains(t, err, "stopped after 10 redirects")
+}
+
+func TestNewRedirectKeepsExtraHeadersOnlyOnSameHost(t *testing.T) {
+	var gotHeaders http.Header
+	target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotHeaders = r.Header.Clone()
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer target.Close()
+	_, port, err := net.SplitHostPort(target.Listener.Addr().String())
+	require.NoError(t, err)
+
+	for _, tc := range []struct {
+		newOrigin func(http.Handler) *httptest.Server
+		host      string
+		kept      bool
+	}{
+		{httptest.NewServer, "127.0.0.1", true},
+		{httptest.NewServer, "localhost", false},
+		{httptest.NewTLSServer, "127.0.0.1", false},
+	} {
+		gotHeaders = nil
+		origin := tc.newOrigin(http.RedirectHandler("http://"+net.JoinHostPort(tc.host, port), http.StatusPermanentRedirect))
+		c := New(origin.URL, true, "the-uuid", "the-token", time.Minute, map[string]string{"X-Proxy-Token": "proxy-token", "Content-Type": "text/plain"})
+		_, _ = c.Ping(t.Context(), connect.NewRequest(&pingv1.PingRequest{Data: "hi"}))
+		origin.Close()
+
+		require.NotNil(t, gotHeaders, origin.URL)
+		require.Equal(t, tc.kept, gotHeaders.Get("X-Proxy-Token") != "", "%s to %s", origin.URL, tc.host)
+		require.Equal(t, "the-token", gotHeaders.Get(protocol.TokenHeader))
+		require.Equal(t, "application/proto", gotHeaders.Get("Content-Type"))
+	}
+}