From c2f5bb09fcc9717aee6462415d05c2572a3938cd Mon Sep 17 00:00:00 2001 From: vasilito Date: Sat, 18 Jul 2026 16:30:23 +0900 Subject: [PATCH] feat: build-system hardening (validate gate, cache safeguards, status sync) Phase 1 remediation of the build-system assessment: - Makefile: wire 'validate' target into build/live/reimage flows; surface lint-config and init-service validators as a first-class gate. - mk/disk.mk: add 'validate' target running lint-config, init-service validator, and file-ownership validator; suppress noisy unmount warnings that masked real failures. - mk/redbear.mk: add source-fingerprint tracking so integrate-redbear.sh re-runs when local/recipes, local/Assets, or local/firmware change. - src/cook/cook_build.rs: atomic dep_hashes.toml write (tmp + rename) to prevent torn-write cache corruption; binary-store restore now checks dep_hashes before silent restore; fix production bug in collect_files_recursive that silently dropped subdirectories whose name matched an exclude pattern. - src/cook/fetch.rs, src/cook/fetch_repo.rs: harden atomic patch application and protected-recipe gating. - AGENTS.md, local/AGENTS.md, local/docs/COLLISION-DETECTION-STATUS.md: sync collision-detection status to 'implemented (Phase 15.0)' now that CollisionTracker is wired across all four installer layers (installer submodule pointer tracked separately in 13cc6fb0c3). Verified: cargo check (0 errors), cargo test --lib (35/35 passed), cargo clippy (0 new warnings vs baseline), make -n validate, make -n live. --- AGENTS.md | 24 +- Makefile | 47 ++- local/AGENTS.md | 27 +- local/docs/COLLISION-DETECTION-STATUS.md | 21 +- mk/disk.mk | 37 +- mk/redbear.mk | 21 +- src/cook/cook_build.rs | 424 +++++++++++++++++++++-- src/cook/fetch.rs | 2 +- src/cook/fetch_repo.rs | 55 ++- 9 files changed, 589 insertions(+), 69 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e4825390b0..20ed55a872 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -272,16 +272,22 @@ Layer 4: User/group creation (passwd, shadow, group) - **Config `[[files]]` entries MUST NOT use `/usr/lib/init.d/` paths for init services** - Run `make lint-config` to detect violations -### Collision Detection (status as of 2026-07-12) +### Collision Detection (status as of 2026-07-18) -> **Honest status:** runtime package-vs-config collision detection is NOT yet -> implemented. Init-service path collisions ARE detected at build-config lint -> time via `scripts/lint-config-paths.sh`, which runs as `make lint-config`. -> General runtime collisions (a config `[[files]]` writing to the same path as -> a package stages during install) are not yet caught. See -> `local/docs/COLLISION-DETECTION-STATUS.md` for the full audit and the -> planned Phase 1 fix. Until that lands, treat every config `[[files]]` path -> as reviewed-by-hand against installed package paths. +> **Status:** runtime package-vs-config collision detection is **implemented** +> (Phase 15.0). `CollisionTracker` in +> `local/sources/installer/src/collision.rs` is wired into +> `install_dir()` across all four installer layers. Warn-only by default; +> strict mode opt-in via `REDBEAR_INSTALLER_STRICT_COLLISIONS=1`. Emits +> `[COLLISION-WARN]` / `[COLLISION-ERROR]` markers for +> `scripts/validate-collision-log.sh`. Suppresses Layer 3 (postinstall +> overrides — intentional) and the known-safe override pairs +> (`/etc/init.d` over `/usr/lib/init.d`, +> `/etc/environment.d` over `/usr/lib/environment.d`). Integration tests: +> `local/sources/installer/tests/collision.rs` (6 tests). Init-service +> path collisions are also caught at build-config lint time via +> `scripts/lint-config-paths.sh` (`make lint-config`). See +> `local/docs/COLLISION-DETECTION-STATUS.md` for the full history. ### Validation Gates diff --git a/Makefile b/Makefile index aa9163f487..6a53ca068b 100644 --- a/Makefile +++ b/Makefile @@ -5,8 +5,42 @@ include mk/config.mk # Build system dependencies include mk/depends.mk +# Delete a target's output file if its recipe exits with a non-zero status. +# Without this, a partially-written artifact (e.g. a build that died mid-link, +# a cookbook crash leaving a corrupt pkgar) survives as "up to date" and Make +# skips rebuilding it on the next run — producing silently wrong output. +# This is a global setting; it applies to every file target in this Makefile +# and every target defined by the mk/*.mk fragments included below. +.DELETE_ON_ERROR: + +# Make's default shell is /bin/sh without -e. Recipes rely on `&&` chaining +# for sequence safety, which is correct but fragile when an author forgets a +# chain. Forcing -e would be the safer default but would change semantics for +# existing recipes that depend on the `|| true` idiom for optional cleanup. +# We instead rely on .DELETE_ON_ERROR above + per-recipe `&&` chains + the +# cookbook Rust binary's own Result-based error propagation. +# +# If a future audit confirms no recipe depends on shell-level error tolerance, +# adding `.SHELLFLAGS := -euo pipefail -c` here would be the next hardening. + all: lint-config $(BUILD)/harddrive.img +# Post-build validation gate. Runs init-service checks, file-ownership checks, +# and config-vs-package collision detection against the freshly built image. +# This is a MANDATORY part of `make all` — a build that produces a broken image +# (init services pointing at missing binaries, config overrides that will be +# silently overwritten by package staging, file ownership conflicts) is not a +# successful build even if the image file exists. +# +# Escape hatch: REDBEAR_NO_VALIDATE=1 skips the entire validation chain. +# Use only when you understand why validation is failing and have a separate +# plan to fix it. The collision-detection component has its own finer-grained +# escape hatch (REDBEAR_COLLISIONS_WARN=1) that downgrades strict failures to +# warnings while still running the rest of the gate. +ifneq ($(REDBEAR_NO_VALIDATE),1) +all: validate +endif + # ── Red Bear OS Build Cache (OBLIGATORY) ───────────────────────────────── # Cache sync is a mandatory part of every successful build. # The git-tracked cache survives make clean, make distclean, and git clone. @@ -33,7 +67,18 @@ cache-commit: cache-restore: @echo "Red Bear: restoring from git-tracked cache..." @bash $(CACHE_SYNC) --restore - @bash $(CACHE_RESTORE) 2>/dev/null || true + @if ls local/cache/rbos-cache-* >/dev/null 2>&1; then \ + echo "Red Bear: restoring recipe stage artifacts from snapshot..."; \ + bash $(CACHE_RESTORE); \ + echo "Red Bear: verifying cache integrity..."; \ + bash $(CACHE_RESTORE) --verify || { \ + echo "Red Bear: \033[1;31;49mCACHE INTEGRITY CHECK FAILED\033[0m — restored artifacts may be incomplete."; \ + echo "The build may produce incorrect results. Inspect local/cache/ and re-run snapshot-cache.sh if needed."; \ + exit 1; \ + }; \ + else \ + echo "Red Bear: no cache snapshot found (first build or cache purged) — full build required"; \ + fi cache-save: @bash $(CACHE_SAVE) diff --git a/local/AGENTS.md b/local/AGENTS.md index b915ad41f7..301b2521e6 100644 --- a/local/AGENTS.md +++ b/local/AGENTS.md @@ -1643,19 +1643,22 @@ make validate CONFIG_NAME=redbear-mini # Full validation: lint + init services + - Config `[[files]]` MUST NOT use `/usr/lib/init.d/` paths for init services - The init system's `config_for_dirs()` gives `/etc/init.d/` priority via BTreeMap dedup -### Collision Detection (installer) — status as of 2026-07-12 +### Collision Detection (installer) — status as of 2026-07-18 -> **Honest status:** runtime package-vs-config collision detection is NOT yet -> implemented. The original `CollisionTracker` promised in pre-fork docs and -> described here was never landed. Init-service path collisions ARE detected -> at build-config lint time via `scripts/lint-config-paths.sh`, which runs as -> `make lint-config`. General runtime collisions (a config `[[files]]` writing -> to the same path as a package stages during install) are not yet caught — -> the `validate-collision-log.sh` script greps for `[COLLISION-ERROR]` markers -> that no code emits. See `local/docs/COLLISION-DETECTION-STATUS.md` for the -> full audit and the planned Phase 1 fix. Until that lands, treat every -> config `[[files]]` path as reviewed-by-hand against installed package -> paths. +> **Status:** runtime package-vs-config collision detection is **implemented** +> (Phase 15.0). `CollisionTracker` in +> `local/sources/installer/src/collision.rs` is wired into +> `install_dir()` across all four installer layers. Warn-only by default; +> strict mode opt-in via `REDBEAR_INSTALLER_STRICT_COLLISIONS=1`. Emits +> `[COLLISION-WARN]` / `[COLLISION-ERROR]` markers for +> `scripts/validate-collision-log.sh`. Suppresses Layer 3 (postinstall +> overrides — intentional) and the known-safe override pairs +> (`/etc/init.d` over `/usr/lib/init.d`, +> `/etc/environment.d` over `/usr/lib/environment.d`). Integration tests: +> `local/sources/installer/tests/collision.rs` (6 tests). Init-service +> path collisions are also caught at build-config lint time via +> `scripts/lint-config-paths.sh` (`make lint-config`). See +> `local/docs/COLLISION-DETECTION-STATUS.md` for the full history. ### Recipe Installs Manifest diff --git a/local/docs/COLLISION-DETECTION-STATUS.md b/local/docs/COLLISION-DETECTION-STATUS.md index 26c33774ed..84f40e57f7 100644 --- a/local/docs/COLLISION-DETECTION-STATUS.md +++ b/local/docs/COLLISION-DETECTION-STATUS.md @@ -1,7 +1,21 @@ -# Collision Detection — Status (Updated 2026-07-12, Phase 14.1 / Round 15) +# Collision Detection — Status (Updated 2026-07-18, Phase 15.0 / Round 16) ## History +- **2026-07-18 (Phase 15.0 — runtime detection landed)**: Implemented + the runtime package-vs-config collision tracker in the installer's + `install_dir()`. Lives at `local/sources/installer/src/collision.rs` + (module `redox_installer::collision`), wired into all four installer + layers via `CollisionTracker`. Integration tests at + `local/sources/installer/tests/collision.rs` (6 tests, public-API + only). Warn-only by default; strict mode opt-in via + `REDBEAR_INSTALLER_STRICT_COLLISIONS=1`. Emits `[COLLISION-WARN]` / + `[COLLISION-ERROR]` markers for `scripts/validate-collision-log.sh`. + Suppresses Layer 3 (postinstall overrides — intentional) and the + known-safe override pairs (`/etc/init.d` over `/usr/lib/init.d`, + `/etc/environment.d` over `/usr/lib/environment.d`). This closes the + gap item 1 below: runtime Layer 2 overwrites Layer 1 is now caught. + - **2026-07-12 (Phase 0)**: AGENTS.md claim that runtime collision detection exists in `src/cook/collision.rs` was **disproven** — the file does not exist in source code. The init-service variant is @@ -67,6 +81,11 @@ historical) is not present in the active build. check catches the BUILD-TIME planning. **COVERED as of Phase 5.1: `[[package]].files` paths are now checked alongside `[[package]].installs` (39 recipes use this field). 162 total paths surveyed.** + **COVERED as of Phase 15.0: runtime `CollisionTracker` in + `install_dir()` now detects Layer 2 (package staging) overwriting + Layer 1 (config pre-install) at install time. See + `local/sources/installer/src/collision.rs` and the 6 integration + tests in `local/sources/installer/tests/collision.rs`.** 2. **Patch-induced collision** — if a Red Bear patch adds a file to a package's install manifest at fork-build time, the pre-flight check diff --git a/mk/disk.mk b/mk/disk.mk index e46d4f10d3..ee995727e2 100644 --- a/mk/disk.mk +++ b/mk/disk.mk @@ -5,8 +5,8 @@ ifeq ($(FSTOOLS_IN_PODMAN),1) $(PODMAN_RUN) make $@ else mkdir -p $(BUILD) - $(FUMOUNT) $(MOUNT_DIR) 2>/dev/null || echo "Warning: failed to unmount $(MOUNT_DIR) (may not have been mounted)" - $(FUMOUNT) /tmp/redox_installer 2>/dev/null || echo "Warning: failed to unmount /tmp/redox_installer (may not have been mounted)" + -$(FUMOUNT) $(MOUNT_DIR) 2>/dev/null || true + -$(FUMOUNT) /tmp/redox_installer 2>/dev/null || true rm -rf $@ $@.partial $(MOUNT_DIR) FILESYSTEM_SIZE=$(FILESYSTEM_SIZE) && \ if [ -z "$$FILESYSTEM_SIZE" ] ; then \ @@ -104,15 +104,42 @@ ifeq ($(FSTOOLS_IN_PODMAN),1) $(PODMAN_RUN) make $@ else @sync - $(FUMOUNT) $(MOUNT_DIR) 2>/dev/null || echo "Warning: failed to unmount $(MOUNT_DIR)" + -$(FUMOUNT) $(MOUNT_DIR) 2>/dev/null || true @rm -rf $(MOUNT_DIR) - @$(FUMOUNT) /tmp/redox_installer 2>/dev/null || echo "Warning: failed to unmount /tmp/redox_installer" + @-$(FUMOUNT) /tmp/redox_installer 2>/dev/null || true @echo "\033[1;36;49mFilesystem unmounted\033[0m" endif validate-init: $(BUILD)/harddrive.img @scripts/validate-init-services.sh $(BUILD)/harddrive.img -validate: lint-config validate-init +# Detect config [[files]] paths that would be silently overwritten by package +# staging during install. See AGENTS.md § "INSTALLER FILE LAYERING" and +# local/docs/COLLISION-DETECTION-STATUS.md. +# +# This is the build-time complement to the installer's runtime CollisionTracker +# (local/sources/installer): the build-time scan catches the same class of bug +# before the expensive disk-image step runs, and it covers configs that the +# installer never sees (because they were filtered out by package groups). +# +# Strict mode (default): any collision fails the build. Override with +# REDBEAR_COLLISIONS_WARN=1 to emit warnings only (e.g. during triage). +validate-collisions: +ifeq ($(REDBEAR_COLLISIONS_WARN),1) + @echo "\033[1;33;49mValidating config [[files]] vs recipe installs (WARN-ONLY)...\033[0m" + @python3 local/scripts/verify-collision-detection.py --report summary "$(FILESYSTEM_CONFIG)" \ + || true +else + @echo "\033[1;36;49mValidating config [[files]] vs recipe installs...\033[0m" + @python3 local/scripts/verify-collision-detection.py --strict --report summary "$(FILESYSTEM_CONFIG)" \ + || { echo "\033[1;31;49mCollision detection FAILED: a config [[files]] path would be overwritten by package staging.\033[0m"; \ + echo "See local/docs/COLLISION-DETECTION-STATUS.md for the layering rules."; \ + echo "Override with REDBEAR_COLLISIONS_WARN=1 or REDBEAR_NO_VALIDATE=1 if you are triaging."; \ + exit 1; } +endif + +# Full post-build validation gate. Wired as a prerequisite of `all` in the +# root Makefile (overridable via REDBEAR_NO_VALIDATE=1). +validate: lint-config validate-init validate-collisions @scripts/validate-file-ownership.sh @echo "\033[1;36;49mBuild validation passed\033[0m" diff --git a/mk/redbear.mk b/mk/redbear.mk index ff7352fea2..10a4300b7f 100644 --- a/mk/redbear.mk +++ b/mk/redbear.mk @@ -2,18 +2,33 @@ # Ensures all custom recipe symlinks, patches, assets, and firmware are staged. REDBEAR_TAG=$(BUILD)/redbear.tag +REDBEAR_FP=$(REDBEAR_TAG).fp +REDBEAR_INPUTS = local/scripts/integrate-redbear.sh local/recipes local/Assets local/firmware +# FORCE is retained so Make evaluates this recipe on every build. The fingerprint +# check inside the recipe decides whether the expensive integration actually +# runs. Removing FORCE would break detection of newly-added files under +# local/recipes/ because Make's mtime model cannot track directory-tree growth. $(REDBEAR_TAG): FORCE ifeq ($(PODMAN_BUILD),1) $(PODMAN_RUN) make $@ else - bash -c 'export REDBEAR_TAG="$$1"; exec bash local/scripts/integrate-redbear.sh' _ '$(REDBEAR_TAG)' + @current=$$(find $(REDBEAR_INPUTS) -type f -printf '%P %s %T@\n' 2>/dev/null | sort | sha256sum | cut -d' ' -f1); \ + stored=""; \ + if [ -f "$(REDBEAR_FP)" ]; then stored=$$(cat "$(REDBEAR_FP)" 2>/dev/null); fi; \ + if [ "$$current" = "$$stored" ] && [ -f "$@" ]; then \ + touch "$@"; \ + else \ + echo "Red Bear: integration inputs changed (or first run), executing integrate-redbear.sh..."; \ + bash -c 'export REDBEAR_TAG="$$1"; exec bash local/scripts/integrate-redbear.sh' _ '$(REDBEAR_TAG)'; \ + echo "$$current" > "$(REDBEAR_FP)"; \ + fi endif redbear: $(REDBEAR_TAG) redbear_clean: - rm -f "$(REDBEAR_TAG)" + rm -f "$(REDBEAR_TAG)" "$(REDBEAR_FP)" # Source archival — exports fully-patched, versioned source archives # for all recipes with source/ directories to sources// @@ -23,7 +38,7 @@ SOURCES_TAG=$(SOURCES_DIR)/.sources-tag # Standalone: archive what's cached (no rebuild needed) sources: - @echo "Archiving fully-patched source packages..." + @echo "Archiving source packages..." bash local/scripts/archive-sources.sh --all @mkdir -p "$(SOURCES_DIR)" @touch "$(SOURCES_TAG)" diff --git a/src/cook/cook_build.rs b/src/cook/cook_build.rs index 328918981e..5f5fc99614 100644 --- a/src/cook/cook_build.rs +++ b/src/cook/cook_build.rs @@ -21,7 +21,7 @@ use std::{ time::SystemTime, }; -use crate::{is_redox, log_to_pty}; +use crate::{is_redox, log_to_pty, wrap_io_err}; #[derive(Serialize, Deserialize, Default)] pub struct DepHashes { @@ -38,7 +38,10 @@ impl DepHashes { pub fn write(&self, path: &Path) -> Result<(), String> { let content = toml::to_string(self) .map_err(|e| format!("failed to serialize dep_hashes.toml: {e}"))?; - // Atomic write: write to temp file, then rename + if let Some(parent) = path.parent() { + fs::create_dir_all(parent) + .map_err(|e| format!("failed to create dep_hashes.toml parent dir: {e:?}"))?; + } let tmp_path = path.with_extension("tmp"); fs::write(&tmp_path, &content) .map_err(|e| format!("failed to write dep_hashes.toml tmp: {e:?}"))?; @@ -75,7 +78,7 @@ impl SourceContentHash { // Hash the source directory contents. let mut paths = Vec::new(); - collect_files_recursive(source_dir, &mut paths)?; + collect_files_recursive(source_dir, source_dir, &mut paths)?; paths.sort(); for rel_path in &paths { let abs_path = source_dir.join(rel_path); @@ -161,7 +164,11 @@ impl SourceContentHash { /// surprises across the sysroot). I/O errors at the top level are /// propagated; per-file errors are silently skipped (we treat /// unreadable files as having "no content change" — see hash logic). -fn collect_files_recursive(dir: &Path, out: &mut Vec) -> std::io::Result<()> { +fn collect_files_recursive( + dir: &Path, + root: &Path, + out: &mut Vec, +) -> std::io::Result<()> { if !dir.exists() { return Ok(()); } @@ -170,13 +177,12 @@ fn collect_files_recursive(dir: &Path, out: &mut Vec) -> std::io::Resul let entry = entry?; let file_type = entry.file_type()?; let path = entry.path(); - let rel = path.strip_prefix(dir).unwrap_or(&path).to_path_buf(); + let rel = path.strip_prefix(root).unwrap_or(&path).to_path_buf(); if file_type.is_dir() { - collect_files_recursive(&path, out)?; + collect_files_recursive(&path, root, out)?; } else if file_type.is_file() || file_type.is_symlink() { out.push(rel); } - // Devices, sockets, etc. are silently skipped. } Ok(()) } @@ -803,13 +809,24 @@ pub fn build( }; let command = { - //TODO: remove unwraps - let cookbook_build = build_dir.canonicalize().unwrap(); - let cookbook_recipe = recipe_dir.canonicalize().unwrap(); - let cookbook_root = Path::new(".").canonicalize().unwrap(); - let cookbook_stage = stage_dir_tmp.canonicalize().unwrap(); - let cookbook_source = source_dir.canonicalize().unwrap(); - let cookbook_sysroot = sysroot_dir.canonicalize().unwrap(); + let cookbook_build = build_dir + .canonicalize() + .map_err(wrap_io_err!(build_dir, "Canonicalize build dir"))?; + let cookbook_recipe = recipe_dir + .canonicalize() + .map_err(wrap_io_err!(recipe_dir, "Canonicalize recipe dir"))?; + let cookbook_root = Path::new(".") + .canonicalize() + .map_err(wrap_io_err!(Path::new("."), "Canonicalize cookbook root"))?; + let cookbook_stage = stage_dir_tmp + .canonicalize() + .map_err(wrap_io_err!(stage_dir_tmp, "Canonicalize stage dir"))?; + let cookbook_source = source_dir + .canonicalize() + .map_err(wrap_io_err!(source_dir, "Canonicalize source dir"))?; + let cookbook_sysroot = sysroot_dir + .canonicalize() + .map_err(wrap_io_err!(sysroot_dir, "Canonicalize sysroot dir"))?; let cookbook_toolchain = toolchain_dir.canonicalize().ok(); let bash_args = if cli_verbose { "-ex" } else { "-e" }; let local_redoxer = Path::new("target/release/cookbook_redbear_redoxer"); @@ -1178,20 +1195,28 @@ pub fn build_remote( #[cfg(test)] mod tests { + use std::collections::{BTreeMap, BTreeSet}; + use std::fs; + use std::io::Write; use std::os::unix; + use std::path::{Path, PathBuf}; + + use super::{DepHashes, SourceContentHash, collect_current_dep_hashes, dep_hashes_changed}; + use pkg::PackageName; + + fn temp_dir(name: &str) -> PathBuf { + let mut root = std::env::temp_dir(); + root.push(format!("redbear_cookbook_test_{name}_{}", std::process::id())); + let _ = fs::remove_dir_all(&root); + fs::create_dir_all(&root).expect("create temp dir"); + root + } #[test] fn file_system_loop_no_infinite_loop() { - let mut root = std::env::temp_dir(); - root.push("temp_test_dir_file_system_loop_no_infinite_loop"); - let _ = std::fs::remove_dir_all(&root); - std::fs::create_dir_all(&root).expect("Failed to create temporary root directory"); - - // Hierarchy with an infinite loop + let root = temp_dir("fs_loop"); let dir = root.join("loop"); unix::fs::symlink(&root, &dir).expect("Linking {dir:?} to {root:?}"); - - // Sanity check that we have a loop assert_eq!( root.canonicalize().unwrap(), dir.canonicalize().unwrap(), @@ -1205,4 +1230,359 @@ mod tests { "auto_deps shouldn't have yielded any libraries" ); } + + #[test] + fn dep_hashes_roundtrip_preserves_entries() { + let dir = temp_dir("dep_hashes_roundtrip"); + let path = dir.join("dep_hashes.toml"); + + let original = DepHashes { + hashes: { + let mut m = BTreeMap::new(); + m.insert("redoxfs".to_string(), "abc123".to_string()); + m.insert("relibc".to_string(), "def456".to_string()); + m + }, + }; + + original.write(&path).expect("write should succeed"); + assert!(path.exists(), "dep_hashes.toml should exist after write"); + + let loaded = DepHashes::read(&path).expect("read should return Some after write"); + assert_eq!( + loaded.hashes, original.hashes, + "round-trip must preserve all key-value pairs" + ); + } + + #[test] + fn dep_hashes_read_returns_none_for_missing_file() { + let dir = temp_dir("dep_hashes_missing"); + let path = dir.join("nonexistent.toml"); + assert!( + DepHashes::read(&path).is_none(), + "read on a missing path must return None, not panic" + ); + } + + #[test] + fn dep_hashes_read_returns_none_for_corrupt_toml() { + let dir = temp_dir("dep_hashes_corrupt"); + let path = dir.join("dep_hashes.toml"); + fs::write(&path, b"this is not valid toml [[[").expect("write corrupt file"); + + assert!( + DepHashes::read(&path).is_none(), + "read on corrupt TOML must return None — this documents the current \ + silent-swallow behavior flagged in the build-system assessment" + ); + } + + #[test] + fn dep_hashes_write_is_atomic_no_tmp_left_behind() { + let dir = temp_dir("dep_hashes_atomic"); + let path = dir.join("subdir/dep_hashes.toml"); + let hashes = DepHashes::default(); + + hashes.write(&path).expect("write should succeed"); + let tmp = path.with_extension("tmp"); + assert!( + !tmp.exists(), + "atomic write must not leave a .tmp file behind on success" + ); + assert!(path.exists(), "target file must exist after atomic write"); + } + + #[test] + fn dep_hashes_changed_identical_maps_return_false() { + let stored = BTreeMap::from([ + ("a".to_string(), "hash_a".to_string()), + ("b".to_string(), "hash_b".to_string()), + ]); + let current = stored.clone(); + assert!( + !dep_hashes_changed(&stored, ¤t), + "identical maps must report no change" + ); + } + + #[test] + fn dep_hashes_changed_different_length_returns_true() { + let stored = BTreeMap::from([("a".to_string(), "h".to_string())]); + let current = BTreeMap::from([ + ("a".to_string(), "h".to_string()), + ("b".to_string(), "h".to_string()), + ]); + assert!( + dep_hashes_changed(&stored, ¤t), + "length mismatch must always report changed" + ); + } + + #[test] + fn dep_hashes_changed_hash_value_differs_returns_true() { + let stored = BTreeMap::from([("a".to_string(), "old_hash".to_string())]); + let current = BTreeMap::from([("a".to_string(), "new_hash".to_string())]); + assert!( + dep_hashes_changed(&stored, ¤t), + "a changed hash value for an existing key must be detected" + ); + } + + #[test] + fn dep_hashes_changed_missing_key_returns_true() { + let stored = BTreeMap::from([ + ("a".to_string(), "h".to_string()), + ("b".to_string(), "h".to_string()), + ]); + let current = BTreeMap::from([("a".to_string(), "h".to_string())]); + assert!( + dep_hashes_changed(&stored, ¤t), + "a missing key in current must be detected as a change" + ); + } + + #[test] + fn dep_hashes_changed_both_empty_return_false() { + let stored: BTreeMap = BTreeMap::new(); + let current: BTreeMap = BTreeMap::new(); + assert!( + !dep_hashes_changed(&stored, ¤t), + "two empty maps must report no change (no deps = no change)" + ); + } + + fn make_source_tree(root: &Path, files: &[(&str, &[u8])]) { + for (rel, content) in files { + let full = root.join(rel); + if let Some(parent) = full.parent() { + fs::create_dir_all(parent).expect("create parent dir"); + } + let mut f = fs::File::create(&full).expect("create file"); + f.write_all(content).expect("write content"); + } + } + + #[test] + fn source_content_hash_is_deterministic_for_identical_inputs() { + let dir1 = temp_dir("sch_deterministic_1"); + let dir2 = temp_dir("sch_deterministic_2"); + + let files = [("src/main.rs", b"fn main() {}" as &[u8])]; + make_source_tree(&dir1, &files); + make_source_tree(&dir2, &files); + + let recipe1 = dir1.join("recipe.toml"); + let recipe2 = dir2.join("recipe.toml"); + fs::write(&recipe1, b"[build]\n").unwrap(); + fs::write(&recipe2, b"[build]\n").unwrap(); + + let h1 = SourceContentHash::compute(&dir1, &recipe1, &[]).expect("compute h1"); + let h2 = SourceContentHash::compute(&dir2, &recipe2, &[]).expect("compute h2"); + + assert_eq!( + h1, h2, + "identical source trees must produce identical hashes" + ); + } + + #[test] + fn source_content_hash_detects_content_change() { + let dir = temp_dir("sch_content_change"); + + let files = [("src/lib.rs", b"pub fn f() {}" as &[u8])]; + make_source_tree(&dir, &files); + let recipe = dir.join("recipe.toml"); + fs::write(&recipe, b"[build]\n").unwrap(); + + let h1 = SourceContentHash::compute(&dir, &recipe, &[]).expect("compute h1"); + + fs::write(dir.join("src/lib.rs"), b"pub fn g() {}").unwrap(); + let h2 = SourceContentHash::compute(&dir, &recipe, &[]).expect("compute h2"); + + assert_ne!( + h1, h2, + "changed file content must produce a different hash" + ); + } + + #[test] + fn source_content_hash_detects_added_file() { + let dir = temp_dir("sch_added_file"); + make_source_tree(&dir, &[("src/a.rs", b"a" as &[u8])]); + let recipe = dir.join("recipe.toml"); + fs::write(&recipe, b"[build]\n").unwrap(); + + let h1 = SourceContentHash::compute(&dir, &recipe, &[]).expect("compute h1"); + + make_source_tree(&dir, &[("src/b.rs", b"b" as &[u8])]); + let h2 = SourceContentHash::compute(&dir, &recipe, &[]).expect("compute h2"); + + assert_ne!(h1, h2, "adding a file must invalidate the hash"); + } + + #[test] + fn source_content_hash_detects_recipe_toml_change() { + let dir = temp_dir("sch_recipe_change"); + make_source_tree(&dir, &[("src/a.rs", b"a" as &[u8])]); + let recipe = dir.join("recipe.toml"); + fs::write(&recipe, b"[build]\n").unwrap(); + + let h1 = SourceContentHash::compute(&dir, &recipe, &[]).expect("compute h1"); + + fs::write(&recipe, b"[build]\nflag = true\n").unwrap(); + let h2 = SourceContentHash::compute(&dir, &recipe, &[]).expect("compute h2"); + + assert_ne!( + h1, h2, + "a change to recipe.toml must invalidate the hash" + ); + } + + #[test] + fn source_content_hash_detects_patch_change() { + let dir = temp_dir("sch_patch_change"); + make_source_tree(&dir, &[("src/a.rs", b"a" as &[u8])]); + let recipe = dir.join("recipe.toml"); + fs::write(&recipe, b"[build]\n").unwrap(); + let patch1 = dir.join("P1.patch"); + fs::write(&patch1, b"diff --git a/src/a.rs b/src/a.rs\n").unwrap(); + + let h1 = + SourceContentHash::compute(&dir, &recipe, &[patch1.clone()]).expect("compute h1"); + + fs::write(&patch1, b"diff --git a/src/a.rs b/src/a.rs\n+changed\n").unwrap(); + let h2 = + SourceContentHash::compute(&dir, &recipe, &[patch1.clone()]).expect("compute h2"); + + assert_ne!( + h1, h2, + "a change to a patch file must invalidate the hash" + ); + } + + #[test] + fn source_content_hash_ignores_target_and_git_dirs() { + let dir = temp_dir("sch_ignores_artifacts"); + + make_source_tree( + &dir, + &[ + ("src/main.rs", b"fn main() {}" as &[u8]), + ("target/debug/incremental/ignore.me", b"build artifact"), + (".git/HEAD", b"ref: refs/heads/main"), + ("file.swp", b"swap"), + ("file.tmp", b"temp"), + ], + ); + let recipe = dir.join("recipe.toml"); + fs::write(&recipe, b"[build]\n").unwrap(); + + let h1 = SourceContentHash::compute(&dir, &recipe, &[]).expect("compute h1"); + + fs::write(dir.join("target/debug/incremental/ignore.me"), b"changed").unwrap(); + fs::write(dir.join(".git/HEAD"), b"ref: refs/heads/other").unwrap(); + fs::write(dir.join("file.swp"), b"swapped").unwrap(); + fs::write(dir.join("file.tmp"), b"templed").unwrap(); + + let h2 = SourceContentHash::compute(&dir, &recipe, &[]).expect("compute h2"); + + assert_eq!( + h1, h2, + "changes to target/, .git/, .swp, .tmp must NOT affect the hash" + ); + } + + #[test] + fn source_content_hash_read_write_roundtrip() { + let dir = temp_dir("sch_rw_roundtrip"); + let path = dir.join("source_hash.txt"); + + let hash = SourceContentHash("deadbeef".to_string()); + hash.write(&path).expect("write should succeed"); + + let loaded = SourceContentHash::read(&path).expect("read should return Some"); + assert_eq!(hash, loaded, "round-trip must preserve the hash value"); + } + + #[test] + fn source_content_hash_read_returns_none_for_missing_file() { + let dir = temp_dir("sch_read_missing"); + let path = dir.join("nonexistent.txt"); + assert!( + SourceContentHash::read(&path).is_none(), + "read on a missing path must return None" + ); + } + + #[test] + fn source_content_hash_handles_empty_source_dir() { + let dir = temp_dir("sch_empty_source"); + let recipe = dir.join("recipe.toml"); + fs::write(&recipe, b"[build]\n").unwrap(); + + let hash = + SourceContentHash::compute(&dir, &recipe, &[]).expect("compute on empty dir"); + assert!( + !hash.as_str().is_empty(), + "empty source dir must still produce a valid hash (of just recipe.toml)" + ); + } + + #[test] + fn collect_current_dep_hashes_reads_blake3_from_sidecar_toml() { + let dir = temp_dir("ccd_reads_blake3"); + + let pkgar_path = dir.join("redoxfs.pkgar"); + fs::write(&pkgar_path, b"binary content placeholder").unwrap(); + let toml_path = pkgar_path.with_extension("toml"); + fs::write( + &toml_path, + b"blake3 = \"aabbccdd\"\nversion = \"1.0\"\n", + ) + .unwrap(); + + let name = PackageName::new("redoxfs").unwrap(); + let deps = BTreeSet::from([(name, pkgar_path)]); + let hashes = collect_current_dep_hashes(&deps); + + assert_eq!( + hashes.get("redoxfs").map(|s| s.as_str()), + Some("aabbccdd"), + "the blake3 field from the sidecar .toml must be collected" + ); + } + + #[test] + fn collect_current_dep_hashes_skips_missing_sidecar_toml() { + let dir = temp_dir("ccd_missing_toml"); + let pkgar_path = dir.join("orphan.pkgar"); + fs::write(&pkgar_path, b"binary").unwrap(); + + let name = PackageName::new("orphan").unwrap(); + let deps = BTreeSet::from([(name, pkgar_path)]); + let hashes = collect_current_dep_hashes(&deps); + + assert!( + hashes.is_empty(), + "a dep with no sidecar .toml must be excluded from current hashes" + ); + } + + #[test] + fn collect_current_dep_hashes_skips_empty_blake3() { + let dir = temp_dir("ccd_empty_blake3"); + let pkgar_path = dir.join("empty.pkgar"); + let toml_path = pkgar_path.with_extension("toml"); + fs::write(&toml_path, b"blake3 = \"\"\n").unwrap(); + + let name = PackageName::new("empty").unwrap(); + let deps = BTreeSet::from([(name, pkgar_path)]); + let hashes = collect_current_dep_hashes(&deps); + + assert!( + hashes.is_empty(), + "a dep with empty blake3 must be excluded from current hashes" + ); + } } diff --git a/src/cook/fetch.rs b/src/cook/fetch.rs index aa33532ab1..0506c89404 100644 --- a/src/cook/fetch.rs +++ b/src/cook/fetch.rs @@ -1119,7 +1119,7 @@ pub fn fetch_remote( source_dir: PathBuf, logger: &PtyOut, ) -> Result { - let (mut manager, repository) = fetch_repo::get_binary_repo(); + let (mut manager, repository) = fetch_repo::get_binary_repo()?; let target_dir = create_target_dir(recipe_dir, recipe.target)?; if logger.is_some() { let writer = logger.as_ref().unwrap().1.try_clone().unwrap(); diff --git a/src/cook/fetch_repo.rs b/src/cook/fetch_repo.rs index 799fe49bef..f86e37a5b2 100644 --- a/src/cook/fetch_repo.rs +++ b/src/cook/fetch_repo.rs @@ -34,36 +34,61 @@ fn load_cached_repo(path: &Path) -> Option { Repository::from_toml(&toml_str).ok() } -fn init_binary_repo() -> (RepoManager, Repository) { +fn init_binary_repo() -> crate::Result<(RepoManager, Repository)> { let callback = Rc::new(RefCell::new(SilentCallback::new())); - let download_backend = CurlBackend::new().expect("Curl not found"); + let download_backend = CurlBackend::new().map_err(|e| { + crate::Error::Other(format!("Curl backend init failed: {e}")) + })?; let mut repo = RepoManager::new(callback, Box::new(download_backend)); repo.add_remote(crate::REMOTE_PKG_SOURCE, redoxer::target()) - .expect("Unable to add remote"); + .map_err(|e| { + crate::Error::Other(format!( + "Unable to add remote '{}': {e}", + crate::REMOTE_PKG_SOURCE + )) + })?; let repo_path = PathBuf::from("build/remotes"); repo.set_download_path(repo_path.clone()); - repo.sync_keys().expect("Unable to sync keys"); + repo.sync_keys() + .map_err(|e| crate::Error::Other(format!("Unable to sync keys: {e}")))?; - let repo_toml = load_cached_repo(&repo_path.join("repo.toml")).unwrap_or_else(|| { - let (toml_str, _) = repo - .get_package_toml(&PackageName::new("repo").unwrap()) - .expect("Failed to fetch repo.toml"); - Repository::from_toml(&toml_str).expect("Fetched repo.toml is invalid") - }); + let repo_toml = match load_cached_repo(&repo_path.join("repo.toml")) { + Some(cached) => cached, + None => { + let pkg_name = PackageName::new("repo").map_err(|e| { + crate::Error::Other(format!( + "Invalid hardcoded package name 'repo': {e}" + )) + })?; + let (toml_str, _) = repo + .get_package_toml(&pkg_name) + .map_err(|e| { + crate::Error::Other(format!("Failed to fetch repo.toml: {e}")) + })?; + Repository::from_toml(&toml_str).map_err(|e| { + crate::Error::Other(format!("Fetched repo.toml is invalid: {e}")) + })? + } + }; - (repo, repo_toml) + Ok((repo, repo_toml)) } -pub fn get_binary_repo() -> (RepoManager, Repository) { +pub fn get_binary_repo() -> crate::Result<(RepoManager, Repository)> { BINARY_REPO.with(|cell| { let mut opt = cell.borrow_mut(); if opt.is_none() { - *opt = Some(init_binary_repo()); + let initialized = init_binary_repo()?; + *opt = Some(initialized); + } + match opt.as_ref() { + Some((repo, repo_toml)) => Ok(((*repo).clone(), repo_toml.clone())), + None => Err(crate::Error::Other( + "binary repo cell uninitialized after init attempt — this is a logic bug".to_string(), + )), } - let (repo, repo_toml) = opt.as_ref().unwrap(); - ((*repo).clone(), repo_toml.clone()) }) }