HarperFast / HarperFast/harper

A bare (no '=value') HARPER_BUILTIN_COMPONENTS entry crashes boot instead of being rejected/skipped

Open
#2,028 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

## Summary

`HARPER_BUILTIN_COMPONENTS` is a comma-separated `name=packageIdentifier` list. `getEnvBuiltInComponents()` (`components/Application.ts:1876`) destructures each entry on `=` and returns `packageIdentifier: undefined` for a bare name (no `=value`) — it doesn't validate or reject that shape.

Two separate call sites then dereference the resulting `undefined` packageIdentifier without a guard:

- `components/Application.ts:1129`, inside `installApplications()`, which runs at boot.
- `server/jobs/jobProcess.ts:39`, `packageIdentifier.startsWith('@/')`.

## Concrete failure scenario

An operator (or CI config) sets `HARPER_BUILTIN_COMPONENTS=secretCustody` (forgetting the `=value` part, which the parser otherwise permits as valid syntax). `installApplications()` dereferences the undefined packageIdentifier at boot and throws a `TypeError` — the whole Harper process fails to start, from what looks like a harmless typo.

## Suggested fix

Reject or skip malformed (missing `=value`) definitions at the source, in `getEnvBuiltInComponents()` itself, rather than guarding each of the (at least two) consumer call sites individually.

## Provenance

Found while writing `integrationTests/server/qa577-upgrade-builtins.test.ts` (a stand-in `@/`-prefixed built-in registration for testing PR #1814's config backfill) — trying a real npm-installable name first (`lodash`) surfaced the parser's tolerance of a bare name, which led to finding this. Orthogonal to #1814's own diff; not exercised by that test file, just documented there.

🤖 Filed by Claude (dispatch pr-fix agent) while addressing review comments on #1972.

Contributor guide

Open the contributing guide

Research direction

Start in getEnvBuiltInComponents() at components/Application.ts:1876, then inspect its consumers at components/Application.ts:1129 and server/jobs/jobProcess.ts:39. Use integrationTests/server/qa577-upgrade-builtins.test.ts for test context and add coverage showing that a bare HARPER_BUILTIN_COMPONENTS entry is rejected or skipped without causing a boot-time TypeError.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.