Skip to content

Improve file traversal performance and correctness #8704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
zwass merged 4 commits into from
Dec 10, 2025
Merged

Improve file traversal performance and correctness #8704

zwass merged 4 commits into from
Dec 10, 2025

Conversation

@Krechals
Copy link
Contributor

@Krechals Krechals commented Oct 29, 2025 •
edited
Loading

This PR fixes #7291

Another issue with the current osquery implementation is that hidden directories don't get traversed.

Currently, recursive traversals terminate early when a cycle is detected.
For example, given the following query:

SELECT path FROM file WHERE file.path LIKE '/home/andrei.topala/osquery-test/%%' AND file.type = 'regular';

and the directory structure:

$ tree
.
|-- 1
|   |-- 2
|   |   |-- 3
|   |   |   |-- 4
|   |   |   |   `-- d.txt
|   |   |   `-- c.txt
|   |   `-- b.txt
|   `-- a.txt
`-- 5
    `-- 6 -> ../5

osquery currently returns:

+--------------------------------------------+
| path                                       |
+--------------------------------------------+
| /home/andrei.topala/osquery-test/1/2/b.txt |
| /home/andrei.topala/osquery-test/1/a.txt   |
+--------------------------------------------+

The fix updates the traversal logic to continue expanding ** recursively until no new folders or files remain to explore.
With this change, osquery now returns:

+------------------------------------------------+
| path                                           |
+------------------------------------------------+
| /home/andrei.topala/osquery-test/1/2/3/4/d.txt |
| /home/andrei.topala/osquery-test/1/2/3/c.txt   |
| /home/andrei.topala/osquery-test/1/2/b.txt     |
| /home/andrei.topala/osquery-test/1/a.txt       |
+------------------------------------------------+

The implementation now calls platformGlob only initially, with directory expansion handled through directory traversal.

A rough benchmark shows this change makes the traversal 2x faster than the current implementation.
Before:

$ sudo time osqueryi "SELECT COUNT(path) from FILE where file.path LIKE '/tmp/random_tree_root%%';" 
+-------------+
| COUNT(path) |
+-------------+
| 535469      |
+-------------+
4.80user 26.85system 0:32.08elapsed 98%CPU (0avgtext+0avgdata 1229140maxresident)k
95752inputs+0outputs (0major+307973minor)pagefaults 0swaps

After:

$ sudo time osquery/osqueryi "SELECT COUNT(path) from FILE where file.path LIKE '/tmp/random_tree_root%%';"
+-------------+
| COUNT(path) |
+-------------+
| 535469      |
+-------------+
3.98user 7.84system 0:12.04elapsed 98%CPU (0avgtext+0avgdata 1228504maxresident)k
0inputs+0outputs (0major+307464minor)pagefaults 0swaps

@Krechals Krechals requested review from a team as code owners October 29, 2025 01:29
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 29, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@Krechals Krechals force-pushed the fix-symlink-loop-tranversal branch from 217c5ae to 590ad28 Compare October 30, 2025 19:13
@Krechals Krechals changed the title Fix recursive traversal when the filesystem includes symbolic link loops Improve file traversal performance and correctness Oct 30, 2025
@Krechals Krechals force-pushed the fix-symlink-loop-tranversal branch from 590ad28 to 654c5ee Compare October 30, 2025 20:02
zwass
zwass requested changes Nov 6, 2025
View reviewed changes
Copy link
Member

@zwass zwass left a comment

Choose a reason for hiding this comment

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

Thank you for this! I'm going to need to take some more time to review and test, but I did kick off the CI and see that the formatting checks fail: https://github.com/osquery/osquery/actions/runs/18953561159/job/54706407760?pr=8704#step:7:11. Those should be pretty easy to fix. You can test locally, see https://osquery.readthedocs.io/en/stable/development/building/#formatting-the-code

@Krechals
Copy link
Contributor Author

Krechals commented Nov 7, 2025 •
edited
Loading

Thanks for your time and for testing! I fixed the formatting.

zwass
zwass requested changes Nov 20, 2025
View reviewed changes
Copy link
Member

@zwass zwass left a comment

Choose a reason for hiding this comment

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

@Krechals we are continuing to see format issues. I'd love to get this merged for the next osquery release.

@Krechals Krechals force-pushed the fix-symlink-loop-tranversal branch from 654c5ee to 2da143f Compare December 1, 2025 10:45
@Krechals
Fix recursive traversal when the filesystem includes symbolic link loops
2134ef0 
Signed-off-by: Krechals <topala.andrei@gmail.com>
@Krechals Krechals force-pushed the fix-symlink-loop-tranversal branch from 2da143f to 2134ef0 Compare December 1, 2025 10:48
@Krechals
Copy link
Contributor Author

Krechals commented Dec 1, 2025

@zwass, the test should pass now

@zwass zwass self-assigned this Dec 2, 2025
zwass
zwass requested changes Dec 2, 2025
View reviewed changes
Copy link
Member

@zwass zwass left a comment

Choose a reason for hiding this comment

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

@Krechals I'm still seeing failures. Thank you for your persistence.

57: [ RUN      ] FilesystemTests.test_wildcard_double_folders
57: /__w/osquery/osquery/workspace/padding-required-by-rpm-packages/src/osquery/filesystem/tests/filesystem.cpp:437: Failure
57: Expected equality of these values:
57:   results.size()
57:     Which is: 3
57:   10U
57:     Which is: 10
57: 
57: /__w/osquery/osquery/workspace/padding-required-by-rpm-packages/src/osquery/filesystem/tests/filesystem.cpp:441: Failure
57: Value of: contains(results, fs::path(fake_directory_ / "deep11/deep2/deep3/") .make_preferred() .string())
57:   Actual: false
57: Expected: true

@zwass
Copy link
Member

zwass commented Dec 9, 2025

I updated to fix the failing test which seemed to be an issue with recursive directory handling. Will see if CI is good now.

@zwass
Copy link
Member

zwass commented Dec 10, 2025

My changes caused some tables (and associated tests) to break because there was an undocumented assumption that directories are returned with no trailing separators in listDirectoriesInDirectory. I updated to document this and add a test for it.

A few places with this assumption:
https://github.com/osquery/osquery/blob/master/osquery/tables/system/python_packages.cpp#L118-L131
https://github.com/osquery/osquery/blob/master/osquery/tables/applications/jetbrains_plugins.cpp#L339-L350
https://github.com/osquery/osquery/blob/master/osquery/tables/applications/browser_firefox.cpp#L194-L208

zwass
zwass approved these changes Dec 10, 2025
View reviewed changes
Copy link
Member

@zwass zwass left a comment

Choose a reason for hiding this comment

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

This is good with me, assuming tests pass. I've pushed changes so I don't think my review allows merging.

@zwass zwass merged commit 07aba27 into osquery:master Dec 10, 2025
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zwass zwass zwass approved these changes

Assignees

@zwass zwass

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SELECT * FROM file WHERE path LIKE "/%%" only traverses ≈3 levels deep

2 participants

@Krechals @zwass