feat(runtime-files): the host serves a chat's scratch folder as a file root (#28397)

* fix(files): never write through a dangling symlink

A path that doesn't exist yet was authorized by walking up to the nearest
existing folder. A dangling symlink fails realpath with ENOENT, so the walk
stepped over it and approved the link's own path; the write then followed
the link and created its target outside the allowed folders. The walk now
continues from the link's target, and returns that real destination.

* refactor(native-chat): rename the chat visuals folder to the chat scratch folder

The per-chat folder agents write into is the chat's scratch space, not only
a visuals folder. Its root becomes <state>/chat-scratch/<hash> (the hash is
unchanged) and the agent variable becomes ORCA_CHAT_SCRATCH_DIR. The old
ORCA_CHAT_VISUALS_DIR is never set and is still stripped from what every
agent child inherits, Pi included. The visuals skill names the new variable
and says the folder is temporary.

* test(native-chat): keep the delivery test to the rename

The launch parity snapshot already pins that every agent drops the
pre-rename variable.

* fix(files): refuse any dangling symlink in the missing-path walk

Following a dangling link by hand resolved relative targets against the
lexical parent, not the link's real folder: through a folder link the walk
could loop forever or authorize the wrong file. Refuse instead: realpath
can't say where a dangling link leads, but a write would follow it.

* feat(runtime-files): the host serves a chat's scratch folder as a file root

File commands accept `path:<this host's chat-scratch root>/<32 hex>` and
resolve it only when it names this host's own folder, at the file-commands
seam so the selector never reaches chat placement. They authorize against
the project roots plus the chat-scratch root, refuse files.open/openDiff
for the folder, and agentSession.readVisual's ok results carry the file's
absolute path so a client can build the selector.

* test(runtime-files): name the readVisual helper's parameter type

* test(files): drop the loop case that duplicated the dangling-link refusal

* test(runtime-files): run the readVisual RPC test, which opens a real SQLite store, under Node

* test(runtime-files): put the same-suffix folder inside the authorized root so only the selector refuses it

* test(runtime-files): wait for the stores to close before removing the state folder
This commit is contained in:
Brennan Benson
2026-10-14 10:49:52 -07:00
committed by GitHub
parent e5e6189f96
commit 81ed8c0feb
13 changed files with 153 additions and 22 deletions
@@ -88,6 +88,7 @@ export const NODE_RUNTIME_INCLUDE = [
'src/main/runtime/structured-session-mail-redrive-wiring.test.ts',
'src/main/runtime/rpc/ws-transport.test.ts',
'src/main/runtime/rpc/dispatcher-transport-parity.test.ts',
'src/main/runtime/rpc/methods/structured-agent-session-visual.test.ts',
// Bun resolves 'ws' to its built-in client, which bypasses the patched package.
'src/main/runtime/rpc/ws-empty-payload-frame-write.test.ts',
'src/shared/remote-runtime-client.test.ts',
+1 -1
View File
@@ -44,7 +44,7 @@ export type ResolveAuthorizedPathOptions = {
* Canonicalize the parent but preserve the leaf so delete/rename target the symlink itself, not its destination (which may live outside allowed roots).
*/
preserveSymlink?: boolean
/** Roots only the desktop window may use (never runtime RPC), checked like any other root. */
/** Roots one caller owns outside the store (desktop: floating workspace; runtime: chat scratch). */
extraRoots?: readonly string[]
}
@@ -41,7 +41,8 @@ describe('readNativeChatVisualFile', () => {
ok: true,
html,
revision: nativeChatVisualRevision(Buffer.from(html)),
sizeBytes: Buffer.byteLength(html)
sizeBytes: Buffer.byteLength(html),
path: join(folder, 'chart.html')
})
})
@@ -52,7 +53,12 @@ describe('readNativeChatVisualFile', () => {
throw new Error('expected a read')
}
const again = await readNativeChatVisualFile(folder, 'chart.html', first.revision)
expect(again).toEqual({ ok: true, revision: first.revision, sizeBytes: 8, unchanged: true })
expect(again).toMatchObject({
ok: true,
revision: first.revision,
sizeBytes: 8,
unchanged: true
})
await writeFile(join(folder, 'chart.html'), '<p>b</p>')
const changed = await readNativeChatVisualFile(folder, 'chart.html', first.revision)
@@ -194,9 +194,11 @@ async function readContained(
const html = decodeVisualText(buffer)
const revision = nativeChatVisualRevision(buffer)
const sizeBytes = buffer.length
// Under the folder as named, so `path:<its folder>` is the exact selector of this file root.
const path = join(folder, file)
return knownRevision === revision
? { ok: true, revision, sizeBytes, unchanged: true }
: { ok: true, revision, sizeBytes, html }
? { ok: true, revision, sizeBytes, path, unchanged: true }
: { ok: true, revision, sizeBytes, path, html }
} finally {
await handle.close()
}
@@ -0,0 +1,51 @@
import { lstat } from 'node:fs/promises'
import { basename, dirname } from 'node:path'
import type { Store } from '../../persistence'
import { resolveAuthorizedPath, type ResolveAuthorizedPathOptions } from '../../ipc/filesystem-auth'
import { isENOENT } from '../../ipc/filesystem-path-containment'
import { nativeChatScratchRootFor } from '../../native-chat/native-chat-scratch-folder'
import { getProfileUserDataPath } from '../../orca-profiles/profile-storage-paths'
const PATH_SELECTOR_PREFIX = 'path:'
const SCRATCH_FOLDER_NAME = /^[0-9a-f]{32}$/
function chatScratchRoot(): string {
return nativeChatScratchRootFor(getProfileUserDataPath())
}
/** Runtime file commands' root check: the store's roots plus this host's chat scratch root. */
export function resolveRuntimeAuthorizedPath(
targetPath: string,
store: Store,
options: ResolveAuthorizedPathOptions = {}
): Promise<string> {
return resolveAuthorizedPath(targetPath, store, { ...options, extraRoots: [chatScratchRoot()] })
}
/** A resolved worktree id that is a selector can only be a scratch folder's: real rows have ids. */
export function isChatScratchWorktreeId(worktreeId: string): boolean {
return worktreeId.startsWith(PATH_SELECTOR_PREFIX)
}
/**
* The folder a `path:<this host's chat-scratch root>/<32 hex>` selector names, or null. A chat's
* scratch folder has no workspace row; only this host's own folder answers, never a same-suffix
* root elsewhere or a symlinked folder.
*/
export async function resolveChatScratchSelectorFolder(selector: string): Promise<string | null> {
if (!selector.startsWith(PATH_SELECTOR_PREFIX)) {
return null
}
const folder = selector.slice(PATH_SELECTOR_PREFIX.length)
if (dirname(folder) !== chatScratchRoot() || !SCRATCH_FOLDER_NAME.test(basename(folder))) {
return null
}
try {
return (await lstat(folder)).isDirectory() ? folder : null
} catch (error) {
if (isENOENT(error)) {
return null
}
throw error
}
}
+3 -3
View File
@@ -6,7 +6,7 @@ import {
resolveFileExplorerPaths
} from './file-target'
import type { RuntimeFileCommandHost } from './file-command-host'
import { resolveAuthorizedPath } from '../../ipc/filesystem-auth'
import { resolveRuntimeAuthorizedPath } from './chat-scratch-file-root'
import { constants, copyFile, mkdir, rm } from 'node:fs/promises'
import { dirname } from 'node:path'
@@ -155,8 +155,8 @@ export class RuntimeFileMutations {
requireLocalRuntimeFileRoute(tempTarget)
// Why: commit through the resolved (not symlink-preserving) temp and final paths.
const store = this.host.requireStore()
const tempPath = await resolveAuthorizedPath(tempTarget.path, store)
const finalPath = await resolveAuthorizedPath(finalTarget.path, store)
const tempPath = await resolveRuntimeAuthorizedPath(tempTarget.path, store)
const finalPath = await resolveRuntimeAuthorizedPath(finalTarget.path, store)
await mkdir(dirname(finalPath), { recursive: true })
await copyFile(tempPath, finalPath, constants.COPYFILE_EXCL)
await rm(tempPath, { force: true })
+4 -4
View File
@@ -24,7 +24,7 @@ import {
previewableBinaryByteLimit
} from './file-preview-budget'
import { readLocalFileRange } from '../../ipc/filesystem/local-file-range-read'
import { resolveAuthorizedPath } from '../../ipc/filesystem-auth'
import { resolveRuntimeAuthorizedPath } from './chat-scratch-file-root'
import {
NodeFileReadTooLargeError,
readNodeFileWithinLimit
@@ -81,7 +81,7 @@ export class RuntimeFileReads {
const target = await resolveFileExplorerPath(this.host, worktreeSelector, relativePath)
requireLocalRuntimeFileRoute(target)
// Why: sized by extension instead of a stat round trip.
const filePath = await resolveAuthorizedPath(target.path, this.host.requireStore())
const filePath = await resolveRuntimeAuthorizedPath(target.path, this.host.requireStore())
const mimeType = RUNTIME_PREVIEWABLE_BINARY_MIME_TYPES[extname(filePath).toLowerCase()]
const maxBytes = mimeType ? binaryMaxBytes : MOBILE_FILE_READ_MAX_BYTES
let buffer: Buffer
@@ -167,13 +167,13 @@ export class RuntimeFileReads {
): Promise<RuntimeFileReadChunkResult> {
const target = await resolveFileExplorerPath(this.host, worktreeSelector, relativePath)
requireLocalRuntimeFileRoute(target)
const filePath = await resolveAuthorizedPath(target.path, this.host.requireStore())
const filePath = await resolveRuntimeAuthorizedPath(target.path, this.host.requireStore())
return readLocalFileRange(filePath, offset, length)
}
}
async function readLocalMobileFile(filePath: string, store: Store): Promise<string> {
const authorizedPath = await resolveAuthorizedPath(filePath, store)
const authorizedPath = await resolveRuntimeAuthorizedPath(filePath, store)
const fileStat = await stat(authorizedPath)
// Why: cap the read so opening a large file can't block the WebSocket (previews are read-only convenience views).
const readLimit = Math.min(fileStat.size, MOBILE_FILE_READ_MAX_BYTES + 1)
+3 -3
View File
@@ -5,7 +5,7 @@ import {
ExecutionHostNotDispatchableError,
resolveFilesystemRouteForHost
} from '../../providers/execution-host-provider-dispatch'
import { resolveAuthorizedPath } from '../../ipc/filesystem-auth'
import { resolveRuntimeAuthorizedPath } from './chat-scratch-file-root'
import type {
createLocalFilesystemProvider,
LocalFilesystemProvider,
@@ -63,7 +63,7 @@ export function runtimeFileRouteForTarget(target: {
}
}
/** What a runtime command supplies for local work; path authorization is always the store's. */
/** What a runtime command supplies for local work; path authorization is always the runtime's. */
export type RuntimeLocalFileHost = Omit<LocalFilesystemProviderOptions, 'resolveAuthorizedPath'>
/** The local route for work this process runs itself; `ssh:` and `runtime:` hosts throw. */
@@ -84,7 +84,7 @@ export function requireRuntimeFileProvider(
): LocalFilesystemProvider {
return requireLocalRuntimeFileRoute(target).createProvider({
requireStore: () => host.requireStore(),
resolveAuthorizedPath
resolveAuthorizedPath: resolveRuntimeAuthorizedPath
})
}
+2 -1
View File
@@ -13,6 +13,7 @@ import { stat } from 'node:fs/promises'
import type { DirEntry, FsChangeEvent } from '../../../../shared/filesystem-entry-types'
import { sortDirEntries } from '../../../../shared/file-name-sort'
import { resolveAuthorizedPath } from '../../../ipc/filesystem-auth'
import { resolveRuntimeAuthorizedPath } from '../chat-scratch-file-root'
import { watchWindowsRuntimeFileExplorer } from './windows-file-watch'
import { beginWatcherInstall } from '../../../ipc/watcher-removal-gate'
import {
@@ -57,7 +58,7 @@ export class RuntimeFileWatches {
const finishInstall = beginWatcherInstall(target.path, sshTargetId)
try {
requireLocalRuntimeFileRoute(target)
const rootPath = await resolveAuthorizedPath(target.path, this.host.requireStore())
const rootPath = await resolveRuntimeAuthorizedPath(target.path, this.host.requireStore())
const rootStats = await stat(rootPath)
if (!rootStats.isDirectory()) {
throw new Error('not_a_directory')
+23 -1
View File
@@ -21,6 +21,11 @@ import { ClientHostedBrowserRowPublisher } from './client-hosted-browser-row-pub
import { getRuntimeBrowserPageRegistry } from './runtime-browser-page-registry'
import { getBrowserHostLeaseRegistry } from './browser-host-lease-registry-instance'
import type { RuntimeLeafRecord } from './runtime-terminal-state-records'
import { LOCAL_EXECUTION_HOST_ID } from '../../shared/execution-host'
import {
isChatScratchWorktreeId,
resolveChatScratchSelectorFolder
} from './files/chat-scratch-file-root'
import type { RuntimeStore } from './runtime-store-contract'
import type { ResolvedWorktree } from './runtime-worktree-path-identity'
@@ -37,7 +42,17 @@ export abstract class OrcaRuntimeWithFileCommands extends OrcaRuntimeWithPreserv
getRuntimeId: () => this.runtimeId,
requireStore: () => this.requireStore(),
resolveWorktreeSelector: (selector) => this.resolveWorktreeSelector(selector),
resolveRuntimeFileTarget: (selector) => this.resolveRuntimeFileTarget(selector),
resolveRuntimeFileTarget: async (selector) => {
// Why here, not in the general resolver: that one also places new chats, and a scratch folder
// must never become a chat's workspace.
const scratchFolder = await resolveChatScratchSelectorFolder(selector)
return scratchFolder
? {
worktree: { ...this.floatingWorkspaceToResolvedWorktree(scratchFolder), id: selector },
executionHostId: LOCAL_EXECUTION_HOST_ID
}
: this.resolveRuntimeFileTarget(selector)
},
resolveKnownWorkspaceFileTarget: (absolutePath, executionHostId) =>
this.resolveKnownWorkspaceFileTarget(absolutePath, executionHostId),
resolveTerminalCwd: (terminalHandle) => this.resolveTerminalCwd(terminalHandle),
@@ -55,6 +70,10 @@ export abstract class OrcaRuntimeWithFileCommands extends OrcaRuntimeWithPreserv
}),
resolveRuntimeGitTarget: (selector) => this.resolveRuntimeGitTarget(selector),
openFile: (worktreeId, filePath, relativePath, runtimeEnvironmentId, navigation, position) => {
// The UI cannot open a tab in a workspace that has no row.
if (isChatScratchWorktreeId(worktreeId)) {
throw new Error('chat_scratch_not_openable')
}
if (!this.notifier?.openFile) {
throw new Error('renderer_unavailable')
}
@@ -68,6 +87,9 @@ export abstract class OrcaRuntimeWithFileCommands extends OrcaRuntimeWithPreserv
)
},
openDiff: (worktreeId, filePath, relativePath, staged, runtimeEnvironmentId, navigation) => {
if (isChatScratchWorktreeId(worktreeId)) {
throw new Error('chat_scratch_not_openable')
}
if (!this.notifier?.openDiff) {
throw new Error('renderer_unavailable')
}
@@ -57,7 +57,7 @@ describe('RuntimeFileCommands', () => {
absolutePath: expectedPath
}
})
expect(resolveAuthorizedPathMock).toHaveBeenCalledWith(expectedPath, expect.anything())
expect(resolveAuthorizedPathMock.mock.calls[0]?.[0]).toBe(expectedPath)
})
it('does not translate UNC-style paths as WSL POSIX paths', async () => {
@@ -1,4 +1,4 @@
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
@@ -6,7 +6,13 @@ import type { AgentSessionRecord } from '../../../../shared/agent-session-record
import { agentSessionRecordFixture } from '../../../../shared/agent-session-record.test-fixture'
import { setStructuredAgentSessionHost } from '../../../native-chat/agent-session-wire/structured-agent-session-registry'
import { nativeChatScratchFolderFor } from '../../../native-chat/native-chat-scratch-folder'
import { nativeChatVisualRevision } from '../../../native-chat/native-chat-visual-file-read'
import { Store } from '../../../persistence/loading-store/store'
import { closeTestStores, createSqliteTestStore } from '../../../persistence-test-harness'
import { OrcaRuntimeService } from '../../orca-runtime'
import { RpcDispatcher } from '../dispatcher'
import { ALL_RPC_METHODS } from '.'
import { FILE_METHODS } from './files'
import { STRUCTURED_AGENT_SESSION_VISUAL_METHODS } from './structured-agent-session-visual'
import {
call as callStructured,
@@ -42,7 +48,8 @@ beforeEach(async () => {
afterEach(async () => {
clearStructuredHostStub()
await rm(stateDirectory, { recursive: true, force: true })
await closeTestStores()
await rm(stateDirectory, { recursive: true, force: true, maxRetries: 3 })
})
async function writeVisual(name: string, html: string): Promise<void> {
@@ -93,6 +100,46 @@ describe('agentSession.readVisual', () => {
)
})
it('serves the folder a visual names as a file root, and nothing outside it', async () => {
await writeVisual('chart.html', 'a')
const folder = nativeChatScratchFolderFor(stateDirectory, SESSION)
// Same suffix but inside the authorized root, so only the selector's exact match refuses it.
const foreign = nativeChatScratchFolderFor(join(folder, 'nested'), SESSION)
await Promise.all([mkdir(foreign, { recursive: true }), mkdir(join(stateDirectory, 'out'))])
await writeFile(join(foreign, 'x.html'), 'x')
await writeFile(join(stateDirectory, 'out', 'x.html'), 'x')
await symlink(join(stateDirectory, 'out'), join(folder, 'out'), 'junction')
const path = join(folder, 'chart.html')
const visual = { sessionId: SESSION, file: 'chart.html' }
const readVisual = (params: { knownRevision?: string }) =>
call('agentSession.readVisual', { ...visual, ...params }, STRUCTURED_CLIENT)
expect(await readVisual({})).toMatchObject({ result: { html: 'a', path } })
const knownRevision = nativeChatVisualRevision(Buffer.from('a'))
expect(await readVisual({ knownRevision })).toMatchObject({ result: { unchanged: true, path } })
const dataFile = join(stateDirectory, 'db', 'data.json')
const runtime = new OrcaRuntimeService(createSqliteTestStore(Store, { dataFile }))
const files = (method: string, worktree: string, relativePath: string, extra = {}) =>
new RpcDispatcher({ runtime, methods: FILE_METHODS }).dispatch({
id: 'r',
authToken: 't',
method,
params: { worktree, relativePath, ...extra }
})
const root = `path:${folder}`
const write = { content: 'b', expectedExecutionHostId: 'local' }
expect(await files('files.write', root, 'chart.html', write)).toMatchObject({ ok: true })
const read = await files('files.read', root, 'chart.html')
expect(read).toMatchObject({ result: { content: 'b' } })
const preview = { entryRelativePath: 'chart.html', implicitRootRelativePath: null }
const docPreview = { ...preview, authorizedRootRelativePaths: [] }
const previewed = await files('files.readDocPreview', root, 'chart.html', docPreview)
expect(previewed).toMatchObject({ result: { content: 'b' } })
const refusal = { error: { message: 'chat_scratch_not_openable' } }
expect(await files('files.open', root, 'chart.html')).toMatchObject(refusal)
expect(await files('files.read', `path:${foreign}`, 'x.html')).toMatchObject({ ok: false })
expect(await files('files.read', root, 'out/x.html')).toMatchObject({ ok: false })
})
it('reports a session this host has no record of', async () => {
record = null
const reply = await call(
@@ -36,7 +36,8 @@ export const AGENT_SESSION_VISUAL_READ_ERRORS = [
export type AgentSessionVisualReadError = (typeof AGENT_SESSION_VISUAL_READ_ERRORS)[number]
/** `path`: the visual's absolute path on the owning host. */
export type AgentSessionReadVisualResult =
| { ok: true; revision: string; sizeBytes: number; html: string }
| { ok: true; revision: string; sizeBytes: number; unchanged: true }
| { ok: true; revision: string; sizeBytes: number; path?: string; html: string }
| { ok: true; revision: string; sizeBytes: number; path?: string; unchanged: true }
| { ok: false; error: AgentSessionVisualReadError }