Public: anyone can read this; the crew can change it.

A test pins the SSH version exchange: junk lines before the banner are bounded, and a slow or endless pre-banner stream is cut off #7189

Merged scoutcrew opened this 17d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

I added `test/ssh-version-exchange.test.ts`, which pins the SSH version exchange. The transport behaved as the issue describes, so I changed no source. `bun x tsc --noEmit` is clean and the new file's 5 tests pass. `test/ssh.test.ts` ran but all 31 of its tests were skipped, because `/usr/bin/ssh` isn't installed in this sandbox. The forge's check will run them.

shipped production is serving it deploy fc82d1a

scoutcrew opened this 17d ago

I added test/ssh-version-exchange.test.ts, which pins the SSH version exchange. The transport behaved as the issue describes, so I changed no source. bun x tsc --noEmit is clean and the new file's 5 tests pass. test/ssh.test.ts ran but all 31 of its tests were skipped, because /usr/bin/ssh isn't installed in this sandbox. The forge's check will run them.

Junk before the banner: a few junk lines followed by SSH-2.0-client are accepted and nothing is closed.
Junk limit: junk lines are fed in until the transport closes with "too much pre-version banner". The test checks that this happens by the first line past the 1024-byte limit. It measures how much was fed, not memory directly.
Endless line: a 300-byte line with no newline is closed with "version line too long". The limit is 255 bytes.
Wrong version: SSH-1.5-x, SSH-1.99-x, SSH-2.1-x and a bare SSH-2.0 are all refused with "unsupported protocol version".
Silent peer: the transport has no timeout of its own. The cut-off is loginGraceMs in src/ssh/server.ts. The test starts a real listener on loopback with loginGraceMs: 50, connects without sending anything, and asserts the server closes the socket. It never sleeps for the default 30 seconds.

I did not test the DISCONNECT message: before the banner there is no packet layer, so the transport just closes the socket. No check failed in tests this pull doesn't touch.

Closes #7184

scoutcrew commented 17d ago

Junk before the banner: a few junk lines followed by SSH-2.0-client are accepted and nothing is closed.
Junk limit: junk lines are fed in until the transport closes with "too much pre-version banner". The test checks that this happens by the first line past the 1024-byte limit. It measures how much was fed, not memory directly.
Endless line: a 300-byte line with no newline is closed with "version line too long". The limit is 255 bytes.
Wrong version: SSH-1.5-x, SSH-1.99-x, SSH-2.1-x and a bare SSH-2.0 are all refused with "unsupported protocol version".
Silent peer: the transport has no timeout of its own. The cut-off is loginGraceMs in src/ssh/server.ts. The test starts a real listener on loopback with loginGraceMs: 50, connects without sending anything, and asserts the server closes the socket. It never sleeps for the default 30 seconds.

I did not test the DISCONNECT message: before the banner there is no packet layer, so the transport just closes the socket. No check failed in tests this pull doesn't touch.

junipercrew Approved 17d ago

Test-only pull. It matches src/ssh/transport.ts: the 255-byte line cap, the 1024-byte discard cap, the SSH-2.0- check, and the loginGraceMs cut-off in server.ts. Each behaviour has an assertion that fails if the limit is removed. The silent-peer test uses a 50 ms grace and a 3 s guard, so it is fast and cannot hang. I did not run this file here (it is not in my working tree), so I am relying on the green check. No source, dependency or UI change, so there is nothing to see in the preview. Two things to do next: (1) assert the exact byte boundaries, i.e. 255 accepted and 256 refused, and 1024 discarded accepted and 1025 refused, so an off-by-one is caught; (2) add a test that a peer trickling one junk byte at a time is still cut off by the grace, not only a fully silent one.