Skip to content

Commit

Permalink
Merge pull request #1416 from snyk/fix/sarif-outputfile-fixes
Browse files Browse the repository at this point in the history
Fix/sarif outputfile fixes
  • Loading branch information
RotemS committed Sep 16, 2020
2 parents 8377289 + fe38684 commit a4ce353
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 11 deletions.
12 changes: 10 additions & 2 deletions src/cli/commands/test/index.ts
Expand Up @@ -223,7 +223,11 @@ async function test(...args: MethodArgs): Promise<TestCommandResult> {
if (options.json || options.sarif) {
// if all results are ok (.ok == true) then return the json
if (errorMappedResults.every((res) => res.ok)) {
return TestCommandResult.createJsonTestCommandResult(stringifiedData);
return TestCommandResult.createJsonTestCommandResult(
stringifiedData,
stringifiedJsonData,
stringifiedSarifData,
);
}

const err = new Error(stringifiedData) as any;
Expand All @@ -233,7 +237,11 @@ async function test(...args: MethodArgs): Promise<TestCommandResult> {
const fail = shouldFail(vulnerableResults, options.failOn);
if (!fail) {
// return here to prevent failure
return TestCommandResult.createJsonTestCommandResult(stringifiedData);
return TestCommandResult.createJsonTestCommandResult(
stringifiedData,
stringifiedJsonData,
stringifiedSarifData,
);
}
}
err.code = 'VULNS';
Expand Down
24 changes: 19 additions & 5 deletions src/cli/commands/types.ts
Expand Up @@ -40,9 +40,11 @@ export abstract class TestCommandResult extends CommandResult {
}

public static createJsonTestCommandResult(
jsonResult: string,
stdout: string,
jsonResult?: string,
sarifResult?: string,
): JsonTestCommandResult {
return new JsonTestCommandResult(jsonResult);
return new JsonTestCommandResult(stdout, jsonResult, sarifResult);
}
}

Expand Down Expand Up @@ -72,11 +74,23 @@ class HumanReadableTestCommandResult extends TestCommandResult {
}

class JsonTestCommandResult extends TestCommandResult {
constructor(jsonResult: string) {
super(jsonResult);
constructor(stdout: string, jsonResult?: string, sarifResult?: string) {
super(stdout);
if (jsonResult) {
this.jsonResult = jsonResult;
}
if (sarifResult) {
this.sarifResult = sarifResult;
} else {
this.jsonResult = stdout;
}
}

public getJsonResult(): string {
return this.result;
return this.jsonResult;
}

public getSarifResult(): string {
return this.sarifResult;
}
}
5 changes: 3 additions & 2 deletions src/cli/index.ts
Expand Up @@ -125,7 +125,7 @@ async function handleError(args, error) {
}

saveResultsToFile(args.options, 'json', error.jsonStringifiedResults);
saveResultsToFile(args.options, 'sarif', error.jsonStringifiedResults);
saveResultsToFile(args.options, 'sarif', error.sarifStringifiedResults);

const analyticsError = vulnsFound
? {
Expand Down Expand Up @@ -416,7 +416,8 @@ function saveResultsToFile(
outputType: string,
jsonResults: string,
) {
const outputFile = options[`${outputType}-file-output`];
const flag = `${outputType}-file-output`;
const outputFile = options[flag];
if (outputFile && jsonResults) {
const outputFileStr = outputFile as string;
const fullOutputFilePath = getFullPath(outputFileStr);
Expand Down
71 changes: 69 additions & 2 deletions test/acceptance/cli-args.test.ts
Expand Up @@ -489,11 +489,11 @@ test('test --sarif-file-output no value produces error message', (t) => {
optionsToTest.forEach(validate);
});

test('`test --json-file-output can be used at the same time as --sarif-file-output`', (t) => {
test('`container test --json-file-output can be used at the same time as --sarif-file-output`', (t) => {
t.plan(3);

exec(
`node ${main} test --json-file-output=snyk-direct-json-test-output.json --sarif-file-output=snyk-direct-sarif-test-output.json`,
`node ${main} container test alpine --file=test/acceptance/fixtures/docker/Dockerfile --sarif-file-output=snyk-direct-sarif-test-output.json --json-file-output=snyk-direct-json-test-output.json`,
(err, stdout) => {
if (err) {
throw err;
Expand All @@ -517,3 +517,70 @@ test('`test --json-file-output can be used at the same time as --sarif-file-outp
},
);
});

test('`test --sarif-file-output can be used at the same time as --sarif`', (t) => {
t.plan(2);

exec(
`node ${main} container test alpine --sarif --file=test/acceptance/fixtures/docker/Dockerfile --sarif-file-output=snyk-direct-sarif-test-output.json`,
(err, stdout) => {
if (err) {
throw err;
}
const sarifOutput = JSON.parse(
readFileSync('snyk-direct-sarif-test-output.json', 'utf-8'),
);

unlinkSync('./snyk-direct-sarif-test-output.json');

t.match(stdout, 'rules', 'stdout is sarif');

t.match(sarifOutput.version, '2.1.0', 'SARIF output file OK');
t.end();
},
);
});

test('`test --sarif-file-output without vulns`', (t) => {
t.plan(1);

exec(
`node ${main} container test alpine --file=test/acceptance/fixtures/docker/Dockerfile --sarif-file-output=snyk-direct-sarif-test-output.json`,
(err) => {
if (err) {
throw err;
}
const sarifOutput = JSON.parse(
readFileSync('snyk-direct-sarif-test-output.json', 'utf-8'),
);

unlinkSync('./snyk-direct-sarif-test-output.json');

t.match(sarifOutput.version, '2.1.0', 'SARIF output file OK');
t.end();
},
);
});

test('`test --sarif-file-output can be used at the same time as --json with vulns`', (t) => {
t.plan(2);

exec(
`node ${main} container test ubuntu --json --file=test/acceptance/fixtures/docker/Dockerfile --sarif-file-output=snyk-direct-sarif-test-output.json`,
(err, stdout) => {
if (err) {
throw err;
}
const sarifOutput = JSON.parse(
readFileSync('snyk-direct-sarif-test-output.json', 'utf-8'),
);

unlinkSync('./snyk-direct-sarif-test-output.json');

const jsonObj = JSON.parse(stdout);
t.notEqual(jsonObj.vulnerabilities.length, 0, 'has vulns');
t.match(sarifOutput.version, '2.1.0', 'SARIF output file OK');
t.end();
},
);
});
1 change: 1 addition & 0 deletions test/acceptance/fixtures/docker/Dockerfile
@@ -0,0 +1 @@
FROM scratch

0 comments on commit a4ce353

Please sign in to comment.