Skip to content

Miscellaneous test coverage and API clarity improvements - #5

Closed
danieldia-dev wants to merge 5 commits into
symbolicsoft:mainfrom
danieldia-dev:main
Closed

Miscellaneous test coverage and API clarity improvements#5
danieldia-dev wants to merge 5 commits into
symbolicsoft:mainfrom
danieldia-dev:main

Conversation

@danieldia-dev

@danieldia-dev danieldia-dev commented May 22, 2026

Copy link
Copy Markdown
Contributor

This PR makes three small independent improvements:

  1. InsecurePsk is renamed to PskTooShort (with the old variant deprecated) because the original name implied the crate validates entropy, which in fact it does not (since the check is purely a length proxy).

  2. A sequential seal/open stress test is added to context.rs that runs 10,000 consecutive seal/open pairs and asserts every ciphertext is unique, directly verifying that nonce derivation does not produce a repeated value across sequential operations.

  3. Three trybuild compile-fail tests are added to lock in type-system invariants that were previously untested: Context is not Clone, ExportOnly cannot be used with seal_base, and PQ KEMs cannot be used with seal_auth. A regression in any of these previously would have silently widened the API.

All compile-fail tests pass under cargo test --features pq compile_fail.

Note: A follow-up fix merges the PskTooShort and InsecurePsk display arms into a single match arm to satisfy clippy::match_same_arms, which is enforced by the crate's clippy::pedantic deny list.

@nadimkobeissi

nadimkobeissi commented May 22, 2026

Copy link
Copy Markdown
Member

InsecurePsk is renamed to PskTooShort (with the old variant deprecated) because the original name implied the crate validates entropy, which in fact it does not (since the check is purely a length proxy).

The PSK is randomly generated, so it's reasonable to assume that it's uniformly random. If it's uniformly random, then its length becomes the only measure of entropy.

A sequential seal/open stress test is added to context.rs that runs 10,000 consecutive seal/open pairs and asserts every ciphertext is unique, directly verifying that nonce derivation does not produce a repeated value across sequential operations.

This is is not a good test. Ideally, you'd test the correctness of the individual components, not do a thing that just runs the same operation 10,000 times.

Three trybuild compile-fail tests are added to lock in type-system invariants that were previously untested: Context is not Clone, ExportOnly cannot be used with seal_base, and PQ KEMs cannot be used with seal_auth. A regression in any of these previously would have silently widened the API.

OK, this is good, could you open a separate PR with this just this change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants