From 7a9721d8298335af4a5c73aa77bb766a96fef304 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 22 Oct 2026 20:40:26 +0000 Subject: [PATCH 1/2] fix: distinguish redacted bypass actors from ruleset drift Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- action.yml | 10 +++++----- src/cli-runner.ts | 15 +++++++++++--- src/github-client.ts | 2 ++ src/governance.ts | 4 ++++ src/ruleset-reconciler.ts | 17 ++++++++++++++-- tests/cli-runner.test.ts | 19 ++++++++++++++++++ tests/github-client.test.ts | 19 ++++++++++++++++++ tests/ruleset-reconciler.test.ts | 34 ++++++++++++++++++++++++++++++++ 8 files changed, 110 insertions(+), 10 deletions(-) diff --git a/action.yml b/action.yml index aecf811..56dac7e 100644 --- a/action.yml +++ b/action.yml @@ -74,13 +74,13 @@ runs: fi node src/cli.ts "${args[@]}" | tee "$RUNNER_TEMP/github-governance-result.txt" - if [[ "$ALLOW_DRIFT" == "true" ]] && grep -q '^DRIFT ' "$RUNNER_TEMP/github-governance-result.txt"; then - echo '::warning::Governance drift detected; review the plan before any manual apply' + if [[ "$ALLOW_DRIFT" == "true" ]] && grep -Eq '^(DRIFT|UNVERIFIED) ' "$RUNNER_TEMP/github-governance-result.txt"; then + echo '::warning::Governance has drift or bypass actors that need privileged verification' { - echo '### Repository governance drift' - echo 'Review these repositories before approving an apply:' + echo '### Repository governance plan' + echo 'DRIFT means confirmed differences; UNVERIFIED means GitHub concealed bypass actors.' echo '```text' - grep '^DRIFT ' "$RUNNER_TEMP/github-governance-result.txt" + grep -E '^(DRIFT|UNVERIFIED) ' "$RUNNER_TEMP/github-governance-result.txt" echo '```' } >> "$GITHUB_STEP_SUMMARY" fi diff --git a/src/cli-runner.ts b/src/cli-runner.ts index d96794b..2d2a46f 100644 --- a/src/cli-runner.ts +++ b/src/cli-runner.ts @@ -112,13 +112,19 @@ function writePlans( ): void { for (const plan of plans) { const changes = plan.changes.filter( - (change) => change.action !== 'unchanged', + (change) => change.action === 'create' || change.action === 'update', + ); + const unverified = plan.changes.filter( + (change) => change.action === 'unverified', ); const metadataDrift = plan.metadata?.action === 'update'; if (changes.length === 0 && !metadataDrift) { - output.log(`OK ${plan.repository}`); + output.log(`${unverified.length > 0 ? 'UNVERIFIED' : 'OK'} ${plan.repository}`); + for (const change of unverified) { + output.log(` - bypass actors hidden: ${change.current.name}`); + } continue; } @@ -126,6 +132,9 @@ function writePlans( for (const change of changes) { output.log(` - ${change.action}: ${change.desired.name}`); } + for (const change of unverified) { + output.log(` - bypass actors hidden: ${change.current.name}`); + } if (metadataDrift) { output.log(` - update metadata: ${plan.metadata!.fields.join(', ')}`); } @@ -137,7 +146,7 @@ function hasDrift( ): boolean { return plans.some( (plan) => - plan.changes.some((change) => change.action !== 'unchanged') || + plan.changes.some((change) => change.action === 'create' || change.action === 'update') || plan.metadata?.action === 'update', ); } diff --git a/src/github-client.ts b/src/github-client.ts index 092d345..5d99a18 100644 --- a/src/github-client.ts +++ b/src/github-client.ts @@ -333,6 +333,7 @@ function parseExistingRuleset(value: unknown): ExistingRepositoryRuleset { throw new Error('Invalid ruleset response from GitHub'); } + const bypassActorsVisible = Array.isArray(value.bypass_actors); const bypassActors = Array.isArray(value.bypass_actors) ? value.bypass_actors.map(parseBypassActor) : []; @@ -346,6 +347,7 @@ function parseExistingRuleset(value: unknown): ExistingRepositoryRuleset { target, enforcement, bypass_actors: bypassActors, + ...(bypassActorsVisible ? {} : { bypass_actors_redacted: true }), conditions: { ref_name: { include: stringArray(conditions.ref_name.include), diff --git a/src/governance.ts b/src/governance.ts index 2b4d36d..6fde4ad 100644 --- a/src/governance.ts +++ b/src/governance.ts @@ -106,6 +106,10 @@ export async function syncRepository( desiredRulesets, desiredMetadata, ); + const unverified = plan.changes.find((change) => change.action === 'unverified'); + if (unverified) { + throw new Error(`Cannot apply ruleset with hidden bypass actors: ${unverified.current.name}`); + } const desired = plan.changes.flatMap((change) => change.action === 'unchanged' ? [change.current] : [change.desired], ); diff --git a/src/ruleset-reconciler.ts b/src/ruleset-reconciler.ts index 71f231f..c14f685 100644 --- a/src/ruleset-reconciler.ts +++ b/src/ruleset-reconciler.ts @@ -6,6 +6,8 @@ import { rulesetsEqual } from './ruleset-normalizer.ts'; export type ExistingRepositoryRuleset = RepositoryRuleset & { id: number; + // GitHub hides bypass_actors unless the caller can write the ruleset. + bypass_actors_redacted?: boolean; }; export interface RepositoryRulesetClient { @@ -31,7 +33,8 @@ export type RulesetChange = current: ExistingRepositoryRuleset; desired: RepositoryRuleset; } - | { action: 'unchanged'; current: ExistingRepositoryRuleset }; + | { action: 'unchanged'; current: ExistingRepositoryRuleset } + | { action: 'unverified'; current: ExistingRepositoryRuleset; desired: RepositoryRuleset }; export function planRepositoryRulesets( existing: ExistingRepositoryRuleset[], @@ -46,7 +49,12 @@ export function planRepositoryRulesets( return { action: 'create', desired }; } - if (rulesetsEqual(current, desired)) { + if (current.bypass_actors_redacted) { + // Compare only the fields GitHub actually exposed; bypass must be verified by Apply. + if (rulesetsEqual({ ...current, bypass_actors: desired.bypass_actors }, desired)) { + return { action: 'unverified', current, desired }; + } + } else if (rulesetsEqual(current, desired)) { return { action: 'unchanged', current }; } @@ -76,6 +84,11 @@ export async function reconcileRepositoryRulesets( desiredRulesets, ); + const redacted = changes.find((change) => change.action === 'unverified'); + if (redacted) { + throw new Error(`Cannot apply with hidden bypass actors: ${redacted.current.name}`); + } + const result: ReconcileResult = { created: [], updated: [], diff --git a/tests/cli-runner.test.ts b/tests/cli-runner.test.ts index ab28c40..259102d 100644 --- a/tests/cli-runner.test.ts +++ b/tests/cli-runner.test.ts @@ -241,6 +241,25 @@ describe('runCli', () => { expect(modes).toEqual([true]); }); + it('reports bypass verification as incomplete without producing false drift', async () => { + const current: ExistingRepositoryRuleset = { + ...structuredClone(protectedBranches), + id: 11, bypass_actors: [], bypass_actors_redacted: true, + }; + const messages: string[] = []; + const code = await runCli( + ['--org', 'ExampleOrg'], + { GITHUB_TOKEN: 'token' }, + () => new FakeClient([current]), + { log: (message) => messages.push(message), error: () => undefined }, + configLoader, + ); + expect(code).toBe(0); + expect(messages).toContain('UNVERIFIED ExampleOrg/project'); + expect(messages).toContain(' - bypass actors hidden: Protect branches'); + expect(messages.some((m) => m.startsWith('DRIFT '))).toBe(false); + }); + it('uses apply mode only when explicitly requested', async () => { const messages: string[] = []; diff --git a/tests/github-client.test.ts b/tests/github-client.test.ts index 3d2d84c..a9db78c 100644 --- a/tests/github-client.test.ts +++ b/tests/github-client.test.ts @@ -262,6 +262,25 @@ describe('GitHubClient', () => { }); }); + it('marks bypass actors hidden by public API access instead of assuming there are none', async () => { + const requests: ExpectedRequest[] = [ + { + url: 'https://api.github.test/repos/ExampleOrg/project/rulesets?per_page=100', + response: Response.json([{ id: 7, source_type: 'Repository' }]), + }, + { + url: 'https://api.github.test/repos/ExampleOrg/project/rulesets/7', + response: Response.json({ + ...protectedBranchesFixture, id: 7, bypass_actors: null, + }), + }, + ]; + const client = new GitHubClient('read-token', fakeFetch(requests), 'https://api.github.test'); + expect(await client.list('ExampleOrg', 'project')).toMatchObject([ + { id: 7, bypass_actors: [], bypass_actors_redacted: true }, + ]); + }); + it('creates and updates repository rulesets with JSON bodies', async () => { const requests: ExpectedRequest[] = [ { diff --git a/tests/ruleset-reconciler.test.ts b/tests/ruleset-reconciler.test.ts index 9d85add..2bc9782 100644 --- a/tests/ruleset-reconciler.test.ts +++ b/tests/ruleset-reconciler.test.ts @@ -4,6 +4,7 @@ import { describe, expect, it } from 'vitest'; import { protectedBranchesFixture } from './ruleset-fixtures.js'; import { + planRepositoryRulesets, reconcileRepositoryRulesets, type ExistingRepositoryRuleset, type RepositoryRulesetClient, @@ -92,6 +93,39 @@ describe('reconcileRepositoryRulesets', () => { ]); }); + it('reports a ruleset as unverified when only its bypass actors are redacted', () => { + const current: ExistingRepositoryRuleset = { + ...structuredClone(protectedBranchesFixture), + id: 17, bypass_actors: [], bypass_actors_redacted: true, + }; + expect(planRepositoryRulesets([current], [protectedBranchesFixture])).toEqual([ + { action: 'unverified', current, desired: protectedBranchesFixture }, + ]); + }); + + it('keeps reporting observable drift when bypass actors are redacted', () => { + const current: ExistingRepositoryRuleset = { + ...structuredClone(protectedBranchesFixture), + id: 17, bypass_actors: [], bypass_actors_redacted: true, + enforcement: 'disabled', + }; + expect(planRepositoryRulesets([current], [protectedBranchesFixture])[0]).toMatchObject({ + action: 'update', id: 17, + }); + }); + + it('never writes a ruleset when bypass actor visibility is insufficient', async () => { + const client = new FakeClient([{ + ...structuredClone(protectedBranchesFixture), + id: 17, bypass_actors: [], bypass_actors_redacted: true, + }]); + await expect(reconcileRepositoryRulesets( + client, 'ExampleOrg', 'project', [protectedBranchesFixture], + )).rejects.toThrow('hidden bypass actors'); + expect(client.created).toEqual([]); + expect(client.updated).toEqual([]); + }); + it('does not delete unrelated rulesets', async () => { const unrelated: ExistingRepositoryRuleset = { ...structuredClone(protectedBranchesFixture), From 0cbd31b136c0a1dd7dcaf805924aacd3e9f0ba71 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Thu, 22 Oct 2026 20:43:55 +0000 Subject: [PATCH 2/2] test: fail closed on redacted bypass with visible drift Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- src/governance.ts | 6 ++++-- src/ruleset-reconciler.ts | 6 ++++-- tests/ruleset-reconciler.test.ts | 12 ++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/governance.ts b/src/governance.ts index 6fde4ad..1b4697c 100644 --- a/src/governance.ts +++ b/src/governance.ts @@ -106,8 +106,10 @@ export async function syncRepository( desiredRulesets, desiredMetadata, ); - const unverified = plan.changes.find((change) => change.action === 'unverified'); - if (unverified) { + const unverified = plan.changes.find( + (change) => change.action !== 'create' && change.current.bypass_actors_redacted, + ); + if (unverified && unverified.action !== 'create') { throw new Error(`Cannot apply ruleset with hidden bypass actors: ${unverified.current.name}`); } const desired = plan.changes.flatMap((change) => diff --git a/src/ruleset-reconciler.ts b/src/ruleset-reconciler.ts index c14f685..643c673 100644 --- a/src/ruleset-reconciler.ts +++ b/src/ruleset-reconciler.ts @@ -84,8 +84,10 @@ export async function reconcileRepositoryRulesets( desiredRulesets, ); - const redacted = changes.find((change) => change.action === 'unverified'); - if (redacted) { + const redacted = changes.find( + (change) => change.action !== 'create' && change.current.bypass_actors_redacted, + ); + if (redacted && redacted.action !== 'create') { throw new Error(`Cannot apply with hidden bypass actors: ${redacted.current.name}`); } diff --git a/tests/ruleset-reconciler.test.ts b/tests/ruleset-reconciler.test.ts index 2bc9782..ca9bd15 100644 --- a/tests/ruleset-reconciler.test.ts +++ b/tests/ruleset-reconciler.test.ts @@ -114,6 +114,18 @@ describe('reconcileRepositoryRulesets', () => { }); }); + it('fails closed even when visible ruleset fields drift and bypass is redacted', async () => { + const client = new FakeClient([{ + ...structuredClone(protectedBranchesFixture), + enforcement: 'disabled', + id: 18, bypass_actors: [], bypass_actors_redacted: true, + }]); + await expect(reconcileRepositoryRulesets( + client, 'ExampleOrg', 'project', [protectedBranchesFixture], + )).rejects.toThrow('hidden bypass actors'); + expect(client.updated).toEqual([]); + }); + it('never writes a ruleset when bypass actor visibility is insufficient', async () => { const client = new FakeClient([{ ...structuredClone(protectedBranchesFixture),