feat(bundler/cli): Add feature flag to use system certificates#24
Conversation
WalkthroughThe PR introduces a new "platform-certs" feature across tauri-bundler and tauri-cli that enables reading system-installed TLS certificates when downloading tools and checking package versions. A shared Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/tauri-bundler/src/utils/http_utils.rs (1)
33-40: Bug: Mirror URL function ignores thegithub_urlparameter.The
generate_github_mirror_url_from_basefunction takesgithub_urlas a parameter but never uses it. It only returns the CDN base URL without appending the GitHub asset path, which breaks the mirror functionality.The function should combine the CDN base with the path portion of the GitHub URL.
🐛 Proposed fix
fn generate_github_mirror_url_from_base(github_url: &str) -> Option<String> { std::env::var("TAURI_BUNDLER_TOOLS_GITHUB_MIRROR") .ok() .and_then(|cdn| Url::parse(&cdn).ok()) - .map(|cdn| { - cdn.to_string() - }) + .and_then(|cdn| { + Url::parse(github_url).ok().map(|github| { + let mut mirror = cdn; + mirror.set_path(github.path()); + mirror.to_string() + }) + }) }
🤖 Fix all issues with AI agents
In `@crates/tauri-bundler/src/bundle/windows/util.rs`:
- Around line 65-68: The code currently uses ureq::get(url) which bypasses
platform TLS certs and proxy config; replace this direct call with the shared
download helper (or use base_ureq_agent()) so the request uses platform
certificates and proxy settings. Specifically, in the block that builds
response/bytes and writes to file_path, call the existing download(url,
&file_path) helper (or construct the request via
base_ureq_agent().get(url).call() and handle errors the same way) so TLS/proxy
configuration is applied and the file is written to file_path with the same
error propagation.
In `@crates/tauri-cli/src/helpers/cargo_manifest.rs`:
- Around line 120-131: The agent built inside the #[cfg(feature =
"platform-certs")] block does not pick up proxy settings, so modify that agent
construction to include the same proxy configuration used by base_ureq_agent()
in tauri-bundler: when building the ureq::Agent via
ureq::Agent::config_builder(), add the proxy configuration from
ureq::Proxy::try_from_env() (or the same proxy helper used by base_ureq_agent())
into the builder before calling .build(), keep the existing tls_config(...) and
.new_agent() calls, and preserve the existing agent.get(&url).call().ok()? error
handling; this ensures users behind a proxy can fetch crate versions when
platform-certs is enabled.
🧹 Nitpick comments (1)
crates/tauri-bundler/src/utils/http_utils.rs (1)
56-73: Consider removing explicitreturnstatement.Idiomatic Rust omits the
returnkeyword for the final expression.♻️ Suggested change
#[cfg(not(feature = "platform-certs"))] let agent: ureq::Agent = ureq::Agent::config_builder() .proxy(ureq::Proxy::try_from_env()) .build() .into(); - return agent; + agent }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.changes/feat-bundler-platform-certs.mdcrates/tauri-bundler/Cargo.tomlcrates/tauri-bundler/src/bundle/windows/util.rscrates/tauri-bundler/src/utils/http_utils.rscrates/tauri-cli/Cargo.tomlcrates/tauri-cli/src/helpers/cargo_manifest.rs
🧰 Additional context used
🧬 Code graph analysis (1)
crates/tauri-bundler/src/bundle/windows/util.rs (1)
crates/tauri-bundler/src/utils/http_utils.rs (2)
base_ureq_agent(56-73)download(76-85)
🔇 Additional comments (5)
.changes/feat-bundler-platform-certs.md (1)
1-7: LGTM!The changelog entry accurately documents the new TLS certificate behavior for the bundler and CLI tools.
crates/tauri-bundler/Cargo.toml (1)
76-81: LGTM!The
platform-certsfeature is correctly defined with theureq/platform-verifierdependency, and appropriately included in the default features alongsiderustls.crates/tauri-cli/Cargo.toml (1)
140-149: LGTM!The
platform-certsfeature correctly propagates totauri-bundler/platform-certsand enablesureq/platform-verifierfor the CLI's direct ureq usage.crates/tauri-cli/src/helpers/cargo_manifest.rs (1)
194-204: LGTM!The condition fix correctly ensures
lock_versionis populated only when there are actual lock packages present.crates/tauri-bundler/src/bundle/windows/util.rs (1)
27-28: LGTM!Correctly uses the shared
base_ureq_agent()for consistent TLS and proxy configuration.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| let response = ureq::get(url).call().map_err(Box::new)?; | ||
| let mut bytes = Vec::new(); | ||
| response.into_body().into_reader().read_to_end(&mut bytes)?; | ||
| std::fs::write(&file_path, bytes)?; |
There was a problem hiding this comment.
Inconsistent HTTP client usage bypasses platform-certs and proxy configuration.
This code uses ureq::get(url) directly instead of base_ureq_agent() or the download() helper. This bypasses the platform certificates TLS configuration and proxy settings that are applied elsewhere, defeating the purpose of this PR.
🐛 Proposed fix using download() helper
if !file_path.exists() {
create_dir_all(dir_path)?;
- let response = ureq::get(url).call().map_err(Box::new)?;
- let mut bytes = Vec::new();
- response.into_body().into_reader().read_to_end(&mut bytes)?;
- std::fs::write(&file_path, bytes)?;
+ std::fs::write(&file_path, download(url)?)?;
}🤖 Prompt for AI Agents
In `@crates/tauri-bundler/src/bundle/windows/util.rs` around lines 65 - 68, The
code currently uses ureq::get(url) which bypasses platform TLS certs and proxy
config; replace this direct call with the shared download helper (or use
base_ureq_agent()) so the request uses platform certificates and proxy settings.
Specifically, in the block that builds response/bytes and writes to file_path,
call the existing download(url, &file_path) helper (or construct the request via
base_ureq_agent().get(url).call() and handle errors the same way) so TLS/proxy
configuration is applied and the file is written to file_path with the same
error propagation.
| #[cfg(feature = "platform-certs")] | ||
| let mut response = { | ||
| let agent = ureq::Agent::config_builder() | ||
| .tls_config( | ||
| ureq::tls::TlsConfig::builder() | ||
| .root_certs(ureq::tls::RootCerts::PlatformVerifier) | ||
| .build(), | ||
| ) | ||
| .build() | ||
| .new_agent(); | ||
| agent.get(&url).call().ok()? | ||
| }; |
There was a problem hiding this comment.
Missing proxy configuration when platform-certs is enabled.
The custom agent for platform-certs doesn't include proxy configuration via ureq::Proxy::try_from_env(), whereas the base_ureq_agent() in tauri-bundler does. This inconsistency means users behind a proxy won't be able to fetch crate versions when platform-certs is enabled.
🐛 Proposed fix
#[cfg(feature = "platform-certs")]
let mut response = {
let agent = ureq::Agent::config_builder()
.tls_config(
ureq::tls::TlsConfig::builder()
.root_certs(ureq::tls::RootCerts::PlatformVerifier)
.build(),
)
+ .proxy(ureq::Proxy::try_from_env())
.build()
.new_agent();
agent.get(&url).call().ok()?
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[cfg(feature = "platform-certs")] | |
| let mut response = { | |
| let agent = ureq::Agent::config_builder() | |
| .tls_config( | |
| ureq::tls::TlsConfig::builder() | |
| .root_certs(ureq::tls::RootCerts::PlatformVerifier) | |
| .build(), | |
| ) | |
| .build() | |
| .new_agent(); | |
| agent.get(&url).call().ok()? | |
| }; | |
| #[cfg(feature = "platform-certs")] | |
| let mut response = { | |
| let agent = ureq::Agent::config_builder() | |
| .tls_config( | |
| ureq::tls::TlsConfig::builder() | |
| .root_certs(ureq::tls::RootCerts::PlatformVerifier) | |
| .build(), | |
| ) | |
| .proxy(ureq::Proxy::try_from_env()) | |
| .build() | |
| .new_agent(); | |
| agent.get(&url).call().ok()? | |
| }; |
🤖 Prompt for AI Agents
In `@crates/tauri-cli/src/helpers/cargo_manifest.rs` around lines 120 - 131, The
agent built inside the #[cfg(feature = "platform-certs")] block does not pick up
proxy settings, so modify that agent construction to include the same proxy
configuration used by base_ureq_agent() in tauri-bundler: when building the
ureq::Agent via ureq::Agent::config_builder(), add the proxy configuration from
ureq::Proxy::try_from_env() (or the same proxy helper used by base_ureq_agent())
into the builder before calling .build(), keep the existing tls_config(...) and
.new_agent() calls, and preserve the existing agent.get(&url).call().ok()? error
handling; this ensures users behind a proxy can fetch crate versions when
platform-certs is enabled.
1 participant
Benchmark PR from qodo-benchmark#172
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.