get_metabytes_by_hash returns rows without checking them against the hash the caller asked for

[thedavidmeister]

Unit

crates/metaboard/src/metaboard_client.rs — MetaboardSubgraphClient::get_metabytes_by_hash.

Intent oracle

cas.md in rainprotocol/specs frames a hash lookup as the point where integrity is established: "any CAS implementation MUST ensure the integrity of a .rain file according to the .rain spec before storing it under its hash. This simplifies all tooling downstream of the CAS", and "Exactly as an invalid hash for some content would be rejected by a CAS before the content is stored under the hash". dotrain.md makes this the import path — an import is @<namespace> <hash>, and tooling guarantees an import is either "found, local, valid, readable ... cryptographic integrity checked according to the hash" or "not found OR found and rejected".

Realistically the subgraph is the CAS client from the perspective of the onchain spec — it is the thing reading MetaV1_2 events and indexing them, and it derives metaHash itself in subgraph/src/metaBoard.ts:

metaV1.metaHash = Bytes.fromByteArray(crypto.keccak256(event.params.meta));

So the obligation lands there, not here. This issue is about redundancy in the subgraph's own client: the caller of get_metabytes_by_hash supplies the hash independently, so checking it costs one keccak per row and removes the need to trust the indexer's arithmetic or the transport.

Gap

get_metabytes_by_hash(&self, metahash: &[u8; 32]) takes the hash the caller is asking for, queries MetasByHash, hex-decodes each row's meta, and returns the bytes. It never computes keccak256(bytes) and compares it to metahash. Whatever the endpoint returns under that query is what the caller receives.

The check is meaningful here in a way it would not be against a row's own metaHash field: metahash is a parameter the caller brought, not a value the responder derived from the same bytes it is serving, so comparing against it is not self-referential.

What it would catch, given the subgraph is the trust root: a buggy or compromised indexer, a tampered response between the endpoint and the client, and a subgraph whose hashing diverges from the client's for any reason. None of these are reachable through a correct subgraph, which is why this is defence in depth rather than a live defect.

Present state of the callers

The one non-test caller, AuthoringMetaV2::fetch_for_contract (crates/cli/src/meta/types/authoring/v2.rs:206), does its own check after the call:

let meta_bytes = metas[0].as_slice();
let meta_bytes_hash = keccak256(meta_bytes);
if meta_bytes_hash.0 != metahash { /* MetaHashMismatch */ }

That was #210. Two consequences:

• The check is duplicated into each caller rather than held once at the point the hash is known, so a second caller gets no protection unless it remembers to repeat it.
• It covers metas[0] only. Rows 1..n are returned unchecked — though fetch_for_contract also never looks at them, which is fetch_for_contract only considers metas[0] and the first cbor document: a valid authoring meta later in the list is unreachable #157 / AuthoringMetaV2::fetch_for_contract panic-indexes metas[0] and decoded[0], relying on other crates' non-empty guarantees #211.

Test that currently pins the absence

test_get_metabytes_by_hash_success (crates/metaboard/src/metaboard_client.rs:172) asserts the unchecked behaviour rather than merely not covering it. Its fixture serves rows whose bytes cannot hash to the metaHash they carry:

{ "meta": "0x01", "metaHash": "0x00", ... },
{ "meta": "0x02", "metaHash": "0x00", ... }

and asserts result == [vec![1], vec![2]]. Adding the check turns this test red, so it is part of the change, not collateral.

Proposed fix

In get_metabytes_by_hash, keep a row only when keccak256(&bytes) == metahash.

Open question for triage — whether a mismatching row is dropped or raised:

• Dropped is consistent with how the subject path handles unusable rows and cannot be used by one bad row to deny a hash to every caller.
• Raised is arguably right here precisely because it is not the subject path: every row under a hash query is claimed to be that hash, so a mismatch is the indexer contradicting itself rather than someone else's data sitting alongside yours, and silently returning fewer rows hides that.

A mismatch means something upstream is wrong rather than merely noisy, which argues for a distinct error variant over Ok with a short vec. Not adjudicated here.

Triage framing

Redundancy, not a live defect against a correct subgraph. Filed for triage; no adjudication. Found while reviewing #287, which is the sibling get_metabytes_by_subject path — that one is not a hash lookup and correctly has no such check, since a subject is the entity metadata is about rather than an address of the content.