diff --git a/api/agent/src/client.js b/api/agent/src/client.js index 1f2b8a61d..7e1ae74a9 100644 --- a/api/agent/src/client.js +++ b/api/agent/src/client.js @@ -146,3 +146,15 @@ export function checkedMessage(message) { } return message; } + +/** A write's last argument as call fields: a Commit message string, or + * `{ message?, mode? }` where `mode` is the permission bits (e.g. `0o755`). + * The engine checks `mode`; other option names are ignored. */ +export function writeOptions(options) { + if (options === undefined || typeof options === "string") return { message: checkedMessage(options) }; + if (options === null || typeof options !== "object" || Array.isArray(options)) { + throw new LayerFSError("InvalidInput", "options must be a message string or { message, mode }", null); + } + const { message, mode } = options; + return { message: checkedMessage(message), ...(mode === undefined ? {} : { mode }) }; +} diff --git a/api/agent/src/fs/mkdir.js b/api/agent/src/fs/mkdir.js index 4324fef11..03a21c7f7 100644 --- a/api/agent/src/fs/mkdir.js +++ b/api/agent/src/fs/mkdir.js @@ -1,5 +1,6 @@ -// fs.mkdir: one positional SDK operation. -import { checkedMessage } from "../client.js"; -export function mkdir(client, branch, path, message) { - return client.call("fs.mkdir", { branch, path, recursive: true, message: checkedMessage(message) }); +// fs.mkdir: one positional SDK operation. The last argument is a message or +// `{ message, mode }`; `mode` applies to `path` when it is created. +import { writeOptions } from "../client.js"; +export function mkdir(client, branch, path, options) { + return client.call("fs.mkdir", { branch, path, recursive: true, ...writeOptions(options) }); } diff --git a/api/agent/src/fs/write.js b/api/agent/src/fs/write.js index 42c9d5427..40313f56b 100644 --- a/api/agent/src/fs/write.js +++ b/api/agent/src/fs/write.js @@ -1,7 +1,8 @@ // fs.write: one positional SDK operation. Text is `content`; anything else -// is `bytes` (see body.js). -import { checkedMessage } from "../client.js"; +// is `bytes` (see body.js). The last argument is a message or +// `{ message, mode }`. +import { writeOptions } from "../client.js"; import { body } from "./body.js"; -export function write(client, branch, path, content, message) { - return client.call("fs.write", { branch, path, ...(typeof content === "string" ? { content } : { bytes: body(content) }), message: checkedMessage(message) }); +export function write(client, branch, path, content, options) { + return client.call("fs.write", { branch, path, ...(typeof content === "string" ? { content } : { bytes: body(content) }), ...writeOptions(options) }); } diff --git a/api/agent/test/fixture/cases.mjs b/api/agent/test/fixture/cases.mjs index 1ecd5d924..b8cd4baa5 100644 --- a/api/agent/test/fixture/cases.mjs +++ b/api/agent/test/fixture/cases.mjs @@ -89,6 +89,10 @@ export const CASES = [ op: "fs.apply", sent: { branch: "cart", changes: [{ op: "write", path: "/a", content: "hi" }], message: "two" }, result: committed }, { name: "fs mkdir", argv: ["fs", "mkdir", "cart", "/d"], sdk: (lf) => lf.fs.mkdir("cart", "/d"), op: "fs.mkdir", sent: { branch: "cart", path: "/d", recursive: true }, result: committed }, + { name: "fs write with a mode", argv: ["fs", "write", "cart", "/run.sh", "--mode", "755"], input: Buffer.from("#!/bin/sh\n"), sdk: (lf) => lf.fs.write("cart", "/run.sh", Buffer.from("#!/bin/sh\n"), { mode: 0o755 }), + op: "fs.write", sent: { branch: "cart", path: "/run.sh", bytes: { base64: "IyEvYmluL3NoCg==" }, mode: 0o755 }, result: committed }, + { name: "fs mkdir with a mode and message", argv: ["fs", "mkdir", "cart", "/d", "--mode", "0o750", "-m", "private"], sdk: (lf) => lf.fs.mkdir("cart", "/d", { message: "private", mode: 0o750 }), + op: "fs.mkdir", sent: { branch: "cart", path: "/d", recursive: true, mode: 0o750, message: "private" }, result: committed }, { name: "fs rm", argv: ["fs", "rm", "cart", "/d"], sdk: (lf) => lf.fs.rm("cart", "/d"), op: "fs.rm", sent: { branch: "cart", path: "/d" }, result: { kind: "UpToDate", head: "h3ad" } }, { name: "fs cp", argv: ["fs", "cp", "cart", "/a", "/b"], sdk: (lf) => lf.fs.cp("cart", "/a", "/b"), diff --git a/api/agent/test/verify/fs_mode_real.mjs b/api/agent/test/verify/fs_mode_real.mjs new file mode 100644 index 000000000..cfbc8a909 --- /dev/null +++ b/api/agent/test/verify/fs_mode_real.mjs @@ -0,0 +1,99 @@ +// #442: permission modes through the JS SDK and the real native binding. +// fs.write and fs.mkdir take { message, mode } (a message string still +// works), fs.apply takes `mode` on write/mkdir and a chmod change, and +// fs.stat reports `mode`. A mounted Workspace of the Branch shows the same +// modes through FUSE (`stat -c %a`, and the new script runs), and a chmod +// made in the Workspace and committed reads back through fs.stat. Refusals: +// set-user-ID is InvalidInput with reason Mode, a text mode is InvalidInput, +// and options that are neither a message nor an object are refused by the +// SDK. Nothing refused is committed. One attempt per step; nothing retried. +// Linux, under the E2E launcher (its Store volume, SDK home and Sandbox image): +// python3 tools/worktree.py run --resources --bundle \ +// --timeout 90 -- node api/agent/test/verify/fs_mode_real.mjs +import assert from "node:assert/strict"; +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const { LayerFS } = await import("@layerfs/agent"); +const source = mkdtempSync(join(tmpdir(), "layerfs-442-")); +writeFileSync(join(source, "tool.sh"), "#!/bin/sh\necho tool\n"); +chmodSync(join(source, "tool.sh"), 0o755); +mkdirSync(join(source, "private")); +writeFileSync(join(source, "private", "f"), "f\n"); +chmodSync(join(source, "private"), 0o750); +writeFileSync(join(source, "a.txt"), "a\n"); +chmodSync(join(source, "a.txt"), 0o644); +const layerfs = await LayerFS.init("site", source); +rmSync(source, { recursive: true, force: true }); +await layerfs.branch.fork("work", "main"); + +const mode = async (ref, path) => (await layerfs.fs.stat(ref, path)).mode; +const commits = async () => (await layerfs.branch.list_commits("work")).items.length; + +// Init keeps the source's modes; fs.stat reports them. +assert.equal(await mode("main", "/tool.sh"), 0o755); +assert.equal(await mode("main", "/private"), 0o750); +assert.equal(await mode("main", "/a.txt"), 0o644); + +// fs.write: an options object, a plain message, and no options. +assert.equal((await layerfs.fs.write("work", "/new.sh", "#!/bin/sh\necho new\n", { mode: 0o755, message: "add new.sh" })).kind, "Committed"); +assert.equal((await layerfs.fs.write("work", "/plain.txt", "p\n", "plain message")).kind, "Committed"); +await layerfs.fs.write("work", "/tool.sh", "#!/bin/sh\necho changed\n"); +const messages = (await layerfs.branch.list_commits("work")).items.map((c) => c.message); +assert.ok(messages.includes("add new.sh") && messages.includes("plain message"), JSON.stringify(messages)); + +// fs.mkdir with a mode; fs.apply with modes on write and mkdir, and chmod. +await layerfs.fs.mkdir("work", "/d/e", { mode: 0o750 }); +const applied = await layerfs.fs.apply("work", [ + { op: "write", path: "/applied.sh", bytes: Buffer.from("#!/bin/sh\necho applied\n"), mode: 0o700 }, + { op: "mkdir", path: "/shared", mode: 0o1777 }, + { op: "chmod", path: "/a.txt", mode: 0o600 }, +], "modes"); +assert.equal(applied.kind, "Committed"); + +const want = { + "/tool.sh": 0o755, // overwritten without a mode: kept + "/new.sh": 0o755, + "/plain.txt": 0o644, + "/applied.sh": 0o700, + "/a.txt": 0o600, + "/private": 0o750, + "/d": 0o755, + "/d/e": 0o750, + "/shared": 0o1777, +}; +for (const [path, m] of Object.entries(want)) assert.equal(await mode("work", path), m, path); +assert.equal(await mode("main", "/a.txt"), 0o644, "main is untouched"); +console.log("STAT ok"); + +// Refused, and nothing committed. +const before = await commits(); +await assert.rejects(layerfs.fs.write("work", "/x", "x", { mode: 0o4755 }), (e) => e.code === "InvalidInput" && e.detail?.reason === "Mode" && e.detail?.mode === "4755" && e.detail?.path === "/x"); +await assert.rejects(layerfs.fs.apply("work", [{ op: "chmod", path: "/a.txt", mode: 0o1644 }]), (e) => e.code === "InvalidInput" && e.detail?.reason === "Mode" && e.detail?.index === 0); +await assert.rejects(layerfs.fs.write("work", "/x", "x", { mode: "755" }), { code: "InvalidInput" }); +await assert.rejects(layerfs.fs.mkdir("work", "/x", 755), { code: "InvalidInput" }); +assert.equal(await commits(), before); +await assert.rejects(layerfs.fs.stat("work", "/x"), { code: "NoSuchPath" }); +console.log("REFUSED ok"); + +// A Workspace of the Branch shows the same modes through FUSE. +const ws = await layerfs.workspace.mount((await layerfs.sandbox.create()).sandbox, "work"); +try { + const paths = Object.keys(want).map((p) => p.slice(1)); + const seen = await layerfs.workspace.exec(ws, `stat -c '%a %n' ${paths.join(" ")} && ./new.sh && ./applied.sh`); + assert.equal(seen.exitCode, 0, seen.stderr); + const lines = paths.map((p) => `${want[`/${p}`].toString(8)} ${p}`); + assert.equal(seen.stdout, `${lines.join("\n")}\nnew\napplied\n`); + console.log("WORKSPACE ok"); + + // The other way: a chmod in the Workspace, committed, reads back. + assert.equal((await layerfs.workspace.exec(ws, "chmod 640 plain.txt")).exitCode, 0); + await layerfs.workspace.commit(ws, "chmod in Workspace"); + assert.equal(await mode("work", "/plain.txt"), 0o640); + console.log("ROUNDTRIP ok"); +} finally { + await layerfs.workspace.unmount(ws, true); + await layerfs.close(); +} +console.log("fs_mode_real: ok"); diff --git a/api/cli/src/argv.js b/api/cli/src/argv.js index a581fbdee..246721176 100644 --- a/api/cli/src/argv.js +++ b/api/cli/src/argv.js @@ -44,6 +44,10 @@ function value(kind, name, text, entry) { if (!/^\d+$/.test(text)) throw new UsageError(`--${name} takes a whole number, not ${JSON.stringify(text)}`, entry); return Number(text); } + if (kind === "octal") { + if (!/^(0o)?[0-7]+$/.test(text)) throw new UsageError(`--${name} takes octal permission bits such as 755, not ${JSON.stringify(text)}`, entry); + return parseInt(text.replace(/^0o/, ""), 8); + } if (kind === "json") { try { return JSON.parse(text); diff --git a/api/cli/src/common.js b/api/cli/src/common.js index 027368049..3343d5edc 100644 --- a/api/cli/src/common.js +++ b/api/cli/src/common.js @@ -5,6 +5,8 @@ import { UsageError } from "./argv.js"; export const PAGE = { "page-size": "int", "page-pos": "int" }; export const MESSAGE = { message: "text" }; +// A write's message and permission bits (`--mode 755`). +export const WRITE = { ...MESSAGE, mode: "octal" }; // --no-hunks sends hunks: false (paths without line changes); left out, hunks is not // sent and the SDK lists line changes, its default. export const DIFF = { ...PAGE, cursor: "text", "no-hunks": "bool" }; diff --git a/api/cli/src/fs/mkdir.js b/api/cli/src/fs/mkdir.js index 380c037f4..681df8fa3 100644 --- a/api/cli/src/fs/mkdir.js +++ b/api/cli/src/fs/mkdir.js @@ -1,9 +1,9 @@ -// layerfs fs mkdir: fs.mkdir(branch, path, message?). -import { MESSAGE } from "../common.js"; +// layerfs fs mkdir: fs.mkdir(branch, path, { message?, mode? }). +import { WRITE } from "../common.js"; export default { args: ["branch", "path"], - options: MESSAGE, - summary: "make a folder; one Commit", - run: (lf, a, o) => lf.fs.mkdir(a.branch, a.path, o.message), + options: WRITE, + summary: "make a folder (--mode for its permission bits); one Commit", + run: (lf, a, o) => lf.fs.mkdir(a.branch, a.path, { message: o.message, mode: o.mode }), }; diff --git a/api/cli/src/fs/write.js b/api/cli/src/fs/write.js index 4460d8b73..a59905dd5 100644 --- a/api/cli/src/fs/write.js +++ b/api/cli/src/fs/write.js @@ -1,10 +1,10 @@ -// layerfs fs write: fs.write(branch, path, content, message?), content from stdin or --from. -import { MESSAGE, body } from "../common.js"; +// layerfs fs write: fs.write(branch, path, content, { message?, mode? }), content from stdin or --from. +import { WRITE, body } from "../common.js"; export default { args: ["branch", "path"], - options: { ...MESSAGE, from: "file" }, - summary: "write a file from stdin (or --from), bytes kept exactly; one Commit", + options: { ...WRITE, from: "file" }, + summary: "write a file from stdin (or --from), bytes kept exactly; --mode sets its permission bits; one Commit", prepare: async (a, o, io) => ({ content: await body(o, io) }), - run: (lf, a, o) => lf.fs.write(a.branch, a.path, a.content, o.message), + run: (lf, a, o) => lf.fs.write(a.branch, a.path, a.content, { message: o.message, mode: o.mode }), }; diff --git a/api/cli/src/help.js b/api/cli/src/help.js index b9b13ca4a..8939df747 100644 --- a/api/cli/src/help.js +++ b/api/cli/src/help.js @@ -2,7 +2,7 @@ // with the fixed sections (settings, shared arguments, output) around them. import { CALLS, FAMILIES, INIT } from "./calls.js"; -const KIND = { text: "", int: "", json: "", file: "" }; +const KIND = { text: "", int: "", json: "", file: "", octal: "" }; /** One command's usage line. */ export function usage(entry) { diff --git a/api/cli/test/cli.test.mjs b/api/cli/test/cli.test.mjs index 3b97c5f12..c91eb9f44 100644 --- a/api/cli/test/cli.test.mjs +++ b/api/cli/test/cli.test.mjs @@ -137,6 +137,8 @@ for (const [why, argv, message] of [ ["both --old and --old-from", ["fs", "edit", "cart", "/a", "--old", "x", "--old-from", "/x", "--new", "y"], "give exactly one of --old and --old-from"], ["neither --new nor --new-from", ["fs", "edit", "cart", "/a", "--old", "x"], "give exactly one of --new and --new-from"], ["unreadable --from", ["fs", "write", "cart", "/a", "--from", "/no/such/file"], "cannot read /no/such/file"], + ["--mode that is not octal", ["fs", "mkdir", "cart", "/d", "--mode", "rwx"], '--mode takes octal permission bits such as 755, not "rwx"'], + ["--mode with a digit past 7", ["fs", "write", "cart", "/a", "--mode", "0789"], '--mode takes octal permission bits such as 755, not "0789"'], ]) { test(`usage error, nothing called: ${why}`, () => { const r = cli(argv); diff --git a/api/docs/agent.md b/api/docs/agent.md index 53f055d86..9ca3ade74 100644 --- a/api/docs/agent.md +++ b/api/docs/agent.md @@ -259,7 +259,7 @@ trace line covers the command's run. | `LayerFS` (2) | `LayerFS.open(name)`, `LayerFS.init(name, folder, timeout?)`: static, each returns a ready client | | project (2) | `list_branches(pageSize?, pagePos?)`, `list_sandboxes(pageSize?, pagePos?)` | | fs reads (7) | `read(ref, path, startLine?, endLine?)`, `list(ref, path?, pageSize?, pagePos?)`, `stat(ref, path)`, `grep(ref, pattern, path?, pageSize?, pagePos?)`, `glob(ref, pattern, path?, pageSize?, pagePos?)`, `diff_files(leftRef, leftPath, rightRef, rightPath)`, `blame(ref, path, fromLine?)` | -| fs writes (7) | `write(branch, path, content, message?)`, `edit(branch, path, oldText, newText, message?)`, `apply(branch, changes, message?)`, `mkdir(branch, path, message?)`, `rm(branch, path, message?)`, `cp(branch, from, to, message?)`, `mv(branch, from, to, message?)` | +| fs writes (7) | `write(branch, path, content, message? \| { message?, mode? })`, `edit(branch, path, oldText, newText, message?)`, `apply(branch, changes, message?)`, `mkdir(branch, path, message? \| { message?, mode? })`, `rm(branch, path, message?)`, `cp(branch, from, to, message?)`, `mv(branch, from, to, message?)` | | branch (6) | `fork(name, fromRef?)`, `list_commits(branch, pageSize?, pagePos?)`, `diff_commits(from, to, pageSize?, pagePos?, { cursor?, hunks? }?)`, `diff_branches(from, to, pageSize?, pagePos?, { cursor?, hunks? }?)`, `merge(a, b, name, message?, units?)`, `publish(branch, commit?)` | | sandbox (3) | `create({ image }?)` → `{ sandbox, image }`, `list_workspaces(sandbox, pageSize?, pagePos?)`, `close(sandbox)` | | workspace (4) | `mount(sandbox, branch)` → `{ sandbox, path, image }`, `exec(ws, command, message?, { stdout?, stderr? }?)`, `commit(ws, message?, force?)`, `unmount(ws, discard?)` | @@ -267,8 +267,8 @@ trace line covers the command's run. A Workspace's status is not an agent call: it is `backend.daemon.workspace.status(ws)` in [`@layerfs/backend`](backend.md). -Every grouped method returns a Promise and takes positional arguments only; no -method takes an options object. A ref is `main`, a Branch name, a Commit +Every grouped method returns a Promise and takes positional arguments; only +the trailing `{ ... }?` arguments shown above are options objects. A ref is `main`, a Branch name, a Commit id or a Layer id. Filesystem paths are absolute in that view. Main changes only through publish. There is no `remove` alias, harness export, agent/call attribution, public inspect or public process-control method. @@ -284,8 +284,8 @@ no history. Unchanged state creates no Commit solely for a message. ### Writes -`apply` takes ordered `{op, ...}` changes (`write`, `edit`, `mkdir`, `rm`, `cp`, -`mv`) and publishes once. A refusal identifies its zero-based `detail.index`. +`apply` takes ordered `{op, ...}` changes (`write`, `edit`, `mkdir`, `chmod`, +`rm`, `cp`, `mv`) and publishes once. A refusal identifies its zero-based `detail.index`. Each call uses a private indexed Workspace, the ordinary Workspace semantic ports and the shared captured Content constructor. Failed validation closes only that private overlay. Uncertain construction/publication keeps original custody. @@ -296,6 +296,18 @@ identity/timestamps. Destinations are exact and must not exist. Same-path/subtre moves or copies are InvalidInput. Mkdir creates parents and an existing directory is a no-op. Edit requires exactly one match and streams large files. +A file's or folder's permission bits are an integer `mode` such as `0o755`. A +`write` (in `fs.write`'s options or an `fs.apply` change) or `mkdir` may carry +one: a new file gets `mode`, else 0o644; an existing file keeps its mode unless +`mode` is given; a new folder gets `mode`, else 0o755, while parents it creates +get 0o755 and an existing folder keeps its mode, as `mkdir -p -m` does. +`{ op: "chmod", path, mode }` sets the mode of an existing file or folder. +`fs.stat` reports `mode`. A mode that is not an integer from 0 to 0o7777 is +`InvalidInput`; one LayerFS cannot store (set-user-ID or set-group-ID on any +entry, the sticky bit on a file) is `InvalidInput` with `detail` +`{ reason: "Mode", path, mode }` (octal text), as in `project.init`. A symbolic +link has no mode of its own: `chmod` of one is `NotFile`. + Branch writes, like `workspace.commit`, read the Branch head, merge with it and publish inside the Store's short publication lane, so concurrent writers never meet a moved head. A call whose private Workspace was bound before another @@ -482,10 +494,10 @@ table differs, and when a family's files stop matching the SDK's. | `fs.glob` | `layerfs fs glob [] [--page-size ] [--page-pos ]` | | `fs.diff_files` | `layerfs fs diff_files ` | | `fs.blame` | `layerfs fs blame [--from-line ]` | -| `fs.write` | `layerfs fs write [-m ] [--from ]` | +| `fs.write` | `layerfs fs write [-m ] [--mode ] [--from ]` | | `fs.edit` | `layerfs fs edit [-m ] [--old ] [--old-from ] [--new ] [--new-from ]` | | `fs.apply` | `layerfs fs apply [-m ] [--from ]` | -| `fs.mkdir` | `layerfs fs mkdir [-m ]` | +| `fs.mkdir` | `layerfs fs mkdir [-m ] [--mode ]` | | `fs.rm` | `layerfs fs rm [-m ]` | | `fs.cp` | `layerfs fs cp [-m ]` | | `fs.mv` | `layerfs fs mv [-m ]` | diff --git a/engine/crates/layerfs-api/src/base/args.rs b/engine/crates/layerfs-api/src/base/args.rs index ff4e32093..365224e7b 100644 --- a/engine/crates/layerfs-api/src/base/args.rs +++ b/engine/crates/layerfs-api/src/base/args.rs @@ -82,6 +82,18 @@ pub fn selector<'a>(args: &'a SdkValue, name: &str, branch: &'a str) -> Arg Result, SdkRefusal> { + match change.get("mode") { + None | Some(SdkValue::Null) => Ok(None), + Some(SdkValue::Int(n)) if (0..=0o7777).contains(n) => Ok(Some(*n as u32)), + Some(_) => Err(invalid( + "mode must be an integer permission mode from 0 to 0o7777", + )), + } +} + /// One change of a file call, from its `op` and that op's fields. A write /// whose bytes were to follow the call (`streamed`) is refused: no bytes /// follow a call on the in-process route. @@ -97,10 +109,12 @@ pub fn change(change: &SdkValue) -> Arg> { (Some(SdkValue::Text(s)), None, None) => Change::Write { path: p("path")?, bytes: s.as_bytes(), + mode: mode(change)?, }, (None, Some(SdkValue::Bytes(b)), None) => Change::Write { path: p("path")?, bytes: b, + mode: mode(change)?, }, (None, None, Some(SdkValue::Int(n))) if *n > 0 => { p("path")?; @@ -113,7 +127,14 @@ pub fn change(change: &SdkValue) -> Arg> { old: text(change, "oldText")?, new: text(change, "newText")?, }, - "mkdir" => Change::Mkdir { path: p("path")? }, + "mkdir" => Change::Mkdir { + path: p("path")?, + mode: mode(change)?, + }, + "chmod" => Change::Chmod { + path: p("path")?, + mode: mode(change)?.ok_or_else(|| invalid("chmod needs a mode"))?, + }, "rm" => Change::Rm { path: p("path")? }, "cp" => Change::Cp { from: p("from")?, diff --git a/engine/crates/layerfs-api/src/fs/mkdir.rs b/engine/crates/layerfs-api/src/fs/mkdir.rs index c5ecf925e..3ef7936ca 100644 --- a/engine/crates/layerfs-api/src/fs/mkdir.rs +++ b/engine/crates/layerfs-api/src/fs/mkdir.rs @@ -3,7 +3,8 @@ use crate::base::args::{commit, single, Answer}; use layerfs_bridge::control::SdkValue; use layerfs_store_service::StoreService; -/// `mkdir(path, { branch })`: one Commit creating a directory and its missing parents. +/// `mkdir(path, { branch, mode? })`: one Commit creating a directory and its +/// missing parents; `mode` applies to `path` when it is created. pub fn mkdir(service: &StoreService, args: &SdkValue) -> Answer { commit(service, args, &[single(args, "mkdir")]) } diff --git a/engine/crates/layerfs-api/src/fs/stat.rs b/engine/crates/layerfs-api/src/fs/stat.rs index 65c06681e..4b7fba448 100644 --- a/engine/crates/layerfs-api/src/fs/stat.rs +++ b/engine/crates/layerfs-api/src/fs/stat.rs @@ -3,7 +3,7 @@ use crate::base::args::{path, snapshot, Answer}; use layerfs_bridge::control::SdkValue; use layerfs_store_service::{read::reads, StoreService}; -/// `stat(path, { branch, commit? })`: type, inode and size of one path. +/// `stat(path, { branch, commit? })`: type, inode, size and mode of one path. pub fn stat(service: &StoreService, args: &SdkValue) -> Answer { let (reader, snap) = snapshot(service, args)?; reads::stat(&reader, &snap, &path(args, "path")?) diff --git a/engine/crates/layerfs-api/src/fs/write.rs b/engine/crates/layerfs-api/src/fs/write.rs index 64c725289..41afc519f 100644 --- a/engine/crates/layerfs-api/src/fs/write.rs +++ b/engine/crates/layerfs-api/src/fs/write.rs @@ -3,7 +3,9 @@ use crate::base::args::{commit, single, Answer}; use layerfs_bridge::control::SdkValue; use layerfs_store_service::StoreService; -/// `write(path, content, { branch })`: one Commit writing a file. +/// `write(path, content, { branch, mode? })`: one Commit writing a file; `mode` +/// sets its permission bits, else a new file is 0o644 and an existing one keeps +/// its mode. pub fn write(service: &StoreService, args: &SdkValue) -> Answer { commit(service, args, &[single(args, "write")]) } diff --git a/engine/crates/layerfs-api/tests/fs_mode.rs b/engine/crates/layerfs-api/tests/fs_mode.rs new file mode 100644 index 000000000..60fa49567 --- /dev/null +++ b/engine/crates/layerfs-api/tests/fs_mode.rs @@ -0,0 +1,237 @@ +//! Permission modes on the Branch file calls (#442). `fs.write` and the +//! `write` and `mkdir` changes of `fs.apply` take an optional `mode`; a +//! `chmod` change sets one; `fs.stat` reports it. Without `mode` a new file +//! is 0o644, a new folder 0o755 and an existing entry keeps its mode. A mode +//! LayerFS cannot store is refused as `project.init` refuses it, and a +//! refused change commits nothing. +#[path = "support/installed_store.rs"] +#[allow(dead_code)] +mod installed; +#[path = "support/sdk.rs"] +#[allow(dead_code)] +mod support; + +use layerfs_bridge::control::{SdkRefusal, SdkValue}; +use std::fs; +use std::os::unix::fs::PermissionsExt; +use support::Fixture; + +fn t(text: &str) -> SdkValue { + SdkValue::text(text) +} + +fn change(fields: Vec<(&str, SdkValue)>) -> SdkValue { + SdkValue::Map(fields.into_iter().map(|(k, v)| (k.into(), v)).collect()) +} + +fn mode(f: &Fixture, branch: &str, path: &str) -> i64 { + match f + .call("fs.stat", vec![("branch", t(branch)), ("path", t(path))]) + .get("mode") + { + Some(SdkValue::Int(n)) => *n, + other => panic!("fs.stat {path} mode: {other:?}"), + } +} + +fn commits(f: &Fixture) -> usize { + support::items(&f.call("branch.list_commits", vec![("branch", t("feat"))])).len() +} + +fn refused(f: &Fixture, changes: Vec) -> SdkRefusal { + f.try_call( + "fs.apply", + vec![("branch", t("feat")), ("changes", SdkValue::List(changes))], + ) + .expect_err("the change is refused") +} + +fn source(root: &std::path::Path) -> usize { + let set = |path: &str, mode| { + fs::set_permissions(root.join(path), fs::Permissions::from_mode(mode)).unwrap() + }; + fs::write(root.join("tool.sh"), b"#!/bin/sh\n").unwrap(); + set("tool.sh", 0o755); + fs::create_dir(root.join("private")).unwrap(); + fs::write(root.join("private/f"), b"f\n").unwrap(); + set("private", 0o750); + fs::write(root.join("a.txt"), b"a\n").unwrap(); + set("a.txt", 0o644); + std::os::unix::fs::symlink("a.txt", root.join("link")).unwrap(); + 5 +} + +#[test] +fn write_mkdir_and_chmod_set_modes_that_stat_reports() { + let f = Fixture::new("fs-mode", source); + f.fork("feat"); + // Init keeps the source's modes. + assert_eq!(mode(&f, "main", "/tool.sh"), 0o755); + assert_eq!(mode(&f, "main", "/private"), 0o750); + assert_eq!(mode(&f, "main", "/a.txt"), 0o644); + + // A new file gets `mode`, else 0o644. + f.call( + "fs.write", + vec![ + ("branch", t("feat")), + ("path", t("/new.sh")), + ("content", t("#!/bin/sh\n")), + ("mode", SdkValue::Int(0o755)), + ], + ); + f.call( + "fs.write", + vec![ + ("branch", t("feat")), + ("path", t("/plain.txt")), + ("content", t("p\n")), + ], + ); + assert_eq!(mode(&f, "feat", "/new.sh"), 0o755); + assert_eq!(mode(&f, "feat", "/plain.txt"), 0o644); + + // In one apply: a binary write with a mode, a folder whose new parent + // gets 0o755, and a sticky folder. + f.apply( + "feat", + vec![ + change(vec![ + ("op", t("write")), + ("path", t("/applied.sh")), + ("bytes", SdkValue::Bytes(vec![0, 1, 2])), + ("mode", SdkValue::Int(0o700)), + ]), + change(vec![ + ("op", t("mkdir")), + ("path", t("/d/e")), + ("mode", SdkValue::Int(0o750)), + ]), + change(vec![ + ("op", t("mkdir")), + ("path", t("/shared")), + ("mode", SdkValue::Int(0o1777)), + ]), + ], + ); + assert_eq!(mode(&f, "feat", "/applied.sh"), 0o700); + assert_eq!(mode(&f, "feat", "/d"), 0o755); + assert_eq!(mode(&f, "feat", "/d/e"), 0o750); + assert_eq!(mode(&f, "feat", "/shared"), 0o1777); + + // An existing file keeps its mode unless one is given. + let write = |content: &str, mode: Option| { + let mut fields = vec![ + ("branch", t("feat")), + ("path", t("/tool.sh")), + ("content", t(content)), + ]; + fields.extend(mode.map(|m| ("mode", SdkValue::Int(m)))); + f.call("fs.write", fields) + }; + write("#!/bin/sh\necho\n", None); + assert_eq!(mode(&f, "feat", "/tool.sh"), 0o755); + // Same bytes, new mode: a mode-only Commit. + assert_eq!( + write("#!/bin/sh\necho\n", Some(0o700)).get_text("kind"), + Some("Committed") + ); + assert_eq!(mode(&f, "feat", "/tool.sh"), 0o700); + + // An existing folder keeps its mode, as `mkdir -p -m` does. + let kept = f.call( + "fs.mkdir", + vec![ + ("branch", t("feat")), + ("path", t("/private")), + ("mode", SdkValue::Int(0o700)), + ], + ); + assert_eq!(kept.get_text("kind"), Some("UpToDate")); + assert_eq!(mode(&f, "feat", "/private"), 0o750); + + // chmod of a file and a folder; the same mode again changes nothing. + let chmod = |path: &str, m: i64| { + f.apply( + "feat", + vec![change(vec![ + ("op", t("chmod")), + ("path", t(path)), + ("mode", SdkValue::Int(m)), + ])], + ) + }; + assert_eq!(chmod("/a.txt", 0o600).get_text("kind"), Some("Committed")); + assert_eq!(chmod("/private", 0o700).get_text("kind"), Some("Committed")); + assert_eq!(chmod("/a.txt", 0o600).get_text("kind"), Some("UpToDate")); + assert_eq!(mode(&f, "feat", "/a.txt"), 0o600); + assert_eq!(mode(&f, "feat", "/private"), 0o700); + // main is untouched. + assert_eq!(mode(&f, "main", "/a.txt"), 0o644); + + // A Workspace chmod and Commit reads back the same way. + f.chmod("feat", "/plain.txt", 0o640); + assert_eq!(mode(&f, "feat", "/plain.txt"), 0o640); + f.cleanup(); +} + +#[test] +fn a_mode_the_store_cannot_hold_is_refused_and_commits_nothing() { + let f = Fixture::new("fs-mode-refusals", source); + f.fork("feat"); + let before = commits(&f); + let write = |m: SdkValue| { + change(vec![ + ("op", t("write")), + ("path", t("/x")), + ("content", t("x")), + ("mode", m), + ]) + }; + + // Not a permission mode at all. + for bad in [t("755"), SdkValue::Int(-1), SdkValue::Int(0o10000)] { + let r = refused(&f, vec![write(bad)]); + assert_eq!(r.code, "InvalidInput"); + assert!(r.message.starts_with("mode must be an integer"), "{r:?}"); + } + + // Bits LayerFS cannot store: reason `Mode`, as in project.init. + for (path, op, m) in [ + ("/x", "write", 0o4755), + ("/x", "write", 0o1644), + ("/d", "mkdir", 0o2755), + ("/a.txt", "chmod", 0o1644), + ] { + let mut fields = vec![("op", t(op)), ("path", t(path)), ("mode", SdkValue::Int(m))]; + if op == "write" { + fields.push(("content", t("x"))); + } + // The refused change is the second: the first one is not committed. + let r = refused( + &f, + vec![ + change(vec![("op", t("mkdir")), ("path", t("/first"))]), + change(fields), + ], + ); + assert_eq!(r.code, "InvalidInput", "{op} {m:o}"); + assert_eq!(r.detail.get_text("reason"), Some("Mode"), "{op} {m:o}"); + assert_eq!(r.detail.get_text("path"), Some(path)); + assert_eq!(r.detail.get_text("mode"), Some(format!("{m:04o}").as_str())); + assert_eq!(r.detail.get("index"), Some(&SdkValue::Int(1))); + } + + // chmod needs a mode, an existing entry, and not a symlink. + let chmod = |path: &str, m: Option| { + let mut fields = vec![("op", t("chmod")), ("path", t(path))]; + fields.extend(m.map(|m| ("mode", SdkValue::Int(m)))); + refused(&f, vec![change(fields)]) + }; + assert_eq!(chmod("/a.txt", None).code, "InvalidInput"); + assert_eq!(chmod("/missing", Some(0o644)).code, "NoSuchPath"); + assert_eq!(chmod("/link", Some(0o644)).code, "NotFile"); + + assert_eq!(commits(&f), before); + f.cleanup(); +} diff --git a/engine/crates/layerfs-api/tests/golden/sdk_methods.txt b/engine/crates/layerfs-api/tests/golden/sdk_methods.txt index bbc682456..4f7db8249 100644 --- a/engine/crates/layerfs-api/tests/golden/sdk_methods.txt +++ b/engine/crates/layerfs-api/tests/golden/sdk_methods.txt @@ -37,8 +37,8 @@ fs.read -> refused Binary "text contains NUL or invalid UTF-8" null fs.list -> ok {kind: "ok", snapshot: "feat@", root: "", path: "/", items: [{name: "a.txt", path: "/a.txt", type: "file", inode: 2}, {name: "applied-dir", path: "/applied-dir", type: "dir", inode: 201}, {name: "applied.txt", path: "/applied.txt", type: "file", inode: 200}, {name: "blob.bin", path: "/blob.bin", type: "file", inode: 3}, {name: "dir", path: "/dir", type: "dir", inode: 4}, {name: "made", path: "/made", type: "dir", inode: 72}, {name: "moved.txt", path: "/moved.txt", type: "file", inode: 137}, {name: "new.txt", path: "/new.txt", type: "file", inode: 8}], hasMore: false, nextPagePos: null} fs.list -> ok {kind: "ok", snapshot: "feat@", root: "", path: "/dir", items: [{name: "b.txt", path: "/dir/b.txt", type: "file", inode: 5}], hasMore: true, nextPagePos: 1} fs.list -> refused NotDirectory "expected a directory" null -fs.stat -> ok {kind: "ok", snapshot: "main@", root: "", path: "/a.txt", type: "file", inode: 2, size: 14} -fs.stat -> ok {kind: "ok", snapshot: "feat@", root: "", path: "/dir", type: "dir", inode: 4, size: 0} +fs.stat -> ok {kind: "ok", snapshot: "main@", root: "", path: "/a.txt", type: "file", inode: 2, size: 14, mode: 420} +fs.stat -> ok {kind: "ok", snapshot: "feat@", root: "", path: "/dir", type: "dir", inode: 4, size: 0, mode: 493} fs.grep -> ok {kind: "ok", snapshot: "feat@", root: "", path: "/", items: [], skipped: 1, hasMore: false, nextPagePos: null} fs.grep -> ok {kind: "ok", snapshot: "feat@", root: "", path: "/dir", items: [{path: "/dir/b.txt", line: 1, text: "bee\n", lineTruncated: false}], skipped: 0, hasMore: false, nextPagePos: null} fs.grep -> ok {kind: "ok", snapshot: "feat@", root: "", path: "/", items: [{path: "/dir/sub/c.rs", line: 1, text: "fn main() {}\n", lineTruncated: false}], skipped: 1, hasMore: false, nextPagePos: null} diff --git a/engine/crates/layerfs-api/tests/relink_commit.rs b/engine/crates/layerfs-api/tests/relink_commit.rs index 85b9ad0c1..977bb3997 100644 --- a/engine/crates/layerfs-api/tests/relink_commit.rs +++ b/engine/crates/layerfs-api/tests/relink_commit.rs @@ -148,7 +148,7 @@ fn relink( name: PathName::new(name).unwrap(), }) .unwrap(); - m.write(file, &bytes).unwrap(); + m.write(file, &bytes, None).unwrap(); let created = m.resolve(file).unwrap().serial; assert_ne!(created, *serial, "a new inode"); m.apply(Operation::SetAttributes { diff --git a/engine/crates/layerfs-store-service/src/read/reads.rs b/engine/crates/layerfs-store-service/src/read/reads.rs index 0544337cd..26ca56702 100644 --- a/engine/crates/layerfs-store-service/src/read/reads.rs +++ b/engine/crates/layerfs-store-service/src/read/reads.rs @@ -12,6 +12,7 @@ use layerfs_bridge::control::{SdkRefusal, SdkValue}; use layerfs_content::{ compare::{memo::Memo, EntryFacts}, filesystem::{ + attributes::{read_portable, AttributeReadWork}, directory::read::{list_after, DirectoryReadWork}, sorted::DirectoryRoot, }, @@ -75,15 +76,24 @@ pub fn read( answer(fields) } -/// The type, inode and size of the entry at `path`. +/// The type, inode, size and permission mode of the entry at `path`. An +/// inline mode costs no object read. pub fn stat(reader: &StorePorts, snap: &Snap, path: &str) -> Answer { let cached = Cached::new(reader)?; let entry = existing(&Memo::new(&cached), snap, path)?; + let metadata = read_portable( + &cached, + entry.value.metadata_root, + entry.value.kind, + &mut AttributeReadWork::default(), + ) + .map_err(args::content)?; let mut fields = header(snap, path); fields.extend([ ("type", SdkValue::text(kind_name(entry.value.kind))), ("inode", SdkValue::Int(entry.serial as i64)), ("size", SdkValue::Int(size(reader, &entry)? as i64)), + ("mode", SdkValue::Int(i64::from(metadata.mode))), ]); answer(fields) } diff --git a/engine/crates/layerfs-store-service/src/write/files.rs b/engine/crates/layerfs-store-service/src/write/files.rs index d533874b5..99acadb89 100644 --- a/engine/crates/layerfs-store-service/src/write/files.rs +++ b/engine/crates/layerfs-store-service/src/write/files.rs @@ -27,16 +27,25 @@ pub fn path(value: &str) -> Result { /// One change of a file call, with checked absolute paths. pub enum Change<'a> { - /// Replace the file's bytes, creating it. - Write { path: String, bytes: &'a [u8] }, + /// Replace the file's bytes, creating it with `mode` (default 0o644). + /// An existing file keeps its mode unless `mode` is given. + Write { + path: String, + bytes: &'a [u8], + mode: Option, + }, /// Replace the one occurrence of `old` with `new`. Edit { path: String, old: &'a str, new: &'a str, }, - /// Create the directory and its missing parents. - Mkdir { path: String }, + /// Create the directory and its missing parents. A created `path` gets + /// `mode` (default 0o755); missing parents get 0o755, and an existing + /// directory keeps its mode, as `mkdir -p -m` does. + Mkdir { path: String, mode: Option }, + /// Set the entry's permission bits. + Chmod { path: String, mode: u32 }, /// Remove the entry, recursively. Rm { path: String }, /// Copy the entry, recursively. @@ -179,9 +188,10 @@ fn change( let mut result = Ok(()); for (index, change) in changes.iter().enumerate() { result = (|| match change.as_ref().map_err(Clone::clone)? { - Change::Write { path, bytes } => mutation.write(path, bytes), + Change::Write { path, bytes, mode } => mutation.write(path, bytes, *mode), Change::Edit { path, old, new } => mutation.edit(path, old, new), - Change::Mkdir { path } => mutation.mkdir(path), + Change::Mkdir { path, mode } => mutation.mkdir(path, *mode), + Change::Chmod { path, mode } => mutation.chmod(path, *mode), Change::Rm { path } => mutation.remove(path), Change::Cp { from, to } => mutation.copy(from, to, index as u64), Change::Mv { from, to } => mutation.rename(from, to), diff --git a/engine/crates/layerfs-store-service/src/write/mutation.rs b/engine/crates/layerfs-store-service/src/write/mutation.rs index 1b82e714d..6c22831af 100644 --- a/engine/crates/layerfs-store-service/src/write/mutation.rs +++ b/engine/crates/layerfs-store-service/src/write/mutation.rs @@ -3,6 +3,7 @@ use crate::history::lookup as args; use crate::store::{BoundWorkspace, StoreOperation}; use layerfs_bridge::control::{SdkRefusal, SdkValue}; use layerfs_content::filesystem::PathName; +use layerfs_content::object::inode_leaf::InodeKind; use layerfs_workspace::{ Operation, Outcome, OverlayJobs, SourceView, Time, ViewStat, WorkspaceError, }; @@ -19,6 +20,37 @@ pub struct Mutation<'a> { fn checked(path: &str, name: &str) -> Result { PathName::new(name).map_err(|_| args::invalid(format!("`{path}` is not valid"))) } +/// A caller's `mode` for the entry of `kind` at `path`: the permission bits +/// LayerFS stores, as `project.init` accepts them. Set-user-ID, set-group-ID +/// and the sticky bit on a file are refused as `InvalidInput` with reason +/// `Mode`; a symbolic link has no mode of its own. +fn permitted(path: &str, kind: InodeKind, mode: u32) -> Result { + let mask = match kind { + InodeKind::Directory => 0o1777, + InodeKind::RegularFile => 0o777, + InodeKind::Symlink => { + return Err(args::refuse( + "NotFile", + format!("`{path}` is a symbolic link, which has no mode of its own"), + SdkValue::Null, + )) + } + }; + if mode & !mask != 0 { + return Err(args::refuse( + "InvalidInput", + format!( + "mode {mode:04o} has bits LayerFS cannot store (set-user-ID, set-group-ID, or sticky on a file)" + ), + SdkValue::map([ + ("reason", SdkValue::text("Mode")), + ("path", SdkValue::text(path)), + ("mode", SdkValue::text(format!("{mode:04o}"))), + ]), + )); + } + Ok(mode) +} pub fn failure(e: WorkspaceError) -> SdkRefusal { let code = match &e { WorkspaceError::Refused(r) => match r { @@ -148,8 +180,8 @@ impl<'a> Mutation<'a> { })?; Ok(()) } - pub fn write(&mut self, path: &str, bytes: &[u8]) -> Result<()> { - let (serial, same) = self.target(path, bytes.len() as u64)?; + pub fn write(&mut self, path: &str, bytes: &[u8], mode: Option) -> Result<()> { + let (serial, same) = self.target(path, bytes.len() as u64, mode)?; let mut changed = !same; let mut offset = 0; for piece in bytes.chunks(layerfs_bridge::duplex::MAX_PLAINTEXT_BYTES) { @@ -171,6 +203,21 @@ impl<'a> Mutation<'a> { self.write_at(serial, offset, piece)?; Ok(true) } + fn set_mode(&mut self, serial: u64, mode: u32) -> Result<()> { + self.apply(Operation::SetAttributes { + serial, + mode: Some(mode), + mtime: None, + size: None, + })?; + Ok(()) + } + /// Sets the permission bits of the entry at `path`. + pub fn chmod(&mut self, path: &str, mode: u32) -> Result<()> { + let stat = self.resolve(path)?; + let mode = permitted(path, stat.kind, mode)?; + self.set_mode(stat.serial, mode) + } fn truncate(&mut self, serial: u64, size: u64) -> Result<()> { self.apply(Operation::SetAttributes { serial, @@ -180,14 +227,15 @@ impl<'a> Mutation<'a> { })?; Ok(()) } - /// The regular file a write of `length` bytes goes to, created when absent, - /// and whether it already has that length. - fn target(&mut self, path: &str, length: u64) -> Result<(u64, bool)> { + /// The regular file a write of `length` bytes goes to, created when absent + /// with `mode` (default 0o644), and whether it already has that length. An + /// existing file takes `mode` only when one is given. + fn target(&mut self, path: &str, length: u64, mode: Option) -> Result<(u64, bool)> { Ok(match self.optional(path)? { Some(stat) => { - if stat.kind != layerfs_content::object::inode_leaf::InodeKind::RegularFile { + if stat.kind != InodeKind::RegularFile { return Err(args::refuse( - if stat.kind == layerfs_content::object::inode_leaf::InodeKind::Directory { + if stat.kind == InodeKind::Directory { "IsDirectory" } else { "NotFile" @@ -196,37 +244,39 @@ impl<'a> Mutation<'a> { SdkValue::Null, )); } + if let Some(mode) = mode { + let mode = permitted(path, InodeKind::RegularFile, mode)?; + self.set_mode(stat.serial, mode)?; + } (stat.serial, stat.logical_len == length) } None => { + let mode = permitted(path, InodeKind::RegularFile, mode.unwrap_or(0o644))?; let (parent, name) = self.parent(path)?; let serial = self - .apply(Operation::Create { - parent, - name, - mode: 0o644, - })? + .apply(Operation::Create { parent, name, mode })? .ok_or_else(|| args::failed("created inode"))? .serial; (serial, false) } }) } - pub fn mkdir(&mut self, path: &str) -> Result<()> { + /// Creates the directory `path` with `mode` (default 0o755) and its + /// missing parents with 0o755; an existing directory keeps its mode. + pub fn mkdir(&mut self, path: &str, mode: Option) -> Result<()> { + let mode = permitted(path, InodeKind::Directory, mode.unwrap_or(0o755))?; let mut at = String::new(); - for name in path.split('/').filter(|s| !s.is_empty()) { + let mut names = path.split('/').filter(|s| !s.is_empty()).peekable(); + while let Some(name) = names.next() { at.push('/'); at.push_str(name); match self.optional(&at)? { - Some(s) if s.kind == layerfs_content::object::inode_leaf::InodeKind::Directory => {} + Some(s) if s.kind == InodeKind::Directory => {} Some(_) => return Err(args::refuse("NotDirectory", at, SdkValue::Null)), None => { let (parent, name) = self.parent(&at)?; - self.apply(Operation::Mkdir { - parent, - name, - mode: 0o755, - })?; + let mode = if names.peek().is_none() { mode } else { 0o755 }; + self.apply(Operation::Mkdir { parent, name, mode })?; } } }