mirror of
https://github.com/stablyai/orca.git
synced 2026-09-23 00:02:29 +00:00
fix(relay): prove the optional handshake field too, not just the required ones
The parser refuses a non-string `version`, `expected` and `got`, then returns the object with `endpointCredential` unproved — the most pre-auth field on the frame. It is safe today only by accident: its one reader compares it, and a non-string loses that comparison. Nothing holds that shape in place, and the next reader to put it in a log line reinstates the template-literal throw this function exists to stop. Present-but-not-a-string is now refused at the parser. Absent stays absent: a bridge presenting no credential is the common case, and refusing it would close every unauthenticated-endpoint connection. Wire-visible delta, deliberate: a peer sending a non-string credential used to get `orca-relay-handshake-credential-mismatch` and exit 43; it now gets a bare close. No first-party client can reach it — `runConnectHandshake` types the parameter `string` and omits it when falsy — and a bare close is the right answer to a frame that was malformed before any credential was checked.
This commit is contained in:
@@ -100,6 +100,36 @@ describe('handshake framing', () => {
|
||||
}
|
||||
})
|
||||
|
||||
// endpointCredential is the one optional field, and it is the most pre-auth thing on the frame.
|
||||
// Its only reader compares it, so a non-string refuses today by inequality rather than by type —
|
||||
// which is luck, not a guarantee. Prove it at the parser, where every reader shares it.
|
||||
it('rejects a present endpointCredential that is not a string', () => {
|
||||
for (const endpointCredential of [{ toString: 1 }, 7, null, ['secret'], true]) {
|
||||
const payload = Buffer.from(
|
||||
JSON.stringify({ type: 'orca-relay-handshake', version: '0.1.0', endpointCredential })
|
||||
)
|
||||
expect(
|
||||
() => parseHandshakeMessage(payload),
|
||||
`endpointCredential=${JSON.stringify(endpointCredential)}`
|
||||
).toThrow(/Handshake field endpointCredential is not a string/)
|
||||
}
|
||||
})
|
||||
|
||||
// Absent must stay absent: a bridge that legitimately presents no credential is the common case,
|
||||
// and refusing it here would close every unauthenticated-endpoint connection in the fleet.
|
||||
it('still accepts a handshake with no endpointCredential, and one with a string', () => {
|
||||
const bare = Buffer.from(JSON.stringify({ type: 'orca-relay-handshake', version: '0.1.0' }))
|
||||
expect(parseHandshakeMessage(bare)).toEqual({ type: 'orca-relay-handshake', version: '0.1.0' })
|
||||
const withCredential = Buffer.from(
|
||||
JSON.stringify({ type: 'orca-relay-handshake', version: '0.1.0', endpointCredential: 'sec' })
|
||||
)
|
||||
expect(parseHandshakeMessage(withCredential)).toEqual({
|
||||
type: 'orca-relay-handshake',
|
||||
version: '0.1.0',
|
||||
endpointCredential: 'sec'
|
||||
})
|
||||
})
|
||||
|
||||
it('still accepts a credential-mismatch reply, which carries no fields', () => {
|
||||
const payload = Buffer.from(
|
||||
JSON.stringify({ type: 'orca-relay-handshake-credential-mismatch' })
|
||||
|
||||
@@ -60,6 +60,20 @@ const HANDSHAKE_STRING_FIELDS: Readonly<Record<HandshakeMessage['type'], readonl
|
||||
'orca-relay-handshake-credential-mismatch': []
|
||||
}
|
||||
|
||||
// Optional fields are peer-supplied too, so the parser only proves the type of what it returns if
|
||||
// it refuses a present-but-wrong one. `endpointCredential` survives today only because its single
|
||||
// reader compares it and never interpolates it; the next reader to log it would restore the bug
|
||||
// this function exists to stop. Absent stays absent — refusing that would break a bridge that
|
||||
// legitimately presents no credential.
|
||||
const HANDSHAKE_OPTIONAL_STRING_FIELDS: Readonly<
|
||||
Record<HandshakeMessage['type'], readonly string[]>
|
||||
> = {
|
||||
'orca-relay-handshake': ['endpointCredential'],
|
||||
'orca-relay-handshake-ok': [],
|
||||
'orca-relay-handshake-mismatch': [],
|
||||
'orca-relay-handshake-credential-mismatch': []
|
||||
}
|
||||
|
||||
export function parseHandshakeMessage(payload: Buffer): HandshakeMessage {
|
||||
const parsed: unknown = JSON.parse(payload.toString('utf-8'))
|
||||
if (typeof parsed !== 'object' || parsed === null) {
|
||||
@@ -82,6 +96,11 @@ export function parseHandshakeMessage(payload: Buffer): HandshakeMessage {
|
||||
throw new Error(`Handshake field ${field} is not a string`)
|
||||
}
|
||||
}
|
||||
for (const field of HANDSHAKE_OPTIONAL_STRING_FIELDS[t as HandshakeMessage['type']]) {
|
||||
if (msg[field] !== undefined && typeof msg[field] !== 'string') {
|
||||
throw new Error(`Handshake field ${field} is not a string`)
|
||||
}
|
||||
}
|
||||
return msg as unknown as HandshakeMessage
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user