Skip to content

fix(s3): preserve SSE-KMS key ID metadata - #2145

Open
fracerma wants to merge 2 commits into
floci-io:mainfrom
fracerma:fix/s3-sse-kms-key-id
Open

fix(s3): preserve SSE-KMS key ID metadata#2145
fracerma wants to merge 2 commits into
floci-io:mainfrom
fracerma:fix/s3-sse-kms-key-id

Conversation

@fracerma

@fracerma fracerma commented Aug 6, 2026

Copy link
Copy Markdown

Summary

  • persist the explicit SSE-KMS key ID received by S3 PutObject
  • return the AWS-compatible key-ID response header from PUT, GET, and HEAD
  • cover the behavior with integration tests and document the supported metadata

Closes #2144

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

AWS Compatibility

Floci already preserved x-amz-server-side-encryption: aws:kms, but omitted
x-amz-server-side-encryption-aws-kms-key-id. AWS S3 returns that explicit key
ID as SSEKMSKeyId on the corresponding PUT, GET, and HEAD SDK responses. This
patch preserves and replays the standard AWS header without adding a custom API.

AWS references:

Verification

  • S3IntegrationTest: 125 passed
  • S3ServiceTest: 41 passed, 1 skipped
  • make docs-check: passed

Checklist

  • ./mvnw test passes locally (focused S3 suites above pass; full suite left to CI)
  • New or updated integration test added
  • Commit messages follow Conventional Commits

Copilot AI lite review requested due to automatic review settings August 6, 2026 12:02
@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown

Greptile Summary

The PR now preserves explicit SSE-KMS key-ID metadata across S3 object storage and copy operations and returns the AWS-compatible header from PUT, GET, and HEAD responses.

  • Adds KMS key-ID fields to object and request-option models.
  • Propagates inherited or replacement key IDs through CopyObject into persisted destination metadata.
  • Adds integration coverage and documents the supported metadata behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported CopyObject metadata loss is fixed because the resolved KMS key ID is now passed into the destination storage options and persisted.

Important Files Changed

Filename Overview
src/main/java/io/github/hectorvent/floci/services/s3/S3Controller.java Parses the SSE-KMS key-ID request header and emits it on relevant object responses.
src/main/java/io/github/hectorvent/floci/services/s3/S3Service.java Persists KMS key IDs and correctly carries inherited or replacement metadata through CopyObject.
src/main/java/io/github/hectorvent/floci/services/s3/model/S3Object.java Adds the persisted object metadata field used to round-trip the explicit KMS key ID.
src/test/java/io/github/hectorvent/floci/services/s3/S3IntegrationTest.java Covers PUT, GET, HEAD, AES filtering, and CopyObject behavior for SSE-KMS metadata.
docs/services/s3.md Documents persistence and replay of the standard SSE-KMS key-ID header.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[PutObject or CopyObject request] --> B[S3Controller parses SSE headers]
  B --> C[PutObjectOptions]
  C --> D[S3Service resolves effective encryption metadata]
  D --> E[S3Object persisted with SSE-KMS key ID]
  E --> F[PUT, GET, and HEAD response headers]
Loading

Reviews (2): Last reviewed commit: "fix(s3): validate and copy SSE-KMS key m..." | Re-trigger Greptile

Comment thread src/main/java/io/github/hectorvent/floci/services/s3/S3Service.java

Copilot AI left a comment

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.

Pull request overview

This PR fixes an S3 AWS-compatibility gap by persisting an explicit SSE-KMS key ID provided on PutObject and replaying it on subsequent object responses, aligning Floci behavior with AWS S3’s x-amz-server-side-encryption-aws-kms-key-id header behavior.

Changes:

  • Capture x-amz-server-side-encryption-aws-kms-key-id on PutObject and persist it on the stored S3Object.
  • Replay the SSE-KMS key ID header on PutObject, GetObject, and HeadObject responses.
  • Add integration coverage and document the supported persisted metadata.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/test/java/io/github/hectorvent/floci/services/s3/S3IntegrationTest.java Adds integration assertions for SSE-KMS key ID header on PUT/GET/HEAD and cleans up the new test object.
src/main/java/io/github/hectorvent/floci/services/s3/S3Service.java Persists SSE-KMS key ID on stored objects and extends object copy helper to include it.
src/main/java/io/github/hectorvent/floci/services/s3/S3Controller.java Reads SSE-KMS key ID request header on PUT and replays it in PUT/GET/HEAD response headers.
src/main/java/io/github/hectorvent/floci/services/s3/model/S3Object.java Adds sseKmsKeyId field with getter/setter to the persisted object model.
src/main/java/io/github/hectorvent/floci/services/s3/model/PutObjectOptions.java Adds sseKmsKeyId to PutObject options plumbing.
docs/services/s3.md Documents that Floci persists and replays SSE-KMS key IDs for object responses.
Suppressed comments (1)

src/main/java/io/github/hectorvent/floci/services/s3/S3Service.java:2419

  • CopyObject currently drops SSE-KMS key ID metadata. Even though the defensive copy helper now copies sseKmsKeyId, the CopyObject path stores the destination via storeObject(... new PutObjectOptions() ...) without carrying source.getSseKmsKeyId() (see copyS3Object building PutObjectOptions around S3Service.java:2675+), so subsequent GET/HEAD on a copied SSE-KMS object will omit x-amz-server-side-encryption-aws-kms-key-id. Consider propagating the key ID when the destination SSE mode is inherited from the source (and adding an integration assertion for copying kms-encrypted.txt).
        copy.setServerSideEncryption(source.getServerSideEncryption());
        copy.setSseKmsKeyId(source.getSseKmsKeyId());

object.setContentDisposition(effectiveOptions.getContentDisposition());
object.setCacheControl(effectiveOptions.getCacheControl());
object.setServerSideEncryption(normalizedServerSideEncryption);
object.setSseKmsKeyId(effectiveOptions.getSseKmsKeyId());

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 2a38810: the key ID is persisted only when the normalized SSE mode is aws:kms. Added coverage proving an AES256 object does not replay a supplied KMS key-ID header.

@hectorvent hectorvent added bug Something isn't working s3 Amazon Simple Storage Service (S3) kms AWS Key Management Service (KMS) labels Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working kms AWS Key Management Service (KMS) s3 Amazon Simple Storage Service (S3)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] S3 omits SSE-KMS key ID from object responses

3 participants