Skip to content

Commit e315770

Browse files
committed
fix(desktop): contain native imports and preserve UTF-8 pages
1 parent 2ea36a2 commit e315770

2 files changed

Lines changed: 176 additions & 7 deletions

File tree

‎apps/desktop/src/main/local-files.test.ts‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,3 +153,151 @@ it('refuses oversized import files before any workspace mutation or bulk allocat
153153
)
154154
).toMatchObject({ ok: false, error: expect.stringContaining('64 MB') })
155155
})
156+
157+
it.each(['file', 'directory'] as const)(
158+
'rejects %s symlinks outside the import source during manifest and chunk reads',
159+
async (kind) => {
160+
const source = join(root, 'selected')
161+
const outside = join(root, 'selected-other')
162+
await mkdir(source)
163+
await mkdir(outside)
164+
await writeFile(join(outside, 'private.txt'), 'outside content')
165+
const target = kind === 'file' ? join(outside, 'private.txt') : outside
166+
await symlink(target, join(source, 'link'))
167+
const authorization = {
168+
toolName: 'import_local_files',
169+
args: { path: source, targetWorkspaceId: 'target' },
170+
}
171+
expect(await executeLocalFileRequest({ operation: 'manifest' }, authorization)).toMatchObject({
172+
ok: false,
173+
error: expect.stringContaining('outside this import source'),
174+
})
175+
const outsideManifest = await executeLocalFileRequest(
176+
{ operation: 'manifest' },
177+
{ toolName: 'import_local_files', args: { path: target, targetWorkspaceId: 'target' } }
178+
)
179+
if (!outsideManifest.ok || outsideManifest.data.kind !== 'manifest')
180+
throw new Error('Expected manifest')
181+
const entry = outsideManifest.data.entries.find((item) => item.kind === 'file')!
182+
expect(
183+
await executeLocalFileRequest(
184+
{
185+
operation: 'chunk',
186+
relativePath: kind === 'file' ? 'link' : 'link/private.txt',
187+
revision: entry.revision,
188+
offset: 0,
189+
},
190+
authorization
191+
)
192+
).toMatchObject({ ok: false, error: expect.stringContaining('outside this import source') })
193+
}
194+
)
195+
196+
it('supports internal symlinks and an explicitly selected symlink root, but rejects a retargeted child', async () => {
197+
const source = join(root, 'selected')
198+
await mkdir(source)
199+
await writeFile(join(source, 'notes.txt'), 'inside')
200+
await symlink(join(source, 'notes.txt'), join(source, 'alias.txt'))
201+
const selectedAlias = join(root, 'selected-alias')
202+
await symlink(source, selectedAlias)
203+
const authorization = {
204+
toolName: 'import_local_files',
205+
args: { path: selectedAlias, targetWorkspaceId: 'target' },
206+
}
207+
const manifest = await executeLocalFileRequest({ operation: 'manifest' }, authorization)
208+
if (!manifest.ok || manifest.data.kind !== 'manifest') throw new Error('Expected manifest')
209+
const entry = manifest.data.entries.find((item) => item.relativePath === 'alias.txt')!
210+
const request = {
211+
operation: 'chunk',
212+
relativePath: 'alias.txt',
213+
revision: entry.revision,
214+
offset: 0,
215+
}
216+
expect(await executeLocalFileRequest(request, authorization)).toEqual({
217+
ok: true,
218+
data: { kind: 'chunk', bytes: new Uint8Array(Buffer.from('inside')), eof: true },
219+
})
220+
await writeFile(join(root, 'outside.txt'), 'outside')
221+
await rm(join(source, 'alias.txt'))
222+
await symlink(join(root, 'outside.txt'), join(source, 'alias.txt'))
223+
expect(await executeLocalFileRequest(request, authorization)).toMatchObject({
224+
ok: false,
225+
error: expect.stringContaining('outside this import source'),
226+
})
227+
})
228+
229+
it.each(['é', '界', '😀', '\uFEFF'])(
230+
'preserves %s across the default text-page boundary',
231+
async (character) => {
232+
const path = join(root, 'unicode.txt')
233+
const prefix = 'a'.repeat(63_999)
234+
const content = `${prefix}${character}end`
235+
await writeFile(path, content)
236+
const first = await executeLocalFileRequest(
237+
{ operation: 'read' },
238+
{
239+
toolName: 'read_local_file',
240+
args: { path },
241+
}
242+
)
243+
expect(first).toMatchObject({
244+
ok: true,
245+
data: {
246+
representation: 'text',
247+
text: prefix,
248+
offset: 0,
249+
nextOffset: 63_999,
250+
truncated: true,
251+
},
252+
})
253+
if (!first.ok || first.data.kind !== 'read') throw new Error('Expected text')
254+
const second = await executeLocalFileRequest(
255+
{ operation: 'read' },
256+
{
257+
toolName: 'read_local_file',
258+
args: { path, offset: first.data.nextOffset },
259+
}
260+
)
261+
expect(second).toMatchObject({
262+
ok: true,
263+
data: {
264+
representation: 'text',
265+
text: `${character}end`,
266+
nextOffset: Buffer.byteLength(content),
267+
truncated: false,
268+
},
269+
})
270+
}
271+
)
272+
273+
it('preserves a UTF-8 BOM and rejects split offsets and limits that cannot fit a character', async () => {
274+
const path = join(root, 'unicode.txt')
275+
await writeFile(path, '\uFEFF😀end')
276+
expect(
277+
await executeLocalFileRequest(
278+
{ operation: 'read' },
279+
{
280+
toolName: 'read_local_file',
281+
args: { path, limit: 4 },
282+
}
283+
)
284+
).toMatchObject({ ok: true, data: { text: '\uFEFF', nextOffset: 3, truncated: true } })
285+
expect(
286+
await executeLocalFileRequest(
287+
{ operation: 'read' },
288+
{
289+
toolName: 'read_local_file',
290+
args: { path, offset: 4 },
291+
}
292+
)
293+
).toMatchObject({ ok: false, error: expect.stringContaining('UTF-8') })
294+
expect(
295+
await executeLocalFileRequest(
296+
{ operation: 'read' },
297+
{
298+
toolName: 'read_local_file',
299+
args: { path, offset: 3, limit: 1 },
300+
}
301+
)
302+
).toMatchObject({ ok: false, error: expect.stringContaining('limit') })
303+
})

‎apps/desktop/src/main/local-files.ts‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ function boundedInteger(value: unknown, fallback: number, max: number): number {
4242
return value
4343
}
4444

45+
function assertImportPath(root: string, candidate: string): void {
46+
const rel = relative(root, candidate)
47+
if (isAbsolute(rel) || rel === '..' || rel.startsWith(`..${sep}`))
48+
throw new Error('The file is outside this import source.')
49+
}
50+
4551
async function inspect(path: string, args: Record<string, unknown>): Promise<DesktopLocalFileRead> {
4652
const info = await stat(path)
4753
if (info.isDirectory()) {
@@ -135,14 +141,26 @@ async function inspect(path: string, args: Record<string, unknown>): Promise<Des
135141
representation: 'binary',
136142
note: 'Binary content cannot be decoded as text. Import this file into Workspace Files for document extraction.',
137143
}
144+
/** Retain partial characters for the next page; preserving BOM keeps byte offsets exact. */
145+
let text: string
146+
try {
147+
text = new TextDecoder('utf-8', { fatal: true, ignoreBOM: true }).decode(bytes, {
148+
stream: offset + bytesRead < info.size,
149+
})
150+
} catch {
151+
throw new Error('Text reads require valid UTF-8 and an offset at a character boundary.')
152+
}
153+
const textBytes = Buffer.byteLength(text)
154+
if (bytesRead > 0 && textBytes === 0)
155+
throw new Error('The read limit must fit at least one complete UTF-8 character.')
138156
return {
139157
kind: 'read',
140158
path,
141159
representation: 'text',
142-
text: bytes.toString('utf8'),
160+
text,
143161
offset,
144-
nextOffset: offset + bytesRead,
145-
truncated: offset + bytesRead < info.size,
162+
nextOffset: offset + textBytes,
163+
truncated: offset + textBytes < info.size,
146164
}
147165
} finally {
148166
await file.close()
@@ -154,13 +172,15 @@ async function manifest(
154172
args: Record<string, unknown>
155173
): Promise<DesktopLocalFileManifest> {
156174
if (typeof args.targetWorkspaceId !== 'string') throw new Error('A target workspace is required.')
175+
const root = await realpath(path)
157176
const entries: DesktopLocalFileEntry[] = []
158177
async function walk(current: string, ancestors: ReadonlySet<string>): Promise<void> {
159178
if (entries.length >= MAX_ENTRIES)
160179
throw new Error(
161180
'The directory exceeds 1,000 entries. Import smaller subdirectories separately.'
162181
)
163182
const canonical = await realpath(current)
183+
assertImportPath(root, canonical)
164184
const info = await stat(canonical)
165185
if (!info.isDirectory() && !info.isFile())
166186
throw new Error(`Cannot import special filesystem entry: ${current}`)
@@ -211,11 +231,12 @@ export async function executeLocalFileRequest(
211231
)
212232
throw new Error('Invalid file chunk request.')
213233
const child = resolve(path, request.relativePath)
214-
const rel = relative(path, child)
215-
if (isAbsolute(rel) || rel === '..' || rel.startsWith(`..${sep}`))
216-
throw new Error('The file is outside this import source.')
234+
assertImportPath(path, child)
235+
const root = await realpath(path)
236+
const canonical = await realpath(child)
237+
assertImportPath(root, canonical)
217238
const offset = boundedInteger(request.offset, 0, Number.MAX_SAFE_INTEGER)
218-
const file = await open(child, 'r')
239+
const file = await open(canonical, 'r')
219240
try {
220241
const info = await file.stat()
221242
if (!info.isFile() || revision(info) !== request.revision)

0 commit comments

Comments
 (0)