From cb1a4be167827e6a2d83ec1e96bede02409a7f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 8 Oct 2026 19:07:28 -0400 Subject: [PATCH] Q117: commands run in an allowlisted environment, and records and approvals say host isolation is advisory Co-Authored-By: Claude Opus 5.5 --- .../migrations/088-host-access-note.ts | 11 ++++++++ src/main/database/schema.ts | 2 ++ src/main/services/organization-service.ts | 4 +-- src/main/services/task-approval.ts | 3 +++ src/main/services/task-run-preparation.ts | 3 +++ src/main/services/workspace-command.ts | 21 ++++++++++++++- tests/unit/domain-services.test.ts | 2 +- tests/unit/execution-modes.test.ts | 18 ++++++++++++- tests/unit/host-access-note-migration.test.ts | 26 +++++++++++++++++++ tests/unit/workspace-command.test.ts | 19 +++++++++++++- 10 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 src/main/database/migrations/088-host-access-note.ts create mode 100644 tests/unit/host-access-note-migration.test.ts diff --git a/src/main/database/migrations/088-host-access-note.ts b/src/main/database/migrations/088-host-access-note.ts new file mode 100644 index 00000000..14d87e4b --- /dev/null +++ b/src/main/database/migrations/088-host-access-note.ts @@ -0,0 +1,11 @@ +// Pending file-access approvals made before the warning existed get it too; the text is frozen here as it was introduced. +export const hostAccessNoteSchema = ` + update approvals + set details = details || char(10) || char(10) || 'Approving lets this task run commands on this computer with your user''s file access: they can read and change files outside the workspace. Only their environment is cleaned of keys and your profile.' + where state = 'pending' and source_entity_type = 'task' and kind in ('task_risk', 'task_execution') + and source_entity_id in (select id from tasks where requires_filesystem = 1) + and details not like '%Approving lets this task run commands on this computer%'; + + insert into schema_migrations(version, name, applied_at) + values (88, 'host_access_note', datetime('now')); +`; diff --git a/src/main/database/schema.ts b/src/main/database/schema.ts index e8268920..e03f157f 100644 --- a/src/main/database/schema.ts +++ b/src/main/database/schema.ts @@ -86,6 +86,7 @@ import { connectorGrantLimitsSchema } from "./migrations/084-connector-grant-lim import { approvalSubjectSnapshotSchema } from "./migrations/085-approval-subject-snapshot"; import { heldDeliverablesSchema } from "./migrations/086-held-deliverables"; import { goalAuditorSchema } from "./migrations/087-goal-auditor"; +import { hostAccessNoteSchema } from "./migrations/088-host-access-note"; interface Migration { version: number; @@ -181,6 +182,7 @@ const MIGRATIONS: Migration[] = [ { version: 85, name: "approval_subject_snapshot", sql: approvalSubjectSnapshotSchema }, { version: 86, name: "held_deliverables", sql: heldDeliverablesSchema }, { version: 87, name: "goal_auditor", sql: goalAuditorSchema }, + { version: 88, name: "host_access_note", sql: hostAccessNoteSchema }, ]; const SCHEMA_VERSION = MIGRATIONS[MIGRATIONS.length - 1].version; diff --git a/src/main/services/organization-service.ts b/src/main/services/organization-service.ts index 94f33bd7..93d8b461 100644 --- a/src/main/services/organization-service.ts +++ b/src/main/services/organization-service.ts @@ -12,7 +12,7 @@ import { TaskMutations } from "./task-mutations"; import { normalizeAvatarColor, normalizeAvatarImage } from "./avatar-values"; import { approvalDecision, autonomy } from "./org-approvals"; import { admitAgent } from "./agent-admission"; -import { TASK_APPROVAL_KINDS_SQL, TASK_SNAPSHOT_SQL } from "./task-approval"; +import { HOST_ACCESS_NOTE, TASK_APPROVAL_KINDS_SQL, TASK_SNAPSHOT_SQL } from "./task-approval"; export class OrganizationService { private readonly audit: AuditService; @@ -302,7 +302,7 @@ export class OrganizationService { id, org_id, kind, title, details, risk, requested_by_type, requested_by_id, source_entity_type, source_entity_id, state, decided_by_employee_id, created_at, decided_at, subject_snapshot ) values (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `).run(approvalId, orgId, approvalKind, task.title, task.description, RISK_LEVEL.HIGH, "system", null, "task", taskId, + `).run(approvalId, orgId, approvalKind, task.title, task.requires_filesystem === 1 ? `${task.description}\n\n${HOST_ACCESS_NOTE}` : task.description, RISK_LEVEL.HIGH, "system", null, "task", taskId, decision.state, decision.decidedBy, now, decision.decidedAt, task.snapshot); this.settleApproval(orgId, approvalId, approvalKind, RISK_LEVEL.HIGH, autoApprove); } diff --git a/src/main/services/task-approval.ts b/src/main/services/task-approval.ts index 2542436d..2b6cd517 100644 --- a/src/main/services/task-approval.ts +++ b/src/main/services/task-approval.ts @@ -2,3 +2,6 @@ export const TASK_SNAPSHOT_SQL = "json_object('title', t.title, 'description', t.description, 'assignee', t.assignee_agent_id, 'risk', t.risk, 'workspace', t.workspace_id, 'filesystem', t.requires_filesystem)"; export const TASK_APPROVAL_KINDS_SQL = "('task_risk', 'task_execution')"; + +/** What approving file access really allows: commands are not confined to the workspace. */ +export const HOST_ACCESS_NOTE = "Approving lets this task run commands on this computer with your user's file access: they can read and change files outside the workspace. Only their environment is cleaned of keys and your profile."; diff --git a/src/main/services/task-run-preparation.ts b/src/main/services/task-run-preparation.ts index 71d0e597..5e318f94 100644 --- a/src/main/services/task-run-preparation.ts +++ b/src/main/services/task-run-preparation.ts @@ -113,11 +113,14 @@ export function createIsolationContext( if (task.requires_filesystem === 1 && !workspace) throw new Error("Filesystem tasks require a workspace."); const serviceIdentityId = requireServiceIdentity(db, orgId, agent.id); const executionContextId = createId("ctx"); + // The mount mode guides the tools; the host does not enforce it, and commands run with the user's file access in an allowlisted environment. const workspaceMounts = workspace ? [{ ...workspaceAccess(task, workspace)[0], name: workspace.name, localPath: workspace.local_path, repoRef: workspace.repo_ref, + hostEnforcement: "advisory", + commandEnvironment: "allowlisted", }] : []; return { executionContextId, diff --git a/src/main/services/workspace-command.ts b/src/main/services/workspace-command.ts index 3f3094fb..8dfbbf0e 100644 --- a/src/main/services/workspace-command.ts +++ b/src/main/services/workspace-command.ts @@ -1,8 +1,14 @@ import { spawn, spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import { mkdirSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { currentRunSignal } from "./run-control"; const COMMAND_TIMEOUT_MS = 120_000; const KEEP_BYTES = 8_000; +/** What a command may inherit: enough for the OS and toolchains to run, nothing that carries the user's keys, tokens or profile. */ +const INHERITED_ENV = ["PATH", "PATHEXT", "SystemRoot", "SystemDrive", "windir", "ComSpec", "NUMBER_OF_PROCESSORS", "PROCESSOR_ARCHITECTURE", "OS", "LANG", "LC_ALL", "TERM"]; /** Keeps the first and last KEEP_BYTES of a stream and counts what it drops. */ export class BoundedOutput { @@ -47,7 +53,7 @@ export function runCommand(workspaceRoot: string, command: string, options: { ti const timeoutMs = options.timeoutMs ?? COMMAND_TIMEOUT_MS; return new Promise((resolvePromise) => { // Its own process group on POSIX, so the whole tree can be killed at once. - const child = spawn(command, { shell: true, cwd: workspaceRoot, windowsHide: true, detached: process.platform !== "win32" }); + const child = spawn(command, { shell: true, cwd: workspaceRoot, env: commandEnvironment(workspaceRoot), windowsHide: true, detached: process.platform !== "win32" }); const output = new BoundedOutput(); let settled = false; const runSignal = currentRunSignal(); @@ -79,6 +85,19 @@ export function runCommand(workspaceRoot: string, command: string, options: { ti }); } +/** A command's environment: the allowlisted variables plus a home and temp folder of the workspace's own, so ~ is not the user's profile. */ +export function commandEnvironment(workspaceRoot: string): NodeJS.ProcessEnv { + const home = join(tmpdir(), `agentcompany-home-${createHash("sha256").update(workspaceRoot).digest("hex").slice(0, 12)}`); + const temp = join(home, "tmp"); + mkdirSync(temp, { recursive: true }); + const env: NodeJS.ProcessEnv = { HOME: home, USERPROFILE: home, APPDATA: join(home, "AppData", "Roaming"), LOCALAPPDATA: join(home, "AppData", "Local"), TMP: temp, TEMP: temp, TMPDIR: temp }; + for (const name of INHERITED_ENV) { + const value = process.env[name]; + if (value !== undefined) env[name] = value; + } + return env; +} + /** The tool result for a finished command; no exit code means it was killed, which is never success. */ export function formatCommandResult(code: number | null, signal: NodeJS.Signals | null, text: string): string { const body = text.trim() || "(no output)"; diff --git a/tests/unit/domain-services.test.ts b/tests/unit/domain-services.test.ts index 3c17b6ef..52cda7dc 100644 --- a/tests/unit/domain-services.test.ts +++ b/tests/unit/domain-services.test.ts @@ -24,7 +24,7 @@ describe("domain services", () => { const connection = createConnection(); const rows = connection.db.prepare("select version from schema_migrations order by version asc").all() as Array<{ version: number }>; - expect(rows.map((row) => row.version)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87]); + expect(rows.map((row) => row.version)).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 80, 81, 82, 83, 84, 85, 86, 87, 88]); connection.close(); }); diff --git a/tests/unit/execution-modes.test.ts b/tests/unit/execution-modes.test.ts index c8d1ed1f..0369d2f1 100644 --- a/tests/unit/execution-modes.test.ts +++ b/tests/unit/execution-modes.test.ts @@ -11,7 +11,7 @@ import { ApprovalService } from "../../src/main/services/approval-service"; import { runJournalLines } from "../../src/main/services/run-journal"; import { DocumentService } from "../../src/main/services/document-service"; import { approvalDrifts } from "../../src/main/services/approval-drift"; -import { executionApprovalProblem, requireAssignedAgent } from "../../src/main/services/task-run-preparation"; +import { createIsolationContext, executionApprovalProblem, loadWorkspace, requireAssignedAgent } from "../../src/main/services/task-run-preparation"; import { TASK_PRIORITY } from "../../src/shared/domain"; import type { TaskExecutionTaskRow } from "../../src/main/services/task-prompts"; @@ -202,6 +202,22 @@ describe("execution modes", () => { } }); + test("file access is approved with its true reach, and the run's records call host isolation advisory", () => { + const h = harness("ai_cofounder"); + const db = h.connection.db; + const taskId = h.connection.transaction(() => h.deps.organization.createTask({ + orgId: h.orgId, title: "Fix the build", description: "Make CI green.", status: "backlog", assigneeAgentId: h.ceoId, risk: "low", + projectId: null, goalId: null, workspaceId: h.ctx.workspace!.id, priority: TASK_PRIORITY.NORMAL, requiresFilesystem: true, + })); + const approval = db.prepare("select details from approvals where source_entity_id = ?").get(taskId) as { details: string }; + expect(approval.details).toBe("Make CI green.\n\nApproving lets this task run commands on this computer with your user's file access: they can read and change files outside the workspace. Only their environment is cleaned of keys and your profile."); + const task = db.prepare("select * from tasks where id = ?").get(taskId) as unknown as Parameters[2]; + const agent = db.prepare("select * from agents where id = ?").get(h.ceoId) as unknown as Parameters[3]; + const provider = db.prepare("select * from providers where id = (select model_provider_id from agents where id = ?)").get(h.ceoId) as unknown as Parameters[4]; + const isolation = createIsolationContext(db, h.orgId, task, agent, provider, loadWorkspace(db, h.ctx.workspace!.id)); + expect(JSON.parse(isolation.workspaceMountJson)[0]).toMatchObject({ mode: "read_write", hostEnforcement: "advisory", commandEnvironment: "allowlisted" }); + }); + test("AI CEO mode ships deliverables without review", () => { const h = harness("ai_ceo"); const wrote = dispatchAgentTool(h.deps, h.ctx, call("write_deliverable", { path: "plan/business-plan.md", content: "# Plan" })); diff --git a/tests/unit/host-access-note-migration.test.ts b/tests/unit/host-access-note-migration.test.ts new file mode 100644 index 00000000..b9fdccce --- /dev/null +++ b/tests/unit/host-access-note-migration.test.ts @@ -0,0 +1,26 @@ +import { randomUUID } from "node:crypto"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { DatabaseSync } from "node:sqlite"; +import { describe, expect, test } from "vitest"; +import { hostAccessNoteSchema } from "../../src/main/database/migrations/088-host-access-note"; +import { HOST_ACCESS_NOTE } from "../../src/main/services/task-approval"; + +describe("host access note migration", () => { + test("pending file-access approvals from before the warning get it; others stay as they were", () => { + const db = new DatabaseSync(join(tmpdir(), `agentcompany-host-note-${randomUUID()}.db`)); + db.exec(` + create table schema_migrations(version integer primary key, name text not null, applied_at text not null); + create table tasks(id text primary key, requires_filesystem integer not null); + create table approvals(id text primary key, kind text not null, state text not null, details text not null, source_entity_type text, source_entity_id text); + insert into tasks values ('tsk_fs', 1), ('tsk_plain', 0); + insert into approvals values + ('app_pending', 'task_execution', 'pending', 'Make CI green.', 'task', 'tsk_fs'), + ('app_decided', 'task_execution', 'approved', 'Old.', 'task', 'tsk_fs'), + ('app_plain', 'task_risk', 'pending', 'Risky.', 'task', 'tsk_plain'); + `); + db.exec(hostAccessNoteSchema); + const details = Object.fromEntries((db.prepare("select id, details from approvals").all() as Array<{ id: string; details: string }>).map((row) => [row.id, row.details])); + expect(details).toEqual({ app_pending: `Make CI green.\n\n${HOST_ACCESS_NOTE}`, app_decided: "Old.", app_plain: "Risky." }); + }); +}); diff --git a/tests/unit/workspace-command.test.ts b/tests/unit/workspace-command.test.ts index 25e01cc1..5fc9b774 100644 --- a/tests/unit/workspace-command.test.ts +++ b/tests/unit/workspace-command.test.ts @@ -1,5 +1,5 @@ import { mkdtempSync, readFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, test, vi } from "vitest"; import { BoundedOutput, formatCommandResult, runCommand } from "../../src/main/services/workspace-command"; @@ -75,3 +75,20 @@ describe("command results", () => { expect(formatCommandResult(null, null, "")).toBe("ERROR: the command ended without an exit code; treat it as failed.\n(no output)"); }); }); + +describe("the environment a command runs in", () => { + test("a command sees no inherited secrets and a home of its own, while PATH still finds tools", async () => { + const root = mkdtempSync(join(tmpdir(), "ac-env-")); + process.env.AGENTCOMPANY_TEST_SECRET = "sk-should-not-leak"; + try { + const output = await runCommand(root, "node -e \"console.log(process.env.AGENTCOMPANY_TEST_SECRET ?? 'none'); console.log(require('os').homedir())\""); + const [code, secret, home] = output.split(/\r?\n/); + expect(code).toBe("exit code 0"); + expect(secret).toBe("none"); + expect(home).not.toBe(homedir()); + expect(home).toContain("agentcompany-home-"); + } finally { + delete process.env.AGENTCOMPANY_TEST_SECRET; + } + }); +});