fix(kas): Fix EC P-521 typo#3075
Conversation
FormatAlg returns "ec:secp512r1" for ALGORITHM_EC_P521 instead of "ec:secp521r1". This causes the SDK's getKasKeyAlg to return ALGORITHM_UNSPECIFIED, making EC-521 base key encryption fail with "unsupported algorithm: ALGORITHM_UNSPECIFIED". The test compares FormatAlg output against the canonical ocrypto.KeyType constants to catch string mismatches. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
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 addresses a critical bug where a typo in the 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. Test finds small typo, Five-two-one, not five-one-two, Code now runs so true. Footnotes
|
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
There was a problem hiding this comment.
Code Review
This pull request adds a new test file to expose a typo in the FormatAlg function for the EC P-521 algorithm. The new tests are well-structured and effectively highlight the existing bug. I've added one comment regarding a common Go pitfall in table-driven tests with subtests to improve the robustness of the test code against future changes.
Add round-trip tests that prove FormatAlg and getKasKeyAlg share the same ocrypto.KeyType contract: - service/pkg/db: FormatAlg(enum) → reverse-map lookup → assert original enum. EC-P521 subtest fails (typo: "ec:secp512r1" vs "ec:secp521r1"). - sdk: formatAlg(enum) → getKasKeyAlg(result) → assert original enum. All pass (SDK formatAlg is correct). Refs: #3070 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:
|
Refs: #3070 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn@virtru.com>
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:
|
🤖 I have created a release *beep* *boop* --- ## [0.13.0](sdk/v0.12.0...sdk/v0.13.0) (2026-02-17) ### ⚠ BREAKING CHANGES * **policy:** remove namespace certificate feature ([#3051](#3051)) ### Bug Fixes * **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.9.0 to 0.10.0 in /sdk ([#3078](#3078)) ([527c34d](527c34d)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.15.0 to 0.16.0 in /sdk ([#3081](#3081)) ([3d4ce33](3d4ce33)) * **docs:** DSPX-2409 replace SDK README code example with working code ([#3055](#3055)) ([566cb6f](566cb6f)) * Go 1.25 ([#3053](#3053)) ([65eb7c3](65eb7c3)) * **kas:** Fix EC P-521 typo ([#3075](#3075)) ([abc088d](abc088d)) * **sdk:** conditionally set client_id based on auth method ([#2968](#2968)) ([abdeb69](abdeb69)) ### Code Refactoring * **policy:** remove namespace certificate feature ([#3051](#3051)) ([48abb81](48abb81)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [0.13.0](service/v0.12.0...service/v0.13.0) (2026-02-18) ### ⚠ BREAKING CHANGES * **policy:** remove namespace certificate feature ([#3051](#3051)) ### Features * **authz:** add casbin roleprovider interface ([#3069](#3069)) ([9d6b3f3](9d6b3f3)) * **core:** add interceptors to start options ([#3031](#3031)) ([e0b4e93](e0b4e93)) ### Bug Fixes * **deps:** bump github.com/opentdf/platform/lib/fixtures from 0.4.0 to 0.5.0 in /service ([#3034](#3034)) ([66b61b1](66b61b1)) * **deps:** bump github.com/opentdf/platform/lib/ocrypto from 0.9.0 to 0.10.0 in /service ([#3080](#3080)) ([49582f0](49582f0)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.15.0 to 0.16.0 in /service ([#3083](#3083)) ([a332f95](a332f95)) * **deps:** vulnerability fix in connect-rpc validate and ristretto ([#3065](#3065)) ([8860fed](8860fed)) * Go 1.25 ([#3053](#3053)) ([65eb7c3](65eb7c3)) * **kas:** dont hardcode P-256 curve ([#3073](#3073)) ([826d857](826d857)) * **kas:** Fix EC P-521 typo ([#3075](#3075)) ([abc088d](abc088d)) * **policy:** reject unencrypted private keys for modes 1/2 ([#3072](#3072)) ([e2dc6d8](e2dc6d8)) ### Code Refactoring * **policy:** remove namespace certificate feature ([#3051](#3051)) ([48abb81](48abb81)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com>
Summary
FormatAlginservice/pkg/db/marshalHelpers.go:159returns"ec:secp512r1"forALGORITHM_EC_P521— should be"ec:secp521r1"ALGORITHM_UNSPECIFIEDwhen reading EC-521 base keys from the well-known config, failing with"unsupported algorithm: ALGORITHM_UNSPECIFIED"at encrypt timeFormatAlgoutput against canonicalocrypto.KeyTypeconstants for all supported algorithmsP-521 subtest fails as expected. Fix is a one-character change:
512→521.Test plan
expected "ec:secp521r1",actual "ec:secp512r1"See failure: https://github.com/opentdf/platform/actions/runs/21959964503/job/63434443522?pr=3075
🤖 Generated with Claude Code