Skip to content

JS: Improved modeling of aws-sdk #19364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 28, 2025
Merged

JS: Improved modeling of aws-sdk #19364

merged 7 commits into from
Apr 28, 2025

Conversation

Napalys
Copy link
Contributor

@Napalys Napalys commented Apr 24, 2025

This pull request adds support for detecting specific patterns in the use of aws-sdk related to hardcoded credentials. The following cases are now identified:

  • Writing directly into the AWS.config object with hardcoded credentials.
  • Creating AWS.ServiceName instances while passing hardcoded credentials.
  • Using hardcoded credentials in the AWS.Credentials object.

@Napalys Napalys marked this pull request as ready for review April 24, 2025 10:28
@Napalys Napalys requested a review from a team as a code owner April 24, 2025 10:28
Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

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

Have you tried with a sinkModel like this?

- `["aws-sdk", "", "AnyMember.Argument[0].Member[secretAccessKey]", "credentials-key"]
- `["aws-sdk", "", "Member[Credentials].Argument[1]", "credentials-key"]

As discussed offline, secret scanning should cover this in practice so we shouldn't spend much time on it. I was going to suggest merging it anyway if it was a simple change as there can be other use-cases for recognising non-constant credential sinks. I'm just not sure why it was done like this. Perhaps we should just quickly move on from this one.

@Napalys Napalys requested a review from asgerf April 28, 2025 10:39
@Napalys Napalys added the no-change-note-required This PR does not need a change note label Apr 28, 2025
@Napalys Napalys merged commit b4c98b4 into github:main Apr 28, 2025
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JS no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants