🧬 fix: Merge Custom Endpoints by Name Instead of Replacing Entire Array (#12586)

* fix: Merge Custom Endpoints by Name Instead of Replacing Entire Array

The DB base config's `endpoints.custom` array was wholesale-replacing
the YAML-derived array, causing YAML endpoint additions to be silently
lost after the first admin panel save. Add path-aware array merging
to `deepMerge` so keyed arrays (matched by `name`) are merged item-by-item
instead of replaced.

* fix: Harden mergeArrayByKey — deduplicate, sanitize, and prevent mutation

- Remove redundant sourceOrder array; iterate Map.keys() instead to
  prevent duplicate entries when source contains repeated names.
- Sanitize override-only appended items through deepMerge to enforce
  UNSAFE_KEYS prototype-pollution protection on all code paths.
- Shallow-copy unmatched base items to prevent mutation leak-back.
- Add post-OVERRIDE_KEY_MAP remapping note to ARRAY_MERGE_KEYS JSDoc.
- Add tests: duplicate source names, base mutation safety, multi-priority
  sequential merging of the same custom endpoint.

* fix: Add inline comments and test for keyless source items

- Document keyless item drop behavior in mergeArrayByKey with inline
  comment and matching test case.
- Add last-write-wins comment to deduplication test assertion.
- Clarify path semantics in mergeArrayByKey target-iteration comment.

* fix: Relocate keyless-item comment and test target-side preserve

- Move keyless-item comment to the if-guard where the skip happens and
  clarify that target-side keyless items are preserved, not dropped.
- Add test verifying base items without a name field are kept in output.
This commit is contained in:
Danny Avila 2026-04-09 09:38:26 -04:00 committed by GitHub
parent 72cdeb0437
commit 632ffbcb87
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 259 additions and 3 deletions

View file

@ -50,12 +50,187 @@ describe('mergeConfigOverrides', () => {
expect(reg.custom).toBe('yes');
});
it('replaces arrays instead of concatenating', () => {
it('replaces plain arrays (no merge key) instead of concatenating', () => {
const configs = [fakeConfig({ endpoints: ['anthropic', 'google'] }, 10)];
const result = mergeConfigOverrides(baseConfig, configs) as unknown as Record<string, unknown>;
expect(result.endpoints).toEqual(['anthropic', 'google']);
});
it('merges endpoints.custom arrays by name instead of replacing', () => {
const base = {
endpoints: {
custom: [
{ name: 'yaml-only', baseURL: 'https://yaml-only.com', apiKey: 'key1' },
{
name: 'shared',
baseURL: 'https://original.com',
apiKey: 'key2',
models: { default: ['m1'] },
},
],
},
} as unknown as AppConfig;
const configs = [
fakeConfig(
{
endpoints: {
custom: [
{ name: 'shared', baseURL: 'https://overridden.com' },
{ name: 'db-only', baseURL: 'https://db-only.com', apiKey: 'key3' },
],
},
},
10,
),
];
const result = mergeConfigOverrides(base, configs) as unknown as Record<string, unknown>;
const endpoints = result.endpoints as Record<string, unknown>;
const custom = endpoints.custom as Array<Record<string, unknown>>;
expect(custom).toHaveLength(3);
// YAML-only item preserved
expect(custom[0]).toEqual({
name: 'yaml-only',
baseURL: 'https://yaml-only.com',
apiKey: 'key1',
});
// Shared item deep-merged: baseURL overridden, apiKey + models preserved from base
expect(custom[1]).toEqual({
name: 'shared',
baseURL: 'https://overridden.com',
apiKey: 'key2',
models: { default: ['m1'] },
});
// DB-only item appended
expect(custom[2]).toEqual({ name: 'db-only', baseURL: 'https://db-only.com', apiKey: 'key3' });
});
it('preserves all YAML custom endpoints when DB override is empty', () => {
const base = {
endpoints: {
custom: [
{ name: 'ep1', baseURL: 'https://ep1.com' },
{ name: 'ep2', baseURL: 'https://ep2.com' },
],
},
} as unknown as AppConfig;
const configs = [fakeConfig({ endpoints: { custom: [] } }, 10)];
const result = mergeConfigOverrides(base, configs) as unknown as Record<string, unknown>;
const endpoints = result.endpoints as Record<string, unknown>;
const custom = endpoints.custom as Array<Record<string, unknown>>;
expect(custom).toHaveLength(2);
expect(custom[0].name).toBe('ep1');
expect(custom[1].name).toBe('ep2');
});
it('deduplicates when source contains repeated endpoint names', () => {
const base = {
endpoints: { custom: [] },
} as unknown as AppConfig;
const configs = [
fakeConfig(
{
endpoints: {
custom: [
{ name: 'dup', baseURL: 'https://first.com' },
{ name: 'dup', baseURL: 'https://second.com' },
],
},
},
10,
),
];
const result = mergeConfigOverrides(base, configs) as unknown as Record<string, unknown>;
const custom = (result.endpoints as Record<string, unknown>).custom as Array<
Record<string, unknown>
>;
expect(custom).toHaveLength(1);
expect(custom[0].name).toBe('dup');
// last-write-wins: Map.set overwrites on duplicate keys
expect(custom[0].baseURL).toBe('https://second.com');
});
it('silently drops source items without a name field', () => {
const base = {
endpoints: { custom: [{ name: 'ep1', baseURL: 'https://ep1.com' }] },
} as unknown as AppConfig;
const configs = [
fakeConfig({ endpoints: { custom: [{ baseURL: 'https://nameless.com' }] } }, 10),
];
const result = mergeConfigOverrides(base, configs) as unknown as Record<string, unknown>;
const custom = (result.endpoints as Record<string, unknown>).custom as Array<
Record<string, unknown>
>;
expect(custom).toHaveLength(1);
expect(custom[0].name).toBe('ep1');
});
it('preserves base items without a name field', () => {
const base = {
endpoints: { custom: [{ baseURL: 'https://ep1.com' }] },
} as unknown as AppConfig;
const configs = [
fakeConfig({ endpoints: { custom: [{ name: 'db-only', baseURL: 'https://db.com' }] } }, 10),
];
const result = mergeConfigOverrides(base, configs) as unknown as Record<string, unknown>;
const custom = (result.endpoints as Record<string, unknown>).custom as Array<
Record<string, unknown>
>;
expect(custom).toHaveLength(2);
expect(custom[0].baseURL).toBe('https://ep1.com');
expect(custom[1].name).toBe('db-only');
});
it('does not mutate base custom endpoint items', () => {
const base = {
endpoints: { custom: [{ name: 'ep1', baseURL: 'https://ep1.com' }] },
} as unknown as AppConfig;
const configs = [fakeConfig({ endpoints: { custom: [] } }, 10)];
const result = mergeConfigOverrides(base, configs) as unknown as Record<string, unknown>;
const custom = (result.endpoints as Record<string, unknown>).custom as Array<
Record<string, unknown>
>;
custom[0].baseURL = 'https://mutated.com';
const original = (base as unknown as Record<string, unknown>).endpoints as Record<
string,
unknown
>;
expect((original.custom as Array<Record<string, unknown>>)[0].baseURL).toBe('https://ep1.com');
});
it('respects priority for custom endpoint merges — higher priority wins', () => {
const base = {
endpoints: { custom: [{ name: 'shared', baseURL: 'https://yaml.com' }] },
} as unknown as AppConfig;
const configs = [
fakeConfig({ endpoints: { custom: [{ name: 'shared', baseURL: 'https://low.com' }] } }, 10),
fakeConfig({ endpoints: { custom: [{ name: 'shared', baseURL: 'https://high.com' }] } }, 100),
];
const result = mergeConfigOverrides(base, configs) as unknown as Record<string, unknown>;
const custom = (result.endpoints as Record<string, unknown>).custom as Array<
Record<string, unknown>
>;
expect(custom[0].baseURL).toBe('https://high.com');
});
it('does not mutate the base config', () => {
const original = JSON.parse(JSON.stringify(baseConfig));
const configs = [fakeConfig({ interface: { modelSelect: false } }, 10)];

View file

@ -7,6 +7,19 @@ type AnyObject = { [key: string]: unknown };
const MAX_MERGE_DEPTH = 10;
const UNSAFE_KEYS = new Set(['__proto__', 'constructor', 'prototype']);
/**
* Paths within the config tree where arrays of objects should be merged by
* a key field rather than replaced wholesale. `deepMerge` matches items by
* the given key, deep-merges matching pairs, preserves unmatched base items,
* and appends new override-only items.
*
* Paths use AppConfig key names (post-OVERRIDE_KEY_MAP remapping),
* not YAML-level key names. E.g. use `interfaceConfig.x`, not `interface.x`.
*/
const ARRAY_MERGE_KEYS: Record<string, string> = {
'endpoints.custom': 'name',
};
/**
* Maps YAML-level override keys (TCustomConfig) to their AppConfig equivalents.
* Overrides are stored with YAML keys but merged into the already-processed AppConfig
@ -23,12 +36,62 @@ const OVERRIDE_KEY_MAP: Partial<Record<keyof TCustomConfig, keyof AppConfig>> =
turnstile: 'turnstileConfig',
};
function deepMerge<T extends AnyObject>(target: T, source: AnyObject, depth = 0): T {
function mergeArrayByKey(
target: AnyObject[],
source: AnyObject[],
keyField: string,
depth: number,
path: string,
): AnyObject[] {
const sourceByKey = new Map<unknown, AnyObject>();
for (const item of source) {
if (item != null && typeof item === 'object') {
const key = item[keyField];
// Source items without a key value are skipped: no stable identity
// for matching or appending. (Keyless target items are preserved as-is below.)
if (key != null) {
sourceByKey.set(key, item);
}
}
}
const result: AnyObject[] = [];
const seen = new Set<unknown>();
// Pass the array container path (not a per-element path) so item
// properties build paths like 'endpoints.custom.baseURL' for any
// nested ARRAY_MERGE_KEYS lookups.
for (const item of target) {
if (item != null && typeof item === 'object') {
const key = item[keyField];
const override = key != null ? sourceByKey.get(key) : undefined;
if (override) {
result.push(deepMerge(item, override, depth + 1, path));
seen.add(key);
} else {
result.push({ ...item });
}
} else {
result.push(item);
}
}
for (const key of sourceByKey.keys()) {
if (!seen.has(key)) {
result.push(deepMerge({} as AnyObject, sourceByKey.get(key)!, depth + 1, path));
}
}
return result;
}
function deepMerge<T extends AnyObject>(target: T, source: AnyObject, depth = 0, path = ''): T {
const result = { ...target } as AnyObject;
for (const key of Object.keys(source)) {
if (UNSAFE_KEYS.has(key)) {
continue;
}
const currentPath = path ? `${path}.${key}` : key;
const sourceVal = source[key];
const targetVal = result[key];
if (
@ -40,7 +103,25 @@ function deepMerge<T extends AnyObject>(target: T, source: AnyObject, depth = 0)
typeof targetVal === 'object' &&
!Array.isArray(targetVal)
) {
result[key] = deepMerge(targetVal as AnyObject, sourceVal as AnyObject, depth + 1);
result[key] = deepMerge(
targetVal as AnyObject,
sourceVal as AnyObject,
depth + 1,
currentPath,
);
} else if (
depth < MAX_MERGE_DEPTH &&
Array.isArray(sourceVal) &&
Array.isArray(targetVal) &&
ARRAY_MERGE_KEYS[currentPath]
) {
result[key] = mergeArrayByKey(
targetVal as AnyObject[],
sourceVal as AnyObject[],
ARRAY_MERGE_KEYS[currentPath],
depth,
currentPath,
);
} else {
result[key] = sourceVal;
}