Session 7d: Audit fixes - rate limiting, error leak, parallel parlays, analyze cache, bundle analyzer

docs/SYSTEM-MANIFEST.md

@@ -62,8 +62,8 @@ Mounted in `src/app.js`. Auth column meanings:
 | GET    | /api/health                           | public      | n/a        | `app.js` (inline)                    |
 | GET    | /api/odds/nba                         | public      | 10mb       | `routes/odds.js`                     |
 | GET    | /api/odds/ncaab                       | public      | 10mb       | `routes/odds.js`                     |
-| POST   | /api/analyze/prop                     | public      | 10mb       | `routes/analyze.js` (demo scan)      |
-| POST   | /api/analyze/batch                    | public      | 10mb       | `routes/analyze.js` ⚠ no rate limit  |
+| POST   | /api/analyze/prop                     | public + 10/min IP | 10mb | `routes/analyze.js` (cached 60s)     |
+| POST   | /api/analyze/batch                    | public + 10/min IP | 10mb | `routes/analyze.js` (cached 60s)     |
 | POST   | /api/scan/parlay                      | user        | 10mb       | `routes/scan.js`                     |
 | GET    | /api/movements                        | public      | 10mb       | `routes/movements.js`                |
 | GET    | /api/alerts                           | user        | 10mb       | `routes/alerts.js`                   |
@@ -410,32 +410,41 @@ No circular imports detected.
 
 ### ARCH — Architecture
 
-- **[ARCH-1] Dual grading paths.** Severity: Medium. The legacy
-  `parlayScanService → propAnalyzer → grader → UnifiedOddsProvider`
-  chain still serves `/api/scan/parlay`, `/api/analyze/*`, and
-  `/api/bets/*`. The new `gradingOrchestrator → engine1 → engine2`
-  chain serves `/api/grading/pipeline`. Both write to `grade_history`
-  with different column sets and factor models. **Scope to fix:**
-  ~1 session — choose one as canonical, migrate the routes, retire
-  the other adapter set.
+- **[ARCH-1] Dual grading paths.** Severity: Medium. Status:
+  **DEFERRED in Session 7d** — the legacy `propAnalyzer → grader →
+  UnifiedOddsProvider` chain returns a 4-letter grade + 0-100
+  confidence + kill-conditions + reasoning.steps shape; the new
+  `engine1` returns an 11-step grade + 0-1 confidence + factors. Both
+  shapes are consumed by different surfaces (GradeCard + DemoScan vs
+  the orchestrator's grade_history writer). Unification requires
+  frontend + backend coordination — out of scope for the audit-fix
+  session. Migration plan: (1) decide which shape the UI keeps; (2)
+  write an adapter from the other shape; (3) rewire one route at a
+  time, starting with `/api/analyze/prop`. Session 7d added a
+  DEPRECATED banner to `src/services/grader.js` to signal that new
+  features should land in engine1.js.
 
 - **[ARCH-2] Two circuit-breaker / rate-limiter modules.** Severity:
   Low (documented in 6a). `src/services/{circuitBreaker.js,
   rateLimiter.js}` (keyed registry, legacy) coexist with
   `src/utils/rateLimiter.js` (factory, new). Consolidation is purely
-  cosmetic — both work. Scope: half-session.
+  cosmetic — both work. Scope: half-session. Status: open.
 
 ### SEC — Security
 
 - **[SEC-1] `/api/analyze/batch` has no auth or rate limit.** Severity:
-  High. A malicious caller can blast through prop-analyzer credits.
-  **Recommended fix:** require auth OR add an IP-keyed rate limiter
-  (10 req/min). Scope: ~30 min.
+  High. Status: **FIXED in Session 7d.** Added IP-keyed rate limit
+  (`src/middleware/rateLimit.js`, 10 req/min) and mounted it on the
+  `/api/analyze` router via `router.use(analyzeLimit)` — covers both
+  `/prop` and `/batch`. 7 unit tests + 1 behavioral test verify the
+  429 path.
 
 - **[SEC-2] `routes/shareCard.js` leaks `err.message` via `detail:`.**
-  Severity: Medium. Lines 185 and 206 expose upstream error messages
-  to public callers. **Recommended fix:** drop `detail` from public
-  responses; log to stderr only. Scope: ~10 min.
+  Severity: Medium. Status: **FIXED in Session 7d.** Lines 185 and
+  206 now log to `console.error('[VYNDR] shareCard ... failed:',
+  err?.message)` (ops-only) and return generic error verbs to public
+  callers. The validation-errors `detail:` on line 166 is intentional
+  user-facing input feedback, retained.
 
 - **[SEC-3] Several `console.log` calls in production code.** Severity:
   Low. All have `[VYNDR]` / `[redis]` / `[poller-X]` prefixes and
@@ -456,14 +465,11 @@ No circular imports detected.
 
 ### DUP — Duplicates
 
-- **[DUP-1] `normalizeName()` exists twice.** Severity: Low.
-  - `src/services/intelligence/trapDetection.js`
-  - `scripts/populate-player-ids.js`
-  Implementations are near-identical (NFD strip, lowercase, suffix
-  removal). **Recommended fix:** extract to `src/utils/normalize.js`;
-  the script can `require('../src/utils/normalize')`. Skipped this
-  session because the script is intentionally self-contained for
-  remote execution.
+- **[DUP-1] `normalizeName()` exists twice.** Severity: Low. Status:
+  **DOCUMENTED in Session 7d.** Both implementations now carry
+  cross-reference comments explaining the divergence (script keeps
+  digits for legacy roster fields; trap detector strips them). Full
+  consolidation deferred until the script can drop its digit support.
 
 - **[DUP-2] `oddsToImplied` etc. only live in `src/utils/odds.js`.**
   Confirmed not duplicated despite the `oddsNormalizer.js` neighbor —
@@ -488,19 +494,27 @@ No circular imports detected.
 ### PERF — Performance
 
 - **[PERF-1] `/api/analyze/*` lacks Redis cache.** Severity: Medium.
-  Each call re-runs `analyzeProp()` end-to-end. **Recommended fix:**
-  cache the result by `(player, stat, line, direction, sport)` for
-  60s. Scope: ~30 min.
+  Status: **FIXED in Session 7d.** Added `cachedAnalyze()` wrapper in
+  `src/routes/analyze.js` with key
+  `analyze:{sport}:{player}:{stat}:{line}:{direction}` and a 60s TTL.
+  Response payload gains `_cache: 'HIT'|'MISS'` for observability.
+  3 unit tests verify HIT/MISS, independent keys, and case folding on
+  player name.
 
 - **[PERF-2] `routes/scan.js` resolves parlays one prop at a time.**
-  Sequential `await analyzeProp()` inside a for-loop. For 6-leg
-  parlays this is ~6x latency vs `Promise.allSettled`. **Recommended
-  fix:** parallelize independent props.
+  Severity: Medium. Status: **FIXED in Session 7d.** Replaced the
+  sequential `for (leg of legs) await analyzeProp(leg)` with
+  `Promise.allSettled(legs.map(analyzeProp))` in
+  `src/services/parlayScanService.js`. The picks-table writes also
+  switched from N sequential inserts to a single batched insert. A
+  behavioral test asserts a 6-leg parlay completes in well under
+  6 × delay wall-clock.
 
-- **[PERF-3] Bundle analysis not run this session.** The
-  `ANALYZE=true` flag would require `@next/bundle-analyzer` which is
-  not installed. **Recommended fix:** install bundle-analyzer dev dep
-  + run once + decide whether to ship a hidden audit/page route.
+- **[PERF-3] Bundle analyzer not installed.** Severity: Low. Status:
+  **FIXED in Session 7d.** `@next/bundle-analyzer@^16.2.9` added as a
+  devDependency and wired into `web/next.config.ts`. Inert by default;
+  emits `web/.next/analyze/{client,edge,nodejs}.html` when
+  `ANALYZE=true npm run build` runs.
 
 ### Frontend ↔ Backend contract