diff --git a/CHANGELOG.md b/CHANGELOG.md index cac0f792e7..28a3105f87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ See the [releases page](https://github.com/github/codeql-action/releases) for th ## [UNRELEASED] -No user facing changes. +- Fixed [a bug](https://github.com/github/codeql-action/issues/3555) which caused the CodeQL Action to fail loading repository properties if a "Multi select" repository property was configured for the repository. [#3557](https://github.com/github/codeql-action/pull/3557) ## 4.32.6 - 05 Mar 2026 diff --git a/lib/init-action.js b/lib/init-action.js index a047977e27..b07f684e90 100644 --- a/lib/init-action.js +++ b/lib/init-action.js @@ -100091,7 +100091,7 @@ var require_follow_redirects = __commonJS({ if (this._ending) { throw new WriteAfterEndError(); } - if (!isString(data) && !isBuffer(data)) { + if (!isString2(data) && !isBuffer(data)) { throw new TypeError("data should be a string, Buffer or Uint8Array"); } if (isFunction(encoding)) { @@ -100346,7 +100346,7 @@ var require_follow_redirects = __commonJS({ function request2(input, options, callback) { if (isURL(input)) { input = spreadUrlObject(input); - } else if (isString(input)) { + } else if (isString2(input)) { input = spreadUrlObject(parseUrl2(input)); } else { callback = options; @@ -100362,7 +100362,7 @@ var require_follow_redirects = __commonJS({ maxBodyLength: exports3.maxBodyLength }, input, options); options.nativeProtocols = nativeProtocols; - if (!isString(options.host) && !isString(options.hostname)) { + if (!isString2(options.host) && !isString2(options.hostname)) { options.hostname = "::1"; } assert.equal(options.protocol, protocol, "protocol mismatch"); @@ -100389,7 +100389,7 @@ var require_follow_redirects = __commonJS({ parsed = new URL2(input); } else { parsed = validateUrl(url.parse(input)); - if (!isString(parsed.protocol)) { + if (!isString2(parsed.protocol)) { throw new InvalidUrlError({ input }); } } @@ -100461,11 +100461,11 @@ var require_follow_redirects = __commonJS({ request2.destroy(error3); } function isSubdomain(subdomain, domain) { - assert(isString(subdomain) && isString(domain)); + assert(isString2(subdomain) && isString2(domain)); var dot = subdomain.length - domain.length - 1; return dot > 0 && subdomain[dot] === "." && subdomain.endsWith(domain); } - function isString(value) { + function isString2(value) { return typeof value === "string" || value instanceof String; } function isFunction(value) { @@ -104408,9 +104408,21 @@ var RepositoryPropertyName = /* @__PURE__ */ ((RepositoryPropertyName2) => { RepositoryPropertyName2["EXTRA_QUERIES"] = "github-codeql-extra-queries"; return RepositoryPropertyName2; })(RepositoryPropertyName || {}); +function isString(value) { + return typeof value === "string"; +} +var stringProperty = { + validate: isString, + parse: parseStringRepositoryProperty +}; +var booleanProperty = { + // The value from the API should come as a string, which we then parse into a boolean. + validate: isString, + parse: parseBooleanRepositoryProperty +}; var repositoryPropertyParsers = { - ["github-codeql-disable-overlay" /* DISABLE_OVERLAY */]: parseBooleanRepositoryProperty, - ["github-codeql-extra-queries" /* EXTRA_QUERIES */]: parseStringRepositoryProperty + ["github-codeql-disable-overlay" /* DISABLE_OVERLAY */]: booleanProperty, + ["github-codeql-extra-queries" /* EXTRA_QUERIES */]: stringProperty }; async function loadPropertiesFromApi(gitHubVersion, logger, repositoryNwo) { if (gitHubVersion.type === "GitHub Enterprise Server" /* GHES */) { @@ -104434,11 +104446,6 @@ async function loadPropertiesFromApi(gitHubVersion, logger, repositoryNwo) { `Expected repository property object to have a 'property_name', but got: ${JSON.stringify(property)}` ); } - if (typeof property.value !== "string") { - throw new Error( - `Expected repository property '${property.property_name}' to have a string value, but got: ${JSON.stringify(property)}` - ); - } if (isKnownPropertyName(property.property_name)) { setProperty2(properties, property.property_name, property.value, logger); } @@ -104463,7 +104470,14 @@ async function loadPropertiesFromApi(gitHubVersion, logger, repositoryNwo) { } } function setProperty2(properties, name, value, logger) { - properties[name] = repositoryPropertyParsers[name](name, value, logger); + const propertyOptions = repositoryPropertyParsers[name]; + if (propertyOptions.validate(value)) { + properties[name] = propertyOptions.parse(name, value, logger); + } else { + throw new Error( + `Unexpected value for repository property '${name}' (${typeof value}), got: ${JSON.stringify(value)}` + ); + } } function parseBooleanRepositoryProperty(name, value, logger) { if (value !== "true" && value !== "false") { diff --git a/src/feature-flags/properties.test.ts b/src/feature-flags/properties.test.ts index afe9369325..c820f8d000 100644 --- a/src/feature-flags/properties.test.ts +++ b/src/feature-flags/properties.test.ts @@ -38,7 +38,7 @@ test.serial( ); test.serial( - "loadPropertiesFromApi throws if response data contains unexpected objects", + "loadPropertiesFromApi throws if response data contains objects without `property_name`", async (t) => { sinon.stub(api, "getRepositoryProperties").resolves({ headers: {}, @@ -64,6 +64,32 @@ test.serial( }, ); +test.serial( + "loadPropertiesFromApi does not throw for unexpected value types of unknown properties", + async (t) => { + sinon.stub(api, "getRepositoryProperties").resolves({ + headers: {}, + status: 200, + url: "", + data: [ + { property_name: "not-used-by-us", value: { foo: "bar" } }, + { property_name: "also-not-used-by-us", value: ["A", "B", "C"] }, + ], + }); + const logger = getRunnerLogger(true); + const mockRepositoryNwo = parseRepositoryNwo("owner/repo"); + await t.notThrowsAsync( + properties.loadPropertiesFromApi( + { + type: util.GitHubVariant.DOTCOM, + }, + logger, + mockRepositoryNwo, + ), + ); + }, +); + test.serial( "loadPropertiesFromApi returns empty object if on GHES", async (t) => { @@ -174,7 +200,7 @@ test.serial( ); test.serial( - "loadPropertiesFromApi throws if property value is not a string", + "loadPropertiesFromApi throws if known property value is not a string", async (t) => { sinon.stub(api, "getRepositoryProperties").resolves({ headers: {}, @@ -194,7 +220,7 @@ test.serial( ), { message: - /Expected repository property 'github-codeql-extra-queries' to have a string value/, + /Unexpected value for repository property 'github-codeql-extra-queries' \(number\), got: 123/, }, ); }, diff --git a/src/feature-flags/properties.ts b/src/feature-flags/properties.ts index 3b55fcb6a0..cb407c3085 100644 --- a/src/feature-flags/properties.ts +++ b/src/feature-flags/properties.ts @@ -12,7 +12,7 @@ export enum RepositoryPropertyName { } /** Parsed types of the known repository properties. */ -type AllRepositoryProperties = { +export type AllRepositoryProperties = { [RepositoryPropertyName.DISABLE_OVERLAY]: boolean; [RepositoryPropertyName.EXTRA_QUERIES]: string; }; @@ -20,16 +20,56 @@ type AllRepositoryProperties = { /** Parsed repository properties. */ export type RepositoryProperties = Partial; +/** Maps known repository properties to the type we expect to get from the API. */ +export type RepositoryPropertyApiType = { + [RepositoryPropertyName.DISABLE_OVERLAY]: string; + [RepositoryPropertyName.EXTRA_QUERIES]: string; +}; + +/** The type of functions which take the `value` from the API and try to convert it to the type we want. */ +export type PropertyParser = ( + name: K, + value: RepositoryPropertyApiType[K], + logger: Logger, +) => AllRepositoryProperties[K]; + +/** Possible types of `value`s we get from the API. */ +export type RepositoryPropertyValue = string | string[]; + +/** The type of repository property configurations. */ +export type PropertyInfo = { + /** A validator which checks that the value received from the API is what we expect. */ + validate: ( + value: RepositoryPropertyValue, + ) => value is RepositoryPropertyApiType[K]; + /** A `PropertyParser` for the property. */ + parse: PropertyParser; +}; + +/** Determines whether a value from the API is a string or not. */ +function isString(value: RepositoryPropertyValue): value is string { + return typeof value === "string"; +} + +/** A repository property that we expect to contain a string value. */ +const stringProperty = { + validate: isString, + parse: parseStringRepositoryProperty, +}; + +/** A repository property that we expect to contain a boolean value. */ +const booleanProperty = { + // The value from the API should come as a string, which we then parse into a boolean. + validate: isString, + parse: parseBooleanRepositoryProperty, +}; + /** Parsers that transform repository properties from the API response into typed values. */ const repositoryPropertyParsers: { - [K in RepositoryPropertyName]: ( - name: K, - value: string, - logger: Logger, - ) => AllRepositoryProperties[K]; + [K in RepositoryPropertyName]: PropertyInfo; } = { - [RepositoryPropertyName.DISABLE_OVERLAY]: parseBooleanRepositoryProperty, - [RepositoryPropertyName.EXTRA_QUERIES]: parseStringRepositoryProperty, + [RepositoryPropertyName.DISABLE_OVERLAY]: booleanProperty, + [RepositoryPropertyName.EXTRA_QUERIES]: stringProperty, }; /** @@ -37,7 +77,7 @@ const repositoryPropertyParsers: { */ export interface GitHubRepositoryProperty { property_name: string; - value: string; + value: RepositoryPropertyValue; } /** @@ -85,12 +125,6 @@ export async function loadPropertiesFromApi( ); } - if (typeof property.value !== "string") { - throw new Error( - `Expected repository property '${property.property_name}' to have a string value, but got: ${JSON.stringify(property)}`, - ); - } - if (isKnownPropertyName(property.property_name)) { setProperty(properties, property.property_name, property.value, logger); } @@ -117,14 +151,30 @@ export async function loadPropertiesFromApi( } } -/** Update the partial set of repository properties with the parsed value of the specified property. */ +/** + * Validate that `value` has the correct type for `K` and, if so, update the partial set of repository + * properties with the parsed value of the specified property. + */ function setProperty( properties: RepositoryProperties, name: K, - value: string, + value: RepositoryPropertyValue, logger: Logger, ): void { - properties[name] = repositoryPropertyParsers[name](name, value, logger); + const propertyOptions = repositoryPropertyParsers[name]; + + // We perform the validation here for two reasons: + // 1. This function is only called if `name` is a property we care about, to avoid throwing + // on unrelated properties that may use representations we do not support. + // 2. The `propertyOptions.validate` function checks that the type of `value` we received from + // the API is what expect and narrows the type accordingly, allowing us to call `parse`. + if (propertyOptions.validate(value)) { + properties[name] = propertyOptions.parse(name, value, logger); + } else { + throw new Error( + `Unexpected value for repository property '${name}' (${typeof value}), got: ${JSON.stringify(value)}`, + ); + } } /** Parse a boolean repository property. */