feat(core): schema DTOs survive class-validator whitelist pipes (#83) - #89
Open
tnramalho wants to merge 32 commits into
Open
feat(core): schema DTOs survive class-validator whitelist pipes (#83)#89tnramalho wants to merge 32 commits into
tnramalho wants to merge 32 commits into
Conversation
Introduce query/command builders with Zod→DTO OpenAPI, fail-closed validation, transport-agnostic OperationContext, and a sample-server ops demo so consumers can copy the pattern. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the hand-written PetTransferController with operationResource over the existing CQRS handler, and drop the toy /ops ping/shout sample. Co-authored-by: Cursor <cursoragent@cursor.com>
CI failed YN0028 — package.json listed @types/express but yarn.lock did not. Express types are no longer imported after OperationRequest façade; remove the unused dep instead of churning the lockfile. Co-authored-by: Cursor <cursoragent@cursor.com>
* main: Harden repository and public API quality gates (#48)
CI api:report:check-built fails after #48 gate — record ResourceKind.Operation, defineOperationResource, and the operationResource zod surface as reviewed additive public declarations. Co-authored-by: Cursor <cursoragent@cursor.com>
Lock the zod authoring API before it ships so the wire contract is readable from the source and the compiler enforces intent instead of comments. - Builders are method-constrained: op.read (GET), op.write (POST/PUT/PATCH), op.delete (DELETE), replacing query/command. - operations is callback-only, so base-path :params can type ctx.params. - Operation path defaults to its key verbatim; path: '' mounts at root. - Drop the dead kind field: HTTP method already decides input sourcing. - Reject operation keys that would shadow Object.prototype members or the generated controller's ModuleRef field. Co-authored-by: Cursor <cursoragent@cursor.com>
Catch response leakage, incoherent 204+body, bad path params, and cross-resource route collisions when the app is planned — not after a wrong route ships to production. Preserve Nest path params that sit outside the resource-level params schema so op-path segments are not stripped by whitelist validation. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Use the minimal adapter inspection contract so route-validator tests typecheck in CI. Refs #49
Honor module/DynamicModule exports when collecting operation handlers so an explicit import provider is not shadowed by a trailing class registration. Co-authored-by: Cursor <cursoragent@cursor.com>
## `{ useClass }` was rejected before it could be interpreted
`assertOperationHandler` in the zod path required `typeof handler ===
'function'`, so the tagged form documented in CONFIGURATION.md and the
core README compiled and then threw during `operationResource()`
construction. It exists precisely for the shape no runtime check can
identify — a class whose `handle` is an instance field — so rejecting it
made the escape hatch unusable. Now accepted, with the malformed case
still failing fast.
## Module re-exports were not followed
`collectImportedExportTokens` read only an imported module's direct
`exports`. Nest re-exports transitively: `exports: [InnerModule]`
publishes everything Inner exports. A handler reachable through
`Outer → Inner` therefore looked unsupplied, was auto-registered in the
generated module, and shadowed the imported provider — and when its
dependencies were private to `InnerModule`, the app failed to boot.
Export walking is now transitive across module classes and
`DynamicModule` re-exports, with a `visited` guard for the
mutually-re-exporting case Nest tolerates.
That exposed the second half: not registering the handler locally meant
`moduleRef.resolve(..., { strict: true })` could no longer find it. The
strict resolve is what stops two operation resources sharing a handler
CLASS from resolving each other's instance, so it stays — for handlers
this module actually registers. A handler supplied by an import has no
local provider to be strict about and resolves through the module's
injector, where exactly one provider for that token is reachable.
`defineOperationResource` now passes the locally-registered token set to
the controller builder, which picks the mode per handler.
Covered by a bootstrap e2e whose handler depends on a value private to
the inner module — the case that used to fail at boot, asserted by
serving a request rather than by inspecting providers.
## Operation IDs and DTO names were not unique
`controllerName + method + key` collides for configurations the planner
accepts: two bundles on one base path can each declare a `GET` keyed
`action` with different explicit paths. Both routes are legal and
distinct; both received one operation ID and one DTO name, so Swagger
pointed them at a single component and the second schema overwrote the
first. The operation's own path is now part of the discriminator —
appended only when it differs from the key, so the common case keeps its
short name. Shared with the zod DTO namer so IDs and components stay in
step.
## Route dimensions were discarded for operation resources
`collectOperationRoutes` hardcoded `host`/`version` to `undefined`, so a
v1 CRUD `GET /widgets` beside an operation `GET /widgets` carrying
`Version('2')` was rejected as a duplicate although Nest routes them
apart. Both are now read off the GENERATED controller — method-level
version wins over the controller's, matching Nest.
(Nest's `Version` is a method decorator: it always writes to
`descriptor.value`. The regression test uses per-operation `decorators`
accordingly.)
## Wildcard overlap (P2)
`segmentListsMayOverlap` returned `true` at the first wildcard and
dropped every remaining segment, so `a/*rest/x` was reported as
overlapping `a/y/z` — a false positive that rejects a valid
configuration. Wildcards now match against zero-or-more segments with the
suffix still required to align. A match that can ONLY be reached by a
wildcard absorbing zero segments depends on router semantics that vary by
version, so it returns `'unknown'`, which the collision validator already
reads as "not proven" and accepts.
A third review pass looked only at whether the previous round's fixes
created new problems. Four did. All verified against the installed Nest
dist, not against docs.
## Handler resolution escaped its module (HIGH)
The previous fix resolved an imported handler with `strict: false`. That
is not "the module that owns it", as the docstring claimed:
`instance-links-host.js` returns `links[links.length - 1]` when given no
module id — the last module scanned app-wide, unrelated to the import
graph. One handler class exported by two modules would silently give both
operation resources the same instance, with per-boot arbitrary ordering.
It traded a loud `UnknownElementException` for a quiet wrong answer.
Resolution is strict again, always. For a handler this module does not
register, `defineOperationResource` now adds a local
`{ provide: Symbol(...), useExisting: HandlerClass }`: `useExisting`
resolves through normal DI so imports are honoured, while the token the
route asks for is local, so the strict lookup is unambiguous by
construction.
## Transitive export walking missed the dynamic-module host (HIGH)
`isModuleClass` tested for `@Module()` metadata keys. `@Module({})`
writes NONE — the decorator only defines keys present in the object it
receives — and that is exactly the canonical
`@Module({}) class X { static forRoot(): DynamicModule }` shape of every
`@concepta/nestjs-*` module. So `exports: [BillingModule]` re-exporting a
dynamic module was invisible, the handler looked unsupplied, and with a
dependency private to that module the app failed to boot with the very
error the previous fix was meant to remove.
A metadata-less exported class is now matched against the host module's
own `imports` for a `DynamicModule` whose `.module` is that class, and
its exports are walked. Covered by a bootstrap e2e that reproduces the
original failure message.
## Operation ids were still collidable (MEDIUM)
`operationDiscriminator` slugifies non-alphanumerics to `_`, which is not
injective: `{ key: 'run', path: 'a' }` and `{ key: 'run_a' }` both yield
`run_a`, as do paths `a/b` and `a-b`. The routes differ, so the path
check passes, and the second OpenAPI component silently overwrites the
first.
Rather than chase an injective slug, the planner now asserts operation-id
uniqueness directly and names both sources. The discriminator goes back
to being a readability optimisation instead of something correctness
rests on.
## Route dimensions produced false negatives (MEDIUM)
`dimensionsMayOverlap` compared `String(value)`. `VERSION_NEUTRAL` is a
Symbol that matches every version in Nest; stringified it compares
unequal to `'1'`, so a version-neutral route sharing a path with a v1
route was reported as disjoint and accepted — then last-wins at runtime.
RegExp and pattern hosts had the same problem.
The dimension half now mirrors the discipline the route-pattern half
already had: only a literal-value inequality may prove routes disjoint.
`VERSION_NEUTRAL` and undecidable hosts count as overlapping, because a
missed collision is silent while an over-reported one is a boot error the
author can resolve.
Refs #43, #50
Issue #50 freezes the `operationResource` authoring surface before 1.0, so this is the last moment these two cost nothing. ## `outputDto?` + `outputDisabled?` → `output: Type<object> | false` One decision was modelled as two optional fields, which made two meaningless states expressible — both set, and neither set — and `defineOperationResource` had to reject each at runtime. Required and closed, they cannot be written, so there is nothing to reject: two of the three validations are deleted rather than kept. Only the genuine rule survives (`status: 204` with an output). The authoring layer already modelled it correctly — `op.read({ output })` has always taken `schema | false`. The compiled form was the half that disagreed; now both say the same thing. The spec that asserted the runtime rejection is replaced by a `@ts-expect-error` pin: the guarantee moved from a throw to the type, and the pin fails if the field ever becomes optional again. ## `PendingBuilder` unexported `'read' | 'write' | 'delete'` was a named export. Publishing the builder taxonomy means adding `op.stream` or `op.upload` later is a breaking change to a union nobody constructs — consumers write `op.read(...)` and never name the type. It is now referenced-only (via `PendingOperation` and `OperationRecord`, which stay public because #54's typed-client work consumes `authored`), so it can widen without breaking a contract. BREAKING CHANGE: `CompiledOperationDescriptor.outputDto` / `outputDisabled` are replaced by a required `output: Type<object> | false`. Only hand-built descriptors are affected — `operationResource()` and `defineOperationResource()` callers are not. `PendingBuilder` is no longer exported from `@concepta/rockets-core/zod`. Refs #50
`da24cff` moved `AccessControlQuery` from the controller class to each route, but the `acl` doc comment — the public interface documentation for the option — still described the class-level behaviour it replaced. That inverts the meaning of the feature. Class-level stamping is precisely what made a per-operation `query` unable to override anything: upstream merges with `getAllAndMerge([getClass(), getHandler()])` and breaks on the first service returning `true`, class-first, so the permissive entry wins. Anyone reading the old text would conclude the opposite of what the code now guarantees. Adds the invariant next to the description so the reason survives the next edit.
Four findings, re-verified against b838177 and still reproducing behind green CI. Two were the incomplete half of the round-2 fixes: the case in the reproduction was closed, the space the public types admit was not. ## One namer for operation ids and component names Operation ids keyed off an underscore slug while DTO names pascal-cased, so `foo-bar` and `fooBar` received DISTINCT ids and ONE component name. The path check and the id check both passed, and the second schema silently replaced the first in the generated document. Both now derive from `operationDtoBaseName`, and the planner asserts the result rather than trusting the transform — `pascal` stays lossy on purpose, because making it injective would be a guess about which characters carry meaning where asserting is total. Same reasoning as the neighbouring id assertion. The assertion compares class IDENTITY, not the name plus where it was declared. One compiled DTO reused as the output of several operations is one class and one component; an earlier revision of this commit compared names, rejected that configuration, and simultaneously swallowed a real collision between two bundles sharing a base path because their source labels matched. `paramsDto` is deliberately not claimed: path params are emitted as inline `ApiParam` entries and never occupy a component. `nameGeneratedDto` is now the single place a generated name is minted. The `z.array` branch of the DTO compiler and `namedZodDto` both bypassed the brand, and `z.array` is the documented shape for a list endpoint, so the most common output was invisible to the very check meant to protect it. ## Module exports are a union, on every path Nest unions a module's static `@Module` exports with the dynamic ones its factory returned, and unions imports the same way (`@nestjs/core/scanner.js:55-57` and `:151-155`). Three shapes each lost one half: - `Host.forRoot()` imported directly, exports declared statically - a host populating BOTH halves, reached through a re-export - a wrapper returning `{ imports: [Inner.forRoot()], exports: [Inner] }` In each case the handler was considered unsupplied, re-registered locally, and failed to resolve a module-private dependency — or, worse, resolved and ran as a silent duplicate instance. All three now have an e2e that fails without the fix. ## A non-object payload is rejected, not narrowed `POST []` against `z.object({ note: z.string().optional() })` returned 200 with an empty input: the body was narrowed to `{}` before validation, and the same narrowing bypassed any all-optional class-validator DTO. Substituting a valid value for an invalid one is not something a validation boundary should do quietly. The predicate checks the prototype rather than `typeof value === 'object'`, so a `Buffer` from a raw parser is rejected too. A MISSING body still becomes `{}` — a `POST` with no payload against an all-optional input stays legal, and a required field still fails with its own field-level message. Express's query object is null-prototype, which the check accounts for. ## Documentation that claimed a guard it does not have `ResourceOperationConfig.acl` said combining `acl` with a manual `AccessControl*` decorator is "rejected at definition time". There is no such check. Worse, the old text called upstream's behaviour a merge: the grant is a plain `SetMetadata` write read with `reflector.get`, the route applies `decorators` first and the `acl` grant last, so `acl` deterministically OVERWRITES a hand-written grant — a manual grant deliberately tighter than the inferred action is discarded in silence. The doc now says that, points at CONFIGURATION.md §5a rather than §7, and records that detection is decidable at controller-build time and simply not implemented. §5a is aligned, and its stale "v1 does not wire ACL grants" line is gone.
Adversarial review of the just-merged route policy audit (#76) returned BLOCKED: the load-bearing judgement — "is this app guarded?" — was wrong in both directions, and the composition the README advertises could not use the feature at all. ## Counting guards is not authentication The audit counted APP_GUARD provider wrappers. Upstream access-control registers one unconditionally and resolves it to `null` under `appGuard: false`, so an app with ZERO authentication reported every route as guarded and `requireAuth` booted green — the permissive lie this feature exists to remove, told by the feature itself. In the other direction, request-scoped guards live in Nest's `injectables`, invisible to `DiscoveryService.getProviders()`, so a correctly guarded app hard-failed. Both defects trace to one false comment: "`ApplicationConfig` is not in `@nestjs/core`'s export map." It is — via `export * from './application-config.js'`, which a grep for the symbol name cannot see. A bad grep became a load-bearing design constraint. The service now reads `getGlobalGuards()` (resolved instances, nulls filtered) and `getGlobalRequestGuards()`, and classifies: a route is `guarded` only when a guard RECOGNISED as authentication is present — `AuthServerGuard` or classes in `routePolicy.authGuards`, subclasses included on both the instance and metatype branches. A throttler or an ACL guard is a global guard that authenticates nothing. ## The flagship composition could not declare a policy `defineRocketsAuth()` swaps the global guard for upstream `JwtGuard` (`providesAppGuard`), so every route in a built-in-auth app read `unguarded-app` and any declared rule aborted the boot — on exactly the routes the README promises to cover. The contributions seam declared THAT an integration owns the guard but not WHICH class. It now carries `authGuards`; `defineRocketsAuth` contributes `[JwtGuard]` (or a custom `appGuard` instance's constructor), and the SERVER composition merges them into the forwarded policy — the merge cannot live in core, which rejects contribution-carrying bootstraps outright. An earlier revision of this commit put it there anyway; it was unreachable code and is gone. ## Smaller truths - `acl-metadata-keys` reads `AccessControlGrant().KEY` — the key `SetMetadata` attaches to the decorator it returns — instead of a 75-line throwaway-class probe. Resolved lazily, throws loudly: a missing key must never degrade into every-route-ungranted silence. - `allowControllers` matches by class identity; a decoy class sharing the name no longer exempts anything (e2e proves the decoy fails). - `staleAllow` fires only while at least one rule is declared: a recognition-only policy polices nothing, and aborting a boot over list hygiene when nothing is enforced would make the audit the incident. - Path arrays (`@Controller(['a','b'])`) report one row per combination. - Docs de-overclaimed twice: the README's staleness sentence and the same sentence surviving unqualified in the public JSDoc. Every mechanism here is pinned by a test that fails when the mechanism is reverted — verified for each by reverting it: wrapper-counting, name-based exemption, the missing merge, the staleAllow gate, first-path collection, and the dropped contribution all turn tests red.
…ce-43 # Conflicts: # CHANGELOG.md # packages/rockets-core/CHANGELOG.md # packages/rockets-core/README.md # packages/rockets-core/package.json
A production field report (issue #83): hand-written controllers using schema-carrying DTOs (`createZodDto`) under a global `ValidationPipe({ whitelist: true })` receive `{}` — the whitelist strips every property without class-validator metadata AFTER the schema already validated the body. Silent, with a success status, which is what makes a consumer delete their manual safety net. ## Three pieces, each with the failure mode it removes **One predicate.** `getCarriedStandardSchema` in the standard-schema subpath is the single definition of "this class carries a schema"; every internal call site imports it, and the tolerant same-named alias in common/utils is gone — a fix written against the wrong of two opposite-contract predicates imports fine and misbehaves at runtime. The DTO pipe now recognises carriers, not just Rockets-branded classes: a bare nestjs-zod DTO was silently skipped by the brand check. **The stamp.** `allowStandardSchemaKeys` writes class-validator `@Allow()` per declared key, so a schema DTO survives ANYONE's whitelist pipe — including pipes Rockets will never own. Generated `compileDtoClass` output ships stamped. The stamp is SURVIVAL, not validation, and the docs say so. It refuses OPEN schemas (catchall/passthrough): stamping declared keys there would let the whitelist strip keys the schema itself accepts — issue #83's silent loss reintroduced through the fix. `compileDtoClass` bypasses that refusal deliberately (an open generated-only resource must not become a boot failure), with the cost named in the README. **The aware pipe.** `StandardSchemaAwareValidationPipe` VALIDATES schema carriers with their own schema. The first draft SKIPPED them, trusting that "something upstream validated" — an assumption it never checked, which meant the pipe used standalone (exactly how the README presented it) disabled all validation: `{ name: '', evil: 'x' }` reached the handler with a 201. Adversarial review blocked it; the shipped pipe has no such trust. Register exactly one schema validator per route — pairing double-parses, and a transforming schema is not idempotent (pinned with `.transform()`, after the first "idempotence" pin was shown to use a schema that could not fail). `transform` and `errorHttpStatusCode` are forwarded so both DTO kinds fail alike; a DTO carrying BOTH a schema and class-validator constraints is rejected loudly as ambiguous, with `@Allow()` metadata never counting as a constraint (probed: it registers as 'whitelistValidation' — the first draft compared against 'whitelist' and would have rejected every DTO Rockets stamps). The trap itself stays pinned: unstamped DTO + plain whitelist pipe still yields `{}` in an e2e, so the hazard the docs describe cannot silently go stale. Two review rounds (one BLOCKED, one REVISE) shaped this commit; every mechanism fails a test when reverted, including two tests the review exposed as vacuous (a metatype the pipe never saw via `InstanceType<typeof X>` emitting `Object`, and the false idempotence pin) that were rewritten to bite.
Two reviews ran over this branch on the owner's order — a clean-room
seal with no prior context, and a separate automated review pass — and
their cross-checked findings surfaced eleven defects that three rounds
of external review had not reached. Every fix below is pinned by a test
proven to fail with the mechanism reverted.
## The serialization leak (both reviews, independently)
Restoring free-form JSON columns had dropped `strategy: 'excludeAll'`
from the outbound options. Without it, an `@Expose()`d relation with no
`@Type()` — the common hand-written class-DTO shape — emitted the FULL
child row where the projection yields `{}`: `owner.passwordHash` on the
wire, proven by probe. Reachable wherever rows are plain objects
(Firestore-style adapters, JSON columns, handler-returned data);
TypeORM-hydrated instances emptied either way. The strategy is
restored, `@FreeFormJson` is required on response DTOs as well as
inputs, and the leak is pinned in unit and e2e so it cannot ship a
second time. The one test asserting the old behaviour was asserting the
leak mechanism as a feature; it now asserts the safe contract.
## ACL truth and loudness
- `acl` + a hand-written `AccessControl*` on one operation now fails at
definition time — grant metadata is last-write-wins, and the
combination silently REPLACED a possibly tighter manual rule. The
read-back covers BOTH keys: the first cut checked only the grant, and
a manual `AccessControlQuery` (a deliberately tighter row filter) was
still being clobbered — the sibling slot of the defect just fixed.
`acl: false` plus manual decorators stays legal (single writer), and
CRUD keeps its documented plan-time limitation; both pinned.
- The route audit reads `AccessControlQuery` from handler THEN class,
mirroring upstream's `getAllAndMerge([class, handler])` — auditing
the handler alone aborted the boot of correctly-enforced apps.
## Validation and error-path resilience
- CRUD-vs-CRUD route collisions are checked in operation-free apps: an
early return had silently gated the entire check on "any operation
bundle exists".
- Route matching is compared case-insensitively, as Express routes.
- Nested class-validator failures name the failing field
(`child.street: ...`) instead of answering `message: []`.
- A throwing custom error serializer falls back to the default envelope
instead of replacing every error response with the adapter's bare 500.
- Hook-hidden columns no longer reappear on create responses.
## Dependency-wiring traps
- Handlers with request-scoped dependencies resolve `REQUEST`:
the generated route now registers the request under the minted
context id before resolving.
- A `forwardRef` whose factory throws (the TDZ circular-import case) is
a sentinel, not a swallowed error — auto-registering a handler next
to one refuses loudly and asks for explicit `providers`, in the
top-level imports AND one frame down inside a module's re-exports,
where the first cut left the silent-duplicate defect alive.
- `path-to-regexp` stays a range matching @nestjs/core's own, with the
reasoning recorded where the next editor will look: an exact pin
guarantees a second copy the day Nest bumps.
Deliberately NOT here, flagged for the owner as API decisions: a typed
transaction context on `OperationContext`, and an allow-style opt-out
for the planner collision check.
Four P1s, revalidated by the reviewer against f1f3ef3 and confirmed current. Each fix is pinned by a test proven to fail with the mechanism reverted — including two whose mutations reproduce the reviewer's exact error messages. ## The module walk descends through ONE function, everywhere Three prior fixes each taught one call site to carry a dynamic module's imports; the re-export resolution path still walked exports without them, so a three-level chain (Platform → Billing → Payments, each re-exporting the bare host class) failed at exactly the depth that site reached: Payments never matched, its handler was re-registered locally, and its module-private symbol failed the boot. Every descent now goes through `addDynamicModuleExports`, which carries BOTH import halves (dynamic and static — Nest unions them, scanner.js:55-57) and the host class's static exports. The fourth variant of a bug is the point where per-site patches stop being fixes. ## Route ids carry the routing dimensions they ignored A public v1 and a guarded v2 of one METHOD+path are different wire routes; one unqualified id collapsed them, and a single `allow` entry exempted BOTH — the silently-widening exemption the design document forbids. Ids gain deterministic qualifiers when the dimension is declared (`GET /widgets [v1]`, `[host:...]`), an unqualified entry that matches nothing fails as stale, and an entry that still matches MORE than one row fails closed as ambiguous. ## Promised injection is real injection `RouteAuditService` was registered but never exported: `app.get()` passed through the container-wide lookup while a consumer module's `inject: [RouteAuditService]` failed DI — the docs promise the latter. Exported under the same `routePolicy` condition; the pin is a real consumer-module factory, and its mutation reproduces the reviewer's exact resolution error. ## The documented import exists `FreeFormJson` was reachable only by deep source import; the README's root-entry example was `undefined` at runtime. Exported from the root barrel (with the serialization option constants), and the e2e now imports it THROUGH the root barrel so the documented path cannot drift from the shipped surface again.
# Conflicts: # CHANGELOG.md # packages/rockets-core/src/infrastructure/resource/operation-resource/build-operation-controller.ts # packages/rockets-core/src/zod/zod-dto.ts
tnramalho
commented
Aug 21, 2026
| typeof def === 'object' && def !== null | ||
| ? Reflect.get(def, 'catchall') | ||
| : undefined; | ||
| if (catchall !== undefined && catchall !== null) { |
Collaborator
Author
There was a problem hiding this comment.
allowStandardSchemaKeys() rejects closed Zod .strict() object schemas as “OPEN”. In Zod 4, .strict() is represented as def.catchall = ZodNever, so this condition treats a closed schema as catchall/passthrough. That breaks the documented “closed object schemas can be stamped” path for strict DTOs. Fix by treating ZodNever / def.type === "never" as closed, and add a test for z.object({ a: z.string() }).strict().
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
2 participants
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.
What
Issue #83 from a production field report: hand-written controllers using schema DTOs (
createZodDto) under a globalValidationPipe({ whitelist: true })receive{}— validated by the schema, then emptied by the whitelist, silently, with a success status.Closes #83
Three pieces:
StandardSchemaDtoValidationPipenow recognises any class carrying a Standard Schema as its staticschema(barenestjs-zodDTOs included), via one predicate (getCarriedStandardSchema) defined once and imported everywhere; the tolerant same-named alias is deleted.allowStandardSchemaKeys(dto)writes@Allow()per declared key so a schema DTO survives anyone's whitelist pipe;compileDtoClassoutput ships stamped. Open (catchall/passthrough) schemas are refused — stamping them would strip keys the schema itself accepts.StandardSchemaAwareValidationPipeVALIDATES schema carriers with their own schema; register exactly one schema validator per route. Rejects the ambiguous both-schema-and-constraints shape loudly.What two review rounds changed
The seal BLOCKED the first version and reshaped the design — the notable catches, each now pinned by a test proven to fail when its mechanism is reverted:
{name:'', evil:'x'}→ 201). It validates now.compileDtoClass's deliberate bypass named in code and docs.InstanceType<typeof X>emitsObjectinto reflected param types), and an "idempotence" pin using a schema that could not fail — replaced by a transforming-schema test showing why pairing pipes is wrong.Escapes documented, limits stated
README names all three escapes (including the
@Body({ schema }) dto: TypeAliasidiom sample-server already uses), what each protects against, and what none of them do: the stamp is survival, not validation. The original trap stays pinned by an e2e so the hazard description cannot go stale.@kauandotnet — two of these pieces sit on your #75 design: recognition widened from brand-gated to carrier (was brand-only deliberate?), and the aware pipe's contract (validate-in-place). Both flagged for your ruling.
Gates
Build,
api:report:check-built,typecheck:spec, 838 unit, 309 package e2e,lint:all. 12 e2e + 5 unit new.Stacked on
feat/operation-resource-43(#49); retargets on its merge.🤖 Generated with Claude Code