Skip to content

fix(lambda): add jti claim to GitHub App JWTs to prevent concurrent collisions #5056

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Brend-Smits merged 1 commit into from
Mar 6, 2026
Merged

fix(lambda): add jti claim to GitHub App JWTs to prevent concurrent collisions #5056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 60 additions & 37 deletions lambdas/functions/control-plane/src/github/auth.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand All @@ -102,21 +92,57 @@ ${decryptedValue}`,

// Assert
expect(mockedCreatAppAuth).toBeCalledTimes(1);
expect(mockedCreatAppAuth).toBeCalledWith({ ...authOptions });
const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record<string, unknown>;
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<string, unknown>).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);

Expand All @@ -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<string, unknown>;
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);
});
Expand All @@ -142,13 +171,6 @@ ${decryptedValue}`,
() => mockedRequestInterface as RequestInterface<object & RequestParameters>,
);

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 });
Expand All @@ -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<string, unknown>;
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);
});
Expand All @@ -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);

Expand All @@ -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<string, unknown>;
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);
});
Expand Down
41 changes: 29 additions & 12 deletions lambdas/functions/control-plane/src/github/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -69,20 +70,36 @@ export async function createGithubInstallationAuth(
return auth(installationAuthOptions);
}

function signJwt(payload: Record<string, unknown>, 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<AuthInterface> {
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}`);
Expand Down
Loading