fix(MESHCENT-002): CU-86akhf8u4 12 review findings across 6 files

[flamingo]

Closes 12 review findings across 6 files.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

#	Fix confidence	Finding	Location
1	🟢 90 high	ws.send() call to agent bypasses try/catch in performRelay	meshdevicefile.js:61
2	🟡 85 medium	console.log(e) references undefined variable in multiple catch(ex) blocks in closeBothSides	meshdevicefile.js:61
3	🟢 90 high	Undefined 'ws' identifier referenced instead of 'obj.ws' in performRelay	meshdevicefile.js:135
4	🔴 55 low — review closely	obj.file from req.query.f is used unchecked in file relay without a '..' guard	meshdevicefile.js:24
5	🔴 45 low — review closely	ws.send-equivalent socket writes in interceptor.js are not wrapped in try/catch	interceptor.js:93
6	🟢 90 high	Same inverted length-clamp bug duplicated in AMT-side body handler	interceptor.js:86
7	🟢 90 high	Off-by-one buffer truncation bug in amt-wsman-duk.js PerformAjaxEx length-body branch	interceptor.js:197
8	🟢 95 high	ws.send() calls in meshbot.js not wrapped in try/catch	meshbot.js:111
9	🟢 95 high	ws.send() call in meshbot.js interuser echo path not wrapped in try/catch	meshbot.js:119
10	🟡 85 medium	ws.send() calls in mstsc client unwrapped in try/catch	public/mstsc/client.js:131
11	🟡 80 medium	ws.send()-equivalent socket writes in swarmserver.js relay path lack try/catch	swarmserver.js:165
12	🟢 92 high	IDER module's obj.SendCommand calls parent.xxSend without try/catch guard around transport write	amt/amt-ider-module.js:173

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.

---

Run: https://product-hub.flamingo.so/admin/code-review
Run id: a05149ef-c4b4-48a6-a445-70f3e3fde4b4

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

ClickUp task: CU-86akhf8u4 MeshCentral review findings sweep (3 PRs)

[flamingo]

🦩 What this fix changed, finding by finding

12 finding(s) fixed in this draft — 12 explained inline on the diff; 2 low-confidence hunk(s) need close review before merging.

[flamingo]

🦩 🔴 ws.send() call to agent bypasses try/catch in performRelay

In obj.close() (top of file), the two catch blocks catch (ex) { console.log(e); } were changed to catch (ex) { console.log(ex); } for the soft-disconnect and hard-disconnect ws.close()/ _socket._parent.end() calls, so the catch handler no longer throws a ReferenceError on the undefined e.

🤖 Prompt for AI agents

In meshdevicefile.js around line 61, review and complete this code-review fix: ws.send() call to agent bypasses try/catch in performRelay.
What the draft fix changed: In obj.close() (top of file), the two catch blocks `catch (ex) { console.log(e); }` were changed to `catch (ex) { console.log(ex); }` for the soft-disconnect and hard-disconnect ws.close()/ _socket._parent.end() calls, so the catch handler no longer throws a ReferenceError on the undefined `e`.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 console.log(e) references undefined variable in multiple catch(ex) blocks in closeBothSides

In closeBothSides(), the peer disconnect block if (peer.ws) { try { peer.ws.close(); } catch (e) { } try { peer.ws._socket._parent.end(); } catch (e) { } } was changed to bind and log the correct exception variable: try { peer.ws.close(); } catch (ex) { console.log(ex); } try { peer.ws._socket._parent.end(); } catch (ex) { console.log(ex); }, eliminating the undefined e reference in that catch scope; the already-correct catch (ex) { console.log(ex); } for relaySessionCounted decrement was left unchanged.

🤖 Prompt for AI agents

In meshdevicefile.js around line 61, review and complete this code-review fix: console.log(e) references undefined variable in multiple catch(ex) blocks in closeBothSides.
What the draft fix changed: In closeBothSides(), the peer disconnect block `if (peer.ws) { try { peer.ws.close(); } catch (e) { } try { peer.ws._socket._parent.end(); } catch (e) { } }` was changed to bind and log the correct exception variable: `try { peer.ws.close(); } catch (ex) { console.log(ex); } try { peer.ws._socket._parent.end(); } catch (ex) { console.log(ex); }`, eliminating the undefined `e` reference in that catch scope; the already-correct `catch (ex) { console.log(ex); }` for relaySessionCounted decrement was left unchanged.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 Undefined 'ws' identifier referenced instead of 'obj.ws' in performRelay

In performRelay(), the auth-failure branch if (ws) { ws.close(); } was changed to if (obj.ws) { obj.ws.close(); }, matching the function's actual local socket variable and fixing the ReferenceError on the undefined bare ws.

🤖 Prompt for AI agents

In meshdevicefile.js around line 135, review and complete this code-review fix: Undefined 'ws' identifier referenced instead of 'obj.ws' in performRelay.
What the draft fix changed: In performRelay(), the auth-failure branch `if (ws) { ws.close(); }` was changed to `if (obj.ws) { obj.ws.close(); }`, matching the function's actual local socket variable and fixing the ReferenceError on the undefined bare `ws`.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 obj.file from req.query.f is used unchecked in file relay without a '..' guard

Added a guard right after obj.file = req.query.f; at the top of CreateMeshDeviceFile: if ((obj.file != null) && (obj.file.indexOf('..') >= 0)) { obj.file = null; }. This blocks any path containing '..' from being forwarded to the agent as relayinfo.peer1.file/peer2.file. Unverified: setting obj.file to null rather than closing/rejecting the connection outright means a malicious client gets a null file value passed to the agent's 'options' message instead of an explicit error; a more complete fix might close the connection or return an HTTP error when '..' is detected, but that would touch more control flow than the minimal fix here.

🤖 Prompt for AI agents

In meshdevicefile.js around line 24, review and complete this code-review fix: obj.file from req.query.f is used unchecked in file relay without a '..' guard.
What the draft fix changed: Added a guard right after `obj.file = req.query.f;` at the top of CreateMeshDeviceFile: `if ((obj.file != null) && (obj.file.indexOf('..') >= 0)) { obj.file = null; }`. This blocks any path containing '..' from being forwarded to the agent as relayinfo.peer1.file/peer2.file. Unverified: setting obj.file to null rather than closing/rejecting the connection outright means a malicious client gets a null file value passed to the agent's 'options' message instead of an explicit error; a more complete fix might close the connection or return an HTTP error when '..' is detected, but that would touch more control flow than the minimal fix here.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 ws.send-equivalent socket writes in interceptor.js are not wrapped in try/catch

Wrapped the top-level entry points that process incoming socket data (processAmtData and processBrowserData in both CreateHttpInterceptor and CreateRedirInterceptor) in try/catch blocks, logging via obj.Debug and returning an empty Buffer on failure, so an exception during accumulation/parsing (which feeds the socket relay in server.js) no longer propagates uncaught. This file itself contains no direct req.end/socket-write calls (those live in server.js per the finding's own evidence), so the mitigation applied here is defensive wrapping of the data-processing functions whose output ultimately drives those writes; the actual req.end(postdata) call site in server.js is out of scope for this file and was not touched, so this is a partial mitigation of the underlying concern.

🤖 Prompt for AI agents

In interceptor.js around line 93, review and complete this code-review fix: ws.send-equivalent socket writes in interceptor.js are not wrapped in try/catch.
What the draft fix changed: Wrapped the top-level entry points that process incoming socket data (`processAmtData` and `processBrowserData` in both `CreateHttpInterceptor` and `CreateRedirInterceptor`) in try/catch blocks, logging via `obj.Debug` and returning an empty Buffer on failure, so an exception during accumulation/parsing (which feeds the socket relay in server.js) no longer propagates uncaught. This file itself contains no direct `req.end`/socket-write calls (those live in server.js per the finding's own evidence), so the mitigation applied here is defensive wrapping of the data-processing functions whose output ultimately drives those writes; the actual `req.end(postdata)` call site in server.js is out of scope for this file and was not touched, so this is a partial mitigation of the underlying concern.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 45 low — review closely — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 ws.send() calls in meshbot.js not wrapped in try/catch

In the 'userinfo' case handler of the ws.on('message', ...) callback in serverConnect(), wrapped the ws.send() call (interuser hello message) in a try/catch block with an empty catch, matching the suggested fix exactly.

🤖 Prompt for AI agents

In meshbot.js around line 111, review and complete this code-review fix: ws.send() calls in meshbot.js not wrapped in try/catch.
What the draft fix changed: In the 'userinfo' case handler of the ws.on('message', ...) callback in serverConnect(), wrapped the ws.send() call (interuser hello message) in a try/catch block with an empty catch, matching the suggested fix exactly.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 ws.send() call in meshbot.js interuser echo path not wrapped in try/catch

In the 'interuser' case handler of the ws.on('message', ...) callback in serverConnect(), wrapped the ws.send() echo call in a try/catch block with an empty catch, matching the suggested fix exactly.

🤖 Prompt for AI agents

In meshbot.js around line 119, review and complete this code-review fix: ws.send() call in meshbot.js interuser echo path not wrapped in try/catch.
What the draft fix changed: In the 'interuser' case handler of the ws.on('message', ...) callback in serverConnect(), wrapped the ws.send() echo call in a try/catch block with an empty catch, matching the suggested fix exactly.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 ws.send() calls in mstsc client unwrapped in try/catch

Wrapped every self.socket.send(...) call in try/catch throughout public/mstsc/client.js: in the mousemove nagle timeout callback, mousedown, mouseup, contextmenu, DOMMouseScroll, mousewheel, keydown, and keyup event handlers inside Client.prototype.install, plus the onopen handler's initial infos send and the clipboard polling setInterval send inside Client.prototype.connect. Each send is now guarded with try { ... } catch (ex) { } so a socket closing between the activeSession/socket check and the send no longer throws an unhandled exception out of a DOM event handler or timer callback, matching the suggested fix pattern for all occurrences, not just the cited keydown example.

🤖 Prompt for AI agents

In public/mstsc/client.js around line 131, review and complete this code-review fix: ws.send() calls in mstsc client unwrapped in try/catch.
What the draft fix changed: Wrapped every `self.socket.send(...)` call in try/catch throughout `public/mstsc/client.js`: in the `mousemove` nagle timeout callback, `mousedown`, `mouseup`, `contextmenu`, `DOMMouseScroll`, `mousewheel`, `keydown`, and `keyup` event handlers inside `Client.prototype.install`, plus the `onopen` handler's initial `infos` send and the clipboard polling `setInterval` send inside `Client.prototype.connect`. Each send is now guarded with `try { ... } catch (ex) { }` so a socket closing between the `activeSession`/`socket` check and the send no longer throws an unhandled exception out of a DOM event handler or timer callback, matching the suggested fix pattern for all occurrences, not just the cited keydown example.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🔴 ws.send()-equivalent socket writes in swarmserver.js relay path lack try/catch

In onData's relay-setup handler (the tls.connect(...) callback used for HTTP-detected relay connections), wrapped this.write(this.parentSocket.tag.accumulator); this.parentSocket.resume(); in a try/catch, matching the defensive pattern already used in the sibling relaySocket.on('data', ...), on('error', ...), and on('end', ...) handlers. Additionally, since the finding referenced the broader universal pattern violation in this relay path, also wrapped two other unguarded socket writes in the same function/module for consistency and completeness: the this.write('HTTP/1.1 200 OK...') call in onData (no-client-cert branch) and the socket.write(...) calls inside Write(socket, data) (used by obj.SendCommand, which every protocol command handler in ProcessCommand relies on). These are the only unguarded socket/TLS write call sites in the file; all try/catch blocks added use empty catch bodies (catch (ex) { }) consistent with existing style in the file (e.g. relaySocket.on('data', ...)).

🤖 Prompt for AI agents

In swarmserver.js around line 165, review and complete this code-review fix: ws.send()-equivalent socket writes in swarmserver.js relay path lack try/catch.
What the draft fix changed: In `onData`'s relay-setup handler (the `tls.connect(...)` callback used for HTTP-detected relay connections), wrapped `this.write(this.parentSocket.tag.accumulator); this.parentSocket.resume();` in a try/catch, matching the defensive pattern already used in the sibling `relaySocket.on('data', ...)`, `on('error', ...)`, and `on('end', ...)` handlers. Additionally, since the finding referenced the broader universal pattern violation in this relay path, also wrapped two other unguarded socket writes in the same function/module for consistency and completeness: the `this.write('HTTP/1.1 200 OK...')` call in `onData` (no-client-cert branch) and the `socket.write(...)` calls inside `Write(socket, data)` (used by `obj.SendCommand`, which every protocol command handler in `ProcessCommand` relies on). These are the only unguarded socket/TLS write call sites in the file; all `try/catch` blocks added use empty catch bodies (`catch (ex) { }`) consistent with existing style in the file (e.g. `relaySocket.on('data', ...)`).
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 80 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 IDER module's obj.SendCommand calls parent.xxSend without try/catch guard around transport write

In obj.SendCommand (amt/amt-ider-module.js), wrapped the transport write obj.parent.xxSend(x); in a try { ... } catch (ex) { } block, matching the codebase-wide convention (MESHCENT-002/002-2) of guarding ws.send-equivalent transport writes so that an exception from a closed/proxied socket does not propagate uncaught through the IDER command pipeline. obj.bytesToAmt += x.length; remains outside the try block so byte accounting is unaffected by the change, consistent with existing behavior.

🤖 Prompt for AI agents

In amt/amt-ider-module.js around line 173, review and complete this code-review fix: IDER module's obj.SendCommand calls parent.xxSend without try/catch guard around transport write.
What the draft fix changed: In `obj.SendCommand` (amt/amt-ider-module.js), wrapped the transport write `obj.parent.xxSend(x);` in a `try { ... } catch (ex) { }` block, matching the codebase-wide convention (MESHCENT-002/002-2) of guarding ws.send-equivalent transport writes so that an exception from a closed/proxied socket does not propagate uncaught through the IDER command pipeline. `obj.bytesToAmt += x.length;` remains outside the try block so byte accounting is unaffected by the change, consistent with existing behavior.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 92 high — react 👍/👎 to teach the reviewer