fix: allow search_files to work inside hidden directory paths - #18645
fix: allow search_files to work inside hidden directory paths#18645Kailigithub wants to merge 1 commit into
Conversation
liuhao1024
left a comment
There was a problem hiding this comment.
The fix correctly handles paths like ~/.hermes/skills/ where a hidden directory is part of the search root. However, there's an edge case with . and .. paths.
os.path.normpath('.') returns '.', and '.'.startswith('.') is True. Same for ... This means:
- Searching in
.(current directory) disables hidden directory exclusions —.git/,.cache/, etc. would be searched - Searching in
..or../siblingalso disables hidden exclusions
These are not hidden directories — they're special POSIX directory entries. The check should exclude them:
_path_parts = os.path.normpath(path).split(os.sep)
_has_hidden_component = any(
p.startswith('.') and p not in ('.', '..')
for p in _path_parts if p
)The same fix should be applied in both _search_files and _search_with_grep.
This matters because the agent commonly passes relative paths like . when the user asks to search the current directory. With the current fix, those searches would silently include .git/ and other hidden directories, returning unexpected results.
8fc7886 to
3a0442b
Compare
… search roots Two bugs in _search_with_grep() when ripgrep is unavailable: 1. BRE mode: grep -rnH uses Basic Regular Expression mode, where | is a literal character. ripgrep ships with Rust regex (| = alternation), so patterns with alternation silently fail on the grep fallback. Fix: add -E to enable ERE (Extended Regular Expression) mode. 2. --exclude-dir='.*': GNU grep applies this to all directory components INCLUDING command-line arguments, so a search under ~/.hermes/workspace skips the entire tree because .hermes matches .*. Ripgrep only applies globs to subdirectories, not the root. Fix: only add --exclude-dir when no path component is hidden, mirroring the logic already used by _search_files (merged in NousResearch#16672). Both bugs produce silent false-negatives: search_files returns total_count=0 without any error. Cross-reference NousResearch#18473, NousResearch#18645, NousResearch#34017. Fixes NousResearch#18473
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the hidden-root fallback issue. The grep half remains relevant on current origin/main: _search_content selects _search_with_grep when rg is unavailable (tools/file_operations.py:2244-2250), and that method still unconditionally adds --exclude-dir='.*' (tools/file_operations.py:2392-2394).
Problems
- The
target='files'/findhalf is already on main through merged PR #19878 (tools/file_operations.py:2118-2186; commit4a1505a54796ac2a7396dcd9c406423291c450b9). - Skipping
--exclude-dirfor a hidden root also permits every hidden descendant below that root. The existing security regression identifies.hub/index-cacheas unvetted adversarial content and requires grep to exclude it (tests/tools/test_search_hidden_dirs.py:3-13,:77-85).
Suggested changes
- Keep the explicit-root bypass for grep, but filter only hidden descendants relative to that root, and add a forced-grep regression covering both a visible file and
.hub/index-cache.
Automated hermes-sweeper review.
| cmd_parts.append("--exclude-dir='.*'") | ||
| # BUT: skip exclusion when the search path itself is inside a hidden | ||
| # directory, because --exclude-dir='.*' would exclude the search root. | ||
| _path_parts = os.path.normpath(path).split(os.sep) |
There was a problem hiding this comment.
When the supplied root has a hidden ancestor, omitting this filter also exposes every hidden descendant below that root, including .hub/index-cache. Preserve the explicit-root bypass, but filter descendant path components relative to the requested root before returning results; tests/tools/test_search_hidden_dirs.py documents why that exclusion is security-sensitive.
|
Closing as superseded — the same fix landed upstream via the salvage track in commit |
|
Closing per supersede comment above. The fix landed via salvage track. |
When the
pathargument contains hidden directories (e.g.~/.hermes/skills/),search_filesreturns 0 results in bothtarget='files'andtarget='content'modes (when ripgrep is not available).Root cause:
_search_with_grep()uses--exclude-dir='.*'which matches any directory component, including the search root itself when it's inside a hidden directory_search_files()find fallback uses-not -path '*/.*'with the same issueFix: Conditionally apply the hidden-directory exclusion only when the search path itself does not contain hidden directory components. This mirrors how ripgrep behaves natively — it excludes hidden dirs by default but still allows explicit hidden-directory search paths.
Changes:
_search_with_grep(): Skip--exclude-dirwhen the path has hidden components_search_files()find fallback: Skip-not -path '*/.*'when the path has hidden componentsVerification:
py_compilepassestest_file_operationstests passCloses #18473