robojerk/codeql-action

Merge pull request #2487 from rvermeulen/rvermeulen/uri-errors-as-warnings

Browse source 
Turn URI errors into warnings
This commit is contained in:
Remco Vermeulen 2024-09-17 16:28:01 -07:00 • committed by GitHub
commit cb28816228
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 59 additions and 32 deletions
Download patch file Download diff file Expand all files Collapse all files

9
lib/upload-lib.js generated
View file

@ -326,8 +326,13 @@ function validateSarifFileSchema(sarifFilePath, logger) {
const result = new jsonschema.Validator().validate(sarif, schema);
// Filter errors related to invalid URIs in the artifactLocation field as this
// is a breaking change. See https://github.com/github/codeql-action/issues/1703
const errors = (result.errors || []).filter((err) => err.argument !== "uri-reference");
const warnings = (result.errors || []).filter((err) => err.argument === "uri-reference");
const warningAttributes = ["uri-reference", "uri"];
const errors = (result.errors ?? []).filter((err) => !(err.name === "format" &&
typeof err.argument === "string" &&
warningAttributes.includes(err.argument)));
const warnings = (result.errors ?? []).filter((err) => err.name === "format" &&
typeof err.argument === "string" &&
warningAttributes.includes(err.argument));
for (const warning of warnings) {
logger.info(`Warning: '${warning.instance}' is not a valid URI in '${warning.property}'.`);
}

2
lib/upload-lib.js.map
View file

File diff suppressed because one or more lines are too long

4
lib/upload-lib.test.js generated
View file

@ -192,8 +192,8 @@ ava_1.default.beforeEach(() => {
};
const sarifFile = `${__dirname}/../src/testdata/with-invalid-uri.sarif`;
uploadLib.validateSarifFileSchema(sarifFile, mockLogger);
t.deepEqual(loggedMessages.length, 2);
t.deepEqual(loggedMessages[1], "Warning: 'not a valid URI' is not a valid URI in 'instance.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri'.");
t.deepEqual(loggedMessages.length, 3);
t.deepEqual(loggedMessages[1], "Warning: 'not a valid URI' is not a valid URI in 'instance.runs[0].tool.driver.rules[0].helpUri'.", "Warning: 'not a valid URI' is not a valid URI in 'instance.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri'.");
});
(0, ava_1.default)("shouldShowCombineSarifFilesDeprecationWarning when on dotcom", async (t) => {
t.true(await uploadLib.shouldShowCombineSarifFilesDeprecationWarning([createMockSarif("abc", "def"), createMockSarif("abc", "def")], {

2
lib/upload-lib.test.js.map
View file

File diff suppressed because one or more lines are too long

54
src/testdata/with-invalid-uri.sarif vendored
View file

@ -8,30 +8,42 @@
"name": "LGTM.com",
"organization": "Semmle",
"version": "1.24.0-SNAPSHOT",
"rules": []
"rules": [
{
"id": "js/unused-local-variable",
"shortDescription": {
"text": "Unused local variable"
},
"helpUri": "not a valid URI"
}
]
}
},
"results" : [ {
"ruleId" : "js/unused-local-variable",
"ruleIndex" : 0,
"message" : {
"text" : "Unused variable foo."
},
"locations" : [ {
"physicalLocation" : {
"artifactLocation" : {
"uri" : "not a valid URI",
"uriBaseId" : "%SRCROOT%",
"index" : 0
},
"region" : {
"startLine" : 2,
"startColumn" : 7,
"endColumn" : 10
"results": [
{
"ruleId": "js/unused-local-variable",
"ruleIndex": 0,
"message": {
"text": "Unused variable foo."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "not a valid URI",
"uriBaseId": "%SRCROOT%",
"index": 0
},
"region": {
"startLine": 2,
"startColumn": 7,
"endColumn": 10
}
}
}
}
} ]
} ],
]
}
],
"columnKind": "utf16CodeUnits",
"properties": {
"semmle.formatSpecifier": "2.1.0",

3
src/upload-lib.test.ts
View file

@ -317,9 +317,10 @@ test("accept results with invalid artifactLocation.uri value", (t) => {
const sarifFile = `${__dirname}/../src/testdata/with-invalid-uri.sarif`;
uploadLib.validateSarifFileSchema(sarifFile, mockLogger);
t.deepEqual(loggedMessages.length, 2);
t.deepEqual(loggedMessages.length, 3);
t.deepEqual(
loggedMessages[1],
"Warning: 'not a valid URI' is not a valid URI in 'instance.runs[0].tool.driver.rules[0].helpUri'.",
"Warning: 'not a valid URI' is not a valid URI in 'instance.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri'.",
);
});

17
src/upload-lib.ts
View file

@ -449,11 +449,20 @@ export function validateSarifFileSchema(sarifFilePath: string, logger: Logger) {
const result = new jsonschema.Validator().validate(sarif, schema);
// Filter errors related to invalid URIs in the artifactLocation field as this
// is a breaking change. See https://github.com/github/codeql-action/issues/1703
const errors = (result.errors || []).filter(
(err) => err.argument !== "uri-reference",
const warningAttributes = ["uri-reference", "uri"];
const errors = (result.errors ?? []).filter(
(err) =>
!(
err.name === "format" &&
typeof err.argument === "string" &&
warningAttributes.includes(err.argument)
),
);
const warnings = (result.errors || []).filter(
(err) => err.argument === "uri-reference",
const warnings = (result.errors ?? []).filter(
(err) =>
err.name === "format" &&
typeof err.argument === "string" &&
warningAttributes.includes(err.argument),
);
for (const warning of warnings) {