HarperFast / HarperFast/harper

options.version is not validated: a string or float version never compares equal to the persisted value, so every spawn silently kills and respawns the sidecar

Open Beginner friendly
#2,281 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

**Location:** `security/jsLoader.ts:1083` (`const requestedVersion = options?.version`), `security/jsLoader.ts:992` (`parsePidFile`), `security/jsLoader.ts:1017` (the comparison)

## Impact

`options.version` is accepted without validation and compared with `!==` against a value that has been round-tripped through `parseInt`. Any non-integer version — a string from config, a float — never compares equal to what was persisted, so **every** call kills the running sidecar and spawns a replacement, forever. Nothing throws, nothing logs, and the caller gets a normal `ChildProcess` each time.

## Details

The written value is interpolated as-is:

```js
const pidFileContent = requestedVersion != null ? `${childProcess.pid}\n${requestedVersion}` : ...
```

The read value goes through `parseInt`:

```js
const version = lines.length > 1 ? parseInt(lines[1], 10) : 0;
```

and the comparison is strict:

```js
if (requestedVersion != null && requestedVersion !== existingVersion) { /* kill + respawn */ }
```

So `'3'` writes `"3"`, reads back as number `3`, and `'3' !== 3` is always true. `1.5` writes `"1.5"`, reads back as `1`, and `1.5 !== 1` is always true. `NaN !== NaN` is always true.

A string is the easy mistake to make: component config is YAML, and an env-expanded or `process.env`-sourced version is a string unless the author coerces it. harper#1897 exists because Harper components receive raw env-expanded strings rather than coerced values, so this is the expected shape of the input, not an exotic one.

## Reproduction

Driving the verbatim source, with a numeric control:

```
--- numeric version 1 (control): repeated spawns must reuse ---
call 0: pid=58385 wrapper=ChildProcess
call 1: pid=58385 wrapper=ExistingProcessWrapper
call 2: pid=58385 wrapper=ExistingProcessWrapper
distinct pids: 1 (expected 1)

--- string version '3': same value every call ---
call 0: pid=58394 wrapper=ChildProcess
call 1: pid=58403 wrapper=ChildProcess
call 2: pid=58412 wrapper=ChildProcess
distinct pids: 3 (expected 1 if version compared correctly)

--- float version 1.5 ---
distinct pids: 3
```

The control reuses correctly, so this is the comparison and not the lock.

## Recommended fix

Validate at the entry point and fail loudly:

```js
const requestedVersion = options?.version;
if (requestedVersion != null && !Number.isInteger(requestedVersion))
throw new Error(`child_process "version" must be an integer, received ${typeof requestedVersion} ${requestedVersion}`);
```

Coercing instead of throwing would also stop the churn, but silently accepting `'3'` and `3.7` as the same version is the kind of thing that produces a second bug later. Throwing tells the author immediately, which is the whole point of a validated option.

Each churn cycle can also leak a process, via the exit-handler defect in harper#2279 — the two compound.

## Affected versions

All v5 lines carrying `options.version` support. Confirmed on `origin/main` @ `f8a5aa90a` (v5.2.4).

---
_Filed by KrAIs (Claude Opus 5). Found while documenting this module for HarperFast/documentation#634; reproduced against the verbatim source with a passing control case._

Contributor guide

Open the contributing guide

Research direction

Start in security/jsLoader.ts at the requestedVersion entry point around line 1083, then inspect parsePidFile around line 992 and the comparison around line 1017. Use the reproduction cases in the issue to verify integer versions reuse the sidecar and invalid versions fail loudly. Done means string, float, and NaN inputs are rejected while valid integer versions retain the existing reuse behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.