HarperFast / HarperFast/harper
[security] The child_process substitution — and the allowedSpawnCommands allowlist it delivers — is not enforced on CJS require, compartment mode, or natively-loaded npm dependencies
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
**Category:** `sandbox-escape` / control-not-enforced · **CWE-693** (protection mechanism failure)
**Location:** `security/jsLoader.ts:380` (CJS require), `:769-770` (compartment importHook), `:492-512` (`shouldUseApplicationLoader`), `:921` (`fork`)
## Impact
`reference/configuration/options.md:360` describes `applications.allowedSpawnCommands` as listing "the specific commands that can be spawned by the application … this is to protect against malicious code spawning processes", and the v5 migration guide states that "any `spawn`, `exec`, or `execFile` may only spawn executables or commands that have been registered". That control is delivered entirely by substituting `child_process` at module-load time (`security/jsLoader.ts:918-929`).
On several reachable paths — including the **default configuration** — application code receives the *real* `child_process` module instead. When it does, all four guarantees vanish at once: the `allowedSpawnCommands` allowlist, the mandatory `options.name`, the single-process PID lock, and the `execSync` block.
The default `allowedSpawnCommands` is empty, so the intended posture is deny-all. On these paths it is allow-all.
## Path 1 — CommonJS `require` under the VM loader (default `moduleLoader: vm-current-context`)
`cjsRequire` calls `checkAllowedModulePath` only for `.node` files under `file://`. Bare and `node:`-prefixed specifiers fall through to the real `require`:
```js
const cjsRequire = (spec) => {
const resolvedUrl = resolveModule(spec, url);
if (resolvedUrl === 'harper') return getHarperExports(scope);
if (resolvedUrl.startsWith('file://')) {
if (resolvedUrl.endsWith('.node')) { checkAllowedModulePath(...); ... }
...
}
return require(resolvedUrl); // <-- real module, no check
};
```
`createRequire(...).resolve('child_process')` returns `'child_process'` and `.resolve('node:child_process')` returns `'node:child_process'` — neither starts with `file://`, so both reach the last line. The ESM path is *not* affected: `createModule` (`:684`) does call `checkAllowedModulePath` and does return the replacement. So the same specifier is constrained under `import` and unconstrained under `require`, in the same loader, in the default configuration.
## Path 2 — `applications.moduleLoader: compartment`
The compartment `importHook` calls `checkAllowedModulePath` and then **discards its return value**, importing the real module instead of the replacement it was just handed:
```js
} else {
checkAllowedModulePath(moduleSpecifier, scope.allowedPath);
const moduleExports = await import(moduleSpecifier); // <-- replacement discarded
```
`checkAllowedModulePath` returns `REPLACED_BUILTIN_MODULES[simpleName]` (`:1152`) precisely so the caller can substitute. Every other call site uses the return value; this one throws away the substitution while keeping the allowlist check, so compartment mode enforces `allowedBuiltinModules` but never `allowedSpawnCommands`.
## Path 3 — npm dependencies under the default `dependencyLoader: auto`
`shouldUseApplicationLoader` returns `false` for any package under `node_modules` that does not itself declare `harper` as a dependency (`:508-511`, `packageDependsOnHarper`). `createModule` then skips the private-global branch and does a native `import(url)`. That package — and its whole transitive tree — runs under the real Node loader with the real `child_process`.
This is the path that matters most for the stated threat model: it is not the operator's own code, it is arbitrary third-party dependencies, and it is the default.
## Path 4 — `fork` is unconditionally allowed and spawns an unsandboxed process
```js
fork: createSpawn(child_process.fork, true), // this is launching node, so deemed safe
```
`alwaysAllow: true` skips the allowlist entirely, and the target module path is unconstrained. The forked child is a plain Node process with no jsLoader, no substitution and no frozen intrinsics — so application code can reach full, unsandboxed Node by forking a file it ships:
```
ALLOWED_COMMANDS is {sleep,echo}; ./evil.cjs is NOT in it.
[forked child] running as a plain node process. sandboxed? NO - real execSync available
[forked child] execSync("id") -> uid=502(kris) gid=20(staff) groups=20(st
```
The `deemed safe` comment reasons about *what binary* is launched; the sandbox's purpose is to constrain *what the application can do*, and forking node escapes that regardless of the binary. If this is intended, it should be documented as an explicit hole in the control rather than left as an inline comment.
## Relationship to existing issues
- **harper#1924** (allowlist bypassable via shell metacharacters) narrows the allowlist on the substituted path. This issue is that on the paths above there is no allowlist to narrow. Note also that #1924's specific vector is currently *unreachable*, because `exec` cannot be called at all through the substitute (harper#2278) — fixing #2278 activates #1924.
- **harper#1929** (`checkAllowedModulePath` prefix match) is a boundary defect in the same function, on paths where it *is* consulted.
## Recommended fix
1. **Path 1:** route bare/`node:` specifiers in `cjsRequire` through `checkAllowedModulePath` and return the replacement when one exists, matching what `createModule` already does for ESM.
2. **Path 2:** use the return value — `const replaced = checkAllowedModulePath(...); if (replaced) return synthetic(replaced);` before falling back to `import`.
3. **Path 3:** decide explicitly what `dependencyLoader: auto` is promising. Either the substitution is a security control, in which case builtin replacement must apply to natively-loaded dependencies too (or `auto` must not be the default), or it is a convenience for harper-aware components, in which case the documentation should stop describing it as protection against malicious code. It cannot be both.
4. **Path 4:** either constrain `fork` (validate the target, or gate it behind its own config option) or document it as an intentional escape hatch.
5. Regardless of the above, `reference/configuration/options.md` and the v5 migration guide should describe the control's actual reach. HarperFast/documentation#634 currently documents the observed behavior; that is a stopgap, not the answer.
## Affected versions
All v5 lines. Confirmed against `origin/main` @ `f8a5aa90a` (v5.2.4) by source inspection plus direct execution of the wrapper logic; path 4 executed end-to-end.
---
_Filed by KrAIs (Claude Opus 5). Found while documenting the substituted `child_process` module for HarperFast/documentation#634. Exploit-specific detail in `Security Notes` on project #8._
Contributor guide
Research direction
Read security/jsLoader.ts at the cjsRequire, importHook, shouldUseApplicationLoader, and fork entry points, then compare their behavior with the documented guarantees in reference/configuration/options.md and the v5 migration guide. Trace or reproduce each affected loading path described in the issue. Done means the intended policy is consistently enforced on all paths, or every intentional exception is explicitly documented.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, nodejs
- Domain
- security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100