golem15/msd-core
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(#751): recognise config-set prototype-pollution guard in CodeQL + test dynamic-key vectors (#752)

Browse Source 
CodeQL alert #26 (js/prototype-pollution-utility) kept firing on setConfigValue
because its dataflow does not trace the #663 Set-based, pre-loop keys.some(...)
forbidden-key check as a sanitising barrier on the write site.

- src/config.cts: replace the Set + pre-loop check with inline literal
  comparisons (key === '__proto__' || 'prototype' || 'constructor') on the exact
  key used to index `current`, immediately before each write (intermediate keys
  in the descent loop, plus the final key). Same forbidden set, same error
  message and ERROR_REASON.CONFIG_PARSE_FAILED — behaviour unchanged from #663,
  but the barrier is now CodeQL-recognised.
- tests/config.test.cjs: add regression tests for schema-valid dynamic-prefix
  keys (agent_skills.__proto__, agent_skills.constructor, agent_skills.prototype,
  features.__proto__, review.models.constructor) that pass the isValidConfigKey
  schema gate and reach the guard. Each asserts the guard's own message fires
  (not the schema gate's "Unknown config key") and Object.prototype is not
  polluted. The prior #663 tests never reached the guard — their keys are
  rejected by the schema gate first — so the guard's real attack surface was
  untested.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-07 00:18:00 -04:00
committed by GitHub
parent 7e76f1a736
commit 612e3e60e8
3 changed files with 142 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
src/config.cts
View File

@@ -429,22 +429,31 @@ function setConfigValue(cwd: string, keyPath: string, parsedValue: unknown): Set
error('Failed to read config.json: ' + (err as Error).message, ERROR_REASON.CONFIG_PARSE_FAILED);
}
// Set nested value using dot notation (e.g., "workflow.research")
// Set nested value using dot notation (e.g., "workflow.research").
// Prototype-pollution guard: reject dangerous segments via inline literal
// comparisons on the exact key used to index `current`, immediately before
// each write. The inline comparison is the barrier CodeQL's
// js/prototype-pollution-utility query recognises — the previous Set-based
// pre-loop check was functionally correct but not traced through, so
// code-scanning alert #26 kept firing. Behaviour is unchanged from #663.
const keys = keyPath.split('.');
const FORBIDDEN_KEYS = new Set(['__proto__', 'prototype', 'constructor']);
if (keys.some((k) => FORBIDDEN_KEYS.has(k))) {
error('Invalid config key (prototype pollution guard): ' + keyPath, ERROR_REASON.CONFIG_PARSE_FAILED);
}
let current: Record<string, unknown> = config;
for (let i = 0; i < keys.length - 1; i++) {
const key = keys[i];
if (key === '__proto__' || key === 'prototype' || key === 'constructor') {
error('Invalid config key (prototype pollution guard): ' + keyPath, ERROR_REASON.CONFIG_PARSE_FAILED);
}
if (current[key] === undefined || typeof current[key] !== 'object') {
current[key] = {};
}
current = current[key] as Record<string, unknown>;
}
const previousValue = current[keys[keys.length - 1]]; // Capture previous value before overwriting
current[keys[keys.length - 1]] = parsedValue;
const lastKey = keys[keys.length - 1];
if (lastKey === '__proto__' || lastKey === 'prototype' || lastKey === 'constructor') {
error('Invalid config key (prototype pollution guard): ' + keyPath, ERROR_REASON.CONFIG_PARSE_FAILED);
}
const previousValue = current[lastKey]; // Capture previous value before overwriting
current[lastKey] = parsedValue;
// Write back
try {