Security: Fix 24 vulnerabilities across auth, rate limiting, and uploads

apps/api/.env.example

@@ -9,6 +9,14 @@ NEXT_PUBLIC_WEB_URL=http://localhost:3000
 # storage adapter, so it must match where the server is reachable.
 NEXT_PUBLIC_API_URL=http://localhost:8000
 
+# === Reverse proxies ===
+# Number of proxies in front of this API. Unset = none, and the client address is
+# the socket's - correct when the API is reached directly. Behind
+# nginx/Traefik/Cloudflare, set the real hop count (usually 1) so
+# `X-Forwarded-For` is read; otherwise everyone behind the proxy shares one
+# rate-limit bucket. Too high a number reaches back into client-supplied text.
+# TRUST_PROXY=1
+
 # === CRON Secret for Internal API Calls ===
 CRON_SECRET=your-secure-cron-secret-key
 

apps/api/docker-compose.yml

@@ -13,7 +13,8 @@ services:
     volumes:
       - ../../docker/dev:/var/lib/postgresql/data
     ports:
-      - "5432:5432"
+      # Loopback only - these are development containers with a default password.
+      - "127.0.0.1:5432:5432"
     networks:
       - vitnode_dev
 
@@ -23,7 +24,7 @@ services:
     restart: unless-stopped
     command: redis-server --requirepass ${REDIS_PASSWORD-root}
     ports:
-      - "6379:6379"
+      - "127.0.0.1:6379:6379"
     networks:
       - vitnode_dev
 

apps/api/src/index.ts

@@ -4,6 +4,8 @@ import { serve, upgradeWebSocket } from "@hono/node-server";
 import { serveStatic } from "@hono/node-server/serve-static";
 import { OpenAPIHono } from "@hono/zod-openapi";
 import { VitNodeAPI } from "@vitnode/core/api/config";
+import { storageStaticHeadersMiddleware } from "@vitnode/core/api/middlewares/storage-static.middleware";
+import { websocketOriginMiddleware } from "@vitnode/core/api/middlewares/websocket-origin.middleware";
 import { handleVitNodeWebSocket } from "@vitnode/core/ws/handle";
 import { mkdirSync } from "node:fs";
 import { WebSocketServer } from "ws";
@@ -24,6 +26,10 @@ if (staticStorage) {
   mkdirSync(staticStorage.root, { recursive: true });
   app.get(
     staticStorage.mountPath,
+    // Stored files are served from this origin - the one the session cookie
+    // belongs to - so anything the browser would treat as a document has to be
+    // stopped from executing in it. See the middleware.
+    storageStaticHeadersMiddleware(),
     serveStatic({
       root: staticStorage.root,
       rewriteRequestPath: path =>
@@ -48,7 +54,14 @@ VitNodeAPI({
 
 const wss = new WebSocketServer({ noServer: true });
 
-app.get("/ws", upgradeWebSocket(handleVitNodeWebSocket()));
+// The handshake is a cookie-authenticated GET, which Hono's `csrf()` does not
+// cover and the same-origin policy does not apply to. Without this any site
+// could open a socket as a visiting user - see the middleware.
+app.get(
+  "/ws",
+  websocketOriginMiddleware({ origin: [webOrigin] }),
+  upgradeWebSocket(handleVitNodeWebSocket()),
+);
 
 serve(
   {

apps/api/src/vitnode.api.config.ts

@@ -23,6 +23,19 @@ config({
 export const POSTGRES_URL =
   process.env.POSTGRES_URL ?? "postgresql://root:root@localhost:5432/vitnode";
 
+/**
+ * How many reverse proxies stand in front of this API, from `TRUST_PROXY`.
+ *
+ * Unset means none, and the client address is then the socket's - the only one a
+ * caller cannot choose. Behind nginx, Traefik, Cloudflare or a platform edge,
+ * set `TRUST_PROXY=1` (or the real hop count) so `X-Forwarded-For` is read
+ * instead; otherwise every visitor behind that proxy shares one rate-limit
+ * bucket and the audit trail records the proxy.
+ */
+const trustProxy = process.env.TRUST_PROXY
+  ? Number(process.env.TRUST_PROXY)
+  : undefined;
+
 export const vitNodeApiConfig = buildApiConfig({
   plugins: [blogApiPlugin(), exampleApiPlugin()],
   ai: {
@@ -157,4 +170,5 @@ export const vitNodeApiConfig = buildApiConfig({
     title: "VitNode API",
     shortTitle: "VitNode",
   },
+  trustProxy,
 });

apps/web/.env.example

@@ -15,6 +15,13 @@ REDIS_URL=redis://localhost:6379
 NEXT_PUBLIC_WEB_URL=http://localhost:3000
 NEXT_PUBLIC_API_URL=http://localhost:3000
 
+# === Reverse proxies ===
+# Number of proxies in front of this app. Unset = none, and the client address is
+# read from the socket. This app serves its API through the Start runtime, which
+# exposes no socket - so behind nginx/Traefik/a platform edge, SET THIS (usually
+# 1), or every visitor shares one rate-limit bucket. The API warns at boot.
+# TRUST_PROXY=1
+
 # === CRON Secret for Internal API Calls ===
 CRON_SECRET=your-secure-cron-secret-key
 

apps/web/src/vitnode.api.config.ts

@@ -26,6 +26,19 @@ export const POSTGRES_URL =
  * them exactly as `apps/api/src/vitnode.api.config.ts` does when this app needs
  * them - `buildApiConfig` treats all of them as optional.
  */
+/**
+ * How many reverse proxies stand in front of this app, from `TRUST_PROXY`.
+ *
+ * Unset means none, and the client address is then the socket's - the only one a
+ * caller cannot choose. Behind nginx, Traefik, Cloudflare or a platform edge,
+ * set `TRUST_PROXY=1` (or the real hop count) so `X-Forwarded-For` is read
+ * instead; otherwise every visitor shares one rate-limit bucket and the audit
+ * trail records the proxy.
+ */
+const trustProxy = process.env.TRUST_PROXY
+  ? Number(process.env.TRUST_PROXY)
+  : undefined
+
 export const vitNodeApiConfig = buildApiConfig({
   plugins: [blogApiPlugin(), exampleApiPlugin()],
   storage: {
@@ -74,4 +87,9 @@ export const vitNodeApiConfig = buildApiConfig({
     title: 'VitNode API',
     shortTitle: 'VitNode',
   },
+  // This mount has no socket to read: the bridge hands Hono a bare `Request`,
+  // so without this every caller resolves to the same fallback address and the
+  // rate limiter degrades to one bucket for the whole site. The API warns at
+  // boot when that is happening.
+  trustProxy,
 })

packages/create-vitnode-app/copy-of-vitnode-app/docker/docker-compose.yml

@@ -12,7 +12,11 @@ services:
     volumes:
       - ./docker/dev:/var/lib/postgresql/data
     ports:
-      - '5432:5432'
+      # Loopback only. These are development containers with a default password
+      # of `root`, and `'5432:5432'` publishes them on every interface the host
+      # has - which on a laptop is whatever café or office network it is joined
+      # to, and on a VPS is the internet.
+      - '127.0.0.1:5432:5432'
     networks:
       - vitnode_dev
 
@@ -22,7 +26,7 @@ services:
     restart: unless-stopped
     command: redis-server --requirepass ${REDIS_PASSWORD-root}
     ports:
-      - '6379:6379'
+      - '127.0.0.1:6379:6379'
     networks:
       - vitnode_dev
 

packages/vitnode/src/api/adapters/search/postgres.ts

@@ -73,6 +73,16 @@ const buildFilters = (params: SearchQueryParams): SQL | undefined => {
   return conditions.length ? and(...conditions) : undefined;
 };
 
+/**
+ * The furthest into a relevance-ranked result set a caller may page.
+ *
+ * Relevance ordering has no stable key to seek on, so its cursor is an offset -
+ * and an offset is the one pagination shape whose cost grows with the page
+ * number. Nobody reaches page ten thousand of a search by reading; they reach it
+ * by editing the URL.
+ */
+const MAX_SEARCH_OFFSET = 10_000;
+
 export const PostgresSearchAdapter = (): SearchProviderApiPlugin => ({
   name: "postgres",
   // Two capabilities that are both true for the same reason: this provider's
@@ -126,7 +136,17 @@ export const PostgresSearchAdapter = (): SearchProviderApiPlugin => ({
       ? sql<number>`ts_rank("core_search_index"."search_vector", websearch_to_tsquery(${config}::regconfig, ${term}))`
       : sql<null | number>`NULL`;
 
-    const cursorValue = params.cursor ? Number(params.cursor) : undefined;
+    // Bounded and checked, because on the relevance path this becomes the
+    // query's `OFFSET`. `Number("abc")` is `NaN`, which Postgres rejects as a
+    // 500 rather than a bad request, and an unbounded one is a full scan a
+    // client can ask for by typing a big number into a URL.
+    const parsedCursor = params.cursor ? Number(params.cursor) : undefined;
+    const cursorValue =
+      parsedCursor !== undefined &&
+      Number.isSafeInteger(parsedCursor) &&
+      parsedCursor >= 0
+        ? Math.min(parsedCursor, MAX_SEARCH_OFFSET)
+        : undefined;
 
     const orderBy: SQL[] = [];
     let where = filters;

packages/vitnode/src/api/adapters/sso/discord.ts

@@ -20,6 +20,7 @@ export const DiscordSSOApiPlugin = ({
     id: z.string(),
     email: z.string(),
     username: z.string(),
+    verified: z.boolean(),
   });
   const tokenSchema = z.object({
     access_token: z.string(),
@@ -78,6 +79,16 @@ export const DiscordSSOApiPlugin = ({
         });
       }
 
+      // As the Google adapter does. An address Discord has not confirmed is an
+      // address the person signing in may not own, and VitNode keys an account
+      // on it - so accepting one lets somebody register under an address they
+      // cannot read, and hold the account the real owner would have had.
+      if (!data.verified) {
+        throw new HTTPException(400, {
+          message: "Email not verified",
+        });
+      }
+
       return data;
     },
     getUrl: ({ state }) => {

packages/vitnode/src/api/config.ts

@@ -2,17 +2,20 @@ import type { OpenAPIHono } from "@hono/zod-openapi";
 import type { Context, Env, Schema } from "hono";
 
 import { swaggerUI } from "@hono/swagger-ui";
+import { bodyLimit } from "hono/body-limit";
 import { cors } from "hono/cors";
 import { csrf } from "hono/csrf";
 import { HTTPException } from "hono/http-exception";
 
 import type { VitNodeApiConfig } from "@/vitnode.config";
 
 import { createCacheClient } from "@/api/lib/cache-client";
+import { clientIpMiddleware } from "@/api/lib/client-ip";
 import { collectCronJobs } from "@/api/lib/cron";
 import { describeError } from "@/api/lib/error-details";
 import { newBuildPluginApiCore } from "@/api/plugin";
 import { CONFIG_PLUGIN } from "@/config";
+import { CONFIG } from "@/lib/config";
 import { initRealtimePubSub } from "@/ws/registry";
 
 import {
@@ -22,6 +25,9 @@ import {
 import { rateLimiterMiddleware } from "./middlewares/rate-limiter.middleware";
 import { registerCronJobs } from "./modules/cron/helpers/register-cron-jobs";
 
+/** 25 MB: room for an image upload, and nothing like enough to be a weapon. */
+const DEFAULT_MAX_BODY_SIZE = 25 * 1024 * 1024;
+
 interface CORSOptions {
   allowHeaders?: string[];
   allowMethods?: string[];
@@ -62,23 +68,51 @@ export function VitNodeAPI({
 
   const plugins = [newBuildPluginApiCore, ...vitNodeApiConfig.plugins];
 
-  app.doc("/swagger/doc", {
-    openapi: "3.0.0",
-    info: {
-      version: CONFIG_PLUGIN.version,
-      title: "VitNode API",
-    },
-    tags: plugins.flatMap(
-      plugin => plugin.openApiTags?.map(name => ({ name })) ?? [],
-    ),
-  });
+  // The generated document names every route, parameter and response shape in
+  // the install, including the admin tree - a map of the attack surface, handed
+  // out unauthenticated. Published in development, where it is the point, and
+  // in production only when an install asks for it via `docs: { enabled: true }`.
+  const docsEnabled = vitNodeApiConfig.docs?.enabled ?? CONFIG.node_development;
+
+  if (docsEnabled) {
+    app.doc("/swagger/doc", {
+      openapi: "3.0.0",
+      info: {
+        version: CONFIG_PLUGIN.version,
+        title: "VitNode API",
+      },
+      tags: plugins.flatMap(
+        plugin => plugin.openApiTags?.map(name => ({ name })) ?? [],
+      ),
+    });
+  }
+
   app.use(cors(corsOptions));
   app.use(csrf(csrfOptions));
+  // Before the rate limiter, which keys its buckets on `ipAddress`. Resolving it
+  // later - as `globalMiddleware` used to - left every request in the
+  // deployment sharing one bucket named after `undefined`.
+  app.use("*", clientIpMiddleware(vitNodeApiConfig.trustProxy));
+  // Nothing bounded a request body before this. `POST /sign_in` reads its JSON
+  // and then runs scrypt unconditionally, so a body the server is willing to
+  // buffer is memory *and* CPU an unauthenticated caller gets to choose the size
+  // of. Uploads are the one thing that legitimately needs room, and they are
+  // bounded per field by the Content Engine's own `maxBytes`; this is the outer
+  // wall, and `maxBodySize` moves it for an install that stores large media.
+  app.use(
+    "*",
+    bodyLimit({
+      maxSize: vitNodeApiConfig.maxBodySize ?? DEFAULT_MAX_BODY_SIZE,
+      onError: c => c.json({ error: "Payload Too Large" }, 413),
+    }),
+  );
   app.use(
     "*",
     rateLimiterMiddleware(vitNodeApiConfig.rateLimiter, redisClient),
   );
-  app.get("/swagger", swaggerUI({ url: "/api/swagger/doc" }));
+  if (docsEnabled) {
+    app.get("/swagger", swaggerUI({ url: "/api/swagger/doc" }));
+  }
   app.use(
     "*",
     globalMiddleware({
@@ -96,6 +130,7 @@ export function VitNodeAPI({
       storage: vitNodeApiConfig.storage,
       plugins,
       cacheClient: redisClient,
+      trustProxy: vitNodeApiConfig.trustProxy,
     }),
   );
   app.use(async (c, next) => {

packages/vitnode/src/api/lib/auth-cookie.ts

@@ -33,6 +33,14 @@ const authCookieOptions = (c: Context): CookieOptions => {
     domain: cookieDomain,
     httpOnly: true,
     path: "/",
+    // Stated rather than left to the browser. Chrome and Firefox default an
+    // omitted `SameSite` to `Lax`, but that is a default and not a rule: Safari
+    // and older engines have their own, and a cookie whose cross-site behaviour
+    // depends on which browser is reading it is one nobody can reason about.
+    // `Lax` and not `Strict` because the SSO round trip lands here as a
+    // top-level cross-site GET - `Strict` would drop the state cookie on the way
+    // back from the provider and break every social sign-in.
+    sameSite: "Lax",
     secure: cookieSecure,
   };
 };