feat(core/zod): opt-in strictInput — reject unknown top-level body keys (#79) - #85
Open
tnramalho wants to merge 32 commits into
Open
feat(core/zod): opt-in strictInput — reject unknown top-level body keys (#79)#85tnramalho 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 zodResource create today accepts `{ "modelId": "x", "unexpected": 1 }`
with 201 and silently drops the unknown key: a plain `z.object` strips
during parse, so a client believes a strict API validated a body it in
fact discarded. Products needing strict creates were attaching a
hand-written guard per route, duplicating the schema the projection had
already derived (issue #79, from a production field report).
`strictInput: true` on a body operation compiles the EFFECTIVE input
schema — the derived projection, or an `input` override, whichever is
in use — with `.strict()`, so unknown keys become zod issues and the
existing body-validation interceptor answers 400 naming them. One
helper replaces the three inline DTO compilations precisely so the flag
cannot apply to one schema source and miss the other. Declaring it on a
bodyless operation fails at definition time. The default is unchanged:
stripping is the long-standing generated-CRUD contract, and flipping it
under existing consumers would turn tolerated clients into broken ones.
A boolean rather than an `unknownKeys: 'strip' | 'reject'` mode was an
explicit owner decision: the third mode ("warn") has no demand and no
channel today, and the house rule is not to widen a published surface
for a hypothesis.
Adversarial review caught three overclaims in the first version of this
change, now corrected and pinned by tests rather than asserted:
- `.strict()` is TOP-LEVEL only — an unknown key inside a nested object
is still stripped. Documented, and pinned by an e2e so a future
deep-strict zod flips a test rather than silently changing contract.
- Fields the projection excludes (`id`, timestamps, `version`, owner
columns) are REJECTED under strict, so echoing a fetched row back
into a strict `replace` is 400 — proven over HTTP, sold in the README
with its upside (an owner-column spoof is named, not overwritten) and
its cost (read-modify-write clients strip server-owned keys first).
- `additionalProperties: false` appears in the OpenAPI document only
after `nestjs-zod`'s `cleanupOpenApiDoc` — which no doc in this repo
had ever mentioned. The e2e asserts both halves: strict DTO gains the
keyword through cleanup, non-strict DTO does not.
Every mechanism is pinned by a test that fails with the mechanism
reverted — verified by reverting: dropping `.strict()` fails the
derived AND override cases.
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.
…e-79 # Conflicts: # CHANGELOG.md # api/public-api-reports.json # packages/rockets-core/README.md # packages/rockets-core/src/__e2e__/rockets-core-zod-operation-io.e2e-spec.ts # packages/rockets-core/src/zod/compile-zod-resource-core.ts # packages/rockets-core/src/zod/zod-operations.ts
…gaps Clean-room review findings on #85: - The echoed-back-row test passed without `.strict()`: the 400 came from `meta: null` failing `f.string().optional()`, and `toMatch(/id/)` matched "Invalid". The fixture now uses `f.string().nullish()` so the echoed row round-trips the input schema, and the assertion names the rejected key. Mutation-proved: stubbing `.strict()` out turns this test (and the new ones below) red. - `compileZodCore` has two callers but only `zodResource` was covered: added a `zodSubResource` strict create case. - The OpenAPI cleanup test now also asserts `additionalProperties: false` on an `input`-override strict DTO, not just the derived one. - `strictInput: false` (e.g. a computed flag) on a non-body operation no longer throws "no request body to be strict about" — only `true` is a config mistake there. - Inlined the single-valued `kind` parameter left over in `overrideDto`. - README: state explicitly that core's `SwaggerUiModule` does not run `cleanupOpenApiDoc`, so the app bootstrap must, or generated clients learn the strict contract only via 400. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Opt-in
strictInput: trueon zodResource body operations (create/update/replace): unknown top-level JSON keys return400naming the offending keys, instead of being silently stripped. Applies to whichever input schema is in effect — the derived projection or aninputoverride. Default unchanged.Closes #79
The contract, stated honestly
Adversarial review caught three overclaims in the first draft; each is now a documented limit pinned by a test rather than a marketing line:
.strict()does not recurse; nested objects still strip. Pinned by e2e.id, timestamps,version, owner columns are outside the input projection, so echoing a fetched row into a strictreplaceis400— proven over HTTP. Upside: an owner-column spoof gets named instead of silently overwritten.additionalProperties: falseappears only once the document passesnestjs-zod'scleanupOpenApiDoc— a step no doc in this repo had ever mentioned. The e2e asserts both halves (strict →false, non-strict → absent).strictInput: booleanoverunknownKeys: 'strip' | 'reject'was an explicit owner decision — no demand and no channel for a third mode today.Coverage
9 new e2e cases: repro 400 naming the key; clean 201; override+strict; derived-update+strict; strict
replaceecho-back; non-strict op on the same resource keeps stripping; nested-key top-level pin; bodyless misuse throws at definition time; OpenAPI keyword through cleanup. Mechanism verified load-bearing by reverting.strict()(derived and override cases both fail).Gates: build,
api:report:check-built,typecheck:spec, 833 unit, 306 package e2e,lint:all.Notes
feat/operation-resource-43(feat(core): operationResource typed non-CRUD endpoints (#43, #50) #49) — same files, would conflict frommain. Do not merge before feat(core): operationResource typed non-CRUD endpoints (#43, #50) #49; retargets automatically.strictInputparity foroperationResourcewrite ops — the sibling surface, tracked from the review.🤖 Generated with Claude Code