chore: 0.48.0 — rustls crypto provider selection via cargo features#732
Merged
Conversation
…ures
Reqwest 0.13's `rustls` feature hardcodes aws-lc-rs as the crypto provider,
which conflicts with workspaces (e.g. dfinity/ic) that still pull in
reqwest 0.12 with ring: Cargo unifies rustls 0.23 with both `ring` and
`aws_lc_rs` features active, and `CryptoProvider::from_crate_features()`
returns None, breaking any direct rustls user that relies on the
process-default builder pattern.
Switch ic-agent's reqwest feature from `rustls` to `rustls-no-provider`,
take a direct optional `rustls` dep, and expose two new features:
- `tls-aws-lc-rs` (default): installs aws-lc-rs as the process-wide
rustls default in the Agent::new default-client path
- `tls-ring`: installs ring instead
Features are additive: when both are on, aws-lc-rs wins (enabling
`tls-ring` while the default is on never silently flips behavior).
Downstreams who need ring opt out of defaults:
`default-features = false, features = ["pem", "tls-ring"]`.
Tests cover three scenarios in process-isolated integration test files:
matching-provider install, idempotency when an application installs
first, and the expected panic when neither feature is enabled.
CI runs the default (aws-lc-rs), --all-features (both on, aws-lc-rs
wins), tls-ring alone, and the no-TLS panic path; the previous blanket
`--no-default-features` pass is dropped (no-op for crates without
features, broken for ic-agent without a provider).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `http_transport` module (`ReqwestTransport`, `AgentBuilder::with_transport`, `AgentBuilder::with_arc_transport`) has been deprecated since v0.38.0 in favor of the dedicated builder methods (`with_url`, `with_http_client`, `with_arc_route_provider`, `with_max_response_body_size`, `with_max_tcp_error_retries`). The original comment in `agent/mod.rs` already slated it for removal "after 0.40"; 0.48.0 is the natural breakage window alongside the rustls provider feature changes. No workspace or dfinity/sdk consumer references these symbols. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bump workspace version 0.47.3 → 0.48.0 and add a CHANGELOG entry covering: - new `tls-aws-lc-rs` (default) / `tls-ring` cargo features for rustls crypto provider selection - breaking: `default-features = false` now requires an explicit TLS feature (or `with_http_client`) to avoid a runtime panic - breaking: removal of the long-deprecated `http_transport` module Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…talled `install_default_crypto_provider` was previously building a fresh `CryptoProvider` on every `Agent::new` only to discard it when `install_default()` returned `Err` (default already set). Fast-path the common warm case with `CryptoProvider::get_default().is_some()`. The check has a benign TOCTOU race: `install_default()` is itself atomic, so concurrent installers still produce a single winner. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds explicit rustls crypto-provider selection to ic-agent via Cargo features to avoid provider conflicts caused by mixed reqwest versions (0.12 ring vs 0.13 aws-lc-rs). It switches reqwest to rustls-no-provider and makes ic-agent install the chosen rustls::crypto::CryptoProvider on the default-client construction path, removes the long-deprecated http_transport module, adds integration tests for feature combinations, and bumps the workspace to 0.48.0 with corresponding changelog/CI updates.
Changes:
- Add
tls-aws-lc-rs(default) /tls-ringfeatures and install the selected rustls provider when building the default reqwest client. - Remove deprecated
ic-agent::agent::http_transportlegacy transport API. - Add integration tests + CI steps to exercise provider-selection scenarios; bump versions and update CHANGELOG.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
ic-agent/src/agent/mod.rs |
Installs a process-wide rustls CryptoProvider based on Cargo features before building the default reqwest client; removes http_transport module export. |
ic-agent/Cargo.toml |
Introduces tls-aws-lc-rs/tls-ring features, switches reqwest to rustls-no-provider, adds optional rustls dependency. |
ic-agent/tests/crypto_provider_default.rs |
Tests that ic-agent installs the expected default provider under enabled TLS feature(s). |
ic-agent/tests/crypto_provider_idempotent.rs |
Tests that ic-agent does not override an application-installed provider. |
ic-agent/tests/crypto_provider_neither.rs |
Tests the expected panic path when no TLS provider feature is enabled. |
ic-agent/src/agent/http_transport/mod.rs |
Removes deprecated legacy transport module (deleted). |
ic-agent/src/agent/http_transport/reqwest_transport.rs |
Removes deprecated legacy transport implementation (deleted). |
.github/workflows/test.yml |
Adjusts CI test matrix to cover TLS-feature combinations and the “no TLS” panic test. |
CHANGELOG.md |
Documents 0.48.0 changes and breaking migrations (TLS feature selection + module removal). |
Cargo.toml |
Bumps workspace version to 0.48.0 and adds workspace rustls dependency version. |
Cargo.lock |
Updates lockfile for version bump and dependency graph changes (incl. rustls). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… default features The default `tls-aws-lc-rs` feature pulled in aws-lc-sys, which fails to cross-compile to wasm32-unknown-unknown. Move the rustls dep into the non-wasm target table so wasm consumers can use default features unchanged. The runtime crypto-provider install was already `cfg(not(target_family = "wasm"))`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The customized-instantiation example builds a reqwest::Client directly instead of going through Agent::new, so reqwest's rustls-no-provider mode panicked with "No provider set". Install the aws-lc-rs provider at the start of main and note why. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ref-tests depended on ic-agent with only the pem feature, which compiles neither tls-aws-lc-rs nor tls-ring. install_default_crypto_provider then becomes a no-op, and Agent::new's reqwest client panics with "No provider set" because use_rustls_tls() builds the TLS config eagerly even though pocket-ic is reached over plain HTTP. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reorganize the test matrix into discrete steps (workspace --all-features, ic-agent default, tls-ring only, no-TLS panic path, WASM, ref-tests, SoftHSM) with a header comment summarizing which step covers what. Switch ref-tests invocations from `cd ref-tests && cargo test` to `cargo test -p ref-tests` so they run from the workspace root. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The provider-identity assertions cast `&'static dyn SecureRandom` to `*const ()`, discarding the vtable. Two distinct ZST-backed impls would then alias on the data pointer alone. Compare the full wide pointer (data + vtable) so type identity is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The step used a backslash line-continuation inside a YAML plain scalar. Plain scalars fold newlines into spaces but don't interpret escapes, so the shell received a literal `\` followed by a space — escaping the space into a single literal-space word rather than continuing the command. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…p-store flakes The apt `chromium-chromedriver` package on Ubuntu 24.04 is a transitional shim that installs Chromium from the snap store, which intermittently times out fetching assertions from api.snapcraft.io and fails the job. Switch to browser-actions/setup-chrome, which downloads Chrome and a matching chromedriver directly without going through snap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…void snap-store flakes" This reverts commit 4dd17ee.
adamspofford-dfinity
approved these changes
May 21, 2026
4 tasks
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
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.
Context
Reqwest 0.13's
rustlsfeature hardcodes aws-lc-rs as the crypto provider. In workspaces that also pull in reqwest 0.12 with ring (e.g. dfinity/ic), Cargo unifies rustls 0.23 with bothringandaws_lc_rsfeatures active,CryptoProvider::from_crate_features()returnsNone, and any direct rustls user relying on the process-defaultClientConfig::builder()panics. The colleague hitting this asked us to give ic-agent a knob.Outline
tls-aws-lc-rs(default) andtls-ring; switch reqwest torustls-no-providerand install the chosen provider on the default-client path. Additive: aws-lc-rs wins when both are enabled. Application-installed providers are not overwritten.http_transportmodule (ReqwestTransport,with_transport,with_arc_transport— deprecated since 0.38.0).Stock-default users see no behavior change — aws-lc-rs has been the only provider available since 0.46.0. Consumers using
default-features = falseneed a one-line edit to enable a TLS feature; the CHANGELOG entry covers the migration.