Merge pull request #428 from github/simon-engledew/detect-merge

Fix race condition with actions/checkout@v1

Author: Simon Engledew

lib/actions-util.js

@@ -25,20 +25,40 @@ test("getRef() returns merge PR ref if GITHUB_SHA still checked out", async (t)
   process.env["GITHUB_REF"] = expectedRef;
   process.env["GITHUB_SHA"] = currentSha;
 
-  sinon.stub(actionsutil, "getCommitOid").resolves(currentSha);
+  const callback = sinon.stub(actionsutil, "getCommitOid");
+  callback.withArgs("HEAD").resolves(currentSha);
 
   const actualRef = await actionsutil.getRef();
   t.deepEqual(actualRef, expectedRef);
+  callback.restore();
 });
 
-test("getRef() returns head PR ref if GITHUB_SHA not currently checked out", async (t) => {
+test("getRef() returns merge PR ref if GITHUB_REF still checked out but sha has changed (actions checkout@v1)", async (t) => {
+  const expectedRef = "refs/pull/1/merge";
+  process.env["GITHUB_REF"] = expectedRef;
+  process.env["GITHUB_SHA"] = "b".repeat(40);
+  const sha = "a".repeat(40);
+
+  const callback = sinon.stub(actionsutil, "getCommitOid");
+  callback.withArgs("refs/pull/1/merge").resolves(sha);
+  callback.withArgs("HEAD").resolves(sha);
+
+  const actualRef = await actionsutil.getRef();
+  t.deepEqual(actualRef, expectedRef);
+  callback.restore();
+});
+
+test("getRef() returns head PR ref if GITHUB_REF no longer checked out", async (t) => {
   process.env["GITHUB_REF"] = "refs/pull/1/merge";
   process.env["GITHUB_SHA"] = "a".repeat(40);
 
-  sinon.stub(actionsutil, "getCommitOid").resolves("b".repeat(40));
+  const callback = sinon.stub(actionsutil, "getCommitOid");
+  callback.withArgs("refs/pull/1/merge").resolves("a".repeat(40));
+  callback.withArgs("HEAD").resolves("b".repeat(40));
 
   const actualRef = await actionsutil.getRef();
   t.deepEqual(actualRef, "refs/pull/1/head");
+  callback.restore();
 });
 
 test("getAnalysisKey() when a local run", async (t) => {

lib/actions-util.js.map

@@ -75,7 +75,7 @@ export function prepareLocalRunEnvironment() {
 /**
  * Gets the SHA of the commit that is currently checked out.
  */
-export const getCommitOid = async function (): Promise<string> {
+export const getCommitOid = async function (ref = "HEAD"): Promise<string> {
   // Try to use git to get the current commit SHA. If that fails then
   // log but otherwise silently fall back to using the SHA from the environment.
   // The only time these two values will differ is during analysis of a PR when
@@ -87,7 +87,7 @@ export const getCommitOid = async function (): Promise<string> {
     let commitOid = "";
     await new toolrunner.ToolRunner(
       await safeWhich.safeWhich("git"),
-      ["rev-parse", "HEAD"],
+      ["rev-parse", ref],
       {
         silent: true,
         listeners: {
@@ -425,19 +425,32 @@ export async function getRef(): Promise<string> {
   // Will be in the form "refs/heads/master" on a push event
   // or in the form "refs/pull/N/merge" on a pull_request event
   const ref = getRequiredEnvParam("GITHUB_REF");
+  const sha = getRequiredEnvParam("GITHUB_SHA");
 
   // For pull request refs we want to detect whether the workflow
   // has run `git checkout HEAD^2` to analyze the 'head' ref rather
   // than the 'merge' ref. If so, we want to convert the ref that
   // we report back.
   const pull_ref_regex = /refs\/pull\/(\d+)\/merge/;
-  const checkoutSha = await getCommitOid();
+  if (!pull_ref_regex.test(ref)) {
+    return ref;
+  }
+
+  const head = await getCommitOid("HEAD");
 
-  if (
-    pull_ref_regex.test(ref) &&
-    checkoutSha !== getRequiredEnvParam("GITHUB_SHA")
-  ) {
-    return ref.replace(pull_ref_regex, "refs/pull/$1/head");
+  // in actions/checkout@v2 we can check if git rev-parse HEAD == GITHUB_SHA
+  // in actions/checkout@v1 this may not be true as it checks out the repository
+  // using GITHUB_REF. There is a subtle race condition where
+  // git rev-parse GITHUB_REF != GITHUB_SHA, so we must check
+  // git git-parse GITHUB_REF == git rev-parse HEAD instead.
+  const hasChangedRef = sha !== head && (await getCommitOid(ref)) !== head;
+
+  if (hasChangedRef) {
+    const newRef = ref.replace(pull_ref_regex, "refs/pull/$1/head");
+    core.debug(
+      `No longer on merge commit, rewriting ref from ${ref} to ${newRef}.`
+    );
+    return newRef;
   } else {
     return ref;
   }