diff --git a/internal/api/handler.go b/internal/api/handler.go index 5deb120ef..faa1ba7e7 100644 --- a/internal/api/handler.go +++ b/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 diff --git a/internal/api/handler_per_account_perms_test.go b/internal/api/handler_per_account_perms_test.go index 9979d639d..7474c293f 100644 --- a/internal/api/handler_per_account_perms_test.go +++ b/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,6 +1247,8 @@ 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, @@ -1254,18 +1256,18 @@ func TestPerAccountPerms_GetRIUtilization_ScopedWithinAllowedProceedsToAWS(t *te 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 ──────────────────────────── diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go index 89a961297..8bccc4d03 100644 --- a/internal/api/handler_ri_exchange.go +++ b/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), diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go index 9e3818c71..a2c94f059 100644 --- a/internal/api/handler_ri_exchange_test.go +++ b/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 diff --git a/internal/api/main_nonet_test.go b/internal/api/main_nonet_test.go new file mode 100644 index 000000000..52d5e4d62 --- /dev/null +++ b/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") +} diff --git a/internal/api/ri_exchange_detached.go b/internal/api/ri_exchange_detached.go index 7b872ff61..03c16c3ce 100644 --- a/internal/api/ri_exchange_detached.go +++ b/internal/api/ri_exchange_detached.go @@ -34,3 +34,12 @@ func (h *Handler) awsExchangeExecutor() func(context.Context, exchange.ExchangeE } return exchange.ExecuteExchange } + +// awsExchangeQuoter returns the AWS RI exchange quote function: the +// test-injected exchangeQuoteFn when set, exchange.GetExchangeQuote otherwise. +func (h *Handler) awsExchangeQuoter() func(context.Context, exchange.ExchangeQuoteRequest) (*exchange.ExchangeQuoteSummary, error) { + if h.exchangeQuoteFn != nil { + return h.exchangeQuoteFn + } + return exchange.GetExchangeQuote +} diff --git a/internal/api/tenant_scope_sweep_test.go b/internal/api/tenant_scope_sweep_test.go index 635e0e8ca..0fe7b301f 100644 --- a/internal/api/tenant_scope_sweep_test.go +++ b/internal/api/tenant_scope_sweep_test.go @@ -6,6 +6,7 @@ import ( "slices" "testing" + "github.com/LeanerCloud/cloud-commitments-go/pkg/exchange" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" "github.com/aws/aws-lambda-go/events" "github.com/aws/aws-sdk-go-v2/aws" @@ -151,29 +152,31 @@ func TestListActiveCommitments_ScopedCallerInScopeAccountFilterReachesStore(t *t store.AssertExpectations(t) } -// quoteHandler scopes the caller to scopedInAccount; the failing credentials -// keep a request that slips past the gate from calling EC2. -func quoteHandler(t *testing.T, deploymentAccount string) *Handler { +// quoteHandler scopes the caller to scopedInAccount. The injected quote stub +// records every request and fails, so a request that slips past the gate is +// counted instead of reaching EC2, and the test never resolves AWS credentials +// or probes IMDS (#855). +func quoteHandler(t *testing.T, deploymentAccount string) (*Handler, *[]exchange.ExchangeQuoteRequest) { h, store := scopedHandler(t, scopedInAccount) store.ListCloudAccountsFn = func(_ context.Context, _ config.CloudAccountFilter) ([]config.CloudAccount, error) { return []config.CloudAccount{inScopeAccount(), outOfScopeAccount()}, nil } h.reshapeAccountResolver = func(_ context.Context) (string, error) { return deploymentAccount, nil } - h.awsCfgOnce.Do(func() { - h.awsCfg = aws.Config{ - Region: "eu-west-1", - Credentials: aws.CredentialsProviderFunc(func(context.Context) (aws.Credentials, error) { - return aws.Credentials{}, errors.New("test: no AWS credentials") - }), - } - }) - return h + // Sealing the once keeps getBaseAWSConfig off LoadDefaultConfig; only the + // region is read on this path. + h.awsCfgOnce.Do(func() { h.awsCfg = aws.Config{Region: "eu-west-1"} }) + var quotes []exchange.ExchangeQuoteRequest + h.exchangeQuoteFn = func(_ context.Context, req exchange.ExchangeQuoteRequest) (*exchange.ExchangeQuoteSummary, error) { + quotes = append(quotes, req) + return nil, errors.New("test: quote stub") + } + return h, "es } const quoteBody = `{"ri_ids":["ri-123"],"target_offering_id":"4b2293b4-5fbc-4017-9c75-d5a9d3aa8c91","target_count":1}` func TestGetExchangeQuote_OutOfScopeDeploymentAccountReturns403(t *testing.T) { - h := quoteHandler(t, scopedOutAccount) + h, quotes := quoteHandler(t, scopedOutAccount) _, err := h.getExchangeQuote(context.Background(), &events.LambdaFunctionURLRequest{ Headers: scopedRequest("").Headers, Body: quoteBody, @@ -184,15 +187,19 @@ func TestGetExchangeQuote_OutOfScopeDeploymentAccountReturns403(t *testing.T) { require.True(t, ok, "expected a ClientError, got: %v", err) assert.Equal(t, 403, ce.code) assert.Contains(t, ce.Error(), "allowed accounts") + assert.Empty(t, *quotes, "an out-of-scope caller must not reach the AWS quote call") } func TestGetExchangeQuote_InScopeDeploymentAccountPassesGate(t *testing.T) { - h := quoteHandler(t, scopedInAccount) + h, quotes := quoteHandler(t, scopedInAccount) _, err := h.getExchangeQuote(context.Background(), &events.LambdaFunctionURLRequest{ Headers: scopedRequest("").Headers, Body: quoteBody, }) - require.Error(t, err, "the test AWS credentials fail, so the quote itself cannot succeed") + require.Error(t, err, "the quote stub fails, so the quote itself cannot succeed") assert.Contains(t, err.Error(), "exchange quote failed", "an in-scope caller must reach the AWS quote call") + require.Len(t, *quotes, 1, "an in-scope caller must reach the AWS quote call exactly once") + assert.Equal(t, "eu-west-1", (*quotes)[0].Region, "the quote must use the region resolved from the AWS config") + assert.Equal(t, []string{"ri-123"}, (*quotes)[0].ReservedIDs) }