Skip to content

Use s3 to verify bucket ownership during get resource state#586

Merged
Zee2413 merged 3 commits into
mainfrom
s3-ownership
May 21, 2026
Merged

Use s3 to verify bucket ownership during get resource state#586
Zee2413 merged 3 commits into
mainfrom
s3-ownership

Conversation

@Zee2413

@Zee2413 Zee2413 commented May 20, 2026

Copy link
Copy Markdown
Contributor

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.

@Zee2413 Zee2413 requested a review from a team as a code owner May 20, 2026 08:01
Comment thread src/services/S3Service.ts Outdated
Zee2413 added 2 commits May 21, 2026 10:21
Replace the GetBucketEncryption + HeadBucket pair with a single
HeadBucket call that passes ExpectedBucketOwner=<caller account>. This
is the only S3 API behavior that distinguishes 'I own this bucket' from
'this bucket is publicly readable but owned by someone else' --
HeadBucket alone returns 200 for public cross-account buckets like
commoncrawl, which would let CCAPI's S3 read handler return their
properties.

Translate 403 (cross-account or wrong owner) and 404 (does not exist)
into ResourceNotFoundException so callers handle them through the same
path as any other CCAPI not-found result. Wrong-region still returns a
distinct error string so the user can correct the region. Other errors
(network, credentials, throttling, 5xx) propagate unchanged instead of
being mislabeled as ownership failures.

Add STSClient support to AwsClient so S3Service can resolve the caller
account ID via sts:GetCallerIdentity. The two S3 permissions used
(s3:ListBucket via HeadBucket) are already part of the AWS::S3::Bucket
read handler permission set, so this adds no new permission burden.
@Zee2413 Zee2413 merged commit 8dc77e6 into main May 21, 2026
15 checks passed
@Zee2413 Zee2413 deleted the s3-ownership branch May 21, 2026 19:09
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