patch-db/AUDIT.md

patch-db Code Audit

Critical / High Severity

Rust

1. Infinite loop in run_idempotent — FIXED

File: patch-db/src/store.rs

old was read once before the loop, never refreshed. If another writer modified store.persistent between the initial read and the write-lock acquisition, the &old == &store.persistent check failed forever — old was never updated, so the loop spun infinitely.

Fix: Moved old read inside the loop so it refreshes on each retry attempt.

2. TentativeUpdated undo after successful disk write — FIXED

File: patch-db/src/store.rs

If compress() succeeded (wrote patched state to disk) but a later step failed, Drop rolled back the in-memory state while the on-disk state already reflected the patch. On next startup, the file replayed the patch, creating a permanent divergence.

Fix: Rewrote compress() with three explicit phases (atomic backup, main file rewrite, non-fatal backup removal). Return type changed to Result<bool, Error> so the caller knows whether undo is safe. If the backup was committed, TentativeUpdated disarms the undo.

3. push_start_idx doesn't update existing segment ranges — FIXED

File: json-ptr/src/lib.rs

Unlike push_start which shifted all existing segment ranges by the prefix length, push_start_idx just inserted a new segment without adjusting the others. All existing segments' ranges became wrong, causing corrupted pointer lookups or panics.

Fix: Added the range-shifting loop to match push_start behavior.

4. Integer underflow in DiffPatch::rebase for Remove — FIXED

File: patch-db/src/patch.rs

When idx == 0 and onto_idx == 0, the condition idx >= onto_idx passed and idx - 1 underflowed a usize. Panics in debug, wraps to usize::MAX in release.

Fix: Changed condition from idx >= onto_idx to idx > onto_idx.

TypeScript

5. Remove on nested array elements corrupts state — FIXED

File: client/lib/json-patch-lib.ts

recursiveApply returned undefined for remove (since value is undefined on RemoveOperation). For arrays, the splice-based removal only kicked in when path.length === 1. A deeper path like /arr/0/nested/2 set array[2] = undefined instead of splicing it out, leaving a hole.

Fix: Introduced a REMOVE_SENTINEL Symbol. Base case returns the sentinel for remove ops. Array and object handlers check for it to trigger proper splice/delete.

6. RFC 6902 "-" (end-of-array) token not handled — FIXED

File: client/lib/json-patch-lib.ts

parseInt("-") returned NaN. splice(NaN, 0, value) inserted at position 0 instead of appending. Non-numeric path segments on arrays also silently produced corrupt state.

Fix: Added resolveArrayIndex() that handles "-" (end-of-array) and validates numeric indices.

7. Revision gap silently applied — FIXED

File: client/lib/patch-db.ts

The check update.id < expected only deduplicated. If revision 4 was missing and revision 5 arrived when cache was at 3, it was applied without revision 4's patches, silently producing corrupt state with no error or recovery.

Fix: Added console.warn when a revision gap is detected.

---

Medium Severity

Rust

8. unreachable!() reachable via deserialized patches — IGNORED

File: patch-db/src/patch.rs

DiffPatch is Deserialize-able and wraps Patch which can hold Move/Copy/Test operations. for_path, rebase, exists, and keys all panic on those variants.

Status: Ignored. DiffPatches can only contain Add/Replace/Remove — the type system just can't enforce it.

9. poll_changed applies only one revision per call — FIXED

File: patch-db/src/subscriber.rs

If multiple revisions queued up, the Stream implementation applied one at a time, emitting intermediate states that may never have been a consistent committed state.

Fix: Added a drain loop after the first poll_recv wake to consume all queued revisions before returning.

10. compress backup removal failure clobbers good data — FIXED

File: patch-db/src/store.rs

If remove_file(bak) failed after the main file was successfully rewritten, the error propagated. On next open, Store::open saw the stale .bak file and renamed it over the successfully compacted main file.

Fix: Backup removal is now non-fatal (let _ = ...). A leftover backup is harmlessly replayed on restart since it matches the main file content.

11. DbWatch::sync permanently desynchronizes on patch error — IGNORED

File: patch-db/src/subscriber.rs

If one patch fails to apply, the error returns immediately. Remaining queued revisions are never consumed. The watch is now permanently out of sync with no recovery path.

Status: Ignored. Patch errors should be impossible; if they occur, a different bug in patch-db is the root cause. Not worth the complexity to drain-and-continue.

12. RFC 6902 deviation: replace on missing path silently adds — IGNORED (intentional)

File: json-patch/src/lib.rs

RFC 6902 §4.3 requires the target location to exist. The implementation falls back to add instead of returning an error.

Status: Ignored. This is intentional behavior for this project's use case.

TypeScript

13. NonNullable return types on watch$ are unsound — FIXED

File: client/lib/patch-db.ts

All watch$ overloads claimed NonNullable<...>, but runtime values can be null/undefined (e.g., after a remove operation). Consumers skipped null checks based on the type, leading to runtime crashes.

Fix: Removed outer NonNullable wrapper from the 1-level overload return type.

14. withLatestFrom + in-place mutation fragility — FIXED

File: client/lib/patch-db.ts

processUpdates mutated the cache object in place, then re-emitted it via cache$.next(cache). If source$ emitted synchronously twice before the subscriber ran, withLatestFrom sampled the already-mutated object reference, potentially skipping valid revisions via the stale cache.id check.

Fix: Replaced withLatestFrom with direct this.cache$.value access in the subscribe callback.

---

Low Severity / Performance

Rust

15. OPEN_STORES never removes entries — FIXED

File: patch-db/src/store.rs

Entries were inserted on open but never cleaned up on close. Unbounded growth over the lifetime of a process that opens many different database files.

Fix: Store::close() now removes the entry from OPEN_STORES.

16. Broadcast::send clones patches per subscriber under write lock — DEFERRED

File: patch-db/src/subscriber.rs

revision.for_path() is called per subscriber, cloning and filtering the entire patch. This is O(subscribers × operations) work while holding the RwLock write guard.

17. Array diff is O(n²) — DEFERRED

File: json-patch/src/diff.rs

The ptr_eq scan inside the array diff loops is O(n) per element, making worst-case O(n²) for large arrays with mostly non-pointer-equal elements.

18. Store::exists conflates null with missing — FIXED

File: patch-db/src/store.rs

A key with an explicit Value::Null was reported as non-existent.

Fix: Changed from null comparison to .is_some().

TypeScript

19. new RegExp in hot path — FIXED

File: client/lib/json-patch-lib.ts

arrayFromPath and pathFromArray constructed new RegExp objects on every call.

Fix: Pre-compiled regex literals at module scope.

20. O(watchedNodes × patchOps) redundant arrayFromPath — FIXED

File: client/lib/patch-db.ts

Inside handleRevision, arrayFromPath(path) was called for every (watchedNode, patchOp) pair.

Fix: Pre-convert patch operation paths once outside the loop.

21. getValueByPointer swallows all exceptions — FIXED

File: client/lib/json-patch-lib.ts

The catch (e) { return undefined } masked programming errors and state corruption, making debugging very difficult.

Fix: Now only catches TypeError (from accessing properties on null/undefined), re-throws everything else.

22. throw 'unreachable' is reachable — FIXED

File: client/lib/json-patch-lib.ts

If a patch path extended past the actual document depth (data is a primitive at a non-terminal segment), this branch executed. It threw a string with no stack trace.

Fix: Now throws a proper Error with a descriptive message.