Upcoming breaking change: Hidden Files will be excluded by default

[joshmgross]

From September 2nd, 2024, we will no longer include hidden files and folders as part of the default upload of the v3 and v4 upload-artifact actions. This reduces the risk that credentials are accidentally uploaded into artifacts. Customers who need to continue to upload these files can use a new option, ‘include-hidden-files’, to continue to do so.

https://github.blog/changelog/2024-08-19-notice-of-upcoming-deprecations-and-breaking-changes-in-github-actions-runners/

• https://github.com/actions/upload-artifact/pull/598
• https://github.com/actions/upload-artifact/pull/604

[dolfinus]

Why introducing breaking changes to v3 and v4 instead of making a new v5 version?

[uerkut]

      - name: Upload Docker Compose Files Artifact
        id: upload_docker_compose_files
        uses: actions/upload-artifact@v4.4.0
        include-hidden-files: true
        with:
          name: docker-compose-files
          path: |
            .env
            docker-compose.yaml

I've edited my workflow like above and getting the error:

The workflow is not valid. In .github/workflows/kondukto-package-build-manual.yml (Line: 318, Col: 11): Error from called workflow kondukto-io/kondukto-build/.github/workflows/docker_compose_package.yml@c343cea5db32d5d329cfe25ea5f7f84a88ecb64a (Line: 38, Col: 9): Unexpected value 'include-hidden-files'

I tried both versions v4 and v4.4.0

[dolfinus]

I've edited my workflow like above and getting the error:

You should place include-hidden-files: true under with: block

[uerkut]

Thanks a lot. My bad.

[catYalere]

Funny thing [in](include-hidden-files: true) doesn't work on v3, in one screenshoot I point directly to file and also doing a cat to see if it exists and exists

[buckett]

This change resulted in deployment failures for us because we no longer included the full set of files in our deployment artifact.

[Drowze]

This also resulted in a lot of CI failures for us... Would be nice if there was at least a warning before rolling breaking changes 🤷‍♂️

Here's how we were calling this action:

    - uses: actions/upload-artifact@v4
      with:
        name: artifacts-${{ strategy.job-index }}
        path: |-
          coverage/.resultset.json
          tests/junit.xml

[joshmgross]

Funny thing [in](include-hidden-files: true) doesn't work on v3, in one screenshoot I point directly to file and also doing a cat to see if it exists and exists

👋 Thanks for the report, we're rolling back the v3 and v3-node20 updates while we work on fixing that issue.

[gurvancampion]

Same here, I suddenly got an issue in my CI while deploying my app that never occurred before:

[tpope]

We also got burned by this. I see the logic in excluding hidden files recursively nested inside directories. But I struggle to see the value in ignoring an explicit path: .dotfile parameter.

[joshmgross]

Funny thing [in](include-hidden-files: true) doesn't work on v3, in one screenshoot I point directly to file and also doing a cat to see if it exists and exists

👋 Thanks for the report, we're rolling back the v3 and v3-node20 updates while we work on fixing that issue.

This is fixed in #608 & #609

https://github.com/actions/upload-artifact/releases/tag/v3.2.1 https://github.com/actions/upload-artifact/releases/tag/v3.2.1-node20

We're updating the major version tags v3 & v3-node20 to include those fixes.

[catYalere]

TY v3 with flag working

[antoniocoratelli]

Breaking changes should bump major version! What kind of semantic versioning is this?

And if old behavior poses security problems, issue a deprecation warning via EMAIL to organization owners that have repos using this very important action.

Breaking developers workflows on such a short notice (if writing a blog post can be considered an actual "notice") is NOT OK

[pdmtt]

This "minor" non-backward compatible API change also broke my organization's workflows by surprise.

In case it helps someone to troubleshoot, we were using this action to upload a directory created with pip install . --target <directory>, which was later downloaded by other runners for code and behaviour testing.

Apparently, pip installs dependencies in a hidden subdirectory. It stopped being uploaded as part of the artifact, which caused No module named <name> exceptions downstream.

As stated above, it was solved by including include-hidden-files: true as input.

[svlandeg]

This breaking change also broke the CI workflows for typer and fastapi, because the test suite was writing and then reading .coverage files ... We've also gone ahead and added include-hidden-files: true specifically to all workflows. Still, I would have expected to see this kind of change in v5, not in a minor release.

[mrgrain]

We also got burned by this. I see the logic in excluding hidden files recursively nested inside directories. But I struggle to see the value in ignoring an explicit path: .dotfile parameter.

Could we at least get explicitly named files back for v3 and v4? There doesn't seem to be a realistic scenario where uploading of explicitly enumerated files poses a security risk. As they have been added explicitly, they should fit the criteria of being vetted.

I can understand if you want a more coherent API. But please just release v5 for that.

[nedbat]

I'll add my voice to this comment:

We also got burned by this. I see the logic in excluding hidden files recursively nested inside directories. But I struggle to see the value in ignoring an explicit path: .dotfile parameter.

It's less secure to ignore my explicit instruction and force me to add another option that could possibly include hidden files I didn't intend. Please change this behavior.

To make an analogy: ls ignores hidden files. ls -a shows them. ls .gitignore shows me the hidden .gitignore file even without the -a flag.

[AlexSkrypnyk]

It got worse: now actionlint does needs to be updated as well https://github.com/rhysd/actionlint/issues/447

[andrewdcato]

Can we get something in the README about this project not following semantic versioning guidelines?

Releasing breaking changes in minor version bumps is nothing short of asinine.

[srinathreddydudi]

Wasted a lot of time trying to diagnose a broken system all of a sudden after deployment. Even tried rolling back the changes but no use. Luckily this wasn't a critical system. After spending a lot of time, Figured out this was the actual issue. As most of the people mentioned, Broken changes should not be introduced in minor versions.

I understand open source packages come with their own risk and maintenance challenges. We appreciate everyone who contribute to this and other open source packages. But official package from github actions which is used by thousands of repositories should follow basic semantic versioning guidelines. Especially when the package versioning itself is in the format of semantic versioning. Could we release a quick version which includes a link to this issue as a warning message so devs can quickly identify the exact issue and update their workflows?

I hope others who are experiencing this issue can quickly find this thread so they can resolve their issues quickly.

[MtkN1]

I understand the context of include-hidden-files: true: https://unit42.paloaltonetworks.com/github-repo-artifacts-leak-tokens/

The directory contains the hidden .git folder that stores the persisted GITHUB_TOKEN, leading the publicly accessible artifacts to contain the GITHUB_TOKEN.

Using the path: . option causes the token stored in the .git folder to leak. In this case, a breaking change might be reasonable to prevent this risk.

      - uses: actions/upload-artifact@v4
        with:
          name: artifact
          path: . 
          # or
          # path: "*" 

However, it also affects explicit options like path: .coverage, which has caused some confusion.

[jacklul]

If you have to make a breaking change this way at least add a logged warning to the workflow so people who get errors right now will have a potential error source right in their log

Spent too much time looking through my long workflow before stumbling on this thread... Semantic versioning is there for a reason and people who break it...

[rantoniuk]

Using the path: . option causes the token stored in the .git folder to leak. In this case, a breaking change might be reasonable to prevent this risk.

Instead of a breaking change to silently ignore all dotfiles, it would be just easier to ignore .git explicitly or via an added optional parameter that has a default of .git.

[jezdez]

I don't understand why this wasn't possible to be added with a default of true and then switch to false in a major version bump. The amount of effort needed for GitHub users to resolve issues caused by this rash rollout does not seem in relation to the security gains as it erodes trust in GitHub's engineering planning. Do better GitHub.

[grncdr]

Also here because CI workflows suddenly stopped working. Thankfully we didn't deploy anything broken 😅

I'd like to echo the other suggestions that an explicit path to a hidden file/directory should be allowed.

Also, it would be cool if this repo had a pinned issue where breaking changes are announced. I'm not subscribed to issues in this repo or the GitHub blog, but I would definitely subscribe to a pinned issue that contains only breaking change announcements.

[patricsteiner]

Putting this here, so people googling why their firebase deployment pipeline on github actions suddenly fails will hopefully find it.

Error: Deploy target admin not configured for project xxx. Configure with:
  firebase target:apply hosting yyy <resources...>

Even if you put your .firebaserc explicitly in the path option of the actions/upload-artifact@v4 action, the .firebaserc file will not be uploaded. You need to explicitly add include-hidden-files: true, like this:

      - uses: actions/upload-artifact@v4
        with:
          name: your-artifact
          include-hidden-files: true
          path: |
            dist/
            .firebaserc

[normanmaurer]

I also would like to raise that how this was rolled out is far from optimal.... Lost hours just because of this. While I agree this might be a good idea in general there should have been some notice at least before switching things in a non major version bump

[fleed]

I also lost hours due to this issue. Azure functions necessitate the presence of the hidden folder .azurefunctions, which inexplicably disappeared from the artifact. This change should have been implemented as a major version update, or as suggested with a warning message.

[bryan-brilivan]

Adding up here for people aggregating their simplecov test coverage using the upload artifact function getting No files were found with the provided path: coverage/.resultset.json. No artifacts will be uploaded.

Even with the path specifically mentioning the actual file, it will fail after this update. include-hidden-files needs to be specified.

        uses: actions/upload-artifact@v4
        with:
          name: coverage-${{ github.sha }}-${{ matrix.check_name }}
          path: coverage/.resultset.json
          retention-days: 1
          include-hidden-files: true

[sato11]

Is it possible that the team will manage to roll it back and re-release it as v5, or is that not even an option when the die has been cast? I feel like keeping from v4.4 in favor of v4.3.6 at least until this issue gets resolved, but am not sure when everything seems so full of confusion 😢

[Drowze]

I guess just for context for whoever is not aware: this is probably to address the recent news about the "ArtiPACKED" GitHub Actions hack/proof-of-concept (I won't link any article, but it should be easy to find more about it), where hackers could find accidentally leaked secrets in artifacts from hundreds of public repositories (from e.g.: Microsoft).

So while I agree this should be rolled as a major version change, I feel there must be a lot of pressure at GitHub right now to just ignore semver conventions. 🤷‍♂️

[Czaki]

I guess just for context for whoever is not aware: this is probably to address the recent news about the "ArtiPACKED" GitHub Actions hack/proof-of-concept (I won't link any article, but it should be easy to find more about it), hackers could found accidentally leaked secrets in artifacts from hundreds of public repositories (from e.g.: Microsoft).

How it could prevent from such an attack if attacker could just specify include-hidden-files: true in workflow file?

[nthekim]

We have big problem with the breaking change in v4 by building artifact for azure functions! By default azure functions need the hidden folder .azurefunctions which is now excluded! This was a big bang after deployment our services in PROD env and we are still fixing the problem ! This change should have been implemented as a major version!

[marc-gajdosik]

For the web-facing folks, check for your .well-known folders I guess 👀 https://en.wikipedia.org/wiki/Well-known_URI

[henriquecarv]

It sounds like it belonged in a major version bumping ☺️

[wszarmach-koia]

According to semver (https://semver.org/#spec-item-7) all breaking changes should be introduced in major versions not in minor ones, so we should be safe (especially when it is suggested to pin to the major version with example in configuration documentation: https://github.com/actions/upload-artifact?tab=readme-ov-file#usage) and there should be no need for such thread as @grncdr mentioned. On the other hand, I agree that this would be a good option, if actions were planned to deliver in the way that does not follow the rules. It could be also an email to the owners of GitHub accounts or technical maintainers (if there was such feature) for the affected accounts. I know that this has been announced in the blog, but two weeks period is too short for people as probably many do not read it without a reason and some might be absent within such short period of time.

@joshmgross I think we should get some explanation from the team whether we should expect breaking changes coming in non-major versions. Now, my team is not sure whether to trust in any changes from github.com/actions and we are considering how to prevent such situations as occurred today. It would be nice to know how we should work with those actions. It's really weird for me that it was worth sacrificing existing and tested pipelines to deliver a feature that protects users against themselves. It does not sound like anything important in comparison to the cost that it caused.

[m-ibot]

@wszarmach-koia I totally agree, that it should have been a major release. I ran in the issue with the breaking changes as well.

But you asked for strategies how to prevent these situations in the future. One thing could be to not only use @v4 as version, but the exact release instead. You can use every Tag as a version.

This of course has a downside as well: You will need to update the used version for the action in your code and you will not profit (but also not suffer ;) ) from every change that is done to the v4 branch. Tools like dependabot or renovate can help you to detect available updates and automate them where wanted.

Everyone has to weigh up for themselves whether the additional stability is worth the extra effort.

[wszarmach-koia]

@m-ibot Thanks for the reply. The suggestion to use exact version was one of the options. We had more to choose from like forking, pinning to minor versions, writing on our own. It would be useful to know what to expect to pick the right one.

The tools you mentioned might also change the approach. Thx.

[NielsJorck]

I also spent the past two hours debugging what was going on here. The default build artifacts from Nuxt is .nuxt or .output, which is being flagged as hidden files. I would also expect breaking changes only to occur in a mayor version, which is why I had it pinned to v4 currently.

[IanVS]

~Note that when using path: .hidden/ nothing will be uploaded, but when specifying path: .hidden/file, the file will be uploaded. That tripped me up for a bit.~

Edit: apparently this isn't accurate. But specifying a subfolder inside a hidden folder does seem to work for us.

[mrgrain]

Note that when using path: .hidden/ nothing will be uploaded, but when specifying path: .hidden/file, the file will be uploaded. That tripped me up for a bit.

We are NOT observing this behavior. For us .hidden/file will NOT be uploaded. (But if it works for you, that's great!)

[IanVS]

Interesting. How about path: .hidden/subfolder/? That's actually the case I have, I guess I assumed it was just the fact that a hidden folder wasn't the last thing being specified. (technically I'm using path: ./.hidden/subfolder/, but I can't imagine the leading ./ would matter...)

[mrgrain]

Interesting. How about path: .hidden/subfolder/? That's actually the case I have, I guess I assumed it was just the fact that a hidden folder wasn't the last thing being specified. (technically I'm using path: ./.hidden/subfolder/, but I can't imagine the leading ./ would matter...)

Ah I see. I'm guessing the leading ./ matters a lot. It is not an indication of a hidden file, but a reference to the current directory. Like ../ is a reference to the parent directory.

[IanVS]

Yes, but then there's a hidden folder right after it. Normally ./.hidden/ and .hidden/ would be equivalent.

[nebuk89]

On the breaking changes - we appreciate the feedback and are sorry for the pain :(

Sometimes we make the hard decision to make changes in place when we believe that the benefit of defaulting everyone to a more secure state/default is more beneficial for the majority than the lead time it takes for folks to move up to a new version.

This comes with the downside that it does break some folks, but we need to weigh up security for the majority and protect against mistakes as often as we can and with as much urgency as we can. It is very rare we make this decision, we did put out a changelog but appreciate there are a lot of these to keep an eye on.

[mrgrain]

@nebuk89 How about addressing the scenario that likely is NOT a security risk (explicitly enumerated hidden files)?

Edit: To be clear, I'm also happy to be told that we are wrong and that explicitly enumerated hidden files still pose a security risk.

[JoeStead]

I think there was a better way of handling this, it is a sensible change (and I would have preferred a major version for this breakage), but instead of that, generating warnings for a few weeks / a month or so in the output before releasing the breaking version would have been a nicer way of rolling out this change imo.

[andrewdcato]

It is very rare we make this decision, we did put out a changelog but appreciate there are a lot of these to keep an eye on.

Publishing notice of breaking changes for a dependency of over one million repositories in a blog post that wasn't accompanied by any sort of proactive, direct communication from GitHub to the owners of those repositories is frankly unacceptable.

[bartekpacia]

On the breaking changes - we appreciate the feedback and are sorry for the pain :(

Sometimes we make the hard decision to make changes in place when we believe that the benefit of defaulting everyone to a more secure state/default is more beneficial for the majority than the lead time it takes for folks to move up to a new version.

This comes with the downside that it does break some folks, but we need to weigh up security for the majority and protect against mistakes as often as we can and with as much urgency as we can. It is very rare we make this decision, we did put out a changelog but appreciate there are a lot of these to keep an eye on.

Ok, fine and dandy, but then please write in big letters in README that you do not follow semver, or have some specific understanding of it.

Actions exist for what... 6 years now? And we've all been exposed to that vulerability, and somehow we lived. Now tens of thousands of repos might require manual intervention because of this "fix".

As others have pointed out, there was absolutely no communication around it, and the notice was way too short.

[ken-cdit]

Wow this is a shocking breaking change... broke all our production builds and wasted a lot of time. This is incredibly bad form.

Considering myself lucky I even tracked this down already, I'm sure many are still scrambling right now.

edit: thinking about this more, this is so grossly irresponsible that I think you should revert it and do it in a v5