fix: merge latest dev updates into main

[nadav-node9]

Auto-generated PR

Merge latest dev changes into main to trigger a release.

⚠️ Important: When you click Squash and Merge, ensure the commit message starts with:

• fix: to publish a Patch release (0.0.X)
• feat: to publish a Minor release (0.X.0)
  If it starts with chore:, no NPM package will be published!

[github-actions]

🤖 Claude Code Review

Code Review: Node9 Proxy — DLP Path Scanning & Regex Cache

Security Issues (High Priority)

1. scanFilePath trusts attacker-controlled cwd
The cwd parameter defaults to process.cwd(), but callers in core.ts pass no cwd. If the agent can influence process.cwd() (e.g., via a prior cd tool call that Node9 approved), symlink resolution happens relative to a compromised working directory. The resolved path should be validated against an expected root (e.g., the user's home or project dir), not just pattern-matched.

2. filePath is reflected into redactedSample without sanitization
The original attacker-controlled filePath string goes directly into the alert object as redactedSample. If this surfaces in a UI or log, it's a vector for log injection or UI spoofing (e.g., newlines, ANSI escape codes). Sanitize or truncate it.

3. ReDoS protection has gaps
The validateRegex check uses regex-on-regex, which doesn't catch all dangerous patterns. Notably, (a+)+, (.+)+, and (a|aa)+ all pass the current checks. The nested-quantifier pattern /\([^)]*[*+{][^)]*\)[*+{]/ won't match (.+)+ because the inner content contains . not just quantifiers inside the group. Consider a more robust check or a dedicated safe-regex library.

4. SENSITIVE_PATH_PATTERNS — .key extension is too broad
/[/\\][^/\\]+\.key$/i will block legitimate tool calls on files like config.key, license.key, registry.key. This will cause false positive blocks on production workflows. Scope this more carefully or downgrade to review severity.

Correctness / Edge Cases

5. notMatches behavior change is a breaking policy change
The original notMatches with val === null returned true; the new code preserves this but adds if (!cond.value) return false. The old code returned true when cond.value was falsy. This is a silent semantic break for existing user rules — it should be called out explicitly in the PR description and changelog.

6. fs.existsSync + fs.realpathSync TOCTOU race
The file could be replaced with a symlink between existsSync and realpathSync. Use fs.realpathSync wrapped in try/catch unconditionally instead of the existence check.

7. argsObjE/argsObj duplication
The same args-unwrapping logic is copy-pasted into both explainPolicy and _authorizeHeadlessCore. Extract it into a shared helper.

Type Safety

8. getCompiledRegex flags parameter is unvalidated
The flags string comes from user-supplied rule config. Invalid flags (e.g., "z") will throw in the new RegExp(pattern, flags) call — caught by the try/catch, but validateRegex never checks flags, so the error path is silent even with debug off.

Summary

The file-path DLP concept is sound and the regex caching is a good improvement, but the TOCTOU race, unvalidated cwd, and path reflection into alerts are concrete security bugs in a security tool. The .key false-positive risk will cause user trust erosion. Fix these before merging.

---

Automated review by Claude Sonnet

[github-actions]

🤖 Claude Code Review

Review: src/__tests__/undo.test.ts

Overall Assessment

This is a well-structured test refactor. The migration from withGitRepo/withStack to withShadowRepo correctly reflects the new shadow-repo architecture. The new test cases cover meaningful behavior. A few issues worth addressing:

---

Issues

1. import path from 'path' is mid-file (line ~203)
This import appears after test describe blocks. While Vitest/ESM hoisting may handle it, it's non-idiomatic and could cause confusion or tooling warnings. Move it to the top with other imports.

2. withShadowRepo silently ignores project-path.txt reads when includeStackFile = false
When includeStackFile is false, readFileSync still returns '' for project-path.txt. If ensureShadowRepo compares this to the real cwd (/mock/project), the empty string won't match, potentially triggering unintended re-initialization in tests that don't expect it. This could mask bugs or produce false positives.

3. readdirSync mock returns [] (typed as string[]) but fs.readdirSync returns Dirent[] | Buffer[] | string[] depending on overload
The cast to ReturnType<typeof fs.readdirSync> is implicit here. If production code calls readdirSync with { withFileTypes: true }, the mock returns the wrong shape silently. Low risk given current scope but worth a comment.

4. GIT_INDEX_FILE cleanup test is too loose

expect(unlinkCalls.some((p) => p.includes('index_'))).toBe(true);

This passes if any file containing index_ is unlinked — including unrelated files. Consider asserting the path also includes .node9/snapshots to match the security invariant (index must never be inside the user's .git).

5. ensureShadowRepo reinit test doesn't assert rmSync path

expect(vi.mocked(fs.rmSync)).toHaveBeenCalled();

For a security tool, this should assert that rmSync was called on the shadow directory specifically, not just that it was called at all. A bug that calls rmSync on the wrong path would pass this test.

6. writeShadowExcludes test passes a third argument to createShadowSnapshot

await createShadowSnapshot('edit', {}, ['node_modules', 'dist']);

If createShadowSnapshot's signature doesn't accept a third ignorePaths parameter, this silently passes in TypeScript strict mode only if the type is any. Verify the exported signature matches.

7. No negative test for GIT_DIR isolation
Given this is a security tool, there's no test asserting that git operations are never invoked without GIT_DIR/GIT_WORK_TREE set. A regression where env vars are dropped would corrupt the user's real repo — this deserves an explicit negative assertion.

---

Minor

• mockGitSuccess handles init in the config branch via a.includes('init') — git init doesn't take a config subcommand, so this conflation is fine for mocking but slightly misleading to read.
• beforeEach re-applies all spy defaults after clearAllMocks — correct pattern, no issues.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.

---

Automated review by Claude Sonnet

[github-actions]

🤖 Claude Code Review

Code Review

Test Coverage — Good Progress, Some Gaps

The test suite additions are solid overall. The new getShadowRepoDir tests are well-structured, and the environment variable assertions (GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE) are exactly the right thing to verify for a security tool.

Gap: The diff appears truncated — the ensureShadowRepo test for hash collision recovery (the project-path.txt sentinel logic) isn't shown. If that code path exists in production, it needs test coverage. Collision detection is a correctness-critical code path.

---

Security Issues

realpathSync.native() mock is an identity function — the test mocks fs.realpathSync as (p) => String(p), but the production code uses fs.realpathSync.native. These are different functions. If the symlink-escape prevention relies on .native, the mock may not actually exercise that code path, giving false confidence in the security test coverage.

Shadow repo path isolation — the test correctly asserts GIT_INDEX_FILE is inside .node9/snapshots, but it doesn't assert that GIT_INDEX_FILE is not the user's .git/index. A negative assertion would be stronger here for a security boundary.

---

Correctness / Edge Cases

withShadowRepo reads project-path.txt returning '/mock/project' — this silently assumes the hash of /mock/project matches the hash derived in production. If normalizeCwdForHash does anything other than realpathSync, this mock could be returning data for the wrong path in collision-check logic without the test noticing.

mockGitSuccess rev-parse check — matching on a.includes('rev-parse') would also match git rev-parse HEAD (used for other purposes). If production code calls both rev-parse --git-dir and rev-parse HEAD, this mock collapses them, potentially hiding bugs.

The truncated test — expect(result).toBe('commit — this is incomplete. If this shipped as-is, it would be a syntax error or failing test.

---

Minor / Type Safety

The as ReturnType<typeof spawnSync> casts are repeated excessively. A local helper returning this type would reduce noise and make the mocks less brittle if spawnSync's return type changes.

---

What Looks Good

• beforeEach properly resets all mocks including the new ones (realpathSync, readdirSync, statSync, rmSync).
• spawn mock with unref correctly handles the background git gc call.
• Renaming withGitRepo → withShadowRepo is semantically correct and improves clarity.
• The index file cleanup test (unlinkSync on index_*) is a good finally-block regression guard.

---

Bottom line: One real security gap (.native mock mismatch), one correctness risk (hash assumption in withShadowRepo), and a truncated test that must be completed before merge.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.

---

Automated review by Claude Sonnet

[github-actions]

🤖 Claude Code Review

Code Review: Node9 Proxy — Shadow Git Snapshots + ReDoS Protection

Test Coverage (Primary Focus of This Diff)

Test helper spawnResult is a clear improvement — eliminates repetitive casting boilerplate. Good.

withShadowRepo replaces withGitRepo correctly, but there's a subtle gap: withShadowRepo mocks readdirSync to return [] (empty array), yet the production ensureShadowRepo likely calls readdirSync to detect whether the shadow repo is initialized. If that logic checks for non-empty, tests could pass for the wrong reason. Verify the mock return value matches what the actual initialization check expects.

The ensureShadowRepo test at the bottom is truncated — the diff cuts off mid-mockImplementation. This means we can't review one of the most security-critical test cases (collision detection, hash collision recovery, path rename recovery). These should be fully visible in the PR.

Missing test cases for new security features:

• Symlink escape prevention via realpathSync.native is mocked as identity — no test actually verifies that a symlink pointing to a protected path is blocked. The mock setup comment acknowledges this but no test exercises it.
• The project-path.txt collision detection / auto-recovery path has no visible test.
• No test for concurrent session isolation (GIT_INDEX_FILE uniqueness across two simultaneous createShadowSnapshot calls).

Security Concerns

realpathSync.native mock approach is fragile:

(fs.realpathSync as unknown as { native: (p: unknown) => string }).native = vi.fn()...

Direct property assignment on the module object can silently fail if the module is frozen or if another import caches the original reference. This is test infrastructure, but if the production symlink-escape check is bypassed in tests, the security guarantee is untested.

CHANGELOG mentions fs.realpathSync.native() for symlink escape prevention — but we can't see src/undo.ts in this diff. The test mocks it but never asserts it was called during a sensitive path check. Add an assertion.

Async / Race Conditions

The GIT_INDEX_FILE cleanup test only checks unlinkSync was called — it doesn't verify cleanup happens on failure paths (e.g., when write-tree returns non-zero). The "finally block" comment implies this is guarded, but no failure-path test validates it.

Type Safety

addCall![2]?.env as Record<string, string> — the non-null assertion on addCall! is fine in tests, but the as Record<string, string> cast could mask a undefined env object. Minor, but use optional chaining throughout for consistency.

Documentation

CHANGELOG is clear and well-written. The "10 snapshots per session" vs "10 snapshots total in snapshots.json" distinction should be clarified — snapshots.json appears to be global, not per-session.

Summary

The test refactoring is directionally good, but three gaps need addressing before merge: (1) the truncated ensureShadowRepo test must be complete and reviewable, (2) the symlink-escape prevention needs an assertion that realpathSync.native was actually called on tool-call paths, and (3) a failure-path test for index file cleanup is needed given this is a security tool where error paths matter as much as happy paths.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.

---

Automated review by Claude Sonnet

[github-actions]

🤖 Claude Code Review

Code Review: Node9 Proxy — Shadow Snapshots, DLP Path Blocking, ReDoS Protection

Correctness & Edge Cases

realpathSync.native mock setup is fragile. In both test files, the .native property is assigned directly on the mocked function object rather than using vi.spyOn. After vi.restoreAllMocks() in dlp.test.ts, the base realpathSync spy is restored, but the .native property assignment survives since it's not tracked by Vitest. This means test isolation isn't guaranteed — a test that runs after restoreAllMocks could see a stale .native implementation. The beforeEach re-assignment in undo.test.ts partially mitigates this, but it's a brittle pattern that breaks if test order changes.

Truncated diff. The diff cuts off mid-sentence at const result = await creat — the final test case for write-tree failure is incomplete. Can't fully verify that test path.

spawnResult helper doesn't cover stderr as null. Some spawnSync failure paths in production return null for stderr/stdout (e.g. when stdio: 'pipe' isn't set). The helper always provides Buffer.from(''), which may mask bugs where production code doesn't null-check these fields.

Security Issues

realpathSync.native mocking obscures the real security guarantee. The symlink-resolution tests mock away the exact function that provides the protection. This verifies call presence, not correctness. If production code has a path where fs.existsSync returns false and symlink resolution is skipped, a symlink pointing to /home/user/.ssh/id_rsa with a non-existent intermediate path could bypass the check. The test for "blocks when symlink resolves to sensitive path" only works because existsSync is mocked to true — there's no test for the case where existsSync throws (e.g. permission denied).

project-path.txt collision detection is mentioned in the changelog but no test covers the auto-recovery path. This is a security-relevant code path — a collision or rename could cause snapshots to be stored in the wrong repo.

CHANGELOG claims notMatches is now fail-closed but there's no test asserting that an invalid regex in notMatches produces a deny, not a pass. This is exactly the kind of regression that should be regression-tested.

Race Conditions / Async

git gc --auto background process: The mock adds spawn: vi.fn().mockReturnValue({ unref: vi.fn() }) but no test verifies unref() is actually called. If production code omits unref(), the gc process will block Node.js exit on every tool call — a correctness issue on the critical path.

TypeScript Type Safety

The repeated (fs.realpathSync as unknown as { native: (p: unknown) => string }) cast appears 6+ times across both files. This should be extracted into a typed helper or the module augmentation should declare the type properly. The unknown intermediary is masking a real typing gap in the production dlp.ts/undo.ts code.

Test Coverage Gaps

• No test for the 10-snapshot rolling window eviction logic
• No test for concurrent session safety (GIT_INDEX_FILE isolation)
• No test for .git / .node9 exclusion from snapshots ("inception prevention" per changelog)
• ReDoS pattern rejection logic has zero test coverage in this diff
• LRU cache eviction at 500 entries is untested

Minor

The import path from 'path' at line ~198 of undo.test.ts appears after the first describe block — imports should be at the top of the file.

Overall: The test infrastructure refactor is a meaningful improvement over the previous approach, but the realpathSync.native mocking strategy is the most concerning issue given this is a security tool. The symlink-escape protection is exactly the kind of thing that needs hermetic tests, not call-presence assertions.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.

---

Automated review by Claude Sonnet

[github-actions]

🤖 Claude Code Review

Code Review: Node9 Proxy — ReDoS Protection, DLP Path Blocking, Shadow Git Snapshots

Security Issues

1. ReDoS detection heuristic is incomplete (medium-high risk)
The test suite accepts (ba|z|da|fi|c|k)?sh as safe, which is correct. However, the heuristic of blocking "quantified alternations" — if implemented as a simple syntactic check — will miss cases like (a{1,10}|b{1,10}){1,10} or (?:a|b){1,100}, which are still ReDoS vectors. The tests only cover basic patterns. There's no test for nested groups with non-capturing variants or alternations inside character classes.

2. realpathSync.native mock restoration is fragile
In dlp.test.ts, the beforeEach assigns .native as a vi.fn() on the spy created by vi.spyOn(fs, 'realpathSync'). However, vi.restoreAllMocks() restores the base function but not .native — hence the manual originalNative restore. This is correctly handled, but it's fragile: if any test throws before afterEach, the restore still runs, but if someone adds a return early or restructures tests, this pattern can silently leave .native mocked across describe blocks.

3. Symlink resolution: realpathSync.native throws on missing files on some platforms
The production scanFilePath guards with existsSync before calling .native, which is correct. But realpathSync.native is documented to throw if the path doesn't exist on Linux (unlike realpathSync which may not). The test that checks existsSync → false never calls .native, which is correct — but there's no test verifying the function doesn't throw when existsSync returns true but .native throws (e.g., a TOCTOU race). This is a real edge case for a security tool.

Test Coverage Gaps

4. createShadowSnapshot diff is cut off
The undo test file ends mid-line (re on the last line). The diff appears truncated. It's impossible to review the full test coverage for createShadowSnapshot, collision detection, or --steps N logic.

5. No test for LRU eviction behavior
getCompiledRegex is tested for cache hits, but there's no test that the cache evicts when it hits 500 entries. If the eviction logic is wrong, the "bounded" cache is unbounded — a memory leak on the hot path.

6. No test for concurrent calls to getCompiledRegex
Node.js is single-threaded but async interleaving could occur if the cache lookup is async-gapped. Not a concern here since it's sync — but worth a comment in code.

TypeScript / Type Safety

7. RealpathWithNative cast is a code smell
Casting fs.realpathSync to a custom interface with .native in two separate test files independently is duplication. This type should be in a shared test utility or use typeof fs.realpathSync & { native: ... } consistently. The current inconsistency between dlp.test.ts (typed alias) and undo.test.ts (unknown cast) is a maintenance risk.

Minor

8. spawnResult helper is a good improvement — reduces boilerplate and improves readability. No issues.

9. CHANGELOG correctness: "Total built-in patterns: 9" — verify this count is accurate after merging; stale counts in changelogs erode trust.

---

Overall: The security intent is sound, but the ReDoS heuristic coverage and TOCTOU edge case on realpathSync.native need attention before merge. The truncated diff is a blocker for full approval.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.

---

Automated review by Claude Sonnet

[github-actions]

🤖 Claude Code Review

Code Review: Node9 Proxy — ReDoS Protection, DLP Path Blocking, Shadow Git Snapshots

Security Issues (High Priority)

1. ReDoS validator may have false negatives
The CHANGELOG describes rejecting "nested quantifiers, quantified alternations, and quantified backreferences," but the test suite doesn't cover backreference-based ReDoS (e.g., (\w+)\1+). If validateRegex in core.ts only checks structural patterns, backreference catastrophic backtracking goes undetected. Needs a dedicated test and corresponding production guard.

2. TOCTOU window in scanFilePath is acknowledged but the fallback weakens the guarantee
The test 'falls back to original path when native throws, still blocks if original path is sensitive' passes because the original path happens to be sensitive. But if an attacker constructs a symlink where:

• the nominal path looks safe (passes the pre-check)
• existsSync returns true
• native throws (race deletion)

...the fallback to path.resolve of the nominal path could return a safe-looking path and allow the call. The test doesn't cover this specific scenario. This is worth an explicit test and possibly a fail-closed fallback (block on native throw rather than falling back).

3. LRU cache key collision risk
getCompiledRegex uses pattern + flags as a cache key (inferred from the test). If pattern contains the delimiter used to join them, two distinct (pattern, flags) pairs could share a key. The test 'treats pattern+flags as a distinct cache key' only covers the '' vs 'i' case. Use a structured key like a Map<string, Map<string, RegExp>> or a separator that can't appear in flags (flags are always [gimsuy]), e.g. \0.

Correctness & Edge Cases

4. validateRegex rejects (foo|bar)+ but shells use this legitimately
The test explicitly expects '(ba|z|da|fi|c|k)?sh' to pass because ? is safe. However, real policy rules like (allow|deny)+ would be rejected. This could cause legitimate user rules to silently fail. The CHANGELOG says invalid patterns are "rejected" — clarify whether this surfaces an error to the user or silently drops the rule, and test that path.

5. spawnResult helper is truncated
The diff ends mid-comment (/** Moc). The spawnResult helper and any tests using spawn (the new mock for spawn suggests background git gc testing) are cut off. Cannot verify coverage of the background GC path or concurrent-session GIT_INDEX_FILE isolation mentioned in the CHANGELOG.

Test Coverage Gaps

6. Shadow repo hash collision / directory rename recovery is untested — the CHANGELOG specifically calls out the project-path.txt sentinel and auto-recovery behavior; no tests visible.

7. --steps N undo path is mentioned in the CHANGELOG but no test for N > 1 or N exceeding snapshot history length.

8. 10-snapshot eviction boundary — no test that snapshot 11 correctly evicts snapshot 1 from snapshots.json.

Minor Issues

• The byExcludePath helper is defined but its usage is cut off by the truncated diff — confirm it's actually used.
• The RealpathWithNative type alias is duplicated across dlp.test.ts and undo.test.ts; extract to a shared test utility file.

Summary

The ReDoS protection and DLP path-blocking concepts are sound and the test approach is solid. The primary concerns are the TOCTOU fallback behavior (should be fail-closed), the backreference ReDoS gap, and the cache key collision risk. The truncated diff makes it impossible to fully review the shadow snapshot and spawn mock coverage.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.

---

Automated review by Claude Sonnet

[github-actions]

🤖 Claude Code Review

Code Review: Node9 Proxy PR

Security Issues (Critical Path)

scanFilePath TOCTOU — fail-closed is correct, but check the implementation detail
The tests assert fail-closed behavior when realpathSync.native throws, which is the right call. However, verify the production code in src/dlp.ts uses realpathSync.native specifically (not realpathSync), since the native variant bypasses JS-layer path normalization. The test mocks both separately — if production only mocks one, the symlink test coverage is misleading.

Symlink resolution scope
scanFilePath resolves symlinks only when the file exists (existsSync check first). This is inherently racy — another TOCTOU window exists between existsSync and native(). The fail-closed throw-catch mitigates it, but consider whether existsSync is even necessary: just call native() unconditionally and catch ENOENT, treating missing files as unresolved (safe, since they can't be read anyway).

ReDoS pattern: (foo|bar)+ rejection may be overly broad
Rejecting all quantified alternations will block legitimate safe patterns like (https?|ftp):// or (allow|deny)\s+. This is a usability cliff for a security tool where users write their own rules. The CHANGELOG doesn't document this user-visible restriction. Consider documenting it explicitly and providing a clearer error message.

Async/Race Conditions

spawn mock in undo tests
The spawn mock returns { unref: vi.fn() } but no stdout/stderr/on handlers. If createShadowSnapshot or git gc --auto background processes listen for events, tests will silently succeed against the mock but fail in production. Verify the mock is complete enough to catch event-listener bugs.

Type Safety

RealpathWithNative cast is duplicated
The same alias is defined identically in both dlp.test.ts and undo.test.ts. This should be extracted to a shared test utility (src/__tests__/helpers.ts) to avoid drift if the type needs updating.

vi.mocked((fs.realpathSync as RealpathWithNative).native) pattern
Casting to a custom type and then calling vi.mocked on a property assigned via direct mutation isn't guaranteed to be tracked by Vitest's mock registry. The nativeSpy assertion (expect(nativeSpy).toHaveBeenCalled()) may pass vacuously if the spy reference doesn't match what production code calls. Prefer vi.spyOn on the object property if the API supports it.

Test Coverage Gaps

• LRU eviction correctness: The 520-pattern test verifies no crash, but doesn't assert the first entry was actually evicted (cache size stays ≤ 500). Add a size assertion if getCompiledRegex exposes cache size, or test that the oldest entry is a cache miss after eviction.
• getCompiledRegex flags validation: No test for invalid flag characters (e.g., 'z'). If production code passes flags from user config without validation, this is an injection surface.
• Shadow repo hash collision recovery: CHANGELOG describes auto-recovery via project-path.txt sentinel — no test coverage for this path is visible in the diff.
• scanFilePath with relative paths: Only one relative-path test (src/app.ts). Missing: paths like ../../.ssh/id_rsa that resolve outside the project root.

Minor

• The [A-Z]{3,} pattern passing validateRegex is correct (bounded, non-nested), but the test comment should note it's safe because the quantifier applies to a character class, not a group.
• CHANGELOG entry for shadow snapshots says "last 10 snapshots" globally — confirm this interacts safely with concurrent sessions (two agents snapshotting simultaneously could race on snapshots.json).

Overall the security intent is sound, but the TOCTOU and symlink mock fidelity issues need verification before merge.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.

---

Automated review by Claude Sonnet

[github-actions]

🤖 Claude Code Review

Code Review: Node9 Proxy — ReDoS Protection, DLP Expansion, Shadow Snapshots

1. Security: scanFilePath TOCTOU — ENOENT treated as safe

The biggest concern here. The test explicitly documents this tradeoff:

"ENOENT on a path that looks safe → treated as safe"

But this is a partial TOCTOU window, not a full fix. An attacker who can race the filesystem can:

1. Create a symlink at /project/harmless.ts → /home/user/.ssh/id_rsa
2. Let native() succeed and get blocked — fine
3. But what if the symlink is created after the policy check and before the tool actually reads the file?

scanFilePath only helps if it's called at the moment of I/O, not just at policy evaluation time. The current architecture intercepts the tool call declaration — verify that scanFilePath is invoked as close to actual file I/O as possible, not just at the /check endpoint entry.

2. Security: validateRegex allowlist coverage may be incomplete

The ReDoS detection is heuristic-based (nested quantifiers, quantified alternations with inner quantifiers, quantified backreferences). Heuristics can miss novel patterns — e.g., (\s+|\t+)+, exponential alternations like (x|xx)*. Consider documenting explicitly that this is a best-effort guard and not a formal guarantee, or integrate a library like safe-regex2 as a secondary check.

3. Correctness: LRU cache max size (500) is never tested for correctness of which entries are evicted

The LRU overflow test only checks that 520 entries don't throw — it doesn't assert that entries 1–20 were actually evicted. A broken LRU that grows unboundedly would pass this test. Add a test asserting re1 !== re521 (first entry evicted after filling).

4. Security: getCompiledRegex flag validation

Good that invalid flags return null. Confirm the validation uses a strict allowlist (/^[gimsuy]*$/) rather than a denylist — the test implies this but doesn't make the implementation explicit in the diff.

5. Async/Race: Shadow snapshot git gc --auto in background

The changelog mentions "periodic background git gc --auto" — confirm this is fire-and-forget with no unhandled rejection. A crashing gc shouldn't fail a tool call approval. Not visible in this diff but warrants a code search.

6. Type Safety: RealpathWithNative cast in tests

The cast typeof fs.realpathSync & { native: (p: unknown) => string } uses unknown for the parameter. This is fine for tests but note that the production fs.realpathSync.native accepts PathLike | string — if scanFilePath passes a non-string PathLike, test mocks may diverge from real behavior.

7. Test gap: scanFilePath with Windows-style paths

No tests for \\.aws\\credentials or mixed separators. If Node9 ever runs on Windows or WSL, path matching could silently fail.

8. Minor: CHANGELOG test naming inconsistency

"exports at least 9 built-in patterns" will become stale again on the next addition. Consider >= DLP_PATTERNS.length derived at runtime, or a snapshot test.

---

Overall: The ReDoS protection and DLP path blocking are well-reasoned additions with good test coverage. The primary concern to address before merge is confirming the TOCTOU window in scanFilePath is as narrow as architecturally possible, and documenting where it isn't.

⚠️ Note: This diff exceeded 20,000 characters and was truncated. The review above covers only the first portion of the changes.

---

Automated review by Claude Sonnet