SNRC resolver · adversarial + developer experience

Fifteen Calls a Name

Two reviews of the resolver as a whole, not only of the endpoint added in this branch: one assuming the caller is hostile, one assuming they are a developer trying to use it and getting no help.

The severe findings are all in the service that already shipped. The embarrassing ones are mine, in code written this session — including a field that claims to be a bytes32 and is not.

1 critical 3 high 2 medium 7 DX 3 of them mine

Adversarial

Assume the caller is hostile and the resolver is reachable. Every finding here is in code that predates this branch except where marked.

A1Critical

/health hands out the upstream RPC URL to anyone who asks

The health endpoint answers, unauthenticated:

{"ok": true, "rpc": "http://reth:8545", "registries": {...}}

Against the documented local reth that is harmless. Against any hosted provider it is a credential: Alchemy, Infura, QuickNode and Ankr all put the API key in the URL path. A resolver pointed at one of those publishes its owner's node key to every caller, and node keys are metered — the finder does not need to break anything, only to spend the budget.

The same value leaks a second way. Both 502 handlers return f"{type(e).__name__}: {e}", and urlopen's exception text carries the URL it failed on. So an attacker who cannot reach /health gets the same string by asking for a name while the node is unreachable — which they can arrange by asking for enough of them.

Fix

Report reachability, not configuration: {"ok": true, "rpc": "reachable", "chainId": 1}. If the endpoint must be identifiable, publish the host without the path. Then stop putting exception text in response bodies — log it, return a correlation id. The URL is the one string in this process that must never be echoed.

snrc-resolve.py · Handler.do_GET /health · resolve/owned-by 502 handlers
A2High

The client can authenticate; the server cannot check

HttpResolver.hs implements RpcAuth with bearer and basic modes, redacts the secret from Show so it cannot land in logs, and attaches Authorization to every request. The Python resolver never reads that header. There is no auth check anywhere in the service.

An operator who configures a token has done real work — chosen a secret, wired it through config, kept it out of logs — and has protected nothing. That is worse than having no auth feature: an absent one prompts a firewall rule, a decorative one prompts confidence.

Fix

Verify the header when a secret is configured, with a constant-time compare, and refuse to start when a resolver binds beyond loopback without one. Until then the Haskell side's RpcAuth should say in its haddock that no known resolver validates it.

HttpResolver.hs · RpcAuth, authHeader · snrc-resolve.py · Handler (no auth path)
A3HighPartly mine

One request buys up to 770 upstream calls

Nothing is cached and nothing is rate limited, so the amplification is the whole story:

/resolve/<name>    15 RPC calls   nameExpires, GRACE_PERIOD, resolver,
                                  owner, 7 text records, 4 coin addresses
/owned-by/<addr>  770 RPC calls   GRACE_PERIOD + balanceOf
                                  + 3 per token x MAX_OWNED (256)

/owned-by is mine, and it is the worse of the two by a factor of fifty. MAX_OWNED bounds a single response; it does nothing about the rate, and the expensive request is the cheap one to send. Against a metered provider this is someone else's invoice. Against self-hosted reth it is a queue nobody else gets through.

The address in /owned-by need not even hold anything — the cost is paid before the balance is known to be zero.

Fix

Cache resolved records for a short TTL, which is safe because the underlying data changes at block cadence, not per request. Then rate-limit per client. If the resolver is meant to be public, /owned-by wants a lower default bound than 256, since the tail of that range is rare and the cost is linear in it.

snrc-resolve.py · resolve, owned_by, TEXT_KEYS, MAX_OWNED
A4High

The path documented for casual use is the one that listens to the world

SNRC_BIND defaults to 0.0.0.0. The compose file publishes 127.0.0.1:8000:8000, so the Docker path is safe — and the README offers a second path for anyone who wants a quick look:

uv run scripts/resolver/service/snrc-resolve.py  # defaults to local reth

That inherits the default and listens on every interface, with no auth (A2) and an endpoint that publishes the node URL (A1). The safe path is the one behind a container; the casual path is the exposed one. That is the wrong way round — defaults should be safe and deployments should opt into exposure.

Fix

Default SNRC_BIND to 127.0.0.1 and set 0.0.0.0 explicitly in docker-compose.yml, where publishing is already deliberate and already scoped to loopback on the host.

snrc-resolve.py · BIND · docker-compose.yml · README "runnable standalone"
A5Medium

The resolver protects its caller and not itself

HttpResolver.hs is careful about exactly this: brReadSome maxResponseBytes, redirectCount = 0, an explicit timeout, with a comment explaining that adversarial endpoints must not be able to exhaust memory. The resolver then calls its own upstream with urlopen(req, timeout=15).read() — a timeout, and no size cap at all.

An RPC endpoint that is compromised, misconfigured or simply pointed at the wrong host can return a body large enough to end the process. The threat model was written down one layer up and not applied one layer down.

Fix

Read with a cap and fail closed past it, mirroring the bound the Haskell client already uses. Consider following redirects zero times there too, for the same reason it is spelled out in HttpResolver.hs.

snrc-resolve.py · rpc() · HttpResolver.hs · httpGet
A6Medium

http.server is documented as not for production, and this is production

CPython's own docs say http.server "is not recommended for production. It only implements basic security checks." There are no request size limits, no header count limits and no slow-read protection. It is fronted by nothing — the compose file exposes the port directly.

This is a reasonable choice for a script and an unreasonable one for a service an smp-server depends on for name resolution. Worth a decision rather than an accident: either it stays a dev tool and the deployment path puts a real server in front, or it becomes a service and gets one.

Fix

Put it behind something that terminates connections properly, or move to a WSGI/ASGI server. Either way the README should say which posture is intended, because right now docker-compose implies production and the implementation implies development.

snrc-resolve.py · ThreadingHTTPServer · docker-compose.yml

Developer experience

Assume a developer writing a client against this, with only the README and the responses. Three of these are defects in the branch under review.

D1HighMine

labelhash is not a hash, it is a Python integer literal

I emit it with hex(token), which produces the shortest representation:

"labelhash": "0xb"          what it returns
"labelhash": "0x0000…000b"  what a bytes32 labelhash is

Every other party in this system — the contract, a block explorer, any client comparing against chain state — represents a labelhash as 32 bytes. The value returned cannot be pasted into a contract call, cannot be compared textually with an on-chain topic, and will silently mismatch rather than fail loudly. It also makes the list's own sort order wrong, since it sorts as a string.

Fix

"0x" + format(token, "064x"). One line, and it should carry a test, because the wrong version looks right for the common case where the leading bytes happen to be non-zero.

snrc-resolve.py · owned_by · this branch
D2Medium

Three error shapes, so a client needs three error handlers

An unhappy response is one of:

{"name": …, "error": …, "configured_tlds": [...]}   TLD not configured
{"error": …, "got": …}                              not fully qualified
{"address": …, "error": …}                          bad address
{"name": …, "status": …, "error": …, …}             lapsed / unregistered

The key naming the subject changes, status is present on some and not others, and configured_tlds appears in two of them and not the third. A client cannot write one function that turns a failure into a message.

Fix

One envelope on every non-2xx: a stable error code a client can branch on, a human message, and the subject under a fixed key. The current strings are messages pretending to be codes.

snrc-resolve.py · resolve, owned_by, Handler.do_GET
D3MediumPartly mine

Two casing conventions in one document

simplexContact, simplexChannel, checkedTlds, graceEnds are camelCase — the file says why, so aeson can derive field names without a rewriting layer. configured_tlds is snake_case, in the same response body.

I added checkedTlds directly alongside the existing configured_tlds without noticing they disagree, which is how a convention with one exception becomes a convention with two.

Fix

Rename configured_tlds to configuredTlds. It appears only in error bodies, so the blast radius is small — and it will only get harder once anything depends on it.

snrc-resolve.py · resolve, owned_by
D4MediumMine

/health cannot tell an operator whether the new endpoint will work

Health reports registries and not REGISTRARS. Since /owned-by and every expiry field depend entirely on the registrar being configured, an operator who has set only the registry gets a healthy resolver, a working /resolve, "status": "unknown" on every name, and a 400 from /owned-by — with nothing in the health check hinting why.

I added the configuration and the dependency on it, and left the diagnostic reporting the older half.

Fix

Report both maps in /health, subject to A1 — the addresses are public on chain, so unlike the RPC URL they are safe to publish.

snrc-resolve.py · Handler /health · REGISTRARS · this branch
D5Medium

Nothing anywhere says which resolver you are talking to

The response payload just grew three fields. A client written last month and one written today receive different documents from the same URL, and neither can ask which it is. There is no version in the path, no version in /health, and no capability list.

This matters more now than it did: status and expires are load-bearing for a renewal reminder, and a client cannot tell whether their absence means "not supported here" or "not knowable for this name".

Fix

A version in /health is enough, and cheapest now. The distinction the client actually needs — unsupported versus unknowable — is otherwise impossible to make from a null.

snrc-resolve.py · Handler /health · README response shape
D6MediumMine

Truncation is a dead end, not a page

/owned-by stops at SNRC_MAX_OWNED and sets truncated: true. There is no offset, no cursor and no ordering guarantee a caller could resume from, so an address holding more than 256 names in a TLD has no way to see the rest — the flag is honest about the problem and offers no way out of it.

truncated is also a single boolean over a merged multi-TLD result, so it does not say which registrar ran out.

Fix

Take ?offset= and echo it back; enumeration is index-based on the registrar, so this is nearly free. Failing that, say in the response which TLD truncated, so the caller can at least narrow the query.

snrc-resolve.py · owned_by · MAX_OWNED · this branch
D7Medium

The schema is a Haskell type in another repository

Names/Record.hs says "the Haskell type IS the schema", which serves the one consumer written in Haskell and nobody else. A TypeScript or Python client has the README and a curl example. There is no OpenAPI document, no JSON Schema, and no fixture file to test against.

The claim is also now slightly untrue: the resolver returns status, expires and graceEnds, and the record type has none of them — safely, since aeson ignores unknown fields, but the two have diverged and only a comment asserts they have not.

Fix

Publish the response shape as a schema next to the script and generate the examples in the README from it, so drift shows up as a failing test rather than as a stale sentence.

Names/Record.hs · scripts/resolver/README.md

What held

Specific things I tried to break and could not.

Path handling. Names arrive percent-encoded from the Haskell client and are unquoted into path segments, but they only ever reach keccak — there is no filesystem, no database and no shell in the path, so a crafted name is a hash of a crafted name and nothing more.

ABI decoding. decode_bytes checks its length before slicing and returns empty rather than throwing on a short return, so a contract answering with garbage produces an empty field rather than a crash.

The available() trap. The obvious way to compute registration status is to call available(id). It is true for a name nobody ever registered, since 0 + GRACE_PERIOD < now, so it silently conflates "never taken" with "released". The implementation uses nameExpires and applies the rule on top, and the test suite pins it.

Address validation. /owned-by rejects a malformed address before any RPC call, so the validation cannot be used as an oracle or as a way to spend upstream budget.