diff --git a/README.md b/README.md index af2ad9c..7b1b436 100644 --- a/README.md +++ b/README.md @@ -3,8 +3,8 @@ A local-first, permission-controlled desktop assistant for Windows. The assistant is named **JARVIS** by default; the product is **Local Agent**. -> **Status: Phase 1 complete. Phase 2, Milestone 4 (provider completion and -> streaming) in progress.** +> **Status: Phase 1 complete. Phase 2, Milestone 6 (controlled coding +> actions) in progress.** > Phase 1 delivered the hardened desktop shell, non-secret settings storage, > an audit-log foundation, the permission-policy runtime, persisted > emergency-stop state, first-run onboarding, an encrypted secret store, and @@ -33,15 +33,47 @@ assistant is named **JARVIS** by default; the product is **Local Agent**. > refused rather than sent — and reads no credential at all. A streamed > fragment is a preview, never a message: the reply committed to the > conversation is still the whole, separately validated one. `anthropic`, -> `openai`, `claude` and `gemini` remain absent. See +> `openai`, `claude` and `gemini` remain absent. Milestone 5 adds the +> **read-only coding workspace**: the user approves one project directory in a +> native picker the main process owns — the renderer cannot name a directory, +> because `workspace.select` takes no argument at all — and the application can +> then list, read, search and prepare a coding plan inside it, and nothing +> else. Every path is contained twice (lexically, then again after `realpath`, +> so a symbolic link cannot lead out); dependencies, build output and +> credential files are never opened; every listing, file and search is +> bounded; and **nothing in this milestone can modify a file**. That is +> structural rather than a policy: no action type, no IPC channel, no preload +> function and no schema can express a modification. A generated plan is +> inert — its `diff` is pinned to `null`, it always awaits explicit approval, +> and approving it unlocks nothing, because there is nothing to unlock. See > [docs/phase-2-chat-architecture.md](docs/phase-2-chat-architecture.md), > [docs/phase-2-provider-architecture.md](docs/phase-2-provider-architecture.md), -> [docs/phase-2-real-provider-architecture.md](docs/phase-2-real-provider-architecture.md) -> and +> [docs/phase-2-real-provider-architecture.md](docs/phase-2-real-provider-architecture.md), > [docs/phase-2-provider-completion.md](docs/phase-2-provider-completion.md) -> for the full design and for what remains explicitly deferred (tool/action -> execution from model output, memory, and everything else Phase 2 has not -> reached yet). +> and +> [docs/phase-2-coding-workspace.md](docs/phase-2-coding-workspace.md) +> for the full design. +> +> Milestone 6 adds **controlled coding actions** — the first three things this +> application can change outside its own data directory, each behind a native +> confirmation the renderer cannot forge or answer. It can **overwrite files** +> inside the approved project: a change is proposed once, shown as a diff, and +> then applied by _identifier_ — `workspace:apply` carries no path and no +> content, so the bytes written are necessarily the bytes that were reviewed. +> A backup is taken outside the project first, the latest change can be undone, +> and nothing can create or delete a file. It can **run one of five commands** +> (`test`, `lint`, `typecheck`, `build`, `format:check`) that the project +> itself declares — named by an enum member, never a command string, with no +> shell, no terminal, no inherited secrets, bounded time and output, and a kill +> that reaches the whole process tree and is triggered by the emergency stop. +> And it can read **Git status and diff** and create **one checkpoint commit** +> on the branch already checked out; there is no reset, checkout, branch, +> push or remote anywhere in the codebase, and a repository's own hooks are +> disabled for every invocation. See +> [docs/phase-2-coding-actions.md](docs/phase-2-coding-actions.md) for the +> design, the bounds, and what remains explicitly deferred (file creation and +> deletion, arbitrary commands, dependency installation, browser and Windows +> automation, memory, and everything else Phase 2 has not reached yet). All rights reserved. No licence has been granted for this project. @@ -83,12 +115,23 @@ postponed. - The preload bridge exposes only narrow, explicitly named, typed functions — `localAgent.health`, `localAgent.settings.{get,update}`, `localAgent.secrets.{status,write,clear}`, - `localAgent.chat.{send,cancel,onChunk}` — never `ipcRenderer` itself, never - a generic invoke-any-channel function, and never a generic + `localAgent.chat.{send,cancel,onChunk}`, + `localAgent.workspace.{status,select,tree,file,search,plan,propose,apply,rollback,changes}`, + `localAgent.command.{list,run,cancel}`, + `localAgent.git.{status,diff,checkpoint}` — never `ipcRenderer` itself, + never a generic invoke-any-channel function, and never a generic listen-to-any-channel one: `chat.onChunk` subscribes to a single fixed, - one-way streaming channel whose payloads are schema-validated in the - preload before any renderer code sees them. None of them can return a - plaintext API key. + one-way streaming channel whose payloads are schema-validated in the preload + before any renderer code sees them. None of them can return a plaintext API + key. Of the ones that change something: `workspace.select` does not accept a + path, since the directory is chosen by the user in a native dialog the main + process owns; `workspace.apply` accepts only a change _identifier_, so the + bytes written are necessarily the ones already diffed and shown; + `command.run` accepts an identifier from a five-value enum, never a command + string; and `git.checkpoint` accepts no argument at all. There is no + function anywhere on the bridge that can create a file, delete one, run an + arbitrary command, or perform a destructive Git operation — asserted against + the real built bridge by an end-to-end test. - A strict Content-Security-Policy blocks remote script and network access outright; navigation, `window.open` and `` are all denied. - An action with no matching policy rule is **denied** — enforced both by the @@ -135,13 +178,27 @@ postponed. anything: there is no path from a chat message, typed, streamed or received, to `main/executor.ts` or to an action proposal. +- The coding workspace (Phase 2, Milestone 5) is **read-only, and structurally + so**. The user approves one directory through a native picker the main + process owns; the renderer cannot name a directory, and no action type, IPC + channel, preload function or schema in this milestone can express a + modification. Every renderer-supplied path is validated lexically and then + re-checked after `realpath`, so a symbolic link cannot lead outside the + approved project. Dependencies, build output and credential files are never + opened. Every listing, file read and search is bounded and reports when a + bound stopped it. A generated coding plan is inert: it always awaits explicit + approval, and approving it unlocks nothing. The approved path lives in memory + for one application run and is never written to disk. + Full detail, including known limitations, is in [docs/security-model.md](docs/security-model.md); the chat-specific design is in [docs/phase-2-chat-architecture.md](docs/phase-2-chat-architecture.md), [docs/phase-2-provider-architecture.md](docs/phase-2-provider-architecture.md), [docs/phase-2-real-provider-architecture.md](docs/phase-2-real-provider-architecture.md) and -[docs/phase-2-provider-completion.md](docs/phase-2-provider-completion.md). +[docs/phase-2-provider-completion.md](docs/phase-2-provider-completion.md); +the workspace design is in +[docs/phase-2-coding-workspace.md](docs/phase-2-coding-workspace.md). ## Where your data lives @@ -211,8 +268,14 @@ src/shared/ Pure schemas, types and constants, plus chat/ (the deterministic mock provider, the approved-provider registry with its fail-closed adapters, the composable timeout decorator, and the local-endpoint classifier that keeps - Ollama local). No I/O, no Electron, no network — safe to - import from any process, including the renderer. + Ollama local), plus workspace/ (the lexical path-safety rules, + the exclusion lists, the normalized error vocabulary, and the + pure coding-plan builder), plus agent/ (the fixed tool + registry and its capability ceiling, the profile registry with + its built-ins and fail-closed resolution, the normalized error + vocabulary, and the pure bounded-orchestration decider). No + I/O, no Electron, no network — safe to import from any + process, including the renderer. src/main/ Privileged Electron main process. Owns the BrowserWindow, the Content-Security-Policy, navigation/window-open/webview hardening, non-secret settings storage (paths.ts, @@ -231,9 +294,17 @@ src/main/ Privileged Electron main process. Owns the BrowserWindow, ollama-provider.ts — the last enforced local), the main-process provider registry that resolves them from settings and the encrypted secret store - (chat-provider-registry.ts), and the eight registered IPC - channels (ipc.ts), of which chat:chunk is the only - main-to-renderer event. + (chat-provider-registry.ts), the read-only coding workspace + (directory-picker.ts — the native project picker; + workspace-paths.ts — canonical path containment; + workspace-session.ts — the approved project, in memory only; + workspace-inspector.ts — the only module that reads a user's + file; workspace-planner.ts), agent profiles and the bounded + orchestrator (agent-profiles.ts — fail-safe, atomic storage + that never persists a built-in; agent-orchestrator.ts — the + run loop, which decides nothing about permissions), and the + registered IPC channels (ipc.ts), of which chat:chunk is the + only main-to-renderer event. src/preload/ The single contextBridge. Exposes a narrow, explicitly enumerated, typed API — never ipcRenderer, never a generic invoke-any-channel function. Bundled into one file: a @@ -246,9 +317,17 @@ src/renderer/ React interface: App.tsx gates on onboardingCompleted, provider is active — and, since Milestone 3, ipc-chat-provider.ts, the one file in this directory permitted to call window.localAgent, reaching the real - provider through chat.send/chat.cancel). No Node, no - Electron, no direct filesystem or network access anywhere - else in this directory — only the bridge at window.localAgent. + provider through chat.send/chat.cancel), and workspace/ is the + Milestone 5 read-only coding surface (Workspace.tsx, the + framework-independent workspace-controller.ts, useWorkspace.ts, + and ipc-workspace-client.ts — the second file permitted to + call window.localAgent), and agent/ is the Milestone 7 profile + and run surface (Agents.tsx, the framework-independent + agent-controller.ts, useAgent.ts, and ipc-agent-client.ts — + the third and last file permitted to call window.localAgent). + No Node, no Electron, no direct filesystem or network access + anywhere else in this directory — only the bridge at + window.localAgent. tests/unit/ Unit tests. `npm test`. tests/e2e/ Playwright + Electron smoke test against the built app. `npm run test:e2e`. diff --git a/docs/architecture.md b/docs/architecture.md index 287dbd3..d53b11b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -28,7 +28,27 @@ > (`src/renderer/Onboarding.tsx`). `main/permissions.ts`, > `main/action-pipeline.ts` and `main/executor.ts` again needed **zero code > changes** — every new channel is routed through the unmodified -> `handleActionProposal`. This document marks which parts exist today. +> `handleActionProposal`. Phase 2 Milestone 5 adds the read-only coding +> workspace (`main/directory-picker.ts`, `main/workspace-paths.ts`, +> `main/workspace-session.ts`, `main/workspace-inspector.ts`, +> `main/workspace-planner.ts`) and six more channels, under a permission +> engine, executor, pipeline and audit writer that once again have **zero +> diff**. Phase 2 Milestone 6 adds the first actions that can change something +> outside `%APPDATA%\Local-Agent`: approved file changes +> (`main/workspace-changes.ts`), a five-entry command registry run through a +> bounded process runner (`main/process-runner.ts`, +> `main/project-commands.ts`), and read-only Git plus a checkpoint commit +> (`main/git-runner.ts`) — ten more channels, four of them on the +> confirmation floor, and the permission engine, executor, pipeline and audit +> writer have **zero diff** for the fifth consecutive milestone. Phase 2 +> Milestone 7 adds agent profiles (`main/agent-profiles.ts`) and a minimal +> bounded orchestrator (`main/agent-orchestrator.ts`, over the pure +> `shared/agent/orchestration.ts`) — eight more channels and four more action +> types, none of which is a new capability: every tool a profile may name maps +> to an action type Milestones 5 and 6 already defined, so a profile can only +> ever _narrow_ what the permission policy already allowed. The permission +> engine, executor, pipeline and audit writer have **zero diff** for the sixth +> consecutive milestone. This document marks which parts exist today. --- @@ -60,36 +80,55 @@ mechanism to. With `sandbox: true`, `contextIsolation: true` and `nodeIntegration: false`, a fully compromised renderer gains only the narrow preload API: `health`, `settings.get`/`settings.update`, `secrets.status`/`secrets.write`/`secrets.clear` as of Milestone 7, -`chat.send`/`chat.cancel` as of Phase 2 Milestone 3, and `chat.onChunk` as of -Phase 2 Milestone 4 — every one of them except the last two policy-gated and -audited before it can perform a privileged action, and none of them able to -return a plaintext key. The two exceptions carry no authority of their own: -`chat.cancel` can only ask an already-authorized call to stop early, and -`chat.onChunk` only subscribes to bounded, validated preview text for a -`chat.send` the renderer itself initiated — see +`chat.send`/`chat.cancel` as of Phase 2 Milestone 3, `chat.onChunk` as of +Phase 2 Milestone 4, and +`workspace.status`/`select`/`tree`/`file`/`search`/`plan` as of Phase 2 +Milestone 5 — every one of them except `chat.cancel` and `chat.onChunk` +policy-gated and audited before it can perform a privileged action, and none +of them able to return a plaintext key. The two exceptions carry no authority +of their own: `chat.cancel` can only ask an already-authorized call to stop +early, and `chat.onChunk` only subscribes to bounded, validated preview text +for a `chat.send` the renderer itself initiated — see `docs/phase-2-real-provider-architecture.md` and `docs/phase-2-provider-completion.md`. +The six workspace functions read, and only read. None of them can name a +directory to open — `workspace.select` takes no argument at all, because the +user chooses in a native dialog the main process owns — and none of them can +write, create, delete or apply anything, because no such function exists to +call. See `docs/phase-2-coding-workspace.md`. + ## Modules -| Module | Responsibility | Must not | -| ------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- | -| `shared` **(exists)** | Zod schemas, derived types, constants. Pure data and pure functions. | Perform I/O; import Electron or Node built-ins; depend on `main`, `preload` or `renderer`. | -| `renderer` **(exists)** | All interface, including first-run onboarding. Renders state, collects input, sends requests over the bridge. | Touch Node APIs, the filesystem or `ipcRenderer`; read secrets; load remote content. | -| `preload` **(exists)** | The single bridge. Exposes an explicitly enumerated, typed API via `contextBridge`. | Expose `ipcRenderer`; provide a generic "invoke any channel" function; expose a Node primitive. | -| `main/ipc` **(exists)** | Receives every request, validates its payload and its response against a schema, and routes it through `main/action-runtime.ts`. | Execute anything itself; call `execute` or `main/secrets.ts` directly, bypassing the permission engine. | -| `main/action-runtime` **(exists)** | Loads the current policy and emergency state fresh per request and calls `handleActionProposal`. | Add a decision step of its own; cache policy or emergency state across requests. | -| `main/confirm` **(exists)** | `showNativeConfirmation` — the real `dialog.showMessageBox`, parented to the main window. | Be called from the renderer; be rendered as HTML; describe a secret in the dialog text. | -| `main/permissions` **(exists)** | Pure decision function. `(proposal, policy, emergencyState) → verdict`. | Perform I/O, show dialogs, or log. | -| `main/executor` **(exists)** | The **only** module that performs side effects. Requires a permission verdict as an argument. | Be called from `renderer` or `preload`; act without a decision; make its own policy judgements. | -| `main/action-pipeline` **(exists)** | Assembles `permissions → [confirm] → executor → audit` into one function every IPC handler calls. | Be bypassed by a handler that wires the pieces together itself. | -| `main/audit` **(exists)** | Append-only event writer with redaction and daily rotation. | Expose any update or delete function; write an unredacted secret. | -| `main/settings` **(exists)** | Loads, validates and atomically writes settings. Fails closed to safe defaults. | Store secrets; write without going through the executor. | -| `main/settings-service` **(exists)** | Reconciles `hasApiKey` against the secret store on every read and write; assembles onboarding/provider settings writes. | Accept `hasApiKey` as caller input; skip re-validating the full document before persisting. | -| `main/secrets` **(exists)** | Encrypts and decrypts via the **synchronous** `safeStorage` API (Windows DPAPI). Presence is answered from the file's shape, never by decrypting. | Return a plaintext secret across IPC; log a secret; fall back to storing plaintext. | -| `main/policy` **(exists)** | Loads and validates the permission policy file. Fails closed to `createDefaultPermissionPolicy()`. | Merge a partially-valid document; expose a write path. | -| `main/emergency` **(exists)** | Loads and atomically persists emergency-stop state; `engageEmergencyStop`/`resetEmergencyStop` are `perform` callbacks for the action pipeline. | Be bypassable by the renderer, a model, or the policy file; make its own permission decision. | -| `main/paths` **(exists)** | Single source of truth for user-data locations. | Accept a user-supplied path. | +| Module | Responsibility | Must not | +| --------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- | +| `shared` **(exists)** | Zod schemas, derived types, constants. Pure data and pure functions. | Perform I/O; import Electron or Node built-ins; depend on `main`, `preload` or `renderer`. | +| `renderer` **(exists)** | All interface, including first-run onboarding. Renders state, collects input, sends requests over the bridge. | Touch Node APIs, the filesystem or `ipcRenderer`; read secrets; load remote content. | +| `preload` **(exists)** | The single bridge. Exposes an explicitly enumerated, typed API via `contextBridge`. | Expose `ipcRenderer`; provide a generic "invoke any channel" function; expose a Node primitive. | +| `main/ipc` **(exists)** | Receives every request, validates its payload and its response against a schema, and routes it through `main/action-runtime.ts`. | Execute anything itself; call `execute` or `main/secrets.ts` directly, bypassing the permission engine. | +| `main/action-runtime` **(exists)** | Loads the current policy and emergency state fresh per request and calls `handleActionProposal`. | Add a decision step of its own; cache policy or emergency state across requests. | +| `main/confirm` **(exists)** | `showNativeConfirmation` — the real `dialog.showMessageBox`, parented to the main window. | Be called from the renderer; be rendered as HTML; describe a secret in the dialog text. | +| `main/permissions` **(exists)** | Pure decision function. `(proposal, policy, emergencyState) → verdict`. | Perform I/O, show dialogs, or log. | +| `main/executor` **(exists)** | The **only** module that performs side effects. Requires a permission verdict as an argument. | Be called from `renderer` or `preload`; act without a decision; make its own policy judgements. | +| `main/action-pipeline` **(exists)** | Assembles `permissions → [confirm] → executor → audit` into one function every IPC handler calls. | Be bypassed by a handler that wires the pieces together itself. | +| `main/audit` **(exists)** | Append-only event writer with redaction and daily rotation. | Expose any update or delete function; write an unredacted secret. | +| `main/settings` **(exists)** | Loads, validates and atomically writes settings. Fails closed to safe defaults. | Store secrets; write without going through the executor. | +| `main/settings-service` **(exists)** | Reconciles `hasApiKey` against the secret store on every read and write; assembles onboarding/provider settings writes. | Accept `hasApiKey` as caller input; skip re-validating the full document before persisting. | +| `main/secrets` **(exists)** | Encrypts and decrypts via the **synchronous** `safeStorage` API (Windows DPAPI). Presence is answered from the file's shape, never by decrypting. | Return a plaintext secret across IPC; log a secret; fall back to storing plaintext. | +| `main/policy` **(exists)** | Loads and validates the permission policy file. Fails closed to `createDefaultPermissionPolicy()`. | Merge a partially-valid document; expose a write path. | +| `main/emergency` **(exists)** | Loads and atomically persists emergency-stop state; `engageEmergencyStop`/`resetEmergencyStop` are `perform` callbacks for the action pipeline. | Be bypassable by the renderer, a model, or the policy file; make its own permission decision. | +| `main/paths` **(exists)** | Single source of truth for user-data locations. | Accept a user-supplied path. | +| `main/directory-picker` **(exists)** | `showDirectoryPicker` — the real `dialog.showOpenDialog`. Takes no path, so the user alone chooses which directory becomes readable. | Be called from the renderer; accept, suggest or remember a location; create a directory. | +| `main/workspace-paths` **(exists)** | Canonical containment: join, check, `realpath`, check again. The layer that catches a symbolic link. | Trust a lexical check alone; put a path or an `errno` into a thrown error. | +| `main/workspace-session` **(exists)** | Holds the one approved project, in memory, for one application run. | Persist the approved path; accept a project that was not canonicalised and validated. | +| `main/workspace-inspector` **(exists)** | The only module that reads a user's own file: list, read, search. Bounded in every dimension. | Write, create, rename or delete anything; log; follow a link out; read an excluded path. | +| `main/workspace-planner` **(exists)** | Gathers observations about the approved project and calls the pure plan builder. | Call a model; produce content that could be applied; read the clock. | +| `main/workspace-changes` **(exists)** | The only module that writes a user's own file. Holds proposed change sets, diffs them, backs up, applies all-or-nothing, restores. | Create or delete a project file; write without a fresh containment and hash re-check; delete a backup. | +| `main/process-runner` **(exists)** | The only module that starts a process. `shell: false`, bounded in time and output, no terminal, allowlisted environment, tree-killed. | Use a shell; build a command from a request; inherit the parent environment; leave output unread. | +| `main/project-commands` **(exists)** | Reads the project's declared scripts and runs one registry entry inside the approved root. | Run a script the registry does not name; take an argument vector from anywhere but a literal. | +| `main/git-runner` **(exists)** | Read-only `status`/`diff`, plus one checkpoint commit on the branch already checked out. Hooks disabled for every invocation. | Reset, checkout, switch, clean, delete a branch, rewrite history, or contact a remote. | +| `main/agent-profiles` **(exists)** | Loads, validates and atomically writes agent profiles. Fails closed to the built-ins. Never persists a built-in. | Store a credential; merge a partially-valid document; let a stored profile claim a built-in identifier. | +| `main/agent-orchestrator` **(exists)** | Drives one bounded run: asks the pure decider what to do next, calls the injected step executor, classifies the result. | Decide that anything is permitted; call a model; reach the filesystem or a process itself. | ## Dependency direction @@ -718,3 +757,45 @@ request — the permission decision, the audit record, `updatedAt` on a written All persisted timestamps are UTC ISO-8601 and reject a UTC offset, so records sort correctly and compare unambiguously. + +`main/workspace-session.ts` and `main/workspace-planner.ts` follow the same +convention: `adoptProjectDirectory({now})` and `createCodingPlan(project, +objective, now)` both take the timestamp from the caller, so a generated plan +is byte-for-byte reproducible in a test. `main/workspace-inspector.ts` reads no +clock at all. + +## Coding workspace (implemented, Phase 2 Milestone 5) + +| File | Describes | +| --------------------------------- | ----------------------------------------------------------------------------- | +| `src/main/directory-picker.ts` | The native project picker. Takes no path. | +| `src/main/workspace-paths.ts` | `isContainedPath`, `resolveProjectPath`, `toWorkspaceError` | +| `src/main/workspace-session.ts` | `createWorkspaceSession`, `adoptProjectDirectory`, `requireApprovedProject` | +| `src/main/workspace-inspector.ts` | `listProjectTree`, `readProjectFile`, `searchProject` | +| `src/main/workspace-planner.ts` | `gatherPlanObservations`, `createCodingPlan` | +| `src/shared/workspace/` | The pure half: path rules, exclusions, error vocabulary, the plan builder | +| `src/renderer/workspace/` | `Workspace.tsx`, `workspace-controller.ts`, `useWorkspace.ts`, the IPC client | + +**No engine, pipeline or audit code changed.** Three new action types +(`workspace.select`, `workspace.read`, `workspace.plan`) and six new channels +are routed through the same unmodified `runAction` → `handleActionProposal` +every action-backed channel already used — the fourth milestone in a row for +which that has been true. + +**The pure/impure split matches `decidePermission`'s.** What a plan _says_ is +computed by `buildCodingPlan` in `src/shared/workspace/plan.ts` — no I/O, no +clock, no model, fully testable without a filesystem. What is _true of a +project_ is measured by `main/workspace-planner.ts`, under the same bounds +every other workspace read obeys. The same split puts the lexical path rules +in `src/shared` and the `realpath` containment check in `src/main`, so both +processes apply one rule rather than two that can drift. + +**The renderer's workspace state is framework-independent**, exactly like +`ConversationController`: `WorkspaceController` holds every transition — +loading, empty, error, retry, staleness, the approval gate — and +`Workspace.tsx` holds none, which is what makes the interface's behaviour +testable in a toolchain with no jsdom. + +Full design, including why the plan is derived rather than model-generated and +why the picker takes no argument, is in +[phase-2-coding-workspace.md](phase-2-coding-workspace.md). diff --git a/docs/data-locations.md b/docs/data-locations.md index f6c14f1..0020054 100644 --- a/docs/data-locations.md +++ b/docs/data-locations.md @@ -58,6 +58,8 @@ user-supplied or renderer-supplied path. ├── settings.json non-secret settings ├── permissions\ │ └── policy.json permission policy +├── agents/ +│ └── profiles.json agent profiles (no credential, ever) ├── secrets\ │ └── secrets.enc encrypted credentials ├── logs\ @@ -68,14 +70,15 @@ user-supplied or renderer-supplied path. └── memory\ reserved, unused in Phase 1 ``` -| Path | Contains | Notes | -| ------------------------- | ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `settings.json` | Assistant name, user name, language, provider selection, `hasApiKey` flag | **[implemented, M3, wired to IPC in M7]** **Never a credential.** Strict schema; unknown keys rejected; written atomically; `hasApiKey` reconciled against `secrets.enc` on every read and write. | -| `permissions\policy.json` | Permission rules | **[implemented, M5]** Human-readable and human-editable. Cannot widen the model beyond the code-enforced floor, even if hand-edited to try. | -| `secrets\secrets.enc` | API keys | **[implemented, M7]** Encrypted with the synchronous `safeStorage` API (Windows DPAPI). Never leaves the main process in plaintext; presence, not content, is all any IPC channel reports. | -| `logs\audit\` | One JSON object per line | **[implemented, M4]** Append-only writer, called for real since M7's IPC channels. Records denials and rejected confirmations exactly like a success. | -| `state\emergency.json` | Emergency-stop state | **[implemented, M6]** Missing file on first launch means _disengaged_. Malformed or unreadable existing file means _engaged_. Written atomically; not called from the running app yet. | -| `memory\` | Reserved | Nothing is written here in Phase 1. | +| Path | Contains | Notes | +| ------------------------- | ------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `settings.json` | Assistant name, user name, language, provider selection, `hasApiKey` flag | **[implemented, M3, wired to IPC in M7]** **Never a credential.** Strict schema; unknown keys rejected; written atomically; `hasApiKey` reconciled against `secrets.enc` on every read and write. | +| `permissions\policy.json` | Permission rules | **[implemented, M5]** Human-readable and human-editable. Cannot widen the model beyond the code-enforced floor, even if hand-edited to try. | +| `agents\profiles.json` | Agent profiles | **[implemented, P2 M7]** Human-readable and human-editable. Holds user profiles only — built-ins are read from reviewed source on every load and are never written here, so a hand edit cannot redefine one. Cannot grant a permission: its decision vocabulary has no `allow`. Declares no field capable of holding a credential. | +| `secrets\secrets.enc` | API keys | **[implemented, M7]** Encrypted with the synchronous `safeStorage` API (Windows DPAPI). Never leaves the main process in plaintext; presence, not content, is all any IPC channel reports. | +| `logs\audit\` | One JSON object per line | **[implemented, M4]** Append-only writer, called for real since M7's IPC channels. Records denials and rejected confirmations exactly like a success. | +| `state\emergency.json` | Emergency-stop state | **[implemented, M6]** Missing file on first launch means _disengaged_. Malformed or unreadable existing file means _engaged_. Written atomically; not called from the running app yet. | +| `memory\` | Reserved | Nothing is written here in Phase 1. | ## Why they are separate diff --git a/docs/phase-2-agent-profiles.md b/docs/phase-2-agent-profiles.md new file mode 100644 index 0000000..806addb --- /dev/null +++ b/docs/phase-2-agent-profiles.md @@ -0,0 +1,333 @@ +# Phase 2 agent profiles and bounded orchestration + +> **Current state.** Phase 2, Milestone 7 adds **agent profiles** — local, +> validated configuration describing which of a fixed set of tools an agent +> may reach for, which part of the approved project it may look at, and how +> long, how many steps and how much output one run may consume — plus a +> **minimal orchestrator** that executes a bounded sequence of those tools. +> +> Not one of the tools is new. Every one maps to an action type Milestones 5 +> and 6 already defined, already gated by the permission engine and already +> audited. `main/permissions.ts`, `main/executor.ts`, +> `main/action-pipeline.ts`, `main/action-runtime.ts`, `main/audit.ts`, +> `main/emergency.ts`, `main/secrets.ts`, `main/settings.ts` and +> `main/policy.ts` all have **zero diff** in this milestone — the sixth +> consecutive one. +> +> Builds on [phase-2-coding-actions.md](phase-2-coding-actions.md). + +--- + +## Summary + +| Property | State | +| ------------------------------------------- | ----------------------------------------------------------------------------------------- | +| Agent profile schema | **[implemented]** — `src/shared/schemas/agent.schema.ts`, strict and bounded | +| A profile can grant a permission | **Never.** Its decision enum is `confirm \| deny`; `allow` is not a member | +| A profile can name a new capability | **Never.** `allowedTools` is a seven-value enum fixed in reviewed source | +| A profile can hold a credential | **Never.** Every object is strict and declares no such field | +| Profile storage | **[implemented]** — `main/agent-profiles.ts`, atomic, fail-safe, never merges | +| Built-in profiles | **[implemented]** — read from source on every load, never persisted | +| A stored profile can impersonate a built-in | **Never.** The store schema refuses the id and the `builtIn` claim alike | +| CRUD, enable, disable | **[implemented]** — all four write operations on the confirmation floor | +| Deleting or disabling the active profile | **[implemented]** — falls back to the most restricted built-in | +| Bounded orchestrator | **[implemented]** — `shared/agent/orchestration.ts` (pure) + `main/agent-orchestrator.ts` | +| Step, time and output ceilings | **[implemented]** — enforced in the schema _and_ at each step | +| Emergency stop halts a run in progress | **[implemented]** — re-read from disk between every step | +| Cancellation | **[implemented]** — also kills a child process the run started | +| An agent run can write a file | **Never.** No tool id exists for it, so no profile can name one | +| A model proposes the steps | **Not yet.** Deterministic; the seam is documented below | +| New action types | `agent.read`, `agent.select`, `agent.write`, `agent.run` | +| Emergency stop blocks them | **[implemented]** — none is on the exemption list | + +--- + +## The three decisions this milestone turns on + +### 1. A profile narrows; it can never grant + +This is the whole security posture of the milestone, and it is a property of +the **type**, not of a check someone remembered to write. + +`agentPermissionRuleSchema`'s `decision` field is `z.enum(['confirm', 'deny'])`. +There is no `'allow'`. A profile can therefore say "ask me first, even though +the policy would not have" or "refuse this, even though the policy would have +permitted it" — and it cannot say "permit this". The global permission policy +remains the only thing in the codebase that can produce an `allow`, and the +orchestrator takes the **stricter** of the two. + +The consequence worth stating plainly: **reading a profile tells you the +maximum an agent could do, and that maximum is never larger than what the +permission policy already allowed.** A compromised renderer that submits a +profile granting itself everything is submitting something the schema cannot +represent. + +The profile's `permissionPolicy` is applied in front of the pipeline, never +instead of it. When a profile raises a step to `confirm`, `main/ipc.ts` asks +the user in a native dialog _before_ proposing the action — and then proposes +it anyway, so the permission engine still decides, and any confirmation the +floor requires is still asked separately. + +### 2. Every tool is an action type that already existed + +``` +workspace.inspect -> workspace.read +workspace.search -> workspace.read +workspace.plan -> workspace.plan +git.status -> git.read +command.test -> command.run [confirmation floor] +command.lint -> command.run [confirmation floor] +command.typecheck -> command.run [confirmation floor] +``` + +Seven tools, four action types, none of them new. So "what can an agent do?" +has the same answer as "what could the interface already do, with the user's +approval?" — never a larger one. + +What is **absent** is the point. There is no tool for `workspace.write`, for +`workspace.rollback` or for `git.checkpoint`. An agent run in this milestone +can look at the approved project, produce an inert plan, read Git state, and +run the project's own verification scripts. It **cannot change a file, apply a +proposed change, or create a commit** — those stay exactly where Milestone 6 +put them: user-driven, one at a time, behind a native confirmation. A profile +cannot name a write tool because no such id exists to name, and +`AGENT_ALLOWED_ACTION_TYPES` states the ceiling as data so a test can assert +it rather than a reader having to re-derive it. + +The three tools that start a process map to `command.run`, which is on the +confirmation floor, and that is not weakened here: a verification step inside +an agent run raises the same native dialog, stating the same command line and +the same project script text, as a user-initiated run does. `main/ipc.ts`'s +`describeCommandRun` is shared by both call sites precisely so the two cannot +drift. + +### 3. A run is named, never described + +`agent:run` carries a run id and an objective string. It cannot carry a step +list, a tool, a path, a command or a limit — `agentRunRequestSchema` is a +`strictObject` with exactly two fields. + +Everything a run is permitted to do comes from the **stored profile**, read +from disk in the main process at the moment the run starts. A renderer can ask +for a run; it cannot widen one by describing it differently. This is the same +shape Milestone 6 used for `workspace:apply` (a change id and nothing else) +and for `command:run` (an enum member and nothing else), applied to the new +surface. + +--- + +## The orchestration loop + +``` +request (objective) + -> active profile, read from the store + -> plan buildAgentPlan(profile, objective) [pure, bounded] + -> native confirmation stating tools, scope and limits [confirmation floor] + -> for each step: + emergency stop? cancelled? project gone? -> stop + time / step / output budget exhausted? -> stop + tool in the profile allowlist? -> else stop + target inside the profile workspace scope? -> else stop + profile says deny? -> stop + profile says confirm? -> ask, then continue + -> runAction -> permissions -> [confirm] -> executor -> audit + -> verification evaluateAgentVerification(profile, steps) + -> result classifyAgentRun(stopReason, verification) +``` + +**The order of the checks is the security property.** `decideNextStep` +evaluates refusals before budgets: emergency stop, then cancellation, then the +missing project, then the disabled profile, then time, steps and output, then +the per-step allowlists. A state that satisfies more than one stop condition +reports the most authoritative one, so an engaged emergency stop is never +reported as "ran out of steps". + +**Every step is re-checked, not trusted from planning time.** The plan is +untrusted input — today because a user-editable profile shaped it, and in a +later milestone because a model proposed it — so the tool allowlist and the +workspace scope are evaluated again at execution time, on every step. + +**The emergency stop reaches a run in progress.** `isEmergencyEngaged` is +re-read from disk between every step rather than cached for the run, and a +running child process also receives the run's `AbortSignal`, so cancelling a +run kills the `npm test` it started rather than only declining to start +another one. + +### Verification means something was observed + +A verification requirement is satisfied only by its own tool completing with +`outcome: 'success'` — which, for a command, means the project's own script +ran and exited zero. A denied step, a declined confirmation, a failed command +and a step that never ran are all equally unsatisfying. Nothing here can be +satisfied by an assertion that the work was done. + +A run that reaches the end of its plan but does not satisfy its stated +verification is reported as `failed` with `verification-failed`, never as a +completion. "The steps ran" is never presented as "the work is verified". + +`passed` is vacuously true when a profile requires nothing, because "this +profile defines no verification" is a different statement from "verification +failed". + +--- + +## No model proposes these steps + +`buildAgentPlan` derives the sequence deterministically from the profile and +the objective, reusing the Milestone 5 keyword extractor. **Nothing in this +milestone calls a provider, and nothing parses model output into tool calls.** +No network request is made by an agent run. + +`AgentPlan` is the seam through which a model-proposed plan would arrive in a +later milestone. When it does, it changes nothing about the gates: every step +is already re-validated against the profile's allowlist and workspace scope at +execution time, and every step already goes through the permission engine on +its own action type. A model-proposed plan would be untrusted input entering +at exactly the point the current plan already enters as untrusted input. + +A profile's `provider` and `fallbackProviders` are validated, stored, and +resolved against the _reconciled_ settings — so a stale `hasApiKey` cannot +make an unusable provider look available — and the resolved value is recorded +on the run record for transparency about what would carry a model call. It is +recorded, not called. + +Likewise, a profile's `instructions` field is validated, bounded and stored, +and is **not** sent anywhere yet. It is never authorization: nothing reads a +permission, a tool or a path out of it, and `allowedTools` and +`approvedWorkspacePaths` are the only things consulted. + +--- + +## Bounds + +| Bound | Value | +| --------------------------- | ---------------------------------------------- | +| Profiles in the store | 32, built-ins included | +| Instruction length | 4 000 characters | +| Tools per profile | 16 (the registry itself has 7) | +| Workspace paths per profile | 16 | +| Verification requirements | 8 | +| Steps per run | 1–24, profile-chosen; default 8 | +| Duration per run | 5 s – 600 s, profile-chosen; default 120 s | +| Output per run | 1 000 – 200 000 bytes, profile-chosen | +| Step summary length | 200 characters, collapsed to one line | +| Profile store file size | 256 000 bytes, checked before the file is read | + +The step, duration and output ceilings are **ranges**, so a profile chooses a +value _inside_ the bound rather than supplying one. The orchestrator then +enforces the chosen value as well — two layers, because the profile file is +user-editable, exactly like the permission policy. + +--- + +## Storage + +`%APPDATA%\Local-Agent\agents\profiles.json`. Its own file beside the +permission policy rather than inside `settings.json`, for the reason the +policy has its own: configuration that shapes authority does not belong in the +same document as a display name. + +It mirrors `main/settings.ts` and `main/policy.ts` in detail: + +- **Loading never throws and never merges.** A missing file, an unreadable + one, an oversized one, malformed JSON, a `__proto__` key anywhere in it, or + a document the schema rejects all resolve to the same default store: no user + profiles, the most restricted built-in active. A document validates in full, + as itself, or it is discarded in full — nine good profiles and one malformed + one yields none, not nine. +- **Writing is atomic and re-validated**, with the same Windows + sharing-violation rename retry. +- **Built-ins are never persisted.** They are rebuilt from reviewed source on + every load, so editing this file cannot turn a shipped read-only profile + into a permissive one. A stored profile claiming a built-in id, or claiming + `builtIn: true`, fails validation — and `mergeAgentProfiles` drops it a + second time, for a store that reached it without passing validation. +- **User data is preserved.** Each mutation reads the current store, changes + the one profile it names, and writes the whole document back. + +Deleting or disabling the **active** profile is allowed, and falls back to the +most restricted built-in — never to "no restrictions". The active id is +resolved on every load rather than trusted, and rewritten on disk so the file +does not keep pointing at something deleted. + +--- + +## IPC + +Eight new channels. + +| Channel | Action | Request | Confirmed | +| ------------------ | -------------- | ---------------------- | --------- | +| `agent:list` | `agent.read` | none | no | +| `agent:select` | `agent.select` | `{profileId}` | no | +| `agent:create` | `agent.write` | `{profile}` | **yes** | +| `agent:update` | `agent.write` | `{profileId, profile}` | **yes** | +| `agent:delete` | `agent.write` | `{profileId}` | **yes** | +| `agent:setEnabled` | `agent.write` | `{profileId, enabled}` | **yes** | +| `agent:run` | `agent.run` | `{runId, objective}` | **yes** | +| `agent:cancel` | — | `{runId}` | n/a | + +`agent:select` is deliberately **not** on the confirmation floor. Selecting a +profile cannot widen anything — every action a run takes is still decided by +the permission engine against the same policy — and a native dialog for every +selection would train people to click through dialogs, which is its own +security problem. + +`agent:cancel` is the one channel with no permission gate, for the reason +`chat:cancel` and `command:cancel` have none: it cannot start anything, read +anything or reach anything. + +**Audit parameters carry no objective, no instructions, no path and no +content.** Only `{operation}`, a _length_ where useful, and `profileId` — a +bounded lowercase slug the schema constrains to `[a-z0-9._-]`, recorded for +the same reason `commandId` is: it is what lets the audit trail answer "which +profile authorized this". A test asserts the objective text, the instructions +and the project path never appear in any record. + +Each step inside a run writes its **own** audit record, on its own action +type, in addition to the record for the run itself. + +--- + +## Security boundary + +Everything Milestones 1–6 established holds unchanged. New in this milestone: + +- **A profile can only narrow.** No schema, channel, preload function or + storage path can express a grant. +- **An agent run cannot write, roll back, or commit.** No tool id, no action + type mapping, no preload function. Tests assert the absence at the registry, + at the schema, at the IPC layer and against the real built bridge. +- **The orchestrator is not a second permission engine.** It can stop a run + earlier than the permission engine would have; it can never let one past. +- **The emergency stop halts a run mid-flight**, not only before it starts. +- **Nothing a profile contains is authorization**, including its instructions. + +--- + +## Testing + +| Area | File | +| ----------------------------------------------------------------------------- | ----------------------------------------------- | +| Every boundary shape, and what a profile cannot express | `tests/unit/shared/agent.schema.test.ts` | +| The tool registry's capability ceiling; merging, fallback, and scope matching | `tests/unit/shared/agent-registry.test.ts` | +| Plan construction, every stop condition and their ordering, verification | `tests/unit/shared/agent-orchestration.test.ts` | +| Real filesystem: fail-safe loading, atomic writes, CRUD, safe fallback | `tests/unit/main/agent-profiles.test.ts` | +| Permission gating, confirmation wording, emergency stop, audit content | `tests/unit/main/ipc-agent.test.ts` | +| Failure wording, declining, staleness, cancellation, retry | `tests/unit/renderer/agent-controller.test.ts` | +| Bridge shape and the absent functions, against the real built app | `tests/e2e/electron-smoke.test.ts` | + +--- + +## What remains deferred + +Genuinely absent — no action type, channel, schema, tool or function for any +of it: + +- Multi-agent coordination; one run at a time, one agent per run +- A model proposing a plan or an action (the seam exists; nothing fills it) +- An agent writing a file, applying a change, or creating a commit +- Memory, embeddings, RAG, scheduling, self-updating +- Browser, email, voice, camera or screen automation +- Administrator or elevated execution +- Storing any credential in a profile diff --git a/docs/phase-2-coding-actions.md b/docs/phase-2-coding-actions.md new file mode 100644 index 0000000..c8d6365 --- /dev/null +++ b/docs/phase-2-coding-actions.md @@ -0,0 +1,379 @@ +# Phase 2 controlled coding actions + +> **Current state.** Phase 2, Milestone 6 gives the coding workspace the +> ability to **change three things**, each behind a native confirmation the +> renderer cannot forge or answer: it can overwrite files inside the approved +> project, it can run one of five named commands the project itself declares, +> and it can create one Git checkpoint commit. Everything Milestone 5 +> established about _reading_ is unchanged. +> +> Builds on [phase-2-coding-workspace.md](phase-2-coding-workspace.md). +> `main/permissions.ts`, `main/executor.ts`, `main/action-pipeline.ts`, +> `main/action-runtime.ts`, `main/audit.ts`, `main/emergency.ts`, +> `main/secrets.ts`, `main/settings.ts` and `main/policy.ts` all have **zero +> diff** in this milestone — the fifth consecutive one. + +--- + +## Summary + +| Property | State | +| ----------------------------------------- | ------------------------------------------------------------------------------------ | +| Proposed change, shown as a diff | **[implemented]** — `src/shared/workspace/diff.ts`, pure and bounded | +| Approval before writing | **[implemented]** — confirmation floor, enforced in code, not policy | +| Renderer can name what gets written | **Never.** `workspace:apply` carries a change _id_ and nothing else | +| Writes restricted to the approved project | **[implemented]** — the Milestone 5 containment layers, re-run at write time | +| Backup before every change | **[implemented]** — outside the project, under `%APPDATA%\Local-Agent\backups` | +| Rollback of the latest applied change | **[implemented]** — `workspace:rollback`, itself confirmed | +| File creation or deletion | **Never.** A change modifies files that already exist | +| Command registry | **[implemented]** — five entries, `src/shared/workspace/command-registry.ts` | +| Renderer can name a command string | **Never.** `commandId` is an enum member; every argument is a literal | +| Shell, PowerShell, `cmd`, elevation | **Absent.** `spawn` with `shell: false`, always | +| Timeout, output caps, step limit, cancel | **[implemented]** — `src/main/process-runner.ts` | +| Emergency stop kills a _running_ command | **[implemented]** — polled during the run, not only before it | +| Git status and diff | **[implemented]** — read-only | +| Git checkpoint | **[implemented]** — one commit, on the branch already checked out, after approval | +| Git reset, checkout, branch, push, remote | **Absent.** No schema, no channel, no argument vector | +| Repository hooks run | **Never.** `core.hooksPath` is redirected to an empty directory | +| New action types | `workspace.write`, `workspace.rollback`, `command.run`, `git.read`, `git.checkpoint` | +| Emergency stop blocks them | **[implemented]** — none is on the exemption list | + +--- + +## The three decisions this milestone turns on + +### 1. A change is proposed once and applied by reference + +This is the single most important shape in the milestone. + +`workspace:propose` is the only channel that carries file content. It reads +what is currently on disk, records a SHA-256 of it, builds the diff, and keeps +the whole change set **in the main process**, keyed by an id the main process +generated. It writes nothing. + +`workspace:apply` then takes **that id and nothing else** — no path, no +content, no destination. `workspaceApplyRequestSchema` is a `strictObject` +with one `uuid` field, so a request carrying a `path` or a `content` is +rejected outright rather than ignored. + +The consequence is worth stating plainly: **the bytes written are necessarily +the bytes that were diffed and shown.** A renderer that is compromised between +the diff appearing on screen and the user approving it can ask for an +already-reviewed change to be applied again; it cannot substitute a different +one, because there is no parameter through which to do so. Had `apply` taken +`{path, content}`, an approval would have meant only "the user approved +something a moment ago", which is not the same claim at all. + +Two further checks run at the moment of writing, not at proposal time: + +- **Containment, again.** Every path is resolved through + `resolveProjectPath` a second time — lexical rules, exclusion rules, join, + `realpath`, and the containment check _after_ canonicalisation. A symbolic + link planted between the proposal and the approval is refused, not followed. +- **Content, again.** The file's hash must still match what was diffed. If the + user changed it in another editor, or a build touched it, the write is + refused as `WORKSPACE_CHANGE_STALE` rather than silently discarding their + work. This is the content half of time-of-check/time-of-use, and unlike the + path half it can be closed completely. + +A change set is **all-or-nothing**: if the third of five writes fails, the two +that already succeeded are restored from the backup before the failure is +reported. A half-applied change is the one outcome a user could not reason +about. + +### 2. A command is named, never spelled + +`commandRunRequestSchema` carries `{runId, commandId}` where `commandId` is +one of five enum members. There is no field anywhere for a command string, an +argument, a working directory, an environment variable or a shell — so "no +arbitrary command strings" is a property of the type rather than a filter +applied to one. Every argument vector is a literal in +`src/shared/workspace/command-registry.ts`; the project supplies the script +_body_, never the command line. + +**The honest limit.** `npm run test` runs whatever the project's +`package.json` says it should, and a project is untrusted input in exactly the +sense `AGENTS.md` §5 means. No registry can change that — running the +project's own test command _is_ the requested feature. So the controls are +placed where they can actually work: + +- the command must be one of the five; +- the project must already declare that script, so nothing is invented; +- `command.run` is on the **confirmation floor**, which no policy edit can + downgrade, so the user approves it in a native dialog the main process owns; +- that dialog states the exact program, the exact arguments, the exact + directory, and the project's own script text — sanitized to one bounded + line, because a multi-line script must not be able to push the real question + off the screen; +- `describeScriptRisks` flags a script that chains commands, reaches the + network, deletes files, asks for elevation or runs inline code, so the + dialog can say what is unusual about this one. Advisory, never authorizing: + their absence proves nothing, because a script can do any of those through + spellings the list does not enumerate. + +This is recorded as the milestone's central limitation in +[security-model.md](security-model.md) rather than described as solved. + +### 3. Git can commit, and cannot do anything else + +Six argument vectors exist, all built in `src/shared/workspace/git.ts` from +literals: `rev-parse --is-inside-work-tree`, `rev-parse --show-toplevel`, +`rev-parse --abbrev-ref HEAD`, `rev-parse --short HEAD`, +`status --porcelain=v1 --branch --untracked-files=all`, +`diff --no-color --no-ext-diff --no-textconv --unified=3 -- `, +`add --all --`, and `commit --message --no-verify`. + +There is no `reset`, `checkout`, `switch`, `restore`, `clean`, `rm`, `branch`, +`push`, `fetch`, `pull`, `remote`, `rebase`, `merge`, `stash`, +`filter-branch`, `reflog` or `config`. `FORBIDDEN_GIT_SUBCOMMANDS` states the +list as data, and a test asserts none of them appears as a quoted literal in +either Git module — with the declaration of the list itself stripped first, or +the scan would match itself and mean nothing. + +Three refusals protect a checkpoint from costing the user anything: + +- **The project must be the repository root.** If the user approved a + _subdirectory_ of a repository, `git add --all` from there would still stage + the whole repository — files outside the directory they approved. + `--show-toplevel` is compared against the approved root, both sides + canonicalised first (a short 8.3 Windows ancestor shares no common root with + its long form, the same defect Milestone 5 found in its user-data guard). +- **HEAD must be on a branch.** A commit on a detached HEAD is trivially lost, + and a checkpoint exists precisely so that nothing is. +- **There must be something to commit**, or the checkpoint is noise in + someone's history. + +**Repository hooks never run.** A repository carries its own executable hooks +in `.git/hooks`, and a checkpoint commit would otherwise execute them — +running code out of a directory the user merely _opened_. Every invocation +therefore passes `-c core.hooksPath=`. `--no-verify` alone would not do it: it suppresses some hooks, not all +of them. A test proves this by installing a real `pre-commit` hook, confirming +with a control commit that hooks execute on the machine at all, and then +asserting the product's checkpoint does not run it. + +--- + +## No shell, ever + +`spawn` is called with `shell: false` in every case. + +Windows makes this awkward rather than impossible: `npm` is a `.cmd` script, +and Node refuses to spawn one without a shell (the fix for CVE-2024-27980). +`resolveProgram` therefore finds the real file on `PATH` itself, and a batch +file is run through the command interpreter with an explicit argument array. + +Two details there are not cosmetic: + +- **`cmd /s /c` strips the first and last quote** of everything after `/c`, so + `/c "C:\Program Files\nodejs\npm.cmd" run test` becomes the command + `C:\Program Files\nodejs\npm.cmd" run test` — split at the space, failing + with `'C:\Program' is not recognized`. The fix, and what Node's own + `shell: true` does, is to wrap the whole remainder in one further pair of + quotes. **This was a real defect, caught by a test on a machine whose Node + lives under `C:\Program Files`**, and it is now covered by a regression test + that asserts the exact string handed to the interpreter. +- **Every argument is checked against `SAFE_ARGUMENT_PATTERN` first.** A run + whose arguments are not all free of `&`, `|`, `<`, `>`, `^`, `%`, `!`, `"`, + backtick, `$`, `;` and control characters is **refused**, not escaped — + because escaping correctly for two different interpreters is a problem this + application does not need to have. In practice no argument ever fails it: + every one is a registry literal or a path this codebase built. + +--- + +## What a child process does not get + +| Withheld | Why | +| ---------------------- | ------------------------------------------------------------------------------------------------- | +| A terminal | `stdin: 'ignore'`, so a command that prompts reads end-of-file and exits rather than hanging | +| The parent environment | Only `INHERITED_ENVIRONMENT_NAMES` passes through — no `*_TOKEN`, no `*_KEY`, no `NODE_OPTIONS` | +| Unbounded time | `COMMAND_TIMEOUT_MS`; the process **tree** is killed, via `taskkill /T` on Windows | +| Unbounded output | Byte, line and line-length caps — and output past the cap is still **drained**, never left unread | +| Concurrency | One command at a time, checked and claimed in the same step so two requests cannot both pass | + +Withholding the parent environment is not incidental: a user who has exported +an API token into their shell must not hand it to a project's build script +because they asked Local Agent to run one. A test asserts a sentinel value in +the parent is absent from the child. + +Draining past the cap matters for a different reason: a child whose stdout +pipe fills up blocks forever, so output that is no longer being kept must +still be read. A test runs a process that prints twenty thousand lines with a +five-line cap and asserts it exits on its own, with its own exit code, rather +than only at its timeout. + +--- + +## The emergency stop reaches a running process + +The permission engine already refuses to _start_ an action while the stop is +engaged. That is only half of what "emergency stop" should mean, so +`runProcess` polls `isEmergencyEngaged` while the process runs and kills it if +the stop is engaged mid-run. The result is reported as `stopped`, distinct +from `failed`, so the interface never tells a user their tests failed when in +fact the run was cut short. + +--- + +## Bounds + +| Dimension | Constant | Value | +| -------------------------------- | --------------------------------- | ---------------------- | +| Files in one change set | `WORKSPACE_MAX_CHANGE_FILES` | 10 | +| Bytes of proposed content | `WORKSPACE_MAX_WRITE_BYTES` | 256,000 | +| Pending proposals held | `WORKSPACE_MAX_PENDING_CHANGES` | 5 | +| Applied change sets remembered | `WORKSPACE_MAX_APPLIED_CHANGES` | 20 | +| Diff lines, whole change set | `WORKSPACE_DIFF_MAX_LINES` | 4,000 | +| One diff line | `WORKSPACE_DIFF_MAX_LINE_LENGTH` | 500 | +| Line-alignment matrix cells | `WORKSPACE_DIFF_MAX_MATRIX_CELLS` | 1,000,000 | +| Command wall-clock time | `COMMAND_TIMEOUT_MS` | 300,000 | +| Captured output bytes / lines | `COMMAND_MAX_OUTPUT_*` | 200,000 / 2,000 | +| Concurrent commands | `COMMAND_MAX_CONCURRENT_RUNS` | 1 | +| Git time / status entries / diff | `GIT_TIMEOUT_MS`, `GIT_MAX_*` | 30,000 / 500 / 200,000 | + +The diff's matrix cap is the one worth explaining. Line alignment is quadratic +in the number of _differing_ lines, so a shared prefix and suffix are stripped +first — an ordinary one-line edit in a five-thousand-line file never +approaches the bound. Two genuinely unrelated files are the pathological case, +and past the cap the differ reports one coarse replacement and **says so** +(`coarse: true`) rather than spending the privileged process's CPU. + +--- + +## Backups + +Backups go to `%APPDATA%\Local-Agent\backups\\`, **outside the +user's project**. A backup written into the project would appear in its file +tree, in its `git status`, and eventually in one of its commits — the +application would be leaving litter in someone else's repository as a side +effect of protecting it. + +Rollback restores only the **most recent applied** change. Restoring an older +one would silently undo everything applied after it, which is not what "undo +the change I just made" means to anyone. + +**Nothing is ever deleted.** Evicting an old change set from memory leaves its +backup on disk; the user's own files are never removed by any code path in +this milestone. `main/workspace-changes.ts` imports `mkdir`, `open`, +`readFile`, `rename`, `rm` and `stat` — and `rm` is used for exactly one +thing: removing the module's own temporary file when an atomic write fails. + +--- + +## What a person actually reads before approving + +Two gates, and they are not the same thing: + +1. **The interface's gate.** `WorkspaceController.approveChange()` records + that the user has read the diff. `applyChange()` refuses without it, and it + resets for every newly proposed change set, so approval is never inherited + by a diff the user has not seen. +2. **The security control.** The native confirmation the main process owns, + built from the change set the main process is holding — never from the + request. A compromised renderer can skip the first; it cannot skip the + second. + +Diff lines are **sanitized before display**, unlike whole-file content in the +Milestone 5 viewer, and the asymmetry is deliberate: the viewer shows a file +exactly as stored because reading it is all that happens, while the diff +screen is where someone decides to _write_. Text that renders differently from +how it is stored would do the most damage there. Any alteration is reported in +`warnings`, so a sanitized diff never passes itself off as a verbatim one, and +`diffLineSchema` refuses a control character or a bidi override outright — the +schema is the check _on_ the sanitizer, not a second implementation of it. + +Declining a confirmation comes back as `outcome: 'aborted'`, which the +controller reports as an ordinary activity note rather than an error banner. +Telling someone their own deliberate refusal had failed would be its own kind +of misinformation. + +--- + +## IPC + +Ten new channels. Four can change something, and every one of those is on the +confirmation floor. + +| Channel | Action | Request | Confirmed | +| -------------------- | -------------------- | ---------------------------- | --------- | +| `workspace:propose` | `workspace.plan` | `{edits: [{path, content}]}` | no | +| `workspace:apply` | `workspace.write` | `{changeId}` | **yes** | +| `workspace:rollback` | `workspace.rollback` | `{changeId}` | **yes** | +| `workspace:changes` | `workspace.read` | none | no | +| `command:list` | `workspace.read` | none | no | +| `command:run` | `command.run` | `{runId, commandId}` | **yes** | +| `command:cancel` | — | `{runId}` | n/a | +| `git:status` | `git.read` | none | no | +| `git:diff` | `git.read` | `{path \| null}` | no | +| `git:checkpoint` | `git.checkpoint` | **none** | **yes** | + +`command:cancel` is the one channel with no permission gate, for exactly the +reason `chat:cancel` has none: it cannot start anything, read anything or +reach anything — it can only ask an already-authorized run to stop early. + +**Audit parameters carry no path, no name, no query and no content.** Only +`{operation}` and, where useful, a _count_: `{operation: 'apply', +fileCount: 1}`, `{operation: 'diff', scoped: true}`. The one exception is +`{operation: 'run', commandId: 'lint'}` — a command id is an enum member, not +user content, and recording it is what lets the audit trail answer "what did +it run". A test asserts the project path, the file paths and the proposed +content never appear in any record. + +--- + +## Security boundary + +Everything Milestones 1–5 established holds unchanged. New in this milestone: + +- **Only one module writes a project file.** `main/workspace-changes.ts`, and + it can only overwrite files that already exist. +- **Only one module starts a process.** `main/process-runner.ts`, always with + `shell: false`, always in the approved project, always bounded. +- **The renderer cannot name what is written or what is run.** A change id and + an enum member, asserted at the schema, at the IPC layer, and against the + real built bridge. +- **Nothing creates or deletes a project file.** No action type, no channel, + no preload function, no schema field. Tests assert the absence at each + layer. +- **Repository hooks and diff filters are disabled** for every Git + invocation. +- **The child process inherits neither a terminal nor the parent's secrets.** + +--- + +## Testing + +| Area | File | +| -------------------------------------------------------------------------------------- | ------------------------------------------------------ | +| Diff correctness, sanitization, the coarse fallback and every bound | `tests/unit/shared/workspace-diff.test.ts` | +| The registry, what is absent from it, and script-risk flags | `tests/unit/shared/workspace-command-registry.test.ts` | +| Git argument vectors, forbidden subcommands, porcelain parsing | `tests/unit/shared/workspace-git.test.ts` | +| Every boundary shape, and what cannot be expressed | `tests/unit/shared/coding.schema.test.ts` | +| Propose/apply/rollback against a real filesystem: containment, symlinks, stale, backup | `tests/unit/main/workspace-changes.test.ts` | +| Real processes: timeout, output caps, draining, cancel, emergency kill, environment | `tests/unit/main/process-runner.test.ts` | +| `package.json` as untrusted input; running a real `npm` script | `tests/unit/main/project-commands.test.ts` | +| A real repository: status, diff, checkpoint, refusals, **hooks do not run** | `tests/unit/main/git-runner.test.ts` | +| Permission gating, confirmation, emergency stop, audit content | `tests/unit/main/ipc-coding.test.ts` | +| The approval gate, declining, cancellation, retry | `tests/unit/renderer/workspace-controller.test.ts` | +| Bridge shape and the absent functions, against the real built app | `tests/e2e/electron-smoke.test.ts` | + +The symbolic-link and Git cases run against real links and real repositories, +and report a skip rather than a false pass when a platform refuses. Both were +genuinely exercised on the development machine. + +--- + +## What remains deferred + +Genuinely absent — no action type, channel, schema or function for any of it: + +- Creating or deleting a file; applying an externally supplied patch +- An arbitrary command, a shell, PowerShell, `cmd`, or elevated execution +- Dependency installation +- Any Git operation that resets, checks out, switches, cleans, deletes a + branch, rewrites history, or contacts a remote +- Browser automation, Windows automation, screen capture, camera, email +- Memory, embeddings, RAG, workflows, multi-agent execution +- A model that writes the proposed content (the change still comes from the + interface; the schema seam exists and nothing uses it) diff --git a/docs/phase-2-coding-workspace.md b/docs/phase-2-coding-workspace.md new file mode 100644 index 0000000..07183bf --- /dev/null +++ b/docs/phase-2-coding-workspace.md @@ -0,0 +1,365 @@ +# Phase 2 coding workspace + +> **Current state.** Phase 2, Milestone 5 adds a **read-only** coding +> workspace: the user approves one project directory in a native picker, and +> the application can list, read, search and reason about what is inside it — +> and nothing else. There is no write, no patch, no terminal, no Git command +> and no shell. That absence is structural, not a policy: no action type, no +> IPC channel, no preload function and no schema in this milestone can express +> a modification. +> +> Builds on [phase-2-chat-architecture.md](phase-2-chat-architecture.md), +> [phase-2-provider-architecture.md](phase-2-provider-architecture.md), +> [phase-2-real-provider-architecture.md](phase-2-real-provider-architecture.md) +> and [phase-2-provider-completion.md](phase-2-provider-completion.md), none of +> which changed. `main/permissions.ts`, `main/executor.ts`, +> `main/action-pipeline.ts`, `main/action-runtime.ts`, `main/audit.ts`, +> `main/emergency.ts`, `main/secrets.ts`, `main/settings.ts` and +> `main/policy.ts` all have **zero diff** in this milestone. + +--- + +## Summary + +| Property | State | +| --------------------------------------- | -------------------------------------------------------------------------------------------------- | +| Project selection | **[implemented]** — native `dialog.showOpenDialog`, owned by the main process | +| Renderer can name a directory to open | **Never.** `workspace:select` takes no arguments at all | +| Read-only listing, viewing, searching | **[implemented]** — `src/main/workspace-inspector.ts` | +| Path containment | **[implemented]** — two independent layers, lexical then `realpath` | +| Symbolic-link escape | **[implemented]** — refused after canonicalisation; the tree walk skips links entirely | +| Credential and dependency exclusion | **[implemented]** — `src/shared/workspace/exclusions.ts` | +| Coding plan | **[implemented]** — deterministic, derived from observation; **no model call** | +| Plan can be applied | **Never.** `diff` is pinned to `null`, `status` to `'awaiting-approval'`, and nothing consumes one | +| New action types | **[implemented]** — `workspace.select`, `workspace.read`, `workspace.plan` | +| Emergency stop blocks workspace actions | **[implemented]** — none is on the exemption list | +| Approved project persisted to disk | **Never.** In memory, for the lifetime of one application run | +| File writing, terminal, Git, shell | **Absent.** See [What remains deferred](#what-remains-deferred) | + +--- + +## The decision this milestone turns on + +A read-only file browser sounds like a small feature. It is not: it is the +first time this application reads a file the user's own machine did not put +under `%APPDATA%\Local-Agent\`, and the first time a directory tree written by +someone else becomes input. `AGENTS.md` §5 already classes such content as +untrusted, in the same category as model output. + +So the design question was never "how do we read files" but "what, exactly, is +the smallest thing that can be read, and who decides". Three answers shape +everything below. + +### 1. The user chooses the directory, in a dialog the renderer cannot reach + +`workspace:select` takes **no arguments**. Not a path it validates — none at +all. The main process opens `dialog.showOpenDialog` (`main/directory-picker.ts`), +the user clicks, and whatever comes back is the only candidate. + +This is the same reasoning that makes `main/confirm.ts`'s confirmation dialog +native rather than HTML: a renderer, or anything that has compromised one, can +ask that the user be asked, and nothing more. It cannot pre-fill the dialog, +suppress it, answer it, or point it anywhere. + +It is also why `workspace.select` is `allow` by default rather than being +added to the confirmation floor. A second, HTML-triggered confirmation before a +native dialog would be ceremony: the picker **is** the consent, and it is a +stronger form of consent than `chat.send`'s (which this codebase already +accepted as sufficient, on the weaker grounds that the user typed and pressed +Send in the renderer). + +### 2. Two containment layers, because neither is sufficient alone + +Every path the renderer sends is _relative_ to the approved root, and passes +through both: + +| Layer | Where | Catches | +| --------------------------------------- | ------------------------------------- | ------------------------------------------------------------------------------------------------- | +| Lexical (pure, no filesystem) | `src/shared/workspace/path-safety.ts` | `..`, `.`, absolute paths, UNC, drive letters, backslashes, NTFS streams, device names, bidi, NUL | +| Canonical (`realpath`, then re-checked) | `src/main/workspace-paths.ts` | Symbolic links leading outside the project — invisible to any lexical rule | + +A lexical rule cannot see a link: `src/notes` may be perfectly well-formed and +still point at `C:\Users\me\.ssh`. A `realpath` check cannot run on a string +that has not been joined onto a root yet — and joining is exactly the step an +absolute path or a `..` segment subverts. So the order is: validate lexically, +refuse excluded segments, join, check containment, canonicalise, **check +containment again**. + +The canonical form is deliberately narrow — `/`-separated, relative, no `.` or +`..`, no `\`, no `:`, no wildcard, no trailing dot or space, no Windows device +name. Every path the renderer ever sends was produced by this codebase's own +listing in exactly that form, so refusing everything else costs nothing. + +Three of those rules are worth naming because they are not obvious: + +- **`\` is refused.** Without it, `..\..\Windows` is a single segment that is + neither `.` nor `..`, so it passes a `/`-oriented segment check — while + `path.resolve` still treats the backslashes as separators on Windows. (Lint + caught a collapsed escape here during development, which is exactly the class + of bug the second containment layer exists to survive.) +- **`:` is refused**, which blocks a drive letter and, less obviously, + `notes.txt:hidden` — an NTFS alternate data stream, a different file that no + listing would ever have shown. +- **A trailing dot or space is refused.** Windows silently strips both, so + `secrets.` and `secrets` name the same file: a second spelling around every + name-based rule. + +### 3. The plan is derived, not generated + +`workspace:plan` produces its plan **deterministically**, from what was +observed about the project. It calls no model. Two reasons: + +- A model-written plan is untrusted text that would then have to be parsed back + into a structure — new attack surface, in the exact place this milestone is + supposed to be establishing that model output is data and never instruction, + and for no gain while nothing can act on a plan anyway. +- It would make the workspace unusable whenever `modelProvider.provider` is + `none`, which is the default — so the approval gate could not be demonstrated + or reviewed without first configuring a provider. + +Every risk and assumption a plan states is _observed_: emitted because a +specific fact about the project was, or was not, found. "No test tooling was +detected" appears only when no test marker exists at the root. A later +milestone that wants a model to elaborate on a plan passes this structure to it +and validates what comes back against `codingPlanSchema` — the seam is the +schema, and it already exists. + +--- + +## Modules + +| File | Responsibility | +| ------------------------------------------------ | -------------------------------------------------------------------------------------- | +| `src/shared/workspace/path-safety.ts` | Pure lexical path validation. The first containment layer. | +| `src/shared/workspace/exclusions.ts` | What is never descended into, read or searched, and why. | +| `src/shared/workspace/errors.ts` | The twelve-code normalized error vocabulary, and the fixed messages that never travel. | +| `src/shared/workspace/plan.ts` | The pure plan builder. No I/O, no clock, no model. | +| `src/shared/schemas/workspace.schema.ts` | Every shape that crosses a boundary, all bounded. | +| `src/main/directory-picker.ts` | The native picker. Takes no path. | +| `src/main/workspace-paths.ts` | Canonical containment. The second layer. | +| `src/main/workspace-session.ts` | The approved project, in memory, for this run only. | +| `src/main/workspace-inspector.ts` | The only module that reads a user's file: tree, read, search. | +| `src/main/workspace-planner.ts` | Gathers observations, calls the pure builder. | +| `src/renderer/workspace/ipc-workspace-client.ts` | The one renderer file permitted to call `window.localAgent`. | +| `src/renderer/workspace/workspace-controller.ts` | Framework-independent state: loading, empty, error, retry, approval. | +| `src/renderer/workspace/Workspace.tsx` | The interface. Holds no logic of its own. | + +--- + +## Bounds + +Nothing accumulates without a cap. Every one is a named constant in +`src/shared/constants.ts`, and every one reports itself rather than silently +shortening a result — a listing that quietly omits half a project is one a +reader would draw the wrong conclusion from. + +| Dimension | Constant | Value | +| -------------------------- | ------------------------------------- | ------------ | +| Relative path length | `WORKSPACE_MAX_RELATIVE_PATH_LENGTH` | 512 | +| Path segments | `WORKSPACE_MAX_PATH_SEGMENTS` | 32 | +| Tree depth | `WORKSPACE_MAX_TREE_DEPTH` | 8 | +| Tree entries | `WORKSPACE_MAX_TREE_ENTRIES` | 2,000 | +| Entries per directory | `WORKSPACE_MAX_DIRECTORY_ENTRIES` | 500 | +| File size (**refused**) | `WORKSPACE_MAX_FILE_BYTES` | 512,000 | +| Binary sniff sample | `WORKSPACE_BINARY_SNIFF_BYTES` | 8,192 | +| Search matches | `WORKSPACE_MAX_SEARCH_RESULTS` | 200 | +| Files opened by one search | `WORKSPACE_MAX_SEARCH_FILES` | 1,000 | +| File size searched | `WORKSPACE_SEARCH_MAX_FILE_BYTES` | 256,000 | +| Match excerpt | `WORKSPACE_SEARCH_EXCERPT_MAX_LENGTH` | 240 | +| Objective | `WORKSPACE_OBJECTIVE_MAX_LENGTH` | 2,000 | +| Plan steps / files / risks | `WORKSPACE_PLAN_MAX_*` | 12 / 20 / 12 | +| Search terms used to plan | `WORKSPACE_PLAN_MAX_SEARCH_TERMS` | 3 | + +An oversized file is **refused, not truncated** — the same reasoning the audit +log applies to an oversized record: a partial source file invites a reader to +conclude something from text that was silently cut off. + +--- + +## Exclusions + +Three lists, for three different reasons, kept separate so the rationale for +each stays readable (`src/shared/workspace/exclusions.ts`): + +- **Excluded directories** — `node_modules`, `.git`, `dist`, `build`, + `coverage`, `.venv`, `.ssh`, `.aws`, and the rest. Excluded because they are + enormous or are not the user's source. They are still **listed**, marked + `excluded`, and never descended into — hiding them would make the tree + disagree with what is on disk. +- **Credential files** — `.env*`, `.netrc`, `.npmrc`, `id_rsa`, `*.pem`, + `*.p12`, `secrets.json`. Never listed as readable, never opened, never + searched. Reading one would put a live secret into renderer memory and onto a + screen. +- **Binary extensions** — a cheap pre-filter. Never the only check: a NUL byte + in the first bytes read, and a failed strict UTF-8 decode, are what actually + decide, since an extension is only a claim a file makes about itself. + +The one subtlety worth stating: the credential-stem rule (`secrets.*`, +`credentials.*`) is **skipped for source and documentation extensions**. This +repository is its own motivating example — `src/main/secrets.ts` and +`src/shared/schemas/secrets.schema.ts` are exactly the files a reader would +want to open when inspecting how secrets are handled, and hiding them would +make the inspector actively misleading about the project it is showing. The +stem rule applies to data files; a credential pasted into a `.ts` file is not +caught, which is the same limitation the audit log's name-based redaction +already has. + +**`.git` is detected without being read.** Whether a project is +version-controlled comes from the mere existence of the entry, so the interface +learns it without anything listing or reading one byte inside. + +--- + +## IPC and the permission pipeline + +Six channels, all read-only, all routed through the unmodified +`runAction` → `handleActionProposal`: + +| Channel | Action | Request | +| ------------------ | ------------------ | ---------------------- | +| `workspace:status` | `workspace.read` | none | +| `workspace:select` | `workspace.select` | **none** | +| `workspace:tree` | `workspace.read` | `{path}` (`''` = root) | +| `workspace:file` | `workspace.read` | `{path}` | +| `workspace:search` | `workspace.read` | `{query, path}` | +| `workspace:plan` | `workspace.plan` | `{objective}` | + +Three action types rather than one, because they are three genuinely different +decisions a user might want to make separately: granting read access to a +directory tree, reading inside one already granted, and preparing the plan that +a future modification would have to pass through. All three are `allow` by +default and **none is emergency-stop exempt**, so an engaged stop denies every +one of them — asserted by a test that walks all six channels. + +**Audit parameters carry no path, no name, no query and no objective.** Only +`{operation}` and, where useful, a _length_: `{operation: 'search', +queryLength: 6}`, `{operation: 'plan', objectiveLength: 22}`. A directory path +can name a person, a client or an unreleased product; a search term is user +content. This follows `chat.send`'s own precedent of recording +`{provider, messageCount}` rather than the conversation, and a test asserts the +project path never appears in any record. + +--- + +## What the interface shows, and how it stays safe + +`src/renderer/workspace/Workspace.tsx` renders the project summary, a file +tree, a search box, a read-only viewer, file metadata, and the plan. Loading, +empty, error and retry states all come from `WorkspaceController`, which is +framework-independent and fully unit-tested without React or jsdom — the same +split `ConversationController` already uses, and for the same reason +([security-model.md](security-model.md) known limitation 18). + +- **Everything from the project is a plain JSX text child.** File names, paths, + excerpts and file contents alike. `dangerouslySetInnerHTML` appears nowhere. +- **No main-process message is ever displayed.** Failures cross as a bounded + `WorkspaceErrorCode`; the sentence a person reads is one of the controller's + own reviewed strings. A denial gets its own message naming the emergency stop + and the permission policy, rather than being folded into "could not read". +- **Retry is offered only where retrying could help.** An excluded path or an + oversized file answers the same next time, so no Retry button is shown for + them. +- **Bidirectional overrides are handled asymmetrically, deliberately.** A file + name carrying one is refused outright (it could disguise an extension); a + _search excerpt_ is sanitized (it is already a lossy fragment, and a results + list is where a reordered line would be most convincing); a _file's contents_ + are shown exactly as stored, with a warning banner — refusing would make + Arabic, Hebrew and Persian source unopenable, and rewriting it would mean the + viewer lied about what is on disk. +- **The read-only marker is the loudest thing in the header**, and the plan's + approval control says plainly that approving unlocks nothing. + +--- + +## The approval gate + +`WorkspaceController.approvePlan()` sets a flag and does nothing else. There is +no modification function for it to unlock, in that module or anywhere else in +this milestone. + +That is the point. The requirement — explicit user approval before any future +modification — is implemented as a real, tested state transition rather than a +promise, so whichever milestone adds a modification has a gate already in place +to route through rather than one to invent. Approval resets for every newly +generated plan, so it is never inherited by a plan the user has not seen, and +the interface states outright that nothing was changed. + +`codingPlanSchema` backs this at the schema level: `diff` is pinned to `null` +(not omitted), `approvalRequired` to `true`, `status` to `'awaiting-approval'`, +and `changeType` admits only `'modify'` and `'review'` — never `'create'` or +`'delete'`. A later edit that starts producing a diff has to change the schema, +and therefore has to be reviewed. + +--- + +## Security boundary + +Everything Milestones 1–4 established holds unchanged. New in this milestone: + +- **All filesystem access is in `src/main`.** `src/shared/workspace/*` is bound + by the same lint purity boundary as the rest of `src/shared`; the renderer + reaches the disk only through six narrow, schema-validated channels. +- **The renderer cannot name a directory to open.** `workspace:select` has no + argument, asserted at the schema, at the IPC layer, and against the real + built bridge. +- **Nothing can express a modification.** No action type, no channel, no + preload function, no schema field. Tests assert the absence at each layer, + including against the real running application's bridge. +- **The inspector never writes and never logs.** Proven, not asserted: a test + snapshots the project before and after a full listing, read, search and plan + — including when every operation fails — and another spies on + `console.log`/`error`/`warn` across the same. +- **The approved path is never persisted.** Not to `settings.json`, not to the + secret store, not across restarts. Read access to someone's source tree is a + per-session grant made by clicking through a dialog, not a preference to be + restored silently on the next launch. +- **The application's own data directory is refused as a project**, in both + directions (inside it, or containing it). Both sides of the comparison are + canonicalised first — a short (8.3) Windows ancestor such as + `C:\Users\VUNHAT~1\…` would otherwise share no common root with its long + form and let the guard pass silently. That was a real defect, found by a test. +- **The two renderer bridge callers are named and enforced.** A source scan + asserts `window.localAgent` appears in exactly two files — + `ipc-chat-provider.ts` and `ipc-workspace-client.ts` — and nowhere under + `src/shared`. + +--- + +## Testing + +| Area | File | +| --------------------------------------------------------------------------------------------------------------- | -------------------------------------------------- | +| Lexical containment: traversal, absolute, UNC, drive letters, backslashes, NTFS streams, device names, bidi | `tests/unit/shared/workspace-path-safety.test.ts` | +| Exclusions, in both directions: what must not be read, and what must stay readable | `tests/unit/shared/workspace-exclusions.test.ts` | +| Plan building: determinism, observed risks, bounds, and the no-diff/awaiting-approval guarantees | `tests/unit/shared/workspace-plan.test.ts` | +| Every boundary shape, including the plan shapes that cannot describe a modification | `tests/unit/shared/workspace.schema.test.ts` | +| Canonical containment and **symlink escape**, against a real filesystem | `tests/unit/main/workspace-paths.test.ts` | +| Project adoption, refusals, and the in-memory session | `tests/unit/main/workspace-session.test.ts` | +| Tree, read and search against a real project; size, binary, excluded and missing handling; **no write, no log** | `tests/unit/main/workspace-inspector.test.ts` | +| Observations measured from a real project; plan changes nothing on disk | `tests/unit/main/workspace-planner.test.ts` | +| Loading, empty, error, retry, staleness, the approval gate | `tests/unit/renderer/workspace-controller.test.ts` | +| Permission gating, emergency-stop denial, request validation, audit content | `tests/unit/main/ipc.test.ts` | +| Request/response schemas and the absence of any write-shaped field | `tests/unit/shared/ipc.schema.test.ts` | +| Bridge shape, and no workspace mutator, against the real built app | `tests/e2e/electron-smoke.test.ts` | + +The symlink-escape cases create real links and detect a platform that refuses +(Windows needs Developer Mode or elevation), reporting a skip rather than a +false pass. They were genuinely exercised on the development machine. The same +escape is _also_ covered lexically, without any symlink. + +--- + +## What remains deferred + +Genuinely absent, not merely unused — there is no action type, channel, schema +or function for any of it: + +- File writing, code modification, patch application +- Terminal, PowerShell, Command Prompt, shell commands +- Git commits, branches, resets, checkouts; dependency installation +- Browser automation, Windows automation, screen capture, camera, email +- Memory, embeddings, RAG, workflows, multi-agent execution +- Administrator or elevated execution +- Unrestricted filesystem access +- A model-assisted plan (the schema seam exists; nothing uses it) +- A diff or proposed file content of any kind diff --git a/docs/security-model.md b/docs/security-model.md index fde18c0..0024760 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -23,9 +23,14 @@ > and Ollama, the latter refused unless its endpoint is a local address) and > adds bounded streaming, introducing `chat:chunk` — the first and only > main → renderer _push_ channel — while adding **no** new action type, no -> new permission decision and no new secret-store access. Controls below are -> marked **[implemented]**, **[enforced by schema]** or **[planned, milestone -> N]**. Nothing here is claimed as working before it exists. +> new permission decision and no new secret-store access. Phase 2, Milestone 5 +> adds the **read-only coding workspace** — the first time this application +> reads a file outside `%APPDATA%\Local-Agent\` — under three new action types +> (`workspace.select`, `workspace.read`, `workspace.plan`), six new channels, +> and a permission engine, executor, pipeline and audit writer that once again +> needed **zero code changes**. Controls below are marked **[implemented]**, +> **[enforced by schema]** or **[planned, milestone N]**. Nothing here is +> claimed as working before it exists. --- @@ -90,8 +95,23 @@ that is safe without a permission decision of its own. cannot declare anything else. An action with no matching rule is denied. **[enforced by schema]** A confirmation floor: `secrets.write`, -`secrets.clear`, `emergency.reset` and `app.exit` cannot be downgraded to -`allow` by editing the policy file. The schema rejects such a file. +`secrets.clear`, `emergency.reset`, `app.exit`, and — since Phase 2 +Milestone 6 — `workspace.write`, `workspace.rollback`, `command.run` and +`git.checkpoint`, plus — since Milestone 7 — `agent.write` and `agent.run`, +cannot be downgraded to `allow` by editing the policy file. The schema rejects +such a file. + +**[enforced by type]** An **agent profile can never grant a permission.** A +profile is user-editable configuration describing which of a fixed set of +tools an agent may reach for, and its own decision vocabulary is +`confirm | deny` — `allow` is not a member of the enum, so "permit this" is +not expressible in a profile at all. Every tool a profile may name maps to an +action type that already existed, so the maximum an agent can do is never +larger than what the permission policy already allowed, and the orchestrator +takes the stricter of the two decisions. A profile that raises a step to +`confirm` causes an **additional** native dialog in front of the pipeline; it +never replaces one. See +[phase-2-agent-profiles.md](phase-2-agent-profiles.md). **[enforced by schema]** An **emergency availability floor**: the policy file is user-editable, so without a floor it could remove the user's own emergency @@ -217,6 +237,107 @@ direct, per-message user action, unlike the rare, one-time changes the confirmation floor (`secrets.write`, `secrets.clear`, `emergency.reset`, `app.exit`) protects. +### Filesystem access + +**[implemented, Phase 2 Milestone 5]** Before this milestone, every path this +application touched derived from `app.getPath('appData')` and none was +user-supplied — "path traversal: not reachable" was literally true. The coding +workspace changes that, so the controls that make it safe are stated in full. + +**The renderer cannot name a directory.** `workspace:select` takes no +arguments at all — not a path it validates, none. `main/directory-picker.ts` +opens `dialog.showOpenDialog` and whatever the _user_ clicks is the only +candidate, which is then still canonicalised, confirmed to be a directory, and +refused if it is (or contains) the application's own data directory. A +compromised renderer can ask that the user be asked, and nothing more. This is +the same reasoning that makes the confirmation dialog native rather than HTML, +and it is why `workspace.select` is `allow` by default: the picker **is** the +consent, and a stronger form of it than a renderer-side click. + +**Containment is enforced twice, by two mechanisms that fail differently.** +Every renderer-supplied path is relative to the approved root and passes: + +1. `src/shared/workspace/path-safety.ts` — pure and lexical. Refuses `..`, + `.`, absolute paths, UNC prefixes, drive letters, `\` (without which + `..\..\Windows` is a single segment that passes a `/`-oriented check while + `path.resolve` still treats it as separators), `:` (a drive letter, and the + NTFS alternate-data-stream separator — `notes.txt:hidden` is a different + file no listing ever showed), wildcards, control characters, bidirectional + overrides, Windows reserved device names, and any trailing dot or space + (Windows strips both, so `secrets.` and `secrets` are the same file — a + second spelling around every name-based rule). +2. `src/main/workspace-paths.ts` — joins onto the real root, checks + containment, canonicalises with `realpath`, and **checks containment + again**. This is the check that catches a symbolic link: a lexical rule + cannot see one, and `realpath` cannot run on a string that has not been + joined onto a root yet. Verified against a real filesystem with real links. + +Containment comparison uses `path.relative`, not a string prefix: on Windows +that compares case-insensitively and normalises separators, and it does not +mistake `…\project-backup` for something inside `…\project`. + +**The tree walk never follows a link.** It keeps only entries that are a +regular file or a real directory, which excludes symbolic links (and with them +directory loops and escapes), sockets, FIFOs and device nodes. A symlinked file +can still be opened directly by path, where the canonical check proves it stays +inside; the walk simply does not offer it. + +**Excluded paths are never opened**, at any depth, and the check runs on every +segment rather than only the last — `node_modules/pkg/index.js` is refused even +though `index.js` alone would be fine. Three lists, for three reasons +(`src/shared/workspace/exclusions.ts`): dependency and build directories +(listed but never descended into, so a tree does not silently misrepresent the +project); credential files (`.env*`, `.netrc`, `.npmrc`, `id_rsa`, `*.pem`, +`*.p12`, `secrets.json` — never listed as readable, never read, never +searched); and binary extensions, as a pre-filter ahead of the real test, which +is a NUL byte in the first bytes read plus a strict UTF-8 decode. + +**Everything is bounded**, and every bound reports itself rather than silently +shortening a result: tree depth (8), entries (2,000), entries per directory +(500), file size (512,000 bytes — **refused**, not truncated, for the same +reason an oversized audit record is rejected), search matches (200), files +opened by one search (1,000), and match excerpt length (240). A search query is +matched as a **literal substring, never compiled as a regular expression** — +compiling user input as a pattern is how a search box becomes a denial of +service against the process that owns every privileged operation here. + +**Nothing in this milestone can modify a file**, and that is structural rather +than a policy: there is no `workspace.write` action type, no write channel, no +preload function, and no request schema field capable of carrying content or a +destination. `main/workspace-inspector.ts` imports exactly `readdir`, +`readFile` and `stat` from `node:fs/promises`, and that import list is the +whole of its filesystem capability. A test snapshots a project before and after +a full listing, read, search and plan — including when every operation fails — +and asserts it is unchanged; another asserts the module logs nothing at all. + +**No path, name, query or file content reaches an audit record or an error.** +Audit parameters carry `{operation}` and at most a _length_ +(`{operation: 'search', queryLength: 6}`); a directory path can name a person, +a client or an unreleased product, and a search term is user content. +Filesystem errors are mapped by `code` alone — never `message`, which contains +the full path — into a twelve-code normalized vocabulary, and the sentence a +user reads is one of the renderer's own reviewed strings, never anything that +crossed the boundary. + +**The approved path is never persisted.** It lives in memory, per +`registerIpcHandlers` call, for one application run. Read access to someone's +source tree is a grant the user makes by clicking through a dialog, not a +preference restored silently on the next launch. + +**A coding plan is inert.** `codingPlanSchema` pins `diff` to `null` (declared, +not omitted, so a later edit that starts producing one must change the schema +and therefore be reviewed), `approvalRequired` to `true`, and `status` to +`'awaiting-approval'`; `changeType` admits only `'modify'` and `'review'`. The +plan is produced deterministically from observation, with **no model call** — +see [phase-2-coding-workspace.md](phase-2-coding-workspace.md) for why. Nothing +in this milestone consumes a plan, so approving one unlocks nothing; the gate +exists so whichever milestone adds a modification has one to route through. + +**Every workspace action is denied while the emergency stop is engaged.** None +of the three is on `EMERGENCY_STOP_EXEMPT_ACTION_TYPES`, and a test walks all +six channels to confirm it — including that the native picker is never even +shown, since `execute` never reaches `perform`. + ### Secret handling **[enforced by schema]** The settings schema is strict and declares no field @@ -621,49 +742,75 @@ exposes no way to address one. No generic pass-through channel exists. ## Threats and status -| Threat | Status | -| -------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| Renderer compromise (XSS, malicious UI dependency) | Addressed, M2 — process isolation and CSP; asserted by an end-to-end test | -| Malicious or malformed IPC payload | Addressed, M2–M7 — schema validation on every registered channel (`health`, `settings:get`, `settings:update`, `secrets:status`, `secrets:write`, `secrets:clear`); no unvalidated or generic channel exists | -| Secret exfiltration through the interface | Addressed — no channel returns a key; asserted by test | -| Secret leakage into logs or errors | Addressed, M4 — schema-level redaction contract plus writer-side redaction, both verified | -| Credential persisted inside a settings _value_ | Addressed — `baseUrl` rejects embedded userinfo | -| Log injection via a crafted user name | Addressed — control characters rejected | -| Display spoofing via bidirectional overrides | Addressed — bidi overrides and isolates rejected in display strings | -| Forged audit record | Addressed — biconditional cross-field integrity rules | -| Unbounded or non-serialisable audit payload | Addressed — bounded JSON-safe parameter schema; writer-side scan bounds the whole record | -| Audit record overwritten or truncated by a write | Addressed, M4 — append-only file handle; proven under concurrent writes by test | -| Prototype-pollution key bypassing schema validation | Addressed, M4 — explicit writer-side scan; schema-only reliance was verified insufficient | -| Settings tampering or corruption | Addressed, M3 — strict validation, fail-safe loading to defaults, atomic writes | -| Policy tampering or corruption | Addressed, M5 — fail-closed loading (`main/policy.ts`) plus engine-side floor enforcement independent of it | -| Policy file removing the user's emergency controls | Addressed, M5 — availability floor enforced at both the schema layer and, independently, by `decidePermission` | -| Policy bypassing schema validation before reaching the engine | Addressed, M5 — floors re-verified by the engine regardless of validation history | -| Corruption of a shared security default in memory | Addressed — exported defaults deeply frozen; resolvers return fresh objects | -| `src/shared` reaching the OS, network or eval | Addressed — lint boundary, verified by probe; not a runtime sandbox | -| Privilege escalation via the executor | Addressed, M5 — `execute` requires a verdict as an explicit argument; proven by tests that no denial or rejection reaches `perform` | -| A side effect running without a permission decision | Addressed, M5 — structural: no code path in `execute` calls `perform` without an authorizing verdict | -| Model rationale/confidence used as authorization | Addressed, M5 — `decidePermission` never reads `proposal.parameters`; proven by test | -| Emergency-stop bypass (decision logic) | Addressed, M5 — engine denies non-exempt actions when engaged, evaluated after both floors | -| Emergency-stop bypass (persistence) | Addressed, M6 — atomic writes, fail-safe loading; reset requires an approved confirmation the pipeline already enforces | -| Emergency-stop state corruption fails open instead of closed | Addressed, M6 — malformed or unreadable state resolves engaged, not disengaged; proven by test | -| Emergency reset triggered by a model or a policy rule alone | Addressed, M6 — always resolves to `confirm`; proven across a policy × emergency-state test matrix | -| Plaintext API key stored as a fallback when encryption is unavailable | Addressed, M7 — `writeSecret` throws `SecretStoreUnavailableError` before any disk write when `safeStorage.isEncryptionAvailable()` is `false`; proven by test | -| Plaintext API key reaching settings.json, an audit record, an error message, or the renderer | Addressed, M7 — `apiKey` never enters `proposal.parameters`; secrets IPC responses carry only `{present: boolean}`; proven by test | -| `hasApiKey` reported as `true` for a provider that does not use a key (`none`, `ollama`) | Addressed, M7 — forced `false` regardless of the secret store; proven by test | -| A denied or rejected secret operation still mutating the store or `hasApiKey` | Addressed, M7 — `perform` is structurally unreachable on `deny`/`reject`; `hasApiKey` is only refreshed after the store write already succeeded; proven by test | -| Renderer-supplied `hasApiKey` reaching a persisted document | Addressed, M7 — `settingsUpdateRequestSchema` has no such field; `strictObject` rejects the extra key outright | -| Path traversal | Not reachable — all paths derive from the app-data directory; none is user-supplied | -| Prompt injection, untrusted model or tool output | Reachable from Phase 2 M3 (a real provider replies) — no tool exists for a model to invoke, so untrusted assistant text still cannot authorize or perform an action; see the proposal/executor split | -| Renderer or a compromised dependency making a network request directly | Addressed, Phase 2 M3 — CSP `connect-src 'none'` (unmodified since M2) plus the `src/shared` and `src/renderer/chat` purity/scan boundaries; only `src/main/openai-compatible-provider.ts` can reach the network | -| Plaintext API key reaching the renderer through a real provider call | Addressed, Phase 2 M3 — stays inside `resolveMainChatProvider`/`openai-compatible-provider.ts`; never in `ActionProposal.parameters`, `chatSendResponseSchema`, an audit record, or a thrown error; proven by test | -| A real provider call bypassing the permission engine or the emergency stop | Addressed, Phase 2 M3 — reached only through the unmodified `runAction`/`handleActionProposal`; denied while the emergency stop is engaged | -| Message content or a raw provider error leaking into the audit trail | Addressed, Phase 2 M3 — the `chat.send` proposal carries only `{provider, messageCount}`; every provider failure normalizes to one of five fixed codes first | -| A "local" provider silently reaching a cloud service | Addressed, Phase 2 M4 — an Ollama endpoint must be a loopback, private or link-local literal (or localhost); a name that merely looks local is refused, and a refused endpoint never falls back to the default | -| A credential stored for one provider being sent to another | Addressed, Phase 2 M4 — the secret read is skipped entirely for providers outside PROVIDERS_REQUIRING_API_KEY, and the Ollama adapter has no config field that could carry a key | -| Unbounded memory or output from a hostile streaming endpoint | Addressed, Phase 2 M4 — bytes read, line length, delta size and accumulated content are each capped independently, in the main process and again in the renderer | -| Streamed model output treated as a message, or as authorization | Addressed, Phase 2 M4 — deltas are previews held outside the message list; the committed reply is the separately validated whole, and no path exists from either to the executor | -| The renderer subscribing to arbitrary IPC events through the new push channel | Addressed, Phase 2 M4 — chat.onChunk fixes the channel in the preload, validates every payload, hands back only an unsubscribe function, and never exposes ipcRenderer or the raw event | -| Supply-chain compromise via npm | Mitigated, not eliminated — see below | +| Threat | Status | +| -------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Renderer compromise (XSS, malicious UI dependency) | Addressed, M2 — process isolation and CSP; asserted by an end-to-end test | +| Malicious or malformed IPC payload | Addressed, M2–M7 — schema validation on every registered channel (`health`, `settings:get`, `settings:update`, `secrets:status`, `secrets:write`, `secrets:clear`); no unvalidated or generic channel exists | +| Secret exfiltration through the interface | Addressed — no channel returns a key; asserted by test | +| Secret leakage into logs or errors | Addressed, M4 — schema-level redaction contract plus writer-side redaction, both verified | +| Credential persisted inside a settings _value_ | Addressed — `baseUrl` rejects embedded userinfo | +| Log injection via a crafted user name | Addressed — control characters rejected | +| Display spoofing via bidirectional overrides | Addressed — bidi overrides and isolates rejected in display strings | +| Forged audit record | Addressed — biconditional cross-field integrity rules | +| Unbounded or non-serialisable audit payload | Addressed — bounded JSON-safe parameter schema; writer-side scan bounds the whole record | +| Audit record overwritten or truncated by a write | Addressed, M4 — append-only file handle; proven under concurrent writes by test | +| Prototype-pollution key bypassing schema validation | Addressed, M4 — explicit writer-side scan; schema-only reliance was verified insufficient | +| Settings tampering or corruption | Addressed, M3 — strict validation, fail-safe loading to defaults, atomic writes | +| Policy tampering or corruption | Addressed, M5 — fail-closed loading (`main/policy.ts`) plus engine-side floor enforcement independent of it | +| Policy file removing the user's emergency controls | Addressed, M5 — availability floor enforced at both the schema layer and, independently, by `decidePermission` | +| Policy bypassing schema validation before reaching the engine | Addressed, M5 — floors re-verified by the engine regardless of validation history | +| Corruption of a shared security default in memory | Addressed — exported defaults deeply frozen; resolvers return fresh objects | +| `src/shared` reaching the OS, network or eval | Addressed — lint boundary, verified by probe; not a runtime sandbox | +| Privilege escalation via the executor | Addressed, M5 — `execute` requires a verdict as an explicit argument; proven by tests that no denial or rejection reaches `perform` | +| A side effect running without a permission decision | Addressed, M5 — structural: no code path in `execute` calls `perform` without an authorizing verdict | +| Model rationale/confidence used as authorization | Addressed, M5 — `decidePermission` never reads `proposal.parameters`; proven by test | +| Emergency-stop bypass (decision logic) | Addressed, M5 — engine denies non-exempt actions when engaged, evaluated after both floors | +| Emergency-stop bypass (persistence) | Addressed, M6 — atomic writes, fail-safe loading; reset requires an approved confirmation the pipeline already enforces | +| Emergency-stop state corruption fails open instead of closed | Addressed, M6 — malformed or unreadable state resolves engaged, not disengaged; proven by test | +| Emergency reset triggered by a model or a policy rule alone | Addressed, M6 — always resolves to `confirm`; proven across a policy × emergency-state test matrix | +| Plaintext API key stored as a fallback when encryption is unavailable | Addressed, M7 — `writeSecret` throws `SecretStoreUnavailableError` before any disk write when `safeStorage.isEncryptionAvailable()` is `false`; proven by test | +| Plaintext API key reaching settings.json, an audit record, an error message, or the renderer | Addressed, M7 — `apiKey` never enters `proposal.parameters`; secrets IPC responses carry only `{present: boolean}`; proven by test | +| `hasApiKey` reported as `true` for a provider that does not use a key (`none`, `ollama`) | Addressed, M7 — forced `false` regardless of the secret store; proven by test | +| A denied or rejected secret operation still mutating the store or `hasApiKey` | Addressed, M7 — `perform` is structurally unreachable on `deny`/`reject`; `hasApiKey` is only refreshed after the store write already succeeded; proven by test | +| Renderer-supplied `hasApiKey` reaching a persisted document | Addressed, M7 — `settingsUpdateRequestSchema` has no such field; `strictObject` rejects the extra key outright | +| Path traversal | Addressed, Phase 2 M5 — user-supplied paths exist for the first time; contained lexically and again after `realpath`, both verified against a real filesystem | +| Symbolic link leading outside the approved project | Addressed, Phase 2 M5 — refused after canonicalisation; the tree walk keeps only regular files and real directories, so it cannot follow or loop on one | +| A credential file in the user's project being read or displayed | Addressed, Phase 2 M5 — `.env*`, key, keystore and credential-stemmed data files are never listed as readable, opened or searched, at any depth | +| The renderer choosing which directory becomes readable | Addressed, Phase 2 M5 — `workspace.select` takes no argument; the user chooses in a native dialog the main process owns, which is then still canonicalised and validated | +| The application's own data directory opened as a "project" | Addressed, Phase 2 M5 — refused in both directions, with both sides canonicalised first so a short (8.3) Windows ancestor cannot defeat the comparison | +| A file modified, created or deleted by the workspace | Addressed, Phase 2 M5 — structurally absent: no action type, channel, preload function or schema field can express one; proven by before/after snapshots across success and failure | +| A project path, file content or search query reaching the audit log | Addressed, Phase 2 M5 — parameters carry `{operation}` and at most a length; asserted by test | +| Unbounded memory or time from a hostile project tree | Addressed, Phase 2 M5 — depth, entry, per-directory, file-size, match and scanned-file caps, each reporting when it stopped; search is literal substring matching, never a compiled pattern | +| Prompt injection, untrusted model or tool output | Reachable from Phase 2 M3 (a real provider replies) — no tool exists for a model to invoke, so untrusted assistant text still cannot authorize or perform an action; see the proposal/executor split | +| Renderer or a compromised dependency making a network request directly | Addressed, Phase 2 M3 — CSP `connect-src 'none'` (unmodified since M2) plus the `src/shared` and `src/renderer/chat` purity/scan boundaries; only `src/main/openai-compatible-provider.ts` can reach the network | +| Plaintext API key reaching the renderer through a real provider call | Addressed, Phase 2 M3 — stays inside `resolveMainChatProvider`/`openai-compatible-provider.ts`; never in `ActionProposal.parameters`, `chatSendResponseSchema`, an audit record, or a thrown error; proven by test | +| A real provider call bypassing the permission engine or the emergency stop | Addressed, Phase 2 M3 — reached only through the unmodified `runAction`/`handleActionProposal`; denied while the emergency stop is engaged | +| Message content or a raw provider error leaking into the audit trail | Addressed, Phase 2 M3 — the `chat.send` proposal carries only `{provider, messageCount}`; every provider failure normalizes to one of five fixed codes first | +| A "local" provider silently reaching a cloud service | Addressed, Phase 2 M4 — an Ollama endpoint must be a loopback, private or link-local literal (or localhost); a name that merely looks local is refused, and a refused endpoint never falls back to the default | +| A credential stored for one provider being sent to another | Addressed, Phase 2 M4 — the secret read is skipped entirely for providers outside PROVIDERS_REQUIRING_API_KEY, and the Ollama adapter has no config field that could carry a key | +| Unbounded memory or output from a hostile streaming endpoint | Addressed, Phase 2 M4 — bytes read, line length, delta size and accumulated content are each capped independently, in the main process and again in the renderer | +| Streamed model output treated as a message, or as authorization | Addressed, Phase 2 M4 — deltas are previews held outside the message list; the committed reply is the separately validated whole, and no path exists from either to the executor | +| The renderer subscribing to arbitrary IPC events through the new push channel | Addressed, Phase 2 M4 — chat.onChunk fixes the channel in the preload, validates every payload, hands back only an unsubscribe function, and never exposes ipcRenderer or the raw event | +| A file written outside the approved project | Addressed, Phase 2 M6 — the Milestone 5 containment layers are re-run at write time, after the diff and after the approval, so a link planted in between is refused rather than followed | +| The renderer naming what gets written, or substituting content after approval | Addressed, Phase 2 M6 — `workspace:apply` carries a change _id_ and nothing else; the bytes written are the bytes the main process already held and already diffed | +| A change silently discarding an edit the user made in another editor | Addressed, Phase 2 M6 — the file's hash must still match what was diffed, or the write is refused as `WORKSPACE_CHANGE_STALE` | +| A half-applied multi-file change | Addressed, Phase 2 M6 — a change set is all-or-nothing; files already written are restored from the backup before the failure is reported | +| A file created or deleted in the user's project | Addressed, Phase 2 M6 — structurally absent: a change modifies files that already exist, and no action type, channel, preload function or schema field can express a create or a delete | +| A backup polluting the user's project or its next commit | Addressed, Phase 2 M6 — backups are written under `%APPDATA%\Local-Agent\backups`, never inside the project | +| An arbitrary command, a shell, PowerShell or elevation reached from the renderer | Addressed, Phase 2 M6 — `commandId` is a five-value enum and every argument is a literal; `spawn` runs with `shell: false` in every case, and an argument carrying interpreter syntax is refused rather than escaped | +| A command running outside the approved project | Addressed, Phase 2 M6 — the working directory is always the approved root; there is no parameter for anything else | +| A project's build script inheriting the user's exported secrets | Addressed, Phase 2 M6 — only an allowlist of environment variables is passed through; no `*_TOKEN`, no `*_KEY`, no `NODE_OPTIONS`. Asserted by test with a sentinel value | +| A command hanging on input, printing forever, or outliving its request | Addressed, Phase 2 M6 — no terminal (`stdin: 'ignore'`), timeout, byte/line caps with continued draining so a full pipe cannot wedge the child, and a process-**tree** kill on Windows | +| A command that was already running when the emergency stop was engaged | Addressed, Phase 2 M6 — the stop is polled during the run and kills the process; reported as `stopped`, never as a test failure | +| A repository's own hooks or diff filters executing because the user opened it | Addressed, Phase 2 M6 — every `git` invocation redirects `core.hooksPath` to an empty directory this application owns, and diffs run `--no-ext-diff --no-textconv`; proven by a test with a real `pre-commit` hook | +| A destructive Git operation (reset, checkout, branch delete, push, remote) | Addressed, Phase 2 M6 — structurally absent: no schema carries a subcommand, every vector is a literal, and a source scan asserts none of the forbidden subcommands appears in either Git module | +| `git add --all` staging files outside the directory the user approved | Addressed, Phase 2 M6 — a checkpoint is refused unless `--show-toplevel` canonically equals the approved root, so a subdirectory of a repository is never checkpointed | +| A checkpoint commit that is easy to lose, or that is empty | Addressed, Phase 2 M6 — refused on a detached HEAD and refused when there is nothing uncommitted | +| A diff line that renders differently from how it is stored | Addressed, Phase 2 M6 — diff lines are sanitized before display and any alteration is reported in `warnings`; `diffLineSchema` refuses a control character or bidi override outright | +| Unbounded CPU from diffing two unrelated files | Addressed, Phase 2 M6 — a shared prefix and suffix are stripped first and the alignment matrix is capped; past the cap the differ reports one coarse replacement and says so | +| A path, file content or a project script reaching the audit log | Addressed, Phase 2 M6 — parameters carry `{operation}` and a count; the only identifier recorded is `commandId`, an enum member. Asserted by test | +| A project's `package.json` misleading a confirmation dialog | Partly addressed, Phase 2 M6 — the script text is sanitized to one bounded single line and risk-flagged, but it is still attacker-influenced text in a security prompt; see limitation 26 | +| Supply-chain compromise via npm | Mitigated, not eliminated — see below | --- @@ -671,6 +818,21 @@ exposes no way to address one. No generic pass-through channel exists. These are real and are stated plainly rather than described as solved. +0. **An agent run executes several actions after one approval.** Phase 2 + Milestone 7's `agent.run` is on the confirmation floor, so a run is + approved in a native dialog that states the profile's tools, its workspace + scope and its three ceilings before the first step — and every individual + step is still decided by the permission engine on its own action type, with + anything on the confirmation floor still prompting separately. But the + approval is nonetheless _per run_, not per read: a person approving a run + is approving a bounded sequence they did not see itemised. The bounds are + what make that acceptable — a fixed seven-tool registry containing nothing + that can write a file, apply a change or create a commit; a step ceiling; a + time ceiling; an output ceiling; and an emergency stop re-read from disk + between every step. It is a real widening of what one approval covers, and + it is recorded here rather than presented as equivalent to approving each + action. + 1. **The audit log is append-only by API, not tamper-proof.** A local user with the same privileges can edit the file directly with a text editor. Tamper-evidence (hash chaining or signing) is deferred beyond Phase 1. The @@ -842,6 +1004,116 @@ These are real and are stated plainly rather than described as solved. channels is in `tests/unit/main/ipc.test.ts`, against temporary directories, not the real application process. +20. **The workspace has a time-of-check/time-of-use gap.** Between `realpath` + returning and the subsequent `open`/`readdir`, a path component could in + principle be replaced with a symbolic link. Closing it properly needs + handle-based, `O_NOFOLLOW`-style APIs that Node does not expose portably. + The exposure is narrow — the attacker must already be able to write inside + the user's own approved project, on the user's own machine, between two + adjacent syscalls — and the consequence is bounded by everything else in + the layer: the result is still read-only, still size-capped, and still + refused if it is not text. Stated rather than described as solved. + +21. **The file viewer warns about bidirectional control characters; it does + not neutralise them.** A source file containing an override is shown + exactly as stored, with a prominent warning, because refusing would make + legitimate Arabic, Hebrew and Persian source unopenable and rewriting it + would mean the viewer lied about what is on disk. The text can therefore + still _render_ differently from how it is stored. File **names** and search + **excerpts** are handled more strictly — a name carrying one is refused + outright, and an excerpt is sanitised — because a name can disguise an + extension and a results list is where a reordered line would be most + convincing. + +22. **Credential detection in a project is name-based, like the audit log's + redaction, and inherits the same limitation.** `.env*`, known credential + file names, key and keystore extensions, and credential-stemmed _data_ + files are excluded. A credential pasted into an ordinary source file — + `src/config.ts` — is not caught, and the stem rule is deliberately skipped + for source and documentation extensions so that files _about_ credential + handling (this repository's own `src/main/secrets.ts`) stay readable. That + is the safe direction for an inspector whose job is to show a project + truthfully, but it is a real gap and is not closed by a value-level check. + +23. **A coding plan is keyword matching, not understanding.** It is derived + deterministically from bounded searches and the project's structure, with + no model call (see + [phase-2-coding-workspace.md](phase-2-coding-workspace.md) for why). It + will miss a file that is relevant without sharing vocabulary with the + request, and it says so in its own risks when nothing matched or when a + search was truncated. It is a grounded starting point, not an analysis. + +24. **The workspace bounds each operation, not the rate of operations.** One + listing, read or search is capped in every dimension, but nothing limits + how many a renderer may issue in succession. In a single-user desktop + application whose renderer is driven by the same person who approved the + project, this is acceptable; it would not be in a multi-user or remote + context, neither of which this project has. + +25. **No component-level render test for `Workspace.tsx`**, for the same reason + as limitation 18 and with the same mitigation: every state transition + (loading, empty, error, retry, staleness, the approval gate) lives in the + framework-independent `WorkspaceController` and is fully covered there. + What remains untested by an automated test is the JSX and event wiring + itself. The e2e suite likewise does not drive the six workspace channels + against the real built application — like `settings:*` and `secrets:*` + (limitation 19), they have real side effects against the developer's actual + `%APPDATA%\Local-Agent\`, and would additionally open a native dialog no + headless run can answer. Their behavioural coverage is in + `tests/unit/main/ipc.test.ts`, against temporary directories; the e2e suite + asserts only the bridge's static shape and the absence of any mutator. + +26. **Running a project's own command executes that project's code, and this + application cannot make that safe.** This is the central limitation of + Milestone 6 and it is inherent, not an oversight: `npm run test` runs + whatever the project's `package.json` says, and a project is untrusted + input in exactly the sense `AGENTS.md` §5 means. The registry restricts + _which of five named scripts_ may be started; it cannot restrict what the + script then does — it could delete files, reach the network, or read + anything the user can read. + + What is actually controlled: the command must be one of five; the project + must already declare it; `command.run` is on the confirmation floor, which + no policy edit can downgrade; the native dialog states the exact program, + arguments, directory and the project's own script text; risky-looking + scripts are flagged; the run is bounded in time and output, inherits + neither a terminal nor the parent's environment, and is killed by the + emergency stop. The residual risk is that a user approves a command in a + project whose `package.json` is hostile. The script preview is sanitized + and bounded precisely because it is attacker-influenced text appearing in + a security prompt, but a determined author can still write a plausible + one-line script that does something else. + +27. **`describeScriptRisks` is a heuristic, and its silence proves nothing.** + It flags command chaining, common network and deletion tools, elevation + and inline code by pattern. A script can do any of those through spellings + it does not enumerate. It exists to warn, never to authorize — its output + changes no decision the permission engine makes. + +28. **A Git checkpoint stages everything, including work the user had + deliberately left unstaged.** `git add --all` followed by `git commit` is + non-destructive — nothing is lost, and the content is in the commit either + way — but it does change the index, which could surprise someone who had + carefully staged a subset. The confirmation dialog states both commands + verbatim before it runs. Undoing a checkpoint is left to the user's own + tools, because every Git command that would undo one is on the forbidden + list. + +29. **Tab characters in `git diff` output are shown as spaces.** Every line + crossing to the renderer is sanitized, and a tab is a control character, + so indentation in the Git diff view is approximate for tab-indented + projects. This is a display trade-off taken deliberately rather than + loosening the rule that no control character reaches the interface; the + workspace's own diff is unaffected, since it is built from text this + application decoded itself. + +30. **The change store is per-session and in memory.** Proposals and the + applied-change history do not survive a restart, so a rollback is only + possible within the run that applied the change. The backup files + themselves do persist under `%APPDATA%\Local-Agent\backups`, and nothing + deletes them — recovering from an older one is a manual operation the user + performs with their own tools. + --- ## Reporting diff --git a/package-lock.json b/package-lock.json index 499bfdf..d7e9a69 100644 --- a/package-lock.json +++ b/package-lock.json @@ -27,7 +27,7 @@ "typescript": "6.0.3", "typescript-eslint": "8.66.0", "vite": "8.2.1", - "vitest": "4.1.10" + "vitest": "4.1.11" }, "engines": { "node": ">=20.0.0" @@ -259,9 +259,9 @@ } }, "node_modules/@jridgewell/sourcemap-codec": { - "version": "1.5.5", - "resolved": "https://registry.npmjs.org/@jridgewell/sourcemap-codec/-/sourcemap-codec-1.5.5.tgz", - "integrity": "sha512-cYQ9310grqxueWbl+WuIUIaiUaDcj7WOq5fVhEljNVgRfOUhY9fy2zTvfoqWsnebh8Sl70VScFbICvJnLKB0Og==", + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/@jridgewell/sourcemap-codec/-/sourcemap-codec-1.6.0.tgz", + "integrity": "sha512-T7jf+5zgsZHwNJ4lvQ7/aezbyk0nNX+zJVWpmHA7VYsEx7a7qr5Rg5IbtJFqkgze5Y2sruq1RUY8Q837Od7iFw==", "dev": true, "license": "MIT" }, @@ -875,16 +875,16 @@ } }, "node_modules/@vitest/expect": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-4.1.10.tgz", - "integrity": "sha512-YsCn+qAk1GWjQOWFEsEcL2gNQ0zmVmQu3T03qP6UyjhtmdtwtbuI+DASn/7iQB3HGTXkdBwGddzxPlmiql5vlA==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-4.1.11.tgz", + "integrity": "sha512-VX2x5vNJXET47KAFzwERI+KRMtTTCSWTfSMKsW7JsUsXV4psq++e3DvZpuTDOpHcxytiDs6p2nhVb2tVDiiUYw==", "dev": true, "license": "MIT", "dependencies": { "@standard-schema/spec": "^1.1.0", "@types/chai": "^5.2.2", - "@vitest/spy": "4.1.10", - "@vitest/utils": "4.1.10", + "@vitest/spy": "4.1.11", + "@vitest/utils": "4.1.11", "chai": "^6.2.2", "tinyrainbow": "^3.1.0" }, @@ -893,13 +893,13 @@ } }, "node_modules/@vitest/mocker": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/mocker/-/mocker-4.1.10.tgz", - "integrity": "sha512-v0xaezt+DKEmKfaxg133ldzADrwLGd7Ze1MfQQTYfvs8OqZIwbxyxaYURivwV7sWy5fqn3rH5uOrSp07bp44Ow==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/mocker/-/mocker-4.1.11.tgz", + "integrity": "sha512-2XJVD55d1o5AZous5CCGKS74g/riOj9odEt2bQpCVZeblHyHdnMeFl4jl0XjU21stf4mbjUkew2eXQZt65g5CQ==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/spy": "4.1.10", + "@vitest/spy": "4.1.11", "estree-walker": "^3.0.3", "magic-string": "^0.30.21" }, @@ -920,9 +920,9 @@ } }, "node_modules/@vitest/pretty-format": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/pretty-format/-/pretty-format-4.1.10.tgz", - "integrity": "sha512-W1HsjSH4MXQ9YfmmhLAoIYf1HRfekQCGngeIgcei6MP5QQGWUe0gkopdZQaVCFO+JDJMrAJGwa5pRpNpvy4P8Q==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/pretty-format/-/pretty-format-4.1.11.tgz", + "integrity": "sha512-yiZzPbGTS9Sr/JpFl8zHrcIkAofNbFV6k21vIgQN/cY/oxZeXhJv5sc/MBJ5jFKWmWs+oJHw0UXLZjmf931+Vw==", "dev": true, "license": "MIT", "dependencies": { @@ -933,13 +933,13 @@ } }, "node_modules/@vitest/runner": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/runner/-/runner-4.1.10.tgz", - "integrity": "sha512-IKI6kpIH+LmpROplyLwBBaCfMgOZOMsygVa6BARD6ahA04VRuJSa6OaVG7kRvSEMD870Vd91rSSw0eegtWyLGg==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/runner/-/runner-4.1.11.tgz", + "integrity": "sha512-LztvUgdwMNJMIkj3hQnnxiC2Xy1zNxq928W/xhjCLaNCzqTZOudjwbQf6v9IntZGPw132i2Lq2rgTRZHD3JHNw==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/utils": "4.1.10", + "@vitest/utils": "4.1.11", "pathe": "^2.0.3" }, "funding": { @@ -947,14 +947,14 @@ } }, "node_modules/@vitest/snapshot": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/snapshot/-/snapshot-4.1.10.tgz", - "integrity": "sha512-xRkfOT1qpTAi/Ti4Y1LtfRc3kEuqxGw59eN2jN9pRWMtS/XDevekhcFSqvQqjUNGksfjMJu3Y+oJ+4Ypn2OaJw==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/snapshot/-/snapshot-4.1.11.tgz", + "integrity": "sha512-pN7ikn1ON7h8ee4gIAp4AzyK+zBtJPzVbqOgu5LCEh4VaJVbPQcgYQYJIMGQPXVeJJq1fnfazis7a5pFNPahog==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/pretty-format": "4.1.10", - "@vitest/utils": "4.1.10", + "@vitest/pretty-format": "4.1.11", + "@vitest/utils": "4.1.11", "magic-string": "^0.30.21", "pathe": "^2.0.3" }, @@ -963,9 +963,9 @@ } }, "node_modules/@vitest/spy": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/spy/-/spy-4.1.10.tgz", - "integrity": "sha512-PLf/Ugvoq5wO/b4rwYCR1h2PSIdXz7wnkQFMiUpLdtM7l6pqVFcQIBEHyT1+l+cj7mNwAfZHzqXqDyjvOuwbDw==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/spy/-/spy-4.1.11.tgz", + "integrity": "sha512-apNa/prQy2qCeywhnixOHPRCgGNhvg7T4Dapfl1GahLp/R+uhBm5cPyFoNVyqsNd2h1nJxL6BqqdIjiABL60YA==", "dev": true, "license": "MIT", "funding": { @@ -973,13 +973,13 @@ } }, "node_modules/@vitest/utils": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/utils/-/utils-4.1.10.tgz", - "integrity": "sha512-fy9am/HWxbaGt/Sawrp90vt6Y6jQwf1RX77cz3uwoJwJVMli/e1IEwRPnMNJ7vKfPTwo0diXifkpPvwH9v7nGA==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/utils/-/utils-4.1.11.tgz", + "integrity": "sha512-zTCVGpyFsGWBhllOyKlTw/vnr6D9qxsfSDyfbyZmTyjHw5N/VuvzHpHoQjm2ZJzn4RJgx5w4r7V0er69CmLgPQ==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/pretty-format": "4.1.10", + "@vitest/pretty-format": "4.1.11", "convert-source-map": "^2.0.0", "tinyrainbow": "^3.1.0" }, @@ -2550,19 +2550,19 @@ } }, "node_modules/vitest": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/vitest/-/vitest-4.1.10.tgz", - "integrity": "sha512-R9jUTe5S4Qb0HCd4TNqpC7oGcrMssMRGXLW80ubjWsW9VH5GF8y1Y0SFLY9AbqSk6nt0PnOx4H4WNJYZ13GUPw==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/vitest/-/vitest-4.1.11.tgz", + "integrity": "sha512-fhACrNXUidIbGSBr5FlbuBkO7VWC1ZyLl0DO4CU2DrQoAPxX84Ysxs+HeGQpii5lZWV1Q4gBZTTu49mF+A6Edw==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/expect": "4.1.10", - "@vitest/mocker": "4.1.10", - "@vitest/pretty-format": "4.1.10", - "@vitest/runner": "4.1.10", - "@vitest/snapshot": "4.1.10", - "@vitest/spy": "4.1.10", - "@vitest/utils": "4.1.10", + "@vitest/expect": "4.1.11", + "@vitest/mocker": "4.1.11", + "@vitest/pretty-format": "4.1.11", + "@vitest/runner": "4.1.11", + "@vitest/snapshot": "4.1.11", + "@vitest/spy": "4.1.11", + "@vitest/utils": "4.1.11", "es-module-lexer": "^2.0.0", "expect-type": "^1.3.0", "magic-string": "^0.30.21", @@ -2590,12 +2590,12 @@ "@edge-runtime/vm": "*", "@opentelemetry/api": "^1.9.0", "@types/node": "^20.0.0 || ^22.0.0 || >=24.0.0", - "@vitest/browser-playwright": "4.1.10", - "@vitest/browser-preview": "4.1.10", - "@vitest/browser-webdriverio": "4.1.10", - "@vitest/coverage-istanbul": "4.1.10", - "@vitest/coverage-v8": "4.1.10", - "@vitest/ui": "4.1.10", + "@vitest/browser-playwright": "4.1.11", + "@vitest/browser-preview": "4.1.11", + "@vitest/browser-webdriverio": "4.1.11", + "@vitest/coverage-istanbul": "4.1.11", + "@vitest/coverage-v8": "4.1.11", + "@vitest/ui": "4.1.11", "happy-dom": "*", "jsdom": "*", "vite": "^6.0.0 || ^7.0.0 || ^8.0.0" diff --git a/package.json b/package.json index 893432e..92b55c5 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,6 @@ "typescript": "6.0.3", "typescript-eslint": "8.66.0", "vite": "8.2.1", - "vitest": "4.1.10" + "vitest": "4.1.11" } } diff --git a/src/main/agent-orchestrator.ts b/src/main/agent-orchestrator.ts new file mode 100644 index 0000000..d43ffbb --- /dev/null +++ b/src/main/agent-orchestrator.ts @@ -0,0 +1,258 @@ +/** + * The bounded agent orchestrator (Phase 2, Milestone 7). + * + * This is the loop that turns one request into a sequence of already-existing + * actions, and it is deliberately small. Every decision it makes — what to do + * next, whether a limit has been reached, whether the profile permits a step, + * whether the result counts as verified — is made by the pure functions in + * `src/shared/agent/orchestration.ts`, which are testable without a + * filesystem. What lives here is only the part that cannot be pure: reading + * the clock, asking whether the emergency stop is engaged, and calling the + * injected step executor. + * + * ## What this module is not + * + * It is **not** a second permission engine. It never decides that something + * is allowed. `runStep` is supplied by `main/ipc.ts` and routes every step + * through the unmodified `handleActionProposal`, so each step is decided by + * `main/permissions.ts` on its own action type, confirmed natively where the + * floor requires it, and written to the audit log — exactly as the same + * operation would be if a person had clicked it. This module can only ever + * stop a run *earlier* than the permission engine would have. + * + * It is **not** a model client. No provider is called and no network request + * is made; the step sequence is derived deterministically from the profile + * and the objective. `resolvedProvider` is recorded for transparency about + * what *would* carry a model call once one exists. + * + * ## The one thing a profile may add + * + * A profile whose `permissionPolicy` says `confirm` for a tool causes + * {@link AgentStepRequest.requiresConfirmation} to be set, and the step + * executor asks the user before proposing the action at all. That is an + * **additional** gate in front of the pipeline, never a substitute for one: a + * profile cannot say `allow`, so nothing here can remove a confirmation the + * floor requires or turn a denial into a permission. + */ + +import { + buildAgentPlan, + classifyAgentRun, + decideNextStep, + describeProfileTools, + evaluateAgentVerification, +} from '../shared/agent/orchestration'; +import type { AgentPlanStep } from '../shared/agent/orchestration'; +import { findAgentTool } from '../shared/agent/tools'; +import type { AgentToolDefinition } from '../shared/agent/tools'; +import type { ModelProvider } from '../shared/constants'; +import { agentRunSchema } from '../shared/schemas/agent.schema'; +import type { + AgentProfile, + AgentRun, + AgentRunStep, + AgentStopReason, +} from '../shared/schemas/agent.schema'; +import type { AuditOutcome } from '../shared/types'; +import type { WorkspaceErrorCode } from '../shared/workspace/errors'; + +/** One step, as handed to the executor. */ +export interface AgentStepRequest { + readonly step: AgentPlanStep; + readonly tool: AgentToolDefinition; + /** The profile asked for this step to be confirmed, on top of any floor. */ + readonly requiresConfirmation: boolean; + /** The run's objective, for the one tool that needs it. */ + readonly objective: string; +} + +/** What the executor reports back. Never a message, never a path. */ +export interface AgentStepOutcome { + readonly outcome: AuditOutcome; + /** Reviewed, bounded wording built from counts — never from file contents. */ + readonly summary: string; + readonly errorCode?: WorkspaceErrorCode; + /** How much of the run's output budget this step consumed. */ + readonly outputBytes: number; +} + +export interface AgentOrchestrationOptions { + readonly runId: string; + readonly profile: AgentProfile; + readonly objective: string; + /** Which provider the profile resolved to. Recorded, not called. */ + readonly provider: ModelProvider; + /** UTC ISO-8601 for the start of the run. */ + readonly startedAt: string; + /** UTC ISO-8601, read again when the run ends. */ + readonly nowFn: () => string; + /** Elapsed-time source, injected so a test can drive the clock. */ + readonly monotonicMs: () => number; + /** Cancellation, from the `agent:cancel` channel. */ + readonly signal: AbortSignal; + /** Re-read from disk between steps, never cached for the run. */ + readonly isEmergencyEngaged: () => Promise; + /** Whether a project is approved *right now*. */ + readonly hasProject: () => boolean; + readonly runStep: (request: AgentStepRequest) => Promise; +} + +/** + * Runs one bounded orchestration and returns its record. + * + * Never throws for an ordinary refusal: a denial, a decline, a limit and a + * cancellation are all outcomes recorded on the returned {@link AgentRun}, + * because a run that was stopped on purpose is not an error. The result is + * validated against `agentRunSchema` before it is returned, so a malformed + * record fails here rather than at the IPC boundary. + */ +export async function runAgentOrchestration(options: AgentOrchestrationOptions): Promise { + const { profile, objective, signal } = options; + + const plan = buildAgentPlan(profile, objective); + const steps: AgentRunStep[] = []; + const startMs = options.monotonicMs(); + + let outputBytes = 0; + let anyStepFailed = false; + // Definite assignment: every path out of the loop below sets this before it + // breaks, and there is no other exit — so an initial value here would be + // dead, and a *wrong* dead default is exactly how a stopped run comes to be + // reported as a completed one. + let stopReason!: AgentStopReason; + + for (;;) { + // Re-read every iteration rather than once: "emergency stop" has to mean + // "stop what is happening", not only "start nothing new", and the same + // applies to the project being closed mid-run. + const emergencyEngaged = await options.isEmergencyEngaged(); + + const decision = decideNextStep(profile, plan, { + completedSteps: steps.length, + elapsedMs: Math.max(0, options.monotonicMs() - startMs), + outputBytes, + cancelled: signal.aborted, + emergencyEngaged, + hasProject: options.hasProject(), + }); + + if (decision.kind === 'stop') { + stopReason = decision.reason; + break; + } + + const tool = findAgentTool(decision.step.tool); + if (tool === null) { + // Unreachable while `decideNextStep` has already resolved the tool; + // treated as a refusal rather than an exception so a run can never + // proceed on a step nothing could describe. + stopReason = 'tool-not-allowed'; + break; + } + + const stepStartedMs = options.monotonicMs(); + const result = await options.runStep({ + step: decision.step, + tool, + requiresConfirmation: decision.requiresConfirmation, + objective, + }); + const durationMs = Math.max(0, options.monotonicMs() - stepStartedMs); + + steps.push({ + index: steps.length, + tool: decision.step.tool, + actionType: tool.actionType, + outcome: result.outcome, + summary: result.summary, + ...(result.errorCode === undefined ? {} : { errorCode: result.errorCode }), + durationMs, + outputBytes: result.outputBytes, + }); + outputBytes += result.outputBytes; + + // A refusal ends the run. Continuing after the permission engine said no, + // or after the user answered no, would be the run arguing with an answer + // it already received. + if (result.outcome === 'denied') { + stopReason = 'step-denied'; + break; + } + if (result.outcome === 'aborted') { + stopReason = 'step-declined'; + break; + } + // A *failure* is information, not a refusal: a lint script exiting + // non-zero is exactly what the run was asked to find out. The remaining + // steps are each separately gated anyway, and the step ceiling still + // bounds how many of them there can be. + if (result.outcome === 'failure') anyStepFailed = true; + } + + if (stopReason === 'completed' && anyStepFailed) stopReason = 'step-failed'; + + const verification = evaluateAgentVerification(profile, steps); + const classified = classifyAgentRun(stopReason, verification); + + return agentRunSchema.parse({ + runId: options.runId, + profileId: profile.id, + profileName: profile.name, + provider: options.provider, + status: classified.status, + stopReason: classified.stopReason, + startedAt: options.startedAt, + finishedAt: options.nowFn(), + steps, + verification, + totals: { + steps: steps.length, + outputBytes, + durationMs: Math.max(0, options.monotonicMs() - startMs), + }, + } satisfies AgentRun); +} + +/** + * The sentence shown in the native dialog before a run starts. + * + * Built here, in the main process, from the *stored* profile — never from the + * request — so a compromised renderer can cause the question to be asked but + * cannot change what it says. It states every one of the milestone's + * "clearly show active permissions and limits" items in plain text: which + * tools, which part of the project, and the three ceilings. + */ +export function describeAgentRun(profile: AgentProfile, plannedSteps: number): string { + const tools = describeProfileTools(profile); + const startsProcess = tools.some((tool) => tool.commandId !== null); + const scope = profile.approvedWorkspacePaths + .map((path) => (path === '' ? ' the whole approved project' : ` ${path}`)) + .join('\n'); + + const lines = [ + `Run the "${profile.name}" agent against the approved project?`, + '', + `Planned steps: ${String(plannedSteps)} (ceiling ${String(profile.limits.maxSteps)})`, + `Time limit: ${String(Math.round(profile.limits.maxDurationMs / 1000))}s`, + `Output limit: ${String(profile.limits.maxOutputBytes)} bytes`, + '', + 'This agent may use only these tools:', + ...tools.map((tool) => ` ${tool.label} (${tool.actionType})`), + '', + 'Limited to these parts of the project:', + scope, + ]; + + if (profile.verification.length > 0) { + lines.push('', `Verification required: ${profile.verification.join(', ')}`); + } + + lines.push( + '', + startsProcess + ? 'This agent can run the project’s own scripts. Each one is confirmed separately before it starts.' + : 'This agent cannot start a process, change a file, or create a commit.', + ); + + return lines.join('\n'); +} diff --git a/src/main/agent-profiles.ts b/src/main/agent-profiles.ts new file mode 100644 index 0000000..402c3cb --- /dev/null +++ b/src/main/agent-profiles.ts @@ -0,0 +1,368 @@ +/** + * Agent profile storage (Phase 2, Milestone 7). + * + * Reads and writes `agents/profiles.json`. It mirrors `main/settings.ts` and + * `main/policy.ts` deliberately and in detail, because a profile file is the + * same *kind* of thing the permission policy is: user-editable configuration + * that shapes what the application will later do. + * + * The properties carried over from those two modules: + * + * - **Loading never throws and never merges.** A missing file, an unreadable + * one, an oversized one, malformed JSON, a `__proto__` key anywhere in it, + * or a document the schema rejects all resolve the same way: the default + * store, which is "no user profiles, the most restricted built-in active". + * A document either validates in full, as itself, or it is discarded in + * full. There is no partial-trust path, so a file with nine good profiles + * and one malformed one yields none of them rather than nine. + * - **Writing is atomic and re-validated.** The document is checked against + * the schema immediately before serialising, written to a uniquely named + * temporary file in the same directory, flushed, and moved into place with + * a single rename — so a crash or a concurrent write leaves either the + * previous complete document or the new one, never a partial write. + * - **Built-ins are never written.** Only user profiles are persisted; the + * built-ins are rebuilt from reviewed source on every load. Editing this + * file therefore cannot redefine a shipped profile into a permissive one. + * + * And one property of its own: + * + * - **User data is preserved across every operation.** Each mutation reads + * the current store, changes exactly the one profile it names, and writes + * the whole document back. No operation regenerates the list, and none + * silently drops a profile it did not understand — because a profile it + * did not understand would have failed validation at load and the whole + * file would already have been refused. + * + * Every function takes a plain file path rather than a `UserDataPaths`, for + * the reason `loadSettings` does: a test points at a temporary directory and + * configures nothing else. + */ + +import { randomUUID } from 'node:crypto'; +import { mkdir, open, readFile, rename, rm, stat } from 'node:fs/promises'; +import { basename, dirname, join } from 'node:path'; + +import { AGENT_MAX_PROFILES, AGENT_PROFILE_STORE_MAX_BYTES } from '../shared/constants'; +import { FORBIDDEN_OBJECT_KEYS } from '../shared/constants'; +import { AgentError } from '../shared/agent/errors'; +import { + createBuiltInAgentProfiles, + createDefaultAgentProfileStore, + findAgentProfile, + isBuiltInAgentProfileId, + resolveAgentRegistry, +} from '../shared/agent/registry'; +import type { AgentRegistry } from '../shared/agent/registry'; +import { agentProfileSchema, agentProfileStoreSchema } from '../shared/schemas/agent.schema'; +import type { + AgentProfile, + AgentProfileInput, + AgentProfileStore, +} from '../shared/schemas/agent.schema'; + +/** + * Mirrors `main/policy.ts`'s own `containsForbiddenKey` exactly, including the + * reasoning: `JSON.parse` does not fall for a literal `__proto__` key, but + * nothing downstream of this loader is allowed to assume that. A third + * independent copy rather than a shared import, so this module gains no + * runtime dependency on an already-reviewed module from an earlier milestone + * for one small, self-contained, pure check. + */ +const MAX_PROFILE_JSON_DEPTH = 64; + +function containsForbiddenKey(value: unknown, depth = 0): boolean { + if (depth > MAX_PROFILE_JSON_DEPTH) return true; + if (value === null || typeof value !== 'object') return false; + + if (Array.isArray(value)) { + return value.some((element) => containsForbiddenKey(element, depth + 1)); + } + + const record = value as Record; + for (const key of Object.keys(record)) { + if (FORBIDDEN_OBJECT_KEYS.includes(key)) return true; + if (containsForbiddenKey(record[key], depth + 1)) return true; + } + return false; +} + +/** + * A destination-file rename can transiently fail on Windows when something + * else briefly holds the destination open. The same sharing-violation retry + * `main/settings.ts` performs, and for the same reason: each attempt is one + * whole-file rename, so retrying risks a delayed write, never a partial one. + */ +const RENAME_MAX_ATTEMPTS = 10; +const RENAME_RETRY_DELAY_MS = 20; + +function isTransientRenameError(error: unknown): boolean { + if (typeof error !== 'object' || error === null || !('code' in error)) return false; + const { code } = error; + return code === 'EPERM' || code === 'EBUSY' || code === 'EACCES'; +} + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +async function renameWithRetry(from: string, to: string): Promise { + for (let attempt = 1; attempt <= RENAME_MAX_ATTEMPTS; attempt += 1) { + try { + await rename(from, to); + return; + } catch (error) { + if (attempt === RENAME_MAX_ATTEMPTS || !isTransientRenameError(error)) throw error; + await delay(RENAME_RETRY_DELAY_MS * attempt); + } + } +} + +/** + * Loads `storeFile`, failing safe on every problem. + * + * Never throws, and never creates a file. A first launch is indistinguishable, + * by design, from a corrupted one: both return the same default store, which + * is the most restricted state rather than the most convenient one. + * + * The size check happens before the read, not after: a file that grew without + * bound should not be pulled into memory to discover that it is too large. + */ +export async function loadAgentProfileStore(storeFile: string): Promise { + try { + const stats = await stat(storeFile); + if (stats.size > AGENT_PROFILE_STORE_MAX_BYTES) return createDefaultAgentProfileStore(); + } catch { + // Missing file (first launch) and any other stat failure fail the same + // way as an unreadable one. Neither the error nor the path is surfaced. + return createDefaultAgentProfileStore(); + } + + let raw: string; + try { + raw = await readFile(storeFile, 'utf8'); + } catch { + return createDefaultAgentProfileStore(); + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return createDefaultAgentProfileStore(); + } + + if (containsForbiddenKey(parsed)) return createDefaultAgentProfileStore(); + + const result = agentProfileStoreSchema.safeParse(parsed); + if (!result.success) return createDefaultAgentProfileStore(); + + return result.data; +} + +/** + * Writes `store` to `storeFile` atomically. + * + * Re-validated against the schema immediately before serialising, so a caller + * cannot persist a document that only *claims* the type at compile time — + * which matters more here than for settings, because the value being + * persisted describes what an agent will later be permitted to reach for. + */ +export async function writeAgentProfileStore( + storeFile: string, + store: AgentProfileStore, +): Promise { + const validated = agentProfileStoreSchema.parse(store); + const payload = JSON.stringify(validated, null, 2); + + const dir = dirname(storeFile); + await mkdir(dir, { recursive: true }); + + const tempFile = join(dir, `${basename(storeFile)}.${randomUUID()}.tmp`); + + try { + const handle = await open(tempFile, 'w'); + try { + await handle.writeFile(payload, 'utf8'); + await handle.sync(); + } finally { + await handle.close(); + } + + await renameWithRetry(tempFile, storeFile); + } catch (error) { + await rm(tempFile, { force: true }).catch(() => undefined); + throw error; + } +} + +/** The merged, resolved registry as the rest of the application sees it. */ +export async function readAgentRegistry(storeFile: string): Promise { + return resolveAgentRegistry(await loadAgentProfileStore(storeFile)); +} + +/** + * Persists a changed store and answers with the freshly resolved registry. + * + * Resolved from the document that was *written*, not from the caller's + * in-memory idea of it, so the answer reflects what the next load will see — + * including any fallback `resolveAgentRegistry` applied because the active + * profile was just deleted or disabled. + */ +async function commit(storeFile: string, store: AgentProfileStore): Promise { + try { + await writeAgentProfileStore(storeFile, store); + } catch { + // The underlying error never crosses a boundary; the caller gets the + // normalized code and the store on disk is unchanged. + throw new AgentError('AGENT_PROFILE_STORE_FAILED'); + } + return resolveAgentRegistry(store); +} + +/** Refuses an operation that would edit a profile shipped in reviewed source. */ +function requireUserProfile(store: AgentProfileStore, profileId: string): AgentProfile { + if (isBuiltInAgentProfileId(profileId)) throw new AgentError('AGENT_PROFILE_READ_ONLY'); + const existing = findAgentProfile(store.profiles, profileId); + if (existing === null) throw new AgentError('AGENT_PROFILE_NOT_FOUND'); + return existing; +} + +/** + * Makes `profileId` the active profile. + * + * Refuses an unknown or disabled profile rather than accepting it and letting + * the load-time fallback quietly substitute a different one: a selection that + * silently does not take effect is worse than a refusal the interface can + * report. + */ +export async function selectAgentProfile( + storeFile: string, + profileId: string, +): Promise { + const store = await loadAgentProfileStore(storeFile); + const merged = [...createBuiltInAgentProfiles(), ...store.profiles]; + const target = findAgentProfile(merged, profileId); + if (target === null) throw new AgentError('AGENT_PROFILE_NOT_FOUND'); + if (!target.enabled) throw new AgentError('AGENT_PROFILE_DISABLED'); + + return commit(storeFile, { ...store, activeProfileId: profileId }); +} + +/** + * Adds a user profile. + * + * The id must be free across *both* lists: a built-in id is refused with the + * same "already exists" code as a user id, because from the caller's point of + * view the id is taken either way. + */ +export async function createAgentProfile( + storeFile: string, + input: AgentProfileInput, + now: string, +): Promise { + const store = await loadAgentProfileStore(storeFile); + + if (isBuiltInAgentProfileId(input.id)) throw new AgentError('AGENT_PROFILE_EXISTS'); + if (findAgentProfile(store.profiles, input.id) !== null) { + throw new AgentError('AGENT_PROFILE_EXISTS'); + } + if (store.profiles.length + createBuiltInAgentProfiles().length >= AGENT_MAX_PROFILES) { + throw new AgentError('AGENT_PROFILE_LIMIT_REACHED'); + } + + // `builtIn` and the timestamps are supplied here, never by the caller — + // `agentProfileInputSchema` has no field for any of them. + const profile = agentProfileSchema.safeParse({ + ...input, + builtIn: false, + createdAt: now, + updatedAt: now, + }); + if (!profile.success) throw new AgentError('AGENT_PROFILE_INVALID'); + + return commit(storeFile, { ...store, profiles: [...store.profiles, profile.data] }); +} + +/** + * Replaces a user profile in place. + * + * `createdAt` is carried over from the existing profile rather than taken + * from the request, so an update cannot rewrite when a profile was created. + * The submitted id must match the addressed one — a mismatch is a refusal, + * not a rename. + */ +export async function updateAgentProfile( + storeFile: string, + profileId: string, + input: AgentProfileInput, + now: string, +): Promise { + const store = await loadAgentProfileStore(storeFile); + const existing = requireUserProfile(store, profileId); + + if (input.id !== profileId) throw new AgentError('AGENT_PROFILE_INVALID'); + + const profile = agentProfileSchema.safeParse({ + ...input, + builtIn: false, + createdAt: existing.createdAt, + updatedAt: now, + }); + if (!profile.success) throw new AgentError('AGENT_PROFILE_INVALID'); + + return commit(storeFile, { + ...store, + profiles: store.profiles.map((entry) => (entry.id === profileId ? profile.data : entry)), + }); +} + +/** + * Removes a user profile. + * + * Deleting the **active** profile is permitted, and is the reason + * `resolveAgentRegistry` resolves the active id rather than trusting it: the + * store is left naming a profile that no longer exists, and the next + * resolution — including the one this function returns — falls back to the + * most restricted built-in. The active id is also rewritten here so the file + * on disk does not keep pointing at something deleted. + */ +export async function deleteAgentProfile( + storeFile: string, + profileId: string, +): Promise { + const store = await loadAgentProfileStore(storeFile); + requireUserProfile(store, profileId); + + const profiles = store.profiles.filter((entry) => entry.id !== profileId); + const next: AgentProfileStore = { ...store, profiles }; + const fallback = resolveAgentRegistry(next); + + return commit(storeFile, { ...next, activeProfileId: fallback.activeProfileId }); +} + +/** + * Enables or disables a user profile. + * + * A built-in cannot be disabled: the default built-in is what every failed + * resolution falls back to, so allowing it to be turned off would leave the + * fallback itself unusable. Disabling the *active* user profile is allowed, + * and falls back exactly as deletion does. + */ +export async function setAgentProfileEnabled( + storeFile: string, + profileId: string, + enabled: boolean, + now: string, +): Promise { + const store = await loadAgentProfileStore(storeFile); + const existing = requireUserProfile(store, profileId); + + const updated: AgentProfile = { ...existing, enabled, updatedAt: now }; + const next: AgentProfileStore = { + ...store, + profiles: store.profiles.map((entry) => (entry.id === profileId ? updated : entry)), + }; + const fallback = resolveAgentRegistry(next); + + return commit(storeFile, { ...next, activeProfileId: fallback.activeProfileId }); +} diff --git a/src/main/directory-picker.ts b/src/main/directory-picker.ts new file mode 100644 index 0000000..142dc11 --- /dev/null +++ b/src/main/directory-picker.ts @@ -0,0 +1,53 @@ +/** + * The native project-directory picker (Phase 2, Milestone 5). + * + * The counterpart to `main/confirm.ts`, and native for exactly the same + * reason: this dialog is the *consent* for the whole workspace feature. It is + * the only place a directory outside the application's own data becomes + * readable, and the renderer must not be able to choose that directory, + * pre-fill it, forge the dialog, or dismiss it on the user's behalf. + * + * So the renderer supplies nothing. `workspaceSelectRequestSchema` takes no + * arguments at all, this function takes no path, and there is no default, + * suggested or remembered location: whatever `dialog.showOpenDialog` returns + * is what the user actually clicked on. A compromised renderer can ask that + * the user be asked, and nothing more. + * + * `properties` is deliberately minimal — `openDirectory` alone. In + * particular there is no `multiSelections` (one project at a time), + * no `createDirectory`/`promptToCreate` (this milestone writes nothing, and a + * picker that can create a folder writes), and no `showHiddenFiles`. + */ + +import type { BrowserWindow, OpenDialogOptions } from 'electron'; +import { dialog } from 'electron'; + +const DIALOG_OPTIONS: OpenDialogOptions = { + title: 'Local Agent — select a project to inspect', + buttonLabel: 'Approve project', + message: 'Local Agent will read files inside this directory. It cannot modify them.', + properties: ['openDirectory'], +}; + +/** + * Shows the picker and resolves to the chosen directory, or `null` when the + * user dismissed it. + * + * `null` is a first-class answer, not an error: cancelling a picker is the + * user declining, and the caller turns it into the dedicated + * `WORKSPACE_SELECTION_CANCELLED` code rather than a generic failure. + * + * The returned string is the raw OS path. It is *not* trusted as a project + * root on the strength of having come from here — `adoptProjectDirectory` + * still canonicalises it, confirms it is a directory, and refuses the + * application's own data directory. A dialog result is still input. + */ +export async function showDirectoryPicker(window: BrowserWindow | null): Promise { + const result = window + ? await dialog.showOpenDialog(window, DIALOG_OPTIONS) + : await dialog.showOpenDialog(DIALOG_OPTIONS); + + if (result.canceled) return null; + const [directory] = result.filePaths; + return directory === undefined || directory.length === 0 ? null : directory; +} diff --git a/src/main/git-runner.ts b/src/main/git-runner.ts new file mode 100644 index 0000000..45da148 --- /dev/null +++ b/src/main/git-runner.ts @@ -0,0 +1,304 @@ +/** + * Git integration (Phase 2, Milestone 6). + * + * Two read-only operations — `status` and `diff` — and exactly one that + * changes anything: a checkpoint commit on the branch that is already checked + * out. Every argument vector comes from `shared/workspace/git.ts`; this module + * builds none of its own, and no request schema anywhere carries a git + * subcommand, so the set of things git can be asked to do here is closed. + * + * ## What a checkpoint may not do + * + * `createGitCheckpoint` refuses rather than proceeds in every case where a + * commit could cost the user something: + * + * - **The project must be the repository root.** If the user approved a + * subdirectory of a repository, `git add --all` from there would still + * stage the whole repository — files outside the directory they approved. + * `--show-toplevel` is compared against the approved root, and a mismatch + * is refused. This is the same containment principle as + * `workspace-paths.ts`, applied to the one tool that has its own idea of + * where the project starts. + * - **HEAD must be on a branch.** A commit on a detached HEAD is trivially + * lost, and the checkpoint exists precisely so that nothing is lost. + * - **There must be something to commit.** An empty checkpoint is noise in + * someone's history. + * + * And what it never does, structurally: reset, checkout, switch, restore, + * clean, remove, rebase, merge, stash, branch, tag, push, fetch, pull, or + * touch a remote or a credential helper. See + * {@link FORBIDDEN_GIT_SUBCOMMANDS}, which a test asserts is absent from this + * file. + * + * ## Hooks are disabled for every invocation + * + * A repository carries executable hooks. Running them because the user opened + * a directory would be executing code from untrusted input, so + * `buildGitArgv` points `core.hooksPath` at an empty directory this + * application owns and keeps empty. That directory is created here rather + * than assumed to exist. + */ + +import { mkdir, realpath } from 'node:fs/promises'; + +import { runProcess } from './process-runner'; +import type { ProcessRunResult } from './process-runner'; +import { isContainedPath } from './workspace-paths'; +import type { ApprovedProject } from './workspace-session'; +import { + COMMAND_MAX_OUTPUT_LINES, + GIT_CHECKPOINT_MESSAGE_PREFIX, + GIT_MAX_DIFF_BYTES, + GIT_TIMEOUT_MS, + WORKSPACE_DIFF_MAX_LINES, +} from '../shared/constants'; +import type { GitCheckpointValue, GitDiffValue, GitStatusValue } from '../shared/schemas'; +import { gitDiffLineSchema } from '../shared/schemas/coding.schema'; +import type { z } from 'zod'; +import { WorkspaceError } from '../shared/workspace/errors'; +import { + buildGitArgv, + FORBIDDEN_GIT_SUBCOMMANDS, + gitBranchArgs, + gitCommitArgs, + gitDiffArgs, + gitHeadArgs, + gitIsRepositoryArgs, + gitStageAllArgs, + gitStatusArgs, + gitToplevelArgs, + parseGitStatus, +} from '../shared/workspace/git'; + +/** Re-exported so a reviewer finds the list from either file. */ +export { FORBIDDEN_GIT_SUBCOMMANDS }; + +type GitDiffLine = z.infer; + +export interface GitRuntime { + /** An empty directory this application owns, used to disable repository hooks. */ + readonly hooksDir: string; +} + +interface GitOutcome { + readonly ok: boolean; + readonly stdout: string; + readonly result: ProcessRunResult; +} + +function joinStream(result: ProcessRunResult, stream: 'stdout' | 'stderr'): string { + return result.output + .filter((line) => line.stream === stream) + .map((line) => line.text) + .join('\n'); +} + +/** + * Runs one git invocation, or throws a normalized error. + * + * Git's own stderr never crosses a boundary — it routinely contains absolute + * paths, branch names and occasionally a remote URL. A failure becomes + * `GIT_COMMAND_FAILED` and nothing more, exactly as a filesystem `errno` + * becomes `WORKSPACE_READ_FAILED` in `workspace-paths.ts`. + */ +async function runGit( + project: ApprovedProject, + runtime: GitRuntime, + operationArgs: readonly string[], + maxOutputBytes = GIT_MAX_DIFF_BYTES, +): Promise { + await mkdir(runtime.hooksDir, { recursive: true }).catch(() => undefined); + + let result: ProcessRunResult; + try { + result = await runProcess({ + program: 'git', + args: buildGitArgv(runtime.hooksDir, operationArgs), + cwd: project.rootPath, + timeoutMs: GIT_TIMEOUT_MS, + maxOutputBytes, + maxOutputLines: COMMAND_MAX_OUTPUT_LINES, + }); + } catch { + throw new WorkspaceError('GIT_UNAVAILABLE'); + } + + return { ok: result.exitCode === 0, stdout: joinStream(result, 'stdout'), result }; +} + +/** + * Confirms the approved project is a Git working tree *and* is its root. + * + * Both halves matter: the first because git is otherwise being asked about a + * directory it knows nothing about, the second because every write-adjacent + * git operation in this module works on the whole working tree. + */ +async function requireRepositoryRoot(project: ApprovedProject, runtime: GitRuntime): Promise { + if (!project.hasGitMetadata) throw new WorkspaceError('GIT_NOT_A_REPOSITORY'); + + const inside = await runGit(project, runtime, gitIsRepositoryArgs()); + if (!inside.ok || inside.stdout.trim() !== 'true') { + throw new WorkspaceError('GIT_NOT_A_REPOSITORY'); + } + + const toplevel = await runGit(project, runtime, gitToplevelArgs()); + if (!toplevel.ok) throw new WorkspaceError('GIT_NOT_A_REPOSITORY'); + + const reported = toplevel.stdout.trim(); + if (reported.length === 0) throw new WorkspaceError('GIT_NOT_A_REPOSITORY'); + + // Canonicalise both sides before comparing, for exactly the reason + // `workspace-session.ts` canonicalises the user-data directory: a short + // (8.3) Windows ancestor shares no common root with its long form, and a + // comparison between the two forms would silently pass. + let canonical: string; + try { + canonical = await realpath(reported); + } catch { + throw new WorkspaceError('GIT_NOT_A_REPOSITORY'); + } + if ( + !isContainedPath(project.rootPath, canonical) || + !isContainedPath(canonical, project.rootPath) + ) { + throw new WorkspaceError('GIT_NOT_A_REPOSITORY'); + } +} + +/** `git status`, parsed, bounded, and with every path re-validated. */ +export async function readGitStatus( + project: ApprovedProject, + runtime: GitRuntime, +): Promise { + await requireRepositoryRoot(project, runtime); + + const status = await runGit(project, runtime, gitStatusArgs()); + if (!status.ok) throw new WorkspaceError('GIT_COMMAND_FAILED'); + + const parsed = parseGitStatus(status.stdout); + return { + branch: parsed.branch, + entries: [...parsed.entries], + clean: parsed.entries.length === 0 && parsed.unparsableEntries === 0, + truncated: parsed.truncated || status.result.outputTruncated, + unparsableEntries: parsed.unparsableEntries, + }; +} + +function classifyDiffLine(line: string): GitDiffLine['kind'] { + if ( + line.startsWith('diff --git') || + line.startsWith('index ') || + line.startsWith('--- ') || + line.startsWith('+++ ') || + line.startsWith('new file') || + line.startsWith('deleted file') || + line.startsWith('old mode') || + line.startsWith('new mode') || + line.startsWith('similarity index') || + line.startsWith('rename ') || + line.startsWith('copy ') || + line.startsWith('Binary files') + ) { + return 'meta'; + } + if (line.startsWith('@@')) return 'hunk'; + if (line.startsWith('+')) return 'added'; + if (line.startsWith('-')) return 'removed'; + return 'context'; +} + +/** + * `git diff` for the working tree, optionally narrowed to one file. + * + * `relativePath` has already been validated by the request schema and is + * passed after `--`, so a file whose name begins with a dash cannot be read + * by git as an option. + * + * Note that tab characters in the diff arrive as spaces: every line crossing + * to the renderer is sanitized by the process runner, and a tab is a control + * character. Indentation is therefore approximate in this view. It is a + * display trade-off, taken deliberately rather than loosening the rule that + * no control character reaches the interface. + */ +export async function readGitDiff( + project: ApprovedProject, + runtime: GitRuntime, + relativePath: string | null, +): Promise { + await requireRepositoryRoot(project, runtime); + + const diff = await runGit(project, runtime, gitDiffArgs(relativePath)); + if (!diff.ok) throw new WorkspaceError('GIT_COMMAND_FAILED'); + + const rawLines = diff.stdout.length === 0 ? [] : diff.stdout.split('\n'); + const lines: GitDiffLine[] = []; + let truncated = diff.result.outputTruncated; + + for (const line of rawLines) { + if (lines.length >= WORKSPACE_DIFF_MAX_LINES) { + truncated = true; + break; + } + lines.push({ kind: classifyDiffLine(line), text: line }); + } + + return { path: relativePath, lines, truncated, empty: lines.length === 0 }; +} + +/** The message a checkpoint commits under. Never renderer or project text. */ +export function buildCheckpointMessage(now: string): string { + return `${GIT_CHECKPOINT_MESSAGE_PREFIX} ${now}`; +} + +/** + * Describes exactly what a checkpoint would do, for the confirmation dialog. + * + * Built before the confirmation rather than after it, so the user approves a + * statement of the real commands rather than a summary of them. + */ +export function describeCheckpoint(now: string): string { + const message = buildCheckpointMessage(now); + return `git add --all\ngit commit --message "${message}" --no-verify`; +} + +/** + * Stages everything in the working tree and creates one commit. + * + * Nothing here removes, resets or moves anything: `add` stages, `commit` + * records. A user who did not want the checkpoint can undo it with their own + * tools, and their content is in the commit either way — which is the whole + * point of taking one before a change. + */ +export async function createGitCheckpoint( + project: ApprovedProject, + runtime: GitRuntime, + now: string, +): Promise { + await requireRepositoryRoot(project, runtime); + + const branchOutcome = await runGit(project, runtime, gitBranchArgs()); + if (!branchOutcome.ok) throw new WorkspaceError('GIT_COMMAND_FAILED'); + const branch = branchOutcome.stdout.trim(); + if (branch.length === 0 || branch === 'HEAD') throw new WorkspaceError('GIT_DETACHED_HEAD'); + + const statusOutcome = await runGit(project, runtime, gitStatusArgs()); + if (!statusOutcome.ok) throw new WorkspaceError('GIT_COMMAND_FAILED'); + const parsed = parseGitStatus(statusOutcome.stdout); + const filesChanged = parsed.entries.length + parsed.unparsableEntries; + if (filesChanged === 0) throw new WorkspaceError('GIT_NOTHING_TO_COMMIT'); + + const staged = await runGit(project, runtime, gitStageAllArgs()); + if (!staged.ok) throw new WorkspaceError('GIT_COMMAND_FAILED'); + + const message = buildCheckpointMessage(now); + const committed = await runGit(project, runtime, gitCommitArgs(message)); + if (!committed.ok) throw new WorkspaceError('GIT_COMMAND_FAILED'); + + const head = await runGit(project, runtime, gitHeadArgs()); + if (!head.ok) throw new WorkspaceError('GIT_COMMAND_FAILED'); + const commit = head.stdout.trim(); + if (!/^[0-9a-f]{4,40}$/.test(commit)) throw new WorkspaceError('GIT_COMMAND_FAILED'); + + return { branch, commit, message, filesChanged, createdAt: now }; +} diff --git a/src/main/index.ts b/src/main/index.ts index 54692c9..3a01b83 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -2,6 +2,7 @@ import { app, BrowserWindow, ipcMain, safeStorage, session } from 'electron'; import { join } from 'node:path'; import { showNativeConfirmation } from './confirm'; +import { showDirectoryPicker } from './directory-picker'; import { loadEmergencyState } from './emergency'; import { registerIpcHandlers } from './ipc'; import { resolveUserDataPaths } from './paths'; @@ -122,6 +123,12 @@ app safeStorage, requestConfirmation: (message) => showNativeConfirmation(window.isDestroyed() ? null : window, message), + // The coding workspace's one grant of read access (Phase 2, Milestone + // 5), and the only place a directory outside `%APPDATA%\Local-Agent` + // becomes readable. Built here, like the confirmation callback above, + // because this is the one file that imports the live `electron` module + // and holds the real window to parent the dialog to. + selectProjectDirectory: () => showDirectoryPicker(window.isDestroyed() ? null : window), nowFn: () => new Date().toISOString(), }); }) diff --git a/src/main/ipc.ts b/src/main/ipc.ts index 2024fe2..a8405b2 100644 --- a/src/main/ipc.ts +++ b/src/main/ipc.ts @@ -28,21 +28,59 @@ import { randomUUID } from 'node:crypto'; import type { IpcMain, IpcMainInvokeEvent, SafeStorage } from 'electron'; +import { + createAgentProfile, + deleteAgentProfile, + readAgentRegistry, + selectAgentProfile, + setAgentProfileEnabled, + updateAgentProfile, +} from './agent-profiles'; +import { describeAgentRun, runAgentOrchestration } from './agent-orchestrator'; +import type { AgentStepOutcome, AgentStepRequest } from './agent-orchestrator'; import { runAction } from './action-runtime'; import type { ActionRuntime } from './action-runtime'; import { resolveMainChatProvider } from './chat-provider-registry'; +import { loadEmergencyState } from './emergency'; import { ActionExecutionError } from './executor'; +import { createGitCheckpoint, describeCheckpoint, readGitDiff, readGitStatus } from './git-runner'; +import type { GitRuntime } from './git-runner'; import type { UserDataPaths } from './paths'; +import { buildCommandCatalog, runCodingCommand } from './project-commands'; import { clearSecret, SecretStoreUnavailableError, writeSecret } from './secrets'; +import { createChangeStore } from './workspace-changes'; +import { listProjectTree, readProjectFile, searchProject } from './workspace-inspector'; +import { createCodingPlan } from './workspace-planner'; +import { + adoptProjectDirectory, + createWorkspaceSession, + requireApprovedProject, + toProjectSummary, +} from './workspace-session'; import { readReconciledSettings, refreshHasApiKeyAfterSecretChange, writeOnboardingSettings, } from './settings-service'; import { loadSettings } from './settings'; +import { AgentError, isAgentErrorCode } from '../shared/agent/errors'; +import type { AgentErrorCode } from '../shared/agent/errors'; +import { buildAgentPlan } from '../shared/agent/orchestration'; +import { resolveActiveProfile, resolveAgentProvider } from '../shared/agent/registry'; +import type { AgentRegistry } from '../shared/agent/registry'; +import { findAgentTool } from '../shared/agent/tools'; import { CHAT_PROVIDER_ERROR_CODES, ChatProviderError } from '../shared/chat/provider'; import type { ChatProviderResult } from '../shared/chat/provider'; -import { CHAT_STREAM_MAX_DELTA_LENGTH, PROVIDERS_REQUIRING_API_KEY } from '../shared/constants'; +import { + AGENT_NAME_MAX_LENGTH, + AGENT_STEP_SUMMARY_MAX_LENGTH, + CHAT_STREAM_MAX_DELTA_LENGTH, + COMMAND_MAX_CONCURRENT_RUNS, + PROVIDERS_REQUIRING_API_KEY, +} from '../shared/constants'; +import type { ModelProvider } from '../shared/constants'; +import { describeCommandLine, findCodingCommand } from '../shared/workspace/command-registry'; +import { collapseToSingleLine } from '../shared/workspace/text-safety'; import { chatCancelRequestSchema, chatCancelResponseSchema, @@ -70,14 +108,118 @@ import { settingsGetResponseSchema, settingsUpdateRequestSchema, settingsUpdateResponseSchema, + IPC_WORKSPACE_FILE_CHANNEL, + IPC_WORKSPACE_PLAN_CHANNEL, + IPC_WORKSPACE_SEARCH_CHANNEL, + IPC_WORKSPACE_SELECT_CHANNEL, + IPC_WORKSPACE_STATUS_CHANNEL, + IPC_WORKSPACE_TREE_CHANNEL, + workspaceFileRequestSchema, + workspaceFileResponseSchema, + workspacePlanRequestSchema, + workspacePlanResponseSchema, + workspaceSearchRequestSchema, + workspaceSearchResponseSchema, + workspaceSelectRequestSchema, + workspaceSelectResponseSchema, + workspaceStatusRequestSchema, + workspaceStatusResponseSchema, + workspaceTreeRequestSchema, + workspaceTreeResponseSchema, + commandCancelRequestSchema, + commandCancelResponseSchema, + commandListRequestSchema, + commandListResponseSchema, + commandRunRequestSchema, + commandRunResponseSchema, + gitCheckpointRequestSchema, + gitCheckpointResponseSchema, + gitDiffRequestSchema, + gitDiffResponseSchema, + gitStatusRequestSchema, + gitStatusResponseSchema, + IPC_COMMAND_CANCEL_CHANNEL, + IPC_COMMAND_LIST_CHANNEL, + IPC_COMMAND_RUN_CHANNEL, + IPC_GIT_CHECKPOINT_CHANNEL, + IPC_GIT_DIFF_CHANNEL, + IPC_GIT_STATUS_CHANNEL, + IPC_WORKSPACE_APPLY_CHANNEL, + IPC_WORKSPACE_CHANGES_CHANNEL, + IPC_WORKSPACE_PROPOSE_CHANNEL, + IPC_WORKSPACE_ROLLBACK_CHANNEL, + workspaceApplyRequestSchema, + workspaceApplyResponseSchema, + workspaceChangesRequestSchema, + workspaceChangesResponseSchema, + workspaceProposeRequestSchema, + workspaceProposeResponseSchema, + workspaceRollbackRequestSchema, + workspaceRollbackResponseSchema, + agentCancelRequestSchema, + agentCancelResponseSchema, + agentCreateRequestSchema, + agentCreateResponseSchema, + agentDeleteRequestSchema, + agentDeleteResponseSchema, + agentListRequestSchema, + agentListResponseSchema, + agentRunRequestSchema, + agentRunResponseSchema, + agentSelectRequestSchema, + agentSelectResponseSchema, + agentSetEnabledRequestSchema, + agentSetEnabledResponseSchema, + agentUpdateRequestSchema, + agentUpdateResponseSchema, + IPC_AGENT_CANCEL_CHANNEL, + IPC_AGENT_CREATE_CHANNEL, + IPC_AGENT_DELETE_CHANNEL, + IPC_AGENT_LIST_CHANNEL, + IPC_AGENT_RUN_CHANNEL, + IPC_AGENT_SELECT_CHANNEL, + IPC_AGENT_SET_ENABLED_CHANNEL, + IPC_AGENT_UPDATE_CHANNEL, } from '../shared/schemas'; import type { + AgentProfile, + AgentProfileInput, + AgentRegistryResponse, + AgentRun, + AgentRunResponse, ChatSendResponse, + CodingPlan, + CommandIdValue, + CommandCatalog, + CommandListResponse, + CommandRunResponse, + CommandRunResult, + GitCheckpointResponse, + GitCheckpointValue, + GitDiffResponse, + GitDiffValue, + GitStatusResponse, + GitStatusValue, SecretsActionResponse, SecretStatusResult, SettingsActionResponse, Settings, + WorkspaceChangeHistory, + WorkspaceChangeResponse, + WorkspaceChangesResponse, + WorkspaceChangeSet, + WorkspaceFile, + WorkspaceFileResponse, + WorkspacePlanResponse, + WorkspaceProjectResponse, + WorkspaceProjectSummary, + WorkspaceSearchResponse, + WorkspaceSearchResult, + WorkspaceTree, + WorkspaceTreeResponse, } from '../shared/schemas'; +import { isWorkspaceErrorCode, WorkspaceError } from '../shared/workspace/errors'; +import type { WorkspaceErrorCode } from '../shared/workspace/errors'; import type { ActionProposal, ActionResult, ActionType, ConfirmationResult } from '../shared/types'; /** `ActionResult.errorCode` values `secrets.write`'s `perform` may throw. */ @@ -114,6 +256,20 @@ export interface IpcHandlerRuntime { readonly requestConfirmation: (message: string) => Promise; /** UTC ISO-8601. Injected so this stays testable with a fixed clock. */ readonly nowFn: () => string; + /** + * Shows the native project-directory picker and resolves to what the user + * chose, or `null` if they dismissed it (Phase 2, Milestone 5). + * + * Injected for exactly the reason `requestConfirmation` is: the real + * implementation calls `electron`'s `dialog`, which exists only inside a + * running Electron process, so keeping the dependency at the boundary — + * built once in `main/index.ts` — is what lets this module's own tests run + * under plain Node while still exercising the real selection path. + * + * Note what this signature does *not* have: a parameter. Nothing the + * renderer sends can influence which directory is offered or chosen. + */ + readonly selectProjectDirectory: () => Promise; } /** @@ -205,6 +361,208 @@ function toChatSendResponse(result: ActionResult): ChatSendR }; } +/** + * Normalizes whatever code a workspace action failed with. + * + * `perform` below only ever throws a `WorkspaceError`, translated to an + * `ActionExecutionError` carrying the same code — but `execute` also has its + * own generic `EXECUTION_FAILED` for an unexpected throw, and that is not a + * member of the workspace vocabulary. Degrading it to `WORKSPACE_READ_FAILED` + * keeps a response schema-valid rather than letting an internal code fail + * validation on the way out, exactly as `toChatSendResponse` already does for + * the provider vocabulary. Defence in depth for a path that should not be + * reachable, not a substitute for `perform` normalizing its own failures. + */ +function toWorkspaceErrorCode(result: ActionResult): WorkspaceErrorCode | undefined { + if (result.errorCode === undefined) return undefined; + return isWorkspaceErrorCode(result.errorCode) ? result.errorCode : 'WORKSPACE_READ_FAILED'; +} + +function toWorkspaceProjectResponse( + result: ActionResult, +): WorkspaceProjectResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { project: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toWorkspaceTreeResponse(result: ActionResult): WorkspaceTreeResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { tree: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toWorkspaceFileResponse(result: ActionResult): WorkspaceFileResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { file: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toWorkspaceSearchResponse( + result: ActionResult, +): WorkspaceSearchResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { results: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toWorkspacePlanResponse(result: ActionResult): WorkspacePlanResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { plan: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toWorkspaceChangeResponse( + result: ActionResult, +): WorkspaceChangeResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { change: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toWorkspaceChangesResponse( + result: ActionResult, +): WorkspaceChangesResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { history: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toCommandListResponse(result: ActionResult): CommandListResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { catalog: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toCommandRunResponse(result: ActionResult): CommandRunResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { run: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toGitStatusResponse(result: ActionResult): GitStatusResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { status: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toGitDiffResponse(result: ActionResult): GitDiffResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { diff: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toGitCheckpointResponse(result: ActionResult): GitCheckpointResponse { + const errorCode = toWorkspaceErrorCode(result); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { checkpoint: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +/** + * Normalizes whatever code an agent action failed with. + * + * The same defence in depth `toWorkspaceErrorCode` provides for the workspace + * vocabulary: `perform` below only ever throws an `AgentError` translated to + * an `ActionExecutionError` carrying the same code, but `execute` has its own + * generic `EXECUTION_FAILED` for an unexpected throw, and that is not a member + * of this vocabulary. Degrading it keeps a response schema-valid rather than + * letting an internal code fail validation on the way out. + * + * The fallback differs by channel because "something went wrong" means + * something different for a profile write than for a run. + */ +function toAgentErrorCode( + result: ActionResult, + fallback: AgentErrorCode, +): AgentErrorCode | undefined { + if (result.errorCode === undefined) return undefined; + return isAgentErrorCode(result.errorCode) ? result.errorCode : fallback; +} + +function toAgentRegistryResponse(result: ActionResult): AgentRegistryResponse { + const errorCode = toAgentErrorCode(result, 'AGENT_PROFILE_STORE_FAILED'); + return { + outcome: result.outcome, + ...(result.value === undefined + ? {} + : { + registry: { + activeProfileId: result.value.activeProfileId, + profiles: [...result.value.profiles], + }, + }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +function toAgentRunResponse(result: ActionResult): AgentRunResponse { + const errorCode = toAgentErrorCode(result, 'AGENT_RUN_FAILED'); + return { + outcome: result.outcome, + ...(result.value === undefined ? {} : { run: result.value }), + ...(errorCode === undefined ? {} : { errorCode }), + }; +} + +/** + * How much of a run's output budget one step's result consumed. + * + * The serialized size of the value the step produced — a listing, a search + * result, a plan, a command's captured output. Measured rather than guessed + * so that "output limit" means something a person can reason about, and + * measured *here* rather than inside each tool so every tool is counted the + * same way. A value that cannot be serialized counts as nothing rather than + * failing the step. + */ +function measureOutputBytes(value: unknown): number { + if (value === undefined) return 0; + try { + // `JSON.stringify` is typed as returning `string` but really returns + // `undefined` for a value it cannot represent, such as a function. The + // `byteLength` call then throws and is caught here, which is why this is + // a try/catch rather than a check the type system would call redundant. + return Buffer.byteLength(JSON.stringify(value), 'utf8'); + } catch { + return 0; + } +} + /** * `ipcMain` is passed in rather than imported here. * @@ -407,4 +765,950 @@ export function registerIpcHandlers(ipcMain: IpcMain, runtime: IpcHandlerRuntime inFlightChatRequests.get(requestId)?.abort(); return chatCancelResponseSchema.parse({ acknowledged: true }); }); + + // ------------------------------------------------------------------------- + // Coding workspace (Phase 2, Milestone 5) + // + // Six channels, all read-only, all routed through the same unmodified + // `runAction` -> `handleActionProposal` pipeline every other action-backed + // channel above uses. There is no seventh channel that writes. + // ------------------------------------------------------------------------- + + /** + * The project the user approved during *this* run. + * + * Created per `registerIpcHandlers` call, exactly like + * {@link inFlightChatRequests} above and for the same reason: one test's + * approved project can never leak into another's, and one application run's + * grant never survives into the next. Nothing persists it to disk. + */ + const workspaceSession = createWorkspaceSession(); + + /** + * Runs one workspace operation as a permission-gated, audited action. + * + * The `parameters` every caller passes below are deliberately thin — + * `operation`, and at most a *length*. No path, no project name, no query + * text and no file content ever enters an audit record: the milestone brief + * requires sensitive paths and contents to be kept out of logs, and this is + * the same discipline `chat.send` already applies by recording only + * `{provider, messageCount}` rather than the conversation. + * + * A thrown {@link WorkspaceError} becomes an `ActionExecutionError` carrying + * its normalized code, which is what `execute` already extracts into + * `ActionResult.errorCode` — the same translation `secrets.write` performs + * for `SecretStoreUnavailableError`. + */ + async function runWorkspaceAction( + actionType: ActionType, + parameters: Record, + perform: (now: string) => TValue | Promise, + /** + * Builds the sentence shown in the native confirmation dialog, if the + * verdict turns out to require one (Phase 2, Milestone 6). + * + * Built here, in the main process, from state the main process already + * holds — a change set it stored, a registry entry, a fixed command + * vector — never from the request. The renderer therefore cannot + * influence what the user is asked, only that they are asked. + * + * Omitted for the read-only actions, which matches `runAction`'s own + * contract: no callback is supplied, so a `confirm` verdict reaching one + * of them fails loudly rather than silently proceeding unconfirmed. + */ + buildConfirmation?: (now: string) => string | Promise, + ): Promise> { + const now = runtime.nowFn(); + const actionRuntime = buildActionRuntime(runtime, now); + const confirmationMessage = + buildConfirmation === undefined ? null : await buildConfirmation(now); + + return runAction( + actionRuntime, + newProposal(actionType, parameters), + confirmationMessage, + async () => { + try { + return await perform(now); + } catch (error) { + if (error instanceof WorkspaceError) { + throw new ActionExecutionError(error.code, error.message); + } + throw error; + } + }, + ); + } + + ipcMain.handle(IPC_WORKSPACE_STATUS_CHANNEL, async (_event, ...args: unknown[]) => { + workspaceStatusRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'workspace.read', + { operation: 'status' }, + () => { + const project = workspaceSession.get(); + return project === null ? null : toProjectSummary(project); + }, + ); + + return workspaceStatusResponseSchema.parse(toWorkspaceProjectResponse(result)); + }); + + ipcMain.handle(IPC_WORKSPACE_SELECT_CHANNEL, async (_event, ...args: unknown[]) => { + workspaceSelectRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'workspace.select', + { operation: 'select' }, + async (now) => { + // The renderer contributed nothing to this call. The user chooses in + // a native dialog the main process owns; a compromised renderer can + // ask that the question be put, and nothing more. + const chosenPath = await runtime.selectProjectDirectory(); + if (chosenPath === null) { + // Dismissing the picker is the user declining, so it gets its own + // code rather than a generic failure — the interface shows no + // error for it, and the audit trail still records honestly that + // the action did not complete. + throw new WorkspaceError('WORKSPACE_SELECTION_CANCELLED'); + } + + const project = await adoptProjectDirectory({ + chosenPath, + now, + userDataDir: runtime.userDataPaths.userDataDir, + }); + workspaceSession.set(project); + return toProjectSummary(project); + }, + ); + + return workspaceSelectResponseSchema.parse(toWorkspaceProjectResponse(result)); + }); + + ipcMain.handle(IPC_WORKSPACE_TREE_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ path }] = workspaceTreeRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'workspace.read', + { operation: 'tree' }, + () => listProjectTree(requireApprovedProject(workspaceSession), path), + ); + + return workspaceTreeResponseSchema.parse(toWorkspaceTreeResponse(result)); + }); + + ipcMain.handle(IPC_WORKSPACE_FILE_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ path }] = workspaceFileRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'workspace.read', + { operation: 'file' }, + () => readProjectFile(requireApprovedProject(workspaceSession), path), + ); + + return workspaceFileResponseSchema.parse(toWorkspaceFileResponse(result)); + }); + + ipcMain.handle(IPC_WORKSPACE_SEARCH_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ query, path }] = workspaceSearchRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'workspace.read', + // The query's *length*, never the query itself: a search term is user + // content, and user content does not belong in a security log. + { operation: 'search', queryLength: query.length }, + () => searchProject(requireApprovedProject(workspaceSession), query, path), + ); + + return workspaceSearchResponseSchema.parse(toWorkspaceSearchResponse(result)); + }); + + ipcMain.handle(IPC_WORKSPACE_PLAN_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ objective }] = workspacePlanRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'workspace.plan', + { operation: 'plan', objectiveLength: objective.length }, + (now) => createCodingPlan(requireApprovedProject(workspaceSession), objective, now), + ); + + return workspacePlanResponseSchema.parse(toWorkspacePlanResponse(result)); + }); + + // ------------------------------------------------------------------------- + // Controlled coding actions (Phase 2, Milestone 6) + // + // Ten channels. Four of them can change something, and every one of those + // four routes through the same unmodified `runAction` -> + // `handleActionProposal` pipeline as everything above, with a confirmation + // message this process builds from state it already holds. + // ------------------------------------------------------------------------- + + /** + * Proposed changes, held in the main process for the lifetime of this run. + * + * Per `registerIpcHandlers` call, exactly like {@link workspaceSession} and + * {@link inFlightChatRequests}: one test's proposal never reaches another's, + * and one application run's proposals never survive into the next. + */ + const changeStore = createChangeStore({ backupsDir: runtime.userDataPaths.backupsDir }); + + const gitRuntime: GitRuntime = { hooksDir: runtime.userDataPaths.gitHooksDir }; + + /** + * The one command allowed to be running, and how to stop it. + * + * A map rather than a single slot so `command:cancel` can address a run by + * id without having to guess which one is current — and its size is the + * concurrency limit, checked inside `perform` where the check and the claim + * happen together and cannot race. + */ + const inFlightCommandRuns = new Map(); + + /** + * Whether the emergency stop is engaged *right now*, re-read from disk. + * + * Passed to a running command so an engaged stop kills it. The permission + * engine already refuses to start an action while the stop is engaged; this + * is the other half, without which "emergency stop" would only mean "no new + * work". + */ + async function isEmergencyEngaged(): Promise { + const state = await loadEmergencyState( + runtime.userDataPaths.emergencyStateFile, + runtime.nowFn(), + ); + return state.engaged; + } + + /** Plain, exact text for the write confirmation, built from the stored change. */ + function describeChangeSet(change: WorkspaceChangeSet | null, verb: string): string { + if (change === null) { + return `${verb} a proposed change? The change is no longer held by this session, so nothing would be written.`; + } + const paths = change.files.map((file) => ` ${file.path}`).join('\n'); + return [ + `${verb} ${String(change.files.length)} file(s) in the approved project?`, + '', + paths, + '', + `+${String(change.totalAdded)} / -${String(change.totalRemoved)} lines.`, + 'A copy of the current contents is kept so the change can be undone.', + ].join('\n'); + } + + /** + * The sentence shown before a registry command starts. + * + * Extracted so that the interface's own `command:run` and an agent run's + * verification step ask the **same** question, in the same words, built the + * same way — from the registry entry and the project's own `package.json`, + * never from a request. Two copies of a security dialog are two chances for + * them to drift. + */ + async function describeCommandRun(commandId: CommandIdValue): Promise { + const command = findCodingCommand(commandId); + const project = workspaceSession.get(); + const catalog = + project === null ? null : await buildCommandCatalog(project, false).catch(() => null); + const descriptor = catalog?.commands.find((entry) => entry.id === commandId) ?? null; + + const lines = [ + `Run a command inside the approved project?`, + '', + `Command: ${command === null ? commandId : describeCommandLine(command)}`, + `Directory: ${project === null ? '(no project approved)' : project.rootPath}`, + ]; + if (descriptor?.scriptPreview != null) { + lines.push('', `The project defines this script as:`, ` ${descriptor.scriptPreview}`); + } + if (descriptor !== null && descriptor.risks.length > 0) { + lines.push('', `Note: this script ${descriptor.risks.join(', ')}.`); + } + lines.push( + '', + 'This runs code from the project you opened. Local Agent bounds how long it may run and how much it may print, but not what it does.', + ); + return lines.join('\n'); + } + + ipcMain.handle(IPC_WORKSPACE_PROPOSE_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ edits }] = workspaceProposeRequestSchema.parse(args); + + // Producing a diff writes nothing, so it is `workspace.plan` — the same + // action type as the inert Milestone 5 plan, for the same reason. + const result = await runWorkspaceAction( + 'workspace.plan', + { operation: 'propose', fileCount: edits.length }, + (now) => + changeStore.propose({ + project: requireApprovedProject(workspaceSession), + edits, + now, + }), + ); + + return workspaceProposeResponseSchema.parse(toWorkspaceChangeResponse(result)); + }); + + ipcMain.handle(IPC_WORKSPACE_APPLY_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ changeId }] = workspaceApplyRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'workspace.write', + // The number of files, never their paths: a path names a person, a + // client or an unreleased product, and this follows the same rule the + // Milestone 5 workspace parameters already do. + { operation: 'apply', fileCount: changeStore.describe(changeId)?.files.length ?? 0 }, + (now) => + changeStore.apply({ + project: requireApprovedProject(workspaceSession), + changeId, + now, + }), + () => describeChangeSet(changeStore.describe(changeId), 'Overwrite'), + ); + + return workspaceApplyResponseSchema.parse(toWorkspaceChangeResponse(result)); + }); + + ipcMain.handle(IPC_WORKSPACE_ROLLBACK_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ changeId }] = workspaceRollbackRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'workspace.rollback', + { operation: 'rollback', fileCount: changeStore.describe(changeId)?.files.length ?? 0 }, + (now) => + changeStore.rollback({ + project: requireApprovedProject(workspaceSession), + changeId, + now, + }), + () => describeChangeSet(changeStore.describe(changeId), 'Restore the previous contents of'), + ); + + return workspaceRollbackResponseSchema.parse(toWorkspaceChangeResponse(result)); + }); + + ipcMain.handle(IPC_WORKSPACE_CHANGES_CHANNEL, async (_event, ...args: unknown[]) => { + workspaceChangesRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'workspace.read', + { operation: 'changes' }, + () => { + requireApprovedProject(workspaceSession); + return changeStore.history(); + }, + ); + + return workspaceChangesResponseSchema.parse(toWorkspaceChangesResponse(result)); + }); + + ipcMain.handle(IPC_COMMAND_LIST_CHANNEL, async (_event, ...args: unknown[]) => { + commandListRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'workspace.read', + { operation: 'commands' }, + () => + buildCommandCatalog(requireApprovedProject(workspaceSession), inFlightCommandRuns.size > 0), + ); + + return commandListResponseSchema.parse(toCommandListResponse(result)); + }); + + ipcMain.handle(IPC_COMMAND_RUN_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ runId, commandId }] = commandRunRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'command.run', + // The command's own identifier is a member of a fixed enum, not user + // content, so recording it costs nothing and makes the audit trail + // actually answer "what did it run". + { operation: 'run', commandId }, + async (now) => { + const approved = requireApprovedProject(workspaceSession); + + // Checked and claimed in the same step, inside `perform`, so two + // concurrent requests cannot both pass the check. + if (inFlightCommandRuns.size >= COMMAND_MAX_CONCURRENT_RUNS) { + throw new WorkspaceError('COMMAND_ALREADY_RUNNING'); + } + const abortController = new AbortController(); + inFlightCommandRuns.set(runId, abortController); + + try { + return await runCodingCommand({ + project: approved, + commandId, + runId, + startedAt: now, + finishedAtFn: runtime.nowFn, + signal: abortController.signal, + isEmergencyEngaged, + }); + } finally { + inFlightCommandRuns.delete(runId); + } + }, + () => describeCommandRun(commandId), + ); + + return commandRunResponseSchema.parse(toCommandRunResponse(result)); + }); + + ipcMain.handle(IPC_COMMAND_CANCEL_CHANNEL, (_event, ...args: unknown[]) => { + const [{ runId }] = commandCancelRequestSchema.parse(args); + // Best-effort and idempotent, exactly like `chat:cancel`, and ungated for + // the same reason: this cannot start an action, read anything or reach + // anything — it can only ask an already-authorized run to stop early. + inFlightCommandRuns.get(runId)?.abort(); + return commandCancelResponseSchema.parse({ acknowledged: true }); + }); + + ipcMain.handle(IPC_GIT_STATUS_CHANNEL, async (_event, ...args: unknown[]) => { + gitStatusRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'git.read', + { operation: 'status' }, + () => readGitStatus(requireApprovedProject(workspaceSession), gitRuntime), + ); + + return gitStatusResponseSchema.parse(toGitStatusResponse(result)); + }); + + ipcMain.handle(IPC_GIT_DIFF_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ path }] = gitDiffRequestSchema.parse(args); + + const result = await runWorkspaceAction( + 'git.read', + // Whether the diff was narrowed, never to what: a path is user content. + { operation: 'diff', scoped: path !== null }, + () => readGitDiff(requireApprovedProject(workspaceSession), gitRuntime, path), + ); + + return gitDiffResponseSchema.parse(toGitDiffResponse(result)); + }); + + ipcMain.handle(IPC_GIT_CHECKPOINT_CHANNEL, async (_event, ...args: unknown[]) => { + gitCheckpointRequestSchema.parse(args); + + const project = workspaceSession.get(); + + const result = await runWorkspaceAction( + 'git.checkpoint', + { operation: 'checkpoint' }, + (now) => createGitCheckpoint(requireApprovedProject(workspaceSession), gitRuntime, now), + (now) => + [ + 'Create a Git checkpoint commit in the approved project?', + '', + `Directory: ${project === null ? '(no project approved)' : project.rootPath}`, + '', + 'Exactly these commands will run, and nothing else:', + describeCheckpoint(now), + '', + 'Nothing is reset, checked out, deleted or pushed. Repository hooks are disabled for this commit.', + ].join('\n'), + ); + + return gitCheckpointResponseSchema.parse(toGitCheckpointResponse(result)); + }); + + // ------------------------------------------------------------------------- + // Agent profiles and runs (Phase 2, Milestone 7) + // + // Eight channels, and not one of them is a new capability. Six read or + // change *configuration*. `agent:run` executes a bounded sequence of the + // actions this file already offered, each one still routed through the same + // unmodified `runAction` -> `handleActionProposal` pipeline, decided on its + // own action type, confirmed natively where the floor requires it, and + // written to the audit log. `main/permissions.ts`, `main/executor.ts`, + // `main/action-pipeline.ts` and `main/audit.ts` have zero diff for the + // sixth consecutive milestone. + // ------------------------------------------------------------------------- + + /** + * The one run allowed to be in flight, and how to stop it. + * + * Per `registerIpcHandlers` call, exactly like {@link inFlightCommandRuns}: + * one test's run never reaches another's, and one application run's + * cancellation handle never survives into the next. + */ + const inFlightAgentRuns = new Map(); + + /** + * Runs one agent operation as a permission-gated, audited action. + * + * The same shape as {@link runWorkspaceAction}, differing only in which + * normalized error vocabulary it translates. A `WorkspaceError` is + * translated too, because a run's steps reach workspace code and a failure + * there must not escape as a raw error. + */ + async function runAgentAction( + actionType: ActionType, + parameters: Record, + perform: (now: string) => TValue | Promise, + buildConfirmation?: (now: string) => string | Promise, + ): Promise> { + const now = runtime.nowFn(); + const actionRuntime = buildActionRuntime(runtime, now); + const confirmationMessage = + buildConfirmation === undefined ? null : await buildConfirmation(now); + + return runAction( + actionRuntime, + newProposal(actionType, parameters), + confirmationMessage, + async () => { + try { + return await perform(now); + } catch (error) { + if (error instanceof AgentError) { + throw new ActionExecutionError(error.code, error.message); + } + if (error instanceof WorkspaceError) { + throw new ActionExecutionError(error.code, error.message); + } + throw error; + } + }, + ); + } + + /** + * The sentence shown before a profile is created, changed or removed. + * + * States what the profile would be allowed to reach for, in the same terms + * the run dialog uses, because "what does this profile permit" is the only + * question that matters when approving a profile edit. The name is + * sanitized before it is interpolated: it is user-typed text about to be + * shown inside a security prompt, which is the worst place for something + * that can move the cursor or reorder itself. + */ + function describeProfileWrite( + verb: string, + profileId: string, + input: AgentProfileInput | null, + ): string { + const lines = [`${verb} the agent profile "${profileId}"?`]; + + if (input !== null) { + const tools = + input.allowedTools.length === 0 + ? [' (no tools — this profile would be able to do nothing)'] + : input.allowedTools.map((toolId) => { + const tool = findAgentTool(toolId); + return ` ${tool === null ? toolId : `${tool.label} (${tool.actionType})`}`; + }); + const scope = + input.approvedWorkspacePaths.length === 0 + ? [' (nothing)'] + : input.approvedWorkspacePaths.map((path) => + path === '' ? ' the whole approved project' : ` ${path}`, + ); + + lines.push( + '', + `Name: ${collapseToSingleLine(input.name, AGENT_NAME_MAX_LENGTH)}`, + '', + 'It would be allowed to use:', + ...tools, + '', + 'Limited to these parts of the approved project:', + ...scope, + '', + `Per run: at most ${String(input.limits.maxSteps)} steps, ${String(Math.round(input.limits.maxDurationMs / 1000))}s, and ${String(input.limits.maxOutputBytes)} bytes of output.`, + ); + } + + lines.push( + '', + 'A profile can only narrow what the permission policy already allows. It cannot grant a permission, and there is no tool it can name that writes a file, undoes a write, or creates a commit.', + ); + + return lines.join('\n'); + } + + /** The sentence shown when a profile itself asks for a step to be confirmed. */ + function describeProfileGatedStep(request: AgentStepRequest): string { + return [ + 'The active agent profile requires confirmation before this step.', + '', + `Step: ${request.tool.label}`, + `Action: ${request.tool.actionType}`, + '', + 'Approving this skips nothing else: the permission policy still decides the action afterwards, and a confirmation the policy itself requires is still asked separately.', + ].join('\n'); + } + + /** Bounded, single-line, control-character-free wording for a step record. */ + function stepSummary(text: string): string { + return collapseToSingleLine(text, AGENT_STEP_SUMMARY_MAX_LENGTH); + } + + /** + * Which provider this profile resolves to, given what is actually + * configured right now. + * + * A provider is usable only when it is the one selected in settings *and*, + * if it authenticates with a key, a key is actually present — read from the + * reconciled settings, so a stale `hasApiKey` cannot make an unusable + * provider look available. Nothing is called: this only records what would + * carry a model request once one exists. + */ + async function resolveRunProvider(profile: AgentProfile, now: string): Promise { + const reconciled = await readReconciledSettings(runtime.userDataPaths, now); + return resolveAgentProvider(profile, (candidate) => { + if (candidate !== reconciled.modelProvider.provider) return false; + if (!PROVIDERS_REQUIRING_API_KEY.includes(candidate)) return true; + return reconciled.modelProvider.hasApiKey; + }); + } + + /** + * Builds the step executor for one run. + * + * A closure over the run's `AbortSignal` so that cancelling the run also + * kills a child process the run started — without it, `agent:cancel` would + * mean "take no further steps" while `npm test` kept running. + * + * Every branch below dispatches through {@link runWorkspaceAction}, the + * same function the interface's own channels use. There is no path here + * that reaches a workspace, command or Git operation without a permission + * decision and an audit record. + */ + function createAgentStepRunner( + signal: AbortSignal, + ): (request: AgentStepRequest) => Promise { + function settle( + result: ActionResult, + describe: (value: TValue) => string, + ): AgentStepOutcome { + if (result.outcome === 'success' && result.value !== undefined) { + return { + outcome: 'success', + summary: stepSummary(describe(result.value)), + outputBytes: measureOutputBytes(result.value), + }; + } + + const errorCode = toWorkspaceErrorCode(result); + const summary = + result.outcome === 'denied' + ? 'Refused before it ran, by the permission policy or the emergency stop.' + : result.outcome === 'aborted' + ? 'The confirmation was declined, so nothing ran.' + : 'The step did not complete.'; + + return { + outcome: result.outcome, + summary: stepSummary(summary), + ...(errorCode === undefined ? {} : { errorCode }), + outputBytes: 0, + }; + } + + async function runCommandStep(commandId: CommandIdValue): Promise { + const stepRunId = randomUUID(); + + const result = await runWorkspaceAction( + 'command.run', + { operation: 'agent-command', commandId }, + async (now) => { + const approved = requireApprovedProject(workspaceSession); + + if (inFlightCommandRuns.size >= COMMAND_MAX_CONCURRENT_RUNS) { + throw new WorkspaceError('COMMAND_ALREADY_RUNNING'); + } + + const controller = new AbortController(); + const forward = (): void => { + controller.abort(); + }; + if (signal.aborted) controller.abort(); + else signal.addEventListener('abort', forward, { once: true }); + inFlightCommandRuns.set(stepRunId, controller); + + try { + return await runCodingCommand({ + project: approved, + commandId, + runId: stepRunId, + startedAt: now, + finishedAtFn: runtime.nowFn, + signal: controller.signal, + isEmergencyEngaged, + }); + } finally { + signal.removeEventListener('abort', forward); + inFlightCommandRuns.delete(stepRunId); + } + }, + // The same dialog, in the same words, the interface's own + // `command:run` shows. A verification step inside an agent run does + // not get a quieter path to starting a process than a person does. + () => describeCommandRun(commandId), + ); + + if (result.outcome === 'success' && result.value !== undefined) { + const run = result.value; + // A command that ran and exited non-zero is a failure *of the thing + // being checked*, and the step records it as `failure` so it cannot + // satisfy a verification requirement. The action itself succeeded, + // which is why `result.outcome` is not what is reported here. + return { + outcome: run.outcome === 'succeeded' ? 'success' : 'failure', + summary: stepSummary( + `${run.commandLine}: ${run.outcome} (exit ${run.exitCode === null ? 'none' : String(run.exitCode)}).`, + ), + outputBytes: measureOutputBytes(run.output), + }; + } + + return settle(result, () => 'The command ran.'); + } + + return async (request: AgentStepRequest): Promise => { + // The profile's own extra gate, in front of the pipeline rather than + // instead of it. A profile can only ever *add* this prompt: its + // decision vocabulary has no `allow`, so nothing here can remove a + // confirmation the floor requires or turn a denial into a permission. + if (request.requiresConfirmation) { + const answer = await runtime.requestConfirmation(describeProfileGatedStep(request)); + if (answer !== 'approved') { + return { + outcome: 'aborted', + summary: stepSummary(`Declined: ${request.tool.label}.`), + outputBytes: 0, + }; + } + } + + switch (request.step.tool) { + case 'workspace.inspect': { + const result = await runWorkspaceAction( + 'workspace.read', + { operation: 'agent-tree' }, + () => listProjectTree(requireApprovedProject(workspaceSession), request.step.target), + ); + return settle( + result, + (tree) => + `Listed ${String(tree.entries.length)} entries${tree.truncated ? ' (truncated)' : ''}.`, + ); + } + + case 'workspace.search': { + const query = request.step.query ?? ''; + const result = await runWorkspaceAction( + 'workspace.read', + // The query's length, never the query: the same rule the + // interface's own search channel already follows. + { operation: 'agent-search', queryLength: query.length }, + () => + searchProject(requireApprovedProject(workspaceSession), query, request.step.target), + ); + return settle( + result, + (search) => + `Found ${String(search.matches.length)} matches across ${String(search.filesScanned)} files.`, + ); + } + + case 'workspace.plan': { + const result = await runWorkspaceAction( + 'workspace.plan', + { operation: 'agent-plan', objectiveLength: request.objective.length }, + (now) => + createCodingPlan(requireApprovedProject(workspaceSession), request.objective, now), + ); + return settle( + result, + (plan) => + `Produced a plan with ${String(plan.steps.length)} steps and ${String(plan.relevantFiles.length)} relevant files.`, + ); + } + + case 'git.status': { + const result = await runWorkspaceAction( + 'git.read', + { operation: 'agent-git-status' }, + () => readGitStatus(requireApprovedProject(workspaceSession), gitRuntime), + ); + return settle(result, (status) => + status.clean + ? 'The working tree is clean.' + : `${String(status.entries.length)} changed paths in the working tree.`, + ); + } + + case 'command.test': + case 'command.lint': + case 'command.typecheck': { + const commandId = request.tool.commandId; + if (commandId === null) { + return { + outcome: 'failure', + summary: stepSummary('That tool names no command to run.'), + outputBytes: 0, + }; + } + return runCommandStep(commandId); + } + } + }; + } + + ipcMain.handle(IPC_AGENT_LIST_CHANNEL, async (_event, ...args: unknown[]) => { + agentListRequestSchema.parse(args); + + const result = await runAgentAction('agent.read', { operation: 'list' }, () => + readAgentRegistry(runtime.userDataPaths.agentProfilesFile), + ); + + return agentListResponseSchema.parse(toAgentRegistryResponse(result)); + }); + + ipcMain.handle(IPC_AGENT_SELECT_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ profileId }] = agentSelectRequestSchema.parse(args); + + const result = await runAgentAction( + // Selecting cannot widen anything: every action a run later takes is + // still decided by the permission engine against the same policy. That + // is why this is its own action type rather than `agent.write`, and why + // it is not on the confirmation floor — a native dialog for every + // selection would train people to click through dialogs, which is its + // own security problem. + 'agent.select', + { operation: 'select', profileId }, + () => selectAgentProfile(runtime.userDataPaths.agentProfilesFile, profileId), + ); + + return agentSelectResponseSchema.parse(toAgentRegistryResponse(result)); + }); + + ipcMain.handle(IPC_AGENT_CREATE_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ profile }] = agentCreateRequestSchema.parse(args); + + const result = await runAgentAction( + 'agent.write', + { operation: 'create', profileId: profile.id, toolCount: profile.allowedTools.length }, + (now) => createAgentProfile(runtime.userDataPaths.agentProfilesFile, profile, now), + () => describeProfileWrite('Create', profile.id, profile), + ); + + return agentCreateResponseSchema.parse(toAgentRegistryResponse(result)); + }); + + ipcMain.handle(IPC_AGENT_UPDATE_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ profileId, profile }] = agentUpdateRequestSchema.parse(args); + + const result = await runAgentAction( + 'agent.write', + { operation: 'update', profileId, toolCount: profile.allowedTools.length }, + (now) => updateAgentProfile(runtime.userDataPaths.agentProfilesFile, profileId, profile, now), + () => describeProfileWrite('Replace', profileId, profile), + ); + + return agentUpdateResponseSchema.parse(toAgentRegistryResponse(result)); + }); + + ipcMain.handle(IPC_AGENT_DELETE_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ profileId }] = agentDeleteRequestSchema.parse(args); + + const result = await runAgentAction( + 'agent.write', + { operation: 'delete', profileId }, + () => deleteAgentProfile(runtime.userDataPaths.agentProfilesFile, profileId), + () => describeProfileWrite('Delete', profileId, null), + ); + + return agentDeleteResponseSchema.parse(toAgentRegistryResponse(result)); + }); + + ipcMain.handle(IPC_AGENT_SET_ENABLED_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ profileId, enabled }] = agentSetEnabledRequestSchema.parse(args); + + const result = await runAgentAction( + 'agent.write', + { operation: 'set-enabled', profileId, enabled }, + (now) => + setAgentProfileEnabled(runtime.userDataPaths.agentProfilesFile, profileId, enabled, now), + () => describeProfileWrite(enabled ? 'Enable' : 'Disable', profileId, null), + ); + + return agentSetEnabledResponseSchema.parse(toAgentRegistryResponse(result)); + }); + + ipcMain.handle(IPC_AGENT_RUN_CHANNEL, async (_event, ...args: unknown[]) => { + const [{ runId, objective }] = agentRunRequestSchema.parse(args); + + // Read once, up front, so the dialog the user reads and the run that + // follows describe the same profile — and read from the *store*, never + // from the request, so a compromised renderer cannot widen a run by + // describing it differently. + const registry = await readAgentRegistry(runtime.userDataPaths.agentProfilesFile); + const profile = resolveActiveProfile(registry); + const plan = buildAgentPlan(profile, objective); + const provider = await resolveRunProvider(profile, runtime.nowFn()); + + const result = await runAgentAction( + 'agent.run', + { + operation: 'run', + // The profile id, never the objective. An id is a bounded, lowercase + // slug the schema constrains to `[a-z0-9._-]`, and recording it is + // what lets the audit trail answer "which profile authorized this" — + // the same reasoning that puts `commandId` in a command record. The + // objective is user content, so only its length is recorded. + profileId: profile.id, + plannedSteps: plan.steps.length, + objectiveLength: objective.length, + }, + async (now) => { + if (inFlightAgentRuns.size > 0) throw new AgentError('AGENT_RUN_ALREADY_RUNNING'); + if (!profile.enabled) throw new AgentError('AGENT_PROFILE_DISABLED'); + + const controller = new AbortController(); + inFlightAgentRuns.set(runId, controller); + + try { + return await runAgentOrchestration({ + runId, + profile, + objective, + provider, + startedAt: now, + nowFn: runtime.nowFn, + monotonicMs: () => Date.now(), + signal: controller.signal, + isEmergencyEngaged, + hasProject: () => workspaceSession.get() !== null, + runStep: createAgentStepRunner(controller.signal), + }); + } finally { + inFlightAgentRuns.delete(runId); + } + }, + () => describeAgentRun(profile, plan.steps.length), + ); + + return agentRunResponseSchema.parse(toAgentRunResponse(result)); + }); + + ipcMain.handle(IPC_AGENT_CANCEL_CHANNEL, (_event, ...args: unknown[]) => { + const [{ runId }] = agentCancelRequestSchema.parse(args); + // Best-effort and idempotent, exactly like `chat:cancel` and + // `command:cancel`, and ungated for the same reason: it cannot start an + // action, read anything or reach anything — only ask an + // already-authorized run to stop early. Aborting the run's signal also + // kills a child process the run had started. + inFlightAgentRuns.get(runId)?.abort(); + return agentCancelResponseSchema.parse({ acknowledged: true }); + }); } diff --git a/src/main/paths.ts b/src/main/paths.ts index 90aa1dc..be54561 100644 --- a/src/main/paths.ts +++ b/src/main/paths.ts @@ -28,6 +28,12 @@ export interface UserDataPaths { readonly auditLogDir: string; readonly emergencyStateFile: string; readonly memoryDir: string; + /** Pre-change backups (Phase 2, Milestone 6). Never inside a user project. */ + readonly backupsDir: string; + /** The empty directory `git` is pointed at instead of a repository's hooks. */ + readonly gitHooksDir: string; + /** Agent profiles (Phase 2, Milestone 7). Holds no credential. */ + readonly agentProfilesFile: string; } /** @@ -47,5 +53,8 @@ export function resolveUserDataPaths(appDataDir: string): UserDataPaths { auditLogDir: join(userDataDir, USER_DATA_PATHS.auditLogDir), emergencyStateFile: join(userDataDir, USER_DATA_PATHS.emergencyStateFile), memoryDir: join(userDataDir, USER_DATA_PATHS.memoryDir), + backupsDir: join(userDataDir, USER_DATA_PATHS.backupsDir), + gitHooksDir: join(userDataDir, USER_DATA_PATHS.gitHooksDir), + agentProfilesFile: join(userDataDir, USER_DATA_PATHS.agentProfilesFile), }; } diff --git a/src/main/process-runner.ts b/src/main/process-runner.ts new file mode 100644 index 0000000..c055c00 --- /dev/null +++ b/src/main/process-runner.ts @@ -0,0 +1,499 @@ +/** + * The bounded process runner (Phase 2, Milestone 6). + * + * The only module in this codebase that starts a process. It is deliberately + * small and deliberately dumb: it takes a program, an argument array and a + * working directory, and it runs them under a set of limits. It makes no + * decision about *what* may run — `shared/workspace/command-registry.ts` and + * `shared/workspace/git.ts` own that, and the permission engine plus a native + * confirmation own whether it happens at all. + * + * ## No shell, ever + * + * `spawn` is called with `shell: false` in every case. That is the difference + * between running a program with an argument vector and handing a string to an + * interpreter that will re-split it on characters this application never + * intended as syntax. + * + * Windows makes this awkward rather than impossible: `npm` is a `.cmd` script, + * and Node refuses to spawn a `.cmd` without a shell (the fix for + * CVE-2024-27980). So {@link resolveProgram} finds the real file on `PATH` + * itself, and a batch file is run by invoking the command interpreter with an + * explicit argument array — never a command string. Because the interpreter + * does its own parsing, every argument is additionally checked against + * {@link SAFE_ARGUMENT_PATTERN} first, and a run is refused outright rather + * than quoted-and-hoped-for if any argument contains a character the + * interpreter treats as syntax. In practice no argument ever does: every one + * comes from a literal in the registry or from a path this codebase built. + * + * ## Nothing is unbounded + * + * | Dimension | Behaviour when the bound is reached | + * | ---------------- | -------------------------------------------------------- | + * | Wall-clock time | The process tree is killed; `timedOut` is reported | + * | Output bytes | Capture stops, draining continues; `outputTruncated` | + * | Output lines | Same | + * | Line length | The line is elided, marked with the truncation marker | + * | Concurrency | Owned by the caller — this module runs what it is given | + * + * Draining rather than pausing matters: a child whose stdout pipe fills up + * blocks forever, so output past the cap is read and discarded rather than + * left unread. + * + * ## Two things the child does not get + * + * - **A terminal.** `stdin` is `'ignore'`, so a command that decides to + * prompt reads end-of-file and exits instead of hanging until its timeout. + * - **The parent's environment.** Only the variables in + * {@link INHERITED_ENVIRONMENT_NAMES} are passed through. A user who has + * exported an API token into their shell does not hand it to a project's + * build script because they asked Local Agent to run one. + * + * ## The emergency stop reaches a running process + * + * The permission engine already refuses to *start* an action while the stop is + * engaged. `isEmergencyEngaged` closes the other half: it is polled while the + * process runs, and an engaged stop kills it. Without that, "emergency stop" + * would mean "no new work" rather than "stop". + */ + +import { spawn } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; +import { accessSync, constants } from 'node:fs'; +import { delimiter, isAbsolute, join } from 'node:path'; + +import { + COMMAND_EMERGENCY_POLL_MS, + COMMAND_KILL_GRACE_MS, + COMMAND_MAX_OUTPUT_LINE_LENGTH, +} from '../shared/constants'; +import type { CommandOutputLine } from '../shared/schemas/coding.schema'; +import { WorkspaceError } from '../shared/workspace/errors'; +import { sanitizeDisplayLine } from '../shared/workspace/text-safety'; + +/** + * Environment variables passed through to a child process. + * + * Everything else is dropped. The list is what a Node or Git toolchain + * genuinely needs to function on Windows and on POSIX, and nothing more — + * notably no `*_TOKEN`, no `*_KEY`, no `NODE_OPTIONS` (which can inject a + * module into every child), and no `GIT_*` beyond what is set explicitly + * below. + */ +export const INHERITED_ENVIRONMENT_NAMES: readonly string[] = [ + 'PATH', + 'PATHEXT', + 'SystemRoot', + 'SystemDrive', + 'windir', + 'ComSpec', + 'TEMP', + 'TMP', + 'HOME', + 'HOMEDRIVE', + 'HOMEPATH', + 'USERPROFILE', + 'APPDATA', + 'LOCALAPPDATA', + 'ProgramData', + 'ProgramFiles', + 'ProgramFiles(x86)', + 'ProgramW6432', + 'NUMBER_OF_PROCESSORS', + 'PROCESSOR_ARCHITECTURE', + 'OS', + 'LANG', + 'LC_ALL', + 'TZ', +] as const; + +/** + * Variables set on every child, regardless of the parent's environment. + * + * `CI` and `NO_COLOR` between them stop a tool drawing progress spinners or + * ANSI colour that would only be sanitized away, and stop it deciding to ask + * an interactive question. `GIT_TERMINAL_PROMPT=0` makes git fail rather than + * block if it ever decides it wants credentials — which it should never do + * here, since nothing this application runs talks to a remote. + */ +const FORCED_ENVIRONMENT: Readonly> = { + CI: '1', + NO_COLOR: '1', + FORCE_COLOR: '0', + GIT_TERMINAL_PROMPT: '0', + GIT_ASKPASS: '', + npm_config_color: 'false', + npm_config_progress: 'false', + npm_config_audit: 'false', + npm_config_fund: 'false', +}; + +/** + * Characters an argument may contain when the command interpreter is going to + * re-parse the argument vector. + * + * Everything the interpreter treats as syntax is absent: `&`, `|`, `<`, `>`, + * `^`, `(`, `)`, `%`, `!`, `"`, backtick, `$`, `;` and every control + * character. A run whose arguments do not all match is refused rather than + * escaped, because escaping correctly for two different interpreters is a + * problem this application does not need to have. + */ +export const SAFE_ARGUMENT_PATTERN = /^[A-Za-z0-9 _.:,=+@/\\[\]{}'#~-]*$/; + +/** What {@link resolveProgram} decided to actually execute. */ +export interface ResolvedProgram { + /** The file `spawn` is given: the program itself, or the interpreter. */ + readonly file: string; + /** The real program. Equal to {@link ResolvedProgram.file} when run directly. */ + readonly target: string; + /** True when a command interpreter will re-parse the argument vector. */ + readonly viaInterpreter: boolean; +} + +/** What `spawn` should actually be handed, once the arguments are known. */ +export interface SpawnArguments { + readonly argv: readonly string[]; + /** True when Node must not re-quote the vector. See {@link buildSpawnArguments}. */ + readonly verbatim: boolean; +} + +/** + * Assembles the final argument vector. + * + * The direct case is trivial: `spawn` is given the program and the arguments, + * and quotes each one itself. + * + * The interpreter case is not, and the reason is a genuine Windows quirk + * rather than a stylistic choice. `cmd.exe /s /c` strips the **first and last** + * quote characters of everything after `/c` and runs what is left verbatim. + * So handing it `/c "C:\Program Files\nodejs\npm.cmd" run test` yields the + * command `C:\Program Files\nodejs\npm.cmd" run test` — the path is split at + * the space and the run fails with `'C:\Program' is not recognized`. The + * documented fix, and what Node's own `shell: true` does, is to wrap the + * entire remainder in one further pair of quotes so that the pair `/s` strips + * is that outer one. + * + * That requires `windowsVerbatimArguments`, because Node would otherwise + * re-quote the string it was handed. Quoting each token here is safe only + * because {@link SAFE_ARGUMENT_PATTERN} has already refused every character + * the interpreter treats as syntax — including `"` itself, `%`, `^` and `!` — + * so there is no escape sequence to get right and nothing to get wrong. + */ +export function buildSpawnArguments( + resolved: ResolvedProgram, + args: readonly string[], +): SpawnArguments { + if (!resolved.viaInterpreter) { + return { argv: [...args], verbatim: false }; + } + + const quoted = [resolved.target, ...args].map((value) => `"${value}"`).join(' '); + return { argv: ['/d', '/s', '/c', `"${quoted}"`], verbatim: true }; +} + +function isExecutableFile(candidate: string): boolean { + try { + accessSync(candidate, constants.F_OK); + return true; + } catch { + return false; + } +} + +/** + * Finds the real file for a bare program name on `PATH`. + * + * Done here rather than left to `spawn` because Windows needs the answer: a + * `.cmd` cannot be spawned directly, and knowing which extension was found is + * what decides between running the program and running it through the command + * interpreter. On POSIX the bare name is handed to `spawn`, which resolves it + * the same way any exec would. + * + * `program` is always a literal from the registry (`npm`, `git`), never a + * value from a request — but an absolute path is accepted too, so a test can + * point this at `process.execPath` and exercise the real spawn path. + */ +export function resolveProgram(program: string, env: NodeJS.ProcessEnv): ResolvedProgram { + const isWindows = process.platform === 'win32'; + + if (isAbsolute(program) || !isWindows) { + return { file: program, target: program, viaInterpreter: false }; + } + + const pathValue = env.PATH ?? env.Path ?? ''; + const extensions = (env.PATHEXT ?? '.COM;.EXE;.BAT;.CMD').split(';').filter(Boolean); + const directories = pathValue.split(delimiter).filter(Boolean); + + for (const directory of directories) { + for (const extension of extensions) { + const candidate = join(directory, program + extension.toLowerCase()); + if (!isExecutableFile(candidate)) continue; + + const lowered = candidate.toLowerCase(); + if (lowered.endsWith('.cmd') || lowered.endsWith('.bat')) { + // A batch file cannot be spawned directly (the fix for + // CVE-2024-27980), so it runs through the command interpreter. `/d` + // skips AutoRun commands from the registry — otherwise a machine + // configuration could inject a command into every run — and `/s` + // fixes how the remainder is quoted; see {@link buildSpawnArguments}. + return { file: env.ComSpec ?? 'cmd.exe', target: candidate, viaInterpreter: true }; + } + return { file: candidate, target: candidate, viaInterpreter: false }; + } + } + + throw new WorkspaceError('COMMAND_LAUNCH_FAILED'); +} + +/** The environment a child receives: an allowlist, plus fixed overrides. */ +export function buildChildEnvironment(parent: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const child: NodeJS.ProcessEnv = {}; + const wanted = new Map(INHERITED_ENVIRONMENT_NAMES.map((name) => [name.toLowerCase(), name])); + + for (const [name, value] of Object.entries(parent)) { + if (value === undefined) continue; + const canonical = wanted.get(name.toLowerCase()); + if (canonical === undefined) continue; + child[name] = value; + } + for (const [name, value] of Object.entries(FORCED_ENVIRONMENT)) child[name] = value; + return child; +} + +export interface RunProcessOptions { + readonly program: string; + readonly args: readonly string[]; + /** Always the approved project root. Nothing runs anywhere else. */ + readonly cwd: string; + readonly timeoutMs: number; + readonly maxOutputBytes: number; + readonly maxOutputLines: number; + /** Aborting kills the process tree. Supplied by the caller, as `chat:send` does. */ + readonly signal?: AbortSignal; + /** Polled while the process runs; a `true` answer kills it. */ + readonly isEmergencyEngaged?: () => Promise; + readonly env?: NodeJS.ProcessEnv; +} + +export interface ProcessRunResult { + /** `null` when the process was killed rather than exiting on its own. */ + readonly exitCode: number | null; + readonly output: readonly CommandOutputLine[]; + readonly outputTruncated: boolean; + readonly timedOut: boolean; + readonly cancelled: boolean; + readonly stoppedByEmergency: boolean; + readonly durationMs: number; +} + +/** + * Kills a process and, on Windows, everything it started. + * + * `child.kill()` signals only the process itself. A `npm run test` on Windows + * is an interpreter that started Node that started a test runner, and killing + * the first leaves the rest running — orphaned processes holding file locks + * inside the user's project. `taskkill /T` is the platform's own answer, with + * a fixed argument vector and a process id this module owns. + */ +function killProcessTree(child: ChildProcess): void { + const pid = child.pid; + child.kill(); + + if (process.platform === 'win32' && typeof pid === 'number' && Number.isInteger(pid) && pid > 0) { + try { + const killer = spawn('taskkill', ['/pid', String(pid), '/t', '/f'], { + stdio: 'ignore', + shell: false, + windowsHide: true, + }); + killer.on('error', () => undefined); + } catch { + // Best effort: the direct kill above has already been sent. + } + return; + } + + setTimeout(() => { + if (child.exitCode === null && child.signalCode === null) child.kill('SIGKILL'); + }, COMMAND_KILL_GRACE_MS).unref(); +} + +/** Accumulates one stream's bytes into bounded, sanitized lines. */ +class OutputCollector { + private readonly lines: CommandOutputLine[] = []; + private readonly partial = new Map(); + private bytes = 0; + private truncatedFlag = false; + + constructor( + private readonly maxBytes: number, + private readonly maxLines: number, + ) {} + + get truncated(): boolean { + return this.truncatedFlag; + } + + push(stream: CommandOutputLine['stream'], chunk: Buffer): void { + this.bytes += chunk.length; + if (this.bytes > this.maxBytes || this.lines.length >= this.maxLines) { + // Past the bound the chunk is still consumed — the child would block on + // a full pipe otherwise — but nothing more is kept. + this.truncatedFlag = true; + return; + } + + const text = (this.partial.get(stream) ?? '') + chunk.toString('utf8'); + const parts = text.split('\n'); + this.partial.set(stream, parts.pop() ?? ''); + + for (const part of parts) { + if (this.lines.length >= this.maxLines) { + this.truncatedFlag = true; + return; + } + this.append(stream, part); + } + } + + /** Flushes whatever was left without a trailing newline. */ + finish(): readonly CommandOutputLine[] { + for (const [stream, remainder] of this.partial) { + if (remainder.length === 0) continue; + if (this.lines.length >= this.maxLines) { + this.truncatedFlag = true; + break; + } + this.append(stream, remainder); + } + this.partial.clear(); + return this.lines; + } + + private append(stream: CommandOutputLine['stream'], raw: string): void { + const withoutCarriageReturn = raw.endsWith('\r') ? raw.slice(0, -1) : raw; + const sanitized = sanitizeDisplayLine(withoutCarriageReturn, COMMAND_MAX_OUTPUT_LINE_LENGTH); + if (sanitized.truncated) this.truncatedFlag = true; + this.lines.push({ stream, text: sanitized.text }); + } +} + +/** + * Runs one process to completion, or until a bound stops it. + * + * Never throws for a non-zero exit — a failing test suite is a result, not an + * error. It throws {@link WorkspaceError} only when the process could not be + * started at all. + */ +export async function runProcess(options: RunProcessOptions): Promise { + const { + program, + args, + cwd, + timeoutMs, + maxOutputBytes, + maxOutputLines, + signal, + isEmergencyEngaged, + } = options; + + const parentEnvironment = options.env ?? process.env; + const env = buildChildEnvironment(parentEnvironment); + const resolved = resolveProgram(program, parentEnvironment); + + if ( + resolved.viaInterpreter && + ![resolved.target, ...args].every((value) => SAFE_ARGUMENT_PATTERN.test(value)) + ) { + // Refused rather than escaped: see SAFE_ARGUMENT_PATTERN. Checked before + // the quoting in `buildSpawnArguments`, which is only safe because this + // has already run. + throw new WorkspaceError('COMMAND_LAUNCH_FAILED'); + } + + const spawnArguments = buildSpawnArguments(resolved, args); + const startedAtMs = Date.now(); + const collector = new OutputCollector(maxOutputBytes, maxOutputLines); + + let child: ChildProcess; + try { + child = spawn(resolved.file, [...spawnArguments.argv], { + cwd, + env, + shell: false, + windowsHide: true, + windowsVerbatimArguments: spawnArguments.verbatim, + // No terminal: a command that decides to prompt reads end-of-file. + stdio: ['ignore', 'pipe', 'pipe'], + }); + } catch { + throw new WorkspaceError('COMMAND_LAUNCH_FAILED'); + } + + return new Promise((resolvePromise, rejectPromise) => { + let timedOut = false; + let cancelled = false; + let stoppedByEmergency = false; + let settled = false; + + child.stdout?.on('data', (chunk: Buffer) => { + collector.push('stdout', chunk); + }); + child.stderr?.on('data', (chunk: Buffer) => { + collector.push('stderr', chunk); + }); + + const timer = setTimeout(() => { + timedOut = true; + killProcessTree(child); + }, timeoutMs); + timer.unref(); + + const onAbort = (): void => { + cancelled = true; + killProcessTree(child); + }; + signal?.addEventListener('abort', onAbort, { once: true }); + + const emergencyTimer = + isEmergencyEngaged === undefined + ? null + : setInterval(() => { + void isEmergencyEngaged() + .then((engaged) => { + if (!engaged || settled) return; + stoppedByEmergency = true; + killProcessTree(child); + }) + .catch(() => undefined); + }, COMMAND_EMERGENCY_POLL_MS); + emergencyTimer?.unref(); + + function cleanUp(): void { + settled = true; + clearTimeout(timer); + if (emergencyTimer !== null) clearInterval(emergencyTimer); + signal?.removeEventListener('abort', onAbort); + } + + child.on('error', () => { + if (settled) return; + cleanUp(); + rejectPromise(new WorkspaceError('COMMAND_LAUNCH_FAILED')); + }); + + child.on('close', (code) => { + if (settled) return; + cleanUp(); + resolvePromise({ + exitCode: code, + output: collector.finish(), + outputTruncated: collector.truncated, + timedOut, + cancelled, + stoppedByEmergency, + durationMs: Math.max(0, Date.now() - startedAtMs), + }); + }); + }); +} diff --git a/src/main/project-commands.ts b/src/main/project-commands.ts new file mode 100644 index 0000000..61eb44c --- /dev/null +++ b/src/main/project-commands.ts @@ -0,0 +1,214 @@ +/** + * Resolving and running the registry's commands (Phase 2, Milestone 6). + * + * `shared/workspace/command-registry.ts` says which five commands exist. This + * module answers the two questions that need the disk: does *this* project + * actually declare the script a command would run, and what happened when it + * ran. + * + * ## Reading `package.json` is reading untrusted input + * + * It is a file from a directory the user merely opened, so it goes through the + * same read path as any other project file — `readProjectFile`, with its + * containment, exclusion, size and encoding rules — and then through the same + * `__proto__`/`constructor`/`prototype` guard every other JSON loader in this + * codebase applies before anything looks at the parsed value. + * + * What is taken from it is deliberately minimal: **whether** a script of a + * fixed name exists, and its text, sanitized to one bounded line, for the + * confirmation dialog. The script text is never parsed, never split, and never + * turned into an argument. The argument vector that eventually reaches + * `spawn` is `['run', ]` in every case. + * + * ## The honest limit + * + * `npm run test` runs whatever the project's `package.json` says it should. + * This module cannot change that, and neither can the registry: running the + * project's own test command *is* the feature. What the design can do — and + * does — is make sure a person sees the exact program, arguments, directory + * and script text, in a native dialog the renderer cannot forge or answer, + * before any of it happens; that the run is bounded in time and output; that + * it inherits neither a terminal nor the parent's environment; and that an + * engaged emergency stop kills it. That is recorded as this milestone's + * central limitation in `docs/security-model.md` rather than described as + * solved. + */ + +import { runProcess, type ProcessRunResult } from './process-runner'; +import { readProjectFile } from './workspace-inspector'; +import type { ApprovedProject } from './workspace-session'; +import { + COMMAND_MAX_OUTPUT_BYTES, + COMMAND_MAX_OUTPUT_LINES, + COMMAND_TIMEOUT_MS, + FORBIDDEN_OBJECT_KEYS, +} from '../shared/constants'; +import type { CommandCatalog, CommandDescriptor, CommandRunResult } from '../shared/schemas'; +import { + CODING_COMMANDS, + describeCommandLine, + describeScriptRisks, + findCodingCommand, + sanitizeScriptPreview, + type CodingCommandDefinition, + type CodingCommandId, +} from '../shared/workspace/command-registry'; +import { WorkspaceError } from '../shared/workspace/errors'; + +/** + * Duplicated from `main/settings.ts` and `main/emergency.ts` for the reason + * those two already duplicate it from each other: one small, pure, + * self-contained check is clearer repeated than turned into a dependency + * between modules with no other relationship. `JSON.parse` does not fall for + * a literal `"__proto__"` key, but nothing downstream is allowed to assume it. + */ +const MAX_PACKAGE_JSON_DEPTH = 64; + +function containsForbiddenKey(value: unknown, depth = 0): boolean { + if (depth > MAX_PACKAGE_JSON_DEPTH) return true; + if (value === null || typeof value !== 'object') return false; + if (Array.isArray(value)) { + return value.some((element) => containsForbiddenKey(element, depth + 1)); + } + const record = value as Record; + for (const key of Object.keys(record)) { + if (FORBIDDEN_OBJECT_KEYS.includes(key)) return true; + if (containsForbiddenKey(record[key], depth + 1)) return true; + } + return false; +} + +/** + * The project's declared scripts, or an empty map. + * + * Never throws: a project with no `package.json`, an unreadable one, one that + * is not JSON, or one whose `scripts` is not an object all mean the same thing + * here — no command is available — and none of them is an error the user + * needs to see as a failure. + */ +export async function readProjectScripts( + project: ApprovedProject, +): Promise> { + let raw: string; + try { + raw = (await readProjectFile(project, 'package.json')).content; + } catch { + return new Map(); + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return new Map(); + } + if (containsForbiddenKey(parsed)) return new Map(); + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) return new Map(); + + const scripts = (parsed as Record).scripts; + if (scripts === null || typeof scripts !== 'object' || Array.isArray(scripts)) return new Map(); + + const found = new Map(); + // Only the fixed names the registry declares are ever looked up, so the + // size of the project's own scripts object cannot influence anything here. + for (const command of CODING_COMMANDS) { + const record = scripts as Record; + if (!Object.hasOwn(record, command.scriptName)) continue; + const body = record[command.scriptName]; + if (typeof body !== 'string') continue; + found.set(command.scriptName, body); + } + return found; +} + +function describe( + command: CodingCommandDefinition, + scripts: ReadonlyMap, +): CommandDescriptor { + const body = scripts.get(command.scriptName); + const available = body !== undefined; + return { + id: command.id, + label: command.label, + description: command.description, + commandLine: describeCommandLine(command), + available, + scriptPreview: body === undefined ? null : sanitizeScriptPreview(body), + risks: body === undefined ? [] : describeScriptRisks(body), + }; +} + +/** Every registry command, with whether this project declares it. */ +export async function buildCommandCatalog( + project: ApprovedProject, + busy: boolean, +): Promise { + const scripts = await readProjectScripts(project); + return { commands: CODING_COMMANDS.map((command) => describe(command, scripts)), busy }; +} + +export interface RunCodingCommandOptions { + readonly project: ApprovedProject; + readonly commandId: CodingCommandId; + readonly runId: string; + /** UTC ISO-8601, supplied by the caller. This module reads no clock of its own. */ + readonly startedAt: string; + readonly finishedAtFn: () => string; + readonly signal?: AbortSignal; + readonly isEmergencyEngaged?: () => Promise; +} + +function toOutcome(result: ProcessRunResult): CommandRunResult['outcome'] { + if (result.timedOut || result.cancelled || result.stoppedByEmergency) return 'stopped'; + return result.exitCode === 0 ? 'succeeded' : 'failed'; +} + +/** + * Runs one registry command inside the approved project. + * + * The working directory is always `project.rootPath` — there is no parameter + * for anything else, so a command cannot run outside the directory the user + * approved. Availability is re-checked here, at the moment of running, rather + * than trusted from whatever the catalog said earlier. + */ +export async function runCodingCommand( + options: RunCodingCommandOptions, +): Promise { + const { project, commandId, runId, startedAt, finishedAtFn, signal, isEmergencyEngaged } = + options; + + const command = findCodingCommand(commandId); + if (command === null) throw new WorkspaceError('COMMAND_NOT_AVAILABLE'); + + const scripts = await readProjectScripts(project); + if (!scripts.has(command.scriptName)) throw new WorkspaceError('COMMAND_NOT_AVAILABLE'); + + const result = await runProcess({ + program: command.program, + args: [...command.args], + cwd: project.rootPath, + timeoutMs: COMMAND_TIMEOUT_MS, + maxOutputBytes: COMMAND_MAX_OUTPUT_BYTES, + maxOutputLines: COMMAND_MAX_OUTPUT_LINES, + ...(signal === undefined ? {} : { signal }), + ...(isEmergencyEngaged === undefined ? {} : { isEmergencyEngaged }), + }); + + return { + runId, + commandId, + commandLine: describeCommandLine(command), + outcome: toOutcome(result), + // `null` when the process was killed: reporting a code it never returned + // would tell the reader the command finished when it did not. + exitCode: result.exitCode === null ? null : Math.max(-1, Math.min(255, result.exitCode)), + startedAt, + finishedAt: finishedAtFn(), + durationMs: result.durationMs, + output: [...result.output], + outputTruncated: result.outputTruncated, + timedOut: result.timedOut, + cancelled: result.cancelled, + stoppedByEmergency: result.stoppedByEmergency, + }; +} diff --git a/src/main/workspace-changes.ts b/src/main/workspace-changes.ts new file mode 100644 index 0000000..59c89fa --- /dev/null +++ b/src/main/workspace-changes.ts @@ -0,0 +1,507 @@ +/** + * Proposed, approved and reversible file changes (Phase 2, Milestone 6). + * + * This is the only module in this codebase that writes a file the user chose. + * `node:fs/promises` is imported here for exactly `mkdir`, `open`, `readFile`, + * `rename`, `rm` and `stat`, and that import list is the whole of its + * filesystem capability — there is no `unlink` of a project file, no `rmdir`, + * no `copyFile`, no `chmod`, and nothing that could create or delete an entry + * inside the approved project. A change *modifies files that already exist*, + * and that is the only thing it can do. + * + * ## The shape that makes approval mean something + * + * A change is proposed once and applied by reference: + * + * 1. {@link proposeChangeSet} takes the proposed content, reads what is + * currently on disk, records a hash of it, builds the diff, and keeps the + * whole thing **in the main process**, keyed by a main-generated id. + * 2. {@link applyChangeSet} takes that id — and nothing else. No path, no + * content, no destination. + * + * So the bytes written are necessarily the bytes that were diffed and shown to + * the user. A renderer that has been compromised between the two calls can ask + * for an already-reviewed change to be applied; it cannot substitute a + * different one, because there is no parameter through which to do so. That is + * the difference between an approval that means something and an approval that + * is merely a step someone took earlier. + * + * ## What protects the file that is already there + * + * - **Containment is re-checked at write time.** Every path is resolved again + * through `resolveProjectPath` — lexical rules, exclusion rules, join, + * `realpath`, and the containment check *after* canonicalisation, which is + * the one that catches a symbolic link. A link planted between the proposal + * and the approval is refused, not followed. + * - **Content is re-checked at write time.** The file's hash must still match + * what it was when the diff was produced. If someone — the user in another + * editor, another process, or a build — changed the file in between, the + * write is refused as `WORKSPACE_CHANGE_STALE` rather than silently + * discarding their work. This is the content half of the + * time-of-check/time-of-use problem, and unlike the path half it can be + * closed completely. + * - **A backup is taken before anything is written**, outside the project, in + * this application's own data directory — so a backup never appears in the + * project tree, never gets committed, and never becomes something the user + * has to clean up before their next commit. + * - **A change set is all-or-nothing.** If the third of five writes fails, + * the two that succeeded are restored from the backup before the failure is + * reported. A half-applied change is the one outcome a user could not + * reason about. + * - **Nothing is ever deleted automatically.** Evicting an old change set + * from memory leaves its backup on disk. The user's own files are never + * removed by this module under any circumstances. + */ + +import { createHash, randomUUID } from 'node:crypto'; +import { mkdir, open, readFile, rename, rm, stat } from 'node:fs/promises'; +import { basename, dirname, join } from 'node:path'; + +import { readProjectFile } from './workspace-inspector'; +import { joinAbsolute, resolveProjectPath, toWorkspaceError } from './workspace-paths'; +import type { ApprovedProject } from './workspace-session'; +import { + WORKSPACE_DIFF_MAX_LINES, + WORKSPACE_MAX_APPLIED_CHANGES, + WORKSPACE_MAX_CHANGE_FILES, + WORKSPACE_MAX_PENDING_CHANGES, + WORKSPACE_MAX_WRITE_BYTES, +} from '../shared/constants'; +import type { + WorkspaceChangeHistory, + WorkspaceChangeSet, + WorkspaceEdit, +} from '../shared/schemas/coding.schema'; +import type { FileDiff } from '../shared/workspace/diff'; +import { buildFileDiff, isUnchanged } from '../shared/workspace/diff'; +import { WorkspaceError } from '../shared/workspace/errors'; + +/** One file inside a change set, as the main process holds it. */ +interface PreparedEdit { + readonly relativePath: string; + readonly name: string; + /** SHA-256 of the file's bytes at the moment the diff was produced. */ + readonly baseHash: string; + readonly diff: FileDiff; + /** + * The proposed replacement. Cleared once the change is applied — by then + * the file on disk *is* this content, and holding a second copy of every + * applied change would grow without bound. + */ + afterContent: string | null; +} + +interface ChangeRecord { + readonly id: string; + readonly createdAt: string; + status: WorkspaceChangeSet['status']; + readonly edits: PreparedEdit[]; + readonly totalAdded: number; + readonly totalRemoved: number; + readonly truncated: boolean; + /** Where the pre-change bytes live, once a write has actually happened. */ + backupDir: string | null; + appliedAt: string | null; + rolledBackAt: string | null; + /** The project this was proposed against; a change never crosses projects. */ + readonly projectRoot: string; +} + +export interface ChangeStore { + propose(options: ProposeChangeOptions): Promise; + apply(options: SettleChangeOptions): Promise; + rollback(options: SettleChangeOptions): Promise; + history(): WorkspaceChangeHistory; + /** The change set a rollback would restore, for building a confirmation. */ + describe(changeId: string): WorkspaceChangeSet | null; + /** The id rollback would target, or `null`. */ + rollbackTarget(): string | null; +} + +/** + * Same atomic-write mechanics as `main/emergency.ts` and `main/settings.ts`, + * duplicated rather than imported for the reason those two already duplicate + * it between themselves: a small, pure, self-contained helper is clearer + * repeated than turned into a shared dependency between modules that have no + * other relationship. The retry exists because a rename over an existing + * destination transiently fails on Windows when something else briefly holds + * it open — an editor with the file open is the common case here, and this + * module writes files the user is very likely to have open. + */ +const RENAME_MAX_ATTEMPTS = 10; +const RENAME_RETRY_DELAY_MS = 20; + +function isTransientRenameError(error: unknown): boolean { + if (typeof error !== 'object' || error === null || !('code' in error)) return false; + const { code } = error; + return code === 'EPERM' || code === 'EBUSY' || code === 'EACCES'; +} + +function delay(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +async function renameWithRetry(from: string, to: string): Promise { + for (let attempt = 1; attempt <= RENAME_MAX_ATTEMPTS; attempt += 1) { + try { + await rename(from, to); + return; + } catch (error) { + if (attempt === RENAME_MAX_ATTEMPTS || !isTransientRenameError(error)) throw error; + await delay(RENAME_RETRY_DELAY_MS * attempt); + } + } +} + +/** + * Replaces a file's contents atomically. + * + * The temporary file is created in the *same directory* as the destination, + * which is what makes the final rename atomic — a rename across volumes is a + * copy and a delete, and a reader can see the gap between them. Its name is + * generated here, so it is never influenced by anything a caller supplied. + * On any failure the temporary file is removed and the original is untouched. + */ +async function writeFileAtomically(absolutePath: string, content: string): Promise { + const directory = dirname(absolutePath); + const tempFile = join(directory, `.${basename(absolutePath)}.local-agent-${randomUUID()}.tmp`); + + try { + const handle = await open(tempFile, 'w'); + try { + await handle.writeFile(content, 'utf8'); + await handle.sync(); + } finally { + await handle.close(); + } + await renameWithRetry(tempFile, absolutePath); + } catch (error) { + await rm(tempFile, { force: true }).catch(() => undefined); + throw error; + } +} + +function hashContent(content: string): string { + return createHash('sha256').update(content, 'utf8').digest('hex'); +} + +function toChangeSet(record: ChangeRecord): WorkspaceChangeSet { + return { + id: record.id, + createdAt: record.createdAt, + status: record.status, + files: record.edits.map((edit) => ({ + path: edit.relativePath, + name: edit.name, + hunks: edit.diff.hunks.map((hunk) => ({ ...hunk, lines: [...hunk.lines] })), + added: edit.diff.added, + removed: edit.diff.removed, + coarse: edit.diff.coarse, + truncated: edit.diff.truncated, + warnings: [...edit.diff.warnings], + })), + totalAdded: record.totalAdded, + totalRemoved: record.totalRemoved, + truncated: record.truncated, + approvalRequired: true, + backupAvailable: record.backupDir !== null, + appliedAt: record.appliedAt, + rolledBackAt: record.rolledBackAt, + }; +} + +export interface ProposeChangeOptions { + readonly project: ApprovedProject; + readonly edits: readonly WorkspaceEdit[]; + /** UTC ISO-8601, supplied by the caller. This module reads no clock. */ + readonly now: string; +} + +export interface SettleChangeOptions { + readonly project: ApprovedProject; + readonly changeId: string; + readonly now: string; +} + +export interface CreateChangeStoreOptions { + /** + * Where pre-change backups are written — a directory under + * `%APPDATA%\Local-Agent`, never inside the user's project. + */ + readonly backupsDir: string; +} + +export function createChangeStore(options: CreateChangeStoreOptions): ChangeStore { + const { backupsDir } = options; + /** Newest last, so eviction and "most recent" are both cheap and obvious. */ + const records: ChangeRecord[] = []; + + function find(changeId: string): ChangeRecord { + const record = records.find((candidate) => candidate.id === changeId); + if (record === undefined) throw new WorkspaceError('WORKSPACE_CHANGE_NOT_FOUND'); + return record; + } + + function latestApplied(): ChangeRecord | null { + for (let index = records.length - 1; index >= 0; index -= 1) { + const record = records[index]; + if (record?.status === 'applied') return record; + } + return null; + } + + /** + * Keeps the store bounded without ever discarding something a rollback + * might need. + * + * A still-pending proposal is only a description, so the oldest is dropped + * once too many accumulate. An applied one is dropped only when the applied + * history is itself over its bound, and dropping it never touches the backup + * on disk — this module deletes nothing. + */ + function evict(): void { + let pending = records.filter((record) => record.status === 'awaiting-approval').length; + while (pending > WORKSPACE_MAX_PENDING_CHANGES) { + const index = records.findIndex((record) => record.status === 'awaiting-approval'); + if (index === -1) break; + records.splice(index, 1); + pending -= 1; + } + + let settled = records.filter((record) => record.status !== 'awaiting-approval').length; + while (settled > WORKSPACE_MAX_APPLIED_CHANGES) { + const index = records.findIndex((record) => record.status !== 'awaiting-approval'); + if (index === -1) break; + records.splice(index, 1); + settled -= 1; + } + } + + async function propose(proposeOptions: ProposeChangeOptions): Promise { + const { project, edits, now } = proposeOptions; + + if (edits.length === 0 || edits.length > WORKSPACE_MAX_CHANGE_FILES) { + throw new WorkspaceError('WORKSPACE_CHANGE_TOO_LARGE'); + } + + const prepared: PreparedEdit[] = []; + let totalAdded = 0; + let totalRemoved = 0; + let budget = WORKSPACE_DIFF_MAX_LINES; + let truncated = false; + let anyChange = false; + + for (const edit of edits) { + if (Buffer.byteLength(edit.content, 'utf8') > WORKSPACE_MAX_WRITE_BYTES) { + throw new WorkspaceError('WORKSPACE_CHANGE_TOO_LARGE'); + } + + // Reuses the read path in full — so a credential file, an excluded + // directory, a binary file, an oversized file and a path that escapes + // the project are all refused here by exactly the rules that already + // refuse them for reading. A file this application will not show is a + // file it will not write either. + const current = await readProjectFile(project, edit.path); + const target = await resolveProjectPath(project.rootPath, edit.path); + + const diff = buildFileDiff({ + before: current.content, + after: edit.content, + maxLines: budget, + }); + budget = Math.max(0, budget - diff.hunks.reduce((sum, hunk) => sum + hunk.lines.length, 0)); + if (diff.truncated) truncated = true; + if (!isUnchanged(current.content, edit.content)) anyChange = true; + + totalAdded += diff.added; + totalRemoved += diff.removed; + + prepared.push({ + relativePath: target.relativePath, + name: current.metadata.name, + baseHash: hashContent(current.content), + diff, + afterContent: edit.content, + }); + } + + // A proposal that changes nothing is refused rather than recorded: it + // would otherwise be approvable, and approving a write that writes the + // same bytes back teaches a user that approving is inconsequential. + if (!anyChange) throw new WorkspaceError('WORKSPACE_CHANGE_EMPTY'); + + const record: ChangeRecord = { + id: randomUUID(), + createdAt: now, + status: 'awaiting-approval', + edits: prepared, + totalAdded, + totalRemoved, + truncated, + backupDir: null, + appliedAt: null, + rolledBackAt: null, + projectRoot: project.rootPath, + }; + records.push(record); + evict(); + + return toChangeSet(record); + } + + /** Copies the current bytes of every target out of the project first. */ + async function backup( + record: ChangeRecord, + targets: readonly { readonly relativePath: string; readonly content: string }[], + ): Promise { + const directory = join(backupsDir, record.id); + try { + for (const target of targets) { + const destination = joinAbsolute(directory, target.relativePath); + await mkdir(dirname(destination), { recursive: true }); + await writeFileAtomically(destination, target.content); + } + } catch { + throw new WorkspaceError('WORKSPACE_BACKUP_FAILED'); + } + return directory; + } + + async function apply(settleOptions: SettleChangeOptions): Promise { + const { project, changeId, now } = settleOptions; + const record = find(changeId); + + if (record.projectRoot !== project.rootPath) { + // A change proposed against one project can never be applied to + // another, even if the same relative paths exist in both. + throw new WorkspaceError('WORKSPACE_CHANGE_NOT_FOUND'); + } + if (record.status !== 'awaiting-approval') { + throw new WorkspaceError('WORKSPACE_CHANGE_SETTLED'); + } + + // Re-resolve and re-read every target *now*. Containment, exclusion and + // the symbolic-link check all run again against the current filesystem, + // and the content hash must still match what was diffed. + const targets: { + readonly relativePath: string; + readonly absolutePath: string; + readonly content: string; + readonly next: string; + }[] = []; + + for (const edit of record.edits) { + const next = edit.afterContent; + if (next === null) throw new WorkspaceError('WORKSPACE_CHANGE_SETTLED'); + + const target = await resolveProjectPath(project.rootPath, edit.relativePath); + let stats; + try { + stats = await stat(target.absolutePath); + } catch (error) { + throw toWorkspaceError(error); + } + if (!stats.isFile()) throw new WorkspaceError('WORKSPACE_UNSUPPORTED_ENTRY'); + + const current = await readProjectFile(project, edit.relativePath); + if (hashContent(current.content) !== edit.baseHash) { + throw new WorkspaceError('WORKSPACE_CHANGE_STALE'); + } + + targets.push({ + relativePath: edit.relativePath, + absolutePath: target.absolutePath, + content: current.content, + next, + }); + } + + const backupDir = await backup(record, targets); + record.backupDir = backupDir; + + const written: typeof targets = []; + try { + for (const target of targets) { + await writeFileAtomically(target.absolutePath, target.next); + written.push(target); + } + } catch { + // All-or-nothing: restore whatever was already written before reporting + // the failure, so the project is never left half-changed. + for (const target of written) { + await writeFileAtomically(target.absolutePath, target.content).catch(() => undefined); + } + record.status = 'failed'; + throw new WorkspaceError('WORKSPACE_WRITE_FAILED'); + } + + record.status = 'applied'; + record.appliedAt = now; + for (const edit of record.edits) edit.afterContent = null; + evict(); + + return toChangeSet(record); + } + + async function rollback(settleOptions: SettleChangeOptions): Promise { + const { project, changeId, now } = settleOptions; + const record = find(changeId); + + if (record.projectRoot !== project.rootPath) { + throw new WorkspaceError('WORKSPACE_CHANGE_NOT_FOUND'); + } + if (record.status !== 'applied') throw new WorkspaceError('WORKSPACE_CHANGE_SETTLED'); + + // Only the most recent applied change can be rolled back. Restoring an + // older one would silently undo everything applied after it, which is not + // what "undo the change I just made" means to anyone. + // `?.` covers both "nothing is applied" and "the applied one is not this". + const target = latestApplied(); + if (target?.id !== record.id) { + throw new WorkspaceError('WORKSPACE_ROLLBACK_UNAVAILABLE'); + } + + const backupDir = record.backupDir; + if (backupDir === null) throw new WorkspaceError('WORKSPACE_ROLLBACK_UNAVAILABLE'); + + const restores: { readonly absolutePath: string; readonly content: string }[] = []; + for (const edit of record.edits) { + const resolved = await resolveProjectPath(project.rootPath, edit.relativePath); + let content: string; + try { + content = await readFile(joinAbsolute(backupDir, edit.relativePath), 'utf8'); + } catch { + throw new WorkspaceError('WORKSPACE_ROLLBACK_UNAVAILABLE'); + } + restores.push({ absolutePath: resolved.absolutePath, content }); + } + + for (const restore of restores) { + try { + await writeFileAtomically(restore.absolutePath, restore.content); + } catch { + throw new WorkspaceError('WORKSPACE_WRITE_FAILED'); + } + } + + record.status = 'rolled-back'; + record.rolledBackAt = now; + return toChangeSet(record); + } + + return { + propose, + apply, + rollback, + history: () => ({ + // Newest first, which is the order the interface reads them in. + changes: [...records].reverse().map(toChangeSet), + rollbackTarget: latestApplied()?.id ?? null, + }), + describe: (changeId) => { + const record = records.find((candidate) => candidate.id === changeId); + return record === undefined ? null : toChangeSet(record); + }, + rollbackTarget: () => latestApplied()?.id ?? null, + }; +} diff --git a/src/main/workspace-inspector.ts b/src/main/workspace-inspector.ts new file mode 100644 index 0000000..78ce867 --- /dev/null +++ b/src/main/workspace-inspector.ts @@ -0,0 +1,456 @@ +/** + * Read-only project inspection (Phase 2, Milestone 5). + * + * The only module in this codebase that reads a file the user chose, and it + * has three functions: list a tree, read one text file, search for a + * substring. There is no fourth. Nothing here opens a write handle, creates a + * directory, renames, deletes, or spawns anything — `node:fs/promises` is + * imported for exactly `readdir`, `readFile` and `stat`, and that import list + * is the whole of this module's filesystem capability. + * + * Every path reaching a syscall here came through + * `workspace-paths.ts`'s `resolveProjectPath`, so it is lexically safe, + * carries no excluded segment, and has been proven — after `realpath` — to + * live inside the approved root. Names discovered *by* the walk are held to + * the identical rule before they are ever returned, so a path the renderer + * receives is always one it can legally send back. + * + * Everything is bounded. A tree stops at a depth, an entry count and a + * per-directory count; a file is refused above a byte cap rather than + * truncated; a search stops at a match count and a scanned-file count. Each + * bound reports itself (`truncated`) rather than silently shortening a + * result, because a listing that quietly omits half a project is one a reader + * would draw the wrong conclusion from. + * + * **Nothing here logs.** No `console` call, no path in a thrown message: a + * file's contents and a user's directory layout are exactly the material the + * milestone brief says must not reach a log. + */ + +import { readdir, readFile, stat } from 'node:fs/promises'; + +import { joinAbsolute, resolveProjectPath, toWorkspaceError } from './workspace-paths'; +import type { ApprovedProject } from './workspace-session'; +import { + BIDI_CONTROL_PATTERN, + WORKSPACE_BINARY_SNIFF_BYTES, + WORKSPACE_MAX_DIRECTORY_ENTRIES, + WORKSPACE_MAX_FILE_BYTES, + WORKSPACE_MAX_FILE_CONTENT_LENGTH, + WORKSPACE_MAX_SEARCH_FILES, + WORKSPACE_MAX_SEARCH_RESULTS, + WORKSPACE_MAX_TREE_DEPTH, + WORKSPACE_MAX_TREE_ENTRIES, + WORKSPACE_SEARCH_EXCERPT_MAX_LENGTH, + WORKSPACE_SEARCH_MAX_FILE_BYTES, +} from '../shared/constants'; +import type { + WorkspaceEntry, + WorkspaceFile, + WorkspaceSearchMatch, + WorkspaceSearchResult, + WorkspaceTree, +} from '../shared/schemas/workspace.schema'; +import { WorkspaceError } from '../shared/workspace/errors'; +import { hasBinaryFileExtension, isExcludedEntryName } from '../shared/workspace/exclusions'; +import { isSafeWorkspaceRelativePath, joinWorkspacePath } from '../shared/workspace/path-safety'; + +/** One directory entry the walk decided to keep, before it was described. */ +interface WalkCandidate { + readonly name: string; + readonly kind: 'file' | 'directory'; + readonly absolutePath: string; + readonly relativePath: string; + readonly depth: number; + readonly excluded: boolean; +} + +/** + * Reads and orders one directory's usable children. + * + * Skips, without reporting an error: + * + * - anything that is neither a regular file nor a directory. This is what + * keeps symbolic links out of the walk entirely, and with them both + * directory loops (`a -> ..`) and escapes; sockets, FIFOs and device nodes + * go the same way, since none is something a code viewer should open. A + * symlinked file can still be opened *directly*, by path, where + * `resolveProjectPath` proves the link stays inside the project — the walk + * simply does not offer it. + * - any name this codebase's own path rules would refuse (a control + * character, a bidirectional override, a reserved device name, a trailing + * dot). Listing such a name would offer the renderer a path it could never + * legally send back. + */ +async function readDirectoryCandidates( + directoryAbsolute: string, + directoryRelative: string, + depth: number, +): Promise<{ readonly candidates: readonly WalkCandidate[]; readonly truncated: boolean }> { + const dirents = await readdir(directoryAbsolute, { withFileTypes: true }); + + const usable = dirents + .filter((dirent) => dirent.isFile() || dirent.isDirectory()) + .filter((dirent) => isSafeWorkspaceRelativePath(dirent.name)); + + usable.sort((a, b) => { + const aDir = a.isDirectory() ? 0 : 1; + const bDir = b.isDirectory() ? 0 : 1; + if (aDir !== bDir) return aDir - bDir; + return a.name.localeCompare(b.name, 'en', { sensitivity: 'base' }); + }); + + const kept = usable.slice(0, WORKSPACE_MAX_DIRECTORY_ENTRIES); + const candidates = kept.map((dirent): WalkCandidate => { + const kind = dirent.isDirectory() ? ('directory' as const) : ('file' as const); + return { + name: dirent.name, + kind, + absolutePath: joinAbsolute(directoryAbsolute, dirent.name), + relativePath: joinWorkspacePath(directoryRelative, dirent.name), + depth, + excluded: isExcludedEntryName(dirent.name, kind), + }; + }); + + return { candidates, truncated: kept.length < usable.length }; +} + +/** A file's size in bytes, or `null` when it cannot be determined. */ +async function fileSize(absolutePath: string): Promise { + try { + const stats = await stat(absolutePath); + return stats.isFile() ? stats.size : null; + } catch { + return null; + } +} + +async function describeCandidate(candidate: WalkCandidate): Promise { + const base = { + path: candidate.relativePath, + name: candidate.name, + kind: candidate.kind, + depth: candidate.depth, + excluded: candidate.excluded, + }; + + if (candidate.kind === 'directory') { + return { ...base, readable: false }; + } + + const size = await fileSize(candidate.absolutePath); + const readable = + !candidate.excluded && + !hasBinaryFileExtension(candidate.name) && + size !== null && + size <= WORKSPACE_MAX_FILE_BYTES; + + return { ...base, ...(size === null ? {} : { size }), readable }; +} + +/** + * Lists a bounded, depth-first tree below one directory of the approved + * project. + * + * Depth-first (pre-order) rather than breadth-first so entries arrive in the + * order a tree is drawn — a directory immediately followed by its children — + * which is what lets the interface render a flat, indented list without + * rebuilding the hierarchy. + * + * An excluded directory is **listed and not descended into**. Hiding it would + * make the tree quietly disagree with what is on disk; the flag tells the + * reader why nothing is shown beneath it. + * + * A subdirectory that cannot be read (permissions, or removed mid-walk) is + * skipped and the walk continues — one unreadable directory must not fail the + * listing of an entire project. The *starting* directory is different: if + * that cannot be read, the caller asked for something it cannot have, and the + * error propagates. + */ +export async function listProjectTree( + project: ApprovedProject, + requestedPath: unknown, +): Promise { + const root = await resolveProjectPath(project.rootPath, requestedPath); + + let rootStats; + try { + rootStats = await stat(root.absolutePath); + } catch (error) { + throw toWorkspaceError(error); + } + if (!rootStats.isDirectory()) throw new WorkspaceError('WORKSPACE_UNSUPPORTED_ENTRY'); + + const entries: WorkspaceEntry[] = []; + let truncated = false; + + let initial; + try { + initial = await readDirectoryCandidates(root.absolutePath, root.relativePath, 0); + } catch (error) { + throw toWorkspaceError(error); + } + if (initial.truncated) truncated = true; + + // Reversed on push so the stack pops them back in sorted order. + const stack: WalkCandidate[] = [...initial.candidates].reverse(); + + while (stack.length > 0) { + if (entries.length >= WORKSPACE_MAX_TREE_ENTRIES) { + truncated = true; + break; + } + + const candidate = stack.pop(); + if (candidate === undefined) break; + + entries.push(await describeCandidate(candidate)); + + if (candidate.kind !== 'directory' || candidate.excluded) continue; + if (candidate.depth + 1 > WORKSPACE_MAX_TREE_DEPTH) { + truncated = true; + continue; + } + + try { + const child = await readDirectoryCandidates( + candidate.absolutePath, + candidate.relativePath, + candidate.depth + 1, + ); + if (child.truncated) truncated = true; + for (let index = child.candidates.length - 1; index >= 0; index -= 1) { + const next = child.candidates[index]; + if (next !== undefined) stack.push(next); + } + } catch { + // One unreadable subdirectory does not fail the whole listing. + truncated = true; + } + } + + return { root: root.relativePath, entries, truncated }; +} + +/** True when a NUL appears in the sampled prefix — the standard "not text" test. */ +function looksBinary(buffer: Buffer): boolean { + const sampleLength = Math.min(buffer.length, WORKSPACE_BINARY_SNIFF_BYTES); + for (let index = 0; index < sampleLength; index += 1) { + if (buffer[index] === 0) return true; + } + return false; +} + +/** + * Reads one text file from the approved project. + * + * Refusals, in the order they are checked — each one before any content + * exists in memory, except the last two, which need the bytes: + * + * 1. not a regular file → `WORKSPACE_UNSUPPORTED_ENTRY` + * 2. larger than the hard cap → `WORKSPACE_FILE_TOO_LARGE` + * 3. binary by extension → `WORKSPACE_BINARY_FILE` + * 4. NUL in the first bytes read → `WORKSPACE_BINARY_FILE` + * 5. not valid UTF-8 → `WORKSPACE_BINARY_FILE` + * + * Exclusion (a credential file, or anything under an excluded directory) was + * already refused by `resolveProjectPath`, at every segment of the path + * rather than only its last one. + * + * Oversized files are **refused, not truncated**: a partial source file + * invites a reader to conclude something from text that was silently cut off. + */ +export async function readProjectFile( + project: ApprovedProject, + requestedPath: unknown, +): Promise { + const target = await resolveProjectPath(project.rootPath, requestedPath); + const name = target.segments[target.segments.length - 1]; + if (name === undefined) throw new WorkspaceError('WORKSPACE_UNSUPPORTED_ENTRY'); + + let stats; + try { + stats = await stat(target.absolutePath); + } catch (error) { + throw toWorkspaceError(error); + } + if (!stats.isFile()) throw new WorkspaceError('WORKSPACE_UNSUPPORTED_ENTRY'); + if (stats.size > WORKSPACE_MAX_FILE_BYTES) { + throw new WorkspaceError('WORKSPACE_FILE_TOO_LARGE'); + } + if (hasBinaryFileExtension(name)) throw new WorkspaceError('WORKSPACE_BINARY_FILE'); + + let buffer: Buffer; + try { + buffer = await readFile(target.absolutePath); + } catch (error) { + throw toWorkspaceError(error); + } + + // Re-checked against what was actually read, not only against the size + // `stat` reported a moment earlier. + if (buffer.length > WORKSPACE_MAX_FILE_BYTES) { + throw new WorkspaceError('WORKSPACE_FILE_TOO_LARGE'); + } + if (looksBinary(buffer)) throw new WorkspaceError('WORKSPACE_BINARY_FILE'); + + let content: string; + try { + // `fatal: true` is the point: a file that is not valid UTF-8 must be + // refused, not silently rendered full of replacement characters as though + // it had been read correctly. + content = new TextDecoder('utf-8', { fatal: true }).decode(buffer); + } catch { + throw new WorkspaceError('WORKSPACE_BINARY_FILE'); + } + + if (content.length > WORKSPACE_MAX_FILE_CONTENT_LENGTH) { + throw new WorkspaceError('WORKSPACE_FILE_TOO_LARGE'); + } + + const warnings = BIDI_CONTROL_PATTERN.test(content) + ? (['bidirectional-control-characters'] as const) + : ([] as const); + + return { + metadata: { + path: target.relativePath, + name, + size: buffer.length, + lineCount: content.length === 0 ? 0 : content.split('\n').length, + encoding: 'utf-8', + warnings: [...warnings], + }, + content, + }; +} + +/** + * Makes one line safe to show in a results list. + * + * Control characters become spaces and bidirectional overrides are dropped + * outright. This is the one place in the workspace where text is altered + * rather than shown as stored, and the asymmetry is deliberate: an excerpt is + * already a lossy fragment, and a results list is exactly where a line + * reordered by a bidi override would be most convincing and least likely to + * be double-checked. Whole-file content is never rewritten this way — it is + * shown verbatim, with a warning. + * + * Written as a character walk rather than a regular expression so no control + * character has to appear in this file's own source. + */ +function sanitizeExcerpt(line: string): string { + let out = ''; + for (const character of line) { + const code = character.codePointAt(0) ?? 0; + if (code < 0x20 || code === 0x7f) { + out += ' '; + continue; + } + if (code >= 0x202a && code <= 0x202e) continue; + if (code >= 0x2066 && code <= 0x2069) continue; + out += character; + } + return out; +} + +/** A bounded window of the line, centred on the match. */ +function buildExcerpt(line: string, matchIndex: number): string { + const sanitized = sanitizeExcerpt(line); + if (sanitized.length <= WORKSPACE_SEARCH_EXCERPT_MAX_LENGTH) return sanitized.trim(); + const start = Math.max(0, matchIndex - Math.floor(WORKSPACE_SEARCH_EXCERPT_MAX_LENGTH / 3)); + return sanitized.slice(start, start + WORKSPACE_SEARCH_EXCERPT_MAX_LENGTH).trim(); +} + +/** Every file the search may open, in tree order, already bounded. */ +async function collectSearchableFiles( + project: ApprovedProject, + requestedPath: unknown, +): Promise<{ readonly files: readonly WorkspaceEntry[]; readonly truncated: boolean }> { + const tree = await listProjectTree(project, requestedPath); + const files = tree.entries.filter((entry) => entry.kind === 'file' && entry.readable); + return { files, truncated: tree.truncated }; +} + +/** + * Searches for a literal substring inside the approved project. + * + * **Never a regular expression.** The query comes from a text field, and + * compiling user input as a pattern is how a search box becomes a denial of + * service against the process that owns every privileged operation in this + * application. Matching is a case-insensitive `indexOf`, and nothing else. + * + * At most one match is reported per line: a single minified line can contain + * hundreds of occurrences, which would fill the whole result budget from one + * file and tell the reader nothing they could act on. + */ +export async function searchProject( + project: ApprovedProject, + query: string, + requestedPath: unknown, +): Promise { + const { files, truncated: treeTruncated } = await collectSearchableFiles(project, requestedPath); + + const needle = query.toLowerCase(); + const matches: WorkspaceSearchMatch[] = []; + let filesScanned = 0; + let truncated = treeTruncated; + + for (const file of files) { + if (matches.length >= WORKSPACE_MAX_SEARCH_RESULTS) { + truncated = true; + break; + } + if (filesScanned >= WORKSPACE_MAX_SEARCH_FILES) { + truncated = true; + break; + } + if (file.size === undefined || file.size > WORKSPACE_SEARCH_MAX_FILE_BYTES) continue; + + let target; + try { + target = await resolveProjectPath(project.rootPath, file.path); + } catch { + // A file that vanished, or that turned out not to resolve inside the + // project after all, is skipped — never searched on the strength of + // having appeared in a listing a moment ago. + continue; + } + + let buffer: Buffer; + try { + buffer = await readFile(target.absolutePath); + } catch { + continue; + } + filesScanned += 1; + if (looksBinary(buffer)) continue; + + let content: string; + try { + content = new TextDecoder('utf-8', { fatal: true }).decode(buffer); + } catch { + continue; + } + + const lines = content.split('\n'); + for (let lineIndex = 0; lineIndex < lines.length; lineIndex += 1) { + if (matches.length >= WORKSPACE_MAX_SEARCH_RESULTS) { + truncated = true; + break; + } + const line = lines[lineIndex] ?? ''; + const column = line.toLowerCase().indexOf(needle); + if (column === -1) continue; + matches.push({ + path: file.path, + line: lineIndex + 1, + column: column + 1, + excerpt: buildExcerpt(line, column), + }); + } + } + + return { query, matches, filesScanned, truncated }; +} diff --git a/src/main/workspace-paths.ts b/src/main/workspace-paths.ts new file mode 100644 index 0000000..c117f16 --- /dev/null +++ b/src/main/workspace-paths.ts @@ -0,0 +1,153 @@ +/** + * Path containment for the read-only workspace (Phase 2, Milestone 5). + * + * This is the **second** of the two containment defences, and the one that + * touches the disk. `src/shared/workspace/path-safety.ts` decides whether a + * string is even shaped like a path inside a project; this module decides + * where that string actually lands once it is joined onto a real root and the + * filesystem has had its say. + * + * Both are required, and neither is a duplicate of the other: + * + * - A lexical rule cannot see a symbolic link. `src/notes` may be a perfectly + * well-formed relative path and still be a link to `C:\Users\me\.ssh`. + * - A `realpath` check cannot run on a string that has not been joined onto a + * root yet — and joining is exactly the step an absolute path or a `..` + * segment subverts, so by the time `realpath` could look, the damage would + * already be in the path it was handed. + * + * So every resolution runs, in order: validate lexically, refuse excluded + * segments, join, check containment on the joined path, canonicalise with + * `realpath`, and check containment **again** on the canonical result. A link + * pointing outside the project fails the last check even though it passed the + * first. + * + * ## The one gap, stated plainly + * + * Between `realpath` returning and the subsequent `open`/`readdir`, a path + * component could in principle be replaced with a link — the classic + * time-of-check/time-of-use race. Closing it properly needs handle-based, + * `O_NOFOLLOW`-style APIs that Node does not expose portably. The exposure + * here is narrow (the attacker must already be able to write inside the + * user's own approved project, on the user's own machine, between two + * adjacent syscalls) and the consequence is bounded by everything else in + * this layer: the result is still read-only, still size-capped, and still + * refused if it is not text. It is recorded in `docs/security-model.md` + * rather than described as solved. + */ + +import { isAbsolute, join, relative, resolve, sep } from 'node:path'; +import { realpath } from 'node:fs/promises'; + +import { WorkspaceError } from '../shared/workspace/errors'; +import { containsExcludedSegment } from '../shared/workspace/exclusions'; +import { normalizeWorkspaceRelativePath } from '../shared/workspace/path-safety'; + +/** + * True when `candidate` is `root` itself or lies beneath it. + * + * Uses `path.relative` rather than a string prefix comparison, which matters + * on Windows for two reasons a prefix test gets wrong: `relative` compares + * case-insensitively there (so `C:\Foo` contains `C:\foo\bar`), and it + * normalises mixed separators. The explicit `'..'` checks are written against + * a whole segment rather than as `startsWith('..')`, so a directory + * legitimately named `..config` is not mistaken for an escape. + */ +export function isContainedPath(root: string, candidate: string): boolean { + const relativePath = relative(root, candidate); + if (relativePath === '') return true; + if (relativePath === '..' || relativePath.startsWith(`..${sep}`)) return false; + // An absolute result means the two share no common root at all — different + // drives on Windows, for instance. + return !isAbsolute(relativePath); +} + +/** One resolved, contained entry inside an approved project. */ +export interface ResolvedProjectPath { + /** The canonical absolute path, after `realpath`. Safe to open. */ + readonly absolutePath: string; + /** The canonical project-relative path; `''` for the project root. */ + readonly relativePath: string; + readonly segments: readonly string[]; +} + +/** + * Maps a filesystem error to the workspace's normalized vocabulary. + * + * Reads only `code`, never `message` — an `errno` message contains the full + * path, which must not reach a thrown error that might later be logged or + * shown. Anything unrecognised becomes the generic read failure rather than + * being passed through. + */ +export function toWorkspaceError(error: unknown): WorkspaceError { + if (error instanceof WorkspaceError) return error; + const code = (error as { code?: unknown } | null)?.code; + if (code === 'ENOENT' || code === 'ENOTDIR') return new WorkspaceError('WORKSPACE_NOT_FOUND'); + if (code === 'EACCES' || code === 'EPERM') return new WorkspaceError('WORKSPACE_ACCESS_DENIED'); + if (code === 'EISDIR' || code === 'ELOOP' || code === 'ENAMETOOLONG') { + return new WorkspaceError('WORKSPACE_UNSUPPORTED_ENTRY'); + } + return new WorkspaceError('WORKSPACE_READ_FAILED'); +} + +/** + * Resolves one renderer-supplied relative path against an approved project + * root, or throws a {@link WorkspaceError}. + * + * `rootPath` must already be canonical — `adoptProjectDirectory` runs + * `realpath` on it once, at selection time, so this function compares two + * canonical paths rather than one canonical and one not. + */ +export async function resolveProjectPath( + rootPath: string, + requestedPath: unknown, +): Promise { + const normalized = normalizeWorkspaceRelativePath(requestedPath); + if (!normalized.ok) throw new WorkspaceError('WORKSPACE_INVALID_PATH'); + + if (containsExcludedSegment(normalized.segments)) { + throw new WorkspaceError('WORKSPACE_PATH_EXCLUDED'); + } + + const joined = + normalized.segments.length === 0 + ? resolve(rootPath) + : resolve(rootPath, ...normalized.segments); + + // Cannot fail given the lexical rules above; kept because "cannot fail" is + // a claim about today's rules, and this is the check that would catch it if + // one of them were ever loosened. + if (!isContainedPath(rootPath, joined)) { + throw new WorkspaceError('WORKSPACE_PATH_OUTSIDE_PROJECT'); + } + + let canonical: string; + try { + canonical = await realpath(joined); + } catch (error) { + throw toWorkspaceError(error); + } + + // The check that actually catches a symbolic link: `joined` was inside the + // project by construction, `canonical` is where it really points. + if (!isContainedPath(rootPath, canonical)) { + throw new WorkspaceError('WORKSPACE_PATH_OUTSIDE_PROJECT'); + } + + return { + absolutePath: canonical, + relativePath: normalized.path, + segments: normalized.segments, + }; +} + +/** + * Joins a directory's absolute path and one child name. + * + * A thin wrapper so the walk never builds an absolute path by string + * concatenation, which is where separator and normalisation mistakes come + * from. + */ +export function joinAbsolute(directory: string, name: string): string { + return join(directory, name); +} diff --git a/src/main/workspace-planner.ts b/src/main/workspace-planner.ts new file mode 100644 index 0000000..a471398 --- /dev/null +++ b/src/main/workspace-planner.ts @@ -0,0 +1,168 @@ +/** + * The coding-plan inspection layer (Phase 2, Milestone 5). + * + * Gathers what can be observed about an approved project — which files + * mention the objective's terms, which project markers exist, whether there + * is test tooling, whether the search saw everything — and hands those + * observations to the pure builder in `src/shared/workspace/plan.ts`. + * + * The split matches the one this codebase already uses for every decision + * that matters: a pure core (`decidePermission`, `buildCodingPlan`) behind a + * thin layer that does the I/O. Everything about *what a plan says* is + * deterministic and testable without a filesystem; everything about *what is + * true of this project* is measured here, once, under the same bounds every + * other workspace read obeys. + * + * This module performs no write, calls no model, and produces nothing that + * can be applied. A plan is a document: `codingPlanSchema` pins its `diff` to + * `null` and its `status` to `'awaiting-approval'`, and there is no function + * anywhere in this milestone that consumes one. + */ + +import { listProjectTree, searchProject } from './workspace-inspector'; +import type { ApprovedProject } from './workspace-session'; +import { WORKSPACE_PLAN_MAX_FILES, WORKSPACE_PLAN_MAX_SEARCH_TERMS } from '../shared/constants'; +import type { CodingPlan } from '../shared/schemas/workspace.schema'; +import { buildCodingPlan, extractObjectiveKeywords } from '../shared/workspace/plan'; +import type { CodingPlanObservations, ObservedFile } from '../shared/workspace/plan'; + +/** + * Runs the bounded searches that identify candidate files. + * + * At most {@link WORKSPACE_PLAN_MAX_SEARCH_TERMS} terms are searched, not + * every keyword: each search opens up to `WORKSPACE_MAX_SEARCH_FILES` files, + * so an unbounded number of terms would multiply into an unbounded amount of + * reading. The terms are already ordered longest-first by + * `extractObjectiveKeywords`, and a longer term is the more specific one, so + * truncating the list keeps the searches that discriminate best. + * + * A file's score is the number of *distinct terms* that matched it, not the + * number of matches: a file mentioning three of the objective's terms once + * each is a better candidate than one mentioning a single term thirty times. + */ +async function searchForKeywords( + project: ApprovedProject, + keywords: readonly string[], +): Promise<{ + readonly scores: ReadonlyMap; + readonly filesInspected: number; + readonly truncated: boolean; +}> { + const scores = new Map(); + let filesInspected = 0; + let truncated = false; + + for (const keyword of keywords.slice(0, WORKSPACE_PLAN_MAX_SEARCH_TERMS)) { + const result = await searchProject(project, keyword, ''); + filesInspected += result.filesScanned; + if (result.truncated) truncated = true; + + const seen = new Set(); + for (const match of result.matches) { + if (seen.has(match.path)) continue; + seen.add(match.path); + scores.set(match.path, (scores.get(match.path) ?? 0) + 1); + } + } + + return { scores, filesInspected, truncated }; +} + +/** + * Adds files whose *path* mentions a keyword, even when their contents do + * not. + * + * A request about "onboarding" should surface `src/renderer/Onboarding.tsx` + * whether or not the word appears inside it. These score below a content + * match, which is what the fractional weight expresses — it keeps a + * path-only match from outranking a file that genuinely discusses the term. + */ +function scorePathMatches( + paths: readonly string[], + keywords: readonly string[], + scores: Map, +): void { + if (keywords.length === 0) return; + for (const path of paths) { + const lowered = path.toLowerCase(); + const hits = keywords.filter((keyword) => lowered.includes(keyword)).length; + if (hits > 0) scores.set(path, (scores.get(path) ?? 0) + hits * 0.5); + } +} + +/** + * Scores files whose path mentions a keyword, from one bounded listing of the + * project root, and reports whether that listing was complete. + * + * Returns `true` — "the picture is partial" — for both a truncated listing + * and a root that could not be listed at all. A project whose root is + * unreadable still produces a plan, one that says so through the builder's + * own observed risks; failing the whole request here would be less useful and + * no safer, since nothing about a plan can act on anything. + * + * Written to return the flag rather than assign an outer `let`: a variable + * that is only ever written inside a `try` is exactly the shape a lint + * auto-fix will "simplify", and simplifying control flow it has misread is + * how a behaviour change slips in unreviewed. + */ +async function scoreProjectPaths( + project: ApprovedProject, + keywords: readonly string[], + scores: Map, +): Promise { + try { + const tree = await listProjectTree(project, ''); + const readablePaths = tree.entries + .filter((entry) => entry.kind === 'file' && entry.readable) + .map((entry) => entry.path); + scorePathMatches(readablePaths, keywords, scores); + return tree.truncated; + } catch { + return true; + } +} + +/** Everything the plan builder needs, measured from the real project. */ +export async function gatherPlanObservations( + project: ApprovedProject, + objective: string, +): Promise { + const keywords = extractObjectiveKeywords(objective); + const { scores, filesInspected, truncated } = await searchForKeywords(project, keywords); + + const mutableScores = new Map(scores); + const treeTruncated = await scoreProjectPaths(project, keywords, mutableScores); + + const files: ObservedFile[] = [...mutableScores.entries()] + .map(([path, score]) => ({ path, matchCount: Math.round(score) })) + .sort((a, b) => + b.matchCount === a.matchCount ? a.path.localeCompare(b.path) : b.matchCount - a.matchCount, + ) + .slice(0, WORKSPACE_PLAN_MAX_FILES); + + return { + projectName: project.name, + markers: project.markers, + hasGitMetadata: project.hasGitMetadata, + hasTestTooling: project.hasTestTooling, + files, + filesInspected, + searchTruncated: truncated || treeTruncated, + }; +} + +/** + * Produces one inert coding plan for an approved project. + * + * `now` is supplied by the caller, following the convention every other + * module here follows: nothing in the workspace layer reads the clock itself, + * so a test's plan is byte-for-byte reproducible. + */ +export async function createCodingPlan( + project: ApprovedProject, + objective: string, + now: string, +): Promise { + const observations = await gatherPlanObservations(project, objective); + return buildCodingPlan({ objective, observations, generatedAt: now }); +} diff --git a/src/main/workspace-session.ts b/src/main/workspace-session.ts new file mode 100644 index 0000000..e1141eb --- /dev/null +++ b/src/main/workspace-session.ts @@ -0,0 +1,236 @@ +/** + * The approved-project session (Phase 2, Milestone 5). + * + * Holds exactly one thing: which directory the user approved in the native + * picker during *this* run of the application. + * + * Three properties, each deliberate: + * + * - **In memory only.** Nothing here is persisted. The approved path is not + * written to `settings.json`, not stored in the secret store, and not + * remembered across restarts. Read access to someone's source tree is not + * a preference to be restored silently on the next launch; it is a grant + * the user makes, per session, by clicking through a dialog. The milestone + * brief says "store only the approved project path in the current + * session", and this is that, literally. + * - **One per `registerIpcHandlers` call**, exactly like `main/ipc.ts`'s + * in-flight chat request map — never a module-level singleton, so one + * test's approved project can never leak into another's. + * - **Nothing outside this module can set it.** The only way in is + * {@link adoptProjectDirectory}, which canonicalises the path, confirms it + * is a directory, and refuses the application's own data directory. + */ + +import { basename, dirname, join } from 'node:path'; +import { realpath, stat } from 'node:fs/promises'; + +import { isContainedPath, toWorkspaceError } from './workspace-paths'; +import { workspaceProjectSummarySchema } from '../shared/schemas/workspace.schema'; +import type { WorkspaceProjectSummary } from '../shared/schemas/workspace.schema'; +import { WorkspaceError } from '../shared/workspace/errors'; +import { + WORKSPACE_PROJECT_MARKER_FILES, + WORKSPACE_TEST_MARKER_FILES, +} from '../shared/workspace/exclusions'; +import { joinAbsolute } from './workspace-paths'; + +/** One approved project, as the main process holds it. */ +export interface ApprovedProject { + /** The canonical root. Every path resolution is relative to this. */ + readonly rootPath: string; + readonly name: string; + readonly selectedAt: string; + /** Which of {@link WORKSPACE_PROJECT_MARKER_FILES} exist at the root. */ + readonly markers: readonly string[]; + readonly hasGitMetadata: boolean; + /** True when a marker implying an automated test setup was found. */ + readonly hasTestTooling: boolean; +} + +export interface WorkspaceSession { + /** The approved project, or `null` when the user has not chosen one. */ + get(): ApprovedProject | null; + set(project: ApprovedProject): void; + clear(): void; +} + +export function createWorkspaceSession(): WorkspaceSession { + let approved: ApprovedProject | null = null; + return { + get: () => approved, + set: (project) => { + approved = project; + }, + clear: () => { + approved = null; + }, + }; +} + +/** + * Returns the approved project, or throws the normalized "no project" error. + * + * Every workspace operation except selection itself starts here, so there is + * one place — not five — where "the renderer asked to read something before + * approving anything" is decided. + */ +export function requireApprovedProject(session: WorkspaceSession): ApprovedProject { + const project = session.get(); + if (project === null) throw new WorkspaceError('WORKSPACE_NO_PROJECT'); + return project; +} + +/** True when the entry exists, whatever it is. Never reads it. */ +async function entryExists(absolutePath: string): Promise { + try { + await stat(absolutePath); + return true; + } catch { + return false; + } +} + +/** + * Detects which project markers exist at the root. + * + * Presence only — not one of these files is opened. `.git` is detected the + * same way, which is the whole of this milestone's "Git metadata" support: + * the interface learns that a project is version-controlled without anything + * listing or reading a single byte inside `.git`, which stays on the excluded + * list for every other purpose. + */ +async function detectMarkers(rootPath: string): Promise { + const found: string[] = []; + for (const marker of WORKSPACE_PROJECT_MARKER_FILES) { + if (await entryExists(joinAbsolute(rootPath, marker))) found.push(marker); + } + return found; +} + +/** + * Canonicalises a path that may not exist yet. + * + * `%APPDATA%\Local-Agent` is created on first write, so on a clean install it + * genuinely may not be there — but the comparison it is used for still has to + * work. When the directory itself cannot be resolved, its parent is + * canonicalised and the final segment re-appended, which is enough because + * `%APPDATA%` always exists. If even that fails, the raw path is returned: + * a comparison against an uncanonical path is weaker than one against a + * canonical path, but it is never *wrong* in the permissive direction for the + * ordinary case where no short name is involved. + */ +async function canonicalizeExistingOrParent(path: string): Promise { + try { + return await realpath(path); + } catch { + try { + return join(await realpath(dirname(path)), basename(path)); + } catch { + return path; + } + } +} + +export interface AdoptProjectOptions { + /** The raw path the native picker returned. Still treated as input. */ + readonly chosenPath: string; + /** UTC ISO-8601, supplied by the caller. This module reads no clock. */ + readonly now: string; + /** + * The application's own user-data directory, refused as a project root. + * + * Opening `%APPDATA%\Local-Agent` as a "project" would point the inspector + * at the settings file, the permission policy and the audit log. The + * encrypted secret store would still be refused by name + * (`isCredentialFileName` matches the `secrets` stem), and everything would + * still be read-only — but the application's own state is categorically not + * a coding project, and refusing it outright is both cheaper and clearer + * than reasoning about which parts of it happen to be safe. + */ + readonly userDataDir: string; +} + +/** + * Turns a picker result into an approved project, or throws a + * {@link WorkspaceError}. + * + * The dialog result is *not* trusted merely because a native dialog produced + * it. It is canonicalised, confirmed to be a directory, checked against the + * application's own data directory in both directions, and finally validated + * as a {@link WorkspaceProjectSummary} — so a directory whose own name + * carries a control character or a bidirectional override is refused here, + * before it can be displayed or become a root that later paths are joined to. + */ +export async function adoptProjectDirectory( + options: AdoptProjectOptions, +): Promise { + const { chosenPath, now, userDataDir } = options; + + let rootPath: string; + try { + rootPath = await realpath(chosenPath); + } catch (error) { + // A picker result that no longer resolves is an unusable project, not a + // missing file the caller asked for by name. + void toWorkspaceError(error); + throw new WorkspaceError('WORKSPACE_INVALID_PROJECT'); + } + + let isDirectory: boolean; + try { + isDirectory = (await stat(rootPath)).isDirectory(); + } catch { + throw new WorkspaceError('WORKSPACE_INVALID_PROJECT'); + } + if (!isDirectory) throw new WorkspaceError('WORKSPACE_INVALID_PROJECT'); + + // Both directions: the chosen root must not be inside the application's + // data directory, and must not contain it either (choosing `%APPDATA%` + // itself, or a drive root, would otherwise pull it in). + // + // Both sides must be canonical for the comparison to mean anything. + // `rootPath` already is; `userDataDir` arrives as `main/paths.ts` built it, + // from whatever `app.getPath('appData')` returned, which on Windows can + // carry a short (8.3) ancestor such as `C:\Users\VUNHAT~1\…`. Comparing a + // short form against a long one finds no common root at all, and the guard + // would silently pass — so it is canonicalised here first. + const canonicalUserDataDir = await canonicalizeExistingOrParent(userDataDir); + if ( + isContainedPath(canonicalUserDataDir, rootPath) || + isContainedPath(rootPath, canonicalUserDataDir) + ) { + throw new WorkspaceError('WORKSPACE_INVALID_PROJECT'); + } + + const name = basename(rootPath); + const markers = await detectMarkers(rootPath); + + const project: ApprovedProject = { + rootPath, + name, + selectedAt: now, + markers, + hasGitMetadata: markers.includes('.git'), + hasTestTooling: markers.some((marker) => WORKSPACE_TEST_MARKER_FILES.includes(marker)), + }; + + // Refuses a root that could not be described safely — an empty name (a + // drive root such as `C:\`), or a name or path carrying a control character + // or a bidirectional override. + if (!workspaceProjectSummarySchema.safeParse(toProjectSummary(project)).success) { + throw new WorkspaceError('WORKSPACE_INVALID_PROJECT'); + } + + return project; +} + +/** The renderer-facing view of an approved project. */ +export function toProjectSummary(project: ApprovedProject): WorkspaceProjectSummary { + return { + name: project.name, + path: project.rootPath, + selectedAt: project.selectedAt, + markers: [...project.markers], + hasGitMetadata: project.hasGitMetadata, + }; +} diff --git a/src/preload/index.ts b/src/preload/index.ts index 27e31a8..98b50c1 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -19,6 +19,66 @@ import { secretsWriteResponseSchema, settingsGetResponseSchema, settingsUpdateResponseSchema, + IPC_WORKSPACE_FILE_CHANNEL, + IPC_WORKSPACE_PLAN_CHANNEL, + IPC_WORKSPACE_SEARCH_CHANNEL, + IPC_WORKSPACE_SELECT_CHANNEL, + IPC_WORKSPACE_STATUS_CHANNEL, + IPC_WORKSPACE_TREE_CHANNEL, + workspaceFileResponseSchema, + workspacePlanResponseSchema, + workspaceSearchResponseSchema, + workspaceSelectResponseSchema, + workspaceStatusResponseSchema, + workspaceTreeResponseSchema, + commandCancelResponseSchema, + commandListResponseSchema, + commandRunResponseSchema, + gitCheckpointResponseSchema, + gitDiffResponseSchema, + gitStatusResponseSchema, + IPC_COMMAND_CANCEL_CHANNEL, + IPC_COMMAND_LIST_CHANNEL, + IPC_COMMAND_RUN_CHANNEL, + IPC_GIT_CHECKPOINT_CHANNEL, + IPC_GIT_DIFF_CHANNEL, + IPC_GIT_STATUS_CHANNEL, + IPC_WORKSPACE_APPLY_CHANNEL, + IPC_WORKSPACE_CHANGES_CHANNEL, + IPC_WORKSPACE_PROPOSE_CHANNEL, + IPC_WORKSPACE_ROLLBACK_CHANNEL, + workspaceApplyResponseSchema, + workspaceChangesResponseSchema, + workspaceProposeResponseSchema, + workspaceRollbackResponseSchema, + agentCancelResponseSchema, + agentCreateResponseSchema, + agentDeleteResponseSchema, + agentListResponseSchema, + agentRunResponseSchema, + agentSelectResponseSchema, + agentSetEnabledResponseSchema, + agentUpdateResponseSchema, + IPC_AGENT_CANCEL_CHANNEL, + IPC_AGENT_CREATE_CHANNEL, + IPC_AGENT_DELETE_CHANNEL, + IPC_AGENT_LIST_CHANNEL, + IPC_AGENT_RUN_CHANNEL, + IPC_AGENT_SELECT_CHANNEL, + IPC_AGENT_SET_ENABLED_CHANNEL, + IPC_AGENT_UPDATE_CHANNEL, + type AgentProfileInput, + type AgentRegistryResponse, + type AgentRunResponse, + type CommandIdValue, + type CommandListResponse, + type CommandRunResponse, + type GitCheckpointResponse, + type GitDiffResponse, + type GitStatusResponse, + type WorkspaceChangeResponse, + type WorkspaceChangesResponse, + type WorkspaceEdit, type ChatChunkEvent, type ChatMessage, type ChatSendResponse, @@ -26,6 +86,11 @@ import { type SecretsActionResponse, type SettingsActionResponse, type SettingsUpdateInput, + type WorkspaceFileResponse, + type WorkspacePlanResponse, + type WorkspaceProjectResponse, + type WorkspaceSearchResponse, + type WorkspaceTreeResponse, } from '../shared/schemas'; /** @@ -143,6 +208,197 @@ const bridge = { }; }, }, + /** + * The read-only coding workspace (Phase 2, Milestone 5). + * + * Six functions, six fixed channels, and — the property worth checking + * first — **no way to name a directory**. `select` takes no argument at + * all: the main process opens a native picker and the user chooses, so + * nothing on this bridge can point the application at a directory of the + * caller's choosing. Every other function takes a path *relative* to + * whatever the user already approved, which the main process validates for + * containment before it touches the filesystem and re-validates after + * resolving it. + * + * There is no `write`, no `create`, no `delete` and no `apply`. The absence + * is the control: a renderer cannot ask for a modification because no + * function here expresses one. + */ + workspace: { + status: async (): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_WORKSPACE_STATUS_CHANNEL); + return workspaceStatusResponseSchema.parse(result); + }, + select: async (): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_WORKSPACE_SELECT_CHANNEL); + return workspaceSelectResponseSchema.parse(result); + }, + tree: async (path: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_WORKSPACE_TREE_CHANNEL, { path }); + return workspaceTreeResponseSchema.parse(result); + }, + file: async (path: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_WORKSPACE_FILE_CHANNEL, { path }); + return workspaceFileResponseSchema.parse(result); + }, + search: async (query: string, path: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_WORKSPACE_SEARCH_CHANNEL, { + query, + path, + }); + return workspaceSearchResponseSchema.parse(result); + }, + plan: async (objective: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_WORKSPACE_PLAN_CHANNEL, { objective }); + return workspacePlanResponseSchema.parse(result); + }, + /** + * Proposes a change and gets back a diff (Phase 2, Milestone 6). + * + * The only function on this bridge that carries file content, and it + * writes nothing: the main process keeps the proposal and answers with a + * diff to show. Applying it is {@link LocalAgentBridge.coding}'s `apply`, + * which takes an id. + */ + propose: async (edits: readonly WorkspaceEdit[]): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_WORKSPACE_PROPOSE_CHANNEL, { edits }); + return workspaceProposeResponseSchema.parse(result); + }, + /** + * Applies a change the main process already holds, already diffed and + * already shown. + * + * Takes a change id and nothing else — no path, no content, no + * destination — so the bytes written are necessarily the bytes that were + * diffed. There is no function here that writes content directly, and + * none that creates or deletes a file. + */ + apply: async (changeId: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_WORKSPACE_APPLY_CHANNEL, { changeId }); + return workspaceApplyResponseSchema.parse(result); + }, + /** Restores the backup taken before the most recent applied change. */ + rollback: async (changeId: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_WORKSPACE_ROLLBACK_CHANNEL, { + changeId, + }); + return workspaceRollbackResponseSchema.parse(result); + }, + changes: async (): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_WORKSPACE_CHANGES_CHANNEL); + return workspaceChangesResponseSchema.parse(result); + }, + }, + /** + * The command registry (Phase 2, Milestone 6). + * + * `run` takes an identifier from a five-value enum. There is no parameter + * here for a command string, an argument, a shell, a working directory or + * an environment variable — so this bridge cannot express an arbitrary + * command, in the same way `workspace.select` cannot express a directory. + * `cancel` is fire-and-forget best effort, exactly like `chat.cancel`. + */ + command: { + list: async (): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_COMMAND_LIST_CHANNEL); + return commandListResponseSchema.parse(result); + }, + run: async (runId: string, commandId: CommandIdValue): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_COMMAND_RUN_CHANNEL, { + runId, + commandId, + }); + return commandRunResponseSchema.parse(result); + }, + cancel: async (runId: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_COMMAND_CANCEL_CHANNEL, { runId }); + commandCancelResponseSchema.parse(result); + }, + }, + /** + * Git (Phase 2, Milestone 6). + * + * `status` and `diff` read. `checkpoint` takes no argument at all: the + * branch is whatever is already checked out, the message is generated by + * the main process, and the command vector is fixed — so nothing here can + * name a ref, a remote, a branch or a commit message, and there is no + * function for a reset, a checkout, a push or a delete. + */ + /** + * Agent profiles and runs (Phase 2, Milestone 7). + * + * Eight functions, eight fixed channels, and — the property worth checking + * first — **none of them can grant a permission**. `create` and `update` + * carry a profile whose own permission entries are `confirm` or `deny`; + * `allow` is not a member of that enum, so "permit this" is not + * expressible through this bridge at all. + * + * `run` takes a run id and an objective. It cannot carry a step list, a + * tool, a path, a command or a limit: what a run may do comes from the + * profile the main process reads from disk, so a compromised renderer can + * ask for a run and cannot widen one. `cancel` is fire-and-forget best + * effort, exactly like `chat.cancel` and `command.cancel`. + */ + agent: { + list: async (): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_AGENT_LIST_CHANNEL); + return agentListResponseSchema.parse(result); + }, + select: async (profileId: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_AGENT_SELECT_CHANNEL, { profileId }); + return agentSelectResponseSchema.parse(result); + }, + create: async (profile: AgentProfileInput): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_AGENT_CREATE_CHANNEL, { profile }); + return agentCreateResponseSchema.parse(result); + }, + update: async ( + profileId: string, + profile: AgentProfileInput, + ): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_AGENT_UPDATE_CHANNEL, { + profileId, + profile, + }); + return agentUpdateResponseSchema.parse(result); + }, + remove: async (profileId: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_AGENT_DELETE_CHANNEL, { profileId }); + return agentDeleteResponseSchema.parse(result); + }, + setEnabled: async (profileId: string, enabled: boolean): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_AGENT_SET_ENABLED_CHANNEL, { + profileId, + enabled, + }); + return agentSetEnabledResponseSchema.parse(result); + }, + run: async (runId: string, objective: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_AGENT_RUN_CHANNEL, { + runId, + objective, + }); + return agentRunResponseSchema.parse(result); + }, + cancel: async (runId: string): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_AGENT_CANCEL_CHANNEL, { runId }); + agentCancelResponseSchema.parse(result); + }, + }, + git: { + status: async (): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_GIT_STATUS_CHANNEL); + return gitStatusResponseSchema.parse(result); + }, + diff: async (path: string | null): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_GIT_DIFF_CHANNEL, { path }); + return gitDiffResponseSchema.parse(result); + }, + checkpoint: async (): Promise => { + const result: unknown = await ipcRenderer.invoke(IPC_GIT_CHECKPOINT_CHANNEL); + return gitCheckpointResponseSchema.parse(result); + }, + }, }; contextBridge.exposeInMainWorld('localAgent', bridge); diff --git a/src/renderer/App.tsx b/src/renderer/App.tsx index 4d4eb02..5b1020d 100644 --- a/src/renderer/App.tsx +++ b/src/renderer/App.tsx @@ -1,11 +1,15 @@ import { useEffect, useState } from 'react'; +import { Agents } from './agent/Agents'; import { Chat } from './chat/Chat'; import { Onboarding } from './Onboarding'; +import { Workspace } from './workspace/Workspace'; import { createDefaultSettings } from '../shared/schemas'; import type { Settings } from '../shared/schemas'; type Phase = 'loading' | 'onboarding' | 'ready'; +/** Which surface the shell is showing (Phase 2, Milestones 5 and 7). */ +type View = 'chat' | 'workspace' | 'agents'; /** * Used only when `settings.get` itself did not succeed (denied, or the @@ -22,6 +26,7 @@ export function App() { const [phase, setPhase] = useState('loading'); const [settings, setSettings] = useState(null); const [health, setHealth] = useState('checking'); + const [view, setView] = useState('chat'); useEffect(() => { void window.localAgent.health().then((result) => { @@ -77,8 +82,46 @@ export function App() {

Welcome back, {settings.user.displayName}.

)}

Main process: {health}

+ {/* + Two surfaces, one shell (Phase 2, Milestone 5). A plain view switch + rather than a redesign: the chat surface is exactly what it was, and + the workspace is a sibling next to it. + */} + - + {view === 'chat' && } + {view === 'workspace' && ( + + )} + {view === 'agents' && } ); } diff --git a/src/renderer/agent/Agents.tsx b/src/renderer/agent/Agents.tsx new file mode 100644 index 0000000..6c84f47 --- /dev/null +++ b/src/renderer/agent/Agents.tsx @@ -0,0 +1,584 @@ +/** + * The agent profile surface (Phase 2, Milestone 7). + * + * A third sibling next to Chat and Workspace, not a redesign: it reuses the + * existing panel, badge, hint and error classes rather than introducing a + * visual language of its own. + * + * What it is for, in the milestone's own terms: listing profiles, choosing + * the active one, showing each profile's provider and capability summary, and + * **stating plainly what the active profile is permitted to do and what + * bounds it runs under**. That last point is why every profile card spells + * out its tools, the action type each tool routes through, its workspace + * scope and its three ceilings, rather than summarising them as a count. + * + * Nothing here is a security control. The form validates before sending + * purely so a person sees a mistake immediately; the main process re-validates + * everything it receives and is the only thing that decides what is stored. + */ + +import { useState } from 'react'; + +import { useAgent } from './useAgent'; +import { + AGENT_TOOLS, + AGENT_VERIFICATION_REQUIREMENTS, + DEFAULT_AGENT_PROFILE_ID, + describeProfileTools, +} from '../../shared/agent'; +import type { AgentToolId, AgentVerificationRequirement } from '../../shared/agent'; +import { + AGENT_DEFAULT_MAX_DURATION_MS, + AGENT_DEFAULT_MAX_OUTPUT_BYTES, + AGENT_DEFAULT_MAX_STEPS, + AGENT_MAX_DURATION_MS, + AGENT_MAX_OUTPUT_BYTES, + AGENT_MAX_STEPS, + AGENT_MIN_DURATION_MS, + AGENT_MIN_OUTPUT_BYTES, + AGENT_MIN_STEPS, + MODEL_PROVIDERS, + WORKSPACE_OBJECTIVE_MIN_LENGTH, +} from '../../shared/constants'; +import { agentProfileInputSchema } from '../../shared/schemas'; +import type { AgentProfile, AgentProfileInput, AgentRun } from '../../shared/schemas'; + +/** A blank profile for the create form: no tools, nothing in scope. */ +function emptyDraft(): AgentProfileInput { + return { + id: '', + name: '', + description: '', + instructions: '', + provider: 'none', + fallbackProviders: [], + allowedTools: [], + approvedWorkspacePaths: [''], + permissionPolicy: [], + verification: [], + limits: { + maxSteps: AGENT_DEFAULT_MAX_STEPS, + maxDurationMs: AGENT_DEFAULT_MAX_DURATION_MS, + maxOutputBytes: AGENT_DEFAULT_MAX_OUTPUT_BYTES, + }, + enabled: true, + }; +} + +/** Strips the derived fields so an existing profile can seed the form. */ +function draftFrom(profile: AgentProfile): AgentProfileInput { + const { builtIn: _builtIn, createdAt: _createdAt, updatedAt: _updatedAt, ...input } = profile; + return input; +} + +function formatScope(paths: readonly string[]): string { + if (paths.length === 0) return 'nothing'; + return paths.map((path) => (path === '' ? 'the whole project' : path)).join(', '); +} + +function toggle(values: readonly TValue[], value: TValue): TValue[] { + return values.includes(value) ? values.filter((entry) => entry !== value) : [...values, value]; +} + +function ProfileCard(props: { + readonly profile: AgentProfile; + readonly active: boolean; + readonly canAct: boolean; + readonly onSelect: () => void; + readonly onEdit: () => void; + readonly onDelete: () => void; + readonly onToggleEnabled: () => void; +}) { + const { profile, active, canAct } = props; + const tools = describeProfileTools(profile); + const startsProcess = tools.some((tool) => tool.commandId !== null); + + return ( +
+
+

{profile.name}

+ {profile.builtIn ? 'Built in' : 'Custom'} + {active && Active} + {!profile.enabled && Disabled} +
+ +

{profile.description}

+ +
+
Provider
+
+ {profile.provider} + {profile.fallbackProviders.length > 0 && + ` (falls back to ${profile.fallbackProviders.join(', ')})`} +
+ +
Allowed tools
+
+ {tools.length === 0 ? ( + 'none — this profile can do nothing' + ) : ( +
    + {tools.map((tool) => ( +
  • + {tool.label} {tool.actionType} +
    • + ))} +
+ )} +
+ +
Workspace scope
+
{formatScope(profile.approvedWorkspacePaths)}
+ +
Limits
+
+ {profile.limits.maxSteps} steps · {Math.round(profile.limits.maxDurationMs / 1000)}s ·{' '} + {profile.limits.maxOutputBytes} bytes +
+ +
Verification
+
+ {profile.verification.length === 0 ? 'none required' : profile.verification.join(', ')} +
+
+ +

+ {startsProcess + ? 'Can run the project’s own scripts. Each one is confirmed separately before it starts.' + : 'Cannot start a process, change a file, or create a commit.'} +

+ +
+ + + + +
+
+ ); +} + +function RunReport(props: { readonly run: AgentRun }) { + const { run } = props; + return ( +
+

+ {run.profileName} — {run.status} +

+

+ Stopped because: {run.stopReason}. {run.totals.steps} step(s), {run.totals.outputBytes}{' '} + bytes, {Math.round(run.totals.durationMs / 1000)}s. Provider recorded: {run.provider}. +

+

+ Verification:{' '} + {run.verification.required.length === 0 + ? 'none required' + : `${run.verification.satisfied.length}/${run.verification.required.length} satisfied (${run.verification.required.join(', ')})`} +

+
    + {run.steps.map((step) => ( +
  1. + {step.tool} + {step.actionType} + {step.outcome} + {step.summary} +
    2. + ))} +
+ {run.steps.length === 0 &&

No step ran.

} +
+ ); +} + +export function Agents() { + const agent = useAgent(); + const { state, canAct } = agent; + + const [objective, setObjective] = useState(''); + const [draft, setDraft] = useState(null); + const [editingId, setEditingId] = useState(null); + + // Validated here only so a mistake is visible immediately. The main process + // re-validates everything it receives and is what actually decides. + const validation = draft === null ? null : agentProfileInputSchema.safeParse(draft); + const draftIssues = + validation === null || validation.success + ? [] + : validation.error.issues.map( + (issue) => `${issue.path.join('.') || 'profile'}: ${issue.message}`, + ); + + const activeProfile = + state.profiles.find((profile) => profile.id === state.activeProfileId) ?? null; + + function updateDraft(changes: Partial): void { + setDraft((current) => (current === null ? current : { ...current, ...changes })); + } + + async function submitDraft(): Promise { + if (draft === null || validation === null || !validation.success) return; + if (editingId === null) await agent.createProfile(validation.data); + else await agent.updateProfile(editingId, validation.data); + setDraft(null); + setEditingId(null); + } + + return ( +
+
+
+

Agents

+

+ A profile narrows what an agent may reach for. It can never grant a permission the + policy does not already allow. +

+
+
+

+ {state.busy === null ? 'Idle' : `Working: ${state.busy}`} +

+ {state.activity !== null &&

{state.activity}

} +
+
+ + {state.error !== null && ( +
+

{state.error.message}

+ + {state.error.retryable && ( + + )} + + +
+ )} + + {!state.initialized &&

Loading profiles…

} + +
+
+
+

Profiles

+ +
+ + {state.profiles.map((profile) => ( + void agent.selectProfile(profile.id)} + onEdit={() => { + setDraft(draftFrom(profile)); + setEditingId(profile.id); + }} + onDelete={() => void agent.deleteProfile(profile.id)} + onToggleEnabled={() => void agent.setProfileEnabled(profile.id, !profile.enabled)} + /> + ))} + + {state.initialized && state.profiles.length === 0 && ( +

No profiles are available.

+ )} +
+ +
+

Run

+ {activeProfile === null ? ( +

No profile is active.

+ ) : ( + <> +

+ Active: {activeProfile.name} —{' '} + {describeProfileTools(activeProfile).length} tool(s), scope{' '} + {formatScope(activeProfile.approvedWorkspacePaths)}, at most{' '} + {activeProfile.limits.maxSteps} steps. +

+

+ A run inspects, plans and verifies. It cannot write a file, undo a write, or create + a commit — no tool exists for any of those. +

+
+ +