mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-24 10:09:01 -06:00
Library lock: Don't write through a link at the owner note's name
LibraryLock::acquire wrote catalog.lock.owner with std::fs::write, which follows a symlink at that name. A library folder someone else prepared (a shared drive, an unpacked archive, a synced folder) could carry catalog.lock.owner -> ~/.bashrc or any other file the user can write, and opening the library replaced that file's content with the owner JSON. The catalog's staging files had the same problem and were fixed in #1502; the owner note was missed. The note is now written through store::create_staging (made pub(crate)): whatever is at the name is removed (a link itself, never its target) and the file is created with create_new. Drop already removed the note with remove_file, which never follows a link. Test (Unix): a link at catalog.lock.owner pointing at a file outside the library leaves that file's bytes unchanged through acquire and drop, and the note lands in the library as a regular file naming this process. Reported-by: BS.ehlas.cz
This commit is contained in:
@@ -25,7 +25,7 @@
|
||||
//! - Browser storage (OPFS) has no file locks: see the web host for its own guard.
|
||||
|
||||
use std::fs::{File, OpenOptions, TryLockError};
|
||||
use std::io;
|
||||
use std::io::{self, Write};
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
/// The lock file in the library directory.
|
||||
@@ -116,9 +116,11 @@ impl LibraryLock {
|
||||
}
|
||||
Err(TryLockError::Error(e)) => return Ok(unlocked(e)),
|
||||
}
|
||||
// A new file in place of whatever is at the name: a link planted there in a library folder
|
||||
// someone else prepared is removed, never written through (as for the staging files, #1502).
|
||||
let owner = LockOwner::this_process(program);
|
||||
if let Ok(json) = serde_json::to_vec_pretty(&owner)
|
||||
&& let Err(e) = std::fs::write(dir.join(OWNER), json)
|
||||
&& let Err(e) = crate::store::create_staging(&dir.join(OWNER)).and_then(|mut f| f.write_all(&json))
|
||||
{
|
||||
log::warn!("library {}: can't write {OWNER}: {e}", dir.display());
|
||||
}
|
||||
|
||||
@@ -75,8 +75,8 @@ impl FsStore {
|
||||
|
||||
/// 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).
|
||||
fn create_staging(tmp: &std::path::Path) -> io::Result<std::fs::File> {
|
||||
/// 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)?;
|
||||
}
|
||||
|
||||
@@ -62,6 +62,28 @@ fn leftover_lock_files_are_not_a_lock() {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// A link planted at the owner note's name (`catalog.lock.owner`) in a library folder someone else
|
||||
/// prepared is removed, never written through: the file it points to keeps its bytes, and the note
|
||||
/// still lands in the library (as for the catalog's staging files, issue #1502).
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn a_link_at_the_owner_note_is_never_written_through() {
|
||||
let _serial = serial();
|
||||
let dir = temp_dir("owner-link");
|
||||
let victim = dir.join("victim.txt");
|
||||
std::fs::write(&victim, b"keep").unwrap();
|
||||
std::os::unix::fs::symlink(&victim, dir.join(OWNER)).unwrap();
|
||||
let l = LibraryLock::acquire(&dir, "LightCraft").unwrap();
|
||||
assert!(l.held());
|
||||
assert_eq!(std::fs::read(&victim).unwrap(), b"keep");
|
||||
assert!(!std::fs::symlink_metadata(dir.join(OWNER)).unwrap().file_type().is_symlink(), "the link was replaced by the note");
|
||||
let owner: LockOwner = serde_json::from_slice(&std::fs::read(dir.join(OWNER)).unwrap()).unwrap();
|
||||
assert_eq!(owner.pid, std::process::id());
|
||||
drop(l);
|
||||
assert_eq!(std::fs::read(&victim).unwrap(), b"keep");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// Another process holds the lock; when it is killed (a crash), the OS releases it.
|
||||
#[test]
|
||||
fn lock_held_by_another_process_is_released_when_it_dies() {
|
||||
|
||||
@@ -1,5 +1,12 @@
|
||||
# What's new in LightCraft
|
||||
|
||||
## 2026-10-22
|
||||
|
||||
### Reliability
|
||||
- Opening a library no longer writes through a link left at the lock's owner note (`catalog.lock.owner`): the link
|
||||
is removed and a new note made in its place, so the file it pointed to is never changed (as for the catalog's
|
||||
temporary files, issue #1502).
|
||||
|
||||
## 2026-10-21
|
||||
|
||||
### Sharper resampling in HDR and panorama merges
|
||||
|
||||
Reference in New Issue
Block a user