zwave-js / zwave-js-ui / 27953507452

8%

push

github

fix: confine store paths against the resolved store dir (#4685)

Follow-up hardening on top of the store path-traversal fix in #4678.
Three issues surfaced in review of that change:

### 1. `storeDir` was not canonicalized (functional regression — could
break real deployments)

`getSafePath` → `assertRealPathInStore` compared a `realpath`-resolved
target against the **unresolved** `storeDir`:

```ts
const real = await realpath(current)
if (real !== storeDir && !real.startsWith(storeDir + path.sep)) // storeDir is lexical
```

`storeDir` is `process.env.STORE_DIR || joinPath(...)` and is never
resolved (`joinPath` is just `path.join`). When the configured store
path itself traverses a symlink — common in this project's
self-hosted/Docker/NAS audience (bind-mounted `/data`→`/mnt/...`,
symlinked `$HOME`, `/tmp`→`/private/tmp` on macOS) — `realpath(target)`
no longer starts with the lexical `storeDir`, so **every** store
read/write/delete/export/restore is rejected with `Path not allowed`.

The pre-#4678 code compared lexical-to-lexical, so it worked; switching
to `realpath` introduced the mismatch. Fixed by comparing the resolved
target against the **resolved** store root.

### 2. The confinement logic had no unit tests

`getSafePath` / `assertRealPathInStore` (the most security-critical code
in the change) lived module-private in `app.ts` and were untested — only
`assertNoEscapingSymlinks` had coverage. Extracted them into `utils.ts`
as `resolveSafeStorePath` / `assertRealPathInStore` (taking `storeDir`
as a parameter, matching the `buildLogConfig(config, logsDir)` DI
pattern introduced in #4678), and added tests — including a regression
test for the symlinked-`storeDir` case above.

### 3. `store-multi` export probed raw paths before validating

The export loop ran `lstat(f)` / `realpath(f)` on raw user input before
`getSafePath`, letting an authenticated user probe arbitrary filesystem
path existence/type. It now confines each path before touching the... (continued)

436 of 564 branches covered (77.3%)

Branch coverage included in aggregate %.

76 of 94 new or added lines in 2 files covered. (80.85%)

4523 of 25099 relevant lines covered (18.02%)

1.13 hits per line