Don't try to login to ghcr.io with GHES tokens #1291
Conversation
| @@ -494,7 +494,8 @@ private void ContainerRegistryLogout(string configLocation) | |||
| private void UpdateRegistryAuthForGitHubToken(IExecutionContext executionContext, ContainerInfo container) | |||
| { | |||
| var registryIsTokenCompatible = container.RegistryServer.Equals("ghcr.io", StringComparison.OrdinalIgnoreCase) || container.RegistryServer.Equals("containers.pkg.github.com", StringComparison.OrdinalIgnoreCase); | |||
| if (!registryIsTokenCompatible) | |||
| var isFallbackTokenFromHostedGithub = HostContext.GetService<IConfigurationStore>().GetSettings().IsHostedServer; | |||
There was a problem hiding this comment.
You mind want to check what's the runner version that we introduced IsHostedServer in the setting file and does that version included in the earliest GHES release.
There was a problem hiding this comment.
Otherwise, the IsHostedServer might return true on those GHES self-hosted runners.
There was a problem hiding this comment.
runner/src/Runner.Worker/JobExtension.cs
Lines 172 to 175 in 62d5686
Good thoughts, thanks for the feedback. These lines above gave me the confidence that IsHostedServer has been in runner versions since GHES Alpha.
Digging a bit more, this PR (#386) from 25 Mar 2020 confirms that IsHostedServer has been there since GHES Alpha.
Which was then rolled out in 2.168.0 (#420)
@TingluoHuang So any runners that were of a version lesser than 2.168.0 at the time they were configured would not benefit from this fix. Should we extend support that far back? If the life expectancy of a GHES runner instance is indeed very long (>~1y), we should either update their .runner with IsHostedServer, or calculate it runtime.
We could perhaps also consider adding settingsVersion: 2.xx.x to the settings so we could more easily handle differences like this.
When running GitHub Enterprise Server with a pool of JIT-configured runners (such as what [philips-labs/terraform-aws-github-runner](https://github.com/philips-labs/terraform-aws-github-runner/) provides), actions#1199 still occurs. The fix in actions#1291 uses the "IsHostedServer" property from the settings which is **not** set by the GHES endpoint for JIT configuration. This change computes the default value of IsHostedServer from the GitHub server URL, as is already done in [lots of other parts of the code](https://github.com/search?q=repo%3Aactions%2Frunner%20IsHostedServer&type=code) to make it consistent.
…rver When running GitHub Enterprise Server with a pool of JIT-configured runners (such as what [philips-labs/terraform-aws-github-runner](https://github.com/philips-labs/terraform-aws-github-runner/) provides), actions#1199 still occurs. The fix in actions#1291 uses the "IsHostedServer" property from the settings which is **not** set by the GHES endpoint for JIT configuration. This change computes the default value of IsHostedServer from the GitHub server URL, as is already done in [lots of other parts of the code](https://github.com/search?q=repo%3Aactions%2Frunner%20IsHostedServer&type=code) to make it consistent.
Fixes #1199
This PR prevents the usage of tokens from a GHES instance when authenticating against ghcr.io. Instead of failing the job due to a failure to authenticate, the runner can now access public images in an unauthenticated session.