[
https://issues.apache.org/jira/browse/IGNITE-23820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17904201#comment-17904201
]
Arnout Engelen commented on IGNITE-23820:
-----------------------------------------
{quote}As I see, the update to PR from the non-maintener user just asks for
another workflow approval
{quote}
That is indeed the case! However, it does not cancel the workflow runs for the
'previous' commits. This should not be a problem, except when you accidentally
use 'checkout' in a way that checks out the 'updated' PR instead of the PR 'as
approved'.
{quote}Are you 100% sure? As I remember my tests show the opposite. Such
workflows not just not run if absent but *execute* only code from the master
branch.
{quote}
It indeed executes the 'workflow code' from the master branch. However, when
that workflow does a checkout of the PR, it can be quite tricky to make 100%
sure that a malicious attacker cannot inject things into the working directory
to hijack that code. In this particular example, I would be concerned that
there might be something in "./mvnw
org.sonarsource.scanner.maven:sonar-maven-plugin:sonar" that could be
influenced by the attacker. Right now that is trivially the case (they could
just replace the 'mvnw' script with something malicious outright), but even
when we lock down that particular script there might be weaknesses in that
sonar plugin itself. It's much safer to make sure you checkout the reviewed
code.
{quote}As I understand the *workflow_run* is exactly designed to support cases
like this. Build in not priviledged workflow and anylyze and post comments to
PR in the priviledged one.
{quote}
Yes, I agree! I completely support your approach of running the unprivileged
build in the 'pull_request'-triggered action and the privileged operations on
the 'workflow_run'-triggered side. Still, when using 'checkout' to switch to
the contributors' codebase, there's some risks to be careful about, which is
what my PR intended to protect against.
> run privileged workflow against approved commit
> -----------------------------------------------
>
> Key: IGNITE-23820
> URL: https://issues.apache.org/jira/browse/IGNITE-23820
> Project: Ignite
> Issue Type: Improvement
> Components: build
> Reporter: Arnout Engelen
> Assignee: Pavel Tupitsyn
> Priority: Minor
> Fix For: 2.17
>
> Attachments: image-2024-12-09-18-53-12-359.png
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> `sonar-pr-from-fork-build.yml` and `sonar-pr-from-fork-scan.yml` analyze PRs.
> `sonar-pr-from-fork-scan.yml` needs privileges to access the
> `SONARCLOUD_TOKEN` and to update the status of the PR check.
> To avoid a malicious PR from accessing those privileges, Ignite requires
> approval for GitHub Actions, and reviews the PR to catch any malicious code
> before approving the workflow.
> Some changes to the workflow are needed to make sure the privileged workflow
> is ran against the commit that was approved, and does not pull in any changes
> that may have been added to the PR after approval.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
