Skip to content

feat(microservices): add opt-in strict request envelope option - #17739

Closed
hktitof wants to merge 1 commit into
nestjs:masterfrom
hktitof:fix/strict-request-envelope
Closed

hktitof wants to merge 1 commit into
nestjs:masterfrom
hktitof:fix/strict-request-envelope

Conversation

@hktitof

@hktitof hktitof commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

the request deserializer treats any packet carrying a pattern or a data key as a native Nest packet, so a foreign payload like { type: 'alert', data: { ... } } reaches handlers unmodified and the handler silently reads the inner data while the rest of the payload is lost

the response side already rejects foreign JSON through an envelope-keys check (#17670), but the request side can't take the same treatment as a default flip bcs bare { data: X } packets are native today, as Kamil laid out when closing #17672

Issue Number: #17672, #17669

Steps to reproduce

  1. take the spec files from this PR, then run git checkout master -- packages/microservices/deserializers/incoming-request.deserializer.ts and npx vitest run packages/microservices/test/deserializers/
  2. Expected: a packet carrying a key outside the request envelope (id, pattern, data) counts as foreign and gets wrapped with the channel pattern
  3. Actual (raw output on untouched master, the strict option is swallowed bcs it does not exist there):
FAIL  packages/microservices/test/deserializers/kafka-request.deserializer.spec.ts > KafkaRequestDeserializer > with the strictRequestEnvelope option > should forward the option to the base deserializer
AssertionError: expected { type: 'alert', data: { topic: 'logs' } } to deeply equal { pattern: 'alerts', data: { …(2) } }

 Test Files  3 failed | 3 passed (6)
      Tests  3 failed | 21 passed (24)

What is the new behavior?

IncomingRequestDeserializer accepts an options object, new IncomingRequestDeserializer({ strictRequestEnvelope: true }). with the option on, isExternal mirrors the response-side check: a packet counts as foreign when it carries any key outside the request envelope (id, pattern, data), so { type: 'alert', data: ... } gets wrapped with the channel pattern and the handler receives the full payload

with the option off (the default) nothing changes for existing users, KafkaRequestDeserializer and NatsRequestJSONDeserializer inherit the option through the base constructor, and bare { data: X } packets stay native in both modes

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

follows the shape Kamil suggested in #17672: the same ENVELOPE_KEYS allowlist #17670 uses on the response side, shipped as an opt-in deserializer option, which keeps the two deserializers consistent

A foreign payload that carries a top level data key passes the request
gate today bcs the legacy check treats any packet with a pattern or
data key as native, so handlers receive the raw payload instead of a
wrapped packet, Kamil outlined the safe shape in nestjs#17672: an allowlist
of envelope keys like nestjs#17670 uses on the response side, shipped as an
opt-in deserializer option, bcs the stricter rule would still change
what native packets deliver

With strictRequestEnvelope on, a packet counts as foreign when it
carries any key outside the request envelope (id, pattern, data), so
{ type: 'alert', data: ... } gets wrapped with the channel pattern,
bare { data: X } packets stay native, and with the option off nothing
changes for existing users

KafkaRequestDeserializer and NatsRequestJSONDeserializer inherit the
option through the base constructor, nestjs#17670 gave the response
deserializer the same allowlist, this keeps both deserializers
consistent
@kamilmysliwiec

Copy link
Copy Markdown
Member

if the default is off, why do we need that need flag in the first place? if someone runs into this issue they can just:

class StrictDeserializer extends IncomingRequestDeserializer {
  isExternal(value: any) {
    if (!value || typeof value !== 'object') return true;
    const keys = Object.keys(value);
    return keys.length === 0 || keys.some(k => !['id', 'pattern', 'data'].includes(k));
  }
}

and pass their own deserializer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants