Merge pull request #657 from keymanapp/auto/A19S29-merge-master-into-staging

mcdurdin · web-flow · commit 71b6baae9e42 · 2026-05-22T16:04:10.000+10:00
auto: A19S29 merge master into staging
diff --git a/README.md b/README.md
@@ -29,6 +29,7 @@ environment variables. These should be added in script `server/localenv.sh`.
 export KEYMANSTATUS_TEAMCITY_TOKEN=[your personal auth token here]
 export KEYMANSTATUS_GITHUB_TOKEN=[your personal auth token here]
 export KEYMANSTATUS_SENTRY_TOKEN=[your personal auth token here]
+# export KEYMANSTATUS_GITHUB_WEBHOOK_SECRET=[github webhook secret here] # optional unless testing the github webhook
 ```
 
 ### @keymanapp-test-bot
@@ -78,6 +79,22 @@ The unit tests will currently stop the back end container before running.
 ./build.sh test --debug
 ```
 
+## Webhooks
+
+* GitHub: 2 webhooks are configured to the same endpoint /webhook/github to POST
+  events from keymanapp organization. These are split to make log review
+  simpler, because check_run and check_suite are very noisy.
+  * check_run, check_suite
+  * issues, issue_comment, milestones, pull requests, pull request reviews
+
+* TeamCity: /webhook/teamcity - for build status
+
+* Discourse: /webhook/discourse - for community.software.sil.org topics
+
+* Sentry: /webhook/sentry - when new errors are logged
+
+* GitHub Testbot app: /webhook/keymanapp-test-bot
+
 ## Production setup
 
 This site is deployed to a Kubernetes cluster via configuration in a private
diff --git a/server/code.ts b/server/code.ts
@@ -20,13 +20,14 @@ import { DataChangeTimingManager } from './util/DataChangeTimingManager.js';
 
 import githubContributionsService from "./services/github/github-contributions.js";
 import discourseService from "./services/discourse/discourse.js";
-import githubTestContributionsService, { USER_TEST_RESULT_REGEX } from "./services/github/github-test-contributions.js";
+import githubTestContributionsService from "./services/github/github-test-contributions.js";
 import gitHubMilestonesService from './services/github/github-milestones.js';
 
 import { testUserTestComment } from './keymanapp-test-bot/test-user-test-results-comment.js';
 import { performanceLog } from './performance-log.js';
 import { consoleLog } from './util/console-log.js';
 import { buildVersion } from '../shared/version.js';
+import { processGithubWebhookEvent } from './webhooks/github-webhook.js';
 
 sms.install();
 
@@ -95,7 +96,7 @@ if(debugTestBot) {
 const port = 80;
 const REFRESH_INTERVAL = environment == Environment.Development ? 180000 : 60000;
 
-const timingManager = new DataChangeTimingManager();
+export const timingManager = new DataChangeTimingManager();
 
 /* Logging all requests and responses */
 
@@ -216,7 +217,7 @@ wsServer.on('connection', socket => {
   sendInitialRefreshMessages(socket);
 });
 
-function reportError(error) {
+export function reportError(error) {
   console.error(error);
   Sentry.captureMessage(error);
 }
@@ -227,84 +228,6 @@ function respondKeymanDataChange() {
     .catch(error => reportError(error));
 }
 
-async function respondGitHubDataChange(request: express.Request) {
-  // For now, removing this so that we get all refreshes -- check_run and
-  // check_suite are busy but only do anything significant if they reference a
-  // specific PR, so majority of those events should have no real impact. The
-  // problem here is that we lose the individual refreshes as they are coalesced
-  // by timingManager, but we need them to keep issues and PRs in sync, so a
-  // redesign is needed.
-  //
-  // if(timingManager.isTooSoon('github', GitHubRefreshRate, () => respondGitHubDataChange(request))) {
-  //   return;
-  // }
-
-  timingManager.start('github');
-  try {
-    const event = request?.headers?.['x-github-event'];
-    const issueNumber = request?.body?.issue?.number;
-    const pullNumber = request?.body?.pull_request?.number;
-    const repo = request?.body?.repository?.name;
-    const action = request?.body?.action;
-
-    if((event == 'issues' || event == 'issue_comment') && !request?.body?.issue?.pull_request) {
-      consoleLog('main', 'github', `POST webhook ${event}.${action} : keymanapp/${repo}#${issueNumber}`);
-      try {
-        if(await statusData.refreshGitHubIssueData(repo, issueNumber)) {
-          sendWsAlert(true, 'github-issues');
-
-          await respondGitHubContributionsDataChange(
-            {issue:true, post:false, pull:false, review:false, test:false}
-          );
-        }
-      } catch(error) {
-        reportError(error);
-      }
-  /*  } else if(event == 'check_run') {
-      // TODO: only refresh data related to check runs?
-      const prNumbers = request.body?.check_suite?.pull_requests?.map(pr => pr.number) ?? [];
-    } else if(event == 'check_suite') {
-      // TODO: only refresh data related to check suites?
-      const prNumbers = request.body?.check_suite?.pull_requests?.map(pr => pr.number) ?? [];
-    } else if(event == 'pull_request') {
-      // TODO: only refresh data related to pull requests
-      const prNumber = request.body?.pull_request?.number;
-  */
-    } else {
-      try {
-        const prNumbers: {hasBeenClosed: boolean, repo: string, pullNumber: number}[] = [];
-        if(issueNumber && repo && request?.body?.issue?.pull_request) {
-          prNumbers.push({hasBeenClosed: false, repo, pullNumber: issueNumber});
-        } else if(pullNumber && repo) {
-          prNumbers.push({hasBeenClosed: request.body.action == 'closed',  repo, pullNumber});
-        } else if(event == 'check_suite') {
-          prNumbers.push(...request.body?.check_suite?.pull_requests?.map(pr => ({ hasBeenClosed: false, repo: pr.base.repo.name, pullNumber: pr.number })) ?? []);
-        } else if(event == 'check_run') {
-          prNumbers.push(...request.body?.check_run?.check_suite?.pull_requests?.map(pr => ({ hasBeenClosed: false, repo: pr.base.repo.name, pullNumber: pr.number })) ?? []);
-        }
-
-        consoleLog('main', 'github', `POST webhook ${event}.${action} : ${prNumbers.map(p => `keymanapp/${p.repo}#${p.pullNumber}`).join(',')}`);
-
-        if(await statusData.refreshGitHubPullRequestsData(prNumbers)) {
-          sendWsAlert(true, 'github'); // TODO: later just refresh prs
-
-          if(event != 'check_run' && event != 'check_suite') {
-            // We'll only refresh contribution data if the event type merits it -- checks do not impact contributions
-            const isUserTestComment =
-              event == 'issue_comment' &&
-              (request?.body?.comment?.body ?? '').match(USER_TEST_RESULT_REGEX);
-            await respondGitHubContributionsDataChange({issue:event=='issues', post:false, pull:event=='pull_request', review:event=='pull_request' || event=='pull_request_review', test:isUserTestComment});
-          }
-        }
-      } catch(error) {
-        reportError(error);
-      }
-    }
-  } finally {
-    timingManager.finish('github');
-  }
-}
-
 function respondCodeOwnersDataChange() {
   return statusData.refreshCodeOwnersData()
     .then(hasChanged => sendWsAlert(hasChanged, 'code-owners'))
@@ -317,7 +240,7 @@ function respondSiteLivelinessDataChange() {
     .catch(error => reportError(error));
 }
 
-function respondGitHubContributionsDataChange(contributionChanges: ContributionChanges) {
+export function respondGitHubContributionsDataChange(contributionChanges: ContributionChanges) {
   return statusData.refreshGitHubContributionsData('current', contributionChanges)
     .then(hasChanged => sendWsAlert(hasChanged, 'github-contributions'))
     .catch(error => reportError(error));
@@ -395,10 +318,11 @@ app.use('/', express.static((environment == Environment.Development ? '' : '../'
 
 /* Web hooks */
 
-app.post('/webhook/github', (request: express.Request, response) => {
+app.post('/webhook/github', (request: express.Request, response: express.Response) => {
   (async () => {
-    respondGitHubDataChange(request);
-    slackLGTM(request.body);
+    if(await processGithubWebhookEvent(request, response)) {
+      slackLGTM(request.body);
+    }
   })();
   response.send('ok');
 });
@@ -435,7 +359,7 @@ app.post('/webhook/discourse', (request, response) => {
 
 app.use('/webhook/keymanapp-test-bot', keymanAppTestBotMiddleware);
 
-function sendWsAlert(hasChanged: boolean, message: string): boolean {
+export function sendWsAlert(hasChanged: boolean, message: string): boolean {
   if(hasChanged) {
     wsServer.clients.forEach((client) => {
       if(client.readyState === ws.OPEN) {
diff --git a/server/services/github/github-status.ts b/server/services/github/github-status.ts
@@ -293,15 +293,20 @@ const repoQuery = (name, after = 'null') => `
 async function httppostgh(query, key) {
   consoleLog('services', `github-status-${key}`, '  starting refresh');
   try {
-    const response = await fetch('https://api.github.com/graphql', {
-      method: "POST",
-      headers: {
-        Authorization: `Bearer ${github_token}`,
-        Accept: 'application/vnd.github.antiope-preview+json, application/vnd.github.shadow-cat-preview+json'
-      },
-      body: JSON.stringify({query: '{' + query + '}'})
-    });
-
+    let response;
+    try {
+      response = await fetch('https://api.github.com/graphql', {
+        method: "POST",
+        headers: {
+          Authorization: `Bearer ${github_token}`,
+          Accept: 'application/vnd.github.antiope-preview+json, application/vnd.github.shadow-cat-preview+json'
+        },
+        body: JSON.stringify({query: '{' + query + '}'})
+      });
+    } catch(e) {
+      console.dir(e);
+      throw e;
+    }
     if(!response.ok) {
       throw new Error(`Failed to query github graphql ${query} for ${key}: ${response.status} ${response.statusText}`);
     }
diff --git a/server/webhooks/github-webhook.ts b/server/webhooks/github-webhook.ts
@@ -0,0 +1,97 @@
+import express from 'express';
+import { timingManager, sendWsAlert, respondGitHubContributionsDataChange, reportError } from '../code.js';
+import { statusData } from '../data/status-data.js';
+import { USER_TEST_RESULT_REGEX } from '../services/github/github-test-contributions.js';
+import { consoleLog } from '../util/console-log.js';
+import { Webhooks } from "@octokit/webhooks";
+
+const webhooks = new Webhooks({
+  secret: process.env.KEYMANSTATUS_GITHUB_WEBHOOK_SECRET ?? 'unset',
+});
+
+export async function processGithubWebhookEvent(request: express.Request, response: express.Response): Promise<boolean> {
+  // For now, removing this so that we get all refreshes -- check_run and
+  // check_suite are busy but only do anything significant if they reference a
+  // specific PR, so majority of those events should have no real impact. The
+  // problem here is that we lose the individual refreshes as they are coalesced
+  // by timingManager, but we need them to keep issues and PRs in sync, so a
+  // redesign is needed.
+  //
+  // if(timingManager.isTooSoon('github', GitHubRefreshRate, () => respondGitHubDataChange(request))) {
+  //   return;
+  // }
+  timingManager.start('github');
+  try {
+
+    const signature = request.headers["x-hub-signature-256"] as string ?? '';
+    const body = JSON.stringify(request.body);
+
+    if (!(await webhooks.verify(body, signature))) {
+      response.status(401).send("Unauthorized");
+      return false;
+    }
+
+    const event = request?.headers?.['x-github-event'];
+    const issueNumber = request?.body?.issue?.number;
+    const pullNumber = request?.body?.pull_request?.number;
+    const repo = request?.body?.repository?.name;
+    const action = request?.body?.action;
+
+    if ((event == 'issues' || event == 'issue_comment') && !request?.body?.issue?.pull_request) {
+      consoleLog('main', 'github', `POST webhook ${event}.${action} : keymanapp/${repo}#${issueNumber}`);
+      try {
+        if (await statusData.refreshGitHubIssueData(repo, issueNumber)) {
+          sendWsAlert(true, 'github-issues');
+
+          await respondGitHubContributionsDataChange(
+            { issue: true, post: false, pull: false, review: false, test: false }
+          );
+        }
+      } catch (error) {
+        reportError(error);
+      }
+      /*  } else if(event == 'check_run') {
+          // TODO: only refresh data related to check runs?
+          const prNumbers = request.body?.check_suite?.pull_requests?.map(pr => pr.number) ?? [];
+        } else if(event == 'check_suite') {
+          // TODO: only refresh data related to check suites?
+          const prNumbers = request.body?.check_suite?.pull_requests?.map(pr => pr.number) ?? [];
+        } else if(event == 'pull_request') {
+          // TODO: only refresh data related to pull requests
+          const prNumber = request.body?.pull_request?.number;
+      */
+    } else {
+      try {
+        const prNumbers: { hasBeenClosed: boolean; repo: string; pullNumber: number; }[] = [];
+        if (issueNumber && repo && request?.body?.issue?.pull_request) {
+          prNumbers.push({ hasBeenClosed: request.body.issue.state == 'closed', repo, pullNumber: issueNumber });
+        } else if (pullNumber && repo) {
+          prNumbers.push({ hasBeenClosed: request.body.action == 'closed', repo, pullNumber });
+        } else if (event == 'check_suite') {
+          prNumbers.push(...request.body?.check_suite?.pull_requests?.map(pr => ({ hasBeenClosed: false, repo: pr.base.repo.name, pullNumber: pr.number })) ?? []);
+        } else if (event == 'check_run') {
+          prNumbers.push(...request.body?.check_run?.check_suite?.pull_requests?.map(pr => ({ hasBeenClosed: false, repo: pr.base.repo.name, pullNumber: pr.number })) ?? []);
+        }
+
+        consoleLog('main', 'github', `POST webhook ${event}.${action} : ${prNumbers.map(p => `keymanapp/${p.repo}#${p.pullNumber}`).join(',')}`);
+
+        if (await statusData.refreshGitHubPullRequestsData(prNumbers)) {
+          sendWsAlert(true, 'github'); // TODO: later just refresh prs
+
+          if (event != 'check_run' && event != 'check_suite') {
+            // We'll only refresh contribution data if the event type merits it -- checks do not impact contributions
+            const isUserTestComment = event == 'issue_comment' &&
+              (request?.body?.comment?.body ?? '').match(USER_TEST_RESULT_REGEX);
+            await respondGitHubContributionsDataChange({ issue: event == 'issues', post: false, pull: event == 'pull_request', review: event == 'pull_request' || event == 'pull_request_review', test: isUserTestComment });
+          }
+        }
+      } catch (error) {
+        reportError(error);
+      }
+    }
+  } finally {
+    timingManager.finish('github');
+  }
+  return true;
+}
+
