Skip to content

Commit d264bca

Browse files
committed
Copy more files into the synthetic variant analysis pack
Before this change and starting with CLI v2.14.3, if you wanted to run a variant analysis query and the pack it is contained in has at least one query that contains an extensible predicate, this would be an error. The reason is that v2.14.3 introduced deep validation for data extensions. We are not copying the query that contains an extensible predicate to the synthetic pack we are generating. This means that deep validation will fail because there will be extensions that target the missing extensible predicate. This change avoids the problem by copying any query files that contain extensible predicates to the synthetic pack. It uses the internal `generate extensible-predicate-metadata` command to discover which query files contain extensible predicates and copies them.
1 parent 247ba4e commit d264bca

File tree

3 files changed

+100
-4
lines changed

3 files changed

+100
-4
lines changed

extensions/ql-vscode/src/codeql-cli/cli.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,15 @@ export type ResolveExtensionsResult = {
123123
};
124124
};
125125

126+
export type GenerateExtensiblePredicateMetadataResult = {
127+
// There are other properties in this object, but they are
128+
// not relevant for its use in the extension, so we omit them.
129+
extensible_predicates: Array<{
130+
// pack relative path
131+
path: string;
132+
}>;
133+
};
134+
126135
/**
127136
* The expected output of `codeql resolve qlref`.
128137
*/
@@ -1458,6 +1467,17 @@ export class CodeQLCliServer implements Disposable {
14581467
);
14591468
}
14601469

1470+
async generateExtensiblePredicateMetadata(
1471+
packRoot: string,
1472+
): Promise<GenerateExtensiblePredicateMetadataResult> {
1473+
return await this.runJsonCodeQlCliCommand(
1474+
["generate", "extensible-predicate-metadata"],
1475+
[packRoot],
1476+
"Generating extensible predicate metadata",
1477+
{ addFormat: false },
1478+
);
1479+
}
1480+
14611481
public async getVersion() {
14621482
if (!this._version) {
14631483
try {
@@ -1830,6 +1850,14 @@ export class CliVersionConstraint {
18301850
*/
18311851
public static CLI_VERSION_WITH_QUICK_EVAL_COUNT = new SemVer("2.13.3");
18321852

1853+
/**
1854+
* CLI version where the `generate extensible-predicate-metadata`
1855+
* command was implemented.
1856+
*/
1857+
public static CLI_VERSION_WITH_EXTENSIBLE_PREDICATE_METADATA = new SemVer(
1858+
"2.14.3",
1859+
);
1860+
18331861
/**
18341862
* CLI version where the langauge server supports visisbility change notifications.
18351863
*/
@@ -1916,4 +1944,10 @@ export class CliVersionConstraint {
19161944
CliVersionConstraint.CLI_VERSION_WITH_QUICK_EVAL_COUNT,
19171945
);
19181946
}
1947+
1948+
async supportsGenerateExtensiblePredicateMetadata() {
1949+
return this.isVersionAtLeast(
1950+
CliVersionConstraint.CLI_VERSION_WITH_EXTENSIBLE_PREDICATE_METADATA,
1951+
);
1952+
}
19191953
}

extensions/ql-vscode/src/variant-analysis/run-remote-query.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,22 @@ async function copyExistingQueryPack(
189189
) {
190190
const toCopy = await cliServer.packPacklist(originalPackRoot, false);
191191

192+
// Also include query files that contain extensible predicates. These query files are not
193+
// needed for the query to run, but they are needed for the query pack to pass deep validation
194+
// of data extensions.
195+
if (
196+
await cliServer.cliConstraints.supportsGenerateExtensiblePredicateMetadata()
197+
) {
198+
const metadata = await cliServer.generateExtensiblePredicateMetadata(
199+
originalPackRoot,
200+
);
201+
metadata.extensible_predicates.forEach((predicate) => {
202+
if (predicate.path.endsWith(".ql")) {
203+
toCopy.push(join(originalPackRoot, predicate.path));
204+
}
205+
});
206+
}
207+
192208
[
193209
// also copy the lock file (either new name or old name) and the query file itself. These are not included in the packlist.
194210
...QLPACK_LOCK_FILENAMES.map((f) => join(originalPackRoot, f)),

extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { CancellationTokenSource, commands, Uri, window } from "vscode";
22
import { extLogger } from "../../../../src/common/logging/vscode";
33
import { setRemoteControllerRepo } from "../../../../src/config";
44
import * as ghApiClient from "../../../../src/variant-analysis/gh-api/gh-api-client";
5-
import { join } from "path";
5+
import { isAbsolute, join } from "path";
66

77
import { VariantAnalysisManager } from "../../../../src/variant-analysis/variant-analysis-manager";
88
import {
@@ -275,20 +275,60 @@ describe("Variant Analysis Manager", () => {
275275
});
276276
});
277277

278+
// Test running core java queries to ensure that we can compile queries in packs
279+
// that contain queries with extensible predicates
280+
it("should run a remote query that is part of the java pack", async () => {
281+
if (
282+
!(await cli.cliConstraints.supportsGenerateExtensiblePredicateMetadata())
283+
) {
284+
console.log(
285+
`Skipping test because generating extensible predicate metadata was only introduced in CLI version ${CliVersionConstraint.CLI_VERSION_WITH_EXTENSIBLE_PREDICATE_METADATA}.`,
286+
);
287+
return;
288+
}
289+
290+
if (!process.env.TEST_CODEQL_PATH) {
291+
fail(
292+
"TEST_CODEQL_PATH environment variable not set. It should point to the absolute path to a checkout of the codeql repository.",
293+
);
294+
}
295+
296+
const queryToRun =
297+
"Security/CWE/CWE-020/ExternalAPIsUsedWithUntrustedData.ql";
298+
const extraQuery = "Telemetry/ExtractorInformation.ql";
299+
300+
await doVariantAnalysisTest({
301+
queryPath: join(
302+
process.env.TEST_CODEQL_PATH,
303+
"java/ql/src",
304+
queryToRun,
305+
),
306+
expectedPackName: "codeql/java-queries",
307+
filesThatExist: [queryToRun, extraQuery],
308+
filesThatDoNotExist: [],
309+
qlxFilesThatExist: [],
310+
dependenciesToCheck: ["codeql/java-all"],
311+
// Don't check the version since it will be the same version
312+
checkVersion: false,
313+
});
314+
});
315+
278316
async function doVariantAnalysisTest({
279317
queryPath,
280318
expectedPackName,
281319
filesThatExist,
282320
qlxFilesThatExist,
283321
filesThatDoNotExist,
284-
dependenciesToCheck = ["codeql/javascript-all"],
322+
dependenciesToCheck = ["codeql/java-all"],
323+
checkVersion = true,
285324
}: {
286325
queryPath: string;
287326
expectedPackName: string;
288327
filesThatExist: string[];
289328
qlxFilesThatExist: string[];
290329
filesThatDoNotExist: string[];
291330
dependenciesToCheck?: string[];
331+
checkVersion?: boolean;
292332
}) {
293333
const fileUri = getFile(queryPath);
294334
await variantAnalysisManager.runVariantAnalysis(
@@ -339,7 +379,9 @@ describe("Variant Analysis Manager", () => {
339379
packFS.fileContents(packFileName).toString("utf-8"),
340380
);
341381
expect(qlpackContents.name).toEqual(expectedPackName);
342-
expect(qlpackContents.version).toEqual("0.0.0");
382+
if (checkVersion) {
383+
expect(qlpackContents.version).toEqual("0.0.0");
384+
}
343385
expect(qlpackContents.dependencies?.["codeql/javascript-all"]).toEqual(
344386
"*",
345387
);
@@ -357,7 +399,11 @@ describe("Variant Analysis Manager", () => {
357399
}
358400

359401
function getFile(file: string): Uri {
360-
return Uri.file(join(baseDir, file));
402+
if (isAbsolute(file)) {
403+
return Uri.file(file);
404+
} else {
405+
return Uri.file(join(baseDir, file));
406+
}
361407
}
362408
});
363409
});

0 commit comments

Comments
 (0)