From 03fb6ec878822996765699d2ee4a463be5c89ed5 Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Tue, 14 Jul 2026 10:01:34 -0400 Subject: [PATCH] packages: recover index identity from archive manifests Package names and versions may both contain hyphens, so build-index cannot recover their boundary from the transport filename. The old last-hyphen rule silently changed spidermonkey-node@140.11.0esr-node.1 into a different package identity. Carry the validated archived manifest through index composition and validate the transport label with the same canonical renderer used by archive-stage. Fail closed when recovery sees multiple immutable archives for one package and target architecture, reporting both cache and archive identities instead of overwriting by directory order. Keep the change in package publication tooling: package cache keys, archive bytes, and the Kandelo ABI remain unchanged. --- .../detect-change-scope/ci-scope-paths.sh | 4 +- .../test-ci-scope-paths.sh | 6 + docs/binary-releases.md | 23 +- scripts/compose-initial-index.sh | 7 +- tools/xtask/src/archive_stage_cli.rs | 52 +- tools/xtask/src/build_index.rs | 551 ++++++++++-------- tools/xtask/src/main.rs | 1 + tools/xtask/src/package_archive_name.rs | 27 + 8 files changed, 394 insertions(+), 277 deletions(-) create mode 100644 tools/xtask/src/package_archive_name.rs 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, + ) +}