Apply suggestions from code review

Co-authored-by: Henry Mercer <henry.mercer@me.com>
This commit is contained in:
Andrew Eisenberg
2023-02-09 11:19:27 -08:00
parent e2f72f11e4
commit 3c81243bb1
15 changed files with 130 additions and 91 deletions

18
lib/codeql.js generated
View File

@@ -99,7 +99,7 @@ exports.CODEQL_VERSION_BETTER_RESOLVE_LANGUAGES = "2.10.3";
*/
exports.CODEQL_VERSION_SECURITY_EXPERIMENTAL_SUITE = "2.12.1";
/**
* Versions 2.12.2+ of the CodeQL CLI support the `--qlconfig` flag in calls to `database init`.
* Versions 2.12.3+ of the CodeQL CLI support the `--qlconfig` flag in calls to `database init`.
*/
exports.CODEQL_VERSION_INIT_WITH_QLCONFIG = "2.12.3";
/**
@@ -324,13 +324,13 @@ async function getCodeQLForCmd(cmd, checkVersion) {
extraArgs.push("--no-internal-use-lua-tracing");
}
}
// A config file is only generated if the CliConfigFileEnabled feature flag is enabled.
const configLocation = await generateCodeScanningConfig(codeql, config, featureEnablement, logger);
// A code scanning config file is only generated if the CliConfigFileEnabled feature flag is enabled.
const codeScanningConfigFile = await generateCodeScanningConfig(codeql, config, featureEnablement, logger);
// Only pass external repository token if a config file is going to be parsed by the CLI.
let externalRepositoryToken;
if (configLocation) {
if (codeScanningConfigFile) {
externalRepositoryToken = (0, actions_util_1.getOptionalInput)("external-repository-token");
extraArgs.push(`--codescanning-config=${configLocation}`);
extraArgs.push(`--codescanning-config=${codeScanningConfigFile}`);
if (externalRepositoryToken) {
extraArgs.push("--external-repository-token-stdin");
}
@@ -722,7 +722,7 @@ async function generateCodeScanningConfig(codeql, config, featureEnablement, log
if (!(await util.useCodeScanningConfigInCli(codeql, featureEnablement))) {
return;
}
const configLocation = path.resolve(config.tempDir, "user-config.yaml");
const codeScanningConfigFile = path.resolve(config.tempDir, "user-config.yaml");
// make a copy so we can modify it
const augmentedConfig = cloneObject(config.originalUserInput);
// Inject the queries from the input
@@ -776,12 +776,12 @@ async function generateCodeScanningConfig(codeql, config, featureEnablement, log
augmentedConfig.packs["javascript"].push(packString);
}
}
logger.info(`Writing augmented user configuration file to ${configLocation}`);
logger.info(`Writing augmented user configuration file to ${codeScanningConfigFile}`);
logger.startGroup("Augmented user configuration file contents");
logger.info(yaml.dump(augmentedConfig));
logger.endGroup();
fs.writeFileSync(configLocation, yaml.dump(augmentedConfig));
return configLocation;
fs.writeFileSync(codeScanningConfigFile, yaml.dump(augmentedConfig));
return codeScanningConfigFile;
}
function cloneObject(obj) {
return JSON.parse(JSON.stringify(obj));

File diff suppressed because one or more lines are too long

38
lib/codeql.test.js generated
View File

@@ -665,22 +665,21 @@ const injectedConfigMacro = ava_1.default.macro({
queries: [],
},
}, {});
(0, ava_1.default)("does not use injected config", async (t) => {
(0, ava_1.default)("does not pass a code scanning config or qlconfig file to the CLI when CLI config passing is disabled", async (t) => {
const runnerConstructorStub = stubToolRunnerConstructor();
const codeqlObject = await codeql.getCodeQLForTesting();
sinon
.stub(codeqlObject, "getVersion")
.resolves(feature_flags_1.featureConfig[feature_flags_1.Feature.CliConfigFileEnabled].minimumVersion);
// stubbed version doesn't matter. It just needs to be valid semver.
sinon.stub(codeqlObject, "getVersion").resolves("0.0.0");
await codeqlObject.databaseInitCluster(stubConfig, "", undefined, (0, testing_utils_1.createFeatures)([]), "/path/to/qlconfig.yml", (0, logging_1.getRunnerLogger)(true));
const args = runnerConstructorStub.firstCall.args[1];
// should not have used a config file
const configArg = args.find((arg) => arg.startsWith("--codescanning-config="));
t.falsy(configArg, "Should NOT have injected a codescanning config");
const hasConfigArg = args.some((arg) => arg.startsWith("--codescanning-config="));
t.false(hasConfigArg, "Should NOT have injected a codescanning config");
// should not have passed a qlconfig file
const qlconfigArg = args.find((arg) => arg.startsWith("--qlconfig="));
t.falsy(qlconfigArg, "Should NOT have injected a codescanning config");
const hasQlconfigArg = args.find((arg) => arg.startsWith("--qlconfig="));
t.false(hasQlconfigArg, "Should NOT have passed a qlconfig file");
});
(0, ava_1.default)("uses injected config AND qlconfig", async (t) => {
(0, ava_1.default)("passes a code scanning config AND qlconfig to the CLI when CLI config passing is enabled", async (t) => {
const runnerConstructorStub = stubToolRunnerConstructor();
const codeqlObject = await codeql.getCodeQLForTesting();
sinon
@@ -689,11 +688,24 @@ const injectedConfigMacro = ava_1.default.macro({
await codeqlObject.databaseInitCluster(stubConfig, "", undefined, (0, testing_utils_1.createFeatures)([feature_flags_1.Feature.CliConfigFileEnabled]), "/path/to/qlconfig.yml", (0, logging_1.getRunnerLogger)(true));
const args = runnerConstructorStub.firstCall.args[1];
// should have used a config file
const configArg = args.find((arg) => arg.startsWith("--codescanning-config="));
t.truthy(configArg, "Should have injected a qlconfig");
const hasCodeScanningConfigArg = args.some((arg) => arg.startsWith("--codescanning-config="));
t.true(hasCodeScanningConfigArg, "Should have injected a qlconfig");
// should have passed a qlconfig file
const qlconfigArg = args.find((arg) => arg.startsWith("--qlconfig="));
t.truthy(qlconfigArg, "Should have injected a codescanning config");
const hasQlconfigArg = args.some((arg) => arg.startsWith("--qlconfig="));
t.truthy(hasQlconfigArg, "Should have injected a codescanning config");
});
(0, ava_1.default)("passes a code scanning config BUT NOT a qlconfig to the CLI when CLI config passing is enabled", async (t) => {
const runnerConstructorStub = stubToolRunnerConstructor();
const codeqlObject = await codeql.getCodeQLForTesting();
sinon.stub(codeqlObject, "getVersion").resolves("2.12.2");
await codeqlObject.databaseInitCluster(stubConfig, "", undefined, (0, testing_utils_1.createFeatures)([feature_flags_1.Feature.CliConfigFileEnabled]), "/path/to/qlconfig.yml", (0, logging_1.getRunnerLogger)(true));
const args = runnerConstructorStub.firstCall.args[1];
// should have used a config file
const hasCodeScanningConfigArg = args.some((arg) => arg.startsWith("--codescanning-config="));
t.true(hasCodeScanningConfigArg, "Should NOT have injected a qlconfig");
// should have passed a qlconfig file
const hasQlconfigArg = args.some((arg) => arg.startsWith("--qlconfig="));
t.false(hasQlconfigArg, "Should have injected a codescanning config");
});
(0, ava_1.default)("databaseInterpretResults() sets --sarif-add-baseline-file-info for 2.11.3", async (t) => {
const runnerConstructorStub = stubToolRunnerConstructor();

File diff suppressed because one or more lines are too long

18
lib/config-utils.js generated
View File

@@ -1056,8 +1056,7 @@ async function getConfig(tempDir, logger) {
}
exports.getConfig = getConfig;
async function downloadPacks(codeQL, languages, packs, apiDetails, registriesInput, tempDir, logger) {
// When config parsing in the cli is used, the registries will be generated
// immediately before the call to database init.
// This code path is only used when config parsing occurs in the Action.
const { registriesAuthTokens, qlconfigFile } = await generateRegistries(registriesInput, codeQL, tempDir, logger);
await wrapEnvironment({
GITHUB_TOKEN: apiDetails.auth,
@@ -1093,22 +1092,22 @@ exports.downloadPacks = downloadPacks;
*
* @param registriesInput The value of the `registries` input.
* @param codeQL a codeQL object, used only for checking the version of CodeQL.
* @param tmpDir a temporary directory to store the generated qlconfig.yml file.
* @param tempDir a temporary directory to store the generated qlconfig.yml file.
* @param logger a logger object.
* @returns The path to the generated `qlconfig.yml` file and the auth tokens to
* use for each registry.
*/
async function generateRegistries(registriesInput, codeQL, tmpDir, logger) {
async function generateRegistries(registriesInput, codeQL, tempDir, logger) {
const registries = parseRegistries(registriesInput);
let registriesAuthTokens;
let qlconfigFile;
if (registries) {
if (!(await (0, util_1.codeQlVersionAbove)(codeQL, codeql_1.CODEQL_VERSION_GHES_PACK_DOWNLOAD))) {
throw new Error(`'registries' input is not supported on CodeQL versions less than ${codeql_1.CODEQL_VERSION_GHES_PACK_DOWNLOAD}.`);
throw new Error(`The 'registries' input is not supported on CodeQL CLI versions earlier than ${codeql_1.CODEQL_VERSION_GHES_PACK_DOWNLOAD}. Please upgrade to CodeQL CLI version ${codeql_1.CODEQL_VERSION_GHES_PACK_DOWNLOAD} or later.`);
}
// generate a qlconfig.yml file to hold the registry configs.
const qlconfig = createRegistriesBlock(registries);
qlconfigFile = path.join(tmpDir, "qlconfig.yml");
qlconfigFile = path.join(tempDir, "qlconfig.yml");
const qlconfigContents = yaml.dump(qlconfig);
fs.writeFileSync(qlconfigFile, qlconfigContents, "utf8");
logger.debug("Generated qlconfig.yml:");
@@ -1117,7 +1116,12 @@ async function generateRegistries(registriesInput, codeQL, tmpDir, logger) {
.map((registry) => `${registry.url}=${registry.token}`)
.join(",");
}
return { registriesAuthTokens, qlconfigFile };
return {
registriesAuthTokens:
// if the user has explicitly set the CODEQL_REGISTRIES_AUTH env var then use that
process.env["CODEQL_REGISTRIES_AUTH"] ?? registriesAuthTokens,
qlconfigFile,
};
}
exports.generateRegistries = generateRegistries;
function createRegistriesBlock(registries) {

File diff suppressed because one or more lines are too long

View File

@@ -1115,8 +1115,7 @@ const calculateAugmentationErrorMacro = ava_1.default.macro({
go: ["c", "d"],
python: ["e", "f"],
}, sampleApiDetails, undefined, // registriesAuthTokens
tmpDir, // qlconfig file path
logger);
tmpDir, logger);
// Expecting packs to be downloaded once for java and once for python
t.deepEqual(packDownloadStub.callCount, 2);
// no config file was created, so pass `undefined` as the config file path
@@ -1256,12 +1255,6 @@ const calculateAugmentationErrorMacro = ava_1.default.macro({
packages: ["codeql/*", "dsp-testing/*"],
token: "not-a-token",
},
{
// with slash
url: "https://containers.GHEHOSTNAME1/v2/",
packages: "semmle/*",
token: "still-not-a-token",
},
]);
const codeQL = (0, codeql_1.setCodeQL)({
// Accepted CLI versions are 2.10.4 or higher
@@ -1269,8 +1262,6 @@ const calculateAugmentationErrorMacro = ava_1.default.macro({
});
const logger = (0, logging_1.getRunnerLogger)(true);
await t.throwsAsync(async () => await configUtils.generateRegistries(registriesInput, codeQL, tmpDir, logger), undefined, "'registries' input is not supported on CodeQL versions less than 2.10.4.");
// t.is(registriesAuthTokens, undefined);
// t.is(qlconfigFile, undefined);
});
});
(0, ava_1.default)("no generateRegistries when registries is undefined", async (t) => {

File diff suppressed because one or more lines are too long

7
lib/init.js generated
View File

@@ -61,9 +61,10 @@ async function runInit(codeql, config, sourceRoot, processName, registriesInput,
fs.mkdirSync(config.dbLocation, { recursive: true });
try {
if (await (0, util_1.codeQlVersionAbove)(codeql, codeql_1.CODEQL_VERSION_NEW_TRACING)) {
// Only create the qlconfig file if we haven't already created it.
// If we are not parsing the config file in the cli, then the qlconfig
// file has already been created.
// When parsing the codeql config in the CLI, we have not yet created the qlconfig file.
// So, create it now.
// If we are parsing the config file in the Action, then the qlconfig file was already created
// before the `pack download` command was invoked. It is not required for the init command.
let registriesAuthTokens;
let qlconfigFile;
if (await util.useCodeScanningConfigInCli(codeql, featureEnablement)) {

File diff suppressed because one or more lines are too long

View File

@@ -1002,12 +1002,11 @@ test(
{}
);
test("does not use injected config", async (t: ExecutionContext<unknown>) => {
test("does not pass a code scanning config or qlconfig file to the CLI when CLI config passing is disabled", async (t: ExecutionContext<unknown>) => {
const runnerConstructorStub = stubToolRunnerConstructor();
const codeqlObject = await codeql.getCodeQLForTesting();
sinon
.stub(codeqlObject, "getVersion")
.resolves(featureConfig[Feature.CliConfigFileEnabled].minimumVersion);
// stubbed version doesn't matter. It just needs to be valid semver.
sinon.stub(codeqlObject, "getVersion").resolves("0.0.0");
await codeqlObject.databaseInitCluster(
stubConfig,
@@ -1020,17 +1019,19 @@ test("does not use injected config", async (t: ExecutionContext<unknown>) => {
const args = runnerConstructorStub.firstCall.args[1];
// should not have used a config file
const configArg = args.find((arg: string) =>
const hasConfigArg = args.some((arg: string) =>
arg.startsWith("--codescanning-config=")
);
t.falsy(configArg, "Should NOT have injected a codescanning config");
t.false(hasConfigArg, "Should NOT have injected a codescanning config");
// should not have passed a qlconfig file
const qlconfigArg = args.find((arg: string) => arg.startsWith("--qlconfig="));
t.falsy(qlconfigArg, "Should NOT have injected a codescanning config");
const hasQlconfigArg = args.find((arg: string) =>
arg.startsWith("--qlconfig=")
);
t.false(hasQlconfigArg, "Should NOT have passed a qlconfig file");
});
test("uses injected config AND qlconfig", async (t: ExecutionContext<unknown>) => {
test("passes a code scanning config AND qlconfig to the CLI when CLI config passing is enabled", async (t: ExecutionContext<unknown>) => {
const runnerConstructorStub = stubToolRunnerConstructor();
const codeqlObject = await codeql.getCodeQLForTesting();
sinon
@@ -1048,14 +1049,43 @@ test("uses injected config AND qlconfig", async (t: ExecutionContext<unknown>) =
const args = runnerConstructorStub.firstCall.args[1];
// should have used a config file
const configArg = args.find((arg: string) =>
const hasCodeScanningConfigArg = args.some((arg: string) =>
arg.startsWith("--codescanning-config=")
);
t.truthy(configArg, "Should have injected a qlconfig");
t.true(hasCodeScanningConfigArg, "Should have injected a qlconfig");
// should have passed a qlconfig file
const qlconfigArg = args.find((arg: string) => arg.startsWith("--qlconfig="));
t.truthy(qlconfigArg, "Should have injected a codescanning config");
const hasQlconfigArg = args.some((arg: string) =>
arg.startsWith("--qlconfig=")
);
t.truthy(hasQlconfigArg, "Should have injected a codescanning config");
});
test("passes a code scanning config BUT NOT a qlconfig to the CLI when CLI config passing is enabled", async (t: ExecutionContext<unknown>) => {
const runnerConstructorStub = stubToolRunnerConstructor();
const codeqlObject = await codeql.getCodeQLForTesting();
sinon.stub(codeqlObject, "getVersion").resolves("2.12.2");
await codeqlObject.databaseInitCluster(
stubConfig,
"",
undefined,
createFeatures([Feature.CliConfigFileEnabled]),
"/path/to/qlconfig.yml",
getRunnerLogger(true)
);
const args = runnerConstructorStub.firstCall.args[1] as any[];
// should have used a config file
const hasCodeScanningConfigArg = args.some((arg: string) =>
arg.startsWith("--codescanning-config=")
);
t.true(hasCodeScanningConfigArg, "Should NOT have injected a qlconfig");
// should have passed a qlconfig file
const hasQlconfigArg = args.some((arg: string) =>
arg.startsWith("--qlconfig=")
);
t.false(hasQlconfigArg, "Should have injected a codescanning config");
});
test("databaseInterpretResults() sets --sarif-add-baseline-file-info for 2.11.3", async (t) => {

View File

@@ -285,7 +285,7 @@ export const CODEQL_VERSION_BETTER_RESOLVE_LANGUAGES = "2.10.3";
export const CODEQL_VERSION_SECURITY_EXPERIMENTAL_SUITE = "2.12.1";
/**
* Versions 2.12.2+ of the CodeQL CLI support the `--qlconfig` flag in calls to `database init`.
* Versions 2.12.3+ of the CodeQL CLI support the `--qlconfig` flag in calls to `database init`.
*/
export const CODEQL_VERSION_INIT_WITH_QLCONFIG = "2.12.3";
@@ -595,8 +595,8 @@ export async function getCodeQLForCmd(
}
}
// A config file is only generated if the CliConfigFileEnabled feature flag is enabled.
const configLocation = await generateCodeScanningConfig(
// A code scanning config file is only generated if the CliConfigFileEnabled feature flag is enabled.
const codeScanningConfigFile = await generateCodeScanningConfig(
codeql,
config,
featureEnablement,
@@ -604,9 +604,9 @@ export async function getCodeQLForCmd(
);
// Only pass external repository token if a config file is going to be parsed by the CLI.
let externalRepositoryToken: string | undefined;
if (configLocation) {
if (codeScanningConfigFile) {
externalRepositoryToken = getOptionalInput("external-repository-token");
extraArgs.push(`--codescanning-config=${configLocation}`);
extraArgs.push(`--codescanning-config=${codeScanningConfigFile}`);
if (externalRepositoryToken) {
extraArgs.push("--external-repository-token-stdin");
}
@@ -1112,7 +1112,10 @@ async function generateCodeScanningConfig(
if (!(await util.useCodeScanningConfigInCli(codeql, featureEnablement))) {
return;
}
const configLocation = path.resolve(config.tempDir, "user-config.yaml");
const codeScanningConfigFile = path.resolve(
config.tempDir,
"user-config.yaml"
);
// make a copy so we can modify it
const augmentedConfig = cloneObject(config.originalUserInput);
@@ -1169,13 +1172,15 @@ async function generateCodeScanningConfig(
augmentedConfig.packs["javascript"].push(packString);
}
}
logger.info(`Writing augmented user configuration file to ${configLocation}`);
logger.info(
`Writing augmented user configuration file to ${codeScanningConfigFile}`
);
logger.startGroup("Augmented user configuration file contents");
logger.info(yaml.dump(augmentedConfig));
logger.endGroup();
fs.writeFileSync(configLocation, yaml.dump(augmentedConfig));
return configLocation;
fs.writeFileSync(codeScanningConfigFile, yaml.dump(augmentedConfig));
return codeScanningConfigFile;
}
function cloneObject<T>(obj: T): T {

View File

@@ -2283,7 +2283,7 @@ test("downloadPacks-no-registries", async (t) => {
},
sampleApiDetails,
undefined, // registriesAuthTokens
tmpDir, // qlconfig file path
tmpDir,
logger
);
@@ -2481,12 +2481,6 @@ test("no generateRegistries when CLI is too old", async (t) => {
packages: ["codeql/*", "dsp-testing/*"],
token: "not-a-token",
},
{
// with slash
url: "https://containers.GHEHOSTNAME1/v2/",
packages: "semmle/*",
token: "still-not-a-token",
},
]);
const codeQL = setCodeQL({
// Accepted CLI versions are 2.10.4 or higher
@@ -2504,9 +2498,6 @@ test("no generateRegistries when CLI is too old", async (t) => {
undefined,
"'registries' input is not supported on CodeQL versions less than 2.10.4."
);
// t.is(registriesAuthTokens, undefined);
// t.is(qlconfigFile, undefined);
});
});
test("no generateRegistries when registries is undefined", async (t) => {

View File

@@ -1903,8 +1903,7 @@ export async function downloadPacks(
tempDir: string,
logger: Logger
) {
// When config parsing in the cli is used, the registries will be generated
// immediately before the call to database init.
// This code path is only used when config parsing occurs in the Action.
const { registriesAuthTokens, qlconfigFile } = await generateRegistries(
registriesInput,
codeQL,
@@ -1956,7 +1955,7 @@ export async function downloadPacks(
*
* @param registriesInput The value of the `registries` input.
* @param codeQL a codeQL object, used only for checking the version of CodeQL.
* @param tmpDir a temporary directory to store the generated qlconfig.yml file.
* @param tempDir a temporary directory to store the generated qlconfig.yml file.
* @param logger a logger object.
* @returns The path to the generated `qlconfig.yml` file and the auth tokens to
* use for each registry.
@@ -1964,7 +1963,7 @@ export async function downloadPacks(
export async function generateRegistries(
registriesInput: string | undefined,
codeQL: CodeQL,
tmpDir: string,
tempDir: string,
logger: Logger
) {
const registries = parseRegistries(registriesInput);
@@ -1975,13 +1974,13 @@ export async function generateRegistries(
!(await codeQlVersionAbove(codeQL, CODEQL_VERSION_GHES_PACK_DOWNLOAD))
) {
throw new Error(
`'registries' input is not supported on CodeQL versions less than ${CODEQL_VERSION_GHES_PACK_DOWNLOAD}.`
`The 'registries' input is not supported on CodeQL CLI versions earlier than ${CODEQL_VERSION_GHES_PACK_DOWNLOAD}. Please upgrade to CodeQL CLI version ${CODEQL_VERSION_GHES_PACK_DOWNLOAD} or later.`
);
}
// generate a qlconfig.yml file to hold the registry configs.
const qlconfig = createRegistriesBlock(registries);
qlconfigFile = path.join(tmpDir, "qlconfig.yml");
qlconfigFile = path.join(tempDir, "qlconfig.yml");
const qlconfigContents = yaml.dump(qlconfig);
fs.writeFileSync(qlconfigFile, qlconfigContents, "utf8");
@@ -1991,7 +1990,12 @@ export async function generateRegistries(
.map((registry) => `${registry.url}=${registry.token}`)
.join(",");
}
return { registriesAuthTokens, qlconfigFile };
return {
registriesAuthTokens:
// if the user has explicitly set the CODEQL_REGISTRIES_AUTH env var then use that
process.env["CODEQL_REGISTRIES_AUTH"] ?? registriesAuthTokens,
qlconfigFile,
};
}
function createRegistriesBlock(registries: RegistryConfigWithCredentials[]): {

View File

@@ -111,9 +111,10 @@ export async function runInit(
try {
if (await codeQlVersionAbove(codeql, CODEQL_VERSION_NEW_TRACING)) {
// Only create the qlconfig file if we haven't already created it.
// If we are not parsing the config file in the cli, then the qlconfig
// file has already been created.
// When parsing the codeql config in the CLI, we have not yet created the qlconfig file.
// So, create it now.
// If we are parsing the config file in the Action, then the qlconfig file was already created
// before the `pack download` command was invoked. It is not required for the init command.
let registriesAuthTokens: string | undefined;
let qlconfigFile: string | undefined;
if (await util.useCodeScanningConfigInCli(codeql, featureEnablement)) {