Open
Description
toolkit/packages/glob/src/internal-globber.ts
Lines 142 to 145 in fc00528
As you can see from the above snippet, internal-globber.ts
doesn't check if the directory is readable before attempting to read it.
There should be a try catch block around that code:
try {
const childLevel = item.level + 1;
const childItems = (await fs.promises.readdir(item.path)).map(
x => new SearchState(path.join(item.path, x), childLevel)
);
stack.push(...childItems.reverse());
catch (err) {
if (err.code === 'EACCES') {
continue;
}
throw err;
}
This issue affects the upload-artifacts
action, plus I'm sure other actions that depend on this package.
Activity
luketomlinson commentedon May 13, 2021
Hi @smac89 would you be able to give a little more information on your desired use case here? Would your desired behavior be to silently ignore directories without read permission? My main worry about a behavior change is users accidentally not matching a directory and never knowing it.
smac89 commentedon May 14, 2021
@luketomlinson No ignoring it would only lead to more problems down the line.
Maybe create a new api function called
tryGlob
which behaves similar toglob
but returns an object which contains (in addition to the list) anerror
field that can be used to view the error message.This way doesn't break compatibility with existing users, while allowing them to opt into the new behavior.
howyi commentedon Apr 14, 2022
@luketomlinson @smac89
PR created due to a problem.
#1015
smac89 commentedon Apr 18, 2022
@howyi that looks good. I'm not a code owner or reviewer, but thanks for creating the PR. Maybe @thboop could take a look