diff --git a/CLAUDE.md b/CLAUDE.md index f39db91..cf678fa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,6 +76,7 @@ task license-fix # Add missing license headers | `mcpcompat/mcp` | Drop-in shim for `mark3labs/mcp-go/mcp` data types; re-exports via aliases, backed by go-sdk elsewhere (Alpha) | | `mcpcompat/client` | Drop-in shim for `mark3labs/mcp-go/client` (+ `client/transport`) reimplemented on the official go-sdk (Alpha) | | `mcpcompat/server` | Drop-in shim for `mark3labs/mcp-go/server` reimplemented on the official go-sdk (Alpha) | +| `networking` | Outbound HTTP client construction with SSRF egress policy: private-IP/link-local dial blocking, redirect policy, body-capped JSON fetch, endpoint/issuer URL + private-IP validation helpers, and port allocation/validation utilities (Alpha) | | `telemetry/metrics` | Shared OTel histogram bucket presets, label-key constants, and emitter-ownership vocabulary (Alpha) | | `telemetry/reconcile` | Unified operator reconcile metric emitter (Alpha) | | `oci/artifact` | Artifact-agnostic OCI tar/gzip/extraction/platform primitives shared by oci/skills and oci/plugins (Alpha) | diff --git a/README.md b/README.md index a7012c3..7b21e0e 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ The ToolHive ecosystem spans multiple Go repositories, and several of these proj | `telemetry/reconcile` | Alpha | Unified operator reconcile metric emitter | | `oci/skills` | Alpha | OCI artifact types, media types, and registry operations for skills | | `oci/plugins` | Alpha | OCI artifact types, media types, and registry operations for plugins | +| `networking` | Alpha | Outbound HTTP client construction with SSRF egress policy: private-IP/link-local dial blocking, redirect policy, body-capped JSON fetch, endpoint/issuer URL + private-IP validation helpers, and port allocation/validation utilities | | `postgres` | Alpha | PostgreSQL connection pool with optional AWS RDS IAM dynamic auth | | `recovery` | Beta | HTTP panic recovery middleware | | `validation/http` | Stable | RFC 7230/8707 compliant HTTP header and URI validation | diff --git a/go.mod b/go.mod index a5c12cd..b948c96 100644 --- a/go.mod +++ b/go.mod @@ -32,12 +32,14 @@ require ( require ( github.com/prometheus/client_golang v1.24.1 + github.com/shirou/gopsutil/v4 v4.26.7 github.com/sigstore/sigstore v1.10.9 go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.45.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.45.0 go.opentelemetry.io/otel/exporters/prometheus v0.67.0 go.opentelemetry.io/otel/sdk v1.45.0 go.opentelemetry.io/otel/trace v1.45.0 + golang.org/x/oauth2 v0.36.0 ) require ( @@ -67,9 +69,11 @@ require ( github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7 // indirect github.com/docker/cli v29.6.2+incompatible // indirect github.com/docker/docker-credential-helpers v0.9.3 // indirect + github.com/ebitengine/purego v0.10.2 // indirect github.com/go-jose/go-jose/v4 v4.1.4 // indirect github.com/go-logr/logr v1.4.4 // indirect github.com/go-logr/stdr v1.2.2 // indirect + github.com/go-ole/go-ole v1.2.6 // indirect github.com/go-openapi/analysis v0.25.5 // indirect github.com/go-openapi/errors v0.22.8 // indirect github.com/go-openapi/jsonpointer v1.0.0 // indirect @@ -112,6 +116,7 @@ require ( github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect + github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.70.1 // indirect github.com/prometheus/otlptranslator v1.0.0 // indirect @@ -136,6 +141,7 @@ require ( github.com/yosida95/uritemplate/v3 v3.0.2 // indirect github.com/youmark/pkcs8 v0.0.0-20240726163527-a2c0da244d78 // indirect github.com/yuin/gopher-lua v1.1.1 // indirect + github.com/yusufpapurcu/wmi v1.2.4 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.45.0 // indirect go.opentelemetry.io/proto/otlp v1.11.0 // indirect @@ -144,7 +150,6 @@ require ( golang.org/x/crypto v0.54.0 // indirect golang.org/x/exp v0.0.0-20251023183803-a4bb9ffd2546 // indirect golang.org/x/mod v0.38.0 // indirect - golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/term v0.45.0 // indirect diff --git a/go.sum b/go.sum index 31449cb..9238fb4 100644 --- a/go.sum +++ b/go.sum @@ -109,6 +109,8 @@ github.com/docker/cli v29.6.2+incompatible h1:/bjePvcbbFTnRrMfWJBY7AjfICdsiLVgHn github.com/docker/cli v29.6.2+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8= github.com/docker/docker-credential-helpers v0.9.3 h1:gAm/VtF9wgqJMoxzT3Gj5p4AqIjCBS4wrsOh9yRqcz8= github.com/docker/docker-credential-helpers v0.9.3/go.mod h1:x+4Gbw9aGmChi3qTLZj8Dfn0TD20M/fuWy0E5+WDeCo= +github.com/ebitengine/purego v0.10.2 h1:W809HbnvzAxgdm+aOvlSekrM16wGCdT/e76+9tS7gzE= +github.com/ebitengine/purego v0.10.2/go.mod h1:iIjxzd6CiRiOG0UyXP+V1+jWqUXVjPKLAI0mRfJZTmQ= github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= @@ -122,6 +124,8 @@ github.com/go-logr/logr v1.4.4 h1:tG4xh9yMsRCAiodLVTxyrkzSZ9+o0L1Kg/+cPVcbP/8= github.com/go-logr/logr v1.4.4/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= +github.com/go-ole/go-ole v1.2.6 h1:/Fpf6oFPoeFik9ty7siob0G6Ke8QvQEuVcuChpwXzpY= +github.com/go-ole/go-ole v1.2.6/go.mod h1:pprOEPIfldk/42T2oK7lQ4v4JSDwmV0As9GaiUsvbm0= github.com/go-openapi/analysis v0.25.5 h1:xPYEvTb90o1y0epuiOPAoG4QqahjP3cdp5xNlHeKJRI= github.com/go-openapi/analysis v0.25.5/go.mod h1:d3UGtQC5uq5Kqqqis2VH09Km/v3vwsWrYkbp4gdm+Rc= github.com/go-openapi/errors v0.22.8 h1:oP7sW7TWc3wFFjrzzj0nI83H2qMBkNjNfSd+XRejk/I= @@ -294,6 +298,8 @@ github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 h1:o4JXh1EVt9k/+g42oCprj/FisM4qX9L3sZB3upGN2ZU= +github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE= github.com/prometheus/client_golang v1.24.1 h1:JnJkREXzWxUdCuPFpIWZiPispT9xVV59uiuyR2bPlnU= github.com/prometheus/client_golang v1.24.1/go.mod h1:F+oSRECHg4sse5ucfYpYDeIv/hu68Zo0uoHKetWnzcE= github.com/prometheus/client_model v0.6.2 h1:oBsgwpGs7iVziMvrGhE53c/GrLUsZdHnqNwqPLxwZyk= @@ -325,6 +331,8 @@ github.com/sergi/go-diff v1.4.0 h1:n/SP9D5ad1fORl+llWyN+D6qoUETXNZARKjyY2/KVCw= github.com/sergi/go-diff v1.4.0/go.mod h1:A0bzQcvG0E7Rwjx0REVgAGH58e96+X0MeOfepqsbeW4= github.com/shibumi/go-pathspec v1.3.0 h1:QUyMZhFo0Md5B8zV8x2tesohbb5kfbpTi9rBnKh5dkI= github.com/shibumi/go-pathspec v1.3.0/go.mod h1:Xutfslp817l2I1cZvgcfeMQJG5QnU2lh5tVaaMCl3jE= +github.com/shirou/gopsutil/v4 v4.26.7 h1:IXzpHz/dkMRYAhKkOXr1HB6SuzWU3eoyyeWe7g3bNZc= +github.com/shirou/gopsutil/v4 v4.26.7/go.mod h1:5O9FjBiXoTDFatIWjZZosqj4pV0DRtLx598xGbBehzM= github.com/sigstore/protobuf-specs v0.5.1 h1:/5OPaNuolRJmQfeZLayJGFXMpsRJEdgC6ah1/+7Px7U= github.com/sigstore/protobuf-specs v0.5.1/go.mod h1:DRBzpFuE+LnvQMN10/dU6nBeKwVLGEQ6o2FovN2Rats= github.com/sigstore/rekor v1.5.3 h1:0Tyolw3zreRgm7PUW8dccFLXGBThi08278jI8EXNSr4= @@ -395,6 +403,8 @@ github.com/ysmood/leakless v0.9.0 h1:qxCG5VirSBvmi3uynXFkcnLMzkphdh3xx5FtrORwDCU github.com/ysmood/leakless v0.9.0/go.mod h1:R8iAXPRaG97QJwqxs74RdwzcRHT1SWCGTNqY8q0JvMQ= github.com/yuin/gopher-lua v1.1.1 h1:kYKnWBjvbNP4XLT3+bPEwAXJx262OhaHDWDVOPjL46M= github.com/yuin/gopher-lua v1.1.1/go.mod h1:GBR0iDaNXjAgGg9zfCvksxSRnQx76gclCIb7kdAd1Pw= +github.com/yusufpapurcu/wmi v1.2.4 h1:zFUKzehAFReQwLys1b/iSMl+JQGSCSjtVqQn9bBrPo0= +github.com/yusufpapurcu/wmi v1.2.4/go.mod h1:SBZ9tNy3G9/m5Oi98Zks0QjeHVDvuK0qfxQmPyzfmi0= github.com/zalando/go-keyring v0.2.3 h1:v9CUu9phlABObO4LPWycf+zwMG7nlbb3t/B5wa97yms= github.com/zalando/go-keyring v0.2.3/go.mod h1:HL4k+OXQfJUWaMnqyuSOc0drfGPX2b51Du6K+MRgZMk= github.com/zeebo/xxh3 v1.1.0 h1:s7DLGDK45Dyfg7++yxI0khrfwq9661w9EN78eP/UZVs= @@ -455,6 +465,8 @@ golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sys v0.0.0-20190916202348-b4ddaad3f8a3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20201204225414-ed752295db88/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= diff --git a/networking/doc.go b/networking/doc.go new file mode 100644 index 0000000..4f58918 --- /dev/null +++ b/networking/doc.go @@ -0,0 +1,9 @@ +// SPDX-FileCopyrightText: Copyright 2026 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Package networking provides outbound HTTP client construction with an +// SSRF/egress policy: private-IP and link-local dial blocking, a same-host +// redirect policy, and a body-capped JSON fetch helper. +// +// Status: Alpha. The API may change without notice. +package networking diff --git a/networking/fetch.go b/networking/fetch.go new file mode 100644 index 0000000..af32190 --- /dev/null +++ b/networking/fetch.go @@ -0,0 +1,196 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking + +import ( + "context" + "encoding/json" + "fmt" + "io" + "mime" + "net/http" + "net/url" + "strings" +) + +const ( + // maxResponseSize is the maximum response body size (1MB). + maxResponseSize = 1024 * 1024 + + // contentTypeJSON is the JSON content type. + contentTypeJSON = "application/json" + + // contentTypeFormURLEncoded is the form-urlencoded content type. + contentTypeFormURLEncoded = "application/x-www-form-urlencoded" +) + +// FetchResult contains the result of a successful JSON fetch operation. +type FetchResult[T any] struct { + // Data is the parsed JSON response body. + Data T + + // Headers are the response headers. + Headers http.Header +} + +// FetchOption configures a fetch request. +type FetchOption func(*fetchOptions) + +// fetchOptions holds the configuration for a fetch request. +type fetchOptions struct { + method string + headers http.Header + body io.Reader + errorHandler func(*http.Response, []byte) error + maxResponseSize int64 +} + +// newFetchOptions creates default fetch options. +func newFetchOptions() *fetchOptions { + return &fetchOptions{ + method: http.MethodGet, + headers: make(http.Header), + maxResponseSize: maxResponseSize, + } +} + +// WithMethod sets the HTTP method for the request. +func WithMethod(method string) FetchOption { + return func(opts *fetchOptions) { + opts.method = method + } +} + +// WithHeader adds a single header to the request. +func WithHeader(key, value string) FetchOption { + return func(opts *fetchOptions) { + opts.headers.Set(key, value) + } +} + +// WithBody sets the request body. +func WithBody(body io.Reader) FetchOption { + return func(opts *fetchOptions) { + opts.body = body + } +} + +// WithMaxResponseSize sets a custom maximum response body size in bytes. +// The default is 1 MB. Use this to enforce tighter limits for endpoints that +// are expected to return small documents (e.g. OAuth metadata, CIMD documents). +func WithMaxResponseSize(size int64) FetchOption { + return func(opts *fetchOptions) { + opts.maxResponseSize = size + } +} + +// WithErrorHandler sets a custom error handler for non-200 responses. +// The handler receives the response and body, and should return an error. +// If the handler returns nil, the default HTTPError will be returned. +// This is useful for parsing structured error responses (e.g., OAuth error responses). +func WithErrorHandler(handler func(*http.Response, []byte) error) FetchOption { + return func(opts *fetchOptions) { + opts.errorHandler = handler + } +} + +// FetchJSON performs an HTTP request and parses the JSON response body. +// It sets the Accept header to application/json by default. +// For non-200 responses, it returns an HTTPError or the result of a custom error handler. +func FetchJSON[T any]( + ctx context.Context, + client HTTPClient, + requestURL string, + opts ...FetchOption, +) (*FetchResult[T], error) { + options := newFetchOptions() + for _, opt := range opts { + opt(options) + } + + // Set default Accept header if not already set + if options.headers.Get("Accept") == "" { + options.headers.Set("Accept", contentTypeJSON) + } + + req, err := http.NewRequestWithContext(ctx, options.method, requestURL, options.body) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + // Apply headers + for key, values := range options.headers { + for _, value := range values { + req.Header.Add(key, value) + } + } + + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("request failed: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + // Read body with size limit + body, err := io.ReadAll(io.LimitReader(resp.Body, options.maxResponseSize)) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + + // Handle non-200 responses + if resp.StatusCode != http.StatusOK { + // Try custom error handler first + if options.errorHandler != nil { + if customErr := options.errorHandler(resp, body); customErr != nil { + return nil, customErr + } + } + + // Fall back to default HTTPError using status text to avoid leaking sensitive body content + return nil, NewHTTPError(resp.StatusCode, requestURL, resp.Status) + } + + // Validate Content-Type for successful responses. Accept application/json + // and application/*+json subtypes (RFC 6839) — the old strings.Contains check + // incorrectly rejected valid subtypes like application/ld+json. + contentType := resp.Header.Get("Content-Type") + mediaType, _, _ := mime.ParseMediaType(contentType) + if mediaType != contentTypeJSON && (!strings.HasPrefix(mediaType, "application/") || !strings.HasSuffix(mediaType, "+json")) { + return nil, fmt.Errorf("unexpected content type: %s", contentType) + } + + // Parse JSON response + var data T + if err := json.Unmarshal(body, &data); err != nil { + return nil, fmt.Errorf("failed to parse JSON response: %w", err) + } + + return &FetchResult[T]{ + Data: data, + Headers: resp.Header, + }, nil +} + +// FetchJSONWithForm performs a POST request with form-urlencoded body and parses JSON response. +// This is a convenience wrapper around FetchJSON for token endpoints and similar APIs. +// It sets Content-Type to application/x-www-form-urlencoded and Accept to application/json. +func FetchJSONWithForm[T any]( + ctx context.Context, + client HTTPClient, + requestURL string, + formData url.Values, + opts ...FetchOption, +) (*FetchResult[T], error) { + // Prepend form-specific options + formOpts := []FetchOption{ + WithMethod(http.MethodPost), + WithHeader("Content-Type", contentTypeFormURLEncoded), + WithBody(strings.NewReader(formData.Encode())), + } + + // Append user options (they can override form options if needed) + allOpts := append(formOpts, opts...) + + return FetchJSON[T](ctx, client, requestURL, allOpts...) +} diff --git a/networking/fetch_test.go b/networking/fetch_test.go new file mode 100644 index 0000000..784e3a2 --- /dev/null +++ b/networking/fetch_test.go @@ -0,0 +1,530 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// testResponse is a sample response type for testing. +type testResponse struct { + Message string `json:"message"` + Value int `json:"value"` +} + +func TestFetchJSON_SuccessfulGET(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "application/json", r.Header.Get("Accept")) + + w.Header().Set("Content-Type", "application/json") + w.Header().Set("X-Custom-Header", "test-value") + _ = json.NewEncoder(w).Encode(testResponse{Message: "hello", Value: 42}) + })) + defer server.Close() + + ctx := context.Background() + client := server.Client() + + result, err := FetchJSON[testResponse](ctx, client, server.URL) + require.NoError(t, err) + + assert.Equal(t, "hello", result.Data.Message) + assert.Equal(t, 42, result.Data.Value) + assert.Equal(t, "test-value", result.Headers.Get("X-Custom-Header")) +} + +func TestFetchJSON_SuccessfulPOST(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "application/json", r.Header.Get("Content-Type")) + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(testResponse{Message: "created", Value: 1}) + })) + defer server.Close() + + ctx := context.Background() + client := server.Client() + + body := strings.NewReader(`{"input": "test"}`) + result, err := FetchJSON[testResponse](ctx, client, server.URL, + WithMethod(http.MethodPost), + WithHeader("Content-Type", "application/json"), + WithBody(body), + ) + require.NoError(t, err) + + assert.Equal(t, "created", result.Data.Message) + assert.Equal(t, 1, result.Data.Value) +} + +func TestFetchJSONWithForm_Success(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "application/x-www-form-urlencoded", r.Header.Get("Content-Type")) + assert.Equal(t, "application/json", r.Header.Get("Accept")) + + err := r.ParseForm() + require.NoError(t, err) + assert.Equal(t, "authorization_code", r.Form.Get("grant_type")) + assert.Equal(t, "test-code", r.Form.Get("code")) + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(testResponse{Message: "token", Value: 3600}) + })) + defer server.Close() + + ctx := context.Background() + client := server.Client() + + formData := url.Values{ + "grant_type": {"authorization_code"}, + "code": {"test-code"}, + } + + result, err := FetchJSONWithForm[testResponse](ctx, client, server.URL, formData) + require.NoError(t, err) + + assert.Equal(t, "token", result.Data.Message) + assert.Equal(t, 3600, result.Data.Value) +} + +func TestFetchJSON_HTTPError4xx(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + statusCode int + expectedStatus string + }{ + {"bad request", http.StatusBadRequest, "400 Bad Request"}, + {"unauthorized", http.StatusUnauthorized, "401 Unauthorized"}, + {"forbidden", http.StatusForbidden, "403 Forbidden"}, + {"not found", http.StatusNotFound, "404 Not Found"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(tt.statusCode) + // Write some body content that should NOT appear in error message + _, _ = w.Write([]byte("sensitive error details")) + })) + defer server.Close() + + ctx := context.Background() + client := server.Client() + + result, err := FetchJSON[testResponse](ctx, client, server.URL) + assert.Nil(t, result) + require.Error(t, err) + + var httpErr *HTTPError + require.True(t, errors.As(err, &httpErr)) + assert.Equal(t, tt.statusCode, httpErr.StatusCode) + // Error message should be HTTP status text, not body content + assert.Equal(t, tt.expectedStatus, httpErr.Message) + assert.Equal(t, server.URL, httpErr.URL) + // Verify body content is not leaked + assert.NotContains(t, httpErr.Message, "sensitive") + }) + } +} + +func TestFetchJSON_HTTPError5xx(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + statusCode int + }{ + {"internal server error", http.StatusInternalServerError}, + {"bad gateway", http.StatusBadGateway}, + {"service unavailable", http.StatusServiceUnavailable}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(tt.statusCode) + _, _ = w.Write([]byte("server error")) + })) + defer server.Close() + + ctx := context.Background() + client := server.Client() + + result, err := FetchJSON[testResponse](ctx, client, server.URL) + assert.Nil(t, result) + require.Error(t, err) + + assert.True(t, IsHTTPError(err, tt.statusCode)) + }) + } +} + +func TestFetchJSON_ContentTypeValidation(t *testing.T) { + t.Parallel() + + t.Run("valid content type", func(t *testing.T) { + t.Parallel() + + contentTypes := []string{ + "application/json", + "application/json; charset=utf-8", + "APPLICATION/JSON", + "application/json;charset=UTF-8", + } + + for _, ct := range contentTypes { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", ct) + _ = json.NewEncoder(w).Encode(testResponse{Message: "ok"}) + })) + + ctx := context.Background() + result, err := FetchJSON[testResponse](ctx, server.Client(), server.URL) + + require.NoError(t, err, "content type %q should be valid", ct) + assert.Equal(t, "ok", result.Data.Message) + + server.Close() + } + }) + + t.Run("invalid content type", func(t *testing.T) { + t.Parallel() + + invalidContentTypes := []string{ + "text/plain", + "text/html", + "application/xml", + "", + } + + for _, ct := range invalidContentTypes { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if ct != "" { + w.Header().Set("Content-Type", ct) + } + _ = json.NewEncoder(w).Encode(testResponse{Message: "ok"}) + })) + + ctx := context.Background() + _, err := FetchJSON[testResponse](ctx, server.Client(), server.URL) + + require.Error(t, err, "content type %q should be invalid", ct) + assert.Contains(t, err.Error(), "unexpected content type") + + server.Close() + } + }) +} + +func TestFetchJSON_ErrorDoesNotLeakBody(t *testing.T) { + t.Parallel() + + // Even with a large body containing sensitive data, the error should only show status text + largeBody := strings.Repeat("sensitive-data-", 500) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(largeBody)) + })) + defer server.Close() + + ctx := context.Background() + _, err := FetchJSON[testResponse](ctx, server.Client(), server.URL) + + require.Error(t, err) + var httpErr *HTTPError + require.True(t, errors.As(err, &httpErr)) + // Error message should be HTTP status text, not body content + assert.Equal(t, "400 Bad Request", httpErr.Message) + assert.NotContains(t, httpErr.Message, "sensitive") +} + +func TestFetchJSON_CustomHeaders(t *testing.T) { + t.Parallel() + + t.Run("single header", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "Bearer test-token", r.Header.Get("Authorization")) + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(testResponse{Message: "authenticated"}) + })) + defer server.Close() + + ctx := context.Background() + result, err := FetchJSON[testResponse](ctx, server.Client(), server.URL, + WithHeader("Authorization", "Bearer test-token"), + ) + + require.NoError(t, err) + assert.Equal(t, "authenticated", result.Data.Message) + }) + + t.Run("multiple headers", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "Bearer token", r.Header.Get("Authorization")) + assert.Equal(t, "custom-value", r.Header.Get("X-Custom")) + assert.Equal(t, "request-123", r.Header.Get("X-Request-ID")) + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(testResponse{Message: "ok"}) + })) + defer server.Close() + + ctx := context.Background() + result, err := FetchJSON[testResponse](ctx, server.Client(), server.URL, + WithHeader("Authorization", "Bearer token"), + WithHeader("X-Custom", "custom-value"), + WithHeader("X-Request-ID", "request-123"), + ) + + require.NoError(t, err) + assert.Equal(t, "ok", result.Data.Message) + }) + + t.Run("override Accept header", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Custom Accept header should override the default + assert.Equal(t, "application/vnd.api+json", r.Header.Get("Accept")) + + // Server responds with JSON content type (validated by FetchJSON) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(testResponse{Message: "custom"}) + })) + defer server.Close() + + ctx := context.Background() + result, err := FetchJSON[testResponse](ctx, server.Client(), server.URL, + WithHeader("Accept", "application/vnd.api+json"), + ) + + require.NoError(t, err) + assert.Equal(t, "custom", result.Data.Message) + }) +} + +func TestFetchJSON_CustomErrorHandler(t *testing.T) { + t.Parallel() + + // oauthError represents an OAuth error response + type oauthError struct { + Error string `json:"error"` + ErrorDescription string `json:"error_description"` + } + + t.Run("error handler returns custom error", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + _ = json.NewEncoder(w).Encode(oauthError{ + Error: "invalid_grant", + ErrorDescription: "The authorization code has expired", + }) + })) + defer server.Close() + + customHandler := func(_ *http.Response, body []byte) error { + var oauthErr oauthError + if err := json.Unmarshal(body, &oauthErr); err == nil && oauthErr.Error != "" { + return fmt.Errorf("oauth error: %s - %s", oauthErr.Error, oauthErr.ErrorDescription) + } + return nil // Fall back to default HTTPError + } + + ctx := context.Background() + _, err := FetchJSON[testResponse](ctx, server.Client(), server.URL, + WithErrorHandler(customHandler), + ) + + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid_grant") + assert.Contains(t, err.Error(), "The authorization code has expired") + // Should NOT be an HTTPError since custom handler returned an error + assert.False(t, IsHTTPError(err, 0)) + }) + + t.Run("error handler returns nil falls back to HTTPError", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte("internal error")) + })) + defer server.Close() + + customHandler := func(_ *http.Response, _ []byte) error { + // Return nil to fall back to default HTTPError + return nil + } + + ctx := context.Background() + _, err := FetchJSON[testResponse](ctx, server.Client(), server.URL, + WithErrorHandler(customHandler), + ) + + require.Error(t, err) + assert.True(t, IsHTTPError(err, http.StatusInternalServerError)) + }) +} + +func TestFetchJSON_ContextCancellation(t *testing.T) { + t.Parallel() + + t.Run("cancelled context", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + // Delay response to allow cancellation + time.Sleep(100 * time.Millisecond) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(testResponse{Message: "too late"}) + })) + defer server.Close() + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // Cancel immediately + + _, err := FetchJSON[testResponse](ctx, server.Client(), server.URL) + + require.Error(t, err) + assert.True(t, errors.Is(err, context.Canceled)) + }) + + t.Run("context timeout", func(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + // Delay response longer than timeout + time.Sleep(200 * time.Millisecond) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(testResponse{Message: "too late"}) + })) + defer server.Close() + + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + + _, err := FetchJSON[testResponse](ctx, server.Client(), server.URL) + + require.Error(t, err) + assert.True(t, errors.Is(err, context.DeadlineExceeded)) + }) +} + +func TestFetchJSON_InvalidJSON(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("not valid json")) + })) + defer server.Close() + + ctx := context.Background() + _, err := FetchJSON[testResponse](ctx, server.Client(), server.URL) + + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to parse JSON") +} + +func TestFetchJSON_EmptyResponse(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("{}")) + })) + defer server.Close() + + ctx := context.Background() + result, err := FetchJSON[testResponse](ctx, server.Client(), server.URL) + + require.NoError(t, err) + assert.Equal(t, "", result.Data.Message) + assert.Equal(t, 0, result.Data.Value) +} + +func TestFetchJSON_InvalidURL(t *testing.T) { + t.Parallel() + + ctx := context.Background() + client := &http.Client{} + + _, err := FetchJSON[testResponse](ctx, client, "://invalid-url") + + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to create request") +} + +func TestFetchJSON_NetworkError(t *testing.T) { + t.Parallel() + + ctx := context.Background() + client := &http.Client{Timeout: 100 * time.Millisecond} + + // Use a URL that will fail to connect + _, err := FetchJSON[testResponse](ctx, client, "http://localhost:1") + + require.Error(t, err) + assert.Contains(t, err.Error(), "request failed") +} + +func TestFetchJSONWithForm_AdditionalOptions(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "application/x-www-form-urlencoded", r.Header.Get("Content-Type")) + assert.Equal(t, "Bearer token", r.Header.Get("Authorization")) + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(testResponse{Message: "with auth"}) + })) + defer server.Close() + + ctx := context.Background() + formData := url.Values{"key": {"value"}} + + result, err := FetchJSONWithForm[testResponse](ctx, server.Client(), server.URL, formData, + WithHeader("Authorization", "Bearer token"), + ) + + require.NoError(t, err) + assert.Equal(t, "with auth", result.Data.Message) +} diff --git a/networking/http_client.go b/networking/http_client.go new file mode 100644 index 0000000..7ebb175 --- /dev/null +++ b/networking/http_client.go @@ -0,0 +1,361 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking + +import ( + "context" + "crypto/tls" + "crypto/x509" + "errors" + "fmt" + "net" + "net/http" + "net/url" + "os" + "strings" + "syscall" + "time" + + "golang.org/x/oauth2" + + "github.com/stacklok/toolhive-core/env" +) + +// HTTPClient is an interface for making HTTP requests. +// This interface is satisfied by *http.Client and allows for dependency injection in testing. +type HTTPClient interface { + Do(req *http.Request) (*http.Response, error) +} + +var privateIPBlocks []*net.IPNet + +// HttpTimeout is the timeout for outgoing HTTP requests +const HttpTimeout = 30 * time.Second + +// HttpsScheme is the HTTPS scheme +const HttpsScheme = "https" + +// HttpScheme is the HTTP scheme +const HttpScheme = "http" + +// MaxRedirects bounds how many HTTP redirects an SSRF-guarded client follows +// before giving up. Matches the cap used by the transparent proxy data path. +const MaxRedirects = 10 + +// ErrRedirectRefused is wrapped by SameHostRedirectPolicy when it declines to +// follow a redirect, so callers can match it with errors.Is. +var ErrRedirectRefused = errors.New("redirect refused") + +// SameHostRedirectPolicy returns a value for http.Client.CheckRedirect that +// follows only same-host redirects, refuses HTTPS-to-HTTP downgrades, and caps +// the chain at MaxRedirects. +// +// Any client that fetches a URL derived from an untrusted remote server — auth +// discovery probes, RFC 9728 resource-metadata fetches, OIDC issuer discovery — +// must install this. Validating only the originally-supplied URL is not enough: +// a malicious server can return a 30x that points the request at an internal +// address (cloud IMDS, RFC1918 services), and the host-side client would follow +// it (CWE-918). Restricting redirects to the same host as the original request +// keeps the request on the endpoint the operator actually configured. +// +// This mirrors the data-plane guard in the toolhive proxy transport +// (github.com/stacklok/toolhive pkg/transport/proxy/transparent, followRedirects); +// if you change this policy, check the corresponding guard there too. +func SameHostRedirectPolicy() func(req *http.Request, via []*http.Request) error { + return func(req *http.Request, via []*http.Request) error { + if len(via) >= MaxRedirects { + return fmt.Errorf("stopped after %d redirects: %w", MaxRedirects, ErrRedirectRefused) + } + // via[0] is the original request; CheckRedirect is only invoked once at + // least one redirect has occurred, so via is never empty. + original := via[0] + // Compare host:port (not just hostname): a redirect to a different port + // on the same host can reach a different internal service (a co-located + // metadata/admin port), so it must be treated as cross-host. + if !strings.EqualFold(req.URL.Host, original.URL.Host) { + return fmt.Errorf("refusing cross-host redirect to %q (original host %q): %w", + req.URL.Host, original.URL.Host, ErrRedirectRefused) + } + if original.URL.Scheme == HttpsScheme && req.URL.Scheme != HttpsScheme { + return fmt.Errorf("refusing redirect that downgrades from HTTPS to %q: %w", + req.URL.Scheme, ErrRedirectRefused) + } + return nil + } +} + +// Dialer control function for validating addresses prior to connection +func protectedDialerControl(_, address string, _ syscall.RawConn) error { + err := AddressReferencesPrivateIp(address) + if err != nil { + return err + } + + return nil +} + +// NewPrivateIPBlockingDialContext returns a DialContext that refuses to connect +// to private, loopback, or link-local addresses. The check runs after DNS +// resolution on the address actually being dialed, so it also defends against +// DNS rebinding and is re-applied on every redirect hop. Pair it with +// Transport.DisableKeepAlives so a pooled connection cannot skip the check on a +// later request. +// +// Use this on clients that fetch a URL derived from untrusted input when the +// operator-configured target is public; SameHostRedirectPolicy is the +// redirect-following counterpart. +func NewPrivateIPBlockingDialContext() func(ctx context.Context, network, addr string) (net.Conn, error) { + return (&net.Dialer{Control: protectedDialerControl}).DialContext +} + +// ValidatingTransport is for validating URLs prior to request +type ValidatingTransport struct { + Transport http.RoundTripper + InsecureAllowHTTP bool + + // EnvReader is consulted for INSECURE_DISABLE_URL_VALIDATION. A nil value + // falls back to the real OS environment, so the zero value of this struct + // preserves the historical os.Getenv-backed behavior. + EnvReader env.Reader +} + +// RoundTrip validates the request URL prior to forwarding +func (t *ValidatingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + reader := t.EnvReader + if reader == nil { + reader = &env.OSReader{} + } + + // Skip validation if INSECURE_DISABLE_URL_VALIDATION is set or if InsecureAllowHTTP is true + if strings.EqualFold(reader.Getenv("INSECURE_DISABLE_URL_VALIDATION"), "true") || t.InsecureAllowHTTP { + return t.Transport.RoundTrip(req) + } + + // Check for valid URL specification + parsedUrl, err := url.Parse(req.URL.String()) + if err != nil { + return nil, fmt.Errorf("the supplied URL %s is malformed", req.URL.String()) + } + + // Check for HTTPS scheme + if parsedUrl.Scheme != HttpsScheme { + return nil, fmt.Errorf("the supplied URL %s is not HTTPS scheme", req.URL.String()) + } + + return t.Transport.RoundTrip(req) +} + +// createTokenSourceFromFile creates an oauth2.TokenSource from a token file +func createTokenSourceFromFile(tokenFile string) (oauth2.TokenSource, error) { + tokenBytes, err := os.ReadFile(tokenFile) // #nosec G304 - tokenFile path is provided by user via CLI flag + if err != nil { + return nil, fmt.Errorf("failed to read auth token file: %w", err) + } + + // Remove any trailing newlines/whitespace + tokenStr := strings.TrimSpace(string(tokenBytes)) + if tokenStr == "" { + return nil, fmt.Errorf("auth token file is empty") + } + + // Create a static token source + token := &oauth2.Token{ + AccessToken: tokenStr, + TokenType: "Bearer", + } + + return oauth2.StaticTokenSource(token), nil +} + +// HttpClientBuilder provides a fluent interface for building HTTP clients +type HttpClientBuilder struct { + clientTimeout time.Duration + tlsHandshakeTimeout time.Duration + responseHeaderTimeout time.Duration + caCertPath string + authTokenFile string + allowPrivate bool + insecureAllowHTTP bool + envReader env.Reader +} + +// NewHttpClientBuilder returns a new HttpClientBuilder +func NewHttpClientBuilder() *HttpClientBuilder { + return &HttpClientBuilder{ + clientTimeout: HttpTimeout, + tlsHandshakeTimeout: 10 * time.Second, + responseHeaderTimeout: 10 * time.Second, + envReader: &env.OSReader{}, + } +} + +// NewHostScopedClientBuilder returns an HttpClientBuilder pre-configured with +// the SSRF-guard policy (CWE-918) appropriate for dialing host. By default the +// returned builder blocks plain HTTP and connections to private/loopback/ +// link-local IP ranges. Both gates are relaxed automatically for loopback +// hosts (development/testing) and when INSECURE_DISABLE_URL_VALIDATION is set. +// +// allowPrivateIPs widens only the private-IP gate — for example an in-cluster +// provider reachable solely over an RFC-1918 address — without enabling plain +// HTTP for non-loopback hosts. insecureAllowHTTP additionally permits +// plain-HTTP for non-loopback hosts and must never be set in production. +// +// The returned builder is not yet built: callers may chain further options +// (e.g. WithTimeout, WithEnvReader) before calling Build. This is the +// single source of truth for the host-scoped guard policy shared by the +// upstream OAuth2/OIDC providers and the DCR resolver so the two paths cannot +// drift. +func NewHostScopedClientBuilder(host string, allowPrivateIPs, insecureAllowHTTP bool) *HttpClientBuilder { + return NewHostScopedClientBuilderWithReader(host, allowPrivateIPs, insecureAllowHTTP, &env.OSReader{}) +} + +// NewHostScopedClientBuilderWithReader is identical to NewHostScopedClientBuilder +// but takes the env.Reader used to evaluate INSECURE_DISABLE_URL_VALIDATION +// explicitly, so callers can inject a fake reader for tests instead of relying +// on a later WithEnvReader call, which would be too late to affect this +// constructor's own allowInsecure computation. +func NewHostScopedClientBuilderWithReader( + host string, allowPrivateIPs, insecureAllowHTTP bool, reader env.Reader, +) *HttpClientBuilder { + builder := NewHttpClientBuilder().WithEnvReader(reader) + allowInsecure := IsLocalhost(host) || + insecureAllowHTTP || + strings.EqualFold(builder.envReader.Getenv("INSECURE_DISABLE_URL_VALIDATION"), "true") + return builder. + WithInsecureAllowHTTP(allowInsecure). + WithPrivateIPs(allowInsecure || allowPrivateIPs) +} + +// WithCABundle sets the CA certificate bundle path +func (b *HttpClientBuilder) WithCABundle(path string) *HttpClientBuilder { + b.caCertPath = path + return b +} + +// WithTokenFromFile sets the auth token file path +func (b *HttpClientBuilder) WithTokenFromFile(path string) *HttpClientBuilder { + b.authTokenFile = path + return b +} + +// WithPrivateIPs allows connections to private IP addresses +func (b *HttpClientBuilder) WithPrivateIPs(allow bool) *HttpClientBuilder { + b.allowPrivate = allow + return b +} + +// WithInsecureAllowHTTP allows HTTP (non-HTTPS) URLs +// WARNING: This is insecure and should NEVER be used in production +func (b *HttpClientBuilder) WithInsecureAllowHTTP(allow bool) *HttpClientBuilder { + b.insecureAllowHTTP = allow + return b +} + +// WithTimeout sets the HTTP client timeout +func (b *HttpClientBuilder) WithTimeout(timeout time.Duration) *HttpClientBuilder { + b.clientTimeout = timeout + return b +} + +// WithEnvReader sets the env.Reader used to read INSECURE_DISABLE_URL_VALIDATION. +// Defaults to the real OS environment; inject a fake reader in tests to avoid +// mutating process-wide state. A nil reader is normalized to &env.OSReader{} +// so callers (including NewHostScopedClientBuilderWithReader) can't panic on +// a nil-interface method call. +func (b *HttpClientBuilder) WithEnvReader(reader env.Reader) *HttpClientBuilder { + if reader == nil { + reader = &env.OSReader{} + } + b.envReader = reader + return b +} + +// Build creates the configured HTTP client. +// +// When private IPs are disallowed, Build installs the per-dial SSRF guard +// (NewPrivateIPBlockingDialContext's underlying check) AND disables HTTP +// keep-alives in the same branch. The guard validates the address on each new +// dial; a pooled, kept-alive connection skips that dial entirely on +// subsequent requests, silently bypassing the check. Pairing the two in one +// branch makes "guard installed with keep-alives left on" structurally +// unreachable rather than merely discouraged in documentation. When private +// IPs are allowed, no guard is installed, so keep-alives may remain enabled. +// +// When WithTokenFromFile configures an auth token, the returned client +// automatically installs SameHostRedirectPolicy as CheckRedirect. Without +// this, oauth2.Transport re-adds the Authorization: Bearer header on every +// redirected request, and http.Client follows cross-host redirects by +// default — so a malicious or compromised server could redirect the request +// to an attacker-controlled host and walk off with the bearer token. There is +// no legitimate reason for a bearer-token-carrying client to need to follow a +// cross-host redirect. Callers authenticating via other mechanisms (e.g. a +// custom RoundTripper that adds its own headers) should layer +// SameHostRedirectPolicy themselves if they need the same protection; Build +// otherwise stays redirect-policy-neutral by default. +func (b *HttpClientBuilder) Build() (*http.Client, error) { + transport := &http.Transport{ + TLSHandshakeTimeout: b.tlsHandshakeTimeout, + ResponseHeaderTimeout: b.responseHeaderTimeout, + } + + if !b.allowPrivate { + transport.DialContext = (&net.Dialer{ + Control: protectedDialerControl, + }).DialContext + transport.DisableKeepAlives = true + } + + if b.caCertPath != "" { + caCert, err := os.ReadFile(b.caCertPath) + if err != nil { + return nil, fmt.Errorf("failed to read CA certificate bundle: %w", err) + } + + caCertPool := x509.NewCertPool() + if !caCertPool.AppendCertsFromPEM(caCert) { + return nil, fmt.Errorf("failed to parse CA certificate bundle") + } + + if transport.TLSClientConfig == nil { + transport.TLSClientConfig = &tls.Config{ + MinVersion: tls.VersionTLS12, + } + } + transport.TLSClientConfig.RootCAs = caCertPool + } + + // Start with validation transport + var clientTransport http.RoundTripper = &ValidatingTransport{ + Transport: transport, + InsecureAllowHTTP: b.insecureAllowHTTP, + EnvReader: b.envReader, + } + + // Add auth transport if token file is provided using oauth2.Transport + if b.authTokenFile != "" { + tokenSource, err := createTokenSourceFromFile(b.authTokenFile) + if err != nil { + return nil, fmt.Errorf("failed to create token source: %w", err) + } + + // oauth2.Transport wraps our existing transport and adds Bearer token authentication + clientTransport = &oauth2.Transport{ + Source: tokenSource, + Base: clientTransport, // Preserves our ValidatingTransport + } + } + + client := &http.Client{ + Transport: clientTransport, + Timeout: b.clientTimeout, + } + + // A bearer token must never be replayed to a redirect target on a + // different host; see the Build doc comment above. + if b.authTokenFile != "" { + client.CheckRedirect = SameHostRedirectPolicy() + } + + return client, nil +} diff --git a/networking/http_client_test.go b/networking/http_client_test.go new file mode 100644 index 0000000..ffd14ff --- /dev/null +++ b/networking/http_client_test.go @@ -0,0 +1,996 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking + +import ( + "crypto/tls" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + "golang.org/x/oauth2" + + "github.com/stacklok/toolhive-core/env" + "github.com/stacklok/toolhive-core/env/mocks" +) + +func TestNewHttpClientBuilder(t *testing.T) { + t.Parallel() + + builder := NewHttpClientBuilder() + + assert.Equal(t, HttpTimeout, builder.clientTimeout) + assert.Equal(t, 10*time.Second, builder.tlsHandshakeTimeout) + assert.Equal(t, 10*time.Second, builder.responseHeaderTimeout) + assert.Empty(t, builder.caCertPath) + assert.Empty(t, builder.authTokenFile) + assert.False(t, builder.allowPrivate) + assert.NotNil(t, builder.envReader) +} + +func TestNewHostScopedClientBuilder(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + host string + allowPrivateIPs bool + insecureAllowHTTP bool + wantAllowPrivate bool + wantInsecureHTTP bool + }{ + { + name: "external host, guarded by default", + host: testIdpExampleHost, + }, + { + name: "external host, allowPrivateIPs widens only the private-IP gate", + host: testIdpExampleHost, + allowPrivateIPs: true, + wantAllowPrivate: true, + wantInsecureHTTP: false, + }, + { + name: "external host, insecureAllowHTTP widens both gates", + host: testIdpExampleHost, + insecureAllowHTTP: true, + wantAllowPrivate: true, + wantInsecureHTTP: true, + }, + { + name: "loopback host is exempted from both gates regardless of flags", + host: "127.0.0.1:8443", + wantAllowPrivate: true, + wantInsecureHTTP: true, + }, + { + name: "loopback host stays exempted even with allowPrivateIPs also set", + host: hostLocalhost, + allowPrivateIPs: true, + wantAllowPrivate: true, + wantInsecureHTTP: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + builder := NewHostScopedClientBuilder(tt.host, tt.allowPrivateIPs, tt.insecureAllowHTTP) + + assert.Equal(t, tt.wantAllowPrivate, builder.allowPrivate, "allowPrivate mismatch") + assert.Equal(t, tt.wantInsecureHTTP, builder.insecureAllowHTTP, "insecureAllowHTTP mismatch") + }) + } +} + +// TestNewHostScopedClientBuilder_InsecureDisableURLValidationEnvVar pins that +// the env var widens both gates the same way a loopback host does. +// NewHostScopedClientBuilder can't accept an injected reader without breaking +// its exported signature, so this test still has to reach for the real +// process environment; kept as a standalone test (not a table case) because +// t.Setenv is incompatible with t.Parallel. See +// TestNewHostScopedClientBuilderWithReader for the dependency-injected +// equivalent that avoids mutating process-wide state. +func TestNewHostScopedClientBuilder_InsecureDisableURLValidationEnvVar(t *testing.T) { + t.Setenv("INSECURE_DISABLE_URL_VALIDATION", "true") + + builder := NewHostScopedClientBuilder(testIdpExampleHost, false, false) + + assert.True(t, builder.allowPrivate, "env var must widen the private-IP gate") + assert.True(t, builder.insecureAllowHTTP, "env var must widen the HTTP scheme gate") +} + +// TestNewHostScopedClientBuilderWithReader pins that +// NewHostScopedClientBuilderWithReader consults the injected env.Reader (not +// the real OS environment) when computing allowInsecure, and that the same +// reader is left installed on the returned builder for subsequent use. +func TestNewHostScopedClientBuilderWithReader(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockReader := mocks.NewMockReader(ctrl) + mockReader.EXPECT().Getenv("INSECURE_DISABLE_URL_VALIDATION").Return("true") + + builder := NewHostScopedClientBuilderWithReader(testIdpExampleHost, false, false, mockReader) + + assert.True(t, builder.allowPrivate, "injected reader must widen the private-IP gate") + assert.True(t, builder.insecureAllowHTTP, "injected reader must widen the HTTP scheme gate") + assert.Same(t, mockReader, builder.envReader, "the injected reader must remain installed on the builder") +} + +// TestNewHostScopedClientBuilderWithReader_NilReader is a regression test: +// WithEnvReader used to store a nil reader as-is, so +// NewHostScopedClientBuilderWithReader(..., nil) panicked on the nil-interface +// Getenv call. WithEnvReader now normalizes nil to &env.OSReader{}, matching +// the fallback ValidatingTransport.RoundTrip already does. +func TestNewHostScopedClientBuilderWithReader_NilReader(t *testing.T) { + t.Parallel() + + require.NotPanics(t, func() { + builder := NewHostScopedClientBuilderWithReader(testIdpExampleHost, false, false, nil) + assert.NotNil(t, builder.envReader, "nil reader must be normalized rather than stored as-is") + assert.IsType(t, &env.OSReader{}, builder.envReader) + }) +} + +func TestHttpClientBuilder_WithCABundle(t *testing.T) { + t.Parallel() + + builder := NewHttpClientBuilder() + path := "/path/to/ca.crt" + + result := builder.WithCABundle(path) + + assert.Same(t, builder, result) // fluent interface + assert.Equal(t, path, builder.caCertPath) +} + +func TestHttpClientBuilder_WithTokenFromFile(t *testing.T) { + t.Parallel() + + builder := NewHttpClientBuilder() + path := "/path/to/token" + + result := builder.WithTokenFromFile(path) + + assert.Same(t, builder, result) // fluent interface + assert.Equal(t, path, builder.authTokenFile) +} + +func TestHttpClientBuilder_WithPrivateIPs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + allow bool + }{ + { + name: "allow private IPs", + allow: true, + }, + { + name: "disallow private IPs", + allow: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + builder := NewHttpClientBuilder() + result := builder.WithPrivateIPs(tt.allow) + + assert.Same(t, builder, result) // fluent interface + assert.Equal(t, tt.allow, builder.allowPrivate) + }) + } +} + +func TestHttpClientBuilder_WithEnvReader(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockReader := mocks.NewMockReader(ctrl) + + builder := NewHttpClientBuilder() + result := builder.WithEnvReader(mockReader) + + assert.Same(t, builder, result) // fluent interface + assert.Same(t, mockReader, builder.envReader) +} + +func TestHttpClientBuilder_Build(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + setupBuilder func() *HttpClientBuilder + setupFiles func(t *testing.T) (string, string) // returns caCertPath, tokenPath + expectError bool + errorContains string + validateClient func(t *testing.T, client *http.Client) + }{ + { + name: "basic client without options", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder() + }, + setupFiles: func(_ *testing.T) (string, string) { + return "", "" + }, + expectError: false, + validateClient: func(t *testing.T, client *http.Client) { + t.Helper() + assert.Equal(t, HttpTimeout, client.Timeout) + assert.IsType(t, &ValidatingTransport{}, client.Transport) + }, + }, + { + name: "client with valid CA bundle", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder() + }, + setupFiles: func(t *testing.T) (string, string) { + t.Helper() + // Create a valid CA certificate for testing + caCert := `-----BEGIN CERTIFICATE----- +MIIDeTCCAmGgAwIBAgIUN4MtKQdT5lEx53a3ZnUoSuAQ5fswDQYJKoZIhvcNAQEL +BQAwTDELMAkGA1UEBhMCVVMxDTALBgNVBAgMBFRlc3QxDTALBgNVBAcMBFRlc3Qx +DTALBgNVBAoMBFRlc3QxEDAOBgNVBAMMB1Rlc3QgQ0EwHhcNMjUwNzA3MTMyNzIw +WhcNMjYwNzA3MTMyNzIwWjBMMQswCQYDVQQGEwJVUzENMAsGA1UECAwEVGVzdDEN +MAsGA1UEBwwEVGVzdDENMAsGA1UECgwEVGVzdDEQMA4GA1UEAwwHVGVzdCBDQTCC +ASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAN/hmz1T3M+HSjarU4qk8oMz +sYX/PI+TMPC5rHSbQ1+Tve2EwbDKUu2d4wT60lHlcVJ3eEw4N6OuRq6DV2mgmbcY +RzJLorgqLG7WsXv660azu0Ln14kK1z+x4cAYzvQ9x54g1PPep7RNPNUEBex0AjG+ +m3BZSk42t76TJg/82KxT2KmmNs6iUwXBptkaGw7CSBKGQOMq00jq0Xcp+ttfZtfx +IGZ9Q5ABc/j1FhPW96NxYbkdTJrhSbsoxWeRx8RSr5r5ZsP4IBw25t3oL8SZKNsR +Ln3Whb9GkupnAfVHxAPOTSwttLa1RqFJJwpBUQErSyD7aoisd5/pMjw0+9wk/IEC +AwEAAaNTMFEwHQYDVR0OBBYEFCl3yBkrEQ9qGGSPanmhwNqyqy7/MB8GA1UdIwQY +MBaAFCl3yBkrEQ9qGGSPanmhwNqyqy7/MA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZI +hvcNAQELBQADggEBAFpv9f+xbCjuvaaNJg1s8UtVzgiJXkMYfvD+EvN2FRHkR++0 +PIpeq1khxoP/INCXFBDz2+4N7nZUi79FH+IkXVAAK9w1Vg8mFOHkiRpCvHxOMU3J +FN0qsmIyA3D8LYQwJZDi6QE9qiNKGTnk7h676rAgk+ez2NS+nJNHUrPKu5zVCU4r +SaYEYg/JrY5DzgHel85LjteLiGE+6HVf8kKXAxSmxdxTDH73jdpEBtxVYxhnnxpF +d3JSN0mL1/vDlI27PofXsisvLH29wRo4Cev+naGLtdB5D8tZ6F6WBYaa9ZK86JSJ +lT/G27CBRUlDiDhthwY1dccTCFhICg6ENUGqh2I= +-----END CERTIFICATE-----` + tmpFile := filepath.Join(t.TempDir(), "ca.crt") + require.NoError(t, os.WriteFile(tmpFile, []byte(caCert), 0644)) + return tmpFile, "" + }, + expectError: false, + validateClient: func(t *testing.T, client *http.Client) { + t.Helper() + transport := client.Transport.(*ValidatingTransport) + httpTransport := transport.Transport.(*http.Transport) + assert.NotNil(t, httpTransport.TLSClientConfig) + assert.NotNil(t, httpTransport.TLSClientConfig.RootCAs) + assert.Equal(t, uint16(tls.VersionTLS12), httpTransport.TLSClientConfig.MinVersion) + }, + }, + { + name: "client with valid token file", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder() + }, + setupFiles: func(t *testing.T) (string, string) { + t.Helper() + tokenFile := filepath.Join(t.TempDir(), "token") + require.NoError(t, os.WriteFile(tokenFile, []byte("test-token-123"), 0644)) + return "", tokenFile + }, + expectError: false, + validateClient: func(t *testing.T, client *http.Client) { + t.Helper() + assert.IsType(t, &oauth2.Transport{}, client.Transport) + }, + }, + { + name: "client with CA bundle and token", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder() + }, + setupFiles: func(t *testing.T) (string, string) { + t.Helper() + caCert := `-----BEGIN CERTIFICATE----- +MIIDeTCCAmGgAwIBAgIUN4MtKQdT5lEx53a3ZnUoSuAQ5fswDQYJKoZIhvcNAQEL +BQAwTDELMAkGA1UEBhMCVVMxDTALBgNVBAgMBFRlc3QxDTALBgNVBAcMBFRlc3Qx +DTALBgNVBAoMBFRlc3QxEDAOBgNVBAMMB1Rlc3QgQ0EwHhcNMjUwNzA3MTMyNzIw +WhcNMjYwNzA3MTMyNzIwWjBMMQswCQYDVQQGEwJVUzENMAsGA1UECAwEVGVzdDEN +MAsGA1UEBwwEVGVzdDENMAsGA1UECgwEVGVzdDEQMA4GA1UEAwwHVGVzdCBDQTCC +ASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAN/hmz1T3M+HSjarU4qk8oMz +sYX/PI+TMPC5rHSbQ1+Tve2EwbDKUu2d4wT60lHlcVJ3eEw4N6OuRq6DV2mgmbcY +RzJLorgqLG7WsXv660azu0Ln14kK1z+x4cAYzvQ9x54g1PPep7RNPNUEBex0AjG+ +m3BZSk42t76TJg/82KxT2KmmNs6iUwXBptkaGw7CSBKGQOMq00jq0Xcp+ttfZtfx +IGZ9Q5ABc/j1FhPW96NxYbkdTJrhSbsoxWeRx8RSr5r5ZsP4IBw25t3oL8SZKNsR +Ln3Whb9GkupnAfVHxAPOTSwttLa1RqFJJwpBUQErSyD7aoisd5/pMjw0+9wk/IEC +AwEAAaNTMFEwHQYDVR0OBBYEFCl3yBkrEQ9qGGSPanmhwNqyqy7/MB8GA1UdIwQY +MBaAFCl3yBkrEQ9qGGSPanmhwNqyqy7/MA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZI +hvcNAQELBQADggEBAFpv9f+xbCjuvaaNJg1s8UtVzgiJXkMYfvD+EvN2FRHkR++0 +PIpeq1khxoP/INCXFBDz2+4N7nZUi79FH+IkXVAAK9w1Vg8mFOHkiRpCvHxOMU3J +FN0qsmIyA3D8LYQwJZDi6QE9qiNKGTnk7h676rAgk+ez2NS+nJNHUrPKu5zVCU4r +SaYEYg/JrY5DzgHel85LjteLiGE+6HVf8kKXAxSmxdxTDH73jdpEBtxVYxhnnxpF +d3JSN0mL1/vDlI27PofXsisvLH29wRo4Cev+naGLtdB5D8tZ6F6WBYaa9ZK86JSJ +lT/G27CBRUlDiDhthwY1dccTCFhICg6ENUGqh2I= +-----END CERTIFICATE-----` + caCertFile := filepath.Join(t.TempDir(), "ca.crt") + require.NoError(t, os.WriteFile(caCertFile, []byte(caCert), 0644)) + + tokenFile := filepath.Join(t.TempDir(), "token") + require.NoError(t, os.WriteFile(tokenFile, []byte("test-token-456"), 0644)) + + return caCertFile, tokenFile + }, + expectError: false, + validateClient: func(t *testing.T, client *http.Client) { + t.Helper() + // Should have oauth2 transport wrapping validating transport + authTransport := client.Transport.(*oauth2.Transport) + assert.IsType(t, &ValidatingTransport{}, authTransport.Base) + }, + }, + { + name: "client with private IPs allowed", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder().WithPrivateIPs(true) + }, + setupFiles: func(_ *testing.T) (string, string) { + return "", "" + }, + expectError: false, + validateClient: func(t *testing.T, client *http.Client) { + t.Helper() + transport := client.Transport.(*ValidatingTransport) + httpTransport := transport.Transport.(*http.Transport) + assert.Nil(t, httpTransport.DialContext, "dial guard must be absent when private IPs are allowed") + assert.False(t, httpTransport.DisableKeepAlives, "keep-alives may stay enabled when no dial guard is installed") + }, + }, + { + name: "client with private IPs disallowed", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder().WithPrivateIPs(false) + }, + setupFiles: func(_ *testing.T) (string, string) { + return "", "" + }, + expectError: false, + validateClient: func(t *testing.T, client *http.Client) { + t.Helper() + transport := client.Transport.(*ValidatingTransport) + httpTransport := transport.Transport.(*http.Transport) + assert.NotNil(t, httpTransport.DialContext, "dial guard must be installed when private IPs are disallowed") + assert.True(t, httpTransport.DisableKeepAlives, + "keep-alives must be disabled whenever the dial guard is installed, "+ + "otherwise a pooled connection skips the per-dial check on later requests") + }, + }, + { + name: "invalid CA certificate file", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder() + }, + setupFiles: func(t *testing.T) (string, string) { + t.Helper() + tmpFile := filepath.Join(t.TempDir(), "invalid-ca.crt") + require.NoError(t, os.WriteFile(tmpFile, []byte("invalid cert data"), 0644)) + return tmpFile, "" + }, + expectError: true, + errorContains: "failed to parse CA certificate bundle", + }, + { + name: "missing CA certificate file", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder() + }, + setupFiles: func(_ *testing.T) (string, string) { + return "/nonexistent/ca.crt", "" + }, + expectError: true, + errorContains: "failed to read CA certificate bundle", + }, + { + name: "missing token file", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder() + }, + setupFiles: func(_ *testing.T) (string, string) { + return "", "/nonexistent/token" + }, + expectError: true, + errorContains: "failed to create token source", + }, + { + name: "empty token file", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder() + }, + setupFiles: func(t *testing.T) (string, string) { + t.Helper() + tmpFile := filepath.Join(t.TempDir(), "empty-token") + require.NoError(t, os.WriteFile(tmpFile, []byte(""), 0644)) + return "", tmpFile + }, + expectError: true, + errorContains: testAuthTokenEmptyErrMsg, + }, + { + name: "token file with whitespace only", + setupBuilder: func() *HttpClientBuilder { + return NewHttpClientBuilder() + }, + setupFiles: func(t *testing.T) (string, string) { + t.Helper() + tmpFile := filepath.Join(t.TempDir(), "whitespace-token") + require.NoError(t, os.WriteFile(tmpFile, []byte(" \n\t "), 0644)) + return "", tmpFile + }, + expectError: true, + errorContains: testAuthTokenEmptyErrMsg, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + builder := tt.setupBuilder() + caCertPath, tokenPath := tt.setupFiles(t) + + if caCertPath != "" { + builder.WithCABundle(caCertPath) + } + if tokenPath != "" { + builder.WithTokenFromFile(tokenPath) + } + + client, err := builder.Build() + + if tt.expectError { + assert.Error(t, err) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + assert.Nil(t, client) + } else { + assert.NoError(t, err) + assert.NotNil(t, client) + if tt.validateClient != nil { + tt.validateClient(t, client) + } + } + }) + } +} + +// TestHttpClientBuilder_Build_KeepAlivePairing pins that the per-dial SSRF +// guard and DisableKeepAlives can only appear together: a pooled, kept-alive +// connection would otherwise reuse a previously-validated dial and skip the +// guard on later requests through that connection. +func TestHttpClientBuilder_Build_KeepAlivePairing(t *testing.T) { + t.Parallel() + + t.Run("private IPs disallowed: guard installed, keep-alives disabled", func(t *testing.T) { + t.Parallel() + + client, err := NewHttpClientBuilder().WithPrivateIPs(false).Build() + require.NoError(t, err) + + transport := client.Transport.(*ValidatingTransport).Transport.(*http.Transport) + assert.NotNil(t, transport.DialContext) + assert.True(t, transport.DisableKeepAlives) + }) + + t.Run("private IPs allowed: guard absent, keep-alives left enabled", func(t *testing.T) { + t.Parallel() + + client, err := NewHttpClientBuilder().WithPrivateIPs(true).Build() + require.NoError(t, err) + + transport := client.Transport.(*ValidatingTransport).Transport.(*http.Transport) + assert.Nil(t, transport.DialContext) + assert.False(t, transport.DisableKeepAlives) + }) +} + +func TestValidatingTransport_RoundTrip(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + url string + insecureAllowHTTP bool + expectError bool + errorContains string + }{ + { + name: testNameValidHTTPSURL, + url: "https://example.com/test", + insecureAllowHTTP: false, + expectError: false, + }, + { + name: "HTTP URL (not HTTPS)", + url: "http://example.com/test", + insecureAllowHTTP: false, + expectError: true, + errorContains: "is not HTTPS scheme", + }, + { + name: "malformed URL", + url: testNotAURL, + insecureAllowHTTP: false, + expectError: true, + errorContains: "is not HTTPS scheme", + }, + { + name: "HTTP URL allowed with InsecureAllowHTTP", + url: "http://localhost:8080/test", + insecureAllowHTTP: true, + expectError: false, + }, + { + name: "HTTPS URL still works with InsecureAllowHTTP", + url: "https://example.com/test", + insecureAllowHTTP: true, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Create a mock transport + mockTransport := &mockRoundTripper{ + response: &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader("OK")), + }, + } + + transport := &ValidatingTransport{ + Transport: mockTransport, + InsecureAllowHTTP: tt.insecureAllowHTTP, + } + + req, err := http.NewRequest("GET", tt.url, nil) + require.NoError(t, err) + + resp, err := transport.RoundTrip(req) + + if tt.expectError { + assert.Error(t, err) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + assert.Nil(t, resp) + } else { + assert.NoError(t, err) + assert.NotNil(t, resp) + assert.True(t, mockTransport.called) + } + }) + } +} + +// TestValidatingTransport_RoundTrip_EnvReader pins that RoundTrip consults +// the injected env.Reader (rather than the process environment) for +// INSECURE_DISABLE_URL_VALIDATION, and that a nil EnvReader falls back to the +// real OS environment. +func TestValidatingTransport_RoundTrip_EnvReader(t *testing.T) { + t.Parallel() + + t.Run("injected reader reporting true skips validation", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockReader := mocks.NewMockReader(ctrl) + mockReader.EXPECT().Getenv("INSECURE_DISABLE_URL_VALIDATION").Return("true") + + mockTransport := &mockRoundTripper{} + transport := &ValidatingTransport{Transport: mockTransport, EnvReader: mockReader} + + req, err := http.NewRequest("GET", "http://example.com/test", nil) + require.NoError(t, err) + + _, err = transport.RoundTrip(req) + require.NoError(t, err) + assert.True(t, mockTransport.called) + }) + + t.Run("injected reader reporting false still enforces HTTPS", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockReader := mocks.NewMockReader(ctrl) + mockReader.EXPECT().Getenv("INSECURE_DISABLE_URL_VALIDATION").Return("false") + + mockTransport := &mockRoundTripper{} + transport := &ValidatingTransport{Transport: mockTransport, EnvReader: mockReader} + + req, err := http.NewRequest("GET", "http://example.com/test", nil) + require.NoError(t, err) + + _, err = transport.RoundTrip(req) + require.Error(t, err) + assert.Contains(t, err.Error(), "is not HTTPS scheme") + assert.False(t, mockTransport.called) + }) + +} + +// TestValidatingTransport_RoundTrip_NilEnvReaderUsesOS pins that a nil +// EnvReader falls back to the real OS environment. Kept as its own +// non-parallel top-level test because t.Setenv panics if any ancestor test +// has called t.Parallel. +func TestValidatingTransport_RoundTrip_NilEnvReaderUsesOS(t *testing.T) { + t.Setenv("INSECURE_DISABLE_URL_VALIDATION", "true") + + mockTransport := &mockRoundTripper{} + transport := &ValidatingTransport{Transport: mockTransport} + + req, err := http.NewRequest("GET", "http://example.com/test", nil) + require.NoError(t, err) + + _, err = transport.RoundTrip(req) + require.NoError(t, err) + assert.True(t, mockTransport.called) +} + +func TestOAuth2Transport_RoundTrip(t *testing.T) { + t.Parallel() + + // Create a test server to capture the Authorization header + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + auth := r.Header.Get("Authorization") + w.Header().Set("X-Auth-Header", auth) + w.WriteHeader(200) + w.Write([]byte("OK")) + })) + defer server.Close() + + // Create temp token file + tokenFile := filepath.Join(t.TempDir(), "token") + testToken := "test-bearer-token-123" + require.NoError(t, os.WriteFile(tokenFile, []byte(testToken), 0644)) + + // Create token source and oauth2 transport + tokenSource, err := createTokenSourceFromFile(tokenFile) + require.NoError(t, err) + + authTransport := &oauth2.Transport{ + Source: tokenSource, + Base: server.Client().Transport, + } + + // Make request + req, err := http.NewRequest("GET", server.URL, nil) + require.NoError(t, err) + + resp, err := authTransport.RoundTrip(req) + require.NoError(t, err) + defer resp.Body.Close() + + // Verify Authorization header was added + expectedAuth := "Bearer " + testToken + actualAuth := resp.Header.Get("X-Auth-Header") + assert.Equal(t, expectedAuth, actualAuth) + + // Verify original request was not modified + assert.Empty(t, req.Header.Get("Authorization")) +} + +func TestCreateTokenSourceFromFile(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + tokenContent string + expectError bool + errorContains string + expectedToken string + }{ + { + name: "valid token", + tokenContent: "valid-token-123", + expectError: false, + expectedToken: "valid-token-123", + }, + { + name: "token with trailing newline", + tokenContent: "token-with-newline\n", + expectError: false, + expectedToken: "token-with-newline", + }, + { + name: "token with whitespace", + tokenContent: " token-with-spaces \n\t", + expectError: false, + expectedToken: "token-with-spaces", + }, + { + name: "empty token", + tokenContent: "", + expectError: true, + errorContains: testAuthTokenEmptyErrMsg, + }, + { + name: "whitespace only token", + tokenContent: " \n\t ", + expectError: true, + errorContains: testAuthTokenEmptyErrMsg, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Create temp token file + tokenFile := filepath.Join(t.TempDir(), "token") + require.NoError(t, os.WriteFile(tokenFile, []byte(tt.tokenContent), 0644)) + + tokenSource, err := createTokenSourceFromFile(tokenFile) + + if tt.expectError { + assert.Error(t, err) + if tt.errorContains != "" { + assert.Contains(t, err.Error(), tt.errorContains) + } + assert.Nil(t, tokenSource) + } else { + assert.NoError(t, err) + assert.NotNil(t, tokenSource) + + // Get token from source and verify + token, err := tokenSource.Token() + require.NoError(t, err) + assert.Equal(t, tt.expectedToken, token.AccessToken) + assert.Equal(t, "Bearer", token.TokenType) + } + }) + } + + t.Run("missing token file", func(t *testing.T) { + t.Parallel() + + tokenSource, err := createTokenSourceFromFile("/nonexistent/token") + + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to read auth token file") + assert.Nil(t, tokenSource) + }) +} + +// mockRoundTripper is a simple mock implementation of http.RoundTripper for testing +type mockRoundTripper struct { + response *http.Response + err error + called bool +} + +func (m *mockRoundTripper) RoundTrip(_ *http.Request) (*http.Response, error) { + m.called = true + if m.err != nil { + return nil, m.err + } + if m.response != nil { + return m.response, nil + } + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(strings.NewReader("OK")), + }, nil +} + +func mustReq(t *testing.T, rawURL string) *http.Request { + t.Helper() + u, err := url.Parse(rawURL) + require.NoError(t, err) + return &http.Request{URL: u, Host: u.Host} +} + +func TestSameHostRedirectPolicy(t *testing.T) { + t.Parallel() + + policy := SameHostRedirectPolicy() + + tests := []struct { + name string + req string + via []string + wantRefus bool + }{ + { + name: "same host same scheme is allowed", + req: "https://mcp.example.com/next", + via: []string{testMCPStartURL}, + }, + { + name: "same host different port is refused (distinct service)", + req: "https://mcp.example.com:8443/next", + via: []string{testMCPStartURL}, + wantRefus: true, + }, + { + name: "cross-host redirect to internal metadata endpoint is refused", + req: "http://169.254.169.254/latest/meta-data/", + via: []string{"https://mcp.example.com/.well-known/oauth-protected-resource"}, + wantRefus: true, + }, + { + name: "cross-host redirect to another public host is refused", + req: "https://evil.example.net/x", + via: []string{testMCPStartURL}, + wantRefus: true, + }, + { + name: "https to http downgrade on same host is refused", + req: "http://mcp.example.com/next", + via: []string{testMCPStartURL}, + wantRefus: true, + }, + { + name: "http to http on same host is allowed (no downgrade)", + req: "http://localhost:9000/next", + via: []string{"http://localhost:9000/start"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + via := make([]*http.Request, 0, len(tt.via)) + for _, v := range tt.via { + via = append(via, mustReq(t, v)) + } + err := policy(mustReq(t, tt.req), via) + if tt.wantRefus { + require.Error(t, err) + assert.ErrorIs(t, err, ErrRedirectRefused) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestSameHostRedirectPolicy_CapsChain(t *testing.T) { + t.Parallel() + + policy := SameHostRedirectPolicy() + via := make([]*http.Request, MaxRedirects) + for i := range via { + via[i] = mustReq(t, testMCPStartURL) + } + err := policy(mustReq(t, "https://mcp.example.com/next"), via) + require.Error(t, err) + assert.ErrorIs(t, err, ErrRedirectRefused) +} + +func TestSameHostRedirectPolicy_Integration(t *testing.T) { + t.Parallel() + + // internal stands in for an internal-only endpoint a malicious server + // would try to reach via a cross-host redirect. + internal := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("SECRET")) + })) + t.Cleanup(internal.Close) + + // attacker redirects any request to the internal endpoint (cross-host). + attacker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/same" { + // same-host relative redirect should be followed + http.Redirect(w, r, "/ok", http.StatusFound) + return + } + if r.URL.Path == "/ok" { + _, _ = w.Write([]byte("OK")) + return + } + http.Redirect(w, r, internal.URL, http.StatusFound) + })) + t.Cleanup(attacker.Close) + + client := &http.Client{CheckRedirect: SameHostRedirectPolicy()} + + t.Run("cross-host redirect is refused", func(t *testing.T) { + t.Parallel() + resp, err := client.Get(attacker.URL + "/evil") //nolint:bodyclose // err path, no body + require.Error(t, err) + assert.ErrorIs(t, err, ErrRedirectRefused) + if resp != nil { + _ = resp.Body.Close() + } + }) + + t.Run("same-host redirect is followed", func(t *testing.T) { + t.Parallel() + resp, err := client.Get(attacker.URL + "/same") + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + assert.Equal(t, "OK", string(body)) + }) +} + +// TestHttpClientBuilder_Build_TokenFileSetsRedirectPolicy pins that a client +// built with WithTokenFromFile refuses to replay its bearer token to a +// redirect target on a different host: oauth2.Transport re-adds the +// Authorization header on every round-trip it makes, including redirected +// requests, so without a same-host CheckRedirect policy a malicious or +// compromised server could redirect the request to an attacker-controlled +// host and capture the token. +func TestHttpClientBuilder_Build_TokenFileSetsRedirectPolicy(t *testing.T) { + t.Parallel() + + tmpFile := filepath.Join(t.TempDir(), "token") + require.NoError(t, os.WriteFile(tmpFile, []byte("secret-token"), 0600)) + + t.Run("CheckRedirect is installed when a token file is configured", func(t *testing.T) { + t.Parallel() + + client, err := NewHttpClientBuilder().WithTokenFromFile(tmpFile).Build() + require.NoError(t, err) + assert.NotNil(t, client.CheckRedirect) + }) + + t.Run("CheckRedirect is left unset without a token file", func(t *testing.T) { + t.Parallel() + + client, err := NewHttpClientBuilder().Build() + require.NoError(t, err) + assert.Nil(t, client.CheckRedirect) + }) + + t.Run("cross-host redirect refuses to replay the bearer token", func(t *testing.T) { + t.Parallel() + + var gotAuthHeader string + var redirectedRequestSeen bool + attackerHost := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + redirectedRequestSeen = true + gotAuthHeader = r.Header.Get("Authorization") + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(attackerHost.Close) + + originHost := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, attackerHost.URL+"/steal", http.StatusFound) + })) + t.Cleanup(originHost.Close) + + client, err := NewHttpClientBuilder(). + WithTokenFromFile(tmpFile). + WithPrivateIPs(true). + WithInsecureAllowHTTP(true). + Build() + require.NoError(t, err) + + resp, err := client.Get(originHost.URL) //nolint:bodyclose // err path, no body + require.Error(t, err) + assert.ErrorIs(t, err, ErrRedirectRefused) + if resp != nil { + _ = resp.Body.Close() + } + assert.False(t, redirectedRequestSeen, "the redirect target must never receive the request") + assert.Empty(t, gotAuthHeader) + }) +} diff --git a/networking/http_error.go b/networking/http_error.go new file mode 100644 index 0000000..604cebf --- /dev/null +++ b/networking/http_error.go @@ -0,0 +1,48 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking + +import ( + "errors" + "fmt" +) + +// HTTPError represents an HTTP error response with status code, URL, and message. +type HTTPError struct { + // StatusCode is the HTTP status code. + StatusCode int + + // Message is a description of the error (may be a preview of the response body). + Message string + + // URL is the requested URL. + URL string +} + +// Error implements the error interface. +func (e *HTTPError) Error() string { + return fmt.Sprintf("HTTP %d for URL %s: %s", e.StatusCode, e.URL, e.Message) +} + +// NewHTTPError creates a new HTTP error. +func NewHTTPError(statusCode int, url, message string) error { + return &HTTPError{ + StatusCode: statusCode, + URL: url, + Message: message, + } +} + +// IsHTTPError checks if an error is an HTTPError with the specified status code. +// If statusCode is 0, it matches any HTTPError. +func IsHTTPError(err error, statusCode int) bool { + var httpErr *HTTPError + if !errors.As(err, &httpErr) { + return false + } + if statusCode == 0 { + return true + } + return httpErr.StatusCode == statusCode +} diff --git a/networking/http_error_test.go b/networking/http_error_test.go new file mode 100644 index 0000000..ca371cb --- /dev/null +++ b/networking/http_error_test.go @@ -0,0 +1,94 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewHTTPError(t *testing.T) { + t.Parallel() + + err := NewHTTPError(404, "http://example.com/api", "not found") + + require.Error(t, err) + var httpErr *HTTPError + require.True(t, errors.As(err, &httpErr)) + assert.Equal(t, 404, httpErr.StatusCode) + assert.Equal(t, "http://example.com/api", httpErr.URL) + assert.Equal(t, "not found", httpErr.Message) +} + +func TestHTTPError_Error(t *testing.T) { + t.Parallel() + + err := &HTTPError{ + StatusCode: 404, + Message: "not found", + URL: "http://example.com/api", + } + + assert.Equal(t, "HTTP 404 for URL http://example.com/api: not found", err.Error()) +} + +func TestIsHTTPError(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + err error + statusCode int + expected bool + }{ + { + name: "matching HTTPError", + err: &HTTPError{StatusCode: 404, URL: testHTTPExampleCom}, + statusCode: 404, + expected: true, + }, + { + name: "non-matching status code", + err: &HTTPError{StatusCode: 404, URL: testHTTPExampleCom}, + statusCode: 500, + expected: false, + }, + { + name: "any HTTPError with statusCode 0", + err: &HTTPError{StatusCode: 403, URL: testHTTPExampleCom}, + statusCode: 0, + expected: true, + }, + { + name: "non-HTTPError", + err: errors.New("some other error"), + statusCode: 404, + expected: false, + }, + { + name: "wrapped HTTPError", + err: fmt.Errorf("wrapped: %w", &HTTPError{StatusCode: 500, URL: testHTTPExampleCom}), + statusCode: 500, + expected: true, + }, + { + name: "nil error", + err: nil, + statusCode: 404, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result := IsHTTPError(tt.err, tt.statusCode) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/networking/port.go b/networking/port.go new file mode 100644 index 0000000..5f24a93 --- /dev/null +++ b/networking/port.go @@ -0,0 +1,309 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking + +import ( + "crypto/rand" + "fmt" + "log/slog" + "math/big" + "net" + "strconv" + "strings" + + gopsutilnet "github.com/shirou/gopsutil/v4/net" +) + +const ( + // MinPort is the minimum port number to use + MinPort = 10000 + // MaxPort is the maximum port number to use + MaxPort = 65535 + // MaxAttempts is the maximum number of attempts to find an available port + MaxAttempts = 10 +) + +// tryListenTCP attempts to bind a TCP listener on 127.0.0.1:port, returning the +// open listener and true on success, or nil and false if the port is unavailable. +func tryListenTCP(port int) (*net.TCPListener, bool) { + tcpAddr, err := net.ResolveTCPAddr("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if err != nil { + return nil, false + } + tcpListener, err := net.ListenTCP("tcp", tcpAddr) + if err != nil { + return nil, false + } + return tcpListener, true +} + +// IsAvailable checks if a port is available. +// +// Known limitation: this binds a probe listener, closes it, then returns a bool — +// there is a time-of-check-to-time-of-use window between the close and whenever the +// caller actually binds the port, during which another process can grab it. Callers +// that intend to bind the port themselves moments later in the same process should +// prefer FindAvailableListener/FindOrUseListener instead, which keep the listener +// open until the caller is ready and so close that window entirely. +func IsAvailable(port int) bool { + // Check TCP + tcpListener, ok := tryListenTCP(port) + if !ok { + return false + } + if err := tcpListener.Close(); err != nil { + // Log the error but continue, as we're just checking if the port is available + slog.Warn("Failed to close TCP listener", "error", err) + } + + // Check UDP + udpAddr, err := net.ResolveUDPAddr("udp", fmt.Sprintf("127.0.0.1:%d", port)) + if err != nil { + return false + } + + udpConn, err := net.ListenUDP("udp", udpAddr) + if err != nil { + return false + } + if err := udpConn.Close(); err != nil { + // Log the error but continue, as we're just checking if the port is available + slog.Warn("Failed to close UDP connection", "error", err) + } + + return true +} + +// FindAvailable finds an available port. +// +// Known limitation: like IsAvailable, this has a bind-then-close-then-return race — +// the port can be taken by another process between this call returning and the +// caller binding it. Prefer FindAvailableListener when you will bind the port +// yourself shortly afterward in the same process. +func FindAvailable() int { + for i := 0; i < MaxAttempts; i++ { + // Generate a cryptographically secure random number + n, err := rand.Int(rand.Reader, big.NewInt(int64(MaxPort-MinPort))) + if err != nil { + // Fall back to sequential search if random generation fails + break + } + port := int(n.Int64()) + MinPort + if IsAvailable(port) { + return port + } + } + + // If we can't find a random port, try sequential ports + for port := MinPort; port <= MaxPort; port++ { + if IsAvailable(port) { + return port + } + } + + // If we still can't find a port, return 0 + return 0 +} + +// FindOrUsePort checks if the provided port is available or finds an available port if none is provided. +// If port is 0, it will find an available port. +// If port is not 0, it will check if the port is available. +// Returns the selected port and an error if any. +// +// Known limitation: this has the same bind-then-close-then-return race as IsAvailable +// and FindAvailable. Prefer FindOrUseListener when you will bind the port yourself +// shortly afterward in the same process. +func FindOrUsePort(port int) (int, error) { + if port != 0 && (port < 1 || port > 65535) { + return 0, fmt.Errorf("invalid port %d: must be 0 (auto-select) or in range 1-65535", port) + } + + if port == 0 { + // Find an available port + port = FindAvailable() + if port == 0 { + return 0, fmt.Errorf("could not find an available port") + } + return port, nil + } + + if IsAvailable(port) { + return port, nil + } + + // Requested port is busy — find an alternative + alt := FindAvailable() + if alt == 0 { + return 0, fmt.Errorf("failed to find an alternative port after requested port %d was unavailable", port) + } + return alt, nil +} + +// FindAvailableListener finds an available port and returns it as a still-open +// *net.TCPListener, closing the find-then-bind race that FindAvailable has: +// since the listener stays open, nothing else can grab the port before the +// caller is ready to use it. The caller is responsible for closing the +// listener (or handing it to something like http.Serve). +func FindAvailableListener() (*net.TCPListener, error) { + for i := 0; i < MaxAttempts; i++ { + n, err := rand.Int(rand.Reader, big.NewInt(int64(MaxPort-MinPort))) + if err != nil { + // Fall back to sequential search if random generation fails + break + } + port := int(n.Int64()) + MinPort + if l, ok := tryListenTCP(port); ok { + return l, nil + } + } + + for port := MinPort; port <= MaxPort; port++ { + if l, ok := tryListenTCP(port); ok { + return l, nil + } + } + + return nil, fmt.Errorf("could not find an available port") +} + +// FindOrUseListener is the race-free counterpart to FindOrUsePort: if port is 0, +// it behaves like FindAvailableListener; otherwise it tries to bind the requested +// port directly, falling back to FindAvailableListener only if that port is +// unavailable. The returned listener is still open; the caller is responsible +// for closing it. +func FindOrUseListener(port int) (*net.TCPListener, error) { + if port != 0 && (port < 1 || port > 65535) { + return nil, fmt.Errorf("invalid port %d: must be 0 (auto-select) or in range 1-65535", port) + } + + if port == 0 { + return FindAvailableListener() + } + + if l, ok := tryListenTCP(port); ok { + return l, nil + } + + l, err := FindAvailableListener() + if err != nil { + return nil, fmt.Errorf("failed to find an alternative port after requested port %d was unavailable: %w", port, err) + } + return l, nil +} + +// ValidateCallbackPort validates that the specified callback port is valid and available. +// It checks that the port is within the valid range (1-65535) and, for pre-registered +// clients (with clientID), it returns an error if the port is not available. +func ValidateCallbackPort(callbackPort int, clientID string) error { + // If port is 0, we'll find an available port later, so no need to validate + if callbackPort == 0 { + return nil + } + + // Validate port range + if callbackPort < 1024 || callbackPort > 65535 { + return fmt.Errorf("OAuth callback port must be between 1024 and 65535, got: %d", callbackPort) + } + + // Check if this is a pre-registered client (has client credentials) + // For pre-registered clients, we need strict port checking + isPreRegisteredClient := IsPreRegisteredClient(clientID) + + if isPreRegisteredClient { + // For pre-registered clients, the port must be available + // The user likely configured this port in their IdP/app + if !IsAvailable(callbackPort) { + return fmt.Errorf("OAuth callback port %d is not available - please choose a different port", callbackPort) + } + } + + return nil +} + +// IsPreRegisteredClient determines if the OAuth client is pre-registered (has client ID) +func IsPreRegisteredClient(clientID string) bool { + return clientID != "" +} + +// GetProcessOnPort returns the PID of the process listening on the given TCP port. +// Returns 0 if the port is free or if the holder cannot be determined. +// Uses gopsutil which provides cross-platform support (Linux: /proc, Windows: GetExtendedTcpTable, +// Darwin/FreeBSD: lsof). +func GetProcessOnPort(port int) (int, error) { + if port <= 0 || port > MaxPort { + return 0, fmt.Errorf("invalid port %d", port) + } + + conns, err := gopsutilnet.Connections("tcp") + if err != nil { + return 0, fmt.Errorf("failed to get TCP connections: %w", err) + } + + for _, c := range conns { + if c.Laddr.Port == uint32(port) && c.Status == "LISTEN" && c.Pid > 0 { //nolint:gosec // G115 - port validated in [1, 65535] + return int(c.Pid), nil + } + } + return 0, nil +} + +// ParsePortSpec parses a port specification string in the format "hostPort:containerPort" or just "containerPort". +// Returns the host port string and container port integer. +// If only a container port is provided, a random available host port is selected +// locally via FindAvailable. +// +// Host port 0 (explicit "0:containerPort" form) is passed through unchanged as +// the string "0" and is NOT resolved to a concrete port here — this is +// intentional. Docker's own PortBinding.HostPort treats "0" as "assign a port +// dynamically at container start", which is a distinct mechanism from this +// function's own container-only path (no ":") that calls FindAvailable to pick +// a host port up front. +func ParsePortSpec(portSpec string) (string, int, error) { + slog.Debug("Parsing port spec", "spec", portSpec) + // Check if it's in host:container format + if strings.Contains(portSpec, ":") { + parts := strings.Split(portSpec, ":") + if len(parts) != 2 { + return "", 0, fmt.Errorf("invalid port specification: %s (expected 'hostPort:containerPort')", portSpec) + } + + hostPortStr := parts[0] + containerPortStr := parts[1] + + // Verify host port is a valid integer (or empty string if we supported random host port with :, but here we expect explicit) + hostPort, err := strconv.Atoi(hostPortStr) + if err != nil { + return "", 0, fmt.Errorf("invalid host port in spec '%s': %w", portSpec, err) + } + if hostPort < 0 || hostPort > 65535 { + return "", 0, fmt.Errorf("invalid host port in spec '%s': %d must be in range 0-65535", portSpec, hostPort) + } + + containerPort, err := strconv.Atoi(containerPortStr) + if err != nil { + return "", 0, fmt.Errorf("invalid container port in spec '%s': %w", portSpec, err) + } + if containerPort < 1 || containerPort > 65535 { + return "", 0, fmt.Errorf("invalid container port in spec '%s': %d must be in range 1-65535", portSpec, containerPort) + } + + return hostPortStr, containerPort, nil + } + + // Try parsing as just container port + containerPort, err := strconv.Atoi(portSpec) + if err == nil { + if containerPort < 1 || containerPort > 65535 { + return "", 0, fmt.Errorf("invalid container port in spec '%s': %d must be in range 1-65535", portSpec, containerPort) + } + // Find a random available host port + hostPort := FindAvailable() + if hostPort == 0 { + return "", 0, fmt.Errorf("could not find an available port for container port %d", containerPort) + } + return fmt.Sprintf("%d", hostPort), containerPort, nil + } + + return "", 0, fmt.Errorf("invalid port specification: %s (expected 'hostPort:containerPort' or 'containerPort')", portSpec) +} diff --git a/networking/port_test.go b/networking/port_test.go new file mode 100644 index 0000000..eb28ec4 --- /dev/null +++ b/networking/port_test.go @@ -0,0 +1,420 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking_test + +import ( + "net" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/stacklok/toolhive-core/networking" +) + +func TestValidateCallbackPort(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + port int + clientID string + wantError bool + errorMsg string + }{ + { + name: "valid port with client ID", + port: 8090, + clientID: "test-client", + wantError: false, + }, + { + name: "valid port without client ID", + port: 8090, + clientID: "", + wantError: false, + }, + { + name: "port zero is allowed (dynamic allocation)", + port: 0, + clientID: "test-client", + wantError: false, + }, + { + name: "negative port is not allowed", + port: -1, + clientID: "", + wantError: true, + errorMsg: "OAuth callback port must be between 1024 and 65535, got: -1", + }, + { + name: "port less than 1024", + port: 1000, + clientID: "", + wantError: true, + errorMsg: "OAuth callback port must be between 1024 and 65535, got: 1000", + }, + { + name: "port too large", + port: 123456778, + clientID: "", + wantError: true, + errorMsg: "OAuth callback port must be between 1024 and 65535, got: 123456778", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := networking.ValidateCallbackPort(tt.port, tt.clientID) + + if tt.wantError { + require.Error(t, err) + if tt.errorMsg != "" { + require.EqualError(t, err, tt.errorMsg) + } + } else { + require.NoError(t, err) + } + }) + } +} + +func TestGetProcessOnPort_InvalidPort(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + port int + }{ + {"zero port", 0}, + {"negative port", -1}, + {"port too large", 65536}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + pid, err := networking.GetProcessOnPort(tt.port) + require.Error(t, err) + assert.Equal(t, 0, pid) + }) + } +} + +func TestGetProcessOnPort_FreePort(t *testing.T) { + t.Parallel() + + // Use a port that FindAvailable guarantees is free + port := networking.FindAvailable() + require.NotZero(t, port, "FindAvailable should find a free port") + + pid, err := networking.GetProcessOnPort(port) + require.NoError(t, err) + assert.Equal(t, 0, pid) +} + +func TestGetProcessOnPort_PortInUse(t *testing.T) { + t.Parallel() + + // Bind to a port, then verify GetProcessOnPort returns our process + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer listener.Close() + + tcpAddr, ok := listener.Addr().(*net.TCPAddr) + require.True(t, ok) + port := tcpAddr.Port + + pid, err := networking.GetProcessOnPort(port) + require.NoError(t, err) + assert.NotZero(t, pid, "port is in use, GetProcessOnPort should return the process PID") +} + +func TestFindAvailableListener_ConcurrentCallsGetDistinctPorts(t *testing.T) { + t.Parallel() + + const numGoroutines = 20 + + var ( + wg sync.WaitGroup + mu sync.Mutex + listeners = make([]*net.TCPListener, 0, numGoroutines) + ports = make(map[int]int) // port -> count + ) + + for i := 0; i < numGoroutines; i++ { + wg.Add(1) + go func() { + defer wg.Done() + + l, err := networking.FindAvailableListener() + if err != nil { + return + } + + addr, ok := l.Addr().(*net.TCPAddr) + require.True(t, ok) + + mu.Lock() + listeners = append(listeners, l) + ports[addr.Port]++ + mu.Unlock() + }() + } + wg.Wait() + + defer func() { + for _, l := range listeners { + _ = l.Close() + } + }() + + require.Len(t, listeners, numGoroutines, "all goroutines should have obtained a listener") + + for port, count := range ports { + assert.Equal(t, 1, count, "port %d was handed out to more than one goroutine", port) + } +} + +func TestFindOrUsePort_InvalidPortRejected(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + port int + wantError bool + }{ + {"negative port is rejected", -1, true}, + {"zero means auto-select", 0, false}, + {"port 1 is a valid boundary", 1, false}, + {"port 65535 is a valid boundary", 65535, false}, + {"port 65536 is rejected", 65536, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, err := networking.FindOrUsePort(tt.port) + if tt.wantError { + require.Error(t, err) + assert.Equal(t, 0, got) + return + } + require.NoError(t, err) + assert.NotZero(t, got) + }) + } +} + +func TestFindOrUseListener_InvalidPortRejected(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + port int + wantError bool + }{ + {"negative port is rejected", -1, true}, + {"zero means auto-select", 0, false}, + {"port 1 is a valid boundary", 1, false}, + {"port 65535 is a valid boundary", 65535, false}, + {"port 65536 is rejected", 65536, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + l, err := networking.FindOrUseListener(tt.port) + if tt.wantError { + require.Error(t, err) + assert.Nil(t, l) + return + } + require.NoError(t, err) + require.NotNil(t, l) + defer l.Close() + }) + } +} + +func TestFindOrUseListener(t *testing.T) { + t.Parallel() + + t.Run("zero port finds an available one", func(t *testing.T) { + t.Parallel() + + l, err := networking.FindOrUseListener(0) + require.NoError(t, err) + defer l.Close() + + addr, ok := l.Addr().(*net.TCPAddr) + require.True(t, ok) + assert.GreaterOrEqual(t, addr.Port, networking.MinPort) + assert.LessOrEqual(t, addr.Port, networking.MaxPort) + }) + + t.Run("specific free port is honored", func(t *testing.T) { + t.Parallel() + + // Find a free port first (and release it) to request specifically. + probe, err := networking.FindAvailableListener() + require.NoError(t, err) + addr, ok := probe.Addr().(*net.TCPAddr) + require.True(t, ok) + wantPort := addr.Port + require.NoError(t, probe.Close()) + + l, err := networking.FindOrUseListener(wantPort) + require.NoError(t, err) + defer l.Close() + + gotAddr, ok := l.Addr().(*net.TCPAddr) + require.True(t, ok) + assert.Equal(t, wantPort, gotAddr.Port) + }) + + t.Run("already-listened-on port falls back to a different one", func(t *testing.T) { + t.Parallel() + + held, err := networking.FindAvailableListener() + require.NoError(t, err) + defer held.Close() + + heldAddr, ok := held.Addr().(*net.TCPAddr) + require.True(t, ok) + + l, err := networking.FindOrUseListener(heldAddr.Port) + require.NoError(t, err) + defer l.Close() + + gotAddr, ok := l.Addr().(*net.TCPAddr) + require.True(t, ok) + assert.NotEqual(t, heldAddr.Port, gotAddr.Port) + }) +} + +func TestParsePortSpec(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + portSpec string + expectedHostPort string + expectedContainer int + wantError bool + }{ + { + name: "host:container", + portSpec: "8003:8001", + expectedHostPort: "8003", + expectedContainer: 8001, + wantError: false, + }, + { + name: "container only", + portSpec: "8001", + expectedHostPort: "", // Random + expectedContainer: 8001, + wantError: false, + }, + { + name: "invalid format", + portSpec: "invalid", + expectedHostPort: "", + expectedContainer: 0, + wantError: true, + }, + { + name: "invalid host port", + portSpec: "abc:8001", + expectedHostPort: "", + expectedContainer: 0, + wantError: true, + }, + { + name: "negative host port is rejected", + portSpec: "-1:0", + expectedHostPort: "", + expectedContainer: 0, + wantError: true, + }, + { + name: "host port above range is rejected", + portSpec: "70000:8001", + expectedHostPort: "", + expectedContainer: 0, + wantError: true, + }, + { + name: "container port above range is rejected", + portSpec: "8000:99999", + expectedHostPort: "", + expectedContainer: 0, + wantError: true, + }, + { + name: "container-only port above range is rejected", + portSpec: "99999", + expectedHostPort: "", + expectedContainer: 0, + wantError: true, + }, + { + name: "container-only port zero is rejected", + portSpec: "0", + expectedHostPort: "", + expectedContainer: 0, + wantError: true, + }, + { + name: "host port 0 is passed through as Docker dynamic-port marker", + portSpec: "0:8080", + expectedHostPort: "0", + expectedContainer: 8080, + wantError: false, + }, + { + name: "host and container port 1 is a valid boundary", + portSpec: "1:1", + expectedHostPort: "1", + expectedContainer: 1, + wantError: false, + }, + { + name: "host and container port 65535 is a valid boundary", + portSpec: "65535:65535", + expectedHostPort: "65535", + expectedContainer: 65535, + wantError: false, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + hostPort, containerPort, err := networking.ParsePortSpec(tt.portSpec) + + if tt.wantError { + require.Error(t, err, "ParsePortSpec(%s) expected error", tt.portSpec) + return + } + + require.NoError(t, err, "ParsePortSpec(%s) unexpected error", tt.portSpec) + + if tt.expectedHostPort != "" { + require.Equal(t, tt.expectedHostPort, hostPort, "ParsePortSpec(%s) unexpected host port", tt.portSpec) + } else { + require.NotEmpty(t, hostPort, "ParsePortSpec(%s) hostPort is empty, want random port", tt.portSpec) + } + + require.Equal(t, tt.expectedContainer, containerPort, "ParsePortSpec(%s) unexpected container port", tt.portSpec) + }) + } +} diff --git a/networking/utilities.go b/networking/utilities.go new file mode 100644 index 0000000..7f72e3b --- /dev/null +++ b/networking/utilities.go @@ -0,0 +1,329 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking + +import ( + "context" + "errors" + "fmt" + "net" + "net/url" + "strings" + + "github.com/stacklok/toolhive-core/env" +) + +// TargetIsPrivate reports whether the host in rawURL refers to — or resolves to — +// a private, loopback, or link-local address. It is used to detect when an +// operator has deliberately pointed ToolHive at an internal target, so that +// discovery fetches derived from untrusted server input may also be allowed to +// reach internal addresses for that deployment. +// +// IP literals and "localhost" are classified without DNS. Hostnames are resolved +// and reported private if ANY resolved address is private. Unparsable input or +// resolution failure returns false (treat as public — the SSRF guard then stays +// engaged, failing secure). +func TargetIsPrivate(ctx context.Context, rawURL string) bool { + u, err := url.Parse(rawURL) + if err != nil { + return false + } + host := u.Hostname() + if host == "" { + return false + } + if IsLocalhost(host) { + return true + } + if ip := net.ParseIP(host); ip != nil { + return IsPrivateIP(ip) + } + addrs, err := net.DefaultResolver.LookupIPAddr(ctx, host) + if err != nil { + return false + } + for _, a := range addrs { + if IsPrivateIP(a.IP) { + return true + } + } + return false +} + +// ErrPrivateIPAddress is returned when the provided URL redirects to a +// private IP address, which is not allowed. +var ErrPrivateIPAddress = errors.New("the provided URL redirects to a private IP address, which is not allowed") + +// nat64Prefixes are the NAT64 translation prefixes whose embedded IPv4 address +// lives in the low 32 bits (RFC 6052 §2.2 "/96" embedding). An address inside +// one of these is routed to that IPv4 by a NAT64 gateway, so its true +// reachability is determined by the embedded IPv4, not by the (global-unicast) +// IPv6 form. +// - 64:ff9b::/96 well-known prefix (RFC 6052), always a /96 +// - 64:ff9b:1::/96 the /96 sub-prefix of the RFC 8215 local-use 64:ff9b:1::/48 +// +// The rest of the local-use 64:ff9b:1::/48 uses a shorter NAT64 prefix, where +// the embedded IPv4 is NOT in the low 32 bits and cannot be located from the +// address alone. Decoding the low 32 bits there could read attacker-chosen +// suffix bytes and misclassify an internal target as public, so that remainder +// is blocked wholesale via privateIPBlocks to avoid a false-negative bypass. +var nat64Prefixes []*net.IPNet + +// embeddedIPv4 returns the IPv4 address embedded in the low 32 bits of a NAT64 +// address if ip falls inside a NAT64 translation prefix, or nil otherwise. +func embeddedIPv4(ip net.IP) net.IP { + v6 := ip.To16() + if v6 == nil || ip.To4() != nil { + return nil + } + for _, block := range nat64Prefixes { + if block.Contains(ip) { + return net.IPv4(v6[12], v6[13], v6[14], v6[15]) + } + } + return nil +} + +func init() { + for _, cidr := range []string{ + "0.0.0.0/8", // RFC1122 "this host" (incl. 0.0.0.0 unspecified) + "127.0.0.0/8", // IPv4 loopback + "10.0.0.0/8", // RFC1918 + "172.16.0.0/12", // RFC1918 + "192.168.0.0/16", // RFC1918 + "169.254.0.0/16", // RFC3927 link-local + "::1/128", // IPv6 loopback + "fe80::/10", // IPv6 link-local + "fc00::/7", // IPv6 unique local addr + "100.64.0.0/10", // RFC6598 shared address space (CGN) + "192.0.2.0/24", // RFC5737 documentation (TEST-NET-1) + "198.51.100.0/24", // RFC5737 documentation (TEST-NET-2) + "203.0.113.0/24", // RFC5737 documentation (TEST-NET-3) + "224.0.0.0/4", // IPv4 multicast + "240.0.0.0/4", // RFC1112 reserved (Class E), incl. 255.255.255.255 broadcast + "ff00::/8", // IPv6 multicast + // NAT64 local-use range (RFC 8215). The 64:ff9b:1::/96 subset is decoded + // to its embedded IPv4 below; this catch-all blocks the remaining + // non-/96 embeddings, which cannot be decoded from the address alone. + "64:ff9b:1::/48", + } { + _, block, err := net.ParseCIDR(cidr) + if err != nil { + panic(fmt.Errorf("parse error on %q: %w", cidr, err)) + } + privateIPBlocks = append(privateIPBlocks, block) + } + for _, cidr := range []string{ + "64:ff9b::/96", // NAT64 well-known prefix, /96 embedding (RFC 6052) + "64:ff9b:1::/96", // /96 sub-prefix of the local-use range (RFC 8215) + } { + _, block, err := net.ParseCIDR(cidr) + if err != nil { + panic(fmt.Errorf("parse error on %q: %w", cidr, err)) + } + nat64Prefixes = append(nat64Prefixes, block) + } +} + +// IsPrivateIP reports whether ip is a private, loopback, link-local, +// unspecified, or otherwise reserved/non-public address. +// +// NAT64-translated addresses are evaluated by the IPv4 address they embed: a +// NAT64 address whose low 32 bits map to a private/link-local IPv4 (e.g. +// 64:ff9b:1::a9fe:a9fe -> 169.254.169.254, the cloud metadata endpoint) is +// treated as private, because behind a NAT64 gateway it reaches exactly that +// internal IPv4, while NAT64 addresses embedding a genuinely public IPv4 remain +// allowed. This /96 decoding covers the well-known 64:ff9b::/96 (RFC 6052) and +// the 64:ff9b:1::/96 sub-prefix of the RFC 8215 local-use range; the rest of +// 64:ff9b:1::/48 uses a non-/96 embedding that cannot be decoded from the +// address alone and is blocked wholesale (see privateIPBlocks). +func IsPrivateIP(ip net.IP) bool { + if ip.IsLoopback() || ip.IsUnspecified() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() { + return true + } + if v4 := embeddedIPv4(ip); v4 != nil { + return IsPrivateIP(v4) + } + for _, block := range privateIPBlocks { + if block.Contains(ip) { + return true + } + } + return false +} + +// AddressReferencesPrivateIp returns an error if the address references a private IP address +func AddressReferencesPrivateIp(address string) error { + host, _, err := net.SplitHostPort(address) + if err != nil { + return err + } + // Check for a private IP address or loopback + ip := net.ParseIP(host) + if ip == nil { + // Fail closed: an unparsable host must not be treated as safe. + return fmt.Errorf("could not parse IP from address %q", address) + } + if IsPrivateIP(ip) { + return ErrPrivateIPAddress + } + + return nil +} + +// ValidateEndpointURL validates that an endpoint URL is secure. It reads +// INSECURE_DISABLE_URL_VALIDATION from the process environment; use +// ValidateEndpointURLWithReader to inject a different env.Reader (e.g. in tests). +func ValidateEndpointURL(endpoint string) error { + return ValidateEndpointURLWithReader(endpoint, &env.OSReader{}) +} + +// ValidateEndpointURLWithReader validates that an endpoint URL is secure, +// reading INSECURE_DISABLE_URL_VALIDATION via reader instead of the process +// environment directly. +func ValidateEndpointURLWithReader(endpoint string, reader env.Reader) error { + skipValidation := strings.EqualFold(reader.Getenv("INSECURE_DISABLE_URL_VALIDATION"), "true") + return validateEndpointURLWithSkip(endpoint, skipValidation) +} + +// ValidateEndpointURLWithInsecure validates that an endpoint URL is secure, allowing HTTP if insecureAllowHTTP is true. +// WARNING: This is insecure and should NEVER be used in production. It reads +// INSECURE_DISABLE_URL_VALIDATION from the process environment; use +// ValidateEndpointURLWithInsecureAndReader to inject a different env.Reader. +func ValidateEndpointURLWithInsecure(endpoint string, insecureAllowHTTP bool) error { + return ValidateEndpointURLWithInsecureAndReader(endpoint, insecureAllowHTTP, &env.OSReader{}) +} + +// ValidateEndpointURLWithInsecureAndReader validates that an endpoint URL is +// secure, allowing HTTP if insecureAllowHTTP is true, reading +// INSECURE_DISABLE_URL_VALIDATION via reader instead of the process +// environment directly. +// WARNING: insecureAllowHTTP is insecure and should NEVER be used in production. +func ValidateEndpointURLWithInsecureAndReader(endpoint string, insecureAllowHTTP bool, reader env.Reader) error { + skipValidation := strings.EqualFold(reader.Getenv("INSECURE_DISABLE_URL_VALIDATION"), "true") + return validateEndpointURLWithSkip(endpoint, skipValidation || insecureAllowHTTP) +} + +// validateEndpointURLWithSkip validates that an endpoint URL is secure, with an option to skip validation +func validateEndpointURLWithSkip(endpoint string, skipValidation bool) error { + if skipValidation { + return nil // Skip validation + } + u, err := url.Parse(endpoint) + if err != nil { + return fmt.Errorf("invalid URL: %w", err) + } + + // Ensure HTTPS for security (except localhost for development) + if u.Scheme != HttpsScheme && !IsLocalhost(u.Host) { + return fmt.Errorf("endpoint must use HTTPS: %s", endpoint) + } + + return nil +} + +// ValidateHTTPSURL checks that rawURL is a valid URL using the https scheme. +// Unlike ValidateEndpointURL, no localhost exception is made — HTTPS is always +// required (suitable for gateway URLs and other production endpoints). +func ValidateHTTPSURL(rawURL string) error { + parsed, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("invalid URL: %w", err) + } + if parsed.Host == "" { + return fmt.Errorf("URL must include a host: %s", rawURL) + } + if parsed.Scheme != HttpsScheme { + return fmt.Errorf("must use HTTPS, got scheme %q", parsed.Scheme) + } + return nil +} + +// ValidateIssuerURL validates that an OIDC issuer URL is well-formed and uses +// HTTPS. HTTP is permitted only for localhost (development). Per OIDC Core +// Section 3.1.2.1 and RFC 8414 Section 2, the issuer MUST use the "https" +// scheme. +func ValidateIssuerURL(rawURL string) error { + u, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("invalid issuer URL %q: %w", rawURL, err) + } + if u.Host == "" { + return fmt.Errorf("issuer URL must include a host: %s", rawURL) + } + if u.Scheme != HttpsScheme && !IsLocalhost(u.Host) { + return fmt.Errorf("issuer URL must use HTTPS (except localhost for development): %s", rawURL) + } + return nil +} + +// ValidateLoopbackAddress returns an error if addr (a host:port string) does +// not contain a literal loopback IP address. Both IPv4 (127.x.x.x) and IPv6 +// (::1) loopback addresses are accepted. Hostnames (including "localhost") are +// not resolved and will be rejected. +func ValidateLoopbackAddress(addr string) error { + host, _, err := net.SplitHostPort(addr) + if err != nil { + return fmt.Errorf("invalid listen address %q: %w", addr, err) + } + ip := net.ParseIP(host) + if ip == nil || !ip.IsLoopback() { + return fmt.Errorf("listen address %q must be a loopback interface (127.x.x.x or ::1)", addr) + } + return nil +} + +// hostLocalhost, hostLoopbackV4, and hostLoopbackV6 are the bare (no-port) +// loopback host forms recognised by IsLocalhost and IsLoopbackHost. +const ( + hostLocalhost = "localhost" + hostLoopbackV4 = "127.0.0.1" + hostLoopbackV6 = "[::1]" +) + +// IsLocalhost checks if a host is a loopback address (for development). +// Recognised forms: "localhost", "localhost:", "127.0.0.1", "127.0.0.1:", +// "[::1]", "[::1]:". +func IsLocalhost(host string) bool { + return strings.HasPrefix(host, hostLocalhost+":") || + strings.HasPrefix(host, hostLoopbackV4+":") || + strings.HasPrefix(host, hostLoopbackV6+":") || + host == hostLocalhost || + host == hostLoopbackV4 || + host == hostLoopbackV6 +} + +// IsLoopbackHost reports whether the Host header value refers to a loopback +// address. It is intended for DNS-rebinding guards on loopback-only listeners. +// It accepts the hostname "localhost" (case-insensitive), any 127.x.x.x +// address, and the IPv6 loopback ::1. Both plain-host and host:port forms are +// accepted. Hostnames other than "localhost" are NOT resolved. +func IsLoopbackHost(host string) bool { + h, _, err := net.SplitHostPort(host) + if err != nil { + // No port present — treat the whole value as the host. + h = host + // Strip brackets from bare IPv6 literals like "[::1]". + if len(h) > 2 && h[0] == '[' && h[len(h)-1] == ']' { + h = h[1 : len(h)-1] + } + } + if strings.EqualFold(h, hostLocalhost) { + return true + } + ip := net.ParseIP(h) + return ip != nil && ip.IsLoopback() +} + +// IsURL checks if the input is a valid HTTP or HTTPS URL +func IsURL(input string) bool { + parsedURL, err := url.Parse(input) + if err != nil { + return false + } + // Must have HTTP or HTTPS scheme and a valid host + return (parsedURL.Scheme == HttpScheme || parsedURL.Scheme == HttpsScheme) && + parsedURL.Host != "" && + parsedURL.Host != "//" +} diff --git a/networking/utilities_test.go b/networking/utilities_test.go new file mode 100644 index 0000000..efcc16d --- /dev/null +++ b/networking/utilities_test.go @@ -0,0 +1,882 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package networking + +import ( + "context" + "net" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/stacklok/toolhive-core/env/mocks" +) + +// Shared test literals. Pulled out as constants (rather than repeated string +// literals) purely to satisfy goconst across this package's test files. +const ( + testNotAURL = "not-a-url" + testHTTPExampleCom = "http://example.com" + testHTTPSEmptyHost = "https://" + testExampleComHost = "example.com" + testLoopbackV4WithPort = "127.0.0.1:8080" + testLoopbackV6WithPort = "[::1]:8080" + testPublicIPv4 = "8.8.8.8" + testHTTPLocalhost8080 = "http://localhost:8080" + testIdpExampleHost = "idp.example.com" + testAuthTokenEmptyErrMsg = "auth token file is empty" + testMCPStartURL = "https://mcp.example.com/start" + testNameValidHTTPSURL = "valid HTTPS URL" + testNameMissingHost = "missing host" + testNameUnsupportedURL = "unsupported scheme" + testNameInvalidURLFormat = "invalid URL format" +) + +func TestTargetIsPrivate(t *testing.T) { + t.Parallel() + tests := []struct { + name string + url string + want bool + }{ + {name: "private IPv4 literal", url: "https://10.0.0.5/path", want: true}, + {name: "link-local IMDS literal", url: "http://169.254.169.254/latest", want: true}, + {name: "loopback IPv4 literal", url: "http://127.0.0.1:8080", want: true}, + {name: "localhost hostname", url: "http://localhost:9000", want: true}, + {name: "public IPv4 literal", url: "https://8.8.8.8", want: false}, + {name: "unparsable", url: "://nope", want: false}, + {name: "empty host", url: "/just/a/path", want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, TargetIsPrivate(context.Background(), tt.url)) + }) + } +} + +func TestIsURL(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input string + expected bool + }{ + // Valid URLs + { + name: "valid https url", + input: "https://example.com", + expected: true, + }, + { + name: "valid http url", + input: testHTTPExampleCom, + expected: true, + }, + { + name: "valid https url with path", + input: "https://example.com/path", + expected: true, + }, + { + name: "valid https url with query params", + input: "https://example.com/path?param=value", + expected: true, + }, + { + name: "valid https url with fragment", + input: "https://example.com/path#fragment", + expected: true, + }, + { + name: "valid https url with port", + input: "https://example.com:8080", + expected: true, + }, + { + name: "valid https url with user info", + input: "https://user:pass@example.com", + expected: true, + }, + + // Invalid URLs + { + name: "empty string", + input: "", + expected: false, + }, + { + name: "invalid URL", + input: testNotAURL, + expected: false, + }, + { + name: testNameUnsupportedURL, + input: "ftp://example.com", + expected: false, + }, + { + name: "missing scheme", + input: testExampleComHost, + expected: false, + }, + { + name: testNameMissingHost, + input: testHTTPSEmptyHost, + expected: false, + }, + { + name: "missing host with path", + input: "https:///path", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result := IsURL(tt.input) + assert.Equal(t, tt.expected, result, "Input: %s", tt.input) + }) + } +} + +func TestIsLocalhost(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + expected bool + }{ + // Valid localhost hosts + { + name: "localhost without port", + input: hostLocalhost, + expected: true, + }, + { + name: "localhost with port", + input: "localhost:8080", + expected: true, + }, + { + name: "localhost with large port", + input: "localhost:65535", + expected: true, + }, + { + name: "127.0.0.1 without port", + input: hostLoopbackV4, + expected: true, + }, + { + name: "127.0.0.1 with port", + input: testLoopbackV4WithPort, + expected: true, + }, + { + name: "127.0.0.1 with large port", + input: "127.0.0.1:65535", + expected: true, + }, + { + name: "IPv6 localhost without port", + input: hostLoopbackV6, + expected: true, + }, + { + name: "IPv6 localhost with port", + input: testLoopbackV6WithPort, + expected: true, + }, + { + name: "IPv6 localhost with large port", + input: "[::1]:65535", + expected: true, + }, + + // Invalid localhost hosts + { + name: "empty string", + input: "", + expected: false, + }, + { + name: "random hostname", + input: testExampleComHost, + expected: false, + }, + { + name: "random hostname with port", + input: "example.com:8080", + expected: false, + }, + { + name: "public IP without port", + input: testPublicIPv4, + expected: false, + }, + { + name: "public IP with port", + input: "8.8.8.8:8080", + expected: false, + }, + { + name: "private IP without port", + input: "192.168.1.1", + expected: false, + }, + { + name: "private IP with port", + input: "192.168.1.1:8080", + expected: false, + }, + { + name: "IPv6 public address", + input: "[2001:db8::1]", + expected: false, + }, + { + name: "IPv6 public address with port", + input: "[2001:db8::1]:8080", + expected: false, + }, + { + name: "localhost with invalid port", + input: "localhost:99999", + expected: true, // Still matches the prefix check + }, + { + name: "127.0.0.1 with invalid port", + input: "127.0.0.1:99999", + expected: true, // Still matches the prefix check + }, + { + name: "IPv6 localhost with invalid port", + input: "[::1]:99999", + expected: true, // Still matches the prefix check + }, + { + name: "localhost with non-numeric port", + input: "localhost:abc", + expected: true, // Still matches the prefix check + }, + { + name: "127.0.0.1 with non-numeric port", + input: "127.0.0.1:abc", + expected: true, // Still matches the prefix check + }, + { + name: "IPv6 localhost with non-numeric port", + input: "[::1]:abc", + expected: true, // Still matches the prefix check + }, + { + name: "localhost with empty port", + input: "localhost:", + expected: true, // Still matches the prefix check + }, + { + name: "127.0.0.1 with empty port", + input: "127.0.0.1:", + expected: true, // Still matches the prefix check + }, + { + name: "IPv6 localhost with empty port", + input: "[::1]:", + expected: true, // Still matches the prefix check + }, + { + name: "case insensitive localhost", + input: "LOCALHOST", + expected: false, // Current implementation is case sensitive + }, + { + name: "case insensitive localhost with port", + input: "LOCALHOST:8080", + expected: false, // Current implementation is case sensitive + }, + { + name: "mixed case localhost", + input: "LocalHost", + expected: false, // Current implementation is case sensitive + }, + { + name: "localhost with spaces", + input: "localhost ", + expected: false, + }, + { + name: "localhost with leading space", + input: " localhost", + expected: false, + }, + { + name: "127.0.0.1 with spaces", + input: "127.0.0.1 ", + expected: false, + }, + { + name: "127.0.0.1 with leading space", + input: " 127.0.0.1", + expected: false, + }, + { + name: "IPv6 localhost with spaces", + input: "[::1] ", + expected: false, + }, + { + name: "IPv6 localhost with leading space", + input: " [::1]", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result := IsLocalhost(tt.input) + assert.Equal(t, tt.expected, result, "Input: %s", tt.input) + }) + } +} + +func TestIsPrivateIP(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + ip string + want bool + }{ + {"CGN 100.64.0.1", "100.64.0.1", true}, + {"CGN 100.127.255.255", "100.127.255.255", true}, + {"documentation TEST-NET-1", "192.0.2.1", true}, + {"documentation TEST-NET-2", "198.51.100.1", true}, + {"documentation TEST-NET-3", "203.0.113.1", true}, + {"public IPv4", testPublicIPv4, false}, + {"public IPv6", "2001:db8::1", false}, + + // Unspecified / "this host" / reserved ranges (defense-in-depth). + {"unspecified IPv4", "0.0.0.0", true}, + {"unspecified IPv6", "::", true}, + {`RFC1122 "this host" 0.x`, "0.1.2.3", true}, + {"Class E reserved", "240.0.0.1", true}, + {"limited broadcast", "255.255.255.255", true}, + + // NAT64 (RFC 6052 / RFC 8215): classified by the embedded IPv4. + // IPv4-mapped form is still caught by the link-local check, not NAT64. + {"IPv4-mapped link-local", "::ffff:169.254.169.254", true}, + // Well-known prefix 64:ff9b::/96 embedding a private/link-local IPv4. + {"NAT64 well-known -> IMDS link-local", "64:ff9b::a9fe:a9fe", true}, + {"NAT64 well-known -> loopback", "64:ff9b::7f00:1", true}, + {"NAT64 well-known -> RFC1918 10.x", "64:ff9b::a00:1", true}, + {"NAT64 well-known -> RFC1918 192.168.x", "64:ff9b::c0a8:1", true}, + // Well-known prefix embedding a genuinely public IPv4 must stay allowed. + {"NAT64 well-known -> public", "64:ff9b::8.8.8.8", false}, + // Local-use /96 sub-prefix is decoded the same way. + {"NAT64 local-use /96 -> IMDS", "64:ff9b:1::a9fe:a9fe", true}, + {"NAT64 local-use /96 -> public", "64:ff9b:1::8.8.8.8", false}, + // Remainder of the local-use /48 uses a non-/96 embedding that cannot be + // decoded, so it is blocked wholesale even when the low 32 bits look + // public (would otherwise be a false-negative SSRF bypass). + {"NAT64 local-use non-/96 blocked", "64:ff9b:1:1::8.8.8.8", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ip := net.ParseIP(tt.ip) + require.NotNil(t, ip, "failed to parse test IP %s", tt.ip) + assert.Equal(t, tt.want, IsPrivateIP(ip)) + }) + } +} + +func TestAddressReferencesPrivateIp(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + address string + expectError bool + }{ + // Public IP addresses (should not error) + { + name: "public IP with port", + address: "8.8.8.8:80", + expectError: false, + }, + { + name: "public IP with different port", + address: "1.1.1.1:443", + expectError: false, + }, + + // Private IP addresses (should error) + { + name: "localhost IP with port", + address: testLoopbackV4WithPort, + expectError: true, + }, + { + name: "RFC1918 10.x.x.x with port", + address: "10.0.0.1:80", + expectError: true, + }, + { + name: "RFC1918 172.16.x.x with port", + address: "172.16.0.1:80", + expectError: true, + }, + { + name: "RFC1918 192.168.x.x with port", + address: "192.168.1.1:80", + expectError: true, + }, + { + name: "link-local 169.254.x.x with port", + address: "169.254.1.1:80", + expectError: true, + }, + { + name: "IPv6 loopback with port", + address: testLoopbackV6WithPort, + expectError: true, + }, + { + name: "IPv6 link-local with port", + address: "[fe80::1]:80", + expectError: true, + }, + { + name: "IPv6 unique local with port", + address: "[fc00::1]:80", + expectError: true, + }, + { + name: "NAT64 well-known to IMDS with port", + address: "[64:ff9b::a9fe:a9fe]:443", + expectError: true, + }, + { + name: "NAT64 local-use to IMDS with port", + address: "[64:ff9b:1::a9fe:a9fe]:443", + expectError: true, + }, + { + name: "NAT64 to public IPv4 with port", + address: "[64:ff9b::8.8.8.8]:443", + expectError: false, + }, + { + name: "unspecified IPv4 with port", + address: "0.0.0.0:80", + expectError: true, + }, + { + name: "unspecified IPv6 with port", + address: "[::]:80", + expectError: true, + }, + + // Invalid addresses (should error due to parsing) + { + name: "invalid address format", + address: "invalid-address", + expectError: true, + }, + { + name: "unparsable host fails closed", + address: "not-an-ip:80", + expectError: true, + }, + { + name: "missing port", + address: testPublicIPv4, + expectError: true, + }, + { + name: "empty address", + address: "", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := AddressReferencesPrivateIp(tt.address) + if tt.expectError { + assert.Error(t, err, "Expected error for address: %s", tt.address) + } else { + assert.NoError(t, err, "Expected no error for address: %s", tt.address) + } + }) + } +} + +func TestValidateEndpointURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + endpoint string + skipValidation bool + expectError bool + }{ + // Valid HTTPS URLs (should not error) + { + name: testNameValidHTTPSURL, + endpoint: "https://example.com", + expectError: false, + }, + { + name: "valid HTTPS URL with path", + endpoint: "https://example.com/api/v1", + expectError: false, + }, + { + name: "valid HTTPS URL with port", + endpoint: "https://example.com:8443", + expectError: false, + }, + + // Localhost URLs with HTTP (should not error) + { + name: "localhost HTTP URL", + endpoint: testHTTPLocalhost8080, + expectError: false, + }, + { + name: "127.0.0.1 HTTP URL", + endpoint: "http://127.0.0.1:8080", + expectError: false, + }, + { + name: "IPv6 localhost HTTP URL", + endpoint: "http://[::1]:8080", + expectError: false, + }, + + // Non-localhost HTTP URLs (should error) + { + name: "HTTP URL for non-localhost", + endpoint: testHTTPExampleCom, + expectError: true, + }, + { + name: "HTTP URL with public IP", + endpoint: "http://8.8.8.8:80", + expectError: true, + }, + + // Invalid URLs (should error) + { + name: testNameInvalidURLFormat, + endpoint: testNotAURL, + expectError: true, + }, + { + name: "empty URL", + endpoint: "", + expectError: true, + }, + { + name: testNameUnsupportedURL, + endpoint: "ftp://example.com", + expectError: true, + }, + + // Skip validation cases (should not error) + { + name: "HTTP URL with validation skipped", + endpoint: testHTTPExampleCom, + skipValidation: true, + expectError: false, + }, + { + name: "invalid URL with validation skipped", + endpoint: testNotAURL, + skipValidation: true, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := validateEndpointURLWithSkip(tt.endpoint, tt.skipValidation) + if tt.expectError { + assert.Error(t, err, "Expected error for endpoint: %s", tt.endpoint) + } else { + assert.NoError(t, err, "Expected no error for endpoint: %s", tt.endpoint) + } + }) + } +} + +// TestValidateEndpointURLWithReader pins that the Reader-accepting sibling of +// ValidateEndpointURL reads INSECURE_DISABLE_URL_VALIDATION through the +// injected env.Reader rather than the process environment, so the test can +// run with t.Parallel instead of t.Setenv. +func TestValidateEndpointURLWithReader(t *testing.T) { + t.Parallel() + + t.Run("reader reports true: validation skipped", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockReader := mocks.NewMockReader(ctrl) + mockReader.EXPECT().Getenv("INSECURE_DISABLE_URL_VALIDATION").Return("true") + + err := ValidateEndpointURLWithReader(testNotAURL, mockReader) + assert.NoError(t, err) + }) + + t.Run("reader reports false: validation enforced", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockReader := mocks.NewMockReader(ctrl) + mockReader.EXPECT().Getenv("INSECURE_DISABLE_URL_VALIDATION").Return("") + + err := ValidateEndpointURLWithReader(testHTTPExampleCom, mockReader) + assert.Error(t, err) + }) +} + +// TestValidateEndpointURL_DefaultOSReader pins that ValidateEndpointURL still +// reads INSECURE_DISABLE_URL_VALIDATION from the process environment by +// default. Kept as its own non-parallel top-level test because t.Setenv +// panics if any ancestor test has called t.Parallel. +func TestValidateEndpointURL_DefaultOSReader(t *testing.T) { + t.Setenv("INSECURE_DISABLE_URL_VALIDATION", "true") + + err := ValidateEndpointURL(testNotAURL) + assert.NoError(t, err) +} + +// TestValidateEndpointURLWithInsecureAndReader mirrors +// TestValidateEndpointURLWithReader for the insecureAllowHTTP variant. +func TestValidateEndpointURLWithInsecureAndReader(t *testing.T) { + t.Parallel() + + t.Run("insecureAllowHTTP true skips validation regardless of the reader", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockReader := mocks.NewMockReader(ctrl) + mockReader.EXPECT().Getenv("INSECURE_DISABLE_URL_VALIDATION").Return("") + + err := ValidateEndpointURLWithInsecureAndReader(testHTTPExampleCom, true, mockReader) + assert.NoError(t, err) + }) + + t.Run("reader reports true: validation skipped", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + mockReader := mocks.NewMockReader(ctrl) + mockReader.EXPECT().Getenv("INSECURE_DISABLE_URL_VALIDATION").Return("true") + + err := ValidateEndpointURLWithInsecureAndReader(testNotAURL, false, mockReader) + assert.NoError(t, err) + }) +} + +// TestValidateEndpointURLWithInsecure_DefaultOSReader mirrors +// TestValidateEndpointURL_DefaultOSReader for the insecureAllowHTTP variant. +func TestValidateEndpointURLWithInsecure_DefaultOSReader(t *testing.T) { + t.Setenv("INSECURE_DISABLE_URL_VALIDATION", "true") + + err := ValidateEndpointURLWithInsecure(testNotAURL, false) + assert.NoError(t, err) +} + +func TestValidateHTTPSURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + url string + expectError bool + }{ + { + name: testNameValidHTTPSURL, + url: "https://llm.example.com", + expectError: false, + }, + { + name: "valid HTTPS URL with path", + url: "https://llm.example.com/api/v1", + expectError: false, + }, + { + name: "valid HTTPS URL with port", + url: "https://llm.example.com:8443", + expectError: false, + }, + { + name: "HTTP rejected even for localhost", + url: testHTTPLocalhost8080, + expectError: true, + }, + { + name: "HTTP rejected for remote host", + url: "http://llm.example.com", + expectError: true, + }, + { + name: testNameMissingHost, + url: testHTTPSEmptyHost, + expectError: true, + }, + { + name: testNameUnsupportedURL, + url: "ftp://llm.example.com", + expectError: true, + }, + { + name: testNameInvalidURLFormat, + url: testNotAURL, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := ValidateHTTPSURL(tt.url) + if tt.expectError { + assert.Error(t, err, "Expected error for URL: %s", tt.url) + } else { + assert.NoError(t, err, "Expected no error for URL: %s", tt.url) + } + }) + } +} + +func TestValidateIssuerURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + url string + expectError bool + }{ + { + name: "valid HTTPS issuer", + url: "https://auth.example.com", + expectError: false, + }, + { + name: "valid HTTPS issuer with path", + url: "https://auth.example.com/realms/myrealm", + expectError: false, + }, + { + name: "localhost HTTP allowed for development", + url: testHTTPLocalhost8080, + expectError: false, + }, + { + name: "127.0.0.1 HTTP allowed for development", + url: "http://127.0.0.1:9000", + expectError: false, + }, + { + name: "HTTP rejected for remote host", + url: "http://auth.example.com", + expectError: true, + }, + { + name: testNameMissingHost, + url: testHTTPSEmptyHost, + expectError: true, + }, + { + name: testNameInvalidURLFormat, + url: testNotAURL, + expectError: true, + }, + { + name: testNameUnsupportedURL, + url: "ftp://auth.example.com", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := ValidateIssuerURL(tt.url) + if tt.expectError { + assert.Error(t, err, "Expected error for URL: %s", tt.url) + } else { + assert.NoError(t, err, "Expected no error for URL: %s", tt.url) + } + }) + } +} + +func TestValidateLoopbackAddress(t *testing.T) { + t.Parallel() + tests := []struct { + addr string + wantErr bool + }{ + {"127.0.0.1:14000", false}, + {"[::1]:14000", false}, + {"0.0.0.0:14000", true}, + {"192.168.1.1:14000", true}, + {"10.0.0.1:14000", true}, + {"notanaddr", true}, + } + for _, tt := range tests { + t.Run(tt.addr, func(t *testing.T) { + t.Parallel() + err := ValidateLoopbackAddress(tt.addr) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestIsLoopbackHost(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + host string + want bool + }{ + {"localhost bare", hostLocalhost, true}, + {"localhost mixed case", "LocalHost", true}, + {"localhost with port", "localhost:8080", true}, + {"IPv4 loopback bare", hostLoopbackV4, true}, + {"IPv4 loopback with port", testLoopbackV4WithPort, true}, + {"IPv6 loopback bracketed", hostLoopbackV6, true}, + {"IPv6 loopback with port", testLoopbackV6WithPort, true}, + {"public hostname", testExampleComHost, false}, + {"public IP", testPublicIPv4, false}, + {"private IP is not loopback", "192.168.1.1", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, IsLoopbackHost(tt.host)) + }) + } +}