Session 10: Internal auth refactor, prefetch cascade keys, Sentry, welcome email (1286 tests)

2026-06-10 20:45:05 -04:00
parent b55dcbd614
commit e5c45ecc8e
22 changed files with 3837 additions and 94 deletions
@@ -1,4 +1,10 @@
 require('dotenv').config();
+// Session 10 — Sentry must initialize BEFORE express is required so
+// the instrumentation hooks attach correctly. Graceful no-op when
+// SENTRY_DSN is unset.
+const { initSentry, Sentry } = require('./utils/sentry');
+initSentry();
+
 const express = require('express');
 const cors = require('cors');
 const oddsRoutes = require('./routes/odds');
@@ -132,4 +138,10 @@ app.use('/api/grading', express.json({ limit: '256kb' }), correctionRoutes);
 const widgetRoutes = require('./routes/widget');
 app.use('/api/widget', widgetRoutes);

+// Session 10 — Sentry's Express error handler catches uncaught
+// errors from every route mounted above. Must come AFTER routes but
+// BEFORE any final express error handler. The noop client makes this
+// a safe no-op when SENTRY_DSN is unset.
+Sentry.setupExpressErrorHandler(app);
+
 module.exports = app;
@@ -0,0 +1,97 @@
+/**
+ * Internal authentication middleware (Session 10).
+ *
+ * Protects internal-only endpoints — the grading pipeline, the
+ * resolution poll-back, the corrections sweep — that are called by
+ * pollers, n8n workflows, and cron jobs but NEVER by browser users.
+ * Uses a shared secret in `VYNDR_INTERNAL_KEY` checked against the
+ * request header.
+ *
+ * Deliberately separate from `requireAuth` (Supabase JWT). Internal
+ * callers don't have user sessions.
+ *
+ * Header compatibility:
+ *   `x-internal-key`            — Session 10 short form (n8n + new callers)
+ *   `X-VYNDR-Internal-Key`      — legacy form, kept for backwards
+ *                                  compatibility with the poller and
+ *                                  the existing test suite. The
+ *                                  middleware accepts either; callers
+ *                                  should prefer the short form.
+ *
+ * Options:
+ *   loopbackOnly (default false) — additionally enforce that the
+ *     request originated from 127.0.0.1 / ::1. Use for endpoints that
+ *     should ONLY be reachable from co-located processes (the poller
+ *     pulling box scores). Endpoints called from n8n or other
+ *     containers MUST omit this option.
+ *
+ * Responses:
+ *   503 — VYNDR_INTERNAL_KEY env var not set (misconfigured)
+ *   401 — header missing OR key mismatch
+ *   403 — loopbackOnly=true and the request came from off-host
+ */
+
+const crypto = require('crypto');
+
+const LOOPBACK_IPS = new Set(['127.0.0.1', '::1', '::ffff:127.0.0.1']);
+
+// Timing-safe string compare. crypto.timingSafeEqual throws on
+// length mismatch, so we pad to a fixed length first to keep the
+// comparison constant-time regardless of input length.
+function timingSafeStringEqual(a, b) {
+  if (typeof a !== 'string' || typeof b !== 'string') return false;
+  const len = Math.max(a.length, b.length);
+  // Pad with a NUL byte (will never appear in a real key) so the
+  // shorter side still has the same length.
+  const ba = Buffer.alloc(len, 0);
+  const bb = Buffer.alloc(len, 0);
+  ba.write(a, 'utf8');
+  bb.write(b, 'utf8');
+  // Even on length mismatch the compare runs to completion; we just
+  // also require lengths match to count as equal.
+  const equal = crypto.timingSafeEqual(ba, bb);
+  return equal && a.length === b.length;
+}
+
+function readHeader(req) {
+  // Express normalizes header names to lowercase. Try the short form
+  // first (the documented one), then the legacy long form.
+  return req.get('x-internal-key') || req.get('X-VYNDR-Internal-Key') || null;
+}
+
+function requireInternalAuth(options = {}) {
+  const loopbackOnly = !!options.loopbackOnly;
+  return function internalAuthMiddleware(req, res, next) {
+    const expected = process.env.VYNDR_INTERNAL_KEY;
+    if (!expected) {
+      // Refuse to serve when the secret is unset — better than
+      // accidentally exposing the endpoint with a default empty
+      // value. n8n uses this to distinguish "misconfigured" (503)
+      // from "wrong key" (401).
+      return res.status(503).json({ error: 'Internal auth not configured' });
+    }
+
+    const provided = readHeader(req);
+    if (!provided || !timingSafeStringEqual(provided, expected)) {
+      return res.status(401).json({ error: 'Invalid internal key' });
+    }
+
+    if (loopbackOnly) {
+      const remoteIp = req.ip || req.socket?.remoteAddress;
+      if (!LOOPBACK_IPS.has(remoteIp)) {
+        return res.status(403).json({ error: 'Origin not permitted' });
+      }
+    }
+
+    return next();
+  };
+}
+
+module.exports = {
+  requireInternalAuth,
+  __internals: {
+    LOOPBACK_IPS,
+    timingSafeStringEqual,
+    readHeader,
+  },
+};
@@ -25,19 +25,11 @@ const { __helpers: gradingHelpers } = require('./grading');
 const router = express.Router();
 const espnLimiter = createLimiter(API_BUDGETS.espn);

-const LOOPBACK_IPS = new Set(['127.0.0.1', '::1', '::ffff:127.0.0.1']);
-
-function requireInternal(req, res, next) {
-  const expected = process.env.VYNDR_INTERNAL_KEY;
-  if (!expected) return res.status(503).json({ error: 'Internal auth not configured' });
-  if (req.get('X-VYNDR-Internal-Key') !== expected) {
-    return res.status(401).json({ error: 'Invalid internal key' });
-  }
-  if (!LOOPBACK_IPS.has(req.ip || req.socket?.remoteAddress)) {
-    return res.status(403).json({ error: 'Origin not permitted' });
-  }
-  return next();
-}
+// Session 10 — uses src/middleware/internalAuth.js. /correct stays
+// loopback-restricted because the morning sweep runs co-located with
+// the API; n8n doesn't call this one.
+const { requireInternalAuth } = require('../middleware/internalAuth');
+const requireInternal = requireInternalAuth({ loopbackOnly: true });

 async function fetchBoxScore(sportCfg, gameId) {
  await espnLimiter.waitForToken();
@@ -27,27 +27,22 @@ const clvTracker = require('../services/intelligence/clvTracker');
 const accuracyTracker = require('../services/intelligence/accuracyTracker');
 const weightAdjuster = require('../services/intelligence/weightAdjuster');

+const { requireInternalAuth } = require('../middleware/internalAuth');
+
 const router = express.Router();

-const LOOPBACK_IPS = new Set(['127.0.0.1', '::1', '::ffff:127.0.0.1']);
-
-function requireInternal(req, res, next) {
-  const expected = process.env.VYNDR_INTERNAL_KEY;
-  if (!expected) {
-    // Refuse to serve if the secret isn't configured — better than
-    // accidentally exposing the endpoint with a default value.
-    return res.status(503).json({ error: 'Internal auth not configured' });
-  }
-  const provided = req.get('X-VYNDR-Internal-Key');
-  if (!provided || provided !== expected) {
-    return res.status(401).json({ error: 'Invalid internal key' });
-  }
-  const remoteIp = req.ip || req.socket?.remoteAddress;
-  if (!LOOPBACK_IPS.has(remoteIp)) {
-    return res.status(403).json({ error: 'Origin not permitted' });
-  }
-  return next();
-}
+// Session 10 — extracted into src/middleware/internalAuth.js. The two
+// internal routes below keep slightly different policies:
+//   /resolve  — loopback-only (called by the on-host poller)
+//   /pipeline — accepts off-host (called by n8n from another container,
+//               which is why the legacy loopback-only check broke it)
+//
+// `requireInternal` stays exported as `__helpers.requireInternal` for
+// the existing test suite — it's a thin alias for the loopback-only
+// variant so the resolution test (which spins up its own server on
+// 127.0.0.1) behaves identically.
+const requireInternal = requireInternalAuth({ loopbackOnly: true });
+const requireInternalAnyOrigin = requireInternalAuth({ loopbackOnly: false });

 // ---------------------------------------------------------------
 // Box-score traversal — sport-specific shapes flattened into a uniform
@@ -412,22 +407,34 @@ router.post('/resolve', requireInternal, async (req, res) => {

 const VALID_SPORTS = new Set(['nba', 'wnba', 'mlb', 'nfl', 'nhl', 'ncaab', 'ncaafb']);

-router.post('/pipeline', requireInternal, async (req, res) => {
+// Session 10 — `/pipeline` accepts off-host callers (n8n runs in a
+// separate container). With a `sport` body field, runs that sport
+// only; with an empty body, iterates every active sport. n8n's
+// Morning Ops workflow sends an empty body; the per-sport workflows
+// pass a specific sport. The legacy header (X-VYNDR-Internal-Key) and
+// the new short form (x-internal-key) both authenticate.
+router.post('/pipeline', requireInternalAnyOrigin, async (req, res) => {
  const { sport, options } = req.body || {};
-  if (!sport || !VALID_SPORTS.has(sport)) {
+  if (sport && !VALID_SPORTS.has(sport)) {
    return res.status(400).json({ error: 'sport must be one of: nba, wnba, mlb, nfl, nhl, ncaab, ncaafb' });
  }
-  // Lazy-load the orchestrator so this route doesn't pay the require cost
-  // until it's actually invoked (and so unit tests of /resolve don't pull
-  // in the whole adapter graph).
  const { runPipeline } = require('../services/intelligence/gradingOrchestrator');
-  try {
-    const summary = await runPipeline(sport, options || {});
-    return res.json(summary);
-  } catch (err) {
-    console.error('[VYNDR] Pipeline error:', err.message);
-    return res.status(503).json({ error: 'Pipeline run failed' });
+  const sportsToRun = sport ? [sport] : ['nba', 'wnba', 'mlb'];
+  const results = [];
+  for (const s of sportsToRun) {
+    try {
+      const summary = await runPipeline(s, options || {});
+      results.push({ sport: s, ...summary });
+    } catch (err) {
+      console.error('[VYNDR] Pipeline error:', s, err.message);
+      results.push({ sport: s, error: err.message });
+    }
  }
+  // If a specific sport was requested, preserve the legacy single-object
+  // shape so existing callers (tests + the n8n per-sport workflows)
+  // don't break. Multi-sport runs return an array.
+  if (sport) return res.json(results[0]);
+  return res.json({ status: 'ok', timestamp: new Date().toISOString(), sports: results });
 });

 // Exported so server.js can wire it up with a larger body limit; also lets
@@ -0,0 +1,119 @@
+/**
+ * Sentry init for the Express backend (Session 10).
+ *
+ * Boot is GRACEFUL when SENTRY_DSN is unset: `initSentry()` no-ops and
+ * the module's exported `Sentry` is a stub whose every method is a
+ * noop. This lets callers wire `Sentry.captureException(...)` etc.
+ * unconditionally — when the DSN isn't configured (dev, CI, tests),
+ * nothing crashes and no events are sent.
+ *
+ * PII posture: `sendDefaultPii: false` plus an `events`-stage hook
+ * that strips `user.ip_address` and `request.cookies`. We log code
+ * paths and errors, not user identities.
+ *
+ * Sample rate: 10% traces (free tier friendly). 100% errors.
+ */
+
+const realSentry = require('@sentry/node');
+
+// Initialized lazily so this module is safe to require even when the
+// DSN is absent.
+let _initialized = false;
+
+function buildClient() {
+  return {
+    init: realSentry.init,
+    captureException: realSentry.captureException,
+    captureMessage: realSentry.captureMessage,
+    setupExpressErrorHandler: realSentry.setupExpressErrorHandler,
+    addBreadcrumb: realSentry.addBreadcrumb,
+    setUser: realSentry.setUser,
+    setTag: realSentry.setTag,
+    setContext: realSentry.setContext,
+  };
+}
+
+function buildNoop() {
+  // Every Sentry surface used in the codebase returns either undefined
+  // (most) or a no-op express middleware (`setupExpressErrorHandler`).
+  const noopMiddleware = (_req, _res, next) => next();
+  return {
+    init: () => {},
+    captureException: () => {},
+    captureMessage: () => {},
+    setupExpressErrorHandler: (_app) => {
+      // The real one mutates the app; the noop simply returns without
+      // attaching anything. The caller pattern is:
+      //   Sentry.setupExpressErrorHandler(app);
+      // which evaluates the call for its side effects.
+    },
+    addBreadcrumb: () => {},
+    setUser: () => {},
+    setTag: () => {},
+    setContext: () => {},
+    Handlers: { errorHandler: () => noopMiddleware },
+  };
+}
+
+let Sentry = buildNoop();
+
+function initSentry({ dsn = process.env.SENTRY_DSN, environment = process.env.NODE_ENV, release } = {}) {
+  if (_initialized) return Sentry;
+  if (!dsn) {
+    // Stay on the noop client.
+    return Sentry;
+  }
+  Sentry = buildClient();
+  Sentry.init({
+    dsn,
+    environment: environment || 'development',
+    release,
+    tracesSampleRate: 0.1,
+    sendDefaultPii: false,
+    beforeSend(event) {
+      // PII scrubbing. Sentry occasionally fills these via auto-context.
+      if (event.user) {
+        delete event.user.ip_address;
+        delete event.user.email;
+      }
+      if (event.request) {
+        delete event.request.cookies;
+        // Strip the Authorization header to avoid logging bearer tokens.
+        if (event.request.headers) {
+          delete event.request.headers.authorization;
+          delete event.request.headers.cookie;
+          delete event.request.headers['x-internal-key'];
+          delete event.request.headers['x-vyndr-internal-key'];
+        }
+      }
+      return event;
+    },
+  });
+  _initialized = true;
+  return Sentry;
+}
+
+function getSentry() {
+  return Sentry;
+}
+
+function isInitialized() {
+  return _initialized;
+}
+
+// Test helper — reset to the noop client so subsequent initSentry()
+// calls re-run. Not exported via the main surface; live behind __internals.
+function __resetForTests() {
+  _initialized = false;
+  Sentry = buildNoop();
+}
+
+module.exports = {
+  initSentry,
+  getSentry,
+  isInitialized,
+  // Convenience re-export so callers can do `const { Sentry } = require(...)`.
+  // Note: this is a live binding — mutated by initSentry() on first call.
+  get Sentry() { return Sentry; },
+  __internals: { __resetForTests, buildNoop, buildClient },
+};