From bfe43684d12a8d7390893d9622a2a0cb20858a8d Mon Sep 17 00:00:00 2001 From: RGB-loop <91311486+RGB-loop@users.noreply.github.com> Date: Tue, 20 Oct 2026 23:12:01 +0000 Subject: [PATCH] fix(sdk): refuse a write through a file or symlink as NotDirectory A write call naming a path below a regular file or a symlink (/a.txt/x, /dlink/x) walked into it: the content layer answered WrongLogicalRole, the write route mapped it to Unknown, and an Unknown keeps its Store call Workspace. fs.write/rm/edit/cp/mv each then held one, and after as many such calls as the Store has call Workspaces every write was refused Capacity. Mutation::resolve now refuses a name below a non-directory as NotDirectory at that entry, as fs.mkdir on the same route already does, so the call is a known refusal and its Workspace is closed. The SDK guide lists NotDirectory among the shared write refusals. Test added: layerfs-api/tests/write_refusals.rs a_write_through_a_non_directory_is_not_directory. No tests deleted. Production LOC: 109,056 -> 109,062 (delta +6) Scope and method: tools/production_loc.py --root on the parent and the staged tree; layerfs-store-service 10,009 -> 10,015; reference absent (retired R9c; last counted 65,417 at tree 498dd1917812). Test and docs are not counted. Co-Authored-By: Claude Opus 5.5 --- .../references/methods/fs-write.md | 1 + .../layerfs-api/tests/write_refusals.rs | 74 +++++++++++++++++++ .../src/write/mutation.rs | 9 +++ 3 files changed, 84 insertions(+) diff --git a/.agents/skills/layerfs-js-sdk/references/methods/fs-write.md b/.agents/skills/layerfs-js-sdk/references/methods/fs-write.md index 8171f985b..74add024c 100644 --- a/.agents/skills/layerfs-js-sdk/references/methods/fs-write.md +++ b/.agents/skills/layerfs-js-sdk/references/methods/fs-write.md @@ -30,6 +30,7 @@ Examples use `layerfs` as connected in [SKILL.md](../../SKILL.md#connect-to-a-pr | `NoSuchBranch` | No such Branch | `branch.fork` it, or check the name | | `HeadMoved` | Another writer published on the Branch during the call; nothing was published | Read the files you changed again, then decide whether to apply your change | | `NoSuchPath` | A path or its parent folder is missing | `fs.list` the parent; `fs.mkdir` first | +| `NotDirectory` | A file or a symlink is in the way of the path; `message` is that entry | `fs.stat` it; choose another path or remove it first | | `TooLarge` | The complete encoded request did not fit one control call; nothing was sent | Inspect the input, then choose a smaller call or a larger atomic Workspace step | | `InvalidInput` | A bad argument (message bytes, relative path, …) | Read `message`; fix the call | diff --git a/core/crates/layerfs-api/tests/write_refusals.rs b/core/crates/layerfs-api/tests/write_refusals.rs index 5b64e0455..ba96142df 100644 --- a/core/crates/layerfs-api/tests/write_refusals.rs +++ b/core/crates/layerfs-api/tests/write_refusals.rs @@ -73,3 +73,77 @@ fn a_write_refusal_names_its_cause() { assert!(support::items(&commits).is_empty()); fixture.cleanup(); } + +/// A name below a file or a symlink (which is not followed) is refused +/// `NotDirectory` at that entry by every write call, as `fs.mkdir` answers. +/// Such a call keeps no Store call Workspace: more of them than the Store +/// has call Workspaces leave a valid write working. `fs.mkdir` never took +/// one, so only the other calls count toward that number. +#[test] +fn a_write_through_a_non_directory_is_not_directory() { + let fixture = Fixture::new("write-through-non-directory", |source| { + fs::create_dir(source.join("dir")).unwrap(); + fs::write(source.join("dir/inner.txt"), b"inner\n").unwrap(); + fs::write(source.join("a.txt"), b"a\n").unwrap(); + std::os::unix::fs::symlink("dir", source.join("dlink")).unwrap(); + 4 + }); + fixture.fork("feat"); + let t = SdkValue::text; + let cases = [ + ( + "fs.write", + vec![("path", t("/a.txt/x")), ("content", t("x"))], + "/a.txt", + ), + ( + "fs.write", + vec![("path", t("/dlink/x")), ("content", t("x"))], + "/dlink", + ), + ("fs.rm", vec![("path", t("/a.txt/x"))], "/a.txt"), + ("fs.mkdir", vec![("path", t("/a.txt/x"))], "/a.txt"), + ( + "fs.edit", + vec![ + ("path", t("/dlink/inner.txt")), + ("oldText", t("inner")), + ("newText", t("x")), + ], + "/dlink", + ), + ( + "fs.cp", + vec![("from", t("/a.txt")), ("to", t("/dlink/c.txt"))], + "/dlink", + ), + ( + "fs.mv", + vec![("from", t("/dir/inner.txt")), ("to", t("/a.txt/y"))], + "/a.txt", + ), + ]; + let calls = layerfs_store_service::OwnerConfig::default().namespaces + 1; + let held_a_workspace = cases.iter().filter(|(method, ..)| *method != "fs.mkdir"); + for (method, fields, at) in cases.iter().chain(held_a_workspace.cycle().take(calls)) { + let mut fields = fields.clone(); + fields.push(("branch", t("feat"))); + let r = fixture + .try_call(method, fields) + .expect_err("the call is refused"); + assert_eq!( + (r.code.as_str(), r.message.as_str()), + ("NotDirectory", *at), + "{method}" + ); + } + fixture.call( + "fs.write", + vec![ + ("branch", t("feat")), + ("path", t("/after.txt")), + ("content", t("after")), + ], + ); + fixture.cleanup(); +} diff --git a/core/crates/layerfs-store-service/src/write/mutation.rs b/core/crates/layerfs-store-service/src/write/mutation.rs index 1b82e714d..bb7e76b44 100644 --- a/core/crates/layerfs-store-service/src/write/mutation.rs +++ b/core/crates/layerfs-store-service/src/write/mutation.rs @@ -102,13 +102,22 @@ impl<'a> Mutation<'a> { } } } + /// The entry at `path`. A name below an entry that is not a directory (a + /// file or a symlink, which is not followed) is `NotDirectory` at that + /// entry, as `mkdir` answers, not an engine failure. pub fn resolve(&self, path: &str) -> Result { let mut stat = self .view .stat(self.op.overlay(), self.view.root_serial()) .map_err(failure)?; + let mut at = String::new(); for name in path.split('/').filter(|s| !s.is_empty()) { let name = checked(path, name)?; + if stat.kind != layerfs_content::object::inode_leaf::InodeKind::Directory { + return Err(args::refuse("NotDirectory", at, SdkValue::Null)); + } + at.push('/'); + at.push_str(name.as_str()); stat = self .view .lookup(self.op.overlay(), stat.serial, &name)