Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@ reviews:
enable_prompt_for_ai_agents: true
path_filters:
- '!src/renderer/locales/**'
- '!**/package-lock.json'
- '!**/yarn.lock'
- '!**/pnpm-lock.yaml'
- '!flatpak/generated-sources.json'
- '!patches/**'
- '!reticulum-sidecar/patches/**'
- '!**/coverage/**'
- '!**/dist/**'
- '!**/dist-electron/**'
- '!**/generated/**'
- '!**/target/**'
path_instructions:
- path: '**/*'
Expand All @@ -38,6 +41,9 @@ reviews:
enabled: true
drafts: false
auto_pause_after_reviewed_commits: 2
exclude_user_handles:
- 'dependabot[bot]'
- 'renovate[bot]'
ignore_title_keywords:
- 'chore: bump'
- 'chore(deps)'
Expand Down
66 changes: 66 additions & 0 deletions src/main/host-link-rtt.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// @vitest-environment node
import net from 'net';
import { afterEach, describe, expect, it, vi } from 'vitest';

import { probeHttpRttMs, probeTcpRttMs } from './host-link-rtt';

describe('probeTcpRttMs', () => {
let server: net.Server | null = null;

afterEach(async () => {
await new Promise<void>((resolve) => {
if (!server) {
resolve();
return;
}
server.close(() => {
resolve();
});
server = null;
});
});

it('returns a finite RTT when the host accepts a TCP connect', async () => {
server = net.createServer((socket) => {
socket.destroy();
});
await new Promise<void>((resolve) => {
server!.listen(0, '127.0.0.1', () => {
resolve();
});
});
const addr = server.address();
if (!addr || typeof addr === 'string') throw new Error('expected TCP port');
const rtt = await probeTcpRttMs('127.0.0.1', addr.port);
expect(rtt).not.toBeNull();
expect(rtt!).toBeGreaterThanOrEqual(0);
expect(rtt!).toBeLessThan(3000);
});

it('returns null when the port is closed', async () => {
const rtt = await probeTcpRttMs('127.0.0.1', 1);
expect(rtt).toBeNull();
});
});

describe('probeHttpRttMs', () => {
it('returns null when fetch fails', async () => {
vi.stubGlobal(
'fetch',
vi.fn(() => Promise.reject(new Error('network down'))),
);
await expect(probeHttpRttMs('127.0.0.1', false)).resolves.toBeNull();
vi.unstubAllGlobals();
});

it('returns RTT when the host answers', async () => {
vi.stubGlobal(
'fetch',
vi.fn(() => Promise.resolve({ ok: true, status: 200 } as Response)),
);
const rtt = await probeHttpRttMs('example.test', false);
expect(rtt).not.toBeNull();
expect(rtt!).toBeGreaterThanOrEqual(0);
vi.unstubAllGlobals();
});
});
83 changes: 83 additions & 0 deletions src/main/host-link-rtt.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import net from 'net';

import { formatHostForSocket } from '../shared/connectHost';
import { MS_PER_SECOND } from '../shared/timeConstants';
import { sanitizeLogMessage } from './log-service';

/** Align with renderer `HOST_LINK_RTT_PROBE_TIMEOUT_MS`. */
export const HOST_LINK_RTT_PROBE_TIMEOUT_MS = 3 * MS_PER_SECOND;

/**
* Time a Meshtastic HTTP `/json/report` GET. Returns RTT ms, or null on failure.
* Does not throw — callers treat null as "no bars".
*/
export async function probeHttpRttMs(host: string, tls: boolean): Promise<number | null> {
const protocol = tls ? 'https' : 'http';
const url = `${protocol}://${host}/json/report`;
const started = Date.now();
try {
const res = await fetch(url, { signal: AbortSignal.timeout(HOST_LINK_RTT_PROBE_TIMEOUT_MS) });
const rtt = Date.now() - started;
if (!res.ok) {
console.debug(
`[hostLink] HTTP probe non-OK ${res.status} for ${sanitizeLogMessage(host)} rtt=${rtt}ms`,
);
// Still usable as latency if the host answered quickly.
return rtt;
}
return rtt;
} catch (err) {
console.debug(
`[hostLink] HTTP probe failed for ${sanitizeLogMessage(host)}:`,
sanitizeLogMessage(err instanceof Error ? err.message : String(err)),
);
return null;
}
}

/**
* Time a TCP connect to host:port, then destroy the socket immediately.
* Measures LAN reachability latency without attaching a protocol session.
*/
export function probeTcpRttMs(host: string, port: number): Promise<number | null> {
return new Promise((resolve) => {
let settled = false;
const socketHost = formatHostForSocket(host);
const socket = new net.Socket();
const started = Date.now();
const finish = (rtt: number | null) => {
if (settled) return;
settled = true;
clearTimeout(timer);
try {
socket.removeAllListeners();
socket.destroy();
} catch {
// catch-no-log-ok probe socket cleanup
}
resolve(rtt);
};
const timer = setTimeout(() => {
finish(null);
}, HOST_LINK_RTT_PROBE_TIMEOUT_MS);
socket.once('connect', () => {
finish(Date.now() - started);
});
socket.once('error', (err) => {
console.debug(
`[hostLink] TCP probe failed for ${sanitizeLogMessage(socketHost)}:${port}:`,
sanitizeLogMessage(err.message),
);
finish(null);
});
try {
socket.connect(port, socketHost);
} catch (err) {
console.debug(
`[hostLink] TCP probe connect threw for ${sanitizeLogMessage(socketHost)}:${port}:`,
sanitizeLogMessage(err instanceof Error ? err.message : String(err)),
);
finish(null);
}
});
}
21 changes: 19 additions & 2 deletions src/main/index.contract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { join } from 'path';
import { describe, expect, it } from 'vitest';

const INDEX_SOURCE = readFileSync(join(__dirname, 'index.ts'), 'utf-8');
const PRELOAD_SOURCE = readFileSync(join(__dirname, '../preload/index.ts'), 'utf-8');

describe('IPC payload size limits (source contract)', () => {
it('defines meshcore tcp-write, http:write, and noble-ble limits and uses them in handlers', () => {
Expand Down Expand Up @@ -292,8 +293,6 @@ describe('Reticulum sidecar IPC handlers (source contract)', () => {
join(__dirname, 'ipc/reticulum-db-handlers.ts'),
'utf8',
);
const PRELOAD_SOURCE = readFileSync(join(__dirname, '../preload/index.ts'), 'utf8');

it('registers reticulum lifecycle and proxy handlers', () => {
expect(INDEX_SOURCE).toContain('registerReticulumIpcHandlers');
expect(RETICULUM_HANDLERS_SOURCE).toContain("ipcMain.handle('reticulum:start'");
Expand Down Expand Up @@ -362,6 +361,24 @@ describe('HTTP bridge IPC handlers (source contract)', () => {
});
});

describe('Host link quality IPC (source contract)', () => {
it('forwards Noble link RSSI and registers HTTP/TCP RTT probes', () => {
expect(INDEX_SOURCE).toContain("webContents.send('noble-ble-link-rssi'");
expect(INDEX_SOURCE).toContain("ipcMain.handle('hostLink:probeHttpRtt'");
expect(INDEX_SOURCE).toContain("ipcMain.handle('hostLink:probeTcpRtt'");
});
});

describe('Host link quality preload surface (source contract)', () => {
it('exposes onNobleBleLinkRssi and hostLink probe APIs', () => {
expect(PRELOAD_SOURCE).toContain('onNobleBleLinkRssi:');
expect(PRELOAD_SOURCE).toContain("ipcRenderer.on('noble-ble-link-rssi'");
expect(PRELOAD_SOURCE).toContain('hostLink:');
expect(PRELOAD_SOURCE).toContain("ipcRenderer.invoke('hostLink:probeHttpRtt'");
expect(PRELOAD_SOURCE).toContain("ipcRenderer.invoke('hostLink:probeTcpRtt'");
});
});

describe('Native crash observability (source contract)', () => {
it('starts crashReporter without upload and logs child-process-gone', () => {
expect(INDEX_SOURCE).toContain(
Expand Down
25 changes: 25 additions & 0 deletions src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ import { finishDbIpcHandler, finishDbIpcReadHandler, getDbForIpc } from './db-ip
import { formatDatabaseSchemaTooNewMessage, showFatalStartupError } from './fatal-startup-dialog';
import { fetchLinkPreview } from './fetchLinkPreview';
import { formatGpxTracks, GPX_EXPORT_MAX_POINTS } from './gpxExportFormat';
import { probeHttpRttMs, probeTcpRttMs } from './host-link-rtt';
import { isValidHttpHostname } from './httpHostValidation';
import { registerGpsIpcHandlers } from './ipc/gps-handlers';
import { registerReticulumDbIpcHandlers } from './ipc/reticulum-db-handlers';
Expand Down Expand Up @@ -2609,6 +2610,12 @@ nobleBleManager.on(
mainWindow?.webContents.send('noble-ble-device-discovered', device);
},
);
nobleBleManager.on(
'linkRssi',
({ sessionId, rssi }: { sessionId: NobleSessionId; rssi: number | null }) => {
mainWindow?.webContents.send('noble-ble-link-rssi', { sessionId, rssi });
},
);
nobleBleManager.on('connected', ({ sessionId }: { sessionId: NobleSessionId }) => {
mainWindow?.webContents.send('noble-ble-connected', { sessionId });
});
Expand Down Expand Up @@ -6339,6 +6346,24 @@ ipcMain.handle('http:preflight', async (event, host: unknown, tls: unknown) => {
await httpPreflight(host, tls);
});

ipcMain.handle('hostLink:probeHttpRtt', async (event, host: unknown, tls: unknown) => {
assertIpcSender(event, 'hostLink:probeHttpRtt');
validateHttpHost(host);
if (typeof tls !== 'boolean') {
throw new Error('Invalid tls');
}
return probeHttpRttMs(host, tls);
});

ipcMain.handle('hostLink:probeTcpRtt', async (event, host: unknown, port: unknown) => {
assertIpcSender(event, 'hostLink:probeTcpRtt');
validateHttpHost(host);
if (!Number.isInteger(port) || (port as number) < 1 || (port as number) > 65535) {
throw new Error('Invalid port');
}
return probeTcpRttMs(host, port as number);
});
Comment on lines +6358 to +6365

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Locate clampTcpPort() and confirm renderer callers of hostLink.probeTcpRtt pass a bare host.
rg -n -C3 'function clampTcpPort|export.*clampTcpPort' --type=ts
rg -n -C5 'probeTcpRtt\(' src/renderer

Repository: Colorado-Mesh/mesh-client

Length of output: 163


🏁 Script executed:

#!/bin/bash
set -u

echo "== files =="
git ls-files | sed -n '1,120p'

echo "== clampTcpPort candidates =="
rg -n "clampTcpPort" . || true

echo "== hostLink probe handlers and relevant helpers =="
rg -n -C4 "probeTcpRtt|probeHttpRtt|validateHttpHost|hostLink:probeTcpRtt|hostLink:probeHttpRtt|clampTcpPort" src/main src/shared src/renderer || true

echo "== renderer probeTcpRtt call sites =="
rg -n -C5 "\.probeTcpRtt|probeTcpRtt\(" . || true

Repository: Colorado-Mesh/mesh-client

Length of output: 50381


Use clampTcpPort() for the TCP probe port validation.

clampTcpPort() is the shared TCP-port helper in src/shared/tcpPort.ts; replace the inline Number.isInteger(port) || port < 1 || port > 65535 check with it to satisfy the shared-validation guideline.

🧰 Tools
🪛 ast-grep (0.45.0)

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from 'child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/index.ts` around lines 6358 - 6365, The hostLink:probeTcpRtt handler
currently duplicates TCP port validation inline. Replace that condition with the
shared clampTcpPort() helper, preserving rejection of invalid inputs and passing
the clamped valid port to probeTcpRttMs.

Source: Coding guidelines


ipcMain.handle('http:connect', async (event, host: unknown, tls: unknown) => {
if (!validateIpcSender(event)) throw new Error('http:connect: unauthorized sender');
validateHttpHost(host);
Expand Down
20 changes: 20 additions & 0 deletions src/main/noble-ble-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,26 @@ describe('NobleBleManager.connect — macOS wake zombie peripheral (regression)'
* MeshCore uses notify-only (like Web Bluetooth); GATT read on NUS TX fails on Windows WinRT.
* Meshtastic keeps a non-Darwin read-pump safety net when notify is active.
*/
describe('NobleBleManager — connected link RSSI polling (regression)', () => {
it('declares link RSSI poll timer fields and start/stop helpers', () => {
expect(SOURCE).toContain('linkRssiPollTimer: ReturnType<typeof setInterval> | null');
expect(SOURCE).toContain('NOBLE_LINK_RSSI_POLL_MS');
expect(SOURCE).toContain('startLinkRssiPolling');
expect(SOURCE).toContain('stopLinkRssiPolling');
expect(SOURCE).toContain("emit('linkRssi'");
expect(SOURCE).toContain('updateRssiAsync');
});

it('starts link RSSI polling after successful connect and stops in clearSessionState', () => {
expect(SOURCE).toMatch(
/this\.startLinkRssiPolling\(sessionId, session, peripheral, connectRssi\)/,
);
const clearMatch = /private clearSessionState\([\s\S]+?\n {2}\}/.exec(SOURCE);
expect(clearMatch).not.toBeNull();
expect(clearMatch![0]).toContain('stopLinkRssiPolling');
});
});

describe('NobleBleManager — notify-first fromRadio read pump strategy (regression)', () => {
it('declares fromRadioNotifyOnly in session state and initialises it to false', () => {
expect(SOURCE).toContain('fromRadioNotifyOnly: boolean');
Expand Down
Loading