diff --git a/crates/http_client/src/github_download.rs b/crates/http_client/src/github_download.rs index 5d11f3e11b..2970d118ad 100644 --- a/crates/http_client/src/github_download.rs +++ b/crates/http_client/src/github_download.rs @@ -6,7 +6,7 @@ use std::{ use anyhow::{Context, Result}; use async_compression::futures::bufread::{BzDecoder, GzipDecoder}; -use futures::{AsyncRead, AsyncSeek, AsyncSeekExt, AsyncWrite, io::BufReader}; +use futures::{AsyncRead, AsyncSeek, AsyncSeekExt, AsyncWrite, AsyncWriteExt, io::BufReader}; use sha2::{Digest, Sha256}; use crate::{HttpClient, github::AssetKind}; @@ -68,6 +68,64 @@ pub async fn download_server_binary( Ok(()) } +pub async fn download_server_raw_binary( + http_client: &dyn HttpClient, + url: &str, + digest: Option<&str>, + destination_path: &Path, + binary_file_name: &str, +) -> Result<(), anyhow::Error> { + log::info!("downloading raw binary from {url}"); + let Some(destination_parent) = destination_path.parent() else { + anyhow::bail!("destination path has no parent: {destination_path:?}"); + }; + + let staging_path = staging_dir_path(destination_parent)?; + let result = async { + let mut response = http_client + .get(url, Default::default(), true) + .await + .with_context(|| format!("downloading release from {url}"))?; + + let binary_path = staging_path.join(binary_file_name); + let mut writer = HashingWriter { + writer: async_fs::File::create(&binary_path) + .await + .with_context(|| format!("creating a file {binary_path:?} for {url}"))?, + hasher: Sha256::new(), + }; + futures::io::copy(&mut BufReader::new(response.body_mut()), &mut writer) + .await + .with_context(|| format!("saving binary contents from {url}"))?; + let asset_sha_256 = writer + .finish() + .await + .with_context(|| format!("flushing binary contents for {url}"))?; + + if let Some(expected_sha_256) = digest { + anyhow::ensure!( + asset_sha_256 == expected_sha_256, + "{url} asset got SHA-256 mismatch. Expected: {expected_sha_256}, Got: {asset_sha_256}", + ); + } + + util::fs::make_file_executable(&binary_path) + .await + .with_context(|| format!("marking {binary_path:?} as executable"))?; + finalize_download(&staging_path, destination_path).await + } + .await; + + if let Err(err) = result { + if let Err(err) = async_fs::remove_dir_all(&staging_path).await { + log::warn!("failed to remove staging directory {staging_path:?}: {err:?}"); + } + return Err(err); + } + + Ok(()) +} + async fn extract_to_staging( body: impl AsyncRead + Unpin, digest: Option<&str>, @@ -117,15 +175,17 @@ async fn extract_to_staging( Ok(()) } +fn staging_dir_path(parent: &Path) -> Result { + let dir = tempfile::Builder::new() + .prefix(".tmp-github-download-") + .tempdir_in(parent) + .with_context(|| format!("creating staging directory in {parent:?}"))?; + Ok(dir.keep()) +} + fn staging_path(parent: &Path, asset_kind: AssetKind) -> Result { match asset_kind { - AssetKind::TarGz | AssetKind::TarBz2 | AssetKind::Zip => { - let dir = tempfile::Builder::new() - .prefix(".tmp-github-download-") - .tempdir_in(parent) - .with_context(|| format!("creating staging directory in {parent:?}"))?; - Ok(dir.keep()) - } + AssetKind::TarGz | AssetKind::TarBz2 | AssetKind::Zip => staging_dir_path(parent), AssetKind::Gz => { let path = tempfile::Builder::new() .prefix(".tmp-github-download-") @@ -261,6 +321,22 @@ struct HashingWriter { hasher: Sha256, } +impl HashingWriter { + /// Closes and drops the inner writer, returning the hex SHA-256 digest of + /// everything written. + /// + /// Taking `self` by value guarantees the writer is dropped before this + /// returns. For file writers this releases the OS handle, which Windows + /// requires before an ancestor directory can be renamed or deleted; note + /// that closing alone is not enough, as `async_fs::File` holds its handle + /// until dropped. + async fn finish(mut self) -> std::io::Result { + self.writer.close().await?; + drop(self.writer); + Ok(format!("{:x}", self.hasher.finalize())) + } +} + impl AsyncWrite for HashingWriter { fn poll_write( mut self: Pin<&mut Self>, @@ -290,3 +366,101 @@ impl AsyncWrite for HashingWriter { Pin::new(&mut self.writer).poll_close(cx) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{AsyncBody, Response}; + use futures::future::BoxFuture; + use http::HeaderValue; + use url::Url; + + struct StaticResponseClient { + body: Vec, + } + + impl HttpClient for StaticResponseClient { + fn send( + &self, + _req: http::Request, + ) -> BoxFuture<'static, anyhow::Result>> { + let body = self.body.clone(); + Box::pin(async move { + Ok(Response::builder() + .status(200) + .body(AsyncBody::from(body)) + .unwrap()) + }) + } + + fn user_agent(&self) -> Option<&HeaderValue> { + None + } + + fn proxy(&self) -> Option<&Url> { + None + } + } + + #[test] + fn downloads_raw_binary_into_destination_dir() { + futures::executor::block_on(async { + let temp_dir = tempfile::tempdir().unwrap(); + let destination_path = temp_dir.path().join("v_1"); + let contents = b"#!/bin/sh\necho hello\n".to_vec(); + let expected_sha_256 = format!("{:x}", Sha256::digest(&contents)); + let client = StaticResponseClient { body: contents }; + + download_server_raw_binary( + &client, + "https://example.com/agent-binary", + Some(&expected_sha_256), + &destination_path, + "agent-binary", + ) + .await + .unwrap(); + + let binary_path = destination_path.join("agent-binary"); + assert_eq!( + std::fs::read(&binary_path).unwrap(), + b"#!/bin/sh\necho hello\n" + ); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&binary_path) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o111, 0o111, "binary should be executable"); + } + }); + } + + #[test] + fn raw_binary_digest_mismatch_cleans_up_staging() { + futures::executor::block_on(async { + let temp_dir = tempfile::tempdir().unwrap(); + let destination_path = temp_dir.path().join("v_1"); + let client = StaticResponseClient { + body: b"some binary".to_vec(), + }; + + let error = download_server_raw_binary( + &client, + "https://example.com/agent-binary", + Some("0000000000000000000000000000000000000000000000000000000000000000"), + &destination_path, + "agent-binary", + ) + .await + .unwrap_err(); + + assert!(error.to_string().contains("SHA-256 mismatch")); + assert!(!destination_path.exists()); + let leftover_entries = std::fs::read_dir(temp_dir.path()).unwrap().count(); + assert_eq!(leftover_entries, 0, "staging directory should be removed"); + }); + } +} diff --git a/crates/project/src/agent_server_store.rs b/crates/project/src/agent_server_store.rs index fe3b581849..caeb28612e 100644 --- a/crates/project/src/agent_server_store.rs +++ b/crates/project/src/agent_server_store.rs @@ -891,23 +891,82 @@ impl ExternalAgentServer for RemoteExternalAgentServer { } } -fn asset_kind_for_archive_url(archive_url: &str) -> Result { +#[derive(Debug, PartialEq, Eq)] +enum RegistryArchiveKind { + Archive(AssetKind), + /// The archive URL points directly at an executable, per the ACP registry + /// schema: "URL to download archive (.zip, .tar.gz, .tgz, .tar.bz2, .tbz2, + /// or raw binary)". + RawBinary { + file_name: String, + }, +} + +fn registry_archive_kind_for_url(archive_url: &str) -> Result { + const UNSUPPORTED_SUFFIXES: &[&str] = &[ + // Installer formats explicitly rejected by the registry schema. + ".dmg", + ".pkg", + ".deb", + ".rpm", + ".msi", + ".appimage", + // Archive formats we cannot extract; treating them as raw binaries + // would produce a broken install. + ".tar.xz", + ".txz", + ".tar", + ".gz", + ".bz2", + ".xz", + ".7z", + ]; + let archive_path = Url::parse(archive_url) .ok() .map(|url| url.path().to_string()) .unwrap_or_else(|| archive_url.to_string()); + let lowercase_path = archive_path.to_lowercase(); - if archive_path.ends_with(".zip") { - Ok(AssetKind::Zip) - } else if archive_path.ends_with(".tar.gz") || archive_path.ends_with(".tgz") { - Ok(AssetKind::TarGz) - } else if archive_path.ends_with(".tar.bz2") || archive_path.ends_with(".tbz2") { - Ok(AssetKind::TarBz2) + if lowercase_path.ends_with(".zip") { + Ok(RegistryArchiveKind::Archive(AssetKind::Zip)) + } else if lowercase_path.ends_with(".tar.gz") || lowercase_path.ends_with(".tgz") { + Ok(RegistryArchiveKind::Archive(AssetKind::TarGz)) + } else if lowercase_path.ends_with(".tar.bz2") || lowercase_path.ends_with(".tbz2") { + Ok(RegistryArchiveKind::Archive(AssetKind::TarBz2)) + } else if let Some(suffix) = UNSUPPORTED_SUFFIXES + .iter() + .find(|suffix| lowercase_path.ends_with(*suffix)) + { + bail!("unsupported archive type {suffix} in URL: {archive_url}"); } else { - bail!("unsupported archive type in URL: {archive_url}"); + let file_name = raw_binary_file_name(&archive_path) + .with_context(|| format!("determining binary file name from URL: {archive_url}"))?; + Ok(RegistryArchiveKind::RawBinary { file_name }) } } +fn raw_binary_file_name(archive_path: &str) -> Result { + let last_segment = archive_path + .rsplit('/') + .next() + .filter(|segment| !segment.is_empty()) + .context("URL has no file name")?; + let file_name = percent_decode_str(last_segment) + .decode_utf8() + .context("file name is not valid UTF-8")? + .into_owned(); + anyhow::ensure!( + !file_name.is_empty() + && file_name != "." + && file_name != ".." + && !file_name.contains(['/', '\\']) + && !file_name.contains('\0'), + "invalid binary file name: {file_name}" + ); + Ok(file_name) +} + struct GithubReleaseArchive { repo_name_with_owner: String, tag: String, @@ -1187,16 +1246,28 @@ impl ExternalAgentServer for LocalRegistryArchiveAgent { None }; - let asset_kind = asset_kind_for_archive_url(archive_url)?; - - ::http_client::github_download::download_server_binary( - &*http_client, - archive_url, - sha256.as_deref(), - &version_dir, - asset_kind, - ) - .await?; + match registry_archive_kind_for_url(archive_url)? { + RegistryArchiveKind::Archive(asset_kind) => { + ::http_client::github_download::download_server_binary( + &*http_client, + archive_url, + sha256.as_deref(), + &version_dir, + asset_kind, + ) + .await?; + } + RegistryArchiveKind::RawBinary { file_name } => { + ::http_client::github_download::download_server_raw_binary( + &*http_client, + archive_url, + sha256.as_deref(), + &version_dir, + &file_name, + ) + .await?; + } + } } let cmd = &target_config.cmd; @@ -1703,45 +1774,84 @@ mod tests { #[test] fn detects_supported_archive_suffixes() { assert!(matches!( - asset_kind_for_archive_url("https://example.com/agent.zip"), - Ok(AssetKind::Zip) + registry_archive_kind_for_url("https://example.com/agent.zip"), + Ok(RegistryArchiveKind::Archive(AssetKind::Zip)) )); assert!(matches!( - asset_kind_for_archive_url("https://example.com/agent.zip?download=1"), - Ok(AssetKind::Zip) + registry_archive_kind_for_url("https://example.com/agent.zip?download=1"), + Ok(RegistryArchiveKind::Archive(AssetKind::Zip)) )); assert!(matches!( - asset_kind_for_archive_url("https://example.com/agent.tar.gz"), - Ok(AssetKind::TarGz) + registry_archive_kind_for_url("https://example.com/agent.tar.gz"), + Ok(RegistryArchiveKind::Archive(AssetKind::TarGz)) )); assert!(matches!( - asset_kind_for_archive_url("https://example.com/agent.tar.gz?download=1#latest"), - Ok(AssetKind::TarGz) + registry_archive_kind_for_url("https://example.com/agent.tar.gz?download=1#latest"), + Ok(RegistryArchiveKind::Archive(AssetKind::TarGz)) )); assert!(matches!( - asset_kind_for_archive_url("https://example.com/agent.tgz"), - Ok(AssetKind::TarGz) + registry_archive_kind_for_url("https://example.com/agent.tgz"), + Ok(RegistryArchiveKind::Archive(AssetKind::TarGz)) )); assert!(matches!( - asset_kind_for_archive_url("https://example.com/agent.tgz#download"), - Ok(AssetKind::TarGz) + registry_archive_kind_for_url("https://example.com/agent.tgz#download"), + Ok(RegistryArchiveKind::Archive(AssetKind::TarGz)) )); assert!(matches!( - asset_kind_for_archive_url("https://example.com/agent.tar.bz2"), - Ok(AssetKind::TarBz2) + registry_archive_kind_for_url("https://example.com/agent.tar.bz2"), + Ok(RegistryArchiveKind::Archive(AssetKind::TarBz2)) )); assert!(matches!( - asset_kind_for_archive_url("https://example.com/agent.tar.bz2?download=1"), - Ok(AssetKind::TarBz2) + registry_archive_kind_for_url("https://example.com/agent.tar.bz2?download=1"), + Ok(RegistryArchiveKind::Archive(AssetKind::TarBz2)) )); assert!(matches!( - asset_kind_for_archive_url("https://example.com/agent.tbz2"), - Ok(AssetKind::TarBz2) + registry_archive_kind_for_url("https://example.com/agent.tbz2"), + Ok(RegistryArchiveKind::Archive(AssetKind::TarBz2)) )); assert!(matches!( - asset_kind_for_archive_url("https://example.com/agent.tbz2#download"), - Ok(AssetKind::TarBz2) + registry_archive_kind_for_url("https://example.com/agent.tbz2#download"), + Ok(RegistryArchiveKind::Archive(AssetKind::TarBz2)) )); + assert!(matches!( + registry_archive_kind_for_url("https://example.com/agent.ZIP"), + Ok(RegistryArchiveKind::Archive(AssetKind::Zip)) + )); + } + + #[test] + fn detects_raw_binary_archive_urls() { + assert_eq!( + registry_archive_kind_for_url("https://x.ai/cli/grok-0.2.20-macos-aarch64").unwrap(), + RegistryArchiveKind::RawBinary { + file_name: "grok-0.2.20-macos-aarch64".to_string() + }, + ); + assert_eq!( + registry_archive_kind_for_url("https://x.ai/cli/grok-0.2.20-windows-x86_64.exe") + .unwrap(), + RegistryArchiveKind::RawBinary { + file_name: "grok-0.2.20-windows-x86_64.exe".to_string() + }, + ); + assert_eq!( + registry_archive_kind_for_url("https://example.com/agent-binary?download=1#latest") + .unwrap(), + RegistryArchiveKind::RawBinary { + file_name: "agent-binary".to_string() + }, + ); + assert_eq!( + registry_archive_kind_for_url("https://example.com/agent%20binary").unwrap(), + RegistryArchiveKind::RawBinary { + file_name: "agent binary".to_string() + }, + ); + // No file name to install the binary as. + assert!(registry_archive_kind_for_url("https://example.com/").is_err()); + // Percent-decoding must not allow path traversal in the file name. + assert!(registry_archive_kind_for_url("https://example.com/a%2F..%2Fevil").is_err()); + assert!(registry_archive_kind_for_url("https://example.com/%2E%2E").is_err()); } #[test] @@ -1758,14 +1868,31 @@ mod tests { #[test] fn rejects_unsupported_archive_suffixes() { - let error = asset_kind_for_archive_url("https://example.com/agent.tar.xz") + let error = registry_archive_kind_for_url("https://example.com/agent.tar.xz") .err() .map(|error| error.to_string()); assert_eq!( error, - Some("unsupported archive type in URL: https://example.com/agent.tar.xz".to_string()), + Some( + "unsupported archive type .tar.xz in URL: https://example.com/agent.tar.xz" + .to_string() + ), ); + + for installer_url in [ + "https://example.com/agent.dmg", + "https://example.com/agent.pkg", + "https://example.com/agent.deb", + "https://example.com/agent.rpm", + "https://example.com/agent.msi", + "https://example.com/agent.AppImage", + ] { + assert!( + registry_archive_kind_for_url(installer_url).is_err(), + "expected {installer_url} to be rejected" + ); + } } #[test]