fix(credentials): assume bastion targets with the bastion's credentials, never the host's

go.mod

@@ -59,7 +59,7 @@ require (
 	go.opentelemetry.io/otel/metric v1.45.0 // indirect
 	go.opentelemetry.io/otel/trace v1.45.0 // indirect
 	golang.org/x/crypto v0.58.0
-	golang.org/x/net v0.61.0 // indirect
+	golang.org/x/net v0.62.0 // indirect
 	golang.org/x/oauth2 v0.37.0
 	golang.org/x/sync v0.24.0
 	golang.org/x/sys v0.49.0 // indirect

go.sum

@@ -367,8 +367,8 @@ golang.org/x/crypto v0.58.0 h1:COkYLr4k7nDI4r6QtMML2AfzeIswWHAJthLS6K/M/54=
 golang.org/x/crypto v0.58.0/go.mod h1:Xh+kl5A+M0gMiWDytH2Hhr5Z7WbNCpv/eB/EFgfSk+w=
 golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg=
 golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs=
-golang.org/x/net v0.61.0 h1:RR+6j/BTBCrggOLugxa+66R8FvoN6MHlzDlZIbN9YFw=
-golang.org/x/net v0.61.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI=
+golang.org/x/net v0.62.0 h1:18ELalfM5+XNRwzM2nYqXsZQ9ByS1uhj2O/U7+5jqRY=
+golang.org/x/net v0.62.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI=
 golang.org/x/oauth2 v0.37.0 h1:JUlcxA8oAtauLfiH8FX2/FkAWHAdi0QtGCGc+hofE98=
 golang.org/x/oauth2 v0.37.0/go.mod h1:IxwZNxUULJmpBFf9K/9NTMSIfZZuvuTy1gGxhigP/58=
 golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA=

internal/api/bastion_resolve_test.go

@@ -0,0 +1,93 @@
+package api
+
+import (
+	"context"
+	"testing"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	awscreds "github.com/aws/aws-sdk-go-v2/credentials"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// Issue #263: every API path that resolves AWS credentials for a cloud account
+// passes the bastion lookup and a bastion STS factory, so a bastion-mode
+// account's target role is assumed with the bastion's own keys, never the
+// host's. STS is faked at the HTTP layer (probeSTS); nothing leaves the process.
+// The account test endpoint is covered in handler_accounts_awsprobe_test.go.
+
+const apiBastionID = "bastion-1"
+
+func apiBastionTarget() *config.CloudAccount {
+	return &config.CloudAccount{
+		ID: probeAccountID, Provider: "aws", Enabled: true, ExternalID: probeTenantAcct,
+		AWSAuthMode: "bastion", AWSBastionID: apiBastionID, AWSRoleARN: probeTenantRole,
+		AWSIsOrgRoot: true,
+	}
+}
+
+// apiBastionHandler returns a handler whose config store knows the bastion (an
+// access_keys account whose stored keys are probeMemberKeyID) and whose host
+// AWS config signs with probeHostKeyID against fake.
+func apiBastionHandler(fake *probeSTS, withStore bool) (*Handler, aws.Config) {
+	base := aws.Config{
+		Region:           "us-east-1",
+		Credentials:      awscreds.NewStaticCredentialsProvider(probeHostKeyID, "host-secret", ""),
+		HTTPClient:       fake,
+		RetryMaxAttempts: 1,
+	}
+	h := &Handler{credStore: &probeCredStore{payload: validAccessKeyPayload}}
+	if withStore {
+		store := new(MockConfigStore)
+		store.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) {
+			if id == apiBastionID {
+				return &config.CloudAccount{ID: apiBastionID, Provider: "aws", Enabled: true, AWSAuthMode: "access_keys"}, nil
+			}
+			return nil, nil
+		}
+		h.config = store
+	}
+	h.awsCfgOnce.Do(func() { h.awsCfg = base })
+	return h, base
+}
+
+var apiBastionCallers = map[string]func(ctx context.Context, h *Handler, base aws.Config) (aws.Config, error){
+	"marketplace": func(ctx context.Context, h *Handler, base aws.Config) (aws.Config, error) {
+		return h.marketplaceAccountAWSConfig(ctx, apiBastionTarget(), base)
+	},
+	"org discovery root": func(ctx context.Context, h *Handler, _ aws.Config) (aws.Config, error) {
+		return h.buildOrgRootAWSConfig(ctx, apiBastionTarget())
+	},
+}
+
+func TestAPIAWSCallers_BastionAssumesTargetAsBastion(t *testing.T) {
+	for name, call := range apiBastionCallers {
+		t.Run(name, func(t *testing.T) {
+			ctx := context.Background()
+			fake := &probeSTS{}
+			h, base := apiBastionHandler(fake, true)
+
+			cfg, err := call(ctx, h, base)
+			require.NoError(t, err)
+			creds, err := cfg.Credentials.Retrieve(ctx)
+			require.NoError(t, err)
+			assert.Equal(t, probeAssumedKey, creds.AccessKeyID)
+			assert.Equal(t, []string{probeMemberKeyID}, fake.assumeRoleSigners(),
+				"target role must be assumed with the bastion's keys, never the host's")
+		})
+	}
+}
+
+func TestAPIAWSCallers_BastionWithoutConfigStoreFailsClosed(t *testing.T) {
+	for name, call := range apiBastionCallers {
+		t.Run(name, func(t *testing.T) {
+			fake := &probeSTS{}
+			h, base := apiBastionHandler(fake, false)
+
+			_, err := call(context.Background(), h, base)
+			require.ErrorContains(t, err, "bastion account lookup and STS client factory are required")
+			assert.Zero(t, fake.assumeRoles.Load(), "no AssumeRole may be sent with the host's credentials")
+		})
+	}
+}

internal/api/handler_accounts.go

@@ -1645,7 +1645,7 @@ func (h *Handler) buildOrgRootAWSConfig(ctx context.Context, root *config.CloudA
 		return aws.Config{}, fmt.Errorf("accounts: load base aws config: %w", err)
 	}
 	stsClient := sts.NewFromConfig(baseCfg)
-	credProvider, err := credentials.ResolveAWSCredentialProvider(ctx, root, h.credStore, stsClient)
+	credProvider, err := credentials.ResolveAWSCredentialProviderWithOpts(ctx, root, h.credStore, stsClient, h.bastionResolveOptions(baseCfg))
 	if err != nil {
 		return aws.Config{}, fmt.Errorf("accounts: resolve credentials for org root %s: %w", root.ID, err)
 	}

internal/api/handler_accounts_awsprobe.go

@@ -45,15 +45,28 @@ func (h *Handler) awsResolverCredResult(ctx context.Context, acct *config.CloudA
 		base.Region = "us-east-1"
 	}
 	hostSTS := sts.NewFromConfig(base)
-	provider, err := credentials.ResolveAWSCredentialProviderWithOpts(ctx, acct, h.credStore, hostSTS,
-		credentials.AWSResolveOptions{AmbientProvider: base.Credentials, AmbientSTS: hostSTS})
+	opts := h.bastionResolveOptions(base)
+	opts.AmbientProvider, opts.AmbientSTS = base.Credentials, hostSTS
+	provider, err := credentials.ResolveAWSCredentialProviderWithOpts(ctx, acct, h.credStore, hostSTS, opts)
 	if err != nil {
 		logging.Warnf("account test %s: resolve AWS credentials: %v", acct.ID, err)
 		return AccountTestResult{OK: false, Message: classifyAWSResolveError(err)}
 	}
 	return probeAWSCallerIdentity(ctx, base, provider, acct)
 }
 
+// bastionResolveOptions returns resolver options carrying the config-store
+// bastion lookup and an STS client factory built on base, so a bastion-mode
+// account's target role is assumed with the bastion account's own
+// credentials, never the host's (issue #263). Every API path that resolves
+// AWS credentials for a cloud account passes these.
+func (h *Handler) bastionResolveOptions(base aws.Config) credentials.AWSResolveOptions {
+	return credentials.AWSResolveOptions{
+		AccountLookup:    credentials.AccountLookupFromStore(h.config),
+		STSClientFactory: credentials.NewSTSClientFactory(base),
+	}
+}
+
 // roleARNNotHostMessage is the fixed 400 for an AWS role_arn account saved
 // with no role ARN whose external_id is not confirmed as the CUDly host
 // account. It names neither the host account nor the STS failure (#531).

internal/api/handler_accounts_awsprobe_test.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -57,6 +58,25 @@ type probeSTS struct {
 	assumeRoles      atomic.Int32
 	webIdentityCalls atomic.Int32
 	identityCalls    atomic.Int32
+	mu               sync.Mutex
+	assumeSigners    []string // access key that signed each AssumeRole
+}
+
+// assumeRoleSigners returns the access key IDs that signed each AssumeRole.
+func (p *probeSTS) assumeRoleSigners() []string {
+	p.mu.Lock()
+	defer p.mu.Unlock()
+	return append([]string(nil), p.assumeSigners...)
+}
+
+// signerKey returns the access key ID in a SigV4 Authorization header.
+func signerKey(auth string) string {
+	_, rest, ok := strings.Cut(auth, "Credential=")
+	if !ok {
+		return ""
+	}
+	key, _, _ := strings.Cut(rest, "/")
+	return key
 }
 
 func probeXML(status int, body string) *http.Response {
@@ -90,6 +110,9 @@ func (p *probeSTS) Do(r *http.Request) (*http.Response, error) {
 		return probeXML(200, probeCredsXML("AssumeRoleWithWebIdentity", probeWebIdentKey, "arn:aws:sts::"+probeTenantAcct+":assumed-role/tenant/w")), nil
 	case strings.Contains(body, "Action=AssumeRole"):
 		p.assumeRoles.Add(1)
+		p.mu.Lock()
+		p.assumeSigners = append(p.assumeSigners, signerKey(r.Header.Get("Authorization")))
+		p.mu.Unlock()
 		if p.denyAssumeRole {
 			return probeError(403, "AccessDenied", "User: "+probeHostRoleARN+" is not authorized to perform: sts:AssumeRole on resource: "+probeTenantRole), nil
 		}
@@ -156,7 +179,15 @@ func runAccountTest(t *testing.T, pc probeCase) AccountTestResult {
 	acct := pc.acct
 	acct.ID = probeAccountID
 	acct.Provider = "aws"
-	store.GetCloudAccountFn = func(context.Context, string) (*config.CloudAccount, error) { return acct, nil }
+	// Any other ID is a bastion: an enabled access_keys account whose keys are
+	// the same stored payload (issue #263: bastion accounts resolve through
+	// their bastion's own credentials).
+	store.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) {
+		if id == probeAccountID {
+			return acct, nil
+		}
+		return &config.CloudAccount{ID: id, Provider: "aws", Enabled: true, AWSAuthMode: "access_keys"}, nil
+	}
 
 	if pc.stsFake == nil {
 		pc.stsFake = &probeSTS{}
@@ -220,7 +251,7 @@ func TestTestAccountCredentials_AWSFailuresAreClassified(t *testing.T) {
 		},
 		{
 			name:    "bastion mode without role ARN",
-			pc:      probeCase{acct: &config.CloudAccount{ExternalID: probeTenantAcct, AWSAuthMode: "bastion", AWSBastionID: "b"}},
+			pc:      probeCase{acct: &config.CloudAccount{ExternalID: probeTenantAcct, AWSAuthMode: "bastion", AWSBastionID: "b"}, payload: validAccessKeyPayload},
 			wantMsg: "credential configuration incomplete: aws_role_arn is required",
 		},
 		{
@@ -324,6 +355,7 @@ func TestTestAccountCredentials_AWSValidConfigsProbeCallerIdentity(t *testing.T)
 		wantAcct    string
 		wantAssume  int32
 		wantWebAuth int32
+		wantSigners []string
 	}{
 		{
 			name:     "access_keys",
@@ -349,10 +381,14 @@ func TestTestAccountCredentials_AWSValidConfigsProbeCallerIdentity(t *testing.T)
 			wantWebAuth: 1,
 		},
 		{
-			name:       "bastion with bastion id and role ARN",
-			acct:       &config.CloudAccount{ExternalID: probeTenantAcct, AWSAuthMode: "bastion", AWSBastionID: "bastion-1", AWSRoleARN: probeTenantRole},
-			wantAcct:   probeTenantAcct,
-			wantAssume: 1,
+			// Issue #263: the target role is assumed with the bastion's own
+			// keys (the stored payload), never the host's.
+			name:        "bastion with bastion id and role ARN",
+			acct:        &config.CloudAccount{ExternalID: probeTenantAcct, AWSAuthMode: "bastion", AWSBastionID: "bastion-1", AWSRoleARN: probeTenantRole},
+			payload:     validAccessKeyPayload,
+			wantAcct:    probeTenantAcct,
+			wantAssume:  1,
+			wantSigners: []string{probeMemberKeyID},
 		},
 	}
 	for _, tt := range tests {
@@ -364,6 +400,9 @@ func TestTestAccountCredentials_AWSValidConfigsProbeCallerIdentity(t *testing.T)
 			assert.NotContains(t, res.Message, "arn:", "success must report the target account, not the caller ARN")
 			assert.Equal(t, tt.wantAssume, fake.assumeRoles.Load())
 			assert.Equal(t, tt.wantWebAuth, fake.webIdentityCalls.Load())
+			if tt.wantSigners != nil {
+				assert.Equal(t, tt.wantSigners, fake.assumeRoleSigners(), "AssumeRole signed by the wrong credentials")
+			}
 			assert.GreaterOrEqual(t, fake.identityCalls.Load(), int32(1), "sts:GetCallerIdentity probe must run")
 		})
 	}

internal/api/handler_marketplace.go

@@ -91,8 +91,9 @@ func (h *Handler) loadMarketplaceAWSConfig(ctx context.Context, row *config.Purc
 // account, resolved through the purchase path's credential resolver.
 func (h *Handler) marketplaceAccountAWSConfig(ctx context.Context, account *config.CloudAccount, base aws.Config) (aws.Config, error) {
 	stsClient := sts.NewFromConfig(base)
-	provider, err := credentials.ResolveAWSCredentialProviderWithOpts(ctx, account, h.credStore, stsClient,
-		credentials.AWSResolveOptions{AmbientProvider: base.Credentials, AmbientSTS: stsClient})
+	opts := h.bastionResolveOptions(base)
+	opts.AmbientProvider, opts.AmbientSTS = base.Credentials, stsClient
+	provider, err := credentials.ResolveAWSCredentialProviderWithOpts(ctx, account, h.credStore, stsClient, opts)
 	if err != nil {
 		if errors.Is(err, credentials.ErrNotHostAccount) {
 			return aws.Config{}, NewClientError(400, err.Error())

internal/credentials/bastion_options.go

@@ -0,0 +1,41 @@
+package credentials
+
+import (
+	"context"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/service/sts"
+)
+
+// CloudAccountGetter is the config-store method the bastion path needs to load
+// the bastion account. config.StoreInterface satisfies it.
+type CloudAccountGetter interface {
+	GetCloudAccount(ctx context.Context, id string) (*config.CloudAccount, error)
+}
+
+// AccountLookupFromStore returns an AccountLookupFunc backed by store, or nil
+// when store is nil (bastion resolution then fails closed instead of
+// panicking on a nil store).
+func AccountLookupFromStore(store CloudAccountGetter) AccountLookupFunc {
+	if store == nil {
+		return nil
+	}
+	return store.GetCloudAccount
+}
+
+// NewSTSClientFactory returns an STSClientFactory that builds an STS client
+// from base (region, endpoint, HTTP client, retryer) signed with the given
+// provider instead of base's own credentials. The provider is cached so a
+// role-based bastion is not re-assumed on every STS call.
+func NewSTSClientFactory(base aws.Config) STSClientFactory {
+	return func(provider aws.CredentialsProvider) STSClient {
+		cfg := base.Copy()
+		if _, cached := provider.(*aws.CredentialsCache); cached {
+			cfg.Credentials = provider
+		} else {
+			cfg.Credentials = aws.NewCredentialsCache(provider)
+		}
+		return sts.NewFromConfig(cfg)
+	}
+}