Skip to content

glob: skip error when subdirectory have not permission #1015

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

howyi
Copy link

@howyi howyi commented Mar 10, 2022

fix: #775

When globstar(**) is used in a directory with non-referenced subdirectories, the following differences exist between shell and @actions/glob.

Sample directory structure

root
├ package-lock.json
├ deniedDir  <-    chmod 000
│ └ package-lock.json
└ visibleDir
  └ package-lock.json

shell(bash/zsh): skip no permission subdirectories

$ls **/package-lock.json
visibleDir/package-lock.json
%ls **/package-lock.json
package-lock.json
visibleDir/package-lock.json

@actions/glob: error

glob = require('@actions/glob')

const p = new Promise(async (resolve, reject) => {
    const matchPatterns = '**/package-lock.json'
    console.log(`@actions/glob ${matchPatterns}`)
    const globber = await glob.create(matchPatterns)
    for await (const file of globber.globGenerator()) {
        console.log(file)
    }
});
p.then()

output

@actions/glob **/package-lock.json
::debug::followSymbolicLinks 'true'
::debug::implicitDescendants 'true'
::debug::matchDirectories 'true'
::debug::omitBrokenSymbolicLinks 'true'
::debug::Search path '/Users/takuya.hayashi/Documents/dev/howyi/gha-hashfiles-parmission-check'
[Error: EACCES: permission denied, scandir '/Users/takuya.hayashi/Documents/dev/howyi/gha-hashfiles-parmission-check/deniedDir'] {
  errno: -13,
  code: 'EACCES',
  syscall: 'scandir',
  path: '/Users/takuya.hayashi/Documents/dev/howyi/gha-hashfiles-parmission-check/deniedDir'
}

Problems caused by this difference

When using hashFiles in @action/cache, etc., you may use the form hashFiles('**/package-lock.json').
ex: https://docs.github.com/ja/actions/using-workflows/caching-dependencies-to-speed-up-workflows#example-using-the-cache-action

      - name: Cache node modules
        uses: actions/cache@v2
        env:
          cache-name: cache-node-modules
        with:
          path: ~/.npm
          key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
          restore-keys: |
            ${{ runner.os }}-build-${{ env.cache-name }}-
            ${{ runner.os }}-build-
            ${{ runner.os }}-

If there is an unprivileged directory, this caching process will fail with an error.

Related issue: actions/cache#753 (comment)

I have created a repository that reproduces this situation. Check here for details. 🐔
https://github.com/howyi/gha-hashfiles-parmission-check

@howyi howyi requested a review from a team as a code owner March 10, 2022 12:15
@howyi
Copy link
Author

howyi commented Mar 10, 2022

actions/runner#1678
With the release of this hashFiles fix a few days ago, CI for repositories with docker volume mounts has been failing in the last few days.
I noticed this behavior then.

As a result, I have seen many OSS repositories with PRs to not use globstar. (See PR linked in related issue).

@Chrischuck
Copy link

This seems to be a common issue w/ not only me, but many people using actions/cache. My tests have been failing because of this, would be awesome if we can get this merged! Thanks for making this pr @howyi

@thboop @konradpabjan would you be able to help get this over the line?

@howyi
Copy link
Author

howyi commented May 2, 2022

@thboop @konradpabjan @bishal-pdMSFT @brcrista @luketomlinson
Mention to those who appear to be maintainers.
Do you ever review this PR? When will it be reviewed?

@thboop
Copy link
Collaborator

thboop commented May 11, 2022

Hey @howyi , could you describe this user scenario a little more?

hashfiles taking into account all files is important because a lot of the time it is used to compute if we need to rebuild something, or if it is okay to pull it from the cache using something like actions/cache.

Are you mounting in volumes that the runner user does not have read access to? What purpose do those files or mounts serve?

Silently ignoring files we don't have permission to read could lead to unexpected behavior for users as well, so i'm curious what scenarios this fix enables.

@howyi
Copy link
Author

howyi commented May 12, 2022

@thboop Thanks for the reply!!!

could you describe this user scenario a little more?

I understand.
As far as I know, most of the cases where this is a problem are running Docker volume mounts on CI.

In my example, I had MySQL data mounted locally.

This technique is described on the MySQL DockerHub Where to Store Data at
https://hub.docker.com/_/mysql

If I had done this mount, the directory where mysql is mounted would be permission denied.

Silently ignoring files we don't have permission to read could lead to unexpected behavior for users as well, so i'm curious what scenarios this fix enables.’

I believe the current situation is behaving unexpectedly.
It should be doing the same thing as the behavior with globstar on the shell, but it is happening on CI with an error.
The more familiar I am with bash globstar, the more confused I think I am.

Also, unless you turn on DEBUG logging, this error is not detailed and you will spend a lot of time investigating.

@howyi
Copy link
Author

howyi commented Jun 7, 2022

@thboop Many people are facing this problem, could you review it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[glob] readdir causes EACCES: permission denied, scandir
3 participants