Conversation
|
|
|
|
YusefSyed
marked this pull request as ready for review
October 2, 2026 21:01
YusefSyed
requested review from
AlenkaF,
HuaHuaY,
pitrou,
raulcd,
rok and
wgtmac
as code owners
October 2, 2026 21:01
|
|
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Fixes #51669. With
AES_GCM_CTR_V1and a plaintext footer, the writer encrypts data pages with CTR but recordsAES_GCM_V1inFileMetaData.encryption_algorithm. A reader consequently uses the wrong file algorithm and cannot read an otherwise successful write.What changes are included in this PR?
Record the configured file algorithm in plaintext-footer metadata. Footer signing and verification continue to use the existing metadata cipher path. Add native checks for the recorded algorithm, synchronous/asynchronous reads, wrong-key rejection, and tampered footer signatures; extend the Python direct-key test across both algorithms and footer modes.
Are these changes tested?
git diff --checkpassed.26.0.0a1.dev2+g84f7d3fec), linked to the task-built native libraries: 22 direct-key tests passed, and the standalone algorithm/footer matrix passed 4/4.Native test command, after building
parquet-encryption-testwith encryption enabled:Validation so far is on macOS arm64. No Windows/Linux local result or live KMS result is claimed.
Are there any user-facing changes?
New plaintext-footer files written with
AES_GCM_CTR_V1carry the correct algorithm metadata. Existing malformed files are not repaired automatically. Public APIs are unchanged.This PR contains a "Critical Fix" under the template's invalid-data criterion: affected writes produced inconsistent encryption metadata and unreadable files. This is not a claim of a newly demonstrated security vulnerability.
Was AI used for this PR?
PR code and description written by:
Reviewed before submission by:
Codex assisted with investigation, implementation, regression tests, and this description. The main Codex agent reviewed the production/test diff, the file-algorithm versus metadata-cipher distinction, and the recorded native test results. No human review or test execution is asserted on the submitter's behalf.