Skip to content

VED-814 Resolve sonarcloud warnings on S3 bucket ownership#882

Merged
dlzhry2nhs merged 1 commit intomasterfrom
feature/VED-814-sonar-cloud-warnings-s3-bucket
Oct 7, 2025
Merged

VED-814 Resolve sonarcloud warnings on S3 bucket ownership#882
dlzhry2nhs merged 1 commit intomasterfrom
feature/VED-814-sonar-cloud-warnings-s3-bucket

Conversation

@dlzhry2nhs
Copy link
Copy Markdown
Contributor

@dlzhry2nhs dlzhry2nhs commented Oct 7, 2025

Summary

  • Routine Change

Resolves the outstanding security warnings we have in Sonarcloud that causes us to fail on master/overall coverage: https://sonarcloud.io/project/issues?issueStatuses=OPEN%2CCONFIRMED&types=VULNERABILITY&severities=BLOCKER%2CCRITICAL%2CMAJOR%2CMINOR&inNewCodePeriod=true&sinceLeakPeriod=true&id=NHSDigital_immunisation-fhir-api

  • Add expected bucket owner account to boto3 S3 calls
  • Update tests to pass using the moto default account ID

Reviews Required

  • Dev

Review Checklist

ℹ️ This section is to be filled in by the reviewer.

  • I have reviewed the changes in this PR and they fill all or part of the acceptance criteria of the ticket, and the code is in a mergeable state.
  • If there were infrastructure, operational, or build changes, I have made sure there is sufficient evidence that the changes will work.
  • I have ensured the changelog has been updated by the submitter, if necessary.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 7, 2025

This branch is working on a ticket in the NHS England VED JIRA Project. Here's a handy link to the ticket:

VED-814

@dlzhry2nhs dlzhry2nhs force-pushed the feature/VED-814-sonar-cloud-warnings-s3-bucket branch from 5066d91 to 77c62e4 Compare October 7, 2025 11:46
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Oct 7, 2025

Key=destination_key
Key=destination_key,
ExpectedBucketOwner=EXPECTED_BUCKET_OWNER_ACCOUNT,
ExpectedSourceBucketOwner=EXPECTED_BUCKET_OWNER_ACCOUNT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just curious as to why two arguments references the same value

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's a good question. So for any copy request (you can check boto3 - S3) you can set both the bucket owner and source bucket i.e. the bucket you are copying from.

In both cases, these are buckets that belong in our account. Hope that makes sense.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hence the same account ID, makes total sense

@dlzhry2nhs dlzhry2nhs merged commit 83a9491 into master Oct 7, 2025
9 checks passed
@dlzhry2nhs dlzhry2nhs deleted the feature/VED-814-sonar-cloud-warnings-s3-bucket branch October 7, 2025 13:03
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.

3 participants