feat: workflow to update actions dist #760

ramonpetgrave64 · 2024-04-23T20:40:23Z

Add a new Post-Commit workflow, to make these renovate-bot updates a bit easier.
Previously, we had to clone the PR locally, run make package, and then push to the PR.
Now we would just need to use the github UI to invoke this new workflow against the PR number.
We could also copy this over to the slsa-github-generator repo.

A workflow to run against renovate-bot's PRs,
such as make package after it updates the package.json and package-lock.json files.
The potentially untrusted code is first run inside a low-privilege Job, and the diff is uploaded as an artifact.
Then a higher-privilege Job applies the diff and pushes the changes to the PR.
It's important to only run this workflow against PRs from trusted sources, after also reviewing the changes!

Testing.

Tested in my own private fork, where when applicable, it pushed a commit of changes to dist/ folders

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ramonpetgrave64 · 2024-04-23T20:41:24Z

@laurentsimon @mihaimaruseac @ianlewis

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

.github/workflows/post-commit.yml

laurentsimon

LGTM. Added a few nits.

.github/workflows/post-commit.yml

laurentsimon · 2024-04-23T21:28:10Z

.github/workflows/post-commit.yml

+        runs-on: ubuntu-latest
+        permissions:
+            pull-requests: read
+            contents: write


Add a comment saying we use CODEOWNERS and it's enforced thru Branch rules, so the token cannot be used to push to the main and release branches.

@mihaimaruseac can you take a look at this PR? We want to update the PR branch for renovatebot PRs

We shouldn't have any branches other than main and release, so we can add a ruleset that prevents branch creation. Or we could add a ruleset that only lets maintainers create branches, which would prevent this action from pushing to new branches without stopping us from doing so as needed.

mihaimaruseac

This seems a little bit more complex. Can this be done similar to guacsec/guac#953 (comment) ?

.github/workflows/post-commit.yml

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ramonpetgrave64 · 2024-04-24T16:31:31Z

This seems a little bit more complex. Can this be done similar to guacsec/guac#953 (comment) ?

I used the gh cli because it has some conveniences like setting up the correct remote branch for the PR. If we used actions/checkout within a normal pull_request event, then we wouldn't need to use gh cli.

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com>

ramonpetgrave64 · 2024-04-24T18:55:41Z

@kpk47 And I added some more Repo rules that make this workflow even safer to use. Safer than cloning the PR locally and running the untrusted code.

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

.github/workflows/post-commit.yml

ianlewis · 2024-04-30T00:13:46Z

.github/workflows/post-commit.yml

@@ -0,0 +1,94 @@
+# A workflow to run against renovate-bot's PRs,


nit: add a doc section to the CONTRIBUTING.md that outlines how to use this workflow.

No existing contributing docs, but I added one.

ianlewis · 2024-04-30T00:22:34Z

.github/workflows/post-commit.yml

+            - name: checkout-pr
+              env:
+                  GH_TOKEN: ${{ github.token }}
+              run: gh pr checkout ${{ inputs.pr_number }}


After git pr checkout, what is the git remote and branch set to? i.e. where does a git push go to?

The remote and branch get set to the PRs repo and branch, and it pushes there.

ianlewis · 2024-04-30T00:23:54Z

.github/workflows/post-commit.yml

+# There have been vulnerabilities with using `git apply` https://github.blog/2023-04-25-git-security-vulnerabilities-announced-4/
+# At this point a compromised git binary cannot modify any of this repo's branches, only the PR fork's branch,
+# due to our branch protection rules and CODEOWNERS.
+# It aslso cannot submit a new release or modify exsiting releases due to tag protection rules.


Is there any reason this needs to run on the slsa-framework/slsa-verifier repo at all? could contributors just run it manually on their fork? That would somewhat limit exposure on our side and reduces toil for reviewers (i.e. if the dist check fails we could just tell them "run this" and point to the doc on how to run it in their fork. better yet, the error from that check could tell them).

AFAICT, this would just checkout the forked PR and push to the appropriate branch on the forked repo so wouldn't need to interact with (and thus need permissions for) the main repo at all.

Also makes it easier for us to audit since we would basically just makes sure there aren't any runs for this action on the main repo

This is mainly intended to run on PRs from renovate-bot's fork. We wouldn't be able to invoke their fork's workflows.

Example:

fix(deps): update dependency @actions/core to v1.10.1 #717

https://github.com/renovate-bot/slsa-verifier/tree/renovate/npm

https://github.com/renovate-bot/slsa-verifier/actions/workflows/pre-submit.actions.yml

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com>

.github/workflows/update-actions-dist-post-commit.yml

Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com>

ramonpetgrave64 · 2024-05-06T21:56:45Z

Thanks everyone for the reviews

# Summary Similar to slsa-verifier's slsa-framework/slsa-verifier#760 This PR adds a manually-invoked workflow to run against renovate-bot's PRs to update the node `dist` folders. I made one small change to use the `${{ inputs.pr_number }} ` as an environment variable, to harden against [script injection](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#good-practices-for-mitigating-script-injection-attacks). See also slsa-framework/slsa-verifier#771 Also updating shellckeck to fix this lint error: - https://github.com/slsa-framework/slsa-github-generator/actions/runs/9101693389/job/25019502486#step:4:21 ``` Error: input type of workflow_dispatch event must be one of "string", "boolean", "choice", "environment" but got "number" ``` ## Testing Process I ran this against my fork's version of PR #3649. It did update the dist folders and the check-dists checks pass - https://github.com/ramonpetgrave64/slsa-github-generator/actions/runs/9101190828/job/25017786420?pr=9 - https://github.com/slsa-framework/slsa-verifier/pull/760/files#diff-4c6b93aa75d5affde60dc3849606c9acd75ed444d52e99f3055fc0c7aa77e9e0 ## Checklist - [x] Review the contributing [guidelines](https://github.com/slsa-framework/slsa-github-generator/blob/main/CONTRIBUTING.md) - [ ] Add a reference to related issues in the PR description. - [x] Update documentation if applicable. - [ ] Add unit tests if applicable. - [ ] Add changes to the [CHANGELOG](https://github.com/slsa-framework/slsa-github-generator/blob/main/CHANGELOG.md) if applicable. --------- Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

Followup to #760 Fix the .github/workflows/update-actions-dist-post-commit.yml workflow to also signoff commit # Testing - [x] Invoked this PR's branch copy of the workflow against #717, and it did signoff the commit. - 9670f76 Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com>

Add a new Post-Commit workflow, to make these renovate-bot updates a bit easier. Previously, we had to clone the PR locally, run `make package`, and then push to the PR. Now we would just need to use the github UI to invoke this new workflow against the PR number. We could also copy this over to the slsa-github-generator repo. > A workflow to run against renovate-bot's PRs, > such as `make package` after it updates the package.json and package-lock.json files. > The potentially untrusted code is first run inside a low-privilege Job, and the diff is uploaded as an artifact. > Then a higher-privilege Job applies the diff and pushes the changes to the PR. > It's important to only run this workflow against PRs from trusted sources, after also reviewing the changes! ## Testing. Tested in my own private fork, where when applicable, it pushed a commit of changes to `dist/` folders - https://github.com/ramonpetgrave64/slsa-verifier/actions/runs/8806815483 - https://github.com/ramonpetgrave64/slsa-verifier/pull/8/commits - https://github.com/ramonpetgrave64/slsa-verifier/actions/runs/8806841353 - https://github.com/ramonpetgrave64/slsa-verifier/pull/16/commits --------- Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com> Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com> Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

Followup to slsa-framework#760 Fix the .github/workflows/update-actions-dist-post-commit.yml workflow to also signoff commit # Testing - [x] Invoked this PR's branch copy of the workflow against slsa-framework#717, and it did signoff the commit. - slsa-framework@9670f76 Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com> Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ramonpetgrave64 added 3 commits April 23, 2024 20:32

add post-commit

15dec13

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

more docs

843b2e5

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

typo

bfdb9a2

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ramonpetgrave64 marked this pull request as ready for review April 23, 2024 20:41

ramonpetgrave64 requested review from laurentsimon and kpk47 as code owners April 23, 2024 20:41

no conditional

f5a747d

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

laurentsimon reviewed Apr 23, 2024

View reviewed changes

.github/workflows/post-commit.yml Outdated Show resolved Hide resolved

laurentsimon approved these changes Apr 23, 2024

View reviewed changes

mihaimaruseac reviewed Apr 24, 2024

View reviewed changes

.github/workflows/post-commit.yml Outdated Show resolved Hide resolved

.github/workflows/post-commit.yml Outdated Show resolved Hide resolved

ramonpetgrave64 added 3 commits April 24, 2024 16:24

set default permissions: {}

a67cc70

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

cleanup

ae3b59b

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

pin actions

023594f

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ramonpetgrave64 and others added 2 commits April 24, 2024 17:12

more security notes

129f707

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

update security guidance

5bf1cf7

Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com>

no env variables

dcd2ea7

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ianlewis reviewed Apr 30, 2024

View reviewed changes

ramonpetgrave64 added 3 commits May 2, 2024 16:07

change name to be more descriptive

acd8dca

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

comment about permissions

aa09a16

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

add contributing docs

3932f9c

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ramonpetgrave64 requested a review from ianlewis May 6, 2024 15:44

ramonpetgrave64 added this to the BYOB milestone May 6, 2024

change commit message

d627410

Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com>

ramonpetgrave64 changed the title ~~feat: post-commit~~ feat: workflow to update actions dist May 6, 2024

mihaimaruseac approved these changes May 6, 2024

View reviewed changes

.github/workflows/update-actions-dist-post-commit.yml Outdated Show resolved Hide resolved

.github/workflows/update-actions-dist-post-commit.yml Outdated Show resolved Hide resolved

ramonpetgrave64 added 2 commits May 6, 2024 15:58

pin actions/checkout

076fb74

Signed-off-by: Ramon Petgrave <32398091+ramonpetgrave64@users.noreply.github.com>

Merge branch 'main' into post-commit

ab175fc

mihaimaruseac approved these changes May 6, 2024

View reviewed changes

ramonpetgrave64 merged commit 23160d8 into slsa-framework:main May 6, 2024
14 checks passed

ramonpetgrave64 mentioned this pull request May 8, 2024

fix: signoff commit #767

Merged

1 task

This was referenced May 15, 2024

feat: add workdflow to update dists ramonpetgrave64/slsa-github-generator#7

Merged

feat: workflow to update actions dist slsa-framework/slsa-github-generator#3653

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: workflow to update actions dist #760

feat: workflow to update actions dist #760

ramonpetgrave64 commented Apr 23, 2024 •

edited

Loading

ramonpetgrave64 commented Apr 23, 2024

laurentsimon left a comment

laurentsimon Apr 23, 2024

kpk47 Apr 24, 2024

mihaimaruseac left a comment

ramonpetgrave64 commented Apr 24, 2024

ramonpetgrave64 commented Apr 24, 2024

ianlewis Apr 30, 2024

ramonpetgrave64 May 2, 2024

ianlewis Apr 30, 2024

ramonpetgrave64 May 2, 2024

ianlewis Apr 30, 2024

ramonpetgrave64 May 2, 2024

ramonpetgrave64 commented May 6, 2024

		@@ -0,0 +1,94 @@
		# A workflow to run against renovate-bot's PRs,

feat: workflow to update actions dist #760

feat: workflow to update actions dist #760

Conversation

ramonpetgrave64 commented Apr 23, 2024 • edited Loading

Testing.

ramonpetgrave64 commented Apr 23, 2024

laurentsimon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mihaimaruseac left a comment

Choose a reason for hiding this comment

ramonpetgrave64 commented Apr 24, 2024

ramonpetgrave64 commented Apr 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramonpetgrave64 commented May 6, 2024

ramonpetgrave64 commented Apr 23, 2024 •

edited

Loading