From 8e70ae21a1b4b30a35f2b8c32f316dd8d1eee60e Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 9 Mar 2026 12:03:34 +0000 Subject: [PATCH 1/7] Update `GitHubRepositoryProperty` to match schema --- src/feature-flags/properties.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/feature-flags/properties.ts b/src/feature-flags/properties.ts index 3b55fcb6a0..e98a414265 100644 --- a/src/feature-flags/properties.ts +++ b/src/feature-flags/properties.ts @@ -37,7 +37,7 @@ const repositoryPropertyParsers: { */ export interface GitHubRepositoryProperty { property_name: string; - value: string; + value: string | string[]; } /** From 9c75a5f60ced7a336710741b803a179f74860d09 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 9 Mar 2026 12:10:30 +0000 Subject: [PATCH 2/7] Only validate property `value` type if we care about the property --- lib/init-action.js | 10 +++++----- src/feature-flags/properties.test.ts | 23 +++++++++++++++++++++++ src/feature-flags/properties.ts | 14 ++++++++------ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/lib/init-action.js b/lib/init-action.js index a047977e27..6cf274d044 100644 --- a/lib/init-action.js +++ b/lib/init-action.js @@ -104434,12 +104434,12 @@ 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)) { + if (typeof property.value !== "string") { + throw new Error( + `Expected repository property '${property.property_name}' to have a string value, but got: ${JSON.stringify(property)}` + ); + } setProperty2(properties, property.property_name, property.value, logger); } } diff --git a/src/feature-flags/properties.test.ts b/src/feature-flags/properties.test.ts index afe9369325..3967806852 100644 --- a/src/feature-flags/properties.test.ts +++ b/src/feature-flags/properties.test.ts @@ -64,6 +64,29 @@ 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" } }], + }); + 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) => { diff --git a/src/feature-flags/properties.ts b/src/feature-flags/properties.ts index e98a414265..64459dd5cf 100644 --- a/src/feature-flags/properties.ts +++ b/src/feature-flags/properties.ts @@ -85,13 +85,15 @@ 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)) { + // Only validate the type of `value` if this is a property we care about, to avoid throwing + // on unrelated properties that may use representations we do not support. + if (typeof property.value !== "string") { + throw new Error( + `Expected repository property '${property.property_name}' to have a string value, but got: ${JSON.stringify(property)}`, + ); + } + setProperty(properties, property.property_name, property.value, logger); } } From 58991590bdeb0592c668edbd4224fc7601648184 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 9 Mar 2026 12:46:16 +0000 Subject: [PATCH 3/7] Validate `value` types returned by API against expectations --- lib/init-action.js | 42 +++++++++----- src/feature-flags/properties.test.ts | 6 +- src/feature-flags/properties.ts | 86 ++++++++++++++++++++++------ 3 files changed, 98 insertions(+), 36 deletions(-) diff --git a/lib/init-action.js b/lib/init-action.js index 6cf274d044..7e59bbb276 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 */) { @@ -104435,11 +104447,6 @@ async function loadPropertiesFromApi(gitHubVersion, logger, repositoryNwo) { ); } if (isKnownPropertyName(property.property_name)) { - if (typeof property.value !== "string") { - throw new Error( - `Expected repository property '${property.property_name}' to have a string value, but got: ${JSON.stringify(property)}` - ); - } 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}', 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 3967806852..d74d50d4bf 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: {}, @@ -197,7 +197,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: {}, @@ -217,7 +217,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', got: 123/, }, ); }, diff --git a/src/feature-flags/properties.ts b/src/feature-flags/properties.ts index 64459dd5cf..680cc48cee 100644 --- a/src/feature-flags/properties.ts +++ b/src/feature-flags/properties.ts @@ -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. */ +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 | string[]; + value: RepositoryPropertyValue; } /** @@ -86,14 +126,6 @@ export async function loadPropertiesFromApi( } if (isKnownPropertyName(property.property_name)) { - // Only validate the type of `value` if this is a property we care about, to avoid throwing - // on unrelated properties that may use representations we do not support. - if (typeof property.value !== "string") { - throw new Error( - `Expected repository property '${property.property_name}' to have a string value, but got: ${JSON.stringify(property)}`, - ); - } - setProperty(properties, property.property_name, property.value, logger); } } @@ -119,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}', got: ${JSON.stringify(value)}`, + ); + } } /** Parse a boolean repository property. */ From 58314dce95a83b737c360e9356356e52a6a60777 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 9 Mar 2026 13:03:47 +0000 Subject: [PATCH 4/7] Export types that weren't already --- src/feature-flags/properties.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/feature-flags/properties.ts b/src/feature-flags/properties.ts index 680cc48cee..276bfe501a 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; }; @@ -21,7 +21,7 @@ type AllRepositoryProperties = { export type RepositoryProperties = Partial; /** Maps known repository properties to the type we expect to get from the API. */ -type RepositoryPropertyApiType = { +export type RepositoryPropertyApiType = { [RepositoryPropertyName.DISABLE_OVERLAY]: string; [RepositoryPropertyName.EXTRA_QUERIES]: string; }; From 5311ed41ea6559c31939a79deab8c6030b41cbaf Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 9 Mar 2026 13:09:22 +0000 Subject: [PATCH 5/7] Include type in error message --- lib/init-action.js | 2 +- src/feature-flags/properties.test.ts | 2 +- src/feature-flags/properties.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/init-action.js b/lib/init-action.js index 7e59bbb276..b07f684e90 100644 --- a/lib/init-action.js +++ b/lib/init-action.js @@ -104475,7 +104475,7 @@ function setProperty2(properties, name, value, logger) { properties[name] = propertyOptions.parse(name, value, logger); } else { throw new Error( - `Unexpected value for repository property '${name}', got: ${JSON.stringify(value)}` + `Unexpected value for repository property '${name}' (${typeof value}), got: ${JSON.stringify(value)}` ); } } diff --git a/src/feature-flags/properties.test.ts b/src/feature-flags/properties.test.ts index d74d50d4bf..86e998de96 100644 --- a/src/feature-flags/properties.test.ts +++ b/src/feature-flags/properties.test.ts @@ -217,7 +217,7 @@ test.serial( ), { message: - /Unexpected value for repository property 'github-codeql-extra-queries', got: 123/, + /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 276bfe501a..cb407c3085 100644 --- a/src/feature-flags/properties.ts +++ b/src/feature-flags/properties.ts @@ -172,7 +172,7 @@ function setProperty( properties[name] = propertyOptions.parse(name, value, logger); } else { throw new Error( - `Unexpected value for repository property '${name}', got: ${JSON.stringify(value)}`, + `Unexpected value for repository property '${name}' (${typeof value}), got: ${JSON.stringify(value)}`, ); } } From 149fd14ac7b1778cea753416012a569738e88e87 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 9 Mar 2026 13:12:37 +0000 Subject: [PATCH 6/7] Add unknown property with `string[]` value --- src/feature-flags/properties.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/feature-flags/properties.test.ts b/src/feature-flags/properties.test.ts index 86e998de96..c820f8d000 100644 --- a/src/feature-flags/properties.test.ts +++ b/src/feature-flags/properties.test.ts @@ -71,7 +71,10 @@ test.serial( headers: {}, status: 200, url: "", - data: [{ property_name: "not-used-by-us", value: { foo: "bar" } }], + 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"); From 6f90eb695f80e8870988630b44b1630ba747f3b9 Mon Sep 17 00:00:00 2001 From: "Michael B. Gale" Date: Mon, 9 Mar 2026 14:24:29 +0000 Subject: [PATCH 7/7] Add changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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