feat(webhook): watched_repos table + ingress switch + boot migration (#485) #506

Merged

claude-desktop merged 1 commit from boss/485 into main

2026-04-28 06:34:19 +00:00

code-lead commented

2026-04-28 00:09:49 +00:00

Collaborator

Summary

Replace the in-memory agents.json#/repos whitelist with a SQLite watched_repos table keyed on (forge_type, owner, name). Per-repo HMAC secrets live on the row (webhook_secret); migrated rows fall back to the shared per-forge secret file via webhook_id IS NULL.

DB schema + helpers (infrastructure/database/db.ts): watched_repos table with getWatchedRepo, listWatchedRepos, listActiveWatchedRepos, upsertWatchedRepo, disableWatchedRepo, getActiveForgeRaw.
F1 boot migration (shared/config/webhook-config.ts): one-shot — when watched_repos is empty, copy agents.json#/repos in, atomically rewrite agents.json without repos, write a .pre-f1.bak backup, log [f1-migration] copied N repos from agents.json; backup at …. Subsequent boots that find repos: reappearing log a warning and ignore it.
Ingress switch (http/webhook.ts): all three forge handlers look up the watched-repo row before verifying the signature/token (so per-repo secrets work) and check service_settings.active_forge_type — inactive forges short-circuit to 202 Accepted. GitHub + GitLab routes verify auth BEFORE the unhandled-event 204 short-circuit so an unrecognised event header can't bypass the auth gate.
isKnownRepo(forge, owner, name) now reads watched_repos directly. Empty table → reject (no back-compat).

Closes #485

Test plan

bun test apps/server/src/{infrastructure/database/db,shared/config/webhook-config,http/webhook,http/webhook-normalize,http/forge-event-broadcast,domain/workflow/event-handlers}.test.ts — 291 pass / 0 fail
bun x turbo run typecheck — green across all 4 workspaces
bun x @biomejs/biome@^2 check . — only unrelated infos
F1 migration: copy + rewrite + backup all covered; reappearing repos: warns + ignores
Per-repo secret precedence: F5-registered row rejects shared-secret fallback; migrated row accepts it
Active-forge gate: 202 on mismatch, 200/204 on match, 200/204 when nothing configured
Disabled row → 404; unknown repo → 404 with forge=<type> repo=<owner/name> log

🤖 Generated with Claude Code

## Summary Replace the in-memory `agents.json#/repos` whitelist with a SQLite `watched_repos` table keyed on `(forge_type, owner, name)`. Per-repo HMAC secrets live on the row (`webhook_secret`); migrated rows fall back to the shared per-forge secret file via `webhook_id IS NULL`. - **DB schema + helpers** (`infrastructure/database/db.ts`): `watched_repos` table with `getWatchedRepo`, `listWatchedRepos`, `listActiveWatchedRepos`, `upsertWatchedRepo`, `disableWatchedRepo`, `getActiveForgeRaw`. - **F1 boot migration** (`shared/config/webhook-config.ts`): one-shot — when `watched_repos` is empty, copy `agents.json#/repos` in, atomically rewrite `agents.json` without `repos`, write a `.pre-f1.bak` backup, log `[f1-migration] copied N repos from agents.json; backup at …`. Subsequent boots that find `repos:` reappearing log a warning and ignore it. - **Ingress switch** (`http/webhook.ts`): all three forge handlers look up the watched-repo row before verifying the signature/token (so per-repo secrets work) and check `service_settings.active_forge_type` — inactive forges short-circuit to 202 Accepted. GitHub + GitLab routes verify auth BEFORE the unhandled-event 204 short-circuit so an unrecognised event header can't bypass the auth gate. - **`isKnownRepo(forge, owner, name)`** now reads `watched_repos` directly. Empty table → reject (no back-compat). Closes #485 ## Test plan - [x] `bun test apps/server/src/{infrastructure/database/db,shared/config/webhook-config,http/webhook,http/webhook-normalize,http/forge-event-broadcast,domain/workflow/event-handlers}.test.ts` — 291 pass / 0 fail - [x] `bun x turbo run typecheck` — green across all 4 workspaces - [x] `bun x @biomejs/biome@^2 check .` — only unrelated infos - [x] F1 migration: copy + rewrite + backup all covered; reappearing `repos:` warns + ignores - [x] Per-repo secret precedence: F5-registered row rejects shared-secret fallback; migrated row accepts it - [x] Active-forge gate: 202 on mismatch, 200/204 on match, 200/204 when nothing configured - [x] Disabled row → 404; unknown repo → 404 with `forge=<type> repo=<owner/name>` log 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit

2026-04-28 00:09:49 +00:00

feat(webhook): watched_repos table + ingress switch + boot migration (#485 )

qa / qa (pull_request) Successful in 8m41s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

d1eff02684

Replace the in-memory `agents.json#/repos` whitelist with a SQLite
`watched_repos` table keyed on `(forge_type, owner, name)`. Per-repo
HMAC secrets live on the row (`webhook_secret`) so the ingress can
verify deliveries against a per-repo key once the F5 OAuth path
registers them; migrated rows fall back to the shared per-forge
secret file via `webhook_id IS NULL`.

A one-shot F1 boot migration copies `agents.json#/repos` into the
table when it is empty, atomically rewrites `agents.json` without
the `repos` field, and writes a `.pre-f1.bak` backup. Subsequent
boots that find `repos:` reappearing log a warning and ignore it
so a stale config edit can't override the DB.

The forge route handlers now look up the row before verifying the
signature/token (so per-repo secrets work) and check the active
forge gate via `service_settings.active_forge_type` — inactive
forges short-circuit to 202 Accepted instead of dispatching. The
GitHub + GitLab routes verify auth BEFORE the unhandled-event 204
short-circuit so an unrecognised event header can't bypass the
auth gate.

Closes #485

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead requested review from reviewer

2026-04-28 00:59:10 +00:00

reviewer approved these changes

2026-04-28 01:05:28 +00:00

reviewer left a comment

All acceptance criteria from #485 verified, CI green, test coverage solid.

watched_repos schema + helpers correct; upsertWatchedRepo update semantics (undefined = preserve, null = clear) properly tested.
F1 migration covers all four states (migrate+rewrite+backup; noop steady-state; warn+ignore when table already populated; noop on empty repos array).
Ingress ordering is correct for all three forges: repo lookup → per-repo secret selection → HMAC/token verify → active-forge gate → dispatch. GitHub + GitLab auth-before-204 ordering is right.
pickWebhookSecret: F5-registered rows (webhook_id != null) correctly reject the shared-secret fallback; migrated rows (webhook_id IS NULL) fall through to the live config. Security property tested explicitly.

behavior (nit, not blocking): runWatchedReposBootMigration (webhook-config.ts ~line 2143) iterates the repoBindings loop calling upsertWatchedRepo without wrapping in a SQLite transaction. The JSDoc states "a write error throws and aborts boot rather than leaving the table half-populated" — but that guarantee requires the inserts to be atomic. A genuine DB failure mid-loop (disk full etc.) would leave N-of-M repos inserted; next boot sees existing.length > 0, skips the migration, and the remaining repos are silently lost. Fix: wrap the loop in getDb().transaction(() => { for (const b of repoBindings) upsertWatchedRepo({…}); })(). The comment should then be accurate.

test-gap (nit, not blocking): handleGitHubWebhook and handleGitLabWebhook have no direct test coverage for the F4 per-repo secret path or active-forge 202 gate. The Forgejo suite covers every case exhaustively; the GH/GL paths follow the same pattern, so correctness is high — but the auth-before-204 ordering (a security property) goes un-exercised for those two routes. Worth adding in a follow-up.

All acceptance criteria from #485 verified, CI green, test coverage solid. - `watched_repos` schema + helpers correct; `upsertWatchedRepo` update semantics (undefined = preserve, null = clear) properly tested. - F1 migration covers all four states (migrate+rewrite+backup; noop steady-state; warn+ignore when table already populated; noop on empty repos array). - Ingress ordering is correct for all three forges: repo lookup → per-repo secret selection → HMAC/token verify → active-forge gate → dispatch. GitHub + GitLab auth-before-204 ordering is right. - `pickWebhookSecret`: F5-registered rows (`webhook_id != null`) correctly reject the shared-secret fallback; migrated rows (`webhook_id IS NULL`) fall through to the live config. Security property tested explicitly. **behavior** (nit, not blocking): `runWatchedReposBootMigration` (`webhook-config.ts` ~line 2143) iterates the `repoBindings` loop calling `upsertWatchedRepo` without wrapping in a SQLite transaction. The JSDoc states "a write error throws and aborts boot rather than leaving the table half-populated" — but that guarantee requires the inserts to be atomic. A genuine DB failure mid-loop (disk full etc.) would leave N-of-M repos inserted; next boot sees `existing.length > 0`, skips the migration, and the remaining repos are silently lost. Fix: wrap the loop in `getDb().transaction(() => { for (const b of repoBindings) upsertWatchedRepo({…}); })()`. The comment should then be accurate. **test-gap** (nit, not blocking): `handleGitHubWebhook` and `handleGitLabWebhook` have no direct test coverage for the F4 per-repo secret path or active-forge 202 gate. The Forgejo suite covers every case exhaustively; the GH/GL paths follow the same pattern, so correctness is high — but the auth-before-204 ordering (a security property) goes un-exercised for those two routes. Worth adding in a follow-up.

code-lead force-pushed boss/485 from d1eff02684

qa / qa (pull_request) Successful in 8m41s

Details

qa / dockerfile (pull_request) Successful in 9s

Details

to af9307056b

qa / qa (pull_request) Successful in 8m34s

Details

qa / dockerfile (pull_request) Successful in 10s

Details

2026-04-28 01:05:35 +00:00

Compare