fix: add zod input validation and remove redundant legacy validation#451
Closed
tony8713 wants to merge 3 commits into
Closed
fix: add zod input validation and remove redundant legacy validation#451tony8713 wants to merge 3 commits into
tony8713 wants to merge 3 commits into
Conversation
Validate JSON-RPC params with zod at each entry point in src/api.ts and return a proper invalid-params error (code -32602, HTTP 400) with a clear issue summary instead of the previous misleading 500 unauthorized. - Add src/helpers/validation.ts with zod schemas for lookup_addresses, resolve_names, lookup_domains and get_owner, plus a formatZodError helper. - Add rpcInvalidParams helper and let rpcError carry a real message (defaults preserved for existing callers). - Replace the manual Array.isArray params check (now covered by zod). - Return HTTP 400 (was 500) for an invalid resolver query on the image route. - Add e2e cases asserting malformed params return -32602/400. Valid requests are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Close the second unvalidated boundary and make validation enforceable by the compiler across the whole resolver call graph. - Validate the GET /:type/:id avatar route inside parseQuery: the stripped, lowercased id must be a valid address or handle (avatarIdSchema), else the route returns HTTP 400 (InvalidQueryError) instead of a 500. The /clear route gets the same 400 path. Size/width/height query clamping is unchanged. - Add branded nominal types in src/helpers/validation.ts: addressSchema / handleSchema now .brand() and export ValidatedAddress / ValidatedHandle, and avatarIdSchema (ValidatedAddress | ValidatedHandle) for the avatar route. All entry-point schemas (JSON-RPC + avatar route) output branded values. - Thread the branded input types through the resolver entry points: lookupAddresses / resolveNames, lookupDomains, getOwner, and the avatar resolver entries (resolvers/ens, resolvers/basename, addressResolvers basename getAvatar / castToEnsName). The compiler now enforces that only validated values reach the resolvers. Plain Address / Handle aliases stay for values flowing OUT of the resolvers (resolved network values are plain). - Drop the now-redundant inner isAddress guard in lookupDomains (shape is guaranteed by the branded Address at the only caller). - Keep normalizeAddresses: its getAddress checksum / lowercase step is NORMALIZATION (required for stable redis cache keys and dedup), not validation, and its reject-filter still guards the unbranded clearCache route param. Cache key shape is unchanged. The batch cap (max 50) stays as a zod .max at the boundary since types cannot encode array length. - Tests: add e2e cases asserting a bad avatar id returns 400 and valid address / handle ids do not; brand integration test inputs via the real schemas (test/helpers/validation.ts); convert the two lookupDomains invalid-input tests to assert the boundary schema rejects them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tony8713
force-pushed
the
fix/zod-input-validation
branch
from
June 8, 2026 12:09
2bf3ee6 to
b8d9207
Compare
chai3-bot
approved these changes
Jun 23, 2026
chai3-bot
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the validation changes and didn’t find any blocking issues.
Local checks:
- yarn typecheck ✅
- yarn lint ✅
I also started the targeted e2e/integration Jest run, but stopped it after it sat on network-heavy tests without more output; GitHub CI is already green for this PR.
Condense the multi-line explanatory comments added in this PR down to the repo's terse single-line style. Comments only; no logic changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 27, 2026
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
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.
Problem
Malformed JSON-RPC input (e.g. a nested params array, a non-address string, the wrong type) currently surfaces as a misleading HTTP 500 with
message: "unauthorized", because the only structured rejection (rpcError) hardcodes that message and the array methods only had a singleArray.isArrayguard. Anything past that guard fell through to the catch-all 500.Reproduction
Bad input (nested
paramsarray) on currentmasterreturns a misleading HTTP 500unauthorized:{"jsonrpc":"2.0","error":{"code":500,"message":"unauthorized","data":{}},"id":null}With this PR the same request returns a proper JSON-RPC invalid-params error (code
-32602) with HTTP 400 and a human readable zod summary indata:{"jsonrpc":"2.0","error":{"code":-32602,"message":"invalid params","data":"0: Invalid input"},"id":null}For contrast, the valid (flat) request is unchanged and returns HTTP 200 both before and after:
{"jsonrpc":"2.0","result":{"0xd8da6bf26964af9d7eed9e03e53415d37aa96045":"vitalik.eth"},"id":null}What this does
src/helpers/validation.tswith zod schemas for every relevant JSON-RPC method:lookup_addressesandresolve_names: a flat, non-empty array, each item a valid EVM (0x+ 40 hex) or Starknet (0x+ 64 hex) address / a valid handle, capped at the same batch limits the code already used (50 / 5).lookup_domains: a single valid EVM address string.get_owner: a single valid handle string.formatZodErrorhelper that produces a short human readable issue summary.src/api.tsbefore any business logic. On failure it returns a real JSON-RPC invalid-params error (code-32602) with HTTP 400 and the zod issue summary indata, instead of the old 500 unauthorized.rpcInvalidParamshelper and extendsrpcErrorto carry a real message (existing callers keep the previous default, so no behavior change for them).Array.isArray(params)check that zod supersedes.resolverquery param now returns HTTP 400 (was a misleading 500).-32602/ 400 (nested array, bad string, wrong type, empty, over-cap) for the JSON-RPC methods, plus a badresolverquery returning 400.Valid requests are byte-for-byte unchanged.
Second boundary + branded types (validate at every boundary)
The JSON-RPC entry was the first boundary. The avatar image route
GET /:type/:idwas a second, unvalidated boundary: it is parsed byparseQuery(no zod) and re-enters the resolvers viaresolvers/ens.tscastToEnsName->lookupAddressesandresolvers/basename.ts->addressResolvers/basename.tsgetAvatar. This follow-up closes it and makes the "only validated values reach the resolvers" invariant enforced by the compiler.Avatar route validation (
src/utils.ts,src/api.ts)parseQuerynow validates the stripped, lowercasedidwithavatarIdSchema(a valid address OR a valid handle/name). Invalid input throwsInvalidQueryError, which the route handler maps to HTTP 400 (a clear REST error, not a JSON-RPC-32602). The/clear/:type/:idroute gets the same 400 path.s/w/hsize query params is kept exactly as before.Branded / nominal types (
src/helpers/validation.ts)addressSchemaandhandleSchemaare now.brand()ed and exportValidatedAddress/ValidatedHandle.avatarIdSchema(ValidatedAddress | ValidatedHandle, exported asAvatarId) is the avatar route output. The schemas at both boundaries (JSON-RPC + avatar route) now emit branded values.Address/Handlealiases insrc/utils.tsare kept for values flowing THROUGH and OUT of the resolvers (resolved network values are ordinary strings); the brands are the validated INPUT types.Branded types threaded through the resolvers
lookupAddresses/resolveNames(src/addressResolvers/index.ts),lookupDomains(src/lookupDomains/index.ts),getOwner(src/getOwner/index.ts), and the avatar resolver entries (resolvers/ens.ts,resolvers/basename.ts,addressResolvers/basename.tsgetAvatar) now accept the branded input types. The compiler enforces that only schema-validated values reach them (the typecheck passing through the whole call graph is the proof branding is threaded correctly).Inner guards: removed vs kept
isAddressguard insrc/lookupDomains/index.ts. Its only caller validates withlookupDomainsSchemaat the boundary, so the brandedAddressalready guarantees shape.normalizeAddressesinsrc/addressResolvers/utils.ts. ItsgetAddresschecksum / lowercase step is normalization required for stable redis cache keys and the dedupSetand must stay; removing it would break cache keys. Its reject-filter is also kept because it still guards one unbranded path: theclearCacheroute param (/clear/address/:id) reaches it as a raw string. Cache key shape is unchanged..maxat the boundary, since types cannot encode array length.After this, no production path reaches the resolvers with an unbranded/unvalidated value.
Verification
yarn lint,yarn typecheck,yarn build: all pass.GET /avatar/not-an-address) returns 400, plus valid address / handle ids do not 400. The existing JSON-RPC invalid-params tests are kept. Integration test inputs are now branded via the real schemas (test/helpers/validation.ts).afterAllhook times out with no server). The single boundary-validation e2e case that does not need the network was run and passes. Please confirm the full suite in CI.Follow-up
The
rpcErrordefaultmessage: "unauthorized"is still misleading for the remaining generic error paths. Cleaning that up across the board is left as a small follow-up to keep this PR scoped.🤖 Generated with Claude Code