feat(bundler/cli): Add feature flag to use system certificates#24
Conversation
Code Review by Qodo
1. crate_latest_version hides errors
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| .proxy(ureq::Proxy::try_from_env()) | ||
| .build() | ||
| .into(); | ||
| return agent; |
There was a problem hiding this comment.
1. return agent; unnecessary 📘 Rule violation ⛯ Reliability
• base_ureq_agent() uses an explicit return, which is commonly flagged by Clippy (needless_return) and would fail a -D warnings Clippy run. • This creates avoidable CI noise and can block merges when Clippy warnings are treated as errors.
Agent prompt
## Issue description
`base_ureq_agent()` ends with `return agent;`, which is likely to trigger Clippy's `needless_return` lint and can fail builds when warnings are denied.
## Issue Context
The project requires `cargo clippy -- -D warnings` to pass.
## Fix Focus Areas
- crates/tauri-bundler/src/utils/http_utils.rs[56-73]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+120
to
+131
| #[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.
2. crate_latest_version hides errors 📘 Rule violation ⛯ Reliability
• The new platform-certs code path uses .call().ok()?, which silently discards the underlying HTTP error and returns None. • This makes failures non-actionable (no error context for debugging) and contradicts the requirement to make fallible operations explicit rather than hidden.
Agent prompt
## Issue description
The platform-certs HTTP request path uses `.call().ok()?`, which drops the error and returns `None`, hiding failure details.
## Issue Context
This code contacts an external service (`crates.io`). When it fails, developers need actionable error context. The compliance checklist also requires using `Result` for fallible operations instead of hiding errors.
## Fix Focus Areas
- crates/tauri-cli/src/helpers/cargo_manifest.rs[117-137]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+65
to
+68
| 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.
3. Unbounded read_to_end download 📘 Rule violation ⛨ Security
• The new download path reads the entire HTTP response into memory via read_to_end with no maximum size enforcement. • Since the response is external input, this can lead to excessive memory usage or denial-of-service behavior if a server returns an unexpectedly large payload.
Agent prompt
## Issue description
`download_webview2_offline_installer` reads the full HTTP response into memory without any size limit.
## Issue Context
The response body is external input. To comply with security-first input handling and robust edge case management, the code should enforce a reasonable maximum size or stream to disk.
## Fix Focus Areas
- crates/tauri-bundler/src/bundle/windows/util.rs[54-70]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
34
to
39
| std::env::var("TAURI_BUNDLER_TOOLS_GITHUB_MIRROR") | ||
| .ok() | ||
| .and_then(|cdn| Url::parse(&cdn).ok()) | ||
| .map(|mut cdn| { | ||
| cdn.set_path(github_url); | ||
| .map(|cdn| { | ||
| cdn.to_string() | ||
| }) |
There was a problem hiding this comment.
4. Mirror url drops path 🐞 Bug ✓ Correctness
• TAURI_BUNDLER_TOOLS_GITHUB_MIRROR support appears broken because generate_github_mirror_url_from_base no longer incorporates the requested GitHub URL into the returned mirror URL. • When the env var is set, downloads will request only the mirror base URL (likely returning HTML/404) instead of the intended GitHub asset. • This is a functional regression for users relying on the base-mirror environment variable.
Agent prompt
### Issue description
`TAURI_BUNDLER_TOOLS_GITHUB_MIRROR` base-mirror support is broken because the computed mirror URL no longer includes the requested GitHub URL.
### Issue Context
`download()` relies on `create_agent_and_url()` to produce a `final_url` to fetch. When `TAURI_BUNDLER_TOOLS_GITHUB_MIRROR` is set, `generate_github_mirror_url_from_base()` should transform a GitHub asset URL into a mirror URL that still uniquely identifies that asset.
### Fix Focus Areas
- crates/tauri-bundler/src/utils/http_utils.rs[33-40]
- crates/tauri-bundler/src/utils/http_utils.rs[42-54]
- crates/tauri-bundler/src/utils/http_utils.rs[179-260]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
52
to
+54
| fn create_agent_and_url(url: &str) -> (ureq::Agent, String) { | ||
| generate_github_alternative_url(url).unwrap_or(( | ||
| ureq::Agent::config_builder() | ||
| .proxy(ureq::Proxy::try_from_env()) | ||
| .build() | ||
| .into(), | ||
| url.to_owned(), | ||
| )) | ||
| generate_github_alternative_url(url).unwrap_or((base_ureq_agent(), url.to_owned())) | ||
| } |
There was a problem hiding this comment.
5. Mirror bypasses platform certs 🐞 Bug ⛯ Reliability
• Even with platform-certs enabled by default, mirrored GitHub downloads still use ureq::agent() rather than the new base_ureq_agent(). • This means users who configure GitHub mirrors won’t benefit from platform certificate verification (and proxy-from-env configuration) that this PR intends to provide. • In corporate TLS environments (custom roots) or proxy-restricted networks, mirrored downloads may still fail unexpectedly.
Agent prompt
### Issue description
Mirrored GitHub downloads still use the default `ureq::agent()` and therefore bypass the new platform-certs TLS configuration and proxy-from-env behavior.
### Issue Context
`create_agent_and_url()` now uses `base_ureq_agent()` only when no alternative (mirror) URL is generated.
### Fix Focus Areas
- crates/tauri-bundler/src/utils/http_utils.rs[42-54]
- crates/tauri-bundler/src/utils/http_utils.rs[56-73]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+65
to
+68
| 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.
6. Webview2 bypasses base agent 🐞 Bug ⛯ Reliability
• download_webview2_offline_installer now uses a raw ureq::get(url) call instead of the shared download(url) helper. • This bypasses the PR’s new base_ureq_agent() (platform certs + proxy-from-env), so the WebView2 offline installer download may still fail in environments that require platform trust stores or proxies. • The inconsistency is particularly notable because webview2_guid_path was updated to use base_ureq_agent(), but the actual file download was not.
Agent prompt
### Issue description
WebView2 offline installer downloads bypass the new centralized ureq agent configuration (platform certs + proxy-from-env).
### Issue Context
`webview2_guid_path()` already uses `base_ureq_agent()`, but `download_webview2_offline_installer()` uses `ureq::get()` directly.
### Fix Focus Areas
- crates/tauri-bundler/src/bundle/windows/util.rs[54-69]
- crates/tauri-bundler/src/utils/http_utils.rs[52-85]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
1 participant
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.
Benchmark PR from qodo-benchmark#172