Use the checkout_path for getting the commit oid

This commit also adds a new integration check to verify this.

When running in test mode, payloads will not be uploaded. Instead, they
will be saved to disk so that they can be inspected later.
This commit is contained in:
Andrew Eisenberg 2022-02-28 11:56:11 -08:00
parent 117a67b074
commit a92e8775d8
9 changed files with 284 additions and 15 deletions

137
.github/workflows/__with-checkout-path.yml generated vendored Normal file
View file

@ -0,0 +1,137 @@
# Warning: This file is generated automatically, and should not be modified.
# Instead, please modify the template in the pr-checks directory and run:
# pip install ruamel.yaml && python3 sync.py
# to regenerate this file.
name: PR Check - Use a custom `checkout_path`
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GO111MODULE: auto
on:
push:
branches:
- main
- v1
pull_request:
types:
- opened
- synchronize
- reopened
- ready_for_review
workflow_dispatch: {}
jobs:
with-checkout-path:
strategy:
matrix:
version:
- stable-20210308
- stable-20210319
- stable-20210809
- cached
- latest
- nightly-latest
os: [ubuntu-latest, macos-latest, windows-2019]
name: Use a custom `checkout_path`
runs-on: ${{ matrix.os }}
steps:
- name: Check out repository
uses: actions/checkout@v2
- name: Prepare test
id: prepare-test
uses: ./.github/prepare-test
with:
version: ${{ matrix.version }}
- uses: actions/checkout@v2
with:
ref: 474bbf07f9247ffe1856c6a0f94aeeb10e7afee6
path: x/y/z/some-path
- uses: ./../action/init
with:
tools: ${{ steps.prepare-test.outputs.tools-url }}
# it's enough to test one compiled language and one interpreted language
languages: java,javascript
source-path: x/y/z/some-path/tests/multi-language-repo
debug: true
- name: Build code
shell: bash
run: x/y/z/some-path/tests/multi-language-repo/build.sh
- uses: ./../action/analyze
with:
checkout_path: x/y/z/some-path/tests/multi-language-repo
ref: v1.1.0
sha: 474bbf07f9247ffe1856c6a0f94aeeb10e7afee6
upload: false
env:
TEST_MODE: true
- name: Verify SARIF after analyze
shell: bash
run: |
EXPECTED_COMMIT_OID="474bbf07f9247ffe1856c6a0f94aeeb10e7afee6"
EXPECTED_REF="v1.1.0"
EXPECTED_CHECKOUT_URI_SUFFIX="/x/y/z/some-path/tests/multi-language-repo"
ACTUAL_COMMIT_OID="$(cat "$RUNNER_TEMP/payload.json" | jq -r .commit_oid)"
ACTUAL_REF="$(cat "$RUNNER_TEMP/payload.json" | jq -r .ref)"
ACTUAL_CHECKOUT_URI="$(cat "$RUNNER_TEMP/payload.json" | jq -r .checkout_uri)"
if [[ "$EXPECTED_COMMIT_OID" != "$ACTUAL_COMMIT_OID" ]]; then
echo "::error Invalid commit oid. Expected: $EXPECTED_COMMIT_OID Actual: $ACTUAL_COMMIT_OID"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
if [[ "$EXPECTED_REF" != "$ACTUAL_REF" ]]; then
echo "::error Invalid ref. Expected: '$EXPECTED_REF' Actual: '$ACTUAL_REF'"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
if [[ "$ACTUAL_CHECKOUT_URI" != *$EXPECTED_CHECKOUT_URI_SUFFIX ]]; then
echo "::error Invalid checkout URI suffix. Expected suffix: $EXPECTED_CHECKOUT_URI_SUFFIX Actual uri: $ACTUAL_CHECKOUT_URI"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
# payload.json will be recreated in the next step.
rm -f "$RUNNER_TEMP/payload.json"
- uses: ./../action/upload-sarif
with:
ref: v1.1.0
sha: 474bbf07f9247ffe1856c6a0f94aeeb10e7afee6
checkout_path: x/y/z/some-path/tests/multi-language-repo
env:
TEST_MODE: true
- name: Verify SARIF after upload
shell: bash
run: |
EXPECTED_COMMIT_OID="474bbf07f9247ffe1856c6a0f94aeeb10e7afee6"
EXPECTED_REF="v1.1.0"
EXPECTED_CHECKOUT_URI_SUFFIX="/x/y/z/some-path/tests/multi-language-repo"
ACTUAL_COMMIT_OID="$(cat "$RUNNER_TEMP/payload.json" | jq -r .commit_oid)"
ACTUAL_REF="$(cat "$RUNNER_TEMP/payload.json" | jq -r .ref)"
ACTUAL_CHECKOUT_URI="$(cat "$RUNNER_TEMP/payload.json" | jq -r .checkout_uri)"
if [[ "$EXPECTED_COMMIT_OID" != "$ACTUAL_COMMIT_OID" ]]; then
echo "::error Invalid commit oid. Expected: $EXPECTED_COMMIT_OID Actual: $ACTUAL_COMMIT_OID"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
if [[ "$EXPECTED_REF" != "$ACTUAL_REF" ]]; then
echo "::error Invalid ref. Expected: '$EXPECTED_REF' Actual: '$ACTUAL_REF'"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
if [[ "$ACTUAL_CHECKOUT_URI" != *$EXPECTED_CHECKOUT_URI_SUFFIX ]]; then
echo "::error Invalid checkout URI suffix. Expected suffix: $EXPECTED_CHECKOUT_URI_SUFFIX Actual uri: $ACTUAL_CHECKOUT_URI"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
env:
INTERNAL_CODEQL_ACTION_DEBUG_LOC: true

View file

@ -6,7 +6,8 @@ No user facing changes.
## 1.1.3 - 23 Feb 2022
- Fix bug where the CLR traces can continue tracing even after tracing should be stopped. [#938](https://github.com/github/codeql-action/pull/938)
- Fix a bug where the CLR traces can continue tracing even after tracing should be stopped. [#938](https://github.com/github/codeql-action/pull/938)
- Fix a bug where an invalid `commit_oid` was being sent to code scanning when a custom checkout path was being used. [#956](https://github.com/github/codeql-action/pull/956)
## 1.1.2 - 17 Feb 2022

13
lib/actions-util.js generated
View file

@ -73,7 +73,7 @@ exports.getToolCacheDirectory = getToolCacheDirectory;
/**
* Gets the SHA of the commit that is currently checked out.
*/
const getCommitOid = async function (ref = "HEAD") {
const getCommitOid = async function (checkoutPath, 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
@ -93,6 +93,7 @@ const getCommitOid = async function (ref = "HEAD") {
process.stderr.write(data);
},
},
cwd: checkoutPath,
}).exec();
return commitOid.trim();
}
@ -112,6 +113,7 @@ const determineMergeBaseCommitOid = async function () {
return undefined;
}
const mergeSha = (0, util_1.getRequiredEnvParam)("GITHUB_SHA");
const checkoutPath = getRequiredInput("checkout_path");
try {
let commitOid = "";
let baseOid = "";
@ -136,6 +138,7 @@ const determineMergeBaseCommitOid = async function () {
process.stderr.write(data);
},
},
cwd: checkoutPath,
}).exec();
// Let's confirm our assumptions: We had a merge commit and the parsed parent data looks correct
if (commitOid === mergeSha &&
@ -424,6 +427,9 @@ async function getRef() {
// or in the form "refs/pull/N/merge" on a pull_request event
const refInput = (0, exports.getOptionalInput)("ref");
const shaInput = (0, exports.getOptionalInput)("sha");
const checkoutPath = (0, exports.getOptionalInput)("checkout_path") ||
(0, exports.getOptionalInput)("source-root") ||
(0, util_1.getRequiredEnvParam)("GITHUB_WORKSPACE");
const hasRefInput = !!refInput;
const hasShaInput = !!shaInput;
// If one of 'ref' or 'sha' are provided, both are required
@ -445,15 +451,14 @@ async function getRef() {
if (!pull_ref_regex.test(ref)) {
return ref;
}
const head = await (0, exports.getCommitOid)("HEAD");
const head = await (0, exports.getCommitOid)(checkoutPath, "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 (0, exports.getCommitOid)(ref.replace(/^refs\/pull\//, "refs/remotes/pull/"))) !==
head;
(await (0, exports.getCommitOid)(checkoutPath, ref.replace(/^refs\/pull\//, "refs/remotes/pull/"))) !== 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}.`);

File diff suppressed because one or more lines are too long

7
lib/upload-lib.js generated
View file

@ -95,7 +95,10 @@ async function uploadPayload(payload, repositoryNwo, apiDetails, logger) {
// If in test mode we don't want to upload the results
const testMode = process.env["TEST_MODE"] === "true" || false;
if (testMode) {
logger.debug("In test mode. Results are not uploaded.");
const payloadSaveFile = path.join(actionsUtil.getTemporaryDirectory(), "payload.json");
logger.info(`In test mode. Results are not uploaded. Saving to ${payloadSaveFile}`);
logger.info(`Payload: ${JSON.stringify(payload, null, 2)}`);
fs.writeFileSync(payloadSaveFile, JSON.stringify(payload, null, 2));
return;
}
const client = api.getApiClient(apiDetails);
@ -134,7 +137,7 @@ exports.findSarifFilesInDir = findSarifFilesInDir;
// depending on what the path happens to refer to.
// Returns true iff the upload occurred and succeeded
async function uploadFromActions(sarifPath, gitHubVersion, apiDetails, logger) {
return await uploadFiles(getSarifFilePaths(sarifPath), (0, repository_1.parseRepositoryNwo)(util.getRequiredEnvParam("GITHUB_REPOSITORY")), await actionsUtil.getCommitOid(), await actionsUtil.getRef(), await actionsUtil.getAnalysisKey(), actionsUtil.getOptionalInput("category"), util.getRequiredEnvParam("GITHUB_WORKFLOW"), actionsUtil.getWorkflowRunID(), actionsUtil.getRequiredInput("checkout_path"), actionsUtil.getRequiredInput("matrix"), gitHubVersion, apiDetails, logger);
return await uploadFiles(getSarifFilePaths(sarifPath), (0, repository_1.parseRepositoryNwo)(util.getRequiredEnvParam("GITHUB_REPOSITORY")), await actionsUtil.getCommitOid(actionsUtil.getRequiredInput("checkout_path")), await actionsUtil.getRef(), await actionsUtil.getAnalysisKey(), actionsUtil.getOptionalInput("category"), util.getRequiredEnvParam("GITHUB_WORKFLOW"), actionsUtil.getWorkflowRunID(), actionsUtil.getRequiredInput("checkout_path"), actionsUtil.getRequiredInput("matrix"), gitHubVersion, apiDetails, logger);
}
exports.uploadFromActions = uploadFromActions;
// Uploads a single sarif file or a directory of sarif files

File diff suppressed because one or more lines are too long

View file

@ -0,0 +1,101 @@
name: "Use a custom `checkout_path`"
description: "Checks that a custom `checkout_path` will find the proper commit_oid"
# Build tracing currently does not support Windows 2022, so use `windows-2019` instead of
# `windows-latest`.
# Must test on all three platforms since this test does path manipulation
os: [ubuntu-latest, macos-latest, windows-2019]
steps:
# Check out the actions repo again, but at a different location.
# choose an arbitrary SHA so that we can later test that the commit_oid is not from main
- uses: actions/checkout@v2
with:
ref: 474bbf07f9247ffe1856c6a0f94aeeb10e7afee6
path: x/y/z/some-path
- uses: ./../action/init
with:
tools: ${{ steps.prepare-test.outputs.tools-url }}
# it's enough to test one compiled language and one interpreted language
languages: java,javascript
source-path: x/y/z/some-path/tests/multi-language-repo
debug: true
- name: Build code
shell: bash
run: x/y/z/some-path/tests/multi-language-repo/build.sh
- uses: ./../action/analyze
with:
checkout_path: x/y/z/some-path/tests/multi-language-repo
ref: v1.1.0
sha: 474bbf07f9247ffe1856c6a0f94aeeb10e7afee6
upload: false
env:
TEST_MODE: true
- name: Verify SARIF after analyze
shell: bash
run: |
EXPECTED_COMMIT_OID="474bbf07f9247ffe1856c6a0f94aeeb10e7afee6"
EXPECTED_REF="v1.1.0"
EXPECTED_CHECKOUT_URI_SUFFIX="/x/y/z/some-path/tests/multi-language-repo"
ACTUAL_COMMIT_OID="$(cat "$RUNNER_TEMP/payload.json" | jq -r .commit_oid)"
ACTUAL_REF="$(cat "$RUNNER_TEMP/payload.json" | jq -r .ref)"
ACTUAL_CHECKOUT_URI="$(cat "$RUNNER_TEMP/payload.json" | jq -r .checkout_uri)"
if [[ "$EXPECTED_COMMIT_OID" != "$ACTUAL_COMMIT_OID" ]]; then
echo "::error Invalid commit oid. Expected: $EXPECTED_COMMIT_OID Actual: $ACTUAL_COMMIT_OID"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
if [[ "$EXPECTED_REF" != "$ACTUAL_REF" ]]; then
echo "::error Invalid ref. Expected: '$EXPECTED_REF' Actual: '$ACTUAL_REF'"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
if [[ "$ACTUAL_CHECKOUT_URI" != *$EXPECTED_CHECKOUT_URI_SUFFIX ]]; then
echo "::error Invalid checkout URI suffix. Expected suffix: $EXPECTED_CHECKOUT_URI_SUFFIX Actual uri: $ACTUAL_CHECKOUT_URI"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
# payload.json will be recreated in the next step.
rm -f "$RUNNER_TEMP/payload.json"
- uses: ./../action/upload-sarif
with:
ref: v1.1.0
sha: 474bbf07f9247ffe1856c6a0f94aeeb10e7afee6
checkout_path: x/y/z/some-path/tests/multi-language-repo
env:
TEST_MODE: true
- name: Verify SARIF after upload
shell: bash
run: |
EXPECTED_COMMIT_OID="474bbf07f9247ffe1856c6a0f94aeeb10e7afee6"
EXPECTED_REF="v1.1.0"
EXPECTED_CHECKOUT_URI_SUFFIX="/x/y/z/some-path/tests/multi-language-repo"
ACTUAL_COMMIT_OID="$(cat "$RUNNER_TEMP/payload.json" | jq -r .commit_oid)"
ACTUAL_REF="$(cat "$RUNNER_TEMP/payload.json" | jq -r .ref)"
ACTUAL_CHECKOUT_URI="$(cat "$RUNNER_TEMP/payload.json" | jq -r .checkout_uri)"
if [[ "$EXPECTED_COMMIT_OID" != "$ACTUAL_COMMIT_OID" ]]; then
echo "::error Invalid commit oid. Expected: $EXPECTED_COMMIT_OID Actual: $ACTUAL_COMMIT_OID"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
if [[ "$EXPECTED_REF" != "$ACTUAL_REF" ]]; then
echo "::error Invalid ref. Expected: '$EXPECTED_REF' Actual: '$ACTUAL_REF'"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi
if [[ "$ACTUAL_CHECKOUT_URI" != *$EXPECTED_CHECKOUT_URI_SUFFIX ]]; then
echo "::error Invalid checkout URI suffix. Expected suffix: $EXPECTED_CHECKOUT_URI_SUFFIX Actual uri: $ACTUAL_CHECKOUT_URI"
echo "$RUNNER_TEMP/payload.json"
exit 1
fi

View file

@ -60,7 +60,10 @@ export function getToolCacheDirectory(): string {
/**
* Gets the SHA of the commit that is currently checked out.
*/
export const getCommitOid = async function (ref = "HEAD"): Promise<string> {
export const getCommitOid = async function (
checkoutPath: string,
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
@ -83,6 +86,7 @@ export const getCommitOid = async function (ref = "HEAD"): Promise<string> {
process.stderr.write(data);
},
},
cwd: checkoutPath,
}
).exec();
return commitOid.trim();
@ -107,6 +111,7 @@ export const determineMergeBaseCommitOid = async function (): Promise<
}
const mergeSha = getRequiredEnvParam("GITHUB_SHA");
const checkoutPath = getRequiredInput("checkout_path");
try {
let commitOid = "";
@ -134,6 +139,7 @@ export const determineMergeBaseCommitOid = async function (): Promise<
process.stderr.write(data);
},
},
cwd: checkoutPath,
}
).exec();
@ -498,6 +504,10 @@ export async function getRef(): Promise<string> {
// or in the form "refs/pull/N/merge" on a pull_request event
const refInput = getOptionalInput("ref");
const shaInput = getOptionalInput("sha");
const checkoutPath =
getOptionalInput("checkout_path") ||
getOptionalInput("source-root") ||
getRequiredEnvParam("GITHUB_WORKSPACE");
const hasRefInput = !!refInput;
const hasShaInput = !!shaInput;
@ -526,7 +536,7 @@ export async function getRef(): Promise<string> {
return ref;
}
const head = await getCommitOid("HEAD");
const head = await getCommitOid(checkoutPath, "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
@ -535,8 +545,10 @@ export async function getRef(): Promise<string> {
// git git-parse GITHUB_REF == git rev-parse HEAD instead.
const hasChangedRef =
sha !== head &&
(await getCommitOid(ref.replace(/^refs\/pull\//, "refs/remotes/pull/"))) !==
head;
(await getCommitOid(
checkoutPath,
ref.replace(/^refs\/pull\//, "refs/remotes/pull/")
)) !== head;
if (hasChangedRef) {
const newRef = ref.replace(pull_ref_regex, "refs/pull/$1/head");

View file

@ -100,7 +100,15 @@ async function uploadPayload(
// If in test mode we don't want to upload the results
const testMode = process.env["TEST_MODE"] === "true" || false;
if (testMode) {
logger.debug("In test mode. Results are not uploaded.");
const payloadSaveFile = path.join(
actionsUtil.getTemporaryDirectory(),
"payload.json"
);
logger.info(
`In test mode. Results are not uploaded. Saving to ${payloadSaveFile}`
);
logger.info(`Payload: ${JSON.stringify(payload, null, 2)}`);
fs.writeFileSync(payloadSaveFile, JSON.stringify(payload, null, 2));
return;
}
@ -165,7 +173,9 @@ export async function uploadFromActions(
return await uploadFiles(
getSarifFilePaths(sarifPath),
parseRepositoryNwo(util.getRequiredEnvParam("GITHUB_REPOSITORY")),
await actionsUtil.getCommitOid(),
await actionsUtil.getCommitOid(
actionsUtil.getRequiredInput("checkout_path")
),
await actionsUtil.getRef(),
await actionsUtil.getAnalysisKey(),
actionsUtil.getOptionalInput("category"),