Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ See the [releases page](https://github.com/github/codeql-action/releases) for th

## [UNRELEASED]

- Add support for SHA-256 Git object IDs. [#3893](https://github.com/github/codeql-action/pull/3893)
- If multiple inputs are provided for the GitHub-internal `analysis-kinds` input, only `code-scanning` will be enabled. The `analysis-kinds` input is experimental, for GitHub-internal use only, and may change without notice at any time. [#3892](https://github.com/github/codeql-action/pull/3892)
- Added an experimental change which, when running a Code Scanning analysis for a PR with [improved incremental analysis](https://github.com/github/roadmap/issues/1158) enabled, prefers CodeQL CLI versions that have a cached overlay-base database for the configured languages. This speeds up analysis for a repository when there is not yet a cached overlay-base database for the latest CLI version. We expect to roll this change out to everyone in May. [#3880](https://github.com/github/codeql-action/pull/3880)

Expand Down
2 changes: 1 addition & 1 deletion lib/analyze-action-post.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions lib/analyze-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/autobuild-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions lib/init-action-post.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/init-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/resolve-environment-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/setup-codeql-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions lib/upload-lib.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions lib/upload-sarif-action.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 2 additions & 5 deletions src/diff-informed-analysis-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ const testShouldPerformDiffInformedAnalysis = makeMacro({
[Feature.DiffInformedQueries]: testCase.featureEnabled,
});

const getGitHubVersionStub = sinon
sinon
.stub(apiClient, "getGitHubVersion")
.resolves(testCase.gitHubVersion);
const getPullRequestBranchesStub = sinon
sinon
.stub(actionsUtil, "getPullRequestBranches")
.returns(testCase.pullRequestBranches);

Expand All @@ -89,9 +89,6 @@ const testShouldPerformDiffInformedAnalysis = makeMacro({
t.is(result, expectedResult);

delete process.env.CODEQL_ACTION_DIFF_INFORMED_QUERIES;

getGitHubVersionStub.restore();
getPullRequestBranchesStub.restore();
});
},
title: (title) => `shouldPerformDiffInformedAnalysis: ${title}`,
Expand Down
91 changes: 77 additions & 14 deletions src/git-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ test.serial(

const actualRef = await gitUtils.getRef();
t.deepEqual(actualRef, expectedRef);
callback.restore();
});
},
);
Expand All @@ -54,7 +53,6 @@ test.serial(

const actualRef = await gitUtils.getRef();
t.deepEqual(actualRef, expectedRef);
callback.restore();
});
},
);
Expand All @@ -73,7 +71,6 @@ test.serial(

const actualRef = await gitUtils.getRef();
t.deepEqual(actualRef, "refs/pull/1/head");
callback.restore();
});
},
);
Expand All @@ -100,8 +97,6 @@ test.serial(

const actualRef = await gitUtils.getRef();
t.deepEqual(actualRef, "refs/pull/2/merge");
callback.restore();
getAdditionalInputStub.restore();
});
},
);
Expand Down Expand Up @@ -161,7 +156,6 @@ test.serial(
"Both 'ref' and 'sha' are required if one of them is provided.",
},
);
getAdditionalInputStub.restore();
});
},
);
Expand All @@ -188,7 +182,6 @@ test.serial(
"Both 'ref' and 'sha' are required if one of them is provided.",
},
);
getAdditionalInputStub.restore();
});
},
);
Expand Down Expand Up @@ -242,7 +235,6 @@ test.serial("isAnalyzingDefaultBranch()", async (t) => {
process.env["GITHUB_EVENT_NAME"] = "schedule";
process.env["GITHUB_REF"] = "refs/heads/main";
t.deepEqual(await gitUtils.isAnalyzingDefaultBranch(), false);
getAdditionalInputStub.restore();
});
});

Expand All @@ -254,8 +246,6 @@ test.serial("determineBaseBranchHeadCommitOid non-pullrequest", async (t) => {
const result = await gitUtils.determineBaseBranchHeadCommitOid(__dirname);
t.deepEqual(result, undefined);
t.deepEqual(0, infoStub.callCount);

infoStub.restore();
});

test.serial(
Expand All @@ -276,8 +266,6 @@ test.serial(
"git call failed. Will calculate the base branch SHA on the server. Error: " +
"The checkout path provided to the action does not appear to be a git repository.",
);

infoStub.restore();
},
);

Expand All @@ -301,10 +289,27 @@ test.serial("determineBaseBranchHeadCommitOid other error", async (t) => {
"The checkout path provided to the action does not appear to be a git repository.",
),
);

infoStub.restore();
});

test.serial(
"determineBaseBranchHeadCommitOid accepts SHA-256 OIDs",
async (t) => {
const mergeSha = "a".repeat(64);
const baseOid = "b".repeat(64);
const headOid = "c".repeat(64);

process.env["GITHUB_EVENT_NAME"] = "pull_request";
process.env["GITHUB_SHA"] = mergeSha;

sinon
.stub(gitUtils as any, "runGitCommand")
.resolves(`commit ${mergeSha}\nparent ${baseOid}\nparent ${headOid}\n`);

const result = await gitUtils.determineBaseBranchHeadCommitOid(__dirname);
t.deepEqual(result, baseOid);
},
);

test.serial("decodeGitFilePath unquoted strings", async (t) => {
t.deepEqual(gitUtils.decodeGitFilePath("foo"), "foo");
t.deepEqual(gitUtils.decodeGitFilePath("foo bar"), "foo bar");
Expand Down Expand Up @@ -436,6 +441,64 @@ test.serial("getFileOidsUnderPath handles quoted paths", async (t) => {
});
});

test.serial("getFileOidsUnderPath handles SHA-256 OIDs", async (t) => {
await withTmpDir(async (tmpDir) => {
const sha256OidA =
"9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2c0d4b7e8f9a1234567890ab";
const sha256OidB =
"aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899";

sinon
.stub(gitUtils as any, "runGitCommand")
.callsFake(async (_cwd: any, args: any) => {
if (args[0] === "rev-parse") {
return `${tmpDir}\n`;
}
return (
`100644 ${sha256OidA} 0\tlib/sha256-file-a.js\n` +
`100644 ${sha256OidB} 0\tsrc/sha256-file-b.ts`
);
});

const result = await gitUtils.getFileOidsUnderPath("/fake/path");

t.deepEqual(result, {
"lib/sha256-file-a.js": sha256OidA,
"src/sha256-file-b.ts": sha256OidB,
});
});
});

test.serial(
"getFileOidsUnderPath rejects OIDs of unsupported length",
async (t) => {
await withTmpDir(async (tmpDir) => {
// 50-char OID: not a valid SHA-1 (40) or SHA-256 (64) length. The regex
// must not accept this even though every character is a valid hex digit.
const invalidLine =
"100644 30d998ded095371488be3a729eb61d86ed721a1830d998ded0 0\tlib/bad.js";
sinon
.stub(gitUtils as any, "runGitCommand")
.callsFake(async (_cwd: any, args: any) => {
if (args[0] === "rev-parse") {
return `${tmpDir}\n`;
}
return invalidLine;
});

await t.throwsAsync(
async () => {
await gitUtils.getFileOidsUnderPath("/fake/path");
},
{
instanceOf: Error,
message: `Unexpected "git ls-files" output: ${invalidLine}`,
},
);
});
},
);

test.serial("getFileOidsUnderPath handles empty output", async (t) => {
await withTmpDir(async (tmpDir) => {
sinon
Expand Down
10 changes: 6 additions & 4 deletions src/git-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,12 @@ export const determineBaseBranchHeadCommitOid = async function (
}
}

// Let's confirm our assumptions: We had a merge commit and the parsed parent data looks correct
// Let's confirm our assumptions: We had a merge commit and the parsed parent
// data looks correct. OIDs are either 40 (SHA-1) or 64 (SHA-256) hex characters.
if (
commitOid === mergeSha &&
headOid.length === 40 &&
baseOid.length === 40
(headOid.length === 40 || headOid.length === 64) &&
(baseOid.length === 40 || baseOid.length === 64)
) {
return baseOid;
}
Expand Down Expand Up @@ -296,7 +297,8 @@ export const getFileOidsUnderPath = async function (
// 100644 4c51bc1d9e86cd86e01b0f340cb8ce095c33b283 0\tsrc/git-utils.test.ts
// 100644 6b792ea543ce75d7a8a03df591e3c85311ecb64f 0\tsrc/git-utils.ts
// The fields are: <mode> <oid> <stage>\t<path>
const regex = /^[0-9]+ ([0-9a-f]{40}) [0-9]+\t(.+)$/;
// The OID is either 40 (SHA-1) or 64 (SHA-256) hex characters.
const regex = /^[0-9]+ ([0-9a-f]{40}|[0-9a-f]{64}) [0-9]+\t(.+)$/;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I think this is a cleaner approach than what was suggested on the other PR.

for (const line of stdout.split("\n")) {
if (line) {
const match = line.match(regex);
Expand Down
Loading
Loading