Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
42 changes: 36 additions & 6 deletions electron/bridges/systemManager/execOnSession.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@ const { isSshConnAlive, isTransportExecError } = require("./execConnHealth.cjs")

function createExecOnSessionApi(ctx) {
with (ctx) {
const DEFAULT_LOCAL_EXEC_MAX_BUFFER = 10 * 1024 * 1024;

function normalizeExecMaxBuffer(value, fallback = DEFAULT_LOCAL_EXEC_MAX_BUFFER) {
const numeric = Number(value);
return Number.isFinite(numeric) && numeric > 0 ? Math.floor(numeric) : fallback;
}

function isExecMaxBufferError(err) {
const code = String(err?.code || "");
const message = String(err?.message || "");
return code === "ERR_CHILD_PROCESS_STDIO_MAXBUFFER" || /maxBuffer/i.test(message);
}

/** Serialize remote exec per session to avoid SSH channel storms. */
const execQueues = new Map();

Expand Down Expand Up @@ -82,6 +95,7 @@ function createExecOnSessionApi(ctx) {
return new Promise((resolve) => {
let settled = false;
let activeStream = null;
const maxBuffer = normalizeExecMaxBuffer(execOptions.maxBuffer);
const settle = (result) => {
if (settled) return;
settled = true;
Expand All @@ -102,9 +116,24 @@ function createExecOnSessionApi(ctx) {
activeStream = stream;
let stdout = "";
let stderr = "";
stream.on("data", (chunk) => { stdout += chunk.toString(); });
const appendOutput = (streamName, current, chunk) => {
const next = current + chunk.toString();
if (next.length > maxBuffer) {
settle({
success: false,
error: `${streamName} maxBuffer exceeded`,
stdout: "",
stderr: "",
code: 1,
});
try { stream.close(); } catch { /* ignore */ }
return current;
}
return next;
};
stream.on("data", (chunk) => { stdout = appendOutput("stdout", stdout, chunk); });
if (stream.stderr) {
stream.stderr.on("data", (chunk) => { stderr += chunk.toString(); });
stream.stderr.on("data", (chunk) => { stderr = appendOutput("stderr", stderr, chunk); });
}
if (typeof execOptions.stdin === "string") {
stream.write(execOptions.stdin);
Expand All @@ -129,6 +158,7 @@ function createExecOnSessionApi(ctx) {
requireTrustedHost: true,
knownHosts: session.etStatsAuth?.knownHosts,
stdin: execOptions.stdin,
maxBuffer: execOptions.maxBuffer,
});
}

Expand Down Expand Up @@ -162,9 +192,9 @@ function createExecOnSessionApi(ctx) {
const child = execFile(
"powershell.exe",
["-NoProfile", "-NonInteractive", "-Command", command],
{ timeout: timeoutMs, maxBuffer: 10 * 1024 * 1024 },
{ timeout: timeoutMs, maxBuffer: normalizeExecMaxBuffer(execOptions.maxBuffer) },
(err, stdout, stderr) => {
if (err && !stdout) {
if (err && (isExecMaxBufferError(err) || !stdout)) {
resolve({ success: false, error: err.message || String(err), stdout: "", stderr: String(stderr || "") });
return;
}
Expand All @@ -181,9 +211,9 @@ function createExecOnSessionApi(ctx) {
const child = execFile(
"sh",
["-c", command],
{ timeout: timeoutMs, maxBuffer: 10 * 1024 * 1024 },
{ timeout: timeoutMs, maxBuffer: normalizeExecMaxBuffer(execOptions.maxBuffer) },
(err, stdout, stderr) => {
if (err && !stdout) {
if (err && (isExecMaxBufferError(err) || !stdout)) {
resolve({ success: false, error: err.message || String(err), stdout: "", stderr: String(stderr || "") });
return;
}
Expand Down
45 changes: 45 additions & 0 deletions electron/bridges/systemManager/execOnSession.stdin.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,48 @@ test("execOnSession closes ssh exec stdin after writing provided input", async (
assert.deepEqual(writes, ["secret\n"]);
assert.equal(ended, true);
});

test("execOnSession reports local maxBuffer errors instead of returning truncated stdout", async () => {
const execApi = createExecOnSessionApi({
sessions: { get: () => ({ type: "local", protocol: "local" }) },
process,
});

const result = await execApi.execOnSession(null, "local", "yes x | head -c 2048", 1000, {
maxBuffer: 128,
});

assert.equal(result.success, false);
assert.match(result.error, /maxBuffer|stdout maxBuffer/i);
});

test("execOnSession enforces maxBuffer for SSH streamed stdout", async () => {
let closed = false;
const stream = new EventEmitter();
stream.stderr = new EventEmitter();
stream.close = () => {
closed = true;
};

const conn = {
exec(_command, callback) {
callback(null, stream);
process.nextTick(() => {
stream.emit("data", Buffer.from("x".repeat(256)));
stream.emit("close", 0);
});
},
};
const execApi = createExecOnSessionApi({
sessions: { get: () => ({ conn, type: "ssh" }) },
});

const result = await execApi.execOnSession(null, "s1", "ps", 1000, {
maxBuffer: 128,
});

assert.equal(result.success, false);
assert.match(result.error, /maxBuffer/i);
assert.equal(result.stdout, "");
assert.equal(closed, true);
});
13 changes: 6 additions & 7 deletions electron/bridges/systemManagerBridge.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@ const CAPABILITY_SCRIPT_POSIX = [
const PROCESS_LIST_SCRIPT_POSIX = [
"exec sh -c ",
"'",
// Safety cap: head -n 2000 prevents maxBuffer/timeout on process-dense hosts.
// This is NOT a functional limit — monitored processes still show accurate metrics.
"ps -eo pid= -o ppid= -o user= -o stat= -o pcpu= -o pmem= -o rss= -o vsz= -o etime= -o args= 2>/dev/null | head -n 2000",
"ps -eo pid= -o ppid= -o user= -o stat= -o pcpu= -o pmem= -o rss= -o vsz= -o etime= -o args= 2>/dev/null",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Stream process output instead of relying on maxBuffer

When this runs for a local Linux/macOS session with enough processes or long command lines, removing the row cap lets ps output exceed execOnLocalMachine's 10 MiB execFile maxBuffer in electron/bridges/systemManager/execOnSession.cjs; that path resolves as success if any stdout was captured, so parseProcessLines can silently show only the truncated prefix rather than all processes. ET sessions have an even smaller default execFile buffer, so this change can also turn large process lists into load failures. The cap removal should be paired with streaming/parsing or an explicit truncation/error path so dense hosts are not still capped by the transport buffer.

Useful? React with 👍 / 👎.

"'",
].join("");
const PROCESS_LIST_MAX_BUFFER = 64 * 1024 * 1024;

function parseCapabilities(stdout, isLocal, localPlatform) {
const text = stdout || "";
Expand Down Expand Up @@ -166,10 +165,8 @@ function createSystemManagerBridge(deps) {
if (!sessionId) return { success: false, error: "Missing sessionId" };

if (isLocalSession(sessionId) && process.platform === "win32") {
// Safety cap: -First 2000 prevents maxBuffer/timeout on process-dense hosts.
// This is NOT a functional limit — monitored processes still show accurate metrics.
const result = await execOnLocalMachine(
"Get-CimInstance Win32_Process | Sort-Object KernelModeTime -Descending | Select-Object -First 2000 ProcessId,ParentProcessId,Name,WorkingSetSize | ConvertTo-Json -Compress",
"Get-CimInstance Win32_Process | Sort-Object KernelModeTime -Descending | Select-Object ProcessId,ParentProcessId,Name,WorkingSetSize | ConvertTo-Json -Compress",
10000,
);
Comment on lines 168 to 172

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Pass the larger buffer to Windows process listing

On local Windows sessions this command now removes the -First 2000 cap, but the execOnLocalMachine call still omits maxBuffer, so it falls back to the 10 MiB default in execOnSession.cjs. On process-dense Windows hosts where the full JSON exceeds that default, listProcesses will now fail with a maxBuffer error instead of showing the uncapped list; this path should use the same larger process-list buffer as the POSIX path.

Useful? React with 👍 / 👎.

if (!result.success) return { success: false, error: result.error };
Expand All @@ -194,7 +191,9 @@ function createSystemManagerBridge(deps) {
}
}

const result = await execOnSession(event, sessionId, PROCESS_LIST_SCRIPT_POSIX, 12000);
const result = await execOnSession(event, sessionId, PROCESS_LIST_SCRIPT_POSIX, 12000, {
maxBuffer: PROCESS_LIST_MAX_BUFFER,
});
Comment on lines +195 to +197

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bound SSH process-list output

For normal SSH sessions, this maxBuffer option is never enforced: execOnSession reaches execOnConnection, whose stream handler just appends every stdout chunk and ignores execOptions.maxBuffer (only local execFile and ET honor it). After removing the head -n 2000 cap, opening the Processes tab against a host with very many processes or long command lines can stream unbounded data into the Electron main process and spike memory or hang instead of failing cleanly at the intended 64 MiB limit.

Useful? React with 👍 / 👎.

if (result.pending) return { success: false, pending: true };
if (!result.success) return { success: false, error: result.error || "Failed to list processes" };
return { success: true, processes: parseProcessLines(result.stdout) };
Expand Down
33 changes: 33 additions & 0 deletions electron/bridges/systemManagerBridge.processes.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
const test = require("node:test");
const assert = require("node:assert/strict");
const { EventEmitter } = require("node:events");
const fs = require("node:fs");
const path = require("node:path");
const { createSystemManagerBridge } = require("./systemManagerBridge.cjs");

function createFakeExecStream(stdout, options = {}) {
Expand All @@ -26,8 +28,10 @@ test("listProcesses uses a ps format that works on CentOS 7 procps", async () =>
" 1 0 root Ss 0.0 0.0 4060 191024 2-19:23:42 /usr/lib/systemd/systemd --switched-root --system --deserialize 21",
].join("\n");

let seenCommand = "";
const conn = {
exec(command, callback) {
seenCommand = command;
const stdout = command.includes(compatiblePsFormat)
? compatibleOutput
: badCentos7Output;
Expand All @@ -46,6 +50,35 @@ test("listProcesses uses a ps format that works on CentOS 7 procps", async () =>
assert.equal(result.processes.length, 1);
assert.equal(result.processes[0].pid, 1);
assert.equal(result.processes[0].command, "/usr/lib/systemd/systemd --switched-root --system --deserialize 21");
assert.doesNotMatch(seenCommand, /head\s+-n\s+2000/);
});

test("process listing commands do not hard-cap the visible list at 2000 entries", () => {
const source = fs.readFileSync(path.join(__dirname, "systemManagerBridge.cjs"), "utf8");

assert.doesNotMatch(source, /head\s+-n\s+2000/);
assert.doesNotMatch(source, /Select-Object\s+-First\s+2000/);
});

test("listProcesses gives ET process listing enough output buffer for dense hosts", async () => {
let seenOptions = null;
const sessions = new Map([["s1", {
type: "et",
etStatsAuth: { knownHosts: [] },
}]]);
const bridge = createSystemManagerBridge({
getSessions: () => sessions,
process,
execOnEtSession: async (_session, _command, _timeoutMs, options) => {
seenOptions = options;
return { success: true, stdout: "" };
},
});

const result = await bridge.listProcesses(null, { sessionId: "s1" });

assert.equal(result.success, true);
assert.ok(seenOptions.maxBuffer > 10 * 1024 * 1024);
});

test("probeCapabilities reports Docker when docker is installed even if plain docker access is denied", async () => {
Expand Down
3 changes: 3 additions & 0 deletions electron/bridges/terminalBridge/etSession.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,9 @@ main();
env: { ...process.env, ...session.sshEnv },
timeout: timeoutMs,
encoding: "utf8",
maxBuffer: Number.isFinite(Number(execOpts.maxBuffer)) && Number(execOpts.maxBuffer) > 0
? Math.floor(Number(execOpts.maxBuffer))
: undefined,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve the default ET exec buffer cap

When execOpts.maxBuffer is omitted, this now passes maxBuffer: undefined to child_process.execFile; in Node this is not the same as omitting the option and leaves ET execs without the default output cap. ET operations that do not supply a maxBuffer, such as capability probes or tmux/docker commands, can therefore buffer unbounded stdout/stderr until the timeout if the remote command is noisy; only include this property for valid overrides or set an explicit default.

Useful? React with 👍 / 👎.

windowsHide: true,
}, (err, stdout, stderr) => {
if (err) {
Expand Down
22 changes: 22 additions & 0 deletions electron/bridges/terminalBridge/etSession.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,28 @@ test("execOnEtSession requireTrustedHost uses strict host-key checking", async (
assert.match(strictContent, /host\.example ssh-ed25519 vaultblob/);
});

test("execOnEtSession forwards maxBuffer to the ssh execFile call", async (t) => {
let capturedOptions = null;
const { api } = makeApi(t, {
execFile: (_cmd, _args, opts, cb) => {
capturedOptions = opts;
process.nextTick(() => cb(null, "", ""));
},
});
const env = api.prepareEtSshEnvironment("sess1", { hostname: "host.example", username: "alice" });
const session = {
sshUserHost: env.userHost,
sshOptions: env.sshOptions,
sshEnv: env.env,
externalAuthArtifacts: env.artifacts,
externalAuthArtifactsCleaned: false,
};

await api.execOnEtSession(session, "echo ok", 1000, { maxBuffer: 64 * 1024 * 1024 });

assert.equal(capturedOptions.maxBuffer, 64 * 1024 * 1024);
});

test("cleanupStaleEtTempDirs only removes Netcatty ET temp directories by prefix", (t) => {
const { api, base } = makeApi(t);
const staleEtDir = path.join(base, "et-ssh-home-old-session");
Expand Down
Loading