diff --git a/src/main/ssh/ssh-relay-handshake-mismatch.test.ts b/src/main/ssh/ssh-relay-handshake-mismatch.test.ts new file mode 100644 index 00000000000..2fae7cc5716 --- /dev/null +++ b/src/main/ssh/ssh-relay-handshake-mismatch.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest' +import { buildRelayHandshakeRefusalError } from './ssh-relay-handshake-mismatch' +import { + RelayVersionMismatchError, + RELAY_EXIT_CODE_VERSION_MISMATCH +} from './ssh-relay-version-mismatch-error' +import { + RelayCredentialMismatchError, + RELAY_EXIT_CODE_CREDENTIAL_MISMATCH +} from './ssh-relay-credential-mismatch-error' + +describe('buildRelayHandshakeRefusalError', () => { + // The bridge's mismatch line now carries `daemonProtocol=` and `ours=` after the version, so a + // `daemon=` capture that merely stops at `;` swallows the separating comma into the version. + it('names the daemon version without the comma that follows it', () => { + const error = buildRelayHandshakeRefusalError( + RELAY_EXIT_CODE_VERSION_MISMATCH, + '[relay-connect] Handshake mismatch: expected=0.1.0+aaa, daemon=0.1.0+bbb, ' + + 'daemonProtocol=6, ours=1; exiting 42\n' + ) + expect(error).toBeInstanceOf(RelayVersionMismatchError) + expect(error).toMatchObject({ expected: '0.1.0+aaa', got: '0.1.0+bbb' }) + }) + + // A relay from before the protocol fields still emits the short form. + it('still reads the pre-negotiation form of the line', () => { + const error = buildRelayHandshakeRefusalError( + RELAY_EXIT_CODE_VERSION_MISMATCH, + '[relay-connect] Handshake mismatch: expected=0.1.0+aaa, daemon=0.1.0+bbb; exiting 42\n' + ) + expect(error).toMatchObject({ expected: '0.1.0+aaa', got: '0.1.0+bbb' }) + }) + + it('reports an unparseable mismatch line without inventing versions', () => { + const error = buildRelayHandshakeRefusalError(RELAY_EXIT_CODE_VERSION_MISMATCH, 'nothing here') + expect(error).toMatchObject({ expected: undefined, got: undefined }) + }) + + it('keeps a refused credential distinct from a version skew', () => { + expect( + buildRelayHandshakeRefusalError(RELAY_EXIT_CODE_CREDENTIAL_MISMATCH, 'refused') + ).toBeInstanceOf(RelayCredentialMismatchError) + }) + + it('returns nothing for an exit code that encodes no refusal', () => { + expect(buildRelayHandshakeRefusalError(1, 'crashed')).toBeNull() + }) +}) diff --git a/src/main/ssh/ssh-relay-handshake-mismatch.ts b/src/main/ssh/ssh-relay-handshake-mismatch.ts index 90841675e3f..646a14ff9c4 100644 --- a/src/main/ssh/ssh-relay-handshake-mismatch.ts +++ b/src/main/ssh/ssh-relay-handshake-mismatch.ts @@ -24,11 +24,13 @@ export function buildRelayHandshakeRefusalError( // Why: extract the expected/got version pair from --connect's stderr line // "Handshake mismatch: expected=, daemon=" so diagnostics name both versions. +// Why a comma is excluded from `daemon`: the line grew trailing `daemonProtocol=`/`ours=` +// fields, so the version is no longer the last thing before the `;`. function parseHandshakeMismatchStderr(stderr: string): { expected: string | undefined got: string | undefined } { - const match = /expected=([^,\s]+),\s*daemon=([^\s;]+)/.exec(stderr) + const match = /expected=([^,\s]+),\s*daemon=([^\s;,]+)/.exec(stderr) if (!match) { return { expected: undefined, got: undefined } } diff --git a/src/relay/protocol.ts b/src/relay/protocol.ts index 0f31b448f55..3f5db21e57d 100644 --- a/src/relay/protocol.ts +++ b/src/relay/protocol.ts @@ -24,6 +24,27 @@ export { } export type { DecodedFrame, FrameDecoderOptions } from './relay-frame-decoder' +import { + MIN_RELAY_PROTOCOL_VERSION, + RELAY_PROTOCOL_VERSION, + describeRelayProtocolVersion, + relayProtocolOffer, + relayProtocolOfferAdmits, + type RelayHandshakeCapabilities, + type RelayProtocolOffer +} from '../shared/relay-protocol-version' + +export { + MIN_RELAY_PROTOCOL_VERSION, + RELAY_PROTOCOL_VERSION, + describeRelayProtocolVersion, + relayProtocolOffer, + relayProtocolOfferAdmits +} +export type { RelayHandshakeCapabilities, RelayProtocolOffer } + +// Why frozen at 0.1.0: the deploy path carries a content hash of the bundle instead, so nothing +// ever needed this to move. `RELAY_PROTOCOL_VERSION` is the number that describes the wire. export const RELAY_VERSION = '0.1.0' export const RELAY_SENTINEL = `ORCA-RELAY v${RELAY_VERSION} READY\n` @@ -37,10 +58,24 @@ export const MessageType = { // reads exactly one Handshake frame before attaching the JSON-RPC dispatcher, // to refuse mismatched-version --connect bridges that would otherwise drive a // stale daemon. +// Why optional on every arm: `parseHandshakeMessage` throws on an unknown `type` and the throw +// closes the socket, so this envelope can only ever grow by optional fields on the arms that +// already ship. `endpointCredential` is the precedent. See +// docs/reference/remote-wire-compatibility.md Rule 1. export type HandshakeMessage = - | { type: 'orca-relay-handshake'; version: string; endpointCredential?: string } - | { type: 'orca-relay-handshake-ok'; version: string } - | { type: 'orca-relay-handshake-mismatch'; expected: string; got: string } + | ({ + type: 'orca-relay-handshake' + version: string + endpointCredential?: string + } & RelayProtocolOffer) + | ({ + type: 'orca-relay-handshake-ok' + version: string + capabilities?: RelayHandshakeCapabilities + } & RelayProtocolOffer) + // `protocolVersion` here tells a client whose offered range missed *what* it missed, which a + // content hash cannot: the two builds may be wire-compatible and merely differ in bytes. + | ({ type: 'orca-relay-handshake-mismatch'; expected: string; got: string } & RelayProtocolOffer) // Why a distinct reply: the bridge exits with its own code so the client can tell a refused // credential from a crashed relay. Old bridges reject the unknown type and exit 1 pre-sentinel. | { type: 'orca-relay-handshake-credential-mismatch' } diff --git a/src/relay/relay-handshake-roundtrip.test.ts b/src/relay/relay-handshake-roundtrip.test.ts index 0713d62295e..68148f1ef8b 100644 --- a/src/relay/relay-handshake-roundtrip.test.ts +++ b/src/relay/relay-handshake-roundtrip.test.ts @@ -13,9 +13,14 @@ import { encodeHandshakeFrame, encodeJsonRpcFrame, FrameDecoder, + parseHandshakeMessage, + MIN_RELAY_PROTOCOL_VERSION, + RELAY_PROTOCOL_VERSION, type DecodedFrame, + type HandshakeMessage, MessageType } from './protocol' +import { PTY_CONSUMER_SESSION_PROTOCOL_VERSION } from '../shared/pty-consumer-session-contract' import { relayTestSocketPath } from './relay-test-socket-path' // Why: --connect normally calls process.exit on mismatch / fatal handshake @@ -210,9 +215,86 @@ describe('handshake round-trip over a real Socket pair', () => { bridgeSock.destroy() }) - it('exits with EXIT_CODE_VERSION_MISMATCH when the daemon reports a mismatch', async () => { + // Reads the daemon's single handshake reply off a raw socket, so a test can assert what the + // envelope carries rather than only whether the bridge accepted it. + function readDaemonReply(sock: Socket): Promise { + return new Promise((resolve) => { + const decoder = new FrameDecoder((frame) => { + if (frame.type === MessageType.Handshake) { + resolve(parseHandshakeMessage(frame.payload)) + } + }) + sock.on('data', (chunk: Buffer) => decoder.feed(chunk)) + }) + } + + it('refuses a cross-build bridge that offers no protocol range at all', async () => { + // Why raw bytes: runConnectHandshake now always offers a range. Every relay already deployed + // sends none, and for those the build hash stays the only gate — this is that fallback. await startDaemon('0.1.0+server-version') + const bridgeSock = connect(sockPath) + await new Promise((r) => bridgeSock.once('connect', () => r())) + const reply = readDaemonReply(bridgeSock) + + bridgeSock.write( + encodeHandshakeFrame({ type: 'orca-relay-handshake', version: '0.1.0+different' }) + ) + + // The refusal now names the daemon's protocol version, which a content hash cannot express. + await expect(reply).resolves.toMatchObject({ + type: 'orca-relay-handshake-mismatch', + expected: '0.1.0+server-version', + got: '0.1.0+different', + protocolVersion: RELAY_PROTOCOL_VERSION, + minProtocolVersion: MIN_RELAY_PROTOCOL_VERSION + }) + + bridgeSock.destroy() + }) + + it('refuses a cross-build bridge whose offered range excludes this daemon', async () => { + await startDaemon('0.1.0+server-version') + + const bridgeSock = connect(sockPath) + await new Promise((r) => bridgeSock.once('connect', () => r())) + const reply = readDaemonReply(bridgeSock) + + bridgeSock.write( + encodeHandshakeFrame({ + type: 'orca-relay-handshake', + version: '0.1.0+different', + protocolVersion: RELAY_PROTOCOL_VERSION + 5, + minProtocolVersion: RELAY_PROTOCOL_VERSION + 5 + }) + ) + + await expect(reply).resolves.toMatchObject({ type: 'orca-relay-handshake-mismatch' }) + + bridgeSock.destroy() + }) + + it('exits with EXIT_CODE_VERSION_MISMATCH when the daemon reports a mismatch', async () => { + server = createServer((sock) => { + trackServerSocket(sock) + const decoder = new FrameDecoder((frame) => { + if (frame.type !== MessageType.Handshake) { + return + } + sock.write( + encodeHandshakeFrame({ + type: 'orca-relay-handshake-mismatch', + expected: '0.1.0+server-version', + got: '0.1.0+different', + protocolVersion: RELAY_PROTOCOL_VERSION + 5, + minProtocolVersion: RELAY_PROTOCOL_VERSION + 5 + }) + ) + }) + sock.on('data', (chunk: Buffer) => decoder.feed(chunk)) + }) + await new Promise((r) => server.listen(sockPath, () => r())) + const bridgeSock = connect(sockPath) await new Promise((r) => bridgeSock.once('connect', () => r())) @@ -226,6 +308,107 @@ describe('handshake round-trip over a real Socket pair', () => { bridgeSock.destroy() }) + // #13852: after an app update the client's bundle hashes differently, so the incumbent relay + // holding every live PTY refused it and that work became permanently unreachable. + it('admits a bridge from a different build once it offers a range this daemon falls in', async () => { + const { accepted } = await startDaemon('0.1.0+incumbent-holding-live-ptys') + + const bridgeSock = connect(sockPath) + await new Promise((r) => bridgeSock.once('connect', () => r())) + + const acceptedCb = vi.fn<(leftover: Buffer) => void>() + runConnectHandshake(bridgeSock, '0.1.0+freshly-updated-client', { onAccepted: acceptedCb }) + + await accepted + await vi.waitFor(() => expect(acceptedCb).toHaveBeenCalledTimes(1)) + expect(exitSpy).not.toHaveBeenCalled() + + bridgeSock.destroy() + }) + + it('answers a cross-build bridge with the protocol version and capabilities it must speak', async () => { + await startDaemon('0.1.0+incumbent-holding-live-ptys') + + const bridgeSock = connect(sockPath) + await new Promise((r) => bridgeSock.once('connect', () => r())) + const reply = readDaemonReply(bridgeSock) + + bridgeSock.write( + encodeHandshakeFrame({ + type: 'orca-relay-handshake', + version: '0.1.0+freshly-updated-client', + protocolVersion: RELAY_PROTOCOL_VERSION + 3, + minProtocolVersion: MIN_RELAY_PROTOCOL_VERSION + }) + ) + + await expect(reply).resolves.toMatchObject({ + type: 'orca-relay-handshake-ok', + // The build the client actually reached, which is no longer its own. + version: '0.1.0+incumbent-holding-live-ptys', + protocolVersion: RELAY_PROTOCOL_VERSION, + minProtocolVersion: MIN_RELAY_PROTOCOL_VERSION, + capabilities: { ptyConsumerSession: PTY_CONSUMER_SESSION_PROTOCOL_VERSION } + }) + + 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((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((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 () => { + const { accepted } = await startDaemon('0.1.0+incumbent', 'secret-credential') + + const bridgeSock = connect(sockPath) + await new Promise((r) => bridgeSock.once('connect', () => r())) + const closed = new Promise((r) => bridgeSock.once('close', () => r())) + + runConnectHandshake( + bridgeSock, + '0.1.0+freshly-updated-client', + { onAccepted: vi.fn() }, + 'wrong-credential' + ) + + await closed + await expect( + Promise.race([ + accepted.then(() => 'accepted'), + new Promise((r) => setTimeout(() => r('closed'), 20)) + ]) + ).resolves.toBe('closed') + }) + it('does not call onAccepted before any handshake-ok frame arrives', async () => { // Why: silent server that never replies. acceptedCb must stay // un-invoked even though the bridge has flushed its handshake frame. diff --git a/src/relay/relay-handshake.ts b/src/relay/relay-handshake.ts index ed76baf024d..27505a6fa06 100644 --- a/src/relay/relay-handshake.ts +++ b/src/relay/relay-handshake.ts @@ -5,14 +5,26 @@ import { existsSync, readFileSync, realpathSync } from 'node:fs' import type { Socket } from 'node:net' import { RELAY_VERSION, + RELAY_PROTOCOL_VERSION, MessageType, FrameDecoder, encodeHandshakeFrame, parseHandshakeMessage, - type DecodedFrame + describeRelayProtocolVersion, + relayProtocolOffer, + relayProtocolOfferAdmits, + type DecodedFrame, + type RelayHandshakeCapabilities } from './protocol' +import { PTY_CONSUMER_SESSION_PROTOCOL_VERSION } from '../shared/pty-consumer-session-contract' import { relayLogLine } from './relay-diagnostic-log' +// Contracts that turn over independently of the handshake integer, so a client cannot infer them +// from `protocolVersion` and must not have to infer them from the build hash either. +function relayHandshakeCapabilities(): RelayHandshakeCapabilities { + return { ptyConsumerSession: PTY_CONSUMER_SESSION_PROTOCOL_VERSION } +} + // Why: clients treat this exit code as non-retryable; other non-zero exits are transient. export const EXIT_CODE_VERSION_MISMATCH = 42 // Why distinct from 42: a refused credential is a live daemon saying no, which the client must @@ -121,16 +133,23 @@ function handleDaemonHandshakeFrame( sock.destroy() return false } - if (msg.version !== launchVersion) { + // Why the build hash is still first: it is the only gate every already-deployed relay has, and + // it stays the fallback for peers that offer no protocol range at all. Negotiation is what lets + // a relay stranded by an app update keep serving the PTYs it already owns (#13852) — the hash + // differs by construction there, while the wire is unchanged. + const negotiated = msg.version === launchVersion || relayProtocolOfferAdmits(msg) + if (!negotiated) { relayLogLine( - `[relay] Handshake mismatch: own=${launchVersion}, client=${msg.version}; closing socket` + `[relay] Handshake mismatch: own=${launchVersion}/p${RELAY_PROTOCOL_VERSION}, ` + + `client=${msg.version}/p${describeRelayProtocolVersion(msg.protocolVersion)}; closing socket` ) try { sock.write( encodeHandshakeFrame({ type: 'orca-relay-handshake-mismatch', expected: launchVersion, - got: msg.version + got: msg.version, + ...relayProtocolOffer() }) ) } catch { @@ -150,8 +169,17 @@ function handleDaemonHandshakeFrame( sock.end() return false } - process.stderr.write(`[relay] Handshake OK from version=${msg.version}\n`) - sock.write(encodeHandshakeFrame({ type: 'orca-relay-handshake-ok', version: launchVersion })) + process.stderr.write( + `[relay] Handshake OK from version=${msg.version} (${msg.version === launchVersion ? 'same build' : `cross-build, protocol ${RELAY_PROTOCOL_VERSION}`})\n` + ) + sock.write( + encodeHandshakeFrame({ + type: 'orca-relay-handshake-ok', + version: launchVersion, + ...relayProtocolOffer(), + capabilities: relayHandshakeCapabilities() + }) + ) return true } @@ -194,7 +222,11 @@ export function runConnectHandshake( process.exit(1) } if (msg.type === 'orca-relay-handshake-ok') { - process.stderr.write(`[relay-connect] Handshake OK at version=${msg.version}\n`) + // 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=${describeRelayProtocolVersion(msg.protocolVersion)}\n` + ) handshakeDone = true const leftover = decoder.drain() sock.removeAllListeners('data') @@ -204,7 +236,9 @@ export function runConnectHandshake( if (msg.type === 'orca-relay-handshake-mismatch') { // 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}; exiting ${EXIT_CODE_VERSION_MISMATCH}\n`, + `[relay-connect] Handshake mismatch: expected=${msg.expected}, daemon=${msg.got}, ` + + `daemonProtocol=${describeRelayProtocolVersion(msg.protocolVersion)}, ours=${RELAY_PROTOCOL_VERSION}; ` + + `exiting ${EXIT_CODE_VERSION_MISMATCH}\n`, () => { sock.destroy() process.exit(EXIT_CODE_VERSION_MISMATCH) @@ -243,7 +277,9 @@ export function runConnectHandshake( encodeHandshakeFrame({ type: 'orca-relay-handshake', version: myVersion, - ...(endpointCredential ? { endpointCredential } : {}) + ...(endpointCredential ? { endpointCredential } : {}), + // A relay that predates the offer ignores these keys and compares the hash as before. + ...relayProtocolOffer() }) ) } diff --git a/src/shared/relay-protocol-version.test.ts b/src/shared/relay-protocol-version.test.ts new file mode 100644 index 00000000000..0fcfeba228d --- /dev/null +++ b/src/shared/relay-protocol-version.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, it } from 'vitest' +import { + MIN_RELAY_PROTOCOL_VERSION, + RELAY_PROTOCOL_VERSION, + describeRelayProtocolVersion, + relayProtocolOffer, + relayProtocolOfferAdmits +} from './relay-protocol-version' + +describe('relay protocol version', () => { + it('is a whole number at or above its own floor', () => { + expect(Number.isSafeInteger(RELAY_PROTOCOL_VERSION)).toBe(true) + expect(Number.isSafeInteger(MIN_RELAY_PROTOCOL_VERSION)).toBe(true) + expect(RELAY_PROTOCOL_VERSION).toBeGreaterThanOrEqual(MIN_RELAY_PROTOCOL_VERSION) + }) + + it('offers its own range, so a same-build peer is admitted by negotiation as well as by hash', () => { + expect(relayProtocolOfferAdmits(relayProtocolOffer())).toBe(true) + }) + + // The whole point of Release N: the stranded relay is always the OLDER side, and it must be + // able to say yes on the first frame with no earlier round trip to downgrade in. + it('admits a newer client whose range reaches back to this relay', () => { + expect(relayProtocolOfferAdmits({ protocolVersion: 9, minProtocolVersion: 1 }, 1)).toBe(true) + expect(relayProtocolOfferAdmits({ protocolVersion: 9, minProtocolVersion: 5 }, 1)).toBe(false) + }) + + it('admits a client older than this relay when the relay still falls in its range', () => { + expect(relayProtocolOfferAdmits({ protocolVersion: 4, minProtocolVersion: 2 }, 3)).toBe(true) + expect(relayProtocolOfferAdmits({ protocolVersion: 2, minProtocolVersion: 1 }, 3)).toBe(false) + }) + + it('treats a lone protocolVersion as a range of exactly one', () => { + expect(relayProtocolOfferAdmits({ protocolVersion: 3 }, 3)).toBe(true) + expect(relayProtocolOfferAdmits({ protocolVersion: 3 }, 2)).toBe(false) + expect(relayProtocolOfferAdmits({ protocolVersion: 3 }, 4)).toBe(false) + }) + + // A peer that sends no offer predates negotiation; admitting it would hand every unversioned + // caller the cross-build path that the build hash is supposed to gate. + it('never admits an absent or empty offer', () => { + expect(relayProtocolOfferAdmits(undefined, 1)).toBe(false) + expect(relayProtocolOfferAdmits({}, 1)).toBe(false) + expect(relayProtocolOfferAdmits({ minProtocolVersion: 1 }, 1)).toBe(false) + }) + + // Zero is the one bad value the band check cannot catch on its own: a peer claiming protocol 0 + // would be admitted by a relay whose own version was also read as 0. + it('rejects a zero protocol version rather than treating it as a real version', () => { + expect(relayProtocolOfferAdmits({ protocolVersion: 0, minProtocolVersion: 0 }, 0)).toBe(false) + }) + + it('rejects offers that are not plausible integers', () => { + for (const bad of [0, -1, 1.5, Number.NaN, Number.POSITIVE_INFINITY, 2_000_000]) { + expect( + relayProtocolOfferAdmits({ protocolVersion: bad, minProtocolVersion: 1 }, 1), + `protocolVersion=${bad}` + ).toBe(false) + } + for (const bad of ['1', null, {}, []]) { + expect( + relayProtocolOfferAdmits( + { protocolVersion: 1, minProtocolVersion: bad as unknown as number }, + 1 + ), + `minProtocolVersion=${JSON.stringify(bad)}` + ).toBe(false) + } + }) + + it('rejects an inverted range instead of silently reordering it', () => { + 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') + } + }) +}) diff --git a/src/shared/relay-protocol-version.ts b/src/shared/relay-protocol-version.ts new file mode 100644 index 00000000000..0a4c6303d9e --- /dev/null +++ b/src/shared/relay-protocol-version.ts @@ -0,0 +1,143 @@ +/** + * Semantic wire version for the relay handshake. + * + * `RELAY_VERSION` ('0.1.0') has never moved, so the deploy path is namespaced by a content hash + * of the bundle instead (`config/scripts/build-relay.mjs`). That hash changes on nearly every + * release, which makes every install directory — and the socket inside it — unreachable to the + * next build, stranding the PTYs and agents the incumbent still owns (#13852). + * + * This integer is the escape: it names what the *wire* can do, independent of what the bytes + * hash to, exactly as `PROTOCOL_VERSION` does for the local terminal daemon + * (`src/main/daemon/daemon-protocol-version.ts`). Bump it only when a client and a relay of + * adjacent versions genuinely cannot drive each other; a rebuild is not a reason. + */ +export const RELAY_PROTOCOL_VERSION = 1 + +/** + * ## This negotiation is not reachable yet. Do not read it as fixing #13852. + * + * Measured, not inferred: the deploy path namespaces the relay directory by the CLIENT'S OWN build + * hash — `remoteInstallDirName` is `relay-` — and the daemon socket lives inside that + * directory (`ssh-relay-deploy.ts`, `--connect --sock-path ~/.orca-remote/relay-/…`; + * the short-socket fallback derives its segment from the same version dir). Every + * `runConnectHandshake` caller takes its path from that one deploy result. So a bridge can only + * ever meet a daemon of its own build, `msg.version === launchVersion` short-circuits first, and + * `relayProtocolOfferAdmits` never decides anything on any live path. The stranded incumbent this + * exists for sits in `relay-/`, which nothing dials. + * + * What ships today is therefore three log lines: `describeRelayProtocolVersion` in + * `relay-handshake.ts`. `MIN_RELAY_PROTOCOL_VERSION` and the `minProtocolVersion` wire field have + * no live reader at all. + * + * Keep it — the mechanism is correct and hostile-input-safe, and it is the part that has to exist + * first. Making it live needs THREE more changes, all of which must land together. Landing only + * the first two ships a negotiation that still refuses, later and less legibly: + * 1. route the bridge to the incumbent's socket rather than to its own version directory; + * 2. stop `validateGrant` (`ssh-pty-consumer-session.ts`) refusing on `serverBuildId`. Its + * premise, "client and relay ship in one build", is still TRUE today and becomes false the + * moment (1) lands — it is a second gate that would refuse what the handshake just admitted. + * 3. give that same refusal a way to survive a protocol-version difference. It is ONE `if` with + * two disjuncts, and deleting the `serverBuildId` clause leaves the other one standing: + * `grant.protocolVersion !== PTY_CONSUMER_SESSION_PROTOCOL_VERSION` — a DIFFERENT constant + * from this file's `RELAY_PROTOCOL_VERSION`, compared for exact equality, with no range, no + * floor and no fallback. Two peers that just negotiated a compatible relay protocol are still + * refused if their PTY-session constants differ by one. And there is nothing to negotiate it + * with: `capabilities` on `orca-relay-handshake-ok` is written at exactly one site + * (`relay-handshake.ts`) and read by nothing outside tests, so (3) means giving that field a + * reader before it can carry the peer's PTY-session protocol. + * + * Budget a debugging session for the error text too: the refusal interpolates only the build ids, + * so a pure protocol-version mismatch reports as "expected build X, got X" with two IDENTICAL ids, + * which points at the gate that is not the problem. + * + * Until all three land, a change here cannot be validated by any end-to-end test, only by the + * handshake's own unit suite. + */ + +/** + * Oldest peer protocol this build can still speak. Raising it strands every relay below the new + * floor for good, so it moves only when serving a version is actually impossible. + * + * No live reader — see the note above. It is serialized onto every handshake and reply so that the + * field exists on the wire before any peer needs it (Rule 1, docs/reference/remote-wire- + * compatibility.md); an older peer ignoring it today is the point. + */ +export const MIN_RELAY_PROTOCOL_VERSION = 1 + +/** Rejects a peer's absurd or non-integer claim before it reaches a comparison. */ +const MAX_PLAUSIBLE_RELAY_PROTOCOL_VERSION = 1_000_000 + +/** + * Contract versions a peer cannot derive from `protocolVersion` alone, because they turn over + * independently of the handshake. + */ +export type RelayHandshakeCapabilities = { + /** `pty.openClient` grant contract this relay mints (`PTY_CONSUMER_SESSION_PROTOCOL_VERSION`). */ + readonly ptyConsumerSession?: number +} + +/** + * The protocol range a peer says it can speak. Both fields are optional on the wire: a relay or + * bridge that predates this negotiation sends neither, which is the signal to fall back to + * comparing build hashes. + */ +export type RelayProtocolOffer = { + readonly protocolVersion?: number + readonly minProtocolVersion?: number +} + +function readVersion(value: unknown): number | null { + return typeof value === 'number' && + Number.isSafeInteger(value) && + value >= 1 && + value <= MAX_PLAUSIBLE_RELAY_PROTOCOL_VERSION + ? value + : 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 { + return { + protocolVersion: RELAY_PROTOCOL_VERSION, + minProtocolVersion: MIN_RELAY_PROTOCOL_VERSION + } +} + +/** + * Whether a peer offering `offer` can be served by a relay speaking `ownVersion`. + * + * The peer states a range rather than a single number so an *older* relay can admit a *newer* + * client on the first frame — there is no earlier round trip in which the client could learn + * what to downgrade to, and the stranded relay is by construction the older side. + * + * An offer with no `protocolVersion` is not a negotiation at all and never admits: that is a + * pre-negotiation peer, and the build-hash comparison stays its only gate. + */ +export function relayProtocolOfferAdmits( + offer: RelayProtocolOffer | undefined, + ownVersion: number = RELAY_PROTOCOL_VERSION +): boolean { + const max = readVersion(offer?.protocolVersion) + if (max === null) { + return false + } + // Why default to `max`: a peer that names one version speaks exactly that one. + const min = offer?.minProtocolVersion === undefined ? max : readVersion(offer.minProtocolVersion) + if (min === null) { + return false + } + // An inverted range admits nothing rather than being reordered into something the peer never + // offered: `min > max` simply fails this band. + return ownVersion >= min && ownVersion <= max +}