Prep wolfProvider 1.2.1 release#451
Conversation
aidangarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: REQUEST_CHANGES
Findings: 7 total — 7 posted, 0 skipped
6 finding(s) posted as inline comments (see file-level comments below)
1 finding(s) not tied to a diff line (full detail below)
Posted findings
- [High] Release notes omit PR 429, a 6-file source change routing KDF key-exchange and MAC-signature through wolfProvider —
ChangeLog.md:9-29 - [Medium] Release notes omit PR 339 RSA key validation and import verification (src/wp_rsa_kmgmt.c, +292 lines) —
ChangeLog.md:9-29 - [Medium] Undocumented DH minimum prime size enforcement (commit 3747ded) shipped with no PR reference —
ChangeLog.md:17-29 - [Medium] PR 431 entry documents only the TLS1 PRF fix, omitting the FIPS module failure reporting change in the same PR —
ChangeLog.md:22 - [Low] PR 413 entry describes only a performance regression; the PR also fixed a FIPS CAST race —
ChangeLog.md:18 - [Low] PR 425 FIPS PILOT DH_MIN_SIZE build fix not listed —
ChangeLog.md:12-15
Findings not tied to a diff line
Stale PR 399 entry in the 1.1.1 section now duplicates the new 1.2.1 entry
File: ChangeLog.md:10,77
Function: N/A (release notes)
Severity: Medium
The new 1.2.1 section correctly credits ML-KEM/ML-DSA to PR 399 at line 10. However line 77, inside the already-shipped # wolfProvider version 1.1.1 (February 09, 2026) section, carries a near-identical bullet: * Add ML-KEM (FIPS 203) and ML-DSA (FIPS 204) post-quantum algorithm support via --enable-pqc (PR 399). PR 399 merged on 2026-06-29 (1d63d90), long after v1.1.1 (Feb 2026) and after v1.2.0 (2026-06-08). git show v1.1.1:ChangeLog.md and git show v1.2.0:ChangeLog.md both contain no reference to PR 399 — the line was introduced by the PQC branch (git blame → boundary commit 662dfb5, dated 2026-06-05) writing into what was then the topmost section, and it got buried when 1.2.0 was cut. The 1.2.1 entry is the correct one; the 1.1.1 entry is false. This PR doesn't create the stale line, but it makes it actively misleading: as published, the ChangeLog announces the same PQC feature in two different releases, so a reader diffing 1.1.1 against 1.2.1 cannot tell which release actually shipped it. This release PR is the natural place to correct it.
Recommendation: Delete line 77 from the 1.1.1 section. Keep the line-10 entry in 1.2.1, which matches the actual merge date of PR 399.
Referenced code: ChangeLog.md:10,77 (7 lines)
Review generated by Skoll
79e0f4e to
9168000
Compare
Summary:
Testing: