fix: replace node-persist with minimal in-repo file storage (#1125)

* fix: replace node-persist with minimal in-repo file storage

Implements the scope agreed in #1107: HAPFileStorage keeps node-persist
0.0.12's on-disk layout byte-for-byte and its four synchronous operations
(initSync, getItem, setItemSync, removeItemSync); everything else from the
removed node-persist API throws with a clear error. Drops the deprecated
q dependency along with node-persist, mkdirp@0.5 and the node-persist
build/type workarounds.

Claude-Session: https://claude.ai/code/session_01DpHuagGzRQVzatVmNiWa7p

* fix: clearer error for non-string storage path values

Claude-Session: https://claude.ai/code/session_01DpHuagGzRQVzatVmNiWa7p

* fix: address review feedback on HAPFileStorage

Use an optional catch binding for the unused error variable and clarify
the dir option documentation (absolute if provided, relative default).

Claude-Session: https://claude.ai/code/session_01DpHuagGzRQVzatVmNiWa7p

* fix: make storage writes atomic and ignore directories on load

Writes go to a temporary file which is renamed over the target, so an
interrupted write can no longer truncate a file holding pairing data.
Directories inside the storage directory are skipped when loading
instead of aborting startup with EISDIR.

Claude-Session: https://claude.ai/code/session_01DpHuagGzRQVzatVmNiWa7p

* test: cover parent directory creation for nested keys

Asserts the temporary file is written beside its target, which is what
keeps the replacing rename atomic.

* test: cover the default storage directory branch of HAPStorage

Every existing HAPStorage test sets a custom storage path, and jest.setup.ts
sets one on the singleton, so the branch which initialises into the default
"persist" directory was no longer executed by any test.

* fix: require storage keys to be a plain filename

A key was read as a path, so "nested/key.json" created a subdirectory and
"../escaped.json" wrote outside the storage directory. Since initSync now skips
directories, a nested key wrote successfully and then read back as undefined
after a restart, where node-persist 0.0.12 at least failed loudly with EISDIR.

Rejecting such keys makes the documented one-file-per-key layout an invariant.
Nothing in HAP-NodeJS uses a key containing a separator.

* fix: close three more ways a stored value could be lost silently

Keys starting with a dot are now rejected. initSync skips dotfiles, so
`.hidden` was written, served from memory while the process ran, and gone
after a restart: the same silent loss as a key naming a subdirectory,
reached from a different direction.

initSync now skips anything which is not a regular file, resolving
symlinks. A symlink pointing at a directory does not report as one to
readdir, so it reached readFileSync and aborted startup with the EISDIR
that check exists to prevent. A link to a file still loads.

The in-memory value is only replaced once the write reached the disk. A
failed write left memory holding a value which was not saved, and since
everything is served from memory that reads as an accessory which is
paired now and unpaired after a restart.

Author: tim-fin

.github/node-persist-ignore.js

@@ -3,6 +3,7 @@ import type { Config } from "jest";
 const config: Config = {
   "preset": "ts-jest",
   "testEnvironment": "node",
+  "setupFilesAfterEnv": ["<rootDir>/jest.setup.ts"],
   "transform": {
     "^.+\\.tsx?$": ["ts-jest", {
       "tsconfig": "tsconfig.spec.json",

__mocks__/node-persist.ts

@@ -0,0 +1,14 @@
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { HAPStorage } from "./src/lib/model/HAPStorage";
+
+// Redirect the HAPStorage singleton into a fresh temporary directory for every test file,
+// so suites which exercise persistence (e.g. through Accessory.publish()) never touch
+// a real "persist" directory inside the repository.
+const storageDir = fs.mkdtempSync(path.join(os.tmpdir(), "hap-nodejs-jest-"));
+HAPStorage.setCustomStoragePath(storageDir);
+
+afterAll(() => {
+  fs.rmSync(storageDir, { recursive: true, force: true });
+});

jest.config.ts

@@ -14,7 +14,7 @@
     "check": "npm install && npm outdated",
     "clean": "rimraf dist && rimraf coverage",
     "lint": "eslint 'src/**/*.{js,ts,json}'",
-    "build": "rimraf dist && tsc && node .github/node-persist-ignore.js",
+    "build": "rimraf dist && tsc",
     "generate-definitions": "ts-node src/lib/definitions/generate-definitions.ts",
     "prepublishOnly": "npm run build",
     "postpublish": "npm run clean",
@@ -58,7 +58,6 @@
     "debug": "^4.4.3",
     "fast-srp-hap": "^2.0.4",
     "futoin-hkdf": "^1.5.3",
-    "node-persist": "^0.0.12",
     "source-map-support": "^0.5.21",
     "tslib": "^2.8.1",
     "tweetnacl": "^1.0.3"

jest.setup.ts

@@ -7,6 +7,7 @@ import { readFileSync } from "node:fs";
  * @group Utils
  */
 export * as uuid from "./lib/util/uuid";
+export * from "./lib/model/HAPFileStorage";
 export * from "./lib/model/HAPStorage";
 export * from "./lib/Accessory";
 export * from "./lib/Bridge";

package-lock.json

@@ -6,8 +6,8 @@ import { AssertionError } from "assert";
 describe("AccessoryInfo", () => {
   describe("#load()", () => {
     it("should default category to Categories.OTHER when missing", () => {
-      const mockStorage = HAPStorage.storage();
-      (mockStorage.getItem as jest.Mock).mockReturnValueOnce({
+      const username = "0E:AE:FC:45:7B:91";
+      HAPStorage.storage().setItemSync(AccessoryInfo.persistKey(username), {
         displayName: "Test",
         pincode: "123-45-678",
         signSk: "aa".repeat(64),
@@ -16,7 +16,7 @@ describe("AccessoryInfo", () => {
         // category intentionally omitted
       });
 
-      const info = AccessoryInfo.load("0E:AE:FC:45:7B:91");
+      const info = AccessoryInfo.load(username);
       expect(info).not.toBeNull();
       expect(info!.category).toBe(Categories.OTHER);
       expect(typeof info!.category).toBe("number");

package.json

@@ -230,7 +230,7 @@ export class AccessoryInfo {
       signPk: this.signPk.toString("hex"),
       pairedClients: {},
       // moving permissions into an extra object, so there is nothing to migrate from old files.
-      // if the legacy node-persist storage should be upgraded some time, it would be reasonable to combine the storage
+      // if the legacy storage format should be upgraded some time, it would be reasonable to combine the storage
       // of public keys (pairedClients object) and permissions.
       pairedClientsPermission: {},
       configVersion: this.configVersion,