Skip to content

Commit cb083ce

Browse files
feat: add sanitize function dynamic labels
1 parent f361d92 commit cb083ce

File tree

5 files changed

+96
-13
lines changed

5 files changed

+96
-13
lines changed

examples/default/main.tf

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@ module "runners" {
5151
# runners_lambda_zip = "../lambdas-download/runners.zip"
5252

5353
enable_organization_runners = true
54-
runner_extra_labels = ["default", "example"]
54+
# Note: labels starting with `ghr-` are ignored during webhook label matching
55+
# when `enable_dynamic_labels` is enabled.
56+
runner_extra_labels = ["default", "example"]
5557

5658
# enable access to the runners via SSM
5759
enable_ssm_on_runners = true

examples/multi-runner/main.tf

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,8 @@ module "runners" {
157157
# }
158158
# }
159159

160-
# Enable dyamic lables
160+
# Enable dynamic labels
161+
# When enabled, labels starting with `ghr-` are ignored during webhook label matching.
161162
# enable_dynamic_labels = true
162163
}
163164

lambdas/functions/webhook/src/runners/dispatch.test.ts

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,18 +216,79 @@ describe('Dispatcher', () => {
216216
expect(canRunJob(workflowLabels, runnerLabels, true, false)).toBe(false);
217217
});
218218

219-
it('should filter out ghr- and ghr-run- labels when enableDynamicLabels is true.', () => {
219+
it('should filter out ghr- labels when enableDynamicLabels is true.', () => {
220220
const workflowLabels = ['self-hosted', 'linux', 'x64', 'ghr-ec2-instance-type:t3.large', 'ghr-run-id:12345'];
221221
const runnerLabels = [['self-hosted', 'linux', 'x64']];
222222
expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true);
223223
});
224224

225-
it('should NOT filter out ghr- and ghr-run- labels when enableDynamicLabels is false.', () => {
225+
it('should NOT filter out ghr- labels when enableDynamicLabels is false.', () => {
226226
const workflowLabels = ['self-hosted', 'linux', 'x64', 'ghr-ec2-instance-type:t3.large'];
227227
const runnerLabels = [['self-hosted', 'linux', 'x64']];
228228
expect(canRunJob(workflowLabels, runnerLabels, true, false)).toBe(false);
229229
});
230230
});
231+
232+
describe('sanitizeGhrLabels (via canRunJob with enableDynamicLabels=true)', () => {
233+
it('should keep valid ghr- labels with allowed characters (alphanumeric, dot, dash, colon, slash)', () => {
234+
const workflowLabels = ['self-hosted', 'ghr-ec2-instance-type:t3.large'];
235+
const runnerLabels = [['self-hosted', 'ghr-ec2-instance-type:t3.large']];
236+
expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true);
237+
});
238+
239+
it('should keep ghr- labels with path-like values containing slashes', () => {
240+
const workflowLabels = ['self-hosted', 'ghr-image:my/custom/image'];
241+
const runnerLabels = [['self-hosted', 'ghr-image:my/custom/image']];
242+
expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true);
243+
});
244+
245+
it('should strip ghr- labels that exceed 128 characters', () => {
246+
const longLabel = 'ghr-' + 'a'.repeat(125); // 129 chars total, exceeds 128
247+
const workflowLabels = ['self-hosted', 'linux', longLabel];
248+
const runnerLabels = [['self-hosted', 'linux']];
249+
// Long label is stripped, remaining labels match
250+
expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true);
251+
});
252+
253+
it('should keep ghr- labels that are exactly 128 characters', () => {
254+
const exactLabel = 'ghr-' + 'a'.repeat(124); // exactly 128 chars
255+
const workflowLabels = ['self-hosted', exactLabel];
256+
const runnerLabels = [['self-hosted', exactLabel]];
257+
expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true);
258+
});
259+
260+
it('should strip ghr- labels with invalid characters (spaces)', () => {
261+
const workflowLabels = ['self-hosted', 'linux', 'ghr-bad label'];
262+
const runnerLabels = [['self-hosted', 'linux']];
263+
// Invalid label is stripped, remaining labels match
264+
expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true);
265+
});
266+
267+
it('should strip ghr- labels with invalid characters (special chars)', () => {
268+
const workflowLabels = ['self-hosted', 'linux', 'ghr-inject;rm -rf'];
269+
const runnerLabels = [['self-hosted', 'linux']];
270+
expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true);
271+
});
272+
273+
it('should never strip non-ghr labels regardless of content', () => {
274+
const workflowLabels = ['self-hosted', 'linux', 'x64'];
275+
const runnerLabels = [['self-hosted', 'linux', 'x64']];
276+
expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true);
277+
});
278+
279+
it('should handle a mix of valid ghr-, invalid ghr-, and regular labels', () => {
280+
const longLabel = 'ghr-' + 'x'.repeat(125); // 129 chars, will be stripped
281+
const workflowLabels = [
282+
'self-hosted',
283+
'linux',
284+
'ghr-valid:value', // valid, kept
285+
'ghr-bad label', // invalid chars, stripped
286+
longLabel, // too long, stripped
287+
];
288+
const runnerLabels = [['self-hosted', 'linux', 'ghr-valid:value']];
289+
expect(canRunJob(workflowLabels, runnerLabels, true, true)).toBe(true);
290+
});
291+
});
231292
});
232293

233294
function mockSSMResponse(runnerConfigInput?: RunnerConfig) {

lambdas/functions/webhook/src/runners/dispatch.ts

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,24 +85,43 @@ async function handleWorkflowJob(
8585
return { statusCode: 202, body: notAcceptedErrorMsg };
8686
}
8787

88+
function sanitizeGhrLabels(labels: string[]): string[] {
89+
const GHR_LABEL_MAX_LENGTH = 128;
90+
const GHR_LABEL_VALUE_PATTERN = /^[a-zA-Z0-9._/\-:]+$/;
91+
92+
return labels
93+
.map((label) => {
94+
if (!label.startsWith('ghr-')) return label;
95+
96+
if (label.length > GHR_LABEL_MAX_LENGTH) {
97+
logger.warn('Dynamic label exceeds max length, stripping', { label: label.substring(0, 40) });
98+
return null;
99+
}
100+
if (!GHR_LABEL_VALUE_PATTERN.test(label)) {
101+
logger.warn('Dynamic label contains invalid characters, stripping', { label });
102+
return null;
103+
}
104+
return null;
105+
})
106+
.filter((l): l is string => l !== null);
107+
}
108+
88109
export function canRunJob(
89110
workflowJobLabels: string[],
90111
runnerLabelsMatchers: string[][],
91112
workflowLabelCheckAll: boolean,
92113
enableDynamicLabels: boolean,
93114
): boolean {
94-
// Filter out ghr- and ghr-run- labels only if dynamic labels config is enabled
95-
const filteredLabels = enableDynamicLabels
96-
? workflowJobLabels.filter((label) => !label.startsWith('ghr-'))
97-
: workflowJobLabels;
115+
// Filter out ghr- labels only and sanitize them if dynamic labels is enabled, otherwise keep all labels as is for matching.
116+
const sanitizedLabels = enableDynamicLabels ? sanitizeGhrLabels(workflowJobLabels) : workflowJobLabels;
98117

99118
runnerLabelsMatchers = runnerLabelsMatchers.map((runnerLabel) => {
100119
return runnerLabel.map((label) => label.toLowerCase());
101120
});
102121
const matchLabels = workflowLabelCheckAll
103-
? runnerLabelsMatchers.some((rl) => filteredLabels.every((wl) => rl.includes(wl.toLowerCase())))
104-
: runnerLabelsMatchers.some((rl) => filteredLabels.some((wl) => rl.includes(wl.toLowerCase())));
105-
const match = filteredLabels.length === 0 ? !matchLabels : matchLabels;
122+
? runnerLabelsMatchers.some((rl) => sanitizedLabels.every((wl) => rl.includes(wl.toLowerCase())))
123+
: runnerLabelsMatchers.some((rl) => sanitizedLabels.some((wl) => rl.includes(wl.toLowerCase())));
124+
const match = sanitizedLabels.length === 0 ? !matchLabels : matchLabels;
106125

107126
logger.debug(
108127
`Received workflow job event with labels: '${JSON.stringify(workflowJobLabels)}'. The event does ${

variables.tf

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ variable "runner_disable_default_labels" {
9292
}
9393

9494
variable "runner_extra_labels" {
95-
description = "Extra (custom) labels for the runners (GitHub). Separate each label by a comma. Labels checks on the webhook can be enforced by setting `enable_workflow_job_labels_check`. GitHub read-only labels should not be provided."
95+
description = "Extra (custom) labels for the runners (GitHub). Separate each label by a comma. Labels checks on the webhook can be enforced by setting `enable_workflow_job_labels_check`. GitHub read-only labels should not be provided. Note: labels starting with `ghr-` are ignored during webhook label matching when `enable_dynamic_labels` is enabled."
9696
type = list(string)
9797
default = []
9898

@@ -674,7 +674,7 @@ variable "enable_ephemeral_runners" {
674674
}
675675

676676
variable "enable_dynamic_labels" {
677-
description = "Experimental! Can be removed / changed without trigger a major release. Enable dynamic EC2 configs based on workflow job labels. When enabled, jobs can request specific configs via the 'gh-ec2-<config type key>:<config type value>' label (e.g., 'gh-ec2-instance-type:t3.large')."
677+
description = "Experimental! Can be removed / changed without trigger a major release. Enable dynamic EC2 configs based on workflow job labels. When enabled, jobs can request specific configs via the 'gh-ec2-<config type key>:<config type value>' label (e.g., 'gh-ec2-instance-type:t3.large'). When enabled, labels starting with `ghr-` are ignored during webhook label matching."
678678
type = bool
679679
default = false
680680
}

0 commit comments

Comments
 (0)