Exclude hidden files by default #598
Conversation
| @@ -9104,6 +9121,10 @@ class DefaultGlobber { | |||
| if (!stats) { | |||
| continue; | |||
| } | |||
| // Hidden file or directory? | |||
| if (options.excludeHiddenFiles && path.basename(item.path).match(/^\./)) { | |||
There was a problem hiding this comment.
what about a few common things like gh-pages' .nojekyll and even .gitignore?
| if (options.excludeHiddenFiles && path.basename(item.path).match(/^\./)) { | |
| if (options.excludeHiddenFiles && path.basename(item.path).match(/^\.(?!nojekyll$)/)) { |
There was a problem hiding this comment.
Thank you for the feedback, for now we are going to go ahead with the 'all in' option as starting to pick and choose common things is a risk we start to go back to 'too many' common things and expose folks to the risk of mistakes causing issues.
I get the idea though, maybe in the future we could look at 'more flags' for "all-common-none", but that will be something we need to revisit at a later date so we could get a better view on what common would be :)
There was a problem hiding this comment.
Well at least there should probably be a black/whitelist option rather than a bool methinks
There was a problem hiding this comment.
path is effectively that allowlist/denylist option - you can allow hidden files and then specify in path which to exclude
https://github.com/actions/upload-artifact#upload-using-multiple-paths-and-exclusions
| @@ -9104,6 +9121,10 @@ class DefaultGlobber { | |||
| if (!stats) { | |||
| continue; | |||
| } | |||
| // Hidden file or directory? | |||
| if (options.excludeHiddenFiles && path.basename(item.path).match(/^\./)) { | |||
There was a problem hiding this comment.
same here
| if (options.excludeHiddenFiles && path.basename(item.path).match(/^\./)) { | |
| if (options.excludeHiddenFiles && path.basename(item.path).match(/^\.(?!nojekyll$)/)) { |
|
This just broke my workflow. I think this warrants a breaking change. |
|
This has just broke 1000s of workflows that rely on artifacts produced by tools like pcov/phpunit etc. and any other reporting tools. This should have been released as a major version. |
Failed our release pipeline as well. Agree. |
|
Hey guys, even though this is definitely a good change, introducing a breaking change on a minor version isn't great. |
|
It's also likely extremely hard to debug for users. "Breaking" was included in #602's title so it's unforgivable to sneak this in a minor version. |
|
This change likely breaks every workflow that uploads the Did GitHub assess the number of (public) artifact uploads that would be affected by this change? I understand that avoiding upload of Some other possibilities:
The first option seems attractive to me, regardless of security implication, since the The second option minimizes harm. I must express my sympathy with all those who advocated for less impactful options, but did not prevail. Sadly, the OSS ecosystem must deal with the consequences. |
|
I'd still suggest reverting in a minor version and re-releasing in a major one regardless of implementation (#598, #599, #598 (comment), etc) |
|
Please direct all feedback to the tracking issue #602 |
Currently, all files within the search
pathare uploaded into the artifact. This includes hidden files, which could contain sensitive values that should not be accessible outside of the workflow run. To enhance the default security of this action, this PR excludes hidden files from thepathby default. Users who require hidden files have validated that there are no sensitive values in their artifacts can opt-in to uploading hidden files with theinclude-hidden-filesinput.This is part of the following breaking change: