fix: merging permissions objects will discard ordering. instead, preserve permissions objects as layers, convert them individually to rulesets, and merge the rulesets (#16157)
This commit is contained in:
parent
3205f122eb
commit
809e1e5347
3 changed files with 146 additions and 6 deletions
|
|
@ -1,4 +1,4 @@
|
|||
import { Config } from "../config"
|
||||
import { Config, ConfigPermission } from "../config"
|
||||
import z from "zod"
|
||||
import { Provider } from "../provider"
|
||||
import { ModelID, ProviderID } from "../provider/schema"
|
||||
|
|
@ -103,7 +103,10 @@ export const layer = Layer.effect(
|
|||
},
|
||||
})
|
||||
|
||||
const user = Permission.fromConfig(cfg.permission ?? {})
|
||||
// Convert permission layers to rulesets and merge them
|
||||
// Each layer's rules come after the previous, so later configs override earlier ones
|
||||
const layers = (cfg.permission_layers ?? []) as ConfigPermission.Info[]
|
||||
const user = Permission.merge(...layers.map((p) => Permission.fromConfig(p)))
|
||||
|
||||
const agents: Record<string, Info> = {
|
||||
build: {
|
||||
|
|
|
|||
|
|
@ -51,6 +51,11 @@ function mergeConfigConcatArrays(target: Info, source: Info): Info {
|
|||
if (target.instructions && source.instructions) {
|
||||
merged.instructions = Array.from(new Set([...target.instructions, ...source.instructions]))
|
||||
}
|
||||
// Accumulate permission layers for later merging as rulesets
|
||||
// This preserves the ordering semantics: later rules override earlier rules
|
||||
if (source.permission) {
|
||||
merged.permission_layers = [...(target.permission_layers ?? []), source.permission]
|
||||
}
|
||||
return merged
|
||||
}
|
||||
|
||||
|
|
@ -190,6 +195,9 @@ export const InfoSchema = Schema.Struct({
|
|||
}),
|
||||
layout: Schema.optional(ConfigLayout.Layout).annotate({ description: "@deprecated Always uses stretch layout." }),
|
||||
permission: Schema.optional(PermissionRef),
|
||||
permission_layers: Schema.optional(Schema.Any.annotate({ [ZodOverride]: z.array(ConfigPermission.Info) })).annotate({
|
||||
description: "Internal: permission configs from each source for layered merging",
|
||||
}),
|
||||
tools: Schema.optional(Schema.Record(Schema.String, Schema.Boolean)),
|
||||
enterprise: Schema.optional(
|
||||
Schema.Struct({
|
||||
|
|
@ -653,11 +661,12 @@ export const layer = Layer.effect(
|
|||
}
|
||||
|
||||
if (Flag.OPENCODE_PERMISSION) {
|
||||
result.permission = mergeDeep(result.permission ?? {}, JSON.parse(Flag.OPENCODE_PERMISSION))
|
||||
const envPermission = JSON.parse(Flag.OPENCODE_PERMISSION) as ConfigPermission.Info
|
||||
result.permission_layers = [...(result.permission_layers ?? []), envPermission]
|
||||
}
|
||||
|
||||
if (result.tools) {
|
||||
const perms: Record<string, ConfigPermission.Action> = {}
|
||||
const perms: ConfigPermission.Info = {}
|
||||
for (const [tool, enabled] of Object.entries(result.tools)) {
|
||||
const action: ConfigPermission.Action = enabled ? "allow" : "deny"
|
||||
if (tool === "write" || tool === "edit" || tool === "patch") {
|
||||
|
|
@ -666,7 +675,8 @@ export const layer = Layer.effect(
|
|||
}
|
||||
perms[tool] = action
|
||||
}
|
||||
result.permission = mergeDeep(perms, result.permission ?? {})
|
||||
// Tools permissions come before other permissions (they can be overridden)
|
||||
result.permission_layers = [perms, ...(result.permission_layers ?? [])]
|
||||
}
|
||||
|
||||
if (!result.username) result.username = os.userInfo().username
|
||||
|
|
|
|||
|
|
@ -1,8 +1,9 @@
|
|||
import { test, expect, describe, mock, afterEach, beforeEach } from "bun:test"
|
||||
import { Effect, Layer, Option } from "effect"
|
||||
import { NodeFileSystem, NodePath } from "@effect/platform-node"
|
||||
import { Config, ConfigManaged } from "../../src/config"
|
||||
import { Config, ConfigManaged, ConfigPermission } from "../../src/config"
|
||||
import { ConfigParse } from "../../src/config/parse"
|
||||
import { Permission } from "../../src/permission"
|
||||
import { EffectFlock } from "@opencode-ai/shared/util/effect-flock"
|
||||
|
||||
import { Instance } from "../../src/project/instance"
|
||||
|
|
@ -1538,6 +1539,132 @@ test("permission config preserves 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 Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const config = await load()
|
||||
// Use permission_layers for correct ordering (each layer's rules come after previous)
|
||||
const layers = (config.permission_layers ?? []) as ConfigPermission.Info[]
|
||||
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 Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const config = await load()
|
||||
// Use permission_layers for correct ordering
|
||||
const layers = (config.permission_layers ?? []) as ConfigPermission.Info[]
|
||||
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 Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const config = await load()
|
||||
// Use permission_layers for correct ordering
|
||||
const layers = (config.permission_layers ?? []) as ConfigPermission.Info[]
|
||||
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")
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
// MCP config merging tests
|
||||
|
||||
test("project config can override MCP server enabled status", async () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue