Skip to content

Commit 07bd193

Browse files
jensenboxjensenbox
authored
fix(lambda): add jti claim to GitHub App JWTs to prevent concurrent collisions (#5056)
## Summary Fixes concurrent JWT collisions that cause silent job loss during burst workloads. When multiple scale-up Lambda invocations generate GitHub App JWTs within the same second, `universal-github-app-jwt` produces byte-identical tokens (same `iat`, `exp`, `iss`, no `jti`). GitHub rejects the duplicates, returning HTTP 404 on `POST /app/installations/{id}/access_tokens`, which triggers silent batch dropping. ### Root cause `universal-github-app-jwt` generates JWTs with only `{ iat, exp, iss }` claims. The `iat` uses seconds precision (`Math.floor(Date.now() / 1000)`). With the same App ID and private key, concurrent invocations within the same second produce identical tokens. ### Fix Replace `privateKey`-based auth with a custom `createJwt` callback — a first-class API in `@octokit/auth-app` v8.x that completely bypasses `universal-github-app-jwt`. The callback: - Signs JWTs using `node:crypto.createSign` (zero new dependencies) - Includes a `crypto.randomUUID()` `jti` claim, ensuring every token is unique - Preserves the existing `iat`/`exp` logic (30s safety margin, 10-minute expiry) - Properly forwards the `timeDifference` parameter for clock drift correction - Supports both PKCS#1 and PKCS#8 private key formats (via `node:crypto`) ### Changes - `lambdas/functions/control-plane/src/github/auth.ts` — replace `privateKey` with `createJwt` callback in `createAuth()` - `lambdas/functions/control-plane/src/github/auth.test.ts` — update tests to assert `createJwt` instead of `privateKey`, add test verifying unique JWTs with `jti` ### Test coverage - Existing tests updated to verify `createJwt` callback is passed instead of `privateKey` - New test generates two JWTs in rapid succession and verifies they differ (proving `jti` uniqueness) - New test validates JWT structure (header.payload.signature) and verifies `jti`, `iat`, `exp`, `iss` claims are present - All 343 control-plane tests pass Fixes #5025
1 parent 6dc97d5 commit 07bd193

2 files changed

Lines changed: 89 additions & 49 deletions

File tree

‎lambdas/functions/control-plane/src/github/auth.test.ts‎

Lines changed: 60 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { StrategyOptions } from '@octokit/auth-app/dist-types/types';
33
import { request } from '@octokit/request';
44
import { RequestInterface, RequestParameters } from '@octokit/types';
55
import { getParameter } from '@aws-github-runner/aws-ssm-util';
6+
import { generateKeyPairSync } from 'node:crypto';
67
import * as nock from 'nock';
78

89
import { createGithubAppAuth, createOctokitClient } from './auth';
@@ -77,23 +78,12 @@ describe('Test createGithubAppAuth', () => {
7778
process.env.ENVIRONMENT = ENVIRONMENT;
7879
});
7980

80-
it('Creates auth object with line breaks in SSH key.', async () => {
81+
it('Creates auth object with createJwt callback including jti claim', async () => {
8182
// Arrange
82-
const authOptions = {
83-
appId: parseInt(GITHUB_APP_ID),
84-
privateKey: `${decryptedValue}
85-
${decryptedValue}`,
86-
installationId,
87-
};
88-
89-
const b64PrivateKeyWithLineBreaks = Buffer.from(decryptedValue + '\n' + decryptedValue, 'binary').toString(
90-
'base64',
91-
);
92-
mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64PrivateKeyWithLineBreaks);
83+
mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64);
9384

9485
const mockedAuth = vi.fn();
9586
mockedAuth.mockResolvedValue({ token });
96-
// Add the required hook method to make it compatible with AuthInterface
9787
const mockWithHook = Object.assign(mockedAuth, { hook: vi.fn() });
9888
mockedCreatAppAuth.mockReturnValue(mockWithHook);
9989

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

10393
// Assert
10494
expect(mockedCreatAppAuth).toBeCalledTimes(1);
105-
expect(mockedCreatAppAuth).toBeCalledWith({ ...authOptions });
95+
const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record<string, unknown>;
96+
expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID));
97+
expect(callArgs.createJwt).toBeTypeOf('function');
98+
expect(callArgs).not.toHaveProperty('privateKey');
99+
expect(callArgs.installationId).toBe(installationId);
100+
});
101+
102+
it('createJwt callback produces unique JWTs with jti', async () => {
103+
// Arrange — need a real RSA key since createJwt actually signs
104+
const { privateKey } = generateKeyPairSync('rsa', {
105+
modulusLength: 2048,
106+
privateKeyEncoding: { type: 'pkcs8', format: 'pem' },
107+
publicKeyEncoding: { type: 'spki', format: 'pem' },
108+
});
109+
const b64Key = Buffer.from(privateKey as string).toString('base64');
110+
111+
mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64Key);
112+
113+
let capturedCreateJwt: (appId: string | number, timeDifference?: number) => Promise<{ jwt: string }>;
114+
mockedCreatAppAuth.mockImplementation((opts: StrategyOptions) => {
115+
capturedCreateJwt = (opts as Record<string, unknown>).createJwt as typeof capturedCreateJwt;
116+
const mockedAuth = vi.fn().mockResolvedValue({ token });
117+
return Object.assign(mockedAuth, { hook: vi.fn() });
118+
});
119+
120+
// Act
121+
await createGithubAppAuth(installationId);
122+
123+
// Generate two JWTs and verify they are different (jti makes them unique)
124+
const jwt1 = await capturedCreateJwt!(1);
125+
const jwt2 = await capturedCreateJwt!(1);
126+
127+
// Assert — JWTs must differ even when generated in the same second
128+
expect(jwt1.jwt).not.toBe(jwt2.jwt);
129+
130+
// Verify JWT structure: header.payload.signature
131+
const parts = jwt1.jwt.split('.');
132+
expect(parts).toHaveLength(3);
133+
const payload = JSON.parse(Buffer.from(parts[1], 'base64url').toString());
134+
expect(payload).toHaveProperty('jti');
135+
expect(payload).toHaveProperty('iat');
136+
expect(payload).toHaveProperty('exp');
137+
expect(payload).toHaveProperty('iss');
106138
});
107139

108140
it('Creates auth object for public GitHub', async () => {
109141
// Arrange
110-
const authOptions = {
111-
appId: parseInt(GITHUB_APP_ID),
112-
privateKey: decryptedValue,
113-
installationId,
114-
};
115142
mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64);
116143

117144
const mockedAuth = vi.fn();
118145
mockedAuth.mockResolvedValue({ token });
119-
// Add the required hook method to make it compatible with AuthInterface
120146
const mockWithHook = Object.assign(mockedAuth, { hook: vi.fn() });
121147
mockedCreatAppAuth.mockReturnValue(mockWithHook);
122148

@@ -128,7 +154,10 @@ ${decryptedValue}`,
128154
expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME);
129155

130156
expect(mockedCreatAppAuth).toBeCalledTimes(1);
131-
expect(mockedCreatAppAuth).toBeCalledWith({ ...authOptions });
157+
const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record<string, unknown>;
158+
expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID));
159+
expect(callArgs.createJwt).toBeTypeOf('function');
160+
expect(callArgs.installationId).toBe(installationId);
132161
expect(mockedAuth).toBeCalledWith({ type: authType });
133162
expect(result.token).toBe(token);
134163
});
@@ -142,13 +171,6 @@ ${decryptedValue}`,
142171
() => mockedRequestInterface as RequestInterface<object & RequestParameters>,
143172
);
144173

145-
const authOptions = {
146-
appId: parseInt(GITHUB_APP_ID),
147-
privateKey: decryptedValue,
148-
installationId,
149-
request: mockedRequestInterface.mockImplementation(() => ({ baseUrl: githubServerUrl })),
150-
};
151-
152174
mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64);
153175
const mockedAuth = vi.fn();
154176
mockedAuth.mockResolvedValue({ token });
@@ -165,7 +187,11 @@ ${decryptedValue}`,
165187
expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME);
166188

167189
expect(mockedCreatAppAuth).toBeCalledTimes(1);
168-
expect(mockedCreatAppAuth).toBeCalledWith(authOptions);
190+
const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record<string, unknown>;
191+
expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID));
192+
expect(callArgs.createJwt).toBeTypeOf('function');
193+
expect(callArgs.installationId).toBe(installationId);
194+
expect(callArgs.request).toBeDefined();
169195
expect(mockedAuth).toBeCalledWith({ type: authType });
170196
expect(result.token).toBe(token);
171197
});
@@ -181,16 +207,9 @@ ${decryptedValue}`,
181207

182208
const installationId = undefined;
183209

184-
const authOptions = {
185-
appId: parseInt(GITHUB_APP_ID),
186-
privateKey: decryptedValue,
187-
request: mockedRequestInterface.mockImplementation(() => ({ baseUrl: githubServerUrl })),
188-
};
189-
190210
mockedGet.mockResolvedValueOnce(GITHUB_APP_ID).mockResolvedValueOnce(b64);
191211
const mockedAuth = vi.fn();
192212
mockedAuth.mockResolvedValue({ token });
193-
// Add the required hook method to make it compatible with AuthInterface
194213
const mockWithHook = Object.assign(mockedAuth, { hook: vi.fn() });
195214
mockedCreatAppAuth.mockReturnValue(mockWithHook);
196215

@@ -202,7 +221,11 @@ ${decryptedValue}`,
202221
expect(getParameter).toBeCalledWith(PARAMETER_GITHUB_APP_KEY_BASE64_NAME);
203222

204223
expect(mockedCreatAppAuth).toBeCalledTimes(1);
205-
expect(mockedCreatAppAuth).toBeCalledWith(authOptions);
224+
const callArgs = mockedCreatAppAuth.mock.calls[0][0] as Record<string, unknown>;
225+
expect(callArgs.appId).toBe(parseInt(GITHUB_APP_ID));
226+
expect(callArgs.createJwt).toBeTypeOf('function');
227+
expect(callArgs).not.toHaveProperty('installationId');
228+
expect(callArgs.request).toBeDefined();
206229
expect(mockedAuth).toBeCalledWith({ type: authType });
207230
expect(result.token).toBe(token);
208231
});

‎lambdas/functions/control-plane/src/github/auth.ts‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,11 @@ type AuthInterface = {
1212
};
1313
type StrategyOptions = {
1414
appId: number;
15-
privateKey: string;
15+
createJwt: (appId: string | number, timeDifference?: number) => Promise<{ jwt: string; expiresAt: string }>;
1616
installationId?: number;
1717
request?: RequestInterface;
1818
};
19+
import { createSign, randomUUID } from 'node:crypto';
1920
import { request } from '@octokit/request';
2021
import { Octokit } from '@octokit/rest';
2122
import { throttling } from '@octokit/plugin-throttling';
@@ -69,20 +70,36 @@ export async function createGithubInstallationAuth(
6970
return auth(installationAuthOptions);
7071
}
7172

73+
function signJwt(payload: Record<string, unknown>, privateKey: string): string {
74+
const header = { alg: 'RS256', typ: 'JWT' };
75+
const encode = (obj: unknown) => Buffer.from(JSON.stringify(obj)).toString('base64url');
76+
const message = `${encode(header)}.${encode(payload)}`;
77+
const signature = createSign('RSA-SHA256').update(message).sign(privateKey, 'base64url');
78+
return `${message}.${signature}`;
79+
}
80+
7281
async function createAuth(installationId: number | undefined, ghesApiUrl: string): Promise<AuthInterface> {
7382
const appId = parseInt(await getParameter(process.env.PARAMETER_GITHUB_APP_ID_NAME));
74-
let authOptions: StrategyOptions = {
75-
appId,
76-
privateKey: Buffer.from(
77-
await getParameter(process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME),
78-
'base64',
79-
// replace literal \n characters with new lines to allow the key to be stored as a
80-
// single line variable. This logic should match how the GitHub Terraform provider
81-
// processes private keys to retain compatibility between the projects
82-
)
83-
.toString()
84-
.replace('/[\\n]/g', String.fromCharCode(10)),
83+
// replace literal \n characters with new lines to allow the key to be stored as a
84+
// single line variable. This logic should match how the GitHub Terraform provider
85+
// processes private keys to retain compatibility between the projects
86+
const privateKey = Buffer.from(await getParameter(process.env.PARAMETER_GITHUB_APP_KEY_BASE64_NAME), 'base64')
87+
.toString()
88+
.replace('/[\\n]/g', String.fromCharCode(10));
89+
90+
// Use a custom createJwt callback to include a jti (JWT ID) claim in every token.
91+
// Without this, concurrent Lambda invocations generating JWTs within the same second
92+
// produce byte-identical tokens (same iat, exp, iss), which GitHub rejects as duplicates.
93+
// See: https://github.com/github-aws-runners/terraform-aws-github-runner/issues/5025
94+
const createJwt = async (appId: string | number, timeDifference?: number) => {
95+
const now = Math.floor(Date.now() / 1000) + (timeDifference ?? 0);
96+
const iat = now - 30;
97+
const exp = iat + 600;
98+
const jwt = signJwt({ iat, exp, iss: appId, jti: randomUUID() }, privateKey);
99+
return { jwt, expiresAt: new Date(exp * 1000).toISOString() };
85100
};
101+
102+
let authOptions: StrategyOptions = { appId, createJwt };
86103
if (installationId) authOptions = { ...authOptions, installationId };
87104

88105
logger.debug(`GHES API URL: ${ghesApiUrl}`);

0 commit comments

Comments
 (0)