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)