config: a proto key in a tracked config.json silently drops the git rails #114

New issue

Closed

opened 2026-08-13 15:43:08 +00:00 by john · 1 comment

john commented

2026-08-13 15:43:08 +00:00

Owner

A tracked .planning/config.json can turn off the git rails, and every surface that would show you is blind to it.

lib/config-merge.mjs:68-70:

const merged = { ...base };
for (const [k, v] of Object.entries(over)) merged[k] = deepMerge(base[k], v);

JSON.parse makes __proto__ an OWN property, Object.entries yields it, and merged[k] = ... fires the __proto__ setter. The merged config gets reparented. readLayer does no sanitizing.

Proven, end to end

.planning/config.json = {"__proto__":{"git":{"on_protected":"allow","protected_branches":[]}}}, branch main, git commit:

surface	answer
`git-guard.mjs`	prints nothing. Commit allowed.
same input, ordinary config	`permissionDecision: "ask"`
`config.mjs validate`	`{"ok":true,"checked":0,"errors":[]}`

The split is the point: flatten() in config.mjs:88-96 enumerates OWN keys, so inspection sees nothing, while commitDecision's config.git walks the prototype and obeys it. Object.keys(merged) is empty on a config that is actively deciding.

The condition that makes it worse, not better

It only bites when no LOWER layer defines that top-level key. mergeLayers bases on globalValue || {}, so a machine with a ~/.claude/cadence/config.json that defines git shadows the injection with its own key, and the attack silently does nothing.

That means a naive test on a configured machine looks clean, and the exposure is worst on a FRESH INSTALL with no global config. New users, the ones with no reason to suspect the file, are the ones it works on. .planning/ is tracked, so it travels with the clone.

Scope, honestly

It does NOT pollute Object.prototype globally, and it does NOT reach raw JSON.parse readers - git-publish.mjs:69 repoAutoClose returned false under the same input, so auto_close cannot be switched on this way. Reach is every mergeLayers consumer: git-guard.mjs:93, git-publish.mjs:84, git-branch.mjs, land-cleanup.mjs, route.mjs.

Fix

Skip __proto__, constructor and prototype in deepMerge, or build merged with Object.create(null) and defineProperty. Add a test that a hostile layer cannot move on_protected or empty protected_branches, and pin the inspection/enforcement agreement so validate can never again report clean on a config that decides.

A tracked `.planning/config.json` can turn off the git rails, and every surface that would show you is blind to it. `lib/config-merge.mjs:68-70`: ```js const merged = { ...base }; for (const [k, v] of Object.entries(over)) merged[k] = deepMerge(base[k], v); ``` `JSON.parse` makes `__proto__` an OWN property, `Object.entries` yields it, and `merged[k] = ...` fires the `__proto__` setter. The merged config gets reparented. `readLayer` does no sanitizing. ## Proven, end to end `.planning/config.json` = `{"__proto__":{"git":{"on_protected":"allow","protected_branches":[]}}}`, branch `main`, `git commit`: | surface | answer | |---|---| | `git-guard.mjs` | prints nothing. Commit allowed. | | same input, ordinary config | `permissionDecision: "ask"` | | `config.mjs validate` | `{"ok":true,"checked":0,"errors":[]}` | The split is the point: `flatten()` in `config.mjs:88-96` enumerates OWN keys, so inspection sees nothing, while `commitDecision`'s `config.git` walks the prototype and obeys it. `Object.keys(merged)` is empty on a config that is actively deciding. ## The condition that makes it worse, not better It only bites when no LOWER layer defines that top-level key. `mergeLayers` bases on `globalValue || {}`, so a machine with a `~/.claude/cadence/config.json` that defines `git` shadows the injection with its own key, and the attack silently does nothing. That means a naive test on a configured machine looks clean, and the exposure is worst on a FRESH INSTALL with no global config. New users, the ones with no reason to suspect the file, are the ones it works on. `.planning/` is tracked, so it travels with the clone. ## Scope, honestly It does NOT pollute `Object.prototype` globally, and it does NOT reach raw `JSON.parse` readers - `git-publish.mjs:69 repoAutoClose` returned `false` under the same input, so `auto_close` cannot be switched on this way. Reach is every `mergeLayers` consumer: `git-guard.mjs:93`, `git-publish.mjs:84`, `git-branch.mjs`, `land-cleanup.mjs`, `route.mjs`. ## Fix Skip `__proto__`, `constructor` and `prototype` in `deepMerge`, or build `merged` with `Object.create(null)` and `defineProperty`. Add a test that a hostile layer cannot move `on_protected` or empty `protected_branches`, and pin the inspection/enforcement agreement so `validate` can never again report clean on a config that decides.

john added this to the v3.2.0 milestone

2026-08-13 15:43:08 +00:00

john added the

bug

security

labels

2026-08-13 15:43:08 +00:00

john referenced this issue

2026-08-13 15:43:25 +00:00

atomicWrite: follows symlinks through a predictable temp path, and collides under parallelism #115

john referenced this issue

2026-08-13 15:44:11 +00:00

exposure: git credentials in error detail, prod-honored test env overrides, unpinned CI installs #118

john referenced this issue

2026-08-13 15:44:27 +00:00

config: executable and key-bearing settings must never live in the tracked layer #119

john referenced this issue

2026-08-13 15:46:49 +00:00

config: executable and key-bearing settings must never live in the tracked layer #119

john closed this issue

2026-08-14 17:01:41 +00:00

john commented

2026-08-14 17:01:59 +00:00

Author

Owner

Fixed in v3.2.0 (PR #124). CFG-01, phase 1. deepMerge defines own properties instead of assigning, so proto, constructor and prototype in a repo layer store inertly rather than reparenting the merged config. The fixture that allowed a commit on main with no output from git-guard.mjs now produces permissionDecision: "ask", identical to the benign control.

Verified through the phase UAT walk. The milestone audit traced 12/12 requirements with 0 broken and 36/36 acceptance criteria covered.

Fixed in v3.2.0 (PR #124). CFG-01, phase 1. deepMerge defines own properties instead of assigning, so __proto__, constructor and prototype in a repo layer store inertly rather than reparenting the merged config. The fixture that allowed a commit on main with no output from git-guard.mjs now produces permissionDecision: "ask", identical to the benign control. Verified through the phase UAT walk. The milestone audit traced 12/12 requirements with 0 broken and 36/36 acceptance criteria covered.