From 1612a9d07ac1f7858965b128680dc9f604cc1953 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Sun, 4 Oct 2026 22:15:01 +0200 Subject: [PATCH] fix(core): a file renamed just as its create lands keeps its access (B174) --- crates/pdfs-core/src/db/share_access.rs | 20 +++++++++++-- crates/pdfs-core/src/db/tests.rs | 39 +++++++++++++++++++++++++ docs/BUGS.md | 21 +++++++++++++ docs/CHANGELOG.md | 2 ++ docs/MILESTONE-3.0.0-STATUS.md | 5 ++-- 5 files changed, 83 insertions(+), 4 deletions(-) diff --git a/crates/pdfs-core/src/db/share_access.rs b/crates/pdfs-core/src/db/share_access.rs index 9ec0ede..cee43f5 100644 --- a/crates/pdfs-core/src/db/share_access.rs +++ b/crates/pdfs-core/src/db/share_access.rs @@ -95,11 +95,27 @@ impl Db { /// Resolve a persisted node's effective access through its nearest recorded /// share root. `None` means the node itself is not persisted, which callers /// must not treat as owned: stale handles need to fail closed. + /// + /// A stand-in ([`local_uid`](super::local_uid)) finds its row by local id, + /// so it still does once its create has landed and the row has the uid + /// Drive gave it. The tree learns that uid a moment later, and a rename in + /// between was refused (`docs/BUGS.md` B174). pub fn effective_node_access(&self, uid: &NodeUid) -> Result> { let conn = self.read(); + let key = uid.to_string(); + let key = match super::local_lid(&key) { + Some(lid) => conn + .query_row("SELECT uid FROM nodes WHERE lid = ?1", [lid], |row| { + row.get::<_, Option>(0) + }) + .optional()? + .flatten() + .unwrap_or(key), + None => key, + }; let exists: bool = conn.query_row( "SELECT EXISTS(SELECT 1 FROM nodes WHERE uid = ?1 AND node_json IS NOT NULL)", - [uid.to_string()], + [&key], |row| row.get(0), )?; if !exists { @@ -126,7 +142,7 @@ impl Db { FROM ancestors a JOIN share_access sa ON sa.root_uid = a.uid ORDER BY a.depth ASC LIMIT 1", - [uid.to_string()], + [&key], |row| row.get::<_, String>(0), ) .optional()?; diff --git a/crates/pdfs-core/src/db/tests.rs b/crates/pdfs-core/src/db/tests.rs index 33e2554..fd47dbc 100644 --- a/crates/pdfs-core/src/db/tests.rs +++ b/crates/pdfs-core/src/db/tests.rs @@ -3243,6 +3243,45 @@ fn an_op_queued_under_a_stand_in_after_its_create_landed_goes_to_the_landed_node assert_eq!(claimed.uid, real, "the drain sends the real uid"); } +#[test] +fn a_stand_in_keeps_its_access_once_its_create_has_landed() { + let db = Db::open_in_memory().unwrap(); + db.upsert_node(&folder("parent", None, "My Files")).unwrap(); + let parent = uid("parent").to_string(); + let lid = db.add_local_row(&parent, "new.txt", false, 1).unwrap(); + let local = local_uid(lid); + let stand_in = NodeUid::new(VolumeId::from("local"), LinkId::from(lid.to_string())); + let mut node = file("new", "parent", "new.txt", 1); + node.uid = stand_in.clone(); + db.upsert_node(&node).unwrap(); + let real = uid("real").to_string(); + let (id, _) = db + .enqueue_op(&op_on(OP_CREATE, &local, Some(&parent))) + .unwrap(); + assert_eq!( + db.effective_node_access(&stand_in).unwrap(), + Some(crate::Access::Owner) + ); + db.claim_next_due_op(1).unwrap().unwrap(); + db.finish_create( + id, + Some("/staging/create"), + &landing(&local, &real, &parent, "new.txt"), + |_| None, + ) + .unwrap(); + + // The tree still names the file by its stand-in for a moment. + assert_eq!( + db.effective_node_access(&stand_in).unwrap(), + Some(crate::Access::Owner) + ); + assert_eq!( + db.effective_node_access(&uid("real")).unwrap(), + Some(crate::Access::Owner) + ); +} + #[test] fn a_node_queued_under_both_its_uids_is_sent_one_op_at_a_time() { let db = Db::open_in_memory().unwrap(); diff --git a/docs/BUGS.md b/docs/BUGS.md index 9c8781e..5a1249d 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -12,6 +12,27 @@ Conventions: --- +## B174 — A file renamed just as its create lands is refused with "Permission denied" + +**Status:** Fixed (unverified). +**Found:** 2026-10-04, by the simulation suite on 378667f (`one_client_on_a_good_link`, seed 6): +`rename c.bin -> b.txt` answered `EACCES` right after a settle. Not reproduced in three replays. + +**Where:** `Db::effective_node_access` in `crates/pdfs-core/src/db/share_access.rs`. + +**Cause.** Every change on the mount first asks whether the node may be written, and a node +with no row is refused. When a create lands, its row takes the uid Drive gave it before the +tree does (B173's gap). A rename in between asked by the stand-in the tree still had, found no +row under it, and was refused. + +**Fix.** A stand-in is looked up by its local id, so it finds its row before and after the +create lands. + +**Test:** `a_stand_in_keeps_its_access_once_its_create_has_landed` in +`crates/pdfs-core/src/db/tests.rs`. + +--- + ## B173 — A file deleted just as its create lands stays on Drive **Status:** Fixed (unverified). diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 60d4aab..c709e42 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -74,6 +74,8 @@ by its local id, in the queue, the stored tree and pins alike. longer waits for the upload's next retry (B171). - **A file deleted just as it reached Drive is removed from Drive too.** It no longer stayed there, and the next file with that name no longer became a conflict copy (B173). +- **Renaming a file just as it reaches Drive no longer fails with "Permission denied"** + (B174). - **A file renamed just as its upload finished keeps its new name.** The mount no longer showed the old name, and a second rename no longer failed with "No such file or directory" (B152, B172). diff --git a/docs/MILESTONE-3.0.0-STATUS.md b/docs/MILESTONE-3.0.0-STATUS.md index 68adadb..3cd894d 100644 --- a/docs/MILESTONE-3.0.0-STATUS.md +++ b/docs/MILESTONE-3.0.0-STATUS.md @@ -9,7 +9,7 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase - **Phase 2**: local ids, inode = local id, one tree for every mount (2.10.0). - **Phase 3, first part**: mkdir, create, rename, unlink and rmdir on the mount are recorded locally and sent from the queue; `"local_first": false` brings back the old path (5e0f30c). -- Fixes from the simulation and account runs since then: B127 and B141 to B173. +- Fixes from the simulation and account runs since then: B127 and B141 to B174. - **Sync issues, first cut** (§6.2): refusals sorted into issues, shown at once in the app, tray and CLI; Export button and `pdfs sync export`; `pdfs sync issues`. @@ -117,7 +117,8 @@ Tracks `docs/MILESTONE-3.0.0.md`. Update it with every commit that moves a phase fixed). Run three at a time on 2502af6, seed 8 failed two of nine runs: a file renamed while its upload was read back showed its old name again (B172, fixed). The loop on d850265 failed seed 12 once in nine runs: a file deleted just as its create - landed stayed on Drive (B173, fixed). + landed stayed on Drive (B173, fixed). The suite on 378667f failed seed 6: a rename + in the same gap was refused with `EACCES` (B174, fixed). 5. **Release**: 2.10.0 is tagged; the 2.9.0 tag still waits for the user's go-ahead. ## After 3.0.0