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

--from-pr has the potential for malicious use by nefarious actors #4259

Open
terjekv opened this issue May 16, 2023 · 0 comments
Open

--from-pr has the potential for malicious use by nefarious actors #4259

terjekv opened this issue May 16, 2023 · 0 comments
Milestone

Comments

@terjekv
Copy link
Contributor

terjekv commented May 16, 2023

When using --from-pr, any updates to the PR will update the code being pulled down and run. This may not be what one wants without having verified the updates that may have taken place in the PR.

One egregious example would be someone making a PR that adds an easyconfig for a specific piece of software, which people start using with --from-pr rather than wait for merging. The PR may even contain minor style or other flaws that prevent it from being merged. Once the contents of the PR are in use, the author then modifies the PR to make the easyconfig still install the software, but also install a back door to the system in question.

The solution (suggested by @ocaisa) is probably to allow using --from with sha hashes, in one of two ways, either:

  1. --from-pr=42#abcdef..., where we specify both PR and sha, or,
  2. --from-sha=abcdef... where we specify the sha directly.

We should be able to validate that the sha belongs to the pr via https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#list-pull-requests-associated-with-a-commit, but do we really care? The main advantage of doing PR + sha is that we verify that the sha and the PR are related, but do we really care? If we do --from-sha=abcdef… we are specifying a distinct sha and, well, that is by the very idea of using the sha hash what want to apply. Does this have to come from a PR at all? Good for review, but maybe it's not a hard requirement?

@boegel boegel added this to the 4.x milestone May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants