-
Notifications
You must be signed in to change notification settings - Fork 2
feat: introduce the base key for nano flows #273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @mkleene, 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 SDK's key management capabilities by introducing a 'base key' concept for NanoTDFs and refactoring the key splitting and autoconfiguration logic. The changes allow for more dynamic and policy-driven key access, improving the flexibility and robustness of TDF creation and decryption processes.
Highlights
- Base Key Support for NanoTDF: Introduced a mechanism to fetch a 'base key' for NanoTDFs from a well-known configuration endpoint. This base key can serve as a default or fallback KAS public key for encryption, enhancing flexibility in key management.
- Enhanced Key Management with Key IDs (KIDs): The SDK's KAS information handling and caching (
KASKeyCache
) have been updated to explicitly incorporate Key IDs (KIDs). This allows for more precise identification and retrieval of specific public keys, especially when multiple keys are associated with a single KAS. - Refactored Key Splitting and Autoconfiguration: The complex logic for determining key splitting plans and resolving KAS public keys has been extracted into a new
Planner
class. This refactoring improves modularity, maintainability, and allows for more sophisticated policy-driven key mapping directly from attribute definitions and values within the policy service. - Platform Dependency Upgrade: The underlying platform protocol dependency has been upgraded from
v0.3.0
tov0.5.0
. This update likely provides compatibility with new backend features that support the base key and enhanced policy capabilities.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Warning Gemini encountered an error creating the review. You can try again by commenting |
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new feature to use a 'base key' for NanoTDF creation, which is fetched from a well-known configuration endpoint as a fallback when no KAS information is provided. The changes look good overall and the new test coverage is appreciated.
I've found a few issues that should be addressed:
- A regression in the
ECCMode
handling that causes the ECDSA binding setting to be lost. - A critical bug in the new test case where the JSON payload is malformed.
- Some redundant code and minor issues in tests.
Please see my detailed comments.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces the concept of a 'base key' for NanoTDF flows, which can be used as a fallback when no specific KAS information is provided. I've identified a critical issue in the new test case where an invalid JSON string is constructed, which would cause the test to fail. I've also provided a couple of suggestions to improve code clarity and conciseness in NanoTDF.java
by using more idiomatic Optional
patterns.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a fallback mechanism to use a "base key" for NanoTDF creation when no specific KAS key is provided. This is achieved by fetching the key from a well-known configuration endpoint. The changes look good and align with the description. I've identified a critical issue in a test case that would prevent it from running successfully, along with a couple of suggestions to improve code formatting and test assertion robustness. Once these are addressed, the PR should be in great shape.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
The logic should be simple; whenever there is no key provided on the
tdfConfig
we reach out to the platformto try to download a base key.