diff --git a/src/commands/initramfs/image.rs b/src/commands/initramfs/image.rs index 21794cf7..52237b9a 100644 --- a/src/commands/initramfs/image.rs +++ b/src/commands/initramfs/image.rs @@ -11,13 +11,13 @@ use crate::utils::{ host_copy::copy_volume_path_to_host, kab_wrap::generate_kab_wrap_script, output::{print_error, print_info, print_success, OutputLevel}, - permissions::{mapping_from_hashmap, render_users_groups_script}, + permissions::{mapping_from_map, render_users_groups_script}, runs_on::RunsOnContext, target::resolve_target_required, }; use crate::commands::rootfs::image::{ - render_auth_files_hash, render_hook_block, resolve_install_hooks, NAMESPACE_UUID, + render_build_id_block, render_hook_block, resolve_install_hooks, BuildIdSpec, NAMESPACE_UUID, }; /// Default post-install commands for the initramfs build. Same shape as @@ -88,31 +88,15 @@ if [ -d "$INITRAMFS_SYSROOT/usr" ]; then {post_install_block} - # Compute deterministic build ID for initramfs. - # - # LC_ALL=C for the same reason as the cpio pipeline below: this hash becomes - # $INITRAMFS_BUILD_ID, which is appended to initrd-release and - # os-release-initrd *inside* the archive. Collation drift would reorder the - # NEVRA list, change the hash, and so change the archive contents for an - # otherwise unchanged package set. - INITRAMFS_PKG_NEVRA=$(rpm -qa --queryformat '%{{NEVRA}}\n' --root "$INITRAMFS_SYSROOT" | LC_ALL=C sort) - INITRAMFS_PKG_HASH=$(echo "$INITRAMFS_PKG_NEVRA" | sha256sum | awk '{{print $1}}') - - # Fold the assembled auth files into the build id so a `permissions:`-only - # change — which the NEVRA set above is blind to — still moves the id and - # OTAs (ENG-2437). See render_auth_files_hash. -{auth_hash_block} - - INITRAMFS_BUILD_ID=$(python3 -c "import uuid; print(uuid.uuid5(uuid.UUID('{namespace_uuid}'), '$INITRAMFS_PKG_HASH:$INITRAMFS_AUTH_HASH'))") - - # Inject identity into initrd-release and os-release-initrd - if [ -f "$INITRAMFS_WORK/usr/lib/initrd-release" ]; then - echo "AVOCADO_OS_BUILD_ID=$INITRAMFS_BUILD_ID" >> "$INITRAMFS_WORK/usr/lib/initrd-release" - fi - if [ -f "$INITRAMFS_WORK/usr/lib/os-release-initrd" ]; then - echo "AVOCADO_OS_BUILD_ID=$INITRAMFS_BUILD_ID" >> "$INITRAMFS_WORK/usr/lib/os-release-initrd" - fi - + # Compute the deterministic build id from the assembled work tree (see + # render_build_id_block). Taken before the identity injection below so the + # hash can't depend on the id it is about to write. LC_ALL=C throughout for + # the same reason as the cpio pipeline: collation must not reorder the hash + # inputs (the id lands in initrd-release / os-release-initrd inside the + # archive, so a shift would change the archive for an unchanged tree). + # Runs BEFORE the build-id derivation so the tree hash covers exactly + # what ships: dnf/rpm state is both nondeterministic and absent from the + # image, so it must be gone (or pruned) before the id is taken. # Purge package-manager bookkeeping from the work copy before archiving. # Same reasoning as the rootfs image — see the comment in # `generate_rootfs_build_script`, including why dnf's logs are NOT on the @@ -126,6 +110,16 @@ if [ -d "$INITRAMFS_SYSROOT/usr" ]; then echo "Purging package-manager state from initramfs image" rm -rf "$INITRAMFS_WORK/var/lib/rpm" "$INITRAMFS_WORK/var/lib/dnf" "$INITRAMFS_WORK/var/cache/dnf" +{build_id_block} + + # Inject identity into initrd-release and os-release-initrd + if [ -f "$INITRAMFS_WORK/usr/lib/initrd-release" ]; then + echo "AVOCADO_OS_BUILD_ID=$INITRAMFS_BUILD_ID" >> "$INITRAMFS_WORK/usr/lib/initrd-release" + fi + if [ -f "$INITRAMFS_WORK/usr/lib/os-release-initrd" ]; then + echo "AVOCADO_OS_BUILD_ID=$INITRAMFS_BUILD_ID" >> "$INITRAMFS_WORK/usr/lib/os-release-initrd" + fi + # Normalize mtimes across the staged tree so the cpio is reproducible. # # `cpio --reproducible` is only --ignore-devno --ignore-dirnlink @@ -189,11 +183,17 @@ if [ -d "$INITRAMFS_SYSROOT/usr" ]; then else echo "No initramfs sysroot found — skipping initramfs image build." fi"#, - namespace_uuid = namespace_uuid, initramfs_filesystem = initramfs_filesystem, post_install_block = post_install_block, permissions_section = permissions_section, - auth_hash_block = render_auth_files_hash("INITRAMFS_WORK", "INITRAMFS_AUTH_HASH"), + build_id_block = render_build_id_block(&BuildIdSpec { + namespace_uuid, + work_var: "INITRAMFS_WORK", + sysroot_var: "INITRAMFS_SYSROOT", + rpm_args: "", + id_var: "INITRAMFS_BUILD_ID", + identity_files: &["usr/lib/initrd-release", "usr/lib/os-release-initrd"], + }), ) } @@ -290,8 +290,8 @@ impl InitramfsImageCommand { .initramfs_default() .and_then(|img| config.resolve_image_permissions(img)) .map(|p| { - let users = mapping_from_hashmap(p.users.as_ref()); - let groups = mapping_from_hashmap(p.groups.as_ref()); + let users = mapping_from_map(p.users.as_ref()); + let groups = mapping_from_map(p.groups.as_ref()); render_users_groups_script( users.as_ref(), groups.as_ref(), @@ -559,6 +559,14 @@ mod tests { .expect("cpio step present"); assert!(purge_at < cpio_at, "purge must precede cpio creation"); + // And before the build-id derivation, so the tree hash covers exactly + // what ships rather than dnf/rpm state that is about to be deleted. + let tree_at = script.find("TREE_HASH=").expect("tree hash present"); + assert!( + purge_at < tree_at, + "purge must precede the build-id tree hash" + ); + // dnf never writes its logs into an installroot (logdir is not // prefixed by prepend_installroot), so a log purge here would be a // no-op that reads as coverage. Pin its absence. @@ -604,6 +612,27 @@ mod tests { ); } + /// Initramfs derives its id through the same render_build_id_block as rootfs + /// (NEVRA package hash + work-tree content hash, rpmdb pruned), so the two + /// images can't diverge on the correctness-critical id logic (ENG-2441). + #[test] + fn test_build_id_uses_shared_tree_hash() { + let s = generate_initramfs_build_script("ns", "cpio.zst", None, ""); + assert!(s.contains("TREE_HASH="), "work-tree content hash present"); + assert!(s.contains("-path ./var/lib/rpm"), "rpmdb pruned"); + assert!( + s.contains("INITRAMFS_BUILD_ID=$(python3"), + "id assigned to the initramfs var" + ); + assert!( + s.contains("'$PKG_HASH:$TREE_HASH'"), + "package + tree hash folded" + ); + // Both initramfs identity files are canonicalized before hashing. + assert!(s.contains(r#"[ -f "$INITRAMFS_WORK/usr/lib/initrd-release" ]"#)); + assert!(s.contains(r#"[ -f "$INITRAMFS_WORK/usr/lib/os-release-initrd" ]"#)); + } + /// gzip already omits MTIME/FNAME when reading stdin, but `-n` keeps that /// true if the pipeline is ever changed to compress a file in place. #[test] @@ -613,36 +642,27 @@ mod tests { } /// ENG-2437: a `permissions:`-only change must move the initramfs build - /// id too, so the id has to fold the auth-file hash into its uuid5 input — - /// the same way the rootfs build id does. + /// id. Under the tree-hash derivation (ENG-2441) the auth files are + /// ordinary tree content, so the guarantee holds iff the tree hash is + /// computed after the permissions section runs and `/etc` is never on the + /// prune list. #[test] - fn test_build_id_folds_auth_files() { + fn test_build_id_sees_permissions_changes() { let marker = "# permissions placeholder"; let script = generate_initramfs_build_script("ns", "cpio.zst", None, marker); + let perms_pos = script.find(marker).expect("permissions section present"); + let tree_pos = script + .find("INITRAMFS_TREE_HASH=") + .or_else(|| script.find("TREE_HASH=")) + .expect("tree hash present"); assert!( - script.contains("INITRAMFS_AUTH_HASH="), - "build id must incorporate a hash of the auth files" - ); - for f in ["passwd", "shadow", "group", "gshadow"] { - assert!( - script.contains(&format!("$INITRAMFS_WORK/etc/{f}")), - "auth hash must cover /etc/{f}" - ); - } - assert!( - script.contains("'$INITRAMFS_PKG_HASH:$INITRAMFS_AUTH_HASH'"), - "uuid5 input must combine the package hash and the auth hash" + perms_pos < tree_pos, + "tree hash must be computed after the permissions section runs" ); - - // The auth hash must be computed after the permissions section runs. - let perms_pos = script.find(marker).expect("permissions section present"); - let auth_pos = script - .find("INITRAMFS_AUTH_HASH=") - .expect("auth hash present"); assert!( - perms_pos < auth_pos, - "auth hash must be computed after the permissions section runs" + !script.contains("-path ./etc"), + "/etc must never be pruned from the tree hash — it is what carries permissions changes into the id" ); } diff --git a/src/commands/rootfs/image.rs b/src/commands/rootfs/image.rs index 343985ee..2c9ec06a 100644 --- a/src/commands/rootfs/image.rs +++ b/src/commands/rootfs/image.rs @@ -11,7 +11,7 @@ use crate::utils::{ host_copy::copy_volume_path_to_host, kab_wrap::generate_kab_wrap_script, output::{print_error, print_info, print_success, OutputLevel}, - permissions::{mapping_from_hashmap, render_users_groups_script}, + permissions::{mapping_from_map, render_users_groups_script}, runs_on::RunsOnContext, target::resolve_target_required, }; @@ -49,36 +49,6 @@ echo \"Applied systemd presets\"; fi", "echo \"Generated ld.so.cache\"", ]; -/// Render the shell snippet that hashes an image's identity/auth files -/// (`/etc/{passwd,shadow,group,gshadow}`) into `out_var`. -/// -/// Folded into the build id alongside the package NEVRA hash so a -/// `permissions:`-only change — which rewrites these files but never the -/// rpmdb — still moves the build id and therefore OTAs (ENG-2437). Shared -/// by the rootfs and initramfs build scripts so both derive their build id -/// the same way. -/// -/// `work_var` names the image work-dir shell variable (e.g. `ROOTFS_WORK`, -/// `INITRAMFS_WORK`). `LC_ALL=C sort` makes the hash independent of the -/// order the permissions section appended entries in (the `users:`/`groups:` -/// maps have no guaranteed iteration order); missing files (e.g. a minimal -/// initramfs with no `/etc/shadow`) contribute nothing rather than aborting. -/// -/// The `cat` is wrapped in a `{ …; } || true` brace group so a missing file -/// doesn't fail the pipeline: the standalone `avocado rootfs image` / -/// `avocado initramfs image` scripts run under `set -euo pipefail`, where an -/// unguarded `cat` failure would take down the `$(…)` assignment — and with -/// its only diagnostic sent to `/dev/null`, kill the build with no output. -pub fn render_auth_files_hash(work_var: &str, out_var: &str) -> String { - format!( - r#" {out_var}=$({{ cat \ - "${work_var}/etc/passwd" \ - "${work_var}/etc/shadow" \ - "${work_var}/etc/group" \ - "${work_var}/etc/gshadow" 2>/dev/null || true; }} | LC_ALL=C sort | sha256sum | awk '{{print $1}}')"# - ) -} - /// Render a list of user-supplied shell commands as an indented block, /// preceded by a one-line "Running … hooks" echo for log clarity. Empty /// input returns an empty string so the surrounding script stays clean. @@ -122,6 +92,111 @@ fi" } } +/// Work-relative paths pruned from the build-id tree hash: their bytes are +/// nondeterministic across builds and are either not part of the image identity +/// or already covered elsewhere. +/// - `var/lib/rpm`: the rpmdb sqlite embeds install timestamps — which is +/// exactly why package identity is hashed from the NEVRA set (`PKG_HASH`), +/// not these bytes. +/// - `var/cache`, `var/log`: dnf caches and logs, build-varying. +// ./var/lib/dnf holds history.sqlite, whose bytes move on every install +// transaction — with id = f(tree) that is an OTA on every build, the exact +// risk-direction flip ENG-2441 warns about. It is also purged from the work +// copy before imaging, but the prune stays so the id is safe even if the +// purge ever moves or narrows. +const BUILD_ID_TREE_PRUNES: &[&str] = + &["./var/lib/rpm", "./var/lib/dnf", "./var/cache", "./var/log"]; + +/// The differences between the rootfs and initramfs build-id derivations, fed to +/// [`render_build_id_block`]. +pub struct BuildIdSpec<'a> { + /// Namespace UUID for the uuid5 derivation. + pub namespace_uuid: &'a str, + /// Shell variable naming the assembled work dir (e.g. `ROOTFS_WORK`). + pub work_var: &'a str, + /// Shell variable naming the sysroot holding the rpmdb (e.g. `ROOTFS_SYSROOT`). + pub sysroot_var: &'a str, + /// Extra `rpm` args to locate the db — rootfs passes `--dbpath /var/lib/rpm`, + /// initramfs the empty string (default path). + pub rpm_args: &'a str, + /// Shell variable to assign the derived id (e.g. `OS_BUILD_ID`). + pub id_var: &'a str, + /// Work-relative identity files to canonicalize before hashing (strip any + /// AVOCADO_* fields a prior build injected), e.g. `usr/lib/os-release`. + pub identity_files: &'a [&'a str], +} + +/// Render the shell that derives a deterministic build id into `spec.id_var`. +/// +/// The id is `uuid5(namespace, "$PKG_HASH:$TREE_HASH")`: +/// - `PKG_HASH` is the sorted NEVRA set — a stable package identity that does +/// not depend on the nondeterministic rpmdb *bytes* (so a version-only bump +/// with an identical file payload still moves the id). +/// - `TREE_HASH` is a content hash of the assembled work tree, so *any* +/// rootfs-affecting change — `permissions:`, `post_install`, `overlay:`, +/// anything future — moves the id and therefore OTAs. It hashes exactly what +/// the image carries: sorted path, type, mode, symlink target, and file +/// content. It excludes what the image build normalizes away (`mkfs.erofs -T` +/// mtimes, `--all-root` ownership) and what is fs-dependent (directory +/// sizes), so it can't churn on noise the image doesn't hold. +/// [`BUILD_ID_TREE_PRUNES`] are excluded. +/// +/// Must be invoked AFTER the permissions and post_install steps and BEFORE the +/// os-release identity lines are appended: the derivation strips `AVOCADO_*` +/// from the identity files first, so the hash can't depend on a prior build's +/// id (self-reference) or the possibly-unpinned runtime version. +pub fn render_build_id_block(spec: &BuildIdSpec) -> String { + let BuildIdSpec { + namespace_uuid, + work_var, + sysroot_var, + rpm_args, + id_var, + identity_files, + } = *spec; + + // Strip any AVOCADO_* fields a prior build injected into the identity files + // (the work copy inherits them from the sysroot) so the tree hash is stable. + let strip = identity_files + .iter() + .map(|f| { + format!( + " if [ -f \"${work_var}/{f}\" ]; then\n \ + sed -i '/^AVOCADO_OS_BUILD_ID=/d;/^AVOCADO_RUNTIME_NAME=/d;/^AVOCADO_RUNTIME_VERSION=/d' \"${work_var}/{f}\"\n fi" + ) + }) + .collect::>() + .join("\n"); + + let prune = BUILD_ID_TREE_PRUNES + .iter() + .map(|p| format!("-path {p}")) + .collect::>() + .join(" -o "); + + format!( + r#" # Canonicalize the identity files before hashing (see render_build_id_block). +{strip} + + # Deterministic package identity from the NEVRA set — independent of the + # rpmdb *bytes*, which embed install timestamps (hence var/lib/rpm is pruned + # from the tree hash below). LC_ALL=C so collation can't reorder it. + PKG_NEVRA=$(rpm {rpm_args} -qa --queryformat '%{{NEVRA}}\n' --root "${sysroot_var}" | LC_ALL=C sort) + PKG_HASH=$(echo "$PKG_NEVRA" | sha256sum | awk '{{print $1}}') + + # Content hash of the assembled work tree: the id must move iff the image + # bytes move. Hash only what the image carries — sorted path, type, mode, + # symlink target, file content — excluding what the image build normalizes + # out (mtime, ownership) or what is fs-dependent (directory sizes). %m is the + # octal mode, %l the symlink target (empty for non-links). + BUILD_ID_META=$(cd "${work_var}" && find . \( {prune} \) -prune -o -printf '%y %m %P\t%l\n' | LC_ALL=C sort) + BUILD_ID_CONTENT=$(cd "${work_var}" && find . \( {prune} \) -prune -o -type f -print0 | LC_ALL=C sort -z | xargs -0 -r sha256sum) + TREE_HASH=$(printf '%s\n%s\n' "$BUILD_ID_META" "$BUILD_ID_CONTENT" | sha256sum | awk '{{print $1}}') + + {id_var}=$(python3 -c "import uuid; print(uuid.uuid5(uuid.UUID('{namespace_uuid}'), '$PKG_HASH:$TREE_HASH'))")"# + ) +} + /// Generate the shell script fragment that builds a rootfs image from the shared sysroot. /// /// The generated script expects these shell variables to be set: @@ -186,31 +261,9 @@ if [ -d "$ROOTFS_SYSROOT/usr" ]; then {post_install_block} - # Compute deterministic AVOCADO_OS_BUILD_ID from installed packages - # LC_ALL=C so NEVRA ordering — and therefore this hash and the - # AVOCADO_OS_BUILD_ID injected into os-release inside the image — can't - # shift with the container's collation. - PKG_NEVRA=$(rpm --dbpath /var/lib/rpm -qa --queryformat '%{{NEVRA}}\n' --root "$ROOTFS_SYSROOT" | LC_ALL=C sort) - PKG_HASH=$(echo "$PKG_NEVRA" | sha256sum | awk '{{print $1}}') - - # Fold the assembled auth files into the build id so a `permissions:`-only - # change — which the NEVRA set above is blind to — still moves the id and - # OTAs (ENG-2437). See render_auth_files_hash. -{auth_hash_block} - - OS_BUILD_ID=$(python3 -c "import uuid; print(uuid.uuid5(uuid.UUID('{namespace_uuid}'), '$PKG_HASH:$AUTH_HASH'))") - - # Inject identity into os-release (work copy for the image, sysroot for stone) - # Strip any prior injected fields from the work copy before appending - sed -i '/^AVOCADO_OS_BUILD_ID=/d;/^AVOCADO_RUNTIME_NAME=/d;/^AVOCADO_RUNTIME_VERSION=/d' "$ROOTFS_WORK/usr/lib/os-release" - echo "AVOCADO_OS_BUILD_ID=$OS_BUILD_ID" >> "$ROOTFS_WORK/usr/lib/os-release" - echo "AVOCADO_RUNTIME_NAME=$RUNTIME_NAME" >> "$ROOTFS_WORK/usr/lib/os-release" - echo "AVOCADO_RUNTIME_VERSION=$RUNTIME_VERSION" >> "$ROOTFS_WORK/usr/lib/os-release" - - # Also write AVOCADO_OS_BUILD_ID to the sysroot so stone bundle can read it - sed -i '/^AVOCADO_OS_BUILD_ID=/d' "$ROOTFS_SYSROOT/usr/lib/os-release" - echo "AVOCADO_OS_BUILD_ID=$OS_BUILD_ID" >> "$ROOTFS_SYSROOT/usr/lib/os-release" - + # Runs BEFORE the build-id derivation so the tree hash covers exactly + # what ships: dnf/rpm state is both nondeterministic and absent from the + # image, so it must be gone (or pruned) before the id is taken. # Purge package-manager bookkeeping from the work copy before imaging. # # dnf installs into the sysroot leave ~13MB of state behind (measured on a @@ -242,6 +295,19 @@ if [ -d "$ROOTFS_SYSROOT/usr" ]; then echo "Purging package-manager state from rootfs image" rm -rf "$ROOTFS_WORK/var/lib/rpm" "$ROOTFS_WORK/var/lib/dnf" "$ROOTFS_WORK/var/cache/dnf" +{build_id_block} + + # Inject identity into os-release (work copy for the image, sysroot for stone). + # The work copy was canonicalized (AVOCADO_* stripped) during id derivation, + # so these appends land in a clean file. + echo "AVOCADO_OS_BUILD_ID=$OS_BUILD_ID" >> "$ROOTFS_WORK/usr/lib/os-release" + echo "AVOCADO_RUNTIME_NAME=$RUNTIME_NAME" >> "$ROOTFS_WORK/usr/lib/os-release" + echo "AVOCADO_RUNTIME_VERSION=$RUNTIME_VERSION" >> "$ROOTFS_WORK/usr/lib/os-release" + + # Also write AVOCADO_OS_BUILD_ID to the sysroot so stone bundle can read it + sed -i '/^AVOCADO_OS_BUILD_ID=/d' "$ROOTFS_SYSROOT/usr/lib/os-release" + echo "AVOCADO_OS_BUILD_ID=$OS_BUILD_ID" >> "$ROOTFS_SYSROOT/usr/lib/os-release" + # Build rootfs image using configured filesystem format ROOTFS_FS="{rootfs_filesystem}" ROOTFS_OUTPUT="$OUTPUT_DIR/avocado-image-rootfs-$TARGET_ARCH.$ROOTFS_FS" @@ -281,11 +347,17 @@ if [ -d "$ROOTFS_SYSROOT/usr" ]; then else echo "No rootfs sysroot found — skipping rootfs image build." fi"#, - namespace_uuid = namespace_uuid, rootfs_filesystem = rootfs_filesystem, post_install_block = post_install_block, permissions_section = permissions_section, - auth_hash_block = render_auth_files_hash("ROOTFS_WORK", "AUTH_HASH"), + build_id_block = render_build_id_block(&BuildIdSpec { + namespace_uuid, + work_var: "ROOTFS_WORK", + sysroot_var: "ROOTFS_SYSROOT", + rpm_args: "--dbpath /var/lib/rpm", + id_var: "OS_BUILD_ID", + identity_files: &["usr/lib/os-release"], + }), ) } @@ -382,8 +454,8 @@ impl RootfsImageCommand { .rootfs_default() .and_then(|img| config.resolve_image_permissions(img)) .map(|p| { - let users = mapping_from_hashmap(p.users.as_ref()); - let groups = mapping_from_hashmap(p.groups.as_ref()); + let users = mapping_from_map(p.users.as_ref()); + let groups = mapping_from_map(p.groups.as_ref()); render_users_groups_script( users.as_ref(), groups.as_ref(), @@ -688,6 +760,14 @@ mod tests { let mkfs_at = script.find("mkfs.erofs").expect("mkfs step present"); assert!(purge_at < mkfs_at, "purge must precede mkfs"); + // And before the build-id derivation, so the tree hash covers exactly + // what ships rather than dnf/rpm state that is about to be deleted. + let tree_at = script.find("TREE_HASH=").expect("tree hash present"); + assert!( + purge_at < tree_at, + "purge must precede the build-id tree hash" + ); + // dnf never writes its logs into an installroot (logdir is not // prefixed by prepend_installroot), so a log purge here would be a // no-op that reads as coverage. Pin its absence. @@ -719,72 +799,91 @@ mod tests { ); } - #[test] - fn test_render_auth_files_hash_snippet() { - // The snippet hashes the four auth files under the given work dir into - // the given var, tolerates missing files, and pins the sort to the C - // locale so the digest is independent of append order. - assert_eq!( - render_auth_files_hash("ROOTFS_WORK", "AUTH_HASH"), - r#" AUTH_HASH=$({ cat \ - "$ROOTFS_WORK/etc/passwd" \ - "$ROOTFS_WORK/etc/shadow" \ - "$ROOTFS_WORK/etc/group" \ - "$ROOTFS_WORK/etc/gshadow" 2>/dev/null || true; } | LC_ALL=C sort | sha256sum | awk '{print $1}')"# - ); - // Work dir and output var are both substituted, so the initramfs build - // reuses it verbatim with its own names. - let initramfs = render_auth_files_hash("INITRAMFS_WORK", "INITRAMFS_AUTH_HASH"); - assert!(initramfs.contains("INITRAMFS_AUTH_HASH=$({ cat")); - assert!(initramfs.contains("\"$INITRAMFS_WORK/etc/passwd\"")); - } - - #[test] - fn test_render_auth_files_hash_survives_pipefail() { - // Standalone `avocado rootfs image` / `avocado initramfs image` run the - // fragment under `set -euo pipefail`. A missing file (e.g. a minimal - // initramfs with no /etc/shadow) must not fail the pipeline and abort - // the build — the `{ …; } || true` brace group absorbs cat's status. - assert!(render_auth_files_hash("ROOTFS_WORK", "AUTH_HASH").contains("|| true; }")); - } - - #[test] - fn test_build_id_folds_auth_files() { - // Regression for ENG-2437: a permissions-only change must move - // AVOCADO_OS_BUILD_ID, so the build id has to hash the assembled - // auth files — not just the package NEVRA set — and feed both into - // the uuid5 derivation. - let script = generate_rootfs_build_script( + fn rootfs_script() -> String { + generate_rootfs_build_script( "00000000-0000-0000-0000-000000000000", "erofs-lz4", None, - "# permissions placeholder\n", - ); + "", + ) + } + #[test] + fn test_build_id_is_pkg_hash_plus_tree_hash() { + // ENG-2441: the id folds a content hash of the assembled work tree in + // beside the NEVRA package hash, so any rootfs-affecting change moves it. + let s = rootfs_script(); + assert!(s.contains("PKG_HASH="), "package identity retained"); + assert!(s.contains("TREE_HASH="), "work-tree content hash present"); assert!( - script.contains("AUTH_HASH="), - "build id must incorporate a hash of the auth files" + s.contains("uuid.uuid5(uuid.UUID('00000000-0000-0000-0000-000000000000'), '$PKG_HASH:$TREE_HASH')"), + "id must derive from both the package hash and the tree hash" ); - for f in ["passwd", "shadow", "group", "gshadow"] { - assert!( - script.contains(&format!("$ROOTFS_WORK/etc/{f}")), - "auth hash must cover /etc/{f}" - ); + } + + #[test] + fn test_tree_hash_prunes_nondeterministic_paths_and_excludes_metadata() { + // The rpmdb (install timestamps) and dnf caches/logs would churn the id + // every build, and mtime/ownership are normalized out of the image — so + // none may feed the hash. It hashes type, mode (%m), path (%P) and + // symlink target (%l) only. + let s = rootfs_script(); + for pruned in [ + "-path ./var/lib/rpm", + "-path ./var/cache", + "-path ./var/log", + ] { + assert!(s.contains(pruned), "tree hash must prune {pruned}"); } assert!( - script.contains("'$PKG_HASH:$AUTH_HASH'"), - "uuid5 input must combine the package hash and the auth hash" + s.contains(r"-printf '%y %m %P\t%l\n'"), + "hash covers type/mode/path/link" ); + // No mtime (%T*), user (%u/%U) or group (%g/%G) directives leak in. + for meta in ["%T", "%u", "%U", "%g", "%G"] { + assert!(!s.contains(meta), "tree hash must not depend on {meta}"); + } + } - // The auth hash must be computed after the permissions section has - // written the files, otherwise it can't observe the change. - let perms_pos = script - .find("# permissions placeholder") - .expect("permissions section present"); - let auth_pos = script.find("AUTH_HASH=").expect("auth hash present"); + #[test] + fn test_build_id_computed_before_identity_is_injected() { + // Taking the hash after the os-release identity append would make the id + // depend on itself (and on the possibly-unpinned runtime version), so the + // derivation — including the strip that canonicalizes os-release — must + // finish before anything is appended. + let s = rootfs_script(); + let strip = s + .find("sed -i '/^AVOCADO_OS_BUILD_ID=/d") + .expect("identity strip present"); + let tree = s.find("TREE_HASH=").expect("tree hash present"); + let derive = s + .find("OS_BUILD_ID=$(python3") + .expect("id derivation present"); + let append = s + .find(r#"echo "AVOCADO_OS_BUILD_ID=$OS_BUILD_ID" >>"#) + .expect("identity append present"); assert!( - perms_pos < auth_pos, - "auth hash must be computed after the permissions section runs" + strip < tree && tree < derive && derive < append, + "order must be: strip identity -> tree hash -> derive id -> append identity" ); } + + #[test] + fn test_render_build_id_block_shape() { + // Direct contract check on the shared helper both images use. + let block = render_build_id_block(&BuildIdSpec { + namespace_uuid: "11111111-1111-1111-1111-111111111111", + work_var: "WK", + sysroot_var: "SR", + rpm_args: "--dbpath /var/lib/rpm", + id_var: "MY_ID", + identity_files: &["usr/lib/os-release"], + }); + assert!(block.contains(r#"[ -f "$WK/usr/lib/os-release" ]"#)); + assert!(block.contains(r#"--root "$SR""#)); + assert!(block.contains(r#"rpm --dbpath /var/lib/rpm -qa"#)); + assert!(block.contains(r#"cd "$WK" && find ."#)); + assert!(block.contains("MY_ID=$(python3")); + assert!(block.contains("'$PKG_HASH:$TREE_HASH'")); + } } diff --git a/src/commands/runtime/build.rs b/src/commands/runtime/build.rs index 93e2c7a2..a8aa773f 100644 --- a/src/commands/runtime/build.rs +++ b/src/commands/runtime/build.rs @@ -6,7 +6,7 @@ use crate::utils::{ config::{ComposedConfig, Config, ImageConfig}, container::{RunConfig, SdkContainer, TuiContext}, output::{print_error, print_info, print_success, OutputLevel}, - permissions::{mapping_from_hashmap, render_users_groups_script}, + permissions::{mapping_from_map, render_users_groups_script}, runs_on::RunsOnContext, stamps::{ compute_runtime_build_input_hash, compute_runtime_install_input_hash, @@ -2300,8 +2300,8 @@ echo "Docker image priming complete.""#, let Some(perms) = image.and_then(|img| config.resolve_image_permissions(img)) else { return String::new(); }; - let users = mapping_from_hashmap(perms.users.as_ref()); - let groups = mapping_from_hashmap(perms.groups.as_ref()); + let users = mapping_from_map(perms.users.as_ref()); + let groups = mapping_from_map(perms.groups.as_ref()); render_users_groups_script(users.as_ref(), groups.as_ref(), etc_dir, None) }; diff --git a/src/utils/config.rs b/src/utils/config.rs index e83cc522..967454f4 100644 --- a/src/utils/config.rs +++ b/src/utils/config.rs @@ -2,7 +2,7 @@ use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::env; use std::fs; use std::path::{Path, PathBuf}; @@ -835,10 +835,16 @@ pub struct ImageConfig { /// (rootfs or initramfs). Values are kept as raw YAML so the existing /// dynamic field parser in [`crate::utils::permissions`] can consume them /// without re-typing every shadow attribute. +/// +/// `BTreeMap` (not `HashMap`) so users/groups are provisioned in a stable, +/// key-sorted order: `render_users_groups_script` appends to /etc/passwd and +/// /etc/group in iteration order and auto-assigns UID/GID in that order, so a +/// random per-process `HashMap` order would otherwise reshuffle the generated +/// script — and the auto-assigned UIDs — on every build. #[derive(Debug, Clone, Deserialize, Serialize, Default)] pub struct PermissionsConfig { - pub users: Option>, - pub groups: Option>, + pub users: Option>, + pub groups: Option>, } /// Provision profile configuration diff --git a/src/utils/permissions.rs b/src/utils/permissions.rs index cc746679..0795c794 100644 --- a/src/utils/permissions.rs +++ b/src/utils/permissions.rs @@ -372,13 +372,16 @@ echo "Set proper permissions on authentication files""# script_lines.join("") } -/// Convert an `Option<&HashMap>` (the shape +/// Convert an `Option<&BTreeMap>` (the shape /// stored in [`crate::utils::config::PermissionsConfig`]) into an owned -/// `serde_yaml::Mapping` ref appropriate for [`render_users_groups_script`]. +/// `serde_yaml::Mapping` appropriate for [`render_users_groups_script`]. +/// +/// `BTreeMap` iterates in key order, so the resulting `Mapping` — and the +/// users/groups the script then provisions — are deterministic across builds. /// /// Returns `None` if the input is `None` or empty. -pub fn mapping_from_hashmap( - src: Option<&std::collections::HashMap>, +pub fn mapping_from_map( + src: Option<&std::collections::BTreeMap>, ) -> Option { let map = src?; if map.is_empty() { @@ -473,4 +476,30 @@ mod tests { assert!(script.contains("Creating group 'docker'")); assert!(script.contains("chown root:root \"/etc/passwd\"")); } + + #[test] + fn mapping_from_map_provisions_in_sorted_order() { + // ENG-2441: users/groups come from a BTreeMap, so mapping_from_map must + // preserve key-sorted order — the generated passwd appends and the + // auto-assigned UIDs follow it, and a stable order is what stops the + // build id (and any UID a user omits) from churning every build. + let mut users = std::collections::BTreeMap::new(); + users.insert("zeta".to_string(), user("")); + users.insert("alpha".to_string(), user("")); + users.insert("mike".to_string(), user("")); + + let mapping = mapping_from_map(Some(&users)).expect("non-empty map yields Some"); + let keys: Vec<&str> = mapping.iter().filter_map(|(k, _)| k.as_str()).collect(); + assert_eq!(keys, ["alpha", "mike", "zeta"]); + + let script = render_users_groups_script(Some(&mapping), None, "/etc", None); + let a = script.find("Creating user 'alpha'").unwrap(); + let m = script.find("Creating user 'mike'").unwrap(); + let z = script.find("Creating user 'zeta'").unwrap(); + assert!(a < m && m < z, "users must be provisioned in sorted order"); + + // Empty / absent maps still fold to nothing. + assert!(mapping_from_map(None).is_none()); + assert!(mapping_from_map(Some(&std::collections::BTreeMap::new())).is_none()); + } }