Skip to content
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

do not attempt to delete existing cache #1183

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ericLemanissier
Copy link

@ericLemanissier ericLemanissier commented Oct 16, 2024

Description:
I don't think it's necessary to delete existing cache before overwriting it. https://github.com/actions/cache does not seem to do it. The problem with deleting a cache entry is that it requires to give actions: write permissions, which means the workflow has all these permissions and this is way too much, and not acceptable for a lot of projects.

Also, do not check if cache exists during restore. It introduces a TOCTOU issue, and is not needed because restoreCache returns undefined if the cache does not exist, cf https://github.com/actions/toolkit/tree/main/packages/cache

Related issue:
fixes #1159
fixes #1133
fixes #1131

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@ericLemanissier ericLemanissier marked this pull request as ready for review October 16, 2024 13:14
@ericLemanissier ericLemanissier requested a review from a team as a code owner October 16, 2024 13:14
It introduces a TOCTOU issue, and is not needed because `restoreCache` returns `undefined` if the cache does not exist
https://github.com/actions/toolkit/tree/main/packages/cache
@ericLemanissier
Copy link
Author

CI results available in https://github.com/ericLemanissier/stale/pull/21/checks

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