test(api): keep internal/api tests off the network and fail the run if one reaches it

internal/api/handler.go

@@ -140,6 +140,13 @@ type Handler struct {
 	// can be exercised end-to-end without live AWS credentials or a real RI.
 	executeExchangeFn func(ctx context.Context, req exchange.ExchangeExecuteRequest) (string, *exchange.ExchangeQuoteSummary, error)
 
+	// exchangeQuoteFn is the RI exchange quote function injected by tests.
+	// When nil (the production default), awsExchangeQuoter returns
+	// exchange.GetExchangeQuote, which builds its own EC2/STS clients from the
+	// ambient AWS SDK chain. Tests inject a stub so getExchangeQuote never
+	// resolves credentials, probes IMDS or calls EC2 (#855).
+	exchangeQuoteFn func(ctx context.Context, req exchange.ExchangeQuoteRequest) (*exchange.ExchangeQuoteSummary, error)
+
 	// encryptionKeySource is the env var name that resolved the credential
 	// encryption key. Empty when no credStore is configured. Read only by the
 	// /health credential_store check, which keeps it out of the public body

internal/api/handler_per_account_perms_test.go

@@ -1234,11 +1234,11 @@ func TestPerAccountPerms_GetRIUtilization_ScopedOutsideAllowedReturnsEmpty(t *te
 // TestPerAccountPerms_GetRIUtilization_ScopedWithinAllowedProceedsToAWS
 // confirms that a scoped user whose allowed_accounts DOES include the
 // deployment CloudAccount UUID is not blocked by the scope check. The handler
-// will then proceed to the AWS SDK (and fail without credentials), but the
-// scope filter itself must be transparent for an in-scope user.
+// then proceeds to the (stubbed) Cost Explorer and EC2 clients; the stubs keep
+// the test off the network and prove the scope filter was transparent (#855).
 //
 // Regression: an overly-broad scope check that blocks ALL scoped users
-// (not just out-of-scope ones) would cause this test to return empty prematurely.
+// (not just out-of-scope ones) would return empty before reaching the stubs.
 func TestPerAccountPerms_GetRIUtilization_ScopedWithinAllowedProceedsToAWS(t *testing.T) {
 	ctx := context.Background()
 
@@ -1247,25 +1247,27 @@ func TestPerAccountPerms_GetRIUtilization_ScopedWithinAllowedProceedsToAWS(t *te
 		return permsAccountList(), nil
 	}
 
+	recs := &countingUtilRecs{rows: []recommendations.RIUtilization{{ReservedInstanceID: "ri-own", UtilizationPercent: 40}}}
+	ec2Stub := &countingUtilEC2{instances: []ec2svc.ConvertibleRI{{ReservedInstanceID: "ri-own"}}}
 	handler := &Handler{
 		auth:   scopedAuthMock(ctx),
 		config: mockStore,
 		// Deployment account is permsAccA — WITHIN the scoped user's allowed set.
 		reshapeAccountResolver: func(_ context.Context) (string, error) {
 			return permsAccA, nil
 		},
+		reshapeRecsFactory: func(_ aws.Config) reshapeRecsClient { return recs },
+		reshapeEC2Factory:  func(_ aws.Config) reshapeEC2Client { return ec2Stub },
 	}
-	// Pre-seal awsCfgOnce so the handler doesn't stall on AWS SDK load.
-	handler.awsCfgOnce.Do(func() {})
+	// Pre-seal awsCfgOnce so the handler never runs LoadDefaultConfig.
+	handler.awsCfgOnce.Do(func() { handler.awsCfg = aws.Config{Region: "us-east-1"} })
 
-	_, err := handler.getRIUtilization(ctx, scopedReq())
-	// The scope check passes; any error from here is AWS-level, not scope-level.
-	if err != nil {
-		assert.NotContains(t, err.Error(), "allowed accounts",
-			"in-scope user must not be blocked by allowed_accounts check")
-		assert.NotContains(t, err.Error(), "cloud account scope",
-			"in-scope user must not be blocked by cloud account scope check")
-	}
+	resp, err := handler.getRIUtilization(ctx, scopedReq())
+	require.NoError(t, err, "in-scope user must not be blocked by the allowed_accounts check")
+	typed, ok := resp.(*RIUtilizationResponse)
+	require.True(t, ok, "expected *RIUtilizationResponse, got %T", resp)
+	assert.Equal(t, 1, recs.calls, "in-scope user must reach the Cost Explorer utilization fetch")
+	assert.Equal(t, []recommendations.RIUtilization{{ReservedInstanceID: "ri-own", UtilizationPercent: 40}}, typed.Utilization)
 }
 
 // ─── 12. GET /ri-exchange/reshape-recommendations ────────────────────────────

internal/api/handler_ri_exchange.go

@@ -1859,7 +1859,7 @@ func (h *Handler) getExchangeQuote(ctx context.Context, req *events.LambdaFuncti
 	}
 	region := cfg.Region
 
-	quote, err := exchange.GetExchangeQuote(ctx, exchange.ExchangeQuoteRequest{
+	quote, err := h.awsExchangeQuoter()(ctx, exchange.ExchangeQuoteRequest{
 		Region:           region,
 		ReservedIDs:      body.RIIDs,
 		Targets:          toExchangeTargets(body.Targets),

internal/api/handler_ri_exchange_test.go

@@ -1806,32 +1806,26 @@ func TestGetExchangeQuote_InvalidOfferingIDFormat(t *testing.T) {
 }
 
 func TestGetExchangeQuote_ValidOfferingIDPassesFormat(t *testing.T) {
-	// A well-formed UUID should pass the format check. The AWS call will
-	// fail because there's no real EC2 client wired -- but the test
-	// exercises that the regex guard does NOT fire on a valid UUID.
-	// We stub the handler to short-circuit before the AWS call.
-	called := false
-	stub := &stubTargetOfferingsEC2{
-		instances: []ec2svc.ConvertibleRI{},
+	// A well-formed UUID should pass the format check and reach the quote
+	// call. The quote function is stubbed so the test never resolves AWS
+	// credentials or calls EC2 (#855).
+	var quotes []exchange.ExchangeQuoteRequest
+	h := &Handler{
+		auth: &mockAuthForExchange{},
+		exchangeQuoteFn: func(_ context.Context, req exchange.ExchangeQuoteRequest) (*exchange.ExchangeQuoteSummary, error) {
+			quotes = append(quotes, req)
+			return &exchange.ExchangeQuoteSummary{IsValidExchange: true}, nil
+		},
 	}
-	_ = stub
-	_ = called
-
-	h := &Handler{auth: &mockAuthForExchange{}}
+	h.awsCfgOnce.Do(func() { h.awsCfg = aws.Config{Region: "us-east-1"} })
 
-	// The handler calls exchange.GetExchangeQuote next; that will fail
-	// with a config error in test (no real AWS cred). We only care that
-	// the regex guard doesn't block a valid UUID -- so any error after
-	// passing the UUID check is acceptable.
 	_, err := h.getExchangeQuote(context.Background(), &events.LambdaFunctionURLRequest{
 		Headers: map[string]string{"authorization": "Bearer test-token"},
 		Body:    `{"ri_ids":["ri-123"],"targets":[{"offering_id":"4b2293b4-5fbc-4017-9c75-d5a9d3aa8c91","count":1}]}`,
 	})
-	// Must NOT be the UUID format error
-	if err != nil {
-		assert.NotContains(t, err.Error(), "offering UUID",
-			"a valid UUID must not be rejected by the format check")
-	}
+	require.NoError(t, err, "a valid UUID must not be rejected by the format check")
+	require.Len(t, quotes, 1, "a valid UUID must reach the quote call")
+	assert.Equal(t, "4b2293b4-5fbc-4017-9c75-d5a9d3aa8c91", quotes[0].Targets[0].OfferingID)
 }
 
 func TestValidateExecuteExchangeBody_InvalidOfferingIDFormat(t *testing.T) {
@@ -2211,34 +2205,34 @@ func TestExecuteExchange_SessionScopeLookupErrorFailsClosed(t *testing.T) {
 // getExchangeQuote must resolve the region from the AWS SDK chain when
 // the caller omits it, matching getReshapeRecommendations, instead of
 // hardcoding us-east-1. We pre-seal awsCfgOnce with a non-default region
-// and verify the quote call receives that region (the call fails because
-// there is no real AWS SDK, but the region check happens first via body
-// validation passing; here we verify no 400 for missing region).
+// and verify the (stubbed, #855) quote call receives that region and the
+// response echoes it.
 func TestGetExchangeQuote_EmptyRegionResolvesFromSDK(t *testing.T) {
 	const cfgRegion = "eu-west-1"
-	h := &Handler{auth: &mockAuthForExchange{}}
+	var quotes []exchange.ExchangeQuoteRequest
+	h := &Handler{
+		auth: &mockAuthForExchange{},
+		exchangeQuoteFn: func(_ context.Context, req exchange.ExchangeQuoteRequest) (*exchange.ExchangeQuoteSummary, error) {
+			quotes = append(quotes, req)
+			return &exchange.ExchangeQuoteSummary{IsValidExchange: true}, nil
+		},
+	}
 	// Pre-seal awsCfgOnce so loadAWSConfigWithRegion returns cfgRegion
 	// without making a real SDK call.
 	h.awsCfgOnce.Do(func() {
 		h.awsCfg = aws.Config{Region: cfgRegion}
 	})
 
-	// The quote call will fail (no real AWS endpoint), but it must NOT fail
-	// with a 400 "region is required" — region resolution from the SDK is
-	// the accepted path for quote (non-mutation). The error will be a 500
-	// from exchange.GetExchangeQuote failing against no real AWS.
-	_, err := h.getExchangeQuote(context.Background(), &events.LambdaFunctionURLRequest{
+	resp, err := h.getExchangeQuote(context.Background(), &events.LambdaFunctionURLRequest{
 		Headers: map[string]string{"authorization": "Bearer test-token"},
-		Body:    `{"ri_ids":["ri-123"],"target_offering_id":"off-1"}`,
-	})
-	// We expect either no error or a 500 from the AWS call — never a 400
-	// complaining about a missing region.
-	if err != nil {
-		ce, ok := IsClientError(err)
-		require.True(t, ok, "expected ClientError, got: %v", err)
-		assert.NotEqual(t, 400, ce.code,
-			"getExchangeQuote must not return 400 for a missing region; it must resolve from the SDK chain")
-	}
+		Body:    `{"ri_ids":["ri-123"],"target_offering_id":"off-1","target_count":1}`,
+	})
+	require.NoError(t, err, "getExchangeQuote must not reject a missing region; it must resolve from the SDK chain")
+	require.Len(t, quotes, 1)
+	assert.Equal(t, cfgRegion, quotes[0].Region, "the quote must use the SDK-resolved region")
+	typed, ok := resp.(ExchangeQuoteResponse)
+	require.True(t, ok, "expected ExchangeQuoteResponse, got %T", resp)
+	assert.Equal(t, cfgRegion, typed.Region, "the response must echo the SDK-resolved region")
 }
 
 // TestExchangeQuoteResponse_MarshalsRegionAlongsideSummaryFields pins

internal/api/main_nonet_test.go

@@ -0,0 +1,216 @@
+package api
+
+import (
+	"bufio"
+	"context"
+	"fmt"
+	"net"
+	"net/http"
+	"os"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// netGuard is the no-network guard TestMain installs for this package (#855).
+var netGuard *noNetworkGuard
+
+// TestMain keeps this package's tests hermetic (#855). Before any test runs it:
+//
+//   - strips every AWS_* variable and points the shared config and
+//     credentials files at the null device, so no test can pick up a
+//     developer's or runner's real AWS identity, profile or region;
+//   - routes every proxy-aware HTTP client (Go's default transport, the AWS
+//     SDK including its IMDS client, the Azure and GCP SDKs) into a local
+//     listener that records and drops the connection;
+//   - replaces net.DefaultResolver with one whose name server is a local
+//     pipe, so a direct dial to a host name outside the hosts file fails and
+//     the looked-up name is recorded.
+//
+// Loopback traffic (httptest servers, local Postgres) bypasses the proxy and
+// is unaffected. If anything was blocked, the run fails after the tests finish
+// and names the blocked targets; each one is also printed as it happens, so
+// with -v it lands next to the test that made it. Not covered: dials to IP
+// literals by code that builds a transport with no proxy, and resolvers other
+// than net.DefaultResolver.
+func TestMain(m *testing.M) {
+	g, err := startNoNetworkGuard()
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "no-network guard:", err)
+		os.Exit(1)
+	}
+	netGuard = g
+	code := m.Run()
+	if blocked := g.stop(); len(blocked) > 0 {
+		fmt.Fprintf(os.Stderr, "FAIL: internal/api tests attempted %d outbound network call(s); stub the client instead (#855):\n  %s\n",
+			len(blocked), strings.Join(blocked, "\n  "))
+		code = 1
+	}
+	os.Exit(code)
+}
+
+type noNetworkGuard struct {
+	ln      net.Listener
+	mu      sync.Mutex
+	blocked []string
+}
+
+func startNoNetworkGuard() (*noNetworkGuard, error) {
+	if err := isolateAWSEnv(); err != nil {
+		return nil, err
+	}
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		return nil, fmt.Errorf("listen: %w", err)
+	}
+	g := &noNetworkGuard{ln: ln}
+	proxy := "http://" + ln.Addr().String()
+	for k, v := range map[string]string{
+		"HTTP_PROXY": proxy, "HTTPS_PROXY": proxy, "http_proxy": proxy, "https_proxy": proxy,
+		"NO_PROXY": "127.0.0.1,localhost,::1", "no_proxy": "127.0.0.1,localhost,::1",
+	} {
+		if err := os.Setenv(k, v); err != nil {
+			_ = ln.Close()
+			return nil, err
+		}
+	}
+	net.DefaultResolver = &net.Resolver{
+		PreferGo: true,
+		Dial: func(context.Context, string, string) (net.Conn, error) {
+			client, server := net.Pipe()
+			go g.recordDNSQuery(server)
+			return client, nil
+		},
+	}
+	go g.serve()
+	return g, nil
+}
+
+// isolateAWSEnv clears the ambient AWS SDK configuration so tests resolve the
+// same (empty) identity locally as in CI. IMDS stays enabled on purpose: a
+// test that falls through to it is then caught by the proxy, not hidden.
+func isolateAWSEnv() error {
+	for _, kv := range os.Environ() {
+		if k, _, _ := strings.Cut(kv, "="); strings.HasPrefix(k, "AWS_") {
+			if err := os.Unsetenv(k); err != nil {
+				return err
+			}
+		}
+	}
+	for _, k := range []string{"AWS_CONFIG_FILE", "AWS_SHARED_CREDENTIALS_FILE"} {
+		if err := os.Setenv(k, os.DevNull); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// serve accepts proxied connections, records the request line (CONNECT host
+// for HTTPS, the absolute URL for plain HTTP) and drops the connection.
+func (g *noNetworkGuard) serve() {
+	for {
+		c, err := g.ln.Accept()
+		if err != nil {
+			return
+		}
+		_ = c.SetReadDeadline(time.Now().Add(5 * time.Second))
+		line, _ := bufio.NewReader(c).ReadString('\n')
+		g.record(strings.TrimSpace(line))
+		_ = c.Close()
+	}
+}
+
+// recordDNSQuery reads the query the resolver writes to its (fake) name
+// server connection, records the looked-up name and drops the connection, so
+// the lookup fails without leaving the machine.
+func (g *noNetworkGuard) recordDNSQuery(c net.Conn) {
+	defer func() { _ = c.Close() }()
+	_ = c.SetReadDeadline(time.Now().Add(5 * time.Second))
+	buf := make([]byte, 1024)
+	n, _ := c.Read(buf)
+	// A pipe is not a net.PacketConn, so the resolver uses DNS-over-TCP
+	// framing: a 2-byte length prefix before the message.
+	msg := buf[:n]
+	if len(msg) >= 2 {
+		msg = msg[2:]
+	}
+	g.record("DNS lookup " + dnsQuestionName(msg))
+}
+
+// dnsQuestionName extracts the first question's name from a DNS query.
+func dnsQuestionName(msg []byte) string {
+	const headerLen = 12
+	var labels []string
+	for i := headerLen; i < len(msg) && msg[i] != 0; {
+		n := int(msg[i])
+		if n > 63 || i+1+n > len(msg) {
+			return "(unparsed query)"
+		}
+		labels = append(labels, string(msg[i+1:i+1+n]))
+		i += 1 + n
+	}
+	if len(labels) == 0 {
+		return "(unparsed query)"
+	}
+	return strings.Join(labels, ".")
+}
+
+func (g *noNetworkGuard) record(what string) {
+	fmt.Fprintf(os.Stderr, "no-network guard: blocked %s\n", what)
+	g.mu.Lock()
+	g.blocked = append(g.blocked, what)
+	g.mu.Unlock()
+}
+
+// takeMatching removes and returns the blocked entries containing substr, so
+// the guard's own self-test does not fail the run.
+func (g *noNetworkGuard) takeMatching(substr string) []string {
+	g.mu.Lock()
+	defer g.mu.Unlock()
+	var taken, kept []string
+	for _, b := range g.blocked {
+		if strings.Contains(b, substr) {
+			taken = append(taken, b)
+		} else {
+			kept = append(kept, b)
+		}
+	}
+	g.blocked = kept
+	return taken
+}
+
+func (g *noNetworkGuard) stop() []string {
+	_ = g.ln.Close()
+	g.mu.Lock()
+	defer g.mu.Unlock()
+	return append([]string(nil), g.blocked...)
+}
+
+// TestNoNetworkGuard_BlocksOutboundCalls pins the guard itself: an HTTPS call
+// through the default client and a DNS lookup through net.DefaultResolver must
+// both be stopped locally and recorded. The entries are taken back out so this
+// self-test does not fail the run.
+func TestNoNetworkGuard_BlocksOutboundCalls(t *testing.T) {
+	const host = "nonet-guard-selftest.invalid"
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, "https://"+host+"/", http.NoBody)
+	require.NoError(t, err)
+	resp, err := http.DefaultClient.Do(req)
+	if err == nil {
+		_ = resp.Body.Close()
+	}
+	require.Error(t, err, "an outbound HTTPS call must not succeed under the guard")
+	assert.Equal(t, []string{"CONNECT " + host + ":443 HTTP/1.1"}, netGuard.takeMatching("CONNECT "+host),
+		"the HTTPS call must reach the guard's proxy and be recorded")
+
+	_, err = net.DefaultResolver.LookupHost(ctx, host)
+	require.Error(t, err, "a DNS lookup must not succeed under the guard")
+	assert.NotEmpty(t, netGuard.takeMatching("DNS lookup "+host),
+		"the DNS lookup must reach the guard's resolver and be recorded")
+}