From 6f71aa4a9e8e563cce9a852491d9df98a49f4acc Mon Sep 17 00:00:00 2001 From: Drewry Pope Date: Mon, 2 Nov 2026 16:20:26 -0600 Subject: [PATCH 1/4] fix(id/currency_poc): one filesystem identity contract for FileIntentStore (#4578) Identity is the conversion_id in the record content; the canonical file {id:032x}.json is the only path the store writes. get(), unresolved(), and put() now share one directory scan grouped by content ID: - duplicate copies of one record collapse to one record, so recovery can never reconcile one conversion twice; - a record stored only under a non-canonical name is found by get(), so prepare()'s never-replace guard cannot be bypassed into writing a conflicting canonical twin; - conflicting records under one conversion_id fail closed: get()/put() error with a diagnostic naming the paths, and unresolved() skips the conversion and reports every copy through unreadable_entries(); - put() refuses a record that differs from the stored one in anything but state, and canonicalizes legitimate writes by removing the stray copies they supersede. Regression tests: duplicate copies reconcile exactly once, misnamed record blocks a conflicting prepare, misnamed-only record refuses a conflicting put and canonicalizes a matching one, conflicting copies fail closed and are reported. --- pkgs/id/src/currency_poc/intent.rs | 305 +++++++++++++++++++++++------ pkgs/id/src/currency_poc/tests.rs | 203 +++++++++++++++++++ 2 files changed, 451 insertions(+), 57 deletions(-) diff --git a/pkgs/id/src/currency_poc/intent.rs b/pkgs/id/src/currency_poc/intent.rs index 0d7cc781c..1a7a632f4 100644 --- a/pkgs/id/src/currency_poc/intent.rs +++ b/pkgs/id/src/currency_poc/intent.rs @@ -11,7 +11,7 @@ //! abandoned. use std::cell::RefCell; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::fs; use std::io; use std::path::{Path, PathBuf}; @@ -73,9 +73,11 @@ impl IntentRecord { } /// A stored entry that a listing skipped because it could not be -/// read or parsed (#4125). Skipped entries are never deleted by the -/// store or the recovery scanner: the path stays on disk so an -/// operator can inspect, repair, or remove it deliberately. +/// read or parsed (#4125), or because its identity conflicts with +/// another stored copy of the same conversion (#4578). Skipped +/// entries are never deleted by the store or the recovery scanner: +/// the path stays on disk so an operator can inspect, repair, or +/// remove it deliberately. #[derive(Clone, Debug, PartialEq, Eq)] pub struct UnreadableIntent { pub path: PathBuf, @@ -83,6 +85,14 @@ pub struct UnreadableIntent { } /// Persistence seam for intent records. +/// +/// Identity contract (#4578): a `conversion_id` names ONE conversion, +/// and every implementation must agree on that identity across +/// `put`, `get`, and `unresolved`. Duplicate stored copies of one +/// record are one record (a listing never yields it twice), and two +/// different records stored under one conversion_id are an ambiguous +/// identity that must fail closed — never silently shadow, bypass, +/// or double-process each other. pub trait IntentStore { /// Insert or replace a record. Must be durable before returning: /// the write-ahead guarantee depends on it. @@ -144,6 +154,17 @@ impl IntentStore for InMemoryIntentStore { /// File-backed store: one JSON file per conversion under a directory, /// written via a temporary file + rename + fsync so a crash mid-write /// never leaves a torn record behind. +/// +/// Filesystem identity contract (#4578): a conversion's identity is +/// the `conversion_id` in the record CONTENT, and its canonical file +/// is `{conversion_id:032x}.json` — the only path this store ever +/// writes. Every read scans the directory and groups by content ID, +/// so `get`, `unresolved`, `put`, and the executor's `prepare` guard +/// (which is `get`-based) all see the same identity surface: +/// duplicate copies of one record are one record; a record stored +/// only under a non-canonical name is still the conversion's record; +/// different records under one conversion_id fail closed with a +/// diagnostic naming the paths (see the method implementations). pub struct FileIntentStore { dir: PathBuf, /// Entries skipped by the most recent `unresolved` listing @@ -160,29 +181,156 @@ impl FileIntentStore { }) } - fn note_unreadable(&self, path: &Path, reason: String) { - self.unreadable.borrow_mut().push(UnreadableIntent { - path: path.to_path_buf(), - reason, - }); - } - fn path_for(&self, conversion_id: u128) -> PathBuf { self.dir.join(format!("{conversion_id:032x}.json")) } + + /// Scan the whole store directory once: every `*.json` file that + /// parses, in path order, plus every entry that could not be read + /// or parsed (#4125). Only a failure to enumerate the directory + /// itself is an `Err` — the store is unreachable then; anything + /// wrong with one entry inside it is reported, never fatal here. + fn scan(&self) -> io::Result { + let entries = fs::read_dir(&self.dir)?; + let mut paths = Vec::new(); + let mut unreadable = Vec::new(); + for entry in entries { + match entry { + Ok(entry) => paths.push(entry.path()), + Err(error) => unreadable.push(UnreadableIntent { + path: self.dir.clone(), + reason: format!("directory entry: {error}"), + }), + } + } + paths.sort(); + let mut scanned = Vec::new(); + for path in paths { + if path.extension().and_then(|ext| ext.to_str()) != Some("json") { + continue; + } + // fs::read also fails for a subdirectory named *.json; + // serde parsing fails for foreign or schema-drifted + // content. Both skip-and-report, never abort, never delete. + let bytes = match fs::read(&path) { + Ok(bytes) => bytes, + Err(error) => { + unreadable.push(UnreadableIntent { + path, + reason: format!("read: {error}"), + }); + continue; + } + }; + match serde_json::from_slice::(&bytes) { + Ok(record) => scanned.push(ScannedIntent { path, record }), + Err(error) => unreadable.push(UnreadableIntent { + path, + reason: format!("parse: {error}"), + }), + } + } + unreadable.sort_by(|a, b| a.path.cmp(&b.path)); + Ok(IntentScan { + entries: scanned, + unreadable, + }) + } +} + +/// One parseable intent file found by a directory scan. +struct ScannedIntent { + path: PathBuf, + record: IntentRecord, +} + +/// The result of one [`FileIntentStore::scan`]. +struct IntentScan { + entries: Vec, + unreadable: Vec, +} + +impl IntentScan { + /// Every stored copy of `conversion_id`, in path order. Identity + /// is the conversion_id in the record CONTENT (#4578): the file + /// name a copy sits under does not change which conversion it is. + fn copies_for(&self, conversion_id: u128) -> Vec<&ScannedIntent> { + self.entries + .iter() + .filter(|entry| entry.record.conversion_id == conversion_id) + .collect() + } + + /// The one distinct record among `copies`, or `None` when the + /// copies disagree — an ambiguous identity (#4578). + fn distinct(copies: &[&ScannedIntent]) -> Option { + let first = &copies.first()?.record; + copies + .iter() + .all(|copy| copy.record == *first) + .then(|| first.clone()) + } +} + +/// The fail-closed error for an ambiguous conversion identity: two +/// different records stored under one conversion_id (#4578). +fn ambiguous_identity_error(conversion_id: u128, copies: &[&ScannedIntent]) -> io::Error { + let paths: Vec = copies + .iter() + .map(|copy| copy.path.display().to_string()) + .collect(); + io::Error::new( + io::ErrorKind::InvalidData, + format!( + "intent store: conversion {conversion_id} has conflicting records at {}; one conversion_id must name one record — reconcile the copies manually (keep the record that matches the ledger, remove the others)", + paths.join(", ") + ), + ) } impl IntentStore for FileIntentStore { fn put(&mut self, record: &IntentRecord) -> io::Result<()> { - let path = self.path_for(record.conversion_id); - let tmp = path.with_extension("json.tmp"); + let canonical = self.path_for(record.conversion_id); + // Identity check (#4578): before writing, every stored copy + // of this conversion — canonical or not — must be the same + // conversion this write belongs to. State is the one field a + // legitimate write may advance, so it is normalised out of + // the comparison (the same trick prepare() uses): a copy + // that differs in anything else is a conflicting record, and + // a conflicting put fails closed and writes nothing. + let scan = self.scan()?; + let copies = scan.copies_for(record.conversion_id); + let mut stray_paths: Vec = Vec::new(); + if !copies.is_empty() { + let Some(stored) = IntentScan::distinct(&copies) else { + return Err(ambiguous_identity_error(record.conversion_id, &copies)); + }; + let mut comparable = stored; + comparable.state = record.state; + if comparable != *record { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "intent store: refusing to write conversion {} over a different persisted intent (stored at {}); a conversion_id names one conversion — reconcile the stored record first", + record.conversion_id, + copies[0].path.display() + ), + )); + } + stray_paths = copies + .iter() + .map(|copy| copy.path.clone()) + .filter(|path| *path != canonical) + .collect(); + } + let tmp = canonical.with_extension("json.tmp"); let bytes = serde_json::to_vec_pretty(record) .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error))?; fs::write(&tmp, bytes)?; let file = fs::File::open(&tmp)?; file.sync_all()?; drop(file); - fs::rename(&tmp, &path)?; + fs::rename(&tmp, &canonical)?; // Sync the containing directory so the rename itself is // durable: syncing the file alone does not guarantee the new // directory entry survives a machine crash, and the @@ -190,63 +338,106 @@ impl IntentStore for FileIntentStore { // by the recovery scanner afterwards. let dir = fs::File::open(&self.dir)?; dir.sync_all()?; + // Canonicalize (#4578): the write above supersedes every + // stray copy of this same record, so remove them — one + // conversion now lives at exactly its canonical path, and no + // later listing can meet the same identity twice. Copies of + // a DIFFERENT record never reach this point (refused above), + // and unreadable entries are never deleted (#4125). + for stray in &stray_paths { + fs::remove_file(stray).map_err(|error| { + io::Error::new( + error.kind(), + format!( + "intent store: conversion {} is persisted at {}, but removing its superseded duplicate copy {} failed: {error}; remove it manually so the conversion has exactly one stored copy", + record.conversion_id, + canonical.display(), + stray.display() + ), + ) + })?; + } + if !stray_paths.is_empty() { + let dir = fs::File::open(&self.dir)?; + dir.sync_all()?; + } Ok(()) } fn get(&self, conversion_id: u128) -> io::Result> { - let path = self.path_for(conversion_id); - if !path.exists() { + let scan = self.scan()?; + // A corrupt canonical file fails closed, as it always has: + // it is the record this conversion's write-ahead guarantee + // rests on, so treating it as absent could let prepare() + // overwrite an intent that may already be posted. (A corrupt + // NON-canonical file is a #4125 skipped entry, not this + // conversion's canonical record.) + let canonical = self.path_for(conversion_id); + if let Some(bad) = scan.unreadable.iter().find(|entry| entry.path == canonical) { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!( + "intent store: canonical intent file {} is unreadable: {}", + canonical.display(), + bad.reason + ), + )); + } + let copies = scan.copies_for(conversion_id); + if copies.is_empty() { return Ok(None); } - let bytes = fs::read(path)?; - let record = serde_json::from_slice(&bytes) - .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error))?; - Ok(Some(record)) + // Duplicate copies of one record are that record; a record + // stored only under a non-canonical name is still found, so + // prepare()'s never-replace guard cannot be bypassed by a + // misnamed file (#4578). Copies that disagree are an + // ambiguous identity and fail closed. + match IntentScan::distinct(&copies) { + Some(record) => Ok(Some(record)), + None => Err(ambiguous_identity_error(conversion_id, &copies)), + } } fn unresolved(&self) -> io::Result> { - // A failure to enumerate the directory itself is fatal (the - // store is unreachable); anything wrong with one entry inside - // it is not (#4125). - let entries = fs::read_dir(&self.dir)?; - self.unreadable.borrow_mut().clear(); + let scan = self.scan()?; + let mut skipped = scan.unreadable.clone(); + // Group by the conversion_id the content names (#4578): + // duplicate copies collapse to one record, so the recovery + // scanner can never reconcile one conversion twice. A + // conversion whose copies disagree is ambiguous — listing + // any one of them would risk reconciling the wrong record — + // so it is skipped and every copy is reported through + // unreadable_entries() while the rest of the listing + // proceeds (#4125 listing contract). + let mut by_conversion: BTreeMap> = BTreeMap::new(); + for entry in &scan.entries { + by_conversion + .entry(entry.record.conversion_id) + .or_default() + .push(entry); + } let mut records = Vec::new(); - for entry in entries { - let entry = match entry { - Ok(entry) => entry, - Err(error) => { - self.note_unreadable(&self.dir, format!("directory entry: {error}")); - continue; + for (conversion_id, copies) in by_conversion { + match IntentScan::distinct(&copies) { + Some(record) => { + if record.state.is_unresolved() { + records.push(record); + } } - }; - let path = entry.path(); - if path.extension().and_then(|ext| ext.to_str()) != Some("json") { - continue; - } - // fs::read also fails for a subdirectory named *.json; - // serde parsing fails for foreign or schema-drifted - // content. Both skip-and-report, never abort, never delete. - let bytes = match fs::read(&path) { - Ok(bytes) => bytes, - Err(error) => { - self.note_unreadable(&path, format!("read: {error}")); - continue; + None => { + for copy in copies { + skipped.push(UnreadableIntent { + path: copy.path.clone(), + reason: format!( + "conflicting records for conversion {conversion_id}: more than one distinct record is stored under this conversion_id, so its identity is ambiguous and it was skipped; reconcile the copies manually" + ), + }); + } } - }; - let record: IntentRecord = match serde_json::from_slice(&bytes) { - Ok(record) => record, - Err(error) => { - self.note_unreadable(&path, format!("parse: {error}")); - continue; - } - }; - if record.state.is_unresolved() { - records.push(record); } } - self.unreadable - .borrow_mut() - .sort_by(|a, b| a.path.cmp(&b.path)); + skipped.sort_by(|a, b| a.path.cmp(&b.path)); + *self.unreadable.borrow_mut() = skipped; Ok(records) } diff --git a/pkgs/id/src/currency_poc/tests.rs b/pkgs/id/src/currency_poc/tests.rs index 7fc839a5e..ec837226e 100644 --- a/pkgs/id/src/currency_poc/tests.rs +++ b/pkgs/id/src/currency_poc/tests.rs @@ -717,3 +717,206 @@ fn recovery_continues_past_a_json_named_subdirectory() { assert!(subdir.is_dir(), "the subdirectory stays in place"); let _ = std::fs::remove_dir_all(&dir); } + +// --- #4578: one filesystem identity contract for the intent store --- + +fn empty_file_store(tag: &str) -> (PathBufForTests, FileIntentStore) { + let dir = std::env::temp_dir().join(format!( + "id-currency-poc-intents-{tag}-{}", + std::process::id() + )); + let _ = std::fs::remove_dir_all(&dir); + let store = FileIntentStore::open(dir.clone()).expect("open store"); + (dir, store) +} + +fn canonical_name(conversion_id: u128) -> String { + format!("{conversion_id:032x}.json") +} + +fn json_files(dir: &std::path::Path) -> Vec { + let mut files: Vec<_> = std::fs::read_dir(dir) + .expect("read store dir") + .filter_map(|entry| entry.ok().map(|entry| entry.path())) + .filter(|path| path.extension().and_then(|ext| ext.to_str()) == Some("json")) + .collect(); + files.sort(); + files +} + +#[test] +fn duplicate_copies_of_one_intent_reconcile_exactly_once() { + // The same record stored three times (canonical + two stray + // copies, e.g. after an operator backup/restore) is ONE intent: + // the listing yields it once and recovery reconciles it once. + let (dir, mut store, mut backend) = file_store_with_recorded_intent("dup4578"); + let canonical = dir.join(canonical_name(777)); + let bytes = std::fs::read(&canonical).expect("read canonical intent"); + std::fs::write(dir.join("copy-a.json"), &bytes).expect("write copy a"); + std::fs::write(dir.join("copy-b.json"), &bytes).expect("write copy b"); + + let listed = store.unresolved().expect("listing with duplicates"); + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].conversion_id, 777); + assert!( + store.unreadable_entries().is_empty(), + "identical duplicates are not unreadable entries" + ); + + let outcomes = reconcile(&mut backend, &mut store, RecoveryPolicy::Resubmit) + .expect("reconcile with duplicates"); + assert_eq!(outcomes, vec![(777, ReconcileOutcome::Resubmitted)]); + assert_eq!(backend.transfers_with_user_data_128(777).len(), 3); + + // The state-advancing put canonicalized the store: the stray + // copies of the superseded record are gone, one file remains. + assert_eq!(json_files(&dir), vec![canonical]); + assert_eq!( + store + .get(777) + .expect("get after reconcile") + .map(|r| r.state), + Some(IntentState::Posted) + ); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn misnamed_record_is_found_by_get_and_blocks_conflicting_prepare() { + // A parseable record stored ONLY under a non-canonical name is + // still the conversion's record: get() must find it, so + // prepare()'s never-replace guard cannot be bypassed into + // writing a conflicting canonical twin. + let (dir, mut store) = empty_file_store("misnamed4578"); + let original = bare_record( + 9, + vec![funding_transfer(501, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + store.put(&original).expect("seed put"); + let canonical = dir.join(canonical_name(9)); + let misnamed = dir.join("misnamed.json"); + std::fs::rename(&canonical, &misnamed).expect("rename to a non-canonical name"); + + assert_eq!(store.get(9).expect("get misnamed"), Some(original.clone())); + + let mut executor = ConversionExecutor::new(funded_ledger(), store); + let mut conflicting = request(); + conflicting.conversion_id = 9; + conflicting.transfer_ids = vec![701, 702, 703]; + conflicting.amount_in = 5_000; + let error = executor + .prepare(&conflicting) + .expect_err("conflicting prepare must be refused"); + assert!( + error.to_string().contains("already has a persisted intent"), + "unexpected prepare error: {error}" + ); + // No canonical twin was written; the misnamed record is intact. + assert_eq!(json_files(&dir), vec![misnamed]); + assert_eq!( + executor.store.get(9).expect("get after refusal"), + Some(original) + ); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn misnamed_only_record_refuses_conflicting_put_and_canonicalizes_matching_put() { + let (dir, mut store) = empty_file_store("put4578"); + let original = bare_record( + 9, + vec![funding_transfer(501, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + store.put(&original).expect("seed put"); + let canonical = dir.join(canonical_name(9)); + let misnamed = dir.join("stray.json"); + std::fs::rename(&canonical, &misnamed).expect("rename to a non-canonical name"); + + // A conflicting put over the misnamed record fails closed and + // writes nothing. + let conflicting = bare_record( + 9, + vec![funding_transfer(502, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + let error = store + .put(&conflicting) + .expect_err("conflicting put must be refused"); + assert!( + error.to_string().contains("refusing to write conversion 9"), + "unexpected put error: {error}" + ); + assert_eq!(json_files(&dir), vec![misnamed.clone()]); + + // A state-advancing put of the SAME record succeeds and + // canonicalizes: the record now lives only at its canonical + // path. + let mut advanced = original.clone(); + advanced.state = IntentState::Posted; + store.put(&advanced).expect("state-advancing put"); + assert_eq!(json_files(&dir), vec![canonical]); + assert_eq!(store.get(9).expect("get canonicalized"), Some(advanced)); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn conflicting_copies_fail_closed_and_are_reported() { + // Two DIFFERENT records stored under one conversion_id: the + // identity is ambiguous. get()/put() fail closed, the listing + // skips the conversion and reports both copies, and other + // conversions reconcile normally. + let (dir, mut store, mut backend) = file_store_with_recorded_intent("conflict4578"); + let record_a = bare_record( + 11, + vec![funding_transfer(501, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + store.put(&record_a).expect("seed canonical copy"); + let canonical_11 = dir.join(canonical_name(11)); + let record_b = bare_record( + 11, + vec![funding_transfer(502, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + let (other_dir, mut other_store) = empty_file_store("conflict4578-b"); + other_store.put(&record_b).expect("seed other copy"); + let other_copy = dir.join("other.json"); + std::fs::copy(other_dir.join(canonical_name(11)), &other_copy).expect("plant conflicting copy"); + let _ = std::fs::remove_dir_all(&other_dir); + + let get_error = store.get(11).expect_err("ambiguous get must fail closed"); + assert!( + get_error.to_string().contains("conflicting records"), + "unexpected get error: {get_error}" + ); + let mut advanced_a = record_a.clone(); + advanced_a.state = IntentState::Posted; + assert!( + store.put(&advanced_a).is_err(), + "ambiguous put must fail closed" + ); + + let listed = store + .unresolved() + .expect("listing with a conflicted conversion"); + assert_eq!( + listed.iter().map(|r| r.conversion_id).collect::>(), + vec![777], + "the conflicted conversion is skipped, the good one is listed" + ); + let reported: Vec<_> = store + .unreadable_entries() + .iter() + .map(|entry| entry.path.clone()) + .collect(); + assert!( + reported.contains(&canonical_11), + "canonical copy reported: {reported:?}" + ); + assert!( + reported.contains(&other_copy), + "stray copy reported: {reported:?}" + ); + + let outcomes = reconcile(&mut backend, &mut store, RecoveryPolicy::Resubmit) + .expect("reconcile past the conflicted conversion"); + assert_eq!(outcomes, vec![(777, ReconcileOutcome::Resubmitted)]); + let _ = std::fs::remove_dir_all(&dir); +} From e3d4db6a51f1b790eb9ae9a300e4fc74ff7d3461 Mon Sep 17 00:00:00 2001 From: Drewry Pope Date: Mon, 2 Nov 2026 16:53:22 -0600 Subject: [PATCH 2/4] fix(id/currency_poc): close the two Codex findings on PR #4622 (#4578) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1: a canonical file holding ANOTHER conversions record now fails get()/put() closed (canonical_occupant_error) instead of reading as absent — prepare()s never-replace guard can no longer be bypassed and the occupants only write-ahead record is never overwritten. P2: copies differing ONLY in state resolve deterministically to the most advanced state (IntentState::precedence) in IntentScan::distinct, so the residue of a state-advancing put interrupted between the canonical write and stray cleanup stays readable and reconcilable instead of bricking the conversion as an ambiguous identity. Regression tests cover both. --- pkgs/id/src/currency_poc/intent.rs | 100 +++++++++++++++++++++++++++-- pkgs/id/src/currency_poc/tests.rs | 91 ++++++++++++++++++++++++++ 2 files changed, 187 insertions(+), 4 deletions(-) diff --git a/pkgs/id/src/currency_poc/intent.rs b/pkgs/id/src/currency_poc/intent.rs index 1a7a632f4..a9c152778 100644 --- a/pkgs/id/src/currency_poc/intent.rs +++ b/pkgs/id/src/currency_poc/intent.rs @@ -40,6 +40,22 @@ impl IntentState { pub fn is_unresolved(self) -> bool { matches!(self, IntentState::Recorded | IntentState::Submitted) } + + /// Deterministic precedence between copies of one record that + /// differ only in state (#4578): the lifecycle advances + /// `Recorded` -> `Submitted`, and a terminal decision outranks an + /// in-flight state — `Posted` (a ledger fact) above `Abandoned` + /// (a decision not to resubmit), so a decision recorded without + /// knowledge of a completed posting never masks it, and an + /// abandon decision is never undone by an older in-flight copy. + pub fn precedence(self) -> u8 { + match self { + IntentState::Recorded => 0, + IntentState::Submitted => 1, + IntentState::Abandoned => 2, + IntentState::Posted => 3, + } + } } /// The write-ahead record for one conversion. @@ -261,17 +277,66 @@ impl IntentScan { .collect() } + /// The scanned entry sitting at `path`, whatever conversion its + /// content names (#4578): a conversion's canonical file can hold + /// ANOTHER conversion's record (a misnamed write or an operator + /// restore), and that occupancy is a fact about the path, not + /// about the content identity `copies_for` groups by. + fn entry_at(&self, path: &Path) -> Option<&ScannedIntent> { + self.entries.iter().find(|entry| entry.path == path) + } + /// The one distinct record among `copies`, or `None` when the /// copies disagree — an ambiguous identity (#4578). + /// + /// Copies that differ ONLY in state are one record at its most + /// advanced state (by [`IntentState::precedence`]): a + /// state-advancing `put` writes the new canonical record before + /// removing superseded copies, so a crash — or a failed removal — + /// in between leaves exactly that residue, and the conversion + /// must stay readable and reconcilable instead of failing closed + /// forever (Codex P2 on PR #4622). Copies that differ in any + /// other field are genuinely conflicting records: ambiguous. fn distinct(copies: &[&ScannedIntent]) -> Option { let first = &copies.first()?.record; - copies - .iter() - .all(|copy| copy.record == *first) - .then(|| first.clone()) + let mut resolved = first.clone(); + for copy in copies { + let mut comparable = copy.record.clone(); + comparable.state = first.state; + if comparable != *first { + return None; + } + if copy.record.state.precedence() > resolved.state.precedence() { + resolved.state = copy.record.state; + } + } + Some(resolved) } } +/// The fail-closed error for a canonical file that currently stores +/// a DIFFERENT conversion's record (#4578, Codex P1 on PR #4622): +/// treating the conversion as absent would let `prepare()` pass its +/// never-replace guard, and overwriting would destroy the occupant's +/// only write-ahead record. +fn canonical_occupant_error( + conversion_id: u128, + canonical: &Path, + occupant_id: u128, +) -> io::Error { + io::Error::new( + io::ErrorKind::InvalidData, + format!( + "intent store: canonical intent file {} for conversion {conversion_id} \ + currently stores conversion {occupant_id}'s record; refusing to treat \ + conversion {conversion_id} as absent or overwrite the occupant — \ + reconcile the file manually (its content names the conversion it \ + belongs to)", + canonical.display() + ), + ) +} + /// The fail-closed error for an ambiguous conversion identity: two /// different records stored under one conversion_id (#4578). fn ambiguous_identity_error(conversion_id: u128, copies: &[&ScannedIntent]) -> io::Error { @@ -299,6 +364,19 @@ impl IntentStore for FileIntentStore { // that differs in anything else is a conflicting record, and // a conflicting put fails closed and writes nothing. let scan = self.scan()?; + // Canonical occupancy (#4578): a record belonging to another + // conversion sitting at this conversion's canonical path is + // never overwritten — it may be that conversion's only + // write-ahead record. + if let Some(occupant) = scan.entry_at(&canonical) { + if occupant.record.conversion_id != record.conversion_id { + return Err(canonical_occupant_error( + record.conversion_id, + &canonical, + occupant.record.conversion_id, + )); + } + } let copies = scan.copies_for(record.conversion_id); let mut stray_paths: Vec = Vec::new(); if !copies.is_empty() { @@ -383,6 +461,20 @@ impl IntentStore for FileIntentStore { ), )); } + // Canonical occupancy (#4578): if this conversion's + // canonical file holds another conversion's record, the + // conversion is NOT absent — answering None here would let + // prepare()'s never-replace guard pass and put() overwrite + // the occupant's only record. Fail closed instead. + if let Some(occupant) = scan.entry_at(&canonical) { + if occupant.record.conversion_id != conversion_id { + return Err(canonical_occupant_error( + conversion_id, + &canonical, + occupant.record.conversion_id, + )); + } + } let copies = scan.copies_for(conversion_id); if copies.is_empty() { return Ok(None); diff --git a/pkgs/id/src/currency_poc/tests.rs b/pkgs/id/src/currency_poc/tests.rs index ec837226e..0db421ed1 100644 --- a/pkgs/id/src/currency_poc/tests.rs +++ b/pkgs/id/src/currency_poc/tests.rs @@ -920,3 +920,94 @@ fn conflicting_copies_fail_closed_and_are_reported() { assert_eq!(outcomes, vec![(777, ReconcileOutcome::Resubmitted)]); let _ = std::fs::remove_dir_all(&dir); } + +// --- #4578 / PR #4622 Codex findings: canonical occupancy (P1) and +// --- state-only divergence after an interrupted canonicalization (P2) + +#[test] +fn canonical_file_holding_another_conversions_record_blocks_get_and_put() { + // Conversion 9's only record sits under conversion 10's canonical + // file name (a misnamed write / operator restore). Conversion 10 + // must NOT read as absent (that would pass prepare()'s + // never-replace guard) and must NOT be written over the occupant. + let (dir, mut store) = empty_file_store("occupant4578"); + let occupant = bare_record( + 9, + vec![funding_transfer(501, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + store.put(&occupant).expect("seed occupant"); + let misplaced = dir.join(canonical_name(10)); + std::fs::rename(dir.join(canonical_name(9)), &misplaced).expect("misplace occupant"); + let occupant_bytes = std::fs::read(&misplaced).expect("read occupant bytes"); + + let get_error = store.get(10).expect_err("occupied get must fail closed"); + assert!( + get_error.to_string().contains("currently stores conversion 9"), + "unexpected get error: {get_error}" + ); + let intruder = bare_record( + 10, + vec![funding_transfer(601, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + let put_error = store.put(&intruder).expect_err("occupied put must fail closed"); + assert!( + put_error.to_string().contains("currently stores conversion 9"), + "unexpected put error: {put_error}" + ); + assert_eq!( + std::fs::read(&misplaced).expect("re-read occupant"), + occupant_bytes, + "the occupant's only record is byte-identical after both refusals" + ); + // The occupant itself is unaffected: content identity still finds + // it, and the listing yields it exactly once. + assert_eq!(store.get(9).expect("get occupant"), Some(occupant)); + let listed = store.unresolved().expect("listing with an occupant"); + assert_eq!(listed.iter().map(|r| r.conversion_id).collect::>(), vec![9]); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn state_only_divergent_copies_resolve_to_the_most_advanced_state() { + // A state-advancing put writes the new canonical record BEFORE + // removing superseded copies; a crash in between leaves copies + // that differ only in state. That residue is one record at its + // most advanced state — readable and listed once — not an + // ambiguous identity that bricks the conversion. + let (dir, mut store) = empty_file_store("states4578"); + let recorded = bare_record( + 11, + vec![funding_transfer(501, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + store.put(&recorded).expect("seed recorded"); + // Plant the post-crash residue by hand: the canonical file holds + // the Submitted copy, a stray still holds the Recorded copy. + let mut submitted = recorded.clone(); + submitted.state = IntentState::Submitted; + let (other_dir, mut other_store) = empty_file_store("states4578-b"); + other_store.put(&submitted).expect("seed submitted"); + std::fs::copy(other_dir.join(canonical_name(11)), dir.join(canonical_name(11))) + .expect("plant advanced canonical copy"); + let _ = std::fs::remove_dir_all(&other_dir); + std::fs::copy(dir.join(canonical_name(11)), dir.join("stray.json")) + .expect("plant stray copy"); + // Downgrade the stray to the Recorded copy's bytes: rewrite it + // from a fresh store holding only the Recorded record. + let (old_dir, mut old_store) = empty_file_store("states4578-c"); + old_store.put(&recorded).expect("seed recorded copy"); + std::fs::copy(old_dir.join(canonical_name(11)), dir.join("stray.json")) + .expect("plant recorded stray copy"); + let _ = std::fs::remove_dir_all(&old_dir); + + assert_eq!(store.get(11).expect("get diverged copies"), Some(submitted)); + let listed = store.unresolved().expect("listing diverged copies"); + assert_eq!(listed.len(), 1, "one conversion, listed once"); + assert_eq!(listed[0].state, IntentState::Submitted); + // And a state-advancing put still canonicalizes the residue away. + let mut posted = recorded.clone(); + posted.state = IntentState::Posted; + store.put(&posted).expect("advancing put over the residue"); + assert_eq!(json_files(&dir), vec![dir.join(canonical_name(11))]); + assert_eq!(store.get(11).expect("get canonicalized"), Some(posted)); + let _ = std::fs::remove_dir_all(&dir); +} From 6002db2b1c671ab577d01f26447ace3aa9a7cc12 Mon Sep 17 00:00:00 2001 From: Drewry Pope Date: Mon, 2 Nov 2026 17:27:33 -0600 Subject: [PATCH 3/4] fix(id/currency_poc): close the Codex re-review findings on PR #4622 (#4578) P1 (intent.rs cleanup): a stray path that is a filesystem alias of the canonical file (case-insensitive spelling, symlink, hardlink) is never a cleanup target - after the rename it names the record just persisted, so removing it would delete the write-ahead record. same_file() compares canonicalised paths and unix dev+inode; entry_at() is alias-aware too. P2 (unresolved listing): a conversion whose canonical file stores another conversions record is blocked - get/put fail closed on the occupancy - so the listing now skips and reports it through unreadable_entries() like a conflicting copy, instead of handing recovery a record whose state-advancing put aborts the scan. Regression tests: put_never_deletes_a_filesystem_alias_of_the_canonical_file, canonically_blocked_conversion_is_skipped_and_reported_during_listing. cargo test -p id --lib currency_poc (in pkgs/id): 33 passed. --- pkgs/id/src/currency_poc/intent.rs | 72 ++++++++++++++++++++++- pkgs/id/src/currency_poc/tests.rs | 94 ++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 1 deletion(-) diff --git a/pkgs/id/src/currency_poc/intent.rs b/pkgs/id/src/currency_poc/intent.rs index a9c152778..635d22529 100644 --- a/pkgs/id/src/currency_poc/intent.rs +++ b/pkgs/id/src/currency_poc/intent.rs @@ -181,6 +181,14 @@ impl IntentStore for InMemoryIntentStore { /// only under a non-canonical name is still the conversion's record; /// different records under one conversion_id fail closed with a /// diagnostic naming the paths (see the method implementations). +/// Two refinements from the Codex re-review of PR #4622: a path that +/// is a filesystem ALIAS of the canonical file (another spelling of +/// the same file on a case-insensitive filesystem, a symlink, a +/// hardlink) is the canonical file, never a stray copy to clean up; +/// and a conversion whose canonical file currently stores ANOTHER +/// conversion's record is blocked — `unresolved` skips and reports +/// it like a conflicting copy instead of handing recovery a record +/// whose state-advancing `put` is guaranteed to fail. pub struct FileIntentStore { dir: PathBuf, /// Entries skipped by the most recent `unresolved` listing @@ -283,7 +291,9 @@ impl IntentScan { /// restore), and that occupancy is a fact about the path, not /// about the content identity `copies_for` groups by. fn entry_at(&self, path: &Path) -> Option<&ScannedIntent> { - self.entries.iter().find(|entry| entry.path == path) + self.entries + .iter() + .find(|entry| entry.path == path || same_file(&entry.path, path)) } /// The one distinct record among `copies`, or `None` when the @@ -314,6 +324,32 @@ impl IntentScan { } } +/// True when two paths name the same filesystem object (Codex P1 +/// on PR #4622): on a case-insensitive filesystem the canonical +/// file's uppercase-hex spelling is the SAME file, and a symlink or +/// hardlink to it resolves to it too. Lexical path comparison +/// misses all of these, and treating such an alias as a stray copy +/// is destructive: after `put` renames the new record over the +/// canonical path, removing the "stray" alias would delete the +/// record just persisted. Canonicalisation covers spellings the OS +/// itself resolves to one path; device+inode covers hardlinks and +/// symlinks whose canonical paths still differ in spelling. +fn same_file(a: &Path, b: &Path) -> bool { + if let (Ok(canon_a), Ok(canon_b)) = (fs::canonicalize(a), fs::canonicalize(b)) { + if canon_a == canon_b { + return true; + } + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + if let (Ok(meta_a), Ok(meta_b)) = (fs::metadata(a), fs::metadata(b)) { + return meta_a.dev() == meta_b.dev() && meta_a.ino() == meta_b.ino(); + } + } + false +} + /// The fail-closed error for a canonical file that currently stores /// a DIFFERENT conversion's record (#4578, Codex P1 on PR #4622): /// treating the conversion as absent would let `prepare()` pass its @@ -423,6 +459,16 @@ impl IntentStore for FileIntentStore { // a DIFFERENT record never reach this point (refused above), // and unreadable entries are never deleted (#4125). for stray in &stray_paths { + // Alias guard (Codex P1 on PR #4622): a "stray" that is + // the canonical file under another spelling or link is + // not a superseded copy — since the rename above, it + // names the record just persisted, so removing it would + // delete the write-ahead record `put` just made durable. + // Leave it in place; listings group by content identity, + // so the alias never double-counts the conversion. + if same_file(stray, &canonical) { + continue; + } fs::remove_file(stray).map_err(|error| { io::Error::new( error.kind(), @@ -510,6 +556,30 @@ impl IntentStore for FileIntentStore { } let mut records = Vec::new(); for (conversion_id, copies) in by_conversion { + // Canonical occupancy (Codex P2 on PR #4622): when this + // conversion's canonical file currently stores ANOTHER + // conversion's record, `get` and `put` for this + // conversion fail closed by design — so a listing that + // still yields the record hands recovery an entry whose + // state-advancing `put` is guaranteed to error out and + // abort the whole scan. Skip the blocked conversion and + // report it through `unreadable_entries()` (the record's + // own copies stay on disk for an operator), exactly like + // a conflicting copy below; every other conversion still + // reconciles. + let canonical = self.path_for(conversion_id); + if let Some(occupant) = scan.entry_at(&canonical) { + if occupant.record.conversion_id != conversion_id { + skipped.push(UnreadableIntent { + path: canonical, + reason: format!( + "conversion {conversion_id} is blocked: its canonical intent file currently stores conversion {}'s record, so the conversion was skipped during recovery listing; reconcile the file manually (its content names the conversion it belongs to)", + occupant.record.conversion_id + ), + }); + continue; + } + } match IntentScan::distinct(&copies) { Some(record) => { if record.state.is_unresolved() { diff --git a/pkgs/id/src/currency_poc/tests.rs b/pkgs/id/src/currency_poc/tests.rs index 0db421ed1..9844da84f 100644 --- a/pkgs/id/src/currency_poc/tests.rs +++ b/pkgs/id/src/currency_poc/tests.rs @@ -1011,3 +1011,97 @@ fn state_only_divergent_copies_resolve_to_the_most_advanced_state() { assert_eq!(store.get(11).expect("get canonicalized"), Some(posted)); let _ = std::fs::remove_dir_all(&dir); } + +// --- #4578 / PR #4622 Codex re-review: alias cleanup (P1) and +// --- canonically blocked identities in recovery listing (P2) + +#[test] +fn put_never_deletes_a_filesystem_alias_of_the_canonical_file() { + // A stray path that NAMES the canonical file (here a symlink to + // it; on a case-insensitive filesystem the uppercase-hex spelling + // of the canonical name is the same situation) must never be a + // cleanup target: after the state-advancing put renames the new + // record over the canonical path, the alias resolves to the NEW + // record, so removing it would delete the record just persisted + // while `put` reports success. + let (dir, mut store) = empty_file_store("alias4578"); + let recorded = bare_record( + 10, + vec![funding_transfer(501, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + store.put(&recorded).expect("seed recorded"); + let canonical = dir.join(canonical_name(10)); + let alias = dir.join(format!("{:032X}.json", 10)); + std::os::unix::fs::symlink(&canonical, &alias).expect("plant alias of canonical"); + + let mut submitted = recorded.clone(); + submitted.state = IntentState::Submitted; + store.put(&submitted).expect("state-advancing put over an alias"); + + assert_eq!( + store.get(10).expect("get after alias put"), + Some(submitted), + "the persisted record survives the cleanup pass" + ); + assert!( + alias.exists(), + "the alias itself is left in place, never deleted as a stray copy" + ); + // The listing still collapses the alias to the one conversion. + let listed = store.unresolved().expect("listing with an alias"); + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].conversion_id, 10); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn canonically_blocked_conversion_is_skipped_and_reported_during_listing() { + // Conversion 1's only record sits in stray.json, while conversion + // 1's canonical file stores conversion 2's record: get(1)/put(1) + // fail closed on the occupancy, so a listing that yielded + // conversion 1 would hand recovery a record whose + // state-advancing put errors out and aborts the whole scan. The + // listing must skip and report the blocked conversion instead, + // and still reconcile the unblocked one. + let (dir, mut store) = empty_file_store("blocked4578"); + let blocked = bare_record( + 1, + vec![funding_transfer(9501, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + store.put(&blocked).expect("seed blocked record"); + let stray = dir.join("stray.json"); + std::fs::rename(dir.join(canonical_name(1)), &stray).expect("strand blocked record"); + let occupant = bare_record( + 2, + vec![funding_transfer(9502, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + store.put(&occupant).expect("seed occupant record"); + std::fs::rename(dir.join(canonical_name(2)), dir.join(canonical_name(1))) + .expect("occupy conversion 1's canonical file"); + + let listed = store + .unresolved() + .expect("listing with a blocked conversion"); + assert_eq!( + listed.iter().map(|r| r.conversion_id).collect::>(), + vec![2], + "only the unblocked conversion is listed" + ); + let skipped = store.unreadable_entries(); + assert_eq!(skipped.len(), 1); + assert_eq!(skipped[0].path, dir.join(canonical_name(1))); + assert!( + skipped[0].reason.contains("blocked") && skipped[0].reason.contains("conversion 2"), + "unexpected skip reason: {}", + skipped[0].reason + ); + + // Recovery reconciles the unblocked conversion and never touches + // the blocked conversion's stranded copy. + let mut backend = funded_ledger(); + let outcomes = reconcile(&mut backend, &mut store, RecoveryPolicy::Abandon) + .expect("reconcile past the blocked conversion"); + assert_eq!(outcomes, vec![(2, ReconcileOutcome::Abandoned)]); + assert!(stray.exists(), "the blocked record's copy stays on disk"); + let _ = std::fs::remove_dir_all(&dir); +} From bc0a779b6687459f556af165643dc4323b38919c Mon Sep 17 00:00:00 2001 From: Drewry Pope Date: Mon, 2 Nov 2026 18:36:56 -0600 Subject: [PATCH 4/4] fix(id): scan casing aliases of intent files and index canonical occupants (#4578) --- pkgs/id/src/currency_poc/intent.rs | 106 +++++++++++++++++++++++++++-- pkgs/id/src/currency_poc/tests.rs | 58 ++++++++++++++++ 2 files changed, 159 insertions(+), 5 deletions(-) diff --git a/pkgs/id/src/currency_poc/intent.rs b/pkgs/id/src/currency_poc/intent.rs index 635d22529..2af73c1a4 100644 --- a/pkgs/id/src/currency_poc/intent.rs +++ b/pkgs/id/src/currency_poc/intent.rs @@ -189,6 +189,12 @@ impl IntentStore for InMemoryIntentStore { /// conversion's record is blocked — `unresolved` skips and reports /// it like a conflicting copy instead of handing recovery a record /// whose state-advancing `put` is guaranteed to fail. +/// The next re-review round added: the scan's extension match is +/// case-insensitive (an uppercase-extension alias is scanned, never +/// invisible), the unreadable-canonical check in `get` matches +/// filesystem aliases and casing variants of the canonical name, +/// and occupant resolution goes through a per-scan index instead +/// of pairwise `same_file` calls (linear, not quadratic, I/O). pub struct FileIntentStore { dir: PathBuf, /// Entries skipped by the most recent `unresolved` listing @@ -230,7 +236,17 @@ impl FileIntentStore { paths.sort(); let mut scanned = Vec::new(); for path in paths { - if path.extension().and_then(|ext| ext.to_str()) != Some("json") { + // The extension match is case-insensitive (Codex P1 on + // PR #4622): on a case-insensitive filesystem a file + // named with an uppercase .JSON extension can BE a + // canonical intent file, and excluding it from the scan + // would hide its conversion's record from every guard + // this contract backs. + let is_json = path + .extension() + .and_then(|ext| ext.to_str()) + .is_some_and(|ext| ext.eq_ignore_ascii_case("json")); + if !is_json { continue; } // fs::read also fails for a subdirectory named *.json; @@ -255,9 +271,34 @@ impl FileIntentStore { } } unreadable.sort_by(|a, b| a.path.cmp(&b.path)); + // Occupant index (Codex P2 on PR #4622): resolve every + // entry's filesystem identity ONCE here - its own path, its + // canonicalized path, and (on Unix) its device+inode - so + // entry_at is a map lookup per query. The previous shape + // called same_file() (canonicalize + metadata syscalls) for + // every (entry, query) pair, which made unresolved() + // quadratic in the number of stored records even when the + // listing came back empty. + let mut by_path: HashMap = HashMap::new(); + let mut by_file_id: HashMap<(u64, u64), usize> = HashMap::new(); + for (index, entry) in scanned.iter().enumerate() { + by_path.entry(entry.path.clone()).or_insert(index); + if let Ok(canon) = fs::canonicalize(&entry.path) { + by_path.entry(canon).or_insert(index); + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + if let Ok(meta) = fs::metadata(&entry.path) { + by_file_id.entry((meta.dev(), meta.ino())).or_insert(index); + } + } + } Ok(IntentScan { entries: scanned, unreadable, + by_path, + by_file_id, }) } } @@ -272,6 +313,12 @@ struct ScannedIntent { struct IntentScan { entries: Vec, unreadable: Vec, + /// Occupant index: path (as scanned, or canonicalized) -> entry. + by_path: HashMap, + /// Occupant index: (device, inode) -> entry; populated on Unix, + /// empty elsewhere. Covers hardlinks, whose canonical paths + /// still differ in spelling. + by_file_id: HashMap<(u64, u64), usize>, } impl IntentScan { @@ -291,9 +338,58 @@ impl IntentScan { /// restore), and that occupancy is a fact about the path, not /// about the content identity `copies_for` groups by. fn entry_at(&self, path: &Path) -> Option<&ScannedIntent> { - self.entries - .iter() - .find(|entry| entry.path == path || same_file(&entry.path, path)) + if let Some(&index) = self.by_path.get(path) { + return Some(&self.entries[index]); + } + if let Ok(canon) = fs::canonicalize(path) { + if let Some(&index) = self.by_path.get(&canon) { + return Some(&self.entries[index]); + } + } + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + if let Ok(meta) = fs::metadata(path) { + if let Some(&index) = self.by_file_id.get(&(meta.dev(), meta.ino())) { + return Some(&self.entries[index]); + } + } + } + // Lexical alias: a file whose NAME differs from the query + // path's name only in casing spells the same canonical name + // (the hex stem is case-insensitive, like the extension), + // so it is the canonical occupant even where the filesystem + // keeps the spellings distinct. String compares only - no + // syscalls - so this never reintroduces the quadratic I/O + // the index above removed. + let name = path.file_name()?; + self.entries.iter().find(|entry| { + entry.path.file_name().is_some_and(|other| { + other + .to_string_lossy() + .eq_ignore_ascii_case(&name.to_string_lossy()) + }) + }) + } + + /// The unreadable entry sitting at `path`, with the same alias + /// matching `entry_at` applies (Codex P2 on PR #4622): an + /// unreadable canonical file spelled with different casing - + /// uppercase hex digits, say - is still this conversion's + /// canonical file, so `get` must fail closed on it instead of + /// treating the conversion as absent. + fn unreadable_at(&self, path: &Path) -> Option<&UnreadableIntent> { + self.unreadable.iter().find(|entry| { + if entry.path == path || same_file(&entry.path, path) { + return true; + } + match (entry.path.file_name(), path.file_name()) { + (Some(a), Some(b)) => a + .to_string_lossy() + .eq_ignore_ascii_case(&b.to_string_lossy()), + _ => false, + } + }) } /// The one distinct record among `copies`, or `None` when the @@ -497,7 +593,7 @@ impl IntentStore for FileIntentStore { // NON-canonical file is a #4125 skipped entry, not this // conversion's canonical record.) let canonical = self.path_for(conversion_id); - if let Some(bad) = scan.unreadable.iter().find(|entry| entry.path == canonical) { + if let Some(bad) = scan.unreadable_at(&canonical) { return Err(io::Error::new( io::ErrorKind::InvalidData, format!( diff --git a/pkgs/id/src/currency_poc/tests.rs b/pkgs/id/src/currency_poc/tests.rs index 9844da84f..d8a37b340 100644 --- a/pkgs/id/src/currency_poc/tests.rs +++ b/pkgs/id/src/currency_poc/tests.rs @@ -921,6 +921,64 @@ fn conflicting_copies_fail_closed_and_are_reported() { let _ = std::fs::remove_dir_all(&dir); } +// --- #4578 / PR #4622 Codex re-review: casing aliases of the +// --- canonical name (P1/P2) --- + +#[test] +fn uppercase_extension_alias_is_scanned_and_blocks_conflicting_put() { + // A record stored under the canonical stem with an UPPERCASE + // extension is still the conversion's record (Codex P1 on + // PR #4622): the scan must not skip it, or get() reads the + // conversion as absent and a conflicting put overwrites it. + let (dir, mut store) = empty_file_store("upext4578"); + let original = bare_record( + 10, + vec![funding_transfer(501, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + store.put(&original).expect("seed put"); + let alias = dir.join(format!("{:032x}.JSON", 10)); + std::fs::rename(dir.join(canonical_name(10)), &alias).expect("rename to .JSON alias"); + + assert_eq!(store.get(10).expect("get via .JSON alias"), Some(original.clone())); + let listed = store.unresolved().expect("listing via .JSON alias"); + assert_eq!(listed, vec![original.clone()]); + + let conflicting = bare_record( + 10, + vec![funding_transfer(601, ISSUER_SRC, USER_SRC, 10_000, SRC)], + ); + let error = store + .put(&conflicting) + .expect_err("conflicting put over the .JSON alias must fail closed"); + assert!( + error.to_string().contains("refusing to write conversion 10"), + "unexpected put error: {error}" + ); + // The alias still holds the original record, byte for byte. + assert_eq!(store.get(10).expect("get after refusal"), Some(original)); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn unreadable_uppercase_canonical_spelling_fails_get_closed() { + // A corrupt file at the canonical name spelled with UPPERCASE + // hex digits is still the conversion's canonical file (Codex + // P2 on PR #4622): get() must fail closed on it, not read the + // conversion as absent and let prepare() overwrite it. + let (dir, store) = empty_file_store("uphex4578"); + let alias = dir.join(format!("{:032X}.json", 10)); + std::fs::write(&alias, b"{ not an intent record").expect("plant corrupt alias"); + + let error = store + .get(10) + .expect_err("get with an unreadable uppercase canonical file must fail closed"); + assert!( + error.to_string().contains("is unreadable"), + "unexpected get error: {error}" + ); + let _ = std::fs::remove_dir_all(&dir); +} + // --- #4578 / PR #4622 Codex findings: canonical occupancy (P1) and // --- state-only divergence after an interrupted canonicalization (P2)