feat(pq-threshold-seal): add cross-language threshold envelopes - #34
feat(pq-threshold-seal): add cross-language threshold envelopes#34exocognosis wants to merge 1 commit into
Conversation
Greptile SummaryAdds an experimental cross-language threshold-sealing package.
Confidence Score: 4/5The PR appears safe to merge, with non-blocking secret-cleanup and TypeScript declaration consistency issues to address. The threshold arithmetic, canonical codec, and cross-language derivations are aligned, while two temporary-secret paths retain sensitive material longer than intended and the exported TypeScript shapes diverge from repository convention. Files Needing Attention: packages/pq-threshold-seal/ts/src/index.ts; packages/pq-threshold-seal/rust/src/lib.rs Important Files Changed
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/pq-threshold-seal/ts/src/index.ts:549-563
**Shamir coefficients remain uncleared**
`splitSecret` allocates the random polynomial coefficients but never overwrites them; only the generated shares are cleared by the caller. This leaves sensitive material capable of deriving every share in JavaScript heap memory until garbage collection.
### Issue 2 of 3
packages/pq-threshold-seal/rust/src/lib.rs:297-298
**Authentication failures retain the KEK**
The `?` on `decrypt` returns before `kek.zeroize()` when a tampered share or wrong context fails authentication. As a result, this expected failure path leaves the derived wrapping key in stack memory rather than clearing it.
### Issue 3 of 3
packages/pq-threshold-seal/ts/src/index.ts:28-35
**Plain data shapes use interfaces**
`BindingContext` and the other exported DTO-like structures are plain readonly data shapes without extension or implementation requirements. Define these as type aliases to follow the repository's TypeScript convention consistently.
Reviews (1): Last reviewed commit: "feat(pq-threshold-seal): add cross-langu..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function splitSecret(secret: Uint8Array, threshold: Threshold, rng: RandomBytes): Uint8Array[] { | ||
| const coefficients = Array.from({ length: threshold.k - 1 }, () => takeRandom(rng, KEY_SIZE)); | ||
| return Array.from({ length: threshold.n }, (_, position) => { | ||
| const index = position + 1; | ||
| const value = Uint8Array.from(secret); | ||
| for (let byte = 0; byte < KEY_SIZE; byte += 1) { | ||
| let power = index; | ||
| for (const coefficient of coefficients) { | ||
| value[byte] ^= gfMultiply(coefficient[byte], power); | ||
| power = gfMultiply(power, index); | ||
| } | ||
| } | ||
| return value; | ||
| }); | ||
| } |
There was a problem hiding this comment.
splitSecret allocates the random polynomial coefficients but never overwrites them; only the generated shares are cleared by the caller. This leaves sensitive material capable of deriving every share in JavaScript heap memory until garbage collection.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-threshold-seal/ts/src/index.ts
Line: 549-563
Comment:
**Shamir coefficients remain uncleared**
`splitSecret` allocates the random polynomial coefficients but never overwrites them; only the generated shares are cleared by the caller. This leaves sensitive material capable of deriving every share in JavaScript heap memory until garbage collection.
How can I resolve this? If you propose a fix, please make it concise.| let mut plaintext = decrypt(&kek, &nonce, &recipient.wrapped_share, &aad)?; | ||
| kek.zeroize(); |
There was a problem hiding this comment.
The ? on decrypt returns before kek.zeroize() when a tampered share or wrong context fails authentication. As a result, this expected failure path leaves the derived wrapping key in stack memory rather than clearing it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-threshold-seal/rust/src/lib.rs
Line: 297-298
Comment:
**Authentication failures retain the KEK**
The `?` on `decrypt` returns before `kek.zeroize()` when a tampered share or wrong context fails authentication. As a result, this expected failure path leaves the derived wrapping key in stack memory rather than clearing it.
How can I resolve this? If you propose a fix, please make it concise.| export interface BindingContext { | ||
| /** Application or protocol domain. */ | ||
| readonly domain: Uint8Array; | ||
| /** Session or transaction identifier. */ | ||
| readonly session: Uint8Array; | ||
| /** Application-computed hash of the ordered recipient roster. */ | ||
| readonly rosterHash: Uint8Array; | ||
| } |
There was a problem hiding this comment.
BindingContext and the other exported DTO-like structures are plain readonly data shapes without extension or implementation requirements. Define these as type aliases to follow the repository's TypeScript convention consistently.
Rule Used: Use type instead of interface for DTOs and sim... (source)
Learned From
cytonic-network/ai-frontend#48
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/pq-threshold-seal/ts/src/index.ts
Line: 28-35
Comment:
**Plain data shapes use interfaces**
`BindingContext` and the other exported DTO-like structures are plain readonly data shapes without extension or implementation requirements. Define these as type aliases to follow the repository's TypeScript convention consistently.
**Rule Used:** Use `type` instead of `interface` for DTOs and sim... ([source](https://app.greptile.com/multivm-labs/-/custom-context?memory=2b2a7a55-162e-44b9-8c4c-3f52514f7037))
**Learned From**
[cytonic-network/ai-frontend#48](https://github.com/cytonic-network/ai-frontend/pull/48)
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Adds
pq-threshold-seal, an experimental Rust and TypeScript package for sealing one byte payload to an ordered ML-KEM-768 committee withk-of-nrecovery.The implementation encrypts the payload once with a random ChaCha20-Poly1305 key, splits that key with GF(256) Shamir secret sharing, and wraps one share for each recipient using a key derived from an ML-KEM-768 shared secret. This keeps body encryption independent of committee size while limiting each recipient record to one ML-KEM ciphertext and one encrypted 32-byte share.
Package(s)
pq-threshold-sealLanguages
Proposed solution
Threshold sealing
nshares using independent Shamir polynomials over the Rijndael GF(256) field.kdistinct shares to reconstruct the body key.Context and roster binding
The caller supplies a domain, session identifier, and ordered roster hash. The package uses a canonical length-prefixed context in the key commitment, wrapping-key derivation, nonce derivation, and authenticated data. Recipient indexes, threshold parameters, the body-key commitment, and each ML-KEM ciphertext are also authenticated.
This makes envelopes specific to their protocol domain, session, threshold, recipient position, and roster. Applications remain responsible for authenticating recipient public keys, calculating the ordered roster hash, issuing unique session identifiers, and enforcing replay policy.
Canonical envelope format
The new
PQTSversion 1 format contains:kandnthreshold valuesnfixed-size recipient records in ascending one-based orderDecoders reject unknown versions, trailing bytes, invalid thresholds, noncanonical recipient order, wrong field lengths, short authentication tags, and body ciphertexts over 16 MiB.
Cross-language consistency
Rust and TypeScript expose equivalent sealing, share recovery, body reconstruction, and envelope codec APIs. A shared deterministic vector supplies:
Both implementations reproduce the same 3,503-byte envelope fingerprint and recover the same plaintext from recipients 1 and 3.
Key validation and secret handling
Validation
cargo +1.78.0 clippy --manifest-path packages/pq-threshold-seal/rust/Cargo.toml --all-targets --all-features -- -D warningscargo +1.78.0 test --manifest-path packages/pq-threshold-seal/rust/Cargo.toml --all-featurescargo +1.78.0 build --manifest-path packages/pq-threshold-seal/rust/Cargo.toml --no-default-featuresbun run --cwd packages/pq-threshold-seal/ts buildbun test packages/pq-threshold-seal/ts/testscargo package -p pq-threshold-seal --allow-dirtynpm pack --dry-run --workspace pq-threshold-sealThe Rust and TypeScript suites cover round trips, canonical encoding, shared-vector compatibility, wrong context, duplicate shares, body tampering, malformed thresholds, malformed envelopes, noncanonical public keys, and corrupt private keys.
Checklist
Related Issues
Fixes #32