Settings: a "__proto__" section key silently corrupts the object (prototype mutation instead of own property)

[xu-kai-quan]

cloneJsonShaped() and mergeLayers() in packages/settings/settings/src/index.ts
build plain objects with out[key] = value while iterating untrusted keys from
Object.entries():

// cloneJsonShaped, line ~277
const out: Record<string, unknown> = {}
for (const [key, entry] of Object.entries(value)) {
  if (entry === undefined) continue
  out[key] = clone(entry, `${path}.${key}`)
}

// mergeLayers, line ~301
const merged: Record<string, unknown> = { ...under }
for (const [key, value] of Object.entries(over)) {
  merged[key] = key in merged ? mergeLayers(merged[key], value) : value
}

When key === "proto", bracket assignment is a [[Set]], not
[[DefineOwnProperty]]. It invokes Object.prototype's proto accessor
and reassigns the object's prototype instead of creating an own data
property. Confirmed with node:

const value = JSON.parse('{"__proto__": {"marker": true}, "ok": 1}')
const out = {}
for (const [k, v] of Object.entries(value)) out[k] = v
Object.keys(out)   // ['ok']  <- "__proto__" silently vanished
out.marker         // true    <- leaked onto the prototype instead

Note this only reproduces via JSON.parse (or Object.defineProperty) —
an object literal like {'proto': {...}} is special-cased by the spec
and never hits this path. JSON.parse has no such special case, so any
settings write built from parsed JSON (an HTTP request body, a config file
patch) can carry a real own "proto" key.

Impact: any settings namespace whose schema allows a dict/record-typed
field (e.g. an MCP-servers map, or anything keyed by a user-chosen name) can
have an entry named proto silently dropped by cloneJsonShaped. Worse,
mergeLayers's isPlainObject check can then see the corrupted-prototype
object as non-plain and replace the entire section wholesale, discarding
every other field the user had set on the next merge — silent data loss, no
error.

The file already carries a TODO(settings-json-properties) comment
acknowledging this gap, and the identical pattern is already fixed correctly
elsewhere in this codebase, e.g. workflow-worker-thread/src/realm.ts:

// defineProperty, never assignment: a "__proto__" key must become an OWN
// data property of the copy, not a prototype mutation.
Object.defineProperty(out, key, { value: ..., enumerable: true, writable: true, configurable: true })

Suggested fix (same pattern applied to cloneJsonShaped/mergeLayers),
plus a regression test, here:
https://github.com/xu-kai-quan/deepseek-harness/tree/fix/settings-proto-key-clone

I understand external PRs aren't being accepted right now per CONTRIBUTING.md,
so posting here per that doc's guidance instead. Happy to elaborate if useful.

[argszero]

Excellent report — I verified every claim against current master (47f9438) and traced the reachable paths end to end. Confirmed:

1. All cited claims check out

• cloneJsonShaped (settings/src/index.ts:253) assigns via out[key] = clone(...) at :280; the TODO(settings-json-properties) comment at :275-276 is exactly this gap.
• mergeLayers (:297) assigns via merged[key] = ... at :302.
• isPlainObject (:186-190) rejects any object whose prototype isn't Object.prototype/null — so a prototype-replaced object makes mergeLayers treat the whole layer as non-plain and replace it wholesale (:299 return over), the silent data-loss claim.
• The already-fixed sibling exists: workflow-worker-thread/src/realm.ts:141-143 (defineProperty, never assignment).

2. The reachable surface is wider than "dict-typed fields" — the RPC write path is exposed

Sections arrive over the JSON-RPC line transport, which decodes with plain JSON.parse (sdk/protocol/src/transport.ts:204) — and JSON.parse preserves __proto__ as a real own key (the reporter's demo). The apiproxy settingsWrite (api-proxy.ts:2001-2013) passes the decoded section object through untouched to the seam, so any exposed namespace's update/replace/mutate is reachable, not just file documents.

Also note both functions are independently reachable:

• update/mutate → cloneJsonShaped(payload) (:607) → mergeLayers(current, snapshot) (:629)
• replace → isPlainObject(section) (:690) → mergeLayers(base, section) (:705) — the raw section goes straight into mergeLayers with no cloneJsonShaped pass

So fixing only cloneJsonShaped leaves replace polluted; both loops must be fixed (the report already says both — confirming that's not optional).

3. One surface I could NOT verify locally: the YAML document path

settings-file uses eemeli's yaml package parseDocument (settings-file/src/index.ts:15) and supports .json documents too (:36-37). Whether the parser preserves __proto__ as an own key or sanitizes it depends on the installed parser version — I couldn't test it in this source-only checkout (package not installed). Worth one empirical check on a machine with the dependency, plus a regression test covering both a .yaml and a .json document. The JSON-RPC path above is definitively exposed regardless of the parser's behavior.

4. Fix shape

The realm.ts pattern is the right minimal fix — Object.defineProperty(out, key, { value, enumerable: true, writable: true, configurable: true }) in both cloneJsonShaped and mergeLayers. A stronger variant is building the containers with Object.create(null): it defeats __proto__ (and any future prototype-accessor key) by construction, and — importantly for this codebase — isPlainObject still accepts null-prototype objects (:190), so mergeLayers/resolve semantics are unchanged. Either way, with the fix the previously-silently-dropped __proto__-named entry (e.g. a user's MCP server literally named __proto__) becomes a real persisted own property, which is strictly better than today's silent loss.

Optional defense-in-depth at the seam: reject __proto__/prototype/constructor keys in settingsWrite with an explicit error, if the maintainers prefer not to persist such keys at all — but that's a policy choice; the construction fix alone already removes the corruption.

5. Family note

This is the settings-domain instance of the same "silent lossy transformation" class we've been tracking in the session/serializer domain (#1538/#1584 ignorable, #1619 agentOptions, #1606 settings exposure): data silently dropped/corrupted with no error, then acting on the corrupted state. The TODO(settings-json-properties) marker means the maintainers already knew; your fork fix plus a regression test is exactly the missing piece. When PRs reopen, this is a small, well-scoped first candidate. Thanks for the precise write-up.

[xu-kai-quan]

Found the same root cause in a second, unrelated location:

packages/guard/repeat-tool-reminder/src/index.ts, sortJsonValue()
(used by canonicalize() to build the repeat-tool-call detector's chain key):

// line ~89-100
function sortJsonValue(value: unknown): unknown {
  if (Array.isArray(value)) return value.map(sortJsonValue)
  if (value !== null && typeof value === 'object') {
    const record = value as Record<string, unknown>
    const sorted: Record<string, unknown> = {}
    for (const key of Object.keys(record).sort()) {
      sorted[key] = sortJsonValue(record[key])   // same [[Set]] vs [[DefineOwnProperty]] bug
    }
    return sorted
  }
  return value
}

Same mechanism as above: sorted[key] = ... for key === "proto" mutates sorted's prototype instead of creating an own property, so that key silently drops out of the object this function builds.

Why it's reachable here too: tool-call arguments reach this guard as the agent loop's JSON.parse output of the model's raw tool-call arguments (per the function's own doc comment) — exactly the vector that produces a real own "proto" key, same as the settings write path above.

Impact: two consecutive tool calls whose arguments differ only under a "proto" key canonicalize to the same string (since that key is dropped), so the guard treats them as identical repeats. This produces a false-positive "you are repeating this call" reminder injected into the model's context even though the calls' actual arguments differ.

Fix: same Object.defineProperty construction, verified with a regression test (3 calls whose arguments differ only under proto; before the fix they wrongly trip the threshold-3 reminder, after the fix they correctly don't since the chain resets each time): https://github.com/xu-kai-quan/deepseek-harness/tree/fix/repeat-tool-reminder-proto-key

Flagging as a reply here rather than a new discussion since it's the same underlying issue, just a second call site the same audit turned up.

[xu-kai-quan]

Found a third instance of the same root cause, this time three call sites in
one function: packages/settings/settings/src/redact.ts, walk()
(used by redactSecrets() to strip role('secret') fields before a settings
value crosses the wire to a configuration UI):

case 'object': {
  ...
  for (const [key, entry] of Object.entries(source)) {
    if (key in properties) continue
    rebuilt[key] = entry                              // site 1
  }
  for (const [key, child] of Object.entries(properties)) {
    const stripped = walk(child, source?.[key], [...path, key], secrets)
    if (stripped !== undefined) rebuilt[key] = stripped // site 2
  }
  ...
}
case 'dict': {
  ...
  for (const [key, entry] of Object.entries(value)) {
    const stripped = walk(node.inner, entry, [...path, key], secrets)
    if (stripped !== undefined) rebuilt[key] = stripped // site 3
  }
  return rebuilt
}

Same mechanism as the two above: rebuilt[key] = ... for key === "__proto__" mutates rebuilt's prototype instead of creating an own
property, so that key silently vanishes from the redacted value.

Concrete failure scenario: a dict-shaped settings field (per-server
config, custom env vars, custom headers — anything keyed by a user-chosen
name) with an entry literally named __proto__ disappears from the redacted
view a configuration UI reads via describe({ redactSecrets: true }), even
though the entry is present in storage. A UI-driven replace() built from
that (incomplete) redacted view would then delete the entry from storage on
the next save.

Fix: same Object.defineProperty construction (via a small setOwn()
helper this time, since there are three call sites), with a regression test
covering both the object passthrough branch and the dict branch:
https://github.com/xu-kai-quan/deepseek-harness/tree/fix/settings-redact-proto-key

This looks to be the last instance of this pattern I could find in the
codebase after a broader pass — happy to keep looking if useful, but wanted
to close out this specific root cause first.

[zoahdev]

Shipped the settings-layer fix you described.

• Branch: zoahdev/deepseek-harness@fix/settings-proto-key-own-property (cherry-pick-ready against 47f943859b).
• packages/settings/settings/src/index.ts: cloneJsonShaped and mergeLayers now assign keys via Object.defineProperty (enumerable/writable/configurable own data), so __proto__ no longer mutates the prototype. mergeLayers is exported for the regression.
• settings.spec 90/90 green with a __proto__ own-key regression.

One honest scope note: the settings layer is fixed, but the vendored schemastery dict parser (vendor/schemastery — for (const key in value) { result[key] = ... }) has the same [[Set]] pattern, so a __proto__ key that reaches a z.dict schema is still dropped at validation time. That's a separate vendored-library fix; I kept this patch to the settings layer you pointed at, but happy to prepare the schemastery dict fix too if maintainers want the full end-to-end guarantee.