Skip to content

Commit e405a34

Browse files
committed
fix(runtime): single-source the write-kind set and add apply_patch CAS behavior tests
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
1 parent 20dcbad commit e405a34

8 files changed

Lines changed: 70 additions & 42 deletions

packages/runtime/src/__tests__/filesystem-target-identity.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,37 @@ describe('filesystem worker target identity CAS', () => {
155155
assert.equal(await readFile(target, 'utf8'), 'replacement\nold\n');
156156
});
157157

158+
test('rejects an apply_patch update when the target inode changed after authorisation', async () => {
159+
const cwd = await temporaryDirectory('maka-identity-applypatch-update-');
160+
const target = join(cwd, 'file.txt');
161+
const replacement = join(cwd, 'replacement.txt');
162+
await writeFile(target, 'line\noriginal\n', 'utf8');
163+
await writeFile(replacement, 'line\nreplacement\n', 'utf8');
164+
165+
const identity = await captureIdentity(target);
166+
await rename(replacement, target);
167+
168+
const response = await executeFilesystemWorkerRequest(
169+
requestFor(
170+
{
171+
kind: 'apply_patch',
172+
cwd,
173+
path: target,
174+
action: 'update',
175+
diff: '--- a\n+++ b\n@@ -1,2 +1,2 @@\n line\n-original\n+updated\n',
176+
},
177+
{ enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' },
178+
identity,
179+
),
180+
);
181+
182+
assert.equal(response.ok, false);
183+
assert.equal(response.error?.code, 'path_changed');
184+
// The replacement content must be untouched (the patch never applied).
185+
const { readFile } = await import('node:fs/promises');
186+
assert.equal(await readFile(target, 'utf8'), 'line\nreplacement\n');
187+
});
188+
158189
test('creates a missing target without requiring an identity', async () => {
159190
const cwd = await temporaryDirectory('maka-identity-missing-');
160191
const target = join(cwd, 'brand-new.txt');

packages/runtime/src/__tests__/filesystem-worker-linux-smoke.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,15 @@ describe('Linux filesystem worker smoke', { skip }, () => {
6161
},
6262
cwd: workspace,
6363
mode: 'ask',
64-
expectedIdentity: 'unchecked',
64+
expectedIdentity: 'unchecked',
6565
});
6666
assert.equal(await readFile(sourceFile, 'utf8'), 'export const healthSignal = true;\n');
6767

6868
const read = await client.execute({
6969
operation: { kind: 'read', path: sourceFile },
7070
cwd: workspace,
7171
mode: 'ask',
72-
expectedIdentity: 'unchecked',
72+
expectedIdentity: 'unchecked',
7373
});
7474
assert.deepEqual(read, {
7575
kind: 'read',
@@ -103,7 +103,7 @@ describe('Linux filesystem worker smoke', { skip }, () => {
103103
},
104104
cwd: workspace,
105105
mode: 'ask',
106-
expectedIdentity: 'unchecked',
106+
expectedIdentity: 'unchecked',
107107
});
108108
assert.deepEqual(glob, { kind: 'glob', files: ['health.ts'] });
109109

@@ -118,7 +118,7 @@ describe('Linux filesystem worker smoke', { skip }, () => {
118118
},
119119
cwd: workspace,
120120
mode: 'ask',
121-
expectedIdentity: 'unchecked',
121+
expectedIdentity: 'unchecked',
122122
});
123123
assert.equal(grep.kind, 'grep');
124124
if (grep.kind === 'grep') {
@@ -139,7 +139,7 @@ describe('Linux filesystem worker smoke', { skip }, () => {
139139
},
140140
cwd: workspace,
141141
mode: 'ask',
142-
expectedIdentity: 'unchecked',
142+
expectedIdentity: 'unchecked',
143143
});
144144

145145
assert.equal(await readFile(target, 'utf8'), 'created');
@@ -155,7 +155,7 @@ describe('Linux filesystem worker smoke', { skip }, () => {
155155
operation: { kind: 'write', path: allowedPath, content: 'blocked' },
156156
cwd: workspace,
157157
mode: 'ask',
158-
expectedIdentity: 'unchecked',
158+
expectedIdentity: 'unchecked',
159159
}),
160160
isPathDenied,
161161
);
@@ -165,7 +165,7 @@ describe('Linux filesystem worker smoke', { skip }, () => {
165165
cwd: workspace,
166166
mode: 'ask',
167167
executionBoundary,
168-
expectedIdentity: 'unchecked',
168+
expectedIdentity: 'unchecked',
169169
}),
170170
(error: unknown) => {
171171
assert.ok(error instanceof FilesystemWorkerClientError);
@@ -184,7 +184,7 @@ describe('Linux filesystem worker smoke', { skip }, () => {
184184
cwd: workspace,
185185
mode: 'ask',
186186
executionBoundary,
187-
expectedIdentity: 'unchecked',
187+
expectedIdentity: 'unchecked',
188188
});
189189
assert.equal(await readFile(allowedPath, 'utf8'), 'outside-ok');
190190
});

packages/runtime/src/__tests__/filesystem-worker-smoke.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin'
4646
operation: { kind: 'write', path: insidePath, content: 'inside-ok' },
4747
cwd: workspace,
4848
mode: 'ask',
49-
expectedIdentity: 'unchecked',
49+
expectedIdentity: 'unchecked',
5050
});
5151
assert.equal(await readFile(insidePath, 'utf8'), 'inside-ok');
5252

@@ -55,7 +55,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin'
5555
operation: { kind: 'write', path: outsidePath, content: 'blocked' },
5656
cwd: workspace,
5757
mode: 'ask',
58-
expectedIdentity: 'unchecked',
58+
expectedIdentity: 'unchecked',
5959
}),
6060
(error: unknown) =>
6161
error instanceof FilesystemWorkerClientError && error.reason === 'path_denied',
@@ -74,7 +74,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin'
7474
},
7575
cwd: workspace,
7676
mode: 'ask',
77-
expectedIdentity: 'unchecked',
77+
expectedIdentity: 'unchecked',
7878
});
7979

8080
assert.equal(await readFile(target, 'utf8'), 'created');
@@ -110,7 +110,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin'
110110
cwd: workspace,
111111
mode: 'ask',
112112
executionBoundary,
113-
expectedIdentity: 'unchecked',
113+
expectedIdentity: 'unchecked',
114114
}),
115115
(error: unknown) => {
116116
assert.ok(error instanceof FilesystemWorkerClientError);
@@ -128,7 +128,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin'
128128
cwd: workspace,
129129
mode: 'ask',
130130
executionBoundary,
131-
expectedIdentity: 'unchecked',
131+
expectedIdentity: 'unchecked',
132132
});
133133
assert.equal(await readFile(allowedPath, 'utf8'), 'outside-ok');
134134
});
@@ -143,7 +143,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin'
143143
operation: grepOperation(sourceFile, 'healthSignal'),
144144
cwd: workspace,
145145
mode: 'ask',
146-
expectedIdentity: 'unchecked',
146+
expectedIdentity: 'unchecked',
147147
});
148148
assert.equal(fileResult.kind, 'grep');
149149
if (fileResult.kind === 'grep') {
@@ -155,7 +155,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin'
155155
operation: grepOperation(sourceDirectory, 'healthSignal'),
156156
cwd: workspace,
157157
mode: 'ask',
158-
expectedIdentity: 'unchecked',
158+
expectedIdentity: 'unchecked',
159159
});
160160
assert.equal(directoryResult.kind, 'grep');
161161
if (directoryResult.kind === 'grep') {
@@ -167,7 +167,7 @@ describe('macOS filesystem worker smoke', { skip: process.platform !== 'darwin'
167167
operation: grepOperation(sourceDirectory, 'does-not-exist'),
168168
cwd: workspace,
169169
mode: 'ask',
170-
expectedIdentity: 'unchecked',
170+
expectedIdentity: 'unchecked',
171171
});
172172
assert.deepEqual(emptyResult, { kind: 'grep', matches: [] });
173173
});

packages/runtime/src/__tests__/filesystem-worker-windows-smoke.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => {
103103
operation: { kind: 'read', path: target },
104104
cwd: workspace,
105105
mode: 'ask',
106-
expectedIdentity: 'unchecked',
106+
expectedIdentity: 'unchecked',
107107
});
108108
assert.equal(read.kind, 'read');
109109
if (read.kind === 'read') assert.match(read.content, /windows-relay-ok/);
@@ -119,7 +119,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => {
119119
operation: { kind: 'write', path: missing, content: 'blocked' },
120120
cwd: workspace,
121121
mode: 'ask',
122-
expectedIdentity: 'unchecked',
122+
expectedIdentity: 'unchecked',
123123
}),
124124
(error: unknown) =>
125125
error instanceof FilesystemWorkerClientError &&
@@ -139,7 +139,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => {
139139
operation: { kind: 'glob', path: sourceDirectory, pattern: '**/*.ts' },
140140
cwd: workspace,
141141
mode: 'ask',
142-
expectedIdentity: 'unchecked',
142+
expectedIdentity: 'unchecked',
143143
});
144144
assert.equal(globResult.kind, 'glob');
145145
if (globResult.kind === 'glob') {
@@ -162,7 +162,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => {
162162
},
163163
cwd: workspace,
164164
mode: 'ask',
165-
expectedIdentity: 'unchecked',
165+
expectedIdentity: 'unchecked',
166166
}),
167167
(error: unknown) =>
168168
error instanceof FilesystemWorkerClientError && error.reason === 'grep_unavailable',
@@ -179,7 +179,7 @@ describe('Windows filesystem worker smoke', { skip: !enabled }, () => {
179179
},
180180
cwd: workspace,
181181
mode: 'ask',
182-
expectedIdentity: 'unchecked',
182+
expectedIdentity: 'unchecked',
183183
}),
184184
(error: unknown) =>
185185
error instanceof FilesystemWorkerClientError && error.reason === 'path_denied',

packages/runtime/src/filesystem-executor.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import type {
3131
} from './filesystem-worker/client.js';
3232
import { isSupportedImagePath, type ImageMimeType } from './image-file.js';
3333
import type { FilesystemWorkerResult } from './filesystem-worker/protocol.js';
34+
import { operationAccess } from './filesystem-worker/protocol.js';
3435
import { resolveCanonicalDirectoryEntryTarget } from './path-containment.js';
3536
import { normalizeSandboxBoundaryPath } from './sandbox-boundary-path.js';
3637
import { SandboxCommandError } from './sandbox/errors.js';
@@ -217,16 +218,13 @@ export function createBoundaryFilesystemExecutor(
217218
...(call.abortSignal ? { abortSignal: call.abortSignal } : {}),
218219
// The worker client now requires an explicit T0 marker (#3484): a
219220
// mutation carries its captured identity, or 'missing' when T0 saw no
220-
// target; a read never participates in CAS and says so. The kind list
221-
// deliberately mirrors `operationAccess` in the worker client
222-
// (write | apply_patch | edit | format_json) — `mutates` is narrower
223-
// and would silently drop the apply_patch identity onto 'unchecked',
224-
// disabling the queue-window CAS on the main editing channel.
221+
// target; a read never participates in CAS and says so. `operationAccess`
222+
// is the single authority on which kinds are writes (write | apply_patch
223+
// | edit | format_json) — `mutates` is narrower and would silently drop
224+
// the apply_patch identity onto 'unchecked', disabling the queue-window
225+
// CAS on the main editing channel.
225226
expectedIdentity:
226-
call.operation.kind === 'write' ||
227-
call.operation.kind === 'apply_patch' ||
228-
call.operation.kind === 'edit' ||
229-
call.operation.kind === 'format_json'
227+
operationAccess(call.operation.kind) === 'write'
230228
? (expectedIdentity ?? 'missing')
231229
: 'unchecked',
232230
});

packages/runtime/src/filesystem-worker/client.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
import {
2626
FILESYSTEM_WORKER_PROTOCOL_VERSION,
2727
FilesystemWorkerOperationSchema,
28+
operationAccess,
2829
operationUsesDirectoryEntry,
2930
parseFilesystemWorkerResponse,
3031
type FilesystemWorkerErrorCode,
@@ -652,12 +653,6 @@ function deriveWorkerProfile(
652653
};
653654
}
654655

655-
function operationAccess(kind: FilesystemWorkerOperation['kind']): 'read' | 'write' {
656-
return kind === 'write' || kind === 'apply_patch' || kind === 'edit' || kind === 'format_json'
657-
? 'write'
658-
: 'read';
659-
}
660-
661656
function operationScope(kind: FilesystemWorkerOperation['kind']): 'exact' | 'subtree' | 'auto' {
662657
if (kind === 'glob') return 'subtree';
663658
return kind === 'grep' ? 'auto' : 'exact';

packages/runtime/src/filesystem-worker/operations.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
import { isSupportedImagePath, readWorkspaceImage } from '../image-file.js';
2525
import {
2626
FILESYSTEM_WORKER_PROTOCOL_VERSION,
27+
operationAccess,
2728
operationUsesDirectoryEntry,
2829
type FilesystemWorkerErrorCode,
2930
type FilesystemWorkerOperation,
@@ -67,12 +68,6 @@ export type FilesystemWorkerGrepRunner = (
6768
input: FilesystemWorkerGrepRunInput,
6869
) => Promise<FilesystemWorkerGrepRunResult>;
6970

70-
function operationAccess(kind: FilesystemWorkerOperation['kind']): 'read' | 'write' {
71-
return kind === 'write' || kind === 'apply_patch' || kind === 'edit' || kind === 'format_json'
72-
? 'write'
73-
: 'read';
74-
}
75-
7671
export async function executeFilesystemWorkerRequest(
7772
request: FilesystemWorkerRequest,
7873
dependencies: FilesystemWorkerOperationDependencies = {},

packages/runtime/src/filesystem-worker/protocol.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,15 @@ import { validateSandboxBoundaryExpansion } from '@maka/core/sandbox-boundary';
88
// JSON protocol boundary.
99
export const FILESYSTEM_WORKER_PROTOCOL_VERSION = 7 as const;
1010

11+
/** The single authority on which operation kinds are writes. Shared by the
12+
* client (permission/identity decisions) and the worker (operation guards) so
13+
* the set cannot drift. */
14+
export function operationAccess(kind: FilesystemWorkerOperation['kind']): 'read' | 'write' {
15+
return kind === 'write' || kind === 'apply_patch' || kind === 'edit' || kind === 'format_json'
16+
? 'write'
17+
: 'read';
18+
}
19+
1120
const path = z.string().min(1).max(4096);
1221
const cwd = z.string().min(1).max(4096);
1322

0 commit comments

Comments
 (0)