Skip to content

Commit eab06cf

Browse files
carabasdanielactions-user
authored andcommitted
aws-source: fix kms-grant issue with service principal (#3831)
Fix for aws source errors seen in pod logs: ``` {"error":"arn: invalid prefix","input":"rds.amazonaws.com","level":"error","msg":"Error parsing principal ARN","scope":"944651592624.eu-west-2","severity":"error","time":"2026-02-09T15:09:42Z"} {"error":"arn: invalid prefix","input":"ec2.eu-west-2.amazonaws.com","level":"error","msg":"Error parsing principal ARN","scope":"944651592624.eu-west-2","severity":"error","time":"2026-02-09T15:09:42Z"} ``` <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: change is isolated to KMS grant link generation and a shared partition-suffix helper, with added unit tests covering the new behavior and no auth/data-path modifications. > > **Overview** > Fixes `kms-grant` discovery failures caused by AWS service principals (e.g. `rds.amazonaws.com`) being treated like ARNs. > > This introduces a shared `awsPartitionDNSSuffixes` map plus `GetAllAWSPartitionDNSSuffixes()` to detect DNS-style service principals across partitions, and updates `grantOutputMapper` to *silently skip* those principals (and downgrade ARN-parse logs from error to warn) so only linkable IAM/KMS items are emitted. Adds unit coverage for service-principal detection and for ensuring service principals don’t generate linked-item queries. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 14d07808f0a26a089203aee10f581104e2141ffb. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> GitOrigin-RevId: bd8474862b2fb7344b2bc3a787c0c9fa693144a3
1 parent e6e9557 commit eab06cf

3 files changed

Lines changed: 194 additions & 16 deletions

File tree

aws-source/adapters/adapterhelpers_util.go

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -176,23 +176,43 @@ func ParseARN(arnString string) (*ARN, error) {
176176
}, nil
177177
}
178178

179+
// awsPartitionDNSSuffixes maps AWS partition names to their DNS suffixes.
180+
// This is the single source of truth for all AWS partition DNS suffixes.
181+
// See: https://docs.aws.amazon.com/general/latest/gr/rande.html
182+
var awsPartitionDNSSuffixes = map[string]string{
183+
"aws": "amazonaws.com",
184+
"aws-us-gov": "amazonaws.com",
185+
"aws-cn": "amazonaws.com.cn",
186+
"aws-iso": "c2s.ic.gov",
187+
"aws-iso-b": "sc2s.sgov.gov",
188+
"aws-eu": "amazonaws.eu",
189+
}
190+
179191
// GetPartitionDNSSuffix returns the DNS suffix for a given AWS partition.
180192
// This is used to construct service URLs that work across all AWS partitions.
181193
func GetPartitionDNSSuffix(partition string) string {
182-
switch partition {
183-
case "aws-cn":
184-
return "amazonaws.com.cn"
185-
case "aws-iso":
186-
return "c2s.ic.gov"
187-
case "aws-iso-b":
188-
return "sc2s.sgov.gov"
189-
case "aws-eu":
190-
return "amazonaws.eu"
191-
case "aws", "aws-us-gov":
192-
return "amazonaws.com"
193-
default:
194-
return "amazonaws.com" // Default to commercial partition
194+
if suffix, ok := awsPartitionDNSSuffixes[partition]; ok {
195+
return suffix
196+
}
197+
return "amazonaws.com" // Default to commercial partition
198+
}
199+
200+
// GetAllAWSPartitionDNSSuffixes returns all known AWS partition DNS suffixes.
201+
// This is useful for checking if a string (like a service principal) belongs
202+
// to any AWS partition.
203+
func GetAllAWSPartitionDNSSuffixes() []string {
204+
// Use a map to deduplicate (aws and aws-us-gov share the same suffix)
205+
seen := make(map[string]bool)
206+
suffixes := make([]string, 0, len(awsPartitionDNSSuffixes))
207+
208+
for _, suffix := range awsPartitionDNSSuffixes {
209+
if !seen[suffix] {
210+
seen[suffix] = true
211+
suffixes = append(suffixes, suffix)
212+
}
195213
}
214+
215+
return suffixes
196216
}
197217

198218
// WrapAWSError Wraps an AWS error in the appropriate SDP error

aws-source/adapters/kms-grant.go

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,15 +97,27 @@ func grantOutputMapper(ctx context.Context, _ *kms.Client, scope string, _ *kms.
9797
9898
dynamodb.us-west-2.amazonaws.com
9999
100-
The following are not supported
100+
The following are not supported (we skip them silently):
101101
- arn:aws:iam::account:root
102102
- arn:aws:sts::account:federated-user/user-name
103103
- arn:aws:sts::account:assumed-role/role-name/role-session-name
104104
- arn:aws:sts::account:self
105-
- dynamodb.us-west-2.amazonaws.com => this will cause an error in ARN parsing
105+
- Service principals like dynamodb.us-west-2.amazonaws.com (not ARNs, not linkable)
106106
*/
107107

108108
for _, principal := range principals {
109+
// Skip AWS service principals (e.g. "rds.eu-west-2.amazonaws.com",
110+
// "dynamodb.us-west-2.amazonaws.com"). These are DNS-style identifiers
111+
// for AWS services, not ARNs, and are not linkable to other items.
112+
if isAWSServicePrincipal(principal) {
113+
log.WithFields(log.Fields{
114+
"input": principal,
115+
"scope": scope,
116+
}).Debug("Skipping AWS service principal (not linkable)")
117+
118+
continue
119+
}
120+
109121
lIQ := &sdp.LinkedItemQuery{
110122
Query: &sdp.Query{
111123
Method: sdp.QueryMethod_GET,
@@ -126,7 +138,7 @@ func grantOutputMapper(ctx context.Context, _ *kms.Client, scope string, _ *kms.
126138
"error": errA,
127139
"input": principal,
128140
"scope": scope,
129-
}).Error("Error parsing principal ARN")
141+
}).Warn("Error parsing principal ARN")
130142

131143
continue
132144
}
@@ -237,3 +249,24 @@ func iamSourceAndQuery(resource string) (string, string) {
237249

238250
return adapter, query // user, user-name-with-path
239251
}
252+
253+
// isAWSServicePrincipal returns true if the principal is an AWS service
254+
// principal (e.g. "rds.eu-west-2.amazonaws.com", "dynamodb.us-west-2.amazonaws.com").
255+
// These are DNS-style identifiers used by AWS services to assume roles or access
256+
// resources, and are not ARNs.
257+
func isAWSServicePrincipal(principal string) bool {
258+
// Service principals don't start with "arn:" and end with a partition-specific
259+
// DNS suffix.
260+
if strings.HasPrefix(principal, "arn:") {
261+
return false
262+
}
263+
264+
// Check all AWS partition DNS suffixes using the shared list
265+
for _, suffix := range GetAllAWSPartitionDNSSuffixes() {
266+
if strings.HasSuffix(principal, "."+suffix) {
267+
return true
268+
}
269+
}
270+
271+
return false
272+
}

aws-source/adapters/kms-grant_test.go

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,82 @@ An example list grants response:
4444
}
4545
*/
4646

47+
func TestIsAWSServicePrincipal(t *testing.T) {
48+
t.Parallel()
49+
50+
tests := []struct {
51+
name string
52+
principal string
53+
expected bool
54+
}{
55+
{
56+
name: "RDS service principal",
57+
principal: "rds.eu-west-2.amazonaws.com",
58+
expected: true,
59+
},
60+
{
61+
name: "DynamoDB service principal",
62+
principal: "dynamodb.us-west-2.amazonaws.com",
63+
expected: true,
64+
},
65+
{
66+
name: "EC2 service principal",
67+
principal: "ec2.amazonaws.com",
68+
expected: true,
69+
},
70+
{
71+
name: "China region service principal (aws-cn)",
72+
principal: "rds.cn-north-1.amazonaws.com.cn",
73+
expected: true,
74+
},
75+
{
76+
name: "EU partition service principal (aws-eu)",
77+
principal: "rds.eu-central-1.amazonaws.eu",
78+
expected: true,
79+
},
80+
{
81+
name: "ISO partition service principal (aws-iso)",
82+
principal: "rds.us-iso-east-1.c2s.ic.gov",
83+
expected: true,
84+
},
85+
{
86+
name: "ISO-B partition service principal (aws-iso-b)",
87+
principal: "rds.us-isob-east-1.sc2s.sgov.gov",
88+
expected: true,
89+
},
90+
{
91+
name: "IAM role ARN",
92+
principal: "arn:aws:iam::123456789012:role/MyRole",
93+
expected: false,
94+
},
95+
{
96+
name: "IAM user ARN",
97+
principal: "arn:aws:iam::123456789012:user/MyUser",
98+
expected: false,
99+
},
100+
{
101+
name: "Account root ARN",
102+
principal: "arn:aws:iam::123456789012:root",
103+
expected: false,
104+
},
105+
{
106+
name: "Random string",
107+
principal: "not-a-principal",
108+
expected: false,
109+
},
110+
}
111+
112+
for _, tt := range tests {
113+
t.Run(tt.name, func(t *testing.T) {
114+
t.Parallel()
115+
result := isAWSServicePrincipal(tt.principal)
116+
if result != tt.expected {
117+
t.Errorf("isAWSServicePrincipal(%q) = %v, expected %v", tt.principal, result, tt.expected)
118+
}
119+
})
120+
}
121+
}
122+
47123
func TestGrantOutputMapper(t *testing.T) {
48124
output := &kms.ListGrantsOutput{
49125
Grants: []types.GrantListEntry{
@@ -109,6 +185,55 @@ func TestGrantOutputMapper(t *testing.T) {
109185
tests.Execute(t, item)
110186
}
111187

188+
func TestGrantOutputMapperWithServicePrincipal(t *testing.T) {
189+
// Test that service principals (like dynamodb.us-west-2.amazonaws.com) are
190+
// properly skipped and don't cause errors or generate linked item queries
191+
output := &kms.ListGrantsOutput{
192+
Grants: []types.GrantListEntry{
193+
{
194+
Constraints: &types.GrantConstraints{
195+
EncryptionContextSubset: map[string]string{
196+
"aws:dynamodb:subscriberId": "123456789012",
197+
"aws:dynamodb:tableName": "Services",
198+
},
199+
},
200+
IssuingAccount: PtrString("arn:aws:iam::123456789012:root"),
201+
Name: PtrString("8276b9a6-6cf0-46f1-b2f0-7993a7f8c89a"),
202+
Operations: []types.GrantOperation{"Decrypt", "Encrypt"},
203+
GrantId: PtrString("1667b97d27cf748cf05b487217dd4179526c949d14fb3903858e25193253fe59"),
204+
KeyId: PtrString("arn:aws:kms:us-west-2:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab"),
205+
// These are service principals, not ARNs - they should be skipped
206+
RetiringPrincipal: PtrString("dynamodb.us-west-2.amazonaws.com"),
207+
GranteePrincipal: PtrString("rds.eu-west-2.amazonaws.com"),
208+
CreationDate: PtrTime(time.Now()),
209+
},
210+
},
211+
}
212+
213+
items, err := grantOutputMapper(context.Background(), nil, "foo", nil, output)
214+
if err != nil {
215+
t.Fatal(err)
216+
}
217+
218+
if len(items) != 1 {
219+
t.Fatalf("expected 1 item, got %v", len(items))
220+
}
221+
222+
item := items[0]
223+
224+
// Should only have the kms-key link, not the service principals
225+
if len(item.GetLinkedItemQueries()) != 1 {
226+
t.Errorf("expected 1 linked item query (kms-key only), got %v", len(item.GetLinkedItemQueries()))
227+
for i, liq := range item.GetLinkedItemQueries() {
228+
t.Logf(" [%d] type=%s query=%s", i, liq.GetQuery().GetType(), liq.GetQuery().GetQuery())
229+
}
230+
}
231+
232+
if item.GetLinkedItemQueries()[0].GetQuery().GetType() != "kms-key" {
233+
t.Errorf("expected linked item query to be kms-key, got %s", item.GetLinkedItemQueries()[0].GetQuery().GetType())
234+
}
235+
}
236+
112237
func TestNewKMSGrantAdapter(t *testing.T) {
113238
config, account, region := GetAutoConfig(t)
114239
client := kms.NewFromConfig(config)

0 commit comments

Comments
 (0)