Keep a __proto__ key in attachment JSON out of Hash's prototype chain

[rosa]

parseTrixDataAttribute() hands raw JSON.parse() output to Hash, and JSON.parse materializes "__proto__" as an ordinary own key. Object.keys() therefore yields it, and copy()'s plain assignment invoked the inherited setter, replacing the new object's prototype with the parsed value rather than storing it as data.

The consequence is an asymmetry rather than global pollution. Object.prototype is untouched, but Hash.get() and Hash.has() resolve the smuggled names through the prototype chain, while Object.keys(), toObject() and toJSON() cannot see them:

after JSON.parse:
  'content' in parsed    : false      <- an embedder gating on this sees nothing
after Hash's copy():
  values['content']      : <smuggled> <- Hash.get() reads this
  'content' in values    : true       <- Hash.has() reads this
  copy(values) ownKeys   : [contentType, filename]   <- toObject() sees this

So an embedder that inspects attachment attributes with own-key operations and gates on the result will pass a payload that Trix then reads back in full. That is not hypothetical: it was reported against a real application, where it skipped an attachment-content scrubber and an image proxy, causing rendering to fetch sender-chosen URLs directly.

The change

Define the property instead of assigning it, so "__proto__" is stored as an ordinary own key. What Trix reads and what callers inspect now agree.

Why not Object.create(null)

That was the other candidate and it fixes the same bug, but copy() also produces the object returned by toObject() and toJSON(). A null prototype there changes the surface embedders receive, so attributes.hasOwnProperty(...) would start throwing. Defining the property leaves that intact:

	get/has smuggled	toObject() own keys	hasOwnProperty usable
before	yes	hidden	yes
Object.create(null)	no	visible	no
this change	no	visible	yes

Tests

src/test/unit/hash_test.js is new. Two of its cases fail without the source change and pass with it, verified by reverting the change and re-running:

FAIL: Hash > a __proto__ key does not replace the prototype of the internal store
FAIL: Hash > what Hash reads and what callers inspect agree for a __proto__ key
506 tests: 475 passed, 2 failed, 29 skipped.

With the change, the full suite is green: 506 tests: 477 passed, 0 failed, 29 skipped.

The vendored Action Text bundle is rebuilt, matching how #1293 and the data-trix-serialized-attributes fix shipped.

Credit

Reported by @bekkaze through a GitHub Security Advisory in February, where we said we would take exactly this hardening and then did not write it. The impact was later demonstrated by another researcher through our HackerOne program. Thanks to both.

Related: #1351 covers a separate prototype-chain issue in the sanitizer and parser option handling.

[Copilot]

🟡 Changes recommended

Hash mutation paths still use plain assignment and can reproduce the same prototype substitution.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Hardens Hash against __proto__ keys parsed from attachment JSON.

Changes:

• Uses property definition instead of assignment during copying.
• Adds regression tests and registers the new test suite.
• Rebuilds the vendored Action Text bundle.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File summaries

File	Description
src/trix/core/collections/hash.js	Safely copies __proto__ as an own property.
src/test/unit/hash_test.js	Adds prototype-substitution regression tests.
src/test/unit.js	Registers the Hash tests.
action_text-trix/app/assets/javascripts/trix.js	Updates the vendored implementation.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Balanced

---

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