diff --git a/lambdas/functions/control-plane/src/github/auth.test.ts b/lambdas/functions/control-plane/src/github/auth.test.ts index 80b4314182..55026fa322 100644 --- a/lambdas/functions/control-plane/src/github/auth.test.ts +++ b/lambdas/functions/control-plane/src/github/auth.test.ts @@ -3,6 +3,7 @@ import { StrategyOptions } from '@octokit/auth-app/dist-types/types'; import { request } from '@octokit/request'; import { RequestInterface, RequestParameters } from '@octokit/types'; import { getParameter } from '@aws-github-runner/aws-ssm-util'; +import { generateKeyPairSync } from 'node:crypto'; import * as nock from 'nock'; import { createGithubAppAuth, createOctokitClient } from './auth'; @@ -77,23 +78,12 @@ describe('Test createGithubAppAuth', () => { process.env.ENVIRONMENT = ENVIRONMENT; }); - it('Creates auth object with line breaks in SSH key.', async () => { + it('Creates auth object with createJwt callback including jti claim', async () => { // Arrange - const authOptions = { - appId: parseInt(GITHUB_APP_ID), - privateKey: `${decryptedValue} -${decryptedValue}`, - installationId, - }; - - const b64PrivateKeyWithLineBreaks = Buffer.from(decryptedValue + '\n' + decryptedValue, 'binary').toString( - 'base64', - ); - mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64PrivateKeyWithLineBreaks); + mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); - // Add the required hook method to make it compatible with AuthInterface const mockWithHook = Object.assign(mockedAuth, { hook: vi.fn() }); mockedCreatAppAuth.mockReturnValue(mockWithHook); @@ -102,21 +92,57 @@ ${decryptedValue}`, // Assert expect(mockedCreatAppAuth).toBeCalledTimes(1); - expect(mockedCreatAppAuth).toBeCalledWith({ ...authOptions }); + const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record; + expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID)); + expect(callArgs.createJwt).toBeTypeOf('function'); + expect(callArgs).not.toHaveProperty('privateKey'); + expect(callArgs.installationId).toBe(installationId); + }); + + it('createJwt callback produces unique JWTs with jti', async () => { + // Arrange — need a real RSA key since createJwt actually signs + const { privateKey } = generateKeyPairSync('rsa', { + modulusLength: 2048, + privateKeyEncoding: { type: 'pkcs8', format: 'pem' }, + publicKeyEncoding: { type: 'spki', format: 'pem' }, + }); + const b64Key = Buffer.from(privateKey as string).toString('base64'); + + mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64Key); + + let capturedCreateJwt: (appId: string | number, timeDifference?: number) => Promise<{ jwt: string }>; + mockedCreatAppAuth.mockImplementation((opts: StrategyOptions) => { + capturedCreateJwt = (opts as Record).createJwt as typeof capturedCreateJwt; + const mockedAuth = vi.fn().mockResolvedValue({ token }); + return Object.assign(mockedAuth, { hook: vi.fn() }); + }); + + // Act + await createGithubAppAuth(installationId); + + // Generate two JWTs and verify they are different (jti makes them unique) + const jwt1 = await capturedCreateJwt!(1); + const jwt2 = await capturedCreateJwt!(1); + + // Assert — JWTs must differ even when generated in the same second + expect(jwt1.jwt).not.toBe(jwt2.jwt); + + // Verify JWT structure: header.payload.signature + const parts = jwt1.jwt.split('.'); + expect(parts).toHaveLength(3); + const payload = JSON.parse(Buffer.from(parts[1], 'base64url').toString()); + expect(payload).toHaveProperty('jti'); + expect(payload).toHaveProperty('iat'); + expect(payload).toHaveProperty('exp'); + expect(payload).toHaveProperty('iss'); }); it('Creates auth object for public GitHub', async () => { // Arrange - const authOptions = { - appId: parseInt(GITHUB_APP_ID), - privateKey: decryptedValue, - installationId, - }; mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); - // Add the required hook method to make it compatible with AuthInterface const mockWithHook = Object.assign(mockedAuth, { hook: vi.fn() }); mockedCreatAppAuth.mockReturnValue(mockWithHook); @@ -128,7 +154,10 @@ ${decryptedValue}`, expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME); expect(mockedCreatAppAuth).toBeCalledTimes(1); - expect(mockedCreatAppAuth).toBeCalledWith({ ...authOptions }); + const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record; + expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID)); + expect(callArgs.createJwt).toBeTypeOf('function'); + expect(callArgs.installationId).toBe(installationId); expect(mockedAuth).toBeCalledWith({ type: authType }); expect(result.token).toBe(token); }); @@ -142,13 +171,6 @@ ${decryptedValue}`, () => mockedRequestInterface as RequestInterface, ); - const authOptions = { - appId: parseInt(GITHUB_APP_ID), - privateKey: decryptedValue, - installationId, - request: mockedRequestInterface.mockImplementation(() => ({ baseUrl: githubServerUrl })), - }; - mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); @@ -165,7 +187,11 @@ ${decryptedValue}`, expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME); expect(mockedCreatAppAuth).toBeCalledTimes(1); - expect(mockedCreatAppAuth).toBeCalledWith(authOptions); + const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record; + expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID)); + expect(callArgs.createJwt).toBeTypeOf('function'); + expect(callArgs.installationId).toBe(installationId); + expect(callArgs.request).toBeDefined(); expect(mockedAuth).toBeCalledWith({ type: authType }); expect(result.token).toBe(token); }); @@ -181,16 +207,9 @@ ${decryptedValue}`, const installationId = undefined; - const authOptions = { - appId: parseInt(GITHUB_APP_ID), - privateKey: decryptedValue, - request: mockedRequestInterface.mockImplementation(() => ({ baseUrl: githubServerUrl })), - }; - mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64); const mockedAuth = vi.fn(); mockedAuth.mockResolvedValue({ token }); - // Add the required hook method to make it compatible with AuthInterface const mockWithHook = Object.assign(mockedAuth, { hook: vi.fn() }); mockedCreatAppAuth.mockReturnValue(mockWithHook); @@ -202,7 +221,11 @@ ${decryptedValue}`, expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME); expect(mockedCreatAppAuth).toBeCalledTimes(1); - expect(mockedCreatAppAuth).toBeCalledWith(authOptions); + const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record; + expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID)); + expect(callArgs.createJwt).toBeTypeOf('function'); + expect(callArgs).not.toHaveProperty('installationId'); + expect(callArgs.request).toBeDefined(); expect(mockedAuth).toBeCalledWith({ type: authType }); expect(result.token).toBe(token); }); diff --git a/lambdas/functions/control-plane/src/github/auth.ts b/lambdas/functions/control-plane/src/github/auth.ts index d529cc81e8..2d99b5979a 100644 --- a/lambdas/functions/control-plane/src/github/auth.ts +++ b/lambdas/functions/control-plane/src/github/auth.ts @@ -12,10 +12,11 @@ type AuthInterface = { }; type StrategyOptions = { appId: number; - privateKey: string; + createJwt: (appId: string | number, timeDifference?: number) => Promise<{ jwt: string; expiresAt: string }>; installationId?: number; request?: RequestInterface; }; +import { createSign, randomUUID } from 'node:crypto'; import { request } from '@octokit/request'; import { Octokit } from '@octokit/rest'; import { throttling } from '@octokit/plugin-throttling'; @@ -69,20 +70,36 @@ export async function createGithubInstallationAuth( return auth(installationAuthOptions); } +function signJwt(payload: Record, privateKey: string): string { + const header = { alg: 'RS256', typ: 'JWT' }; + const encode = (obj: unknown) => Buffer.from(JSON.stringify(obj)).toString('base64url'); + const message = `${encode(header)}.${encode(payload)}`; + const signature = createSign('RSA-SHA256').update(message).sign(privateKey, 'base64url'); + return `${message}.${signature}`; +} + async function createAuth(installationId: number | undefined, ghesApiUrl: string): Promise { const appId = parseInt(await getParameter(process.env.PARAMETER_GITHUB_APP_ID_NAME)); - let authOptions: StrategyOptions = { - appId, - privateKey: Buffer.from( - await getParameter(process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME), - 'base64', - // replace literal \n characters with new lines to allow the key to be stored as a - // single line variable. This logic should match how the GitHub Terraform provider - // processes private keys to retain compatibility between the projects - ) - .toString() - .replace('/[\\n]/g', String.fromCharCode(10)), + // replace literal \n characters with new lines to allow the key to be stored as a + // single line variable. This logic should match how the GitHub Terraform provider + // processes private keys to retain compatibility between the projects + const privateKey = Buffer.from(await getParameter(process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME), 'base64') + .toString() + .replace('/[\\n]/g', String.fromCharCode(10)); + + // Use a custom createJwt callback to include a jti (JWT ID) claim in every token. + // Without this, concurrent Lambda invocations generating JWTs within the same second + // produce byte-identical tokens (same iat, exp, iss), which GitHub rejects as duplicates. + // See: https://github.com/github-aws-runners/terraform-aws-github-runner/issues/5025 + const createJwt = async (appId: string | number, timeDifference?: number) => { + const now = Math.floor(Date.now() / 1000) + (timeDifference ?? 0); + const iat = now - 30; + const exp = iat + 600; + const jwt = signJwt({ iat, exp, iss: appId, jti: randomUUID() }, privateKey); + return { jwt, expiresAt: new Date(exp * 1000).toISOString() }; }; + + let authOptions: StrategyOptions = { appId, createJwt }; if (installationId) authOptions = { ...authOptions, installationId }; logger.debug(`GHES API URL: ${ghesApiUrl}`);