diff --git a/.github/scripts/get-bot-token.mjs b/.github/scripts/get-bot-token.mjs index 52bf9f9ba4..054d5817c7 100644 --- a/.github/scripts/get-bot-token.mjs +++ b/.github/scripts/get-bot-token.mjs @@ -1,8 +1,10 @@ #!/usr/bin/env node /** * get-bot-token.mjs - * Generates a short-lived GitHub installation token for the commitperclip app. - * Reads COMMITPERCLIP_KEY env var (PEM content of private key). + * Generates a short-lived GitHub installation token. + * + * Generic callers set GITHUB_APP_ID and GITHUB_APP_PRIVATE_KEY. Existing + * commitperclip callers may continue to set COMMITPERCLIP_KEY. * Prints the token to stdout. * * Also exports: generateJWT(privateKey), ghFetch(path, token, options) @@ -11,13 +13,13 @@ import { createSign } from 'node:crypto'; import { fileURLToPath } from 'node:url'; -const APP_ID = '3718661'; +const COMMITPERCLIP_APP_ID = '3718661'; const OWNER_PATTERN = /^[a-zA-Z0-9_.-]+$/; const REPO_PATTERN = /^[a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+$/; -export function generateJWT(privateKey) { +export function generateJWT(privateKey, appId = COMMITPERCLIP_APP_ID) { const now = Math.floor(Date.now() / 1000); - const payload = { iat: now - 10, exp: now + 60, iss: APP_ID }; + const payload = { iat: now - 10, exp: now + 60, iss: appId }; const header = Buffer.from(JSON.stringify({ alg: 'RS256', typ: 'JWT' })).toString('base64url'); const body = Buffer.from(JSON.stringify(payload)).toString('base64url'); const data = `${header}.${body}`; @@ -59,7 +61,7 @@ export async function ghFetch(path, token, options = {}) { } } -export async function resolveInstallationId(fetchInstallation, token, repo, owner) { +export async function resolveInstallationId(fetchInstallation, token, repo, owner, appName = 'GitHub App') { if (repo) { if (!REPO_PATTERN.test(repo)) { throw new Error('ERROR: GH_REPO/GITHUB_REPOSITORY must be in owner/repo format.'); @@ -71,9 +73,7 @@ export async function resolveInstallationId(fetchInstallation, token, repo, owne const installations = await fetchInstallation('/app/installations', token); if (!installations.length) { - throw new Error( - 'ERROR: No installations found for commitperclip. Install URL: https://github.com/apps/commitperclip/installations/new' - ); + throw new Error(`ERROR: No installations found for ${appName}.`); } if (owner) { @@ -95,23 +95,45 @@ export async function resolveInstallationId(fetchInstallation, token, repo, owne } throw new Error( - 'ERROR: Multiple commitperclip installations found. Set GH_REPO or GITHUB_REPOSITORY so the correct installation can be selected.' + `ERROR: Multiple ${appName} installations found. Set GH_REPO or GITHUB_REPOSITORY so the correct installation can be selected.` ); } +export function resolveAppCredentials(environment) { + const explicitAppId = environment.GITHUB_APP_ID; + const explicitPrivateKey = environment.GITHUB_APP_PRIVATE_KEY; + if (Boolean(explicitAppId) !== Boolean(explicitPrivateKey)) { + throw new Error('ERROR: GITHUB_APP_ID and GITHUB_APP_PRIVATE_KEY must be set together.'); + } + if (explicitAppId && explicitPrivateKey) { + return { + appId: explicitAppId, + privateKey: explicitPrivateKey, + appName: environment.GITHUB_APP_NAME ?? 'GitHub App', + }; + } + if (!environment.COMMITPERCLIP_KEY) { + throw new Error('ERROR: GITHUB_APP_PRIVATE_KEY or COMMITPERCLIP_KEY env var not set.'); + } + return { + appId: COMMITPERCLIP_APP_ID, + privateKey: environment.COMMITPERCLIP_KEY, + appName: 'commitperclip', + }; +} + async function main() { - const privateKey = process.env.COMMITPERCLIP_KEY; - if (!privateKey) { - console.error('ERROR: COMMITPERCLIP_KEY env var not set.'); - console.error('Add to ~/.bash_profile: export COMMITPERCLIP_KEY="$(cat ~/.config/commitperclip/private-key.pem)"'); + const { appId, privateKey, appName } = resolveAppCredentials(process.env); + if (!/^\d+$/.test(appId)) { + console.error('ERROR: GITHUB_APP_ID must be a numeric GitHub App ID.'); process.exit(1); } - const jwt = generateJWT(privateKey); + const jwt = generateJWT(privateKey, appId); const repo = process.env.GH_REPO ?? process.env.GITHUB_REPOSITORY; const owner = process.env.GITHUB_REPOSITORY_OWNER ?? repo?.split('/')[0]; - const installationId = await resolveInstallationId(ghFetch, jwt, repo, owner); + const installationId = await resolveInstallationId(ghFetch, jwt, repo, owner, appName); const { token } = await ghFetch( `/app/installations/${installationId}/access_tokens`, diff --git a/.github/scripts/tests/get-bot-token.test.mjs b/.github/scripts/tests/get-bot-token.test.mjs index e17953f1ad..eb38afad80 100644 --- a/.github/scripts/tests/get-bot-token.test.mjs +++ b/.github/scripts/tests/get-bot-token.test.mjs @@ -1,6 +1,42 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { resolveInstallationId } from '../get-bot-token.mjs'; +import { generateKeyPairSync } from 'node:crypto'; +import { + generateJWT, + resolveAppCredentials, + resolveInstallationId, +} from '../get-bot-token.mjs'; + +test('generateJWT: uses an explicitly selected GitHub App ID as the issuer', () => { + const { privateKey } = generateKeyPairSync('rsa', { modulusLength: 2048 }); + const token = generateJWT(privateKey, '987654'); + const payload = JSON.parse(Buffer.from(token.split('.')[1], 'base64url').toString('utf8')); + + assert.equal(payload.iss, '987654'); +}); + +test('resolveAppCredentials: selects an explicit app without mixing legacy credentials', () => { + assert.deepEqual(resolveAppCredentials({ + GITHUB_APP_ID: '987654', + GITHUB_APP_PRIVATE_KEY: 'dedicated-key', + GITHUB_APP_NAME: 'paperclip-evals', + COMMITPERCLIP_KEY: 'legacy-key', + }), { + appId: '987654', + privateKey: 'dedicated-key', + appName: 'paperclip-evals', + }); +}); + +test('resolveAppCredentials: rejects a partially configured explicit app', () => { + assert.throws( + () => resolveAppCredentials({ + GITHUB_APP_ID: '987654', + COMMITPERCLIP_KEY: 'legacy-key', + }), + /GITHUB_APP_ID and GITHUB_APP_PRIVATE_KEY must be set together/ + ); +}); test('resolveInstallationId: uses the repo installation endpoint when repo context is available', async () => { const seenPaths = []; @@ -28,6 +64,6 @@ test('resolveInstallationId: rejects ambiguous installations without repo or own { id: 1, account: { login: 'org-one' } }, { id: 2, account: { login: 'org-two' } }, ]), 'jwt'), - /Multiple commitperclip installations found/ + /Multiple GitHub App installations found/ ); }); diff --git a/.github/workflows/runner-protocol-live-evals.yml b/.github/workflows/runner-protocol-live-evals.yml index f659f5ec65..c0378e0efc 100644 --- a/.github/workflows/runner-protocol-live-evals.yml +++ b/.github/workflows/runner-protocol-live-evals.yml @@ -104,7 +104,9 @@ jobs: - name: Generate private eval-repository token id: evals_token env: - COMMITPERCLIP_KEY: ${{ secrets.COMMITPERCLIP_KEY }} + GITHUB_APP_ID: ${{ vars.PAPERCLIP_EVALS_APP_ID }} + GITHUB_APP_PRIVATE_KEY: ${{ secrets.PAPERCLIP_EVALS_APP_PRIVATE_KEY }} + GITHUB_APP_NAME: paperclip-evals GH_REPO: paperclipai/paperclip-evals run: | set -euo pipefail @@ -176,7 +178,9 @@ jobs: - name: Generate private eval-repository token id: evals_token env: - COMMITPERCLIP_KEY: ${{ secrets.COMMITPERCLIP_KEY }} + GITHUB_APP_ID: ${{ vars.PAPERCLIP_EVALS_APP_ID }} + GITHUB_APP_PRIVATE_KEY: ${{ secrets.PAPERCLIP_EVALS_APP_PRIVATE_KEY }} + GITHUB_APP_NAME: paperclip-evals GH_REPO: paperclipai/paperclip-evals run: | set -euo pipefail @@ -331,7 +335,9 @@ jobs: - name: Generate private eval-repository token id: evals_token env: - COMMITPERCLIP_KEY: ${{ secrets.COMMITPERCLIP_KEY }} + GITHUB_APP_ID: ${{ vars.PAPERCLIP_EVALS_APP_ID }} + GITHUB_APP_PRIVATE_KEY: ${{ secrets.PAPERCLIP_EVALS_APP_PRIVATE_KEY }} + GITHUB_APP_NAME: paperclip-evals GH_REPO: paperclipai/paperclip-evals run: | set -euo pipefail @@ -501,7 +507,9 @@ jobs: - name: Generate private eval-repository token id: evals_token env: - COMMITPERCLIP_KEY: ${{ secrets.COMMITPERCLIP_KEY }} + GITHUB_APP_ID: ${{ vars.PAPERCLIP_EVALS_APP_ID }} + GITHUB_APP_PRIVATE_KEY: ${{ secrets.PAPERCLIP_EVALS_APP_PRIVATE_KEY }} + GITHUB_APP_NAME: paperclip-evals GH_REPO: paperclipai/paperclip-evals run: | set -euo pipefail diff --git a/packages/paperclip-runner/docs/runner-protocol-live-evals.md b/packages/paperclip-runner/docs/runner-protocol-live-evals.md index 6ed8f6f214..a8c01a7de7 100644 --- a/packages/paperclip-runner/docs/runner-protocol-live-evals.md +++ b/packages/paperclip-runner/docs/runner-protocol-live-evals.md @@ -37,9 +37,13 @@ from the default branch and provide: attempt explicitly reports a retryable infrastructure failure. The authorization job resolves the Paperclip branch to a commit and verifies -the supplied eval commit before any checkout. A short-lived bot token generated -from `COMMITPERCLIP_KEY` authorizes each checkout of the private eval repository; -the token is masked and is never forwarded to a provider process. The workflow +the supplied eval commit before any checkout. A short-lived installation token +from the dedicated `paperclip-evals` GitHub App authorizes each checkout of the +private eval repository; the token is masked and is never forwarded to a +provider process. The app is installed only on `paperclipai/paperclip-evals`, +with repository metadata and read-only contents access. The workflow reads its +numeric App ID from the `PAPERCLIP_EVALS_APP_ID` repository variable and its +private key from the `PAPERCLIP_EVALS_APP_PRIVATE_KEY` repository secret. It uses the same numeric actor allowlist, protected `runner-e2e-paid` environment, RunsOn fleet selector, and `RUNNER_E2E_MAX_PARALLEL` ceiling as the full-stack E2E workflow. Two balanced @@ -102,6 +106,11 @@ role admits only the `paperclipai/paperclip` repository's protected Scheduled runs additionally require `RUNNER_PROTOCOL_EVAL_NIGHTLY_ENABLED=true` and the pinned `RUNNER_PROTOCOL_EVALS_SHA` repository variable. +The dedicated App has no webhook, organization permissions, or repository +write permissions. Rotate its private key by adding the replacement key to the +repository secret, proving a workflow authorization/checkout, and only then +deleting the previous key in the App settings. + ## Reports and history Each cell uploads its immutable run directory to an access-controlled Actions diff --git a/packages/paperclip-runner/scripts/runner-protocol-eval-workflow-security.test.mjs b/packages/paperclip-runner/scripts/runner-protocol-eval-workflow-security.test.mjs index 0569facb0a..4df6b62668 100644 --- a/packages/paperclip-runner/scripts/runner-protocol-eval-workflow-security.test.mjs +++ b/packages/paperclip-runner/scripts/runner-protocol-eval-workflow-security.test.mjs @@ -61,7 +61,15 @@ test("resolves both repositories immutably and bounds total matrix concurrency", authorize, /repos\/paperclipai\/paperclip-evals\/commits\/\$EVALS_SHA/u, ); - assert.match(authorize, /COMMITPERCLIP_KEY/u); + assert.match( + authorize, + /GITHUB_APP_ID: \$\{\{ vars\.PAPERCLIP_EVALS_APP_ID \}\}/u, + ); + assert.match( + authorize, + /GITHUB_APP_PRIVATE_KEY: \$\{\{ secrets\.PAPERCLIP_EVALS_APP_PRIVATE_KEY \}\}/u, + ); + assert.doesNotMatch(workflow, /COMMITPERCLIP_KEY/u); assert.match(authorize, /GH_REPO: paperclipai\/paperclip-evals/u); assert.match( authorize, @@ -91,6 +99,16 @@ test("resolves both repositories immutably and bounds total matrix concurrency", ]; assert.equal(privateTokenSteps.length, 4); for (const tokenStep of privateTokenSteps) { + assert.match( + tokenStep.groups.body, + /^ {10}GITHUB_APP_ID: \$\{\{ vars\.PAPERCLIP_EVALS_APP_ID \}\}$/mu, + "every private-eval token must use the dedicated eval app ID", + ); + assert.match( + tokenStep.groups.body, + /^ {10}GITHUB_APP_PRIVATE_KEY: \$\{\{ secrets\.PAPERCLIP_EVALS_APP_PRIVATE_KEY \}\}$/mu, + "every private-eval token must use the dedicated eval app key", + ); assert.match( tokenStep.groups.body, /^ {10}GH_REPO: paperclipai\/paperclip-evals$/mu,