Skip to content

Commit 0566c7e

Browse files
committed
fix(sdk): make the v1 byte types refuse what they used to swallow
Three defects an adversarial pass found in the conversion itself. Rust's `report_data` kept `#[builder(into)]`, and both `&str` and `String` implement `Into<Vec<u8>>`. So `.report_data("00ff")` still compiled and attested the four ASCII bytes of that string -- exactly the failure the change was made to eliminate, on the one field a caller sends rather than receives. It is now a `ReportData` newtype that converts from a `Vec<u8>`, an array or a slice and from nothing else, so the builder keeps the coercion that made those three ergonomic while a string becomes a type error. A `compile_fail` doctest keeps it one. JavaScript decoded with `Buffer.from(value, 'hex')`, which stops at the first pair it cannot parse and returns the prefix, silently: a corrupted `app_id` came back as a short `Uint8Array`, `signature_chain: ["aabb","qq"]` came back one link short, and an absent required field came back as an empty array that is *truthy*, so `if (!info.app_id)` guards became dead code. Rust, Python and Go all reject these. It now throws and names the field, and absence of a required field is an error rather than empty bytes -- `os_image_hash` and `mr_aggregated` still read as empty, so a degraded `Info` stays parseable. Python accepted hex with embedded whitespace, which `bytes.fromhex` skips, while raising a message that said it did not; and it rejected a response that omits `os_image_hash` or `mr_aggregated`, where Rust has `#[serde(default)]`. Both aligned. None of this was covered: JS and Python had zero negative tests for any v1 byte field, which is why a green suite proved nothing here. Added, and mutation-checked -- four of the five new JS tests fail against the old lenient decoder. Also corrects the CHANGELOG, which said four of the eleven fields had no Rust decoder where the number is three, and which claimed "the wire is unchanged" without saying that only the JSON wire is: borsh writes a `Vec<u8>` as length-prefixed bytes where it wrote a hex `String`, so a 0.5.x blob deserializes without error into the wrong content. Three comments called `os_image_hash` and `mr_aggregated` `optional` on the wire; the proto declares them plain `bytes`, and `not_before` and `not_after` are the only `optional` fields it has. Tolerating their absence is a client-side choice, and now says so.
1 parent 5f829bc commit 0566c7e

6 files changed

Lines changed: 297 additions & 30 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6161
- sdk: the Go SDK's v1 `IssueCert` defaults `usage_server_auth` to true, as the Rust, Python and JavaScript v1 clients already did. Go was the odd one out, so the same argument-free call produced a certificate that could serve TLS in three languages and one that could not in the fourth — and a certificate you cannot serve with is useless to most callers. `WithCertUsageServerAuth(false)` opts out. v0's `GetTlsKey` keeps its `false` default deliberately: that is what the released 0.5.x Go SDK sent, and `DstackClientV0` mirrors released behaviour rather than the better choice
6262
- sdk: the JavaScript v1 `issueCert` response no longer carries a raw-bytes accessor. `asUint8Array()` is **removed rather than renamed**: it existed to feed the private key into the blockchain adapters, and v1 has no chain-flavoured surface. `IssueCert` returns TLS material, PEM is the form a TLS stack takes, and a caller who genuinely needs DER converts it with a standard library. The Rust, Python and Go v1 clients already returned the PEM string and the chain alone, so all four now agree. v0's `GetTlsKeyResponse.asUint8Array` is untouched — released API, and the viem and solana adapters depend on its truncating behaviour
6363
- sdk: the JavaScript v1 GPU evidence bundle's `asUint8Array()` accessor is gone, and `evidence` is the vendor's bytes directly, as Go's `Evidence` always was. Byte-exact off the wire, because sha256 over precisely those bytes is what the measured `gpu-attestation` event commits to
64-
- sdk: every field the `dstack.guest.v1` proto declares `bytes` is now that language's byte type on the v1 clients — Rust `Vec<u8>`, Python `bytes`, JavaScript `Uint8Array`, Go `[]byte` — and the `decode_*` helpers are gone along with the hex strings they decoded. Eleven fields move: `GetKeyResponse`'s `key`, `public_key` and `signature_chain`; `AttestResponse.attestation`; `GpuEvidenceBundle.evidence`; and `InfoResponse`'s `app_id`, `instance_id`, `compose_hash`, `device_id`, `os_image_hash` and `mr_aggregated`. Rust's `AttestConfig.report_data` moves with them, so the public builder and `attest()` finally agree on a type. **The wire is unchanged**JSON still carries lowercase hex; the encoding moved into the serialization layer, as serde's `hex::serde` in Rust, an annotated pydantic type in Python, and the client's decode step in JavaScript. Go already did this and is untouched.
64+
- sdk: every field the `dstack.guest.v1` proto declares `bytes` is now that language's byte type on the v1 clients — Rust `Vec<u8>`, Python `bytes`, JavaScript `Uint8Array`, Go `[]byte` — and the `decode_*` helpers are gone along with the hex strings they decoded. Eleven fields move: `GetKeyResponse`'s `key`, `public_key` and `signature_chain`; `AttestResponse.attestation`; `GpuEvidenceBundle.evidence`; and `InfoResponse`'s `app_id`, `instance_id`, `compose_hash`, `device_id`, `os_image_hash` and `mr_aggregated`. Rust's `AttestConfig.report_data` moves with them, so the public builder and `attest()` finally agree on a type. **The JSON wire is unchanged**it still carries lowercase hex; the encoding moved into the serialization layer, as serde's `hex::serde` in Rust, an annotated pydantic type in Python, and the client's decode step in JavaScript. Go already did this and is untouched.
6565

66-
The old typing did quiet damage. `docs/guest-api-v1.md` says of the v1 key claim that `public_key` is the raw derived public key, *not* a hex string, and its verification steps rebuild the claim from raw bytes — so a Rust or Python caller passing `response.public_key` straight into a claim builder built it over 66 ASCII characters instead of 33 bytes. No type error, no exception, just a chain that never verifies. `evidence` had the same shape of problem: three separate documents had to keep repeating "hash the decoded bytes, not the string as returned", precisely because the type did not say it. In Go the mistake was unspellable, and now it is unspellable everywhere. Nor was there one line to learn: before this, four of the eleven fields had no decoder at all in Rust, six had none in Python, and JavaScript had decoded three of them for a while.
66+
The old typing did quiet damage. `docs/guest-api-v1.md` says of the v1 key claim that `public_key` is the raw derived public key, *not* a hex string, and its verification steps rebuild the claim from raw bytes — so a Rust or Python caller passing `response.public_key` straight into a claim builder built it over 66 ASCII characters instead of 33 bytes. No type error, no exception, just a chain that never verifies. `evidence` had the same shape of problem: three separate documents had to keep repeating "hash the decoded bytes, not the string as returned", precisely because the type did not say it. In Go the mistake was unspellable, and now it is unspellable everywhere -- which is why Rust's `report_data` is a `ReportData` newtype rather than a bare `Vec<u8>`: `&str` and `String` both implement `Into<Vec<u8>>`, so under the builder's `into` coercion `.report_data("00ff")` would still compile and attest the four ASCII bytes of that string. The newtype converts from a `Vec<u8>`, an array or a slice and from nothing else, so the ergonomics survive and the string does not. A `compile_fail` doctest keeps it that way. Nor was there one line to learn: before this, three of the eleven fields had no decoder at all in Rust, six had none in Python, and JavaScript had decoded three of them for a while.
67+
68+
Decoding got stricter where it was silently lenient. JavaScript relied on `Buffer.from(value, 'hex')`, which stops at the first pair it cannot parse and returns the prefix, so a corrupted `app_id` became a short `Uint8Array` and a signature chain with one bad link came back quietly one link short; it now throws and names the field, as Rust, Python and Go already did. A required field that is absent altogether is an error rather than empty bytes -- `os_image_hash` and `mr_aggregated` are the two exceptions, read as empty so a degraded `Info` stays parseable, which is what Rust's `#[serde(default)]` already did and what Python now does instead of rejecting the response. Python also stops accepting hex with embedded whitespace, which `bytes.fromhex` skips and Rust refuses.
69+
70+
The `borsh` encoding of these structs does change, since borsh writes a `Vec<u8>` as length-prefixed bytes where it wrote a hex `String` before. Blobs written by 0.5.x deserialize without error into the new types and yield the ASCII of the hex string, so do not read old ones with the new types. Only the JSON wire is compatible.
6771

6872
**v0 deliberately keeps its hex strings and `decode_*` helpers.** That surface mirrors the released 0.5.x SDK so a 0.5.x program keeps working by changing only the class name; retyping every byte field would break that promise on an API that is frozen anyway. The blockchain adapters are v0-typed and unaffected
6973
- sdk: the v0 modules carry a `_v0` suffix, so the file a reader opens matches the client it holds. Rust's `dstack_sdk::dstack_client` becomes `dstack_sdk::dstack_client_v0` and `dstack_sdk_types::dstack` becomes `dstack_sdk_types::dstack_v0`; Python's `dstack_sdk.dstack_client` becomes `dstack_sdk.dstack_client_v0`; Go's `client.go`/`client_test.go` become `client_v0.go`/`client_v0_test.go`; and the JavaScript `index.ts`, which held both surfaces in one file, splits into `client-v0.ts`, `client-v1.ts` and a `shared.ts`, leaving `index.ts` as a barrel that re-exports exactly the names it always did. Until now the unsuffixed *file* meant v0 while the unsuffixed *class* meant v1, so a reader opening `dstack_client.rs` for the recommended client found the legacy one instead. **There are deliberately no backward-compat module aliases**: 0.6.0 is the loud-break release, and an import of an old module path fails at build time rather than silently binding the frozen surface under a name that now means something else. Package-level exports are untouched in every SDK — `dstack_sdk::DstackClient`, `from dstack_sdk import DstackClientV0` and `@phala/dstack-sdk`'s public surface are exactly what they were; only a deep import of the module path moves. In Go this is file naming alone, since it is all one `package dstack`

sdk/js/src/__tests__/index-v1.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,72 @@ describe('DstackClientV1', () => {
266266
})
267267
})
268268

269+
// Every one of these decoded to a shorter-than-asked-for Uint8Array with no
270+
// error before the hex decoding was made strict: Node's decoder stops at the
271+
// first pair it cannot parse and returns the prefix. A truncated `key` or a
272+
// signature chain quietly one link short is a verification failure nobody can
273+
// trace back to its cause.
274+
describe('malformed hex from the agent', () => {
275+
async function withAgentAnswering(body: unknown, fn: (client: DstackClientV1) => Promise<void>) {
276+
const server = http.createServer((_req, res) => {
277+
res.writeHead(200, { 'Content-Type': 'application/json' })
278+
res.end(JSON.stringify(body))
279+
})
280+
await new Promise<void>(resolve => server.listen(0, '127.0.0.1', () => resolve()))
281+
try {
282+
const { port } = server.address() as AddressInfo
283+
await fn(new DstackClientV1(`http://127.0.0.1:${port}`))
284+
} finally {
285+
await new Promise<void>(resolve => server.close(() => resolve()))
286+
}
287+
}
288+
289+
const identity = {
290+
app_id: 'aa'.repeat(20),
291+
compose_hash: 'bb'.repeat(32),
292+
instance_id: 'cc'.repeat(20),
293+
device_id: 'dd'.repeat(32),
294+
os_image_hash: 'ee'.repeat(32),
295+
mr_aggregated: 'ff'.repeat(48),
296+
}
297+
298+
it('should reject a non-hex character rather than truncate', async () => {
299+
await withAgentAnswering({ ...identity, app_id: 'aabbzz' + 'aa'.repeat(17) }, client =>
300+
expect(client.info()).rejects.toThrow(/malformed app_id/)
301+
)
302+
})
303+
304+
it('should reject an odd-length string rather than drop a digit', async () => {
305+
await withAgentAnswering({ ...identity, compose_hash: 'abc' }, client =>
306+
expect(client.info()).rejects.toThrow(/malformed compose_hash/)
307+
)
308+
})
309+
310+
it('should name the chain link that is malformed', async () => {
311+
await withAgentAnswering(
312+
{ key: 'aa'.repeat(32), public_key: 'bb'.repeat(33), signature_chain: ['aabb', 'qq'] },
313+
client => expect(client.getKey('x', 'secp256k1')).rejects.toThrow(/signature_chain\[1\]/)
314+
)
315+
})
316+
317+
it('should reject an absent required field instead of returning empty bytes', async () => {
318+
const { instance_id: _dropped, ...without_instance_id } = identity
319+
await withAgentAnswering(without_instance_id, client =>
320+
expect(client.info()).rejects.toThrow(/no instance_id/)
321+
)
322+
})
323+
324+
it('should accept an absent optional field as empty', async () => {
325+
const { os_image_hash: _a, mr_aggregated: _b, ...older_agent } = identity
326+
await withAgentAnswering(older_agent, async client => {
327+
const info = await client.info()
328+
expect(info.os_image_hash).toEqual(new Uint8Array(0))
329+
expect(info.mr_aggregated).toEqual(new Uint8Array(0))
330+
expect(info.app_id.length).toBe(20)
331+
})
332+
})
333+
})
334+
269335
describe('info', () => {
270336
it('should return the flat identity shape', async () => {
271337
const client = new DstackClientV1()

sdk/js/src/client-v1.ts

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,55 @@
88
import { send_rpc_request } from './send-rpc-request'
99
import { to_hex, throwOnRpcError, resolveDstackEndpoint } from './shared'
1010

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: string, field: string): Uint8Array {
25+
if (!HEX_ONLY.test(value)) {
26+
throw new Error(
27+
`the agent returned a malformed ${field}: expected an even-length hex string`
28+
)
29+
}
30+
return new Uint8Array(Buffer.from(value, 'hex'))
31+
}
32+
33+
/**
34+
* Decode a `bytes` field the proto declares required.
35+
*
36+
* Absence is an error rather than the empty default: `app_id` and `key` are
37+
* answers the agent always has, so a response without one is a response that
38+
* did not come from a working agent. An empty *string* still decodes to zero
39+
* bytes, which is what every other SDK does with it.
40+
*/
41+
function from_hex(value: string | undefined, field: string): Uint8Array {
42+
if (value === undefined || value === null) {
43+
throw new Error(`the agent returned no ${field}`)
44+
}
45+
return decode_hex(value, field)
46+
}
47+
1148
/**
12-
* Decode a wire hex string into the bytes the field is declared as.
49+
* Decode a `bytes` field, treating absence as the empty default.
1350
*
14-
* Every `bytes` field in the v1 proto travels as lowercase hex and surfaces
15-
* here as a `Uint8Array`, so this runs on all of them. A missing field is
16-
* proto3's empty default, not an error.
51+
* `os_image_hash` and `mr_aggregated` are the two that get this. Both are
52+
* plain `bytes` in the proto, so a current agent always sends them -- empty
53+
* when it could not compute one. Reading a missing key as those same empty
54+
* bytes rather than an error keeps a degraded `Info` readable instead of
55+
* unparseable; Rust spells the same rule `#[serde(default)]`. It costs
56+
* nothing, because neither field means anything unattested anyway.
1757
*/
18-
function from_hex(value: string | undefined): Uint8Array {
19-
return new Uint8Array(Buffer.from(value ?? '', 'hex'))
58+
function from_optional_hex(value: string | undefined, field: string): Uint8Array {
59+
return value === undefined || value === null ? new Uint8Array(0) : decode_hex(value, field)
2060
}
2161

2262
export interface IssueCertOptionsV1 {
@@ -157,8 +197,9 @@ export interface InfoResponseV1 {
157197
/**
158198
* `InfoResponseV1` as it arrives: identity fields hex, everything else final.
159199
*
160-
* `os_image_hash` and `mr_aggregated` are `optional` on the wire, so an older
161-
* agent may omit them entirely rather than send an empty string.
200+
* `os_image_hash` and `mr_aggregated` are optional here rather than on the
201+
* wire: the proto declares them plain `bytes` and the agent always sends them,
202+
* but a response that omits one is read as empty rather than rejected.
162203
*/
163204
type InfoResponseV1Wire =
164205
Omit<InfoResponseV1, '__name__' | 'app_id' | 'compose_hash' | 'instance_id'
@@ -190,7 +231,7 @@ function to_gpu_evidence_bundles(
190231
): GpuEvidenceBundleV1[] {
191232
return (bundles ?? []).map(bundle => Object.freeze({
192233
...bundle,
193-
evidence: from_hex(bundle.evidence),
234+
evidence: from_hex(bundle.evidence, 'GPU evidence'),
194235
}))
195236
}
196237

@@ -284,9 +325,11 @@ export class DstackClientV1 {
284325
this.endpoint, '/v1/GetKey', payload)
285326
throwOnRpcError(result)
286327
return Object.freeze({
287-
key: from_hex(result.key),
288-
public_key: from_hex(result.public_key),
289-
signature_chain: result.signature_chain.map(from_hex),
328+
key: from_hex(result.key, 'key'),
329+
public_key: from_hex(result.public_key, 'public_key'),
330+
signature_chain: result.signature_chain.map((link, i) =>
331+
from_hex(link, `signature_chain[${i}]`)
332+
),
290333
__name__: 'GetKeyResponseV1' as const,
291334
})
292335
}
@@ -321,7 +364,7 @@ export class DstackClientV1 {
321364
throwOnRpcError(result)
322365
return Object.freeze({
323366
__name__: 'AttestResponseV1' as const,
324-
attestation: from_hex(result.attestation),
367+
attestation: from_hex(result.attestation, 'attestation'),
325368
boottime_gpu_evidence: to_gpu_evidence_bundles(result.boottime_gpu_evidence),
326369
})
327370
}
@@ -358,12 +401,12 @@ export class DstackClientV1 {
358401
throwOnRpcError(result)
359402
return Object.freeze({
360403
...result,
361-
app_id: from_hex(result.app_id),
362-
compose_hash: from_hex(result.compose_hash),
363-
instance_id: from_hex(result.instance_id),
364-
device_id: from_hex(result.device_id),
365-
os_image_hash: from_hex(result.os_image_hash),
366-
mr_aggregated: from_hex(result.mr_aggregated),
404+
app_id: from_hex(result.app_id, 'app_id'),
405+
compose_hash: from_hex(result.compose_hash, 'compose_hash'),
406+
instance_id: from_hex(result.instance_id, 'instance_id'),
407+
device_id: from_hex(result.device_id, 'device_id'),
408+
os_image_hash: from_optional_hex(result.os_image_hash, 'os_image_hash'),
409+
mr_aggregated: from_optional_hex(result.mr_aggregated, 'mr_aggregated'),
367410
__name__: 'InfoResponseV1' as const,
368411
})
369412
}

sdk/python/src/dstack_sdk/dstack_client_v1.py

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"""
1818

1919
import binascii
20+
import re
2021
from typing import Annotated
2122
from typing import Any
2223
from typing import Dict
@@ -31,14 +32,22 @@
3132
from .dstack_client_v0 import BaseClient
3233
from .dstack_client_v0 import call_async
3334

35+
#: An even number of hex digits, and nothing else.
36+
_HEX_ONLY = re.compile(r"\A(?:[0-9a-fA-F]{2})*\Z")
37+
3438

3539
def _decode_hex(value: Any) -> Any:
36-
"""Turn a wire hex string into bytes, leaving anything else to pydantic."""
40+
r"""Turn a wire hex string into bytes, leaving anything else to pydantic.
41+
42+
The regex is not redundant with ``bytes.fromhex``: that helper skips ASCII
43+
whitespace, so ``"aa bb"`` and ``"aa\nbb"`` decode happily while the error
44+
message here promises they do not. Rust and Go reject both, and a field
45+
one SDK accepts and another refuses is a field verifiers cannot rely on.
46+
"""
3747
if isinstance(value, str):
38-
try:
39-
return bytes.fromhex(value)
40-
except ValueError as err:
41-
raise ValueError(f"expected a lowercase hex string: {err}") from err
48+
if not _HEX_ONLY.match(value):
49+
raise ValueError(f"expected an even-length hex string, got {value!r}")
50+
return bytes.fromhex(value)
4251
return value
4352

4453

@@ -152,8 +161,11 @@ class InfoResponseV1(BaseModel):
152161
instance_id: HexBytes
153162
# Identifies the host machine, not this instance.
154163
device_id: HexBytes
155-
os_image_hash: HexBytes
156-
mr_aggregated: HexBytes
164+
# Plain `bytes` in the proto, so the agent always sends these -- empty when
165+
# it could not compute one. A response that omits one is read as those same
166+
# empty bytes rather than rejected, as Rust's `#[serde(default)]` does.
167+
os_image_hash: HexBytes = b""
168+
mr_aggregated: HexBytes = b""
157169
vm_config: str = ""
158170
key_provider_info: str = ""
159171
cloud_vendor: str = ""

0 commit comments

Comments
 (0)