From ea0df4fcf97f7bcef2f34ef2d9ccc12d69cc7545 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 17 Dec 2025 16:36:30 -0500 Subject: [PATCH] build-sys: Consistently use `RUN --network=none` and add check Ensure all RUN instructions after the "external dependency cutoff point" marker include `--network=none` right after `RUN`. This enforces that external dependencies are clearly delineated in the early stages of the Dockerfile. The check is part of `cargo xtask check-buildsys` and includes unit tests. Assisted-by: OpenCode (Sonnet 4) Signed-off-by: Colin Walters --- Dockerfile | 17 ++-- crates/xtask/src/buildsys.rs | 165 +++++++++++++++++++++++++++++++++++ crates/xtask/src/xtask.rs | 48 +--------- 3 files changed, 177 insertions(+), 53 deletions(-) create mode 100644 crates/xtask/src/buildsys.rs diff --git a/Dockerfile b/Dockerfile index 8e98021a..ca821407 100644 --- a/Dockerfile +++ b/Dockerfile @@ -65,8 +65,12 @@ ENV container=oci STOPSIGNAL SIGRTMIN+3 CMD ["/sbin/init"] +# ------------- +# external dependency cutoff point: # NOTE: Every RUN instruction past this point should use `--network=none`; we want to ensure # all external dependencies are clearly delineated. +# This is verified in `cargo xtask check-buildsys`. +# ------------- FROM buildroot as build # Version for RPM build (optional, computed from git in Justfile) @@ -75,7 +79,7 @@ ARG pkgversion ARG SOURCE_DATE_EPOCH ENV SOURCE_DATE_EPOCH=${SOURCE_DATE_EPOCH} # Build RPM directly from source, using cached target directory -RUN --mount=type=cache,target=/src/target --mount=type=cache,target=/var/roothome --network=none RPM_VERSION="${pkgversion}" /src/contrib/packaging/build-rpm +RUN --network=none --mount=type=cache,target=/src/target --mount=type=cache,target=/var/roothome RPM_VERSION="${pkgversion}" /src/contrib/packaging/build-rpm FROM buildroot as sdboot-signed # The secureboot key and cert are passed via Justfile @@ -91,11 +95,11 @@ FROM build as units # A place that we're more likely to be able to set xattrs VOLUME /var/tmp ENV TMPDIR=/var/tmp -RUN --mount=type=cache,target=/src/target --mount=type=cache,target=/var/roothome --network=none make install-unit-tests +RUN --network=none --mount=type=cache,target=/src/target --mount=type=cache,target=/var/roothome make install-unit-tests # This just does syntax checking FROM buildroot as validate -RUN --mount=type=cache,target=/src/target --mount=type=cache,target=/var/roothome --network=none make validate +RUN --network=none --mount=type=cache,target=/src/target --mount=type=cache,target=/var/roothome make validate # Common base for final images: configures variant, rootfs, and injects extra content FROM base as final-common @@ -105,13 +109,12 @@ RUN --network=none --mount=type=bind,from=packaging,target=/run/packaging \ --mount=type=bind,from=sdboot-signed,target=/run/sdboot-signed \ /run/packaging/configure-variant "${variant}" ARG rootfs="" -RUN --mount=type=bind,from=packaging,target=/run/packaging /run/packaging/configure-rootfs "${variant}" "${rootfs}" +RUN --network=none --mount=type=bind,from=packaging,target=/run/packaging /run/packaging/configure-rootfs "${variant}" "${rootfs}" COPY --from=packaging /usr-extras/ /usr/ # Final target: installs pre-built packages from /run/packages volume mount. # Use with: podman build --target=final -v path/to/packages:/run/packages:ro FROM final-common as final -RUN --mount=type=bind,from=packaging,target=/run/packaging \ - --network=none \ +RUN --network=none --mount=type=bind,from=packaging,target=/run/packaging \ /run/packaging/install-rpm-and-setup /run/packages -RUN bootc container lint --fatal-warnings +RUN --network=none bootc container lint --fatal-warnings diff --git a/crates/xtask/src/buildsys.rs b/crates/xtask/src/buildsys.rs new file mode 100644 index 00000000..9f26a288 --- /dev/null +++ b/crates/xtask/src/buildsys.rs @@ -0,0 +1,165 @@ +//! Build system validation checks. + +use std::collections::BTreeMap; + +use anyhow::{Context, Result}; +use camino::{Utf8Path, Utf8PathBuf}; +use fn_error_context::context; +use xshell::{cmd, Shell}; + +const DOCKERFILE_NETWORK_CUTOFF: &str = "external dependency cutoff point"; + +/// Check build system properties +/// +/// - Reproducible builds for the RPM +/// - Dockerfile network isolation after cutoff point +#[context("Checking build system")] +pub fn check_buildsys(sh: &Shell, dockerfile_path: &Utf8Path) -> Result<()> { + check_package_reproducibility(sh)?; + check_dockerfile_network_isolation(dockerfile_path)?; + Ok(()) +} + +/// Verify that consecutive `just package` invocations produce identical RPM checksums. +#[context("Checking package reproducibility")] +fn check_package_reproducibility(sh: &Shell) -> Result<()> { + println!("Checking reproducible builds..."); + // Helper to compute SHA256 of bootc RPMs in target/packages/ + fn get_rpm_checksums(sh: &Shell) -> Result> { + // Find bootc*.rpm files in target/packages/ + let packages_dir = Utf8Path::new("target/packages"); + let mut rpm_files: Vec = Vec::new(); + for entry in std::fs::read_dir(packages_dir).context("Reading target/packages")? { + let entry = entry?; + let path = Utf8PathBuf::try_from(entry.path())?; + if path.extension() == Some("rpm") { + rpm_files.push(path); + } + } + + assert!(!rpm_files.is_empty()); + + let mut checksums = BTreeMap::new(); + for rpm_path in &rpm_files { + let output = cmd!(sh, "sha256sum {rpm_path}").read()?; + let (hash, filename) = output + .split_once(" ") + .with_context(|| format!("failed to parse sha256sum output: '{}'", output))?; + checksums.insert(filename.to_owned(), hash.to_owned()); + } + Ok(checksums) + } + + cmd!(sh, "just package").run()?; + let first_checksums = get_rpm_checksums(sh)?; + cmd!(sh, "just package").run()?; + let second_checksums = get_rpm_checksums(sh)?; + + itertools::assert_equal(first_checksums, second_checksums); + println!("ok package reproducibility"); + + Ok(()) +} + +/// Verify that all RUN instructions in the Dockerfile after the network cutoff +/// point include `--network=none`. +#[context("Checking Dockerfile network isolation")] +fn check_dockerfile_network_isolation(dockerfile_path: &Utf8Path) -> Result<()> { + println!("Checking Dockerfile network isolation..."); + let dockerfile = std::fs::read_to_string(dockerfile_path).context("Reading Dockerfile")?; + verify_dockerfile_network_isolation(&dockerfile)?; + println!("ok Dockerfile network isolation"); + Ok(()) +} + +const RUN_NETWORK_NONE: &str = "RUN --network=none"; + +/// Verify that all RUN instructions after the network cutoff marker start with +/// `RUN --network=none`. +/// +/// Returns Ok(()) if all RUN instructions comply, or an error listing violations. +pub fn verify_dockerfile_network_isolation(dockerfile: &str) -> Result<()> { + // Find the cutoff point + let cutoff_line = dockerfile + .lines() + .position(|line| line.contains(DOCKERFILE_NETWORK_CUTOFF)) + .ok_or_else(|| { + anyhow::anyhow!( + "Dockerfile missing '{}' marker comment", + DOCKERFILE_NETWORK_CUTOFF + ) + })?; + + // Check all RUN instructions after the cutoff point + let mut errors = Vec::new(); + + for (idx, line) in dockerfile.lines().enumerate().skip(cutoff_line + 1) { + let line_num = idx + 1; // 1-based line numbers + let trimmed = line.trim(); + + // Check if this is a RUN instruction + if trimmed.starts_with("RUN ") { + // Must start with exactly "RUN --network=none" + if !trimmed.starts_with(RUN_NETWORK_NONE) { + errors.push(format!( + " line {}: RUN instruction must start with `{}`", + line_num, RUN_NETWORK_NONE + )); + } + } + } + + if !errors.is_empty() { + anyhow::bail!( + "Dockerfile has RUN instructions after '{}' that don't start with `{}`:\n{}", + DOCKERFILE_NETWORK_CUTOFF, + RUN_NETWORK_NONE, + errors.join("\n") + ); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_network_isolation_valid() { + let dockerfile = r#" +FROM base +RUN echo "before cutoff, no network restriction needed" +# external dependency cutoff point +RUN --network=none echo "good" +RUN --network=none --mount=type=bind,from=foo,target=/bar some-command +"#; + verify_dockerfile_network_isolation(dockerfile).unwrap(); + } + + #[test] + fn test_network_isolation_missing_flag() { + let dockerfile = r#" +FROM base +# external dependency cutoff point +RUN --network=none echo "good" +RUN echo "bad - missing network flag" +"#; + let err = verify_dockerfile_network_isolation(dockerfile).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("line 5"), "error should mention line 5: {msg}"); + } + + #[test] + fn test_network_isolation_wrong_position() { + // --network=none must come immediately after RUN + let dockerfile = r#" +FROM base +# external dependency cutoff point +RUN --mount=type=bind,from=foo,target=/bar --network=none echo "bad" +"#; + let err = verify_dockerfile_network_isolation(dockerfile).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("line 4"), "error should mention line 4: {msg}"); + } +} diff --git a/crates/xtask/src/xtask.rs b/crates/xtask/src/xtask.rs index 97b267c6..6921a681 100644 --- a/crates/xtask/src/xtask.rs +++ b/crates/xtask/src/xtask.rs @@ -14,6 +14,7 @@ use clap::{Args, Parser, Subcommand}; use fn_error_context::context; use xshell::{cmd, Shell}; +mod buildsys; mod man; mod tmt; @@ -137,7 +138,7 @@ fn try_main() -> Result<()> { Commands::Spec => spec(&sh), Commands::RunTmt(args) => tmt::run_tmt(&sh, &args), Commands::TmtProvision(args) => tmt::tmt_provision(&sh, &args), - Commands::CheckBuildsys => check_buildsys(&sh), + Commands::CheckBuildsys => buildsys::check_buildsys(&sh, "Dockerfile".into()), } } @@ -405,48 +406,3 @@ fn update_generated(sh: &Shell) -> Result<()> { Ok(()) } - -/// Check build system properties -/// -/// - Reproducible builds for the RPM -#[context("Checking build system")] -fn check_buildsys(sh: &Shell) -> Result<()> { - use std::collections::BTreeMap; - - println!("Checking reproducible builds..."); - // Helper to compute SHA256 of bootc RPMs in target/packages/ - fn get_rpm_checksums(sh: &Shell) -> Result> { - // Find bootc*.rpm files in target/packages/ - let packages_dir = Utf8Path::new("target/packages"); - let mut rpm_files: Vec = Vec::new(); - for entry in std::fs::read_dir(packages_dir).context("Reading target/packages")? { - let entry = entry?; - let path = Utf8PathBuf::try_from(entry.path())?; - if path.extension() == Some("rpm") { - rpm_files.push(path); - } - } - - assert!(!rpm_files.is_empty()); - - let mut checksums = BTreeMap::new(); - for rpm_path in &rpm_files { - let output = cmd!(sh, "sha256sum {rpm_path}").read()?; - let (hash, filename) = output - .split_once(" ") - .with_context(|| format!("failed to parse sha256sum output: '{}'", output))?; - checksums.insert(filename.to_owned(), hash.to_owned()); - } - Ok(checksums) - } - - cmd!(sh, "just package").run()?; - let first_checksums = get_rpm_checksums(sh)?; - cmd!(sh, "just package").run()?; - let second_checksums = get_rpm_checksums(sh)?; - - itertools::assert_equal(first_checksums, second_checksums); - println!("ok package reproducibility"); - - Ok(()) -}