robojerk/codeql-action

Add a bunch of tests cases and harden the function aganst malformed workflows

Browse source
This commit is contained in:
Simon Engledew 2020-12-03 13:41:55 +00:00
parent 107fe8422f
commit 7100f22932
No known key found for this signature in database
GPG key ID: 84302E7B02FE8BCE
6 changed files with 179 additions and 36 deletions
Download patch file Download diff file Expand all files Collapse all files

33
lib/actions-util.js generated
View file

@ -160,18 +160,20 @@ exports.WorkflowErrors = toCodedErrors({
LintFailed: `Unable to lint workflow for CodeQL.`,
});
function validateWorkflow(doc) {
var _a, _b, _c, _d, _e, _f, _g;
var _a, _b, _c, _d, _e, _f, _g, _h;
const errors = [];
// .jobs[key].steps[].run
for (const job of Object.values(((_a = doc) === null || _a === void 0 ? void 0 : _a.jobs) || {})) {
for (const step of ((_b = job) === null || _b === void 0 ? void 0 : _b.steps) || []) {
// this was advice that we used to give in the README
// we actually want to run the analysis on the merge commit
// to produce results that are more inline with expectations
// (i.e: this is what will happen if you merge this PR)
// and avoid some race conditions
if (((_c = step) === null || _c === void 0 ? void 0 : _c.run) === "git checkout HEAD^2") {
errors.push(exports.WorkflowErrors.CheckoutWrongHead);
if (Array.isArray((_b = job) === null || _b === void 0 ? void 0 : _b.steps)) {
for (const step of (_c = job) === null || _c === void 0 ? void 0 : _c.steps) {
// this was advice that we used to give in the README
// we actually want to run the analysis on the merge commit
// to produce results that are more inline with expectations
// (i.e: this is what will happen if you merge this PR)
// and avoid some race conditions
if (((_d = step) === null || _d === void 0 ? void 0 : _d.run) === "git checkout HEAD^2") {
errors.push(exports.WorkflowErrors.CheckoutWrongHead);
}
}
}
}
@ -208,21 +210,21 @@ function validateWorkflow(doc) {
missing = missing | MissingTriggers.Push;
}
else {
const paths = (_d = doc.on.push) === null || _d === void 0 ? void 0 : _d.paths;
const paths = (_e = doc.on.push) === null || _e === void 0 ? void 0 : _e.paths;
// if you specify paths or paths-ignore you can end up with commits that have no baseline
// if they didn't change any files
// currently we cannot go back through the history and find the most recent baseline
if (Array.isArray(paths) && paths.length > 0) {
errors.push(exports.WorkflowErrors.PathsSpecified);
}
const pathsIgnore = (_e = doc.on.push) === null || _e === void 0 ? void 0 : _e["paths-ignore"];
const pathsIgnore = (_f = doc.on.push) === null || _f === void 0 ? void 0 : _f["paths-ignore"];
if (Array.isArray(pathsIgnore) && pathsIgnore.length > 0) {
errors.push(exports.WorkflowErrors.PathsIgnoreSpecified);
}
}
const push = branchesToArray((_f = doc.on.push) === null || _f === void 0 ? void 0 : _f.branches);
const push = branchesToArray((_g = doc.on.push) === null || _g === void 0 ? void 0 : _g.branches);
if (push !== "**") {
const pull_request = branchesToArray((_g = doc.on.pull_request) === null || _g === void 0 ? void 0 : _g.branches);
const pull_request = branchesToArray((_h = doc.on.pull_request) === null || _h === void 0 ? void 0 : _h.branches);
if (pull_request !== "**") {
const difference = pull_request.filter((value) => !push.some((o) => patternIsSuperset(o, value)));
if (difference.length > 0) {
@ -238,6 +240,11 @@ function validateWorkflow(doc) {
}
}
}
else {
// on is not a known type
// this workflow is likely malformed
missing = MissingTriggers.Push | MissingTriggers.PullRequest;
}
switch (missing) {
case MissingTriggers.PullRequest | MissingTriggers.Push:
errors.push(exports.WorkflowErrors.MissingHooks);

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

File diff suppressed because one or more lines are too long

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

@ -89,13 +89,13 @@ ava_1.default("validateWorkflow() when on.push is valid", (t) => {
const errors = actionsutil.validateWorkflow({
on: ["push", "pull_request"],
});
t.deepEqual(errors.length, 0);
t.deepEqual(errors, []);
});
ava_1.default("validateWorkflow() when on.push is a valid superset", (t) => {
const errors = actionsutil.validateWorkflow({
on: ["push", "pull_request", "schedule"],
});
t.deepEqual(errors.length, 0);
t.deepEqual(errors, []);
});
ava_1.default("validateWorkflow() when on.push should not have a path", (t) => {
const errors = actionsutil.validateWorkflow({
@ -110,7 +110,7 @@ ava_1.default("validateWorkflow() when on.push is a correct object", (t) => {
const errors = actionsutil.validateWorkflow({
on: { push: { branches: ["main"] }, pull_request: { branches: ["main"] } },
});
t.deepEqual(errors.length, 0);
t.deepEqual(errors, []);
});
ava_1.default("validateWorkflow() when on.pull_requests is a string", (t) => {
const errors = actionsutil.validateWorkflow({
@ -128,8 +128,7 @@ ava_1.default("validateWorkflow() when on.push is correct with empty objects", (
const errors = actionsutil.validateWorkflow({
on: { push: undefined, pull_request: undefined },
});
console.log(errors);
t.deepEqual(errors.length, 0);
t.deepEqual(errors, []);
});
ava_1.default("validateWorkflow() when on.push is mismatched", (t) => {
const errors = actionsutil.validateWorkflow({
@ -147,7 +146,7 @@ ava_1.default("validateWorkflow() when on.push is not mismatched", (t) => {
pull_request: { branches: ["main"] },
},
});
t.deepEqual(errors.length, 0);
t.deepEqual(errors, []);
});
ava_1.default("validateWorkflow() when on.push is mismatched for pull_request", (t) => {
const errors = actionsutil.validateWorkflow({
@ -158,6 +157,52 @@ ava_1.default("validateWorkflow() when on.push is mismatched for pull_request",
});
t.deepEqual(errors, [actionsutil.WorkflowErrors.MismatchedBranches]);
});
ava_1.default("validateWorkflow() for a range of malformed workflows", (t) => {
t.deepEqual(actionsutil.validateWorkflow({
on: {
push: 1,
pull_request: 1,
},
}), []);
t.deepEqual(actionsutil.validateWorkflow({
on: 1,
}), [actionsutil.WorkflowErrors.MissingHooks]);
t.deepEqual(actionsutil.validateWorkflow({
on: 1,
jobs: 1,
}), [actionsutil.WorkflowErrors.MissingHooks]);
t.deepEqual(actionsutil.validateWorkflow({
on: 1,
jobs: [1],
}), [actionsutil.WorkflowErrors.MissingHooks]);
t.deepEqual(actionsutil.validateWorkflow({
on: 1,
jobs: { 1: 1 },
}), [actionsutil.WorkflowErrors.MissingHooks]);
t.deepEqual(actionsutil.validateWorkflow({
on: 1,
jobs: { test: 1 },
}), [actionsutil.WorkflowErrors.MissingHooks]);
t.deepEqual(actionsutil.validateWorkflow({
on: 1,
jobs: { test: [1] },
}), [actionsutil.WorkflowErrors.MissingHooks]);
t.deepEqual(actionsutil.validateWorkflow({
on: 1,
jobs: { test: { steps: 1 } },
}), [actionsutil.WorkflowErrors.MissingHooks]);
t.deepEqual(actionsutil.validateWorkflow({
on: 1,
jobs: { test: { steps: [{ notrun: "git checkout HEAD^2" }] } },
}), [actionsutil.WorkflowErrors.MissingHooks]);
t.deepEqual(actionsutil.validateWorkflow({
on: 1,
jobs: { test: [undefined] },
}), [actionsutil.WorkflowErrors.MissingHooks]);
t.deepEqual(actionsutil.validateWorkflow(1), [
actionsutil.WorkflowErrors.MissingHooks,
]);
});
ava_1.default("validateWorkflow() when on.pull_request for every branch but push specifies branches", (t) => {
const errors = actionsutil.validateWorkflow({
on: {

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

File diff suppressed because one or more lines are too long

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

@ -113,7 +113,7 @@ test("validateWorkflow() when on.push is valid", (t) => {
on: ["push", "pull_request"],
});
t.deepEqual(errors.length, 0);
t.deepEqual(errors, []);
});
test("validateWorkflow() when on.push is a valid superset", (t) => {
@ -121,7 +121,7 @@ test("validateWorkflow() when on.push is a valid superset", (t) => {
on: ["push", "pull_request", "schedule"],
});
t.deepEqual(errors.length, 0);
t.deepEqual(errors, []);
});
test("validateWorkflow() when on.push should not have a path", (t) => {
@ -140,7 +140,7 @@ test("validateWorkflow() when on.push is a correct object", (t) => {
on: { push: { branches: ["main"] }, pull_request: { branches: ["main"] } },
});
t.deepEqual(errors.length, 0);
t.deepEqual(errors, []);
});
test("validateWorkflow() when on.pull_requests is a string", (t) => {
@ -164,9 +164,7 @@ test("validateWorkflow() when on.push is correct with empty objects", (t) => {
on: { push: undefined, pull_request: undefined },
});
console.log(errors);
t.deepEqual(errors.length, 0);
t.deepEqual(errors, []);
});
test("validateWorkflow() when on.push is mismatched", (t) => {
@ -188,7 +186,7 @@ test("validateWorkflow() when on.push is not mismatched", (t) => {
},
});
t.deepEqual(errors.length, 0);
t.deepEqual(errors, []);
});
test("validateWorkflow() when on.push is mismatched for pull_request", (t) => {
@ -202,6 +200,93 @@ test("validateWorkflow() when on.push is mismatched for pull_request", (t) => {
t.deepEqual(errors, [actionsutil.WorkflowErrors.MismatchedBranches]);
});
test("validateWorkflow() for a range of malformed workflows", (t) => {
t.deepEqual(
actionsutil.validateWorkflow({
on: {
push: 1,
pull_request: 1,
},
} as any),
[]
);
t.deepEqual(
actionsutil.validateWorkflow({
on: 1,
} as any),
[actionsutil.WorkflowErrors.MissingHooks]
);
t.deepEqual(
actionsutil.validateWorkflow({
on: 1,
jobs: 1,
} as any),
[actionsutil.WorkflowErrors.MissingHooks]
);
t.deepEqual(
actionsutil.validateWorkflow({
on: 1,
jobs: [1],
} as any),
[actionsutil.WorkflowErrors.MissingHooks]
);
t.deepEqual(
actionsutil.validateWorkflow({
on: 1,
jobs: { 1: 1 },
} as any),
[actionsutil.WorkflowErrors.MissingHooks]
);
t.deepEqual(
actionsutil.validateWorkflow({
on: 1,
jobs: { test: 1 },
} as any),
[actionsutil.WorkflowErrors.MissingHooks]
);
t.deepEqual(
actionsutil.validateWorkflow({
on: 1,
jobs: { test: [1] },
} as any),
[actionsutil.WorkflowErrors.MissingHooks]
);
t.deepEqual(
actionsutil.validateWorkflow({
on: 1,
jobs: { test: { steps: 1 } },
} as any),
[actionsutil.WorkflowErrors.MissingHooks]
);
t.deepEqual(
actionsutil.validateWorkflow({
on: 1,
jobs: { test: { steps: [{ notrun: "git checkout HEAD^2" }] } },
} as any),
[actionsutil.WorkflowErrors.MissingHooks]
);
t.deepEqual(
actionsutil.validateWorkflow({
on: 1,
jobs: { test: [undefined] },
} as any),
[actionsutil.WorkflowErrors.MissingHooks]
);
t.deepEqual(actionsutil.validateWorkflow(1 as any), [
actionsutil.WorkflowErrors.MissingHooks,
]);
});
test("validateWorkflow() when on.pull_request for every branch but push specifies branches", (t) => {
const errors = actionsutil.validateWorkflow({
on: {

22
src/actions-util.ts
View file

@ -209,14 +209,16 @@ export function validateWorkflow(doc: Workflow): CodedError[] {
// .jobs[key].steps[].run
for (const job of Object.values(doc?.jobs || {})) {
for (const step of job?.steps || []) {
// this was advice that we used to give in the README
// we actually want to run the analysis on the merge commit
// to produce results that are more inline with expectations
// (i.e: this is what will happen if you merge this PR)
// and avoid some race conditions
if (step?.run === "git checkout HEAD^2") {
errors.push(WorkflowErrors.CheckoutWrongHead);
if (Array.isArray(job?.steps)) {
for (const step of job?.steps) {
// this was advice that we used to give in the README
// we actually want to run the analysis on the merge commit
// to produce results that are more inline with expectations
// (i.e: this is what will happen if you merge this PR)
// and avoid some race conditions
if (step?.run === "git checkout HEAD^2") {
errors.push(WorkflowErrors.CheckoutWrongHead);
}
}
}
}
@ -284,6 +286,10 @@ export function validateWorkflow(doc: Workflow): CodedError[] {
errors.push(WorkflowErrors.MismatchedBranches);
}
}
} else {
// on is not a known type
// this workflow is likely malformed
missing = MissingTriggers.Push | MissingTriggers.PullRequest;
}
switch (missing) {