fixed fuse bugs from BUGS.md

- Durable scratch/staging publication and preservation on failure.
      - Transactional pending-operation supersession.
      - Conflict copies cannot overwrite older copies or lose the source.
      - Single-entry wipe protection and incomplete-scan aborts.
      - Failed local deletion retains its baseline.
      - Downloads are fsynced before baseline settlement.
      - Future database schemas are refused.
      - Event cursors advance only after successful invalidation and persistence.
      - IPC frame, timeout, and concurrency limits.
      - Atomic config saves; malformed configs are preserved.
      - Sensitive directories require correct ownership, real directories, and 0700.
      - Debian package now includes service and tray autostart files.
      - Release workflow enforces versions, tests, lint, formatting, and offline acceptance.
      - CI now runs offline filesystem acceptance.
This commit is contained in:
nvrl committed 2026-07-22 14:49:06 +02:00
1 parent a76f36bdff
commit 8be0f04b61
16 files changed
+976 -125

No files matched your search

+3
View File
@@ -44,6 +44,9 @@ jobs:
- name: Unit tests - name: Unit tests
run: cargo test --workspace --locked run: cargo test --workspace --locked
- name: Account-free filesystem acceptance
run: scripts/fuse-acceptance.sh --offline-only
- name: Acceptance runner safety guard - name: Acceptance runner safety guard
run: | run: |
if scripts/fuse-acceptance.sh /tmp >acceptance.out 2>&1; then if scripts/fuse-acceptance.sh /tmp >acceptance.out 2>&1; then
+23 -1
View File
@@ -35,11 +35,29 @@ jobs:
- name: Set up Rust - name: Set up Rust
uses: actions-rust-lang/setup-rust-toolchain@v1 uses: actions-rust-lang/setup-rust-toolchain@v1
with: with:
toolchain: stable toolchain: '1.96'
components: rustfmt, clippy
- name: Cache Cargo target - name: Cache Cargo target
uses: swatinem/rust-cache@v2 uses: swatinem/rust-cache@v2
- name: Verify tag and manifest versions
if: startsWith(github.ref, 'refs/tags/')
shell: bash
run: |
tag_version="${GITHUB_REF_NAME#v}"
manifest_version="$(sed -n 's/^version = "\([^"]*\)"/\1/p' Cargo.toml | head -n1)"
pkgbuild_version="$(sed -n 's/^pkgver=//p' packaging/PKGBUILD | head -n1)"
test "$tag_version" = "$manifest_version"
test "$tag_version" = "$pkgbuild_version"
- name: Release quality gates
run: |
cargo fmt --all -- --check
cargo clippy --workspace --all-targets -- -D warnings
cargo test --workspace --locked
scripts/fuse-acceptance.sh --offline-only
- name: Build release binaries - name: Build release binaries
run: | run: |
cargo build --release --locked cargo build --release --locked
@@ -76,6 +94,8 @@ jobs:
mkdir -p deb/usr/share/applications mkdir -p deb/usr/share/applications
mkdir -p deb/usr/share/icons/hicolor/scalable/apps mkdir -p deb/usr/share/icons/hicolor/scalable/apps
mkdir -p deb/usr/share/licenses/proton-drive-linux mkdir -p deb/usr/share/licenses/proton-drive-linux
mkdir -p deb/usr/lib/systemd/user
mkdir -p deb/etc/xdg/autostart
# Copy binaries # Copy binaries
cp target/release/pdfs deb/usr/bin/ cp target/release/pdfs deb/usr/bin/
@@ -85,7 +105,9 @@ jobs:
# Copy desktop integration files # Copy desktop integration files
cp packaging/io.narl.proton-drive-linux.desktop deb/usr/share/applications/ cp packaging/io.narl.proton-drive-linux.desktop deb/usr/share/applications/
cp packaging/io.narl.proton-drive-linux-tray.desktop deb/etc/xdg/autostart/
cp packaging/io.narl.proton-drive-linux.svg deb/usr/share/icons/hicolor/scalable/apps/ cp packaging/io.narl.proton-drive-linux.svg deb/usr/share/icons/hicolor/scalable/apps/
cp packaging/proton-drive.service deb/usr/lib/systemd/user/
cp LICENSE deb/usr/share/licenses/proton-drive-linux/ cp LICENSE deb/usr/share/licenses/proton-drive-linux/
# Calculate Installed-Size in KB for the control file # Calculate Installed-Size in KB for the control file
Generated
+1
View File
@@ -3538,6 +3538,7 @@ dependencies = [
"directories", "directories",
"ignore", "ignore",
"keyring", "keyring",
"libc",
"parking_lot", "parking_lot",
"proton-drive-rs", "proton-drive-rs",
"proton-sdk", "proton-sdk",
+1
View File
@@ -21,3 +21,4 @@ sha2.workspace = true
rusqlite.workspace = true rusqlite.workspace = true
keyring.workspace = true keyring.workspace = true
parking_lot.workspace = true parking_lot.workspace = true
libc.workspace = true
+122 -33
View File
@@ -241,7 +241,9 @@ impl ContentCache {
let recovery_dir = content_dir.join("recovery"); let recovery_dir = content_dir.join("recovery");
std::fs::create_dir_all(&recovery_dir)?; std::fs::create_dir_all(&recovery_dir)?;
Self::rescue_scratch(&scratch_dir, &recovery_dir); Self::rescue_scratch(&scratch_dir, &recovery_dir);
let _ = std::fs::remove_dir_all(&scratch_dir); // A failed rescue may leave the only durable copy here. Never clear the
// directory wholesale; discard only blobs that carry no fsync marker.
Self::discard_unmarked_scratch(&scratch_dir);
std::fs::create_dir_all(&scratch_dir)?; std::fs::create_dir_all(&scratch_dir)?;
// Staging, by contrast, is deliberately preserved across runs. // Staging, by contrast, is deliberately preserved across runs.
let staging_dir = content_dir.join("staging"); let staging_dir = content_dir.join("staging");
@@ -669,18 +671,24 @@ impl ContentCache {
pub fn create_scratch(&self) -> Result<(std::fs::File, PathBuf)> { pub fn create_scratch(&self) -> Result<(std::fs::File, PathBuf)> {
use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::atomic::{AtomicU64, Ordering};
static N: AtomicU64 = AtomicU64::new(0); static N: AtomicU64 = AtomicU64::new(0);
let path = self.scratch_dir.join(format!( loop {
"w-{}-{}", let path = self.scratch_dir.join(format!(
std::process::id(), "w-{}-{}-{}",
N.fetch_add(1, Ordering::Relaxed) std::process::id(),
)); now_millis(),
let file = std::fs::OpenOptions::new() N.fetch_add(1, Ordering::Relaxed)
.create(true) ));
.truncate(true) match std::fs::OpenOptions::new()
.read(true) .create_new(true)
.write(true) .read(true)
.open(&path)?; .write(true)
Ok((file, path)) .open(&path)
{
Ok(file) => return Ok((file, path)),
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
Err(e) => return Err(e.into()),
}
}
} }
/// The sidecar path for a scratch file, holding the [`StagedWrite`] that /// The sidecar path for a scratch file, holding the [`StagedWrite`] that
@@ -709,16 +717,23 @@ impl ContentCache {
/// have synced `scratch` first: a sidecar promising bytes that never reached /// have synced `scratch` first: a sidecar promising bytes that never reached
/// the disk is worse than no sidecar. /// the disk is worse than no sidecar.
pub fn mark_scratch_durable(&self, scratch: &Path, meta: &StagedWrite) -> Result<()> { pub fn mark_scratch_durable(&self, scratch: &Path, meta: &StagedWrite) -> Result<()> {
use std::io::Write as _;
let side = Self::scratch_sidecar(scratch); let side = Self::scratch_sidecar(scratch);
let tmp = side.with_extension("json.tmp"); let tmp = side.with_extension("json.tmp");
std::fs::write(&tmp, serde_json::to_vec(meta)?)?; let mut file = std::fs::OpenOptions::new()
.write(true)
.create(true)
.truncate(true)
.open(&tmp)?;
file.write_all(&serde_json::to_vec(meta)?)?;
file.sync_all()?;
drop(file);
std::fs::rename(&tmp, &side)?; std::fs::rename(&tmp, &side)?;
// The rename is what publishes the sidecar; fsync the directory so the // The rename is what publishes the sidecar; fsync the directory so the
// rename itself survives, not just the bytes it points at. // rename itself survives, not just the bytes it points at.
if let Some(dir) = side.parent() if let Some(dir) = side.parent() {
&& let Ok(d) = std::fs::File::open(dir) std::fs::File::open(dir)?.sync_all()?;
{
let _ = d.sync_all();
} }
Ok(()) Ok(())
} }
@@ -796,6 +811,21 @@ impl ContentCache {
} }
} }
fn discard_unmarked_scratch(scratch_dir: &Path) {
let Ok(entries) = std::fs::read_dir(scratch_dir) else {
return;
};
for entry in entries.flatten() {
let blob = entry.path();
if blob.extension().is_some_and(|e| e == "json") {
continue;
}
if !Self::scratch_sidecar(&blob).exists() {
let _ = std::fs::remove_file(blob);
}
}
}
/// Emergency eviction of unpinned blobs when local disk runs out of space (ENOSPC). /// Emergency eviction of unpinned blobs when local disk runs out of space (ENOSPC).
pub fn emergency_evict(&self) { pub fn emergency_evict(&self) {
let pinned: HashSet<String> = self let pinned: HashSet<String> = self
@@ -826,7 +856,8 @@ impl ContentCache {
/// Writes rescued from an unclean shutdown, as `(blob, meta)` pairs for the /// Writes rescued from an unclean shutdown, as `(blob, meta)` pairs for the
/// daemon to hand to the upload queue. A blob whose sidecar did not survive /// daemon to hand to the upload queue. A blob whose sidecar did not survive
/// the rescue is deleted here — nothing can address it. /// is not actionable automatically, but is retained for manual recovery:
/// unexplained bytes are never evidence that user data is disposable.
pub fn recovered_writes(&self) -> Vec<(PathBuf, StagedWrite)> { pub fn recovered_writes(&self) -> Vec<(PathBuf, StagedWrite)> {
let Ok(entries) = std::fs::read_dir(&self.recovery_dir) else { let Ok(entries) = std::fs::read_dir(&self.recovery_dir) else {
return Vec::new(); return Vec::new();
@@ -837,14 +868,11 @@ impl ContentCache {
if blob.extension().is_some_and(|e| e == "json") { if blob.extension().is_some_and(|e| e == "json") {
continue; continue;
} }
match std::fs::read(Self::scratch_sidecar(&blob)) if let Some(meta) = std::fs::read(Self::scratch_sidecar(&blob))
.ok() .ok()
.and_then(|b| serde_json::from_slice::<StagedWrite>(&b).ok()) .and_then(|b| serde_json::from_slice::<StagedWrite>(&b).ok())
{ {
Some(meta) => out.push((blob, meta)), out.push((blob, meta));
None => {
let _ = std::fs::remove_file(&blob);
}
} }
} }
out out
@@ -872,6 +900,8 @@ impl ContentCache {
/// failure here means we could not save the bytes at all, so it is reported /// failure here means we could not save the bytes at all, so it is reported
/// rather than swallowed. /// rather than swallowed.
pub fn stage_write(&self, meta: &StagedWrite, scratch: &Path) -> Result<PathBuf> { pub fn stage_write(&self, meta: &StagedWrite, scratch: &Path) -> Result<PathBuf> {
use std::io::Write as _;
static N: AtomicU64 = AtomicU64::new(0); static N: AtomicU64 = AtomicU64::new(0);
// The uid goes in the name so a staged file can be tied back to its node // The uid goes in the name so a staged file can be tied back to its node
// without a database; `/` in a uid would otherwise open a path. // without a database; `/` in a uid would otherwise open a path.
@@ -882,18 +912,32 @@ impl ContentCache {
now_millis(), now_millis(),
N.fetch_add(1, Ordering::Relaxed) N.fetch_add(1, Ordering::Relaxed)
)); ));
if let Err(e) = std::fs::rename(scratch, &path) { let copied = if let Err(e) = std::fs::rename(scratch, &path) {
// Cross-device: rename cannot move it, so copy and drop the original. if e.kind() != std::io::ErrorKind::CrossesDevices {
let _ = e; return Err(e.into());
}
std::fs::copy(scratch, &path)?; std::fs::copy(scratch, &path)?;
let _ = std::fs::remove_file(scratch); true
} } else {
false
};
std::fs::File::open(&path)?.sync_all()?;
// Sidecar last: a staged file without one is still recoverable bytes, // Sidecar last: a staged file without one is still recoverable bytes,
// while a sidecar without a file would describe nothing. // while a sidecar without a file would describe nothing.
std::fs::write( let side = path.with_extension("json");
path.with_extension("json"), let side_tmp = path.with_extension("json.tmp");
serde_json::to_vec_pretty(meta)?, let mut side_file = std::fs::OpenOptions::new()
)?; .write(true)
.create_new(true)
.open(&side_tmp)?;
side_file.write_all(&serde_json::to_vec_pretty(meta)?)?;
side_file.sync_all()?;
drop(side_file);
std::fs::rename(&side_tmp, &side)?;
std::fs::File::open(&self.staging_dir)?.sync_all()?;
if copied {
std::fs::remove_file(scratch)?;
}
Ok(path) Ok(path)
} }
@@ -1906,6 +1950,51 @@ mod tests {
assert!(c2.recovered_writes().is_empty(), "recovery does not repeat"); assert!(c2.recovered_writes().is_empty(), "recovery does not repeat");
} }
#[test]
fn recovery_keeps_an_orphan_blob_when_its_sidecar_is_missing() {
let (c, dir) = cache();
let orphan = dir.path().join("content/recovery/orphaned-user-bytes");
std::fs::write(&orphan, b"retain me").unwrap();
assert!(c.recovered_writes().is_empty());
assert_eq!(
std::fs::read(&orphan).unwrap(),
b"retain me",
"metadata loss must not trigger deletion of unexplained content"
);
}
#[test]
fn failed_scratch_rescue_does_not_delete_the_durable_source() {
use std::io::Write as _;
let dir = TempDir::new();
let db = Arc::new(Db::open_in_memory().unwrap());
let content = dir.path().join("content");
let pins = dir.path().join("pins.json");
let c = ContentCache::open(content.clone(), pins.clone(), 0, db.clone()).unwrap();
let (mut file, scratch) = c.create_scratch().unwrap();
file.write_all(b"durable source").unwrap();
file.sync_all().unwrap();
let meta = StagedWrite {
uid: uid("rescue").to_string(),
len: 14,
base_size: 0,
base_mtime: 0,
authored: vec![(0, 14)],
complete: true,
based_on: None,
};
c.mark_scratch_durable(&scratch, &meta).unwrap();
let blocking_destination = content.join("recovery").join(scratch.file_name().unwrap());
std::fs::create_dir(&blocking_destination).unwrap();
drop(file);
drop(c);
let _c2 = ContentCache::open(content, pins, 0, db).unwrap();
assert_eq!(std::fs::read(&scratch).unwrap(), b"durable source");
assert!(ContentCache::scratch_sidecar(&scratch).exists());
}
/// Releasing a write hands the bytes to staging, so the sidecar an earlier /// Releasing a write hands the bytes to staging, so the sidecar an earlier
/// `fsync` left must not offer recovery a second, stale copy of it. /// `fsync` left must not offer recovery a second, stale copy of it.
#[test] #[test]
+95 -24
View File
@@ -105,11 +105,21 @@ impl AppDirs {
/// Load config from disk, creating default if missing. /// Load config from disk, creating default if missing.
pub fn load_config(&self) -> AppConfig { pub fn load_config(&self) -> AppConfig {
let path = self.config_path(); let path = self.config_path();
if path.exists() if path.exists() {
&& let Ok(content) = std::fs::read_to_string(&path) match std::fs::read_to_string(&path)
&& let Ok(config) = serde_json::from_str::<AppConfig>(&content) .map_err(Error::from)
{ .and_then(|content| {
return config; serde_json::from_str::<AppConfig>(&content).map_err(Error::from)
}) {
Ok(config) => return config,
Err(e) => {
// Preserve the original bytes for diagnosis/recovery. In
// particular, never replace a truncated config with defaults
// and silently lose mount, device, or ignore settings.
tracing::error!(path = %path.display(), error = %e, "config is unreadable; using in-memory defaults without overwriting it");
return AppConfig::default();
}
}
} }
let default_config = AppConfig::default(); let default_config = AppConfig::default();
@@ -125,10 +135,26 @@ impl AppDirs {
/// cache budget or mountpoint; the daemon re-reads the file on its next mount. /// cache budget or mountpoint; the daemon re-reads the file on its next mount.
pub fn save_config(&self, config: &AppConfig) -> Result<()> { pub fn save_config(&self, config: &AppConfig) -> Result<()> {
let path = self.config_path(); let path = self.config_path();
if let Some(parent) = path.parent() { let parent = path
std::fs::create_dir_all(parent)?; .parent()
} .ok_or_else(|| Error::Other("configuration path has no parent".into()))?;
std::fs::write(&path, serde_json::to_string_pretty(config)?)?; std::fs::create_dir_all(parent)?;
let nonce = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.map(|duration| duration.as_nanos())
.unwrap_or(0);
let tmp = path.with_extension(format!("json.tmp-{}-{nonce}", std::process::id()));
use std::os::unix::fs::OpenOptionsExt as _;
let mut file = std::fs::OpenOptions::new()
.write(true)
.create_new(true)
.mode(0o600)
.open(&tmp)?;
use std::io::Write as _;
file.write_all(&serde_json::to_vec_pretty(config)?)?;
file.sync_all()?;
std::fs::rename(&tmp, &path)?;
std::fs::File::open(parent)?.sync_all()?;
Ok(()) Ok(())
} }
@@ -211,24 +237,41 @@ impl AppDirs {
pub fn ensure(&self) -> Result<()> { pub fn ensure(&self) -> Result<()> {
for dir in [self.state_dir(), self.cache_dir()] { for dir in [self.state_dir(), self.cache_dir()] {
std::fs::create_dir_all(&dir)?; std::fs::create_dir_all(&dir)?;
restrict_dir(&dir); restrict_dir(&dir)?;
} }
let config_dir = self.dirs.config_dir().to_path_buf(); let config_dir = self.dirs.config_dir().to_path_buf();
if std::fs::create_dir_all(&config_dir).is_ok() { std::fs::create_dir_all(&config_dir)?;
restrict_dir(&config_dir); restrict_dir(&config_dir)?;
}
Ok(()) Ok(())
} }
} }
/// Make `dir` owner-only (`0700`). Best effort: a mode we cannot set (an /// Make `dir` owner-only (`0700`). These directories contain decrypted bytes,
/// exotic filesystem, a directory we do not own) is not worth refusing to /// plaintext names, and an authenticated control socket, so failure is fatal.
/// start over, and the caller has no better answer than continuing. fn restrict_dir(dir: &Path) -> Result<()> {
fn restrict_dir(dir: &Path) { use std::os::unix::fs::{MetadataExt as _, PermissionsExt};
use std::os::unix::fs::PermissionsExt; let before = std::fs::symlink_metadata(dir)?;
if let Err(e) = std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o700)) { if before.file_type().is_symlink() || !before.is_dir() {
tracing::warn!(path = %dir.display(), error = %e, "could not restrict directory to 0700"); return Err(Error::Other(format!(
"sensitive path is not a real directory: {}",
dir.display()
)));
} }
if before.uid() != unsafe { libc::geteuid() } {
return Err(Error::Other(format!(
"sensitive directory is not owned by the current user: {}",
dir.display()
)));
}
std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o700))?;
let after = std::fs::symlink_metadata(dir)?;
if after.permissions().mode() & 0o777 != 0o700 {
return Err(Error::Other(format!(
"sensitive directory did not become owner-only: {}",
dir.display()
)));
}
Ok(())
} }
/// Make a just-bound Unix socket owner-only (`0600`). /// Make a just-bound Unix socket owner-only (`0600`).
@@ -263,29 +306,57 @@ mod tests {
/// and the control socket that commands the daemon's session. /// and the control socket that commands the daemon's session.
#[test] #[test]
fn restrict_dir_makes_a_directory_owner_only() { fn restrict_dir_makes_a_directory_owner_only() {
let dir = std::env::temp_dir().join(format!("pdfs-cfg-{}", std::process::id())); let dir = std::env::temp_dir().join(format!(
"pdfs-cfg-{}-{:?}",
std::process::id(),
std::thread::current().id()
));
let _ = std::fs::remove_dir_all(&dir); let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap(); std::fs::create_dir_all(&dir).unwrap();
// Start deliberately world-readable, as `create_dir_all` under a typical // Start deliberately world-readable, as `create_dir_all` under a typical
// umask would leave it. // umask would leave it.
std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o755)).unwrap(); std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o755)).unwrap();
restrict_dir(&dir); restrict_dir(&dir).unwrap();
assert_eq!(mode_of(&dir), 0o700, "group and other must have no access"); assert_eq!(mode_of(&dir), 0o700, "group and other must have no access");
let _ = std::fs::remove_dir_all(&dir); let _ = std::fs::remove_dir_all(&dir);
} }
#[test]
fn restrict_dir_refuses_a_symlink() {
let root = std::env::temp_dir().join(format!(
"pdfs-cfg-link-{}-{:?}",
std::process::id(),
std::thread::current().id()
));
let target = root.join("target");
let link = root.join("state");
std::fs::create_dir_all(&target).unwrap();
std::os::unix::fs::symlink(&target, &link).unwrap();
assert!(restrict_dir(&link).is_err());
let _ = std::fs::remove_dir_all(&root);
}
/// Connecting to the control socket confers the daemon's authenticated /// Connecting to the control socket confers the daemon's authenticated
/// session with no credential, so it is an authority boundary rather than /// session with no credential, so it is an authority boundary rather than
/// merely private data. /// merely private data.
#[test] #[test]
fn restrict_socket_makes_a_socket_owner_only() { fn restrict_socket_makes_a_socket_owner_only() {
let dir = std::env::temp_dir().join(format!("pdfs-sock-{}", std::process::id())); let dir = std::env::temp_dir().join(format!(
"pdfs-sock-{}-{:?}",
std::process::id(),
std::thread::current().id()
));
let _ = std::fs::remove_dir_all(&dir); let _ = std::fs::remove_dir_all(&dir);
std::fs::create_dir_all(&dir).unwrap(); std::fs::create_dir_all(&dir).unwrap();
let sock = dir.join("control.sock"); let sock = dir.join("control.sock");
let listener = std::os::unix::net::UnixListener::bind(&sock).unwrap(); // chmod semantics are identical for a socket path and a regular file;
// use a file so this regression also runs in sandboxes that prohibit
// creating Unix sockets.
let listener = std::fs::File::create(&sock).unwrap();
restrict_socket(&sock).unwrap(); restrict_socket(&sock).unwrap();
+6 -1
View File
@@ -31,7 +31,12 @@ impl Db {
.and_then(|v| v.parse().ok()) .and_then(|v| v.parse().ok())
.unwrap_or(0); .unwrap_or(0);
if current >= SCHEMA_VERSION { if current > SCHEMA_VERSION {
return Err(crate::Error::Other(format!(
"database schema {current} is newer than this build supports ({SCHEMA_VERSION}); refusing to open it to avoid corrupting user data"
)));
}
if current == SCHEMA_VERSION {
return Ok(()); return Ok(());
} }
+8 -5
View File
@@ -118,9 +118,10 @@ pub struct AttachedBlob {
impl Db { impl Db {
pub fn enqueue_op(&self, op: &PendingOp) -> Result<(i64, Option<String>)> { pub fn enqueue_op(&self, op: &PendingOp) -> Result<(i64, Option<String>)> {
let conn = self.conn.lock(); let mut conn = self.conn.lock();
let tx = conn.transaction()?;
let superseded: Option<String> = if op_supersedes(&op.kind) { let superseded: Option<String> = if op_supersedes(&op.kind) {
let blob: Option<String> = conn let blob: Option<String> = tx
.query_row( .query_row(
"SELECT blob_path FROM pending_op WHERE uid = ?1 AND kind = ?2", "SELECT blob_path FROM pending_op WHERE uid = ?1 AND kind = ?2",
params![op.uid, op.kind], params![op.uid, op.kind],
@@ -128,7 +129,7 @@ impl Db {
) )
.optional()? .optional()?
.flatten(); .flatten();
conn.execute( tx.execute(
"DELETE FROM pending_op WHERE uid = ?1 AND kind = ?2", "DELETE FROM pending_op WHERE uid = ?1 AND kind = ?2",
params![op.uid, op.kind], params![op.uid, op.kind],
)?; )?;
@@ -136,7 +137,7 @@ impl Db {
} else { } else {
None None
}; };
conn.execute( tx.execute(
"INSERT INTO pending_op "INSERT INTO pending_op
(kind, uid, parent_uid, name, blob_path, meta_json, created_at, next_attempt_at) (kind, uid, parent_uid, name, blob_path, meta_json, created_at, next_attempt_at)
VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)",
@@ -151,7 +152,9 @@ impl Db {
op.next_attempt_at op.next_attempt_at
], ],
)?; )?;
Ok((conn.last_insert_rowid(), superseded)) let id = tx.last_insert_rowid();
tx.commit()?;
Ok((id, superseded))
} }
/// Point a queued create at the bytes that were just written to it, returning /// Point a queued create at the bytes that were just written to it, returning
+68
View File
@@ -784,6 +784,41 @@ fn opens_and_migrates() {
assert_eq!(version, SCHEMA_VERSION.to_string()); assert_eq!(version, SCHEMA_VERSION.to_string());
} }
#[test]
fn refuses_a_schema_newer_than_the_running_build() {
let path = std::env::temp_dir().join(format!(
"pdfs-db-future-{}-{}.db",
std::process::id(),
std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos()
));
let conn = rusqlite::Connection::open(&path).unwrap();
conn.execute_batch(
"CREATE TABLE sync_state (key TEXT PRIMARY KEY, value TEXT);
INSERT INTO sync_state VALUES ('schema_version', '9999');",
)
.unwrap();
drop(conn);
let err = Db::open(&path)
.err()
.expect("future schema must fail closed");
assert!(err.to_string().contains("newer than this build supports"));
let conn = rusqlite::Connection::open(&path).unwrap();
let version: String = conn
.query_row(
"SELECT value FROM sync_state WHERE key = 'schema_version'",
[],
|row| row.get(0),
)
.unwrap();
assert_eq!(version, "9999", "failed open must not rewrite the database");
drop(conn);
let _ = std::fs::remove_file(path);
}
/// A queued write whose baseline is restamped must keep everything else. /// A queued write whose baseline is restamped must keep everything else.
/// ///
/// The restamp happens after our *own* upload seals a new revision under a /// The restamp happens after our *own* upload seals a new revision under a
@@ -867,6 +902,39 @@ fn a_second_write_supersedes_the_first_pending_op() {
assert_eq!(db.pending_op_counts().unwrap().uploads, 1); assert_eq!(db.pending_op_counts().unwrap().uploads, 1);
} }
#[test]
fn failed_superseding_insert_keeps_the_old_pending_op() {
let db = Db::open_in_memory().unwrap();
let op = |blob: &str| PendingOp {
id: 0,
kind: OP_REVISION.to_string(),
uid: uid("atomic").to_string(),
parent_uid: None,
name: None,
blob_path: Some(blob.to_string()),
meta_json: Some("{}".to_string()),
created_at: 1,
attempts: 0,
last_error: None,
next_attempt_at: 0,
};
db.enqueue_op(&op("/staging/original")).unwrap();
db.with_conn(|conn| {
conn.execute_batch(
"CREATE TRIGGER reject_replacement BEFORE INSERT ON pending_op
WHEN NEW.blob_path = '/staging/replacement'
BEGIN SELECT RAISE(ABORT, 'injected insert failure'); END;",
)?;
Ok(())
})
.unwrap();
assert!(db.enqueue_op(&op("/staging/replacement")).is_err());
let ops = db.pending_ops().unwrap();
assert_eq!(ops.len(), 1);
assert_eq!(ops[0].blob_path.as_deref(), Some("/staging/original"));
}
/// Deleting a folder that was created offline must take the ops queued /// Deleting a folder that was created offline must take the ops queued
/// underneath it with it: they name a placeholder parent that will now never /// underneath it with it: they name a placeholder parent that will now never
/// become real, so nothing could ever drain them and nothing is left to /// become real, so nothing could ever drain them and nothing is left to
+20 -12
View File
@@ -126,12 +126,16 @@ pub(super) async fn run_event_sync(
match client.enumerate_events(&scope, None).await { match client.enumerate_events(&scope, None).await {
Ok(events) => { Ok(events) => {
let head = events.last().map(|e| e.id().clone()); let head = events.last().map(|e| e.id().clone());
if let Some(c) = &head let Some(c) = &head else {
&& let Err(e) = db.set_event_cursor(c.as_str()) break None;
{ };
warn!(error = %e, "persist seed cursor failed"); if let Err(e) = db.set_event_cursor(c.as_str()) {
warn!(error = %e, ?delay, "persist seed cursor failed; retrying");
tokio::time::sleep(delay).await;
delay = (delay * 2).min(ONLINE_PROBE_MAX);
continue;
} }
break head; break Some(c.clone());
} }
Err(e) => { Err(e) => {
warn!(error = %e, ?delay, "seed event cursor failed; retrying"); warn!(error = %e, ?delay, "seed event cursor failed; retrying");
@@ -168,15 +172,19 @@ pub(super) async fn run_event_sync(
// the life of the daemon (SDK plan #9). `apply_event` only touches // the life of the daemon (SDK plan #9). `apply_event` only touches
// our FUSE state, so nothing else does this. // our FUSE state, so nothing else does this.
if let Err(e) = client.invalidate_caches_for_event(event).await { if let Err(e) = client.invalidate_caches_for_event(event).await {
warn!(error = %e, "sdk cache invalidation for event failed"); warn!(error = %e, "sdk cache invalidation for event failed; retaining cursor for retry");
break;
} }
apply_event(&state, &content, &pending, &notifier, event); apply_event(&state, &content, &pending, &notifier, event);
} let applied = event.id().clone();
cursor = events.last().map(|e| e.id().clone()); if let Err(e) = db.set_event_cursor(applied.as_str()) {
if let Some(c) = &cursor warn!(error = %e, "persist event cursor failed; retaining prior cursor for retry");
&& let Err(e) = db.set_event_cursor(c.as_str()) break;
{ }
warn!(error = %e, "persist event cursor failed"); // Advance only after this exact event is durably acknowledged. If a
// later event fails, polling resumes from here and replays nothing
// that was not already made visible locally.
cursor = Some(applied);
} }
} }
} }
+121 -7
View File
@@ -12,8 +12,8 @@
use std::io::{BufRead, BufReader, Write as _}; use std::io::{BufRead, BufReader, Write as _};
use std::os::unix::net::{UnixListener, UnixStream}; use std::os::unix::net::{UnixListener, UnixStream};
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::sync::atomic::Ordering; use std::sync::atomic::{AtomicUsize, Ordering};
use std::time::Instant; use std::time::{Duration, Instant};
use pdfs_core::config::AppDirs; use pdfs_core::config::AppDirs;
use pdfs_core::control::{ use pdfs_core::control::{
@@ -27,6 +27,67 @@ use tracing::{info, warn};
use super::transfers::CountingReader; use super::transfers::CountingReader;
use super::{Core, count_noun, human_bytes, human_duration, parse_uid}; use super::{Core, count_noun, human_bytes, human_duration, parse_uid};
const MAX_CONTROL_REQUEST_BYTES: u64 = 1024 * 1024;
const CONTROL_READ_TIMEOUT: Duration = Duration::from_secs(10);
const MAX_CONTROL_HANDLERS: usize = 64;
static ACTIVE_CONTROL_HANDLERS: AtomicUsize = AtomicUsize::new(0);
struct ControlHandlerPermit;
impl ControlHandlerPermit {
fn acquire() -> Option<Self> {
ACTIVE_CONTROL_HANDLERS
.fetch_update(Ordering::AcqRel, Ordering::Acquire, |active| {
(active < MAX_CONTROL_HANDLERS).then_some(active + 1)
})
.ok()
.map(|_| Self)
}
}
impl Drop for ControlHandlerPermit {
fn drop(&mut self) {
ACTIVE_CONTROL_HANDLERS.fetch_sub(1, Ordering::AcqRel);
}
}
fn read_request_line(reader: &mut impl BufRead) -> std::io::Result<Option<String>> {
let mut bytes = Vec::new();
loop {
let available = reader.fill_buf()?;
if available.is_empty() {
break;
}
let take = available
.iter()
.position(|byte| *byte == b'\n')
.map_or(available.len(), |index| index + 1);
if bytes.len().saturating_add(take) > MAX_CONTROL_REQUEST_BYTES as usize {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidData,
"control request exceeds size limit",
));
}
bytes.extend_from_slice(&available[..take]);
reader.consume(take);
if bytes.ends_with(b"\n") {
break;
}
}
if bytes.is_empty() {
return Ok(None);
}
if bytes.len() as u64 > MAX_CONTROL_REQUEST_BYTES || !bytes.ends_with(b"\n") {
return Err(std::io::Error::new(
std::io::ErrorKind::InvalidData,
"control request exceeds limit or is not newline terminated",
));
}
String::from_utf8(bytes)
.map(Some)
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e))
}
/// Turn a CLI-supplied path into a mountpoint-relative path. An absolute path /// Turn a CLI-supplied path into a mountpoint-relative path. An absolute path
/// must live under `mountpoint`; a relative path is taken as already relative to /// must live under `mountpoint`; a relative path is taken as already relative to
/// the mount root. /// the mount root.
@@ -44,6 +105,10 @@ fn rel_to_mount(mountpoint: &Path, path: &str) -> CoreResult<PathBuf> {
/// Handle one control-socket connection: read a single JSON request line, /// Handle one control-socket connection: read a single JSON request line,
/// dispatch it against `core`, and write back a JSON response line. /// dispatch it against `core`, and write back a JSON response line.
fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: UnixStream) { fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: UnixStream) {
if let Err(e) = stream.set_read_timeout(Some(CONTROL_READ_TIMEOUT)) {
warn!(error = %e, "control: could not set request timeout");
return;
}
let mut reader = BufReader::new(match stream.try_clone() { let mut reader = BufReader::new(match stream.try_clone() {
Ok(s) => s, Ok(s) => s,
Err(e) => { Err(e) => {
@@ -51,10 +116,14 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U
return; return;
} }
}); });
let mut line = String::new(); let line = match read_request_line(&mut reader) {
if reader.read_line(&mut line).is_err() || line.trim().is_empty() { Ok(Some(line)) if !line.trim().is_empty() => line,
return; Ok(_) => return,
} Err(e) => {
warn!(error = %e, "control: rejected request");
return;
}
};
let response = match serde_json::from_str::<CtlRequest>(line.trim()) { let response = match serde_json::from_str::<CtlRequest>(line.trim()) {
Ok(CtlRequest::Status) => { Ok(CtlRequest::Status) => {
let pins = core.cache.list_pins(); let pins = core.cache.list_pins();
@@ -792,12 +861,22 @@ pub(crate) fn run_control_socket(
for conn in listener.incoming() { for conn in listener.incoming() {
match conn { match conn {
Ok(stream) => { Ok(stream) => {
let Some(permit) = ControlHandlerPermit::acquire() else {
warn!(
limit = MAX_CONTROL_HANDLERS,
"control: rejecting connection at handler limit"
);
continue;
};
let core = core.clone(); let core = core.clone();
let username = username.clone(); let username = username.clone();
let mountpoint = mountpoint.clone(); let mountpoint = mountpoint.clone();
if let Err(e) = std::thread::Builder::new() if let Err(e) = std::thread::Builder::new()
.name("pdfs-control".into()) .name("pdfs-control".into())
.spawn(move || handle_control_conn(&core, &username, &mountpoint, stream)) .spawn(move || {
let _permit = permit;
handle_control_conn(&core, &username, &mountpoint, stream);
})
{ {
warn!(error = %e, "control: spawn handler failed"); warn!(error = %e, "control: spawn handler failed");
} }
@@ -808,3 +887,38 @@ pub(crate) fn run_control_socket(
} }
} }
} }
#[cfg(test)]
mod request_limit_tests {
use super::*;
use std::io::Cursor;
#[test]
fn accepts_one_bounded_newline_terminated_request() {
let mut input = Cursor::new(b"{\"Status\":null}\n".to_vec());
assert_eq!(
read_request_line(&mut input).unwrap().as_deref(),
Some("{\"Status\":null}\n")
);
}
#[test]
fn rejects_a_request_without_a_terminating_newline() {
let mut input = Cursor::new(b"{\"Status\":null}".to_vec());
assert_eq!(
read_request_line(&mut input).unwrap_err().kind(),
std::io::ErrorKind::InvalidData
);
}
#[test]
fn rejects_a_request_larger_than_the_limit() {
let mut bytes = vec![b'x'; MAX_CONTROL_REQUEST_BYTES as usize + 1];
bytes.push(b'\n');
let mut input = Cursor::new(bytes);
assert_eq!(
read_request_line(&mut input).unwrap_err().kind(),
std::io::ErrorKind::InvalidData
);
}
}
+7 -8
View File
@@ -1129,17 +1129,16 @@ impl Filesystem for ProtonFs {
// A failure past this point has already trashed any node the rename was // A failure past this point has already trashed any node the rename was
// replacing, so put it back rather than leaving the caller with neither // replacing, so put it back rather than leaving the caller with neither
// the source moved nor the destination intact. // the source moved nor the destination intact.
if newname != name { if newname != name
if let Err(e) = self && let Err(e) = self
.core .core
.rt .rt
.block_on(self.core.client.rename_node(&uid, &newname, None)) .block_on(self.core.client.rename_node(&uid, &newname, None))
{ {
error!(%uid, error = %e, "rename failed"); error!(%uid, error = %e, "rename failed");
self.core.restore_replaced(victim.as_ref(), &newname); self.core.restore_replaced(victim.as_ref(), &newname);
reply.error(Errno::EIO); reply.error(Errno::EIO);
return; return;
}
} }
if newparent != parent { if newparent != parent {
let mut attempts = 0u32; let mut attempts = 0u32;
+26 -2
View File
@@ -1548,8 +1548,32 @@ impl Core {
); );
} }
Err(e) => { Err(e) => {
error!(%uid, error = %e, "staging write failed; bytes lost"); // `src` is the only known copy of bytes the kernel already
let _ = std::fs::remove_file(src); // accepted. A failure to publish them into staging must never
// turn into permission to delete them: leave the scratch file
// in place so startup recovery or a human can still salvage it.
error!(
%uid,
error = %e,
scratch = %src.display(),
"staging write failed; retaining the only scratch copy"
);
let name = {
let st = self.state.lock();
st.entries
.get(&ino)
.map(|entry| entry.node.name.clone())
.unwrap_or_default()
};
self.log_activity(
ActivityKind::Upload,
&name,
format!(
"write not queued and could not be staged; recovery copy retained at {}",
src.display()
),
false,
);
} }
} }
} }
+146 -29
View File
@@ -55,9 +55,11 @@ use local_scan::is_write_mode;
use local_scan::open_for_write_set; use local_scan::open_for_write_set;
use model::{LocalItem, Outcome, PassAbort, Pending, RemoteItem}; use model::{LocalItem, Outcome, PassAbort, Pending, RemoteItem};
pub(crate) use planner::base_name; pub(crate) use planner::base_name;
#[cfg(test)]
use planner::conflict_path;
use planner::{ use planner::{
FilePlan, classification_order, conflict_path, filter_baseline, guard_local_wipe, join_rel, FilePlan, classification_order, conflict_path_with_suffix, filter_baseline, guard_local_wipe,
parent_rel, plan_file, rel_to_path, remote_sig, unchanged_remote_size, join_rel, parent_rel, plan_file, rel_to_path, remote_sig, unchanged_remote_size,
}; };
/// What a [`Pending`] op did, so the reconcile can update shared state on the /// What a [`Pending`] op did, so the reconcile can update shared state on the
/// engine thread (never inside a task). /// engine thread (never inside a task).
@@ -711,7 +713,17 @@ impl Core {
} }
}, },
FilePlan::DeleteLocal => { FilePlan::DeleteLocal => {
let _ = std::fs::remove_file(local_root.join(rel_to_path(rel))); if let Err(e) = std::fs::remove_file(local_root.join(rel_to_path(rel))) {
warn!(rel, error = %e, "sync: remove local file failed");
self.log_activity(
ActivityKind::Trash,
base_name(rel),
e.to_string(),
false,
);
outcome.errors += 1;
continue;
}
let _ = self.db.sync_entry_remove(folder_id, rel); let _ = self.db.sync_entry_remove(folder_id, rel);
self.log_activity(ActivityKind::Trash, base_name(rel), "removed locally", true); self.log_activity(ActivityKind::Trash, base_name(rel), "removed locally", true);
outcome.deleted += 1; outcome.deleted += 1;
@@ -758,7 +770,12 @@ impl Core {
// Deferred folder deletions, deepest first. // Deferred folder deletions, deepest first.
delete_local_dirs.sort_by_key(|p| std::cmp::Reverse(p.matches('/').count())); delete_local_dirs.sort_by_key(|p| std::cmp::Reverse(p.matches('/').count()));
for rel in delete_local_dirs { for rel in delete_local_dirs {
let _ = std::fs::remove_dir_all(local_root.join(rel_to_path(&rel))); if let Err(e) = std::fs::remove_dir_all(local_root.join(rel_to_path(&rel))) {
warn!(rel, error = %e, "sync: remove local folder failed");
self.log_activity(ActivityKind::Trash, base_name(&rel), e.to_string(), false);
outcome.errors += 1;
continue;
}
let _ = self.db.sync_entry_remove(folder_id, &rel); let _ = self.db.sync_entry_remove(folder_id, &rel);
self.log_activity( self.log_activity(
ActivityKind::Trash, ActivityKind::Trash,
@@ -802,18 +819,24 @@ impl Core {
out: &mut HashMap<String, LocalItem>, out: &mut HashMap<String, LocalItem>,
) -> Result<(), String> { ) -> Result<(), String> {
let entries = std::fs::read_dir(dir).map_err(|e| format!("read {}: {e}", dir.display()))?; let entries = std::fs::read_dir(dir).map_err(|e| format!("read {}: {e}", dir.display()))?;
for entry in entries.filter_map(|e| e.ok()) { for entry in entries {
let entry = entry.map_err(|e| format!("read entry in {}: {e}", dir.display()))?;
let path = entry.path(); let path = entry.path();
let meta = match std::fs::symlink_metadata(&path) { let meta = std::fs::symlink_metadata(&path)
Ok(m) => m, .map_err(|e| format!("stat {}: {e}", path.display()))?;
Err(_) => continue, let stripped = path.strip_prefix(root).map_err(|e| {
}; format!(
let Ok(stripped) = path.strip_prefix(root) else { "local path {} escaped sync root {}: {e}",
continue; path.display(),
}; root.display()
let Some(rel) = stripped.to_str() else { )
continue; })?;
}; let rel = stripped.to_str().ok_or_else(|| {
format!(
"local path {} is not valid UTF-8; refusing a destructive sync pass",
path.display()
)
})?;
// Ignore our own in-flight download temp files. // Ignore our own in-flight download temp files.
if rel.contains(".pdfs-tmp-") { if rel.contains(".pdfs-tmp-") {
continue; continue;
@@ -1153,12 +1176,9 @@ impl Core {
// pass), then take the remote version as the shared truth. // pass), then take the remote version as the shared truth.
let path = local_root.join(rel_to_path(rel)); let path = local_root.join(rel_to_path(rel));
if path.exists() { if path.exists() {
let conflict = conflict_path(&path, now_secs()); preserve_conflict_copy(&path, now_secs())
if let Err(e) = std::fs::rename(&path, &conflict) { .map_err(|e| format!("set aside conflict copy for {rel}: {e}"))?;
warn!(rel, error = %e, "sync: could not set aside conflict copy"); info!(rel, "sync: kept local changes as a conflict copy");
} else {
info!(rel, "sync: kept local changes as a conflict copy");
}
} }
self.download_file(folder_id, local_root, rel, uid, *mtime, *size) self.download_file(folder_id, local_root, rel, uid, *mtime, *size)
.await?; .await?;
@@ -1171,8 +1191,7 @@ impl Core {
// evicted. Push the conflict copy now, while it still exists, and // evicted. Push the conflict copy now, while it still exists, and
// leave the remote file holding the original path. // leave the remote file holding the original path.
let path = local_root.join(rel_to_path(rel)); let path = local_root.join(rel_to_path(rel));
let conflict = conflict_path(&path, now_secs()); let conflict = preserve_conflict_copy(&path, now_secs())
std::fs::rename(&path, &conflict)
.map_err(|e| format!("set aside conflict copy for {rel}: {e}"))?; .map_err(|e| format!("set aside conflict copy for {rel}: {e}"))?;
let name = conflict let name = conflict
.file_name() .file_name()
@@ -1297,13 +1316,24 @@ impl Core {
let _ = std::fs::remove_file(&tmp); let _ = std::fs::remove_file(&tmp);
format!("download {rel}: {e}") format!("download {rel}: {e}")
})?; })?;
out.into_inner().flush().ok(); let mut out = out.into_inner();
out.flush().map_err(|e| format!("flush tmp {rel}: {e}"))?;
out.sync_all().map_err(|e| format!("sync tmp {rel}: {e}"))?;
} }
std::fs::rename(&tmp, &path).map_err(|e| format!("place {rel}: {e}"))?; std::fs::rename(&tmp, &path).map_err(|e| format!("place {rel}: {e}"))?;
// Match local mtime to the remote's so neither side looks "changed" next pass. // Match local mtime to the remote's so neither side looks "changed" next pass.
if let Ok(f) = std::fs::File::options().write(true).open(&path) { let f = std::fs::File::options()
let _ = .write(true)
f.set_modified(SystemTime::UNIX_EPOCH + Duration::from_secs(mtime.max(0) as u64)); .open(&path)
.map_err(|e| format!("open placed file {rel}: {e}"))?;
f.set_modified(SystemTime::UNIX_EPOCH + Duration::from_secs(mtime.max(0) as u64))
.map_err(|e| format!("set mtime for {rel}: {e}"))?;
f.sync_all()
.map_err(|e| format!("sync placed file {rel}: {e}"))?;
if let Some(parent) = path.parent() {
std::fs::File::open(parent)
.and_then(|dir| dir.sync_all())
.map_err(|e| format!("sync directory for {rel}: {e}"))?;
} }
self.record_file_baseline(folder_id, rel, &path, uid).await self.record_file_baseline(folder_id, rel, &path, uid).await
} }
@@ -1350,6 +1380,46 @@ impl Core {
} }
} }
/// Publish a conflict copy without overwriting an earlier conflict bearing the
/// same timestamp. The original is removed only after the new copy is durable.
fn preserve_conflict_copy(path: &Path, stamp: i64) -> std::io::Result<PathBuf> {
let metadata = std::fs::metadata(path)?;
for suffix in 0..=u32::MAX {
let conflict = conflict_path_with_suffix(path, stamp, suffix);
let mut source = std::fs::File::open(path)?;
let mut target = match std::fs::OpenOptions::new()
.write(true)
.create_new(true)
.open(&conflict)
{
Ok(file) => file,
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue,
Err(e) => return Err(e),
};
if let Err(e) = std::io::copy(&mut source, &mut target).and_then(|_| target.sync_all()) {
let _ = std::fs::remove_file(&conflict);
return Err(e);
}
target.set_permissions(metadata.permissions())?;
if let Ok(modified) = metadata.modified() {
target.set_modified(modified)?;
}
target.sync_all()?;
sync_parent_dir(&conflict)?;
std::fs::remove_file(path)?;
sync_parent_dir(&conflict)?;
return Ok(conflict);
}
unreachable!("u32 conflict suffix space exhausted")
}
fn sync_parent_dir(path: &Path) -> std::io::Result<()> {
match path.parent() {
Some(parent) => std::fs::File::open(parent)?.sync_all(),
None => Ok(()),
}
}
// ---- helpers -------------------------------------------------------------- // ---- helpers --------------------------------------------------------------
/// A remote node's `(mtime, size)` change signature. Size prefers the plaintext /// A remote node's `(mtime, size)` change signature. Size prefers the plaintext
@@ -1609,9 +1679,10 @@ mod tests {
guard_local_wipe(&base, &one).is_ok(), guard_local_wipe(&base, &one).is_ok(),
"one survivor means the tree is readable; the rest are real deletions" "one survivor means the tree is readable; the rest are real deletions"
); );
// A one-entry baseline is a genuine single delete, not a wipe to detect. // A one-entry folder can disappear because its mount or parent became
// unavailable just as easily as a larger tree can.
let single: HashMap<String, ()> = [("a.txt".to_string(), ())].into_iter().collect(); let single: HashMap<String, ()> = [("a.txt".to_string(), ())].into_iter().collect();
assert!(guard_local_wipe(&single, &none).is_ok()); assert!(guard_local_wipe(&single, &none).is_err());
// Nothing synced yet: a first pass over an empty folder is not a wipe. // Nothing synced yet: a first pass over an empty folder is not a wipe.
assert!(guard_local_wipe(&none, &none).is_ok()); assert!(guard_local_wipe(&none, &none).is_ok());
// Local paths that are not in the baseline are new uploads, and do not // Local paths that are not in the baseline are new uploads, and do not
@@ -1781,6 +1852,52 @@ mod tests {
assert_eq!(no_ext, PathBuf::from("/home/me/README (sync-conflict 42)")); assert_eq!(no_ext, PathBuf::from("/home/me/README (sync-conflict 42)"));
} }
#[test]
fn conflict_preservation_never_replaces_an_existing_copy() {
let dir = sync_test_dir("conflict-collision");
let source = dir.join("notes.txt");
let existing = conflict_path(&source, 1700);
std::fs::write(&source, b"new local edit").unwrap();
std::fs::write(&existing, b"older conflict").unwrap();
let preserved = preserve_conflict_copy(&source, 1700).unwrap();
assert_eq!(std::fs::read(&existing).unwrap(), b"older conflict");
assert_eq!(std::fs::read(&preserved).unwrap(), b"new local edit");
assert_eq!(
preserved.file_name().unwrap(),
"notes (sync-conflict 1700-1).txt"
);
assert!(!source.exists());
std::fs::remove_dir_all(dir).unwrap();
}
#[test]
fn conflict_preservation_failure_keeps_the_source() {
let dir = sync_test_dir("conflict-failure");
// A directory cannot be copied through the regular-file preservation
// path, deterministically forcing publication to fail after the source
// has been discovered.
let source = dir.join("not-a-file.txt");
std::fs::create_dir(&source).unwrap();
assert!(preserve_conflict_copy(&source, 1700).is_err());
assert!(source.is_dir(), "failure must not remove the source");
assert!(!conflict_path(&source, 1700).exists());
std::fs::remove_dir_all(dir).unwrap();
}
fn sync_test_dir(label: &str) -> PathBuf {
let nonce = SystemTime::now()
.duration_since(SystemTime::UNIX_EPOCH)
.unwrap()
.as_nanos();
let path =
std::env::temp_dir().join(format!("pdfs-sync-{label}-{}-{nonce}", std::process::id()));
std::fs::create_dir(&path).unwrap();
path
}
// ---- open-for-write detection tests ----------------------------------- // ---- open-for-write detection tests -----------------------------------
#[test] #[test]
+15 -3
View File
@@ -99,7 +99,7 @@ pub(super) fn guard_local_wipe<B, L>(
baseline: &HashMap<String, B>, baseline: &HashMap<String, B>,
local: &HashMap<String, L>, local: &HashMap<String, L>,
) -> Result<(), String> { ) -> Result<(), String> {
if baseline.len() >= 2 && baseline.keys().all(|rel| !local.contains_key(rel)) { if !baseline.is_empty() && baseline.keys().all(|rel| !local.contains_key(rel)) {
return Err(format!( return Err(format!(
"every one of the {} synced paths is missing locally; refusing to trash \ "every one of the {} synced paths is missing locally; refusing to trash \
them on Drive. Check that the folder is mounted and readable.", them on Drive. Check that the folder is mounted and readable.",
@@ -162,12 +162,24 @@ pub(super) fn rel_to_path(rel: &str) -> PathBuf {
} }
/// The name for a conflict copy of `path`, e.g. `notes (sync-conflict 1700000000).txt`. /// The name for a conflict copy of `path`, e.g. `notes (sync-conflict 1700000000).txt`.
#[cfg(test)]
pub(super) fn conflict_path(path: &Path, stamp: i64) -> PathBuf { pub(super) fn conflict_path(path: &Path, stamp: i64) -> PathBuf {
conflict_path_with_suffix(path, stamp, 0)
}
/// As [`conflict_path`], with a deterministic suffix used when that name already
/// exists. This keeps conflict preservation from ever replacing an older copy.
pub(super) fn conflict_path_with_suffix(path: &Path, stamp: i64, suffix: u32) -> PathBuf {
let stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or("file"); let stem = path.file_stem().and_then(|s| s.to_str()).unwrap_or("file");
let ext = path.extension().and_then(|s| s.to_str()); let ext = path.extension().and_then(|s| s.to_str());
let suffix = if suffix == 0 {
String::new()
} else {
format!("-{suffix}")
};
let name = match ext { let name = match ext {
Some(ext) => format!("{stem} (sync-conflict {stamp}).{ext}"), Some(ext) => format!("{stem} (sync-conflict {stamp}{suffix}).{ext}"),
None => format!("{stem} (sync-conflict {stamp})"), None => format!("{stem} (sync-conflict {stamp}{suffix})"),
}; };
match path.parent() { match path.parent() {
Some(dir) => dir.join(name), Some(dir) => dir.join(name),
+314
View File
@@ -1772,3 +1772,317 @@ the FUSE handler invalidates both exact dentries and both directory inodes; the
ordering matters because an invalidation sent before the rename reply is ordering matters because an invalidation sent before the rename reply is
overwritten by the kernel's response processing. Uids are immutable and never overwritten by the kernel's response processing. Uids are immutable and never
reused, making a session-long tombstone safe. reused, making a session-long tombstone safe.
---
## B49 — Failed Conflict Preservation Still Overwrites the Local File (CRIT-05)
**Status:** Fixed in code with deterministic regressions; live/fault verification pending
**Found:** 2026-07-22, 1.0 data-safety audit
**Where:** `crates/pdfs-fuse/src/sync.rs`, download/conflict handling
**Cause:** If renaming a concurrently changed local file to a conflict copy
fails, sync only logs the error and continues publishing the remote download
over the local version. Second-resolution conflict names can also collide.
**Required fix/test:** Abort download publication unless preservation succeeds;
use collision-resistant names. Test existing names, permissions, `ENOSPC`,
`EIO`, and cross-filesystem behavior. This is B39's failure path; B25 remains
the separate weak-baseline problem.
---
## B50 — Failed Orphan Staging Deletes the Sole Scratch Copy (CRIT-06)
**Status:** Fixed in code; injected filesystem-failure verification pending
**Found:** 2026-07-22, 1.0 data-safety audit
**Where:** `crates/pdfs-fuse/src/lib.rs`, `Core::stage_orphaned_write`
**Cause:** If preserving an orphaned scratch write into staging fails, the
failure path removes the original even though no other durable copy exists.
**Required fix/test:** Never unlink the source until a durable replacement and
metadata are committed. Inject `ENOSPC`, `EIO`, `EROFS`, permission, and
cross-device failures and assert a byte-identical copy remains discoverable.
Related: B23, B27, and B28.
---
## B51 — Scratch Sidecar Does Not Establish a Durable Recovery Record (CRIT-07)
**Status:** Fixed in code; power-loss verification pending
**Found:** 2026-07-22, 1.0 crash-consistency audit
**Where:** `crates/pdfs-core/src/cache.rs`, scratch durability markers
**Cause:** A temporary sidecar is renamed without syncing the sidecar first or
the directory afterward. Power loss after successful user `fsync` can preserve
bytes but lose authored-range metadata; recovery may discard the write or treat
a sparse partial write as complete and replace untouched ranges with zeros.
**Required fix/test:** Sync data, write and sync temporary metadata, rename it,
then sync the directory before acknowledging durability. Crash-test every
boundary and corrupt/partial sidecars. B20 and B23 do not cover this invariant.
---
## B52 — Staged-Write Publication Is Not Crash-Safe (CRIT-08)
**Status:** Fixed in code; cross-filesystem and power-loss verification pending
**Found:** 2026-07-22, 1.0 crash-consistency audit
**Where:** `crates/pdfs-core/src/cache.rs`, staged-write publication
**Cause:** Rename/copy and direct sidecar writes lack a complete sync protocol.
The cross-filesystem fallback can remove its source after an unsynced copy,
leaving no complete data/metadata pair after a crash.
**Required fix/test:** Publish synced temporary data and metadata via atomic
renames and directory syncs; delete the source only after verification. Inject
failure after every write, sync, rename, DB enqueue, and deletion. Related: B50
and B51.
---
## B53 — Superseding a Pending Operation Is Not Transactional (CRIT-09)
**Status:** Fixed with rollback regression; crash/ENOSPC verification pending
**Found:** 2026-07-22, 1.0 SQLite durability audit
**Where:** `crates/pdfs-core/src/db/ops.rs`, `Db::enqueue_op`
**Cause:** The old durable row is deleted before its replacement is inserted,
outside a transaction. A crash, constraint error, full disk, or I/O failure can
erase acknowledged upload work and orphan its blob.
**Required fix/test:** Select and replace in one transaction; retain the old
blob until commit and make cleanup recoverable. Inject insert/commit failures
and prove that old or new operation—never neither—remains queued. B27 concerns
a retained failing row; this issue erases the row.
---
## B54 — Total-Wipe Guard Does Not Protect a One-Entry Baseline (CRIT-10)
**Status:** Fixed with one-entry regression; live mode-matrix verification pending
**Found:** 2026-07-22, 1.0 mirror-sync audit
**Where:** `crates/pdfs-fuse/src/sync/planner.rs`
**Cause:** The safeguard activates only when the baseline has at least two
paths. A one-file local folder that is missing, unreadable, or wrongly mounted
can therefore trash its sole remote file.
**Required fix/test:** Protect every non-empty baseline and require an
independently complete scan plus an explicit deliberate-delete signal for a
total wipe. Test absent roots, replaced mountpoints, permissions, and one-file
folders in every mode combination.
---
## B55 — Incomplete Local Scans Are Interpreted as Deletions (CRIT-11)
**Status:** Fixed in code; injected iterator/stat failure tests pending
**Found:** 2026-07-22, 1.0 mirror-sync audit
**Where:** `crates/pdfs-fuse/src/sync.rs`, local scan/reconciliation
**Cause:** Directory-entry and metadata errors omit paths instead of marking the
snapshot incomplete. Reconciliation can treat omissions as intentional local
deletions and propagate them remotely.
**Required fix/test:** Carry completeness/errors into planning; an incomplete
subtree must make the pass non-destructive and retain its prior baseline. Test
`readdir`, `stat`, permission, transient-I/O, and disappearing-entry failures.
B42 covers the inverse local-delete failure.
---
## B56 — Database Schema Version Parsing Fails Open (HIGH-13)
**Status:** Partly fixed — future schemas are refused; malformed-version handling remains open
**Found:** 2026-07-22, 1.0 database audit
**Where:** `crates/pdfs-core/src/db/migrations.rs`, `Db::migrate`
**Cause:** Missing, malformed, or non-numeric version values become zero, while
all future versions are accepted. Corruption can replay migrations, and an
older binary can open a newer incompatible database.
**Required fix/test:** Distinguish a new DB from corrupt metadata; reject
malformed/future versions without mutation and validate required schema. Test
empty, malformed, negative, future, partial/interrupted migration, and upgrade
fixtures from every released schema.
---
## B57 — Event Cursor Can Advance Past Unapplied State (HIGH-14)
**Status:** Partly fixed — cursor now advances per successfully invalidated event; DB/apply fault tests remain
**Found:** 2026-07-22, 1.0 event-recovery audit
**Where:** `crates/pdfs-fuse/src/background.rs`, `run_event_sync`
**Cause:** Cache invalidation failures are ignored while the batch cursor still
advances. Cursor-persistence failure also leaves the in-memory cursor moving;
first-seed persistence failure can later reseed at a newer head. Applying state
and committing its cursor have no atomic relationship.
**Required fix/test:** Apply idempotently and persist derived DB state plus the
cursor atomically, or advance only through the last durable event. Retry failed
prerequisites. Crash/fault-test every event and cursor boundary, first seed,
duplicate replay, malformed events, and database-full conditions.
---
## B58 — Control Socket Has Unbounded Frames and Connections (HIGH-15)
**Status:** Fixed in code with frame-limit regressions; connection-flood verification pending
**Found:** 2026-07-22, 1.0 IPC audit
**Where:** `crates/pdfs-fuse/src/control.rs`
**Cause:** The server uses unbounded `read_line`, a new thread per connection,
and no concurrency or idle limit. A local process can exhaust memory, file
descriptors, and threads, blocking filesystem control and safe shutdown.
**Required fix/test:** Bound frames, clients, and idle lifetime. Test slow/no-
newline clients, oversized/truncated JSON, floods, and recovery. B6/B35 cover
socket authority and permissions, not resource bounds.
---
## B59 — Private State Permission Enforcement Fails Open (HIGH-16)
**Status:** Fixed in code with symlink regression; wrong-owner integration verification pending
**Found:** 2026-07-22, 1.0 local-privacy audit
**Where:** `crates/pdfs-core/src/config.rs`, `AppDirs::ensure`
**Cause:** chmod/creation failures are warned about or ignored while decrypted
content, plaintext metadata, and the authenticated socket may remain accessible
to other local users.
**Required fix/test:** Before loading credentials or serving data, verify every
sensitive directory is owned by the effective user, is not a symlink, and is
owner-only. Test wrong owner, symlink substitution, read-only filesystems,
permissive restored directories, and chmod failure. This tightens B6/B35.
---
## B60 — Config Writes Are Non-Atomic and Parse Errors Are Overwritten (HIGH-17)
**Status:** Fixed in code; fault/concurrent-save verification pending
**Found:** 2026-07-22, 1.0 configuration audit
**Where:** `crates/pdfs-core/src/config.rs`, `load_config` / `save_config`
**Cause:** Save truncates in place without sync. Load treats read/parse failure
as absence and best-effort overwrites with defaults, silently losing mountpoint,
ignore policy, budget, and explicit device adoption.
**Required fix/test:** Atomically publish a restricted, synced temporary file
and sync its directory. Preserve malformed input and report an actionable error.
Test invalid/truncated JSON, permissions, `ENOSPC`, crash points, and concurrent
saves.
---
## B61 — Rotated Single-Use Credentials Can Be Lost (HIGH-18)
**Status:** Open — 1.0 authentication/recovery blocker
**Found:** 2026-07-22, 1.0 authentication audit
**Where:** `crates/pdfs-core/src/auth.rs`, refresh callback
**Cause:** Failure to serialize or persist rotated credentials is only logged
(and some setup failures are silent). The daemon continues in memory while the
keyring retains an invalid single-use refresh token, so restart loses login.
**Required fix/test:** Expose unhealthy persistence, retry with bounded backoff,
notify the user, and define a safe re-login/shutdown path. Test locked/full/
unavailable keyrings, callback races, repeated rotations, death, and restart.
---
## B62 — Debian Artifact Omits Required Service and Autostart Units (HIGH-19)
**Status:** Fixed in workflow; clean-install VM verification pending
**Found:** 2026-07-22, 1.0 packaging audit
**Where:** `.github/workflows/release.yml`; `packaging/`
**Cause:** The generated package omits `proton-drive.service` and the tray
autostart entry even though normal login/GUI flows assume that lifecycle.
**Required fix/test:** Package all units/resources with correct paths, modes,
and hooks. In clean supported-distribution VMs test install, login, reboot,
daemon/tray/FUSE, upgrade, rollback, and uninstall without deleting user state.
---
## B63 — Release Version Identity Is Inconsistent and Unenforced (HIGH-20)
**Status:** Partly fixed — tag/workspace/PKGBUILD equality is enforced; protocol identity remains
**Found:** 2026-07-22, 1.0 release audit
**Where:** manifests, client identity constants, tags, release workflow
**Cause:** Workspace/binary versions, protocol identity strings, and tags can
disagree; the workflow trusts any `v*` tag without checking package metadata or
installed `--version` output.
**Required fix/test:** Establish one version source and fail CI on every tag,
manifest, package, and binary mismatch. Document any intentionally independent
protocol identity.
---
## B64 — Stable Publishing Lacks Data-Safety and Recovery Gates (CRIT-12)
**Status:** Partly fixed — automated local gates added; managed-live/recovery approval remains open
**Found:** 2026-07-22, 1.0 release audit
**Where:** CI and `.github/workflows/release.yml`
**Cause:** A tag can publish without required format/lint/tests, offline and
dedicated-account live acceptance, migrations, or crash/recovery drills. Fixes
still marked unverified can ship as stable.
**Required fix/test:** Publish immutable RC artifacts only after all workspace
checks, full managed mode matrix, migration/power-loss recovery, clean install/
upgrade tests, and explicit sign-off for every critical/high integrity issue.
---
## B65 — Release Artifacts Lack Supply-Chain Verification (MED-11)
**Status:** Open — 1.0 release/security blocker
**Found:** 2026-07-22, 1.0 supply-chain audit
**Where:** CI and release workflows
**Cause:** Releases do not enforce advisory/license policy or publish signed
provenance, SBOMs, and verifiable checksums for exact distributed artifacts.
**Required fix/test:** Add locked audit/license gates, SBOM and provenance,
checksums and signatures, documented verification, and an independent clean-job
verification/install test.
---
## B66 — Local-Only Pending Data Is Not Protected at Shutdown (HIGH-21)
**Status:** Open — 1.0 data-safety/UX blocker
**Found:** 2026-07-22, 1.0 recovery audit
**Where:** shutdown, CLI/GUI status, logout/unregister workflows
**Cause:** Undrained writes may exist only on one machine, but there is no flush
command with a completion contract, prominent pending byte/count health, or
shutdown/logout warning/inhibition.
**Required fix/test:** Add `pdfs sync flush`, expose pending operations/bytes
and last errors, and protect destructive lifecycle actions. Test offline stop,
logout, unregister, reboot, retry exhaustion, and drain. Credential removal must
never delete staged bytes.
---
## B67 — Fresh-State Restore Can Silently Select a New Device (HIGH-22)
**Status:** Open — 1.0 recovery blocker
**Found:** 2026-07-22, 1.0 disaster-recovery audit
**Where:** device discovery/adoption; `docs/RECOVERY.md`
**Cause:** Restore depends on hostname and folder basename. After local-state
loss, a mismatch can silently create/select another device rather than attach
the intended remote roots, making recovery appear complete while data is absent.
**Required fix/test:** Require explicit adoption/confirmation on ambiguity and
show old/proposed device and root IDs. Test state loss, hostname change,
duplicates, renamed folders, multiple devices, and byte-identical restoration.