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..2411952be1 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -1501,10 +1501,8 @@ test("permission config canonicalises known keys first, preserves rest-key inser // 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. + // Permission.fromConfig preserves this effective order, and permission + // evaluation uses last-match-wins precedence. await using tmp = await tmpdir({ init: async (dir) => { await Filesystem.write( diff --git a/packages/opencode/test/permission/next.test.ts b/packages/opencode/test/permission/next.test.ts index 372e1be7eb..f298b5cf2d 100644 --- a/packages/opencode/test/permission/next.test.ts +++ b/packages/opencode/test/permission/next.test.ts @@ -130,49 +130,45 @@ test("fromConfig - does not expand tilde in middle of path", () => { // 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. +// fromConfig preserves top-level key order. Combined with `findLast` in +// evaluate(), later matching rules win. // // 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. +// also depends on insertion order. -test("fromConfig - specific key beats wildcard regardless of JSON key order", () => { +test("fromConfig - top-level key order controls wildcard precedence", () => { const wildcardFirst = Permission.fromConfig({ "*": "deny", bash: "allow" }) const specificFirst = Permission.fromConfig({ bash: "allow", "*": "deny" }) - // Both orderings produce the same ruleset - expect(wildcardFirst).toEqual(specificFirst) - - // 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 - regression: trailing wildcard overrides earlier specific rule", () => { + const ruleset = Permission.fromConfig({ bash: "ask", "*": "allow" }) + expect(Permission.evaluate("bash", "glab", ruleset).action).toBe("allow") }) test("fromConfig - wildcard acts as fallback for permissions with no specific rule", () => { - const ruleset = Permission.fromConfig({ bash: "allow", "*": "ask" }) + 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 - preserves top-level ordering", () => { 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)", () => { +test("fromConfig - sub-pattern insertion order inside a tool key is preserved", () => { // 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. + // top-level order must not affect sub-pattern ordering. const ruleset = Permission.fromConfig({ bash: { "*": "deny", "git *": "allow" } }) expect(ruleset.map((r) => r.pattern)).toEqual(["*", "git *"]) // * fallback for unknown commands