Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 42 additions & 11 deletions src/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ fn verify_sha256(path: &std::path::Path, expected_hex: &str) -> Result<()> {
/// Ensure a filename is a single normal path component (no `..`, no path
/// separators, not absolute) before it is joined into a destination directory.
/// Shared by archive extraction and raw-asset download to block path traversal.
fn ensure_safe_component(name: &str) -> Result<()> {
pub(crate) fn ensure_safe_component(name: &str) -> Result<()> {
let p = std::path::Path::new(name);
anyhow::ensure!(
p.components().count() == 1
Expand Down Expand Up @@ -248,20 +248,39 @@ fn extract_binary_from_archive(
}
}

/// Download a release asset to `<colony_apps_dir>/<repo_name>/<filename>`.
/// If `expected_sha256` is provided, verifies the file integrity after download.
/// If `binary_name` is provided, treats the downloaded file as an archive and
/// extracts the named binary from it (supports .zip and .tar.gz).
/// Returns the final path on success.
/// Everything needed to install one resolved release asset.
pub struct AssetInstall {
pub repo_name: String,
/// The resolved (never "latest") release tag being installed.
pub tag: String,
/// The resolved asset name to download.
pub filename: String,
/// When set, the download is an archive and this named binary is extracted.
pub binary_name: Option<String>,
/// When set, the download is integrity-checked against this hex digest.
pub expected_sha256: Option<String>,
/// True when `filename` was resolved from a filePattern: the name is then
/// persisted next to the binary so `installed_app_path` can find the
/// install again.
pub record_asset: bool,
}

/// Download a release asset to `<colony_apps_dir>/<repo_name>/<filename>`,
/// verify/extract it, atomically promote it into place, and record the
/// installed version. Returns the final path on success.
pub async fn download_release_asset(
token: Option<String>,
repo_name: String,
tag: String,
filename: String,
binary_name: Option<String>,
expected_sha256: Option<String>,
install: AssetInstall,
progress_tx: Option<futures::channel::mpsc::UnboundedSender<f32>>,
) -> Result<PathBuf> {
let AssetInstall {
repo_name,
tag,
filename,
binary_name,
expected_sha256,
record_asset,
} = install;
// The manifest-supplied filename becomes a local path — guard it against
// traversal (`../`, absolute paths) before joining, mirroring the archive
// `binary` guard.
Expand Down Expand Up @@ -324,6 +343,18 @@ pub async fn download_release_asset(
std::fs::set_permissions(&final_path, perms)?;
}

// Record WHAT is installed here, atomically with the install
// itself - not in the UI message handler. A cancel mid-install
// drops the awaiting future but this blocking task runs to
// completion: writing the version from the handler meant an
// installed binary with no version file, silently excluded from
// every future update check (or, in the filePattern case, an
// orphaned binary the app no longer even sees as installed).
crate::persistence::save_installed_version(&repo_name, &tag)?;
if record_asset {
crate::persistence::save_installed_asset(&repo_name, &filename)?;
}

Ok(final_path)
})
.await?
Expand Down
54 changes: 51 additions & 3 deletions src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ pub use crate::download::{apply_launcher_update, download_launcher_asset, downlo
pub use crate::persistence::{
colony_apps_dir, colony_data_dir, installed_app_path, load_favorites,
load_installed_version, load_preferences, load_repos_cache, load_scan_cache, read_repo_doc,
save_favorites, save_installed_asset, save_installed_version, save_preferences,
save_repos_cache, save_scan_cache, CachedApp, UserPreferences,
save_favorites, save_preferences, save_repos_cache, save_scan_cache, CachedApp,
UserPreferences,
};
use crate::persistence::save_repo_doc;
use crate::persistence::save_repo_icon;
Expand Down Expand Up @@ -679,11 +679,29 @@ pub async fn fetch_release_info(

/// Find an asset whose name contains the given pattern (case-insensitive).
/// Returns an error if zero or multiple assets match.
/// Metadata companions published alongside release binaries (signatures,
/// checksums, updater manifests). Never installable, so they are excluded from
/// pattern matching - otherwise `app-linux.sig` would make the pattern
/// "linux" ambiguous the day a repo starts signing its releases (Colony's own
/// releases already ship `.sig` siblings).
const NON_INSTALLABLE_SUFFIXES: &[&str] =
&[".sig", ".asc", ".sha256", ".sha256sum", ".txt", ".yml", ".yaml", ".json"];

pub fn find_asset_by_pattern(assets: &[String], pattern: &str) -> Result<String> {
let pattern_lower = pattern.to_lowercase();
// An exact (case-insensitive) name match always wins: it cannot be
// ambiguous, and it lets a manifest pin `app-macos` even though
// `app-macos-x86` also contains that pattern as a substring.
if let Some(exact) = assets.iter().find(|n| n.to_lowercase() == pattern_lower) {
return Ok(exact.clone());
}
let matches: Vec<&String> = assets
.iter()
.filter(|name| name.to_lowercase().contains(&pattern_lower))
.filter(|name| {
let lower = name.to_lowercase();
lower.contains(&pattern_lower)
&& !NON_INSTALLABLE_SUFFIXES.iter().any(|s| lower.ends_with(s))
})
.collect();
match matches.len() {
0 => anyhow::bail!("No release asset matching pattern '{pattern}'"),
Expand Down Expand Up @@ -1043,6 +1061,36 @@ mod tests {
assert!(result.unwrap_err().to_string().contains("Ambiguous"));
}

#[test]
fn find_asset_by_pattern_exact_name_beats_substring_overlap() {
// "app-macos" is a substring of "app-macos-x86": an exact name match
// must win instead of erroring as ambiguous, so Apple Silicon
// manifests can pin the shorter asset name.
let assets = vec!["app-macos".to_string(), "app-macos-x86".to_string()];
assert_eq!(
find_asset_by_pattern(&assets, "app-macos").unwrap(),
"app-macos"
);
assert_eq!(
find_asset_by_pattern(&assets, "app-macos-x86").unwrap(),
"app-macos-x86"
);
}

#[test]
fn find_asset_by_pattern_ignores_signature_and_checksum_siblings() {
// The day a repo signs its releases (like Colony itself), every binary
// grows a .sig sibling containing the same name: the pattern must
// keep resolving to the binary, not error as ambiguous.
let assets = vec![
"app-linux".to_string(),
"app-linux.sig".to_string(),
"app-linux.sha256".to_string(),
"latest-linux.yml".to_string(),
];
assert_eq!(find_asset_by_pattern(&assets, "linux").unwrap(), "app-linux");
}

#[test]
fn parse_colony_manifest_missing_required_field() {
// category is required, so missing it should fail
Expand Down
12 changes: 12 additions & 0 deletions src/persistence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,18 @@ pub fn installed_app_path(repo: &ColonyRepo) -> Option<PathBuf> {
// filePattern was used — check saved resolved asset name
load_installed_asset(&repo.name)?
};
// The filename comes straight from the repo's own manifest: the same
// traversal guard as the install path applies, or a hostile manifest
// could point the Launch button at an arbitrary executable on disk
// (e.g. `binary: "../../somewhere/else"`).
if crate::download::ensure_safe_component(&filename).is_err() {
tracing::warn!(
repo = %repo.name,
%filename,
"refusing manifest launch path outside the app's install dir"
);
return None;
}
let path = colony_apps_dir().ok()?.join(&repo.name).join(&filename);
if path.exists() {
Some(path)
Expand Down
33 changes: 18 additions & 15 deletions src/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,23 +470,26 @@ impl App {
(tag, f)
};

// The version/asset records are written by
// download_release_asset itself, inside the
// blocking install step: writing them here (or
// in DownloadCompleted) meant a cancel landing
// mid-install detached the blocking task and
// left an installed binary with no metadata.
let path = github::download_release_asset(
token,
repo_name.clone(),
resolved_tag.clone(),
resolved_file.clone(),
binary,
expected_sha256,
crate::download::AssetInstall {
repo_name: repo_name.clone(),
tag: resolved_tag.clone(),
filename: resolved_file.clone(),
binary_name: binary,
expected_sha256,
record_asset: file_pattern.is_some(),
},
Some(progress_tx),
)
.await?;

// Save resolved asset name when using filePattern (for installed_app_path)
if file_pattern.is_some() {
let _ =
github::save_installed_asset(&repo_name, &resolved_file);
}

Ok((path, dl_repo, resolved_tag))
},
|result: Result<_, anyhow::Error>| {
Expand Down Expand Up @@ -525,10 +528,10 @@ impl App {
self.is_downloading = false;
self.download_abort = None;
match result {
Ok((path, repo_name, tag)) => {
if let Err(e) = github::save_installed_version(&repo_name, &tag) {
tracing::warn!("Failed to save version info: {e}");
}
// Version/asset records were written atomically with the
// install (inside download_release_asset), so the tag is
// no longer needed here.
Ok((path, repo_name, _tag)) => {
// The just-installed version IS the one the badge was
// advertising: clear it, or the card keeps showing
// "Update vX -> vX" until the next global check.
Expand Down
Loading