Skip to content

server: harden the file_glob_search directory walk - #26626

Merged
ServeurpersoCom merged 7 commits into
ggml-org:masterfrom
ServeurpersoCom:server/file-glob-search-hardening
Aug 5, 2026
Merged

server: harden the file_glob_search directory walk#26626
ServeurpersoCom merged 7 commits into
ggml-org:masterfrom
ServeurpersoCom:server/file-glob-search-hardening

Conversation

@ServeurpersoCom

Copy link
Copy Markdown
Contributor

Overview

Windows junctions are no longer walked. A junction looks like an ordinary directory to std::filesystem, so one pointing back at a parent made the search spin through the same folders over and over until path names got too long.

Junk directory names are matched case insensitively on Windows, so a folder called Build is skipped like build is.

A directory the walk cannot open or read no longer disappears in silence. Results now say they are incomplete instead of looking whole while an entire subtree is missing.

An invalid limit is rejected instead of quietly falling back to the default, the same way an invalid type already is.

Paths sent back to clients come out normalized: no . or .. left in them, and forward slashes on every platform.

Under the hood, the listing code was simplified and the time budget is now checked more often, so a very deep or very wide tree cannot outstay it.

Additional information

Follow-up #26518

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES + manual Linux and Windows testing

std::filesystem reports a junction as a plain directory, so the symlink
guard misses it and a junction pointing back at an ancestor is walked
until the path length gives out

read the reparse tag and treat a symlink and a mount point as links,
leaving any other reparse point walkable so cloud placeholders and dedup
stubs still get searched

look junk directory names up case insensitively on Windows, where NTFS
makes Build the same directory as build

test that a junk directory stays selectable while its contents stay out
of search results
a directory that fails to open or to iterate was skipped in silence, so
a caller got a listing that looked complete while a whole subtree was
missing: a path over the platform limit, a volume going away, a name the
filesystem rejects

skip_permission_denied never reaches this path, so an error here is an
incomplete answer rather than a deliberate omission, and it now sets the
truncated flag
return a small result struct instead of two out params and a caller path
that only fed an error string, taking list_entries from six parameters
down to three

scope the error code to the directory being read, act on the status code
the entry lookups already returned, and treat an unreadable link state as
a link so the walk never descends on a guess

check the deadline when a directory is popped, not only per entry, so a
tree of empty directories cannot outlive the budget

read the path parameter once, and reject an invalid limit the way an
invalid type is already rejected, instead of silently falling back

normalize the resolved path, so a "." or ".." a caller typed reaches
neither git nor the client, and return the generic path form with '/'
separators on every platform, so the base sent to clients no longer needs
a local fixup
the cache grew for the lifetime of the component: entries went stale
after the TTL but were never removed, so every distinct query typed in a
session stayed in memory

drop expired entries when a new result is stored
@ServeurpersoCom
ServeurpersoCom requested review from a team as code owners August 5, 2026 09:19
@ServeurpersoCom
ServeurpersoCom marked this pull request as draft August 5, 2026 09:20
@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

Draft for some remaining testing under Windows 11

Comment thread tools/server/server-tools.cpp Outdated
}

#if defined(_WIN32)
// std::filesystem reports a junction as a plain directory, so a junction

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remember to clean up comments per agents.md

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Comments have been cut down to one line each, and two that just restated the code are gone.

Comment thread tools/server/server-tools.cpp Outdated
return false;
}

static std::string junk_lookup_name(const std::string & fname) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe call it "get_effective_name", the logic has nothing to do with "junk"

also might be better to move them to tools_io private members

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

junk_lookup_name is now get_effective_name, and both helpers moved to private static members next to junk_dir_names().

Comment thread tools/server/server-tools.cpp Outdated
return lowered;
}
#else
static bool is_dir_link(const fs::path &) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be handle properly on linux too? i.e. symlink?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows and Linux logic deduplicated into a single is_link(), symlinks are checked on every platform and junctions are added on Windows.

trim comments to one line each, and drop two that restate the code

rename junk_lookup_name to get_effective_name, and move it and the link
check to private static members next to junk_dir_names

merge the Windows and Linux link checks into one is_link, so symlinks are
checked everywhere and junctions only add to it on Windows
a narrow path uses the active code page there, so a file name came back
mangled and a path with an accent could not be opened at all

convert explicitly at every crossing between a std::string, which always
carries UTF-8 here, and fs::path

read the home directory through the wide environment, since the narrow
one returns the profile path in the active code page too

the walker no longer normalizes separators by hand, since paths now come
back in generic form
@ServeurpersoCom
ServeurpersoCom marked this pull request as ready for review August 5, 2026 11:07
@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

What's tested :

Tested on Windows 11 dedicated machine (build b10294-10bf199e2, tree in C:\A\tests)

Junction looping back on its own base: listed, never walked, no recursion
Junction to a sibling directory: listed, not walked, no duplicated content
Junction pointing outside the tree: listed, its content invisible to both glob and grep
Inherited Documents and Settings junction at the C:/ root: listed without descent, a real case not a fabricated one
Base pointed directly at a junction: walked normally, the barrier only applies to automatic descent
Build in uppercase treated as junk on NTFS, its content absent from results
base always returned with forward slashes including C:/, input accepted with backslashes or forward slashes
Accented and Japanese names returned intact by file_glob_search, no replacement characters
read_file on an accented path, on a Japanese path, and on an ASCII file under an accented directory
grep_search in a tree with non ASCII names
get_info returning a clean cwd
exec_shell_command with accents, confirming the OEM fix lives alongside the path conversion
Deep tree of 60 levels and 245 characters read in full, no false truncation

Re-tested on Linux (pod, started from /mnt/inputs)

64 pytest cases: test_tools_builtin, test_mcp_servers, test_security
519 UI unit tests, plus svelte-check, prettier and eslint
Symlink loop, link to a sibling, link pointing outside, link to a file, dangling link, chain of two links
No leak outside the base in glob or grep, with the pattern still found when pointing at the link explicitly
Build in uppercase still walked and build still ignored, case handling stays per platform
Accented and Japanese names in glob, read_file and grep_search
Paths with .. and . normalized in base
Invalid limit rejected, huge limit capped at 100
Tree beyond PATH_MAX raising the truncated flag, normal tree not raising it
Picker keystroke cost measured at 67 ms, unchanged

  • Some manual Windows/Linux testing like encoding :
Sans titre

match the shape of the other helpers, one definition with the #if inside,
instead of two definitions wrapped in #if and #else

inline the single caller helper and trim the comment
@ngxson

ngxson commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

can be merged whenever you add the merge-ready label

@ServeurpersoCom ServeurpersoCom added the merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. label Aug 5, 2026

@allozaur allozaur left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perf

@ServeurpersoCom

Copy link
Copy Markdown
Contributor Author

I wait the real CI and I merge. Easy to follow-up on anything. That gives us a really handy all in one local agent with a web interface, which seems to be a first!

@ServeurpersoCom
ServeurpersoCom merged commit f9e832c into ggml-org:master Aug 5, 2026
24 of 31 checks passed
satindergrewal pushed a commit to satindergrewal/llama.cpp that referenced this pull request Aug 11, 2026
* server: don't walk Windows junctions in file_glob_search

std::filesystem reports a junction as a plain directory, so the symlink
guard misses it and a junction pointing back at an ancestor is walked
until the path length gives out

read the reparse tag and treat a symlink and a mount point as links,
leaving any other reparse point walkable so cloud placeholders and dedup
stubs still get searched

look junk directory names up case insensitively on Windows, where NTFS
makes Build the same directory as build

test that a junk directory stays selectable while its contents stay out
of search results

* server: report a directory the walk could not read

a directory that fails to open or to iterate was skipped in silence, so
a caller got a listing that looked complete while a whole subtree was
missing: a path over the platform limit, a volume going away, a name the
filesystem rejects

skip_permission_denied never reaches this path, so an error here is an
incomplete answer rather than a deliberate omission, and it now sets the
truncated flag

* server: simplify the file_glob_search listing plumbing

return a small result struct instead of two out params and a caller path
that only fed an error string, taking list_entries from six parameters
down to three

scope the error code to the directory being read, act on the status code
the entry lookups already returned, and treat an unreadable link state as
a link so the walk never descends on a guess

check the deadline when a directory is popped, not only per entry, so a
tree of empty directories cannot outlive the budget

read the path parameter once, and reject an invalid limit the way an
invalid type is already rejected, instead of silently falling back

normalize the resolved path, so a "." or ".." a caller typed reaches
neither git nor the client, and return the generic path form with '/'
separators on every platform, so the base sent to clients no longer needs
a local fixup

* ui: expire cached picker searches

the cache grew for the lifetime of the component: entries went stale
after the TTL but were never removed, so every distinct query typed in a
session stayed in memory

drop expired entries when a new result is stored

* server: address review from @ngxson

trim comments to one line each, and drop two that restate the code

rename junk_lookup_name to get_effective_name, and move it and the link
check to private static members next to junk_dir_names

merge the Windows and Linux link checks into one is_link, so symlinks are
checked everywhere and junctions only add to it on Windows

* server: convert tool paths as UTF-8 on Windows

a narrow path uses the active code page there, so a file name came back
mangled and a path with an accent could not be opened at all

convert explicitly at every crossing between a std::string, which always
carries UTF-8 here, and fs::path

read the home directory through the wide environment, since the narrow
one returns the profile path in the active code page too

the walker no longer normalizes separators by hand, since paths now come
back in generic form

* server: fold the platform branch inside console_output_to_utf8

match the shape of the other helpers, one definition with the #if inside,
instead of two definitions wrapped in #if and #else

inline the single caller helper and trim the comment
satindergrewal pushed a commit to satindergrewal/llama.cpp that referenced this pull request Aug 12, 2026
* server: don't walk Windows junctions in file_glob_search

std::filesystem reports a junction as a plain directory, so the symlink
guard misses it and a junction pointing back at an ancestor is walked
until the path length gives out

read the reparse tag and treat a symlink and a mount point as links,
leaving any other reparse point walkable so cloud placeholders and dedup
stubs still get searched

look junk directory names up case insensitively on Windows, where NTFS
makes Build the same directory as build

test that a junk directory stays selectable while its contents stay out
of search results

* server: report a directory the walk could not read

a directory that fails to open or to iterate was skipped in silence, so
a caller got a listing that looked complete while a whole subtree was
missing: a path over the platform limit, a volume going away, a name the
filesystem rejects

skip_permission_denied never reaches this path, so an error here is an
incomplete answer rather than a deliberate omission, and it now sets the
truncated flag

* server: simplify the file_glob_search listing plumbing

return a small result struct instead of two out params and a caller path
that only fed an error string, taking list_entries from six parameters
down to three

scope the error code to the directory being read, act on the status code
the entry lookups already returned, and treat an unreadable link state as
a link so the walk never descends on a guess

check the deadline when a directory is popped, not only per entry, so a
tree of empty directories cannot outlive the budget

read the path parameter once, and reject an invalid limit the way an
invalid type is already rejected, instead of silently falling back

normalize the resolved path, so a "." or ".." a caller typed reaches
neither git nor the client, and return the generic path form with '/'
separators on every platform, so the base sent to clients no longer needs
a local fixup

* ui: expire cached picker searches

the cache grew for the lifetime of the component: entries went stale
after the TTL but were never removed, so every distinct query typed in a
session stayed in memory

drop expired entries when a new result is stored

* server: address review from @ngxson

trim comments to one line each, and drop two that restate the code

rename junk_lookup_name to get_effective_name, and move it and the link
check to private static members next to junk_dir_names

merge the Windows and Linux link checks into one is_link, so symlinks are
checked everywhere and junctions only add to it on Windows

* server: convert tool paths as UTF-8 on Windows

a narrow path uses the active code page there, so a file name came back
mangled and a path with an accent could not be opened at all

convert explicitly at every crossing between a std::string, which always
carries UTF-8 here, and fs::path

read the home directory through the wide environment, since the narrow
one returns the profile path in the active code page too

the walker no longer normalizes separators by hand, since paths now come
back in generic form

* server: fold the platform branch inside console_output_to_utf8

match the shape of the other helpers, one definition with the #if inside,
instead of two definitions wrapped in #if and #else

inline the single caller helper and trim the comment
huaxel pushed a commit to huaxel/CachyLLama that referenced this pull request Aug 12, 2026
* server: don't walk Windows junctions in file_glob_search

std::filesystem reports a junction as a plain directory, so the symlink
guard misses it and a junction pointing back at an ancestor is walked
until the path length gives out

read the reparse tag and treat a symlink and a mount point as links,
leaving any other reparse point walkable so cloud placeholders and dedup
stubs still get searched

look junk directory names up case insensitively on Windows, where NTFS
makes Build the same directory as build

test that a junk directory stays selectable while its contents stay out
of search results

* server: report a directory the walk could not read

a directory that fails to open or to iterate was skipped in silence, so
a caller got a listing that looked complete while a whole subtree was
missing: a path over the platform limit, a volume going away, a name the
filesystem rejects

skip_permission_denied never reaches this path, so an error here is an
incomplete answer rather than a deliberate omission, and it now sets the
truncated flag

* server: simplify the file_glob_search listing plumbing

return a small result struct instead of two out params and a caller path
that only fed an error string, taking list_entries from six parameters
down to three

scope the error code to the directory being read, act on the status code
the entry lookups already returned, and treat an unreadable link state as
a link so the walk never descends on a guess

check the deadline when a directory is popped, not only per entry, so a
tree of empty directories cannot outlive the budget

read the path parameter once, and reject an invalid limit the way an
invalid type is already rejected, instead of silently falling back

normalize the resolved path, so a "." or ".." a caller typed reaches
neither git nor the client, and return the generic path form with '/'
separators on every platform, so the base sent to clients no longer needs
a local fixup

* ui: expire cached picker searches

the cache grew for the lifetime of the component: entries went stale
after the TTL but were never removed, so every distinct query typed in a
session stayed in memory

drop expired entries when a new result is stored

* server: address review from @ngxson

trim comments to one line each, and drop two that restate the code

rename junk_lookup_name to get_effective_name, and move it and the link
check to private static members next to junk_dir_names

merge the Windows and Linux link checks into one is_link, so symlinks are
checked everywhere and junctions only add to it on Windows

* server: convert tool paths as UTF-8 on Windows

a narrow path uses the active code page there, so a file name came back
mangled and a path with an accent could not be opened at all

convert explicitly at every crossing between a std::string, which always
carries UTF-8 here, and fs::path

read the home directory through the wide environment, since the narrow
one returns the profile path in the active code page too

the walker no longer normalizes separators by hand, since paths now come
back in generic form

* server: fold the platform branch inside console_output_to_utf8

match the shape of the other helpers, one definition with the #if inside,
instead of two definitions wrapped in #if and #else

inline the single caller helper and trim the comment
brittlewis12 pushed a commit to brittlewis12/llama.cpp that referenced this pull request Aug 17, 2026
* server: don't walk Windows junctions in file_glob_search

std::filesystem reports a junction as a plain directory, so the symlink
guard misses it and a junction pointing back at an ancestor is walked
until the path length gives out

read the reparse tag and treat a symlink and a mount point as links,
leaving any other reparse point walkable so cloud placeholders and dedup
stubs still get searched

look junk directory names up case insensitively on Windows, where NTFS
makes Build the same directory as build

test that a junk directory stays selectable while its contents stay out
of search results

* server: report a directory the walk could not read

a directory that fails to open or to iterate was skipped in silence, so
a caller got a listing that looked complete while a whole subtree was
missing: a path over the platform limit, a volume going away, a name the
filesystem rejects

skip_permission_denied never reaches this path, so an error here is an
incomplete answer rather than a deliberate omission, and it now sets the
truncated flag

* server: simplify the file_glob_search listing plumbing

return a small result struct instead of two out params and a caller path
that only fed an error string, taking list_entries from six parameters
down to three

scope the error code to the directory being read, act on the status code
the entry lookups already returned, and treat an unreadable link state as
a link so the walk never descends on a guess

check the deadline when a directory is popped, not only per entry, so a
tree of empty directories cannot outlive the budget

read the path parameter once, and reject an invalid limit the way an
invalid type is already rejected, instead of silently falling back

normalize the resolved path, so a "." or ".." a caller typed reaches
neither git nor the client, and return the generic path form with '/'
separators on every platform, so the base sent to clients no longer needs
a local fixup

* ui: expire cached picker searches

the cache grew for the lifetime of the component: entries went stale
after the TTL but were never removed, so every distinct query typed in a
session stayed in memory

drop expired entries when a new result is stored

* server: address review from @ngxson

trim comments to one line each, and drop two that restate the code

rename junk_lookup_name to get_effective_name, and move it and the link
check to private static members next to junk_dir_names

merge the Windows and Linux link checks into one is_link, so symlinks are
checked everywhere and junctions only add to it on Windows

* server: convert tool paths as UTF-8 on Windows

a narrow path uses the active code page there, so a file name came back
mangled and a path with an accent could not be opened at all

convert explicitly at every crossing between a std::string, which always
carries UTF-8 here, and fs::path

read the home directory through the wide environment, since the narrow
one returns the profile path in the active code page too

the walker no longer normalizes separators by hand, since paths now come
back in generic form

* server: fold the platform branch inside console_output_to_utf8

match the shape of the other helpers, one definition with the #if inside,
instead of two definitions wrapped in #if and #else

inline the single caller helper and trim the comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. server/ui server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants