Harden AI search tool path filtering

[crynta]

Summary

• filter every AI grep and glob hit through the readable-path guard before returning auto-approved tool results
• preserve root canonicalization while handling absolute and relative native hit paths
• add regression tests for sensitive grep contents and glob path enumeration

Verification

• pnpm lint
• pnpm check-types
• pnpm test
• cargo clippy --all-targets --locked -- -D warnings
• cargo nextest run --locked
• git diff --check

Note: full repo formatter checks currently report unrelated pre-existing formatting drift outside this PR.

Summary by CodeRabbit

• Bug Fixes
  • Search results now exclude paths that fail safety checks, so only readable, allowed files are returned.
  • Grep results no longer include unsafe matches from restricted locations.
  • Glob results now filter out sensitive or traversal-based paths before being shown.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a checkReadable-based path-safety layer to the search tools module. New helpers pathForSafety and isReadableSearchHit filter grep and glob tool results to exclude unreadable/unsafe hits before returning them, alongside reformatted checkReadableCanonical calls and new tests validating the filtering.

Changes

Search tool path safety

Layer / File(s)	Summary
Path-safety helpers src/modules/ai/tools/search.ts	Imports checkReadable and adds pathForSafety/isReadableSearchHit helpers to normalize hit paths and check readability against the resolved root.
Grep result filtering src/modules/ai/tools/search.ts	Filters grep's res.hits via isReadableSearchHit before mapping to response objects; reformats the checkReadableCanonical call.
Glob result filtering and tests src/modules/ai/tools/search.ts, src/modules/ai/tools/search.test.ts	Filters glob's hits via isReadableSearchHit instead of returning all hits; reformats checkReadableCanonical call; adds tests verifying unsafe hits (private keys, .ssh, /etc/passwd traversal) are excluded from both tools.

Estimated code review effort: 2 (Simple) | ~15 minutes

Related PRs: None identified from the provided context.

Suggested labels: security, tests

Suggested reviewers: None identified from the provided context.

A grep peeked at secrets stashed in .ssh,
a glob poked at /etc/passwd — quite rash!
Now checkReadable stands guard at the gate,
filtering hits before it's too late.
Safe paths hop through, unsafe ones must wait. 🐇🔒

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title describes the change, but it is not in Conventional Commits format as required.	Rewrite it as a Conventional Commit, for example: "fix(ai): harden search tool path filtering".

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/modules/ai/tools/search.ts (1)

95-107: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Apply the result cap after safety filtering.

maxResults is passed to native search before unsafe hits are filtered. If sensitive matches fill the native cap, this can return zero safe hits while later safe matches exist. Consider pushing the readability filter into native search, or over-fetching internally and slicing after filtering so the user-requested cap applies to safe hits.

As per path instructions, watch for blast radius and silent behavior changes beyond the local fix.

Also applies to: 140-141

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/modules/ai/tools/search.ts` around lines 95 - 107, The result cap is
being applied before safety filtering in the search flow, so unsafe hits can
consume the native limit and hide later readable matches. Update the search path
in search.ts around the res.hits filtering and result mapping so that maxResults
applies to the post-filtered safe hits, either by moving the isReadableSearchHit
check earlier into the native search path or by over-fetching and slicing after
filtering. Keep the change localized to the search result processing and the
analogous logic referenced in the other affected section, and avoid altering
unrelated behavior such as truncation or file-scanned reporting.

Source: Path instructions

🧹 Nitpick comments (1)

src/modules/ai/tools/search.test.ts (1)

52-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Grep test only exercises absolute hit paths.

The glob test covers both a relative hit (service-account-prod.json) and an absolute one (/etc/passwd) exercising pathForSafety's branch logic, but the grep test's fixtures are all absolute paths. Since pathForSafety has a distinct relative-path branch (joining with root), adding one relative-path grep fixture would close a small coverage gap for the shared helper.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/modules/ai/tools/search.test.ts` around lines 52 - 96, The grep test in
search.test.ts only uses absolute hit paths, so it misses the relative-path
branch in pathForSafety. Update the fixture in buildSearchTools/makeContext grep
coverage to include at least one relative hit path alongside the existing
absolute paths, and assert it is filtered or resolved the same way so both
branches of the shared safety helper are exercised.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/modules/ai/tools/search.ts`:
- Around line 95-107: The result cap is being applied before safety filtering in
the search flow, so unsafe hits can consume the native limit and hide later
readable matches. Update the search path in search.ts around the res.hits
filtering and result mapping so that maxResults applies to the post-filtered
safe hits, either by moving the isReadableSearchHit check earlier into the
native search path or by over-fetching and slicing after filtering. Keep the
change localized to the search result processing and the analogous logic
referenced in the other affected section, and avoid altering unrelated behavior
such as truncation or file-scanned reporting.

---

Nitpick comments:
In `@src/modules/ai/tools/search.test.ts`:
- Around line 52-96: The grep test in search.test.ts only uses absolute hit
paths, so it misses the relative-path branch in pathForSafety. Update the
fixture in buildSearchTools/makeContext grep coverage to include at least one
relative hit path alongside the existing absolute paths, and assert it is
filtered or resolved the same way so both branches of the shared safety helper
are exercised.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cf8eb458-a0e0-4345-a15a-656be58e9623

📥 Commits

Reviewing files that changed from the base of the PR and between cb75fae and 5398084.

📒 Files selected for processing (2)

• src/modules/ai/tools/search.test.ts
• src/modules/ai/tools/search.ts