Skip to content

Commit 33da358

Browse files
naimish-browseclaude
authored andcommitted
test-companion: say why signing in as the GitHub App failed
Any failure to look up the App's installation was reported as "the GitHub App is not installed on this repository", hiding GitHub's real answer. A wrong App ID or a key from another App looked exactly like a missing installation. Now only a 404 is reported as "not installed". Any other failure is a warning that carries GitHub's own reason, e.g. "Bad credentials (HTTP 401)", and says which inputs to check. A failed token request also carries GitHub's reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 874a8e4 commit 33da358

3 files changed

Lines changed: 68 additions & 4 deletions

File tree

‎test-companion/src/app-auth.mjs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,32 @@ export function jwtClaims(appId, nowSeconds) {
2323
return { iat, exp: iat + JWT_LIFETIME_SECONDS, iss: String(appId) }
2424
}
2525

26+
/**
27+
* GitHub's reason a `gh api` call failed, e.g. "Bad credentials (HTTP 401)", for the log.
28+
* gh's error output holds only the response message, never the token it sent.
29+
*/
30+
export function ghFailureReason(result) {
31+
const line = String(result?.stderr ?? "")
32+
.split("\n")
33+
.map((l) => l.trim())
34+
.find(Boolean)
35+
return line ? line.replace(/^gh:\s*/, "") : "no detail from GitHub"
36+
}
37+
38+
/** What to log when looking up the App's installation on this repository fails. */
39+
export function installationLookupMessage(result) {
40+
const reason = ghFailureReason(result)
41+
if (/\bHTTP 404\b/.test(reason)) {
42+
return { level: "notice", text: "Test Companion: the GitHub App is not installed on this repository; using the workflow token." }
43+
}
44+
return {
45+
level: "warning",
46+
text:
47+
`Test Companion: could not sign in as the GitHub App (${reason}); using the workflow token. ` +
48+
"Check that app-id is the App's numeric ID and app-private-key is a key generated for that App.",
49+
}
50+
}
51+
2652
export function signJwt({ appId, privateKey, now = Date.now() / 1000 }) {
2753
const header = b64url(JSON.stringify({ alg: "RS256", typ: "JWT" }))
2854
const payload = b64url(JSON.stringify(jwtClaims(appId, now)))

‎test-companion/src/main.mjs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,13 @@ import { canIsolate, prepareSandbox, stopSandbox, handToAgent, writeAgentEnv, re
1414
import { agentEnvFrom, asAgentCommand, checkPrHandoff, readClaimResult } from "./sandbox-policy.mjs"
1515
import { startRelay } from "./relay.mjs"
1616
import { runCapturingAnswer } from "./answer.mjs"
17-
import { appCredentials, signJwt, installationTokenRequest } from "./app-auth.mjs"
17+
import {
18+
appCredentials,
19+
signJwt,
20+
installationTokenRequest,
21+
ghFailureReason,
22+
installationLookupMessage,
23+
} from "./app-auth.mjs"
1824
import { OUTCOMES, parseSummary, resolveOutcome, checkConclusion, stepShouldFail, approvalWasSpent } from "./outcome.mjs"
1925
import {
2026
renderJobSummary,
@@ -181,7 +187,8 @@ function appInstallationToken() {
181187

182188
const found = asApp(["api", `repos/${REPO}/installation`])
183189
if (found.status !== 0) {
184-
notice("Test Companion: the GitHub App is not installed on this repository; using the workflow token.")
190+
const { level, text } = installationLookupMessage(found)
191+
;(level === "warning" ? warn : notice)(text)
185192
return undefined
186193
}
187194
let installationId
@@ -213,7 +220,7 @@ function appInstallationToken() {
213220
const minted = mint(scope)
214221
if (minted.status !== 0) {
215222
warn(
216-
"Could not get a GitHub App token with the permissions Test Companion needs; using the workflow token instead. " +
223+
`Could not get a GitHub App token with the permissions Test Companion needs (${ghFailureReason(minted)}); using the workflow token instead. ` +
217224
"If the App's permissions were changed, an organisation owner may need to accept them.",
218225
)
219226
return undefined

‎test-companion/test/app-auth.test.mjs‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
import { test, describe } from "node:test"
22
import assert from "node:assert/strict"
33
import { generateKeyPairSync, createVerify } from "node:crypto"
4-
import { appCredentials, jwtClaims, signJwt, installationTokenRequest } from "../src/app-auth.mjs"
4+
import {
5+
appCredentials,
6+
jwtClaims,
7+
signJwt,
8+
installationTokenRequest,
9+
ghFailureReason,
10+
installationLookupMessage,
11+
} from "../src/app-auth.mjs"
512

613
const { privateKey, publicKey } = generateKeyPairSync("rsa", { modulusLength: 2048 })
714
const pem = privateKey.export({ type: "pkcs1", format: "pem" })
@@ -88,3 +95,27 @@ describe("installationTokenRequest", () => {
8895
assert.equal(installationTokenRequest(undefined), undefined)
8996
})
9097
})
98+
99+
describe("App sign-in failures say why", () => {
100+
test("GitHub's reason is taken from gh's error output", () => {
101+
assert.equal(ghFailureReason({ stderr: "gh: Bad credentials (HTTP 401)\n" }), "Bad credentials (HTTP 401)")
102+
})
103+
104+
test("an empty error output still gives a reason", () => {
105+
assert.equal(ghFailureReason({ stderr: "" }), "no detail from GitHub")
106+
assert.equal(ghFailureReason(undefined), "no detail from GitHub")
107+
})
108+
109+
test("only a 404 means the App is not installed", () => {
110+
const m = installationLookupMessage({ stderr: "gh: Not Found (HTTP 404)" })
111+
assert.equal(m.level, "notice")
112+
assert.match(m.text, /not installed on this repository/)
113+
})
114+
115+
test("any other failure is a warning that carries GitHub's reason", () => {
116+
const m = installationLookupMessage({ stderr: "gh: A JSON web token could not be decoded (HTTP 401)" })
117+
assert.equal(m.level, "warning")
118+
assert.match(m.text, /A JSON web token could not be decoded \(HTTP 401\)/)
119+
assert.doesNotMatch(m.text, /not installed/)
120+
})
121+
})

0 commit comments

Comments
 (0)