Merge branch 'main' into aeisenberg/qlconfig-in-cli

This commit is contained in:
Andrew Eisenberg 2023-02-26 18:35:21 -08:00
commit 8f19113f88
29 changed files with 510 additions and 133 deletions

View file

@ -3,15 +3,18 @@ import * as path from "path";
import test, { ExecutionContext } from "ava";
import * as yaml from "js-yaml";
import * as sinon from "sinon";
import {
convertPackToQuerySuiteEntry,
createQuerySuiteContents,
runQueries,
validateQueryFilters,
QueriesStatusReport,
} from "./analyze";
import { setCodeQL } from "./codeql";
import { Config } from "./config-utils";
import { CodeQL, setCodeQL } from "./codeql";
import { Config, QueriesWithSearchPath } from "./config-utils";
import { Feature } from "./feature-flags";
import { Language } from "./languages";
import { getRunnerLogger } from "./logging";
import { setupTests, setupActionsVars, createFeatures } from "./testing-utils";
@ -229,6 +232,175 @@ test("status report fields and search path setting", async (t) => {
}
});
function mockCodeQL(): Partial<CodeQL> {
return {
getVersion: async () => "2.12.2",
databaseRunQueries: sinon.spy(),
databaseInterpretResults: async () => "",
databasePrintBaseline: async () => "",
};
}
function createBaseConfig(tmpDir: string): Config {
return {
languages: [],
queries: {},
pathsIgnore: [],
paths: [],
originalUserInput: {},
tempDir: "tempDir",
codeQLCmd: "",
gitHubVersion: {
type: util.GitHubVariant.DOTCOM,
} as util.GitHubVersion,
dbLocation: path.resolve(tmpDir, "codeql_databases"),
packs: {},
debugMode: false,
debugArtifactName: util.DEFAULT_DEBUG_ARTIFACT_NAME,
debugDatabaseName: util.DEFAULT_DEBUG_DATABASE_NAME,
augmentationProperties: {
injectedMlQueries: false,
packsInputCombines: false,
queriesInputCombines: false,
},
trapCaches: {},
trapCacheDownloadTime: 0,
};
}
function createQueryConfig(
builtin: string[],
custom: string[]
): { builtin: string[]; custom: QueriesWithSearchPath[] } {
return {
builtin,
custom: custom.map((c) => ({ searchPath: "/search", queries: [c] })),
};
}
async function runQueriesWithConfig(
config: Config,
features: Feature[]
): Promise<QueriesStatusReport> {
for (const language of config.languages) {
fs.mkdirSync(util.getCodeQLDatabasePath(config, language), {
recursive: true,
});
}
return runQueries(
"sarif-folder",
"--memFlag",
"--addSnippetsFlag",
"--threadsFlag",
undefined,
config,
getRunnerLogger(true),
createFeatures(features)
);
}
function getDatabaseRunQueriesCalls(mock: Partial<CodeQL>) {
return (mock.databaseRunQueries as sinon.SinonSpy).getCalls();
}
test("optimizeForLastQueryRun for one language", async (t) => {
return await util.withTmpDir(async (tmpDir) => {
const codeql = mockCodeQL();
setCodeQL(codeql);
const config: Config = createBaseConfig(tmpDir);
config.languages = [Language.cpp];
config.queries.cpp = createQueryConfig(["foo.ql"], []);
await runQueriesWithConfig(config, []);
t.deepEqual(
getDatabaseRunQueriesCalls(codeql).map((c) => c.args[4]),
[true]
);
});
});
test("optimizeForLastQueryRun for two languages", async (t) => {
return await util.withTmpDir(async (tmpDir) => {
const codeql = mockCodeQL();
setCodeQL(codeql);
const config: Config = createBaseConfig(tmpDir);
config.languages = [Language.cpp, Language.java];
config.queries.cpp = createQueryConfig(["foo.ql"], []);
config.queries.java = createQueryConfig(["bar.ql"], []);
await runQueriesWithConfig(config, []);
t.deepEqual(
getDatabaseRunQueriesCalls(codeql).map((c) => c.args[4]),
[true, true]
);
});
});
test("optimizeForLastQueryRun for two languages, with custom queries", async (t) => {
return await util.withTmpDir(async (tmpDir) => {
const codeql = mockCodeQL();
setCodeQL(codeql);
const config: Config = createBaseConfig(tmpDir);
config.languages = [Language.cpp, Language.java];
config.queries.cpp = createQueryConfig(["foo.ql"], ["c1.ql", "c2.ql"]);
config.queries.java = createQueryConfig(["bar.ql"], ["c3.ql"]);
await runQueriesWithConfig(config, []);
t.deepEqual(
getDatabaseRunQueriesCalls(codeql).map((c) => c.args[4]),
[false, false, true, false, true]
);
});
});
test("optimizeForLastQueryRun for two languages, with custom queries and packs", async (t) => {
return await util.withTmpDir(async (tmpDir) => {
const codeql = mockCodeQL();
setCodeQL(codeql);
const config: Config = createBaseConfig(tmpDir);
config.languages = [Language.cpp, Language.java];
config.queries.cpp = createQueryConfig(["foo.ql"], ["c1.ql", "c2.ql"]);
config.queries.java = createQueryConfig(["bar.ql"], ["c3.ql"]);
config.packs.cpp = ["a/cpp-pack1@0.1.0"];
config.packs.java = ["b/java-pack1@0.2.0", "b/java-pack2@0.3.3"];
await runQueriesWithConfig(config, []);
t.deepEqual(
getDatabaseRunQueriesCalls(codeql).map((c) => c.args[4]),
[false, false, false, true, false, false, true]
);
});
});
test("optimizeForLastQueryRun for one language, CliConfigFileEnabled", async (t) => {
return await util.withTmpDir(async (tmpDir) => {
const codeql = mockCodeQL();
setCodeQL(codeql);
const config: Config = createBaseConfig(tmpDir);
config.languages = [Language.cpp];
await runQueriesWithConfig(config, [Feature.CliConfigFileEnabled]);
t.deepEqual(
getDatabaseRunQueriesCalls(codeql).map((c) => c.args[4]),
[true]
);
});
});
test("optimizeForLastQueryRun for two languages, CliConfigFileEnabled", async (t) => {
return await util.withTmpDir(async (tmpDir) => {
const codeql = mockCodeQL();
setCodeQL(codeql);
const config: Config = createBaseConfig(tmpDir);
config.languages = [Language.cpp, Language.java];
await runQueriesWithConfig(config, [Feature.CliConfigFileEnabled]);
t.deepEqual(
getDatabaseRunQueriesCalls(codeql).map((c) => c.args[4]),
[true, true]
);
});
});
test("validateQueryFilters", (t) => {
t.notThrows(() => validateQueryFilters([]));
t.notThrows(() => validateQueryFilters(undefined));

View file

@ -212,6 +212,7 @@ export async function runQueries(
const statusReport: QueriesStatusReport = {};
const codeql = await getCodeQL(config.codeQLCmd);
const queryFlags = [memoryFlag, threadsFlag];
await util.logCodeScanningConfigInCli(codeql, featureEnablement, logger);
@ -231,7 +232,7 @@ export async function runQueries(
// another to interpret the results.
logger.startGroup(`Running queries for ${language}`);
const startTimeBuiltIn = new Date().getTime();
await runQueryGroup(language, "all", undefined, undefined);
await runQueryGroup(language, "all", undefined, undefined, true);
// TODO should not be using `builtin` here. We should be using `all` instead.
// The status report does not support `all` yet.
statusReport[`analyze_builtin_queries_${language}_duration_ms`] =
@ -267,16 +268,24 @@ export async function runQueries(
);
}
const customQueryIndices: number[] = [];
for (let i = 0; i < queries.custom.length; ++i) {
if (queries.custom[i].queries.length > 0) {
customQueryIndices.push(i);
}
}
logger.startGroup(`Running queries for ${language}`);
const querySuitePaths: string[] = [];
if (queries["builtin"].length > 0) {
if (queries.builtin.length > 0) {
const startTimeBuiltIn = new Date().getTime();
querySuitePaths.push(
(await runQueryGroup(
language,
"builtin",
createQuerySuiteContents(queries["builtin"], queryFilters),
undefined
createQuerySuiteContents(queries.builtin, queryFilters),
undefined,
customQueryIndices.length === 0 && packsWithVersion.length === 0
)) as string
);
statusReport[`analyze_builtin_queries_${language}_duration_ms`] =
@ -284,21 +293,18 @@ export async function runQueries(
}
const startTimeCustom = new Date().getTime();
let ranCustom = false;
for (let i = 0; i < queries["custom"].length; ++i) {
if (queries["custom"][i].queries.length > 0) {
querySuitePaths.push(
(await runQueryGroup(
language,
`custom-${i}`,
createQuerySuiteContents(
queries["custom"][i].queries,
queryFilters
),
queries["custom"][i].searchPath
)) as string
);
ranCustom = true;
}
for (const i of customQueryIndices) {
querySuitePaths.push(
(await runQueryGroup(
language,
`custom-${i}`,
createQuerySuiteContents(queries.custom[i].queries, queryFilters),
queries.custom[i].searchPath,
i === customQueryIndices[customQueryIndices.length - 1] &&
packsWithVersion.length === 0
)) as string
);
ranCustom = true;
}
if (packsWithVersion.length > 0) {
querySuitePaths.push(
@ -306,7 +312,8 @@ export async function runQueries(
language,
"packs",
packsWithVersion,
queryFilters
queryFilters,
true
)
);
ranCustom = true;
@ -373,7 +380,8 @@ export async function runQueries(
language: Language,
type: string,
querySuiteContents: string | undefined,
searchPath: string | undefined
searchPath: string | undefined,
optimizeForLastQueryRun: boolean
): Promise<string | undefined> {
const databasePath = util.getCodeQLDatabasePath(config, language);
// Pass the queries to codeql using a file instead of using the command
@ -391,8 +399,8 @@ export async function runQueries(
databasePath,
searchPath,
querySuitePath,
memoryFlag,
threadsFlag
queryFlags,
optimizeForLastQueryRun
);
logger.debug(`BQRS results produced for ${language} (queries: ${type})"`);
@ -402,7 +410,8 @@ export async function runQueries(
language: Language,
type: string,
packs: string[],
queryFilters: configUtils.QueryFilter[]
queryFilters: configUtils.QueryFilter[],
optimizeForLastQueryRun: boolean
): Promise<string> {
const databasePath = util.getCodeQLDatabasePath(config, language);
@ -424,8 +433,8 @@ export async function runQueries(
databasePath,
undefined,
querySuitePath,
memoryFlag,
threadsFlag
queryFlags,
optimizeForLastQueryRun
);
return querySuitePath;

View file

@ -149,13 +149,19 @@ export interface CodeQL {
): Promise<void>;
/**
* Run 'codeql database run-queries'.
*
* @param optimizeForLastQueryRun Whether to apply additional optimization for
* the last database query run in the action.
* It is always safe to set it to false.
* It should be set to true only for the very
* last databaseRunQueries() call.
*/
databaseRunQueries(
databasePath: string,
extraSearchPath: string | undefined,
querySuitePath: string | undefined,
memoryFlag: string,
threadsFlag: string
flags: string[],
optimizeForLastQueryRun: boolean
): Promise<void>;
/**
* Run 'codeql database interpret-results'.
@ -801,19 +807,24 @@ export async function getCodeQLForCmd(
databasePath: string,
extraSearchPath: string | undefined,
querySuitePath: string | undefined,
memoryFlag: string,
threadsFlag: string
flags: string[],
optimizeForLastQueryRun: boolean
): Promise<void> {
const codeqlArgs = [
"database",
"run-queries",
memoryFlag,
threadsFlag,
...flags,
databasePath,
"--min-disk-free=1024", // Try to leave at least 1GB free
"-v",
...getExtraOptionsFromEnv(["database", "run-queries"]),
];
if (
optimizeForLastQueryRun &&
(await util.supportExpectDiscardedCache(this))
) {
codeqlArgs.push("--expect-discarded-cache");
}
if (extraSearchPath !== undefined) {
codeqlArgs.push("--additional-packs", extraSearchPath);
}

View file

@ -1,6 +1,6 @@
{
"bundleVersion": "codeql-bundle-20230207",
"cliVersion": "2.12.2",
"priorBundleVersion": "codeql-bundle-20230120",
"priorCliVersion": "2.12.1"
"bundleVersion": "codeql-bundle-20230217",
"cliVersion": "2.12.3",
"priorBundleVersion": "codeql-bundle-20230207",
"priorCliVersion": "2.12.2"
}

View file

@ -37,7 +37,6 @@ export enum Feature {
CliConfigFileEnabled = "cli_config_file_enabled",
DisableKotlinAnalysisEnabled = "disable_kotlin_analysis_enabled",
MlPoweredQueriesEnabled = "ml_powered_queries_enabled",
TrapCachingEnabled = "trap_caching_enabled",
UploadFailedSarifEnabled = "upload_failed_sarif_enabled",
}
@ -57,10 +56,6 @@ export const featureConfig: Record<
envVar: "CODEQL_ML_POWERED_QUERIES",
minimumVersion: "2.7.5",
},
[Feature.TrapCachingEnabled]: {
envVar: "CODEQL_TRAP_CACHING",
minimumVersion: undefined,
},
[Feature.UploadFailedSarifEnabled]: {
envVar: "CODEQL_ACTION_UPLOAD_FAILED_SARIF",
minimumVersion: "2.11.3",
@ -153,13 +148,17 @@ export class Features implements FeatureEnablement {
class GitHubFeatureFlags implements FeatureEnablement {
private cachedApiResponse: GitHubFeatureFlagsApiResponse | undefined;
// We cache whether the feature flags were accessed or not in order to accurately report whether flags were
// incorrectly configured vs. inaccessible in our telemetry.
private hasAccessedRemoteFeatureFlags: boolean;
constructor(
private readonly gitHubVersion: util.GitHubVersion,
private readonly repositoryNwo: RepositoryNwo,
private readonly featureFlagsFile: string,
private readonly logger: Logger
) {
/**/
this.hasAccessedRemoteFeatureFlags = false; // Not accessed by default.
}
private getCliVersionFromFeatureFlag(f: string): string | undefined {
@ -192,7 +191,9 @@ class GitHubFeatureFlags implements FeatureEnablement {
const defaultDotComCliVersion = await this.getDefaultDotcomCliVersion();
return {
cliVersion: defaultDotComCliVersion.version,
toolsFeatureFlagsValid: defaultDotComCliVersion.toolsFeatureFlagsValid,
toolsFeatureFlagsValid: this.hasAccessedRemoteFeatureFlags
? defaultDotComCliVersion.toolsFeatureFlagsValid
: undefined,
variant,
};
}
@ -205,7 +206,7 @@ class GitHubFeatureFlags implements FeatureEnablement {
async getDefaultDotcomCliVersion(): Promise<{
version: string;
toolsFeatureFlagsValid: boolean;
toolsFeatureFlagsValid: boolean | undefined;
}> {
const response = await this.getAllFeatures();
@ -233,7 +234,9 @@ class GitHubFeatureFlags implements FeatureEnablement {
);
return {
version: defaults.cliVersion,
toolsFeatureFlagsValid: false,
toolsFeatureFlagsValid: this.hasAccessedRemoteFeatureFlags
? false
: undefined,
};
}
@ -331,6 +334,7 @@ class GitHubFeatureFlags implements FeatureEnablement {
this.logger.debug(
"Not running against github.com. Disabling all toggleable features."
);
this.hasAccessedRemoteFeatureFlags = false;
return {};
}
try {
@ -346,6 +350,7 @@ class GitHubFeatureFlags implements FeatureEnablement {
"Loaded the following default values for the feature flags from the Code Scanning API: " +
`${JSON.stringify(remoteFlags)}`
);
this.hasAccessedRemoteFeatureFlags = true;
return remoteFlags;
} catch (e) {
if (util.isHTTPError(e) && e.status === 403) {
@ -355,6 +360,7 @@ class GitHubFeatureFlags implements FeatureEnablement {
"This could be because the Action is running on a pull request from a fork. If not, " +
`please ensure the Action has the 'security-events: write' permission. Details: ${e}`
);
this.hasAccessedRemoteFeatureFlags = false;
return {};
} else {
// Some features, such as `ml_powered_queries_enabled` affect the produced alerts.
@ -366,6 +372,5 @@ class GitHubFeatureFlags implements FeatureEnablement {
);
}
}
return {};
}
}

View file

@ -16,7 +16,7 @@ import {
import { getGitHubVersion } from "./api-client";
import { CodeQL, CODEQL_VERSION_NEW_TRACING } from "./codeql";
import * as configUtils from "./config-utils";
import { Feature, FeatureEnablement, Features } from "./feature-flags";
import { Feature, Features } from "./feature-flags";
import {
initCodeQL,
initConfig,
@ -86,7 +86,8 @@ interface InitWithConfigStatusReport extends InitStatusReport {
interface InitToolsDownloadFields {
/** Time taken to download the bundle, in milliseconds. */
tools_download_duration_ms?: number;
/** Whether the relevant tools dotcom feature flags have been misconfigured.
/**
* Whether the relevant tools dotcom feature flags have been misconfigured.
* Only populated if we attempt to determine the default version based on the dotcom feature flags. */
tools_feature_flags_valid?: boolean;
}
@ -117,13 +118,14 @@ async function sendInitStatusReport(
workflow_languages: workflowLanguages || "",
};
let initToolsDownloadFields: InitToolsDownloadFields = {};
const initToolsDownloadFields: InitToolsDownloadFields = {};
if (toolsSource === ToolsSource.Download) {
initToolsDownloadFields = {
tools_download_duration_ms: toolsDownloadDurationMs,
tools_feature_flags_valid: toolsFeatureFlagsValid,
};
if (toolsDownloadDurationMs !== undefined) {
initToolsDownloadFields.tools_download_duration_ms =
toolsDownloadDurationMs;
}
if (toolsFeatureFlagsValid !== undefined) {
initToolsDownloadFields.tools_feature_flags_valid = toolsFeatureFlagsValid;
}
if (config !== undefined) {
@ -254,7 +256,7 @@ async function run() {
registriesInput,
getOptionalInput("config-file"),
getOptionalInput("db-location"),
await getTrapCachingEnabled(features),
getTrapCachingEnabled(),
// Debug mode is enabled if:
// - The `init` Action is passed `debug: true`.
// - Actions step debugging is enabled (e.g. by [enabling debug logging for a rerun](https://docs.github.com/en/actions/managing-workflow-runs/re-running-workflows-and-jobs#re-running-all-the-jobs-in-a-workflow),
@ -388,9 +390,7 @@ async function run() {
);
}
async function getTrapCachingEnabled(
featureEnablement: FeatureEnablement
): Promise<boolean> {
function getTrapCachingEnabled(): boolean {
// If the workflow specified something always respect that
const trapCaching = getOptionalInput("trap-caching");
if (trapCaching !== undefined) return trapCaching === "true";
@ -398,8 +398,8 @@ async function getTrapCachingEnabled(
// On self-hosted runners which may have slow network access, disable TRAP caching by default
if (!isHostedRunner()) return false;
// On hosted runners, respect the feature flag
return await featureEnablement.getValue(Feature.TrapCachingEnabled);
// On hosted runners, enable TRAP caching by default
return true;
}
async function runWrapper() {

View file

@ -463,7 +463,9 @@ export async function waitForProcessing(
util.assertNever(status);
}
await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS);
await util.delay(STATUS_CHECK_FREQUENCY_MILLISECONDS, {
allowProcessExit: false,
});
}
} finally {
logger.endGroup();

View file

@ -548,16 +548,38 @@ export async function bundleDb(
return databaseBundlePath;
}
export async function delay(milliseconds: number) {
// Immediately `unref` the timer such that it only prevents the process from exiting if the
// surrounding promise is being awaited.
return new Promise((resolve) => setTimeout(resolve, milliseconds).unref());
/**
* @param milliseconds time to delay
* @param opts options
* @param opts.allowProcessExit if true, the timer will not prevent the process from exiting
*/
export async function delay(
milliseconds: number,
{ allowProcessExit }: { allowProcessExit: boolean }
) {
return new Promise((resolve) => {
const timer = setTimeout(resolve, milliseconds);
if (allowProcessExit) {
// Immediately `unref` the timer such that it only prevents the process from exiting if the
// surrounding promise is being awaited.
timer.unref();
}
});
}
export function isGoodVersion(versionSpec: string) {
return !BROKEN_VERSIONS.includes(versionSpec);
}
/**
* Checks whether the CodeQL CLI supports the `--expect-discarded-cache` command-line flag.
*/
export async function supportExpectDiscardedCache(
codeQL: CodeQL
): Promise<boolean> {
return codeQlVersionAbove(codeQL, "2.12.1");
}
export const ML_POWERED_JS_QUERIES_PACK_NAME =
"codeql/javascript-experimental-atm-queries";
@ -748,7 +770,7 @@ export async function withTimeout<T>(
return result;
};
const timeoutTask = async () => {
await delay(timeoutMs);
await delay(timeoutMs, { allowProcessExit: true });
if (!finished) {
// Workaround: While the promise racing below will allow the main code
// to continue, the process won't normally exit until the asynchronous
@ -773,7 +795,7 @@ export async function checkForTimeout() {
core.info(
"A timeout occurred, force exiting the process after 30 seconds to prevent hanging."
);
await delay(30_000);
await delay(30_000, { allowProcessExit: true });
process.exit();
}
}