fix(pem)!: OpenSSL header delimiter + audit citation sweep - #89
Merged
Conversation
parseTraditionalPem keyed on ': ', so a conformant encapsulated header written 'DEK-Info:AES-256-CBC,...' ended the header scan early and folded into the base64 body. Split on the first ':' and trim the value. Also correct the spec citations the audit flagged, all comment-only: - pkcs7 module doc scopes the RFC 2315 support claim; curve/digest pairing splits RFC 5753 §8 (curves) from RFC 5754 §2 (digests). - PKCS#10 version doc notes the encoded INTEGER is 0 (RFC 2986 §4.1). - inhibitAnyPolicy counts non-self-issued certificates (RFC 5280 §4.2.1.14). - rfc822Name cites RFC 2821 §4.1.2 Mailbox; the DN emailAddress cites PKCS #9 / RFC 2985 §5.2.1; name shorthand drops the X.501 attribution. - OCSP CertID SHA-1 cites RFC 5019 §2.1.1 (RFC 6960 defines no default); the responder attribute map names the X.520 id-at arc plus PKCS #9. - identity cites RFC 9525 with the CN fallback scoped to RFC 6125 §6.4.4; userConstrainedPolicies documents the RFC 9618 §5.5 (g)(5)-(6) superset. - SEC1 ECPrivateKey quotes the constrained RFC 5915 §3 syntax; PBKDF2-params restates the RFC 8018 A.2 salt CHOICE and DEFAULT prf. - PrintableString cites X.680 §41.4 Table 10; OID arcs split X.660 (root) from X.690 §8.19.4 (the <40 packing). Vendor the RFCs these citations reference and add an `rfc` fetch helper.
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
🚫 Excluded labels (none allowed) (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis change adds RFC source files and an RFC-fetching command, preserves PKCS#7 certificate choices, and replaces flattened signer identifiers with discriminated unions. It introduces discriminated models for CRL distribution points, issuing scopes, OCSP statuses, and self-signed key sources. Traditional PEM headers now accept colon-delimited values without spaces while validating names and duplicates. Exhaustiveness guards, public type exports, documentation, compiler settings, and corresponding tests are updated. Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
Comment |
commit: |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
micro509 | b7ec74e | Commit Preview URL Branch Preview URL |
Jul 25 2026, 10:33 AM |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This comment has been minimized.
This comment has been minimized.
kjanat
added
cr:review
kjanat
marked this pull request as draft
July 25, 2026 02:51
# Conflicts: # CHANGELOG.md # package.json
Reject malformed traditional PEM header names and accept the tagged non-X.509 CertificateChoices CMS permits while rejecting unknown tags. Correct related standards citations and replace the RFC fetch shell snippet with a typed CLI that reports HTTP failures.
Expose every RFC 5652 CertificateChoices alternative instead of silently dropping non-X.509 entries. Keep tagged DER for round-tripping and decode the other-format OID so callers can distinguish mixed certificate sets.
Represent CMS signer identifiers, CRL distribution-point names, and OCSP statuses as discriminated unions. Callers can narrow each wire choice without handling impossible combinations of optional fields.
Fourteen switches over closed unions had no default, so a value from outside the union fell off the end. Four were `assert*` helpers in `keys.ts` returning void, where an unrecognized `algorithm.kind` made the SPKI, PKCS#8, and JWK checks pass silently; the rest returned `undefined` into WebCrypto, DER encoding, or a `Result`. Each now ends in an `_exhaustive` default, which also turns a future variant into a compile error. `CreateSelfSignedCertificateInput` splits into a base and a key-source union. `algorithm` was documented as ignored when `keyPair` is present; supplying both no longer compiles. `ParsedIssuingDistributionPoint` gains the scope union its builder input already had, recording the RFC 5280 §5.2.5 at-most-one rule the parser only enforced at runtime. An explicitly encoded FALSE stays distinct from an omitted flag.
kjanat
marked this pull request as ready for review
July 25, 2026 09:19
coderabbitai
Bot
added
api
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Disallow revocation-only fields on good and unknown OCSP builder inputs, including structurally typed values that bypass excess-property checks. Add compile-time coverage and document the exported type contract migrations in the changelog.
Parse certificate choices through the bounded child iterator so a malformed entry cannot consume bytes from the following SignedData field. Cover the overrun and keep verification fixtures type-safe.
Exercise invalid runtime variants across key, extension, revocation, and crypto helpers so closed-union fallthrough regressions fail explicitly.
coderabbitai
Bot
removed
pkcs
Enable differential verification during the coverage job so CI catches behavioral divergence from OpenSSL on every test run.
kjanat
added
breaking-change
kjanat
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Check `revokedAt` and `revocationReasonCode` separately for good and unknown statuses so either field becoming assignable fails the type suite.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Clears the remaining
_audit.mdtail — the one Low code finding plus the Low citation errors — so nothing but the doc backlog trails 0.14.0.RFC 1421 header delimiter (Low code)
parseTraditionalPemkeyed on': ', so a conformant OpenSSL-style encapsulated header written without the space (DEK-Info:AES-256-CBC,…) ended the header scan on its first line and folded the headers into the base64 body, failing with "Traditional PEM encryption headers missing". It now splits on the first:and trims the value. Per the audit, the RFC 1421 citations are scoped to OpenSSL's format (every other reference in the file already says "OpenSSL-style"); RFC 822 folding is not implemented and the citation not kept.Citation corrections (Low, comment-only)
Each is the audit's prescribed rewording, verified against the vendored spec:
0(RFC 2986 §4.1), normalized to the v1 ordinal1.emailAddresscites PKCS ci: run OpenSSL differential harness in digest-pinned container #9 / RFC 2985 §5.2.1 and its §4.1.2.6 deprecation; the name-shorthand doc drops the incorrect X.501 attribution.id-atarc plus legacy PKCS ci: run OpenSSL differential harness in digest-pinned container #9.userConstrainedPoliciesdocuments the RFC 9618 §5.5 (g)(5)-(6) superset relationship.specifiedalternative is accepted.(X*40)+Ypacking that bounds the second arc< 40).Two audit citation findings — the name-constraints-engine emailAddress-fallback trigger and dNSName leading-period — were already at the audit's target wording on master; no change, deleted from the audit.
Vendored specs
The RFCs these citations reference are vendored (2315, 2821, 2986, 5019, 5753, 5754, 8419, 9525), and an
RFC=<n> run rfcfetch helper is added.Tests
A no-space-after-colon variant of the encrypted-PEM round-trip (
Proc-Type:…/DEK-Info:…) imports and matches the original key. Comment-only citation changes have no behavioral test surface.Gate: typecheck, biome, docs:lint, 1381 tests, 11 OpenSSL differential.