HarperFast / HarperFast/harper

allowedDirectory containment uses a raw string prefix, so a sibling directory sharing a name prefix passes the check

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

`security/jsLoader.ts:1202-1211`:

```js
if (moduleUrl.startsWith('file:')) {
let path = fileURLToPath(moduleUrl);
try { path = realpathSync(path); } catch {}
if (!allowedPath || path.startsWith(allowedPath)) { return; }
throw new Error(`Can not load module at ${path} outside of allowed path ${allowedPath}`);
}
```

`allowedPath` is the realpath of the component directory (`components/componentLoader.ts:729`), and containment is a bare `String.prototype.startsWith`. It is not separator-aware, so an application rooted at `/components/foo` can load `/components/foo-other/file.js` — the string test passes even though the file is outside the application's tree.

With `applications.allowedDirectory: app` (the default in production since v5.0.4), that means the directory restriction can be satisfied by any sibling path sharing a name prefix.

Suggested fix — a `path.relative` containment check:

```js
const rel = relative(allowedPath, path);
if (!rel || (!rel.startsWith('..' + sep) && rel !== '..' && !isAbsolute(rel))) return;
```

Worth a test for the sibling-prefix case specifically (`/x/app` vs `/x/app-other`), since that is the one a prefix comparison gets wrong.

Related: #2505 covers a separate gap in the same check — the CommonJS path does not call it at all.

Found while documenting module loading in HarperFast/documentation#664 (review comment from @kriszyp). That PR describes `allowedDirectory` as a configuration guardrail rather than a security boundary; it can be stated more strongly once this and #2505 are fixed.

sent with Claude Opus 5

Contributor guide

Open the contributing guide

Research direction

Start in security/jsLoader.ts:1202-1211 and compare allowedPath construction in components/componentLoader.ts:729. Trace the file-URL loading path, then add a regression test for sibling prefixes such as /x/app versus /x/app-other. Done means paths outside the allowed directory are rejected while paths within it remain allowed.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, nodejs
Domain
backend, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.