Wildcard expansion can affect INPUT_FILE_PATTERN before it's passed to git status/add

[ryandy]

Version of the Action v4.9.2

Describe the bug If INPUT_FILE_PATTERN contains a wildcard, the shell will attempt to expand it to the name(s) of matching files if they exist. This list of filenames will then be passed to git status and git add as part of _git_is_dirty() and _add_files(), instead of the originally specified string. The list of files (obtained from shell globbing) can differ from the list of files that would have matched using git pathspec.

To Reproduce

1. Start with a repo with two files:

   .
   ├── package
   │   └── module.py
   └── setup.py

2. Modify package/module.py
3. Run git-auto-commit-action with a file_pattern input of "*.py".
4. See output message of "Working tree clean. Nothing to commit". This is because the shell expanded INPUT_FILE_PATTERN from "*.py" to "setup.py" before passing it to git status. Because setup.py was not modified, git status will report no change.

Expected behavior git status should report that package/module.py was modified. And then git add should add package/module.py.

Possible fix Running set -o noglob in entrypoint.sh will turn off glob file expansion. This could probably be run at the top of the script, but could also be run in a more local, targeted fashion, so long as it covers both the call to git status and git add.

[stefanzweifel]

Thanks for reporting!

I think instead of setting set -o noglob it would probably be better to quote the $INPUT_FILE_PATTERN. What do you think?

For some reason I've deliberately disabled the shellcheck for these calls. (Can't remember why)

• https://github.com/stefanzweifel/git-auto-commit-action/blob/master/entrypoint.sh#L37
• https://github.com/koalaman/shellcheck/wiki/SC2086

[ryandy]

Hey Stefan, thanks for the quick response.

Quoting was also my first thought, but I don't think quoting will work in the case of specifying 2+ pathspecs, e.g.:

  file_pattern: "*.txt *.py"

I think if you were to quote $INPUT_FILE_PATTERN in this case, git would interpret it as a single pathspec with a space in the middle.

[stefanzweifel]

@ryandy Howdy, I've created a PR in #154 which would solve this problem. I've decided to add a new option called disable_globbing to prevent the shell from expanding the filenames.

Would be great if you could update your workflow like seen below and report, if this solution solves your problem.

-- uses: stefanzweifel/git-auto-commit-action@v4
+- uses: stefanzweifel/git-auto-commit-action@fixes/153
+    with:
+      disable_globbing: true

(As mentioned in the PR, I don't feel comfortable disabling globbing for all users. 🤷 )

[ryandy]

I tested it out, and that works for me. Thanks for pulling this together!

[stefanzweifel]

I've just tagged a new minor version which includes the disable_globbing option: v4.10.0.

I will close this issue now. Thanks again for reporting this!

[spookylukey]

Do you think disable_globbing: true should be the default instead?

I just spent several hours tracking down why this action no longer seemed to be working in my repo, when:

• it was definitely working before
• I had pinned all versions of all actions
• nothing else relevant had changed in the workflow.

It turns out someone added something.yml to the root of the repo, and from then on all changes to other */*.yml made by my linters were no longer being seen. This is a fairly nasty gotcha!

[stefanzweifel]

@spookylukey Thanks for bringing this up again. Sorry that you wasted hours on this issue :/

When I've added disable_globbing in #154 I've deliberately didn't make it the default as this StackOverflow answer made by nervous that it could break other stuff.

Wouldn't be something I would change in a minor version but something that we could consider doing in v5. (I currently don't have big plans for this Action, as it works great for my use case – and for many others too. I still have a private note where I collect improvements and changed that could be made in v5. Added this to the list)