From 5fa30414d7a68bab5e44f9a0220c1975edb4083c Mon Sep 17 00:00:00 2001 From: MatMoul Date: Sun, 10 May 2026 00:56:58 +0200 Subject: [PATCH] fix: normalize bridge errors and support nested group paths Distinguish invalid KeePass requests from backend failures in the Python bridge, improve nested group path resolution, and add coverage for nested group creation plus payload forwarding. --- .memory/project.md | 7 +++-- .memory/state.md | 6 ++-- .memory/todo.md | 7 ++--- README.md | 6 +++- src/keepass.ts | 21 +++++++------- src/python/bridge.py | 8 +++-- tests/integration/pykeepass.test.ts | 22 ++++++++++++++ tests/unit/keepass.test.ts | 45 +++++++++++++++++++++++++++++ 8 files changed, 99 insertions(+), 23 deletions(-) diff --git a/.memory/project.md b/.memory/project.md index 3f999cb..00a750e 100644 --- a/.memory/project.md +++ b/.memory/project.md @@ -9,7 +9,8 @@ Provide a TypeScript wrapper around KeePass `.kdbx` databases using a Python bri - TypeScript spawns a Python process per request; there is no persistent worker yet. - JSON is exchanged over stdin/stdout. - Bridge errors, empty output, invalid JSON, missing files, and backend exceptions are surfaced as TypeScript errors. -- Coverage now includes `keyFile` payload propagation and core API smoke checks. +- Bridge error reporting now distinguishes invalid KeePass requests from backend errors. +- Coverage now includes `keyFile` payload propagation, nested group payload shaping, and core API smoke checks. ## Public API - `openKeePassDatabase(path, options)` @@ -37,7 +38,7 @@ Provide a TypeScript wrapper around KeePass `.kdbx` databases using a Python bri - Bundled fixtures: `tests/fixtures/data.kdbx` and `tests/fixtures/empty.kdbx`. - Companion JSON fixture: `tests/fixtures/data.kdbx.json` stores the password and expected content. - Unit tests in `tests/unit/` mock the child process and validate bridge parsing, error handling, command forwarding, and payload shaping. -- Integration tests in `tests/integration/` use `data.kdbx` to verify entries, groups, partial search, OTP/TOTP output, and basic write persistence on a temporary copy when `pykeepass` is installed. +- Integration tests in `tests/integration/` use `data.kdbx` to verify entries, groups, partial search, OTP/TOTP output, write persistence on a temporary copy, and nested group creation when `pykeepass` is installed. - The integration test runner checks for `pykeepass` and skips cleanly when it is unavailable. - Memory tracking files: `.memory/state.md` and `.memory/todo.md`. @@ -48,4 +49,4 @@ Provide a TypeScript wrapper around KeePass `.kdbx` databases using a Python bri - `bun run setup:python` ## Current direction -Keep improving failure-path coverage, keep write support minimal and predictable, and continue validating persistence on temporary copies. +Keep improving failure-path coverage, keep write support minimal and predictable, and continue validating persistence on temporary copies and nested group behavior. diff --git a/.memory/state.md b/.memory/state.md index 5aa100d..9fdf68a 100644 --- a/.memory/state.md +++ b/.memory/state.md @@ -1,5 +1,7 @@ # State -- Added failure-path, command-forwarding, and keyFile payload unit tests for the bridge. -- Latest unit and full test runs passed. +- Hardened bridge error handling and nested group path resolution. +- Added unit coverage for invalid JSON, empty output, nested group path forwarding, and keyFile payloads. +- Added integration coverage for creating groups on temporary copies. +- Latest test run passed: 20 tests, 0 failures. - Project renamed to ts-pykeepass-wrapper; current focus remains the TypeScript wrapper + Python bridge for KeePass. diff --git a/.memory/todo.md b/.memory/todo.md index 9f79c68..c5da7aa 100644 --- a/.memory/todo.md +++ b/.memory/todo.md @@ -1,6 +1,5 @@ # Todo -- Improve failure-path coverage for write operations. -- Verify and harden nested group path resolution. -- Keep integration write tests on temporary copies only. -- Keep API minimal and predictable. +- Keep write-path behavior predictable and well-documented. +- Preserve minimal API surface until update/delete/move is required. +- Consider typed bridge errors in TypeScript if more granularity is needed later. diff --git a/README.md b/README.md index 95e3c7c..dd168fb 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,8 @@ Creates a new entry in the target database and persists it immediately. - `notes`: optional notes - `groupPath`: optional target group path +`groupPath` is resolved as an existing group path when possible. Nested paths such as `Folder1/SubFolder` are supported when the target group exists. + ### `createGroup(group)` Creates a new group and persists it immediately. @@ -92,6 +94,8 @@ Creates a new group and persists it immediately. - `name`: group name - `path`: optional parent group path +`path` is resolved as an existing parent group path when possible, including nested paths. + ### `save()` Persists the current database state. @@ -102,7 +106,7 @@ No-op for now. - The bridge currently launches a Python process per call. - This is simple and robust for a first version. -- Errors from the Python bridge are propagated to the TypeScript API, including invalid or empty output. +- Errors from the Python bridge are propagated to the TypeScript API, including invalid or empty output. Bridge failures are normalized to distinguish invalid requests and backend errors. - A persistent Python process can be added later if needed. - Write operations currently open, modify, and save the database per command. - Bundled fixtures include `tests/fixtures/data.kdbx` and `tests/fixtures/empty.kdbx`; the companion JSON file stores the password and expected content for tests/examples. diff --git a/src/keepass.ts b/src/keepass.ts index 9fcc397..ea5db5b 100644 --- a/src/keepass.ts +++ b/src/keepass.ts @@ -60,19 +60,23 @@ export class KeePassDatabase { const bridgeFile = fileURLToPath(this.bridgePath); const result = await new Promise<{ stdout: string; stderr: string; code: number }>((resolve, reject) => { - const child = spawn(this.pythonPath, [bridgeFile], { - stdio: ["pipe", "pipe", "pipe"], - }); - + const child = spawn(this.pythonPath, [bridgeFile], { stdio: ["pipe", "pipe", "pipe"] }); let stdout = ""; let stderr = ""; let settled = false; + const fail = (error: unknown) => { + if (!settled) { + settled = true; + reject(error instanceof Error ? error : new Error(String(error))); + } + }; + try { child.stdin.write(payload); child.stdin.end(); } catch (error) { - reject(error); + fail(error); } child.stdout.on("data", (chunk) => { @@ -83,12 +87,7 @@ export class KeePassDatabase { stderr += chunk.toString(); }); - child.on("error", (error) => { - if (!settled) { - settled = true; - reject(error); - } - }); + child.on("error", fail); child.on("close", (code) => { if (!settled) { diff --git a/src/python/bridge.py b/src/python/bridge.py index c669ec5..0f8b063 100644 --- a/src/python/bridge.py +++ b/src/python/bridge.py @@ -86,7 +86,8 @@ def resolve_group_by_path(db, group_path): if matching_groups: return matching_groups[0] - matching_groups = db.find_groups(name=normalized.split("/")[-1]) + segments = [segment for segment in normalized.strip("/").split("/") if segment] + matching_groups = db.find_groups(name=segments[-1]) for group in matching_groups: if path_to_string(group.path).endswith(normalized): return group @@ -196,8 +197,11 @@ def main(): emit({"ok": False, "error": f"Unknown command: {command}"}) return 1 + except ValueError as exc: + emit({"ok": False, "error": f"Invalid KeePass request: {exc}"}) + return 1 except Exception as exc: - emit({"ok": False, "error": str(exc)}) + emit({"ok": False, "error": f"KeePass backend error: {exc}"}) return 1 diff --git a/tests/integration/pykeepass.test.ts b/tests/integration/pykeepass.test.ts index 107fea9..c05018c 100644 --- a/tests/integration/pykeepass.test.ts +++ b/tests/integration/pykeepass.test.ts @@ -166,6 +166,28 @@ test("creates entries in a temporary copy of the bundled fixture and persists th }); +test("creates nested groups on a temporary copy", async () => { + const [{ password }, pykeepassReady] = await Promise.all([ + readFile(FIXTURE_DATA_PATH, "utf8").then((raw) => JSON.parse(raw) as FixtureData), + ensurePyKeePass(), + ]); + + if (!pykeepassReady) { + console.log("Skipping integration test: pykeepass is not installed"); + return; + } + + await withTempCopy(FIXTURE_PATH, async (tempPath) => { + const db = openKeePassDatabase(tempPath, { password }); + const createdGroup = await db.createGroup({ name: "Nested", path: "Folder1" }); + expect(createdGroup.path).toBe("Folder1/Nested"); + + const groups = await db.listGroups(); + expect(groups.some((group) => group.path === "Folder1/Nested")).toBe(true); + }); +}); + + test("uses the JSON fixture content as the source of truth for expectations", async () => { const { content } = JSON.parse(await readFile(FIXTURE_DATA_PATH, "utf8")) as FixtureData; diff --git a/tests/unit/keepass.test.ts b/tests/unit/keepass.test.ts index d127050..4ca70f8 100644 --- a/tests/unit/keepass.test.ts +++ b/tests/unit/keepass.test.ts @@ -85,6 +85,23 @@ describe("KeePassDatabase", () => { await expect(db.listEntries()).rejects.toThrow("Invalid JSON from Python bridge"); }); + test("throws a useful error when the bridge exits without output", async () => { + spawnMock.mockImplementation(() => { + const child = { + stdin: { write: () => undefined, end: () => undefined }, + stdout: { on: () => undefined }, + stderr: { on: (_event: string, cb: (chunk: Buffer | string) => void) => cb("bridge crashed") }, + on: (event: string, cb: (code?: number | null) => void) => { + if (event === "close") queueMicrotask(() => cb(1)); + }, + }; + return child as never; + }); + + const db = new KeePassDatabase("db.kdbx", { password: "secret" }, "python3", new URL("file:///tmp/bridge.py")); + await expect(db.listEntries()).rejects.toThrow("bridge crashed"); + }); + test("throws on spawn error", async () => { spawnMock.mockImplementation(() => { const child = { @@ -122,6 +139,34 @@ describe("KeePassDatabase", () => { expect(spawnMock).toHaveBeenCalled(); }); + test("createEntry forwards nested group paths in the payload", async () => { + let payload = ""; + spawnMock.mockImplementation(() => { + const child = { + stdin: { + write: (chunk: string) => { + payload += chunk; + }, + end: () => undefined, + }, + stdout: { on: (_event: string, cb: (chunk: Buffer | string) => void) => cb(JSON.stringify({ ok: true, data: { title: "New" } })) }, + stderr: { on: () => undefined }, + on: (event: string, cb: (code?: number | null) => void) => { + if (event === "close") queueMicrotask(() => cb(0)); + }, + }; + return child as never; + }); + + const db = new KeePassDatabase("db.kdbx", { password: "secret" }, "python3", new URL("file:///tmp/bridge.py")); + await db.createEntry({ title: "New", groupPath: "Folder/SubFolder" }); + + expect(JSON.parse(payload)).toMatchObject({ + command: "create-entry", + entry: { title: "New", groupPath: "Folder/SubFolder" }, + }); + }); + test("save forwards the save command", async () => { mockSuccessfulBridgeResponse(null);