Skip to content

Commit cbe68b3

Browse files
authored
fix: match KMS name filters against bare alias name as well as full alias (#1162)
* fix: match KMS exclusion against full alias so dedicated-test-key survives nuke The KMSCustomerKeys exclusion used '^dedicated-test-key$', but cloud-nuke filters KMS keys on the full alias from ListAliases, which includes the 'alias/' prefix. The anchored pattern never matched, so the key was deleted on every run despite the exclusion being in place since #1025. Confirmed in CloudTrail: cloud-nuke-gha scheduled the key for deletion on 2026-05-26 and again on 2026-07-25, each time within hours of it being manually recreated. * fix: match KMS name filters against bare alias name as well as full alias KMS keys are filtered on the alias returned by ListAliases, which carries an 'alias/' prefix. A pattern written against the name users actually think in, such as '^dedicated-test-key$', silently matched nothing, so keys were deleted despite an exclusion being configured. This is what happened to the shared dedicated-test-key in PhxDevOps: the exclusion added in #1025 never took effect and the key was nuked repeatedly. Name filters now match against both the full alias and the bare name, so either form works. Patterns written against the prefixed form keep working unchanged. Exclusions are now also evaluated across every alias on a key: if any alias matches an exclude rule the key is protected, rather than being deleted because some other alias on it was not excluded. Note for the changelog: an include rule written against a bare name previously matched nothing and now matches, so such a rule will start selecting keys for deletion as its author intended. * test: pin that a matching exclude rule always protects a KMS key Verified by differential testing against the previous implementation across 600 combinations of alias sets and name patterns: with exclude-only configs there is no case where the new logic deletes a key the old logic protected.
1 parent 4e4b735 commit cbe68b3

4 files changed

Lines changed: 181 additions & 13 deletions

File tree

.github/nuke_config.yml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,9 @@ KMSCustomerKeys:
131131
exclude:
132132
names_regex:
133133
# Shared test key referenced by multiple repos (terragrunt, terraform-aws-security, terraform-aws-eks, etc.)
134-
- "^dedicated-test-key$"
134+
# Either "^alias/dedicated-test-key$" or "^dedicated-test-key$" matches. The prefixed
135+
# form is spelled out here so the exclusion holds regardless of cloud-nuke version.
136+
- "^alias/dedicated-test-key$"
135137

136138
VPC:
137139
exclude:

aws/resources/kms_customer_key.go

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package resources
22

33
import (
44
"context"
5+
"strings"
56

67
"github.com/aws/aws-sdk-go-v2/aws"
78
"github.com/aws/aws-sdk-go-v2/service/kms"
@@ -125,22 +126,59 @@ func getKeyAliasesMap(ctx context.Context, client KmsCustomerKeysAPI) (map[strin
125126
return keyAliases, nil
126127
}
127128

128-
// shouldIncludeKey determines if a key should be included for deletion.
129-
func shouldIncludeKey(ctx context.Context, client KmsCustomerKeysAPI, keyId string, aliases []string, cfg config.ResourceType, includeUnaliasedKeys bool) (bool, error) {
130-
// Skip keys without aliases unless explicitly configured to include them
131-
if len(aliases) == 0 && !includeUnaliasedKeys {
132-
return false, nil
133-
}
129+
// kmsAliasPrefix is the prefix AWS returns on every alias from ListAliases.
130+
const kmsAliasPrefix = "alias/"
134131

135-
// Check if any alias matches the name filter
136-
matchedByName := len(aliases) == 0 && includeUnaliasedKeys // Unaliased keys pass if configured
132+
// aliasNameCandidates returns the strings that name filters are matched against for a
133+
// given set of aliases. AWS reports aliases as "alias/my-key", but the "alias/" prefix is
134+
// an API artifact rather than part of the name users think in, so both the full alias and
135+
// the bare name are considered. A pattern matching either form applies to the key.
136+
func aliasNameCandidates(aliases []string) []string {
137+
candidates := make([]string, 0, len(aliases)*2)
137138
for _, alias := range aliases {
138-
if config.ShouldInclude(&alias, cfg.IncludeRule.NamesRegExp, cfg.ExcludeRule.NamesRegExp) {
139-
matchedByName = true
140-
break
139+
candidates = append(candidates, alias)
140+
if bare := strings.TrimPrefix(alias, kmsAliasPrefix); bare != alias {
141+
candidates = append(candidates, bare)
142+
}
143+
}
144+
return candidates
145+
}
146+
147+
// matchesNameFilters reports whether a key passes the configured name filters.
148+
//
149+
// An exclude rule matching any form of any alias protects the whole key, so that a key
150+
// carrying several aliases cannot be deleted just because one of its other aliases was
151+
// not excluded. When include rules are present, at least one form must match.
152+
func matchesNameFilters(aliases []string, cfg config.ResourceType) bool {
153+
candidates := aliasNameCandidates(aliases)
154+
155+
for _, name := range candidates {
156+
if !config.ShouldInclude(&name, nil, cfg.ExcludeRule.NamesRegExp) {
157+
return false
158+
}
159+
}
160+
161+
if len(cfg.IncludeRule.NamesRegExp) == 0 {
162+
return true
163+
}
164+
165+
for _, name := range candidates {
166+
if config.ShouldInclude(&name, cfg.IncludeRule.NamesRegExp, nil) {
167+
return true
141168
}
142169
}
143-
if !matchedByName {
170+
return false
171+
}
172+
173+
// shouldIncludeKey determines if a key should be included for deletion.
174+
func shouldIncludeKey(ctx context.Context, client KmsCustomerKeysAPI, keyId string, aliases []string, cfg config.ResourceType, includeUnaliasedKeys bool) (bool, error) {
175+
// Skip keys without aliases unless explicitly configured to include them. Unaliased
176+
// keys have no name to match, so they bypass name filtering entirely.
177+
if len(aliases) == 0 {
178+
if !includeUnaliasedKeys {
179+
return false, nil
180+
}
181+
} else if !matchesNameFilters(aliases, cfg) {
144182
return false, nil
145183
}
146184

aws/resources/kms_customer_key_test.go

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,27 @@ func TestListKmsCustomerKeys(t *testing.T) {
9292
},
9393
expected: []string{key2},
9494
},
95+
// Anchored patterns written against the full alias must keep working, since
96+
// existing configs rely on the "alias/" prefix being matchable.
97+
"prefixedAnchoredExclusionFilter": {
98+
configObj: config.ResourceType{
99+
ExcludeRule: config.FilterRule{
100+
NamesRegExp: []config.Expression{{RE: *regexp.MustCompile(`^alias/key1$`)}},
101+
},
102+
},
103+
expected: []string{key2},
104+
},
105+
// Anchored patterns written against the bare name must also work. Before this was
106+
// supported, a pattern like "^key1$" silently matched nothing and the key was
107+
// deleted despite an exclusion being configured.
108+
"bareNameAnchoredExclusionFilter": {
109+
configObj: config.ResourceType{
110+
ExcludeRule: config.FilterRule{
111+
NamesRegExp: []config.Expression{{RE: *regexp.MustCompile(`^key1$`)}},
112+
},
113+
},
114+
expected: []string{key2},
115+
},
95116
"nameInclusionFilter": {
96117
configObj: config.ResourceType{
97118
IncludeRule: config.FilterRule{
@@ -100,6 +121,14 @@ func TestListKmsCustomerKeys(t *testing.T) {
100121
},
101122
expected: []string{key1},
102123
},
124+
"bareNameAnchoredInclusionFilter": {
125+
configObj: config.ResourceType{
126+
IncludeRule: config.FilterRule{
127+
NamesRegExp: []config.Expression{{RE: *regexp.MustCompile(`^key1$`)}},
128+
},
129+
},
130+
expected: []string{key1},
131+
},
103132
"timeAfterExclusionFilter": {
104133
configObj: config.ResourceType{
105134
ExcludeRule: config.FilterRule{
@@ -119,6 +148,89 @@ func TestListKmsCustomerKeys(t *testing.T) {
119148
}
120149
}
121150

151+
// TestListKmsCustomerKeys_MultiAliasExclusion covers a key carrying more than one alias.
152+
// An exclusion matching any one of them protects the key, so a key cannot be deleted
153+
// merely because one of its other aliases was not excluded.
154+
func TestListKmsCustomerKeys_MultiAliasExclusion(t *testing.T) {
155+
t.Parallel()
156+
157+
now := time.Now()
158+
protectedKey, otherKey := "protected", "other"
159+
160+
mock := &mockKmsClient{
161+
ListKeysOutput: kms.ListKeysOutput{
162+
Keys: []types.KeyListEntry{
163+
{KeyId: aws.String(protectedKey)},
164+
{KeyId: aws.String(otherKey)},
165+
},
166+
},
167+
ListAliasesOutput: kms.ListAliasesOutput{
168+
Aliases: []types.AliasListEntry{
169+
{AliasName: aws.String("alias/dedicated-test-key"), TargetKeyId: aws.String(protectedKey)},
170+
{AliasName: aws.String("alias/some-other-name"), TargetKeyId: aws.String(protectedKey)},
171+
{AliasName: aws.String("alias/unrelated"), TargetKeyId: aws.String(otherKey)},
172+
},
173+
},
174+
DescribeKeyOutput: map[string]kms.DescribeKeyOutput{
175+
protectedKey: {KeyMetadata: &types.KeyMetadata{
176+
KeyId: aws.String(protectedKey),
177+
KeyManager: types.KeyManagerTypeCustomer,
178+
CreationDate: aws.Time(now),
179+
}},
180+
otherKey: {KeyMetadata: &types.KeyMetadata{
181+
KeyId: aws.String(otherKey),
182+
KeyManager: types.KeyManagerTypeCustomer,
183+
CreationDate: aws.Time(now),
184+
}},
185+
},
186+
}
187+
188+
cfg := config.ResourceType{
189+
ExcludeRule: config.FilterRule{
190+
NamesRegExp: []config.Expression{{RE: *regexp.MustCompile(`^dedicated-test-key$`)}},
191+
},
192+
}
193+
194+
names, err := listKmsCustomerKeys(context.Background(), mock, cfg, false)
195+
require.NoError(t, err)
196+
require.Equal(t, []string{otherKey}, aws.ToStringSlice(names))
197+
}
198+
199+
// TestMatchesNameFilters_ExclusionIsAlwaysHonored pins the safety property that matters
200+
// most for a deletion tool: whenever an exclude rule matches any form of any alias on a
201+
// key, the key is protected. Regressing this silently deletes keys operators believe are
202+
// safe, which is exactly how dedicated-test-key was lost.
203+
func TestMatchesNameFilters_ExclusionIsAlwaysHonored(t *testing.T) {
204+
t.Parallel()
205+
206+
tests := []struct {
207+
name string
208+
aliases []string
209+
exclude string
210+
}{
211+
{"bare name, anchored", []string{"alias/dedicated-test-key"}, `^dedicated-test-key$`},
212+
{"full alias, anchored", []string{"alias/dedicated-test-key"}, `^alias/dedicated-test-key$`},
213+
{"unanchored substring", []string{"alias/dedicated-test-key"}, `dedicated-test-key`},
214+
{"prefix wildcard", []string{"alias/dedicated-test-key"}, `^alias/`},
215+
{"match everything", []string{"alias/dedicated-test-key"}, `.*`},
216+
{"excluded alias listed first", []string{"alias/dedicated-test-key", "alias/other"}, `^dedicated-test-key$`},
217+
{"excluded alias listed second", []string{"alias/other", "alias/dedicated-test-key"}, `^dedicated-test-key$`},
218+
{"excluded alias among several", []string{"alias/a", "alias/b", "alias/dedicated-test-key"}, `^dedicated-test-key$`},
219+
}
220+
221+
for _, tc := range tests {
222+
t.Run(tc.name, func(t *testing.T) {
223+
cfg := config.ResourceType{
224+
ExcludeRule: config.FilterRule{
225+
NamesRegExp: []config.Expression{{RE: *regexp.MustCompile(tc.exclude)}},
226+
},
227+
}
228+
require.False(t, matchesNameFilters(tc.aliases, cfg),
229+
"exclude %q must protect a key with aliases %v", tc.exclude, tc.aliases)
230+
})
231+
}
232+
}
233+
122234
func TestListKmsCustomerKeys_IncludeUnaliasedKeys(t *testing.T) {
123235
t.Parallel()
124236

docs/configuration.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,22 @@ S3:
4040
4141
Filtering is **commutative** — include and exclude filters can be applied in any order with the same result. In the example above, buckets matching `^alb-.*-access-logs$` are included unless they also match `public` or `prod`.
4242

43+
Patterns are not implicitly anchored: they match anywhere in the name unless you add `^` and `$` yourself.
44+
45+
#### KMS key names
46+
47+
KMS customer-managed keys are matched by alias. AWS returns aliases with an `alias/` prefix, so both the full alias and the bare name are matched, and a pattern matching either form applies. These are equivalent:
48+
49+
```yaml
50+
KMSCustomerKeys:
51+
exclude:
52+
names_regex:
53+
- ^alias/my-key$
54+
- ^my-key$
55+
```
56+
57+
When a key carries several aliases, an exclusion matching any one of them protects the key.
58+
4359
### time_after / time_before
4460

4561
Filter resources by creation time.

0 commit comments

Comments
 (0)