From 2b09cc84e9456b6642b16d7f769efc4918de0ec1 Mon Sep 17 00:00:00 2001 From: Adrian Bonpin Date: Wed, 7 Oct 2026 16:51:41 +0800 Subject: [PATCH] docs: spec file system access for Lam Seven file tools behind a path policy and an approval step, with the platform findings that shaped it: extensions are unsandboxed Node, confirmAlert exists, rememberUserChoice does not, and environment.supportPath is the one directory an extension is invited to write to. Records the decisions taken with the user (read, write, edit and destructive tools; one approval per path per answer; home minus a credential denylist; extra roots in preferences) and the assumptions that follow from them, including the two that go beyond the questions asked: delete and move asking every time, and file access being off until switched on. --- .../2026-10-07-file-system-access-design.md | 404 ++++++++++++++++++ 1 file changed, 404 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-07-file-system-access-design.md diff --git a/docs/superpowers/specs/2026-10-07-file-system-access-design.md b/docs/superpowers/specs/2026-10-07-file-system-access-design.md new file mode 100644 index 0000000..cb376df --- /dev/null +++ b/docs/superpowers/specs/2026-10-07-file-system-access-design.md @@ -0,0 +1,404 @@ +# File system access for Lam + +Status: draft for review + +Date: 2026-10-07 + +## Problem + +Lam is a chat window that cannot look at anything on the machine unless the user +attaches a screenshot to a question. Every question about a file is therefore +answered from memory of a description the user typed, and the README says so in +as many words: "Chat only. Lam answers questions. It does not touch your files or +run commands." + +The capability is not the problem. `src/lib/attachments.ts` already imports +`readFile` from `node:fs/promises` to turn an attached screenshot into base64, and +`src/lib/capture.ts` writes capture files. The problem is that nothing here has a +permission model, and an assistant that can read and write without one is a +liability rather than a feature. + +### What the platform gives us, and what it does not + +Established by reading the Vicinae source in `/tmp/vicinae-src` +(`vicinaehq/vicinae`, `7733e95`): + +- **Extensions are unsandboxed Node.** Each command runs in a worker thread of the + extension manager (`src/typescript/extension-manager/src/index.ts`), and the only + `require` patch is a lookup table for React and `@vicinae/api` + (`src/typescript/extension-manager/src/patch-require.ts`). Everything else falls + through to the real loader, so `node:fs` and `node:path` work. Vicinae's own + boilerplate extension uses them (`extra/extension-boilerplate/src/wallpaper.tsx` + imports `readdir` from `node:fs/promises`). +- **There is no OS-level permission to lean on**, no manifest permission block + (`schemas/manifest.ts` has none), and `environment.canAccess` only covers + `FileSearch`, `BrowserExtension`, `Wallpaper`, `AI` and `WindowManagement`. If + Lam is to be bounded at all, this repository has to do the bounding. +- **`confirmAlert` works.** `src/server/src/extension/api/ui-service.hpp` implements + it with a `CallbackAlertWidget`, so an approval step can be shown from a running + command. +- **`rememberUserChoice` does not.** `Alert.Options` declares it, and no C++ file + mentions it, so "do not ask again" would have to be our own bookkeeping and must + not claim to be the platform's. +- **`environment.supportPath`** is documented as the directory an extension may read + and write, and is a sensible default write root. +- **`FileSearch`** exposes Vicinae's file index read only. It returns indexed + filename matches, needs the index to be built, and yields no file contents, so it + is not the shape the model needs. Not used. + +### The risk this feature creates + +Everything Lam reads is sent to the model, which is Ollama Cloud unless the user +self hosts. So reading a file is a disclosure even though it changes nothing on +disk. That is why the read boundary in this spec is as considered as the write +boundary, and why the README has to say it out loud. + +## Goals + +1. Lam can list a directory, find a file by name or content, and read a file. +2. Lam can create, overwrite and edit files, and can delete and move them. +3. Nothing outside the configured paths is readable or writable, and a symlink or a + `..` segment cannot change that. +4. Every operation that changes the disk is approved by the user, with the exact + affected path on screen, and a refusal is a normal outcome rather than a failure. +5. Credential stores stay unreadable by default, because their contents would end up + in a model request. +6. A refusal or a policy failure tells the model what happened and what it may do + instead, so the turn continues and produces an answer. + +## Non-goals + +- Running commands. There is no shell tool here, and adding one would make every + other boundary in this spec decorative. +- Watching files, tailing logs, or any long lived handle. +- Recursive delete. See A4. +- Editing in place with a patch format. Replacement is exact string substitution. +- A directory creation tool. The only way a directory comes into existence here is + the parent chain `write_file` creates for the file it was asked to write, which is + disclosed in the alert. +- A file picker or a browse view. The tools take paths the model writes. +- Reading or writing outside `$HOME`, except through a preference the user sets + themselves. +- Copying. `move_path` covers the useful case and copying doubles the files a + mistake can create. + +## Assumptions + +The four decisions this cycle rested on were put to the user directly; the smaller +ones that follow from them are recorded here. + +- **A1. An approval covers a path for one answer.** One `runAgent` call is one + answer, so the ledger lives for exactly that turn. The first write to a resolved + path raises one alert; further writes to that same path in that answer go through + without one; the next question starts empty. Chosen over "ask every time" because + a model that needs three passes at one file would otherwise interrupt three times, + and over "the roots are the permission" because a path mistake inside a write root + would then destroy a file silently. +- **A2. Delete and move always ask, even for a path already approved for writing.** + The one deliberate exception to A1, and the deviation from the user's answer that + was put back to them and confirmed. Every other operation leaves something + recoverable behind: an overwrite is a change the user watched, an edit is a + replacement, but a delete is the end of the file. An approval granted for an edit + ten seconds earlier is not consent to remove the thing that was just edited. + `delete_path` and `move_path` therefore skip the ledger entirely. +- **A3. Reads are bounded by `$HOME` minus a credential denylist.** Anything under + the home directory is readable except the locations in the denylist below. Chosen + over an unrestricted filesystem, which would put `/etc` and mounted volumes in a + model request, and over an allowlist that starts empty, which would make the + feature useless until it was configured. +- **A4. `delete_path` refuses a non-empty directory and never recurses.** A recursive + delete is the one call where a misread path destroys an unbounded amount of work, + and no confirmation dialog makes it recoverable. A single file, or an empty + directory, is the whole scope. The refusal names what is in the directory so the + model can ask for the contents one at a time. +- **A5. `move_path` refuses to overwrite an existing destination.** The alternative is + a move that silently destroys the file it lands on. The model can delete the + destination first, which asks again under A2. +- **A6. `write_file` overwrites without a separate flag, and discloses which it is.** + The approval alert says whether the file exists, how big it is, and how many lines + it has, so an overwrite is never a surprise. A `mustNotExist` flag would add a + round trip and a second approval to the common case to guard against something the + alert already shows. +- **A7. Relative paths resolve against `$HOME`, not the working directory.** A launcher + command has no meaningful working directory, and `~` is expanded, because both are + what the model will write by habit and a rejection would only cost a round trip. +- **A8. `search_files` searches names and optionally content.** Content search is + "which file mentions this string", which is the question that actually motivates + searching at all. It reads files, so it discloses what it reads exactly as + `read_file` does, and it is capped in depth and results. +- **A9. If the approval dialog cannot be shown, the call is refused.** Fail closed. + An unverified path in this repository is the confirmation rendering while a turn + is streaming, which cannot be tested locally; refusing is the only safe default. +- **A10. The ledger is keyed by resolved real path, not by the string the model + wrote.** `~/Downloads/x.txt`, `/Users/me/Downloads/x.txt` and a symlink to it are + one approval, because they are one file. Keying on the written form would let a + second spelling of the same path slip past an approval the user already gave for + it, and would show the same file's change twice as two permissions. +- **A11. File access is off until it is switched on, and when it is off the tools do + not exist for the model.** This is a third preference beyond the two the user asked + for, added because the read side is the silent one: a write always raises a dialog, + but a read goes straight into a model request, and the model may choose to read + without the user having asked about any file. `ipGeolocation` in this repository is + already off by default for the weaker version of the same reason (it discloses one + thing about the user to a third party), so the defaults here follow that precedent + rather than the `webSearchEnabled` one. Off means absent from `toolsFor`, not merely + refusing: the model never learns the tools exist, and the prompt paragraph is not + added. One checkbox turns it on. + +## Design + +### Module layout + +| file | job | +| --------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `src/lib/files.ts` (new) | Path policy and every file operation. No Vicinae import, so the whole boundary unit tests headlessly. | +| `src/lib/approval.ts` (new) | The only Vicinae facing part: the `confirmAlert` wording, and the per-answer ledger. | +| `src/lib/tools.ts` (edit) | Seven definitions and thin handlers, plus a `FILE_TOOL_NAMES` set so `toolsFor` can leave them out entirely when the feature is off (A11). Approval arrives as an injected callback, the way `ipPlace` already does. | +| `src/lib/preferences.ts` (edit) | `readRoots` and `writeRoots`, parsed by a pure `splitRoots()`. | +| `src/lib/constants.ts` (edit) | The caps, and the prompt paragraph. | +| `src/lib/hooks/useChat.ts` (edit) | Creates the ledger per turn and injects the confirm callback. | +| `package.json` (edit) | The two new preferences in the manifest. | +| `README.md` (edit) | The promise that Lam does not touch files, replaced by what it now does. | + +`files.ts` owns no UI and `approval.ts` owns no path logic, so the policy can be +tested without a dialog and the dialog without a disk. + +### Path model + +``` +read roots: $HOME minus the denylist +write roots: ~/Downloads + ~/Documents/Lam Agent + environment.supportPath + + whatever readRoots / writeRoots preferences add +``` + +Resolution, in order, for every operation that takes a path: + +1. Reject a non-string, an empty string, or a path containing a NUL byte. +2. Expand a leading `~` or `~/`; resolve a relative path against `$HOME` (A7). +3. `path.resolve` to a normal form, so `..` segments are gone. +4. `realpath` the path. If it does not exist (the create case), `realpath` the + nearest existing ancestor and re-append the remainder. +5. Check containment against the roots **after** step 4, so a symlink pointing + outside a root is judged by where it lands rather than by where it is written. +6. For a read, check the denylist against the resolved path. + +Containment is a component-wise comparison, not a string prefix: +`/Users/me/Downloads-evil` must not match the root `/Users/me/Downloads`. + +The denylist, matched on the resolved path, either exactly or as a directory +prefix: + +| rule | why | +| ----------------------------------------------------------------------------------- | ----------------------------------------------- | +| `~/.ssh`, `~/.gnupg`, `~/.aws`, `~/.config/gh`, `~/.docker/config.json`, `~/.netrc` | Credential stores and key material | +| basename `.env` or starting `.env.` | Application secrets, the common shape | +| basename `id_rsa`, `id_dsa`, `id_ecdsa`, `id_ed25519`, `credentials.json` | Private keys and cloud credential files | +| extension `.pem`, `.key`, `.p12` | Key material under a name no other rule catches | + +The denylist is a guard, not a proof: a credential file with a name nobody thought +of is still readable. That is stated in the risks rather than papered over. + +### Operations + +Every function returns a result object rather than throwing, so a policy refusal +and an I/O failure reach the model in the same shape. + +```ts +type FileResult = + | { ok: true; content: string; path: string; bytes?: number } + | { ok: false; content: string; refused: "policy" | "approval" | "io" }; +``` + +The distinction matters in one place only: a policy refusal means "there is +nothing here you are allowed to touch", while an approval refusal means "the user +said no to this one", and the model is told to stop trying in the first case and to +ask rather than retry in the second. + +| tool | parameters | notes | +| ---------------- | ------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `list_directory` | `path`, `hidden?` | Directories first, then files, each sorted. Capped at `MAX_LIST_ENTRIES` (200), with a count of what was left out. | +| `search_files` | `path`, `pattern`, `contains?` | `pattern` is a glob (`*`, `?`, `**`) matched against the basename; `contains` is a case-insensitive substring test on the file's first `MAX_SEARCH_BYTES` (64 KB). Capped at `MAX_SEARCH_RESULTS` (50) and `MAX_SEARCH_DEPTH` (8). A path that fails the read check is skipped, contents and all, so the denylist holds for search results as well as for direct reads. | +| `read_file` | `path`, `offset?`, `limit?` | Numbered lines, stopping at whichever comes first of the default 200 lines and `MAX_READ_CHARS` (8000) characters, with the notice naming that cap so it is clear which one stopped it. Binary content (a NUL byte in the first 8 KB) is refused with its size and detected type rather than returned as mojibake. | +| `write_file` | `path`, `content` | Creates the parent directory chain when it is missing, disclosed in the alert, since `~/Downloads/reports/may.md` is a normal thing to ask for and there is no separate mkdir tool. Reports `Created` or `Overwrote (was 1,240 bytes, 40 lines)`. | +| `edit_file` | `path`, `old_string`, `new_string`, `all?` | Exact substitution. Refuses when `old_string` occurs zero times or more than once without `all`, naming the count so the model can add context instead of guessing. | +| `delete_path` | `path` | One file, or an empty directory. A non-empty directory is refused with its entry count (A4). | +| `move_path` | `from`, `to` | Both the source and the destination are policy checked and both are approved. Refuses an existing destination (A5). | + +`toolLabel` gains "Reading a file", "Writing a file", "Editing a file", "Deleting a +file", "Moving a file", "Listing a folder", "Searching your files" for the activity +line. + +### Approval + +`approval.ts` exports `createApprovalLedger(confirm)` and the wording. + +```ts +type FileApproval = { + /** What is about to happen, in the user's terms. */ + title: string; + /** The concrete path, tilde shortened, plus the size and shape of what changes. */ + body: string; + /** Whether this one is irreversible, which decides the button style. */ + destructive: boolean; +}; +``` + +`confirm` is `(approval: FileApproval) => Promise`, implemented in +`useChat.ts` with `confirmAlert`. The ledger remembers resolved paths approved +during the turn (A1, A10) and is consulted only by the five mutating operations +other than delete and move, which always ask (A2). + +The alert content is the point of the whole design, so it is specified rather than +left to the implementation: + +``` +Title: Overwrite report.md? +Body: /Users/me/Downloads/report.md + Replaces 1,240 bytes, 40 lines, last changed 3 days ago. + New content is 2,180 bytes, 61 lines. + [Allow] [Decline] +``` + +When the parent directory does not exist, the body gains a line before the rest: +`Creates the folder ~/Downloads/reports as well.` Nothing else appears in the +body that the user cannot act on, and the exact path is always there, because the +path is the thing being approved. + +- `edit_file` shows the substitution itself, both sides truncated to 200 + characters: `old_string → new_string`, which is the one alert where showing the + change is better than showing the size of the file. +- `delete_path` uses `Alert.ActionStyle.Destructive` on the primary action and adds + "This cannot be undone." +- `move_path` shows both paths and says whether the destination exists. +- A policy refusal never reaches the user as a dialog at all, because there is + nothing to approve. +- The tool record keeps the refusal text, so a transcript with Show Tool Calls on + shows what was declined next to what was allowed. No new field is needed for that, + which is why `ToolCallRecord` does not change in this spec. + +### Refusals the model reads + +Short, factual, and actionable, in the same voice as the rest of the tool output: + +``` +Refused: /etc/passwd is outside the paths Lam may read. Readable: your home +directory, except credential stores. Nothing was read. + +Refused: ~/.ssh/id_ed25519 is a credential store, so Lam does not read it. + +Refused: the user declined to write /Users/me/Downloads/draft.md. Nothing was +written. Ask before trying again. + +Refused: ~/Downloads/archive is a directory with 42 entries, and Lam does not +delete directories recursively. List it and delete what you actually need. +``` + +### Prompt + +The system prompt gains a paragraph naming the write roots, saying that every +change is approved by the user and that a refusal is final, that paths outside the +roots are refused outright, and that a large file should be read in parts. It also +gains one line of disclosure: file contents go to the model provider, so Lam reads +what the question needs and nothing more. + +### Preferences + +Two new text fields in `package.json`, mirrored in `LamPreferences`, plus the switch +from A11: + +| preference | shape | +| ------------ | ------------------------------------------------------------------------------------------------------------------------- | +| `fileAccess` | Checkbox. Off by default, and off means the seven tools are absent from `toolsFor` and the prompt paragraph is not added. | +| `readRoots` | Extra read roots, one absolute path per line | +| `writeRoots` | Extra write roots, one absolute path per line | + +`splitRoots(value)` in `files.ts` is pure and tested: it splits on newlines and +commas, trims, expands `~`, drops empty entries, keeps a path that does not exist +yet (a write root may be created later), and reports entries that are not absolute +rather than silently ignoring them. A bad entry does not disable the others, and +the defaults above always apply, so an empty preference is the same as no +preference. + +## Error handling + +| Situation | Behaviour | +| ------------------------------- | ----------------------------------------------------------------------- | +| Path outside every root | Refused, naming the roots. The user is not prompted. | +| Path inside the denylist | Refused, naming the rule. | +| Symlink or `..` escaping a root | Resolved first, then refused, and the refusal names the real location. | +| Path does not exist (read) | `ENOENT` reported with the path, not a stack. | +| Path exists (create) | Reported as an overwrite with the current size. | +| Permission denied by the OS | Refused with the OS message, which is already specific. | +| File larger than the cap | The cap is applied and stated; nothing is silently cut. | +| Binary file | Refused with size and detected type. | +| Non-empty directory delete | Refused with the entry count (A4). | +| Move onto an existing path | Refused, naming both paths (A5). | +| User declines | Refusal returned as a tool result with `isError: true`, turn continues. | +| Approval dialog cannot be shown | Refused (A9). | +| Ledger already holds the path | No dialog for a write or an edit; a delete or a move asks anyway (A2). | +| Feature off (A11) | The tools are not offered, so the model cannot call them | + +## Testing + +- `files.test.ts`, new, the bulk of the suite: a table of resolution cases + (`~`, relative, `..`, symlink out of a root, `/Users/me/Downloads-evil`, + denylist hits and near misses, non-existent parent chains), plus operation tests + against a real temporary directory built with `mkdtemp`, including the delete and + move refusals, the binary guard, line offsets, and the caps. +- `approval.test.ts`, new: the ledger approves once per resolved path per turn, + treats two spellings of one path as one entry (A10), and never suppresses a + delete or a move (A2); the wording for each of the five operations. +- `tools.test.ts`: each tool dispatches to the right operation; a declined call + returns `isError` with the refusal text; a policy refusal never calls the confirm + callback; the tools stay out of the list when the feature is off. +- **`preferences.test.ts`**: `splitRoots` for empty, comma-separated, newline-separated, + padded, `~`, relative and repeated input. +- Mutation checks on the policy table: inverting a containment comparison, checking + before `realpath`, and keying the ledger on the written path must each fail a test. + +## Approaches considered + +- **One implementation in `tools.ts`, importing `confirmAlert` directly.** Fewer + files, but it makes the tool layer Vicinae dependent, against the deliberate split + that lets `tools.ts` and `agent.ts` run in a plain unit test, and it puts the + policy somewhere a test cannot reach it without the runtime. +- **A separate process holding the file access, with the extension talking to it.** + A real boundary, and the only one an attacker could not walk around, but it needs + the toolchain to spawn and supervise a process, and it moves the approval dialog + somewhere it cannot be shown. +- **Vicinae's `FileSearch` for discovery.** Rejected: indexed names only, no + contents, and it needs the user's index to be enabled and current. +- **An allowlist that starts empty.** Rejected in A3: the feature would do nothing + until configured, and the first experience of a new capability should not be a + refusal. +- **Asking on every mutating call (the "every write asks" option).** Rejected in A1 + in favour of one approval per path per answer. +- **Recursive delete with a stronger confirmation.** Rejected in A4. + +## Risks + +- **Reading is disclosure.** Every byte read leaves the machine in a model request. + The denylist and the home boundary limit the blast radius; they cannot make a file + safe to read. The README will say this plainly, and the prompt will tell the model + to read only what the question needs. +- **The denylist is a list of guesses.** A key file with an unusual name under an + allowed path is readable. This is a denylist by construction, so it is bounded by + what was thought of. +- **The confirmation has not been seen rendering.** Vicinae cannot be run in this + environment, so `confirmAlert` during a streaming turn is reasoned from the C++ + implementation. A9 fails closed, and the first real use will tell us. +- **`rememberUserChoice` is not implemented**, so nothing here may offer "do not ask + again" as though the platform provided it. The ledger is per answer, which is a + defensible unit and not a promise about the future. +- **A model with write access can still do damage inside a write root.** Once + `~/Downloads` is approved, a wrong path inside it is real damage. That is why the + alert shows the concrete path, size and line count rather than the tool name. +- **A long turn can spend many approvals.** The ledger limits this to one per + distinct path, and the tool round cap bounds it further. +- **The check and the write are not atomic.** Between resolving a path and acting on + it, a symlink could be swapped so that the write lands somewhere the policy did not + clear. Defending that needs `O_NOFOLLOW` and an open file handle per operation, + which is a different design, and the threat it guards against is another process + already running as the user. Out of scope, stated rather than implied.