Skip to content

Commit f956577

Browse files
V48 (impl-only): Fix GitHub install claim for numeric pending cookie
Accept JSON-number installation_id in the staged pending cookie so claim no longer reports claim-no-pending-cookie while the browser still holds a valid cookie. Also percent-decode cookie bodies, fall back to the Cookie header, and cover the production payload in tests.
1 parent 65b31ce commit f956577

3 files changed

Lines changed: 179 additions & 7 deletions

File tree

uapi/app/tps/github/_callback-handler.ts

Lines changed: 103 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,20 @@ function readString(value: unknown) {
5252
return typeof value === 'string' && value.trim() ? value.trim() : null;
5353
}
5454

55+
/**
56+
* Coerce query/cookie JSON values to a positive integer.
57+
* Critical for staged install cookies: `JSON.stringify({ installation_id: n })`
58+
* emits a JSON number; treating only strings as valid made claim see a present
59+
* cookie as missing (`claim-no-pending-cookie` with browser cookie still set).
60+
*/
5561
function readPositiveInteger(value: unknown) {
62+
if (typeof value === 'number') {
63+
return Number.isSafeInteger(value) && value > 0 ? value : null;
64+
}
65+
if (typeof value === 'bigint') {
66+
const asNumber = Number(value);
67+
return Number.isSafeInteger(asNumber) && asNumber > 0 ? asNumber : null;
68+
}
5669
const stringValue = readString(value);
5770
if (!stringValue) return null;
5871
const parsed = Number(stringValue);
@@ -79,6 +92,8 @@ function buildConnectsRedirect(
7992
// legacy /terminal overlay root (QA ledger F8).
8093
// Canonical origin: prefer NEXT_PUBLIC_APP_URL so apex/www callbacks both
8194
// return to the operator-facing host and keep session + claim cookies aligned.
95+
// Pending install uses cookies().set (App Router merges Set-Cookie onto this
96+
// Response). Plain Response keeps Jest/node route tests constructible.
8297
const origin = resolveCanonicalAppOrigin(request);
8398
const redirectUrl = new URL(
8499
`/packs?${AUXILLARY_OPEN_QUERY_PARAM}=externals`,
@@ -229,11 +244,26 @@ type PendingInstallationCookie = {
229244
captured_at?: string;
230245
};
231246

232-
function readPendingInstallationCookie(): PendingInstallationCookie | null {
247+
/**
248+
* Parse staged install cookie. Browsers / proxies may leave the value
249+
* percent-encoded (%7B…); JSON.parse without decode fails. installation_id may
250+
* be a JSON number (from JSON.stringify) or a string (query-style payloads).
251+
*/
252+
export function parsePendingInstallationCookieValue(
253+
raw: string | null | undefined,
254+
): PendingInstallationCookie | null {
255+
if (!raw || !String(raw).trim()) return null;
256+
let text = String(raw).trim();
257+
try {
258+
// Decode when the jar still holds percent-encoding (DevTools often shows this).
259+
if (text.includes('%7B') || text.includes('%22') || text.startsWith('%')) {
260+
text = decodeURIComponent(text);
261+
}
262+
} catch {
263+
// keep original text
264+
}
233265
try {
234-
const raw = cookies().get(PENDING_INSTALLATION_COOKIE)?.value;
235-
if (!raw) return null;
236-
const parsed = JSON.parse(raw) as PendingInstallationCookie;
266+
const parsed = JSON.parse(text) as PendingInstallationCookie;
237267
const installationId = readPositiveInteger(parsed?.installation_id);
238268
if (!installationId) return null;
239269
return {
@@ -243,11 +273,57 @@ function readPendingInstallationCookie(): PendingInstallationCookie | null {
243273
state: readString(parsed.state),
244274
};
245275
} catch {
246-
// Jest / non-request contexts have no Next cookie store.
247276
return null;
248277
}
249278
}
250279

280+
function readCookieHeaderValue(
281+
request: Request | undefined,
282+
name: string,
283+
): string | null {
284+
if (!request) return null;
285+
try {
286+
const header = request.headers.get('cookie') || '';
287+
if (!header) return null;
288+
for (const part of header.split(';')) {
289+
const trimmed = part.trim();
290+
if (!trimmed) continue;
291+
const eq = trimmed.indexOf('=');
292+
if (eq <= 0) continue;
293+
const key = trimmed.slice(0, eq).trim();
294+
if (key !== name) continue;
295+
return trimmed.slice(eq + 1);
296+
}
297+
} catch {
298+
return null;
299+
}
300+
return null;
301+
}
302+
303+
type PendingCookieRead =
304+
| { status: 'ok'; value: PendingInstallationCookie }
305+
| { status: 'missing' }
306+
| { status: 'unreadable'; rawPresent: true };
307+
308+
function readPendingInstallationCookie(request?: Request): PendingCookieRead {
309+
let raw: string | null | undefined;
310+
try {
311+
raw = cookies().get(PENDING_INSTALLATION_COOKIE)?.value;
312+
} catch {
313+
raw = null;
314+
}
315+
// Prefer Next cookie store; fall back to the inbound Cookie header (claim on
316+
// /api/vcs/github/connection sometimes sees the header when cookies() is empty).
317+
if (!raw) {
318+
raw = readCookieHeaderValue(request, PENDING_INSTALLATION_COOKIE);
319+
}
320+
if (!raw) return { status: 'missing' };
321+
322+
const parsed = parsePendingInstallationCookieValue(raw);
323+
if (!parsed) return { status: 'unreadable', rawPresent: true };
324+
return { status: 'ok', value: parsed };
325+
}
326+
251327
function clearPendingInstallationCookie(request?: Request) {
252328
try {
253329
const options = request
@@ -309,8 +385,8 @@ export async function claimPendingGitHubInstallation(
309385
request?: Request,
310386
): Promise<ClaimPendingGitHubInstallationResult> {
311387
const host = request ? readRequestHostname(request) : null;
312-
const pending = readPendingInstallationCookie();
313-
if (!pending) {
388+
const pendingRead = readPendingInstallationCookie(request);
389+
if (pendingRead.status === 'missing') {
314390
const diagnostic = logInstallLifecycle('info', 'claim-no-pending-cookie', {
315391
host,
316392
hasPendingCookie: false,
@@ -320,6 +396,25 @@ export async function claimPendingGitHubInstallation(
320396
bitcodeServerTelemetry('debug', 'github-callback', 'claim-no-pending-cookie');
321397
return { claimed: false, diagnostic };
322398
}
399+
if (pendingRead.status === 'unreadable') {
400+
// Cookie name present but body not JSON (often percent-encoded without decode).
401+
const diagnostic = logInstallLifecycle('warn', 'claim-pending-cookie-unreadable', {
402+
host,
403+
hasPendingCookie: true,
404+
hasSession: null,
405+
message: 'pending_cookie_unreadable',
406+
errorClass: 'cookie',
407+
});
408+
bitcodeServerTelemetry('warn', 'github-callback', 'claim-pending-cookie-unreadable');
409+
return {
410+
claimed: false,
411+
error: 'pending_cookie_unreadable',
412+
errorClass: 'cookie',
413+
diagnostic,
414+
};
415+
}
416+
417+
const pending = pendingRead.value;
323418

324419
logInstallLifecycle('info', 'claim-start', {
325420
installationId: pending.installation_id,
@@ -355,6 +450,7 @@ export async function claimPendingGitHubInstallation(
355450
}
356451

357452
if (!userContext?.user) {
453+
// No Connect session yet — cookie is fine; operator must Connect wallet first.
358454
const diagnostic = logInstallLifecycle('warn', 'claim-result', {
359455
installationId: pending.installation_id,
360456
host,

uapi/lib/github-install-diagnostics.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export type GitHubInstallLifecycleEvent =
2323
| 'claim-start'
2424
| 'claim-result'
2525
| 'claim-no-pending-cookie'
26+
| 'claim-pending-cookie-unreadable'
2627
| 'session-resolve-failed'
2728
| 'app-not-configured';
2829

uapi/tests/api/vcsGithubCallbackRoute.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,4 +192,79 @@ describe('GitHub App callback handling', () => {
192192
expect(location.startsWith('https://www.bitcode.exchange/packs?')).toBe(true);
193193
expect(location).toContain('vcsConnection=installation_connected');
194194
});
195+
196+
it('parses staged pending cookie with numeric installation_id and percent-encoding', async () => {
197+
const { parsePendingInstallationCookieValue } = await import(
198+
'@/app/tps/github/_callback-handler'
199+
);
200+
201+
// Production stage path: JSON.stringify writes installation_id as a number.
202+
const plain = JSON.stringify({
203+
installation_id: 146662656,
204+
setup_action: 'install',
205+
state: null,
206+
account: {
207+
login: 'advancedengineeredsoftware',
208+
id: '84343342',
209+
type: 'Organization',
210+
html_url: 'https://github.com/advancedengineeredsoftware',
211+
},
212+
captured_at: '2026-07-15T03:53:14.351Z',
213+
});
214+
expect(parsePendingInstallationCookieValue(plain)?.installation_id).toBe(146662656);
215+
216+
// DevTools / some proxies leave the Cookie header percent-encoded.
217+
const encoded =
218+
'%7B%22installation_id%22%3A146662656%2C%22setup_action%22%3A%22install%22%2C%22state%22%3Anull%2C%22account%22%3A%7B%22login%22%3A%22advancedengineeredsoftware%22%2C%22id%22%3A%2284343342%22%2C%22type%22%3A%22Organization%22%2C%22html_url%22%3A%22https%3A%2F%2Fgithub.com%2Fadvancedengineeredsoftware%22%7D%2C%22captured_at%22%3A%222026-07-15T03%3A53%3A14.351Z%22%7D';
219+
const fromEncoded = parsePendingInstallationCookieValue(encoded);
220+
expect(fromEncoded?.installation_id).toBe(146662656);
221+
expect(fromEncoded?.setup_action).toBe('install');
222+
expect(fromEncoded?.account?.login).toBe('advancedengineeredsoftware');
223+
224+
// String form still accepted (query-style / hand-edited cookies).
225+
expect(
226+
parsePendingInstallationCookieValue(
227+
JSON.stringify({ installation_id: '131722518', setup_action: 'install' }),
228+
)?.installation_id,
229+
).toBe(131722518);
230+
});
231+
232+
it('claims staged install from Cookie header when cookies() store is empty', async () => {
233+
const cookieBody = JSON.stringify({
234+
installation_id: 146662656,
235+
setup_action: 'install',
236+
state: null,
237+
account: { login: 'advancedengineeredsoftware' },
238+
captured_at: '2026-07-15T03:53:14.351Z',
239+
});
240+
241+
const { claimPendingGitHubInstallation } = await import(
242+
'@/app/tps/github/_callback-handler'
243+
);
244+
const request = new Request('https://www.bitcode.exchange/api/vcs/github/connection', {
245+
headers: {
246+
cookie: `bitcode_github_installation_pending=${encodeURIComponent(cookieBody)}`,
247+
},
248+
});
249+
250+
const result = await claimPendingGitHubInstallation(request as any);
251+
// Account login comes from getInstallation (mock), not only the staged cookie.
252+
expect(result.claimed).toBe(true);
253+
expect(result.installationId).toBe(146662656);
254+
expect(result.account).toBe('engineeredsoftware');
255+
expect(mockGetInstallation).toHaveBeenCalledWith(146662656);
256+
expect(mockGenerateInstallationToken).toHaveBeenCalledWith(146662656);
257+
expect(mockSaveConnection).toHaveBeenCalledWith(
258+
'user-1',
259+
'github',
260+
expect.objectContaining({
261+
accessToken: 'ghs_installation_token',
262+
providerUserId: '146662656',
263+
metadata: expect.objectContaining({
264+
auth_source: 'github_app_installation',
265+
installation_id: 146662656,
266+
}),
267+
}),
268+
);
269+
});
195270
});

0 commit comments

Comments
 (0)