fix(xtest): correct PQC version threshold and algorithm error detection#462
fix(xtest): correct PQC version threshold and algorithm error detection#462dmihalcik-virtru wants to merge 1 commit into
Conversation
… error detection Two bugs causing spurious CI failures in test_pqc.py when testing against older platform versions: 1. **`tdfs.py` version threshold** — `mechanism-xwing` and `mechanism-secpmlkem` were gated at `>= (0, 14, 0)`, but the key management API for hpqt:* algorithms didn't land until after `service/v0.15.0`. Platform v0.15.0 rejects them with a protobuf validation error (`key_algorithm_defined`), causing tests to ERROR (fixture crash) rather than SKIP. Raise threshold to `(0, 16, 0)`. 2. **`otdfctl.py` error string** — `kas_registry_create_key` only matched `"Invalid key parameters: invalid algorithm"` to raise `InvalidAlgorithm`. Platform v0.15.0 returns `"key_algorithm_defined"` (a protobuf validation error), which fell through to a bare `AssertionError`, turning what should be a pytest.skip into a test ERROR. Extend the match condition. These fixes target PR #461 (fix/DSPX-3356-xwing-kao-size-assertions) which was failing on `xct (v0.15.0, go@v0.32.0)` for exactly these reasons. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![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 updates the error handling in otdfctl.py to include a new validation error string and adjusts the version requirement for hybrid PQ/T KEM support in tdfs.py to version 0.16.0. Feedback was provided to add a descriptive comment for the new error string to improve code maintainability.
| if ( | ||
| "Invalid key parameters: invalid algorithm" in err_str | ||
| or "key_algorithm_defined" in err_str | ||
| ): |
There was a problem hiding this comment.
While this string matching correctly identifies the specific protobuf validation error returned by platform v0.15.0, it is a bit of a "magic string" that lacks context in the code. Adding a brief comment explaining that this string corresponds to a specific platform version's error response would improve maintainability.
if (
"Invalid key parameters: invalid algorithm" in err_str
# Platform v0.15.0 returns this protobuf validation error for unsupported algorithms
or "key_algorithm_defined" in err_str
):
1 participant
Context
This is a prerequisite fix for PR #461 (
fix/DSPX-3356-xwing-kao-size-assertions), which is failing on two CI jobs:xct (main, go@main)— all PQC tests fail at decrypt (platform bug, unrelated to this PR)xct (v0.15.0, go@v0.32.0)— PQC tests ERROR instead of SKIP ← fixed hereWhat this fixes
1.
tdfs.py— wrong version threshold formechanism-xwing/mechanism-secpmlkemmechanism-xwingwas gated at>= (0, 14, 0), but platform v0.15.0 rejectshpqt:xwingkey creation with:{"message": "Failed to create kas key: invalid_argument: validation error:\n - key_algorithm: The key_algorithm must be one of the defined values. [key_algorithm_defined]"}This means v0.15.0 passes the version check, reaches key creation, fails, and — because the error string doesn't match the existing guard — crashes the fixture with
AssertionErrorinstead of callingpytest.skip.Raised threshold to
(0, 16, 0), the expected next minor release after the commit that added hpqt:* key management support (one commit pastservice/v0.15.0).2.
otdfctl.py— missing error string inInvalidAlgorithmdetectionkas_registry_create_keyonly recognised"Invalid key parameters: invalid algorithm"as an unsupported-algorithm signal. Platform v0.15.0 returns a protobuf field-validation error containing"key_algorithm_defined"instead. Without the match, the error fell through toassert process.returncode == 0, turning what should be apytest.skipinto a testERROR.Added
"key_algorithm_defined"as a second trigger forInvalidAlgorithm.What this does NOT fix
The
xct (main, go@main)failures: all PQC tests encrypt successfully but fail at decrypt — every SDK (go, java) returns exit 1. That is the underlying platform bug (DSPX-3356): the KAS atmainencodes thewrappedKeyusingec:secp384r1wrapping rather than raw X-Wing KEM encapsulation, producing a ciphertext no SDK can unwrap. That fix belongs inopentdf/platform, not the test suite.Test plan
xct (v0.15.0, go@v0.32.0): alltest_pqc.pycases SKIP cleanly (no ERROR)xct (main, go@main): no change expected (decrypt failures are a platform bug)🤖 Generated with Claude Code