HarperFast / HarperFast/harper
options.name is neither validated nor namespaced: '../' escapes <rootPath>/pids/, and two components choosing the same name silently adopt each other's process
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
**Location:** `security/jsLoader.ts:1078` (`const processName = options?.name`), `security/jsLoader.ts:1086-1089` (path construction)
## Impact
`options.name` is interpolated straight into a filesystem path and shared globally, with no validation and no per-component namespace. Two consequences:
1. **The lock namespace is global.** Two unrelated components that both name their sidecar `redis` share one lock, and the second one silently adopts the first one's process instead of starting its own.
2. **A name can escape `/pids/`.** `../` in the name writes the lock file to an arbitrary path outside the directory.
Neither is reported. In case 1 the second component believes its sidecar started.
## Details
```js
const processName = options?.name;
if (!processName) throw new Error(`Calling ${spawnFunction.name} in Harper must have a process "name" ...`);
...
const pidDir = join(basePath, 'pids');
mkdirSync(pidDir, { recursive: true });
const pidFilePath = join(pidDir, `${processName}.pid`);
```
The only check is truthiness. `join` normalizes `..` segments rather than rejecting them, so `name: '../../foo'` resolves above `pids/`. And nothing in the key includes the component identity, so the name is a global namespace shared by every component on the node.
The collision case is the one that will actually happen: `redis`, `postgres`, `worker`, `sidecar` are the names people pick, and two apps on one node picking the same one is not a stretch.
## Reproduction
```
--- name traversal escapes /pids/ ---
name '../../etc-adjacent' -> resolved lock: /etc-adjacent.pid
file written outside pids/? true
contents of /pids: []
--- two unrelated components, same name -> adopt each other's process ---
componentA got pid=59788 (ChildProcess)
componentB got pid=59788 (ExistingProcessWrapper) <-- adopted A's process
```
## Recommended fix
Two independent changes:
1. **Validate the name.** Reject anything that isn't a safe single path component — e.g. `/^[A-Za-z0-9._-]{1,64}$/`, explicitly excluding `.` and `..`. Throw with the offending value; this is caller-supplied config, so a loud failure is right. Belt-and-braces: `resolve()` the joined path and assert it's still inside `pidDir`.
2. **Namespace by component.** Key the lock on `(component, name)` rather than `name` alone — `/pids//.pid`, or a sanitized `__.pid`. `ApplicationScope` already carries `name` (`components/ApplicationScope.ts:37`), so the identity is available at the point `createSpawn` is constructed.
Note that (2) is a behavior change for anyone deliberately relying on cross-component sharing of one sidecar. Nothing documents that as supported, but it is worth a deliberate call rather than an accident.
## Security framing
The traversal half is a file-confinement gap of the same family as harper#1929 (`checkAllowedModulePath` prefix match without a separator boundary). It is bounded — the write is a small PID file to an attacker-chosen path, not arbitrary content — and it presumes component code that is already deliberately hostile, which has larger problems available to it (see harper#2284). Filed as a normal issue on that basis; specifics in `Security Notes`.
## Affected versions
All v5 lines. Confirmed on `origin/main` @ `f8a5aa90a` (v5.2.4).
---
_Filed by KrAIs (Claude Opus 5). Found while documenting this module for HarperFast/documentation#634; both halves reproduced against the verbatim source._
Contributor guide
Research direction
Start at security/jsLoader.ts:1078 and the path construction at lines 1086-1089 to trace how options.name becomes the PID path. Then inspect createSpawn and ApplicationScope at components/ApplicationScope.ts:37 to understand the available component identity. Done means traversal names are rejected and same-name sidecars from different components no longer adopt one another's process; verify both reproduction cases.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- node.js, typescript
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100