From 05764e8af797b5abb8076bc78ce32d4130505e93 Mon Sep 17 00:00:00 2001 From: Tom Planche Date: Mon, 1 Dec 2025 09:00:27 +0100 Subject: [PATCH] http_client: Add integrity checks for GitHub binaries using digest checks (#43737) Generalizes the digest verification logic from `rust-analyzer` and `clangd` into a reusable helper function in `http_client::github_download`. This removes ~100 lines of duplicated code across the two language adapters and makes it easier for other language servers to adopt digest verification in the future. Closes #35201 Release Notes: - N/A --- crates/http_client/src/github_download.rs | 61 +++++++++++++++++++++- crates/languages/src/c.rs | 55 ++++++-------------- crates/languages/src/rust.rs | 62 ++++++----------------- 3 files changed, 92 insertions(+), 86 deletions(-) diff --git a/crates/http_client/src/github_download.rs b/crates/http_client/src/github_download.rs index 02dee08b215e547d632caaf5f94b0872aa6aa20d..3c16d5e692786282c32217108277faf2b42cf220 100644 --- a/crates/http_client/src/github_download.rs +++ b/crates/http_client/src/github_download.rs @@ -1,4 +1,4 @@ -use std::{path::Path, pin::Pin, task::Poll}; +use std::{future::Future, path::Path, pin::Pin, task::Poll}; use anyhow::{Context, Result}; use async_compression::futures::bufread::GzipDecoder; @@ -85,6 +85,65 @@ pub async fn download_server_binary( Ok(()) } +pub async fn fetch_github_binary_with_digest_check( + binary_path: &Path, + metadata_path: &Path, + expected_digest: Option, + url: &str, + asset_kind: AssetKind, + download_destination: &Path, + http_client: &dyn HttpClient, + validity_check: ValidityCheck, +) -> Result<()> +where + ValidityCheck: FnOnce() -> ValidityCheckFuture, + ValidityCheckFuture: Future>, +{ + let metadata = GithubBinaryMetadata::read_from_file(metadata_path) + .await + .ok(); + + if let Some(metadata) = metadata { + let validity_check_result = validity_check().await; + + if let (Some(actual_digest), Some(expected_digest_ref)) = + (&metadata.digest, &expected_digest) + { + if actual_digest == expected_digest_ref { + if validity_check_result.is_ok() { + return Ok(()); + } + } else { + log::info!( + "SHA-256 mismatch for {binary_path:?} asset, downloading new asset. Expected: {expected_digest_ref}, Got: {actual_digest}" + ); + } + } else if validity_check_result.is_ok() { + return Ok(()); + } + } + + download_server_binary( + http_client, + url, + expected_digest.as_deref(), + download_destination, + asset_kind, + ) + .await?; + + GithubBinaryMetadata::write_to_file( + &GithubBinaryMetadata { + metadata_version: 1, + digest: expected_digest, + }, + metadata_path, + ) + .await?; + + Ok(()) +} + async fn stream_response_archive( response: impl AsyncRead + Unpin, url: &str, diff --git a/crates/languages/src/c.rs b/crates/languages/src/c.rs index 8fe2bae693d702346a1ecc96334d35b89d179b3b..eb33bca0222abb0e03987081470549619c8e976d 100644 --- a/crates/languages/src/c.rs +++ b/crates/languages/src/c.rs @@ -3,7 +3,7 @@ use async_trait::async_trait; use futures::StreamExt; use gpui::{App, AsyncApp}; use http_client::github::{AssetKind, GitHubLspBinaryVersion, latest_github_release}; -use http_client::github_download::{GithubBinaryMetadata, download_server_binary}; +use http_client::github_download::fetch_github_binary_with_digest_check; pub use language::*; use lsp::{InitializeParams, LanguageServerBinary, LanguageServerName}; use project::lsp_store::clangd_ext; @@ -85,56 +85,33 @@ impl LspInstaller for CLspAdapter { }; let metadata_path = version_dir.join("metadata"); - let metadata = GithubBinaryMetadata::read_from_file(&metadata_path) - .await - .ok(); - if let Some(metadata) = metadata { - let validity_check = async || { + + let binary_path_for_check = binary_path.clone(); + fetch_github_binary_with_digest_check( + &binary_path, + &metadata_path, + expected_digest, + &url, + AssetKind::Zip, + &container_dir, + &*delegate.http_client(), + || async move { delegate .try_exec(LanguageServerBinary { - path: binary_path.clone(), + path: binary_path_for_check, arguments: vec!["--version".into()], env: None, }) .await .inspect_err(|err| { - log::warn!("Unable to run {binary_path:?} asset, redownloading: {err:#}",) + log::warn!("Unable to run clangd asset, redownloading: {err:#}") }) - }; - if let (Some(actual_digest), Some(expected_digest)) = - (&metadata.digest, &expected_digest) - { - if actual_digest == expected_digest { - if validity_check().await.is_ok() { - return Ok(binary); - } - } else { - log::info!( - "SHA-256 mismatch for {binary_path:?} asset, downloading new asset. Expected: {expected_digest}, Got: {actual_digest}" - ); - } - } else if validity_check().await.is_ok() { - return Ok(binary); - } - } - download_server_binary( - &*delegate.http_client(), - &url, - expected_digest.as_deref(), - &container_dir, - AssetKind::Zip, - ) - .await?; - remove_matching(&container_dir, |entry| entry != version_dir).await; - GithubBinaryMetadata::write_to_file( - &GithubBinaryMetadata { - metadata_version: 1, - digest: expected_digest, }, - &metadata_path, ) .await?; + remove_matching(&container_dir, |entry| entry != version_dir).await; + Ok(binary) } diff --git a/crates/languages/src/rust.rs b/crates/languages/src/rust.rs index 0b29ee93f3d4d1f1bc79e9ff93d99f80a24f473b..a8638a468604e6e73cc7f21527258c86f5a2246b 100644 --- a/crates/languages/src/rust.rs +++ b/crates/languages/src/rust.rs @@ -5,7 +5,7 @@ use futures::StreamExt; use gpui::{App, AppContext, AsyncApp, SharedString, Task}; use http_client::github::AssetKind; use http_client::github::{GitHubLspBinaryVersion, latest_github_release}; -use http_client::github_download::{GithubBinaryMetadata, download_server_binary}; +use http_client::github_download::fetch_github_binary_with_digest_check; pub use language::*; use lsp::{InitializeParams, LanguageServerBinary}; use project::lsp_store::rust_analyzer_ext::CARGO_DIAGNOSTICS_SOURCE_NAME; @@ -514,64 +514,34 @@ impl LspInstaller for RustLspAdapter { AssetKind::Zip => destination_path.clone().join("rust-analyzer.exe"), // zip contains a .exe }; - let binary = LanguageServerBinary { - path: server_path.clone(), - env: None, - arguments: Default::default(), - }; - let metadata_path = destination_path.with_extension("metadata"); - let metadata = GithubBinaryMetadata::read_from_file(&metadata_path) - .await - .ok(); - if let Some(metadata) = metadata { - let validity_check = async || { + + let server_path_for_check = server_path.clone(); + fetch_github_binary_with_digest_check( + &server_path, + &metadata_path, + expected_digest, + &url, + Self::GITHUB_ASSET_KIND, + &destination_path, + &*delegate.http_client(), + || async move { delegate .try_exec(LanguageServerBinary { - path: server_path.clone(), + path: server_path_for_check, arguments: vec!["--version".into()], env: None, }) .await .inspect_err(|err| { - log::warn!("Unable to run {server_path:?} asset, redownloading: {err:#}",) + log::warn!("Unable to run rust-analyzer asset, redownloading: {err:#}") }) - }; - if let (Some(actual_digest), Some(expected_digest)) = - (&metadata.digest, &expected_digest) - { - if actual_digest == expected_digest { - if validity_check().await.is_ok() { - return Ok(binary); - } - } else { - log::info!( - "SHA-256 mismatch for {destination_path:?} asset, downloading new asset. Expected: {expected_digest}, Got: {actual_digest}" - ); - } - } else if validity_check().await.is_ok() { - return Ok(binary); - } - } - - download_server_binary( - &*delegate.http_client(), - &url, - expected_digest.as_deref(), - &destination_path, - Self::GITHUB_ASSET_KIND, + }, ) .await?; + make_file_executable(&server_path).await?; remove_matching(&container_dir, |path| path != destination_path).await; - GithubBinaryMetadata::write_to_file( - &GithubBinaryMetadata { - metadata_version: 1, - digest: expected_digest, - }, - &metadata_path, - ) - .await?; Ok(LanguageServerBinary { path: server_path,