Skip to content

Conversation

@alexxa
Copy link
Collaborator

@alexxa alexxa commented Feb 2, 2026

No description provided.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • Before merging, consider gating the AWS config dump behind a debug flag or environment variable so it doesn’t run (and clutter logs) in normal CI runs.
  • Even though access keys are filtered, please double-check whether other sensitive values (e.g., account IDs, ARNs, or role names) should also be masked or omitted from the config output.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Before merging, consider gating the AWS config dump behind a debug flag or environment variable so it doesn’t run (and clutter logs) in normal CI runs.
- Even though access keys are filtered, please double-check whether other sensitive values (e.g., account IDs, ARNs, or role names) should also be masked or omitted from the config output.

## Individual Comments

### Comment 1
<location> `ci/aws.sh:166-167` </location>
<code_context>
 trap 'sudo pkill -P ${WORKER_JOURNAL_PID}' EXIT

+# Debugging
+greenprint "🔍 Inspecting AWS Config (Masking Credentials)"
+cat "$AWS_CONFIG" | grep -vE "accessKeyID|secretAccessKey" 
+
 # Start the compose and upload to AWS.
</code_context>

<issue_to_address>
**🚨 issue (security):** Current masking only covers two keys and may still leak other sensitive fields

`grep -vE "accessKeyID|secretAccessKey"` assumes those are the only sensitive fields. If additional secrets (e.g. `sessionToken`, `aws_session_token`, account IDs) appear in the config, they’ll be logged. Prefer printing only a minimal set of known-safe fields (e.g. profile names, regions) or explicitly grepping for allowed keys instead of masking specific secret names.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +166 to +167
greenprint "🔍 Inspecting AWS Config (Masking Credentials)"
cat "$AWS_CONFIG" | grep -vE "accessKeyID|secretAccessKey"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Current masking only covers two keys and may still leak other sensitive fields

grep -vE "accessKeyID|secretAccessKey" assumes those are the only sensitive fields. If additional secrets (e.g. sessionToken, aws_session_token, account IDs) appear in the config, they’ll be logged. Prefer printing only a minimal set of known-safe fields (e.g. profile names, regions) or explicitly grepping for allowed keys instead of masking specific secret names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant