RFR: 1072: Skara should validate the commit hash of a Backport PR

Erik Joelsson erikj at openjdk.java.net
Tue Jun 8 18:52:40 UTC 2021


On Mon, 7 Jun 2021 23:18:13 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Add a check for "Backport <hash>" titles that the given hash isn't the head of the PR itself. If the user mistakenly uses that hash, Skara is very likely to get fooled into thinking the backport is clean.
>> 
>> While investigating this, I also discovered that the CheckWorkItem was inconsistent with not repeating "backport" errors, so I implemented a general mechanism for this, which guarantees that the same exact error message is never repeated. I believe this will be good enough, and certainly better than the existing behavior.
>
> bots/pr/src/main/java/org/openjdk/skara/bots/pr/CheckWorkItem.java line 232:
> 
>> 230:             if (m.matches()) {
>> 231:                 var hash = new Hash(m.group(1));
>> 232:                 if (pr.headHash().equals(hash)) {
> 
> Isn't it the hash of the first commit in the PR, rather than the HEAD commit of the PR, that is likely to cause the problem? In case there is more than one commit in the PR, might it be better to check whether it is an ancestor of the HEAD commit?

Right, if there are multiple changes in the PR branch, any one of them could potentially be the original change. Checking that is a bit more complicated though as I think it requires a local clone to inspect. I will need to investigate how best to solve it.

-------------

PR: https://git.openjdk.java.net/skara/pull/1184


More information about the skara-dev mailing list