-
Notifications
You must be signed in to change notification settings - Fork 84
fix(shared): protect against infinite recursion if a pattern references a JSON schema #2012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(shared): protect against infinite recursion if a pattern references a JSON schema #2012
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an infinite recursion issue in schema validation that occurs when patterns reference JSON Schema drafts other than CALM schema or JSON Schema Draft 2020-12. The fix adds an early validation check to reject attempts to load standard JSON Schemas, preventing AJV from entering an infinite loop.
Key changes:
- Added protection against loading standard JSON Schemas by checking for json-schema.org URLs
- Added unit and E2E tests to verify the fix works correctly
- Code formatting improvements in test files (whitespace cleanup)
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| shared/src/schema-directory.ts | Added check to reject json-schema.org URL references before attempting to load them |
| shared/src/schema-directory.spec.ts | Added unit test for JSON Schema rejection and cleaned up whitespace formatting |
| shared/src/commands/validate/validate.e2e.spec.ts | Added E2E tests for both invalid and valid JSON Schema references |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
shared/src/schema-directory.ts
Outdated
| public async getSchema(schemaId: string): Promise<object> { | ||
| if (!this.schemas.has(schemaId)) { | ||
| try { | ||
| if (schemaId.startsWith('https://json-schema.org') || schemaId.startsWith('http://json-schema.org')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use regex matching here, something like http(s?)://..., but this is not a big deal.
Description
@ConfuddledPenguin reported an issue during their Advent of CALM, where Copilot generated incorrect schema in a pattern, causing
calm validateto fail after many seconds with an out of memory exception.Here is a minimal pattern that would fail:
calm validate -p <filename>.I traced this to an infinite loop in schema resolution by AJV. If the CALM schema, or JSON Schema Draft 2020-12 is referenced, all is well, but any other JSON Schema Draft leads to an infinite loop.
This PR causes validation to fail quickly and with a meaningful error if AJV seeks to load a JSON Schema:
Type of Change
Affected Components
cli/)shared/)calm-widgets/)calm-hub/)calm-hub-ui/)docs/)calm-plugins/vscode/)Commit Message Format ✅
Testing
Checklist