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

Security: lts/* installs old LTS versions up to 1 month+ after release #1236

Closed
2 of 5 tasks
karlhorky opened this issue Jan 27, 2025 · 15 comments
Closed
2 of 5 tasks
Assignees
Labels
feature request New feature or request to improve the current logic wontfix This will not be worked on

Comments

@karlhorky
Copy link

karlhorky commented Jan 27, 2025

Update: Delays are undefined, possibly up to 1 month or even longer, due to slow updates in actions/runner-images 😨

Description:

Using the lts/* alias with actions/setup-node installed Node.js v22.13.0 as of 25 Jan 2025, an old version. Node.js v22.13.1 has been out since 21 Jan 2025.

⚠️ Security: Node.js v22.13.1 contains security updates and as such, this can be considered a security problem

      - uses: actions/setup-node@v4
        with:
          node-version: 'lts/*'
          check-latest: true

Workflow logs:

Run actions/setup-node@v4
  with:
    node-version: lts/*
    always-auth: false
    check-latest: false
    token: ***
  ...
Attempt to resolve LTS alias from manifest...
Found in cache @ /opt/hostedtoolcache/node/22.13.0/x64
Environment details
  node: v22.13.0
  npm: 10.9.2
  yarn: 1.22.22

Longer update delays of over 5 days can be seen in #940

Action version:

actions/setup-node@v4

Platform:

  • Ubuntu
  • macOS
  • Windows

Runner type:

  • Hosted
  • Self-hosted

Tools version:

Node.js lts/*

  node: v22.13.0
  npm: 10.9.2
  yarn: 1.22.22

Repro steps:

Use the configuration above and observe the output above

Expected behavior:

Node.js latest LTS (20.11.0) is installed

Actual behavior:

Node.js older LTS (20.10.0) is installed

History:

Originally reported in #940, but closed without resolution, with @aparnajyothi-y saying that it should be handled on the runner side:

@aparnajyothi-y in comment 2247503445: cache eviction should not be handled on the runner side

In speaking with the runner images team, @hemanthmanga mentioned it should not be handled on the runner side:

@hemanthmanga in comment 2263151956: As the runner images team, we believe cache eviction should be handled through tasks, not the runner itself

@mahabaleshwars
Copy link

Hi @karlhorky,
Thank you for creating this issue. We will investigate it and provide feedback as soon as we have some updates.

@gowridurgad
Copy link

gowridurgad commented Jan 31, 2025

Hi @karlhorky, Thank you for reporting this issue. Based on your logs, it seems that although you have set check-latest: true in your YAML configuration, the logs show that the workflow is running with check-latest: false. This discrepancy has led to the installation of Node.js v22.13.0, the version cached on the virtual machine (VM), instead of the latest version, v22.13.1. Typically, the VM cache is up-to-date, and in the worst-case scenario, it can be 2-3 days old. When check-latest: true is passed, the action bypasses the VM cache and downloads the latest version directly from versions-manifest.json. Thus, using version: lts/* with check-latest: true should provide the exact behaviour you expect.

We have tested the setup with check-latest: true, and it successfully picks up the latest LTS version. Additionally, with check-latest: false, the action now resolves to the latest LTS version as well, as the VM cache has been updated to include the latest release. For your reference, we have attached a screenshots.

If you have any further questions or need additional support, please feel free to reach out to us.
Screenshot1: check-latest: false
Image
Screenshot2: check-latest: true
Image

@karlhorky
Copy link
Author

karlhorky commented Jan 31, 2025

@gowridurgad Thanks for the answer.

When check-latest: true is passed, the action bypasses the VM cache and downloads the latest version directly from versions-manifest.json

This is incorrect - the bug also occurs with check-latest: true:

      - uses: actions/setup-node@v4
        with:
          node-version: 'lts/*'
          check-latest: true

logs:

Run actions/setup-node@v4
  with:
    node-version: lts/*
    always-auth: false
    check-latest: true
    token: ***
  ...
Attempt to resolve LTS alias from manifest...
Found in cache @ /opt/hostedtoolcache/node/22.13.0/x64
Environment details
  node: v22.13.0
  npm: 10.9.2
  yarn: 1.22.22

To reproduce this, what you need is a stale, out-of-date cache and a new version that has been released.

it can be 2-3 days old

This is also incorrect - I've observed delay times of up to 5 days.

We have tested the setup with check-latest: true, and it successfully picks up the latest LTS version

Probably your tests are not reproducing the behavior correctly - you will require a stale, out-of-date cache and a new version that has been released in the last days.

If you can provide steps on how we can reproduce such a situation (eg. what steps we need to perform to insert outdated information in hostedtoolcache), I'm sure that we can together easily find a reproduction of this security bug.

@gowridurgad
Copy link

gowridurgad commented Feb 6, 2025

Hi @karlhorky, Thank you for your response. When the check-latest: true option is passed, the action ensures it bypasses the VM cache and directly downloads the latest version from the versions-manifest.json. You can find more details in the setup-node documentation. Typically, the versions-manifest.json file is updated within 1–2 working days after a new version is released, provided there are no unforeseen issues or failures. This ensures timely access to new versions for users relying on the check-latest: true option. However, the hosted tool cache updates follow the image update schedule maintained in the runner-images repository. During this time, while the tool cache update is in progress, the check-latest: true option compensates by directly fetching the latest version from the manifest, enabling users to access the most recent version without waiting for the image update.
To illustrate, regarding the recent PR #196 merged on January 22, the following behavior was observed: Before the PR was merged, using the check-latest option on January 21 fetched Node.js version 22.13.0. After the PR was merged, using check-latest on January 23 fetched the latest version 23.13.1 as defined in the updated versions-manifest.json. This highlights the role of check-latest: true in ensuring the latest version is fetched directly, even before the hosted tool cache reflects the change during the image update.
If you have further questions or need assistance, feel free to reach out, and we’ll do our best to assist.

@karlhorky
Copy link
Author

To illustrate, regarding the recent PR #196 merged on January 22

This page is 404, did you mean to link something else?

Image

Typically, the versions-manifest.json file is updated within 1–2 working days after a new version is released, provided there are no unforeseen issues or failures.

This has been demonstrated to be incorrect, with delays of up to 4 or 5 days, which is not really timely anymore.

Even 1 or 2 days is problematic.

@karlhorky
Copy link
Author

karlhorky commented Feb 6, 2025

@gowridurgad what about offering a default-fresh option for users (as fast as nvm for new version updates), with the default configuration that is recommended to users?

Eg. currently, the first "Basic" example in the readme is this:

steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
  with:
    node-version: 18
- run: npm ci
- run: npm test

It would be great if the default settings of actions/setup-node would not be 4 or 5 day delay on new updates (or even 1 or 2), but rather: as fast as nvm.

Good defaults out of the box with immediate access to security updates is something that I think is important to users.

@gowridurgad
Copy link

Hi @karlhorky, Thank you for pointing out the incorrect link in our previous comment. We’ve updated the comment with the correct link now. Apologies for the oversight.
As a follow-up to our earlier explanation, we would like to provide further clarification on the behaviour of the check-latest option:

After the new version was released, we merged the corresponding PR #197 on the same day. Before the PR was merged, the check-latest: true option fetched Node.js version 20.18.2 as shown in screenshot 1. After the PR merge, it fetched the updated version 20.18.3 as shown in screenshot 2. This shows how the check-latest option dynamically picks up the latest version directly from the versions-manifest.json.

Similarly, after the new version 22.14.0 was released, we merged PR #198 on the same day. Before the PR was merged, the check-latest: true option fetched Node.js version 22.13.1. After the PR merge, it fetched the updated version 22.14.0.

These examples demonstrate that the check-latest option ensures it fetches the most recent version directly from the versions-manifest.json, even before the hosted tool cache reflects the update. This behaviour allows users to access the latest version without delay, regardless of the image update schedule.

Please let me know if you have further questions or need additional details. We're here to help!

Screenshot 1:

Image

Screenshot 2:

Image

@karlhorky
Copy link
Author

karlhorky commented Feb 13, 2025

After the new version was released, we merged the corresponding PR #197 on the same day. Before the PR was merged, the check-latest: true option fetched Node.js version 20.18.2 as shown in screenshot 1. After the PR merge, it fetched the updated version 20.18.3 as shown in screenshot 2. This shows how the check-latest option dynamically picks up the latest version directly from the versions-manifest.json.

Similarly, after the new version 22.14.0 was released, we merged PR actions/setup-node#198 on the same day. Before the PR was merged, the check-latest: true option fetched Node.js version 22.13.1. After the PR merge, it fetched the updated version 22.14.0.

These examples demonstrate that the check-latest option ensures it fetches the most recent version directly from the versions-manifest.json, even before the hosted tool cache reflects the update. This behaviour allows users to access the latest version without delay, regardless of the image update schedule.

Ok, then what you're proposing is that with fast enough merging of the PRs by the team, the check-latest: true option does appear to work, at least in those tests provided above. In your tests, did you use a repo that already had a manifest? If not, then this does not prove what you were trying to prove.

What I have observed in the past is that check-latest: true does not update to the latest version, possibly because the PRs were not merged yet, but also possibly because the manifest caching / retrieval system is broken in a subtle way.

Two ideas for making this more bulletproof out of the box for users, regardless of how fast the PRs get merged or whether there is a bug in the manifest system:

  1. What about making check-latest: true the default when using an nvm alias?
  2. What about getting rid of the manifest + PR merging system (which sometimes fails even with check-latest, as I have observed), and instead downloading the version based on other existing tools like nvm?

Maybe you can agree that users should be able to have a working, updated config out of the box for nvm aliases, with no extra delays introduced by slow PR merging.

@gowridurgad
Copy link

Hi @karlhorky, We are marking this issue as blocked for now as we are currently discussing it with our team. Thank you for your patience and understanding.

@MikeMcC399
Copy link

MikeMcC399 commented Feb 20, 2025

lts/* currently uses Node.js 22.13.1

Node.js v22.14.0 was released on Feb 11, 2025, which is now 9 days in the past.

It should be updating much faster than this using default settings:

Run actions/setup-node@v4
  with:
    node-version: lts/*
    always-auth: false
    check-latest: false
    token: ***
Attempt to resolve LTS alias from manifest...
Found in cache @ /opt/hostedtoolcache/node/22.13.1/x64
Environment details
  node: v22.13.1
  npm: 10.9.2
  yarn: 1.22.22

Edit: ubuntu-24.04 Image Version: 20250223.1.1 contains Node.js v22.14.0. This represents approximately a 2 week delay in caching an updated Node.js Active LTS version in GitHub Runner Images.

@karlhorky karlhorky changed the title Security: lts/* installs old LTS versions up to 5 days after release Security: lts/* installs old LTS versions up to 9 days after release Feb 20, 2025
@karlhorky
Copy link
Author

@MikeMcC399 thanks for the update!

I've changed the title of the issue to include the "up to 9 days after release" metric.

@gowridurgad
Copy link

Hi @karlhorky, Thank you for your thoughtful suggestions and observations. We understand the concerns you're raising and would like to address them in detail regarding the check-latest: true option and the manifest + PR merging system.
Making check-latest: true the default:
When the setup-node action was initially implemented, the primary goal was to optimize workflow duration and reduce virtual machine (VM) costs. As a result, the check-latest option is disabled by default, allowing the action to use a cached version of Node.js on the VM.
Setting check-latest: true by default would increase action execution duration for all customers because Node.js would always download on-demand. This has performance implications as downloading versions of Node.js is slower than using cached versions.
Getting rid of the manifest + PR merging system:
The manifest system ensures that users have access to the latest versions in a timely manner. Typically, the versions-manifest.json file is updated on the same day a new version is released.
During the cache update process, the check-latest: true option ensures users still receive the most recent version by directly fetching it from the manifest, avoiding delays caused by waiting for the cache to update.
However, the hosted tool cache updates follow the image update schedule maintained in the runner-images repository. For users needing a more up-to-date cache more quickly, we are transfering this issue to the runner-images repository to address faster cache updates.

@HarithaVattikuti HarithaVattikuti transferred this issue from actions/setup-node Feb 25, 2025
@erik-bershel erik-bershel transferred this issue from actions/runner-images Feb 26, 2025
@karlhorky
Copy link
Author

karlhorky commented Mar 6, 2025

@gowridurgad Ok great, so the problem is the slow hosted tool cache updates / image update schedule from actions/runner-images.

I see that:

  1. The issue was transferred to actions/runner-images
  2. Then @erik-bershel transferred it back here to actions/setup-node

I'm unsure why 2 happened, or what the reasons / justification behind it was.

But if actions/runner-images and actions/setup-node cannot agree on the responsibility of fixing the issue, maybe revisiting a rework of the current system is good.

I reworded the suggestion:

What about reworking the Node.js manifest update system to remove reliance on the slow actions/runner-images hosted tool cache updates? (potentially instead downloading the version based on other existing tools like nvm)

@erik-bershel
Copy link

In the current situation, shifting the solution to runner-images is pointless. Images will not be updated more often due to architectural limitations. Accordingly, there will always be a chance that cached versions are out of date for a period of one to several days (most likely up to six days, but there are exceptions, like now with macOS, where the image has not been updated for a month or so).

We provide a cache - on the side of other users (including setup-* actions) the choice to use it or download a "more current" version, if desired.

In general, there is no problem as such in principle, given that the discussed action supports both behaviours (use cache if available / force download the latest version). This option is documented on the main page of the tool and the choice is up to the user. The default behaviour is intended to reduce execution time, which is currently a higher priority for the community than the relevance of the package version.

Note: Like the other values, * will get the latest [locally-cached Node.js version](https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#nodejs), or the latest version from [actions/node-versions](https://github.com/actions/node-versions/blob/main/versions-manifest.json), depending on the [check-latest](https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#check-latest-version) input.

@erik-bershel erik-bershel closed this as not planned Won't fix, can't repro, duplicate, stale Mar 6, 2025
@erik-bershel erik-bershel added wontfix This will not be worked on feature request New feature or request to improve the current logic and removed bug Something isn't working labels Mar 6, 2025
@karlhorky karlhorky changed the title Security: lts/* installs old LTS versions up to 9 days after release Security: lts/* installs old LTS versions up to 1 month after release Mar 6, 2025
@karlhorky
Copy link
Author

karlhorky commented Mar 6, 2025

most likely up to six days, but there are exceptions, like now with macOS, where the image has not been updated for a month or so

@erik-bershel Hm, that is very surprising default behavior for actions/setup-node with any aliases or version ranges.

Users do not like surprising caching behavior. See as a reference the community feedback to the "cached by default" strategy for Next.js over the last years, which Next.js eventually rolled back.

This option is documented on the main page of the tool and the choice is up to the user.

Note: Like the other values, * will get the latest [locally-cached Node.js version](https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#nodejs), or the latest version from [actions/node-versions](https://github.com/actions/node-versions/blob/main/versions-manifest.json), depending on the [check-latest](https://github.com/actions/setup-node/blob/main/docs/advanced-usage.md#check-latest-version) input.

This documentation doesn't imply that the default caching behavior can lead to up to 1 month of delay.

When hearing "latest locally-cached Node.js version" from software from a company with a good reputation like GitHub, I would not imagine that it's significantly out of date, as is the case in the scenarios that we've explored. (and 1 month is of course even more so)

So I would suggest one of two things:

  1. If users use aliases or ranges, disable the cache with check-latest: true (unless this is also still broken and doesn't retrieve the latest version, which I have experienced a few times too)
  2. If the default behavior won't change, change the documentation to show that usage of aliases is dangerous with the default actions/setup-node and can lead to undetermined delays of up to 1 month or longer

@karlhorky karlhorky changed the title Security: lts/* installs old LTS versions up to 1 month after release Security: lts/* installs old LTS versions up to 1 month+ after release Mar 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request to improve the current logic wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

6 participants