Skip to content

Commit 4764f83

Browse files
committed
Escape category names
Ensure category names are sanitized before converting them to an environment variable.
1 parent 730dae8 commit 4764f83

9 files changed

+41
-7
lines changed

lib/actions-util.js

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/actions-util.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/upload-lib.js

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/upload-lib.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/upload-lib.test.js

+8
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/upload-lib.test.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/actions-util.ts

+1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ export const getCommitOid = async function (ref = "HEAD"): Promise<string> {
8585
core.info(
8686
`Failed to call git to get current commit. Continuing with data from environment: ${e}`
8787
);
88+
core.info((e as Error).stack || "NO STACK");
8889
return getRequiredEnvParam("GITHUB_SHA");
8990
}
9091
};

src/upload-lib.test.ts

+10
Original file line numberDiff line numberDiff line change
@@ -185,4 +185,14 @@ test("validateUniqueCategory", (t) => {
185185

186186
t.notThrows(() => uploadLib.validateUniqueCategory("def"));
187187
t.throws(() => uploadLib.validateUniqueCategory("def"));
188+
189+
// Our category sanitization is not perfect. Here are some examples
190+
// of where we see false clashes
191+
t.notThrows(() => uploadLib.validateUniqueCategory("abc/def"));
192+
t.throws(() => uploadLib.validateUniqueCategory("abc@def"));
193+
t.throws(() => uploadLib.validateUniqueCategory("abc_def"));
194+
t.throws(() => uploadLib.validateUniqueCategory("abc def"));
195+
196+
// this one is fine
197+
t.notThrows(() => uploadLib.validateUniqueCategory("abc_ def"));
188198
});

src/upload-lib.ts

+17-3
Original file line numberDiff line numberDiff line change
@@ -404,15 +404,29 @@ async function uploadFiles(
404404
export function validateUniqueCategory(category: string | undefined) {
405405
if (util.isActions()) {
406406
// This check only works on actions as env vars don't persist between calls to the runner
407-
const sentinelEnvVar = `CODEQL_UPLOAD_SARIF + ${
408-
category ? `_${category}` : ""
407+
const sentinelEnvVar = `CODEQL_UPLOAD_SARIF${
408+
category ? `_${sanitize(category)}` : ""
409409
}`;
410410
if (process.env[sentinelEnvVar]) {
411411
throw new Error(
412412
"Aborting upload: only one run of the codeql/analyze or codeql/upload-sarif actions is allowed per job per category. " +
413-
"Please specify a unique `category` to call this action multiple times."
413+
"Please specify a unique `category` to call this action multiple times. " +
414+
`Category: ${category ? category : "(none)"}`
414415
);
415416
}
416417
core.exportVariable(sentinelEnvVar, sentinelEnvVar);
417418
}
418419
}
420+
421+
/**
422+
* Santizes a string to be used as an environment variable name.
423+
* This will replace all non-alphanumeric characters with underscores.
424+
* There could still be some false category clashes if two uploads
425+
* occur that differ only in their non-alphanumeric characters. This is
426+
* unlikely.
427+
*
428+
* @param str the initial value to sanitize
429+
*/
430+
function sanitize(str: string) {
431+
return str.replace(/[^a-zA-Z0-9_]/g, "_");
432+
}

0 commit comments

Comments
 (0)