From 452ee9ea1891915cc2ca9ee0800d6999dbb8f201 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 4 Nov 2026 03:45:14 +0000 Subject: [PATCH 1/3] fix(governance): reconcile signed CI rulesets and bound checks Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- src/config-validation.ts | 10 ++++++++-- src/github-client.ts | 8 ++++++-- src/ruleset-normalizer.ts | 13 ++++++++++++- src/types.ts | 4 ++-- tests/config-validation.test.ts | 27 +++++++++++++++++++++++++++ tests/ruleset-normalizer.test.ts | 23 +++++++++++++++++++++++ 6 files changed, 78 insertions(+), 7 deletions(-) diff --git a/src/config-validation.ts b/src/config-validation.ts index 67f9e30..4f15496 100644 --- a/src/config-validation.ts +++ b/src/config-validation.ts @@ -233,7 +233,7 @@ function validateRule(value: unknown, path: string): RulesetRule { const record = expectRecord(value, path); const type = expectString(record.type, `${path}.type`); - if (type === 'deletion' || type === 'non_fast_forward') { + if (type === 'deletion' || type === 'non_fast_forward' || type === 'required_signatures') { rejectUnknownKeys(record, path, ['type']); return { type }; } @@ -312,13 +312,19 @@ function validateRule(value: unknown, path: string): RulesetRule { rejectUnknownKeys( checkRecord, `${path}.parameters.required_status_checks[${index}]`, - ['context'], + ['context', 'integration_id'], ); return { context: expectNonEmptyString( checkRecord.context, `${path}.parameters.required_status_checks[${index}].context`, ), + ...('integration_id' in checkRecord ? { + integration_id: expectNumber( + checkRecord.integration_id, + `${path}.parameters.required_status_checks[${index}].integration_id`, + ), + } : {}), }; }); diff --git a/src/github-client.ts b/src/github-client.ts index 5d99a18..9f28df5 100644 --- a/src/github-client.ts +++ b/src/github-client.ts @@ -392,7 +392,7 @@ function parseRule(value: unknown): RulesetRule { throw new Error('Invalid ruleset rule from GitHub'); } - if (value.type === 'deletion' || value.type === 'non_fast_forward') { + if (value.type === 'deletion' || value.type === 'non_fast_forward' || value.type === 'required_signatures') { return { type: value.type }; } @@ -444,7 +444,11 @@ function parseRule(value: unknown): RulesetRule { if (!isRecord(check) || typeof check.context !== 'string') { throw new Error('Invalid required status check from GitHub'); } - return { context: check.context }; + const integrationId = check.integration_id; + if (integrationId !== undefined && (typeof integrationId !== 'number' || !Number.isInteger(integrationId))) { + throw new Error('Invalid required status check integration from GitHub'); + } + return { context: check.context, ...(integrationId !== undefined ? { integration_id: integrationId } : {}) }; }) : []; diff --git a/src/ruleset-normalizer.ts b/src/ruleset-normalizer.ts index c6f335f..719ddfe 100644 --- a/src/ruleset-normalizer.ts +++ b/src/ruleset-normalizer.ts @@ -72,5 +72,16 @@ export function rulesetsEqual( left: RepositoryRuleset, right: RepositoryRuleset, ): boolean { - return JSON.stringify(normalizeRuleset(left)) === JSON.stringify(normalizeRuleset(right)); + const current = normalizeRuleset(left); + const desired = normalizeRuleset(right); + for (const rule of current.rules) { + if (rule.type !== 'required_status_checks') continue; + const expected = desired.rules.find((candidate) => candidate.type === 'required_status_checks'); + if (expected?.type !== 'required_status_checks') continue; + rule.parameters.required_status_checks = rule.parameters.required_status_checks.map((check) => { + const wanted = expected.parameters.required_status_checks.find((candidate) => candidate.context === check.context); + return wanted?.integration_id === undefined ? { context: check.context } : check; + }); + } + return JSON.stringify(current) === JSON.stringify(desired); } diff --git a/src/types.ts b/src/types.ts index 64066af..3beec65 100644 --- a/src/types.ts +++ b/src/types.ts @@ -22,14 +22,14 @@ export type PullRequestRule = { export type RequiredStatusChecksRule = { type: 'required_status_checks'; parameters: { - required_status_checks: Array<{ context: string }>; + required_status_checks: Array<{ context: string; integration_id?: number }>; strict_required_status_checks_policy: boolean; do_not_enforce_on_create: boolean; }; }; export type SimpleRule = { - type: 'deletion' | 'non_fast_forward'; + type: 'deletion' | 'non_fast_forward' | 'required_signatures'; }; export type RulesetRule = PullRequestRule | RequiredStatusChecksRule | SimpleRule; diff --git a/tests/config-validation.test.ts b/tests/config-validation.test.ts index 7cce7bd..45b5b25 100644 --- a/tests/config-validation.test.ts +++ b/tests/config-validation.test.ts @@ -69,6 +69,33 @@ describe('validateGovernanceConfig', () => { ); }); + it('accepts signed commits and bound required CI checks', () => { + const config = structuredClone(validConfig) as any; + config.policies.protected.rules.push({ type: 'required_signatures' }); + config.policies.protected.rules.push({ + type: 'required_status_checks', + parameters: { + required_status_checks: [{ context: 'PHPUnit', integration_id: 15368 }], + strict_required_status_checks_policy: true, + do_not_enforce_on_create: false, + }, + }); + expect(validateGovernanceConfig(config)).toEqual(config); + }); + + it('rejects invalid CI integration identifiers', () => { + const config = structuredClone(validConfig) as any; + config.policies.protected.rules.push({ + type: 'required_status_checks', + parameters: { + required_status_checks: [{ context: 'PHPUnit', integration_id: 'invalid' }], + strict_required_status_checks_policy: true, + do_not_enforce_on_create: false, + }, + }); + expect(() => validateGovernanceConfig(config)).toThrow('integration_id'); + }); + it('rejects unsupported rule types', () => { const config = structuredClone(validConfig) as any; config.policies.protected.rules = [{ type: 'creation' }]; diff --git a/tests/ruleset-normalizer.test.ts b/tests/ruleset-normalizer.test.ts index 641ae9f..9eabf10 100644 --- a/tests/ruleset-normalizer.test.ts +++ b/tests/ruleset-normalizer.test.ts @@ -13,6 +13,29 @@ describe('normalizeRuleset', () => { expect(rulesetsEqual(current, protectedBranchesFixture)).toBe(true); }); + it('compares integration-bound checks without drifting unbound configurations', () => { + const current = structuredClone(requiredCiFixture); + const desired = structuredClone(requiredCiFixture); + const currentRule = current.rules.find((rule) => rule.type === 'required_status_checks'); + const desiredRule = desired.rules.find((rule) => rule.type === 'required_status_checks'); + if (currentRule?.type !== 'required_status_checks' || desiredRule?.type !== 'required_status_checks') { + throw new Error('missing fixture checks'); + } + currentRule.parameters.required_status_checks[0]!.integration_id = 15368; + expect(rulesetsEqual(current, desired)).toBe(true); + desiredRule.parameters.required_status_checks[0]!.integration_id = 15368; + expect(rulesetsEqual(current, desired)).toBe(true); + currentRule.parameters.required_status_checks[0]!.integration_id = 999; + expect(rulesetsEqual(current, desired)).toBe(false); + }); + + it('preserves required signatures as a managed rule', () => { + const ruleset = structuredClone(protectedBranchesFixture); + ruleset.rules.push({ type: 'required_signatures' }); + expect(normalizeRuleset(ruleset).rules.some((rule) => rule.type === 'required_signatures')).toBe(true); + expect(rulesetsEqual(ruleset, protectedBranchesFixture)).toBe(false); + }); + it('preserves required status checks as managed policy', () => { const normalized = normalizeRuleset(requiredCiFixture); const rule = normalized.rules.find( From 411abbf9c3f017c1704d83ea39da6199cf8ac023 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 4 Nov 2026 18:15:12 +0000 Subject: [PATCH 2/3] test(governance): preserve signature regression and validate integration IDs Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- src/config-validation.ts | 9 ++++++++- src/github-client.ts | 2 +- tests/config-validation.test.ts | 22 ++++++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/config-validation.ts b/src/config-validation.ts index 4f15496..db068ae 100644 --- a/src/config-validation.ts +++ b/src/config-validation.ts @@ -320,7 +320,7 @@ function validateRule(value: unknown, path: string): RulesetRule { `${path}.parameters.required_status_checks[${index}].context`, ), ...('integration_id' in checkRecord ? { - integration_id: expectNumber( + integration_id: expectNonNegativeInteger( checkRecord.integration_id, `${path}.parameters.required_status_checks[${index}].integration_id`, ), @@ -401,6 +401,13 @@ function expectBoolean(value: unknown, path: string): boolean { return value; } +function expectNonNegativeInteger(value: unknown, path: string): number { + if (typeof value !== 'number' || !Number.isSafeInteger(value) || value < 0) { + fail(path, 'must be a non-negative safe integer'); + } + return value; +} + function expectNumber(value: unknown, path: string): number { if (typeof value !== 'number' || !Number.isFinite(value)) { fail(path, 'must be a finite number'); diff --git a/src/github-client.ts b/src/github-client.ts index 9f28df5..8d46dce 100644 --- a/src/github-client.ts +++ b/src/github-client.ts @@ -445,7 +445,7 @@ function parseRule(value: unknown): RulesetRule { throw new Error('Invalid required status check from GitHub'); } const integrationId = check.integration_id; - if (integrationId !== undefined && (typeof integrationId !== 'number' || !Number.isInteger(integrationId))) { + if (integrationId !== undefined && (typeof integrationId !== 'number' || !Number.isSafeInteger(integrationId) || integrationId < 0)) { throw new Error('Invalid required status check integration from GitHub'); } return { context: check.context, ...(integrationId !== undefined ? { integration_id: integrationId } : {}) }; diff --git a/tests/config-validation.test.ts b/tests/config-validation.test.ts index 45b5b25..87e83a8 100644 --- a/tests/config-validation.test.ts +++ b/tests/config-validation.test.ts @@ -69,6 +69,12 @@ describe('validateGovernanceConfig', () => { ); }); + it('accepts an explicit signed-commit ruleset', () => { + const config = structuredClone(validConfig) as any; + config.policies.protected.rules.push({ type: 'required_signatures' }); + expect(validateGovernanceConfig(config)).toEqual(config); + }); + it('accepts signed commits and bound required CI checks', () => { const config = structuredClone(validConfig) as any; config.policies.protected.rules.push({ type: 'required_signatures' }); @@ -96,6 +102,22 @@ describe('validateGovernanceConfig', () => { expect(() => validateGovernanceConfig(config)).toThrow('integration_id'); }); + it.each([-1, 1.5, Number.MAX_SAFE_INTEGER + 1])( + 'rejects invalid numerical integration ID %s', + (integrationId) => { + const config = structuredClone(validConfig) as any; + config.policies.protected.rules.push({ + type: 'required_status_checks', + parameters: { + required_status_checks: [{ context: 'PHPUnit', integration_id: integrationId }], + strict_required_status_checks_policy: true, + do_not_enforce_on_create: false, + }, + }); + expect(() => validateGovernanceConfig(config)).toThrow('integration_id'); + }, + ); + it('rejects unsupported rule types', () => { const config = structuredClone(validConfig) as any; config.policies.protected.rules = [{ type: 'creation' }]; From 07259f1f5652be196391fa0624786d57df805406 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 4 Nov 2026 18:15:49 +0000 Subject: [PATCH 3/3] fix(governance): isolate validation tests from main branch changes Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- tests/config-validation.test.ts | 66 +++++++++++++++------------------ 1 file changed, 30 insertions(+), 36 deletions(-) diff --git a/tests/config-validation.test.ts b/tests/config-validation.test.ts index 87e83a8..6f74a64 100644 --- a/tests/config-validation.test.ts +++ b/tests/config-validation.test.ts @@ -69,10 +69,37 @@ describe('validateGovernanceConfig', () => { ); }); - it('accepts an explicit signed-commit ruleset', () => { + it('rejects unsupported rule types', () => { const config = structuredClone(validConfig) as any; - config.policies.protected.rules.push({ type: 'required_signatures' }); - expect(validateGovernanceConfig(config)).toEqual(config); + config.policies.protected.rules = [{ type: 'creation' }]; + + expect(() => validateGovernanceConfig(config)).toThrow( + 'unsupported rule type: creation', + ); + }); + + it('rejects invalid bypass actors', () => { + const config = structuredClone(validConfig) as any; + config.policies.protected.bypass_actors = [ + { + actor_id: 1, + actor_type: 'Robot', + bypass_mode: 'always', + }, + ]; + + expect(() => validateGovernanceConfig(config)).toThrow( + '$.policies.protected.bypass_actors[0].actor_type', + ); + }); + + it('rejects unsupported repository metadata keys', () => { + const config = structuredClone(validConfig) as any; + config.repositories['ExampleOrg/project'].metadata.typo = true; + + expect(() => validateGovernanceConfig(config)).toThrow( + '$.repositories.ExampleOrg/project.metadata.typo', + ); }); it('accepts signed commits and bound required CI checks', () => { @@ -118,37 +145,4 @@ describe('validateGovernanceConfig', () => { }, ); - it('rejects unsupported rule types', () => { - const config = structuredClone(validConfig) as any; - config.policies.protected.rules = [{ type: 'creation' }]; - - expect(() => validateGovernanceConfig(config)).toThrow( - 'unsupported rule type: creation', - ); - }); - - it('rejects invalid bypass actors', () => { - const config = structuredClone(validConfig) as any; - config.policies.protected.bypass_actors = [ - { - actor_id: 1, - actor_type: 'Robot', - bypass_mode: 'always', - }, - ]; - - expect(() => validateGovernanceConfig(config)).toThrow( - '$.policies.protected.bypass_actors[0].actor_type', - ); - }); - - it('rejects unsupported repository metadata keys', () => { - const config = structuredClone(validConfig) as any; - config.repositories['ExampleOrg/project'].metadata.typo = true; - - expect(() => validateGovernanceConfig(config)).toThrow( - '$.repositories.ExampleOrg/project.metadata.typo', - ); - }); - });