Skip to content
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

AWS - [Security] Don't throw exception with secret value in message #61

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

davidsloan
Copy link
Contributor

@davidsloan davidsloan commented Oct 23, 2023

This is a rebase/rework of #23 by @adrianisk against the latest version of the code base.

Original PR description:

Summary

This change prevents the library from throwing an exception that contains the secret text in the exception message when using the AWS SecretsManager provider. While doing some local testing I noticed the thrown exception, along with the secret, will end up in Kafka Connect's logs which isn't great. To fix I updated the code to catch the JsonParseException which contains the secret value, and throw an exception with a generic parsing error message in its place.

Test

I added a unit test to verify, prior to the fix it failed with

- should not throw exception with secret value in message *** FAILED *** (53 milliseconds)
  "Unrecognized token 'secret': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')
   at [Source: (String)"secret-value"; line: 1, column: 7]" included substring "secret-value" (AWSSecretProviderTest.scala:144)

@davidsloan davidsloan force-pushed the contrib/pr-23-rebase branch from 3467c7f to 660b2de Compare October 23, 2023 16:09
@andrewstevenson andrewstevenson self-requested a review October 24, 2023 09:13
@andrewstevenson andrewstevenson merged commit 6966df0 into master Oct 24, 2023
2 checks passed
@andrewstevenson andrewstevenson deleted the contrib/pr-23-rebase branch October 24, 2023 09:13
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.

2 participants