From 76fcdc8ae250c81aeb055533c2741385ac9f0b7b Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Wed, 4 Nov 2026 18:07:12 +0000 Subject: [PATCH] fix(governance): separate public content reads from ruleset administration Keep the GitHub App scoped to Administration (write), and use a separate read-only token for verified public content classification. Fail closed on private or unverified repository metadata; retain protected Apply review. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- action.yml | 4 +++ docs/adoption.md | 5 ++- docs/configuration.md | 16 ++++++--- docs/security.md | 7 ++++ src/cli-runner.ts | 5 +-- src/cli.ts | 6 ++-- src/github-client.ts | 29 +++++++++++++--- tests/cli-runner.test.ts | 16 +++++++++ tests/github-client.test.ts | 66 +++++++++++++++++++++++++++++++++++++ 9 files changed, 139 insertions(+), 15 deletions(-) diff --git a/action.yml b/action.yml index 0d839a0..be49273 100644 --- a/action.yml +++ b/action.yml @@ -8,6 +8,9 @@ inputs: token: description: Read-only token for public planning, administration write token for Apply required: true + contents-token: + description: Optional separate read-only token for public repository content classification during privileged Apply + required: false repository: description: Repository in OWNER/REPO form (exclusive with organization) required: false @@ -48,6 +51,7 @@ runs: shell: bash env: GITHUB_TOKEN: ${{ inputs.token }} + GOVERNANCE_CONTENTS_TOKEN: ${{ inputs.contents-token }} TARGET_REPOSITORY: ${{ inputs.repository }} TARGET_ORGANIZATION: ${{ inputs.organization }} CONFIG_PATH: ${{ inputs.config }} diff --git a/docs/adoption.md b/docs/adoption.md index 2bf13fd..999c5ed 100644 --- a/docs/adoption.md +++ b/docs/adoption.md @@ -29,7 +29,10 @@ Use a GitHub App owned by or trusted by the organization being managed. The current public-repository model needs repository administration access. GitHub requires administration write access for complete ruleset responses, -including bypass actors, even during dry-run. +including bypass actors, even during dry-run. To classify public repositories +without broadening the App installation, pass a separate caller read-only token +using `contents-token: ${{ github.token }}`. The App then needs repository +Administration (write), not Contents (read). Keep the App private key in an environment secret. Do not commit it or move it to the shared governance repository. diff --git a/docs/configuration.md b/docs/configuration.md index 964cd44..9d2f57a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -372,11 +372,17 @@ ruleset responses without write access. Dry-run safety therefore comes from the engine's explicit `apply=false` behavior rather than from reducing the token to administration read-only access. -The current managed scope is public, non-archived repositories. Public content -probes used by `file_exists` have been validated without requesting a separate -`contents: read` permission from the GitHub App installation. If private or -internal repository management is added later, review the permission model -explicitly. +The current managed scope is public, non-archived repositories. A privileged +caller may supply the optional action input `contents-token` with a separate +read-only token (for example `${{ github.token }}`). The GitHub App token then +needs only `administration: write`, while content probes use the separate token. +The engine verifies public repository metadata before using the separate token, +and rejects content API errors instead of silently treating them as missing +files. Ruleset reads and writes always use the GitHub App token. + +A separate contents token must not be used for private or internal repositories: +this mode deliberately fails closed. If private or internal repository management +is added later, review the permission model explicitly. See also: diff --git a/docs/security.md b/docs/security.md index 6af0fa9..77d4b78 100644 --- a/docs/security.md +++ b/docs/security.md @@ -42,6 +42,13 @@ Mutation-capable credentials are not shared across the matrix. The private key must remain in the caller's protected environment. +For public repositories, the caller can provide a separate `contents-token` +using its short-lived, read-only `GITHUB_TOKEN`. Only public repository content +probes use that token; protected ruleset reads/writes and branch-protection +preflights continue using the privileged App token. The engine confirms public +repository metadata before probing, and rejects non-404 errors. This avoids +unnecessarily granting the ruleset-management App Contents permission. + ## Workflow controls Production callers should: diff --git a/src/cli-runner.ts b/src/cli-runner.ts index cdabb33..6d11de7 100644 --- a/src/cli-runner.ts +++ b/src/cli-runner.ts @@ -18,6 +18,7 @@ import { export type CliEnvironment = { GITHUB_TOKEN?: string | undefined; + GOVERNANCE_CONTENTS_TOKEN?: string | undefined; }; export type CliOutput = { @@ -32,7 +33,7 @@ export type GovernanceConfigLoader = ( export async function runCli( args: string[], environment: CliEnvironment, - clientFactory: (token: string, publicDiscovery: boolean) => GovernanceClient, + clientFactory: (token: string, publicDiscovery: boolean, contentsToken?: string) => GovernanceClient, output: CliOutput, configLoader: GovernanceConfigLoader = loadGovernanceConfig, ): Promise { @@ -65,7 +66,7 @@ export async function runCli( return 2; } - const client = clientFactory(token, publicDiscovery); + const client = clientFactory(token, publicDiscovery, environment.GOVERNANCE_CONTENTS_TOKEN || undefined); const config = await configLoader(configPath); const resolveRulesets = (repository: { owner: string; diff --git a/src/cli.ts b/src/cli.ts index 75afd83..738581b 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -8,9 +8,9 @@ import { GitHubClient } from './github-client.ts'; const code = await runCli( process.argv.slice(2), - { GITHUB_TOKEN: process.env.GITHUB_TOKEN }, - (token, publicDiscovery) => - new GitHubClient(token, fetch, 'https://api.github.com', publicDiscovery ? 'public' : 'installation'), + { GITHUB_TOKEN: process.env.GITHUB_TOKEN, GOVERNANCE_CONTENTS_TOKEN: process.env.GOVERNANCE_CONTENTS_TOKEN }, + (token, publicDiscovery, contentsToken) => + new GitHubClient(token, fetch, 'https://api.github.com', publicDiscovery ? 'public' : 'installation', contentsToken), { log: console.log, error: console.error, diff --git a/src/github-client.ts b/src/github-client.ts index 8d1627b..440ff6e 100644 --- a/src/github-client.ts +++ b/src/github-client.ts @@ -39,6 +39,8 @@ export class GitHubClient implements GitHubContentProbe, RepositoryRulesetClient { private readonly token: string; + private readonly contentsToken: string; + private readonly publicContentsRepositories = new Set(); private readonly fetchImpl: FetchLike; private readonly apiUrl: string; private readonly discovery: 'installation' | 'public'; @@ -48,8 +50,10 @@ export class GitHubClient fetchImpl: FetchLike = fetch, apiUrl = 'https://api.github.com', discovery: 'installation' | 'public' = 'installation', + contentsToken = token, ) { this.token = token; + this.contentsToken = contentsToken; this.fetchImpl = fetchImpl; this.apiUrl = apiUrl; this.discovery = discovery; @@ -87,6 +91,10 @@ export class GitHubClient throw new Error(`Invalid repository response for ${owner}/${repository}`); } + if (visibility === 'public' && !archived) { + this.publicContentsRepositories.add(`${owner}/${repository}`); + } + return { owner: login, name, @@ -148,10 +156,14 @@ export class GitHubClient } } - return result.filter( + const eligible = result.filter( (repository) => repository.visibility === 'public' && !repository.archived, ); + for (const repository of eligible) { + this.publicContentsRepositories.add(`${repository.owner}/${repository.name}`); + } + return eligible; } async updateRepositoryMetadata( @@ -185,6 +197,15 @@ export class GitHubClient repository: string, path: string, ): Promise { + // A different read token is accepted only after the privileged client + // has confirmed that this is an eligible public repository. Otherwise + // an inaccessible private path could look like a missing appinfo file. + if ( + this.contentsToken !== this.token && + !this.publicContentsRepositories.has(`${owner}/${repository}`) + ) { + throw new Error(`Cannot probe contents without verified public repository metadata: ${owner}/${repository}`); + } const response = await this.fetchImpl( this.url( `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repository)}/contents/${path @@ -193,7 +214,7 @@ export class GitHubClient .join('/')}`, ), { - headers: this.headers(), + headers: this.headers(this.contentsToken), }, ); @@ -317,10 +338,10 @@ export class GitHubClient return (await response.json()) as T; } - private headers(): Record { + private headers(token = this.token): Record { return { Accept: 'application/vnd.github+json', - Authorization: `Bearer ${this.token}`, + Authorization: `Bearer ${token}`, 'X-GitHub-Api-Version': '2026-03-10', }; } diff --git a/tests/cli-runner.test.ts b/tests/cli-runner.test.ts index d8ace2b..71a6da2 100644 --- a/tests/cli-runner.test.ts +++ b/tests/cli-runner.test.ts @@ -99,6 +99,22 @@ describe('runCli', () => { ]); }); + it('passes the optional separate public contents token to the client', async () => { + const received: Array<[string, boolean, string | undefined]> = []; + const code = await runCli( + ['--repo', 'ExampleOrg/project', '--allow-drift'], + { GITHUB_TOKEN: 'admin-token', GOVERNANCE_CONTENTS_TOKEN: 'contents-token' }, + (token, publicDiscovery, contentsToken) => { + received.push([token, publicDiscovery, contentsToken]); + return new FakeClient(); + }, + { log: () => undefined, error: () => undefined }, + configLoader, + ); + expect(code).toBe(0); + expect(received).toEqual([['admin-token', false, 'contents-token']]); + }); + it('requires a GitHub token', async () => { const errors: string[] = []; diff --git a/tests/github-client.test.ts b/tests/github-client.test.ts index c038ce1..c358fe2 100644 --- a/tests/github-client.test.ts +++ b/tests/github-client.test.ts @@ -416,3 +416,69 @@ describe('GitHubClient', () => { }); }); + + +describe('GitHubClient split public read and privileged administration tokens', () => { + it('uses a separate public contents token but keeps administration on the App token', async () => { + const observed: string[] = []; + const fetcher: typeof fetch = async (input, init) => { + const path = new URL(String(input)).pathname; + const authorization = new Headers(init?.headers).get('Authorization'); + observed.push(`${path}:${authorization}`); + if (path === '/repos/ExampleOrg/app') { + return Response.json({ + owner: { login: 'ExampleOrg' }, name: 'app', + visibility: 'public', archived: false, default_branch: 'main', + }); + } + if (path === '/repos/ExampleOrg/app/contents/appinfo/info.xml') { + return Response.json({ type: 'file' }); + } + if (String(input).endsWith('/repos/ExampleOrg/app/rulesets?per_page=100')) { + return Response.json([]); + } + throw new Error(`Unexpected request: ${String(input)}`); + }; + const client = new GitHubClient('admin-token', fetcher, 'https://api.github.test', 'installation', 'contents-token'); + await client.getRepository('ExampleOrg', 'app'); + expect(await client.exists('ExampleOrg', 'app', 'appinfo/info.xml')).toBe(true); + expect(await client.list('ExampleOrg', 'app')).toEqual([]); + expect(observed).toEqual([ + '/repos/ExampleOrg/app:Bearer admin-token', + '/repos/ExampleOrg/app/contents/appinfo/info.xml:Bearer contents-token', + '/repos/ExampleOrg/app/rulesets:Bearer admin-token', + ]); + }); + + it('refuses a separate public read token for unverified or private repositories', async () => { + const requests: ExpectedRequest[] = [{ + url: 'https://api.github.test/repos/ExampleOrg/internal', + response: Response.json({ + owner: { login: 'ExampleOrg' }, name: 'internal', + visibility: 'private', archived: false, default_branch: 'main', + }), + }]; + const client = new GitHubClient('admin-token', fakeFetch(requests), 'https://api.github.test', 'installation', 'contents-token'); + await expect(client.exists('ExampleOrg', 'unknown', 'appinfo/info.xml')) + .rejects.toThrow('verified public repository metadata'); + await client.getRepository('ExampleOrg', 'internal'); + await expect(client.exists('ExampleOrg', 'internal', 'appinfo/info.xml')) + .rejects.toThrow('verified public repository metadata'); + expect(requests).toHaveLength(0); + }); + + it('fails closed when the separate public contents token is forbidden', async () => { + const requests: ExpectedRequest[] = [ + { url: 'https://api.github.test/repos/ExampleOrg/app', response: Response.json({ + owner: { login: 'ExampleOrg' }, name: 'app', visibility: 'public', archived: false, + }) }, + { url: 'https://api.github.test/repos/ExampleOrg/app/contents/appinfo/info.xml', + response: new Response(null, { status: 403 }) }, + ]; + const client = new GitHubClient('admin-token', fakeFetch(requests), 'https://api.github.test', 'installation', 'contents-token'); + await client.getRepository('ExampleOrg', 'app'); + await expect(client.exists('ExampleOrg', 'app', 'appinfo/info.xml')) + .rejects.toThrow('HTTP 403'); + expect(requests).toHaveLength(0); + }); +});