Skip to content

Commit 5b7f7b4

Browse files
committed
Handle GitHub webhook non-POST methods and cleanup installs
Improves the GitHub webhook route to handle non-POST requests (e.g., health checks) by logging and returning 200 without signature validation. Adds a method to delete GitHub App installations in the service, and updates project resolver to remove GitHub installations on disconnect. Also standardizes error key to 'apiError' in OAuth redirects and improves webhook logging.
1 parent af9b99b commit 5b7f7b4

File tree

4 files changed

+72
-11
lines changed

4 files changed

+72
-11
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "hawk.api",
3-
"version": "1.4.0",
3+
"version": "1.4.1",
44
"main": "index.ts",
55
"license": "BUSL-1.1",
66
"scripts": {

src/integrations/github/routes.ts

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,8 @@ export function createGitHubRouter(factories: ContextFactories): express.Router
208208
logger(prefix, ...logArgs);
209209
}
210210

211+
const WEBHOOK_LOG_PREFIX = '[🍏 🍎 ✨ Webhook] ';
212+
211213
/**
212214
* GET /integration/github/connect?projectId=<projectId>
213215
* Initiate GitHub integration connection
@@ -290,13 +292,13 @@ export function createGitHubRouter(factories: ContextFactories): express.Router
290292
*/
291293
if (!code || typeof code !== 'string') {
292294
return res.redirect(buildGarageRedirectUrl('/', {
293-
error: 'Missing or invalid OAuth code',
295+
apiError: 'Missing or invalid OAuth code',
294296
}));
295297
}
296298

297299
if (!state || typeof state !== 'string') {
298300
return res.redirect(buildGarageRedirectUrl('/', {
299-
error: 'Missing or invalid state',
301+
apiError: 'Missing or invalid state',
300302
}));
301303
}
302304

@@ -310,7 +312,7 @@ export function createGitHubRouter(factories: ContextFactories): express.Router
310312
log('warn', `Invalid or expired state: ${sgr(state.slice(0, 8), Effect.ForegroundGray)}...`);
311313

312314
return res.redirect(buildGarageRedirectUrl('/', {
313-
error: 'Invalid or expired state. Please try connecting again.',
315+
apiError: 'Invalid or expired state. Please try connecting again.',
314316
}));
315317
}
316318

@@ -508,16 +510,47 @@ export function createGitHubRouter(factories: ContextFactories): express.Router
508510
/**
509511
* POST /integration/github/webhook
510512
* Handle GitHub App webhook events
513+
*
514+
* GitHub delivers signed webhook payloads as POST requests.
515+
* For non-POST methods (e.g. GET/HEAD checks), respond with 200 without signature validation.
511516
*/
512-
router.post('/webhook', express.raw({ type: 'application/json' }), async (req, res, next) => {
517+
router.all('/webhook', express.raw({ type: 'application/json' }), async (req, res, next) => {
518+
log('info', `${WEBHOOK_LOG_PREFIX}/webhook route called with method ${req.method}`);
519+
520+
/**
521+
* For non-POST methods (for example, GitHub hitting Setup URL or doing health checks),
522+
* just log query parameters and respond with 200 without signature validation.
523+
*/
524+
if (req.method !== 'POST') {
525+
const { code, installation_id, setup_action, state, ...restQuery } = req.query as Record<string, unknown>;
526+
527+
if (code || installation_id || state || setup_action) {
528+
// eslint-disable-next-line @typescript-eslint/camelcase, camelcase
529+
log('info', `${WEBHOOK_LOG_PREFIX}Received non-POST request on /webhook with OAuth-like params`, {
530+
// eslint-disable-next-line @typescript-eslint/camelcase, camelcase
531+
code,
532+
installation_id,
533+
setup_action,
534+
state,
535+
query: restQuery,
536+
});
537+
} else {
538+
log('info', `${WEBHOOK_LOG_PREFIX}Received non-POST request on /webhook without signature (likely a health check or misconfigured URL)`, {
539+
query: req.query,
540+
});
541+
}
542+
543+
return res.status(200).json({ ok: true });
544+
}
545+
513546
try {
514547
/**
515548
* Get webhook secret from environment
516549
*/
517550
const webhookSecret = process.env.GITHUB_WEBHOOK_SECRET;
518551

519552
if (!webhookSecret) {
520-
log('error', 'GITHUB_WEBHOOK_SECRET is not configured');
553+
log('error', `${WEBHOOK_LOG_PREFIX}GITHUB_WEBHOOK_SECRET is not configured`);
521554
res.status(500).json({ error: 'Webhook secret not configured' });
522555

523556
return;
@@ -530,7 +563,7 @@ export function createGitHubRouter(factories: ContextFactories): express.Router
530563
const signature = req.headers['x-hub-signature-256'] as string | undefined;
531564

532565
if (!signature) {
533-
log('warn', 'Missing X-Hub-Signature-256 header');
566+
log('warn', `${WEBHOOK_LOG_PREFIX}Missing X-Hub-Signature-256 header`);
534567

535568
return res.status(401).json({ error: 'Missing signature header' });
536569
}
@@ -569,7 +602,7 @@ export function createGitHubRouter(factories: ContextFactories): express.Router
569602
}
570603

571604
if (!signatureValid) {
572-
log('warn', 'Invalid webhook signature');
605+
log('warn', `${WEBHOOK_LOG_PREFIX}Invalid webhook signature`);
573606

574607
return res.status(401).json({ error: 'Invalid signature' });
575608
}
@@ -584,15 +617,15 @@ export function createGitHubRouter(factories: ContextFactories): express.Router
584617
try {
585618
payloadData = JSON.parse(payload.toString()) as GitHubWebhookPayload;
586619
} catch (error) {
587-
log('error', 'Failed to parse webhook payload:', error);
620+
log('error', `${WEBHOOK_LOG_PREFIX}Failed to parse webhook payload:`, error);
588621

589622
return res.status(400).json({ error: 'Invalid JSON payload' });
590623
}
591624

592625
const eventType = req.headers['x-github-event'] as string | undefined;
593626
const installationId = payloadData.installation?.id?.toString();
594627

595-
log('info', `Received webhook event: ${sgr(eventType || 'unknown', Effect.ForegroundCyan)}`);
628+
log('info', `${WEBHOOK_LOG_PREFIX}Received webhook event: ${sgr(eventType || 'unknown', Effect.ForegroundCyan)}`);
596629

597630
/**
598631
* Handle installation.deleted event

src/integrations/github/service.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,22 @@ export class GitHubService {
188188
return `https://github.com/apps/${this.appSlug}/installations/new?state=${encodeURIComponent(state)}&redirect_url=${encodeURIComponent(redirectUrl)}`;
189189
}
190190

191+
/**
192+
* Delete GitHub App installation by installation ID
193+
*
194+
* @param {string} installationId - GitHub App installation ID
195+
* @returns {Promise<void>}
196+
*/
197+
public async deleteInstallation(installationId: string): Promise<void> {
198+
const token = this.createJWT();
199+
const octokit = this.createOctokit(token);
200+
201+
await octokit.rest.apps.deleteInstallation({
202+
// eslint-disable-next-line @typescript-eslint/camelcase, camelcase
203+
installation_id: parseInt(installationId, 10),
204+
});
205+
}
206+
191207
/**
192208
* Get installation information
193209
*

src/resolvers/project.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const getEventsFactory = require('./helpers/eventsFactory').default;
99
const ProjectToWorkspace = require('../models/projectToWorkspace');
1010
const { dateFromObjectId } = require('../utils/dates');
1111
const ProjectModel = require('../models/project').default;
12+
const { GitHubService } = require('../integrations/github/service');
1213

1314
const EVENTS_GROUP_HASH_INDEX_NAME = 'groupHashUnique';
1415
const REPETITIONS_GROUP_HASH_INDEX_NAME = 'groupHash_hashed';
@@ -414,9 +415,20 @@ module.exports = {
414415
}
415416

416417
try {
418+
/**
419+
* If Task Manager is configured with GitHub and has installationId,
420+
* try to delete GitHub App installation as part of disconnect
421+
*/
422+
const taskManager = project.taskManager;
423+
424+
if (taskManager && taskManager.type === 'github' && taskManager.config?.installationId) {
425+
const githubService = new GitHubService();
426+
427+
await githubService.deleteInstallation(taskManager.config.installationId);
428+
}
429+
417430
/**
418431
* Remove taskManager field from project
419-
* Set updatedAt to current date
420432
*/
421433
return await project.updateProject({
422434
taskManager: null,

0 commit comments

Comments
 (0)