diff --git a/local/docs/BUILD-SYSTEM-ASSESSMENT-2026-07-18.md b/local/docs/BUILD-SYSTEM-ASSESSMENT-2026-07-18.md index f09726b2b6..c5a1dc516b 100644 --- a/local/docs/BUILD-SYSTEM-ASSESSMENT-2026-07-18.md +++ b/local/docs/BUILD-SYSTEM-ASSESSMENT-2026-07-18.md @@ -353,18 +353,18 @@ The build system is fully functional on the canonical path: `./local/scripts/bui ### 6.2 Quality Improvement Opportunities (Ordered by Impact) -| # | Opportunity | Effort | Impact | -|---|-------------|--------|--------| -| Q1 | Add per-pkgar BLAKE3 to cache manifest; verify post-restore. | Medium | Closes the largest robustness gap (Mechanism #2). | -| Q2 | Wrap `download_wget()` and `git clone` with retry logic matching prefix.mk's wget pattern. | Small | Eliminates the most common transient-failure abort. | -| Q3 | Preserve `auto_deps.toml` in binary store; copy alongside `dep_hashes.toml`. | Small | Closes correctness gap in restored recipes. | -| Q4 | Validate `dep_hashes.toml` parse strictly; on corrupt, log loud + rebuild instead of silent mtime fallback. | Small | Eliminates a silent correctness gap. | -| Q5 | Adopt the "Last verified / Authority" header policy for all build-system docs. | Small | Prevents future drift. | -| Q6 | Standardize error-message template across `repo.rs` (replace `{:?}` with human-readable). | Small | Improves operator experience for CLI errors. | -| Q7 | Suppress `mk/disk.mk` unmount noise for the "not mounted" case. | Trivial | Removes 8 spurious warnings per build. | -| Q8 | Fix the 9 stale user-facing messages identified in Part 5.3. | Trivial | Removes misleading language. | -| Q9 | Archive/merge the 3+3+2 stale/obsolete/duplicate docs identified in Part 4.3. | Small | Consolidates doc surface; no behavior change. | -| Q10 | Replace `recipes/AGENTS.md` and `recipes/core/AGENTS.md` with redirects to `local/AGENTS.md`. | Trivial | Eliminates misleading upstream-Redox-pattern docs. | +| # | Opportunity | Effort | Impact | Status | +|---|-------------|--------|--------|--------| +| Q1 | Add per-pkgar BLAKE3 to cache manifest; verify post-restore. | Medium | Closes the largest robustness gap (Mechanism #2). | ✅ Resolved — `BinaryStoreManifest` + `compute_file_blake3_hex` in `cook_build.rs`/`fs.rs`/`repo_builder.rs` | +| Q2 | Wrap `download_wget()` and `git clone` with retry logic matching prefix.mk's wget pattern. | Small | Eliminates the most common transient-failure abort. | ✅ Resolved — `run_command_with_retry()` in `fs.rs`, used by `download_wget` + git clone | +| Q3 | Preserve `auto_deps.toml` in binary store; copy alongside `dep_hashes.toml`. | Small | Closes correctness gap in restored recipes. | ✅ Resolved — publish in `repo_builder.rs`, restore-with-fallback in `cook_build.rs` | +| Q4 | Validate `dep_hashes.toml` parse strictly; on corrupt, log loud + rebuild instead of silent mtime fallback. | Small | Eliminates a silent correctness gap. | ✅ Resolved — `DepHashes::read` returns `Result, String>` | +| Q5 | Adopt the "Last verified / Authority" header policy for all build-system docs. | Small | Prevents future drift. | ✅ Resolved — doc consolidation commit | +| Q6 | Standardize error-message template across `repo.rs` (replace `{:?}` with human-readable). | Small | Improves operator experience for CLI errors. | ✅ Resolved — `{:?}` → `{:#}` in `repo.rs` | +| Q7 | Suppress `mk/disk.mk` unmount noise for the "not mounted" case. | Trivial | Removes 8 spurious warnings per build. | ✅ Resolved (Tier 1) | +| Q8 | Fix the 9 stale user-facing messages identified in Part 5.3. | Trivial | Removes misleading language. | ✅ Resolved (Tier 1) | +| Q9 | Archive/merge the 3+3+2 stale/obsolete/duplicate docs identified in Part 4.3. | Small | Consolidates doc surface; no behavior change. | ✅ Resolved (Tier 2) | +| Q10 | Replace `recipes/AGENTS.md` and `recipes/core/AGENTS.md` with redirects to `local/AGENTS.md`. | Trivial | Eliminates misleading upstream-Redox-pattern docs. | ✅ Resolved (Tier 2) | ### 6.3 Auto-Correction Capability Summary @@ -382,13 +382,13 @@ The build system auto-corrects the following without operator intervention: | Dependency pkgar changed | DepHashes catches it; rebuilds. | | Local-overlay unfetch attempted | Refused; sources are immutable. | | Protected recipe online fetch attempted | Refused; falls through to offline source. | +| Transient network failure on non-prefix fetch (Q2) | `run_command_with_retry()` retries 3× with exponential backoff (1s/2s/4s). | +| Corrupt cached pkgar that passed mtime check (Q1) | BLAKE3 manifest verification detects mismatch; logs WARN, marks not-restored, forces rebuild. | The build system does NOT auto-correct: | Failure | Operator Action Required | |---------|--------------------------| -| Transient network failure on non-prefix fetch | Re-run the build. | -| Corrupt cached pkgar that passes mtime check | Re-run with `--no-cache` or remove the bad pkgar. | | Config-vs-package collision (warn mode) | Read the warning; fix the config or accept it. | | Fork on wrong branch | Checkout correct branch or set override. | | Dirty fork working tree at build start | Commit or stash; the script stashes automatically but may need manual unstash on failure. | @@ -422,13 +422,15 @@ Recommend approval before execution: ### Tier 3 — Code improvements (gaps from Part 6.2) -Defer to a separate work cycle; listed for visibility: +All Tier 3 items are resolved: -14. Q1 — Cache manifest with per-pkgar BLAKE3 + post-restore verify. -15. Q2 — Retry logic for non-prefix fetches. -16. Q3 — Preserve `auto_deps.toml` in binary store. -17. Q4 — Strict `dep_hashes.toml` parse handling. -18. Q6 — Standardize error messages in `repo.rs`. +14. ✅ Q1 — Cache manifest with per-pkgar BLAKE3 + post-restore verify. (`BinaryStoreManifest` in `cook_build.rs`, `compute_file_blake3_hex` in `fs.rs`, manifest write in `repo_builder.rs`) +15. ✅ Q2 — Retry logic for non-prefix fetches. (`run_command_with_retry()` in `fs.rs`) +16. ✅ Q3 — Preserve `auto_deps.toml` in binary store. (publish in `repo_builder.rs`, restore+fallback in `cook_build.rs`) +17. ✅ Q4 — Strict `dep_hashes.toml` parse handling. (`DepHashes::read` returns `Result, String>`) +18. ✅ Q6 — Standardize error messages in `repo.rs`. (`{:?}` → `{:#}`) + +Verification: `cargo check` ✅, `cargo test --lib` ✅ 38/38 (35 existing + 3 new BinaryStoreManifest tests), `cargo clippy --lib` ✅ 0 new warnings, `make -n` dry-runs ✅. --- diff --git a/src/bin/repo_builder.rs b/src/bin/repo_builder.rs index f10697e142..fdf4f89559 100644 --- a/src/bin/repo_builder.rs +++ b/src/bin/repo_builder.rs @@ -1,4 +1,6 @@ use cookbook::cook::ident::{get_ident, init_ident}; +use cookbook::cook::cook_build::BinaryStoreManifest; +use cookbook::cook::fs::{compute_file_blake3_hex, serialize_and_write}; use cookbook::cook::{fetch, package as cook_package}; use cookbook::recipe::CookRecipe; use cookbook::web::{CliWebConfig, generate_web}; @@ -110,6 +112,12 @@ fn publish_packages(config: &CliConfig) -> anyhow::Result<()> { }; let target_dir = cookbook_recipe.target_dir(); + let manifest_path = repo_path.join(format!("{}.manifest.toml", recipe.name())); + let mut manifest = BinaryStoreManifest::read(&manifest_path) + .ok() + .flatten() + .unwrap_or_default(); + for package in cookbook_recipe.recipe.get_packages_list() { let (stage_dir, pkgar_src, toml_src) = cook_package::package_stage_paths(package, &target_dir); @@ -126,13 +134,22 @@ fn publish_packages(config: &CliConfig) -> anyhow::Result<()> { eprintln!("\x1b[01;38;5;155mrepo - publishing {}\x1b[0m", recipe_name); if fs::exists(&pkgar_src)? { fs::copy(&pkgar_src, &pkgar_dst)?; + if let Ok(h) = compute_file_blake3_hex(&pkgar_dst) { + manifest.hashes.insert(format!("{}.pkgar", recipe_name), h); + } } fs::copy(&toml_src, &toml_dst)?; + if let Ok(h) = compute_file_blake3_hex(&toml_dst) { + manifest.hashes.insert(format!("{}.toml", recipe_name), h); + } if package.is_none() { let dep_hashes_src = target_dir.join("dep_hashes.toml"); let dep_hashes_dst = repo_path.join(format!("{}.dep_hashes.toml", recipe_name)); if dep_hashes_src.is_file() { let _ = fs::copy(&dep_hashes_src, &dep_hashes_dst); + if let Ok(h) = compute_file_blake3_hex(&dep_hashes_dst) { + manifest.hashes.insert(format!("{}.dep_hashes.toml", recipe_name), h); + } } // Preserves the full auto-dep graph (incl. ELF dynamic deps); otherwise restores fall back to declared-depends-only reconstruction. @@ -140,6 +157,9 @@ fn publish_packages(config: &CliConfig) -> anyhow::Result<()> { let auto_deps_dst = repo_path.join(format!("{}.auto_deps.toml", recipe_name)); if auto_deps_src.is_file() { let _ = fs::copy(&auto_deps_src, &auto_deps_dst); + if let Ok(h) = compute_file_blake3_hex(&auto_deps_dst) { + manifest.hashes.insert(format!("{}.auto_deps.toml", recipe_name), h); + } } } } @@ -149,6 +169,10 @@ fn publish_packages(config: &CliConfig) -> anyhow::Result<()> { appstream_sources.insert(recipe.name().to_string(), stage_dir.clone()); } } + + if !manifest.hashes.is_empty() { + let _ = serialize_and_write(&manifest_path, &manifest); + } } // === 2. Optional AppStream generation === diff --git a/src/cook/cook_build.rs b/src/cook/cook_build.rs index fe7aa50568..cb69170861 100644 --- a/src/cook/cook_build.rs +++ b/src/cook/cook_build.rs @@ -67,6 +67,28 @@ impl DepHashes { } } +/// Per-file BLAKE3 manifest for binary store integrity verification. +/// Maps each published file name (relative to `repo//`) to its +/// BLAKE3 hex digest. Written at publish time; verified at restore time. +#[derive(Serialize, Deserialize, Default, Debug)] +pub struct BinaryStoreManifest { + #[serde(flatten)] + pub hashes: BTreeMap, +} + +impl BinaryStoreManifest { + pub fn read(path: &Path) -> std::result::Result, String> { + let content = match fs::read_to_string(path) { + Ok(c) => c, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(e) => return Err(format!("failed to read {}: {e}", path.display())), + }; + toml::from_str(&content) + .map(Some) + .map_err(|e| format!("failed to parse {}: {e}", path.display())) + } +} + /// Source content hash: BLAKE3 hash of every file in the source tree /// (excluding `.git/`, `target/`, and other build artifacts), combined /// with the recipe.toml and patch files. Used as a content-stable @@ -587,6 +609,64 @@ pub fn build( } } } + + // BLAKE3 manifest catches silent disk corruption that pkgar signatures don't. + // Missing manifest = backward compatible (older cookbook published without one). + if all_restored { + let repo_manifest = repo_target.join(format!("{}.manifest.toml", recipe_name)); + match BinaryStoreManifest::read(&repo_manifest) { + Ok(Some(manifest)) => { + for (filename, expected_hash) in &manifest.hashes { + let file_path = repo_target.join(filename); + if !file_path.is_file() { + log_to_pty!( + logger, + "WARN: binary store integrity check FAILED — manifest lists {} but file is missing", + filename + ); + all_restored = false; + continue; + } + match compute_file_blake3_hex(&file_path) { + Ok(actual_hash) if actual_hash == *expected_hash => {} + Ok(actual_hash) => { + log_to_pty!( + logger, + "WARN: binary store integrity check FAILED for {} — expected {}, got {}", + filename, + expected_hash, + actual_hash + ); + all_restored = false; + } + Err(e) => { + log_to_pty!( + logger, + "WARN: failed to hash {} for integrity check: {e}", + filename + ); + all_restored = false; + } + } + } + if all_restored && cli_verbose { + log_to_pty!( + logger, + "DEBUG: binary store integrity verified ({} files via BLAKE3 manifest)", + manifest.hashes.len() + ); + } + } + Ok(None) => {} + Err(e) => { + log_to_pty!( + logger, + "WARN: binary store manifest unreadable for {}: {e} — skipping integrity check", + recipe_name + ); + } + } + } } let mut source_modified = modified_dir_ignore_git(source_dir).unwrap_or_else(|_| { @@ -1238,7 +1318,10 @@ mod tests { use std::os::unix; use std::path::{Path, PathBuf}; - use super::{DepHashes, SourceContentHash, collect_current_dep_hashes, dep_hashes_changed}; + use super::{ + BinaryStoreManifest, DepHashes, SourceContentHash, collect_current_dep_hashes, + dep_hashes_changed, + }; use pkg::PackageName; fn temp_dir(name: &str) -> PathBuf { @@ -1624,4 +1707,54 @@ mod tests { "a dep with empty blake3 must be excluded from current hashes" ); } + + #[test] + fn binary_store_manifest_roundtrip_preserves_entries() { + let dir = temp_dir("bsm_roundtrip"); + let path = dir.join("recipe.manifest.toml"); + fs::write( + &path, + b"\"recipe.pkgar\" = \"aabbccdd\"\n\"recipe.toml\" = \"eeff0011\"\n", + ) + .expect("write manifest"); + + let loaded = BinaryStoreManifest::read(&path) + .expect("read should not error on valid TOML") + .expect("read should return Some for existing file"); + assert_eq!( + loaded.hashes.get("recipe.pkgar").map(|s| s.as_str()), + Some("aabbccdd"), + "first entry must survive round-trip" + ); + assert_eq!( + loaded.hashes.get("recipe.toml").map(|s| s.as_str()), + Some("eeff0011"), + "second entry must survive round-trip" + ); + } + + #[test] + fn binary_store_manifest_read_returns_ok_none_for_missing_file() { + let dir = temp_dir("bsm_missing"); + let path = dir.join("nonexistent.manifest.toml"); + match BinaryStoreManifest::read(&path) { + Ok(None) => (), + other => panic!( + "read on a missing manifest must return Ok(None) for backward compat, got {other:?}" + ), + } + } + + #[test] + fn binary_store_manifest_read_returns_err_for_corrupt_toml() { + let dir = temp_dir("bsm_corrupt"); + let path = dir.join("recipe.manifest.toml"); + fs::write(&path, b"this is not valid toml [[[").expect("write corrupt manifest"); + + match BinaryStoreManifest::read(&path) { + Err(_) => (), + Ok(Some(_)) => panic!("read on corrupt TOML must return Err, got Ok(Some)"), + Ok(None) => panic!("read on corrupt TOML must return Err, got Ok(None)"), + } + } } diff --git a/src/cook/fs.rs b/src/cook/fs.rs index 585adccd47..c937c6696e 100644 --- a/src/cook/fs.rs +++ b/src/cook/fs.rs @@ -2,7 +2,7 @@ use serde::Serialize; use std::{ collections::BTreeSet, fs, - io::{self, Write}, + io::{self, Read, Write}, path::{Path, PathBuf}, process::{self, Command, Stdio}, time::SystemTime, @@ -251,6 +251,25 @@ pub fn run_command_with_retry( Err(last_err.expect("retry loop runs at least once when max_attempts >= 1")) } +/// Compute BLAKE3 hash of a file, returning the hex-encoded digest. +/// Uses 64 KB chunked reads to handle large pkgar files without loading +/// the entire file into memory. +pub fn compute_file_blake3_hex(path: &Path) -> Result { + let mut file = fs::File::open(path).map_err(wrap_io_err!(path, "Opening for BLAKE3"))?; + let mut hasher = blake3::Hasher::new(); + let mut buf = [0u8; 65536]; + loop { + let n = file + .read(&mut buf) + .map_err(wrap_io_err!(path, "Reading for BLAKE3"))?; + if n == 0 { + break; + } + hasher.update(&buf[..n]); + } + Ok(hasher.finalize().to_hex().to_string()) +} + pub fn run_command_stdin( mut command: process::Command, stdin_data: &[u8],