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

build: move GitHub actions to versions allowed by prodsec #17238

Merged
merged 2 commits into from
May 19, 2023

Conversation

tgross
Copy link
Member

@tgross tgross commented May 18, 2023

The backspace/ember-asset-size action we're using is unmaintained and has a bunch of vulns in it, so it won't pass security screening (this is a NodeJS action so it has piles of dependencies, 99% of which won't be in use but fails automated screening anyways). Move this to the upstream version.

The machine-learning-apps/pr-comment action also presents a problem for the ProdSec security screening because it's archived and also runs an external Docker image. Move this to a likely-ok maintained action for now, until we can spare some time to remove this in lieu of something more reasonable that isn't a GitHub action.

Once this is tested, I'll then have to follow up in https://github.com/hashicorp/security-tsccr/pull/428 to pin the SHA, and then I'll have to follow-up once that other PR is merged to pin the SHAs here.

The `backspace/ember-asset-size` action we're using is unmaintained and has a
bunch of vulns in it, so it won't pass security screening (this is a NodeJS
action so it has piles of dependencies, 99% of which won't be in use but fails
automated screening anyways). Move this to the upstream version.

The `machine-learning-apps/pr-comment` action also presents a problem for the
ProdSec security screening because it's archived and also runs an external
Docker image. Move this to a likely-ok maintained action for now, until we can
spare some time to remove this in lieu of something more reasonable that isn't a
GitHub action.
Copy link
Contributor

@philrenaud philrenaud left a comment

Choose a reason for hiding this comment

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

Thanks, Tim! Any idea why we were using forked / lesser-used versions before?

@github-actions
Copy link

github-actions bot commented May 18, 2023

Ember Test Audit comparison

main f796cdb change
passes 1487 1487 0
failures 4 4 0
flaky 0 0 0
duration 000ms 000ms -000ms

@tgross
Copy link
Member Author

tgross commented May 18, 2023

Thanks, Tim! Any idea why we were using forked / lesser-used versions before?

@backspace is a former Nomad team member, so it looks like we wanted this: backspace/ember-asset-size-action@040f42b which changes the action to update an existing comment (I think?). I don't know if that ever got upstreamed. But it also didn't trigger for this PR. Is there a way to make that trigger here so we can make sure it still works?

Also, we have this fork too, https://github.com/backspace/ember-test-audit which is a fork of https://github.com/DingoEatingFuzz/ember-test-audit. It looks like @DingoEatingFuzz's version is further along. Should we switch to that as well?

@philrenaud
Copy link
Contributor

At a glance yeah I think using @DingoEatingFuzz 's ember-test-audit is the right move (Michael, please let us know if you think otherwise!)

My (very limited!) understanding was that GH Actions run in the PR in which they're being added, so also surprised that asset-size didn't run. But we can revisit that later if it doesn't materialize in future PRs.

@tgross
Copy link
Member Author

tgross commented May 18, 2023

At a glance yeah I think using @DingoEatingFuzz 's ember-test-audit is the right move (Michael, please let us know if you think otherwise!)

Sorry, I mixed up ember-test-audit (which is the program being run) with https://github.com/backspace/ember-test-audit-comparison-action (which is the thing with the action.yml that lets us run in GHA). That is not a fork. I'm an advocate of vendoring things like this into the HashiCorp organization, but I don't need to do that to ship this.

I'm going to update https://github.com/hashicorp/security-tsccr/pull/428 with the resulting changes, and then after that's merged I'll update this PR with the pinned SHAs.

@tgross tgross self-assigned this May 18, 2023
@tgross tgross added this to the 1.6.0 milestone May 18, 2023
@tgross
Copy link
Member Author

tgross commented May 19, 2023

https://github.com/hashicorp/security-tsccr/pull/428 has been merged and I've pinned all workflows. Once CI is green I'll merge and backport this.

@tgross tgross merged commit c9f4425 into main May 19, 2023
@tgross tgross deleted the gha-actions-shuffle branch May 19, 2023 13:07
tgross added a commit that referenced this pull request May 19, 2023
The file path in the TSCCR repo for the `returntocorp/semgrep` action was
incorrect, so the pinning tool was not able to find the correct entry and it was
not pinned in #17238.

The repository is fixed in hashicorp/security-tsccr#431
tgross added a commit that referenced this pull request May 19, 2023
The file path in the TSCCR repo for the `returntocorp/semgrep` action was
incorrect, so the pinning tool was not able to find the correct entry and it was
not pinned in #17238.

The repository is fixed in hashicorp/security-tsccr#431
@tgross tgross added backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line labels May 19, 2023
@tgross
Copy link
Member Author

tgross commented May 19, 2023

Despite the very confusing PRs linked-to above, this was in fact backported to 1.3.x.

tgross added a commit that referenced this pull request May 19, 2023
The file path in the TSCCR repo for the `returntocorp/semgrep` action was
incorrect, so the pinning tool was not able to find the correct entry and it was
not pinned in #17238.

The repository is fixed in hashicorp/security-tsccr#431
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.3.x backport to 1.3.x release line backport/1.4.x backport to 1.4.x release line backport/1.5.x backport to 1.5.x release line theme/build-infrastructure theme/ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants