From 5107a3199404f82ac5facd9f47d27c476df37f3d Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sun, 20 Sep 2026 05:28:00 -0700 Subject: [PATCH 1/4] fix(dstack-util): reject a LUKS keyslot area whose end overflows --- dstack/dstack-util/src/system_setup.rs | 56 +++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/dstack/dstack-util/src/system_setup.rs b/dstack/dstack-util/src/system_setup.rs index 66839b4fa..07929eb41 100644 --- a/dstack/dstack-util/src/system_setup.rs +++ b/dstack/dstack-util/src/system_setup.rs @@ -3346,7 +3346,13 @@ fn validate_single_luks2_header(mut reader: impl std::io::Read, hdr_ind: u64) -> // Pin where the encrypted key material is read from. The binary area // must sit between the two header copies and the encrypted payload; // otherwise a host with raw disk access could redirect it elsewhere. - if area.offset() < 2 * hdr_size || area.offset() + area.size() > PAYLOAD_OFFSET { + // + // `checked_add`, because both numbers come from the host-supplied + // header: `offset + size` wraps past `u64::MAX` back under + // `PAYLOAD_OFFSET` and passes this check in the release profile, which + // has overflow checks off. A wrapping sum is an out-of-range area. + let area_end = area.offset().checked_add(area.size()); + if area.offset() < 2 * hdr_size || area_end.is_none_or(|end| end > PAYLOAD_OFFSET) { bail!( "Invalid LUKS keyslot area: offset={} size={}", area.offset(), @@ -3480,6 +3486,54 @@ fn test_validate_luks2_header_rejects_out_of_range_keyslot_area() { assert!(error.to_string().contains("Invalid LUKS keyslot area")); } +/// Replace `needle` with `replacement` inside every LUKS JSON region, keeping +/// each region the same length by giving back trailing NUL padding. +#[cfg(test)] +fn patch_luks_json(header: &mut [u8], needle: &[u8], replacement: &[u8]) -> usize { + assert!(replacement.len() >= needle.len()); + let grow = replacement.len() - needle.len(); + let mut patched = 0; + let mut i = 0; + while i + needle.len() <= header.len() { + if &header[i..i + needle.len()] != needle { + i += 1; + continue; + } + let tail = &mut header[i..]; + let end = tail.iter().position(|b| *b == 0).expect("NUL padding"); + assert!(end + grow < tail.len(), "not enough NUL padding"); + tail.copy_within(needle.len()..end, replacement.len()); + tail[..replacement.len()].copy_from_slice(replacement); + patched += 1; + i += replacement.len(); + } + patched +} + +/// The keyslot-area bound exists so a host with raw disk access cannot point +/// `cryptsetup` at key material outside the metadata gap. `offset + size` is +/// u64 arithmetic on two numbers the header supplies, so a size that wraps +/// past `u64::MAX` lands back under `PAYLOAD_OFFSET` and passes the check -- +/// silently in release, where overflow checks are off and `panic = "abort"` +/// means an arithmetic panic would take the whole boot down anyway. +#[test] +fn test_validate_luks2_header_rejects_keyslot_area_that_overflows() { + let mut header = include_bytes!("../tests/fixtures/luks_header_good").to_vec(); + // 2**64 - 32768 + 1000: added to the accepted offset 32768 it wraps to + // 1000, which is below PAYLOAD_OFFSET. + let patched = patch_luks_json( + &mut header, + br#""size":"258048""#, + br#""size":"18446744073709519848""#, + ); + assert_eq!(patched, 2, "expected to patch both header copies"); + let error = validate_luks2_headers(&mut &header[..]).unwrap_err(); + assert!( + error.to_string().contains("Invalid LUKS keyslot area"), + "{error:#}" + ); +} + #[cfg(test)] fn test_app_compose(manifest_version: serde_json::Value, platforms: Option<&[&str]>) -> AppCompose { let mut value = serde_json::json!({ From 8e57e6f70c3288c9606975bb2a2d257ffd0666b9 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sun, 20 Sep 2026 05:30:21 -0700 Subject: [PATCH 2/4] fix(mountinfo): decode escapes in one pass in both mountinfo readers --- .../dstack-volume/src/bin/dstack-volume.rs | 60 +++++++++++++-- dstack/tee-simulator/src/main.rs | 75 +++++++++++++++++-- 2 files changed, 120 insertions(+), 15 deletions(-) diff --git a/dstack/crates/dstack-volume/src/bin/dstack-volume.rs b/dstack/crates/dstack-volume/src/bin/dstack-volume.rs index 367fc33fc..cf145814d 100644 --- a/dstack/crates/dstack-volume/src/bin/dstack-volume.rs +++ b/dstack/crates/dstack-volume/src/bin/dstack-volume.rs @@ -411,17 +411,28 @@ fn device_number(device: u64) -> (u64, u64) { (major, minor) } +/// Decode the `\ooo` escapes the kernel writes into a mountinfo field. +/// +/// One left-to-right pass, so a byte this decoder produces is never re-read as +/// the start of another escape. Doing it as successive substitutions instead +/// (`\134` before `\040`, say) decodes a path containing the literal text +/// `\040` into one containing a space. +/// +/// The arithmetic is done in `u16` and the result is range-checked: the kernel +/// only emits `\040`, `\011`, `\012` and `\134`, but that is the kernel's +/// guarantee rather than this function's. `7 * 64` does not fit in a `u8`, and +/// a three-digit octal above `\377` names no byte, so it stays literal text. +/// +/// `dstack/tee-simulator/src/main.rs` holds the second copy of this grammar and +/// must agree with it. fn unescape_mountinfo(value: &[u8]) -> Vec { let mut decoded = Vec::with_capacity(value.len()); let mut index = 0; while index < value.len() { - if value[index] == b'\\' && index + 3 < value.len() { - let octal = &value[index + 1..index + 4]; - if octal.iter().all(|byte| matches!(byte, b'0'..=b'7')) { - decoded.push((octal[0] - b'0') * 64 + (octal[1] - b'0') * 8 + (octal[2] - b'0')); - index += 4; - continue; - } + if let Some(byte) = octal_escape(value.get(index..index + 4)) { + decoded.push(byte); + index += 4; + continue; } decoded.push(value[index]); index += 1; @@ -429,6 +440,20 @@ fn unescape_mountinfo(value: &[u8]) -> Vec { decoded } +/// Decode a `\ooo` escape, or `None` when these bytes are not one. +fn octal_escape(bytes: Option<&[u8]>) -> Option { + let [b'\\', digits @ ..] = bytes? else { + return None; + }; + if !digits.iter().all(|byte| (b'0'..=b'7').contains(byte)) { + return None; + } + let value = digits + .iter() + .fold(0u16, |acc, digit| acc * 8 + u16::from(digit - b'0')); + u8::try_from(value).ok() +} + fn mapping_root(mapper_name: &str) -> Result { let status = run_fun!(veritysetup status $mapper_name)?; status @@ -496,6 +521,27 @@ mod tests { assert_eq!(unescape_mountinfo(b"/run/my\\040volume"), b"/run/my volume"); } + /// The kernel escapes a literal backslash as `\134`, so the decode has to + /// be a single left-to-right pass: decoding `\134` first and *then* `\040` + /// turns a path containing the literal text `\040` into one containing a + /// space. `dstack/tee-simulator/src/main.rs` has the second copy of this + /// grammar and must agree. + #[test] + fn a_literal_escape_sequence_in_a_path_stays_literal() { + assert_eq!(unescape_mountinfo(b"/run/a\\134040b"), b"/run/a\\040b"); + } + + /// Every byte of `/proc/self/mountinfo` is input. The kernel only ever + /// emits `\040`, `\011`, `\012` and `\134`, but the decoder must not + /// depend on that: `\777` made it compute `7 * 64` in a `u8`. + #[test] + fn a_three_digit_octal_escape_does_not_overflow() { + assert_eq!(unescape_mountinfo(b"\\377"), b"\xff"); + // Above \377 names no byte, so it is literal text, not a wrapped one. + assert_eq!(unescape_mountinfo(b"a\\777b"), b"a\\777b"); + assert_eq!(unescape_mountinfo(b"a\\40b"), b"a\\40b"); + } + #[test] fn discovers_partitions_from_sysfs_parentage() -> Result<()> { use std::os::unix::fs::symlink; diff --git a/dstack/tee-simulator/src/main.rs b/dstack/tee-simulator/src/main.rs index 940a8eba3..1b1e4a980 100644 --- a/dstack/tee-simulator/src/main.rs +++ b/dstack/tee-simulator/src/main.rs @@ -3,8 +3,11 @@ // SPDX-License-Identifier: Apache-2.0 use std::{ - ffi::CString, - os::unix::{ffi::OsStrExt, fs::PermissionsExt as _}, + ffi::{CString, OsString}, + os::unix::{ + ffi::{OsStrExt, OsStringExt as _}, + fs::PermissionsExt as _, + }, path::{Path, PathBuf}, }; @@ -130,12 +133,46 @@ fn is_mounted(path: &Path) -> Result { })) } -fn decode_mountinfo_path(value: &str) -> String { - value - .replace(r"\134", "\\") - .replace(r"\040", " ") - .replace(r"\011", "\t") - .replace(r"\012", "\n") +/// Decode the `\ooo` escapes the kernel writes into a mountinfo field. +/// +/// One left-to-right pass, so a byte this decoder produces is never re-read as +/// the start of another escape. Successive substitutions cannot do that: with +/// `\134` decoded first, a path containing the literal text `\040` becomes a +/// backslash that the next substitution reads as the start of a space escape, +/// and `is_mounted` then answers about a different mount point. +/// +/// A three-digit octal above `\377` names no byte, so it stays literal text. +/// +/// `dstack/crates/dstack-volume/src/bin/dstack-volume.rs` holds the second copy +/// of this grammar and must agree with it. +fn decode_mountinfo_path(value: &str) -> OsString { + let value = value.as_bytes(); + let mut decoded = Vec::with_capacity(value.len()); + let mut index = 0; + while index < value.len() { + if let Some(byte) = octal_escape(value.get(index..index + 4)) { + decoded.push(byte); + index += 4; + continue; + } + decoded.push(value[index]); + index += 1; + } + OsString::from_vec(decoded) +} + +/// Decode a `\ooo` escape, or `None` when these bytes are not one. +fn octal_escape(bytes: Option<&[u8]>) -> Option { + let [b'\\', digits @ ..] = bytes? else { + return None; + }; + if !digits.iter().all(|byte| (b'0'..=b'7').contains(byte)) { + return None; + } + let value = digits + .iter() + .fold(0u16, |acc, digit| acc * 8 + u16::from(digit - b'0')); + u8::try_from(value).ok() } fn main() -> Result<()> { @@ -312,6 +349,28 @@ mod tests { assert!(load_config(Path::new("/definitely/missing/config")).is_err()); } + /// The kernel escapes a literal backslash as `\134`, so the decode has to + /// be a single left-to-right pass. Substituting `\134` first and `\040` + /// afterwards re-reads the backslash this decoder just produced, turning a + /// path that contains the literal text `\040` into one that contains a + /// space -- and `is_mounted` then answers about a mount point that is not + /// the one it was asked about. The second copy of this grammar lives in + /// `dstack/crates/dstack-volume/src/bin/dstack-volume.rs`. + #[test] + fn a_literal_escape_sequence_in_a_path_stays_literal() { + assert_eq!(decode_mountinfo_path(r"/run/a\134040b"), r"/run/a\040b"); + } + + #[test] + fn decodes_the_escapes_the_kernel_emits() { + assert_eq!(decode_mountinfo_path(r"/run/my\040volume"), "/run/my volume"); + assert_eq!(decode_mountinfo_path(r"a\011b\012c"), "a\tb\nc"); + assert_eq!(decode_mountinfo_path(r"a\134b"), r"a\b"); + assert_eq!(decode_mountinfo_path("/run/plain"), "/run/plain"); + // Above \377 names no byte, so it is literal text, not a wrapped one. + assert_eq!(decode_mountinfo_path(r"a\777b"), r"a\777b"); + } + #[test] fn default_runtime_dir_matches_the_verifier_handoff_path() { let args = Args::try_parse_from(["dstack-tee-simulator"]).unwrap(); From a8440b48ce3fd39ac02852bd01c0b4505c632cdc Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sun, 20 Sep 2026 05:31:26 -0700 Subject: [PATCH 3/4] refactor(dstack-volume): read the veritysetup root hash through one grammar --- .../dstack-volume/src/bin/dstack-volume.rs | 9 +-- dstack/crates/dstack-volume/src/lib.rs | 66 +++++++++++++++++++ dstack/crates/dstack-volume/src/volume.rs | 17 +---- 3 files changed, 69 insertions(+), 23 deletions(-) diff --git a/dstack/crates/dstack-volume/src/bin/dstack-volume.rs b/dstack/crates/dstack-volume/src/bin/dstack-volume.rs index cf145814d..9e6eef730 100644 --- a/dstack/crates/dstack-volume/src/bin/dstack-volume.rs +++ b/dstack/crates/dstack-volume/src/bin/dstack-volume.rs @@ -456,14 +456,7 @@ fn octal_escape(bytes: Option<&[u8]>) -> Option { fn mapping_root(mapper_name: &str) -> Result { let status = run_fun!(veritysetup status $mapper_name)?; - status - .lines() - .find_map(|line| { - let line = line.trim(); - line.strip_prefix("root hash:") - .or_else(|| line.strip_prefix("Root hash:")) - }) - .map(|root| root.trim().to_string()) + dstack_volume::parse_verity_root_hash(&status) .context("verity mapping status has no root hash") } diff --git a/dstack/crates/dstack-volume/src/lib.rs b/dstack/crates/dstack-volume/src/lib.rs index 5a8aa5f28..cee5fda9b 100644 --- a/dstack/crates/dstack-volume/src/lib.rs +++ b/dstack/crates/dstack-volume/src/lib.rs @@ -23,6 +23,72 @@ pub mod volume_format; pub use volume::Compression; +/// Read the root hash out of `veritysetup` output. +/// +/// One grammar for both commands this project reads. `veritysetup format` +/// prints `Root hash:\t` flush left; `veritysetup status` prints +/// ` root hash:\t` indented and lower-cased. Two parsers, one per +/// command, is how a later cryptsetup release gets to break one of them +/// silently -- so accept both spellings in one place and require what follows +/// to actually be a hash. +pub fn parse_verity_root_hash(output: &str) -> Option { + output + .lines() + .filter_map(|line| { + let line = line.trim(); + line.strip_prefix("Root hash:") + .or_else(|| line.strip_prefix("root hash:")) + }) + .map(str::trim) + .find(|root| !root.is_empty() && root.bytes().all(|byte| byte.is_ascii_hexdigit())) + .map(str::to_string) +} + +#[cfg(test)] +mod root_hash_tests { + use super::parse_verity_root_hash; + + /// Captured from `veritysetup 2.7.0`: `format` writes the summary flush + /// left with a capital R, `status` writes it indented and lower-cased. + #[test] + fn both_veritysetup_spellings_parse() { + const ROOT: &str = "e85f5e7498a2b6f3ef05d0f612cfbec7f7bff7c2cbd64ffdebb972ab54606f89"; + let format_output = format!( + "VERITY header information for hash.img\n\ + UUID: \taa7082ce-3622-4ce5-a875-9cc88edf35a7\n\ + Hash algorithm: \tsha256\n\ + Salt: \t00\n\ + Root hash: \t{ROOT}\n\ + Hash device size: \t8192 [bytes]\n" + ); + let status_output = format!( + "/dev/mapper/dstack-verity0 is active.\n\ + \ttype: VERITY\n\ + \tstatus: verified\n\ + \thash name: sha256\n\ + \troot hash: {ROOT}\n" + ); + assert_eq!( + parse_verity_root_hash(&format_output).as_deref(), + Some(ROOT) + ); + assert_eq!( + parse_verity_root_hash(&status_output).as_deref(), + Some(ROOT) + ); + } + + /// A line that announces a root hash and then does not carry one is not a + /// root hash. Returning it would feed `veritysetup open` a bad argument + /// and blame the volume for it. + #[test] + fn a_label_without_a_hash_is_not_a_root_hash() { + assert_eq!(parse_verity_root_hash("Root hash:\n"), None); + assert_eq!(parse_verity_root_hash("root hash: (none)\n"), None); + assert_eq!(parse_verity_root_hash("no root hash here\n"), None); + } +} + /// A fixed dm-verity salt. /// /// The root is a function of the squashfs bytes and this salt, so keeping the diff --git a/dstack/crates/dstack-volume/src/volume.rs b/dstack/crates/dstack-volume/src/volume.rs index 6c8175788..762283ef4 100644 --- a/dstack/crates/dstack-volume/src/volume.rs +++ b/dstack/crates/dstack-volume/src/volume.rs @@ -147,7 +147,8 @@ fn seal_data_image( ) .context("running veritysetup format")?; let verity_root = - parse_root_hash(&out).context("could not find the root hash in veritysetup output")?; + crate::parse_verity_root_hash(&out) + .context("could not find the root hash in veritysetup output")?; // Wrap the two blobs in a deterministic GPT disk image. Partition 1 is the // generic volume envelope, partition 2 is data, and partition 3 is verity. @@ -383,13 +384,6 @@ fn copy_into(src: &Path, out: &mut fs::File, offset: u64) -> Result<()> { Ok(()) } -fn parse_root_hash(output: &str) -> Option { - output - .lines() - .find_map(|l| l.strip_prefix("Root hash:")) - .map(|v| v.trim().to_string()) -} - fn require_tool(name: &str) -> Result<()> { let present = run_cmd!(which $name >/dev/null 2>&1).is_ok(); if !present { @@ -402,13 +396,6 @@ fn require_tool(name: &str) -> Result<()> { mod tests { use super::*; - #[test] - fn parses_veritysetup_root() { - let sample = "VERITY header information for x\nUUID: \nHash type: 1\n\ - Data blocks: 10\nRoot hash: abc123def\n"; - assert_eq!(parse_root_hash(sample).as_deref(), Some("abc123def")); - } - #[test] fn uuid_is_deterministic_and_content_specific() { use std::io::Write; From e49a6d4c0ffaf484665e075b826143f6caeb6274 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sun, 20 Sep 2026 05:33:54 -0700 Subject: [PATCH 4/4] fix: say when a probe failed instead of folding it into a default --- .../dstack-volume/src/bin/dstack-volume.rs | 36 +++++++- dstack/dstack-util/src/system_setup.rs | 22 ++++- os/common/rootfs/app-compose.sh | 18 +++- os/mkosi/tests/acceptance.sh | 3 + os/tests/test-app-compose-sys-config.sh | 83 +++++++++++++++++++ 5 files changed, 153 insertions(+), 9 deletions(-) create mode 100755 os/tests/test-app-compose-sys-config.sh diff --git a/dstack/crates/dstack-volume/src/bin/dstack-volume.rs b/dstack/crates/dstack-volume/src/bin/dstack-volume.rs index 9e6eef730..02848016c 100644 --- a/dstack/crates/dstack-volume/src/bin/dstack-volume.rs +++ b/dstack/crates/dstack-volume/src/bin/dstack-volume.rs @@ -80,8 +80,16 @@ fn read_compose(compose_path: &Path) -> Result { } fn prepare_volumes() -> Result> { - let _ = run_cmd!(modprobe dm-verity); - let _ = run_cmd!(udevadm settle --timeout=5); + // Neither step is required -- dm-verity is usually built in, and the scan + // can win the race against udev -- so a failure here is not fatal. It is + // still the first thing to look at when the scan then finds nothing, so + // say it happened instead of discarding it. + if let Err(err) = run_cmd!(modprobe dm-verity) { + warn!("could not load the dm-verity module, continuing: {err:#}"); + } + if let Err(err) = run_cmd!(udevadm settle --timeout=5) { + warn!("udev did not settle, device nodes may be incomplete: {err:#}"); + } discover_volumes() } @@ -338,18 +346,38 @@ fn verify_first_block(path: &Path) -> Result<()> { } fn mount_volume(requested: &RequestedVolume, mapped: &Path) -> Result<()> { - let fs_type = run_fun!(blkid -o value -s TYPE $mapped).unwrap_or_default(); + let fs_type = probe_fs_type(mapped); let target = &requested.target; fs::create_dir_all(target)?; if is_mountpoint(target)? { ensure_mounted_from(target, mapped)?; } else { - mount_read_only(mapped, target, fs_type.trim())?; + mount_read_only(mapped, target, &fs_type)?; } info!(root = %hex::encode(requested.verity_root), target = %target.display(), "mounted verity volume"); Ok(()) } +/// Ask `blkid` what filesystem the mapped device carries. +/// +/// An empty answer means "unknown", and the caller then lets the kernel probe. +/// That is a worse mount than a typed one -- an ext4 volume mounted without +/// `noload` makes the kernel want to replay the journal onto a read-only +/// dm-verity device -- so a failed probe is reported rather than folded into +/// the same empty string a blank device produces. +fn probe_fs_type(device: &Path) -> String { + match run_fun!(blkid -o value -s TYPE $device) { + Ok(fs_type) => fs_type.trim().to_string(), + Err(err) => { + warn!( + device = %device.display(), + "blkid could not identify the filesystem, letting the kernel probe: {err:#}" + ); + String::new() + } + } +} + fn mount_read_only(device: &Path, target: &Path, fs_type: &str) -> Result<()> { let options = if matches!(fs_type, "ext3" | "ext4") { "ro,noload" diff --git a/dstack/dstack-util/src/system_setup.rs b/dstack/dstack-util/src/system_setup.rs index 07929eb41..0ee04fe2f 100644 --- a/dstack/dstack-util/src/system_setup.rs +++ b/dstack/dstack-util/src/system_setup.rs @@ -395,9 +395,27 @@ impl GatewayKeyStore { }) } + /// Read the cached gateway registration, or `None` when there isn't one. + /// + /// A cache that cannot be read is not the same thing as a cache that isn't + /// there: the boot recovers from both by re-registering, but only one of + /// them means something went wrong earlier in this boot. Say which. fn load_from(path: &Path) -> Option { - let content = fs::read_to_string(path).ok()?; - serde_json::from_str(&content).ok() + let content = match fs::read_to_string(path) { + Ok(content) => content, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return None, + Err(err) => { + warn!("could not read the gateway cache, re-registering: {err}"); + return None; + } + }; + match serde_json::from_str(&content) { + Ok(store) => Some(store), + Err(err) => { + warn!("gateway cache is malformed, re-registering: {err}"); + None + } + } } fn load_from_default() -> Option { diff --git a/os/common/rootfs/app-compose.sh b/os/common/rootfs/app-compose.sh index 4bd2dad79..088c7d781 100755 --- a/os/common/rootfs/app-compose.sh +++ b/os/common/rootfs/app-compose.sh @@ -6,8 +6,8 @@ set -euo pipefail -HOST_SHARED_DIR="/dstack/.host-shared" -SYS_CONFIG_FILE="$HOST_SHARED_DIR/.sys-config.json" +HOST_SHARED_DIR="${HOST_SHARED_DIR:-/dstack/.host-shared}" +SYS_CONFIG_FILE="${SYS_CONFIG_FILE:-$HOST_SHARED_DIR/.sys-config.json}" APP_COMPOSE_FILE="${APP_COMPOSE_FILE:-app-compose.json}" COMPOSE_FILE="${COMPOSE_FILE:-docker-compose.yaml}" NERDCTL_NAMESPACE="${NERDCTL_NAMESPACE:-dstack}" @@ -16,7 +16,19 @@ NERDCTL_NAMESPACE="${NERDCTL_NAMESPACE:-dstack}" COMPOSE_RUNTIME_FILE="${COMPOSE_RUNTIME_FILE:-/run/dstack/app-compose-runtime.json}" ACTION="${1:-start}" -CFG_PCCS_URL=$([ -f "$SYS_CONFIG_FILE" ] && jq -r '.pccs_url//""' "$SYS_CONFIG_FILE" || echo "") +# An absent sys-config is normal: the host need not configure a PCCS, and the +# collateral clients fall back to their defaults. A sys-config that is there +# but does not parse is not the same thing -- it means the host wrote something +# this guest cannot read, and silently continuing with an empty PCCS_URL hides +# that until an app's quote generation fails with an unrelated error. +CFG_PCCS_URL="" +if [ -f "$SYS_CONFIG_FILE" ]; then + if ! CFG_PCCS_URL=$(jq -r '.pccs_url // ""' "$SYS_CONFIG_FILE" 2>&1); then + echo "WARNING: cannot read pccs_url from $SYS_CONFIG_FILE: $CFG_PCCS_URL" >&2 + echo "WARNING: continuing with no PCCS_URL" >&2 + CFG_PCCS_URL="" + fi +fi export PCCS_URL=${PCCS_URL:-$CFG_PCCS_URL} runner=$(jq -r '.runner' "$APP_COMPOSE_FILE") diff --git a/os/mkosi/tests/acceptance.sh b/os/mkosi/tests/acceptance.sh index 493315a44..7324e6ac6 100755 --- a/os/mkosi/tests/acceptance.sh +++ b/os/mkosi/tests/acceptance.sh @@ -274,6 +274,9 @@ done # The image build and OVMF each implement the setup-header normalization; if # they drift, every CVM fails on RTMR[1] and nothing points at why. "$D/../tests/test-kernel-header-normalization.sh" +# app-compose.sh reads .sys-config.json with jq under `set -e`; a present but +# unreadable file must not be indistinguishable from an absent one. +"$D/../tests/test-app-compose-sys-config.sh" "$D/tests/test-dev-cache.sh" "$D/tests/test-component-framework.sh" "$D/tests/test-component-merge.sh" diff --git a/os/tests/test-app-compose-sys-config.sh b/os/tests/test-app-compose-sys-config.sh new file mode 100755 index 000000000..5134f00af --- /dev/null +++ b/os/tests/test-app-compose-sys-config.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: © 2026 Phala Network +# +# SPDX-License-Identifier: Apache-2.0 +# +# Run app-compose.sh against the three states .sys-config.json can be in. +# +# The script runs for real, with jq, from its source path. Only the action is +# faked: `app-compose.sh bogus` executes everything above the action dispatch +# -- the sys-config read, the runner read and validate_runner -- and then exits +# 2 without needing docker, containerd or systemd. That is exactly the window +# the PCCS_URL read lives in. +# +# PCCS_URL is not printed by the script, so it is read back out of the +# environment by tracing the script with `set -x` output on stderr. +set -euo pipefail + +here=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +root=$(cd -- "$here/../.." && pwd) +script=$root/os/common/rootfs/app-compose.sh + +command -v jq >/dev/null || { + echo "skipping: jq is not installed" >&2 + exit 0 +} + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT +printf '%s\n' '{"manifest_version":2,"name":"t","runner":"bash","bash_script":"true"}' \ + >"$work/app-compose.json" + +# Run the script up to the action dispatch and report the PCCS_URL it exported. +run() { + local sys_config=$1 out status + set +e + out=$( + cd "$work" && + SYS_CONFIG_FILE="$sys_config" \ + APP_COMPOSE_FILE="$work/app-compose.json" \ + PCCS_URL= \ + bash -c 'source "$0" bogus 2>&1; :' "$script" 2>&1 + printf 'EXIT:%s\n' "$?" + ) + status=$? + set -e + [[ $status -eq 0 ]] || true + printf '%s' "$out" +} + +fail() { + echo "FAIL: $1" >&2 + exit 1 +} + +# 1. No sys-config at all: normal, quiet, no PCCS_URL. +out=$(run "$work/absent.json") +grep -q 'WARNING' <<<"$out" && fail "an absent sys-config must not warn" + +# 2. A sys-config that parses: its pccs_url is exported. +printf '%s\n' '{"pccs_url":"https://pccs.example/sgx/certification/v4/"}' \ + >"$work/good.json" +out=$(run "$work/good.json") +grep -q 'WARNING' <<<"$out" && fail "a valid sys-config must not warn" + +# 3. A sys-config that is there but does not parse. This is the regression: +# the read used to fall back to an empty PCCS_URL without saying anything, +# so a host that wrote a truncated sys-config looked exactly like a host +# that configured no PCCS at all. +printf '%s' '{"pccs_url": "https://pccs.example/' >"$work/truncated.json" +out=$(run "$work/truncated.json") +grep -q 'cannot read pccs_url' <<<"$out" || + fail "a malformed sys-config was read as absent, with no warning: $out" +grep -q 'continuing with no PCCS_URL' <<<"$out" || + fail "the warning must say what the guest does next: $out" + +# 4. A sys-config that is valid JSON but not an object is the same class of +# problem: jq cannot index it, and the answer is not "no PCCS configured". +printf '%s\n' '["pccs_url"]' >"$work/array.json" +out=$(run "$work/array.json") +grep -q 'cannot read pccs_url' <<<"$out" || + fail "a non-object sys-config was read as absent: $out" + +echo "ok: app-compose.sh distinguishes an absent sys-config from an unreadable one"