Skip to content

Commit e2630c5

Browse files
lilyshen0722sprint-impl
andcommitted
feat: route signed GitHub activity into bound pods
Add a flag-guarded GitHub App webhook with raw-body signature validation, explicit repository-to-pod binding, permanent delivery deduplication, and closed-catalog activity cards. Complete a matching task when its linked PR merges. Keep provider prose outside agent-visible content pending ruling. Written by Sprint Impl, a Commonly agent Co-authored-by: sprint-impl <WyJzcHJpbnQtaW1wbCIsImRlZmF1bHQiXQ@agents.commonly.invalid> Agent-Model: codex/gpt-6-sol/medium
1 parent cd37b32 commit e2630c5

24 files changed

Lines changed: 759 additions & 0 deletions
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
const crypto = require('crypto');
2+
const express = require('express');
3+
const request = require('supertest');
4+
5+
const mockBinding = jest.fn();
6+
const mockCreateReceipt = jest.fn();
7+
const mockHandle = jest.fn();
8+
jest.mock('../../../services/githubGigService', () => ({
9+
githubLocation: jest.requireActual('../../../services/githubGigService').githubLocation,
10+
boundPodForGithubEvent: (...args) => mockBinding(...args),
11+
}));
12+
jest.mock('../../../models/GithubGigDelivery', () => ({ create: (...args) => mockCreateReceipt(...args) }));
13+
jest.mock('../../../services/installable/eventHandlers', () => ({
14+
eventHandlers: { 'github.gig': (...args) => mockHandle(...args) },
15+
}));
16+
17+
const route = require('../../../routes/webhooks/github');
18+
const app = express();
19+
app.use('/api/webhooks/github', express.raw({ type: 'application/json' }));
20+
app.use(express.json());
21+
app.use('/api/webhooks/github', route);
22+
23+
const payload = {
24+
action: 'opened', installation: { id: 123 },
25+
repository: { owner: { login: 'Team-Commonly' }, name: 'commonly' },
26+
pull_request: { number: 42, title: 'External text' },
27+
};
28+
const deliver = (body, overrides = {}) => {
29+
const signature = `sha256=${crypto.createHmac('sha256', 'secret').update(body).digest('hex')}`;
30+
return request(app).post('/api/webhooks/github')
31+
.set('Content-Type', 'application/json')
32+
.set('X-Hub-Signature-256', signature)
33+
.set('X-GitHub-Event', 'pull_request')
34+
.set('X-GitHub-Delivery', '123e4567-e89b-12d3-a456-426614174000')
35+
.set(overrides)
36+
.send(body);
37+
};
38+
39+
beforeEach(() => {
40+
process.env.GITHUB_GIG_WEBHOOK_SECRET = 'secret';
41+
process.env.GITHUB_GIG_ENABLED = 'true';
42+
mockBinding.mockReset().mockResolvedValue(null);
43+
mockCreateReceipt.mockReset().mockResolvedValue({});
44+
mockHandle.mockReset().mockResolvedValue(undefined);
45+
});
46+
afterAll(() => {
47+
delete process.env.GITHUB_GIG_WEBHOOK_SECRET;
48+
delete process.env.GITHUB_GIG_ENABLED;
49+
});
50+
51+
test('missing and invalid signatures return 401 before parsing or lookup', async () => {
52+
const missing = await request(app).post('/api/webhooks/github')
53+
.set('Content-Type', 'application/json').send('{bad json');
54+
const invalid = await deliver('{bad json', { 'X-Hub-Signature-256': 'sha256=0000' });
55+
expect(missing.status).toBe(401);
56+
expect(invalid.status).toBe(401);
57+
expect(missing.text).toBe('');
58+
expect(mockBinding).not.toHaveBeenCalled();
59+
expect(mockCreateReceipt).not.toHaveBeenCalled();
60+
});
61+
62+
test('signature covers the raw bytes, including whitespace', async () => {
63+
const body = JSON.stringify(payload, null, 2);
64+
const response = await deliver(body);
65+
expect(response.status).toBe(204);
66+
expect(mockBinding).toHaveBeenCalledWith({
67+
installationId: '123', owner: 'Team-Commonly', repo: 'commonly', number: '42',
68+
});
69+
expect(mockCreateReceipt).not.toHaveBeenCalled();
70+
});
71+
72+
test('an unbound App-wide delivery is silent and does not write a receipt', async () => {
73+
const response = await deliver(JSON.stringify(payload));
74+
expect(response.status).toBe(204);
75+
expect(mockBinding).toHaveBeenCalledTimes(1);
76+
expect(mockCreateReceipt).not.toHaveBeenCalled();
77+
expect(mockHandle).not.toHaveBeenCalled();
78+
});
79+
80+
test('a bound delivery is claimed once and replay does not post twice', async () => {
81+
mockBinding.mockResolvedValue('aaaaaaaaaaaaaaaaaaaaaaaa');
82+
mockCreateReceipt.mockResolvedValueOnce({}).mockRejectedValueOnce(Object.assign(new Error('duplicate'), { code: 11000 }));
83+
const first = await deliver(JSON.stringify(payload));
84+
await new Promise((resolve) => setImmediate(resolve));
85+
const replay = await deliver(JSON.stringify(payload));
86+
expect(first.status).toBe(204);
87+
expect(replay.status).toBe(204);
88+
expect(mockCreateReceipt).toHaveBeenCalledTimes(2);
89+
expect(mockHandle).toHaveBeenCalledTimes(1);
90+
});
91+
92+
test('checks remain off until checks:read is granted', async () => {
93+
const check = { ...payload, action: 'completed', check_run: { conclusion: 'failure', pull_requests: [{ number: 42 }] } };
94+
const response = await deliver(JSON.stringify(check), { 'X-GitHub-Event': 'check_run' });
95+
expect(response.status).toBe(204);
96+
expect(mockBinding).not.toHaveBeenCalled();
97+
});
98+
99+
test.each([
100+
['pull_request_review', 'submitted', { pull_request: { number: 42 }, review: { state: 'approved', body: 'outsider' } }],
101+
['issues', 'opened', { issue: { number: 42, title: 'outsider' } }],
102+
['issues', 'closed', { issue: { number: 42, body: 'outsider' } }],
103+
])('%s %s routes only when bound', async (eventType, action, fields) => {
104+
mockBinding.mockResolvedValue('aaaaaaaaaaaaaaaaaaaaaaaa');
105+
const body = JSON.stringify({
106+
action, installation: payload.installation, repository: payload.repository, ...fields,
107+
});
108+
const response = await deliver(body, {
109+
'X-GitHub-Event': eventType,
110+
'X-GitHub-Delivery': `123e4567-e89b-12d3-a456-42661417400${action === 'closed' ? 1 : eventType === 'issues' ? 2 : 3}`,
111+
});
112+
await new Promise((resolve) => setImmediate(resolve));
113+
expect(response.status).toBe(204);
114+
expect(mockBinding).toHaveBeenCalledTimes(1);
115+
expect(mockCreateReceipt).toHaveBeenCalledTimes(1);
116+
expect(mockHandle).toHaveBeenCalledWith(expect.objectContaining({ eventType }));
117+
});
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
const express = require('express');
2+
const request = require('supertest');
3+
const mongoose = require('mongoose');
4+
const { MongoMemoryServer } = require('mongodb-memory-server');
5+
6+
jest.mock('../../../middleware/auth', () => (req, _res, next) => { req.user = { id: 'bbbbbbbbbbbbbbbbbbbbbbbb' }; next(); });
7+
jest.mock('../../../middleware/adminAuth', () => (req, res, next) => (
8+
req.get('x-admin') === 'yes' ? next() : res.status(403).end()
9+
));
10+
11+
let mongod;
12+
let app;
13+
let Pod;
14+
let Integration;
15+
let GithubGigBinding;
16+
const podId = new mongoose.Types.ObjectId();
17+
const adminId = new mongoose.Types.ObjectId();
18+
19+
beforeAll(async () => {
20+
mongod = await MongoMemoryServer.create();
21+
await mongoose.connect(mongod.getUri());
22+
Pod = require('../../../models/Pod');
23+
Integration = require('../../../models/Integration');
24+
GithubGigBinding = require('../../../models/GithubGigBinding');
25+
app = express();
26+
app.use(express.json());
27+
app.use('/api/integrations', require('../../../routes/integrations'));
28+
await Pod.create({ _id: podId, name: 'Build', createdBy: adminId, members: [adminId] });
29+
});
30+
afterAll(async () => { await mongoose.disconnect(); if (mongod) await mongod.stop(); });
31+
32+
test('only an admin can bind and unbind a connected App repository to a real pod', async () => {
33+
const row = await Integration.create({
34+
installationId: '7654321', type: 'github-app', scope: 'user', status: 'connected',
35+
config: { installationId: '7654321', owner: 'Team-Commonly', repo: 'commonly' },
36+
createdBy: adminId, isActive: true,
37+
});
38+
const path = `/api/integrations/github-app/${row._id}/gig-pod`;
39+
const denied = await request(app).put(path).send({ podId: String(podId) });
40+
expect(denied.status).toBe(403);
41+
expect(await GithubGigBinding.countDocuments()).toBe(0);
42+
43+
const missing = await request(app).put(path).set('x-admin', 'yes').send({ podId: String(new mongoose.Types.ObjectId()) });
44+
expect(missing.status).toBe(404);
45+
const bound = await request(app).put(path).set('x-admin', 'yes').send({ podId: String(podId) });
46+
expect(bound.status).toBe(200);
47+
expect(String((await GithubGigBinding.findOne({ installationId: '7654321' })).podId)).toBe(String(podId));
48+
49+
const secondRepo = await request(app).put(path).set('x-admin', 'yes')
50+
.send({ podId: String(podId), owner: 'Team-Commonly', repo: 'another-repo' });
51+
expect(secondRepo.status).toBe(200);
52+
expect(await GithubGigBinding.countDocuments({ installationId: '7654321' })).toBe(2);
53+
54+
const unbound = await request(app).delete(path).set('x-admin', 'yes');
55+
expect(unbound.status).toBe(204);
56+
expect(await GithubGigBinding.countDocuments()).toBe(1);
57+
});
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
const mockIntegrationFind = jest.fn();
2+
const mockTaskUpdate = jest.fn();
3+
const mockPost = jest.fn();
4+
const mockEmit = jest.fn();
5+
jest.mock('../../../models/GithubGigBinding', () => ({ findOne: (...args) => mockIntegrationFind(...args) }));
6+
jest.mock('../../../models/Task', () => ({ findOneAndUpdate: (...args) => mockTaskUpdate(...args) }));
7+
jest.mock('../../../services/agentIdentityService', () => ({ getOrCreateAgentUser: jest.fn().mockResolvedValue({ _id: 'bot' }) }));
8+
jest.mock('../../../services/agentMessageService', () => ({ _postToTarget: (...args) => mockPost(...args) }));
9+
jest.mock('../../../services/taskEventService', () => ({ emitTaskUpdated: (...args) => mockEmit(...args) }));
10+
11+
const {
12+
githubLocation, boundPodForGithubEvent, buildGithubGigCard,
13+
completeTaskForMergedPull, handleGithubGig,
14+
} = require('../../../services/githubGigService');
15+
16+
const podId = 'aaaaaaaaaaaaaaaaaaaaaaaa';
17+
const location = { installationId: '7654321', owner: 'Team-Commonly', repo: 'commonly', number: '42' };
18+
const payload = {
19+
action: 'closed', installation: { id: 7654321 },
20+
repository: { owner: { login: 'Team-Commonly' }, name: 'commonly' },
21+
pull_request: { number: 42, merged: true, title: '<script>outsider</script>' },
22+
};
23+
24+
beforeEach(() => {
25+
jest.clearAllMocks();
26+
mockPost.mockResolvedValue({ message: { id: 'm1' } });
27+
mockTaskUpdate.mockResolvedValue(null);
28+
});
29+
30+
test('accepts only structural repository coordinates and numeric ids', () => {
31+
expect(githubLocation(payload)).toEqual(location);
32+
expect(githubLocation({ ...payload, repository: { owner: { login: 'github.com.evil' }, name: 'repo/x' } })).toBeNull();
33+
expect(githubLocation({ ...payload, pull_request: { number: '1/../../admin' } })).toBeNull();
34+
});
35+
36+
test('one indexed connection lookup selects an explicitly bound pod', async () => {
37+
mockIntegrationFind.mockReturnValue({ select: () => ({ lean: async () => ({ podId }) }) });
38+
expect(await boundPodForGithubEvent(location)).toBe(podId);
39+
expect(mockIntegrationFind).toHaveBeenCalledWith(expect.objectContaining({
40+
installationId: location.installationId, owner: location.owner, repo: location.repo,
41+
}));
42+
});
43+
44+
test('card excludes outsider text and builds a canonical GitHub URL', () => {
45+
const card = buildGithubGigCard('pull_request', payload, location);
46+
expect(JSON.stringify(card)).not.toContain('outsider');
47+
expect(card.elements[2].href).toBe('https://github.com/Team-Commonly/commonly/pull/42');
48+
expect(card.elements[0].children[0].text).toBe('Merged');
49+
});
50+
51+
test('a merged PR completes only its matching open task and emits a board update', async () => {
52+
const task = { taskId: 'TASK-11', status: 'done' };
53+
mockTaskUpdate.mockResolvedValue(task);
54+
const href = 'https://github.com/Team-Commonly/commonly/pull/42';
55+
await completeTaskForMergedPull(podId, href);
56+
const [filter, update] = mockTaskUpdate.mock.calls[0];
57+
expect(String(filter.podId)).toBe(podId);
58+
expect(filter.prUrl).toBe(href);
59+
expect(filter.status).toEqual({ $in: ['pending', 'claimed'] });
60+
expect(update.$set.status).toBe('done');
61+
expect(mockEmit).toHaveBeenCalledWith(podId, task, 'updated');
62+
});
63+
64+
test('posts a display card while agent-visible content stays fixed', async () => {
65+
await handleGithubGig({ eventType: 'pull_request', payload, podId, location });
66+
const posted = mockPost.mock.calls[0][0];
67+
expect(posted.content).toBe('[GitHub activity]');
68+
expect(posted.payload.kind).toBe('gig-card');
69+
expect(posted.noOutboundRelay).toBe(true);
70+
expect(JSON.stringify(posted.payload)).not.toContain('outsider');
71+
expect(mockTaskUpdate).toHaveBeenCalledTimes(1);
72+
});

‎backend/models/GithubGigBinding.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import mongoose, { Document, Model, Schema, Types } from 'mongoose';
2+
3+
/** Explicit opt-in for one App installation/repository to post in one pod. */
4+
export interface IGithubGigBinding extends Document {
5+
installationId: string;
6+
owner: string;
7+
repo: string;
8+
podId: Types.ObjectId;
9+
createdBy: Types.ObjectId;
10+
}
11+
12+
const GithubGigBindingSchema = new Schema<IGithubGigBinding>({
13+
installationId: { type: String, required: true },
14+
owner: { type: String, required: true },
15+
repo: { type: String, required: true },
16+
podId: { type: Schema.Types.ObjectId, ref: 'Pod', required: true },
17+
createdBy: { type: Schema.Types.ObjectId, ref: 'User', required: true },
18+
}, { timestamps: true });
19+
20+
GithubGigBindingSchema.index({ installationId: 1, owner: 1, repo: 1 }, { unique: true });
21+
22+
export const GithubGigBinding: Model<IGithubGigBinding> = mongoose.model<IGithubGigBinding>('GithubGigBinding', GithubGigBindingSchema);
23+
export default GithubGigBinding;
24+
// eslint-disable-next-line @typescript-eslint/no-require-imports
25+
module.exports = exports["default"]; Object.assign(module.exports, exports);
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import mongoose, { Document, Model, Schema } from 'mongoose';
2+
3+
/** Permanent replay receipt: manual GitHub redelivery has no 24-hour limit. */
4+
export interface IGithubGigDelivery extends Document {
5+
deliveryId: string;
6+
createdAt: Date;
7+
}
8+
9+
const GithubGigDeliverySchema = new Schema<IGithubGigDelivery>({
10+
deliveryId: { type: String, required: true, unique: true },
11+
}, { timestamps: { createdAt: true, updatedAt: false } });
12+
13+
export const GithubGigDelivery: Model<IGithubGigDelivery> = mongoose.model<IGithubGigDelivery>('GithubGigDelivery', GithubGigDeliverySchema);
14+
export default GithubGigDelivery;
15+
// eslint-disable-next-line @typescript-eslint/no-require-imports
16+
module.exports = exports["default"]; Object.assign(module.exports, exports);

‎backend/models/Task.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,10 @@ const TaskSchema = new Schema<ITask>(
8787

8888
TaskSchema.index({ podId: 1, status: 1 });
8989
TaskSchema.index({ podId: 1, assignee: 1, status: 1 });
90+
TaskSchema.index(
91+
{ podId: 1, prUrl: 1, status: 1 },
92+
{ partialFilterExpression: { prUrl: { $type: 'string' } } },
93+
);
9094
TaskSchema.index({ podId: 1, taskId: 1 }, { unique: true });
9195
// Partial, NOT sparse: a compound sparse index still indexes any doc where
9296
// podId is present (i.e. all of them), so the second no-sourceRef task in a

‎backend/routes/integrations.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ const hostedMcpConnectRoutes = require('./hostedMcpConnect');
1313
// eslint-disable-next-line global-require
1414
const Integration = require('../models/Integration');
1515
// eslint-disable-next-line global-require
16+
const GithubGigBinding = require('../models/GithubGigBinding');
17+
// eslint-disable-next-line global-require
1618
const DiscordIntegration = require('../models/DiscordIntegration');
1719
// eslint-disable-next-line global-require
1820
const DiscordService = require('../services/discordService');
@@ -267,6 +269,42 @@ router.post('/github-app', writeIntegrationsRateLimit, auth, adminAuth, async (r
267269
}
268270
});
269271

272+
// Explicit admin bind for the App-wide activity webhook. Connection creation
273+
// alone never opts a repository into a pod or agent-visible activity.
274+
router.put('/github-app/:id/gig-pod', writeIntegrationsRateLimit, auth, adminAuth, async (req: AuthReq, res: Res) => {
275+
const podId = String(req.body?.podId || '');
276+
const integrationId = String(req.params?.id || '');
277+
if (!Types.ObjectId.isValid(podId)) return res.status(400).json({ message: 'Valid podId required' });
278+
if (!Types.ObjectId.isValid(integrationId)) return res.status(404).json({ message: 'GitHub App connection not found' });
279+
const integration = await Integration.findOne({ _id: integrationId, type: 'github-app', isActive: true, status: 'connected' }).lean();
280+
if (!integration) return res.status(404).json({ message: 'GitHub App connection not found' });
281+
const owner = String(req.body?.owner || integration.config?.owner || '');
282+
const repo = String(req.body?.repo || integration.config?.repo || '');
283+
if (!/^[A-Za-z0-9_.-]{1,100}$/.test(owner) || !/^[A-Za-z0-9_.-]{1,100}$/.test(repo)) {
284+
return res.status(400).json({ message: 'Valid owner and repo required' });
285+
}
286+
const pod = await Pod.exists({ _id: new Types.ObjectId(podId) });
287+
if (!pod) return res.status(404).json({ message: 'Pod not found' });
288+
const binding = await GithubGigBinding.findOneAndUpdate(
289+
{ installationId: String(integration.installationId), owner, repo },
290+
{ $set: { podId: new Types.ObjectId(podId) }, $setOnInsert: { createdBy: new Types.ObjectId(String(req.user?.id)) } },
291+
{ new: true, upsert: true },
292+
);
293+
return res.json({ binding });
294+
});
295+
296+
router.delete('/github-app/:id/gig-pod', writeIntegrationsRateLimit, auth, adminAuth, async (req: AuthReq, res: Res) => {
297+
const integrationId = String(req.params?.id || '');
298+
if (!Types.ObjectId.isValid(integrationId)) return res.status(404).json({ message: 'GitHub App connection not found' });
299+
const integration = await Integration.findOne({ _id: integrationId, type: 'github-app' }).lean();
300+
if (!integration) return res.status(404).json({ message: 'GitHub App connection not found' });
301+
const owner = String(req.body?.owner || integration.config?.owner || '');
302+
const repo = String(req.body?.repo || integration.config?.repo || '');
303+
if (!/^[A-Za-z0-9_.-]{1,100}$/.test(owner) || !/^[A-Za-z0-9_.-]{1,100}$/.test(repo)) return res.status(400).json({ message: 'Valid owner and repo required' });
304+
await GithubGigBinding.deleteOne({ installationId: String(integration.installationId), owner, repo });
305+
return res.status(204).json({});
306+
});
307+
270308
router.post('/ingest', ingestAuth, async (req: AuthReq, res: Res) => {
271309
try {
272310
const { provider, integrationId, event, messages } = (req.body || {}) as { provider?: string; integrationId?: string; event?: unknown; messages?: unknown[] };

0 commit comments

Comments
 (0)