From 7e8403936b541cef4cae1bae797512329a285570 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Fri, 31 Jul 2026 09:46:51 +0200 Subject: [PATCH] fixed permission error on on-demand mounts --- crates/pdfs-fuse/src/state.rs | 151 +++++++++++++++++++++++++++++++++- docs/BUGS.md | 73 ++++++++++++++++ 2 files changed, 223 insertions(+), 1 deletion(-) diff --git a/crates/pdfs-fuse/src/state.rs b/crates/pdfs-fuse/src/state.rs index 7613824..ddbcb4a 100644 --- a/crates/pdfs-fuse/src/state.rs +++ b/crates/pdfs-fuse/src/state.rs @@ -247,6 +247,17 @@ impl State { self.access_changes.drain().collect() } + /// Whether `uid` lives on the volume this mount is rooted in. + /// + /// Used to decide how a node with no resolvable parent is classified: + /// our own volume fails open (it is owned content), a foreign volume + /// fails closed (it can only be there because of a share). + pub(crate) fn is_own_volume(&self, uid: &NodeUid) -> bool { + self.entries + .get(&crate::ROOT_INO) + .is_some_and(|root| root.uid.volume_id == uid.volume_id) + } + fn stored_share_access(&self, uid: &NodeUid) -> Option { self.share_access.get(uid).copied() } @@ -506,12 +517,28 @@ impl State { } else if let Some(stored) = stored { stored } else if !state.entries.contains_key(&entry.parent) { + // The parent is not resident, so there is nothing to inherit + // from. This is the normal shape of a device-folder root: its + // parent is the device root, which is never persisted as a + // node, so every on-demand folder hydrates parentless here. + // Answer the way the persisted authority (`effective_node_access`) + // does — a recorded share row above it if we have one, otherwise + // fail open on our own volume and closed on a foreign one. + // Failing closed on our own volume denies every write in every + // secondary mount, because `require_uid_writable` intersects + // this state with the mount that actually owns the node. entry .node .parent_uid .as_ref() .and_then(|parent_uid| state.share_access.get(parent_uid).copied()) - .unwrap_or(Access::Unknown) + .unwrap_or_else(|| { + if state.is_own_volume(&entry.uid) { + Access::Owner + } else { + Access::Unknown + } + }) } else { let parent_access = resolve(state, entry.parent, memo, visiting); if entry.node.membership.is_none() || parent_access != Access::Owner { @@ -1261,6 +1288,128 @@ mod tests { assert!(st.require_writable(owned_root).is_ok()); } + /// Materialize `nodes` the way `Core::hydrate` does: allocate every inode + /// first, then insert entries with `Access::Unknown`, resolving each parent + /// by uid and parking a node whose parent never made it to disk at + /// `ORPHAN_INO`. `nodes[0]` becomes the mount root. + fn hydrate_from(st: &mut State, nodes: &[Node]) -> Vec { + for node in nodes { + st.db.upsert_node(node).unwrap(); + } + let inos: Vec = nodes + .iter() + .enumerate() + .map(|(i, node)| { + let ino = crate::ROOT_INO + i as u64; + st.by_uid.insert(node.uid.clone(), ino); + ino + }) + .collect(); + st.next_ino = crate::ROOT_INO + nodes.len() as u64; + // Reverse order so entries are inserted before their parents, matching + // the hydration path's non-topological materialization. + for (i, node) in nodes.iter().enumerate().rev() { + let ino = inos[i]; + let parent = if ino == crate::ROOT_INO { + crate::ROOT_INO + } else { + node.parent_uid + .as_ref() + .and_then(|p| st.by_uid.get(p).copied()) + .unwrap_or(crate::ORPHAN_INO) + }; + st.entries.insert( + ino, + Entry { + uid: node.uid.clone(), + parent, + node: node.clone(), + access: Access::Unknown, + lookup_count: 1, + open_count: 0, + unlinked: false, + }, + ); + } + st.hydrate_access(); + inos + } + + /// A device folder's parent is the device root, which is never persisted as + /// a node, so the folder hydrates with no resident parent. Classifying that + /// as `Unknown` denied every write in every on-demand mount: the mount's own + /// fork says `Owner`, but `require_uid_writable` intersects it with this + /// state, and the intersection is what reaches the queue guards. + #[test] + fn own_volume_nodes_without_a_resident_parent_stay_owned() { + let (mut st, _dir) = state(); + let nodes = [ + node("root", "none", "My Files", true), + // `device-root` is deliberately absent from `nodes`. + node("documents", "device-root", "Documents", true), + node("folder", "documents", "folder", true), + node("leaf", "folder", "leaf.txt", false), + ]; + let inos = hydrate_from(&mut st, &nodes); + + for ino in inos { + assert_eq!(st.entries[&ino].access, Access::Owner); + assert!(st.require_writable(ino).is_ok()); + } + } + + /// The same shape on a foreign volume can only be there because of a share, + /// so it keeps failing closed. + #[test] + fn foreign_volume_nodes_without_a_resident_parent_fail_closed() { + let (mut st, _dir) = state(); + let foreign = |link: &str, parent: &str, name: &str, is_dir: bool| { + let mut node = node(link, parent, name, is_dir); + node.uid = NodeUid::new(VolumeId::from("other-vol"), LinkId::from(link)); + node.parent_uid = Some(NodeUid::new( + VolumeId::from("other-vol"), + LinkId::from(parent), + )); + node + }; + let nodes = [ + node("root", "none", "My Files", true), + // `their-share` is absent, so the subtree has no resident parent. + foreign("their-folder", "their-share", "Team Budget", true), + foreign("their-leaf", "their-folder", "budget.ods", false), + ]; + let inos = hydrate_from(&mut st, &nodes); + + assert_eq!(st.entries[&inos[0]].access, Access::Owner); + for ino in &inos[1..] { + assert_eq!(st.entries[ino].access, Access::Unknown); + assert_eq!(st.require_writable(*ino).unwrap_err().code(), libc::EACCES); + } + } + + /// Fail-open is only for the absence of a recorded authority: a share row on + /// the missing parent still governs the subtree below it. + #[test] + fn persisted_authority_outranks_the_own_volume_fail_open() { + let (mut st, _dir) = state(); + st.db + .set_share_access(&uid("device-root"), Access::Viewer) + .unwrap(); + st.share_access = st.db.all_share_access().unwrap(); + let nodes = [ + node("root", "none", "My Files", true), + node("documents", "device-root", "Documents", true), + node("leaf", "documents", "leaf.txt", false), + ]; + let inos = hydrate_from(&mut st, &nodes); + + assert_eq!(st.entries[&inos[0]].access, Access::Owner); + for ino in &inos[1..] { + assert_eq!(st.entries[ino].access, Access::Viewer); + assert_eq!(st.require_writable(*ino).unwrap_err().code(), libc::EACCES); + } + } + #[test] fn reinterned_share_root_propagates_a_viewer_downgrade() { let (mut st, _dir) = state(); diff --git a/docs/BUGS.md b/docs/BUGS.md index 7416150..69aca82 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,79 @@ Conventions: --- +## B79 — Every write in an on-demand device-folder mount fails EACCES + +**Status:** Fixed (verified live 2026-07-31 — packaged 1.2.1 is still affected +until the next build ships) +**Found:** 2026-07-31, user on packaged 1.2.1: `touch ~/Documents/test` → +`Permission denied`, while the same operation under `~/ProtonDrive` succeeds. +**Where:** `crates/pdfs-fuse/src/state.rs`, `State::hydrate_access` + +Not a mode-bit problem — the mount root reads `drwxr-xr-x` and the kernel's +`default_permissions` check passes. The denial comes from the queue guard: +`serve_create` → `Core::require_uid_writable`, which **intersects the access +every live inode space reports for that uid** (`lib.rs:721`) before anything +reaches `pending_op`. + +The on-demand fork itself says `Owner`. The **primary** state says `Unknown`: + +- `Core::hydrate` (`lib.rs:1012`) loads *every* `nodes` row into the primary + state, including the subtrees that belong to on-demand device folders — they + are on the same volume, in the same table. +- A device folder's `parent_uid` is the **device root**, which is never + persisted as a node. In this account's DB all six device-folder roots + (`Downloads`, `Pictures`, `Music`, `Documents`, `Videos`, `narl`) have + `parent_uid = ` with no matching row, so pass 2 parks them at + `ORPHAN_INO`. +- `hydrate_access` then hit its "parent is not resident" branch, which had no + answer and fell back to `Access::Unknown` — fail closed. +- `Unknown` is not writable, so the intersection denies every create, mkdir, + write, rename, setattr and trash in every on-demand mount, permanently: those + entries are never re-interned in the primary state, so nothing recomputes them. + +`~/ProtonDrive` was unaffected because its nodes descend from `ROOT_INO`, which +`ProtonFs::new` hardcodes to `Owner`. + +Two facts make this a *classification* bug rather than a policy one: the +persisted authority `Db::effective_node_access` already answers `Owner` for +exactly these uids (no `share_access` row covers them — the table holds only +`virtual~sharedwithme` and one accepted share), and the design table in +`mount-architecture.md` §2.2 says *not under a share, no role → Owner, fail +open*. The in-memory path disagreed with both. + +**Fix:** the non-resident-parent branch now mirrors the persisted authority — a +recorded `share_access` row above the node still wins, otherwise fail **open** +to `Owner` on the mount's own volume and fail **closed** to `Unknown` on a +foreign one (`State::is_own_volume`). Foreign-volume shared content keeps its +fail-closed behavior, which is what B34 needs. + +**Second-order effect, also fixed by the same change:** `downgrade_known_shared_access` +selects roots with `entry.access != Access::Owner`, so a `ScopeAccessLost` / +`SharedWithMeUpdated` event would additionally have force-downgraded every +device-folder tree to `Viewer` in memory. + +**Tests:** `state::tests::own_volume_nodes_without_a_resident_parent_stay_owned`, +`foreign_volume_nodes_without_a_resident_parent_fail_closed`, +`persisted_authority_outranks_the_own_volume_fail_open`. The first fails on the +pre-fix branch with `left: Unknown, right: Owner`. + +**Verified live (2026-07-31):** the packaged 1.2.1 daemon denies +`touch ~/Documents/pdfs-b79-test`, `mkdir`, and `>` redirect while +`~/ProtonDrive` accepts them; a local `--release` build of this fix accepts all +three on the same account and DB, the write drains (`pending_op` back to 0, the +new nodes carry real `G88km…` remote uids rather than local ones), and the +daemon log has no `403`/EACCES. `~/ProtonDrive/Shared with me/` stayed +`dr-xr-xr-x`, and the accepted editor-role share under it stayed writable — the +foreign-volume fail-closed path is unchanged. + +**Side note for whoever runs this next:** stopping the daemon and touching a +file in an on-demand *mountpoint* while it is unmounted leaves that file in the +local directory, and `restore_ondemand_mounts` then refuses to mount over the +non-empty dir (`WARN … local dir is not empty; refusing to mount over it`). +Empty the directory and restart the unit. + +--- + ## B78 — Profile backup fails: Cannot create file at the root of a device **Status:** Open