From 8be0f04b61b307caf1ac28f0f94d2b9f2e007a1e Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Wed, 22 Jul 2026 14:49:06 +0200 Subject: [PATCH 1/2] 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. --- .github/workflows/ci.yml | 3 + .github/workflows/release.yml | 24 +- Cargo.lock | 1 + crates/pdfs-core/Cargo.toml | 1 + crates/pdfs-core/src/cache.rs | 155 ++++++++++--- crates/pdfs-core/src/config.rs | 119 ++++++++-- crates/pdfs-core/src/db/migrations.rs | 7 +- crates/pdfs-core/src/db/ops.rs | 13 +- crates/pdfs-core/src/db/tests.rs | 68 ++++++ crates/pdfs-fuse/src/background.rs | 32 ++- crates/pdfs-fuse/src/control.rs | 128 ++++++++++- crates/pdfs-fuse/src/filesystem.rs | 15 +- crates/pdfs-fuse/src/lib.rs | 28 ++- crates/pdfs-fuse/src/sync.rs | 175 +++++++++++--- crates/pdfs-fuse/src/sync/planner.rs | 18 +- docs/BUGS.md | 314 ++++++++++++++++++++++++++ 16 files changed, 976 insertions(+), 125 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0cb3817..63b577d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -44,6 +44,9 @@ jobs: - name: Unit tests run: cargo test --workspace --locked + - name: Account-free filesystem acceptance + run: scripts/fuse-acceptance.sh --offline-only + - name: Acceptance runner safety guard run: | if scripts/fuse-acceptance.sh /tmp >acceptance.out 2>&1; then diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 700ea74..090571e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -35,11 +35,29 @@ jobs: - name: Set up Rust uses: actions-rust-lang/setup-rust-toolchain@v1 with: - toolchain: stable + toolchain: '1.96' + components: rustfmt, clippy - name: Cache Cargo target 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 run: | cargo build --release --locked @@ -76,6 +94,8 @@ jobs: mkdir -p deb/usr/share/applications mkdir -p deb/usr/share/icons/hicolor/scalable/apps mkdir -p deb/usr/share/licenses/proton-drive-linux + mkdir -p deb/usr/lib/systemd/user + mkdir -p deb/etc/xdg/autostart # Copy binaries cp target/release/pdfs deb/usr/bin/ @@ -85,7 +105,9 @@ jobs: # Copy desktop integration files 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/proton-drive.service deb/usr/lib/systemd/user/ cp LICENSE deb/usr/share/licenses/proton-drive-linux/ # Calculate Installed-Size in KB for the control file diff --git a/Cargo.lock b/Cargo.lock index b42a492..868e407 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3538,6 +3538,7 @@ dependencies = [ "directories", "ignore", "keyring", + "libc", "parking_lot", "proton-drive-rs", "proton-sdk", diff --git a/crates/pdfs-core/Cargo.toml b/crates/pdfs-core/Cargo.toml index 4c5880d..fa88d05 100644 --- a/crates/pdfs-core/Cargo.toml +++ b/crates/pdfs-core/Cargo.toml @@ -21,3 +21,4 @@ sha2.workspace = true rusqlite.workspace = true keyring.workspace = true parking_lot.workspace = true +libc.workspace = true diff --git a/crates/pdfs-core/src/cache.rs b/crates/pdfs-core/src/cache.rs index 8e7ccec..2ae3589 100644 --- a/crates/pdfs-core/src/cache.rs +++ b/crates/pdfs-core/src/cache.rs @@ -241,7 +241,9 @@ impl ContentCache { let recovery_dir = content_dir.join("recovery"); std::fs::create_dir_all(&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)?; // Staging, by contrast, is deliberately preserved across runs. let staging_dir = content_dir.join("staging"); @@ -669,18 +671,24 @@ impl ContentCache { pub fn create_scratch(&self) -> Result<(std::fs::File, PathBuf)> { use std::sync::atomic::{AtomicU64, Ordering}; static N: AtomicU64 = AtomicU64::new(0); - let path = self.scratch_dir.join(format!( - "w-{}-{}", - std::process::id(), - N.fetch_add(1, Ordering::Relaxed) - )); - let file = std::fs::OpenOptions::new() - .create(true) - .truncate(true) - .read(true) - .write(true) - .open(&path)?; - Ok((file, path)) + loop { + let path = self.scratch_dir.join(format!( + "w-{}-{}-{}", + std::process::id(), + now_millis(), + N.fetch_add(1, Ordering::Relaxed) + )); + match std::fs::OpenOptions::new() + .create_new(true) + .read(true) + .write(true) + .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 @@ -709,16 +717,23 @@ impl ContentCache { /// have synced `scratch` first: a sidecar promising bytes that never reached /// the disk is worse than no sidecar. pub fn mark_scratch_durable(&self, scratch: &Path, meta: &StagedWrite) -> Result<()> { + use std::io::Write as _; + let side = Self::scratch_sidecar(scratch); 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)?; // The rename is what publishes the sidecar; fsync the directory so the // rename itself survives, not just the bytes it points at. - if let Some(dir) = side.parent() - && let Ok(d) = std::fs::File::open(dir) - { - let _ = d.sync_all(); + if let Some(dir) = side.parent() { + std::fs::File::open(dir)?.sync_all()?; } 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). pub fn emergency_evict(&self) { let pinned: HashSet = self @@ -826,7 +856,8 @@ impl ContentCache { /// 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 - /// 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)> { let Ok(entries) = std::fs::read_dir(&self.recovery_dir) else { return Vec::new(); @@ -837,14 +868,11 @@ impl ContentCache { if blob.extension().is_some_and(|e| e == "json") { continue; } - match std::fs::read(Self::scratch_sidecar(&blob)) + if let Some(meta) = std::fs::read(Self::scratch_sidecar(&blob)) .ok() .and_then(|b| serde_json::from_slice::(&b).ok()) { - Some(meta) => out.push((blob, meta)), - None => { - let _ = std::fs::remove_file(&blob); - } + out.push((blob, meta)); } } out @@ -872,6 +900,8 @@ impl ContentCache { /// failure here means we could not save the bytes at all, so it is reported /// rather than swallowed. pub fn stage_write(&self, meta: &StagedWrite, scratch: &Path) -> Result { + use std::io::Write as _; + static N: AtomicU64 = AtomicU64::new(0); // 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. @@ -882,18 +912,32 @@ impl ContentCache { now_millis(), N.fetch_add(1, Ordering::Relaxed) )); - if let Err(e) = std::fs::rename(scratch, &path) { - // Cross-device: rename cannot move it, so copy and drop the original. - let _ = e; + let copied = if let Err(e) = std::fs::rename(scratch, &path) { + if e.kind() != std::io::ErrorKind::CrossesDevices { + return Err(e.into()); + } 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, // while a sidecar without a file would describe nothing. - std::fs::write( - path.with_extension("json"), - serde_json::to_vec_pretty(meta)?, - )?; + let side = path.with_extension("json"); + let side_tmp = path.with_extension("json.tmp"); + 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) } @@ -1906,6 +1950,51 @@ mod tests { 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 /// `fsync` left must not offer recovery a second, stale copy of it. #[test] diff --git a/crates/pdfs-core/src/config.rs b/crates/pdfs-core/src/config.rs index 6b5a0a2..df73b2d 100644 --- a/crates/pdfs-core/src/config.rs +++ b/crates/pdfs-core/src/config.rs @@ -105,11 +105,21 @@ impl AppDirs { /// Load config from disk, creating default if missing. pub fn load_config(&self) -> AppConfig { let path = self.config_path(); - if path.exists() - && let Ok(content) = std::fs::read_to_string(&path) - && let Ok(config) = serde_json::from_str::(&content) - { - return config; + if path.exists() { + match std::fs::read_to_string(&path) + .map_err(Error::from) + .and_then(|content| { + serde_json::from_str::(&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(); @@ -125,10 +135,26 @@ impl AppDirs { /// cache budget or mountpoint; the daemon re-reads the file on its next mount. pub fn save_config(&self, config: &AppConfig) -> Result<()> { let path = self.config_path(); - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; - } - std::fs::write(&path, serde_json::to_string_pretty(config)?)?; + let parent = path + .parent() + .ok_or_else(|| Error::Other("configuration path has no parent".into()))?; + 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(()) } @@ -211,24 +237,41 @@ impl AppDirs { pub fn ensure(&self) -> Result<()> { for dir in [self.state_dir(), self.cache_dir()] { std::fs::create_dir_all(&dir)?; - restrict_dir(&dir); + restrict_dir(&dir)?; } let config_dir = self.dirs.config_dir().to_path_buf(); - if std::fs::create_dir_all(&config_dir).is_ok() { - restrict_dir(&config_dir); - } + std::fs::create_dir_all(&config_dir)?; + restrict_dir(&config_dir)?; Ok(()) } } -/// Make `dir` owner-only (`0700`). Best effort: a mode we cannot set (an -/// exotic filesystem, a directory we do not own) is not worth refusing to -/// start over, and the caller has no better answer than continuing. -fn restrict_dir(dir: &Path) { - use std::os::unix::fs::PermissionsExt; - if let Err(e) = std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o700)) { - tracing::warn!(path = %dir.display(), error = %e, "could not restrict directory to 0700"); +/// Make `dir` owner-only (`0700`). These directories contain decrypted bytes, +/// plaintext names, and an authenticated control socket, so failure is fatal. +fn restrict_dir(dir: &Path) -> Result<()> { + use std::os::unix::fs::{MetadataExt as _, PermissionsExt}; + let before = std::fs::symlink_metadata(dir)?; + if before.file_type().is_symlink() || !before.is_dir() { + 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`). @@ -263,29 +306,57 @@ mod tests { /// and the control socket that commands the daemon's session. #[test] 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); std::fs::create_dir_all(&dir).unwrap(); // Start deliberately world-readable, as `create_dir_all` under a typical // umask would leave it. 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"); 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 /// session with no credential, so it is an authority boundary rather than /// merely private data. #[test] 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); std::fs::create_dir_all(&dir).unwrap(); 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(); diff --git a/crates/pdfs-core/src/db/migrations.rs b/crates/pdfs-core/src/db/migrations.rs index d9a1e37..93d6349 100644 --- a/crates/pdfs-core/src/db/migrations.rs +++ b/crates/pdfs-core/src/db/migrations.rs @@ -31,7 +31,12 @@ impl Db { .and_then(|v| v.parse().ok()) .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(()); } diff --git a/crates/pdfs-core/src/db/ops.rs b/crates/pdfs-core/src/db/ops.rs index 501b6a4..07d7098 100644 --- a/crates/pdfs-core/src/db/ops.rs +++ b/crates/pdfs-core/src/db/ops.rs @@ -118,9 +118,10 @@ pub struct AttachedBlob { impl Db { pub fn enqueue_op(&self, op: &PendingOp) -> Result<(i64, Option)> { - let conn = self.conn.lock(); + let mut conn = self.conn.lock(); + let tx = conn.transaction()?; let superseded: Option = if op_supersedes(&op.kind) { - let blob: Option = conn + let blob: Option = tx .query_row( "SELECT blob_path FROM pending_op WHERE uid = ?1 AND kind = ?2", params![op.uid, op.kind], @@ -128,7 +129,7 @@ impl Db { ) .optional()? .flatten(); - conn.execute( + tx.execute( "DELETE FROM pending_op WHERE uid = ?1 AND kind = ?2", params![op.uid, op.kind], )?; @@ -136,7 +137,7 @@ impl Db { } else { None }; - conn.execute( + tx.execute( "INSERT INTO pending_op (kind, uid, parent_uid, name, blob_path, meta_json, created_at, next_attempt_at) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)", @@ -151,7 +152,9 @@ impl Db { 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 diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index cd7a303..bf6f9c4 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -784,6 +784,41 @@ fn opens_and_migrates() { 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. /// /// 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); } +#[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 /// 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 diff --git a/crates/pdfs-fuse/src/background.rs b/crates/pdfs-fuse/src/background.rs index 7bb12eb..2414691 100644 --- a/crates/pdfs-fuse/src/background.rs +++ b/crates/pdfs-fuse/src/background.rs @@ -126,12 +126,16 @@ pub(super) async fn run_event_sync( match client.enumerate_events(&scope, None).await { Ok(events) => { let head = events.last().map(|e| e.id().clone()); - if let Some(c) = &head - && let Err(e) = db.set_event_cursor(c.as_str()) - { - warn!(error = %e, "persist seed cursor failed"); + let Some(c) = &head else { + break None; + }; + 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) => { 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 // our FUSE state, so nothing else does this. 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, ¬ifier, event); - } - cursor = events.last().map(|e| e.id().clone()); - if let Some(c) = &cursor - && let Err(e) = db.set_event_cursor(c.as_str()) - { - warn!(error = %e, "persist event cursor failed"); + let applied = event.id().clone(); + if let Err(e) = db.set_event_cursor(applied.as_str()) { + warn!(error = %e, "persist event cursor failed; retaining prior cursor for retry"); + break; + } + // 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); } } } diff --git a/crates/pdfs-fuse/src/control.rs b/crates/pdfs-fuse/src/control.rs index e58cfa6..d16ba59 100644 --- a/crates/pdfs-fuse/src/control.rs +++ b/crates/pdfs-fuse/src/control.rs @@ -12,8 +12,8 @@ use std::io::{BufRead, BufReader, Write as _}; use std::os::unix::net::{UnixListener, UnixStream}; use std::path::{Path, PathBuf}; -use std::sync::atomic::Ordering; -use std::time::Instant; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::time::{Duration, Instant}; use pdfs_core::config::AppDirs; use pdfs_core::control::{ @@ -27,6 +27,67 @@ use tracing::{info, warn}; use super::transfers::CountingReader; 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 { + 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> { + 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 /// must live under `mountpoint`; a relative path is taken as already relative to /// the mount root. @@ -44,6 +105,10 @@ fn rel_to_mount(mountpoint: &Path, path: &str) -> CoreResult { /// Handle one control-socket connection: read a single JSON request line, /// dispatch it against `core`, and write back a JSON response line. 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() { Ok(s) => s, Err(e) => { @@ -51,10 +116,14 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U return; } }); - let mut line = String::new(); - if reader.read_line(&mut line).is_err() || line.trim().is_empty() { - return; - } + let line = match read_request_line(&mut reader) { + Ok(Some(line)) if !line.trim().is_empty() => line, + Ok(_) => return, + Err(e) => { + warn!(error = %e, "control: rejected request"); + return; + } + }; let response = match serde_json::from_str::(line.trim()) { Ok(CtlRequest::Status) => { let pins = core.cache.list_pins(); @@ -792,12 +861,22 @@ pub(crate) fn run_control_socket( for conn in listener.incoming() { match conn { 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 username = username.clone(); let mountpoint = mountpoint.clone(); if let Err(e) = std::thread::Builder::new() .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"); } @@ -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 + ); + } +} diff --git a/crates/pdfs-fuse/src/filesystem.rs b/crates/pdfs-fuse/src/filesystem.rs index 8ad17dd..c2ba6cf 100644 --- a/crates/pdfs-fuse/src/filesystem.rs +++ b/crates/pdfs-fuse/src/filesystem.rs @@ -1129,17 +1129,16 @@ impl Filesystem for ProtonFs { // 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 // the source moved nor the destination intact. - if newname != name { - if let Err(e) = self + if newname != name + && let Err(e) = self .core .rt .block_on(self.core.client.rename_node(&uid, &newname, None)) - { - error!(%uid, error = %e, "rename failed"); - self.core.restore_replaced(victim.as_ref(), &newname); - reply.error(Errno::EIO); - return; - } + { + error!(%uid, error = %e, "rename failed"); + self.core.restore_replaced(victim.as_ref(), &newname); + reply.error(Errno::EIO); + return; } if newparent != parent { let mut attempts = 0u32; diff --git a/crates/pdfs-fuse/src/lib.rs b/crates/pdfs-fuse/src/lib.rs index e373d22..0e0e37a 100644 --- a/crates/pdfs-fuse/src/lib.rs +++ b/crates/pdfs-fuse/src/lib.rs @@ -1548,8 +1548,32 @@ impl Core { ); } Err(e) => { - error!(%uid, error = %e, "staging write failed; bytes lost"); - let _ = std::fs::remove_file(src); + // `src` is the only known copy of bytes the kernel already + // 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, + ); } } } diff --git a/crates/pdfs-fuse/src/sync.rs b/crates/pdfs-fuse/src/sync.rs index 39ba35b..8f7fdeb 100644 --- a/crates/pdfs-fuse/src/sync.rs +++ b/crates/pdfs-fuse/src/sync.rs @@ -55,9 +55,11 @@ use local_scan::is_write_mode; use local_scan::open_for_write_set; use model::{LocalItem, Outcome, PassAbort, Pending, RemoteItem}; pub(crate) use planner::base_name; +#[cfg(test)] +use planner::conflict_path; use planner::{ - FilePlan, classification_order, conflict_path, filter_baseline, guard_local_wipe, join_rel, - parent_rel, plan_file, rel_to_path, remote_sig, unchanged_remote_size, + FilePlan, classification_order, conflict_path_with_suffix, filter_baseline, guard_local_wipe, + 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 /// engine thread (never inside a task). @@ -711,7 +713,17 @@ impl Core { } }, 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); self.log_activity(ActivityKind::Trash, base_name(rel), "removed locally", true); outcome.deleted += 1; @@ -758,7 +770,12 @@ impl Core { // Deferred folder deletions, deepest first. delete_local_dirs.sort_by_key(|p| std::cmp::Reverse(p.matches('/').count())); 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); self.log_activity( ActivityKind::Trash, @@ -802,18 +819,24 @@ impl Core { out: &mut HashMap, ) -> Result<(), String> { 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 meta = match std::fs::symlink_metadata(&path) { - Ok(m) => m, - Err(_) => continue, - }; - let Ok(stripped) = path.strip_prefix(root) else { - continue; - }; - let Some(rel) = stripped.to_str() else { - continue; - }; + let meta = std::fs::symlink_metadata(&path) + .map_err(|e| format!("stat {}: {e}", path.display()))?; + let stripped = path.strip_prefix(root).map_err(|e| { + format!( + "local path {} escaped sync root {}: {e}", + path.display(), + root.display() + ) + })?; + 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. if rel.contains(".pdfs-tmp-") { continue; @@ -1153,12 +1176,9 @@ impl Core { // pass), then take the remote version as the shared truth. let path = local_root.join(rel_to_path(rel)); if path.exists() { - let conflict = conflict_path(&path, now_secs()); - if let Err(e) = std::fs::rename(&path, &conflict) { - warn!(rel, error = %e, "sync: could not set aside conflict copy"); - } else { - info!(rel, "sync: kept local changes as a conflict copy"); - } + preserve_conflict_copy(&path, now_secs()) + .map_err(|e| format!("set aside conflict copy for {rel}: {e}"))?; + info!(rel, "sync: kept local changes as a conflict copy"); } self.download_file(folder_id, local_root, rel, uid, *mtime, *size) .await?; @@ -1171,8 +1191,7 @@ impl Core { // evicted. Push the conflict copy now, while it still exists, and // leave the remote file holding the original path. let path = local_root.join(rel_to_path(rel)); - let conflict = conflict_path(&path, now_secs()); - std::fs::rename(&path, &conflict) + let conflict = preserve_conflict_copy(&path, now_secs()) .map_err(|e| format!("set aside conflict copy for {rel}: {e}"))?; let name = conflict .file_name() @@ -1297,13 +1316,24 @@ impl Core { let _ = std::fs::remove_file(&tmp); 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}"))?; // 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.set_modified(SystemTime::UNIX_EPOCH + Duration::from_secs(mtime.max(0) as u64)); + let f = std::fs::File::options() + .write(true) + .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 } @@ -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 { + 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 -------------------------------------------------------------- /// 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(), "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 = [("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. assert!(guard_local_wipe(&none, &none).is_ok()); // 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)")); } + #[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 ----------------------------------- #[test] diff --git a/crates/pdfs-fuse/src/sync/planner.rs b/crates/pdfs-fuse/src/sync/planner.rs index 37000e5..2942801 100644 --- a/crates/pdfs-fuse/src/sync/planner.rs +++ b/crates/pdfs-fuse/src/sync/planner.rs @@ -99,7 +99,7 @@ pub(super) fn guard_local_wipe( baseline: &HashMap, local: &HashMap, ) -> 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!( "every one of the {} synced paths is missing locally; refusing to trash \ 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`. +#[cfg(test)] 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 ext = path.extension().and_then(|s| s.to_str()); + let suffix = if suffix == 0 { + String::new() + } else { + format!("-{suffix}") + }; let name = match ext { - Some(ext) => format!("{stem} (sync-conflict {stamp}).{ext}"), - None => format!("{stem} (sync-conflict {stamp})"), + Some(ext) => format!("{stem} (sync-conflict {stamp}{suffix}).{ext}"), + None => format!("{stem} (sync-conflict {stamp}{suffix})"), }; match path.parent() { Some(dir) => dir.join(name), diff --git a/docs/BUGS.md b/docs/BUGS.md index a0edc33..e756f94 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -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 overwritten by the kernel's response processing. Uids are immutable and never 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. From 0d8a474442f7bd90c7d0c669fec33b07ebff4810 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Wed, 22 Jul 2026 15:11:33 +0200 Subject: [PATCH 2/2] version bump --- Cargo.lock | 8 ++++---- Cargo.toml | 2 +- docs/BUGS.md | 34 +++++++++++++++++++++++++++++----- packaging/PKGBUILD | 2 +- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 868e407..f36dd3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3518,7 +3518,7 @@ checksum = "35fb2e5f958ec131621fdd531e9fc186ed768cbe395337403ae56c17a74c68ec" [[package]] name = "pdfs-cli" -version = "0.7.1" +version = "1.0.0" dependencies = [ "anyhow", "clap 4.6.2", @@ -3533,7 +3533,7 @@ dependencies = [ [[package]] name = "pdfs-core" -version = "0.7.1" +version = "1.0.0" dependencies = [ "directories", "ignore", @@ -3553,7 +3553,7 @@ dependencies = [ [[package]] name = "pdfs-fuse" -version = "0.7.1" +version = "1.0.0" dependencies = [ "fuser", "image", @@ -3569,7 +3569,7 @@ dependencies = [ [[package]] name = "pdfs-gui" -version = "0.7.1" +version = "1.0.0" dependencies = [ "anyhow", "async-channel", diff --git a/Cargo.toml b/Cargo.toml index f40ac45..d45b70d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "2" members = ["crates/pdfs-core", "crates/pdfs-fuse", "crates/pdfs-cli", "crates/pdfs-gui"] [workspace.package] -version = "0.7.1" +version = "1.0.0" edition = "2024" license = "MIT" authors = ["Nils Pukropp "] diff --git a/docs/BUGS.md b/docs/BUGS.md index e756f94..182c7dd 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -1206,7 +1206,8 @@ retracted. ## B19 — Remote Folder Trashing on Mode Switch Failure (CRIT-02) -**Status:** Fixed in code (unverified on a live mount, 2026-07-22) +**Status:** Fixed; all four managed-live mode pairs verified 2026-07-22, +fault-injected mount failure still pending **Found:** 2026-07-21, multi-agent sync engine audit (`audit_bugs.md` CRIT-02) **Where:** `crates/pdfs-fuse/src/devices.rs`, `apply_sync_folder_mode` @@ -1220,8 +1221,9 @@ user deletion, evict the underlying local mirror while holding the sync-folder lock, and only then mount FUSE. A mount failure leaves an inert `ondemand` row; switching back to `mirror` clears the baseline and restores the local copy. -**Verified:** `cargo clippy -p pdfs-fuse --all-targets -- -D warnings` and the -`pdfs-fuse` unit suite pass. A fault-injected/live mode-switch test is still needed. +**Verified:** `cargo clippy -p pdfs-fuse --all-targets -- -D warnings`, the +`pdfs-fuse` unit suite, and the complete managed-live matrix passed against the +1.0.0 release binary. A fault-injected mount-failure test is still needed. --- @@ -1646,7 +1648,7 @@ and combined move/rename cases. ## B47 — FUSE Accepts Path Components Longer Than NAME_MAX -**Status:** Fixed (unverified) +**Status:** Fixed and managed-live verified (2026-07-22) **Found:** 2026-07-22, managed FUSE acceptance suite **Where:** `crates/pdfs-fuse/src/filesystem.rs`, name-taking callbacks @@ -2027,7 +2029,7 @@ 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 +**Status:** Partly fixed — automated gates and managed-live matrix pass; recovery approval remains open **Found:** 2026-07-22, 1.0 release audit **Where:** CI and `.github/workflows/release.yml` @@ -2086,3 +2088,25 @@ 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. + +--- + +## B68 — Profile Backup Cannot Be Written to the Device Root + +**Status:** Open — live-reproduced 2026-07-22 +**Found:** complete managed-live mode matrix against the 1.0.0 release binary +**Where:** `crates/pdfs-fuse/src/profile.rs`, profile backup destination + +**Repro:** Every sync-folder registration or mode transition attempted the +profile backup and received `NotEnoughPermissions` (HTTP 422): `Cannot create +file at the root of a device`. The filesystem matrix itself passed, but profile +backup never became durable remotely. + +**Impact:** The recovery feature appears active but does not preserve the device +profile, so a replacement machine still lacks modes, pins, mount settings, and +the explicit remote-folder mapping. Repeated retries also add noisy warnings. + +**Required fix/test:** Store the profile below a writable application folder +rather than directly at the device root, reuse that folder by stable identity, +and make backup health visible. Verify upload, replacement, restart restore, and +byte-identical fresh-state recovery on a dedicated account. diff --git a/packaging/PKGBUILD b/packaging/PKGBUILD index 6c05f1b..97f4b20 100644 --- a/packaging/PKGBUILD +++ b/packaging/PKGBUILD @@ -9,7 +9,7 @@ # Build + install: cd packaging && makepkg -fi pkgname=proton-drive-linux -pkgver=0.7.1 +pkgver=1.0.0 pkgrel=1 pkgdesc="Proton Drive client for Linux: FUSE files-on-demand mount, CLI, GTK4 app + tray" arch=('x86_64')