Skip to content

Commit 2f70a98

Browse files
committed
Skip validating SARIF produced by CodeQL
1 parent f681ad6 commit 2f70a98

File tree

9 files changed

+97
-63
lines changed

9 files changed

+97
-63
lines changed

lib/analyze.js

Lines changed: 2 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/analyze.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/upload-lib.js

Lines changed: 28 additions & 19 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/upload-lib.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/upload-lib.test.js

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/upload-lib.test.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/analyze.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import { getRepositoryNwoFromEnv } from "./repository";
2626
import { DatabaseCreationTimings, EventReport } from "./status-report";
2727
import { ToolsFeature } from "./tools-features";
2828
import { endTracingForCluster } from "./tracer-config";
29-
import { validateSarifFileSchema } from "./upload-lib";
3029
import * as util from "./util";
3130
import { BuildMode } from "./util";
3231

@@ -630,7 +629,7 @@ export async function runQueries(
630629
logger.info(analysisSummary);
631630

632631
if (await features.getValue(Feature.QaTelemetryEnabled)) {
633-
const perQueryAlertCounts = getPerQueryAlertCounts(sarifFile, logger);
632+
const perQueryAlertCounts = getPerQueryAlertCounts(sarifFile);
634633

635634
const perQueryAlertCountEventReport: EventReport = {
636635
event: "codeql database interpret-results",
@@ -682,11 +681,7 @@ export async function runQueries(
682681
}
683682

684683
/** Get an object with all queries and their counts parsed from a SARIF file path. */
685-
function getPerQueryAlertCounts(
686-
sarifPath: string,
687-
log: Logger,
688-
): Record<string, number> {
689-
validateSarifFileSchema(sarifPath, log);
684+
function getPerQueryAlertCounts(sarifPath: string): Record<string, number> {
690685
const sarifObject = JSON.parse(
691686
fs.readFileSync(sarifPath, "utf8"),
692687
) as util.SarifFile;

src/upload-lib.test.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,22 @@ test.beforeEach(() => {
1717
test("validateSarifFileSchema - valid", (t) => {
1818
const inputFile = `${__dirname}/../src/testdata/valid-sarif.sarif`;
1919
t.notThrows(() =>
20-
uploadLib.validateSarifFileSchema(inputFile, getRunnerLogger(true)),
20+
uploadLib.validateSarifFileSchema(
21+
uploadLib.readSarifFile(inputFile),
22+
inputFile,
23+
getRunnerLogger(true),
24+
),
2125
);
2226
});
2327

2428
test("validateSarifFileSchema - invalid", (t) => {
2529
const inputFile = `${__dirname}/../src/testdata/invalid-sarif.sarif`;
2630
t.throws(() =>
27-
uploadLib.validateSarifFileSchema(inputFile, getRunnerLogger(true)),
31+
uploadLib.validateSarifFileSchema(
32+
uploadLib.readSarifFile(inputFile),
33+
inputFile,
34+
getRunnerLogger(true),
35+
),
2836
);
2937
});
3038

@@ -314,7 +322,11 @@ test("accept results with invalid artifactLocation.uri value", (t) => {
314322
} as Logger;
315323

316324
const sarifFile = `${__dirname}/../src/testdata/with-invalid-uri.sarif`;
317-
uploadLib.validateSarifFileSchema(sarifFile, mockLogger);
325+
uploadLib.validateSarifFileSchema(
326+
uploadLib.readSarifFile(sarifFile),
327+
sarifFile,
328+
mockLogger,
329+
);
318330

319331
t.deepEqual(loggedMessages.length, 3);
320332
t.deepEqual(

src/upload-lib.ts

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -434,18 +434,35 @@ function countResultsInSarif(sarif: string): number {
434434
return numResults;
435435
}
436436

437-
// Validates that the given file path refers to a valid SARIF file.
438-
// Throws an error if the file is invalid.
439-
export function validateSarifFileSchema(sarifFilePath: string, logger: Logger) {
440-
logger.info(`Validating ${sarifFilePath}`);
441-
let sarif;
437+
export function readSarifFile(sarifFilePath: string): SarifFile {
442438
try {
443-
sarif = JSON.parse(fs.readFileSync(sarifFilePath, "utf8")) as SarifFile;
439+
return JSON.parse(fs.readFileSync(sarifFilePath, "utf8")) as SarifFile;
444440
} catch (e) {
445441
throw new InvalidSarifUploadError(
446442
`Invalid SARIF. JSON syntax error: ${getErrorMessage(e)}`,
447443
);
448444
}
445+
}
446+
447+
// Validates that the given file path refers to a valid SARIF file.
448+
// Throws an error if the file is invalid.
449+
export function validateSarifFileSchema(
450+
sarif: SarifFile,
451+
sarifFilePath: string,
452+
logger: Logger,
453+
) {
454+
if (
455+
areAllRunsProducedByCodeQL([sarif]) &&
456+
// We want to validate CodeQL SARIF in testing environments.
457+
!util.getTestingEnvironment()
458+
) {
459+
logger.debug(
460+
`Skipping SARIF schema validation for ${sarifFilePath} as all runs are produced by CodeQL.`,
461+
);
462+
return;
463+
}
464+
465+
logger.info(`Validating ${sarifFilePath}`);
449466
// eslint-disable-next-line @typescript-eslint/no-require-imports
450467
const schema = require("../src/sarif-schema-2.1.0.json") as jsonschema.Schema;
451468

@@ -551,41 +568,44 @@ export function buildPayload(
551568
}
552569

553570
/**
554-
* Uploads a single SARIF file or a directory of SARIF files depending on what `sarifPath` refers
571+
* Uploads a single SARIF file or a directory of SARIF files depending on what `inputSarifPath` refers
555572
* to.
556573
*/
557574
export async function uploadFiles(
558-
sarifPath: string,
575+
inputSarifPath: string,
559576
checkoutPath: string,
560577
category: string | undefined,
561578
features: FeatureEnablement,
562579
logger: Logger,
563580
): Promise<UploadResult> {
564-
const sarifFiles = getSarifFilePaths(sarifPath);
581+
const sarifPaths = getSarifFilePaths(inputSarifPath);
565582

566583
logger.startGroup("Uploading results");
567-
logger.info(`Processing sarif files: ${JSON.stringify(sarifFiles)}`);
584+
logger.info(`Processing sarif files: ${JSON.stringify(sarifPaths)}`);
568585

569586
const gitHubVersion = await getGitHubVersion();
570587

571-
try {
588+
let sarif: SarifFile;
589+
590+
if (sarifPaths.length > 1) {
572591
// Validate that the files we were asked to upload are all valid SARIF files
573-
for (const file of sarifFiles) {
574-
validateSarifFileSchema(file, logger);
575-
}
576-
} catch (e) {
577-
if (e instanceof SyntaxError) {
578-
throw new InvalidSarifUploadError(e.message);
592+
for (const sarifPath of sarifPaths) {
593+
const parsedSarif = readSarifFile(sarifPath);
594+
validateSarifFileSchema(parsedSarif, sarifPath, logger);
579595
}
580-
throw e;
596+
597+
sarif = await combineSarifFilesUsingCLI(
598+
sarifPaths,
599+
gitHubVersion,
600+
features,
601+
logger,
602+
);
603+
} else {
604+
const sarifPath = sarifPaths[0];
605+
sarif = readSarifFile(sarifPath);
606+
validateSarifFileSchema(sarif, sarifPath, logger);
581607
}
582608

583-
let sarif = await combineSarifFilesUsingCLI(
584-
sarifFiles,
585-
gitHubVersion,
586-
features,
587-
logger,
588-
);
589609
sarif = filterAlertsByDiffRange(logger, sarif);
590610
sarif = await fingerprints.addFingerprints(sarif, checkoutPath, logger);
591611

0 commit comments

Comments
 (0)