Skip to content

Add Phase 1 shadow GitHub webhook ingestion - #226

Merged
ecarreras merged 3 commits into
mainfrom
feat/issue-191-phase-1
Oct 2, 2026
Merged

ecarreras merged 3 commits into
mainfrom
feat/issue-191-phase-1

Conversation

@giscebot

@giscebot giscebot commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • add a signed POST /api/webhooks/github endpoint in shadow-only mode
  • persist delivery metadata, payload hashes, canonical event keys, and duplicate counts without storing raw payloads
  • support independent current/rotation secrets per GitHub organization or repository owner, with an explicit owner allowlist
  • restrict shadow metrics to dashboard administrators
  • add an admin-only dashboard tab for webhook shadow status, visible only when webhook ingestion is configured
  • document step-by-step repository/organization webhook setup and maintain an inventory of GitHub webhook event support
  • keep comment/review edits observational and separate from actionable created identities

Safety boundary

This implements only Phase 1 of #191. Webhooks do not create jobs, do not claim rows in github_events, and do not replace IMAP. Phase 2 must explicitly define enqueue policy and recovery before dual ingest is enabled.

The legacy single-secret configuration remains available for one trusted owner, but accepts any repository signed with that secret. Multi-owner deployments should use GITHUB_AGENT_BRIDGE_WEBHOOK_SECRETS_BY_OWNER so each owner has an independent secret and rotation window.

The webhook dashboard tab and status endpoint are restricted to dashboard administrators. The tab is hidden when no webhook secret configuration is active.

Validation

  • pytest -q — 380 passed
  • dashboard npm test -- --run — 54 passed
  • dashboard npm run build — passed

Requested by: @ecarreras

Co-authored-by: ecarreras <294235+ecarreras@users.noreply.github.com>

@ecarreras ecarreras left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Em falta documentació de com configurar el webhook a github, quines opcions marcar, com encaixa això amb les orgnaitzacions, es pot tenir més d'una organització configurada? es pot configurar només un webhook de repo.

Comment thread docs/ingestion.md Outdated
@giscebot

giscebot commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

Tens raó: la documentació actual no és suficient per operar-ho i, revisant el codi, cal aclarir també una limitació real de la implementació.

Configuració que hauria de quedar documentada:

  • Payload URL: https://<host>/api/webhooks/github
  • Content type: application/json
  • Secret: el mateix valor que GITHUB_AGENT_BRIDGE_WEBHOOK_SECRET
  • SSL verification: activada
  • Events individuals: Issue comments, Pull request reviews, Pull request review comments, Commit comments i Workflow runs; no cal Send me everything
  • El webhook ha d'estar Active. En aquesta fase només observa i persisteix metadades; no crea jobs.

Sobre l'abast:

  • Webhook de repositori: sí, funciona per un únic repo.
  • Webhook d'organització: sí, un webhook creat a l'organització envia els events dels seus repositoris a aquest mateix endpoint.
  • Més d'una organització: tècnicament només funciona ara si tots els webhooks comparteixen el mateix secret. La configuració només admet secret actual + secret anterior per rotació; no admet secrets independents per organització.

Això últim és un límit que no hauríem de dissimular: el codi actual tampoc valida installation, organització ni una allowlist de repositoris, malgrat que la taula de documentació diu “installation/repository policy”. Per tant, aquest PR és un webhook genèric signat en shadow, no encara una integració GitHub App amb scope verificat.

Abans de merge demanaria:

  1. afegir la guia pas a pas anterior;
  2. documentar explícitament repo webhook vs org webhook i el límit multi-org;
  3. corregir la descripció de trust perquè coincideixi amb el que realment implementa;
  4. deixar decidit si Phase 1 accepta provisionalment un secret compartit o si volem secrets independents per org/instal·lació. Jo prefereixo secrets independents: evita ampliar el blast radius i permet rotar una organització sense afectar les altres.

@ecarreras

Copy link
Copy Markdown
Member

@giscebot fes el que comentes i també estaria bé saber tenir un inventari de tots els esdeveniments que existeixen i quins donem suport, potser això més endavant ens dona joc per fer més integracions i altres coses.

Co-authored-by: ecarreras <294235+ecarreras@users.noreply.github.com>
@giscebot

giscebot commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

Fet a b0a528b:

  • guia pas a pas per configurar webhooks de repo i d’organització, amb els cinc events de Phase 1;
  • secrets actual/anterior independents per owner via GITHUB_AGENT_BRIDGE_WEBHOOK_SECRETS_BY_OWNER; quan està configurat també actua com allowlist i rebutja owners desconeguts;
  • /api/webhooks/github/status ara exigeix rol admin del dashboard (401 sense sessió, 403 per usuari autenticat no admin);
  • inventari mantingut dels events documentats per GitHub, separant els 5 amb clau canònica dels observats/no seleccionats, perquè serveixi de backlog per futures integracions;
  • trust model corregit: no afirmem validació d’installation que encara no existeix.

Validació: suite completa 379 passed; suite webhook 8 passed. També he actualitzat el cos de la PR perquè reflecteixi l’abast i el risc real de Phase 1.

@ecarreras

Copy link
Copy Markdown
Member

@giscebot pots fer la part de dashboard per poder anar monitoritzant el webhook com va, en aquesta fase seria mostrar la informació de l'status que ja s'exporta i activar només la pestanya al dashboard si està activat

Co-authored-by: ecarreras <294235+ecarreras@users.noreply.github.com>
@giscebot

giscebot commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

Fet al commit 4bc0b43.

  • nova pestanya Webhooks al dashboard, visible només per administradors i només quan hi ha secrets de webhook configurats;
  • mostra directament l’status existent: mode shadow, receipts totals i per estat, duplicats i coincidències entre webhook/IMAP;
  • la ruta frontend /webhooks conserva l’autenticació del dashboard i les dades continuen protegides per l’endpoint admin-only;
  • artefactes del dashboard regenerats i cos de la PR actualitzat.

Validació: 380 tests Python, 54 tests de dashboard i build de producció correctes.

@ecarreras
ecarreras merged commit 604df2e into main Oct 2, 2026
3 checks passed
@ecarreras
ecarreras deleted the feat/issue-191-phase-1 branch October 2, 2026 10:25
@giscebot

giscebot commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

Auditoria post-merge: hi ha dos punts operatius que convé resoldre abans de considerar Phase 1 llesta per exposar a producció:

  1. GITHUB_AGENT_BRIDGE_WEBHOOK_MAX_BYTES no limita realment la memòria consumida quan la petició arriba sense Content-Length (p. ex. transferència chunked): await request.body() materialitza tot el payload i el len(...) es comprova després. Cal llegir l’stream incrementalment i abortar en superar el límit, o imposar el límit al reverse proxy i documentar-lo explícitament com a frontera de seguretat.
  2. webhook_shadow_receipts no té cap política ni procés de retenció. En shadow cada delivery queda persistida indefinidament i el dashboard fa agregacions sobre tota la taula; això farà créixer disc i cost de consulta sense límit. Proposo una retenció configurable (amb una finestra prou llarga per comparar IMAP/webhook) i que l’status exposi la finestra observada.

No he modificat la branca ni metadades (review-only). La inspecció estàtica confirma que l’endpoint d’status és admin-only i que els secrets per owner actuen com allowlist. No he pogut repetir tests/test_webhook.py localment perquè aquest checkout no té fastapi instal·lat; el bloqueig és de l’entorn local, no una fallada del test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants