fix(runtime): detect a same-size terminal artifact swap the granted stat cannot see (#21436)

* fix(runtime): detect a same-size terminal artifact swap the granted stat cannot see

A local terminal-artifact grant pinned the file as `dev:ino:nlink:size:mtimeMs`.
On Linux every one of those can survive an unlink+recreate: ext4 reuses the
just-freed inode (measured: 100% of the time), nlink and size are unchanged for a
same-size replacement, and the mtime clock is tick-quantized to 1ms, so a swap
inside one tick produces a byte-identical identity string. The grant then served
the attacker's bytes as if nothing had changed.

Local grants now also pin a sha256 of the artifact's content, taken from the same
handle as the stat so nothing can swap the file between them, and every local
read, preview and write re-checks it before returning or committing content.

The stat identity string itself is unchanged: the relay recomputes it verbatim to
honour `expectedStatIdentity`, so its format is a wire contract. Remote grants
keep the stat-only check and are untouched.

This is also the mechanism behind the intermittent
`orca-runtime-files-terminal-artifact-io.test.ts` failure on
`rejects stale absolute terminal artifact previews before returning changed
content`: it replaces an 8-byte artifact with 8 different bytes, so whenever the
two writes shared a 1ms tick the product genuinely could not tell them apart.

* docs(runtime): record what the terminal artifact grant checks do not close

The digest makes the same-size swap detectable; it does not make the sequence
atomic. A reader arriving at the access module would reasonably assume otherwise,
so write down the measured limits of the stat identity, why the identity string
cannot change, and the four windows that stay open — the write path's surviving
rename() gap above all.
This commit is contained in:
Neil
2026-09-18 23:33:21 -07:00
committed by GitHub
parent edd9e3125b
commit 2bf538a4e1
8 changed files with 332 additions and 12 deletions
@@ -0,0 +1,85 @@
# Terminal artifact grant integrity
Clicking a path in terminal output mints a short-lived grant over one file in a
world-writable temp directory. Between minting the grant and using it, anything on the
host can replace that file. This page is the contract for the checks that catch such a
replacement — and, more importantly, for the windows they do **not** close.
Read this before weakening a check in
`src/main/runtime/runtime-file-commands-terminal-artifact-access.ts`, before assuming
the sequence is atomic, or before extending the guarantee to remote hosts.
## The stat identity is weaker than it looks
A grant pins the file as `dev:ino:nlink:size:mtimeMs`. Four of those five fields
survive an unlink-and-recreate at the same path, measured on Linux (Node 24) by
replaying that exact sequence:
| Field | After `rm` + recreate in the same directory |
| --------- | ------------------------------------------------------------------------ |
| `dev` | unchanged — same filesystem |
| `ino` | **reused 100% of the time** (3000/3000); ext4 hands back the freed inode |
| `nlink` | `1` before and after |
| `size` | unchanged whenever the replacement is the same length |
| `mtimeMs` | **quantised to 1 ms** — the kernel's coarse clock advances once a tick |
So for a same-length replacement the whole identity string collapses to a 1 ms
timestamp race. The full string collided in 63.7% of back-to-back iterations, 19.8%
with a 0.5 ms gap, and 0% at ≥1 ms. The same probe on macOS collided 0 times in 2000 —
no inode reuse, nanosecond mtimes — which is why this only ever showed up on Linux CI.
`ino` contributes no discriminating power against the exact case it is there to catch.
Do not add `ctimeMs` or `birthtimeMs` hoping to fix this: they come from the same
coarse clock and collide in the same window. `bigint: true` stats do not help either —
the precision loss is in the stored kernel timestamp, not in Node's `Number`.
## The content digest sits alongside the identity, and cannot replace it
Local grants also pin a sha256 of the artifact's bytes, read from the **same open
handle** as the stat so the two describe one inode with no gap between them.
The identity string stays exactly as it is because it is a wire contract, not a
host-local detail: `src/relay/fs-handler-terminal-artifact.ts` recomputes that same
`dev:ino:nlink:size:mtimeMs` format field-for-field from its own stat and compares it
against the `expectedStatIdentity` the host sends. Changing the format would have a new
host publishing a string an older relay can never reproduce, failing every remote
artifact read as `terminal_file_grant_stale` — a break that reaches old peers with no
wire-schema change at all. See [remote wire compatibility](./remote-wire-compatibility.md).
## What is still open
The digest narrows these checks. It does not make any of them atomic.
- **Read and preview — closed.** The bytes returned to the caller are the same
in-memory buffer that was digested, with no re-read in between, and the handle pins
the inode for the whole operation. A swap landing after the `open` leaves the handle
on the granted inode, so the granted content is what is served; a swap landing before
it fails the digest.
- **Write — a window survives.** Between the final pre-rename verification and the
`rename()` itself, the target path can still be swapped, and the rename clobbers
whatever is there. POSIX `rename()` has no "only if the target is still inode X"
form; Linux's `renameat2(RENAME_EXCHANGE)` would close it but is not portable and is
not exposed by Node. Narrowing this further means changing the commit strategy, not
adding another check before it.
- **Artifacts over 10 MB fall back to stat-only.** They digest to `null`, so only the
identity guards them. Nothing leaks: every read, preview, and write path rejects on
size before reading. The weakness is unreachable, not fixed — a later cap change
could expose it.
- **Remote and SSH grants are untouched.** They keep the stat-only check, with the full
1 ms weakness on whatever filesystem the relay runs. Closing it needs a negotiated
capability so an older relay is never sent a digest it cannot verify.
## What is proven, and what is inferred
The filesystem numbers above are direct measurements. That identical stats plus changed
content previously returned the swapped bytes, and now do not, is pinned by
`orca-runtime-files-terminal-artifact-swap-detection.test.ts`, which replays the first
stat seen for a path so the collision is deterministic rather than a 1 ms coin flip.
The join between the two is a chain, not a single observation. This defect surfaced as
an intermittent failure of `orca-runtime-files-terminal-artifact-io.test.ts` on
`rejects stale absolute terminal artifact previews before returning changed content`,
which swaps an 8-byte artifact for 8 different bytes. No one has instrumented a Linux
runner to prove that a specific CI failure was a same-tick mtime collision; the
conclusion rests on every ingredient being measured separately. Treat it accordingly if
a future failure does not fit.
@@ -0,0 +1,153 @@
import { describe, expect, it, vi } from 'vitest'
import { rm, writeFile } from 'node:fs/promises'
import type * as FsPromises from 'node:fs/promises'
import { openMock, resolveAuthorizedPathMock } from './orca-runtime-files-mock-registry'
import {
createRuntimeFileCommands,
useRuntimeFileCommandsLifecycle
} from './orca-runtime-files-test-harness'
import {
absoluteFileTarget,
resolveTerminalArtifactPath,
useTerminalArtifactTempFiles
} from './orca-runtime-files-terminal-artifact-fixtures'
vi.mock('fs', async () => (await import('./orca-runtime-files-mock-registry')).fsModuleMock())
vi.mock('fs/promises', async () =>
(await import('./orca-runtime-files-mock-registry')).fsPromisesModuleMock()
)
vi.mock(
'./file-watcher-host',
async () => (await import('./orca-runtime-files-mock-registry')).fileWatcherHostMock
)
vi.mock('../ipc/filesystem-auth', async () =>
(await import('./orca-runtime-files-mock-registry')).filesystemAuthModuleMock()
)
vi.mock('../git/runner', async () =>
(await import('./orca-runtime-files-mock-registry')).gitRunnerModuleMock()
)
vi.mock(
'../ipc/rg-availability',
async () => (await import('./orca-runtime-files-mock-registry')).rgAvailabilityMock
)
vi.mock(
'../ipc/local-worktree-runtime-options',
async () => (await import('./orca-runtime-files-mock-registry')).localWorktreeRuntimeOptionsMock
)
vi.mock(
'../ipc/filesystem-search-git',
async () => (await import('./orca-runtime-files-mock-registry')).filesystemSearchGitMock
)
vi.mock(
'../providers/ssh-filesystem-dispatch',
async () => (await import('./orca-runtime-files-mock-registry')).sshFilesystemDispatchMock
)
/**
* Replays the first stat seen for a path on every later open of it. That is what Linux hands the
* grant check for free after an unlink+recreate: ext4 reuses the just-freed inode, nlink and size
* are unchanged for a same-size swap, and the coarse mtime clock only advances once per timer tick
* (measured: 1ms), so a swap inside one tick is byte-for-byte identical to the granted stat.
*/
function replayFirstStatPerPath(): void {
const frozen = new Map<string, unknown>()
openMock.mockImplementation(async (...args) => {
const actual = await vi.importActual<typeof FsPromises>('fs/promises')
const handle = await actual.open(...args)
const key = String(args[0])
return {
stat: async () => {
if (!frozen.has(key)) {
frozen.set(key, await handle.stat())
}
return frozen.get(key)
},
read: (...readArgs: Parameters<typeof handle.read>) => handle.read(...readArgs),
close: () => handle.close()
}
})
}
describe('terminal artifact swap detection', () => {
useRuntimeFileCommandsLifecycle()
const { tempFile } = useTerminalArtifactTempFiles()
async function grantFor(artifactPath: string) {
const { commands } = createRuntimeFileCommands({ path: '/repo' })
resolveAuthorizedPathMock.mockImplementation(async (p: string) => p)
const result = await resolveTerminalArtifactPath(commands, artifactPath)
return { commands, target: absoluteFileTarget(result) }
}
it('rejects a same-size preview swap that the granted stat cannot tell apart', async () => {
replayFirstStatPerPath()
const artifactPath = await tempFile('result.png', 'fake-png')
const { commands, target } = await grantFor(artifactPath)
await rm(artifactPath)
await writeFile(artifactPath, 'changed!')
await expect(
commands.readTerminalArtifactPreview(
'id:wt-1',
target.grantId,
target.absolutePath,
'client-a'
)
).rejects.toThrow('terminal_file_grant_stale')
})
it('rejects a same-size read swap that the granted stat cannot tell apart', async () => {
replayFirstStatPerPath()
const artifactPath = await tempFile('result.json', '{"ok":true}')
const { commands, target } = await grantFor(artifactPath)
await rm(artifactPath)
await writeFile(artifactPath, '{"ok":ext}')
await expect(
commands.readTerminalArtifactFile('id:wt-1', target.grantId, target.absolutePath, 'client-a')
).rejects.toThrow('terminal_file_grant_stale')
})
it('rejects a same-size write swap before the file is changed', async () => {
replayFirstStatPerPath()
const artifactPath = await tempFile('result.json', '{"ok":true}')
const { commands, target } = await grantFor(artifactPath)
await rm(artifactPath)
await writeFile(artifactPath, '{"ok":ext}')
await expect(
commands.writeTerminalArtifactFile(
'id:wt-1',
target.grantId,
target.absolutePath,
'{"ok":no}',
'client-a'
)
).rejects.toThrow('terminal_file_grant_stale')
})
// Control: the replayed stat alone must not reject, or the swap cases above prove nothing.
it('still serves an untouched artifact under the same replayed stat', async () => {
replayFirstStatPerPath()
const artifactPath = await tempFile('result.png', 'fake-png')
const { commands, target } = await grantFor(artifactPath)
await expect(
commands.readTerminalArtifactPreview(
'id:wt-1',
target.grantId,
target.absolutePath,
'client-a'
)
).resolves.toMatchObject({
content: Buffer.from('fake-png').toString('base64'),
isBinary: true,
isImage: true,
mimeType: 'image/png'
})
})
})
@@ -295,6 +295,10 @@ describe('RuntimeFileCommands', () => {
ino: 2,
mtimeMs: 3
})),
read: vi.fn(async (buffer: Buffer) => {
const bytesRead = buffer.write('{}')
return { bytesRead, buffer }
}),
close: vi.fn(async () => undefined)
})
@@ -169,6 +169,9 @@ export type TerminalFileGrant = {
clientId?: string
expiresAt: number
statIdentity: string | null
// Why: on Linux an unlink+recreate in the same directory reuses the inode and the mtime clock is
// tick-quantized, so statIdentity alone cannot see a same-size swap. Local grants pin content too.
contentDigest: string | null
readOnly: boolean
provenance: 'terminal-output' | 'native-chat'
expiryTimer?: ReturnType<typeof setTimeout>
@@ -8,6 +8,7 @@ import {
assertTerminalArtifactNotHardLinked,
canonicalPathForArtifactComparison,
isTerminalArtifactHardLinked,
localTerminalArtifactContentDigest,
terminalFileStatIdentity
} from './runtime-file-commands-terminal-artifact-access'
import {
@@ -63,9 +64,15 @@ export class RuntimeFileCommandsWithResolveAllowedTerminalArtifactPath extends R
readOnly?: boolean
provenance?: TerminalFileGrant['provenance']
}): Promise<RuntimeTerminalPathResolution> {
const stats = args.connectionId
? await this.statRemoteTerminalPath(args.artifactPath, args.connectionId)
: await this.statLocalTerminalPath(args.artifactPath)
let contentDigest: string | null = null
let stats: RuntimeFileStatLike & { isDirectory: () => boolean }
if (args.connectionId) {
stats = await this.statRemoteTerminalPath(args.artifactPath, args.connectionId)
} else {
const local = await this.statLocalTerminalArtifact(args.artifactPath)
stats = local.stats
contentDigest = local.contentDigest
}
const isDirectory = stats.isDirectory()
if (!isDirectory && isTerminalArtifactHardLinked(stats)) {
return {
@@ -86,7 +93,8 @@ export class RuntimeFileCommandsWithResolveAllowedTerminalArtifactPath extends R
clientId: args.clientId,
readOnly: args.readOnly === true,
provenance: args.provenance ?? 'terminal-output',
stats
stats,
contentDigest
})
return {
worktree: args.worktreeId,
@@ -135,10 +143,22 @@ export class RuntimeFileCommandsWithResolveAllowedTerminalArtifactPath extends R
protected async statLocalTerminalPath(
absolutePath: string
): Promise<RuntimeFileStatLike & { isDirectory: () => boolean }> {
return (await this.statLocalTerminalArtifact(absolutePath)).stats
}
/** Stat and content digest taken from one handle, so nothing can swap the file between them. */
protected async statLocalTerminalArtifact(absolutePath: string): Promise<{
stats: RuntimeFileStatLike & { isDirectory: () => boolean }
contentDigest: string | null
}> {
await assertLocalTerminalArtifactPathStillCanonical(absolutePath)
const handle = await open(absolutePath, 'r')
try {
return handle.stat()
const stats = await handle.stat()
const contentDigest = stats.isDirectory()
? null
: await localTerminalArtifactContentDigest(handle, stats.size)
return { stats, contentDigest }
} finally {
await handle.close()
}
@@ -153,6 +173,7 @@ export class RuntimeFileCommandsWithResolveAllowedTerminalArtifactPath extends R
readOnly?: boolean
provenance: TerminalFileGrant['provenance']
stats: RuntimeFileStatLike
contentDigest?: string | null
}): TerminalFileGrant {
assertTerminalArtifactNotHardLinked(args.stats)
const grant: TerminalFileGrant = {
@@ -164,6 +185,7 @@ export class RuntimeFileCommandsWithResolveAllowedTerminalArtifactPath extends R
...(args.clientId ? { clientId: args.clientId } : {}),
expiresAt: Date.now() + TERMINAL_FILE_GRANT_TTL_MS,
statIdentity: terminalFileStatIdentity(args.stats),
contentDigest: args.contentDigest ?? null,
readOnly: args.readOnly === true,
provenance: args.provenance
}
@@ -1,5 +1,6 @@
// @ts-nocheck -- mechanically split declarations.
import { tmpdir } from 'node:os'
import { createHash } from 'node:crypto'
import { parseWslPath, toWindowsWslPath } from '../wsl'
import { realpath } from 'node:fs/promises'
import type { FileHandle } from 'node:fs/promises'
@@ -7,7 +8,10 @@ import type {
RuntimeFileStatLike,
TerminalFileGrant
} from './runtime-file-commands-mobile-file-list-limit'
import { MOBILE_FILE_READ_MAX_BYTES } from './runtime-file-commands-mobile-file-list-limit'
import {
LOCAL_PREVIEWABLE_BINARY_MAX_BYTES,
MOBILE_FILE_READ_MAX_BYTES
} from './runtime-file-commands-mobile-file-list-limit'
export async function localTerminalArtifactRoots(worktreePath: string): Promise<string[]> {
const roots = new Set<string>([tmpdir()])
@@ -42,6 +46,47 @@ export async function readFileHandleBufferBounded(
return buffer.subarray(0, bytesRead)
}
/**
* Content fingerprint pinned into a local grant. Bounded by the largest previewable size so any
* artifact an access path can return is covered whole; larger files digest to null because both
* read paths reject them on size before any content leaves the host.
*/
export async function localTerminalArtifactContentDigest(
handle: FileHandle,
size: number
): Promise<string | null> {
if (size > LOCAL_PREVIEWABLE_BINARY_MAX_BYTES) {
return null
}
const buffer = await readFileHandleBufferBounded(handle, LOCAL_PREVIEWABLE_BINARY_MAX_BYTES + 1)
return terminalArtifactContentDigest(buffer)
}
export function terminalArtifactContentDigest(buffer: Buffer): string {
return createHash('sha256').update(buffer).digest('hex')
}
/**
* Rejects a local artifact whose bytes changed since the grant was minted. `buffer` must hold the
* whole file — every local access path size-checks before it reads, so a buffer that reaches here
* is the complete artifact.
*/
export function assertTerminalArtifactContentUnchanged(
grant: Pick<TerminalFileGrant, 'contentDigest'>,
buffer: Buffer
): void {
if (
grant.contentDigest !== null &&
grant.contentDigest !== terminalArtifactContentDigest(buffer)
) {
throw new Error('terminal_file_grant_stale')
}
}
// Wire contract: the relay recomputes this exact string from its own stat to honour
// expectedStatIdentity, so the format cannot change without a negotiated capability.
// What this identity cannot distinguish, and which windows stay open after the digest:
// docs/reference/terminal-artifact-grant-integrity.md
export function terminalFileStatIdentity(stats: RuntimeFileStatLike): string | null {
const dev = typeof stats.dev === 'number' ? stats.dev : null
const ino = typeof stats.ino === 'number' ? stats.ino : null
@@ -14,6 +14,7 @@ import {
} from './runtime-file-commands-mobile-file-list-limit'
import type { FileHandle } from 'node:fs/promises'
import {
assertTerminalArtifactContentUnchanged,
assertTerminalFileGrantFresh,
canonicalPathForArtifactComparison,
localTerminalArtifactRoots,
@@ -84,6 +85,7 @@ export async function readLocalTerminalArtifactFileFromHandle(
}
assertTerminalFileGrantFresh(grant, fileStat)
const buffer = await readFileHandleBufferBounded(handle, MOBILE_FILE_READ_MAX_BYTES + 1)
assertTerminalArtifactContentUnchanged(grant, buffer)
if (isBinaryBuffer(buffer)) {
throw new Error('binary_file')
}
@@ -113,6 +115,7 @@ export async function readLocalTerminalArtifactPreviewFromHandle(
if (buffer.byteLength > binaryMaxBytes) {
throw new Error('file_too_large')
}
assertTerminalArtifactContentUnchanged(grant, buffer)
return {
content: buffer.toString('base64'),
isBinary: true,
@@ -8,6 +8,7 @@ import {
} from './runtime-file-commands-mobile-file-list-limit'
import { isBinaryBuffer, isMobileBinaryPath } from './runtime-file-command-host'
import {
assertTerminalArtifactContentUnchanged,
assertTerminalFileGrantFresh,
readFileHandleBufferBounded,
terminalFileStatIdentity
@@ -75,9 +76,9 @@ export class RuntimeFileCommandsWithWriteTerminalArtifactFile extends RuntimeFil
throw new Error('file_too_large')
}
assertTerminalFileGrantFresh(grant, fileStats)
if (
isBinaryBuffer(await readFileHandleBufferBounded(handle, MOBILE_FILE_READ_MAX_BYTES + 1))
) {
const buffer = await readFileHandleBufferBounded(handle, MOBILE_FILE_READ_MAX_BYTES + 1)
assertTerminalArtifactContentUnchanged(grant, buffer)
if (isBinaryBuffer(buffer)) {
throw new Error('binary_file')
}
} finally {
@@ -95,13 +96,17 @@ export class RuntimeFileCommandsWithWriteTerminalArtifactFile extends RuntimeFil
const freshHandle = await openLocalTerminalArtifactGrant(grant, constants.O_RDONLY)
try {
assertTerminalFileGrantFresh(grant, await freshHandle.stat())
assertTerminalArtifactContentUnchanged(
grant,
await readFileHandleBufferBounded(freshHandle, MOBILE_FILE_READ_MAX_BYTES + 1)
)
} finally {
await freshHandle.close()
}
await rename(tempPath, grant.absolutePath)
grant.statIdentity = terminalFileStatIdentity(
await this.statLocalTerminalPath(grant.absolutePath)
)
const committed = await this.statLocalTerminalArtifact(grant.absolutePath)
grant.statIdentity = terminalFileStatIdentity(committed.stats)
grant.contentDigest = committed.contentDigest
this.refreshTerminalFileGrant(grant)
return { ok: true }
} finally {