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

fix(bootstrap): remove Security Hub finding S3.10 #24175

Merged
merged 1 commit into from Feb 15, 2023
Merged

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Feb 15, 2023

NOTE: This PR bumps the version of the bootstrap stack to 16, but there is no need to update your bootstrap stacks, unless it is to get rid of the Security Hub finding; this change has no effect on the functionality of any CDK app deployed to the environment.

Security Hub Finding S3.10 says:

S3 buckets with versioning enabled should have lifecycle policies configured

Presumably so you're not unknowingly accumulating a bigger and bigger S3 bucket as you are overwriting existing files.

CDK will never do that, as files are content-addressed and immutable, but Security Hub can't know that and so it complains.

Add a lifecycle rule to the S3 bucket to get rid of the finding. Expiration time of non-current files is set to 1 year. This should give enough opportunity to diagnose potential issues and audit the any funkiness in the bucket if the assumption that files are never overwritten should ever be violated.


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

**NOTE**: This PR bumps the version of the bootstrap stack to `16`, but
there is no need to update your bootstrap stacks, unless it is to get
rid of the Security Hub finding; this change has no effect on the
functionality of any CDK app deployed to the environment.

[Security Hub Finding
S3.10](https://docs.aws.amazon.com/securityhub/latest/userguide/securityhub-standards-fsbp-controls.html#fsbp-s3-10)
says:

> S3 buckets with versioning enabled should have lifecycle policies configured

Presumably so you're not unknowingly accumulating a bigger and bigger S3
bucket as you are overwriting existing files.

CDK will never do that, as files are content-addressed and immutable,
but Security Hub can't know that and so it complains.

Add a lifecycle rule to the S3 bucket to get rid of the finding.
Expiration time of non-current files is set to 1 year. This should give
enough opportunity to diagnose potential issues and audit the any
funkiness in the bucket if the assumption that files are never
overwritten should ever be violated.
@rix0rrr rix0rrr requested a review from a team February 15, 2023 10:07
@rix0rrr rix0rrr self-assigned this Feb 15, 2023
@rix0rrr rix0rrr changed the title fix(bootstrap): remove Security Hub finding [S3.10] fix(bootstrap): remove Security Hub finding S3.10 Feb 15, 2023
@gitpod-io
Copy link

gitpod-io bot commented Feb 15, 2023

@github-actions github-actions bot added the p2 label Feb 15, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team February 15, 2023 10:07
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Feb 15, 2023
@rix0rrr rix0rrr added pr-linter/exempt-test The PR linter will not require test changes pr-linter/exempt-integ-test The PR linter will not require integ test changes labels Feb 15, 2023
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 5c3f29c
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Feb 15, 2023

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit a1da757 into main Feb 15, 2023
@mergify mergify bot deleted the huijbers/security-hub branch February 15, 2023 13:11
mergify bot pushed a commit that referenced this pull request Mar 28, 2023
…because of missing lifecycle policy (#24735)

After enabling AWS Foundational Security Best Practices v1.0.0 in the security hub,
I am always frustrated when I see failed checks.

Similar to #24175 I would like to see a lifecycle rule that does not do much but at least per default resolves the finding.

I know that there is an RFC for garbage collection in the works but this is a simple immediate fix.

_This is heavily inspired by https://github.com/aws/aws-cdk/pull/24175_

Closes #24723.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
homakk pushed a commit to homakk/aws-cdk that referenced this pull request Mar 28, 2023
…because of missing lifecycle policy (aws#24735)

After enabling AWS Foundational Security Best Practices v1.0.0 in the security hub,
I am always frustrated when I see failed checks.

Similar to aws#24175 I would like to see a lifecycle rule that does not do much but at least per default resolves the finding.

I know that there is an RFC for garbage collection in the works but this is a simple immediate fix.

_This is heavily inspired by https://github.com/aws/aws-cdk/pull/24175_

Closes aws#24723.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exempt-test The PR linter will not require test changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants