fix: drop unmatched properties when additionalProperties is false - #879
Merged
Tony133 merged 2 commits intoSep 8, 2026
Conversation
A schema that combines `patternProperties` with `additionalProperties: false`
leaked every property that the patterns did not match, instead of ignoring it.
`build({ type: 'object', patternProperties: { '^str': { type: 'string' } },
additionalProperties: false })({ str1: 'a', leaked: 'secret' })` returned
`{"str1":"a","leaked":"secret"}`. Removing `additionalProperties` entirely made
the same schema behave correctly, so the stricter setting produced the looser
output. README "Additional properties" states that when `additionalProperties`
is absent or `false`, every property not listed in `properties` and not matched
by `patternProperties` is ignored.
`buildExtraObjectPropertiesSerializer` in index.js gated its
`additionalProperties` branch on `additionalPropertiesSchema !== undefined`, so
the boolean `false` schema passed the guard and fell through to the non-`true`
else branch, which calls `buildValue` on it. `buildValue` serializes any
boolean schema as `json += JSON.stringify(value)`, emitting the property
verbatim. `buildInnerObject` only calls the function when
`schema.patternProperties || schema.additionalProperties` is truthy, which is
why the leak needed a sibling `patternProperties` to appear.
Exclude `false` from that guard so no branch is emitted for it and unmatched
keys fall off the end of the generated loop body. `additionalProperties: true`
and object schemas are unchanged.
Signed-off-by: Max Freedom Pollard <272618364+MaxFreedomPollard@users.noreply.github.com>
Tony133
approved these changes
Sep 8, 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
A schema that combines
patternPropertieswithadditionalProperties: falseserializes every property the patterns did not match, instead of ignoring it.Deleting the
additionalProperties: falseline from that same schema produces the correct{"nickname":"nick","matchnum":3}, so writing the stricter setting is what widens the output. README "Additional properties" states the intended rule: "If additionalProperties is not present or is set tofalse, every property that is not explicitly listed in the properties and patternProperties objects will be ignored".This matters beyond a formatting difference.
additionalProperties: falseis the way a response schema is written to guarantee that only declared fields reach the client, and here it is the one spelling that does not hold.Root cause
buildExtraObjectPropertiesSerializergates itsadditionalPropertiesbranch onadditionalPropertiesSchema !== undefined(index.js:387onmain). A booleanfalseschema is notundefined, so it passes that guard, and it is not=== true, so it lands in the else branch that callsbuildValue(context, propertyLocation, 'value')on it.buildValueserializes any boolean schema asjson += JSON.stringify(${input})(index.js:1306), which emits the property verbatim.buildInnerObjectcalls the function only whenschema.patternProperties || schema.additionalPropertiesis truthy (index.js:532). WithadditionalProperties: falseand nopatternPropertiesthat guard is false and the function never runs, which is why the leak requires a siblingpatternPropertiesto surface and whyadditionalProperties: falseon its own has always been correct.Here is the generated loop body for the schema above, before the fix. The final three lines are the emitted
falsebranch:Fix
Exclude
falsefrom the guard, so no branch is emitted for it and an unmatched key simply falls off the end of the loop body and is dropped:additionalProperties: truestill goes through itsJSON.stringifyfast path, object schemas still go throughbuildValue, andadditionalProperties: falsewithoutpatternPropertiesis untouched because that path never reached this function.Tests
Two cases added to
test/additionalProperties.test.js, one with a declaredpropertiesmap and one without, each asserting the exact output string and that it round-trips throughJSON.parse. Both fail onmain('{"str1":"a","leaked":"secret"}' == '{"str1":"a"}') and pass with the fix.Verification run on Node 22.23.2, macOS arm64, from a clone of
mainat e02b5bb:npx c8 --100 --all --include=index.js --include='lib/**/*.js' --exclude=lib/schema-validator.js node --test --test-concurrency=2gives 506/506 passing and holds 100% statements, branches, functions and lines onindex.jsand every file underlib/, which the--100threshold enforces.npm run lintis clean andnpm run test:typescriptpasses 14/14 assertions. No type change is needed:types/index.d.ts:136already declaresadditionalProperties?: Schema | boolean.npm run benchmarkcompletes with no errors. I am not reporting a delta from it, because no scenario inbenchmark/usespatternProperties, so the changed branch is never generated there and any difference in those numbers would be machine noise rather than an effect of this patch.No documentation change: README already describes the behaviour this restores.
Checklist
npm run test && npm run benchmark --if-presentand the Code of conduct