diff --git a/crates/smolvm-pack/src/assets.rs b/crates/smolvm-pack/src/assets.rs index a9cbd33d5..f4508f2de 100644 --- a/crates/smolvm-pack/src/assets.rs +++ b/crates/smolvm-pack/src/assets.rs @@ -664,8 +664,12 @@ pub fn decompress_assets(compressed: &[u8], output_dir: &Path) -> Result<()> { .map_err(|e| PackError::Compression(e.to_string()))?; let mut archive = tar::Archive::new(decoder); - archive - .unpack(output_dir) + // Route through safe_unpack (not the raw tar `unpack`): a pulled pack's + // asset blob is untrusted, and its libs are later dlopen'd — the raw path + // would create escaping symlinks (e.g. `lib/libkrun → /tmp/evil.so`) and + // has no entry/size caps. safe_unpack rejects symlinks/hardlinks/traversal + // and bounds the extraction. + crate::extract::safe_unpack(&mut archive, output_dir) .map_err(|e| PackError::Tar(e.to_string()))?; Ok(()) @@ -680,8 +684,9 @@ pub fn decompress_assets_from_file(compressed_path: &Path, output_dir: &Path) -> zstd::stream::Decoder::new(file).map_err(|e| PackError::Compression(e.to_string()))?; let mut archive = tar::Archive::new(decoder); - archive - .unpack(output_dir) + // See decompress_assets: use safe_unpack so an untrusted pack can't plant + // escaping symlinks or blow past the resource caps. + crate::extract::safe_unpack(&mut archive, output_dir) .map_err(|e| PackError::Tar(e.to_string()))?; Ok(()) @@ -737,6 +742,34 @@ pub fn crc32_file_range(path: &Path, offset: u64, size: u64) -> Result { mod tests { use super::*; + // A pulled pack's asset blob is untrusted; decompress must go through + // safe_unpack, which rejects a symlink that escapes the output dir (the raw + // tar `unpack` would create it, enabling a `lib/... -> /evil` dlopen swap). + #[test] + fn decompress_assets_rejects_escaping_symlink() { + let mut builder = tar::Builder::new(Vec::new()); + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Symlink); + header.set_size(0); + header.set_mode(0o777); + builder + .append_link(&mut header, "evil", "../../../../../../etc/passwd") + .unwrap(); + let tar_bytes = builder.into_inner().unwrap(); + let compressed = zstd::encode_all(&tar_bytes[..], ZSTD_LEVEL).unwrap(); + + let tmp = tempfile::tempdir().unwrap(); + let out = tmp.path().join("out"); + assert!( + decompress_assets(&compressed, &out).is_err(), + "escaping symlink must be rejected" + ); + assert!( + std::fs::symlink_metadata(out.join("evil")).is_err(), + "no escaping symlink should be created" + ); + } + #[test] fn digest_to_filename_rejects_path_traversal() { // A valid hex digest is accepted. diff --git a/crates/smolvm-pack/src/detect.rs b/crates/smolvm-pack/src/detect.rs index a8d939e19..52be906a9 100644 --- a/crates/smolvm-pack/src/detect.rs +++ b/crates/smolvm-pack/src/detect.rs @@ -42,6 +42,18 @@ pub enum PackedMode { }, } +/// Minimum section byte length required to hold the header plus a manifest and +/// assets of the declared sizes, or `None` if the sum overflows `usize` (a +/// crafted `assets_size` must not wrap the bound into a small value that a +/// later `size >= expected` check would accept). Pure and platform-independent +/// so the overflow guard is unit-testable off macOS. +#[cfg(any(target_os = "macos", test))] +fn embedded_section_extent(manifest_size: u32, assets_size: u64) -> Option { + crate::format::SECTION_HEADER_SIZE + .checked_add(manifest_size as usize) + .and_then(|s| s.checked_add(usize::try_from(assets_size).ok()?)) +} + /// Detect whether this process is running as a packed binary. /// /// Checks in order: @@ -203,12 +215,15 @@ fn read_embedded_section() -> Option { let header_bytes = std::slice::from_raw_parts(data_ptr, SECTION_HEADER_SIZE); let section_header = SectionHeader::from_bytes(header_bytes).ok()?; - // Validate sizes - let expected_size = SECTION_HEADER_SIZE - + section_header.manifest_size as usize - + section_header.assets_size as usize; - if size < expected_size { - return None; + // Validate sizes with checked arithmetic. A crafted or corrupt + // `assets_size` (u64) could otherwise overflow the sum, wrapping the + // expected size small so `size <` passes and a bogus `assets_size` + // reaches the raw `assets_ptr` below as an out-of-bounds length. The + // file-parsing path (`packer.rs::validate_footer`) already uses + // `checked_add`; keep them consistent. + match embedded_section_extent(section_header.manifest_size, section_header.assets_size) { + Some(exp) if size >= exp => {} + _ => return None, } // Parse manifest @@ -233,6 +248,19 @@ fn read_embedded_section() -> Option { mod tests { use super::*; + // A crafted assets_size (u64 near max) must not overflow the section-extent + // sum: an unchecked `+` wraps it small, defeating the `size >= expected` + // bound and handing a huge assets_size to the raw pointer read. + #[test] + fn embedded_section_extent_rejects_overflow() { + assert_eq!( + embedded_section_extent(100, 200), + Some(crate::format::SECTION_HEADER_SIZE + 300) + ); + assert_eq!(embedded_section_extent(100, u64::MAX), None); + assert_eq!(embedded_section_extent(u32::MAX, u64::MAX), None); + } + #[test] fn test_detect_returns_none_for_normal_binary() { // The test binary is a normal Rust executable, not a packed binary. diff --git a/crates/smolvm-pack/src/extract.rs b/crates/smolvm-pack/src/extract.rs index f7b0d7729..d1ffbfdb9 100644 --- a/crates/smolvm-pack/src/extract.rs +++ b/crates/smolvm-pack/src/extract.rs @@ -274,7 +274,10 @@ fn verify_parent_within_dest(path: &Path, real_dest: &Path) -> std::io::Result<( /// `lib/libkrun.dylib → /tmp/evil.so`, and subsequent `dlopen()` would /// load the attacker's library. This function rejects any entry that is /// not a regular file or directory. -fn safe_unpack(archive: &mut tar::Archive, dest: &Path) -> std::io::Result<()> { +pub(crate) fn safe_unpack( + archive: &mut tar::Archive, + dest: &Path, +) -> std::io::Result<()> { safe_unpack_with_limits(archive, dest, &SafeUnpackLimits::from_env()) } diff --git a/crates/smolvm-pack/src/macho.rs b/crates/smolvm-pack/src/macho.rs index 47c446fe3..6a19d7635 100644 --- a/crates/smolvm-pack/src/macho.rs +++ b/crates/smolvm-pack/src/macho.rs @@ -443,16 +443,41 @@ impl MachoFile { )); } - // Read load commands - let mut load_commands = Vec::with_capacity(header.ncmds as usize); + // Read load commands. Cap the pre-allocation by the byte budget (each + // command is at least LoadCommand::SIZE), so a bogus `ncmds` can't demand + // a huge allocation before a single byte is validated. + let mut load_commands = + Vec::with_capacity((header.ncmds as usize).min(data.len() / LoadCommand::SIZE + 1)); for _ in 0..header.ncmds { let cmd_start = cursor.position() as usize; let lc = LoadCommand::read(&mut cursor)?; + // Validate the declared command size before trusting it: at least its + // own 8-byte header (a smaller value underflows `cmdsize - SIZE` below + // into an enormous allocation) and within the file (so the fallback + // `vec![0u8; remaining]` and `set_position` stay bounded by the input). + let cmd_end = cmd_start.checked_add(lc.cmdsize as usize); + if (lc.cmdsize as usize) < LoadCommand::SIZE + || cmd_end.is_none_or(|end| end > data.len()) + { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "malformed Mach-O load command 0x{:x}: cmdsize {}", + lc.cmd, lc.cmdsize + ), + )); + } + let parsed = match lc.cmd { LC_SEGMENT_64 => { let segment = SegmentCommand64::read(&mut cursor, lc.cmd, lc.cmdsize)?; - let mut sections = Vec::with_capacity(segment.nsects as usize); + // Cap by the byte budget: `nsects` is attacker-controlled and + // each section is Section64::SIZE bytes, so an inflated count + // must not drive the pre-allocation. + let mut sections = Vec::with_capacity( + (segment.nsects as usize).min(data.len() / Section64::SIZE + 1), + ); for _ in 0..segment.nsects { sections.push(Section64::read(&mut cursor)?); } @@ -1162,6 +1187,43 @@ mod tests { assert_eq!(header.ncmds, parsed.ncmds); } + fn header_with_one_cmd() -> Vec { + let header = MachHeader64 { + magic: MH_MAGIC_64, + cputype: 0x0100000c, // ARM64 + cpusubtype: 0, + filetype: 2, // MH_EXECUTE + ncmds: 1, + sizeofcmds: 0, + flags: 0, + reserved: 0, + }; + let mut buf = Vec::new(); + header.write(&mut buf).unwrap(); + buf + } + + // A load command claiming cmdsize < its own 8-byte header would underflow + // `cmdsize - SIZE` into an enormous allocation; parse must reject it cleanly + // so the packer's "not a Mach-O, fall back" path works instead of aborting. + #[test] + fn parse_rejects_undersized_load_command() { + let mut buf = header_with_one_cmd(); + buf.extend_from_slice(&0x99u32.to_le_bytes()); // unknown cmd + buf.extend_from_slice(&4u32.to_le_bytes()); // cmdsize = 4 (< 8) + assert!(MachoFile::parse(&buf).is_err()); + } + + // A cmdsize that runs past the end of the file must error rather than drive a + // multi-gigabyte `vec![0u8; cmdsize - SIZE]` allocation. + #[test] + fn parse_rejects_out_of_bounds_cmdsize() { + let mut buf = header_with_one_cmd(); + buf.extend_from_slice(&0x99u32.to_le_bytes()); // unknown cmd + buf.extend_from_slice(&u32::MAX.to_le_bytes()); // cmdsize ~4 GiB, no data + assert!(MachoFile::parse(&buf).is_err()); + } + #[test] fn test_page_align() { // Test exact page boundaries diff --git a/crates/smolvm-pack/src/packer.rs b/crates/smolvm-pack/src/packer.rs index 1b5aa2aad..a5914dc10 100644 --- a/crates/smolvm-pack/src/packer.rs +++ b/crates/smolvm-pack/src/packer.rs @@ -858,7 +858,7 @@ mod tests { // Add a fake layer (digest must be at least 12 chars after sha256:) collector - .add_layer("sha256:embedded123456", b"embedded layer content") + .add_layer("sha256:beefcafe1234", b"embedded layer content") .unwrap(); // Create manifest @@ -879,7 +879,7 @@ mod tests { // Verify we can read the manifest with layer info let manifest = read_manifest(&output_path).unwrap(); assert_eq!(manifest.assets.layers.len(), 1); - assert_eq!(manifest.assets.layers[0].digest, "sha256:embedded123456"); + assert_eq!(manifest.assets.layers[0].digest, "sha256:beefcafe1234"); // Verify footer indicates embedded mode let footer = read_footer(&output_path).unwrap(); @@ -890,7 +890,7 @@ mod tests { let extract_dir = temp_dir.path().join("extracted"); extract_assets(&output_path, &extract_dir).unwrap(); - let layer_file = extract_dir.join("layers/embedded1234.tar"); // First 12 chars + let layer_file = extract_dir.join("layers/beefcafe1234.tar"); // First 12 chars assert!(layer_file.exists()); assert_eq!( fs::read_to_string(&layer_file).unwrap(), diff --git a/crates/smolvm-registry/src/cache.rs b/crates/smolvm-registry/src/cache.rs index 97395b435..3a73c73d3 100644 --- a/crates/smolvm-registry/src/cache.rs +++ b/crates/smolvm-registry/src/cache.rs @@ -59,6 +59,12 @@ impl BlobCache { Ok(Self { root, max_size }) } + /// The cache's byte cap — an absolute upper bound for a single blob, used to + /// bound a streamed download when the manifest declares no usable size. + pub fn max_size(&self) -> u64 { + self.max_size + } + /// Look up a blob by digest. Returns the path if it exists. pub fn get(&self, digest: &str) -> Option { let path = self.blob_path(digest); diff --git a/crates/smolvm-registry/src/peer.rs b/crates/smolvm-registry/src/peer.rs index 0d0256f25..469f67a2d 100644 --- a/crates/smolvm-registry/src/peer.rs +++ b/crates/smolvm-registry/src/peer.rs @@ -116,13 +116,14 @@ pub(crate) async fn fetch_blob_from_peers( client: &reqwest::Client, peers: &[String], digest: &str, + max_bytes: u64, output: Option<&Path>, cache: &BlobCache, ) -> Option { let partial_path = cache.blob_path_for(digest).with_extension("partial"); for peer in peers { - match fetch_one(client, peer, digest, output, cache).await { + match fetch_one(client, peer, digest, max_bytes, output, cache).await { Ok(result) => { tracing::info!(peer = %peer, digest = %digest, "fetched layer blob from peer (P2P)"); return Some(result); @@ -149,6 +150,7 @@ async fn fetch_one( client: &reqwest::Client, peer: &str, digest: &str, + max_bytes: u64, output: Option<&Path>, cache: &BlobCache, ) -> Result { @@ -167,7 +169,7 @@ async fn fetch_one( // Identical verification + cache accounting to the registry path: the digest // is re-checked while streaming, so a lying peer can never poison the cache. - stream_verify_adopt(resp.bytes_stream(), digest, output, cache).await + stream_verify_adopt(resp.bytes_stream(), digest, max_bytes, output, cache).await } #[cfg(test)] @@ -210,7 +212,7 @@ mod tests { mount_blob(&peer, &digest, 200, data.clone()).await; let client = reqwest::Client::new(); - let result = fetch_blob_from_peers(&client, &[peer.uri()], &digest, None, &cache) + let result = fetch_blob_from_peers(&client, &[peer.uri()], &digest, u64::MAX, None, &cache) .await .expect("peer hit must yield a result"); @@ -237,7 +239,7 @@ mod tests { let client = reqwest::Client::new(); let result = - fetch_blob_from_peers(&client, &[miss.uri(), hit.uri()], &digest, None, &cache) + fetch_blob_from_peers(&client, &[miss.uri(), hit.uri()], &digest, u64::MAX, None, &cache) .await .expect("second peer must satisfy the fetch"); assert_eq!(result.size, data.len() as u64); @@ -257,7 +259,7 @@ mod tests { let client = reqwest::Client::new(); let result = - fetch_blob_from_peers(&client, &[liar.uri(), honest.uri()], &digest, None, &cache) + fetch_blob_from_peers(&client, &[liar.uri(), honest.uri()], &digest, u64::MAX, None, &cache) .await .expect("honest peer must satisfy the fetch"); assert_eq!(result.size, data.len() as u64); @@ -293,7 +295,7 @@ mod tests { .build() .unwrap(); let result = - fetch_blob_from_peers(&client, &[slow.uri(), fast.uri()], &digest, None, &cache) + fetch_blob_from_peers(&client, &[slow.uri(), fast.uri()], &digest, u64::MAX, None, &cache) .await .expect("fast peer must satisfy the fetch after the slow one times out"); assert_eq!(result.size, data.len() as u64); @@ -313,7 +315,7 @@ mod tests { let client = reqwest::Client::new(); let result = - fetch_blob_from_peers(&client, &[a.uri(), b.uri()], &digest, None, &cache).await; + fetch_blob_from_peers(&client, &[a.uri(), b.uri()], &digest, u64::MAX, None, &cache).await; assert!(result.is_none(), "all-peers-fail must return None"); assert!(cache.get(&digest).is_none(), "cache must stay empty"); let partial = cache.blob_path_for(&digest).with_extension("partial"); @@ -334,7 +336,7 @@ mod tests { let client = reqwest::Client::new(); // Port 1 has nothing listening → connection refused. let dead = "http://127.0.0.1:1".to_string(); - let result = fetch_blob_from_peers(&client, &[dead, hit.uri()], &digest, None, &cache) + let result = fetch_blob_from_peers(&client, &[dead, hit.uri()], &digest, u64::MAX, None, &cache) .await .expect("live peer must satisfy the fetch"); assert_eq!(result.size, data.len() as u64); diff --git a/crates/smolvm-registry/src/pull.rs b/crates/smolvm-registry/src/pull.rs index 15c6ae95d..272ad318f 100644 --- a/crates/smolvm-registry/src/pull.rs +++ b/crates/smolvm-registry/src/pull.rs @@ -97,11 +97,22 @@ pub async fn pull( // byte-for-byte as before. Peers arrive only from the control plane, and // a node with no serve-TLS identity has no peer client, so this is also a // no-op there. + // Bound the streamed download. A correct blob is exactly its declared size, + // so cap there; if the manifest declares no usable size, fall back to the + // cache's absolute byte cap so a hostile registry still can't fill the disk. + let max_bytes = if size > 0 { size } else { cache.max_size() }; + if !blob_peers.is_empty() { if let Some(peer_client) = crate::peer::peer_client() { - if let Some(result) = - crate::peer::fetch_blob_from_peers(peer_client, blob_peers, digest, output, cache) - .await + if let Some(result) = crate::peer::fetch_blob_from_peers( + peer_client, + blob_peers, + digest, + max_bytes, + output, + cache, + ) + .await { return Ok(result); } @@ -114,7 +125,7 @@ pub async fn pull( tracing::info!(digest = %digest, size, "downloading blob..."); let stream = client.pull_blob_stream(repo, digest).await?; - let result = stream_verify_adopt(stream, digest, output, cache).await?; + let result = stream_verify_adopt(stream, digest, max_bytes, output, cache).await?; tracing::info!(digest = %digest, size = result.size, "pull complete"); @@ -134,6 +145,7 @@ pub async fn pull( pub(crate) async fn stream_verify_adopt( stream: S, digest: &str, + max_bytes: u64, output: Option<&Path>, cache: &BlobCache, ) -> Result @@ -148,9 +160,27 @@ where let mut stream = std::pin::pin!(stream); while let Some(chunk_result) = stream.next().await { let chunk: bytes::Bytes = chunk_result.map_err(RegistryError::Http)?; + total_bytes += chunk.len() as u64; + // Bound the download: a correct blob is exactly its declared size, so a + // registry or peer that streams more is hostile or broken. Abort before + // writing the offending chunk (the digest check only runs after the whole + // stream, so without this an unbounded body fills the host disk first). + if total_bytes > max_bytes { + drop(file); + if let Err(e) = tokio::fs::remove_file(&partial_path).await { + tracing::warn!( + error = %e, + path = %partial_path.display(), + "failed to clean up partial blob after exceeding size cap" + ); + } + return Err(RegistryError::ResponseTooLarge { + what: "blob".to_string(), + limit: max_bytes.min(usize::MAX as u64) as usize, + }); + } hasher.update(&chunk); file.write_all(&chunk).await?; - total_bytes += chunk.len() as u64; } file.flush().await?; drop(file); @@ -195,6 +225,28 @@ mod tests { use super::*; use crate::{CONFIG_MEDIA_TYPE, MANIFEST_MEDIA_TYPE}; use wiremock::matchers::{method, path}; + + // A stream that delivers more bytes than the cap must be rejected before the + // (post-stream) digest check, and its partial file cleaned up — so a hostile + // registry/peer can't fill the host disk with garbage under any digest. + #[tokio::test] + async fn stream_exceeding_max_bytes_is_rejected_and_partial_removed() { + let tmp = tempfile::tempdir().unwrap(); + let cache = BlobCache::open(tmp.path().to_path_buf(), 1024 * 1024).unwrap(); + let digest = format!("sha256:{}", "a".repeat(64)); + // 300 bytes streamed against a 100-byte cap. + let chunks: Vec> = + (0..3).map(|_| Ok(bytes::Bytes::from(vec![0u8; 100]))).collect(); + let res = + stream_verify_adopt(futures_util::stream::iter(chunks), &digest, 100, None, &cache) + .await; + assert!( + matches!(res, Err(RegistryError::ResponseTooLarge { .. })), + "oversized stream must be rejected, got {res:?}" + ); + let partial = cache.blob_path_for(&digest).with_extension("partial"); + assert!(!partial.exists(), "partial must be removed after the cap trips"); + } use wiremock::{Mock, MockServer, ResponseTemplate}; /// With empty `blob_peers`, `pull` takes the registry path exactly as before: diff --git a/src/cli/machine.rs b/src/cli/machine.rs index f474bd2b6..9364c9b92 100644 --- a/src/cli/machine.rs +++ b/src/cli/machine.rs @@ -164,6 +164,43 @@ fn parse_published_sockets( )); } } + // Endpoints a bridge *creates* must be unique, or one silently clobbers + // another at launch (the listener file is removed and re-bound). An `expose` + // socket has the host create the listener — at the explicit host path, or the + // guest basename in the per-VM dir when defaulted — so two exposes that + // resolve to the same host file collide. A `mount` socket has the guest + // create the listener at its guest path, so two mounts on one guest path + // collide. + let mut host_listeners = std::collections::HashSet::new(); + let mut guest_listeners = std::collections::HashSet::new(); + for s in &out { + if s.direction.host_listens() { + let key = match &s.host_path { + Some(h) => h.clone(), + None => std::path::Path::new(&s.guest_path) + .file_name() + .map(|b| b.to_string_lossy().into_owned()) + .unwrap_or_else(|| "published.sock".to_string()), + }; + if !host_listeners.insert(key.clone()) { + return Err(smolvm::Error::config( + "expose-socket", + format!( + "two exposed sockets resolve to the same host endpoint '{key}'; \ + give distinct host paths as GUEST_PATH:HOST_PATH" + ), + )); + } + } else if !guest_listeners.insert(s.guest_path.clone()) { + return Err(smolvm::Error::config( + "mount-socket", + format!( + "two mounted sockets resolve to the same guest path '{}'", + s.guest_path + ), + )); + } + } Ok(out) } @@ -1787,6 +1824,44 @@ mod tests { assert!(parse_published_sockets(&["/run/a;b.sock".to_string()], &[]).is_err()); } + #[test] + fn parse_published_sockets_rejects_colliding_endpoints() { + // Two exposes whose default host paths share a basename collide on the + // host (both land at /api.sock). + assert!(parse_published_sockets( + &["/run/a/api.sock".to_string(), "/run/b/api.sock".to_string()], + &[], + ) + .is_err()); + // Two exposes pinned to the same explicit host path collide. + assert!(parse_published_sockets( + &[ + "/run/a.sock:/tmp/h.sock".to_string(), + "/run/b.sock:/tmp/h.sock".to_string(), + ], + &[], + ) + .is_err()); + // Two mounts targeting the same guest path collide in the guest. + assert!(parse_published_sockets( + &[], + &[ + "/run/h1.sock:/run/g.sock".to_string(), + "/run/h2.sock:/run/g.sock".to_string(), + ], + ) + .is_err()); + // Same basename but distinct pinned host paths is fine (no collision). + assert!(parse_published_sockets( + &[ + "/run/a/api.sock:/tmp/a-api.sock".to_string(), + "/run/b/api.sock:/tmp/b-api.sock".to_string(), + ], + &[], + ) + .is_ok()); + } + #[test] fn parse_cli_secret_refs_builds_env_and_file_refs() { let refs = parse_cli_secret_refs( diff --git a/src/secrets.rs b/src/secrets.rs index a7f8ef773..d0b989070 100644 --- a/src/secrets.rs +++ b/src/secrets.rs @@ -59,6 +59,10 @@ enum FileSourceError { Symlink, /// File size exceeded the per-call cap. TooLarge, + /// The path is not a regular file (FIFO, device, socket, directory). + /// Reading such a source can block indefinitely or return unbounded / + /// meaningless data, so a secret must come from a real file. + NotRegular, /// Any other I/O failure (missing, permission denied, non-UTF-8 /// content, etc.). The inner error is available for callers that /// want to log it; classifiers treat this as `FileReadFailed`. @@ -74,6 +78,7 @@ impl std::fmt::Display for FileSourceError { (store the canonicalized target instead)" ), Self::TooLarge => write!(f, "file size exceeds maximum"), + Self::NotRegular => write!(f, "from_file path is not a regular file"), Self::Io(e) => write!(f, "{}", e), } } @@ -122,7 +127,11 @@ fn read_from_file_source(path: &std::path::Path) -> std::result::Result std::result::Result std::result::Result ResolutionFailure { match e { FileSourceError::TooLarge => ResolutionFailure::FileTooLarge, - FileSourceError::Symlink | FileSourceError::Io(_) => ResolutionFailure::FileReadFailed, + FileSourceError::Symlink | FileSourceError::NotRegular | FileSourceError::Io(_) => { + ResolutionFailure::FileReadFailed + } } } @@ -610,6 +626,21 @@ mod tests { ); } + // A from_file pointing at a FIFO must return a clean error, not block + // forever opening a writer-less FIFO. This test completing is itself the + // no-hang proof (a broken resolver would never return here). + #[cfg(unix)] + #[test] + fn file_ref_at_fifo_errors_without_hanging() { + let tmp = tempfile::tempdir().unwrap(); + let p = tmp.path().join("fifo"); + let c = std::ffi::CString::new(p.to_str().unwrap()).unwrap(); + assert_eq!(unsafe { libc::mkfifo(c.as_ptr(), 0o600) }, 0, "mkfifo failed"); + let r = file_ref_for(&p); + let res = resolve_secret_ref_classified("FIFO", &r, ResolutionScope::TrustedLocal); + assert_eq!(res.err(), Some(ResolutionFailure::FileReadFailed)); + } + #[test] fn validate_rejects_no_source_and_multiple_sources() { let none = SecretRef {