fix: validate discriminated oneOf schemas against only the branch selected by the discriminator in the drift command - #2957
Conversation
…ected by the discriminator in the drift command
🦋 Changeset detectedLatest commit: 10645f2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
Performance Benchmark (Lower is Faster)
|
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ce416ab. Configure here.
Co-authored-by: Jacek Łękawa <164185257+JLekawa@users.noreply.github.com>
| // Ajv rejects loosely defined discriminators at compile time (for example | ||
| // when the tag property has no `const`/`enum` or is not required); such | ||
| // schemas fall back to being validated against every oneOf branch. | ||
| this.fallbackAjv = new AjvConstructor({ ...ajvOptions, discriminator: false }); |
There was a problem hiding this comment.
How does the lint handle this case? i cant find any fallback
There was a problem hiding this comment.
Lint doesn't have a fallback — it just reports that Ajv couldn't compile the schema (the try/catch is in validateExample in packages/core/src/rules/utils.ts), which is fair there, since it points to an issue in the OAD itself. Drift is a different story. Lint only compiles a schema when it has an example attached, so even a spec with no lint issues can contain a loose discriminator. Without the fallback, drift would then report "Schema compilation failed" for every exchange hitting that schema — errors about perfectly valid traffic. The fallback is used in compileSchema() at the bottom of this file: if the strict instance throws, the schema is recompiled with discriminator: false, which validates against every oneOf branch — same as drift did before this PR.
There was a problem hiding this comment.
Looks good, the const/enum + required constraints come from Ajv, not the spec (docs), so the fallback is ok
|
Reviewed the diff. Overall this looks good — no blockers, all 44 checks green, sound design. A few optional nits and one question below. What it does: fixes false positives in Notes:
Tests: good — the new Backward compatibility checks out — the fallback faithfully reproduces the old behavior ( Generated by Claude Code |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 10645f2. Configure here.
| // Ajv rejects loosely defined discriminators at compile time (for example | ||
| // when the tag property has no `const`/`enum` or is not required); such | ||
| // schemas fall back to being validated against every oneOf branch. | ||
| this.fallbackAjv = new AjvConstructor({ ...ajvOptions, discriminator: false }); |
There was a problem hiding this comment.
Missing schema validation disable
Medium Severity
Enabling discriminator: true without validateSchema: false leaves Ajv's default schema checks on. Ajv's discriminator meta-schema disallows properties beyond propertyName and mapping, so valid OpenAPI discriminators with defaultMapping or extensions fail compilation and silently use the no-discriminator fallback, keeping the noisy oneOf false positives this change aims to remove. Elsewhere in the repo, discriminator: true is paired with validateSchema: false.
Reviewed by Cursor Bugbot for commit 10645f2. Configure here.
| // Ajv rejects loosely defined discriminators at compile time (for example | ||
| // when the tag property has no `const`/`enum` or is not required); such | ||
| // schemas fall back to being validated against every oneOf branch. | ||
| this.fallbackAjv = new AjvConstructor({ ...ajvOptions, discriminator: false }); |
There was a problem hiding this comment.
Looks good, the const/enum + required constraints come from Ajv, not the spec (docs), so the fallback is ok


What/Why/How?
Fix for handling
oneOfschemas with adiscriminatorinsidedriftcommand. Extended list of well known headers from 3rd party providers.Reference
Testing
Screenshots (optional)
Check yourself
Security
Note
Low Risk
Scoped to drift validation and header filtering; fallback preserves prior validation when discriminators cannot compile.
Overview
Fixes false positives in drift
schema-consistencywhen traffic matches aoneOf+discriminatorpayload but Ajv previously validated every branch.SchemaValidatornow compiles with Ajvdiscriminator: true, so only the branch for the discriminator value is checked. If compilation fails (loosely defined discriminators), a second Ajv instance withdiscriminator: falsecompiles the schema so behavior stays like before.Undocumented-header handling in
http.tsadds built-in ignores forx-amzn-*,x-github-*, andx-hub-signature/x-hub-signature-256(replacing a lonex-amzn-trace-idexact entry with the prefix). Tests cover both areas.Reviewed by Cursor Bugbot for commit 10645f2. Bugbot is set up for automated code reviews on this repo. Configure here.