Skip to content

Conversation

@thboop
Copy link
Collaborator

@thboop thboop commented Oct 30, 2020

Context

A moderate security vulnerability has been identified in the GitHub Actions runner that can allow environment variable and path injection in workflows that log untrusted data to stdout. This can result in environment variables being introduced or modified without the intention of the workflow author. To address this issue we have introduced a new set of files to manage environment and path updates in workflows. This PR disables the old commands.

Blog Post

Mitigation

If you are using these old commands, the steps that use them in your workflow will fail. You will want to move towards using Environment Files.

You may also opt into unsecure command execution as well, at a job level or for all jobs on your self hosted runner. We recommend you do not choose to do this, and instead update to the new Environment Files.

  • To opt in at a job level, set ACTIONS_ALLOW_UNSECURE_COMMANDS to true
jobs:
  test:
    runs-on: self-hosted
    env:
      ACTIONS_ALLOW_UNSECURE_COMMANDS: true
  • To opt in for all jobs run on a self hosted runner, set ACTIONS_ALLOW_UNSECURE_COMMANDS=true in the .env file found at the root of the runner, much like you would set an http_proxy

@thboop thboop marked this pull request as draft October 30, 2020 19:02
@thboop thboop marked this pull request as ready for review November 13, 2020 19:13
@thboop thboop force-pushed the disableOldRunnerCommands branch from 58328f2 to c7f8980 Compare November 13, 2020 20:10
var configurationStore = HostContext.GetService<IConfigurationStore>();
var isHostedServer = configurationStore.GetSettings().IsHostedServer;

{
Copy link
Member

Choose a reason for hiding this comment

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

should this method only has one line?

throw new Exception(String.Format(Constants.Runner.UnsupportedCommandMessageDisabled, this.Command));

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See above :)

public Type ExtensionType => typeof(IActionCommandExtension);

public void ProcessCommand(IExecutionContext context, string line, ActionCommand command, ContainerInfo container)
{
Copy link
Member

Choose a reason for hiding this comment

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

should this method only has one line?

throw new Exception(String.Format(Constants.Runner.UnsupportedCommandMessageDisabled, this.Command));

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No because we want users to be able to set ACTIONS_ALLOW_UNSECURE_COMMANDS to use the old commands if needed, some actions may not have updated already and giving users a workaround is helpful until they do get updated!

Copy link
Member

Choose a reason for hiding this comment

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

confirmed with @chrispat
APPROVED.

Copy link
Contributor

@juliobbv juliobbv left a comment

Choose a reason for hiding this comment

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

Changes LGTM 👍

@thboop thboop merged commit a2e3217 into main Nov 16, 2020
@thboop thboop deleted the disableOldRunnerCommands branch November 16, 2020 13:20
thboop added a commit that referenced this pull request Nov 16, 2020
* Disable Old Runner Commands set-env and add-path

* update dotnet install scripts

* update runner version and release notes
AdamOlech pushed a commit to antmicro/runner that referenced this pull request Jan 28, 2021
* Disable Old Runner Commands set-env and add-path

* update dotnet install scripts

* update runner version and release notes
TingluoHuang pushed a commit that referenced this pull request Apr 21, 2021
* Disable Old Runner Commands set-env and add-path

* update dotnet install scripts

* update runner version and release notes
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.

4 participants