fix(#49): Object.hasOwn guards + model_policy precedence in resolveModelForTier
Squashed from claude/fervent-booth-fb7b1f. Hardens resolveModelPolicy against prototype pollution and fixes resolveModelForTier to check model_policy before dynamic_routing. 199 tests green.
This commit is contained in:
5
.changeset/steady-jays-sing.md
Normal file
5
.changeset/steady-jays-sing.md
Normal file
@@ -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.
|
||||
@@ -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) {
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user