fix: undo byte-fidelity, undo path guard, and result-cap bypasses

[dynamics365ninja]

Full-repo audit of the CLI, Core, MCP, and Bridge projects. Eight defects, each with a regression test. Build is warning-free; 657 tests pass (648 existing + 9 new).

Bugs fixed

undo did not restore the exact bytes it promises

The journal's contract is "restoring the exact pre-image bytes", but PreImage is stored as a decoded string and the decoder swallows the UTF-8 BOM. AtomicWriteText then rewrote the file without one. Reproduced end-to-end:

before undo:  ef bb bf 3c   ("<" behind a BOM)
after  undo:  3c 3f 78 6d   ("<?xm" — BOM gone)

D365FO and Visual Studio both write AOT XML with a BOM, so every undo left a spurious change in git diff. JournalEntry gains an optional PreImageHadBom flag — backward compatible, since entries from older builds deserialize to null and fall back to the BOM state of the file on disk. After the fix the restored file is byte-identical to the original.

undo bypassed PathGuard

ReplayDisk called File.Delete and wrote files with no boundary check, unlike every other write path in the codebase. The target path comes out of the journal store rather than the current command line, so a journal carried over from a different D365FO_PACKAGES_PATH — or hand-edited — could reach anywhere on disk.

A negative --limit disabled the row cap

SQLite reads a negative LIMIT as "no limit at all", so --limit -1 turned a bounded lookup into a full-index dump — the exact token blow-up the result caps exist to prevent. The MCP layer already guarded this (limit <= 0 ? 50); the CLI and Core did not. Adds ClampLimit across all 27 query methods, plus a hard MaxRowLimit = 1000 ceiling.

BridgeClient desynchronised after a timeout

On timeout the abandoned ReadLine stayed parked on the pipe and consumed the bridge's late reply — which was then handed to the next request, shifting every subsequent response by one. Replies were never correlated by JSON-RPC id either. The child process is now discarded on timeout so the next call starts clean, and a mismatched id is a hard error rather than silently misattributed data.

undo --steps 0 reverted an entry

Both UndoEngine (Math.Max(1, steps)) and the CLI (settings.Steps <= 0 ? 1) silently rounded 0 up to 1. Undo is destructive, so a caller asking for no steps now gets INVALID_ARGS instead of an unrequested revert.

Three smaller ones

• Unescaped LIKE wildcards in FindFormsByPattern — --pattern "Simple_List" also matched SimpleXList.
• Grounding-token binding was too loose — the bidirectional substring match meant a token issued for Cust authorized writes to anything containing "Cust", and a token for CustTable authorized the unrelated shorter Cust. Now a prefix match, which still allows derived names like CustTable_MyExt_Extension and CustTableEventHandler.
• Build hygiene — 3× CS1573, and a doc comment on OpenReadOnly promising an EnsureSchema fallback that does not exist in the code.

Verification

dotnet build -c Release is clean, and the fixes were exercised against the MiniAot sample through the real CLI, not only through unit tests: BOM round-trip, --steps 0/-3, --limit -1, and --pattern "Simple_List".

Deliberately not changed

Three findings alter operational behavior and are left as follow-ups:

• HttpServerHost binds 0.0.0.0 even with no API_KEY (log warning only). A loopback default would be safer but breaks Azure App Service deployments, which require 0.0.0.0.
• CallDedup is a static cache shared across all HTTP clients — in a shared team deployment client B can be served client A's cached answer. Read-only metadata from the same index, so not a leak, but it is cross-session state.
• PathGuard always allows the whole %TEMP% tree. Intentional for tests, but it widens the boundary for production writes too.

🤖 Generated with Claude Code