Skip to content

fix(core): don't send kms_key as an SSE-C key in S3Downloader.download - #6414

Open
AkshayShah03 wants to merge 1 commit into
aws:masterfrom
AkshayShah03:fix/s3-downloader-kms-key
Open

AkshayShah03 wants to merge 1 commit into
aws:masterfrom
AkshayShah03:fix/s3-downloader-kms-key

Conversation

@AkshayShah03

Copy link
Copy Markdown

Issue #, if available: Same bug as #4034, which was closed but the v3 code is unchanged.

Description of changes:

S3Downloader.download turns kms_key into extra_args={"SSECustomerKey": kms_key}. SSECustomerKey is the SSE-C header: it expects a raw 256-bit customer key together with SSECustomerAlgorithm, not a KMS key ARN. Any download with kms_key set therefore fails with ClientError (400) HeadObject: Bad Request. S3 decrypts SSE-KMS objects transparently, so a GET doesn't need a key at all.

This breaks DatasetBuilder.to_dataframe() for KMS-encrypted feature store output. feature_utils downloads the query result with the dataset's KMS key.

This change keeps the kms_key parameter for backwards compatibility, documents that it's unused, and stops sending it.

Testing:

  • test_download_with_kms_key asserted the SSE-C behaviour. It now expects extra_args=None.
  • tests/unit/s3: 105 passed.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

S3Downloader.download passed kms_key as extra_args={'SSECustomerKey': kms_key}.
SSECustomerKey is the SSE-C header, which expects a raw 256-bit customer key
plus SSECustomerAlgorithm, not a KMS key ARN, so every download with kms_key
failed with 400 Bad Request. S3 decrypts SSE-KMS objects transparently, so no
key is needed on GET.

This breaks DatasetBuilder.to_dataframe() for KMS-encrypted feature store
output, as reported in aws#4034 (closed, but the v3 code is unchanged).

Keep the kms_key parameter for backwards compatibility but no longer send it.

This branch had an error being deployed

1 failed deployment
manual-approval — b1bc0dea Deployed Oct 10, 2026 by AkshayShah03 via wait-for-approval #1690
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.

1 participant