diff --git a/.changeset/steady-jays-sing.md b/.changeset/steady-jays-sing.md new file mode 100644 index 000000000..e6796101a --- /dev/null +++ b/.changeset/steady-jays-sing.md @@ -0,0 +1,5 @@ +--- +type: Security +pr: 572 +--- +Hardened resolveModelPolicy against prototype pollution: Object.hasOwn guards now block \_\_proto\_\_ / constructor keys in user-supplied provider/budget/runtime_tiers from reaching inherited prototype slots. Also fixed resolveModelForTier to check model_policy before dynamic_routing so provider presets are not silently bypassed when dynamic routing is enabled. diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index 49bc5781c..a60af82ac 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -1569,13 +1569,15 @@ function resolveModelPolicy(policy, tier) { // merging config.runtime into the policy object (see step 2.5 in resolveModelInternal). const runtime = policy.runtime; const rtOverrides = policy.runtime_tiers; - if (runtime && rtOverrides && typeof rtOverrides === 'object') { - const runtimeEntry = rtOverrides[runtime]; - if (runtimeEntry && typeof runtimeEntry === 'object') { - const raw = runtimeEntry[tier]; - if (raw != null) { - const entry = typeof raw === 'string' ? { model: raw } : raw; - if (entry && entry.model) return entry.model; + if (runtime && typeof runtime === 'string' && rtOverrides && typeof rtOverrides === 'object') { + if (Object.hasOwn(rtOverrides, runtime)) { + const runtimeEntry = rtOverrides[runtime]; + if (runtimeEntry && typeof runtimeEntry === 'object' && Object.hasOwn(runtimeEntry, tier)) { + const raw = runtimeEntry[tier]; + if (raw != null) { + const entry = typeof raw === 'string' ? { model: raw } : raw; + if (entry && entry.model) return entry.model; + } } } } @@ -1595,14 +1597,19 @@ function resolveModelPolicy(policy, tier) { return (v && typeof v === 'string') ? v : null; } + // Object.hasOwn guards prevent __proto__ / constructor key escalation from + // user-controlled policy.provider / policy.budget reaching inherited slots. + if (!Object.hasOwn(PROVIDER_PRESETS, provider)) return null; const presetForProvider = PROVIDER_PRESETS[provider]; - if (!presetForProvider) return null; // Unknown provider — fall through silently. + if (!presetForProvider || typeof presetForProvider !== 'object') return null; + if (!Object.hasOwn(presetForProvider, tier)) return null; const tierPresets = presetForProvider[tier]; - if (!tierPresets) return null; // Tier not in preset (partial preset catalog entry). + if (!tierPresets || typeof tierPresets !== 'object') return null; // Budget defaults to 'medium' — mirrors the existing 'balanced' default bias. const budget = (policy.budget && typeof policy.budget === 'string') ? policy.budget : 'medium'; + if (!Object.hasOwn(tierPresets, budget)) return null; const budgetEntry = tierPresets[budget]; if (!budgetEntry || !budgetEntry.model) return null; // Missing or null budget slot. @@ -1748,6 +1755,14 @@ function resolveModelForTier(cwd, agentType, attempt) { const override = config.model_overrides?.[agentType]; if (override) return override; + // model_policy beats dynamic_routing (#49 Codex adversarial HIGH finding). + // Delegate to resolveModelInternal which handles step 2.5 (policy) correctly + // and falls through to runtimeTierDefaults on a miss. Gate on non-Claude + // runtime to match the same condition in resolveModelInternal step 2.5. + if (config.model_policy && config.runtime && config.runtime !== 'claude') { + return resolveModelInternal(cwd, agentType); + } + const dr = config.dynamic_routing; // Disabled / missing / non-object → fall back to the existing resolver. if (!dr || typeof dr !== 'object' || dr.enabled !== true) { diff --git a/tests/feat-49-model-policy-presets.test.cjs b/tests/feat-49-model-policy-presets.test.cjs index 3d13efe8a..2d44cd211 100644 --- a/tests/feat-49-model-policy-presets.test.cjs +++ b/tests/feat-49-model-policy-presets.test.cjs @@ -63,6 +63,7 @@ const os = require('node:os'); const { resolveModelInternal, resolveModelPolicy, + resolveModelForTier, KNOWN_PROVIDERS, _resetRuntimeWarningCacheForTests, } = require('../get-shit-done/bin/lib/core.cjs'); @@ -678,3 +679,108 @@ describe('#49 KNOWN_PROVIDERS exports from model-catalog.cjs and core.cjs', () = 'KNOWN_PROVIDERS from core.cjs (re-export) must match model-catalog.cjs canonical export'); }); }); + +// ─── resolveModelPolicy: Object.hasOwn prototype-pollution guards ──────────── + +describe('#49 resolveModelPolicy: prototype-pollution guards', () => { + test('__proto__ as provider returns null without throwing', () => { + assert.strictEqual(resolveModelPolicy({ provider: '__proto__', budget: 'medium' }, 'sonnet'), null); + }); + + test('constructor as provider returns null without throwing', () => { + assert.strictEqual(resolveModelPolicy({ provider: 'constructor', budget: 'medium' }, 'sonnet'), null); + }); + + test('__proto__ as budget returns null without throwing', () => { + assert.strictEqual(resolveModelPolicy({ provider: 'openai', budget: '__proto__' }, 'haiku'), null); + }); + + test('toString as budget returns null without throwing', () => { + assert.strictEqual(resolveModelPolicy({ provider: 'openai', budget: 'toString' }, 'haiku'), null); + }); + + test('__proto__ as runtime_tiers key returns null without throwing', () => { + const policy = { + runtime: '__proto__', + runtime_tiers: { '__proto__': { haiku: { model: 'evil' } } }, + }; + assert.strictEqual(resolveModelPolicy(policy, 'haiku'), null); + }); + + test('__proto__ as tier inside runtime_tiers returns null without throwing', () => { + const policy = { + runtime: 'codex', + runtime_tiers: { codex: { '__proto__': { model: 'evil' } } }, + }; + assert.strictEqual(resolveModelPolicy(policy, '__proto__'), null); + }); + + test('valid provider+tier+budget still resolves correctly after guards', () => { + const result = resolveModelPolicy({ provider: 'openai', budget: 'low' }, 'haiku'); + assert.ok(typeof result === 'string' && result.length > 0, + 'valid openai/haiku/low lookup must still resolve after adding hasOwn guards'); + }); +}); + +// ─── resolveModelForTier: model_policy beats dynamic_routing ───────────────── + +describe('#49 resolveModelForTier: model_policy beats dynamic_routing', () => { + let tmpDir; + beforeEach(() => { tmpDir = makeTmp('for-tier-'); }); + afterEach(() => { rmr(tmpDir); }); + + test('model_policy wins over dynamic_routing.tier_models when both are set', () => { + writeConfig(tmpDir, { + runtime: 'codex', + model_policy: { provider: 'openai', budget: 'low' }, + dynamic_routing: { + enabled: true, + tier_models: { light: 'haiku', standard: 'sonnet', heavy: 'opus' }, + }, + }); + // model_policy fires before dynamic_routing in resolveModelForTier + const result = resolveModelForTier(tmpDir, 'gsd-executor', 0); + // gsd-executor is standard/sonnet tier; openai+low+sonnet preset model + assert.ok(typeof result === 'string' && result.length > 0, + 'model_policy must return a model string'); + assert.notStrictEqual(result, 'sonnet', + 'dynamic_routing tier alias must not win over model_policy'); + }); + + test('model_overrides still beats model_policy in resolveModelForTier', () => { + writeConfig(tmpDir, { + runtime: 'codex', + model_policy: { provider: 'openai', budget: 'high' }, + dynamic_routing: { + enabled: true, + tier_models: { light: 'haiku', standard: 'sonnet', heavy: 'opus' }, + }, + model_overrides: { 'gsd-planner': 'custom-model-id' }, + }); + assert.strictEqual(resolveModelForTier(tmpDir, 'gsd-planner', 0), 'custom-model-id'); + }); + + test('dynamic_routing.tier_models used normally when model_policy absent', () => { + writeConfig(tmpDir, { + runtime: 'codex', + dynamic_routing: { + enabled: true, + tier_models: { light: 'haiku', standard: 'my-custom-sonnet', heavy: 'opus' }, + }, + }); + assert.strictEqual(resolveModelForTier(tmpDir, 'gsd-executor', 0), 'my-custom-sonnet'); + }); + + test('model_policy with Claude runtime does not interrupt dynamic_routing', () => { + // model_policy only gates on non-Claude runtimes; with runtime absent/claude, + // dynamic_routing must still work normally. + writeConfig(tmpDir, { + model_policy: { provider: 'openai', budget: 'low' }, + dynamic_routing: { + enabled: true, + tier_models: { light: 'haiku', standard: 'my-sonnet', heavy: 'opus' }, + }, + }); + assert.strictEqual(resolveModelForTier(tmpDir, 'gsd-executor', 0), 'my-sonnet'); + }); +});