feat: endpoint security hardening — auth, rate limiting, info leak fixes (#118)

Author: TerrifiedBug

docs/public/operations/configuration.md

@@ -145,6 +145,22 @@ For more details, see [Backup & Restore](backup-restore.md).
 
 OIDC is configured in the Settings page under the **Authentication** tab. See [Authentication](authentication.md) for full setup instructions.
 
+## Prometheus metrics
+
+The `/api/metrics` endpoint exposes metrics in Prometheus exposition format. It requires a service account API token with the `metrics.read` permission.
+
+Create a service account in **Settings > Service Accounts** with `metrics.read` permission, then configure your Prometheus scrape config:
+
+```yaml
+scrape_configs:
+  - job_name: vectorflow
+    scheme: https
+    metrics_path: /api/metrics
+    bearer_token: "vf_your_service_account_key"
+    static_configs:
+      - targets: ["vectorflow.example.com"]
+```
+
 ## Ports reference
 
 | Service | Default Port | Description |

src/app/api/_lib/__tests__/ip-rate-limit.test.ts

@@ -0,0 +1,95 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
+import { checkIpRateLimit } from "../ip-rate-limit";
+
+function makeRequest(
+  ip?: string,
+  headers?: Record<string, string>,
+): Request {
+  const h: Record<string, string> = { ...headers };
+  if (ip) h["x-forwarded-for"] = ip;
+  return new Request("http://localhost/api/test", { headers: h });
+}
+
+describe("checkIpRateLimit", () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  it("returns null when under the limit", () => {
+    const result = checkIpRateLimit(makeRequest("1.2.3.4"), "enroll", 10);
+    expect(result).toBeNull();
+  });
+
+  it("returns 429 response when limit exceeded", () => {
+    for (let i = 0; i < 10; i++) {
+      checkIpRateLimit(makeRequest("5.6.7.8"), "enroll", 10);
+    }
+    const result = checkIpRateLimit(makeRequest("5.6.7.8"), "enroll", 10);
+    expect(result).not.toBeNull();
+    expect(result!.status).toBe(429);
+  });
+
+  it("includes Retry-After header on 429", () => {
+    for (let i = 0; i < 5; i++) {
+      checkIpRateLimit(makeRequest("9.0.1.2"), "setup", 5);
+    }
+    const result = checkIpRateLimit(makeRequest("9.0.1.2"), "setup", 5);
+    expect(result!.headers.get("Retry-After")).toBeTruthy();
+  });
+
+  it("isolates limits between different IPs", () => {
+    for (let i = 0; i < 10; i++) {
+      checkIpRateLimit(makeRequest("10.0.0.1"), "enroll", 10);
+    }
+    expect(checkIpRateLimit(makeRequest("10.0.0.1"), "enroll", 10)).not.toBeNull();
+    expect(checkIpRateLimit(makeRequest("10.0.0.2"), "enroll", 10)).toBeNull();
+  });
+
+  it("extracts IP from x-forwarded-for (rightmost entry)", () => {
+    const req = makeRequest(undefined, {
+      "x-forwarded-for": "203.0.113.50, 70.41.3.18, 150.172.238.178",
+    });
+    for (let i = 0; i < 10; i++) {
+      checkIpRateLimit(req, "enroll", 10);
+    }
+    // Rightmost entry is the proxy-appended IP
+    const blocked = checkIpRateLimit(
+      makeRequest("150.172.238.178"),
+      "enroll",
+      10,
+    );
+    expect(blocked).not.toBeNull();
+  });
+
+  it("falls back to x-real-ip when x-forwarded-for missing", () => {
+    const req = new Request("http://localhost/api/test", {
+      headers: { "x-real-ip": "192.168.1.1" },
+    });
+    for (let i = 0; i < 5; i++) {
+      checkIpRateLimit(req, "setup", 5);
+    }
+    const result = checkIpRateLimit(req, "setup", 5);
+    expect(result).not.toBeNull();
+  });
+
+  it("uses 'unknown' key when no IP headers present", () => {
+    const req = new Request("http://localhost/api/test");
+    const result = checkIpRateLimit(req, "enroll", 10);
+    expect(result).toBeNull();
+  });
+
+  it("resets after window expires", () => {
+    for (let i = 0; i < 10; i++) {
+      checkIpRateLimit(makeRequest("1.1.1.1"), "enroll", 10);
+    }
+    expect(checkIpRateLimit(makeRequest("1.1.1.1"), "enroll", 10)).not.toBeNull();
+
+    vi.advanceTimersByTime(61_000);
+
+    expect(checkIpRateLimit(makeRequest("1.1.1.1"), "enroll", 10)).toBeNull();
+  });
+});

src/app/api/_lib/ip-rate-limit.ts

@@ -0,0 +1,41 @@
+import { rateLimiter } from "@/app/api/v1/_lib/rate-limiter";
+
+function getClientIp(request: Request): string {
+  const forwarded = request.headers.get("x-forwarded-for");
+  if (forwarded) {
+    const parts = forwarded.split(",");
+    return parts[parts.length - 1].trim();
+  }
+
+  const realIp = request.headers.get("x-real-ip");
+  if (realIp) return realIp.trim();
+
+  return "unknown";
+}
+
+/**
+ * Check an IP-keyed rate limit for unauthenticated endpoints.
+ * Returns a 429 Response if the limit is exceeded, or null if allowed.
+ */
+export function checkIpRateLimit(
+  request: Request,
+  endpoint: string,
+  limit: number,
+): Response | null {
+  const ip = getClientIp(request);
+  const key = `ip:${endpoint}:${ip}`;
+
+  const result = rateLimiter.checkKey(key, limit);
+
+  if (!result.allowed) {
+    return new Response(JSON.stringify({ error: "Too many requests" }), {
+      status: 429,
+      headers: {
+        "Content-Type": "application/json",
+        "Retry-After": String(result.retryAfter),
+      },
+    });
+  }
+
+  return null;
+}

src/app/api/agent/enroll/route.ts

@@ -5,6 +5,7 @@ import { verifyEnrollmentToken, generateNodeToken } from "@/server/services/agen
 import { fireEventAlert } from "@/server/services/event-alerts";
 import { debugLog } from "@/lib/logger";
 import { nodeMatchesGroup } from "@/lib/node-group-utils";
+import { checkIpRateLimit } from "@/app/api/_lib/ip-rate-limit";
 
 const enrollSchema = z.object({
   token: z.string().min(1),
@@ -15,6 +16,9 @@ const enrollSchema = z.object({
 });
 
 export async function POST(request: Request) {
+  const rateLimited = checkIpRateLimit(request, "enroll", 10);
+  if (rateLimited) return rateLimited;
+
   try {
     const body = await request.json();
     const parsed = enrollSchema.safeParse(body);

src/app/api/health/__tests__/route.test.ts

@@ -0,0 +1,34 @@
+import { vi, describe, it, expect } from "vitest";
+
+vi.mock("@/lib/prisma", () => ({
+  prisma: {
+    $queryRaw: vi.fn(),
+  },
+}));
+
+import { GET } from "../route";
+import { prisma } from "@/lib/prisma";
+
+describe("GET /api/health", () => {
+  it("returns { status: 'ok' } with 200 when DB is reachable", async () => {
+    vi.mocked(prisma.$queryRaw).mockResolvedValue([{ "?column?": 1 }]);
+
+    const response = await GET();
+    const body = await response.json();
+
+    expect(response.status).toBe(200);
+    expect(body).toEqual({ status: "ok" });
+    expect(body).not.toHaveProperty("db");
+  });
+
+  it("returns { status: 'error' } with 503 when DB is unreachable", async () => {
+    vi.mocked(prisma.$queryRaw).mockRejectedValue(new Error("ECONNREFUSED"));
+
+    const response = await GET();
+    const body = await response.json();
+
+    expect(response.status).toBe(503);
+    expect(body).toEqual({ status: "error" });
+    expect(body).not.toHaveProperty("db");
+  });
+});

src/app/api/health/route.ts

@@ -3,11 +3,8 @@ import { prisma } from "@/lib/prisma";
 export async function GET() {
   try {
     await prisma.$queryRaw`SELECT 1`;
-    return Response.json({ status: "ok", db: "connected" });
+    return Response.json({ status: "ok" });
   } catch {
-    return Response.json(
-      { status: "error", db: "disconnected" },
-      { status: 503 },
-    );
+    return Response.json({ status: "error" }, { status: 503 });
   }
 }

src/app/api/metrics/__tests__/route.test.ts

@@ -1,80 +1,46 @@
 import { vi, describe, it, expect, beforeEach } from "vitest";
 
-// ── Hoisted mocks (available inside vi.mock factories) ──────────
 const { mockCollectMetrics, mockAuthenticateApiKey } = vi.hoisted(() => ({
   mockCollectMetrics: vi.fn(),
   mockAuthenticateApiKey: vi.fn(),
 }));
 
-// ── Mock PrometheusMetricsService ───────────────────────────────
 vi.mock("@/server/services/prometheus-metrics", () => ({
   PrometheusMetricsService: class {
     collectMetrics = mockCollectMetrics;
   },
 }));
 
-// ── Mock authenticateApiKey ─────────────────────────────────────
 vi.mock("@/server/middleware/api-auth", () => ({
   authenticateApiKey: (...args: unknown[]) => mockAuthenticateApiKey(...args),
+  hasPermission: (ctx: { permissions: string[] }, perm: string) =>
+    ctx.permissions.includes(perm),
 }));
 
 import { GET } from "@/app/api/metrics/route";
 
-// ─── Helpers ────────────────────────────────────────────────────
-
 function makeRequest(headers?: Record<string, string>): Request {
   return new Request("http://localhost:3000/api/metrics", {
     method: "GET",
     headers: headers ?? {},
   });
 }
 
-// ─── Tests ──────────────────────────────────────────────────────
-
 describe("GET /api/metrics", () => {
   beforeEach(() => {
     vi.clearAllMocks();
-    // Default: auth not required
-    delete process.env.METRICS_AUTH_REQUIRED;
-  });
-
-  it("returns metrics with correct content type when auth disabled (default)", async () => {
-    const metricsOutput = '# HELP vectorflow_node_status Node status\nvectorflow_node_status{node_id="n1"} 1\n';
-    mockCollectMetrics.mockResolvedValue(metricsOutput);
-
-    const response = await GET(makeRequest());
-
-    expect(response.status).toBe(200);
-    expect(response.headers.get("Content-Type")).toBe(
-      "text/plain; version=0.0.4; charset=utf-8",
-    );
-    const body = await response.text();
-    expect(body).toBe(metricsOutput);
-    expect(mockAuthenticateApiKey).not.toHaveBeenCalled();
-  });
-
-  it("does not require auth header when METRICS_AUTH_REQUIRED is unset", async () => {
-    mockCollectMetrics.mockResolvedValue("");
-
-    const response = await GET(makeRequest());
-    expect(response.status).toBe(200);
-    expect(mockAuthenticateApiKey).not.toHaveBeenCalled();
   });
 
-  it("returns 401 when auth required and no token provided", async () => {
-    process.env.METRICS_AUTH_REQUIRED = "true";
+  it("returns 401 when no auth header provided", async () => {
     mockAuthenticateApiKey.mockResolvedValue(null);
 
     const response = await GET(makeRequest());
 
     expect(response.status).toBe(401);
-    const body = await response.text();
-    expect(body).toContain("Unauthorized");
     expect(mockAuthenticateApiKey).toHaveBeenCalledWith(null);
   });
 
-  it("returns 401 when auth required and invalid token provided", async () => {
-    process.env.METRICS_AUTH_REQUIRED = "true";
+  it("returns 401 when invalid token provided", async () => {
     mockAuthenticateApiKey.mockResolvedValue(null);
 
     const response = await GET(
@@ -85,13 +51,27 @@ describe("GET /api/metrics", () => {
     expect(mockAuthenticateApiKey).toHaveBeenCalledWith("Bearer invalid_token");
   });
 
-  it("returns metrics when auth required and valid token provided", async () => {
-    process.env.METRICS_AUTH_REQUIRED = "true";
+  it("returns 401 when token lacks metrics.read permission", async () => {
+    mockAuthenticateApiKey.mockResolvedValue({
+      serviceAccountId: "sa-1",
+      serviceAccountName: "deploy-bot",
+      environmentId: "env-1",
+      permissions: ["pipelines.deploy"],
+    });
+
+    const response = await GET(
+      makeRequest({ Authorization: "Bearer vf_deploy_token" }),
+    );
+
+    expect(response.status).toBe(401);
+  });
+
+  it("returns metrics when valid token provided", async () => {
     mockAuthenticateApiKey.mockResolvedValue({
       serviceAccountId: "sa-1",
       serviceAccountName: "prom-scraper",
       environmentId: "env-1",
-      permissions: ["read"],
+      permissions: ["metrics.read"],
     });
     mockCollectMetrics.mockResolvedValue("vectorflow_node_status 1\n");
 
@@ -100,20 +80,28 @@ describe("GET /api/metrics", () => {
     );
 
     expect(response.status).toBe(200);
+    expect(response.headers.get("Content-Type")).toBe(
+      "text/plain; version=0.0.4; charset=utf-8",
+    );
     const body = await response.text();
     expect(body).toBe("vectorflow_node_status 1\n");
   });
 
   it("returns 500 when collectMetrics throws", async () => {
+    mockAuthenticateApiKey.mockResolvedValue({
+      serviceAccountId: "sa-1",
+      serviceAccountName: "prom-scraper",
+      environmentId: "env-1",
+      permissions: ["metrics.read"],
+    });
     mockCollectMetrics.mockRejectedValue(new Error("Service crash"));
     const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
 
-    const response = await GET(makeRequest());
+    const response = await GET(
+      makeRequest({ Authorization: "Bearer vf_valid_token" }),
+    );
 
     expect(response.status).toBe(500);
-    const body = await response.text();
-    expect(body).toContain("Internal Server Error");
-
     consoleSpy.mockRestore();
   });
 });