From 811435a04513254790c7cfe0e4e27a98a7ee169b Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Mon, 19 Oct 2026 18:49:29 +0200 Subject: [PATCH 01/11] fix(manifest): close the remaining portable-name gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folding uppercased then lowercased, which misses a merge APFS makes: `ẞ` lowercases to `ß` while `ß` becomes `ss`, so `straẞe` and `strasse` passed as two entries but are one file on a Mac. The key now applies full Unicode case folding after uppercasing. Checked against every BMP character in the Windows XP and Windows 7 NTFS upcase tables and every cased character on a live case-insensitive APFS volume, it misses no merge either makes. It also drops the code points HFS+ ignores in names, so `ab` and `ab` collide as they do on HFS+. Components now reject every Unicode control character, including the C1 range some terminals act on when entry names are printed, and the bidirectional embedding, override and isolate controls that make a name display as something it is not (`invoicefdp.exe`). Joiners and directional marks stay allowed for Persian, emoji and right-to-left names. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 1 + ant-core/Cargo.toml | 2 + ant-core/src/data/manifest/path.rs | 119 ++++++++++++++---- docs/adr/ADR-0006-file-manifests-and-links.md | 32 +++-- 4 files changed, 122 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a6d2d45b..913b3bbe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -901,6 +901,7 @@ dependencies = [ "tower-http", "tracing", "tracing-subscriber", + "unicase", "unicode-normalization", "url", "utoipa", diff --git a/ant-core/Cargo.toml b/ant-core/Cargo.toml index 6518e24e..da377117 100644 --- a/ant-core/Cargo.toml +++ b/ant-core/Cargo.toml @@ -30,6 +30,8 @@ sha2 = "0.10" # Portable WebRTC Direct profile shared with ant-node. saorsa-transport = { version = "0.38.1", default-features = false, features = ["webrtc"] } thiserror = "2" +# Full Unicode case folding for the portable manifest path rules (ADR-0006). +unicase = "2.9" # NFC normalisation for the portable manifest path rules (ADR-0006). unicode-normalization = "0.1" url = "2" diff --git a/ant-core/src/data/manifest/path.rs b/ant-core/src/data/manifest/path.rs index a445b5ea..fba927ca 100644 --- a/ant-core/src/data/manifest/path.rs +++ b/ant-core/src/data/manifest/path.rs @@ -7,6 +7,7 @@ use std::collections::HashSet; use thiserror::Error; +use unicase::UniCase; use unicode_normalization::UnicodeNormalization; /// The only separator a manifest path may use. @@ -52,10 +53,19 @@ const RESERVED_DEVICE_NAMES: &[&str] = &[ "LPT\u{b2}", "LPT\u{b3}", ]; -/// Lowest non-control character. -const FIRST_PRINTABLE: char = '\u{20}'; -/// The DEL control character. -const DELETE: char = '\u{7F}'; +/// Bidirectional embedding, override and isolate controls. They make a name +/// display differently from what it is, so `invoice\u{202E}fdp.exe` shows as +/// `invoiceexe.pdf`. +const BIDI_CONTROLS: &[char] = &[ + '\u{202A}', '\u{202B}', '\u{202C}', '\u{202D}', '\u{202E}', '\u{2066}', '\u{2067}', '\u{2068}', + '\u{2069}', +]; +/// Code points HFS+ ignores when it compares names, so a name containing +/// them is the same file as one without. +const HFS_IGNORABLE_CHARS: &[char] = &[ + '\u{200C}', '\u{200D}', '\u{200E}', '\u{200F}', '\u{202A}', '\u{202B}', '\u{202C}', '\u{202D}', + '\u{202E}', '\u{206A}', '\u{206B}', '\u{206C}', '\u{206D}', '\u{206E}', '\u{206F}', '\u{FEFF}', +]; /// Replacement for a forbidden character when reducing a name to a /// portable component. const COMPONENT_REPLACEMENT: char = '_'; @@ -138,10 +148,7 @@ pub fn validate_component(component: &str) -> Result<(), PathError> { if component == "." || component == ".." { return Err(PathError::DotComponent(component.to_string())); } - if let Some(ch) = component - .chars() - .find(|c| *c < FIRST_PRINTABLE || *c == DELETE || FORBIDDEN_CHARS.contains(c)) - { + if let Some(ch) = component.chars().find(|c| is_forbidden_char(*c)) { return Err(PathError::ForbiddenChar { component: component.to_string(), ch, @@ -174,7 +181,7 @@ pub fn portable_component(name: &str) -> Option { let mut replaced: String = name .chars() .map(|c| { - if c < FIRST_PRINTABLE || c == DELETE || FORBIDDEN_CHARS.contains(&c) { + if is_forbidden_char(c) { COMPONENT_REPLACEMENT } else { c @@ -195,21 +202,32 @@ pub fn portable_component(name: &str) -> Option { validate_component(&candidate).ok().map(|()| candidate) } -/// The key two paths are compared under: NFC normalised, case folded by -/// uppercasing then lowercasing, and normalised again. +/// Whether a component may never contain `c`: any Unicode control +/// character (C0, DEL and C1, which some terminals act on), a bidirectional +/// control, or a character some platform reserves. +fn is_forbidden_char(c: char) -> bool { + c.is_control() || FORBIDDEN_CHARS.contains(&c) || BIDI_CONTROLS.contains(&c) +} + +/// The key two paths are compared under: without the code points HFS+ +/// ignores, NFC normalised, uppercased, fully case folded, and normalised +/// again. /// -/// Lowercasing alone leaves pairs a case-insensitive filesystem merges, -/// because their uppercase forms are equal: `ſ` and `s`, `ς` and `σ`, `ı` -/// and `i`. Mapping through uppercase first joins them; the final NFC pass -/// recomposes anything case mapping decomposed. The result rejects at -/// least everything APFS and NTFS would merge, and possibly a little more. +/// Each step covers merges the others miss. NTFS compares uppercase forms, +/// which joins pairs such as `ſ` and `s` or `ς` and `σ`. APFS applies full +/// Unicode case folding, which joins `ß`, `ẞ` and `ss`; lowercasing the +/// uppercase form would leave `ẞ` as `ß`. Dropping the ignorable code points +/// first lets characters on either side of one compose, and the final NFC +/// pass recomposes anything case mapping decomposed. The result rejects at +/// least everything APFS, HFS+ and NTFS would merge, and possibly a little +/// more, such as `ı` and `i`. pub fn fold(path: &str) -> String { - path.nfc() - .collect::() - .to_uppercase() - .to_lowercase() - .nfc() - .collect() + let significant: String = path + .chars() + .filter(|c| !HFS_IGNORABLE_CHARS.contains(c)) + .collect(); + let upper = significant.nfc().collect::().to_uppercase(); + UniCase::new(upper).to_folded_case().nfc().collect() } /// Reject duplicate effective names and file-versus-directory conflicts @@ -393,6 +411,9 @@ mod tests { ("\u{3c2}", "\u{3c3}"), ("\u{131}", "i"), ("stra\u{df}e", "STRASSE"), + // APFS folds capital sharp s to `ss` as well. + ("stra\u{1e9e}e", "strasse"), + ("\u{df}", "\u{1e9e}"), ] { assert_eq!( check_collisions([a, b]), @@ -419,4 +440,58 @@ mod tests { )); assert_eq!(check_collisions(["a", "a-b", "ab/c"]), Ok(())); } + + #[test] + fn components_reject_c1_and_bidi_controls_but_keep_joiners_and_marks() { + for (name, ch) in [ + ("csi\u{9B}31m", '\u{9B}'), + ("next\u{85}line", '\u{85}'), + ("invoice\u{202E}fdp.exe", '\u{202E}'), + ("isolated\u{2066}name", '\u{2066}'), + ] { + assert!( + matches!( + validate_component(name), + Err(PathError::ForbiddenChar { ch: found, .. }) if found == ch + ), + "{name:?}" + ); + } + assert_eq!( + portable_component("invoice\u{202E}fdp.exe").as_deref(), + Some("invoice_fdp.exe") + ); + // Persian writes ZWNJ inside words, emoji sequences join with ZWJ, + // and right-to-left names may carry directional marks. + for name in [ + "می\u{200C}خواهم", + "family-\u{1F468}\u{200D}\u{1F469}", + "\u{5E9}\u{200F}.txt", + ] { + assert_eq!(validate_component(name), Ok(()), "{name:?}"); + } + } + + #[test] + fn collisions_ignore_the_code_points_hfs_plus_ignores() { + for (a, b) in [ + ("ab", "a\u{200C}b"), + ("ab", "\u{FEFF}ab"), + ("ab", "a\u{200E}b"), + // Composition happens across the ignored code point. + ("caf\u{e9}", "cafe\u{200C}\u{301}"), + ] { + assert!( + matches!(check_collisions([a, b]), Err(PathError::Duplicate(_))), + "{a:?} / {b:?}" + ); + assert!( + matches!( + check_collisions([a, &format!("{b}/child")]), + Err(PathError::PrefixConflict { .. }) + ), + "{a:?} / {b:?}" + ); + } + } } diff --git a/docs/adr/ADR-0006-file-manifests-and-links.md b/docs/adr/ADR-0006-file-manifests-and-links.md index de984158..0178f66f 100644 --- a/docs/adr/ADR-0006-file-manifests-and-links.md +++ b/docs/adr/ADR-0006-file-manifests-and-links.md @@ -2,7 +2,7 @@ - **Status:** Proposed - **Date:** 2026-09-23 -- **Last amended:** 2026-10-15 +- **Last amended:** 2026-10-19 - **Decision owners:** Mick - **Reviewers:** - **Supersedes:** none @@ -283,8 +283,14 @@ A path is valid when all of the following hold: - Valid UTF-8, with `/` as the only separator. `\` is a forbidden character, never a separator. - No leading `/`, no empty component, no component equal to `.` or `..`. -- No component contains NUL, a control character (below `0x20`, or `0x7F`), - or any of `< > : " | ? * \`. +- No component contains a Unicode control character (`U+0000`–`U+001F`, + `U+007F`–`U+009F`; some terminals act on the C1 range, and entry names are + printed), a bidirectional embedding, override or isolate control + (`U+202A`–`U+202E`, `U+2066`–`U+2069`), or any of `< > : " | ? * \`. + Bidirectional controls make a name display as something it is not: + `invoicefdp.exe` shows as `invoiceexe.pdf`. Joiners and directional + marks stay allowed because real names in Persian, emoji and right-to-left + scripts use them. - No component ends with a space or a `.`. - No component, with any extension and the spaces before it removed, equals a Windows reserved device name ignoring case: `CON`, `PRN`, `AUX`, @@ -292,13 +298,19 @@ A path is valid when all of the following hold: and `LPT¹`–`LPT³`. So `CON .txt` is rejected as well as `CON.txt`. - Each component is at most `MAX_PATH_COMPONENT_BYTES` (255) bytes; the whole path is at most `MAX_PATH_BYTES` (1024) bytes. -- Effective names are compared under a folded key: NFC normalisation, then - uppercasing followed by lowercasing, then NFC again. Lowercasing alone - misses pairs that APFS and NTFS merge because their uppercase forms are - equal (`ſ` and `s`, `ς` and `σ`, `ı` and `i`); going through uppercase - joins them, and the final normalisation recomposes anything case mapping - decomposed. The key may reject a few pairs a given filesystem would keep - apart (`ß` and `ss`), which is the safe direction. Under that comparison +- Effective names are compared under a folded key: the code points HFS+ + ignores in names are dropped (zero-width joiners, directional marks and + formatting controls, and `U+FEFF`), then NFC normalisation, uppercasing, + full Unicode case folding, and NFC again. Each step covers merges the + others miss. NTFS compares uppercase forms (`ſ` and `s`, `ς` and `σ` on + older volumes). APFS applies full case folding (`ß`, `ẞ` and `ss`; `µ` and + `μ`), which lowercasing the uppercase form would miss for `ẞ`. HFS+ treats + `ab` and `ab` as one name. The final normalisation recomposes + anything case mapping decomposed. The key is checked against every BMP + character in the Windows XP and Windows 7 NTFS upcase tables and against + every cased character on a live case-insensitive APFS volume; it misses + no merge either makes. It rejects a few pairs those filesystems keep + apart (`ı` and `i`), which is the safe direction. Under that comparison no two effective names are equal, and none is a directory prefix of another: `a` and `a/b` cannot both be entries, and neither can `A` and `a/b`. From 75a481565ff5dcee26897f9465cb096a7c9678e6 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:04:59 +0200 Subject: [PATCH 02/11] fix(manifest): resolve extraction targets from held directories Extraction checked each directory by path, then created the temporary file, re-read it for the SHA-256 and renamed it by path. A process able to write inside the output root could swap a checked directory for a symlink in between and have the file written wherever the link pointed, which the ADR listed as the remaining hardening step. Every name beneath the root is now resolved from directories held open. On Unix each directory is a descriptor opened without following links and every create, check and rename is relative to it, so renaming a directory or replacing a path to it changes nothing. On Windows each directory from the root down is held without delete sharing, so none can be renamed, deleted or replaced by a junction while an entry is written; directories are opened as the reparse point itself. The rename into place is a no-replace rename (renameat2/renameatx_np, then a hard link, then a check just before the rename on filesystems with neither). The SHA-256 is computed as the download is written, through the new Client::file_download_to_writer, and the file is synced before it takes its name. Overwrite no longer replaces a file the same extraction already wrote. Two entries a filesystem treats as one name, such as a long name and its Windows 8.3 alias, now end with the second failing instead of silently replacing the first. Files are told apart by device and inode, or by volume and file id on Windows. The path-based `extract::prepare_target` is removed. The Windows code is type-checked and linted for x86_64-pc-windows-msvc; CI has no Windows test runner. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 3 +- Cargo.lock | 1 + ant-core/Cargo.toml | 5 +- ant-core/src/data/client/file/native.rs | 21 + ant-core/src/data/manifest/extract.rs | 440 ++++++----------- ant-core/src/data/manifest/mod.rs | 2 + ant-core/src/data/manifest/output/mod.rs | 454 ++++++++++++++++++ ant-core/src/data/manifest/output/unix.rs | 171 +++++++ ant-core/src/data/manifest/output/windows.rs | 205 ++++++++ docs/adr/ADR-0006-file-manifests-and-links.md | 57 ++- 10 files changed, 1054 insertions(+), 305 deletions(-) create mode 100644 ant-core/src/data/manifest/output/mod.rs create mode 100644 ant-core/src/data/manifest/output/unix.rs create mode 100644 ant-core/src/data/manifest/output/windows.rs diff --git a/CLAUDE.md b/CLAUDE.md index 0086c02d..a1fd741e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,7 +85,8 @@ ant-core/src/ │ ├── build.rs # ManifestBuilder (uploads files, embedded or compact refs) │ ├── embed.rs # Root-vs-shrunk DataMap choice for embedding │ ├── compact.rs # Plan/publish/apply compaction (embedded DataMaps -> addresses) -│ └── extract.rs # Containment-safe extraction with per-entry results +│ ├── extract.rs # Containment-safe extraction with per-entry results +│ └── output/ # Handle-relative output directories (fds on Unix, held handles on Windows) └── node/ # Node management ├── mod.rs # add_nodes, remove_node, reset ├── types.rs # DaemonConfig, DaemonStatus, NodeConfig, NodeInfo, AddNodeOpts, etc. diff --git a/Cargo.lock b/Cargo.lock index 913b3bbe..6973dd96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -880,6 +880,7 @@ dependencies = [ "rand 0.8.6", "reqwest 0.12.28", "rmp-serde", + "rustix", "saorsa-transport", "self-replace", "self_encryption", diff --git a/ant-core/Cargo.toml b/ant-core/Cargo.toml index da377117..fe687e12 100644 --- a/ant-core/Cargo.toml +++ b/ant-core/Cargo.toml @@ -127,9 +127,12 @@ libc = "0.2" # Without this, the musl CI builds fail because the runners lack musl-compatible # OpenSSL dev libraries. Not needed on Windows (uses schannel via native-tls). openssl = { version = "0.10", features = ["vendored"] } +# Directory-relative file operations for containment-safe manifest +# extraction (ADR-0006). Already in the tree through tempfile. +rustix = { version = "1.1", features = ["fs"] } [target.'cfg(windows)'.dependencies] -windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_System_Console", "Win32_System_Threading"] } +windows-sys = { version = "0.61", features = ["Win32_Foundation", "Win32_Storage_FileSystem", "Win32_System_Console", "Win32_System_Threading"] } [features] # Preserve the existing native ant-core API for ordinary consumers. WASM diff --git a/ant-core/src/data/client/file/native.rs b/ant-core/src/data/client/file/native.rs index 052b7b97..0834c238 100644 --- a/ant-core/src/data/client/file/native.rs +++ b/ant-core/src/data/client/file/native.rs @@ -3617,6 +3617,27 @@ impl Client { }) .await } + + /// Download and decrypt a file into `writer`, with optional progress + /// events. + /// + /// For callers that open the destination themselves, such as a manifest + /// extraction that resolves every path from directories it holds open. + /// Constant memory (one decrypt batch resident at a time). Nothing is + /// renamed or removed here: on an error `writer` may hold part of the + /// file, and the caller decides what to do with it. + pub async fn file_download_to_writer( + &self, + data_map: &DataMap, + writer: &mut W, + progress: Option>, + ) -> Result { + let peer_count = self.config().close_group_size; + self.download_decrypted_chunks(data_map, progress, peer_count, None, None, |bytes| { + std::future::ready(writer.write_all(&bytes).map_err(Error::from)) + }) + .await + } } #[cfg(test)] diff --git a/ant-core/src/data/manifest/extract.rs b/ant-core/src/data/manifest/extract.rs index 4a4a5194..b7a51d2e 100644 --- a/ant-core/src/data/manifest/extract.rs +++ b/ant-core/src/data/manifest/extract.rs @@ -2,16 +2,15 @@ //! //! Containment is a filesystem contract, not a string check. Inside the //! output root the extractor never follows a symlink, junction or reparse -//! point: every intermediate directory is created, or confirmed to be a -//! real directory, without following links, before the download, and the -//! whole parent chain and the target are checked again, without following -//! links, immediately before the final rename. The file is written through -//! a reserved temporary name in its parent. An existing target is a -//! per-entry failure unless overwrite was requested, and even then only a -//! regular file is replaced. - -use std::fs; -use std::io; +//! point, and every name is resolved from directories held open, so +//! nothing that changes while an entry downloads can redirect where it is +//! written (see the `output` module). The file is written through a +//! temporary name in its parent and renamed into place without replacing +//! anything. An existing target is a per-entry failure unless overwrite was +//! requested, and even then only a regular file this extraction did not +//! write is replaced. + +use std::io::{self, Write}; use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; @@ -19,7 +18,6 @@ use futures::stream::{self, StreamExt}; use self_encryption::DataMap; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; -use tempfile::Builder as TempBuilder; use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; @@ -27,16 +25,10 @@ use crate::data::client::file::DownloadEvent; use crate::data::error::Error; use crate::data::Client; +use super::output::OutputRoot; use super::path::PATH_SEPARATOR; use super::{ContentRef, Manifest, ManifestEntry, ManifestError, SHA256_LEN}; -#[cfg(all(test, unix))] -use std::os::unix::fs::symlink; - -/// Read buffer for checking a downloaded file's SHA-256. -const SHA256_READ_BUFFER: usize = 1024 * 1024; -/// Prefix of the temporary file an entry is downloaded into. -const TEMP_PREFIX: &str = ".ant-extract-"; /// Capacity of the per-entry download progress channel. const DOWNLOAD_PROGRESS_CAPACITY: usize = 64; @@ -187,23 +179,22 @@ pub async fn extract_manifest( ) -> Result { manifest.validate()?; let selected = select_entries(manifest, &options.selection)?; - fs::create_dir_all(&options.output_root)?; - let total = selected.len(); + let root = OutputRoot::open(&options.output_root)?; + let extraction = Extraction { + client, + root: &root, + options, + total: selected.len(), + }; + let extraction = &extraction; let outcomes: Vec<(usize, EntryOutcome)> = stream::iter(selected.into_iter().enumerate()) .map(|(index, (name, entry))| { let progress = progress.clone(); async move { - let status = extract_entry( - client, - entry, - &name, - index, - total, - options, - progress.as_ref(), - ) - .await; + let status = extraction + .extract_entry(entry, &name, index, progress.as_ref()) + .await; if let Some(tx) = &progress { let _ = tx .send(ExtractEvent::EntryFinished { @@ -226,125 +217,137 @@ pub async fn extract_manifest( }) } -async fn extract_entry( - client: &Client, - entry: &ManifestEntry, - name: &str, - index: usize, +/// What every entry of one extraction shares. +struct Extraction<'a> { + client: &'a Client, + root: &'a OutputRoot, + options: &'a ExtractOptions, + /// Number of selected entries. total: usize, - options: &ExtractOptions, - progress: Option<&mpsc::Sender>, -) -> EntryStatus { - if options.cancel.is_cancelled() { - return EntryStatus::Cancelled; - } - if let Some(tx) = progress { - let _ = tx - .send(ExtractEvent::EntryStarted { - name: name.to_string(), - index, - total, - }) - .await; +} + +impl Extraction<'_> { + async fn extract_entry( + &self, + entry: &ManifestEntry, + name: &str, + index: usize, + progress: Option<&mpsc::Sender>, + ) -> EntryStatus { + if self.options.cancel.is_cancelled() { + return EntryStatus::Cancelled; + } + if let Some(tx) = progress { + let _ = tx + .send(ExtractEvent::EntryStarted { + name: name.to_string(), + index, + total: self.total, + }) + .await; + } + let work = self.download_entry(entry, name, progress); + tokio::select! { + _ = self.options.cancel.cancelled() => EntryStatus::Cancelled, + result = work => match result { + Ok(bytes) => EntryStatus::Written { bytes }, + Err(e) => EntryStatus::Failed { error: e.to_string() }, + }, + } } - let work = download_entry(client, entry, name, options, progress); - tokio::select! { - _ = options.cancel.cancelled() => EntryStatus::Cancelled, - result = work => match result { - Ok(bytes) => EntryStatus::Written { bytes }, - Err(e) => EntryStatus::Failed { error: e.to_string() }, - }, + + async fn download_entry( + &self, + entry: &ManifestEntry, + name: &str, + progress: Option<&mpsc::Sender>, + ) -> Result { + let overwrite = self.options.overwrite; + let target = self.root.target(name)?; + target.check(overwrite)?; + + // The root map lists every chunk, so its size is the file's real + // size, unlike the creator's unverified `size` hint. The download + // would resolve the root anyway. + let client = self.client; + let data_map = client + .data_map_resolve_root(&resolve_data_map(client, &entry.source).await?) + .await?; + ensure_space(target.parent_path(), data_map.original_file_size())?; + + let download_progress = progress.map(|tx| { + let (dl_tx, mut dl_rx) = mpsc::channel(DOWNLOAD_PROGRESS_CAPACITY); + let tx = tx.clone(); + let name = name.to_string(); + tokio::spawn(async move { + while let Some(event) = dl_rx.recv().await { + let _ = tx + .send(ExtractEvent::Download { + name: name.clone(), + event, + }) + .await; + } + }); + dl_tx + }); + + // Dropping the staged file on any early return, including a + // cancellation, removes it. + let mut staged = target.stage()?; + let mut writer = HashingWriter::new(staged.file()?, entry.sha256.is_some()); + let bytes = client + .file_download_to_writer(&data_map, &mut writer, download_progress) + .await?; + + // Check the creator's recorded SHA-256 before the file takes its + // name, so nothing unverified is left at the target. + if let (Some(expected), Some(actual)) = (entry.sha256, writer.finish()) { + if actual != expected { + return Err(ManifestError::Sha256Mismatch { + expected: hex::encode(expected), + actual: hex::encode(actual), + }); + } + } + staged.commit(overwrite)?; + Ok(bytes) } } -async fn download_entry( - client: &Client, - entry: &ManifestEntry, - name: &str, - options: &ExtractOptions, - progress: Option<&mpsc::Sender>, -) -> Result { - let target = prepare_target(&options.output_root, name, options.overwrite)?; - let parent = target - .parent() - .ok_or_else(|| ManifestError::Build(format!("{} has no parent", target.display())))?; - - // The root map lists every chunk, so its size is the file's real size, - // unlike the creator's unverified `size` hint. The download would - // resolve the root anyway. - let data_map = client - .data_map_resolve_root(&resolve_data_map(client, &entry.source).await?) - .await?; - ensure_space(parent, data_map.original_file_size())?; - - let temp = TempBuilder::new() - .prefix(TEMP_PREFIX) - .tempfile_in(parent)? - .into_temp_path(); - - let download_progress = progress.map(|tx| { - let (dl_tx, mut dl_rx) = mpsc::channel(DOWNLOAD_PROGRESS_CAPACITY); - let tx = tx.clone(); - let name = name.to_string(); - tokio::spawn(async move { - while let Some(event) = dl_rx.recv().await { - let _ = tx - .send(ExtractEvent::Download { - name: name.clone(), - event, - }) - .await; - } - }); - dl_tx - }); - - let bytes = client - .file_download_with_progress(&data_map, &temp, download_progress) - .await?; - - // Check the creator's recorded SHA-256 before the file takes its name. - // On a mismatch the temporary file is dropped, so nothing unverified is - // left behind. - if let Some(expected) = entry.sha256 { - let path = temp.to_path_buf(); - let actual = tokio::task::spawn_blocking(move || file_sha256(&path)) - .await - .map_err(|e| ManifestError::Build(format!("hashing task failed: {e}")))??; - if actual != expected { - return Err(ManifestError::Sha256Mismatch { - expected: hex::encode(expected), - actual: hex::encode(actual), - }); +/// Writes through to a file, hashing what it writes when asked to, so a +/// download is checked in the same pass that stores it. +struct HashingWriter<'a, W> { + inner: &'a mut W, + hasher: Option, +} + +impl<'a, W: Write> HashingWriter<'a, W> { + fn new(inner: &'a mut W, hash: bool) -> Self { + Self { + inner, + hasher: hash.then(Sha256::new), } } - // Re-check the parent chain and the target immediately before the - // rename; either may have changed while the download ran. - verify_parent_chain(&options.output_root, name)?; - check_target(&target, options.overwrite)?; - let persisted = if options.overwrite { - temp.persist(&target) - } else { - temp.persist_noclobber(&target) - }; - persisted.map_err(|e| ManifestError::Io(e.error))?; - Ok(bytes) + /// The SHA-256 of everything written, when hashing was asked for. + fn finish(self) -> Option<[u8; SHA256_LEN]> { + self.hasher.map(|hasher| hasher.finalize().into()) + } } -/// SHA-256 of the file at `path`, read in one streaming pass. -fn file_sha256(path: &Path) -> io::Result<[u8; SHA256_LEN]> { - let mut file = fs::File::open(path)?; - let mut hasher = Sha256::new(); - let mut buffer = vec![0u8; SHA256_READ_BUFFER]; - loop { - let n = io::Read::read(&mut file, &mut buffer)?; - if n == 0 { - break; +impl Write for HashingWriter<'_, W> { + fn write(&mut self, buf: &[u8]) -> io::Result { + let written = self.inner.write(buf)?; + if let Some(hasher) = &mut self.hasher { + hasher.update(&buf[..written]); } - hasher.update(&buffer[..n]); + Ok(written) + } + + fn flush(&mut self) -> io::Result<()> { + self.inner.flush() } - Ok(hasher.finalize().into()) } /// Fail the entry before downloading when `dir`'s filesystem cannot hold @@ -371,115 +374,27 @@ async fn resolve_data_map(client: &Client, source: &ContentRef) -> Result Result { - let mut components = name.split(PATH_SEPARATOR).peekable(); - let mut current = root.to_path_buf(); - while let Some(component) = components.next() { - current.push(component); - if components.peek().is_none() { - check_target(¤t, overwrite)?; - return Ok(current); - } - ensure_real_directory(¤t)?; - } - Err(ManifestError::Build("empty effective name".to_string())) -} - -/// Confirm, without creating anything or following links, that every -/// directory component of `name` beneath `root` is still a real directory. -fn verify_parent_chain(root: &Path, name: &str) -> Result<(), ManifestError> { - let mut components = name.split(PATH_SEPARATOR).peekable(); - let mut current = root.to_path_buf(); - while let Some(component) = components.next() { - if components.peek().is_none() { - return Ok(()); - } - current.push(component); - let meta = fs::symlink_metadata(¤t)?; - verify_directory(¤t, &meta)?; - } - Ok(()) -} - -fn ensure_real_directory(path: &Path) -> Result<(), ManifestError> { - match fs::symlink_metadata(path) { - Ok(meta) => verify_directory(path, &meta), - Err(e) if e.kind() == io::ErrorKind::NotFound => { - match fs::create_dir(path) { - Ok(()) => {} - Err(e) if e.kind() == io::ErrorKind::AlreadyExists => {} - Err(e) => return Err(e.into()), - } - let meta = fs::symlink_metadata(path)?; - verify_directory(path, &meta) - } - Err(e) => Err(e.into()), - } -} - -fn verify_directory(path: &Path, meta: &fs::Metadata) -> Result<(), ManifestError> { - if meta.file_type().is_symlink() { - return Err(ManifestError::Build(format!( - "{} is a symlink; extraction does not follow links inside the output directory", - path.display() - ))); - } - if !meta.is_dir() { - return Err(ManifestError::Build(format!( - "{} exists and is not a directory", - path.display() - ))); - } - Ok(()) -} - -fn check_target(target: &Path, overwrite: bool) -> Result<(), ManifestError> { - match fs::symlink_metadata(target) { - Ok(meta) => { - if meta.file_type().is_symlink() { - return Err(ManifestError::Build(format!( - "{} is a symlink and will not be replaced", - target.display() - ))); - } - if !overwrite { - return Err(ManifestError::Build(format!( - "{} already exists; pass overwrite to replace it", - target.display() - ))); - } - if !meta.is_file() { - return Err(ManifestError::Build(format!( - "{} exists and is not a regular file", - target.display() - ))); - } - Ok(()) - } - Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()), - Err(e) => Err(e.into()), - } -} - #[cfg(test)] #[allow(clippy::unwrap_used, clippy::expect_used)] mod tests { use super::*; #[test] - fn file_sha256_matches_a_one_shot_digest() { - let dir = tempfile::tempdir().unwrap(); - // Spans several read buffers, with a ragged tail. - let data: Vec = (0..(2 * SHA256_READ_BUFFER + 4321)) - .map(|i| (i % 251) as u8) - .collect(); - let path = dir.path().join("f.bin"); - fs::write(&path, &data).unwrap(); + fn hashing_writer_matches_a_one_shot_digest_and_writes_everything() { + let data: Vec = (0..100_003u32).map(|i| (i % 251) as u8).collect(); + let mut out = Vec::new(); + let mut writer = HashingWriter::new(&mut out, true); + for piece in data.chunks(4096) { + writer.write_all(piece).unwrap(); + } let expected: [u8; SHA256_LEN] = Sha256::digest(&data).into(); - assert_eq!(file_sha256(&path).unwrap(), expected); + assert_eq!(writer.finish(), Some(expected)); + assert_eq!(out, data); + + let mut unhashed = Vec::new(); + let mut writer = HashingWriter::new(&mut unhashed, false); + writer.write_all(&data).unwrap(); + assert_eq!(writer.finish(), None); } fn entry(path: &str) -> ManifestEntry { @@ -517,51 +432,4 @@ mod tests { assert_eq!(trailing.len(), 2); assert!(select_entries(&manifest, &["zzz".into()]).is_err()); } - - #[test] - fn prepare_target_creates_directories_and_refuses_existing_file() { - let root = tempfile::tempdir().unwrap(); - let target = prepare_target(root.path(), "x/y/z.txt", false).unwrap(); - assert_eq!(target, root.path().join("x").join("y").join("z.txt")); - assert!(root.path().join("x/y").is_dir()); - fs::write(&target, b"old").unwrap(); - assert!(prepare_target(root.path(), "x/y/z.txt", false).is_err()); - assert!(prepare_target(root.path(), "x/y/z.txt", true).is_ok()); - // A directory where a file should go is never replaced. - assert!(prepare_target(root.path(), "x/y", true).is_err()); - // A file where a directory should go. - assert!(prepare_target(root.path(), "x/y/z.txt/deeper", false).is_err()); - } - - #[cfg(unix)] - #[test] - fn prepare_target_never_follows_symlinks_inside_root() { - let outside = tempfile::tempdir().unwrap(); - let root = tempfile::tempdir().unwrap(); - - // Pre-existing directory symlink at the first level. - symlink(outside.path(), root.path().join("safe")).unwrap(); - let err = prepare_target(root.path(), "safe/file.txt", false).unwrap_err(); - assert!(err.to_string().contains("symlink"), "{err}"); - assert!(!outside.path().join("file.txt").exists()); - - // Deeper: a real directory containing a symlinked directory. - fs::create_dir(root.path().join("real")).unwrap(); - symlink(outside.path(), root.path().join("real/link")).unwrap(); - assert!(prepare_target(root.path(), "real/link/f", false).is_err()); - - // A symlink at the target itself, with and without overwrite. - symlink(outside.path().join("victim"), root.path().join("t.txt")).unwrap(); - assert!(prepare_target(root.path(), "t.txt", false).is_err()); - assert!(prepare_target(root.path(), "t.txt", true).is_err()); - assert!(!outside.path().join("victim").exists()); - - // A directory swapped for a symlink after preparation is caught by - // the pre-rename check. - prepare_target(root.path(), "late/file", false).unwrap(); - fs::remove_dir(root.path().join("late")).unwrap(); - symlink(outside.path(), root.path().join("late")).unwrap(); - assert!(verify_parent_chain(root.path(), "late/file").is_err()); - assert!(verify_parent_chain(root.path(), "real/f").is_ok()); - } } diff --git a/ant-core/src/data/manifest/mod.rs b/ant-core/src/data/manifest/mod.rs index f7d5777f..9d46a639 100644 --- a/ant-core/src/data/manifest/mod.rs +++ b/ant-core/src/data/manifest/mod.rs @@ -46,6 +46,8 @@ pub mod extract; pub mod file; #[cfg(feature = "native")] pub mod history; +#[cfg(feature = "native")] +mod output; #[cfg(feature = "native")] pub use self::embed::embeddable_data_map; diff --git a/ant-core/src/data/manifest/output/mod.rs b/ant-core/src/data/manifest/output/mod.rs new file mode 100644 index 00000000..b4da4c9c --- /dev/null +++ b/ant-core/src/data/manifest/output/mod.rs @@ -0,0 +1,454 @@ +//! Containment for extraction: every name beneath the output root is +//! resolved from directories held open, never re-resolved from a path +//! string after it was checked. +//! +//! On Unix each directory is a file descriptor and every operation is +//! relative to one (`openat`, `mkdirat`, `renameat`), opened without +//! following links. Once a directory is open, renaming it or replacing any +//! path to it with a symlink cannot redirect a later write. +//! +//! On Windows each directory from the output root down is held open +//! without delete sharing. While those handles are open, Windows refuses to +//! rename, delete or replace any directory on the chain, so operations by +//! path stay inside it. Directories beneath the root are opened as the +//! reparse point itself, so a junction or symlink is detected, not followed. +//! +//! A file is written under a temporary name in its parent and renamed into +//! place without replacing anything. An existing name is replaced only when +//! overwrite was requested and it is a regular file this extraction did not +//! write. The last rule means two entries that a filesystem treats as one +//! name, such as a long name and its Windows 8.3 alias, never end as one +//! file: the second fails instead of silently replacing the first. + +use std::collections::HashSet; +use std::fs::{self, File}; +use std::io; +use std::path::{Path, PathBuf}; +use std::sync::{Mutex, PoisonError}; + +use super::path::PATH_SEPARATOR; +use super::ManifestError; + +#[cfg(test)] +use std::io::Write; +#[cfg(all(test, unix))] +use std::os::unix::fs::symlink; + +#[cfg(unix)] +mod unix; +#[cfg(unix)] +use unix as platform; +#[cfg(windows)] +mod windows; +#[cfg(windows)] +use windows as platform; + +use platform::{Dir, FileIdentity}; + +/// Prefix of the temporary name an entry is written under. +const TEMP_PREFIX: &str = ".ant-extract-"; +/// Temporary names tried before giving up on finding a free one. +const MAX_TEMP_ATTEMPTS: u32 = 16; + +/// What a name in a directory is, read without following a link. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum EntryKind { + File, + Directory, + Symlink, + Other, +} + +/// A name found in a directory, with the file's identity when it is a +/// regular file. +struct Entry { + kind: EntryKind, + identity: Option, +} + +/// The output root of one extraction, held open while it runs. +pub(super) struct OutputRoot { + dir: Dir, + /// Files this extraction has written, by filesystem identity. + written: Mutex>, +} + +impl OutputRoot { + /// Create `root` if it is missing and open it. The root itself may be a + /// link, since it is the caller's choice; nothing beneath it is followed. + pub(super) fn open(root: &Path) -> Result { + fs::create_dir_all(root)?; + Ok(Self { + dir: Dir::open_root(root)?, + written: Mutex::default(), + }) + } + + /// Resolve `name` beneath the root, creating each of its directories or + /// confirming it is a real one, without following links. + pub(super) fn target(&self, name: &str) -> Result, ManifestError> { + let mut components: Vec<&str> = name.split(PATH_SEPARATOR).collect(); + let file_name = components + .pop() + .filter(|component| !component.is_empty()) + .ok_or_else(|| ManifestError::Build("empty effective name".to_string()))?; + let mut chain: Vec = Vec::new(); + for component in components { + let child = chain.last().unwrap_or(&self.dir).open_child(component)?; + if !platform::HOLD_ANCESTORS { + chain.clear(); + } + chain.push(child); + } + Ok(Target { + root: self, + chain, + file_name: file_name.to_string(), + }) + } +} + +/// Where one entry is written: its parent directory, held open, and the +/// file name within it. +pub(super) struct Target<'r> { + root: &'r OutputRoot, + /// The directories below the root, or only the parent where holding + /// the parent alone keeps the chain fixed. + chain: Vec, + file_name: String, +} + +impl Target<'_> { + fn dir(&self) -> &Dir { + self.chain.last().unwrap_or(&self.root.dir) + } + + /// The target's path, for messages. + pub(super) fn path(&self) -> PathBuf { + self.dir().display().join(&self.file_name) + } + + /// The parent directory's path, for messages and free-space checks. + pub(super) fn parent_path(&self) -> &Path { + self.dir().display() + } + + /// Refuse, before any download, a target that could not be written. + pub(super) fn check(&self, overwrite: bool) -> Result<(), ManifestError> { + let written = self + .root + .written + .lock() + .unwrap_or_else(PoisonError::into_inner); + self.existing(overwrite, &written).map(|_| ()) + } + + /// Create the temporary file the entry is downloaded into. + pub(super) fn stage(&self) -> Result, ManifestError> { + let mut last_error = None; + for _ in 0..MAX_TEMP_ATTEMPTS { + let temp_name = format!("{TEMP_PREFIX}{:016x}", rand::random::()); + match self.dir().create_new(&temp_name) { + Ok(file) => { + return Ok(Staged { + target: self, + temp_name: Some(temp_name), + file: Some(file), + }) + } + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => { + last_error = Some(error); + } + Err(error) => return Err(error.into()), + } + } + Err(last_error + .unwrap_or_else(|| io::Error::from(io::ErrorKind::AlreadyExists)) + .into()) + } + + /// Whether the target exists and may be replaced. Refuses a link, any + /// existing name without overwrite, anything but a regular file, and a + /// file this extraction already wrote under a name the filesystem + /// treats as the same. + fn existing( + &self, + overwrite: bool, + written: &HashSet, + ) -> Result { + let Some(entry) = self.dir().entry(&self.file_name)? else { + return Ok(false); + }; + let path = self.path(); + match entry.kind { + EntryKind::Symlink => Err(ManifestError::Build(format!( + "{} is a symlink and will not be replaced", + path.display() + ))), + _ if !overwrite => Err(ManifestError::Build(format!( + "{} already exists; pass overwrite to replace it", + path.display() + ))), + EntryKind::File if entry.identity.is_some_and(|id| written.contains(&id)) => { + Err(ManifestError::Build(format!( + "{} is a file this extraction already wrote, under a name this \ + filesystem treats as the same; it will not be replaced", + path.display() + ))) + } + EntryKind::File => Ok(true), + EntryKind::Directory | EntryKind::Other => Err(ManifestError::Build(format!( + "{} exists and is not a regular file", + path.display() + ))), + } + } +} + +/// An entry's temporary file. Dropping it without [`Staged::commit`] +/// removes the file, so a failed or cancelled entry leaves nothing behind. +pub(super) struct Staged<'t> { + target: &'t Target<'t>, + /// The temporary name, until the file takes the target's name. + temp_name: Option, + /// Open until the commit closes it. + file: Option, +} + +impl Staged<'_> { + /// The open temporary file to write the entry into. + pub(super) fn file(&mut self) -> Result<&mut File, ManifestError> { + self.file + .as_mut() + .ok_or_else(|| ManifestError::Build("the temporary file is closed".to_string())) + } + + /// Make the file durable and give it the target's name. Commits within + /// one extraction are serialised, so the existence check, the rename + /// and recording the result happen as one step. + pub(super) fn commit(mut self, overwrite: bool) -> Result<(), ManifestError> { + if let Some(file) = self.file.take() { + file.sync_all()?; + } + let target = self.target; + let dir = target.dir(); + let temp_name = self + .temp_name + .clone() + .ok_or_else(|| ManifestError::Build("the entry was already committed".to_string()))?; + let mut written = target + .root + .written + .lock() + .unwrap_or_else(PoisonError::into_inner); + if target.existing(overwrite, &written)? { + dir.rename_replace(&temp_name, &target.file_name)?; + } else { + dir.rename_noreplace(&temp_name, &target.file_name) + .map_err(|error| match error.kind() { + io::ErrorKind::AlreadyExists => ManifestError::Build(format!( + "{} appeared while the entry downloaded; it was not replaced", + target.path().display() + )), + _ => error.into(), + })?; + } + self.temp_name = None; + // Read the identity back from the final name: on some filesystems, + // such as FAT, a rename changes it. + if let Some(identity) = dir + .entry(&target.file_name)? + .and_then(|entry| entry.identity) + { + written.insert(identity); + } + Ok(()) + } +} + +impl Drop for Staged<'_> { + fn drop(&mut self) { + // Close before removing: Windows cannot remove an open file that was + // opened without delete sharing. + drop(self.file.take()); + if let Some(temp_name) = &self.temp_name { + let _ = self.target.dir().remove_file(temp_name); + } + } +} + +/// The error for a link inside the output root. +fn not_followed(path: &Path) -> ManifestError { + ManifestError::Build(format!( + "{} is a symlink; extraction does not follow links inside the output directory", + path.display() + )) +} + +/// The error for a non-directory where a directory is needed. +fn not_a_directory(path: &Path) -> ManifestError { + ManifestError::Build(format!("{} exists and is not a directory", path.display())) +} + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::*; + + fn write_entry(root: &OutputRoot, name: &str, bytes: &[u8], overwrite: bool) { + let target = root.target(name).unwrap(); + target.check(overwrite).unwrap(); + let mut staged = target.stage().unwrap(); + staged.file().unwrap().write_all(bytes).unwrap(); + staged.commit(overwrite).unwrap(); + } + + fn temp_files(dir: &Path) -> Vec { + fs::read_dir(dir) + .unwrap() + .map(|entry| entry.unwrap().file_name().to_string_lossy().into_owned()) + .filter(|name| name.starts_with(TEMP_PREFIX)) + .collect() + } + + #[test] + fn writes_beneath_the_root_and_refuses_what_exists() { + let dir = tempfile::tempdir().unwrap(); + let root = OutputRoot::open(&dir.path().join("out")).unwrap(); + write_entry(&root, "x/y/z.txt", b"first", false); + let path = dir.path().join("out/x/y/z.txt"); + assert_eq!(fs::read(&path).unwrap(), b"first"); + + let target = root.target("x/y/z.txt").unwrap(); + assert_eq!(target.path(), path); + assert!(target.check(false).is_err()); + // This extraction wrote it, so overwrite does not replace it either. + assert!(target.check(true).is_err()); + // A directory where a file should go is never replaced. + assert!(root.target("x/y").unwrap().check(true).is_err()); + // A file where a directory should go. + assert!(root.target("x/y/z.txt/deeper").is_err()); + assert!(temp_files(&dir.path().join("out/x/y")).is_empty()); + } + + #[test] + fn overwrite_replaces_only_a_pre_existing_regular_file() { + let dir = tempfile::tempdir().unwrap(); + fs::write(dir.path().join("old.txt"), b"old").unwrap(); + let root = OutputRoot::open(dir.path()).unwrap(); + assert!(root.target("old.txt").unwrap().check(false).is_err()); + write_entry(&root, "old.txt", b"new", true); + assert_eq!(fs::read(dir.path().join("old.txt")).unwrap(), b"new"); + } + + #[test] + fn a_staged_file_that_is_not_committed_is_removed() { + let dir = tempfile::tempdir().unwrap(); + let root = OutputRoot::open(dir.path()).unwrap(); + let target = root.target("a/b.bin").unwrap(); + let mut staged = target.stage().unwrap(); + staged.file().unwrap().write_all(b"partial").unwrap(); + assert_eq!(temp_files(&dir.path().join("a")).len(), 1); + drop(staged); + assert!(temp_files(&dir.path().join("a")).is_empty()); + assert!(!dir.path().join("a/b.bin").exists()); + } + + #[test] + fn a_target_that_appears_during_the_download_is_not_replaced() { + let dir = tempfile::tempdir().unwrap(); + let root = OutputRoot::open(dir.path()).unwrap(); + let target = root.target("late.bin").unwrap(); + target.check(false).unwrap(); + let mut staged = target.stage().unwrap(); + staged.file().unwrap().write_all(b"ours").unwrap(); + fs::write(dir.path().join("late.bin"), b"theirs").unwrap(); + let error = staged.commit(false).unwrap_err(); + assert!(error.to_string().contains("already exists"), "{error}"); + assert_eq!(fs::read(dir.path().join("late.bin")).unwrap(), b"theirs"); + assert!(temp_files(dir.path()).is_empty()); + + // The rename itself refuses to replace, for a name that appears + // after the commit's own check. + fs::write(dir.path().join("mine.tmp"), b"ours").unwrap(); + let error = root + .dir + .rename_noreplace("mine.tmp", "late.bin") + .unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::AlreadyExists); + assert_eq!(fs::read(dir.path().join("late.bin")).unwrap(), b"theirs"); + } + + #[cfg(unix)] + #[test] + fn links_inside_the_root_are_never_followed() { + let outside = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().unwrap(); + let root = OutputRoot::open(dir.path()).unwrap(); + + // A pre-existing directory symlink at the first level. + symlink(outside.path(), dir.path().join("safe")).unwrap(); + let error = root.target("safe/file.txt").err().unwrap(); + assert!(error.to_string().contains("symlink"), "{error}"); + + // Deeper: a real directory containing a symlinked directory. + fs::create_dir(dir.path().join("real")).unwrap(); + symlink(outside.path(), dir.path().join("real/link")).unwrap(); + assert!(root.target("real/link/f").is_err()); + + // A symlink at the target itself, with and without overwrite. + symlink(outside.path().join("victim"), dir.path().join("t.txt")).unwrap(); + let target = root.target("t.txt").unwrap(); + assert!(target.check(false).is_err()); + assert!(target.check(true).is_err()); + assert!(!outside.path().join("victim").exists()); + assert!(fs::read_dir(outside.path()).unwrap().next().is_none()); + } + + /// The window the path-based design left open: a directory checked as + /// real is swapped for a symlink to somewhere else while the download + /// runs. The open directory is still where the file lands. + #[cfg(unix)] + #[test] + fn a_directory_swapped_for_a_symlink_mid_download_cannot_redirect_the_file() { + let outside = tempfile::tempdir().unwrap(); + let dir = tempfile::tempdir().unwrap(); + let root = OutputRoot::open(dir.path()).unwrap(); + let target = root.target("late/file.bin").unwrap(); + let mut staged = target.stage().unwrap(); + staged.file().unwrap().write_all(b"payload").unwrap(); + + fs::rename(dir.path().join("late"), dir.path().join("moved")).unwrap(); + symlink(outside.path(), dir.path().join("late")).unwrap(); + staged.commit(false).unwrap(); + + assert_eq!( + fs::read(dir.path().join("moved/file.bin")).unwrap(), + b"payload" + ); + assert!(fs::read_dir(outside.path()).unwrap().next().is_none()); + } + + /// Two names the filesystem resolves to one file, as a long name and + /// its 8.3 alias do on Windows, simulated with a hard link. + #[cfg(unix)] + #[test] + fn overwrite_never_replaces_a_file_this_extraction_wrote_under_another_name() { + let dir = tempfile::tempdir().unwrap(); + let root = OutputRoot::open(dir.path()).unwrap(); + write_entry(&root, "report final.pdf", b"first entry", true); + fs::hard_link( + dir.path().join("report final.pdf"), + dir.path().join("REPORT~1.PDF"), + ) + .unwrap(); + + let target = root.target("REPORT~1.PDF").unwrap(); + let error = target.check(true).unwrap_err(); + assert!(error.to_string().contains("already wrote"), "{error}"); + assert_eq!( + fs::read(dir.path().join("report final.pdf")).unwrap(), + b"first entry" + ); + } +} diff --git a/ant-core/src/data/manifest/output/unix.rs b/ant-core/src/data/manifest/output/unix.rs new file mode 100644 index 00000000..8c62480e --- /dev/null +++ b/ant-core/src/data/manifest/output/unix.rs @@ -0,0 +1,171 @@ +//! Unix containment: every operation is relative to a directory file +//! descriptor, and nothing beneath the root is opened through a link. + +use std::fs::File; +use std::io; +use std::os::fd::OwnedFd; +use std::path::{Path, PathBuf}; + +use rustix::fs::{ + linkat, mkdirat, open, openat, renameat, statat, unlinkat, AtFlags, FileType, Mode, OFlags, + Stat, +}; +use rustix::io::Errno; + +use super::{not_a_directory, not_followed, Entry, EntryKind}; +use crate::data::manifest::ManifestError; + +#[cfg(any(target_os = "linux", target_os = "android", target_vendor = "apple"))] +use rustix::fs::{renameat_with, RenameFlags}; + +/// A descriptor pins its directory however paths to it change, so only the +/// parent needs holding. +pub(super) const HOLD_ANCESTORS: bool = false; + +/// Errors from a no-replace rename on a filesystem that cannot do one. +#[cfg(any(target_os = "linux", target_os = "android", target_vendor = "apple"))] +const NO_ATOMIC_NOREPLACE: &[Errno] = + &[Errno::INVAL, Errno::NOSYS, Errno::NOTSUP, Errno::OPNOTSUPP]; + +/// Identity of a file: its device and inode. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub(super) struct FileIdentity { + device: u64, + inode: u64, +} + +/// A directory held open. +pub(super) struct Dir { + fd: OwnedFd, + /// Where it was found, for messages. + display: PathBuf, +} + +impl Dir { + /// Open the output root, following a link if the root is one. + pub(super) fn open_root(path: &Path) -> io::Result { + let fd = open( + path, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC, + Mode::empty(), + )?; + Ok(Self { + fd, + display: path.to_path_buf(), + }) + } + + /// Open the directory `name` in this one, creating it when missing, + /// without following a link. + pub(super) fn open_child(&self, name: &str) -> Result { + let display = self.display.join(name); + match mkdirat(&self.fd, name, new_directory_mode()) { + Ok(()) | Err(Errno::EXIST) => {} + Err(error) => return Err(io::Error::from(error).into()), + } + let flags = OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC; + match openat(&self.fd, name, flags, Mode::empty()) { + Ok(fd) => Ok(Self { fd, display }), + Err(error) => Err(match self.entry(name) { + Ok(Some(Entry { + kind: EntryKind::Symlink, + .. + })) => not_followed(&display), + Ok(Some(_)) => not_a_directory(&display), + _ => io::Error::from(error).into(), + }), + } + } + + /// What `name` is in this directory, without following a link, or + /// `None` when nothing has that name. + pub(super) fn entry(&self, name: &str) -> io::Result> { + let stat = match statat(&self.fd, name, AtFlags::SYMLINK_NOFOLLOW) { + Ok(stat) => stat, + Err(Errno::NOENT) => return Ok(None), + Err(error) => return Err(error.into()), + }; + let kind = match FileType::from_raw_mode(stat.st_mode) { + FileType::RegularFile => EntryKind::File, + FileType::Directory => EntryKind::Directory, + FileType::Symlink => EntryKind::Symlink, + _ => EntryKind::Other, + }; + Ok(Some(Entry { + kind, + identity: (kind == EntryKind::File).then(|| identity(&stat)), + })) + } + + /// Create the file `name`, failing if anything has that name, even a + /// dangling link. + pub(super) fn create_new(&self, name: &str) -> io::Result { + let flags = + OFlags::WRONLY | OFlags::CREATE | OFlags::EXCL | OFlags::NOFOLLOW | OFlags::CLOEXEC; + Ok(File::from(openat(&self.fd, name, flags, new_file_mode())?)) + } + + /// Rename `from` to `to` in this directory, failing if `to` exists. + pub(super) fn rename_noreplace(&self, from: &str, to: &str) -> io::Result<()> { + #[cfg(any(target_os = "linux", target_os = "android", target_vendor = "apple"))] + match renameat_with(&self.fd, from, &self.fd, to, RenameFlags::NOREPLACE) { + Ok(()) => return Ok(()), + Err(error) if NO_ATOMIC_NOREPLACE.contains(&error) => {} + Err(error) => return Err(error.into()), + } + // A hard link fails when the target exists. Once it succeeds the file + // is in place, so failing to drop the temporary name only leaves a + // second name for it. + match linkat(&self.fd, from, &self.fd, to, AtFlags::empty()) { + Ok(()) => { + let _ = unlinkat(&self.fd, from, AtFlags::empty()); + return Ok(()); + } + Err(Errno::EXIST) => return Err(Errno::EXIST.into()), + // No hard links either, as on FAT. + Err(_) => {} + } + // Only a writer outside this extraction can create `to` between this + // check and the rename. + if self.entry(to)?.is_some() { + return Err(io::ErrorKind::AlreadyExists.into()); + } + renameat(&self.fd, from, &self.fd, to).map_err(Into::into) + } + + /// Rename `from` over `to` in this directory. A link at `to` is + /// replaced, never followed. + pub(super) fn rename_replace(&self, from: &str, to: &str) -> io::Result<()> { + renameat(&self.fd, from, &self.fd, to).map_err(Into::into) + } + + /// Remove the file `name` from this directory. + pub(super) fn remove_file(&self, name: &str) -> io::Result<()> { + unlinkat(&self.fd, name, AtFlags::empty()).map_err(Into::into) + } + + /// Where the directory was found, for messages. + pub(super) fn display(&self) -> &Path { + &self.display + } +} + +/// `rwx` for everyone, narrowed by the umask, as `std::fs::create_dir` does. +fn new_directory_mode() -> Mode { + Mode::RWXU | Mode::RWXG | Mode::RWXO +} + +/// `rw` for everyone, narrowed by the umask, as `std::fs::File::create` does. +fn new_file_mode() -> Mode { + Mode::RUSR | Mode::WUSR | Mode::RGRP | Mode::WGRP | Mode::ROTH | Mode::WOTH +} + +// `st_dev` and `st_ino` are `u64` on Linux but other widths elsewhere, such +// as an `i32` device on Apple platforms. +#[allow(clippy::unnecessary_cast)] +fn identity(stat: &Stat) -> FileIdentity { + FileIdentity { + device: stat.st_dev as u64, + inode: stat.st_ino as u64, + } +} diff --git a/ant-core/src/data/manifest/output/windows.rs b/ant-core/src/data/manifest/output/windows.rs new file mode 100644 index 00000000..2998da71 --- /dev/null +++ b/ant-core/src/data/manifest/output/windows.rs @@ -0,0 +1,205 @@ +//! Windows containment: each directory from the output root down is held +//! open without delete sharing, so none of them can be renamed, deleted or +//! replaced while an entry is written, and operations by path stay inside +//! the chain. Directories beneath the root are opened as the reparse point +//! itself, so a junction or symlink is seen rather than followed. + +use std::fs::{self, File, OpenOptions}; +use std::io; +use std::os::windows::fs::OpenOptionsExt; +use std::os::windows::io::AsRawHandle; +use std::path::{Path, PathBuf}; + +use tempfile::TempPath; +use windows_sys::Win32::Storage::FileSystem::{ + FileIdInfo, GetFileInformationByHandle, GetFileInformationByHandleEx, + BY_HANDLE_FILE_INFORMATION, FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT, + FILE_ID_INFO, FILE_READ_ATTRIBUTES, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, +}; + +use super::{not_a_directory, not_followed, Entry, EntryKind}; +use crate::data::manifest::ManifestError; + +/// Only open handles keep a directory from being renamed, so every +/// directory on the chain is held. +pub(super) const HOLD_ANCESTORS: bool = true; + +/// Sharing for a held directory: others may read and write in it, but not +/// open it for deletion, which renaming it needs as well. +const HOLD_SHARE_MODE: u32 = FILE_SHARE_READ | FILE_SHARE_WRITE; +/// Sharing for a brief look at a file's identity, which must not get in +/// anyone's way. +const LOOK_SHARE_MODE: u32 = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; +/// Size of the 128-bit identity record, as the API takes it. +const FILE_ID_INFO_SIZE: u32 = std::mem::size_of::() as u32; + +/// Identity of a file: its volume and its id on that volume. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub(super) struct FileIdentity { + volume: u64, + file: u128, +} + +/// A directory held open. +pub(super) struct Dir { + /// Keeps the directory from being renamed, deleted or replaced. + _held: File, + /// The directory's full path, resolved when the root was opened. + path: PathBuf, + /// Where it was found, for messages. + display: PathBuf, +} + +impl Dir { + /// Open the output root. A link in the root's own path is resolved once; + /// holding the result keeps that resolution valid. + pub(super) fn open_root(path: &Path) -> io::Result { + let resolved = fs::canonicalize(path)?; + let held = hold(&resolved, FILE_FLAG_BACKUP_SEMANTICS)?; + if !held.metadata()?.is_dir() { + return Err(io::ErrorKind::NotADirectory.into()); + } + Ok(Self { + _held: held, + path: resolved, + display: path.to_path_buf(), + }) + } + + /// Open the directory `name` in this one, creating it when missing, + /// without following a link. + pub(super) fn open_child(&self, name: &str) -> Result { + let path = self.path.join(name); + let display = self.display.join(name); + match fs::create_dir(&path) { + Ok(()) => {} + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => {} + Err(error) => return Err(error.into()), + } + let held = hold( + &path, + FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, + )?; + let metadata = held.metadata()?; + if metadata.file_type().is_symlink() { + return Err(not_followed(&display)); + } + if !metadata.is_dir() { + return Err(not_a_directory(&display)); + } + Ok(Self { + _held: held, + path, + display, + }) + } + + /// What `name` is in this directory, without following a link, or + /// `None` when nothing has that name. + pub(super) fn entry(&self, name: &str) -> io::Result> { + let path = self.path.join(name); + let metadata = match fs::symlink_metadata(&path) { + Ok(metadata) => metadata, + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(error), + }; + let file_type = metadata.file_type(); + let kind = if file_type.is_symlink() { + EntryKind::Symlink + } else if file_type.is_dir() { + EntryKind::Directory + } else if file_type.is_file() { + EntryKind::File + } else { + EntryKind::Other + }; + let identity = match kind { + EntryKind::File => Some(identity_at(&path)?), + _ => None, + }; + Ok(Some(Entry { kind, identity })) + } + + /// Create the file `name`, failing if anything has that name. + pub(super) fn create_new(&self, name: &str) -> io::Result { + OpenOptions::new() + .write(true) + .create_new(true) + .custom_flags(FILE_FLAG_OPEN_REPARSE_POINT) + .open(self.path.join(name)) + } + + /// Rename `from` to `to` in this directory, failing if `to` exists. + pub(super) fn rename_noreplace(&self, from: &str, to: &str) -> io::Result<()> { + TempPath::try_from_path(self.path.join(from))? + .persist_noclobber(self.path.join(to)) + .map_err(|error| { + // The staged file removes its own temporary name. + let _ = error.path.keep(); + error.error + }) + } + + /// Rename `from` over `to` in this directory. A link at `to` is + /// replaced, never followed. + pub(super) fn rename_replace(&self, from: &str, to: &str) -> io::Result<()> { + fs::rename(self.path.join(from), self.path.join(to)) + } + + /// Remove the file `name` from this directory. + pub(super) fn remove_file(&self, name: &str) -> io::Result<()> { + fs::remove_file(self.path.join(name)) + } + + /// Where the directory was found, for messages. + pub(super) fn display(&self) -> &Path { + &self.display + } +} + +/// Open the directory at `path` and hold it against renaming and deletion. +fn hold(path: &Path, flags: u32) -> io::Result { + OpenOptions::new() + .read(true) + .share_mode(HOLD_SHARE_MODE) + .custom_flags(flags) + .open(path) +} + +/// The identity of the file at `path`, opened as itself if it is a link. +fn identity_at(path: &Path) -> io::Result { + let file = OpenOptions::new() + .access_mode(FILE_READ_ATTRIBUTES) + .share_mode(LOOK_SHARE_MODE) + .custom_flags(FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT) + .open(path)?; + let handle = file.as_raw_handle(); + let mut info = FILE_ID_INFO::default(); + // SAFETY: `handle` stays open while `file` is alive, and `info` is a + // writable FILE_ID_INFO whose size is passed alongside it. + let found = unsafe { + GetFileInformationByHandleEx( + handle, + FileIdInfo, + std::ptr::from_mut(&mut info).cast(), + FILE_ID_INFO_SIZE, + ) + }; + if found != 0 { + return Ok(FileIdentity { + volume: info.VolumeSerialNumber, + file: u128::from_le_bytes(info.FileId.Identifier), + }); + } + // Filesystems without 128-bit ids report the 64-bit index instead. + let mut basic = BY_HANDLE_FILE_INFORMATION::default(); + // SAFETY: as above, for a writable BY_HANDLE_FILE_INFORMATION. + if unsafe { GetFileInformationByHandle(handle, &mut basic) } == 0 { + return Err(io::Error::last_os_error()); + } + let index = (u64::from(basic.nFileIndexHigh) << u32::BITS) | u64::from(basic.nFileIndexLow); + Ok(FileIdentity { + volume: u64::from(basic.dwVolumeSerialNumber), + file: u128::from(index), + }) +} diff --git a/docs/adr/ADR-0006-file-manifests-and-links.md b/docs/adr/ADR-0006-file-manifests-and-links.md index 0178f66f..06a4e937 100644 --- a/docs/adr/ADR-0006-file-manifests-and-links.md +++ b/docs/adr/ADR-0006-file-manifests-and-links.md @@ -481,26 +481,45 @@ portable path rules. Any violation rejects the manifest. Containment is a filesystem contract, not a string check. Within the output root the extractor never follows a symlink or a junction, that is, anything the platform's no-follow metadata reports as a link (on Windows, the -name-surrogate reparse points): each -intermediate directory is created, or confirmed to be a real directory, by a -non-following metadata check immediately before use, and the file is -downloaded into a reserved temporary name in its parent directory and renamed -into place with no-clobber semantics. The target is checked again, without -following links, immediately before the rename. An entry whose path would -traverse a link, or whose target already exists, is a per-entry failure -unless overwrite was requested, in which case only a regular file at the -exact target is replaced, never a link. Handle-relative (resolve-beneath) -opening would close the remaining check-to-use window against a concurrent -local attacker and is a later hardening step, not part of this decision. +name-surrogate reparse points). It resolves every name from directories it +holds open, never again from a path string once checked, so nothing that +changes while an entry downloads can redirect where the entry is written, +even a concurrent local process that can write inside the output root: + +- On Unix each directory is held as a file descriptor opened without + following links, and every create, rename and check is relative to it. + Renaming a directory or replacing any path to it with a symlink does not + change the directory a descriptor refers to. +- On Windows each directory from the root down is held open without delete + sharing, which stops it from being renamed, deleted or replaced by a link + while the entry is written. Directories beneath the root are opened as the + reparse point itself, so a link is seen rather than followed. + +The output root itself may be a link, since it is the caller's choice; links +in its own path are resolved once when it is opened. Each intermediate +directory is created, or confirmed to be a real directory, as it is opened. +The file is downloaded into a temporary name in its parent and renamed into +place without replacing anything (an atomic no-replace rename where the +filesystem has one, otherwise a hard link, otherwise a check just before the +rename). An entry whose path would traverse a link, or whose target already +exists, is a per-entry failure unless overwrite was requested. Even then only +a regular file is replaced, never a link, and never a file this extraction +already wrote: two entries that a filesystem treats as one name, such as a +long name and its Windows 8.3 alias, end with the second failing rather than +silently replacing the first. Files are told apart by device and inode on +Unix and by volume and file id on Windows. Commits within one extraction are +serialised, so that check, the rename and recording the written file are +one step. Per entry: resolve the DataMap (`Public` needs one fetch), resolve it to its root (a shrunk map needs its wrapper records), and check that the target's filesystem has room for the root's exact plaintext size before downloading; too little space is a per-entry failure. Then download through the ordinary file download and write as above. When the entry records a `sha256`, the -downloaded temporary file is hashed before it takes its name; a mismatch is -a per-entry failure and the temporary file is removed, so nothing unverified -is left at the target. The recorded `size` decides nothing; tools show it +download is hashed as it is written, in the same pass, and checked before +the file takes its name; a mismatch is a per-entry failure and the temporary +file is removed, so nothing unverified is left at the target. The file is +flushed to disk before the rename. The recorded `size` decides nothing; tools show it marked as the creator's claim, and show the exact size instead wherever an embedded root map provides one. Extracted files get the platform's default permissions. @@ -611,9 +630,13 @@ initial release. every depth inside the output root causes a per-entry failure and nothing is written outside the root; an existing target fails without overwrite and is replaced only when it is a regular file; a link at the target is - never followed even with overwrite. Windows uses the same no-follow - metadata checks, but CI has no Windows unit-test runner. Junction tests - are to be added with one. + never followed even with overwrite; a directory swapped for a symlink to + elsewhere while an entry downloads cannot redirect the file; a name that + appears during the download is not replaced; overwrite never replaces a + file the same extraction wrote under another name (simulated with a hard + link); an abandoned temporary file is removed. The Windows implementation + is type-checked and linted for `x86_64-pc-windows-msvc`, but CI has no + Windows test runner. Junction tests are to be added with one. - End-to-end on a local network: build a manifest with an embedded entry, a compact entry and an entry added by address; extract it from a `.ant` file and from a manifest link; extract a selection; verify bytes and per-entry From ee8c6c70ab55eb886a8ccf536c09ee556c59d222 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:11:12 +0200 Subject: [PATCH 03/11] fix(manifest): never pay for an upload the manifest cannot record Embedded mode embeds each file's root map when it is at most 64 KiB, and a manifest is capped at 64 MiB, so about a thousand large files could fill it. The cap was only enforced when the finished manifest was encoded, after every upload had been paid for, and then the history record and the output file both failed. The builder now keeps a size budget. Preflight counts every queued file at the smallest entry it can become and refuses a build that cannot fit even so. Each file is uploaded only if the largest entry it can become, its own shrunk DataMap of at most three chunk records, would still fit; otherwise the build stops before paying for it, as a file failure that keeps the partial result. After an upload the entry is counted at its real size, and the embedded root map, or any larger reference, is used only when the manifest still fits with it. Every partial or complete result can therefore be encoded and recorded. Co-Authored-By: Claude Opus 5.5 --- ant-core/src/data/manifest/build.rs | 332 +++++++++++++++++- docs/adr/ADR-0006-file-manifests-and-links.md | 14 +- 2 files changed, 332 insertions(+), 14 deletions(-) diff --git a/ant-core/src/data/manifest/build.rs b/ant-core/src/data/manifest/build.rs index 61060446..9bbf1f63 100644 --- a/ant-core/src/data/manifest/build.rs +++ b/ant-core/src/data/manifest/build.rs @@ -8,10 +8,14 @@ use std::fs; use std::path::{Path, PathBuf}; -use self_encryption::MIN_ENCRYPTABLE_BYTES; +use self_encryption::{ChunkInfo, DataMap, MIN_ENCRYPTABLE_BYTES}; use serde::{Deserialize, Serialize}; use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; +use xor_name::XorName; + +#[cfg(test)] +use self_encryption::{shrink_data_map, MAX_CHUNK_SIZE}; use crate::data::client::file::{UploadEvent, Visibility}; use crate::data::client::merkle::PaymentMode; @@ -21,11 +25,17 @@ use super::embed::embeddable_data_map; use super::path::{check_collisions, validate_component, validate_path, PATH_SEPARATOR}; use super::{ data_map_address, ContentRef, Manifest, ManifestEntry, ManifestError, TorrentReference, - ADDRESS_LEN, MAX_MANIFEST_ENTRIES, + ADDRESS_LEN, MANIFEST_HEADER_LEN, MAX_MANIFEST_BYTES, MAX_MANIFEST_ENTRIES, SHA256_LEN, }; /// Capacity of the per-file upload progress channel. const UPLOAD_PROGRESS_CAPACITY: usize = 64; +/// Most chunks an upload's own DataMap lists: self_encryption shrinks every +/// upload's map until it lists at most three (`shrink_data_map`). +const SHRUNK_MAP_MAX_CHUNKS: usize = 3; +/// Bytes the entry array's msgpack header can grow by as entries are added: +/// small counts live in the marker byte, the largest take four more bytes. +const ENTRY_ARRAY_HEADER_GROWTH: usize = std::mem::size_of::(); /// How a locally uploaded file is referenced from the manifest. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] @@ -245,9 +255,11 @@ impl<'a> ManifestBuilder<'a> { } /// Check everything that could make the finished manifest invalid - /// before the first paid upload: file sizes, the name, and collisions - /// among all queued paths and address entries. - fn preflight(&self) -> Result<(), ManifestError> { + /// before the first paid upload: file sizes, the name, collisions among + /// all queued paths and address entries, and whether the manifest can + /// fit [`MAX_MANIFEST_BYTES`] even with every file at its smallest + /// entry. Returns the size budget the uploads are admitted against. + fn preflight(&self) -> Result { let count = self.pending.len().saturating_add(self.entries.len()); if count > MAX_MANIFEST_ENTRIES { return Err(ManifestError::TooManyEntries { @@ -269,6 +281,7 @@ impl<'a> ManifestBuilder<'a> { )); } let mut names = Vec::with_capacity(self.pending.len() + self.entries.len()); + let mut budget = SizeBudget::new(&self.options)?; for file in &self.pending { let size = fs::metadata(&file.local)?.len(); if size < MIN_ENCRYPTABLE_BYTES as u64 { @@ -278,11 +291,15 @@ impl<'a> ManifestBuilder<'a> { ))); } names.push(file.path.clone()); + budget.queue_upload(&file.path, size)?; } for entry in &self.entries { names.push(entry.effective_name()?); + budget.add(entry)?; } - check_collisions(names.iter().map(String::as_str)).map_err(ManifestError::Conflict) + check_collisions(names.iter().map(String::as_str)).map_err(ManifestError::Conflict)?; + budget.ensure_fits()?; + Ok(budget) } /// Upload every queued file and assemble the manifest. @@ -297,7 +314,7 @@ impl<'a> ManifestBuilder<'a> { mut self, progress: Option>, ) -> Result { - self.preflight()?; + let mut budget = self.preflight()?; let total = self.pending.len(); let mut files_uploaded = 0; let mut chunks_stored = 0; @@ -319,6 +336,11 @@ impl<'a> ManifestBuilder<'a> { break; } }; + // Nothing is paid for that the manifest could not record. + if let Err(error) = budget.admit_upload(index, &file.path) { + failure = Some((file.path, error)); + break; + } if let Some(tx) = &progress { let _ = tx .send(BuildEvent::FileStarted { @@ -369,17 +391,26 @@ impl<'a> ManifestBuilder<'a> { // Keep the upload's usable DataMap before any fallible cost // parsing or reference optimisation. A later error must not // discard the file that has just been paid for and uploaded. - self.entries.push(ManifestEntry { + let uploaded = ManifestEntry { path: Some(file.path.clone()), size: Some(size), sha256: Some(result.sha256), source: ContentRef::Embedded { data_map: result.data_map.clone(), }, - }); + }; + let counted = budget.uploaded(index, &uploaded); + self.entries.push(uploaded); files_uploaded += 1; chunks_stored += result.chunks_stored; gas_cost_wei = gas_cost_wei.saturating_add(result.gas_cost_wei); + let counted = match counted { + Ok(counted) => counted, + Err(error) => { + failure = Some((file.path, error)); + break; + } + }; match parse_atto(&result.storage_cost_atto) { Ok(cost) => storage_cost_atto = storage_cost_atto.saturating_add(cost), Err(error) => { @@ -398,11 +429,17 @@ impl<'a> ManifestBuilder<'a> { break; } }; - let reference = source.kind(); - // The entry just appended is always present at this point. - if let Some(entry) = self.entries.last_mut() { - entry.source = source; + // The entry just appended is always present at this point. A + // larger reference, such as an embedded root map, replaces the + // uploaded map only if the manifest still fits with it. + let Some(entry) = self.entries.last_mut() else { + break; + }; + if let Err(error) = budget.offer_source(entry, counted, source) { + failure = Some((file.path, error)); + break; } + let reference = entry.source.kind(); if let Some(tx) = &progress { let _ = tx @@ -511,6 +548,161 @@ fn file_name_utf8(path: &Path) -> Result { .ok_or_else(|| ManifestError::Build(format!("{} has no UTF-8 file name", path.display()))) } +/// Keeps a build's manifest within [`MAX_MANIFEST_BYTES`] however its +/// uploads turn out. +/// +/// Each queued file is counted at the smallest entry it can become, and a +/// file is uploaded only if the largest entry it can become would still +/// fit. Once uploaded it is counted at its real size, and a larger +/// reference, such as an embedded root map, is taken only if it fits as +/// well. Every partial or complete result can therefore be encoded and +/// recorded, and nothing is paid for that could not be. +struct SizeBudget { + /// Encoded bytes the manifest needs at least, as counted so far. + reserved: usize, + /// Each queued file's smallest entry, in queue order. + smallest: Vec, +} + +impl SizeBudget { + /// The fixed part of a manifest with these options and no entries. + fn new(options: &BuildOptions) -> Result { + let empty = Manifest { + name: options.name.clone(), + torrent: options.torrent.clone(), + entries: Vec::new(), + }; + Ok(Self { + reserved: MANIFEST_HEADER_LEN + encoded_len(&empty)? + ENTRY_ARRAY_HEADER_GROWTH, + smallest: Vec::new(), + }) + } + + /// Count a queued file at the smallest entry it can become: a public + /// address. + fn queue_upload(&mut self, path: &str, size: u64) -> Result<(), ManifestError> { + let smallest = encoded_len(&upload_entry( + path, + size, + ContentRef::Public { + address: [0; ADDRESS_LEN], + }, + ))?; + self.smallest.push(smallest); + self.reserved = self.reserved.saturating_add(smallest); + Ok(()) + } + + /// Count an entry added without uploading. + fn add(&mut self, entry: &ManifestEntry) -> Result<(), ManifestError> { + self.reserved = self.reserved.saturating_add(encoded_len(entry)?); + Ok(()) + } + + /// Refuse a manifest that cannot fit even with every file at its + /// smallest entry. + fn ensure_fits(&self) -> Result<(), ManifestError> { + fits(self.reserved) + } + + /// Refuse to upload queued file `index` unless the largest entry it can + /// become would still fit. + fn admit_upload(&self, index: usize, path: &str) -> Result<(), ManifestError> { + let largest = encoded_len(&upload_entry( + path, + u64::MAX, + ContentRef::Embedded { + data_map: largest_upload_map(), + }, + ))?; + fits(self.swapped(self.smallest_of(index), largest)) + } + + /// Count queued file `index` at its uploaded entry from now on, and + /// return that entry's size. + fn uploaded(&mut self, index: usize, entry: &ManifestEntry) -> Result { + let len = encoded_len(entry)?; + self.reserved = self.swapped(self.smallest_of(index), len); + Ok(len) + } + + /// Give `entry`, counted at `current` bytes, the reference `source` when + /// the manifest still fits with it; otherwise it keeps the one it has. + fn offer_source( + &mut self, + entry: &mut ManifestEntry, + current: usize, + source: ContentRef, + ) -> Result<(), ManifestError> { + let previous = std::mem::replace(&mut entry.source, source); + let offered = match encoded_len(entry) { + Ok(len) => len, + Err(error) => { + entry.source = previous; + return Err(error); + } + }; + let projected = self.swapped(current, offered); + if projected <= MAX_MANIFEST_BYTES { + self.reserved = projected; + } else { + entry.source = previous; + } + Ok(()) + } + + fn smallest_of(&self, index: usize) -> usize { + self.smallest.get(index).copied().unwrap_or_default() + } + + /// The reserved total with `old` bytes replaced by `new`. + fn swapped(&self, old: usize, new: usize) -> usize { + self.reserved.saturating_sub(old).saturating_add(new) + } +} + +/// Refuse a manifest of `len` encoded bytes when it exceeds the limit. +fn fits(len: usize) -> Result<(), ManifestError> { + if len > MAX_MANIFEST_BYTES { + return Err(ManifestError::TooLarge { + len, + max: MAX_MANIFEST_BYTES, + }); + } + Ok(()) +} + +/// An uploaded file's entry with the given size and reference. +fn upload_entry(path: &str, size: u64, source: ContentRef) -> ManifestEntry { + ManifestEntry { + path: Some(path.to_string()), + size: Some(size), + sha256: Some([0; SHA256_LEN]), + source, + } +} + +/// The largest DataMap an upload can produce, as the manifest encodes it: +/// the most chunks a shrunk map lists, at the deepest child level. +fn largest_upload_map() -> DataMap { + let infos = (0..SHRUNK_MAP_MAX_CHUNKS) + .map(|index| ChunkInfo { + index, + dst_hash: XorName::default(), + src_hash: XorName::default(), + src_size: 0, + }) + .collect(); + DataMap::with_child(infos, usize::MAX) +} + +/// Bytes `value` takes in a manifest's msgpack encoding. +fn encoded_len(value: &T) -> Result { + rmp_serde::to_vec_named(value) + .map(|bytes| bytes.len()) + .map_err(|e| ManifestError::Encode(e.to_string())) +} + fn parse_atto(value: &str) -> Result { value .parse::() @@ -522,6 +714,120 @@ fn parse_atto(value: &str) -> Result { mod tests { use super::*; + fn embedded(data_map: DataMap) -> ContentRef { + ContentRef::Embedded { data_map } + } + + fn root_of(chunks: usize) -> DataMap { + DataMap::new( + (0..chunks) + .map(|index| ChunkInfo { + index, + dst_hash: XorName::from_content(&index.to_le_bytes()), + src_hash: XorName::from_content(&index.to_be_bytes()), + src_size: MAX_CHUNK_SIZE, + }) + .collect(), + ) + } + + #[test] + fn the_largest_upload_map_bounds_a_real_shrunk_map() { + let (shrunk, _) = shrink_data_map(root_of(1_000), |_, _| Ok(())).unwrap(); + assert!(shrunk.is_child()); + assert!(shrunk.len() <= SHRUNK_MAP_MAX_CHUNKS); + let real = encoded_len(&upload_entry("dir/f.bin", 12_345, embedded(shrunk))).unwrap(); + let bound = encoded_len(&upload_entry( + "dir/f.bin", + u64::MAX, + embedded(largest_upload_map()), + )) + .unwrap(); + assert!(real <= bound, "{real} > {bound}"); + } + + #[test] + fn the_fixed_part_counts_everything_but_the_entries() { + let options = BuildOptions { + name: Some("photos".into()), + ..BuildOptions::default() + }; + let mut budget = SizeBudget::new(&options).unwrap(); + let entry = upload_entry("a.bin", 4_096, embedded(root_of(3))); + budget.add(&entry).unwrap(); + let manifest = Manifest { + name: options.name, + torrent: None, + entries: vec![entry], + }; + let actual = manifest.encode().unwrap().len(); + assert!(actual <= budget.reserved, "{actual} > {}", budget.reserved); + assert!(budget.reserved - actual <= ENTRY_ARRAY_HEADER_GROWTH); + } + + #[test] + fn an_upload_is_admitted_only_when_its_largest_entry_fits() { + let mut budget = SizeBudget::new(&BuildOptions::default()).unwrap(); + budget.queue_upload("a.bin", 4_096).unwrap(); + budget.ensure_fits().unwrap(); + budget.admit_upload(0, "a.bin").unwrap(); + + let largest = encoded_len(&upload_entry( + "a.bin", + u64::MAX, + embedded(largest_upload_map()), + )) + .unwrap(); + // One byte less headroom than the largest entry needs. + budget.reserved = MAX_MANIFEST_BYTES - largest + budget.smallest[0] + 1; + assert!(matches!( + budget.admit_upload(0, "a.bin"), + Err(ManifestError::TooLarge { .. }) + )); + + budget.reserved = MAX_MANIFEST_BYTES + 1; + assert!(matches!( + budget.ensure_fits(), + Err(ManifestError::TooLarge { .. }) + )); + } + + #[test] + fn a_larger_reference_is_taken_only_when_it_fits() { + let mut budget = SizeBudget::new(&BuildOptions::default()).unwrap(); + budget.queue_upload("a.bin", 4_096).unwrap(); + let mut entry = upload_entry("a.bin", 4_096, embedded(largest_upload_map())); + let counted = budget.uploaded(0, &entry).unwrap(); + + // With room, a large root map replaces the uploaded map. + budget + .offer_source(&mut entry, counted, embedded(root_of(100))) + .unwrap(); + assert!( + matches!(&entry.source, ContentRef::Embedded { data_map } if data_map.len() == 100) + ); + let with_root = encoded_len(&entry).unwrap(); + + // Full to the limit: a still larger map is refused and the entry + // keeps the one it has; a smaller reference is always taken. + budget.reserved = MAX_MANIFEST_BYTES; + budget + .offer_source(&mut entry, with_root, embedded(root_of(200))) + .unwrap(); + assert!( + matches!(&entry.source, ContentRef::Embedded { data_map } if data_map.len() == 100) + ); + assert_eq!(budget.reserved, MAX_MANIFEST_BYTES); + let public = ContentRef::Public { + address: [7; ADDRESS_LEN], + }; + budget + .offer_source(&mut entry, with_root, public.clone()) + .unwrap(); + assert_eq!(entry.source, public); + assert!(budget.reserved < MAX_MANIFEST_BYTES); + } + #[test] fn file_name_requires_utf8_and_a_name() { assert_eq!(file_name_utf8(Path::new("a/b.txt")).unwrap(), "b.txt"); diff --git a/docs/adr/ADR-0006-file-manifests-and-links.md b/docs/adr/ADR-0006-file-manifests-and-links.md index 06a4e937..8920f3cf 100644 --- a/docs/adr/ADR-0006-file-manifests-and-links.md +++ b/docs/adr/ADR-0006-file-manifests-and-links.md @@ -409,7 +409,19 @@ Two reference modes for uploaded files: Everything that could make the finished manifest invalid, such as file sizes, the name and path collisions, is checked before the first paid upload, and -the output location is checked before it too. Cancellation stops between +the output location is checked before it too. + +The finished manifest must also fit `MAX_MANIFEST_BYTES`, which a few +thousand embedded root maps can exceed. Before the first upload every queued +file is counted at the smallest entry it can become (a public address with +its path, size and hash), and a build that cannot fit even so is refused. A +file is uploaded only when the largest entry it can become, its own shrunk +DataMap of at most three chunk records, would still fit; otherwise the build +stops before paying for it, as a file failure that keeps the partial result. +After an upload the entry is counted at its real size, and a larger +reference such as an embedded root map is used only when the manifest still +fits with it; the shrunk map is kept otherwise. Every partial or complete +result can therefore be encoded and recorded. Cancellation stops between files or abandons the upload in flight and returns the partial manifest, which the client records in the upload history so paid uploads are not lost. Ordinary file failures also preserve the partial result in From e31cb8af9d30a8354ed85f62cf3ff622113a9fe1 Mon Sep 17 00:00:00 2001 From: Mick van Dijke <12992260+mickvandijke@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:26:10 +0200 Subject: [PATCH 04/11] fix(manifest): journal each paid upload so a crash cannot lose it A manifest build held every completed upload in memory until it ended. Cancellation and file failures returned the partial result for the CLI to record, but a crash, a kill, a power cut or the second Ctrl-C lost every DataMap uploaded so far, and for a private upload that DataMap exists nowhere else. With BuildOptions::history set, the builder now creates .ant.partial in the upload history before the first paid upload and flushes each completed upload's entry to it before paying for the next; a finalised reference is recorded again and the later record wins. The journal is length-prefixed, CRC-32-checked frames, so reading stops cleanly where a write was cut short. When the build ends, complete, cancelled or failed, the builder writes the ordinary record under the same id and removes the journal, and returns the id in BuildResult::history_id. A build that uploaded nothing leaves no record, and one that cannot create its journal stops before paying for anything. A journal left behind is listed as an interrupted upload holding every entry that reached the disk, and its id works with show, link, export and download. While its build runs the journal is locked and listed as in progress. Records and journals never share an id. `ant manifest list` marks both states and its JSON gains `state`. The CLI no longer records builds itself: it passes the history directory and reports the builder's record id. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 6 +- Cargo.lock | 1 + ant-cli/src/commands/data/manifest.rs | 63 ++- ant-core/Cargo.toml | 2 + ant-core/src/data/manifest/build.rs | 110 ++++- ant-core/src/data/manifest/history.rs | 207 ++++++-- ant-core/src/data/manifest/journal.rs | 449 ++++++++++++++++++ ant-core/src/data/manifest/mod.rs | 2 + ant-core/src/data/mod.rs | 2 +- ant-core/tests/e2e_manifest.rs | 75 ++- docs/adr/ADR-0006-file-manifests-and-links.md | 54 ++- 11 files changed, 873 insertions(+), 98 deletions(-) create mode 100644 ant-core/src/data/manifest/journal.rs diff --git a/CLAUDE.md b/CLAUDE.md index a1fd741e..bf962dd0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -81,6 +81,7 @@ ant-core/src/ │ ├── link.rs # ant://
and ant://manifest/ links │ ├── file.rs # .ant file read/write │ ├── history.rs # Upload history: one .ant per upload under /uploads +│ ├── journal.rs # Crash-safe journal of a running build (.ant.partial) │ ├── wire.rs # Manifest-owned DataMap layout; pre-decode structure check │ ├── build.rs # ManifestBuilder (uploads files, embedded or compact refs) │ ├── embed.rs # Root-vs-shrunk DataMap choice for embedding @@ -163,7 +164,10 @@ cargo run --bin ant -- --help # Run the CLI a plain file link. Every upload (`ant file upload`, `ant manifest create`) is also recorded as a manifest under `/uploads/`; `ant manifest list` shows them and `show`/`link`/`export`/`download` accept a record id. Entry paths obey the portable rules in `data/manifest/path.rs` at build, - decode and extract time, and extraction never follows symlinks inside the output directory. + decode and extract time, and extraction never follows symlinks inside the output directory + and resolves every name from directories it holds open (`data/manifest/output/`). A manifest + build journals each completed upload to `.ant.partial` before paying for the next, so a + crash loses no paid upload; leftover journals list as interrupted records. See `docs/adr/ADR-0006-file-manifests-and-links.md`. - **Log forwarding is opt-in and node-logging-dependent**: `ant node logs forward enable` is the consent act. It only forwards nodes whose `NodeConfig.log_dir` is `Some` — node file logging is diff --git a/Cargo.lock b/Cargo.lock index 6973dd96..3009910b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -863,6 +863,7 @@ dependencies = [ "blake3", "bytes", "console_error_panic_hook", + "crc32fast", "fips204", "flate2", "fs2", diff --git a/ant-cli/src/commands/data/manifest.rs b/ant-cli/src/commands/data/manifest.rs index 1d103d57..4c593d48 100644 --- a/ant-cli/src/commands/data/manifest.rs +++ b/ant-cli/src/commands/data/manifest.rs @@ -19,8 +19,8 @@ use ant_core::data::{ manifest_link_bytes, manifest_link_from_bytes, parse_link, plan_compaction, publish_data_maps, read_manifest_file, record_upload, write_manifest_file, BuildEvent, BuildOptions, BuildResult, Client, ContentRef, DownloadEvent, EntryStatus, ExtractEvent, ExtractOptions, Link, Manifest, - ManifestBuilder, ManifestError, PaymentMode, ReferenceMode, TorrentReference, UploadEvent, - UploadRecord, Visibility, MANIFEST_LINK_RECOMMENDED_MAX_BYTES, + ManifestBuilder, ManifestError, PaymentMode, RecordState, ReferenceMode, TorrentReference, + UploadEvent, UploadRecord, Visibility, MANIFEST_LINK_RECOMMENDED_MAX_BYTES, }; use clap::Subcommand; use serde_json::json; @@ -491,6 +491,9 @@ async fn create(client: &Client, args: CreateArgs, json: bool) -> anyhow::Result payment_mode: args.payment_mode, follow_symlinks: args.follow_symlinks, cancel: CancellationToken::new(), + // Each completed upload is journaled here before the next is paid + // for; a build that cannot record itself stops before paying. + history: Some(default_history_dir()?), }; // Fail before any paid upload if the manifest could not be written. let out = resolve_output(options.name.as_deref(), args.output.clone())?; @@ -547,8 +550,7 @@ async fn create(client: &Client, args: CreateArgs, json: bool) -> anyhow::Result result }; - let (result, history_id) = - record_build_result(built, |manifest| record_upload_manifest(manifest, None))?; + let (result, history_id) = build_outcome(built)?; write_manifest(&result.manifest, &out, args.overwrite)?; if !json { eprintln!( @@ -571,10 +573,11 @@ async fn create(client: &Client, args: CreateArgs, json: bool) -> anyhow::Result ) } -/// Persist completed uploads before reporting cancellation or failure. -fn record_build_result( +/// Turn a build's outcome into its result and record id, or an error that +/// says where the completed uploads are recorded. The builder has already +/// recorded them in the history, complete or partial. +fn build_outcome( built: Result, - record: impl FnOnce(&Manifest) -> Option, ) -> anyhow::Result<(BuildResult, Option)> { let result = match built { Ok(result) => result, @@ -583,24 +586,35 @@ fn record_build_result( cause, partial, }) => { - let history_id = record(&partial.manifest); + warn_unfinished_record(&partial); anyhow::bail!( "manifest build failed at {path:?} after {} of {} file(s): {cause}{}", partial.files_uploaded, partial.total_to_upload, - partial_record_message(history_id.as_deref()), + partial_record_message(partial.history_id.as_deref()), ); } Err(error) => return Err(error.into()), }; - - let history_id = record(&result.manifest); + warn_unfinished_record(&result); + let history_id = result.history_id.clone(); if result.cancelled { return Err(cancelled_build_error(&result, history_id.as_deref())); } Ok((result, history_id)) } +/// Say when a build's record could not be written. Its journal stays in the +/// history with every completed upload, listed as interrupted. +fn warn_unfinished_record(result: &BuildResult) { + if let (Some(id), Some(error)) = (&result.history_id, &result.history_error) { + eprintln!( + "warning: the record of upload {id} could not be written ({error}); \ + it stays in the history as interrupted, with every completed file" + ); + } +} + /// The output path: the one given, or `.ant` in the current directory. fn resolve_output(name: Option<&str>, output: Option) -> anyhow::Result { if let Some(name) = name { @@ -759,6 +773,7 @@ fn list_history(json: bool) -> anyhow::Result<()> { "name": r.manifest.name, "entries": r.manifest.entries.len(), "total_size": r.manifest.total_size(), + "state": r.state, "file": r.path.display().to_string(), }) }) @@ -803,8 +818,13 @@ fn print_history_row(record: &UploadRecord) { // Drop the trailing " UTC"; the header already says so. let stamp = format_timestamp(record.recorded_at); let stamp = stamp.strip_suffix(" UTC").unwrap_or(&stamp); + let state = match record.state { + RecordState::Complete => "", + RecordState::InProgress => " (in progress)", + RecordState::Interrupted => " (interrupted: the files listed were uploaded)", + }; println!( - "{:<23} {:>7} {:>12} {}", + "{:<23} {:>7} {:>12} {}{state}", stamp, record.manifest.entries.len(), size, @@ -1215,6 +1235,8 @@ mod tests { chunks_stored: 3, storage_cost_atto: 0, gas_cost_wei: 0, + history_id: Some("record-id".into()), + history_error: None, }; for index in 0..3 { result.manifest.entries.push(ant_core::data::ManifestEntry { @@ -1233,7 +1255,7 @@ mod tests { } #[test] - fn build_failure_and_cancellation_are_recorded_before_returning_an_error() { + fn build_failure_and_cancellation_report_where_the_build_was_recorded() { let manifest = Manifest { name: Some("partial".into()), torrent: None, @@ -1254,6 +1276,8 @@ mod tests { chunks_stored: 3, storage_cost_atto: 12, gas_cost_wei: 34, + history_id: Some("20261019T120000Z-partial".into()), + history_error: None, }; let built = if cancelled { Ok(partial) @@ -1264,15 +1288,10 @@ mod tests { partial: Box::new(partial), }) }; - let history = tempfile::tempdir().unwrap(); - let error = record_build_result(built, |manifest| { - Some(record_upload(history.path(), manifest, None).unwrap().id) - }) - .unwrap_err(); - let listing = list_uploads(history.path()).unwrap(); - assert_eq!(listing.records.len(), 1); - assert_eq!(listing.records[0].manifest, manifest); - assert!(error.to_string().contains(&listing.records[0].id)); + let error = build_outcome(built).unwrap_err(); + assert!(error + .to_string() + .contains("the partial manifest is recorded as 20261019T120000Z-partial")); assert!(error.to_string().contains("1 of 2 file(s)")); if !cancelled { assert!(error.to_string().contains("later.bin")); diff --git a/ant-core/Cargo.toml b/ant-core/Cargo.toml index fe687e12..ad796145 100644 --- a/ant-core/Cargo.toml +++ b/ant-core/Cargo.toml @@ -20,6 +20,8 @@ blake3 = "1" base64 = "0.22" bytes = "1" hex = "0.4" +# Checksums for the frames of an upload journal (ADR-0006). +crc32fast = "1.5" rmp-serde = "1" serde = { version = "1", features = ["derive"] } serde_bytes = "0.11" diff --git a/ant-core/src/data/manifest/build.rs b/ant-core/src/data/manifest/build.rs index 9bbf1f63..c74cdff8 100644 --- a/ant-core/src/data/manifest/build.rs +++ b/ant-core/src/data/manifest/build.rs @@ -22,6 +22,7 @@ use crate::data::client::merkle::PaymentMode; use crate::data::Client; use super::embed::embeddable_data_map; +use super::journal::UploadJournal; use super::path::{check_collisions, validate_component, validate_path, PATH_SEPARATOR}; use super::{ data_map_address, ContentRef, Manifest, ManifestEntry, ManifestError, TorrentReference, @@ -70,6 +71,13 @@ pub struct BuildOptions { /// Cancels the build between files, or during an upload. Files already /// uploaded are kept in the partial result. pub cancel: CancellationToken, + /// Upload history directory to record the build in. A journal is + /// created there before the first paid upload and each completed upload + /// is flushed to it before the next is paid for, so a crash or a forced + /// quit loses none of them. When the build ends, complete or not, its + /// manifest becomes the record. A build that cannot create the journal + /// stops before paying for anything. + pub history: Option, } /// Progress during [`ManifestBuilder::finish`]. @@ -124,6 +132,15 @@ pub struct BuildResult { pub storage_cost_atto: u128, /// Gas paid across all uploads, in wei. pub gas_cost_wei: u128, + /// The upload history record of this build, when it was recorded. A + /// build that uploaded nothing before it stopped is not recorded. + #[serde(default)] + pub history_id: Option, + /// Why the record could not be written, when it could not. The + /// journal then stays in the history, listed as interrupted, with every + /// entry the build completed. + #[serde(default)] + pub history_error: Option, } struct PendingFile { @@ -315,6 +332,7 @@ impl<'a> ManifestBuilder<'a> { progress: Option>, ) -> Result { let mut budget = self.preflight()?; + let mut journal = self.start_journal()?; let total = self.pending.len(); let mut files_uploaded = 0; let mut chunks_stored = 0; @@ -400,10 +418,20 @@ impl<'a> ManifestBuilder<'a> { }, }; let counted = budget.uploaded(index, &uploaded); + // On disk before anything else can fail or be paid for. If it + // cannot be, the build stops rather than pay for uploads a crash + // would lose. + let journaled = journal + .as_mut() + .map_or(Ok(()), |journal| journal.append(&uploaded)); self.entries.push(uploaded); files_uploaded += 1; chunks_stored += result.chunks_stored; gas_cost_wei = gas_cost_wei.saturating_add(result.gas_cost_wei); + if let Err(error) = journaled { + failure = Some((file.path, error)); + break; + } let counted = match counted { Ok(counted) => counted, Err(error) => { @@ -435,9 +463,20 @@ impl<'a> ManifestBuilder<'a> { let Some(entry) = self.entries.last_mut() else { break; }; - if let Err(error) = budget.offer_source(entry, counted, source) { - failure = Some((file.path, error)); - break; + let changed = match budget.offer_source(entry, counted, source) { + Ok(changed) => changed, + Err(error) => { + failure = Some((file.path, error)); + break; + } + }; + // Record the final reference too; the uploaded form is already + // safe, so a failure here only stops the build. + if let (true, Some(journal)) = (changed, journal.as_mut()) { + if let Err(error) = journal.append(entry) { + failure = Some((file.path, error)); + break; + } } let reference = entry.source.kind(); @@ -457,8 +496,26 @@ impl<'a> ManifestBuilder<'a> { torrent: self.options.torrent.clone(), entries: std::mem::take(&mut self.entries), }; + // On an error here the journal stays, with every completed upload. manifest.canonicalize()?; + let stopped_early = cancelled || failure.is_some(); + let (history_id, history_error) = match journal { + None => (None, None), + // Nothing was paid for, so there is nothing to keep. + Some(journal) if stopped_early && files_uploaded == 0 => { + let _ = journal.discard(); + (None, None) + } + Some(journal) => { + let id = journal.id().to_string(); + match journal.finish(&manifest) { + Ok(record) => (Some(record.id), None), + Err(error) => (Some(id), Some(error.to_string())), + } + } + }; + let result = BuildResult { manifest, cancelled, @@ -468,6 +525,8 @@ impl<'a> ManifestBuilder<'a> { chunks_stored, storage_cost_atto, gas_cost_wei, + history_id, + history_error, }; match failure { Some((path, cause)) => Err(ManifestError::BuildFailed { @@ -479,6 +538,26 @@ impl<'a> ManifestBuilder<'a> { } } + /// Create the build's journal, when it is recorded, holding the entries + /// added without uploading. Runs before the first paid upload. + fn start_journal(&self) -> Result, ManifestError> { + let Some(dir) = &self.options.history else { + return Ok(None); + }; + let mut journal = UploadJournal::create( + dir, + self.options.name.as_deref(), + self.options.torrent.as_ref(), + )?; + for entry in &self.entries { + if let Err(error) = journal.append(entry) { + let _ = journal.discard(); + return Err(error); + } + } + Ok(Some(journal)) + } + /// Embedded mode always embeds a DataMap, even when the upload was /// public, and embeds the root map when it is small enough so readers /// skip the wrapper-record fetches. Compact mode records the public @@ -628,12 +707,13 @@ impl SizeBudget { /// Give `entry`, counted at `current` bytes, the reference `source` when /// the manifest still fits with it; otherwise it keeps the one it has. + /// Returns whether the reference changed. fn offer_source( &mut self, entry: &mut ManifestEntry, current: usize, source: ContentRef, - ) -> Result<(), ManifestError> { + ) -> Result { let previous = std::mem::replace(&mut entry.source, source); let offered = match encoded_len(entry) { Ok(len) => len, @@ -645,10 +725,11 @@ impl SizeBudget { let projected = self.swapped(current, offered); if projected <= MAX_MANIFEST_BYTES { self.reserved = projected; + Ok(entry.source != previous) } else { entry.source = previous; + Ok(false) } - Ok(()) } fn smallest_of(&self, index: usize) -> usize { @@ -800,9 +881,9 @@ mod tests { let counted = budget.uploaded(0, &entry).unwrap(); // With room, a large root map replaces the uploaded map. - budget + assert!(budget .offer_source(&mut entry, counted, embedded(root_of(100))) - .unwrap(); + .unwrap()); assert!( matches!(&entry.source, ContentRef::Embedded { data_map } if data_map.len() == 100) ); @@ -811,9 +892,9 @@ mod tests { // Full to the limit: a still larger map is refused and the entry // keeps the one it has; a smaller reference is always taken. budget.reserved = MAX_MANIFEST_BYTES; - budget + assert!(!budget .offer_source(&mut entry, with_root, embedded(root_of(200))) - .unwrap(); + .unwrap()); assert!( matches!(&entry.source, ContentRef::Embedded { data_map } if data_map.len() == 100) ); @@ -821,9 +902,16 @@ mod tests { let public = ContentRef::Public { address: [7; ADDRESS_LEN], }; - budget + assert!(budget .offer_source(&mut entry, with_root, public.clone()) - .unwrap(); + .unwrap()); + // Offering the same reference again changes nothing. + let now = budget.reserved; + let current = encoded_len(&entry).unwrap(); + assert!(!budget + .offer_source(&mut entry, current, public.clone()) + .unwrap()); + assert_eq!(budget.reserved, now); assert_eq!(entry.source, public); assert!(budget.reserved < MAX_MANIFEST_BYTES); } diff --git a/ant-core/src/data/manifest/history.rs b/ant-core/src/data/manifest/history.rs index a8e5f0c1..dc91a16a 100644 --- a/ant-core/src/data/manifest/history.rs +++ b/ant-core/src/data/manifest/history.rs @@ -8,8 +8,11 @@ //! When an id is already taken, which happens when uploads with the same label //! finish within one second, `-2`, `-3` and so on are appended. Records list //! newest first, and within one second by that suffix compared as a number. -//! Nothing else is stored, so the history directory is just a folder of -//! manifests that any manifest tool can read. +//! Apart from the journal of a manifest build that has not finished +//! (`.ant.partial`, see the `journal` module), nothing else is stored, so +//! the history directory is just a folder of manifests that any manifest +//! tool can read. An unfinished build's journal is listed as a record in +//! progress or interrupted. use std::fs; use std::path::{Path, PathBuf}; @@ -18,6 +21,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use serde::{Deserialize, Serialize}; use super::file::{read_manifest_file, write_manifest_file}; +use super::journal::{journal_path, read_journal, JOURNAL_SUFFIX}; use super::{Manifest, ManifestError, MANIFEST_EXTENSION}; #[cfg(test)] @@ -78,14 +82,34 @@ const FEBRUARY: u32 = 2; pub struct UploadRecord { /// Filename stem: `-