fix(sdk): compose hash disagrees across SDKs (HTML escaping, dropped fields) - #1024
Open
Leechael wants to merge 8 commits into
Open
fix(sdk): compose hash disagrees across SDKs (HTML escaping, dropped fields)#1024Leechael wants to merge 8 commits into
Leechael wants to merge 8 commits into
Conversation
The Go compose hash implementation had no tests at all, while the JS SDK has a full suite. A compose hash is the identity of a deployed app: if Go disagrees with the reference implementation by one byte, an app deployed through Go fails attestation. That is not something to leave uncovered. CROSS_LANGUAGE_CONSISTENCY_TESTING.md names the JavaScript SDK as the canonical implementation, so the expected hashes are reference values produced by sdk/js/src/get-compose-hash.ts. Each was additionally cross-checked against the Python SDK to rule out a mistake in generating them. The vectors cover the fields Go declares today: the minimal compose, a full legacy field set, both normalization branches (bash drops docker_compose_file, docker-compose drops bash_script), empty pre_launch_script removal, requirements and the nerdctl snapshotter. All pass against the current implementation, so this commit pins existing behaviour rather than changing it.
GetComposeHash marshals the AppCompose struct, so any key the struct does not declare is silently dropped before hashing. The other SDKs do not behave this way: the JavaScript AppCompose carries an index signature and the Python one accepts **kwargs, so both hash whatever the caller passed. That makes Go the only SDK that computes a different compose hash for the same app_compose document. Since the compose hash is the app's on-chain identity, the failure is silent and total: the Go caller registers one hash, the guest measures another, attestation fails. Every field added to dstack-types since this struct was last synced reproduces it — port_policy is simply the one that surfaced it. Add an Extra map, merged into the top-level object on marshal and populated from it on unmarshal, so a compose using fields newer than this SDK round-trips and hashes identically to JS and Python. An Extra key that collides with a declared field is rejected instead of silently overriding it, since which one won would otherwise depend on map iteration order. The declared-name set is derived by reflection over the struct tags rather than from a marshalled document, so the collision check still fires for a field that omitempty left out. Tests use the same JS reference vectors as the existing ones, including port_policy and non-ASCII values, plus a round trip that decodes a document with scrambled key order and re-hashes it.
The Go AppCompose had drifted behind dstack-types AppCompose
(dstack/dstack-types/src/lib.rs). The previous commit stops the drift from
corrupting compose hashes, but callers still had no typed way to set these
fields. Declare them:
- port_policy (PortPolicy / PortAttrs): per-port PROXY protocol opt-in and the
restrict-mode port whitelist.
- init_script: bash scripts run before the application runner starts.
- storage_fs, swap_size: guest filesystem and swap sizing.
- event_log_version: the event log digest format, serialized as a number.
- verity_volumes (VerityVolume): pre-baked read-only dm-verity volumes.
- KeyProviderTPM, which the Rust, JS and Python SDKs already had.
Two wire-format details that a straight port would get wrong, both pinned by
tests:
- swap_size is serialized by dstack-types through its human_size helper, which
emits a *string* ("2G") for human-readable formats such as JSON — not a byte
count. The Go field is therefore a string.
- dstack-types always serializes port_policy, but emitting an empty policy for
an app that does not use one would change that app's compose hash. The Go
field is a pointer with omitempty, so absent stays absent.
Expected hashes are JS reference values, as in the surrounding tests. Two
guards accompany them: a minimal compose must still hash to its pre-existing
value (otherwise every deployed app changes identity), and a document carrying
these keys must now decode into the struct with Extra left empty.
The passthrough tests from the previous commit move from port_policy to a
made-up future_policy key, since port_policy is a declared field now and the
collision guard rejects it in Extra.
encoding/json escapes <, > and & as <, > and & unless told otherwise. JSON.stringify and json.dumps emit them literally, so Go hashed different bytes than every other SDK for any compose containing one of those three characters. They are not exotic. ">=" is the canonical form of a semver requirement, so every requirements.os_version range hit this. "&&" chains shell commands, so every docker_compose_file with a compound command hit it too. This affected a large share of real apps, and like the dropped-field bug it failed silently: the Go caller registers one hash, the guest measures another, attestation fails with nothing pointing at the cause. Switch toDeterministicJSON to json.Encoder with SetEscapeHTML(false), trimming the trailing newline Encoder appends — a stray newline would change every hash. The intermediate Marshal/Unmarshal round trip inside GetComposeHash needs no change, since unmarshalling reverses the escaping before the final encode. Verified against the JS SDK and cross-checked with the Python SDK: all three now agree on the same three cases, which the tests pin.
dstack-types Requirements has a fifth field, gpu_policy, that none of the SDKs declared. It is not inert: an omitted field is parsed and measured as the default empty policy, and its JCS-canonicalized digest is emitted as the gpu-policy-hash launch event right after compose-hash. A caller who needs a GPU policy had no way to express one. Every GpuPolicy field is omitted when unset. That is a safety property, not a style choice: attest_gpu defaults to true guest-side, so emitting a bare false for a caller who only set Rego would silently disable GPU attestation. AttestGPU is therefore a *bool, and a dedicated test asserts an unset one stays out of the document. The three allow_* flags default false in both directions, so plain bools with omitempty are equivalent. Expected hashes are JS reference values, as in the surrounding tests.
AppCompose.Extra only covered the top level, so the bug it was meant to fix survived one level down: Requirements, GpuPolicy, DockerConfig, PortPolicy, PortAttrs and VerityVolume are all plain structs, and each dropped unknown keys on marshal. requirements.gpu_policy was a live example — before the previous commit declared it, a Go caller could not produce the same hash as JS for a compose carrying it. Generalize the marshal/unmarshal pair into marshalWithExtra / unmarshalExtra plus a per-type cache of declared JSON names, then give every nested compose object the same Extra field and the same collision guard. Requirements and GpuPolicy carry deny_unknown_fields in dstack-types, so an unknown key there yields a compose the guest will refuse to parse. Passing it through is still correct for hashing: the SDK's job is to hash the document the caller supplied, and the JS SDK does exactly that. Failing at launch with a clear parse error beats registering a hash nobody can reproduce. Vectors are JS reference values covering each nested object, plus a round trip that decodes nested unknown keys and re-hashes unchanged.
…ects
AppCompose already accepted **kwargs, so an unknown top-level key hashed fine.
Its nested objects did not: from_dict calls DockerConfig(**dc) and
Requirements(**req), so any key outside their fixed signatures raised
TypeError and the compose could not be hashed at all.
requirements.gpu_policy is a live example. It exists in dstack-types but was
never added here, so `get_compose_hash({"requirements": {"gpu_policy": ...}})`
raised `TypeError: Requirements.__init__() got an unexpected keyword argument`.
Give both classes the same **kwargs plus to_dict merge that AppCompose uses,
and declare gpu_policy explicitly since it is a real field rather than a
hypothetical future one.
Failing loudly is better than the Go SDK's silent wrong hash, but it still
means a document the JS SDK hashes fine is unusable here. Expected values in
the tests are JS reference hashes for the same documents.
…he last sync
Both SDKs hashed these fields correctly already — JS through its index
signature, Python through **kwargs — so this is a typing gap, not a
correctness one. Declaring them gives callers type checking and, more
usefully, pins the wire shapes that are easy to get wrong:
- swap_size is a string ("2G"), because dstack-types serializes it through its
human_size helper, not as a byte count.
- event_log_version is a number, omitted for v1.
- attest_gpu inside gpu_policy defaults to true guest-side, so it is optional
rather than defaulted to false.
Fields declared: init_script, storage_fs, swap_size, event_log_version,
port_policy (with PortPolicy / PortAttrs), verity_volumes (with VerityVolume)
and requirements.gpu_policy (with GpuPolicy).
The JS tests double as the source of the cross-language vectors used by the Go
and Python suites: they run against the real @noble/hashes implementation, so
they confirm those vectors are the reference implementation's own output.
Leechael
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three independent bugs made the Go SDK compute a different compose hash than the JS and Python SDKs for the same
app_composedocument, plus one that made Python refuse to hash a valid document at all. Since the compose hash is an app's on-chain identity, all of them fail the same silent way: the SDK registers one hash, the guest measures another, attestation fails with nothing pointing at the cause.Started as "add
port_policyto the GoAppCompose". That field turned out to be a symptom.The bugs
1. Go HTML-escaped the hashed JSON — widest blast radius
encoding/jsonescapes<,>and&as<,>,&unless told otherwise.JSON.stringifyandjson.dumpsemit them literally.These are not exotic characters in a compose document:
">=0.6.0"— the canonical form of a semver requirement, so everyrequirements.os_versionrange hit this."sh -c \"migrate && serve\""— so everydocker_compose_filewith a compound shell command hit it too.Verified against unmodified
master: all three probe cases mismatched JS and Python, which agreed with each other.2. Go dropped fields it did not declare
GetComposeHashmarshals a struct, so unknown keys vanish before hashing. JS carries an index signature and Python accepts**kwargs, so both hash whatever the caller passed. Go was the only SDK that silently discarded them — and it did so at every level, top-level and nested.3. Python raised on unknown fields in nested objects
from_dictcallsDockerConfig(**dc)andRequirements(**req), so any key outside their fixed signatures raisedTypeError.requirements.gpu_policy— a realdstack-typesfield — was unhashable in Python. Failing loudly beats Go's silent wrong hash, but the document still could not be processed.4. Drift against
dstack-typesMissing everywhere:
init_script,storage_fs,swap_size,event_log_version,port_policy,verity_volumes,requirements.gpu_policy, and thetpmkey provider.Commits
test(sdk/go)fix(sdk/go)Extrapassthrough at the top levelfeat(sdk/go)KeyProviderTPMfix(sdk/go)feat(sdk/go)requirements.gpu_policyfix(sdk/go)fix(sdk/python)feat(sdk/js,sdk/python)Test vectors
sdk/CROSS_LANGUAGE_CONSISTENCY_TESTING.mdnames the JavaScript SDK as canonical, so every expected hash is a JS reference value. Each was independently reproduced by the Python SDK, and the JS tests added here run against the real@noble/hashesimplementation — so the vectors are confirmed to be the reference implementation's own output rather than a reimplementation's.Wire-format details a straight port gets wrong
Both pinned by tests:
swap_sizeis a string, not a byte count.dstack-typesserializes it throughsize_parser::human_size, which emits"2G"for JSON.attest_gpumust stay absent when unset. Its guest-side default istrue, so emitting a barefalsefor a caller who only setregowould silently disable GPU attestation. It is a pointer/optional in all three SDKs, with a dedicated test.port_policymust stay absent when unset.dstack-typesalways serializes it, but the SDKs are client-side constructors: emitting an empty policy for an app that does not use one would change that app's compose hash.Upgrade safety
TestUnsetNewFieldsDoNotChangeExistingHashesasserts a compose that predates these fields still hashes to its previous value. The HTML-escaping fix does change Go's output — necessarily, since that output was wrong. Any hash previously computed by the Go SDK for a compose containing<,>or&was never reproducible by the guest or the other SDKs, so nothing correct depended on it.Note on
deny_unknown_fieldsRequirementsandGpuPolicycarrydeny_unknown_fieldsindstack-types, so an unknown key there yields a compose the guest refuses to parse. Passing it through is still correct for hashing — the SDK's job is to hash the document the caller supplied, and JS does exactly that. Failing at launch with a clear parse error beats registering a hash nobody can reproduce.Verification
Pre-existing and untouched:
sdk/go/ratls/ratls.gois unformatted onmaster;sdk/go/dstack/client_test.goand the Python client tests need a live socket /solders.