From e25ffe8b79700ccb96c276377afec601900e9347 Mon Sep 17 00:00:00 2001 From: nymius <155548262+nymius@users.noreply.github.com> Date: Mon, 10 Aug 2026 15:57:57 -0300 Subject: [PATCH 1/2] refactor(file_store)!: replace bincode by postcard `bincode` is no longer maintained. `postcard` is the closest maintained project with +52M downloads on crates.io, regular releases, and activity on its repo. Concurrent access to the file store is ruled out from support, unless for the current change. BREAKING CHANGE: - Magic Bytes should be changed to avoid accidentaly modifying old file store blobs. - The internal encoding has changed as a result of using postcard. Old file stores won't be recoverable using the latest file_store version. - As this is a development environment store, we don't provide migration utilities. - `StoreError::Bincode` has been renamed to `StoreError::Decode`, and now contains `postcard::Error`s - From now on, trailing bytes after decoding are rejected. - `append` attaches changesets to the current file position, assumed to be the EOF of a well written changeset, rolls back file pointer on failure. --- crates/file_store/Cargo.toml | 4 +- crates/file_store/src/entry_iter.rs | 238 +++++++++++++++++++++++++--- crates/file_store/src/lib.rs | 11 +- crates/file_store/src/store.rs | 227 +++++++++++++++++++++++--- 4 files changed, 421 insertions(+), 59 deletions(-) diff --git a/crates/file_store/Cargo.toml b/crates/file_store/Cargo.toml index 8fbdc358de..8e14769b0e 100644 --- a/crates/file_store/Cargo.toml +++ b/crates/file_store/Cargo.toml @@ -16,8 +16,8 @@ workspace = true [dependencies] bdk_core = { path = "../core", version = "0.6.1", features = ["serde"]} -bincode = { version = "1" } -serde = { version = "1", features = ["derive"] } +postcard = { version = "1.1", default-features = false, features = ["use-std"] } +serde = { version = "1", default-features = false, features = ["derive"] } [dev-dependencies] tempfile = "3" diff --git a/crates/file_store/src/entry_iter.rs b/crates/file_store/src/entry_iter.rs index 8b284f1814..1c96d40f60 100644 --- a/crates/file_store/src/entry_iter.rs +++ b/crates/file_store/src/entry_iter.rs @@ -1,18 +1,18 @@ use crate::StoreError; -use bincode::Options; use std::{ fs::File, - io::{self, BufReader, Seek}, + io::{self, BufRead, BufReader, Read, Seek}, marker::PhantomData, }; -use crate::bincode_options; - /// Iterator over entries in a file store. /// /// Reads and returns an entry each time [`next`] is called. If an error occurs while reading the /// iterator will yield a `Result::Err(_)` instead and then `None` for the next call to `next`. /// +/// Each entry is stored as a `postcard`-encoded `u64` varint length prefix followed by that many +/// bytes of `postcard`-encoded data. +/// /// [`next`]: Self::next pub struct EntryIter<'t, T> { /// Buffered reader around the file @@ -44,31 +44,132 @@ where if self.finished { return None; } - (|| { - if let Some(start) = self.start_pos.take() { - self.db_file.seek(io::SeekFrom::Start(start))?; + match self.read_entry() { + Ok(entry) => entry.map(Ok), + Err(e) => { + self.finished = true; + Some(Err(e)) + } + } + } +} + +impl EntryIter<'_, T> +where + T: serde::de::DeserializeOwned, +{ + /// Reads the next entry, or `Ok(None)` on clean end-of-file. + /// + /// On error the file is rewound to the start of the failed entry, so it isn't left mid-entry. + fn read_entry(&mut self) -> Result, StoreError> { + if let Some(start) = self.start_pos.take() { + self.db_file.seek(io::SeekFrom::Start(start))?; + } + let pos_before_read = self.db_file.stream_position()?; + + // An empty buffer here is a clean end-of-file, not a torn entry. Done before the rewind + // scope below because a failed peek consumes nothing. + if self.db_file.fill_buf()?.is_empty() { + return Ok(None); + } + + let entry = self.read_frame(); + if entry.is_err() { + // Leave the file at the start of the failed entry. + self.db_file.seek(io::SeekFrom::Start(pos_before_read))?; + } + entry.map(Some) + } + + /// Reads a single frame. + /// + /// A frame is a `postcard` varint length prefix followed by that many bytes of + /// `postcard`-encoded data. + fn read_frame(&mut self) -> Result { + let len = self.read_len_prefix()?; + let payload_start = self.db_file.stream_position()?; + let payload = self.read_payload(len, payload_start)?; + decode_frame(&payload) + } + + /// Reads the frame length prefix. + /// + /// The varint length prefix is a `postcard`-encoded `u64`, at most 10 bytes, where the high bit + /// of each byte (0x80 mask) is the continuation flag. + fn read_len_prefix(&mut self) -> Result { + let mut buf = [0_u8; 10]; + + for (i, byte) in buf.iter_mut().enumerate() { + if self.db_file.read(std::slice::from_mut(byte))? == 0 { + // Prefix cut short by end-of-file: a torn entry. + return Err(StoreError::Decode( + postcard::Error::DeserializeUnexpectedEnd, + )); } - let pos_before_read = self.db_file.stream_position()?; - match bincode_options().deserialize_from(&mut self.db_file) { - Ok(changeset) => Ok(Some(changeset)), - Err(e) => { - self.finished = true; - let pos_after_read = self.db_file.stream_position()?; - // allow unexpected EOF if 0 bytes were read - if let bincode::ErrorKind::Io(inner) = &*e { - if inner.kind() == io::ErrorKind::UnexpectedEof - && pos_after_read == pos_before_read - { - return Ok(None); - } - } - self.db_file.seek(io::SeekFrom::Start(pos_before_read))?; - Err(StoreError::Bincode(*e)) - } + if *byte & 0x80 == 0 { + return postcard::from_bytes(&buf[..=i]).map_err(StoreError::Decode); } - })() - .transpose() + } + + // Continuation flag still set after 10 bytes: not a valid u64 varint. + Err(StoreError::Decode( + postcard::Error::DeserializeUnexpectedEnd, + )) + } + + /// Reads `len` payload bytes into a fresh buffer. + fn read_payload(&mut self, len: u64, payload_start: u64) -> Result, StoreError> { + let mut payload = Vec::new(); + // Reserve exactly `len` bytes up front. Fail fast on a corrupt, oversized length prefix. + // Avoids unnecessary reads and allocations. + let alloc_failed = match usize::try_from(len) { + Ok(len) => payload.try_reserve_exact(len).is_err(), + Err(_) => true, + }; + if alloc_failed { + return Err(self.alloc_failure_error(len, payload_start)); + } + + let bytes_read = (&mut self.db_file).take(len).read_to_end(&mut payload)?; + if bytes_read as u64 != len { + return Err(StoreError::Decode( + postcard::Error::DeserializeUnexpectedEnd, + )); + } + Ok(payload) + } + + /// Discover the kind of allocation error. + /// + /// This only runs after `len` has already failed to allocate, so it is a big number but not + /// necessarily corrupt. Here we distinguish whether it exceeds the bytes actually remaining + /// in the file (a decode error), or if it fits within the file but exceeds what this + /// machine can allocate right now (an environment failure, not a format one). + fn alloc_failure_error(&self, len: u64, payload_start: u64) -> StoreError { + let remaining = self + .db_file + .get_ref() + .metadata() + .map(|m| m.len().saturating_sub(payload_start)) + .unwrap_or(0); + if len > remaining { + StoreError::Decode(postcard::Error::DeserializeUnexpectedEnd) + } else { + StoreError::Io(io::Error::other("failed to allocate memory for entry")) + } + } +} + +/// Decodes one framed payload. +/// +/// The length prefix stays authoritative for framing, so bytes left over after decoding are +/// corruption, not a format `postcard` has a dedicated variant for. +fn decode_frame(payload: &[u8]) -> Result { + match postcard::take_from_bytes(payload) { + Ok((changeset, [])) => Ok(changeset), + Ok(_) => Err(StoreError::Decode(postcard::Error::SerdeDeCustom)), + Err(e) => Err(StoreError::Decode(e)), } } @@ -81,3 +182,88 @@ impl Drop for EntryIter<'_, T> { } } } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod test { + use super::*; + + // A single `0x80` byte is a varint continuation flag with no terminating byte: the length + // prefix is cut short by end-of-file, i.e. a torn entry. + fn torn_prefix_file() -> tempfile::NamedTempFile { + let mut file = tempfile::NamedTempFile::new().unwrap(); + std::io::Write::write_all(file.as_file_mut(), &[0x80]).unwrap(); + file + } + + // The iterator yields the error for a torn length prefix, then is fused: subsequent calls to + // `next` return `None` (and the file is rewound to the start of the failed entry). + #[test] + fn next_returns_none_after_error() { + let mut file = torn_prefix_file(); + let mut iter = EntryIter::::new(0, file.as_file_mut()); + + match iter.next() { + Some(Err(StoreError::Decode(postcard::Error::DeserializeUnexpectedEnd))) => {} + unexpected => panic!("unexpected result: {unexpected:?}"), + } + assert_eq!(iter.db_file.stream_position().unwrap(), 0); + // subsequent calls to `next` return `None` + assert!(iter.next().is_none()); + // check twice + assert!(iter.next().is_none()); + } + + // A length prefix cut short by end-of-file is a torn entry, not a clean end-of-file. + #[test] + fn errors_on_truncated_length_prefix() { + let mut file = torn_prefix_file(); + let mut iter = EntryIter::::new(0, file.as_file_mut()); + + match iter.next() { + Some(Err(StoreError::Decode(postcard::Error::DeserializeUnexpectedEnd))) => {} + unexpected => panic!("unexpected result: {unexpected:?}"), + } + } + + // Ten bytes with the continuation flag still set is not a valid `u64` varint. + #[test] + fn errors_on_overlong_length_prefix() { + let mut file = tempfile::NamedTempFile::new().unwrap(); + std::io::Write::write_all(file.as_file_mut(), &[0xFF; 10]).unwrap(); + let mut iter = EntryIter::::new(0, file.as_file_mut()); + + match iter.next() { + Some(Err(StoreError::Decode(postcard::Error::DeserializeUnexpectedEnd))) => {} + unexpected => panic!("unexpected result: {unexpected:?}"), + } + } + + // A length prefix that fits within the file but is too large to allocate is an environment + // failure (`Io`), not a format one (`Decode`). + // + // `try_reserve_exact` allocates the full requested size, so a length larger than the + // machine's available memory fails. A sparse file of 1 TiB makes such a length fit within the + // file while still exceeding what any reasonable machine can allocate. Skipped on filesystems + // that cannot create large sparse files (e.g. tmpfs). + #[test] + fn errors_with_io_when_length_fits_but_allocation_fails() { + let mut file = tempfile::NamedTempFile::new().unwrap(); + let file_size = 1u64 << 40; // 1 TiB + assert!( + file.as_file_mut().set_len(file_size).is_ok(), + "Filesystem can't create a large sparse file (e.g. tmpfs); can't exercise this path." + ); + + // A length that fits within the file but is far too large to allocate. + let len = file_size - 1000; + std::io::Write::write_all(file.as_file_mut(), &postcard::to_allocvec(&len).unwrap()) + .unwrap(); + + let mut iter = EntryIter::::new(0, file.as_file_mut()); + match iter.next() { + Some(Err(StoreError::Io(_))) => {} + unexpected => panic!("unexpected result: {unexpected:?}"), + } + } +} diff --git a/crates/file_store/src/lib.rs b/crates/file_store/src/lib.rs index 3731d50309..0c91fc01ec 100644 --- a/crates/file_store/src/lib.rs +++ b/crates/file_store/src/lib.rs @@ -4,14 +4,9 @@ mod entry_iter; mod store; use std::io; -use bincode::{DefaultOptions, Options}; pub use entry_iter::*; pub use store::*; -pub(crate) fn bincode_options() -> impl bincode::Options { - DefaultOptions::new().with_varint_encoding() -} - /// Error that occurs due to problems encountered with the file. #[derive(Debug)] pub enum StoreError { @@ -19,8 +14,8 @@ pub enum StoreError { Io(io::Error), /// Magic bytes do not match what is expected. InvalidMagicBytes { got: Vec, expected: Vec }, - /// Failure to decode data from the file. - Bincode(bincode::ErrorKind), + /// Failure to decode an entry from the file. + Decode(postcard::Error), } impl core::fmt::Display for StoreError { @@ -34,7 +29,7 @@ impl core::fmt::Display for StoreError { match self { Self::Io(e) => write!(f, "io error while reading store file: {}", e), - Self::Bincode(e) => write!(f, "bincode error while decoding entry {}", e), + Self::Decode(e) => write!(f, "error while decoding store entry: {}", e), Self::InvalidMagicBytes { got, expected } => { write!(f, "invalid magic bytes: ")?; write!(f, "expected 0x")?; diff --git a/crates/file_store/src/store.rs b/crates/file_store/src/store.rs index 858b9d2cdf..8b62996704 100644 --- a/crates/file_store/src/store.rs +++ b/crates/file_store/src/store.rs @@ -1,16 +1,18 @@ -use crate::{bincode_options, EntryIter, StoreError}; +use crate::{EntryIter, StoreError}; use bdk_core::Merge; -use bincode::Options; use std::{ fmt::{self, Debug}, fs::{File, OpenOptions}, - io::{self, Read, Write}, + io::{self, Read, Seek, Write}, marker::PhantomData, path::Path, }; /// Persists an append-only list of changesets (`C`) to a single file. /// +/// A [`Store`] assumes exclusive access to its file: never have more than one handle open on the +/// same file at a time. +/// /// > ⚠ This is a development/testing database. It does not natively support backwards compatible /// > BDK version upgrades so should not be used in production. #[derive(Debug)] @@ -60,7 +62,7 @@ where /// /// If there exist changesets in the file, [`load`] will try to aggregate them in /// a single changeset to verify their integrity. If aggregation fails - /// [`StoreErrorWithDump`] will be returned with the [`StoreError::Bincode`] error variant in + /// [`StoreErrorWithDump`] will be returned with the [`StoreError::Decode`] error variant in /// its error field and the aggregated changeset so far in the changeset field. /// /// To get a new working file store from this error use [`Store::create`] and [`Store::append`] @@ -178,7 +180,7 @@ where /// /// If there exist changesets in the file, [`dump`] will try to aggregate them in a single /// changeset. If aggregation fails [`StoreErrorWithDump`] will be returned with the - /// [`StoreError::Bincode`] error variant in its error field and the aggregated changeset so + /// [`StoreError::Decode`] error variant in its error field and the aggregated changeset so /// far in the changeset field. /// /// [`dump`]: Store::dump @@ -226,28 +228,41 @@ where } } - /// Append a new changeset to the file. Does nothing if the changeset is empty. Truncation is - /// not needed because file pointer is always moved to the end of the last decodable data from - /// beginning to end. + /// Append a new changeset to the file. Does nothing if the changeset is empty. + /// + /// The changeset is written at the current file position, which always follows the last valid + /// entry: [`create`] leaves it after the magic bytes, [`load`] leaves it at the end of the + /// fully validated file, and each [`append`] leaves it after the frame it writes. + /// Because a [`Store`] assumes exclusive access to its file and [`load`] rejects files with + /// undecodable data (returning no [`Store`]), appends only ever land after valid data; see + /// [`load`] for the recovery procedure of broken files. /// - /// If multiple garbage writes are produced on the file, the next load will only retrieve the - /// first chunk of valid changesets. + /// If a write fails partway through, the partial frame is truncated before the error is + /// returned, so a failed append leaves the file unchanged. /// - /// If garbage data is written and then valid changesets, the next load will still only - /// retrieve the first chunk of valid changesets. The recovery of those valid changesets after - /// the garbage data is responsibility of the user. + /// [`create`]: Store::create + /// [`load`]: Store::load + /// [`append`]: Self::append pub fn append(&mut self, changeset: &C) -> Result<(), io::Error> { // no need to write anything if changeset is empty if changeset.is_empty() { return Ok(()); } - bincode_options() - .serialize_into(&mut self.db_file, changeset) - .map_err(|e| match *e { - bincode::ErrorKind::Io(error) => error, - unexpected_err => panic!("unexpected bincode error: {unexpected_err}"), - })?; + let bytes = postcard::to_allocvec(changeset).map_err(io::Error::other)?; + let len_bytes = postcard::to_allocvec(&(bytes.len() as u64)).map_err(io::Error::other)?; + + let start = self.db_file.stream_position()?; + + let result = self + .db_file + .write_all(&len_bytes) + .and_then(|()| self.db_file.write_all(&bytes)); + if let Err(e) = result { + // Roll back the partial frame so a failed append leaves no torn data behind. + let _ = self.db_file.set_len(start); + return Err(e); + } Ok(()) } @@ -369,7 +384,7 @@ mod test { match Store::::load(&TEST_MAGIC_BYTES, file_path) { Err(StoreErrorWithDump { changeset, - error: StoreError::Bincode(_), + error: StoreError::Decode(_), }) => { assert_eq!(changeset, Some(Box::new(test_changesets))) } @@ -397,7 +412,7 @@ mod test { match store.dump() { Err(StoreErrorWithDump { changeset, - error: StoreError::Bincode(_), + error: StoreError::Decode(_), }) => { assert_eq!(changeset, Some(Box::new(test_changesets))) } @@ -406,7 +421,7 @@ mod test { } #[test] - fn append() { + fn append_succeeds() { let temp_dir = tempfile::tempdir().unwrap(); let file_path = temp_dir.path().join("db_file"); @@ -474,7 +489,10 @@ mod test { TestChangeSet::from(["4".into(), "5".into(), "6".into()]), ]; let last_changeset = TestChangeSet::from(["7".into(), "8".into(), "9".into()]); - let last_changeset_bytes = bincode_options().serialize(&last_changeset).unwrap(); + let last_changeset_payload = postcard::to_allocvec(&last_changeset).unwrap(); + let mut last_changeset_bytes = + postcard::to_allocvec(&(last_changeset_payload.len() as u64)).unwrap(); + last_changeset_bytes.extend_from_slice(&last_changeset_payload); for short_write_len in 1..last_changeset_bytes.len() - 1 { let file_path = temp_dir.path().join(format!("{short_write_len}.dat")); @@ -599,4 +617,167 @@ mod test { // current position matches EOF assert_eq!(current_pointer, expected_pointer); } + + #[test] + fn load_does_not_panic_on_oversized_length_prefix() { + // Build a file whose varint length prefix decodes to `u64::MAX`. Without a guard on the + // length prefix, this would trigger an allocation of that many bytes, panicking/aborting + // instead of returning a graceful error. + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("db_file"); + + let mut bytes = Vec::new(); + bytes.extend_from_slice(&TEST_MAGIC_BYTES); + let huge_len_encoded: Vec = postcard::to_allocvec(&u64::MAX).unwrap(); + bytes.extend_from_slice(&huge_len_encoded); + + std::fs::write(&file_path, &bytes).unwrap(); + + let result = Store::::load(&TEST_MAGIC_BYTES, &file_path); + assert!( + result.is_err(), + "load should fail gracefully on oversized length prefix" + ); + } + + #[test] + fn load_fails_on_frame_with_trailing_bytes() { + // Craft a well-formed frame whose declared length is 2 bytes longer than the valid + // payload it contains. The length prefix stays authoritative for framing, but the + // payload itself doesn't consume its whole declared length, which must be treated as + // corruption rather than silently ignored. + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("db_file"); + + let changeset = TestChangeSet::from(["hello".to_string()]); + let payload = postcard::to_allocvec(&changeset).unwrap(); + + let mut bytes = TEST_MAGIC_BYTES.to_vec(); + bytes.extend_from_slice(&postcard::to_allocvec(&((payload.len() + 2) as u64)).unwrap()); + bytes.extend_from_slice(&payload); + bytes.extend_from_slice(&[0xaa, 0xbb]); + + fs::write(&file_path, bytes).expect("should write crafted store"); + + match Store::::load(&TEST_MAGIC_BYTES, &file_path) { + Err(StoreErrorWithDump { + error: StoreError::Decode(_), + .. + }) => {} + unexpected => panic!("unexpected result: {unexpected:?}"), + } + } + + #[test] + fn load_fails_on_genuinely_undecodable_payload() { + // Craft a frame with a correct length prefix but invalid payload (a string that claims 4 + // bytes which are not valid UTF-8). + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("db_file"); + + let payload = vec![0x01, 0x04, 0xff, 0xff, 0xff, 0xff]; + let mut bytes = TEST_MAGIC_BYTES.to_vec(); + bytes.extend_from_slice(&postcard::to_allocvec(&(payload.len() as u64)).unwrap()); + bytes.extend_from_slice(&payload); + + fs::write(&file_path, bytes).expect("should write crafted store"); + + match Store::::load(&TEST_MAGIC_BYTES, &file_path) { + Err(StoreErrorWithDump { + error: StoreError::Decode(postcard::Error::DeserializeBadUtf8), + .. + }) => {} + unexpected => panic!("unexpected result: {unexpected:?}"), + } + } + + #[test] + fn load_decodes_zero_width_changeset() { + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("db_file"); + let mut bytes = TEST_MAGIC_BYTES.to_vec(); + // A single, well-formed frame with a zero-length payload loads and returns. + bytes.extend_from_slice(&postcard::to_allocvec(&0u64).unwrap()); + fs::write(&file_path, bytes).expect("should write crafted store"); + + let (_, changeset) = Store::<()>::load(&TEST_MAGIC_BYTES, &file_path) + .expect("zero-width changeset should load successfully"); + assert!(changeset.is_some()); + } + + #[test] + fn load_roundtrips_at_varint_length_boundaries() { + // The varint length prefix is 1 byte for values < 128 and 2 bytes for values >= 128. + // Exercise both sides of that boundary: a payload of exactly 127 bytes (1-byte prefix) + // and exactly 128 bytes (2-byte prefix). + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("db_file"); + + // `TestChangeSet` (`BTreeSet`) with one entry encodes as: + // varint(1) [1 byte] + varint(str.len()) [1 byte, for str.len() < 128] + str bytes. + // i.e., base_len = 2 bytes + let base_len = postcard::to_allocvec(&TestChangeSet::from([String::new()])) + .unwrap() + .len(); + + let mut store = Store::::create(&TEST_MAGIC_BYTES, &file_path).unwrap(); + let mut changesets = Vec::new(); + for target_len in [127_usize, 128] { + let changeset = TestChangeSet::from(["x".repeat(target_len - base_len)]); + assert_eq!( + postcard::to_allocvec(&changeset).unwrap().len(), + target_len, + "test setup: payload should be exactly {target_len} bytes" + ); + store.append(&changeset).unwrap(); + changesets.push(changeset); + } + drop(store); + + let (_, aggregated) = Store::::load(&TEST_MAGIC_BYTES, &file_path).unwrap(); + let expected = changesets + .into_iter() + .reduce(|mut acc, cs| { + Merge::merge(&mut acc, cs); + acc + }) + .unwrap(); + assert_eq!(aggregated, Some(expected)); + } + + // A failed append must roll back the partial frame so the file is left unchanged. + // + // The write is forced to fail by swapping the store's private file handle for a read-only + // one: `write_all` then errors, the rollback (`set_len`) runs, and the original file is + // untouched. + #[test] + fn append_failure_leaves_file_unchanged() { + let temp_dir = tempfile::tempdir().unwrap(); + let file_path = temp_dir.path().join("db_file"); + let changeset = TestChangeSet::from(["one".to_string()]); + + let mut store = + Store::::create(&TEST_MAGIC_BYTES, &file_path).expect("must create"); + store.append(&changeset).expect("must append changeset"); + let bytes_before = fs::read(&file_path).expect("must read store file"); + + // Replace the writable handle with a read-only one so the next write fails. + store.db_file = fs::File::open(&file_path).expect("must open read-only handle"); + let result = store.append(&changeset); + + assert!( + result.is_err(), + "append through a read-only handle must fail" + ); + let bytes_after = fs::read(&file_path).expect("must read store file"); + assert_eq!( + bytes_before, bytes_after, + "failed append left torn data behind" + ); + + // The store still loads and recovers the original changeset. + let (_, recovered) = + Store::::load(&TEST_MAGIC_BYTES, &file_path).expect("must load"); + assert_eq!(recovered, Some(changeset)); + } } From a6e7d0c364950fe234cb167ecd6a36edf7be1dbc Mon Sep 17 00:00:00 2001 From: nymius <155548262+nymius@users.noreply.github.com> Date: Mon, 12 Oct 2026 18:28:42 -0300 Subject: [PATCH 2/2] fix(file_store): create store file with owner-only permissions on Unix --- crates/file_store/src/store.rs | 23 +++++++++++++++++++---- crates/file_store/tests/permissions.rs | 12 ++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 crates/file_store/tests/permissions.rs diff --git a/crates/file_store/src/store.rs b/crates/file_store/src/store.rs index 8b62996704..593631de98 100644 --- a/crates/file_store/src/store.rs +++ b/crates/file_store/src/store.rs @@ -31,17 +31,31 @@ where /// `magic` is the prefixed bytes to write to the new file. This will be checked when loading /// the [`Store`] in the future with [`load`]. /// + /// # File permissions + /// + /// The file may contain sensitive wallet data. On Unix, it is created readable and writable + /// only by its owner (`0o600`); on other platforms the default file permissions apply. + /// The caller is responsible for restricting access to the parent directory, which this method + /// does not modify. + /// /// [`load`]: Store::load pub fn create

(magic: &[u8], file_path: P) -> Result where P: AsRef, { - let mut f = OpenOptions::new() + let mut options = OpenOptions::new(); + options .create_new(true) .read(true) .write(true) - .truncate(true) - .open(file_path)?; + .truncate(true); + // Wallet changesets may be sensitive, so keep the new file to its owner only. + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + options.mode(0o600); + } + let mut f = options.open(file_path)?; f.write_all(magic)?; Ok(Self { magic_len: magic.len(), @@ -205,7 +219,8 @@ where /// Attempt to load existing [`Store`] file; create it if the file does not exist. /// - /// Internally, this calls either [`load`] or [`create`]. + /// Internally, this calls either [`load`] or [`create`]. New files are created with the + /// permissions described in [`create`]. /// /// [`load`]: Store::load /// [`create`]: Store::create diff --git a/crates/file_store/tests/permissions.rs b/crates/file_store/tests/permissions.rs new file mode 100644 index 0000000000..34dfa17647 --- /dev/null +++ b/crates/file_store/tests/permissions.rs @@ -0,0 +1,12 @@ +#[cfg(unix)] +#[test] +fn new_store_permissions() { + use std::{collections::BTreeSet, os::unix::fs::PermissionsExt}; + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("wallet.store"); + let _store = bdk_file_store::Store::>::create(b"test", &path).unwrap(); + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + + assert_eq!(mode & 0o077, 0, "store file mode: {mode:04o}"); +}