fix(data-inspector): block prototype-chain writes (#324)

Author: antfubot

plans/README.md

@@ -10,7 +10,7 @@ Generated by the improve skill on 2026-09-01 at commit `2d978f84`. Execute in th
 | 002 | Require authentication on route-based MCP | P1 | M | 001 | TODO |
 | 003 | Enforce shared-state exposure policy on direct MCP reads | P1 | S | 002 | TODO |
 | 004 | Contain remote asset materialization | P1 | S | - | TODO |
-| 005 | Block Data Inspector prototype-chain writes | P1 | S | - | TODO |
+| 005 | Block Data Inspector prototype-chain writes | P1 | S | - | DONE |
 | 006 | Validate request-derived authentication-link origins | P1 | M | - | TODO |
 | 007 | Reject pre-existing symlink escapes from filesystem roots | P2 | M | - | TODO |
 

plugins/data-inspector/src/engine/normalize.ts

@@ -103,7 +103,11 @@ export function navigate(value: unknown, path: NodePath, options: Pick<Normalize
       return undefined
     switch (kind) {
       case 'k':
-        cur = cur instanceof Map ? cur.get(at) : (cur as Record<string, unknown>)[at]
+        // Own properties only — mirrors the walker, which never descends into
+        // inherited properties, and keeps live re-navigation off the prototype chain.
+        cur = cur instanceof Map
+          ? cur.get(at)
+          : Object.hasOwn(cur, at) ? (cur as Record<string, unknown>)[at] : undefined
         break
       case 'i': {
         const arr = cur as unknown[]

plugins/data-inspector/src/engine/write.ts

@@ -22,6 +22,29 @@ class WriteError extends Error {
   }
 }
 
+/**
+ * Property names that reach or replace a shared prototype through ordinary
+ * property access (`__proto__`, `constructor.prototype`, …). Plain-object
+ * set/add/rename destinations reject these; Map keys are data, not property
+ * names, and never go through this check.
+ */
+const UNSAFE_OBJECT_KEYS = new Set(['__proto__', 'prototype', 'constructor'])
+
+/** Reject a plain-object property name that could reach a shared prototype. */
+function assertSafeObjectKey(key: string): void {
+  if (UNSAFE_OBJECT_KEYS.has(key))
+    throw new WriteError('InvalidKey', `"${key}" is a prototype-sensitive property name`)
+}
+
+/**
+ * Create an own data property with a plain descriptor, bypassing any setter
+ * inherited from the prototype chain. Used for every write that introduces a
+ * property name the target doesn't already own (`add`, `rename`'s new key).
+ */
+function defineOwnDataProperty(target: object, key: string, value: unknown): void {
+  Object.defineProperty(target, key, { configurable: true, enumerable: true, writable: true, value })
+}
+
 /** Decode a discriminated wire value into the raw JS value to write. */
 function decode(value: WriteValue): unknown {
   return value.kind === 'undefined' ? undefined : value.value
@@ -86,9 +109,15 @@ function setAt(parent: object, seg: PathSegment, value: unknown, opts: WriteAppl
       }
       assertMutableObject(parent)
       const key = at as string
-      const desc = Object.getOwnPropertyDescriptor(parent, key)
-      if (desc && !desc.writable && !desc.set)
+      assertSafeObjectKey(key)
+      if (!Object.hasOwn(parent, key))
+        throw new WriteError('PathNotFound', `property "${key}" does not exist`)
+      const desc = Object.getOwnPropertyDescriptor(parent, key)!
+      if (!desc.writable && !desc.set)
         throw new WriteError('ReadonlyProperty', `property "${key}" has no setter`)
+      // The property is verified own, so bracket assignment can only run
+      // this object's own setter (or write its own data slot) — never one
+      // inherited from a shared prototype.
       const record = parent as Record<string, unknown>
       record[key] = value
       return
@@ -195,8 +224,11 @@ function addTo(container: object, key: WriteValue | undefined, value: unknown, o
   const propKey = decodeKey(key, 'add')
   if (typeof propKey !== 'string')
     throw new WriteError('InvalidKey', 'an object property key must be a string')
-  const record = container as Record<string, unknown>
-  record[propKey] = value
+  assertSafeObjectKey(propKey)
+  // A fresh own data property, never a bracket assignment: the key is new to
+  // this object, so assignment would otherwise walk the prototype chain and
+  // could run an inherited setter.
+  defineOwnDataProperty(container, propKey, value)
 }
 
 function renameAt(parent: object, seg: PathSegment, newKey: unknown): void {
@@ -218,11 +250,14 @@ function renameAt(parent: object, seg: PathSegment, newKey: unknown): void {
       throw new WriteError('PathNotFound', `property "${key}" does not exist`)
     if (typeof newKey !== 'string')
       throw new WriteError('InvalidKey', 'an object property key must be a string')
+    assertSafeObjectKey(newKey)
     if (newKey === key)
       return
     const value = (parent as Record<string, unknown>)[key]
     delete (parent as Record<string, unknown>)[key]
-    ;(parent as Record<string, unknown>)[newKey] = value
+    // The new key is fresh to this object; define it directly rather than
+    // assigning through the prototype chain.
+    defineOwnDataProperty(parent, newKey, value)
     return
   }
   if (kind === 'mk' || kind === 'mv') {

plugins/data-inspector/test/write.test.ts

@@ -181,6 +181,108 @@ describe('applyWrite — rename', () => {
   })
 })
 
+describe('applyWrite — prototype-chain safety', () => {
+  const unsafeKeys = ['__proto__', 'prototype', 'constructor'] as const
+
+  it('rejects set of prototype-sensitive destination keys', () => {
+    for (const key of unsafeKeys) {
+      const root = {}
+      const out = applyWrite(root, { op: 'set', path: [['k', key]], value: json({ polluted: true }) })
+      expect(out).toMatchObject({ ok: false, error: { name: 'InvalidKey' } })
+    }
+    expect(Object.prototype).not.toHaveProperty('polluted')
+  })
+
+  it('rejects add of prototype-sensitive destination keys', () => {
+    for (const key of unsafeKeys) {
+      const out = applyWrite({}, { op: 'add', path: [], key: json(key), value: json({ polluted: true }) })
+      expect(out).toMatchObject({ ok: false, error: { name: 'InvalidKey' } })
+    }
+    expect(Object.prototype).not.toHaveProperty('polluted')
+  })
+
+  it('rejects rename onto a prototype-sensitive destination key', () => {
+    for (const key of unsafeKeys) {
+      const root = { a: 1 }
+      const out = applyWrite(root, { op: 'rename', path: [['k', 'a']], key: json(key) })
+      expect(out).toMatchObject({ ok: false, error: { name: 'InvalidKey' } })
+      expect(root).toEqual({ a: 1 })
+    }
+    expect(Object.prototype).not.toHaveProperty('polluted')
+  })
+
+  it('treats an inherited property as absent, reporting nested set as PathNotFound', () => {
+    const proto = { shared: { secret: 1 } }
+    const root = Object.create(proto) as Record<string, unknown>
+    // `shared` is inherited, not an own property of `root`.
+    const out = applyWrite(root, { op: 'set', path: [['k', 'shared'], ['k', 'secret']], value: json(2) })
+    expect(out).toMatchObject({ ok: false, error: { name: 'PathNotFound' } })
+    expect(proto.shared.secret).toBe(1)
+  })
+
+  it('treats an inherited property as absent, reporting delete/rename as PathNotFound', () => {
+    const proto = { shared: 1 }
+    const root = Object.create(proto) as Record<string, unknown>
+    expect(applyWrite(root, { op: 'delete', path: [['k', 'shared']] })).toMatchObject({ ok: false, error: { name: 'PathNotFound' } })
+    expect(applyWrite(root, { op: 'rename', path: [['k', 'shared']], key: json('renamed') })).toMatchObject({ ok: false, error: { name: 'PathNotFound' } })
+    expect(proto.shared).toBe(1)
+  })
+
+  it('add creates an own data property without invoking an inherited setter', () => {
+    const proto: Record<string, unknown> = {}
+    let setterCalls = 0
+    const bumpSetterCalls = () => setterCalls++
+    Object.defineProperty(proto, 'name', { configurable: true, enumerable: true, get: () => 'proto-value', set: bumpSetterCalls })
+    try {
+      const root: Record<string, unknown> = Object.create(proto)
+      const out = applyWrite(root, { op: 'add', path: [], key: json('name'), value: json('own-value') })
+      expect(out.ok).toBe(true)
+      expect(setterCalls).toBe(0)
+      expect(Object.hasOwn(root, 'name')).toBe(true)
+      expect(root.name).toBe('own-value')
+    }
+    finally {
+      delete proto.name
+    }
+  })
+
+  it('rename creates an own data property at the destination without invoking an inherited setter', () => {
+    const proto: Record<string, unknown> = {}
+    let setterCalls = 0
+    const bumpSetterCalls = () => setterCalls++
+    Object.defineProperty(proto, 'name', { configurable: true, enumerable: true, get: () => 'proto-value', set: bumpSetterCalls })
+    try {
+      const root: Record<string, unknown> = Object.create(proto)
+      root.oldKey = 'own-value'
+      const out = applyWrite(root, { op: 'rename', path: [['k', 'oldKey']], key: json('name') })
+      expect(out.ok).toBe(true)
+      expect(setterCalls).toBe(0)
+      expect(Object.hasOwn(root, 'name')).toBe(true)
+      expect(root.name).toBe('own-value')
+    }
+    finally {
+      delete proto.name
+    }
+  })
+
+  it('lets a Map use __proto__/prototype/constructor as ordinary data keys', () => {
+    const map = new Map<unknown, unknown>()
+    for (const key of unsafeKeys) {
+      const out = applyWrite(map, { op: 'add', path: [], key: json(key), value: json(`value:${key}`) })
+      expect(out.ok).toBe(true)
+    }
+    for (const key of unsafeKeys)
+      expect(map.get(key)).toBe(`value:${key}`)
+
+    expect(applyWrite(map, { op: 'set', path: [['k', '__proto__']], value: json('updated') })).toMatchObject({ ok: true })
+    expect(map.get('__proto__')).toBe('updated')
+
+    expect(applyWrite(map, { op: 'rename', path: [['k', 'prototype']], key: json('renamed-prototype') })).toMatchObject({ ok: true })
+    expect(map.get('renamed-prototype')).toBe('value:prototype')
+    expect(map.has('prototype')).toBe(false)
+  })
+})
+
 describe('applyWrite — request typing', () => {
   it('round-trips through JSON (wire-safety of the request shape)', () => {
     const request: WriteRequest = { op: 'set', path: [['k', 'a'], ['i', 0]], value: { kind: 'undefined' } }