mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-07-27 12:18:59 +00:00
fix(proxy): skip express.json() for remote proxy requests to fix body forwarding
The previous fix attempted to re-stream req.body via proxyReq.write() in the proxyReq event handler. This raced against http-proxy's synchronous req.pipe(proxyReq) call: when express.json() consumed the IncomingMessage stream, Node.js's pipe() detected readableEnded and scheduled process.nextTick(proxyReq.end), which fired before the proxyReq socket event. Any subsequent write() threw "write after end", leaving the remote server stalled waiting for body bytes that never arrived (the "Add Rule spins forever" bug). Root fix: a conditional middleware now skips express.json() entirely for requests targeting a remote node on non-auth, non-nodes API paths. The raw stream is left intact, allowing http-proxy's req.pipe(proxyReq) to forward the body correctly.
This commit is contained in:
+1
-1
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- **Fixed:** `SettingsModal` Notifications tab is no longer hidden when a remote node is active — users can now configure Discord/Slack/Webhook channels directly on any remote node. The section header shows the remote node name and a "Remote" badge with a tooltip explaining that channels are saved on the remote instance.
|
||||
- **Fixed:** `StackAlertSheet` form error handling now surfaces the actual server error message (`err.error`) instead of a generic "Failed to add alert rule." string; console error logging added for all failure paths to aid debugging.
|
||||
- **Fixed:** `POST /api/alerts` now validates the request body with a Zod schema — rejects unknown metric/operator values, negative thresholds, and missing required fields with a structured 400 response instead of passing raw input to SQLite.
|
||||
- **Fixed:** Remote node HTTP proxy (`remoteNodeProxy` in `index.ts`) now re-streams the parsed request body for POST/PUT/PATCH requests — `express.json()` drains the incoming stream into `req.body`, leaving the raw stream empty; the proxy then forwarded a request with a `Content-Length` header but zero body bytes, causing the remote Express server to stall waiting for the body indefinitely (the "Add Rule spins forever" bug on remote nodes). The `proxyReq` handler now writes `JSON.stringify(req.body)` with correct `Content-Type` and `Content-Length` headers before forwarding.
|
||||
- **Fixed:** Remote node HTTP proxy body forwarding — replaced the manual `proxyReq.write(JSON.stringify(req.body))` approach (which raced against `http-proxy`'s `process.nextTick(proxyReq.end)` and lost, causing "write after end" and the request to hang) with a conditional JSON body parser that skips `express.json()` entirely for remote-targeted `/api/` requests. The raw `IncomingMessage` stream is left unconsumed so `http-proxy`'s `req.pipe(proxyReq)` can forward it intact. This fixes the "Add Rule spins forever on remote nodes" bug.
|
||||
- **Fixed:** WebSocket notification reconnect in `EditorLayout` upgraded to exponential backoff (1 s → 2 s → 4 s → 8 s → 16 s → 30 s max) instead of a flat 5-second retry; `ws.onerror` now logs the event rather than silently calling `close()`; cleanup correctly guards against closing a WebSocket that is already in CLOSING/CLOSED state, eliminating the "WebSocket is closed before the connection is established" console error on React StrictMode double-mount.
|
||||
- **Security:** Host Console and container exec WebSocket endpoints now reject `node_proxy` scoped JWT tokens with HTTP 403 — machine-to-machine proxy credentials can no longer be used to open interactive terminals.
|
||||
- **Security:** `stackParam` query parameter on `/api/system/host-console` is now validated against `path.resolve` + `startsWith(baseDir)` to prevent directory traversal when setting the PTY working directory.
|
||||
|
||||
+30
-13
@@ -68,7 +68,32 @@ app.use(cors({
|
||||
origin: true,
|
||||
credentials: true,
|
||||
}));
|
||||
app.use(express.json());
|
||||
// Conditionally parse JSON bodies. Remote proxy requests must NOT have their body
|
||||
// consumed here: express.json() drains the IncomingMessage stream into req.body
|
||||
// and http-proxy then pipes an already-ended stream to the remote server.
|
||||
// When Node.js pipes an ended readable it calls process.nextTick(dest.end()),
|
||||
// which fires *before* the proxyReq socket event, so any attempt to write the
|
||||
// body inside the proxyReq handler results in "write after end" and the request
|
||||
// hangs. Solution: skip JSON parsing for remote-targeted /api/ requests so the
|
||||
// raw stream flows through the proxy intact.
|
||||
app.use((req: Request, res: Response, next: NextFunction): void => {
|
||||
const nodeIdHeader = req.headers['x-node-id'];
|
||||
if (nodeIdHeader) {
|
||||
const nodeId = parseInt(nodeIdHeader as string, 10);
|
||||
const node = NodeRegistry.getInstance().getNode(nodeId);
|
||||
if (
|
||||
node?.type === 'remote' &&
|
||||
req.path.startsWith('/api/') &&
|
||||
!req.path.startsWith('/api/auth/') &&
|
||||
!req.path.startsWith('/api/nodes')
|
||||
) {
|
||||
// Preserve body stream for proxy piping
|
||||
next();
|
||||
return;
|
||||
}
|
||||
}
|
||||
express.json()(req, res, next);
|
||||
});
|
||||
app.use(cookieParser());
|
||||
|
||||
// Node Context Middleware
|
||||
@@ -370,18 +395,10 @@ const remoteNodeProxy = createProxyMiddleware<Request, Response>({
|
||||
const newQs = params.toString();
|
||||
proxyReq.path = pathname + (newQs ? `?${newQs}` : '');
|
||||
}
|
||||
// Re-stream the request body for POST/PUT/PATCH requests.
|
||||
// express.json() fully drains the incoming stream and stores the parsed
|
||||
// object in req.body. http-proxy-middleware then pipes an already-consumed
|
||||
// stream to the remote, which causes the remote server to stall waiting for
|
||||
// body bytes that never arrive (the Content-Length header says N bytes but
|
||||
// 0 are forwarded), hanging the request indefinitely.
|
||||
if (req.body && Object.keys(req.body).length > 0) {
|
||||
const bodyData = JSON.stringify(req.body);
|
||||
proxyReq.setHeader('Content-Type', 'application/json');
|
||||
proxyReq.setHeader('Content-Length', Buffer.byteLength(bodyData));
|
||||
proxyReq.write(bodyData);
|
||||
}
|
||||
// Body forwarding: the conditional json parser (see top of file) skips
|
||||
// parsing for remote requests, so req's raw stream is intact and
|
||||
// http-proxy's req.pipe(proxyReq) forwards the body automatically.
|
||||
// No manual body rewriting needed here.
|
||||
},
|
||||
error: (err, _req, proxyRes) => {
|
||||
console.error('[Proxy] Remote node error:', (err as Error).message);
|
||||
|
||||
Reference in New Issue
Block a user