Skip to content

feat(bundler/cli): Add feature flag to use system certificates#24

Open
hila-f-qodo wants to merge 2 commits intodevin_pr_code_review_bench_100_devin2_base_featbundler_cli_add_feature_flag_to_use_system_certificates_pr172from
devin_pr_code_review_bench_100_devin2_head_featbundler_cli_add_feature_flag_to_use_system_certificates_pr172
Open

feat(bundler/cli): Add feature flag to use system certificates#24
hila-f-qodo wants to merge 2 commits intodevin_pr_code_review_bench_100_devin2_base_featbundler_cli_add_feature_flag_to_use_system_certificates_pr172from
devin_pr_code_review_bench_100_devin2_head_featbundler_cli_add_feature_flag_to_use_system_certificates_pr172

Conversation

@hila-f-qodo
Copy link
Copy Markdown

@hila-f-qodo hila-f-qodo commented Jan 26, 2026

Benchmark PR from qodo-benchmark#172


Open with Devin

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 potential issues.

View issues and 3 additional flags in Devin Review.

Open in Devin Review

Comment on lines +37 to 39
.map(|cdn| {
cdn.to_string()
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 GitHub mirror URL no longer includes the original URL path

The generate_github_mirror_url_from_base function no longer uses its github_url parameter, causing GitHub mirror downloads to fail.

Click to expand

Root Cause

The original code appended the GitHub URL to the mirror base URL:

.map(|mut cdn| {
  cdn.set_path(github_url);
  cdn.to_string()
})

The new code just returns the CDN base URL without the path:

.map(|cdn| {
  cdn.to_string()
})

Impact

When TAURI_BUNDLER_TOOLS_GITHUB_MIRROR is set, all GitHub asset downloads will request the mirror's base URL instead of the correct mirrored asset path. For example, if the mirror is https://mirror.example.com/ and the asset URL is https://github.com/owner/repo/releases/download/v1.0/file.zip, the request will go to https://mirror.example.com/ instead of https://mirror.example.com/https://github.com/owner/repo/releases/download/v1.0/file.zip.

This will cause download failures or incorrect files being downloaded for users relying on GitHub mirrors.

Recommendation: Restore the cdn.set_path(github_url); call to properly construct the mirror URL with the original GitHub URL path.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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)?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 WebView2 offline installer download bypasses platform certificates and proxy settings

The download_webview2_offline_installer function uses ureq::get(url) directly instead of using base_ureq_agent() or the download() helper, bypassing the platform certificate and proxy configuration.

Click to expand

Root Cause

The code was changed from:

std::fs::write(&file_path, download(url)?)?;

To:

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)?;

The new code calls ureq::get(url) directly, which creates a default agent without:

  1. Platform certificate verification (the main feature this PR adds)
  2. Proxy settings from environment variables

Impact

Users who need platform certificates for TLS verification (e.g., in corporate environments with custom CA certificates) will experience TLS errors when downloading the WebView2 offline installer, even though the platform-certs feature is enabled. This defeats the purpose of the PR for this specific download.

Recommendation: Replace the direct ureq::get(url) call with the existing download(url)? helper function or use base_ureq_agent() to ensure platform certificates and proxy settings are used.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants