From a301c5ceb02fda01c920375028f0ea95d4d7e70f Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Tue, 12 May 2026 18:23:52 -0500 Subject: [PATCH] more fixes for perms --- packages/opencode/src/config/config.ts | 45 +++++++------- packages/opencode/src/config/permission.ts | 12 +++- packages/opencode/test/config/config.test.ts | 62 ++++++++++++++++++++ 3 files changed, 92 insertions(+), 27 deletions(-) diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index dec1397a75..6c7310f75d 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -45,21 +45,16 @@ import { Npm } from "@opencode-ai/core/npm" const log = Log.create({ service: "config" }) -// Custom merge function that concatenates array fields instead of replacing them -// Keep remeda's deep conditional merge type out of hot config-loading paths; TS profiling showed it dominates here. -function mergeConfig(target: Info, source: Info): Info { - return mergeDeep(target, source) as Info -} - -function mergeConfigConcatArrays(target: Info, source: Info): Info { - const merged = mergeConfig(target, source) +// Custom merge: deep-merges most fields, but dedupes `instructions` and +// concatenates `permission` as layered configs so user-written rule ordering +// is preserved across config sources. +// Keep remeda's deep conditional merge type out of hot config-loading paths; +// TS profiling showed it dominates here. +function mergeConfigs(target: Info, source: Info): Info { + const merged = mergeDeep(target, source) as Info if (target.instructions && source.instructions) { merged.instructions = Array.from(new Set([...target.instructions, ...source.instructions])) } - // Accumulate permission layers for later merging as rulesets. - // This preserves the ordering semantics: later rules override earlier rules. - // Each layer keeps the raw shape the user wrote on disk; consumers should use - // ConfigPermission.toLayers to normalise. if (source.permission) { merged.permission = [...ConfigPermission.toLayers(target.permission), ...ConfigPermission.toLayers(source.permission)] } @@ -236,9 +231,7 @@ export const Info = Schema.Struct({ description: "Additional instruction files or patterns to include", }), layout: Schema.optional(ConfigLayout.Layout).annotate({ description: "@deprecated Always uses stretch layout." }), - permission: Schema.optional( - Schema.Union([ConfigPermission.Info, Schema.mutable(Schema.Array(ConfigPermission.Info))]), - ).annotate({ + permission: Schema.optional(ConfigPermission.LayersInput).annotate({ description: "Permission configuration. Accepts a single object (per-tool action map) or an array of layered configs; arrays are merged in order so later layers override earlier ones.", }), @@ -431,9 +424,9 @@ export const layer = Layer.effect( .pipe(Effect.catch(() => Effect.void)) } } - result = mergeConfig(result, yield* loadFile(path.join(Global.Path.config, "config.json"))) - result = mergeConfig(result, yield* loadFile(path.join(Global.Path.config, "opencode.json"))) - result = mergeConfig(result, yield* loadFile(path.join(Global.Path.config, "opencode.jsonc"))) + result = mergeConfigs(result, yield* loadFile(path.join(Global.Path.config, "config.json"))) + result = mergeConfigs(result, yield* loadFile(path.join(Global.Path.config, "opencode.json"))) + result = mergeConfigs(result, yield* loadFile(path.join(Global.Path.config, "opencode.jsonc"))) const legacy = path.join(Global.Path.config, "config") if (existsSync(legacy)) { @@ -443,7 +436,7 @@ export const layer = Layer.effect( const { provider, model, ...rest } = mod.default if (provider && model) result.model = `${provider}/${model}` result["$schema"] = "https://opencode.ai/config.json" - result = mergeConfig(result, rest) + result = mergeConfigs(result, rest) await fsNode.writeFile(path.join(Global.Path.config, "config.json"), JSON.stringify(result, null, 2)) await fsNode.unlink(legacy) }) @@ -523,7 +516,7 @@ export const layer = Layer.effect( }) const merge = (source: string, next: Info, kind?: ConfigPlugin.Scope) => { - result = mergeConfigConcatArrays(result, next) + result = mergeConfigs(result, next) return mergePluginOrigins(source, next.plugin, kind) } @@ -557,7 +550,7 @@ export const layer = Layer.effect( return isRecord(data) && isRecord(data.config) ? data.config : data })) as Record) : {} - const remoteConfig = mergeConfig(wellknown.config ?? {}, fetchedConfig as Info) + const remoteConfig = mergeConfigs(wellknown.config ?? {}, fetchedConfig as Info) if (!remoteConfig.$schema) remoteConfig.$schema = "https://opencode.ai/config.json" const source = `${url}/.well-known/opencode` const next = yield* loadConfig(JSON.stringify(remoteConfig), { @@ -701,7 +694,7 @@ export const layer = Layer.effect( // macOS managed preferences (.mobileconfig deployed via MDM) override everything const managed = yield* Effect.promise(() => ConfigManaged.readManagedPreferences()) if (managed) { - result = mergeConfigConcatArrays( + result = mergeConfigs( result, yield* loadConfig(managed.text, { dir: path.dirname(managed.source), @@ -720,8 +713,12 @@ export const layer = Layer.effect( } if (Flag.OPENCODE_PERMISSION) { - const envPermission = JSON.parse(Flag.OPENCODE_PERMISSION) as ConfigPermission.Info - result.permission = [...ConfigPermission.toLayers(result.permission), envPermission] + const envPermission = ConfigParse.schema( + ConfigPermission.LayersInput, + JSON.parse(Flag.OPENCODE_PERMISSION), + "OPENCODE_PERMISSION", + ) + result.permission = [...ConfigPermission.toLayers(result.permission), ...ConfigPermission.toLayers(envPermission)] } if (result.tools) { diff --git a/packages/opencode/src/config/permission.ts b/packages/opencode/src/config/permission.ts index e9853f47a3..a75e71170a 100644 --- a/packages/opencode/src/config/permission.ts +++ b/packages/opencode/src/config/permission.ts @@ -59,9 +59,15 @@ type _Info = Schema.Schema.Type export type Info = { -readonly [K in keyof _Info]: _Info[K] } // Top-level config accepts either a single permission object or an array of -// layered configs. Internal merging produces arrays; this helper normalises -// either shape into the array form expected by consumers. -export function toLayers(value: Info | Info[] | undefined): Info[] { +// layered configs. Validated input goes through this union; runtime merging +// always produces arrays. +export const LayersInput = Schema.Union([Info, Schema.mutable(Schema.Array(Info))]).annotate({ + identifier: "PermissionLayersInput", +}) +export type LayersInput = Schema.Schema.Type + +// Normalise either shape into the array form expected by consumers. +export function toLayers(value: LayersInput | undefined): Info[] { if (!value) return [] return Array.isArray(value) ? value : [value] } diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index a2e4391777..2bbcedc772 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -1891,6 +1891,68 @@ test("user bash catchall overrides inherited bash rules", async () => { }) }) +// Permissions split across multiple global config files (config.json + opencode.json) +// must layer in load order rather than deep-merging into a single object. +test("multiple global config files preserve permission layer ordering", async () => { + await using globalTmp = await tmpdir() + await using tmp = await tmpdir() + const prev = Global.Path.config + ;(Global.Path as { config: string }).config = globalTmp.path + await clear(true) + + try { + // First global file: deny rm-style commands. + await writeConfig(globalTmp.path, { + $schema: "https://opencode.ai/config.json", + permission: { bash: { "rm *": "deny" } }, + }, "config.json") + // Second global file: top-level catchall "ask" — must come *after* the deny layer. + await writeConfig(globalTmp.path, { + $schema: "https://opencode.ai/config.json", + permission: { "*": "ask" }, + }, "opencode.json") + + await WithInstance.provide({ + directory: tmp.path, + fn: async () => { + const config = await load() + const layers = ConfigPermission.toLayers(config.permission) + // Each global file contributes its own layer. + expect(layers.length).toBeGreaterThanOrEqual(2) + const ruleset = Permission.merge(...layers.map((p) => Permission.fromConfig(p))) + // Later "*": "ask" overrides earlier "rm *": "deny" — ordering is preserved. + expect(Permission.evaluate("bash", "rm -rf /", ruleset).action).toBe("ask") + }, + }) + } finally { + ;(Global.Path as { config: string }).config = prev + await clear(true) + } +}) + +test("OPENCODE_PERMISSION env var rejects malformed input", () => { + // Validates the env-var parser surfaces clear errors instead of silently casting. + expect(() => + ConfigParse.schema( + ConfigPermission.LayersInput, + { bash: "maybe" }, + "OPENCODE_PERMISSION", + ), + ).toThrow() +}) + +test("OPENCODE_PERMISSION env var accepts both single-object and array syntax", () => { + const single = ConfigParse.schema(ConfigPermission.LayersInput, { bash: "deny" }, "OPENCODE_PERMISSION") + expect(ConfigPermission.toLayers(single)).toHaveLength(1) + + const layered = ConfigParse.schema( + ConfigPermission.LayersInput, + [{ bash: "deny" }, { bash: { "echo *": "allow" } }], + "OPENCODE_PERMISSION", + ) + expect(ConfigPermission.toLayers(layered)).toHaveLength(2) +}) + test("config parser preserves permission order while rejecting unknown top-level keys", () => { const config = ConfigParse.schema( Config.Info,