| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -4,7 +4,7 @@ See the [releases page](https://github.com/github/codeql-action/releases) for th | |||
| 4 | 4 | ||
| 5 | 5 | ## [UNRELEASED] | |
| 6 | 6 | ||
| 7 | - No user facing changes. | ||
| 7 | + - 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) | ||
| 8 | 8 | ||
| 9 | 9 | ## 4.32.6 - 05 Mar 2026 | |
| 10 | 10 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -38,7 +38,7 @@ test.serial( | |||
| 38 | 38 | ); | |
| 39 | 39 | ||
| 40 | 40 | test.serial( | |
| 41 | - "loadPropertiesFromApi throws if response data contains unexpected objects", | ||
| 41 | + "loadPropertiesFromApi throws if response data contains objects without `property_name`", | ||
| 42 | 42 | async (t) => { | |
| 43 | 43 | sinon.stub(api, "getRepositoryProperties").resolves({ | |
| 44 | 44 | headers: {}, | |
@@ -64,6 +64,32 @@ test.serial( | |||
| 64 | 64 | }, | |
| 65 | 65 | ); | |
| 66 | 66 | ||
| 67 | + test.serial( | ||
| 68 | + "loadPropertiesFromApi does not throw for unexpected value types of unknown properties", | ||
| 69 | + async (t) => { | ||
| 70 | + sinon.stub(api, "getRepositoryProperties").resolves({ | ||
| 71 | + headers: {}, | ||
| 72 | + status: 200, | ||
| 73 | + url: "", | ||
| 74 | + data: [ | ||
| 75 | + { property_name: "not-used-by-us", value: { foo: "bar" } }, | ||
| 76 | + { property_name: "also-not-used-by-us", value: ["A", "B", "C"] }, | ||
| 77 | + ], | ||
| 78 | + }); | ||
| 79 | + const logger = getRunnerLogger(true); | ||
| 80 | + const mockRepositoryNwo = parseRepositoryNwo("owner/repo"); | ||
| 81 | + await t.notThrowsAsync( | ||
| 82 | + properties.loadPropertiesFromApi( | ||
| 83 | + { | ||
| 84 | + type: util.GitHubVariant.DOTCOM, | ||
| 85 | + }, | ||
| 86 | + logger, | ||
| 87 | + mockRepositoryNwo, | ||
| 88 | + ), | ||
| 89 | + ); | ||
| 90 | + }, | ||
| 91 | + ); | ||
| 92 | + | ||
| 67 | 93 | test.serial( | |
| 68 | 94 | "loadPropertiesFromApi returns empty object if on GHES", | |
| 69 | 95 | async (t) => { | |
@@ -174,7 +200,7 @@ test.serial( | |||
| 174 | 200 | ); | |
| 175 | 201 | ||
| 176 | 202 | test.serial( | |
| 177 | - "loadPropertiesFromApi throws if property value is not a string", | ||
| 203 | + "loadPropertiesFromApi throws if known property value is not a string", | ||
| 178 | 204 | async (t) => { | |
| 179 | 205 | sinon.stub(api, "getRepositoryProperties").resolves({ | |
| 180 | 206 | headers: {}, | |
@@ -194,7 +220,7 @@ test.serial( | |||
| 194 | 220 | ), | |
| 195 | 221 | { | |
| 196 | 222 | message: | |
| 197 | - /Expected repository property 'github-codeql-extra-queries' to have a string value/, | ||
| 223 | + /Unexpected value for repository property 'github-codeql-extra-queries' \(number\), got: 123/, | ||
| 198 | 224 | }, | |
| 199 | 225 | ); | |
| 200 | 226 | }, | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -12,32 +12,72 @@ export enum RepositoryPropertyName { | |||
| 12 | 12 | } | |
| 13 | 13 | ||
| 14 | 14 | /** Parsed types of the known repository properties. */ | |
| 15 | - type AllRepositoryProperties = { | ||
| 15 | + export type AllRepositoryProperties = { | ||
| 16 | 16 | [RepositoryPropertyName.DISABLE_OVERLAY]: boolean; | |
| 17 | 17 | [RepositoryPropertyName.EXTRA_QUERIES]: string; | |
| 18 | 18 | }; | |
| 19 | 19 | ||
| 20 | 20 | /** Parsed repository properties. */ | |
| 21 | 21 | export type RepositoryProperties = Partial<AllRepositoryProperties>; | |
| 22 | 22 | ||
| 23 | + /** Maps known repository properties to the type we expect to get from the API. */ | ||
| 24 | + export type RepositoryPropertyApiType = { | ||
| 25 | + [RepositoryPropertyName.DISABLE_OVERLAY]: string; | ||
| 26 | + [RepositoryPropertyName.EXTRA_QUERIES]: string; | ||
| 27 | + }; | ||
| 28 | + | ||
| 29 | + /** The type of functions which take the `value` from the API and try to convert it to the type we want. */ | ||
| 30 | + export type PropertyParser<K extends RepositoryPropertyName> = ( | ||
| 31 | + name: K, | ||
| 32 | + value: RepositoryPropertyApiType[K], | ||
| 33 | + logger: Logger, | ||
| 34 | + ) => AllRepositoryProperties[K]; | ||
| 35 | + | ||
| 36 | + /** Possible types of `value`s we get from the API. */ | ||
| 37 | + export type RepositoryPropertyValue = string | string[]; | ||
| 38 | + | ||
| 39 | + /** The type of repository property configurations. */ | ||
| 40 | + export type PropertyInfo<K extends RepositoryPropertyName> = { | ||
| 41 | + /** A validator which checks that the value received from the API is what we expect. */ | ||
| 42 | + validate: ( | ||
| 43 | + value: RepositoryPropertyValue, | ||
| 44 | + ) => value is RepositoryPropertyApiType[K]; | ||
| 45 | + /** A `PropertyParser` for the property. */ | ||
| 46 | + parse: PropertyParser<K>; | ||
| 47 | + }; | ||
| 48 | + | ||
| 49 | + /** Determines whether a value from the API is a string or not. */ | ||
| 50 | + function isString(value: RepositoryPropertyValue): value is string { | ||
| 51 | + return typeof value === "string"; | ||
| 52 | + } | ||
| 53 | + | ||
| 54 | + /** A repository property that we expect to contain a string value. */ | ||
| 55 | + const stringProperty = { | ||
| 56 | + validate: isString, | ||
| 57 | + parse: parseStringRepositoryProperty, | ||
| 58 | + }; | ||
| 59 | + | ||
| 60 | + /** A repository property that we expect to contain a boolean value. */ | ||
| 61 | + const booleanProperty = { | ||
| 62 | + // The value from the API should come as a string, which we then parse into a boolean. | ||
| 63 | + validate: isString, | ||
| 64 | + parse: parseBooleanRepositoryProperty, | ||
| 65 | + }; | ||
| 66 | + | ||
| 23 | 67 | /** Parsers that transform repository properties from the API response into typed values. */ | |
| 24 | 68 | const repositoryPropertyParsers: { | |
| 25 | - [K in RepositoryPropertyName]: ( | ||
| 26 | - name: K, | ||
| 27 | - value: string, | ||
| 28 | - logger: Logger, | ||
| 29 | - ) => AllRepositoryProperties[K]; | ||
| 69 | + [K in RepositoryPropertyName]: PropertyInfo<K>; | ||
| 30 | 70 | } = { | |
| 31 | - [RepositoryPropertyName.DISABLE_OVERLAY]: parseBooleanRepositoryProperty, | ||
| 32 | - [RepositoryPropertyName.EXTRA_QUERIES]: parseStringRepositoryProperty, | ||
| 71 | + [RepositoryPropertyName.DISABLE_OVERLAY]: booleanProperty, | ||
| 72 | + [RepositoryPropertyName.EXTRA_QUERIES]: stringProperty, | ||
| 33 | 73 | }; | |
| 34 | 74 | ||
| 35 | 75 | /** | |
| 36 | 76 | * A repository property has a name and a value. | |
| 37 | 77 | */ | |
| 38 | 78 | export interface GitHubRepositoryProperty { | |
| 39 | 79 | property_name: string; | |
| 40 | - value: string; | ||
| 80 | + value: RepositoryPropertyValue; | ||
| 41 | 81 | } | |
| 42 | 82 | ||
| 43 | 83 | /** | |
@@ -85,12 +125,6 @@ export async function loadPropertiesFromApi( | |||
| 85 | 125 | ); | |
| 86 | 126 | } | |
| 87 | 127 | ||
| 88 | - if (typeof property.value !== "string") { | ||
| 89 | - throw new Error( | ||
| 90 | - `Expected repository property '${property.property_name}' to have a string value, but got: ${JSON.stringify(property)}`, | ||
| 91 | - ); | ||
| 92 | - } | ||
| 93 | - | ||
| 94 | 128 | if (isKnownPropertyName(property.property_name)) { | |
| 95 | 129 | setProperty(properties, property.property_name, property.value, logger); | |
| 96 | 130 | } | |
@@ -117,14 +151,30 @@ export async function loadPropertiesFromApi( | |||
| 117 | 151 | } | |
| 118 | 152 | } | |
| 119 | 153 | ||
| 120 | - /** Update the partial set of repository properties with the parsed value of the specified property. */ | ||
| 154 | + /** | ||
| 155 | + * Validate that `value` has the correct type for `K` and, if so, update the partial set of repository | ||
| 156 | + * properties with the parsed value of the specified property. | ||
| 157 | + */ | ||
| 121 | 158 | function setProperty<K extends RepositoryPropertyName>( | |
| 122 | 159 | properties: RepositoryProperties, | |
| 123 | 160 | name: K, | |
| 124 | - value: string, | ||
| 161 | + value: RepositoryPropertyValue, | ||
| 125 | 162 | logger: Logger, | |
| 126 | 163 | ): void { | |
| 127 | - properties[name] = repositoryPropertyParsers[name](name, value, logger); | ||
| 164 | + const propertyOptions = repositoryPropertyParsers[name]; | ||
| 165 | + | ||
| 166 | + // We perform the validation here for two reasons: | ||
| 167 | + // 1. This function is only called if `name` is a property we care about, to avoid throwing | ||
| 168 | + // on unrelated properties that may use representations we do not support. | ||
| 169 | + // 2. The `propertyOptions.validate` function checks that the type of `value` we received from | ||
| 170 | + // the API is what expect and narrows the type accordingly, allowing us to call `parse`. | ||
| 171 | + if (propertyOptions.validate(value)) { | ||
| 172 | + properties[name] = propertyOptions.parse(name, value, logger); | ||
| 173 | + } else { | ||
| 174 | + throw new Error( | ||
| 175 | + `Unexpected value for repository property '${name}' (${typeof value}), got: ${JSON.stringify(value)}`, | ||
| 176 | + ); | ||
| 177 | + } | ||
| 128 | 178 | } | |
| 129 | 179 | ||
| 130 | 180 | /** Parse a boolean repository property. */ | |
| Back | FazBrowse Home | New Git URL |
0 commit comments