Skip to content

Commit 6fce643

Browse files
committed
fix(sdk-js): decode v0 hex strictly instead of silently truncating
Node's hex decoder stops at the first pair it cannot parse and returns the prefix, with no error. A `GetKey` response whose 64 valid digits are followed by junk therefore yielded a plausible 32-byte `key`, and `toViemAccount` turned that into a working account at an address nobody chose. Rust, Python and Go all refuse the identical response. v1 already guards this (`decode_hex` in `client-v1.ts`); the frozen v0 client never got the same treatment. Move v1's decoders into `shared.ts` -- which exists for exactly this -- and use them from both surfaces, so the two cannot drift again. Also from the same pass: a null or absent `signature_chain` threw a bare `TypeError: Cannot read properties of null (reading 'map')`, which names no field and reads like an SDK bug; and `info()` accepted a numeric `tcb_info`, because `JSON.parse` stringifies its argument, handing back `42` typed as `TcbInfo` with every `.mrtd` read `undefined`. Compat: this rejects three response shapes that previously returned a value -- malformed hex, a missing repeated field, and a non-string `tcb_info`. In every case the value returned was unusable (a truncated private key, an empty chain, a number typed as a struct), so no working application can depend on it. A well-formed response, an empty hex string and an empty chain are unchanged. `getKey`, `info` and `sign` now call `throwOnRpcError` like the rest of v0, so an RPC error response throws instead of being decoded as a result.
1 parent 81cb016 commit 6fce643

4 files changed

Lines changed: 203 additions & 102 deletions

File tree

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// SPDX-FileCopyrightText: © 2026 Phala Network <dstack@phala.network>
2+
//
3+
// SPDX-License-Identifier: Apache-2.0
4+
5+
import http from 'http'
6+
import type { AddressInfo } from 'net'
7+
import { describe, expect, it } from 'vitest'
8+
import { DstackClientV0 } from '../client-v0'
9+
10+
async function withAgentAnswering(body: unknown, fn: (client: DstackClientV0) => Promise<void>) {
11+
const server = http.createServer((_req, res) => {
12+
res.writeHead(200, { 'Content-Type': 'application/json' })
13+
res.end(JSON.stringify(body))
14+
})
15+
await new Promise<void>(resolve => server.listen(0, '127.0.0.1', () => resolve()))
16+
try {
17+
const { port } = server.address() as AddressInfo
18+
await fn(new DstackClientV0(`http://127.0.0.1:${port}`))
19+
} finally {
20+
await new Promise<void>(resolve => server.close(() => resolve()))
21+
}
22+
}
23+
24+
const KEY = '11'.repeat(32)
25+
const CHAIN = ['aa'.repeat(64)]
26+
27+
describe('DstackClientV0.getKey', () => {
28+
it.each([
29+
['valid prefix followed by junk', { key: KEY + 'GARBAGE', signature_chain: CHAIN }, /malformed key/],
30+
['non-hex key', { key: 'zz'.repeat(32), signature_chain: CHAIN }, /malformed key/],
31+
['odd-length key', { key: '0011222', signature_chain: CHAIN }, /malformed key/],
32+
['absent key', { signature_chain: CHAIN }, /no key/],
33+
['non-hex chain link', { key: KEY, signature_chain: ['aa', 'zz'] }, /signature_chain\[1\]/],
34+
['null signature_chain', { key: KEY, signature_chain: null }, /signature_chain/],
35+
['absent signature_chain', { key: KEY }, /signature_chain/],
36+
])('rejects %s', async (_, body, error) => {
37+
await withAgentAnswering(body, client => expect(client.getKey('d')).rejects.toThrow(error))
38+
})
39+
40+
it('accepts a well-formed response', async () => {
41+
await withAgentAnswering({ key: KEY, signature_chain: CHAIN }, async client => {
42+
const result = await client.getKey('d')
43+
expect(result.key).toEqual(new Uint8Array(Buffer.from(KEY, 'hex')))
44+
expect(result.signature_chain).toHaveLength(1)
45+
})
46+
})
47+
48+
it('reads an empty key as zero bytes', async () => {
49+
await withAgentAnswering({ key: '', signature_chain: [] }, async client => {
50+
expect((await client.getKey('d')).key).toEqual(new Uint8Array(0))
51+
})
52+
})
53+
})
54+
55+
describe('DstackClientV0.info', () => {
56+
const base = {
57+
app_id: 'aa'.repeat(32),
58+
instance_id: 'cc'.repeat(32),
59+
app_cert: 'x',
60+
app_name: 'demo',
61+
device_id: 'dd'.repeat(32),
62+
key_provider_info: '{}',
63+
compose_hash: 'bb'.repeat(32),
64+
}
65+
66+
it.each([
67+
['non-string tcb_info', { ...base, tcb_info: 42 }],
68+
['absent tcb_info', base],
69+
['unparseable tcb_info', { ...base, tcb_info: '{' }],
70+
])('rejects %s', async (_, body) => {
71+
await withAgentAnswering(body, client => expect(client.info()).rejects.toThrow(/tcb_info/))
72+
})
73+
74+
it('parses a well-formed tcb_info', async () => {
75+
await withAgentAnswering({ ...base, tcb_info: JSON.stringify({ mrtd: '00'.repeat(48) }) }, async client => {
76+
expect((await client.info()).tcb_info.mrtd).toBe('00'.repeat(48))
77+
})
78+
})
79+
})

‎sdk/js/src/client-v0.ts‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@
88

99
import fs from 'fs'
1010
import { send_rpc_request } from './send-rpc-request'
11-
import { to_hex, throwOnRpcError, resolveDstackEndpoint, type Hex } from './shared'
11+
import {
12+
to_hex, throwOnRpcError, resolveDstackEndpoint, type Hex,
13+
from_hex, require_string, to_list,
14+
} from './shared'
1215

1316
export interface GetTlsKeyResponse {
1417
__name__: Readonly<'GetTlsKeyResponse'>
@@ -131,6 +134,16 @@ function x509key_to_uint8array(pem: string, max_length?: number) {
131134
return result
132135
}
133136

137+
/** `JSON.parse` stringifies non-strings, so check the type first. */
138+
function parse_tcb_info<T extends TcbInfo>(value: unknown): T {
139+
const text = require_string(value, 'tcb_info')
140+
try {
141+
return JSON.parse(text) as T
142+
} catch (error) {
143+
throw new Error(`the agent returned a malformed tcb_info: ${(error as Error).message}`)
144+
}
145+
}
146+
134147
export interface TlsKeyOptions {
135148
subject?: string;
136149
altNames?: string[];
@@ -192,9 +205,11 @@ export class DstackClientV0<T extends TcbInfo = TcbInfoV05x> {
192205
algorithm: algorithm
193206
})
194207
const result = await send_rpc_request<{ key: string, signature_chain: string[] }>(this.endpoint, '/GetKey', payload)
208+
throwOnRpcError(result)
195209
return Object.freeze({
196-
key: new Uint8Array(Buffer.from(result.key, 'hex')),
197-
signature_chain: result.signature_chain.map(sig => new Uint8Array(Buffer.from(sig, 'hex'))),
210+
key: from_hex(result.key, 'key'),
211+
signature_chain: to_list(result.signature_chain, 'signature_chain', 'error')
212+
.map((sig, i) => from_hex(sig, `signature_chain[${i}]`)),
198213
__name__: 'GetKeyResponse',
199214
})
200215
}
@@ -291,9 +306,10 @@ export class DstackClientV0<T extends TcbInfo = TcbInfoV05x> {
291306

292307
async info(): Promise<InfoResponse<T>> {
293308
const result = await send_rpc_request<Omit<InfoResponse<TcbInfo>, 'tcb_info'> & { tcb_info: string }>(this.endpoint, '/Info', '{}')
309+
throwOnRpcError(result)
294310
return Object.freeze({
295311
...result,
296-
tcb_info: JSON.parse(result.tcb_info) as T,
312+
tcb_info: parse_tcb_info<T>(result.tcb_info),
297313
})
298314
}
299315

@@ -368,10 +384,13 @@ export class DstackClientV0<T extends TcbInfo = TcbInfoV05x> {
368384

369385
const result = await send_rpc_request<{ signature: string, signature_chain: string[], public_key: string }>(this.endpoint, '/Sign', payload);
370386

387+
throwOnRpcError(result)
388+
371389
return Object.freeze({
372-
signature: new Uint8Array(Buffer.from(result.signature, 'hex')),
373-
signature_chain: result.signature_chain.map(sig => new Uint8Array(Buffer.from(sig, 'hex'))),
374-
public_key: new Uint8Array(Buffer.from(result.public_key, 'hex')),
390+
signature: from_hex(result.signature, 'signature'),
391+
signature_chain: to_list(result.signature_chain, 'signature_chain', 'error')
392+
.map((sig, i) => from_hex(sig, `signature_chain[${i}]`)),
393+
public_key: from_hex(result.public_key, 'public_key'),
375394
__name__: 'SignResponse',
376395
});
377396
}

‎sdk/js/src/client-v1.ts‎

Lines changed: 4 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -6,57 +6,10 @@
66
// unsuffixed `DstackClient` names since 0.6.0.
77

88
import { send_rpc_request } from './send-rpc-request'
9-
import { to_hex, throwOnRpcError, resolveDstackEndpoint } from './shared'
10-
11-
/** An even number of hex digits, and nothing else. */
12-
const HEX_ONLY = /^(?:[0-9a-fA-F]{2})*$/
13-
14-
/**
15-
* Decode a wire hex string, or say which field was malformed.
16-
*
17-
* Strict on purpose. Node's hex decoder stops at the first pair it cannot
18-
* parse and returns the prefix it managed, without error: `Buffer.from(
19-
* '0102zz', 'hex')` is two bytes, and an odd-length string loses its last
20-
* digit. These fields are private keys, signature chain links and application
21-
* identity -- handing back a silently truncated one is worse than throwing,
22-
* and Rust, Python and Go all refuse the same input.
23-
*/
24-
function decode_hex(value: unknown, field: string): Uint8Array {
25-
// The type check is not redundant with the regex, and dropping it is a
26-
// silent-wrong-value bug rather than a style regression. `RegExp.test`
27-
// stringifies its argument, so a one-element array passes -- `['00112233']`
28-
// becomes `'00112233'` -- and `Buffer.from` then ignores the `'hex'`
29-
// argument for a non-string input and coerces the elements as octets:
30-
// `Number('00112233') & 0xff`, one attacker-chosen byte, no error. TypeScript
31-
// cannot stop this because a JSON response is `any` at runtime.
32-
if (typeof value !== 'string') {
33-
throw new Error(
34-
`the agent returned a malformed ${field}: expected a hex string, got ${
35-
value === null ? 'null' : Array.isArray(value) ? 'an array' : typeof value}`
36-
)
37-
}
38-
if (!HEX_ONLY.test(value)) {
39-
throw new Error(
40-
`the agent returned a malformed ${field}: expected an even-length hex string`
41-
)
42-
}
43-
return new Uint8Array(Buffer.from(value, 'hex'))
44-
}
45-
46-
/**
47-
* Decode a `bytes` field the proto declares required.
48-
*
49-
* Absence is an error rather than the empty default: `app_id` and `key` are
50-
* answers the agent always has, so a response without one is a response that
51-
* did not come from a working agent. An empty *string* still decodes to zero
52-
* bytes, which is what every other SDK does with it.
53-
*/
54-
function from_hex(value: unknown, field: string): Uint8Array {
55-
if (value === undefined) {
56-
throw new Error(`the agent returned no ${field}`)
57-
}
58-
return decode_hex(value, field)
59-
}
9+
import {
10+
to_hex, throwOnRpcError, resolveDstackEndpoint,
11+
decode_hex, from_hex, require_string, to_list,
12+
} from './shared'
6013

6114
/**
6215
* Decode a `bytes` field, treating an absent key as the empty default.
@@ -277,50 +230,6 @@ function to_string(value: unknown, field: string): string {
277230
return require_string(value, field)
278231
}
279232

280-
/**
281-
* Read a `string` field the response is meaningless without.
282-
*
283-
* A bundle's `vendor` and `format` are what a caller dispatches on to pick a
284-
* verifier, so handing back `undefined` there does not degrade the answer, it
285-
* routes the evidence to no verifier at all -- quietly, since `undefined`
286-
* matches no `case`. Rust and Python both make these required.
287-
*/
288-
function require_string(value: unknown, field: string): string {
289-
if (typeof value !== 'string') {
290-
throw new Error(
291-
`the agent returned a malformed ${field}: expected a string, got ${
292-
value === undefined ? 'nothing'
293-
: value === null ? 'null'
294-
: Array.isArray(value) ? 'an array' : typeof value}`
295-
)
296-
}
297-
return value
298-
}
299-
300-
/**
301-
* Read a `repeated` field, or say which one was not a list.
302-
*
303-
* `Array.isArray` rather than a truthiness check: a bare `.map()` on a `null`
304-
* or absent field throws `TypeError: Cannot read properties of null`, which
305-
* names no field and reads like an SDK bug rather than a bad response.
306-
*
307-
* `whenAbsent` follows the proto. A missing `boottime_gpu_evidence` is the
308-
* empty list, because the field is only populated when asked for; a missing
309-
* `bundles` or `signature_chain` is a malformed response, because those are
310-
* the whole answer of the call that returns them.
311-
*/
312-
function to_list(
313-
value: unknown, field: string, whenAbsent: 'empty' | 'error',
314-
): unknown[] {
315-
if (value === undefined && whenAbsent === 'empty') {
316-
return []
317-
}
318-
if (!Array.isArray(value)) {
319-
throw new Error(`the agent returned a malformed ${field}: expected a list`)
320-
}
321-
return value
322-
}
323-
324233
/**
325234
* Decode the bundles a v1 RPC returned.
326235
*

‎sdk/js/src/shared.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77

88
import fs from 'fs'
99

10+
/** An even number of hex digits, and nothing else. */
11+
const HEX_ONLY = /^(?:[0-9a-fA-F]{2})*$/
12+
1013
export type Hex = `${string}`
1114

1215
export function to_hex(data: string | Buffer | Uint8Array): string {
@@ -59,3 +62,94 @@ export function resolveDstackEndpoint(endpoint: string | undefined): string {
5962
}
6063
return endpoint
6164
}
65+
66+
/**
67+
* Decode a wire hex string, or say which field was malformed.
68+
*
69+
* Strict on purpose. Node's hex decoder stops at the first pair it cannot
70+
* parse and returns the prefix it managed, without error: `Buffer.from(
71+
* '0102zz', 'hex')` is two bytes, and an odd-length string loses its last
72+
* digit. These fields are private keys, signature chain links and application
73+
* identity -- handing back a silently truncated one is worse than throwing,
74+
* and Rust, Python and Go all refuse the same input.
75+
*/
76+
export function decode_hex(value: unknown, field: string): Uint8Array {
77+
// The type check is not redundant with the regex, and dropping it is a
78+
// silent-wrong-value bug rather than a style regression. `RegExp.test`
79+
// stringifies its argument, so a one-element array passes -- `['00112233']`
80+
// becomes `'00112233'` -- and `Buffer.from` then ignores the `'hex'`
81+
// argument for a non-string input and coerces the elements as octets:
82+
// `Number('00112233') & 0xff`, one attacker-chosen byte, no error. TypeScript
83+
// cannot stop this because a JSON response is `any` at runtime.
84+
if (typeof value !== 'string') {
85+
throw new Error(
86+
`the agent returned a malformed ${field}: expected a hex string, got ${
87+
value === null ? 'null' : Array.isArray(value) ? 'an array' : typeof value}`
88+
)
89+
}
90+
if (!HEX_ONLY.test(value)) {
91+
throw new Error(
92+
`the agent returned a malformed ${field}: expected an even-length hex string`
93+
)
94+
}
95+
return new Uint8Array(Buffer.from(value, 'hex'))
96+
}
97+
98+
/**
99+
* Decode a `bytes` field the proto declares required.
100+
*
101+
* Absence is an error rather than the empty default: `app_id` and `key` are
102+
* answers the agent always has, so a response without one is a response that
103+
* did not come from a working agent. An empty *string* still decodes to zero
104+
* bytes, which is what every other SDK does with it.
105+
*/
106+
export function from_hex(value: unknown, field: string): Uint8Array {
107+
if (value === undefined) {
108+
throw new Error(`the agent returned no ${field}`)
109+
}
110+
return decode_hex(value, field)
111+
}
112+
113+
/**
114+
* Read a `string` field the response is meaningless without.
115+
*
116+
* A bundle's `vendor` and `format` are what a caller dispatches on to pick a
117+
* verifier, so handing back `undefined` there does not degrade the answer, it
118+
* routes the evidence to no verifier at all -- quietly, since `undefined`
119+
* matches no `case`. Rust and Python both make these required.
120+
*/
121+
export function require_string(value: unknown, field: string): string {
122+
if (typeof value !== 'string') {
123+
throw new Error(
124+
`the agent returned a malformed ${field}: expected a string, got ${
125+
value === undefined ? 'nothing'
126+
: value === null ? 'null'
127+
: Array.isArray(value) ? 'an array' : typeof value}`
128+
)
129+
}
130+
return value
131+
}
132+
133+
/**
134+
* Read a `repeated` field, or say which one was not a list.
135+
*
136+
* `Array.isArray` rather than a truthiness check: a bare `.map()` on a `null`
137+
* or absent field throws `TypeError: Cannot read properties of null`, which
138+
* names no field and reads like an SDK bug rather than a bad response.
139+
*
140+
* `whenAbsent` follows the proto. A missing `boottime_gpu_evidence` is the
141+
* empty list, because the field is only populated when asked for; a missing
142+
* `bundles` or `signature_chain` is a malformed response, because those are
143+
* the whole answer of the call that returns them.
144+
*/
145+
export function to_list(
146+
value: unknown, field: string, whenAbsent: 'empty' | 'error',
147+
): unknown[] {
148+
if (value === undefined && whenAbsent === 'empty') {
149+
return []
150+
}
151+
if (!Array.isArray(value)) {
152+
throw new Error(`the agent returned a malformed ${field}: expected a list`)
153+
}
154+
return value
155+
}

0 commit comments

Comments
 (0)