From 6d187c58c6a9123c508a2a3142d584bcbfea321d Mon Sep 17 00:00:00 2001 From: Baptiste LAFOURCADE Date: Mon, 12 Oct 2026 12:14:06 +0200 Subject: [PATCH] fix(cli): validate github token responses without leaking credentials Cover the HTTP authentication boundary and normalize failures without preserving request credentials. Refs #467 --- .../validation.md | 42 +++++++++++++ cli/src/runtime/auth/gh-token-adapter.ts | 19 +++++- .../auth/gh-token-adapter.unit.test.ts | 60 +++++++++++++++++-- 3 files changed, 112 insertions(+), 9 deletions(-) create mode 100644 aidd_docs/tasks/2026_10/2026_10_12_github-token-http-boundary-467/validation.md diff --git a/aidd_docs/tasks/2026_10/2026_10_12_github-token-http-boundary-467/validation.md b/aidd_docs/tasks/2026_10/2026_10_12_github-token-http-boundary-467/validation.md new file mode 100644 index 000000000..7ab347a18 --- /dev/null +++ b/aidd_docs/tasks/2026_10/2026_10_12_github-token-http-boundary-467/validation.md @@ -0,0 +1,42 @@ +# #467 — Contrat HTTP de vérification du token GitHub + +## Priorité et périmètre + +Source : [AIDD Roadmap, projet GitHub n°8](https://github.com/orgs/ai-driven-dev/projects/8). Le champ Priority exposé dans ce projet classe #467 en High. Aucun élément ouvert n'est Urgent ; sept issues ouvertes sont High. #467 a été retenue pour son périmètre précis lié à l'authentification, sans PR ouverte identifiée sur ce sujet. + +Base de publication : `next`, commit `d31fb5ee5e69b5ec61731ede2717a2342ab74d33`. Branche : `test/github-token-http-boundary`. + +Deux tests existaient déjà : succès et objet sans login. Les nouveaux tests ont montré 11 échecs sur la base : corps null ou undefined, login vide ou blanc, réponse non réussie portant un login, erreur HTTP conservant une valeur de token synthétique. + +## Correction + +`GhTokenAdapter` accepte uniquement une réponse HTTP 2xx avec un login non vide. Une réponse invalide ou une requête rejetée produit `AuthenticationError("GitHub API")`. L'erreur HTTP d'origine n'est pas conservée comme cause, afin de ne pas exposer les éventuelles données de requête qu'elle contient. Le détail HTTP ou réseau est donc remplacé par le message d'authentification générique. + +Les tests utilisent uniquement le port HttpGet et des tokens synthétiques. Ils vérifient le succès, la transmission du token dans options.token, les réponses malformées, les statuts 199/300/401/403/404/500 et la confidentialité de l'erreur. Aucune requête GitHub ni CLI d'hôte n'est appelée par ces tests. + +Fichiers : `cli/src/runtime/auth/gh-token-adapter.ts`, `cli/tests/runtime/auth/gh-token-adapter.unit.test.ts`. + +## Validation finale + +| Commande, depuis cli/ | Résultat | +| --- | --- | +| `pnpm exec vitest run --project=unit tests/runtime/auth` | 42 tests réussis | +| `pnpm exec vitest run --project=integration tests/runtime/auth tests/runtime/http` | 82 tests réussis | +| `pnpm test:coverage` | 6 960 tests réussis, 1 ignoré ; tous les seuils respectés | +| `pnpm exec stryker run --mutate src/runtime/auth/gh-token-adapter.ts --concurrency 2` | 39 mutations détectées sur 39, aucune survivante | +| `pnpm typecheck` | Réussite | +| `pnpm lint` | Réussite ; un avertissement préexistant dans uninstall-use-case.ts, hors périmètre | +| `pnpm test:arch` | 144 tests réussis | +| `pnpm knip` | Réussite | +| `pnpm build` | 750,75 Ko pour le budget existant de 751 Ko | +| `git diff --check` | Réussite | + +Couverture finale : statements 98,77 %, branches 95,76 %, functions 99,55 %, lines 99,38 %. Aucun seuil de couverture ou de mutation modifié. + +Après mise à jour sur `next`, le correctif tient dans le budget existant de 751 Ko. Aucun fichier de budget n'est modifié par cette livraison. + +La revue indépendante du code de l'adaptateur et des tests ne relève aucun blocage. L'adaptateur et ses tests sont inchangés depuis cette revue et le contrôle de mutation. + +## État + +Validation locale terminée avant publication. #467 est In Progress dans le projet ; ce rapport précède la CI distante et le merge. diff --git a/cli/src/runtime/auth/gh-token-adapter.ts b/cli/src/runtime/auth/gh-token-adapter.ts index 244b992ec..5b72e94b2 100644 --- a/cli/src/runtime/auth/gh-token-adapter.ts +++ b/cli/src/runtime/auth/gh-token-adapter.ts @@ -5,9 +5,22 @@ export class GhTokenAdapter implements TokenAuthProvider { constructor(private readonly http: HttpGet) {} async verifyToken(token: string): Promise { - const response = await this.http.get("https://api.github.com/user", { token }); - const body = response.body as Record; - if (typeof body.login !== "string") throw new AuthenticationError("GitHub API"); + const response = await this.http.get("https://api.github.com/user", { token }).catch(() => { + // HTTP errors may carry request credentials; expose only the authentication failure. + throw new AuthenticationError("GitHub API"); + }); + const body = response.body; + if ( + response.statusCode < 200 || + response.statusCode >= 300 || + typeof body !== "object" || + body === null || + !("login" in body) || + typeof body.login !== "string" || + body.login.trim().length === 0 + ) { + throw new AuthenticationError("GitHub API"); + } return body.login; } } diff --git a/cli/tests/runtime/auth/gh-token-adapter.unit.test.ts b/cli/tests/runtime/auth/gh-token-adapter.unit.test.ts index 145a306cf..d818572c4 100644 --- a/cli/tests/runtime/auth/gh-token-adapter.unit.test.ts +++ b/cli/tests/runtime/auth/gh-token-adapter.unit.test.ts @@ -7,13 +7,16 @@ import type { HttpResponse, } from "../../../src/runtime/http/http-client.js"; -function http(body: unknown): HttpGet & { calls: { url: string; options?: HttpGetOptions }[] } { +function http( + body: unknown, + statusCode = 200 +): HttpGet & { calls: { url: string; options?: HttpGetOptions }[] } { const calls: { url: string; options?: HttpGetOptions }[] = []; return { calls, get: async (url, options): Promise => { calls.push({ url, options }); - return { body, statusCode: 200, contentType: "application/json" }; + return { body, statusCode, contentType: "application/json" }; }, }; } @@ -24,10 +27,12 @@ describe("GhTokenAdapter", () => { const login = await new GhTokenAdapter(client).verifyToken("ghp_x"); - expect({ login, calls: client.calls }).toStrictEqual({ - login: "octocat", - calls: [{ url: "https://api.github.com/user", options: { token: "ghp_x" } }], - }); + expect(login).toBe("octocat"); + expect(client.calls).toHaveLength(1); + expect(client.calls[0]?.url).toBe("https://api.github.com/user"); + expect(Object.keys(client.calls[0]?.options ?? {})).toEqual(["token"]); + // Compare a boolean so a failed assertion cannot print the supplied credential. + expect(client.calls[0]?.options?.token === "ghp_x").toBe(true); }); it("refuses an answer carrying no login", async () => { @@ -35,4 +40,47 @@ describe("GhTokenAdapter", () => { new AuthenticationError("GitHub API") ); }); + + it.each([ + ["null", null], + ["undefined", undefined], + ["text", "octocat"], + ["array", ["octocat"]], + ["numeric login", { login: 42 }], + ["null login", { login: null }], + ["empty login", { login: "" }], + ["blank login", { login: " " }], + ])("refuses a malformed user response: %s", async (_label, body) => { + const error: unknown = await new GhTokenAdapter(http(body)) + .verifyToken("ghp_x") + .catch((error: unknown) => error); + + expect(error instanceof AuthenticationError).toBe(true); + }); + + it.each([199, 300, 401, 403, 404, 500])( + "refuses HTTP %i even when the response carries a login", + async (statusCode) => { + await expect( + new GhTokenAdapter(http({ login: "octocat" }, statusCode)).verifyToken("ghp_x") + ).rejects.toThrow(new AuthenticationError("GitHub API")); + } + ); + + it("reports a rejected request without exposing its token or underlying error", async () => { + const token = "synthetic-private-credential"; + const client: HttpGet = { + get: async () => { + throw new Error(`Request failed for ${token}`); + }, + }; + const error: unknown = await new GhTokenAdapter(client) + .verifyToken(token) + .catch((error: unknown) => error); + + expect(String(error).includes(token)).toBe(false); + expect(error instanceof AuthenticationError).toBe(true); + expect(String(error)).toBe(String(new AuthenticationError("GitHub API"))); + expect(error instanceof Error && error.cause === undefined).toBe(true); + }); });