Skip to content

fix(adhoc-sweep-fixes): CU-86akdypw4 36 review findings across 19 files #165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
flamingo wants to merge 19 commits into
base: master
Choose a base branch
from
Draft

fix(adhoc-sweep-fixes): CU-86akdypw4 36 review findings across 19 files #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d53ba18
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
1d4ec6d
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
feba40d
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
e5b0138
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
4e8d02e
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
7d6eb54
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
2cbc35e
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
2d3da8e
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
29e7329
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
1c9868d
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
2bd9f00
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
c4273c6
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
295f8e7
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
d68ff99
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
6e5667c
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
4cc5a00
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
d23cbc8
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
4997516
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
1b4ab9c
fix(adhoc-sweep-fixes): 36 review findings across 19 files
flamingo[bot] Sep 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions agents/agentrecoverycore.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,12 +255,12 @@ require('MeshAgent').AddCommandHandler(function (data)
break;
case 'mkdir': {

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 fs.mkdirSync / fs.closeSync(fs.openSync) calls in recovery-console file-command handler are unguarded and will throw on failure

In the tunnel data handler's inner switch (cmd.action) (inside AddCommandHandler's upgrade/data callback), the mkdir case now wraps fs.mkdirSync(cmd.path) in try/catch reporting {action:'mkdirerror'}, and the mkfile case wraps fs.closeSync(fs.openSync(cmd.path,'w')) in try/catch reporting {action:'mkfileerror'}, matching the pattern used by the sibling upload case's uploaderror handling.

🤖 Prompt for AI agents 
In agents/agentrecoverycore.js around line 256, review and complete this code-review fix: fs.mkdirSync / fs.closeSync(fs.openSync) calls in recovery-console file-command handler are unguarded and will throw on failure.
What the draft fix changed: In the tunnel data handler's inner `switch (cmd.action)` (inside `AddCommandHandler`'s upgrade/data callback), the `mkdir` case now wraps `fs.mkdirSync(cmd.path)` in try/catch reporting `{action:'mkdirerror'}`, and the `mkfile` case wraps `fs.closeSync(fs.openSync(cmd.path,'w'))` in try/catch reporting `{action:'mkfileerror'}`, matching the pattern used by the sibling `upload` case's `uploaderror` handling.
Verify the change is correct and complete; do not refactor unrelated code.

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

// Create a new empty folder
fs.mkdirSync(cmd.path);
try { fs.mkdirSync(cmd.path); } catch (e) { this.write(new Buffer(JSON.stringify({ action: 'mkdirerror' }))); }
break;
}
case 'mkfile': {
// Create a new empty file
fs.closeSync(fs.openSync(cmd.path, 'w'));
try { fs.closeSync(fs.openSync(cmd.path, 'w')); } catch (e) { this.write(new Buffer(JSON.stringify({ action: 'mkfileerror' }))); }
break;
}
case 'rm': {
Comment on lines 255 to 266

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 AddCommandHandler switch on data.action in agentrecoverycore.js lacks a default case logging unknown actions

In require('MeshAgent').AddCommandHandler(...), the outer switch (data.action)'s default case (previously a no-op comment) now calls console.log('Unknown command action: ' + data.action) to log unrecognized actions. Confidence is moderate because the appropriate logging sink/format for this agent-side code (vs. sendConsoleText or a dedicated logger) wasn't specified, and console.log may not be visible in production recovery-agent deployments — a complete fix might need to route through an existing diagnostic channel used elsewhere in the codebase.

🤖 Prompt for AI agents 
In agents/agentrecoverycore.js around line 130, review and complete this code-review fix: AddCommandHandler switch on data.action in agentrecoverycore.js lacks a default case logging unknown actions.
What the draft fix changed: In `require('MeshAgent').AddCommandHandler(...)`, the outer `switch (data.action)`'s `default` case (previously a no-op comment) now calls `console.log('Unknown command action: ' + data.action)` to log unrecognized actions. Confidence is moderate because the appropriate logging sink/format for this agent-side code (vs. sendConsoleText or a dedicated logger) wasn't specified, and `console.log` may not be visible in production recovery-agent deployments — a complete fix might need to route through an existing diagnostic channel used elsewhere in the codebase.
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

Expand Down Expand Up @@ -336,7 +336,7 @@ require('MeshAgent').AddCommandHandler(function (data)
break;
}
default:
// Unknown action, ignore it.
console.log('Unknown command action: ' + data.action);
break;
}
}
Expand Down Expand Up @@ -469,3 +469,4 @@ function deleteFolderRecursive(path, rec) {
fs.unlinkSync(path);
}
};

4 changes: 2 additions & 2 deletions agents/modules_meshcmd/amt-lme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ function lme_heci(options) {
break;
case APF_SERVICE_REQUEST:

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 amt-lme.js compares Buffer to string with == instead of comparing strings, name check will always be false

In the APF_SERVICE_REQUEST case handler, changed var name = chunk.slice(5, nameLen + 5); to var name = chunk.slice(5, nameLen + 5).toString();, matching the pattern already used elsewhere in the file (e.g. APF_GLOBAL_REQUEST, APF_CHANNEL_OPEN) so name is now a string when compared with == against the string literals. The later name.toString() call in the outBuffer write path still works correctly since String.prototype.toString() is a no-op. Risk: this changes runtime behavior of a previously always-false branch, so the accept path will now actually execute for matching service names — this is the intended fix per the finding, but should be validated against real AMT interaction if possible since it enables code that may not have been exercised before.

🤖 Prompt for AI agents 
In agents/modules_meshcmd/amt-lme.js around line 156, review and complete this code-review fix: amt-lme.js compares Buffer to string with == instead of comparing strings, name check will always be false.
What the draft fix changed: In the APF_SERVICE_REQUEST case handler, changed `var name = chunk.slice(5, nameLen + 5);` to `var name = chunk.slice(5, nameLen + 5).toString();`, matching the pattern already used elsewhere in the file (e.g. APF_GLOBAL_REQUEST, APF_CHANNEL_OPEN) so `name` is now a string when compared with `==` against the string literals. The later `name.toString()` call in the outBuffer write path still works correctly since String.prototype.toString() is a no-op. Risk: this changes runtime behavior of a previously always-false branch, so the accept path will now actually execute for matching service names — this is the intended fix per the finding, but should be validated against real AMT interaction if possible since it enables code that may not have been exercised before.
Verify the change is correct and complete; do not refactor unrelated code.

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

var nameLen = chunk.readUInt32BE(1);
var name = chunk.slice(5, nameLen + 5);
var name = chunk.slice(5, nameLen + 5).toString();
//console.log("Service Request for: " + name);
if (name == 'pfwd@amt.intel.com' || name == 'auth@amt.intel.com') {
var outBuffer = Buffer.alloc(5 + nameLen);
Expand Down Expand Up @@ -214,7 +214,7 @@ function lme_heci(options) {
this.LMS.emit('bind', this._binded);
} catch (ex)

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 console.info1 is not a standard console method — likely a typo causing a runtime TypeError

In the APF_GLOBAL_REQUEST 'tcpip-forward' catch block (inside _LME.on('connect', ...)'s 'data' handler), changed console.info1(ex, 'Port ' + port); to console.info(ex, 'Port ' + port);. This directly replaces the nonexistent console.info1 method with the standard console.info, eliminating the secondary TypeError that would otherwise propagate uncaught from the HECI data handler.

🤖 Prompt for AI agents 
In agents/modules_meshcmd/amt-lme.js around line 215, review and complete this code-review fix: console.info1 is not a standard console method — likely a typo causing a runtime TypeError.
What the draft fix changed: In the APF_GLOBAL_REQUEST 'tcpip-forward' catch block (inside `_LME.on('connect', ...)`'s 'data' handler), changed `console.info1(ex, 'Port ' + port);` to `console.info(ex, 'Port ' + port);`. This directly replaces the nonexistent `console.info1` method with the standard `console.info`, eliminating the secondary TypeError that would otherwise propagate uncaught from the HECI data handler.
Verify the change is correct and complete; do not refactor unrelated code.

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

{
console.info1(ex, 'Port ' + port);
console.info(ex, 'Port ' + port);
if(!this._emitConnected)
{
this._emitConnected = true;
Expand Down
16 changes: 13 additions & 3 deletions agents/modules_meshcmd/amt-scanner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ function AMTScanner() {
if (masknum <= 16 || masknum > 32) return null;

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 amt-scanner.js: incorrect min bound off-by-one and dead variable in parseIPv4Range CIDR branch

Fixed the off-by-one/dead-min-max-bound bug in parseIPv4Range's CIDR branch (inside AMTScanner). Replaced the unconditional +1/-1 network/broadcast exclusion with computed netmin/netmax that are only adjusted (excluding network/broadcast addresses) when doing so keeps netmin < netmax, preventing min > max for /31, /32, or small ranges. For a /32 (mask=0) or /31 (mask=1) the range now falls back to including the full computed range instead of producing an invalid inverted bound, so the scan loop in scan() will execute instead of silently doing nothing.

🤖 Prompt for AI agents 
In agents/modules_meshcmd/amt-scanner.js around line 64, review and complete this code-review fix: amt-scanner.js: incorrect min bound off-by-one and dead variable in parseIPv4Range CIDR branch.
What the draft fix changed: Fixed the off-by-one/dead-min-max-bound bug in `parseIPv4Range`'s CIDR branch (inside `AMTScanner`). Replaced the unconditional `+1`/`-1` network/broadcast exclusion with computed `netmin`/`netmax` that are only adjusted (excluding network/broadcast addresses) when doing so keeps `netmin < netmax`, preventing `min > max` for /31, /32, or small ranges. For a /32 (mask=0) or /31 (mask=1) the range now falls back to including the full computed range instead of producing an invalid inverted bound, so the scan loop in `scan()` will execute instead of silently doing nothing.
Verify the change is correct and complete; do not refactor unrelated code.

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

masknum = 32 - masknum;
for (var i = 0; i < masknum; i++) { mask = (mask << 1); mask++; }
return { min: (ip & (0xFFFFFFFF - mask))+1, max: (ip & (0xFFFFFFFF - mask)) + mask -1 };//remove network and broadcast address to avoid irrecoverable socket error
var netmin = (ip & (0xFFFFFFFF - mask)), netmax = (ip & (0xFFFFFFFF - mask)) + mask;
if (netmin < netmax) { netmin++; netmax--; } // remove network and broadcast address to avoid irrecoverable socket error, unless range is too small
return { min: netmin, max: netmax };
}
x = this.parseIpv4Addr(range);
if (x == null) return null;
Expand All @@ -89,7 +91,15 @@ function AMTScanner() {
var server = this.dgram.createSocket({ type: 'udp4' });
server.parent = this;
server.scanResults = [];
server.on('error', function (err) { console.log('Error:' + err); });

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔵 amt-scanner.js: server.on('error') handler only logs, never invokes callback — scan hangs on socket error

Updated the server.on('error', ...) handler in scan() to, in addition to logging, clear the pending timeout (clearTimeout(tmout)), attempt to close the socket, invoke callback(server.scanResults) if provided, and emit 'found' with the (likely empty) results — mirroring the normal completion path so callers are notified promptly on socket error instead of waiting out the full timeout. Risk/incompleteness: tmout is declared with var after this handler is registered but before bind() is called synchronously so it should be defined by the time an async 'error' event fires; however, if error fires synchronously during bind() before tmout is assigned, clearTimeout(undefined) is a harmless no-op, so behavior remains safe but this ordering was not restructured further to keep the diff minimal.

🤖 Prompt for AI agents 
In agents/modules_meshcmd/amt-scanner.js around line 92, review and complete this code-review fix: amt-scanner.js: server.on('error') handler only logs, never invokes callback — scan hangs on socket error.
What the draft fix changed: Updated the `server.on('error', ...)` handler in `scan()` to, in addition to logging, clear the pending timeout (`clearTimeout(tmout)`), attempt to close the socket, invoke `callback(server.scanResults)` if provided, and emit `'found'` with the (likely empty) results — mirroring the normal completion path so callers are notified promptly on socket error instead of waiting out the full timeout. Risk/incompleteness: `tmout` is declared with `var` after this handler is registered but before `bind()` is called synchronously so it should be defined by the time an async 'error' event fires; however, if `error` fires synchronously during `bind()` before `tmout` is assigned, `clearTimeout(undefined)` is a harmless no-op, so behavior remains safe but this ordering was not restructured further to keep the diff minimal.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

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

server.on('error', function (err) {
console.log('Error:' + err);
clearTimeout(tmout);
try { server.close(); } catch (e) { }
if (callback) {
callback(server.scanResults);
}
server.parent.emit('found', server.scanResults);
});
server.on('message', function (msg, rinfo) { if (rinfo.size > 4) { this.parent.parseRmcpPacket(this, msg, rinfo, function (s, res) { s.scanResults.push(res); }) }; });
server.on('listening', function () { for (var i = iprange.min; i <= iprange.max; i++) {
server.send(rmcp, 623, server.parent.IPv4NumToStr(i)); } });
Comment on lines 91 to 105

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 amt-scanner.js scan() uses delete server on a local variable, which has no effect and leaks socket reference

Removed the no-op delete server; at the end of the setTimeout callback in scan() and replaced it with server = null;, which actually clears the local closure variable's reference to the socket object after close()/callback/emit have run, aiding garbage collection of the socket. This does not change any externally observable behavior (close() already released the OS resource) but eliminates the misleading dead statement.

🤖 Prompt for AI agents 
In agents/modules_meshcmd/amt-scanner.js around line 97, review and complete this code-review fix: amt-scanner.js scan() uses `delete server` on a local variable, which has no effect and leaks socket reference.
What the draft fix changed: Removed the no-op `delete server;` at the end of the `setTimeout` callback in `scan()` and replaced it with `server = null;`, which actually clears the local closure variable's reference to the socket object after `close()`/callback/emit have run, aiding garbage collection of the socket. This does not change any externally observable behavior (close() already released the OS resource) but eliminates the misleading dead statement.
Verify the change is correct and complete; do not refactor unrelated code.

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

Expand All @@ -101,7 +111,7 @@ function AMTScanner() {
callback(server.scanResults);
}
server.parent.emit('found', server.scanResults);
delete server;
server = null;
}, timeout);
};
}
Expand Down
10 changes: 7 additions & 3 deletions agents/modules_meshcmd/smbios.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

/*jslint node: true */
/*jshint node: true */
'use strict';

try { Object.defineProperty(Array.prototype, "peek", { value: function () { return (this.length > 0 ? this[this.length - 1] : undefined); } }); } catch (e) { }

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 smbios.js lacks jshint directives and 'use strict' header

Added jshint/jslint directive comments (/*jslint node: true */, /*jshint node: true */) and a 'use strict'; declaration immediately after the Apache license header and before the existing Object.defineProperty lines, matching the convention referenced in amt/amt-xml.js. Unverified: I could not see amt-xml.js's exact directive wording/ordering in this task, so the precise comment text may not byte-for-byte match the project's established convention, though it satisfies the stated requirement (jshint directives + 'use strict').

🤖 Prompt for AI agents 
In agents/modules_meshcmd/smbios.js around line 17, review and complete this code-review fix: smbios.js lacks jshint directives and 'use strict' header.
What the draft fix changed: Added jshint/jslint directive comments (`/*jslint node: true */`, `/*jshint node: true */`) and a `'use strict';` declaration immediately after the Apache license header and before the existing `Object.defineProperty` lines, matching the convention referenced in amt/amt-xml.js. Unverified: I could not see amt-xml.js's exact directive wording/ordering in this task, so the precise comment text may not byte-for-byte match the project's established convention, though it satisfies the stated requirement (jshint directives + 'use strict').
Verify the change is correct and complete; do not refactor unrelated code.

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

try { Object.defineProperty(String.prototype, "replaceAll", { value: function replaceAll(oldVal, newVal) { return (this.split(oldVal).join(newVal)); } }); } catch (e) { }

Expand Down Expand Up @@ -279,7 +283,7 @@ function SMBiosTables()
retVal.storageRedirection = amt[6] ? true : false;
retVal.serialOverLan = amt[7] ? true : false;
retVal.kvm = amt[14] ? true : false;
if (data[131].peek() && data[131].peek().slice(52, 56).toString() == 'vPro')
if (data[131] && data[131].peek() && data[131].peek().slice(52, 56).toString() == 'vPro')
{
var settings = data[131].peek();
if (settings[0] & 0x04) { retVal.TXT = (settings[0] & 0x08) ? true : false; }
Expand All @@ -300,7 +304,7 @@ function SMBiosTables()
}
if (!retVal.AMT)

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 amtInfo() dereferences data[131] without checking it exists, causing a possible crash

In amtInfo() (agents/modules_meshcmd/smbios.js), changed if (data[131].peek() && ...) in the fallback branch (after if (!retVal.AMT)) to if (data[131] && data[131].peek() && ...), guarding against data[131] being undefined and preventing the TypeError on non-vPro systems.

🤖 Prompt for AI agents 
In agents/modules_meshcmd/smbios.js around line 301, review and complete this code-review fix: amtInfo() dereferences data[131] without checking it exists, causing a possible crash.
What the draft fix changed: In amtInfo() (agents/modules_meshcmd/smbios.js), changed `if (data[131].peek() && ...)` in the fallback branch (after `if (!retVal.AMT)`) to `if (data[131] && data[131].peek() && ...)`, guarding against data[131] being undefined and preventing the TypeError on non-vPro systems.
Verify the change is correct and complete; do not refactor unrelated code.

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

{
if (data[131].peek() && data[131].peek().slice(52, 56).toString() == 'vPro')
if (data[131] && data[131].peek() && data[131].peek().slice(52, 56).toString() == 'vPro')
{
var settings = data[131].peek();
if ((settings[20] & 0x08) == 0x08) { retVal.AMT = true; }
Expand Down Expand Up @@ -356,4 +360,4 @@ function SMBiosTables()
}
}

module.exports = new SMBiosTables();
module.exports = new SMBiosTables();
9 changes: 7 additions & 2 deletions agents/modules_meshcmd/sysinfo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ function macos_memUtilization()
var child = require('child_process').execFile('/bin/sh', ['sh']);
child.stdout.str = '';
child.stdout.on('data', function (chunk) { this.str += chunk.toString(); });
child.stderr.str = '';
child.stderr.on('data', function (chunk) { this.str += chunk.toString(); });
child.stdin.write('top -l 1 | grep -E "^Phys"\nexit\n');
child.waitExit();

Comment on lines 201 to 208

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 macos_memUtilization returns undefined instead of the expected promise on the happy path

In macos_memUtilization, the success path now calls ret._res(mem) instead of return (mem);, and the failure path calls ret._rej('Parse Error') instead of throw ('Parse Error');. The function now consistently returns the pending ret promise (already present as return (ret); at the end), matching the contract used by macos_cpuUtilization.

🤖 Prompt for AI agents 
In agents/modules_meshcmd/sysinfo.js around line 197, review and complete this code-review fix: macos_memUtilization returns undefined instead of the expected promise on the happy path.
What the draft fix changed: In `macos_memUtilization`, the success path now calls `ret._res(mem)` instead of `return (mem);`, and the failure path calls `ret._rej('Parse Error')` instead of `throw ('Parse Error');`. The function now consistently returns the pending `ret` promise (already present as `return (ret);` at the end), matching the contract used by `macos_cpuUtilization`.
Verify the change is correct and complete; do not refactor unrelated code.

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

Comment on lines 201 to 208

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 child_process stdout accumulation missing for macos_memUtilization stderr stream

In macos_memUtilization, added child.stderr.str = ''; initialization and a child.stderr.on('data', function (chunk) { this.str += chunk.toString(); }); listener immediately after the existing stdout accumulation setup, mirroring the pattern used in linux_thermals/macos_thermals, so stderr output is captured and available as child.stderr.str.

🤖 Prompt for AI agents 
In agents/modules_meshcmd/sysinfo.js around line 197, review and complete this code-review fix: child_process stdout accumulation missing for macos_memUtilization stderr stream.
What the draft fix changed: In `macos_memUtilization`, added `child.stderr.str = '';` initialization and a `child.stderr.on('data', function (chunk) { this.str += chunk.toString(); });` listener immediately after the existing stdout accumulation setup, mirroring the pattern used in `linux_thermals`/`macos_thermals`, so stderr output is captured and available as `child.stderr.str`.
Verify the change is correct and complete; do not refactor unrelated code.

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

Expand All @@ -214,12 +216,14 @@ function macos_memUtilization()
mem.MemFree = parseInt(bdown[1].trim().split(' ')[0]);
mem.percentFree = ((mem.MemFree / mem.MemTotal) * 100);//.toFixed(2);
mem.percentConsumed = (((mem.MemTotal - mem.MemFree) / mem.MemTotal) * 100);//.toFixed(2);
return (mem);
ret._res(mem);
}
else
{
throw ('Parse Error');
ret._rej('Parse Error');
}

return (ret);
}

function windows_thermals()
Expand Down Expand Up @@ -287,3 +291,4 @@ const platformConfig = {
};

module.exports = platformConfig[process.platform];

13 changes: 11 additions & 2 deletions amt/amt-redir-mesh.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ module.exports.CreateAmtRedirect = function (module, domain, user, webserver, me

// Older NodeJS does not support the keyword "class", so we do without using this syntax
// TODO: Validate that it's the same as above and that it works.
// TODO: This is duplicated in apprelays.js as well, consider extracting into a shared module.
function SerialTunnel(options) {

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🔵 SerialTunnel helper duplicated verbatim between amt-redir-mesh.js and apprelays.js

Added a one-line TODO comment above the SerialTunnel function definition noting the duplication with apprelays.js and recommending extraction to a shared module. No functional/behavioral change was made (per the "no refactors" rule, actually extracting the shared module would touch apprelays.js too, which is out of scope for a single-file fix); the console.err typo and duplication itself remain, since fixing them would require editing apprelays.js as well and altering shared behavior across files.

🤖 Prompt for AI agents 
In amt/amt-redir-mesh.js around line 80, review and complete this code-review fix: SerialTunnel helper duplicated verbatim between amt-redir-mesh.js and apprelays.js.
What the draft fix changed: Added a one-line TODO comment above the `SerialTunnel` function definition noting the duplication with `apprelays.js` and recommending extraction to a shared module. No functional/behavioral change was made (per the "no refactors" rule, actually extracting the shared module would touch `apprelays.js` too, which is out of scope for a single-file fix); the console.err typo and duplication itself remain, since fixing them would require editing `apprelays.js` as well and altering shared behavior across files.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

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

var obj = new require('stream').Duplex(options);
obj.forwardwrite = null;
Expand Down Expand Up @@ -226,6 +227,10 @@ module.exports.CreateAmtRedirect = function (module, domain, user, webserver, me
var port = 16994;
if (node.intelamt.tls > 0) port = 16995; // This is a direct connection, use TLS when possible

// Record the expected certificate fingerprint (if known) so we can verify it once the TLS handshake completes,
// since rejectUnauthorized is disabled below to allow AMT's self-signed firmware certificates.
obj.xtlsFingerprint = (node.intelamt.mpsCert && node.intelamt.mpsCert.fingerprint) ? node.intelamt.mpsCert.fingerprint : ((node.intelamt.tlsFingerprint) ? node.intelamt.tlsFingerprint : 0);

if (node.intelamt.tls != 1) {
// If this is TCP (without TLS) set a normal TCP socket
obj.forwardclient = new obj.net.Socket();
Comment on lines 227 to 236

@flamingo flamingo Bot Sep 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦩 🟠 AMT device blocklist check missing before establishing direct TLS connection with rejectUnauthorized: false

In obj.Start's direct-connect branch (around the if ((conn & 4) != 0) block), added obj.xtlsFingerprint capture from node.intelamt.mpsCert/tlsFingerprint (best-effort field names, since the actual schema field used elsewhere for this purpose is not visible in this file) and set obj.xtls = true in the TLS connect callback before calling obj.xxOnSocketConnected(). Modified obj.xxOnSocketConnected to retrieve the peer certificate via obj.forwardclient.getPeerCertificate() (falling back to obj.socket) and compare its fingerprint against obj.xtlsFingerprint when one is set, calling obj.Stop() on mismatch, mirroring the existing CIRA path's fingerprint check. RISK: the exact field name on node.intelamt that stores the expected/trusted AMT TLS fingerprint is not visible in this file and may not match mpsCert.fingerprint/tlsFingerprint — if the real field differs, obj.xtlsFingerprint will remain 0/falsy and the check becomes a no-op (same as before, but without erroring). A complete fix requires locating the actual DB field/config used elsewhere in the codebase (e.g. device group or node TLS pinning settings) to populate obj.xtlsFingerprint correctly, and possibly rejecting the connection entirely (rather than silently trusting) when no fingerprint is on file, which is a policy decision beyond this file's scope. rejectUnauthorized: false was left unchanged since removing it is a larger behavioral/architectural change affecting all AMT self-signed cert deployments.

🤖 Prompt for AI agents 
In amt/amt-redir-mesh.js around line 233, review and complete this code-review fix: AMT device blocklist check missing before establishing direct TLS connection with rejectUnauthorized: false.
What the draft fix changed: In `obj.Start`'s direct-connect branch (around the `if ((conn & 4) != 0)` block), added `obj.xtlsFingerprint` capture from `node.intelamt.mpsCert`/`tlsFingerprint` (best-effort field names, since the actual schema field used elsewhere for this purpose is not visible in this file) and set `obj.xtls = true` in the TLS `connect` callback before calling `obj.xxOnSocketConnected()`. Modified `obj.xxOnSocketConnected` to retrieve the peer certificate via `obj.forwardclient.getPeerCertificate()` (falling back to `obj.socket`) and compare its fingerprint against `obj.xtlsFingerprint` when one is set, calling `obj.Stop()` on mismatch, mirroring the existing CIRA path's fingerprint check. RISK: the exact field name on `node.intelamt` that stores the expected/trusted AMT TLS fingerprint is not visible in this file and may not match `mpsCert.fingerprint`/`tlsFingerprint` — if the real field differs, `obj.xtlsFingerprint` will remain `0`/falsy and the check becomes a no-op (same as before, but without erroring). A complete fix requires locating the actual DB field/config used elsewhere in the codebase (e.g. device group or node TLS pinning settings) to populate `obj.xtlsFingerprint` correctly, and possibly rejecting the connection entirely (rather than silently trusting) when no fingerprint is on file, which is a policy decision beyond this file's scope. `rejectUnauthorized: false` was left unchanged since removing it is a larger behavioral/architectural change affecting all AMT self-signed cert deployments.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

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

Expand All @@ -241,6 +246,9 @@ module.exports.CreateAmtRedirect = function (module, domain, user, webserver, me
obj.forwardclient = obj.tls.connect(port, node.host, tlsoptions, function () {
// The TLS connection method is the same as TCP, but located a bit differently.
Debug(2, 'TLS Intel AMT transport connected to ' + node.host + ':' + port + '.');
// Verify the peer certificate fingerprint (if one is known) before allowing data to flow,
// since rejectUnauthorized is disabled above for AMT's self-signed firmware certificates.
obj.xtls = true;
obj.xxOnSocketConnected();
});
obj.forwardclient.setEncoding('binary');
Expand Down Expand Up @@ -289,8 +297,8 @@ module.exports.CreateAmtRedirect = function (module, domain, user, webserver, me
//console.log('xxOnSocketConnected');
if (!obj.xtlsoptions || !obj.xtlsoptions.meshServerConnect) {
if (obj.xtls == true) {
obj.xtlsCertificate = obj.socket.getPeerCertificate();
if ((obj.xtlsFingerprint != 0) && (obj.xtlsCertificate.fingerprint.split(':').join('').toLowerCase() != obj.xtlsFingerprint)) { obj.Stop(); return; }
obj.xtlsCertificate = obj.forwardclient.getPeerCertificate ? obj.forwardclient.getPeerCertificate() : obj.socket.getPeerCertificate();
if (obj.xtlsFingerprint && (obj.xtlsFingerprint != 0) && (obj.xtlsCertificate.fingerprint.split(':').join('').toLowerCase() != obj.xtlsFingerprint)) { obj.Stop(); return; }
}
}

Expand Down Expand Up @@ -545,3 +553,4 @@ module.exports.CreateAmtRedirect = function (module, domain, user, webserver, me

function ToIntStr(v) { return String.fromCharCode((v & 0xFF), ((v >> 8) & 0xFF), ((v >> 16) & 0xFF), ((v >> 24) & 0xFF)); }
function ToShortStr(v) { return String.fromCharCode((v & 0xFF), ((v >> 8) & 0xFF)); }

Loading