diff --git a/.github/actions/detect-change-scope/ci-scope-paths.sh b/.github/actions/detect-change-scope/ci-scope-paths.sh index 0a970c797..d48e320f7 100644 --- a/.github/actions/detect-change-scope/ci-scope-paths.sh +++ b/.github/actions/detect-change-scope/ci-scope-paths.sh @@ -14,7 +14,7 @@ package_archive_changed_files() { -e '^sdk/(activate\.sh|config\.site|package(-lock)?\.json|tsconfig\.json)$' \ -e '^sdk/(bin|kandelo|src)/' \ -e '^tools/xtask/Cargo\.toml$' \ - -e '^tools/xtask/src/(archive_stage|archive_stage_cli|build_deps|host_tool_probe|main|pkg_manifest|source_extract|util)\.rs$' \ + -e '^tools/xtask/src/(archive_stage|archive_stage_cli|build_deps|host_tool_probe|main|package_archive_name|pkg_manifest|source_extract|util)\.rs$' \ -e '^tools/mkrootfs/(bin|src)/' \ -e '^tools/mkrootfs/(package(-lock)?\.json|tsconfig\.json)$' \ -e '^crates/fork-instrument/(Cargo\.toml|src/)' \ @@ -49,7 +49,7 @@ package_publish_flow_changed_files() { -e '^\.github/actions/detect-change-scope/(ci-scope-paths|test-ci-scope-paths)\.sh$' \ -e '^\.github/workflows/(staging-build|prepare-merge|activate-merge-candidate|recover-rejected-merge-candidate|staging-cleanup|force-rebuild|reusable-package-source-publish)\.yml$' \ -e '^\.github/scripts/(activate-merge-candidate|cleanup-merge-candidates|clone-rejected-merge-candidate|compose-staging-release-snapshots|download-verified-release-asset|fetch-canonical-index|github-api-get|init-merge-candidate|latest-merge-gate-status|mark-merge-candidate-ready|reconcile-merge-candidates|recover-canonical-indexes|require-exact-head-approval|select-package-archive-source|state-lock|test-activate-merge-candidate|test-cleanup-merge-candidates|test-clone-rejected-merge-candidate|test-download-verified-release-asset|test-fetch-canonical-index|test-init-merge-candidate|test-latest-merge-gate-status|test-merge-candidate-workflows|test-reconcile-merge-candidates|test-recover-canonical-indexes|test-require-exact-head-approval|test-select-package-archive-source|test-state-lock|test-validate-staging-release|test-verify-merge-candidate|validate-staging-release|verify-merge-candidate)\.sh$' \ - -e '^tools/xtask/src/(build_index|bundle_program|index_candidate|index_toml|index_update|staging_reuse|update_pkg_manifest)\.rs$' \ + -e '^tools/xtask/src/(build_index|bundle_program|index_candidate|index_toml|index_update|package_archive_name|staging_reuse|update_pkg_manifest)\.rs$' \ -e '^scripts/(compose-initial-index|index-has-current-entry|index-update|prepare-sdk-package|publish-package-source|release-index-state|sync-package-source)\.sh$' \ -e '^tests/scripts/(index-update|package-publish-flow|release-index-state)\.sh$' \ || true diff --git a/.github/actions/detect-change-scope/test-ci-scope-paths.sh b/.github/actions/detect-change-scope/test-ci-scope-paths.sh index fae7ea959..7de352324 100755 --- a/.github/actions/detect-change-scope/test-ci-scope-paths.sh +++ b/.github/actions/detect-change-scope/test-ci-scope-paths.sh @@ -40,6 +40,9 @@ assert_not_matches() { assert_matches package_archive_changed_files \ "tools/xtask/src/build_deps.rs" \ "tools/xtask/src/build_deps.rs" +assert_matches package_archive_changed_files \ + "tools/xtask/src/package_archive_name.rs" \ + "tools/xtask/src/package_archive_name.rs" assert_not_matches package_archive_changed_files \ "tools/xtask/src/remote_fetch.rs" \ "tools/xtask/src/remote_fetch.rs" @@ -105,6 +108,9 @@ assert_matches package_publish_flow_changed_files \ assert_matches package_publish_flow_changed_files \ "tools/xtask/src/index_candidate.rs" \ "tools/xtask/src/index_candidate.rs" +assert_matches package_publish_flow_changed_files \ + "tools/xtask/src/package_archive_name.rs" \ + "tools/xtask/src/package_archive_name.rs" assert_matches package_publish_flow_changed_files \ "tests/scripts/index-update.sh" \ "tests/scripts/index-update.sh" diff --git a/docs/binary-releases.md b/docs/binary-releases.md index 639dbe9da..b3f53c73a 100644 --- a/docs/binary-releases.md +++ b/docs/binary-releases.md @@ -392,6 +392,15 @@ sha for that manifest. Two archives with the same `(name, version, revision, arch)` but different transitive deps get distinct shas and thus distinct names. +The filename is a transport label, not a parseable identity record: +package names and versions may both contain `-`, so the boundary between +them is ambiguous in the string alone. Index composition reads `name`, +`version`, `revision`, architecture, ABI compatibility, and cache key from +the archive's validated `manifest.toml`, then requires the filename to equal +the canonical string reconstructed from those fields. Archive creation and +index recovery call the same renderer so the producer and validator cannot +drift to different filename grammars. + ### Archive interior layout Each `.tar.zst` carries exactly two top-level entries: @@ -637,10 +646,16 @@ checklist. When migrating a release from the legacy schema (or recovering after a corrupted index), `scripts/compose-initial-index.sh ` downloads every `.tar.zst` from the release, -extracts each archive's internal `manifest.toml` to recover -`cache_key_sha` + `build_timestamp`, and uploads a freshly composed -`index.toml`. The script acquires the same state-lock as the -matrix-build path, so it serializes against any active CI rebuilds. +extracts each archive's internal `manifest.toml` as the authoritative +package identity and compatibility record, verifies that the archive's +filename is the canonical rendering of those fields, and uploads a freshly +composed `index.toml`. The script acquires the same state-lock as the +matrix-build path, so it serializes against any active CI rebuilds. It does +not infer the package name or version by splitting the archive filename. If +the downloaded inventory contains more than one archive for the same package +name and target architecture, composition fails and reports both filenames, +cache keys, and archive hashes. Recovery must select one explicit immutable +archive rather than depend on directory traversal order. Day-to-day publishes don't use this script — they go through `scripts/index-update.sh` per-matrix-entry. compose-initial-index is diff --git a/scripts/compose-initial-index.sh b/scripts/compose-initial-index.sh index 104b60366..a426ebee8 100755 --- a/scripts/compose-initial-index.sh +++ b/scripts/compose-initial-index.sh @@ -9,9 +9,10 @@ # # 1. Downloads every *.tar.zst archive from the release. # 2. Runs `xtask build-index` against the downloaded set, which -# decompresses each archive, parses its internal manifest.toml's -# [compatibility] block, and emits a Success entry per -# (package, arch). +# decompresses each archive, uses its internal manifest.toml as +# the package identity, verifies the canonical transport filename, +# rejects ambiguous duplicate (package, arch) archives, and emits a +# Success entry per unique identity. # 3. Publishes the composed index through the crash-recoverable canonical # release-index transaction. # diff --git a/tools/xtask/src/archive_stage_cli.rs b/tools/xtask/src/archive_stage_cli.rs index a74f0804f..fe3771334 100644 --- a/tools/xtask/src/archive_stage_cli.rs +++ b/tools/xtask/src/archive_stage_cli.rs @@ -122,7 +122,7 @@ pub fn run(args: Vec) -> Result<(), String> { fs::create_dir_all(&parsed.out_dir) .map_err(|e| format!("mkdir {}: {e}", parsed.out_dir.display()))?; - // Filename convention (single source of truth for archive naming): + // Filename convention (rendered by package_archive_name): // --rev-abi--.tar.zst // The `` suffix is the first 8 hex chars of the cache_key // sha so a freshly-published archive is content-addressable from @@ -190,11 +190,9 @@ pub fn run(args: Vec) -> Result<(), String> { } /// Compute the canonical archive filename + path for a (manifest, arch, -/// abi) triple under `out_dir`. The shape (`--rev- -/// abi--.tar.zst`) is parsed by `build_index` to -/// recover `(name, version, revision, abi, arch, short_sha)` when -/// regenerating `index.toml`, so the formatter and parser MUST stay -/// aligned. +/// abi) triple under `out_dir`. `build_index` recovers identity from the +/// archive's structured manifest and validates the transport filename with +/// the same renderer; it never reverses an ambiguous `-` string. fn archive_path_for_sha( out_dir: &Path, manifest: &DepsManifest, @@ -202,17 +200,9 @@ fn archive_path_for_sha( abi: u32, sha_hex: &str, ) -> PathBuf { - let short = &sha_hex[..8]; - let archive_name = format!( - "{}-{}-rev{}-abi{}-{}-{}.tar.zst", - manifest.name, - manifest.version, - manifest.revision, - abi, - arch.as_str(), - short, - ); - out_dir.join(archive_name) + out_dir.join(crate::package_archive_name::render( + manifest, arch, abi, sha_hex, + )) } /// Compute the cache-key sha for a manifest as a 64-char lowercase hex @@ -626,6 +616,34 @@ index_url = "file:///tmp/wpk-nonexistent-binaries-abi-v{{abi}}/index.toml" assert!(short .chars() .all(|c| c.is_ascii_hexdigit() && !c.is_uppercase())); + + let index_path = dir.join("index.toml"); + crate::build_index::run(vec![ + "--abi".into(), + shared::ABI_VERSION.to_string(), + "--generator".into(), + "archive-stage integration test".into(), + "--archives-dir".into(), + out_dir.display().to_string(), + "--out".into(), + index_path.display().to_string(), + "--generated-at".into(), + "2026-05-05T00:00:00Z".into(), + ]) + .expect("build-index must accept archive-stage's canonical filename"); + let index = crate::index_toml::IndexToml::parse( + &fs::read_to_string(index_path).expect("read composed index"), + ) + .expect("parse composed index"); + assert_eq!(index.packages.len(), 1); + assert_eq!(index.packages[0].name, "z"); + assert_eq!(index.packages[0].version, "1.0.0"); + assert_eq!( + index.packages[0].binary[&TargetArch::Wasm32] + .archive_url + .as_deref(), + Some(name.as_str()) + ); } #[test] diff --git a/tools/xtask/src/build_index.rs b/tools/xtask/src/build_index.rs index 5be4700f3..0d8e59a3b 100644 --- a/tools/xtask/src/build_index.rs +++ b/tools/xtask/src/build_index.rs @@ -15,13 +15,13 @@ //! `xtask index-update` (per-package atomic updates under the //! state-lock), not this command. //! -//! Filename convention (must match what `archive-stage` writes): -//! `--rev-abi--.tar.zst`. -//! -//! The seed flow extracts each archive's internal `manifest.toml` -//! to recover `cache_key_sha` + `build_timestamp` (from the -//! `[compatibility]` block) and stamps them into the per-entry -//! fields the resolver requires. +//! The seed flow extracts each archive's internal `manifest.toml` as +//! the authoritative package identity and compatibility record. It +//! verifies that the transport filename exactly matches the canonical +//! name `archive-stage` would produce from those structured fields, +//! then stamps the manifest data into the per-entry fields the resolver +//! requires. Package names and versions can both contain `-`, so they +//! must never be inferred by splitting the filename. use std::collections::BTreeMap; use std::fs; @@ -30,7 +30,7 @@ use std::path::{Path, PathBuf}; use sha2::{Digest, Sha256}; use crate::index_toml::IndexToml; -use crate::pkg_manifest::TargetArch; +use crate::pkg_manifest::{DepsManifest, TargetArch}; use crate::util::hex; /// Parsed CLI args. @@ -46,33 +46,11 @@ struct Args { generated_at: Option, } -/// Components extracted from an archive filename. -#[derive(Clone, Debug)] -struct ParsedArchive { - /// Package name (`mariadb`). - name: String, - /// Upstream version (`10.5.27`). - version: String, - /// Recipe revision (`1`). Same package + version + revision in two - /// arches must agree on `version`/`name`/`revision`; we don't carry - /// `revision` into `index.toml` because the consumer keys on - /// (name, arch, sha) — but we validate consistency below as a - /// defense against accidentally publishing two archives that claim - /// the same `(name, arch)` from different recipe revisions. - revision: u32, - /// ABI generation embedded in the filename. Cross-checked against - /// `--abi` so a mistakenly-mixed batch (e.g. abi5 + abi6 archives - /// in the same dir) fails loud rather than silently producing an - /// incoherent manifest. - abi: u32, - arch: TargetArch, - /// 8-char hex slot from the filename (cache_key_sha prefix). Not - /// emitted into index.toml — included here so error messages can - /// reference the specific archive that failed validation. - // Keep this modeled so filename parsing validates the full archive - // naming schema, even though production code does not read it yet. - #[allow(dead_code)] - short_sha: String, +/// One archive whose structured identity has been recovered from its +/// internal `manifest.toml` and checked against its transport filename. +struct CollectedArchive { + manifest: DepsManifest, + archive_sha256: String, /// Bare filename (relative archive_url). Mirror-friendly per the /// design doc's URL semantics: a self-contained source directory /// (manifest + archives) is bit-identically mirrorable to any @@ -84,8 +62,8 @@ struct ParsedArchive { /// /// Required flags (order-independent, both `--flag value` and /// `--flag=value` accepted): -/// --abi Cross-checked against each archive's -/// `abi` filename slot; mismatch → error. +/// --abi Cross-checked against each archived +/// manifest and canonical filename. /// --generator Free-form provenance line, e.g. /// `"kandelo CI @ "`. /// --archives-dir Directory holding the `.tar.zst` archives. @@ -111,39 +89,47 @@ pub fn run(args: Vec) -> Result<(), String> { let mut pkg_revision: BTreeMap = BTreeMap::new(); let mut pkg_version: BTreeMap = BTreeMap::new(); - for (parsed_archive, archive_sha_hex, meta) in entries { - if let Some(prev) = pkg_version.get(&parsed_archive.name) { - if prev != &parsed_archive.version { + for archive in entries { + let manifest = &archive.manifest; + let compatibility = manifest + .compatibility + .as_ref() + .expect("collect_archives only returns archived manifests"); + if let Some(prev) = pkg_version.get(&manifest.name) { + if prev != &manifest.version { return Err(format!( "package {:?}: archive {:?} declares version {:?}, but a sibling \ arch already declared {:?} — every arch of a package must agree on version", - parsed_archive.name, parsed_archive.filename, parsed_archive.version, prev, + manifest.name, archive.filename, manifest.version, prev, )); } } else { - pkg_version.insert(parsed_archive.name.clone(), parsed_archive.version.clone()); + pkg_version.insert(manifest.name.clone(), manifest.version.clone()); } - if let Some(prev) = pkg_revision.get(&parsed_archive.name) { - if prev != &parsed_archive.revision { + if let Some(prev) = pkg_revision.get(&manifest.name) { + if prev != &manifest.revision { return Err(format!( "package {:?}: archive {:?} declares revision {}, but a sibling \ arch already declared revision {} — every arch must agree on revision", - parsed_archive.name, parsed_archive.filename, parsed_archive.revision, prev, + manifest.name, archive.filename, manifest.revision, prev, )); } } else { - pkg_revision.insert(parsed_archive.name.clone(), parsed_archive.revision); + pkg_revision.insert(manifest.name.clone(), manifest.revision); } idx.update_entry_success( - &parsed_archive.name, - &parsed_archive.version, - parsed_archive.revision, - parsed_archive.arch, - parsed_archive.filename.clone(), - archive_sha_hex, - meta.cache_key_sha, - meta.build_timestamp.unwrap_or_else(|| generated_at.clone()), + &manifest.name, + &manifest.version, + manifest.revision, + compatibility.target_arch, + archive.filename, + archive.archive_sha256, + compatibility.cache_key_sha.clone(), + compatibility + .build_timestamp + .clone() + .unwrap_or_else(|| generated_at.clone()), parsed.generator.clone(), ); } @@ -156,25 +142,11 @@ pub fn run(args: Vec) -> Result<(), String> { Ok(()) } -/// Metadata extracted from a `.tar.zst`'s internal `manifest.toml`'s -/// `[compatibility]` block. The fields are recipe-stable (the same -/// archive produces the same metadata across decompressions), so -/// re-running `build-index` over the same archives produces a -/// byte-identical `index.toml`. -struct ArchiveMetadata { - cache_key_sha: String, - /// `compatibility.build_timestamp` if the archive recorded one - /// (Phase A-bis onward). Older archives may have None; the - /// caller falls back to `generated_at` so the entry still has - /// a `built_at` value. - build_timestamp: Option, -} - /// Decompress + un-tar an archive in memory, find the /// `manifest.toml` entry, parse it through -/// `DepsManifest::parse_archived`, and return the -/// `[compatibility]` fields the ledger needs. -fn read_archive_metadata(bytes: &[u8]) -> Result { +/// `DepsManifest::parse_archived`, and return the validated structured +/// manifest that owns the archive's package identity. +fn read_archive_manifest(bytes: &[u8]) -> Result { let decoder = zstd::stream::read::Decoder::new(bytes).map_err(|e| format!("zstd decode: {e}"))?; let mut tar = tar::Archive::new(decoder); @@ -191,38 +163,27 @@ fn read_archive_metadata(bytes: &[u8]) -> Result { entry .read_to_string(&mut text) .map_err(|e| format!("read manifest.toml: {e}"))?; - let archived = crate::pkg_manifest::DepsManifest::parse_archived( - &text, - std::path::PathBuf::from("/dev/null"), - )?; - let compat = archived - .compatibility - .as_ref() - .ok_or_else(|| "archived manifest missing [compatibility]".to_string())?; - return Ok(ArchiveMetadata { - cache_key_sha: compat.cache_key_sha.clone(), - build_timestamp: compat.build_timestamp.clone(), - }); + return DepsManifest::parse_archived(&text, PathBuf::from("/dev/null")); } } Err("archive missing manifest.toml at the root".into()) } -/// Walk `archives_dir` for `*.tar.zst` files, parse each filename, and -/// compute its sha256. Returns the parsed entries paired with their -/// computed `archive_sha256`. Sorted by (name, arch) for deterministic +/// Walk `archives_dir` for `*.tar.zst` files, recover each identity from +/// its internal manifest, validate its canonical filename, and compute +/// its sha256. Sorted by (manifest name, target arch) for deterministic /// output downstream. fn collect_archives( archives_dir: &Path, expected_abi: u32, -) -> Result, String> { +) -> Result, String> { if !archives_dir.is_dir() { return Err(format!( "archives-dir {} is not a directory or does not exist", archives_dir.display() )); } - let mut out: Vec<(ParsedArchive, String, ArchiveMetadata)> = Vec::new(); + let mut out: Vec = Vec::new(); for dirent in fs::read_dir(archives_dir) .map_err(|e| format!("read_dir {}: {e}", archives_dir.display()))? { @@ -238,117 +199,112 @@ fn collect_archives( if !name.ends_with(".tar.zst") { continue; } - let parsed = parse_archive_filename(&name)?; - if parsed.abi != expected_abi { - return Err(format!( - "archive {name}: filename declares abi{} but --abi is {}", - parsed.abi, expected_abi - )); - } let bytes = fs::read(&path).map_err(|e| format!("read {}: {e}", path.display()))?; let mut hasher = Sha256::new(); hasher.update(&bytes); let sha = hex(&Into::<[u8; 32]>::into(hasher.finalize())); - let meta = read_archive_metadata(&bytes).map_err(|e| { + let manifest = read_archive_manifest(&bytes).map_err(|e| { format!( "archive {name}: {e}. The seed flow needs each .tar.zst's internal \ - manifest.toml to recover cache_key_sha + build_timestamp." + manifest.toml to recover its package identity and compatibility." ) })?; - out.push((parsed, sha, meta)); + let compatibility = manifest + .compatibility + .as_ref() + .expect("parse_archived requires compatibility"); + if !compatibility.abi_versions.contains(&expected_abi) { + return Err(format!( + "archive {name}: manifest abi_versions {:?} does not include --abi {}", + compatibility.abi_versions, expected_abi + )); + } + let expected_name = canonical_archive_filename(&manifest, expected_abi); + if name != expected_name { + return Err(format!( + "archive {name:?}: filename does not match manifest identity \ + {}@{} revision {} for {} at ABI {}; expected {expected_name:?}", + manifest.name, + manifest.version, + manifest.revision, + compatibility.target_arch.as_str(), + expected_abi, + )); + } + out.push(CollectedArchive { + manifest, + archive_sha256: sha, + filename: name, + }); } - // Deterministic enumeration order: by (name, arch). Same set of - // archives → same index.toml regardless of dirent traversal order. + // Deterministic enumeration order: by (name, arch, filename). Same set + // of archives → same result regardless of dirent traversal order, including + // the order in which a duplicate-identity error names the two archives. out.sort_by(|a, b| { - a.0.name - .cmp(&b.0.name) - .then_with(|| a.0.arch.as_str().cmp(b.0.arch.as_str())) + a.manifest.name.cmp(&b.manifest.name).then_with(|| { + archive_target_arch(a) + .cmp(&archive_target_arch(b)) + .then_with(|| a.filename.cmp(&b.filename)) + }) }); - Ok(out) -} - -/// Parse `--rev-abi--.tar.zst`. -/// Lenient on `` and `` (each can contain `-`); rigorous -/// on the trailing 4 segments which have a fixed shape. -fn parse_archive_filename(name: &str) -> Result { - let stem = name - .strip_suffix(".tar.zst") - .ok_or_else(|| format!("filename {name:?} does not have .tar.zst suffix"))?; - let parts: Vec<&str> = stem.split('-').collect(); - // Need at least: name, version, rev, abi, arch, short → 6 parts. - if parts.len() < 6 { - return Err(format!( - "filename {name:?} has too few `-`-separated segments (need at least \ - 6: --rev-abi--)" - )); - } - let short = *parts.last().unwrap(); - if !is_short_sha(short) { - return Err(format!( - "filename {name:?}: trailing segment {short:?} must be 8 lowercase hex chars" - )); - } - let arch_seg = parts[parts.len() - 2]; - let arch = match arch_seg { - "wasm32" => TargetArch::Wasm32, - "wasm64" => TargetArch::Wasm64, - other => { + for archives in out.windows(2) { + let first = &archives[0]; + let second = &archives[1]; + if first.manifest.name == second.manifest.name + && archive_target_arch(first) == archive_target_arch(second) + { return Err(format!( - "filename {name:?}: arch segment {other:?} must be wasm32 or wasm64" + "duplicate archives for package {:?} target {}: {:?} \ + (cache_key_sha {}, archive_sha256 {}) and {:?} \ + (cache_key_sha {}, archive_sha256 {}); index recovery \ + cannot choose between immutable archives", + first.manifest.name, + archive_target_arch(first).as_str(), + first.filename, + archive_cache_key(first), + first.archive_sha256, + second.filename, + archive_cache_key(second), + second.archive_sha256, )); } - }; - let abi_seg = parts[parts.len() - 3]; - let abi: u32 = abi_seg - .strip_prefix("abi") - .and_then(|s| s.parse().ok()) - .ok_or_else(|| { - format!("filename {name:?}: 3rd-from-last segment {abi_seg:?} must be `abi`") - })?; - let rev_seg = parts[parts.len() - 4]; - let revision: u32 = rev_seg - .strip_prefix("rev") - .and_then(|s| s.parse().ok()) - .ok_or_else(|| { - format!("filename {name:?}: 4th-from-last segment {rev_seg:?} must be `rev`") - })?; - // Everything before rev is `-`. Find the boundary: - // version starts at the segment immediately after the package name's - // last segment. We can't tell where `` ends and `` - // begins by looking at the string alone (both can contain `-`); we - // adopt the convention that the LAST `-` before `rev` separates - // them. That matches every package in the registry today - // (mariadb-10.5.27, php-8.4.5, ncurses-6.5, ...). - let pre_rev = &parts[..parts.len() - 4]; - if pre_rev.len() < 2 { - return Err(format!( - "filename {name:?}: need at least one segment each for and \ - before rev{revision}" - )); } - let version = pre_rev[pre_rev.len() - 1].to_string(); - let pkg_name = pre_rev[..pre_rev.len() - 1].join("-"); - if pkg_name.is_empty() { - return Err(format!("filename {name:?}: empty package name")); - } - if version.is_empty() { - return Err(format!("filename {name:?}: empty version")); - } - Ok(ParsedArchive { - name: pkg_name, - version, - revision, - abi, - arch, - short_sha: short.to_string(), - filename: name.to_string(), - }) + Ok(out) +} + +fn archive_target_arch(archive: &CollectedArchive) -> TargetArch { + archive + .manifest + .compatibility + .as_ref() + .expect("parse_archived requires compatibility") + .target_arch } -fn is_short_sha(s: &str) -> bool { - s.len() == 8 - && s.bytes() - .all(|b| b.is_ascii_hexdigit() && !b.is_ascii_uppercase()) +fn archive_cache_key(archive: &CollectedArchive) -> &str { + &archive + .manifest + .compatibility + .as_ref() + .expect("parse_archived requires compatibility") + .cache_key_sha +} + +/// Reconstruct the transport filename from the archive's validated +/// structured identity. This must match `archive_stage_cli`'s producer +/// format, but is intentionally one-way: no identity field is recovered +/// by splitting this ambiguous string. +fn canonical_archive_filename(manifest: &DepsManifest, abi: u32) -> String { + let compatibility = manifest + .compatibility + .as_ref() + .expect("parse_archived requires compatibility"); + crate::package_archive_name::render( + manifest, + compatibility.target_arch, + abi, + &compatibility.cache_key_sha, + ) } /// Hand-rolled CLI parser. Mirrors the shape of `archive_stage_cli`'s @@ -505,7 +461,7 @@ mod tests { /// Build a real .tar.zst archive matching `name@version-rev@abi-arch` /// with a manifest.toml carrying [compatibility]. Returns the /// path. The internal manifest content is shaped to satisfy - /// `read_archive_metadata` (DepsManifest::parse_archived). + /// `read_archive_manifest` (DepsManifest::parse_archived). fn write_real_archive( dir: &Path, name: &str, @@ -513,7 +469,6 @@ mod tests { rev: u32, abi: u32, arch: &str, - short: &str, cache_key_sha: &str, ) -> PathBuf { let manifest_text = format!( @@ -543,7 +498,19 @@ build_timestamp = "2026-05-05T12:34:56Z" ); let bytes = crate::remote_fetch::build_test_archive(&manifest_text, &[("lib/out.a", b"PAYLOAD")]); - let fname = format!("{name}-{version}-rev{rev}-abi{abi}-{arch}-{short}.tar.zst"); + let manifest = DepsManifest::parse_archived(&manifest_text, PathBuf::from("/dev/null")) + .expect("test archive manifest must parse"); + let target_arch = manifest + .compatibility + .as_ref() + .expect("test archive manifest must have compatibility") + .target_arch; + let fname = crate::package_archive_name::render( + &manifest, + target_arch, + abi, + cache_key_sha, + ); let path = dir.join(&fname); fs::write(&path, &bytes).unwrap(); path @@ -553,6 +520,12 @@ build_timestamp = "2026-05-05T12:34:56Z" fs::read_to_string(out_path).unwrap() } + fn file_sha256(path: &Path) -> String { + let mut hasher = Sha256::new(); + hasher.update(fs::read(path).unwrap()); + hex(&Into::<[u8; 32]>::into(hasher.finalize())) + } + /// Smoke: 2 packages × 2 arches → all 4 entries with status=success, /// cache_key_sha + built_at + built_by populated. #[test] @@ -569,7 +542,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 1, 6, "wasm32", - "11111111", &"a".repeat(64), ); write_real_archive( @@ -579,7 +551,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 1, 6, "wasm64", - "22222222", &"b".repeat(64), ); write_real_archive( @@ -589,7 +560,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 1, 6, "wasm32", - "33333333", &"c".repeat(64), ); write_real_archive( @@ -599,7 +569,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 1, 6, "wasm64", - "44444444", &"d".repeat(64), ); @@ -641,7 +610,7 @@ build_timestamp = "2026-05-05T12:34:56Z" // status + relative archive_url + cache_key_sha + built_*. assert_eq!(text.matches("status = \"success\"").count(), 4); assert!( - text.contains("archive_url = \"alpha-1.0.0-rev1-abi6-wasm32-11111111.tar.zst\""), + text.contains("archive_url = \"alpha-1.0.0-rev1-abi6-wasm32-aaaaaaaa.tar.zst\""), "got:\n{text}" ); assert!( @@ -709,7 +678,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 1, 6, "wasm32", - "aaaaaaaa", &"e".repeat(64), ); @@ -755,7 +723,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 1, 6, "wasm32", - "11111111", &"a".repeat(64), ); write_real_archive( @@ -765,7 +732,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 1, 6, "wasm64", - "22222222", &"b".repeat(64), ); write_real_archive( @@ -775,7 +741,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 7, 6, "wasm32", - "33333333", &"c".repeat(64), ); @@ -809,18 +774,7 @@ build_timestamp = "2026-05-05T12:34:56Z" ); } - /// Duplicate (name, arch) → error. Catches a mistake where two - /// archives with the same package + arch but different shas land in - /// the same dir. - /// - /// IndexToml::update_entry_success is idempotent on a single - /// (name, version, arch) key (the second call overwrites the first), - /// so an earlier `[binary]`-block writer was the one rejecting - /// duplicates. We don't reject anymore — the build pipeline ensures - /// uniqueness via the cache_key_sha-suffixed filename — but we DO - /// reject when two archives have the same (name, arch) AND - /// different revisions or versions, because that's a real bug. - /// Test the divergent-version + divergent-revision paths. + /// A package's architecture slices must agree on version and revision. #[test] fn divergent_version_across_arches_is_rejected() { let dir = tempdir("divergent-ver"); @@ -835,7 +789,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 1, 6, "wasm32", - "11111111", &"a".repeat(64), ); write_real_archive( @@ -845,7 +798,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 1, 6, "wasm64", - "22222222", &"b".repeat(64), ); @@ -863,19 +815,14 @@ build_timestamp = "2026-05-05T12:34:56Z" assert!(err.contains("version"), "got: {err}"); } - /// `--abi` mismatch with the filename's `abi` slot → error. - /// The filename-vs-CLI check fires before archive bytes are - /// touched, so a stub archive's contents are sufficient here. + /// `--abi` mismatch with the structured compatibility record → error. #[test] - fn abi_mismatch_in_filename_is_rejected() { + fn abi_mismatch_in_manifest_is_rejected() { let dir = tempdir("abi-mismatch"); let archives = dir.join("archives"); fs::create_dir_all(&archives).unwrap(); let out = dir.join("index.toml"); - // Real archive so collect_archives gets past the read step - // and into the abi-check (which happens before metadata - // extraction). write_real_archive( &archives, "x", @@ -883,7 +830,6 @@ build_timestamp = "2026-05-05T12:34:56Z" 1, 5, "wasm32", - "11111111", &"a".repeat(64), ); @@ -899,48 +845,151 @@ build_timestamp = "2026-05-05T12:34:56Z" ]) .expect_err("abi mismatch must error"); assert!( - err.contains("abi5") && err.contains("--abi is 6"), + err.contains("manifest abi_versions [5]") && err.contains("--abi 6"), "got: {err}" ); } - /// Filename parser: rejects bad shapes with messages that name the - /// failing segment. #[test] - fn filename_parser_rejects_malformed_inputs() { - // Missing .tar.zst suffix. - let err = parse_archive_filename("foo.tar.gz").unwrap_err(); - assert!(err.contains(".tar.zst"), "got: {err}"); - - // Too few segments. - let err = parse_archive_filename("a-b-c.tar.zst").unwrap_err(); - assert!(err.contains("too few"), "got: {err}"); - - // Bad short_sha (uppercase). - let err = parse_archive_filename("x-1.0.0-rev1-abi6-wasm32-AAAAAAAA.tar.zst").unwrap_err(); - assert!(err.contains("8 lowercase hex"), "got: {err}"); - - // Bad arch. - let err = parse_archive_filename("x-1.0.0-rev1-abi6-armv7-aaaaaaaa.tar.zst").unwrap_err(); - assert!(err.contains("wasm32 or wasm64"), "got: {err}"); - - // Bad abi prefix. - let err = parse_archive_filename("x-1.0.0-rev1-foo6-wasm32-aaaaaaaa.tar.zst").unwrap_err(); - assert!(err.contains("abi"), "got: {err}"); + fn manifest_identity_handles_hyphens_in_package_name_and_version() { + let dir = tempdir("hyphenated-identity"); + let archives = dir.join("archives"); + fs::create_dir_all(&archives).unwrap(); + let out = dir.join("index.toml"); + + let cache_key_sha = "a".repeat(64); + let archive = write_real_archive( + &archives, + "spidermonkey-node", + "140.11.0esr-node.1", + 4, + 39, + "wasm32", + &cache_key_sha, + ); + + super::run(vec![ + "--abi".into(), + "39".into(), + "--generator".into(), + "test".into(), + "--archives-dir".into(), + archives.display().to_string(), + "--out".into(), + out.display().to_string(), + "--generated-at".into(), + "2026-07-14T00:00:00Z".into(), + ]) + .unwrap(); + + let index = crate::index_toml::IndexToml::parse(&read_index(&out)).unwrap(); + assert_eq!(index.packages.len(), 1); + assert_eq!(index.packages[0].name, "spidermonkey-node"); + assert_eq!(index.packages[0].version, "140.11.0esr-node.1"); + assert_eq!(index.packages[0].revision, 4); + assert_eq!( + index.packages[0].binary[&TargetArch::Wasm32] + .archive_url + .as_deref(), + archive.file_name().and_then(|name| name.to_str()) + ); + } + + #[test] + fn duplicate_manifest_identity_is_rejected_with_both_archive_identities() { + let dir = tempdir("duplicate-manifest-identity"); + let archives = dir.join("archives"); + fs::create_dir_all(&archives).unwrap(); + let out = dir.join("index.toml"); + + let first_key = + "0f5290453e6ea7f68e5ee1e50bd6dbf23221368e7aeb7a54c34953cef453920d"; + let second_key = + "a88651d0cd72a9100a67c90fa4b5600659258b10890c852ff10ab125cf770212"; + let first = write_real_archive( + &archives, + "spidermonkey-node", + "140.11.0esr-node.1", + 4, + 39, + "wasm32", + first_key, + ); + let second = write_real_archive( + &archives, + "spidermonkey-node", + "140.11.0esr-node.1", + 4, + 39, + "wasm32", + second_key, + ); + let first_sha = file_sha256(&first); + let second_sha = file_sha256(&second); + + let err = super::run(vec![ + "--abi".into(), + "39".into(), + "--generator".into(), + "test".into(), + "--archives-dir".into(), + archives.display().to_string(), + "--out".into(), + out.display().to_string(), + ]) + .expect_err("duplicate package/arch archives must not overwrite one another"); + + assert!(err.contains("duplicate archives"), "got: {err}"); + assert!(err.contains("spidermonkey-node"), "got: {err}"); + assert!(err.contains("wasm32"), "got: {err}"); + for value in [ + first.file_name().unwrap().to_str().unwrap(), + first_key, + first_sha.as_str(), + second.file_name().unwrap().to_str().unwrap(), + second_key, + second_sha.as_str(), + ] { + assert!(err.contains(value), "missing {value:?} from: {err}"); + } + assert!(!out.exists(), "a rejected inventory must not write an index"); } - /// Multi-segment package name (e.g. `mariadb-test-10.5.27`) must - /// parse correctly: the LAST `-` before `rev` is the - /// name/version boundary. #[test] - fn filename_parser_handles_multi_segment_names() { - let p = parse_archive_filename("mariadb-test-10.5.27-rev3-abi6-wasm32-abc12345.tar.zst") - .unwrap(); - assert_eq!(p.name, "mariadb-test"); - assert_eq!(p.version, "10.5.27"); - assert_eq!(p.revision, 3); - assert_eq!(p.abi, 6); - assert_eq!(p.arch, TargetArch::Wasm32); - assert_eq!(p.short_sha, "abc12345"); + fn filename_cannot_override_manifest_identity() { + let dir = tempdir("filename-identity-override"); + let archives = dir.join("archives"); + fs::create_dir_all(&archives).unwrap(); + let out = dir.join("index.toml"); + + let cache_key_sha = "b".repeat(64); + let archive = write_real_archive( + &archives, + "trusted-package", + "1.0-beta-1", + 2, + 39, + "wasm32", + &cache_key_sha, + ); + let misleading_name = + "attacker-trusted-package-1.0-beta-1-rev2-abi39-wasm32-bbbbbbbb.tar.zst"; + fs::rename(&archive, archives.join(misleading_name)).unwrap(); + + let err = super::run(vec![ + "--abi".into(), + "39".into(), + "--generator".into(), + "test".into(), + "--archives-dir".into(), + archives.display().to_string(), + "--out".into(), + out.display().to_string(), + ]) + .expect_err("the embedded manifest must own package identity"); + + assert!(err.contains("manifest identity"), "got: {err}"); + assert!(err.contains(misleading_name), "got: {err}"); + assert!(err.contains("trusted-package-1.0-beta-1"), "got: {err}"); } } diff --git a/tools/xtask/src/main.rs b/tools/xtask/src/main.rs index ef924b99a..73ecb5952 100644 --- a/tools/xtask/src/main.rs +++ b/tools/xtask/src/main.rs @@ -61,6 +61,7 @@ mod host_tool_probe; mod index_candidate; mod index_toml; mod index_update; +mod package_archive_name; mod package_matrix; mod pkg_manifest; mod remote_fetch; diff --git a/tools/xtask/src/package_archive_name.rs b/tools/xtask/src/package_archive_name.rs new file mode 100644 index 000000000..871e63d3c --- /dev/null +++ b/tools/xtask/src/package_archive_name.rs @@ -0,0 +1,27 @@ +//! Canonical transport naming for Kandelo package archives. +//! +//! Package names and versions may both contain `-`, so the rendered name is +//! intentionally one-way. Consumers recover identity from `manifest.toml` and +//! use this renderer only to verify that the transport label is canonical. + +use crate::pkg_manifest::{DepsManifest, TargetArch}; + +pub(crate) fn render( + manifest: &DepsManifest, + arch: TargetArch, + abi: u32, + cache_key_sha: &str, +) -> String { + let short = cache_key_sha + .get(..8) + .expect("package archive cache key must be validated before rendering"); + format!( + "{}-{}-rev{}-abi{}-{}-{}.tar.zst", + manifest.name, + manifest.version, + manifest.revision, + abi, + arch.as_str(), + short, + ) +}