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.