Conversation
github-actions
bot
added
comp:sdk
Summary of ChangesHello @pflynn-virtru, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness and compatibility of the experimental TDF SDK by introducing a suite of new regression and cross-SDK compatibility tests. These additions address critical edge cases in key management, particularly concerning legacy attribute grants and the merging of default KAS public keys, while also ensuring the integrity of segment hashing. Furthermore, the changes validate that data encrypted by the experimental SDK can be seamlessly decrypted and processed by the production SDK, reinforcing the reliability of the experimental implementation. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. A bug, a feature, hard to tell, A test now written, all is well. With hashes strong and keys aligned, No more regressions, peace of mind. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces valuable regression and compatibility tests for the experimental TDF SDK. The new tests cover important edge cases like URI-only attribute grants and ensure segment integrity hashing is correct. A comprehensive cross-decryption test suite has also been added to validate compatibility with the production SDK. The changes are well-structured and significantly improve test coverage. I've found a minor but repeated issue in the test implementation related to slice indexing, which would cause compilation errors.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
pflynn-virtru
force-pushed
the
fix/exp-sdk
branch
from
bc24caa to
61fc100
Compare
X-Test Failure Report |
…y hashing Signed-off-by: Paul Flynn <pflynn@virtru.com>
pflynn-virtru
force-pushed
the
fix/exp-sdk
branch
from
61fc100 to
e9a4874
Compare
X-Test Failure Report |
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces valuable regression and compatibility tests for the experimental TDF SDK. The changes to the production code are well-targeted, correctly addressing legacy attribute grant handling and ensuring segment hash compatibility with the production SDK. The new tests are particularly impressive in their thoroughness, covering important edge cases for key management and verifying cross-decryption and TDF format compatibility. Overall, this is a high-quality contribution that significantly strengthens the reliability of the experimental SDK. I have one minor suggestion to improve code readability.
pflynn-virtru
changed the title
…ental benchmark Fix multiple concurrency bugs in the experimental TDF writer benchmark: - goroutine captured loop variable by closure reference (data race on index) - goroutine wrote to outer `err` variable (data race) - shared payload sub-slices passed to EncryptInPlace which overwrites input buffer - remainder segment truncated when payload not evenly divisible by chunk size Also add TLS skip support (--insecureSkipVerify), save TDF to disk, and verify decrypt roundtrip with the production SDK. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
Reduce repeated GetPublicKey() calls by assigning to a local variable, improving readability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
2 participants
This pull request adds comprehensive regression and compatibility tests to the experimental TDF SDK, focusing on key management edge cases and ensuring encryption format compatibility with the production SDK. The new tests verify correct handling of legacy attribute grants, segment integrity hashing, and cross-decryption between SDKs. Additionally, new imports were added to support these tests.
See xtest failure: opentdf/tests#414
Regression and Compatibility Testing Enhancements:
xor_splitter_test.goto ensure that when an attribute grant references a KAS URL by URI only (without an embedded public key), the default KAS public key is correctly merged, and that existing keys are not overwritten by the default.writer_test.goto verify that segment hashes include both nonce and ciphertext (not just ciphertext), and that finalization succeeds for URI-only grants by leveraging the default KAS public key.Cross-SDK Compatibility:
writer_test.goto validate that the experimental writer's output is fully compatible with the production SDK's decryption and segment hash verification, including full TDF ZIP assembly and parsing with the production ZIP reader.Test Infrastructure Improvements: