agaltsovav/opencode
1
0
Fork 0
Code Issues Pull Requests Actions 84 Packages Projects Releases Wiki Activity

temporarily revert: preserve permission ordering by accepting a layered array (#27258)

Browse Source
This commit is contained in:
Aiden Cline
2026-05-12 23:55:45 -05:00
committed by GitHub
parent 10b99b2f5a
commit 28204720dc
5 changed files with 18 additions and 224 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/opencode/test/config/config.test.ts
+7 -171
View File
@@ -3,9 +3,7 @@ import { Effect, Layer, Option } from "effect"
import { NodeFileSystem, NodePath } from "@effect/platform-node"
import { Config } from "@/config/config"
import { ConfigManaged } from "@/config/managed"
import { ConfigPermission } from "@/config/permission"
import { ConfigParse } from "../../src/config/parse"
import { Permission } from "../../src/permission"
import { EffectFlock } from "@opencode-ai/core/util/effect-flock"
import { Instance } from "../../src/project/instance"
@@ -278,40 +276,6 @@ 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) => {
@@ -1749,10 +1713,7 @@ test("permission config preserves user key order", async () => {
directory: tmp.path,
fn: async () => {
const config = await load()
// 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([
expect(Object.keys(config.permission!)).toEqual([
"*",
"edit",
"write",
@@ -1768,129 +1729,6 @@ test("permission config preserves user key order", async () => {
})
})
// Global bash "rm *" deny is inherited, but user's top-level "*" ask comes after and overrides it
test("user top-level catchall overrides inherited bash rules", 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",
permission: {
bash: { "rm *": "deny" },
},
}),
)
const opencodeDir = path.join(dir, ".opencode")
await fs.mkdir(opencodeDir, { recursive: true })
await Filesystem.write(
path.join(opencodeDir, "opencode.json"),
JSON.stringify({
$schema: "https://opencode.ai/config.json",
permission: {
"*": "ask",
bash: { "ls *": "allow" },
},
}),
)
},
})
await WithInstance.provide({
directory: tmp.path,
fn: async () => {
const config = await load()
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")
expect(Permission.evaluate("bash", "ls -la", ruleset).action).toBe("allow")
expect(Permission.evaluate("bash", "echo hello", ruleset).action).toBe("ask")
},
})
})
// No top-level catchall, so global bash "rm *" deny is preserved
test("inherited bash rules apply when no user top-level catchall", 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",
permission: {
bash: { "rm *": "deny" },
},
}),
)
const opencodeDir = path.join(dir, ".opencode")
await fs.mkdir(opencodeDir, { recursive: true })
await Filesystem.write(
path.join(opencodeDir, "opencode.json"),
JSON.stringify({
$schema: "https://opencode.ai/config.json",
permission: {
bash: { "ls *": "allow" },
},
}),
)
},
})
await WithInstance.provide({
directory: tmp.path,
fn: async () => {
const config = await load()
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")
expect(Permission.evaluate("bash", "ls -la", ruleset).action).toBe("allow")
},
})
})
// User's bash "*" catchall overrides global "rm *" deny
test("user bash catchall overrides inherited bash rules", 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",
permission: {
bash: { "rm *": "deny" },
},
}),
)
const opencodeDir = path.join(dir, ".opencode")
await fs.mkdir(opencodeDir, { recursive: true })
await Filesystem.write(
path.join(opencodeDir, "opencode.json"),
JSON.stringify({
$schema: "https://opencode.ai/config.json",
permission: {
bash: { "*": "ask", "ls *": "allow" },
},
}),
)
},
})
await WithInstance.provide({
directory: tmp.path,
fn: async () => {
const config = await load()
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")
expect(Permission.evaluate("bash", "ls -la", ruleset).action).toBe("allow")
expect(Permission.evaluate("bash", "echo hello", ruleset).action).toBe("ask")
// Non-bash permissions should use the top-level "*" rule
expect(Permission.evaluate("read", "foo.txt", ruleset).action).toBe("ask")
},
})
})
test("config parser preserves permission order while rejecting unknown top-level keys", () => {
const config = ConfigParse.schema(
Config.Info,
@@ -1904,8 +1742,7 @@ test("config parser preserves permission order while rejecting unknown top-level
"test",
)
// ConfigParse.schema preserves the raw shape the user wrote
expect(Object.keys(config.permission as ConfigPermission.Info)).toEqual(["bash", "*", "edit"])
expect(Object.keys(config.permission!)).toEqual(["bash", "*", "edit"])
try {
ConfigParse.schema(Config.Info, { invalid_field: true }, "test")
throw new Error("expected config parse to fail")
@@ -2742,12 +2579,11 @@ test("parseManagedPlist parses permission rules", async () => {
),
"test:mobileconfig",
)
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<string, string>
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<string, string>
expect(bash?.["rm -rf *"]).toBe("deny")
expect(bash?.["curl *"]).toBe("deny")
})
packages/opencode/test/permission-task.test.ts
+6 -19
View File
@@ -1,7 +1,6 @@
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"
@@ -164,9 +163,7 @@ describe("permission.task with real config files", () => {
directory: tmp.path,
fn: async () => {
const config = await load()
const ruleset = Permission.merge(
...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)),
)
const ruleset = Permission.fromConfig(config.permission ?? {})
// 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")
@@ -191,9 +188,7 @@ describe("permission.task with real config files", () => {
directory: tmp.path,
fn: async () => {
const config = await load()
const ruleset = Permission.merge(
...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)),
)
const ruleset = Permission.fromConfig(config.permission ?? {})
// 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")
@@ -218,9 +213,7 @@ describe("permission.task with real config files", () => {
directory: tmp.path,
fn: async () => {
const config = await load()
const ruleset = Permission.merge(
...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)),
)
const ruleset = Permission.fromConfig(config.permission ?? {})
expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow")
expect(Permission.evaluate("task", "code-reviewer", ruleset).action).toBe("deny")
// Unspecified agents default to "ask"
@@ -247,9 +240,7 @@ describe("permission.task with real config files", () => {
directory: tmp.path,
fn: async () => {
const config = await load()
const ruleset = Permission.merge(
...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)),
)
const ruleset = Permission.fromConfig(config.permission ?? {})
// Verify task permissions
expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow")
@@ -287,9 +278,7 @@ describe("permission.task with real config files", () => {
directory: tmp.path,
fn: async () => {
const config = await load()
const ruleset = Permission.merge(
...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)),
)
const ruleset = Permission.fromConfig(config.permission ?? {})
// Last matching rule wins - "*" deny is last, so all agents are denied
expect(Permission.evaluate("task", "general", ruleset).action).toBe("deny")
@@ -320,9 +309,7 @@ describe("permission.task with real config files", () => {
directory: tmp.path,
fn: async () => {
const config = await load()
const ruleset = Permission.merge(
...ConfigPermission.toLayers(config.permission).map((p) => Permission.fromConfig(p)),
)
const ruleset = Permission.fromConfig(config.permission ?? {})
// Evaluate uses findLast - "general" allow comes after "*" deny
expect(Permission.evaluate("task", "general", ruleset).action).toBe("allow")