diff --git a/.github/workflows/release-windows-check.yml b/.github/workflows/release-windows-check.yml index f6d426055c3..7bc5dcf54a2 100644 --- a/.github/workflows/release-windows-check.yml +++ b/.github/workflows/release-windows-check.yml @@ -83,6 +83,8 @@ on: - 'packages/runtime/src/child-fd-input.ts' - 'packages/runtime/src/child-process-lifecycle.ts' - 'packages/runtime/src/edit-replace.ts' + - 'packages/runtime/src/file-mutation-guard.ts' + - 'packages/runtime/src/file-mutation-guard-contract.ts' - 'packages/runtime/src/file-stable-write.ts' - 'packages/runtime/src/image-file.ts' - 'packages/runtime/src/process-tree-terminator.ts' diff --git a/.github/workflows/windows-recovery.yml b/.github/workflows/windows-recovery.yml index fa19e503c87..7bbb4465028 100644 --- a/.github/workflows/windows-recovery.yml +++ b/.github/workflows/windows-recovery.yml @@ -117,6 +117,7 @@ on: - 'packages/runtime/src/__tests__/runtime-resume-crash.test.ts' - 'packages/runtime/src/__tests__/shell-owner-death.test.ts' - 'packages/runtime/src/builtin-tools.ts' + - 'packages/runtime/src/file-mutation-guard.ts' - 'packages/runtime/src/file-stable-write.ts' - 'packages/runtime/src/filesystem-worker/client.ts' - 'packages/runtime/src/filesystem-worker/launch-spec.ts' diff --git a/docs/architecture/file-mutation-guards.md b/docs/architecture/file-mutation-guards.md new file mode 100644 index 00000000000..96ea1c46991 --- /dev/null +++ b/docs/architecture/file-mutation-guards.md @@ -0,0 +1,39 @@ + + +# Write/Edit execution-time content guards + +Write/Edit calls using the durable ToolRuntime preparation path can reject a target that changes after preparation and before the execution-time check. This is normal execution protection, not crash reconciliation. + +## Execution boundary + +The filesystem executor holds the existing per-target lock across read-only preparation, the ordinary T1 commit and execution. Input eligibility is computed from the private argument snapshot before preparation. Preparation requests carry the operation kind, target and eligibility flag, without replacement text. Ineligible inputs still pass the preparation permission checks but skip content observation. Preparation records only the canonical workspace/target, directory identities, target identity and bounded pre-state byte count/hash (or a missing target). It does not predict a successful result, compute an expected-after hash, or persist recovery evidence. Runtime owns only the prepared executor's execute/cancel lifecycle; T1/T2 retain their upstream schemas and recovery policy. + +Execution checks the same directory identities and the descriptor it will mutate before truncation. A changed identity, content length or hash rejects the write. Missing Write targets use exclusive creation. Edit matching and result/diff construction happen once, during execution. The guard remains private to the prepared executor, whose callers receive only execute/cancel. The private argument snapshot cannot be changed through the caller. The filesystem worker protocol rejects older versions rather than silently ignoring guards. + +## Coverage and limits + +- Existing readable regular UTF-8 files up to 1 MiB are eligible. Mutation input strings are bounded to 1 MiB each and must round-trip as UTF-8. There is no predicted-result/diff eligibility limit: results are produced only during execution and retain ordinary result handling. +- Unsupported targets or executors keep the ordinary execution path. Once a guard is established, failed validation never falls back to an unguarded write. +- The snapshot starts at preparation, not at the model's earlier Read. A file changed before preparation becomes the baseline. +- The process-local lock does not exclude Bash or external writers. The final check and write are not atomic across processes; there remains a check-to-write race. This is not a filesystem transaction or compare-and-swap guarantee. +- Both Write and Edit remain `never_auto_retry`. A crash with T1 but no T2 follows the current unknown-outcome contract; no recovery registry, reconciler or bundle writer is introduced. +- Worker-backed eligible calls still require a preparation subprocess and an execution subprocess. Local executors prepare in-process. Calls that did not previously enter durable preparation are not expanded into this path. + +Tests are in `packages/runtime/src/__tests__/file-mutation-guard.test.ts`, with worker permission/protocol coverage in `filesystem-worker-client.test.ts` and lifecycle coverage in `tool-runtime-settlement.test.ts`. diff --git a/packages/runtime/src/__tests__/builtin-tools-file-worker.test.ts b/packages/runtime/src/__tests__/builtin-tools-file-worker.test.ts index 420b06b75b9..af78c8733fb 100644 --- a/packages/runtime/src/__tests__/builtin-tools-file-worker.test.ts +++ b/packages/runtime/src/__tests__/builtin-tools-file-worker.test.ts @@ -186,6 +186,9 @@ describe('builtin file tools use the sandboxed worker', () => { execute: async (input) => { calls.push(input); switch (input.operation.kind) { + case 'prepare_write': + case 'prepare_edit': + return { kind: input.operation.kind }; case 'read': return { kind: 'read', diff --git a/packages/runtime/src/__tests__/file-mutation-guard.test.ts b/packages/runtime/src/__tests__/file-mutation-guard.test.ts new file mode 100644 index 00000000000..dda940e83ae --- /dev/null +++ b/packages/runtime/src/__tests__/file-mutation-guard.test.ts @@ -0,0 +1,574 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import { mkdtemp, readFile, realpath, rm, writeFile, rename, mkdir, lstat } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test, type TestContext } from 'node:test'; +import { createSqliteRuntimeStore } from '@maka/storage/sqlite-runtime-store'; +import type { RuntimeCommitSink, ToolPreparedCommit } from '../runtime-commit-sink.js'; +import { createTestToolRuntime } from './execution-boundary-test-helpers.js'; +import { buildBuiltinTools } from '../builtin-tools.js'; +import { createLocalWorkspaceExecutor, LocalWorkspaceExecutor } from '../workspace-executor.js'; +import { createBoundaryFilesystemExecutor } from '../filesystem-executor.js'; +import { + FILE_MUTATION_MAX_BYTES, + FileMutationGuardSchema, + type FileMutationGuard, +} from '../file-mutation-guard-contract.js'; +import { + prepareFileMutation, + fileMutationInputEligible, + executePreparedFileMutation, +} from '../file-mutation-guard.js'; +import { runFilesystemWorkerProcess } from '../filesystem-worker/process-runner.js'; +import { + FILESYSTEM_WORKER_PROTOCOL_VERSION, + parseFilesystemWorkerResponse, +} from '../filesystem-worker/protocol.js'; + +async function fixture(t: TestContext, kind: 'write' | 'edit' = 'write') { + const cwd = await realpath(await mkdtemp(join(tmpdir(), 'maka-write-evidence-'))); + const path = join(cwd, 'notes.txt'); + const dbPath = join(cwd, 'runtime.sqlite'); + const store = createSqliteRuntimeStore(dbPath); + t.after(async () => { + store.close(); + await rm(cwd, { recursive: true, force: true }); + }); + const executor = createLocalWorkspaceExecutor(); + const filesystem = createBoundaryFilesystemExecutor({ workspace: executor }); + const tool = buildBuiltinTools({ executor }).find( + (candidate) => candidate.name === (kind === 'write' ? 'Write' : 'Edit'), + )!; + const call = (signal?: AbortSignal) => ({ + operation: + kind === 'write' + ? { kind: 'write' as const, path, content: 'after\n' } + : { kind: 'edit' as const, path, oldString: 'before', newString: 'after' }, + cwd, + executionBoundary: { kind: 'bypass' as const, revision: 0 }, + abortSignal: signal, + }); + function execute(sink: RuntimeCommitSink = store, abortSignal = new AbortController().signal) { + let id = 0; + const runtime = createTestToolRuntime({ + sessionId: 'session-1', + runId: 'run-1', + invocationId: 'invocation-1', + header: { + id: 'session-1', + cwd, + workspaceRoot: cwd, + createdAt: 1, + name: 'test', + titleIsManual: false, + isFlagged: false, + labels: [], + isArchived: false, + status: 'active', + statusUpdatedAt: 1, + hasUnread: false, + backend: 'ai-sdk', + llmConnectionSlug: 'test', + connectionLocked: true, + model: 'test', + permissionMode: 'bypass', + schemaVersion: 1, + }, + connection: { slug: 'test', providerType: 'openai', defaultModel: 'test' }, + modelId: 'test', + newId: () => `event-${++id}`, + now: () => ++id, + getPermissionPauseTarget: () => null, + readExecutionBoundary: async () => ({ kind: 'bypass', revision: 0 }), + runtimeCommitSink: sink, + }); + return runtime.settleToolCall({ + tool, + turnId: 'turn-1', + toolCallId: 'provider-call-1', + input: + kind === 'write' + ? { path, content: 'after\n' } + : { path, old_string: 'before', new_string: 'after' }, + abortSignal, + eventSink: { push: () => {}, pushAndWaitUntilConsumed: async () => {} }, + }); + } + return { cwd, path, dbPath, store, filesystem, call, execute }; +} + +test('prepared writes reject inode replacement and a file appearing at a missing target', async (t) => { + const f = await fixture(t); + const missing = await f.filesystem.prepareMutation(f.call()); + await writeFile(f.path, 'external create'); + await assert.rejects(missing.execute(), /appeared|created/); + assert.equal(await readFile(f.path, 'utf8'), 'external create'); + const existing = await f.filesystem.prepareMutation(f.call()); + await rename(f.path, join(f.cwd, 'old.txt')); + await writeFile(f.path, 'replacement'); + await assert.rejects(existing.execute(), /changed/); + assert.equal(await readFile(f.path, 'utf8'), 'replacement'); + assert.equal(await readFile(join(f.cwd, 'old.txt'), 'utf8'), 'external create'); +}); + +test('cancellation and duplicate execution cannot leak a prepared lock or write twice', async (t) => { + const f = await fixture(t); + const controller = new AbortController(); + const first = await f.filesystem.prepareMutation(f.call(controller.signal)); + const waitingController = new AbortController(); + const waiting = f.filesystem.prepareMutation(f.call(waitingController.signal)); + waitingController.abort(new Error('cancel waiting')); + await assert.rejects(waiting, /cancel waiting/); + controller.abort(new Error('cancel prepared')); + await assert.rejects(first.execute(), /already settled/); + first.cancel(); + const next = await f.filesystem.prepareMutation(f.call()); + await next.execute(); + await writeFile(f.path, 'later edit'); + await assert.rejects(next.execute(), /already settled/); + next.cancel(); + assert.equal(await readFile(f.path, 'utf8'), 'later edit'); +}); + +test('a failed preparation worker releases the lock without dispatching a write', async (t) => { + const f = await fixture(t); + const operations: string[] = []; + const filesystem = createBoundaryFilesystemExecutor({ + workspace: createLocalWorkspaceExecutor(), + worker: { + execute: async ({ operation }) => { + operations.push(operation.kind); + throw new Error('worker died'); + }, + }, + }); + const call = f.call(); + // The embedding default routes to its configured worker. + const { executionBoundary: _boundary, ...workerCall } = call; + await assert.rejects(filesystem.prepareMutation(workerCall), /worker died/); + assert.deepEqual(operations, ['prepare_write']); + await assert.rejects(readFile(f.path), { code: 'ENOENT' }); + const next = await f.filesystem.prepareMutation(f.call()); + next.cancel(); +}); + +test('prepared and ordinary local Write retain identical result shapes', async (t) => { + const f = await fixture(t); + for (const previous of ['', 'before', 'new', 'unknown']) { + await writeFile(f.path, previous); + const prepared = await f.filesystem.prepareMutation(f.call()); + const result = await prepared.execute(); + await writeFile(f.path, previous); + assert.deepEqual(await f.filesystem.execute(f.call()), result); + } + const call = f.call(); + call.operation.path = join(f.cwd, 'image.png'); + await writeFile(call.operation.path, 'text with an image extension'); + const prepared = await f.filesystem.prepareMutation(call); + const result = await prepared.execute(); + assert.equal(result.kind === 'write' && result.diff, undefined); +}); + +test('prepared Edit keeps exact and fuzzy production results and rejects ambiguous matches before writing', async (t) => { + const f = await fixture(t, 'edit'); + for (const [source, oldString, newString, matchedVia] of [ + ['before\n', 'before', 'after', 'exact'], + [' first line\n second line\n', 'first line\nsecond line', 'new text', 'line-trimmed'], + ['const value = 1;\n', 'const value = 1;', 'const value = 2;', 'whitespace'], + ['line1\nline2', 'line1\\nline2', 'next', 'escape'], + ] as const) { + const call = { + ...f.call(), + operation: { + kind: 'edit' as const, + path: f.path, + oldString: oldString!, + newString: newString!, + }, + }; + await writeFile(f.path, source!); + const prepared = await f.filesystem.prepareMutation(call); + assert.equal(await readFile(f.path, 'utf8'), source); + const result = await prepared.execute(); + assert.equal(result.kind, 'edit'); + if (result.kind === 'edit') assert.equal(result.matchedVia, matchedVia); + const after = await readFile(f.path, 'utf8'); + await writeFile(f.path, source!); + assert.deepEqual(await f.filesystem.execute(call), result); + assert.equal(await readFile(f.path, 'utf8'), after); + } + await writeFile(f.path, 'before before'); + const ambiguous = await f.filesystem.prepareMutation(f.call()); + await assert.rejects(ambiguous.execute(), /not unique/); + assert.equal(await readFile(f.path, 'utf8'), 'before before'); +}); + +test('Edit cancellation, identity substitution and duplicate execution preserve outside content', async (t) => { + const f = await fixture(t, 'edit'); + await writeFile(f.path, 'before\n'); + const controller = new AbortController(); + const cancelled = await f.filesystem.prepareMutation(f.call(controller.signal)); + controller.abort(); + await assert.rejects(cancelled.execute(), /already settled/); + const replaced = await f.filesystem.prepareMutation(f.call()); + await rename(f.path, join(f.cwd, 'original')); + await writeFile(f.path, 'before\n'); + await assert.rejects(replaced.execute(), /changed/); + assert.equal(await readFile(f.path, 'utf8'), 'before\n'); + const once = await f.filesystem.prepareMutation(f.call()); + await once.execute(); + await writeFile(f.path, 'outside'); + await assert.rejects(once.execute(), /already settled/); + assert.equal(await readFile(f.path, 'utf8'), 'outside'); +}); + +test('input eligibility preserves UTF-8 byte limits for both mutation kinds', () => { + const boundary = 'é'.repeat(FILE_MUTATION_MAX_BYTES / 2); + for (const [content, eligible] of [ + [boundary, true], + [boundary + 'x', false], + ['\ud800', false], + ] as const) { + assert.equal(fileMutationInputEligible({ kind: 'write', content }), eligible); + assert.equal( + fileMutationInputEligible({ kind: 'edit', oldString: content, newString: 'after' }), + eligible, + ); + assert.equal( + fileMutationInputEligible({ kind: 'edit', oldString: 'before', newString: content }), + eligible, + ); + } +}); + +for (const kind of ['write', 'edit'] as const) { + test(`${kind}: ineligible input keeps ordinary execution after content changes`, async (t) => { + const f = await fixture(t, kind); + await writeFile(f.path, 'before\n'); + const content = 'x'.repeat(FILE_MUTATION_MAX_BYTES + 1); + const call = { + ...f.call(), + operation: + kind === 'write' + ? { kind, path: f.path, content } + : { kind, path: f.path, oldString: 'before', newString: content }, + }; + const prepared = await f.filesystem.prepareMutation(call); + await writeFile(f.path, 'before external\n'); + const result = await prepared.execute(); + assert.equal( + await readFile(f.path, 'utf8'), + kind === 'write' ? content : content + ' external\n', + ); + await writeFile(f.path, 'before external\n'); + assert.deepEqual(await f.filesystem.execute(call), result); + }); +} + +test('invalid UTF-8, oversized content, and unsupported executor guards stay conservative', async (t) => { + const f = await fixture(t); + await writeFile(f.path, Buffer.from([0xff, 0xfe])); + assert.equal( + await prepareFileMutation(f.cwd, f.path, { kind: 'write', eligible: true }), + undefined, + ); + assert.equal( + await prepareFileMutation(f.cwd, f.path, { + kind: 'write', + eligible: fileMutationInputEligible({ + kind: 'write', + content: 'a'.repeat(FILE_MUTATION_MAX_BYTES + 1), + }), + }), + undefined, + ); + const prepared = await f.filesystem.prepareMutation(f.call()); + await prepared.execute(); + assert.equal(await readFile(f.path, 'utf8'), 'after\n'); + const unsupported = createBoundaryFilesystemExecutor({ + workspace: Object.assign(createLocalWorkspaceExecutor(), { + prepareMutation: undefined, + executePreparedMutation: undefined, + }), + }); + const fallback = await unsupported.prepareMutation(f.call()); + await fallback.execute(); +}); + +for (const kind of ['write', 'edit'] as const) { + test(`${kind}: external same-inode edit after T1 is rejected without truncation`, async (t) => { + const f = await fixture(t, kind); + await writeFile(f.path, 'before\n'); + const before = await lstat(f.path, { bigint: true }); + const result = await f.execute({ + commitToolPrepared: async (input) => { + const committed = await f.store.commitToolPrepared(input); + await writeFile(f.path, 'outside\n'); + const after = await lstat(f.path, { bigint: true }); + assert.equal(after.ino, before.ino); + return committed; + }, + commitToolOutcome: (input) => f.store.commitToolOutcome(input), + }); + assert.match(JSON.stringify(result), /prepared file mutation target changed/); + assert.equal(await readFile(f.path, 'utf8'), 'outside\n'); + const next = await f.filesystem.prepareMutation(f.call()); + next.cancel(); + }); + + test(`${kind}: successful dispatch persists ordinary T1 only`, async (t) => { + const f = await fixture(t, kind); + await writeFile(f.path, 'before\n'); + let dispatched: ToolPreparedCommit | undefined; + await f.execute({ + commitToolPrepared: async (input) => { + dispatched = input; + assert.equal(await readFile(f.path, 'utf8'), 'before\n'); + return f.store.commitToolPrepared(input); + }, + commitToolOutcome: (input) => f.store.commitToolOutcome(input), + }); + assert.ok(dispatched); + const dispatch = dispatched.dispatchRuntimeEvent.actions?.toolDispatch; + assert.ok(dispatch); + assert.deepEqual( + Object.keys(dispatch).sort(), + [ + 'protocol', + 'resultProjectionVersion', + 'operationId', + 'providerToolCallId', + 'toolName', + 'canonicalArgsHash', + 'recoveryMode', + ].sort(), + ); + assert.equal(dispatch.recoveryMode, 'never_auto_retry'); + assert.equal(await readFile(f.path, 'utf8'), 'after\n'); + }); + + test(`${kind}: T1 failure leaves the target intact and releases its lock`, async (t) => { + const f = await fixture(t, kind); + await writeFile(f.path, 'before\n'); + await assert.rejects( + f.execute({ + commitToolPrepared: async () => { + throw new Error('T1 failed'); + }, + commitToolOutcome: (input) => f.store.commitToolOutcome(input), + }), + /T1 failed/, + ); + assert.equal(await readFile(f.path, 'utf8'), 'before\n'); + const next = await f.filesystem.prepareMutation(f.call()); + await next.execute(); + assert.equal(await readFile(f.path, 'utf8'), 'after\n'); + }); +} + +for (const kind of ['write', 'edit'] as const) { + test(`${kind}: prepared execution hides its guard and isolates caller/backend objects`, async (t) => { + const f = await fixture(t, kind); + await writeFile(f.path, 'before\n'); + const workspace = new LocalWorkspaceExecutor(); + const prepare = workspace.prepareMutation.bind(workspace); + let backendGuard: FileMutationGuard | undefined; + const filesystem = createBoundaryFilesystemExecutor({ + workspace: Object.assign(workspace, { + prepareMutation: async (...args: Parameters) => { + backendGuard = await prepare(...args); + return backendGuard; + }, + }), + }); + const call = f.call(); + const prepared = await filesystem.prepareMutation(call); + assert.deepEqual(Object.keys(prepared).sort(), ['cancel', 'execute']); + assert.ok(backendGuard); + backendGuard.target.path = join(f.cwd, 'wrong.txt'); + call.operation.path = join(f.cwd, 'wrong.txt'); + if (call.operation.kind === 'write') call.operation.content = 'wrong'; + else call.operation.newString = 'wrong'; + await prepared.execute(); + assert.equal(await readFile(f.path, 'utf8'), 'after\n'); + await assert.rejects(readFile(join(f.cwd, 'wrong.txt')), { code: 'ENOENT' }); + }); +} + +test('parent replacement is rejected without touching the replacement file', async (t) => { + const f = await fixture(t); + const parent = join(f.cwd, 'parent'); + await mkdir(parent); + const path = join(parent, 'notes.txt'); + await writeFile(path, 'before'); + const prepared = await f.filesystem.prepareMutation({ + ...f.call(), + operation: { kind: 'write', path, content: 'after' }, + }); + await rename(parent, join(f.cwd, 'old-parent')); + await mkdir(parent); + await writeFile(path, 'outside'); + await assert.rejects(prepared.execute(), /changed/); + assert.equal(await readFile(path, 'utf8'), 'outside'); + assert.equal(await readFile(join(f.cwd, 'old-parent', 'notes.txt'), 'utf8'), 'before'); +}); + +test('guard shape rejects legacy evidence and cross-tool reuse before mutation', async (t) => { + const f = await fixture(t); + const guard = await prepareFileMutation(f.cwd, f.path, { kind: 'write', eligible: true }); + assert.ok(guard); + assert.equal( + FileMutationGuardSchema.safeParse({ ...guard, after: { bytes: 5, digest: 'unused' } }).success, + false, + ); + await assert.rejects( + executePreparedFileMutation( + f.cwd, + f.path, + { kind: 'edit', oldString: 'before', newString: 'after' }, + guard, + ), + /changed/, + ); + await assert.rejects(readFile(f.path), { code: 'ENOENT' }); +}); + +for (const kind of ['write', 'edit'] as const) { + test(`${kind}: real worker checks transient guards and rejects prior wire versions`, async (t) => { + const f = await fixture(t, kind); + await writeFile(f.path, 'before\n'); + const metadata = await lstat(f.path, { bigint: true }); + const operation = + kind === 'write' + ? { kind, path: f.path, cwd: f.cwd, content: 'after\n' } + : { kind, path: f.path, cwd: f.cwd, oldString: 'before', newString: 'after' }; + const request = { + version: FILESYSTEM_WORKER_PROTOCOL_VERSION, + requestId: 'guard-worker', + operation, + operationBoundary: { + filesystem: { entries: [{ path: f.path, access: 'write', scope: 'exact' }] }, + }, + expectedTarget: { + enforcementPath: f.path, + access: 'write', + scope: 'exact', + targetType: 'file', + identity: { dev: String(metadata.dev), ino: String(metadata.ino) }, + }, + }; + const run = async (body: unknown) => { + const result = await runFilesystemWorkerProcess({ + argv: [ + process.execPath, + fileURLToPath(new URL('../filesystem-worker/worker-entry.js', import.meta.url)), + ], + cwd: f.cwd, + env: process.env, + stdin: JSON.stringify(body), + }); + assert.equal(result.exitCode, 0, result.stderrTail); + return parseFilesystemWorkerResponse(JSON.parse(result.stdout)); + }; + const preparationOperation = { + kind: kind === 'write' ? 'prepare_write' : 'prepare_edit', + path: f.path, + cwd: f.cwd, + eligible: true, + }; + const declined = await run({ + ...request, + operation: { ...preparationOperation, eligible: false }, + }); + assert.ok( + declined.ok && + (declined.result.kind === 'prepare_write' || declined.result.kind === 'prepare_edit'), + ); + assert.equal(declined.result.guard, undefined); + const deniedCwd = join(f.cwd, 'restricted'); + await mkdir(deniedCwd); + const denied = await run({ + ...request, + operation: { ...preparationOperation, cwd: deniedCwd, eligible: false }, + operationBoundary: { + filesystem: { entries: [{ path: f.path, access: 'read', scope: 'exact' }] }, + }, + }); + assert.ok(!denied.ok && denied.error.code === 'path_denied'); + const payloadRejected = await run({ + ...request, + operation: { ...operation, ...preparationOperation }, + }); + assert.ok(!payloadRejected.ok && payloadRejected.error.code === 'invalid_request'); + assert.equal(await readFile(f.path, 'utf8'), 'before\n'); + const prepared = await run({ ...request, operation: preparationOperation }); + assert.ok( + prepared.ok && + (prepared.result.kind === 'prepare_write' || prepared.result.kind === 'prepare_edit') && + prepared.result.guard, + ); + const guard = prepared.result.guard; + assert.equal(await readFile(f.path, 'utf8'), 'before\n'); + assert.equal('result' in guard, false); + assert.equal('after' in guard, false); + for (const version of [12, 13, 14]) { + const legacy = await run({ ...request, version, operation: { ...operation, guard } }); + assert.equal(legacy.ok, false); + } + assert.equal(await readFile(f.path, 'utf8'), 'before\n'); + await writeFile(f.path, 'outside\n'); + const rejected = await run({ ...request, operation: { ...operation, guard } }); + assert.ok(!rejected.ok && rejected.error.code === 'path_changed'); + assert.equal(await readFile(f.path, 'utf8'), 'outside\n'); + await writeFile(f.path, 'before\n'); + const executed = await run({ ...request, operation: { ...operation, guard } }); + assert.ok(executed.ok); + assert.equal(await readFile(f.path, 'utf8'), 'after\n'); + if (kind === 'edit') { + for (const [name, content, message] of [ + ['missing match', 'unrelated text', /not found/], + ['ambiguous match', 'before before', /not unique/], + ] as const) { + await t.test(`preserves ${name} as an actionable edit conflict`, async () => { + await writeFile(f.path, content); + const preparation = await run({ + ...request, + operation: { kind: 'prepare_edit', path: f.path, cwd: f.cwd, eligible: true }, + }); + assert.ok(preparation.ok && preparation.result.kind === 'prepare_edit'); + assert.ok(preparation.result.guard); + const ordinary = await run(request); + const guarded = await run({ + ...request, + operation: { ...operation, guard: preparation.result.guard }, + }); + assert.ok(!ordinary.ok && ordinary.error.code === 'edit_conflict'); + assert.match(ordinary.error.message, message); + assert.ok(!guarded.ok); + assert.deepEqual(guarded.error, ordinary.error); + assert.equal(await readFile(f.path, 'utf8'), content); + }); + } + } + }); +} diff --git a/packages/runtime/src/__tests__/filesystem-worker-client.test.ts b/packages/runtime/src/__tests__/filesystem-worker-client.test.ts index 82471edb10f..fdeb4740ab2 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-client.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-client.test.ts @@ -25,6 +25,7 @@ import { mkdir, readFile, realpath, + rename, rm, symlink, writeFile, @@ -35,6 +36,8 @@ import { afterEach, describe, test } from 'node:test'; import { createManagedExecutionBoundary } from '@maka/core/sandbox-boundary'; import { createWorkspaceWritePermissionProfile } from '@maka/core/permission-profile'; import { MAX_READ_IMAGE_BYTES } from '@maka/core/attachments'; +import { prepareFileMutation } from '../file-mutation-guard.js'; +import { executeFilesystemWorkerRequest } from '../filesystem-worker/operations.js'; import { canReadPath, canWritePath, @@ -79,6 +82,244 @@ test('Read image payloads fit within the filesystem worker response limit', () = }); describe('filesystem worker client permission snapshots', () => { + test('Write preparation uses the same write grant and missing-target identity as execution', async () => { + const workspace = await temporaryDirectory('maka-worker-prepare-grant-'); + const { client, requests } = fakeClient(); + for (const kind of ['prepare_write', 'write'] as const) { + await client.execute({ + operation: + kind === 'prepare_write' + ? { kind, path: 'notes.txt', eligible: true } + : { kind, path: 'notes.txt', content: 'after' }, + cwd: workspace, + mode: 'ask', + expectedIdentity: 'missing', + }); + } + assert.equal(requests[0]!.operation.kind, 'prepare_write'); + assert.deepEqual(Object.keys(requests[0]!.operation).sort(), [ + 'cwd', + 'eligible', + 'kind', + 'path', + ]); + assert.deepEqual(requests[0]!.operationBoundary, requests[1]!.operationBoundary); + assert.deepEqual(requests[0]!.expectedTarget, requests[1]!.expectedTarget); + assert.equal(requests[0]!.expectedTarget.access, 'write'); + // Native namespaces must recheck their actual directories inside the worker. + assert.equal(requests[0]!.hostDirectories, undefined); + }); + + test('Edit preparation uses the execution write grant and existing identity', async () => { + const cwd = await temporaryDirectory('maka-edit-prepare-grant-'); + const path = join(cwd, 'notes.txt'); + await writeFile(path, 'before'); + const stat = await lstat(path, { bigint: true }); + const { client, requests } = fakeClient(); + for (const kind of ['prepare_edit', 'edit'] as const) { + await client.execute({ + operation: + kind === 'prepare_edit' + ? { kind, path, eligible: true } + : { kind, path, oldString: 'before', newString: 'after' }, + cwd, + mode: 'ask', + expectedIdentity: { dev: String(stat.dev), ino: String(stat.ino) }, + }); + } + assert.deepEqual(Object.keys(requests[0]!.operation).sort(), [ + 'cwd', + 'eligible', + 'kind', + 'path', + ]); + assert.deepEqual(requests[0]!.operationBoundary, requests[1]!.operationBoundary); + assert.deepEqual(requests[0]!.expectedTarget, requests[1]!.expectedTarget); + assert.equal(requests[0]!.expectedTarget.access, 'write'); + assert.equal(requests[0]!.hostDirectories, undefined); + }); + + for (const eligible of [true, false]) { + test(`a read-only profile cannot prepare an Edit (eligible=${eligible})`, async () => { + const cwd = await temporaryDirectory('maka-edit-prepare-denied-'); + const path = join(cwd, 'notes.txt'); + await writeFile(path, 'before'); + const { client, requests } = fakeClient(); + await assert.rejects( + client.execute({ + operation: { kind: 'prepare_edit', path, eligible }, + cwd, + permissionProfile: createReadOnlyPermissionProfile(), + expectedIdentity: 'unchecked', + }), + (error: unknown) => + error instanceof FilesystemWorkerClientError && error.reason === 'path_denied', + ); + assert.deepEqual(requests, []); + }); + } + + test('a prepared Write rejects substituted host directory identity before worker launch', async () => { + const workspace = await temporaryDirectory('maka-worker-prepare-host-'); + const path = join(workspace, 'notes.txt'); + const guard = await prepareFileMutation(workspace, path, { kind: 'write', eligible: true }); + assert.ok(guard); + guard.workspace.identity.ino += '1'; + const { client, requests } = fakeClient(); + await assert.rejects( + client.execute({ + operation: { kind: 'write', path, content: 'after', guard }, + cwd: workspace, + mode: 'ask', + expectedIdentity: 'missing', + }), + (error: unknown) => + error instanceof FilesystemWorkerClientError && error.reason === 'path_changed', + ); + assert.deepEqual(requests, []); + }); + + for (const swapAt of ['launch', 'worker', 'after-write'] as const) { + test(`a prepared Write rejects a parent replacement during ${swapAt}`, async () => { + const cwd = await temporaryDirectory('maka-worker-parent-race-'); + const parent = join(cwd, 'parent'); + const previous = join(cwd, 'previous'); + const path = join(parent, 'target.txt'); + await mkdir(parent); + const guard = await prepareFileMutation(cwd, path, { kind: 'write', eligible: true }); + assert.ok(guard); + const swap = async () => { + await rename(parent, previous); + await mkdir(parent); + }; + let dispatched = false; + const client = new FilesystemWorkerClient({ + platform: 'darwin', + sandboxManager: new SandboxManager([new MacosSeatbeltBackend()]), + getLaunchSpec: async () => { + if (swapAt === 'launch') await swap(); + return { + ok: true, + spec: { + program: process.execPath, + args: [], + env: {}, + runtimeReadableRoots: [], + executableRoots: [process.execPath], + }, + }; + }, + runProcess: async (input) => { + dispatched = true; + const request = FilesystemWorkerRequestSchema.parse(JSON.parse(input.stdin)); + assert.equal(request.hostDirectories, undefined); + if (swapAt === 'worker') await swap(); + const response = await executeFilesystemWorkerRequest(request); + if (swapAt === 'after-write') { + assert.ok(response.ok); + await swap(); + } + return { + exitCode: 0, + stdout: JSON.stringify(response), + stderrTail: '', + timedOut: false, + aborted: false, + responseOverflow: false, + dispatched: true, + }; + }, + }); + await assert.rejects( + client.execute({ + operation: { kind: 'write', path, content: 'after', guard }, + cwd, + mode: 'ask', + expectedIdentity: 'missing', + }), + (error: unknown) => + error instanceof FilesystemWorkerClientError && + error.reason === (swapAt === 'after-write' ? 'outcome_unknown' : 'path_changed'), + ); + assert.equal(dispatched, swapAt !== 'launch'); + await assert.rejects(readFile(path), { code: 'ENOENT' }); + if (swapAt === 'after-write') { + assert.equal(await readFile(join(previous, 'target.txt'), 'utf8'), 'after'); + } else { + await assert.rejects(readFile(join(previous, 'target.txt')), { code: 'ENOENT' }); + } + }); + } + + test('a prepared Write rejects a replacement Linux mount source', { + skip: process.platform !== 'linux', + }, async () => { + const cwd = await temporaryDirectory('maka-worker-mount-race-'); + const parent = join(cwd, 'parent'); + await mkdir(parent); + const path = join(parent, 'target.txt'); + const guard = await prepareFileMutation(cwd, path, { kind: 'write', eligible: true }); + assert.ok(guard); + let transformed = false; + const manager = new SandboxManager([]); + const transform = manager.transform.bind(manager); + const client = new FilesystemWorkerClient({ + platform: 'linux', + sandboxManager: Object.assign(manager, { + transform(request: SandboxTransformRequest): SandboxTransformResult { + transformed = true; + return transform(request); + }, + }), + getLaunchSpec: async () => { + await rename(parent, join(cwd, 'previous')); + await mkdir(parent); + return { + ok: true, + spec: { + program: process.execPath, + args: [], + env: {}, + runtimeReadableRoots: [], + executableRoots: [process.execPath], + }, + }; + }, + runProcess: async () => { + throw new Error('must not dispatch'); + }, + }); + await assert.rejects( + client.execute({ + operation: { kind: 'write', path, content: 'after', guard }, + cwd, + mode: 'ask', + expectedIdentity: 'missing', + }), + (error: unknown) => + error instanceof FilesystemWorkerClientError && + error.reason === 'path_changed' && + error.message.includes('before pinning'), + ); + assert.equal(transformed, false); + await assert.rejects(readFile(path), { code: 'ENOENT' }); + }); + + test('a read-only profile cannot prepare a Write', async () => { + const workspace = await temporaryDirectory('maka-worker-prepare-denied-'); + const { client, requests } = fakeClient(); + await assert.rejects( + client.execute({ + operation: { kind: 'prepare_write', path: 'notes.txt', eligible: true }, + cwd: workspace, + permissionProfile: createReadOnlyPermissionProfile(), + expectedIdentity: 'missing', + }), + (error: unknown) => + error instanceof FilesystemWorkerClientError && error.reason === 'path_denied', + ); + assert.deepEqual(requests, []); + }); test('authorizes delete against the symlink entry', async () => { const workspace = await temporaryDirectory('maka-worker-client-delete-link-'); const target = join(workspace, 'target.txt'); @@ -467,6 +708,33 @@ describe('filesystem worker operation-scoped Seatbelt profile', () => { ); }); + test('rejects the upstream v12 worker before accepting a response', async () => { + const workspace = await temporaryDirectory('maka-worker-client-old-protocol-'); + const target = join(workspace, 'file.ts'); + await writeFile(target, 'const value = 1;', 'utf8'); + const { client, requests } = fakeClient({ responseVersion: 12 }); + + await assert.rejects( + client.execute({ + operation: grepOperation(target), + cwd: workspace, + mode: 'ask', + expectedIdentity: 'unchecked', + }), + (error: unknown) => { + assert.ok(error instanceof FilesystemWorkerClientError); + assert.equal(error.reason, 'invalid_response'); + assert.equal(error.stage, 'protocol'); + return true; + }, + ); + assert.equal(requests.length, 1); + assert.equal( + FilesystemWorkerRequestSchema.safeParse({ ...requests[0], version: 12 }).success, + false, + ); + }); + test('preserves an explicit sandbox denial from the worker protocol', async () => { const workspace = await temporaryDirectory('maka-worker-client-sandbox-denial-'); const target = join(workspace, 'file.ts'); @@ -633,6 +901,7 @@ describe('filesystem worker Linux path context', () => { function fakeClient( options: { operationErrorCode?: FilesystemWorkerErrorCode; + responseVersion?: number; platform?: SandboxPlatform; beforeLaunchSpecReturn?: () => void; } = {}, @@ -685,7 +954,7 @@ function fakeClient( stdout: JSON.stringify( options.operationErrorCode ? { - version: FILESYSTEM_WORKER_PROTOCOL_VERSION, + version: options.responseVersion ?? FILESYSTEM_WORKER_PROTOCOL_VERSION, requestId: request.requestId, ok: false, error: { @@ -694,7 +963,7 @@ function fakeClient( }, } : { - version: FILESYSTEM_WORKER_PROTOCOL_VERSION, + version: options.responseVersion ?? FILESYSTEM_WORKER_PROTOCOL_VERSION, requestId: request.requestId, ok: true, result: fakeResult(request), @@ -713,6 +982,19 @@ function fakeClient( function fakeResult(request: FilesystemWorkerRequest): FilesystemWorkerResult { switch (request.operation.kind) { + case 'prepare_write': + case 'prepare_edit': + return { kind: request.operation.kind }; + case 'edit': + return { + kind: 'edit', + ok: true, + path: request.operation.path, + replacements: 1, + matchedVia: 'exact', + startLine: 1, + endLine: 1, + }; case 'read': return request.operation.path.endsWith('.png') ? { kind: 'read_image', base64: 'iVBORw==', mimeType: 'image/png' } diff --git a/packages/runtime/src/__tests__/tool-runtime-settlement.test.ts b/packages/runtime/src/__tests__/tool-runtime-settlement.test.ts index 0633090b8ca..9ff3d69e8b9 100644 --- a/packages/runtime/src/__tests__/tool-runtime-settlement.test.ts +++ b/packages/runtime/src/__tests__/tool-runtime-settlement.test.ts @@ -188,6 +188,72 @@ describe('ToolRuntime settlement', () => { assert.deepEqual(settlement.result, { ok: true }); }); + for (const failureAt of ['publication', 'execution'] as const) { + it(`keeps prepared cleanup ownership when ${failureAt} fails after T1`, async () => { + const order: string[] = []; + const clientTool: MakaTool = { + name: 'client_browser', + description: 'client browser', + parameters: {}, + hostAdmission: 'client_capability', + prepareExecution: async () => { + order.push('prepare'); + return { + execute: () => { + order.push('execute'); + try { + throw new Error('execution failed'); + } finally { + order.push('release'); + } + }, + cancel: () => { + order.push('cancel'); + }, + }; + }, + impl: () => assert.fail('Prepared work must not use the direct implementation'), + }; + const runtime = makeRuntime({ + readExecutionBoundary: async () => createBypassExecutionBoundary(0), + runId: 'run-1', + invocationId: 'invocation-1', + runtimeCommitSink: { + commitToolPrepared: async () => { + order.push('T1'); + return { created: true, runtimeEventSeq: 1 }; + }, + commitToolOutcome: async () => ({ created: true, runtimeEventSeq: 2 }), + }, + }); + const settlement = await runtime.settleToolCall({ + tool: clientTool, + turnId: 'turn-1', + stepId: 'step-1', + toolCallId: `call-${failureAt}-failure`, + input: {}, + abortSignal: new AbortController().signal, + eventSink: { + push: (event) => { + if (failureAt === 'publication' && event.type === 'tool_start') + throw new Error('publication failed'); + }, + pushAndWaitUntilConsumed: async () => undefined, + }, + }); + assert.deepEqual( + order, + failureAt === 'publication' + ? ['prepare', 'T1', 'cancel'] + : ['prepare', 'T1', 'execute', 'release'], + ); + assert.match( + String((settlement.result as { error?: unknown }).error), + new RegExp(`${failureAt} failed`, 'u'), + ); + }); + } + it('cancels prepared Client Capability work when T1 fails', async () => { const order: string[] = []; const clientTool: MakaTool = { diff --git a/packages/runtime/src/builtin-tools.ts b/packages/runtime/src/builtin-tools.ts index df4d176468c..9baf45dfe1e 100644 --- a/packages/runtime/src/builtin-tools.ts +++ b/packages/runtime/src/builtin-tools.ts @@ -81,7 +81,7 @@ import { // tool-runtime.ts is the single source of truth for the tool shape; this // re-export only keeps back-compat for callers that imported from // builtin-tools directly. -import type { MakaTool, MakaToolContext } from './tool-runtime.js'; +import type { MakaTool, MakaToolContext, MakaToolPreparationContext } from './tool-runtime.js'; export type { MakaTool, MakaToolContext }; import { profileRequiresSandbox, type SandboxManager } from './sandbox/sandbox-manager.js'; import { SandboxCommandError } from './sandbox/errors.js'; @@ -430,16 +430,22 @@ export function buildBuiltinTools(options: BuildBuiltinToolsOptions = {}): MakaT content: z.string().describe('The complete text to write to the file.'), }), executionFacts, + prepareExecution: async ({ path, content }, ctx) => { + const prepared = await filesystem.prepareMutation({ + operation: { kind: 'write', path, content }, + ...filesystemCall(ctx), + }); + return { + cancel: () => prepared.cancel(), + execute: async () => writeToolOutput(await prepared.execute()), + }; + }, impl: async ({ path, content }, ctx) => { const result = await filesystem.execute({ operation: { kind: 'write', path, content }, ...filesystemCall(ctx), }); - if (result.kind !== 'write') - throw internalFilesystemWriteFailure('Write', 'the file was written'); - if (result.diff !== undefined) - return { kind: 'file_diff' as const, paths: [result.path], diff: result.diff }; - return { kind: 'file_write' as const, path: result.path, bytes: result.bytes }; + return writeToolOutput(result); }, toModelOutput: ({ output }) => fileWriteToolResultToModelOutput('Write', output), }, @@ -458,6 +464,16 @@ export function buildBuiltinTools(options: BuildBuiltinToolsOptions = {}): MakaT new_string: z.string(), }), executionFacts, + prepareExecution: async ({ path, old_string, new_string }, ctx) => { + const prepared = await filesystem.prepareMutation({ + operation: { kind: 'edit', path, oldString: old_string, newString: new_string }, + ...filesystemCall(ctx), + }); + return { + cancel: () => prepared.cancel(), + execute: async () => editToolOutput(await prepared.execute()), + }; + }, impl: async ({ path, old_string, new_string }, ctx) => { const result = await filesystem.execute({ operation: { @@ -468,22 +484,7 @@ export function buildBuiltinTools(options: BuildBuiltinToolsOptions = {}): MakaT }, ...filesystemCall(ctx), }); - if (result.kind !== 'edit') - throw internalFilesystemWriteFailure( - 'Edit', - 'the edit was applied', - 'a different old_string will not help', - ); - if (result.diff !== undefined) - return { kind: 'file_diff' as const, paths: [result.path], diff: result.diff }; - return { - ok: result.ok, - path: result.path, - replacements: result.replacements, - matchedVia: result.matchedVia, - startLine: result.startLine, - endLine: result.endLine, - }; + return editToolOutput(result); }, toModelOutput: ({ output }) => fileWriteToolResultToModelOutput('Edit', output), }, @@ -611,7 +612,7 @@ export function buildBuiltinTools(options: BuildBuiltinToolsOptions = {}): MakaT /** The per-call context every file tool hands to the filesystem authority. */ function filesystemCall( - ctx: MakaToolContext, + ctx: MakaToolPreparationContext, ): Pick { return { cwd: ctx.cwd, @@ -621,6 +622,37 @@ function filesystemCall( }; } +function writeToolOutput( + result: Awaited['execute']>>, +) { + if (result.kind !== 'write') + throw internalFilesystemWriteFailure('Write', 'the file was written'); + if (result.diff !== undefined) + return { kind: 'file_diff' as const, paths: [result.path], diff: result.diff }; + return { kind: 'file_write' as const, path: result.path, bytes: result.bytes }; +} + +function editToolOutput( + result: Awaited['execute']>>, +) { + if (result.kind !== 'edit') + throw internalFilesystemWriteFailure( + 'Edit', + 'the edit was applied', + 'a different old_string will not help', + ); + if (result.diff !== undefined) + return { kind: 'file_diff' as const, paths: [result.path], diff: result.diff }; + return { + ok: result.ok, + path: result.path, + replacements: result.replacements, + matchedVia: result.matchedVia, + startLine: result.startLine, + endLine: result.endLine, + }; +} + interface ExecutorBashSandboxOptions { permissionProfile?: PermissionProfile; sandboxManager?: SandboxManager; diff --git a/packages/runtime/src/file-mutation-guard-contract.ts b/packages/runtime/src/file-mutation-guard-contract.ts new file mode 100644 index 00000000000..9c0b98114e9 --- /dev/null +++ b/packages/runtime/src/file-mutation-guard-contract.ts @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { z } from 'zod'; + +/** Transient execution preconditions, never persisted as recovery authority. */ +export const FILE_MUTATION_MAX_BYTES = 1_048_576; +const path = z + .string() + .min(1) + .max(4096) + .refine((s) => !s.includes('\0')); +const identity = z + .object({ dev: z.string().regex(/^\d+$/), ino: z.string().regex(/^\d+$/) }) + .strict(); +export const FileMutationGuardSchema = z + .object({ + kind: z.enum(['write', 'edit']), + workspace: z.object({ path, identity }).strict(), + target: z.object({ path, parentIdentity: identity }).strict(), + before: z.discriminatedUnion('kind', [ + z.object({ kind: z.literal('missing') }).strict(), + z + .object({ + kind: z.literal('file'), + identity, + bytes: z.number().int().min(0).max(FILE_MUTATION_MAX_BYTES), + digest: z.string().regex(/^sha256:[0-9a-f]{64}$/), + }) + .strict(), + ]), + }) + .strict() + .refine((value) => value.kind !== 'edit' || value.before.kind === 'file'); +export type FileMutationGuard = z.infer; +export type FileIdentity = z.infer; +export interface FileMutationDirectories { + workspace: FileIdentity; + parent: FileIdentity; +} +/** Eligibility is computed from the owning executor's private argument snapshot. */ +export interface FileMutationPreparation { + kind: 'write' | 'edit'; + eligible: boolean; +} +export type FileMutation = + | { kind: 'write'; content: string } + | { kind: 'edit'; oldString: string; newString: string }; +export interface FileWriteResult { + kind: 'write'; + ok: true; + path: string; + bytes: number; + diff?: string; +} +export interface FileEditResult { + kind: 'edit'; + ok: true; + path: string; + replacements: 1; + matchedVia: 'exact' | 'line-trimmed' | 'whitespace' | 'escape'; + startLine: number; + endLine: number; + diff?: string; +} diff --git a/packages/runtime/src/file-mutation-guard.ts b/packages/runtime/src/file-mutation-guard.ts new file mode 100644 index 00000000000..54e6602f8b4 --- /dev/null +++ b/packages/runtime/src/file-mutation-guard.ts @@ -0,0 +1,284 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { createHash } from 'node:crypto'; +import { constants } from 'node:fs'; +import { lstat, open, type FileHandle } from 'node:fs/promises'; +import { dirname } from 'node:path'; +import { + FILE_MUTATION_MAX_BYTES, + FileMutationGuardSchema, + type FileMutation, + type FileMutationPreparation, + type FileWriteResult, + type FileEditResult, + type FileIdentity, + type FileMutationDirectories, + type FileMutationGuard, +} from './file-mutation-guard-contract.js'; +import { createUnifiedDiff, createEditUnifiedDiff } from './unified-diff.js'; +import { computeEditedSource, type EditMatch } from './edit-replace.js'; +import { + hostVisibilityAfterWrite, + openStableTarget, + StableWriteFailure, + writeThroughHandle, +} from './file-stable-write.js'; + +function identity(metadata: { dev: bigint; ino: bigint }): FileIdentity { + return { dev: String(metadata.dev), ino: String(metadata.ino) }; +} + +function sameIdentity(left: FileIdentity, right: FileIdentity): boolean { + return left.dev === right.dev && left.ino === right.ino; +} + +function contentDigest(content: Uint8Array): string { + return `sha256:${createHash('sha256').update(content).digest('hex')}`; +} + +/** Shared production Write result, including the existing prior-content convention. */ +export function buildFileWriteResult( + path: string, + content: string, + previous: string, +): FileWriteResult { + const diff = + previous === 'unknown' + ? undefined + : createUnifiedDiff(path, previous === 'new' ? undefined : previous, content); + return { + kind: 'write', + ok: true, + path, + bytes: Buffer.byteLength(content, 'utf8'), + ...(diff === undefined ? {} : { diff }), + }; +} + +async function directoryIdentity(path: string): Promise { + const metadata = await lstat(path, { bigint: true }); + if (!metadata.isDirectory() || metadata.isSymbolicLink()) + throw new Error('Not a stable directory'); + return identity(metadata); +} + +/** Bounded even if a concurrent writer grows the file after fstat. */ +async function readBounded(handle: FileHandle): Promise { + const limit = FILE_MUTATION_MAX_BYTES; + const buffer = Buffer.alloc(limit + 1); + let size = 0; + while (size < buffer.length) { + const read = await handle.read(buffer, size, buffer.length - size, size); + if (read.bytesRead === 0) break; + size += read.bytesRead; + } + return size > limit ? undefined : buffer.subarray(0, size); +} + +export function buildFileEditResult( + path: string, + previous: string, + edited: EditMatch, +): FileEditResult { + const diff = createEditUnifiedDiff(path, previous, edited.content, edited); + return { + kind: 'edit', + ok: true, + path, + replacements: 1, + matchedVia: edited.matchedVia, + startLine: edited.startLine, + endLine: edited.endLine, + ...(diff === undefined ? {} : { diff }), + }; +} + +/** Keep Edit matching failures actionable across the worker boundary. */ +export class FileMutationEditConflict extends Error { + constructor(error: unknown) { + super(error instanceof Error ? error.message : 'Edit could not be applied.'); + this.name = 'FileMutationEditConflict'; + } +} + +function transform(path: string, mutation: FileMutation, previous: string | undefined) { + if (mutation.kind === 'write') + return { + content: mutation.content, + result: buildFileWriteResult(path, mutation.content, previous ?? 'new'), + }; + if (previous === undefined) throw new Error('Edit requires an existing file'); + let edited: EditMatch; + try { + edited = computeEditedSource(previous, mutation.oldString, mutation.newString, path); + } catch (error) { + throw new FileMutationEditConflict(error); + } + return { content: edited.content, result: buildFileEditResult(path, previous, edited) }; +} + +/** Pure input eligibility check, before any file observation or content transport. */ +export function fileMutationInputEligible(mutation: FileMutation): boolean { + const inputs = + mutation.kind === 'write' ? [mutation.content] : [mutation.oldString, mutation.newString]; + return inputs.every( + (input) => + Buffer.byteLength(input, 'utf8') <= FILE_MUTATION_MAX_BYTES && + Buffer.from(input, 'utf8').toString('utf8') === input, + ); +} + +/** Called only after the owning executor has authorized and canonicalized both paths. */ +export async function prepareFileMutation( + cwd: string, + path: string, + preparation: FileMutationPreparation, + hostDirectories?: FileMutationDirectories, +): Promise { + // Authorization still runs before this helper, including for ineligible inputs. + if (!preparation.eligible) return; + // Unsupported targets retain ordinary execution, without a content guard. + try { + const workspace = { + path: cwd, + identity: hostDirectories?.workspace ?? (await directoryIdentity(cwd)), + }; + const target = { + path, + parentIdentity: hostDirectories?.parent ?? (await directoryIdentity(dirname(path))), + }; + const metadata = await lstat(path, { bigint: true }).catch((error: NodeJS.ErrnoException) => { + if (error.code === 'ENOENT') return undefined; + throw error; + }); + let before: FileMutationGuard['before'] = { kind: 'missing' }; + if (metadata) { + if ( + !metadata.isFile() || + metadata.isSymbolicLink() || + metadata.size > BigInt(FILE_MUTATION_MAX_BYTES) + ) + return; + const handle = await open( + path, + constants.O_RDONLY | + (process.platform === 'win32' ? 0 : constants.O_NOFOLLOW | constants.O_NONBLOCK), + ); + try { + const opened = await handle.stat({ bigint: true }); + if (!opened.isFile() || !sameIdentity(identity(metadata), identity(opened))) return; + const bytes = await readBounded(handle); + if (!bytes || !Buffer.from(bytes.toString('utf8'), 'utf8').equals(bytes)) return; + if (await hostVisibilityAfterWrite(path, handle)) return; + before = { + kind: 'file', + identity: identity(opened), + bytes: bytes.length, + digest: contentDigest(bytes), + }; + } finally { + await handle.close(); + } + } + if (preparation.kind === 'edit' && before.kind === 'missing') return; + const guard: FileMutationGuard = { kind: preparation.kind, workspace, target, before }; + return FileMutationGuardSchema.safeParse(guard).success ? guard : undefined; + } catch { + return undefined; + } +} + +/** Validate through the descriptor that will be mutated, before any truncate. */ +export async function executePreparedFileMutation( + cwd: string, + path: string, + mutation: FileMutation, + guard: FileMutationGuard, + hostDirectories?: FileMutationDirectories, +): Promise { + const changed = () => + new StableWriteFailure( + 'path_changed', + 'The prepared file mutation target changed; re-read before writing.', + ); + if ( + !FileMutationGuardSchema.safeParse(guard).success || + cwd !== guard.workspace.path || + path !== guard.target.path || + mutation.kind !== guard.kind + ) + throw changed(); + if ( + !sameIdentity( + hostDirectories?.workspace ?? (await directoryIdentity(cwd)), + guard.workspace.identity, + ) || + !sameIdentity( + hostDirectories?.parent ?? (await directoryIdentity(dirname(path))), + guard.target.parentIdentity, + ) + ) + throw changed(); + const prior = guard.before; + // A missing Write target is created only here, after T1, using exclusive creation. + const handle = await openStableTarget({ + path, + approvedIdentity: prior.kind === 'file' ? prior.identity : undefined, + targetType: prior.kind === 'file' ? 'file' : 'missing', + }); + try { + // Only Linux mount namespaces receive hostDirectories; their launcher validates + // pinned mount sources and host visibility before dispatch and after completion. + const sameDirectories = + hostDirectories !== undefined || + (await Promise.all([directoryIdentity(cwd), directoryIdentity(dirname(path))]) + .then( + ([workspace, parent]) => + sameIdentity(workspace, guard.workspace.identity) && + sameIdentity(parent, guard.target.parentIdentity), + ) + .catch(() => false)); + if (!sameDirectories) { + if (prior.kind === 'missing') + throw new StableWriteFailure( + 'outcome_unknown', + 'The prepared directory changed after exclusive creation.', + ); + throw changed(); + } + let previous: string | undefined; + if (prior.kind === 'file') { + const metadata = await handle.stat({ bigint: true }); + if (!metadata.isFile()) throw changed(); + const bytes = await readBounded(handle); + if (!bytes || bytes.length !== prior.bytes || contentDigest(bytes) !== prior.digest) + throw changed(); + if (await hostVisibilityAfterWrite(path, handle)) throw changed(); + previous = bytes.toString('utf8'); + } + const next = transform(path, mutation, previous); + await writeThroughHandle(handle, next.content); + const visibility = await hostVisibilityAfterWrite(path, handle); + if (visibility) throw visibility; + return next.result; + } finally { + await handle.close(); + } +} diff --git a/packages/runtime/src/file-write-lock.ts b/packages/runtime/src/file-write-lock.ts index 799d844017d..f9067319ad7 100644 --- a/packages/runtime/src/file-write-lock.ts +++ b/packages/runtime/src/file-write-lock.ts @@ -35,6 +35,40 @@ const tails = new Map>(); +/** A prepared write owns the same queue across preparation, T1 and execution. */ +export async function acquireFileWriteLock(key: string, signal?: AbortSignal): Promise<() => void> { + signal?.throwIfAborted(); + const previous = tails.get(key) ?? Promise.resolve(); + let release!: () => void; + const held = new Promise((resolve) => { + release = resolve; + }); + const tail = previous.then(() => held); + tails.set(key, tail); + void tail.then(() => { + if (tails.get(key) === tail) tails.delete(key); + }); + let abort!: () => void; + const cancelled = new Promise((_, reject) => { + abort = () => { + release(); + reject(signal?.reason ?? new Error('Write preparation aborted')); + }; + signal?.addEventListener('abort', abort, { once: true }); + }); + try { + if (signal?.aborted) abort(); + await Promise.race([previous, cancelled]); + signal?.throwIfAborted(); + return release; + } catch (error) { + release(); + throw error; + } finally { + signal?.removeEventListener('abort', abort); + } +} + /** * Runs `fn` exclusively for `key`: it waits until any prior work for `key` * settles, then runs, then releases the key for the next waiter. Distinct keys diff --git a/packages/runtime/src/filesystem-executor.ts b/packages/runtime/src/filesystem-executor.ts index 99921db8dc2..86ba59f86f4 100644 --- a/packages/runtime/src/filesystem-executor.ts +++ b/packages/runtime/src/filesystem-executor.ts @@ -29,6 +29,11 @@ // "full access" stricter than ask mode, which grants :slash_tmp outright (#2083). import { Buffer } from 'node:buffer'; +import { + buildFileWriteResult, + buildFileEditResult, + fileMutationInputEligible, +} from './file-mutation-guard.js'; import { readPage } from './read-page.js'; import { formatJsonText } from './format-json.js'; import { lstat, realpath, stat } from 'node:fs/promises'; @@ -38,14 +43,14 @@ import type { PermissionMode } from '@maka/core/permission'; import type { PermissionProfile } from '@maka/core/permission-profile'; import { ToolOutcomeUnknownError } from '@maka/core/events'; import { computeEditedSource } from './edit-replace.js'; -import { createEditUnifiedDiff, createUnifiedDiff } from './unified-diff.js'; +import { createUnifiedDiff } from './unified-diff.js'; import { classifyFailedMutationOutcome, type FilesystemTargetIdentity, } from './filesystem-authority.js'; import { StableWriteFailure } from './file-stable-write.js'; import { applyUpdateToContent } from './apply-patch-file.js'; -import { withFileWriteLock } from './file-write-lock.js'; +import { acquireFileWriteLock, withFileWriteLock } from './file-write-lock.js'; import type { FilesystemWorkerClient, FilesystemWorkerClientOperation, @@ -61,6 +66,7 @@ import type { WorkspaceApplyPatchExecutor, WorkspacePathScope, WorkspaceReadModifyWriteExecutor, + WorkspacePreparedMutationExecutor, WorkspaceSearchExecutor, WorkspaceWriteExecutor, } from './workspace-executor.js'; @@ -108,6 +114,11 @@ export interface ApplyPatchResult { } export interface FilesystemExecutor { + prepareMutation( + input: FilesystemExecuteInput & { + operation: Extract; + }, + ): Promise; /** * Run one operation under the authority of the boundary it carries. A mutating * operation holds the target's write lock for its whole read-modify-write, so @@ -117,11 +128,17 @@ export interface FilesystemExecutor { applyPatch(input: FilesystemApplyPatchInput): Promise; } +export interface PreparedFilesystemMutation { + execute(): Promise; + cancel(): void; +} + /** The workspace primitives the host-local backend drives. */ export type FilesystemWorkspaceExecutor = WorkspaceWriteExecutor & WorkspaceEditExecutor & Partial & Partial & + Partial & WorkspaceSearchExecutor; export interface BoundaryFilesystemExecutorInput { @@ -288,6 +305,85 @@ export function createBoundaryFilesystemExecutor( return { key: normalized.enforcementPath, canonicalPath: normalized.enforcementPath }; } return { + async prepareMutation(input) { + const call = { + ...input, + ...(input.executionBoundary + ? { executionBoundary: structuredClone(input.executionBoundary) } + : {}), + }; + // Keep an immutable private copy: the caller never owns execution args. + const operation = structuredClone(call.operation); + const eligible = fileMutationInputEligible(operation); + const { key, canonicalPath } = await writeLockTarget(call, operation.path); + const expectedIdentity = await captureIdentityAtLockAcquisition(canonicalPath, true); + const release = await acquireFileWriteLock(key, call.abortSignal); + let state: 'prepared' | 'executing' | 'closed' = 'prepared'; + const cancel = () => { + if (state !== 'prepared') return; + state = 'closed'; + call.abortSignal?.removeEventListener('abort', cancel); + release(); + }; + try { + call.abortSignal?.throwIfAborted(); + const prepared = await run( + { + ...call, + operation: + operation.kind === 'write' + ? { kind: 'prepare_write', path: operation.path, eligible } + : { + kind: 'prepare_edit', + path: operation.path, + eligible, + }, + }, + expectedIdentity, + ); + if (prepared.kind !== (operation.kind === 'write' ? 'prepare_write' : 'prepare_edit')) + throw new Error('Invalid Write preparation response'); + const guard = prepared.guard ? structuredClone(prepared.guard) : undefined; + if ( + guard && + (expectedIdentity + ? guard.before.kind !== 'file' || + guard.before.identity.dev !== expectedIdentity.dev || + guard.before.identity.ino !== expectedIdentity.ino + : guard.before.kind !== 'missing') + ) { + throw new StableWriteFailure( + 'path_changed', + 'The Write target changed while waiting for its lock.', + ); + } + call.abortSignal?.throwIfAborted(); + call.abortSignal?.addEventListener('abort', cancel, { once: true }); + return { + cancel, + async execute() { + if (state !== 'prepared') throw new Error('Prepared Write has already settled'); + call.abortSignal?.throwIfAborted(); + state = 'executing'; + call.abortSignal?.removeEventListener('abort', cancel); + try { + return await run( + { ...call, operation: { ...operation, ...(guard ? { guard } : {}) } }, + expectedIdentity, + ); + } catch (error) { + throw settleMutationFailure(error); + } finally { + state = 'closed'; + release(); + } + }, + }; + } catch (error) { + cancel(); + throw error; + } + }, async execute(call) { if (operationAccess(call.operation.kind) !== 'write') return await run(call); // Canonicalisation without any containment check, so a target the policy @@ -384,6 +480,22 @@ function createWorkspaceFilesystemExecutor( return { async execute({ operation, cwd, abortSignal }, scope, expectedIdentity) { switch (operation.kind) { + case 'prepare_write': + case 'prepare_edit': { + const guard = + workspace.prepareMutation && workspace.executePreparedMutation + ? await workspace.prepareMutation({ + cwd, + path: operation.path, + preparation: { + kind: operation.kind === 'prepare_write' ? 'write' : 'edit', + eligible: operation.eligible, + }, + scope, + }) + : undefined; + return { kind: operation.kind, ...(guard ? { guard } : {}) }; + } case 'read': { const { path } = await workspace.resolveExistingPath({ cwd, @@ -406,6 +518,17 @@ function createWorkspaceFilesystemExecutor( }; } case 'write': { + if (operation.guard) { + if (!workspace.executePreparedMutation) + throw new Error('Prepared Write executor unavailable'); + return workspace.executePreparedMutation({ + cwd, + path: operation.path, + mutation: operation, + scope, + guard: operation.guard, + }); + } const { path } = await workspace.resolveWritablePath({ cwd, path: operation.path, @@ -423,21 +546,7 @@ function createWorkspaceFilesystemExecutor( approvedIdentity: expectedIdentity, transform: () => operation.content, }); - const diff = - result.previous === 'unknown' - ? undefined - : createUnifiedDiff( - path, - result.previous === 'new' ? undefined : result.previous, - operation.content, - ); - return { - kind: 'write', - ok: true, - path, - bytes: Buffer.byteLength(operation.content, 'utf8'), - ...(diff !== undefined ? { diff } : {}), - }; + return buildFileWriteResult(path, operation.content, result.previous); } // Fallback (remote/isolated workspace): path-based, unprotected by // the identity authority. @@ -499,6 +608,17 @@ function createWorkspaceFilesystemExecutor( return { kind: 'apply_patch', ok: true, path: patched.path }; } case 'edit': { + if (operation.guard) { + if (!workspace.executePreparedMutation) + throw new Error('Prepared Edit executor unavailable'); + return workspace.executePreparedMutation({ + cwd, + path: operation.path, + mutation: operation, + scope, + guard: operation.guard, + }); + } const { path } = await workspace.resolveExistingPath({ cwd, path: operation.path, @@ -526,17 +646,7 @@ function createWorkspaceFilesystemExecutor( return edited.content; }, }); - const diff = createEditUnifiedDiff(path, originalContent, edited.content, edited); - return { - kind: 'edit', - ok: true, - path, - replacements: 1, - matchedVia: edited.matchedVia, - startLine: edited.startLine, - endLine: edited.endLine, - ...(diff !== undefined ? { diff } : {}), - }; + return buildFileEditResult(path, originalContent, edited); } const read = await workspace.readFile({ cwd, path }); if ('bytes' in read) throw new Error('Edit does not support image files.'); @@ -547,17 +657,7 @@ function createWorkspaceFilesystemExecutor( operation.path, ); await workspace.writeFile({ cwd, path, content: edited.content }); - const diff = createEditUnifiedDiff(path, read.content, edited.content, edited); - return { - kind: 'edit', - ok: true, - path, - replacements: 1, - matchedVia: edited.matchedVia, - startLine: edited.startLine, - endLine: edited.endLine, - ...(diff !== undefined ? { diff } : {}), - }; + return buildFileEditResult(path, read.content, edited); } case 'format_json': { const { path } = await workspace.resolveExistingPath({ diff --git a/packages/runtime/src/filesystem-worker/client.ts b/packages/runtime/src/filesystem-worker/client.ts index 8d32e02a297..5ecba1d91da 100644 --- a/packages/runtime/src/filesystem-worker/client.ts +++ b/packages/runtime/src/filesystem-worker/client.ts @@ -18,6 +18,7 @@ */ import { randomUUID } from 'node:crypto'; +import type { FileMutationDirectories } from '../file-mutation-guard-contract.js'; import { lstat, realpath } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; @@ -100,7 +101,7 @@ export interface FilesystemWorkerExecuteInput { permissionProfile?: PermissionProfile; abortSignal?: AbortSignal; /** - * The caller's T0 observation, see `FilesystemWorkerExpectedIdentity`. + * The caller's T0 guard, see `FilesystemWorkerExpectedIdentity`. * * REQUIRED for write operations: the client throws at runtime when a write * arrives without one, so a JavaScript caller (which TypeScript cannot @@ -352,11 +353,73 @@ export class FilesystemWorkerClient { ...parsedOperation.data, path: target.enforcementPath, }); + // Linux mount namespaces may expose synthetic ancestor inodes. Keep the + // host identity bound to the actual mount source and verify host visibility. + let hostDirectories: FileMutationDirectories | undefined; + const captureDirectory = async (path: string) => { + const metadata = await lstat(path, { bigint: true }); + if (!metadata.isDirectory() || metadata.isSymbolicLink()) + throw new Error('Directory changed'); + return { dev: String(metadata.dev), ino: String(metadata.ino) }; + }; + const directoriesUnchanged = async () => { + if (!hostDirectories) return true; + try { + const workspace = await captureDirectory(canonicalCwd); + const parent = await captureDirectory(dirname(target.enforcementPath)); + return ( + workspace.dev === hostDirectories.workspace.dev && + workspace.ino === hostDirectories.workspace.ino && + parent.dev === hostDirectories.parent.dev && + parent.ino === hostDirectories.parent.ino + ); + } catch { + return false; + } + }; + if ( + operation.kind === 'prepare_write' || + operation.kind === 'prepare_edit' || + ((operation.kind === 'write' || operation.kind === 'edit') && operation.guard) + ) { + try { + hostDirectories = { + workspace: await captureDirectory(canonicalCwd), + parent: await captureDirectory(dirname(target.enforcementPath)), + }; + } catch { + throw clientError( + 'path_changed', + 'validation', + requestId, + 'Write directories could not be observed.', + ); + } + if ((operation.kind === 'write' || operation.kind === 'edit') && operation.guard) { + const guard = operation.guard; + if ( + guard.workspace.path !== canonicalCwd || + guard.target.path !== target.enforcementPath || + guard.workspace.identity.dev !== hostDirectories.workspace.dev || + guard.workspace.identity.ino !== hostDirectories.workspace.ino || + guard.target.parentIdentity.dev !== hostDirectories.parent.dev || + guard.target.parentIdentity.ino !== hostDirectories.parent.ino + ) { + throw clientError( + 'path_changed', + 'validation', + requestId, + 'The prepared Write directories changed.', + ); + } + } + } const request = { version: FILESYSTEM_WORKER_PROTOCOL_VERSION, requestId, operation, operationBoundary, + ...(hostDirectories ? { hostDirectories } : {}), expectedTarget: { enforcementPath: target.enforcementPath, access, @@ -369,7 +432,7 @@ export class FilesystemWorkerClient { identity: typeof writeIdentity === 'object' ? (identity ?? 'missing') : writeIdentity, }, } as const; - const requestJson = JSON.stringify(request); + let requestJson = JSON.stringify(request); if (Buffer.byteLength(requestJson, 'utf8') > FILESYSTEM_WORKER_MAX_REQUEST_BYTES) { throw clientError('request_overflow', 'validation', requestId); } @@ -511,6 +574,19 @@ export class FilesystemWorkerClient { pinnedMetadata.push({ ...pinned, path: metadata.path }); } } + if ( + hostDirectories && + pinnedRuntimeWritableRoot && + (String(pinnedRuntimeWritableRoot.device) !== hostDirectories.parent.dev || + String(pinnedRuntimeWritableRoot.inode) !== hostDirectories.parent.ino) + ) { + throw clientError( + 'path_changed', + 'validation', + requestId, + 'The prepared Write parent changed before pinning.', + ); + } transformed = this.input.sandboxManager.transform({ platform, command: { @@ -564,6 +640,22 @@ export class FilesystemWorkerClient { }); } + if (!(await directoriesUnchanged())) { + releasePinnedPaths(); + throw clientError( + 'path_changed', + 'validation', + requestId, + 'The prepared Write directories changed before launch.', + ); + } + // Native namespaces must validate their real directories inside the worker. + // Only Linux bind mounts need the host identities in place of synthetic ones. + requestJson = JSON.stringify({ + ...request, + hostDirectories: transformed.sandboxType === 'linux' ? hostDirectories : undefined, + }); + let processResult: Awaited>; try { processResult = await this.runProcess({ @@ -700,6 +792,19 @@ export class FilesystemWorkerClient { true, ); } + if (!(await directoriesUnchanged())) { + throw clientError( + operation.kind === 'write' || operation.kind === 'edit' + ? 'outcome_unknown' + : 'path_changed', + 'operation', + requestId, + 'The prepared Write directories changed while the worker ran.', + false, + {}, + true, + ); + } return response.result; } } diff --git a/packages/runtime/src/filesystem-worker/operations.ts b/packages/runtime/src/filesystem-worker/operations.ts index 75fcdb9f77c..16805e38abf 100644 --- a/packages/runtime/src/filesystem-worker/operations.ts +++ b/packages/runtime/src/filesystem-worker/operations.ts @@ -18,7 +18,15 @@ */ import { searchFiles, GrepSearchError, type GrepRunner } from '../grep-search.js'; +import { + prepareFileMutation, + FileMutationEditConflict, + executePreparedFileMutation, + buildFileEditResult, + buildFileWriteResult, +} from '../file-mutation-guard.js'; import { promises as fs } from 'node:fs'; +import type { FileMutationDirectories } from '../file-mutation-guard-contract.js'; import { globFiles } from '../glob-search.js'; import { dirname, isAbsolute, parse, resolve } from 'node:path'; import { isPathInside } from '../path-containment.js'; @@ -38,7 +46,7 @@ import { import { computeEditedSource } from '../edit-replace.js'; import { readPage } from '../read-page.js'; import { formatJsonText } from '../format-json.js'; -import { createEditUnifiedDiff, createUnifiedDiff } from '../unified-diff.js'; +import { createUnifiedDiff } from '../unified-diff.js'; import { compareAndDeleteEntry, hostVisibilityAfterWrite, @@ -67,6 +75,8 @@ import { isLikelySandboxDenial } from '../sandbox/detect.js'; const { realpath, realpathAllowMissing, resolveCanonicalDirectoryEntryTarget } = sandboxPathApi(); export interface FilesystemWorkerOperationDependencies { + /** Host identities captured outside the sandbox mount namespace. */ + hostDirectories?: FileMutationDirectories; grepExecutable?: string; /** Where this worker runs, as the Host observed it; names the install location in Grep's guidance. */ ripgrepEnvironment?: RipgrepEnvironment; @@ -94,7 +104,7 @@ export async function executeFilesystemWorkerRequest( result: await executeFilesystemOperation( request.operation, request.operationBoundary, - dependencies, + { ...dependencies, hostDirectories: request.hostDirectories }, request.expectedTarget, ), }; @@ -116,6 +126,25 @@ export async function executeFilesystemOperation( expectedTarget?: FilesystemWorkerTarget, ): Promise { switch (operation.kind) { + case 'prepare_write': + case 'prepare_edit': { + const path = await resolveWritableAllowed( + operation.cwd, + operation.path, + 'Write', + operationBoundary, + ); + const guard = await prepareFileMutation( + operation.cwd, + path, + { + kind: operation.kind === 'prepare_write' ? 'write' : 'edit', + eligible: operation.eligible, + }, + dependencies.hostDirectories, + ); + return { kind: operation.kind, ...(guard ? { guard } : {}) }; + } case 'read': { const path = await resolveExistingAllowed( operation.cwd, @@ -160,6 +189,15 @@ export async function executeFilesystemOperation( 'Write', operationBoundary, ); + if (operation.guard) { + return executePreparedFileMutation( + operation.cwd, + path, + { kind: 'write', content: operation.content }, + operation.guard, + dependencies.hostDirectories, + ); + } // Pin the approved object (#2600): open once, validate the identity on // the descriptor, and write through that descriptor — a path swap between // validation and the write cannot divert the bytes onto the replacement. @@ -192,17 +230,7 @@ export async function executeFilesystemOperation( // the bytes went to an orphan and the visible file is the replacement. const visibility = await hostVisibilityAfterWrite(path, handle); if (visibility) throw visibility; - const diff = - previous === 'unknown' - ? undefined - : createUnifiedDiff(path, previous === 'new' ? undefined : previous, operation.content); - return { - kind: 'write', - ok: true, - path, - bytes: Buffer.byteLength(operation.content, 'utf8'), - ...(diff !== undefined ? { diff } : {}), - }; + return buildFileWriteResult(path, operation.content, previous); } finally { await handle.close(); } @@ -262,6 +290,14 @@ export async function executeFilesystemOperation( 'write', operationBoundary, ); + if (operation.guard) + return executePreparedFileMutation( + operation.cwd, + path, + operation, + operation.guard, + dependencies.hostDirectories, + ); const handle = await openStableTarget({ path, approvedIdentity: @@ -287,17 +323,7 @@ export async function executeFilesystemOperation( await writeThroughHandle(handle, source.content); const visibility = await hostVisibilityAfterWrite(path, handle); if (visibility) throw visibility; - const diff = createEditUnifiedDiff(path, content, source.content, source); - return { - kind: 'edit', - ok: true, - path, - replacements: 1, - matchedVia: source.matchedVia, - startLine: source.startLine, - endLine: source.endLine, - ...(diff !== undefined ? { diff } : {}), - }; + return buildFileEditResult(path, content, source); } finally { await handle.close(); } @@ -461,7 +487,7 @@ function normalizeOperationError(error: unknown): FilesystemOperationError { if (error instanceof StableWriteFailure) { return operationError(error.code, error.message); } - if (error instanceof ApplyPatchRejectedError) { + if (error instanceof ApplyPatchRejectedError || error instanceof FileMutationEditConflict) { return operationError('edit_conflict', error.message); } const code = nodeErrorCode(error); diff --git a/packages/runtime/src/filesystem-worker/protocol.ts b/packages/runtime/src/filesystem-worker/protocol.ts index bf1512c4fe5..4659d9033fd 100644 --- a/packages/runtime/src/filesystem-worker/protocol.ts +++ b/packages/runtime/src/filesystem-worker/protocol.ts @@ -18,19 +18,25 @@ */ import { z } from 'zod'; +import { FileMutationGuardSchema } from '../file-mutation-guard-contract.js'; import { readContinuationSchema, readPageSchema } from '../read-page.js'; import { validateSandboxBoundaryExpansion } from '@maka/core/sandbox-boundary'; import { GREP_MAX_LINES, GREP_MAX_LINES_PER_FILE, GREP_MAX_MATCH_BYTES } from '../grep-search.js'; -// v12 adds chat image reads with a bounded payload and no model dimension limit. -// Older workers cannot satisfy this read policy and must be rejected at the handshake. -export const FILESYSTEM_WORKER_PROTOCOL_VERSION = 12 as const; +// v15 removes mutation content from preparation requests; only input eligibility crosses. +// Older workers must be rejected rather than silently discard mutation guards. +export const FILESYSTEM_WORKER_PROTOCOL_VERSION = 15 as const; /** The single authority on which operation kinds are writes. Shared by the * client (permission/identity decisions) and the worker (operation guards) so * the set cannot drift. */ export function operationAccess(kind: FilesystemWorkerOperation['kind']): 'read' | 'write' { - return kind === 'write' || kind === 'apply_patch' || kind === 'edit' || kind === 'format_json' + return kind === 'write' || + kind === 'prepare_write' || + kind === 'prepare_edit' || + kind === 'apply_patch' || + kind === 'edit' || + kind === 'format_json' ? 'write' : 'read'; } @@ -112,7 +118,24 @@ export const FilesystemWorkerOperationSchema = z.union([ continuation: readContinuationSchema.optional(), }) .strict(), - z.object({ kind: z.literal('write'), cwd, path, content: z.string() }).strict(), + z + .object({ + kind: z.literal('write'), + cwd, + path, + content: z.string(), + guard: FileMutationGuardSchema.optional(), + }) + .strict(), + z.object({ kind: z.literal('prepare_write'), cwd, path, eligible: z.boolean() }).strict(), + z + .object({ + kind: z.literal('prepare_edit'), + cwd, + path, + eligible: z.boolean(), + }) + .strict(), z .object({ kind: z.literal('apply_patch'), @@ -130,6 +153,7 @@ export const FilesystemWorkerOperationSchema = z.union([ path, oldString: z.string(), newString: z.string(), + guard: FileMutationGuardSchema.optional(), }) .strict(), z @@ -170,10 +194,20 @@ export const FilesystemWorkerRequestSchema = z operation: FilesystemWorkerOperationSchema, operationBoundary: OperationBoundarySchema, expectedTarget: FilesystemWorkerTargetSchema, + hostDirectories: z + .object({ workspace: FilesystemTargetIdentitySchema, parent: FilesystemTargetIdentitySchema }) + .strict() + .optional(), }) .strict(); export const FilesystemWorkerResultSchema = z.discriminatedUnion('kind', [ + z + .object({ + kind: z.enum(['prepare_write', 'prepare_edit']), + guard: FileMutationGuardSchema.optional(), + }) + .strict(), readPageSchema.extend({ kind: z.literal('read') }).strict(), z .object({ diff --git a/packages/runtime/src/tool-runtime.ts b/packages/runtime/src/tool-runtime.ts index 0ab11a1756c..99eb83eb4a9 100644 --- a/packages/runtime/src/tool-runtime.ts +++ b/packages/runtime/src/tool-runtime.ts @@ -1409,17 +1409,20 @@ export class ToolRuntime { return this.errorReturn(reason); } - let clientCapabilityBoundary: ExecutionBoundary | undefined; - let clientCapabilityPermissionMode: PermissionMode | undefined; + let preparedBoundary: ExecutionBoundary | undefined; + let preparedPermissionMode: PermissionMode | undefined; let preparedExecution: PreparedMakaToolExecution | undefined; - if (tool.hostAdmission === 'client_capability') { + if ( + tool.hostAdmission === 'client_capability' || + (tool.prepareExecution && this.input.runtimeCommitSink) + ) { try { - clientCapabilityBoundary = await this.readExecutionBoundary(); - clientCapabilityPermissionMode = await this.livePermissionMode(clientCapabilityBoundary); + preparedBoundary = await this.readExecutionBoundary(); + preparedPermissionMode = await this.livePermissionMode(preparedBoundary); } catch (error) { const reason = formatSyntheticToolErrorText(error); await refuseBeforeDispatch(reason); - trace?.emit('tool', 'tool_failed', 'Client Capability boundary read failed', { + trace?.emit('tool', 'tool_failed', 'Tool preparation boundary read failed', { toolUseId, toolName: tool.name, status: 'error', @@ -1428,11 +1431,14 @@ export class ToolRuntime { this.recordLoopGateOutcome(callSignature, true); return this.errorReturn(reason); } - const admissionFailure = !tool.prepareExecution - ? CLIENT_CAPABILITY_PREPARATION_MESSAGE - : clientCapabilityBoundary.kind !== 'bypass' && clientCapabilityPermissionMode !== 'ask' - ? CLIENT_CAPABILITY_BOUNDARY_MESSAGE - : undefined; + const admissionFailure = + tool.hostAdmission !== 'client_capability' + ? undefined + : !tool.prepareExecution + ? CLIENT_CAPABILITY_PREPARATION_MESSAGE + : preparedBoundary.kind !== 'bypass' && preparedPermissionMode !== 'ask' + ? CLIENT_CAPABILITY_BOUNDARY_MESSAGE + : undefined; if (admissionFailure) { await refuseBeforeDispatch(admissionFailure, { reason: 'requires_bypass', @@ -1447,8 +1453,8 @@ export class ToolRuntime { this.recordLoopGateOutcome(callSignature, true); return this.errorReturn(admissionFailure); } - // Narrowed by admissionFailure above; Bypass still prepares so the - // provider cannot regain a direct pre-T1 dispatch path. + // Client capabilities always prepare. Durable filesystem tools also + // prepare before T1, under the same boundary used for execution. const prepareExecution = tool.prepareExecution!; const pauseTarget = this.input.getPermissionPauseTarget(); pauseTarget?.pause(); @@ -1458,22 +1464,31 @@ export class ToolRuntime { turnId, ...(runId ? { runId } : {}), cwd: this.input.header.cwd, - executionBoundary: clientCapabilityBoundary, - permissionMode: clientCapabilityPermissionMode, + executionBoundary: preparedBoundary, + permissionMode: preparedPermissionMode, toolCallId: toolUseId, abortSignal: ctx.abortSignal, }); } catch (error) { const reason = formatSyntheticToolErrorText(error); - await refuseBeforeDispatch(reason); - trace?.emit('tool', 'tool_failed', 'Client Capability preparation failed', { + const sandbox = serializeSandboxError(error); + const boundaryKind = sandboxBoundaryFailureKind(sandbox); + const boundaryDetails = sandboxBoundaryFailureSignal(sandbox); + if (boundaryKind) + this.recordSandboxBoundaryFailure( + boundaryKind, + boundaryCorrectionStep, + sandboxBoundaryDecisionGeneration, + ); + await refuseBeforeDispatch(reason, boundaryDetails); + trace?.emit('tool', 'tool_failed', 'Tool preparation failed', { toolUseId, toolName: tool.name, status: 'error', - errorClass: 'ClientCapabilityPreparation', + errorClass: 'ToolPreparation', }); - this.recordLoopGateOutcome(callSignature, true); - return this.errorReturn(reason); + this.recordLoopGateOutcome(callSignature, true, boundaryKind, boundaryDetails); + return sandbox ? { error: reason, sandbox } : this.errorReturn(reason); } finally { pauseTarget?.resume(); } @@ -1508,11 +1523,6 @@ export class ToolRuntime { if (reservedSubagentSlot) this.releaseSubagentSlot(tool); throw error; } - publishCallEvent(buildCallEvent('dispatch')); - emitToolStartedTrace(); - if (durableAttempt) { - this.durableToolAttempts.set(durableAttemptKey(turnId, toolUseId), durableAttempt); - } const startedAt = this.input.now(); const output = createToolOutputDeltaEmitter({ sessionId: this.input.sessionId, @@ -1531,6 +1541,11 @@ export class ToolRuntime { let attemptBoundaryKind: SandboxBoundaryFailureKind | undefined; let attemptBoundaryDetails: SandboxBoundaryFailureDetails; try { + publishCallEvent(buildCallEvent('dispatch')); + emitToolStartedTrace(); + if (durableAttempt) { + this.durableToolAttempts.set(durableAttemptKey(turnId, toolUseId), durableAttempt); + } // Pause the stream idle watchdog for the whole tool execution. In the // ai-sdk step loop a tool runs *between* model requests — the tool-call // step's stream already finished and the next request has not started — @@ -1543,9 +1558,9 @@ export class ToolRuntime { pauseTarget?.pause(); try { const runId = this.input.runId; - const executionBoundary = clientCapabilityBoundary ?? (await this.readExecutionBoundary()); + const executionBoundary = preparedBoundary ?? (await this.readExecutionBoundary()); const permissionMode = - clientCapabilityPermissionMode ?? (await this.livePermissionMode(executionBoundary)); + preparedPermissionMode ?? (await this.livePermissionMode(executionBoundary)); const toolContext: MakaToolContext = { sessionId: this.input.sessionId, turnId, @@ -1630,10 +1645,15 @@ export class ToolRuntime { queue, ), }; - const invokeTool = () => - preparedExecution - ? preparedExecution.execute(toolContext) - : tool.impl(structuredClone(executionArgs) as never, toolContext); + const invokeTool = () => { + if (!preparedExecution) + return tool.impl(structuredClone(executionArgs) as never, toolContext); + // Execution takes ownership of cleanup, including when it throws. + // Only a preparation that never starts may be cancelled below. + const execution = preparedExecution; + preparedExecution = undefined; + return execution.execute(toolContext); + }; const prepareOperationValue = async () => { const result = await invokeTool(); if ( @@ -1924,6 +1944,7 @@ export class ToolRuntime { }); return sandboxError ? { error: msg, sandbox: sandboxError } : this.errorReturn(msg); } finally { + await preparedExecution?.cancel(); this.recordLoopGateOutcome( callSignature, attemptFailed, diff --git a/packages/runtime/src/workspace-executor.ts b/packages/runtime/src/workspace-executor.ts index fc09a8507bc..47d66f2a1f8 100644 --- a/packages/runtime/src/workspace-executor.ts +++ b/packages/runtime/src/workspace-executor.ts @@ -19,6 +19,14 @@ import { promises as fs } from 'node:fs'; import { exec } from 'node:child_process'; +import type { + FileMutationGuard, + FileMutation, + FileMutationPreparation, + FileWriteResult, + FileEditResult, +} from './file-mutation-guard-contract.js'; +import { prepareFileMutation, executePreparedFileMutation } from './file-mutation-guard.js'; import { globFiles } from './glob-search.js'; import { isAbsolute, resolve } from 'node:path'; import { @@ -165,6 +173,22 @@ export interface WorkspaceReadModifyWriteExecutor { readModifyWrite(input: WorkspaceReadModifyWriteInput): Promise; } +export interface WorkspacePreparedMutationExecutor { + prepareMutation(input: { + cwd: string; + path: string; + preparation: FileMutationPreparation; + scope: WorkspacePathScope; + }): Promise; + executePreparedMutation(input: { + cwd: string; + path: string; + mutation: FileMutation; + scope: WorkspacePathScope; + guard: FileMutationGuard; + }): Promise; +} + /** * Which path space a resolution may land in. * @@ -351,6 +375,24 @@ export class LocalWorkspaceExecutor implements WorkspaceExecutor { }; } + async prepareMutation( + input: Parameters[0], + ): Promise { + const cwd = await fs.realpath(input.cwd); + const { path } = await canonicalPathInScope(cwd, input.path, 'Write', input.scope); + // Match the local executor's binary/image result policy. + if (isSupportedImagePath(path)) return undefined; + return prepareFileMutation(cwd, path, input.preparation); + } + + async executePreparedMutation( + input: Parameters[0], + ): Promise { + const cwd = await fs.realpath(input.cwd); + const { path } = await canonicalPathInScope(cwd, input.path, 'Write', input.scope); + return executePreparedFileMutation(cwd, path, input.mutation, input.guard); + } + async readModifyWrite( input: WorkspaceReadModifyWriteInput, ): Promise {