Parse the config input once

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Henry Mercer
2026-10-02 14:29:22 +01:00
parent e869836b5a
commit 113b18e688
8 changed files with 175 additions and 90 deletions

43
lib/entry-points.js generated
View File

@@ -149271,16 +149271,7 @@ var DEFAULT_SETUP_CONFIG_SCHEMA = {
function checkDefaultSetupConfig(config) {
return checkSchema(DEFAULT_SETUP_CONFIG_SCHEMA, config);
}
function matchesDefaultSetupConfigSchema(contents) {
let config;
try {
config = load(contents);
} catch (error3) {
if (error3 instanceof YAMLException) {
return false;
}
throw error3;
}
function matchesDefaultSetupConfigSchema(config) {
if (!isObject(config)) {
return false;
}
@@ -150864,20 +150855,22 @@ async function applyIncrementalAnalysisSettings(config, hasDiffRanges, codeql, l
});
}
}
async function determineUserConfig(action, tempDir, inputs) {
const validateConfig = await action.features.getValue(
"validate_db_config" /* ValidateDbConfig */
async function parseConfigInput({ logger, features }, configInput) {
if (configInput === void 0) {
return void 0;
}
return parseUserConfig(
logger,
"`config` input",
configInput,
await features.getValue("validate_db_config" /* ValidateDbConfig */)
);
if (inputs.configInput) {
}
async function determineUserConfig(action, tempDir, inputs) {
if (inputs.configInput !== void 0) {
const computedConfigPath = userConfigFromActionPath(tempDir);
const allowMergeConfigs = () => action.features.getValue("allow_merge_config_files" /* AllowMergeConfigFiles */);
if (inputs.configFile && isDefaultSetup(action.env) && await allowMergeConfigs()) {
const fromConfigInput = parseUserConfig(
action.logger,
"`config` input",
inputs.configInput,
validateConfig
);
const fromConfigFile = await loadUserConfig(
action,
inputs.configFile,
@@ -150887,7 +150880,7 @@ async function determineUserConfig(action, tempDir, inputs) {
);
const mergedConfig = mergeDefaultSetupAndUserConfigs(
action.logger,
fromConfigInput,
inputs.configInput,
fromConfigFile
);
fs10.writeFileSync(computedConfigPath, dump(mergedConfig));
@@ -150902,11 +150895,12 @@ async function determineUserConfig(action, tempDir, inputs) {
`Both a config file and config input were provided. Ignoring config file.`
);
}
fs10.writeFileSync(computedConfigPath, inputs.configInput);
fs10.writeFileSync(computedConfigPath, dump(inputs.configInput));
inputs.configFile = computedConfigPath;
action.logger.debug(
`Using config from action input: ${inputs.configFile}`
);
return inputs.configInput;
}
}
if (!inputs.configFile) {
@@ -162396,7 +162390,10 @@ async function run3(actionState) {
const rawLanguages = getRawLanguagesNoAutodetect(
getOptionalInput("languages")
);
const configInput = getOptionalInput("config");
const configInput = await parseConfigInput(
actionStateWithFeatures,
getOptionalInput("config")
);
const queriesInput = getOptionalInput("queries");
const otherLanguagePacksReason = getOtherLanguagePacksReason({
configFile,

View File

@@ -473,6 +473,9 @@ const simpleConfigFileContents = `
queries:
- uses: ./foo_file`;
/** The configuration in `simpleConfigFileContents`, as parsed from the `config` input. */
const simpleConfigInput = yaml.load(simpleConfigFileContents) as UserConfig;
/** A less minimal configuration file. */
const otherConfigFileContents = `
name: my config
@@ -591,7 +594,7 @@ test.serial(
createTestInitConfigInputs({
languagesInput,
configFile: configFilePath,
configInput,
configInput: yaml.load(configInput) as UserConfig,
tempDir,
codeql,
workspacePath: tempDir,
@@ -2343,6 +2346,40 @@ test("applyIncrementalAnalysisSettings: adds exclusions for diff-informed-only r
]);
});
test("parseConfigInput - returns undefined when the input isn't set", async (t) => {
await callee(configUtils.parseConfigInput)
.withArgs(undefined)
.passes(t.is, undefined);
});
test("parseConfigInput - parses the input as YAML", async (t) => {
await callee(configUtils.parseConfigInput)
.withArgs(simpleConfigFileContents)
.passes(t.deepEqual, {
name: "my config",
queries: [{ uses: "./foo_file" }],
});
});
test("parseConfigInput - throws a ConfigurationError naming the input if it isn't valid YAML", async (t) => {
await callee(configUtils.parseConfigInput)
.withArgs("queries: [")
.throws(t, {
instanceOf: ConfigurationError,
message: /^Cannot parse "`config` input"/,
});
});
test("parseConfigInput - throws a ConfigurationError naming the input if validation fails", async (t) => {
await callee(configUtils.parseConfigInput)
.withFeatures([Feature.ValidateDbConfig])
.withArgs("queries: 1")
.throws(t, {
instanceOf: ConfigurationError,
message: /^The configuration file "`config` input" is invalid/,
});
});
test("determineUserConfig - empty config when neither input is specified", async (t) => {
await withTmpDir(async (tmpDir) => {
const target = callee(configUtils.determineUserConfig)
@@ -2413,7 +2450,7 @@ test("determineUserConfig - loads config input", async (t) => {
const expectedConfigPath = configUtils.userConfigFromActionPath(tmpDir);
const inputs = createTestInitConfigInputs({
configInput: simpleConfigFileContents,
configInput: simpleConfigInput,
configFile: undefined,
workspacePath: tmpDir,
});
@@ -2423,17 +2460,15 @@ test("determineUserConfig - loads config input", async (t) => {
await target
// The input source and path of the generated config file should have been logged.
.logs(
t,
"Using config from action input:",
`Using configuration file: ${expectedConfigPath}`,
)
// The message about no configuration input and
// the warning about both inputs should not have been logged.
.logs(t, `Using config from action input: ${expectedConfigPath}`)
// The message about no configuration input and the warning about both inputs should not have
// been logged. The generated config file isn't loaded, since the `config` input has already
// been parsed.
.notLogs(
t,
"No configuration file was provided",
"Both a config file and config input were provided. Ignoring config file.",
`Using configuration file: ${expectedConfigPath}`,
)
// The loaded configuration should match `simpleConfigFileContents`.
.passes(t.deepEqual, {
@@ -2452,7 +2487,7 @@ test("determineUserConfig - ignores config file input when both specified", asyn
const expectedConfigPath = configUtils.userConfigFromActionPath(tmpDir);
const inputs = createTestInitConfigInputs({
configInput: simpleConfigFileContents,
configInput: simpleConfigInput,
configFile: configFilePath,
workspacePath: tmpDir,
});
@@ -2466,10 +2501,14 @@ test("determineUserConfig - ignores config file input when both specified", asyn
.logs(
t,
`Using config from action input: ${expectedConfigPath}`,
`Using configuration file: ${expectedConfigPath}`,
"Both a config file and config input were provided. Ignoring config file.",
)
.notLogs(t, "No configuration file was provided")
// The generated config file isn't loaded, since the `config` input has already been parsed.
.notLogs(
t,
"No configuration file was provided",
`Using configuration file: ${expectedConfigPath}`,
)
// The loaded configuration should match `simpleConfigFileContents`.
.passes(t.deepEqual, {
name: "my config",
@@ -2481,12 +2520,12 @@ test("determineUserConfig - ignores config file input when both specified", asyn
});
});
/** A `config` input that we might get from Default Setup. */
const defaultSetupConfigInput = `
/** The configuration from a `config` input that we might get from Default Setup. */
const defaultSetupConfigInput = yaml.load(`
threat-models: [local, remote]
default-setup:
org:
model-packs: [foo, bar]`;
model-packs: [foo, bar]`) as UserConfig;
test("determineUserConfig - merges configs if FF is enabled in Default Setup", async (t) => {
await withTmpDir(async (tmpDir) => {
@@ -2555,7 +2594,7 @@ test("determineUserConfig - ignores config file input in Default Setup if FF is
.withArgs(
tmpDir,
createTestInitConfigInputs({
configInput: simpleConfigFileContents,
configInput: simpleConfigInput,
configFile: configFilePath,
workspacePath: tmpDir,
}),
@@ -2565,10 +2604,14 @@ test("determineUserConfig - ignores config file input in Default Setup if FF is
.logs(
t,
`Using config from action input: ${expectedConfigPath}`,
`Using configuration file: ${expectedConfigPath}`,
"Both a config file and config input were provided. Ignoring config file.",
)
.notLogs(t, "No configuration file was provided")
// The generated config file isn't loaded, since the `config` input has already been parsed.
.notLogs(
t,
"No configuration file was provided",
`Using configuration file: ${expectedConfigPath}`,
)
.passes(t.deepEqual, {
name: "my config",
queries: [{ uses: "./foo_file" }],
@@ -2587,7 +2630,7 @@ test("determineUserConfig - ignores config file input outside Default Setup if F
.withArgs(
tmpDir,
createTestInitConfigInputs({
configInput: simpleConfigFileContents,
configInput: simpleConfigInput,
configFile: configFilePath,
workspacePath: tmpDir,
}),
@@ -2597,10 +2640,14 @@ test("determineUserConfig - ignores config file input outside Default Setup if F
.logs(
t,
`Using config from action input: ${expectedConfigPath}`,
`Using configuration file: ${expectedConfigPath}`,
"Both a config file and config input were provided. Ignoring config file.",
)
.notLogs(t, "No configuration file was provided")
// The generated config file isn't loaded, since the `config` input has already been parsed.
.notLogs(
t,
"No configuration file was provided",
`Using configuration file: ${expectedConfigPath}`,
)
.passes(t.deepEqual, {
name: "my config",
queries: [{ uses: "./foo_file" }],

View File

@@ -337,7 +337,8 @@ export interface InitConfigInputs {
packsInput: string | undefined;
configFile: string | undefined;
dbLocation: string | undefined;
configInput: string | undefined;
/** The configuration from the `config` input. */
configInput: UserConfig | undefined;
buildModeInput: string | undefined;
ramInput: string | undefined;
dependencyCachingEnabled: string | undefined;
@@ -1043,6 +1044,29 @@ export async function applyIncrementalAnalysisSettings(
}
}
/**
* Parses the `config` input, which contains a configuration in YAML.
*
* @returns The configuration, or `undefined` if the input isn't set. Unless configuration validation
* is enabled, the configuration might not be a mapping.
* @throws A `ConfigurationError` if the input isn't valid YAML or, when configuration validation is
* enabled, isn't a valid configuration.
*/
export async function parseConfigInput(
{ logger, features }: ActionState<["Logger", "FeatureFlags"]>,
configInput: string | undefined,
): Promise<UserConfig | undefined> {
if (configInput === undefined) {
return undefined;
}
return parseUserConfig(
logger,
"`config` input",
configInput,
await features.getValue(Feature.ValidateDbConfig),
);
}
/**
* Determines where to load the `UserConfig` for the CLI from and loads it.
*
@@ -1057,17 +1081,13 @@ export async function determineUserConfig(
tempDir: string,
inputs: InitConfigInputs,
): Promise<UserConfig> {
const validateConfig = await action.features.getValue(
Feature.ValidateDbConfig,
);
// We have the following cases:
// 1. A `config` or `config-file` input is provided, but not both: use the provided one.
// 2. Both are provided and we are in an advanced workflow: ignore the `config-file` input.
// 3. Both are provided and we are in Default Setup: the `config` input uses a limited
// set of options, which are supported by `mergeDefaultSetupAndUserConfigs`,
// and we merge the two configs.
if (inputs.configInput) {
if (inputs.configInput !== undefined) {
const computedConfigPath = userConfigFromActionPath(tempDir);
// Get a function which enables us to determine whether the FF that allows us to
@@ -1084,12 +1104,6 @@ export async function determineUserConfig(
) {
// If the FF is enabled and we are in Default Setup, combine the supported
// configuration file properties and write the result to disk.
const fromConfigInput = parseUserConfig(
action.logger,
"`config` input",
inputs.configInput,
validateConfig,
);
const fromConfigFile = await loadUserConfig(
action,
inputs.configFile,
@@ -1102,7 +1116,7 @@ export async function determineUserConfig(
// the CLI or other CodeQL Action steps.
const mergedConfig = mergeDefaultSetupAndUserConfigs(
action.logger,
fromConfigInput,
inputs.configInput,
fromConfigFile,
);
fs.writeFileSync(computedConfigPath, yaml.dump(mergedConfig));
@@ -1122,12 +1136,13 @@ export async function determineUserConfig(
);
}
// Write the `config` input straight to disk.
fs.writeFileSync(computedConfigPath, inputs.configInput);
// Write the `config` input to disk.
fs.writeFileSync(computedConfigPath, yaml.dump(inputs.configInput));
inputs.configFile = computedConfigPath;
action.logger.debug(
`Using config from action input: ${inputs.configFile}`,
);
return inputs.configInput;
}
}

View File

@@ -626,6 +626,16 @@ test("mergeDefaultSetupAndUserConfigs - warns about invalid keys from Default Se
]);
});
/** Parses `contents` as a configuration without validating it. */
function parseUnvalidatedConfig(contents: string): dbConfig.UserConfig {
return dbConfig.parseUserConfig(
getRunnerLogger(true),
"test",
contents,
false,
);
}
test("matchesDefaultSetupConfigSchema - returns true for configurations that only use Default Setup properties", (t) => {
for (const contents of [
[
@@ -637,7 +647,12 @@ test("matchesDefaultSetupConfigSchema - returns true for configurations that onl
"threat-models: [ local ]",
"{}",
]) {
t.true(dbConfig.matchesDefaultSetupConfigSchema(contents), contents);
t.true(
dbConfig.matchesDefaultSetupConfigSchema(
parseUnvalidatedConfig(contents),
),
contents,
);
}
});
@@ -647,7 +662,12 @@ test("matchesDefaultSetupConfigSchema - returns false for configurations that us
"paths-ignore: [ tests ]",
"default-setup: { org: { model-packs: [], queries: [] } }",
]) {
t.false(dbConfig.matchesDefaultSetupConfigSchema(contents), contents);
t.false(
dbConfig.matchesDefaultSetupConfigSchema(
parseUnvalidatedConfig(contents),
),
contents,
);
}
});
@@ -656,12 +676,22 @@ test("matchesDefaultSetupConfigSchema - returns false for invalid Default Setup
"threat-models: local",
"default-setup: { org: { model-packs: [ 1 ] } }",
]) {
t.false(dbConfig.matchesDefaultSetupConfigSchema(contents), contents);
t.false(
dbConfig.matchesDefaultSetupConfigSchema(
parseUnvalidatedConfig(contents),
),
contents,
);
}
});
test("matchesDefaultSetupConfigSchema - returns false for contents that aren't a YAML object", (t) => {
for (const contents of ["threat-models: [", "- threat-models", "local", ""]) {
t.false(dbConfig.matchesDefaultSetupConfigSchema(contents), contents);
test("matchesDefaultSetupConfigSchema - returns false for configurations that aren't mappings", (t) => {
for (const contents of ["- threat-models", "local", "null"]) {
t.false(
dbConfig.matchesDefaultSetupConfigSchema(
parseUnvalidatedConfig(contents),
),
contents,
);
}
});

View File

@@ -106,24 +106,16 @@ function checkDefaultSetupConfig(
}
/**
* Returns whether `contents` is a YAML mapping that only sets the properties that Default Setup
* uses, which are threat models and model packs, to valid values. Returns `false` otherwise,
* including if `contents` isn't valid YAML.
* Returns whether `config` is a mapping that only sets the properties that Default Setup sets in
* the `config` input, to valid values.
*/
export function matchesDefaultSetupConfigSchema(contents: string): boolean {
let config: unknown;
try {
config = yaml.load(contents);
} catch (error) {
if (error instanceof yaml.YAMLException) {
return false;
}
throw error;
}
export function matchesDefaultSetupConfigSchema(config: UserConfig): boolean {
// Unless validation is enabled, `parseUserConfig` doesn't check that the YAML is a mapping.
if (!json.isObject(config)) {
return false;
}
const result = checkDefaultSetupConfig(config);
const result = checkDefaultSetupConfig(config as json.UnvalidatedObject<any>);
// `valid` doesn't account for unknown properties.
return result.valid && result.unknownKeys.length === 0;
}

View File

@@ -306,7 +306,10 @@ async function run(
const rawLanguages = configUtils.getRawLanguagesNoAutodetect(
getOptionalInput("languages"),
);
const configInput = getOptionalInput("config");
const configInput = await configUtils.parseConfigInput(
actionStateWithFeatures,
getOptionalInput("config"),
);
const queriesInput = getOptionalInput("queries");
const otherLanguagePacksReason = getOtherLanguagePacksReason({
configFile,

View File

@@ -227,13 +227,13 @@ test("getOtherLanguagePacksReason returns undefined for a config input that only
t.is(
getOtherLanguagePacksReason({
...NO_QUERY_CONFIG,
// The shape of the `config` input that default setup passes.
configInput: [
"default-setup:",
" org:",
" model-packs: [ github/immutable-actions-list@0.0.1 ]",
"threat-models: [ ]",
].join("\n"),
// The configuration from the `config` input that default setup passes.
configInput: {
"default-setup": {
org: { "model-packs": ["github/immutable-actions-list@0.0.1"] },
},
"threat-models": [],
},
}),
undefined,
);
@@ -254,7 +254,7 @@ test("getOtherLanguagePacksReason explains a config input that uses other proper
t.is(
getOtherLanguagePacksReason({
...NO_QUERY_CONFIG,
configInput: "queries: [ { uses: ./queries/show_ifs.ql } ]",
configInput: { queries: [{ uses: "./queries/show_ifs.ql" }] },
}),
"the 'config' input may use queries that need library packs for other languages",
);

View File

@@ -7,6 +7,7 @@ import {
matchesDefaultSetupConfigSchema,
parseQueriesFromInput,
QuerySpec,
UserConfig,
} from "./config/db-config";
import { Feature } from "./feature-flags";
import { RepositoryPropertyName } from "./feature-flags/properties";
@@ -42,8 +43,8 @@ const PER_LANGUAGE_BUNDLE_LANGUAGES: Readonly<
export interface QueryConfigInputs {
/** The configuration file from the `config-file` input or repository property. */
configFile: string | undefined;
/** The `config` input. */
configInput: string | undefined;
/** The configuration from the `config` input. */
configInput: UserConfig | undefined;
/** The `queries` input. */
queriesInput: string | undefined;
/** The `github-codeql-extra-queries` repository property. */