feat: Enhance documentation with architecture overview, design decisions, and security considerations

ColinDaglish · ColinDaglish · commit f7046dea4c38 · 2026-03-20T12:13:01.000Z
diff --git a/.gitignore b/.gitignore
@@ -35,6 +35,13 @@ outputs/*
 *.tif
 *.svg
 
+# Documentation assets: allow images that are intentionally part of the docs.
+!docs/assets/**/*.png
+!docs/assets/**/*.jpg
+!docs/assets/**/*.jpeg
+!docs/assets/**/*.tif
+!docs/assets/**/*.svg
+
 # The following was created by https://www.toptal.com/developers/gitignore/api/macos,windows,r,python
 # Edit at https://www.toptal.com/developers/gitignore?templates=macos,windows,r,python
 
diff --git a/README.md b/README.md
@@ -7,6 +7,8 @@ A GitHub App backend for secure webhook processing and automation, deployed on G
 
 If you want to replicate the full setup (GCP remote state, Terraform apply phases, GitHub App creation, deploy + verification), follow the end-to-end tutorial in `docs/tutorial/README.md`.
 
+For architecture and security background (intended for technical architects, security colleagues, and interested engineers), see `docs/architecture/README.md`.
+
 ## Features
 
 - FastAPI-based webhook handler (`/webhooks/github`)
diff --git a/docs/architecture/README.md b/docs/architecture/README.md
@@ -0,0 +1,101 @@
+# Architecture overview (ons-github-app)
+
+This folder documents how `ons-github-app` is put together, why key design decisions were made, and what the security posture looks like.
+
+Audience:
+
+- Technical architects (cloud topology, IaC, operational model)
+- Security colleagues (threat model, key controls, “what could go wrong”)
+- Interested engineers (how the pieces fit)
+
+If you want to *deploy* the system end-to-end, start with the setup tutorial at `docs/tutorial/README.md`.
+
+## What the system does (today)
+
+`ons-github-app` is a small FastAPI service that receives GitHub webhooks at `POST /webhooks/github`.
+
+Current demo behavior:
+
+- Verify webhook authenticity using `X-Hub-Signature-256` (HMAC-SHA256)
+- For `pull_request` events with `action=opened`, post a comment back to the PR as the GitHub App
+
+## High-level topology
+
+Inbound webhook traffic flows through API Gateway to Cloud Run.
+
+```mermaid
+flowchart LR
+  GH[GitHub Webhooks] -->|HTTPS POST /webhooks/github| GW[GCP API Gateway]
+  GW -->|HTTPS| CR[Cloud Run service]
+  CR --> APP[FastAPI app]
+
+  APP -->|Reads secrets as files| SM[GCP Secret Manager]
+  APP -->|JWT -> installation token| GHA[GitHub REST API]
+```
+
+Key components:
+
+- **FastAPI app** (runtime): `src/app.py`, `src/webhook.py`, `src/github_app.py`, `src/config.py`
+- **Cloud Run** (compute): runs the container and scales based on request volume
+- **API Gateway** (ingress): routes `POST /webhooks/github` to Cloud Run
+- **Secret Manager** (secrets): stores the GitHub App private key and webhook secret
+- **Artifact Registry** (images): stores container images deployed to Cloud Run
+- **Terraform** (IaC): declares and provisions all the above
+
+## Endpoints
+
+- `GET /healthz` — liveness/health endpoint
+- `POST /webhooks/github` — GitHub webhook receiver
+
+## Data flow: webhook verification and response
+
+1. GitHub sends a webhook payload and includes `X-Hub-Signature-256: sha256=<digest>`
+2. The app reads the raw request body bytes
+3. The app recomputes the expected digest using the shared webhook secret
+4. The app compares digests using a constant-time comparison
+5. If valid and the event is accepted, the app executes the handler (e.g. post PR comment)
+
+## Secrets and configuration model
+
+This repo documents a single recommended approach:
+
+- Secrets are **stored as files**
+- The app receives **non-secret** environment variables that point at those files:
+  - `GITHUB_PRIVATE_KEY_FILE`
+  - `GITHUB_WEBHOOK_SECRET_FILE`
+
+Local development:
+
+- Keep secret files under `./local-secrets/` (ignored by git)
+
+Cloud Run:
+
+- Terraform provisions the Secret Manager *secret containers*
+- Secret *values* are added outside Terraform using `gcloud secrets versions add ...`
+- Terraform mounts the secrets into the container filesystem, and sets the `*_FILE` env vars
+
+## Infrastructure (Terraform) shape
+
+This repo is designed for a two-phase apply, described in `docs/tutorial/README.md`:
+
+1. **Bootstrap/shared infra** (no image)
+   - APIs enabled
+   - service account
+   - Artifact Registry
+   - Secret Manager secrets (containers only)
+
+2. **Deploy** (image provided)
+   - Cloud Run service
+   - API Gateway
+   - IAM binding granting API Gateway permission to invoke Cloud Run
+
+## Operational notes
+
+- Logging: the app logs to stdout/stderr; on Cloud Run this lands in Cloud Logging.
+- Scaling: Cloud Run scales horizontally with incoming requests. (No explicit autoscaling tuning is configured in Terraform today.)
+- CI: GitHub Actions runs Checkov and Trivy filesystem scans; locally, `pre-commit` includes detect-secrets and terraform checks.
+
+## Design decisions (where to read more)
+
+- See `docs/architecture/design-decisions.md` for the short “why we chose X” list.
+- See `docs/architecture/security.md` for a security-focused view (threats, controls, and open gaps).
diff --git a/docs/architecture/design-decisions.md b/docs/architecture/design-decisions.md
@@ -0,0 +1,105 @@
+# Design decisions
+
+This document captures the key architectural decisions in `ons-github-app` and the rationale behind them.
+
+## Cloud Run for compute
+
+Decision:
+
+- Run the webhook handler as a container on **Google Cloud Run**.
+
+Why:
+
+- Stateless request/response workload suits serverless containers.
+- Scales down when idle and scales up when webhook volume increases.
+- Avoids VM management and reduces operational overhead.
+
+Trade-offs:
+
+- Cold starts can add latency for the first request after idle.
+- Long-running background work is not a good fit; heavy jobs should be offloaded.
+
+## API Gateway in front of Cloud Run
+
+Decision:
+
+- Put **GCP API Gateway** in front of the Cloud Run service.
+
+Why:
+
+- Central place to route and evolve HTTP ingress without exposing Cloud Run URLs directly.
+- Keeps the public interface stable while Cloud Run revisions change.
+
+Trade-offs / notes:
+
+- API Gateway is currently configured as a simple reverse proxy using an OpenAPI spec.
+- Rate limiting / auth policies are not configured here today; webhook authenticity is enforced at the application layer.
+
+## GitHub webhook authenticity via HMAC signature verification
+
+Decision:
+
+- Treat the request body as untrusted and verify `X-Hub-Signature-256` using HMAC-SHA256.
+
+Why:
+
+- Prevents spoofed/fabricated webhook events from triggering automation.
+- Verification occurs before parsing JSON (avoids trusting payload content prematurely).
+
+Trade-offs:
+
+- Does not stop replay by itself (see security doc for mitigations/opportunities).
+
+## GitHub App authentication: JWT + installation tokens
+
+Decision:
+
+- Authenticate to GitHub as a GitHub App:
+  - sign a short-lived JWT using the app private key
+  - exchange it for an installation access token scoped to the installation
+
+Why:
+
+- Avoids long-lived personal access tokens.
+- Installation token scope is bounded to installed repos and expires.
+
+## Secrets strategy: files + Secret Manager
+
+Decision:
+
+- Secrets are not stored in Terraform variables.
+- Secret *values* live in **Secret Manager**, mounted as **files** into Cloud Run.
+- Locally, secrets are also files under `./local-secrets/`.
+
+Why:
+
+- Keeps secret values out of:
+  - git history
+  - container images
+  - Terraform state
+- Using files matches Cloud Run’s secret mount model.
+
+## Two-phase Terraform apply
+
+Decision:
+
+- Use a two-phase workflow:
+  1) apply shared infra with `image = ""`
+  2) push image + add secret versions + apply again
+
+Why:
+
+- Allows provisioning to succeed before an image exists.
+- Ensures secret versions can be added outside Terraform.
+
+## CI security scanning
+
+Decision:
+
+- Run IaC and repo scanning in CI:
+  - Checkov (IaC policy scanning)
+  - Trivy filesystem scan (dependency and config scanning)
+
+Why:
+
+- Prevents obvious misconfigurations and vulnerable dependencies from landing unreviewed.
diff --git a/docs/architecture/security.md b/docs/architecture/security.md
@@ -0,0 +1,71 @@
+# Security considerations
+
+This document is a security-focused view of `ons-github-app`.
+
+## Threat model (what we assume)
+
+- The webhook endpoint is internet reachable (via API Gateway).
+- Attackers may discover the endpoint URL.
+- Attackers may attempt:
+  - spoofing webhook events
+  - tampering with payloads in transit
+  - replaying valid payloads
+  - causing excessive request volume (resource/cost pressure)
+  - stealing secrets from source control, Terraform state, build logs, or runtime environment
+
+## Primary controls implemented
+
+### Webhook signature verification
+
+- The app verifies `X-Hub-Signature-256` using an HMAC-SHA256 digest of the raw request body.
+- Uses constant-time comparison.
+- Rejects invalid signatures with HTTP 401.
+
+Outcome:
+- Prevents forged payloads that don’t possess the shared webhook secret.
+
+### Short-lived GitHub App auth
+
+- The app signs a JWT (RS256) using the GitHub App private key.
+- Exchanges it for an installation access token.
+- Uses the installation token to call GitHub’s API.
+
+Outcome:
+- Avoids long-lived PATs and bounds token blast radius.
+
+### Secrets kept out of Terraform state and git
+
+- Terraform creates Secret Manager *secret containers* only.
+- Secret versions are added with `gcloud secrets versions add ...` outside Terraform.
+- Cloud Run mounts secrets as files; the app reads them via `*_FILE` env vars.
+- `./local-secrets/` is ignored via `.gitignore`.
+
+Outcome:
+- Reduces likelihood of accidental credential leakage.
+
+### Minimal exposed endpoints
+
+- The public API surface is intentionally small:
+  - `GET /healthz`
+  - `POST /webhooks/github`
+
+## Operational security notes
+
+- Logging: avoid logging secret material or full request bodies. This repo logs the GitHub event type/action only.
+- IAM: Cloud Run uses a dedicated service account. That service account is granted Secret Manager access only for the required secrets.
+
+## Known gaps / future hardening ideas
+
+These are not necessarily required for a demo, but are common asks from security review.
+
+- Replay resistance: GitHub webhooks include delivery IDs (e.g. `X-GitHub-Delivery`). Consider storing recent delivery IDs and rejecting duplicates.
+- Rate limiting / abuse controls: API Gateway is currently acting as a router. Consider adding quotas/rate limits and alerting on abnormal volume.
+- Structured audit logging: capture request metadata (delivery ID, event type, signature verification result) in a structured form.
+- Dependency management: requirements are pinned; consider adding dependency update automation and/or vulnerability gates on container images.
+
+## Where to look in code
+
+- Signature verification: `src/webhook.py`
+- Secret loading: `src/config.py`
+- GitHub App auth + API calls: `src/github_app.py`
+- Webhook handler routing: `src/app.py`
diff --git a/docs/assets/screenshot-bot-pr-comment.png b/docs/assets/screenshot-bot-pr-comment.png
diff --git a/docs/tutorial/04-deploy-and-verify.md b/docs/tutorial/04-deploy-and-verify.md
@@ -210,6 +210,11 @@ Set your GitHub App webhook URL to that value.
 
 1) Open a pull request in the repo where the app is installed.
 2) In GitHub → your PR, you should see a comment posted by the app.
+
+   Example output:
+
+   ![Example PR comment created by the GitHub App](../assets/screenshot-bot-pr-comment.png)
+
 3) If it doesn’t appear, check:
 
 - Cloud Run logs (look for `event=pull_request action=opened`)
diff --git a/docs/tutorial/README.md b/docs/tutorial/README.md
@@ -30,3 +30,9 @@ This repo is designed for a **two-phase Terraform apply**:
 - 02: Remote state bootstrap — see 02-remote-state.md
 - 03: GitHub App setup — see 03-github-app.md
 - 04: Deploy & verify (local + cloud) — see 04-deploy-and-verify.md
+
+## Architecture and security
+
+- Architecture overview — see ../architecture/README.md
+- Design decisions — see ../architecture/design-decisions.md
+- Security considerations — see ../architecture/security.md
