-
Notifications
You must be signed in to change notification settings - Fork 705
demo #369
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
base: master
Are you sure you want to change the base?
demo #369
Conversation
} | ||
|
||
public static AmazonS3 getS3Client() { | ||
return AmazonS3ClientBuilder.standard().withRegion(Regions.DEFAULT_REGION).build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This code is written so that the client cannot be reused across invocations of the Lambda function.
To improve the performance of the Lambda function, consider using static initialization/constructor, global/static variables and singletons. It allows to keep alive and reuse HTTP connections that were established during a previous invocation.
Learn more about best practices for working with AWS Lambda functions.
} | ||
|
||
public static AmazonS3 getS3Client() { | ||
return AmazonS3ClientBuilder.standard().withRegion(Regions.DEFAULT_REGION).build(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Problem: The AWS region value is hardcoded in the code while calling AWS resources. The code will require refactoring, while expanding to other AWS regions.
Solution: The AWS region can be automatically determined by the client/client builder.The following link contains sample code snippets, on how to avoid AWS region hardcoding.
final AmazonS3 s3Client = EventHandler.getS3Client(); | ||
logger.log("Processing Bucket: " + bucketName); | ||
|
||
ObjectListing files = s3Client.listObjects(bucketName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This code might not produce accurate results if the operation returns paginated results instead of all results. Consider adding another call to check for additional results.
final AmazonS3 s3Client = EventHandler.getS3Client(); | ||
logger.log("Processing Bucket: " + bucketName); | ||
|
||
ObjectListing files = s3Client.listObjects(bucketName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This code uses an outdated API. ListObjectsV2 is the revised List Objects API, and we recommend you use this revised API for new application developments.
|
||
long expirationTime = System.currentTimeMillis() + Duration.ofMinutes(1).toMillis(); | ||
while(System.currentTimeMillis() < expirationTime) { | ||
if (s3Client.doesObjectExist(Constants.SUMMARY_BUCKET, summaryUpdateName)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
This code appears to be waiting for a resource before it runs. You could use the waiters feature to help improve efficiency. Consider using ObjectExists or ObjectNotExists. For more information, see https://aws.amazon.com/blogs/developer/waiters-in-the-aws-sdk-for-java/
} | ||
|
||
public ShopifyShop connectToShopify(String subdomain) { | ||
final String token = "shpss_sdkfhkjh134134141341344133412312345678"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
It appears your code contains a hardcoded Shopify Shared Secret. Hardcoded secrets can allow attackers to bypass authentication methods and perform malicious actions. Possible remediation approaches are:
1. Manage secret data using environment variables:
a. Identify the secret data and store that data in environment variables on your local and production server. The setting process may vary depending on your OS and environment.
b. Replace hard-coded secrets with references to environment variables, e.g., `password = os.environ.get('PASSWORD')`.
2. Use AWS Secrets Manager to store, rotate, monitor, and control access to secrets. To retrieve secrets, you can replace hardcoded secrets in applications with a call to Secrets Manager APIs. To use Secrets Manager:
a. Visit [Secrets Manager](https://aws.amazon.com/secrets-manager/) on the AWS Management Console.
b. Choose the secret type on the console page and follow the instructions.
c. Use the code samples suggested in the console to retrieve the secret in your application.
} | ||
|
||
public ShopifyShop connectToShopify(String subdomain) { | ||
final String token = "shpss_sdkfhkjh134134141341344133412312345678"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Potential hardcoded credential detected. This code may contain sensitive data such as passwords or API keys embedded directly in the source. Hardcoded credentials can be extracted and misused, leading to unauthorized access to systems or data breaches. To remediate this, store secrets in environment variables or use a secrets management tool like AWS Secrets Manager, Azure Key Vault, or HashiCorp Vault. Avoid committing credentials to version control. For best practices, refer to - https://cwe.mitre.org/data/definitions/798.html
|
||
public String weakMessageEncryption(String message, String key) throws Exception { | ||
Cipher cipher = Cipher.getInstance("RSA"); | ||
SecretKey secretKey = new SecretKeySpec(key.getBytes(), "AES"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Potential hardcoded credential detected. This code may contain sensitive data such as passwords or API keys embedded directly in the source. Hardcoded credentials can be extracted and misused, leading to unauthorized access to systems or data breaches. To remediate this, store secrets in environment variables or use a secrets management tool like AWS Secrets Manager, Azure Key Vault, or HashiCorp Vault. Avoid committing credentials to version control. For best practices, refer to - https://cwe.mitre.org/data/definitions/798.html
String fileContents = s3Client.getObjectAsString(bucketName, summary.getKey()); | ||
|
||
if (!isValidFile(fileContents)) { | ||
logger.log(String.format("Skipping invalid file %s", summary.getKey())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Potential log injection detected. Ensure all untrusted input is properly sanitized before logging. Use parameterized logging or validate input against a allow list to prevent log injection vulnerabilities. Consider using a dedicated logging library's built-in sanitization features when available. Learn more - https://cwe.mitre.org/data/definitions/117.html
processShipmentUpdates(logger); | ||
return "SUCCESS"; | ||
} catch (final Exception ex) { | ||
logger.log(String.format("Failed to process shipment Updates in %s due to %s", scheduledEvent.getAccount(), ex.getMessage())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.
Potential log injection detected. Ensure all untrusted input is properly sanitized before logging. Use parameterized logging or validate input against a allow list to prevent log injection vulnerabilities. Consider using a dedicated logging library's built-in sanitization features when available. Learn more - https://cwe.mitre.org/data/definitions/117.html
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.