From e6a58db78c954d601da83d8b6599bac1a17c2e1d Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 22 Aug 2026 19:55:54 +0800 Subject: [PATCH 1/7] fix(runtime): distinguish unchecked writes from T0-missing races in the filesystem worker Fixes #3484. expectedIdentity was optional, so a caller without a T0 snapshot was indistinguishable from a T0-missing target that appeared while queued; every write to an existing file without an identity failed with path_changed, breaking verify-windows-sandbox-e2e and blocking the Release Windows check on four PRs. - FilesystemWorkerExpectedIdentity is now required: {dev,ino} | 'missing' | 'unchecked'. Every caller must decide explicitly which CAS contract it participates in. - The wire request carries the T0 marker explicitly (protocol v7) so the worker can tell 'created while queued' from 'no CAS snapshot'. - Worker: t0 'missing' + existing target fails path_changed; t0 'existing' without identity fails invalid_request (buggy caller); t0 'unchecked' skips CAS. Identity-present targets still CAS. - openStableTarget opens an existing target without an identity when the caller is 'unchecked' (no comparison), keeping the create path exclusive (wx) unchanged. - Callers: FilesystemExecutor passes its captured identity or 'missing' for mutations and 'unchecked' for reads; the runtime-host read-only adapter and verify-windows-sandbox-e2e pass 'unchecked'. - Tests: unchecked write to an existing target passes and carries no identity; 'missing' with an existing target still fails path_changed; CAS mismatch still fails; T0-missing marker is preserved on the wire. Generated-by: DSv4F-AstroHan --- .../src/server/execution-composition.ts | 4 +- .../filesystem-mutation-outcome.test.ts | 10 +- .../filesystem-target-identity.test.ts | 9 +- .../filesystem-worker-client.test.ts | 21 +++++ .../filesystem-worker-linux-smoke.test.ts | 8 ++ .../__tests__/filesystem-worker-smoke.test.ts | 8 ++ .../filesystem-worker-windows-smoke.test.ts | 5 + .../src/__tests__/filesystem-worker.test.ts | 60 +++++++++++- packages/runtime/src/file-stable-write.ts | 92 +++++++++++-------- packages/runtime/src/filesystem-executor.ts | 11 ++- .../runtime/src/filesystem-worker/client.ts | 84 +++++++++++------ .../src/filesystem-worker/operations.ts | 36 ++++++-- .../runtime/src/filesystem-worker/protocol.ts | 22 +++-- scripts/verify-windows-sandbox-e2e.mjs | 16 ++-- 14 files changed, 287 insertions(+), 99 deletions(-) diff --git a/packages/runtime-host/src/server/execution-composition.ts b/packages/runtime-host/src/server/execution-composition.ts index 85fab99da3..a3a5ca12b3 100644 --- a/packages/runtime-host/src/server/execution-composition.ts +++ b/packages/runtime-host/src/server/execution-composition.ts @@ -1631,7 +1631,9 @@ function adaptManagedWorkspaceFilesystemWorker( ): ManagedWorkspaceFilesystemWorker { return { async execute(input) { - const result = await worker.execute(input); + // Read-only operations never participate in CAS; the adapter says so + // explicitly (#3484) instead of relying on an absent optional field. + const result = await worker.execute({ ...input, expectedIdentity: 'unchecked' }); switch (result.kind) { case 'read': case 'read_image': diff --git a/packages/runtime/src/__tests__/filesystem-mutation-outcome.test.ts b/packages/runtime/src/__tests__/filesystem-mutation-outcome.test.ts index b5d2b92746..def54c5721 100644 --- a/packages/runtime/src/__tests__/filesystem-mutation-outcome.test.ts +++ b/packages/runtime/src/__tests__/filesystem-mutation-outcome.test.ts @@ -17,6 +17,7 @@ import { type FilesystemWorkerClient, type FilesystemWorkerClientErrorReason, type FilesystemWorkerExecuteInput, + type FilesystemWorkerExpectedIdentity, } from '../filesystem-worker/client.js'; import type { FilesystemWorkerResult } from '../filesystem-worker/protocol.js'; import { createLocalWorkspaceExecutor } from '../workspace-executor.js'; @@ -216,7 +217,7 @@ describe('filesystem mutation T0 identity capture (queue-window closure)', () => releaseFirst = resolve; }); let calls = 0; - let queuedIdentity: { dev: string; ino: string } | undefined; + let queuedIdentity: FilesystemWorkerExpectedIdentity | undefined; const gatedWorker: { execute: (input: FilesystemWorkerExecuteInput) => Promise; } = { @@ -255,9 +256,12 @@ describe('filesystem mutation T0 identity capture (queue-window closure)', () => releaseFirst(); await Promise.all([first, second]); - assert.ok(queuedIdentity, 'the queued mutation should have dispatched to the worker'); + assert.ok( + queuedIdentity && typeof queuedIdentity !== 'string', + 'the queued mutation should have dispatched with the captured identity', + ); assert.equal( - queuedIdentity.ino, + (queuedIdentity as { dev: string; ino: string }).ino, String(original.ino), 'identity must be the inode captured at lock acquisition (before the replacement); a T1 capture would sample the replacement', ); diff --git a/packages/runtime/src/__tests__/filesystem-target-identity.test.ts b/packages/runtime/src/__tests__/filesystem-target-identity.test.ts index 48d59c4c0f..0240f9900c 100644 --- a/packages/runtime/src/__tests__/filesystem-target-identity.test.ts +++ b/packages/runtime/src/__tests__/filesystem-target-identity.test.ts @@ -29,7 +29,7 @@ async function temporaryDirectory(prefix: string): Promise { function requestFor( operation: FilesystemWorkerRequest['operation'], - expectedTarget: FilesystemWorkerTarget, + expectedTarget: Omit, ): FilesystemWorkerRequest { return { version: FILESYSTEM_WORKER_PROTOCOL_VERSION, @@ -40,7 +40,12 @@ function requestFor( entries: [{ path: expectedTarget.enforcementPath, access: 'write', scope: 'exact' }], }, }, - expectedTarget, + expectedTarget: { + ...expectedTarget, + // These tests always CAS against a captured identity or an approved + // missing target; neither is an unchecked caller. + t0: expectedTarget.identity ? 'existing' : 'missing', + }, }; } diff --git a/packages/runtime/src/__tests__/filesystem-worker-client.test.ts b/packages/runtime/src/__tests__/filesystem-worker-client.test.ts index 8ea0ed0296..1f522e8a32 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-client.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-client.test.ts @@ -101,6 +101,7 @@ describe('filesystem worker client permission snapshots', () => { cwd: workspace, executionBoundary: { kind, revision: 1 }, mode: 'execute', + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -129,6 +130,7 @@ describe('filesystem worker client permission snapshots', () => { createWorkspaceWritePermissionProfile(), 0, ), + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -154,6 +156,7 @@ describe('filesystem worker client permission snapshots', () => { cwd: workspace, mode: 'execute', permissionProfile: createReadOnlyPermissionProfile(), + expectedIdentity: 'unchecked', }), isPathDenied, ); @@ -183,6 +186,7 @@ describe('filesystem worker client permission snapshots', () => { cwd: workspace, mode: 'execute', permissionProfile: profile, + expectedIdentity: 'unchecked', }), isPathDenied, ); @@ -210,6 +214,7 @@ describe('filesystem worker client permission snapshots', () => { operation: { kind: 'read', path: target }, cwd: workspace, mode: 'explore', + expectedIdentity: 'unchecked', }), isPathDenied, ); @@ -218,6 +223,7 @@ describe('filesystem worker client permission snapshots', () => { cwd: workspace, mode: 'explore', permissionProfile: profile, + expectedIdentity: 'unchecked', }); assert.deepEqual(result, { kind: 'read', content: 'worker-content' }); @@ -243,6 +249,7 @@ describe('filesystem worker client Grep target scope', () => { operation: grepOperation(target), cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.deepEqual(result, { kind: 'grep', matches: ['file.ts:1:value'] }); @@ -259,6 +266,7 @@ describe('filesystem worker client Grep target scope', () => { operation: grepOperation(directory), cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.equal(requests[0]?.expectedTarget.scope, 'subtree'); @@ -283,6 +291,7 @@ describe('filesystem worker operation-scoped Seatbelt profile', () => { operation: { kind: 'write', path: target, content: 'target' }, cwd: workspace, mode: 'execute', + expectedIdentity: 'unchecked', }); const transform = transforms[0]; @@ -315,6 +324,7 @@ describe('filesystem worker operation-scoped Seatbelt profile', () => { operation: grepOperation(target), cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -338,6 +348,7 @@ describe('filesystem worker operation-scoped Seatbelt profile', () => { operation: grepOperation(target), cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -365,6 +376,7 @@ describe('filesystem worker Linux path context', () => { operation: { kind: 'glob', path: target, pattern: '*.ts', limit: 20 }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -385,6 +397,7 @@ describe('filesystem worker Linux path context', () => { operation: { kind: 'read', path: target }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); const processInput = processInputs[0]; @@ -407,6 +420,8 @@ describe('filesystem worker Linux path context', () => { operation: { kind: 'write', path: target, content: 'new' }, cwd: workspace, mode: 'ask', + // T0 observed no target (a create), so the T0 marker is 'missing'. + expectedIdentity: 'missing', }); const processInput = processInputs[0]; @@ -621,6 +636,7 @@ describe('filesystem worker client dispatch classification', () => { operation: { kind: 'write', path: '/tmp/maka-dispatch-incomplete.txt', content: 'x' }, cwd: '/tmp', mode: 'ask', + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -638,6 +654,7 @@ describe('filesystem worker client dispatch classification', () => { operation: { kind: 'write', path: '/tmp/maka-dispatch-spawn.txt', content: 'x' }, cwd: '/tmp', mode: 'ask', + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -656,6 +673,7 @@ describe('filesystem worker client dispatch classification', () => { operation: { kind: 'write', path: '/tmp/maka-dispatch-noflag.txt', content: 'x' }, cwd: '/tmp', mode: 'ask', + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -757,6 +775,9 @@ describe('filesystem worker client dispatch classification', () => { operation: { kind: 'write', path: target, content: 'new' }, cwd: workspace, mode: 'ask', + // T0 approved the target as missing; it appeared by T1 — the + // "created while queued" race, which must stay path_changed. + expectedIdentity: 'missing', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); diff --git a/packages/runtime/src/__tests__/filesystem-worker-linux-smoke.test.ts b/packages/runtime/src/__tests__/filesystem-worker-linux-smoke.test.ts index f5dfbf8bb3..2bd5b5ec7f 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-linux-smoke.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-linux-smoke.test.ts @@ -61,6 +61,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.equal(await readFile(sourceFile, 'utf8'), 'export const healthSignal = true;\n'); @@ -68,6 +69,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { operation: { kind: 'read', path: sourceFile }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.deepEqual(read, { kind: 'read', @@ -101,6 +103,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.deepEqual(glob, { kind: 'glob', files: ['health.ts'] }); @@ -115,6 +118,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.equal(grep.kind, 'grep'); if (grep.kind === 'grep') { @@ -135,6 +139,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.equal(await readFile(target, 'utf8'), 'created'); @@ -150,6 +155,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { operation: { kind: 'write', path: allowedPath, content: 'blocked' }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }), isPathDenied, ); @@ -159,6 +165,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { cwd: workspace, mode: 'ask', executionBoundary, + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -177,6 +184,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { cwd: workspace, mode: 'ask', executionBoundary, + expectedIdentity: 'unchecked', }); assert.equal(await readFile(allowedPath, 'utf8'), 'outside-ok'); }); diff --git a/packages/runtime/src/__tests__/filesystem-worker-smoke.test.ts b/packages/runtime/src/__tests__/filesystem-worker-smoke.test.ts index 1e5d5fd6d0..4709dfa397 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-smoke.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-smoke.test.ts @@ -46,6 +46,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' operation: { kind: 'write', path: insidePath, content: 'inside-ok' }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.equal(await readFile(insidePath, 'utf8'), 'inside-ok'); @@ -54,6 +55,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' operation: { kind: 'write', path: outsidePath, content: 'blocked' }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }), (error: unknown) => error instanceof FilesystemWorkerClientError && error.reason === 'path_denied', @@ -72,6 +74,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.equal(await readFile(target, 'utf8'), 'created'); @@ -107,6 +110,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' cwd: workspace, mode: 'ask', executionBoundary, + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -124,6 +128,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' cwd: workspace, mode: 'ask', executionBoundary, + expectedIdentity: 'unchecked', }); assert.equal(await readFile(allowedPath, 'utf8'), 'outside-ok'); }); @@ -138,6 +143,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' operation: grepOperation(sourceFile, 'healthSignal'), cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.equal(fileResult.kind, 'grep'); if (fileResult.kind === 'grep') { @@ -149,6 +155,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' operation: grepOperation(sourceDirectory, 'healthSignal'), cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.equal(directoryResult.kind, 'grep'); if (directoryResult.kind === 'grep') { @@ -160,6 +167,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' operation: grepOperation(sourceDirectory, 'does-not-exist'), cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.deepEqual(emptyResult, { kind: 'grep', matches: [] }); }); diff --git a/packages/runtime/src/__tests__/filesystem-worker-windows-smoke.test.ts b/packages/runtime/src/__tests__/filesystem-worker-windows-smoke.test.ts index 967779d4fa..34596fb684 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-windows-smoke.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-windows-smoke.test.ts @@ -103,6 +103,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => { operation: { kind: 'read', path: target }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.equal(read.kind, 'read'); if (read.kind === 'read') assert.match(read.content, /windows-relay-ok/); @@ -118,6 +119,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => { operation: { kind: 'write', path: missing, content: 'blocked' }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }), (error: unknown) => error instanceof FilesystemWorkerClientError && @@ -137,6 +139,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => { operation: { kind: 'glob', path: sourceDirectory, pattern: '**/*.ts' }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }); assert.equal(globResult.kind, 'glob'); if (globResult.kind === 'glob') { @@ -159,6 +162,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => { }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }), (error: unknown) => error instanceof FilesystemWorkerClientError && error.reason === 'grep_unavailable', @@ -175,6 +179,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => { }, cwd: workspace, mode: 'ask', + expectedIdentity: 'unchecked', }), (error: unknown) => error instanceof FilesystemWorkerClientError && error.reason === 'path_denied', diff --git a/packages/runtime/src/__tests__/filesystem-worker.test.ts b/packages/runtime/src/__tests__/filesystem-worker.test.ts index f4ebb2a44d..48b67972e7 100644 --- a/packages/runtime/src/__tests__/filesystem-worker.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker.test.ts @@ -419,6 +419,45 @@ describe('filesystem worker operations', () => { if (!response.ok) assert.equal(response.error.code, 'path_changed'); }); + test('accepts an unchecked write target without an identity (#3484)', async () => { + const root = await temporaryDirectory('maka-worker-unchecked-'); + const target = join(root, 'file.txt'); + await writeFile(target, 'existing', 'utf8'); + + const response = await executeFilesystemWorkerRequest( + await requestFor( + { kind: 'write', cwd: root, path: target, content: 'new' }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' }, + target, + 'unchecked', + ), + ); + + assert.ok(response.ok); + assert.equal(response.result.kind, 'write'); + if (response.result.kind !== 'write') return; + assert.equal(await readFile(target, 'utf8'), 'new'); + }); + + test('rejects a write whose T0-missing target exists at execution time (#3484)', async () => { + const root = await temporaryDirectory('maka-worker-created-'); + const target = join(root, 'file.txt'); + // T0 approved the target as missing; something created it before the + // worker executed. Writing would clobber content the caller never saw. + await writeFile(target, 'external', 'utf8'); + + const response = await executeFilesystemWorkerRequest( + await requestFor( + { kind: 'write', cwd: root, path: target, content: 'new' }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'missing' }, + ), + ); + assert.equal(response.ok, false); + if (!response.ok) assert.equal(response.error.code, 'path_changed'); + // The interloper's content was never touched. + assert.equal(await readFile(target, 'utf8'), 'external'); + }); + test('omits the diff when the content is too large to diff cheaply', async () => { const root = await temporaryDirectory('maka-worker-huge-'); const target = join(root, 'huge.ts'); @@ -504,8 +543,9 @@ describe('filesystem worker operations', () => { async function requestFor( operation: FilesystemWorkerOperation, - expectedTarget: FilesystemWorkerTarget, + expectedTarget: Omit, permissionPath = operation.path, + t0?: FilesystemWorkerTarget['t0'], ): Promise { const operationBoundary: FilesystemWorkerRequest['operationBoundary'] = { filesystem: { @@ -520,8 +560,20 @@ async function requestFor( }; // The real caller captures the target identity at T0; mirror that here so // the worker's mandatory-identity check is satisfied for non-missing targets. - let resolvedTarget = expectedTarget; - if (expectedTarget.targetType !== 'missing' && !expectedTarget.identity) { + let resolvedTarget: FilesystemWorkerTarget = { + ...expectedTarget, + // t0 is always replaced below; the placeholder keeps the type total. + t0: 'existing', + }; + if (t0 !== undefined) { + // The test chose the T0 marker explicitly (e.g. 'unchecked'); keep the + // target exactly as given. + resolvedTarget = { ...expectedTarget, t0 }; + } else if (expectedTarget.targetType === 'missing') { + resolvedTarget = { ...expectedTarget, t0: 'missing' }; + } else if (expectedTarget.identity) { + resolvedTarget = { ...expectedTarget, t0: 'existing' }; + } else { const follow = expectedTarget.targetType !== 'symlink'; try { const metadata = follow @@ -529,11 +581,13 @@ async function requestFor( : await lstat(expectedTarget.enforcementPath, { bigint: true }); resolvedTarget = { ...expectedTarget, + t0: 'existing', identity: { dev: String(metadata.dev), ino: String(metadata.ino) }, }; } catch { // Target may not exist at request construction time (the test sets it up // differently); leave identity absent and let the worker surface it. + resolvedTarget = { ...expectedTarget, t0: 'existing' }; } } return { diff --git a/packages/runtime/src/file-stable-write.ts b/packages/runtime/src/file-stable-write.ts index 08afc9060a..6b69ab3437 100644 --- a/packages/runtime/src/file-stable-write.ts +++ b/packages/runtime/src/file-stable-write.ts @@ -58,51 +58,20 @@ function pathChanged(message: string): StableWriteFailure { * so a failed identity check leaves the file untouched. Windows has no * O_NOFOLLOW; the link is detected with an lstat just before the open and * the residual window is closed by the identity comparison on the fd. - * - Approved-missing target (approvedIdentity undefined): `open(path, 'wx')` — - * atomic create-if-absent. EEXIST means something appeared in the gap. + * - Existing target without an identity (an 'unchecked' caller, targetType is + * a concrete type): same open, no identity comparison — there is nothing to + * compare, and the caller declared it does not participate in CAS. + * - Approved-missing target (approvedIdentity undefined, targetType + * 'missing'): `open(path, 'wx')` — atomic create-if-absent. EEXIST means + * something appeared in the gap. */ export async function openStableTarget(input: { path: string; approvedIdentity: FilesystemTargetIdentity | undefined; + targetType?: 'file' | 'directory' | 'symlink' | 'other' | 'missing'; }): Promise { - const noFollow = process.platform === 'win32' ? 0 : constants.O_NOFOLLOW; if (input.approvedIdentity) { - if (process.platform === 'win32') { - const entry = await lstat(input.path).catch(() => null); - if (entry?.isSymbolicLink()) { - throw pathChanged( - 'The approved filesystem target is a symbolic link; refusing to follow it.', - ); - } - } - let handle: FileHandle; - try { - handle = await open(input.path, constants.O_RDWR | noFollow); - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code === 'ELOOP' || code === 'ENOTDIR' || code === 'ENOENT') { - throw pathChanged('The approved filesystem target changed before execution.'); - } - if (code === 'EACCES' || code === 'EPERM') { - // A write-only target (e.g. mode 0o222) refuses 'r+' but is still a - // legitimate mutation target — unlink semantics need no read - // permission. Retry write-only (still no truncate: identity is - // validated on the descriptor before writeThroughHandle truncates). - // The pinned read of the previous content will fail and the caller - // reports 'unknown' (no diff), which is the pre-existing behaviour. - try { - handle = await open(input.path, constants.O_WRONLY | noFollow); - } catch (retry) { - const retryCode = (retry as NodeJS.ErrnoException).code; - if (retryCode === 'ELOOP' || retryCode === 'ENOTDIR' || retryCode === 'ENOENT') { - throw pathChanged('The approved filesystem target changed before execution.'); - } - throw retry; - } - } else { - throw error; - } - } + const handle = await openExistingNoTruncate(input.path); // The compare in compare-and-update, performed on the descriptor itself. const metadata = await handle.stat({ bigint: true }); if ( @@ -114,6 +83,12 @@ export async function openStableTarget(input: { } return handle; } + if (input.targetType !== undefined && input.targetType !== 'missing') { + // An existing target with no identity: the caller explicitly opted out of + // CAS (#3484). Open without truncation but perform no comparison — the + // caller's own absence of a T0 snapshot is the contract. + return openExistingNoTruncate(input.path); + } try { return await open(input.path, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL); } catch (error) { @@ -124,6 +99,45 @@ export async function openStableTarget(input: { } } +/** Open an existing target for read-write without truncating it. */ +async function openExistingNoTruncate(path: string): Promise { + const noFollow = process.platform === 'win32' ? 0 : constants.O_NOFOLLOW; + if (process.platform === 'win32') { + const entry = await lstat(path).catch(() => null); + if (entry?.isSymbolicLink()) { + throw pathChanged( + 'The approved filesystem target is a symbolic link; refusing to follow it.', + ); + } + } + try { + return await open(path, constants.O_RDWR | noFollow); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === 'ELOOP' || code === 'ENOTDIR' || code === 'ENOENT') { + throw pathChanged('The approved filesystem target changed before execution.'); + } + if (code === 'EACCES' || code === 'EPERM') { + // A write-only target (e.g. mode 0o222) refuses 'r+' but is still a + // legitimate mutation target — unlink semantics need no read + // permission. Retry write-only (still no truncate: identity is + // validated on the descriptor before writeThroughHandle truncates). + // The pinned read of the previous content will fail and the caller + // reports 'unknown' (no diff), which is the pre-existing behaviour. + try { + return await open(path, constants.O_WRONLY | noFollow); + } catch (retry) { + const retryCode = (retry as NodeJS.ErrnoException).code; + if (retryCode === 'ELOOP' || retryCode === 'ENOTDIR' || retryCode === 'ENOENT') { + throw pathChanged('The approved filesystem target changed before execution.'); + } + throw retry; + } + } + throw error; + } +} + /** * Write `content` through the pinned descriptor. The truncation happens here, * only after the identity was validated on the fd. Write-step failures diff --git a/packages/runtime/src/filesystem-executor.ts b/packages/runtime/src/filesystem-executor.ts index 2773626f7a..337adf9e0e 100644 --- a/packages/runtime/src/filesystem-executor.ts +++ b/packages/runtime/src/filesystem-executor.ts @@ -215,7 +215,16 @@ export function createBoundaryFilesystemExecutor( mode: call.permissionMode ?? 'ask', ...(input.permissionProfile ? { permissionProfile: input.permissionProfile } : {}), ...(call.abortSignal ? { abortSignal: call.abortSignal } : {}), - ...(expectedIdentity ? { expectedIdentity } : {}), + // The worker client now requires an explicit T0 marker (#3484): a + // mutation carries its captured identity, or 'missing' when T0 saw no + // target; a read never participates in CAS and says so. The kind check + // mirrors `mutates` for the backend input union. + expectedIdentity: + call.operation.kind === 'write' || + call.operation.kind === 'edit' || + call.operation.kind === 'format_json' + ? (expectedIdentity ?? 'missing') + : 'unchecked', }); if (result.kind === 'read_image') { return { diff --git a/packages/runtime/src/filesystem-worker/client.ts b/packages/runtime/src/filesystem-worker/client.ts index 3e3a23a6aa..c412e720f3 100644 --- a/packages/runtime/src/filesystem-worker/client.ts +++ b/packages/runtime/src/filesystem-worker/client.ts @@ -50,6 +50,26 @@ export interface FilesystemWorkerClientInput { platform?: SandboxPlatform; } +/** + * What the caller observed about the operation target at lock acquisition + * (T0). Required, so every caller must decide explicitly which CAS contract it + * is participating in — an absent field is no longer a silently accepted "no + * CAS" that a queue window can slip through (#3484). + * + * - `{ dev, ino }`: T0 observed an existing target; the worker compare-and- + * swaps this against the on-disk inode at T1. + * - `'missing'`: T0 observed no target (a create). If the target exists by T1, + * something created it while this call waited — writing would clobber + * content this call never saw, so the operation fails with `path_changed`. + * - `'unchecked'`: the caller does not participate in CAS (no T0 snapshot, + * e.g. a verification script or a read). Writes proceed without an identity + * check; use deliberately, never as a default for mutations. + */ +export type FilesystemWorkerExpectedIdentity = + | { readonly dev: string; readonly ino: string } + | 'missing' + | 'unchecked'; + export interface FilesystemWorkerExecuteInput { operation: FilesystemWorkerClientOperation; cwd: string; @@ -58,13 +78,8 @@ export interface FilesystemWorkerExecuteInput { /** Explicit embedding policy. Mode-based defaults are compiled only when omitted. */ permissionProfile?: PermissionProfile; abortSignal?: AbortSignal; - /** - * The target identity captured at lock acquisition (T0), passed in by the - * caller so the client does not re-derive it after acquiring the lock (T1). - * The worker compare-and-swaps this against the on-disk inode. Undefined for - * a missing target (create) or when the host-local backend is in use. - */ - expectedIdentity?: { dev: string; ino: string }; + /** Required: the caller's T0 observation, see `FilesystemWorkerExpectedIdentity`. */ + expectedIdentity: FilesystemWorkerExpectedIdentity; } export type FilesystemWorkerClientErrorReason = @@ -175,21 +190,25 @@ export class FilesystemWorkerClient { const access = operationAccess(parsedOperation.data.kind); const entryMode = operationUsesDirectoryEntry(parsedOperation.data); - const target: FilesystemWorkerTarget & { writableAncestor?: string } = await (entryMode - ? normalizeDirectoryEntryTarget({ - path: parsedOperation.data.path, - access, - cwd: canonicalCwd, - }) - : normalizeSandboxBoundaryPath({ - path: parsedOperation.data.path, - access, - scope: operationScope(parsedOperation.data.kind), - cwd: canonicalCwd, - }) - ).catch(() => { - throw clientError('invalid_operation', 'validation', requestId); - }); + // t0 is derived below from the caller's explicit expectedIdentity and + // added to the wire request; the normalised target itself has no T0 + // marker, so the declared type omits it. + const target: Omit & { writableAncestor?: string } = + await (entryMode + ? normalizeDirectoryEntryTarget({ + path: parsedOperation.data.path, + access, + cwd: canonicalCwd, + }) + : normalizeSandboxBoundaryPath({ + path: parsedOperation.data.path, + access, + scope: operationScope(parsedOperation.data.kind), + cwd: canonicalCwd, + }) + ).catch(() => { + throw clientError('invalid_operation', 'validation', requestId); + }); // The identity was captured by the caller at lock acquisition (T0) and // passed in as expectedIdentity. Do NOT re-derive it here: re-deriving at // this point (after the lock is held) would sample the post-queue inode, @@ -203,14 +222,18 @@ export class FilesystemWorkerClient { // mutation proceed as a fresh exclusive create ("delete then rewrite" // stays a clean apply; a rename-swap is NOT this case — it leaves an // existing inode and is caught by the identity comparison instead). - // - T0 missing (no identity) but T1 existing: the target was created while + // - T0 missing ('missing') but T1 existing: the target was created while // this call waited. Writing would clobber content this call never saw, // so fail with a meaningful path_changed (never invalid_request). + // - 'unchecked': the caller does not participate in CAS. The target may + // be present at T1 without this being a race — the caller simply has no + // T0 snapshot, so nothing can be compared (#3484). + const targetExistsAtT1 = target.targetType !== 'missing'; const identity = - input.expectedIdentity && target.targetType !== 'missing' + targetExistsAtT1 && typeof input.expectedIdentity === 'object' ? input.expectedIdentity : undefined; - if (!identity && target.targetType !== 'missing' && access === 'write') { + if (targetExistsAtT1 && access === 'write' && input.expectedIdentity === 'missing') { throw clientError( 'path_changed', 'validation', @@ -297,6 +320,15 @@ export class FilesystemWorkerClient { access, scope: target.scope, targetType: target.targetType, + // What T0 observed, carried explicitly so the worker can tell "created + // while queued" (missing) from "caller has no CAS snapshot" + // (unchecked) — the two were conflated on this wire and cost #3484. + t0: + input.expectedIdentity === 'unchecked' + ? 'unchecked' + : input.expectedIdentity === 'missing' + ? 'missing' + : 'existing', ...(identity ? { identity } : {}), }, } as const; @@ -641,7 +673,7 @@ async function normalizeDirectoryEntryTarget(input: { path: string; cwd: string; access: 'read' | 'write'; -}): Promise { +}): Promise & { writableAncestor?: string }> { const target = await resolveCanonicalDirectoryEntryTarget(input.cwd, input.path); let targetType: FilesystemWorkerTarget['targetType']; try { diff --git a/packages/runtime/src/filesystem-worker/operations.ts b/packages/runtime/src/filesystem-worker/operations.ts index 5dad9dd871..64313072e2 100644 --- a/packages/runtime/src/filesystem-worker/operations.ts +++ b/packages/runtime/src/filesystem-worker/operations.ts @@ -160,6 +160,7 @@ export async function executeFilesystemOperation( const handle = await openStableTarget({ path, approvedIdentity: expectedTarget?.identity, + targetType: expectedTarget?.targetType, }); try { // Read-before-write (for the diff): only through the pinned descriptor. @@ -226,6 +227,7 @@ export async function executeFilesystemOperation( const handle = await openStableTarget({ path, approvedIdentity: expectedTarget?.identity, + targetType: expectedTarget?.targetType, }); try { await readModifyWriteThroughHandle(handle, (existing) => @@ -249,6 +251,7 @@ export async function executeFilesystemOperation( const handle = await openStableTarget({ path, approvedIdentity: expectedTarget?.identity, + targetType: expectedTarget?.targetType, }); try { const content = await handle.readFile('utf8'); @@ -295,6 +298,7 @@ export async function executeFilesystemOperation( const handle = await openStableTarget({ path, approvedIdentity: expectedTarget?.identity, + targetType: expectedTarget?.targetType, }); try { const original = await handle.readFile('utf8'); @@ -485,16 +489,28 @@ async function assertTargetUnchanged( // while the call waited for the lock has a different inode even when its // canonical path and type still match. // - // A non-missing WRITE target MUST carry an identity — if it does not, the - // CAS is silently skipped and the entire defence collapses. Fail loudly - // rather than degrading to "no check", so a buggy caller that omits the - // identity is caught immediately instead of leaving the window open. Reads - // are exempt: they do not mutate, so there is no queue window to close. - if (access === 'write' && expected.targetType !== 'missing' && !expected.identity) { - throw operationError( - 'invalid_request', - 'A non-missing filesystem target must carry an identity for CAS.', - ); + // The T0 marker on the wire distinguishes the two ways a non-missing WRITE + // target can arrive without a concrete identity (#3484): + // - t0 'missing': T0 saw no target but T1 does — something created it while + // this call waited. Writing would clobber content the caller never saw. + // - t0 'existing' without identity: a buggy caller that captured a T0 but + // failed to send it — fail loudly rather than degrading to "no check", so + // the queue-window defence cannot silently collapse. + // - t0 'unchecked': the caller deliberately does not participate in CAS. + // Reads never mutate and are exempt either way. + if (access === 'write' && expected.targetType !== 'missing') { + if (expected.t0 === 'missing') { + throw operationError( + 'path_changed', + 'The target was created while this call waited for the lock; re-read before writing.', + ); + } + if (expected.t0 === 'existing' && !expected.identity) { + throw operationError( + 'invalid_request', + 'A non-missing filesystem target must carry an identity for CAS.', + ); + } } if (expected.identity) { const metadata = noFollowFinalSymlink diff --git a/packages/runtime/src/filesystem-worker/protocol.ts b/packages/runtime/src/filesystem-worker/protocol.ts index 00f09015cd..f6583968fe 100644 --- a/packages/runtime/src/filesystem-worker/protocol.ts +++ b/packages/runtime/src/filesystem-worker/protocol.ts @@ -6,7 +6,7 @@ import { validateSandboxBoundaryExpansion } from '@maka/core/sandbox-boundary'; // inode that was authorised at lock acquisition instead of only the path // string. The identity is carried as strings because bigint cannot cross the // JSON protocol boundary. -export const FILESYSTEM_WORKER_PROTOCOL_VERSION = 6 as const; +export const FILESYSTEM_WORKER_PROTOCOL_VERSION = 7 as const; const path = z.string().min(1).max(4096); const cwd = z.string().min(1).max(4096); @@ -53,11 +53,15 @@ export const FilesystemWorkerTargetSchema = z access: z.enum(['read', 'write']), scope: z.enum(['exact', 'subtree']), targetType: z.enum(['file', 'directory', 'symlink', 'other', 'missing']), - // Captured at lock acquisition (T0). Present (and required) for every - // targetType except 'missing'. The contract module (FilesystemTargetDescriptor) - // models this as a discriminated union; the wire schema keeps the field - // optional so it can omit it for missing targets, and the client that - // builds the request enforces the invariant. + // What the caller observed about the target at lock acquisition (T0): + // - 'existing' — an identity was captured and must be CAS'd at T1. + // - 'missing' — T0 saw no target; a T1-existing target was created while + // the call waited and must fail. + // - 'unchecked' — the caller does not participate in CAS (no T0 snapshot); + // the write proceeds without an identity comparison. + t0: z.enum(['existing', 'missing', 'unchecked']), + // The concrete T0 identity (dev/ino). Present exactly when t0 is + // 'existing'; the client building the request enforces the invariant. identity: FilesystemTargetIdentitySchema.optional(), }) .strict() @@ -68,6 +72,12 @@ export const FilesystemWorkerTargetSchema = z message: 'A missing target cannot carry an identity.', }); } + if (target.t0 !== 'existing' && target.identity !== undefined) { + context.addIssue({ + code: 'custom', + message: 'An identity requires t0 to be "existing".', + }); + } }); export const FilesystemWorkerOperationSchema = z.union([ diff --git a/scripts/verify-windows-sandbox-e2e.mjs b/scripts/verify-windows-sandbox-e2e.mjs index 73c3840354..43d09288a1 100644 --- a/scripts/verify-windows-sandbox-e2e.mjs +++ b/scripts/verify-windows-sandbox-e2e.mjs @@ -1,5 +1,5 @@ import { existsSync } from 'node:fs'; -import { mkdir, mkdtemp, readFile, realpath, rm, stat, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, realpath, rm, writeFile } from 'node:fs/promises'; import { homedir, tmpdir } from 'node:os'; import { basename, dirname, join, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; @@ -88,18 +88,18 @@ export async function verifyWindowsSandboxWorkerE2E(appDirectoryPath) { platform: 'win32', getLaunchSpec, }); - const execute = (operation, expectedIdentity) => - client.execute({ operation, cwd: workspace, mode: 'ask', expectedIdentity }); + // The sandbox preview verifies relay, not CAS: the script owns every path + // it writes and has no T0 snapshot to compare, so it opts out of the + // identity check explicitly (#3484) rather than being mistaken for a + // "target created while queued" race. + const execute = (operation) => + client.execute({ operation, cwd: workspace, mode: 'ask', expectedIdentity: 'unchecked' }); // Exact writes stay exact in the preview: the target is pre-seeded so the // grant covers only this file object, never its parent directory. const insidePath = join(workspace, 'inside.txt'); await writeFile(insidePath, 'seeded'); - const insideMetadata = await stat(insidePath, { bigint: true }); - await execute( - { kind: 'write', path: insidePath, content: 'packaged-relay-ok' }, - { dev: String(insideMetadata.dev), ino: String(insideMetadata.ino) }, - ); + await execute({ kind: 'write', path: insidePath, content: 'packaged-relay-ok' }); assertCondition( (await readFile(insidePath, 'utf8')) === 'packaged-relay-ok', 'Sandboxed write did not land in the workspace.', From 4f61ab5d2cc7dbc9f78f3ccbe90c72d05b8d9dbd Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 22 Aug 2026 20:03:29 +0800 Subject: [PATCH 2/7] fix(runtime): forward the apply_patch identity through the executor's worker branch The T0 marker computation in the executor aligned with `mutates` (write/edit/format_json) instead of the worker client's `operationAccess` (write/apply_patch/edit/format_json). apply_patch therefore fell into the 'unchecked' branch, silently dropping the identity captured by applyPatch() and disabling the queue-window CAS on the main editing channel (create/update/delete). Add apply_patch to the mutation list, point the comment at operationAccess as the authority, and cover the forwarding with an executor-level regression test. Generated-by: DSv4F-AstroHan --- .../filesystem-mutation-outcome.test.ts | 30 +++++++++++++++++++ packages/runtime/src/filesystem-executor.ts | 8 +++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/packages/runtime/src/__tests__/filesystem-mutation-outcome.test.ts b/packages/runtime/src/__tests__/filesystem-mutation-outcome.test.ts index def54c5721..e516c0e9a4 100644 --- a/packages/runtime/src/__tests__/filesystem-mutation-outcome.test.ts +++ b/packages/runtime/src/__tests__/filesystem-mutation-outcome.test.ts @@ -266,6 +266,36 @@ describe('filesystem mutation T0 identity capture (queue-window closure)', () => 'identity must be the inode captured at lock acquisition (before the replacement); a T1 capture would sample the replacement', ); }); + + test('an apply_patch mutation forwards its captured identity, not unchecked (#3484 regression)', async () => { + const cwd = await realpath(await mkdtemp(join(tmpdir(), 'maka-t0-applypatch-'))); + cleanup.push(cwd); + const target = join(cwd, 'file.txt'); + await writeFile(target, 'original', 'utf8'); + + const original = await stat(target, { bigint: true }); + let dispatched: FilesystemWorkerExpectedIdentity | undefined; + const gatedWorker: { + execute: (input: FilesystemWorkerExecuteInput) => Promise; + } = { + async execute(input) { + dispatched = input.expectedIdentity; + return { kind: 'apply_patch', ok: true, path: target }; + }, + }; + const fs = executorWith(gatedWorker); + + await fs.applyPatch({ + operation: { type: 'update_file', path: target, diff: '--- a\n+++ b\n' }, + cwd, + }); + + assert.ok( + dispatched && typeof dispatched !== 'string', + 'apply_patch must dispatch with the captured identity, not unchecked', + ); + assert.equal(dispatched.ino, String(original.ino)); + }); }); function sleep(ms: number): Promise { diff --git a/packages/runtime/src/filesystem-executor.ts b/packages/runtime/src/filesystem-executor.ts index 337adf9e0e..83004fc4b8 100644 --- a/packages/runtime/src/filesystem-executor.ts +++ b/packages/runtime/src/filesystem-executor.ts @@ -217,10 +217,14 @@ export function createBoundaryFilesystemExecutor( ...(call.abortSignal ? { abortSignal: call.abortSignal } : {}), // The worker client now requires an explicit T0 marker (#3484): a // mutation carries its captured identity, or 'missing' when T0 saw no - // target; a read never participates in CAS and says so. The kind check - // mirrors `mutates` for the backend input union. + // target; a read never participates in CAS and says so. The kind list + // deliberately mirrors `operationAccess` in the worker client + // (write | apply_patch | edit | format_json) — `mutates` is narrower + // and would silently drop the apply_patch identity onto 'unchecked', + // disabling the queue-window CAS on the main editing channel. expectedIdentity: call.operation.kind === 'write' || + call.operation.kind === 'apply_patch' || call.operation.kind === 'edit' || call.operation.kind === 'format_json' ? (expectedIdentity ?? 'missing') From 20dcbad2bc0feab38145546b012efa5e9b52818d Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 22 Aug 2026 20:08:25 +0800 Subject: [PATCH 3/7] refactor(runtime): collapse the wire identity contract to one required three-state field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (P2, Opus5 + Muse converged): the previous wire carried a required `t0` enum plus an optional `identity` plus two cross-field superRefines — two models for one concept, with the looser wire able to express illegal combinations that only runtime constraints rejected. The wire now carries a single required three-state `identity` ({dev,ino} | 'missing' | 'unchecked') mirroring the client input, so an illegal state cannot be expressed on the wire at all. The worker treats 'missing' with an existing target as created-while-queued (path_changed), CASes objects, and skips everything for 'unchecked'. The "buggy caller omitted identity" invalid_request guard disappears because the wire can no longer express that state. Generated-by: DSv4F-AstroHan --- .../filesystem-target-identity.test.ts | 25 ++++---- .../filesystem-worker-client.test.ts | 11 +++- .../src/__tests__/filesystem-worker.test.ts | 25 ++++---- .../runtime/src/filesystem-worker/client.ts | 28 ++++----- .../src/filesystem-worker/operations.ts | 61 +++++++++---------- .../runtime/src/filesystem-worker/protocol.ts | 27 +++----- 6 files changed, 84 insertions(+), 93 deletions(-) diff --git a/packages/runtime/src/__tests__/filesystem-target-identity.test.ts b/packages/runtime/src/__tests__/filesystem-target-identity.test.ts index 0240f9900c..fe270a3bb9 100644 --- a/packages/runtime/src/__tests__/filesystem-target-identity.test.ts +++ b/packages/runtime/src/__tests__/filesystem-target-identity.test.ts @@ -29,7 +29,8 @@ async function temporaryDirectory(prefix: string): Promise { function requestFor( operation: FilesystemWorkerRequest['operation'], - expectedTarget: Omit, + expectedTarget: Omit, + identity: FilesystemWorkerTarget['identity'] = 'missing', ): FilesystemWorkerRequest { return { version: FILESYSTEM_WORKER_PROTOCOL_VERSION, @@ -40,12 +41,7 @@ function requestFor( entries: [{ path: expectedTarget.enforcementPath, access: 'write', scope: 'exact' }], }, }, - expectedTarget: { - ...expectedTarget, - // These tests always CAS against a captured identity or an approved - // missing target; neither is an unchecked caller. - t0: expectedTarget.identity ? 'existing' : 'missing', - }, + expectedTarget: { ...expectedTarget, identity }, }; } @@ -72,7 +68,8 @@ describe('filesystem worker target identity CAS', () => { const response = await executeFilesystemWorkerRequest( requestFor( { kind: 'write', cwd, path: target, content: 'new' }, - { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file', identity }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' }, + identity, ), ); @@ -95,7 +92,8 @@ describe('filesystem worker target identity CAS', () => { const response = await executeFilesystemWorkerRequest( requestFor( { kind: 'write', cwd, path: target, content: 'updated' }, - { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file', identity }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' }, + identity, ), ); @@ -120,7 +118,8 @@ describe('filesystem worker target identity CAS', () => { const response = await executeFilesystemWorkerRequest( requestFor( { kind: 'apply_patch', cwd, path: target, action: 'delete' }, - { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file', identity }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' }, + identity, ), ); @@ -144,7 +143,8 @@ describe('filesystem worker target identity CAS', () => { const response = await executeFilesystemWorkerRequest( requestFor( { kind: 'edit', cwd, path: target, oldString: 'old', newString: 'new' }, - { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file', identity }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' }, + identity, ), ); @@ -191,7 +191,8 @@ describe('filesystem worker target identity CAS', () => { const response = await executeFilesystemWorkerRequest( requestFor( { kind: 'write', cwd, path: target, content: 'new' }, - { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file', identity }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' }, + identity, ), ); diff --git a/packages/runtime/src/__tests__/filesystem-worker-client.test.ts b/packages/runtime/src/__tests__/filesystem-worker-client.test.ts index 1f522e8a32..d0199767ec 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-client.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-client.test.ts @@ -86,8 +86,10 @@ describe('filesystem worker client permission snapshots', () => { assert.equal(expectedTarget?.targetType, 'symlink'); // The symlink entry's own identity (lstat, no follow) is captured at T0 and // forwarded; only its shape is stable, not its value. - assert.equal(typeof expectedTarget?.identity?.dev, 'string'); - assert.equal(typeof expectedTarget?.identity?.ino, 'string'); + assert.equal(typeof expectedTarget?.identity, 'object'); + const forwarded = expectedTarget?.identity as { dev: string; ino: string }; + assert.equal(typeof forwarded.dev, 'string'); + assert.equal(typeof forwarded.ino, 'string'); }); for (const kind of ['bypass', 'external'] as const) { @@ -741,7 +743,10 @@ describe('filesystem worker client dispatch classification', () => { }); assert.equal(requests[0]?.expectedTarget.targetType, 'missing'); - assert.equal(requests[0]?.expectedTarget.identity, undefined); + // The stale identity is never sent on a missing target; the wire's + // required identity contract reports 'missing' (nothing to compare), + // which lets the worker proceed as a fresh exclusive create. + assert.equal(requests[0]?.expectedTarget.identity, 'missing'); }); test('rejects a write whose target was created while queued (never invalid_request)', async () => { diff --git a/packages/runtime/src/__tests__/filesystem-worker.test.ts b/packages/runtime/src/__tests__/filesystem-worker.test.ts index 48b67972e7..c378e22373 100644 --- a/packages/runtime/src/__tests__/filesystem-worker.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker.test.ts @@ -543,9 +543,9 @@ describe('filesystem worker operations', () => { async function requestFor( operation: FilesystemWorkerOperation, - expectedTarget: Omit, + expectedTarget: Omit, permissionPath = operation.path, - t0?: FilesystemWorkerTarget['t0'], + identity?: FilesystemWorkerTarget['identity'], ): Promise { const operationBoundary: FilesystemWorkerRequest['operationBoundary'] = { filesystem: { @@ -562,17 +562,15 @@ async function requestFor( // the worker's mandatory-identity check is satisfied for non-missing targets. let resolvedTarget: FilesystemWorkerTarget = { ...expectedTarget, - // t0 is always replaced below; the placeholder keeps the type total. - t0: 'existing', + // Always replaced below; the placeholder keeps the type total. + identity: 'unchecked', }; - if (t0 !== undefined) { - // The test chose the T0 marker explicitly (e.g. 'unchecked'); keep the - // target exactly as given. - resolvedTarget = { ...expectedTarget, t0 }; + if (identity !== undefined) { + // The test chose the identity contract explicitly (e.g. 'unchecked' or a + // stale inode); keep the target exactly as given. + resolvedTarget = { ...expectedTarget, identity }; } else if (expectedTarget.targetType === 'missing') { - resolvedTarget = { ...expectedTarget, t0: 'missing' }; - } else if (expectedTarget.identity) { - resolvedTarget = { ...expectedTarget, t0: 'existing' }; + resolvedTarget = { ...expectedTarget, identity: 'missing' }; } else { const follow = expectedTarget.targetType !== 'symlink'; try { @@ -581,13 +579,12 @@ async function requestFor( : await lstat(expectedTarget.enforcementPath, { bigint: true }); resolvedTarget = { ...expectedTarget, - t0: 'existing', identity: { dev: String(metadata.dev), ino: String(metadata.ino) }, }; } catch { // Target may not exist at request construction time (the test sets it up - // differently); leave identity absent and let the worker surface it. - resolvedTarget = { ...expectedTarget, t0: 'existing' }; + // differently); fall back to 'missing' so the worker's own checks decide. + resolvedTarget = { ...expectedTarget, identity: 'missing' }; } } return { diff --git a/packages/runtime/src/filesystem-worker/client.ts b/packages/runtime/src/filesystem-worker/client.ts index c412e720f3..f3fd7738db 100644 --- a/packages/runtime/src/filesystem-worker/client.ts +++ b/packages/runtime/src/filesystem-worker/client.ts @@ -190,10 +190,10 @@ export class FilesystemWorkerClient { const access = operationAccess(parsedOperation.data.kind); const entryMode = operationUsesDirectoryEntry(parsedOperation.data); - // t0 is derived below from the caller's explicit expectedIdentity and - // added to the wire request; the normalised target itself has no T0 - // marker, so the declared type omits it. - const target: Omit & { writableAncestor?: string } = + // The wire identity contract is derived below from the caller's explicit + // expectedIdentity; the normalised target itself has no identity field, + // so the declared type omits it. + const target: Omit & { writableAncestor?: string } = await (entryMode ? normalizeDirectoryEntryTarget({ path: parsedOperation.data.path, @@ -320,16 +320,14 @@ export class FilesystemWorkerClient { access, scope: target.scope, targetType: target.targetType, - // What T0 observed, carried explicitly so the worker can tell "created - // while queued" (missing) from "caller has no CAS snapshot" - // (unchecked) — the two were conflated on this wire and cost #3484. - t0: - input.expectedIdentity === 'unchecked' - ? 'unchecked' - : input.expectedIdentity === 'missing' - ? 'missing' - : 'existing', - ...(identity ? { identity } : {}), + // The execution-time identity contract. A concrete identity is only + // carried when the target still exists at T1; a target that vanished + // while queued (or was never there) is 'missing', and a caller that + // does not participate in CAS says 'unchecked' (#3484). + identity: + typeof input.expectedIdentity === 'object' + ? (identity ?? 'missing') + : input.expectedIdentity, }, } as const; const requestJson = JSON.stringify(request); @@ -673,7 +671,7 @@ async function normalizeDirectoryEntryTarget(input: { path: string; cwd: string; access: 'read' | 'write'; -}): Promise & { writableAncestor?: string }> { +}): Promise & { writableAncestor?: string }> { const target = await resolveCanonicalDirectoryEntryTarget(input.cwd, input.path); let targetType: FilesystemWorkerTarget['targetType']; try { diff --git a/packages/runtime/src/filesystem-worker/operations.ts b/packages/runtime/src/filesystem-worker/operations.ts index 64313072e2..0495d7268b 100644 --- a/packages/runtime/src/filesystem-worker/operations.ts +++ b/packages/runtime/src/filesystem-worker/operations.ts @@ -159,7 +159,8 @@ export async function executeFilesystemOperation( // appeared in the gap is `path_changed`, never truncated. const handle = await openStableTarget({ path, - approvedIdentity: expectedTarget?.identity, + approvedIdentity: + typeof expectedTarget?.identity === 'object' ? expectedTarget.identity : undefined, targetType: expectedTarget?.targetType, }); try { @@ -211,7 +212,8 @@ export async function executeFilesystemOperation( else await compareAndDeleteEntry({ path, - approvedIdentity: expectedTarget?.identity, + approvedIdentity: + typeof expectedTarget?.identity === 'object' ? expectedTarget.identity : undefined, }); return { kind: 'apply_patch', ok: true, path }; } @@ -226,7 +228,8 @@ export async function executeFilesystemOperation( // rejected patch propagates before any truncation, so the file is intact. const handle = await openStableTarget({ path, - approvedIdentity: expectedTarget?.identity, + approvedIdentity: + typeof expectedTarget?.identity === 'object' ? expectedTarget.identity : undefined, targetType: expectedTarget?.targetType, }); try { @@ -250,7 +253,8 @@ export async function executeFilesystemOperation( ); const handle = await openStableTarget({ path, - approvedIdentity: expectedTarget?.identity, + approvedIdentity: + typeof expectedTarget?.identity === 'object' ? expectedTarget.identity : undefined, targetType: expectedTarget?.targetType, }); try { @@ -297,7 +301,8 @@ export async function executeFilesystemOperation( ); const handle = await openStableTarget({ path, - approvedIdentity: expectedTarget?.identity, + approvedIdentity: + typeof expectedTarget?.identity === 'object' ? expectedTarget.identity : undefined, targetType: expectedTarget?.targetType, }); try { @@ -489,42 +494,34 @@ async function assertTargetUnchanged( // while the call waited for the lock has a different inode even when its // canonical path and type still match. // - // The T0 marker on the wire distinguishes the two ways a non-missing WRITE - // target can arrive without a concrete identity (#3484): - // - t0 'missing': T0 saw no target but T1 does — something created it while + // The wire carries one required three-state identity contract (#3484): + // - { dev, ino }: CAS against the on-disk inode. + // - 'missing': T0 saw no target but T1 does — something created it while // this call waited. Writing would clobber content the caller never saw. - // - t0 'existing' without identity: a buggy caller that captured a T0 but - // failed to send it — fail loudly rather than degrading to "no check", so - // the queue-window defence cannot silently collapse. - // - t0 'unchecked': the caller deliberately does not participate in CAS. + // - 'unchecked': the caller deliberately does not participate in CAS. // Reads never mutate and are exempt either way. if (access === 'write' && expected.targetType !== 'missing') { - if (expected.t0 === 'missing') { + if (expected.identity === 'missing') { throw operationError( 'path_changed', 'The target was created while this call waited for the lock; re-read before writing.', ); } - if (expected.t0 === 'existing' && !expected.identity) { - throw operationError( - 'invalid_request', - 'A non-missing filesystem target must carry an identity for CAS.', - ); - } - } - if (expected.identity) { - const metadata = noFollowFinalSymlink - ? await fs.lstat(enforcementPath, { bigint: true }) - : await fs.stat(enforcementPath, { bigint: true }); - if ( - String(metadata.dev) !== expected.identity.dev || - String(metadata.ino) !== expected.identity.ino - ) { - throw operationError( - 'path_changed', - 'The approved filesystem target changed before execution.', - ); + if (typeof expected.identity === 'object') { + const metadata = noFollowFinalSymlink + ? await fs.lstat(enforcementPath, { bigint: true }) + : await fs.stat(enforcementPath, { bigint: true }); + if ( + String(metadata.dev) !== expected.identity.dev || + String(metadata.ino) !== expected.identity.ino + ) { + throw operationError( + 'path_changed', + 'The approved filesystem target changed before execution.', + ); + } } + // identity === 'unchecked': nothing to compare, nothing to fail. } } diff --git a/packages/runtime/src/filesystem-worker/protocol.ts b/packages/runtime/src/filesystem-worker/protocol.ts index f6583968fe..07fa986127 100644 --- a/packages/runtime/src/filesystem-worker/protocol.ts +++ b/packages/runtime/src/filesystem-worker/protocol.ts @@ -53,31 +53,24 @@ export const FilesystemWorkerTargetSchema = z access: z.enum(['read', 'write']), scope: z.enum(['exact', 'subtree']), targetType: z.enum(['file', 'directory', 'symlink', 'other', 'missing']), - // What the caller observed about the target at lock acquisition (T0): - // - 'existing' — an identity was captured and must be CAS'd at T1. - // - 'missing' — T0 saw no target; a T1-existing target was created while - // the call waited and must fail. - // - 'unchecked' — the caller does not participate in CAS (no T0 snapshot); - // the write proceeds without an identity comparison. - t0: z.enum(['existing', 'missing', 'unchecked']), - // The concrete T0 identity (dev/ino). Present exactly when t0 is - // 'existing'; the client building the request enforces the invariant. - identity: FilesystemTargetIdentitySchema.optional(), + // The execution-time identity contract, one required field (no separate + // T0 marker — a single three-state shape mirrors the client input, so an + // illegal combination cannot be expressed on the wire): + // - { dev, ino }: the T0 identity the worker must CAS against at T1. + // - 'missing': T0 saw no target; a target present at execution time was + // created while the call waited and must fail. + // - 'unchecked': the caller does not participate in CAS; the write + // proceeds without an identity comparison. + identity: FilesystemTargetIdentitySchema.or(z.literal('missing')).or(z.literal('unchecked')), }) .strict() .superRefine((target, context) => { - if (target.targetType === 'missing' && target.identity !== undefined) { + if (target.targetType === 'missing' && typeof target.identity === 'object') { context.addIssue({ code: 'custom', message: 'A missing target cannot carry an identity.', }); } - if (target.t0 !== 'existing' && target.identity !== undefined) { - context.addIssue({ - code: 'custom', - message: 'An identity requires t0 to be "existing".', - }); - } }); export const FilesystemWorkerOperationSchema = z.union([ From e405a340d8e452ea0e38a2c44d66c81d4b95f045 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 22 Aug 2026 20:12:00 +0800 Subject: [PATCH 4/7] fix(runtime): single-source the write-kind set and add apply_patch CAS behavior tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups (#3487, Opus5/Luna): - `operationAccess` (write | apply_patch | edit | format_json) now lives once in the shared protocol module; the client, the worker and the executor's T0-marker decision all call it, so the set cannot drift — the executor previously hand-copied a third list that silently dropped apply_patch onto 'unchecked'. - Worker-level behavior tests: an apply_patch update against a swapped inode fails path_changed and leaves the replacement content untouched (delete and edit already had such tests; update was the gap). - Format the smoke-test fixtures so the exact-head format check passes. Generated-by: DSv4F-AstroHan --- .../filesystem-target-identity.test.ts | 31 +++++++++++++++++++ .../filesystem-worker-linux-smoke.test.ts | 16 +++++----- .../__tests__/filesystem-worker-smoke.test.ts | 16 +++++----- .../filesystem-worker-windows-smoke.test.ts | 10 +++--- packages/runtime/src/filesystem-executor.ts | 16 +++++----- .../runtime/src/filesystem-worker/client.ts | 7 +---- .../src/filesystem-worker/operations.ts | 7 +---- .../runtime/src/filesystem-worker/protocol.ts | 9 ++++++ 8 files changed, 70 insertions(+), 42 deletions(-) diff --git a/packages/runtime/src/__tests__/filesystem-target-identity.test.ts b/packages/runtime/src/__tests__/filesystem-target-identity.test.ts index fe270a3bb9..a87bdd2dd0 100644 --- a/packages/runtime/src/__tests__/filesystem-target-identity.test.ts +++ b/packages/runtime/src/__tests__/filesystem-target-identity.test.ts @@ -155,6 +155,37 @@ describe('filesystem worker target identity CAS', () => { assert.equal(await readFile(target, 'utf8'), 'replacement\nold\n'); }); + test('rejects an apply_patch update when the target inode changed after authorisation', async () => { + const cwd = await temporaryDirectory('maka-identity-applypatch-update-'); + const target = join(cwd, 'file.txt'); + const replacement = join(cwd, 'replacement.txt'); + await writeFile(target, 'line\noriginal\n', 'utf8'); + await writeFile(replacement, 'line\nreplacement\n', 'utf8'); + + const identity = await captureIdentity(target); + await rename(replacement, target); + + const response = await executeFilesystemWorkerRequest( + requestFor( + { + kind: 'apply_patch', + cwd, + path: target, + action: 'update', + diff: '--- a\n+++ b\n@@ -1,2 +1,2 @@\n line\n-original\n+updated\n', + }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' }, + identity, + ), + ); + + assert.equal(response.ok, false); + assert.equal(response.error?.code, 'path_changed'); + // The replacement content must be untouched (the patch never applied). + const { readFile } = await import('node:fs/promises'); + assert.equal(await readFile(target, 'utf8'), 'line\nreplacement\n'); + }); + test('creates a missing target without requiring an identity', async () => { const cwd = await temporaryDirectory('maka-identity-missing-'); const target = join(cwd, 'brand-new.txt'); diff --git a/packages/runtime/src/__tests__/filesystem-worker-linux-smoke.test.ts b/packages/runtime/src/__tests__/filesystem-worker-linux-smoke.test.ts index 2bd5b5ec7f..1c90892c24 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-linux-smoke.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-linux-smoke.test.ts @@ -61,7 +61,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(await readFile(sourceFile, 'utf8'), 'export const healthSignal = true;\n'); @@ -69,7 +69,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { operation: { kind: 'read', path: sourceFile }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.deepEqual(read, { kind: 'read', @@ -103,7 +103,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.deepEqual(glob, { kind: 'glob', files: ['health.ts'] }); @@ -118,7 +118,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(grep.kind, 'grep'); if (grep.kind === 'grep') { @@ -139,7 +139,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(await readFile(target, 'utf8'), 'created'); @@ -155,7 +155,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { operation: { kind: 'write', path: allowedPath, content: 'blocked' }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }), isPathDenied, ); @@ -165,7 +165,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { cwd: workspace, mode: 'ask', executionBoundary, - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -184,7 +184,7 @@ describe('Linux filesystem worker smoke', { skip }, () => { cwd: workspace, mode: 'ask', executionBoundary, - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(await readFile(allowedPath, 'utf8'), 'outside-ok'); }); diff --git a/packages/runtime/src/__tests__/filesystem-worker-smoke.test.ts b/packages/runtime/src/__tests__/filesystem-worker-smoke.test.ts index 4709dfa397..7b3db0ae8a 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-smoke.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-smoke.test.ts @@ -46,7 +46,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' operation: { kind: 'write', path: insidePath, content: 'inside-ok' }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(await readFile(insidePath, 'utf8'), 'inside-ok'); @@ -55,7 +55,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' operation: { kind: 'write', path: outsidePath, content: 'blocked' }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }), (error: unknown) => error instanceof FilesystemWorkerClientError && error.reason === 'path_denied', @@ -74,7 +74,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(await readFile(target, 'utf8'), 'created'); @@ -110,7 +110,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' cwd: workspace, mode: 'ask', executionBoundary, - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }), (error: unknown) => { assert.ok(error instanceof FilesystemWorkerClientError); @@ -128,7 +128,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' cwd: workspace, mode: 'ask', executionBoundary, - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(await readFile(allowedPath, 'utf8'), 'outside-ok'); }); @@ -143,7 +143,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' operation: grepOperation(sourceFile, 'healthSignal'), cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(fileResult.kind, 'grep'); if (fileResult.kind === 'grep') { @@ -155,7 +155,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' operation: grepOperation(sourceDirectory, 'healthSignal'), cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(directoryResult.kind, 'grep'); if (directoryResult.kind === 'grep') { @@ -167,7 +167,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin' operation: grepOperation(sourceDirectory, 'does-not-exist'), cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.deepEqual(emptyResult, { kind: 'grep', matches: [] }); }); diff --git a/packages/runtime/src/__tests__/filesystem-worker-windows-smoke.test.ts b/packages/runtime/src/__tests__/filesystem-worker-windows-smoke.test.ts index 34596fb684..063aecea70 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-windows-smoke.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-windows-smoke.test.ts @@ -103,7 +103,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => { operation: { kind: 'read', path: target }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(read.kind, 'read'); if (read.kind === 'read') assert.match(read.content, /windows-relay-ok/); @@ -119,7 +119,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => { operation: { kind: 'write', path: missing, content: 'blocked' }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }), (error: unknown) => error instanceof FilesystemWorkerClientError && @@ -139,7 +139,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => { operation: { kind: 'glob', path: sourceDirectory, pattern: '**/*.ts' }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }); assert.equal(globResult.kind, 'glob'); if (globResult.kind === 'glob') { @@ -162,7 +162,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => { }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }), (error: unknown) => error instanceof FilesystemWorkerClientError && error.reason === 'grep_unavailable', @@ -179,7 +179,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => { }, cwd: workspace, mode: 'ask', - expectedIdentity: 'unchecked', + expectedIdentity: 'unchecked', }), (error: unknown) => error instanceof FilesystemWorkerClientError && error.reason === 'path_denied', diff --git a/packages/runtime/src/filesystem-executor.ts b/packages/runtime/src/filesystem-executor.ts index 83004fc4b8..27022e8371 100644 --- a/packages/runtime/src/filesystem-executor.ts +++ b/packages/runtime/src/filesystem-executor.ts @@ -31,6 +31,7 @@ import type { } from './filesystem-worker/client.js'; import { isSupportedImagePath, type ImageMimeType } from './image-file.js'; import type { FilesystemWorkerResult } from './filesystem-worker/protocol.js'; +import { operationAccess } from './filesystem-worker/protocol.js'; import { resolveCanonicalDirectoryEntryTarget } from './path-containment.js'; import { normalizeSandboxBoundaryPath } from './sandbox-boundary-path.js'; import { SandboxCommandError } from './sandbox/errors.js'; @@ -217,16 +218,13 @@ export function createBoundaryFilesystemExecutor( ...(call.abortSignal ? { abortSignal: call.abortSignal } : {}), // The worker client now requires an explicit T0 marker (#3484): a // mutation carries its captured identity, or 'missing' when T0 saw no - // target; a read never participates in CAS and says so. The kind list - // deliberately mirrors `operationAccess` in the worker client - // (write | apply_patch | edit | format_json) — `mutates` is narrower - // and would silently drop the apply_patch identity onto 'unchecked', - // disabling the queue-window CAS on the main editing channel. + // target; a read never participates in CAS and says so. `operationAccess` + // is the single authority on which kinds are writes (write | apply_patch + // | edit | format_json) — `mutates` is narrower and would silently drop + // the apply_patch identity onto 'unchecked', disabling the queue-window + // CAS on the main editing channel. expectedIdentity: - call.operation.kind === 'write' || - call.operation.kind === 'apply_patch' || - call.operation.kind === 'edit' || - call.operation.kind === 'format_json' + operationAccess(call.operation.kind) === 'write' ? (expectedIdentity ?? 'missing') : 'unchecked', }); diff --git a/packages/runtime/src/filesystem-worker/client.ts b/packages/runtime/src/filesystem-worker/client.ts index f3fd7738db..be0342cf1d 100644 --- a/packages/runtime/src/filesystem-worker/client.ts +++ b/packages/runtime/src/filesystem-worker/client.ts @@ -25,6 +25,7 @@ import { import { FILESYSTEM_WORKER_PROTOCOL_VERSION, FilesystemWorkerOperationSchema, + operationAccess, operationUsesDirectoryEntry, parseFilesystemWorkerResponse, type FilesystemWorkerErrorCode, @@ -652,12 +653,6 @@ function deriveWorkerProfile( }; } -function operationAccess(kind: FilesystemWorkerOperation['kind']): 'read' | 'write' { - return kind === 'write' || kind === 'apply_patch' || kind === 'edit' || kind === 'format_json' - ? 'write' - : 'read'; -} - function operationScope(kind: FilesystemWorkerOperation['kind']): 'exact' | 'subtree' | 'auto' { if (kind === 'glob') return 'subtree'; return kind === 'grep' ? 'auto' : 'exact'; diff --git a/packages/runtime/src/filesystem-worker/operations.ts b/packages/runtime/src/filesystem-worker/operations.ts index 0495d7268b..681a6107dc 100644 --- a/packages/runtime/src/filesystem-worker/operations.ts +++ b/packages/runtime/src/filesystem-worker/operations.ts @@ -24,6 +24,7 @@ import { import { isSupportedImagePath, readWorkspaceImage } from '../image-file.js'; import { FILESYSTEM_WORKER_PROTOCOL_VERSION, + operationAccess, operationUsesDirectoryEntry, type FilesystemWorkerErrorCode, type FilesystemWorkerOperation, @@ -67,12 +68,6 @@ export type FilesystemWorkerGrepRunner = ( input: FilesystemWorkerGrepRunInput, ) => Promise; -function operationAccess(kind: FilesystemWorkerOperation['kind']): 'read' | 'write' { - return kind === 'write' || kind === 'apply_patch' || kind === 'edit' || kind === 'format_json' - ? 'write' - : 'read'; -} - export async function executeFilesystemWorkerRequest( request: FilesystemWorkerRequest, dependencies: FilesystemWorkerOperationDependencies = {}, diff --git a/packages/runtime/src/filesystem-worker/protocol.ts b/packages/runtime/src/filesystem-worker/protocol.ts index 07fa986127..8f40a097c9 100644 --- a/packages/runtime/src/filesystem-worker/protocol.ts +++ b/packages/runtime/src/filesystem-worker/protocol.ts @@ -8,6 +8,15 @@ import { validateSandboxBoundaryExpansion } from '@maka/core/sandbox-boundary'; // JSON protocol boundary. export const FILESYSTEM_WORKER_PROTOCOL_VERSION = 7 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' + ? 'write' + : 'read'; +} + const path = z.string().min(1).max(4096); const cwd = z.string().min(1).max(4096); From 89db39fbf03c77ccb6ab5df55b954db05bc77942 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 22 Aug 2026 20:18:59 +0800 Subject: [PATCH 5/7] fix(release): keep the sandbox e2e participating in CAS after rebase The rebase conflict on verify-windows-sandbox-e2e.mjs was resolved with the branch's own 'unchecked' variant, which would have reverted the #3457 hardening (the script now captures a real T0 identity via stat and passes it). Restore the main version: the script verifies relay AND the packaged CAS path; 'unchecked' would silently skip the identity comparison on the release lane. Generated-by: DSv4F-AstroHan --- scripts/verify-windows-sandbox-e2e.mjs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/verify-windows-sandbox-e2e.mjs b/scripts/verify-windows-sandbox-e2e.mjs index 43d09288a1..73c3840354 100644 --- a/scripts/verify-windows-sandbox-e2e.mjs +++ b/scripts/verify-windows-sandbox-e2e.mjs @@ -1,5 +1,5 @@ import { existsSync } from 'node:fs'; -import { mkdir, mkdtemp, readFile, realpath, rm, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, readFile, realpath, rm, stat, writeFile } from 'node:fs/promises'; import { homedir, tmpdir } from 'node:os'; import { basename, dirname, join, resolve } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; @@ -88,18 +88,18 @@ export async function verifyWindowsSandboxWorkerE2E(appDirectoryPath) { platform: 'win32', getLaunchSpec, }); - // The sandbox preview verifies relay, not CAS: the script owns every path - // it writes and has no T0 snapshot to compare, so it opts out of the - // identity check explicitly (#3484) rather than being mistaken for a - // "target created while queued" race. - const execute = (operation) => - client.execute({ operation, cwd: workspace, mode: 'ask', expectedIdentity: 'unchecked' }); + const execute = (operation, expectedIdentity) => + client.execute({ operation, cwd: workspace, mode: 'ask', expectedIdentity }); // Exact writes stay exact in the preview: the target is pre-seeded so the // grant covers only this file object, never its parent directory. const insidePath = join(workspace, 'inside.txt'); await writeFile(insidePath, 'seeded'); - await execute({ kind: 'write', path: insidePath, content: 'packaged-relay-ok' }); + const insideMetadata = await stat(insidePath, { bigint: true }); + await execute( + { kind: 'write', path: insidePath, content: 'packaged-relay-ok' }, + { dev: String(insideMetadata.dev), ino: String(insideMetadata.ino) }, + ); assertCondition( (await readFile(insidePath, 'utf8')) === 'packaged-relay-ok', 'Sandboxed write did not land in the workspace.', From fa4397aae35fbb87fad7008c02c51d7ca1aa3184 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 22 Aug 2026 20:59:48 +0800 Subject: [PATCH 6/7] fix(runtime): close the maintainer-review gaps on the identity contract (#3487) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from the external maintainer review: 1. JavaScript callers were not migrated and the required-field argument gave them no protection. The client now auto-generates the wire 'unchecked' identity for reads (callers cannot get reads wrong, even from plain .mjs that bypass TypeScript) and enforces at runtime that write operations carry an explicit identity contract — a JS caller that omits it fails loudly instead of silently skipping the queue-window CAS. verify-macos-arm64-dmg.mjs's raw v7 write request gains the required identity: 'missing'. 2. Creation diff was lost: the truthy 'missing' string made the old `!expectedTarget?.identity` test fail, collapsing new-file writes into unknown and hiding the `--- /dev/null` diff. The worker now derives "approved missing" from targetType, with a behavior test asserting a missing-target write reports a creation diff. 3. mutates() removed; the executor's mutation gate and the T0-marker decision now use the single operationAccess authority. Verification: runtime+runtime-host typecheck clean; focused suites 71/71; full @maka/runtime test:dist 3080 pass / 0 fail; format:check clean; request shapes for both verifier scripts validated against the worker's schema (write/'missing' and read/'unchecked' parse; the pre-fix no-identity shape is rejected). The packaged Release Windows verifier and the macOS DMG verifier cannot run on this machine (no Windows runner, no packaged artifacts); their request construction is covered by the schema checks above. Generated-by: DSv4F-AstroHan --- .../filesystem-target-identity.test.ts | 26 ++++++ .../filesystem-worker-client.test.ts | 87 +++++++++++++++++++ packages/runtime/src/filesystem-executor.ts | 13 ++- .../runtime/src/filesystem-worker/client.ts | 42 ++++++--- .../src/filesystem-worker/operations.ts | 5 +- scripts/verify-macos-arm64-dmg.mjs | 3 + 6 files changed, 156 insertions(+), 20 deletions(-) diff --git a/packages/runtime/src/__tests__/filesystem-target-identity.test.ts b/packages/runtime/src/__tests__/filesystem-target-identity.test.ts index a87bdd2dd0..80435e3ec6 100644 --- a/packages/runtime/src/__tests__/filesystem-target-identity.test.ts +++ b/packages/runtime/src/__tests__/filesystem-target-identity.test.ts @@ -206,6 +206,32 @@ describe('filesystem worker target identity CAS', () => { assert.equal(await readFile(target, 'utf8'), 'created'); }); + test('a write to a missing target reports a creation diff (#3487 P1)', async () => { + const cwd = await temporaryDirectory('maka-identity-missing-write-'); + const target = join(cwd, 'brand-new.txt'); + + // The truthy 'missing' identity string used to make the "approved + // missing" truthiness test fail, collapsing the diff into unknown and + // hiding new-file changes from review. The write must report the creation + // diff (`--- /dev/null`) instead. + const response = await executeFilesystemWorkerRequest( + requestFor( + { kind: 'write', cwd, path: target, content: 'created\n' }, + { enforcementPath: target, access: 'write', scope: 'exact', targetType: 'missing' }, + ), + ); + + assert.equal(response.ok, true); + assert.equal(response.result.kind, 'write'); + if (response.result.kind !== 'write') return; + assert.ok( + response.result.diff?.includes('--- /dev/null'), + 'a created file must carry a creation diff with --- /dev/null', + ); + const { readFile } = await import('node:fs/promises'); + assert.equal(await readFile(target, 'utf8'), 'created\n'); + }); + test('reports path_changed when the target is replaced before the write (pre-write CAS)', async () => { const cwd = await temporaryDirectory('maka-identity-prewrite-'); const target = join(cwd, 'file.txt'); diff --git a/packages/runtime/src/__tests__/filesystem-worker-client.test.ts b/packages/runtime/src/__tests__/filesystem-worker-client.test.ts index d0199767ec..8af583dbaa 100644 --- a/packages/runtime/src/__tests__/filesystem-worker-client.test.ts +++ b/packages/runtime/src/__tests__/filesystem-worker-client.test.ts @@ -793,6 +793,93 @@ describe('filesystem worker client dispatch classification', () => { // The interloper's content was never touched. assert.equal(await readFile(target, 'utf8'), 'external-content'); }); + + test('lets an unchecked caller write an existing target without a CAS identity (#3484)', async () => { + const workspace = await temporaryDirectory('maka-client-unchecked-'); + const target = join(workspace, 'file.txt'); + // The target already exists; the caller has no T0 snapshot to compare + // (e.g. a verification script that owns the path itself). + await writeFile(target, 'existing', 'utf8'); + + const { client, requests } = fakeClient(); + await client.execute({ + operation: { kind: 'write', path: target, content: 'new' }, + cwd: workspace, + mode: 'ask', + expectedIdentity: 'unchecked', + }); + + const expectedTarget = requests[0]?.expectedTarget; + assert.equal(expectedTarget?.targetType, 'file'); + assert.equal(expectedTarget?.identity, 'unchecked'); + }); + + test('marks a T0-missing create as missing on the wire, not unchecked (#3484)', async () => { + const workspace = await temporaryDirectory('maka-client-t0missing-'); + const target = join(workspace, 'new.txt'); + + const { client, requests } = fakeClient(); + await client.execute({ + operation: { kind: 'write', path: target, content: 'new' }, + cwd: workspace, + mode: 'ask', + expectedIdentity: 'missing', + }); + + const expectedTarget = requests[0]?.expectedTarget; + assert.equal(expectedTarget?.targetType, 'missing'); + assert.equal(expectedTarget?.identity, 'missing'); + }); + + test('a read without expectedIdentity sends unchecked automatically (#3487)', async () => { + const workspace = await temporaryDirectory('maka-client-read-auto-'); + const target = join(workspace, 'file.txt'); + await writeFile(target, 'content', 'utf8'); + + const { client, requests } = fakeClient(); + await client.execute({ + operation: { kind: 'read', path: target }, + cwd: workspace, + mode: 'ask', + // No expectedIdentity: reads never participate in CAS, and the client + // must not reject or silently omit the wire field — a plain-JavaScript + // caller that bypasses TypeScript has no way to get this wrong. + }); + + assert.equal(requests[0]?.expectedTarget.identity, 'unchecked'); + }); + + test('a write without expectedIdentity is rejected at runtime (#3487)', async () => { + const workspace = await temporaryDirectory('maka-client-write-required-'); + const target = join(workspace, 'file.txt'); + await writeFile(target, 'existing', 'utf8'); + + const client = new FilesystemWorkerClient({ + sandboxManager: new SandboxManager([new MacosSeatbeltBackend()]), + platform: 'darwin', + getLaunchSpec: async () => ({ + ok: false, + reason: 'worker_bundle_unavailable', + message: 'unused', + }), + runProcess: async () => { + throw new Error('must not dispatch'); + }, + }); + + await assert.rejects( + client.execute({ + operation: { kind: 'write', path: target, content: 'new' }, + cwd: workspace, + mode: 'ask', + }), + (error: unknown) => { + assert.ok(error instanceof FilesystemWorkerClientError); + assert.equal(error.reason, 'invalid_request'); + return true; + }, + ); + }); }); function isPathDenied(error: unknown): boolean { diff --git a/packages/runtime/src/filesystem-executor.ts b/packages/runtime/src/filesystem-executor.ts index 27022e8371..2d0bce408d 100644 --- a/packages/runtime/src/filesystem-executor.ts +++ b/packages/runtime/src/filesystem-executor.ts @@ -122,12 +122,11 @@ function pathScopeForBoundary(boundary: ExecutionBoundary | undefined): Workspac return boundary?.kind === 'bypass' ? 'host' : 'workspace'; } -/** Operations that read, modify and write back, and so must hold the target's lock. */ -function mutates(operation: FilesystemOperation): boolean { - return ( - operation.kind === 'write' || operation.kind === 'edit' || operation.kind === 'format_json' - ); -} +/** + * Operations that read, modify and write back, and so must hold the target's lock. + * The single authority on which kinds are writes is `operationAccess` in the + * worker protocol; `mutates` was a second, narrower list that drifted. + */ /** * Capture the target's stable identity at lock acquisition (T0) — *before* @@ -267,7 +266,7 @@ export function createBoundaryFilesystemExecutor( } return { async execute(call) { - if (!mutates(call.operation)) return await run(call); + if (operationAccess(call.operation.kind) !== 'write') return await run(call); // Canonicalisation without any containment check, so a target the policy // goes on to reject still takes the same lock as its other spellings. The // key is derived from the same canonicalisation the backend will resolve diff --git a/packages/runtime/src/filesystem-worker/client.ts b/packages/runtime/src/filesystem-worker/client.ts index be0342cf1d..ad1788b543 100644 --- a/packages/runtime/src/filesystem-worker/client.ts +++ b/packages/runtime/src/filesystem-worker/client.ts @@ -79,8 +79,16 @@ export interface FilesystemWorkerExecuteInput { /** Explicit embedding policy. Mode-based defaults are compiled only when omitted. */ permissionProfile?: PermissionProfile; abortSignal?: AbortSignal; - /** Required: the caller's T0 observation, see `FilesystemWorkerExpectedIdentity`. */ - expectedIdentity: FilesystemWorkerExpectedIdentity; + /** + * The caller's T0 observation, see `FilesystemWorkerExpectedIdentity`. + * + * REQUIRED for write operations: the client throws at runtime when a write + * arrives without one, so a JavaScript caller (which TypeScript cannot + * guard) fails loudly instead of silently skipping the queue-window CAS. + * Reads never participate in CAS: the client sends 'unchecked' for them + * automatically, so read callers have no way to get this wrong. + */ + expectedIdentity?: FilesystemWorkerExpectedIdentity; } export type FilesystemWorkerClientErrorReason = @@ -190,6 +198,21 @@ export class FilesystemWorkerClient { if (!parsedOperation.success) throw clientError('invalid_operation', 'validation', requestId); const access = operationAccess(parsedOperation.data.kind); + // Reads never participate in CAS: the client sends 'unchecked' for them + // automatically, so read callers (including plain-JavaScript verifiers + // that bypass TypeScript) have no way to get the identity wrong. + // Writes require an explicit T0 state, enforced at runtime: a caller that + // omits it fails loudly here instead of silently skipping the + // queue-window CAS (#3487, maintainer review). + const writeIdentity = access === 'write' ? input.expectedIdentity : 'unchecked'; + if (access === 'write' && writeIdentity === undefined) { + throw clientError( + 'invalid_request', + 'validation', + requestId, + 'A write operation requires an explicit expectedIdentity: {dev, ino}, "missing", or "unchecked".', + ); + } const entryMode = operationUsesDirectoryEntry(parsedOperation.data); // The wire identity contract is derived below from the caller's explicit // expectedIdentity; the normalised target itself has no identity field, @@ -231,10 +254,8 @@ export class FilesystemWorkerClient { // T0 snapshot, so nothing can be compared (#3484). const targetExistsAtT1 = target.targetType !== 'missing'; const identity = - targetExistsAtT1 && typeof input.expectedIdentity === 'object' - ? input.expectedIdentity - : undefined; - if (targetExistsAtT1 && access === 'write' && input.expectedIdentity === 'missing') { + targetExistsAtT1 && typeof writeIdentity === 'object' ? writeIdentity : undefined; + if (targetExistsAtT1 && access === 'write' && writeIdentity === 'missing') { throw clientError( 'path_changed', 'validation', @@ -323,12 +344,9 @@ export class FilesystemWorkerClient { targetType: target.targetType, // The execution-time identity contract. A concrete identity is only // carried when the target still exists at T1; a target that vanished - // while queued (or was never there) is 'missing', and a caller that - // does not participate in CAS says 'unchecked' (#3484). - identity: - typeof input.expectedIdentity === 'object' - ? (identity ?? 'missing') - : input.expectedIdentity, + // while queued (or was never there) is 'missing'; reads always say + // 'unchecked' (the client generates it, callers cannot get it wrong). + identity: typeof writeIdentity === 'object' ? (identity ?? 'missing') : writeIdentity, }, } as const; const requestJson = JSON.stringify(request); diff --git a/packages/runtime/src/filesystem-worker/operations.ts b/packages/runtime/src/filesystem-worker/operations.ts index 681a6107dc..ca59f41ec8 100644 --- a/packages/runtime/src/filesystem-worker/operations.ts +++ b/packages/runtime/src/filesystem-worker/operations.ts @@ -161,8 +161,11 @@ export async function executeFilesystemOperation( try { // Read-before-write (for the diff): only through the pinned descriptor. // An approved-missing target was just created by 'wx', so it is new. + // The wire identity is three-state (#3484): 'missing' is a truthy + // string, so a truthiness test can no longer stand in for "the target + // was approved as missing" — targetType is the authority here. let previous: 'new' | 'unknown' | string; - if (!expectedTarget?.identity) { + if (expectedTarget?.targetType === 'missing') { previous = 'new'; } else { try { diff --git a/scripts/verify-macos-arm64-dmg.mjs b/scripts/verify-macos-arm64-dmg.mjs index c9635e13d8..11cf2ae5bb 100644 --- a/scripts/verify-macos-arm64-dmg.mjs +++ b/scripts/verify-macos-arm64-dmg.mjs @@ -56,6 +56,9 @@ export async function smokePackagedFilesystemWorker( access: 'write', scope: 'exact', targetType: 'missing', + // Protocol v7 requires the three-state identity; a write to a target + // approved as missing carries 'missing' (#3484 / #3487). + identity: 'missing', }, }; From d9831155c1b953f7fface42ba4b5dbd5b78738db Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 22 Aug 2026 21:05:31 +0800 Subject: [PATCH 7/7] fix(release): give the sandbox e2e negative cases a truthful T0 identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The client now rejects a write without an explicit expectedIdentity at runtime (#3487). Two intentionally-failing cases in verify-windows-sandbox-e2e.mjs wrote without one, so they were rejected by the parameter validation with invalid_request instead of reaching the sandbox path they were meant to exercise (parent-entry fail-closed, and the workspace-boundary path_denied). Pass 'missing' — the truthful T0 state for targets that do not exist — so the intended checks run again. Generated-by: DSv4F-AstroHan --- scripts/verify-windows-sandbox-e2e.mjs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/verify-windows-sandbox-e2e.mjs b/scripts/verify-windows-sandbox-e2e.mjs index 73c3840354..c10f01ad3a 100644 --- a/scripts/verify-windows-sandbox-e2e.mjs +++ b/scripts/verify-windows-sandbox-e2e.mjs @@ -110,7 +110,13 @@ export async function verifyWindowsSandboxWorkerE2E(appDirectoryPath) { // any launch. let parentEntryDenied = false; try { - await execute({ kind: 'write', path: join(workspace, 'missing.txt'), content: 'x' }); + // The target is genuinely missing; 'missing' is the truthful T0 state + // so the client lets the request through and the sandbox's own + // fail-closed parent-entry check is what rejects it (#3487). + await execute( + { kind: 'write', path: join(workspace, 'missing.txt'), content: 'x' }, + 'missing', + ); } catch (error) { parentEntryDenied = error instanceof FilesystemWorkerClientError && @@ -161,7 +167,12 @@ export async function verifyWindowsSandboxWorkerE2E(appDirectoryPath) { let denied = false; try { - await execute({ kind: 'write', path: join(outside, 'blocked.txt'), content: 'blocked' }); + // Same contract as above: a truthful T0 state so the client's own + // permission gate (not the identity validation) is what denies. + await execute( + { kind: 'write', path: join(outside, 'blocked.txt'), content: 'blocked' }, + 'missing', + ); } catch (error) { denied = error instanceof FilesystemWorkerClientError && error.reason === 'path_denied'; }