From baa07f1f7458b0613d558221933bbe30efd34670 Mon Sep 17 00:00:00 2001 From: devswha Date: Fri, 6 Nov 2026 05:32:10 +0900 Subject: [PATCH 1/4] feat(files): edit a text file from the file viewer, opt-in (#771) Settings > Chat > File viewer > Edit text files (off by default) adds Edit to the viewer for a text file in the pane's folder. Save asks once more and is refused when the file changed since it was opened. GET/PUT /api/fs/text (server/file-edit.ts): only an existing UTF-8 file under the pane folder's real path (never .git, never a filesystem root), owned by this user, with one link and one kind of line ending, up to 1 MiB; written to a temp file beside it with its mode and group and renamed over it. On Linux every step after the proof goes through an O_DIRECTORY|O_NOFOLLOW descriptor of its folder (/proc/self/fd/N), on macOS the owned-folders proof runs again right before the rename; other platforms answer 501. The SHA-256 revision is checked before the temp file is written and again right before the rename. Only a device that drives the PC (not watch, Portal or an open LAN window); the PC proxy forwards fs/text and the other PC applies the same rules. --- DESIGN.md | 10 ++ changelog.d/edit-text-files.md | 4 + docs/development.md | 1 + docs/guide.md | 1 + scripts/ci-browser.sh | 1 + scripts/text-file-edit-regression.ts | 137 +++++++++++++++ server/dispatch.ts | 7 + server/file-edit.test.ts | 216 ++++++++++++++++++++++++ server/file-edit.ts | 224 +++++++++++++++++++++++++ server/file-routes.ts | 38 ++++- server/file-view.ts | 2 +- server/machine-api.ts | 2 +- server/machine-proxy.test.ts | 21 +++ shared/files.ts | 45 +++++ src/App.tsx | 6 +- src/components/FileViewer.css | 61 +++++++ src/components/FileViewer.tsx | 37 ++-- src/components/SettingsDialog.tsx | 3 + src/components/TextFileEditor.test.tsx | 125 ++++++++++++++ src/components/TextFileEditor.tsx | 128 ++++++++++++++ src/lib/api.ts | 13 +- src/lib/i18n.test.ts | 2 +- src/lib/i18n/files.ja.ts | 18 +- src/lib/i18n/files.ko.ts | 18 +- src/lib/i18n/files.zh.ts | 18 +- src/lib/i18n/settings.ja.ts | 2 + src/lib/i18n/settings.ko.ts | 2 + src/lib/i18n/settings.zh.ts | 2 + src/lib/machineContext.tsx | 2 + src/lib/settings.legacy-fixture.ts | 2 +- src/lib/settings.ts | 8 +- 31 files changed, 1132 insertions(+), 24 deletions(-) create mode 100644 changelog.d/edit-text-files.md create mode 100644 scripts/text-file-edit-regression.ts create mode 100644 server/file-edit.test.ts create mode 100644 server/file-edit.ts create mode 100644 src/components/TextFileEditor.test.tsx create mode 100644 src/components/TextFileEditor.tsx diff --git a/DESIGN.md b/DESIGN.md index 8cd8ce0436..6b4e283038 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -428,6 +428,16 @@ blocks of `src/styles.css`. ### File viewer +- Edit text files (off by default, Settings → File viewer): a pencil **Edit file** button in the header + turns the body into a text box (`.file-editor-text`, the source's mono size, never under + `--fs-input` on a touch screen so iOS does not zoom) under one bar: what is going on on the left + in `--text-dim` (`--notice` for a conflict or a refusal, with `role="alert"`), the buttons on the right. + Save asks again in the same bar ("Save changes to NAME?", Keep editing / Save) instead of a + dialog over the dialog. A file changed since it was opened follows Zed's "changed on disk" handling: + the typed text stays in the box to copy, and Reload file shows the file as it is now; unlike + Zed there is no Overwrite, so a save never replaces a change the editor has not seen. While the + text differs from the file, Escape, a press on the scrim and the sheet swipe do not close the + viewer; Cancel and Close do. - `--notice` colors a note that the view is partial (a file cut short, code too long to color): the size in the header's meta line becomes `256 KB of 1.3 MB`, always beside a warning icon so the color is not the only sign. It is a warning yellow, at least 4.5:1 on diff --git a/changelog.d/edit-text-files.md b/changelog.d/edit-text-files.md new file mode 100644 index 0000000000..c731285b1b --- /dev/null +++ b/changelog.d/edit-text-files.md @@ -0,0 +1,4 @@ +### Added +- The file viewer can edit a text file in the pane's folder: turn on Settings → File viewer → Edit + text files (off by default). Save asks once more, and a file that changed on the PC since you + opened it is not overwritten. Only a device that drives the PC can edit, on Linux and macOS PCs. diff --git a/docs/development.md b/docs/development.md index b679621871..43ca38ee3a 100644 --- a/docs/development.md +++ b/docs/development.md @@ -256,6 +256,7 @@ bun scripts/settings-pages-demo-regression.ts # every Settings page on a 390px a bun scripts/agent-entry-points-demo-regression.ts # Add another agent… in New workspace and New worktree, Settings' agent list (installed, the tiles to install) and Open setup guide, and none of it on a watching device, at 1280 and 390 bun scripts/interface-font-demo-regression.ts # Interface font at 390 and 1440: System and a typed list lead --font-ui, a reload keeps it, Default restores it; the message box takes the Chat font bun scripts/file-text-size-demo-regression.ts # Settings → File text size at 390 and 1280, light and dark: a Markdown preview follows the chat and a source the interface until a size is chosen, then both take it while the chat and the viewer's header stay; Reset drops it +bun scripts/text-file-edit-regression.ts # Settings → Chat → File viewer → Edit text files at 1280 and 390, light and dark, on an owned herdr pane: off by default; on, Save asks once more and writes, Cancel leaves the file, a file changed on the PC is not overwritten (Reload file shows it), a read-only file says it can't be written bun scripts/files-panel-demo-regression.ts # Settings → Chat → Files beside the pane at 1280 and 390, light and dark: off, Files is the dialog; on, a wide window shows it right of the pane with the pane live, a docked file takes its place and closing it brings the list back; a phone keeps the sheet bun scripts/palette-local-page-demo-regression.ts # Web links on In the app, at 1280 and 390, light and dark: the palette opens a page this PC serves by its typed address (`localhost:`) beside the pane, a phone's popup closes with Back; off by default, the palette offers no page bun scripts/theme-first-paint-demo-regression.ts # the saved theme in every painted frame before and after the app starts, root and demo builds under the app's CSP diff --git a/docs/guide.md b/docs/guide.md index aa4872e85b..39c0a38ad0 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -339,6 +339,7 @@ across an Ovis restart, which forgets where it saw each `letta` before. Check th | **Compose** | `/` commands and `@` file mentions, any file or image up to 32 MB attached by path, and a draft per pane. Send during work schedules a next-turn message; use its ↑ Send now action to send it now. A strip under the input card names the pane's working folder (the whole path on hover) and, for a workspace of a repository with a worktree open, its branch at the other end; a phone leaves it out. Pick or type a terminal-only command and send normally: after acceptance, the pane's terminal opens over the chat, in this browser only, with **Back to Chat** and **Keep Terminal**; the chat stays where you were reading. For Claude Code's panels (`/config`, `/permissions`, `/theme` and the like) the chat may come back by itself when you close the panel, once Claude records the close; for Codex, omp, pi, gjc, OmO, Hermes (classic CLI), Devin, agy and Grok it comes back after their recorded pickers and panels, once the agent's empty input row is back on the screen; otherwise use **Back to Chat**, the header's Chat, or Back on a phone. If you go back to the same pane's chat while the command's screen is still open, the composer says so (Claude Code and every agent above, not OpenCode) and holds Send, quick replies and a queued message's Send now until the screen closes (**Send anyway** sends the message once); **Show terminal** opens it again. Messages the PC sends by itself at the end of a turn are not held. Automatic queued delivery never changes your view. Chat keeps one quiet **Ran in terminal** row with **Open Terminal**, which shows the terminal over the chat again; ordinary commands keep their outcome chips, and Hermes keeps its informational result cards. `/resume` opens **Earlier sessions…**. [Agent commands in Chat](agent-commands.md) explains where each agent's commands come from and what counts as a result. | | **Follow every agent** | Live RUN / INPUT / DONE / READY status for all panes, and alerts when an agent needs input, finishes or its terminal ends. | | **Open what agents make** | A file path in an answer opens in a viewer (images, video, audio, PDF, text), or find it with **Browse files**, and download it to your phone. A source file opens whole up to 8 MB and 200,000 lines; past 2,000 lines it scrolls as a window of rows, colored up to 256 KB and 5,000 lines and plain beyond. A link with `:line` or `#L12` opens at that line. | +| **Edit a text file** | Off by default: turn on **Settings → Chat → File viewer → Edit text files** on this device. A text file under the pane's folder, up to 1 MB, then shows **Edit file** in the viewer: change it in a plain text box and press **Save** twice (the second press confirms). If the file changed on the PC since you opened it, nothing is written: your text stays in the box to copy, and **Reload file** shows the file as it is now. Only a device that drives the PC can edit (not a watching device, a Portal visitor or an open LAN window), and only a UTF-8 file with one kind of line ending that is yours, not read-only and not hard-linked; a file outside the pane's folder, or in `.git`, never opens for editing. The file is replaced in one step with its permissions and line endings kept. Saving works on Linux and macOS PCs; a Windows PC says it can't save yet. A remote PC's file is edited and checked on that PC. | | **See what changed** | **Browse files → Changes** lists what changed in the git repository the pane sits in: the **Working tree**, what is **Staged**, or two **Commits** you name, as a folder tree with git's status letter and the added and removed lines. A row opens its diff, **Unified** or **Split** (Settings → Chat → File viewer → Diff layout; a phone shows one column), with **Copy path**, **Copy patch** and **Open file**. The files a turn's edit tools changed, listed in its work block, open the same diff. It only reads; a remote PC's repository is read on that PC. | | **Manage sessions** | Start an agent in a folder you type or pick with **Browse**. In New workspace, Browse filters the currently loaded folders as you type (case-insensitive); opening a folder updates Directory, so **Start** uses that path even without **Use this folder** (which closes Browse), and a folder under home stays written as `~/…`. **New folder** (the folder-plus button in Browse's bar) makes a folder inside the one shown and opens it. It does not search subfolders or folders beyond the displayed 500. Add a tab to a workspace (as herdr's prefix+c), switch tabs from the strip over the pane, rename workspaces from their ⋯/right-click menu and panes from their own menu, reorder workspaces (drag a row, or on a phone long-press it and drag; Alt+Up/Down moves a focused row one step), and jump anywhere from the command palette. For Claude Code, New workspace and New tab have an **Ultracode** switch under Model and Effort, which asks Claude Code to start the session with Ultracode on at the chosen effort level. It needs Claude Code 2.1.284 or later, a model with xhigh and dynamic workflows on; otherwise Claude Code ignores it without a word (see [Starting an agent with a first message](agent-commands.md#starting-an-agent-with-a-first-message)). | | **Speak instead of typing** | A mic beside Attach in the composer and beside Send in the terminal input line. Tap to start and again to finish, or hold it while you talk (not on a touch screen without an API key); the words land at the caret and are never sent by themselves. See [Voice input](#voice-input). | diff --git a/scripts/ci-browser.sh b/scripts/ci-browser.sh index f109c39423..1488bdf298 100644 --- a/scripts/ci-browser.sh +++ b/scripts/ci-browser.sh @@ -145,6 +145,7 @@ run_script scripts/omp-codex-image-browser-qa.ts run_script scripts/grok-image-browser-qa.ts run_script scripts/copilot-image-browser-qa.ts run_script scripts/file-viewer-regression.ts +run_script scripts/text-file-edit-regression.ts run_script scripts/web-panel-mobile-regression.ts run_script scripts/palette-local-page-demo-regression.ts run_script scripts/pwa-update-browser-qa.ts diff --git a/scripts/text-file-edit-regression.ts b/scripts/text-file-edit-regression.ts new file mode 100644 index 0000000000..04ee92af76 --- /dev/null +++ b/scripts/text-file-edit-regression.ts @@ -0,0 +1,137 @@ +/** + * Settings → File viewer → Edit text files (#771) at 1280 and 390, light and dark, on an owned herdr + * pane: off by default no Edit; on, a file in the pane's folder is edited, Save asks once more and + * writes it, Cancel leaves it, a file changed on the PC meanwhile is not overwritten (the typed + * text stays, Reload file shows the PC's), and a read-only file says it cannot be written. + * Screenshots go to UI_EVIDENCE_DIR when it is set. + */ +import "./test-herdr.ts"; +import assert from "node:assert/strict"; +import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { chromium, type Browser, type Locator, type Page } from "playwright-core"; +import { createServer } from "../server/index.ts"; +import { workspaceClose, workspaceCreate } from "../server/herdr/client.ts"; +import { chromePath } from "./chrome-path.ts"; + +const root = mkdtempSync(join(tmpdir(), "ovis-text-edit-")); +const project = join(root, "project"); +mkdirSync(project); +const evidence = process.env["UI_EVIDENCE_DIR"] || null; +let server: ReturnType | undefined; +let workspace: string | undefined; +let browser: Browser | undefined; + +async function open(origin: string, pane: string, width: number, theme: "light" | "dark", settings: Record, file: string): Promise<{ page: Page; viewer: Locator; errors: string[]; close: () => Promise }> { + const phone = width <= 480; + const context = await browser!.newContext({ viewport: { width, height: phone ? 844 : 900 }, locale: "en-US", hasTouch: phone, isMobile: phone, colorScheme: theme }); + await context.addInitScript((value) => { if (window.top === window && localStorage.getItem("ovis:settings") === null) localStorage.setItem("ovis:settings", value); }, JSON.stringify({ language: "en", theme, fileOpen: "popup", ...settings })); + const page = await context.newPage(); + page.setDefaultTimeout(10_000); + const errors: string[] = []; + page.on("pageerror", (error) => errors.push(error.message)); + await page.goto(`${origin}/?pane=${encodeURIComponent(pane)}&file=${encodeURIComponent(file)}`); + const viewer = page.getByRole("dialog", { name: file, exact: true }); + await viewer.waitFor(); + return { page, viewer, errors, close: () => context.close() }; +} + +const shot = async (page: Page, name: string): Promise => { if (evidence) await page.screenshot({ path: join(evidence, `${name}.png`) }); }; + +try { + const created = await workspaceCreate({ cwd: project, label: "ovis-test-text-edit" }); + workspace = created.workspace.workspace_id; + const pane = created.root_pane.pane_id; + server = createServer({ port: 0, hostname: "127.0.0.1", token: "", stateDir: join(root, "state") }); + const origin = `http://127.0.0.1:${server.port}`; + browser = await chromium.launch({ executablePath: chromePath(), headless: true, args: ["--no-sandbox"] }); + + for (const width of [1280, 390]) { + for (const theme of ["light", "dark"] as const) { + const tag = `${width}-${theme}`; + const notes = join(project, `notes-${tag}.md`); + writeFileSync(notes, "# Notes\n\nFirst line.\n"); + + // off by default: the viewer offers no Edit + const off = await open(origin, pane, width, theme, {}, `notes-${tag}.md`); + await off.viewer.getByRole("button", { name: "Show source", exact: true }).waitFor(); + assert.equal(await off.viewer.getByRole("button", { name: "Edit file", exact: true }).count(), 0, `${tag}: no Edit while the setting is off`); + // Settings → Chat → File viewer holds the switch, off + await off.page.goto(`${origin}/?pane=${encodeURIComponent(pane)}&settings=chat`); + const toggle = off.page.locator(".settings-dialog").getByRole("switch", { name: "Edit text files", exact: true }); + await toggle.scrollIntoViewIfNeeded(); + assert.equal(await toggle.getAttribute("aria-checked"), "false", `${tag}: Edit text files is off by default`); + await shot(off.page, `text-edit-setting-${tag}`); + await off.close(); + + const on = await open(origin, pane, width, theme, { editTextFiles: true }, `notes-${tag}.md`); + const { page, viewer } = on; + await viewer.getByRole("button", { name: "Edit file", exact: true }).click(); + const box = viewer.getByRole("textbox", { name: `Text of notes-${tag}.md`, exact: true }); + assert.equal(await box.inputValue(), "# Notes\n\nFirst line.\n"); + await box.fill("# Notes\n\nFirst line.\nSecond line.\n"); + await viewer.getByText("Unsaved changes", { exact: true }).waitFor(); + // Save fades in as it is enabled: shown once it is + await viewer.getByRole("button", { name: "Save", exact: true }).evaluate((button) => Promise.all(button.getAnimations().map((animation) => animation.finished))); + await shot(page, `text-edit-editing-${tag}`); + // Escape does not close over changes not saved + await page.keyboard.press("Escape"); + assert.equal(await viewer.isVisible(), true, `${tag}: Escape keeps the editor open over changes`); + await viewer.getByRole("button", { name: "Save", exact: true }).click(); + await viewer.getByText(`Save changes to notes-${tag}.md?`, { exact: true }).waitFor(); + assert.equal(readFileSync(notes, "utf8"), "# Notes\n\nFirst line.\n", `${tag}: the first Save writes nothing`); + await shot(page, `text-edit-confirm-${tag}`); + await viewer.getByRole("button", { name: "Save", exact: true }).click(); + await viewer.getByRole("button", { name: "Edit file", exact: true }).waitFor(); + assert.equal(readFileSync(notes, "utf8"), "# Notes\n\nFirst line.\nSecond line.\n", `${tag}: the second Save writes the file`); + await viewer.getByText("Second line.", { exact: true }).waitFor(); + console.log(`PASS ${tag}: Edit, Save asks once more, then the file holds the text and the viewer shows it`); + + // Cancel leaves the file as it is + await viewer.getByRole("button", { name: "Edit file", exact: true }).click(); + await box.fill("thrown away\n"); + await viewer.getByRole("button", { name: "Cancel", exact: true }).click(); + await viewer.getByRole("button", { name: "Edit file", exact: true }).waitFor(); + assert.equal(readFileSync(notes, "utf8"), "# Notes\n\nFirst line.\nSecond line.\n", `${tag}: Cancel writes nothing`); + console.log(`PASS ${tag}: Cancel leaves the file as it was`); + + // the PC's copy changes while the text is edited: nothing is overwritten + await viewer.getByRole("button", { name: "Edit file", exact: true }).click(); + await box.fill("mine\n"); + writeFileSync(notes, "the agent's version\n"); + await viewer.getByRole("button", { name: "Save", exact: true }).click(); + await viewer.getByRole("button", { name: "Save", exact: true }).click(); + await viewer.getByRole("alert").getByText(/your changes were not saved/).waitFor(); + assert.equal(readFileSync(notes, "utf8"), "the agent's version\n", `${tag}: a changed file is not overwritten`); + assert.equal(await box.inputValue(), "mine\n", `${tag}: the typed text stays to copy`); + await shot(page, `text-edit-conflict-${tag}`); + await viewer.getByRole("button", { name: "Reload file", exact: true }).click(); + await page.waitForFunction(([name]) => document.querySelector(`textarea[aria-label="Text of ${name}"]`)?.value === "the agent's version\n", [`notes-${tag}.md`]); + await viewer.getByRole("button", { name: "Cancel", exact: true }).click(); + console.log(`PASS ${tag}: a file changed on the PC is not overwritten; Reload file shows the PC's text`); + assert.deepEqual(on.errors, [], `${tag}: no page errors`); + await on.close(); + + // a read-only file opens for editing but says it cannot be written + const locked = join(project, `locked-${tag}.md`); + writeFileSync(locked, "locked\n"); + chmodSync(locked, 0o444); + const ro = await open(origin, pane, width, theme, { editTextFiles: true }, `locked-${tag}.md`); + await ro.viewer.getByRole("button", { name: "Edit file", exact: true }).click(); + await ro.viewer.getByRole("textbox").fill("changed\n"); + await ro.viewer.getByRole("button", { name: "Save", exact: true }).click(); + await ro.viewer.getByRole("button", { name: "Save", exact: true }).click(); + await ro.viewer.getByRole("alert").getByText("This file can't be written here.", { exact: true }).waitFor(); + assert.equal(readFileSync(locked, "utf8"), "locked\n"); + await shot(ro.page, `text-edit-refused-${tag}`); + console.log(`PASS ${tag}: a read-only file says it can't be written, and stays as it was`); + await ro.close(); + } + } +} finally { + await browser?.close(); + server?.stop(); + if (workspace) await workspaceClose(workspace); + rmSync(root, { recursive: true, force: true }); +} diff --git a/server/dispatch.ts b/server/dispatch.ts index fc0633bba3..db17a0a0ea 100644 --- a/server/dispatch.ts +++ b/server/dispatch.ts @@ -29,6 +29,7 @@ import type { AgentDetector } from "./agent-catalog-detect.ts"; import { handleAgentCatalogRequest } from "./agent-catalog-routes.ts"; import { type AgentInstaller, INSTALL_PATH, SIGN_IN_PATH } from "./agent-catalog-install.ts"; import { forgetPageMedia } from "./file-view.ts"; +import { textEditAllowed } from "./file-edit.ts"; import { handleFileRequest } from "./file-routes.ts"; import { BACKGROUND_AGENT_PATH, handleBackgroundAgentRequest } from "./background-agent.ts"; import { paneGet, ping } from "./herdr/client.ts"; @@ -261,6 +262,12 @@ export async function handleRequest(request: Request, ctx: ServerContext): Promi setRequestRoute("watch-refusal"); return jsonResponse({ error: { code: "read_only", message: "this device can only watch" } }, 403); } + // a text file is edited (read for it and saved) only by a device that drives this PC, here or through it on another PC (file-edit.ts) + const textEdit = /^\/api\/(?:machines\/[^/]+\/)?fs\/text$/.test(pathname); + if (textEdit && !textEditAllowed(access, bridgeAuthorized, pathname.startsWith("/api/machines/"))) { + setRequestRoute("drive-refusal"); + return jsonResponse({ error: { code: "drive_only", message: "Only a device that drives this PC can edit its files" } }, 403); + } // Portal visitors use the same watch checks above as paired watch devices. Even a Portal // identity explicitly given drive in local state cannot use these controls to leave access // behind: device changes (including redeeming a pairing code), relay controls (a relay offer diff --git a/server/file-edit.test.ts b/server/file-edit.test.ts new file mode 100644 index 0000000000..c086b27ce4 --- /dev/null +++ b/server/file-edit.test.ts @@ -0,0 +1,216 @@ +import { afterAll, afterEach, beforeAll, describe, expect, it, spyOn } from "bun:test"; +import { chmodSync, linkSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, renameSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createHash } from "node:crypto"; + +import type { Access } from "./access.ts"; +import { handleRequest, type ServerContext } from "./dispatch.ts"; +import { MAX_TEXT_SAVE_BODY_BYTES, textEditAllowed, textSaveHooks } from "./file-edit.ts"; +import * as herdr from "./herdr/client.ts"; +import { createServer } from "./index.ts"; +import { MAX_EDITABLE_TEXT_BYTES, type EditableText } from "../shared/files.ts"; + +const sha = (bytes: string | Uint8Array) => createHash("sha256").update(bytes).digest("hex"); + +describe("editing a text file (#771)", () => { + let server: ReturnType; + let stateDir: string; + let folder: string; + let outside: string; + let paneCwd: string; + const url = (path: string) => `http://127.0.0.1:${server.port}${path}`; + const q = (value: string) => encodeURIComponent(value); + const error = async (response: Response) => [response.status, ((await response.json()) as { error: { code: string } }).error.code]; + const open = (path: string) => fetch(url(`/api/fs/text?pane_id=w1:p1&path=${q(path)}`)); + const save = (path: string, revision: string, text: string, headers: Record = {}) => fetch(url("/api/fs/text"), { + method: "PUT", headers: { origin: url(""), "content-type": "application/json", ...headers }, + body: JSON.stringify({ pane_id: "w1:p1", path, revision, text }), + }); + /** a file under the pane's folder, written fresh for one test */ + const file = (relative: string, content: string | Uint8Array, mode?: number): string => { + const path = join(folder, relative); + mkdirSync(join(path, ".."), { recursive: true }); + writeFileSync(path, content); + if (mode !== undefined) chmodSync(path, mode); + return path; + }; + let paneGet: ReturnType; + + beforeAll(() => { + stateDir = mkdtempSync(join(tmpdir(), "ovis-file-edit-")); + folder = join(stateDir, "project"); + outside = join(stateDir, "outside"); + mkdirSync(folder); + mkdirSync(outside); + paneCwd = folder; + paneGet = spyOn(herdr, "paneGet").mockImplementation(async (paneId) => ({ pane_id: paneId, workspace_id: "w1", tab_id: "w1:t1", cwd: paneCwd, agent_status: "idle" }) as unknown as Awaited>); + server = createServer({ port: 0, hostname: "127.0.0.1", stateDir, token: "", machines: false, sidecar: false }); + }); + afterEach(() => { + delete textSaveHooks.beforeRename; + delete textSaveHooks.platform; + paneCwd = folder; + }); + afterAll(() => { + server.stop(); + paneGet.mockRestore(); + rmSync(stateDir, { recursive: true, force: true }); + }); + + it("opens a file in the pane's folder and saves it in its place, with its mode and nothing left beside it", async () => { + const notes = file("docs/notes.md", "# Notes\n", 0o640); + const opened = await open("docs/notes.md"); + expect(opened.status).toBe(200); + const body = await opened.json() as EditableText; + expect(body).toMatchObject({ path: notes, text: "# Notes\n", revision: sha("# Notes\n"), line_endings: "lf", size: 8 }); + const saved = await save(body.path, body.revision, "# Notes\n\nDone.\n"); + expect(saved.status).toBe(200); + expect(await saved.json()).toMatchObject({ path: notes, revision: sha("# Notes\n\nDone.\n"), size: 15 }); + expect(readFileSync(notes, "utf8")).toBe("# Notes\n\nDone.\n"); + expect(statSync(notes).mode & 0o777).toBe(0o640); + expect(readdirSync(join(folder, "docs"))).toEqual(["notes.md"]); + }); + + it("gives a CRLF file back with LF and saves it with CRLF again, its BOM kept", async () => { + const crlf = file("windows.txt", "\uFEFFone\r\ntwo\r\n"); + const body = await (await open(crlf)).json() as EditableText; + expect(body).toMatchObject({ text: "\uFEFFone\ntwo\n", line_endings: "crlf" }); + expect((await save(crlf, body.revision, "\uFEFFone\ntwo\nthree\n")).status).toBe(200); + expect(readFileSync(crlf, "utf8")).toBe("\uFEFFone\r\ntwo\r\nthree\r\n"); + }); + + it("refuses a save to a file that changed since it was opened, and writes nothing", async () => { + const config = file("config.json", "{\"a\": 1}\n"); + const body = await (await open(config)).json() as EditableText; + writeFileSync(config, "{\"a\": 2}\n"); + const refused = await save(config, body.revision, "{\"a\": 3}\n"); + expect(refused.status).toBe(409); + expect(await refused.json()).toMatchObject({ error: { code: "stale_revision", revision: sha("{\"a\": 2}\n") } }); + expect(readFileSync(config, "utf8")).toBe("{\"a\": 2}\n"); + }); + + it("refuses a file another program changes between the checks, and leaves that program's text", async () => { + const plan = file("plan.md", "v1\n"); + textSaveHooks.beforeRename = () => writeFileSync(plan, "agent's v2\n"); + expect(await error(await save(plan, sha("v1\n"), "mine\n"))).toEqual([409, "stale_revision"]); + expect(readFileSync(plan, "utf8")).toBe("agent's v2\n"); + expect(readdirSync(folder).filter((name) => name.includes(".ovis-"))).toEqual([]); + }); + + it("refuses every file that is not under the pane's folder, however it is named", async () => { + const secret = join(outside, "secret.txt"); + writeFileSync(secret, "secret\n"); + mkdirSync(join(outside, "docs"), { recursive: true }); + writeFileSync(join(outside, "docs", "x.md"), "x\n"); + symlinkSync(secret, join(folder, "link.txt")); + symlinkSync(join(outside, "docs"), join(folder, "linked-docs")); + file(".git/config", "[core]\n"); + for (const path of ["../outside/secret.txt", secret, "link.txt", "linked-docs/x.md", ".git/config", join(folder, ".git", "config")]) { + expect([path, ...await error(await open(path))]).toEqual([path, 403, "outside_pane"]); + expect([path, ...await error(await save(path, sha("secret\n"), "pwned\n"))]).toEqual([path, 403, "outside_pane"]); + } + expect(readFileSync(secret, "utf8")).toBe("secret\n"); + expect(readFileSync(join(outside, "docs", "x.md"), "utf8")).toBe("x\n"); + // a pane at a filesystem's root opens nothing for editing + paneCwd = "/"; + expect(await error(await open(secret.slice(1)))).toEqual([403, "outside_pane"]); + paneCwd = folder; + expect(await error(await open("missing.md"))).toEqual([404, "not_found"]); + }); + + it("sends the save nowhere else when a folder on the way is swapped for a link to another folder", async () => { + const note = file("swap/note.md", "before\n"); + writeFileSync(join(outside, "note.md"), "outside\n"); + textSaveHooks.beforeRename = () => { + renameSync(join(folder, "swap"), join(folder, "swap-moved")); + symlinkSync(outside, join(folder, "swap")); + }; + const saved = await save(note, sha("before\n"), "after\n"); + // the rename went through the folder that was opened, wherever it now is + expect(saved.status).toBe(200); + expect(readFileSync(join(folder, "swap-moved", "note.md"), "utf8")).toBe("after\n"); + expect(readFileSync(join(outside, "note.md"), "utf8")).toBe("outside\n"); + // and a folder moved out of the pane's folder meanwhile is not written in at all + const kept = file("away/kept.md", "kept\n"); + textSaveHooks.beforeRename = () => renameSync(join(folder, "away"), join(outside, "away")); + expect(await error(await save(kept, sha("kept\n"), "after\n"))).toEqual([409, "stale_revision"]); + expect(readFileSync(join(outside, "away", "kept.md"), "utf8")).toBe("kept\n"); + expect(readdirSync(join(outside, "away"))).toEqual(["kept.md"]); + }); + + it("refuses a binary, non-UTF-8, mixed-endings or oversized file, and a read-only or hard-linked one", async () => { + const late = new Uint8Array(9000).fill(0x61); + late[8999] = 0; + expect(await error(await open(file("late-nul.log", late)))).toEqual([415, "not_text"]); + expect(await error(await open(file("latin1.txt", new Uint8Array([0x63, 0x61, 0x66, 0xe9]))))).toEqual([415, "not_text"]); + expect(await error(await open(file("mixed.txt", "a\r\nb\n")))).toEqual([415, "mixed_line_endings"]); + expect(await error(await open(file("lone-cr.txt", "a\rb")))).toEqual([415, "mixed_line_endings"]); + expect(await error(await open(file("big.txt", "a".repeat(MAX_EDITABLE_TEXT_BYTES + 1))))).toEqual([413, "too_large"]); + const exact = file("exact.txt", "a".repeat(MAX_EDITABLE_TEXT_BYTES)); + expect(await error(await save(exact, sha("a".repeat(MAX_EDITABLE_TEXT_BYTES)), "b".repeat(MAX_EDITABLE_TEXT_BYTES + 1)))).toEqual([413, "too_large"]); + const locked = file("locked.md", "locked\n", 0o444); + expect(await error(await save(locked, sha("locked\n"), "x\n"))).toEqual([403, "not_writable"]); + const linked = file("linked.md", "linked\n"); + linkSync(linked, join(folder, "other-name.md")); + expect(await error(await save(linked, sha("linked\n"), "x\n"))).toEqual([403, "linked_file"]); + expect(readFileSync(join(folder, "other-name.md"), "utf8")).toBe("linked\n"); + }); + + it("takes only a JSON body from this app, within its limits", async () => { + const plain = file("plain.md", "plain\n"); + expect(await error(await save(plain, sha("plain\n"), "x\n", { "content-type": "text/plain" }))).toEqual([415, "invalid_content_type"]); + expect(await error(await save(plain, sha("plain\n"), "x\n", { origin: "https://evil.example" }))).toEqual([403, "invalid_origin"]); + expect(await error(await save(plain, sha("plain\n"), "x\r\n"))).toEqual([400, "invalid_text"]); + expect(await error(await save(plain, sha("plain\n"), "\ud800"))).toEqual([400, "invalid_text"]); + expect(await error(await save(plain, "not-a-hash", "x\n"))).toEqual([400, "invalid_revision"]); + const huge = await fetch(url("/api/fs/text"), { method: "PUT", headers: { origin: url(""), "content-type": "application/json" }, body: `"${"a".repeat(MAX_TEXT_SAVE_BODY_BYTES)}"` }); + expect(await error(huge)).toEqual([413, "too_large"]); + expect(await error(await fetch(url(`/api/fs/text?path=${q(plain)}`)))).toEqual([400, "missing_pane"]); + expect(await error(await fetch(url("/api/fs/text"), { method: "POST", headers: { origin: url("") } }))).toEqual([400, "method_not_allowed"]); + expect(readFileSync(plain, "utf8")).toBe("plain\n"); + }); + + it("saves nothing on a platform that cannot prove where the file lies", async () => { + const note = file("win.md", "win\n"); + textSaveHooks.platform = "win32"; + expect(await error(await save(note, sha("win\n"), "x\n"))).toEqual([501, "unsupported_platform"]); + expect(readFileSync(note, "utf8")).toBe("win\n"); + }); + + it("is for a driving device only, over the relay and on the LAN too", async () => { + const note = file("relay.md", "relay\n"); + const request = (method: string) => new Request(`https://relay.ovis.invalid/api/fs/text?pane_id=w1:p1&path=${q(note)}`, { method, headers: { origin: "https://relay.ovis.invalid", "content-type": "application/json" }, ...(method === "PUT" ? { body: JSON.stringify({ pane_id: "w1:p1", path: note, revision: sha("relay\n"), text: "x\n" }) } : {}) }); + const through = (role: "watch" | "drive"): ServerContext => ({ + ...server.context, peer: { kind: "relay", device: { id: "0123456789abcdef", label: "Phone", role }, sessionId: "s", serverId: "0123456789abcdef0123456789abcdef", key: "" }, + requestIP: () => null, timeout: () => {}, upgrade: () => false, + }); + for (const method of ["GET", "PUT"]) expect(await error((await handleRequest(request(method), through("watch")))!)).toEqual([403, "read_only"]); + expect((await handleRequest(request("GET"), through("drive")))?.status).toBe(200); + // an open LAN install's window signed in as nobody + const lan: ServerContext = { ...server.context, requestIP: () => ({ address: "192.168.1.20" }), timeout: () => {}, upgrade: () => false }; + const fromLan = new Request(`http://192.168.1.10:${server.port}/api/fs/text?pane_id=w1:p1&path=${q(note)}`, { headers: { host: `192.168.1.10:${server.port}` } }); + expect(await error((await handleRequest(fromLan, lan))!)).toEqual([403, "drive_only"]); + expect(readFileSync(note, "utf8")).toBe("relay\n"); + }); +}); + +describe("who may edit a file", () => { + const full = (via: string, role: "drive" | "watch") => ({ level: "full", via, role }) as Access; + const none = { level: "none", reason: "token_required" } as Access; + it.each([ + ["a paired driving device", full("device", "drive"), false, false, true], + ["the access token", full("token", "drive"), false, false, true], + ["this PC's own browser", full("local", "drive"), false, false, true], + ["a driving phone over the relay", full("relay", "drive"), false, false, true], + ["a driving device, for another PC", full("device", "drive"), false, true, true], + ["a watching device", full("device", "watch"), false, false, false], + ["a Portal visitor given drive", full("portal", "drive"), false, false, false], + ["an open LAN window", full("open", "drive"), false, false, false], + ["the connection server's bridge token", none, true, false, true], + ["the bridge token, for a PC behind this one", none, true, true, false], + ["nobody", none, false, false, false], + ])("%s", (_who, access, bridge, proxied, allowed) => { + expect(textEditAllowed(access, bridge, proxied)).toBe(allowed); + }); +}); diff --git a/server/file-edit.ts b/server/file-edit.ts new file mode 100644 index 0000000000..375542c7ba --- /dev/null +++ b/server/file-edit.ts @@ -0,0 +1,224 @@ +import { createHash, randomBytes } from "node:crypto"; +import { type BigIntStats, closeSync, constants, fchmodSync, fchownSync, fstatSync, fsyncSync, openSync, readlinkSync, readSync, realpathSync, renameSync, statSync, unlinkSync, writeSync } from "node:fs"; +import { basename, dirname, isAbsolute, join, parse, relative, sep } from "node:path"; + +import { MAX_EDITABLE_TEXT_BYTES, type EditableText, type TextSaved } from "../shared/files.ts"; +import type { Access } from "./access.ts"; +import { openAuthorized, ownedFolders, release, resolveFilePath } from "./file-view.ts"; + +/** + * Editing a text file from the file viewer (#771): a Markdown note or a JSON file an agent wrote, + * changed from a phone with no terminal to open an editor in. Reading a file reaches whatever this + * server's user can read (file-view.ts); writing reaches less: one existing text file under the + * pane's folder, for a device that drives this PC (`textEditAllowed`). + * + * A save is written to a new file in the same folder, given the file's mode and group, and renamed + * over it, so a reader sees the old text or the new, never half of it. What is replaced must be the + * file that was read: its SHA-256 (`revision`) is checked once before the new file is written and + * again right before the rename, and a file changed since it was read is not written (409). On + * Linux every step after the file was proven to lie in the pane's folder goes through a descriptor + * of its folder (`/proc/self/fd/N/`), so a folder on the way swapped for a link meanwhile + * cannot send the rename anywhere else; on macOS the folders from the pane's folder down must be + * this user's own and writable by nobody else (`ownedFolders`), checked right before the rename. + * Other platforms do not save yet. All of it is synchronous: two saves on this server never + * interleave. + */ + +/** A request this route will not carry out, with the status and code it answers. */ +export class TextEditRefused extends Error { + constructor(readonly status: number, readonly code: string, message: string, readonly revision?: string) { super(message); } +} + +/** Test seams: `platform` stands in for the process's own; `beforeRename` runs between the last look and the rename. Never set outside tests. */ +export const textSaveHooks: { platform?: NodeJS.Platform; beforeRename?: () => void } = {}; + +const platform = (): NodeJS.Platform => textSaveHooks.platform ?? process.platform; + +/** + * Who may read a file to edit it and save it: a device that drives this PC, never a watching one, a + * Portal visitor (even one given drive) or a window of an open LAN install, which signed in as + * nobody. The connection server reaching this PC with its bridge token (an SSH PC) has checked its + * own caller already; that token opens only this PC's own route, never a PC behind it. + */ +export function textEditAllowed(access: Access, bridgeAuthorized: boolean, proxied: boolean): boolean { + if (access.level === "full") return access.role === "drive" && access.via !== "portal" && access.via !== "open"; + return bridgeAuthorized && !proxied; +} + +/** The request body's limit: 1 MiB of text with every character escaped as JSON may grow about six times. */ +export const MAX_TEXT_SAVE_BODY_BYTES = 8 * 1024 * 1024; + +const isGit = (segment: string): boolean => (platform() === "linux" ? segment : segment.toLowerCase()) === ".git"; + +/** Whether `path` lies below `root` (not `root` itself), with no `.git` on the way. */ +function inside(root: string, path: string): boolean { + const rel = relative(root, path); + return rel !== "" && rel !== ".." && !rel.startsWith(`..${sep}`) && !isAbsolute(rel) && !rel.split(sep).some(isGit); +} + +/** A UTF-16 surrogate without its pair, which UTF-8 cannot hold (encoding would put U+FFFD in its place). */ +const LONE_SURROGATE = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? createHash("sha256").update(bytes).digest("hex"); + +/** The pane's folder as it resolves: never a filesystem's root, and never inside `.git`. */ +function sandbox(cwd: string): string { + if (platform() !== "linux" && platform() !== "darwin") throw new TextEditRefused(501, "unsupported_platform", "Saving files from the app is not available on this PC yet"); + let root: string; + try { root = realpathSync(cwd); } catch { throw new TextEditRefused(404, "not_found", "the pane's folder is not there"); } + if (parse(root).root === root || root.split(sep).some(isGit)) throw new TextEditRefused(403, "outside_pane", "Only a file inside the pane's folder can be edited"); + return root; +} + +/** The pane's folder and the path asked for, once its real path is shown to lie inside the folder. */ +function locate(input: string, cwd: string): { root: string; requested: string } { + const root = sandbox(cwd); + const requested = resolveFilePath(input, cwd); + if (requested === null) throw new TextEditRefused(400, "missing_path", "path is required"); + let real: string; + try { real = realpathSync(requested); } catch { throw new TextEditRefused(404, "not_found", "no file at that path"); } + if (!inside(root, real)) throw new TextEditRefused(403, "outside_pane", "Only a file inside the pane's folder can be edited"); + return { root, requested }; +} + +/** Up to `limit` bytes of the file open at `fd`, from its start. */ +function readUpTo(fd: number, limit: number): Buffer { + const buffer = Buffer.alloc(limit); + let filled = 0; + while (filled < limit) { + const read = readSync(fd, buffer, filled, limit - filled, filled); + if (read === 0) break; + filled += read; + } + return buffer.subarray(0, filled); +} + +/** The file `requested` names, opened as one inside `root` (`openAuthorized`): its real path, identity and bytes. */ +function readCurrent(requested: string, root: string): { file: string; stat: BigIntStats; bytes: Buffer } { + const opened = openAuthorized(requested, [root], true); + if (opened === null) throw new TextEditRefused(404, "not_found", "no file at that path"); + try { + if (!inside(root, opened.file)) throw new TextEditRefused(403, "outside_pane", "Only a file inside the pane's folder can be edited"); + const stat = fstatSync(opened.fd, { bigint: true }); + // read one byte past the limit: a file grown since its size was looked at is still too large + const bytes = readUpTo(opened.fd, MAX_EDITABLE_TEXT_BYTES + 1); + if (bytes.length > MAX_EDITABLE_TEXT_BYTES) throw new TextEditRefused(413, "too_large", "This file is too large to edit here"); + return { file: opened.file, stat, bytes }; + } finally { release(opened.fd); } +} + +/** The text of a whole file, with LF line endings, and the endings it uses; a binary file, or one that mixes endings, is refused. */ +function decode(bytes: Uint8Array): { text: string; lineEndings: "lf" | "crlf" } { + if (bytes.includes(0)) throw new TextEditRefused(415, "not_text", "Only a text file can be edited here"); + let text: string; + // a BOM stays in the text, so it is written back + try { text = new TextDecoder("utf-8", { fatal: true, ignoreBOM: true }).decode(bytes); } catch { throw new TextEditRefused(415, "not_text", "Only a UTF-8 text file can be edited here"); } + const crlf = text.split("\r\n").length - 1; + const cr = text.split("\r").length - 1; + const lf = text.split("\n").length - 1; + // a text box turns every line ending into LF: one kind of ending is put back, a mix could not be + if (cr !== crlf || (crlf > 0 && crlf !== lf)) throw new TextEditRefused(415, "mixed_line_endings", "This file mixes line endings; edit it in the terminal"); + return crlf > 0 ? { text: text.replaceAll("\r\n", "\n"), lineEndings: "crlf" } : { text, lineEndings: "lf" }; +} + +/** GET /api/fs/text: the file `input` names in the pane's folder `cwd`, to edit (shared/files.ts). */ +export function readEditableText(input: string, cwd: string): EditableText { + const { root, requested } = locate(input, cwd); + const current = readCurrent(requested, root); + const { text, lineEndings } = decode(current.bytes); + return { path: current.file, text, revision: sha256(current.bytes), line_endings: lineEndings, size: current.bytes.length, modified: new Date(Number(current.stat.mtimeMs)).toISOString() }; +} + +/** The file at `path` opened with `flags`, refused unless it is still the file `stat` describes. */ +function openSame(path: string, flags: number, stat: BigIntStats): number { + const fd = openSync(path, flags | constants.O_NOFOLLOW | (constants.O_NONBLOCK ?? 0)); + const now = fstatSync(fd, { bigint: true }); + if (!now.isFile() || now.dev !== stat.dev || now.ino !== stat.ino) { + closeSync(fd); + throw new TextEditRefused(409, "stale_revision", "The file was replaced since it was opened"); + } + return fd; +} + +/** A permission the filesystem refused, as the 403 that says the file cannot be written here. */ +function refusedWrite(error: unknown): unknown { + const code = (error as NodeJS.ErrnoException | null)?.code; + return code === "EACCES" || code === "EPERM" || code === "EROFS" ? new TextEditRefused(403, "not_writable", "This file can't be written here") : error; +} + +/** PUT /api/fs/text: `text` in place of the file `input` names, when that file still has `revision` (shared/files.ts). */ +export function saveEditableText(input: string, cwd: string, revision: string, text: string): TextSaved { + if (!/^[0-9a-f]{64}$/.test(revision)) throw new TextEditRefused(400, "invalid_revision", "revision is the SHA-256 the file was read with"); + if (text.includes("\r") || text.includes("\0") || LONE_SURROGATE.test(text)) throw new TextEditRefused(400, "invalid_text", "text has LF line endings and no NUL or unpaired surrogate"); + const { root, requested } = locate(input, cwd); + const current = readCurrent(requested, root); + const stale = (now: Uint8Array) => new TextEditRefused(409, "stale_revision", "The file changed since it was opened", sha256(now)); + if (sha256(current.bytes) !== revision) throw stale(current.bytes); + const { lineEndings } = decode(current.bytes); + const bytes = Buffer.from(lineEndings === "crlf" ? text.replaceAll("\n", "\r\n") : text, "utf8"); + if (bytes.length > MAX_EDITABLE_TEXT_BYTES) throw new TextEditRefused(413, "too_large", "The text is too large to save here"); + // a rename gives the path a new file: one with other links, or another owner, would not be that file afterwards + if (current.stat.nlink > 1n) throw new TextEditRefused(403, "linked_file", "This file has other links; edit it in the terminal"); + const uid = process.getuid?.(); + if (uid === undefined || current.stat.uid !== BigInt(uid)) throw new TextEditRefused(403, "not_owner", "This file belongs to another user"); + + const folder = dirname(current.file); + const name = basename(current.file); + let folderFd: number | null = null; + let temp: string | null = null; + let anchor = folder; + try { + if (platform() === "linux") { + folderFd = openSync(folder, constants.O_RDONLY | constants.O_DIRECTORY | constants.O_NOFOLLOW); + anchor = `/proc/self/fd/${folderFd}`; + // the folder opened is the one the proven file lies in + if (readlinkSync(anchor) !== folder) throw new TextEditRefused(409, "stale_revision", "The file was moved since it was opened"); + } + // the file itself must be writable: opening it to write (without truncating) changes nothing + closeSync(openSame(join(anchor, name), constants.O_WRONLY, current.stat)); + temp = `.${name}.ovis-${randomBytes(6).toString("hex")}.tmp`; + const fd = openSync(join(anchor, temp), constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | constants.O_NOFOLLOW, 0o600); + try { + fchmodSync(fd, Number(current.stat.mode & 0o7777n)); + if (BigInt(fstatSync(fd).gid) !== current.stat.gid) fchownSync(fd, uid, Number(current.stat.gid)); + for (let written = 0; written < bytes.length;) written += writeSync(fd, bytes, written, bytes.length - written); + fsyncSync(fd); + } finally { closeSync(fd); } + textSaveHooks.beforeRename?.(); + // the last look: the folder still inside the pane's folder, and the same file, unchanged + const proven = platform() === "linux" ? inside(root, join(readlinkSync(anchor), name)) : ownedFolders(root, current.file, uid) && realpathSync(folder) === folder; + if (!proven) throw new TextEditRefused(409, "stale_revision", "The file was moved since it was opened"); + const check = openSame(join(anchor, name), constants.O_RDONLY, current.stat); + let now: Buffer; + try { now = readUpTo(check, MAX_EDITABLE_TEXT_BYTES + 1); } finally { closeSync(check); } + if (sha256(now) !== revision) throw stale(now); + renameSync(join(anchor, temp), join(anchor, name)); + temp = null; + if (folderFd !== null) fsyncSync(folderFd); + const saved = statSync(join(anchor, name)); + return { path: current.file, revision: sha256(bytes), size: saved.size, modified: saved.mtime.toISOString() }; + } catch (error) { + throw refusedWrite(error); + } finally { + if (temp !== null) try { unlinkSync(join(anchor, temp)); } catch { /* never created, or already gone */ } + if (folderFd !== null) closeSync(folderFd); + } +} + +/** A request body as text, refused past `limit` bytes whether or not it said its length. */ +export async function readCappedText(request: Request, limit: number): Promise { + const tooLarge = () => new TextEditRefused(413, "too_large", "The text is too large to save here"); + if (Number(request.headers.get("content-length") ?? 0) > limit) throw tooLarge(); + const chunks: Uint8Array[] = []; + let total = 0; + const reader = request.body?.getReader(); + if (reader === undefined) return ""; + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + total += value.byteLength; + if (total > limit) { await reader.cancel(); throw tooLarge(); } + chunks.push(value); + } + return Buffer.concat(chunks).toString("utf8"); +} diff --git a/server/file-routes.ts b/server/file-routes.ts index ac78bc0a2e..6ceb7b285f 100644 --- a/server/file-routes.ts +++ b/server/file-routes.ts @@ -1,5 +1,6 @@ /** - * The file routes: what a file is and its bytes (GET/HEAD /api/fs/stat, /api/fs/file), a folder's + * The file routes: what a file is and its bytes (GET/HEAD /api/fs/stat, /api/fs/file), a text file + * to edit and its save (GET/PUT /api/fs/text, file-edit.ts), a folder's * project icon (/api/fs/favicon) and a rendered page's own media (/api/fs/page-media/:media_id). * The dispatcher in index.ts runs the access gate first and hands over who asks: a page's media * plays only for the client and device that rendered it (file-view.ts). @@ -11,9 +12,10 @@ import type { Access } from "./access.ts"; import { isSecureRequest } from "./auth.ts"; import { claudeSessionTempDir, claudeTempBases } from "./claude-store.ts"; import { claudePaneSession } from "./conversation.ts"; +import { MAX_TEXT_SAVE_BODY_BYTES, readCappedText, readEditableText, saveEditableText, TextEditRefused } from "./file-edit.ts"; import { fileResponse, findFile, crossSiteFileRead, pageMediaResponse, pageMediaSource, PAGE_MEDIA_PATH, watchPageMedia } from "./file-view.ts"; import { paneGet } from "./herdr/client.ts"; -import { badRequest, errorResponse, jsonResponse, setRequestRoute } from "./http.ts"; +import { badRequest, errorResponse, isJsonObject, jsonResponse, setRequestRoute } from "./http.ts"; import { paneContext, resolveAgent } from "./pane-context.ts"; import { findProjectFavicon } from "./project-favicon.ts"; @@ -46,6 +48,17 @@ export async function handleFileRequest(request: Request, url: URL, pathname: st try { return await fileRoute(request, url, pathname, { bridgeAuthorized, requestHost, forwarded, client, access }, watch); } catch (error) { return errorResponse(error); } finally { watch.done(); } } + // a text file read to edit and saved (file-edit.ts); who may is checked by the dispatcher (textEditAllowed) + if (pathname === "/api/fs/text") { + setRequestRoute(pathname); + if (request.method !== "GET" && request.method !== "PUT") return badRequest("method_not_allowed", "use GET or PUT"); + if (crossSiteFileRead(request)) return jsonResponse({ error: { code: "cross_site", message: "A file is edited from this app only" } }, 403); + try { return await textRoute(request, url); } catch (error) { + if (error instanceof TextEditRefused) return jsonResponse({ error: { code: error.code, message: error.message, ...(error.revision ? { revision: error.revision } : {}) } }, error.status); + return errorResponse(error); + } + } + // a rendered page's own video, by the address its render gave it: the page has no origin and // no cookie, so the address is all it holds (requiresAuth lets this path through) if (pathname.startsWith(PAGE_MEDIA_PATH)) { @@ -57,6 +70,27 @@ export async function handleFileRequest(request: Request, url: URL, pathname: st return null; } +/** GET and PUT /api/fs/text (shared/files.ts): the pane's folder is the one a file may be edited in. */ +async function textRoute(request: Request, url: URL): Promise { + if (request.method === "GET") { + const paneId = url.searchParams.get("pane_id") ?? ""; + if (paneId === "") return badRequest("missing_pane", "pane_id is required"); + return jsonResponse(readEditableText(url.searchParams.get("path") ?? "", (await paneContext(paneId)).cwd)); + } + // JSON only: a form or a text/plain body from another page is refused before it is read + if (request.headers.get("content-type")?.split(";")[0]?.trim().toLowerCase() !== "application/json") return jsonResponse({ error: { code: "invalid_content_type", message: "send application/json" } }, 415); + let body: unknown; + try { body = JSON.parse(await readCappedText(request, MAX_TEXT_SAVE_BODY_BYTES)); } catch (error) { + if (error instanceof TextEditRefused) throw error; + return badRequest("invalid_json", "request body must be JSON"); + } + if (!isJsonObject(body)) return badRequest("invalid_body", "request body must be a JSON object"); + const { pane_id: paneId, path, revision, text } = body; + if (typeof paneId !== "string" || paneId === "") return badRequest("missing_pane", "pane_id is required"); + if (typeof path !== "string" || typeof revision !== "string" || typeof text !== "string") return badRequest("invalid_body", "path, revision and text are strings"); + return jsonResponse(saveEditableText(path, (await paneContext(paneId)).cwd, revision, text)); +} + /** GET/HEAD /api/fs/stat, /api/fs/file and /api/fs/favicon, past the method and site checks. */ async function fileRoute(request: Request, url: URL, pathname: string, { bridgeAuthorized, requestHost, forwarded, client, access }: FileRequester, watch: ReturnType): Promise { // a folder's project icon: every place it may be kept, in one answer that a miss is part of diff --git a/server/file-view.ts b/server/file-view.ts index a56993ad3f..4476a75bb0 100644 --- a/server/file-view.ts +++ b/server/file-view.ts @@ -687,7 +687,7 @@ function pageOpenedAs(fd: number, file: string, opened: BigIntStats): boolean { } /** Every folder from `root` to `file`'s own is a folder (not a link), owned by `uid`, and writable by no group or other user. */ -function ownedFolders(root: string, file: string, uid: number | undefined): boolean { +export function ownedFolders(root: string, file: string, uid: number | undefined): boolean { if (uid === undefined) return false; const base = root.endsWith(sep) ? root.slice(0, -1) : root; const below = dirname(file).slice(base.length).split(sep).filter(Boolean); diff --git a/server/machine-api.ts b/server/machine-api.ts index 3b1e933938..a5d33ec013 100644 --- a/server/machine-api.ts +++ b/server/machine-api.ts @@ -99,7 +99,7 @@ export const slowMachinePath = (path: string): boolean => path === "worktree/cre * forwards, so such a path must be no route at all rather than "some other route". */ export const nonCanonicalPath = (pathname: string): boolean => pathname.includes("//") || (pathname.length > 1 && pathname.endsWith("/")) || /%2f|%5c/i.test(pathname); -export const MACHINE_PROXY_PATH = /^(?:session|sessions\/(?:history|resume)|agents|agent-catalog(?:\/(?:install|sign-in))?|integrations|system|usage\/statusline|pane\/(?:read|scroll|find|selection|conversation(?:\/image|\/tool-output)?|commands|files|omo-tasks|background-agent|plan|artifacts|btw|prompt|prompt\/answer|input|keys|close|rename|image|split|zoom|swap|move|resize|clear|focus)|workspace\/(?:create|rename|move|close|directories|diff\/files|diff\/file|diff\/refs|pr|pr\/log|pr\/merge)|worktree\/(?:create|list|open|remove)|tab\/(?:create|rename|close|move|move-to-workspace)|layout\/ratio|agent\/(?:rename|commands)|plugins\/actions|plugin\/action|fs\/(?:stat|file|favicon))$/; +export const MACHINE_PROXY_PATH = /^(?:session|sessions\/(?:history|resume)|agents|agent-catalog(?:\/(?:install|sign-in))?|integrations|system|usage\/statusline|pane\/(?:read|scroll|find|selection|conversation(?:\/image|\/tool-output)?|commands|files|omo-tasks|background-agent|plan|artifacts|btw|prompt|prompt\/answer|input|keys|close|rename|image|split|zoom|swap|move|resize|clear|focus)|workspace\/(?:create|rename|move|close|directories|diff\/files|diff\/file|diff\/refs|pr|pr\/log|pr\/merge)|worktree\/(?:create|list|open|remove)|tab\/(?:create|rename|close|move|move-to-workspace)|layout\/ratio|agent\/(?:rename|commands)|plugins\/actions|plugin\/action|fs\/(?:stat|file|favicon|text))$/; /** diff --git a/server/machine-proxy.test.ts b/server/machine-proxy.test.ts index f9e75375c9..ee52fa341b 100644 --- a/server/machine-proxy.test.ts +++ b/server/machine-proxy.test.ts @@ -13,6 +13,7 @@ const commandsAsked: string[] = []; const filesAsked: string[] = []; const worktreesAsked: string[] = []; const faviconAsked: { search: string; authorization: string | null; cookie: string | null }[] = []; +const textAsked: { method: string; body: string; authorization: string | null; contentType: string | null }[] = []; const diffAsked: { path: string; search: string; authorization: string | null; cookie: string | null; forwarded: string | null }[] = []; const remote = Bun.serve({ port: 0, @@ -40,6 +41,12 @@ const remote = Bun.serve({ worktreesAsked.push(path); return Response.json({ source: { repo_key: "repo" }, worktrees: [{ path: "/work-wt/SECRET", branch: "SECRET", open_workspace_id: null }] }); } + if (path === "/api/fs/text") { + return request.text().then((body) => { + textAsked.push({ method: request.method, body, authorization: request.headers.get("authorization"), contentType: request.headers.get("content-type") }); + return Response.json({ error: { code: "stale_revision", message: "changed", revision: "b".repeat(64) } }, { status: 409 }); + }); + } if (path === "/api/fs/favicon") { faviconAsked.push({ search: new URL(request.url).search, authorization: request.headers.get("authorization"), cookie: request.headers.get("cookie") }); return Response.json({ icon: null }); @@ -121,6 +128,20 @@ it("never asks a PC for a pane's or a folder's command rows for a watching devic expect(commandsAsked).toEqual(["/api/pane/commands"]); }); +it("carries a save to a PC's file with its body and the PC's credential, and its conflict back, but never for a watching device (#771)", async () => { + const body = JSON.stringify({ pane_id: "w1:p1", path: "/work/notes.md", revision: "a".repeat(64), text: "new\n" }); + const put = (readOnly: boolean) => handleMachineRequest(new Request("http://127.0.0.1/api/machines/pc1/fs/text", { + method: "PUT", body, headers: { origin: "http://127.0.0.1", "x-herdr-machine": "1", "content-type": "application/json" }, + }), manager, undefined, undefined, readOnly); + const watched = await put(true); + expect([watched.status, ((await watched.json()) as { error: { code: string } }).error.code]).toEqual([403, "read_only"]); + expect(textAsked).toEqual([]); + const driven = await put(false); + expect(driven.status).toBe(409); + expect(await driven.json()).toMatchObject({ error: { code: "stale_revision", revision: "b".repeat(64) } }); + expect(textAsked).toEqual([{ method: "PUT", body, authorization: "Bearer remote-token", contentType: "application/json" }]); +}); + it("passes on a PC's 503 with its Retry-After, so the menu knows to ask again (#2254)", async () => { const busy = await handleMachineRequest(new Request("http://127.0.0.1/api/machines/pc1/pane/commands?pane_id=busy"), manager); expect(busy.status).toBe(503); diff --git a/shared/files.ts b/shared/files.ts index bec999374e..652a9c6472 100644 --- a/shared/files.ts +++ b/shared/files.ts @@ -16,6 +16,51 @@ export interface FileInfo { kind: FileKind; } +/** The largest text file the viewer edits and saves (`/api/fs/text`), in bytes as stored. */ +export const MAX_EDITABLE_TEXT_BYTES = 1024 * 1024; + +/** + * GET /api/fs/text?pane_id=&path=: a text file to edit, for a device that drives this PC (not a + * watching one, a Portal visitor or an open LAN window; an SSH PC's own bridge token too). Only a + * file under the pane's folder (its real path, links followed, never in `.git`) that this server's + * user owns, with one link, at most MAX_EDITABLE_TEXT_BYTES, UTF-8 with no NUL and one kind of line + * ending. 400 missing_pane / missing_path, 403 outside_pane / drive_only, 404 not_found, 413 + * too_large, 415 not_text / mixed_line_endings, 501 unsupported_platform (only Linux and macOS save). + */ +export interface EditableText { + /** the file's real path, which a save names */ + path: string; + /** with LF line endings whatever the file uses (`line_endings`) */ + text: string; + /** the SHA-256 of the file's bytes (hex), which a save must name */ + revision: string; + line_endings: "lf" | "crlf"; + size: number; + modified: string; +} + +/** + * PUT /api/fs/text, application/json: `text` (LF line endings, no CR or NUL) replaces the file that + * had `revision`, written to a new file beside it that takes its place, with the file's mode, group + * and line endings. The rules of GET, and the file must be writable (403 not_writable, linked_file, + * not_owner). 409 stale_revision (`error.revision`: the file's own now) when it changed since it + * was read: nothing is written. A change another program makes in the moment between that last + * look and the replacement is not seen, as with any editor that does not lock the file. + */ +export interface TextSaveRequest { + pane_id: string; + path: string; + revision: string; + text: string; +} + +export interface TextSaved { + path: string; + revision: string; + size: number; + modified: string; +} + /** * GET /api/fs/favicon?root=: the icon the project in folder `root` keeps, or null. The PC looks * through every place a project keeps one (`FAVICON_CANDIDATES` in server/project-favicon.ts), diff --git a/src/App.tsx b/src/App.tsx index e9f5236693..9252db26cf 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -237,6 +237,8 @@ export function App() { const { receive: receivePreferences, saveError: settingsSaveError, paused: settingsSavePaused, retrying: settingsSaveRetrying, retry: retrySettingsSave, resubscribed: preferencesResubscribed, disconnected: preferencesDisconnected, closed: preferencesClosed } = usePreferencesSync(locked === false); // labels from first messages (#2019): keyed once their pane is seen, gone with their pane, all gone with the setting off const tabLabelWatching = auth?.role === "watch"; + // a file is edited only from a browser that drives this PC, as the server checks too (server/file-edit.ts) + const fileEditable = auth?.role === "drive" && auth.via !== "portal" && auth.via !== "open"; useEffect(() => { setTabLabelLearning(settings.autoTabNames, tabLabelWatching); const settle = (): void => settleTabLabels(machines); @@ -1084,7 +1086,7 @@ export function App() { {/* last in the row: the file beside the pane (from 980px), the pane column shrinks and its terminal refits */} {viewing !== null && fileDocked && - } path={viewing.path} paneId={viewing.paneId} onClose={closeFile} onOpen={(path) => openFile({ ...viewing, path, presentation })} onKind={reportFileKind} onTogglePresentation={toggleFilePresentation} keyboardActive={!settingsOpen} /> + } path={viewing.path} paneId={viewing.paneId} onClose={closeFile} onOpen={(path) => openFile({ ...viewing, path, presentation })} onKind={reportFileKind} onTogglePresentation={toggleFilePresentation} keyboardActive={!settingsOpen} editable={fileEditable} /> } {filesOpen && filesDocked && filesDialog(true)} {webPage !== null && !(viewing !== null && fileDocked) && @@ -1167,7 +1169,7 @@ export function App() { })()} /> } {viewing !== null && !fileDocked && - openFile({ ...viewing, path, presentation })} onKind={reportFileKind} onTogglePresentation={toggleFilePresentation} keyboardActive={!settingsOpen} /> + openFile({ ...viewing, path, presentation })} onKind={reportFileKind} onTogglePresentation={toggleFilePresentation} keyboardActive={!settingsOpen} editable={fileEditable} /> } {unsentFor && } setPaletteOpen(false)} snapshot={snapshot} online={selectedMachine?.state === "connected"} selectedPaneId={selectedPaneId} diff --git a/src/components/FileViewer.css b/src/components/FileViewer.css index 4a065e9874..3633c0bb6e 100644 --- a/src/components/FileViewer.css +++ b/src/components/FileViewer.css @@ -286,6 +286,67 @@ resize: none; } +/* a text file being edited (TextFileEditor): its bar, then a text box filling the rest of the body */ +.file-editor { + flex: 1 1 auto; + align-self: stretch; + min-width: 0; + min-height: min(60vh, 28rem); + display: flex; + flex-direction: column; + gap: var(--space-2); +} + +.file-editor-bar { + flex: none; + display: flex; + flex-wrap: wrap; + align-items: center; + justify-content: flex-end; + gap: var(--space-2); +} + +.file-editor-status { + flex: 1 1 12rem; + min-width: 0; + color: var(--text-dim); + font-size: var(--fs-sm); +} + +.file-editor-alert .file-editor-status { + color: var(--notice); + font-weight: var(--fw-medium); +} + +/* a phone gives a warning a line of its own, the buttons under it */ +@media (max-width: 600px) { + .file-editor-alert .file-editor-status { flex-basis: 100%; } +} + +.file-editor-text { + flex: 1 1 auto; + min-height: 0; + width: 100%; + margin: 0; + padding: var(--space-3); + border: var(--hairline) solid var(--border); + border-radius: var(--radius-md); + background: var(--bg-input); + color: var(--text); + font-family: var(--font-mono); + font-size: calc(var(--fs-xs) * var(--file-source-scale, 1)); + line-height: var(--lh-base); + white-space: pre; + resize: none; +} + +/* a phone keeps the size iOS does not zoom the page in for, whatever the file text size */ +@media (pointer: coarse) { + .file-editor-text { font-size: max(var(--fs-input), calc(var(--fs-xs) * var(--file-source-scale, 1))); } +} + +.file-viewer-docked .file-editor { padding-inline: var(--space-3); } + /* * A rendered document reads on the viewer's own background, like a page: no panel around it. Its * size and font are the chat's (ChatView.css). The Markdown width setting keeps it to the chat's diff --git a/src/components/FileViewer.tsx b/src/components/FileViewer.tsx index 4048818513..628a2ee2cb 100644 --- a/src/components/FileViewer.tsx +++ b/src/components/FileViewer.tsx @@ -1,5 +1,5 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState, type KeyboardEvent as ReactKeyboardEvent, type ReactNode, type RefObject } from "react"; -import { Braces, Check, Code, Copy, Download, Ellipsis, ExternalLink, Link2, Maximize2, PanelRight, TriangleAlert, X } from "lucide-react"; +import { Braces, Check, Code, Copy, Download, Ellipsis, ExternalLink, Link2, Maximize2, PanelRight, Pencil, TriangleAlert, X } from "lucide-react"; import "./FileViewer.css"; import { DirectoryBrowser } from "./DirectoryBrowser.tsx"; @@ -7,10 +7,12 @@ import { FileGlyph } from "./FileGlyph.tsx"; import { useHighlightedLines } from "./HighlightedCode.tsx"; import { RenderBoundary } from "./RenderBoundary.tsx"; import { RelayRenderedPage } from "./RelayRenderedPage.tsx"; +import { TextFileEditor } from "./TextFileEditor.tsx"; import { TextFileView } from "./TextFileView.tsx"; import { MarkdownImageContext } from "./Markdown.tsx"; import type { FileInfo } from "../../shared/protocol.ts"; +import { MAX_EDITABLE_TEXT_BYTES } from "../../shared/files.ts"; import { ApiError } from "../lib/api.ts"; import { formatBytes } from "../lib/bridgeProgress.ts"; import { LOCAL_MACHINE, paneStorageId } from "../../shared/machines.ts"; @@ -29,6 +31,7 @@ import type { MarkdownBlock } from "../../shared/markdown.ts"; import { knownPreview, parsePreviewOffThread } from "../lib/markdownPreview.ts"; import { hasPreview, type TextViewMode } from "../lib/textPreview.ts"; import { useSheetSwipe } from "../lib/sheets.ts"; +import { useSettings } from "../lib/settings.ts"; import { readFilePreviewText, SOURCE_PREVIEW_LINES, type FilePreviewText } from "../lib/filePreviewText.ts"; import { readerHref } from "../lib/fileReader.ts"; import type { FilePresentation } from "../lib/filePresentation.ts"; @@ -84,6 +87,8 @@ export interface FileViewerProps { panelStart?: ReactNode; onKind?: (presentation: FilePresentation) => void; onTogglePresentation?: () => void; + /** this browser drives the PC (not a watching device, a Portal visitor or an open LAN window): with Settings → Edit text files on, a text file can be edited */ + editable?: boolean; } /** @@ -148,8 +153,14 @@ function selectSource(element: HTMLElement): void { * A file an agent wrote, opened in the browser: images, video and audio (streamed, so they * play and seek at once), PDFs, HTML pages, and the start of a text file (markdown drawn, its images read through the file endpoint). Anything can be downloaded. */ -export function FileViewer({ path: asked, paneId, onClose, onOpen, keyboardActive = true, page = false, docked = false, panelStart, onKind, onTogglePresentation }: FileViewerProps) { +export function FileViewer({ path: asked, paneId, onClose, onOpen, keyboardActive = true, page = false, docked = false, panelStart, onKind, onTogglePresentation, editable = false }: FileViewerProps) { const t = useT(); + const { settings } = useSettings(); + // the file's text in an editor (TextFileEditor), and whether it holds changes not saved, which nothing closes over but Cancel and Close + const [editing, setEditing] = useState(false); + const [editChanged, setEditChanged] = useState(false); + // a save reads the file again, for the viewer to show what is now on disk + const [saves, setSaves] = useState(0); const locale = useLocale(); const { fetchFileInfo, fileUrl, fetchDirectories } = useMachineApi(); // a remote PC's bridge reads a relative folder from the pane's folder only from its next bundle @@ -204,8 +215,9 @@ export function FileViewer({ path: asked, paneId, onClose, onOpen, keyboardActiv if (sourceRef.current) selectSource(sourceRef.current); }, [mode, loaded, source, selectable]); - useSheetSwipe(surface, page || docked ? null : onClose); + useSheetSwipe(surface, page || docked || editChanged ? null : onClose); useEffect(() => setPath(asked), [asked]); + useEffect(() => { setEditing(false); setEditChanged(false); }, [path]); useEffect(() => setSource(target.line !== null), [path, target.line]); useEffect(() => { @@ -247,7 +259,7 @@ export function FileViewer({ path: asked, paneId, onClose, onOpen, keyboardActiv setError(reason instanceof ApiError && reason.status === 404 ? "missing" : "unreadable"); }); return () => { cancelled = true; clearTimeout(deadline); abort.abort(); }; - }, [target.path, paneId, fetchFileInfo, fileUrl, fetchDirectories, remote]); + }, [target.path, paneId, fetchFileInfo, fileUrl, fetchDirectories, remote, saves]); useEffect(() => { if ((info === null && directory === null) || reportedKind.current === path) return; @@ -278,13 +290,13 @@ export function FileViewer({ path: asked, paneId, onClose, onOpen, keyboardActiv // docked, the pane beside it is live: Escape there is the terminal's, so only one inside the panel closes it /** Escape closes the viewer from anywhere in it. */ const onKey = (event: KeyboardEvent): void => { - if (event.key !== "Escape" || nativeModalOver(surface.current)) return; + if (event.key !== "Escape" || nativeModalOver(surface.current) || editChanged) return; if (docked && !surface.current?.contains(document.activeElement)) return; onClose(); }; window.addEventListener("keydown", onKey); return () => window.removeEventListener("keydown", onKey); - }, [onClose, keyboardActive, page, docked]); + }, [onClose, keyboardActive, page, docked, editChanged]); // the More menu: it takes the focus on its first item; a press outside it, Escape (before the // viewer's own) or a wider screen (where More is gone) closes it alone; Escape and a chosen item @@ -398,6 +410,8 @@ export function FileViewer({ path: asked, paneId, onClose, onOpen, keyboardActiv ].filter((note): note is string => typeof note === "string"); const noticed = notes.length > 0; const copyable = textFile && loaded !== null && !loaded.truncated; + // shown as text (an HTML page as its source), in a pane's folder the server checks it lies in, and small enough + const canEdit = editable && settings.editTextFiles && !page && paneId !== null && textFile && (!htmlPage || source) && info.size <= MAX_EDITABLE_TEXT_BYTES; const markdown = info?.kind === "text" && /\.(?:md|markdown)$/i.test(info.name); // a page of just the document, not the app over it (FileReader) const previewUrl = readerHref(machineId, shownPath + (target.line === null ? "" : `#L${target.line}`)); @@ -436,6 +450,8 @@ export function FileViewer({ path: asked, paneId, onClose, onOpen, keyboardActiv case "pdf": return