Skip to content

Commit a7e3843

Browse files
committed
fix(secure-exec): normalize probe errors and remove stale logging test
1 parent d33f3c2 commit a7e3843

6 files changed

Lines changed: 84 additions & 6 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ Follow the style in `packages/secure-exec/src/index.ts`.
3838
- explain intent/why, not obvious mechanics
3939
- keep comments concise and consistent (`Set up`, `Transform`, `Wait for`, `Get`)
4040
- comment tricky ordering/invariants; skip noise
41+
- add inline comments and doc comments when behavior is non-obvious, especially where runtime/bridge/driver pieces depend on each other
4142

4243
## Skills
4344

docs-internal/todo/secure-exec.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@
126126
- [ ] Add resolver memoization (positive + negative) to avoid repeated miss probes across `require()`/`import()`.
127127
- `packages/secure-exec/src/package-bundler.ts`, `packages/secure-exec/src/shared/require-setup.ts`, `packages/secure-exec/src/index.ts`
128128

129+
- [ ] Document and verify package manager support for `node_modules` loading behavior.
130+
- Cover expected resolver behavior and known caveats for npm, pnpm, yarn, and bun installs.
131+
- Add/maintain compatibility fixtures that exercise transitive dependency loading across supported package manager layouts.
132+
129133
- [ ] Cap and cache `package.json` parsing in resolver paths.
130134
- `packages/secure-exec/src/package-bundler.ts`
131135

packages/secure-exec/src/bridge/process.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ const config = {
6262
version:
6363
(typeof _processConfig !== "undefined" && _processConfig.version) ||
6464
"v22.0.0",
65-
cwd: (typeof _processConfig !== "undefined" && _processConfig.cwd) || "/",
65+
cwd: (typeof _processConfig !== "undefined" && _processConfig.cwd) || "/root",
6666
env: (typeof _processConfig !== "undefined" && _processConfig.env) || {},
6767
argv:
6868
(typeof _processConfig !== "undefined" && _processConfig.argv) || [

packages/secure-exec/src/index.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,9 @@ const DEFAULT_ISOLATE_JSON_PAYLOAD_BYTES = 4 * 1024 * 1024;
172172
const MIN_CONFIGURED_PAYLOAD_BYTES = 1024;
173173
const MAX_CONFIGURED_PAYLOAD_BYTES = 64 * 1024 * 1024;
174174
const PAYLOAD_LIMIT_ERROR_CODE = "ERR_SANDBOX_PAYLOAD_TOO_LARGE";
175+
const DEFAULT_SANDBOX_CWD = "/root";
176+
const DEFAULT_SANDBOX_HOME = "/root";
177+
const DEFAULT_SANDBOX_TMPDIR = "/tmp";
175178

176179
class PayloadLimitError extends Error {
177180
constructor(payloadLabel: string, maxBytes: number, actualBytes: number) {
@@ -228,10 +231,18 @@ export class NodeProcess {
228231
this.networkAdapter = driver.network
229232
? wrapNetworkAdapter(driver.network, permissions)
230233
: createNetworkStub();
231-
const processConfig = options.processConfig ?? {};
234+
const processConfig = {
235+
...(options.processConfig ?? {}),
236+
};
237+
processConfig.cwd ??= DEFAULT_SANDBOX_CWD;
232238
processConfig.env = filterEnv(processConfig.env, permissions);
233239
this.processConfig = processConfig;
234-
this.osConfig = options.osConfig ?? {};
240+
const osConfig = {
241+
...(options.osConfig ?? {}),
242+
};
243+
osConfig.homedir ??= DEFAULT_SANDBOX_HOME;
244+
osConfig.tmpdir ??= DEFAULT_SANDBOX_TMPDIR;
245+
this.osConfig = osConfig;
235246
this.cpuTimeLimitMs = options.cpuTimeLimitMs;
236247
this.timingMitigation =
237248
options.timingMitigation ?? DEFAULT_TIMING_MITIGATION;

packages/secure-exec/src/node/module-access.ts

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ const MODULE_ACCESS_INVALID_CONFIG = "ERR_MODULE_ACCESS_INVALID_CONFIG";
1717
const MODULE_ACCESS_OUT_OF_SCOPE = "ERR_MODULE_ACCESS_OUT_OF_SCOPE";
1818
const MODULE_ACCESS_NATIVE_ADDON = "ERR_MODULE_ACCESS_NATIVE_ADDON";
1919

20-
const SANDBOX_APP_ROOT = "/app";
20+
const SANDBOX_APP_ROOT = "/root";
2121
const SANDBOX_NODE_MODULES_ROOT = `${SANDBOX_APP_ROOT}/node_modules`;
2222

2323
const VIRTUAL_DIR_MODE = 0o040755;
@@ -80,9 +80,59 @@ function isNativeAddonPath(pathValue: string): boolean {
8080
return pathValue.endsWith(".node");
8181
}
8282

83+
function collectOverlayAllowedRoots(hostNodeModulesRoot: string): string[] {
84+
const roots = new Set<string>([hostNodeModulesRoot]);
85+
const symlinkScanRoots = [hostNodeModulesRoot, path.join(hostNodeModulesRoot, ".pnpm", "node_modules")];
86+
87+
const addSymlinkTarget = (entryPath: string): void => {
88+
try {
89+
const target = fsSync.realpathSync(entryPath);
90+
roots.add(target);
91+
} catch {
92+
// Ignore broken symlinks.
93+
}
94+
};
95+
96+
const scanDirForSymlinks = (scanRoot: string): void => {
97+
let entries: fsSync.Dirent[] = [];
98+
try {
99+
entries = fsSync.readdirSync(scanRoot, { withFileTypes: true });
100+
} catch {
101+
return;
102+
}
103+
104+
for (const entry of entries) {
105+
const entryPath = path.join(scanRoot, entry.name);
106+
if (entry.isSymbolicLink()) {
107+
addSymlinkTarget(entryPath);
108+
continue;
109+
}
110+
if (entry.isDirectory() && entry.name.startsWith("@")) {
111+
let scopedEntries: fsSync.Dirent[] = [];
112+
try {
113+
scopedEntries = fsSync.readdirSync(entryPath, { withFileTypes: true });
114+
} catch {
115+
continue;
116+
}
117+
for (const scopedEntry of scopedEntries) {
118+
if (!scopedEntry.isSymbolicLink()) continue;
119+
addSymlinkTarget(path.join(entryPath, scopedEntry.name));
120+
}
121+
}
122+
}
123+
};
124+
125+
for (const scanRoot of symlinkScanRoots) {
126+
scanDirForSymlinks(scanRoot);
127+
}
128+
129+
return [...roots];
130+
}
131+
83132
export class ModuleAccessFileSystem implements VirtualFileSystem {
84133
private readonly baseFileSystem?: VirtualFileSystem;
85134
private readonly hostNodeModulesRoot: string | null;
135+
private readonly overlayAllowedRoots: string[];
86136

87137
constructor(baseFileSystem: VirtualFileSystem | undefined, options: ModuleAccessOptions) {
88138
this.baseFileSystem = baseFileSystem;
@@ -99,11 +149,17 @@ export class ModuleAccessFileSystem implements VirtualFileSystem {
99149
const nodeModulesPath = path.join(cwd, "node_modules");
100150
try {
101151
this.hostNodeModulesRoot = fsSync.realpathSync(nodeModulesPath);
152+
this.overlayAllowedRoots = collectOverlayAllowedRoots(this.hostNodeModulesRoot);
102153
} catch {
103154
this.hostNodeModulesRoot = null;
155+
this.overlayAllowedRoots = [];
104156
}
105157
}
106158

159+
private isWithinAllowedOverlayRoots(canonicalPath: string): boolean {
160+
return this.overlayAllowedRoots.some((root) => isWithinPath(canonicalPath, root));
161+
}
162+
107163
private isSyntheticPath(virtualPath: string): boolean {
108164
if (virtualPath === "/" || virtualPath === SANDBOX_APP_ROOT) {
109165
return true;
@@ -174,10 +230,13 @@ export class ModuleAccessFileSystem implements VirtualFileSystem {
174230

175231
try {
176232
const canonical = await fs.realpath(hostPath);
177-
if (!this.hostNodeModulesRoot || !isWithinPath(canonical, this.hostNodeModulesRoot)) {
233+
if (
234+
!this.hostNodeModulesRoot ||
235+
!this.isWithinAllowedOverlayRoots(canonical)
236+
) {
178237
throw createModuleAccessError(
179238
MODULE_ACCESS_OUT_OF_SCOPE,
180-
`resolved path '${canonical}' escapes '${this.hostNodeModulesRoot}'`,
239+
`resolved path '${canonical}' escapes overlay roots rooted at '${this.hostNodeModulesRoot}'`,
181240
);
182241
}
183242
if (isNativeAddonPath(canonical)) {

packages/secure-exec/src/package-bundler.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,9 @@ function getNodeModulesCandidatePackageDirs(
218218
): string[] {
219219
const candidates = new Set<string>();
220220
candidates.add(join(dir, "node_modules", packageName));
221+
candidates.add(
222+
join(dir, "node_modules", ".pnpm", "node_modules", packageName),
223+
);
221224

222225
// Match Node's "parent node_modules" lookup when the current directory is
223226
// already a node_modules folder.

0 commit comments

Comments
 (0)