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.

Sign up for GitHub

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

Jump to bottom

fix(s3-deployment): sanitize log message in CustomCDKBucketDeployment handler #30225

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(s3-deployment): sanitize log message in CustomCDKBucketDeployment handler #30225

wants to merge 1 commit into from

Conversation

godwingrs22
Copy link
Member

@godwingrs22 godwingrs22 commented May 15, 2024 •
edited

Issue # (if applicable)

Closes #30211.

Reason for this change

Currently the s3_dest and old_s3_dest are logged as received. AWS inspector has identified as HIGH findings(CWE-117,93 - Log injection) in the lambda code.

Description of changes

We are sanitizing the message before logging to mitigate the CWE-117,93 - Log injection vulnerabilites.

Description of how you validated changes

Run all the existing integ test for s3-deployment custom resource and checked the AWS inspector if the finding still exists.

image

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@aws-cdk-automation aws-cdk-automation requested a review from a team May 15, 2024 23:20
@github-actions github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 labels May 15, 2024
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label May 15, 2024
aws-cdk-automation
aws-cdk-automation previously requested changes May 15, 2024
View reviewed changes
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@godwingrs22 godwingrs22 added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label May 15, 2024
@godwingrs22
Copy link
Member Author

Exemption Request: Changes only related to logging and doesn't require integ test.

@godwingrs22 godwingrs22 marked this pull request as ready for review May 15, 2024 23:25
@aws-cdk-automation aws-cdk-automation dismissed their stale review May 15, 2024 23:25

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

shikha372
shikha372 reviewed May 15, 2024
View reviewed changes
Copy link
Contributor

@shikha372 shikha372 left a comment

Choose a reason for hiding this comment

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

LGTM.. will wait for current build to pass

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 0acfeea
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@shikha372 shikha372 self-assigned this May 16, 2024
@github-actions github-actions bot mentioned this pull request May 20, 2024
Closed
@shikha372
Copy link
Contributor

Below five failing tests to be addressed . cc @godwingrs22

@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3731137301/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-codepipeline-actions/test/integ.pipeline-elastic-beanstalk-deploy.js
@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3731137301/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-dynamodb/test/integ.import-source.js
@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3731137301/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-ecs/test/ec2/integ.environment-file.js
@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3731137301/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-servicecatalog/test/integ.product.js
@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3731137301/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-servicecatalog/test/integ.two-products.js

@github-actions github-actions bot mentioned this pull request Jun 1, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shikha372 shikha372 shikha372 left review comments

@aws-cdk-automation aws-cdk-automation aws-cdk-automation left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@shikha372 shikha372

Labels
bug This issue is a bug. contribution/core This is a PR that came from AWS. effort/small Small work item – less than a day of effort p1 pr-linter/exempt-integ-test The PR linter will not require integ test changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CDK custom resource CustomCDKBucketDeployment: SecurityHub HIGH notification: CWE-117,93 - Log injection
3 participants
@godwingrs22 @aws-cdk-automation @shikha372