Skip to content

fs: probe glob literals when parent listing is denied - #66172

Open
xia-chao wants to merge 2 commits into
nodejs:mainfrom
xia-chao:fs-glob-perm-literal-probe
Open

xia-chao wants to merge 2 commits into
nodejs:mainfrom
xia-chao:fs-glob-perm-literal-probe

Conversation

@xia-chao

@xia-chao xia-chao commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

--allow-fs-read=somedir/ still makes fs.globSync('somedir/*') (and the async APIs) return nothing. This is still true after native glob.

The walk starts at the current directory. If that parent is not readable, it is treated as “not a directory”, so the walk never takes the existing shortcut that stats somedir directly. #61552 patched the old JS walker; it does not apply to src/glob.

This change only keeps that shortcut when the next pattern part is a single literal, so a granted child can still be found. * and ** from the parent stay empty — those still have to list the directory.

Left is before, right is after. Same command, same tree:

before vs after

Fixes: #61499

A permission deny on the walk's starting directory was treated as
"not a directory", so VisitDirectory returned before the single-
literal shortcut that stats a granted child without listing cwd.
`fs.globSync('somedir/*')` with `--allow-fs-read=somedir/` then
matched nothing.

Fixes: nodejs#61499
Signed-off-by: Xia Chao <shapirolutts@gmail.com>
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Sep 21, 2026
Signed-off-by: Xia Chao <shapirolutts@gmail.com>
@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.28%. Comparing base (513981d) to head (a3d7b18).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/glob/glob_walker.cc 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66172      +/-   ##
==========================================
- Coverage   90.28%   90.28%   -0.01%     
==========================================
  Files         790      790              
  Lines      272043   272047       +4     
  Branches    51945    51934      -11     
==========================================
- Hits       245619   245605      -14     
- Misses      16931    16939       +8     
- Partials     9493     9503      +10     
Files with missing lines Coverage Δ
src/glob/glob_walker.cc 85.65% <80.00%> (-0.04%) ⬇️

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xia-chao

xia-chao commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Both shots are current main, before this PR.

First one: cwd is not on the allow-fs-read list, so the permission check fails.
a178138e61cfa42b04d2b2e28b0f448f

Second one: VisitDirectory then treats cwd as not a directory and returns. It never gets to the shortcut that would look at somedir without listing cwd.
1b45b8c9bd604c23c7d50981acd8f6d3

@jasnell Leaving these here in case they help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.glob (async and sync) returns no matches when --allow-fs-read is used

2 participants