Skip to content

fix(deps): update rcgen to 0.14#349

Merged
jdx merged 1 commit into
mainfrom
fix/rcgen-0.14
Apr 11, 2026
Merged

fix(deps): update rcgen to 0.14#349
jdx merged 1 commit into
mainfrom
fix/rcgen-0.14

Conversation

@jdx

@jdx jdx commented Apr 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • Updates rcgen from 0.13 to 0.14
  • Migrates from CertificateParams::from_ca_cert_pem() to Issuer::from_ca_cert_pem() (rcgen 0.14 moved issuer logic into a dedicated Issuer type)
  • signed_by() now takes &Issuer instead of separate &Certificate + &KeyPair args
  • Switched cached cert expiry check to use x509-parser directly instead of rcgen's removed CertificateParams::from_ca_cert_pem

Closes #332

Test plan

  • cargo build succeeds
  • cargo nextest run — 400 passed, 0 failed
  • Proxy TLS cert generation and caching paths exercised by test_e2e_proxy tests

🤖 Generated with Claude Code


Note

Medium Risk
Medium risk because it changes on-the-fly TLS certificate signing and cached-cert validation logic in the proxy, which could break HTTPS interception if parsing/signing behavior differs. Dependency upgrades in the ASN.1/X.509 stack may also affect certificate handling edge cases.

Overview
Upgrades the proxy TLS stack to rcgen 0.14 and updates the SNI cert resolver to use rcgen::Issuer for CA-based leaf signing (replacing the previous reconstructed Certificate + KeyPair issuer flow).

Adds x509-parser to the proxy-tls feature and switches the on-disk cached certificate expiry check to parse the first cert’s not_after directly via x509-parser, avoiding removed/changed rcgen APIs. Lockfile updates reflect the bumped ASN.1/X.509-related transitive dependencies (asn1-rs, der-parser, oid-registry, x509-parser).

Reviewed by Cursor Bugbot for commit b485fd2. Bugbot is set up for automated code reviews on this repo. Configure here.

rcgen 0.14 moved `CertificateParams::from_ca_cert_pem()` to
`Issuer::from_ca_cert_pem()` and `signed_by()` now takes an `&Issuer`
instead of separate cert + key args. Also switched the cached cert
expiry check from rcgen params parsing to x509-parser directly.

Closes #332

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Apr 11, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR upgrades rcgen from 0.13 to 0.14.7, migrating to the new Issuer-centric API: Issuer::from_ca_cert_pem() replaces CertificateParams::from_ca_cert_pem(), and signed_by() now takes &Issuer directly. The cert-expiry check in load_from_disk is replaced with direct x509-parser usage since the rcgen helper that was previously (mis)used for this purpose was removed in 0.14.

Confidence Score: 5/5

Safe to merge — clean dependency upgrade with correct API migration and passing tests.

All findings are P2 or lower. The rcgen 0.14 API is used correctly (Issuer::from_ca_cert_pem, signed_by(&leaf_key, &self.issuer)), the Issuer<'static, KeyPair> lifetime is appropriate for a 'static-required rustls resolver, the x509-parser expiry check is a straightforward replacement, and both version bounds are consistent in Cargo.lock. Tests pass per the PR description.

No files require special attention.

Important Files Changed

Filename Overview
Cargo.toml Bumps rcgen to 0.14, adds the x509-parser feature to rcgen, and retains the direct x509-parser = "0.18" dependency; version bounds are consistent with Cargo.lock resolution to 0.18.1.
Cargo.lock Lock file updated to rcgen 0.14.7 and x509-parser 0.18.1; single version of x509-parser shared between rcgen and the direct dep — no duplicate resolution issues.
src/proxy/server.rs Correctly migrates to rcgen::Issuer::from_ca_cert_pem() and signed_by(&leaf_key, &self.issuer); expiry check now uses x509-parser directly; Issuer<'static, KeyPair> lifetime is correct since SniCertResolver must be 'static for rustls.

Sequence Diagram

sequenceDiagram
    participant TLS as TLS Connection (SNI)
    participant SNI as SniCertResolver
    participant L1 as L1 Memory Cache
    participant L2 as L2 Disk Cache
    participant rcgen as rcgen 0.14 (Issuer)
    participant x509 as x509-parser

    TLS->>SNI: resolve(domain)
    SNI->>L1: check cache
    alt cache hit
        L1-->>SNI: CertifiedKey
    else cache miss
        SNI->>L2: load_from_disk(path)
        alt disk hit
            L2->>x509: parse_x509_certificate(der)
            x509-->>L2: validity.not_after.timestamp()
            alt not expired
                L2-->>SNI: CertifiedKey
            else expired
                L2-->>SNI: Err (regenerate)
            end
        end
        SNI->>rcgen: CertificateParams::signed_by(&leaf_key, &issuer)
        note over rcgen: Issuer loaded via Issuer::from_ca_cert_pem() at startup
        rcgen-->>SNI: leaf Certificate
        SNI->>L1: insert(domain, CertifiedKey)
        SNI->>L2: persist combined PEM (0600)
    end
    SNI-->>TLS: CertifiedKey
Loading

Reviews (1): Last reviewed commit: "fix(deps): update rcgen to 0.14" | Re-trigger Greptile

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates several dependencies, including an upgrade of rcgen to version 0.14 and the addition of x509-parser. The SniCertResolver has been refactored to use the rcgen::Issuer type, which simplifies CA loading and leaf certificate signing. Additionally, the manual PEM parsing previously used for certificate expiry checks has been replaced with x509-parser. Feedback is provided regarding the error handling of the x509_parser result to ensure more descriptive error messages in the event of a parsing failure.

Comment thread src/proxy/server.rs
Comment on lines +688 to 690
let (_, cert) = x509_parser::parse_x509_certificate(&cert_ders[0]).map_err(|e| {
miette::miette!("Failed to parse certificate from {}: {e}", path.display())
})?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The x509_parser::parse_x509_certificate function returns a nom::IResult, which contains a tuple of the remaining input and the parsed certificate. While ignoring the remaining input is acceptable here, the error type returned by nom parsers is nom::Err<X509Error>. To provide a more descriptive error message in the miette report, you might want to explicitly handle the nom::Err variants or ensure that the Display implementation of the error provides enough context.

@jdx jdx enabled auto-merge (squash) April 11, 2026 13:45
@jdx jdx merged commit f66afcf into main Apr 11, 2026
6 checks passed
@jdx jdx deleted the fix/rcgen-0.14 branch April 11, 2026 13:46
@jdx jdx mentioned this pull request Apr 11, 2026
jdx added a commit that referenced this pull request Apr 12, 2026
## 🤖 New release

* `pitchfork-cli`: 2.5.0 -> 2.6.0

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [2.6.0](v2.5.0...v2.6.0) -
2026-04-12

### Added

- *(proxy)* auto start when visiting the proxied URL
([#347](#347))

### Fixed

- some issues related to sudo supervisor
([#323](#323))
- *(port)* should fail when ready_port is in use
([#350](#350))
- *(deps)* update rcgen to 0.14
([#349](#349))
- *(deps)* update reqwest to 0.13
([#348](#348))
- detect port conflicts on loopback addresses, not just 0.0.0.0
([#345](#345))
- narrow REAPED_STATUSES cfg to non-Linux unix only
([#346](#346))
- *(deps)* update rust crate ratatui to 0.30
([#331](#331))
- *(deps)* update rust crate toml to v1
([#344](#344))
- *(deps)* update rust crate strum to 0.28
([#334](#334))
- *(deps)* update rust crate notify-debouncer-full to 0.7
([#330](#330))
- *(deps)* update rust crate nix to 0.31
([#329](#329))
- *(deps)* update rust crate listeners to 0.5
([#328](#328))
- *(deps)* update rust crate sysinfo to 0.38
([#335](#335))
- *(deps)* update rust crate cron to 0.16
([#324](#324))
- *(deps)* update rust crate crossterm to 0.29
([#325](#325))

### Other

- *(deps)* update rust crate rmcp to v1.4.0
([#327](#327))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Low risk: this PR only bumps the crate version and updates release
notes, with no runtime code changes.
> 
> **Overview**
> Prepares the `pitchfork-cli` **v2.6.0** release by bumping the package
version from `2.5.0` to `2.6.0` in `Cargo.toml`/`Cargo.lock`.
> 
> Updates `CHANGELOG.md` with the `2.6.0` release notes (proxy
auto-start behavior, several fixes, and dependency updates).
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
faea6c5. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant