From 831edceed4d8722e89a8cb6a8a65a93210984fa6 Mon Sep 17 00:00:00 2001 From: Evgeny Shurakov Date: Tue, 8 Sep 2026 14:19:18 +0200 Subject: [PATCH] fix(cloud-agent-next): diagnose GitHub upstream quotas --- .../src/github-rate-limit-diagnostics.test.ts | 245 ++++++++++++++++++ .../src/github-rate-limit-diagnostics.ts | 214 +++++++++++++++ .../src/sandbox-outbound.test.ts | 156 ++++++++++- .../cloud-agent-next/src/sandbox-outbound.ts | 32 +++ services/cloud-agent-next/src/types.ts | 8 +- services/git-token-service/src/index.test.ts | 35 ++- services/git-token-service/src/index.ts | 6 + 7 files changed, 691 insertions(+), 5 deletions(-) create mode 100644 services/cloud-agent-next/src/github-rate-limit-diagnostics.test.ts create mode 100644 services/cloud-agent-next/src/github-rate-limit-diagnostics.ts diff --git a/services/cloud-agent-next/src/github-rate-limit-diagnostics.test.ts b/services/cloud-agent-next/src/github-rate-limit-diagnostics.test.ts new file mode 100644 index 00000000000..53a2e90134c --- /dev/null +++ b/services/cloud-agent-next/src/github-rate-limit-diagnostics.test.ts @@ -0,0 +1,245 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + classifyGitHubRateLimitBody, + GITHUB_RATE_LIMIT_DIAGNOSTIC_BODY_DEADLINE_MS, + GITHUB_RATE_LIMIT_DIAGNOSTIC_MAX_BODY_BYTES, + inspectGitHubRateLimitResponse, +} from './github-rate-limit-diagnostics.js'; + +describe('classifyGitHubRateLimitBody', () => { + it.each([ + ['API rate limit exceeded', 'primary_limit'], + ['You have exceeded a secondary rate limit.', 'secondary_limit'], + ['You have triggered an abuse detection mechanism.', 'abuse_detection'], + ['429 Too Many Requests', 'too_many_requests'], + ['The upstream service is unavailable.', 'unavailable'], + ['permission denied', 'none'], + ] as const)('classifies %s as %s', (body, signal) => { + expect(classifyGitHubRateLimitBody(body)).toBe(signal); + }); +}); + +describe('inspectGitHubRateLimitResponse', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it.each([200, 404])('does not inspect a %i response', async status => { + const response = new Response('API rate limit exceeded', { status }); + + await expect(inspectGitHubRateLimitResponse(response)).resolves.toBeUndefined(); + await expect(response.text()).resolves.toBe('API rate limit exceeded'); + }); + + it('classifies zero remaining as primary exhaustion and copies only safe headers', async () => { + const response = new Response('permission denied', { + status: 403, + headers: { + 'x-ratelimit-limit': '5000', + 'x-ratelimit-remaining': '0', + 'x-ratelimit-used': '5000', + 'x-ratelimit-reset': '1700000000', + 'x-ratelimit-resource': 'core', + 'retry-after': '60', + authorization: 'Bearer body-secret', + }, + }); + + await expect(inspectGitHubRateLimitResponse(response)).resolves.toEqual({ + upstreamStatus: 403, + githubRateLimitLimit: '5000', + githubRateLimitRemaining: '0', + githubRateLimitUsed: '5000', + githubRateLimitReset: '1700000000', + githubRateLimitResource: 'core', + githubRetryAfter: '60', + quotaClass: 'primary_exhausted', + bodySignal: 'none', + }); + }); + + it('classifies a primary-limit body without remaining as a primary signal', async () => { + const response = new Response('API rate limit exceeded', { status: 429 }); + + await expect(inspectGitHubRateLimitResponse(response)).resolves.toMatchObject({ + quotaClass: 'primary_signal', + bodySignal: 'primary_limit', + }); + }); + + it.each([ + ['You have exceeded a secondary rate limit.', 'secondary_limit'], + ['You have triggered an abuse detection mechanism.', 'abuse_detection'], + ] as const)('classifies %s as secondary or abuse evidence', async (body, bodySignal) => { + const response = new Response(body, { status: 403 }); + + await expect(inspectGitHubRateLimitResponse(response)).resolves.toMatchObject({ + quotaClass: 'secondary_or_abuse_signal', + bodySignal, + }); + }); + + it('does not turn a generic too-many-requests body into a secondary quota claim', async () => { + const response = new Response('Too Many Requests', { status: 429 }); + + await expect(inspectGitHubRateLimitResponse(response)).resolves.toMatchObject({ + quotaClass: 'unknown', + bodySignal: 'too_many_requests', + }); + }); + + it('keeps an empty diagnostic unknown when headers and body provide no usable evidence', async () => { + const response = new Response('', { + status: 429, + headers: { + 'x-ratelimit-limit': 'not-a-number', + 'x-ratelimit-remaining': '0, 1', + 'x-ratelimit-used': '-1', + 'x-ratelimit-reset': '99999999999999999', + 'x-ratelimit-resource': 'core resource', + 'retry-after': 'not-a-delay', + }, + }); + + await expect(inspectGitHubRateLimitResponse(response)).resolves.toEqual({ + upstreamStatus: 429, + quotaClass: 'unknown', + bodySignal: 'none', + }); + }); + + it('does not retain or decode bytes beyond the bounded prefix', async () => { + const body = `${'x'.repeat(GITHUB_RATE_LIMIT_DIAGNOSTIC_MAX_BODY_BYTES)} API rate limit exceeded`; + const response = new Response(body, { status: 429 }); + + await expect(inspectGitHubRateLimitResponse(response)).resolves.toMatchObject({ + quotaClass: 'unknown', + bodySignal: 'none', + }); + await expect(response.text()).resolves.toBe(body); + }); + + it('leaves the original response body readable after inspecting a clone', async () => { + const body = 'provider-body-secret: API rate limit exceeded'; + const response = new Response(body, { status: 403 }); + + const diagnostic = await inspectGitHubRateLimitResponse(response); + + expect(await response.text()).toBe(body); + expect(JSON.stringify(diagnostic)).not.toContain(body); + expect(JSON.stringify(diagnostic)).not.toContain('provider-body-secret'); + }); + + it('finishes a stalled cloned body at the total deadline', async () => { + vi.useFakeTimers(); + const response = new Response( + new ReadableStream({ + pull() { + return new Promise(() => undefined); + }, + }), + { status: 403 } + ); + const pending = inspectGitHubRateLimitResponse(response); + + await vi.advanceTimersByTimeAsync(GITHUB_RATE_LIMIT_DIAGNOSTIC_BODY_DEADLINE_MS); + + await expect(pending).resolves.toMatchObject({ quotaClass: 'unknown', bodySignal: 'none' }); + }); + + it('uses one total deadline for a slow trickle instead of a deadline per chunk', async () => { + vi.useFakeTimers(); + let scheduled: ReturnType | undefined; + const response = new Response( + new ReadableStream({ + pull(controller) { + if (scheduled !== undefined) return; + scheduled = setTimeout(() => { + scheduled = undefined; + controller.enqueue(new TextEncoder().encode('x')); + }, 60); + }, + }), + { status: 429 } + ); + const pending = inspectGitHubRateLimitResponse(response); + + await vi.advanceTimersByTimeAsync(GITHUB_RATE_LIMIT_DIAGNOSTIC_BODY_DEADLINE_MS + 1); + + await expect(pending).resolves.toMatchObject({ quotaClass: 'unknown', bodySignal: 'none' }); + }); + + it('does not wait for a clone cancellation that rejects', async () => { + const cancel = vi.fn().mockRejectedValue(new Error('cancel failed')); + const reader = { + read: vi.fn().mockResolvedValue({ done: true, value: undefined }), + cancel, + }; + const response = { + status: 429, + headers: new Headers(), + clone: vi.fn(() => ({ body: { getReader: () => reader } })), + } as unknown as Response; + + await expect(inspectGitHubRateLimitResponse(response)).resolves.toMatchObject({ + quotaClass: 'unknown', + bodySignal: 'none', + }); + expect(cancel).toHaveBeenCalledOnce(); + }); + + it('does not wait for a clone cancellation that never settles', async () => { + vi.useFakeTimers(); + const cancel = vi.fn(() => new Promise(() => undefined)); + const reader = { + read: vi.fn().mockResolvedValue({ done: true, value: undefined }), + cancel, + }; + const response = { + status: 429, + headers: new Headers(), + clone: vi.fn(() => ({ body: { getReader: () => reader } })), + } as unknown as Response; + const inspection = inspectGitHubRateLimitResponse(response).then(() => 'resolved' as const); + const outcome = Promise.race([ + inspection, + new Promise<'timed_out'>(resolve => setTimeout(() => resolve('timed_out'), 10)), + ]); + + await vi.advanceTimersByTimeAsync(10); + + await expect(outcome).resolves.toBe('resolved'); + expect(cancel).toHaveBeenCalledOnce(); + }); + + it('turns clone and read failures into unknown body evidence', async () => { + const cloneFailure = { + status: 403, + headers: new Headers(), + clone: vi.fn(() => { + throw new Error('clone failed'); + }), + } as unknown as Response; + await expect(inspectGitHubRateLimitResponse(cloneFailure)).resolves.toMatchObject({ + quotaClass: 'unknown', + bodySignal: 'none', + }); + + const readFailure = { + status: 403, + headers: new Headers(), + clone: vi.fn(() => ({ + body: { + getReader: () => ({ + read: vi.fn().mockRejectedValue(new Error('read failed')), + cancel: vi.fn().mockResolvedValue(undefined), + }), + }, + })), + } as unknown as Response; + await expect(inspectGitHubRateLimitResponse(readFailure)).resolves.toMatchObject({ + quotaClass: 'unknown', + bodySignal: 'none', + }); + }); +}); diff --git a/services/cloud-agent-next/src/github-rate-limit-diagnostics.ts b/services/cloud-agent-next/src/github-rate-limit-diagnostics.ts new file mode 100644 index 00000000000..31038adff67 --- /dev/null +++ b/services/cloud-agent-next/src/github-rate-limit-diagnostics.ts @@ -0,0 +1,214 @@ +export const GITHUB_RATE_LIMIT_DIAGNOSTIC_MAX_BODY_BYTES = 2 * 1024; +export const GITHUB_RATE_LIMIT_DIAGNOSTIC_BODY_DEADLINE_MS = 100; +const MAX_NUMERIC_HEADER_LENGTH = 16; +const MAX_RESOURCE_HEADER_LENGTH = 32; +const MAX_RETRY_AFTER_HEADER_LENGTH = 64; + +const BODY_READ_TIMED_OUT = Symbol('github body read timed out'); + +const HTTP_DATE_RE = + /^(?:Mon|Tue|Wed|Thu|Fri|Sat|Sun), \d{2} (?:Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec) \d{4} \d{2}:\d{2}:\d{2} GMT$/; + +export type GitHubRateLimitBodySignal = + | 'primary_limit' + | 'secondary_limit' + | 'abuse_detection' + | 'too_many_requests' + | 'unavailable' + | 'none'; + +export type GitHubRateLimitQuotaClass = + | 'primary_exhausted' + | 'primary_signal' + | 'secondary_or_abuse_signal' + | 'unknown'; + +export type GitHubRateLimitDiagnostic = { + upstreamStatus: 403 | 429; + githubRateLimitLimit?: string; + githubRateLimitRemaining?: string; + githubRateLimitUsed?: string; + githubRateLimitReset?: string; + githubRateLimitResource?: string; + githubRetryAfter?: string; + quotaClass: GitHubRateLimitQuotaClass; + bodySignal: GitHubRateLimitBodySignal; +}; + +type RateLimitHeaderValues = Omit< + GitHubRateLimitDiagnostic, + 'upstreamStatus' | 'quotaClass' | 'bodySignal' +>; + +function readHeader(headers: Headers, name: string): string | undefined { + const value = headers.get(name)?.trim(); + return value || undefined; +} + +function readNonNegativeIntegerHeader(headers: Headers, name: string): string | undefined { + const value = readHeader(headers, name); + if (!value || value.length > MAX_NUMERIC_HEADER_LENGTH) return undefined; + if (!new RegExp(`^(?:0|[1-9]\\d{0,${MAX_NUMERIC_HEADER_LENGTH - 1}})$`).test(value)) { + return undefined; + } + return Number.isSafeInteger(Number(value)) ? value : undefined; +} + +function readResourceHeader(headers: Headers): string | undefined { + const value = readHeader(headers, 'x-ratelimit-resource'); + if ( + !value || + value.length > MAX_RESOURCE_HEADER_LENGTH || + !/^[A-Za-z0-9][A-Za-z0-9._-]*$/.test(value) + ) { + return undefined; + } + return value; +} + +function readRetryAfterHeader(headers: Headers): string | undefined { + const value = readHeader(headers, 'retry-after'); + if (!value || value.length > MAX_RETRY_AFTER_HEADER_LENGTH) return undefined; + if (/^(?:0|[1-9]\d{0,9})$/.test(value) || HTTP_DATE_RE.test(value)) return value; + return undefined; +} + +function readRateLimitHeaders(headers: Headers): RateLimitHeaderValues { + const values: RateLimitHeaderValues = {}; + const limit = readNonNegativeIntegerHeader(headers, 'x-ratelimit-limit'); + const remaining = readNonNegativeIntegerHeader(headers, 'x-ratelimit-remaining'); + const used = readNonNegativeIntegerHeader(headers, 'x-ratelimit-used'); + const reset = readNonNegativeIntegerHeader(headers, 'x-ratelimit-reset'); + const resource = readResourceHeader(headers); + const retryAfter = readRetryAfterHeader(headers); + + if (limit !== undefined) values.githubRateLimitLimit = limit; + if (remaining !== undefined) values.githubRateLimitRemaining = remaining; + if (used !== undefined) values.githubRateLimitUsed = used; + if (reset !== undefined) values.githubRateLimitReset = reset; + if (resource !== undefined) values.githubRateLimitResource = resource; + if (retryAfter !== undefined) values.githubRetryAfter = retryAfter; + return values; +} + +/** Classifies known provider phrases without retaining or returning the body. */ +export function classifyGitHubRateLimitBody(body: string): GitHubRateLimitBodySignal { + const normalized = body.toLowerCase().replace(/\s+/g, ' '); + + if (/\bsecondary[ -]?rate[ -]?limit\b/.test(normalized)) return 'secondary_limit'; + if (/\babuse detection\b|\babuse[- ]?detected\b/.test(normalized)) { + return 'abuse_detection'; + } + if ( + /\b(?:api|primary)[ -]?rate[ -]?limit\b/.test(normalized) || + /\brate[ -]?limit(?: has been| is)? (?:exceeded|reached)\b/.test(normalized) || + /\b(?:exceeded|reached) (?:the )?(?:api|primary|rate[ -]?limit)\b/.test(normalized) + ) { + return 'primary_limit'; + } + if (/\btoo many requests\b|\b429\b/.test(normalized)) return 'too_many_requests'; + if (/\bunavailable\b/.test(normalized)) return 'unavailable'; + return 'none'; +} + +function quotaClassFor( + remaining: string | undefined, + bodySignal: GitHubRateLimitBodySignal +): GitHubRateLimitQuotaClass { + if (remaining !== undefined && Number(remaining) === 0) return 'primary_exhausted'; + if (bodySignal === 'primary_limit') return 'primary_signal'; + if (bodySignal === 'secondary_limit' || bodySignal === 'abuse_detection') { + return 'secondary_or_abuse_signal'; + } + return 'unknown'; +} + +async function readWithDeadline( + reader: ReadableStreamDefaultReader, + timeoutMs: number +): Promise | typeof BODY_READ_TIMED_OUT> { + let timeout: ReturnType | undefined; + try { + return await Promise.race([ + reader.read(), + new Promise(resolve => { + timeout = setTimeout(() => resolve(BODY_READ_TIMED_OUT), timeoutMs); + }), + ]); + } finally { + if (timeout !== undefined) clearTimeout(timeout); + } +} + +async function readBodySignal(response: Response): Promise { + let reader: ReadableStreamDefaultReader | undefined; + const chunks: Uint8Array[] = []; + let bytesRead = 0; + const deadline = Date.now() + GITHUB_RATE_LIMIT_DIAGNOSTIC_BODY_DEADLINE_MS; + + try { + const clone = response.clone(); + if (!clone.body) return 'none'; + reader = clone.body.getReader(); + + while (bytesRead < GITHUB_RATE_LIMIT_DIAGNOSTIC_MAX_BODY_BYTES) { + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) break; + const result = await readWithDeadline(reader, remainingMs); + if (result === BODY_READ_TIMED_OUT || result.done) break; + if (!(result.value instanceof Uint8Array)) break; + + const bytesToKeep = Math.min( + result.value.byteLength, + GITHUB_RATE_LIMIT_DIAGNOSTIC_MAX_BODY_BYTES - bytesRead + ); + if (bytesToKeep > 0) { + // slice() copies the prefix so an oversized provider chunk is not retained. + chunks.push(result.value.slice(0, bytesToKeep)); + bytesRead += bytesToKeep; + } + if (bytesToKeep < result.value.byteLength) break; + } + } catch { + return 'none'; + } finally { + if (reader) { + try { + // The clone's cancellation can wait on the untouched tee branch. Never await it. + void reader.cancel().catch(() => undefined); + } catch { + // A diagnostic cancellation failure must not affect the upstream response. + } + } + } + + if (chunks.length === 0) return 'none'; + const bodyBytes = new Uint8Array(bytesRead); + let offset = 0; + for (const chunk of chunks) { + bodyBytes.set(chunk, offset); + offset += chunk.byteLength; + } + return classifyGitHubRateLimitBody(new TextDecoder().decode(bodyBytes)); +} + +export async function inspectGitHubRateLimitResponse( + response: Response +): Promise { + if (response.status !== 403 && response.status !== 429) return undefined; + + const headers = readRateLimitHeaders(response.headers); + let bodySignal: GitHubRateLimitBodySignal = 'none'; + try { + bodySignal = await readBodySignal(response); + } catch { + // Keep the response boundary best-effort if the runtime rejects inspection. + } + + return { + upstreamStatus: response.status, + ...headers, + quotaClass: quotaClassFor(headers.githubRateLimitRemaining, bodySignal), + bodySignal, + }; +} diff --git a/services/cloud-agent-next/src/sandbox-outbound.test.ts b/services/cloud-agent-next/src/sandbox-outbound.test.ts index b2d6e277871..e01fdcbf187 100644 --- a/services/cloud-agent-next/src/sandbox-outbound.test.ts +++ b/services/cloud-agent-next/src/sandbox-outbound.test.ts @@ -314,8 +314,27 @@ describe('handleManagedScmOutbound', () => { const redeemGitHubSessionCapability = vi.fn().mockResolvedValue({ success: true, authorization: REDEEMED_GIT_AUTHORIZATION, + installationId: 'installation-123', + source: 'installation', + appType: 'standard', }); - vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('forbidden', { status: 403 }))); + const body = 'upstream-body-secret: permission denied'; + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue( + new Response(body, { + status: 403, + headers: { + 'x-ratelimit-limit': '5000', + 'x-ratelimit-remaining': '0', + 'x-ratelimit-used': '5000', + 'x-ratelimit-reset': '1700000000', + 'x-ratelimit-resource': 'core', + 'retry-after': '60', + }, + }) + ) + ); const response = await handleOutbound( new Request('https://api.github.com/repos/acme/repo/pulls/1', { @@ -331,7 +350,58 @@ describe('handleManagedScmOutbound', () => { expect(logging.logger.withFields).toHaveBeenCalledWith( expect.objectContaining({ upstreamStatus: 403 }) ); - expect(serializedLogCalls()).not.toContain('upstream-token'); + expect(logging.logger.withFields).toHaveBeenCalledWith( + expect.objectContaining({ + githubInstallationId: 'installation-123', + githubAuthSource: 'installation', + githubAppType: 'standard', + upstreamStatus: 403, + githubRateLimitLimit: '5000', + githubRateLimitRemaining: '0', + githubRateLimitUsed: '5000', + githubRateLimitReset: '1700000000', + githubRateLimitResource: 'core', + githubRetryAfter: '60', + quotaClass: 'primary_exhausted', + bodySignal: 'none', + }) + ); + expect(logging.logger.warn).toHaveBeenCalledWith('Managed GitHub upstream rate-limit response'); + await expect(response.text()).resolves.toBe(body); + const logs = serializedLogCalls(); + expect(logs).not.toContain('upstream-body-secret'); + expect(logs).not.toContain('upstream-token'); + }); + + it('keeps forwarding and logs null correlation fields for an older redemption response', async () => { + const redeemGitHubSessionCapability = vi.fn().mockResolvedValue({ + success: true, + authorization: REDEEMED_GIT_AUTHORIZATION, + }); + const body = 'Too Many Requests'; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(body, { status: 429 }))); + + const response = await handleOutbound( + new Request('https://api.github.com/repos/acme/repo/pulls/1', { + headers: { + Authorization: `Bearer ${CAPABILITY}`, + 'User-Agent': 'GitHub CLI 2.82.1', + }, + }), + createEnv(redeemGitHubSessionCapability) + ); + + expect(response.status).toBe(429); + expect(logging.logger.withFields).toHaveBeenCalledWith( + expect.objectContaining({ + githubInstallationId: null, + githubAuthSource: null, + githubAppType: null, + quotaClass: 'unknown', + bodySignal: 'too_many_requests', + }) + ); + await expect(response.text()).resolves.toBe(body); }); it('keeps diagnostics failures from changing a successful forwarded response', async () => { @@ -355,6 +425,88 @@ describe('handleManagedScmOutbound', () => { ); expect(response.status).toBe(204); + expect(logging.logger.warn).not.toHaveBeenCalledWith( + 'Managed GitHub upstream rate-limit response' + ); + }); + + it('keeps a 429 response when rate-limit diagnostics fail', async () => { + const redeemGitHubSessionCapability = vi.fn().mockResolvedValue({ + success: true, + authorization: REDEEMED_GIT_AUTHORIZATION, + }); + const upstreamResponse = new Response('Too Many Requests', { status: 429 }); + vi.spyOn(upstreamResponse, 'clone').mockImplementation(() => { + throw new Error('clone unavailable'); + }); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(upstreamResponse)); + + const response = await handleOutbound( + new Request('https://api.github.com/repos/acme/repo/pulls/1', { + headers: { + Authorization: `Bearer ${CAPABILITY}`, + 'User-Agent': 'GitHub CLI 2.82.1', + }, + }), + createEnv(redeemGitHubSessionCapability) + ); + + expect(response.status).toBe(429); + await expect(response.text()).resolves.toBe('Too Many Requests'); + expect(logging.logger.withFields).toHaveBeenCalledWith( + expect.objectContaining({ quotaClass: 'unknown', bodySignal: 'none' }) + ); + }); + + it('keeps a 429 response when the rate-limit warning logger fails', async () => { + const redeemGitHubSessionCapability = vi.fn().mockResolvedValue({ + success: true, + authorization: REDEEMED_GIT_AUTHORIZATION, + }); + const body = 'Too Many Requests'; + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response(body, { status: 429 }))); + logging.logger.warn.mockImplementationOnce(() => { + throw new Error('warning logger unavailable'); + }); + + const response = await handleOutbound( + new Request('https://api.github.com/repos/acme/repo/pulls/1', { + headers: { + Authorization: `Bearer ${CAPABILITY}`, + 'User-Agent': 'GitHub CLI 2.82.1', + }, + }), + createEnv(redeemGitHubSessionCapability) + ); + + expect(response.status).toBe(429); + await expect(response.text()).resolves.toBe(body); + }); + + it('does not inspect or warn for a successful GitHub response', async () => { + const redeemGitHubSessionCapability = vi.fn().mockResolvedValue({ + success: true, + authorization: REDEEMED_GIT_AUTHORIZATION, + }); + const clone = vi.spyOn(Response.prototype, 'clone'); + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(new Response('forwarded', { status: 200 }))); + + const response = await handleOutbound( + new Request('https://api.github.com/repos/acme/repo/pulls/1', { + headers: { + Authorization: `Bearer ${CAPABILITY}`, + 'User-Agent': 'GitHub CLI 2.82.1', + }, + }), + createEnv(redeemGitHubSessionCapability) + ); + + expect(response.status).toBe(200); + expect(clone).not.toHaveBeenCalled(); + expect(logging.logger.warn).not.toHaveBeenCalledWith( + 'Managed GitHub upstream rate-limit response' + ); + clone.mockRestore(); }); it('distinguishes upstream forwarding failures without logging their messages', async () => { diff --git a/services/cloud-agent-next/src/sandbox-outbound.ts b/services/cloud-agent-next/src/sandbox-outbound.ts index 6cd54282873..65ee211a23b 100644 --- a/services/cloud-agent-next/src/sandbox-outbound.ts +++ b/services/cloud-agent-next/src/sandbox-outbound.ts @@ -12,6 +12,7 @@ import { withDORetry } from './utils/do-retry.js'; import type { GitTokenService } from './types.js'; import { MeteredSandbox } from './container-usage.js'; import type { SandboxClassName } from './container-usage-context.js'; +import { inspectGitHubRateLimitResponse } from './github-rate-limit-diagnostics.js'; import { cloudAgentSessionScopeHeaders, cloudAgentSessionScopeProtocolVersion, @@ -574,6 +575,37 @@ async function handleManagedGitHubOutbound( { ...logFields, upstreamStatus: response.status }, 'Managed GitHub outbound request forwarded' ); + if (response.status === 403 || response.status === 429) { + let rateLimitDiagnostic; + try { + rateLimitDiagnostic = await inspectGitHubRateLimitResponse(response); + } catch { + rateLimitDiagnostic = undefined; + } + logDiagnostic( + 'warn', + { + ...logFields, + githubInstallationId: result.installationId ?? null, + githubAuthSource: result.source ?? null, + githubAppType: result.appType ?? null, + upstreamStatus: response.status, + quotaClass: rateLimitDiagnostic?.quotaClass ?? 'unknown', + bodySignal: rateLimitDiagnostic?.bodySignal ?? 'none', + ...(rateLimitDiagnostic + ? { + githubRateLimitLimit: rateLimitDiagnostic.githubRateLimitLimit, + githubRateLimitRemaining: rateLimitDiagnostic.githubRateLimitRemaining, + githubRateLimitUsed: rateLimitDiagnostic.githubRateLimitUsed, + githubRateLimitReset: rateLimitDiagnostic.githubRateLimitReset, + githubRateLimitResource: rateLimitDiagnostic.githubRateLimitResource, + githubRetryAfter: rateLimitDiagnostic.githubRetryAfter, + } + : {}), + }, + 'Managed GitHub upstream rate-limit response' + ); + } return response; } diff --git a/services/cloud-agent-next/src/types.ts b/services/cloud-agent-next/src/types.ts index 83783b1acb6..dd2ec4e6719 100644 --- a/services/cloud-agent-next/src/types.ts +++ b/services/cloud-agent-next/src/types.ts @@ -266,7 +266,13 @@ type IssueGitHubSessionCapabilityResult = }; type RedeemGitHubSessionCapabilityResult = - | { success: true; authorization: string } + | { + success: true; + authorization: string; + installationId?: string; + source?: 'user' | 'installation'; + appType?: 'standard' | 'lite'; + } | { success: false; reason: diff --git a/services/git-token-service/src/index.test.ts b/services/git-token-service/src/index.test.ts index 8268ce1f9c1..ccb2e89b4d1 100644 --- a/services/git-token-service/src/index.test.ts +++ b/services/git-token-service/src/index.test.ts @@ -1190,6 +1190,9 @@ describe('GitTokenRPCEntrypoint GitHub session capability RPCs', () => { ).resolves.toEqual({ success: true, authorization: `Basic ${Buffer.from('x-access-token:installation-token').toString('base64')}`, + installationId: '123', + source: 'installation', + appType: 'standard', }); expect(serviceMocks.getTokenForRepo).toHaveBeenCalledOnce(); }); @@ -1279,7 +1282,12 @@ describe('GitTokenRPCEntrypoint GitHub session capability RPCs', () => { expect(redemption).toEqual({ success: true, authorization: `Basic ${Buffer.from('x-access-token:installation-token').toString('base64')}`, + installationId: '123', + source: 'installation', + appType: 'standard', }); + expect(redemption).not.toHaveProperty('githubToken'); + expect(redemption).not.toHaveProperty('token'); expect(serviceMocks.selectUserAuthorization).not.toHaveBeenCalled(); expect(serviceMocks.getTokenForRepo).toHaveBeenCalledOnce(); } @@ -1332,6 +1340,9 @@ describe('GitTokenRPCEntrypoint GitHub session capability RPCs', () => { expect(redemption).toEqual({ success: true, authorization: `Basic ${Buffer.from('x-access-token:installation-token').toString('base64')}`, + installationId: '123', + source: 'installation', + appType: 'standard', }); expect(serviceMocks.getTokenForRepo).toHaveBeenCalledOnce(); }); @@ -1369,6 +1380,9 @@ describe('GitTokenRPCEntrypoint GitHub session capability RPCs', () => { authorization: requestUrl.startsWith('https://github.com/') ? `Basic ${Buffer.from('x-access-token:installation-token').toString('base64')}` : 'Bearer installation-token', + installationId: '123', + source: 'installation', + appType: 'standard', }); expect(serviceMocks.getTokenForRepo).toHaveBeenCalledOnce(); } @@ -1403,7 +1417,15 @@ describe('GitTokenRPCEntrypoint GitHub session capability RPCs', () => { requestUrl, }); - expect(redemption).toEqual({ success: true, authorization: 'Bearer refreshed-user-token' }); + expect(redemption).toEqual({ + success: true, + authorization: 'Bearer refreshed-user-token', + installationId: '123', + source: 'user', + appType: 'standard', + }); + expect(redemption).not.toHaveProperty('githubToken'); + expect(redemption).not.toHaveProperty('token'); expect(serviceMocks.selectUserAuthorization).toHaveBeenCalledOnce(); } ); @@ -1426,7 +1448,13 @@ describe('GitTokenRPCEntrypoint GitHub session capability RPCs', () => { requestMethod: 'GET', requestUrl: 'https://api.github.com/repos/acme/repo/pulls/42', }) - ).resolves.toEqual({ success: true, authorization: 'Bearer user-token' }); + ).resolves.toEqual({ + success: true, + authorization: 'Bearer user-token', + installationId: '123', + source: 'user', + appType: 'standard', + }); expect(serviceMocks.selectUserAuthorization).toHaveBeenCalledOnce(); }); @@ -1465,6 +1493,9 @@ describe('GitTokenRPCEntrypoint GitHub session capability RPCs', () => { authorization: requestUrl.startsWith('https://github.com/') ? `Basic ${Buffer.from('x-access-token:user-token').toString('base64')}` : 'Bearer user-token', + installationId: '123', + source: 'user', + appType: 'standard', }); expect(serviceMocks.selectUserAuthorization).toHaveBeenCalledOnce(); } diff --git a/services/git-token-service/src/index.ts b/services/git-token-service/src/index.ts index 51d979d328f..ec701293fc6 100644 --- a/services/git-token-service/src/index.ts +++ b/services/git-token-service/src/index.ts @@ -181,6 +181,9 @@ export type RedeemGitHubSessionCapabilityParams = { export type RedeemGitHubSessionCapabilitySuccess = { success: true; authorization: string; + installationId: string; + source: 'user' | 'installation'; + appType: GitHubAppType; }; export type RedeemGitHubSessionCapabilityFailureReason = | GitHubSessionCapabilityFailureReason @@ -1024,6 +1027,9 @@ export class GitTokenRPCEntrypoint extends WorkerEntrypoint { return { success: true, authorization: this.formatUpstreamAuthorization(params.requestUrl, auth.githubToken), + installationId: auth.installationId, + source: auth.source, + appType: auth.appType, }; }