diff --git a/packages/opencode/src/agent/agent.ts b/packages/opencode/src/agent/agent.ts index fc0b645ab5..1f0579b0db 100644 --- a/packages/opencode/src/agent/agent.ts +++ b/packages/opencode/src/agent/agent.ts @@ -119,7 +119,7 @@ export const layer = Layer.effect( // Convert permission layers to rulesets and merge them // Each layer's rules come after the previous, so later configs override earlier ones - const layers = (cfg.permission_layers ?? []) as ConfigPermission.Info[] + const layers = ConfigPermission.toLayers(cfg.permission) const user = Permission.merge(...layers.map((p) => Permission.fromConfig(p))) const agents: Record = { diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 12127d5090..dec1397a75 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -56,10 +56,12 @@ function mergeConfigConcatArrays(target: Info, source: Info): 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 + // 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_layers = [...(target.permission_layers ?? []), source.permission] + merged.permission = [...ConfigPermission.toLayers(target.permission), ...ConfigPermission.toLayers(source.permission)] } return merged } @@ -234,9 +236,11 @@ 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(ConfigPermission.Info), - permission_layers: Schema.optional(Schema.mutable(Schema.Array(ConfigPermission.Info))).annotate({ - description: "Internal: permission configs from each source for layered merging", + permission: Schema.optional( + Schema.Union([ConfigPermission.Info, Schema.mutable(Schema.Array(ConfigPermission.Info))]), + ).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.", }), tools: Schema.optional(Schema.Record(Schema.String, Schema.Boolean)), attachment: Schema.optional(ConfigAttachment.Info).annotate({ @@ -717,7 +721,7 @@ export const layer = Layer.effect( if (Flag.OPENCODE_PERMISSION) { const envPermission = JSON.parse(Flag.OPENCODE_PERMISSION) as ConfigPermission.Info - result.permission_layers = [...(result.permission_layers ?? []), envPermission] + result.permission = [...ConfigPermission.toLayers(result.permission), envPermission] } if (result.tools) { @@ -731,7 +735,7 @@ export const layer = Layer.effect( perms[tool] = action } // Tools permissions come before other permissions (they can be overridden) - result.permission_layers = [perms, ...(result.permission_layers ?? [])] + result.permission = [perms, ...ConfigPermission.toLayers(result.permission)] } if (!result.username) result.username = os.userInfo().username diff --git a/packages/opencode/src/config/permission.ts b/packages/opencode/src/config/permission.ts index a04b404e86..e9853f47a3 100644 --- a/packages/opencode/src/config/permission.ts +++ b/packages/opencode/src/config/permission.ts @@ -57,3 +57,11 @@ export const Info = InputSchema.pipe( ).annotate({ identifier: "PermissionConfig" }) 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[] { + 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 454f3e487a..a2e4391777 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -278,6 +278,40 @@ test("updates global config and omits empty shell key in json", async () => { } }) +test("global config update preserves single-object permission shape on disk", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Filesystem.write( + path.join(dir, "opencode.json"), + JSON.stringify({ + $schema: "https://opencode.ai/config.json", + shell: "bash", + permission: { bash: "ask" }, + }), + ) + }, + }) + + const prev = Global.Path.config + ;(Global.Path as { config: string }).config = tmp.path + await clear(true) + + try { + // Updating an unrelated key must not rewrite `permission` from object to array form. + await saveGlobal({ shell: "zsh" }) + + const written = await Filesystem.readJson<{ permission?: unknown; shell?: string }>( + path.join(tmp.path, "opencode.json"), + ) + expect(written.shell).toBe("zsh") + expect(Array.isArray(written.permission)).toBe(false) + expect(written.permission).toEqual({ bash: "ask" }) + } finally { + ;(Global.Path as { config: string }).config = prev + await clear(true) + } +}) + test("updates global config and omits empty shell key in jsonc", async () => { await using tmp = await tmpdir({ init: async (dir) => { @@ -1715,7 +1749,10 @@ test("permission config preserves user key order", async () => { directory: tmp.path, fn: async () => { const config = await load() - expect(Object.keys(config.permission!)).toEqual([ + // load() goes through the merge pipeline, producing the layered array form + expect(config.permission).toHaveLength(1) + const perm = (config.permission as ConfigPermission.Info[])[0] + expect(Object.keys(perm)).toEqual([ "*", "edit", "write", @@ -1758,12 +1795,11 @@ test("user top-level catchall overrides inherited bash rules", async () => { ) }, }) - await Instance.provide({ + await WithInstance.provide({ directory: tmp.path, fn: async () => { const config = await load() - // Use permission_layers for correct ordering (each layer's rules come after previous) - const layers = (config.permission_layers ?? []) as ConfigPermission.Info[] + const layers = ConfigPermission.toLayers(config.permission) const ruleset = Permission.merge(...layers.map((p) => Permission.fromConfig(p))) expect(Permission.evaluate("bash", "rm -rf /", ruleset).action).toBe("ask") @@ -1799,12 +1835,11 @@ test("inherited bash rules apply when no user top-level catchall", async () => { ) }, }) - await Instance.provide({ + await WithInstance.provide({ directory: tmp.path, fn: async () => { const config = await load() - // Use permission_layers for correct ordering - const layers = (config.permission_layers ?? []) as ConfigPermission.Info[] + const layers = ConfigPermission.toLayers(config.permission) const ruleset = Permission.merge(...layers.map((p) => Permission.fromConfig(p))) expect(Permission.evaluate("bash", "rm -rf /", ruleset).action).toBe("deny") @@ -1839,12 +1874,11 @@ test("user bash catchall overrides inherited bash rules", async () => { ) }, }) - await Instance.provide({ + await WithInstance.provide({ directory: tmp.path, fn: async () => { const config = await load() - // Use permission_layers for correct ordering - const layers = (config.permission_layers ?? []) as ConfigPermission.Info[] + const layers = ConfigPermission.toLayers(config.permission) const ruleset = Permission.merge(...layers.map((p) => Permission.fromConfig(p))) expect(Permission.evaluate("bash", "rm -rf /", ruleset).action).toBe("ask") @@ -1870,7 +1904,8 @@ test("config parser preserves permission order while rejecting unknown top-level "test", ) - expect(Object.keys(config.permission!)).toEqual(["bash", "*", "edit"]) + // ConfigParse.schema preserves the raw shape the user wrote + expect(Object.keys(config.permission as ConfigPermission.Info)).toEqual(["bash", "*", "edit"]) try { ConfigParse.schema(Config.Info, { invalid_field: true }, "test") throw new Error("expected config parse to fail") @@ -2707,11 +2742,12 @@ test("parseManagedPlist parses permission rules", async () => { ), "test:mobileconfig", ) - expect(config.permission?.["*"]).toBe("ask") - expect(config.permission?.grep).toBe("allow") - expect(config.permission?.webfetch).toBe("ask") - expect(config.permission?.["~/.ssh/*"]).toBe("deny") - const bash = config.permission?.bash as Record + const perm = config.permission as ConfigPermission.Info + expect(perm?.["*"]).toBe("ask") + expect(perm?.grep).toBe("allow") + expect(perm?.webfetch).toBe("ask") + expect(perm?.["~/.ssh/*"]).toBe("deny") + const bash = perm?.bash as Record expect(bash?.["rm -rf *"]).toBe("deny") expect(bash?.["curl *"]).toBe("deny") }) diff --git a/packages/opencode/test/permission-task.test.ts b/packages/opencode/test/permission-task.test.ts index 64b93bb8bc..77ceda8a4c 100644 --- a/packages/opencode/test/permission-task.test.ts +++ b/packages/opencode/test/permission-task.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, test, expect } from "bun:test" import { Permission } from "../src/permission" import { Config } from "@/config/config" +import { ConfigPermission } from "@/config/permission" import { Instance } from "../src/project/instance" import { WithInstance } from "../src/project/with-instance" import { disposeAllInstances, tmpdir } from "./fixture/fixture" @@ -163,7 +164,9 @@ describe("permission.task with real config files", () => { directory: tmp.path, fn: async () => { const config = await load() - const ruleset = Permission.fromConfig(config.permission ?? {}) + const ruleset = Permission.merge( + ...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)), + ) // general and orchestrator-fast should be allowed, code-reviewer denied expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow") expect(Permission.evaluate("task", "orchestrator-fast", ruleset).action).toBe("allow") @@ -188,7 +191,9 @@ describe("permission.task with real config files", () => { directory: tmp.path, fn: async () => { const config = await load() - const ruleset = Permission.fromConfig(config.permission ?? {}) + const ruleset = Permission.merge( + ...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)), + ) // general and code-reviewer should be ask, orchestrator-* denied expect(Permission.evaluate("task", "general", ruleset).action).toBe("ask") expect(Permission.evaluate("task", "code-reviewer", ruleset).action).toBe("ask") @@ -213,7 +218,9 @@ describe("permission.task with real config files", () => { directory: tmp.path, fn: async () => { const config = await load() - const ruleset = Permission.fromConfig(config.permission ?? {}) + const ruleset = Permission.merge( + ...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)), + ) expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow") expect(Permission.evaluate("task", "code-reviewer", ruleset).action).toBe("deny") // Unspecified agents default to "ask" @@ -240,7 +247,9 @@ describe("permission.task with real config files", () => { directory: tmp.path, fn: async () => { const config = await load() - const ruleset = Permission.fromConfig(config.permission ?? {}) + const ruleset = Permission.merge( + ...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)), + ) // Verify task permissions expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow") @@ -278,7 +287,9 @@ describe("permission.task with real config files", () => { directory: tmp.path, fn: async () => { const config = await load() - const ruleset = Permission.fromConfig(config.permission ?? {}) + const ruleset = Permission.merge( + ...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)), + ) // Last matching rule wins - "*" deny is last, so all agents are denied expect(Permission.evaluate("task", "general", ruleset).action).toBe("deny") @@ -309,7 +320,9 @@ describe("permission.task with real config files", () => { directory: tmp.path, fn: async () => { const config = await load() - const ruleset = Permission.fromConfig(config.permission ?? {}) + const ruleset = Permission.merge( + ...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)), + ) // Evaluate uses findLast - "general" allow comes after "*" deny expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow")