Security hardening: setting scopes, workspace-confined agent fs access, permission matching

- Scope sigit.agents, sigit.agent.default, sigit.registry.url, and sigit.permission.mode as machine settings so a workspace's .vscode/settings.json can no longer inject an arbitrary agent command, flip the permission mode to allow, or point the registry at a malicious catalog. - Declare untrustedWorkspaces/virtualWorkspaces capabilities so the extension stays disabled in Restricted Mode instead of silently spawning agents. - Confine the agent's fs/read_text_file and fs/write_text_file channel to the workspace folders; paths outside the workspace now follow sigit.permission.mode (allow / deny / modal prompt) instead of being granted unconditionally. - Classify permission options by the ACP kind field first; the previous unanchored name regex let agent-chosen labels like "Allow now" match /no/ and be auto-selected as the deny option (and vice versa). The name fallback is now word-bounded. - Reject in-flight JSON-RPC requests when the agent closes the connection, so a crashed agent no longer leaves the chat stuck busy. - Cap the ndjson read buffer at 16 MiB and truncate unparseable lines in error messages. - Honor the ACP line/limit parameters in fs/read_text_file instead of always returning the whole file. - Generate the webview CSP nonce with crypto.randomBytes instead of Math.random. - Run the registry parser test in CI alongside smoke and webview tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YDM1YxP1jqcjSGpcXwKtwG

Claude committed Jul 5, 2026 at 21:58 UTC 2e06c39c12aa43b4458eaf96228594dc8dc5ae08
4 files changed +100 -16
.github/workflows/ci.yml
+3
index 11d2e02..8748f75 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,3 +38,6 @@ jobs: - name: Webview test (tool-call rendering) run: pnpm run test:webview + + - name: Registry test (catalog parsing) + run: pnpm run test:registry
package.json
+14
index 6a61dd9..a78bfc8 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,16 @@ ], "main": "./dist/extension.js", "activationEvents": [], + "capabilities": { + "untrustedWorkspaces": { + "supported": false, + "description": "siGit spawns agent processes that can read and modify workspace files, so it requires a trusted workspace." + }, + "virtualWorkspaces": { + "supported": false, + "description": "siGit spawns local agent processes and needs a local file system." + } + }, "contributes": { "commands": [ { @@ -95,16 +105,19 @@ "properties": { "sigit.agent.default": { "type": "string", + "scope": "machine", "default": "sigit", "markdownDescription": "Key of the agent to use by default, looked up in `#sigit.agents#`. Defaults to the on-device `sigit` binary." }, "sigit.registry.url": { "type": "string", + "scope": "machine", "default": "https://cdn.agentclientprotocol.com/registry/v1/latest/registry.json", "markdownDescription": "URL of the [Agent Client Protocol registry](https://github.com/agentclientprotocol/registry) catalog used by **siGit: Browse Agent Registry**. Installing an agent only registers a launch command in `#sigit.agents#` (via `npx`/`uvx`, or a binary you place on your PATH); siGit never downloads or runs agent binaries on your behalf." }, "sigit.agents": { "type": "object", + "scope": "machine", "markdownDescription": "Registry of ACP agents that siGit Code can spawn. Each entry maps a key to an agent definition. The on-device `sigit` agent is the local-first default; add other ACP-compatible agents here.", "default": { "sigit": { @@ -147,6 +160,7 @@ }, "sigit.permission.mode": { "type": "string", + "scope": "machine", "enum": [ "prompt", "allow",
src/acp/connection.ts
+25 -6
index 7f66676..81746e3 100644 --- a/src/acp/connection.ts +++ b/src/acp/connection.ts @@ -61,12 +61,19 @@ export class Connection extends EventEmitter { private nextId = 1; private disposed = false; + /** Cap on buffered bytes awaiting a newline, so a misbehaving agent can't grow memory unboundedly. */ + private static readonly MAX_BUFFER_LENGTH = 16 * 1024 * 1024; + constructor(readable: Readable, writable: Writable) { super(); this.writable = writable; readable.on("data", (chunk: Buffer | string) => this.onData(chunk.toString())); readable.on("error", (err) => this.emit("error", err)); - readable.on("close", () => this.emit("close")); + readable.on("close", () => { + // The agent is gone; unblock every in-flight request instead of hanging. + this.failPending(new Error("Agent closed the connection")); + this.emit("close"); + }); } /** Send a request and resolve with its result (or reject on error). */ @@ -109,14 +116,17 @@ export class Connection extends EventEmitter { return; } this.disposed = true; - const err = new Error("Connection disposed"); + this.failPending(new Error("Connection disposed")); + this.requestHandlers.clear(); + this.notificationHandlers.clear(); + this.removeAllListeners(); + } + + private failPending(err: Error): void { for (const { reject } of this.pending.values()) { reject(err); } this.pending.clear(); - this.requestHandlers.clear(); - this.notificationHandlers.clear(); - this.removeAllListeners(); } private send(message: JsonValue | JsonRpcRequest | JsonRpcNotification | JsonRpcResponse): void { @@ -129,6 +139,14 @@ export class Connection extends EventEmitter { private onData(text: string): void { this.buffer += text; + if (this.buffer.length > Connection.MAX_BUFFER_LENGTH && !this.buffer.includes("\n")) { + this.buffer = ""; + this.emit( + "error", + new Error(`Agent sent more than ${Connection.MAX_BUFFER_LENGTH} bytes without a newline; discarding`) + ); + return; + } let newlineIndex: number; while ((newlineIndex = this.buffer.indexOf("\n")) !== -1) { const line = this.buffer.slice(0, newlineIndex).trim(); @@ -145,7 +163,8 @@ export class Connection extends EventEmitter { try { message = JSON.parse(line); } catch (err) { - this.emit("error", new Error(`Failed to parse message: ${line}`)); + // Truncate: the line is agent output and can be arbitrarily large. + this.emit("error", new Error(`Failed to parse message: ${line.slice(0, 256)}`)); return; }
src/chatView.ts
+58 -10
index 7065b59..6e8c14f 100644 --- a/src/chatView.ts +++ b/src/chatView.ts @@ -1,3 +1,5 @@ +import { randomBytes } from "crypto"; +import * as path from "path"; import * as vscode from "vscode"; import { AcpClient, PermissionRequest, PermissionResponse, SessionUpdate } from "./acp/client"; import { defaultAgentKey, listAgents, permissionMode, resolveAgent } from "./agents"; @@ -209,8 +211,13 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { private async handlePermission(request: PermissionRequest): Promise<PermissionResponse> { const mode = permissionMode(); const options = request.options ?? []; - const allowOption = options.find((o) => /allow|yes|approve/i.test(o.name ?? o.optionId)); - const denyOption = options.find((o) => /deny|no|reject|cancel/i.test(o.name ?? o.optionId)); + // Classify by the ACP `kind` field; the option *name* is agent-controlled + // free text ("Allow now" would otherwise match /no/), so it is only a + // word-bounded fallback for agents that omit `kind`. + const byKind = (kinds: string[]) => options.find((o) => o.kind !== undefined && kinds.includes(o.kind)); + const byName = (re: RegExp) => options.find((o) => re.test(o.name ?? o.optionId)); + const allowOption = byKind(["allow_once", "allow_always"]) ?? byName(/\b(allow|yes|approve)\b/i); + const denyOption = byKind(["reject_once", "reject_always"]) ?? byName(/\b(deny|no|reject|cancel)\b/i); if (mode === "allow" && allowOption) { return { outcome: { outcome: "selected", optionId: allowOption.optionId } }; @@ -256,17 +263,63 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { return `The agent wants to ${what}. Allow?`; } - private async readFile(params: { path: string }): Promise<string> { + private async readFile(params: { path: string; line?: number; limit?: number }): Promise<string> { const uri = this.resolveUri(params.path); + await this.ensureFsAccess(uri, "read"); const bytes = await vscode.workspace.fs.readFile(uri); - return Buffer.from(bytes).toString("utf8"); + let text = Buffer.from(bytes).toString("utf8"); + const { line, limit } = params; + if (typeof line === "number" || typeof limit === "number") { + const lines = text.split("\n"); + const start = typeof line === "number" ? Math.max(0, line - 1) : 0; + const end = typeof limit === "number" ? start + limit : lines.length; + text = lines.slice(start, end).join("\n"); + } + return text; } private async writeFile(params: { path: string; content: string }): Promise<void> { const uri = this.resolveUri(params.path); + await this.ensureFsAccess(uri, "write"); await vscode.workspace.fs.writeFile(uri, Buffer.from(params.content, "utf8")); } + /** + * Confine the agent's fs/* channel to the workspace. These requests carry no + * ACP permission prompt of their own, so paths outside every workspace + * folder follow `sigit.permission.mode`: allow silently, deny outright, or + * ask the user. + */ + private async ensureFsAccess(uri: vscode.Uri, action: "read" | "write"): Promise<void> { + if (this.isInWorkspace(uri)) { + return; + } + const mode = permissionMode(); + if (mode === "allow") { + return; + } + if (mode === "prompt") { + const allow = "Allow"; + const choice = await vscode.window.showWarningMessage( + `The agent wants to ${action} a file outside the workspace: ${uri.fsPath}`, + { modal: true }, + allow + ); + if (choice === allow) { + return; + } + } + throw new Error(`Access denied: ${action} outside the workspace (${uri.fsPath})`); + } + + private isInWorkspace(uri: vscode.Uri): boolean { + const folders = vscode.workspace.workspaceFolders ?? []; + return folders.some((folder) => { + const rel = path.relative(folder.uri.fsPath, uri.fsPath); + return rel === "" || (!rel.startsWith("..") && !path.isAbsolute(rel)); + }); + } + private resolveUri(p: string): vscode.Uri { if (p.startsWith("/") || /^[a-zA-Z]:[\\/]/.test(p)) { return vscode.Uri.file(p); @@ -392,10 +445,5 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { } function getNonce(): string { - const chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; - let text = ""; - for (let i = 0; i < 32; i++) { - text += chars.charAt(Math.floor(Math.random() * chars.length)); - } - return text; + return randomBytes(16).toString("hex"); }