Skip to content

fix(json-schema): remove settings/additionalProperties#6420

Merged
jdx merged 2 commits intojdx:mainfrom
tpansino:fix-json-schema-additionalProperties
Sep 25, 2025
Merged

fix(json-schema): remove settings/additionalProperties#6420
jdx merged 2 commits intojdx:mainfrom
tpansino:fix-json-schema-additionalProperties

Conversation

@tpansino
Copy link
Contributor

Resolves #6353

@jdx
Copy link
Owner

jdx commented Sep 25, 2025

that's not the right way to fix this

@jdx jdx closed this Sep 25, 2025
@jdx jdx reopened this Sep 25, 2025
@jdx
Copy link
Owner

jdx commented Sep 25, 2025

sorry read the original thread, it seems more complicated than I thought

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Schema Validation Error: Unintended Permissiveness

Removing additionalProperties: false from the settings object makes the schema overly permissive, allowing undefined or misspelled properties in settings configurations to pass validation. This could mask configuration errors, and the maintainer's comment suggests this isn't the intended fix.

schema/mise.json#L1730-L1731

mise/schema/mise.json

Lines 1730 to 1731 in d1a58bb

"settings": {
"$ref": "#/$defs/settings",

Fix in Cursor Fix in Web


@jdx
Copy link
Owner

jdx commented Sep 25, 2025

according to gpt-5 (which I think is right):

schema/mise.json: Dropping the additionalProperties: false that sits beside the $ref does not make validation stricter; it just removes a keyword that draft‑2019‑09 ignores in that position, so extra keys are still accepted. To fail on unknown settings you need the constraint inside the target schema itself (e.g. add additionalProperties: false to #/$defs/settings, or wrap the $ref in an allOf that adds that keyword). GitHub PR #6420
Next step suggestion
Move additionalProperties: false into #/$defs/settings (or equivalent) so the validator enforces it, then re-run your schema tests.

@tpansino
Copy link
Contributor Author

tpansino commented Sep 25, 2025

Is this what you mean? I originally looked at the render.ts code, thinking I needed to fix it there, but then I saw it reads in the existing mise.json file and only updates specific sections that may change frequently. Is that right?

@jdx
Copy link
Owner

jdx commented Sep 25, 2025

Yeah this looks correct to me

@jdx jdx merged commit e01bb63 into jdx:main Sep 25, 2025
17 checks passed
@jdx jdx mentioned this pull request Sep 25, 2025
@tpansino tpansino deleted the fix-json-schema-additionalProperties branch September 26, 2025 20:40
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