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

fix: use pr_number as env variable #771

Merged
merged 2 commits into from
May 22, 2024
Merged

Conversation

ramonpetgrave64
Copy link
Contributor

@ramonpetgrave64 ramonpetgrave64 commented May 15, 2024

changing the update-dist workflow to use the pr_number input as an env variable to avoid script injection.

Our workflows are only invokable by our trusted maintainers so we should be okay. This is just an extra hardening measure.

Open issue actions/runner#1070 (comment)

Testing

I confirmed the issue by invoking the workflow with 650 && echo SCRIPT INJECTION, and it did also do the extra echo command.

after invoking the workflow again with this PR's version, the problem is mitigated.

Signed-off-by: Ramon Petgrave <[email protected]>
@ramonpetgrave64 ramonpetgrave64 marked this pull request as ready for review May 15, 2024 19:25
@ramonpetgrave64 ramonpetgrave64 enabled auto-merge (squash) May 15, 2024 19:27
@ramonpetgrave64
Copy link
Contributor Author

ianlewis pushed a commit to slsa-framework/slsa-github-generator that referenced this pull request May 16, 2024
# 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 <[email protected]>
@ramonpetgrave64 ramonpetgrave64 disabled auto-merge May 21, 2024 18:43
@ramonpetgrave64 ramonpetgrave64 merged commit b55bf59 into main May 22, 2024
17 checks passed
ramonpetgrave64 added a commit to ramonpetgrave64/slsa-verifier that referenced this pull request Jun 10, 2024
changing the update-dist workflow to use the `pr_number` input as an env
variable to avoid [script
injection](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#good-practices-for-mitigating-script-injection-attacks).

Our workflows are only invokable by our trusted maintainers so we should
be okay. This is just an extra hardening measure.

Open issue
actions/runner#1070 (comment)

## Testing

I confirmed the issue by invoking the workflow with `650 && echo SCRIPT
INJECTION`, and it did also do the extra `echo` command.
-
https://github.com/slsa-framework/slsa-verifier/actions/runs/9101350247/job/25018333703#step:3:36

after invoking the workflow again with this PR's version, the problem is
mitigated.
-
https://github.com/slsa-framework/slsa-verifier/actions/runs/9101495332/job/25018812710#step:3:8
-
https://github.com/slsa-framework/slsa-verifier/actions/runs/9101516757/job/25018888519#step:3:7

Signed-off-by: Ramon Petgrave <[email protected]>
Signed-off-by: Ramon Petgrave <[email protected]>
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