Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions packages/opencode/src/config/permission.ts
Original file line number Diff line number Diff line change
@@ -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"])
Expand All @@ -18,17 +19,9 @@ export const Rule = Schema.Union([Action, Object])
.pipe(withStatics((s) => ({ zod: zod(s) })))
export type Rule = Schema.Schema.Type<typeof Rule>

// 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),
Expand Down Expand Up @@ -60,6 +53,18 @@ const InputSchema = Schema.Union([Action, InputObject])
const normalizeInput = (input: Schema.Schema.Type<typeof InputSchema>): Schema.Schema.Type<typeof InputObject> =>
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),
Expand All @@ -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.
Expand Down
12 changes: 1 addition & 11 deletions packages/opencode/src/permission/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 6 additions & 16 deletions packages/opencode/test/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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",
"write",
"external_directory",
"read",
"todowrite",
// rest keys (not in the known list), in user's insertion order
"*",
"write",
"thoughts_*",
"reasoning_model_*",
"tools_*",
Expand Down
46 changes: 15 additions & 31 deletions packages/opencode/test/permission/next.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.

test("fromConfig - specific key beats wildcard regardless of JSON key order", () => {
// 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 - 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")
Expand Down Expand Up @@ -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" },
Expand Down
Loading