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

[actions] add step security runner #2659

Merged
merged 1 commit into from
Dec 10, 2021

Conversation

varunsh-coder
Copy link
Contributor

This PR adds the step-security/harden-runner GitHub Action to the lint.yml workflow to restrict outbound calls. This is a new GitHub Action that provides runtime security for GitHub hosted runner. As of now, it can be used to set allowed outbound traffic for workflow runs to prevent credential exfiltration. You can see the results of running this on a fork here.

@ljharb would love to pilot step-security/harden-runner on this workflow to get feedback and improve the experience. Do let me know if you have any questions. Thanks!

Comment on lines 51 to 54
allowed-endpoints:
agent.api.stepsecurity.io:443
github.com:443
nodejs.org:443
registry.npmjs.org:443
Copy link
Member

Choose a reason for hiding this comment

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

is there any way an individual step could be annotated rather than broadly allowing every step to access these endpoints?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @ljharb , individual steps are annotated in the web UI/ API (thanks for that idea BTW) and I can detect and alert that a step has called an endpoint it was not supposed to, but technically I cannot stop it from doing so. Configuring the firewall per step would be hard. Let me know if it would be better to just detect and alert at a step level v.s. restricting at a job level.

Copy link
Member

Choose a reason for hiding this comment

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

What if i use the runner multiple times, and each time, I define a brand new set of allowed endpoints, that only apply to further steps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats an interesting idea. I can experiment with it. It might lead to lot of steps that call harden-runner which may make the workflow file look cluttered. It would be ideal if this could be done without the additional steps. Let me think about it.

with:
allowed-endpoints:
agent.api.stepsecurity.io:443
github.com:443
Copy link
Member

Choose a reason for hiding this comment

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

for example, the actions/checkout action both always should be able to access github.com, and also, is the only part of the action that needs to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I understand what you are saying, and that would be ideal. I wrote about this in the comment above.

Copy link

Choose a reason for hiding this comment

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

**

.github/workflows/lint.yml Outdated Show resolved Hide resolved
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

This looks good; linking it to main means you can't ever make breaking changes; should it be pointed to a branch or tag?

Any update about per-step allowances?

@varunsh-coder
Copy link
Contributor Author

This looks good; linking it to main means you can't ever make breaking changes; should it be pointed to a branch or tag?

Any update about per-step allowances?

Pointed it to the latest tag.

No update on per-step restriction. Will take more time to address it...

@varunsh-coder
Copy link
Contributor Author

Hi @ljharb made few updates to the PR.

  1. Updated tag to v1 (now managing it in same way as actions/ checkout)
  2. Added egress-policy as an input in the action with default of block. One of the jobs has it as audit just to pilot it.
  3. The web UI now has annotations (tooltip/ dialog) about why certain calls are made by an Action based on KB.
    Thanks for the feedback!

@ljharb
Copy link
Member

ljharb commented Dec 7, 2021

Could we add this to all the other workflows as well?

@varunsh-coder
Copy link
Contributor Author

Could we add this to all the other workflows as well?

Hi @ljharb would love to add to other workflows.

That is also a great opportunity to pilot the automation to add it to workflows.

Secure workflows is an open source API to make it easy to add harden-runner (and token permissions) to workflows, but as of now it is only available via a web UI. Instead it would be better to have the automation create PRs. One of the challenges here is that the first PR would be to add egress-policy: audit and after a couple of runs another PR will add the allowed-endpoints.

To create PRs via automation, I could write a GitHub App or an OAuth App (or both). I believe that in an OAuth App the access is granted to all repos for a given user. So, I think GitHub App would be preferred for this scenario. Would you be ok to pilot a GitHub app for this? It would need write access to code, workflows, and PRs. It will be open source (part of secure workflows).

Please let me know. Thanks!

@ljharb
Copy link
Member

ljharb commented Dec 7, 2021

I'm a bit confused; can't the PR here just start out setting allowed endpoints to "nothing", and then it would fail, and i could update the list to get the PR to pass?

@varunsh-coder
Copy link
Contributor Author

I'm a bit confused; can't the PR here just start out setting allowed endpoints to "nothing", and then it would fail, and i could update the list to get the PR to pass?

I added harden-runner to all workflows except the windows-npm.yml since it only works on Linux.
For couple of jobs, there were no outbound calls (just a single statement - run: 'echo tests completed'), so set egress-policy: block.
The automation rebase workflow is failing because GitHub API is returning the PRs rebaseable attribute as false. Not sure why that is the case...

@ljharb
Copy link
Member

ljharb commented Dec 7, 2021

I'm not sure either, don't worry about it tho.

@varunsh-coder
Copy link
Contributor Author

Hi @ljharb based on the outbound traffic in shellcheck.yml workflow, I realized that homebrew sends data to Google Analytics. It can be opted-out of and removing www.google-analytics.com from the allowed list also works - the call is dropped and the workflow still works (tried on a fork). Do let me know if you want me to remove www.google-analytics.com from the allowed list for shellcheck.yml workflow in PR.

@ljharb
Copy link
Member

ljharb commented Dec 8, 2021

Let's both remove it and set HOMEBREW_NO_ANALYTICS=1 in the env.

@varunsh-coder
Copy link
Contributor Author

Let's both remove it and set HOMEBREW_NO_ANALYTICS=1 in the env.

Done

.github/workflows/lint.yml Outdated Show resolved Hide resolved
@ljharb ljharb changed the title Secure workflow lint.yml [actions] add step security runner Dec 10, 2021
@ljharb ljharb added the testing Stuff related to testing nvm itself. label Dec 10, 2021
@ljharb
Copy link
Member

ljharb commented Dec 10, 2021

ha, i couldn't figure out why the dockerfile_lint job was failing, but turns out it's the harden runner doing its job: https://github.com/nvm-sh/nvm/compare/4aa8f97c9d9a9df72ab7845e859184c7c932d6d6..6cc90a4b8daa44ad3050828c2bf98fa77a16bf90

@ljharb ljharb merged commit 6cc90a4 into nvm-sh:master Dec 10, 2021
@ljharb
Copy link
Member

ljharb commented Dec 10, 2021

@varunsh-coder this is another reason why a separate PR status would be super helpful, because then it would be very obvious when a workflow had requests blocked by the harden runner.

@varunsh-coder
Copy link
Contributor Author

@varunsh-coder this is another reason why a separate PR status would be super helpful, because then it would be very obvious when a workflow had requests blocked by the harden runner.

I will work on this. Will also show in annotation in the workflow when a request is blocked.

Don't know how I missed those endpoints in the dockerfile_lint job. May be it was using cached tools/ dependencies earlier, so those endpoints were not called...

@varunsh-coder varunsh-coder deleted the varunsh-coder-patch-1 branch December 18, 2021 00:55
Copy link

@jen801 jen801 left a comment

Choose a reason for hiding this comment

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

with:
allowed-endpoints:
agent.api.stepsecurity.io:443
github.com:443
Copy link

Choose a reason for hiding this comment

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

**

Copy link

@jen801 jen801 left a comment

Choose a reason for hiding this comment

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

Duplicate of #
Duplicate of #

with:
allowed-endpoints:
iojs.org:443
nodejs.org:443
Copy link

Choose a reason for hiding this comment

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

.github/workflows/toc.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Stuff related to testing nvm itself.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants