Skip to content

Conversation

@feelepxyz
Copy link
Contributor

@feelepxyz feelepxyz commented Apr 4, 2023

Problem

runner_environment was added to the OIDC id token claims but it's currently not exposed in the runner context or ENV.

We would like to access the runner env value from the system ENV on the runner in order to include it in the provenance statement generated in the npm CLI when constructing the builder.id: https://github.com/npm/cli/blob/latest/workspaces/libnpmpublish/lib/provenance.js#L7

@feelepxyz feelepxyz changed the title WIP: Set runner environment in context and env Set runner environment in context and env May 23, 2023
@feelepxyz feelepxyz force-pushed the feelepxyz/add-runner-env branch 3 times, most recently from ed1209c to 8a1090d Compare May 23, 2023 16:15
@feelepxyz feelepxyz marked this pull request as ready for review May 23, 2023 16:15
@feelepxyz feelepxyz requested a review from a team as a code owner May 23, 2023 16:15
Extract runner_environment from the global context and expose in the
`github.runner` context and env as `RUNNER_ENVIRONMENT`.

Signed-off-by: Philip Harrison <[email protected]>
@feelepxyz feelepxyz force-pushed the feelepxyz/add-runner-env branch from 8a1090d to 6c40884 Compare May 26, 2023 08:15
@feelepxyz feelepxyz requested a review from TingluoHuang May 26, 2023 08:15
_runnerSettings = HostContext.GetService<IConfigurationStore>().GetSettings();
jobContext.SetRunnerContext("name", _runnerSettings.AgentName);

if (jobContext.Global.Variables.TryGetValue(WellKnownDistributedTaskVariables.RunnerEnvironment, out var runnerEnvironment))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TingluoHuang presuming this tryGetValue should make it safe to deploy this change independently of the backend change?

Also had a poke around and doesn't look like there's any allow list for what goes in the RUNNER_{KEY} ENV values: https://github.com/actions/runner/blob/main/src/Runner.Worker/RunnerContext.cs

Looks like the GitHub context does have an allow list: https://github.com/actions/runner/blob/main/src/Runner.Worker/GitHubContext.cs#L9

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have an allow list before for the RUNNER_* since before this change all RUNNER_* are set by the runner, while most GITHUB_* are coming from the service.


if (jobContext.Global.Variables.TryGetValue(WellKnownDistributedTaskVariables.RunnerEnvironment, out var runnerEnvironment))
{
jobContext.SetRunnerContext("environment", runnerEnvironment);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TingluoHuang another q: Looking at these docs: https://docs.github.com/en/actions/learn-github-actions/variables#detecting-the-operating-system - it seems you can access the runner context before the job runs, e.g. if: runner.os == 'Windows'. From our sync yesterday, is this a problem for us? Presuming it should be ok to set it here as everything else is set here too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if: runner.os == 'Windows' in the example in under steps (not before job land on the runner), which will be on the runner for sure.

TingluoHuang
TingluoHuang previously approved these changes May 26, 2023
@TingluoHuang TingluoHuang merged commit 21b49c5 into actions:main May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants