From b36e70cca1265cfeded2f2846a4382a6dba0ac36 Mon Sep 17 00:00:00 2001 From: Jo <10510431+j178@users.noreply.github.com> Date: Mon, 26 Oct 2026 22:11:14 +0800 Subject: [PATCH] Make custom CA certificates replace default roots --- Cargo.lock | 4 +- Cargo.toml | 2 +- crates/prek-consts/src/env_vars.rs | 5 +- crates/prek/src/checksum.rs | 4 +- crates/prek/src/cli/self_update.rs | 4 +- crates/prek/src/http.rs | 126 +++++++++++++----- crates/prek/src/languages/deno/installer.rs | 4 +- crates/prek/src/languages/dotnet/installer.rs | 4 +- crates/prek/src/languages/golang/installer.rs | 4 +- crates/prek/src/languages/node/installer.rs | 4 +- crates/prek/src/languages/python/uv.rs | 6 +- crates/prek/src/languages/ruby/installer.rs | 4 +- crates/prek/tests/http.rs | 54 ++++++++ docs/reference/environment-variables.md | 17 ++- 14 files changed, 186 insertions(+), 56 deletions(-) create mode 100644 crates/prek/tests/http.rs diff --git a/Cargo.lock b/Cargo.lock index 7a1183536..c2c64a4b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2610,9 +2610,9 @@ dependencies = [ [[package]] name = "ssl-certs" -version = "0.1.1" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2bec6ae64c07c77e2f158e26315a42be873612fa4a0b94104ffaf5a165df4a40" +checksum = "3c4fe8e658dd9ac6d14e0280696918b384c1940067ed6e74425cbb5d12bb8f8e" dependencies = [ "rustls-pki-types", ] diff --git a/Cargo.toml b/Cargo.toml index 107b4985c..9e5992833 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -74,7 +74,7 @@ serde_json = { version = "1.0.132", features = ["preserve_order", "unbounded_dep serde_stacker = { version = "0.1.12" } shlex = { version = "2.0.0" } simdutf8 = { version = "0.1.5" } -ssl-certs = { version = "0.1.1" } +ssl-certs = { version = "0.2.0" } strum = { version = "0.28.0", features = ["derive"] } target-lexicon = { version = "0.13.0" } tempfile = { version = "3.25.0" } diff --git a/crates/prek-consts/src/env_vars.rs b/crates/prek-consts/src/env_vars.rs index a9d22068f..e7a1023dd 100644 --- a/crates/prek-consts/src/env_vars.rs +++ b/crates/prek-consts/src/env_vars.rs @@ -97,6 +97,7 @@ impl EnvVars { pub const PREK_NO_FAST_PATH: &'static str = "PREK_NO_FAST_PATH"; pub const PREK_UV_SOURCE: &'static str = "PREK_UV_SOURCE"; pub const PREK_UV_VERSION: &'static str = "PREK_UV_VERSION"; + pub const PREK_SYSTEM_CERTS: &'static str = "PREK_SYSTEM_CERTS"; pub const PREK_NATIVE_TLS: &'static str = "PREK_NATIVE_TLS"; pub const PREK_DOWNLOAD_CHECKSUM_POLICY: &'static str = "PREK_DOWNLOAD_CHECKSUM_POLICY"; pub const SSL_CERT_FILE: &'static str = "SSL_CERT_FILE"; @@ -307,7 +308,7 @@ mod tests { let env_vars = EnvVars::from_map(&[ (EnvVars::PREK_DOCKER_NO_INIT, "yes"), - (EnvVars::PREK_NATIVE_TLS, "0"), + (EnvVars::PREK_SYSTEM_CERTS, "0"), (EnvVars::PREK_ALLOW_NO_CONFIG, "maybe"), ]); assert_eq!( @@ -315,7 +316,7 @@ mod tests { Ok(Some(true)) ); assert_eq!( - env_vars.var_as_bool(EnvVars::PREK_NATIVE_TLS), + env_vars.var_as_bool(EnvVars::PREK_SYSTEM_CERTS), Ok(Some(false)) ); assert_eq!( diff --git a/crates/prek/src/checksum.rs b/crates/prek/src/checksum.rs index 4bb8c6a52..1d98bbf10 100644 --- a/crates/prek/src/checksum.rs +++ b/crates/prek/src/checksum.rs @@ -7,7 +7,7 @@ use anyhow::{Context, Result, bail}; use aws_lc_rs::digest::{Context as Sha256Context, SHA256}; use tokio::io::{AsyncRead, ReadBuf}; -use crate::http::REQWEST_CLIENT; +use crate::http::reqwest_client; #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub(crate) struct Sha256Digest([u8; 32]); @@ -98,7 +98,7 @@ pub(crate) async fn fetch_checksum_with( checksum_url: &str, customize_request: impl FnOnce(reqwest::RequestBuilder) -> reqwest::RequestBuilder, ) -> Result> { - let response = customize_request(REQWEST_CLIENT.get(checksum_url)) + let response = customize_request(reqwest_client()?.get(checksum_url)) .send() .await .with_context(|| format!("Failed to fetch checksum from {checksum_url}"))?; diff --git a/crates/prek/src/cli/self_update.rs b/crates/prek/src/cli/self_update.rs index 0119d1c4b..b4d75783c 100644 --- a/crates/prek/src/cli/self_update.rs +++ b/crates/prek/src/cli/self_update.rs @@ -28,7 +28,7 @@ use prek_update::{UpdateRequest, Updater}; use tracing::{debug, enabled}; use crate::cli::ExitStatus; -use crate::http::REQWEST_CLIENT; +use crate::http::reqwest_client; use crate::install_source::InstallSource; use crate::printer::Printer; @@ -106,7 +106,7 @@ pub(crate) async fn self_update( // available version of prek. match updater .run( - &REQWEST_CLIENT, + reqwest_client()?, update_request, token.as_deref(), enabled!(tracing::Level::DEBUG), diff --git a/crates/prek/src/http.rs b/crates/prek/src/http.rs index 1834c0a9b..db99a0405 100644 --- a/crates/prek/src/http.rs +++ b/crates/prek/src/http.rs @@ -114,7 +114,7 @@ async fn download_to_temp_file( debug!(%url, "Downloading"); - let response = customize_request(REQWEST_CLIENT.get(url)) + let response = customize_request(reqwest_client()?.get(url)) .send() .await .and_then(reqwest::Response::error_for_status) @@ -145,16 +145,37 @@ async fn download_to_temp_file( )) } -pub(crate) static REQWEST_CLIENT: LazyLock = - LazyLock::new(|| reqwest_client_from_env(&EnvVars)); +#[derive(Debug, thiserror::Error)] +pub(crate) enum ClientError { + #[error("Failed to initialize HTTP client")] + Build(#[source] reqwest::Error), -fn reqwest_client_from_env(env_vars: &impl EnvVarsRead) -> reqwest::Client { - let native_tls = env_vars - .var_as_bool(EnvVars::PREK_NATIVE_TLS) + #[error( + "Failed to initialize HTTP client with system certificates. Ensure system CA certificates are installed, or set `PREK_SYSTEM_CERTS=false` to use bundled certificates" + )] + SystemCertificates(#[source] reqwest::Error), +} + +pub(crate) fn reqwest_client() -> Result<&'static reqwest::Client, &'static ClientError> { + static CLIENT: LazyLock> = + LazyLock::new(reqwest_client_from_env); + + CLIENT.as_ref() +} + +fn reqwest_client_from_env() -> Result { + // TODO: Remove PREK_NATIVE_TLS compatibility in the next breaking release. + let system_certs_var = if EnvVars.is_set(EnvVars::PREK_SYSTEM_CERTS) { + EnvVars::PREK_SYSTEM_CERTS + } else { + EnvVars::PREK_NATIVE_TLS + }; + let system_certs = EnvVars + .var_as_bool(system_certs_var) .unwrap_or_else(|value| { warn_user!( "Invalid value for {}: {:?}. Expected a boolean value; using default ({:?})", - EnvVars::PREK_NATIVE_TLS, + system_certs_var, value, "false", ); @@ -162,16 +183,18 @@ fn reqwest_client_from_env(env_vars: &impl EnvVarsRead) -> reqwest::Client { }) .unwrap_or(false); - let certs = ssl_certs::load_certs_from_env(); - for error in certs.errors { - warn_user!("Failed to load certificates: {error}"); - } - let certs = certs - .certs - .iter() - .filter_map(|cert| Certificate::from_der(cert).ok()) - .collect(); - create_reqwest_client(native_tls, certs) + // An explicitly configured source replaces default roots even when empty. + let certs = ssl_certs::load_certs_from_env().map(|certs| { + for error in certs.errors { + warn_user!("Failed to load certificates: {error}"); + } + certs + .certs + .iter() + .filter_map(|cert| Certificate::from_der(cert).ok()) + .collect() + }); + create_reqwest_client(system_certs, certs) } // Bound the connect phase, and fail a download that stalls (no bytes received for this long) @@ -180,29 +203,40 @@ fn reqwest_client_from_env(env_vars: &impl EnvVarsRead) -> reqwest::Client { const CONNECT_TIMEOUT: Duration = Duration::from_secs(30); const READ_TIMEOUT: Duration = Duration::from_secs(30); -fn create_reqwest_client(native_tls: bool, custom_certs: Vec) -> reqwest::Client { +fn create_reqwest_client( + system_certs: bool, + custom_certs: Option>, +) -> Result { + let system_certs = system_certs && custom_certs.is_none(); let builder = reqwest::ClientBuilder::new() + .tls_backend_rustls() .user_agent(format!("prek/{}", crate::version::version())) .connect_timeout(CONNECT_TIMEOUT) .read_timeout(READ_TIMEOUT); - let builder = if native_tls { - debug!("Using native TLS for reqwest client"); - // Use rustls with rustls-platform-verifier which uses the platform's native certificate facilities. - builder.tls_backend_rustls().tls_certs_merge(custom_certs) + let builder = if let Some(custom_certs) = custom_certs { + debug!("Using custom certificates for reqwest client"); + builder.tls_certs_only(custom_certs) + } else if system_certs { + debug!("Using system certificates for reqwest client"); + // With no explicit roots, reqwest uses rustls-platform-verifier. + builder } else { + debug!("Using bundled certificates for reqwest client"); let root_certs = webpki_root_certs::TLS_SERVER_ROOT_CERTS .iter() .filter_map(|cert_der| Certificate::from_der(cert_der).ok()); - // Merge custom certificates on top of webpki-root-certs - builder - .tls_backend_rustls() - .tls_certs_only(custom_certs) - .tls_certs_merge(root_certs) + builder.tls_certs_only(root_certs) }; - builder.build().expect("Failed to build reqwest client") + builder.build().map_err(|error| { + if system_certs { + ClientError::SystemCertificates(error) + } else { + ClientError::Build(error) + } + }) } #[cfg(test)] @@ -305,11 +339,39 @@ mod tests { ); } + /// Default roots trust public HTTPS. Unrelated or empty custom roots replace + /// them and fail during certificate verification. #[tokio::test] - async fn test_native_tls() { - let client = super::create_reqwest_client(true, vec![]); - let resp = client.get("https://github.com").send().await; - assert!(resp.is_ok(), "Failed to send request with native TLS"); + async fn test_tls_certificates() -> Result<()> { + let custom_cert = reqwest::Certificate::from_pem(indoc::indoc! {b" + -----BEGIN CERTIFICATE----- + MIIBdDCCARqgAwIBAgIUZ1kmigC8aj49VPno0N1d7vKCDbswCgYIKoZIzj0EAwIw + FzEVMBMGA1UEAwwMcHJlayB0ZXN0IENBMCAXDTI2MTAyNjExNTM1OFoYDzIxMjYx + MDAyMTE1MzU4WjAXMRUwEwYDVQQDDAxwcmVrIHRlc3QgQ0EwWTATBgcqhkjOPQIB + BggqhkjOPQMBBwNCAATnHgUgx6Q0+Oeri/poUmyQLJiXk09eceAySysTGV+SI5wc + dxLzmCZvsolKlQ4A66QRleU3emn0GBKgnz2uqxFYo0IwQDAdBgNVHQ4EFgQU53Go + +jl4xGYHpgHFQpoYrttAbsQwDwYDVR0TAQH/BAUwAwEB/zAOBgNVHQ8BAf8EBAMC + AQYwCgYIKoZIzj0EAwIDSAAwRQIhALvrJ3U2Yk1k+iLH+rjj9jSnpt3bv5bhhKxK + WMKctiPPAiAcPHp1Si2wO726t4qJK5lX9cnTC3Azy/ZASjAzf8Dx1g== + -----END CERTIFICATE----- + "})?; + + for system_certs in [false, true] { + let client = super::create_reqwest_client(system_certs, None)?; + client.get("https://example.com").send().await?; + + for certs in [vec![], vec![custom_cert.clone()]] { + let client = super::create_reqwest_client(system_certs, Some(certs))?; + let Err(error) = client.get("https://example.com").send().await else { + anyhow::bail!("Custom certificates must replace default roots"); + }; + assert!( + format!("{error:?}").contains("UnknownIssuer"), + "Expected an untrusted issuer with system_certs={system_certs}: {error:?}" + ); + } + } + Ok(()) } #[tokio::test] diff --git a/crates/prek/src/languages/deno/installer.rs b/crates/prek/src/languages/deno/installer.rs index 934e8243f..48e347d94 100644 --- a/crates/prek/src/languages/deno/installer.rs +++ b/crates/prek/src/languages/deno/installer.rs @@ -16,7 +16,7 @@ use tracing::{debug, trace, warn}; use crate::archive; use crate::checksum::{Sha256Digest, digest_from_sha256sums, fetch_checksum}; use crate::fs::LockedFile; -use crate::http::{REQWEST_CLIENT, download_artifact}; +use crate::http::{download_artifact, reqwest_client}; use crate::languages::deno::DenoRequest; use crate::languages::deno::version::DenoVersion; use crate::languages::version::{ToolchainPolicy, ToolchainSource, find_system_executables}; @@ -187,7 +187,7 @@ impl DenoInstaller { } let url = "https://deno.com/versions.json"; - let response: VersionsResponse = REQWEST_CLIENT.get(url).send().await?.json().await?; + let response: VersionsResponse = reqwest_client()?.get(url).send().await?.json().await?; // Versions are already sorted in descending order (newest first) let versions: Vec = response diff --git a/crates/prek/src/languages/dotnet/installer.rs b/crates/prek/src/languages/dotnet/installer.rs index c1682dc49..94d3d777b 100644 --- a/crates/prek/src/languages/dotnet/installer.rs +++ b/crates/prek/src/languages/dotnet/installer.rs @@ -11,7 +11,7 @@ use tracing::{debug, trace, warn}; use super::version::DotnetVersion; use crate::fs::LockedFile; -use crate::http::REQWEST_CLIENT; +use crate::http::reqwest_client; use crate::languages::dotnet::DotnetRequest; use crate::languages::version::{ToolchainPolicy, ToolchainSource, find_system_executables}; use crate::process::Cmd; @@ -239,7 +239,7 @@ impl DotnetInstaller { let script_dir = tempfile::tempdir()?; let script_path = script_dir.path().join(script_name); - let response = REQWEST_CLIENT + let response = reqwest_client()? .get(script_url) .send() .await? diff --git a/crates/prek/src/languages/golang/installer.rs b/crates/prek/src/languages/golang/installer.rs index e977bae66..7139fbf9e 100644 --- a/crates/prek/src/languages/golang/installer.rs +++ b/crates/prek/src/languages/golang/installer.rs @@ -15,7 +15,7 @@ use crate::archive; use crate::checksum::Sha256Digest; use crate::fs::LockedFile; use crate::git; -use crate::http::{REQWEST_CLIENT, download_artifact}; +use crate::http::{download_artifact, reqwest_client}; use crate::languages::golang::GoRequest; use crate::languages::golang::golang::bin_dir; use crate::languages::golang::version::GoVersion; @@ -256,7 +256,7 @@ impl GoInstaller { async fn fetch_checksum(version: &str, filename: &str) -> Result> { let url = "https://go.dev/dl/?mode=json&include=all"; - let response = REQWEST_CLIENT + let response = reqwest_client()? .get(url) .send() .await diff --git a/crates/prek/src/languages/node/installer.rs b/crates/prek/src/languages/node/installer.rs index 703565966..56bb2014f 100644 --- a/crates/prek/src/languages/node/installer.rs +++ b/crates/prek/src/languages/node/installer.rs @@ -16,7 +16,7 @@ use tracing::{debug, trace, warn}; use crate::archive; use crate::checksum::{digest_from_sha256sums, fetch_checksum}; use crate::fs::{LockedFile, is_executable}; -use crate::http::{REQWEST_CLIENT, download_artifact}; +use crate::http::{download_artifact, reqwest_client}; use crate::languages::node::NodeRequest; use crate::languages::node::version::NodeVersion; use crate::languages::version::{ToolchainPolicy, ToolchainSource, find_system_executables}; @@ -191,7 +191,7 @@ impl NodeInstaller { /// List all versions of Node.js available on the Node.js website. async fn list_remote_versions(&self) -> Result> { let url = "https://nodejs.org/dist/index.json"; - let versions: Vec = REQWEST_CLIENT.get(url).send().await?.json().await?; + let versions: Vec = reqwest_client()?.get(url).send().await?.json().await?; Ok(versions) } diff --git a/crates/prek/src/languages/python/uv.rs b/crates/prek/src/languages/python/uv.rs index a266d1143..e3c3d31f4 100644 --- a/crates/prek/src/languages/python/uv.rs +++ b/crates/prek/src/languages/python/uv.rs @@ -15,7 +15,7 @@ use prek_consts::env_vars::{EnvVars, EnvVarsRead}; use crate::archive; use crate::checksum::{digest_from_sha256sums, fetch_checksum}; use crate::fs::LockedFile; -use crate::http::{REQWEST_CLIENT, TempDownload, download_artifact}; +use crate::http::{TempDownload, download_artifact, reqwest_client}; use crate::process::Cmd; use crate::store::{CacheBucket, Store}; use crate::warn_user; @@ -381,7 +381,7 @@ impl InstallSource { }; debug!("Fetching uv metadata from: {}", api_url); - let response = REQWEST_CLIENT + let response = reqwest_client()? .get(&api_url) .header("Accept", "*/*") .send() @@ -426,7 +426,7 @@ impl InstallSource { let simple_url = format!("{}uv/", source.url()); debug!("Fetching from simple API: {}", simple_url); - let response = REQWEST_CLIENT + let response = reqwest_client()? .get(&simple_url) .header(ACCEPT, "*/*") .send() diff --git a/crates/prek/src/languages/ruby/installer.rs b/crates/prek/src/languages/ruby/installer.rs index eb210f009..e120bfcdf 100644 --- a/crates/prek/src/languages/ruby/installer.rs +++ b/crates/prek/src/languages/ruby/installer.rs @@ -15,7 +15,7 @@ use tracing::{debug, trace, warn}; use crate::archive; use crate::checksum::{digest_from_sha256sums, fetch_checksum_with}; use crate::fs::LockedFile; -use crate::http::{DownloadChecksumPolicy, REQWEST_CLIENT, download_artifact_with}; +use crate::http::{DownloadChecksumPolicy, download_artifact_with, reqwest_client}; use crate::languages::ruby::RubyRequest; use crate::languages::version::{ToolchainPolicy, ToolchainSource, find_system_executables}; use crate::process::Cmd; @@ -290,7 +290,7 @@ impl RubyInstaller { let (api_url, is_github) = rv_ruby_api_url(&EnvVars); let suffix = format!(".{platform}.tar.gz"); - let req = REQWEST_CLIENT + let req = reqwest_client()? .get(&api_url) .header("Accept", "application/vnd.github+json"); let req = maybe_add_github_auth(req, is_github, &EnvVars); diff --git a/crates/prek/tests/http.rs b/crates/prek/tests/http.rs new file mode 100644 index 000000000..fa1ad5e6a --- /dev/null +++ b/crates/prek/tests/http.rs @@ -0,0 +1,54 @@ +mod common; + +use prek_consts::env_vars::EnvVars; + +use common::{TestEnv, cmd_snapshot}; + +fn download_context() -> TestEnv { + TestEnv::new() + .with_config(indoc::indoc! {r" + repos: + - repo: local + hooks: + - id: deno + name: deno + language: deno + entry: deno --version + always_run: true + "}) + .init_git() +} + +/// Invalid custom CA data must produce a normal CLI error with its cause, even +/// when system certificates are enabled, without suggesting a system CA fix. +#[test] +fn invalid_custom_certificate() { + let context = download_context().with_file( + "invalid.pem", + "-----BEGIN CERTIFICATE-----\nAA==\n-----END CERTIFICATE-----\n", + ); + + let mut command = context.run(); + command + .env( + EnvVars::PREK_INTERNAL__DENO_BINARY_NAME, + "deno-not-installed", + ) + .env(EnvVars::PREK_SYSTEM_CERTS, "true") + .env(EnvVars::SSL_CERT_FILE, context.child("invalid.pem").path()) + .env_remove(EnvVars::SSL_CERT_DIR); + + cmd_snapshot!(context, command, @" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to install hook `deno` + caused by: Failed to install deno + caused by: Failed to list remote versions + caused by: Failed to initialize HTTP client + caused by: builder error + caused by: invalid peer certificate: BadEncoding + "); +} diff --git a/docs/reference/environment-variables.md b/docs/reference/environment-variables.md index 98a72f019..bb8a1fc6c 100644 --- a/docs/reference/environment-variables.md +++ b/docs/reference/environment-variables.md @@ -85,9 +85,22 @@ If not set, prek uses its default uv version. This applies to all This only affects new uv installations. A compatible uv found alongside prek, on `PATH`, or in prek's cache is still reused, even if its version differs. +### `PREK_SYSTEM_CERTS` + +Set to `true` to use the system trust store through +[`rustls-platform-verifier`](https://crates.io/crates/rustls-platform-verifier). +By default, prek uses the certificates bundled with +[`webpki-root-certs`](https://crates.io/crates/webpki-root-certs). + +On Linux, this requires an available system CA bundle. + +When `SSL_CERT_FILE` or `SSL_CERT_DIR` is set to a non-empty value, prek uses only +certificates loaded from those paths, regardless of this setting. System and +bundled certificates are not loaded. + ### `PREK_NATIVE_TLS` -Use the system trusted store instead of the bundled `webpki-roots` crate. +Deprecated alias for [`PREK_SYSTEM_CERTS`](#prek_system_certs). ### `PREK_DOWNLOAD_CHECKSUM_POLICY` @@ -191,7 +204,7 @@ prek also honors variables owned by Git, ecosystem tools, or its HTTP stack: | Variable | Effect | | -- | -- | -| `SSL_CERT_FILE`, `SSL_CERT_DIR` | Add certificate locations used by HTTPS requests | +| `SSL_CERT_FILE`, `SSL_CERT_DIR` | Non-empty values replace the default HTTPS trust roots | | `GITHUB_TOKEN` | Authenticate supported GitHub API requests, including self-update and an exact GitHub Ruby mirror | | `HTTP_PROXY`, `HTTPS_PROXY`, `NO_PROXY` and lowercase forms | Configure inherited network proxy behavior |