diff --git a/lib/entry-points.js b/lib/entry-points.js index 11e1aef19..ab220e392 100644 --- a/lib/entry-points.js +++ b/lib/entry-points.js @@ -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, diff --git a/src/config-utils.test.ts b/src/config-utils.test.ts index 29d72f3af..3ea71ebc4 100644 --- a/src/config-utils.test.ts +++ b/src/config-utils.test.ts @@ -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" }], diff --git a/src/config-utils.ts b/src/config-utils.ts index 4dd14b290..ba15138dc 100644 --- a/src/config-utils.ts +++ b/src/config-utils.ts @@ -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 { + 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 { - 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; } } diff --git a/src/config/db-config.test.ts b/src/config/db-config.test.ts index ecf77194a..76fa91801 100644 --- a/src/config/db-config.test.ts +++ b/src/config/db-config.test.ts @@ -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, + ); } }); diff --git a/src/config/db-config.ts b/src/config/db-config.ts index 977da17c7..e7924bf02 100644 --- a/src/config/db-config.ts +++ b/src/config/db-config.ts @@ -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); + // `valid` doesn't account for unknown properties. return result.valid && result.unknownKeys.length === 0; } diff --git a/src/init-action.ts b/src/init-action.ts index e1ac5df5e..2b3c95e0b 100644 --- a/src/init-action.ts +++ b/src/init-action.ts @@ -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, diff --git a/src/per-language-bundles.test.ts b/src/per-language-bundles.test.ts index c94b8d6db..09500d2f7 100644 --- a/src/per-language-bundles.test.ts +++ b/src/per-language-bundles.test.ts @@ -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", ); diff --git a/src/per-language-bundles.ts b/src/per-language-bundles.ts index 308fdcccc..3a481c244 100644 --- a/src/per-language-bundles.ts +++ b/src/per-language-bundles.ts @@ -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. */