fix(relay): do not let a peer's version claim crash the daemon that logs it

This commit is contained in:
Neil
2026-09-11 01:19:59 -07:00
parent 0b568e6a07
commit f5cee3fbf4
5 changed files with 64 additions and 3 deletions
+2
View File
@@ -27,6 +27,7 @@ export type { DecodedFrame, FrameDecoderOptions } from './relay-frame-decoder'
import {
MIN_RELAY_PROTOCOL_VERSION,
RELAY_PROTOCOL_VERSION,
describeRelayProtocolVersion,
relayProtocolOffer,
relayProtocolOfferAdmits,
type RelayHandshakeCapabilities,
@@ -36,6 +37,7 @@ import {
export {
MIN_RELAY_PROTOCOL_VERSION,
RELAY_PROTOCOL_VERSION,
describeRelayProtocolVersion,
relayProtocolOffer,
relayProtocolOfferAdmits
}
@@ -354,6 +354,36 @@ describe('handshake round-trip over a real Socket pair', () => {
bridgeSock.destroy()
})
// The refusal path logs the peer's claim, and `JSON.parse` yields objects a template literal
// cannot stringify. A throw there is inside the frame-decoder callback, so it would kill the
// daemon — and every PTY it still holds — on an unauthenticated frame.
it('refuses a hostile protocolVersion claim without taking the daemon down', async () => {
const { accepted } = await startDaemon('0.1.0+server-version')
const bridgeSock = connect(sockPath)
await new Promise<void>((r) => bridgeSock.once('connect', () => r()))
const reply = readDaemonReply(bridgeSock)
bridgeSock.write(
encodeHandshakeFrame(
JSON.parse(
'{"type":"orca-relay-handshake","version":"0.1.0+different","protocolVersion":{"toString":1}}'
) as HandshakeMessage
)
)
await expect(reply).resolves.toMatchObject({ type: 'orca-relay-handshake-mismatch' })
// Still serving: a second, well-formed client is admitted after the hostile one.
const good = connect(sockPath)
await new Promise<void>((r) => good.once('connect', () => r()))
runConnectHandshake(good, '0.1.0+server-version', { onAccepted: vi.fn() })
await accepted
bridgeSock.destroy()
good.destroy()
})
// Tolerance replaces a compatibility gate, never the auth gate: the credential file lives inside
// the incumbent's own install dir, so presenting it is what proves the caller may reach it.
it('still refuses a cross-build bridge that cannot present the endpoint credential', async () => {
+4 -3
View File
@@ -10,6 +10,7 @@ import {
FrameDecoder,
encodeHandshakeFrame,
parseHandshakeMessage,
describeRelayProtocolVersion,
relayProtocolOffer,
relayProtocolOfferAdmits,
type DecodedFrame,
@@ -140,7 +141,7 @@ function handleDaemonHandshakeFrame(
if (!negotiated) {
relayLogLine(
`[relay] Handshake mismatch: own=${launchVersion}/p${RELAY_PROTOCOL_VERSION}, ` +
`client=${msg.version}/p${msg.protocolVersion ?? 'none'}; closing socket`
`client=${msg.version}/p${describeRelayProtocolVersion(msg.protocolVersion)}; closing socket`
)
try {
sock.write(
@@ -224,7 +225,7 @@ export function runConnectHandshake(
// Why both numbers: with negotiation the daemon's build can legitimately differ from this
// bridge's, so the version alone no longer says which relay answered.
process.stderr.write(
`[relay-connect] Handshake OK at version=${msg.version} protocol=${msg.protocolVersion ?? 'none'}\n`
`[relay-connect] Handshake OK at version=${msg.version} protocol=${describeRelayProtocolVersion(msg.protocolVersion)}\n`
)
handshakeDone = true
const leftover = decoder.drain()
@@ -236,7 +237,7 @@ export function runConnectHandshake(
// Why: exit inside the write callback; stderr is async on pipe transports, so exiting early drops the version detail.
process.stderr.write(
`[relay-connect] Handshake mismatch: expected=${msg.expected}, daemon=${msg.got}, ` +
`daemonProtocol=${msg.protocolVersion ?? 'none'}, ours=${RELAY_PROTOCOL_VERSION}; ` +
`daemonProtocol=${describeRelayProtocolVersion(msg.protocolVersion)}, ours=${RELAY_PROTOCOL_VERSION}; ` +
`exiting ${EXIT_CODE_VERSION_MISMATCH}\n`,
() => {
sock.destroy()
+17
View File
@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'
import {
MIN_RELAY_PROTOCOL_VERSION,
RELAY_PROTOCOL_VERSION,
describeRelayProtocolVersion,
relayProtocolOffer,
relayProtocolOfferAdmits
} from './relay-protocol-version'
@@ -71,4 +72,20 @@ describe('relay protocol version', () => {
expect(relayProtocolOfferAdmits({ protocolVersion: 1, minProtocolVersion: 5 }, 1)).toBe(false)
expect(relayProtocolOfferAdmits({ protocolVersion: 1, minProtocolVersion: 5 }, 5)).toBe(false)
})
// A relay daemon logs the peer's claim on the refusal path, and `JSON.parse` can produce an
// object a template literal cannot stringify. Throwing there is inside the frame-decoder
// callback, which would take the daemon and every PTY it still holds down with it.
it('renders a peer version claim that a template literal would throw on', () => {
const hostile = JSON.parse('{"protocolVersion": {"toString": 1}}').protocolVersion
expect(() => `${hostile}`).toThrow()
expect(describeRelayProtocolVersion(hostile)).toBe('none')
})
it('renders real numbers and refuses every other shape', () => {
expect(describeRelayProtocolVersion(7)).toBe('7')
for (const bad of [undefined, null, '3', {}, [], Number.NaN, Number.POSITIVE_INFINITY]) {
expect(describeRelayProtocolVersion(bad), JSON.stringify(bad ?? null)).toBe('none')
}
})
})
+11
View File
@@ -50,6 +50,17 @@ function readVersion(value: unknown): number | null {
: null
}
/**
* Renders a peer's claimed version for a log line without interpolating it.
*
* `JSON.parse` can hand back `{ "toString": 1 }`, and a template literal on that throws
* `Cannot convert object to primitive value` — inside the frame-decoder callback, which would
* take the daemon and every PTY it holds down with it.
*/
export function describeRelayProtocolVersion(value: unknown): string {
return typeof value === 'number' && Number.isFinite(value) ? String(value) : 'none'
}
/** This build's own offer, sent on every handshake and reply. */
export function relayProtocolOffer(): Required<RelayProtocolOffer> {
return {