feature(aws): add org discovery and account blacklist support#735
feature(aws): add org discovery and account blacklist support#735girish-cheedala wants to merge 3 commits intodevfrom
Conversation
WalkthroughAdds AWS Organizations-based account discovery to the AWS provider: when Changes
Sequence DiagramsequenceDiagram
participant Provider as AWS Provider
participant Session as AWS Session
participant STS as STS (AssumeRole)
participant OrgAPI as AWS Organizations API
Provider->>Provider: Detect OrgDiscoveryRoleArn set
alt OrgDiscoveryRoleArn configured
Provider->>Session: Create session/config for org role
Session->>STS: AssumeRole(OrgDiscoveryRoleArn)
STS-->>Session: Return temporary creds
Provider->>OrgAPI: ListAccounts (using temporary creds)
OrgAPI-->>Provider: Return active accounts
Provider->>Provider: Filter out ExcludeAccountIds
Provider->>Provider: Append discovered accounts to AccountIds
else No OrgDiscoveryRoleArn
Provider->>Provider: Use configured AccountIds only
end
Provider->>Provider: Finalize AccountIds for downstream flows
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/schema/schema.go (1)
243-257:⚠️ Potential issue | 🟠 MajorScalar
exclude_account_idsvalues are silently dropped.After adding
exclude_account_idsto this special-case branch, scalar YAML values (for example,exclude_account_ids: "111,222") no longer get stored because only[]interface{}is handled here.💡 Proposed fix
case "account_ids", "exclude_account_ids", "urls", "services": if valueArr, ok := value.([]interface{}); ok { var strArr []string for _, v := range valueArr { switch v := v.(type) { case string: strArr = append(strArr, v) case int: strArr = append(strArr, fmt.Sprint(v)) default: return fmt.Errorf("unsupported type %T in %s", v, key) } } (*ob)[key] = strings.Join(strArr, ",") + } else { + (*ob)[key] = fmt.Sprint(value) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/schema/schema.go` around lines 243 - 257, The branch that handles keys "account_ids", "exclude_account_ids", "urls", "services" only processes value when it's a []interface{} (valueArr) and silently drops scalar inputs; update the handling in the same switch case so that if value is a scalar (string/int or other simple types) you convert it to a string (using fmt.Sprint) and assign it to (*ob)[key] directly (same comma-joined format used for arrays), while retaining the existing loop for []interface{} to build strArr; reference the variables and symbols key, value, valueArr, strArr and the map pointer ob to locate and implement the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/providers/aws/aws.go`:
- Around line 107-111: The CSV parsing for account IDs in the assignments to
p.AccountIds and p.ExcludeAccountIds can produce empty/invalid entries (e.g.,
trailing commas) because it only strips literal spaces; update the parsing to
split, trim whitespace for each element, filter out empty strings (and
optionally validate the ID format), then dedupe for p.AccountIds and assign the
cleaned list to p.ExcludeAccountIds; locate the code around p.AccountIds =
sliceutil.Dedupe(...) and the block.GetMetadata(excludeAccountIds) branch to
implement the per-item Trim + filter + (optional) validation step before
dedup/assignment.
- Around line 212-218: Change discoverOrgAccounts to accept a context.Context
and propagate the caller's context when invoking it from the block that checks
options.OrgDiscoveryRoleArn; update the call site to pass the ctx (e.g.
provider.discoverOrgAccounts(ctx, sess, config)). Inside discoverOrgAccounts,
replace blocking SDK calls with their context-aware variants (use
sts.AssumeRoleWithContext instead of AssumeRole, and
organizations.ListAccountsPagesWithContext instead of ListAccountsPages) and
ensure any other AWS requests are invoked with the provided ctx so the operation
can be cancelled. Ensure function signature and all internal calls are updated
accordingly.
---
Outside diff comments:
In `@pkg/schema/schema.go`:
- Around line 243-257: The branch that handles keys "account_ids",
"exclude_account_ids", "urls", "services" only processes value when it's a
[]interface{} (valueArr) and silently drops scalar inputs; update the handling
in the same switch case so that if value is a scalar (string/int or other simple
types) you convert it to a string (using fmt.Sprint) and assign it to (*ob)[key]
directly (same comma-joined format used for arrays), while retaining the
existing loop for []interface{} to build strArr; reference the variables and
symbols key, value, valueArr, strArr and the map pointer ob to locate and
implement the fix.
| // Discover accounts from AWS Organizations if configured | ||
| if options.OrgDiscoveryRoleArn != "" { | ||
| if options.AssumeRoleName == "" { | ||
| return nil, errors.New("assume_role_name is required when using org_discovery_role_arn") | ||
| } | ||
| discovered, err := provider.discoverOrgAccounts(sess, config) | ||
| if err != nil { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the overall structure of the aws.go file and locate the discoverOrgAccounts method
wc -l pkg/providers/aws/aws.goRepository: projectdiscovery/cloudlist
Length of output: 98
🏁 Script executed:
# Find the discoverOrgAccounts method and check AWS SDK calls
rg -n 'func.*discoverOrgAccounts|AssumeRole|ListAccountsPages' pkg/providers/aws/aws.go | head -40Repository: projectdiscovery/cloudlist
Length of output: 1559
🏁 Script executed:
# Check if context-aware versions are already used
rg -n 'AssumeRoleWithContext|ListAccountsPagesWithContext|WithContext' pkg/providers/aws/aws.goRepository: projectdiscovery/cloudlist
Length of output: 52
🏁 Script executed:
# Look at the specific lines mentioned (212-218 and 310-348)
sed -n '210,220p' pkg/providers/aws/aws.goRepository: projectdiscovery/cloudlist
Length of output: 479
🏁 Script executed:
# Also check the range 310-348
sed -n '308,350p' pkg/providers/aws/aws.goRepository: projectdiscovery/cloudlist
Length of output: 1555
🏁 Script executed:
# Check the Resources() method signature to see if it receives context
rg -n 'func.*Resources\(' pkg/providers/aws/aws.go | head -5Repository: projectdiscovery/cloudlist
Length of output: 152
🏁 Script executed:
# Look at lines around 212 to see the full Resources() method context
sed -n '200,220p' pkg/providers/aws/aws.goRepository: projectdiscovery/cloudlist
Length of output: 700
🏁 Script executed:
# Check if there are other AWS SDK calls with non-context versions
rg -n 'stsClient\.|orgClient\.|ec2Client\.' pkg/providers/aws/aws.go | grep -E '\.(Assume|List|Describe|Get)\(' | head -20Repository: projectdiscovery/cloudlist
Length of output: 52
🏁 Script executed:
# Check what methods are in the 310-348 range to understand "Also applies to"
sed -n '310,350p' pkg/providers/aws/aws.go | head -50Repository: projectdiscovery/cloudlist
Length of output: 1412
🏁 Script executed:
# Look for other non-context AWS SDK calls throughout the provider
rg -n '\.AssumeRole\(|\.ListAccountsPages\(' pkg/providers/aws/aws.goRepository: projectdiscovery/cloudlist
Length of output: 391
🏁 Script executed:
# Check if there are other long-running API calls that should use context
rg -n 'DescribeInstances|DescribeDBInstances|GetSecurityGroupRules|ListBuckets|DescribeLoadBalancers' pkg/providers/aws/aws.go | head -10Repository: projectdiscovery/cloudlist
Length of output: 304
🏁 Script executed:
# Verify discoverOrgAccounts is only called once
rg -n 'discoverOrgAccounts' pkg/providers/aws/aws.goRepository: projectdiscovery/cloudlist
Length of output: 322
🏁 Script executed:
# Check the New() function signature to understand initialization context
rg -n 'func.*New\(' pkg/providers/aws/aws.go | head -3Repository: projectdiscovery/cloudlist
Length of output: 129
Pass context to org discovery API calls to enable cancellation.
discoverOrgAccounts uses non-context AWS SDK calls (AssumeRole and ListAccountsPages) that can block indefinitely during initialization if AWS stalls. Per coding guidelines, long-running API calls must honor context for cancellation support.
Update discoverOrgAccounts to accept and use context in AWS SDK calls:
Changes required
- discovered, err := provider.discoverOrgAccounts(sess, config)
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+ defer cancel()
+ discovered, err := provider.discoverOrgAccounts(ctx, sess, config)-func (p *Provider) discoverOrgAccounts(sess *session.Session, config *aws.Config) ([]string, error) {
+func (p *Provider) discoverOrgAccounts(ctx context.Context, sess *session.Session, config *aws.Config) ([]string, error) {
stsClient := sts.New(sess)
roleInput := &sts.AssumeRoleInput{
RoleArn: aws.String(p.options.OrgDiscoveryRoleArn),
RoleSessionName: aws.String("cloudlist-org-discovery"),
}
- assumeOut, err := stsClient.AssumeRole(roleInput)
+ assumeOut, err := stsClient.AssumeRoleWithContext(ctx, roleInput)- err = orgClient.ListAccountsPages(&organizations.ListAccountsInput{}, func(page *organizations.ListAccountsOutput, lastPage bool) bool {
+ err = orgClient.ListAccountsPagesWithContext(ctx, &organizations.ListAccountsInput{}, func(page *organizations.ListAccountsOutput, lastPage bool) bool {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/providers/aws/aws.go` around lines 212 - 218, Change discoverOrgAccounts
to accept a context.Context and propagate the caller's context when invoking it
from the block that checks options.OrgDiscoveryRoleArn; update the call site to
pass the ctx (e.g. provider.discoverOrgAccounts(ctx, sess, config)). Inside
discoverOrgAccounts, replace blocking SDK calls with their context-aware
variants (use sts.AssumeRoleWithContext instead of AssumeRole, and
organizations.ListAccountsPagesWithContext instead of ListAccountsPages) and
ensure any other AWS requests are invoked with the provided ctx so the operation
can be cancelled. Ensure function signature and all internal calls are updated
accordingly.
✅ Incremental Review Complete for PR #735 (Commit 7067375)Review Link: #735 What Changed in This CommitThe new commit added 6 lines (lines 142-154 in
This prevents unintended role chaining when incompatible options are specified together. Security AssessmentNew Vulnerabilities: ✅ None found Previous Vulnerabilities Fixed: ❌ 0 out of 4 All 4 security findings from the previous review (d14844d) remain UNFIXED:
ConclusionThe new commit improves configuration safety by preventing incompatible option combinations, but does not address any of the injection vulnerabilities that allow privilege escalation via malicious configuration files. The 4 existing review comments remain valid and should be addressed. |
![neo-by-projectdiscovery-dev[bot]](https://avatars.githubusercontent.com/in/2878468?s=60&v=4){kind=small}
| @@ -99,6 +106,9 @@ func (p *ProviderOptions) ParseOptionBlock(block schema.OptionBlock) error { | |||
| if accountIds, ok := block.GetMetadata(accountIds); ok { | |||
| p.AccountIds = sliceutil.Dedupe(strings.Split(accountIds, ",")) | |||
There was a problem hiding this comment.
🟡 Missing whitespace sanitization in account ID CSV parsing (CWE-20) — CSV-parsed account_ids are split by comma but not trimmed of whitespace. PR description claims whitespace trimming was added, but strings.TrimSpace is not present in the code.
Attack Example
account_ids: "111111111111, 222222222222" and exclude_account_ids: "222222222222" - the second ID has leading space so won't match the exclude filter
Suggested Fix
Apply strings.TrimSpace to each account ID after split: for i, id := range accountIds { accountIds[i] = strings.TrimSpace(id) }
| stsClient := sts.New(sess) | ||
|
|
||
| roleInput := &sts.AssumeRoleInput{ | ||
| RoleArn: aws.String(p.options.OrgDiscoveryRoleArn), |
There was a problem hiding this comment.
🟡 Missing ARN format validation for org_discovery_role_arn (CWE-20) — OrgDiscoveryRoleArn is passed directly to STS AssumeRole without validating it's a properly formatted IAM role ARN. Malformed ARNs could cause unexpected behavior.
Attack Example
org_discovery_role_arn: "arn:aws:s3:::my-bucket" would attempt to assume an S3 bucket as a role, revealing account structure in error messages
Suggested Fix
Validate ARN format matches arn:aws:iam::[0-9]{12}:role/.+ before use
There was a problem hiding this comment.
♻️ Duplicate comments (2)
pkg/providers/aws/aws.go (2)
217-218:⚠️ Potential issue | 🟠 MajorUse context-aware AWS SDK calls in org discovery.
Line 217 invokes org discovery without context, and Line 321/Line 341 use blocking SDK methods (
AssumeRole,ListAccountsPages). This can stall initialization and prevents cancellation/timeout control.💡 Proposed fix
- discovered, err := provider.discoverOrgAccounts(sess, config) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + discovered, err := provider.discoverOrgAccounts(ctx, sess, config)-func (p *Provider) discoverOrgAccounts(sess *session.Session, config *aws.Config) ([]string, error) { +func (p *Provider) discoverOrgAccounts(ctx context.Context, sess *session.Session, config *aws.Config) ([]string, error) { stsClient := sts.New(sess) @@ - assumeOut, err := stsClient.AssumeRole(roleInput) + assumeOut, err := stsClient.AssumeRoleWithContext(ctx, roleInput) @@ - err = orgClient.ListAccountsPages(&organizations.ListAccountsInput{}, func(page *organizations.ListAccountsOutput, lastPage bool) bool { + err = orgClient.ListAccountsPagesWithContext(ctx, &organizations.ListAccountsInput{}, func(page *organizations.ListAccountsOutput, lastPage bool) bool { ... })#!/bin/bash # Verify org discovery uses context-aware AWS SDK methods and context in signature. rg -n 'func \(p \*Provider\) discoverOrgAccounts\(|AssumeRoleWithContext\(|AssumeRole\(|ListAccountsPagesWithContext\(|ListAccountsPages\(' pkg/providers/aws/aws.goAs per coding guidelines, "Always pass and honor context.Context in Resources() and long-running API calls to support cancellation".
Also applies to: 310-348
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/providers/aws/aws.go` around lines 217 - 218, The discoverOrgAccounts flow is using blocking AWS SDK calls without context; update Provider.discoverOrgAccounts to accept a context.Context (ensure Resources() passes it through) and replace synchronous calls: use sts.AssumeRoleWithContext instead of AssumeRole and organizations.ListAccountsPagesWithContext instead of ListAccountsPages (or their v2 equivalents), passing the provided ctx to each AWS call and honoring ctx cancellation in loops and retries; ensure any session or client creation used by discoverOrgAccounts can accept/propagate the context as needed and update all call sites (e.g., where discoverOrgAccounts is invoked) to supply the context.
106-111:⚠️ Potential issue | 🟠 MajorTrim and filter CSV account IDs before dedupe.
strings.Split(..., ",")on raw metadata still keeps whitespace and empty tokens (for example trailing commas), which can produce invalid account IDs and exclusion mismatches.💡 Proposed fix
+ parseCSV := func(raw string) []string { + parts := strings.Split(raw, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.TrimSpace(p) + if p != "" { + out = append(out, p) + } + } + return out + } + if accountIds, ok := block.GetMetadata(accountIds); ok { - p.AccountIds = sliceutil.Dedupe(strings.Split(accountIds, ",")) + p.AccountIds = sliceutil.Dedupe(parseCSV(accountIds)) } if eids, ok := block.GetMetadata(excludeAccountIds); ok { - p.ExcludeAccountIds = sliceutil.Dedupe(strings.Split(eids, ",")) + p.ExcludeAccountIds = sliceutil.Dedupe(parseCSV(eids)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/providers/aws/aws.go` around lines 106 - 111, The metadata parsing currently uses strings.Split(..., ",") then sliceutil.Dedupe which leaves whitespace and empty tokens; update the blocks that set p.AccountIds and p.ExcludeAccountIds (the branches using block.GetMetadata) to split the CSV, map strings.TrimSpace over each token, filter out empty strings, then call sliceutil.Dedupe on the cleaned slice so only valid, trimmed account IDs are stored; target the code around block.GetMetadata, p.AccountIds/p.ExcludeAccountIds, strings.Split and sliceutil.Dedupe when implementing this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/providers/aws/aws.go`:
- Around line 217-218: The discoverOrgAccounts flow is using blocking AWS SDK
calls without context; update Provider.discoverOrgAccounts to accept a
context.Context (ensure Resources() passes it through) and replace synchronous
calls: use sts.AssumeRoleWithContext instead of AssumeRole and
organizations.ListAccountsPagesWithContext instead of ListAccountsPages (or
their v2 equivalents), passing the provided ctx to each AWS call and honoring
ctx cancellation in loops and retries; ensure any session or client creation
used by discoverOrgAccounts can accept/propagate the context as needed and
update all call sites (e.g., where discoverOrgAccounts is invoked) to supply the
context.
- Around line 106-111: The metadata parsing currently uses strings.Split(...,
",") then sliceutil.Dedupe which leaves whitespace and empty tokens; update the
blocks that set p.AccountIds and p.ExcludeAccountIds (the branches using
block.GetMetadata) to split the CSV, map strings.TrimSpace over each token,
filter out empty strings, then call sliceutil.Dedupe on the cleaned slice so
only valid, trimmed account IDs are stored; target the code around
block.GetMetadata, p.AccountIds/p.ExcludeAccountIds, strings.Split and
sliceutil.Dedupe when implementing this change.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
pkg/providers/aws/aws.go (2)
107-112:⚠️ Potential issue | 🟠 MajorNormalize and sanitize CSV account IDs before dedupe.
strings.Splitdirectly on raw CSV still leaves whitespace and empty entries (e.g., trailing commas), which breaks exclusion matching and can produce invalid role ARNs downstream.💡 Suggested fix
if accountIds, ok := block.GetMetadata(accountIds); ok { - p.AccountIds = sliceutil.Dedupe(strings.Split(accountIds, ",")) + parts := strings.Split(accountIds, ",") + parsed := make([]string, 0, len(parts)) + for _, id := range parts { + id = strings.TrimSpace(id) + if id != "" { + parsed = append(parsed, id) + } + } + p.AccountIds = sliceutil.Dedupe(parsed) } if eids, ok := block.GetMetadata(excludeAccountIds); ok { - p.ExcludeAccountIds = sliceutil.Dedupe(strings.Split(eids, ",")) + parts := strings.Split(eids, ",") + parsed := make([]string, 0, len(parts)) + for _, id := range parts { + id = strings.TrimSpace(id) + if id != "" { + parsed = append(parsed, id) + } + } + p.ExcludeAccountIds = sliceutil.Dedupe(parsed) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/providers/aws/aws.go` around lines 107 - 112, The CSV parsing of account IDs currently uses strings.Split and then sliceutil.Dedupe, which leaves whitespace and empty entries; update the parsing for both accountIds and excludeAccountIds (the branches that set p.AccountIds and p.ExcludeAccountIds after block.GetMetadata) to: split the raw string, trim whitespace from each token, discard any empty tokens (including those from trailing commas), then pass the cleaned slice into sliceutil.Dedupe so stored IDs are normalized and free of blanks before further ARN construction or matching.
218-218:⚠️ Potential issue | 🟠 MajorPropagate context through org discovery AWS calls.
Org discovery currently uses blocking non-context SDK calls, so initialization cannot be canceled promptly under AWS/API stalls.
💡 Suggested fix
+import "time" ... - discovered, err := provider.discoverOrgAccounts(sess, config) + discoveryCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + discovered, err := provider.discoverOrgAccounts(discoveryCtx, sess, config)-func (p *Provider) discoverOrgAccounts(sess *session.Session, config *aws.Config) ([]string, error) { +func (p *Provider) discoverOrgAccounts(ctx context.Context, sess *session.Session, config *aws.Config) ([]string, error) ([]string, error) { ... - assumeOut, err := stsClient.AssumeRole(roleInput) + assumeOut, err := stsClient.AssumeRoleWithContext(ctx, roleInput) ... - err = orgClient.ListAccountsPages(&organizations.ListAccountsInput{}, func(page *organizations.ListAccountsOutput, lastPage bool) bool { + err = orgClient.ListAccountsPagesWithContext(ctx, &organizations.ListAccountsInput{}, func(page *organizations.ListAccountsOutput, lastPage bool) bool {#!/bin/bash set -euo pipefail # Verify non-context org discovery calls still exist (should return no matches after fix) rg -nP 'discoverOrgAccounts\(|\.AssumeRole\(|\.ListAccountsPages\(' pkg/providers/aws/aws.go # Verify context-aware variants are used (should return matches after fix) rg -n 'AssumeRoleWithContext|ListAccountsPagesWithContext' pkg/providers/aws/aws.goAs per coding guidelines, "Always pass and honor context.Context in Resources() and long-running API calls to support cancellation".
Also applies to: 316-354
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/providers/aws/aws.go` at line 218, The org discovery code calls blocking AWS SDK methods without a context; update discoverOrgAccounts (and its callers such as the Resources() path that triggers it) to accept and propagate a context.Context parameter, and replace blocking SDK calls (e.g., AssumeRole and ListAccountsPages) with their context-aware variants (AssumeRoleWithContext, ListAccountsPagesWithContext), passing the same ctx through AssumeRoleWithContext, the STS/client calls, and any paginator/page callbacks so cancellation is honored; ensure discoverOrgAccounts returns promptly on ctx cancellation and update any call sites that invoke provider.discoverOrgAccounts to supply the context (also apply the same changes to the other discovery block around the 316-354 region).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/providers/aws/aws.go`:
- Around line 241-243: The current gologger.Info().Msgf call in the AWS provider
(referencing options.AccountIds and options.AssumeRoleName) prints the full list
of account IDs; change it to avoid emitting full account lists at info level by
logging only the count and role name (e.g., "Will assume role X in N accounts")
or by logging a redacted/limited sample at debug level; update the call site
that invokes gologger.Info().Msgf to either use gologger.Debug().Msgf for the
full list or remove the joined options.AccountIds from the message and keep only
len(options.AccountIds), ensuring no raw account IDs are written to info logs.
---
Duplicate comments:
In `@pkg/providers/aws/aws.go`:
- Around line 107-112: The CSV parsing of account IDs currently uses
strings.Split and then sliceutil.Dedupe, which leaves whitespace and empty
entries; update the parsing for both accountIds and excludeAccountIds (the
branches that set p.AccountIds and p.ExcludeAccountIds after block.GetMetadata)
to: split the raw string, trim whitespace from each token, discard any empty
tokens (including those from trailing commas), then pass the cleaned slice into
sliceutil.Dedupe so stored IDs are normalized and free of blanks before further
ARN construction or matching.
- Line 218: The org discovery code calls blocking AWS SDK methods without a
context; update discoverOrgAccounts (and its callers such as the Resources()
path that triggers it) to accept and propagate a context.Context parameter, and
replace blocking SDK calls (e.g., AssumeRole and ListAccountsPages) with their
context-aware variants (AssumeRoleWithContext, ListAccountsPagesWithContext),
passing the same ctx through AssumeRoleWithContext, the STS/client calls, and
any paginator/page callbacks so cancellation is honored; ensure
discoverOrgAccounts returns promptly on ctx cancellation and update any call
sites that invoke provider.discoverOrgAccounts to supply the context (also apply
the same changes to the other discovery block around the 316-354 region).
| if len(options.AccountIds) > 0 && options.AssumeRoleName != "" { | ||
| gologger.Info().Msgf("Will assume role %s in %d accounts: %s", options.AssumeRoleName, len(options.AccountIds), strings.Join(options.AccountIds, ", ")) | ||
| } |
There was a problem hiding this comment.
Avoid logging full account ID lists at info level.
This log line emits all target account IDs, which can leak sensitive organizational topology in shared logs.
💡 Suggested fix
-if len(options.AccountIds) > 0 && options.AssumeRoleName != "" {
- gologger.Info().Msgf("Will assume role %s in %d accounts: %s", options.AssumeRoleName, len(options.AccountIds), strings.Join(options.AccountIds, ", "))
-}
+if len(options.AccountIds) > 0 && options.AssumeRoleName != "" {
+ gologger.Info().Msgf("Will assume role %s in %d accounts", options.AssumeRoleName, len(options.AccountIds))
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if len(options.AccountIds) > 0 && options.AssumeRoleName != "" { | |
| gologger.Info().Msgf("Will assume role %s in %d accounts: %s", options.AssumeRoleName, len(options.AccountIds), strings.Join(options.AccountIds, ", ")) | |
| } | |
| if len(options.AccountIds) > 0 && options.AssumeRoleName != "" { | |
| gologger.Info().Msgf("Will assume role %s in %d accounts", options.AssumeRoleName, len(options.AccountIds)) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/providers/aws/aws.go` around lines 241 - 243, The current
gologger.Info().Msgf call in the AWS provider (referencing options.AccountIds
and options.AssumeRoleName) prints the full list of account IDs; change it to
avoid emitting full account lists at info level by logging only the count and
role name (e.g., "Will assume role X in N accounts") or by logging a
redacted/limited sample at debug level; update the call site that invokes
gologger.Info().Msgf to either use gologger.Debug().Msgf for the full list or
remove the joined options.AccountIds from the message and keep only
len(options.AccountIds), ensuring no raw account IDs are written to info logs.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes