From 4edb662b63338132fcad6c138a7cf815b0cb09c7 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Sat, 8 Aug 2026 02:19:30 +0200 Subject: [PATCH] fix(plugin-kit): regSubKeys could never return a subkey MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found on hardware by the GOG plugin's own parity gate, on a box with exactly one GOG game installed: HKLM\SOFTWARE\WOW6432Node\GOG.com\Games -> 1 subkey (IRON NEST ...) host's built-in scanner: 1 entry plugin: detect: absent, 0 games parity FAILED - 1 missing, exit 1 `reg.exe` ALWAYS echoes the full hive name in its output rows, never the abbreviation it was given: query `HKLM\SOFTWARE\...` and every line comes back `HKEY_LOCAL_MACHINE\SOFTWARE\...`. regSubKeys built its match prefix from the `HKLM\...` string it was handed, so no line ever matched and it returned `[]` — on every machine, for every key, always. Measured verbatim on .173: reg.exe: [HKEY_LOCAL_MACHINE\SOFTWARE\WOW6432Node\GOG.com\Games\2013434102] regSubKeys: [] Its only consumer is the GOG plugin, so the symptom was "GOG reports no games installed" rather than an error — the same shape as the SQLite reader in 0.3.1: a total failure that every layer degrades into an empty library. The contract was wrong too, and the hive bug hid it. regSubKeys returned whole key PATHS while the GOG plugin uses each result as a bare NAME (`const key = \`${GAMES_KEY}\\${id}\``, and the subkey name IS the product id that becomes `external_id`). Even with the prefix fixed, paths would have composed nonsense keys. It now returns names, which is what the sole consumer and its own comment always assumed. Parsing is split into an exported `parseRegSubKeys(stdout, key)` for the same reason `parseRegQuery` is exported — this is a text format that breaks quietly, and it had NO test coverage at all. Six added, using the verbatim .173 output: names not paths, multiple subkeys, grandchildren ignored, the queried key is not its own subkey, case-insensitivity, and empty/error input. Four of the six FAIL against the old behaviour. 0.3.1 -> 0.3.2. Gates: biome clean, tsc clean, 67/67 tests, build clean. --- plugin-kit/package.json | 2 +- plugin-kit/src/library/parsers/index.ts | 5 +- plugin-kit/src/library/parsers/registry.ts | 43 +++++-- plugin-kit/test/library-parsers.test.ts | 134 ++++++++++++++++++--- 4 files changed, 154 insertions(+), 30 deletions(-) diff --git a/plugin-kit/package.json b/plugin-kit/package.json index 4906d1f5..d1058736 100644 --- a/plugin-kit/package.json +++ b/plugin-kit/package.json @@ -1,6 +1,6 @@ { "name": "@punktfunk/plugin-kit", - "version": "0.3.1", + "version": "0.3.2", "description": "Effect-based framework for punktfunk plugins: lifecycle runtime, config/state, sync engine, UI serving, CLI scaffold, and browser helpers.", "type": "module", "license": "MIT OR Apache-2.0", diff --git a/plugin-kit/src/library/parsers/index.ts b/plugin-kit/src/library/parsers/index.ts index 697f2f12..2a20f2dc 100644 --- a/plugin-kit/src/library/parsers/index.ts +++ b/plugin-kit/src/library/parsers/index.ts @@ -32,13 +32,13 @@ export { } from "./http.js"; export { parseRegQuery, + parseRegSubKeys, + type RegValue, regQueryValue, regQueryValues, regSubKeys, - type RegValue, validRegKey, } from "./registry.js"; -export { openReadOnly, type ReadOnlyDb, withReadOnlyDb } from "./sqlite.js"; export { crc32, parseShortcuts, @@ -46,6 +46,7 @@ export { shortcutAppId, shortcutGameId, } from "./shortcuts.js"; +export { openReadOnly, type ReadOnlyDb, withReadOnlyDb } from "./sqlite.js"; export { steamLibraryDirs, steamRoots, diff --git a/plugin-kit/src/library/parsers/registry.ts b/plugin-kit/src/library/parsers/registry.ts index 4b420d66..5ebafe88 100644 --- a/plugin-kit/src/library/parsers/registry.ts +++ b/plugin-kit/src/library/parsers/registry.ts @@ -59,17 +59,46 @@ export const regQueryValue = (key: string, name: string): string | undefined => regQueryValues(key).find((v) => v.name.toLowerCase() === name.toLowerCase()) ?.data; -/** The immediate SUBKEY paths under one HKLM key (GOG lists one subkey per installed game). */ +/** + * `reg.exe` always echoes the FULL hive name in its output rows, never the abbreviation it was + * given: query `HKLM\SOFTWARE\…` and every line comes back `HKEY_LOCAL_MACHINE\SOFTWARE\…`. + */ +const HKLM_FULL = "HKEY_LOCAL_MACHINE\\"; + +/** + * Parse `reg.exe query ` output into the immediate subkey NAMES under `key`. + * + * Exported for tests, like {@link parseRegQuery}, and for the same reason — this is a text format + * that quietly breaks, and it did: the previous version matched output lines against the + * abbreviated `HKLM\…` prefix it was handed, while reg.exe prints `HKEY_LOCAL_MACHINE\…`. Nothing + * ever matched, so it returned `[]` on every machine, forever, and the one plugin that uses it + * (GOG) reported "no games installed" instead of failing. See the regSubKeys tests. + * + * Returns NAMES, not paths: the sole consumer composes `${key}\\${name}`, and a GOG subkey name IS + * the product id that becomes the entry's `external_id`. + */ +export const parseRegSubKeys = (stdout: string, key: string): string[] => { + const full = key.toUpperCase().startsWith(HKLM) + ? HKLM_FULL + key.slice(HKLM.length) + : key; + const prefix = `${full.toLowerCase()}\\`; + return ( + stdout + .split(/\r?\n/) + .map((l) => l.trim()) + .filter((l) => l.toLowerCase().startsWith(prefix)) + .map((l) => l.slice(full.length + 1)) + // Immediate children only — a deeper path still starts with the prefix. + .filter((name) => name !== "" && !name.includes("\\")) + ); +}; + +/** The immediate SUBKEY NAMES under one HKLM key (GOG lists one subkey per installed game). */ export const regSubKeys = (key: string): string[] => { if (!validRegKey(key)) return []; const out = run(["query", key]); if (out === undefined) return []; - const prefix = `${key.toLowerCase()}\\`; - return out - .split(/\r?\n/) - .map((l) => l.trim()) - .filter((l) => l.toLowerCase().startsWith(prefix)) - .filter((l) => !l.slice(key.length + 1).includes("\\")); + return parseRegSubKeys(out, key); }; /** diff --git a/plugin-kit/test/library-parsers.test.ts b/plugin-kit/test/library-parsers.test.ts index ae290c7d..55ead4bf 100644 --- a/plugin-kit/test/library-parsers.test.ts +++ b/plugin-kit/test/library-parsers.test.ts @@ -5,23 +5,24 @@ // and launches nothing. Where a Rust test exists, its assertions are carried over verbatim — the // per-plugin parity harness (design M5) then checks the whole pipeline against a live host, but // these catch a drift long before that. -import { describe, expect, test } from "bun:test"; + import { Database } from "bun:sqlite"; +import { describe, expect, test } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; import { confinedJoin, crc32, + fileUrl, findGridArtFile, findLocalArtFile, - fileUrl, gridFilenames, isSteamTool, - withReadOnlyDb, openReadOnly, parseAppManifest, parseRegQuery, + parseRegSubKeys, parseShortcuts, readTextCapped, shortcutAppId, @@ -29,6 +30,7 @@ import { steamCdnUrl, vdfPaths, vdfValue, + withReadOnlyDb, } from "../src/library/parsers/index.js"; const tmp = (name: string): string => { @@ -85,7 +87,9 @@ describe("text VDF / ACF", () => { }); test("isSteamTool keeps runtimes out of a game library", () => { - expect(isSteamTool(228980, "Steamworks Common Redistributables")).toBe(true); + expect(isSteamTool(228980, "Steamworks Common Redistributables")).toBe( + true, + ); expect(isSteamTool(1628350, "Steam Linux Runtime 3.0 (sniper)")).toBe(true); expect(isSteamTool(999, "Proton 9.0")).toBe(true); expect(isSteamTool(999, "SteamVR")).toBe(true); @@ -109,7 +113,12 @@ describe("binary shortcuts.vdf", () => { parts.push(0); }; const i32 = (v: number) => { - parts.push(v & 0xff, (v >>> 8) & 0xff, (v >>> 16) & 0xff, (v >>> 24) & 0xff); + parts.push( + v & 0xff, + (v >>> 8) & 0xff, + (v >>> 16) & 0xff, + (v >>> 24) & 0xff, + ); }; parts.push(0x00); cstr("shortcuts"); @@ -205,9 +214,13 @@ describe("path confinement", () => { path.join(base, "bin", "game.exe"), ); // The three shapes a crafted goggame-*.info would use to point elsewhere. - expect(confinedJoin(base, "../../windows/system32/cmd.exe")).toBeUndefined(); + expect( + confinedJoin(base, "../../windows/system32/cmd.exe"), + ).toBeUndefined(); expect(confinedJoin(base, "/etc/passwd")).toBeUndefined(); - expect(confinedJoin(base, "C:\\Windows\\system32\\cmd.exe")).toBeUndefined(); + expect( + confinedJoin(base, "C:\\Windows\\system32\\cmd.exe"), + ).toBeUndefined(); expect(confinedJoin(base, "")).toBeUndefined(); }); }); @@ -237,8 +250,14 @@ describe("art locations", () => { test("grid filenames follow Steam's per-kind naming", () => { expect(gridFilenames(570, "portrait")).toEqual(["570p.png", "570p.jpg"]); - expect(gridFilenames(570, "hero")).toEqual(["570_hero.png", "570_hero.jpg"]); - expect(gridFilenames(570, "logo")).toEqual(["570_logo.png", "570_logo.jpg"]); + expect(gridFilenames(570, "hero")).toEqual([ + "570_hero.png", + "570_hero.jpg", + ]); + expect(gridFilenames(570, "logo")).toEqual([ + "570_logo.png", + "570_logo.jpg", + ]); expect(gridFilenames(570, "header")).toEqual(["570.png", "570.jpg"]); }); @@ -307,8 +326,12 @@ describe("openReadOnly", () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "pf-kit-sqlite-")); const file = path.join(dir, "pga.db"); const seed = new Database(file); - seed.run("CREATE TABLE games (id INTEGER PRIMARY KEY, name TEXT, installed INT)"); - seed.run("INSERT INTO games (id, name, installed) VALUES (1, 'Ubisoft Connect', 1)"); + seed.run( + "CREATE TABLE games (id INTEGER PRIMARY KEY, name TEXT, installed INT)", + ); + seed.run( + "INSERT INTO games (id, name, installed) VALUES (1, 'Ubisoft Connect', 1)", + ); seed.close(); try { return use(file); @@ -321,9 +344,9 @@ describe("openReadOnly", () => { withDb((file) => { const db = openReadOnly(file); expect(db).toBeDefined(); - expect(db?.query("SELECT id, name FROM games WHERE installed = 1")).toEqual([ - { id: 1, name: "Ubisoft Connect" }, - ]); + expect( + db?.query("SELECT id, name FROM games WHERE installed = 1"), + ).toEqual([{ id: 1, name: "Ubisoft Connect" }]); db?.close(); }); }); @@ -337,7 +360,9 @@ describe("openReadOnly", () => { seed.run("INSERT INTO games (id) VALUES (7)"); seed.close(); try { - expect(openReadOnly(file)?.query("SELECT id FROM games")).toEqual([{ id: 7 }]); + expect(openReadOnly(file)?.query("SELECT id FROM games")).toEqual([ + { id: 7 }, + ]); } finally { fs.rmSync(dir, { recursive: true, force: true }); } @@ -345,16 +370,20 @@ describe("openReadOnly", () => { test("withReadOnlyDb reads, then closes", () => { withDb((file) => { - expect(withReadOnlyDb(file, (h) => h.query("SELECT name FROM games"))).toEqual([ - { name: "Ubisoft Connect" }, - ]); + expect( + withReadOnlyDb(file, (h) => h.query("SELECT name FROM games")), + ).toEqual([{ name: "Ubisoft Connect" }]); }); }); // The "not installed" contract — an absent file is `undefined`, never a throw. test("absent file is undefined, not an error", () => { - expect(openReadOnly(path.join(os.tmpdir(), "pf-kit-nope", "pga.db"))).toBeUndefined(); - expect(withReadOnlyDb(path.join(os.tmpdir(), "pf-kit-nope", "pga.db"), () => 1)).toBeUndefined(); + expect( + openReadOnly(path.join(os.tmpdir(), "pf-kit-nope", "pga.db")), + ).toBeUndefined(); + expect( + withReadOnlyDb(path.join(os.tmpdir(), "pf-kit-nope", "pga.db"), () => 1), + ).toBeUndefined(); }); // Schema drift degrades to no rows rather than taking the plugin down. @@ -366,3 +395,68 @@ describe("openReadOnly", () => { }); }); }); + +// Subkey enumeration, against the output reg.exe ACTUALLY prints. +// +// This had no coverage and was broken end to end: it matched lines against the abbreviated +// `HKLM\…` prefix it was handed, but reg.exe echoes `HKEY_LOCAL_MACHINE\…`. Nothing ever matched, +// so it returned [] on every machine, and the GOG plugin — its only consumer — reported "no games +// installed" rather than failing. Caught on hardware by the parity gate: the host's built-in +// scanner found IRON NEST, the plugin found nothing. +// +// The fixture is the verbatim output from .173 (a blank line, then one subkey row). +describe("parseRegSubKeys", () => { + const KEY = "HKLM\\SOFTWARE\\WOW6432Node\\GOG.com\\Games"; + + test("returns subkey NAMES from real reg.exe output", () => { + const stdout = [ + "", + "HKEY_LOCAL_MACHINE\\SOFTWARE\\WOW6432Node\\GOG.com\\Games\\2013434102", + "", + ].join("\r\n"); + // The name is the GOG product id, and the consumer composes `${KEY}\\${name}`. + expect(parseRegSubKeys(stdout, KEY)).toEqual(["2013434102"]); + }); + + test("several subkeys, in order", () => { + const base = "HKEY_LOCAL_MACHINE\\SOFTWARE\\WOW6432Node\\GOG.com\\Games"; + const stdout = ["", `${base}\\1207658930`, `${base}\\2013434102`].join( + "\r\n", + ); + expect(parseRegSubKeys(stdout, KEY)).toEqual(["1207658930", "2013434102"]); + }); + + // reg.exe /s output nests deeper; only immediate children are subkeys of this key. + test("ignores grandchildren", () => { + const base = "HKEY_LOCAL_MACHINE\\SOFTWARE\\WOW6432Node\\GOG.com\\Games"; + const stdout = [ + "", + `${base}\\2013434102`, + `${base}\\2013434102\\tasks`, + ].join("\r\n"); + expect(parseRegSubKeys(stdout, KEY)).toEqual(["2013434102"]); + }); + + // The queried key itself is echoed as a header when it has values; it is not its own subkey. + test("does not return the queried key itself", () => { + const stdout = [ + "", + "HKEY_LOCAL_MACHINE\\SOFTWARE\\WOW6432Node\\GOG.com\\Games", + "", + ].join("\r\n"); + expect(parseRegSubKeys(stdout, KEY)).toEqual([]); + }); + + test("case-insensitive on the hive and path", () => { + const stdout = + "hkey_local_machine\\software\\wow6432node\\gog.com\\games\\42"; + expect(parseRegSubKeys(stdout, KEY)).toEqual(["42"]); + }); + + test("no subkeys is empty, not a throw", () => { + expect(parseRegSubKeys("", KEY)).toEqual([]); + expect( + parseRegSubKeys("ERROR: The system was unable to find...", KEY), + ).toEqual([]); + }); +});