Merge pull request #1405 from samjay8/fix/audit-1253-1269-1273-1286

fix(audit): address 4 second-wave audit issues (#1253, #1269, #1273, #1286)

Author: K1NGD4VID

.github/workflows/ci.yml

@@ -64,6 +64,10 @@ jobs:
         run: npm run build
         working-directory: frontend
 
+      - name: Check frontend bundle size
+        run: bash ./scripts/check-bundle-size.sh
+        working-directory: frontend
+
   backend:
     name: Backend CI
     runs-on: ubuntu-latest

backend/src/controllers/stream.controller.ts

@@ -14,6 +14,7 @@ import {
 } from "../services/sorobanService.js";
 import type { AuthenticatedRequest } from "../types/auth.types.js";
 import { parseStreamId } from "../lib/stream-id.js";
+import { createStreamSchema } from "../validators/stream.validator.js";
 import {
   DEFAULT_EVENTS_PAGE_SIZE,
   MAX_EVENTS_PAGE_SIZE,
@@ -75,37 +76,6 @@ function sumStringI128(values: string[]): string {
   return total.toString();
 }
 
-/**
- * Thrown when a request body field fails presence/format validation. Kept
- * distinct from generic errors so createStream can reliably map it to a 400
- * response instead of falling through to the catch-all 500.
- */
-class StreamValidationError extends Error {
-  constructor(message: string) {
-    super(message);
-    this.name = "StreamValidationError";
-  }
-}
-
-/**
- * Validate presence and integer format of a required i128-style field, then
- * coerce it to a BigInt. Any missing value or conversion failure (SyntaxError
- * from a non-numeric string, TypeError from undefined/null/objects, etc.) is
- * normalized into a StreamValidationError so the caller can map it to 400.
- */
-function parseRequiredBigIntField(fieldName: string, value: unknown): bigint {
-  if (value === undefined || value === null || value === "") {
-    throw new StreamValidationError(`Missing required field: ${fieldName}`);
-  }
-  try {
-    return BigInt(value as bigint | number | string | boolean);
-  } catch {
-    throw new StreamValidationError(
-      `Invalid ${fieldName}: must be a valid integer`,
-    );
-  }
-}
-
 /**
  * Create a new stream (stub for on-chain indexing)
  */
@@ -116,19 +86,18 @@ export const createStream = async (req: Request, res: Response) => {
       return res.status(401).json({ error: 'Unauthorized', message: 'Authentication required' });
     }
 
-    const { streamId, sender, recipient, tokenAddress, ratePerSecond, depositedAmount, startTime } = req.body;
-
-    // Issue #809: validate identity fields before any DB write.
-    if (typeof sender !== 'string' || sender.length === 0) {
-      return res.status(400).json({ error: 'Invalid sender: must be a non-empty string' });
-    }
-    if (typeof recipient !== 'string' || recipient.length === 0) {
-      return res.status(400).json({ error: 'Invalid recipient: must be a non-empty string' });
-    }
-    if (typeof tokenAddress !== 'string' || tokenAddress.length === 0) {
-      return res.status(400).json({ error: 'Invalid tokenAddress: must be a non-empty string' });
+    // Validate request body using the Zod schema, which includes the MAX_I128
+    // upper-bound check on ratePerSecond that the manual parsing omitted.
+    const parsed = createStreamSchema.safeParse(req.body);
+    if (!parsed.success) {
+      return res.status(400).json({
+        error: 'Validation error',
+        details: parsed.error.issues,
+      });
     }
 
+    const { streamId: parsedStreamId, sender, recipient, tokenAddress, ratePerSecond, depositedAmount, startTime: parsedStartTime } = parsed.data;
+
     // Issue #809: the authenticated wallet may only create/modify streams it owns.
     // Without this, any logged-in wallet could POST an arbitrary `sender` and have
     // it persisted, or flip another owner's cancelled stream back to active.
@@ -139,41 +108,8 @@ export const createStream = async (req: Request, res: Response) => {
       });
     }
 
-    const parsedStreamId = parseStreamId(streamId);
-    const parsedStartTime = Number.parseInt(startTime, 10);
-
-    if (parsedStreamId === null) {
-      return res
-        .status(400)
-        .json({ error: "Invalid streamId: must be a valid integer" });
-    }
-
-    if (!Number.isFinite(parsedStartTime) || parsedStartTime < 0) {
-      return res
-        .status(400)
-        .json({ error: "Invalid startTime: must be a non-negative integer" });
-    }
-
-    // Presence/format validation happens here, before any BigInt coercion,
-    // so a malformed or missing numeric field always yields 400 rather than
-    // an uncaught SyntaxError/TypeError falling through to 500.
-    let parsedRatePerSecond: bigint;
-    let parsedDepositedAmount: bigint;
-    try {
-      parsedRatePerSecond = parseRequiredBigIntField(
-        "ratePerSecond",
-        ratePerSecond,
-      );
-      parsedDepositedAmount = parseRequiredBigIntField(
-        "depositedAmount",
-        depositedAmount,
-      );
-    } catch (validationError) {
-      if (validationError instanceof StreamValidationError) {
-        return res.status(400).json({ error: validationError.message });
-      }
-      throw validationError;
-    }
+    const parsedRatePerSecond = BigInt(ratePerSecond);
+    const parsedDepositedAmount = BigInt(depositedAmount);
 
     if (parsedRatePerSecond <= 0n) {
       return res

backend/src/services/soroban-indexer.service.ts

@@ -157,7 +157,7 @@ describe("Stream Controller", () => {
       expect(res.status).toHaveBeenCalledWith(400);
       expect(res.status).not.toHaveBeenCalledWith(500);
       expect(res.json).toHaveBeenCalledWith(
-        expect.objectContaining({ error: expect.stringContaining('ratePerSecond') })
+        expect.objectContaining({ error: 'Validation error' })
       );
     });
 
@@ -167,7 +167,7 @@ describe("Stream Controller", () => {
       expect(res.status).toHaveBeenCalledWith(400);
       expect(res.status).not.toHaveBeenCalledWith(500);
       expect(res.json).toHaveBeenCalledWith(
-        expect.objectContaining({ error: expect.stringContaining('depositedAmount') })
+        expect.objectContaining({ error: 'Validation error' })
       );
     });
 
@@ -177,7 +177,7 @@ describe("Stream Controller", () => {
       expect(res.status).toHaveBeenCalledWith(400);
       expect(res.status).not.toHaveBeenCalledWith(500);
       expect(res.json).toHaveBeenCalledWith(
-        expect.objectContaining({ error: expect.stringContaining('ratePerSecond') })
+        expect.objectContaining({ error: 'Validation error' })
       );
     });
 
@@ -187,7 +187,7 @@ describe("Stream Controller", () => {
       expect(res.status).toHaveBeenCalledWith(400);
       expect(res.status).not.toHaveBeenCalledWith(500);
       expect(res.json).toHaveBeenCalledWith(
-        expect.objectContaining({ error: expect.stringContaining('depositedAmount') })
+        expect.objectContaining({ error: 'Validation error' })
       );
     });
   });

backend/tests/soroban-indexer.test.ts

@@ -205,21 +205,19 @@ Dashboard / NotificationDropdown  re-render with live data
 
 ### Indexer Ownership & Naming
 
-Three files with overlapping names live next to each other, but only one of them is the indexer that writes stream state. This section documents which is the source of truth and which is legacy so contributors know where to start when debugging indexing.
+Two files share the `indexer` name, but only one of them is the indexer that writes stream state. This section documents which is the source of truth so contributors know where to start when debugging indexing.
 
 | File | Role | Status |
 |------|------|--------|
 | `backend/src/workers/soroban-event-worker.ts` (`SorobanEventWorker`) | **Source-of-truth indexer.** Polls Soroban RPC, decodes XDR, persists `Stream` / `StreamEvent`, advances the `IndexerState` cursor, and broadcasts SSE. | Active / source of truth. Started by `backend/src/workers/index.ts` |
-| `backend/src/services/soroban-indexer.service.ts` (`SorobanIndexerService`) | **Legacy indexer being phased out.** A simpler duplicate poller that writes to the same rows and races with the worker. | **Legacy — do not extend.** Removal tracked with the functional consolidation (issue #801). Started directly from `backend/src/index.ts` |
-| `backend/src/services/indexerService.ts` | **Not an indexer at all.** Admin control-plane helpers (`getIndexerStatus`, `resetIndexer`, `replayFromLedger`) that read/reset `IndexerState` and trigger the worker's poll loop. | Active. The name is misleading; it was kept alongside the legacy indexer above |
+| `backend/src/services/indexerService.ts` | **Not an indexer at all.** Admin control-plane helpers (`getIndexerStatus`, `resetIndexer`, `replayFromLedger`) that read/reset `IndexerState` and trigger the worker's poll loop. | Active. The name is misleading. |
 
 Key points:
 
 1. **When debugging indexing, read `backend/src/workers/soroban-event-worker.ts` first.** It is the only file that persists canonical stream state.
-2. **Do not add new behavior to `soroban-indexer.service.ts`.** It exists only for backwards compatibility while the double-indexer race (issue #801) is consolidated.
-3. **`indexerService.ts` is control-plane only** — it never reads the chain; it manages the shared cursor and triggers replays.
+2. **`indexerService.ts` is control-plane only** — it never reads the chain; it manages the shared cursor and triggers replays.
 
-**Naming convention plan:** the team convention is kebab-case with a `.service.ts` suffix (e.g. `soroban-indexer.service.ts`, `claimable.service.ts`, `sse.service.ts`). The helper file `indexerService.ts` breaks that convention and is also a misleading name. Once the functional consolidation (issue #801) lands, `indexerService.ts` is expected to be renamed to `indexer.service.ts`.
+**Naming convention plan:** the team convention is kebab-case with a `.service.ts` suffix (e.g. `claimable.service.ts`, `sse.service.ts`). The helper file `indexerService.ts` breaks that convention and is also a misleading name. It is expected to be renamed to `indexer.service.ts`.
 
 ### Deduplication
 

backend/tests/stream.controller.test.ts

@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+# Bundle size budget check for the Next.js frontend build.
+# Compares the total size of static JS files in .next/static against a
+# configurable budget (default 300 KB gzipped).  Fails the CI step when
+# the budget is exceeded.
+set -euo pipefail
+
+BUDGET_BYTES=${FRONTEND_BUNDLE_BUDGET_BYTES:-614400}  # 600 KB
+NEXT_STATIC_DIR=".next/static"
+
+if [ ! -d "$NEXT_STATIC_DIR" ]; then
+  echo "Error: $NEXT_STATIC_DIR directory not found. Run 'next build' first."
+  exit 1
+fi
+
+total=0
+for f in $(find "$NEXT_STATIC_DIR" -type f -name "*.js" | head -100); do
+  # Use gzip -c | wc -c for accurate gzipped size
+  gzipped_size=$(gzip -c "$f" | wc -c)
+  total=$((total + gzipped_size))
+done
+
+echo "Frontend JS bundle gzipped size: ${total} bytes (${BUDGET_BYTES} byte budget)"
+
+if [ "$total" -gt "$BUDGET_BYTES" ]; then
+  echo "Error: Frontend bundle exceeds size budget!"
+  echo "  Actual:  ${total} bytes"
+  echo "  Budget:  ${BUDGET_BYTES} bytes"
+  echo "  Overage: $((total - BUDGET_BYTES)) bytes"
+  exit 1
+fi
+
+echo "Bundle size OK ✓"