From 24c166d616e9c6fd2195ca4649ddf283ecaddb6c Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sat, 24 Oct 2026 08:43:13 -0700
Subject: [PATCH] fix(webhooks): omit URLs and raw delivery errors from logs

Keep agent name, HTTP status and timeout diagnostics without logging the
registered endpoint or transport exception text, which may echo credentials.
Cover HTTP failure, timeout, generic error and null rejection through enqueue.

Written by Sprint Impl, a Commonly agent
Pod: https://commonly.me/pods/6a692a1be833c668acdb84cf

Co-authored-by: sprint-impl <WyJzcHJpbnQtaW1wbCIsImRlZmF1bHQiXQ@agents.commonly.invalid>
Agent-Model: codex/gpt-6.1-sol/medium
---
 .../agentEventService.lifecycle.test.js       | 53 ++++++++++++++++++-
 backend/services/agentEventService.ts         | 10 ++--
 2 files changed, 58 insertions(+), 5 deletions(-)

diff --git a/backend/__tests__/unit/services/agentEventService.lifecycle.test.js b/backend/__tests__/unit/services/agentEventService.lifecycle.test.js
index 57c8d21620..96699066e9 100644
--- a/backend/__tests__/unit/services/agentEventService.lifecycle.test.js
+++ b/backend/__tests__/unit/services/agentEventService.lifecycle.test.js
@@ -422,6 +422,57 @@ describe('driver paths reach a terminal state', () => {
     expect(update.delivery.updatedAt).toBeInstanceOf(Date);
   });
 
+  test.each(['http', 'timeout', 'error', 'non-error'])('webhook %s logs exclude secrets', async (failure) => {
+    const webhookUrl = 'https://fake-user:fake-password@example.test/private-fake-path'
+      + '?token=fake-query-secret#fake-fragment';
+    const event = {
+      _id: 'evt-private-hook',
+      agentName: 'hookbot',
+      instanceId: 'default',
+      podId: POD_P1,
+      type: 'chat.mention',
+      payload: {},
+      status: 'pending',
+      attempts: 0,
+    };
+    AgentEvent.create.mockResolvedValue(event);
+    AgentInstallation.findOne.mockReturnValue({ lean: async () => null });
+    AgentEvent.find.mockReturnValue({ sort: () => ({ limit: () => ({ lean: async () => [] }) }) });
+    AgentInstallation.find.mockReturnValue({
+      lean: async () => [{ config: { runtime: { webhookUrl } } }],
+    });
+    if (failure === 'http') {
+      postToPublicAddress.mockResolvedValueOnce({ ok: false, status: 503, text: webhookUrl });
+    } else {
+      const error = new Error(`Request to ${webhookUrl} failed: fake-query-secret`);
+      if (failure === 'timeout') error.name = 'AbortError';
+      postToPublicAddress.mockRejectedValueOnce(failure === 'non-error' ? null : error);
+    }
+    const warning = jest.spyOn(console, 'warn').mockImplementation(() => {});
+    try {
+      await AgentEventService.enqueue({
+        agentName: 'hookbot', instanceId: 'default', podId: POD_P1, type: 'chat.mention', payload: {},
+      });
+      await flush();
+
+      expect(postToPublicAddress).toHaveBeenCalledWith(webhookUrl, expect.any(Object));
+      expect(AgentEvent.findByIdAndUpdate).not.toHaveBeenCalled();
+      const logs = JSON.stringify(warning.mock.calls);
+      const sensitiveParts = [
+        'fake-user', 'fake-password', 'fake-query-secret', 'private-fake-path', 'fake-fragment', 'example.test',
+      ];
+      sensitiveParts.forEach((secret) => {
+        expect(logs).not.toContain(secret);
+      });
+      const diagnostic = {
+        http: 'HTTP 503', timeout: 'timed out after 10s', error: 'delivery failed', 'non-error': 'delivery failed',
+      }[failure];
+      expect(warning).toHaveBeenCalledWith(`[webhook] hookbot ${diagnostic}`);
+    } finally {
+      warning.mockRestore();
+    }
+  });
+
   test.each([{ text: 'original text' }, { prompt: 'original prompt' }])('a webhook wake preserves fallback text and settles context after accepted delivery: %j', async (wakePayload) => {
     const ambient = {
       _id: 'ambient-1', agentName: 'hookbot', instanceId: 'default', podId: POD_P1,
@@ -483,7 +534,7 @@ describe('driver paths reach a terminal state', () => {
     expect(httpSpy).not.toHaveBeenCalled();
     expect(httpsSpy).not.toHaveBeenCalled();
     expect(AgentEvent.findByIdAndUpdate.mock.calls.some(([, u]) => u && u.status === 'acked')).toBe(false);
-    expect(warn.mock.calls.some((args) => String(args[1] || '').includes('outbound URL refused'))).toBe(true);
+    expect(warn).toHaveBeenCalledWith('[webhook] hookbot delivery failed');
   });
 
   describe('native runtime', () => {
diff --git a/backend/services/agentEventService.ts b/backend/services/agentEventService.ts
index 96fb847c4f..53262e0e8c 100644
--- a/backend/services/agentEventService.ts
+++ b/backend/services/agentEventService.ts
@@ -335,7 +335,7 @@ const deliverEventViaWebhook = async (installation: InstallationDoc, event: Even
     });
 
     if (!res.ok) {
-      console.warn(`[webhook] ${event.agentName} ${webhookUrl} → HTTP ${res.status}`);
+      console.warn(`[webhook] ${event.agentName} HTTP ${res.status}`);
       return;
     }
 
@@ -382,10 +382,12 @@ const deliverEventViaWebhook = async (installation: InstallationDoc, event: Even
       });
     }
   } catch (err) {
-    if ((err as Error).name === 'AbortError') {
-      console.warn(`[webhook] ${event.agentName} ${webhookUrl} timed out after 10s`);
+    // URLs and transport error text can contain installer credentials, including
+    // redirect destinations. Keep diagnostics independent of those values.
+    if ((err as Error)?.name === 'AbortError') {
+      console.warn(`[webhook] ${event.agentName} timed out after 10s`);
     } else {
-      console.warn(`[webhook] ${event.agentName} ${webhookUrl} error:`, (err as Error).message);
+      console.warn(`[webhook] ${event.agentName} delivery failed`);
     }
   }
 };