From 23a032a4b8ceb99419575ac5e2816df6474c281e Mon Sep 17 00:00:00 2001 From: nicksinas Date: Tue, 18 Aug 2026 19:00:16 -0500 Subject: [PATCH 1/2] fix(stamps): hash overlay contents for verbatim overlays too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An overlay is applied to the sysroot by `cp`, not RPM, so its file contents must be folded into the install stamp — otherwise editing an overlay file leaves the stamp current, `avocado build` proceeds, and the stale sysroot ships the old file with no warning (ENG-2440). The content digest already existed but was gated to overlays that opt into preprocessing; verbatim overlays hashed only the config value. Drop the gate so content is always hashed (raw bytes for verbatim, post-{{ }} for preprocessed), and route the overlay dir through the shared parse_overlay_config so a bare-string overlay hashes the right tree. --- src/commands/rootfs/install.rs | 23 +---------- src/utils/overlay_preprocess.rs | 69 ++++++++++++++++++++++++++------ src/utils/stamps.rs | 70 +++++++++++++++++++++++---------- 3 files changed, 109 insertions(+), 53 deletions(-) diff --git a/src/commands/rootfs/install.rs b/src/commands/rootfs/install.rs index 5608a5a1..6328b6b2 100644 --- a/src/commands/rootfs/install.rs +++ b/src/commands/rootfs/install.rs @@ -5,28 +5,7 @@ use std::collections::{HashMap, HashSet}; use std::path::Path; use std::sync::Arc; -/// Parse the `overlay:` config value into `(dir, opaque)`. -/// Accepts either a plain string (`"path/to/dir"`) or a mapping -/// (`{ dir: "path/to/dir", mode: "opaque" | "merge" }`). -fn parse_overlay_config(value: &serde_yaml::Value) -> (String, bool) { - if let Some(dir_str) = value.as_str() { - (dir_str.to_string(), false) - } else if let Some(table) = value.as_mapping() { - let dir = table - .get("dir") - .and_then(|d| d.as_str()) - .unwrap_or("overlay") - .to_string(); - let opaque = table - .get("mode") - .and_then(|m| m.as_str()) - .map(|m| m == "opaque") - .unwrap_or(false); - (dir, opaque) - } else { - ("overlay".to_string(), false) - } -} +use crate::utils::overlay_preprocess::parse_overlay_config; /// Build the shell snippet that applies an overlay directory into a sysroot. /// `overlay_dir` is the path relative to `/opt/src` (the project root inside the container). diff --git a/src/utils/overlay_preprocess.rs b/src/utils/overlay_preprocess.rs index b4e777d1..9003311c 100644 --- a/src/utils/overlay_preprocess.rs +++ b/src/utils/overlay_preprocess.rs @@ -24,6 +24,29 @@ use crate::utils::interpolation::{preprocess_text, AvocadoContext}; /// scratch after the lock file moved to the top-level `avocado.lock`. const STAGING_SUBDIR: &str = ".avocado/overlay-staging"; +/// Parse the `overlay:` config value into `(dir, opaque)`. +/// Accepts either a plain string (`"path/to/dir"`) or a mapping +/// (`{ dir: "path/to/dir", mode: "opaque" | "merge" }`). +pub fn parse_overlay_config(value: &Value) -> (String, bool) { + if let Some(dir_str) = value.as_str() { + (dir_str.to_string(), false) + } else if let Some(table) = value.as_mapping() { + let dir = table + .get("dir") + .and_then(|d| d.as_str()) + .unwrap_or("overlay") + .to_string(); + let opaque = table + .get("mode") + .and_then(|m| m.as_str()) + .map(|m| m == "opaque") + .unwrap_or(false); + (dir, opaque) + } else { + ("overlay".to_string(), false) + } +} + /// Which overlay files to run the `{{ }}` preprocessor over. #[derive(Debug, Clone, PartialEq, Eq)] pub enum PreprocessSpec { @@ -169,12 +192,14 @@ fn rel_str(overlay_src: &Path, path: &Path) -> Result> { } } -/// Compute a deterministic digest of the overlay tree *after* preprocessing, -/// without materializing it to disk. Returns `None` when preprocessing is -/// disabled or the overlay dir does not exist (so stamps are unchanged for the -/// verbatim path). Folded into the rootfs/initramfs/ext build input hashes so a -/// changed template value (e.g. a new claim token) or an edited overlay file -/// forces a rebuild. Only the SHA-256 is retained — never the plaintext. +/// Compute a deterministic digest of the overlay tree, without materializing it +/// to disk. Preprocessing is applied per `spec`: a verbatim overlay +/// ([`PreprocessSpec::None`]) hashes raw file bytes, an enabled one hashes the +/// post-`{{ }}` content (so a changed template value — e.g. a new claim token — +/// also moves the digest). Returns `None` only when the overlay dir does not +/// exist. Folded into the rootfs/initramfs/ext build input hashes so any edit +/// to an overlay file forces a rebuild (ENG-2440); only the SHA-256 is retained +/// — never the plaintext. pub fn overlay_content_digest( project_root: &Path, overlay_rel_dir: &str, @@ -182,9 +207,6 @@ pub fn overlay_content_digest( root: &Value, context: &AvocadoContext, ) -> Result> { - if !spec.is_enabled() { - return Ok(None); - } let overlay_src = project_root.join(overlay_rel_dir); if !overlay_src.is_dir() { return Ok(None); @@ -524,11 +546,36 @@ mod tests { .unwrap() .unwrap(); assert_ne!(h1, h2, "digest must change when a templated value changes"); + } - // Disabled spec yields no digest (verbatim path unchanged). + #[test] + fn verbatim_digest_tracks_raw_bytes() { + // A verbatim overlay (no `preprocess`) still gets a content digest, over + // raw bytes — editing a file must move it (ENG-2440). `{{ }}` markers are + // left untouched since nothing selects the file for preprocessing. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + std::fs::create_dir_all(root.join("overlay/etc")).unwrap(); + std::fs::write(root.join("overlay/etc/f.txt"), "v1 {{ env.NOPE }}").unwrap(); + + let h1 = + overlay_content_digest(root, "overlay", &PreprocessSpec::None, &Value::Null, &ctx()) + .unwrap() + .expect("verbatim overlay must produce a digest"); + std::fs::write(root.join("overlay/etc/f.txt"), "v2-different").unwrap(); + let h2 = + overlay_content_digest(root, "overlay", &PreprocessSpec::None, &Value::Null, &ctx()) + .unwrap() + .unwrap(); + assert_ne!( + h1, h2, + "editing a verbatim overlay file must change the digest" + ); + + // A missing overlay dir is the one case that yields no digest. assert!(overlay_content_digest( root, - "overlay", + "absent", &PreprocessSpec::None, &Value::Null, &ctx() diff --git a/src/utils/stamps.rs b/src/utils/stamps.rs index d143400d..5e1180fa 100644 --- a/src/utils/stamps.rs +++ b/src/utils/stamps.rs @@ -893,13 +893,14 @@ fn script_hash_value(project_root: &Path, rel_path: &str) -> serde_yaml::Value { serde_yaml::Value::Mapping(m) } -/// Fold a digest of an overlay's post-preprocessing tree into `hash_data` under -/// `key`, but only when the overlay opts into preprocessing -/// (`overlay: { ..., preprocess: ... }`). For verbatim overlays this is a no-op, -/// preserving today's hashing (which keys only on the overlay config value, not -/// file contents). When enabled, a changed template value (e.g. a new claim -/// token) or an edited overlay file changes the digest and forces a rebuild. -/// Only the SHA-256 is stored — never the resolved plaintext. +/// Fold a digest of an overlay's tree into `hash_data` under `key`, so that an +/// edit to any overlay file forces a rebuild. The overlay is applied to the +/// sysroot by a plain `cp` (not RPM), so without this its file contents are +/// invisible to the install stamp and a change silently never reaches the image +/// (ENG-2440). A verbatim overlay hashes raw bytes; one that opts into +/// preprocessing (`overlay: { ..., preprocess: ... }`) hashes the post-`{{ }}` +/// content, so a changed template value (e.g. a new claim token) invalidates +/// too. Only the SHA-256 is stored — never the resolved plaintext. // The (target, runtime, cli_target_board) trio mirrors the interpolation // context the materialize step builds; bundling them into a context struct is // the right cleanup once a fourth CLI override lands. @@ -914,15 +915,11 @@ fn fold_overlay_content_hash( runtime: Option<&str>, cli_target_board: Option<&str>, ) -> Result<()> { - use crate::utils::overlay_preprocess::PreprocessSpec; + use crate::utils::overlay_preprocess::{parse_overlay_config, PreprocessSpec}; let spec = PreprocessSpec::from_overlay_value(overlay); - if !spec.is_enabled() { - return Ok(()); - } - let dir = overlay - .get("dir") - .and_then(|d| d.as_str()) - .unwrap_or("overlay"); + // `dir` via the shared parser so a bare-string overlay (`overlay: mydir`) + // hashes the right tree, not the "overlay" default. + let (dir, _opaque) = parse_overlay_config(overlay); // Build the same interpolation context the build's materialize step uses, so // the digest reflects the exact rendered overlay content. `target` keeps // `{{ avocado.target }}` accurate and `cli_target_board` keeps @@ -942,7 +939,7 @@ fn fold_overlay_content_hash( // let a broken overlay silently skip rebuild invalidation. if let Some(digest) = crate::utils::overlay_preprocess::overlay_content_digest( project_root, - dir, + &dir, &spec, config, &context, @@ -4301,9 +4298,11 @@ extensions: } #[test] - fn rootfs_verbatim_overlay_ignores_file_contents() { - // Without `preprocess`, the hash keys only on the overlay config value, - // not file contents (today's behavior) — no content digest is folded in. + fn rootfs_verbatim_overlay_hashes_file_contents() { + // A verbatim overlay is applied by `cp`, not RPM, so its contents must be + // folded into the install stamp — editing an overlay file has to make the + // stamp stale, otherwise the change silently never reaches the image + // (ENG-2440). let tmp = tempfile::TempDir::new().unwrap(); std::fs::create_dir_all(tmp.path().join("overlay/etc")).unwrap(); std::fs::write(tmp.path().join("overlay/etc/f.txt"), "v1").unwrap(); @@ -4322,7 +4321,38 @@ rootfs: let h1 = rootfs_config_hash(&config, tmp.path()); std::fs::write(tmp.path().join("overlay/etc/f.txt"), "v2-different").unwrap(); let h2 = rootfs_config_hash(&config, tmp.path()); - assert_eq!(h1, h2); + assert_ne!( + h1, h2, + "editing a verbatim overlay file must invalidate the stamp" + ); + } + + #[test] + fn rootfs_bare_string_overlay_hashes_file_contents() { + // The bare-string form (`overlay: dirname`) must hash the named dir, not + // the "overlay" default — a regression guard for the shared + // parse_overlay_config path. + let tmp = tempfile::TempDir::new().unwrap(); + std::fs::create_dir_all(tmp.path().join("custom/etc")).unwrap(); + std::fs::write(tmp.path().join("custom/etc/f.txt"), "v1").unwrap(); + + let config: serde_yaml::Value = serde_yaml::from_str( + r#" +rootfs: + packages: + avocado-pkg-rootfs: "*" + overlay: custom +"#, + ) + .unwrap(); + + let h1 = rootfs_config_hash(&config, tmp.path()); + std::fs::write(tmp.path().join("custom/etc/f.txt"), "v2-different").unwrap(); + let h2 = rootfs_config_hash(&config, tmp.path()); + assert_ne!( + h1, h2, + "editing a bare-string overlay's file must invalidate the stamp" + ); } #[test] From 403c15cc04d35a20d5552c88de8ef5c01eec4586 Mon Sep 17 00:00:00 2001 From: nicksinas Date: Wed, 19 Aug 2026 11:44:00 -0500 Subject: [PATCH 2/2] fix(stamps): don't fail verbatim overlays on non-UTF-8 filenames MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the content digest runs for verbatim overlays too, routing every path through rel_str made a non-UTF-8 filename hard-fail the build — a regression, since `cp -a` copies such a name fine and it built before. Hash the path from raw bytes (rel_bytes/os_bytes, lossless) so a verbatim overlay digests without demanding UTF-8. rel_str is now reached only by the preprocessing paths (glob matching + staging), which genuinely need UTF-8 — so its "scope/disable preprocess" error message is accurate again (addresses the PR review). For UTF-8 names the digest bytes are unchanged, so existing stamps don't invalidate. --- src/utils/overlay_preprocess.rs | 89 +++++++++++++++++++++++++++++---- 1 file changed, 80 insertions(+), 9 deletions(-) diff --git a/src/utils/overlay_preprocess.rs b/src/utils/overlay_preprocess.rs index 9003311c..d85a82af 100644 --- a/src/utils/overlay_preprocess.rs +++ b/src/utils/overlay_preprocess.rs @@ -169,11 +169,41 @@ fn sorted_entries(overlay_src: &Path) -> Vec { entries } +/// Overlay-relative path as raw bytes, `None` for the overlay root itself. +/// Lossless and infallible on a non-UTF-8 name, unlike [`rel_str`] — so the +/// verbatim digest (below) can hash a byte-for-byte-copied overlay without +/// demanding UTF-8 filenames. Only the preprocessing paths (glob matching + +/// staging), which are genuinely string-based, need [`rel_str`]. +fn rel_bytes(overlay_src: &Path, path: &Path) -> Option> { + let rel = path.strip_prefix(overlay_src).ok()?; + if rel.as_os_str().is_empty() { + return None; + } + Some(os_bytes(rel.as_os_str())) +} + +/// Raw bytes of an `OsStr`, losslessly on unix (where a filename is arbitrary +/// bytes). On other platforms fall back to the lossy UTF-8 form with `\` +/// separators normalized to `/`; a Windows filename is effectively always valid +/// UTF-16, so this is lossless in practice there too. +#[cfg(unix)] +fn os_bytes(s: &std::ffi::OsStr) -> Vec { + use std::os::unix::ffi::OsStrExt; + s.as_bytes().to_vec() +} + +#[cfg(not(unix))] +fn os_bytes(s: &std::ffi::OsStr) -> Vec { + s.to_string_lossy().replace('\\', "/").into_bytes() +} + /// Overlay-relative, forward-slash path. `Ok(None)` for the overlay root -/// itself. Errors on a non-UTF-8 path: staging and the digest are string-based, -/// so `to_string_lossy` would mangle a non-UTF-8 name to U+FFFD (and two such -/// names could collide / hash equal), unlike the byte-preserving verbatim `cp -a` -/// path. Reject explicitly rather than silently corrupt. +/// itself. Errors on a non-UTF-8 path because the *preprocessing* paths that +/// use it — glob matching and on-disk staging — are string-based, and +/// `to_string_lossy` would mangle a non-UTF-8 name to U+FFFD (two such names +/// could then collide). The verbatim digest hashes [`rel_bytes`] instead, so a +/// non-UTF-8 filename only trips this when the overlay opts into preprocessing — +/// where the suggested fix (scope/disable `preprocess`) genuinely applies. fn rel_str(overlay_src: &Path, path: &Path) -> Result> { let rel = match path.strip_prefix(overlay_src) { Ok(r) => r, @@ -215,7 +245,7 @@ pub fn overlay_content_digest( use sha2::{Digest, Sha256}; let mut hasher = Sha256::new(); for entry in sorted_entries(&overlay_src) { - let Some(rel) = rel_str(&overlay_src, entry.path())? else { + let Some(rel) = rel_bytes(&overlay_src, entry.path()) else { continue; }; let ft = entry.file_type(); @@ -223,14 +253,20 @@ pub fn overlay_content_digest( // Fold directory modes so a permission drift (e.g. 0700 .ssh widened // to 0755) invalidates the stamp — materialize preserves dir modes. let mode = file_mode(entry.path()); - hasher.update(format!("D\0{rel}\0{mode:o}\0").as_bytes()); + hasher.update(b"D\0"); + hasher.update(&rel); + hasher.update(format!("\0{mode:o}\0").as_bytes()); } else if ft.is_symlink() { // Fail the same way `materialize` does on an unreadable link, so the // stamp check and the build can't disagree. let target = std::fs::read_link(entry.path()).with_context(|| { format!("Failed to read overlay symlink: {}", entry.path().display()) })?; - hasher.update(format!("L\0{rel}\0{}\0", target.to_string_lossy()).as_bytes()); + hasher.update(b"L\0"); + hasher.update(&rel); + hasher.update(b"\0"); + hasher.update(os_bytes(target.as_os_str())); + hasher.update(b"\0"); } else if ft.is_file() { // Propagate read/preprocess errors rather than silently dropping the // digest, which would let a broken overlay skip rebuild invalidation. @@ -238,9 +274,20 @@ pub fn overlay_content_digest( format!("Failed to read overlay file: {}", entry.path().display()) })?; let mode = file_mode(entry.path()); - let bytes = process_file_bytes(&rel, raw, spec, root, context)?; + // Only preprocessing inspects the path (to glob-match) and rewrites + // content, and it needs a UTF-8 path to do so. A verbatim overlay + // copies bytes untouched, so don't demand UTF-8 of its filenames. + let bytes = if spec.is_enabled() { + let rel_utf8 = rel_str(&overlay_src, entry.path())? + .expect("a non-root file always has a relative path"); + process_file_bytes(&rel_utf8, raw, spec, root, context)? + } else { + raw + }; let file_hash = Sha256::digest(&bytes); - hasher.update(format!("F\0{rel}\0{mode:o}\0").as_bytes()); + hasher.update(b"F\0"); + hasher.update(&rel); + hasher.update(format!("\0{mode:o}\0").as_bytes()); hasher.update(file_hash); } } @@ -583,4 +630,28 @@ mod tests { .unwrap() .is_none()); } + + // A non-UTF-8 filename can't be created on a UTF-8-enforcing filesystem + // (e.g. macOS APFS returns EILSEQ), so exercise the path handling directly + // and in memory rather than through a temp file. + #[cfg(unix)] + #[test] + fn non_utf8_path_hashes_losslessly_but_preprocessing_rejects_it() { + use std::os::unix::ffi::OsStrExt; + let raw = b"bad\xffname"; + let name = std::ffi::OsStr::from_bytes(raw); + + // The verbatim digest keys on raw bytes, so a `cp -a`-copyable non-UTF-8 + // filename hashes losslessly instead of failing the build (ENG-2440 + // review) — `to_string_lossy` would collapse 0xff to U+FFFD. + assert_eq!(os_bytes(name), raw); + let root = Path::new("/proj/overlay"); + assert_eq!(rel_bytes(root, &root.join(name)).as_deref(), Some(&raw[..])); + + // Preprocessing is genuinely string-based, so it still rejects the name + // with a clear error — the one place the "scope/disable preprocess" + // guidance applies. + let err = rel_str(root, &root.join(name)).unwrap_err(); + assert!(err.to_string().contains("non-UTF-8")); + } }