fix(github): keep a malformed mint answer out of reports

docs/ops/artifact-delivery-rollout.md

@@ -119,11 +119,11 @@ Personal tokens are fallbacks only. They repeat a read GitHub denied the App, su
 1. Builds try `ARTIFACT_GITHUB_TOKEN`, then `GITHUB_TOKEN`, then the token of the signed-in account that asked for the run.
 2. Every other caller tries `GITHUB_TOKEN`.
 
-Each denial emits an `app-denied` event: `github-credential` for registry reads and `artifact-github-credential` for builds. The `github.credential` field names the fallback, or `none`. Without a fallback, the read fails for that Repository only: a build answers `SOURCE_ACCESS_DENIED`, and the sync records a failure without pausing. If the fallback is spent or expired, the sync also records a failure for that Repository without pausing. The sync paces itself on the App's quota, never on the fallback's.
+Each denial emits an `app-denied` event: `github-credential` for registry reads and `artifact-github-credential` for builds. The `github.credential` field names the fallback, or `none`. Without a fallback, the read fails for that Repository only: a build answers `SOURCE_ACCESS_DENIED`, and the sync records a failure without pausing. If the fallback is spent or expired, the sync also records a failure for that Repository without pausing. After a denial, the sync paces itself on the App's quota, not on the fallback's.
 
 If GitHub answers 401 to the cached installation token, the read mints one new token and repeats. If GitHub rejects the new token too, the read uses the fallback and emits an `app-token-rejected` event.
 
-If the App secrets are set but unusable, reads use the fallback and emit an event with the reason.
+If the App secrets are set but unusable, or GitHub refuses to mint a token, every read uses the fallback and emits an event with the reason. The sync then paces itself on the fallback's quota.
 Keep the read App apart from the `GITHUB_APP_*` secrets below. Those belong to private delivery.
 
 Changing `NUXT_TOKEN_KEY` breaks stored GitHub credentials.

layers/artifact-delivery/server/utils/queue.ts

@@ -163,8 +163,8 @@ export function createKvGithubObjectCache(kv: KVNamespace): GithubObjectCache {
  * GitHub counts a personal token's quota per account, and the registry sync
  * and `/gh` page views spend `GITHUB_TOKEN` to zero before each hourly
  * reset. Every run that needed GitHub in those minutes failed RATE_LIMITED.
- * Builds read with the read App's installation token, whose bucket nothing
- * else spends, then `ARTIFACT_GITHUB_TOKEN`, then `GITHUB_TOKEN`.
+ * Builds read with the read App's installation token, whose bucket no
+ * personal tool spends, then `ARTIFACT_GITHUB_TOKEN`, then `GITHUB_TOKEN`.
  */
 export function createArtifactGithubSource(
   env: GithubCredentialEnv,

shared/server/github-app-credential.ts

@@ -472,5 +472,12 @@ async function readBoundedJson(response: Response, maximumBytes: number): Promis
     bytes.set(chunk, offset)
     offset += chunk.byteLength
   }
-  return JSON.parse(new TextDecoder('utf-8', { fatal: true }).decode(bytes)) as unknown
+  const text = new TextDecoder('utf-8', { fatal: true }).decode(bytes)
+  // A JSON.parse error quotes the start of the body, which holds the token.
+  try {
+    return JSON.parse(text) as unknown
+  }
+  catch {
+    throw new Error('GitHub App returned invalid JSON')
+  }
 }

test/unit/artifact-github-read-app.test.ts

@@ -125,6 +125,17 @@ describe('the GitHub credential public Artifact builds read with', () => {
     expect(reports).toHaveLength(1)
   })
 
+  // A JSON parse error quotes the start of the body, which holds the token.
+  it('never puts part of a malformed installation token answer in a report', async () => {
+    const keys = await appKeys()
+    const github = fakeGithub({ expiresAt: NOW + 3600, mintBody: '{"token":ghs_SECRETVALUE"}' })
+    const reports: GithubCredentialReport[] = []
+    await createArtifactGithubSource(appEnv(keys.pem), runtime(github, { report: event => reports.push(event) })).resolve(request)
+
+    expect(reports).toHaveLength(1)
+    expect(JSON.stringify(reports)).not.toContain('ghs_')
+  })
+
   it('repeats a read the Repository denied the read App with the fallback token, and reports it', async () => {
     const keys = await appKeys()
     const github = fakeGithub({ expiresAt: NOW + 3600, answersApp: appDenied })
@@ -230,7 +241,7 @@ function appDenied(): Response {
 }
 
 /** Answers every read 404, every read with an installation token `answersApp` when set, and 401 to a token in `rejects`. */
-function fakeGithub(options: { expiresAt: number, mintStatus?: number, answersApp?: () => Response, rejects?: string[] }): FakeGithub {
+function fakeGithub(options: { expiresAt: number, mintStatus?: number, mintBody?: string, answersApp?: () => Response, rejects?: string[] }): FakeGithub {
   const mints: FakeGithub['mints'] = []
   const readAuthorizations: FakeGithub['readAuthorizations'] = []
   const fetcher = async (input: RequestInfo | URL, init?: RequestInit): Promise<Response> => {
@@ -240,6 +251,8 @@ function fakeGithub(options: { expiresAt: number, mintStatus?: number, answersAp
       mints.push({ url, authorization: headers.get('authorization') ?? '', body: JSON.parse(String(init?.body)) })
       if (options.mintStatus)
         return new Response('{}', { status: options.mintStatus })
+      if (options.mintBody)
+        return new Response(options.mintBody, { status: 201 })
       return Response.json({ token: `ghs_installation_${mints.length}`, expires_at: new Date(options.expiresAt * 1000).toISOString() }, { status: 201 })
     }
     const authorization = headers.get('authorization')

test/unit/registry-github-app-credential.test.ts

@@ -195,15 +195,19 @@ describe('a read GitHub answered 401 for the cached installation token', () => {
   })
 
   it('mints once for reads GitHub rejected together', async () => {
-    const github = await stubGithub({ rejects: ['ghs_app_1'] })
+    const rejects: string[] = []
+    const github = await stubGithub({ rejects })
     const bindings = resolveGithubBindings(await appEnv({ GITHUB_TOKEN: 'site' }), runtime())
     await getRepo('nuxt', 'ui', bindings)
+    rejects.push('ghs_app_1')
     github.reads.length = 0
 
-    await Promise.all([getRepo('nuxt', 'ui', bindings), getRepo('nuxt', 'nuxt', bindings), getRepo('nuxt', 'image', bindings)])
+    const repos = await Promise.all([getRepo('nuxt', 'ui', bindings), getRepo('nuxt', 'nuxt', bindings), getRepo('nuxt', 'image', bindings)])
 
+    expect(repos.map(repo => repo.status)).toEqual([200, 200, 200])
     expect(github.mints).toBe(2)
-    expect(github.reads.filter(read => read === 'Bearer site')).toEqual([])
+    expect(github.reads.filter(read => read === 'Bearer ghs_app_1')).toHaveLength(3)
+    expect(github.reads).not.toContain('Bearer site')
   })
 
   it('reads with GITHUB_TOKEN, and reports it, when GitHub rejects the new token too', async () => {