Skip to content

fix: proto could be set during parse - #328320

Open
rzhao271 wants to merge 3 commits into
mainfrom
rzhao271/hurt-canid
Open

fix: proto could be set during parse#328320
rzhao271 wants to merge 3 commits into
mainfrom
rzhao271/hurt-canid

Conversation

@rzhao271

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings July 30, 2026 23:33
@rzhao271 rzhao271 added this to the 1.132.0 milestone Jul 30, 2026
@rzhao271
rzhao271 requested a review from aeschli July 30, 2026 23:33
@rzhao271 rzhao271 self-assigned this Jul 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Hardens the shared JSON parser against __proto__ prototype mutation.

Changes:

  • Skips __proto__ during object construction.
  • Adds a regression test verifying the prototype remains unchanged.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/vs/base/common/json.ts Guards object property assignment.
src/vs/base/test/common/json.test.ts Tests __proto__ handling.

Comment thread src/vs/base/common/json.ts Outdated
Comment thread src/vs/base/common/json.ts Outdated
Comment on lines 854 to 855
} else if (currentProperty !== null && currentProperty !== '__proto__') {
currentParent[currentProperty] = value;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Done. The safe write is now isolated in an exported setObjectProperty helper in json.ts (using Object.defineProperty for __proto__, a plain assignment otherwise). Both json.parse and ConfigurationModelParser.doParseContent (lines 368-373) now call that helper, so the configuration-parser path no longer allows prototype pollution. A regression test was added to configurationModels.test.ts asserting that {"__proto__":{"editor.fontSize":100}} does not pollute Object.prototype and that the own property is still retrievable.

Co-authored-by: rzhao271 <7199958+rzhao271@users.noreply.github.com>
…Parser

Co-authored-by: rzhao271 <7199958+rzhao271@users.noreply.github.com>
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.

3 participants