From 28079d30f6d4ce112207872ed4b91e94c27cf671 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 11 Jun 2026 00:57:30 -0700 Subject: [PATCH 1/3] fix(providers/azure): harden Synapse nil HTTP client fallback NewClientWithHTTP in the Synapse client was the only one of the Azure NewClientWithHTTP constructors substituting http.DefaultClient when the injected client is nil, bypassing the SSRF/IMDS-blocking transport from the project httpclient package. Replace the fallback with httpclient.New() to match the production NewClient constructor and the established savingsplans/managedredis pattern, and add a regression test asserting the nil fallback is never http.DefaultClient and rejects IMDS connections. Closes #1143 --- providers/azure/services/synapse/client.go | 5 +++-- .../azure/services/synapse/client_test.go | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/providers/azure/services/synapse/client.go b/providers/azure/services/synapse/client.go index 7a7a915fc..700f4ed4d 100644 --- a/providers/azure/services/synapse/client.go +++ b/providers/azure/services/synapse/client.go @@ -64,10 +64,11 @@ func NewClient(cred azcore.TokenCredential, subscriptionID, region string) *Syna } // NewClientWithHTTP creates a new Azure Synapse client with a custom HTTP client (for testing). -// If httpClient is nil, http.DefaultClient is used. +// When httpClient is nil, the SSRF-hardened httpclient.New() is used so the nil +// fallback also blocks IMDS connections. func NewClientWithHTTP(cred azcore.TokenCredential, subscriptionID, region string, httpClient HTTPClient) *SynapseClient { if httpClient == nil { - httpClient = http.DefaultClient + httpClient = httpclient.New() } return &SynapseClient{ cred: cred, diff --git a/providers/azure/services/synapse/client_test.go b/providers/azure/services/synapse/client_test.go index 30d67d9ed..4847dc87a 100644 --- a/providers/azure/services/synapse/client_test.go +++ b/providers/azure/services/synapse/client_test.go @@ -905,3 +905,21 @@ func TestPurchaseCommitment_canonicalReservedResourceType(t *testing.T) { armreservations.ReservedResourceType(capturedRRT), "reservedResourceType %q must be a member of armreservations.PossibleReservedResourceTypeValues()", capturedRRT) } + +// ---- nil HTTP client fallback ---------------------------------------------- + +// TestNewClientWithHTTP_NilFallbackIsHardened is a regression test for the +// codebase-review finding SEC-03 (issue #1143): when httpClient is nil, the +// httpClient falls back to httpclient.New() (SSRF-hardened), not +// http.DefaultClient, so the fallback also rejects IMDS connections. +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) + _, err = c.httpClient.Do(req) //nolint:bodyclose // Do always errors here; no body to close + require.Error(t, err, "nil-fallback client must reject IMDS connections") + assert.Contains(t, err.Error(), "blocked") +} From c6316b7dd8c5b85812408558460207b6c350fd30 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 20 Jun 2026 00:08:02 +0200 Subject: [PATCH 2/3] fix(lint): correct misspelling unrecognised->unrecognized in synapse comment --- providers/azure/services/synapse/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/azure/services/synapse/client.go b/providers/azure/services/synapse/client.go index 700f4ed4d..652810d1d 100644 --- a/providers/azure/services/synapse/client.go +++ b/providers/azure/services/synapse/client.go @@ -346,7 +346,7 @@ func (c *SynapseClient) GetOfferingDetails(ctx context.Context, rec common.Recom upfrontCost = 0 recurringCost = totalCost / (float64(termYears) * 12) default: - // Fail loud on an unrecognised payment option rather than silently + // Fail loud on an unrecognized payment option rather than silently // billing it as all-upfront (owner policy: no silent fallbacks on // money-affecting fields). return nil, fmt.Errorf("unsupported payment option for Azure Synapse offering details: %q", rec.PaymentOption) From 238f030e91a4cc488e4d58b65f003e1aee0960dd Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 20 Jun 2026 13:08:49 +0200 Subject: [PATCH 3/3] fix(lint): close HTTP response body in synapse nil-fallback test Replace nolint:bodyclose with proper deferred body close guarded by nil check, per the no-nolint policy. --- providers/azure/services/synapse/client_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/providers/azure/services/synapse/client_test.go b/providers/azure/services/synapse/client_test.go index 4847dc87a..14127491e 100644 --- a/providers/azure/services/synapse/client_test.go +++ b/providers/azure/services/synapse/client_test.go @@ -919,7 +919,10 @@ func TestNewClientWithHTTP_NilFallbackIsHardened(t *testing.T) { "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) - _, err = c.httpClient.Do(req) //nolint:bodyclose // Do always errors here; no body to close + 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") }