From d619b8e9abdf0176197a31180937ea59388c76b9 Mon Sep 17 00:00:00 2001 From: "mrge[bot]" <177809658+mrge[bot]@users.noreply.github.com> Date: Thu, 26 Mar 2026 01:59:18 +0000 Subject: [PATCH] fix(googlechat): validate sub claim for Chat issuer tokens to prevent auth bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app-url token verification returned ok immediately for the Chat issuer (chat@system.gserviceaccount.com) without checking the sub claim. This allowed tokens from any Google Chat app to authenticate, since Google signs them with the victim's webhook URL as the audience. Require the sub claim (Project Number/App ID) to match the configured appPrincipal for all token types, not only Add-on tokens. Rename the param from expectedAddOnPrincipal to expectedPrincipal to reflect its broader scope. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- extensions/googlechat/src/auth.ts | 17 +++--- .../googlechat/src/monitor-webhook.test.ts | 2 +- extensions/googlechat/src/monitor-webhook.ts | 4 +- extensions/googlechat/src/targets.test.ts | 53 +++++++++++++++++-- 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/extensions/googlechat/src/auth.ts b/extensions/googlechat/src/auth.ts index dd20d1267f734..6a29cd1b71a87 100644 --- a/extensions/googlechat/src/auth.ts +++ b/extensions/googlechat/src/auth.ts @@ -94,7 +94,7 @@ export async function verifyGoogleChatRequest(params: { bearer?: string | null; audienceType?: GoogleChatAudienceType | null; audience?: string | null; - expectedAddOnPrincipal?: string | null; + expectedPrincipal?: string | null; }): Promise<{ ok: boolean; reason?: string }> { const bearer = params.bearer?.trim(); if (!bearer) { @@ -119,23 +119,20 @@ export async function verifyGoogleChatRequest(params: { if (!payload?.email_verified) { return { ok: false, reason: "email not verified" }; } - if (email === CHAT_ISSUER) { - return { ok: true }; - } - if (!ADDON_ISSUER_PATTERN.test(email)) { + if (email !== CHAT_ISSUER && !ADDON_ISSUER_PATTERN.test(email)) { return { ok: false, reason: `invalid issuer: ${email}` }; } - const expectedAddOnPrincipal = params.expectedAddOnPrincipal?.trim().toLowerCase(); - if (!expectedAddOnPrincipal) { - return { ok: false, reason: "missing add-on principal binding" }; + const expectedPrincipal = params.expectedPrincipal?.trim().toLowerCase(); + if (!expectedPrincipal) { + return { ok: false, reason: "missing principal binding (appPrincipal config required)" }; } const tokenPrincipal = String(payload?.sub ?? "") .trim() .toLowerCase(); - if (!tokenPrincipal || tokenPrincipal !== expectedAddOnPrincipal) { + if (!tokenPrincipal || tokenPrincipal !== expectedPrincipal) { return { ok: false, - reason: `unexpected add-on principal: ${tokenPrincipal || ""}`, + reason: `unexpected principal: ${tokenPrincipal || ""}`, }; } return { ok: true }; diff --git a/extensions/googlechat/src/monitor-webhook.test.ts b/extensions/googlechat/src/monitor-webhook.test.ts index 6cab10382b146..a1b56832be1c3 100644 --- a/extensions/googlechat/src/monitor-webhook.test.ts +++ b/extensions/googlechat/src/monitor-webhook.test.ts @@ -120,7 +120,7 @@ describe("googlechat monitor webhook", () => { expect(verifyGoogleChatRequest).toHaveBeenCalledWith( expect.objectContaining({ bearer: "addon-token", - expectedAddOnPrincipal: "chat-app", + expectedPrincipal: "chat-app", }), ); expect(processEvent).toHaveBeenCalledWith( diff --git a/extensions/googlechat/src/monitor-webhook.ts b/extensions/googlechat/src/monitor-webhook.ts index cdb89195dad8b..e349a10b6dec6 100644 --- a/extensions/googlechat/src/monitor-webhook.ts +++ b/extensions/googlechat/src/monitor-webhook.ts @@ -141,7 +141,7 @@ export function createGoogleChatWebhookRequestHandler(params: { bearer: headerBearer, audienceType: target.audienceType, audience: target.audience, - expectedAddOnPrincipal: target.account.config.appPrincipal, + expectedPrincipal: target.account.config.appPrincipal, }); return verification.ok; }, @@ -176,7 +176,7 @@ export function createGoogleChatWebhookRequestHandler(params: { bearer: parsed.addOnBearerToken, audienceType: target.audienceType, audience: target.audience, - expectedAddOnPrincipal: target.account.config.appPrincipal, + expectedPrincipal: target.account.config.appPrincipal, }); return verification.ok; }, diff --git a/extensions/googlechat/src/targets.test.ts b/extensions/googlechat/src/targets.test.ts index 363520b9c2118..c4a12802295c9 100644 --- a/extensions/googlechat/src/targets.test.ts +++ b/extensions/googlechat/src/targets.test.ts @@ -208,11 +208,12 @@ function mockTicket(payload: Record) { } describe("verifyGoogleChatRequest", () => { - it("accepts Google Chat app-url tokens from the Chat issuer", async () => { + it("accepts Chat issuer tokens when principal matches", async () => { mocks.verifyIdToken.mockReset(); mockTicket({ email: "chat@system.gserviceaccount.com", email_verified: true, + sub: "12345", }); await expect( @@ -220,10 +221,52 @@ describe("verifyGoogleChatRequest", () => { bearer: "token", audienceType: "app-url", audience: "https://example.com/googlechat", + expectedPrincipal: "12345", }), ).resolves.toEqual({ ok: true }); }); + it("rejects Chat issuer tokens when no principal binding is configured", async () => { + mocks.verifyIdToken.mockReset(); + mockTicket({ + email: "chat@system.gserviceaccount.com", + email_verified: true, + sub: "12345", + }); + + await expect( + verifyGoogleChatRequest({ + bearer: "token", + audienceType: "app-url", + audience: "https://example.com/googlechat", + }), + ).resolves.toEqual({ + ok: false, + reason: "missing principal binding (appPrincipal config required)", + }); + }); + + it("rejects Chat issuer tokens when principal does not match", async () => { + mocks.verifyIdToken.mockReset(); + mockTicket({ + email: "chat@system.gserviceaccount.com", + email_verified: true, + sub: "99999", + }); + + await expect( + verifyGoogleChatRequest({ + bearer: "token", + audienceType: "app-url", + audience: "https://example.com/googlechat", + expectedPrincipal: "12345", + }), + ).resolves.toEqual({ + ok: false, + reason: "unexpected principal: 99999", + }); + }); + it("rejects add-on tokens when no principal binding is configured", async () => { mocks.verifyIdToken.mockReset(); mockTicket({ @@ -240,7 +283,7 @@ describe("verifyGoogleChatRequest", () => { }), ).resolves.toEqual({ ok: false, - reason: "missing add-on principal binding", + reason: "missing principal binding (appPrincipal config required)", }); }); @@ -257,7 +300,7 @@ describe("verifyGoogleChatRequest", () => { bearer: "token", audienceType: "app-url", audience: "https://example.com/googlechat", - expectedAddOnPrincipal: "principal-1", + expectedPrincipal: "principal-1", }), ).resolves.toEqual({ ok: true }); }); @@ -275,11 +318,11 @@ describe("verifyGoogleChatRequest", () => { bearer: "token", audienceType: "app-url", audience: "https://example.com/googlechat", - expectedAddOnPrincipal: "principal-1", + expectedPrincipal: "principal-1", }), ).resolves.toEqual({ ok: false, - reason: "unexpected add-on principal: principal-2", + reason: "unexpected principal: principal-2", }); }); });