diff --git a/.github/workflows/registry-download-smoke.yml b/.github/workflows/registry-download-smoke.yml new file mode 100644 index 00000000..6a951d91 --- /dev/null +++ b/.github/workflows/registry-download-smoke.yml @@ -0,0 +1,115 @@ +name: registry-download-smoke + +on: + pull_request: + paths: + - crates/commandf-pkg/src/archive.rs + - crates/commandf-pkg/src/registry.rs + - .github/workflows/registry-download-smoke.yml + push: + branches: + - main + paths: + - crates/commandf-pkg/src/archive.rs + - crates/commandf-pkg/src/registry.rs + - .github/workflows/registry-download-smoke.yml + workflow_dispatch: + +permissions: + contents: read + +jobs: + registry-download: + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5 / node24 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@032958afbdc797a9164d3bc0b56325c1308924a5 # 1.97.1 + with: + components: rustfmt, clippy + + - name: Format + run: cargo fmt --all -- --check + + - name: Focused registry unit tests + run: cargo test --locked -p commandf-pkg registry::tests -- --skip real_primary_us_core_is_direct_gzip --skip real_secondary_us_core_follows_only_expected_tarball + + - name: Focused archive bound tests + run: cargo test --locked -p commandf-pkg archive::tests + + - name: Real primary US Core archive response + shell: bash + run: | + set -euo pipefail + retry() { + local attempt=1 + until "$@"; do + if (( attempt >= 3 )); then + echo "real primary registry probe failed after ${attempt} attempts" >&2 + return 1 + fi + echo "real primary registry probe failed on attempt ${attempt}; retrying" >&2 + sleep $((attempt * 5)) + attempt=$((attempt + 1)) + done + } + retry cargo test --locked -p commandf-pkg registry::tests::real_primary_us_core_is_direct_gzip -- --ignored --exact + + - name: Real secondary redirect-to-tarball response + shell: bash + run: | + set -euo pipefail + retry() { + local attempt=1 + until "$@"; do + if (( attempt >= 3 )); then + echo "real secondary registry probe failed after ${attempt} attempts" >&2 + return 1 + fi + echo "real secondary registry probe failed on attempt ${attempt}; retrying" >&2 + sleep $((attempt * 5)) + attempt=$((attempt + 1)) + done + } + retry cargo test --locked -p commandf-pkg registry::tests::real_secondary_us_core_follows_only_expected_tarball -- --ignored --exact + + - name: End-to-end exact VSAC fallback resolve and verify + shell: bash + run: | + set -euo pipefail + for attempt in 1 2 3; do + rm -rf /tmp/commandf-registry-smoke + if cargo run --locked --quiet -p commandf -- \ + pkg resolve us.nlm.vsac@0.24.0 \ + --cache /tmp/commandf-registry-smoke/cache \ + --lock /tmp/commandf-registry-smoke/commandf.lock \ + && cargo run --locked --quiet -p commandf -- \ + pkg verify \ + --cache /tmp/commandf-registry-smoke/cache \ + --lock /tmp/commandf-registry-smoke/commandf.lock \ + && python - <<'PY' + import json + from pathlib import Path + + lock = json.loads(Path('/tmp/commandf-registry-smoke/commandf.lock').read_text()) + matches = [ + item for item in lock['packages'] + if item['name'] == 'us.nlm.vsac' and item['version'] == '0.24.0' + ] + assert len(matches) == 1 + package = matches[0] + assert len(package['sha256']) == 64 + assert package['source'] == 'https://packages2.fhir.org/web/us.nlm.vsac-0.24.0.tgz' + PY + then + exit 0 + fi + if (( attempt >= 3 )); then + echo "VSAC fallback resolve/verify failed after ${attempt} attempts" >&2 + exit 1 + fi + echo "VSAC fallback resolve/verify failed on attempt ${attempt}; retrying" >&2 + sleep $((attempt * 5)) + done diff --git a/crates/commandf-pkg/src/archive.rs b/crates/commandf-pkg/src/archive.rs index f1127060..c360cadc 100644 --- a/crates/commandf-pkg/src/archive.rs +++ b/crates/commandf-pkg/src/archive.rs @@ -7,7 +7,13 @@ use tar::Archive; use crate::{model::PackageManifest, PackageError}; const MAX_MANIFEST_BYTES: u64 = 1024 * 1024; -const MAX_ARCHIVE_DECOMPRESSED_BYTES: u64 = 512 * 1024 * 1024; +const MIN_MANIFEST_SCAN_DECOMPRESSED_BYTES: u64 = 512 * 1024 * 1024; +// The official us.nlm.vsac@0.24.0 fallback tarball was measured at 78,238,082 +// compressed bytes and requires 905,712,128 decompressed bytes to reach +// package/package.json. A 12x budget gives that immutable version ~33 MiB of +// headroom, while this 896 MiB absolute cap keeps worst-case work below 1 GiB. +const MAX_MANIFEST_SCAN_DECOMPRESSED_BYTES: u64 = 896 * 1024 * 1024; +const MANIFEST_SCAN_EXPANSION_RATIO: u64 = 12; const MAX_ARCHIVE_ENTRIES: usize = 50_000; struct BoundedReader { @@ -52,7 +58,21 @@ impl Read for BoundedReader { } pub(crate) fn read_manifest(bytes: &[u8]) -> Result { - read_manifest_with_limits(bytes, MAX_ARCHIVE_DECOMPRESSED_BYTES, MAX_ARCHIVE_ENTRIES) + read_manifest_with_limits( + bytes, + manifest_scan_decompressed_limit(bytes.len()), + MAX_ARCHIVE_ENTRIES, + ) +} + +fn manifest_scan_decompressed_limit(compressed_bytes: usize) -> u64 { + let compressed_bytes = u64::try_from(compressed_bytes).unwrap_or(u64::MAX); + compressed_bytes + .saturating_mul(MANIFEST_SCAN_EXPANSION_RATIO) + .clamp( + MIN_MANIFEST_SCAN_DECOMPRESSED_BYTES, + MAX_MANIFEST_SCAN_DECOMPRESSED_BYTES, + ) } fn read_manifest_with_limits( @@ -120,6 +140,29 @@ mod tests { encoder.finish().unwrap() } + #[test] + fn manifest_scan_budget_preserves_floor_scales_and_caps() { + assert_eq!( + manifest_scan_decompressed_limit(1), + MIN_MANIFEST_SCAN_DECOMPRESSED_BYTES + ); + assert_eq!( + manifest_scan_decompressed_limit(40 * 1024 * 1024), + MIN_MANIFEST_SCAN_DECOMPRESSED_BYTES + ); + assert_eq!( + manifest_scan_decompressed_limit(50 * 1024 * 1024), + 600 * 1024 * 1024 + ); + let vsac_budget = manifest_scan_decompressed_limit(78_238_082); + assert_eq!(vsac_budget, 938_856_984); + assert!(vsac_budget > 905_712_128); + assert_eq!( + manifest_scan_decompressed_limit(usize::MAX), + MAX_MANIFEST_SCAN_DECOMPRESSED_BYTES + ); + } + #[test] fn rejects_excessive_entry_count_before_manifest() { let bytes = archive_with_entries(&[("one", b""), ("two", b""), ("three", b"")]); diff --git a/crates/commandf-pkg/src/registry.rs b/crates/commandf-pkg/src/registry.rs index 212ab569..781cb41c 100644 --- a/crates/commandf-pkg/src/registry.rs +++ b/crates/commandf-pkg/src/registry.rs @@ -1,5 +1,5 @@ use std::collections::BTreeMap; -use std::time::Duration; +use std::time::{Duration, Instant}; use semver::Version; use serde::Deserialize; @@ -9,9 +9,11 @@ use crate::{PackageArchive, PackageError, PackageName, PackageSource}; const PRIMARY: &str = "https://packages.fhir.org"; const SECONDARY: &str = "https://packages2.fhir.org/packages"; +const SECONDARY_TARBALL_BASE: &str = "https://packages2.fhir.org/web"; const METADATA_LIMIT: u64 = 4 * 1024 * 1024; const ARCHIVE_LIMIT: u64 = 128 * 1024 * 1024; const REQUEST_TIMEOUT: Duration = Duration::from_secs(30); +const GZIP_MAGIC: [u8; 2] = [0x1f, 0x8b]; #[derive(Debug, Deserialize)] struct RegistryMetadata { @@ -26,12 +28,8 @@ pub struct FhirRegistrySource { impl Default for FhirRegistrySource { fn default() -> Self { - let config = Agent::config_builder() - .timeout_global(Some(REQUEST_TIMEOUT)) - .max_redirects(0) - .build(); Self { - agent: config.into(), + agent: agent_with_timeout(REQUEST_TIMEOUT), } } } @@ -71,22 +69,123 @@ impl FhirRegistrySource { name: &PackageName, version: &Version, ) -> Result { + let started = Instant::now(); let url = format!("{endpoint}/{name}/{version}"); let mut response = self .agent .get(&url) .call() .map_err(|error| error.to_string())?; - let bytes = response - .body_mut() - .with_config() - .limit(ARCHIVE_LIMIT) - .read_to_vec() - .map_err(|error| error.to_string())?; + + let status = response.status().as_u16(); + if (300..400).contains(&status) { + let location = response + .headers() + .get("location") + .and_then(|value| value.to_str().ok()) + .ok_or_else(|| { + format!("registry redirect from {url} omitted a valid Location header") + })?; + let target = validated_secondary_redirect(endpoint, status, name, version, location)?; + let remaining = remaining_request_timeout(started.elapsed())?; + return direct_archive_from_url(&target, remaining); + } + + if !(200..300).contains(&status) { + return Err(format!( + "registry download from {url} returned HTTP {status}" + )); + } + + let bytes = read_archive_body(&mut response, &url)?; + validate_gzip_archive(&bytes, &url)?; Ok(PackageArchive { bytes, source: url }) } } +fn agent_with_timeout(timeout: Duration) -> Agent { + Agent::config_builder() + .timeout_global(Some(timeout)) + .max_redirects(0) + .build() + .into() +} + +fn remaining_request_timeout(elapsed: Duration) -> Result { + REQUEST_TIMEOUT + .checked_sub(elapsed) + .filter(|remaining| !remaining.is_zero()) + .ok_or_else(|| { + format!( + "registry secondary redirect exhausted the {} second acquisition timeout", + REQUEST_TIMEOUT.as_secs() + ) + }) +} + +fn direct_archive_from_url(url: &str, timeout: Duration) -> Result { + let agent = agent_with_timeout(timeout); + let mut response = agent.get(url).call().map_err(|error| error.to_string())?; + let status = response.status().as_u16(); + if !(200..300).contains(&status) { + return Err(format!( + "registry tarball download from {url} returned HTTP {status}; redirects are not followed recursively" + )); + } + let bytes = read_archive_body(&mut response, url)?; + validate_gzip_archive(&bytes, url)?; + Ok(PackageArchive { + bytes, + source: url.to_owned(), + }) +} + +fn read_archive_body( + response: &mut ureq::http::Response, + url: &str, +) -> Result, String> { + response + .body_mut() + .with_config() + .limit(ARCHIVE_LIMIT) + .read_to_vec() + .map_err(|error| format!("registry archive body from {url} failed: {error}")) +} + +fn validate_gzip_archive(bytes: &[u8], url: &str) -> Result<(), String> { + if !bytes.starts_with(&GZIP_MAGIC) { + return Err(format!( + "registry response from {url} is not a gzip package archive" + )); + } + Ok(()) +} + +fn expected_secondary_tarball(name: &PackageName, version: &Version) -> String { + format!("{SECONDARY_TARBALL_BASE}/{name}-{version}.tgz") +} + +fn validated_secondary_redirect( + endpoint: &str, + status: u16, + name: &PackageName, + version: &Version, + location: &str, +) -> Result { + if endpoint != SECONDARY || status != 302 { + return Err(format!( + "unexpected registry redirect for {name}@{version}: endpoint={endpoint} status={status}" + )); + } + let expected = expected_secondary_tarball(name, version); + if location != expected { + return Err(format!( + "unexpected secondary registry redirect for {name}@{version}: expected {expected}, found {location}" + )); + } + Ok(expected) +} + impl PackageSource for FhirRegistrySource { fn source_id(&self) -> String { "fhir-package-registry".to_owned() @@ -141,3 +240,114 @@ impl PackageSource for FhirRegistrySource { ))) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn package() -> PackageName { + PackageName::parse("hl7.fhir.us.core").unwrap() + } + + fn version() -> Version { + Version::parse("8.0.1").unwrap() + } + + #[test] + fn accepts_expected_secondary_redirect_only() { + let name = package(); + let version = version(); + let expected = "https://packages2.fhir.org/web/hl7.fhir.us.core-8.0.1.tgz"; + assert_eq!( + validated_secondary_redirect(SECONDARY, 302, &name, &version, expected).unwrap(), + expected + ); + } + + #[test] + fn rejects_redirect_from_primary_endpoint() { + let error = validated_secondary_redirect( + PRIMARY, + 302, + &package(), + &version(), + "https://packages2.fhir.org/web/hl7.fhir.us.core-8.0.1.tgz", + ) + .unwrap_err(); + assert!(error.contains("unexpected registry redirect")); + } + + #[test] + fn rejects_unexpected_secondary_redirect_target() { + let error = validated_secondary_redirect( + SECONDARY, + 302, + &package(), + &version(), + "https://example.invalid/hl7.fhir.us.core-8.0.1.tgz", + ) + .unwrap_err(); + assert!(error.contains("unexpected secondary registry redirect")); + } + + #[test] + fn rejects_non_302_secondary_redirect() { + let error = validated_secondary_redirect( + SECONDARY, + 307, + &package(), + &version(), + "https://packages2.fhir.org/web/hl7.fhir.us.core-8.0.1.tgz", + ) + .unwrap_err(); + assert!(error.contains("unexpected registry redirect")); + } + + #[test] + fn rejects_non_gzip_registry_body() { + let error = validate_gzip_archive(b"Found. Redirecting", "https://example.test/package") + .unwrap_err(); + assert!(error.contains("not a gzip package archive")); + } + + #[test] + fn accepts_gzip_magic() { + validate_gzip_archive(&[0x1f, 0x8b, 0x08, 0x00], "https://example.test/package").unwrap(); + } + + #[test] + fn secondary_redirect_reuses_remaining_timeout_budget() { + assert_eq!( + remaining_request_timeout(Duration::from_secs(29)).unwrap(), + Duration::from_secs(1) + ); + assert!(remaining_request_timeout(REQUEST_TIMEOUT).is_err()); + assert!(remaining_request_timeout(REQUEST_TIMEOUT + Duration::from_millis(1)).is_err()); + } + + #[test] + #[ignore = "requires public packages.fhir.org network access"] + fn real_primary_us_core_is_direct_gzip() { + let archive = FhirRegistrySource::new() + .archive_from(PRIMARY, &package(), &version()) + .unwrap(); + assert!(archive.bytes.starts_with(&GZIP_MAGIC)); + assert_eq!( + archive.source, + "https://packages.fhir.org/hl7.fhir.us.core/8.0.1" + ); + } + + #[test] + #[ignore = "requires public packages2.fhir.org network access"] + fn real_secondary_us_core_follows_only_expected_tarball() { + let archive = FhirRegistrySource::new() + .archive_from(SECONDARY, &package(), &version()) + .unwrap(); + assert!(archive.bytes.starts_with(&GZIP_MAGIC)); + assert_eq!( + archive.source, + "https://packages2.fhir.org/web/hl7.fhir.us.core-8.0.1.tgz" + ); + } +}