Skip to content

options.name is neither validated nor namespaced: '../' escapes <rootPath>/pids/, and two components choosing the same name silently adopt each other's process #2283

Description

@kriszyp

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 <rootPath>/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

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 <rootPath>/pids/ ---
  name '../../etc-adjacent' -> resolved lock: <scratch>/etc-adjacent.pid
  file written outside pids/? true
  contents of <base>/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 — <rootPath>/pids/<component>/<name>.pid, or a sanitized <component>__<name>.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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Fields

    Priority

    P2

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions