mirror of
https://github.com/binaricat/Netcatty.git
synced 2026-10-09 15:58:34 +00:00
fix(ai): prime echo suppression before agent input to stop OpenWrt ash variable leak (#3385)
* fix(#3384): automated AI fix * fix(ai): order display suppression with terminal output --------- Co-authored-by: netcatty-bot <308658023+netcatty-bot@users.noreply.github.com> Co-authored-by: bincxz <16399091+binaricat@users.noreply.github.com>
This commit is contained in:
@@ -64,6 +64,7 @@ function startPtyJob(ptyStream, command, options) {
|
||||
expectedPrompt,
|
||||
typedInput = false,
|
||||
echoCommand,
|
||||
onEchoSuppressionPrime,
|
||||
maxBufferedChars = 0,
|
||||
normalizeFinalOutput = true,
|
||||
enforceWallTimeout = false,
|
||||
@@ -812,6 +813,21 @@ function startPtyJob(ptyStream, command, options) {
|
||||
writeInput(`${buildPendingInputClearPrefix(resolvedShellKind)}${wrapped}`);
|
||||
}
|
||||
|
||||
// Prime the renderer's display suppression before the first byte is typed
|
||||
// (issue #3384). Shells whose line editor echoes input, such as BusyBox ash
|
||||
// on OpenWrt, break long echoed lines at the terminal width with CR/LF; the
|
||||
// wrapped fragments no longer contain the marker and would leak through the
|
||||
// per-line echo filter as visible "variables" until the wrapper's own
|
||||
// _I printf runs. Delivering the _I line over the data channel up front
|
||||
// suppresses the whole echo; the _S output releases it, and finish() sends
|
||||
// the _R reset (onProbeAborted) when the command never starts.
|
||||
if (typeof onEchoSuppressionPrime === "function") {
|
||||
try {
|
||||
onEchoSuppressionPrime(marker);
|
||||
} catch {
|
||||
// Display suppression must never prevent the command from starting.
|
||||
}
|
||||
}
|
||||
if (probingShell) {
|
||||
writeInput(`${buildPendingInputClearPrefix(resolvedShellKind)}${buildLiveShellProbe(marker)}`);
|
||||
} else {
|
||||
|
||||
@@ -1420,3 +1420,44 @@ test("startPtyJob types the wrapper one code point per write for strict bastions
|
||||
assert.equal(result.ok, false);
|
||||
assert.equal(result.error, "Cancelled");
|
||||
});
|
||||
|
||||
test("echo suppression prime is delivered before the first typed input (#3384)", async () => {
|
||||
for (const probeLiveShell of [false, true]) {
|
||||
const pty = new EventEmitter();
|
||||
const events = [];
|
||||
pty.write = (data) => {
|
||||
if (data === "\x03") return;
|
||||
events.push(`write:${data}`);
|
||||
};
|
||||
const job = startPtyJob(pty, "echo prime-order", {
|
||||
shellKind: "posix",
|
||||
probeLiveShell,
|
||||
timeoutMs: 1000,
|
||||
onEchoSuppressionPrime: (marker) => events.push(`prime:${marker}`),
|
||||
});
|
||||
assert.equal(events[0], `prime:${job.marker}`, "prime precedes every typed byte");
|
||||
assert.ok((events[1] || "").startsWith("write:"), "first write follows the prime");
|
||||
assert.equal(events.filter((event) => event.startsWith("prime:")).length, 1);
|
||||
if (probeLiveShell) {
|
||||
pty.emit("data", `${job.marker}_P:sh\n${job.marker}_Q`);
|
||||
}
|
||||
pty.emit("data", `${job.marker}_S\ndone\n${job.marker}_E:0\n`);
|
||||
const result = await job.resultPromise;
|
||||
assert.equal(result.ok, true, JSON.stringify(result));
|
||||
}
|
||||
});
|
||||
|
||||
test("a throwing echo suppression prime callback still types the command (#3384)", () => {
|
||||
const writes = [];
|
||||
const pty = new EventEmitter();
|
||||
pty.write = (data) => writes.push(String(data));
|
||||
const job = startPtyJob(pty, "echo resilient", {
|
||||
shellKind: "posix",
|
||||
timeoutMs: 1000,
|
||||
onEchoSuppressionPrime: () => {
|
||||
throw new Error("Renderer closed");
|
||||
},
|
||||
});
|
||||
assert.ok(writes.some((data) => data.includes("echo resilient")));
|
||||
job.cancel();
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
// Module-level require on purpose: code inside registerCattyExecHandlers
|
||||
// runs under `with (ctx)` where bare `require` resolves to ctx.require
|
||||
// (based in electron/bridges/). Requiring here keeps the path unambiguous.
|
||||
const { emitTerminalSessionData } = require("../emitTerminalSessionData.cjs");
|
||||
const { formatSyntheticEcho } = require("../ai/shellUtils.cjs");
|
||||
const { remoteDisallowsExecChannelProbe, ensureSessionShellKindForExec } = require("../ai/sessionShellKind.cjs");
|
||||
|
||||
@@ -180,7 +181,11 @@ function registerCattyExecHandlers(ctx) {
|
||||
bastionKeystrokes: remoteDisallowsExecChannelProbe(session.remoteSshVersion),
|
||||
onProbeAborted: (marker) => {
|
||||
const contents = electronModule?.webContents?.fromId?.(session.webContentsId);
|
||||
safeSend(contents, "netcatty:data", { sessionId, data: `${marker}_R\n` });
|
||||
emitTerminalSessionData(contents, sessionId, `${marker}_R\n`, { session });
|
||||
},
|
||||
onEchoSuppressionPrime: (marker) => {
|
||||
const contents = electronModule?.webContents?.fromId?.(session.webContentsId);
|
||||
emitTerminalSessionData(contents, sessionId, `${marker}_I\n`, { session });
|
||||
},
|
||||
chatSessionId,
|
||||
expectedPrompt: getFreshIdlePrompt(session),
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
const assert = require('node:assert/strict');
|
||||
const { EventEmitter } = require('node:events');
|
||||
const test = require('node:test');
|
||||
const { configureTerminalSessionDataEmitter, emitTerminalSessionData } = require('./emitTerminalSessionData.cjs');
|
||||
const { registerCattyExecHandlers } = require('./aiBridge/cattyExecHandlers.cjs');
|
||||
const { execViaPty } = require('./ai/ptyExec.cjs');
|
||||
const bridge = require('./mcpServerBridge.cjs');
|
||||
|
||||
for (const surface of ['catty', 'mcp-exec', 'mcp-job']) {
|
||||
for (const hasPort of [true, false]) {
|
||||
test(`${surface} primes and resets on the PTY output transport (port=${hasPort})`, async (t) => {
|
||||
const deliveries = [];
|
||||
const pty = new EventEmitter();
|
||||
const session = { protocol: 'local', shellKind: 'posix', pty, webContentsId: 7 };
|
||||
const contents = { isDestroyed: () => false, send: (_channel, payload) => {
|
||||
if (!payload.syntheticEcho) deliveries.push(['ipc', payload.data]);
|
||||
} };
|
||||
const sessions = new Map([['session', session]]);
|
||||
configureTerminalSessionDataEmitter({ getSession: id => sessions.get(id), outputChannel: {
|
||||
send: (_id, data) => {
|
||||
if (!hasPort) return false;
|
||||
deliveries.push(['port', data]);
|
||||
return true;
|
||||
},
|
||||
} });
|
||||
t.after(() => { bridge.cleanup(); configureTerminalSessionDataEmitter(); });
|
||||
const transport = hasPort ? 'port' : 'ipc';
|
||||
let writes = 0;
|
||||
pty.write = data => {
|
||||
if (data === '\x03') return;
|
||||
if (writes++ === 0) {
|
||||
assert.equal(deliveries.length, 1, 'prime arrives before any typed bytes');
|
||||
assert.equal(deliveries[0][0], transport);
|
||||
assert.match(deliveries[0][1], /__NCMCP_.*_I\r?\n$/);
|
||||
emitTerminalSessionData(contents, 'session', 'wrapped echo\r\n', { session });
|
||||
}
|
||||
};
|
||||
const electronModule = { webContents: { fromId: () => contents } };
|
||||
bridge.init({ sessions, electronModule });
|
||||
bridge.setPermissionMode('auto');
|
||||
bridge.updateSessionMetadata([{ sessionId: 'session', protocol: 'local', connected: true }], 'chat');
|
||||
let pending;
|
||||
if (surface === 'catty') {
|
||||
const handlers = new Map();
|
||||
registerCattyExecHandlers({ ipcMain: { handle: (key, fn) => handlers.set(key, fn) },
|
||||
validateSender: () => true, sessions, mcpServerBridge: bridge, electronModule,
|
||||
safeSend: (target, ...args) => target.send(...args), execViaPty, getFreshIdlePrompt: () => '' });
|
||||
pending = handlers.get('netcatty:ai:exec')({ sender: contents }, { sessionId: 'session', command: 'echo test', chatSessionId: 'chat' });
|
||||
} else {
|
||||
pending = bridge.dispatchBuiltinRpc(surface === 'mcp-job' ? 'netcatty/jobStart' : 'netcatty/exec', {
|
||||
sessionId: 'session', command: 'echo test', chatSessionId: 'chat',
|
||||
});
|
||||
}
|
||||
await new Promise(resolve => setTimeout(resolve, 30));
|
||||
assert.ok(writes > 0, 'execution must reach the PTY');
|
||||
bridge.cancelAllPtyExecs();
|
||||
pty.emit("close");
|
||||
await pending;
|
||||
assert.equal(deliveries[1][0], transport);
|
||||
assert.equal(deliveries[1][1], 'wrapped echo\r\n');
|
||||
assert.equal(deliveries.at(-1)[0], transport);
|
||||
assert.match(deliveries.at(-1)[1], /__NCMCP_.*_R\r?\n$/);
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -21,6 +21,7 @@ const {
|
||||
} = require("./ai/commandSafety.cjs");
|
||||
const { execViaPty, startPtyJob, execViaChannel, execViaRawPty } = require("./ai/ptyExec.cjs");
|
||||
const { safeSend } = require("./ipcUtils.cjs");
|
||||
const { emitTerminalSessionData } = require("./emitTerminalSessionData.cjs");
|
||||
const { getCliDiscoveryFilePath } = require("../cli/discoveryPath.cjs");
|
||||
const { EXTERNAL_MCP_CHAT_SESSION_ID } = require("../cli/externalMcpDiscoveryPath.cjs");
|
||||
const sftpBridge = require("./sftpBridge.cjs");
|
||||
@@ -487,6 +488,10 @@ function shutdownHost({ preserveScopedMetadata = false } = {}) {
|
||||
function echoCommandToSession(session, sessionId, command, { syntheticEcho = true } = {}) {
|
||||
if (!electronModule || !session?.webContentsId || !command) return;
|
||||
const contents = electronModule.webContents?.fromId?.(session.webContentsId);
|
||||
if (!syntheticEcho) {
|
||||
emitTerminalSessionData(contents, sessionId, formatSyntheticEcho(command), { session });
|
||||
return;
|
||||
}
|
||||
safeSend(contents, "netcatty:data", {
|
||||
sessionId,
|
||||
data: formatSyntheticEcho(command),
|
||||
|
||||
@@ -149,6 +149,7 @@ function createExecHandlerApi(ctx) {
|
||||
probeLiveShell: true,
|
||||
bastionKeystrokes: remoteDisallowsExecChannelProbe(session.remoteSshVersion),
|
||||
onProbeAborted: (marker) => echoCommandToSession(session, sessionId, `${marker}_R`, { syntheticEcho: false }),
|
||||
onEchoSuppressionPrime: (marker) => echoCommandToSession(session, sessionId, `${marker}_I`, { syntheticEcho: false }),
|
||||
expectedPrompt: getFreshIdlePrompt(session),
|
||||
typedInput: true,
|
||||
echoCommand: (rawCommand) => echoCommandToSession(session, sessionId, rawCommand),
|
||||
@@ -319,6 +320,7 @@ function createExecHandlerApi(ctx) {
|
||||
probeLiveShell: true,
|
||||
bastionKeystrokes: remoteDisallowsExecChannelProbe(session.remoteSshVersion),
|
||||
onProbeAborted: (marker) => echoCommandToSession(session, sessionId, `${marker}_R`, { syntheticEcho: false }),
|
||||
onEchoSuppressionPrime: (marker) => echoCommandToSession(session, sessionId, `${marker}_I`, { syntheticEcho: false }),
|
||||
chatSessionId,
|
||||
expectedPrompt: getFreshIdlePrompt(session),
|
||||
typedInput: true,
|
||||
|
||||
@@ -1802,6 +1802,34 @@ test("OpenWrt bounded wrapper continuations stay hidden across fragmented echoes
|
||||
}
|
||||
});
|
||||
|
||||
test("primed suppression hides BusyBox ash echo wrapped mid-marker (#3384)", () => {
|
||||
const preload = loadPreloadWithFakeElectron();
|
||||
try {
|
||||
const received = [];
|
||||
const sessionId = 'ash-wrap';
|
||||
const marker = '__NCMCP_mttikd5b_ccbc892e865a115a80c88afdc77b96a6__';
|
||||
preload.api.onSessionData(sessionId, chunk => received.push(chunk));
|
||||
// The exec bridge primes display suppression over the data channel before
|
||||
// the wrapper is typed, mirroring onEchoSuppressionPrime in ptyExec.cjs.
|
||||
preload.handlers.get('netcatty:data')({}, { sessionId, data: `${marker}_I\n` });
|
||||
// BusyBox ash's line editor breaks the echoed first wrapper line at the
|
||||
// terminal width, so the second PTY line carries no complete __NCMCP_
|
||||
// marker and the per-line echo filter alone cannot drop it.
|
||||
const firstLine = ` ${marker}=0; printf '\\n%s\\n' '${marker}_I'`;
|
||||
const secondLine = ` : '${marker}'; ${marker}_cmd='echo visible-output'; \\`;
|
||||
const echo = `${firstLine.slice(0, 55)}\r\n${firstLine.slice(55)}\r\n`
|
||||
+ `${secondLine.slice(0, 70)}\r\n${secondLine.slice(70)}\r\n`
|
||||
+ `> : '${marker}'; printf '%s\\n' '${marker}_S'\r\n`;
|
||||
const data = `${echo}\n${marker}_S\r\nvisible-output\r\n${marker}_E:0\r\n`;
|
||||
for (let offset = 0; offset < data.length; offset += 7) {
|
||||
preload.handlers.get('netcatty:data')({}, { sessionId, data: data.slice(offset, offset + 7) });
|
||||
}
|
||||
assert.equal(received.join(''), 'visible-output\r\n');
|
||||
} finally {
|
||||
preload.cleanup();
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
test("ordinary text resembling an OpenWrt continuation is released", async () => {
|
||||
const preload = loadPreloadWithFakeElectron();
|
||||
@@ -1873,3 +1901,61 @@ test("aborted input releases custom prompts and ignores a late input marker", ()
|
||||
assert.equal(received.join(""), "normal output\nlate prompt\n");
|
||||
} finally { preload.cleanup(); }
|
||||
});
|
||||
|
||||
test('real OpenWrt ash hides wrapped input and preserves output with priming (#3384)', {
|
||||
skip: !process.env.NETCATTY_OPENWRT_SSH_PORT,
|
||||
timeout: 60000,
|
||||
}, async () => {
|
||||
const { Client } = require('ssh2');
|
||||
const { execViaPty } = require('./bridges/ai/ptyExec.cjs');
|
||||
const client = new Client();
|
||||
await new Promise((resolve, reject) => client.once('ready', resolve).once('error', reject).connect({
|
||||
host: '127.0.0.1', port: Number(process.env.NETCATTY_OPENWRT_SSH_PORT), username: 'root', password: '',
|
||||
}));
|
||||
try {
|
||||
for (const cols of [80, 120]) {
|
||||
for (const probeLiveShell of [false, true]) {
|
||||
const preload = loadPreloadWithFakeElectron();
|
||||
const stream = await new Promise((resolve, reject) => client.shell({ term: 'xterm', cols, rows: 30 },
|
||||
(error, value) => error ? reject(error) : resolve(value)));
|
||||
try {
|
||||
await new Promise((resolve, reject) => {
|
||||
let output = '';
|
||||
const timer = setTimeout(() => reject(new Error('OpenWrt prompt missing')), 5000);
|
||||
const onData = data => {
|
||||
output += data;
|
||||
if (output.includes(':~# ')) {
|
||||
clearTimeout(timer);
|
||||
stream.removeListener('data', onData);
|
||||
resolve();
|
||||
}
|
||||
};
|
||||
stream.on('data', onData);
|
||||
});
|
||||
const received = [];
|
||||
const sessionId = `openwrt-${cols}-${probeLiveShell}`;
|
||||
preload.api.onSessionData(sessionId, chunk => received.push(chunk));
|
||||
const deliver = data => preload.handlers.get('netcatty:data')({}, { sessionId, data: String(data) });
|
||||
stream.on('data', deliver);
|
||||
const result = await execViaPty(stream, "printf 'visible-output\\n'", {
|
||||
shellKind: 'posix', probeLiveShell, typedInput: true, timeoutMs: 5000,
|
||||
onEchoSuppressionPrime: marker => deliver(`${marker}_I\n`),
|
||||
onProbeAborted: marker => deliver(`${marker}_R\n`),
|
||||
});
|
||||
await sleep(150);
|
||||
assert.equal(result.ok, true, JSON.stringify(result));
|
||||
assert.equal(result.stdout.trim(), 'visible-output');
|
||||
const display = received.join('');
|
||||
assert.match(display, /visible-output/);
|
||||
assert.doesNotMatch(display, /__nc_|__NCMCP_|printf|eval|unset|_cmd=|_d=/,
|
||||
JSON.stringify({ cols, probeLiveShell, display }));
|
||||
} finally {
|
||||
stream.close();
|
||||
preload.cleanup();
|
||||
}
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
client.end();
|
||||
}
|
||||
});
|
||||
|
||||
@@ -298,6 +298,12 @@ function createWorkerAiExecHandler({
|
||||
data: `${marker}_R\n`,
|
||||
});
|
||||
},
|
||||
onEchoSuppressionPrime: (marker) => {
|
||||
event?.sender?.send?.("netcatty:data", {
|
||||
sessionId,
|
||||
data: `${marker}_I\n`,
|
||||
});
|
||||
},
|
||||
chatSessionId,
|
||||
expectedPrompt: getFreshIdlePrompt(session),
|
||||
typedInput: true,
|
||||
@@ -502,6 +508,12 @@ function createWorkerAiJobStartHandler({
|
||||
data: `${marker}_R\n`,
|
||||
});
|
||||
},
|
||||
onEchoSuppressionPrime: (marker) => {
|
||||
event?.sender?.send?.("netcatty:data", {
|
||||
sessionId,
|
||||
data: `${marker}_I\n`,
|
||||
});
|
||||
},
|
||||
chatSessionId,
|
||||
expectedPrompt: getFreshIdlePrompt(session),
|
||||
typedInput: true,
|
||||
|
||||
@@ -145,6 +145,7 @@ test("worker AI background jobs start, poll, stop, and block overlapping exec",
|
||||
assert.equal(started.command, "npm test");
|
||||
assert.equal(started.status, "running");
|
||||
assert.equal(started.outputMode, "foreground-mirrored");
|
||||
const marker = await extractMarker(pty.writes);
|
||||
assert.deepEqual(event.rendererMessages, [
|
||||
{
|
||||
channel: "netcatty:data",
|
||||
@@ -154,9 +155,14 @@ test("worker AI background jobs start, poll, stop, and block overlapping exec",
|
||||
syntheticEcho: true,
|
||||
},
|
||||
},
|
||||
{
|
||||
channel: "netcatty:data",
|
||||
payload: {
|
||||
sessionId: "ssh-1",
|
||||
data: `${marker}_I\n`,
|
||||
},
|
||||
},
|
||||
]);
|
||||
|
||||
const marker = await extractMarker(pty.writes);
|
||||
pty.emit("data", `${marker}_S\r\nready\r\n`);
|
||||
await nextTick();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user