From a300f2496723501ec72b05261cbd15fa76250b87 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Thu, 5 Nov 2026 17:36:40 -0500 Subject: [PATCH 01/11] feat(native): build images in the guest engine with runtime build-image Adds the runtime operation that builds one Compose-style image inside the native VM's engine with the pinned host BuildKit client, and the isolated build network it needs. Native up does not call it yet. - Egress pools give dockerd a fixed default bridge (10.213.255.0/24, icc=false) instead of --bridge=none; isolated pools keep none. guest-build-network.sh isolates it after the engine starts (drops build traffic to the guest, the host gateway and link-local metadata; allows DNS to the guest resolvers) and writes a boot-bound readiness marker. The engine client now refuses any container create that does not name an owned network, none or a container namespace, and any connect to the default bridge, so only BuildKit RUN steps can use it. - runtime build-image --request --json: paths resolve inside the project without symlinks; the Dockerfile is read (never run) and scanned; every base image resolves to a digest-pinned manifest and is handed to BuildKit as a named build context; base images with stored ONBUILD triggers refuse. The build key covers namespace (project root + branch), paths, Dockerfile and .dockerignore bytes, target, salted argument hashes and builder/engine versions; context contents are not hashed (Compose parity). - Argument values reach the client only through its environment (--build-arg NAME); names that would change the client's own environment refuse. Labels carry a digest of the pool incarnation, never the token. - The provider lease is held only for admission and adoption, never while BuildKit runs. A shared build fence makes runtime down refuse instead of stopping the engine under a build. A per-key lock serializes builds; a building receipt found under it is a dead attempt and is recorded indeterminate before building again. - Adoption requires the same incarnation, boot and engine as admission, the metadata's config digest, linux/arm64, exactly the owned labels, no other reserved ownership labels and no image VOLUMEs. Reuse requires a verified receipt from the same checkout, incarnation and engine whose image still inspects that way. - SIGINT/SIGTERM/SIGHUP and the time budget (default 30 min, max 2 h) stop the client's whole process group (SIGTERM, then SIGKILL after 10 s). Refs HACK-1270 Co-Authored-By: Claude Opus 5.5 --- docs/guides/native-candidate.md | 29 + packages/runtime-core/README.md | 11 +- packages/runtime-core/src/main.rs | 8 + packages/runtime-core/src/project/mod.rs | 1 - .../runtime-core/src/provider/artifact.rs | 5 + packages/runtime-core/src/provider/engine.rs | 67 + .../src/provider/guest-build-network.sh | 33 + .../runtime-core/src/provider/guest-daemon.sh | 13 +- .../runtime-core/src/provider/image_build.rs | 1141 +++++++++++++++++ .../src/provider/image_build/tests.rs | 744 +++++++++++ .../runtime-core/src/provider/lifecycle.rs | 17 + packages/runtime-core/src/provider/mod.rs | 1 + .../src/provider/registry_image.rs | 102 +- 13 files changed, 2157 insertions(+), 15 deletions(-) create mode 100644 packages/runtime-core/src/provider/guest-build-network.sh create mode 100644 packages/runtime-core/src/provider/image_build.rs create mode 100644 packages/runtime-core/src/provider/image_build/tests.rs diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index a220a37f0..74caf19b2 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -457,6 +457,35 @@ state and checks its code signature. If a prepared client exists, the command ch it again and does not replace it. If the prepared tree changes, `runtime probe` refuses; it reports `builder_prepared` when the client is present and unchanged. +`runtime build-image --request --json` builds one image in the running +runtime's engine with that client. It is a building block: native `up` still refuses +`build:` services and does not call it. The request is a private JSON file (a singly +linked, mode-`0600` file owned by you), because build-argument values are in it: + +```json +{"version": 1, "project_root": "/absolute/project", "context": "app", + "dockerfile": "Dockerfile", "target": null, "args": {"NAME": "literal"}, + "policy": "missing", "timeout_seconds": 1800} +``` + +- The context and Dockerfile must be inside the project and reached without symlinks. + The Dockerfile is read, never run, to find every image it can pull. Each image must + be a public Docker Hub reference; it is pinned to a digest before the build starts. + Base images with stored `ONBUILD` triggers refuse. +- Argument values reach the client only through its environment, never its command + line, receipts or output. Names that would change the client's own environment + (`PATH`, `HOME`, `DOCKER_*`, `BUILDX_*`, `BUILDKIT_*`, proxies, locale, Git and SSH + settings) refuse. +- `missing` reuses a verified image for the same build key without running the client; + `rebuild` always builds. The key covers the Dockerfile and `.dockerignore` bytes, + target, arguments and versions, not the context's other files (as with Compose). +- The runtime must have network egress and the isolated build network that a runtime + started by this version sets up. An older runtime refuses with + `native_build_runtime_restart`: stop it and start it again. +- `runtime down` refuses while a build runs. Ctrl-C or SIGTERM cancels the build and + stops the client's whole process group; so does the time budget (default 30 minutes, + at most 2 hours). + Before startup, network-tool preparation additionally requires all four pinned Alpine aarch64 APKs in a current-user-owned private directory (mode `0700`), with regular, singly linked mode-`0600` files. Acquire the matching public package bytes; diff --git a/packages/runtime-core/README.md b/packages/runtime-core/README.md index 9633d7895..db8af2a41 100644 --- a/packages/runtime-core/README.md +++ b/packages/runtime-core/README.md @@ -478,8 +478,15 @@ The default private build includes native HTTP graph probes (local `_docs/docs/p and requires Zig 0.15.2 plus a host C compiler. [Provider pins](provider-pins.json) record the exact package inputs; the engine runs inside the VM. The pinned host BuildKit client (`runtime prepare-builder`) and the pure Dockerfile -image-reference scanner (`provider/build_scan.rs`) prepare for native image builds; -no command builds images yet. +image-reference scanner (`provider/build_scan.rs`) back `runtime build-image` +(`provider/image_build.rs`), which builds one Compose-style image in the guest engine +from a private request file. Its build key, receipts, leases, the `runtime down` +build fence, cancellation and timeout rules are documented in that module. Egress +pools give dockerd a fixed default bridge (`10.213.255.0/24`, `icc=false`) that only +BuildKit `RUN` steps use; `guest-build-network.sh` isolates it from the guest, the host +gateway and link-local metadata, and the engine client refuses any container create +that does not name an owned network or `none`. Native `up` does not call the build +operation yet. Source binaries embed their build checkout. If a copied Cargo target directory retains a different checkout identity, use a fresh `--target-dir` for checks and diff --git a/packages/runtime-core/src/main.rs b/packages/runtime-core/src/main.rs index c52f4e015..a10acd084 100644 --- a/packages/runtime-core/src/main.rs +++ b/packages/runtime-core/src/main.rs @@ -102,6 +102,7 @@ Usage: hack-local runtime prepared-base status --profile research|development [--store ] [--json] hack-local runtime prepared-base remove --base-id [--store ] [--json] hack-local runtime admit-image --reference <[namespace/]repository[:tag][@sha256:digest]|sha256:image-id> --json + hack-local runtime build-image --request --json hack-local runtime ensure-image --reference <[namespace/]repository[:tag]|[namespace/]repository@sha256:digest> --json hack-local runtime resolve-image --reference <[namespace/]repository[:tag]> --json hack-local runtime fetch-image --reference <[namespace/]repository@sha256:digest> --archive [--json] @@ -948,6 +949,13 @@ fn run() -> Result<(), CandidateError> { reference, )?)?; } + ["runtime", "build-image", "--request", request, "--json"] => { + // Build output streams to stderr; only the result is printed on stdout. + print_json(&hack_runtime_core::provider::image_build::build_image( + &discover_candidate(&requested)?, + Path::new(request), + )?)?; + } ["runtime", "admit-image", "--reference", reference, "--json"] => { // Pure: no candidate discovery, network, credentials or runtime effects. print_json(&hack_runtime_core::provider::registry_image::admit( diff --git a/packages/runtime-core/src/project/mod.rs b/packages/runtime-core/src/project/mod.rs index 043076259..0d6b6b04c 100644 --- a/packages/runtime-core/src/project/mod.rs +++ b/packages/runtime-core/src/project/mod.rs @@ -23,7 +23,6 @@ use std::collections::BTreeMap; use std::path::{Path, PathBuf}; pub use enrollment::{EnrollmentReceipt, enroll, status, status_with_branch}; -#[cfg(feature = "native-config-plan")] pub(crate) use source::resolve as resolve_source; pub use source::{SourceEntry, SourceSelection}; diff --git a/packages/runtime-core/src/provider/artifact.rs b/packages/runtime-core/src/provider/artifact.rs index 1be865b98..b68405f09 100644 --- a/packages/runtime-core/src/provider/artifact.rs +++ b/packages/runtime-core/src/provider/artifact.rs @@ -311,6 +311,11 @@ pub fn prepare_builder(candidate: &Candidate, binary: &Path) -> Result PathBuf { + binary_root(candidate, &BUILDER).join(BUILDER.executable) +} + /// Verify the prepared BuildKit client before any use. pub fn verify_builder(candidate: &Candidate) -> Result { verify_binary(candidate, &BUILDER) diff --git a/packages/runtime-core/src/provider/engine.rs b/packages/runtime-core/src/provider/engine.rs index e0299cd9c..bbb9ac86f 100644 --- a/packages/runtime-core/src/provider/engine.rs +++ b/packages/runtime-core/src/provider/engine.rs @@ -94,6 +94,7 @@ impl Transport { { return Err(failure("Invalid private engine request path.")); } + require_owned_network(&method, path, body)?; let requires_admission = ![Method::GET, Method::HEAD, Method::DELETE].contains(&method); let mut request = self .client @@ -127,6 +128,40 @@ impl Transport { } } +/// Every container Hack creates names its network explicitly: an owned network, `none` +/// or another container's namespace. Egress pools give the engine a default bridge that +/// only BuildKit `RUN` steps may use (guest-daemon.sh); an unset, `default`, `bridge` +/// or `host` mode, or a connect to the default bridge, would put a container on it or +/// in the guest root namespace, so both refuse before dispatch. +fn require_owned_network( + method: &Method, + path: &str, + body: Option<&Value>, +) -> Result<(), CandidateError> { + if *method != Method::POST { + return Ok(()); + } + let route = path.split('?').next().unwrap_or(path); + let refused = || { + CandidateError::new( + "engine_network_refused", + "Containers must name an owned network or none; the engine default bridge is reserved for image builds.", + ) + }; + if route.ends_with("/containers/create") { + let mode = body + .and_then(|body| body["HostConfig"]["NetworkMode"].as_str()) + .unwrap_or(""); + if mode.is_empty() || ["default", "bridge", "host"].contains(&mode) { + return Err(refused()); + } + } + if route.ends_with("/networks/bridge/connect") { + return Err(refused()); + } + Ok(()) +} + // One minimum budget reaches dispatch. A later explicit deadline cannot replace // an earlier mutation-admission deadline; cleanup admission exemptions stay above. fn request_timeout( @@ -691,6 +726,38 @@ mod tests { use std::io::Write; use std::os::unix::net::UnixListener; + #[test] + fn containers_must_name_their_network_and_never_join_the_build_bridge() { + let create = |mode: Value| { + require_owned_network( + &Method::POST, + "/v1.53/containers/create?name=x", + Some(&serde_json::json!({"HostConfig": {"NetworkMode": mode}})), + ) + }; + for refused in [ + Value::Null, + serde_json::json!(""), + serde_json::json!("default"), + serde_json::json!("bridge"), + serde_json::json!("host"), + serde_json::json!(7), + ] { + assert_eq!(create(refused).unwrap_err().code, "engine_network_refused"); + } + for allowed in ["none", "hack-local-owned", "container:abc"] { + assert!(create(serde_json::json!(allowed)).is_ok()); + } + assert!(require_owned_network(&Method::POST, "/v1.53/containers/create", None).is_err()); + assert!( + require_owned_network(&Method::POST, "/v1.53/networks/bridge/connect", None).is_err() + ); + assert!( + require_owned_network(&Method::POST, "/v1.53/networks/owned/connect", None).is_ok() + ); + assert!(require_owned_network(&Method::GET, "/v1.53/networks/bridge", None).is_ok()); + } + #[test] fn explicit_request_budget_never_extends_mutation_admission() { let now = Instant::now(); diff --git a/packages/runtime-core/src/provider/guest-build-network.sh b/packages/runtime-core/src/provider/guest-build-network.sh new file mode 100644 index 000000000..c78c09ed7 --- /dev/null +++ b/packages/runtime-core/src/provider/guest-build-network.sh @@ -0,0 +1,33 @@ +set -eu +# Isolate the build bridge (docker0) that only BuildKit `RUN` steps use. Docker's +# own isolation chains already drop docker0 <-> owned project bridges; these rules +# also drop build traffic to the guest itself (relays, sockets, the engine), to +# the host gateway and to link-local metadata, while allowing DNS to the guest's +# configured resolvers. External reach is then only what the VM boundary allows. +# Arguments: network mode, guest boot ID. The marker binds the rules to this boot. +case "${1-}" in + isolated) exit 0 ;; + approved-hosts|internet|host-gateway) ;; + *) exit 64 ;; +esac +test -d /sys/class/net/docker0 +marker=/run/hack-local/build-network +for chain in HACK-BUILD-FORWARD HACK-BUILD-INPUT; do + iptables -N "$chain" 2>/dev/null || iptables -F "$chain" +done +for resolver in $(awk '$1 == "nameserver" { print $2 }' /etc/resolv.conf); do + case "$resolver" in *:*|127.*) continue ;; esac + for protocol in udp tcp; do + iptables -A HACK-BUILD-FORWARD -d "$resolver" -p "$protocol" --dport 53 -j RETURN + iptables -A HACK-BUILD-INPUT -d "$resolver" -p "$protocol" --dport 53 -j ACCEPT + done +done +iptables -A HACK-BUILD-FORWARD -d 100.96.0.1 -j DROP +iptables -A HACK-BUILD-FORWARD -d 169.254.0.0/16 -j DROP +iptables -A HACK-BUILD-INPUT -j DROP +iptables -C DOCKER-USER -i docker0 -j HACK-BUILD-FORWARD 2>/dev/null || + iptables -I DOCKER-USER 1 -i docker0 -j HACK-BUILD-FORWARD +iptables -C INPUT -i docker0 -j HACK-BUILD-INPUT 2>/dev/null || + iptables -I INPUT 1 -i docker0 -j HACK-BUILD-INPUT +printf 'build-network-v1 %s\n' "$2" > "$marker.pending" +mv "$marker.pending" "$marker" diff --git a/packages/runtime-core/src/provider/guest-daemon.sh b/packages/runtime-core/src/provider/guest-daemon.sh index 68fc19f53..017c004af 100644 --- a/packages/runtime-core/src/provider/guest-daemon.sh +++ b/packages/runtime-core/src/provider/guest-daemon.sh @@ -1,12 +1,17 @@ set -eu +# Egress pools give dockerd a default bridge with a fixed range outside Docker's +# default address pools. Only BuildKit `RUN` steps use it: graph containers always +# name an owned network or `none`, and the engine client refuses any other create. +# guest-build-network.sh isolates it after the engine starts. Isolated pools keep +# no default bridge, so builds refuse there. case "${1-}" in - isolated) gateway_rules=false; set -- ;; - approved-hosts|internet) gateway_rules=true; set -- ;; - host-gateway) gateway_rules=true; set -- --host-gateway-ip=100.96.0.1 ;; + isolated) gateway_rules=false; set -- --bridge=none ;; + approved-hosts|internet) gateway_rules=true; set -- --bip=10.213.255.1/24 --icc=false ;; + host-gateway) gateway_rules=true; set -- --bip=10.213.255.1/24 --icc=false --host-gateway-ip=100.96.0.1 ;; *) exit 64 ;; esac dockerd --storage-driver=overlay2 --data-root=/var/lib/docker \ - --bridge=none --iptables="$gateway_rules" --ip6tables=false \ + --iptables="$gateway_rules" --ip6tables=false \ --ip-forward="$gateway_rules" --ip-masq="$gateway_rules" "$@" \ --exec-root=/run/hack-local/exec --pidfile=/run/hack-local/docker.pid \ --host=unix:///run/hack-local/docker.sock --log-level=warn \ diff --git a/packages/runtime-core/src/provider/image_build.rs b/packages/runtime-core/src/provider/image_build.rs new file mode 100644 index 000000000..ff6268d46 --- /dev/null +++ b/packages/runtime-core/src/provider/image_build.rs @@ -0,0 +1,1141 @@ +//! Native Compose `build:` images: `runtime build-image`. +//! +//! One operation builds one image for one build key inside the guest engine, through +//! the pinned host BuildKit client (`docker-buildx`, docker driver) talking to the +//! engine's verified forwarded socket. The built image lands in the guest image store, +//! where graph admission already looks, and the caller pins the service to the content +//! ID it returns, so review, receipts and restore treat it like a pulled image. +//! +//! Authority and ownership: +//! +//! - The build key binds the local inputs: namespace (project root and branch), context +//! and Dockerfile paths, the Dockerfile and `.dockerignore` bytes, target, salted +//! argument hashes, platform, and the builder and engine versions. Context file +//! contents are not hashed, as with Compose: an existing image is reused until an +//! explicit rebuild. Base-image tags resolve only when a build runs. +//! - Every image the Dockerfile can pull (see [`super::build_scan`]) resolves to a digest +//! before BuildKit runs and is handed to it as a named build context, so BuildKit pulls +//! only those exact manifests. Base images with stored `ONBUILD` triggers refuse. +//! - The provider lease is held only for short admission and adoption steps, never while +//! BuildKit runs. A shared build fence (`run/image-builds/active.lock`), taken under the +//! lease and held for the whole operation, makes `runtime down` refuse instead of +//! stopping the engine under a running build. +//! - A receipt `run/image-builds//.json` records the attempt. Only a +//! `content-id-verified` receipt from the same checkout, provider incarnation and +//! engine, whose image still inspects with exactly the owned labels, is reused. A +//! `building` receipt found under the per-key lock belongs to a dead attempt (the lock +//! is released only when its holder exits): it is recorded `indeterminate` and the +//! image is built again. Build outputs are content addressed and adoption always +//! rechecks labels and incarnation, so no operator action is needed. +//! - Argument values never appear in argv, receipts, messages or output: the client +//! receives `--build-arg NAME` and reads the value from its private environment. +//! - Nothing here removes images, prunes caches or adopts an image whose labels differ. +use super::{ + artifact, + build_scan::{self, BaseImage}, + registry_image::{self, BuildBase}, + state, +}; +use crate::{Candidate, CandidateError}; +use serde::{Deserialize, Serialize}; +use serde_json::Value; +use sha2::{Digest, Sha256}; +use std::{ + collections::BTreeMap, + fs::{self, File, OpenOptions}, + io::Read, + os::fd::AsRawFd, + os::unix::fs::{MetadataExt, OpenOptionsExt}, + os::unix::process::CommandExt, + path::{Path, PathBuf}, + process::{Child, Stdio}, + sync::atomic::{AtomicBool, Ordering}, + time::{Duration, Instant}, +}; + +#[cfg(test)] +mod tests; + +/// The default bridge range guest-daemon.sh gives egress pools; only builds use it. +pub(super) const BUILD_NETWORK_SUBNET: &str = "10.213.255.0/24"; +const BUILD_NETWORK_MARKER: &str = "build-network-v1"; +/// Repository for every Hack-built image; review never accepts it as an authored image. +pub const BUILD_REPOSITORY: &str = "hack-local-build"; +const KIND: &str = "build-image"; +const DEFAULT_TIMEOUT_SECONDS: u64 = 30 * 60; +const MAX_TIMEOUT_SECONDS: u64 = 2 * 60 * 60; +const CANCEL_GRACE: Duration = Duration::from_secs(10); +const POLL: Duration = Duration::from_millis(50); +const LEASE_WAIT: Duration = Duration::from_secs(5); +const MAX_REQUEST_BYTES: u64 = 192 * 1024; +const MAX_RECEIPT_BYTES: u64 = 16 * 1024; +const MAX_METADATA_BYTES: u64 = 1024 * 1024; +const MAX_IGNORE_BYTES: u64 = 1024 * 1024; +const MAX_ARGS: usize = 64; +const MAX_ARG_BYTES: usize = 64 * 1024; +/// Labels a Dockerfile (or a base image) must not set on an image Hack runs: the same +/// reserved ownership namespaces the frontend refuses on authored services. +const RESERVED_LABEL_PREFIXES: [&str; 5] = [ + "com.docker.compose.", + "io.hack-local.", + "io.hack.native-config.", + "io.hack.native-file.", + "io.hack.storage-witness.", +]; + +fn refusal(code: &'static str, message: &str) -> CandidateError { + CandidateError::new(code, message) +} +fn invalid(message: &str) -> CandidateError { + refusal("native_build_request", message) +} +fn sha256_hex(bytes: &[u8]) -> String { + format!("{:x}", Sha256::digest(bytes)) +} +fn hex64(value: &str) -> bool { + value.len() == 64 + && value + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) +} +fn image_id(value: &str) -> bool { + value.strip_prefix("sha256:").is_some_and(hex64) +} + +/// When the caller wants a fresh build. +#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "kebab-case")] +pub enum Policy { + /// Reuse a verified image for this build key; build only when it is missing. + Missing, + /// Always build (BuildKit's cache keeps unchanged steps cheap) and re-resolve bases. + Rebuild, +} + +/// The private request file. It is read only from a singly linked 0600 file owned by +/// this user, because argument values are in it. +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct BuildRequest { + pub version: u32, + /// Exact canonical project root; the namespace is derived from it and `branch`. + pub project_root: PathBuf, + #[serde(default)] + pub branch: Option, + /// Build context, relative to the project root. + pub context: String, + /// Dockerfile, relative to the context. + #[serde(default = "default_dockerfile")] + pub dockerfile: String, + #[serde(default)] + pub target: Option, + /// Literal build arguments. Values are secrets for every output purpose. + #[serde(default)] + pub args: BTreeMap, + pub policy: Policy, + #[serde(default)] + pub timeout_seconds: Option, +} +fn default_dockerfile() -> String { + "Dockerfile".into() +} + +/// The result of one `runtime build-image`. +#[derive(Debug, Serialize)] +pub struct BuildOutcome { + pub image_id: String, + pub build_key: String, + pub tag: String, + /// True when a verified image was reused without running the builder. + pub reused: bool, + /// Named build context to the pinned reference it resolved to, for this image. + pub base_pins: BTreeMap, +} + +/// Receipt phases. `building` with no live holder of the key lock is a dead attempt. +#[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] +#[serde(rename_all = "kebab-case")] +pub(super) enum Phase { + Building, + ContentIdVerified, + Failed, + Canceled, + Indeterminate, +} + +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub(super) struct Receipt { + version: u32, + checkout: PathBuf, + namespace: String, + build_key: String, + provider_incarnation: String, + provider_boot_id: String, + engine_id: String, + phase: Phase, + #[serde(default, skip_serializing_if = "Option::is_none")] + image_id: Option, + base_pins: BTreeMap, +} + +/// The pool identity a build is admitted against and must find unchanged at adoption. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(super) struct PoolIdentity { + pub incarnation: String, + pub boot_id: String, + pub engine_id: String, + /// Host path of the verified forwarded engine socket. + pub socket: PathBuf, +} + +/// The effects a build needs from the provider. The live implementation holds the +/// provider lease inside `Lease`; tests substitute a fake pool and a fake builder. +pub(super) trait BuildPool { + type Lease; + /// Take the provider lease, verify the running pool, its build network and engine. + fn lease(&self) -> Result<(Self::Lease, PoolIdentity), CandidateError>; + /// Inspect one image by content ID or tag; `None` when the engine reports it absent. + fn inspect(&self, lease: &Self::Lease, image: &str) -> Result, CandidateError>; + /// Resolve one base image to its pinned manifest. Network only; no lease is held. + fn resolve(&self, base: &BaseImage) -> Result; + /// The verified builder executable. + fn builder(&self) -> Result; +} + +/// Validated local inputs and the build key derived from them. +#[derive(Debug)] +pub(super) struct Inputs { + namespace: String, + key: String, + context: PathBuf, + dockerfile: PathBuf, + target: Option, + args: BTreeMap, + images: Vec, + policy: Policy, + timeout: Duration, +} + +impl Inputs { + fn tag(&self) -> String { + format!( + "{BUILD_REPOSITORY}/{}:{}", + &self.namespace[..12], + &self.key[..32] + ) + } +} + +/// The non-secret owner value an image carries: a digest of the pool incarnation, so +/// the incarnation itself never reaches the client's argv. +pub(super) fn owner_label(incarnation: &str) -> String { + sha256_hex(format!("hack-native-build-owner-v1\0{incarnation}").as_bytes())[..32].to_owned() +} + +fn labels(inputs: &Inputs, identity: &PoolIdentity) -> BTreeMap<&'static str, String> { + BTreeMap::from([ + ("io.hack-local.build-key", inputs.key.clone()), + ("io.hack-local.kind", KIND.to_owned()), + ("io.hack-local.namespace", inputs.namespace.clone()), + ("io.hack-local.owner", owner_label(&identity.incarnation)), + ]) +} + +/// The namespace builds for one project root and branch belong to. Worktrees and +/// branches sharing one pool get distinct namespaces, so images never cross them. +pub(super) fn namespace(root: &Path, branch: Option<&str>) -> String { + let mut digest = Sha256::new(); + digest.update(b"hack-native-build-namespace-v1\0"); + digest.update(root.as_os_str().as_encoded_bytes()); + digest.update(b"\0"); + digest.update(branch.unwrap_or("").as_bytes()); + format!("{:x}", digest.finalize()) +} + +/// Client environment the build must not let an argument name override, plus names +/// the client itself interprets (proxies would redirect the client's own traffic). +fn reserved_argument(name: &str) -> bool { + const EXACT: [&str; 6] = ["PATH", "HOME", "LANG", "TMPDIR", "USER", "SHELL"]; + const PREFIXES: [&str; 3] = ["DOCKER_", "BUILDX_", "BUILDKIT_"]; + let upper = name.to_ascii_uppercase(); + EXACT.contains(&upper.as_str()) + || PREFIXES.iter().any(|prefix| upper.starts_with(prefix)) + || upper.ends_with("_PROXY") + || upper.starts_with("LC_") + || upper.starts_with("GIT_") + || upper.starts_with("SSH_") +} + +fn valid_argument_name(name: &str) -> bool { + let mut bytes = name.bytes(); + name.len() <= 128 + && bytes + .next() + .is_some_and(|b| b.is_ascii_alphabetic() || b == b'_') + && bytes.all(|b| b.is_ascii_alphanumeric() || b == b'_') +} + +fn valid_identifier(value: &str) -> bool { + !value.is_empty() + && value.len() <= 128 + && value.as_bytes()[0].is_ascii_alphanumeric() + && value + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b"_.-".contains(&b)) +} + +fn valid_branch(value: &str) -> bool { + !value.is_empty() + && value.len() <= 63 + && !value.starts_with('-') + && !value.ends_with('-') + && value + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-') +} + +fn validate_request(request: &BuildRequest) -> Result<(), CandidateError> { + if request.version != 1 { + return Err(invalid("Unsupported build request version.")); + } + if !request.project_root.is_absolute() { + return Err(invalid("The project root must be an absolute path.")); + } + if request.branch.as_deref().is_some_and(|b| !valid_branch(b)) { + return Err(invalid( + "The branch selector must be a lowercase DNS label.", + )); + } + if request + .target + .as_deref() + .is_some_and(|t| !valid_identifier(t)) + { + return Err(invalid("Invalid build target name.")); + } + if request.args.len() > MAX_ARGS { + return Err(refusal( + "native_build_argument", + "A native build accepts at most 64 arguments.", + )); + } + let mut total = 0_usize; + for (name, value) in &request.args { + if !valid_argument_name(name) { + return Err(refusal( + "native_build_argument", + "Build argument names must match [A-Za-z_][A-Za-z0-9_]* and be at most 128 bytes.", + )); + } + if reserved_argument(name) { + return Err(refusal( + "native_build_argument", + "Build argument names may not set the build client's own environment (PATH, HOME, DOCKER_*, BUILDX_*, BUILDKIT_*, proxies, locale, Git or SSH settings).", + )); + } + if value.bytes().any(|b| b == 0) { + return Err(refusal( + "native_build_argument", + "Build argument values may not contain NUL bytes.", + )); + } + total += name.len() + value.len(); + } + if total > MAX_ARG_BYTES { + return Err(refusal( + "native_build_argument", + "Build arguments exceed 64 KiB in total.", + )); + } + let timeout = request.timeout_seconds.unwrap_or(DEFAULT_TIMEOUT_SECONDS); + if timeout == 0 || timeout > MAX_TIMEOUT_SECONDS { + return Err(invalid( + "The build timeout must be between 1 and 7200 seconds.", + )); + } + Ok(()) +} + +/// Read one optional bounded regular file without following a final symlink. +fn read_bounded_file(path: &Path, limit: u64) -> Result>, CandidateError> { + let file = match OpenOptions::new() + .read(true) + .custom_flags(libc::O_NOFOLLOW | libc::O_NONBLOCK) + .open(path) + { + Ok(file) => file, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(_) => { + return Err(refusal( + "native_build_source", + "A build input is a symlink or cannot be opened.", + )); + } + }; + let metadata = file.metadata().map_err(state::io)?; + if !metadata.is_file() || metadata.nlink() != 1 || metadata.len() > limit { + return Err(refusal( + "native_build_source", + "Build inputs must be singly linked regular files within their size limits.", + )); + } + let mut bytes = Vec::new(); + file.take(limit + 1) + .read_to_end(&mut bytes) + .map_err(state::io)?; + if bytes.len() as u64 > limit { + return Err(refusal( + "native_build_source", + "A build input grew beyond its size limit.", + )); + } + Ok(Some(bytes)) +} + +/// The ignore files BuildKit applies to this context: a Dockerfile-specific +/// `.dockerignore` wins over the context's `.dockerignore`. +fn ignore_digests(context: &Path, dockerfile: &Path) -> Result { + let mut specific = dockerfile.as_os_str().to_owned(); + specific.push(".dockerignore"); + let digest = |path: &Path| -> Result { + Ok(read_bounded_file(path, MAX_IGNORE_BYTES)? + .map_or(Value::Null, |bytes| Value::String(sha256_hex(&bytes)))) + }; + Ok(serde_json::json!({ + "context": digest(&context.join(".dockerignore"))?, + "dockerfile": digest(Path::new(&specific))?, + })) +} + +/// Validate the request, resolve paths inside the project without following symlinks, +/// read and scan the Dockerfile and derive the build key. Pure local reads. +pub(super) fn prepare(request: BuildRequest) -> Result { + validate_request(&request)?; + let root = fs::canonicalize(&request.project_root).map_err(|_| { + refusal( + "native_build_source", + "The project root does not exist or cannot be inspected.", + ) + })?; + if root != request.project_root || !root.is_dir() { + return Err(refusal( + "native_build_source", + "The project root must be an exact canonical directory.", + )); + } + let context = crate::project::resolve_source(&root, &root, &request.context, false)?; + if !fs::symlink_metadata(&context).is_ok_and(|m| m.is_dir()) { + return Err(refusal( + "native_build_source", + "The build context must be an existing project directory.", + )); + } + let dockerfile = crate::project::resolve_source(&root, &context, &request.dockerfile, false)?; + let bytes = read_bounded_file(&dockerfile, build_scan::MAX_DOCKERFILE_BYTES as u64)? + .ok_or_else(|| { + refusal( + "native_build_source", + "The Dockerfile must be an existing regular project file.", + ) + })?; + let scan = build_scan::scan(&build_scan::ScanRequest { + dockerfile: &bytes, + target: request.target.as_deref(), + args: &request.args, + })?; + let namespace = namespace(&root, request.branch.as_deref()); + let relative = |path: &Path, base: &Path| -> String { + path.strip_prefix(base) + .map(|p| p.to_string_lossy().into_owned()) + .unwrap_or_default() + }; + let salted_args: BTreeMap<&str, String> = request + .args + .iter() + .map(|(name, value)| { + let mut digest = Sha256::new(); + digest.update(namespace.as_bytes()); + digest.update(b"\0"); + digest.update(name.as_bytes()); + digest.update(b"\0"); + digest.update(value.as_bytes()); + (name.as_str(), format!("{:x}", digest.finalize())) + }) + .collect(); + let key_input = serde_json::json!({ + "v": 1, + "namespace": namespace, + "context": relative(&context, &root), + "dockerfile": relative(&dockerfile, &context), + "dockerfile_sha256": sha256_hex(&bytes), + "dockerignore": ignore_digests(&context, &dockerfile)?, + "target": request.target, + "args": salted_args, + "platform": "linux/arm64", + "builder": {"buildx": artifact::BUILDER_VERSION, "engine": artifact::ENGINE_VERSION}, + }); + let key = sha256_hex( + &serde_json::to_vec(&key_input).map_err(|_| invalid("Cannot encode the build key."))?, + ); + Ok(Inputs { + namespace, + key, + context, + dockerfile, + target: request.target, + args: request.args, + images: scan.images, + policy: request.policy, + timeout: Duration::from_secs(request.timeout_seconds.unwrap_or(DEFAULT_TIMEOUT_SECONDS)), + }) +} + +fn builds_root(candidate: &Candidate) -> PathBuf { + candidate.state_root.join("run/image-builds") +} + +/// A cooperative `flock` on a private lock file, shared or exclusive, never blocking. +struct Flock(File); +impl Flock { + fn open(path: &Path) -> Result { + let file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .mode(0o600) + .custom_flags(libc::O_NOFOLLOW) + .open(path) + .map_err(state::io)?; + let metadata = file.metadata().map_err(state::io)?; + // SAFETY: geteuid has no preconditions. + if !metadata.is_file() + || metadata.nlink() != 1 + || metadata.uid() != unsafe { libc::geteuid() } + || metadata.mode() & 0o077 != 0 + { + return Err(refusal("foreign_state", "Unsafe image build lock.")); + } + Ok(file) + } + fn try_lock(file: File, operation: libc::c_int) -> Result, CandidateError> { + // SAFETY: `file` is a live owned descriptor; flock retains no pointers. + if unsafe { libc::flock(file.as_raw_fd(), operation | libc::LOCK_NB) } == 0 { + return Ok(Some(Self(file))); + } + let error = std::io::Error::last_os_error(); + if error.raw_os_error() == Some(libc::EWOULDBLOCK) { + Ok(None) + } else { + Err(state::io(error)) + } + } +} +impl Drop for Flock { + fn drop(&mut self) { + // SAFETY: the descriptor is live until this value drops. + unsafe { + libc::flock(self.0.as_raw_fd(), libc::LOCK_UN); + } + } +} + +/// Refuse to stop the engine while a build operation is admitted. The caller holds the +/// provider lease: a build takes its shared fence only under that lease, so a build +/// cannot start between this check and the caller's effect. +pub(super) fn require_no_active_build(candidate: &Candidate) -> Result<(), CandidateError> { + let path = builds_root(candidate).join("active.lock"); + if !path.try_exists().map_err(state::io)? { + return Ok(()); + } + match Flock::try_lock(Flock::open(&path)?, libc::LOCK_EX)? { + Some(_probe) => Ok(()), + None => Err(refusal( + "provider_busy", + "A native image build is running; let it finish or cancel it, then stop the runtime.", + )), + } +} + +fn read_receipt(path: &Path) -> Result, CandidateError> { + if !path.try_exists().map_err(state::io)? { + return Ok(None); + } + state::read_bounded(path, MAX_RECEIPT_BYTES).map(Some) +} + +fn label_violation(config_labels: &Value, expected: &BTreeMap<&'static str, String>) -> bool { + let labels = match config_labels { + Value::Null => return true, + Value::Object(labels) => labels, + _ => return true, + }; + for (key, value) in expected { + if labels.get(*key).and_then(Value::as_str) != Some(value.as_str()) { + return true; + } + } + labels.keys().any(|key| { + RESERVED_LABEL_PREFIXES + .iter() + .any(|prefix| key.starts_with(prefix)) + && !expected.contains_key(key.as_str()) + }) +} + +/// Whether an inspected image is exactly the one this build key owns. +fn owned_image( + inspected: &Value, + id: &str, + expected: &BTreeMap<&'static str, String>, +) -> Result<(), CandidateError> { + if inspected["Id"] != id || inspected["Architecture"] != "arm64" || inspected["Os"] != "linux" { + return Err(refusal( + "native_build_image", + "The built image identity or platform differs from the build result.", + )); + } + if label_violation(&inspected["Config"]["Labels"], expected) { + return Err(refusal( + "native_build_reserved_label", + "The built image lacks its ownership labels or carries other reserved ownership labels (com.docker.compose.*, io.hack-local.*, io.hack.native-config.*, io.hack.native-file.*, io.hack.storage-witness.*).", + )); + } + match &inspected["Config"]["Volumes"] { + Value::Null => Ok(()), + Value::Object(volumes) if volumes.is_empty() => Ok(()), + _ => Err(refusal( + "native_build_image_volumes", + "The built image declares VOLUME paths; native services refuse implicit image volumes.", + )), + } +} + +/// Run one build operation end to end. See the module documentation for the protocol. +/// `canceled` reports a pending cancellation request; it is checked before each +/// unleased step and while the client runs. +pub(super) fn run( + candidate: &Candidate, + pool: &P, + inputs: &Inputs, + canceled: &dyn Fn() -> bool, +) -> Result { + let canceled_early = || { + refusal( + "native_build_canceled", + "The image build was canceled before the build client started; no image was adopted.", + ) + }; + let root = builds_root(candidate); + state::private_directory(&root)?; + let directory = root.join(&inputs.namespace); + let receipt_path = directory.join(format!("{}.json", inputs.key)); + // Step 1, under the lease: fence, key lock and the reuse decision. + let (lease, identity) = pool.lease()?; + let _fence = Flock::try_lock(Flock::open(&root.join("active.lock"))?, libc::LOCK_SH)? + .ok_or_else(|| { + refusal( + "provider_busy", + "The runtime is stopping; no build was admitted.", + ) + })?; + state::private_directory(&directory)?; + let _key_lock = Flock::try_lock( + Flock::open(&directory.join(format!("{}.lock", inputs.key)))?, + libc::LOCK_EX, + )? + .ok_or_else(|| { + refusal( + "native_build_busy", + "Another operation is building this image; wait for it and retry.", + ) + })?; + let expected = labels(inputs, &identity); + let previous = read_receipt(&receipt_path)?; + if let Some(previous) = &previous { + if previous.version != 1 + || previous.namespace != inputs.namespace + || previous.build_key != inputs.key + { + return Err(refusal( + "native_build_receipt", + "A build receipt does not match its path; no image was adopted or replaced.", + )); + } + if inputs.policy == Policy::Missing + && previous.phase == Phase::ContentIdVerified + && previous.checkout == candidate.checkout + && previous.provider_incarnation == identity.incarnation + && previous.engine_id == identity.engine_id + { + if let Some(id) = previous.image_id.as_deref().filter(|id| image_id(id)) { + if let Some(inspected) = pool.inspect(&lease, id)? { + if owned_image(&inspected, id, &expected).is_ok() { + if previous.provider_boot_id != identity.boot_id { + let mut current = previous.clone(); + current.provider_boot_id = identity.boot_id.clone(); + state::write(&receipt_path, ¤t)?; + } + return Ok(BuildOutcome { + image_id: id.to_owned(), + build_key: inputs.key.clone(), + tag: inputs.tag(), + reused: true, + base_pins: previous.base_pins.clone(), + }); + } + } + } + } + if previous.phase == Phase::Building { + // The key lock proves no live holder: that attempt died mid-build. + let mut dead = previous.clone(); + dead.phase = Phase::Indeterminate; + state::write(&receipt_path, &dead)?; + } + } + drop(lease); + + // Step 2, unleased: resolve every base image to a pinned manifest. + let mut pins = BTreeMap::new(); + for base in &inputs.images { + if canceled() { + return Err(canceled_early()); + } + let resolved = pool.resolve(base)?; + if !image_id(&resolved.source_digest) + || !resolved + .pinned_reference + .ends_with(&format!("@{}", resolved.source_digest)) + { + return Err(refusal( + "native_build_base_image", + "A build base image did not resolve to a pinned manifest.", + )); + } + pins.insert(base.context_name.clone(), resolved.pinned_reference); + } + let builder = pool.builder()?; + if canceled() { + return Err(canceled_early()); + } + + // Step 3, under the lease: same pool, then record the attempt. + let (lease, current) = pool.lease()?; + if current != identity { + return Err(refusal( + "runtime_changed", + "The runtime changed before the build started; no build was run.", + )); + } + let mut receipt = Receipt { + version: 1, + checkout: candidate.checkout.clone(), + namespace: inputs.namespace.clone(), + build_key: inputs.key.clone(), + provider_incarnation: identity.incarnation.clone(), + provider_boot_id: identity.boot_id.clone(), + engine_id: identity.engine_id.clone(), + phase: Phase::Building, + image_id: None, + base_pins: pins.clone(), + }; + state::write(&receipt_path, &receipt)?; + drop(lease); + + // Step 4, unleased: run the client in its own process group. + let scratch = directory.join(format!("{}.client", inputs.key)); + state::private_directory(&scratch)?; + let metadata = scratch.join("metadata.json"); + match fs::remove_file(&metadata) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(state::io(error)), + } + let ended = supervise( + command( + &builder, &scratch, &metadata, inputs, &identity, &pins, &expected, + )?, + inputs.timeout, + canceled, + ); + let ended = match ended { + Ok(ended) => ended, + Err(error) => { + receipt.phase = Phase::Indeterminate; + state::write(&receipt_path, &receipt)?; + return Err(error); + } + }; + match ended { + Ended::Exited(true) => {} + Ended::Exited(false) => { + receipt.phase = Phase::Failed; + state::write(&receipt_path, &receipt)?; + return Err(refusal( + "native_build_failed", + "The image build failed; its output is shown above. No image was adopted.", + )); + } + Ended::Canceled => { + receipt.phase = Phase::Canceled; + state::write(&receipt_path, &receipt)?; + return Err(refusal( + "native_build_canceled", + "The image build was canceled; no image was adopted.", + )); + } + Ended::TimedOut => { + receipt.phase = Phase::Canceled; + state::write(&receipt_path, &receipt)?; + return Err(refusal( + "native_build_timeout", + "The image build exceeded its time budget and was stopped; no image was adopted.", + )); + } + } + + // Step 5, under the lease: the same pool and engine, then adopt by content ID. + let (lease, current) = pool.lease()?; + if current != identity { + receipt.phase = Phase::Indeterminate; + state::write(&receipt_path, &receipt)?; + return Err(refusal( + "runtime_changed", + "The runtime changed during the build; the result was not adopted.", + )); + } + let adopted = built_image_id(&metadata).and_then(|id| { + let inspected = pool.inspect(&lease, &id)?.ok_or_else(|| { + refusal( + "native_build_image", + "The built image is absent from the engine.", + ) + })?; + owned_image(&inspected, &id, &expected)?; + Ok(id) + }); + let id = match adopted { + Ok(id) => id, + Err(error) => { + // A refused or missing result is a failed build; an engine or transport + // error leaves the outcome unknown. + receipt.phase = if error.code.starts_with("native_build_") { + Phase::Failed + } else { + Phase::Indeterminate + }; + state::write(&receipt_path, &receipt)?; + return Err(error); + } + }; + receipt.phase = Phase::ContentIdVerified; + receipt.image_id = Some(id.clone()); + state::write(&receipt_path, &receipt)?; + drop(lease); + Ok(BuildOutcome { + image_id: id, + build_key: inputs.key.clone(), + tag: inputs.tag(), + reused: false, + base_pins: pins, + }) +} + +/// The image config digest the client reports for the built image. +fn built_image_id(metadata: &Path) -> Result { + let bytes = read_bounded_file(metadata, MAX_METADATA_BYTES)?.ok_or_else(|| { + refusal( + "native_build_image", + "The build client reported no image metadata.", + ) + })?; + let value: Value = serde_json::from_slice(&bytes).map_err(|_| { + refusal( + "native_build_image", + "The build client wrote malformed image metadata.", + ) + })?; + let id = value["containerimage.config.digest"] + .as_str() + .filter(|id| image_id(id)) + .ok_or_else(|| { + refusal( + "native_build_image", + "The build metadata has no image content ID.", + ) + })?; + Ok(id.to_owned()) +} + +/// The client command: argv carries only paths, names and non-secret labels; argument +/// values reach the client through its environment. +fn command( + builder: &Path, + scratch: &Path, + metadata: &Path, + inputs: &Inputs, + identity: &PoolIdentity, + pins: &BTreeMap, + labels: &BTreeMap<&'static str, String>, +) -> Result { + let config = scratch.join("docker-config"); + let buildx = scratch.join("buildx"); + state::private_directory(&config)?; + state::private_directory(&buildx)?; + let mut command = super::process::clean_command(builder); + command + .current_dir(scratch) + .env("HOME", scratch) + .env("TMPDIR", scratch) + .env( + "DOCKER_HOST", + format!("unix://{}", identity.socket.display()), + ) + // An empty private client configuration: no contexts, credential helpers or + // proxies from the user's Docker setup. + .env("DOCKER_CONFIG", &config) + .env("BUILDX_CONFIG", &buildx) + .env("BUILDX_NO_DEFAULT_ATTESTATIONS", "1") + .env("DOCKER_CLI_HINTS", "false") + .args(["build", "--progress=plain", "--platform", "linux/arm64"]) + .args(["--provenance=false", "--sbom=false", "--load"]) + .arg("--file") + .arg(&inputs.dockerfile) + .arg("--metadata-file") + .arg(metadata) + .args(["--tag", &inputs.tag()]); + if let Some(target) = &inputs.target { + command.args(["--target", target]); + } + for (name, value) in &inputs.args { + command.args(["--build-arg", name]).env(name, value); + } + for (name, reference) in pins { + command + .arg("--build-context") + .arg(format!("{name}=docker-image://{reference}")); + } + for (name, value) in labels { + command.arg("--label").arg(format!("{name}={value}")); + } + command + .arg(&inputs.context) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::inherit()) + .process_group(0); + Ok(command) +} + +#[derive(Debug, Eq, PartialEq)] +enum Ended { + Exited(bool), + Canceled, + TimedOut, +} + +static CANCEL_REQUESTED: AtomicBool = AtomicBool::new(false); +extern "C" fn request_cancel(_: libc::c_int) { + CANCEL_REQUESTED.store(true, Ordering::Release); +} + +/// SIGINT, SIGTERM and SIGHUP request cancellation for the whole operation; the +/// previous dispositions return when this guard drops. +struct CancelSignals(Vec<(libc::c_int, libc::sigaction)>); +impl CancelSignals { + fn install() -> Result { + CANCEL_REQUESTED.store(false, Ordering::Release); + let mut previous_actions = Vec::new(); + for signal in [libc::SIGINT, libc::SIGTERM, libc::SIGHUP] { + // SAFETY: sigaction structs are plain C storage, zero-initialized before use. + let mut action: libc::sigaction = unsafe { std::mem::zeroed() }; + // SAFETY: as above; the kernel fills it with the previous disposition. + let mut previous: libc::sigaction = unsafe { std::mem::zeroed() }; + action.sa_sigaction = request_cancel as *const () as usize; + // SAFETY: valid writable mask storage. + unsafe { libc::sigemptyset(&mut action.sa_mask) }; + // SAFETY: both pointers reference live storage for the duration of the call. + if unsafe { libc::sigaction(signal, &action, &mut previous) } != 0 { + return Err(state::io(std::io::Error::last_os_error())); + } + previous_actions.push((signal, previous)); + } + Ok(Self(previous_actions)) + } +} +impl Drop for CancelSignals { + fn drop(&mut self) { + for (signal, previous) in &self.0 { + // SAFETY: restores a disposition the kernel returned for this signal. + unsafe { + libc::sigaction(*signal, previous, std::ptr::null_mut()); + } + } + } +} + +/// Spawn the client and wait for it, honoring cancellation and the time budget. On +/// either, the whole process group gets SIGTERM, then SIGKILL after a grace period. +/// The group is signaled only while its leader is unreaped, so its ID is still ours. +fn supervise( + mut command: std::process::Command, + timeout: Duration, + canceled: &dyn Fn() -> bool, +) -> Result { + let mut child = command.spawn().map_err(|_| { + refusal( + "native_build_client", + "The pinned build client could not be started.", + ) + })?; + let deadline = Instant::now() + timeout; + loop { + if let Some(status) = child.try_wait().map_err(state::io)? { + return Ok(Ended::Exited(status.success())); + } + let cancel = canceled(); + if cancel || Instant::now() >= deadline { + stop_group(&mut child)?; + return Ok(if cancel { + Ended::Canceled + } else { + Ended::TimedOut + }); + } + std::thread::sleep(POLL); + } +} + +fn stop_group(child: &mut Child) -> Result<(), CandidateError> { + let group = child.id() as libc::pid_t; + // SAFETY: the leader is unreaped, so this process group ID cannot be reused yet. + unsafe { libc::killpg(group, libc::SIGTERM) }; + let grace = Instant::now() + CANCEL_GRACE; + while Instant::now() < grace { + if child.try_wait().map_err(state::io)?.is_some() { + return Ok(()); + } + std::thread::sleep(POLL); + } + // SAFETY: still unreaped (checked above), so the group ID is still ours. + unsafe { libc::killpg(group, libc::SIGKILL) }; + child.wait().map_err(state::io)?; + Ok(()) +} + +/// The live pool: the candidate's running provider and its pinned builder. +struct LivePool<'a> { + candidate: &'a Candidate, +} + +#[cfg(target_os = "macos")] +fn connect(candidate: &Candidate) -> Result, CandidateError> { + super::engine::Engine::connect_until(candidate, Instant::now() + LEASE_WAIT, || false) +} +#[cfg(not(target_os = "macos"))] +fn connect(candidate: &Candidate) -> Result, CandidateError> { + let _ = LEASE_WAIT; + super::engine::Engine::connect(candidate) +} + +/// Require the isolated build bridge this boot set up (guest-daemon.sh and +/// guest-build-network.sh). A pool started by an older Hack has neither; restarting the +/// runtime is the only remedy, and nothing here restarts the engine. +fn verify_build_network(engine: &super::engine::Engine<'_>) -> Result<(), CandidateError> { + if matches!( + engine.guest().network_intent(), + super::NetworkIntent::Isolated + ) { + return Err(refusal( + "native_build_isolated", + "Native image builds need network egress; this runtime was started isolated.", + )); + } + let restart = || { + refusal( + "native_build_runtime_restart", + "This native runtime has no isolated build network; stop the native runtime and run `hack up` again.", + ) + }; + let bridge = match engine.request(reqwest::Method::GET, "/v1.53/networks/bridge", None) { + Ok(bridge) => bridge, + Err(error) if error.code == "engine_not_found" => return Err(restart()), + Err(error) => return Err(error), + }; + if bridge["IPAM"]["Config"][0]["Subnet"] != BUILD_NETWORK_SUBNET + || bridge["Options"]["com.docker.network.bridge.enable_icc"] != "false" + { + return Err(restart()); + } + let boot = engine.guest().boot_id().to_owned(); + let observed = engine + .guest() + .execute( + &format!( + "test \"$(cat /run/hack-local/build-network)\" = \"{BUILD_NETWORK_MARKER} $1\"\n\ + iptables -C DOCKER-USER -i docker0 -j HACK-BUILD-FORWARD\n\ + iptables -C INPUT -i docker0 -j HACK-BUILD-INPUT\n\ + printf 'build-network-ready\\n'" + ), + &[&boot], + None, + ) + .map_err(|_| restart())?; + if observed != "build-network-ready\n" { + return Err(restart()); + } + Ok(()) +} + +impl<'a> BuildPool for LivePool<'a> { + type Lease = super::engine::Engine<'a>; + fn lease(&self) -> Result<(Self::Lease, PoolIdentity), CandidateError> { + let engine = connect(self.candidate)?; + verify_build_network(&engine)?; + let info = engine.request(reqwest::Method::GET, "/v1.53/info", None)?; + let engine_id = info["ID"] + .as_str() + .filter(|id| { + !id.is_empty() && id.len() <= 128 && id.bytes().all(|b| b.is_ascii_graphic()) + }) + .ok_or_else(|| refusal("engine_protocol", "The engine reported no usable identity."))? + .to_owned(); + let identity = PoolIdentity { + incarnation: engine.guest().incarnation().to_owned(), + boot_id: engine.guest().boot_id().to_owned(), + engine_id, + socket: engine.guest().engine_socket()?, + }; + Ok((engine, identity)) + } + fn inspect(&self, lease: &Self::Lease, image: &str) -> Result, CandidateError> { + match lease.request( + reqwest::Method::GET, + &format!("/v1.53/images/{image}/json"), + None, + ) { + Ok(value) => Ok(Some(value)), + Err(error) if error.code == "engine_not_found" => Ok(None), + Err(error) => Err(error), + } + } + fn resolve(&self, base: &BaseImage) -> Result { + registry_image::resolve_build_base(&base.reference) + } + fn builder(&self) -> Result { + artifact::verify_builder(self.candidate)?; + Ok(artifact::builder_executable(self.candidate)) + } +} + +/// `runtime build-image --request `: build or reuse one native image. +pub fn build_image(candidate: &Candidate, request: &Path) -> Result { + let request: BuildRequest = state::read_bounded(request, MAX_REQUEST_BYTES)?; + let inputs = prepare(request)?; + let _signals = CancelSignals::install()?; + run(candidate, &LivePool { candidate }, &inputs, &|| { + CANCEL_REQUESTED.load(Ordering::Acquire) + }) +} diff --git a/packages/runtime-core/src/provider/image_build/tests.rs b/packages/runtime-core/src/provider/image_build/tests.rs new file mode 100644 index 000000000..4b209401b --- /dev/null +++ b/packages/runtime-core/src/provider/image_build/tests.rs @@ -0,0 +1,744 @@ +//! `runtime build-image` against a fake pool and a fake build client. +//! +//! The fake client is `/bin/sh` reading a script named `build` from the client's private +//! working directory, so no test writes and then executes a fresh binary. It records its +//! argv, the one build-argument variable it was given, and a call count. +use super::*; +use std::{cell::RefCell, os::unix::fs::PermissionsExt}; + +const IMAGE: &str = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; +const PIN: &str = "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; +const SECRET: &str = "literal-value-that-must-not-leak"; + +struct Fixture { + root: PathBuf, + candidate: Candidate, + project: PathBuf, +} +impl Fixture { + fn new() -> Self { + let tmp = std::env::temp_dir().canonicalize().unwrap(); + let root = crate::test_root::private_test_root(&tmp, "hack-image-build"); + let checkout = root.join("checkout"); + state::private_directory(&checkout).unwrap(); + let candidate = Candidate::discover(&checkout).unwrap(); + let project = root.join("project"); + fs::create_dir(&project).unwrap(); + fs::create_dir(project.join("app")).unwrap(); + fs::write( + project.join("app/Dockerfile"), + "ARG BASE=alpine:3.20\nFROM ${BASE}\nARG GREETING\nRUN echo hi\n", + ) + .unwrap(); + fs::write(project.join("app/main.txt"), "one").unwrap(); + Self { + root, + candidate, + project, + } + } + fn request(&self, policy: Policy) -> BuildRequest { + BuildRequest { + version: 1, + project_root: self.project.clone(), + branch: None, + context: "app".into(), + dockerfile: "Dockerfile".into(), + target: None, + args: BTreeMap::from([("GREETING".to_owned(), SECRET.to_owned())]), + policy, + timeout_seconds: Some(30), + } + } + fn inputs(&self, policy: Policy) -> Inputs { + prepare(self.request(policy)).unwrap() + } + fn scratch(&self, inputs: &Inputs) -> PathBuf { + builds_root(&self.candidate) + .join(&inputs.namespace) + .join(format!("{}.client", inputs.key)) + } + fn receipt(&self, inputs: &Inputs) -> Receipt { + state::read( + &builds_root(&self.candidate) + .join(&inputs.namespace) + .join(format!("{}.json", inputs.key)), + ) + .unwrap() + } + /// Install the fake client for this build key. `mode` selects its behavior. + fn client(&self, inputs: &Inputs, mode: &str) { + let scratch = self.scratch(inputs); + state::private_directory(&scratch).unwrap(); + let script = format!( + r#"set -eu +count=$(cat calls 2>/dev/null || echo 0) +echo $((count + 1)) > calls +printf '%s\n' "$@" > argv +printf '%s' "${{GREETING-}}" > greeting +meta= +previous= +for argument do + if [ "$previous" = --metadata-file ]; then meta=$argument; fi + previous=$argument +done +case {mode} in + ok) printf '{{"containerimage.config.digest":"{IMAGE}"}}' > "$meta" ;; + fail) exit 3 ;; + nometa) ;; + sleep) sleep 30 & echo $! > child; wait ;; +esac +"# + ); + fs::write(scratch.join("build"), script).unwrap(); + fs::set_permissions(scratch.join("build"), fs::Permissions::from_mode(0o600)).unwrap(); + } + fn calls(&self, inputs: &Inputs) -> u32 { + fs::read_to_string(self.scratch(inputs).join("calls")) + .map(|v| v.trim().parse().unwrap()) + .unwrap_or(0) + } +} +impl Drop for Fixture { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.root); + } +} + +fn identity(incarnation: &str) -> PoolIdentity { + PoolIdentity { + incarnation: incarnation.into(), + boot_id: "boot-1".into(), + engine_id: "engine-1".into(), + socket: PathBuf::from("/nonexistent/docker.sock"), + } +} + +/// A fake pool: identities returned per lease in order (the last repeats), images by ID. +struct FakePool { + identities: RefCell>, + images: RefCell>, + leases: RefCell, + resolve_error: bool, +} +impl FakePool { + fn new(incarnation: &str) -> Self { + Self { + identities: RefCell::new(vec![identity(incarnation)]), + images: RefCell::new(BTreeMap::new()), + leases: RefCell::new(0), + resolve_error: false, + } + } + /// Register the image the fake client "builds", with the labels the build expects. + fn built(&self, inputs: &Inputs, incarnation: &str, extra: Value) { + let mut labels = serde_json::to_value(labels(inputs, &identity(incarnation))).unwrap(); + if let Value::Object(more) = extra { + labels.as_object_mut().unwrap().extend(more); + } + self.images.borrow_mut().insert( + IMAGE.into(), + serde_json::json!({"Id": IMAGE, "Architecture": "arm64", "Os": "linux", + "Config": {"Labels": labels, "Volumes": null}}), + ); + } +} +impl BuildPool for FakePool { + type Lease = (); + fn lease(&self) -> Result<((), PoolIdentity), CandidateError> { + *self.leases.borrow_mut() += 1; + let mut identities = self.identities.borrow_mut(); + let current = if identities.len() > 1 { + identities.remove(0) + } else { + identities[0].clone() + }; + Ok(((), current)) + } + fn inspect(&self, _: &(), image: &str) -> Result, CandidateError> { + Ok(self.images.borrow().get(image).cloned()) + } + fn resolve(&self, base: &BaseImage) -> Result { + if self.resolve_error { + return Err(CandidateError::new("registry_image", "offline")); + } + Ok(BuildBase { + pinned_reference: format!("docker.io/{}@{PIN}", base.repository()), + source_digest: PIN.into(), + image_id: IMAGE.into(), + }) + } + fn builder(&self) -> Result { + Ok(PathBuf::from("/bin/sh")) + } +} + +fn never() -> bool { + false +} + +#[test] +fn cold_build_passes_values_only_through_the_environment_and_records_the_result() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + let pool = FakePool::new("pool-1"); + fixture.client(&inputs, "ok"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + let outcome = run(&fixture.candidate, &pool, &inputs, &never).unwrap(); + assert_eq!(outcome.image_id, IMAGE); + assert!(!outcome.reused); + assert_eq!( + outcome.base_pins, + BTreeMap::from([( + "alpine:3.20".to_owned(), + format!("docker.io/library/alpine@{PIN}") + )]) + ); + let scratch = fixture.scratch(&inputs); + let argv = fs::read_to_string(scratch.join("argv")).unwrap(); + assert!(!argv.contains(SECRET), "argument value reached argv"); + let lines: Vec<&str> = argv.lines().collect(); + assert!(lines.windows(2).any(|w| w == ["--build-arg", "GREETING"])); + assert!(lines.windows(2).any(|w| w + == [ + "--build-context", + &format!("alpine:3.20=docker-image://docker.io/library/alpine@{PIN}") + ])); + assert!(lines.windows(2).any(|w| w == ["--platform", "linux/arm64"])); + assert!(lines.contains(&"--provenance=false")); + assert!( + lines + .windows(2) + .any(|w| w == ["--label", "io.hack-local.kind=build-image"]) + ); + assert!( + !argv.contains("pool-1"), + "the pool incarnation reached argv" + ); + assert_eq!( + fs::read_to_string(scratch.join("greeting")).unwrap(), + SECRET + ); + let receipt = fixture.receipt(&inputs); + assert_eq!(receipt.phase, Phase::ContentIdVerified); + assert_eq!(receipt.image_id.as_deref(), Some(IMAGE)); + let text = fs::read_to_string( + builds_root(&fixture.candidate) + .join(&inputs.namespace) + .join(format!("{}.json", inputs.key)), + ) + .unwrap(); + assert!(!text.contains(SECRET), "argument value reached the receipt"); +} + +#[test] +fn verified_image_is_reused_without_running_the_client() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + let pool = FakePool::new("pool-1"); + fixture.client(&inputs, "ok"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + run(&fixture.candidate, &pool, &inputs, &never).unwrap(); + let again = run(&fixture.candidate, &pool, &inputs, &never).unwrap(); + assert!(again.reused); + assert_eq!(again.image_id, IMAGE); + assert_eq!(fixture.calls(&inputs), 1); + // An explicit rebuild runs the client again. + let rebuild = fixture.inputs(Policy::Rebuild); + let rebuilt = run(&fixture.candidate, &pool, &rebuild, &never).unwrap(); + assert!(!rebuilt.reused); + assert_eq!(fixture.calls(&inputs), 2); +} + +#[test] +fn a_new_incarnation_or_a_missing_or_relabeled_image_is_built_again() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + let pool = FakePool::new("pool-1"); + fixture.client(&inputs, "ok"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + run(&fixture.candidate, &pool, &inputs, &never).unwrap(); + + // Storage recreated: the old receipt is never trusted. + let recreated = FakePool::new("pool-2"); + recreated.built(&inputs, "pool-2", serde_json::json!({})); + assert!( + !run(&fixture.candidate, &recreated, &inputs, &never) + .unwrap() + .reused + ); + assert_eq!(fixture.calls(&inputs), 2); + + // Image removed from the engine. + recreated.images.borrow_mut().clear(); + let error = run(&fixture.candidate, &recreated, &inputs, &never).unwrap_err(); + assert_eq!(error.code, "native_build_image"); + assert_eq!(fixture.calls(&inputs), 3); + assert_eq!(fixture.receipt(&inputs).phase, Phase::Failed); + + // Same ID with labels that are not ours is never adopted. + recreated.built(&inputs, "pool-1", serde_json::json!({})); + let error = run(&fixture.candidate, &recreated, &inputs, &never).unwrap_err(); + assert_eq!(error.code, "native_build_reserved_label"); + assert_eq!(fixture.receipt(&inputs).phase, Phase::Failed); +} + +#[test] +fn reserved_labels_and_image_volumes_refuse_adoption() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + fixture.client(&inputs, "ok"); + let pool = FakePool::new("pool-1"); + pool.built( + &inputs, + "pool-1", + serde_json::json!({"io.hack-local.graph": "forged"}), + ); + let error = run(&fixture.candidate, &pool, &inputs, &never).unwrap_err(); + assert_eq!(error.code, "native_build_reserved_label"); + assert!(!error.message.contains("forged")); + + pool.built( + &inputs, + "pool-1", + serde_json::json!({"com.docker.compose.project": "x"}), + ); + assert_eq!( + run(&fixture.candidate, &pool, &inputs, &never) + .unwrap_err() + .code, + "native_build_reserved_label" + ); + + pool.built(&inputs, "pool-1", serde_json::json!({"org.example": "ok"})); + pool.images.borrow_mut().get_mut(IMAGE).unwrap()["Config"]["Volumes"] = + serde_json::json!({"/data": {}}); + assert_eq!( + run(&fixture.candidate, &pool, &inputs, &never) + .unwrap_err() + .code, + "native_build_image_volumes" + ); + + pool.images.borrow_mut().get_mut(IMAGE).unwrap()["Config"]["Volumes"] = Value::Null; + pool.images.borrow_mut().get_mut(IMAGE).unwrap()["Architecture"] = "amd64".into(); + assert_eq!( + run(&fixture.candidate, &pool, &inputs, &never) + .unwrap_err() + .code, + "native_build_image" + ); +} + +#[test] +fn client_failure_and_missing_metadata_adopt_nothing() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + let pool = FakePool::new("pool-1"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + fixture.client(&inputs, "fail"); + let error = run(&fixture.candidate, &pool, &inputs, &never).unwrap_err(); + assert_eq!(error.code, "native_build_failed"); + let receipt = fixture.receipt(&inputs); + assert_eq!(receipt.phase, Phase::Failed); + assert!(receipt.image_id.is_none()); + + fixture.client(&inputs, "nometa"); + let error = run(&fixture.candidate, &pool, &inputs, &never).unwrap_err(); + assert_eq!(error.code, "native_build_image"); +} + +fn process_alive(pid: &str) -> bool { + let pid: libc::pid_t = pid.trim().parse().unwrap(); + // SAFETY: signal 0 only probes existence. + unsafe { libc::kill(pid, 0) == 0 } +} + +#[test] +fn timeout_and_cancellation_stop_the_whole_client_group() { + let fixture = Fixture::new(); + let mut request = fixture.request(Policy::Missing); + request.timeout_seconds = Some(1); + let inputs = prepare(request).unwrap(); + let pool = FakePool::new("pool-1"); + fixture.client(&inputs, "sleep"); + let started = Instant::now(); + let error = run(&fixture.candidate, &pool, &inputs, &never).unwrap_err(); + assert_eq!(error.code, "native_build_timeout"); + assert!(started.elapsed() < Duration::from_secs(20)); + assert_eq!(fixture.receipt(&inputs).phase, Phase::Canceled); + let child = fs::read_to_string(fixture.scratch(&inputs).join("child")).unwrap(); + let deadline = Instant::now() + Duration::from_secs(10); + while process_alive(&child) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(20)); + } + assert!(!process_alive(&child), "a client group member survived"); + + let inputs = fixture.inputs(Policy::Missing); + fixture.client(&inputs, "sleep"); + fs::remove_file(fixture.scratch(&inputs).join("child")).unwrap(); + let scratch = fixture.scratch(&inputs); + let cancel = move || scratch.join("child").exists(); + let error = run(&fixture.candidate, &pool, &inputs, &cancel).unwrap_err(); + assert_eq!(error.code, "native_build_canceled"); + assert_eq!(fixture.receipt(&inputs).phase, Phase::Canceled); + let child = fs::read_to_string(fixture.scratch(&inputs).join("child")).unwrap(); + let deadline = Instant::now() + Duration::from_secs(10); + while process_alive(&child) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(20)); + } + assert!(!process_alive(&child)); + + // A request canceled before the client starts never runs it. + let error = run(&fixture.candidate, &pool, &inputs, &|| true).unwrap_err(); + assert_eq!(error.code, "native_build_canceled"); +} + +#[test] +fn a_dead_attempt_is_recorded_indeterminate_and_built_again() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + let pool = FakePool::new("pool-1"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + fixture.client(&inputs, "ok"); + let directory = builds_root(&fixture.candidate).join(&inputs.namespace); + state::private_directory(&directory).unwrap(); + let path = directory.join(format!("{}.json", inputs.key)); + let leftover = Receipt { + version: 1, + checkout: fixture.candidate.checkout.clone(), + namespace: inputs.namespace.clone(), + build_key: inputs.key.clone(), + provider_incarnation: "pool-1".into(), + provider_boot_id: "boot-1".into(), + engine_id: "engine-1".into(), + phase: Phase::Building, + image_id: None, + base_pins: BTreeMap::new(), + }; + state::write(&path, &leftover).unwrap(); + // Even with an adoptable image present, the dead attempt's receipt is not trusted. + let outcome = run(&fixture.candidate, &pool, &inputs, &never).unwrap(); + assert!(!outcome.reused); + assert_eq!(fixture.receipt(&inputs).phase, Phase::ContentIdVerified); + + // A receipt that names another key is never adopted or replaced. + let mut foreign = leftover; + foreign.build_key = "0".repeat(64); + fs::remove_file(&path).unwrap(); + state::write(&path, &foreign).unwrap(); + assert_eq!( + run(&fixture.candidate, &pool, &inputs, &never) + .unwrap_err() + .code, + "native_build_receipt" + ); +} + +#[test] +fn the_pool_must_be_unchanged_between_admission_and_adoption() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + fixture.client(&inputs, "ok"); + let pool = FakePool::new("pool-1"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + let mut rebooted = identity("pool-1"); + rebooted.boot_id = "boot-2".into(); + *pool.identities.borrow_mut() = vec![identity("pool-1"), identity("pool-1"), rebooted]; + let error = run(&fixture.candidate, &pool, &inputs, &never).unwrap_err(); + assert_eq!(error.code, "runtime_changed"); + assert_eq!(fixture.receipt(&inputs).phase, Phase::Indeterminate); + + // A change before the client starts runs nothing. + let pool = FakePool::new("pool-1"); + *pool.identities.borrow_mut() = vec![identity("pool-1"), identity("pool-9")]; + let calls = fixture.calls(&inputs); + assert_eq!( + run(&fixture.candidate, &pool, &inputs, &never) + .unwrap_err() + .code, + "runtime_changed" + ); + assert_eq!(fixture.calls(&inputs), calls); +} + +#[test] +fn the_key_lock_and_the_down_fence_refuse_instead_of_waiting() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + fixture.client(&inputs, "ok"); + let pool = FakePool::new("pool-1"); + let directory = builds_root(&fixture.candidate).join(&inputs.namespace); + state::private_directory(&directory).unwrap(); + let held = Flock::try_lock( + Flock::open(&directory.join(format!("{}.lock", inputs.key))).unwrap(), + libc::LOCK_EX, + ) + .unwrap() + .unwrap(); + assert_eq!( + run(&fixture.candidate, &pool, &inputs, &never) + .unwrap_err() + .code, + "native_build_busy" + ); + drop(held); + + assert!(require_no_active_build(&fixture.candidate).is_ok()); + let build = Flock::try_lock( + Flock::open(&builds_root(&fixture.candidate).join("active.lock")).unwrap(), + libc::LOCK_SH, + ) + .unwrap() + .unwrap(); + assert_eq!( + require_no_active_build(&fixture.candidate) + .unwrap_err() + .code, + "provider_busy" + ); + drop(build); + assert!(require_no_active_build(&fixture.candidate).is_ok()); +} + +#[test] +fn base_resolution_failures_run_nothing() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + fixture.client(&inputs, "ok"); + let mut pool = FakePool::new("pool-1"); + pool.resolve_error = true; + assert_eq!( + run(&fixture.candidate, &pool, &inputs, &never) + .unwrap_err() + .code, + "registry_image" + ); + assert_eq!(fixture.calls(&inputs), 0); +} + +fn code(request: BuildRequest) -> &'static str { + prepare(request).unwrap_err().code +} + +#[test] +fn requests_refuse_unsafe_paths_arguments_and_bounds() { + let fixture = Fixture::new(); + let base = || fixture.request(Policy::Missing); + + let mut request = base(); + request.version = 2; + assert_eq!(code(request), "native_build_request"); + let mut request = base(); + request.project_root = PathBuf::from("relative"); + assert_eq!(code(request), "native_build_request"); + let mut request = base(); + request.project_root = fixture.project.join("app/.."); + assert_eq!(code(request), "native_build_source"); + let mut request = base(); + request.context = "../outside".into(); + assert_eq!(code(request), "source_escape"); + let mut request = base(); + request.dockerfile = "../../Dockerfile".into(); + assert_eq!(code(request), "source_escape"); + std::os::unix::fs::symlink(fixture.project.join("app"), fixture.project.join("link")).unwrap(); + let mut request = base(); + request.context = "link".into(); + assert_eq!(code(request), "source_symlink"); + let mut request = base(); + request.context = "missing".into(); + assert_eq!(code(request), "missing_source"); + let mut request = base(); + request.branch = Some("Not_A_Label".into()); + assert_eq!(code(request), "native_build_request"); + let mut request = base(); + request.target = Some("-bad".into()); + assert_eq!(code(request), "native_build_request"); + for timeout in [0, MAX_TIMEOUT_SECONDS + 1] { + let mut request = base(); + request.timeout_seconds = Some(timeout); + assert_eq!(code(request), "native_build_request"); + } + for name in [ + "PATH", + "home", + "DOCKER_HOST", + "BUILDX_CONFIG", + "BUILDKIT_SYNTAX", + "HTTPS_PROXY", + "no_proxy", + "LC_ALL", + "GIT_DIR", + "SSH_AUTH_SOCK", + "1BAD", + "bad-name", + "", + ] { + let mut request = base(); + request.args = BTreeMap::from([(name.to_owned(), "x".to_owned())]); + assert_eq!(code(request), "native_build_argument", "{name}"); + } + let mut request = base(); + request.args = BTreeMap::from([("GREETING".to_owned(), "a\0b".to_owned())]); + assert_eq!(code(request), "native_build_argument"); + let mut request = base(); + request.args = (0..=MAX_ARGS) + .map(|i| (format!("A{i}"), "x".into())) + .collect(); + assert_eq!(code(request), "native_build_argument"); + let mut request = base(); + request.args = BTreeMap::from([("BIG".to_owned(), "x".repeat(MAX_ARG_BYTES))]); + assert_eq!(code(request), "native_build_argument"); + // Refusals never echo argument values. + let mut request = base(); + request.args = BTreeMap::from([("DOCKER_HOST".to_owned(), SECRET.to_owned())]); + assert!(!prepare(request).unwrap_err().message.contains(SECRET)); +} + +#[test] +fn the_build_key_binds_local_inputs_but_not_context_contents() { + let fixture = Fixture::new(); + let key = |request: BuildRequest| prepare(request).unwrap().key; + let original = key(fixture.request(Policy::Missing)); + assert_eq!(original, key(fixture.request(Policy::Rebuild))); + + fs::write(fixture.project.join("app/main.txt"), "two").unwrap(); + assert_eq!(original, key(fixture.request(Policy::Missing))); + + let mut request = fixture.request(Policy::Missing); + request.args.insert("GREETING".into(), "other".into()); + assert_ne!(original, key(request)); + let mut request = fixture.request(Policy::Missing); + request.branch = Some("feature".into()); + let branched = prepare(request).unwrap(); + assert_ne!(original, branched.key); + assert_ne!( + branched.namespace, + fixture.inputs(Policy::Missing).namespace + ); + + fs::write(fixture.project.join("app/.dockerignore"), "*.txt\n").unwrap(); + let ignored = key(fixture.request(Policy::Missing)); + assert_ne!(original, ignored); + fs::write( + fixture.project.join("app/Dockerfile.dockerignore"), + "*.md\n", + ) + .unwrap(); + assert_ne!(ignored, key(fixture.request(Policy::Missing))); + + fs::write( + fixture.project.join("app/Dockerfile"), + "FROM alpine:3.21\nRUN echo hi\n", + ) + .unwrap(); + assert_ne!(original, key(fixture.request(Policy::Missing))); + + let inputs = fixture.inputs(Policy::Missing); + let tag = inputs.tag(); + assert!(tag.starts_with("hack-local-build/")); + assert_eq!(tag.len(), "hack-local-build/".len() + 12 + 1 + 32); +} + +#[test] +fn dockerfile_and_ignore_inputs_must_be_plain_files() { + let fixture = Fixture::new(); + std::os::unix::fs::symlink( + fixture.project.join("app/main.txt"), + fixture.project.join("app/.dockerignore"), + ) + .unwrap(); + assert_eq!( + code(fixture.request(Policy::Missing)), + "native_build_source" + ); + fs::remove_file(fixture.project.join("app/.dockerignore")).unwrap(); + fs::hard_link( + fixture.project.join("app/Dockerfile"), + fixture.project.join("Dockerfile.copy"), + ) + .unwrap(); + assert_eq!( + code(fixture.request(Policy::Missing)), + "native_build_source" + ); +} + +#[test] +fn owner_labels_never_carry_the_incarnation() { + let label = owner_label("0123456789abcdef0123456789abcdef"); + assert_eq!(label.len(), 32); + assert!(!label.contains("0123456789abcdef")); + assert_ne!(label, owner_label("another")); +} + +/// Run guest-daemon.sh with `dockerd` replaced by a shell function that records its argv. +fn daemon_arguments(mode: &str) -> (bool, String) { + let tmp = std::env::temp_dir().canonicalize().unwrap(); + let root = crate::test_root::private_test_root(&tmp, "hack-guest-daemon"); + let script = format!( + "dockerd() {{ printf '%s\\n' \"$@\" > '{}/argv'; }}\n{}", + root.display(), + include_str!("../guest-daemon.sh") + ); + let status = std::process::Command::new("/bin/sh") + .args(["-c", &script, "daemon-test", mode]) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .unwrap(); + let argv = fs::read_to_string(root.join("argv")).unwrap_or_default(); + fs::remove_dir_all(&root).unwrap(); + (status.code() == Some(64), argv) +} + +#[test] +fn only_egress_pools_get_the_fixed_build_bridge() { + let (refused, argv) = daemon_arguments("isolated"); + assert!(!refused); + assert!(argv.lines().any(|l| l == "--bridge=none")); + assert!(!argv.contains("--bip")); + for mode in ["internet", "approved-hosts", "host-gateway"] { + let (refused, argv) = daemon_arguments(mode); + assert!(!refused, "{mode}"); + let lines: Vec<&str> = argv.lines().collect(); + assert!(lines.contains(&"--bip=10.213.255.1/24"), "{mode}"); + assert!(lines.contains(&"--icc=false"), "{mode}"); + assert!(!lines.contains(&"--bridge=none"), "{mode}"); + assert!(lines.contains(&"--iptables=true"), "{mode}"); + assert_eq!( + lines.contains(&"--host-gateway-ip=100.96.0.1"), + mode == "host-gateway" + ); + } + let (refused, argv) = daemon_arguments("bridge"); + assert!(refused); + assert!(argv.is_empty()); + assert_eq!(BUILD_NETWORK_SUBNET, "10.213.255.0/24"); +} + +#[test] +fn build_network_script_parses_and_ignores_isolated_pools() { + let script = include_str!("../guest-build-network.sh"); + let run = |args: &[&str]| { + std::process::Command::new("/bin/sh") + .args(["-c", script, "network-test"]) + .args(args) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status() + .unwrap() + .code() + }; + assert!( + std::process::Command::new("/bin/sh") + .args(["-n", "-c", script]) + .status() + .unwrap() + .success() + ); + assert_eq!(run(&["isolated", "boot"]), Some(0)); + assert_eq!(run(&["bridge", "boot"]), Some(64)); + assert_eq!(run(&[]), Some(64)); + assert!(script.contains(BUILD_NETWORK_MARKER)); +} diff --git a/packages/runtime-core/src/provider/lifecycle.rs b/packages/runtime-core/src/provider/lifecycle.rs index 9c3a20650..2b0f272d7 100644 --- a/packages/runtime-core/src/provider/lifecycle.rs +++ b/packages/runtime-core/src/provider/lifecycle.rs @@ -1658,6 +1658,21 @@ exit 43"#, } owner.save(candidate)?; engine_ping(&socket(candidate, owner, "docker.sock")?)?; + if network_mode != "isolated" { + // Build-bridge isolation is not a startup requirement: graph containers never + // join the default bridge. A failure leaves no readiness marker, and native + // image builds refuse on that pool (see `image_build::verify_build_network`). + let _ = guest( + candidate, + owner, + include_str!("guest-build-network.sh"), + &[ + network_mode, + owner.guest_boot_id.as_deref().expect("verified boot"), + ], + false, + ); + } phase(candidate, owner, "running")?; status(candidate) } @@ -1817,6 +1832,8 @@ pub fn down(candidate: &Candidate) -> Result { } artifact::verify(candidate)?; let _lock = state::Lock::acquire(&root(candidate))?; + // Builds take their shared fence only under this lease, so none can start after this. + super::image_build::require_no_active_build(candidate)?; let mut owner = Owner::load(candidate)?; super::hostname_authority::managed::stop(candidate, &owner)?; super::publication::release(candidate, &owner.token, None)?; diff --git a/packages/runtime-core/src/provider/mod.rs b/packages/runtime-core/src/provider/mod.rs index 5a7337fee..c3e1a3f97 100644 --- a/packages/runtime-core/src/provider/mod.rs +++ b/packages/runtime-core/src/provider/mod.rs @@ -34,6 +34,7 @@ pub mod hostname_authority; pub mod http_probe; pub mod https_recovery; mod identity; +pub mod image_build; pub mod image_ensure; mod image_load; pub mod image_reference; diff --git a/packages/runtime-core/src/provider/registry_image.rs b/packages/runtime-core/src/provider/registry_image.rs index 19c02c89f..735101f2b 100644 --- a/packages/runtime-core/src/provider/registry_image.rs +++ b/packages/runtime-core/src/provider/registry_image.rs @@ -336,14 +336,26 @@ pub struct ResolvedImage { /// fetch must use pinned_reference: reusing the original tag would discard this review. pub fn resolve(reference_text: &str) -> Result { let (repo, tag) = tagged_reference(reference_text)?; - let (repo, tag) = (repo.as_str(), tag.as_str()); + resolve_manifest(&repo, &tag, None).map(|(resolved, _)| resolved) +} + +/// Resolve one repository at a tag or digest to a Linux ARM64 image and its config bytes. +/// `expected_source` binds a digest-pinned request to the manifest actually returned. +fn resolve_manifest( + repo: &str, + reference: &str, + expected_source: Option<&str>, +) -> Result<(ResolvedImage, Vec), CandidateError> { let fetch = authenticated_fetch(repo)?; let url = Url::parse(&format!( - "https://registry-1.docker.io/v2/{repo}/manifests/{tag}" + "https://registry-1.docker.io/v2/{repo}/manifests/{reference}" )) .map_err(|_| error("Invalid manifest address."))?; let initial = fetch.bytes(url, MAX_METADATA, true)?; let source_digest = digest(&initial); + if expected_source.is_some_and(|expected| expected != source_digest) { + return Err(error("Registry manifest digest or size mismatch.")); + } let mut manifest = parse_manifest(&initial, &source_digest)?; let mut manifest_digest = source_digest.clone(); if !manifest.manifests.is_empty() { @@ -360,12 +372,66 @@ pub fn resolve(reference_text: &str) -> Result { .ok_or_else(|| error("Image config descriptor is missing."))?; let config_bytes = fetch.object(repo, "blobs", &config, MAX_METADATA)?; verify_platform(&config_bytes)?; - Ok(ResolvedImage { - pinned_reference: format!("docker.io/{repo}@{source_digest}"), - source_digest, - manifest_digest, - image_id: config.digest, - platform: "linux/arm64", + Ok(( + ResolvedImage { + pinned_reference: format!("docker.io/{repo}@{source_digest}"), + source_digest, + manifest_digest, + image_id: config.digest, + platform: "linux/arm64", + }, + config_bytes, + )) +} + +/// A base image a native build may pull, pinned to the exact manifest the registry +/// served when the build was admitted. +#[derive(Clone, Debug, Eq, PartialEq, Serialize)] +pub struct BuildBase { + /// `docker.io/@`: the only form handed to the builder. + pub pinned_reference: String, + /// Digest of the manifest or index the reference resolved to. + pub source_digest: String, + /// Content ID of the selected Linux ARM64 image. + pub image_id: String, +} + +/// Resolve one normalized base-image reference for a native build. A tag resolves once +/// to its current manifest; a digest-pinned reference must resolve to exactly that pin. +/// Either way the image must offer exactly one Linux ARM64 image, and its configuration +/// must declare no `ONBUILD` triggers: a stored trigger could pull images the Dockerfile +/// scan never saw. No layers, credentials or runtime state are touched. +pub fn resolve_build_base(named: &NamedReference) -> Result { + let repo = named.docker_hub_repository().ok_or_else(|| { + error("Only public Docker Hub [namespace/]repository[:tag][@sha256] references are supported.") + })?; + let (reference, expected) = match (&named.digest, &named.tag) { + (Some(pin), _) => (pin.as_str(), Some(pin.as_str())), + (None, Some(tag)) => (tag.as_str(), None), + (None, None) => ("latest", None), + }; + let (resolved, config) = resolve_manifest(repo, reference, expected)?; + if declares_onbuild(&config)? { + return Err(CandidateError::new( + "native_build_base_image", + "A build base image declares ONBUILD triggers; native builds refuse stored triggers.", + )); + } + Ok(BuildBase { + pinned_reference: resolved.pinned_reference, + source_digest: resolved.source_digest, + image_id: resolved.image_id, + }) +} + +/// Whether an image configuration carries stored `ONBUILD` triggers. +fn declares_onbuild(config_bytes: &[u8]) -> Result { + let config: serde_json::Value = + serde_json::from_slice(config_bytes).map_err(|_| error("Invalid image configuration."))?; + Ok(match &config["config"]["OnBuild"] { + serde_json::Value::Null => false, + serde_json::Value::Array(triggers) => !triggers.is_empty(), + _ => return Err(error("Invalid image configuration.")), }) } fn verify_platform(config_bytes: &[u8]) -> Result<(), CandidateError> { @@ -503,6 +569,26 @@ fn publish( #[cfg(test)] mod tests { use super::*; + #[test] + fn stored_onbuild_triggers_are_detected_and_malformed_configs_refuse() { + assert!(!declares_onbuild(br#"{"config":{}}"#).unwrap()); + assert!(!declares_onbuild(br#"{"config":{"OnBuild":null}}"#).unwrap()); + assert!(!declares_onbuild(br#"{"config":{"OnBuild":[]}}"#).unwrap()); + assert!(declares_onbuild(br#"{"config":{"OnBuild":["COPY --from=x / /"]}}"#).unwrap()); + assert!(declares_onbuild(br#"{"config":{"OnBuild":"RUN x"}}"#).is_err()); + assert!(declares_onbuild(b"not json").is_err()); + } + + #[test] + fn build_bases_accept_only_docker_hub_references() { + let named = |value: &str| match normalize(value).unwrap() { + ImageReference::Named(named) => named, + ImageReference::ImageId(_) => unreachable!(), + }; + let error = resolve_build_base(&named("ghcr.io/example/app:1")).unwrap_err(); + assert_eq!(error.code, "registry_image"); + } + #[test] fn publication_preserves_existing_output_and_failed_writes_leave_no_final_file() { static NEXT_TEST: AtomicU64 = AtomicU64::new(0); From e5eb57778f22c3c83ffa23e335c3d5c5b1c5f35d Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Thu, 5 Nov 2026 17:50:18 -0500 Subject: [PATCH 02/11] test(native): keep the build cancellation test valid on a loaded host The fake client now publishes its child PID atomically, cancellation waits for that published PID, and the timeout case checks the child only when the client forked it within the one-second budget. Refs HACK-1270 Co-Authored-By: Claude Opus 5.5 --- .../src/provider/image_build/tests.rs | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/runtime-core/src/provider/image_build/tests.rs b/packages/runtime-core/src/provider/image_build/tests.rs index 4b209401b..eeec6f91b 100644 --- a/packages/runtime-core/src/provider/image_build/tests.rs +++ b/packages/runtime-core/src/provider/image_build/tests.rs @@ -86,7 +86,7 @@ case {mode} in ok) printf '{{"containerimage.config.digest":"{IMAGE}"}}' > "$meta" ;; fail) exit 3 ;; nometa) ;; - sleep) sleep 30 & echo $! > child; wait ;; + sleep) sleep 30 & echo $! > child.pending; mv child.pending child; wait ;; esac "# ); @@ -348,6 +348,14 @@ fn client_failure_and_missing_metadata_adopt_nothing() { assert_eq!(error.code, "native_build_image"); } +fn wait_dead(pid: &str) { + let deadline = Instant::now() + Duration::from_secs(30); + while process_alive(pid) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(20)); + } + assert!(!process_alive(pid), "a client group member survived"); +} + fn process_alive(pid: &str) -> bool { let pid: libc::pid_t = pid.trim().parse().unwrap(); // SAFETY: signal 0 only probes existence. @@ -367,27 +375,22 @@ fn timeout_and_cancellation_stop_the_whole_client_group() { assert_eq!(error.code, "native_build_timeout"); assert!(started.elapsed() < Duration::from_secs(20)); assert_eq!(fixture.receipt(&inputs).phase, Phase::Canceled); - let child = fs::read_to_string(fixture.scratch(&inputs).join("child")).unwrap(); - let deadline = Instant::now() + Duration::from_secs(10); - while process_alive(&child) && Instant::now() < deadline { - std::thread::sleep(Duration::from_millis(20)); + // On a loaded host the client may not have forked its child within the budget. + let child = fixture.scratch(&inputs).join("child"); + if let Ok(pid) = fs::read_to_string(&child) { + wait_dead(&pid); } - assert!(!process_alive(&child), "a client group member survived"); let inputs = fixture.inputs(Policy::Missing); fixture.client(&inputs, "sleep"); - fs::remove_file(fixture.scratch(&inputs).join("child")).unwrap(); - let scratch = fixture.scratch(&inputs); - let cancel = move || scratch.join("child").exists(); + let _ = fs::remove_file(&child); + let marker = child.clone(); + let cancel = move || marker.exists(); let error = run(&fixture.candidate, &pool, &inputs, &cancel).unwrap_err(); assert_eq!(error.code, "native_build_canceled"); assert_eq!(fixture.receipt(&inputs).phase, Phase::Canceled); - let child = fs::read_to_string(fixture.scratch(&inputs).join("child")).unwrap(); - let deadline = Instant::now() + Duration::from_secs(10); - while process_alive(&child) && Instant::now() < deadline { - std::thread::sleep(Duration::from_millis(20)); - } - assert!(!process_alive(&child)); + // Cancellation was requested only once the child existed (published atomically). + wait_dead(&fs::read_to_string(&child).unwrap()); // A request canceled before the client starts never runs it. let error = run(&fixture.candidate, &pool, &inputs, &|| true).unwrap_err(); From d2a2080e43a527a48f333d16b0050976bad3c289 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:06:39 -0500 Subject: [PATCH 03/11] test(native): name an owned network in engine admission controls The default-bridge guard refuses a container create that names no network before admission is consulted, so the dispatch-level admission controls (built only with environment-launcher) now send NetworkMode none. Co-Authored-By: Claude Opus 5.5 --- .../src/provider/engine/admission_tests.rs | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/runtime-core/src/provider/engine/admission_tests.rs b/packages/runtime-core/src/provider/engine/admission_tests.rs index 3fcf9ad19..a46a489b2 100644 --- a/packages/runtime-core/src/provider/engine/admission_tests.rs +++ b/packages/runtime-core/src/provider/engine/admission_tests.rs @@ -72,6 +72,12 @@ fn empty_admission() -> super::super::managed_environment::Managed { managed } +/// Every create Hack dispatches names its network; the engine guard refuses one that +/// does not before admission is consulted, so these controls name `none`. +fn owned_create() -> Value { + serde_json::json!({"HostConfig": {"NetworkMode": "none"}}) +} + fn expire(deadline: Instant) { while let Some(remaining) = deadline.checked_duration_since(Instant::now()) { std::thread::sleep(remaining + Duration::from_millis(1)); @@ -129,7 +135,11 @@ fn empty_admission_expiring_before_create_sends_no_request() { let transport = fixture.transport(managed.deadline(), Duration::from_millis(250)); expire(managed.deadline()); let failure = transport - .request(Method::POST, "/v1.53/containers/create?name=owned", None) + .request( + Method::POST, + "/v1.53/containers/create?name=owned", + Some(&owned_create()), + ) .unwrap_err(); assert_eq!(failure.code, "graph_environment_input"); fixture.assert_no_request(); @@ -183,7 +193,11 @@ fn empty_admission_expiring_after_create_blocks_start_but_allows_owned_delete() None, ); transport - .request(Method::POST, "/v1.53/containers/create?name=owned", None) + .request( + Method::POST, + "/v1.53/containers/create?name=owned", + Some(&owned_create()), + ) .unwrap(); create.join().unwrap(); let id = "1".repeat(64); @@ -243,7 +257,11 @@ fn in_flight_mutation_cannot_renew_empty_admission_transport_budget() { // The request was dispatched while valid, but its response arrives after expiry. // Without the remaining-lifetime timeout it would succeed under the 3s default. let failure = transport - .request(Method::POST, "/v1.53/containers/create?name=owned", None) + .request( + Method::POST, + "/v1.53/containers/create?name=owned", + Some(&owned_create()), + ) .unwrap_err(); assert_eq!(failure.code, "engine_protocol"); assert!(failure.message.contains("effect may be uncertain")); From a0c58c89b56b271d1394d6faad4e01c4b92d4055 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:06:43 -0500 Subject: [PATCH 04/11] fix(native): build from the scanned Dockerfile and filter build output The build client now reads a private copy of the Dockerfile bytes that were scanned and hashed, with the effective ignore rules written beside it, instead of reopening the project paths while bases resolve. Its stderr is piped through a filter that redacts every build-argument value, strips terminal control characters, cuts long lines and bounds total output before it reaches the caller, and stops writing before the operation returns. Co-Authored-By: Claude Opus 5.5 --- docs/guides/native-candidate.md | 10 +- .../runtime-core/src/provider/image_build.rs | 117 +++++-- .../src/provider/image_build/output.rs | 309 ++++++++++++++++++ .../src/provider/image_build/tests.rs | 96 +++++- 4 files changed, 510 insertions(+), 22 deletions(-) create mode 100644 packages/runtime-core/src/provider/image_build/output.rs diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index 74caf19b2..a8f5b187d 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -471,11 +471,17 @@ linked, mode-`0600` file owned by you), because build-argument values are in it: - The context and Dockerfile must be inside the project and reached without symlinks. The Dockerfile is read, never run, to find every image it can pull. Each image must be a public Docker Hub reference; it is pinned to a digest before the build starts. - Base images with stored `ONBUILD` triggers refuse. + Base images with stored `ONBUILD` triggers refuse. The client builds from a private + copy of the Dockerfile and ignore rules that were scanned, so editing either file + during the build has no effect on it. - Argument values reach the client only through its environment, never its command - line, receipts or output. Names that would change the client's own environment + line or receipts. Names that would change the client's own environment (`PATH`, `HOME`, `DOCKER_*`, `BUILDX_*`, `BUILDKIT_*`, proxies, locale, Git and SSH settings) refuse. +- Build output is shown on stderr after filtering: every argument value is replaced + with `[redacted]`, terminal control characters are removed, lines are cut at 16 KiB + and output stops after 4 MiB. A value that a Dockerfile transforms before printing + it (for example, by encoding it) is not detected. - `missing` reuses a verified image for the same build key without running the client; `rebuild` always builds. The key covers the Dockerfile and `.dockerignore` bytes, target, arguments and versions, not the context's other files (as with Compose). diff --git a/packages/runtime-core/src/provider/image_build.rs b/packages/runtime-core/src/provider/image_build.rs index ff6268d46..d276e3ce7 100644 --- a/packages/runtime-core/src/provider/image_build.rs +++ b/packages/runtime-core/src/provider/image_build.rs @@ -27,8 +27,12 @@ //! is released only when its holder exits): it is recorded `indeterminate` and the //! image is built again. Build outputs are content addressed and adoption always //! rechecks labels and incarnation, so no operator action is needed. -//! - Argument values never appear in argv, receipts, messages or output: the client -//! receives `--build-arg NAME` and reads the value from its private environment. +//! - Argument values never appear in argv, receipts or messages: the client receives +//! `--build-arg NAME` and reads the value from its private environment. Its output +//! reaches the caller only through [`output::Filter`], which redacts every value and +//! strips terminal control sequences, within fixed bounds. +//! - The client builds from a private copy of the Dockerfile and ignore bytes that were +//! scanned and hashed, never from the project paths again. //! - Nothing here removes images, prunes caches or adopts an image whose labels differ. use super::{ artifact, @@ -43,7 +47,7 @@ use sha2::{Digest, Sha256}; use std::{ collections::BTreeMap, fs::{self, File, OpenOptions}, - io::Read, + io::{Read, Write}, os::fd::AsRawFd, os::unix::fs::{MetadataExt, OpenOptionsExt}, os::unix::process::CommandExt, @@ -53,6 +57,7 @@ use std::{ time::{Duration, Instant}, }; +mod output; #[cfg(test)] mod tests; @@ -201,15 +206,24 @@ pub(super) trait BuildPool { fn resolve(&self, base: &BaseImage) -> Result; /// The verified builder executable. fn builder(&self) -> Result; + /// Where the client's filtered output goes. + fn output(&self) -> Box; } /// Validated local inputs and the build key derived from them. +/// +/// The Dockerfile and its effective ignore file are held as the exact bytes that were +/// scanned and hashed: the client builds from a private copy of them, never from the +/// mutable project paths, so a file edited while bases resolve cannot reach BuildKit. #[derive(Debug)] pub(super) struct Inputs { namespace: String, key: String, context: PathBuf, - dockerfile: PathBuf, + dockerfile: Vec, + /// The ignore rules BuildKit applies: the Dockerfile-specific file if present, else + /// the context's `.dockerignore`, else empty (no exclusions). + ignore: Vec, target: Option, args: BTreeMap, images: Vec, @@ -395,17 +409,34 @@ fn read_bounded_file(path: &Path, limit: u64) -> Result>, Candida /// The ignore files BuildKit applies to this context: a Dockerfile-specific /// `.dockerignore` wins over the context's `.dockerignore`. -fn ignore_digests(context: &Path, dockerfile: &Path) -> Result { - let mut specific = dockerfile.as_os_str().to_owned(); - specific.push(".dockerignore"); - let digest = |path: &Path| -> Result { - Ok(read_bounded_file(path, MAX_IGNORE_BYTES)? - .map_or(Value::Null, |bytes| Value::String(sha256_hex(&bytes)))) - }; - Ok(serde_json::json!({ - "context": digest(&context.join(".dockerignore"))?, - "dockerfile": digest(Path::new(&specific))?, - })) +struct IgnoreFiles { + context: Option>, + dockerfile: Option>, +} +impl IgnoreFiles { + fn read(context: &Path, dockerfile: &Path) -> Result { + let mut specific = dockerfile.as_os_str().to_owned(); + specific.push(".dockerignore"); + Ok(Self { + context: read_bounded_file(&context.join(".dockerignore"), MAX_IGNORE_BYTES)?, + dockerfile: read_bounded_file(Path::new(&specific), MAX_IGNORE_BYTES)?, + }) + } + fn digests(&self) -> Value { + let digest = |bytes: &Option>| { + bytes + .as_deref() + .map_or(Value::Null, |bytes| Value::String(sha256_hex(bytes))) + }; + serde_json::json!({ + "context": digest(&self.context), + "dockerfile": digest(&self.dockerfile), + }) + } + /// The rules in force. An empty file excludes nothing, exactly like an absent one. + fn effective(self) -> Vec { + self.dockerfile.or(self.context).unwrap_or_default() + } } /// Validate the request, resolve paths inside the project without following symlinks, @@ -463,13 +494,14 @@ pub(super) fn prepare(request: BuildRequest) -> Result { (name.as_str(), format!("{:x}", digest.finalize())) }) .collect(); + let ignore = IgnoreFiles::read(&context, &dockerfile)?; let key_input = serde_json::json!({ "v": 1, "namespace": namespace, "context": relative(&context, &root), "dockerfile": relative(&dockerfile, &context), "dockerfile_sha256": sha256_hex(&bytes), - "dockerignore": ignore_digests(&context, &dockerfile)?, + "dockerignore": ignore.digests(), "target": request.target, "args": salted_args, "platform": "linux/arm64", @@ -482,7 +514,8 @@ pub(super) fn prepare(request: BuildRequest) -> Result { namespace, key, context, - dockerfile, + dockerfile: bytes, + ignore: ignore.effective(), target: request.target, args: request.args, images: scan.images, @@ -759,6 +792,8 @@ pub(super) fn run( )?, inputs.timeout, canceled, + output::Filter::new(inputs.args.values().map(String::as_str)), + pool.output(), ); let ended = match ended { Ok(ended) => ended, @@ -869,6 +904,37 @@ fn built_image_id(metadata: &Path) -> Result { Ok(id.to_owned()) } +/// Write the scanned Dockerfile bytes and the effective ignore rules into a private +/// directory the client reads them from. The client applies `.dockerignore` +/// beside the Dockerfile in preference to the context's `.dockerignore`, so writing the +/// effective rules there (empty when there are none) keeps the hashed ignore semantics +/// without reading either project file again. +fn stage_dockerfile(scratch: &Path, inputs: &Inputs) -> Result { + let directory = scratch.join("dockerfile"); + state::private_directory(&directory)?; + let dockerfile = directory.join("Dockerfile"); + write_private(&dockerfile, &inputs.dockerfile)?; + write_private(&directory.join("Dockerfile.dockerignore"), &inputs.ignore)?; + Ok(dockerfile) +} + +/// Replace one private file with exactly these bytes, never following a symlink. +fn write_private(path: &Path, bytes: &[u8]) -> Result<(), CandidateError> { + match fs::remove_file(path) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(state::io(error)), + } + let mut file = OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .custom_flags(libc::O_NOFOLLOW) + .open(path) + .map_err(state::io)?; + file.write_all(bytes).map_err(state::io) +} + /// The client command: argv carries only paths, names and non-secret labels; argument /// values reach the client through its environment. fn command( @@ -884,6 +950,7 @@ fn command( let buildx = scratch.join("buildx"); state::private_directory(&config)?; state::private_directory(&buildx)?; + let dockerfile = stage_dockerfile(scratch, inputs)?; let mut command = super::process::clean_command(builder); command .current_dir(scratch) @@ -902,7 +969,7 @@ fn command( .args(["build", "--progress=plain", "--platform", "linux/arm64"]) .args(["--provenance=false", "--sbom=false", "--load"]) .arg("--file") - .arg(&inputs.dockerfile) + .arg(&dockerfile) .arg("--metadata-file") .arg(metadata) .args(["--tag", &inputs.tag()]); @@ -924,7 +991,8 @@ fn command( .arg(&inputs.context) .stdin(Stdio::null()) .stdout(Stdio::null()) - .stderr(Stdio::inherit()) + // Output reaches the caller only through `output::Filter`. + .stderr(Stdio::piped()) .process_group(0); Ok(command) } @@ -979,10 +1047,14 @@ impl Drop for CancelSignals { /// Spawn the client and wait for it, honoring cancellation and the time budget. On /// either, the whole process group gets SIGTERM, then SIGKILL after a grace period. /// The group is signaled only while its leader is unreaped, so its ID is still ours. +/// The client's stderr passes through `filter` to `sink`; forwarding ends, bounded, +/// before this returns, so nothing from the client follows the operation's result. fn supervise( mut command: std::process::Command, timeout: Duration, canceled: &dyn Fn() -> bool, + filter: output::Filter, + sink: Box, ) -> Result { let mut child = command.spawn().map_err(|_| { refusal( @@ -990,6 +1062,10 @@ fn supervise( "The pinned build client could not be started.", ) })?; + let _forwarder = child + .stderr + .take() + .map(|stderr| output::Forwarder::spawn(stderr, filter, sink)); let deadline = Instant::now() + timeout; loop { if let Some(status) = child.try_wait().map_err(state::io)? { @@ -1128,6 +1204,9 @@ impl<'a> BuildPool for LivePool<'a> { artifact::verify_builder(self.candidate)?; Ok(artifact::builder_executable(self.candidate)) } + fn output(&self) -> Box { + Box::new(std::io::stderr()) + } } /// `runtime build-image --request `: build or reuse one native image. diff --git a/packages/runtime-core/src/provider/image_build/output.rs b/packages/runtime-core/src/provider/image_build/output.rs new file mode 100644 index 000000000..9a4fbf215 --- /dev/null +++ b/packages/runtime-core/src/provider/image_build/output.rs @@ -0,0 +1,309 @@ +//! The build client's output boundary. +//! +//! The client's stderr is a pipe read here, never inherited. Each line is redacted of +//! every build-argument value (the request treats them all as secrets), stripped of +//! terminal control characters and escape sequences, cut at a fixed length, and counted +//! against a fixed budget before it is written to the caller's stream. Redaction is +//! literal: a Dockerfile that transforms a value before printing it (encoding, slicing) +//! is not detected. +use std::{ + io::{Read, Write}, + sync::{Arc, Mutex, mpsc}, + thread::JoinHandle, + time::Duration, +}; + +pub(super) const REDACTED: &str = "[redacted]"; +const LINE_CUT: &str = " [line truncated]"; +const OUTPUT_CUT: &str = "[build output truncated]\n"; +/// Longest forwarded line; the rest of a longer line is dropped. +const MAX_LINE_BYTES: usize = 16 * 1024; +/// Total forwarded bytes; later output is drained and dropped. +const MAX_OUTPUT_BYTES: usize = 4 * 1024 * 1024; +/// How long forwarding may continue after the client exits (a process outside its group +/// can hold the pipe open). After that, nothing more is written. +const DRAIN_WAIT: Duration = Duration::from_secs(2); + +/// Line filter: redaction, terminal sanitizing and bounds. +pub(super) struct Filter { + /// Literal byte strings to redact, longest first. + needles: Vec>, + forwarded: usize, + exhausted: bool, +} + +/// Keep tabs and printable characters; drop every other control character, which +/// removes the ESC that starts any terminal escape sequence. +fn sanitize(bytes: &[u8]) -> Vec { + String::from_utf8_lossy(bytes) + .chars() + .filter(|c| *c == '\t' || !c.is_control()) + .collect::() + .into_bytes() +} + +fn redact(bytes: &[u8], needles: &[Vec]) -> Vec { + let mut out = Vec::with_capacity(bytes.len()); + let mut at = 0; + 'scan: while at < bytes.len() { + for needle in needles { + if bytes[at..].starts_with(needle) { + out.extend_from_slice(REDACTED.as_bytes()); + at += needle.len(); + continue 'scan; + } + } + out.push(bytes[at]); + at += 1; + } + out +} + +impl Filter { + /// Redact each value and each of its lines, raw and as sanitizing would leave them, + /// so a value spanning lines or containing control characters is still caught. + pub(super) fn new<'a>(values: impl IntoIterator) -> Self { + let mut needles = Vec::new(); + for value in values { + let parts = std::iter::once(value).chain(value.split(['\n', '\r'])); + for part in parts { + needles.push(part.as_bytes().to_vec()); + needles.push(sanitize(part.as_bytes())); + } + } + needles.retain(|needle| !needle.is_empty()); + needles.sort_by(|a, b| b.len().cmp(&a.len()).then_with(|| a.cmp(b))); + needles.dedup(); + Self { + needles, + forwarded: 0, + exhausted: false, + } + } + + /// The bytes to forward for one line (without its terminator). `cut` marks a line + /// longer than the line bound whose remainder is dropped: a trailing partial match + /// of any value is removed too, so no prefix of a value survives the cut. + pub(super) fn line(&mut self, line: &[u8], cut: bool) -> Vec { + if self.exhausted { + return Vec::new(); + } + let mut raw = line; + let mut partial = false; + if cut { + let tail = self + .needles + .iter() + .filter_map(|needle| { + (1..needle.len().min(raw.len() + 1)) + .rev() + .find(|k| raw.ends_with(&needle[..*k])) + }) + .max() + .unwrap_or(0); + raw = &raw[..raw.len() - tail]; + partial = tail > 0; + } + let mut out = redact(&sanitize(&redact(raw, &self.needles)), &self.needles); + if partial { + out.extend_from_slice(REDACTED.as_bytes()); + } + if cut { + out.extend_from_slice(LINE_CUT.as_bytes()); + } + out.push(b'\n'); + if self.forwarded + out.len() > MAX_OUTPUT_BYTES { + self.exhausted = true; + return OUTPUT_CUT.as_bytes().to_vec(); + } + self.forwarded += out.len(); + out + } + + /// Split a byte stream into bounded lines and filter each one. + fn pump(&mut self, mut source: impl Read, mut emit: impl FnMut(&[u8]) -> bool) { + let mut line = Vec::new(); + let mut skipping = false; + let mut chunk = [0_u8; 8192]; + loop { + let size = match source.read(&mut chunk) { + Ok(0) => break, + Ok(size) => size, + // The cancellation handlers are installed without SA_RESTART. + Err(error) if error.kind() == std::io::ErrorKind::Interrupted => continue, + Err(_) => break, + }; + for &byte in &chunk[..size] { + if byte == b'\n' { + if !skipping && !emit(&self.line(&line, false)) { + return; + } + line.clear(); + skipping = false; + } else if !skipping { + line.push(byte); + if line.len() == MAX_LINE_BYTES { + if !emit(&self.line(&line, true)) { + return; + } + line.clear(); + skipping = true; + } + } + } + } + if !line.is_empty() && !skipping { + emit(&self.line(&line, false)); + } + } +} + +type Sink = Arc>>>; + +/// Forwards one pipe through a [`Filter`] on its own thread. Dropping it waits a +/// bounded time for the pipe to close, then closes the sink so no later write lands. +pub(super) struct Forwarder { + sink: Sink, + done: mpsc::Receiver<()>, + thread: Option>, +} + +impl Forwarder { + pub(super) fn spawn( + source: impl Read + Send + 'static, + mut filter: Filter, + sink: Box, + ) -> Self { + let sink: Sink = Arc::new(Mutex::new(Some(sink))); + let shared = Arc::clone(&sink); + let (sender, done) = mpsc::channel(); + let thread = std::thread::spawn(move || { + filter.pump(source, |bytes| { + let mut guard = shared.lock().unwrap_or_else(|e| e.into_inner()); + let Some(sink) = guard.as_mut() else { + return false; + }; + // A failing caller stream must not stall the client; keep draining. + let _ = sink.write_all(bytes).and_then(|()| sink.flush()); + true + }); + let _ = sender.send(()); + }); + Self { + sink, + done, + thread: Some(thread), + } + } +} + +impl Drop for Forwarder { + fn drop(&mut self) { + let finished = self.done.recv_timeout(DRAIN_WAIT).is_ok(); + self.sink.lock().unwrap_or_else(|e| e.into_inner()).take(); + if let Some(thread) = self.thread.take().filter(|_| finished) { + let _ = thread.join(); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn filtered(values: &[&str], input: &[u8]) -> String { + let mut filter = Filter::new(values.iter().copied()); + let mut out = Vec::new(); + filter.pump(input, |bytes| { + out.extend_from_slice(bytes); + true + }); + String::from_utf8(out).unwrap() + } + + #[test] + fn values_are_redacted_even_across_lines_and_control_characters() { + let out = filtered( + &["s3cret", "two\nlines", "es\x1bc"], + b"#5 s3cret and s3cret\nhead two\nlines tail\nraw es\x1bc and es[c\n", + ); + assert!(!out.contains("s3cret") && !out.contains("two") && !out.contains("lines")); + assert_eq!(out.matches(REDACTED).count(), 5, "{out}"); + assert!(!out.contains("esc"), "{out}"); + assert!(!out.contains('\x1b')); + } + + #[test] + fn terminal_control_sequences_are_stripped_and_tabs_kept() { + let out = filtered( + &[], + b"\x1b[2J\x1b]0;title\x07ok\tdone\r\n\x00\x7f\xc2\x9bend", + ); + assert_eq!(out, "[2J]0;titleok\tdone\nend\n"); + assert!( + out.bytes() + .all(|b| b == b'\t' || b == b'\n' || !b.is_ascii_control()) + ); + } + + #[test] + fn long_lines_are_cut_without_leaving_a_value_prefix() { + let secret = "abcdefgh"; + let mut input = vec![b'x'; MAX_LINE_BYTES - 3]; + input.extend_from_slice(b"abcdefgh-rest-of-line\nnext\n"); + let out = filtered(&[secret], &input); + let first = out.lines().next().unwrap(); + assert!( + first.ends_with(&format!("{REDACTED}{LINE_CUT}")), + "{first:?}" + ); + assert!(!first.contains("abc")); + assert!(!out.contains("rest-of-line")); + assert!(out.ends_with("next\n")); + } + + #[test] + fn output_beyond_the_budget_is_dropped_with_one_notice() { + let line = vec![b'y'; 1023]; + let mut input = Vec::new(); + for _ in 0..(MAX_OUTPUT_BYTES / 1024 + 10) { + input.extend_from_slice(&line); + input.push(b'\n'); + } + let out = filtered(&[], &input); + assert!(out.len() <= MAX_OUTPUT_BYTES + OUTPUT_CUT.len()); + assert!(out.ends_with(OUTPUT_CUT)); + assert_eq!(out.matches(OUTPUT_CUT).count(), 1); + } + + #[test] + fn a_held_pipe_does_not_outlive_the_forwarder() { + let (reader, writer) = std::os::unix::net::UnixStream::pair().unwrap(); + let captured = Arc::new(Mutex::new(Vec::new())); + struct Shared(Arc>>); + impl Write for Shared { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + let forwarder = Forwarder::spawn( + reader, + Filter::new(["v"]), + Box::new(Shared(Arc::clone(&captured))), + ); + (&writer).write_all(b"before v\n").unwrap(); + let started = std::time::Instant::now(); + while captured.lock().unwrap().is_empty() { + assert!(started.elapsed() < Duration::from_secs(5)); + std::thread::sleep(Duration::from_millis(5)); + } + drop(forwarder); + (&writer).write_all(b"after\n").unwrap(); + std::thread::sleep(Duration::from_millis(50)); + assert_eq!(&*captured.lock().unwrap(), b"before [redacted]\n"); + } +} diff --git a/packages/runtime-core/src/provider/image_build/tests.rs b/packages/runtime-core/src/provider/image_build/tests.rs index eeec6f91b..7dde84ed0 100644 --- a/packages/runtime-core/src/provider/image_build/tests.rs +++ b/packages/runtime-core/src/provider/image_build/tests.rs @@ -4,7 +4,11 @@ //! working directory, so no test writes and then executes a fresh binary. It records its //! argv, the one build-argument variable it was given, and a call count. use super::*; -use std::{cell::RefCell, os::unix::fs::PermissionsExt}; +use std::{ + cell::RefCell, + os::unix::fs::PermissionsExt, + sync::{Arc, Mutex}, +}; const IMAGE: &str = "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; const PIN: &str = "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; @@ -77,13 +81,20 @@ echo $((count + 1)) > calls printf '%s\n' "$@" > argv printf '%s' "${{GREETING-}}" > greeting meta= +file= previous= for argument do if [ "$previous" = --metadata-file ]; then meta=$argument; fi + if [ "$previous" = --file ]; then file=$argument; fi previous=$argument done +cat "$file" > seen-dockerfile +cat "$file.dockerignore" > seen-dockerignore case {mode} in ok) printf '{{"containerimage.config.digest":"{IMAGE}"}}' > "$meta" ;; + leak) + printf '#5 0.1 value=%s\n\033[31mred\033[0m\033]0;title\007\n' "$GREETING" >&2 + printf '{{"containerimage.config.digest":"{IMAGE}"}}' > "$meta" ;; fail) exit 3 ;; nometa) ;; sleep) sleep 30 & echo $! > child.pending; mv child.pending child; wait ;; @@ -120,6 +131,7 @@ struct FakePool { images: RefCell>, leases: RefCell, resolve_error: bool, + output: Arc>>, } impl FakePool { fn new(incarnation: &str) -> Self { @@ -128,6 +140,7 @@ impl FakePool { images: RefCell::new(BTreeMap::new()), leases: RefCell::new(0), resolve_error: false, + output: Arc::default(), } } /// Register the image the fake client "builds", with the labels the build expects. @@ -171,6 +184,21 @@ impl BuildPool for FakePool { fn builder(&self) -> Result { Ok(PathBuf::from("/bin/sh")) } + fn output(&self) -> Box { + Box::new(Captured(Arc::clone(&self.output))) + } +} + +/// The caller's stream as the fake pool sees it. +struct Captured(Arc>>); +impl Write for Captured { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + self.0.lock().unwrap().extend_from_slice(bytes); + Ok(bytes.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } } fn never() -> bool { @@ -231,6 +259,72 @@ fn cold_build_passes_values_only_through_the_environment_and_records_the_result( assert!(!text.contains(SECRET), "argument value reached the receipt"); } +#[test] +fn client_output_reaches_the_caller_redacted_and_without_terminal_controls() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + let pool = FakePool::new("pool-1"); + fixture.client(&inputs, "leak"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + run(&fixture.candidate, &pool, &inputs, &never).unwrap(); + let output = String::from_utf8(pool.output.lock().unwrap().clone()).unwrap(); + assert_eq!(output, "#5 0.1 value=[redacted]\n[31mred[0m]0;title\n"); + assert!(!output.contains(SECRET)); +} + +#[test] +fn the_client_builds_the_scanned_dockerfile_and_ignore_rules_not_later_edits() { + let fixture = Fixture::new(); + let app = fixture.project.join("app"); + let scanned = fs::read(app.join("Dockerfile")).unwrap(); + fs::write(app.join(".dockerignore"), "*.txt\n").unwrap(); + let inputs = fixture.inputs(Policy::Missing); + // Edits after the scan (while bases resolve) must not reach the client. + fs::write(app.join("Dockerfile"), "FROM unpinned:latest\nRUN id\n").unwrap(); + fs::write(app.join(".dockerignore"), "!*\n").unwrap(); + fs::write(app.join("Dockerfile.dockerignore"), "main.txt\n").unwrap(); + let pool = FakePool::new("pool-1"); + fixture.client(&inputs, "ok"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + run(&fixture.candidate, &pool, &inputs, &never).unwrap(); + let scratch = fixture.scratch(&inputs); + assert_eq!(fs::read(scratch.join("seen-dockerfile")).unwrap(), scanned); + assert_eq!( + fs::read_to_string(scratch.join("seen-dockerignore")).unwrap(), + "*.txt\n" + ); + let argv = fs::read_to_string(scratch.join("argv")).unwrap(); + let lines: Vec<&str> = argv.lines().collect(); + let file = lines + .windows(2) + .find(|w| w[0] == "--file") + .map(|w| PathBuf::from(w[1])) + .unwrap(); + assert!(file.starts_with(&scratch), "the client read a project path"); + + // A Dockerfile-specific ignore file wins; with none at all, the private rules are + // empty, so a context .dockerignore created later is never applied. + let inputs = fixture.inputs(Policy::Rebuild); + fs::remove_file(app.join("Dockerfile.dockerignore")).unwrap(); + fixture.client(&inputs, "ok"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + run(&fixture.candidate, &pool, &inputs, &never).unwrap_or_else(|e| panic!("{e:?}")); + assert_eq!( + fs::read_to_string(fixture.scratch(&inputs).join("seen-dockerignore")).unwrap(), + "main.txt\n" + ); + fs::remove_file(app.join(".dockerignore")).unwrap(); + let inputs = fixture.inputs(Policy::Rebuild); + fs::write(app.join(".dockerignore"), "*\n").unwrap(); + fixture.client(&inputs, "ok"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + run(&fixture.candidate, &pool, &inputs, &never).unwrap_or_else(|e| panic!("{e:?}")); + assert_eq!( + fs::read(fixture.scratch(&inputs).join("seen-dockerignore")).unwrap(), + b"" + ); +} + #[test] fn verified_image_is_reused_without_running_the_client() { let fixture = Fixture::new(); From 1ea5f339e4f6a1d9e89256a0bab65ff628c5547b Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:19:39 -0500 Subject: [PATCH 05/11] fix(native): bound build shutdown on a stalled caller and time base resolution Closing the output forwarder no longer waits on the sink lock: it marks the sink closed, waits at most one second for a write in progress, and returns, so a caller stream that stops draining cannot hold the operation or its build fence. The time budget now starts before base-image resolution: each resolution gets the operation deadline, runs on its own thread so Ctrl-C, SIGTERM and the deadline interrupt it, and the client gets only what remains. Co-Authored-By: Claude Opus 5.5 --- docs/guides/native-candidate.md | 3 +- .../runtime-core/src/provider/image_build.rs | 83 ++++++++++++++++-- .../src/provider/image_build/output.rs | 84 ++++++++++++++++--- .../src/provider/image_build/tests.rs | 51 ++++++++++- .../src/provider/registry_image.rs | 26 ++++-- 5 files changed, 217 insertions(+), 30 deletions(-) diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index a8f5b187d..9fdad5276 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -490,7 +490,8 @@ linked, mode-`0600` file owned by you), because build-argument values are in it: `native_build_runtime_restart`: stop it and start it again. - `runtime down` refuses while a build runs. Ctrl-C or SIGTERM cancels the build and stops the client's whole process group; so does the time budget (default 30 minutes, - at most 2 hours). + at most 2 hours). Both also apply while base images resolve, before the client + starts: the budget covers the whole operation. Before startup, network-tool preparation additionally requires all four pinned Alpine aarch64 APKs in a current-user-owned private directory (mode `0700`), with diff --git a/packages/runtime-core/src/provider/image_build.rs b/packages/runtime-core/src/provider/image_build.rs index d276e3ce7..ddf3599ed 100644 --- a/packages/runtime-core/src/provider/image_build.rs +++ b/packages/runtime-core/src/provider/image_build.rs @@ -203,7 +203,13 @@ pub(super) trait BuildPool { /// Inspect one image by content ID or tag; `None` when the engine reports it absent. fn inspect(&self, lease: &Self::Lease, image: &str) -> Result, CandidateError>; /// Resolve one base image to its pinned manifest. Network only; no lease is held. - fn resolve(&self, base: &BaseImage) -> Result; + /// Returns, with an error, by `deadline` or soon after `canceled` reports true. + fn resolve( + &self, + base: &BaseImage, + deadline: Instant, + canceled: &dyn Fn() -> bool, + ) -> Result; /// The verified builder executable. fn builder(&self) -> Result; /// Where the client's filtered output goes. @@ -647,19 +653,27 @@ fn owned_image( /// Run one build operation end to end. See the module documentation for the protocol. /// `canceled` reports a pending cancellation request; it is checked before each -/// unleased step and while the client runs. +/// unleased step, during base resolution and while the client runs. The time budget +/// starts here: base resolution and the client share it. pub(super) fn run( candidate: &Candidate, pool: &P, inputs: &Inputs, canceled: &dyn Fn() -> bool, ) -> Result { + let deadline = Instant::now() + inputs.timeout; let canceled_early = || { refusal( "native_build_canceled", "The image build was canceled before the build client started; no image was adopted.", ) }; + let expired_early = || { + refusal( + "native_build_timeout", + "The image build exceeded its time budget before the build client started; no image was adopted.", + ) + }; let root = builds_root(candidate); state::private_directory(&root)?; let directory = root.join(&inputs.namespace); @@ -736,7 +750,10 @@ pub(super) fn run( if canceled() { return Err(canceled_early()); } - let resolved = pool.resolve(base)?; + if Instant::now() >= deadline { + return Err(expired_early()); + } + let resolved = pool.resolve(base, deadline, canceled)?; if !image_id(&resolved.source_digest) || !resolved .pinned_reference @@ -753,6 +770,9 @@ pub(super) fn run( if canceled() { return Err(canceled_early()); } + if Instant::now() >= deadline { + return Err(expired_early()); + } // Step 3, under the lease: same pool, then record the attempt. let (lease, current) = pool.lease()?; @@ -790,7 +810,7 @@ pub(super) fn run( command( &builder, &scratch, &metadata, inputs, &identity, &pins, &expected, )?, - inputs.timeout, + deadline, canceled, output::Filter::new(inputs.args.values().map(String::as_str)), pool.output(), @@ -1051,7 +1071,7 @@ impl Drop for CancelSignals { /// before this returns, so nothing from the client follows the operation's result. fn supervise( mut command: std::process::Command, - timeout: Duration, + deadline: Instant, canceled: &dyn Fn() -> bool, filter: output::Filter, sink: Box, @@ -1066,7 +1086,6 @@ fn supervise( .stderr .take() .map(|stderr| output::Forwarder::spawn(stderr, filter, sink)); - let deadline = Instant::now() + timeout; loop { if let Some(status) = child.try_wait().map_err(state::io)? { return Ok(Ended::Exited(status.success())); @@ -1197,8 +1216,18 @@ impl<'a> BuildPool for LivePool<'a> { Err(error) => Err(error), } } - fn resolve(&self, base: &BaseImage) -> Result { - registry_image::resolve_build_base(&base.reference) + fn resolve( + &self, + base: &BaseImage, + deadline: Instant, + canceled: &dyn Fn() -> bool, + ) -> Result { + let reference = base.reference.clone(); + interruptible( + move || registry_image::resolve_build_base(&reference, deadline), + deadline, + canceled, + ) } fn builder(&self) -> Result { artifact::verify_builder(self.candidate)?; @@ -1209,6 +1238,44 @@ impl<'a> BuildPool for LivePool<'a> { } } +/// Run a blocking, effect-free network step on its own thread and wait for it until +/// `deadline` or a cancellation request. An abandoned step ends by its own deadline; +/// its result is discarded and it changes no local state. +fn interruptible( + step: impl FnOnce() -> Result + Send + 'static, + deadline: Instant, + canceled: &dyn Fn() -> bool, +) -> Result { + let (sender, result) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let _ = sender.send(step()); + }); + loop { + match result.recv_timeout(POLL) { + Ok(outcome) => return outcome, + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => { + return Err(refusal( + "native_build_base_image", + "Base-image resolution ended without a result.", + )); + } + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => {} + } + if canceled() { + return Err(refusal( + "native_build_canceled", + "The image build was canceled before the build client started; no image was adopted.", + )); + } + if Instant::now() >= deadline { + return Err(refusal( + "native_build_timeout", + "The image build exceeded its time budget before the build client started; no image was adopted.", + )); + } + } +} + /// `runtime build-image --request `: build or reuse one native image. pub fn build_image(candidate: &Candidate, request: &Path) -> Result { let request: BuildRequest = state::read_bounded(request, MAX_REQUEST_BYTES)?; diff --git a/packages/runtime-core/src/provider/image_build/output.rs b/packages/runtime-core/src/provider/image_build/output.rs index 9a4fbf215..b3e6c9720 100644 --- a/packages/runtime-core/src/provider/image_build/output.rs +++ b/packages/runtime-core/src/provider/image_build/output.rs @@ -8,9 +8,13 @@ //! is not detected. use std::{ io::{Read, Write}, - sync::{Arc, Mutex, mpsc}, + sync::{ + Arc, Mutex, TryLockError, + atomic::{AtomicBool, Ordering}, + mpsc, + }, thread::JoinHandle, - time::Duration, + time::{Duration, Instant}, }; pub(super) const REDACTED: &str = "[redacted]"; @@ -23,6 +27,9 @@ const MAX_OUTPUT_BYTES: usize = 4 * 1024 * 1024; /// How long forwarding may continue after the client exits (a process outside its group /// can hold the pipe open). After that, nothing more is written. const DRAIN_WAIT: Duration = Duration::from_secs(2); +/// How long closing waits for a write already in progress. A caller stream that stops +/// draining must not hold the operation (and its build fence) open. +const CLOSE_WAIT: Duration = Duration::from_secs(1); /// Line filter: redaction, terminal sanitizing and bounds. pub(super) struct Filter { @@ -158,12 +165,14 @@ impl Filter { } } -type Sink = Arc>>>; - /// Forwards one pipe through a [`Filter`] on its own thread. Dropping it waits a -/// bounded time for the pipe to close, then closes the sink so no later write lands. +/// bounded time for the pipe to close, then marks the sink closed. Each write checks +/// that mark while holding the sink lock, and dropping waits (bounded) for that lock, +/// so no write starts after the drop returns. A write already blocked on a caller +/// stream that stopped draining is abandoned after [`CLOSE_WAIT`] rather than awaited. pub(super) struct Forwarder { - sink: Sink, + sink: Arc>>, + closed: Arc, done: mpsc::Receiver<()>, thread: Option>, } @@ -174,15 +183,16 @@ impl Forwarder { mut filter: Filter, sink: Box, ) -> Self { - let sink: Sink = Arc::new(Mutex::new(Some(sink))); - let shared = Arc::clone(&sink); + let sink = Arc::new(Mutex::new(sink)); + let closed = Arc::new(AtomicBool::new(false)); + let (shared, stop) = (Arc::clone(&sink), Arc::clone(&closed)); let (sender, done) = mpsc::channel(); let thread = std::thread::spawn(move || { filter.pump(source, |bytes| { - let mut guard = shared.lock().unwrap_or_else(|e| e.into_inner()); - let Some(sink) = guard.as_mut() else { + let mut sink = shared.lock().unwrap_or_else(|e| e.into_inner()); + if stop.load(Ordering::Acquire) { return false; - }; + } // A failing caller stream must not stall the client; keep draining. let _ = sink.write_all(bytes).and_then(|()| sink.flush()); true @@ -191,6 +201,7 @@ impl Forwarder { }); Self { sink, + closed, done, thread: Some(thread), } @@ -200,7 +211,14 @@ impl Forwarder { impl Drop for Forwarder { fn drop(&mut self) { let finished = self.done.recv_timeout(DRAIN_WAIT).is_ok(); - self.sink.lock().unwrap_or_else(|e| e.into_inner()).take(); + self.closed.store(true, Ordering::Release); + // Never block on the lock: a writer stuck on a full caller pipe holds it. + let deadline = Instant::now() + CLOSE_WAIT; + while matches!(self.sink.try_lock(), Err(TryLockError::WouldBlock)) + && Instant::now() < deadline + { + std::thread::sleep(Duration::from_millis(5)); + } if let Some(thread) = self.thread.take().filter(|_| finished) { let _ = thread.join(); } @@ -276,6 +294,48 @@ mod tests { assert_eq!(out.matches(OUTPUT_CUT).count(), 1); } + #[test] + fn a_caller_stream_that_stops_draining_cannot_hold_shutdown() { + let (release, blocked) = mpsc::channel::<()>(); + let writes = Arc::new(Mutex::new(0_usize)); + struct Stuck(mpsc::Receiver<()>, Arc>); + impl Write for Stuck { + fn write(&mut self, bytes: &[u8]) -> std::io::Result { + *self.1.lock().unwrap() += 1; + // Like a full pipe whose reader stopped consuming. + let _ = self.0.recv(); + Ok(bytes.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + let (reader, writer) = std::os::unix::net::UnixStream::pair().unwrap(); + let forwarder = Forwarder::spawn( + reader, + Filter::new([]), + Box::new(Stuck(blocked, Arc::clone(&writes))), + ); + (&writer).write_all(b"one\n").unwrap(); + let started = Instant::now(); + while *writes.lock().unwrap() == 0 { + assert!(started.elapsed() < Duration::from_secs(5)); + std::thread::sleep(Duration::from_millis(5)); + } + (&writer).write_all(b"two\n").unwrap(); + let started = Instant::now(); + drop(forwarder); + let elapsed = started.elapsed(); + assert!( + elapsed < DRAIN_WAIT + CLOSE_WAIT + Duration::from_secs(2), + "shutdown waited {elapsed:?} on a blocked writer" + ); + // The blocked write completes later; no further line is written after close. + release.send(()).unwrap(); + std::thread::sleep(Duration::from_millis(100)); + assert_eq!(*writes.lock().unwrap(), 1); + } + #[test] fn a_held_pipe_does_not_outlive_the_forwarder() { let (reader, writer) = std::os::unix::net::UnixStream::pair().unwrap(); diff --git a/packages/runtime-core/src/provider/image_build/tests.rs b/packages/runtime-core/src/provider/image_build/tests.rs index 7dde84ed0..c980c371d 100644 --- a/packages/runtime-core/src/provider/image_build/tests.rs +++ b/packages/runtime-core/src/provider/image_build/tests.rs @@ -131,6 +131,8 @@ struct FakePool { images: RefCell>, leases: RefCell, resolve_error: bool, + resolve_delay: Duration, + resolve_deadlines: RefCell>, output: Arc>>, } impl FakePool { @@ -140,6 +142,8 @@ impl FakePool { images: RefCell::new(BTreeMap::new()), leases: RefCell::new(0), resolve_error: false, + resolve_delay: Duration::ZERO, + resolve_deadlines: RefCell::new(Vec::new()), output: Arc::default(), } } @@ -171,7 +175,14 @@ impl BuildPool for FakePool { fn inspect(&self, _: &(), image: &str) -> Result, CandidateError> { Ok(self.images.borrow().get(image).cloned()) } - fn resolve(&self, base: &BaseImage) -> Result { + fn resolve( + &self, + base: &BaseImage, + deadline: Instant, + _: &dyn Fn() -> bool, + ) -> Result { + self.resolve_deadlines.borrow_mut().push(deadline); + std::thread::sleep(self.resolve_delay); if self.resolve_error { return Err(CandidateError::new("registry_image", "offline")); } @@ -325,6 +336,44 @@ fn the_client_builds_the_scanned_dockerfile_and_ignore_rules_not_later_edits() { ); } +#[test] +fn the_time_budget_starts_before_base_resolution() { + let fixture = Fixture::new(); + let mut inputs = fixture.inputs(Policy::Missing); + inputs.timeout = Duration::from_millis(300); + let mut pool = FakePool::new("pool-1"); + pool.resolve_delay = Duration::from_millis(400); + fixture.client(&inputs, "ok"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + let started = Instant::now(); + let error = run(&fixture.candidate, &pool, &inputs, &never).unwrap_err(); + let finished = Instant::now(); + assert_eq!(error.code, "native_build_timeout"); + assert_eq!(fixture.calls(&inputs), 0, "the client ran after the budget"); + // Resolution got the operation's deadline, which had passed when it returned. + let deadlines = pool.resolve_deadlines.borrow(); + assert_eq!(deadlines.len(), 1); + assert!(deadlines[0] >= started + inputs.timeout && deadlines[0] < finished); +} + +#[test] +fn blocking_resolution_yields_to_the_deadline_and_to_cancellation() { + let slow = || { + std::thread::sleep(Duration::from_secs(3)); + Ok(()) + }; + let started = Instant::now(); + let error = interruptible(slow, started + Duration::from_millis(100), &never).unwrap_err(); + assert_eq!(error.code, "native_build_timeout"); + let error = interruptible(slow, started + Duration::from_secs(60), &|| true).unwrap_err(); + assert_eq!(error.code, "native_build_canceled"); + assert!(started.elapsed() < Duration::from_secs(2)); + assert_eq!( + interruptible(|| Ok(7), Instant::now() + Duration::from_secs(5), &never).unwrap(), + 7 + ); +} + #[test] fn verified_image_is_reused_without_running_the_client() { let fixture = Fixture::new(); diff --git a/packages/runtime-core/src/provider/registry_image.rs b/packages/runtime-core/src/provider/registry_image.rs index 735101f2b..36c4a7404 100644 --- a/packages/runtime-core/src/provider/registry_image.rs +++ b/packages/runtime-core/src/provider/registry_image.rs @@ -245,7 +245,10 @@ pub struct AcquiredImage { pub archive_sha256: String, pub archive_bytes: u64, } -fn authenticated_fetch(repo: &str) -> Result { +/// The complete-acquisition bound for one registry resolution or fetch. +const ACQUISITION_BUDGET: Duration = Duration::from_secs(300); + +fn authenticated_fetch(repo: &str, deadline: Instant) -> Result { let mut fetch = Fetch { client: Client::builder() .https_only(true) @@ -255,7 +258,7 @@ fn authenticated_fetch(repo: &str) -> Result { .build() .map_err(|_| error("Cannot initialize registry TLS client."))?, token: Zeroizing::new(String::new()), - deadline: Instant::now() + Duration::from_secs(300), + deadline: deadline.min(Instant::now() + ACQUISITION_BUDGET), }; let mut token_url = Url::parse("https://auth.docker.io/token").map_err(|_| error("Invalid token endpoint."))?; @@ -336,7 +339,8 @@ pub struct ResolvedImage { /// fetch must use pinned_reference: reusing the original tag would discard this review. pub fn resolve(reference_text: &str) -> Result { let (repo, tag) = tagged_reference(reference_text)?; - resolve_manifest(&repo, &tag, None).map(|(resolved, _)| resolved) + resolve_manifest(&repo, &tag, None, Instant::now() + ACQUISITION_BUDGET) + .map(|(resolved, _)| resolved) } /// Resolve one repository at a tag or digest to a Linux ARM64 image and its config bytes. @@ -345,8 +349,9 @@ fn resolve_manifest( repo: &str, reference: &str, expected_source: Option<&str>, + deadline: Instant, ) -> Result<(ResolvedImage, Vec), CandidateError> { - let fetch = authenticated_fetch(repo)?; + let fetch = authenticated_fetch(repo, deadline)?; let url = Url::parse(&format!( "https://registry-1.docker.io/v2/{repo}/manifests/{reference}" )) @@ -401,7 +406,11 @@ pub struct BuildBase { /// Either way the image must offer exactly one Linux ARM64 image, and its configuration /// must declare no `ONBUILD` triggers: a stored trigger could pull images the Dockerfile /// scan never saw. No layers, credentials or runtime state are touched. -pub fn resolve_build_base(named: &NamedReference) -> Result { +/// `deadline` bounds the whole resolution (never beyond the usual acquisition budget). +pub fn resolve_build_base( + named: &NamedReference, + deadline: Instant, +) -> Result { let repo = named.docker_hub_repository().ok_or_else(|| { error("Only public Docker Hub [namespace/]repository[:tag][@sha256] references are supported.") })?; @@ -410,7 +419,7 @@ pub fn resolve_build_base(named: &NamedReference) -> Result (tag.as_str(), None), (None, None) => ("latest", None), }; - let (resolved, config) = resolve_manifest(repo, reference, expected)?; + let (resolved, config) = resolve_manifest(repo, reference, expected, deadline)?; if declares_onbuild(&config)? { return Err(CandidateError::new( "native_build_base_image", @@ -458,7 +467,7 @@ pub fn acquire(reference_text: &str, output: &Path) -> Result named, ImageReference::ImageId(_) => unreachable!(), }; - let error = resolve_build_base(&named("ghcr.io/example/app:1")).unwrap_err(); + let error = + resolve_build_base(&named("ghcr.io/example/app:1"), Instant::now()).unwrap_err(); assert_eq!(error.code, "registry_image"); } From f26acd9d2ed7a8686a426ca26b9bff1c4744d3e8 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:39:21 -0500 Subject: [PATCH 06/11] fix(native): pass build arguments in a private bake definition and widen redaction The build client now runs buildx bake with a private mode-0600 definition file that carries the argument values, the context, the pinned base contexts, labels and tag. It is removed after the run, so values never enter the client's command line or environment, which stays a fixed allowlist. Every string in the definition is escaped so bake never interpolates project text. Argument names must be plain identifiers; only names BuildKit or buildx interpret themselves (BUILDKIT_*, BUILDX_*, SOURCE_DATE_EPOCH) refuse. The output filter also redacts each value's Go-quoted, JSON-escaped and bake encoded forms. On a cut line it trims a trailing partial value from the sanitized bytes that are forwarded, not the raw ones. Co-Authored-By: Claude Opus 5.5 --- docs/guides/native-candidate.md | 16 +- .../runtime-core/src/provider/image_build.rs | 142 +++++++++---- .../src/provider/image_build/output.rs | 167 +++++++++++++--- .../src/provider/image_build/tests.rs | 187 ++++++++++++++---- 4 files changed, 399 insertions(+), 113 deletions(-) diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index 9fdad5276..744a6994a 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -474,14 +474,16 @@ linked, mode-`0600` file owned by you), because build-argument values are in it: Base images with stored `ONBUILD` triggers refuse. The client builds from a private copy of the Dockerfile and ignore rules that were scanned, so editing either file during the build has no effect on it. -- Argument values reach the client only through its environment, never its command - line or receipts. Names that would change the client's own environment - (`PATH`, `HOME`, `DOCKER_*`, `BUILDX_*`, `BUILDKIT_*`, proxies, locale, Git and SSH - settings) refuse. +- Argument values reach the client only through a private definition file (mode + `0600`, removed after the build), never its command line, its environment or + receipts; the client's environment is a fixed list of its own settings. Names must + be plain identifiers. Names BuildKit or buildx interpret themselves (`BUILDKIT_*`, + `BUILDX_*`, `SOURCE_DATE_EPOCH`) and the platform arguments refuse. - Build output is shown on stderr after filtering: every argument value is replaced - with `[redacted]`, terminal control characters are removed, lines are cut at 16 KiB - and output stops after 4 MiB. A value that a Dockerfile transforms before printing - it (for example, by encoding it) is not detected. + with `[redacted]`, also when quoted or escaped the way the client prints values, + terminal control characters are removed, lines are cut at 16 KiB and output stops + after 4 MiB. A value that a Dockerfile transforms before printing it (for example, + by encoding it) is not detected. - `missing` reuses a verified image for the same build key without running the client; `rebuild` always builds. The key covers the Dockerfile and `.dockerignore` bytes, target, arguments and versions, not the context's other files (as with Compose). diff --git a/packages/runtime-core/src/provider/image_build.rs b/packages/runtime-core/src/provider/image_build.rs index ddf3599ed..ad006d5a7 100644 --- a/packages/runtime-core/src/provider/image_build.rs +++ b/packages/runtime-core/src/provider/image_build.rs @@ -27,10 +27,12 @@ //! is released only when its holder exits): it is recorded `indeterminate` and the //! image is built again. Build outputs are content addressed and adoption always //! rechecks labels and incarnation, so no operator action is needed. -//! - Argument values never appear in argv, receipts or messages: the client receives -//! `--build-arg NAME` and reads the value from its private environment. Its output -//! reaches the caller only through [`output::Filter`], which redacts every value and -//! strips terminal control sequences, within fixed bounds. +//! - Argument values never appear in argv, the client's environment, receipts or +//! messages: the client runs `buildx bake` with a private mode-0600 definition file +//! that carries them, removed after the run, and its environment is cleared to a +//! fixed allowlist. Its output reaches the caller only through [`output::Filter`], +//! which redacts every value and strips terminal control sequences, within fixed +//! bounds. //! - The client builds from a private copy of the Dockerfile and ignore bytes that were //! scanned and hashed, never from the project paths again. //! - Nothing here removes images, prunes caches or adopts an image whose labels differ. @@ -273,18 +275,15 @@ pub(super) fn namespace(root: &Path, branch: Option<&str>) -> String { format!("{:x}", digest.finalize()) } -/// Client environment the build must not let an argument name override, plus names -/// the client itself interprets (proxies would redirect the client's own traffic). +/// Build-argument names BuildKit or buildx interpret themselves rather than passing to +/// the Dockerfile: `BUILDKIT_*` (for example `BUILDKIT_SYNTAX` selects a frontend +/// image), `BUILDX_*` and `SOURCE_DATE_EPOCH`. Any case refuses. Other names, including +/// ones a process environment would interpret (`PATH`, `LD_*`, `GO*`, proxies), are +/// ordinary Dockerfile arguments here: values travel only in the private bake +/// definition and never enter the client's environment. fn reserved_argument(name: &str) -> bool { - const EXACT: [&str; 6] = ["PATH", "HOME", "LANG", "TMPDIR", "USER", "SHELL"]; - const PREFIXES: [&str; 3] = ["DOCKER_", "BUILDX_", "BUILDKIT_"]; let upper = name.to_ascii_uppercase(); - EXACT.contains(&upper.as_str()) - || PREFIXES.iter().any(|prefix| upper.starts_with(prefix)) - || upper.ends_with("_PROXY") - || upper.starts_with("LC_") - || upper.starts_with("GIT_") - || upper.starts_with("SSH_") + upper.starts_with("BUILDKIT_") || upper.starts_with("BUILDX_") || upper == "SOURCE_DATE_EPOCH" } fn valid_argument_name(name: &str) -> bool { @@ -351,7 +350,7 @@ fn validate_request(request: &BuildRequest) -> Result<(), CandidateError> { if reserved_argument(name) { return Err(refusal( "native_build_argument", - "Build argument names may not set the build client's own environment (PATH, HOME, DOCKER_*, BUILDX_*, BUILDKIT_*, proxies, locale, Git or SSH settings).", + "Build argument names may not be ones BuildKit or buildx interpret themselves (BUILDKIT_*, BUILDX_*, SOURCE_DATE_EPOCH).", )); } if value.bytes().any(|b| b == 0) { @@ -806,6 +805,8 @@ pub(super) fn run( Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} Err(error) => return Err(state::io(error)), } + // The definition carries argument values; it never outlives the client run. + let _definition = RemoveOnDrop(scratch.join("definition").join(BAKE_DEFINITION)); let ended = supervise( command( &builder, &scratch, &metadata, inputs, &identity, &pins, &expected, @@ -912,7 +913,7 @@ fn built_image_id(metadata: &Path) -> Result { "The build client wrote malformed image metadata.", ) })?; - let id = value["containerimage.config.digest"] + let id = value[BAKE_TARGET]["containerimage.config.digest"] .as_str() .filter(|id| image_id(id)) .ok_or_else(|| { @@ -955,8 +956,70 @@ fn write_private(path: &Path, bytes: &[u8]) -> Result<(), CandidateError> { file.write_all(bytes).map_err(state::io) } -/// The client command: argv carries only paths, names and non-secret labels; argument -/// values reach the client through its environment. +/// The one bake target a build defines; its metadata is keyed by this name. +const BAKE_TARGET: &str = "build"; +const BAKE_DEFINITION: &str = "definition.json"; + +/// Removes one private file when dropped, on every exit path. +struct RemoveOnDrop(PathBuf); +impl Drop for RemoveOnDrop { + fn drop(&mut self) { + let _ = fs::remove_file(&self.0); + } +} + +/// Escape a string for the bake definition. Bake reads JSON definitions as HCL, where +/// `${...}` interpolates (variables, the environment, functions) and `%{...}` is a +/// template directive; `$${` and `%%{` are their literal forms. Every string in the +/// definition passes through here, so no project-supplied text is ever evaluated. +fn bake_literal(text: &str) -> String { + text.replace("${", "$${").replace("%{", "%%{") +} + +fn bake_strings(value: Value) -> Value { + match value { + Value::String(text) => Value::String(bake_literal(&text)), + Value::Array(items) => Value::Array(items.into_iter().map(bake_strings).collect()), + Value::Object(map) => Value::Object( + map.into_iter() + .map(|(key, value)| (bake_literal(&key), bake_strings(value))) + .collect(), + ), + other => other, + } +} + +/// The private bake definition: everything the build needs, argument values +/// included. It is written mode 0600 in the client's private scratch directory. +fn bake_definition( + inputs: &Inputs, + dockerfile: &Path, + pins: &BTreeMap, + labels: &BTreeMap<&'static str, String>, +) -> Value { + let contexts: BTreeMap<&str, String> = pins + .iter() + .map(|(name, reference)| (name.as_str(), format!("docker-image://{reference}"))) + .collect(); + let mut target = serde_json::json!({ + "context": inputs.context.to_string_lossy(), + "dockerfile": dockerfile.to_string_lossy(), + "args": inputs.args, + "contexts": contexts, + "labels": labels, + "tags": [inputs.tag()], + "platforms": ["linux/arm64"], + "output": ["type=docker"], + }); + if let Some(stage) = &inputs.target { + target["target"] = Value::String(stage.clone()); + } + bake_strings(serde_json::json!({"target": {BAKE_TARGET: target}})) +} + +/// The client command: `buildx bake` with the private definition. Argv carries only +/// paths and fixed flags, and the environment is cleared to a fixed allowlist: argument +/// values travel only in the definition, never in argv or the client's environment. fn command( builder: &Path, scratch: &Path, @@ -968,9 +1031,15 @@ fn command( ) -> Result { let config = scratch.join("docker-config"); let buildx = scratch.join("buildx"); + let bake = scratch.join("definition"); state::private_directory(&config)?; state::private_directory(&buildx)?; + state::private_directory(&bake)?; let dockerfile = stage_dockerfile(scratch, inputs)?; + let definition = bake.join(BAKE_DEFINITION); + let encoded = serde_json::to_vec(&bake_definition(inputs, &dockerfile, pins, labels)) + .map_err(|_| invalid("Cannot encode the build definition."))?; + write_private(&definition, &encoded)?; let mut command = super::process::clean_command(builder); command .current_dir(scratch) @@ -986,29 +1055,22 @@ fn command( .env("BUILDX_CONFIG", &buildx) .env("BUILDX_NO_DEFAULT_ATTESTATIONS", "1") .env("DOCKER_CLI_HINTS", "false") - .args(["build", "--progress=plain", "--platform", "linux/arm64"]) - .args(["--provenance=false", "--sbom=false", "--load"]) + .args([ + "bake", + "--progress=plain", + "--provenance=false", + "--sbom=false", + ]) .arg("--file") - .arg(&dockerfile) + .arg(&definition) .arg("--metadata-file") .arg(metadata) - .args(["--tag", &inputs.tag()]); - if let Some(target) = &inputs.target { - command.args(["--target", target]); - } - for (name, value) in &inputs.args { - command.args(["--build-arg", name]).env(name, value); - } - for (name, reference) in pins { - command - .arg("--build-context") - .arg(format!("{name}=docker-image://{reference}")); - } - for (name, value) in labels { - command.arg("--label").arg(format!("{name}={value}")); - } - command - .arg(&inputs.context) + // Bake requires explicit read access outside its working directory. + .arg("--allow") + .arg(format!("fs.read={}", inputs.context.display())) + .arg("--allow") + .arg(format!("fs.read={}", bake_parent(&dockerfile).display())) + .arg(BAKE_TARGET) .stdin(Stdio::null()) .stdout(Stdio::null()) // Output reaches the caller only through `output::Filter`. @@ -1017,6 +1079,10 @@ fn command( Ok(command) } +fn bake_parent(path: &Path) -> &Path { + path.parent().unwrap_or(path) +} + #[derive(Debug, Eq, PartialEq)] enum Ended { Exited(bool), diff --git a/packages/runtime-core/src/provider/image_build/output.rs b/packages/runtime-core/src/provider/image_build/output.rs index b3e6c9720..5e417e683 100644 --- a/packages/runtime-core/src/provider/image_build/output.rs +++ b/packages/runtime-core/src/provider/image_build/output.rs @@ -3,9 +3,12 @@ //! The client's stderr is a pipe read here, never inherited. Each line is redacted of //! every build-argument value (the request treats them all as secrets), stripped of //! terminal control characters and escape sequences, cut at a fixed length, and counted -//! against a fixed budget before it is written to the caller's stream. Redaction is -//! literal: a Dockerfile that transforms a value before printing it (encoding, slicing) -//! is not detected. +//! against a fixed budget before it is written to the caller's stream. Each value is +//! matched raw and in the forms the client prints it in: Go `%q` quoting, JSON string +//! escaping (with and without Go's HTML escaping), and the bake definition's own +//! encoding, which the client quotes in its parse errors. Redaction is otherwise literal: +//! a Dockerfile that transforms a value before printing it (encoding, slicing) is not +//! detected. use std::{ io::{Read, Write}, sync::{ @@ -49,6 +52,88 @@ fn sanitize(bytes: &[u8]) -> Vec { .into_bytes() } +/// The body of Go's `strconv.Quote` (without the surrounding quotes). Printability +/// follows Rust's control and whitespace classes, a close approximation of Go's +/// `unicode.IsPrint`. +fn go_quoted(value: &str) -> String { + let mut out = String::new(); + for c in value.chars() { + match c { + '"' => out.push_str("\\\""), + '\\' => out.push_str("\\\\"), + '\u{7}' => out.push_str("\\a"), + '\u{8}' => out.push_str("\\b"), + '\u{c}' => out.push_str("\\f"), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + '\u{b}' => out.push_str("\\v"), + c if (c as u32) < 0x80 && c.is_control() => { + out.push_str(&format!("\\x{:02x}", c as u32)); + } + c if c.is_control() || (c.is_whitespace() && c != ' ') => { + if (c as u32) < 0x10000 { + out.push_str(&format!("\\u{:04x}", c as u32)); + } else { + out.push_str(&format!("\\U{:08x}", c as u32)); + } + } + c => out.push(c), + } + } + out +} + +/// The body of a JSON string. `html` adds Go `encoding/json`'s default escaping of +/// `<`, `>`, `&`, U+2028 and U+2029. +fn json_escaped(value: &str, html: bool) -> String { + let mut out = String::new(); + for c in value.chars() { + match c { + '"' => out.push_str("\\\""), + '\\' => out.push_str("\\\\"), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + '\u{8}' => out.push_str("\\b"), + '\u{c}' => out.push_str("\\f"), + c if (c as u32) < 0x20 => out.push_str(&format!("\\u{:04x}", c as u32)), + '<' | '>' | '&' | '\u{2028}' | '\u{2029}' if html => { + out.push_str(&format!("\\u{:04x}", c as u32)); + } + c => out.push(c), + } + } + out +} + +/// Every form of one value the filter redacts. +fn value_forms(value: &str) -> Vec { + let mut forms = vec![ + value.to_owned(), + go_quoted(value), + json_escaped(value, false), + json_escaped(value, true), + json_escaped(&super::bake_literal(value), false), + ]; + // A raw value spanning lines is also caught line by line. + forms.extend(value.split(['\n', '\r']).map(str::to_owned)); + forms +} + +/// The length of the longest suffix of `bytes` that is a proper prefix of a needle. +fn partial_tail(bytes: &[u8], needles: &[Vec]) -> usize { + needles + .iter() + .filter_map(|needle| { + (1..needle.len().min(bytes.len() + 1)) + .rev() + .find(|k| bytes.ends_with(&needle[..*k])) + }) + .max() + .unwrap_or(0) +} + fn redact(bytes: &[u8], needles: &[Vec]) -> Vec { let mut out = Vec::with_capacity(bytes.len()); let mut at = 0; @@ -67,15 +152,15 @@ fn redact(bytes: &[u8], needles: &[Vec]) -> Vec { } impl Filter { - /// Redact each value and each of its lines, raw and as sanitizing would leave them, - /// so a value spanning lines or containing control characters is still caught. + /// Redact every form of each value (see [`value_forms`]), raw and as sanitizing + /// would leave it, so a value containing quotes, escapes, control characters or line + /// breaks is still caught. pub(super) fn new<'a>(values: impl IntoIterator) -> Self { let mut needles = Vec::new(); for value in values { - let parts = std::iter::once(value).chain(value.split(['\n', '\r'])); - for part in parts { - needles.push(part.as_bytes().to_vec()); - needles.push(sanitize(part.as_bytes())); + for form in value_forms(value) { + needles.push(sanitize(form.as_bytes())); + needles.push(form.into_bytes()); } } needles.retain(|needle| !needle.is_empty()); @@ -88,30 +173,24 @@ impl Filter { } } - /// The bytes to forward for one line (without its terminator). `cut` marks a line - /// longer than the line bound whose remainder is dropped: a trailing partial match - /// of any value is removed too, so no prefix of a value survives the cut. + /// The bytes to forward for one line (without its terminator). Values are redacted + /// in the raw bytes (catching values that contain control characters), the result + /// is sanitized, and redacted again in the sanitized bytes that are forwarded. `cut` + /// marks a line longer than the line bound whose remainder is dropped: a trailing + /// partial match of any value is then trimmed from the sanitized bytes, the ones + /// actually forwarded, so no prefix of a value survives the cut. pub(super) fn line(&mut self, line: &[u8], cut: bool) -> Vec { if self.exhausted { return Vec::new(); } - let mut raw = line; + let mut clean = sanitize(&redact(line, &self.needles)); let mut partial = false; if cut { - let tail = self - .needles - .iter() - .filter_map(|needle| { - (1..needle.len().min(raw.len() + 1)) - .rev() - .find(|k| raw.ends_with(&needle[..*k])) - }) - .max() - .unwrap_or(0); - raw = &raw[..raw.len() - tail]; + let tail = partial_tail(&clean, &self.needles); + clean.truncate(clean.len() - tail); partial = tail > 0; } - let mut out = redact(&sanitize(&redact(raw, &self.needles)), &self.needles); + let mut out = redact(&clean, &self.needles); if partial { out.extend_from_slice(REDACTED.as_bytes()); } @@ -251,6 +330,44 @@ mod tests { assert!(!out.contains('\x1b')); } + #[test] + fn quoted_and_escaped_forms_of_values_are_redacted() { + let value = "p\"a\\s&\nw\u{1b}d"; + let forms = [ + // Go %q, as buildx prints values in errors. + "p\\\"a\\\\s&\\nw\\x1bd", + // JSON, and Go's HTML-escaped JSON. + "p\\\"a\\\\s&\\nw\\u001bd", + "p\\\"a\\\\s\\u003cs\\u003e\\u0026\\nw\\u001bd", + ]; + for form in forms { + let out = filtered(&[value], format!("err: \"{form}\" done\n").as_bytes()); + assert_eq!(out, format!("err: \"{REDACTED}\" done\n"), "{form}"); + } + // The bake definition encodes `${`, and the client quotes it in parse errors. + let out = filtered(&["a${b}c"], b"1 | {\"args\":{\"X\":\"a$${b}c\"}}\n"); + assert_eq!(out, format!("1 | {{\"args\":{{\"X\":\"{REDACTED}\"}}}}\n")); + assert_eq!( + go_quoted("\u{2028}\u{1f600}\u{7f}"), + "\\u2028\u{1f600}\\x7f" + ); + } + + #[test] + fn a_value_split_by_a_cut_after_sanitizing_leaves_no_prefix() { + // Raw bytes end in ESC, so only the sanitized (forwarded) bytes end in a prefix. + let mut input = vec![b'x'; MAX_LINE_BYTES - 4]; + input.extend_from_slice(b"abc\x1bdefgh tail\n"); + let out = filtered(&["abcdefgh"], &input); + let first = out.lines().next().unwrap(); + assert!( + first.ends_with(&format!("x{REDACTED}{LINE_CUT}")), + "{:?}", + &first[first.len() - 40..] + ); + assert!(!out.contains("abc")); + } + #[test] fn terminal_control_sequences_are_stripped_and_tabs_kept() { let out = filtered( diff --git a/packages/runtime-core/src/provider/image_build/tests.rs b/packages/runtime-core/src/provider/image_build/tests.rs index c980c371d..1c87732ab 100644 --- a/packages/runtime-core/src/provider/image_build/tests.rs +++ b/packages/runtime-core/src/provider/image_build/tests.rs @@ -1,8 +1,9 @@ //! `runtime build-image` against a fake pool and a fake build client. //! -//! The fake client is `/bin/sh` reading a script named `build` from the client's private -//! working directory, so no test writes and then executes a fresh binary. It records its -//! argv, the one build-argument variable it was given, and a call count. +//! The fake client is `/bin/sh` reading a script named `bake` (its first argument) from +//! the client's private working directory, so no test writes and then executes a fresh +//! binary. It records its argv, its whole environment, a copy of the bake definition, +//! the staged Dockerfile and ignore rules, and a call count. use super::*; use std::{ cell::RefCell, @@ -79,7 +80,7 @@ impl Fixture { count=$(cat calls 2>/dev/null || echo 0) echo $((count + 1)) > calls printf '%s\n' "$@" > argv -printf '%s' "${{GREETING-}}" > greeting +env > env meta= file= previous= @@ -88,21 +89,22 @@ for argument do if [ "$previous" = --file ]; then file=$argument; fi previous=$argument done -cat "$file" > seen-dockerfile -cat "$file.dockerignore" > seen-dockerignore +cat "$file" > seen-bake.json +cat dockerfile/Dockerfile > seen-dockerfile +cat dockerfile/Dockerfile.dockerignore > seen-dockerignore case {mode} in - ok) printf '{{"containerimage.config.digest":"{IMAGE}"}}' > "$meta" ;; + ok) printf '{{"build":{{"containerimage.config.digest":"{IMAGE}"}}}}' > "$meta" ;; leak) - printf '#5 0.1 value=%s\n\033[31mred\033[0m\033]0;title\007\n' "$GREETING" >&2 - printf '{{"containerimage.config.digest":"{IMAGE}"}}' > "$meta" ;; + printf '#5 0.1 value=%s\n\033[31mred\033[0m\033]0;title\007\n' '{SECRET}' >&2 + printf '{{"build":{{"containerimage.config.digest":"{IMAGE}"}}}}' > "$meta" ;; fail) exit 3 ;; nometa) ;; sleep) sleep 30 & echo $! > child.pending; mv child.pending child; wait ;; esac "# ); - fs::write(scratch.join("build"), script).unwrap(); - fs::set_permissions(scratch.join("build"), fs::Permissions::from_mode(0o600)).unwrap(); + fs::write(scratch.join("bake"), script).unwrap(); + fs::set_permissions(scratch.join("bake"), fs::Permissions::from_mode(0o600)).unwrap(); } fn calls(&self, inputs: &Inputs) -> u32 { fs::read_to_string(self.scratch(inputs).join("calls")) @@ -217,7 +219,7 @@ fn never() -> bool { } #[test] -fn cold_build_passes_values_only_through_the_environment_and_records_the_result() { +fn cold_build_passes_values_only_through_the_private_definition_and_records_the_result() { let fixture = Fixture::new(); let inputs = fixture.inputs(Policy::Missing); let pool = FakePool::new("pool-1"); @@ -237,27 +239,37 @@ fn cold_build_passes_values_only_through_the_environment_and_records_the_result( let argv = fs::read_to_string(scratch.join("argv")).unwrap(); assert!(!argv.contains(SECRET), "argument value reached argv"); let lines: Vec<&str> = argv.lines().collect(); - assert!(lines.windows(2).any(|w| w == ["--build-arg", "GREETING"])); - assert!(lines.windows(2).any(|w| w - == [ - "--build-context", - &format!("alpine:3.20=docker-image://docker.io/library/alpine@{PIN}") - ])); - assert!(lines.windows(2).any(|w| w == ["--platform", "linux/arm64"])); - assert!(lines.contains(&"--provenance=false")); - assert!( - lines - .windows(2) - .any(|w| w == ["--label", "io.hack-local.kind=build-image"]) - ); + // `sh` ran the script named by the first argument, so that argument was `bake`. + assert_eq!(lines.last(), Some(&"build")); + assert!(lines.contains(&"--provenance=false") && lines.contains(&"--sbom=false")); + assert!(lines.windows(2).any(|w| w[0] == "--allow" + && w[1] == format!("fs.read={}", fixture.project.join("app").display()))); assert!( !argv.contains("pool-1"), "the pool incarnation reached argv" ); + let env = fs::read_to_string(scratch.join("env")).unwrap(); + assert!( + !env.contains(SECRET), + "argument value reached the client environment" + ); + assert!(!env.lines().any(|line| line.starts_with("GREETING="))); + let bake: Value = + serde_json::from_slice(&fs::read(scratch.join("seen-bake.json")).unwrap()).unwrap(); + let target = &bake["target"]["build"]; + assert_eq!(target["args"], serde_json::json!({"GREETING": SECRET})); assert_eq!( - fs::read_to_string(scratch.join("greeting")).unwrap(), - SECRET + target["contexts"], + serde_json::json!({ + "alpine:3.20": format!("docker-image://docker.io/library/alpine@{PIN}") + }) ); + assert_eq!(target["platforms"], serde_json::json!(["linux/arm64"])); + assert_eq!(target["output"], serde_json::json!(["type=docker"])); + assert_eq!(target["labels"]["io.hack-local.kind"], "build-image"); + assert_eq!(target["tags"], serde_json::json!([inputs.tag()])); + // The definition carries argument values; it is gone once the client has run. + assert!(!scratch.join("definition").join(BAKE_DEFINITION).exists()); let receipt = fixture.receipt(&inputs); assert_eq!(receipt.phase, Phase::ContentIdVerified); assert_eq!(receipt.image_id.as_deref(), Some(IMAGE)); @@ -270,6 +282,104 @@ fn cold_build_passes_values_only_through_the_environment_and_records_the_result( assert!(!text.contains(SECRET), "argument value reached the receipt"); } +#[test] +fn environment_names_are_plain_arguments_that_never_reach_the_client_environment() { + let fixture = Fixture::new(); + let names = [ + "DYLD_INSERT_LIBRARIES", + "LD_PRELOAD", + "GODEBUG", + "PATH", + "HTTPS_PROXY", + "KUBECONFIG", + "DOCKER_HOST", + ]; + let mut request = fixture.request(Policy::Missing); + for name in names { + request + .args + .insert(name.to_owned(), format!("{SECRET}-{name}")); + } + let inputs = prepare(request).unwrap(); + let pool = FakePool::new("pool-1"); + fixture.client(&inputs, "ok"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + run(&fixture.candidate, &pool, &inputs, &never).unwrap(); + let scratch = fixture.scratch(&inputs); + let env = fs::read_to_string(scratch.join("env")).unwrap(); + assert!( + !env.contains(SECRET), + "an argument value reached the client environment" + ); + // The client environment is exactly the fixed allowlist (plus what `sh` adds itself). + let allowed = [ + "PATH", + "LANG", + "HOME", + "TMPDIR", + "DOCKER_HOST", + "DOCKER_CONFIG", + "BUILDX_CONFIG", + "BUILDX_NO_DEFAULT_ATTESTATIONS", + "DOCKER_CLI_HINTS", + "PWD", + "OLDPWD", + "SHLVL", + "_", + ]; + for line in env.lines() { + let name = line.split('=').next().unwrap(); + assert!(allowed.contains(&name), "unexpected client variable {name}"); + } + assert!( + env.lines() + .any(|line| line == "PATH=/usr/bin:/bin:/usr/sbin:/sbin") + ); + assert!( + !env.lines() + .any(|line| line.starts_with("DYLD_") || line.starts_with("GODEBUG=")) + ); + let bake: Value = + serde_json::from_slice(&fs::read(scratch.join("seen-bake.json")).unwrap()).unwrap(); + for name in names { + assert_eq!( + bake["target"]["build"]["args"][name], + format!("{SECRET}-{name}") + ); + } +} + +#[test] +fn bake_definitions_never_evaluate_project_text() { + assert_eq!(bake_literal("${HOME}"), "$${HOME}"); + assert_eq!( + bake_literal("%{ if true }x%{ endif }"), + "%%{ if true }x%%{ endif }" + ); + assert_eq!(bake_literal("$${x} a$ % \\${y}"), "$$${x} a$ % \\$${y}"); + let escaped = bake_strings(serde_json::json!({"a${b}": ["${c}", {"d": "%{e}"}], "n": 1})); + assert_eq!( + escaped, + serde_json::json!({"a$${b}": ["$${c}", {"d": "%%{e}"}], "n": 1}) + ); + let fixture = Fixture::new(); + let mut request = fixture.request(Policy::Missing); + request + .args + .insert("TEMPLATE".into(), "${HOME}%{ if true }".into()); + let inputs = prepare(request).unwrap(); + let definition = bake_definition( + &inputs, + Path::new("/s/Dockerfile"), + &BTreeMap::new(), + &BTreeMap::new(), + ); + assert_eq!( + definition["target"]["build"]["args"]["TEMPLATE"], + "$${HOME}%%{ if true }" + ); +} + #[test] fn client_output_reaches_the_caller_redacted_and_without_terminal_controls() { let fixture = Fixture::new(); @@ -304,13 +414,9 @@ fn the_client_builds_the_scanned_dockerfile_and_ignore_rules_not_later_edits() { fs::read_to_string(scratch.join("seen-dockerignore")).unwrap(), "*.txt\n" ); - let argv = fs::read_to_string(scratch.join("argv")).unwrap(); - let lines: Vec<&str> = argv.lines().collect(); - let file = lines - .windows(2) - .find(|w| w[0] == "--file") - .map(|w| PathBuf::from(w[1])) - .unwrap(); + let bake: Value = + serde_json::from_slice(&fs::read(scratch.join("seen-bake.json")).unwrap()).unwrap(); + let file = PathBuf::from(bake["target"]["build"]["dockerfile"].as_str().unwrap()); assert!(file.starts_with(&scratch), "the client read a project path"); // A Dockerfile-specific ignore file wins; with none at all, the private rules are @@ -706,16 +812,11 @@ fn requests_refuse_unsafe_paths_arguments_and_bounds() { assert_eq!(code(request), "native_build_request"); } for name in [ - "PATH", - "home", - "DOCKER_HOST", "BUILDX_CONFIG", "BUILDKIT_SYNTAX", - "HTTPS_PROXY", - "no_proxy", - "LC_ALL", - "GIT_DIR", - "SSH_AUTH_SOCK", + "buildkit_inline_cache", + "SOURCE_DATE_EPOCH", + "TARGETPLATFORM", "1BAD", "bad-name", "", @@ -737,7 +838,7 @@ fn requests_refuse_unsafe_paths_arguments_and_bounds() { assert_eq!(code(request), "native_build_argument"); // Refusals never echo argument values. let mut request = base(); - request.args = BTreeMap::from([("DOCKER_HOST".to_owned(), SECRET.to_owned())]); + request.args = BTreeMap::from([("BUILDKIT_SYNTAX".to_owned(), SECRET.to_owned())]); assert!(!prepare(request).unwrap_err().message.contains(SECRET)); } From 0dff25c911efac6d9bd056502c585a1f1a9e51ee Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:47:08 -0500 Subject: [PATCH 07/11] docs(native): state that build arguments are not secrets Build arguments follow Docker: an image records their values in its history, and resolved base references an argument selects are reported in receipts and results. The guide now says never to pass secrets as build arguments and points to BuildKit build secrets, which native builds do not support yet. Output redaction stays as defense in depth. Co-Authored-By: Claude Opus 5.5 --- docs/guides/native-candidate.md | 22 +++++++++++-------- .../runtime-core/src/provider/image_build.rs | 8 ++++++- .../src/provider/image_build/output.rs | 2 +- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index 744a6994a..897d9e553 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -460,7 +460,7 @@ refuses; it reports `builder_prepared` when the client is present and unchanged. `runtime build-image --request --json` builds one image in the running runtime's engine with that client. It is a building block: native `up` still refuses `build:` services and does not call it. The request is a private JSON file (a singly -linked, mode-`0600` file owned by you), because build-argument values are in it: +linked, mode-`0600` file owned by you): ```json {"version": 1, "project_root": "/absolute/project", "context": "app", @@ -474,16 +474,20 @@ linked, mode-`0600` file owned by you), because build-argument values are in it: Base images with stored `ONBUILD` triggers refuse. The client builds from a private copy of the Dockerfile and ignore rules that were scanned, so editing either file during the build has no effect on it. +- Build arguments are not secrets, exactly as with Docker: the built image records + their values in its history, and an argument that selects a base image appears in + the resolved base references that the receipt and the result report. Never pass a + secret as a build argument. BuildKit build secrets are the right mechanism for + secrets; native builds do not support them yet. - Argument values reach the client only through a private definition file (mode - `0600`, removed after the build), never its command line, its environment or - receipts; the client's environment is a fixed list of its own settings. Names must - be plain identifiers. Names BuildKit or buildx interpret themselves (`BUILDKIT_*`, + `0600`, removed after the build), never its command line or its environment; the + client's environment is a fixed list of its own settings. Names must be plain + identifiers. Names BuildKit or buildx interpret themselves (`BUILDKIT_*`, `BUILDX_*`, `SOURCE_DATE_EPOCH`) and the platform arguments refuse. -- Build output is shown on stderr after filtering: every argument value is replaced - with `[redacted]`, also when quoted or escaped the way the client prints values, - terminal control characters are removed, lines are cut at 16 KiB and output stops - after 4 MiB. A value that a Dockerfile transforms before printing it (for example, - by encoding it) is not detected. +- Build output is shown on stderr after filtering, as defense in depth: every + argument value is replaced with `[redacted]`, also when quoted or escaped the way + the client prints values, terminal control characters are removed, lines are cut at + 16 KiB and output stops after 4 MiB. - `missing` reuses a verified image for the same build key without running the client; `rebuild` always builds. The key covers the Dockerfile and `.dockerignore` bytes, target, arguments and versions, not the context's other files (as with Compose). diff --git a/packages/runtime-core/src/provider/image_build.rs b/packages/runtime-core/src/provider/image_build.rs index ad006d5a7..d1235b15d 100644 --- a/packages/runtime-core/src/provider/image_build.rs +++ b/packages/runtime-core/src/provider/image_build.rs @@ -35,6 +35,11 @@ //! bounds. //! - The client builds from a private copy of the Dockerfile and ignore bytes that were //! scanned and hashed, never from the project paths again. +//! - Build arguments are not secrets, as with Docker: an image records their values in +//! its history, and resolved base references (which an argument may select) are +//! reported in receipts and results. Hack still keeps values out of argv, the +//! client's environment and its own messages, and redacts them from forwarded output +//! as defense in depth. The caller refuses values equal to a managed secret. //! - Nothing here removes images, prunes caches or adopts an image whose labels differ. use super::{ artifact, @@ -136,7 +141,8 @@ pub struct BuildRequest { pub dockerfile: String, #[serde(default)] pub target: Option, - /// Literal build arguments. Values are secrets for every output purpose. + /// Literal build arguments. They are not secrets (an image records them in its + /// history), but Hack keeps them out of argv and its own messages. #[serde(default)] pub args: BTreeMap, pub policy: Policy, diff --git a/packages/runtime-core/src/provider/image_build/output.rs b/packages/runtime-core/src/provider/image_build/output.rs index 5e417e683..ce5d23e1c 100644 --- a/packages/runtime-core/src/provider/image_build/output.rs +++ b/packages/runtime-core/src/provider/image_build/output.rs @@ -1,7 +1,7 @@ //! The build client's output boundary. //! //! The client's stderr is a pipe read here, never inherited. Each line is redacted of -//! every build-argument value (the request treats them all as secrets), stripped of +//! every build-argument value (defense in depth; arguments are not secrets), stripped of //! terminal control characters and escape sequences, cut at a fixed length, and counted //! against a fixed budget before it is written to the caller's stream. Each value is //! matched raw and in the forms the client prints it in: Go `%q` quoting, JSON string From 8c39b609e7ab14eb174a11ad5f7ff9dbeffb6eee Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Thu, 5 Nov 2026 23:55:13 -0500 Subject: [PATCH 08/11] fix(native): run every build under the pinned-base source policy The bake client now receives EXPERIMENTAL_BUILDKIT_SOURCE_POLICY naming a private mode-0600 policy file written beside the definition and removed with it on every path. The policy comes from build_policy::document for exactly the pinned base references the definition names as contexts. It allows those images and denies every other image and network source, including sources a base image's stored ONBUILD triggers add. Co-Authored-By: Claude Opus 5.5 --- docs/guides/native-candidate.md | 5 ++ .../runtime-core/src/provider/image_build.rs | 23 ++++++- .../src/provider/image_build/tests.rs | 67 +++++++++++++++++++ 3 files changed, 93 insertions(+), 2 deletions(-) diff --git a/docs/guides/native-candidate.md b/docs/guides/native-candidate.md index 897d9e553..0d778fd12 100644 --- a/docs/guides/native-candidate.md +++ b/docs/guides/native-candidate.md @@ -479,6 +479,11 @@ linked, mode-`0600` file owned by you): the resolved base references that the receipt and the result report. Never pass a secret as a build argument. BuildKit build secrets are the right mechanism for secrets; native builds do not support them yet. +- Every build runs under a BuildKit source policy: only the pinned base images + resolve as images, and network sources (`ADD` of an `http`, `https` or git URL, and + image blob sources) are denied. The engine would otherwise fetch those from outside + the isolated build network. The policy also covers sources that a base image's + stored `ONBUILD` triggers add; such bases refuse before the build anyway. - Argument values reach the client only through a private definition file (mode `0600`, removed after the build), never its command line or its environment; the client's environment is a fixed list of its own settings. Names must be plain diff --git a/packages/runtime-core/src/provider/image_build.rs b/packages/runtime-core/src/provider/image_build.rs index d1235b15d..770394ab9 100644 --- a/packages/runtime-core/src/provider/image_build.rs +++ b/packages/runtime-core/src/provider/image_build.rs @@ -35,6 +35,10 @@ //! bounds. //! - The client builds from a private copy of the Dockerfile and ignore bytes that were //! scanned and hashed, never from the project paths again. +//! - Every build runs under [`build_policy::document`] for its pins, a BuildKit source +//! policy the client sends with the solve: only the pinned base references resolve as +//! images, and network sources (`ADD` URLs, git, image blobs) are denied, including +//! ones a base image's stored `ONBUILD` triggers add. //! - Build arguments are not secrets, as with Docker: an image records their values in //! its history, and resolved base references (which an argument may select) are //! reported in receipts and results. Hack still keeps values out of argv, the @@ -42,7 +46,7 @@ //! as defense in depth. The caller refuses values equal to a managed secret. //! - Nothing here removes images, prunes caches or adopts an image whose labels differ. use super::{ - artifact, + artifact, build_policy, build_scan::{self, BaseImage}, registry_image::{self, BuildBase}, state, @@ -811,8 +815,10 @@ pub(super) fn run( Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} Err(error) => return Err(state::io(error)), } - // The definition carries argument values; it never outlives the client run. + // Neither the definition (it carries argument values) nor the source policy + // outlives the client run. let _definition = RemoveOnDrop(scratch.join("definition").join(BAKE_DEFINITION)); + let _policy = RemoveOnDrop(scratch.join("definition").join(SOURCE_POLICY)); let ended = supervise( command( &builder, &scratch, &metadata, inputs, &identity, &pins, &expected, @@ -965,6 +971,8 @@ fn write_private(path: &Path, bytes: &[u8]) -> Result<(), CandidateError> { /// The one bake target a build defines; its metadata is keyed by this name. const BAKE_TARGET: &str = "build"; const BAKE_DEFINITION: &str = "definition.json"; +/// The BuildKit source policy file beside the definition (see [`build_policy`]). +const SOURCE_POLICY: &str = "source-policy.json"; /// Removes one private file when dropped, on every exit path. struct RemoveOnDrop(PathBuf); @@ -1046,6 +1054,13 @@ fn command( let encoded = serde_json::to_vec(&bake_definition(inputs, &dockerfile, pins, labels)) .map_err(|_| invalid("Cannot encode the build definition."))?; write_private(&definition, &encoded)?; + // The policy allows exactly the pinned references the definition names as + // contexts, and denies every other image and network source. + let policy = bake.join(SOURCE_POLICY); + let policy_bytes = + serde_json::to_vec(&build_policy::document(pins.values().map(String::as_str))?) + .map_err(|_| invalid("Cannot encode the build source policy."))?; + write_private(&policy, &policy_bytes)?; let mut command = super::process::clean_command(builder); command .current_dir(scratch) @@ -1061,6 +1076,10 @@ fn command( .env("BUILDX_CONFIG", &buildx) .env("BUILDX_NO_DEFAULT_ATTESTATIONS", "1") .env("DOCKER_CLI_HINTS", "false") + // The client sends this policy with the solve, so BuildKit evaluates every + // source of the build against it, including ones a base image's ONBUILD + // triggers add. + .env("EXPERIMENTAL_BUILDKIT_SOURCE_POLICY", &policy) .args([ "bake", "--progress=plain", diff --git a/packages/runtime-core/src/provider/image_build/tests.rs b/packages/runtime-core/src/provider/image_build/tests.rs index 1c87732ab..805f9f764 100644 --- a/packages/runtime-core/src/provider/image_build/tests.rs +++ b/packages/runtime-core/src/provider/image_build/tests.rs @@ -81,6 +81,9 @@ count=$(cat calls 2>/dev/null || echo 0) echo $((count + 1)) > calls printf '%s\n' "$@" > argv env > env +[ -n "${{EXPERIMENTAL_BUILDKIT_SOURCE_POLICY-}}" ] || exit 9 +cat "$EXPERIMENTAL_BUILDKIT_SOURCE_POLICY" > seen-policy.json +printf '%s' "$EXPERIMENTAL_BUILDKIT_SOURCE_POLICY" > seen-policy-path meta= file= previous= @@ -270,6 +273,30 @@ fn cold_build_passes_values_only_through_the_private_definition_and_records_the_ assert_eq!(target["tags"], serde_json::json!([inputs.tag()])); // The definition carries argument values; it is gone once the client has run. assert!(!scratch.join("definition").join(BAKE_DEFINITION).exists()); + // The client ran under the source policy for exactly these pins, from a private + // file removed with the definition. + let policy_path = PathBuf::from(fs::read_to_string(scratch.join("seen-policy-path")).unwrap()); + assert_eq!(policy_path, scratch.join("definition").join(SOURCE_POLICY)); + assert!(!policy_path.exists(), "the source policy outlived the run"); + let policy: Value = + serde_json::from_slice(&fs::read(scratch.join("seen-policy.json")).unwrap()).unwrap(); + let pinned = format!("docker.io/library/alpine@{PIN}"); + assert_eq!(policy, build_policy::document([pinned.as_str()]).unwrap()); + assert_eq!( + policy_decision(&policy, &format!("docker-image://{pinned}")), + "ALLOW" + ); + for denied in [ + "docker-image://docker.io/library/alpine:3.20", + "docker-image://docker.io/library/busybox:latest", + "oci-layout://store@sha256:x", + "https://example.test/archive.tgz", + "git://example.test/repo.git", + "docker-image+blob://docker.io/library/alpine@sha256:x", + ] { + assert_eq!(policy_decision(&policy, denied), "DENY", "{denied}"); + } + assert_eq!(policy_decision(&policy, "local://context"), "ALLOW"); let receipt = fixture.receipt(&inputs); assert_eq!(receipt.phase, Phase::ContentIdVerified); assert_eq!(receipt.image_id.as_deref(), Some(IMAGE)); @@ -322,6 +349,7 @@ fn environment_names_are_plain_arguments_that_never_reach_the_client_environment "BUILDX_CONFIG", "BUILDX_NO_DEFAULT_ATTESTATIONS", "DOCKER_CLI_HINTS", + "EXPERIMENTAL_BUILDKIT_SOURCE_POLICY", "PWD", "OLDPWD", "SHLVL", @@ -349,6 +377,45 @@ fn environment_names_are_plain_arguments_that_never_reach_the_client_environment } } +/// BuildKit's evaluation of a source identifier: the last matching rule wins and an +/// unmatched source is allowed. `WILDCARD` patterns here use only a trailing `*`. +fn policy_decision(policy: &Value, identifier: &str) -> &'static str { + let mut decision = "ALLOW"; + for rule in policy["rules"].as_array().unwrap() { + let pattern = rule["selector"]["identifier"].as_str().unwrap(); + let matched = match rule["selector"]["match_type"].as_str().unwrap() { + "EXACT" => identifier == pattern, + "WILDCARD" => identifier.starts_with(pattern.strip_suffix('*').unwrap()), + other => panic!("unexpected match type {other}"), + }; + if matched { + decision = if rule["action"] == "DENY" { + "DENY" + } else { + "ALLOW" + }; + } + } + decision +} + +#[test] +fn private_build_files_are_removed_even_when_the_client_fails() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + let pool = FakePool::new("pool-1"); + fixture.client(&inputs, "fail"); + let error = run(&fixture.candidate, &pool, &inputs, &never).unwrap_err(); + assert_eq!(error.code, "native_build_failed"); + let scratch = fixture.scratch(&inputs); + assert!( + scratch.join("seen-policy.json").exists(), + "the client did not run" + ); + assert!(!scratch.join("definition").join(BAKE_DEFINITION).exists()); + assert!(!scratch.join("definition").join(SOURCE_POLICY).exists()); +} + #[test] fn bake_definitions_never_evaluate_project_text() { assert_eq!(bake_literal("${HOME}"), "$${HOME}"); From 3b42fdd93c71a4d50875b3326a1a3e7b48e9d72c Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Fri, 6 Nov 2026 00:00:03 -0500 Subject: [PATCH 09/11] fix(native): size the build receipt bound for every accepted base pin The receipt read bound is now 64 KiB, which covers a receipt with the most base pins the scanner accepts at their longest, so a receipt a build writes can always be read again. A new end-to-end test also shows that remote ADD sources (URLs, git refs, variables, ONBUILD forms) refuse at prepare, before any build effect. Co-Authored-By: Claude Opus 5.5 --- .../runtime-core/src/provider/image_build.rs | 4 +- .../src/provider/image_build/tests.rs | 57 +++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/packages/runtime-core/src/provider/image_build.rs b/packages/runtime-core/src/provider/image_build.rs index 770394ab9..8a3b3fdc4 100644 --- a/packages/runtime-core/src/provider/image_build.rs +++ b/packages/runtime-core/src/provider/image_build.rs @@ -84,7 +84,9 @@ const CANCEL_GRACE: Duration = Duration::from_secs(10); const POLL: Duration = Duration::from_millis(50); const LEASE_WAIT: Duration = Duration::from_secs(5); const MAX_REQUEST_BYTES: u64 = 192 * 1024; -const MAX_RECEIPT_BYTES: u64 = 16 * 1024; +/// Covers a receipt with the most base pins the scanner accepts at their longest +/// (about 32 KiB pretty-printed), so a receipt the build writes is always readable. +const MAX_RECEIPT_BYTES: u64 = 64 * 1024; const MAX_METADATA_BYTES: u64 = 1024 * 1024; const MAX_IGNORE_BYTES: u64 = 1024 * 1024; const MAX_ARGS: usize = 64; diff --git a/packages/runtime-core/src/provider/image_build/tests.rs b/packages/runtime-core/src/provider/image_build/tests.rs index 805f9f764..14fa81c90 100644 --- a/packages/runtime-core/src/provider/image_build/tests.rs +++ b/packages/runtime-core/src/provider/image_build/tests.rs @@ -416,6 +416,63 @@ fn private_build_files_are_removed_even_when_the_client_fails() { assert!(!scratch.join("definition").join(SOURCE_POLICY).exists()); } +#[test] +fn receipts_with_the_most_and_longest_accepted_pins_stay_readable() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + let mut receipt = Receipt { + version: 1, + checkout: fixture.candidate.checkout.clone(), + namespace: inputs.namespace.clone(), + build_key: inputs.key.clone(), + provider_incarnation: "i".repeat(128), + provider_boot_id: "b".repeat(128), + engine_id: "e".repeat(128), + phase: Phase::ContentIdVerified, + image_id: Some(IMAGE.into()), + base_pins: BTreeMap::new(), + }; + // The scanner's bounds: 32 images, Docker Hub paths up to 200 bytes, tags up to + // 128, each with a digest; the pinned reference keeps the name and adds the pin. + for index in 0..build_scan::MAX_IMAGE_REFERENCES { + let repository = format!("{index:03}{}/{}", "r".repeat(96), "p".repeat(100)); + let name = format!("{repository}:{}@{PIN}", "t".repeat(128)); + receipt + .base_pins + .insert(name.clone(), format!("docker.io/{name}")); + } + let directory = builds_root(&fixture.candidate).join(&inputs.namespace); + state::private_directory(&directory).unwrap(); + let path = directory.join(format!("{}.json", inputs.key)); + state::write(&path, &receipt).unwrap(); + assert!(fs::metadata(&path).unwrap().len() > 16 * 1024); + let read = read_receipt(&path).unwrap().unwrap(); + assert_eq!(read.base_pins.len(), build_scan::MAX_IMAGE_REFERENCES); +} + +#[test] +fn remote_add_sources_refuse_before_any_build_effect() { + let fixture = Fixture::new(); + for dockerfile in [ + "FROM alpine:3.20\nADD http://100.96.0.1:8080/secret /secret\n", + "FROM alpine:3.20\nADD https://example.test/a.tgz /a\n", + "FROM alpine:3.20\nADD git@example.test:org/repo.git /src\n", + "FROM alpine:3.20\nARG SRC\nADD $SRC /src\n", + "FROM alpine:3.20 AS base\nONBUILD ADD https://example.test/a /a\n", + ] { + fs::write(fixture.project.join("app/Dockerfile"), dockerfile).unwrap(); + let error = prepare(fixture.request(Policy::Missing)).unwrap_err(); + assert_eq!(error.code, "native_build_dockerfile", "{dockerfile}"); + assert!(!error.message.contains("100.96.0.1") && !error.message.contains("example.test")); + } + fs::write( + fixture.project.join("app/Dockerfile"), + "FROM alpine:3.20\nADD main.txt /main.txt\n", + ) + .unwrap(); + assert!(prepare(fixture.request(Policy::Missing)).is_ok()); +} + #[test] fn bake_definitions_never_evaluate_project_text() { assert_eq!(bake_literal("${HOME}"), "$${HOME}"); From 5a40b44f452faf80a865d528d54ace30e3fdc30d Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Fri, 6 Nov 2026 00:03:49 -0500 Subject: [PATCH 10/11] fix(native): discard an interrupted receipt publication under the key lock state::write publishes a receipt through .pending and a rename. A process that died between the two left that file, and every later write for the key failed with AlreadyExists. Under the per-key lock, which proves no writer is live, the build now discards the leftover before writing; the committed receipt, if any, is unaffected. Co-Authored-By: Claude Opus 5.5 --- .../runtime-core/src/provider/image_build.rs | 8 +++++++ .../src/provider/image_build/tests.rs | 23 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/packages/runtime-core/src/provider/image_build.rs b/packages/runtime-core/src/provider/image_build.rs index 8a3b3fdc4..7eec8d06e 100644 --- a/packages/runtime-core/src/provider/image_build.rs +++ b/packages/runtime-core/src/provider/image_build.rs @@ -709,6 +709,14 @@ pub(super) fn run( "Another operation is building this image; wait for it and retry.", ) })?; + // `state::write` publishes through `.pending` and a rename. A process that + // died between the two left that file; the key lock proves no writer is live, so + // the leftover is discarded (the committed receipt, if any, is unaffected). + match fs::remove_file(receipt_path.with_extension("pending")) { + Ok(()) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(state::io(error)), + } let expected = labels(inputs, &identity); let previous = read_receipt(&receipt_path)?; if let Some(previous) = &previous { diff --git a/packages/runtime-core/src/provider/image_build/tests.rs b/packages/runtime-core/src/provider/image_build/tests.rs index 14fa81c90..252663d17 100644 --- a/packages/runtime-core/src/provider/image_build/tests.rs +++ b/packages/runtime-core/src/provider/image_build/tests.rs @@ -473,6 +473,29 @@ fn remote_add_sources_refuse_before_any_build_effect() { assert!(prepare(fixture.request(Policy::Missing)).is_ok()); } +#[test] +fn an_interrupted_receipt_publication_does_not_block_the_key() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + let directory = builds_root(&fixture.candidate).join(&inputs.namespace); + state::private_directory(&directory).unwrap(); + let receipt = directory.join(format!("{}.json", inputs.key)); + // A process died after creating the temporary file and before renaming it, with + // no committed receipt yet. + fs::write(receipt.with_extension("pending"), b"{\"partial").unwrap(); + let pool = FakePool::new("pool-1"); + fixture.client(&inputs, "ok"); + pool.built(&inputs, "pool-1", serde_json::json!({})); + run(&fixture.candidate, &pool, &inputs, &never).unwrap(); + assert_eq!(fixture.receipt(&inputs).phase, Phase::ContentIdVerified); + assert!(!receipt.with_extension("pending").exists()); + // The same with a committed receipt present: a rebuild still proceeds. + fs::write(receipt.with_extension("pending"), b"{").unwrap(); + let rebuild = fixture.inputs(Policy::Rebuild); + run(&fixture.candidate, &pool, &rebuild, &never).unwrap(); + assert_eq!(fixture.calls(&inputs), 2); +} + #[test] fn bake_definitions_never_evaluate_project_text() { assert_eq!(bake_literal("${HOME}"), "$${HOME}"); From 460ddff233bd5ea7f4f94635cecb8bc746620450 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Fri, 6 Nov 2026 00:12:33 -0500 Subject: [PATCH 11/11] fix(native): keep a surviving build client fenced after the CLI is killed The build client now inherits the build fence and the per-key lock. A flock belongs to the open file description, so if hack-native is SIGKILLed while the client runs, both stay held until the client and anything it left running exit. runtime down keeps refusing, and a later attempt of the same key refuses with native_build_busy instead of sharing the surviving client's attempt files. An orderly return still releases both. Co-Authored-By: Claude Opus 5.5 --- .../runtime-core/src/provider/image_build.rs | 42 ++++++- .../src/provider/image_build/tests.rs | 104 ++++++++++++++++++ 2 files changed, 141 insertions(+), 5 deletions(-) diff --git a/packages/runtime-core/src/provider/image_build.rs b/packages/runtime-core/src/provider/image_build.rs index 7eec8d06e..b088e78d8 100644 --- a/packages/runtime-core/src/provider/image_build.rs +++ b/packages/runtime-core/src/provider/image_build.rs @@ -546,6 +546,10 @@ fn builds_root(candidate: &Candidate) -> PathBuf { } /// A cooperative `flock` on a private lock file, shared or exclusive, never blocking. +/// +/// A `flock` belongs to the open file description, so a child that inherits the +/// descriptor holds the lock for as long as it keeps it open if this process is +/// killed before it drops the lock (see [`inherit_locks`]). struct Flock(File); impl Flock { fn open(path: &Path) -> Result { @@ -584,6 +588,10 @@ impl Flock { } impl Drop for Flock { fn drop(&mut self) { + // An orderly drop releases the lock for every holder of the description, so a + // transient copy in another thread's forked child cannot keep it. Only a + // process that dies without dropping (SIGKILL) leaves the lock with the + // descriptors its build client inherited. // SAFETY: the descriptor is live until this value drops. unsafe { libc::flock(self.0.as_raw_fd(), libc::LOCK_UN); @@ -691,7 +699,7 @@ pub(super) fn run( let receipt_path = directory.join(format!("{}.json", inputs.key)); // Step 1, under the lease: fence, key lock and the reuse decision. let (lease, identity) = pool.lease()?; - let _fence = Flock::try_lock(Flock::open(&root.join("active.lock"))?, libc::LOCK_SH)? + let fence = Flock::try_lock(Flock::open(&root.join("active.lock"))?, libc::LOCK_SH)? .ok_or_else(|| { refusal( "provider_busy", @@ -699,7 +707,7 @@ pub(super) fn run( ) })?; state::private_directory(&directory)?; - let _key_lock = Flock::try_lock( + let key_lock = Flock::try_lock( Flock::open(&directory.join(format!("{}.lock", inputs.key)))?, libc::LOCK_EX, )? @@ -829,10 +837,12 @@ pub(super) fn run( // outlives the client run. let _definition = RemoveOnDrop(scratch.join("definition").join(BAKE_DEFINITION)); let _policy = RemoveOnDrop(scratch.join("definition").join(SOURCE_POLICY)); + let mut client = command( + &builder, &scratch, &metadata, inputs, &identity, &pins, &expected, + )?; + inherit_locks(&mut client, &[&fence, &key_lock]); let ended = supervise( - command( - &builder, &scratch, &metadata, inputs, &identity, &pins, &expected, - )?, + client, deadline, canceled, output::Filter::new(inputs.args.values().map(String::as_str)), @@ -1165,6 +1175,28 @@ impl Drop for CancelSignals { } } +/// Let the client inherit the build fence and the key lock. If this process is killed +/// while the client runs, both stay held until the client and everything it leaves +/// running exit: `runtime down` keeps refusing, and a later attempt of the same key +/// refuses with `native_build_busy` instead of sharing the surviving client's attempt +/// files. An orderly return (after the client was reaped or stopped) releases them. +fn inherit_locks(command: &mut std::process::Command, locks: &[&Flock]) { + let descriptors: Vec = locks.iter().map(|lock| lock.0.as_raw_fd()).collect(); + // SAFETY: runs in the forked child before exec; fcntl is async-signal-safe and the + // descriptors are live (their owners outlive the spawn). + unsafe { + command.pre_exec(move || { + for &fd in &descriptors { + let flags = libc::fcntl(fd, libc::F_GETFD); + if flags < 0 || libc::fcntl(fd, libc::F_SETFD, flags & !libc::FD_CLOEXEC) < 0 { + return Err(std::io::Error::last_os_error()); + } + } + Ok(()) + }); + } +} + /// Spawn the client and wait for it, honoring cancellation and the time budget. On /// either, the whole process group gets SIGTERM, then SIGKILL after a grace period. /// The group is signaled only while its leader is unreaped, so its ID is still ours. diff --git a/packages/runtime-core/src/provider/image_build/tests.rs b/packages/runtime-core/src/provider/image_build/tests.rs index 252663d17..9e44f6e46 100644 --- a/packages/runtime-core/src/provider/image_build/tests.rs +++ b/packages/runtime-core/src/provider/image_build/tests.rs @@ -42,6 +42,14 @@ impl Fixture { project, } } + /// A fixture another process created at `root` (see `coordinator_child`). + fn at(root: PathBuf) -> Self { + Self { + candidate: Candidate::discover(&root.join("checkout")).unwrap(), + project: root.join("project"), + root, + } + } fn request(&self, policy: Policy) -> BuildRequest { BuildRequest { version: 1, @@ -103,6 +111,7 @@ case {mode} in fail) exit 3 ;; nometa) ;; sleep) sleep 30 & echo $! > child.pending; mv child.pending child; wait ;; + hold) echo $$ > client-pid.pending; mv client-pid.pending client-pid; sleep 30 ;; esac "# ); @@ -496,6 +505,101 @@ fn an_interrupted_receipt_publication_does_not_block_the_key() { assert_eq!(fixture.calls(&inputs), 2); } +/// The environment variable naming the fixture root the coordinator child builds in. +const COORDINATOR_ROOT: &str = "HACK_TEST_IMAGE_BUILD_COORDINATOR_ROOT"; + +#[test] +#[ignore = "coordinator process killed by a_killed_coordinator_leaves_its_client_fenced"] +fn coordinator_child() { + let Some(root) = std::env::var_os(COORDINATOR_ROOT) else { + return; + }; + // Never cleaned up here: the parent test owns the root, and this process is killed. + let fixture = std::mem::ManuallyDrop::new(Fixture::at(PathBuf::from(root))); + let inputs = fixture.inputs(Policy::Missing); + let pool = FakePool::new("pool-1"); + let _ = run(&fixture.candidate, &pool, &inputs, &never); +} + +#[test] +fn a_killed_coordinator_leaves_its_client_fenced() { + let fixture = Fixture::new(); + let inputs = fixture.inputs(Policy::Missing); + fixture.client(&inputs, "hold"); + let scratch = fixture.scratch(&inputs); + let mut coordinator = std::process::Command::new(std::env::current_exe().unwrap()) + .args([ + "--exact", + "provider::image_build::tests::coordinator_child", + "--ignored", + "--nocapture", + ]) + .env(COORDINATOR_ROOT, &fixture.root) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .unwrap(); + let started = Instant::now(); + while !scratch.join("client-pid").exists() { + assert!( + started.elapsed() < Duration::from_secs(60), + "the client never started" + ); + std::thread::sleep(Duration::from_millis(20)); + } + // SIGKILL: the coordinator drops nothing, as when hack-native itself is killed. + coordinator.kill().unwrap(); + coordinator.wait().unwrap(); + let client: libc::pid_t = fs::read_to_string(scratch.join("client-pid")) + .unwrap() + .trim() + .parse() + .unwrap(); + let root = builds_root(&fixture.candidate); + let key = root + .join(&inputs.namespace) + .join(format!("{}.lock", inputs.key)); + let fence = root.join("active.lock"); + let held = |path: &Path| { + Flock::try_lock(Flock::open(path).unwrap(), libc::LOCK_SH) + .unwrap() + .is_none() + || Flock::try_lock(Flock::open(path).unwrap(), libc::LOCK_EX) + .unwrap() + .is_none() + }; + assert!( + held(&key), + "the key lock was released under a surviving client" + ); + assert!( + held(&fence), + "the build fence was released under a surviving client" + ); + assert!(require_no_active_build(&fixture.candidate).is_err()); + let pool = FakePool::new("pool-1"); + let busy = run( + &fixture.candidate, + &pool, + &fixture.inputs(Policy::Rebuild), + &never, + ) + .unwrap_err(); + assert_eq!(busy.code, "native_build_busy"); + // SAFETY: the surviving client leads its own process group, created for it. + unsafe { libc::killpg(client, libc::SIGKILL) }; + let started = Instant::now(); + while held(&key) || held(&fence) { + assert!( + started.elapsed() < Duration::from_secs(10), + "locks were never released" + ); + std::thread::sleep(Duration::from_millis(20)); + } + assert!(require_no_active_build(&fixture.candidate).is_ok()); +} + #[test] fn bake_definitions_never_evaluate_project_text() { assert_eq!(bake_literal("${HOME}"), "$${HOME}");