API hygiene: unknown /api/* routes return SPA HTML with 200, fallback answers all HTTP methods, numeric params accept trailing garbage

[LarsLaskowski]

Severity: Low
Category: Correctness / hardening (server)
Affected files: src/server/index.ts (SPA fallback, lines 71–74), src/server/routes.ts (parseSyncLimit lines 170–186, parseAfterId lines 188–196), src/server/config.ts (clampInteger lines 5–21). Line numbers refer to main @ 3ac64da.

Problem

Three small, related hygiene gaps:

1. Unknown API routes return HTML with 200. GET /api/anything-unknown falls through the API router, does not match express.static, and lands in the catch-all sendFile(index.html). A caller expecting JSON gets a 200 text/html response. This masks client bugs (a typo'd endpoint "succeeds", then fails later at response.json()), and confuses monitoring/uptime probes.
2. The SPA fallback answers every HTTP method. It is registered with app.use, so POST /, PUT /whatever, DELETE /x all receive the HTML with 200 instead of an error status.
3. Lenient integer parsing. Number.parseInt('50abc', 10) → 50, so limit=50abc and afterId=7xyz are silently accepted by /api/bootstrap and /api/resync, and env values like PORT=9001garbage are accepted by clampInteger in config.ts.

Suggested fix

1. JSON 404 for unknown API routes — at the end of createApiRouter in src/server/routes.ts, after the four route registrations, add a terminal handler (it runs for any method/path under /api that nothing above matched):

router.use((_request: Request, response: Response) => {
  response.status(404).json({ error: 'Not found' });
});

2. Restrict the SPA fallback to GET/HEAD — in src/server/index.ts, inside the final catch-all middleware, before sendFile:

app.use((request, response) => {
  if (request.method !== 'GET' && request.method !== 'HEAD') {
    response.status(404).end();
    return;
  }
  response.sendFile(path.join(clientDistDir, 'index.html'));
});

3. Strict integer parsing — add one helper in src/server/routes.ts:

function parseStrictNonNegativeInt(value: string): number | null {
  if (!/^\d+$/.test(value)) {
    return null;
  }
  const parsed = Number.parseInt(value, 10);
  return Number.isSafeInteger(parsed) ? parsed : null;
}

Use it inside parseSyncLimit (a value failing the check → return null → existing 400 response; keep the parsedLimit < 1 rule and Math.min(parsedLimit, maxLimit) clamping) and parseAfterId (failing → null → 400; >= 0 already covered by the regex). In src/server/config.ts clampInteger, apply the same /^\d+$/ pre-check on the trimmed raw value (env values are never negative-with-sign today; anything non-matching falls back) and keep the existing warn-and-fallback behavior — configuration must never make startup fail here.

Acceptance criteria

• GET /api/nope → 404 with JSON body { "error": "Not found" }.
• POST / (and other non-GET/HEAD to non-API paths) → 404, no HTML body.
• GET / and GET /some/client/route still return index.html with 200 (SPA behavior unchanged); static assets unaffected.
• GET /api/bootstrap?limit=50abc → 400; GET /api/resync?afterId=7x&limit=10 → 400; plain limit=50 / afterId=7 keep working.
• PORT=9001x falls back to the default with the existing [config] warning (test in src/server/config.test.ts).
• npm test and npm run typecheck pass.