Files
lightcraft/crates/catalog/src/store.rs
T
Dylan M. TaylorandCodebuff 4ae3089420 security: bounded keyword, repair and journal-recovery work (CodeRabbit) (#2763)
* security: harden dev-tool scripts and the testkit skip-log test (CodeRabbit)

- tools/install-sam3.sh: the HF bearer token went through curl's argv, where
  `ps` exposes it to other local accounts (CWE-214); it now travels through
  a private 0600 temp config file removed on exit.
- packaging/macos/import-cert.sh: the decoded .p12 was written to a
  predictable path in a shared TMPDIR, where a precreated attacker-readable
  file would receive it (CWE-377); both cert and keychain are created inside
  a mktemp -d 0700 directory now.
- docs/showcase/run.py: the reply reader copied whole responses with no size
  cap (obvious quadratic-copy and unbounded-memory DoS). It accumulates in a
  bytearray now and closes the connection past 128 MiB.
- tools/spectral_camera_table.py: alias/make/model strings and comments from
  the rawtoaces-data checkout moved directly into Rust source; one newline in
  a comment became Rust code. External text is now escaped as Rust literal
  content and comments are normalized to one printable line.
- crates/testkit: appended_lines_accumulate reused a pip-predictable temp
  dir by creating it with create_dir_all and carried on after failing to
  remove its log path, so the append would follow an attacker-planted
  symlink (CWE-59); it now fails the test if the directory exists instead of
  testing through it, and removes any existing log path only after clearly
  checking it, always failing the test instead of proceeding when removal fails.

Findings #10080/#10078 PR (rows 2, 6, 8, 9, 15 of the
storytold-lightcraft-agent-findings report).

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>

* security: bounded catalog keyword, repair, journal recovery work (CodeRabbit)

- keywords: from_file searched every hierarchical level for each flat keyword
  and `same` lower-cased two strings per comparison — one lowered level set
  instead (CWE-407, row 97).
- keywords: parse_keyword_list scanned each keyword's growing synonym list per
  line — a lowered-synonym set per keyword instead (CWE-407, row 100).
- keywords: keyword_tree stored every level's prefix twice, so a photo XMP
  keyword nested deeper than a list file accepts made stored bytes quadratic
  in the level count; the tree caps at MAX_LEVELS, and keyword_list_text names
  whatever lies deeper from the photos' and list's own spellings, so the file
  still reads back (CWE-400, row 103).
- catalog: repair validated a folder chain once per album — memoized by chain
  now, so a valid chain in a flat snapshot costs one walk overall (CWE-407,
  row 101).
- store: journal append and truncate follow a planted catalog.log symlink;
  both refuse any non-regular file at the name before writing through it
  (CWE-59, row 107), like the staging writes since #1502.
- journal: resync re-validated the whole remaining suffix UTF-8 for every
  candidate record start; a whole line is validated once (covering every
  candidate in it), and an invalid line's re-checks stop at a 64 MiB budget,
  with the tail then staying a torn one the damaged path keeps and reports
  (CWE-407, row 109).

New tests: from_file case-insensitive level match, synonyms kept once, a
4096-level keyword cut at MAX_LEVELS, a link at catalog.log refused, and
resync over a fragment of 1024 record starts.

Findings rows 97, 100, 101, 103, 107, 109 of the
storytold-lightcraft-agent-findings report.

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>

---------

Co-authored-by: Codebuff <noreply@codebuff.com>
2026-10-23 22:45:21 -04:00

308 lines
13 KiB
Rust

//! Byte storage for the catalog files (`catalog.snap`, `catalog.log`).
//!
//! [`FsStore`] is the native implementation (a library directory); [`MemStore`] keeps files in
//! memory (tests, ephemeral sessions) and lets tests simulate crashes by editing the bytes. A web
//! host can implement [`Store`] over OPFS.
use std::collections::BTreeMap;
use std::io;
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};
/// A flat namespace of named files with the few operations the journal needs.
pub trait Store: Send {
/// The whole file, or `None` if it doesn't exist.
fn read(&mut self, name: &str) -> io::Result<Option<Vec<u8>>>;
/// Replace a file atomically: after a crash either the old or the new content is visible,
/// never a mix (native: write temp + fsync + rename + fsync directory).
fn write_atomic(&mut self, name: &str, data: &[u8]) -> io::Result<()>;
/// [`Store::write_atomic`] with streamed content: `fill` writes the new content into the
/// given writer (an error aborts and leaves the old file). Same durability: the file is
/// replaced only once all of it is written (native: and fsynced). Returns its length.
///
/// The default buffers the content in memory; [`FsStore`] streams it into the temp file, so
/// a large snapshot is never held in memory as a whole.
fn write_atomic_with(&mut self, name: &str, fill: &mut dyn FnMut(&mut dyn io::Write) -> io::Result<()>) -> io::Result<u64> {
let mut buf = Vec::new();
fill(&mut buf)?;
self.write_atomic(name, &buf)?;
Ok(buf.len() as u64)
}
/// Append and make durable (fsync) before returning. On an error, part of `data` may have
/// been written (a full disk, a dropped network share): the journal cuts the file back to its
/// previous length with [`Store::truncate`] before appending again.
fn append(&mut self, name: &str, data: &[u8]) -> io::Result<()>;
/// Cut a file to `len` bytes (drop a torn tail before appending again). A missing file is
/// left missing.
fn truncate(&mut self, name: &str, len: u64) -> io::Result<()>;
/// Human-readable location (diagnostics).
fn describe(&self) -> String;
/// Another handle on the same files that a worker thread can write `catalog.snap` through
/// while this one keeps appending to the log (background compaction). `None` (the default):
/// snapshots are written synchronously.
fn background_writer(&self) -> Option<Box<dyn Store>> {
None
}
}
/// Appends and truncates open the name's own file, so anything else at the name is the library's
/// problem: a link planted there redirects the write outside the directory (CWE-59), and a device
/// hangs the open. Refused with an error the open can show, before anything is written through.
fn check_regular_directory_entry(path: &Path) -> io::Result<()> {
match std::fs::symlink_metadata(path) {
Ok(md) if md.is_file() => Ok(()),
Ok(md) => Err(io::Error::other(format!("{} is a {:?}, not a regular file", path.display(), md.file_type()))),
Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()),
Err(e) => Err(e),
}
}
/// Files in a directory.
pub struct FsStore {
dir: PathBuf,
/// Open append handle (kept between appends; dropped on rewrite/truncate).
appender: Option<(String, std::fs::File)>,
}
impl FsStore {
/// Use `dir` (created if missing).
pub fn open(dir: impl AsRef<Path>) -> io::Result<FsStore> {
std::fs::create_dir_all(dir.as_ref())?;
Ok(FsStore { dir: dir.as_ref().to_path_buf(), appender: None })
}
pub fn dir(&self) -> &Path {
&self.dir
}
fn path(&self, name: &str) -> PathBuf {
self.dir.join(name)
}
fn sync_dir(&self) {
// Makes the rename durable on POSIX; opening a directory fails on Windows (where
// `rename` is already journaled by NTFS), so errors are ignored.
if let Ok(d) = std::fs::File::open(&self.dir) {
let _ = d.sync_all();
}
}
}
/// A new staging file at `tmp`: whatever is already there (a leftover from a crashed save, or a link) is removed first
/// (a link itself, never what it points to), then the file is created exclusively, so a write never follows a link
/// planted at that name into another file (issue #1502). Also writes the lock's owner note.
pub(crate) fn create_staging(tmp: &std::path::Path) -> io::Result<std::fs::File> {
if std::fs::symlink_metadata(tmp).is_ok() {
std::fs::remove_file(tmp)?;
}
std::fs::OpenOptions::new().write(true).create_new(true).open(tmp)
}
impl Store for FsStore {
fn read(&mut self, name: &str) -> io::Result<Option<Vec<u8>>> {
match std::fs::read(self.path(name)) {
Ok(b) => Ok(Some(b)),
Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(None),
Err(e) => Err(e),
}
}
fn write_atomic(&mut self, name: &str, data: &[u8]) -> io::Result<()> {
use std::io::Write;
if self.appender.as_ref().is_some_and(|(n, _)| n == name) {
self.appender = None;
}
let tmp = self.path(&format!("{name}.tmp"));
{
let mut f = create_staging(&tmp)?;
f.write_all(data)?;
f.sync_all()?;
}
std::fs::rename(&tmp, self.path(name))?;
self.sync_dir();
Ok(())
}
fn write_atomic_with(&mut self, name: &str, fill: &mut dyn FnMut(&mut dyn io::Write) -> io::Result<()>) -> io::Result<u64> {
use std::io::Write;
if self.appender.as_ref().is_some_and(|(n, _)| n == name) {
self.appender = None;
}
let tmp = self.path(&format!("{name}.tmp"));
let written = (|| {
let mut w = Counter { inner: io::BufWriter::with_capacity(1 << 20, create_staging(&tmp)?), n: 0 };
fill(&mut w)?;
w.flush()?;
let f = w.inner.into_inner().map_err(|e| e.into_error())?;
f.sync_all()?;
Ok(w.n)
})();
let n = match written {
Ok(n) => n,
Err(e) => {
let _ = std::fs::remove_file(&tmp);
return Err(e);
}
};
std::fs::rename(&tmp, self.path(name))?;
self.sync_dir();
Ok(n)
}
fn append(&mut self, name: &str, data: &[u8]) -> io::Result<()> {
use std::io::Write;
if self.appender.as_ref().is_none_or(|(n, _)| n != name) {
check_regular_directory_entry(&self.path(name))?;
let existed = self.path(name).exists();
let f = std::fs::OpenOptions::new().create(true).append(true).open(self.path(name))?;
if !existed {
self.sync_dir();
}
self.appender = Some((name.to_string(), f));
}
let Some((_, f)) = self.appender.as_mut() else { return Err(io::Error::other("appender not open")) };
let written = f.write_all(data).and_then(|()| f.sync_data());
if written.is_err() {
// The handle may be dead (a network share that reconnected, a removed volume): the next
// attempt reopens the file. Part of `data` may be in the file; the caller cuts it back
// ([`Store::truncate`]) before appending again.
self.appender = None;
}
written
}
fn truncate(&mut self, name: &str, len: u64) -> io::Result<()> {
if self.appender.as_ref().is_some_and(|(n, _)| n == name) {
self.appender = None;
}
check_regular_directory_entry(&self.path(name))?;
let f = match std::fs::OpenOptions::new().write(true).open(self.path(name)) {
Ok(f) => f,
// nothing to cut (an append that failed before creating the file)
Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(()),
Err(e) => return Err(e),
};
f.set_len(len)?;
f.sync_all()
}
fn describe(&self) -> String {
self.dir.display().to_string()
}
#[cfg(not(target_arch = "wasm32"))]
fn background_writer(&self) -> Option<Box<dyn Store>> {
Some(Box::new(FsStore { dir: self.dir.clone(), appender: None }))
}
}
/// Counts the bytes written through it.
struct Counter<W> {
inner: W,
n: u64,
}
impl<W: io::Write> io::Write for Counter<W> {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
let k = self.inner.write(buf)?;
self.n += k as u64;
Ok(k)
}
fn flush(&mut self) -> io::Result<()> {
self.inner.flush()
}
}
/// In-memory files. Clones share the same files (so a test can "reopen" or corrupt them).
#[derive(Clone, Default)]
pub struct MemStore {
pub files: Arc<Mutex<BTreeMap<String, Vec<u8>>>>,
/// Offer a [`Store::background_writer`] (off by default: snapshots stay synchronous).
pub background: bool,
}
impl MemStore {
pub fn new() -> MemStore {
MemStore::default()
}
pub fn get(&self, name: &str) -> Option<Vec<u8>> {
self.files.lock().unwrap_or_else(|e| e.into_inner()).get(name).cloned()
}
pub fn set(&self, name: &str, data: Vec<u8>) {
self.files.lock().unwrap_or_else(|e| e.into_inner()).insert(name.to_string(), data);
}
}
impl Store for MemStore {
fn read(&mut self, name: &str) -> io::Result<Option<Vec<u8>>> {
Ok(self.get(name))
}
fn write_atomic(&mut self, name: &str, data: &[u8]) -> io::Result<()> {
self.set(name, data.to_vec());
Ok(())
}
fn append(&mut self, name: &str, data: &[u8]) -> io::Result<()> {
self.files.lock().unwrap_or_else(|e| e.into_inner()).entry(name.to_string()).or_default().extend_from_slice(data);
Ok(())
}
fn truncate(&mut self, name: &str, len: u64) -> io::Result<()> {
if let Some(f) = self.files.lock().unwrap_or_else(|e| e.into_inner()).get_mut(name) {
f.truncate(len as usize);
}
Ok(())
}
fn describe(&self) -> String {
"memory".into()
}
fn background_writer(&self) -> Option<Box<dyn Store>> {
self.background.then(|| Box::new(self.clone()) as Box<dyn Store>)
}
}
#[cfg(all(test, unix))]
mod tests_staging {
use super::*;
/// A link planted at a staging name (`<name>.tmp`) is removed, never followed: the file it points to keeps its
/// bytes and the save still lands (issue #1502).
#[cfg(unix)]
#[test]
fn a_link_at_the_staging_name_is_never_written_through() {
let dir = std::env::temp_dir().join(format!("lc-staging-link-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
let mut store = FsStore::open(&dir).unwrap();
let victim = dir.join("victim.txt");
std::fs::write(&victim, b"keep").unwrap();
std::os::unix::fs::symlink(&victim, dir.join("catalog.snap.tmp")).unwrap();
store.write_atomic("catalog.snap", b"{}").unwrap();
assert_eq!(std::fs::read(&victim).unwrap(), b"keep");
assert_eq!(std::fs::read(dir.join("catalog.snap")).unwrap(), b"{}");
std::os::unix::fs::symlink(&victim, dir.join("catalog.log.tmp")).unwrap();
store.write_atomic_with("catalog.log", &mut |w| w.write_all(b"[1]")).unwrap();
assert_eq!(std::fs::read(&victim).unwrap(), b"keep");
assert_eq!(std::fs::read(dir.join("catalog.log")).unwrap(), b"[1]");
let _ = std::fs::remove_dir_all(&dir);
}
/// The journal's append and truncate (which recovery uses) refuse a non-regular file at the
/// name instead of following it, so a link planted at `catalog.log` never receives library
/// bytes (CWE-59, Audit row 107); the snapshot write already removes such a link (#1502).
#[test]
fn append_and_truncate_refuse_a_link_at_the_name() {
let dir = std::env::temp_dir().join(format!("lc-log-link-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
let victim = dir.join("victim.txt");
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(&victim, b"keep").unwrap();
std::os::unix::fs::symlink(&victim, dir.join("catalog.log")).unwrap();
let mut store = FsStore::open(&dir).unwrap();
assert!(store.append("catalog.log", b"appended").is_err());
assert!(store.truncate("catalog.log", 0).is_err());
assert_eq!(std::fs::read(&victim).unwrap(), b"keep");
assert!(std::fs::symlink_metadata(dir.join("catalog.log")).unwrap().file_type().is_symlink());
// A regular file keeps working.
std::fs::remove_file(dir.join("catalog.log")).unwrap();
std::fs::write(dir.join("catalog.log"), b"abc").unwrap();
store.truncate("catalog.log", 1).unwrap();
assert_eq!(std::fs::read(dir.join("catalog.log")).unwrap(), b"a");
let _ = std::fs::remove_dir_all(&dir);
}
}