From c9c2155973187d54d301ecabb92f0643949090d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Wed, 17 Jun 2026 14:34:55 +0200 Subject: [PATCH 01/12] feat(extension): support shared release repositories --- Cargo.lock | 73 ++ Cargo.toml | 6 + docs/EXTENSIONS.md | 137 ++-- src/commands/extension.rs | 61 +- src/extensions/discovery.rs | 9 + src/extensions/exec.rs | 44 ++ src/extensions/install.rs | 1378 +++++++++++++++++++++++++++++++++-- src/extensions/manifest.rs | 41 ++ src/main.rs | 21 + src/test_commands.rs | 112 +++ 10 files changed, 1782 insertions(+), 100 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3822dc98..4d99c5c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1183,6 +1183,16 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "64cd1e32ddd350061ae6edb1b082d7c54915b5c672c389143b9a63403a109f24" +[[package]] +name = "filetime" +version = "0.2.29" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c287a33c7f0a620c38e641e7f60827713987b3c0f26e8ddc9462cc69cf75759" +dependencies = [ + "cfg-if", + "libc", +] + [[package]] name = "find-msvc-tools" version = "0.1.9" @@ -1197,6 +1207,7 @@ checksum = "843fba2746e448b37e26a819579957415c8cef339bf08564fe8b7ddbd959573c" dependencies = [ "crc32fast", "miniz_oxide", + "zlib-rs", ] [[package]] @@ -2856,6 +2867,7 @@ dependencies = [ "comfy-table", "datadog-api-client", "dirs", + "flate2", "futures", "futures-util", "getrandom 0.4.2", @@ -2881,6 +2893,7 @@ dependencies = [ "sha2 0.11.0", "shell-words", "ssh-key 0.6.7", + "tar", "tokio", "tokio-tungstenite", "tokio-util", @@ -2890,6 +2903,7 @@ dependencies = [ "wasm-bindgen-futures", "web-sys", "yamux", + "zip", ] [[package]] @@ -4147,6 +4161,17 @@ dependencies = [ "libc", ] +[[package]] +name = "tar" +version = "0.4.46" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f6221d9a6003c78398e3b239969f352578258df48c8eb051caadae0015bc840" +dependencies = [ + "filetime", + "libc", + "xattr", +] + [[package]] name = "thiserror" version = "1.0.69" @@ -4411,6 +4436,12 @@ dependencies = [ "thiserror 2.0.18", ] +[[package]] +name = "typed-path" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e28f89b80c87b8fb0cf04ab448d5dd0dd0ade2f8891bae878de66a75a28600e" + [[package]] name = "typenum" version = "1.20.0" @@ -5222,6 +5253,16 @@ version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1ffae5123b2d3fc086436f8834ae3ab053a283cfac8fe0a0b8eaae044768a4c4" +[[package]] +name = "xattr" +version = "1.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32e45ad4206f6d2479085147f02bc2ef834ac85886624a23575ae137c8aa8156" +dependencies = [ + "libc", + "rustix", +] + [[package]] name = "yamux" version = "0.13.10" @@ -5341,12 +5382,44 @@ dependencies = [ "syn", ] +[[package]] +name = "zip" +version = "8.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d04a6b5381502aa6087c94c669499eb1602eb9c5e8198e534de571f7154809b" +dependencies = [ + "crc32fast", + "flate2", + "indexmap 2.14.0", + "memchr", + "typed-path", + "zopfli", +] + +[[package]] +name = "zlib-rs" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3be3d40e40a133f9c916ee3f9f4fa2d9d63435b5fbe1bfc6d9dae0aa0ada1513" + [[package]] name = "zmij" version = "1.0.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b8848ee67ecc8aedbaf3e4122217aff892639231befc6a1b58d29fff4c2cabaa" +[[package]] +name = "zopfli" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f05cd8797d63865425ff89b5c4a48804f35ba0ce8d125800027ad6017d2b5249" +dependencies = [ + "bumpalo", + "crc32fast", + "log", + "simd-adler32", +] + [[package]] name = "zstd" version = "0.13.3" diff --git a/Cargo.toml b/Cargo.toml index 1fbf1c4d..3ae44fdc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,9 @@ native = [ "dep:ssh-key", "dep:futures", "dep:tokio-util", + "dep:flate2", + "dep:tar", + "dep:zip", ] wasi = [ "dep:datadog-api-client", @@ -126,6 +129,9 @@ russh = { version = "0.61", optional = true } ssh-key = { version = "0.6", features = ["p256", "std"], optional = true } futures = { version = "0.3", optional = true } tokio-util = { version = "0.7", features = ["compat", "io"], optional = true } +flate2 = { version = "1", optional = true } +tar = { version = "0.4", optional = true } +zip = { version = "8", default-features = false, features = ["deflate"], optional = true } # Datadog API client — pinned to master HEAD 2026-05-29 # Use default-features = false; feature sets are activated per-target via features above diff --git a/docs/EXTENSIONS.md b/docs/EXTENSIONS.md index 1865e2a1..8814c677 100644 --- a/docs/EXTENSIONS.md +++ b/docs/EXTENSIONS.md @@ -2,7 +2,7 @@ ## Overview -Pup extensions are standalone executables that add new subcommands to pup. When you run `pup terraform ...`, pup checks if `terraform` is a built-in command. If not, it looks for an installed extension named `pup-terraform` and runs it with your arguments and auth credentials. +Pup extensions are standalone executables that add new subcommands to pup. When you run `pup foo ...`, pup checks if `foo` is a built-in command. If not, it looks for an installed extension named `pup-foo` and runs it with your arguments and auth credentials. Extensions let teams ship experimental features independently without modifying pup's core or doing a full release. Any language works - extensions are just executables. @@ -12,27 +12,36 @@ Extensions let teams ship experimental features independently without modifying ```bash # Install from a GitHub repository (downloads the latest release) -pup extension install jkirsteins/pup-hello +pup extension install owner/pup-foo # Install a specific release version -pup extension install jkirsteins/pup-hello --tag v1.0.0 +pup extension install owner/pup-foo --tag v1.0.0 + +# List extensions available from a shared release repository +pup extension list-remote owner/repo + +# Install one extension from a shared release repository +pup extension install owner/repo --extension foo + +# Install all extensions from a shared release repository's latest release +pup extension install owner/repo --all ``` ### Install from a local file ```bash # Install from a local binary -pup extension install --local /path/to/pup-my-tool +pup extension install --local /path/to/pup-foo # Install as a symlink (for development) -pup extension install --local /path/to/pup-my-tool --link +pup extension install --local /path/to/pup-foo --link ``` ### Use it ```bash # The extension becomes a pup subcommand -pup my-tool --some-flag value +pup foo --some-flag value ``` ### Manage extensions @@ -45,13 +54,13 @@ pup extension list pup -o table extension list # Upgrade a single extension to the latest release -pup extension upgrade my-tool +pup extension upgrade foo # Upgrade all installed extensions pup extension upgrade --all # Remove an extension -pup extension remove my-tool +pup extension remove foo ``` ## Writing an Extension @@ -67,11 +76,11 @@ echo "Site: $DD_SITE" echo "Args: $@" ``` -Save this as `pup-hello`, make it executable (`chmod +x pup-hello`), and install it: +Save this as `pup-foo`, make it executable (`chmod +x pup-foo`), and install it: ```bash -pup extension install --local ./pup-hello -pup hello world +pup extension install --local ./pup-foo +pup foo world # Output: # Hello from pup extension! # Site: datadoghq.com @@ -84,8 +93,8 @@ pup hello world - `` must be lowercase letters, digits, and hyphens only, starting with a letter - `` must not conflict with a built-in pup command (e.g., `monitors`, `logs`, `auth`) -Valid: `pup-terraform`, `pup-cost-report`, `pup-lint` -Invalid: `pup-Terraform`, `pup-2fast`, `pup-my_tool`, `pup-monitors` +Valid: `pup-foo`, `pup-cost-report`, `pup-lint` +Invalid: `pup-Foo`, `pup-2fast`, `pup-my_tool`, `pup-monitors` ## Auth Forwarding @@ -160,14 +169,14 @@ pup api v2/tags/hosts/myhost -X POST -F source=web #!/bin/bash results='[{"id":1,"name":"alpha"},{"id":2,"name":"beta"}]' -# Honor whatever -o the user passed to `pup my-tool`. +# Honor whatever -o the user passed to `pup foo`. echo "$results" | pup format # Or force a specific format. echo "$results" | pup format --output table # Populate the agent-mode envelope metadata. -echo "$results" | pup format --count 2 --command "my-tool list" +echo "$results" | pup format --count 2 --command "foo list" ``` ### Combine them @@ -185,7 +194,7 @@ Pup's global flags (`--output`, `--yes`, `--agent`, `--read-only`, `--org`) are ```bash # --output table is consumed by pup, extension receives PUP_OUTPUT=table -pup --output table my-tool do-something +pup --output table foo do-something # The extension receives only: ["do-something"] # Not: ["--output", "table", "do-something"] @@ -194,7 +203,7 @@ pup --output table my-tool do-something Extension-specific flags (anything pup doesn't recognize) are passed through to the extension unchanged: ```bash -pup my-tool plan --workspace prod --var-file vars.tfvars +pup foo plan --workspace prod --var-file vars.tfvars # Extension receives: ["plan", "--workspace", "prod", "--var-file", "vars.tfvars"] ``` @@ -206,7 +215,7 @@ pup my-tool plan --workspace prod --var-file vars.tfvars pup extension install owner/repo ``` -Downloads the platform-specific binary from the repository's latest GitHub Release and installs it. The extension name is derived from the repo name (stripping the `pup-` prefix if present). For example, `jkirsteins/pup-hello` installs as `hello`. +Downloads the platform-specific binary from the repository's latest GitHub Release and installs it. The extension name is derived from the repo name (stripping the `pup-` prefix if present). For example, `owner/pup-foo` installs as `foo`. GitHub releases must include assets following the naming convention: @@ -215,18 +224,18 @@ pup--- ``` Where: -- `` is the extension name (e.g., `hello`) +- `` is the extension name (e.g., `foo`) - `` is one of: `darwin`, `linux`, `windows` - `` is one of: `x86_64`, `aarch64` -Example assets for an extension named `hello`: +Example assets for an extension named `foo`: ``` -pup-hello-darwin-aarch64 -pup-hello-darwin-x86_64 -pup-hello-linux-aarch64 -pup-hello-linux-x86_64 -pup-hello-windows-x86_64.exe +pup-foo-darwin-aarch64 +pup-foo-darwin-x86_64 +pup-foo-linux-aarch64 +pup-foo-linux-x86_64 +pup-foo-windows-x86_64.exe ``` To install a specific release tag: @@ -235,10 +244,56 @@ To install a specific release tag: pup extension install owner/repo --tag v1.0.0 ``` +### Shared GitHub release repositories + +A GitHub repository can also publish one platform archive containing multiple top-level extension executables: + +``` +repo_1.2.3_Darwin_arm64.tar.gz +repo_1.2.3_Linux_x86_64.tar.gz +repo_1.2.3_Windows_x86_64.zip +``` + +Each archive can contain executables such as: + +``` +pup-foo +pup-bar +``` + +List remote versions inferred from release archives: + +```bash +pup extension list-remote owner/repo +pup extension list-remote owner/repo --extension foo +``` + +Install one extension from the newest release archive that contains it: + +```bash +pup extension install owner/repo --extension foo +``` + +Install all extensions from the latest release archive: + +```bash +pup extension install owner/repo --all +``` + +If a release archive contains exactly one extension, `pup extension install owner/repo` can infer it. If it contains multiple extensions, pup asks you to choose `--extension ` or `--all`. + +Private GitHub repositories are supported with either an explicit token or an existing GitHub CLI login. Token resolution order is: + +1. `GH_TOKEN`, `GITHUB_TOKEN`, or `HOMEBREW_GITHUB_API_TOKEN` +2. the active GitHub CLI account from `gh auth token --hostname github.com` +3. anonymous GitHub access for public repositories + +`gh` is optional. Pup uses it only as a credential helper when no explicit token is set. Pup does not switch accounts, refresh scopes, create tokens, store GitHub tokens, or pass GitHub tokens to extensions. If access fails and multiple GitHub CLI accounts are configured, choose the desired account with `gh auth switch --hostname github.com` or set `GH_TOKEN` explicitly. + ### Local install (copy) ```bash -pup extension install --local /path/to/pup-my-tool +pup extension install --local /path/to/pup-foo ``` Copies the binary into pup's extensions directory and sets executable permissions. @@ -246,7 +301,7 @@ Copies the binary into pup's extensions directory and sets executable permission ### Local install (symlink) ```bash -pup extension install --local /path/to/pup-my-tool --link +pup extension install --local /path/to/pup-foo --link ``` Creates a symlink instead of copying. Useful during development so changes to the source binary take effect immediately without reinstalling. @@ -254,7 +309,7 @@ Creates a symlink instead of copying. Useful during development so changes to th ### Custom name ```bash -pup extension install --local /path/to/my-binary --name my-tool +pup extension install --local /path/to/my-binary --name foo ``` By default, the extension name is derived from the filename (stripping `pup-` prefix and `.exe` suffix) for local installs, or from the repo name for GitHub installs. Use `--name` to override. @@ -262,7 +317,7 @@ By default, the extension name is derived from the filename (stripping `pup-` pr ### Force reinstall ```bash -pup extension install --local /path/to/pup-my-tool --force +pup extension install --local /path/to/pup-foo --force pup extension install owner/repo --force ``` @@ -273,10 +328,10 @@ Overwrites an existing extension with the same name. ### Upgrade a single extension ```bash -pup extension upgrade hello +pup extension upgrade foo ``` -Checks the GitHub release for a newer version. If one is available, downloads and installs it. If the extension is already at the latest version, prints a message and does nothing. +Checks GitHub for a newer version. For single-binary repositories, pup checks the latest release. For shared release repositories, pup searches releases newest-first and upgrades to the newest release archive that contains that extension. If the extension is already at the latest version, prints a message and does nothing. ### Upgrade all extensions @@ -298,8 +353,8 @@ Extensions are stored in pup's config directory: ``` /extensions/ - pup-my-tool/ - pup-my-tool # the executable + pup-foo/ + pup-foo # the executable manifest.json # metadata (written by pup at install time) ``` @@ -401,22 +456,8 @@ To extract an existing pup feature into an extension: 4. Remove the feature from pup's core `Commands` enum 5. Distribute the extension binary separately -## Demo Extension - -A demo extension is available for testing at [jkirsteins/pup-hello](https://github.com/jkirsteins/pup-hello): - -```bash -pup extension install jkirsteins/pup-hello -pup hello world -# Output: -# Hello from pup extension! (v1.1.0) -# Site: datadoghq.com -# Args: world -``` - ## Limitations -- **Public repositories only**: GitHub-based installation works with public repositories. Private repository support (token forwarding) is not implemented. - **Source must be a regular file**: `pup extension install --local` requires the source path to be a regular file, not a directory. - **Agent-mode help**: `pup --agent --help` prints pup's top-level schema, not the extension's help. In normal mode, `--help` is passed through to the extension. -- **No signing or verification**: Downloaded binaries are not cryptographically verified. Only install extensions from trusted sources. +- **No signing**: Downloaded binaries are not signed. If a release includes `checksums.txt`, pup verifies the selected archive checksum before installing. Only install extensions from trusted sources. diff --git a/src/commands/extension.rs b/src/commands/extension.rs index 423ec6ad..a1c925e8 100644 --- a/src/commands/extension.rs +++ b/src/commands/extension.rs @@ -59,6 +59,8 @@ pub fn list(cfg: &Config) -> Result<()> { /// Options for installing an extension. pub struct InstallOptions { pub source: String, + pub extension: Option, + pub all: bool, pub tag: Option, pub local: bool, pub link: bool, @@ -71,6 +73,8 @@ pub struct InstallOptions { pub fn install(_cfg: &Config, opts: InstallOptions) -> Result<()> { let InstallOptions { source, + extension, + all, tag, local, link, @@ -79,6 +83,9 @@ pub fn install(_cfg: &Config, opts: InstallOptions) -> Result<()> { description, } = opts; if local { + if extension.is_some() || all { + bail!("--extension and --all are only supported for GitHub installs"); + } let source_path = PathBuf::from(&source); // Derive name from filename if not provided. let ext_name = match name { @@ -117,20 +124,62 @@ pub fn install(_cfg: &Config, opts: InstallOptions) -> Result<()> { } // GitHub-based installation: source is "owner/repo". - extensions::install::install_from_github( + let installed = extensions::install::install_from_github( &source, tag.as_deref(), name.as_deref(), + extension.as_deref(), + all, force, description.as_deref(), )?; - let display_name = name.unwrap_or_else(|| { - let repo = source.split('/').nth(1).unwrap_or(&source); - extensions::install::derive_name_from_repo(repo) - }); - println!("Installed extension '{display_name}' from github:{source}"); + if installed.len() == 1 { + println!( + "Installed extension '{}' from github:{source}", + installed[0] + ); + } else { + println!( + "Installed {} extensions from github:{source}: {}", + installed.len(), + installed.join(", ") + ); + } + + Ok(()) +} +/// List extensions available from a remote GitHub repository. +pub fn list_remote(cfg: &Config, source: String, extension: Option) -> Result<()> { + let items = extensions::install::list_remote_extensions(&source, extension.as_deref())?; + match cfg.output_format { + crate::config::OutputFormat::Table => { + if items.is_empty() { + println!("No remote extensions found."); + } else { + for item in &items { + println!("{} v{} ({})", item.name, item.version, item.tag); + } + } + } + _ => { + let values: Vec = items + .iter() + .map(|item| { + serde_json::json!({ + "name": item.name, + "version": item.version, + "tag": item.tag, + "source": item.source, + "asset": item.asset, + "inferred_from_archive": item.inferred_from_archive, + }) + }) + .collect(); + crate::formatter::format_and_print(&values, &cfg.output_format, cfg.agent_mode, None)?; + } + } Ok(()) } diff --git a/src/extensions/discovery.rs b/src/extensions/discovery.rs index 2d91f672..8b13ea7f 100644 --- a/src/extensions/discovery.rs +++ b/src/extensions/discovery.rs @@ -187,6 +187,9 @@ mod tests { name: "hello".to_string(), version: "1.0.0".to_string(), source: "local:/tmp/pup-hello".to_string(), + source_kind: None, + source_release_tag: None, + source_asset: None, installed_at: "2026-03-29T00:00:00Z".to_string(), binary: "pup-hello".to_string(), description: "Hello world".to_string(), @@ -233,6 +236,9 @@ mod tests { name: name.to_string(), version: "1.0.0".to_string(), source: format!("local:/tmp/pup-{name}"), + source_kind: None, + source_release_tag: None, + source_asset: None, installed_at: "2026-03-29T00:00:00Z".to_string(), binary: format!("pup-{name}"), description: desc.to_string(), @@ -266,6 +272,9 @@ mod tests { name: "nodesc".to_string(), version: "2.5.0".to_string(), source: "local:/tmp/pup-nodesc".to_string(), + source_kind: None, + source_release_tag: None, + source_asset: None, installed_at: "2026-03-29T00:00:00Z".to_string(), binary: "pup-nodesc".to_string(), description: String::new(), diff --git a/src/extensions/exec.rs b/src/extensions/exec.rs index c6f10658..9ce6be81 100644 --- a/src/extensions/exec.rs +++ b/src/extensions/exec.rs @@ -76,6 +76,12 @@ fn inject_auth_env(cmd: &mut std::process::Command, cfg: &Config) { } } + // GitHub tokens are used only by pup for extension install/list/upgrade. + // Extensions receive Datadog auth, not repository access credentials. + for name in ["GH_TOKEN", "GITHUB_TOKEN", "HOMEBREW_GITHUB_API_TOKEN"] { + cmd.env_remove(name); + } + // Boolean mode flags - set when active, unset when not. if cfg.auto_approve { cmd.env("PUP_AUTO_APPROVE", "true"); @@ -93,3 +99,41 @@ fn inject_auth_env(cmd: &mut std::process::Command, cfg: &Config) { cmd.env_remove("PUP_AGENT_MODE"); } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::OutputFormat; + + fn test_config() -> Config { + Config { + api_key: None, + app_key: None, + access_token: None, + site: "datadoghq.com".to_string(), + site_explicit: false, + org: None, + output_format: OutputFormat::Json, + auto_approve: false, + agent_mode: false, + read_only: false, + } + } + + fn removed_env(cmd: &std::process::Command, name: &str) -> bool { + cmd.get_envs() + .any(|(key, value)| key == name && value.is_none()) + } + + #[test] + fn test_inject_auth_env_removes_github_tokens() { + let cfg = test_config(); + let mut cmd = std::process::Command::new("pup-foo"); + + inject_auth_env(&mut cmd, &cfg); + + assert!(removed_env(&cmd, "GH_TOKEN")); + assert!(removed_env(&cmd, "GITHUB_TOKEN")); + assert!(removed_env(&cmd, "HOMEBREW_GITHUB_API_TOKEN")); + } +} diff --git a/src/extensions/install.rs b/src/extensions/install.rs index a2a54bb6..e1d41c64 100644 --- a/src/extensions/install.rs +++ b/src/extensions/install.rs @@ -1,5 +1,9 @@ use anyhow::{bail, Context, Result}; +use sha2::{Digest, Sha256}; +use std::io::{Cursor, Read}; use std::path::Path; +use std::process::Command; +use std::sync::OnceLock; use super::discovery::extension_dir; use super::manifest::Manifest; @@ -9,6 +13,8 @@ use crate::version; #[derive(Debug, serde::Deserialize)] struct GitHubAsset { name: String, + #[serde(default)] + url: Option, browser_download_url: String, } @@ -19,6 +25,30 @@ struct GitHubRelease { assets: Vec, } +#[derive(Debug)] +struct ArchiveDownload { + release_tag: String, + version: String, + asset_name: String, + bytes: Vec, + extensions: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum GitHubAuthSource { + Env(&'static str), + GhActive, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct GitHubAuthResolution { + token: Option, + source: Option, + gh_error: Option, +} + +static GITHUB_AUTH: OnceLock = OnceLock::new(); + /// Map `std::env::consts::OS` to the asset name convention. fn platform_os() -> &'static str { match std::env::consts::OS { @@ -38,6 +68,25 @@ fn platform_arch() -> &'static str { } } +/// Map `std::env::consts::OS` to the GoReleaser archive OS convention. +fn archive_platform_os() -> &'static str { + match std::env::consts::OS { + "macos" => "Darwin", + "linux" => "Linux", + "windows" => "Windows", + other => other, + } +} + +/// Map `std::env::consts::ARCH` to the GoReleaser archive arch convention. +fn archive_platform_arch() -> &'static str { + match std::env::consts::ARCH { + "x86_64" => "x86_64", + "aarch64" => "arm64", + other => other, + } +} + /// Build a reqwest client with a User-Agent header (required by GitHub API). fn github_client() -> Result { reqwest::Client::builder() @@ -46,6 +95,199 @@ fn github_client() -> Result { .context("building HTTP client for GitHub API") } +fn resolve_github_auth_with( + env_lookup: EnvLookup, + gh_token: GhToken, +) -> GitHubAuthResolution +where + EnvLookup: Fn(&str) -> Option, + GhToken: Fn() -> Result, +{ + for name in ["GH_TOKEN", "GITHUB_TOKEN", "HOMEBREW_GITHUB_API_TOKEN"] { + if let Some(token) = env_lookup(name) + .map(|token| token.trim().to_string()) + .filter(|token| !token.is_empty()) + { + return GitHubAuthResolution { + token: Some(token), + source: Some(GitHubAuthSource::Env(name)), + gh_error: None, + }; + } + } + + match gh_token() { + Ok(token) => { + let token = token.trim().to_string(); + if token.is_empty() { + GitHubAuthResolution { + token: None, + source: None, + gh_error: Some("gh auth token returned an empty token".to_string()), + } + } else { + GitHubAuthResolution { + token: Some(token), + source: Some(GitHubAuthSource::GhActive), + gh_error: None, + } + } + } + Err(err) => GitHubAuthResolution { + token: None, + source: None, + gh_error: Some(err.to_string()), + }, + } +} + +fn active_gh_token() -> Result { + let output = Command::new("gh") + .args(["auth", "token", "--hostname", "github.com"]) + .output() + .context("running gh auth token --hostname github.com")?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + let stderr = stderr.trim(); + if stderr.is_empty() { + bail!("gh auth token failed with status {}", output.status); + } + bail!("gh auth token failed: {stderr}"); + } + + String::from_utf8(output.stdout).context("gh auth token output was not UTF-8") +} + +fn resolve_github_auth() -> GitHubAuthResolution { + resolve_github_auth_with(|name| std::env::var(name).ok(), active_gh_token) +} + +fn github_auth() -> &'static GitHubAuthResolution { + GITHUB_AUTH.get_or_init(resolve_github_auth) +} + +fn github_token() -> Option<&'static str> { + github_auth().token.as_deref() +} + +fn github_auth_status_diagnostic() -> Option { + let output = Command::new("gh") + .args([ + "auth", + "status", + "--hostname", + "github.com", + "--json", + "hosts", + ]) + .output() + .ok()?; + + if !output.status.success() { + return None; + } + + let stdout = String::from_utf8(output.stdout).ok()?; + github_auth_status_diagnostic_from_json(&stdout) +} + +fn github_auth_status_diagnostic_from_json(json: &str) -> Option { + let value: serde_json::Value = serde_json::from_str(json).ok()?; + let accounts = value + .get("hosts")? + .get("github.com")? + .as_array() + .map(Vec::as_slice)?; + if accounts.is_empty() { + return Some("No GitHub CLI accounts are authenticated for github.com.".to_string()); + } + + let active_login = accounts + .iter() + .find(|account| { + account + .get("active") + .and_then(serde_json::Value::as_bool) + .unwrap_or(false) + }) + .and_then(|account| account.get("login")) + .and_then(serde_json::Value::as_str); + + if accounts.len() > 1 { + let active = active_login.unwrap_or("unknown"); + Some(format!( + "GitHub CLI has multiple accounts configured for github.com. Active account: {active}." + )) + } else { + let active = active_login + .or_else(|| accounts[0].get("login").and_then(serde_json::Value::as_str)) + .unwrap_or("unknown"); + Some(format!("GitHub CLI active account: {active}.")) + } +} + +fn github_access_guidance( + owner: &str, + repo: &str, + auth: &GitHubAuthResolution, + gh_status: Option<&str>, +) -> String { + let mut message = match auth.source { + Some(GitHubAuthSource::Env(name)) => format!( + "GitHub access failed for {owner}/{repo} using token from {name}.\n\n\ + Check that the token can access this repository." + ), + Some(GitHubAuthSource::GhActive) => format!( + "GitHub access failed for {owner}/{repo} using the active GitHub CLI account.\n\n\ + Check the active account:\n\ + gh auth status --hostname github.com\n\n\ + To switch accounts:\n\ + gh auth switch --hostname github.com\n\n\ + Or provide a token explicitly:\n\ + GH_TOKEN= pup extension install {owner}/{repo} --extension foo" + ), + None => format!( + "GitHub access failed for {owner}/{repo}.\n\n\ + For private repositories, provide a token:\n\ + export GH_TOKEN=\n\n\ + Or authenticate with GitHub CLI:\n\ + gh auth login --hostname github.com --scopes repo" + ), + }; + + if let Some(gh_error) = &auth.gh_error { + message.push_str("\n\nGitHub CLI token lookup failed: "); + message.push_str(gh_error); + } + + if let Some(gh_status) = gh_status { + message.push_str("\n\n"); + message.push_str(gh_status); + } + + message +} + +fn github_failure_guidance(owner: &str, repo: &str) -> String { + let auth = github_auth(); + let gh_status = match auth.source { + Some(GitHubAuthSource::Env(_)) => None, + Some(GitHubAuthSource::GhActive) | None => github_auth_status_diagnostic(), + }; + github_access_guidance(owner, repo, auth, gh_status.as_deref()) +} + +fn github_api_get(client: &reqwest::Client, url: &str) -> reqwest::RequestBuilder { + let mut req = client + .get(url) + .header("Accept", "application/vnd.github+json"); + if let Some(token) = github_token() { + req = req.header("Authorization", format!("Bearer {token}")); + } + req +} + /// Fetch a GitHub release (latest or by tag). async fn fetch_github_release( client: &reqwest::Client, @@ -58,29 +300,32 @@ async fn fetch_github_release( None => format!("https://api.github.com/repos/{owner}/{repo}/releases/latest"), }; - let resp = client - .get(&url) + let resp = github_api_get(client, &url) .send() .await .with_context(|| format!("fetching release from {url}"))?; let status = resp.status(); if status == reqwest::StatusCode::NOT_FOUND { + let guidance = github_failure_guidance(owner, repo); match tag { Some(t) => bail!( "release tag '{t}' not found in {owner}/{repo}. \ - Check that the tag exists at https://github.com/{owner}/{repo}/releases" + Check that the tag exists at https://github.com/{owner}/{repo}/releases\n\n\ + {guidance}" ), None => bail!( "no releases found for {owner}/{repo}. \ Check that the repository exists and has at least one release at \ - https://github.com/{owner}/{repo}/releases" + https://github.com/{owner}/{repo}/releases\n\n\ + {guidance}" ), } } if !status.is_success() { let body = resp.text().await.unwrap_or_default(); - bail!("GitHub API returned {status} for {url}: {body}"); + let guidance = github_failure_guidance(owner, repo); + bail!("GitHub API returned {status} for {url}: {body}\n\n{guidance}"); } resp.json::() @@ -88,6 +333,54 @@ async fn fetch_github_release( .with_context(|| format!("parsing release JSON from {url}")) } +/// Fetch GitHub releases newest-first. +async fn fetch_github_releases( + client: &reqwest::Client, + owner: &str, + repo: &str, +) -> Result> { + let mut releases = Vec::new(); + let mut page = 1; + + loop { + let url = format!( + "https://api.github.com/repos/{owner}/{repo}/releases?per_page=100&page={page}" + ); + let resp = github_api_get(client, &url) + .send() + .await + .with_context(|| format!("fetching releases from {url}"))?; + + let status = resp.status(); + if status == reqwest::StatusCode::NOT_FOUND { + let guidance = github_failure_guidance(owner, repo); + bail!( + "no releases found for {owner}/{repo}. \ + Check that the repository exists and is accessible\n\n\ + {guidance}" + ); + } + if !status.is_success() { + let body = resp.text().await.unwrap_or_default(); + let guidance = github_failure_guidance(owner, repo); + bail!("GitHub API returned {status} for {url}: {body}\n\n{guidance}"); + } + + let mut page_releases = resp + .json::>() + .await + .with_context(|| format!("parsing release JSON from {url}"))?; + let count = page_releases.len(); + releases.append(&mut page_releases); + if count < 100 { + break; + } + page += 1; + } + + Ok(releases) +} + /// Find the matching asset for the current platform in a release. fn find_platform_asset<'a>(release: &'a GitHubRelease, ext_name: &str) -> Result<&'a GitHubAsset> { let os = platform_os(); @@ -115,10 +408,238 @@ fn find_platform_asset<'a>(release: &'a GitHubRelease, ext_name: &str) -> Result }) } -/// Download a binary asset from a URL. -async fn download_asset(client: &reqwest::Client, url: &str) -> Result> { - let resp = client - .get(url) +/// Find a platform archive that bundles one or more `pup-*` executables. +fn find_platform_archive_asset<'a>( + release: &'a GitHubRelease, + project_name: &str, +) -> Result<&'a GitHubAsset> { + let version = extract_version(&release.tag_name); + let os = archive_platform_os(); + let arch = archive_platform_arch(); + let expected_tar = format!("{project_name}_{version}_{os}_{arch}.tar.gz"); + let expected_zip = format!("{project_name}_{version}_{os}_{arch}.zip"); + + release + .assets + .iter() + .find(|a| a.name == expected_tar || a.name == expected_zip) + .ok_or_else(|| { + let available: Vec<&str> = release.assets.iter().map(|a| a.name.as_str()).collect(); + anyhow::anyhow!( + "no matching archive asset for platform {os}-{arch} \ + (expected '{expected_tar}' or '{expected_zip}'). Available assets: {}", + if available.is_empty() { + "(none)".to_string() + } else { + available.join(", ") + } + ) + }) +} + +fn extension_name_from_archive_path(path: &Path) -> Option { + if path.components().count() != 1 { + return None; + } + let file_name = path.file_name()?.to_str()?; + let file_name = file_name.strip_suffix(".exe").unwrap_or(file_name); + let name = file_name.strip_prefix("pup-")?; + if validate_extension_name(name).is_ok() { + Some(name.to_string()) + } else { + None + } +} + +fn extension_archive_member_matches(path: &Path, name: &str) -> bool { + if path.components().count() != 1 { + return false; + } + let Some(file_name) = path.file_name().and_then(|s| s.to_str()) else { + return false; + }; + file_name == format!("pup-{name}") || file_name == format!("pup-{name}.exe") +} + +fn extension_names_from_archive(asset_name: &str, bytes: &[u8]) -> Result> { + if asset_name.ends_with(".tar.gz") { + extension_names_from_tar_gz(bytes) + } else if asset_name.ends_with(".zip") { + extension_names_from_zip(bytes) + } else { + bail!("unsupported extension archive format: {asset_name}"); + } +} + +fn extension_names_from_tar_gz(bytes: &[u8]) -> Result> { + let decoder = flate2::read::GzDecoder::new(Cursor::new(bytes)); + let mut archive = tar::Archive::new(decoder); + let mut names = Vec::new(); + + for entry in archive + .entries() + .context("reading tar.gz archive entries")? + { + let entry = entry.context("reading tar.gz archive entry")?; + if !entry.header().entry_type().is_file() { + continue; + } + let path = entry.path().context("reading tar.gz archive path")?; + if let Some(name) = extension_name_from_archive_path(path.as_ref()) { + names.push(name); + } + } + + names.sort(); + names.dedup(); + Ok(names) +} + +fn extension_names_from_zip(bytes: &[u8]) -> Result> { + let mut archive = zip::ZipArchive::new(Cursor::new(bytes)).context("reading zip archive")?; + let mut names = Vec::new(); + + for i in 0..archive.len() { + let file = archive + .by_index(i) + .with_context(|| format!("reading zip archive entry {i}"))?; + if !file.is_file() { + continue; + } + if let Some(name) = extension_name_from_archive_path(Path::new(file.name())) { + names.push(name); + } + } + + names.sort(); + names.dedup(); + Ok(names) +} + +fn extract_extension_from_archive(asset_name: &str, bytes: &[u8], name: &str) -> Result> { + validate_extension_name(name)?; + if asset_name.ends_with(".tar.gz") { + extract_extension_from_tar_gz(bytes, name) + } else if asset_name.ends_with(".zip") { + extract_extension_from_zip(bytes, name) + } else { + bail!("unsupported extension archive format: {asset_name}"); + } +} + +fn extract_extension_from_tar_gz(bytes: &[u8], name: &str) -> Result> { + let decoder = flate2::read::GzDecoder::new(Cursor::new(bytes)); + let mut archive = tar::Archive::new(decoder); + + for entry in archive + .entries() + .context("reading tar.gz archive entries")? + { + let mut entry = entry.context("reading tar.gz archive entry")?; + if !entry.header().entry_type().is_file() { + continue; + } + let path = entry.path().context("reading tar.gz archive path")?; + if extension_archive_member_matches(path.as_ref(), name) { + let mut bytes = Vec::new(); + entry + .read_to_end(&mut bytes) + .context("reading extension binary from tar.gz archive")?; + return Ok(bytes); + } + } + + bail!("archive does not contain extension 'pup-{name}'") +} + +fn extract_extension_from_zip(bytes: &[u8], name: &str) -> Result> { + let mut archive = zip::ZipArchive::new(Cursor::new(bytes)).context("reading zip archive")?; + + for i in 0..archive.len() { + let mut file = archive + .by_index(i) + .with_context(|| format!("reading zip archive entry {i}"))?; + if !file.is_file() { + continue; + } + if extension_archive_member_matches(Path::new(file.name()), name) { + let mut bytes = Vec::new(); + file.read_to_end(&mut bytes) + .context("reading extension binary from zip archive")?; + return Ok(bytes); + } + } + + bail!("archive does not contain extension 'pup-{name}'") +} + +fn selected_archive_extension_names( + available: &[String], + extension: Option<&str>, + all: bool, +) -> Result> { + if extension.is_some() && all { + bail!("choose either --extension or --all, not both"); + } + + let mut available = available.to_vec(); + available.sort(); + available.dedup(); + + if all { + if available.is_empty() { + bail!("release archive does not contain any pup extensions"); + } + return Ok(available); + } + + if let Some(extension) = extension { + validate_extension_name(extension)?; + if available.iter().any(|name| name == extension) { + return Ok(vec![extension.to_string()]); + } + bail!( + "release archive does not contain extension '{extension}'. Available extensions: {}", + if available.is_empty() { + "(none)".to_string() + } else { + available.join(", ") + } + ); + } + + match available.as_slice() { + [] => bail!("release archive does not contain any pup extensions"), + [name] => Ok(vec![name.clone()]), + _ => bail!( + "release archive contains multiple extensions: {}.\n\ + Install one with: pup extension install --extension \n\ + Install all with: pup extension install --all", + available.join(", ") + ), + } +} + +/// Download a release asset. Authenticated GitHub API asset URLs are used when +/// a GitHub token is available; public browser URLs remain the fallback. +async fn download_asset(client: &reqwest::Client, asset: &GitHubAsset) -> Result> { + let token = github_token(); + let use_api_asset = token.is_some() && asset.url.is_some(); + let url = if use_api_asset { + asset.url.as_deref().unwrap() + } else { + &asset.browser_download_url + }; + + let mut req = client.get(url); + if use_api_asset { + req = req.header("Accept", "application/octet-stream"); + if let Some(token) = token { + req = req.header("Authorization", format!("Bearer {token}")); + } + } + + let resp = req .send() .await .with_context(|| format!("downloading asset from {url}"))?; @@ -223,26 +744,221 @@ fn write_extension_binary(ext_dir: &Path, name: &str, bytes: &[u8]) -> Result, +} + +struct GitHubInstallArtifacts { + version: String, + source_kind: Option, + source_release_tag: Option, + source_asset: Option, + payloads: Vec, +} + +fn archive_extension_payloads( + archive: &ArchiveDownload, + names: &[String], +) -> Result> { + names + .iter() + .map(|name| { + let bytes = extract_extension_from_archive(&archive.asset_name, &archive.bytes, name)?; + Ok(ExtensionPayload { + name: name.clone(), + bytes, + }) + }) + .collect() +} + +fn save_github_payloads( + source: &str, + artifacts: GitHubInstallArtifacts, + force: bool, + description: Option<&str>, +) -> Result> { + if artifacts.payloads.is_empty() { + bail!("no extensions selected to install"); + } + if artifacts.payloads.len() > 1 && description.is_some() { + bail!("--description can only be used when installing one extension"); + } + + for payload in &artifacts.payloads { + validate_extension_name(&payload.name)?; + } + + let ext_base = + extension_dir().context("could not determine config directory for extensions")?; + + for payload in &artifacts.payloads { + let ext_dir = ext_base.join(format!("pup-{}", payload.name)); + if ext_dir.exists() && !force { + bail!( + "extension '{}' is already installed (use --force to overwrite)", + payload.name + ); + } + } + + let mut installed = Vec::new(); + for payload in artifacts.payloads { + let ext_dir = ext_base.join(format!("pup-{}", payload.name)); + prepare_extension_dir(&ext_dir)?; + let exe_name = write_extension_binary(&ext_dir, &payload.name, &payload.bytes)?; + + let manifest = Manifest { + name: payload.name.clone(), + version: artifacts.version.clone(), + source: format!("github:{source}"), + source_kind: artifacts.source_kind.clone(), + source_release_tag: artifacts.source_release_tag.clone(), + source_asset: artifacts.source_asset.clone(), + installed_at: chrono_now_iso(), + binary: exe_name, + description: description.unwrap_or_default().to_string(), + installed_by_pup: version::VERSION.to_string(), + }; + manifest.save(&ext_dir.join("manifest.json"))?; + installed.push(payload.name); + } + + Ok(installed) +} + +async fn required_archive_from_release( + client: &reqwest::Client, + owner: &str, + repo: &str, + release: &GitHubRelease, +) -> Result { + match download_archive_from_release(client, release, repo).await? { + Some(archive) => Ok(archive), + None => bail!( + "release {} in {owner}/{repo} does not include a platform archive for {}-{}", + release.tag_name, + archive_platform_os(), + archive_platform_arch() + ), + } +} + +async fn archive_artifacts_for_request( + client: &reqwest::Client, + owner: &str, + repo: &str, + tag: Option<&str>, + extension: Option<&str>, + all: bool, +) -> Result { + if let Some(extension) = extension { + validate_extension_name(extension)?; + } + + if tag.is_none() { + let Some(extension) = extension else { + let release = fetch_github_release(client, owner, repo, tag).await?; + let archive = required_archive_from_release(client, owner, repo, &release).await?; + let names = selected_archive_extension_names(&archive.extensions, None, all)?; + let payloads = archive_extension_payloads(&archive, &names)?; + return Ok(GitHubInstallArtifacts { + version: archive.version, + source_kind: Some("github_archive".to_string()), + source_release_tag: Some(archive.release_tag), + source_asset: Some(archive.asset_name), + payloads, + }); + }; + let releases = fetch_github_releases(client, owner, repo).await?; + for release in &releases { + let Some(archive) = download_archive_from_release(client, release, repo).await? else { + continue; + }; + if archive.extensions.iter().any(|name| name == extension) { + let names = vec![extension.to_string()]; + let payloads = archive_extension_payloads(&archive, &names)?; + return Ok(GitHubInstallArtifacts { + version: archive.version, + source_kind: Some("github_archive".to_string()), + source_release_tag: Some(archive.release_tag), + source_asset: Some(archive.asset_name), + payloads, + }); + } + } + bail!( + "no release archive in {owner}/{repo} contains extension '{extension}' for {}-{}", + archive_platform_os(), + archive_platform_arch() + ); + } + + let release = fetch_github_release(client, owner, repo, tag).await?; + let archive = required_archive_from_release(client, owner, repo, &release).await?; + let names = selected_archive_extension_names(&archive.extensions, extension, all)?; + let payloads = archive_extension_payloads(&archive, &names)?; + Ok(GitHubInstallArtifacts { + version: archive.version, + source_kind: Some("github_archive".to_string()), + source_release_tag: Some(archive.release_tag), + source_asset: Some(archive.asset_name), + payloads, + }) +} + +async fn latest_archive_artifacts_for_extension( + client: &reqwest::Client, + owner: &str, + repo: &str, + extension: &str, +) -> Result { + validate_extension_name(extension)?; + + let releases = fetch_github_releases(client, owner, repo).await?; + for release in &releases { + let Some(archive) = download_archive_from_release(client, release, repo).await? else { + continue; + }; + if archive.extensions.iter().any(|name| name == extension) { + let names = vec![extension.to_string()]; + let payloads = archive_extension_payloads(&archive, &names)?; + return Ok(GitHubInstallArtifacts { + version: archive.version, + source_kind: Some("github_archive".to_string()), + source_release_tag: Some(archive.release_tag), + source_asset: Some(archive.asset_name), + payloads, + }); + } + } + + bail!( + "no release archive in {owner}/{repo} contains extension '{extension}' for {}-{}", + archive_platform_os(), + archive_platform_arch() + ); +} + /// Install an extension from a GitHub repository. /// Downloads the appropriate platform-specific binary from GitHub Releases. pub fn install_from_github( source: &str, tag: Option<&str>, name_override: Option<&str>, + extension: Option<&str>, + all: bool, force: bool, description: Option<&str>, -) -> Result<()> { +) -> Result> { let (owner, repo) = parse_owner_repo(source)?; - // The asset name is always derived from the repo (e.g., "hello" from "pup-hello"). - // The ext_name may be overridden by the user via --name for the local directory/manifest. - let asset_name = derive_name_from_repo(repo); - let ext_name = match name_override { - Some(n) => n.to_string(), - None => asset_name.clone(), - }; - - validate_extension_name(&ext_name)?; - + if all && description.is_some() { + bail!("--description can only be used when installing one extension"); + } + if (extension.is_some() || all) && name_override.is_some() { + bail!("--name can only be used with single-binary GitHub installs"); + } if let Some(t) = tag { if !is_valid_tag(t) { bail!( @@ -252,46 +968,234 @@ pub fn install_from_github( } } - let ext_base = - extension_dir().context("could not determine config directory for extensions")?; - let ext_dir = ext_base.join(format!("pup-{ext_name}")); - - if ext_dir.exists() && !force { - bail!("extension '{ext_name}' is already installed (use --force to overwrite)"); + if extension.is_some() || all { + let artifacts = tokio::task::block_in_place(|| { + tokio::runtime::Handle::current().block_on(async { + let client = github_client()?; + archive_artifacts_for_request(&client, owner, repo, tag, extension, all).await + }) + })?; + return save_github_payloads(source, artifacts, force, description); } + // The asset name is always derived from the repo (e.g., "hello" from "pup-hello"). + // The ext_name may be overridden by the user via --name for the local directory/manifest. + let asset_name = derive_name_from_repo(repo); + let ext_name = match name_override { + Some(n) => n.to_string(), + None => asset_name.clone(), + }; + + validate_extension_name(&ext_name)?; + // Run the async download inside the existing tokio runtime. - let (release, asset_bytes) = tokio::task::block_in_place(|| { + let artifacts = tokio::task::block_in_place(|| { tokio::runtime::Handle::current().block_on(async { let client = github_client()?; let release = fetch_github_release(&client, owner, repo, tag).await?; - let asset = find_platform_asset(&release, &asset_name)?; - let bytes = download_asset(&client, &asset.browser_download_url).await?; - Ok::<_, anyhow::Error>((release, bytes)) + match find_platform_asset(&release, &asset_name) { + Ok(asset) => { + let bytes = download_asset(&client, asset).await?; + Ok::<_, anyhow::Error>(GitHubInstallArtifacts { + version: extract_version(&release.tag_name), + source_kind: None, + source_release_tag: None, + source_asset: None, + payloads: vec![ExtensionPayload { + name: ext_name, + bytes, + }], + }) + } + Err(single_binary_error) => { + if name_override.is_some() { + return Err(single_binary_error); + } + let Some(archive) = + download_archive_from_release(&client, &release, repo).await? + else { + return Err(single_binary_error); + }; + let names = selected_archive_extension_names(&archive.extensions, None, false)?; + let payloads = archive_extension_payloads(&archive, &names)?; + Ok(GitHubInstallArtifacts { + version: archive.version, + source_kind: Some("github_archive".to_string()), + source_release_tag: Some(archive.release_tag), + source_asset: Some(archive.asset_name), + payloads, + }) + } + } }) })?; - let release_version = extract_version(&release.tag_name); + save_github_payloads(source, artifacts, force, description) +} - // Create (or recreate) the extension directory. - prepare_extension_dir(&ext_dir)?; +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RemoteExtensionVersion { + pub name: String, + pub version: String, + pub tag: String, + pub source: String, + pub asset: String, + pub inferred_from_archive: bool, +} - let exe_name = write_extension_binary(&ext_dir, &ext_name, &asset_bytes)?; +#[derive(Debug, Clone, PartialEq, Eq)] +struct ArchiveInventory { + tag: String, + version: String, + asset: String, + extensions: Vec, +} - let manifest = Manifest { - name: ext_name.clone(), - version: release_version, - source: format!("github:{source}"), - installed_at: chrono_now_iso(), - binary: exe_name, - description: description.unwrap_or_default().to_string(), - installed_by_pup: version::VERSION.to_string(), +fn remote_versions_from_archive_inventory( + source: &str, + inventories: &[ArchiveInventory], + extension_filter: Option<&str>, +) -> Vec { + let mut versions = Vec::new(); + for inventory in inventories { + let mut names = inventory.extensions.clone(); + names.sort(); + names.dedup(); + for name in names { + if extension_filter.is_some_and(|filter| filter != name) { + continue; + } + versions.push(RemoteExtensionVersion { + name, + version: inventory.version.clone(), + tag: inventory.tag.clone(), + source: format!("github:{source}"), + asset: inventory.asset.clone(), + inferred_from_archive: true, + }); + } + } + versions +} + +fn parse_checksums(checksums: &str, asset_name: &str) -> Option { + checksums.lines().find_map(|line| { + let mut parts = line.split_whitespace(); + let digest = parts.next()?; + let file = parts.next()?; + if file == asset_name { + Some(digest.to_ascii_lowercase()) + } else { + None + } + }) +} + +fn sha256_hex(bytes: &[u8]) -> String { + Sha256::digest(bytes) + .as_slice() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect() +} + +async fn verify_release_asset_checksum( + client: &reqwest::Client, + release: &GitHubRelease, + asset: &GitHubAsset, + bytes: &[u8], +) -> Result<()> { + let Some(checksums_asset) = release.assets.iter().find(|a| a.name == "checksums.txt") else { + return Ok(()); + }; + + let checksums_bytes = download_asset(client, checksums_asset).await?; + let checksums = String::from_utf8(checksums_bytes).context("checksums.txt is not UTF-8")?; + let Some(expected) = parse_checksums(&checksums, &asset.name) else { + return Ok(()); }; - manifest.save(&ext_dir.join("manifest.json"))?; + let actual = sha256_hex(bytes); + if actual != expected { + bail!( + "checksum mismatch for {}: expected {}, got {}", + asset.name, + expected, + actual + ); + } Ok(()) } +async fn archive_inventory_from_release( + client: &reqwest::Client, + release: &GitHubRelease, + project_name: &str, +) -> Result> { + Ok(download_archive_from_release(client, release, project_name) + .await? + .map(|archive| ArchiveInventory { + tag: archive.release_tag, + version: archive.version, + asset: archive.asset_name, + extensions: archive.extensions, + })) +} + +async fn download_archive_from_release( + client: &reqwest::Client, + release: &GitHubRelease, + project_name: &str, +) -> Result> { + let asset = match find_platform_archive_asset(release, project_name) { + Ok(asset) => asset, + Err(_) => return Ok(None), + }; + let bytes = download_asset(client, asset).await?; + verify_release_asset_checksum(client, release, asset, &bytes).await?; + let extensions = extension_names_from_archive(&asset.name, &bytes)?; + + Ok(Some(ArchiveDownload { + release_tag: release.tag_name.clone(), + version: extract_version(&release.tag_name), + asset_name: asset.name.clone(), + bytes, + extensions, + })) +} + +pub fn list_remote_extensions( + source: &str, + extension: Option<&str>, +) -> Result> { + let (owner, repo) = parse_owner_repo(source)?; + if let Some(extension) = extension { + validate_extension_name(extension)?; + } + + let inventories = tokio::task::block_in_place(|| { + tokio::runtime::Handle::current().block_on(async { + let client = github_client()?; + let releases = fetch_github_releases(&client, owner, repo).await?; + let mut inventories = Vec::new(); + for release in &releases { + if let Some(inventory) = + archive_inventory_from_release(&client, release, repo).await? + { + inventories.push(inventory); + } + } + Ok::<_, anyhow::Error>(inventories) + }) + })?; + + Ok(remote_versions_from_archive_inventory( + source, + &inventories, + extension, + )) +} + /// Upgrade a single GitHub-sourced extension. Returns a message describing what happened. pub fn upgrade_extension(name: &str) -> Result { validate_extension_name(name)?; @@ -338,6 +1242,37 @@ pub fn upgrade_extension(name: &str) -> Result { // Build the HTTP client once for both the metadata fetch and the binary download. let client = github_client()?; + if manifest.source_kind.as_deref() == Some("github_archive") { + let old_version = manifest.version.clone(); + let old_release_tag = manifest.source_release_tag.clone(); + let description = manifest.description.clone(); + let artifacts = tokio::task::block_in_place(|| { + tokio::runtime::Handle::current().block_on(async { + latest_archive_artifacts_for_extension(&client, owner, repo, name).await + }) + })?; + + let new_version = artifacts.version.clone(); + let new_release_tag = artifacts.source_release_tag.clone(); + let already_latest = match old_release_tag { + Some(old_tag) => Some(old_tag.as_str()) == new_release_tag.as_deref(), + None => new_version == old_version, + }; + + if already_latest { + return Ok(format!("{name}: already at latest version ({new_version})")); + } + + let description = if description.is_empty() { + None + } else { + Some(description.as_str()) + }; + save_github_payloads(gh_source, artifacts, true, description)?; + + return Ok(format!("{name}: upgraded {old_version} -> {new_version}")); + } + // Step 1: Fetch the release metadata (small JSON) and check version BEFORE downloading. let release = tokio::task::block_in_place(|| { tokio::runtime::Handle::current() @@ -354,11 +1289,9 @@ pub fn upgrade_extension(name: &str) -> Result { // Step 2: Version differs - now download the binary. let asset = find_platform_asset(&release, &asset_name)?; - let asset_url = asset.browser_download_url.clone(); let asset_bytes = tokio::task::block_in_place(|| { - tokio::runtime::Handle::current() - .block_on(async { download_asset(&client, &asset_url).await }) + tokio::runtime::Handle::current().block_on(async { download_asset(&client, asset).await }) })?; // Prepare (recreate) the extension directory before writing, so a failed write @@ -371,6 +1304,9 @@ pub fn upgrade_extension(name: &str) -> Result { let updated_manifest = Manifest { version: new_version.clone(), source: format!("github:{gh_source}"), + source_kind: None, + source_release_tag: None, + source_asset: None, installed_at: chrono_now_iso(), binary: exe_name, installed_by_pup: version::VERSION.to_string(), @@ -518,6 +1454,9 @@ pub fn install_from_local( name: name.to_string(), version: "unknown".to_string(), source: source_str, + source_kind: None, + source_release_tag: None, + source_asset: None, installed_at: chrono_now_iso(), binary: exe_name, description: description.unwrap_or_default().to_string(), @@ -614,6 +1553,97 @@ mod tests { ); } + #[test] + fn test_resolve_github_auth_prefers_env_token_over_gh() { + let auth = resolve_github_auth_with( + |name| match name { + "GH_TOKEN" => Some(" env-token \n".to_string()), + _ => None, + }, + || Ok("gh-token\n".to_string()), + ); + + assert_eq!(auth.token.as_deref(), Some("env-token")); + assert_eq!(auth.source, Some(GitHubAuthSource::Env("GH_TOKEN"))); + assert!(auth.gh_error.is_none()); + } + + #[test] + fn test_resolve_github_auth_ignores_empty_env_and_uses_gh() { + let auth = resolve_github_auth_with( + |name| match name { + "GH_TOKEN" => Some(" ".to_string()), + "GITHUB_TOKEN" => None, + "HOMEBREW_GITHUB_API_TOKEN" => None, + _ => None, + }, + || Ok(" gh-token \n".to_string()), + ); + + assert_eq!(auth.token.as_deref(), Some("gh-token")); + assert_eq!(auth.source, Some(GitHubAuthSource::GhActive)); + assert!(auth.gh_error.is_none()); + } + + #[test] + fn test_resolve_github_auth_falls_back_to_anonymous_when_gh_fails() { + let auth = + resolve_github_auth_with(|_| None, || Err(anyhow::anyhow!("gh is not installed"))); + + assert_eq!(auth.token, None); + assert_eq!(auth.source, None); + assert_eq!(auth.gh_error.as_deref(), Some("gh is not installed")); + } + + #[test] + fn test_github_access_guidance_for_active_gh_token_does_not_include_token() { + let auth = GitHubAuthResolution { + token: Some("secret-token".to_string()), + source: Some(GitHubAuthSource::GhActive), + gh_error: None, + }; + + let guidance = github_access_guidance("owner", "repo", &auth, None); + + assert!(guidance.contains("active GitHub CLI account")); + assert!(guidance.contains("gh auth status --hostname github.com")); + assert!(guidance.contains("gh auth switch --hostname github.com")); + assert!(!guidance.contains("secret-token")); + } + + #[test] + fn test_github_access_guidance_for_no_token_mentions_env_and_gh_login() { + let auth = GitHubAuthResolution { + token: None, + source: None, + gh_error: Some("gh is not installed".to_string()), + }; + + let guidance = github_access_guidance("owner", "repo", &auth, None); + + assert!(guidance.contains("export GH_TOKEN=")); + assert!(guidance.contains("gh auth login --hostname github.com --scopes repo")); + assert!(guidance.contains("gh is not installed")); + } + + #[test] + fn test_github_auth_status_diagnostic_reports_multiple_accounts() { + let json = r#"{ + "hosts": { + "github.com": [ + {"state": "success", "active": true, "login": "alice"}, + {"state": "success", "active": false, "login": "bob"} + ] + } + }"#; + + let diagnostic = github_auth_status_diagnostic_from_json(json).unwrap(); + + assert!(diagnostic.contains("multiple accounts")); + assert!(diagnostic.contains("alice")); + assert!(!diagnostic.contains("bob")); + } + #[test] fn test_remove_rejects_path_traversal() { // remove_extension must reject names with path traversal characters @@ -704,18 +1734,22 @@ mod tests { assets: vec![ GitHubAsset { name: "pup-hello-linux-x86_64".to_string(), + url: None, browser_download_url: "https://example.com/linux-x86_64".to_string(), }, GitHubAsset { name: "pup-hello-darwin-aarch64".to_string(), + url: None, browser_download_url: "https://example.com/darwin-aarch64".to_string(), }, GitHubAsset { name: "pup-hello-darwin-x86_64".to_string(), + url: None, browser_download_url: "https://example.com/darwin-x86_64".to_string(), }, GitHubAsset { name: "pup-hello-windows-x86_64".to_string(), + url: None, browser_download_url: "https://example.com/windows-x86_64".to_string(), }, ], @@ -730,6 +1764,7 @@ mod tests { tag_name: "v1.0.0".to_string(), assets: vec![GitHubAsset { name: "pup-hello-fakeos-fakearch".to_string(), + url: None, browser_download_url: "https://example.com/fake".to_string(), }], }; @@ -802,6 +1837,7 @@ mod tests { tag_name: "v1.0.0".to_string(), assets: vec![GitHubAsset { name: format!("pup-hello-{os}-{arch}"), + url: None, browser_download_url: "https://example.com/hello".to_string(), }], }; @@ -819,4 +1855,254 @@ mod tests { assert_eq!(derive_name_from_repo("pup-my-extension"), "my-extension"); assert_eq!(derive_name_from_repo("my-tool"), "my-tool"); } + + fn make_tar_gz(entries: &[(&str, &[u8])]) -> Vec { + let mut gz = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); + { + let mut tar = tar::Builder::new(&mut gz); + for (path, bytes) in entries { + let mut header = tar::Header::new_gnu(); + header.set_size(bytes.len() as u64); + header.set_mode(0o755); + header.set_cksum(); + tar.append_data(&mut header, path, *bytes).unwrap(); + } + tar.finish().unwrap(); + } + gz.finish().unwrap() + } + + fn make_zip(entries: &[(&str, &[u8])]) -> Vec { + let mut cursor = Cursor::new(Vec::new()); + { + let mut zip = zip::ZipWriter::new(&mut cursor); + let options = zip::write::SimpleFileOptions::default() + .compression_method(zip::CompressionMethod::Deflated) + .unix_permissions(0o755); + for (path, bytes) in entries { + zip.start_file(path, options).unwrap(); + std::io::Write::write_all(&mut zip, bytes).unwrap(); + } + zip.finish().unwrap(); + } + cursor.into_inner() + } + + #[test] + fn test_find_platform_archive_asset_found() { + let version = "1.2.3"; + let release = GitHubRelease { + tag_name: format!("v{version}"), + assets: vec![ + GitHubAsset { + name: format!("bundle_{version}_Darwin_arm64.tar.gz"), + url: None, + browser_download_url: "https://example.com/darwin-arm64".to_string(), + }, + GitHubAsset { + name: format!("bundle_{version}_Darwin_x86_64.tar.gz"), + url: None, + browser_download_url: "https://example.com/darwin-x86_64".to_string(), + }, + GitHubAsset { + name: format!("bundle_{version}_Linux_arm64.tar.gz"), + url: None, + browser_download_url: "https://example.com/linux-arm64".to_string(), + }, + GitHubAsset { + name: format!("bundle_{version}_Linux_x86_64.tar.gz"), + url: None, + browser_download_url: "https://example.com/linux-x86_64".to_string(), + }, + GitHubAsset { + name: format!("bundle_{version}_Windows_x86_64.zip"), + url: None, + browser_download_url: "https://example.com/windows-x86_64".to_string(), + }, + ], + }; + + let asset = find_platform_archive_asset(&release, "bundle").unwrap(); + assert!( + asset.name.contains(&format!( + "_{}_{}.", + archive_platform_os(), + archive_platform_arch() + )), + "selected archive should match this platform, got {}", + asset.name + ); + } + + #[test] + fn test_extension_names_from_archive_tar_gz() { + let archive = make_tar_gz(&[ + ("README.md", b"docs"), + ("pup-foo", b"foo"), + ("pup-bar", b"bar"), + ("nested/pup-hidden", b"hidden"), + ("not-pup", b"ignored"), + ]); + + let names = extension_names_from_archive("bundle_1.2.3_Darwin_arm64.tar.gz", &archive) + .expect("archive should parse"); + + assert_eq!(names, vec!["bar".to_string(), "foo".to_string()]); + } + + #[test] + fn test_extract_extension_from_archive_tar_gz() { + let archive = make_tar_gz(&[ + ("pup-foo", b"foo"), + ("pup-bar", b"bar"), + ("nested/pup-bar", b"wrong"), + ]); + + let extracted = + extract_extension_from_archive("bundle_1.2.3_Darwin_arm64.tar.gz", &archive, "bar") + .expect("bar should be extracted"); + + assert_eq!(extracted, b"bar"); + } + + #[test] + fn test_extension_names_from_archive_zip() { + let archive = make_zip(&[ + ("README.md", b"docs"), + ("pup-foo.exe", b"foo"), + ("pup-bar.exe", b"bar"), + ("nested/pup-hidden.exe", b"hidden"), + ]); + + let names = extension_names_from_archive("bundle_1.2.3_Windows_x86_64.zip", &archive) + .expect("zip archive should parse"); + + assert_eq!(names, vec!["bar".to_string(), "foo".to_string()]); + } + + #[test] + fn test_extract_extension_from_archive_zip() { + let archive = make_zip(&[ + ("pup-foo.exe", b"foo"), + ("pup-bar.exe", b"bar"), + ("nested/pup-bar.exe", b"wrong"), + ]); + + let extracted = + extract_extension_from_archive("bundle_1.2.3_Windows_x86_64.zip", &archive, "bar") + .expect("bar should be extracted from zip"); + + assert_eq!(extracted, b"bar"); + } + + #[test] + fn test_selected_archive_extension_names_accepts_exact_extension() { + let available = vec!["bar".to_string(), "foo".to_string()]; + + let selected = selected_archive_extension_names(&available, Some("foo"), false) + .expect("foo should be selectable"); + + assert_eq!(selected, vec!["foo".to_string()]); + } + + #[test] + fn test_selected_archive_extension_names_rejects_missing_extension() { + let available = vec!["bar".to_string()]; + + let result = selected_archive_extension_names(&available, Some("foo"), false); + + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("Available extensions: bar")); + } + + #[test] + fn test_selected_archive_extension_names_requires_choice_for_multiple() { + let available = vec!["foo".to_string(), "bar".to_string()]; + + let result = selected_archive_extension_names(&available, None, false); + + assert!(result.is_err()); + let message = result.unwrap_err().to_string(); + assert!(message.contains("--extension")); + assert!(message.contains("--all")); + } + + #[test] + fn test_selected_archive_extension_names_infers_single() { + let available = vec!["foo".to_string()]; + + let selected = selected_archive_extension_names(&available, None, false) + .expect("single extension should be inferred"); + + assert_eq!(selected, vec!["foo".to_string()]); + } + + #[test] + fn test_selected_archive_extension_names_all_returns_sorted_names() { + let available = vec!["foo".to_string(), "bar".to_string()]; + + let selected = + selected_archive_extension_names(&available, None, true).expect("all should select"); + + assert_eq!(selected, vec!["bar".to_string(), "foo".to_string()]); + } + + #[test] + fn test_remote_versions_are_inferred_per_extension() { + let releases = vec![ + ArchiveInventory { + tag: "v0.2.0".to_string(), + version: "0.2.0".to_string(), + asset: "bundle_0.2.0_Darwin_arm64.tar.gz".to_string(), + extensions: vec!["bar".to_string(), "foo".to_string()], + }, + ArchiveInventory { + tag: "v0.1.0".to_string(), + version: "0.1.0".to_string(), + asset: "bundle_0.1.0_Darwin_arm64.tar.gz".to_string(), + extensions: vec!["foo".to_string()], + }, + ]; + + let versions = remote_versions_from_archive_inventory("owner/repo", &releases, None); + + assert_eq!( + versions + .iter() + .map(|v| (v.name.as_str(), v.tag.as_str())) + .collect::>(), + vec![("bar", "v0.2.0"), ("foo", "v0.2.0"), ("foo", "v0.1.0")] + ); + } + + #[test] + fn test_remote_versions_can_filter_one_extension() { + let releases = vec![ + ArchiveInventory { + tag: "v0.2.0".to_string(), + version: "0.2.0".to_string(), + asset: "bundle_0.2.0_Darwin_arm64.tar.gz".to_string(), + extensions: vec!["bar".to_string(), "foo".to_string()], + }, + ArchiveInventory { + tag: "v0.1.0".to_string(), + version: "0.1.0".to_string(), + asset: "bundle_0.1.0_Darwin_arm64.tar.gz".to_string(), + extensions: vec!["foo".to_string()], + }, + ]; + + let versions = remote_versions_from_archive_inventory("owner/repo", &releases, Some("foo")); + + assert_eq!( + versions + .iter() + .map(|v| (v.name.as_str(), v.tag.as_str())) + .collect::>(), + vec![("foo", "v0.2.0"), ("foo", "v0.1.0")] + ); + } } diff --git a/src/extensions/manifest.rs b/src/extensions/manifest.rs index b3998675..76347804 100644 --- a/src/extensions/manifest.rs +++ b/src/extensions/manifest.rs @@ -7,6 +7,12 @@ pub struct Manifest { pub name: String, pub version: String, pub source: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub source_kind: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub source_release_tag: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub source_asset: Option, pub installed_at: String, pub binary: String, pub description: String, @@ -38,6 +44,9 @@ mod tests { name: "hello".to_string(), version: "0.1.0".to_string(), source: "local:/tmp/pup-hello".to_string(), + source_kind: None, + source_release_tag: None, + source_asset: None, installed_at: "2026-03-29T00:00:00Z".to_string(), binary: "pup-hello".to_string(), description: "A hello world extension".to_string(), @@ -64,6 +73,38 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn test_manifest_archive_metadata_roundtrip() { + let dir = std::env::temp_dir().join("pup-test-manifest-archive-roundtrip"); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("manifest.json"); + + let original = Manifest { + name: "foo".to_string(), + version: "1.2.3".to_string(), + source: "github:owner/repo".to_string(), + source_kind: Some("github_archive".to_string()), + source_release_tag: Some("v1.2.3".to_string()), + source_asset: Some("repo_1.2.3_Darwin_arm64.tar.gz".to_string()), + installed_at: "2026-03-29T00:00:00Z".to_string(), + binary: "pup-foo".to_string(), + description: String::new(), + installed_by_pup: "1.1.0".to_string(), + }; + original.save(&path).unwrap(); + + let loaded = Manifest::load(&path).unwrap(); + assert_eq!(loaded.source_kind.as_deref(), Some("github_archive")); + assert_eq!(loaded.source_release_tag.as_deref(), Some("v1.2.3")); + assert_eq!( + loaded.source_asset.as_deref(), + Some("repo_1.2.3_Darwin_arm64.tar.gz") + ); + + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn test_manifest_load_missing() { let result = Manifest::load(Path::new("/nonexistent/manifest.json")); diff --git a/src/main.rs b/src/main.rs index 5c812ec1..16038122 100644 --- a/src/main.rs +++ b/src/main.rs @@ -2781,6 +2781,12 @@ enum ExtensionActions { Install { /// Source: local file path (with --local) or GitHub owner/repo source: String, + /// Install one extension from a GitHub release archive (without pup- prefix) + #[arg(long, conflicts_with_all = ["local", "name", "all"])] + extension: Option, + /// Install all extensions found in a GitHub release archive + #[arg(long, conflicts_with_all = ["local", "extension", "name", "description"])] + all: bool, /// Install a specific release tag (GitHub only) #[arg(long, conflicts_with = "local")] tag: Option, @@ -2800,6 +2806,14 @@ enum ExtensionActions { #[arg(long)] description: Option, }, + /// List extensions available from a remote GitHub repository + ListRemote { + /// Source: GitHub owner/repo + source: String, + /// Filter to one extension (without pup- prefix) + #[arg(long)] + extension: Option, + }, /// Remove an installed extension Remove { /// Extension name (without pup- prefix) @@ -15267,6 +15281,8 @@ async fn main_inner() -> anyhow::Result<()> { ExtensionActions::List => commands::extension::list(&cfg)?, ExtensionActions::Install { source, + extension, + all, tag, local, link, @@ -15278,6 +15294,8 @@ async fn main_inner() -> anyhow::Result<()> { &cfg, commands::extension::InstallOptions { source, + extension, + all, tag, local, link, @@ -15287,6 +15305,9 @@ async fn main_inner() -> anyhow::Result<()> { }, )?; } + ExtensionActions::ListRemote { source, extension } => { + commands::extension::list_remote(&cfg, source, extension)?; + } ExtensionActions::Remove { name } => commands::extension::remove(&cfg, name)?, ExtensionActions::Upgrade { name, all } => { commands::extension::upgrade(&cfg, name, all)?; diff --git a/src/test_commands.rs b/src/test_commands.rs index 868f92b1..f68db77f 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -162,6 +162,118 @@ fn test_auth_status_site_flag_is_optional() { } } +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn test_extension_install_accepts_remote_extension_selector() { + use clap::Parser; + + let cli = crate::Cli::try_parse_from([ + "pup", + "extension", + "install", + "owner/repo", + "--extension", + "foo", + ]) + .expect("extension install --extension should parse"); + + match cli.command { + crate::Commands::Extension { action } => match action { + crate::ExtensionActions::Install { + source, extension, .. + } => { + assert_eq!(source, "owner/repo"); + assert_eq!(extension.as_deref(), Some("foo")); + } + _ => panic!("expected ExtensionActions::Install"), + }, + _ => panic!("expected Commands::Extension"), + } +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn test_extension_install_accepts_all_remote_extensions() { + use clap::Parser; + + let cli = crate::Cli::try_parse_from(["pup", "extension", "install", "owner/repo", "--all"]) + .expect("extension install --all should parse"); + + match cli.command { + crate::Commands::Extension { action } => match action { + crate::ExtensionActions::Install { all, .. } => { + assert!(all); + } + _ => panic!("expected ExtensionActions::Install"), + }, + _ => panic!("expected Commands::Extension"), + } +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn test_extension_install_rejects_remote_extension_with_name_override() { + use clap::Parser; + + let result = crate::Cli::try_parse_from([ + "pup", + "extension", + "install", + "owner/repo", + "--extension", + "foo", + "--name", + "bar", + ]); + + assert!(result.is_err()); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn test_extension_install_rejects_all_with_description() { + use clap::Parser; + + let result = crate::Cli::try_parse_from([ + "pup", + "extension", + "install", + "owner/repo", + "--all", + "--description", + "example", + ]); + + assert!(result.is_err()); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn test_extension_list_remote_parses() { + use clap::Parser; + + let cli = crate::Cli::try_parse_from([ + "pup", + "extension", + "list-remote", + "owner/repo", + "--extension", + "foo", + ]) + .expect("extension list-remote should parse"); + + match cli.command { + crate::Commands::Extension { action } => match action { + crate::ExtensionActions::ListRemote { source, extension } => { + assert_eq!(source, "owner/repo"); + assert_eq!(extension.as_deref(), Some("foo")); + } + _ => panic!("expected ExtensionActions::ListRemote"), + }, + _ => panic!("expected Commands::Extension"), + } +} + #[test] fn test_top_level_commands_sorted_alphabetically() { let app = crate::Cli::command(); From eed1fc47445527a2edcad6876e51bf402c501076 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Wed, 17 Jun 2026 14:47:07 +0200 Subject: [PATCH 02/12] fix(extension): clarify exact release tag installs --- docs/EXTENSIONS.md | 10 ++++ src/extensions/install.rs | 109 ++++++++++++++++++++++++++++++++------ 2 files changed, 102 insertions(+), 17 deletions(-) diff --git a/docs/EXTENSIONS.md b/docs/EXTENSIONS.md index 8814c677..9ba91a69 100644 --- a/docs/EXTENSIONS.md +++ b/docs/EXTENSIONS.md @@ -244,6 +244,8 @@ To install a specific release tag: pup extension install owner/repo --tag v1.0.0 ``` +`--tag` expects the exact GitHub release tag. If the release is tagged `v1.0.0`, use `--tag v1.0.0`, not `--tag 1.0.0`. + ### Shared GitHub release repositories A GitHub repository can also publish one platform archive containing multiple top-level extension executables: @@ -268,12 +270,20 @@ pup extension list-remote owner/repo pup extension list-remote owner/repo --extension foo ``` +The table output shows both the extension version and the GitHub release tag in parentheses. Use the tag value when installing a specific release. + Install one extension from the newest release archive that contains it: ```bash pup extension install owner/repo --extension foo ``` +Install one extension from a specific release tag: + +```bash +pup extension install owner/repo --extension foo --tag v1.0.0 +``` + Install all extensions from the latest release archive: ```bash diff --git a/src/extensions/install.rs b/src/extensions/install.rs index e1d41c64..9b12fd97 100644 --- a/src/extensions/install.rs +++ b/src/extensions/install.rs @@ -295,10 +295,30 @@ async fn fetch_github_release( repo: &str, tag: Option<&str>, ) -> Result { - let url = match tag { - Some(t) => format!("https://api.github.com/repos/{owner}/{repo}/releases/tags/{t}"), - None => format!("https://api.github.com/repos/{owner}/{repo}/releases/latest"), - }; + if let Some(tag) = tag { + let url = format!("https://api.github.com/repos/{owner}/{repo}/releases/tags/{tag}"); + let resp = github_api_get(client, &url) + .send() + .await + .with_context(|| format!("fetching release from {url}"))?; + + let status = resp.status(); + if status == reqwest::StatusCode::NOT_FOUND { + bail!("{}", release_tag_not_found_message(owner, repo, tag)); + } + if !status.is_success() { + let body = resp.text().await.unwrap_or_default(); + let guidance = github_failure_guidance(owner, repo); + bail!("GitHub API returned {status} for {url}: {body}\n\n{guidance}"); + } + + return resp + .json::() + .await + .with_context(|| format!("parsing release JSON from {url}")); + } + + let url = format!("https://api.github.com/repos/{owner}/{repo}/releases/latest"); let resp = github_api_get(client, &url) .send() @@ -308,19 +328,12 @@ async fn fetch_github_release( let status = resp.status(); if status == reqwest::StatusCode::NOT_FOUND { let guidance = github_failure_guidance(owner, repo); - match tag { - Some(t) => bail!( - "release tag '{t}' not found in {owner}/{repo}. \ - Check that the tag exists at https://github.com/{owner}/{repo}/releases\n\n\ - {guidance}" - ), - None => bail!( - "no releases found for {owner}/{repo}. \ - Check that the repository exists and has at least one release at \ - https://github.com/{owner}/{repo}/releases\n\n\ - {guidance}" - ), - } + bail!( + "no releases found for {owner}/{repo}. \ + Check that the repository exists and has at least one release at \ + https://github.com/{owner}/{repo}/releases\n\n\ + {guidance}" + ); } if !status.is_success() { let body = resp.text().await.unwrap_or_default(); @@ -674,6 +687,38 @@ fn is_valid_tag(tag: &str) -> bool { }) } +fn release_tag_suggestion(tag: &str) -> Option { + if !tag.starts_with('v') && looks_like_semver_tag(tag) { + Some(format!("v{tag}")) + } else { + None + } +} + +fn looks_like_semver_tag(tag: &str) -> bool { + let core = tag.split_once(['-', '+']).map_or(tag, |(core, _)| core); + let parts: Vec<&str> = core.split('.').collect(); + parts.len() == 3 + && parts + .iter() + .all(|part| !part.is_empty() && part.chars().all(|c| c.is_ascii_digit())) +} + +fn release_tag_not_found_message(owner: &str, repo: &str, tag: &str) -> String { + let hint = if let Some(suggestion) = release_tag_suggestion(tag) { + format!( + "\n\nIf you copied a version from `pup extension list-remote`, use the tag shown in parentheses:\n pup extension install {owner}/{repo} --extension foo --tag {}", + suggestion + ) + } else { + String::new() + }; + format!( + "release tag '{tag}' not found in {owner}/{repo}. `--tag` uses exact GitHub release tags. \ + Check available releases at https://github.com/{owner}/{repo}/releases{hint}" + ) +} + /// Parse an "owner/repo" string into (owner, repo). pub fn parse_owner_repo(source: &str) -> Result<(&str, &str)> { let parts: Vec<&str> = source.splitn(2, '/').collect(); @@ -1827,6 +1872,36 @@ mod tests { assert!(!is_valid_tag("v1.0.0\nnewline")); } + #[test] + fn test_release_tag_suggestion_adds_v_for_plain_version() { + assert_eq!(release_tag_suggestion("0.2.1").as_deref(), Some("v0.2.1")); + } + + #[test] + fn test_release_tag_suggestion_ignores_existing_v_tag() { + assert_eq!(release_tag_suggestion("v0.2.1"), None); + } + + #[test] + fn test_release_tag_suggestion_ignores_slash_tags() { + assert_eq!(release_tag_suggestion("release/v2.0"), None); + } + + #[test] + fn test_release_tag_suggestion_ignores_non_semver_tags() { + assert_eq!(release_tag_suggestion("latest"), None); + } + + #[test] + fn test_release_tag_not_found_message_suggests_listed_tag() { + let message = release_tag_not_found_message("owner", "repo", "0.2.1"); + + assert!(message.contains("release tag '0.2.1' not found")); + assert!(message.contains("exact GitHub release tags")); + assert!(message.contains("--tag v0.2.1")); + assert!(!message.contains("GitHub access failed")); + } + #[test] fn test_find_platform_asset_uses_asset_name_not_ext_name() { // Verify that find_platform_asset uses the repo-derived name, not a user override. From daac2038a62fac4e942726d07fe40385c1ad7a05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Wed, 17 Jun 2026 15:08:48 +0200 Subject: [PATCH 03/12] fix(extension): harden shared release installs --- src/extensions/install.rs | 319 ++++++++++++++++++++++++++++++++------ 1 file changed, 268 insertions(+), 51 deletions(-) diff --git a/src/extensions/install.rs b/src/extensions/install.rs index 9b12fd97..64e8ba3e 100644 --- a/src/extensions/install.rs +++ b/src/extensions/install.rs @@ -9,6 +9,10 @@ use super::discovery::extension_dir; use super::manifest::Manifest; use crate::version; +const MAX_REMOTE_LIST_RELEASES: usize = 100; +const MAX_REMOTE_LIST_ARCHIVE_BYTES: usize = 100 * 1024 * 1024; +const MAX_IMPLICIT_RELEASE_SCAN: usize = 100; + /// GitHub release asset metadata (subset of the GitHub Releases API response). #[derive(Debug, serde::Deserialize)] struct GitHubAsset { @@ -22,6 +26,10 @@ struct GitHubAsset { #[derive(Debug, serde::Deserialize)] struct GitHubRelease { tag_name: String, + #[serde(default)] + draft: bool, + #[serde(default)] + prerelease: bool, assets: Vec, } @@ -245,7 +253,7 @@ fn github_access_guidance( To switch accounts:\n\ gh auth switch --hostname github.com\n\n\ Or provide a token explicitly:\n\ - GH_TOKEN= pup extension install {owner}/{repo} --extension foo" + GH_TOKEN= pup extension install {owner}/{repo} --extension " ), None => format!( "GitHub access failed for {owner}/{repo}.\n\n\ @@ -294,6 +302,7 @@ async fn fetch_github_release( owner: &str, repo: &str, tag: Option<&str>, + extension_hint: Option<&str>, ) -> Result { if let Some(tag) = tag { let url = format!("https://api.github.com/repos/{owner}/{repo}/releases/tags/{tag}"); @@ -304,7 +313,12 @@ async fn fetch_github_release( let status = resp.status(); if status == reqwest::StatusCode::NOT_FOUND { - bail!("{}", release_tag_not_found_message(owner, repo, tag)); + let guidance = github_failure_guidance(owner, repo); + bail!( + "{}\n\n{}", + release_tag_not_found_message(owner, repo, tag, extension_hint), + guidance + ); } if !status.is_success() { let body = resp.text().await.unwrap_or_default(); @@ -347,10 +361,11 @@ async fn fetch_github_release( } /// Fetch GitHub releases newest-first. -async fn fetch_github_releases( +async fn fetch_github_releases_with_limit( client: &reqwest::Client, owner: &str, repo: &str, + max_releases: Option, ) -> Result> { let mut releases = Vec::new(); let mut page = 1; @@ -384,6 +399,14 @@ async fn fetch_github_releases( .await .with_context(|| format!("parsing release JSON from {url}"))?; let count = page_releases.len(); + if let Some(max_releases) = max_releases { + if releases.len() + count > max_releases { + bail!( + "release scan for {owner}/{repo} is limited to {max_releases} releases. \ + Use --tag to install an exact release, or publish a smaller release index." + ); + } + } releases.append(&mut page_releases); if count < 100 { break; @@ -394,6 +417,10 @@ async fn fetch_github_releases( Ok(releases) } +fn is_stable_release(release: &GitHubRelease) -> bool { + !release.draft && !release.prerelease +} + /// Find the matching asset for the current platform in a release. fn find_platform_asset<'a>(release: &'a GitHubRelease, ext_name: &str) -> Result<&'a GitHubAsset> { let os = platform_os(); @@ -636,6 +663,14 @@ fn selected_archive_extension_names( /// Download a release asset. Authenticated GitHub API asset URLs are used when /// a GitHub token is available; public browser URLs remain the fallback. async fn download_asset(client: &reqwest::Client, asset: &GitHubAsset) -> Result> { + download_asset_with_limit(client, asset, None).await +} + +async fn download_asset_with_limit( + client: &reqwest::Client, + asset: &GitHubAsset, + max_bytes: Option, +) -> Result> { let token = github_token(); let use_api_asset = token.is_some() && asset.url.is_some(); let url = if use_api_asset { @@ -662,6 +697,36 @@ async fn download_asset(client: &reqwest::Client, asset: &GitHubAsset) -> Result bail!("download failed with HTTP {status} for {url}"); } + if let Some(max_bytes) = max_bytes { + if resp + .content_length() + .is_some_and(|content_length| content_length > max_bytes as u64) + { + bail!( + "asset '{}' is larger than the {} byte scan limit", + asset.name, + max_bytes + ); + } + + use futures_util::StreamExt; + + let mut bytes = Vec::new(); + let mut stream = resp.bytes_stream(); + while let Some(chunk) = stream.next().await { + let chunk = chunk.with_context(|| format!("reading asset bytes from {url}"))?; + if bytes.len() + chunk.len() > max_bytes { + bail!( + "asset '{}' is larger than the {} byte scan limit", + asset.name, + max_bytes + ); + } + bytes.extend_from_slice(&chunk); + } + return Ok(bytes); + } + resp.bytes() .await .map(|b| b.to_vec()) @@ -704,17 +769,24 @@ fn looks_like_semver_tag(tag: &str) -> bool { .all(|part| !part.is_empty() && part.chars().all(|c| c.is_ascii_digit())) } -fn release_tag_not_found_message(owner: &str, repo: &str, tag: &str) -> String { +fn release_tag_not_found_message( + owner: &str, + repo: &str, + tag: &str, + extension_hint: Option<&str>, +) -> String { let hint = if let Some(suggestion) = release_tag_suggestion(tag) { + let extension_arg = extension_hint + .map(|extension| format!(" --extension {extension}")) + .unwrap_or_default(); format!( - "\n\nIf you copied a version from `pup extension list-remote`, use the tag shown in parentheses:\n pup extension install {owner}/{repo} --extension foo --tag {}", - suggestion + "\n\nIf you copied a version from `pup extension list-remote`, use the tag shown in parentheses:\n pup extension install {owner}/{repo}{extension_arg} --tag {suggestion}" ) } else { String::new() }; format!( - "release tag '{tag}' not found in {owner}/{repo}. `--tag` uses exact GitHub release tags. \ + "release tag '{tag}' not found or repository inaccessible in {owner}/{repo}. `--tag` uses exact GitHub release tags. \ Check available releases at https://github.com/{owner}/{repo}/releases{hint}" ) } @@ -904,7 +976,7 @@ async fn archive_artifacts_for_request( if tag.is_none() { let Some(extension) = extension else { - let release = fetch_github_release(client, owner, repo, tag).await?; + let release = fetch_github_release(client, owner, repo, tag, None).await?; let archive = required_archive_from_release(client, owner, repo, &release).await?; let names = selected_archive_extension_names(&archive.extensions, None, all)?; let payloads = archive_extension_payloads(&archive, &names)?; @@ -916,8 +988,10 @@ async fn archive_artifacts_for_request( payloads, }); }; - let releases = fetch_github_releases(client, owner, repo).await?; - for release in &releases { + let releases = + fetch_github_releases_with_limit(client, owner, repo, Some(MAX_IMPLICIT_RELEASE_SCAN)) + .await?; + for release in releases.iter().filter(|release| is_stable_release(release)) { let Some(archive) = download_archive_from_release(client, release, repo).await? else { continue; }; @@ -940,7 +1014,7 @@ async fn archive_artifacts_for_request( ); } - let release = fetch_github_release(client, owner, repo, tag).await?; + let release = fetch_github_release(client, owner, repo, tag, extension).await?; let archive = required_archive_from_release(client, owner, repo, &release).await?; let names = selected_archive_extension_names(&archive.extensions, extension, all)?; let payloads = archive_extension_payloads(&archive, &names)?; @@ -961,8 +1035,10 @@ async fn latest_archive_artifacts_for_extension( ) -> Result { validate_extension_name(extension)?; - let releases = fetch_github_releases(client, owner, repo).await?; - for release in &releases { + let releases = + fetch_github_releases_with_limit(client, owner, repo, Some(MAX_IMPLICIT_RELEASE_SCAN)) + .await?; + for release in releases.iter().filter(|release| is_stable_release(release)) { let Some(archive) = download_archive_from_release(client, release, repo).await? else { continue; }; @@ -1037,7 +1113,7 @@ pub fn install_from_github( let artifacts = tokio::task::block_in_place(|| { tokio::runtime::Handle::current().block_on(async { let client = github_client()?; - let release = fetch_github_release(&client, owner, repo, tag).await?; + let release = fetch_github_release(&client, owner, repo, tag, None).await?; match find_platform_asset(&release, &asset_name) { Ok(asset) => { let bytes = download_asset(&client, asset).await?; @@ -1123,17 +1199,29 @@ fn remote_versions_from_archive_inventory( versions } -fn parse_checksums(checksums: &str, asset_name: &str) -> Option { - checksums.lines().find_map(|line| { +fn parse_checksums(checksums: &str, asset_name: &str) -> Result> { + for line in checksums.lines() { + let line = line.trim(); + if line.is_empty() || line.starts_with('#') { + continue; + } let mut parts = line.split_whitespace(); - let digest = parts.next()?; - let file = parts.next()?; + let Some(digest) = parts.next() else { + continue; + }; + let Some(file) = parts.next() else { + continue; + }; + let file = file.trim_start_matches('*'); if file == asset_name { - Some(digest.to_ascii_lowercase()) - } else { - None + let digest = digest.to_ascii_lowercase(); + if digest.len() != 64 || !digest.chars().all(|c| c.is_ascii_hexdigit()) { + bail!("invalid SHA-256 checksum for {asset_name} in checksums.txt"); + } + return Ok(Some(digest)); } - }) + } + Ok(None) } fn sha256_hex(bytes: &[u8]) -> String { @@ -1144,6 +1232,23 @@ fn sha256_hex(bytes: &[u8]) -> String { .collect() } +fn verify_checksum_contents(checksums: &str, asset_name: &str, bytes: &[u8]) -> Result<()> { + let Some(expected) = parse_checksums(checksums, asset_name)? else { + bail!("checksums.txt does not contain a SHA-256 checksum for {asset_name}"); + }; + + let actual = sha256_hex(bytes); + if actual != expected { + bail!( + "checksum mismatch for {}: expected {}, got {}", + asset_name, + expected, + actual + ); + } + Ok(()) +} + async fn verify_release_asset_checksum( client: &reqwest::Client, release: &GitHubRelease, @@ -1156,20 +1261,7 @@ async fn verify_release_asset_checksum( let checksums_bytes = download_asset(client, checksums_asset).await?; let checksums = String::from_utf8(checksums_bytes).context("checksums.txt is not UTF-8")?; - let Some(expected) = parse_checksums(&checksums, &asset.name) else { - return Ok(()); - }; - - let actual = sha256_hex(bytes); - if actual != expected { - bail!( - "checksum mismatch for {}: expected {}, got {}", - asset.name, - expected, - actual - ); - } - Ok(()) + verify_checksum_contents(&checksums, &asset.name, bytes) } async fn archive_inventory_from_release( @@ -1177,26 +1269,40 @@ async fn archive_inventory_from_release( release: &GitHubRelease, project_name: &str, ) -> Result> { - Ok(download_archive_from_release(client, release, project_name) - .await? - .map(|archive| ArchiveInventory { - tag: archive.release_tag, - version: archive.version, - asset: archive.asset_name, - extensions: archive.extensions, - })) + Ok(download_archive_from_release_with_limit( + client, + release, + project_name, + Some(MAX_REMOTE_LIST_ARCHIVE_BYTES), + ) + .await? + .map(|archive| ArchiveInventory { + tag: archive.release_tag, + version: archive.version, + asset: archive.asset_name, + extensions: archive.extensions, + })) } async fn download_archive_from_release( client: &reqwest::Client, release: &GitHubRelease, project_name: &str, +) -> Result> { + download_archive_from_release_with_limit(client, release, project_name, None).await +} + +async fn download_archive_from_release_with_limit( + client: &reqwest::Client, + release: &GitHubRelease, + project_name: &str, + max_bytes: Option, ) -> Result> { let asset = match find_platform_archive_asset(release, project_name) { Ok(asset) => asset, Err(_) => return Ok(None), }; - let bytes = download_asset(client, asset).await?; + let bytes = download_asset_with_limit(client, asset, max_bytes).await?; verify_release_asset_checksum(client, release, asset, &bytes).await?; let extensions = extension_names_from_archive(&asset.name, &bytes)?; @@ -1221,9 +1327,15 @@ pub fn list_remote_extensions( let inventories = tokio::task::block_in_place(|| { tokio::runtime::Handle::current().block_on(async { let client = github_client()?; - let releases = fetch_github_releases(&client, owner, repo).await?; + let releases = fetch_github_releases_with_limit( + &client, + owner, + repo, + Some(MAX_REMOTE_LIST_RELEASES), + ) + .await?; let mut inventories = Vec::new(); - for release in &releases { + for release in releases.iter().filter(|release| !release.draft) { if let Some(inventory) = archive_inventory_from_release(&client, release, repo).await? { @@ -1321,7 +1433,7 @@ pub fn upgrade_extension(name: &str) -> Result { // Step 1: Fetch the release metadata (small JSON) and check version BEFORE downloading. let release = tokio::task::block_in_place(|| { tokio::runtime::Handle::current() - .block_on(async { fetch_github_release(&client, owner, repo, None).await }) + .block_on(async { fetch_github_release(&client, owner, repo, None, None).await }) })?; let new_version = extract_version(&release.tag_name); @@ -1776,6 +1888,8 @@ mod tests { let expected_name = format!("pup-hello-{os}-{arch}"); let release = GitHubRelease { tag_name: "v1.0.0".to_string(), + draft: false, + prerelease: false, assets: vec![ GitHubAsset { name: "pup-hello-linux-x86_64".to_string(), @@ -1807,6 +1921,8 @@ mod tests { fn test_find_platform_asset_not_found() { let release = GitHubRelease { tag_name: "v1.0.0".to_string(), + draft: false, + prerelease: false, assets: vec![GitHubAsset { name: "pup-hello-fakeos-fakearch".to_string(), url: None, @@ -1826,6 +1942,8 @@ mod tests { fn test_find_platform_asset_empty() { let release = GitHubRelease { tag_name: "v1.0.0".to_string(), + draft: false, + prerelease: false, assets: vec![], }; let result = find_platform_asset(&release, "hello"); @@ -1894,14 +2012,48 @@ mod tests { #[test] fn test_release_tag_not_found_message_suggests_listed_tag() { - let message = release_tag_not_found_message("owner", "repo", "0.2.1"); + let message = release_tag_not_found_message("owner", "repo", "0.2.1", Some("foo")); - assert!(message.contains("release tag '0.2.1' not found")); + assert!(message.contains("release tag '0.2.1' not found or repository inaccessible")); assert!(message.contains("exact GitHub release tags")); - assert!(message.contains("--tag v0.2.1")); + assert!(message.contains("extension install owner/repo --extension foo --tag v0.2.1")); assert!(!message.contains("GitHub access failed")); } + #[test] + fn test_release_tag_not_found_message_omits_extension_when_unknown() { + let message = release_tag_not_found_message("owner", "repo", "0.2.1", None); + + assert!(message.contains("extension install owner/repo --tag v0.2.1")); + assert!(!message.contains("--extension")); + } + + #[test] + fn test_is_stable_release_rejects_drafts_and_prereleases() { + let stable = GitHubRelease { + tag_name: "v1.0.0".to_string(), + draft: false, + prerelease: false, + assets: vec![], + }; + let draft = GitHubRelease { + tag_name: "v1.0.1".to_string(), + draft: true, + prerelease: false, + assets: vec![], + }; + let prerelease = GitHubRelease { + tag_name: "v1.1.0-rc.1".to_string(), + draft: false, + prerelease: true, + assets: vec![], + }; + + assert!(is_stable_release(&stable)); + assert!(!is_stable_release(&draft)); + assert!(!is_stable_release(&prerelease)); + } + #[test] fn test_find_platform_asset_uses_asset_name_not_ext_name() { // Verify that find_platform_asset uses the repo-derived name, not a user override. @@ -1910,6 +2062,8 @@ mod tests { let arch = platform_arch(); let release = GitHubRelease { tag_name: "v1.0.0".to_string(), + draft: false, + prerelease: false, assets: vec![GitHubAsset { name: format!("pup-hello-{os}-{arch}"), url: None, @@ -1931,6 +2085,67 @@ mod tests { assert_eq!(derive_name_from_repo("my-tool"), "my-tool"); } + #[test] + fn test_parse_checksums_accepts_sha256_entry() { + let digest = sha256_hex(b"payload"); + let checksums = format!("{digest} archive.tar.gz\n"); + + let parsed = parse_checksums(&checksums, "archive.tar.gz").unwrap(); + + assert_eq!(parsed.as_deref(), Some(digest.as_str())); + } + + #[test] + fn test_parse_checksums_accepts_binary_mode_filename() { + let digest = sha256_hex(b"payload"); + let checksums = format!("{digest} *archive.tar.gz\n"); + + let parsed = parse_checksums(&checksums, "archive.tar.gz").unwrap(); + + assert_eq!(parsed.as_deref(), Some(digest.as_str())); + } + + #[test] + fn test_parse_checksums_rejects_invalid_matching_digest() { + let result = parse_checksums("not-a-sha archive.tar.gz\n", "archive.tar.gz"); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("invalid SHA-256")); + } + + #[test] + fn test_verify_checksum_contents_accepts_match() { + let digest = sha256_hex(b"payload"); + let checksums = format!("{digest} archive.tar.gz\n"); + + verify_checksum_contents(&checksums, "archive.tar.gz", b"payload").unwrap(); + } + + #[test] + fn test_verify_checksum_contents_rejects_missing_asset() { + let digest = sha256_hex(b"payload"); + let checksums = format!("{digest} other.tar.gz\n"); + let result = verify_checksum_contents(&checksums, "archive.tar.gz", b"payload"); + + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("does not contain a SHA-256 checksum")); + } + + #[test] + fn test_verify_checksum_contents_rejects_mismatch() { + let checksums = format!("{} archive.tar.gz\n", "0".repeat(64)); + let result = verify_checksum_contents(&checksums, "archive.tar.gz", b"payload"); + + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("checksum mismatch")); + } + fn make_tar_gz(entries: &[(&str, &[u8])]) -> Vec { let mut gz = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); { @@ -1968,6 +2183,8 @@ mod tests { let version = "1.2.3"; let release = GitHubRelease { tag_name: format!("v{version}"), + draft: false, + prerelease: false, assets: vec![ GitHubAsset { name: format!("bundle_{version}_Darwin_arm64.tar.gz"), From 5ec36c8ad2f340b48b299c88116298cef0aa63b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Wed, 17 Jun 2026 15:26:55 +0200 Subject: [PATCH 04/12] fix(extension): make shared archive installs safer --- src/extensions/install.rs | 692 ++++++++++++++++++++++++++++++-------- 1 file changed, 554 insertions(+), 138 deletions(-) diff --git a/src/extensions/install.rs b/src/extensions/install.rs index 64e8ba3e..26e4f151 100644 --- a/src/extensions/install.rs +++ b/src/extensions/install.rs @@ -1,9 +1,11 @@ use anyhow::{bail, Context, Result}; use sha2::{Digest, Sha256}; +use std::collections::HashSet; use std::io::{Cursor, Read}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::process::Command; use std::sync::OnceLock; +use std::time::{SystemTime, UNIX_EPOCH}; use super::discovery::extension_dir; use super::manifest::Manifest; @@ -12,6 +14,10 @@ use crate::version; const MAX_REMOTE_LIST_RELEASES: usize = 100; const MAX_REMOTE_LIST_ARCHIVE_BYTES: usize = 100 * 1024 * 1024; const MAX_IMPLICIT_RELEASE_SCAN: usize = 100; +const MAX_INSTALL_ARCHIVE_BYTES: usize = 100 * 1024 * 1024; +const MAX_EXTENSION_BINARY_BYTES: usize = 100 * 1024 * 1024; +const MAX_ARCHIVE_ENTRIES: usize = 4096; +const MAX_CHECKSUMS_BYTES: usize = 1024 * 1024; /// GitHub release asset metadata (subset of the GitHub Releases API response). #[derive(Debug, serde::Deserialize)] @@ -360,61 +366,56 @@ async fn fetch_github_release( .with_context(|| format!("parsing release JSON from {url}")) } -/// Fetch GitHub releases newest-first. -async fn fetch_github_releases_with_limit( +/// Fetch one page of GitHub releases, newest-first. +async fn fetch_github_release_page( client: &reqwest::Client, owner: &str, repo: &str, - max_releases: Option, + page: usize, ) -> Result> { - let mut releases = Vec::new(); - let mut page = 1; - - loop { - let url = format!( - "https://api.github.com/repos/{owner}/{repo}/releases?per_page=100&page={page}" - ); - let resp = github_api_get(client, &url) - .send() - .await - .with_context(|| format!("fetching releases from {url}"))?; + let url = + format!("https://api.github.com/repos/{owner}/{repo}/releases?per_page=100&page={page}"); + let resp = github_api_get(client, &url) + .send() + .await + .with_context(|| format!("fetching releases from {url}"))?; - let status = resp.status(); - if status == reqwest::StatusCode::NOT_FOUND { - let guidance = github_failure_guidance(owner, repo); - bail!( - "no releases found for {owner}/{repo}. \ + let status = resp.status(); + if status == reqwest::StatusCode::NOT_FOUND { + let guidance = github_failure_guidance(owner, repo); + bail!( + "no releases found for {owner}/{repo}. \ Check that the repository exists and is accessible\n\n\ {guidance}" - ); - } - if !status.is_success() { - let body = resp.text().await.unwrap_or_default(); - let guidance = github_failure_guidance(owner, repo); - bail!("GitHub API returned {status} for {url}: {body}\n\n{guidance}"); - } - - let mut page_releases = resp - .json::>() - .await - .with_context(|| format!("parsing release JSON from {url}"))?; - let count = page_releases.len(); - if let Some(max_releases) = max_releases { - if releases.len() + count > max_releases { - bail!( - "release scan for {owner}/{repo} is limited to {max_releases} releases. \ - Use --tag to install an exact release, or publish a smaller release index." - ); - } - } - releases.append(&mut page_releases); - if count < 100 { - break; - } - page += 1; + ); + } + if !status.is_success() { + let body = resp.text().await.unwrap_or_default(); + let guidance = github_failure_guidance(owner, repo); + bail!("GitHub API returned {status} for {url}: {body}\n\n{guidance}"); } - Ok(releases) + resp.json::>() + .await + .with_context(|| format!("parsing release JSON from {url}")) +} + +fn releases_within_scan_limit<'a>( + releases: &'a [GitHubRelease], + scanned: &mut usize, + max_releases: usize, +) -> &'a [GitHubRelease] { + let remaining = max_releases.saturating_sub(*scanned); + let count = remaining.min(releases.len()); + *scanned += count; + &releases[..count] +} + +fn release_scan_limit_message(owner: &str, repo: &str, max_releases: usize) -> String { + format!( + "searched the first {max_releases} releases in {owner}/{repo} without finding a matching \ + platform archive. Use --tag to install an exact release." + ) } fn is_stable_release(release: &GitHubRelease) -> bool { @@ -516,10 +517,14 @@ fn extension_names_from_tar_gz(bytes: &[u8]) -> Result> { let mut archive = tar::Archive::new(decoder); let mut names = Vec::new(); - for entry in archive + for (index, entry) in archive .entries() .context("reading tar.gz archive entries")? + .enumerate() { + if index >= MAX_ARCHIVE_ENTRIES { + bail!("archive contains more than {MAX_ARCHIVE_ENTRIES} entries"); + } let entry = entry.context("reading tar.gz archive entry")?; if !entry.header().entry_type().is_file() { continue; @@ -539,6 +544,10 @@ fn extension_names_from_zip(bytes: &[u8]) -> Result> { let mut archive = zip::ZipArchive::new(Cursor::new(bytes)).context("reading zip archive")?; let mut names = Vec::new(); + if archive.len() > MAX_ARCHIVE_ENTRIES { + bail!("archive contains more than {MAX_ARCHIVE_ENTRIES} entries"); + } + for i in 0..archive.len() { let file = archive .by_index(i) @@ -557,44 +566,108 @@ fn extension_names_from_zip(bytes: &[u8]) -> Result> { } fn extract_extension_from_archive(asset_name: &str, bytes: &[u8], name: &str) -> Result> { + extract_extension_from_archive_with_limit(asset_name, bytes, name, MAX_EXTENSION_BINARY_BYTES) +} + +fn extract_extension_from_archive_with_limit( + asset_name: &str, + bytes: &[u8], + name: &str, + max_member_bytes: usize, +) -> Result> { validate_extension_name(name)?; if asset_name.ends_with(".tar.gz") { - extract_extension_from_tar_gz(bytes, name) + extract_extension_from_tar_gz(bytes, name, max_member_bytes) } else if asset_name.ends_with(".zip") { - extract_extension_from_zip(bytes, name) + extract_extension_from_zip(bytes, name, max_member_bytes) } else { bail!("unsupported extension archive format: {asset_name}"); } } -fn extract_extension_from_tar_gz(bytes: &[u8], name: &str) -> Result> { +fn ensure_archive_member_size(size: u64, name: &str, max_member_bytes: usize) -> Result<()> { + if size > max_member_bytes as u64 { + bail!( + "archive member for extension '{name}' is larger than the {} byte limit", + max_member_bytes + ); + } + Ok(()) +} + +fn read_limited_archive_member( + reader: R, + name: &str, + max_member_bytes: usize, + context: &str, +) -> Result> { + let mut limited = reader.take(max_member_bytes as u64 + 1); + let mut bytes = Vec::new(); + limited + .read_to_end(&mut bytes) + .context(context.to_string())?; + if bytes.len() > max_member_bytes { + bail!( + "archive member for extension '{name}' is larger than the {} byte limit", + max_member_bytes + ); + } + Ok(bytes) +} + +fn extract_extension_from_tar_gz( + bytes: &[u8], + name: &str, + max_member_bytes: usize, +) -> Result> { let decoder = flate2::read::GzDecoder::new(Cursor::new(bytes)); let mut archive = tar::Archive::new(decoder); - for entry in archive + for (index, entry) in archive .entries() .context("reading tar.gz archive entries")? + .enumerate() { + if index >= MAX_ARCHIVE_ENTRIES { + bail!("archive contains more than {MAX_ARCHIVE_ENTRIES} entries"); + } let mut entry = entry.context("reading tar.gz archive entry")?; if !entry.header().entry_type().is_file() { continue; } - let path = entry.path().context("reading tar.gz archive path")?; - if extension_archive_member_matches(path.as_ref(), name) { - let mut bytes = Vec::new(); - entry - .read_to_end(&mut bytes) - .context("reading extension binary from tar.gz archive")?; - return Ok(bytes); + let matches = { + let path = entry.path().context("reading tar.gz archive path")?; + extension_archive_member_matches(path.as_ref(), name) + }; + if matches { + let size = entry + .header() + .size() + .context("reading tar.gz archive entry size")?; + ensure_archive_member_size(size, name, max_member_bytes)?; + return read_limited_archive_member( + &mut entry, + name, + max_member_bytes, + "reading extension binary from tar.gz archive", + ); } } bail!("archive does not contain extension 'pup-{name}'") } -fn extract_extension_from_zip(bytes: &[u8], name: &str) -> Result> { +fn extract_extension_from_zip( + bytes: &[u8], + name: &str, + max_member_bytes: usize, +) -> Result> { let mut archive = zip::ZipArchive::new(Cursor::new(bytes)).context("reading zip archive")?; + if archive.len() > MAX_ARCHIVE_ENTRIES { + bail!("archive contains more than {MAX_ARCHIVE_ENTRIES} entries"); + } + for i in 0..archive.len() { let mut file = archive .by_index(i) @@ -603,10 +676,13 @@ fn extract_extension_from_zip(bytes: &[u8], name: &str) -> Result> { continue; } if extension_archive_member_matches(Path::new(file.name()), name) { - let mut bytes = Vec::new(); - file.read_to_end(&mut bytes) - .context("reading extension binary from zip archive")?; - return Ok(bytes); + ensure_archive_member_size(file.size(), name, max_member_bytes)?; + return read_limited_archive_member( + &mut file, + name, + max_member_bytes, + "reading extension binary from zip archive", + ); } } @@ -861,6 +937,104 @@ fn write_extension_binary(ext_dir: &Path, name: &str, bytes: &[u8]) -> Result PathBuf { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_nanos()) + .unwrap_or_default(); + parent.join(format!(".{prefix}-{}-{nanos}", std::process::id())) +} + +fn cleanup_dir(path: &Path) { + if path.exists() { + let _ = std::fs::remove_dir_all(path); + } +} + +fn commit_staged_extensions(staged: &[StagedExtension], force: bool) -> Result<()> { + commit_staged_extensions_with_hook(staged, force, |_| Ok(())) +} + +fn commit_staged_extensions_with_hook( + staged: &[StagedExtension], + force: bool, + mut before_install: F, +) -> Result<()> +where + F: FnMut(usize) -> Result<()>, +{ + let mut backups = Vec::new(); + let mut installed_targets = Vec::new(); + + let result = (|| -> Result<()> { + for staged_extension in staged { + if staged_extension.target_dir.exists() { + if !force { + bail!( + "extension '{}' is already installed (use --force to overwrite)", + staged_extension + .target_dir + .file_name() + .and_then(|name| name.to_str()) + .unwrap_or("unknown") + .trim_start_matches("pup-") + ); + } + std::fs::rename(&staged_extension.target_dir, &staged_extension.backup_dir) + .with_context(|| { + format!( + "backing up existing extension directory {}", + staged_extension.target_dir.display() + ) + })?; + backups.push(( + staged_extension.target_dir.clone(), + staged_extension.backup_dir.clone(), + )); + } + } + + for (index, staged_extension) in staged.iter().enumerate() { + before_install(index)?; + std::fs::rename(&staged_extension.stage_dir, &staged_extension.target_dir) + .with_context(|| { + format!( + "installing extension directory {}", + staged_extension.target_dir.display() + ) + })?; + installed_targets.push(staged_extension.target_dir.clone()); + } + + Ok(()) + })(); + + if let Err(error) = result { + for target in installed_targets.iter().rev() { + cleanup_dir(target); + } + for (target, backup) in backups.iter().rev() { + if backup.exists() && !target.exists() { + let _ = std::fs::rename(backup, target); + } + } + return Err(error); + } + + for (_, backup) in backups { + cleanup_dir(&backup); + } + + Ok(()) +} + struct ExtensionPayload { name: String, bytes: Vec, @@ -906,6 +1080,12 @@ fn save_github_payloads( for payload in &artifacts.payloads { validate_extension_name(&payload.name)?; } + let mut seen_names = HashSet::new(); + for payload in &artifacts.payloads { + if !seen_names.insert(payload.name.as_str()) { + bail!("extension '{}' was selected more than once", payload.name); + } + } let ext_base = extension_dir().context("could not determine config directory for extensions")?; @@ -920,28 +1100,71 @@ fn save_github_payloads( } } - let mut installed = Vec::new(); - for payload in artifacts.payloads { - let ext_dir = ext_base.join(format!("pup-{}", payload.name)); - prepare_extension_dir(&ext_dir)?; - let exe_name = write_extension_binary(&ext_dir, &payload.name, &payload.bytes)?; - - let manifest = Manifest { - name: payload.name.clone(), - version: artifacts.version.clone(), - source: format!("github:{source}"), - source_kind: artifacts.source_kind.clone(), - source_release_tag: artifacts.source_release_tag.clone(), - source_asset: artifacts.source_asset.clone(), - installed_at: chrono_now_iso(), - binary: exe_name, - description: description.unwrap_or_default().to_string(), - installed_by_pup: version::VERSION.to_string(), - }; - manifest.save(&ext_dir.join("manifest.json"))?; - installed.push(payload.name); + let staging_base = unique_work_dir(&ext_base, "pup-install-staging"); + std::fs::create_dir_all(&staging_base) + .with_context(|| format!("creating staging directory {}", staging_base.display()))?; + let backup_base = unique_work_dir(&ext_base, "pup-install-backup"); + if let Err(error) = std::fs::create_dir_all(&backup_base) + .with_context(|| format!("creating backup directory {}", backup_base.display())) + { + cleanup_dir(&staging_base); + return Err(error); } + let stage_result = (|| -> Result<(Vec, Vec)> { + let mut staged = Vec::new(); + let mut installed = Vec::new(); + let installed_at = chrono_now_iso(); + + for payload in artifacts.payloads { + let ext_dir = ext_base.join(format!("pup-{}", payload.name)); + let stage_dir = staging_base.join(format!("pup-{}", payload.name)); + std::fs::create_dir_all(&stage_dir) + .with_context(|| format!("creating {}", stage_dir.display()))?; + let exe_name = write_extension_binary(&stage_dir, &payload.name, &payload.bytes)?; + + let manifest = Manifest { + name: payload.name.clone(), + version: artifacts.version.clone(), + source: format!("github:{source}"), + source_kind: artifacts.source_kind.clone(), + source_release_tag: artifacts.source_release_tag.clone(), + source_asset: artifacts.source_asset.clone(), + installed_at: installed_at.clone(), + binary: exe_name, + description: description.unwrap_or_default().to_string(), + installed_by_pup: version::VERSION.to_string(), + }; + manifest.save(&stage_dir.join("manifest.json"))?; + + staged.push(StagedExtension { + target_dir: ext_dir, + stage_dir, + backup_dir: backup_base.join(format!("pup-{}", payload.name)), + }); + installed.push(payload.name); + } + + Ok((staged, installed)) + })(); + + let (staged, installed) = match stage_result { + Ok(result) => result, + Err(error) => { + cleanup_dir(&staging_base); + cleanup_dir(&backup_base); + return Err(error); + } + }; + + if let Err(error) = commit_staged_extensions(&staged, force) { + cleanup_dir(&staging_base); + cleanup_dir(&backup_base); + return Err(error); + } + + cleanup_dir(&staging_base); + cleanup_dir(&backup_base); Ok(installed) } @@ -988,30 +1211,7 @@ async fn archive_artifacts_for_request( payloads, }); }; - let releases = - fetch_github_releases_with_limit(client, owner, repo, Some(MAX_IMPLICIT_RELEASE_SCAN)) - .await?; - for release in releases.iter().filter(|release| is_stable_release(release)) { - let Some(archive) = download_archive_from_release(client, release, repo).await? else { - continue; - }; - if archive.extensions.iter().any(|name| name == extension) { - let names = vec![extension.to_string()]; - let payloads = archive_extension_payloads(&archive, &names)?; - return Ok(GitHubInstallArtifacts { - version: archive.version, - source_kind: Some("github_archive".to_string()), - source_release_tag: Some(archive.release_tag), - source_asset: Some(archive.asset_name), - payloads, - }); - } - } - bail!( - "no release archive in {owner}/{repo} contains extension '{extension}' for {}-{}", - archive_platform_os(), - archive_platform_arch() - ); + return latest_archive_artifacts_for_extension(client, owner, repo, extension).await; } let release = fetch_github_release(client, owner, repo, tag, extension).await?; @@ -1035,24 +1235,47 @@ async fn latest_archive_artifacts_for_extension( ) -> Result { validate_extension_name(extension)?; - let releases = - fetch_github_releases_with_limit(client, owner, repo, Some(MAX_IMPLICIT_RELEASE_SCAN)) - .await?; - for release in releases.iter().filter(|release| is_stable_release(release)) { - let Some(archive) = download_archive_from_release(client, release, repo).await? else { - continue; - }; - if archive.extensions.iter().any(|name| name == extension) { - let names = vec![extension.to_string()]; - let payloads = archive_extension_payloads(&archive, &names)?; - return Ok(GitHubInstallArtifacts { - version: archive.version, - source_kind: Some("github_archive".to_string()), - source_release_tag: Some(archive.release_tag), - source_asset: Some(archive.asset_name), - payloads, - }); + let mut scanned = 0; + let mut page = 1; + + loop { + let releases = fetch_github_release_page(client, owner, repo, page).await?; + if releases.is_empty() { + break; + } + + let page_len = releases.len(); + for release in + releases_within_scan_limit(&releases, &mut scanned, MAX_IMPLICIT_RELEASE_SCAN) + .iter() + .filter(|release| is_stable_release(release)) + { + let Some(archive) = download_archive_from_release(client, release, repo).await? else { + continue; + }; + if archive.extensions.iter().any(|name| name == extension) { + let names = vec![extension.to_string()]; + let payloads = archive_extension_payloads(&archive, &names)?; + return Ok(GitHubInstallArtifacts { + version: archive.version, + source_kind: Some("github_archive".to_string()), + source_release_tag: Some(archive.release_tag), + source_asset: Some(archive.asset_name), + payloads, + }); + } + } + + if page_len < 100 { + break; + } + if scanned >= MAX_IMPLICIT_RELEASE_SCAN { + bail!( + "{}", + release_scan_limit_message(owner, repo, MAX_IMPLICIT_RELEASE_SCAN) + ); } + page += 1; } bail!( @@ -1259,7 +1482,8 @@ async fn verify_release_asset_checksum( return Ok(()); }; - let checksums_bytes = download_asset(client, checksums_asset).await?; + let checksums_bytes = + download_asset_with_limit(client, checksums_asset, Some(MAX_CHECKSUMS_BYTES)).await?; let checksums = String::from_utf8(checksums_bytes).context("checksums.txt is not UTF-8")?; verify_checksum_contents(&checksums, &asset.name, bytes) } @@ -1289,7 +1513,13 @@ async fn download_archive_from_release( release: &GitHubRelease, project_name: &str, ) -> Result> { - download_archive_from_release_with_limit(client, release, project_name, None).await + download_archive_from_release_with_limit( + client, + release, + project_name, + Some(MAX_INSTALL_ARCHIVE_BYTES), + ) + .await } async fn download_archive_from_release_with_limit( @@ -1327,21 +1557,35 @@ pub fn list_remote_extensions( let inventories = tokio::task::block_in_place(|| { tokio::runtime::Handle::current().block_on(async { let client = github_client()?; - let releases = fetch_github_releases_with_limit( - &client, - owner, - repo, - Some(MAX_REMOTE_LIST_RELEASES), - ) - .await?; let mut inventories = Vec::new(); - for release in releases.iter().filter(|release| !release.draft) { - if let Some(inventory) = - archive_inventory_from_release(&client, release, repo).await? + let mut scanned = 0; + let mut page = 1; + + loop { + let releases = fetch_github_release_page(&client, owner, repo, page).await?; + if releases.is_empty() { + break; + } + + let page_len = releases.len(); + for release in + releases_within_scan_limit(&releases, &mut scanned, MAX_REMOTE_LIST_RELEASES) + .iter() + .filter(|release| !release.draft) { - inventories.push(inventory); + if let Some(inventory) = + archive_inventory_from_release(&client, release, repo).await? + { + inventories.push(inventory); + } + } + + if page_len < 100 || scanned >= MAX_REMOTE_LIST_RELEASES { + break; } + page += 1; } + Ok::<_, anyhow::Error>(inventories) }) })?; @@ -2054,6 +2298,26 @@ mod tests { assert!(!is_stable_release(&prerelease)); } + #[test] + fn test_releases_within_scan_limit_returns_first_page_before_limit() { + let releases: Vec = (0..101) + .map(|index| GitHubRelease { + tag_name: format!("v{index}"), + draft: false, + prerelease: false, + assets: vec![], + }) + .collect(); + let mut scanned = 0; + + let scanned_releases = releases_within_scan_limit(&releases, &mut scanned, 100); + + assert_eq!(scanned_releases.len(), 100); + assert_eq!(scanned, 100); + assert_eq!(scanned_releases[0].tag_name, "v0"); + assert_eq!(scanned_releases[99].tag_name, "v99"); + } + #[test] fn test_find_platform_asset_uses_asset_name_not_ext_name() { // Verify that find_platform_asset uses the repo-derived name, not a user override. @@ -2257,6 +2521,21 @@ mod tests { assert_eq!(extracted, b"bar"); } + #[test] + fn test_extract_extension_from_archive_tar_gz_rejects_oversized_member() { + let archive = make_tar_gz(&[("pup-foo", b"foo")]); + + let result = extract_extension_from_archive_with_limit( + "bundle_1.2.3_Darwin_arm64.tar.gz", + &archive, + "foo", + 2, + ); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("byte limit")); + } + #[test] fn test_extension_names_from_archive_zip() { let archive = make_zip(&[ @@ -2287,6 +2566,143 @@ mod tests { assert_eq!(extracted, b"bar"); } + #[test] + fn test_extract_extension_from_archive_zip_rejects_oversized_member() { + let archive = make_zip(&[("pup-foo.exe", b"foo")]); + + let result = extract_extension_from_archive_with_limit( + "bundle_1.2.3_Windows_x86_64.zip", + &archive, + "foo", + 2, + ); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("byte limit")); + } + + fn write_test_file(path: &Path, contents: &[u8]) { + std::fs::write(path, contents).unwrap(); + } + + #[test] + fn test_commit_staged_extensions_rolls_back_before_install() { + let dir = std::env::temp_dir().join(format!( + "pup-test-rollback-before-{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + + let target_foo = dir.join("pup-foo"); + let target_bar = dir.join("pup-bar"); + let stage_foo = dir.join("stage-foo"); + let stage_bar = dir.join("stage-bar"); + let backup_foo = dir.join("backup-foo"); + let backup_bar = dir.join("backup-bar"); + for path in [&target_foo, &target_bar, &stage_foo, &stage_bar] { + std::fs::create_dir_all(path).unwrap(); + } + write_test_file(&target_foo.join("pup-foo"), b"old-foo"); + write_test_file(&target_bar.join("pup-bar"), b"old-bar"); + write_test_file(&stage_foo.join("pup-foo"), b"new-foo"); + write_test_file(&stage_bar.join("pup-bar"), b"new-bar"); + + let staged = vec![ + StagedExtension { + target_dir: target_foo.clone(), + stage_dir: stage_foo, + backup_dir: backup_foo, + }, + StagedExtension { + target_dir: target_bar.clone(), + stage_dir: stage_bar, + backup_dir: backup_bar, + }, + ]; + + let result = commit_staged_extensions_with_hook(&staged, true, |index| { + if index == 0 { + anyhow::bail!("injected failure"); + } + Ok(()) + }); + + assert!(result.is_err()); + assert_eq!( + std::fs::read(target_foo.join("pup-foo")).unwrap(), + b"old-foo" + ); + assert_eq!( + std::fs::read(target_bar.join("pup-bar")).unwrap(), + b"old-bar" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn test_commit_staged_extensions_rolls_back_after_partial_install() { + let dir = std::env::temp_dir().join(format!( + "pup-test-rollback-after-{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + + let target_foo = dir.join("pup-foo"); + let target_bar = dir.join("pup-bar"); + let stage_foo = dir.join("stage-foo"); + let stage_bar = dir.join("stage-bar"); + let backup_foo = dir.join("backup-foo"); + let backup_bar = dir.join("backup-bar"); + for path in [&target_foo, &target_bar, &stage_foo, &stage_bar] { + std::fs::create_dir_all(path).unwrap(); + } + write_test_file(&target_foo.join("pup-foo"), b"old-foo"); + write_test_file(&target_bar.join("pup-bar"), b"old-bar"); + write_test_file(&stage_foo.join("pup-foo"), b"new-foo"); + write_test_file(&stage_bar.join("pup-bar"), b"new-bar"); + + let staged = vec![ + StagedExtension { + target_dir: target_foo.clone(), + stage_dir: stage_foo, + backup_dir: backup_foo, + }, + StagedExtension { + target_dir: target_bar.clone(), + stage_dir: stage_bar, + backup_dir: backup_bar, + }, + ]; + + let result = commit_staged_extensions_with_hook(&staged, true, |index| { + if index == 1 { + anyhow::bail!("injected failure"); + } + Ok(()) + }); + + assert!(result.is_err()); + assert_eq!( + std::fs::read(target_foo.join("pup-foo")).unwrap(), + b"old-foo" + ); + assert_eq!( + std::fs::read(target_bar.join("pup-bar")).unwrap(), + b"old-bar" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn test_selected_archive_extension_names_accepts_exact_extension() { let available = vec!["bar".to_string(), "foo".to_string()]; From 1df678d4545a58530d83d67867f5ea5496b32e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Wed, 17 Jun 2026 15:27:57 +0200 Subject: [PATCH 05/12] fix(extension): align remote listing with stable installs --- src/extensions/install.rs | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/extensions/install.rs b/src/extensions/install.rs index 26e4f151..c511d45b 100644 --- a/src/extensions/install.rs +++ b/src/extensions/install.rs @@ -1571,7 +1571,7 @@ pub fn list_remote_extensions( for release in releases_within_scan_limit(&releases, &mut scanned, MAX_REMOTE_LIST_RELEASES) .iter() - .filter(|release| !release.draft) + .filter(|release| is_stable_release(release)) { if let Some(inventory) = archive_inventory_from_release(&client, release, repo).await? @@ -2298,6 +2298,38 @@ mod tests { assert!(!is_stable_release(&prerelease)); } + #[test] + fn test_remote_listing_release_filter_matches_implicit_install() { + let releases = [ + GitHubRelease { + tag_name: "v1.0.0".to_string(), + draft: false, + prerelease: false, + assets: vec![], + }, + GitHubRelease { + tag_name: "v1.1.0-rc.1".to_string(), + draft: false, + prerelease: true, + assets: vec![], + }, + GitHubRelease { + tag_name: "v1.2.0".to_string(), + draft: true, + prerelease: false, + assets: vec![], + }, + ]; + + let listed_tags = releases + .iter() + .filter(|release| is_stable_release(release)) + .map(|release| release.tag_name.as_str()) + .collect::>(); + + assert_eq!(listed_tags, vec!["v1.0.0"]); + } + #[test] fn test_releases_within_scan_limit_returns_first_page_before_limit() { let releases: Vec = (0..101) From 9f9f5e1aaad09768f94b89227ee6e97cf5fb820f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Wed, 17 Jun 2026 17:37:33 +0200 Subject: [PATCH 06/12] fix(extension): bound remote extension archive scans --- src/extensions/install.rs | 346 ++++++++++++++++++++++++++++++++++---- 1 file changed, 310 insertions(+), 36 deletions(-) diff --git a/src/extensions/install.rs b/src/extensions/install.rs index c511d45b..2c2686af 100644 --- a/src/extensions/install.rs +++ b/src/extensions/install.rs @@ -14,8 +14,12 @@ use crate::version; const MAX_REMOTE_LIST_RELEASES: usize = 100; const MAX_REMOTE_LIST_ARCHIVE_BYTES: usize = 100 * 1024 * 1024; const MAX_IMPLICIT_RELEASE_SCAN: usize = 100; +const MAX_RAW_RELEASE_SCAN: usize = 1000; +const MAX_ARCHIVE_SCAN_TOTAL_BYTES: usize = 512 * 1024 * 1024; const MAX_INSTALL_ARCHIVE_BYTES: usize = 100 * 1024 * 1024; const MAX_EXTENSION_BINARY_BYTES: usize = 100 * 1024 * 1024; +const MAX_SELECTED_ARCHIVE_EXTENSIONS: usize = 100; +const MAX_TOTAL_EXTENSION_PAYLOAD_BYTES: usize = 256 * 1024 * 1024; const MAX_ARCHIVE_ENTRIES: usize = 4096; const MAX_CHECKSUMS_BYTES: usize = 1024 * 1024; @@ -25,6 +29,8 @@ struct GitHubAsset { name: String, #[serde(default)] url: Option, + #[serde(default)] + size: Option, browser_download_url: String, } @@ -48,6 +54,35 @@ struct ArchiveDownload { extensions: Vec, } +struct ArchiveScanBudget { + remaining_bytes: usize, +} + +impl ArchiveScanBudget { + fn new(max_bytes: usize) -> Self { + Self { + remaining_bytes: max_bytes, + } + } + + fn remaining(&self) -> usize { + self.remaining_bytes + } + + fn consume(&mut self, asset: &GitHubAsset, bytes: usize) -> Result<()> { + if bytes > self.remaining_bytes { + bail!( + "archive scan budget exceeded while downloading '{}': {} bytes remaining, {} bytes needed", + asset.name, + self.remaining_bytes, + bytes + ); + } + self.remaining_bytes -= bytes; + Ok(()) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] enum GitHubAuthSource { Env(&'static str), @@ -400,15 +435,19 @@ async fn fetch_github_release_page( .with_context(|| format!("parsing release JSON from {url}")) } -fn releases_within_scan_limit<'a>( +fn stable_releases_within_scan_limit<'a>( releases: &'a [GitHubRelease], scanned: &mut usize, max_releases: usize, -) -> &'a [GitHubRelease] { +) -> Vec<&'a GitHubRelease> { let remaining = max_releases.saturating_sub(*scanned); - let count = remaining.min(releases.len()); - *scanned += count; - &releases[..count] + let selected = releases + .iter() + .filter(|release| is_stable_release(release)) + .take(remaining) + .collect::>(); + *scanned += selected.len(); + selected } fn release_scan_limit_message(owner: &str, repo: &str, max_releases: usize) -> String { @@ -418,6 +457,25 @@ fn release_scan_limit_message(owner: &str, repo: &str, max_releases: usize) -> S ) } +fn raw_release_scan_limit_message(owner: &str, repo: &str, max_releases: usize) -> String { + format!( + "searched the first {max_releases} releases in {owner}/{repo} without finding enough \ + stable release archives. Use --tag to install an exact release." + ) +} + +fn remote_list_raw_release_scan_limit_message( + owner: &str, + repo: &str, + max_releases: usize, +) -> String { + format!( + "searched the first {max_releases} releases in {owner}/{repo} without completing remote \ + extension discovery. If you know the extension and release tag, install that exact release \ + with --tag." + ) +} + fn is_stable_release(release: &GitHubRelease) -> bool { !release.draft && !release.prerelease } @@ -774,6 +832,13 @@ async fn download_asset_with_limit( } if let Some(max_bytes) = max_bytes { + if asset.size.is_some_and(|size| size > max_bytes as u64) { + bail!( + "asset '{}' is larger than the {} byte scan limit", + asset.name, + max_bytes + ); + } if resp .content_length() .is_some_and(|content_length| content_length > max_bytes as u64) @@ -1052,16 +1117,44 @@ fn archive_extension_payloads( archive: &ArchiveDownload, names: &[String], ) -> Result> { - names - .iter() - .map(|name| { - let bytes = extract_extension_from_archive(&archive.asset_name, &archive.bytes, name)?; - Ok(ExtensionPayload { - name: name.clone(), - bytes, - }) - }) - .collect() + archive_extension_payloads_with_limits( + archive, + names, + MAX_SELECTED_ARCHIVE_EXTENSIONS, + MAX_TOTAL_EXTENSION_PAYLOAD_BYTES, + ) +} + +fn archive_extension_payloads_with_limits( + archive: &ArchiveDownload, + names: &[String], + max_selected: usize, + max_total_bytes: usize, +) -> Result> { + if names.len() > max_selected { + bail!("selected archive contains more than {max_selected} extensions"); + } + + let mut payloads = Vec::with_capacity(names.len()); + let mut total_bytes = 0usize; + for name in names { + let bytes = extract_extension_from_archive(&archive.asset_name, &archive.bytes, name)?; + total_bytes = total_bytes + .checked_add(bytes.len()) + .context("selected extension payload size overflowed")?; + if total_bytes > max_total_bytes { + bail!( + "selected archive extensions are larger than the {} byte aggregate limit", + max_total_bytes + ); + } + payloads.push(ExtensionPayload { + name: name.clone(), + bytes, + }); + } + + Ok(payloads) } fn save_github_payloads( @@ -1235,8 +1328,10 @@ async fn latest_archive_artifacts_for_extension( ) -> Result { validate_extension_name(extension)?; - let mut scanned = 0; + let mut stable_scanned = 0; + let mut raw_scanned = 0; let mut page = 1; + let mut scan_budget = ArchiveScanBudget::new(MAX_ARCHIVE_SCAN_TOTAL_BYTES); loop { let releases = fetch_github_release_page(client, owner, repo, page).await?; @@ -1245,12 +1340,21 @@ async fn latest_archive_artifacts_for_extension( } let page_len = releases.len(); - for release in - releases_within_scan_limit(&releases, &mut scanned, MAX_IMPLICIT_RELEASE_SCAN) - .iter() - .filter(|release| is_stable_release(release)) - { - let Some(archive) = download_archive_from_release(client, release, repo).await? else { + raw_scanned += page_len; + for release in stable_releases_within_scan_limit( + &releases, + &mut stable_scanned, + MAX_IMPLICIT_RELEASE_SCAN, + ) { + let Some(archive) = download_archive_from_release_for_scan( + client, + release, + repo, + MAX_INSTALL_ARCHIVE_BYTES, + &mut scan_budget, + ) + .await? + else { continue; }; if archive.extensions.iter().any(|name| name == extension) { @@ -1269,12 +1373,18 @@ async fn latest_archive_artifacts_for_extension( if page_len < 100 { break; } - if scanned >= MAX_IMPLICIT_RELEASE_SCAN { + if stable_scanned >= MAX_IMPLICIT_RELEASE_SCAN { bail!( "{}", release_scan_limit_message(owner, repo, MAX_IMPLICIT_RELEASE_SCAN) ); } + if raw_scanned >= MAX_RAW_RELEASE_SCAN { + bail!( + "{}", + raw_release_scan_limit_message(owner, repo, MAX_RAW_RELEASE_SCAN) + ); + } page += 1; } @@ -1340,6 +1450,7 @@ pub fn install_from_github( match find_platform_asset(&release, &asset_name) { Ok(asset) => { let bytes = download_asset(&client, asset).await?; + verify_release_asset_checksum(&client, &release, asset, &bytes).await?; Ok::<_, anyhow::Error>(GitHubInstallArtifacts { version: extract_version(&release.tag_name), source_kind: None, @@ -1492,12 +1603,14 @@ async fn archive_inventory_from_release( client: &reqwest::Client, release: &GitHubRelease, project_name: &str, + scan_budget: &mut ArchiveScanBudget, ) -> Result> { - Ok(download_archive_from_release_with_limit( + Ok(download_archive_from_release_for_scan( client, release, project_name, - Some(MAX_REMOTE_LIST_ARCHIVE_BYTES), + MAX_REMOTE_LIST_ARCHIVE_BYTES, + scan_budget, ) .await? .map(|archive| ArchiveInventory { @@ -1522,6 +1635,39 @@ async fn download_archive_from_release( .await } +async fn download_archive_from_release_for_scan( + client: &reqwest::Client, + release: &GitHubRelease, + project_name: &str, + max_asset_bytes: usize, + scan_budget: &mut ArchiveScanBudget, +) -> Result> { + let asset = match find_platform_archive_asset(release, project_name) { + Ok(asset) => asset, + Err(_) => return Ok(None), + }; + + let max_bytes = max_asset_bytes.min(scan_budget.remaining()); + if max_bytes == 0 { + bail!( + "archive scan budget exhausted before downloading '{}'", + asset.name + ); + } + let bytes = download_asset_with_limit(client, asset, Some(max_bytes)).await?; + scan_budget.consume(asset, bytes.len())?; + verify_release_asset_checksum(client, release, asset, &bytes).await?; + let extensions = extension_names_from_archive(&asset.name, &bytes)?; + + Ok(Some(ArchiveDownload { + release_tag: release.tag_name.clone(), + version: extract_version(&release.tag_name), + asset_name: asset.name.clone(), + bytes, + extensions, + })) +} + async fn download_archive_from_release_with_limit( client: &reqwest::Client, release: &GitHubRelease, @@ -1558,8 +1704,10 @@ pub fn list_remote_extensions( tokio::runtime::Handle::current().block_on(async { let client = github_client()?; let mut inventories = Vec::new(); - let mut scanned = 0; + let mut stable_scanned = 0; + let mut raw_scanned = 0; let mut page = 1; + let mut scan_budget = ArchiveScanBudget::new(MAX_ARCHIVE_SCAN_TOTAL_BYTES); loop { let releases = fetch_github_release_page(&client, owner, repo, page).await?; @@ -1568,21 +1716,33 @@ pub fn list_remote_extensions( } let page_len = releases.len(); - for release in - releases_within_scan_limit(&releases, &mut scanned, MAX_REMOTE_LIST_RELEASES) - .iter() - .filter(|release| is_stable_release(release)) - { + raw_scanned += page_len; + for release in stable_releases_within_scan_limit( + &releases, + &mut stable_scanned, + MAX_REMOTE_LIST_RELEASES, + ) { if let Some(inventory) = - archive_inventory_from_release(&client, release, repo).await? + archive_inventory_from_release(&client, release, repo, &mut scan_budget) + .await? { inventories.push(inventory); } } - if page_len < 100 || scanned >= MAX_REMOTE_LIST_RELEASES { + if page_len < 100 || stable_scanned >= MAX_REMOTE_LIST_RELEASES { break; } + if raw_scanned >= MAX_RAW_RELEASE_SCAN { + bail!( + "{}", + remote_list_raw_release_scan_limit_message( + owner, + repo, + MAX_RAW_RELEASE_SCAN + ) + ); + } page += 1; } @@ -1692,7 +1852,11 @@ pub fn upgrade_extension(name: &str) -> Result { let asset = find_platform_asset(&release, &asset_name)?; let asset_bytes = tokio::task::block_in_place(|| { - tokio::runtime::Handle::current().block_on(async { download_asset(&client, asset).await }) + tokio::runtime::Handle::current().block_on(async { + let asset_bytes = download_asset(&client, asset).await?; + verify_release_asset_checksum(&client, &release, asset, &asset_bytes).await?; + Ok::<_, anyhow::Error>(asset_bytes) + }) })?; // Prepare (recreate) the extension directory before writing, so a failed write @@ -2138,21 +2302,25 @@ mod tests { GitHubAsset { name: "pup-hello-linux-x86_64".to_string(), url: None, + size: None, browser_download_url: "https://example.com/linux-x86_64".to_string(), }, GitHubAsset { name: "pup-hello-darwin-aarch64".to_string(), url: None, + size: None, browser_download_url: "https://example.com/darwin-aarch64".to_string(), }, GitHubAsset { name: "pup-hello-darwin-x86_64".to_string(), url: None, + size: None, browser_download_url: "https://example.com/darwin-x86_64".to_string(), }, GitHubAsset { name: "pup-hello-windows-x86_64".to_string(), url: None, + size: None, browser_download_url: "https://example.com/windows-x86_64".to_string(), }, ], @@ -2170,6 +2338,7 @@ mod tests { assets: vec![GitHubAsset { name: "pup-hello-fakeos-fakearch".to_string(), url: None, + size: None, browser_download_url: "https://example.com/fake".to_string(), }], }; @@ -2331,7 +2500,7 @@ mod tests { } #[test] - fn test_releases_within_scan_limit_returns_first_page_before_limit() { + fn test_stable_releases_within_scan_limit_returns_first_stable_releases_before_limit() { let releases: Vec = (0..101) .map(|index| GitHubRelease { tag_name: format!("v{index}"), @@ -2342,7 +2511,7 @@ mod tests { .collect(); let mut scanned = 0; - let scanned_releases = releases_within_scan_limit(&releases, &mut scanned, 100); + let scanned_releases = stable_releases_within_scan_limit(&releases, &mut scanned, 100); assert_eq!(scanned_releases.len(), 100); assert_eq!(scanned, 100); @@ -2350,6 +2519,31 @@ mod tests { assert_eq!(scanned_releases[99].tag_name, "v99"); } + #[test] + fn test_stable_releases_within_scan_limit_skips_prereleases_before_counting() { + let mut releases: Vec = (0..100) + .map(|index| GitHubRelease { + tag_name: format!("v1.0.{index}-rc.1"), + draft: false, + prerelease: true, + assets: vec![], + }) + .collect(); + releases.push(GitHubRelease { + tag_name: "v1.0.0".to_string(), + draft: false, + prerelease: false, + assets: vec![], + }); + let mut scanned = 0; + + let scanned_releases = stable_releases_within_scan_limit(&releases, &mut scanned, 1); + + assert_eq!(scanned_releases.len(), 1); + assert_eq!(scanned, 1); + assert_eq!(scanned_releases[0].tag_name, "v1.0.0"); + } + #[test] fn test_find_platform_asset_uses_asset_name_not_ext_name() { // Verify that find_platform_asset uses the repo-derived name, not a user override. @@ -2363,6 +2557,7 @@ mod tests { assets: vec![GitHubAsset { name: format!("pup-hello-{os}-{arch}"), url: None, + size: None, browser_download_url: "https://example.com/hello".to_string(), }], }; @@ -2474,6 +2669,40 @@ mod tests { cursor.into_inner() } + fn test_archive_download( + asset_name: &str, + bytes: Vec, + extensions: Vec, + ) -> ArchiveDownload { + ArchiveDownload { + release_tag: "v1.2.3".to_string(), + version: "1.2.3".to_string(), + asset_name: asset_name.to_string(), + bytes, + extensions, + } + } + + #[test] + fn test_archive_scan_budget_rejects_over_budget_consume() { + let asset = GitHubAsset { + name: "bundle.tar.gz".to_string(), + url: None, + size: None, + browser_download_url: "https://example.com/bundle".to_string(), + }; + let mut budget = ArchiveScanBudget::new(3); + + budget.consume(&asset, 2).unwrap(); + let result = budget.consume(&asset, 2); + + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("archive scan budget exceeded")); + } + #[test] fn test_find_platform_archive_asset_found() { let version = "1.2.3"; @@ -2485,26 +2714,31 @@ mod tests { GitHubAsset { name: format!("bundle_{version}_Darwin_arm64.tar.gz"), url: None, + size: None, browser_download_url: "https://example.com/darwin-arm64".to_string(), }, GitHubAsset { name: format!("bundle_{version}_Darwin_x86_64.tar.gz"), url: None, + size: None, browser_download_url: "https://example.com/darwin-x86_64".to_string(), }, GitHubAsset { name: format!("bundle_{version}_Linux_arm64.tar.gz"), url: None, + size: None, browser_download_url: "https://example.com/linux-arm64".to_string(), }, GitHubAsset { name: format!("bundle_{version}_Linux_x86_64.tar.gz"), url: None, + size: None, browser_download_url: "https://example.com/linux-x86_64".to_string(), }, GitHubAsset { name: format!("bundle_{version}_Windows_x86_64.zip"), url: None, + size: None, browser_download_url: "https://example.com/windows-x86_64".to_string(), }, ], @@ -2568,6 +2802,46 @@ mod tests { assert!(result.unwrap_err().to_string().contains("byte limit")); } + #[test] + fn test_archive_extension_payloads_rejects_too_many_selected_extensions() { + let archive = test_archive_download( + "bundle_1.2.3_Darwin_arm64.tar.gz", + make_tar_gz(&[("pup-foo", b"foo")]), + vec!["foo".to_string()], + ); + let names = (0..3) + .map(|index| format!("foo{index}")) + .collect::>(); + + let result = archive_extension_payloads_with_limits(&archive, &names, 2, 1024); + + assert!(result.is_err()); + assert!(result + .err() + .unwrap() + .to_string() + .contains("more than 2 extensions")); + } + + #[test] + fn test_archive_extension_payloads_rejects_aggregate_payload_limit() { + let archive = test_archive_download( + "bundle_1.2.3_Darwin_arm64.tar.gz", + make_tar_gz(&[("pup-foo", b"foo"), ("pup-bar", b"bar")]), + vec!["foo".to_string(), "bar".to_string()], + ); + let names = vec!["foo".to_string(), "bar".to_string()]; + + let result = archive_extension_payloads_with_limits(&archive, &names, 10, 5); + + assert!(result.is_err()); + assert!(result + .err() + .unwrap() + .to_string() + .contains("aggregate limit")); + } + #[test] fn test_extension_names_from_archive_zip() { let archive = make_zip(&[ From 65de05a2042ff7d455679a0ca1b46021812d99ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Wed, 17 Jun 2026 18:08:06 +0200 Subject: [PATCH 07/12] fix(extension): cap decoded tar archive scans --- src/extensions/install.rs | 140 +++++++++++++++++++++++++++++++++++--- 1 file changed, 129 insertions(+), 11 deletions(-) diff --git a/src/extensions/install.rs b/src/extensions/install.rs index 2c2686af..d504e59f 100644 --- a/src/extensions/install.rs +++ b/src/extensions/install.rs @@ -18,6 +18,7 @@ const MAX_RAW_RELEASE_SCAN: usize = 1000; const MAX_ARCHIVE_SCAN_TOTAL_BYTES: usize = 512 * 1024 * 1024; const MAX_INSTALL_ARCHIVE_BYTES: usize = 100 * 1024 * 1024; const MAX_EXTENSION_BINARY_BYTES: usize = 100 * 1024 * 1024; +const MAX_ARCHIVE_DECODED_BYTES: usize = 256 * 1024 * 1024; const MAX_SELECTED_ARCHIVE_EXTENSIONS: usize = 100; const MAX_TOTAL_EXTENSION_PAYLOAD_BYTES: usize = 256 * 1024 * 1024; const MAX_ARCHIVE_ENTRIES: usize = 4096; @@ -571,9 +572,22 @@ fn extension_names_from_archive(asset_name: &str, bytes: &[u8]) -> Result Result> { + extension_names_from_tar_gz_with_limits( + bytes, + MAX_EXTENSION_BINARY_BYTES, + MAX_ARCHIVE_DECODED_BYTES, + ) +} + +fn extension_names_from_tar_gz_with_limits( + bytes: &[u8], + max_member_bytes: usize, + max_decoded_bytes: usize, +) -> Result> { let decoder = flate2::read::GzDecoder::new(Cursor::new(bytes)); let mut archive = tar::Archive::new(decoder); let mut names = Vec::new(); + let mut decoded_bytes = 0; for (index, entry) in archive .entries() @@ -587,8 +601,22 @@ fn extension_names_from_tar_gz(bytes: &[u8]) -> Result> { if !entry.header().entry_type().is_file() { continue; } - let path = entry.path().context("reading tar.gz archive path")?; - if let Some(name) = extension_name_from_archive_path(path.as_ref()) { + let path = entry + .path() + .context("reading tar.gz archive path")? + .into_owned(); + let size = entry + .header() + .size() + .context("reading tar.gz archive entry size")?; + account_tar_file_size( + &mut decoded_bytes, + &path, + size, + max_member_bytes, + max_decoded_bytes, + )?; + if let Some(name) = extension_name_from_archive_path(&path) { names.push(name); } } @@ -653,6 +681,32 @@ fn ensure_archive_member_size(size: u64, name: &str, max_member_bytes: usize) -> Ok(()) } +fn account_tar_file_size( + decoded_bytes: &mut u64, + path: &Path, + size: u64, + max_member_bytes: usize, + max_decoded_bytes: usize, +) -> Result<()> { + if size > max_member_bytes as u64 { + bail!( + "archive member '{}' is larger than the {} byte limit", + path.display(), + max_member_bytes + ); + } + *decoded_bytes = decoded_bytes + .checked_add(size) + .context("tar.gz archive decoded size overflowed")?; + if *decoded_bytes > max_decoded_bytes as u64 { + bail!( + "tar.gz archive file contents are larger than the {} byte decoded limit", + max_decoded_bytes + ); + } + Ok(()) +} + fn read_limited_archive_member( reader: R, name: &str, @@ -677,9 +731,24 @@ fn extract_extension_from_tar_gz( bytes: &[u8], name: &str, max_member_bytes: usize, +) -> Result> { + extract_extension_from_tar_gz_with_limits( + bytes, + name, + max_member_bytes, + MAX_ARCHIVE_DECODED_BYTES, + ) +} + +fn extract_extension_from_tar_gz_with_limits( + bytes: &[u8], + name: &str, + max_member_bytes: usize, + max_decoded_bytes: usize, ) -> Result> { let decoder = flate2::read::GzDecoder::new(Cursor::new(bytes)); let mut archive = tar::Archive::new(decoder); + let mut decoded_bytes = 0; for (index, entry) in archive .entries() @@ -693,16 +762,23 @@ fn extract_extension_from_tar_gz( if !entry.header().entry_type().is_file() { continue; } - let matches = { - let path = entry.path().context("reading tar.gz archive path")?; - extension_archive_member_matches(path.as_ref(), name) - }; + let path = entry + .path() + .context("reading tar.gz archive path")? + .into_owned(); + let size = entry + .header() + .size() + .context("reading tar.gz archive entry size")?; + account_tar_file_size( + &mut decoded_bytes, + &path, + size, + max_member_bytes, + max_decoded_bytes, + )?; + let matches = extension_archive_member_matches(&path, name); if matches { - let size = entry - .header() - .size() - .context("reading tar.gz archive entry size")?; - ensure_archive_member_size(size, name, max_member_bytes)?; return read_limited_archive_member( &mut entry, name, @@ -2653,6 +2729,18 @@ mod tests { gz.finish().unwrap() } + fn make_tar_gz_with_declared_file_size(path: &str, size: u64) -> Vec { + let mut header = tar::Header::new_gnu(); + header.set_path(path).unwrap(); + header.set_size(size); + header.set_mode(0o755); + header.set_cksum(); + + let mut gz = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); + std::io::Write::write_all(&mut gz, header.as_bytes()).unwrap(); + gz.finish().unwrap() + } + fn make_zip(entries: &[(&str, &[u8])]) -> Vec { let mut cursor = Cursor::new(Vec::new()); { @@ -2772,6 +2860,16 @@ mod tests { assert_eq!(names, vec!["bar".to_string(), "foo".to_string()]); } + #[test] + fn test_extension_names_from_archive_tar_gz_rejects_oversized_nonmatching_member() { + let archive = make_tar_gz_with_declared_file_size("not-pup", 3); + + let result = extension_names_from_tar_gz_with_limits(&archive, 2, 10); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("byte limit")); + } + #[test] fn test_extract_extension_from_archive_tar_gz() { let archive = make_tar_gz(&[ @@ -2787,6 +2885,26 @@ mod tests { assert_eq!(extracted, b"bar"); } + #[test] + fn test_extract_extension_from_archive_tar_gz_rejects_oversized_nonmatching_member() { + let archive = make_tar_gz_with_declared_file_size("not-pup", 3); + + let result = extract_extension_from_tar_gz_with_limits(&archive, "foo", 2, 10); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("byte limit")); + } + + #[test] + fn test_extract_extension_from_archive_tar_gz_rejects_decoded_limit() { + let archive = make_tar_gz(&[("not-pup", b"abc"), ("pup-foo", b"foo")]); + + let result = extract_extension_from_tar_gz_with_limits(&archive, "foo", 10, 5); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("decoded limit")); + } + #[test] fn test_extract_extension_from_archive_tar_gz_rejects_oversized_member() { let archive = make_tar_gz(&[("pup-foo", b"foo")]); From d4efeb5c538d1fddc5844700087f5cbaabdef958 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Wed, 17 Jun 2026 18:26:35 +0200 Subject: [PATCH 08/12] fix(extension): cap decoded tar reader --- src/extensions/install.rs | 121 ++++++++++++++++++++++++++++++++++---- 1 file changed, 109 insertions(+), 12 deletions(-) diff --git a/src/extensions/install.rs b/src/extensions/install.rs index d504e59f..d58d586d 100644 --- a/src/extensions/install.rs +++ b/src/extensions/install.rs @@ -1,7 +1,7 @@ use anyhow::{bail, Context, Result}; use sha2::{Digest, Sha256}; use std::collections::HashSet; -use std::io::{Cursor, Read}; +use std::io::{self, Cursor, Read}; use std::path::{Path, PathBuf}; use std::process::Command; use std::sync::OnceLock; @@ -84,6 +84,49 @@ impl ArchiveScanBudget { } } +struct DecodedByteLimitReader { + inner: R, + remaining_bytes: usize, + max_bytes: usize, +} + +impl DecodedByteLimitReader { + fn new(inner: R, max_bytes: usize) -> Self { + Self { + inner, + remaining_bytes: max_bytes, + max_bytes, + } + } +} + +impl Read for DecodedByteLimitReader { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + if buf.is_empty() { + return Ok(0); + } + if self.remaining_bytes == 0 { + let mut probe = [0_u8; 1]; + return match self.inner.read(&mut probe) { + Ok(0) => Ok(0), + Ok(_) => Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "tar.gz archive decoded stream is larger than the {} byte decoded limit", + self.max_bytes + ), + )), + Err(err) => Err(err), + }; + } + + let max_read = buf.len().min(self.remaining_bytes); + let read = self.inner.read(&mut buf[..max_read])?; + self.remaining_bytes -= read; + Ok(read) + } +} + #[derive(Debug, Clone, PartialEq, Eq)] enum GitHubAuthSource { Env(&'static str), @@ -585,6 +628,7 @@ fn extension_names_from_tar_gz_with_limits( max_decoded_bytes: usize, ) -> Result> { let decoder = flate2::read::GzDecoder::new(Cursor::new(bytes)); + let decoder = DecodedByteLimitReader::new(decoder, max_decoded_bytes); let mut archive = tar::Archive::new(decoder); let mut names = Vec::new(); let mut decoded_bytes = 0; @@ -605,10 +649,7 @@ fn extension_names_from_tar_gz_with_limits( .path() .context("reading tar.gz archive path")? .into_owned(); - let size = entry - .header() - .size() - .context("reading tar.gz archive entry size")?; + let size = entry.size(); account_tar_file_size( &mut decoded_bytes, &path, @@ -747,6 +788,7 @@ fn extract_extension_from_tar_gz_with_limits( max_decoded_bytes: usize, ) -> Result> { let decoder = flate2::read::GzDecoder::new(Cursor::new(bytes)); + let decoder = DecodedByteLimitReader::new(decoder, max_decoded_bytes); let mut archive = tar::Archive::new(decoder); let mut decoded_bytes = 0; @@ -766,10 +808,7 @@ fn extract_extension_from_tar_gz_with_limits( .path() .context("reading tar.gz archive path")? .into_owned(); - let size = entry - .header() - .size() - .context("reading tar.gz archive entry size")?; + let size = entry.size(); account_tar_file_size( &mut decoded_bytes, &path, @@ -2741,6 +2780,43 @@ mod tests { gz.finish().unwrap() } + fn append_raw_tar_entry(tar: &mut Vec, header: &mut tar::Header, data: &[u8]) { + header.set_cksum(); + std::io::Write::write_all(tar, header.as_bytes()).unwrap(); + std::io::Write::write_all(tar, data).unwrap(); + let padding = (512 - data.len() % 512) % 512; + if padding > 0 { + std::io::Write::write_all(tar, &vec![0; padding]).unwrap(); + } + } + + fn gzip_tar_bytes(tar: &[u8]) -> Vec { + let mut gz = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); + std::io::Write::write_all(&mut gz, tar).unwrap(); + gz.finish().unwrap() + } + + fn make_tar_gz_with_pax_size_override() -> Vec { + let pax = b"9 size=3\n"; + let mut tar = Vec::new(); + + let mut pax_header = tar::Header::new_ustar(); + pax_header.set_path("PaxHeaders/pup-foo").unwrap(); + pax_header.set_entry_type(tar::EntryType::XHeader); + pax_header.set_size(pax.len() as u64); + pax_header.set_mode(0o644); + append_raw_tar_entry(&mut tar, &mut pax_header, pax); + + let mut file_header = tar::Header::new_gnu(); + file_header.set_path("pup-foo").unwrap(); + file_header.set_size(1); + file_header.set_mode(0o755); + append_raw_tar_entry(&mut tar, &mut file_header, b"abc"); + + tar.extend_from_slice(&[0; 1024]); + gzip_tar_bytes(&tar) + } + fn make_zip(entries: &[(&str, &[u8])]) -> Vec { let mut cursor = Cursor::new(Vec::new()); { @@ -2864,12 +2940,33 @@ mod tests { fn test_extension_names_from_archive_tar_gz_rejects_oversized_nonmatching_member() { let archive = make_tar_gz_with_declared_file_size("not-pup", 3); - let result = extension_names_from_tar_gz_with_limits(&archive, 2, 10); + let result = extension_names_from_tar_gz_with_limits(&archive, 2, 4096); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("byte limit")); } + #[test] + fn test_extension_names_from_archive_tar_gz_uses_pax_size_override() { + let archive = make_tar_gz_with_pax_size_override(); + + let result = extension_names_from_tar_gz_with_limits(&archive, 2, 4096); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("byte limit")); + } + + #[test] + fn test_extension_names_from_archive_tar_gz_caps_decoded_longname_payloads() { + let long_path = format!("not-pup-{}", "a".repeat(160)); + let archive = make_tar_gz(&[(&long_path, b"x")]); + + let result = extension_names_from_tar_gz_with_limits(&archive, 1024, 1024); + + assert!(result.is_err()); + assert!(format!("{:?}", result.unwrap_err()).contains("decoded limit")); + } + #[test] fn test_extract_extension_from_archive_tar_gz() { let archive = make_tar_gz(&[ @@ -2889,7 +2986,7 @@ mod tests { fn test_extract_extension_from_archive_tar_gz_rejects_oversized_nonmatching_member() { let archive = make_tar_gz_with_declared_file_size("not-pup", 3); - let result = extract_extension_from_tar_gz_with_limits(&archive, "foo", 2, 10); + let result = extract_extension_from_tar_gz_with_limits(&archive, "foo", 2, 4096); assert!(result.is_err()); assert!(result.unwrap_err().to_string().contains("byte limit")); @@ -2902,7 +2999,7 @@ mod tests { let result = extract_extension_from_tar_gz_with_limits(&archive, "foo", 10, 5); assert!(result.is_err()); - assert!(result.unwrap_err().to_string().contains("decoded limit")); + assert!(format!("{:?}", result.unwrap_err()).contains("decoded limit")); } #[test] From 366b354a15c9b1c0c13d9ca4e82a5ad9de85411f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Wed, 17 Jun 2026 18:46:51 +0200 Subject: [PATCH 09/12] fix(extension): harden GitHub auth and rollback --- src/extensions/install.rs | 279 +++++++++++++++++++++++++++++--------- 1 file changed, 217 insertions(+), 62 deletions(-) diff --git a/src/extensions/install.rs b/src/extensions/install.rs index d58d586d..3acac8d3 100644 --- a/src/extensions/install.rs +++ b/src/extensions/install.rs @@ -3,9 +3,9 @@ use sha2::{Digest, Sha256}; use std::collections::HashSet; use std::io::{self, Cursor, Read}; use std::path::{Path, PathBuf}; -use std::process::Command; +use std::process::{Command, Output, Stdio}; use std::sync::OnceLock; -use std::time::{SystemTime, UNIX_EPOCH}; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use super::discovery::extension_dir; use super::manifest::Manifest; @@ -23,6 +23,7 @@ const MAX_SELECTED_ARCHIVE_EXTENSIONS: usize = 100; const MAX_TOTAL_EXTENSION_PAYLOAD_BYTES: usize = 256 * 1024 * 1024; const MAX_ARCHIVE_ENTRIES: usize = 4096; const MAX_CHECKSUMS_BYTES: usize = 1024 * 1024; +const GH_AUTH_COMMAND_TIMEOUT: Duration = Duration::from_secs(5); /// GitHub release asset metadata (subset of the GitHub Releases API response). #[derive(Debug, serde::Deserialize)] @@ -141,6 +142,7 @@ struct GitHubAuthResolution { } static GITHUB_AUTH: OnceLock = OnceLock::new(); +static GITHUB_GH_AUTH: OnceLock = OnceLock::new(); /// Map `std::env::consts::OS` to the asset name convention. fn platform_os() -> &'static str { @@ -188,13 +190,9 @@ fn github_client() -> Result { .context("building HTTP client for GitHub API") } -fn resolve_github_auth_with( - env_lookup: EnvLookup, - gh_token: GhToken, -) -> GitHubAuthResolution +fn resolve_github_env_auth_with(env_lookup: EnvLookup) -> GitHubAuthResolution where EnvLookup: Fn(&str) -> Option, - GhToken: Fn() -> Result, { for name in ["GH_TOKEN", "GITHUB_TOKEN", "HOMEBREW_GITHUB_API_TOKEN"] { if let Some(token) = env_lookup(name) @@ -209,6 +207,17 @@ where } } + GitHubAuthResolution { + token: None, + source: None, + gh_error: None, + } +} + +fn resolve_github_gh_auth_with(gh_token: GhToken) -> GitHubAuthResolution +where + GhToken: Fn() -> Result, +{ match gh_token() { Ok(token) => { let token = token.trim().to_string(); @@ -234,10 +243,35 @@ where } } +fn command_output_with_timeout(command: &mut Command, timeout: Duration) -> Result { + let mut child = command + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .context("starting command")?; + let started = Instant::now(); + + loop { + if child + .try_wait() + .context("checking command status")? + .is_some() + { + return child.wait_with_output().context("reading command output"); + } + if started.elapsed() >= timeout { + let _ = child.kill(); + let _ = child.wait(); + bail!("command timed out after {} seconds", timeout.as_secs()); + } + std::thread::sleep(Duration::from_millis(25)); + } +} + fn active_gh_token() -> Result { - let output = Command::new("gh") - .args(["auth", "token", "--hostname", "github.com"]) - .output() + let mut command = Command::new("gh"); + command.args(["auth", "token", "--hostname", "github.com"]); + let output = command_output_with_timeout(&mut command, GH_AUTH_COMMAND_TIMEOUT) .context("running gh auth token --hostname github.com")?; if !output.status.success() { @@ -253,29 +287,47 @@ fn active_gh_token() -> Result { } fn resolve_github_auth() -> GitHubAuthResolution { - resolve_github_auth_with(|name| std::env::var(name).ok(), active_gh_token) + resolve_github_env_auth_with(|name| std::env::var(name).ok()) +} + +fn resolve_github_gh_auth() -> GitHubAuthResolution { + resolve_github_gh_auth_with(active_gh_token) } fn github_auth() -> &'static GitHubAuthResolution { GITHUB_AUTH.get_or_init(resolve_github_auth) } +fn github_gh_auth() -> &'static GitHubAuthResolution { + GITHUB_GH_AUTH.get_or_init(resolve_github_gh_auth) +} + +fn github_effective_auth() -> &'static GitHubAuthResolution { + if github_auth().source.is_some() { + github_auth() + } else { + GITHUB_GH_AUTH.get().unwrap_or_else(github_auth) + } +} + fn github_token() -> Option<&'static str> { - github_auth().token.as_deref() + github_auth() + .token + .as_deref() + .or_else(|| GITHUB_GH_AUTH.get().and_then(|auth| auth.token.as_deref())) } fn github_auth_status_diagnostic() -> Option { - let output = Command::new("gh") - .args([ - "auth", - "status", - "--hostname", - "github.com", - "--json", - "hosts", - ]) - .output() - .ok()?; + let mut command = Command::new("gh"); + command.args([ + "auth", + "status", + "--hostname", + "github.com", + "--json", + "hosts", + ]); + let output = command_output_with_timeout(&mut command, GH_AUTH_COMMAND_TIMEOUT).ok()?; if !output.status.success() { return None; @@ -363,7 +415,7 @@ fn github_access_guidance( } fn github_failure_guidance(owner: &str, repo: &str) -> String { - let auth = github_auth(); + let auth = github_effective_auth(); let gh_status = match auth.source { Some(GitHubAuthSource::Env(_)) => None, Some(GitHubAuthSource::GhActive) | None => github_auth_status_diagnostic(), @@ -381,6 +433,37 @@ fn github_api_get(client: &reqwest::Client, url: &str) -> reqwest::RequestBuilde req } +fn should_retry_github_api_with_gh(status: reqwest::StatusCode) -> bool { + matches!( + status, + reqwest::StatusCode::UNAUTHORIZED + | reqwest::StatusCode::FORBIDDEN + | reqwest::StatusCode::NOT_FOUND + ) +} + +async fn send_github_api_get( + client: &reqwest::Client, + url: &str, + context: &str, +) -> Result { + let resp = github_api_get(client, url) + .send() + .await + .with_context(|| format!("{context} from {url}"))?; + if github_auth().source.is_none() + && GITHUB_GH_AUTH.get().is_none() + && should_retry_github_api_with_gh(resp.status()) + && github_gh_auth().token.is_some() + { + return github_api_get(client, url) + .send() + .await + .with_context(|| format!("{context} from {url} with GitHub CLI token")); + } + Ok(resp) +} + /// Fetch a GitHub release (latest or by tag). async fn fetch_github_release( client: &reqwest::Client, @@ -391,10 +474,7 @@ async fn fetch_github_release( ) -> Result { if let Some(tag) = tag { let url = format!("https://api.github.com/repos/{owner}/{repo}/releases/tags/{tag}"); - let resp = github_api_get(client, &url) - .send() - .await - .with_context(|| format!("fetching release from {url}"))?; + let resp = send_github_api_get(client, &url, "fetching release").await?; let status = resp.status(); if status == reqwest::StatusCode::NOT_FOUND { @@ -419,10 +499,7 @@ async fn fetch_github_release( let url = format!("https://api.github.com/repos/{owner}/{repo}/releases/latest"); - let resp = github_api_get(client, &url) - .send() - .await - .with_context(|| format!("fetching release from {url}"))?; + let resp = send_github_api_get(client, &url, "fetching release").await?; let status = resp.status(); if status == reqwest::StatusCode::NOT_FOUND { @@ -454,10 +531,7 @@ async fn fetch_github_release_page( ) -> Result> { let url = format!("https://api.github.com/repos/{owner}/{repo}/releases?per_page=100&page={page}"); - let resp = github_api_get(client, &url) - .send() - .await - .with_context(|| format!("fetching releases from {url}"))?; + let resp = send_github_api_get(client, &url, "fetching releases").await?; let status = resp.status(); if status == reqwest::StatusCode::NOT_FOUND { @@ -1138,6 +1212,59 @@ fn cleanup_dir(path: &Path) { } } +fn remove_dir_all_if_exists(path: &Path) -> Result<()> { + if path.exists() { + std::fs::remove_dir_all(path) + .with_context(|| format!("removing directory {}", path.display()))?; + } + Ok(()) +} + +fn rollback_staged_extensions( + installed_targets: &[PathBuf], + backups: &[(PathBuf, PathBuf)], +) -> Result<()> { + let mut errors = Vec::new(); + + for target in installed_targets.iter().rev() { + if let Err(error) = remove_dir_all_if_exists(target) { + errors.push(error.to_string()); + } + } + + for (target, backup) in backups.iter().rev() { + if !backup.exists() { + continue; + } + if target.exists() { + errors.push(format!( + "could not restore backup {} to {} because target still exists", + backup.display(), + target.display() + )); + continue; + } + if let Err(error) = std::fs::rename(backup, target).with_context(|| { + format!( + "restoring backup {} to {}", + backup.display(), + target.display() + ) + }) { + errors.push(error.to_string()); + } + } + + if errors.is_empty() { + Ok(()) + } else { + bail!( + "rollback after failed install was incomplete:\n- {}", + errors.join("\n- ") + ) + } +} + fn commit_staged_extensions(staged: &[StagedExtension], force: bool) -> Result<()> { commit_staged_extensions_with_hook(staged, force, |_| Ok(())) } @@ -1197,13 +1324,8 @@ where })(); if let Err(error) = result { - for target in installed_targets.iter().rev() { - cleanup_dir(target); - } - for (target, backup) in backups.iter().rev() { - if backup.exists() && !target.exists() { - let _ = std::fs::rename(backup, target); - } + if let Err(rollback_error) = rollback_staged_extensions(&installed_targets, &backups) { + bail!("{error}\n\n{rollback_error}"); } return Err(error); } @@ -2235,13 +2357,10 @@ mod tests { #[test] fn test_resolve_github_auth_prefers_env_token_over_gh() { - let auth = resolve_github_auth_with( - |name| match name { - "GH_TOKEN" => Some(" env-token \n".to_string()), - _ => None, - }, - || Ok("gh-token\n".to_string()), - ); + let auth = resolve_github_env_auth_with(|name| match name { + "GH_TOKEN" => Some(" env-token \n".to_string()), + _ => None, + }); assert_eq!(auth.token.as_deref(), Some("env-token")); assert_eq!(auth.source, Some(GitHubAuthSource::Env("GH_TOKEN"))); @@ -2249,16 +2368,22 @@ mod tests { } #[test] - fn test_resolve_github_auth_ignores_empty_env_and_uses_gh() { - let auth = resolve_github_auth_with( - |name| match name { - "GH_TOKEN" => Some(" ".to_string()), - "GITHUB_TOKEN" => None, - "HOMEBREW_GITHUB_API_TOKEN" => None, - _ => None, - }, - || Ok(" gh-token \n".to_string()), - ); + fn test_resolve_github_auth_ignores_empty_env_without_gh_lookup() { + let auth = resolve_github_env_auth_with(|name| match name { + "GH_TOKEN" => Some(" ".to_string()), + "GITHUB_TOKEN" => None, + "HOMEBREW_GITHUB_API_TOKEN" => None, + _ => None, + }); + + assert_eq!(auth.token, None); + assert_eq!(auth.source, None); + assert!(auth.gh_error.is_none()); + } + + #[test] + fn test_resolve_github_gh_auth_uses_gh_token_when_requested() { + let auth = resolve_github_gh_auth_with(|| Ok(" gh-token \n".to_string())); assert_eq!(auth.token.as_deref(), Some("gh-token")); assert_eq!(auth.source, Some(GitHubAuthSource::GhActive)); @@ -2267,8 +2392,7 @@ mod tests { #[test] fn test_resolve_github_auth_falls_back_to_anonymous_when_gh_fails() { - let auth = - resolve_github_auth_with(|_| None, || Err(anyhow::anyhow!("gh is not installed"))); + let auth = resolve_github_gh_auth_with(|| Err(anyhow::anyhow!("gh is not installed"))); assert_eq!(auth.token, None); assert_eq!(auth.source, None); @@ -3224,6 +3348,37 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn test_rollback_staged_extensions_reports_unrestored_backup() { + let dir = std::env::temp_dir().join(format!( + "pup-test-rollback-report-{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + + let target = dir.join("pup-foo"); + let backup = dir.join("backup-foo"); + std::fs::create_dir_all(&target).unwrap(); + std::fs::create_dir_all(&backup).unwrap(); + write_test_file(&target.join("pup-foo"), b"new-foo"); + write_test_file(&backup.join("pup-foo"), b"old-foo"); + + let result = rollback_staged_extensions(&[], &[(target.clone(), backup.clone())]); + + assert!(result.is_err()); + let message = result.unwrap_err().to_string(); + assert!(message.contains("rollback after failed install was incomplete")); + assert!(message.contains("target still exists")); + assert_eq!(std::fs::read(target.join("pup-foo")).unwrap(), b"new-foo"); + assert_eq!(std::fs::read(backup.join("pup-foo")).unwrap(), b"old-foo"); + + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn test_selected_archive_extension_names_accepts_exact_extension() { let available = vec!["bar".to_string(), "foo".to_string()]; From ea0b8ec811456d459e1ef668bcb43f9ba2665009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Wed, 17 Jun 2026 19:25:56 +0200 Subject: [PATCH 10/12] fix(extension): unify staged extension writes --- src/extensions/install.rs | 400 ++++++++++++++++++++++++++++---------- 1 file changed, 299 insertions(+), 101 deletions(-) diff --git a/src/extensions/install.rs b/src/extensions/install.rs index 3acac8d3..baf85982 100644 --- a/src/extensions/install.rs +++ b/src/extensions/install.rs @@ -986,7 +986,7 @@ fn selected_archive_extension_names( /// Download a release asset. Authenticated GitHub API asset URLs are used when /// a GitHub token is available; public browser URLs remain the fallback. async fn download_asset(client: &reqwest::Client, asset: &GitHubAsset) -> Result> { - download_asset_with_limit(client, asset, None).await + download_asset_with_limit(client, asset, Some(MAX_EXTENSION_BINARY_BYTES)).await } async fn download_asset_with_limit( @@ -994,6 +994,16 @@ async fn download_asset_with_limit( asset: &GitHubAsset, max_bytes: Option, ) -> Result> { + if let Some(max_bytes) = max_bytes { + if asset.size.is_some_and(|size| size > max_bytes as u64) { + bail!( + "asset '{}' is larger than the {} byte limit", + asset.name, + max_bytes + ); + } + } + let token = github_token(); let use_api_asset = token.is_some() && asset.url.is_some(); let url = if use_api_asset { @@ -1021,19 +1031,12 @@ async fn download_asset_with_limit( } if let Some(max_bytes) = max_bytes { - if asset.size.is_some_and(|size| size > max_bytes as u64) { - bail!( - "asset '{}' is larger than the {} byte scan limit", - asset.name, - max_bytes - ); - } if resp .content_length() .is_some_and(|content_length| content_length > max_bytes as u64) { bail!( - "asset '{}' is larger than the {} byte scan limit", + "asset '{}' is larger than the {} byte limit", asset.name, max_bytes ); @@ -1047,7 +1050,7 @@ async fn download_asset_with_limit( let chunk = chunk.with_context(|| format!("reading asset bytes from {url}"))?; if bytes.len() + chunk.len() > max_bytes { bail!( - "asset '{}' is larger than the {} byte scan limit", + "asset '{}' is larger than the {} byte limit", asset.name, max_bytes ); @@ -1153,20 +1156,6 @@ pub fn derive_name_from_repo(repo: &str) -> String { repo.strip_prefix("pup-").unwrap_or(repo).to_string() } -/// Prepare (create or recreate) an extension directory. -fn prepare_extension_dir(ext_dir: &Path) -> Result<()> { - if ext_dir.exists() { - std::fs::remove_dir_all(ext_dir).with_context(|| { - format!( - "removing existing extension directory: {}", - ext_dir.display() - ) - })?; - } - std::fs::create_dir_all(ext_dir).with_context(|| format!("creating {}", ext_dir.display()))?; - Ok(()) -} - /// Write a binary to the extension directory and set executable permissions. /// Returns the executable filename (e.g., "pup-hello" or "pup-hello.exe"). fn write_extension_binary(ext_dir: &Path, name: &str, bytes: &[u8]) -> Result { @@ -1198,6 +1187,40 @@ struct StagedExtension { backup_dir: PathBuf, } +#[derive(Debug)] +struct CommitStagedError { + message: String, + rollback_incomplete: bool, +} + +impl CommitStagedError { + fn new(error: anyhow::Error) -> Self { + Self { + message: error.to_string(), + rollback_incomplete: false, + } + } + + fn rollback_incomplete(error: anyhow::Error, rollback_error: anyhow::Error) -> Self { + Self { + message: format!("{error}\n\n{rollback_error}"), + rollback_incomplete: true, + } + } + + fn into_anyhow(self) -> anyhow::Error { + anyhow::anyhow!(self.message) + } +} + +impl std::fmt::Display for CommitStagedError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.message) + } +} + +impl std::error::Error for CommitStagedError {} + fn unique_work_dir(parent: &Path, prefix: &str) -> PathBuf { let nanos = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -1265,7 +1288,10 @@ fn rollback_staged_extensions( } } -fn commit_staged_extensions(staged: &[StagedExtension], force: bool) -> Result<()> { +fn commit_staged_extensions( + staged: &[StagedExtension], + force: bool, +) -> std::result::Result<(), CommitStagedError> { commit_staged_extensions_with_hook(staged, force, |_| Ok(())) } @@ -1273,7 +1299,7 @@ fn commit_staged_extensions_with_hook( staged: &[StagedExtension], force: bool, mut before_install: F, -) -> Result<()> +) -> std::result::Result<(), CommitStagedError> where F: FnMut(usize) -> Result<()>, { @@ -1325,9 +1351,12 @@ where if let Err(error) = result { if let Err(rollback_error) = rollback_staged_extensions(&installed_targets, &backups) { - bail!("{error}\n\n{rollback_error}"); + return Err(CommitStagedError::rollback_incomplete( + error, + rollback_error, + )); } - return Err(error); + return Err(CommitStagedError::new(error)); } for (_, backup) in backups { @@ -1337,6 +1366,50 @@ where Ok(()) } +fn commit_staged_extensions_and_cleanup( + staged: &[StagedExtension], + force: bool, + staging_base: &Path, + backup_base: &Path, +) -> Result<()> { + commit_staged_extensions_with_cleanup( + staged, + force, + staging_base, + backup_base, + commit_staged_extensions, + ) +} + +fn commit_staged_extensions_with_cleanup( + staged: &[StagedExtension], + force: bool, + staging_base: &Path, + backup_base: &Path, + commit: F, +) -> Result<()> +where + F: FnOnce(&[StagedExtension], bool) -> std::result::Result<(), CommitStagedError>, +{ + if let Err(error) = commit(staged, force) { + let rollback_incomplete = error.rollback_incomplete; + let error = error.into_anyhow(); + cleanup_dir(staging_base); + if rollback_incomplete { + bail!( + "{error}\n\nremaining extension backups were preserved in {}", + backup_base.display() + ); + } + cleanup_dir(backup_base); + return Err(error); + } + + cleanup_dir(staging_base); + cleanup_dir(backup_base); + Ok(()) +} + struct ExtensionPayload { name: String, bytes: Vec, @@ -1400,6 +1473,25 @@ fn save_github_payloads( force: bool, description: Option<&str>, ) -> Result> { + save_github_payloads_with_commit( + source, + artifacts, + force, + description, + commit_staged_extensions, + ) +} + +fn save_github_payloads_with_commit( + source: &str, + artifacts: GitHubInstallArtifacts, + force: bool, + description: Option<&str>, + commit: F, +) -> Result> +where + F: FnOnce(&[StagedExtension], bool) -> std::result::Result<(), CommitStagedError>, +{ if artifacts.payloads.is_empty() { bail!("no extensions selected to install"); } @@ -1487,14 +1579,7 @@ fn save_github_payloads( } }; - if let Err(error) = commit_staged_extensions(&staged, force) { - cleanup_dir(&staging_base); - cleanup_dir(&backup_base); - return Err(error); - } - - cleanup_dir(&staging_base); - cleanup_dir(&backup_base); + commit_staged_extensions_with_cleanup(&staged, force, &staging_base, &backup_base, commit)?; Ok(installed) } @@ -2096,25 +2181,26 @@ pub fn upgrade_extension(name: &str) -> Result { }) })?; - // Prepare (recreate) the extension directory before writing, so a failed write - // does not leave a partially-corrupted state. - prepare_extension_dir(&ext_dir)?; - - let exe_name = write_extension_binary(&ext_dir, name, &asset_bytes)?; - - // Update the manifest - let updated_manifest = Manifest { - version: new_version.clone(), - source: format!("github:{gh_source}"), - source_kind: None, - source_release_tag: None, - source_asset: None, - installed_at: chrono_now_iso(), - binary: exe_name, - installed_by_pup: version::VERSION.to_string(), - ..manifest + let description = if manifest.description.is_empty() { + None + } else { + Some(manifest.description.as_str()) }; - updated_manifest.save(&ext_dir.join("manifest.json"))?; + save_github_payloads( + gh_source, + GitHubInstallArtifacts { + version: new_version.clone(), + source_kind: None, + source_release_tag: None, + source_asset: None, + payloads: vec![ExtensionPayload { + name: name.to_string(), + bytes: asset_bytes, + }], + }, + true, + description, + )?; Ok(format!("{name}: upgraded {old_version} -> {new_version}")) } @@ -2210,61 +2296,93 @@ pub fn install_from_local( bail!("extension '{name}' is already installed (use --force to overwrite)"); } - prepare_extension_dir(&ext_dir)?; + let staging_base = unique_work_dir(&ext_base, "pup-install-staging"); + std::fs::create_dir_all(&staging_base) + .with_context(|| format!("creating staging directory {}", staging_base.display()))?; + let backup_base = unique_work_dir(&ext_base, "pup-install-backup"); + if let Err(error) = std::fs::create_dir_all(&backup_base) + .with_context(|| format!("creating backup directory {}", backup_base.display())) + { + cleanup_dir(&staging_base); + return Err(error); + } + + let stage_dir = staging_base.join(format!("pup-{name}")); + let stage_result = (|| -> Result { + std::fs::create_dir_all(&stage_dir) + .with_context(|| format!("creating {}", stage_dir.display()))?; - let exe_name = if link { - // For symlinks, we need to create the link directly rather than writing bytes. - let exe_name = if cfg!(target_os = "windows") { - format!("pup-{name}.exe") + let exe_name = if link { + // For symlinks, we need to create the link directly rather than writing bytes. + let exe_name = if cfg!(target_os = "windows") { + format!("pup-{name}.exe") + } else { + format!("pup-{name}") + }; + let dest = stage_dir.join(&exe_name); + + #[cfg(unix)] + std::os::unix::fs::symlink(&source, &dest).with_context(|| { + format!( + "creating symlink {} -> {}", + dest.display(), + source.display() + ) + })?; + #[cfg(windows)] + std::os::windows::fs::symlink_file(&source, &dest).with_context(|| { + format!( + "creating symlink {} -> {}", + dest.display(), + source.display() + ) + })?; + + exe_name } else { - format!("pup-{name}") + let bytes = std::fs::read(&source) + .with_context(|| format!("reading source file: {}", source.display()))?; + write_extension_binary(&stage_dir, name, &bytes)? }; - let dest = ext_dir.join(&exe_name); - #[cfg(unix)] - std::os::unix::fs::symlink(&source, &dest).with_context(|| { - format!( - "creating symlink {} -> {}", - dest.display(), - source.display() - ) - })?; - #[cfg(windows)] - std::os::windows::fs::symlink_file(&source, &dest).with_context(|| { - format!( - "creating symlink {} -> {}", - dest.display(), - source.display() - ) - })?; + let source_str = if link { + format!("local-link:{}", source.display()) + } else { + format!("local:{}", source.display()) + }; - exe_name - } else { - let bytes = std::fs::read(&source) - .with_context(|| format!("reading source file: {}", source.display()))?; - write_extension_binary(&ext_dir, name, &bytes)? - }; + // Local installs have no version source (unlike GitHub releases which provide a tag). + let manifest = Manifest { + name: name.to_string(), + version: "unknown".to_string(), + source: source_str, + source_kind: None, + source_release_tag: None, + source_asset: None, + installed_at: chrono_now_iso(), + binary: exe_name, + description: description.unwrap_or_default().to_string(), + installed_by_pup: version::VERSION.to_string(), + }; + manifest.save(&stage_dir.join("manifest.json"))?; - let source_str = if link { - format!("local-link:{}", source.display()) - } else { - format!("local:{}", source.display()) - }; + Ok(StagedExtension { + target_dir: ext_dir, + stage_dir, + backup_dir: backup_base.join(format!("pup-{name}")), + }) + })(); - // Local installs have no version source (unlike GitHub releases which provide a tag). - let manifest = Manifest { - name: name.to_string(), - version: "unknown".to_string(), - source: source_str, - source_kind: None, - source_release_tag: None, - source_asset: None, - installed_at: chrono_now_iso(), - binary: exe_name, - description: description.unwrap_or_default().to_string(), - installed_by_pup: version::VERSION.to_string(), + let staged = match stage_result { + Ok(staged) => vec![staged], + Err(error) => { + cleanup_dir(&staging_base); + cleanup_dir(&backup_base); + return Err(error); + } }; - manifest.save(&ext_dir.join("manifest.json"))?; + + commit_staged_extensions_and_cleanup(&staged, force, &staging_base, &backup_base)?; Ok(()) } @@ -2448,6 +2566,26 @@ mod tests { assert!(!diagnostic.contains("bob")); } + #[test] + fn test_download_asset_applies_default_binary_size_limit_from_metadata() { + let client = github_client().unwrap(); + let asset = GitHubAsset { + name: "pup-foo-linux-x86_64".to_string(), + url: None, + size: Some(MAX_EXTENSION_BINARY_BYTES as u64 + 1), + browser_download_url: "http://127.0.0.1:1/pup-foo".to_string(), + }; + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + + let result = runtime.block_on(download_asset(&client, &asset)); + + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("byte limit")); + } + #[test] fn test_remove_rejects_path_traversal() { // remove_extension must reject names with path traversal characters @@ -3379,6 +3517,66 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn test_save_github_payloads_preserves_backups_after_incomplete_rollback() { + let dir = std::env::temp_dir().join(format!( + "pup-test-save-preserve-backup-{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + + let _guard = crate::test_utils::ENV_LOCK.blocking_lock(); + std::env::set_var("PUP_CONFIG_DIR", &dir); + + let artifacts = GitHubInstallArtifacts { + version: "1.2.3".to_string(), + source_kind: None, + source_release_tag: None, + source_asset: None, + payloads: vec![ExtensionPayload { + name: "foo".to_string(), + bytes: b"new-foo".to_vec(), + }], + }; + + let result = + save_github_payloads_with_commit("owner/repo", artifacts, true, None, |staged, _| { + std::fs::create_dir_all(&staged[0].backup_dir).unwrap(); + write_test_file(&staged[0].backup_dir.join("pup-foo"), b"old-foo"); + Err(CommitStagedError::rollback_incomplete( + anyhow::anyhow!("install failed"), + anyhow::anyhow!("rollback failed"), + )) + }); + + assert!(result.is_err()); + let message = result.unwrap_err().to_string(); + assert!(message.contains("rollback failed")); + assert!(message.contains("remaining extension backups were preserved")); + + let backup_dirs = std::fs::read_dir(dir.join("extensions")) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter(|path| { + path.file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.starts_with(".pup-install-backup-")) + }) + .collect::>(); + assert_eq!(backup_dirs.len(), 1); + assert_eq!( + std::fs::read(backup_dirs[0].join("pup-foo").join("pup-foo")).unwrap(), + b"old-foo" + ); + + std::env::remove_var("PUP_CONFIG_DIR"); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn test_selected_archive_extension_names_accepts_exact_extension() { let available = vec!["bar".to_string(), "foo".to_string()]; From a8d0514e94a455535464b06565cc0a0a9c424852 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Thu, 18 Jun 2026 19:04:10 +0200 Subject: [PATCH 11/12] docs(wasm): document native-only extensions --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 1c0a113c..2983bdd2 100644 --- a/README.md +++ b/README.md @@ -497,6 +497,7 @@ The `pup auth status` command works in WASM and reports which credentials are co - No local token storage (keychain/file) — use `DD_ACCESS_TOKEN` or API keys - No browser-based OAuth login flow +- Extensions are not included in WASM builds; `pup extension ...` and installed extension dispatch are native-only - Networking relies on the host runtime's networking capabilities ### Running with Wasmtime From 8fc8f3338d4e8e6642acac527044187c71dae9d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=81nis=20Kir=C5=A1teins?= Date: Thu, 18 Jun 2026 19:22:12 +0200 Subject: [PATCH 12/12] fix(ci): update test configs for jq field --- src/config.rs | 1 + src/extensions/exec.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/src/config.rs b/src/config.rs index 4e3d38a2..d82aae22 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1936,6 +1936,7 @@ mod tests { auto_approve: false, agent_mode: false, read_only: false, + jq: None, }; super::apply_org_override(&mut cfg, "unknown-org".into()).unwrap(); diff --git a/src/extensions/exec.rs b/src/extensions/exec.rs index da140dcc..ff171186 100644 --- a/src/extensions/exec.rs +++ b/src/extensions/exec.rs @@ -128,6 +128,7 @@ mod tests { auto_approve: false, agent_mode: false, read_only: false, + jq: None, } }