From 4ac157b76d7843e0a5d99eac139f9ac21fcd052a Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Sat, 22 Aug 2026 00:59:03 +0700 Subject: [PATCH] fix(opencode): guard the provider block when merging an existing config (#11004) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⭐5 — mergeOpenCodeConfig aplicava guard de objeto na raiz mas espalhava safeConfig.provider sem guard, produzindo configs estruturalmente válidas mas semanticamente corrompidas para provider não-objeto (array vira índices numéricos, string vira caracteres). Aplica o mesmo guard já usado na raiz. TDD, 7 casos novos + 19 testes-irmãos verdes, typecheck/lint limpos. --- .../fixes/opencode-merge-provider-guard.md | 1 + src/shared/services/opencodeConfig.ts | 14 +++- .../opencode-merge-provider-guard.test.ts | 68 +++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 changelog.d/fixes/opencode-merge-provider-guard.md create mode 100644 tests/unit/opencode-merge-provider-guard.test.ts diff --git a/changelog.d/fixes/opencode-merge-provider-guard.md b/changelog.d/fixes/opencode-merge-provider-guard.md new file mode 100644 index 0000000000..aa1f02e63b --- /dev/null +++ b/changelog.d/fixes/opencode-merge-provider-guard.md @@ -0,0 +1 @@ +- **OpenCode config merge:** stop `mergeOpenCodeConfig` splaying a malformed `provider` block into index keys. The root was already guarded against a non-object; the `provider` branch it spreads one level down was not, so an existing `"provider": ["a", "b"]` merged to `{"0": "a", "1": "b", …}`. Its sibling `mergeOpenCodeConfigText` already refuses the same input. diff --git a/src/shared/services/opencodeConfig.ts b/src/shared/services/opencodeConfig.ts index c597bea4d5..a398585dba 100644 --- a/src/shared/services/opencodeConfig.ts +++ b/src/shared/services/opencodeConfig.ts @@ -91,11 +91,23 @@ export const mergeOpenCodeConfig = ( ? existingConfig : {}; + // Same guard as the root above, one level down. Spreading a non-object here + // does not throw, it splays the value into index keys: an existing + // `"provider": ["a", "b"]` merged to `{"0": "a", "1": "b", omniroute: ... }` + // and a string was exploded one character per key. mergeOpenCodeConfigText + // refuses the same input outright, so the two disagreed on what to do with a + // malformed config. + const existingProvider = (safeConfig as Record).provider; + const safeProvider = + existingProvider && typeof existingProvider === "object" && !Array.isArray(existingProvider) + ? (existingProvider as Record) + : {}; + return { ...safeConfig, $schema: safeConfig.$schema || "https://opencode.ai/config.json", provider: { - ...((safeConfig as any).provider || {}), + ...safeProvider, omniroute: buildOpenCodeProviderConfig(input), }, }; diff --git a/tests/unit/opencode-merge-provider-guard.test.ts b/tests/unit/opencode-merge-provider-guard.test.ts new file mode 100644 index 0000000000..d83dc573c1 --- /dev/null +++ b/tests/unit/opencode-merge-provider-guard.test.ts @@ -0,0 +1,68 @@ +// `mergeOpenCodeConfig` guarded the root against a non-object but not the +// `provider` branch it spreads one level down. Spreading a non-object does not +// throw, it splays the value into index keys, so an existing config with a +// malformed `provider` was rewritten into nonsense instead of being rejected: +// +// provider: ["a", "b"] -> { "0": "a", "1": "b", omniroute: {...} } +// provider: "oops" -> { "0": "o", "1": "o", "2": "p", "3": "s", ... } +// +// Its sibling `mergeOpenCodeConfigText` throws on the same input +// ("Can not add index to parent of type array"), so the two disagreed on what a +// malformed config means. +import test from "node:test"; +import assert from "node:assert/strict"; + +const { mergeOpenCodeConfig, mergeOpenCodeConfigText } = + await import("../../src/shared/services/opencodeConfig.ts"); + +const INPUT = { + baseUrl: "http://localhost:20128/v1", + apiKey: "sk_test_opencode", + model: "claude-sonnet-4-5-thinking", +}; + +for (const [label, provider] of [ + ["an array", ["a", "b"]], + ["a string", "oops"], + ["a number", 7], + ["null", null], +] as const) { + test(`mergeOpenCodeConfig drops a provider block that is ${label}`, () => { + const merged = mergeOpenCodeConfig({ provider } as never, INPUT); + + assert.deepEqual( + Object.keys(merged.provider), + ["omniroute"], + `a provider block that is ${label} must not be splayed into index keys` + ); + assert.equal(merged.provider.omniroute.options.baseURL, "http://localhost:20128/v1"); + }); +} + +test("mergeOpenCodeConfig still preserves sibling providers", () => { + // The guard must only reject non-objects, never a legitimate provider map. + const merged = mergeOpenCodeConfig( + { provider: { custom: { name: "Custom Provider" }, other: { name: "Other" } } } as never, + INPUT + ); + + assert.deepEqual(Object.keys(merged.provider).sort(), ["custom", "omniroute", "other"]); + assert.equal(merged.provider.custom.name, "Custom Provider"); +}); + +test("mergeOpenCodeConfig still guards the root itself", () => { + for (const existing of [["a"], "oops", 7, null, undefined]) { + const merged = mergeOpenCodeConfig(existing as never, INPUT); + assert.deepEqual(Object.keys(merged.provider), ["omniroute"]); + assert.equal(merged.$schema, "https://opencode.ai/config.json"); + } +}); + +test("mergeOpenCodeConfigText keeps refusing the same malformed input", () => { + // Pinning the sibling's behaviour: the object variant now drops the bad + // block, the text variant still refuses to touch the file. Both are safe; + // neither corrupts. + for (const text of ['{"provider": ["a","b"]}', '{"provider": "oops"}']) { + assert.throws(() => mergeOpenCodeConfigText(text, INPUT)); + } +});