fix(googlechat): log webhook auth reject reasons and warn on appPrincipal misconfig (#71145)

* fix(googlechat): log webhook auth reject reasons and warn on appPrincipal misconfig Closes #71078 Webhook auth failures previously returned 401 with no log line, leaving operators no signal to diagnose. Additionally, app-url audience requires a numeric OAuth 2.0 client ID as appPrincipal, but a misconfigured email or empty value silently caused all requests to be rejected. Changes: - Log a WARN with accountId and reject reason when verifyGoogleChatRequest fails. - Add warnAppPrincipalMisconfiguration() called at provider init: warns when audienceType=app-url and appPrincipal is missing or contains '@'. Tests: +9 cases in monitor-webhook.test.ts (3 reject-reason scenarios + 4 warner cases). * fix(googlechat): defer auth rejection logs * docs: note googlechat webhook auth fix --------- Co-authored-by: luyao618 <luyao618@users.noreply.github.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>
2026-05-06 06:20:43 +00:00 · 2026-04-25 03:10:57 +08:00
parent 0651e5dc97
commit 0e50fee996
5 changed files with 354 additions and 12 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -47,6 +47,7 @@ Docs: https://docs.openclaw.ai

 ### Fixes

+- Plugins/Google Chat: log webhook auth rejection reasons only after all candidates fail, and warn when add-on `appPrincipal` values do not match configuration. Fixes #71078. (#71145) Thanks @luyao618.
 - Models/configure: preserve the existing default model when provider auth is re-run from configure while keeping explicit default-setting commands authoritative. Fixes #70696. (#70793) Thanks @Sathvik-1007.
 - Codex harness/models: keep legacy `codex/*` harness shorthand out of model picker and `/models` choice surfaces while migrating primary legacy refs to canonical `openai/*` plus explicit Codex harness config. (#71193) Thanks @vincentkoc.
 - Plugins/runtime deps: respect explicit plugin and channel disablement when repairing bundled runtime dependencies, so doctor and health checks no longer install deps for disabled configured channels.
--- a/extensions/googlechat/src/monitor-webhook.test.ts
+++ b/extensions/googlechat/src/monitor-webhook.test.ts
@@ -23,6 +23,7 @@ vi.mock("./auth.js", () => ({

 type ProcessEventFn = (event: GoogleChatEvent, target: WebhookTarget) => Promise<void>;
 let createGoogleChatWebhookRequestHandler: typeof import("./monitor-webhook.js").createGoogleChatWebhookRequestHandler;
+let warnAppPrincipalMisconfiguration: typeof import("./monitor-webhook.js").warnAppPrincipalMisconfiguration;

 function createRequest(authorization?: string): IncomingMessage {
  return {
@@ -93,7 +94,8 @@ async function runWebhookHandler(options?: {

 describe("googlechat monitor webhook", () => {
  beforeAll(async () => {
-    ({ createGoogleChatWebhookRequestHandler } = await import("./monitor-webhook.js"));
+    ({ createGoogleChatWebhookRequestHandler, warnAppPrincipalMisconfiguration } =
+      await import("./monitor-webhook.js"));
  });

  beforeEach(() => {
@@ -156,14 +158,217 @@ describe("googlechat monitor webhook", () => {
    expect(res.headers["Content-Type"]).toBe("application/json");
  });

+  it("logs WARN with reason when verification fails (missing token)", async () => {
+    const logFn = vi.fn();
+    installSimplePipeline([
+      {
+        account: {
+          accountId: "acct-1",
+          config: { appPrincipal: "chat-app" },
+        },
+        runtime: { log: logFn, error: vi.fn() },
+        audienceType: "app-url",
+        audience: "https://example.com/googlechat",
+      },
+    ]);
+    readJsonWebhookBodyOrReject.mockResolvedValue({
+      ok: true,
+      value: {
+        commonEventObject: { hostApp: "CHAT" },
+        authorizationEventObject: { systemIdToken: "bad-token" },
+        chat: {
+          messagePayload: {
+            space: { name: "spaces/AAA" },
+            message: { name: "spaces/AAA/messages/1", text: "hi" },
+          },
+        },
+      },
+    });
+    resolveWebhookTargetWithAuthOrReject.mockImplementation(async ({ isMatch, targets, res }) => {
+      for (const target of targets) {
+        if (await isMatch(target)) {
+          return target;
+        }
+      }
+      res.statusCode = 401;
+      res.end("unauthorized");
+      return null;
+    });
+    verifyGoogleChatRequest.mockResolvedValue({ ok: false, reason: "missing token" });
+    const { processEvent, res } = await runWebhookHandler();
+
+    expect(logFn).toHaveBeenCalledWith(expect.stringContaining("acct-1"));
+    expect(logFn).toHaveBeenCalledWith(expect.stringContaining("missing token"));
+    expect(processEvent).not.toHaveBeenCalled();
+    expect(res.statusCode).toBe(401);
+  });
+
+  it("logs WARN with reason when verification fails (unexpected principal)", async () => {
+    const logFn = vi.fn();
+    installSimplePipeline([
+      {
+        account: {
+          accountId: "acct-2",
+          config: { appPrincipal: "chat-app" },
+        },
+        runtime: { log: logFn, error: vi.fn() },
+        audienceType: "app-url",
+        audience: "https://example.com/googlechat",
+      },
+    ]);
+    readJsonWebhookBodyOrReject.mockResolvedValue({
+      ok: true,
+      value: {
+        commonEventObject: { hostApp: "CHAT" },
+        authorizationEventObject: { systemIdToken: "bad-token" },
+        chat: {
+          messagePayload: {
+            space: { name: "spaces/AAA" },
+            message: { name: "spaces/AAA/messages/1", text: "hi" },
+          },
+        },
+      },
+    });
+    resolveWebhookTargetWithAuthOrReject.mockImplementation(async ({ isMatch, targets, res }) => {
+      for (const target of targets) {
+        if (await isMatch(target)) {
+          return target;
+        }
+      }
+      res.statusCode = 401;
+      res.end("unauthorized");
+      return null;
+    });
+    verifyGoogleChatRequest.mockResolvedValue({
+      ok: false,
+      reason: "unexpected add-on principal: 999999999999999999999",
+    });
+    const { processEvent, res } = await runWebhookHandler();
+
+    expect(logFn).toHaveBeenCalledWith(expect.stringContaining("acct-2"));
+    expect(logFn).toHaveBeenCalledWith(
+      expect.stringContaining("unexpected add-on principal: 999999999999999999999"),
+    );
+    expect(processEvent).not.toHaveBeenCalled();
+    expect(res.statusCode).toBe(401);
+  });
+
+  it("does not log WARN when verification succeeds", async () => {
+    const logFn = vi.fn();
+    installSimplePipeline([
+      {
+        account: {
+          accountId: "acct-ok",
+          config: { appPrincipal: "chat-app" },
+        },
+        runtime: { log: logFn, error: vi.fn() },
+        statusSink: vi.fn(),
+        audienceType: "app-url",
+        audience: "https://example.com/googlechat",
+      },
+    ]);
+    readJsonWebhookBodyOrReject.mockResolvedValue({
+      ok: true,
+      value: {
+        commonEventObject: { hostApp: "CHAT" },
+        authorizationEventObject: { systemIdToken: "good-token" },
+        chat: {
+          eventTime: "2026-03-22T00:00:00.000Z",
+          user: { name: "users/123" },
+          messagePayload: {
+            space: { name: "spaces/AAA" },
+            message: { name: "spaces/AAA/messages/1", text: "hi" },
+          },
+        },
+      },
+    });
+    resolveWebhookTargetWithAuthOrReject.mockImplementation(async ({ isMatch, targets }) => {
+      for (const target of targets) {
+        if (await isMatch(target)) {
+          return target;
+        }
+      }
+      return null;
+    });
+    verifyGoogleChatRequest.mockResolvedValue({ ok: true });
+    const { res } = await runWebhookHandler();
+
+    expect(logFn).not.toHaveBeenCalled();
+    expect(res.statusCode).toBe(200);
+  });
+
+  it("does not log failed candidate targets when another target verifies", async () => {
+    const logA = vi.fn();
+    const logB = vi.fn();
+    installSimplePipeline([
+      {
+        account: {
+          accountId: "acct-a",
+          config: { appPrincipal: "chat-app-a" },
+        },
+        runtime: { log: logA, error: vi.fn() },
+        audienceType: "app-url",
+        audience: "https://example.com/googlechat",
+      },
+      {
+        account: {
+          accountId: "acct-b",
+          config: { appPrincipal: "chat-app-b" },
+        },
+        runtime: { log: logB, error: vi.fn() },
+        statusSink: vi.fn(),
+        audienceType: "app-url",
+        audience: "https://example.com/googlechat",
+      },
+    ]);
+    readJsonWebhookBodyOrReject.mockResolvedValue({
+      ok: true,
+      value: {
+        commonEventObject: { hostApp: "CHAT" },
+        authorizationEventObject: { systemIdToken: "shared-path-token" },
+        chat: {
+          eventTime: "2026-03-22T00:00:00.000Z",
+          user: { name: "users/123" },
+          messagePayload: {
+            space: { name: "spaces/BBB" },
+            message: { name: "spaces/BBB/messages/1", text: "hi" },
+          },
+        },
+      },
+    });
+    resolveWebhookTargetWithAuthOrReject.mockImplementation(async ({ isMatch, targets }) => {
+      for (const target of targets) {
+        if (await isMatch(target)) {
+          return target;
+        }
+      }
+      return null;
+    });
+    verifyGoogleChatRequest
+      .mockResolvedValueOnce({ ok: false, reason: "unexpected add-on principal: 111" })
+      .mockResolvedValueOnce({ ok: true });
+    const { processEvent, res } = await runWebhookHandler();
+
+    expect(logA).not.toHaveBeenCalled();
+    expect(logB).not.toHaveBeenCalled();
+    expect(processEvent).toHaveBeenCalledWith(
+      expect.anything(),
+      expect.objectContaining({
+        account: expect.objectContaining({ accountId: "acct-b" }),
+      }),
+    );
+    expect(res.statusCode).toBe(200);
+  });
+
  it("rejects missing add-on bearer tokens before dispatch", async () => {
+    const logFn = vi.fn();
    installSimplePipeline([
      {
        account: {
          accountId: "default",
          config: { appPrincipal: "chat-app" },
        },
-        runtime: { error: vi.fn() },
+        runtime: { log: logFn, error: vi.fn() },
      },
    ]);
    readJsonWebhookBodyOrReject.mockResolvedValue({
@@ -181,7 +386,61 @@ describe("googlechat monitor webhook", () => {
    const { processEvent, res } = await runWebhookHandler();

    expect(processEvent).not.toHaveBeenCalled();
+    expect(logFn).toHaveBeenCalledWith(expect.stringContaining("default"));
+    expect(logFn).toHaveBeenCalledWith(expect.stringContaining("missing token"));
    expect(res.statusCode).toBe(401);
    expect(res.body).toBe("unauthorized");
  });
 });
+
+describe("warnAppPrincipalMisconfiguration", () => {
+  it("warns when appPrincipal is missing for app-url audience", () => {
+    const log = vi.fn();
+    warnAppPrincipalMisconfiguration({
+      accountId: "acct-missing",
+      audienceType: "app-url",
+      appPrincipal: undefined,
+      log,
+    });
+    expect(log).toHaveBeenCalledOnce();
+    expect(log).toHaveBeenCalledWith(expect.stringContaining("acct-missing"));
+    expect(log).toHaveBeenCalledWith(expect.stringContaining("appPrincipal is missing"));
+    expect(log).toHaveBeenCalledWith(expect.stringContaining("numeric OAuth 2.0 client ID"));
+  });
+
+  it("warns when appPrincipal contains @ for app-url audience", () => {
+    const log = vi.fn();
+    warnAppPrincipalMisconfiguration({
+      accountId: "acct-email",
+      audienceType: "app-url",
+      appPrincipal: "bot@example.iam.gserviceaccount.com",
+      log,
+    });
+    expect(log).toHaveBeenCalledOnce();
+    expect(log).toHaveBeenCalledWith(expect.stringContaining("acct-email"));
+    expect(log).toHaveBeenCalledWith(expect.stringContaining("looks like an email"));
+    expect(log).toHaveBeenCalledWith(expect.stringContaining("numeric OAuth 2.0 client ID"));
+  });
+
+  it("does not warn for valid numeric appPrincipal with app-url audience", () => {
+    const log = vi.fn();
+    warnAppPrincipalMisconfiguration({
+      accountId: "acct-ok",
+      audienceType: "app-url",
+      appPrincipal: "123456789012345678901",
+      log,
+    });
+    expect(log).not.toHaveBeenCalled();
+  });
+
+  it("does not warn for project-number audience even with missing appPrincipal", () => {
+    const log = vi.fn();
+    warnAppPrincipalMisconfiguration({
+      accountId: "acct-pn",
+      audienceType: "project-number",
+      appPrincipal: undefined,
+      log,
+    });
+    expect(log).not.toHaveBeenCalled();
+  });
+});
--- a/extensions/googlechat/src/monitor-webhook.ts
+++ b/extensions/googlechat/src/monitor-webhook.ts
@@ -101,17 +101,84 @@ function parseGoogleChatInboundPayload(
  return { ok: true, event, addOnBearerToken };
 }

-async function isAuthorizedGoogleChatTarget(
+type GoogleChatWebhookAuthRejection = {
+  target: WebhookTarget;
+  reason: string;
+};
+
+async function verifyGoogleChatTargetAuth(
  target: WebhookTarget,
  bearer: string,
-): Promise<boolean> {
+): Promise<{ ok: true } | { ok: false; reason: string }> {
  const verification = await verifyGoogleChatRequest({
    bearer,
    audienceType: target.audienceType,
    audience: target.audience,
    expectedAddOnPrincipal: target.account.config.appPrincipal,
  });
-  return verification.ok;
+  return verification.ok ? { ok: true } : { ok: false, reason: verification.reason ?? "unknown" };
+}
+
+function logGoogleChatWebhookAuthRejections(rejections: GoogleChatWebhookAuthRejection[]): void {
+  for (const rejection of rejections) {
+    rejection.target.runtime.log?.(
+      `[${rejection.target.account.accountId}] Google Chat webhook auth rejected: ${rejection.reason}`,
+    );
+  }
+}
+
+function logGoogleChatWebhookAuthRejectedForTargets(
+  targets: readonly WebhookTarget[],
+  reason: string,
+): void {
+  logGoogleChatWebhookAuthRejections(targets.map((target) => ({ target, reason })));
+}
+
+async function resolveGoogleChatWebhookTargetWithAuthOrReject(params: {
+  targets: readonly WebhookTarget[];
+  res: ServerResponse;
+  bearer: string;
+}): Promise<WebhookTarget | null> {
+  const rejections: GoogleChatWebhookAuthRejection[] = [];
+  let verifiedTargetCount = 0;
+  const selectedTarget = await resolveWebhookTargetWithAuthOrReject({
+    targets: params.targets,
+    res: params.res,
+    isMatch: async (target) => {
+      const verification = await verifyGoogleChatTargetAuth(target, params.bearer);
+      if (verification.ok) {
+        verifiedTargetCount += 1;
+        return true;
+      }
+      rejections.push({ target, reason: verification.reason });
+      return false;
+    },
+  });
+  if (!selectedTarget && verifiedTargetCount === 0) {
+    logGoogleChatWebhookAuthRejections(rejections);
+  }
+  return selectedTarget;
+}
+
+export function warnAppPrincipalMisconfiguration(params: {
+  accountId: string;
+  audienceType?: string;
+  appPrincipal?: string | null;
+  log?: (message: string) => void;
+}): void {
+  if (params.audienceType !== "app-url") {
+    return;
+  }
+  const principal = params.appPrincipal?.trim();
+  if (!principal) {
+    params.log?.(
+      `[${params.accountId}] appPrincipal is missing for audienceType "app-url"; add-on token verification will fail. Set appPrincipal to the numeric OAuth 2.0 client ID (uniqueId, 21 digits), not an email.`,
+    );
+  } else if (principal.includes("@")) {
+    params.log?.(
+      `[${params.accountId}] appPrincipal "${principal}" looks like an email address. Set appPrincipal to the numeric OAuth 2.0 client ID (uniqueId, 21 digits), not an email.`,
+    );
+  }
 }

 export function createGoogleChatWebhookRequestHandler(params: {
@@ -156,10 +223,10 @@ export function createGoogleChatWebhookRequestHandler(params: {
        };

        if (headerBearer) {
-          selectedTarget = await resolveWebhookTargetWithAuthOrReject({
+          selectedTarget = await resolveGoogleChatWebhookTargetWithAuthOrReject({
            targets,
            res,
-            isMatch: (target) => isAuthorizedGoogleChatTarget(target, headerBearer),
+            bearer: headerBearer,
          });
          if (!selectedTarget) {
            return true;
@@ -178,15 +245,16 @@ export function createGoogleChatWebhookRequestHandler(params: {
          parsedEvent = parsed.event;

          if (!parsed.addOnBearerToken) {
+            logGoogleChatWebhookAuthRejectedForTargets(targets, "missing token");
            res.statusCode = 401;
            res.end("unauthorized");
            return true;
          }

-          selectedTarget = await resolveWebhookTargetWithAuthOrReject({
+          selectedTarget = await resolveGoogleChatWebhookTargetWithAuthOrReject({
            targets,
            res,
-            isMatch: (target) => isAuthorizedGoogleChatTarget(target, parsed.addOnBearerToken),
+            bearer: parsed.addOnBearerToken,
          });
          if (!selectedTarget) {
            return true;
--- a/extensions/googlechat/src/monitor.ts
+++ b/extensions/googlechat/src/monitor.ts
@@ -30,6 +30,7 @@ import type {
  GoogleChatRuntimeEnv,
  WebhookTarget,
 } from "./monitor-types.js";
+import { warnAppPrincipalMisconfiguration } from "./monitor-webhook.js";
 import { getGoogleChatRuntime } from "./runtime.js";
 import type { GoogleChatAttachment, GoogleChatEvent } from "./types.js";
 export type { GoogleChatMonitorOptions, GoogleChatRuntimeEnv } from "./monitor-types.js";
@@ -477,6 +478,13 @@ export function monitorGoogleChatProvider(options: GoogleChatMonitorOptions): ()
  const audience = options.account.config.audience?.trim();
  const mediaMaxMb = options.account.config.mediaMaxMb ?? 20;

+  warnAppPrincipalMisconfiguration({
+    accountId: options.account.accountId,
+    audienceType,
+    appPrincipal: options.account.config.appPrincipal,
+    log: options.runtime.log,
+  });
+
  const unregisterTarget = registerGoogleChatWebhookTarget({
    account: options.account,
    config: options.config,
--- a/extensions/googlechat/src/monitor.webhook-routing.test.ts
+++ b/extensions/googlechat/src/monitor.webhook-routing.test.ts
@@ -88,13 +88,15 @@ const baseAccount = (accountId: string) =>
 function registerTwoTargets() {
  const sinkA = vi.fn();
  const sinkB = vi.fn();
+  const logA = vi.fn();
+  const logB = vi.fn();
  const core = {} as PluginRuntime;
  const config = {} as OpenClawConfig;

  const unregisterA = registerGoogleChatWebhookTarget({
    account: baseAccount("A"),
    config,
-    runtime: {},
+    runtime: { log: logA },
    core,
    path: "/googlechat",
    statusSink: sinkA,
@@ -103,7 +105,7 @@ function registerTwoTargets() {
  const unregisterB = registerGoogleChatWebhookTarget({
    account: baseAccount("B"),
    config,
-    runtime: {},
+    runtime: { log: logB },
    core,
    path: "/googlechat",
    statusSink: sinkB,
@@ -111,6 +113,8 @@ function registerTwoTargets() {
  });

  return {
+    logA,
+    logB,
    sinkA,
    sinkB,
    unregister: () => {
@@ -177,7 +181,7 @@ describe("Google Chat webhook routing", () => {
  it("routes to the single verified target when earlier targets fail verification", async () => {
    mockSecondVerifierSuccess();

-    const { sinkA, sinkB, unregister } = registerTwoTargets();
+    const { logA, logB, sinkA, sinkB, unregister } = registerTwoTargets();

    try {
      await expectVerifiedRoute({
@@ -190,6 +194,8 @@ describe("Google Chat webhook routing", () => {
        sinkB,
        expectedSink: "B",
      });
+      expect(logA).not.toHaveBeenCalled();
+      expect(logB).not.toHaveBeenCalled();
    } finally {
      unregister();
    }