Skip to content

Commit 8a8f593

Browse files
committed
fix(desktop): enumerate approved directory descriptors
1 parent 635e680 commit 8a8f593

9 files changed

Lines changed: 362 additions & 130 deletions

File tree

‎apps/desktop/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ src/main/ # main process (bundled to dist/main.cjs)
3838
src/preload/ # isolated renderer bridges
3939
index.ts # hosted-app contextBridge IPC bridge (dist/preload.cjs)
4040
browser/ # minimal agent-browser credential helper (dist/browser-preload.cjs)
41-
native/ # Node-API/AppKit bridge for native macOS Help docs search
41+
native/ # Node-API bridges for directory enumeration and macOS Help docs search
4242
static/ # bundled local pages (offline.html, server.html), served over sim-shell:
4343
e2e/ # Playwright _electron smoke suite
4444
```
@@ -190,7 +190,7 @@ Copilot can inspect user-selected local directories through the ordinary VFS too
190190

191191
The native `read_local_file` and `import_local_files` tools also accept absolute or `~/` paths. They reuse the same remembered folder grants as the VFS tools. For an unapproved path, Electron displays a bundled, isolated dialog showing the canonical folder and connected server. **Allow folder** grants read and import access to that folder and its subfolders across chats and normal app restarts. A file request proposes its containing folder explicitly; no wider folder is approved silently. Closing or declining the dialog returns no contents. Users can add or forget folders through **File → Folder Access**. As with VFS grants, sign-out and server changes clear access, and unavailable secure storage limits persistence to the app session. New consent prompts are serialized, but reads of approved folders proceed independently. Existing encrypted path-based approvals retain their scope and acquire folder-identity metadata on their first restore.
192192

193-
Approved native reads can return bounded text, directory listings, images, or PDFs to the chat. Approved imports transfer file bytes to Workspace Files. Electron revalidates every pending call before using a grant, including remembered grants, checks canonical containment and grant identity throughout the operation, and opens files with no-follow and descriptor identity checks. A model or hosted renderer cannot answer the local consent dialog. These permissions govern the native file tools; the separately enabled terminal still runs with the user's OS privileges.
193+
Approved native reads can return bounded text, directory listings, images, or PDFs to the chat. Approved imports transfer file bytes to Workspace Files. Electron revalidates every pending call before using a grant, including remembered grants, checks canonical containment and grant identity throughout the operation, and opens files with no-follow and descriptor identity checks. Directory enumeration uses `fdopendir` on the verified descriptor, so replacing a parent path cannot redirect the listing. A model or hosted renderer cannot answer the local consent dialog. These permissions govern the native file tools; the separately enabled terminal still runs with the user's OS privileges.
194194

195195
## Auto-update, channels, rollout, rollback
196196

‎apps/desktop/e2e/local-files.spec.ts‎

Lines changed: 98 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -253,40 +253,113 @@ test('native file tools remember folder consent across chats and restarts until
253253
rmSync(other)
254254
}
255255
})
256-
await test.step('a directory swapped out and back cannot leak outside entries', async () => {
256+
await test.step('swapping an ancestor cannot redirect directory enumeration', async () => {
257+
const parent = join(realpathSync(source), 'parent')
258+
const child = join(parent, 'child')
259+
const backup = join(realpathSync(source), 'parent-backup')
260+
const otherParent = join(outside, 'parent')
261+
mkdirSync(child, { recursive: true })
262+
mkdirSync(join(otherParent, 'child'), { recursive: true })
263+
writeFileSync(join(child, 'allowed.txt'), 'allowed')
264+
writeFileSync(join(otherParent, 'child', 'private.txt'), 'outside')
265+
calls.ancestorRace = { toolName: 'read_local_file', args: { path: child } }
257266
await app?.evaluate(
258267
(_electron, paths) => {
259268
const fs = process.getBuiltinModule(
260269
'node:fs/promises'
261270
) as typeof import('node:fs/promises')
262-
const originalOpen = fs.opendir
263-
const originalRead = fs.readdir
264-
const swapped = async <T>(operation: () => Promise<T>): Promise<T> => {
265-
fs.opendir = originalOpen
266-
fs.readdir = originalRead
267-
await fs.rename(paths.source, paths.backup)
268-
await fs.symlink(paths.outside, paths.source)
269-
try {
270-
return await operation()
271-
} finally {
272-
await fs.rm(paths.source)
273-
await fs.rename(paths.backup, paths.source)
271+
const originalStat = fs.lstat
272+
const originalRealpath = fs.realpath
273+
let swapped = false
274+
const restore = async () => {
275+
fs.lstat = originalStat
276+
fs.realpath = originalRealpath
277+
if (swapped) {
278+
await fs.rm(paths.parent)
279+
await fs.rename(paths.backup, paths.parent)
280+
swapped = false
274281
}
275282
}
276-
fs.opendir = (path, options) =>
277-
path === paths.source
278-
? swapped(() => originalOpen(path, options))
279-
: originalOpen(path, options)
280-
fs.readdir = ((...args: Parameters<typeof fs.readdir>) =>
281-
args[0] === paths.source
282-
? swapped(() => originalRead(...args))
283-
: originalRead(...args)) as typeof fs.readdir
283+
;(
284+
globalThis as typeof globalThis & { restoreLocalFileRace?: () => Promise<void> }
285+
).restoreLocalFileRace = restore
286+
fs.lstat = (async (...args: Parameters<typeof fs.lstat>) => {
287+
const result = await originalStat(...args)
288+
if (args[0] === paths.child) {
289+
fs.lstat = originalStat
290+
await fs.rename(paths.parent, paths.backup)
291+
await fs.symlink(paths.otherParent, paths.parent)
292+
swapped = true
293+
}
294+
return result
295+
}) as typeof fs.lstat
296+
fs.realpath = (async (...args: Parameters<typeof fs.realpath>) => {
297+
if (swapped && args[0] === paths.child) await restore()
298+
return originalRealpath(...args)
299+
}) as typeof fs.realpath
284300
},
285-
{ source: realpathSync(source), backup: join(root, 'reports-backup'), outside }
301+
{ parent, child, backup, otherParent }
286302
)
287-
expect(await invoke({ operation: 'read', toolCallId: 'directory' })).toMatchObject({
288-
ok: false,
289-
})
303+
try {
304+
expect(await invoke({ operation: 'read', toolCallId: 'ancestorRace' })).toMatchObject({
305+
ok: true,
306+
data: { entries: [{ name: 'allowed.txt', kind: 'file' }] },
307+
})
308+
} finally {
309+
await app?.evaluate(async () => {
310+
const runtime = globalThis as typeof globalThis & {
311+
restoreLocalFileRace?: () => Promise<void>
312+
}
313+
await runtime.restoreLocalFileRace?.()
314+
runtime.restoreLocalFileRace = undefined
315+
})
316+
rmSync(parent, { recursive: true, force: true })
317+
rmSync(otherParent, { recursive: true, force: true })
318+
}
319+
})
320+
await test.step('cancelling after a file opens prevents its contents from returning', async () => {
321+
await app?.evaluate(
322+
(_electron, path) => {
323+
const fs = process.getBuiltinModule(
324+
'node:fs/promises'
325+
) as typeof import('node:fs/promises')
326+
const original = fs.open
327+
fs.open = async (...args: Parameters<typeof fs.open>) => {
328+
const handle = await original(...args)
329+
if (args[0] === path) {
330+
fs.open = original
331+
await new Promise<void>((resolve) => {
332+
;(
333+
globalThis as typeof globalThis & { releaseLocalFileRead?: () => void }
334+
).releaseLocalFileRead = resolve
335+
})
336+
}
337+
return handle
338+
}
339+
},
340+
realpathSync(join(source, 'report.txt'))
341+
)
342+
const reading = invoke({ operation: 'read', toolCallId: 'text' })
343+
void reading.catch(() => {})
344+
try {
345+
await expect
346+
.poll(() =>
347+
app?.evaluate(
348+
() =>
349+
typeof (globalThis as typeof globalThis & { releaseLocalFileRead?: () => void })
350+
.releaseLocalFileRead === 'function'
351+
)
352+
)
353+
.toBe(true)
354+
await invoke({ operation: 'cancel', toolCallId: 'text' })
355+
} finally {
356+
await app?.evaluate(() => {
357+
const runtime = globalThis as typeof globalThis & { releaseLocalFileRead?: () => void }
358+
runtime.releaseLocalFileRead?.()
359+
runtime.releaseLocalFileRead = undefined
360+
})
361+
}
362+
expect(await reading).toMatchObject({ ok: false })
290363
})
291364
await test.step('cancelling in the renderer closes consent without remembering access', async () => {
292365
calls.cancelled = {

‎apps/desktop/native/directory.cc‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
#include <node_api.h>
2+
3+
#include <dirent.h>
4+
#include <fcntl.h>
5+
#include <sys/stat.h>
6+
#include <unistd.h>
7+
8+
#include <cerrno>
9+
#include <climits>
10+
#include <cmath>
11+
#include <cstring>
12+
#include <string>
13+
#include <vector>
14+
15+
struct Entry {
16+
std::string name;
17+
const char* kind;
18+
};
19+
20+
struct DirectoryRead {
21+
napi_async_work work = nullptr;
22+
napi_deferred deferred = nullptr;
23+
int descriptor = -1;
24+
size_t limit = 0;
25+
bool truncated = false;
26+
std::string error;
27+
std::vector<Entry> entries;
28+
};
29+
30+
static const char* EntryKind(DIR* directory, const dirent* entry) {
31+
switch (entry->d_type) {
32+
case DT_REG: return "file";
33+
case DT_DIR: return "directory";
34+
case DT_LNK: return "symlink";
35+
case DT_UNKNOWN: {
36+
struct stat metadata;
37+
if (fstatat(dirfd(directory), entry->d_name, &metadata, AT_SYMLINK_NOFOLLOW) == 0) {
38+
if (S_ISREG(metadata.st_mode)) return "file";
39+
if (S_ISDIR(metadata.st_mode)) return "directory";
40+
if (S_ISLNK(metadata.st_mode)) return "symlink";
41+
}
42+
return "other";
43+
}
44+
default: return "other";
45+
}
46+
}
47+
48+
static void ReadEntries(napi_env, void* data) {
49+
auto* read = static_cast<DirectoryRead*>(data);
50+
DIR* directory = fdopendir(read->descriptor);
51+
if (!directory) {
52+
close(read->descriptor);
53+
read->descriptor = -1;
54+
read->error = "Could not enumerate the approved directory.";
55+
return;
56+
}
57+
read->descriptor = -1;
58+
while (true) {
59+
errno = 0;
60+
const dirent* entry = readdir(directory);
61+
if (!entry) {
62+
if (errno != 0) read->error = "Could not finish reading the approved directory.";
63+
break;
64+
}
65+
if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) continue;
66+
if (read->entries.size() == read->limit) {
67+
read->truncated = true;
68+
break;
69+
}
70+
read->entries.push_back({entry->d_name, EntryKind(directory, entry)});
71+
}
72+
closedir(directory);
73+
}
74+
75+
static napi_value String(napi_env env, const char* value) {
76+
napi_value result;
77+
napi_create_string_utf8(env, value, NAPI_AUTO_LENGTH, &result);
78+
return result;
79+
}
80+
81+
static void Complete(napi_env env, napi_status status, void* data) {
82+
auto* read = static_cast<DirectoryRead*>(data);
83+
if (read->descriptor >= 0) close(read->descriptor);
84+
if (status != napi_ok || !read->error.empty()) {
85+
napi_value error;
86+
napi_create_error(env, nullptr,
87+
String(env, read->error.empty() ? "Directory read cancelled." : read->error.c_str()),
88+
&error);
89+
napi_reject_deferred(env, read->deferred, error);
90+
} else {
91+
napi_value result;
92+
napi_value entries;
93+
napi_value truncated;
94+
napi_create_object(env, &result);
95+
napi_create_array_with_length(env, read->entries.size(), &entries);
96+
for (size_t index = 0; index < read->entries.size(); index++) {
97+
napi_value entry;
98+
napi_create_object(env, &entry);
99+
napi_set_named_property(env, entry, "name", String(env, read->entries[index].name.c_str()));
100+
napi_set_named_property(env, entry, "kind", String(env, read->entries[index].kind));
101+
napi_set_element(env, entries, index, entry);
102+
}
103+
napi_get_boolean(env, read->truncated, &truncated);
104+
napi_set_named_property(env, result, "entries", entries);
105+
napi_set_named_property(env, result, "truncated", truncated);
106+
napi_resolve_deferred(env, read->deferred, result);
107+
}
108+
napi_delete_async_work(env, read->work);
109+
delete read;
110+
}
111+
112+
/** The descriptor is duplicated before scheduling; no pathname is reopened by the worker. */
113+
static napi_value ReadDirectory(napi_env env, napi_callback_info info) {
114+
size_t count = 2;
115+
napi_value arguments[2];
116+
double descriptor = -1;
117+
double limit = 0;
118+
if (napi_get_cb_info(env, info, &count, arguments, nullptr, nullptr) != napi_ok || count != 2 ||
119+
napi_get_value_double(env, arguments[0], &descriptor) != napi_ok ||
120+
napi_get_value_double(env, arguments[1], &limit) != napi_ok ||
121+
!std::isfinite(descriptor) || descriptor < 0 || descriptor > INT_MAX ||
122+
descriptor != std::floor(descriptor) || limit < 1 || limit > 1000 || limit != std::floor(limit)) {
123+
napi_throw_type_error(env, nullptr, "Expected a directory descriptor and an entry limit from 1 to 1000.");
124+
return nullptr;
125+
}
126+
auto* read = new DirectoryRead();
127+
read->limit = static_cast<size_t>(limit);
128+
read->descriptor = fcntl(static_cast<int>(descriptor), F_DUPFD_CLOEXEC, 0);
129+
if (read->descriptor < 0) {
130+
delete read;
131+
napi_throw_error(env, nullptr, "Could not retain the approved directory descriptor.");
132+
return nullptr;
133+
}
134+
napi_value promise;
135+
if (napi_create_promise(env, &read->deferred, &promise) != napi_ok ||
136+
napi_create_async_work(env, nullptr, String(env, "ReadApprovedDirectory"), ReadEntries,
137+
Complete, read, &read->work) != napi_ok ||
138+
napi_queue_async_work(env, read->work) != napi_ok) {
139+
close(read->descriptor);
140+
if (read->work) napi_delete_async_work(env, read->work);
141+
delete read;
142+
napi_throw_error(env, nullptr, "Could not schedule the directory read.");
143+
return nullptr;
144+
}
145+
return promise;
146+
}
147+
148+
NAPI_MODULE_INIT() {
149+
napi_property_descriptor property = {
150+
"readDirectory", nullptr, ReadDirectory, nullptr, nullptr, nullptr, napi_default, nullptr
151+
};
152+
napi_define_properties(env, exports, 1, &property);
153+
return exports;
154+
}

‎apps/desktop/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
"lint:check": "biome check .",
2626
"format": "biome format --write .",
2727
"format:check": "biome format .",
28-
"test": "vitest run",
29-
"test:watch": "vitest",
28+
"test": "bun run scripts/build-native.ts && vitest run",
29+
"test:watch": "bun run scripts/build-native.ts && vitest",
3030
"test:e2e": "playwright test"
3131
},
3232
"dependencies": {
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { execFileSync } from 'node:child_process'
2+
import { existsSync, mkdirSync, statSync } from 'node:fs'
3+
import { dirname, join } from 'node:path'
4+
import { createLogger } from '@sim/logger'
5+
6+
const logger = createLogger('DesktopNativeBuild')
7+
if (process.platform !== 'darwin' && process.platform !== 'linux') {
8+
throw new Error('Native desktop modules require macOS or Linux.')
9+
}
10+
const nodeExecutable = execFileSync('node', ['-p', 'process.execPath'], { encoding: 'utf8' }).trim()
11+
const includeDirectory = join(dirname(nodeExecutable), '..', 'include', 'node')
12+
if (!existsSync(join(includeDirectory, 'node_api.h'))) {
13+
throw new Error(`Could not find Node-API headers in ${includeDirectory}`)
14+
}
15+
const outputDirectory = 'dist/native'
16+
mkdirSync(outputDirectory, { recursive: true })
17+
const modules = [
18+
{ name: 'directory', source: 'native/directory.cc', appKit: false },
19+
...(process.platform === 'darwin'
20+
? [{ name: 'help-search', source: 'native/help-search.mm', appKit: true }]
21+
: []),
22+
]
23+
for (const module of modules) {
24+
const output = join(outputDirectory, `${module.name}.node`)
25+
if (
26+
existsSync(output) &&
27+
statSync(output).mtimeMs >=
28+
Math.max(statSync(module.source).mtimeMs, statSync(import.meta.filename).mtimeMs)
29+
)
30+
continue
31+
const macOS = process.platform === 'darwin'
32+
execFileSync(
33+
macOS ? 'xcrun' : 'c++',
34+
[
35+
...(macOS ? ['clang++'] : []),
36+
'-std=c++17',
37+
'-DNAPI_VERSION=8',
38+
...(macOS
39+
? [
40+
'-bundle',
41+
'-undefined',
42+
'dynamic_lookup',
43+
'-mmacosx-version-min=12.0',
44+
'-arch',
45+
'arm64',
46+
'-arch',
47+
'x86_64',
48+
]
49+
: ['-shared', '-fPIC']),
50+
'-I',
51+
includeDirectory,
52+
...(module.appKit
53+
? ['-fobjc-arc', '-fblocks', '-framework', 'AppKit', '-framework', 'Foundation']
54+
: []),
55+
'-o',
56+
output,
57+
module.source,
58+
],
59+
{ stdio: 'inherit' }
60+
)
61+
logger.info('Compiled native desktop module', { module: module.name })
62+
}

0 commit comments

Comments
 (0)