Merge pull request #1 from babs/feat/security-hardening

feat: security hardening with replay store, metrics, and rate limits

Author: babs

.github/workflows/release.yml

@@ -14,10 +14,11 @@ jobs:
   build-image:
     name: "Build OCI image"
     runs-on: ${{ matrix.os }}
+    # Build job only reads source and pushes images — no release creation
+    # happens here, so contents:read is sufficient.
     permissions:
-      contents: write
+      contents: read
       packages: write
-      attestations: write
       id-token: write
 
     strategy:
@@ -115,13 +116,14 @@ jobs:
     name: "Merge platform images into one"
     needs: build-image
     runs-on: ubuntu-latest
+    # contents:write is required to create the GitHub Release below.
+    # packages:write lets docker manifest push into GHCR.
     permissions:
       contents: write
       packages: write
-      attestations: write
-      id-token: write
 
     steps:
+      - uses: actions/checkout@v4
       - name: Login to GitHub Container Registry
         uses: docker/login-action@v3.4.0
         with:
@@ -148,7 +150,17 @@ jobs:
             done
           done
 
-      - uses: "marvinpinto/action-automatic-releases@latest"
-        with:
-          repo_token: "${{ secrets.GITHUB_TOKEN }}"
-          prerelease: false
+      # Use the preinstalled gh CLI instead of a third-party release action.
+      # Only runs on tag pushes; skipped on workflow_dispatch against main.
+      - name: Create GitHub Release
+        if: github.ref_type == 'tag'
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          if ! gh release view "${GITHUB_REF_NAME}" >/dev/null 2>&1; then
+            gh release create "${GITHUB_REF_NAME}" \
+              --title "${GITHUB_REF_NAME}" \
+              --generate-notes
+          else
+            echo "Release ${GITHUB_REF_NAME} already exists; skipping."
+          fi

Dockerfile

@@ -19,7 +19,10 @@ RUN CGO_ENABLED=0 GOOS=linux go build \
       -X 'main.ProjectURL=${PROJECT_URL}'" \
     -o mcp-auth-proxy ./main.go
 
-FROM debian:bookworm-slim
+# distroless/static-debian13:nonroot ships ca-certificates and runs as UID
+# 65532 by default — no shell, no apt, minimal attack surface. The static
+# Go binary (CGO_ENABLED=0) needs nothing else.
+FROM gcr.io/distroless/static-debian13:nonroot
 
 ARG BUILD_TIMESTAMP="1970-01-01T00:00:00+00:00"
 ARG COMMIT_HASH="00000000-dirty"
@@ -31,13 +34,8 @@ LABEL org.opencontainers.image.created=${BUILD_TIMESTAMP}
 LABEL org.opencontainers.image.version=${VERSION}
 LABEL org.opencontainers.image.revision=${COMMIT_HASH}
 
-# Security: install CA certs for TLS, then run as non-root
-RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates \
-    && rm -rf /var/lib/apt/lists/* \
-    && groupadd -r app && useradd -r -g app -s /usr/sbin/nologin app
-
 COPY --from=builder /app/mcp-auth-proxy /usr/local/bin/mcp-auth-proxy
 
-USER app:app
-EXPOSE 8080
-ENTRYPOINT ["mcp-auth-proxy"]
+USER nonroot:nonroot
+EXPOSE 8080 9090
+ENTRYPOINT ["/usr/local/bin/mcp-auth-proxy"]

README.md

@@ -33,8 +33,13 @@
 - Federates authentication to **any OIDC-compliant IdP** via auto-discovery
   (no vendor lock-in, zero IdP-specific code)
 - Reverse-proxies to your **unmodified** upstream MCP server
-- **Stateless** — no database, no Redis, no sticky sessions; scale
+- **Stateless by default** — no database, no sticky sessions; scale
   horizontally by sharing one secret
+- **Optional Redis** unlocks strict single-use authorization codes and
+  refresh-rotation reuse detection (OAuth 2.1 §6.1) across replicas
+- Per-IP **rate limiting** on every pre-auth endpoint, `email_verified`
+  enforcement on the IdP id_token, and **Prometheus metrics** for every
+  security-relevant event
 
 ---
 
@@ -116,6 +121,9 @@ All configuration via **environment variables**. Bold = required.
 | `REVOKE_BEFORE` | (empty) | RFC3339 cutoff for bulk revocation (applies to access *and* refresh tokens) |
 | `PKCE_REQUIRED` | `true` | Set `false` for clients that omit PKCE (Cursor, MCP Inspector, ChatGPT) |
 | `SHUTDOWN_TIMEOUT` | `120s` | Graceful shutdown; must be ≥ longest expected SSE stream |
+| `REDIS_URL` | (empty) | Optional. Enables single-use authz codes + refresh-rotation reuse detection. `rediss://` for TLS |
+| `REDIS_KEY_PREFIX` | `mcp-auth-proxy:` | Key prefix for shared Redis; set to empty to opt out of namespacing |
+| `RATE_LIMIT_ENABLED` | `true` | Per-IP rate limiting on pre-auth endpoints. Disable only behind a WAF that already enforces it |
 
 ---
 
@@ -132,7 +140,7 @@ required to operate this service.
 |---|---|---|
 | Client registration | `client_id` | 24h |
 | Authorize session | IdP `state` parameter | 10min |
-| Authorization code | `code` parameter | 5min |
+| Authorization code | `code` parameter | 60s |
 | Access token | Opaque bearer | 1h |
 | Refresh token | Opaque bearer | 7d |
 
@@ -156,7 +164,8 @@ rollout notes, and K8s deployment shape.
 | `GET  /authorize` | PKCE authorization endpoint |
 | `GET  /callback` | OIDC callback from the IdP |
 | `POST /token` | `authorization_code` + `refresh_token` grants |
-| `GET  /healthz` | Liveness / readiness probe |
+| `GET  /healthz` | Liveness probe (always 200 while the process is up) |
+| `GET  /readyz` | Readiness probe (503 when Redis is configured but unreachable) |
 | `*` (any other path) | Reverse-proxied to `UPSTREAM_MCP_URL` after Bearer check |
 | `GET /metrics` (port 9090) | Prometheus metrics |
 
@@ -166,10 +175,20 @@ rollout notes, and K8s deployment shape.
 
 - **Structured logs** — zap, JSON in production, console when run on a TTY.
   Every request carries a `request_id` in the log and in the
-  `X-Request-Id` response header.
-- **Metrics** — Prometheus on a dedicated port so it's never exposed
-  through the public listener.
-- **Health** — `GET /healthz` returns `200 OK`.
+  `X-Request-Id` response header. Inbound `X-Request-Id` is stripped to
+  prevent log-forgery.
+- **Metrics** — Prometheus on a dedicated port (`:9090`, separate listener,
+  not exposed through the public router). Alongside the default Go
+  runtime counters, the proxy emits:
+  - `mcp_auth_tokens_issued_total{grant_type}` — access tokens minted
+  - `mcp_auth_access_denied_total{reason}` — group / `email_unverified`
+    / `refresh_family_revoked` rejections
+  - `mcp_auth_replay_detected_total{kind}` — `code` or `refresh` replays
+    caught by the Redis-backed store
+  - `mcp_auth_rate_limited_total{endpoint}` — httprate 429s by endpoint
+  - `mcp_auth_clients_registered_total` — RFC 7591 registrations
+- **Health** — `GET /healthz` (liveness) and `GET /readyz` (reflects
+  Redis reachability when `REDIS_URL` is set).
 
 ---
 

build.sh

@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash
 set -euo pipefail
 
 MODULE=$(grep module go.mod | cut -d\  -f2)

config/config.go

@@ -22,6 +22,9 @@ type Config struct {
 	RevokeBefore       time.Time     // tokens issued before this time are rejected (zero = disabled)
 	PKCERequired       bool          // require PKCE on /authorize (default true, set false for Cursor/MCP Inspector)
 	ShutdownTimeout    time.Duration // graceful shutdown deadline; raise to drain long-lived SSE streams
+	RedisURL           string        // optional; when set, enables single-use authorization codes (replay protection)
+	RedisKeyPrefix     string        // prefix applied to every Redis key (for shared-Redis deployments); default "mcp-auth-proxy:"
+	RateLimitEnabled   bool          // enable per-IP rate limiting on pre-auth endpoints (default true)
 }
 
 func Load() (*Config, error) {
@@ -102,6 +105,16 @@ func Load() (*Config, error) {
 		}
 	}
 
+	c.RedisURL = os.Getenv("REDIS_URL")
+	// LookupEnv so operators can opt into an empty prefix explicitly
+	// (REDIS_KEY_PREFIX="") without tripping the default.
+	if v, ok := os.LookupEnv("REDIS_KEY_PREFIX"); ok {
+		c.RedisKeyPrefix = v
+	} else {
+		c.RedisKeyPrefix = "mcp-auth-proxy:"
+	}
+	c.RateLimitEnabled = strings.ToLower(os.Getenv("RATE_LIMIT_ENABLED")) != "false"
+
 	return c, nil
 }
 

config/config_test.go

@@ -303,3 +303,41 @@ func TestLoad_ShutdownTimeout_Negative(t *testing.T) {
 		t.Errorf("error %q should mention positive requirement", err)
 	}
 }
+
+func TestLoad_RedisKeyPrefix_Default(t *testing.T) {
+	setAllRequired(t)
+	cfg, err := Load()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if cfg.RedisKeyPrefix != "mcp-auth-proxy:" {
+		t.Errorf("RedisKeyPrefix default = %q, want %q", cfg.RedisKeyPrefix, "mcp-auth-proxy:")
+	}
+}
+
+func TestLoad_RedisKeyPrefix_Custom(t *testing.T) {
+	setAllRequired(t)
+	t.Setenv("REDIS_KEY_PREFIX", "tenant-42:")
+	cfg, err := Load()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if cfg.RedisKeyPrefix != "tenant-42:" {
+		t.Errorf("RedisKeyPrefix = %q, want %q", cfg.RedisKeyPrefix, "tenant-42:")
+	}
+}
+
+// TestLoad_RedisKeyPrefix_ExplicitEmpty verifies that setting REDIS_KEY_PREFIX
+// to the empty string is respected — operators can opt out of namespacing
+// without being overridden back to the default.
+func TestLoad_RedisKeyPrefix_ExplicitEmpty(t *testing.T) {
+	setAllRequired(t)
+	t.Setenv("REDIS_KEY_PREFIX", "")
+	cfg, err := Load()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if cfg.RedisKeyPrefix != "" {
+		t.Errorf("RedisKeyPrefix = %q, want empty (explicit opt-out)", cfg.RedisKeyPrefix)
+	}
+}