diff --git a/go.mod b/go.mod index a799c3a16..1e223cdab 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,7 @@ require ( github.com/jackc/pgx/v5 v5.10.0 github.com/klauspost/cpuid/v2 v2.4.0 github.com/miekg/dns v1.1.73 - github.com/mulgadc/bluebottle v1.18.0 + github.com/mulgadc/bluebottle v1.18.1-0.20260826044308-1a1e1242c217 github.com/mulgadc/northstar v1.18.0 github.com/mulgadc/predastore v1.18.0 github.com/mulgadc/viperblock v1.18.0 diff --git a/go.sum b/go.sum index 5e42c2109..8a806ae11 100644 --- a/go.sum +++ b/go.sum @@ -1329,6 +1329,8 @@ github.com/muesli/termenv v0.16.0 h1:S5AlUN9dENB57rsbnkPyfdGuWIlkmzJjbFf0Tf5FWUc github.com/muesli/termenv v0.16.0/go.mod h1:ZRfOIKPFDYQoDFF4Olj7/QJbW60Ol/kL1pU3VfY/Cnk= github.com/mulgadc/bluebottle v1.18.0 h1:aNrogJBz59gJAxCwpERGBC5bGXkSoUCCCBbNvnamuRU= github.com/mulgadc/bluebottle v1.18.0/go.mod h1:KkUyzqfKxyI8PvKR27EVqJnVm4BSbajl1Q/u+PQxAYw= +github.com/mulgadc/bluebottle v1.18.1-0.20260826044308-1a1e1242c217 h1:zRKEFadLkUQxNCK2L3MYSQxRmu4VEEHWTxMwh04Qmeo= +github.com/mulgadc/bluebottle v1.18.1-0.20260826044308-1a1e1242c217/go.mod h1:KkUyzqfKxyI8PvKR27EVqJnVm4BSbajl1Q/u+PQxAYw= github.com/mulgadc/northstar v1.18.0 h1:ksRjIKE6qtrw/ZoYaJh8JRBh+xdnuDw8FsmFLFCiXbg= github.com/mulgadc/northstar v1.18.0/go.mod h1:z6iol77R1vsIQyIBc1JQboUPhSJ6eG4bipzfbZZbLEM= github.com/mulgadc/predastore v1.18.0 h1:RW1Qw1SM2iYxqi6HaHLv5Qb1h1vhZSO3x6pOhPAb9mw= diff --git a/spinifex/handlers/iam/roles.go b/spinifex/handlers/iam/roles.go index 77f11cc86..a74618ad4 100644 --- a/spinifex/handlers/iam/roles.go +++ b/spinifex/handlers/iam/roles.go @@ -15,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/service/iam" "github.com/nats-io/nats.go/jetstream" + iamarn "github.com/mulgadc/bluebottle/pkg/auth" "github.com/mulgadc/spinifex/spinifex/arn" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/mulgadc/spinifex/spinifex/kvutil" @@ -536,17 +537,17 @@ func (s *IAMServiceImpl) GetRolePolicies(accountID, roleName string) ([]PolicyDo // stored and round-tripped opaquely so stock EKS tooling that attaches them // works without a backing policy document. func isAWSManagedPolicyARN(arn string) bool { - return strings.HasPrefix(arn, "arn:aws:iam::aws:policy/") + return iamarn.IsAWSManagedPolicyARN(arn) } // managedPolicyNameFromARN returns the final path segment of an AWS-managed // policy ARN, e.g. .../service-role/AmazonEKS_CNI_Policy -> AmazonEKS_CNI_Policy. +// Display-only, so an unparseable ARN falls back to itself rather than erroring. func managedPolicyNameFromARN(arn string) string { - name := strings.TrimPrefix(arn, "arn:aws:iam::aws:policy/") - if i := strings.LastIndex(name, "/"); i >= 0 { - name = name[i+1:] + if _, name, err := iamarn.ParsePolicyARN(arn); err == nil { + return name } - return name + return arn } func (s *IAMServiceImpl) getRole(ctx context.Context, accountID, roleName string) (*Role, error) { diff --git a/spinifex/handlers/iam/roles_test.go b/spinifex/handlers/iam/roles_test.go index a1ad16f1a..78053a391 100644 --- a/spinifex/handlers/iam/roles_test.go +++ b/spinifex/handlers/iam/roles_test.go @@ -554,6 +554,24 @@ func TestAttachRolePolicy_AWSManagedPolicyNotSeeded(t *testing.T) { assert.Empty(t, out.AttachedPolicies) } +func TestAttachRolePolicy_MalformedAWSManagedPolicyRejected(t *testing.T) { + svc := setupTestIAMService(t) + role := createTestRole(t, svc, "malformed-managed-policy") + + _, err := svc.AttachRolePolicy(testAccountID, &iam.AttachRolePolicyInput{ + RoleName: role.RoleName, + PolicyArn: aws.String("arn:aws:iam::aws:policy/"), + }) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorIAMNoSuchEntity) + + out, err := svc.ListAttachedRolePolicies(testAccountID, &iam.ListAttachedRolePoliciesInput{ + RoleName: role.RoleName, + }) + require.NoError(t, err) + assert.Empty(t, out.AttachedPolicies) +} + func TestDetachRolePolicy(t *testing.T) { svc := setupTestIAMService(t) role := createTestRole(t, svc, "detach-role") diff --git a/spinifex/handlers/iam/service_impl.go b/spinifex/handlers/iam/service_impl.go index c1685d27d..f15423fde 100644 --- a/spinifex/handlers/iam/service_impl.go +++ b/spinifex/handlers/iam/service_impl.go @@ -20,6 +20,7 @@ import ( "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" + iamarn "github.com/mulgadc/bluebottle/pkg/auth" "github.com/mulgadc/bluebottle/pkg/iampolicy" "github.com/mulgadc/bluebottle/pkg/masterkey" "github.com/mulgadc/spinifex/spinifex/admin" @@ -1833,13 +1834,12 @@ func (s *IAMServiceImpl) GetUserPolicies(accountID, userName string) ([]PolicyDo // --------------------------------------------------------------------------- func (s *IAMServiceImpl) getPolicyByARN(ctx context.Context, accountID, policyARN string) (*Policy, error) { - parts := strings.SplitN(policyARN, ":policy", 2) - if len(parts) != 2 || parts[1] == "" { - return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) - } - segments := strings.Split(parts[1], "/") - policyName := segments[len(segments)-1] - if policyName == "" { + _, policyName, err := iamarn.ParsePolicyARN(policyARN) + if err != nil { + // NoSuchEntity is the response contract, so log the parse reason — + // otherwise a malformed ARN is indistinguishable from a missing policy. + slog.Debug("getPolicyByARN: unparseable policy ARN", + "accountID", accountID, "policyArn", policyARN, "err", err) return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) } diff --git a/spinifex/handlers/iam/service_impl_test.go b/spinifex/handlers/iam/service_impl_test.go index 99d2390cf..1859e0f8e 100644 --- a/spinifex/handlers/iam/service_impl_test.go +++ b/spinifex/handlers/iam/service_impl_test.go @@ -1072,6 +1072,9 @@ func TestGetPolicy_ARNParsingEdgeCases(t *testing.T) { {"trailing slash only", "arn:aws:iam::" + testAccountID + ":policy/"}, {"empty after policy", "arn:aws:iam::" + testAccountID + ":policy"}, {"no policy segment", "arn:aws:iam::" + testAccountID + ":user/TestARN"}, + {"policy-backup near miss", "arn:aws:iam::" + testAccountID + ":policy-backup/TestARN"}, + {"policyset near miss", "arn:aws:iam::" + testAccountID + ":policyset/TestARN"}, + {"empty account", "arn:aws:iam:::policy/TestARN"}, } for _, tc := range testCases { @@ -1079,7 +1082,8 @@ func TestGetPolicy_ARNParsingEdgeCases(t *testing.T) { _, err := svc.GetPolicy(testAccountID, &iam.GetPolicyInput{ PolicyArn: aws.String(tc.arn), }) - assert.Error(t, err, "ARN %q should fail", tc.arn) + require.Error(t, err, "ARN %q should fail", tc.arn) + assert.Contains(t, err.Error(), awserrors.ErrorIAMNoSuchEntity, "ARN %q", tc.arn) }) } }