refactor: fold permission_layers into permission as a union

Replace the internal permission_layers config field with a union on permission
itself (single object or array of layered configs). Add ConfigPermission.toLayers
to normalise at consumption sites. Schema has no decode transform, so user
files round-trip through Config.update / updateGlobal without their permission
section being rewritten into array form.
This commit is contained in:
Aiden Cline 2026-05-12 01:30:10 -05:00
commit d4e3106fca
5 changed files with 92 additions and 31 deletions

View file

@ -119,7 +119,7 @@ export const layer = Layer.effect(
// Convert permission layers to rulesets and merge them // Convert permission layers to rulesets and merge them
// Each layer's rules come after the previous, so later configs override earlier ones // 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 user = Permission.merge(...layers.map((p) => Permission.fromConfig(p)))
const agents: Record<string, Info> = { const agents: Record<string, Info> = {

View file

@ -56,10 +56,12 @@ function mergeConfigConcatArrays(target: Info, source: Info): Info {
if (target.instructions && source.instructions) { if (target.instructions && source.instructions) {
merged.instructions = Array.from(new Set([...target.instructions, ...source.instructions])) merged.instructions = Array.from(new Set([...target.instructions, ...source.instructions]))
} }
// Accumulate permission layers for later merging as rulesets // Accumulate permission layers for later merging as rulesets.
// This preserves the ordering semantics: later rules override earlier rules // 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) { if (source.permission) {
merged.permission_layers = [...(target.permission_layers ?? []), source.permission] merged.permission = [...ConfigPermission.toLayers(target.permission), ...ConfigPermission.toLayers(source.permission)]
} }
return merged return merged
} }
@ -234,9 +236,11 @@ export const Info = Schema.Struct({
description: "Additional instruction files or patterns to include", description: "Additional instruction files or patterns to include",
}), }),
layout: Schema.optional(ConfigLayout.Layout).annotate({ description: "@deprecated Always uses stretch layout." }), layout: Schema.optional(ConfigLayout.Layout).annotate({ description: "@deprecated Always uses stretch layout." }),
permission: Schema.optional(ConfigPermission.Info), permission: Schema.optional(
permission_layers: Schema.optional(Schema.mutable(Schema.Array(ConfigPermission.Info))).annotate({ Schema.Union([ConfigPermission.Info, Schema.mutable(Schema.Array(ConfigPermission.Info))]),
description: "Internal: permission configs from each source for layered merging", ).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)), tools: Schema.optional(Schema.Record(Schema.String, Schema.Boolean)),
attachment: Schema.optional(ConfigAttachment.Info).annotate({ attachment: Schema.optional(ConfigAttachment.Info).annotate({
@ -717,7 +721,7 @@ export const layer = Layer.effect(
if (Flag.OPENCODE_PERMISSION) { if (Flag.OPENCODE_PERMISSION) {
const envPermission = JSON.parse(Flag.OPENCODE_PERMISSION) as ConfigPermission.Info 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) { if (result.tools) {
@ -731,7 +735,7 @@ export const layer = Layer.effect(
perms[tool] = action perms[tool] = action
} }
// Tools permissions come before other permissions (they can be overridden) // 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 if (!result.username) result.username = os.userInfo().username

View file

@ -57,3 +57,11 @@ export const Info = InputSchema.pipe(
).annotate({ identifier: "PermissionConfig" }) ).annotate({ identifier: "PermissionConfig" })
type _Info = Schema.Schema.Type<typeof InputObject> type _Info = Schema.Schema.Type<typeof InputObject>
export type Info = { -readonly [K in keyof _Info]: _Info[K] } 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]
}

View file

@ -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 () => { test("updates global config and omits empty shell key in jsonc", async () => {
await using tmp = await tmpdir({ await using tmp = await tmpdir({
init: async (dir) => { init: async (dir) => {
@ -1715,7 +1749,10 @@ test("permission config preserves user key order", async () => {
directory: tmp.path, directory: tmp.path,
fn: async () => { fn: async () => {
const config = await load() 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", "edit",
"write", "write",
@ -1758,12 +1795,11 @@ test("user top-level catchall overrides inherited bash rules", async () => {
) )
}, },
}) })
await Instance.provide({ await WithInstance.provide({
directory: tmp.path, directory: tmp.path,
fn: async () => { fn: async () => {
const config = await load() const config = await load()
// Use permission_layers for correct ordering (each layer's rules come after previous) const layers = ConfigPermission.toLayers(config.permission)
const layers = (config.permission_layers ?? []) as ConfigPermission.Info[]
const ruleset = Permission.merge(...layers.map((p) => Permission.fromConfig(p))) const ruleset = Permission.merge(...layers.map((p) => Permission.fromConfig(p)))
expect(Permission.evaluate("bash", "rm -rf /", ruleset).action).toBe("ask") 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, directory: tmp.path,
fn: async () => { fn: async () => {
const config = await load() const config = await load()
// Use permission_layers for correct ordering const layers = ConfigPermission.toLayers(config.permission)
const layers = (config.permission_layers ?? []) as ConfigPermission.Info[]
const ruleset = Permission.merge(...layers.map((p) => Permission.fromConfig(p))) const ruleset = Permission.merge(...layers.map((p) => Permission.fromConfig(p)))
expect(Permission.evaluate("bash", "rm -rf /", ruleset).action).toBe("deny") 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, directory: tmp.path,
fn: async () => { fn: async () => {
const config = await load() const config = await load()
// Use permission_layers for correct ordering const layers = ConfigPermission.toLayers(config.permission)
const layers = (config.permission_layers ?? []) as ConfigPermission.Info[]
const ruleset = Permission.merge(...layers.map((p) => Permission.fromConfig(p))) const ruleset = Permission.merge(...layers.map((p) => Permission.fromConfig(p)))
expect(Permission.evaluate("bash", "rm -rf /", ruleset).action).toBe("ask") 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", "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 { try {
ConfigParse.schema(Config.Info, { invalid_field: true }, "test") ConfigParse.schema(Config.Info, { invalid_field: true }, "test")
throw new Error("expected config parse to fail") throw new Error("expected config parse to fail")
@ -2707,11 +2742,12 @@ test("parseManagedPlist parses permission rules", async () => {
), ),
"test:mobileconfig", "test:mobileconfig",
) )
expect(config.permission?.["*"]).toBe("ask") const perm = config.permission as ConfigPermission.Info
expect(config.permission?.grep).toBe("allow") expect(perm?.["*"]).toBe("ask")
expect(config.permission?.webfetch).toBe("ask") expect(perm?.grep).toBe("allow")
expect(config.permission?.["~/.ssh/*"]).toBe("deny") expect(perm?.webfetch).toBe("ask")
const bash = config.permission?.bash as Record<string, string> expect(perm?.["~/.ssh/*"]).toBe("deny")
const bash = perm?.bash as Record<string, string>
expect(bash?.["rm -rf *"]).toBe("deny") expect(bash?.["rm -rf *"]).toBe("deny")
expect(bash?.["curl *"]).toBe("deny") expect(bash?.["curl *"]).toBe("deny")
}) })

View file

@ -1,6 +1,7 @@
import { afterEach, describe, test, expect } from "bun:test" import { afterEach, describe, test, expect } from "bun:test"
import { Permission } from "../src/permission" import { Permission } from "../src/permission"
import { Config } from "@/config/config" import { Config } from "@/config/config"
import { ConfigPermission } from "@/config/permission"
import { Instance } from "../src/project/instance" import { Instance } from "../src/project/instance"
import { WithInstance } from "../src/project/with-instance" import { WithInstance } from "../src/project/with-instance"
import { disposeAllInstances, tmpdir } from "./fixture/fixture" import { disposeAllInstances, tmpdir } from "./fixture/fixture"
@ -163,7 +164,9 @@ describe("permission.task with real config files", () => {
directory: tmp.path, directory: tmp.path,
fn: async () => { fn: async () => {
const config = await load() 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 // general and orchestrator-fast should be allowed, code-reviewer denied
expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow") expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow")
expect(Permission.evaluate("task", "orchestrator-fast", 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, directory: tmp.path,
fn: async () => { fn: async () => {
const config = await load() 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 // general and code-reviewer should be ask, orchestrator-* denied
expect(Permission.evaluate("task", "general", ruleset).action).toBe("ask") expect(Permission.evaluate("task", "general", ruleset).action).toBe("ask")
expect(Permission.evaluate("task", "code-reviewer", 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, directory: tmp.path,
fn: async () => { fn: async () => {
const config = await load() 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", "general", ruleset).action).toBe("allow")
expect(Permission.evaluate("task", "code-reviewer", ruleset).action).toBe("deny") expect(Permission.evaluate("task", "code-reviewer", ruleset).action).toBe("deny")
// Unspecified agents default to "ask" // Unspecified agents default to "ask"
@ -240,7 +247,9 @@ describe("permission.task with real config files", () => {
directory: tmp.path, directory: tmp.path,
fn: async () => { fn: async () => {
const config = await load() 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 // Verify task permissions
expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow") expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow")
@ -278,7 +287,9 @@ describe("permission.task with real config files", () => {
directory: tmp.path, directory: tmp.path,
fn: async () => { fn: async () => {
const config = await load() 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 // Last matching rule wins - "*" deny is last, so all agents are denied
expect(Permission.evaluate("task", "general", ruleset).action).toBe("deny") expect(Permission.evaluate("task", "general", ruleset).action).toBe("deny")
@ -309,7 +320,9 @@ describe("permission.task with real config files", () => {
directory: tmp.path, directory: tmp.path,
fn: async () => { fn: async () => {
const config = await load() 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 // Evaluate uses findLast - "general" allow comes after "*" deny
expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow") expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow")