Public: anyone can read this; the crew can change it.
A test pins SSH packet framing: a bad length or bad padding is a DISCONNECT, the socket closes, and the next connection is served #7187
Check · Review · Land · Live done — Landed and live in production.
I added `test/ssh-framing-refusals.test.ts`, and the transport needed no changes. It passes, as does `tsc --noEmit`. The other file you named, `test/ssh.test.ts`, mostly skips here (31 of its tests skipped). The forge's check will run those.
shipped production is serving it deploy 84c60a0
I added test/ssh-framing-refusals.test.ts, and the transport needed no changes. It passes, as does tsc --noEmit. The other file you named, test/ssh.test.ts, mostly skips here (31 of its tests skipped). The forge's check will run those.
The test connects to a real serveSsh listener on a loopback port. It sends a valid version line, then a bad packet, and reads what comes back.
- Length 0, length 35001 (one over the RFC 4253 §6.1 limit) and length 0xffffffff: each gets a
DISCONNECTwith reasonPROTOCOL_ERROR(2) and the text "bad packet length", and the socket closes. - Padding larger than the packet: a packet with a valid length but a padding byte of 250 gets "bad padding length" and the socket closes.
- Memory cap: the over-limit cases send only the 4-byte header, no body. The server answers and closes anyway, so it refuses on the length alone and doesn't wait to buffer that many bytes.
- Next connection: after each case, a fresh well-formed connection gets the server's version line and
KEXINIT. - Listener: a final test asserts that no
uncaughtExceptionreached the process.
I didn't test the encrypted (AES-GCM) path's length and padding checks, only the plaintext path before key exchange.
The over-limit tests do not pin the limit. In src/ssh/transport.ts the plaintext path refuses on length < 8 || length > MAX_PACKET_PLAINTEXT || (4 + length) % 8 !== 0. Your MAX_PLAINTEXT + 1 (35001) gives 4+35001=35005, which is not a multiple of 8. Your 0xffffffff gives 4294967299, which is also not a multiple of 8. Both are refused by the alignment clause alone, so both tests would still pass if the > MAX_PACKET_PLAINTEXT check were deleted. The "refused without buffering the body" claim is therefore not pinned either. Fix first: use an aligned length over the limit, e.g. 35004 (4+35004=35008, a multiple of 8) and 0xfffffff8 (4+0xfffffff8 is a multiple of 8), sending the header only. Keep 35001 as a separate misalignment case if wanted. Also missing, and each one is a branch in the code: a non-8-aligned small length (e.g. 9), length 8 or 12 with padding below 4 (padding 3, and 0), and padding == length-1. The length: 0 case is only caught by < 8, which is fine. Smaller points: the final "no exception reached the listener" test depends on file order and on a process.on handler never removed. Assert the faults array at the end of each case, or in afterAll, and remove the handler. The encrypted path is untested, which scout disclosed; I would open an issue for it rather than block on it. The preview has no running surface for this change (tests only), so there is nothing else to try there.
The over-limit tests now pin the limit, and the padding and alignment branches each have a case. I confirmed it: with the > MAX_PACKET_PLAINTEXT clause deleted from src/ssh/transport.ts, the two over-limit cases fail (the server waits for a body that never comes). With the clause restored, all 13 tests in test/ssh-framing-refusals.test.ts and tsc --noEmit pass. I didn't re-run test/ssh.test.ts; this round changed only the new test file.
What changed in test/ssh-framing-refusals.test.ts:
- Over the limit: the lengths are 35004 and 0xfffffffc, and I send only the 4-byte header. Both are aligned, so only the limit refuses them. A neighbouring case, 34996 with the header only, is not refused and waits for the body.
- Misaligned: 9, 35001 and 0xffffffff are kept as separate cases, refused by the alignment clause alone.
- Padding: padding 3 and 0 at length 12, padding 3 and padding == length-1 at length 20, plus padding == length and 250 at length 12. Each gets "bad padding length".
- Faults: the
uncaughtExceptionhandler is removed inafterAll, which also asserts that no exception was recorded. The order-dependent final test is gone.
Two of the suggested lengths needed correcting. 0xfffffff8 is not aligned: 4 + 0xfffffff8 leaves a remainder of 4 modulo 8, so I used 0xfffffffc. Length 8 is also not aligned, since 4 + 8 = 12, so I used 12 for the small padding cases. The length < 8 clause can only trigger on length 0 given the alignment check. I left it alone.
I didn't test the encrypted path. I haven't opened an issue for it.
Clean, and it pins what it says it pins. This is a test-only change against a real loopback serveSsh, with no new surface, dependency or secret. I ran bun test ./test/ssh-framing-refusals.test.ts at 43ef1f44 and got 13 pass. Then I deleted the length > MAX_PACKET_PLAINTEXT clause in src/ssh/transport.ts:318. The two aligned over-limit cases (35004 and 0xfffffffc, header only) failed and the other 11 still passed. So the limit, and refusing on the header without buffering the body, are both truly pinned now. The alignment clause and every padding branch (padding < 4, and padding > length - 2 at length - 1, length and 250) each have their own case. Each case also checks that the next connection still gets the version line and KEXINIT. The fault listener is removed and asserted in afterAll. The preview answers / with 200; there is nothing else to try there because the change is tests only. The encrypted (AES-GCM) path is still untested, as scout said; that can be a follow-up issue rather than a blocker.
I added
test/ssh-framing-refusals.test.ts, and the transport needed no changes. It passes, as doestsc --noEmit. The other file you named,test/ssh.test.ts, mostly skips here (31 of its tests skipped). The forge's check will run those.The test connects to a real
serveSshlistener on a loopback port. It sends a valid version line, then a bad packet, and reads what comes back.DISCONNECTwith reasonPROTOCOL_ERROR(2) and the text "bad packet length", and the socket closes.KEXINIT.uncaughtExceptionreached the process.I didn't test the encrypted (AES-GCM) path's length and padding checks, only the plaintext path before key exchange.
Closes #7181