Skip to content

fix(compiler,openapi): keep object value members named __proto__ - #11744

Open
om singhal (Om-singhaI) wants to merge 1 commit into
microsoft:mainfrom
Om-singhaI:fix/marshal-proto-member
Open

fix(compiler,openapi): keep object value members named __proto__#11744
om singhal (Om-singhaI) wants to merge 1 commit into
microsoft:mainfrom
Om-singhaI:fix/marshal-proto-member

Conversation

@Om-singhaI

Copy link
Copy Markdown

fix(compiler,openapi): keep object value members named __proto__

Fixes #11743

What is broken and why

When the compiler marshals an object value for a decorator, objectValueToJs in packages/compiler/src/core/js-marshaller.ts (lines 88 to 94 on main) added each member with a plain assignment:

const result: Record<string, unknown> = {};
for (const [key, value] of type.properties) {
  result[key] = marshalTypeForJs(value.value, undefined);
}
return result;

For a member named __proto__ that assignment does not create a property. It invokes the Object.prototype.__proto__ setter instead. Two things follow:

  • The member never reaches the decorator, and no diagnostic reports it.
  • When the member holds an object or an array, that value becomes the prototype of the marshalled object, so a decorator that reads any name it did not declare can read the author's data (value.polluted === true with polluted not an own property).

The checker stores object value members in a Map (checkObjectLiteralProperties in checker.ts), and unmarshalJsToValue in the same marshaller file already builds a Map from entries, so this loop was the only place in the compiler where the member was lost.

The issue reproduces through @extension from @typespec/openapi, and that path has a second copy of the same pattern. convertRemainingValuesToExtensions in packages/openapi/src/decorators.ts (lines 101 to 108 on main) rebuilds the object it receives from the compiler with Object.entries followed by result[key] = .... Once the compiler hands it an object with an own __proto__ property, Object.entries lists that key and the assignment hits the setter again, so the extension stored by the decorator lost the member and had its prototype replaced in exactly the same way. Fixing only the compiler would not have changed the output of the issue's playground.

The fix

Both sites now build the object with Object.fromEntries, which defines every member as an own enumerable data property (it uses CreateDataProperty) and never consults the setter.

packages/compiler/src/core/js-marshaller.ts:

function objectValueToJs(type: ObjectValue): Record<string, unknown> {
  // Object.fromEntries defines each member as an own property, so a member named `__proto__`
  // is kept instead of going through the Object.prototype setter and being dropped.
  return Object.fromEntries(
    [...type.properties].map(([key, value]) => [key, marshalTypeForJs(value.value, undefined)]),
  );
}

packages/openapi/src/decorators.ts, in the plain object branch of convertRemainingValuesToExtensions:

return Object.fromEntries(
  Object.entries(value)
    .filter(([, val]) => val !== undefined)
    .map(([key, val]) => [key, convertRemainingValuesToExtensions(program, val)]),
);

The openapi copy keeps its existing behaviour of skipping undefined values, and the nested case was already handled by recursion. The result still inherits from Object.prototype in both places. Object.create(null) was deliberately not used because it would change the prototype of every object handed to every decorator, which is a larger behaviour change than the bug warrants. Object.fromEntries was preferred over Object.defineProperty because it reads like the surrounding code (unmarshalJsToValue already uses it) and needs no descriptor flags.

Change entries: one fix entry for @typespec/compiler and a separate fix entry for @typespec/openapi, as the repository guidelines ask for one entry per package. chronus verify reports both packages documented.

Testing

Reproduction before the fix

End to end through @extension, compiled with the openapi test host (Tester.compile) and read back with getExtensions, using the three cases from the issue. With the compiler fix applied but @typespec/openapi untouched, the stored extensions measured:

x-a-object:  ownKeys=["ok"]         hasOwn(__proto__)=false  protoIsObjectPrototype=false  prototypeOwnNames=["polluted"]  value.polluted=true
x-b-string:  ownKeys=["ok"]         hasOwn(__proto__)=false  protoIsObjectPrototype=true   value.polluted=undefined
x-c-control: ownKeys=["normal","ok"] hasOwn(__proto__)=false  protoIsObjectPrototype=true   value.polluted=undefined

So the compiler change alone did not reach the reporter's output. On pristine main the decorator argument itself measured the same way (ownKeys=["ok"], prototypeOwnNames=["polluted"], arg.polluted === true for the object case; ownKeys=["ok"] for the string case).

With both fixes:

x-a-object:  ownKeys=["__proto__","ok"] hasOwn(__proto__)=true  protoIsObjectPrototype=true  value.polluted=undefined
x-b-string:  ownKeys=["__proto__","ok"] hasOwn(__proto__)=true  protoIsObjectPrototype=true  value.polluted=undefined
x-c-control: ownKeys=["normal","ok"]    hasOwn(__proto__)=false protoIsObjectPrototype=true  value.polluted=undefined

New tests

packages/compiler/test/checker/decorators.test.ts (under value marshalling > passing an object value):

  • keeps a member named __proto__ holding a string as an own property
  • keeps a member named __proto__ holding an object as an own property

packages/openapi/test/decorators.test.ts (under @extension):

  • keeps a member named __proto__ holding a string as an own property
  • keeps a member named __proto__ holding an object as an own property

Each test asserts Object.prototype.hasOwnProperty.call(value, "__proto__"), Object.keys(value) equals ["__proto__", "ok"], the member's value, that Object.getPrototypeOf(value) === Object.prototype, and for the object case that value.polluted is undefined.

Commands and counts

All runs use npx vitest run (vitest 4.1.10) from the package directory. The openapi tests execute the compiler and the openapi decorator from their dist builds, so tsc -p tsconfig.build.json was run in both packages before each openapi run.

Run Command Result
Compiler, pristine main, full suite packages/compiler: npx vitest run 160 files passed, 4107 passed, 6 skipped (4113)
Compiler, new tests against main's js-marshaller.ts (file restored from a791115b) npx vitest run test/checker/decorators.test.ts 2 failed, 75 passed (77); the two failures are the new __proto__ tests, failing at the hasOwnProperty assertion
Compiler, with fix npx vitest run test/checker/decorators.test.ts 77 passed (77)
Compiler, with fix, full suite npx vitest run 160 files passed, 4109 passed, 6 skipped (4115)
OpenAPI, before openapi fix (compiler fix present), full suite packages/openapi: npx vitest run 3 files passed, 64 passed (64)
OpenAPI, new tests with src/decorators.ts stashed and dist rebuilt npx vitest run test/decorators.test.ts 2 failed, 54 passed (56); the two failures are the new __proto__ tests, failing at the hasOwnProperty assertion
OpenAPI, with fix npx vitest run test/decorators.test.ts 56 passed (56)
OpenAPI, with fix, full suite npx vitest run 3 files passed, 66 passed (66)

Lint and formatting

  • prettier --check on the four changed source files and the two change entries: clean.
  • oxlint --deny-warnings on the four changed source files: no findings.
  • cspell on all six changed files: 0 issues.
  • chronus verify: all changed packages documented (@typespec/compiler, @typespec/openapi).
  • tsc -p tsconfig.build.json in packages/compiler and packages/openapi: clean.
  • oxlint . --deny-warnings (the pnpm lint script) in packages/openapi: exit 0, no findings. The same oxlint invocation was checked to report a debugger; statement (exit 1), so it is actually linting.
  • tsp compile . --warn-as-error --import @typespec/library-linter --no-emit (the lint-typespec-library build step) in packages/openapi: compilation completed successfully.

Notes for reviewers

  • The fix is bounded to what the compiler and @extension hand over. A decorator that copies its argument with Object.assign(target, arg) or a for ... in loop with assignment will trigger the setter again on its own copy, because those use [[Set]]. Object spread and Object.fromEntries do not, so copies made that way keep the member.
  • Three other places in the compiler build plain objects from user chosen keys with assignment and were left alone because they are not on this issue's path: serializeObjectValueAsJson in packages/compiler/src/lib/examples.ts (line 161, model property or encoded names), the Model case of typespecTypeToJsonInternal in packages/compiler/src/core/decorator-utils.ts (line 262, model property names), and the argument record in packages/compiler/src/core/auto-decorator.ts (line 43, decorator parameter names). A model property or parameter literally named __proto__ would hit the same setter there. Happy to handle them in this PR or a follow up if maintainers prefer.
  • Type aware oxlint (--type-aware) could only be run with packages/compiler/dist moved aside; with dist present it errors on a tsconfig input overlap for dist/src/server/tmlanguage.d.ts that is unrelated to this change.
  • Only the compiler, openapi, and their workspace dependency closures (http, rest, streams, library-linter, tspd and the internal build utilities) were installed and built locally; the rest of the monorepo was not built or tested, and pnpm lint and pnpm format:check were run per file rather than at the monorepo root.

When the compiler marshals an object value for a decorator, objectValueToJs
in the JS marshaller added each member with a plain assignment. For a member
named __proto__ that assignment does not create a property; it invokes the
Object.prototype.__proto__ setter. The member silently disappeared, and when
its value was an object or an array that value became the prototype of the
marshalled object, so the decorator could read names the .tsp author never
declared as members. Nothing reported a diagnostic.

The checker already stores object value members in a Map, and
unmarshalJsToValue in the same file builds its Map from entries, so the
marshalling loop was the only place in the compiler where the member was
lost. objectValueToJs now builds the result with Object.fromEntries, which
defines every member as an own data property and never consults the setter.

The same pattern existed one step further along the issue's own path.
convertRemainingValuesToExtensions in @typespec/openapi copies the object it
receives from the compiler with Object.entries followed by assignment, so
after the compiler fix it would have dropped the member again and polluted
the prototype of the stored extension in the same way. That copy now goes
through Object.fromEntries as well, keeping its existing behaviour of
skipping undefined values.

Regression tests cover both packages: the compiler tests check the object a
decorator receives, and the openapi tests check what @extension stores, for
a member named __proto__ holding a string and holding an object. Each test
asserts the member is an own enumerable property and that the prototype of
the result is still Object.prototype.

Fixes microsoft#11743
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

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

This PR fixes a JavaScript marshalling edge case where object-value members named __proto__ were being dropped (and could mutate the marshalled object’s prototype) when passed from the TypeSpec compiler to decorators, and when @typespec/openapi’s @extension decorator re-materialized those objects.

Changes:

  • Update the compiler’s object-value marshalling to use Object.fromEntries(...) so __proto__ becomes an own data property instead of triggering the prototype setter.
  • Update OpenAPI’s convertRemainingValuesToExtensions object branch to use Object.fromEntries(...) (preserving the existing undefined-skipping behavior) to avoid reintroducing the same issue.
  • Add regression tests in both @typespec/compiler and @typespec/openapi, plus separate Chronus fix entries for each package.

Reviewed changes

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

Show a summary per file
File Description
packages/compiler/src/core/js-marshaller.ts Switch object-value marshalling to Object.fromEntries to preserve __proto__ safely.
packages/openapi/src/decorators.ts Switch extension object conversion to Object.fromEntries to avoid __proto__ setter behavior.
packages/compiler/test/checker/decorators.test.ts Add compiler regression tests covering __proto__ as string/object values.
packages/openapi/test/decorators.test.ts Add OpenAPI @extension regression tests covering __proto__ as string/object values.
.chronus/changes/fix-marshal-proto-member-2026-8-21.md Changelog entry for the compiler fix.
.chronus/changes/fix-extension-proto-member-2026-8-22.md Changelog entry for the OpenAPI fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler:core Issues for @typespec/compiler lib:openapi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: A member named __proto__ is silently dropped from a decorator's object value

2 participants