Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Improve test and code type-safety #268

Merged
merged 1 commit into from May 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .c8rc.json
@@ -1,6 +1,6 @@
{
"statements": "78",
"branches": "80",
"branches": "78",
"functions": "82",
"lines": "78"
}
4 changes: 2 additions & 2 deletions lib/allowlist.ts
@@ -1,6 +1,6 @@
import type { GitHubAdvisoryId } from "audit-types";
import { isGitHubAdvisoryId } from "./common";
import { AuditCiPreprocessedConfig } from "./config";
import type { AuditCiPreprocessedConfig } from "./config";

class Allowlist {
modules: string[];
Expand All @@ -9,7 +9,7 @@ class Allowlist {
/**
* @param input the allowlisted module names, advisories, and module paths
*/
constructor(input: string[]) {
constructor(input?: string[]) {
this.modules = [];
this.advisories = [];
this.paths = [];
Expand Down
2 changes: 1 addition & 1 deletion lib/audit-ci-version.ts
@@ -1,4 +1,4 @@
import { AuditCiConfig } from "./config";
import type { AuditCiConfig } from "./config";
// Ignoring importing package.json because that changes the package's build
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { bugs, version } = require("../package.json");
Expand Down
4 changes: 2 additions & 2 deletions lib/audit.ts
@@ -1,6 +1,6 @@
import { yellow } from "./colors";
import { AuditCiConfig } from "./config";
import { Summary } from "./model";
import type { AuditCiConfig } from "./config";
import type { Summary } from "./model";
import * as npmAuditer from "./npm-auditer";
import * as pnpmAuditer from "./pnpm-auditer";
import * as yarnAuditer from "./yarn-auditer";
Expand Down
23 changes: 20 additions & 3 deletions lib/common.ts
Expand Up @@ -113,6 +113,18 @@ export function reportAudit(summary: Summary, config: AuditCiConfig) {
return summary;
}

function hasMessage(value: unknown): value is { message: unknown } {
return typeof value === "object" && value != undefined && "message" in value;
}

function hasStatusCode(
value: unknown
): value is { statusCode: unknown; message: unknown } {
return (
typeof value === "object" && value != undefined && "statusCode" in value
);
}

export function runProgram(
command: string,
arguments_: readonly string[],
Expand All @@ -130,15 +142,20 @@ export function runProgram(
// @ts-ignore
.pipe(JSONStream.parse())
.pipe(
eventStream.mapSync((data) => {
eventStream.mapSync((data: unknown) => {
if (!data) return;
try {
// due to response without error
if (data.message && data.message.includes("ENOTFOUND")) {
if (
hasMessage(data) &&
typeof data.message === "string" &&
data.message.includes("ENOTFOUND")
) {
stderrListener(data.message);
return;
}
if (data.statusCode === 404) {
// TODO: There are no tests that cover this case, not sure when this happens.
if (hasStatusCode(data) && data.statusCode === 404) {
stderrListener(data.message);
return;
}
Expand Down
12 changes: 7 additions & 5 deletions lib/model.ts
Expand Up @@ -11,8 +11,8 @@ import {
matchString,
partition,
} from "./common";
import { AuditCiConfig } from "./config";
import { VulnerabilityLevels } from "./map-vulnerability";
import type { AuditCiConfig } from "./config";
import type { VulnerabilityLevels } from "./map-vulnerability";
import type { DeepWriteable } from "./types";

const SUPPORTED_SEVERITY_LEVELS = new Set([
Expand All @@ -22,8 +22,10 @@ const SUPPORTED_SEVERITY_LEVELS = new Set([
"low",
]);

const prependPath = (newItem: string, currentPath: string) =>
`${newItem}>${currentPath}`;
const prependPath = <N extends string, C extends string>(
newItem: N,
currentPath: C
): `${N}>${C}` => `${newItem}>${currentPath}`;

export interface Summary {
advisoriesFound: GitHubAdvisoryId[];
Expand Down Expand Up @@ -57,7 +59,7 @@ class Model {
advisoriesFound: ProcessedAdvisory[];
advisoryPathsFound: string[];

constructor(config: AuditCiConfig) {
constructor(config: Pick<AuditCiConfig, "allowlist" | "levels">) {
const unsupported = Object.keys(config.levels).filter(
(level) => !SUPPORTED_SEVERITY_LEVELS.has(level)
);
Expand Down
2 changes: 1 addition & 1 deletion lib/npm-auditer.ts
@@ -1,7 +1,7 @@
import type { NPMAuditReportV1, NPMAuditReportV2 } from "audit-types";
import { blue } from "./colors";
import { reportAudit, runProgram } from "./common";
import { AuditCiConfig } from "./config";
import type { AuditCiConfig } from "./config";
import Model from "./model";

async function runNpmAudit(
Expand Down
4 changes: 2 additions & 2 deletions lib/pnpm-auditer.ts
@@ -1,8 +1,8 @@
import type { PNPMAuditReport } from "audit-types";
import { blue, yellow } from "./colors";
import { reportAudit, runProgram } from "./common";
import { AuditCiConfig } from "./config";
import Model, { Summary } from "./model";
import type { AuditCiConfig } from "./config";
import Model, { type Summary } from "./model";

async function runPnpmAudit(
config: AuditCiConfig
Expand Down
11 changes: 4 additions & 7 deletions lib/yarn-auditer.ts
@@ -1,10 +1,10 @@
import type { YarnAudit, YarnBerryAuditReport } from "audit-types";
import * as childProcess from "child_process";
import { execSync } from "child_process";
import * as semver from "semver";
import { blue, red, yellow } from "./colors";
import { reportAudit, runProgram } from "./common";
import { AuditCiConfig } from "./config";
import Model, { Summary } from "./model";
import type { AuditCiConfig } from "./config";
import Model, { type Summary } from "./model";

const MINIMUM_YARN_CLASSIC_VERSION = "1.12.3";
const MINIMUM_YARN_BERRY_VERSION = "2.4.0";
Expand All @@ -16,10 +16,7 @@ const MINIMUM_YARN_BERRY_VERSION = "2.4.0";
const MINIMUM_YARN_AUDIT_REGISTRY_VERSION = "99.99.99";

function getYarnVersion(cwd?: string) {
const version = childProcess
.execSync("yarn -v", { cwd })
.toString()
.replace("\n", "");
const version = execSync("yarn -v", { cwd }).toString().replace("\n", "");
return version;
}

Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

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

2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -63,7 +63,7 @@
"@types/yargs": "^17.0.9",
"@typescript-eslint/eslint-plugin": "^5.14.0",
"@typescript-eslint/parser": "^5.14.0",
"audit-types": "^0.5.2",
"audit-types": "^0.5.3",
"c8": "^7.11.3",
"chai": "^4.3.6",
"eslint": "^8.11.0",
Expand Down
75 changes: 75 additions & 0 deletions test/allowlist.spec.js
@@ -0,0 +1,75 @@
// @ts-check
const { expect } = require("chai");
const { default: Allowlist } = require("../dist/allowlist");

describe("Allowlist", () => {
it("can map config to empty Allowlist", () => {
const { advisories, modules, paths } = Allowlist.mapConfigToAllowlist({
allowlist: [],
});
expect(advisories).to.deep.equal([]);
expect(modules).to.deep.equal([]);
expect(paths).to.deep.equal([]);
});
it("can map config to modules Allowlist", () => {
const { advisories, modules, paths } = Allowlist.mapConfigToAllowlist({
allowlist: ["axios", "mocha"],
});
expect(advisories).to.deep.equal([]);
expect(modules).to.deep.equal(["axios", "mocha"]);
expect(paths).to.deep.equal([]);
});
it("can map config to advisories Allowlist", () => {
const { advisories, modules, paths } = Allowlist.mapConfigToAllowlist({
allowlist: [
"GHSA-pw2r-vq6v-hr8c",
"GHSA-74fj-2j2h-c42q",
"GHSA-cph5-m8f7-6c5x",
"GHSA-4w2v-q235-vp99",
"GHSA-42xw-2xvc-qx8m",
],
});
expect(advisories).to.deep.equal([
"GHSA-pw2r-vq6v-hr8c",
"GHSA-74fj-2j2h-c42q",
"GHSA-cph5-m8f7-6c5x",
"GHSA-4w2v-q235-vp99",
"GHSA-42xw-2xvc-qx8m",
]);
expect(modules).to.deep.equal([]);
expect(paths).to.deep.equal([]);
});

it("can map config to paths Allowlist", () => {
const { advisories, modules, paths } = Allowlist.mapConfigToAllowlist({
allowlist: [
"GHSA-pw2r-vq6v-hr8c|axios>follow-redirects",
"GHSA-74fj-2j2h-c42q|axios>follow-redirects",
"GHSA-cph5-m8f7-6c5x|axios",
"GHSA-4w2v-q235-vp99|axios",
"GHSA-42xw-2xvc-qx8m|axios",
],
});
expect(advisories).to.deep.equal([]);
expect(modules).to.deep.equal([]);
expect(paths).to.deep.equal([
"GHSA-pw2r-vq6v-hr8c|axios>follow-redirects",
"GHSA-74fj-2j2h-c42q|axios>follow-redirects",
"GHSA-cph5-m8f7-6c5x|axios",
"GHSA-4w2v-q235-vp99|axios",
"GHSA-42xw-2xvc-qx8m|axios",
]);
});

it("can remove duplicates", () => {
const { advisories, modules, paths } = Allowlist.mapConfigToAllowlist({
allowlist: [
"GHSA-74fj-2j2h-c42q|axios>follow-redirects",
"GHSA-74fj-2j2h-c42q|axios>follow-redirects",
],
});
expect(advisories).to.deep.equal([]);
expect(modules).to.deep.equal([]);
expect(paths).to.deep.equal(["GHSA-74fj-2j2h-c42q|axios>follow-redirects"]);
});
});
1 change: 1 addition & 0 deletions test/common.js
@@ -1,3 +1,4 @@
// @ts-check
const path = require("path");
const { default: Allowlist } = require("../dist/allowlist");
const { mapVulnerabilityLevelInput } = require("../dist/map-vulnerability");
Expand Down
1 change: 1 addition & 0 deletions test/common.spec.js
@@ -1,3 +1,4 @@
// @ts-check
const { expect } = require("chai");
const {
matchString,
Expand Down
1 change: 1 addition & 0 deletions test/npm-auditer.spec.js
@@ -1,3 +1,4 @@
// @ts-check
const { expect } = require("chai");
const { audit, report } = require("../dist/npm-auditer");
const { default: Allowlist } = require("../dist/allowlist");
Expand Down
1 change: 1 addition & 0 deletions test/npm7-auditer.spec.js
@@ -1,3 +1,4 @@
// @ts-check
const { expect } = require("chai");
const { audit, report } = require("../dist/npm-auditer");
const { default: Allowlist } = require("../dist/allowlist");
Expand Down
1 change: 1 addition & 0 deletions test/pnpm-auditer.spec.js
@@ -1,3 +1,4 @@
// @ts-check
const { expect } = require("chai");
const { report } = require("../dist/pnpm-auditer");
const { default: Allowlist } = require("../dist/allowlist");
Expand Down
1 change: 1 addition & 0 deletions test/yarn-auditer.spec.js
@@ -1,3 +1,4 @@
// @ts-check
const { expect } = require("chai");
const childProcess = require("child_process");
const semver = require("semver");
Expand Down