Merge pull request #1347 from Dstack-TEE/fix/sdk-js-v0-strict-hex

fix(sdk-js): decode v0 hex strictly instead of silently truncating

Author: kvinwang

sdk/js/src/__tests__/client-v0-malformed.test.ts

@@ -0,0 +1,79 @@
+// SPDX-FileCopyrightText: © 2026 Phala Network <dstack@phala.network>
+//
+// SPDX-License-Identifier: Apache-2.0
+
+import http from 'http'
+import type { AddressInfo } from 'net'
+import { describe, expect, it } from 'vitest'
+import { DstackClientV0 } from '../client-v0'
+
+async function withAgentAnswering(body: unknown, fn: (client: DstackClientV0) => Promise<void>) {
+  const server = http.createServer((_req, res) => {
+    res.writeHead(200, { 'Content-Type': 'application/json' })
+    res.end(JSON.stringify(body))
+  })
+  await new Promise<void>(resolve => server.listen(0, '127.0.0.1', () => resolve()))
+  try {
+    const { port } = server.address() as AddressInfo
+    await fn(new DstackClientV0(`http://127.0.0.1:${port}`))
+  } finally {
+    await new Promise<void>(resolve => server.close(() => resolve()))
+  }
+}
+
+const KEY = '11'.repeat(32)
+const CHAIN = ['aa'.repeat(64)]
+
+describe('DstackClientV0.getKey', () => {
+  it.each([
+    ['valid prefix followed by junk', { key: KEY + 'GARBAGE', signature_chain: CHAIN }, /malformed key/],
+    ['non-hex key', { key: 'zz'.repeat(32), signature_chain: CHAIN }, /malformed key/],
+    ['odd-length key', { key: '0011222', signature_chain: CHAIN }, /malformed key/],
+    ['absent key', { signature_chain: CHAIN }, /no key/],
+    ['non-hex chain link', { key: KEY, signature_chain: ['aa', 'zz'] }, /signature_chain\[1\]/],
+    ['null signature_chain', { key: KEY, signature_chain: null }, /signature_chain/],
+    ['absent signature_chain', { key: KEY }, /signature_chain/],
+  ])('rejects %s', async (_, body, error) => {
+    await withAgentAnswering(body, client => expect(client.getKey('d')).rejects.toThrow(error))
+  })
+
+  it('accepts a well-formed response', async () => {
+    await withAgentAnswering({ key: KEY, signature_chain: CHAIN }, async client => {
+      const result = await client.getKey('d')
+      expect(result.key).toEqual(new Uint8Array(Buffer.from(KEY, 'hex')))
+      expect(result.signature_chain).toHaveLength(1)
+    })
+  })
+
+  it('reads an empty key as zero bytes', async () => {
+    await withAgentAnswering({ key: '', signature_chain: [] }, async client => {
+      expect((await client.getKey('d')).key).toEqual(new Uint8Array(0))
+    })
+  })
+})
+
+describe('DstackClientV0.info', () => {
+  const base = {
+    app_id: 'aa'.repeat(32),
+    instance_id: 'cc'.repeat(32),
+    app_cert: 'x',
+    app_name: 'demo',
+    device_id: 'dd'.repeat(32),
+    key_provider_info: '{}',
+    compose_hash: 'bb'.repeat(32),
+  }
+
+  it.each([
+    ['non-string tcb_info', { ...base, tcb_info: 42 }],
+    ['absent tcb_info', base],
+    ['unparseable tcb_info', { ...base, tcb_info: '{' }],
+  ])('rejects %s', async (_, body) => {
+    await withAgentAnswering(body, client => expect(client.info()).rejects.toThrow(/tcb_info/))
+  })
+
+  it('parses a well-formed tcb_info', async () => {
+    await withAgentAnswering({ ...base, tcb_info: JSON.stringify({ mrtd: '00'.repeat(48) }) }, async client => {
+      expect((await client.info()).tcb_info.mrtd).toBe('00'.repeat(48))
+    })
+  })
+})

sdk/js/src/client-v0.ts

@@ -8,7 +8,10 @@
 
 import fs from 'fs'
 import { send_rpc_request } from './send-rpc-request'
-import { to_hex, throwOnRpcError, resolveDstackEndpoint, type Hex } from './shared'
+import {
+  to_hex, throwOnRpcError, resolveDstackEndpoint, type Hex,
+  from_hex, require_string, to_list,
+} from './shared'
 
 export interface GetTlsKeyResponse {
   __name__: Readonly<'GetTlsKeyResponse'>
@@ -131,6 +134,16 @@ function x509key_to_uint8array(pem: string, max_length?: number) {
   return result
 }
 
+/** `JSON.parse` stringifies non-strings, so check the type first. */
+function parse_tcb_info<T extends TcbInfo>(value: unknown): T {
+  const text = require_string(value, 'tcb_info')
+  try {
+    return JSON.parse(text) as T
+  } catch (error) {
+    throw new Error(`the agent returned a malformed tcb_info: ${(error as Error).message}`)
+  }
+}
+
 export interface TlsKeyOptions {
   subject?: string;
   altNames?: string[];
@@ -192,9 +205,11 @@ export class DstackClientV0<T extends TcbInfo = TcbInfoV05x> {
       algorithm: algorithm
     })
     const result = await send_rpc_request<{ key: string, signature_chain: string[] }>(this.endpoint, '/GetKey', payload)
+    throwOnRpcError(result)
     return Object.freeze({
-      key: new Uint8Array(Buffer.from(result.key, 'hex')),
-      signature_chain: result.signature_chain.map(sig => new Uint8Array(Buffer.from(sig, 'hex'))),
+      key: from_hex(result.key, 'key'),
+      signature_chain: to_list(result.signature_chain, 'signature_chain', 'error')
+        .map((sig, i) => from_hex(sig, `signature_chain[${i}]`)),
       __name__: 'GetKeyResponse',
     })
   }
@@ -291,9 +306,10 @@ export class DstackClientV0<T extends TcbInfo = TcbInfoV05x> {
 
   async info(): Promise<InfoResponse<T>> {
     const result = await send_rpc_request<Omit<InfoResponse<TcbInfo>, 'tcb_info'> & { tcb_info: string }>(this.endpoint, '/Info', '{}')
+    throwOnRpcError(result)
     return Object.freeze({
       ...result,
-      tcb_info: JSON.parse(result.tcb_info) as T,
+      tcb_info: parse_tcb_info<T>(result.tcb_info),
     })
   }
 
@@ -368,10 +384,13 @@ export class DstackClientV0<T extends TcbInfo = TcbInfoV05x> {
 
     const result = await send_rpc_request<{ signature: string, signature_chain: string[], public_key: string }>(this.endpoint, '/Sign', payload);
 
+    throwOnRpcError(result)
+
     return Object.freeze({
-        signature: new Uint8Array(Buffer.from(result.signature, 'hex')),
-        signature_chain: result.signature_chain.map(sig => new Uint8Array(Buffer.from(sig, 'hex'))),
-        public_key: new Uint8Array(Buffer.from(result.public_key, 'hex')),
+        signature: from_hex(result.signature, 'signature'),
+        signature_chain: to_list(result.signature_chain, 'signature_chain', 'error')
+          .map((sig, i) => from_hex(sig, `signature_chain[${i}]`)),
+        public_key: from_hex(result.public_key, 'public_key'),
         __name__: 'SignResponse',
     });
   }

sdk/js/src/client-v1.ts

@@ -6,57 +6,10 @@
 // unsuffixed `DstackClient` names since 0.6.0.
 
 import { send_rpc_request } from './send-rpc-request'
-import { to_hex, throwOnRpcError, resolveDstackEndpoint } from './shared'
-
-/** An even number of hex digits, and nothing else. */
-const HEX_ONLY = /^(?:[0-9a-fA-F]{2})*$/
-
-/**
- * Decode a wire hex string, or say which field was malformed.
- *
- * Strict on purpose. Node's hex decoder stops at the first pair it cannot
- * parse and returns the prefix it managed, without error: `Buffer.from(
- * '0102zz', 'hex')` is two bytes, and an odd-length string loses its last
- * digit. These fields are private keys, signature chain links and application
- * identity -- handing back a silently truncated one is worse than throwing,
- * and Rust, Python and Go all refuse the same input.
- */
-function decode_hex(value: unknown, field: string): Uint8Array {
-  // The type check is not redundant with the regex, and dropping it is a
-  // silent-wrong-value bug rather than a style regression. `RegExp.test`
-  // stringifies its argument, so a one-element array passes -- `['00112233']`
-  // becomes `'00112233'` -- and `Buffer.from` then ignores the `'hex'`
-  // argument for a non-string input and coerces the elements as octets:
-  // `Number('00112233') & 0xff`, one attacker-chosen byte, no error. TypeScript
-  // cannot stop this because a JSON response is `any` at runtime.
-  if (typeof value !== 'string') {
-    throw new Error(
-      `the agent returned a malformed ${field}: expected a hex string, got ${
-        value === null ? 'null' : Array.isArray(value) ? 'an array' : typeof value}`
-    )
-  }
-  if (!HEX_ONLY.test(value)) {
-    throw new Error(
-      `the agent returned a malformed ${field}: expected an even-length hex string`
-    )
-  }
-  return new Uint8Array(Buffer.from(value, 'hex'))
-}
-
-/**
- * Decode a `bytes` field the proto declares required.
- *
- * Absence is an error rather than the empty default: `app_id` and `key` are
- * answers the agent always has, so a response without one is a response that
- * did not come from a working agent. An empty *string* still decodes to zero
- * bytes, which is what every other SDK does with it.
- */
-function from_hex(value: unknown, field: string): Uint8Array {
-  if (value === undefined) {
-    throw new Error(`the agent returned no ${field}`)
-  }
-  return decode_hex(value, field)
-}
+import {
+  to_hex, throwOnRpcError, resolveDstackEndpoint,
+  decode_hex, from_hex, require_string, to_list,
+} from './shared'
 
 /**
  * Decode a `bytes` field, treating an absent key as the empty default.
@@ -277,50 +230,6 @@ function to_string(value: unknown, field: string): string {
   return require_string(value, field)
 }
 
-/**
- * Read a `string` field the response is meaningless without.
- *
- * A bundle's `vendor` and `format` are what a caller dispatches on to pick a
- * verifier, so handing back `undefined` there does not degrade the answer, it
- * routes the evidence to no verifier at all -- quietly, since `undefined`
- * matches no `case`. Rust and Python both make these required.
- */
-function require_string(value: unknown, field: string): string {
-  if (typeof value !== 'string') {
-    throw new Error(
-      `the agent returned a malformed ${field}: expected a string, got ${
-        value === undefined ? 'nothing'
-          : value === null ? 'null'
-          : Array.isArray(value) ? 'an array' : typeof value}`
-    )
-  }
-  return value
-}
-
-/**
- * Read a `repeated` field, or say which one was not a list.
- *
- * `Array.isArray` rather than a truthiness check: a bare `.map()` on a `null`
- * or absent field throws `TypeError: Cannot read properties of null`, which
- * names no field and reads like an SDK bug rather than a bad response.
- *
- * `whenAbsent` follows the proto. A missing `boottime_gpu_evidence` is the
- * empty list, because the field is only populated when asked for; a missing
- * `bundles` or `signature_chain` is a malformed response, because those are
- * the whole answer of the call that returns them.
- */
-function to_list(
-  value: unknown, field: string, whenAbsent: 'empty' | 'error',
-): unknown[] {
-  if (value === undefined && whenAbsent === 'empty') {
-    return []
-  }
-  if (!Array.isArray(value)) {
-    throw new Error(`the agent returned a malformed ${field}: expected a list`)
-  }
-  return value
-}
-
 /**
  * Decode the bundles a v1 RPC returned.
  *

sdk/js/src/shared.ts

@@ -7,6 +7,9 @@
 
 import fs from 'fs'
 
+/** An even number of hex digits, and nothing else. */
+const HEX_ONLY = /^(?:[0-9a-fA-F]{2})*$/
+
 export type Hex = `${string}`
 
 export function to_hex(data: string | Buffer | Uint8Array): string {
@@ -59,3 +62,94 @@ export function resolveDstackEndpoint(endpoint: string | undefined): string {
   }
   return endpoint
 }
+
+/**
+ * Decode a wire hex string, or say which field was malformed.
+ *
+ * Strict on purpose. Node's hex decoder stops at the first pair it cannot
+ * parse and returns the prefix it managed, without error: `Buffer.from(
+ * '0102zz', 'hex')` is two bytes, and an odd-length string loses its last
+ * digit. These fields are private keys, signature chain links and application
+ * identity -- handing back a silently truncated one is worse than throwing,
+ * and Rust, Python and Go all refuse the same input.
+ */
+export function decode_hex(value: unknown, field: string): Uint8Array {
+  // The type check is not redundant with the regex, and dropping it is a
+  // silent-wrong-value bug rather than a style regression. `RegExp.test`
+  // stringifies its argument, so a one-element array passes -- `['00112233']`
+  // becomes `'00112233'` -- and `Buffer.from` then ignores the `'hex'`
+  // argument for a non-string input and coerces the elements as octets:
+  // `Number('00112233') & 0xff`, one attacker-chosen byte, no error. TypeScript
+  // cannot stop this because a JSON response is `any` at runtime.
+  if (typeof value !== 'string') {
+    throw new Error(
+      `the agent returned a malformed ${field}: expected a hex string, got ${
+        value === null ? 'null' : Array.isArray(value) ? 'an array' : typeof value}`
+    )
+  }
+  if (!HEX_ONLY.test(value)) {
+    throw new Error(
+      `the agent returned a malformed ${field}: expected an even-length hex string`
+    )
+  }
+  return new Uint8Array(Buffer.from(value, 'hex'))
+}
+
+/**
+ * Decode a `bytes` field the proto declares required.
+ *
+ * Absence is an error rather than the empty default: `app_id` and `key` are
+ * answers the agent always has, so a response without one is a response that
+ * did not come from a working agent. An empty *string* still decodes to zero
+ * bytes, which is what every other SDK does with it.
+ */
+export function from_hex(value: unknown, field: string): Uint8Array {
+  if (value === undefined) {
+    throw new Error(`the agent returned no ${field}`)
+  }
+  return decode_hex(value, field)
+}
+
+/**
+ * Read a `string` field the response is meaningless without.
+ *
+ * A bundle's `vendor` and `format` are what a caller dispatches on to pick a
+ * verifier, so handing back `undefined` there does not degrade the answer, it
+ * routes the evidence to no verifier at all -- quietly, since `undefined`
+ * matches no `case`. Rust and Python both make these required.
+ */
+export function require_string(value: unknown, field: string): string {
+  if (typeof value !== 'string') {
+    throw new Error(
+      `the agent returned a malformed ${field}: expected a string, got ${
+        value === undefined ? 'nothing'
+          : value === null ? 'null'
+          : Array.isArray(value) ? 'an array' : typeof value}`
+    )
+  }
+  return value
+}
+
+/**
+ * Read a `repeated` field, or say which one was not a list.
+ *
+ * `Array.isArray` rather than a truthiness check: a bare `.map()` on a `null`
+ * or absent field throws `TypeError: Cannot read properties of null`, which
+ * names no field and reads like an SDK bug rather than a bad response.
+ *
+ * `whenAbsent` follows the proto. A missing `boottime_gpu_evidence` is the
+ * empty list, because the field is only populated when asked for; a missing
+ * `bundles` or `signature_chain` is a malformed response, because those are
+ * the whole answer of the call that returns them.
+ */
+export function to_list(
+  value: unknown, field: string, whenAbsent: 'empty' | 'error',
+): unknown[] {
+  if (value === undefined && whenAbsent === 'empty') {
+    return []
+  }
+  if (!Array.isArray(value)) {
+    throw new Error(`the agent returned a malformed ${field}: expected a list`)
+  }
+  return value
+}