fix(validation): ZodEffects unknown-keys engine, abortEarly, SSRF guard - #8
Merged
Merged
Conversation
…t downgrade, allowUnknown fallthrough), real abortEarly, SSRF guard; 4.5x sync throughput, 25x async batch
- npm install was broken outright: unused fastify-zod dep peers fastify ^4
against fastify 5; removed. Added missing @fastify/swagger dep (imported by
server.ts) and bumped fastify-plugin to v5 to match fastify 5.
- Unknown-keys handling rebuilt: stripUnknown/allowUnknown/strict were silent
no-ops on any ZodEffects schema (every schema with .refine/.transform, incl.
registration); engine defaults silently replaced schemas' own .strict() with
.strip() (profileUpdate accepted unknown keys despite 'Prevent unknown
fields'); { allowUnknown: true } alone was ignored (stripUnknown default won
the if-chain). Now: effects are unwrapped and rebuilt, .strict() is never
downgraded by defaults (explicit caller options still can), precedence is
allowUnknown > stripUnknown > strict.
- abortEarly did nothing (both branches ran full validation); now honored in
the error payload (first issue only).
- Perf: static schema asyncness analysis routes provably-sync schemas through
safeParse, skipping per-call setTimeout + Promise.race; configured schema
variants cached in a WeakMap instead of reallocated per call. 115k -> 550k
ops/s (~4.5-4.8x) on a typical request schema. Runtime sync-probing was
rejected deliberately: safeParse on an async schema orphans the abandoned
refinement promise and a late rejection kills the process (verified).
- validateBatch gains opt-in concurrency (index-stable results/errors):
200 items x 5ms async refine 1059ms -> 42ms (25.4x). Default stays
sequential.
- Schema fixes: email .trim()/.toLowerCase() moved before .email() (padded
input was rejected); priceMin/priceMax falsy-zero bypass (priceMin=500,
priceMax=0 passed); timezone regex rejected most of the IANA db (3-segment
zones, UTC, Etc/GMT+8) -> Intl-based check; fileUpload accepted '.', '..',
trailing dots/spaces; webhook URLs accepted localhost/169.254.169.254/RFC1918
-> SSRF guard; dateOfBirth now accepts date-only + calendar-accurate age
(365.25 approximation drifted around birthdays); requireContentType matched
substrings ('application/jsonx' passed for 'application/json').
- validateData/validateBatch wrappers no longer drop context; engine accepts
transforming schemas (ZodType<T, Def, unknown>).
- +16 regression tests (37 total, all green); bench.ts + frozen original
engine included for reproducible numbers.
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.
Validation: the unknown-keys engine was a no-op for half the schemas — plus 4.5x/25x speedups
The
npm installinsrc/api/validationdoesn't even complete (fastify-zod@1.4.0peersfastify@^4against thefastify@5, and it isn't imported anywhere), so let's start there and work up.Broken install
fastify-zoddependency that hard-fails dependency resolution.@fastify/swagger—server.tsimports it,package.jsonnever declared it.fastify-plugin4 → 5 to match Fastify 5.The unknown-keys engine didn't do what it says
Three compounding bugs, all with regression tests:
stripUnknown/allowUnknown/ strict all hinge onschema instanceof z.ZodObject. Any schema with.refine()or.transform()is aZodEffects— including the existingUserSchemas.registration— so every unknown-keys option was a silent no-op for exactly the schemas most worth protecting. The engine now unwraps effects recursively and rebuilds the wrapper around the configured inner object..strict()silently downgraded.profileUpdateis declared.strict()with the comment "Prevent unknown fields". The engine's defaultstripUnknown: truereplaced it with.strip(), so unknown keys were quietly accepted on the one schema that explicitly forbade them. Engine defaults no longer weaken an author's.strict(); an explicit caller option still can.{ allowUnknown: true }alone was ignored. The if-chain checkedstripUnknownfirst, and the default istrue, so passing onlyallowUnknowndid nothing. Precedence is nowallowUnknown > stripUnknown > strict.Also:
abortEarlywas decorative. Both branches ran complete validation (safeParseAsyncvsparseAsync— identical issue lists; Zod v3 has no mid-parse abort). The option is now honored where callers can actually observe it: the error payload carries exactly the first issue.Performance
Single validate: 115k → 550k ops/s (~4.5–4.8x) on a typical request schema.
Two changes: (a) a one-time static walk of the schema tree decides whether it can possibly do async work; provably-sync schemas (params, query, pagination — the hot path) go through
safeParseand skip the per-callsetTimeout+Promise.race+ microtask machinery entirely; (b) configured schema variants (.strip()/.strict()/.passthrough()each allocate a whole new schema) are cached in aWeakMapinstead of rebuilt on every request.Why static analysis and not runtime probing: calling
safeParseon an async schema makes Zod start the refinement, then throw and abandon its promise — if that orphaned promise rejects, the process dies with an unhandled rejection (Node 15+). Verified empirically against zod 3.25, including zod's own~standard.validatewrapper which probes sync-first and has the same hazard. There's a regression test asserting no unhandled rejection when an async refinement fails after the timeout fires.Batch with async refinements: 1059ms → 42ms (25.4x).
validateBatchserialized every item — item N+1's DB-lookup refinement couldn't start until item N's finished. It now takes opt-in concurrency ({ concurrency: 32 }) with index-stable results and error fields. Default remains sequential; zero behavior change unless opted in.bench.ts+ the frozen original engine (bench-original-engine.ts) are included —npx tsx bench.tsreproduces every number above.Schema correctness
.toLowerCase().trim()ran after.email()(Zod applies string checks in chain order), so" User@Example.com "— the input the normalizers exist for — was rejected. Normalizers now run first.if (data.priceMin && data.priceMax)skips the check when either bound is0(prices are in cents; 0 is legal).priceMin=500, priceMax=0validated successfully. Now!= null.^[A-Za-z_]+/[A-Za-z_]+$failsAmerica/Argentina/Buenos_Aires,UTC, andEtc/GMT+8while acceptingFoo/Bar. Replaced with anIntl.DateTimeFormatcheck against the runtime's actual tz database..,.., and trailing dots/spaces — traversal primitives and Windows-hostile names that the character allowlist alone doesn't catch.https://localhost/,https://169.254.169.254/(cloud metadata), and all of RFC1918. Now rejected (loopback, private, link-local, CGNAT, and IPv6 equivalents), with the DNS-rebinding caveat documented for the HTTP client layer."1990-05-10"was invalid. Now accepts date-only or datetime, and the age check is calendar-accurate instead ofelapsed / 365.25, which drifts by up to a day around birthdays (someone turning 13 today could be rejected).application/jsonxpassed forapplication/json. Now compares the parsed media type exactly (parameters likecharsetstill stripped).validateData/validateBatchwrappers silently dropped thecontextparam; engine signatures now accept transforming schemas (ZodType<T, Def, unknown>), which is whyregistrationcould never be run through the engine's own typed API before.Verification
tsc --noEmitclean,eslintclean.npm run test:all: validation suite passes; the pre-existing failures onmain(nextjs-backend ×4, autocomplete, api-scenarios) are untouched — this diff is confined tosrc/api/validation/.Public API is backward compatible: every previously-passing call still passes with the same shape; the behavior deltas are exactly the documented bugs above.
— Lumen Industries