Allow using system trusted store by PREK_NATIVE_TLS

[st1971]

update reqwest cargo import to add feature rustls-tls-native-roots, this allow prek to work in an enviroment where additional system level trusted certificates are required, feature rustls-tls-native-roots respected the SSL_CERT_FILE env var.

Verified on macos / linux only.

[codecov]

Codecov Report

❌ Patch coverage is 85.86957% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.10%. Comparing base (c2f0468) to head (4e88411).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/languages/mod.rs	83.07%	11 Missing ⚠️
src/languages/python/uv.rs	90.90%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #959      +/-   ##
==========================================
- Coverage   90.15%   90.10%   -0.06%     
==========================================
  Files          66       66              
  Lines       12244    12289      +45     
==========================================
+ Hits        11039    11073      +34     
- Misses       1205     1216      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

📦 Cargo Bloat Comparison

Binary size change: +0.62% (16.1 MiB → 16.2 MiB)

Expand for cargo-bloat output

PR Branch Results

 File  .text     Size          Crate Name
 0.6%   1.3% 101.1KiB          prek? <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
 0.5%   1.2%  88.0KiB           prek prek::languages::<impl prek::config::Language>::run::{{closure}}
 0.5%   1.1%  85.8KiB           prek prek::builtin::pre_commit_hooks::Implemented::run::{{closure}}
 0.4%   0.9%  70.0KiB           prek prek::run::{{closure}}
 0.4%   0.9%  63.8KiB           prek prek::archive::unpack::{{closure}}
 0.3%   0.6%  42.0KiB           prek prek::languages::<impl prek::config::Language>::install::{{closure}}
 0.2%   0.5%  40.9KiB regex_automata regex_automata::meta::strategy::new
 0.2%   0.5%  40.1KiB          prek? <prek::cli::RunArgs as clap_builder::derive::Args>::augment_args
 0.2%   0.5%  38.5KiB           prek prek::identify::by_extension::{{closure}}
 0.2%   0.4%  32.9KiB           prek prek::workspace::Workspace::discover
 0.2%   0.4%  32.8KiB           prek prek::cli::run::run::run::{{closure}}
 0.2%   0.4%  32.3KiB           prek prek::languages::node::installer::NodeInstaller::install::{{closure}}
 0.2%   0.4%  31.8KiB             h2 h2::proto::connection::DynConnection<B>::recv_frame
 0.2%   0.4%  31.7KiB             h2 h2::proto::connection::Connection<T,P,B>::poll
 0.2%   0.3%  26.2KiB     hyper_util hyper_util::client::legacy::client::Client<C,B>::send_request::{{closure}}
 0.2%   0.3%  25.8KiB     hyper_util hyper_util::client::legacy::client::Client<C,B>::connect_to::{{closure}}::{{closure}}::{{closure}}
 0.2%   0.3%  25.0KiB        globset globset::GlobSetBuilder::build
 0.2%   0.3%  25.0KiB           prek prek::main
 0.1%   0.3%  24.2KiB           prek prek::languages::golang::installer::GoInstaller::install::{{closure}}
 0.1%   0.3%  23.7KiB          hyper hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_loop
39.6%  87.3%   6.4MiB                And 10179 smaller methods. Use -n N to show more.
45.4% 100.0%   7.3MiB                .text section size, the file size is 16.2MiB

Base Branch Results

 File  .text     Size          Crate Name
 0.6%   1.4% 101.1KiB          prek? <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
 0.5%   1.2%  88.0KiB           prek prek::languages::<impl prek::config::Language>::run::{{closure}}
 0.5%   1.1%  85.8KiB           prek prek::builtin::pre_commit_hooks::Implemented::run::{{closure}}
 0.4%   0.9%  70.0KiB           prek prek::run::{{closure}}
 0.4%   0.9%  63.8KiB           prek prek::archive::unpack::{{closure}}
 0.3%   0.6%  42.0KiB           prek prek::languages::<impl prek::config::Language>::install::{{closure}}
 0.2%   0.5%  40.9KiB regex_automata regex_automata::meta::strategy::new
 0.2%   0.5%  40.1KiB          prek? <prek::cli::RunArgs as clap_builder::derive::Args>::augment_args
 0.2%   0.5%  38.5KiB           prek prek::identify::by_extension::{{closure}}
 0.2%   0.4%  32.9KiB           prek prek::workspace::Workspace::discover
 0.2%   0.4%  32.8KiB           prek prek::cli::run::run::run::{{closure}}
 0.2%   0.4%  32.7KiB           prek prek::languages::node::installer::NodeInstaller::install::{{closure}}
 0.2%   0.4%  31.8KiB             h2 h2::proto::connection::DynConnection<B>::recv_frame
 0.2%   0.4%  31.7KiB             h2 h2::proto::connection::Connection<T,P,B>::poll
 0.2%   0.4%  26.2KiB     hyper_util hyper_util::client::legacy::client::Client<C,B>::send_request::{{closure}}
 0.2%   0.3%  25.8KiB     hyper_util hyper_util::client::legacy::client::Client<C,B>::connect_to::{{closure}}::{{closure}}::{{closure}}
 0.2%   0.3%  25.0KiB        globset globset::GlobSetBuilder::build
 0.2%   0.3%  25.0KiB           prek prek::main
 0.1%   0.3%  24.6KiB           prek prek::languages::golang::installer::GoInstaller::install::{{closure}}
 0.1%   0.3%  23.7KiB          hyper hyper::proto::h1::dispatch::Dispatcher<D,Bs,I,T>::poll_loop
39.6%  87.2%   6.4MiB                And 10127 smaller methods. Use -n N to show more.
45.4% 100.0%   7.3MiB                .text section size, the file size is 16.1MiB

[j178]

I noticed that in uv, you have to pass the --native-tls flag to enable native TLS roots, because "reading the native certificates can add hundreds of milliseconds to client initialization." For performance reasons, should we maybe make this opt-in too? Check out astral-sh/uv#2362

[st1971]

the PR that was highlighted is no longer used in UV, however, looked at the UV code and have used a similer method to what is current in use. If SSL_CERT_FILE is set to a file that exists or PREK_NATIVE_TLS is set to true then system tls trust store will be used, if these are not set it will fall back to webpki-roots.

the change has dropped the test coverage level, however to put some meaninful tests on this change we would need to update the environment table during testing which would require making the test single threaded.

[j178]

put some meaninful tests on this change we would need to update the environment table during testing which would require making the test single threaded

Since we call prek in a subprocess during integration tests, I assume it's safe to use subprocess-specific environment variables here?

Could you also document the new environment variables in docs/configuration.md?

[st1971]

Sorry, struggling a little with this, to be honest this change should be covered using unit test which as we should really be testing that the returned client can access an https end point when configured in different ways, which is only possible if the unit tests are single threaded as the mutation of the environment table is not safe, could do some stuff with extra locks if the EnvVars type was in the same scope as the main binary code.

Testing this within integration tests, not really sure how to make this work, within the integration test cannot construct a reqwest client instance so would have to set-up multiple language tests just to trigger downloads, which seems somewhat excessive.

What i could do is move the reqwest client creation and resource downloading code into a lib, in the same way EnvVars is, in this way it can be tested independently of the rest of the code.

Thoughts?

[j178]

I extracted the env var reading from get_reqwest_client, so now we can create a native TLS client without messing with the process env table. I also added a basic test just to make sure the native TLS client actually works.

And I took the opportunity to do a bit more refactoring by making the client a global shared instance - hope that makes sense.

[st1971]

Nice, had been messing about moving the client / download_and_extract to a lib which also works well from a test perspective, but yours is cleaner. if your happy with it then i am.

its really hard to get reqwest to error even when you want it to!