Skip to content

Conversation

@b-long
Copy link
Owner

@b-long b-long commented Nov 8, 2025

Complete support for NanoTDF, a lightweight TDF format, including its specific header structure, payload formatting, and key management mechanisms.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @b-long, 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 by adding full support for NanoTDF, a compact and efficient TDF format. The core of this enhancement lies in the implementation of Elliptic Curve Diffie-Hellman (ECDH) for robust key exchange and derivation, ensuring secure and interoperable NanoTDF operations. The changes span across various modules, from CLI argument parsing to low-level cryptographic operations and KAS client interactions, all while maintaining backward compatibility where appropriate. New tests validate the correctness and interoperability of the new NanoTDF features.

Highlights

  • NanoTDF Core Implementation: Introduced comprehensive support for NanoTDF, a lightweight TDF format, including its specific header structure, payload formatting, and key management mechanisms.
  • ECDH Key Exchange: Implemented Elliptic Curve Diffie-Hellman (ECDH) key exchange with HKDF (HMAC-based Key Derivation Function) for secure key derivation, supporting curves like secp256r1, secp384r1, secp521r1, and secp256k1.
  • KAS Integration for NanoTDF: Updated the Key Access Server (KAS) client to support NanoTDF-specific requests, including sending the full NanoTDF header for key unwrapping and handling ECDH-derived keys.
  • Resource Locator Specification: Refactored the ResourceLocator class to strictly adhere to the NanoTDF specification for encoding resource URLs and identifiers, including protocol and identifier length enums.
  • Improved Error Handling & Logging: Enhanced error logging in the CLI to always print tracebacks for unexpected exceptions, aiding in debugging.
  • Interoperability Testing: Added extensive integration tests to ensure interoperability between the Python SDK's NanoTDF implementation and otdfctl, validating both encryption and decryption workflows.
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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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 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.

Footnotes

  1. 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.

@b-long
Copy link
Owner Author

b-long commented Nov 8, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 significant enhancements for NanoTDF support, including the implementation of ECDH key exchange, spec-compliant header and payload formats, and updates to the KAS client to handle NanoTDF-specific rewrap requests. The changes are extensive and well-structured, with good separation of concerns in the new ecdh.py module. The refactoring of nanotdf.py and kas_client.py to support both legacy TDF and the new NanoTDF ECDH flow is particularly noteworthy. The addition of comprehensive unit and integration tests, including interoperability tests with otdfctl, provides strong confidence in the correctness of the implementation. My review includes a few minor suggestions for code cleanup and consistency, such as using constants for magic numbers and organizing imports. Overall, this is a high-quality contribution that significantly improves NanoTDF functionality.

- ephemeral_public_key_compressed: Compressed ephemeral public key to store in header (None for RSA)
- kas_public_key: KAS public key PEM string (or None if not available)
"""
import logging
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The logging module is imported multiple times locally within this file (here and on lines 475, 510, 635, 713). It's standard practice to have a single import logging at the top of the file.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 significant and well-structured changes to add support for NanoTDF, including implementing the ECDH key exchange mechanism and aligning the binary format with the official specification. The code is well-refactored, with improved modularity and extensive test coverage for the new functionality. I've identified a critical issue in the decryption logic related to backward compatibility that needs to be addressed.

@b-long b-long force-pushed the chore/fix-nanotdf-support branch 5 times, most recently from 600bcf4 to 39ed91d Compare November 9, 2025 02:06
@b-long b-long force-pushed the chore/fix-nanotdf-support branch from 39ed91d to 5eb8a25 Compare November 9, 2025 02:14
@b-long b-long requested a review from Copilot November 9, 2025 02:27
Copy link
Contributor

Copilot AI left a comment

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 implements NanoTDF support with ECDH (Elliptic Curve Diffie-Hellman) key exchange, replacing the previous RSA-based approach. The changes enable the Python SDK to create and decrypt NanoTDF files compatible with the OpenTDF specification and interoperable with other tools like otdfctl.

Key changes:

  • Implemented ECDH key exchange with HKDF key derivation for NanoTDF encryption
  • Updated NanoTDF header structure to align with the specification (separated policy binding from policy info)
  • Added comprehensive integration and unit tests for ECDH and NanoTDF functionality

Reviewed Changes

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

Show a summary per file
File Description
tests/test_nanotdf_integration.py Updated to use EC keypairs instead of RSA for NanoTDF tests
tests/test_nanotdf_ecdh.py New comprehensive test suite for NanoTDF with ECDH across multiple curves
tests/test_nanotdf.py Fixed integration test by removing skip decorator and adding symmetric key
tests/test_header.py Updated to reflect policy binding as separate field in Header
tests/test_ecdh.py New unit tests for ECDH module covering all operations
tests/integration/otdfctl_to_python/test_python_nanotdf_only.py New Python-only NanoTDF CLI integration test
tests/integration/otdfctl_to_python/test_nanotdf_cli_comparison.py New cross-tool interoperability tests between otdfctl and Python CLI
src/otdf_python/resource_locator.py Updated to conform to NanoTDF spec format with protocol/identifier encoding
src/otdf_python/policy_info.py Removed binding field (moved to Header per spec)
src/otdf_python/nanotdf.py Major refactor to support ECDH, proper header format, and KAS integration
src/otdf_python/kas_client.py Enhanced to support NanoTDF header-based unwrapping and EC key types
src/otdf_python/header.py Added policy_binding field and GMAC size constant
src/otdf_python/ecdh.py New module implementing ECDH key exchange and HKDF derivation
src/otdf_python/ecc_mode.py Added curve name mapping and string parsing support
src/otdf_python/cli.py Added KAS endpoint derivation from platform URL

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

except Exception as e:
logger.error(f"Unexpected error: {e}")
logger.debug("", exc_info=True)
logger.error("", exc_info=True) # Always print traceback for unexpected errors
Copy link

Copilot AI Nov 9, 2025

Choose a reason for hiding this comment

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

The first argument to logger.error() is an empty string, which produces no message. Consider providing a descriptive message like 'Traceback:' or using logger.exception(e) which automatically includes exception info.

Copilot uses AI. Check for mistakes.
@b-long b-long merged commit 8f09297 into main Nov 10, 2025
9 checks passed
@b-long b-long deleted the chore/fix-nanotdf-support branch November 10, 2025 12:48
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.

2 participants