Skip to content

Commit b39251f

Browse files
authored
Merge pull request #3557 from github/mbg/repo-props/multi-select
Fix handling of non-`string` values from repository properties API
2 parents f054eea + 6f90eb6 commit b39251f

File tree

4 files changed

+126
-36
lines changed

4 files changed

+126
-36
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ See the [releases page](https://github.com/github/codeql-action/releases) for th
44

55
## [UNRELEASED]
66

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)
88

99
## 4.32.6 - 05 Mar 2026
1010

lib/init-action.js

Lines changed: 28 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/feature-flags/properties.test.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ test.serial(
3838
);
3939

4040
test.serial(
41-
"loadPropertiesFromApi throws if response data contains unexpected objects",
41+
"loadPropertiesFromApi throws if response data contains objects without `property_name`",
4242
async (t) => {
4343
sinon.stub(api, "getRepositoryProperties").resolves({
4444
headers: {},
@@ -64,6 +64,32 @@ test.serial(
6464
},
6565
);
6666

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+
6793
test.serial(
6894
"loadPropertiesFromApi returns empty object if on GHES",
6995
async (t) => {
@@ -174,7 +200,7 @@ test.serial(
174200
);
175201

176202
test.serial(
177-
"loadPropertiesFromApi throws if property value is not a string",
203+
"loadPropertiesFromApi throws if known property value is not a string",
178204
async (t) => {
179205
sinon.stub(api, "getRepositoryProperties").resolves({
180206
headers: {},
@@ -194,7 +220,7 @@ test.serial(
194220
),
195221
{
196222
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/,
198224
},
199225
);
200226
},

src/feature-flags/properties.ts

Lines changed: 68 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,32 +12,72 @@ export enum RepositoryPropertyName {
1212
}
1313

1414
/** Parsed types of the known repository properties. */
15-
type AllRepositoryProperties = {
15+
export type AllRepositoryProperties = {
1616
[RepositoryPropertyName.DISABLE_OVERLAY]: boolean;
1717
[RepositoryPropertyName.EXTRA_QUERIES]: string;
1818
};
1919

2020
/** Parsed repository properties. */
2121
export type RepositoryProperties = Partial<AllRepositoryProperties>;
2222

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+
2367
/** Parsers that transform repository properties from the API response into typed values. */
2468
const repositoryPropertyParsers: {
25-
[K in RepositoryPropertyName]: (
26-
name: K,
27-
value: string,
28-
logger: Logger,
29-
) => AllRepositoryProperties[K];
69+
[K in RepositoryPropertyName]: PropertyInfo<K>;
3070
} = {
31-
[RepositoryPropertyName.DISABLE_OVERLAY]: parseBooleanRepositoryProperty,
32-
[RepositoryPropertyName.EXTRA_QUERIES]: parseStringRepositoryProperty,
71+
[RepositoryPropertyName.DISABLE_OVERLAY]: booleanProperty,
72+
[RepositoryPropertyName.EXTRA_QUERIES]: stringProperty,
3373
};
3474

3575
/**
3676
* A repository property has a name and a value.
3777
*/
3878
export interface GitHubRepositoryProperty {
3979
property_name: string;
40-
value: string;
80+
value: RepositoryPropertyValue;
4181
}
4282

4383
/**
@@ -85,12 +125,6 @@ export async function loadPropertiesFromApi(
85125
);
86126
}
87127

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-
94128
if (isKnownPropertyName(property.property_name)) {
95129
setProperty(properties, property.property_name, property.value, logger);
96130
}
@@ -117,14 +151,30 @@ export async function loadPropertiesFromApi(
117151
}
118152
}
119153

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+
*/
121158
function setProperty<K extends RepositoryPropertyName>(
122159
properties: RepositoryProperties,
123160
name: K,
124-
value: string,
161+
value: RepositoryPropertyValue,
125162
logger: Logger,
126163
): 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+
}
128178
}
129179

130180
/** Parse a boolean repository property. */

0 commit comments

Comments
 (0)