Merge pull request #2938 from github/cklin/default-query-filters

Keep user-provided query filters first
This commit is contained in:
Chuan-kai Lin 2025-06-19 13:46:30 -07:00 committed by GitHub
commit 66d72553a2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
13 changed files with 74 additions and 115 deletions

View file

@ -4,7 +4,7 @@ See the [releases page](https://github.com/github/codeql-action/releases) for th
## [UNRELEASED]
No user facing changes.
- Fix bug in PR analysis where user-provided `include` query filter fails to exclude non-included queries. [#2938](https://github.com/github/codeql-action/pull/2938)
## 3.29.0 - 11 Jun 2025

6
lib/codeql.js generated
View file

@ -797,8 +797,12 @@ async function generateCodeScanningConfig(config, logger) {
delete augmentedConfig.packs;
}
augmentedConfig["query-filters"] = [
...(config.augmentationProperties.defaultQueryFilters || []),
// Ordering matters. If the first filter is an inclusion, it implicitly
// excludes all queries that are not included. If it is an exclusion,
// it implicitly includes all queries that are not excluded. So user
// filters (if any) should always be first to preserve intent.
...(augmentedConfig["query-filters"] || []),
...(config.augmentationProperties.extraQueryExclusions || []),
];
if (augmentedConfig["query-filters"]?.length === 0) {
delete augmentedConfig["query-filters"];

File diff suppressed because one or more lines are too long

22
lib/codeql.test.js generated
View file

@ -49,6 +49,7 @@ const sinon = __importStar(require("sinon"));
const actionsUtil = __importStar(require("./actions-util"));
const cli_errors_1 = require("./cli-errors");
const codeql = __importStar(require("./codeql"));
const config_utils_1 = require("./config-utils");
const defaults = __importStar(require("./defaults.json"));
const doc_url_1 = require("./doc-url");
const languages_1 = require("./languages");
@ -350,18 +351,16 @@ const injectedConfigMacro = ava_1.default.macro({
title: (providedTitle = "") => `databaseInitCluster() injected config: ${providedTitle}`,
});
(0, ava_1.default)("basic", injectedConfigMacro, {
queriesInputCombines: false,
packsInputCombines: false,
...config_utils_1.defaultAugmentationProperties,
}, {}, {});
(0, ava_1.default)("injected packs from input", injectedConfigMacro, {
queriesInputCombines: false,
packsInputCombines: false,
...config_utils_1.defaultAugmentationProperties,
packsInput: ["xxx", "yyy"],
}, {}, {
packs: ["xxx", "yyy"],
});
(0, ava_1.default)("injected packs from input with existing packs combines", injectedConfigMacro, {
queriesInputCombines: false,
...config_utils_1.defaultAugmentationProperties,
packsInputCombines: true,
packsInput: ["xxx", "yyy"],
}, {
@ -376,8 +375,7 @@ const injectedConfigMacro = ava_1.default.macro({
},
});
(0, ava_1.default)("injected packs from input with existing packs overrides", injectedConfigMacro, {
queriesInputCombines: false,
packsInputCombines: false,
...config_utils_1.defaultAugmentationProperties,
packsInput: ["xxx", "yyy"],
}, {
originalUserInput: {
@ -390,8 +388,7 @@ const injectedConfigMacro = ava_1.default.macro({
});
// similar, but with queries
(0, ava_1.default)("injected queries from input", injectedConfigMacro, {
queriesInputCombines: false,
packsInputCombines: false,
...config_utils_1.defaultAugmentationProperties,
queriesInput: [{ uses: "xxx" }, { uses: "yyy" }],
}, {}, {
queries: [
@ -404,8 +401,7 @@ const injectedConfigMacro = ava_1.default.macro({
],
});
(0, ava_1.default)("injected queries from input overrides", injectedConfigMacro, {
queriesInputCombines: false,
packsInputCombines: false,
...config_utils_1.defaultAugmentationProperties,
queriesInput: [{ uses: "xxx" }, { uses: "yyy" }],
}, {
originalUserInput: {
@ -422,8 +418,8 @@ const injectedConfigMacro = ava_1.default.macro({
],
});
(0, ava_1.default)("injected queries from input combines", injectedConfigMacro, {
...config_utils_1.defaultAugmentationProperties,
queriesInputCombines: true,
packsInputCombines: false,
queriesInput: [{ uses: "xxx" }, { uses: "yyy" }],
}, {
originalUserInput: {
@ -443,6 +439,7 @@ const injectedConfigMacro = ava_1.default.macro({
],
});
(0, ava_1.default)("injected queries from input combines 2", injectedConfigMacro, {
...config_utils_1.defaultAugmentationProperties,
queriesInputCombines: true,
packsInputCombines: true,
queriesInput: [{ uses: "xxx" }, { uses: "yyy" }],
@ -457,6 +454,7 @@ const injectedConfigMacro = ava_1.default.macro({
],
});
(0, ava_1.default)("injected queries and packs, but empty", injectedConfigMacro, {
...config_utils_1.defaultAugmentationProperties,
queriesInputCombines: true,
packsInputCombines: true,
queriesInput: [],

File diff suppressed because one or more lines are too long

10
lib/config-utils.js generated
View file

@ -81,7 +81,7 @@ exports.defaultAugmentationProperties = {
packsInput: undefined,
queriesInput: undefined,
qualityQueriesInput: undefined,
defaultQueryFilters: [],
extraQueryExclusions: [],
};
function getPacksStrInvalid(packStr, configFile) {
return configFile
@ -327,9 +327,11 @@ async function calculateAugmentation(codeql, features, rawPacksInput, rawQueries
const queriesInputCombines = shouldCombine(rawQueriesInput);
const queriesInput = parseQueriesFromInput(rawQueriesInput, queriesInputCombines);
const qualityQueriesInput = parseQueriesFromInput(rawQualityQueriesInput, false);
const defaultQueryFilters = [];
const extraQueryExclusions = [];
if (await (0, diff_informed_analysis_utils_1.shouldPerformDiffInformedAnalysis)(codeql, features, logger)) {
defaultQueryFilters.push({ exclude: { tags: "exclude-from-incremental" } });
extraQueryExclusions.push({
exclude: { tags: "exclude-from-incremental" },
});
}
return {
packsInputCombines,
@ -337,7 +339,7 @@ async function calculateAugmentation(codeql, features, rawPacksInput, rawQueries
queriesInput,
queriesInputCombines,
qualityQueriesInput,
defaultQueryFilters,
extraQueryExclusions,
};
}
function parseQueriesFromInput(rawQueriesInput, queriesInputCombines) {

File diff suppressed because one or more lines are too long

View file

@ -631,44 +631,28 @@ const calculateAugmentationMacro = ava_1.default.macro({
title: (_, title) => `Calculate Augmentation: ${title}`,
});
(0, ava_1.default)(calculateAugmentationMacro, "All empty", undefined, undefined, undefined, [languages_1.Language.javascript], {
queriesInputCombines: false,
queriesInput: undefined,
qualityQueriesInput: undefined,
packsInputCombines: false,
packsInput: undefined,
defaultQueryFilters: [],
...configUtils.defaultAugmentationProperties,
});
(0, ava_1.default)(calculateAugmentationMacro, "With queries", undefined, " a, b , c, d", undefined, [languages_1.Language.javascript], {
queriesInputCombines: false,
...configUtils.defaultAugmentationProperties,
queriesInput: [{ uses: "a" }, { uses: "b" }, { uses: "c" }, { uses: "d" }],
qualityQueriesInput: undefined,
packsInputCombines: false,
packsInput: undefined,
defaultQueryFilters: [],
});
(0, ava_1.default)(calculateAugmentationMacro, "With queries combining", undefined, " + a, b , c, d ", undefined, [languages_1.Language.javascript], {
...configUtils.defaultAugmentationProperties,
queriesInputCombines: true,
queriesInput: [{ uses: "a" }, { uses: "b" }, { uses: "c" }, { uses: "d" }],
qualityQueriesInput: undefined,
packsInputCombines: false,
packsInput: undefined,
defaultQueryFilters: [],
});
(0, ava_1.default)(calculateAugmentationMacro, "With quality queries", undefined, undefined, " a, b , c, d", [languages_1.Language.javascript], {
queriesInputCombines: false,
queriesInput: undefined,
...configUtils.defaultAugmentationProperties,
qualityQueriesInput: [
{ uses: "a" },
{ uses: "b" },
{ uses: "c" },
{ uses: "d" },
],
packsInputCombines: false,
packsInput: undefined,
defaultQueryFilters: [],
});
(0, ava_1.default)(calculateAugmentationMacro, "With security and quality queries", undefined, " a, b , c, d", "e, f , g,h", [languages_1.Language.javascript], {
queriesInputCombines: false,
...configUtils.defaultAugmentationProperties,
queriesInput: [{ uses: "a" }, { uses: "b" }, { uses: "c" }, { uses: "d" }],
qualityQueriesInput: [
{ uses: "e" },
@ -676,25 +660,15 @@ const calculateAugmentationMacro = ava_1.default.macro({
{ uses: "g" },
{ uses: "h" },
],
packsInputCombines: false,
packsInput: undefined,
defaultQueryFilters: [],
});
(0, ava_1.default)(calculateAugmentationMacro, "With packs", " codeql/a , codeql/b , codeql/c , codeql/d ", undefined, undefined, [languages_1.Language.javascript], {
queriesInputCombines: false,
queriesInput: undefined,
qualityQueriesInput: undefined,
packsInputCombines: false,
...configUtils.defaultAugmentationProperties,
packsInput: ["codeql/a", "codeql/b", "codeql/c", "codeql/d"],
defaultQueryFilters: [],
});
(0, ava_1.default)(calculateAugmentationMacro, "With packs combining", " + codeql/a, codeql/b, codeql/c, codeql/d", undefined, undefined, [languages_1.Language.javascript], {
queriesInputCombines: false,
queriesInput: undefined,
qualityQueriesInput: undefined,
...configUtils.defaultAugmentationProperties,
packsInputCombines: true,
packsInput: ["codeql/a", "codeql/b", "codeql/c", "codeql/d"],
defaultQueryFilters: [],
});
const calculateAugmentationErrorMacro = ava_1.default.macro({
exec: async (t, _title, rawPacksInput, rawQueriesInput, rawQualityQueriesInput, languages, expectedError) => {

File diff suppressed because one or more lines are too long

View file

@ -14,7 +14,11 @@ import * as actionsUtil from "./actions-util";
import { GitHubApiDetails } from "./api-client";
import { CliError } from "./cli-errors";
import * as codeql from "./codeql";
import { AugmentationProperties, Config } from "./config-utils";
import {
AugmentationProperties,
Config,
defaultAugmentationProperties,
} from "./config-utils";
import * as defaults from "./defaults.json";
import { DocUrl } from "./doc-url";
import { FeatureEnablement } from "./feature-flags";
@ -537,8 +541,7 @@ test(
"basic",
injectedConfigMacro,
{
queriesInputCombines: false,
packsInputCombines: false,
...defaultAugmentationProperties,
},
{},
{},
@ -548,8 +551,7 @@ test(
"injected packs from input",
injectedConfigMacro,
{
queriesInputCombines: false,
packsInputCombines: false,
...defaultAugmentationProperties,
packsInput: ["xxx", "yyy"],
},
{},
@ -562,7 +564,7 @@ test(
"injected packs from input with existing packs combines",
injectedConfigMacro,
{
queriesInputCombines: false,
...defaultAugmentationProperties,
packsInputCombines: true,
packsInput: ["xxx", "yyy"],
},
@ -584,8 +586,7 @@ test(
"injected packs from input with existing packs overrides",
injectedConfigMacro,
{
queriesInputCombines: false,
packsInputCombines: false,
...defaultAugmentationProperties,
packsInput: ["xxx", "yyy"],
},
{
@ -605,8 +606,7 @@ test(
"injected queries from input",
injectedConfigMacro,
{
queriesInputCombines: false,
packsInputCombines: false,
...defaultAugmentationProperties,
queriesInput: [{ uses: "xxx" }, { uses: "yyy" }],
},
{},
@ -626,8 +626,7 @@ test(
"injected queries from input overrides",
injectedConfigMacro,
{
queriesInputCombines: false,
packsInputCombines: false,
...defaultAugmentationProperties,
queriesInput: [{ uses: "xxx" }, { uses: "yyy" }],
},
{
@ -651,8 +650,8 @@ test(
"injected queries from input combines",
injectedConfigMacro,
{
...defaultAugmentationProperties,
queriesInputCombines: true,
packsInputCombines: false,
queriesInput: [{ uses: "xxx" }, { uses: "yyy" }],
},
{
@ -679,6 +678,7 @@ test(
"injected queries from input combines 2",
injectedConfigMacro,
{
...defaultAugmentationProperties,
queriesInputCombines: true,
packsInputCombines: true,
queriesInput: [{ uses: "xxx" }, { uses: "yyy" }],
@ -700,6 +700,7 @@ test(
"injected queries and packs, but empty",
injectedConfigMacro,
{
...defaultAugmentationProperties,
queriesInputCombines: true,
packsInputCombines: true,
queriesInput: [],

View file

@ -1266,8 +1266,12 @@ async function generateCodeScanningConfig(
}
augmentedConfig["query-filters"] = [
...(config.augmentationProperties.defaultQueryFilters || []),
// Ordering matters. If the first filter is an inclusion, it implicitly
// excludes all queries that are not included. If it is an exclusion,
// it implicitly includes all queries that are not excluded. So user
// filters (if any) should always be first to preserve intent.
...(augmentedConfig["query-filters"] || []),
...(config.augmentationProperties.extraQueryExclusions || []),
];
if (augmentedConfig["query-filters"]?.length === 0) {
delete augmentedConfig["query-filters"];

View file

@ -834,13 +834,8 @@ test(
undefined,
[Language.javascript],
{
queriesInputCombines: false,
queriesInput: undefined,
qualityQueriesInput: undefined,
packsInputCombines: false,
packsInput: undefined,
defaultQueryFilters: [],
} as configUtils.AugmentationProperties,
...configUtils.defaultAugmentationProperties,
},
);
test(
@ -851,13 +846,9 @@ test(
undefined,
[Language.javascript],
{
queriesInputCombines: false,
...configUtils.defaultAugmentationProperties,
queriesInput: [{ uses: "a" }, { uses: "b" }, { uses: "c" }, { uses: "d" }],
qualityQueriesInput: undefined,
packsInputCombines: false,
packsInput: undefined,
defaultQueryFilters: [],
} as configUtils.AugmentationProperties,
},
);
test(
@ -868,13 +859,10 @@ test(
undefined,
[Language.javascript],
{
...configUtils.defaultAugmentationProperties,
queriesInputCombines: true,
queriesInput: [{ uses: "a" }, { uses: "b" }, { uses: "c" }, { uses: "d" }],
qualityQueriesInput: undefined,
packsInputCombines: false,
packsInput: undefined,
defaultQueryFilters: [],
} as configUtils.AugmentationProperties,
},
);
test(
@ -885,18 +873,14 @@ test(
" a, b , c, d",
[Language.javascript],
{
queriesInputCombines: false,
queriesInput: undefined,
...configUtils.defaultAugmentationProperties,
qualityQueriesInput: [
{ uses: "a" },
{ uses: "b" },
{ uses: "c" },
{ uses: "d" },
],
packsInputCombines: false,
packsInput: undefined,
defaultQueryFilters: [],
} as configUtils.AugmentationProperties,
},
);
test(
@ -907,7 +891,7 @@ test(
"e, f , g,h",
[Language.javascript],
{
queriesInputCombines: false,
...configUtils.defaultAugmentationProperties,
queriesInput: [{ uses: "a" }, { uses: "b" }, { uses: "c" }, { uses: "d" }],
qualityQueriesInput: [
{ uses: "e" },
@ -915,10 +899,7 @@ test(
{ uses: "g" },
{ uses: "h" },
],
packsInputCombines: false,
packsInput: undefined,
defaultQueryFilters: [],
} as configUtils.AugmentationProperties,
},
);
test(
@ -929,13 +910,9 @@ test(
undefined,
[Language.javascript],
{
queriesInputCombines: false,
queriesInput: undefined,
qualityQueriesInput: undefined,
packsInputCombines: false,
...configUtils.defaultAugmentationProperties,
packsInput: ["codeql/a", "codeql/b", "codeql/c", "codeql/d"],
defaultQueryFilters: [],
} as configUtils.AugmentationProperties,
},
);
test(
@ -946,13 +923,10 @@ test(
undefined,
[Language.javascript],
{
queriesInputCombines: false,
queriesInput: undefined,
qualityQueriesInput: undefined,
...configUtils.defaultAugmentationProperties,
packsInputCombines: true,
packsInput: ["codeql/a", "codeql/b", "codeql/c", "codeql/d"],
defaultQueryFilters: [],
} as configUtils.AugmentationProperties,
},
);
const calculateAugmentationErrorMacro = test.macro({

View file

@ -186,9 +186,9 @@ export interface AugmentationProperties {
packsInput?: string[];
/**
* Default query filters to apply to the queries in the config.
* Extra query exclusions to append to the config.
*/
defaultQueryFilters?: QueryFilter[];
extraQueryExclusions?: ExcludeQueryFilter[];
}
/**
@ -201,7 +201,7 @@ export const defaultAugmentationProperties: AugmentationProperties = {
packsInput: undefined,
queriesInput: undefined,
qualityQueriesInput: undefined,
defaultQueryFilters: [],
extraQueryExclusions: [],
};
export type Packs = Partial<Record<Language, string[]>>;
@ -671,9 +671,11 @@ export async function calculateAugmentation(
false,
);
const defaultQueryFilters: QueryFilter[] = [];
const extraQueryExclusions: ExcludeQueryFilter[] = [];
if (await shouldPerformDiffInformedAnalysis(codeql, features, logger)) {
defaultQueryFilters.push({ exclude: { tags: "exclude-from-incremental" } });
extraQueryExclusions.push({
exclude: { tags: "exclude-from-incremental" },
});
}
return {
@ -682,7 +684,7 @@ export async function calculateAugmentation(
queriesInput,
queriesInputCombines,
qualityQueriesInput,
defaultQueryFilters,
extraQueryExclusions,
};
}