Merge pull request #217 from github/rneatherway/optional-merge

Do not always overwrite the GITHUB_REF for PRs

src/actions-util.test.ts

@@ -1,13 +1,35 @@
 import test from "ava";
-
-import { getRef, prepareLocalRunEnvironment } from "./actions-util";
+import sinon from "sinon";
+import * as actionsutil from "./actions-util";
 import { setupTests } from "./testing-utils";
 
 setupTests(test);
 
-test("getRef() throws on the empty string", (t) => {
+test("getRef() throws on the empty string", async (t) => {
   process.env["GITHUB_REF"] = "";
-  t.throws(getRef);
+  await t.throwsAsync(actionsutil.getRef);
+});
+
+test("getRef() returns merge PR ref if GITHUB_SHA still checked out", async (t) => {
+  const expectedRef = "refs/pull/1/merge";
+  const currentSha = "a".repeat(40);
+  process.env["GITHUB_REF"] = expectedRef;
+  process.env["GITHUB_SHA"] = currentSha;
+
+  sinon.stub(actionsutil, "getCommitOid").resolves(currentSha);
+
+  const actualRef = await actionsutil.getRef();
+  t.deepEqual(actualRef, expectedRef);
+});
+
+test("getRef() returns head PR ref if GITHUB_SHA not currently 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 actualRef = await actionsutil.getRef();
+  t.deepEqual(actualRef, "refs/pull/1/head");
 });
 
 test("prepareEnvironment() when a local run", (t) => {
@@ -16,21 +38,21 @@ test("prepareEnvironment() when a local run", (t) => {
   process.env.CODEQL_LOCAL_RUN = "false";
   process.env.GITHUB_JOB = "YYY";
 
-  prepareLocalRunEnvironment();
+  actionsutil.prepareLocalRunEnvironment();
 
   // unchanged
   t.deepEqual(process.env.GITHUB_JOB, "YYY");
 
   process.env.CODEQL_LOCAL_RUN = "true";
 
-  prepareLocalRunEnvironment();
+  actionsutil.prepareLocalRunEnvironment();
 
   // unchanged
   t.deepEqual(process.env.GITHUB_JOB, "YYY");
 
   process.env.GITHUB_JOB = "";
 
-  prepareLocalRunEnvironment();
+  actionsutil.prepareLocalRunEnvironment();
 
   // updated
   t.deepEqual(process.env.GITHUB_JOB, "UNKNOWN-JOB");

src/actions-util.ts

@@ -57,7 +57,7 @@ export function prepareLocalRunEnvironment() {
 /**
  * Gets the SHA of the commit that is currently checked out.
  */
-export async function getCommitOid(): Promise<string> {
+export const getCommitOid = async function (): 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
@@ -85,7 +85,7 @@ export async function getCommitOid(): Promise<string> {
     );
     return getRequiredEnvParam("GITHUB_SHA");
   }
-}
+};
 
 /**
  * Get the path of the currently executing workflow.
@@ -149,17 +149,22 @@ export async function getAnalysisKey(): Promise<string> {
 /**
  * Get the ref currently being analyzed.
  */
-export function getRef(): string {
+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");
 
-  // For pull request refs we want to convert from the 'merge' ref
-  // to the 'head' ref, as that is what we want to analyse.
-  // There should have been some code earlier in the workflow to do
-  // the checkout, but we have no way of verifying that here.
+  // 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/;
-  if (pull_ref_regex.test(ref)) {
+  const checkoutSha = await getCommitOid();
+
+  if (
+    pull_ref_regex.test(ref) &&
+    checkoutSha !== getRequiredEnvParam("GITHUB_SHA")
+  ) {
     return ref.replace(pull_ref_regex, "refs/pull/$1/head");
   } else {
     return ref;
@@ -219,7 +224,7 @@ export async function createStatusReportBase(
   exception?: string
 ): Promise<StatusReportBase> {
   const commitOid = process.env["GITHUB_SHA"] || "";
-  const ref = getRef();
+  const ref = await getRef();
   const workflowRunIDStr = process.env["GITHUB_RUN_ID"];
   let workflowRunID = -1;
   if (workflowRunIDStr) {

src/analyze-action.ts

@@ -64,7 +64,7 @@ async function run() {
     stats = await runAnalyze(
       parseRepositoryNwo(actionsUtil.getRequiredEnvParam("GITHUB_REPOSITORY")),
       await actionsUtil.getCommitOid(),
-      actionsUtil.getRef(),
+      await actionsUtil.getRef(),
       await actionsUtil.getAnalysisKey(),
       actionsUtil.getRequiredEnvParam("GITHUB_WORKFLOW"),
       actionsUtil.getWorkflowRunID(),

src/upload-sarif-action.ts

@@ -45,7 +45,7 @@ async function run() {
       actionsUtil.getRequiredInput("sarif_file"),
       parseRepositoryNwo(actionsUtil.getRequiredEnvParam("GITHUB_REPOSITORY")),
       await actionsUtil.getCommitOid(),
-      actionsUtil.getRef(),
+      await actionsUtil.getRef(),
       await actionsUtil.getAnalysisKey(),
       actionsUtil.getRequiredEnvParam("GITHUB_WORKFLOW"),
       actionsUtil.getWorkflowRunID(),