Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions packages/loopover-engine/src/ams-policy-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,13 @@ function normalizeNonNegativeInteger(value: unknown, field: string, fallback: nu
}

function normalizeCapLimits(value: unknown, fallback: AmsCapLimits, warnings: string[]): AmsCapLimits {
if (value === undefined || value === null) return fallback;
// Fresh copy on the fallback paths too (same reasoning as normalizeNetworkAllowlist): `fallback` is the
// deep-frozen DEFAULT_AMS_POLICY_SPEC.capLimits, so returning it by reference would alias the shared
// singleton into a caller's parsed spec.
if (value === undefined || value === null) return { ...fallback };
if (typeof value !== "object" || Array.isArray(value)) {
warnings.push('AmsPolicySpec field "capLimits" must be a mapping; falling back to defaults.');
return fallback;
return { ...fallback };
}
const record = value as Record<string, unknown>;
return {
Expand All @@ -226,10 +229,12 @@ function normalizeConvergenceThresholds(
fallback: PortfolioConvergenceThresholds,
warnings: string[],
): PortfolioConvergenceThresholds {
if (value === undefined || value === null) return fallback;
// Fresh copy on the fallback paths too: `fallback` is the deep-frozen
// DEFAULT_AMS_POLICY_SPEC.convergenceThresholds, so returning it by reference would alias the shared singleton.
if (value === undefined || value === null) return { ...fallback };
if (typeof value !== "object" || Array.isArray(value)) {
warnings.push('AmsPolicySpec field "convergenceThresholds" must be a mapping; falling back to defaults.');
return fallback;
return { ...fallback };
}
const record = value as Record<string, unknown>;
return {
Expand Down
42 changes: 42 additions & 0 deletions packages/loopover-engine/test/ams-policy-spec-parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,3 +189,45 @@ test("parseAmsPolicySpecContent: JSON and YAML both parse, malformed content deg
assert.equal(oversized.present, false);
assert.match(oversized.warnings.join(" "), /exceeded/i);
});

test("REGRESSION (#9995): parsed capLimits/convergenceThresholds are fresh copies, never aliased to the frozen DEFAULT singleton", () => {
const parsed = parseAmsPolicySpec({ submissionMode: "enforce" }).spec;
// VALUES are unchanged...
assert.deepEqual(parsed.capLimits, DEFAULT_AMS_POLICY_SPEC.capLimits);
assert.deepEqual(parsed.convergenceThresholds, DEFAULT_AMS_POLICY_SPEC.convergenceThresholds);
// ...but the objects are distinct, not the shared frozen singleton returned by reference.
assert.notStrictEqual(parsed.capLimits, DEFAULT_AMS_POLICY_SPEC.capLimits);
assert.notStrictEqual(parsed.convergenceThresholds, DEFAULT_AMS_POLICY_SPEC.convergenceThresholds);
// The returned sub-objects are writable, and mutating them cannot corrupt the shared DEFAULT.
assert.equal(Object.isFrozen(parsed.capLimits), false);
assert.equal(Object.isFrozen(parsed.convergenceThresholds), false);
parsed.capLimits.budget = 999;
parsed.convergenceThresholds.maxReenqueues = 999;
assert.notEqual(DEFAULT_AMS_POLICY_SPEC.capLimits.budget, 999);
assert.notEqual(DEFAULT_AMS_POLICY_SPEC.convergenceThresholds.maxReenqueues, 999);
});

test("REGRESSION (#9995): the not-a-mapping fallback paths also return fresh, non-aliased copies", () => {
const parsed = parseAmsPolicySpec({ capLimits: "nope", convergenceThresholds: [] }).spec;
assert.deepEqual(parsed.capLimits, DEFAULT_AMS_POLICY_SPEC.capLimits);
assert.deepEqual(parsed.convergenceThresholds, DEFAULT_AMS_POLICY_SPEC.convergenceThresholds);
assert.notStrictEqual(parsed.capLimits, DEFAULT_AMS_POLICY_SPEC.capLimits);
assert.notStrictEqual(parsed.convergenceThresholds, DEFAULT_AMS_POLICY_SPEC.convergenceThresholds);

// With ANOTHER field configured (so the parser returns the normalized spec rather than the all-defaults
// clone), the not-a-mapping fallback is the path that actually reaches the caller — it must be fresh too.
const configured = parseAmsPolicySpec({ submissionMode: "enforce", capLimits: "nope", convergenceThresholds: [] }).spec;
assert.deepEqual(configured.capLimits, DEFAULT_AMS_POLICY_SPEC.capLimits);
assert.deepEqual(configured.convergenceThresholds, DEFAULT_AMS_POLICY_SPEC.convergenceThresholds);
assert.notStrictEqual(configured.capLimits, DEFAULT_AMS_POLICY_SPEC.capLimits);
assert.notStrictEqual(configured.convergenceThresholds, DEFAULT_AMS_POLICY_SPEC.convergenceThresholds);
assert.equal(Object.isFrozen(configured.capLimits), false);
});

test("DEFAULT_AMS_POLICY_SPEC and its three sub-objects remain deep-frozen after parsing (#9995)", () => {
parseAmsPolicySpec({ submissionMode: "enforce" });
assert.equal(Object.isFrozen(DEFAULT_AMS_POLICY_SPEC), true);
assert.equal(Object.isFrozen(DEFAULT_AMS_POLICY_SPEC.capLimits), true);
assert.equal(Object.isFrozen(DEFAULT_AMS_POLICY_SPEC.convergenceThresholds), true);
assert.equal(Object.isFrozen(DEFAULT_AMS_POLICY_SPEC.networkAllowlist), true);
});