diff --git a/packages/opencode/src/config/permission.ts b/packages/opencode/src/config/permission.ts index fdd5746837..909112c7c5 100644 --- a/packages/opencode/src/config/permission.ts +++ b/packages/opencode/src/config/permission.ts @@ -1,6 +1,7 @@ export * as ConfigPermission from "./permission" import { Schema, SchemaGetter } from "effect" -import { zod } from "@/util/effect-zod" +import z from "zod" +import { ZodOverride, zod } from "@/util/effect-zod" import { withStatics } from "@/util/schema" export const Action = Schema.Literals(["ask", "allow", "deny"]) @@ -18,17 +19,9 @@ export const Rule = Schema.Union([Action, Object]) .pipe(withStatics((s) => ({ zod: zod(s) }))) export type Rule = Schema.Schema.Type -// Known permission keys get explicit types — most are full Rule (either a -// single Action or a per-pattern object), but a handful of tools take no -// sub-target patterns and are Action-only. Unknown keys fall through the -// Record rest signature as Rule. -// -// StructWithRest canonicalises key order on decode (known first, then rest), -// which used to require the `__originalKeys` preprocess hack because -// `Permission.fromConfig` depended on the user's insertion order. That -// dependency is gone — `fromConfig` now sorts top-level keys so wildcard -// permissions come before specifics, making the final precedence -// order-independent. +// Known permission keys get explicit types in the Effect schema for generated +// docs/types. Runtime config parsing uses `InfoZod` below so user key order is +// preserved for permission precedence. const InputObject = Schema.StructWithRest( Schema.Struct({ read: Schema.optional(Rule), @@ -60,6 +53,18 @@ const InputSchema = Schema.Union([Action, InputObject]) const normalizeInput = (input: Schema.Schema.Type): Schema.Schema.Type => typeof input === "string" ? { "*": input } : input +const ACTION_ONLY = new Set(["todowrite", "question", "webfetch", "websearch", "codesearch", "doom_loop"]) + +const InfoZod = z + .union([zod(Action), z.record(z.string(), z.union([zod(Action), z.record(z.string(), zod(Action))]))]) + .transform(normalizeInput) + .superRefine((input, ctx) => { + for (const [key, value] of globalThis.Object.entries(input)) { + if (!ACTION_ONLY.has(key) || typeof value === "string") continue + ctx.addIssue({ code: "custom", message: `${key} must be a permission action`, path: [key] }) + } + }) + export const Info = InputSchema.pipe( Schema.decodeTo(InputObject, { decode: SchemaGetter.transform(normalizeInput), @@ -70,6 +75,7 @@ export const Info = InputSchema.pipe( }), ) .annotate({ identifier: "PermissionConfig" }) + .annotate({ [ZodOverride]: InfoZod }) .pipe( // Walker already emits the decodeTo transform into the derived zod (see // `encoded()` in effect-zod.ts), so just expose that directly. diff --git a/packages/opencode/src/permission/index.ts b/packages/opencode/src/permission/index.ts index 05c832016d..428514ecd6 100644 --- a/packages/opencode/src/permission/index.ts +++ b/packages/opencode/src/permission/index.ts @@ -288,18 +288,8 @@ function expand(pattern: string): string { } export function fromConfig(permission: ConfigPermission.Info) { - // Sort top-level keys so wildcard permissions (`*`, `mcp_*`) come before - // specific ones. Combined with `findLast` in evaluate(), this gives the - // intuitive semantic "specific tool rules override the `*` fallback" - // regardless of the user's JSON key order. Sub-pattern order inside a - // single permission key is preserved — only top-level keys are sorted. - const entries = Object.entries(permission).sort(([a], [b]) => { - const aWild = a.includes("*") - const bWild = b.includes("*") - return aWild === bWild ? 0 : aWild ? -1 : 1 - }) const ruleset: Ruleset = [] - for (const [key, value] of entries) { + for (const [key, value] of Object.entries(permission)) { if (typeof value === "string") { ruleset.push({ permission: key, action: value, pattern: "*" }) continue diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index 73dd46e319..361ac0b5df 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -1495,16 +1495,9 @@ test("merges legacy tools with existing permission config", async () => { }) }) -test("permission config canonicalises known keys first, preserves rest-key insertion order", async () => { - // ConfigPermission.Info is a StructWithRest schema — the decoder reorders - // keys into declaration-order for known permission names (edit, read, - // todowrite, external_directory are declared in `config/permission.ts`), - // followed by rest keys in the user's insertion order. - // - // Rule precedence is NOT affected by this reordering: `Permission.fromConfig` - // sorts wildcards before specifics before iterating. See the - // "fromConfig - specific key beats wildcard regardless of JSON key order" - // test in test/permission/next.test.ts for the behavioural guarantee. +test("permission config preserves user key order", async () => { + // Permission precedence follows the order users write in config, so parsing + // must not canonicalise known keys ahead of wildcard or custom keys. await using tmp = await tmpdir({ init: async (dir) => { await Filesystem.write( @@ -1532,15 +1525,12 @@ test("permission config canonicalises known keys first, preserves rest-key inser fn: async () => { const config = await load() expect(Object.keys(config.permission!)).toEqual([ - // known fields that the user provided, in declaration order from - // config/permission.ts (read, edit, ..., external_directory, todowrite) - "read", - "edit", - "external_directory", - "todowrite", - // rest keys (not in the known list), in user's insertion order "*", + "edit", "write", + "external_directory", + "read", + "todowrite", "thoughts_*", "reasoning_model_*", "tools_*", diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index 372e1be7eb..b58c716d8a 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -128,61 +128,45 @@ test("fromConfig - does not expand tilde in middle of path", () => { expect(result).toEqual([{ permission: "external_directory", pattern: "/some/~/path", action: "allow" }]) }) -// Top-level wildcard-vs-specific precedence semantics. -// -// fromConfig sorts top-level keys so wildcard permissions (containing "*") -// come before specific permissions. Combined with `findLast` in evaluate(), -// this gives the intuitive semantic "specific tool rules override the `*` -// fallback", regardless of the order the user wrote the keys in their JSON. -// -// Sub-pattern order inside a single permission key (e.g. `bash: { "*": "allow", "rm": "deny" }`) -// still depends on insertion order — only top-level keys are sorted. +// Permission precedence follows config insertion order. `evaluate()` uses the +// last matching rule, so later config entries intentionally override earlier +// entries even when a wildcard appears after a specific permission. -test("fromConfig - specific key beats wildcard regardless of JSON key order", () => { +test("fromConfig - preserves top-level config key order", () => { const wildcardFirst = Permission.fromConfig({ "*": "deny", bash: "allow" }) const specificFirst = Permission.fromConfig({ bash: "allow", "*": "deny" }) - // Both orderings produce the same ruleset - expect(wildcardFirst).toEqual(specificFirst) + expect(wildcardFirst.map((r) => r.permission)).toEqual(["*", "bash"]) + expect(specificFirst.map((r) => r.permission)).toEqual(["bash", "*"]) - // And both evaluate bash → allow (bash rule wins over * fallback) expect(Permission.evaluate("bash", "ls", wildcardFirst).action).toBe("allow") - expect(Permission.evaluate("bash", "ls", specificFirst).action).toBe("allow") + expect(Permission.evaluate("bash", "ls", specificFirst).action).toBe("deny") }) -test("fromConfig - wildcard acts as fallback for permissions with no specific rule", () => { - const ruleset = Permission.fromConfig({ bash: "allow", "*": "ask" }) +test("fromConfig - wildcard acts as fallback when it appears before specifics", () => { + const ruleset = Permission.fromConfig({ "*": "ask", bash: "allow" }) expect(Permission.evaluate("edit", "foo.ts", ruleset).action).toBe("ask") expect(Permission.evaluate("bash", "ls", ruleset).action).toBe("allow") }) -test("fromConfig - top-level ordering: wildcards first, specifics after", () => { +test("fromConfig - top-level ordering is not sorted by wildcard specificity", () => { const ruleset = Permission.fromConfig({ bash: "allow", "*": "ask", edit: "deny", "mcp_*": "allow", }) - // wildcards (* and mcp_*) come before specifics (bash, edit) - const permissions = ruleset.map((r) => r.permission) - expect(permissions.slice(0, 2).sort()).toEqual(["*", "mcp_*"]) - expect(permissions.slice(2)).toEqual(["bash", "edit"]) + expect(ruleset.map((r) => r.permission)).toEqual(["bash", "*", "edit", "mcp_*"]) }) -test("fromConfig - sub-pattern insertion order inside a tool key is preserved (only top-level sorts)", () => { - // Sub-patterns within a single tool key use the documented "`*` first, - // specific patterns after" convention (findLast picks specifics). The - // top-level sort must not touch sub-pattern ordering. +test("fromConfig - sub-pattern insertion order inside a tool key is preserved", () => { const ruleset = Permission.fromConfig({ bash: { "*": "deny", "git *": "allow" } }) expect(ruleset.map((r) => r.pattern)).toEqual(["*", "git *"]) - // * fallback for unknown commands expect(Permission.evaluate("bash", "rm foo", ruleset).action).toBe("deny") - // specific pattern wins for git commands (it's last, findLast picks it) expect(Permission.evaluate("bash", "git status", ruleset).action).toBe("allow") }) -test("fromConfig - canonical documented example unchanged", () => { - // Regression guard for the example in docs/permissions.mdx +test("fromConfig - documented fallback-first example", () => { const ruleset = Permission.fromConfig({ "*": "ask", bash: "allow", edit: "deny" }) expect(Permission.evaluate("bash", "ls", ruleset).action).toBe("allow") expect(Permission.evaluate("edit", "foo.ts", ruleset).action).toBe("deny") @@ -448,7 +432,7 @@ test("evaluate - wildcard permission fallback for unknown tool", () => { expect(result.action).toBe("ask") }) -test("evaluate - permission patterns sorted by length regardless of object order", () => { +test("evaluate - later wildcard permission can override earlier specific permission", () => { const result = Permission.evaluate("bash", "rm", [ { permission: "bash", pattern: "*", action: "allow" }, { permission: "*", pattern: "*", action: "deny" },