Farhan Beg commited on
Commit
7f18e15
Β·
1 Parent(s): 6303999

bugfix: error handlers, timeouts, global guards (audit Group D)

Browse files

Five bug fixes in health-server.js from the comprehensive audit:

- D1 upstream.on('error'): every piped upstream IncomingMessage
(proxyRequest body + non-body paths, proxyDashboard non-rewrite path)
now has an error listener. An unhandled 'error' on a piped stream
throws and can crash the router β€” a backend socket reset mid-response
would take down every fronted service.
- D2 headersSent guards: the proxyRequest non-body error handler and
the proxyDashboard ClientRequest + upRes error handlers called
res.writeHead() unconditionally. If the response callback already
fired and is streaming, that throws ERR_HTTP_HEADERS_SENT inside the
error handler β†’ unhandled exception. Now guarded; late errors call
res.destroy() instead.
- D3 upstream timeouts: 30s setTimeout on every proxied request
(proxyRequest body + non-body, proxyDashboard). A hung backend that
accepts the socket but never responds previously held the request +
upstream socket open indefinitely, exhausting FDs under load. Now
returns 504 + destroys the socket.
- D4 global guards: process.on('uncaughtException'/'unhandledRejection')
+ server.on('error') log and continue instead of crashing the router
on a single bad request. Last-resort backstop for any stream error
we missed an 'error' listener on.
- D5 referer routing comment: the refererIsDashboard routing is
client-controlled (Referer is spoofable), so document that it's a
functional routing hint, not a privilege boundary β€” the block calls
requireAuth() before proxying, so a spoofed Referer grants nothing.

Files changed (1) hide show
  1. health-server.js +114 -8
health-server.js CHANGED
@@ -436,12 +436,34 @@ function proxyRequest(
436
  (upstream) => {
437
  res.writeHead(upstream.statusCode || 502, upstream.headers);
438
  upstream.pipe(res);
 
 
 
 
 
 
 
 
 
 
 
439
  },
440
  );
 
 
 
 
 
 
 
 
 
441
  proxy.on("error", (error) => {
442
  if (!res.headersSent) {
443
  res.writeHead(502, { "content-type": "application/json" });
444
  res.end(JSON.stringify({ error: "proxy_error", message: error.message }));
 
 
445
  }
446
  });
447
  if (size > 0) proxy.write(Buffer.concat(chunks));
@@ -468,12 +490,42 @@ function proxyRequest(
468
  (upstream) => {
469
  res.writeHead(upstream.statusCode || 502, upstream.headers);
470
  upstream.pipe(res);
 
 
 
 
 
 
 
 
 
 
 
 
 
 
471
  },
472
  );
473
 
 
 
 
 
 
 
 
 
 
 
 
 
474
  proxy.on("error", (error) => {
475
- res.writeHead(502, { "content-type": "application/json" });
476
- res.end(JSON.stringify({ error: "proxy_error", message: error.message }));
 
 
 
 
477
  });
478
 
479
  req.pipe(proxy);
@@ -541,6 +593,18 @@ function proxyDashboard(req, res) {
541
  if (!shouldRewrite) {
542
  res.writeHead(upRes.statusCode || 502, upRes.headers);
543
  upRes.pipe(res);
 
 
 
 
 
 
 
 
 
 
 
 
544
  return;
545
  }
546
 
@@ -581,17 +645,40 @@ function proxyDashboard(req, res) {
581
  res.end(buf);
582
  });
583
  upRes.on("error", () => {
584
- try {
585
- res.writeHead(502);
586
- res.end();
587
- } catch {}
 
 
 
 
 
 
 
588
  });
589
  },
590
  );
591
 
 
 
 
 
 
 
 
 
 
 
 
 
592
  upstream.on("error", (error) => {
593
- res.writeHead(502, { "content-type": "application/json" });
594
- res.end(JSON.stringify({ error: "proxy_error", message: error.message }));
 
 
 
 
595
  });
596
 
597
  // Buffer body before forwarding β€” same chunked-encoding fix as proxyRequest.
@@ -1228,6 +1315,11 @@ except Exception:
1228
  })();
1229
  const refererIsDashboard = refererPath.startsWith(`${HM_PREFIX}/app`);
1230
 
 
 
 
 
 
1231
  if (refererIsDashboard) {
1232
  // Anything with a Referer from the dashboard goes to the dashboard,
1233
  // *except* requests that explicitly start with /webui (escape hatch).
@@ -1306,6 +1398,20 @@ server.listen(PORT, "0.0.0.0", () => {
1306
  console.log(`HuggingMes + Hermes WebUI router listening on 0.0.0.0:${PORT}`);
1307
  });
1308
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
1309
  /* ── WebSocket upgrade handling ─────────────────────────────────────
1310
  *
1311
  * Both the Hermes dashboard and hermes-webui can open WebSocket
 
436
  (upstream) => {
437
  res.writeHead(upstream.statusCode || 502, upstream.headers);
438
  upstream.pipe(res);
439
+ // D1: handle mid-response backend socket errors without crashing.
440
+ upstream.on("error", () => {
441
+ if (!res.headersSent) {
442
+ try {
443
+ res.writeHead(502, { "content-type": "application/json" });
444
+ res.end(JSON.stringify({ error: "upstream_error" }));
445
+ } catch {}
446
+ } else {
447
+ try { res.destroy(); } catch {}
448
+ }
449
+ });
450
  },
451
  );
452
+ // D3: 30s timeout on the upstream request.
453
+ proxy.setTimeout(30000, () => {
454
+ if (!res.headersSent) {
455
+ res.writeHead(504, { "content-type": "application/json" });
456
+ res.end(JSON.stringify({ error: "upstream_timeout" }));
457
+ }
458
+ try { proxy.destroy(new Error("upstream_timeout")); } catch {}
459
+ });
460
+ // D2: guard headersSent on the proxy error handler.
461
  proxy.on("error", (error) => {
462
  if (!res.headersSent) {
463
  res.writeHead(502, { "content-type": "application/json" });
464
  res.end(JSON.stringify({ error: "proxy_error", message: error.message }));
465
+ } else {
466
+ try { res.destroy(); } catch {}
467
  }
468
  });
469
  if (size > 0) proxy.write(Buffer.concat(chunks));
 
490
  (upstream) => {
491
  res.writeHead(upstream.statusCode || 502, upstream.headers);
492
  upstream.pipe(res);
493
+ // D1: an unhandled 'error' on a piped IncomingMessage throws and can
494
+ // crash the router. If the backend socket resets mid-response, log +
495
+ // destroy the response cleanly instead of taking down every fronted
496
+ // service.
497
+ upstream.on("error", () => {
498
+ if (!res.headersSent) {
499
+ try {
500
+ res.writeHead(502, { "content-type": "application/json" });
501
+ res.end(JSON.stringify({ error: "upstream_error" }));
502
+ } catch {}
503
+ } else {
504
+ try { res.destroy(); } catch {}
505
+ }
506
+ });
507
  },
508
  );
509
 
510
+ // D3: 30s timeout so a hung backend (accepts the socket but never
511
+ // responds) can't hold a request + upstream socket open forever.
512
+ proxy.setTimeout(30000, () => {
513
+ if (!res.headersSent) {
514
+ res.writeHead(504, { "content-type": "application/json" });
515
+ res.end(JSON.stringify({ error: "upstream_timeout" }));
516
+ }
517
+ try { proxy.destroy(new Error("upstream_timeout")); } catch {}
518
+ });
519
+
520
+ // D2: guard headersSent so a late 'error' after headers were already
521
+ // written doesn't throw ERR_HTTP_HEADERS_SENT inside the error handler.
522
  proxy.on("error", (error) => {
523
+ if (!res.headersSent) {
524
+ res.writeHead(502, { "content-type": "application/json" });
525
+ res.end(JSON.stringify({ error: "proxy_error", message: error.message }));
526
+ } else {
527
+ try { res.destroy(); } catch {}
528
+ }
529
  });
530
 
531
  req.pipe(proxy);
 
593
  if (!shouldRewrite) {
594
  res.writeHead(upRes.statusCode || 502, upRes.headers);
595
  upRes.pipe(res);
596
+ // D1: handle mid-response backend socket errors on the non-rewrite
597
+ // path (the rewrite path has its own upRes.on('error') below).
598
+ upRes.on("error", () => {
599
+ if (!res.headersSent) {
600
+ try {
601
+ res.writeHead(502, { "content-type": "application/json" });
602
+ res.end(JSON.stringify({ error: "upstream_error" }));
603
+ } catch {}
604
+ } else {
605
+ try { res.destroy(); } catch {}
606
+ }
607
+ });
608
  return;
609
  }
610
 
 
645
  res.end(buf);
646
  });
647
  upRes.on("error", () => {
648
+ // D2: guard headersSent β€” in the rewrite path, res.writeHead may
649
+ // already have fired (it fires in the 'end' handler). The old code
650
+ // called writeHead unconditionally β†’ ERR_HTTP_HEADERS_SENT.
651
+ if (!res.headersSent) {
652
+ try {
653
+ res.writeHead(502);
654
+ res.end();
655
+ } catch {}
656
+ } else {
657
+ try { res.destroy(); } catch {}
658
+ }
659
  });
660
  },
661
  );
662
 
663
+ // D3: 30s timeout on the dashboard upstream request.
664
+ upstream.setTimeout(30000, () => {
665
+ if (!res.headersSent) {
666
+ res.writeHead(504, { "content-type": "application/json" });
667
+ res.end(JSON.stringify({ error: "upstream_timeout" }));
668
+ }
669
+ try { upstream.destroy(new Error("upstream_timeout")); } catch {}
670
+ });
671
+
672
+ // D2: guard headersSent on the ClientRequest error handler. The old code
673
+ // called writeHead unconditionally, which throws if the response callback
674
+ // already fired and is streaming.
675
  upstream.on("error", (error) => {
676
+ if (!res.headersSent) {
677
+ res.writeHead(502, { "content-type": "application/json" });
678
+ res.end(JSON.stringify({ error: "proxy_error", message: error.message }));
679
+ } else {
680
+ try { res.destroy(); } catch {}
681
+ }
682
  });
683
 
684
  // Buffer body before forwarding β€” same chunked-encoding fix as proxyRequest.
 
1315
  })();
1316
  const refererIsDashboard = refererPath.startsWith(`${HM_PREFIX}/app`);
1317
 
1318
+ // NOTE: Referer is client-controlled, so a caller who sets Referer: /hm/app
1319
+ // can route requests that would otherwise go to WebUI (e.g. /api/*) to the
1320
+ // dashboard. This is functional routing, not a privilege boundary β€” the
1321
+ // block below calls requireAuth() before proxying to the dashboard, so a
1322
+ // spoofed Referer doesn't grant any access the caller didn't already have.
1323
  if (refererIsDashboard) {
1324
  // Anything with a Referer from the dashboard goes to the dashboard,
1325
  // *except* requests that explicitly start with /webui (escape hatch).
 
1398
  console.log(`HuggingMes + Hermes WebUI router listening on 0.0.0.0:${PORT}`);
1399
  });
1400
 
1401
+ // D4: last-resort guards so one bad request/response can't crash the
1402
+ // router and take down every fronted service. An unhandled stream error
1403
+ // (e.g. a backend socket reset on a response we forgot to attach an
1404
+ // 'error' listener to) would otherwise throw and terminate the process.
1405
+ process.on("uncaughtException", (err) => {
1406
+ console.error("uncaughtException in router (continuing):", err && err.stack ? err.stack : err);
1407
+ });
1408
+ process.on("unhandledRejection", (err) => {
1409
+ console.error("unhandledRejection in router (continuing):", err);
1410
+ });
1411
+ server.on("error", (err) => {
1412
+ console.error("router server error:", err && err.stack ? err.stack : err);
1413
+ });
1414
+
1415
  /* ── WebSocket upgrade handling ─────────────────────────────────────
1416
  *
1417
  * Both the Hermes dashboard and hermes-webui can open WebSocket