Skip to content

Commit b11570a

Browse files
committed
fix(config): address copilot review comments
- Strip unknown config keys before schema validation to preserve valid fields - Sanitize log.error calls to avoid leaking config content via cause - Handle plugin resolution per-item to avoid partial mutation on failure - Add test for plugin resolution failure
1 parent 702fd0e commit b11570a

2 files changed

Lines changed: 40 additions & 8 deletions

File tree

packages/opencode/src/config/config.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import * as Cause from "effect/Cause"
21
import * as Log from "@opencode-ai/core/util/log"
32
import { serviceUse } from "@opencode-ai/core/effect/service-use"
43
import path from "path"
@@ -73,6 +72,23 @@ function normalizeLoadedConfig(data: unknown, source: string) {
7372
return copy
7473
}
7574

75+
const infoKeys = new Set([
76+
"$schema", "shell", "logLevel", "server", "command", "skills",
77+
"reference", "watcher", "snapshot", "plugin", "share", "autoshare",
78+
"autoupdate", "disabled_providers", "enabled_providers", "model",
79+
"small_model", "default_agent", "username", "mode", "agent",
80+
"instructions", "account_token", "url", "headers",
81+
])
82+
83+
function stripUnknownKeys(data: unknown): unknown {
84+
if (typeof data !== "object" || data === null || Array.isArray(data)) return data
85+
const result: Record<string, unknown> = {}
86+
for (const [key, value] of Object.entries(data)) {
87+
if (infoKeys.has(key)) result[key] = value
88+
}
89+
return result
90+
}
91+
7692
async function substituteWellKnownRemoteConfig(input: {
7793
value: unknown
7894
dir: string
@@ -118,9 +134,11 @@ const WellKnownConfig = Schema.Struct({
118134
async function resolveLoadedPlugins<T extends { plugin?: ConfigPlugin.Spec[] }>(config: T, filepath: string) {
119135
if (!config.plugin) return config
120136
for (let i = 0; i < config.plugin.length; i++) {
121-
// Normalize path-like plugin specs while we still know which config file declared them.
122-
// This prevents `./plugin.ts` from being reinterpreted relative to some later merge location.
123-
config.plugin[i] = await ConfigPlugin.resolvePluginSpec(config.plugin[i], filepath)
137+
try {
138+
config.plugin[i] = await ConfigPlugin.resolvePluginSpec(config.plugin[i], filepath)
139+
} catch {
140+
log.error("plugin resolution failed", { spec: config.plugin[i], path: filepath })
141+
}
124142
}
125143
return config
126144
}
@@ -424,23 +442,24 @@ export const layer = Layer.effect(
424442
let parsedOk = false
425443
const data = yield* Effect.sync(() => {
426444
const parsed = ConfigParse.jsonc(expanded, source)
427-
const result = ConfigParse.schema(Info, normalizeLoadedConfig(parsed, source), source)
445+
const cleaned = stripUnknownKeys(normalizeLoadedConfig(parsed, source))
446+
const result = ConfigParse.schema(Info, cleaned, source)
428447
parsedOk = true
429448
return result
430449
}).pipe(
431450
Effect.catchCause((cause) =>
432451
Effect.sync(() => {
433-
log.error("invalid config: config file could not be parsed", { path: source, cause: Cause.pretty(cause) })
452+
log.error("invalid config: config file could not be parsed", { path: source })
434453
return {} as Info
435454
}),
436455
),
437456
)
438457
if (!("path" in options)) return data
439458

440459
yield* Effect.promise(() => resolveLoadedPlugins(data, options.path)).pipe(
441-
Effect.catchCause((cause) =>
460+
Effect.catchCause(() =>
442461
Effect.sync(() => {
443-
log.error("plugin resolution failed", { path: source, cause: Cause.pretty(cause) })
462+
log.error("plugin resolution failed", { path: source })
444463
}),
445464
),
446465
)

packages/opencode/test/config/config.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,19 @@ it.instance("skips bad config file but merges others", () =>
639639
}),
640640
)
641641

642+
it.instance("handles plugin resolution failure gracefully", () =>
643+
Effect.gen(function* () {
644+
const test = yield* TestInstance
645+
yield* writeConfigEffect(test.directory, {
646+
$schema: "https://opencode.ai/config.json",
647+
model: "has-plugin",
648+
plugin: ["./non-existent-plugin.ts"],
649+
})
650+
const config = yield* Config.use.get()
651+
expect(config.model).toBe("has-plugin")
652+
}),
653+
)
654+
642655
it.instance("handles agent configuration", () =>
643656
Effect.gen(function* () {
644657
const test = yield* TestInstance

0 commit comments

Comments
 (0)