From dffcefc3088cc04afde14d72a23e4dcc545807ef Mon Sep 17 00:00:00 2001 From: Netcatty-Bot Date: Fri, 18 Sep 2026 14:10:52 +0800 Subject: [PATCH] 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> --- electron/bridges/ai/ptyExec.cjs | 16 ++++ electron/bridges/ai/ptyExec.test.cjs | 41 +++++++++ .../bridges/aiBridge/cattyExecHandlers.cjs | 7 +- .../aiEchoSuppressionOrdering.test.cjs | 65 ++++++++++++++ electron/bridges/mcpServerBridge.cjs | 5 ++ .../bridges/mcpServerBridge/execHandlers.cjs | 2 + electron/preloadDataBacklog.test.cjs | 86 +++++++++++++++++++ electron/terminalWorker/aiExec.cjs | 12 +++ electron/terminalWorker/aiExec.test.cjs | 10 ++- 9 files changed, 241 insertions(+), 3 deletions(-) create mode 100644 electron/bridges/aiEchoSuppressionOrdering.test.cjs diff --git a/electron/bridges/ai/ptyExec.cjs b/electron/bridges/ai/ptyExec.cjs index c15f071e2..f9628fd17 100644 --- a/electron/bridges/ai/ptyExec.cjs +++ b/electron/bridges/ai/ptyExec.cjs @@ -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 { diff --git a/electron/bridges/ai/ptyExec.test.cjs b/electron/bridges/ai/ptyExec.test.cjs index 74fee605f..2ab352524 100644 --- a/electron/bridges/ai/ptyExec.test.cjs +++ b/electron/bridges/ai/ptyExec.test.cjs @@ -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(); +}); diff --git a/electron/bridges/aiBridge/cattyExecHandlers.cjs b/electron/bridges/aiBridge/cattyExecHandlers.cjs index 890057e4a..8cf6dfb8f 100644 --- a/electron/bridges/aiBridge/cattyExecHandlers.cjs +++ b/electron/bridges/aiBridge/cattyExecHandlers.cjs @@ -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), diff --git a/electron/bridges/aiEchoSuppressionOrdering.test.cjs b/electron/bridges/aiEchoSuppressionOrdering.test.cjs new file mode 100644 index 000000000..a929172a7 --- /dev/null +++ b/electron/bridges/aiEchoSuppressionOrdering.test.cjs @@ -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$/); + }); + } +} diff --git a/electron/bridges/mcpServerBridge.cjs b/electron/bridges/mcpServerBridge.cjs index 6b6533653..f92469fde 100644 --- a/electron/bridges/mcpServerBridge.cjs +++ b/electron/bridges/mcpServerBridge.cjs @@ -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), diff --git a/electron/bridges/mcpServerBridge/execHandlers.cjs b/electron/bridges/mcpServerBridge/execHandlers.cjs index 80816c99f..9747a2be5 100644 --- a/electron/bridges/mcpServerBridge/execHandlers.cjs +++ b/electron/bridges/mcpServerBridge/execHandlers.cjs @@ -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, diff --git a/electron/preloadDataBacklog.test.cjs b/electron/preloadDataBacklog.test.cjs index 0e6e03b4b..a2458a476 100644 --- a/electron/preloadDataBacklog.test.cjs +++ b/electron/preloadDataBacklog.test.cjs @@ -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(); + } +}); diff --git a/electron/terminalWorker/aiExec.cjs b/electron/terminalWorker/aiExec.cjs index 4c93e9544..dc38fa061 100644 --- a/electron/terminalWorker/aiExec.cjs +++ b/electron/terminalWorker/aiExec.cjs @@ -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, diff --git a/electron/terminalWorker/aiExec.test.cjs b/electron/terminalWorker/aiExec.test.cjs index ca73781ce..985fa77ed 100644 --- a/electron/terminalWorker/aiExec.test.cjs +++ b/electron/terminalWorker/aiExec.test.cjs @@ -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();