From 6a4e6ce28aa91eb7cc19e6453880814b6d3cbeeb Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Mon, 6 Jul 2026 21:28:15 -0700 Subject: [PATCH] perf(ssh): bound the system-ssh port-forward stderr tail (#7647) The stderr handler in system-ssh-port-forward-provider.ts stays attached for the forward process's entire lifetime and appends every chunk to an unbounded string, only ever read to build the exit-error detail. A chatty/warning-spamming remote sshd over a long-lived `ssh -N -L` forward could grow it without bound. Fix: keep only the most-recent 64 KB tail, mirroring MAX_RELAY_STARTUP_BUFFER_BYTES in ssh-relay-deploy-helpers.ts. The tail is what the error message surfaces anyway. Test (red->green): emitting >64 KB of stderr then exiting keeps the recent tail marker and drops the oldest, detail bounded below the produced size; without the cap the whole string is retained. --- src/main/ssh/ssh-port-forward.test.ts | 21 +++++++++++++++++++ .../ssh/system-ssh-port-forward-provider.ts | 8 +++++++ 2 files changed, 29 insertions(+) diff --git a/src/main/ssh/ssh-port-forward.test.ts b/src/main/ssh/ssh-port-forward.test.ts index 31e26a5bc40..b49c627b8b3 100644 --- a/src/main/ssh/ssh-port-forward.test.ts +++ b/src/main/ssh/ssh-port-forward.test.ts @@ -267,6 +267,27 @@ describe('SshPortForwardManager', () => { ) }) + it('keeps only a bounded tail of a chatty forward stderr (memory-leak regression)', async () => { + const onForwardClosed = vi.fn() + manager.setCallbacks({ onForwardClosed }) + const forward = createFakeSystemSshForward() + startSystemSshPortForwardProcessMock.mockReturnValue(forward) + const conn = createSystemSshConn() + + await manager.addForward('conn-1', conn as never, 3000, '127.0.0.1', 8080) + // A long-lived forward against a chatty remote sshd: emit >64 KB of stderr. + forward.process.stderr.emit('data', Buffer.from(`HEAD_MARKER${'x'.repeat(70 * 1024)}`)) + forward.process.stderr.emit('data', Buffer.from('TAIL_MARKER')) + forward.process.emit('exit', 255) + + const detail = onForwardClosed.mock.calls[0]?.[1]?.detail as string + // The oldest bytes are trimmed; the most-recent tail is retained. + expect(detail).toContain('TAIL_MARKER') + expect(detail).not.toContain('HEAD_MARKER') + // Bounded well below the ~70 KB produced. + expect(detail.length).toBeLessThan(66 * 1024) + }) + it('lists forwards filtered by connectionId', async () => { const conn = createMockConn() await manager.addForward('conn-1', conn as never, 3000, 'localhost', 8080) diff --git a/src/main/ssh/system-ssh-port-forward-provider.ts b/src/main/ssh/system-ssh-port-forward-provider.ts index 74227f67cfe..e2b53abab7b 100644 --- a/src/main/ssh/system-ssh-port-forward-provider.ts +++ b/src/main/ssh/system-ssh-port-forward-provider.ts @@ -33,9 +33,17 @@ export class SystemSshPortForwardProvider implements SshPortForwardProvider { ) await forward.waitForStartup() + // Why: this stderr stays attached for the forward's whole lifetime but is + // only used to build the exit-error detail, so keep a bounded tail — a chatty + // remote sshd could otherwise grow it unbounded on a long-lived forward + // (mirrors MAX_RELAY_STARTUP_BUFFER_BYTES in ssh-relay-deploy-helpers). + const MAX_STDERR_TAIL_BYTES = 64 * 1024 let stderr = '' const onStderr = (chunk: Buffer): void => { stderr += chunk.toString('utf-8') + if (stderr.length > MAX_STDERR_TAIL_BYTES) { + stderr = stderr.slice(-MAX_STDERR_TAIL_BYTES) + } } forward.process.stderr?.on('data', onStderr)