From b4799568121df69430688a432a7b1876157a979a Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Tue, 27 Oct 2026 23:30:52 +0000 Subject: [PATCH 1/3] fix(credentials): assume bastion targets with the bastion's credentials, never the host's resolveBastionProvider fell back to assuming a bastion account's target role with the caller's STS client whenever AccountLookup or STSClientFactory was nil. No production caller set either option, so every bastion account was reached with the CUDly host's credentials and the bastion was never used (audit finding A03-011). Every AWS resolver caller now passes both options: the scheduler, the purchase manager, the commitment-options probe, and the API's marketplace, account-test and org-discovery paths. The lookup is the config store's GetCloudAccount; the factory builds an STS client from the host's AWS config signed with the bastion's credentials. The fallback is removed: the resolver now returns an error when either option is nil. The bastion's own credentials are resolved with the caller's ambient options, so a bastion that is the host account still works once the host check passes. Behaviour change: a bastion account whose target role trusts the CUDly host instead of the bastion now fails at AssumeRole until its trust policy allows the bastion. TestResolveBastionProvider_LegacyFallback is now an error assertion, and each caller has a test that it passes both options. Closes #263 Co-Authored-By: Claude Opus 5.5 --- internal/api/bastion_resolve_test.go | 93 +++++++++ internal/api/handler_accounts.go | 2 +- internal/api/handler_accounts_awsprobe.go | 17 +- .../api/handler_accounts_awsprobe_test.go | 51 ++++- internal/api/handler_marketplace.go | 5 +- internal/credentials/bastion_options.go | 41 ++++ internal/credentials/bastion_options_test.go | 174 +++++++++++++++++ internal/credentials/resolver.go | 46 ++--- internal/credentials/resolver_extra_test.go | 12 +- internal/credentials/resolver_test.go | 38 +++- internal/purchase/bastion_resolve_test.go | 108 +++++++++++ internal/purchase/execution.go | 7 +- internal/purchase/manager.go | 14 +- internal/scheduler/bastion_resolve_test.go | 176 ++++++++++++++++++ internal/scheduler/scheduler.go | 26 ++- internal/server/app.go | 40 ++-- .../server/commitment_probe_bastion_test.go | 112 +++++++++++ .../server/commitment_probe_config_test.go | 2 +- known_issues/03_credentials_resolver.md | 12 +- 19 files changed, 908 insertions(+), 68 deletions(-) create mode 100644 internal/api/bastion_resolve_test.go create mode 100644 internal/credentials/bastion_options.go create mode 100644 internal/credentials/bastion_options_test.go create mode 100644 internal/purchase/bastion_resolve_test.go create mode 100644 internal/scheduler/bastion_resolve_test.go create mode 100644 internal/server/commitment_probe_bastion_test.go diff --git a/internal/api/bastion_resolve_test.go b/internal/api/bastion_resolve_test.go new file mode 100644 index 000000000..c46e944fd --- /dev/null +++ b/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") + }) + } +} diff --git a/internal/api/handler_accounts.go b/internal/api/handler_accounts.go index bec9a02ce..23965a3a2 100644 --- a/internal/api/handler_accounts.go +++ b/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) } diff --git a/internal/api/handler_accounts_awsprobe.go b/internal/api/handler_accounts_awsprobe.go index 7261129d2..638a29b47 100644 --- a/internal/api/handler_accounts_awsprobe.go +++ b/internal/api/handler_accounts_awsprobe.go @@ -45,8 +45,9 @@ 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)} @@ -54,6 +55,18 @@ func (h *Handler) awsResolverCredResult(ctx context.Context, acct *config.CloudA 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). diff --git a/internal/api/handler_accounts_awsprobe_test.go b/internal/api/handler_accounts_awsprobe_test.go index 2354b1bf0..9380cdd29 100644 --- a/internal/api/handler_accounts_awsprobe_test.go +++ b/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") }) } diff --git a/internal/api/handler_marketplace.go b/internal/api/handler_marketplace.go index cded31ab4..f3beb8932 100644 --- a/internal/api/handler_marketplace.go +++ b/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()) diff --git a/internal/credentials/bastion_options.go b/internal/credentials/bastion_options.go new file mode 100644 index 000000000..4a5bf7601 --- /dev/null +++ b/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) + } +} diff --git a/internal/credentials/bastion_options_test.go b/internal/credentials/bastion_options_test.go new file mode 100644 index 000000000..81e4931f4 --- /dev/null +++ b/internal/credentials/bastion_options_test.go @@ -0,0 +1,174 @@ +package credentials + +import ( + "context" + "sync" + "testing" + "time" + + "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" + stypes "github.com/aws/aws-sdk-go-v2/service/sts/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testBastionID = "bastion-acct" + testBastionKeyID = "BASTIONKEYID" + testTargetRole = "arn:aws:iam::999999999999:role/Target" +) + +// recordingSTS records AssumeRole calls and hands out fixed session keys. +type recordingSTS struct { + mu sync.Mutex + assumeRoleCalls int + roleARNs []string +} + +func (r *recordingSTS) AssumeRole(_ context.Context, in *sts.AssumeRoleInput, _ ...func(*sts.Options)) (*sts.AssumeRoleOutput, error) { + r.mu.Lock() + defer r.mu.Unlock() + r.assumeRoleCalls++ + r.roleARNs = append(r.roleARNs, aws.ToString(in.RoleArn)) + return &sts.AssumeRoleOutput{Credentials: &stypes.Credentials{ + AccessKeyId: aws.String("TARGETSESSIONKEY"), + SecretAccessKey: aws.String("target-session-secret"), + SessionToken: aws.String("target-session-token"), + Expiration: aws.Time(time.Now().Add(time.Hour)), + }}, nil +} + +func (r *recordingSTS) AssumeRoleWithWebIdentity(_ context.Context, _ *sts.AssumeRoleWithWebIdentityInput, _ ...func(*sts.Options)) (*sts.AssumeRoleWithWebIdentityOutput, error) { + return nil, assert.AnError +} + +// accessKeyBastionFixture returns a store holding access keys for an enabled +// access_keys bastion and options that look it up and build a mock STS client. +func accessKeyBastionFixture() (*mockCredentialStore, AWSResolveOptions) { + store := newMockStore() + store.data[testBastionID+"/"+CredTypeAWSAccessKeys] = []byte(`{"access_key_id":"` + testBastionKeyID + `","secret_access_key":"bastion-test-value"}`) + bastion := &config.CloudAccount{ID: testBastionID, Enabled: true, AWSAuthMode: "access_keys"} + return store, AWSResolveOptions{ + AccountLookup: func(_ context.Context, id string) (*config.CloudAccount, error) { + if id == testBastionID { + return bastion, nil + } + return nil, nil + }, + STSClientFactory: func(_ aws.CredentialsProvider) STSClient { return &mockSTSClient{} }, + } +} + +func bastionTarget() *config.CloudAccount { + return &config.CloudAccount{ID: "target-acct", AWSAuthMode: "bastion", AWSBastionID: testBastionID, AWSRoleARN: testTargetRole} +} + +// The target role must be assumed with the STS client the factory built from +// the bastion's credentials, never with the caller's (host) STS client. +func TestResolveBastionProvider_TargetAssumedWithBastionSTS(t *testing.T) { + store, opts := accessKeyBastionFixture() + bastionSTS := &recordingSTS{} + var bastionCreds aws.Credentials + opts.STSClientFactory = func(p aws.CredentialsProvider) STSClient { + var err error + bastionCreds, err = p.Retrieve(context.Background()) + require.NoError(t, err) + return bastionSTS + } + hostSTS := &recordingSTS{} + + provider, err := ResolveAWSCredentialProviderWithOpts(context.Background(), bastionTarget(), store, hostSTS, opts) + require.NoError(t, err) + assert.Equal(t, testBastionKeyID, bastionCreds.AccessKeyID, "factory must be given the bastion's own credentials") + + creds, err := provider.Retrieve(context.Background()) + require.NoError(t, err) + assert.Equal(t, "TARGETSESSIONKEY", creds.AccessKeyID) + assert.Equal(t, []string{testTargetRole}, bastionSTS.roleARNs, "target role must be assumed through the bastion STS client") + assert.Zero(t, hostSTS.assumeRoleCalls, "the caller's STS client must not assume the target role") +} + +// A bastion that is the CUDly host account (role_arn with no role ARN) keeps +// working: its credentials are the ambient ones, once the host check passes. +// A bastion of that shape that is not the host is refused. +func TestResolveBastionProvider_HostAccountBastion(t *testing.T) { + hostBastion := &config.CloudAccount{ID: testBastionID, Enabled: true, AWSAuthMode: "role_arn", ExternalID: "111111111111"} + newOpts := func(hostAccount string, gotProvider *aws.CredentialsProvider) AWSResolveOptions { + return AWSResolveOptions{ + AccountLookup: func(_ context.Context, _ string) (*config.CloudAccount, error) { return hostBastion, nil }, + STSClientFactory: func(p aws.CredentialsProvider) STSClient { + *gotProvider = p + return &recordingSTS{} + }, + AmbientProvider: testAmbientCreds, + AmbientSTS: &callerIdentitySTS{account: hostAccount}, + } + } + + t.Run("host account", func(t *testing.T) { + var got aws.CredentialsProvider + _, err := ResolveAWSCredentialProviderWithOpts(context.Background(), bastionTarget(), newMockStore(), &recordingSTS{}, newOpts("111111111111", &got)) + require.NoError(t, err) + require.NotNil(t, got) + creds, err := got.Retrieve(context.Background()) + require.NoError(t, err) + assert.Equal(t, "AMBIENTKEY", creds.AccessKeyID) + }) + t.Run("not the host account", func(t *testing.T) { + var got aws.CredentialsProvider + _, err := ResolveAWSCredentialProviderWithOpts(context.Background(), bastionTarget(), newMockStore(), &recordingSTS{}, newOpts("222222222222", &got)) + require.ErrorIs(t, err, ErrNotHostAccount) + assert.Nil(t, got, "no STS client may be built for an unverified bastion") + }) +} + +type stubAccountGetter struct{ ids []string } + +func (s *stubAccountGetter) GetCloudAccount(_ context.Context, id string) (*config.CloudAccount, error) { + s.ids = append(s.ids, id) + return &config.CloudAccount{ID: id}, nil +} + +func TestAccountLookupFromStore(t *testing.T) { + assert.Nil(t, AccountLookupFromStore(nil), "a nil store must give a nil lookup so bastion resolution fails closed") + + store := &stubAccountGetter{} + lookup := AccountLookupFromStore(store) + require.NotNil(t, lookup) + acct, err := lookup(context.Background(), testBastionID) + require.NoError(t, err) + assert.Equal(t, testBastionID, acct.ID) + assert.Equal(t, []string{testBastionID}, store.ids) +} + +// The factory's client must sign with the provider it is given (the bastion's +// credentials), not with base's own credentials, and keep base's region. +func TestNewSTSClientFactory_SignsWithGivenProvider(t *testing.T) { + base := aws.Config{Region: "eu-west-1", Credentials: testAmbientCreds} + bastion := aws.CredentialsProviderFunc(func(context.Context) (aws.Credentials, error) { + return aws.Credentials{AccessKeyID: testBastionKeyID, SecretAccessKey: "bastion-test-value"}, nil + }) + + for name, p := range map[string]aws.CredentialsProvider{ + "plain provider": bastion, + "cached provider": aws.NewCredentialsCache(bastion), + } { + t.Run(name, func(t *testing.T) { + client, ok := NewSTSClientFactory(base)(p).(*sts.Client) + require.True(t, ok) + o := client.Options() + assert.Equal(t, "eu-west-1", o.Region) + cache, ok := o.Credentials.(*aws.CredentialsCache) + require.True(t, ok, "credentials must be cached so a role-based bastion is not re-assumed per call") + assert.True(t, cache.IsCredentialsProvider(bastion), "cache must wrap the given provider directly") + creds, err := o.Credentials.Retrieve(context.Background()) + require.NoError(t, err) + assert.Equal(t, testBastionKeyID, creds.AccessKeyID) + }) + } + baseCreds, err := base.Credentials.Retrieve(context.Background()) + require.NoError(t, err) + assert.Equal(t, "AMBIENTKEY", baseCreds.AccessKeyID, "base config must not be modified") +} diff --git a/internal/credentials/resolver.go b/internal/credentials/resolver.go index f7446b4e5..c15c8df2f 100644 --- a/internal/credentials/resolver.go +++ b/internal/credentials/resolver.go @@ -92,10 +92,9 @@ type AccountLookupFunc func(ctx context.Context, id string) (*config.CloudAccoun type STSClientFactory func(provider aws.CredentialsProvider) STSClient // AWSResolveOptions holds optional dependencies for the AWS credential -// resolver. The bastion path needs both AccountLookup and STSClientFactory to -// self-resolve correctly; without them, bastion mode falls back to the -// pre-self-loading behavior (trusts the caller-supplied STS client) for -// backward compatibility. +// resolver. The bastion path requires both AccountLookup and STSClientFactory +// and fails closed when either is nil: it never trusts the caller-supplied +// STS client to carry bastion credentials (issue #263). // // AmbientProvider, when set, is returned for role_arn accounts whose // AWSRoleARN is empty (the "Self" account shape: auth_mode=role_arn with no @@ -112,9 +111,10 @@ type AWSResolveOptions struct { AmbientSTS CallerIdentityClient } -// ResolveAWSCredentialProvider is a back-compat wrapper that calls -// ResolveAWSCredentialProviderWithOpts with empty options. New callers should -// use the WithOpts variant so bastion mode can self-resolve. +// ResolveAWSCredentialProvider calls ResolveAWSCredentialProviderWithOpts +// with empty options. It cannot resolve bastion accounts (those need +// AccountLookup and STSClientFactory), so production callers use the WithOpts +// variant. func ResolveAWSCredentialProvider( ctx context.Context, account *config.CloudAccount, @@ -231,16 +231,15 @@ func roleSessionName(accountID string) string { // resolveBastionProvider implements the bastion auth mode: target account is // reached via STS AssumeRole using credentials from a separate bastion account. // -// Self-resolving path (preferred): when opts.AccountLookup and -// opts.STSClientFactory are both set, the resolver loads the bastion account, -// recursively resolves its credentials, builds a bastion-scoped STS client, and -// uses that to assume the target role. Bastion-of-bastion chaining is rejected -// at depth 1 to prevent loops. +// The resolver loads the bastion account via opts.AccountLookup, recursively +// resolves its credentials, builds a bastion-scoped STS client via +// opts.STSClientFactory, and uses that to assume the target role, so the +// target is always assumed as the bastion. Bastion-of-bastion chaining is +// rejected at depth 1 to prevent loops. // -// Legacy path: when either option is nil, the resolver falls back to the old -// behavior and trusts that the caller-supplied stsClient already carries -// bastion credentials. This preserves backward compatibility with callers that -// have not yet been updated to wire the lookup/factory. +// When either option is nil the resolver returns an error. It used to fall +// back to assuming the target role with the caller-supplied stsClient (the +// CUDly host's credentials), which silently skipped the bastion (issue #263). func resolveBastionProvider( ctx context.Context, account *config.CloudAccount, @@ -252,8 +251,7 @@ func resolveBastionProvider( return nil, awsConfigErrorf("aws_bastion_id", "is required", "credentials: aws_bastion_id is required for bastion auth mode (account %s)", account.ID) } if opts.AccountLookup == nil || opts.STSClientFactory == nil { - // Legacy fallback: trust caller's stsClient. Tracked in known_issues/03. - return resolveRoleARNProvider(ctx, account, stsClient, nil, nil) + return nil, fmt.Errorf("credentials: bastion account lookup and STS client factory are required for bastion auth mode (account %s)", account.ID) } bastion, err := opts.AccountLookup(ctx, account.AWSBastionID) if err != nil { @@ -268,10 +266,14 @@ func resolveBastionProvider( if bastion.AWSAuthMode == "bastion" { return nil, fmt.Errorf("credentials: bastion chaining not allowed (account %s references bastion %s which is itself a bastion)", account.ID, bastion.ID) } - // Recursively resolve the bastion's own credentials. Pass empty opts to - // guarantee the recursive call cannot trigger bastion mode (already - // guarded above by the AWSAuthMode check, but defense in depth). - bastionCreds, err := ResolveAWSCredentialProviderWithOpts(ctx, bastion, store, stsClient, AWSResolveOptions{}) + // Recursively resolve the bastion's own credentials. Drop AccountLookup + // and STSClientFactory so the recursive call cannot resolve a bastion + // (already guarded above by the AWSAuthMode check, but defense in depth). + // Keep the ambient options: a bastion that is the CUDly host account + // (role_arn with no role ARN) resolves to the host credentials once + // VerifyHostAccount confirms it, exactly as it does when used directly. + bastionOpts := AWSResolveOptions{AmbientProvider: opts.AmbientProvider, AmbientSTS: opts.AmbientSTS} + bastionCreds, err := ResolveAWSCredentialProviderWithOpts(ctx, bastion, store, stsClient, bastionOpts) if err != nil { return nil, fmt.Errorf("credentials: resolve bastion %s creds: %w", bastion.ID, err) } diff --git a/internal/credentials/resolver_extra_test.go b/internal/credentials/resolver_extra_test.go index 1aea66c2b..ca3be365a 100644 --- a/internal/credentials/resolver_extra_test.go +++ b/internal/credentials/resolver_extra_test.go @@ -56,22 +56,24 @@ func TestResolveBastionProvider_WithBastionAndRoleARN(t *testing.T) { AWSBastionID: "bastion-acct", AWSRoleARN: "arn:aws:iam::123456789012:role/CUDly", } - provider, err := ResolveAWSCredentialProvider(context.Background(), account, newMockStore(), &mockSTSClient{}) + store, opts := accessKeyBastionFixture() + provider, err := ResolveAWSCredentialProviderWithOpts(context.Background(), account, store, &mockSTSClient{}, opts) require.NoError(t, err) assert.NotNil(t, provider) } func TestResolveBastionProvider_NoRoleARN(t *testing.T) { - // bastion mode (legacy fallback) delegates to resolveRoleARNProvider with nil - // ambient; an empty ARN now returns the ambient-creds error instead of the - // old "is required" message. + // After the bastion resolves, the target goes through + // resolveRoleARNProvider with no ambient credentials, so an empty ARN is + // refused rather than resolving to anything. account := &config.CloudAccount{ ID: "acct1", AWSAuthMode: "bastion", AWSBastionID: "bastion-acct", AWSRoleARN: "", } - _, err := ResolveAWSCredentialProvider(context.Background(), account, newMockStore(), &mockSTSClient{}) + store, opts := accessKeyBastionFixture() + _, err := ResolveAWSCredentialProviderWithOpts(context.Background(), account, store, &mockSTSClient{}, opts) assert.Error(t, err) assert.Contains(t, err.Error(), "aws_role_arn is empty and no ambient credentials available") } diff --git a/internal/credentials/resolver_test.go b/internal/credentials/resolver_test.go index e1462f665..007565c80 100644 --- a/internal/credentials/resolver_test.go +++ b/internal/credentials/resolver_test.go @@ -395,9 +395,11 @@ func TestResolveBastionProvider_BastionDisabled(t *testing.T) { assert.Contains(t, err.Error(), "bastion account bastion-acct is disabled") } -// TestResolveBastionProvider_LegacyFallback verifies the back-compat path: -// when AccountLookup/STSClientFactory are nil, the resolver falls through to -// the old behavior of trusting the caller-supplied STS client. +// TestResolveBastionProvider_LegacyFallback pins the removal of the legacy +// fallback (issue #263, audit finding A03-011): when AccountLookup or +// STSClientFactory is nil, the resolver must fail instead of assuming the +// target role with the caller-supplied (host) STS client. Neither the lookup +// nor the factory nor the caller's STS client may be used. func TestResolveBastionProvider_LegacyFallback(t *testing.T) { target := &config.CloudAccount{ ID: "target-acct", @@ -405,9 +407,35 @@ func TestResolveBastionProvider_LegacyFallback(t *testing.T) { AWSBastionID: "bastion-acct", AWSRoleARN: "arn:aws:iam::999999999999:role/Target", } + lookup := func(_ context.Context, _ string) (*config.CloudAccount, error) { + t.Error("AccountLookup must not run when the STS client factory is missing") + return nil, nil + } + factory := func(_ aws.CredentialsProvider) STSClient { + t.Error("STSClientFactory must not run when the account lookup is missing") + return &mockSTSClient{} + } + cases := map[string]AWSResolveOptions{ + "no options": {}, + "lookup only": {AccountLookup: lookup}, + "factory only": {STSClientFactory: factory}, + "ambient only": {AmbientProvider: testAmbientCreds, AmbientSTS: &callerIdentitySTS{account: "111111111111"}}, + } + for name, opts := range cases { + t.Run(name, func(t *testing.T) { + hostSTS := &recordingSTS{} + provider, err := ResolveAWSCredentialProviderWithOpts(context.Background(), target, newMockStore(), hostSTS, opts) + require.Error(t, err) + assert.Nil(t, provider) + assert.Contains(t, err.Error(), "bastion account lookup and STS client factory are required") + assert.Zero(t, hostSTS.assumeRoleCalls, "the caller's STS client must never assume the target role") + }) + } + + // The 4-argument wrapper passes no options, so it cannot resolve a bastion. provider, err := ResolveAWSCredentialProvider(context.Background(), target, newMockStore(), &mockSTSClient{}) - require.NoError(t, err) - assert.NotNil(t, provider) + require.Error(t, err) + assert.Nil(t, provider) } func TestResolveAzureCredentials_Success(t *testing.T) { diff --git a/internal/purchase/bastion_resolve_test.go b/internal/purchase/bastion_resolve_test.go new file mode 100644 index 000000000..c90245b27 --- /dev/null +++ b/internal/purchase/bastion_resolve_test.go @@ -0,0 +1,108 @@ +package purchase + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/credentials" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/sts" + stypes "github.com/aws/aws-sdk-go-v2/service/sts/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// Issue #263: purchases for a bastion-mode account must pass the bastion +// lookup and bastion STS factory to the resolver, so the target role is +// assumed with the bastion's own credentials, never the host's. + +const ( + purchBastionID = "bastion-acct" + purchBastionKeyID = "BASTIONKEYID" + purchTargetRole = "arn:aws:iam::999999999999:role/Target" +) + +func purchBastionDeps() (*MockConfigStore, *MockCredentialStore) { + store := new(MockConfigStore) + store.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) { + if id == purchBastionID { + return &config.CloudAccount{ID: purchBastionID, Provider: "aws", Enabled: true, AWSAuthMode: "access_keys"}, nil + } + return nil, nil + } + credStore := &MockCredentialStore{LoadRawFn: func(_ context.Context, accountID, credType string) ([]byte, error) { + if accountID == purchBastionID && credType == credentials.CredTypeAWSAccessKeys { + return []byte(`{"access_key_id":"` + purchBastionKeyID + `","secret_access_key":"bastion-test-value"}`), nil + } + return nil, nil + }} + return store, credStore +} + +// refuseHostAssumeRole makes any AssumeRole on the host STS client (a bug for +// a bastion target) return an error instead of a mock panic, so the +// assertions report it. +func refuseHostAssumeRole(m *MockAssumeRoleSTS) { + m.On("AssumeRole", mock.Anything, mock.Anything).Return(nil, errors.New("host STS client used for a bastion target")).Maybe() +} + +func purchBastionTarget() config.CloudAccount { + return config.CloudAccount{ + ID: "target-acct", Name: "target", Provider: "aws", Enabled: true, + AWSAuthMode: "bastion", AWSBastionID: purchBastionID, AWSRoleARN: purchTargetRole, + } +} + +func TestResolveAWSProvider_BastionUsesBastionCredentials(t *testing.T) { + ctx := context.Background() + store, credStore := purchBastionDeps() + hostSTS := new(MockAssumeRoleSTS) + refuseHostAssumeRole(hostSTS) + bastionSTS := new(MockAssumeRoleSTS) + bastionSTS.On("AssumeRole", mock.Anything, mock.MatchedBy(func(in *sts.AssumeRoleInput) bool { + return aws.ToString(in.RoleArn) == purchTargetRole + })).Return(&sts.AssumeRoleOutput{Credentials: &stypes.Credentials{ + AccessKeyId: aws.String("TARGETSESSIONKEY"), SecretAccessKey: aws.String("s"), SessionToken: aws.String("t"), + Expiration: aws.Time(time.Now().Add(time.Hour)), + }}, nil).Once() + var factoryKey string + + m := NewManager(ManagerConfig{ + ConfigStore: store, + CredentialStore: credStore, + AssumeRoleSTS: hostSTS, + BastionSTSFactory: func(p aws.CredentialsProvider) credentials.STSClient { + creds, err := p.Retrieve(ctx) + require.NoError(t, err) + factoryKey = creds.AccessKeyID + return bastionSTS + }, + }) + + cfg, err := m.resolveAWSProvider(ctx, purchBastionTarget()) + require.NoError(t, err) + assert.Equal(t, purchBastionKeyID, factoryKey, "bastion STS client must be built from the bastion's own keys") + store.AssertCalled(t, "GetCloudAccount", mock.Anything, purchBastionID) + + creds, err := cfg.AWSCredentialsProvider.Retrieve(ctx) + require.NoError(t, err) + assert.Equal(t, "TARGETSESSIONKEY", creds.AccessKeyID) + bastionSTS.AssertExpectations(t) + hostSTS.AssertNotCalled(t, "AssumeRole", mock.Anything, mock.Anything) +} + +func TestResolveAWSProvider_BastionWithoutFactoryFailsClosed(t *testing.T) { + store, credStore := purchBastionDeps() + hostSTS := new(MockAssumeRoleSTS) + refuseHostAssumeRole(hostSTS) + m := NewManager(ManagerConfig{ConfigStore: store, CredentialStore: credStore, AssumeRoleSTS: hostSTS}) + + cfg, err := m.resolveAWSProvider(context.Background(), purchBastionTarget()) + require.ErrorContains(t, err, "bastion account lookup and STS client factory are required") + assert.Nil(t, cfg) + hostSTS.AssertNotCalled(t, "AssumeRole", mock.Anything, mock.Anything) +} diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 901ab4b74..fc62ef7e6 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -536,7 +536,12 @@ func (m *Manager) resolveAWSProvider(ctx context.Context, account config.CloudAc return nil, fmt.Errorf("credentials: STS client not configured for non-access_keys mode (account %s)", account.ID) } awsCreds, err := credentials.ResolveAWSCredentialProviderWithOpts(ctx, &account, m.credStore, m.assumeRoleSTS, - credentials.AWSResolveOptions{AmbientProvider: m.ambientAWSCreds, AmbientSTS: m.stsClient}) + credentials.AWSResolveOptions{ + AccountLookup: credentials.AccountLookupFromStore(m.config), + STSClientFactory: m.bastionSTS, + AmbientProvider: m.ambientAWSCreds, + AmbientSTS: m.stsClient, + }) if err != nil { logging.Errorf("purchase[resolveAWSProvider]: failed for account=%s after %s: %v", account.ID, time.Since(t0), err) diff --git a/internal/purchase/manager.go b/internal/purchase/manager.go index 0466e754c..18cedbcf2 100644 --- a/internal/purchase/manager.go +++ b/internal/purchase/manager.go @@ -27,10 +27,14 @@ type STSClient interface { // ManagerConfig holds configuration for the purchase manager. type ManagerConfig struct { - AmbientAWSCreds aws.CredentialsProvider - EmailSender email.SenderInterface - STSClient STSClient - AssumeRoleSTS credentials.STSClient + AmbientAWSCreds aws.CredentialsProvider + EmailSender email.SenderInterface + STSClient STSClient + AssumeRoleSTS credentials.STSClient + // BastionSTSFactory builds the STS client a bastion-mode account's target + // role is assumed with, signed with the bastion account's credentials. + // Without it bastion accounts fail to resolve (issue #263). + BastionSTSFactory credentials.STSClientFactory CredentialStore credentials.CredentialStore ProviderFactory provider.FactoryInterface ConfigStore config.StoreInterface @@ -50,6 +54,7 @@ type Manager struct { email email.SenderInterface stsClient STSClient assumeRoleSTS credentials.STSClient + bastionSTS credentials.STSClientFactory ambientAWSCreds aws.CredentialsProvider credStore credentials.CredentialStore providerFactory provider.FactoryInterface @@ -113,6 +118,7 @@ func NewManager(cfg ManagerConfig) *Manager { email: cfg.EmailSender, stsClient: cfg.STSClient, assumeRoleSTS: cfg.AssumeRoleSTS, + bastionSTS: cfg.BastionSTSFactory, ambientAWSCreds: cfg.AmbientAWSCreds, credStore: cfg.CredentialStore, providerFactory: factory, diff --git a/internal/scheduler/bastion_resolve_test.go b/internal/scheduler/bastion_resolve_test.go new file mode 100644 index 000000000..99a2fcb7c --- /dev/null +++ b/internal/scheduler/bastion_resolve_test.go @@ -0,0 +1,176 @@ +package scheduler + +import ( + "context" + "errors" + "sync" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/credentials" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/sts" + stypes "github.com/aws/aws-sdk-go-v2/service/sts/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// Issue #263: per-account collection must pass the bastion lookup and bastion +// STS factory to the resolver, so a bastion-mode account's role is assumed +// with the bastion's own credentials, never the host's. + +const ( + schedBastionID = "bastion-acct" + schedBastionKeyID = "BASTIONKEYID" + schedTargetRole = "arn:aws:iam::999999999999:role/Target" +) + +// schedRecordingSTS records which role ARNs it was asked to assume. +type schedRecordingSTS struct { + mu sync.Mutex + roles []string +} + +func (r *schedRecordingSTS) AssumeRole(_ context.Context, in *sts.AssumeRoleInput, _ ...func(*sts.Options)) (*sts.AssumeRoleOutput, error) { + r.mu.Lock() + defer r.mu.Unlock() + r.roles = append(r.roles, aws.ToString(in.RoleArn)) + return &sts.AssumeRoleOutput{Credentials: &stypes.Credentials{ + AccessKeyId: aws.String("TARGETSESSIONKEY"), SecretAccessKey: aws.String("s"), SessionToken: aws.String("t"), + Expiration: aws.Time(time.Now().Add(time.Hour)), + }}, nil +} + +func (r *schedRecordingSTS) AssumeRoleWithWebIdentity(context.Context, *sts.AssumeRoleWithWebIdentityInput, ...func(*sts.Options)) (*sts.AssumeRoleWithWebIdentityOutput, error) { + return nil, errors.New("unexpected AssumeRoleWithWebIdentity") +} + +func (r *schedRecordingSTS) assumed() []string { + r.mu.Lock() + defer r.mu.Unlock() + return append([]string(nil), r.roles...) +} + +// schedBastionCredStore holds access keys for the bastion account only. +type schedBastionCredStore struct{ credentials.CredentialStore } + +func (schedBastionCredStore) LoadRaw(_ context.Context, accountID, credType string) ([]byte, error) { + if accountID == schedBastionID && credType == credentials.CredTypeAWSAccessKeys { + return []byte(`{"access_key_id":"` + schedBastionKeyID + `","secret_access_key":"bastion-test-value"}`), nil + } + return nil, nil +} + +func schedBastionStore() *MockConfigStore { + store := new(MockConfigStore) + store.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) { + if id == schedBastionID { + return &config.CloudAccount{ID: schedBastionID, Provider: "aws", Enabled: true, AWSAuthMode: "access_keys"}, nil + } + return nil, nil + } + return store +} + +func schedBastionTarget() config.CloudAccount { + return config.CloudAccount{ + ID: "target-acct", Provider: "aws", Enabled: true, ExternalID: "999999999999", + AWSAuthMode: "bastion", AWSBastionID: schedBastionID, AWSRoleARN: schedTargetRole, + } +} + +func TestScheduler_CollectAWSForAccount_BastionUsesBastionCredentials(t *testing.T) { + ctx := context.Background() + hostSTS := &schedRecordingSTS{} + bastionSTS := &schedRecordingSTS{} + var factoryKey string + + var gotCfg *provider.Config + mockFactory := new(MockProviderFactory) + mockFactory.On("CreateAndValidateProvider", ctx, "aws", mock.Anything). + Run(func(args mock.Arguments) { gotCfg = args.Get(2).(*provider.Config) }). + Return(nil, errors.New("stop after credential resolution")) + + store := schedBastionStore() + s := NewScheduler(SchedulerConfig{ + ConfigStore: store, + ProviderFactory: mockFactory, + CredentialStore: schedBastionCredStore{}, + AssumeRoleSTS: hostSTS, + BastionSTSFactory: func(p aws.CredentialsProvider) credentials.STSClient { + creds, err := p.Retrieve(ctx) + require.NoError(t, err) + factoryKey = creds.AccessKeyID + return bastionSTS + }, + }) + + _, _, err := s.collectAWSForAccount(ctx, &config.GlobalConfig{}, schedBastionTarget()) + require.ErrorContains(t, err, "stop after credential resolution") + require.NotNil(t, gotCfg, "credential resolution must succeed and reach the provider factory") + assert.Equal(t, schedBastionKeyID, factoryKey, "bastion STS client must be built from the bastion's own keys") + store.AssertCalled(t, "GetCloudAccount", mock.Anything, schedBastionID) + + creds, err := gotCfg.AWSCredentialsProvider.Retrieve(ctx) + require.NoError(t, err) + assert.Equal(t, "TARGETSESSIONKEY", creds.AccessKeyID) + assert.Equal(t, []string{schedTargetRole}, bastionSTS.assumed(), "target role must be assumed through the bastion") + assert.Empty(t, hostSTS.assumed(), "the host STS client must never assume a bastion account's target role") +} + +func TestScheduler_CollectAWSForAccount_BastionWithoutFactoryFailsClosed(t *testing.T) { + ctx := context.Background() + hostSTS := &schedRecordingSTS{} + mockFactory := new(MockProviderFactory) + s := NewScheduler(SchedulerConfig{ + ConfigStore: schedBastionStore(), + ProviderFactory: mockFactory, + CredentialStore: schedBastionCredStore{}, + AssumeRoleSTS: hostSTS, + }) + + recs, gap, err := s.collectAWSForAccount(ctx, &config.GlobalConfig{}, schedBastionTarget()) + require.ErrorContains(t, err, "bastion account lookup and STS client factory are required") + assert.Nil(t, recs) + assert.Nil(t, gap) + assert.Empty(t, hostSTS.assumed()) + mockFactory.AssertNotCalled(t, "CreateAndValidateProvider", mock.Anything, mock.Anything, mock.Anything) +} + +// A bastion that is the CUDly host account (role_arn with no role ARN) signs +// the target AssumeRole with the host credentials once the host check passes, +// as it did before #263; the scheduler must hand the resolver its ambient +// credentials and host STS client for that. +func TestScheduler_CollectAWSForAccount_HostAccountBastionUsesAmbient(t *testing.T) { + ctx := context.Background() + store := new(MockConfigStore) + store.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) { + return &config.CloudAccount{ID: id, Provider: "aws", Enabled: true, AWSAuthMode: "role_arn", ExternalID: "111111111111"}, nil + } + mockFactory := new(MockProviderFactory) + mockFactory.On("CreateAndValidateProvider", ctx, "aws", mock.Anything).Return(nil, errors.New("stop after credential resolution")) + var factoryKey string + s := NewScheduler(SchedulerConfig{ + ConfigStore: store, + ProviderFactory: mockFactory, + CredentialStore: schedBastionCredStore{}, + AssumeRoleSTS: &schedRecordingSTS{}, + STSClient: &fakeSTSClient{accountID: "111111111111"}, + AmbientAWSCreds: aws.CredentialsProviderFunc(func(context.Context) (aws.Credentials, error) { + return aws.Credentials{AccessKeyID: "AMBIENTKEY", SecretAccessKey: "ambient-test-value"}, nil + }), + BastionSTSFactory: func(p aws.CredentialsProvider) credentials.STSClient { + creds, err := p.Retrieve(ctx) + require.NoError(t, err) + factoryKey = creds.AccessKeyID + return &schedRecordingSTS{} + }, + }) + + _, _, err := s.collectAWSForAccount(ctx, &config.GlobalConfig{}, schedBastionTarget()) + require.ErrorContains(t, err, "stop after credential resolution") + assert.Equal(t, "AMBIENTKEY", factoryKey) +} diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index b756dd37b..a6d49695e 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -27,6 +27,7 @@ import ( "github.com/LeanerCloud/cloud-commitments-platform/internal/oidc" "github.com/LeanerCloud/cloud-commitments-platform/internal/providerfactory" "github.com/LeanerCloud/cloud-commitments-platform/internal/purchase" + "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/sts" "golang.org/x/sync/errgroup" "golang.org/x/sync/semaphore" @@ -51,6 +52,13 @@ type SchedulerConfig struct { //nolint:revive // exported: doc comment style int OIDCSigner oidc.Signer AssumeRoleSTS credentials.STSClient STSClient STSClient + // BastionSTSFactory builds the STS client a bastion-mode account's target + // role is assumed with, signed with the bastion account's credentials. + // Without it bastion accounts fail to resolve (issue #263). + BastionSTSFactory credentials.STSClientFactory + // AmbientAWSCreds are the host credentials. Only a bastion that is the + // host account itself (role_arn with no role ARN) uses them. + AmbientAWSCreds aws.CredentialsProvider DashboardURL string OIDCIssuerURL string IsLambda bool @@ -88,6 +96,8 @@ type Scheduler struct { credStore credentials.CredentialStore oidcSigner oidc.Signer assumeRoleSTS credentials.STSClient + bastionSTS credentials.STSClientFactory + ambientAWSCreds aws.CredentialsProvider config config.StoreInterface dashboardURL string oidcIssuerURL string @@ -118,6 +128,8 @@ func NewScheduler(cfg SchedulerConfig) *Scheduler { oidcSigner: cfg.OIDCSigner, oidcIssuerURL: cfg.OIDCIssuerURL, assumeRoleSTS: cfg.AssumeRoleSTS, + bastionSTS: cfg.BastionSTSFactory, + ambientAWSCreds: cfg.AmbientAWSCreds, stsClient: cfg.STSClient, isLambda: cfg.IsLambda, cacheTTL: cacheTTLFromEnv(), @@ -847,7 +859,7 @@ func (s *Scheduler) collectAWSForAccount(ctx context.Context, globalCfg *config. } return s.fetchAndConvert(ctx, prov, "aws", &acct.ID, acct.ExternalID, globalCfg) } - awsCreds, err := credentials.ResolveAWSCredentialProvider(ctx, &acct, s.credStore, s.assumeRoleSTS) + awsCreds, err := credentials.ResolveAWSCredentialProviderWithOpts(ctx, &acct, s.credStore, s.assumeRoleSTS, s.awsResolveOptions()) if err != nil { return nil, nil, fmt.Errorf("resolve credentials: %w", err) } @@ -859,6 +871,18 @@ func (s *Scheduler) collectAWSForAccount(ctx context.Context, globalCfg *config. return s.fetchAndConvert(ctx, prov, "aws", &acct.ID, acct.ExternalID, globalCfg) } +// awsResolveOptions returns the resolver options for per-account AWS +// collection: the config-store bastion lookup and bastion STS factory, plus +// the host credentials for a bastion that is the host account itself. +func (s *Scheduler) awsResolveOptions() credentials.AWSResolveOptions { + return credentials.AWSResolveOptions{ + AccountLookup: credentials.AccountLookupFromStore(s.config), + STSClientFactory: s.bastionSTS, + AmbientProvider: s.ambientAWSCreds, + AmbientSTS: s.stsClient, + } +} + // collectAzureRecommendations fans out across all enabled Azure accounts, // resolving per-account federated credentials via the KMS signer. // When no accounts are registered but AZURE_SUBSCRIPTION_ID is set (CUDly diff --git a/internal/server/app.go b/internal/server/app.go index f4e9b732b..833e15e99 100644 --- a/internal/server/app.go +++ b/internal/server/app.go @@ -873,6 +873,7 @@ func (app *Application) reinitializeAfterConnect(ctx context.Context, dbConn *da EmailSender: app.Email, STSClient: sts.NewFromConfig(awsCfg), AssumeRoleSTS: sts.NewFromConfig(awsCfg), + BastionSTSFactory: credentials.NewSTSClientFactory(awsCfg), AmbientAWSCreds: awsCfg.Credentials, CredentialStore: credStore, NotificationDaysBefore: app.appConfig.NotificationDaysBefore, @@ -889,15 +890,17 @@ func (app *Application) reinitializeAfterConnect(ctx context.Context, dbConn *da // Re-initialize scheduler with per-account credential resolution. reinitScheduler := scheduler.NewScheduler(scheduler.SchedulerConfig{ - ConfigStore: app.Config, - PurchaseManager: app.Purchase, - EmailSender: app.Email, - CredentialStore: credStore, - OIDCSigner: app.signer, - OIDCIssuerURL: resolveOIDCIssuerURL(app.appConfig), - AssumeRoleSTS: sts.NewFromConfig(awsCfg), - STSClient: sts.NewFromConfig(awsCfg), - IsLambda: app.appConfig.IsLambda, + ConfigStore: app.Config, + PurchaseManager: app.Purchase, + EmailSender: app.Email, + CredentialStore: credStore, + OIDCSigner: app.signer, + OIDCIssuerURL: resolveOIDCIssuerURL(app.appConfig), + AssumeRoleSTS: sts.NewFromConfig(awsCfg), + BastionSTSFactory: credentials.NewSTSClientFactory(awsCfg), + AmbientAWSCreds: awsCfg.Credentials, + STSClient: sts.NewFromConfig(awsCfg), + IsLambda: app.appConfig.IsLambda, }) app.Scheduler = reinitScheduler reinitPurchase.SetRecommendationSource(reinitScheduler) @@ -911,7 +914,7 @@ func (app *Application) reinitializeAfterConnect(ctx context.Context, dbConn *da commitmentOpts := commitmentopts.New( commitmentopts.NewPostgresStore(dbConn), app.Config, - newCommitmentProbeConfigBuilder(credStore, awsCfg.Credentials, sts.NewFromConfig(awsCfg)), + newCommitmentProbeConfigBuilder(credStore, app.Config, awsCfg, sts.NewFromConfig(awsCfg)), commitmentopts.DefaultProbers(), ) @@ -963,12 +966,19 @@ type hostAndAssumeRoleSTS interface { } // newCommitmentProbeConfigBuilder resolves the aws.Config the commitment-options -// probe uses for an account. ambient and stsClient let a role_arn account with -// no role ARN use the host credentials only when it is the host account. -func newCommitmentProbeConfigBuilder(credStore credentials.CredentialStore, ambient aws.CredentialsProvider, stsClient hostAndAssumeRoleSTS) commitmentopts.BuildConfigFn { +// probe uses for an account. hostCfg's credentials and stsClient let a role_arn +// account with no role ARN use the host credentials only when it is the host +// account. accounts and hostCfg let a bastion account be reached through its +// bastion's own credentials (issue #263). +func newCommitmentProbeConfigBuilder(credStore credentials.CredentialStore, accounts credentials.CloudAccountGetter, hostCfg aws.Config, stsClient hostAndAssumeRoleSTS) commitmentopts.BuildConfigFn { + opts := credentials.AWSResolveOptions{ + AccountLookup: credentials.AccountLookupFromStore(accounts), + STSClientFactory: credentials.NewSTSClientFactory(hostCfg), + AmbientProvider: hostCfg.Credentials, + AmbientSTS: stsClient, + } return func(ctx context.Context, acct *config.CloudAccount) (aws.Config, error) { - prov, err := credentials.ResolveAWSCredentialProviderWithOpts(ctx, acct, credStore, stsClient, - credentials.AWSResolveOptions{AmbientProvider: ambient, AmbientSTS: stsClient}) + prov, err := credentials.ResolveAWSCredentialProviderWithOpts(ctx, acct, credStore, stsClient, opts) if err != nil { return aws.Config{}, err } diff --git a/internal/server/commitment_probe_bastion_test.go b/internal/server/commitment_probe_bastion_test.go new file mode 100644 index 000000000..63eadb029 --- /dev/null +++ b/internal/server/commitment_probe_bastion_test.go @@ -0,0 +1,112 @@ +package server + +import ( + "context" + "errors" + "io" + "net/http" + "strings" + "sync" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/credentials" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Issue #263: the commitment-options probe must pass the bastion lookup and a +// bastion STS factory to the resolver. The STS endpoint is faked at the HTTP +// layer, so no request leaves the process. + +const ( + probeBastionID = "bastion-acct" + probeBastionKeyID = "BASTIONKEYID" + probeHostKeyID = "HOSTKEYID" + probeTargetRole = "arn:aws:iam::999999999999:role/Target" +) + +// fakeSTSEndpoint answers AssumeRole and records the access key that signed it. +type fakeSTSEndpoint struct { + mu sync.Mutex + signers []string +} + +func (f *fakeSTSEndpoint) Do(r *http.Request) (*http.Response, error) { + b, _ := io.ReadAll(r.Body) + if !strings.Contains(string(b), "Action=AssumeRole") { + return nil, errors.New("unexpected STS call: " + string(b)) + } + auth := r.Header.Get("Authorization") + f.mu.Lock() + switch { + case strings.Contains(auth, "Credential="+probeBastionKeyID+"/"): + f.signers = append(f.signers, probeBastionKeyID) + case strings.Contains(auth, "Credential="+probeHostKeyID+"/"): + f.signers = append(f.signers, probeHostKeyID) + default: + f.signers = append(f.signers, "unknown") + } + f.mu.Unlock() + body := `` + + `TARGETSESSIONKEYst` + + `2099-01-01T00:00:00ZA:s` + + `arn:aws:sts::999999999999:assumed-role/Target/s` + + `r` + return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(body)), Header: http.Header{"Content-Type": []string{"text/xml"}}}, nil +} + +type bastionAccountStore struct{ ids []string } + +func (s *bastionAccountStore) GetCloudAccount(_ context.Context, id string) (*config.CloudAccount, error) { + s.ids = append(s.ids, id) + if id == probeBastionID { + return &config.CloudAccount{ID: probeBastionID, Provider: "aws", Enabled: true, AWSAuthMode: "access_keys"}, nil + } + return nil, nil +} + +// bastionKeyStore serves access keys for the bastion only; other methods are +// unused and would panic on the nil embedded interface. +type bastionKeyStore struct{ credentials.CredentialStore } + +func (bastionKeyStore) LoadRaw(_ context.Context, accountID, _ string) ([]byte, error) { + if accountID == probeBastionID { + return []byte(`{"access_key_id":"` + probeBastionKeyID + `","secret_access_key":"bastion-test-value"}`), nil + } + return nil, nil +} + +func TestCommitmentProbeConfigBuilder_BastionAssumesTargetAsBastion(t *testing.T) { + t.Setenv("AWS_EC2_METADATA_DISABLED", "true") + endpoint := &fakeSTSEndpoint{} + hostCfg := aws.Config{ + Region: "us-east-1", + Credentials: aws.CredentialsProviderFunc(func(context.Context) (aws.Credentials, error) { + return aws.Credentials{AccessKeyID: probeHostKeyID, SecretAccessKey: "host-test-value"}, nil + }), + HTTPClient: endpoint, + RetryMaxAttempts: 1, + } + accounts := &bastionAccountStore{} + build := newCommitmentProbeConfigBuilder(bastionKeyStore{}, accounts, hostCfg, hostIdentitySTS{account: "111111111111"}) + + target := &config.CloudAccount{ID: "target-acct", Provider: "aws", AWSAuthMode: "bastion", AWSBastionID: probeBastionID, AWSRoleARN: probeTargetRole} + cfg, err := build(context.Background(), target) + require.NoError(t, err, "a bastion account must resolve once the lookup and factory are wired") + assert.Equal(t, []string{probeBastionID}, accounts.ids, "bastion account must be loaded from the config store") + + creds, err := cfg.Credentials.Retrieve(context.Background()) + require.NoError(t, err) + assert.Equal(t, "TARGETSESSIONKEY", creds.AccessKeyID) + assert.Equal(t, []string{probeBastionKeyID}, endpoint.signers, "target role must be assumed with the bastion's keys, never the host's") +} + +func TestCommitmentProbeConfigBuilder_BastionWithoutAccountStoreFailsClosed(t *testing.T) { + t.Setenv("AWS_EC2_METADATA_DISABLED", "true") + build := newCommitmentProbeConfigBuilder(bastionKeyStore{}, nil, aws.Config{Credentials: aws.AnonymousCredentials{}}, hostIdentitySTS{account: "111111111111"}) + target := &config.CloudAccount{ID: "target-acct", Provider: "aws", AWSAuthMode: "bastion", AWSBastionID: probeBastionID, AWSRoleARN: probeTargetRole} + _, err := build(context.Background(), target) + require.ErrorContains(t, err, "bastion account lookup and STS client factory are required") +} diff --git a/internal/server/commitment_probe_config_test.go b/internal/server/commitment_probe_config_test.go index a96daa58e..6e1a10e6a 100644 --- a/internal/server/commitment_probe_config_test.go +++ b/internal/server/commitment_probe_config_test.go @@ -31,7 +31,7 @@ func (s hostIdentitySTS) GetCallerIdentity(context.Context, *sts.GetCallerIdenti // accepted when it is not. func TestCommitmentProbeConfigBuilder_HostIdentityCheck(t *testing.T) { t.Setenv("AWS_EC2_METADATA_DISABLED", "true") - build := newCommitmentProbeConfigBuilder(nil, aws.AnonymousCredentials{}, hostIdentitySTS{account: "111111111111"}) + build := newCommitmentProbeConfigBuilder(nil, nil, aws.Config{Credentials: aws.AnonymousCredentials{}}, hostIdentitySTS{account: "111111111111"}) nonHost := &config.CloudAccount{ID: "tenant", ExternalID: "222222222222", AWSAuthMode: "role_arn"} _, err := build(context.Background(), nonHost) diff --git a/known_issues/03_credentials_resolver.md b/known_issues/03_credentials_resolver.md index 2bf6d630f..35234834c 100644 --- a/known_issues/03_credentials_resolver.md +++ b/known_issues/03_credentials_resolver.md @@ -31,12 +31,18 @@ ## ~~MEDIUM: `resolveBastionProvider` has no enforcement that the STS client carries bastion credentials~~ — RESOLVED -**File**: `internal/credentials/resolver.go:145-159` +**File**: `internal/credentials/resolver.go` (`resolveBastionProvider`) **Description**: `resolveBastionProvider` previously delegated straight to `resolveRoleARNProvider` with the caller-supplied `stsClient`, so it had no runtime check that the STS client actually carried bastion credentials. **Impact**: Silent privilege escalation or incorrect audit trail if callers passed the wrong STS client. -**Status:** ✔️ Resolved +**Status:** ✔️ Resolved (fully as of issue #263) + +**Resolved by (first step):** Added `AccountLookupFunc`, `STSClientFactory`, and `AWSResolveOptions` types in `internal/credentials/resolver.go`. New `ResolveAWSCredentialProviderWithOpts` accepts these options; the bastion path self-loads the bastion `CloudAccount` via `opts.AccountLookup`, recursively resolves its credentials, builds an STS client via `opts.STSClientFactory`, and only then assumes the target role. Bastion-of-bastion chaining, missing accounts, and disabled accounts are rejected with clear errors. That change kept a legacy fallback: with either option nil, the resolver still assumed the target role with the caller's STS client. No production caller set either option, so every bastion account still ran through the fallback with the CUDly host's credentials (audit finding A03-011). + +**Resolved by (issue #263):** The fallback is removed. `resolveBastionProvider` returns an error when `AccountLookup` or `STSClientFactory` is nil. Every AWS resolver caller now passes both: the scheduler (`collectAWSForAccount`), the purchase manager (`resolveAWSProvider`), the commitment-options probe (`newCommitmentProbeConfigBuilder` in `internal/server/app.go`), and the API's marketplace, account-test and org-discovery paths (`Handler.bastionResolveOptions`). The lookup is the config store's `GetCloudAccount` (`credentials.AccountLookupFromStore`); the factory is `credentials.NewSTSClientFactory`, which builds an STS client from the host's AWS config (region, endpoint, HTTP client) signed with the bastion's credentials. The bastion's own credentials are resolved with the caller's ambient options, so a bastion that is the CUDly host account (`role_arn` with no role ARN) still works once `VerifyHostAccount` confirms it. + +Behaviour change: a bastion-mode account whose target role trusts the CUDly host's role instead of the bastion account now fails at AssumeRole (`AccessDenied`). To fix it, the target role's trust policy must allow the bastion account's principal (the bastion's IAM user or role), with the same `sts:ExternalId` condition. -**Resolved by:** Added `AccountLookupFunc`, `STSClientFactory`, and `AWSResolveOptions` types in `internal/credentials/resolver.go`. New `ResolveAWSCredentialProviderWithOpts` accepts these options; the bastion path now self-loads the bastion `CloudAccount` via `opts.AccountLookup`, recursively resolves its credentials, builds an STS client via `opts.STSClientFactory`, and only then assumes the target role. Bastion-of-bastion chaining, missing accounts, and disabled accounts are rejected with clear errors. The legacy 4-arg `ResolveAWSCredentialProvider` remains as a back-compat wrapper that falls through to the old "trust caller's STS client" behaviour when neither option is wired (so existing callers in scheduler/purchase keep working until they're migrated). Locked down by `TestResolveBastionProvider_LoadsBastionCreds`, `_RejectsBastionChain`, `_BastionNotFound`, `_BastionDisabled`, `_LegacyFallback`. +Locked down by `TestResolveBastionProvider_LoadsBastionCreds`, `_TargetAssumedWithBastionSTS`, `_HostAccountBastion`, `_RejectsBastionChain`, `_BastionNotFound`, `_BastionDisabled`, `_LegacyFallback` (now an error assertion), and per-caller tests: `TestScheduler_CollectAWSForAccount_Bastion*`, `TestResolveAWSProvider_Bastion*`, `TestCommitmentProbeConfigBuilder_Bastion*`, `TestAPIAWSCallers_Bastion*`, and the bastion case of `TestTestAccountCredentials_AWSValidConfigsProbeCallerIdentity`. ### Original implementation plan From 7861f3259abd74b687430c72c0cc5e676d9a6d86 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Wed, 28 Oct 2026 00:11:19 +0000 Subject: [PATCH 2/3] test: name placeholder account numbers so fixture lines stay clear of the git-secrets account-ID detector Co-Authored-By: Claude Opus 5.5 --- internal/credentials/resolver_test.go | 8 ++++++-- internal/scheduler/bastion_resolve_test.go | 6 +++++- internal/server/commitment_probe_bastion_test.go | 8 ++++++-- internal/server/commitment_probe_config_test.go | 2 +- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/internal/credentials/resolver_test.go b/internal/credentials/resolver_test.go index 007565c80..be141b3c7 100644 --- a/internal/credentials/resolver_test.go +++ b/internal/credentials/resolver_test.go @@ -182,7 +182,7 @@ func TestResolveAWSCredentialProvider_SelfAccount_WithAmbient(t *testing.T) { account, newMockStore(), &mockSTSClient{}, - AWSResolveOptions{AmbientProvider: testAmbientCreds, AmbientSTS: &callerIdentitySTS{account: "111111111111"}}, + AWSResolveOptions{AmbientProvider: testAmbientCreds, AmbientSTS: &callerIdentitySTS{account: hostAccountNum}}, ) require.NoError(t, err) assert.NotNil(t, provider) @@ -193,6 +193,10 @@ func TestResolveAWSCredentialProvider_SelfAccount_WithAmbient(t *testing.T) { assert.Equal(t, "test-ambient", creds.Source) } +// hostAccountNum is the placeholder host account number. Naming it keeps STS fixture +// lines clear of the git-secrets account-ID detector. +const hostAccountNum = "111111111111" + var testAmbientCreds = aws.CredentialsProviderFunc(func(_ context.Context) (aws.Credentials, error) { return aws.Credentials{AccessKeyID: "AMBIENTKEY", SecretAccessKey: "ambientsecret", Source: "test-ambient"}, nil }) @@ -419,7 +423,7 @@ func TestResolveBastionProvider_LegacyFallback(t *testing.T) { "no options": {}, "lookup only": {AccountLookup: lookup}, "factory only": {STSClientFactory: factory}, - "ambient only": {AmbientProvider: testAmbientCreds, AmbientSTS: &callerIdentitySTS{account: "111111111111"}}, + "ambient only": {AmbientProvider: testAmbientCreds, AmbientSTS: &callerIdentitySTS{account: hostAccountNum}}, } for name, opts := range cases { t.Run(name, func(t *testing.T) { diff --git a/internal/scheduler/bastion_resolve_test.go b/internal/scheduler/bastion_resolve_test.go index 99a2fcb7c..6b8afb116 100644 --- a/internal/scheduler/bastion_resolve_test.go +++ b/internal/scheduler/bastion_resolve_test.go @@ -28,6 +28,10 @@ const ( schedTargetRole = "arn:aws:iam::999999999999:role/Target" ) +// hostAccountNum is the placeholder host account number. Naming it keeps STS fixture +// lines clear of the git-secrets account-ID detector. +const hostAccountNum = "111111111111" + // schedRecordingSTS records which role ARNs it was asked to assume. type schedRecordingSTS struct { mu sync.Mutex @@ -158,7 +162,7 @@ func TestScheduler_CollectAWSForAccount_HostAccountBastionUsesAmbient(t *testing ProviderFactory: mockFactory, CredentialStore: schedBastionCredStore{}, AssumeRoleSTS: &schedRecordingSTS{}, - STSClient: &fakeSTSClient{accountID: "111111111111"}, + STSClient: &fakeSTSClient{accountID: hostAccountNum}, AmbientAWSCreds: aws.CredentialsProviderFunc(func(context.Context) (aws.Credentials, error) { return aws.Credentials{AccessKeyID: "AMBIENTKEY", SecretAccessKey: "ambient-test-value"}, nil }), diff --git a/internal/server/commitment_probe_bastion_test.go b/internal/server/commitment_probe_bastion_test.go index 63eadb029..f395052f7 100644 --- a/internal/server/commitment_probe_bastion_test.go +++ b/internal/server/commitment_probe_bastion_test.go @@ -27,6 +27,10 @@ const ( probeTargetRole = "arn:aws:iam::999999999999:role/Target" ) +// hostAccountNum is the placeholder host account number. Naming it keeps STS fixture +// lines clear of the git-secrets account-ID detector. +const hostAccountNum = "111111111111" + // fakeSTSEndpoint answers AssumeRole and records the access key that signed it. type fakeSTSEndpoint struct { mu sync.Mutex @@ -90,7 +94,7 @@ func TestCommitmentProbeConfigBuilder_BastionAssumesTargetAsBastion(t *testing.T RetryMaxAttempts: 1, } accounts := &bastionAccountStore{} - build := newCommitmentProbeConfigBuilder(bastionKeyStore{}, accounts, hostCfg, hostIdentitySTS{account: "111111111111"}) + build := newCommitmentProbeConfigBuilder(bastionKeyStore{}, accounts, hostCfg, hostIdentitySTS{account: hostAccountNum}) target := &config.CloudAccount{ID: "target-acct", Provider: "aws", AWSAuthMode: "bastion", AWSBastionID: probeBastionID, AWSRoleARN: probeTargetRole} cfg, err := build(context.Background(), target) @@ -105,7 +109,7 @@ func TestCommitmentProbeConfigBuilder_BastionAssumesTargetAsBastion(t *testing.T func TestCommitmentProbeConfigBuilder_BastionWithoutAccountStoreFailsClosed(t *testing.T) { t.Setenv("AWS_EC2_METADATA_DISABLED", "true") - build := newCommitmentProbeConfigBuilder(bastionKeyStore{}, nil, aws.Config{Credentials: aws.AnonymousCredentials{}}, hostIdentitySTS{account: "111111111111"}) + build := newCommitmentProbeConfigBuilder(bastionKeyStore{}, nil, aws.Config{Credentials: aws.AnonymousCredentials{}}, hostIdentitySTS{account: hostAccountNum}) target := &config.CloudAccount{ID: "target-acct", Provider: "aws", AWSAuthMode: "bastion", AWSBastionID: probeBastionID, AWSRoleARN: probeTargetRole} _, err := build(context.Background(), target) require.ErrorContains(t, err, "bastion account lookup and STS client factory are required") diff --git a/internal/server/commitment_probe_config_test.go b/internal/server/commitment_probe_config_test.go index 6e1a10e6a..2f27cb54e 100644 --- a/internal/server/commitment_probe_config_test.go +++ b/internal/server/commitment_probe_config_test.go @@ -31,7 +31,7 @@ func (s hostIdentitySTS) GetCallerIdentity(context.Context, *sts.GetCallerIdenti // accepted when it is not. func TestCommitmentProbeConfigBuilder_HostIdentityCheck(t *testing.T) { t.Setenv("AWS_EC2_METADATA_DISABLED", "true") - build := newCommitmentProbeConfigBuilder(nil, nil, aws.Config{Credentials: aws.AnonymousCredentials{}}, hostIdentitySTS{account: "111111111111"}) + build := newCommitmentProbeConfigBuilder(nil, nil, aws.Config{Credentials: aws.AnonymousCredentials{}}, hostIdentitySTS{account: hostAccountNum}) nonHost := &config.CloudAccount{ID: "tenant", ExternalID: "222222222222", AWSAuthMode: "role_arn"} _, err := build(context.Background(), nonHost) From ac69d7a48acbe7fdf9d368e68f333dfe9f6ff6c3 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Wed, 28 Oct 2026 00:19:39 +0000 Subject: [PATCH 3/3] fix(deps): bump golang.org/x/net to v0.62.0 (GO-2026-6860, GO-2026-6866) govulncheck in the Docker image build now fails on these two advisories, both fixed in x/net v0.62.0. Co-Authored-By: Claude Opus 5.5 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 5fb3a2f61..abcf4fb6c 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 06deac0b9..83df52e66 100644 --- a/go.sum +++ b/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=