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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ semver = "1.0.27"
serde = { version = "1.0.228", features = ["derive"] }
serde_json = { version = "1.0.149", features = ["indexmap"] }
serial_test = "3.3.1"
sha2 = "0.10.9"
soar-config = { version = "0.7.0", path = "crates/soar-config" }
soar-core = { version = "0.15.0", path = "crates/soar-core" }
soar-db = { version = "0.5.1", path = "crates/soar-db" }
Expand Down
18 changes: 12 additions & 6 deletions crates/soar-core/src/package/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,13 @@ impl PackageInstaller {
callback(Progress::Aborted);
}
// Return error after max retries
return Err(last_error.unwrap_or_else(|| {
DownloadError::Multiple {
errors: vec!["Download failed after 5 retries".into()],
}
}))?;
return Err(last_error
.unwrap_or_else(|| {
DownloadError::Multiple {
errors: vec!["Download failed after 5 retries".into()],
}
})
.into());
}
match dl.clone().execute() {
Ok(_) => {
Expand All @@ -605,7 +607,7 @@ impl PackageInstaller {
}
last_error = Some(err);
} else {
return Err(err)?;
return Err(err.into());
}
}
}
Expand Down Expand Up @@ -634,6 +636,10 @@ impl PackageInstaller {
.extract(should_extract)
.extract_to(&extract_dir);

if let Some(ref bsum) = self.package.bsum {
dl = dl.checksum(bsum);
}

if let Some(ref cb) = self.progress_callback {
let cb = cb.clone();
dl = dl.progress(move |p| {
Expand Down
1 change: 1 addition & 0 deletions crates/soar-dl/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ percent-encoding = { workspace = true }
regex = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
sha2 = { workspace = true }
soar-utils = { workspace = true }
thiserror = { workspace = true }
tracing = { workspace = true }
Expand Down
90 changes: 87 additions & 3 deletions crates/soar-dl/src/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub struct Download {
pub extract_to: Option<PathBuf>,
pub on_progress: Option<Arc<dyn Fn(Progress) + Send + Sync>>,
pub ghcr_blob: bool,
pub expected_checksum: Option<String>,
}

impl Download {
Expand Down Expand Up @@ -63,9 +64,25 @@ impl Download {
extract_to: None,
on_progress: None,
ghcr_blob: false,
expected_checksum: None,
}
}

/// Sets the expected blake3 checksum (hex) to verify the downloaded file
/// against before it is made executable or extracted.
///
/// # Examples
///
/// ```
/// use soar_dl::download::Download;
///
/// let _ = Download::new("https://example.com/file").checksum("abcdef123456");
/// ```
pub fn checksum(mut self, checksum: impl Into<String>) -> Self {
self.expected_checksum = Some(checksum.into());
self
}

/// Turns on GHCR blob support.
///
/// When enabled, the `Authorization` header is set to `Bearer QQ==`
Expand Down Expand Up @@ -257,10 +274,16 @@ impl Download {
// Only skip if there's no resume info (complete download)
// If resume info exists, it's a partial download that should continue
if resume_info.is_none() {
debug!(path = %output_path.display(), "file exists, skipping download");
return Ok(output_path);
if self.verify_checksum(&output_path).is_ok() {
debug!(path = %output_path.display(), "file exists, skipping download");
return Ok(output_path);
}
warn!(path = %output_path.display(), "cached file failed checksum, re-downloading");
fs::remove_file(&output_path)?;
resume_info = None;
} else {
debug!(path = %output_path.display(), "file exists but is partial, resuming download");
}
debug!(path = %output_path.display(), "file exists but is partial, resuming download");
}
OverwriteMode::Force => {
debug!(path = %output_path.display(), "file exists, forcing overwrite");
Expand All @@ -287,6 +310,11 @@ impl Download {

self.download_to_file(&output_path, resume_info)?;

if let Err(e) = self.verify_checksum(&output_path) {
fs::remove_file(&output_path).ok();
return Err(e);
}

if is_elf(&output_path) {
trace!(path = %output_path.display(), "detected ELF binary, setting executable permissions");
std::fs::set_permissions(&output_path, Permissions::from_mode(0o755))?;
Expand All @@ -310,6 +338,22 @@ impl Download {
Ok(output_path)
}

fn verify_checksum(&self, path: &Path) -> Result<(), DownloadError> {
let Some(ref expected) = self.expected_checksum else {
return Ok(());
};
let actual = soar_utils::hash::calculate_checksum(path)
.map_err(|e| DownloadError::Io(std::io::Error::other(e.to_string())))?;
if actual.eq_ignore_ascii_case(expected) {
Ok(())
} else {
Err(DownloadError::ChecksumMismatch {
expected: expected.clone(),
got: actual,
})
}
}

/// Streams the HTTP response body for this download's URL to standard output.
///
/// # Examples
Expand Down Expand Up @@ -502,3 +546,43 @@ fn prompt_overwrite(path: &Path) -> std::io::Result<bool> {

Ok(matches!(line.trim().to_lowercase().as_str(), "y" | "yes"))
}

#[cfg(test)]
mod tests {
use std::io::Write;

use super::*;

fn temp_with(contents: &[u8]) -> tempfile::NamedTempFile {
let mut f = tempfile::NamedTempFile::new().unwrap();
f.write_all(contents).unwrap();
f.flush().unwrap();
f
}

#[test]
fn verify_checksum_ok_when_absent_or_matching() {
let f = temp_with(b"hello soar");
let expected = soar_utils::hash::calculate_checksum(f.path()).unwrap();

let dl = Download::new("https://example.com/x");
assert!(dl.verify_checksum(f.path()).is_ok());

let dl = Download::new("https://example.com/x").checksum(expected.to_uppercase());
assert!(dl.verify_checksum(f.path()).is_ok());
}

#[test]
fn verify_checksum_rejects_mismatch() {
let f = temp_with(b"hello soar");
let dl = Download::new("https://example.com/x").checksum("deadbeef");
match dl.verify_checksum(f.path()) {
Err(DownloadError::ChecksumMismatch {
expected, ..
}) => {
assert_eq!(expected, "deadbeef");
}
other => panic!("expected ChecksumMismatch, got {other:?}"),
}
}
}
8 changes: 8 additions & 0 deletions crates/soar-dl/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@ pub enum DownloadError {
#[diagnostic(code(soar_dl::unsafe_layer_path))]
UnsafeLayerPath { title: String },

#[error("Checksum mismatch: expected {expected}, got {got}")]
#[diagnostic(code(soar_dl::checksum_mismatch))]
ChecksumMismatch { expected: String, got: String },

#[error("Digest mismatch: expected {expected}, got {got}")]
#[diagnostic(code(soar_dl::digest_mismatch))]
DigestMismatch { expected: String, got: String },

#[error("Invalid response from server")]
#[diagnostic(code(soar_dl::invalid_response))]
InvalidResponse,
Expand Down
81 changes: 77 additions & 4 deletions crates/soar-dl/src/oci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use std::{
};

use serde::Deserialize;
use sha2::{Digest, Sha256};
use soar_utils::fs::is_elf;
use tracing::{debug, trace};
use ureq::http::header::{ACCEPT, AUTHORIZATION, ETAG, IF_RANGE, RANGE};
Expand Down Expand Up @@ -153,14 +154,46 @@ impl OciLayer {
/// Only the final path component of the title is used, so titles such as
/// `../../etc/passwd`, `/etc/passwd`, or `..` cannot escape `output_dir`.
fn safe_layer_path(output_dir: &Path, title: &str) -> Result<PathBuf, DownloadError> {
let name = Path::new(title)
.file_name()
.ok_or_else(|| DownloadError::UnsafeLayerPath {
let name = Path::new(title).file_name().ok_or_else(|| {
DownloadError::UnsafeLayerPath {
title: title.to_string(),
})?;
}
})?;
Ok(output_dir.join(name))
}

/// Verifies the file at `path` against an OCI content-addressable digest
/// (e.g. `sha256:<hex>`). On mismatch the file is removed and an error returned.
fn verify_layer_digest(path: &Path, digest: &str) -> Result<(), DownloadError> {
let expected = digest.strip_prefix("sha256:").unwrap_or(digest);

let mut file = File::open(path)?;
let mut hasher = Sha256::new();
let mut buffer = [0u8; 8192];
loop {
let n = file.read(&mut buffer)?;
if n == 0 {
break;
}
hasher.update(&buffer[..n]);
}
let got: String = hasher
.finalize()
.iter()
.map(|b| format!("{b:02x}"))
.collect();

if got.eq_ignore_ascii_case(expected) {
Ok(())
} else {
std::fs::remove_file(path).ok();
Err(DownloadError::DigestMismatch {
expected: expected.to_string(),
got,
})
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

#[derive(Clone)]
pub struct OciDownload {
reference: OciReference,
Expand Down Expand Up @@ -483,6 +516,7 @@ impl OciDownload {
if path.is_file() {
if let Ok(metadata) = path.metadata() {
if metadata.len() == layer.size {
verify_layer_digest(&path, &layer.digest)?;
downloaded += layer.size;
if let Some(ref cb) = self.on_progress {
cb(Progress::Chunk {
Expand Down Expand Up @@ -590,6 +624,10 @@ impl OciDownload {
if path.is_file() {
if let Ok(metadata) = path.metadata() {
if metadata.len() == layer.size {
if let Err(e) = verify_layer_digest(&path, &layer.digest) {
errors.lock().unwrap().push(format!("{e}"));
continue;
}
let current = downloaded
.fetch_add(layer.size, Ordering::Relaxed)
+ layer.size;
Expand Down Expand Up @@ -766,6 +804,8 @@ impl OciDownload {

let path = dl.execute()?;

verify_layer_digest(&path, &self.reference.tag)?;

Ok(vec![path])
}

Expand Down Expand Up @@ -957,6 +997,8 @@ fn download_layer_impl(
}
}

verify_layer_digest(path, &layer.digest)?;

if is_elf(path) {
trace!(path = %path.display(), "setting executable permissions on ELF binary");
std::fs::set_permissions(path, Permissions::from_mode(0o755))?;
Expand All @@ -975,6 +1017,37 @@ fn download_layer_impl(
mod tests {
use super::*;

#[test]
fn verify_layer_digest_accepts_correct_and_rejects_wrong() {
use std::io::Write;

let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("layer.bin");
{
let mut f = File::create(&path).unwrap();
f.write_all(b"layer-bytes").unwrap();
}

let mut hasher = Sha256::new();
hasher.update(b"layer-bytes");
let hex: String = hasher
.finalize()
.iter()
.map(|b| format!("{b:02x}"))
.collect();

assert!(verify_layer_digest(&path, &format!("sha256:{hex}")).is_ok());
assert!(path.exists());

match verify_layer_digest(&path, "sha256:00ff") {
Err(DownloadError::DigestMismatch {
..
}) => {}
other => panic!("expected DigestMismatch, got {other:?}"),
}
assert!(!path.exists(), "file should be removed on digest mismatch");
}

#[test]
fn safe_layer_path_keeps_plain_name_inside_output_dir() {
let out = Path::new("/tmp/soar-out");
Expand Down
Loading