From c5e5e6636cf30e068194d7a8b6c240f4cd3f4fe2 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 20:30:11 +0200 Subject: [PATCH] fix(azure): fall back to hardened httpclient on nil httpClient in 5 constructors Add the nil guard already present in savingsplans, synapse and managedredis to the database, cache, search, cosmosdb and compute NewClientWithHTTP, so a nil client uses the IMDS-blocking httpclient.New() instead of panicking on first use. Align the doc comments and add a regression test per package. Closes #17 Co-Authored-By: Claude Opus 5.5 --- providers/azure/services/cache/client.go | 8 ++++++-- providers/azure/services/cache/client_test.go | 18 ++++++++++++++++++ providers/azure/services/compute/client.go | 8 ++++++-- .../azure/services/compute/client_test.go | 18 ++++++++++++++++++ providers/azure/services/cosmosdb/client.go | 8 ++++++-- .../azure/services/cosmosdb/client_test.go | 18 ++++++++++++++++++ providers/azure/services/database/client.go | 8 ++++++-- .../azure/services/database/client_test.go | 18 ++++++++++++++++++ providers/azure/services/search/client.go | 8 ++++++-- providers/azure/services/search/client_test.go | 18 ++++++++++++++++++ 10 files changed, 120 insertions(+), 10 deletions(-) diff --git a/providers/azure/services/cache/client.go b/providers/azure/services/cache/client.go index 7fb60ada..2c577b20 100644 --- a/providers/azure/services/cache/client.go +++ b/providers/azure/services/cache/client.go @@ -100,10 +100,14 @@ func NewClient(cred azcore.TokenCredential, subscriptionID, region string) *Clie } // NewClientWithHTTP creates a new Azure Cache client with a custom HTTP client (for testing). -// httpClient is used as-is and bypasses the SSRF dial guard of -// httpclient.New(): metadata, NAT64/6to4 and unspecified addresses are +// When httpClient is nil, the SSRF-hardened httpclient.New() is used so the nil +// fallback also blocks IMDS connections. A non-nil httpClient is used as-is and +// bypasses that guard: metadata, NAT64/6to4 and unspecified addresses are // dialable unless the caller's client blocks them itself. func NewClientWithHTTP(cred azcore.TokenCredential, subscriptionID, region string, httpClient HTTPClient) *Client { + if httpClient == nil { + httpClient = httpclient.New() + } return &Client{ cred: cred, subscriptionID: subscriptionID, diff --git a/providers/azure/services/cache/client_test.go b/providers/azure/services/cache/client_test.go index 0890a3ea..051474f6 100644 --- a/providers/azure/services/cache/client_test.go +++ b/providers/azure/services/cache/client_test.go @@ -1488,3 +1488,21 @@ func TestCacheClient_GetValidResourceTypes_PageCapFires(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, skus, "page cap must trigger fallback to common SKUs, not an empty result") } + +// TestNewClientWithHTTP_NilFallbackIsHardened verifies that passing nil as the +// httpClient falls back to the SSRF-hardened httpclient.New(), which rejects +// IMDS connections, instead of leaving a nil client that panics on first use. +func TestNewClientWithHTTP_NilFallbackIsHardened(t *testing.T) { + c := NewClientWithHTTP(nil, "sub-123", "eastus", nil) + require.NotNil(t, c.httpClient) + require.NotEqual(t, http.DefaultClient, c.httpClient, + "nil fallback must never be http.DefaultClient") + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://169.254.169.254/metadata/instance", nil) + require.NoError(t, err) + resp, err := c.httpClient.Do(req) + if resp != nil { + defer resp.Body.Close() + } + require.Error(t, err, "nil-fallback client must reject IMDS connections") + assert.Contains(t, err.Error(), "blocked") +} diff --git a/providers/azure/services/compute/client.go b/providers/azure/services/compute/client.go index 24bf75f5..71e6f8ff 100644 --- a/providers/azure/services/compute/client.go +++ b/providers/azure/services/compute/client.go @@ -147,10 +147,14 @@ func NewClient(cred azcore.TokenCredential, subscriptionID, region string) *Clie } // NewClientWithHTTP creates a new Azure Compute client with a custom HTTP client (for testing). -// httpClient is used as-is and bypasses the SSRF dial guard of -// httpclient.New(): metadata, NAT64/6to4 and unspecified addresses are +// When httpClient is nil, the SSRF-hardened httpclient.New() is used so the nil +// fallback also blocks IMDS connections. A non-nil httpClient is used as-is and +// bypasses that guard: metadata, NAT64/6to4 and unspecified addresses are // dialable unless the caller's client blocks them itself. func NewClientWithHTTP(cred azcore.TokenCredential, subscriptionID, region string, httpClient HTTPClient) *Client { + if httpClient == nil { + httpClient = httpclient.New() + } return &Client{ cred: cred, subscriptionID: subscriptionID, diff --git a/providers/azure/services/compute/client_test.go b/providers/azure/services/compute/client_test.go index 0a0ff600..4718d0cb 100644 --- a/providers/azure/services/compute/client_test.go +++ b/providers/azure/services/compute/client_test.go @@ -1851,3 +1851,21 @@ func TestPurchaseBody_SKUAndQuantityStayInMatchingUnits(t *testing.T) { }) } } + +// TestNewClientWithHTTP_NilFallbackIsHardened verifies that passing nil as the +// httpClient falls back to the SSRF-hardened httpclient.New(), which rejects +// IMDS connections, instead of leaving a nil client that panics on first use. +func TestNewClientWithHTTP_NilFallbackIsHardened(t *testing.T) { + c := NewClientWithHTTP(nil, "sub-123", "eastus", nil) + require.NotNil(t, c.httpClient) + require.NotEqual(t, http.DefaultClient, c.httpClient, + "nil fallback must never be http.DefaultClient") + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://169.254.169.254/metadata/instance", nil) + require.NoError(t, err) + resp, err := c.httpClient.Do(req) + if resp != nil { + defer resp.Body.Close() + } + require.Error(t, err, "nil-fallback client must reject IMDS connections") + assert.Contains(t, err.Error(), "blocked") +} diff --git a/providers/azure/services/cosmosdb/client.go b/providers/azure/services/cosmosdb/client.go index c0b035e4..aa9df5e3 100644 --- a/providers/azure/services/cosmosdb/client.go +++ b/providers/azure/services/cosmosdb/client.go @@ -98,10 +98,14 @@ func NewClient(cred azcore.TokenCredential, subscriptionID, region string) *Clie } // NewClientWithHTTP creates a new Azure Cosmos DB client with a custom HTTP client (for testing). -// httpClient is used as-is and bypasses the SSRF dial guard of -// httpclient.New(): metadata, NAT64/6to4 and unspecified addresses are +// When httpClient is nil, the SSRF-hardened httpclient.New() is used so the nil +// fallback also blocks IMDS connections. A non-nil httpClient is used as-is and +// bypasses that guard: metadata, NAT64/6to4 and unspecified addresses are // dialable unless the caller's client blocks them itself. func NewClientWithHTTP(cred azcore.TokenCredential, subscriptionID, region string, httpClient HTTPClient) *Client { + if httpClient == nil { + httpClient = httpclient.New() + } return &Client{ cred: cred, subscriptionID: subscriptionID, diff --git a/providers/azure/services/cosmosdb/client_test.go b/providers/azure/services/cosmosdb/client_test.go index 0ab5aee8..c878a525 100644 --- a/providers/azure/services/cosmosdb/client_test.go +++ b/providers/azure/services/cosmosdb/client_test.go @@ -1704,3 +1704,21 @@ func TestReservationResourceTypeCosmosDB_IsCanonical(t *testing.T) { "Cosmos DB filter value must be the Azure REST API canonical enum \"CosmosDB\"; "+ "\"CosmosDb\" (pre-fix) is not a valid resourceType") } + +// TestNewClientWithHTTP_NilFallbackIsHardened verifies that passing nil as the +// httpClient falls back to the SSRF-hardened httpclient.New(), which rejects +// IMDS connections, instead of leaving a nil client that panics on first use. +func TestNewClientWithHTTP_NilFallbackIsHardened(t *testing.T) { + c := NewClientWithHTTP(nil, "sub-123", "eastus", nil) + require.NotNil(t, c.httpClient) + require.NotEqual(t, http.DefaultClient, c.httpClient, + "nil fallback must never be http.DefaultClient") + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://169.254.169.254/metadata/instance", nil) + require.NoError(t, err) + resp, err := c.httpClient.Do(req) + if resp != nil { + defer resp.Body.Close() + } + require.Error(t, err, "nil-fallback client must reject IMDS connections") + assert.Contains(t, err.Error(), "blocked") +} diff --git a/providers/azure/services/database/client.go b/providers/azure/services/database/client.go index 573776a2..220577e7 100644 --- a/providers/azure/services/database/client.go +++ b/providers/azure/services/database/client.go @@ -124,10 +124,14 @@ func NewClient(cred azcore.TokenCredential, subscriptionID, region string) *Clie } // NewClientWithHTTP creates a new Azure Database client with a custom HTTP client (for testing). -// httpClient is used as-is and bypasses the SSRF dial guard of -// httpclient.New(): metadata, NAT64/6to4 and unspecified addresses are +// When httpClient is nil, the SSRF-hardened httpclient.New() is used so the nil +// fallback also blocks IMDS connections. A non-nil httpClient is used as-is and +// bypasses that guard: metadata, NAT64/6to4 and unspecified addresses are // dialable unless the caller's client blocks them itself. func NewClientWithHTTP(cred azcore.TokenCredential, subscriptionID, region string, httpClient HTTPClient) *Client { + if httpClient == nil { + httpClient = httpclient.New() + } return &Client{ cred: cred, subscriptionID: subscriptionID, diff --git a/providers/azure/services/database/client_test.go b/providers/azure/services/database/client_test.go index e59548a2..00e430e2 100644 --- a/providers/azure/services/database/client_test.go +++ b/providers/azure/services/database/client_test.go @@ -2118,3 +2118,21 @@ func TestDatabaseClient_ServerInfo_FetchedOnce(t *testing.T) { assert.Equal(t, 0, serversPager.CallCount, "servers pager NextPage not called when it has no pages") } + +// TestNewClientWithHTTP_NilFallbackIsHardened verifies that passing nil as the +// httpClient falls back to the SSRF-hardened httpclient.New(), which rejects +// IMDS connections, instead of leaving a nil client that panics on first use. +func TestNewClientWithHTTP_NilFallbackIsHardened(t *testing.T) { + c := NewClientWithHTTP(nil, "sub-123", "eastus", nil) + require.NotNil(t, c.httpClient) + require.NotEqual(t, http.DefaultClient, c.httpClient, + "nil fallback must never be http.DefaultClient") + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://169.254.169.254/metadata/instance", nil) + require.NoError(t, err) + resp, err := c.httpClient.Do(req) + if resp != nil { + defer resp.Body.Close() + } + require.Error(t, err, "nil-fallback client must reject IMDS connections") + assert.Contains(t, err.Error(), "blocked") +} diff --git a/providers/azure/services/search/client.go b/providers/azure/services/search/client.go index 5e008013..bb1ded54 100644 --- a/providers/azure/services/search/client.go +++ b/providers/azure/services/search/client.go @@ -75,10 +75,14 @@ func NewClient(cred azcore.TokenCredential, subscriptionID, region string) *Clie } // NewClientWithHTTP creates a new Azure Search client with a custom HTTP client (for testing). -// httpClient is used as-is and bypasses the SSRF dial guard of -// httpclient.New(): metadata, NAT64/6to4 and unspecified addresses are +// When httpClient is nil, the SSRF-hardened httpclient.New() is used so the nil +// fallback also blocks IMDS connections. A non-nil httpClient is used as-is and +// bypasses that guard: metadata, NAT64/6to4 and unspecified addresses are // dialable unless the caller's client blocks them itself. func NewClientWithHTTP(cred azcore.TokenCredential, subscriptionID, region string, httpClient HTTPClient) *Client { + if httpClient == nil { + httpClient = httpclient.New() + } return &Client{ cred: cred, subscriptionID: subscriptionID, diff --git a/providers/azure/services/search/client_test.go b/providers/azure/services/search/client_test.go index 4a9d6c44..677284b6 100644 --- a/providers/azure/services/search/client_test.go +++ b/providers/azure/services/search/client_test.go @@ -1540,3 +1540,21 @@ func TestSearchClient_PurchaseCommitment_DisplayNameConformsToAzureAllowlist(t * assert.Regexp(t, `^search-`, capturedDisplayName) assert.Contains(t, capturedDisplayName, "standard2") } + +// TestNewClientWithHTTP_NilFallbackIsHardened verifies that passing nil as the +// httpClient falls back to the SSRF-hardened httpclient.New(), which rejects +// IMDS connections, instead of leaving a nil client that panics on first use. +func TestNewClientWithHTTP_NilFallbackIsHardened(t *testing.T) { + c := NewClientWithHTTP(nil, "sub-123", "eastus", nil) + require.NotNil(t, c.httpClient) + require.NotEqual(t, http.DefaultClient, c.httpClient, + "nil fallback must never be http.DefaultClient") + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "http://169.254.169.254/metadata/instance", nil) + require.NoError(t, err) + resp, err := c.httpClient.Do(req) + if resp != nil { + defer resp.Body.Close() + } + require.Error(t, err, "nil-fallback client must reject IMDS connections") + assert.Contains(t, err.Error(), "blocked") +}