Listing modified files does not work when running on the pull_request_target event

[vkucera]

Describe the bug
MegaLinter seems to be getting the list of modified files by diffing the checked out repo against the target branch. This does not work well when the action runs on pull_request_target, because

This event runs in the context of the base of the pull request, rather than in the context of the merge commit, as the pull_request event does.

which results in getting no modified files to check.

If one sets the checkout action to checkout the HEAD of the PR source branch instead, using

ref: ${{ github.event.pull_request.head.sha }}

the comparison against the target branch may include also unrelated files from commits present in the target branch but missing in the (not up-to-date) source branch.
This can be avoided by diffing against the most recent ancestor of the source and target branches instead, using

git diff --name-only --merge-base "$BASE_SHA"

where:

env:
  BASE_SHA: ${{ github.event.pull_request.base.sha }}

is the commit of the target branch.

Expected behavior
Only files modified by the PR should be checked.

[nvuillam]

@vkucera thanks for reporting !

Any chance you'd be able to transpose your recommended updates here ?

megalinter/megalinter/MegaLinter.py

Line 643 in f1fb3d6

def list_files_git_diff(self):

[Kurt-von-Laven]

@vkucera, the GitHub docs you linked to advise against using the pull_request_target event for the use case of running MegaLinter: "Avoid using this event if you need to build or run code from the pull request." What is the rationale for ignoring this guidance?

[vkucera]

@vkucera, the GitHub docs you linked to advise against using the pull_request_target event for the use case of running MegaLinter: "Avoid using this event if you need to build or run code from the pull request." What is the rationale for ignoring this guidance?

Hi @Kurt-von-Laven , I'm not sure I understand your question. I linked the GitHub docs as a reference for the context of the pull_request_target event. I am not advising for or against using it. But since it is a legitimate event to run a CI workflow on, I think it is reasonable to expect MegaLinter to work on it too.

[Kurt-von-Laven]

Avoid using this event if you need to build or run code from the pull request.

I am saying that this is a quote directly from the GitHub docs on the pull_request_target. I'm not getting why someone would want to run MegaLinter on a pull request but not on the code from the pull request. Someone wanting to run MegaLinter on the code from the pull request should use a different event that is intended for that purpose.

[vkucera]

Maybe I am missing something. Running MegaLinter on pull_request_target makes it possible to let MegaLinter check the code from the pull request and use the secrets to create a pull request with the formatting fixes from a dedicated GitHub account. If there is another way to do it without running on pull_request_target, please let me know.
Therefore I don't understand the following in your replies:

• Why does running MegaLinter on pull_request_target imply building or running code from the pull request?
• Why does running MegaLinter on pull_request_target imply not running on the code from the pull request?

[Kurt-von-Laven]

Running MegaLinter on pull_request_target makes it possible to let MegaLinter check the code from the pull request and use the secrets to create a pull request with the formatting fixes from a dedicated GitHub account.

If, by dedicated GitHub account, you mean one that is not specifically associated with any identity, you should be warned that this is a known bad security practice. Imagine you are a hacker who intends to break into your own system. Which would you rather gain control of (a) an account actively used and monitored by a single individual concerned they may be held culpable for activity originating from that account or (b) an account (in many cases with a weak, shared password and no 2-fa) that is not associated with any single individual or logged into by a human on any regular basis? What you are proposing is, of course, theoretically possible, but GitHub Actions neither currently has nor plans to develop a sufficiently nuanced security model to permit it conveniently and securely. See, for example, pre-commit/action#164, which removed a convenience feature that opened GitHub pull requests containing the changes made by pre-commit hooks. At the time of that PR, the action had already been deprecated and untouched for over a year, and no modifications have been made since. Hopefully that gives a sense of the significance of the security risks posed by that feature.

If there is another way to do it without running on pull_request_target, please let me know.

In the spirit of "shifting left," meaning receiving feedback as early as possible in the development life-cycle, we run MegaLinter locally as a pre-commit hook for the purpose you described. We use ScribeMD/pre-commit-action to reuse our pre-commit configuration in CI, which includes MegaLinter and a number of other hooks. (Full disclosure: I am the author of this GitHub Action.) This also means that our mistakes are fixed before they are ever committed or pushed, which saves our reviewers time and helps us make better use of git bisect, git blame, git log, etc.

• Why does running MegaLinter on pull_request_target imply building or running code from the pull request?
• Why does running MegaLinter on pull_request_target imply not running on the code from the pull request?

Now that I understand what you are trying to do, I think I get the miscommunication. As you discovered, pull_request_target doesn't check out the code from the pull request at all. GitHub can't stop users from checking out whatever code they like, but the friction here is deliberate, as doing so will often expose oneself to security vulnerabilities from maliciously crafted pull requests. I'm sure you can appreciate why I don't wish to elaborate on the why publicly, but I'm confident there are ways to execute untrusted code when MegaLinter is run on untrusted code. This is neither a peculiarity of MegaLinter nor feasible to change, which is why GitHub Actions, as a blanket policy in response to frequent exploits, changed the behavior of pull_request_target so that it no longer checks out the code from the pull request at all.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

[vkucera]

Thanks @Kurt-von-Laven for the detailed and very instructive reply.
If I understand well, the MegaLinter action is supposed to run only on pull_request and push events, right?
Let me then ask a different question: How can one create a pull request with applied fixes in those events?
Please note that the steps in the workflow template did not work last time I tried (see #481).

[Kurt-von-Laven]

If I understand well, the MegaLinter action is supposed to run only on pull_request and push events, right?

Those are certainly the most common use cases, yes. There are many other events, so I won't claim there is no value in running MegaLinter on any of the others, but pull_request_target is generally not appropriate for any action intended to run on the head of a pull request.

How can one create a pull request with applied fixes in those events?

This can be achieved most easily by opening pull requests directly from the repository itself rather than from a fork. If your repository is public and you desire this feature when opening pull requests from forks, GitHub Actions is intentionally designed to create friction for you here, because this is an inherently an insecure objective. In that case, I recommend you give pre-commit a try instead and abandon this goal. Otherwise, your goal can only be achieved by exposing yourself to abuse, and again, none of this is specific to MegaLinter. If you remain deeply committed to this feature, you would need to investigate other CI services that may have different security models. You can enable workflows from forks of private repositories, but it's likely easier and safer to open pull requests directly from the repository itself in that scenario.

See #2392 for additional context.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity.
It will be closed in 14 days if no further activity occurs.
Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.