From 900c742b95d3e646b34f0c7036badb1691d5978e Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Tue, 20 Oct 2026 14:54:54 +0100 Subject: [PATCH] test(api): keep the RI-exchange carve-out test offline under mutation (#175) TestExecuteExchange_PlainAdminIsRefused drives the real executeExchange handler but injected no account resolver. With {execute, ri-exchange} dropped from adminCarvedOuts (the mutation this test exists to catch), the handler passed the gate and authorizeAWSExchangeExecution ran a live sts:GetCallerIdentity from ambient credentials. AWS_EC2_METADATA_DISABLED only cut off the IMDS fallback, so on a machine with credentials in the environment the mutated test still called STS for real. Inject the handler's existing reshapeAccountResolver seam with a stub that records it was reached and returns a sentinel error, plus an executeExchangeFn that fails the test if called. Under the mutation the test now fails in ~0s on its own "got past the gate" assertion, with no network I/O; unmutated it passes as before. The t.Setenv is dropped as it no longer guards anything. Closes #175 Co-Authored-By: Claude Opus 5.5 --- internal/api/ri_exchange_carveout_test.go | 56 ++++++++++++++++------- 1 file changed, 39 insertions(+), 17 deletions(-) diff --git a/internal/api/ri_exchange_carveout_test.go b/internal/api/ri_exchange_carveout_test.go index d143feafe..b4ff6aef7 100644 --- a/internal/api/ri_exchange_carveout_test.go +++ b/internal/api/ri_exchange_carveout_test.go @@ -2,8 +2,11 @@ package api import ( "context" + "errors" + "sync/atomic" "testing" + "github.com/LeanerCloud/cloud-commitments-go/pkg/exchange" "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" "github.com/aws/aws-lambda-go/events" "github.com/stretchr/testify/assert" @@ -115,11 +118,10 @@ func TestRIExchangeCarveOut_AdminKeepsNonCarvedVerbs(t *testing.T) { // (handler.go's requireSessionPermission), and executeExchange returns it // unwrapped (handler_ri_exchange.go:1713-1716). Asserting substrings of the // message ("execute", "ri-exchange") is fragile in the wrong direction: if -// the carve-out is dropped, executeExchange proceeds into -// exchange.ExecuteExchange, which builds its own AWS clients from ambient -// credentials and fails at AWS credential/STS resolution before ever -// touching the store -- an error whose wording has nothing to do with -// permissions. A prior version of this test relied on that STS error's +// the carve-out is dropped, executeExchange proceeds past the gate into +// cloud account scope resolution and then the AWS exchange call, and fails +// there with an error whose wording has nothing to do with permissions. +// A prior version of this test relied on that STS error's // message happening not to contain "execute"/"ri-exchange", which discovers // a regression only by accident and stops working the moment that wording // changes. Asserting the 403 ClientError identity instead means the test @@ -133,17 +135,22 @@ func TestRIExchangeCarveOut_AdminKeepsNonCarvedVerbs(t *testing.T) { // this handler and does not discriminate the mutation. It stays in case a // future change routes this handler through the store. // -// t.Setenv("AWS_EC2_METADATA_DISABLED", "true") bounds the AWS SDK client -// construction inside exchange.ExecuteExchange, reached only if the -// carve-out regresses (#1644), to fail fast against an unreachable -// credential source instead of depending on what credentials happen to be -// configured in whichever environment re-runs this test (tracked more -// broadly as #1760: executeExchange builds AWS clients from ambient -// credentials with no injected seam, unlike internal/server's -// riExchangeClients). +// The test stays offline even when the carve-out regresses (#175). Past the +// permission gate, executeExchange resolves the deployment's cloud account +// (authorizeAWSExchangeExecution -> resolveReshapeCloudAccountID) and, with +// no resolver injected, that runs a live sts:GetCallerIdentity from ambient +// credentials. AWS_EC2_METADATA_DISABLED only cut off the IMDS fallback; on a +// machine with credentials in the environment the mutated test still called +// STS for real. So the test injects the handler's own seams instead: +// +// - reshapeAccountResolver records that it was reached and returns +// errGateBypassed, so a regressed handler stops at the first step past +// the gate with no network I/O and the test fails on gateBypassed +// below, not on a timeout or a mock panic. +// - executeExchangeFn fails the test if called. It is unreachable while the +// resolver errors, and stays as a backstop in case a future change moves +// scope resolution after the AWS call. func TestExecuteExchange_PlainAdminIsRefused(t *testing.T) { - t.Setenv("AWS_EC2_METADATA_DISABLED", "true") - ctx := context.Background() mockStore := new(MockConfigStore) mockAuth := new(MockAuthService) @@ -153,7 +160,20 @@ func TestExecuteExchange_PlainAdminIsRefused(t *testing.T) { mockAuth.On("ValidateSession", ctx, "exchange-token").Return(session, nil) mockAuth.grantPermissions([]auth.Permission{{Action: auth.ActionAdmin, Resource: auth.ResourceAll}}) - h := &Handler{config: mockStore, auth: mockAuth} + errGateBypassed := errors.New("test stub: cloud account scope resolved past the execute:ri-exchange gate") + var gateBypassed atomic.Bool + h := &Handler{ + config: mockStore, + auth: mockAuth, + reshapeAccountResolver: func(context.Context) (string, error) { + gateBypassed.Store(true) + return "", errGateBypassed + }, + executeExchangeFn: func(context.Context, exchange.ExchangeExecuteRequest) (string, *exchange.ExchangeQuoteSummary, error) { + t.Error("executeExchange reached the AWS exchange executor for a plain admin (#1644)") + return "", nil, errGateBypassed + }, + } req := &events.LambdaFunctionURLRequest{ Headers: map[string]string{"Authorization": "Bearer exchange-token"}, Body: `{"ri_ids":["ri-1"],"target_offering_id":"off-1","region":"us-east-1","target_count":1,"max_payment_due_usd":"100.00"}`, @@ -163,8 +183,10 @@ func TestExecuteExchange_PlainAdminIsRefused(t *testing.T) { require.Error(t, err, "a plain admin must not be able to execute an RI exchange (#1644)") assert.Nil(t, result) + assert.False(t, gateBypassed.Load(), "a plain admin got past the execute:ri-exchange gate into cloud account scope resolution (#1644)") + assert.NotErrorIs(t, err, errGateBypassed, "the refusal must come from the permission gate, not from a step behind it") ce, ok := IsClientError(err) - require.True(t, ok, "a carve-out denial must be a client error, not a downstream AWS failure") + require.True(t, ok, "a carve-out denial must be a client error, not a downstream failure") assert.Equal(t, 403, ce.code, "must be refused at the permission gate, not fail later for an unrelated reason") mockStore.AssertNotCalled(t, "SaveRIExchangeRecord", mock.Anything, mock.Anything) }