fix: reject file writes with missing content (#2925) (#2936)

* fix: reject file writes with missing content

files.write / files.writeBase64 coerced a missing or non-string
`content` to '' and wrote it, silently truncating the target file.
Require a real string instead; an explicit '' (empty file) is still
allowed. Applies to writeBase64Chunk via its base schema.

Closes #2925

* review: add files.writeBase64Chunk missing-content test

Code review noted the chunk variant's missing-content rejection was only
covered transitively via schema inheritance. Add an explicit test.

* review: expand file write content validation tests

- cover null and non-string write content for text, base64, and chunk writes
- keep explicit empty base64 writes valid

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Jinwoo-H <jinwoo0825@gmail.com>
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Trevin Chow 2026-05-28 00:38:52 -07:00 committed by GitHub
parent 5f2e0584ac
commit 4512da4256
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 145 additions and 4 deletions

View File

@ -330,6 +330,146 @@ describe('file RPC methods', () => {
expect(response).toMatchObject({ ok: true, result: { ok: true } })
})
it.each([
['missing content', { worktree: 'id:wt-1', relativePath: 'src/index.ts' }],
['null content', { worktree: 'id:wt-1', relativePath: 'src/index.ts', content: null }],
['non-string content', { worktree: 'id:wt-1', relativePath: 'src/index.ts', content: 0 }]
])('rejects a write with %s instead of truncating the file', async (_name, params) => {
const runtime = {
getRuntimeId: () => 'test-runtime',
writeFileExplorerFile: vi.fn().mockResolvedValue({ ok: true })
} as unknown as OrcaRuntimeService
const dispatcher = new RpcDispatcher({ runtime, methods: FILE_METHODS })
const response = await dispatcher.dispatch(makeRequest('files.write', params))
expect(response).toMatchObject({ ok: false })
expect(runtime.writeFileExplorerFile).not.toHaveBeenCalled()
})
it('still allows writing an explicit empty string (empty file)', async () => {
const runtime = {
getRuntimeId: () => 'test-runtime',
writeFileExplorerFile: vi.fn().mockResolvedValue({ ok: true })
} as unknown as OrcaRuntimeService
const dispatcher = new RpcDispatcher({ runtime, methods: FILE_METHODS })
const response = await dispatcher.dispatch(
makeRequest('files.write', { worktree: 'id:wt-1', relativePath: 'src/index.ts', content: '' })
)
expect(runtime.writeFileExplorerFile).toHaveBeenCalledWith('id:wt-1', 'src/index.ts', '')
expect(response).toMatchObject({ ok: true, result: { ok: true } })
})
it('allows writing explicit empty base64 content', async () => {
const runtime = {
getRuntimeId: () => 'test-runtime',
writeFileExplorerFileBase64: vi.fn().mockResolvedValue({ ok: true })
} as unknown as OrcaRuntimeService
const dispatcher = new RpcDispatcher({ runtime, methods: FILE_METHODS })
const response = await dispatcher.dispatch(
makeRequest('files.writeBase64', {
worktree: 'id:wt-1',
relativePath: 'assets/logo.png',
contentBase64: ''
})
)
expect(runtime.writeFileExplorerFileBase64).toHaveBeenCalledWith(
'id:wt-1',
'assets/logo.png',
''
)
expect(response).toMatchObject({ ok: true, result: { ok: true } })
})
it.each([
['missing content', { worktree: 'id:wt-1', relativePath: 'assets/logo.png' }],
['null content', { worktree: 'id:wt-1', relativePath: 'assets/logo.png', contentBase64: null }],
[
'non-string content',
{ worktree: 'id:wt-1', relativePath: 'assets/logo.png', contentBase64: 0 }
]
])('rejects a base64 write with %s', async (_name, params) => {
const runtime = {
getRuntimeId: () => 'test-runtime',
writeFileExplorerFileBase64: vi.fn().mockResolvedValue({ ok: true })
} as unknown as OrcaRuntimeService
const dispatcher = new RpcDispatcher({ runtime, methods: FILE_METHODS })
const response = await dispatcher.dispatch(makeRequest('files.writeBase64', params))
expect(response).toMatchObject({ ok: false })
expect(runtime.writeFileExplorerFileBase64).not.toHaveBeenCalled()
})
it('allows writing an explicit empty base64 chunk', async () => {
const runtime = {
getRuntimeId: () => 'test-runtime',
writeFileExplorerFileBase64Chunk: vi.fn().mockResolvedValue({ ok: true })
} as unknown as OrcaRuntimeService
const dispatcher = new RpcDispatcher({ runtime, methods: FILE_METHODS })
const response = await dispatcher.dispatch(
makeRequest('files.writeBase64Chunk', {
worktree: 'id:wt-1',
relativePath: 'assets/video.mov',
contentBase64: '',
append: true
})
)
expect(runtime.writeFileExplorerFileBase64Chunk).toHaveBeenCalledWith(
'id:wt-1',
'assets/video.mov',
'',
true
)
expect(response).toMatchObject({ ok: true, result: { ok: true } })
})
it.each([
[
'missing content',
{
worktree: 'id:wt-1',
relativePath: 'assets/video.mov',
append: true
}
],
[
'null content',
{
worktree: 'id:wt-1',
relativePath: 'assets/video.mov',
contentBase64: null,
append: true
}
],
[
'non-string content',
{
worktree: 'id:wt-1',
relativePath: 'assets/video.mov',
contentBase64: 0,
append: true
}
]
])('rejects a base64 chunk write with %s (inherits the schema)', async (_name, params) => {
const runtime = {
getRuntimeId: () => 'test-runtime',
writeFileExplorerFileBase64Chunk: vi.fn().mockResolvedValue({ ok: true })
} as unknown as OrcaRuntimeService
const dispatcher = new RpcDispatcher({ runtime, methods: FILE_METHODS })
const response = await dispatcher.dispatch(makeRequest('files.writeBase64Chunk', params))
expect(response).toMatchObject({ ok: false })
expect(runtime.writeFileExplorerFileBase64Chunk).not.toHaveBeenCalled()
})
it('commits staged runtime uploads without clobbering the final destination', async () => {
const runtime = {
getRuntimeId: () => 'test-runtime',

View File

@ -30,18 +30,19 @@ const FileTreePath = WorktreeSelector.extend({
.pipe(z.string())
})
// Why: write content must be a real string. Coercing a missing/non-string value
// to '' silently truncated the target file to empty instead of erroring. An
// explicit '' is still accepted (writing an empty file is legitimate).
const FileWrite = FileOpen.extend({
content: z
.unknown()
.transform((v) => (typeof v === 'string' ? v : ''))
.pipe(z.string())
.refine((v): v is string => typeof v === 'string', { message: 'Missing file content' })
})
const FileWriteBase64 = FileOpen.extend({
contentBase64: z
.unknown()
.transform((v) => (typeof v === 'string' ? v : ''))
.pipe(z.string())
.refine((v): v is string => typeof v === 'string', { message: 'Missing file content' })
})
const FileWriteBase64Chunk = FileWriteBase64.extend({