robojerk/codeql-action

Count the number of parents of the current commit to check it is still a merge

Browse source 
Work around a race condition in actions where sometimes GITHUB_SHA != git rev-parse head
This commit is contained in:
Simon Engledew 2021-03-22 09:07:26 +00:00
parent 5d467d014b
commit ef92c5ac5f
No known key found for this signature in database
GPG key ID: 84302E7B02FE8BCE
6 changed files with 75 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

20
lib/actions-util.js generated
View file

@ -77,7 +77,7 @@ exports.prepareLocalRunEnvironment = prepareLocalRunEnvironment;
/**
* Gets the SHA of the commit that is currently checked out.
*/
exports.getCommitOid = async function () {
exports.getCommitOid = async function (ref = "HEAD") {
// 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 @@ exports.getCommitOid = async function () {
// reported on the merge commit.
try {
let commitOid = "";
await new toolrunner.ToolRunner(await safeWhich.safeWhich("git"), ["rev-parse", "HEAD"], {
await new toolrunner.ToolRunner(await safeWhich.safeWhich("git"), ["rev-parse", ref], {
silent: true,
listeners: {
stdout: (data) => {
@ -354,15 +354,23 @@ async function getRef() {
// 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 exports.getCommitOid();
if (pull_ref_regex.test(ref) &&
checkoutSha !== getRequiredEnvParam("GITHUB_SHA")) {
return ref.replace(pull_ref_regex, "refs/pull/$1/head");
const head = await exports.getCommitOid("HEAD");
// in actions@v2 we can check if git rev-parse HEAD == GITHUB_SHA
// in actions@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 exports.getCommitOid(ref)) !== head;
if (pull_ref_regex.test(ref) && hasChangedRef) {
const newRef = ref.replace(pull_ref_regex, "refs/pull/$1/head");
core.info(`No longer on merge commit, rewriting ref from ${ref} to ${newRef}.`);
return newRef;
}
else {
return ref;

2
lib/actions-util.js.map
View file

File diff suppressed because one or more lines are too long

23
lib/actions-util.test.js generated
View file

@ -28,16 +28,33 @@ ava_1.default("getRef() returns merge PR ref if GITHUB_SHA still checked out", a
const currentSha = "a".repeat(40);
process.env["GITHUB_REF"] = expectedRef;
process.env["GITHUB_SHA"] = currentSha;
sinon_1.default.stub(actionsutil, "getCommitOid").resolves(currentSha);
const callback = sinon_1.default.stub(actionsutil, "getCommitOid");
callback.withArgs("HEAD").resolves(currentSha);
const actualRef = await actionsutil.getRef();
t.deepEqual(actualRef, expectedRef);
callback.restore();
});
ava_1.default("getRef() returns head PR ref if GITHUB_SHA not currently checked out", async (t) => {
ava_1.default("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_1.default.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();
});
ava_1.default("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_1.default.stub(actionsutil, "getCommitOid").resolves("b".repeat(40));
const callback = sinon_1.default.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();
});
ava_1.default("getAnalysisKey() when a local run", async (t) => {
process.env.CODEQL_LOCAL_RUN = "true";

2
lib/actions-util.test.js.map
View file

File diff suppressed because one or more lines are too long

26
src/actions-util.test.ts
View file

@ -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) => {

24
src/actions-util.ts
View file

@ -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,27 @@ 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();
const head = await getCommitOid("HEAD");
// in actions@v2 we can check if git rev-parse HEAD == GITHUB_SHA
// in actions@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 (
pull_ref_regex.test(ref) &&
checkoutSha !== getRequiredEnvParam("GITHUB_SHA")
) {
return ref.replace(pull_ref_regex, "refs/pull/$1/head");
if (pull_ref_regex.test(ref) && hasChangedRef) {
const newRef = ref.replace(pull_ref_regex, "refs/pull/$1/head");
core.info(
`No longer on merge commit, rewriting ref from ${ref} to ${newRef}.`
);
return newRef;
} else {
return ref;
}