diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index ead3a0149b4..7881dcc7bf9 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -600,7 +600,8 @@ export namespace Config { // Convert legacy maxSteps to steps const steps = agent.steps ?? agent.maxSteps - return { ...agent, options, permission, steps } as typeof agent & { + const { tools, maxSteps, ...rest } = agent + return { ...rest, options, permission, steps } as typeof agent & { options?: Record permission?: Permission steps?: number diff --git a/packages/opencode/src/session/index.ts b/packages/opencode/src/session/index.ts index 0776590d6a9..67b186bf66e 100644 --- a/packages/opencode/src/session/index.ts +++ b/packages/opencode/src/session/index.ts @@ -185,6 +185,21 @@ export namespace Session { directory: string permission?: PermissionNext.Ruleset }) { + // Inherit permissions from parent session if parentID is provided + let mergedPermissions = input.permission + if (input.parentID) { + try { + const parentSession = await Storage.read(["session", Instance.project.id, input.parentID]) + if (parentSession?.permission) { + const parentPermissions = parentSession.permission + const childPermissions = input.permission ?? [] + mergedPermissions = PermissionNext.merge(parentPermissions, childPermissions) + } + } catch (error) { + log.warn("Failed to load parent session for permission inheritance", { parentID: input.parentID, error }) + } + } + const result: Info = { id: Identifier.descending("session", input.id), version: Installation.VERSION, @@ -192,7 +207,7 @@ export namespace Session { directory: input.directory, parentID: input.parentID, title: input.title ?? createDefaultTitle(!!input.parentID), - permission: input.permission, + permission: mergedPermissions, time: { created: Date.now(), updated: Date.now(), diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index f891612272c..92392a438a3 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -165,10 +165,11 @@ export namespace SessionPrompt { }) } if (permissions.length > 0) { - session.permission = permissions - await Session.update(session.id, (draft) => { - draft.permission = permissions + const updatedSession = await Session.update(session.id, (draft) => { + draft.permission = PermissionNext.merge(draft.permission ?? [], permissions) }) + // Update session object to reflect the changes + session.permission = updatedSession.permission } if (input.noReply === true) { diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index eb76681ded4..d76116a4919 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -25,6 +25,8 @@ import { Flag } from "@/flag/flag" import { Log } from "@/util/log" import { LspTool } from "./lsp" import { Truncate } from "./truncation" +import { PermissionNext } from "@/permission/next" +import { Wildcard } from "@/util/wildcard" export namespace ToolRegistry { const log = Log.create({ service: "tool.registry" }) @@ -119,13 +121,53 @@ export namespace ToolRegistry { export async function tools(providerID: string, agent?: Agent.Info) { const tools = await all() + const config = await Config.get() + + // Build permission ruleset: global config + agent permissions (agent takes precedence) + let permissionRuleset: PermissionNext.Ruleset = [] + if (config.permission) { + // Global config permissions come first + permissionRuleset = PermissionNext.merge(permissionRuleset, PermissionNext.fromConfig(config.permission)) + } + if (agent?.permission) { + // Agent permissions come last (they win on last-match-wins evaluation) + permissionRuleset = PermissionNext.merge(permissionRuleset, agent.permission) + } + + // Filter out disabled tools based on permissions + // Only filter tools if there are no allow rules for that permission type const result = await Promise.all( tools .filter((t) => { + // Check if tool has an explicit deny rule with no allow rules + const permission = t.id + + // Find all rules for this permission + const permissionRules = permissionRuleset.filter((r) => Wildcard.match(permission, r.permission)) + + // If no rules apply, allow the tool + if (permissionRules.length === 0) { + return true + } + + // If there are allow rules, allow the tool (command-level filtering happens later) + const hasAllowRule = permissionRules.some((r) => r.action === "allow") + if (hasAllowRule) { + return true + } + + // If there are only deny rules and one matches "*" pattern, filter out the tool + const hasWildcardDeny = permissionRules.some((r) => r.action === "deny" && r.pattern === "*") + + if (hasWildcardDeny) { + return false + } + // Enable websearch/codesearch for zen users OR via enable flag if (t.id === "codesearch" || t.id === "websearch") { return providerID === "opencode" || Flag.OPENCODE_ENABLE_EXA } + return true }) .map(async (t) => { diff --git a/packages/opencode/src/tool/task.ts b/packages/opencode/src/tool/task.ts index 53b501ba91a..964c52e6f91 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -7,7 +7,7 @@ import { MessageV2 } from "../session/message-v2" import { Identifier } from "../id/id" import { Agent } from "../agent/agent" import { SessionPrompt } from "../session/prompt" -import { iife } from "@/util/iife" + import { defer } from "@/util/defer" import { Config } from "../config/config" import { PermissionNext } from "@/permission/next" @@ -54,41 +54,50 @@ export const TaskTool = Tool.define("task", async (ctx) => { }) } + // Find the agent by subagent_type const agent = await Agent.get(params.subagent_type) - if (!agent) throw new Error(`Unknown agent type: ${params.subagent_type} is not a valid agent type`) - const session = await iife(async () => { - if (params.session_id) { - const found = await Session.get(params.session_id).catch(() => {}) - if (found) return found - } - return await Session.create({ - parentID: ctx.sessionID, - title: params.description + ` (@${agent.name} subagent)`, - permission: [ - { - permission: "todowrite", - pattern: "*", - action: "deny", - }, - { - permission: "todoread", - pattern: "*", - action: "deny", - }, - { - permission: "task", - pattern: "*", - action: "deny", - }, - ...(config.experimental?.primary_tools?.map((t) => ({ - pattern: "*", - action: "allow" as const, - permission: t, - })) ?? []), - ], - }) + // Create session for the subagent task with proper security restrictions + // IMPORTANT: Task restrictions must come AFTER agent permissions to override "*": "allow" + const taskPermissions = PermissionNext.merge( + agent.permission ?? [], + [ + { + permission: "task", + pattern: "*", + action: "deny", + }, + { + permission: "todowrite", + pattern: "*", + action: "deny", + }, + { + permission: "todoread", + pattern: "*", + action: "deny", + }, + ], + // Git-agent restrictions must be LAST to override agent defaults + [ + { + permission: "git", + pattern: "*", + action: "deny", + }, + ...(config.experimental?.primary_tools?.map((t) => ({ + pattern: "*", + action: "allow" as const, + permission: t, + })) ?? []), + ], + ) + + const session = await Session.create({ + parentID: ctx.sessionID, + permission: taskPermissions, }) + const msg = await MessageV2.get({ sessionID: ctx.sessionID, messageID: ctx.messageID }) if (msg.info.role !== "assistant") throw new Error("Not an assistant message") @@ -143,12 +152,6 @@ export const TaskTool = Tool.define("task", async (ctx) => { providerID: model.providerID, }, agent: agent.name, - tools: { - todowrite: false, - todoread: false, - task: false, - ...Object.fromEntries((config.experimental?.primary_tools ?? []).map((t) => [t, false])), - }, parts: promptParts, }) unsub() @@ -179,3 +182,10 @@ export const TaskTool = Tool.define("task", async (ctx) => { }, } }) + +export function filterSubagents(agents: Agent.Info[], ruleset: PermissionNext.Ruleset): Agent.Info[] { + return agents.filter((agent) => { + const permission = PermissionNext.evaluate("task", agent.name, ruleset) + return permission.action !== "deny" + }) +} diff --git a/packages/opencode/test/session/permission-inheritance.test.ts b/packages/opencode/test/session/permission-inheritance.test.ts new file mode 100644 index 00000000000..062563bb4c2 --- /dev/null +++ b/packages/opencode/test/session/permission-inheritance.test.ts @@ -0,0 +1,396 @@ +import { describe, expect, test } from "bun:test" +import path from "path" +import { SessionPrompt } from "../../src/session/prompt" +import { Session } from "../../src/session" +import { MessageV2 } from "../../src/session/message-v2" +import { Instance } from "../../src/project/instance" +import { Agent } from "../../src/agent/agent" +import { PermissionNext } from "../../src/permission/next" +import { Config } from "../../src/config/config" +import { Identifier } from "../../src/id/id" + +const projectRoot = path.join(__dirname, "../..") + +describe("Session permission inheritance and merging", () => { + const createMockSession = async (parentID?: string, permissions?: PermissionNext.Ruleset) => { + return await Session.create({ + parentID, + title: "Test session", + permission: permissions ?? [], + }) + } + + const createMockAgent = async (permissions?: PermissionNext.Ruleset) => { + const agent = await Agent.get("developer") + return { + ...agent, + permission: permissions ?? [], + } as Agent.Info + } + + test("subagent inherits parent's session permissions by default", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const parentSession = await createMockSession(undefined, [ + { permission: "bash", pattern: "*", action: "allow" }, + { permission: "edit", pattern: "*", action: "deny" }, + ]) + + const agent = await createMockAgent([]) + const childSession = await createMockSession(parentSession.id, [ + { permission: "read", pattern: "*", action: "allow" }, + ]) + + // Child session should have parent permissions preserved + expect(childSession.permission).toContainEqual({ permission: "bash", pattern: "*", action: "allow" }) + expect(childSession.permission).toContainEqual({ permission: "edit", pattern: "*", action: "deny" }) + expect(childSession.permission).toContainEqual({ permission: "read", pattern: "*", action: "allow" }) + }, + }) + }) + + test("SessionPrompt.prompt() merges tools with existing permissions", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const session = await createMockSession(undefined, [ + { permission: "bash", pattern: "*", action: "allow" }, + { permission: "edit", pattern: "*", action: "deny" }, + ]) + + const agent = await createMockAgent([]) + + // Prompt with additional tools should MERGE, not REPLACE + await SessionPrompt.prompt({ + sessionID: session.id, + agent: agent.name, + parts: [{ type: "text", text: "Test prompt" }], + tools: { + read: true, + write: false, + }, + }) + + const updatedSession = await Session.get(session.id) + + // Original permissions should be preserved + expect(updatedSession.permission).toContainEqual({ permission: "bash", pattern: "*", action: "allow" }) + expect(updatedSession.permission).toContainEqual({ permission: "edit", pattern: "*", action: "deny" }) + + // New tool permissions should be added + expect(updatedSession.permission).toContainEqual({ permission: "read", pattern: "*", action: "allow" }) + expect(updatedSession.permission).toContainEqual({ permission: "write", pattern: "*", action: "deny" }) + }, + }) + }) + + test("SessionPrompt.prompt() with empty tools preserves existing permissions", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const session = await createMockSession(undefined, [ + { permission: "bash", pattern: "*", action: "allow" }, + { permission: "edit", pattern: "/tmp/*", action: "allow" }, + ]) + + const agent = await createMockAgent([]) + + // Prompt with empty tools should not affect permissions + await SessionPrompt.prompt({ + sessionID: session.id, + agent: agent.name, + parts: [{ type: "text", text: "Test prompt" }], + tools: {}, + }) + + const updatedSession = await Session.get(session.id) + + // All original permissions should be preserved + expect(updatedSession.permission).toEqual([ + { permission: "bash", pattern: "*", action: "allow" }, + { permission: "edit", pattern: "/tmp/*", action: "allow" }, + ]) + }, + }) + }) + + test("SessionPrompt.prompt() without tools preserves existing permissions", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const session = await createMockSession(undefined, [ + { permission: "bash", pattern: "git*", action: "allow" }, + { permission: "bash", pattern: "*", action: "deny" }, + ]) + + const agent = await createMockAgent([]) + + // Prompt without tools parameter should not affect permissions + await SessionPrompt.prompt({ + sessionID: session.id, + agent: agent.name, + parts: [{ type: "text", text: "Test prompt" }], + }) + + const updatedSession = await Session.get(session.id) + + // All original permissions should be preserved + expect(updatedSession.permission).toEqual([ + { permission: "bash", pattern: "git*", action: "allow" }, + { permission: "bash", pattern: "*", action: "deny" }, + ]) + }, + }) + }) + + test("Task tool respects configured agent permission blocks", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const parentSession = await createMockSession(undefined, [ + { permission: "task", pattern: "forbidden-agent", action: "deny" }, + { permission: "task", pattern: "allowed-agent", action: "allow" }, + ]) + + // Test that task tool filtering respects session permissions + const ruleset = PermissionNext.evaluate("task", "forbidden-agent", parentSession.permission!) + expect(ruleset.action).toBe("deny") + + const allowedRuleset = PermissionNext.evaluate("task", "allowed-agent", parentSession.permission!) + expect(allowedRuleset.action).toBe("allow") + }, + }) + }) + + test("Complex inheritance scenario: parent + agent + tools", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const parentSession = await createMockSession(undefined, [ + { permission: "bash", pattern: "*", action: "deny" }, + { permission: "read", pattern: "*.md", action: "allow" }, + ]) + + const agent = await createMockAgent([ + { permission: "read", pattern: "*", action: "allow" }, + ]) + + // Create child session with inherited permissions + const childSession = await createMockSession(parentSession.id, [ + { permission: "write", pattern: "/tmp/*", action: "allow" }, + ]) + + // Prompt with additional tools should merge all permissions + await SessionPrompt.prompt({ + sessionID: childSession.id, + agent: agent.name, + parts: [{ type: "text", text: "Test prompt" }], + tools: { + git: true, // Should be allowed + deploy: false, // Should be denied + }, + }) + + const finalSession = await Session.get(childSession.id) + + // Parent permissions should be inherited + expect(finalSession.permission).toContainEqual({ permission: "bash", pattern: "*", action: "deny" }) + expect(finalSession.permission).toContainEqual({ permission: "read", pattern: "*.md", action: "allow" }) + + // Child session permissions should be preserved + expect(finalSession.permission).toContainEqual({ permission: "write", pattern: "/tmp/*", action: "allow" }) + + // Agent permissions should be merged + expect(finalSession.permission).toContainEqual({ permission: "read", pattern: "*", action: "allow" }) + + // Tool permissions from prompt should be added + expect(finalSession.permission).toContainEqual({ permission: "git", pattern: "*", action: "allow" }) + expect(finalSession.permission).toContainEqual({ permission: "deploy", pattern: "*", action: "deny" }) + }, + }) + }) + + test("Permission inheritance works with wildcard patterns", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const parentSession = await createMockSession(undefined, [ + { permission: "*", pattern: "*", action: "deny" }, + { permission: "git", pattern: "*", action: "allow" }, + { permission: "read", pattern: "*.txt", action: "allow" }, + ]) + + // Create child session with specific allow overrides + const childSession = await createMockSession(parentSession.id, [ + { permission: "bash", pattern: "*", action: "allow" }, + ]) + + // Check permission evaluation follows inheritance rules + expect(PermissionNext.evaluate("bash", "*", childSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("git", "*", childSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("read", "*.txt", childSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("write", "*", childSession.permission!).action).toBe("deny") + + // Prompt should not break inherited permission structure + await SessionPrompt.prompt({ + sessionID: childSession.id, + agent: "developer", + parts: [{ type: "text", text: "Test" }], + tools: { deploy: true }, + }) + + const updatedSession = await Session.get(childSession.id) + + // Inherited permissions should still work + expect(PermissionNext.evaluate("bash", "*", updatedSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("git", "*", updatedSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("deploy", "*", updatedSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("write", "*", updatedSession.permission!).action).toBe("deny") + }, + }) + }) + + test("Nested subagent permissions cascade correctly", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + // Create parent -> child -> grandchild hierarchy + const parentSession = await createMockSession(undefined, [ + { permission: "bash", pattern: "*", action: "allow" }, + ]) + + const childSession = await createMockSession(parentSession.id, [ + { permission: "edit", pattern: "*", action: "allow" }, + ]) + + const grandchildSession = await createMockSession(childSession.id, [ + { permission: "read", pattern: "*", action: "allow" }, + ]) + + // Grandchild should inherit all parent permissions + expect(grandchildSession.permission).toContainEqual({ permission: "bash", pattern: "*", action: "allow" }) + expect(grandchildSession.permission).toContainEqual({ permission: "edit", pattern: "*", action: "allow" }) + expect(grandchildSession.permission).toContainEqual({ permission: "read", pattern: "*", action: "allow" }) + + // Prompt on grandchild should preserve all inherited permissions + await SessionPrompt.prompt({ + sessionID: grandchildSession.id, + agent: "developer", + parts: [{ type: "text", text: "Test" }], + tools: { deploy: true }, + }) + + const finalSession = await Session.get(grandchildSession.id) + + // All permissions should be preserved + expect(PermissionNext.evaluate("bash", "*", finalSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("edit", "*", finalSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("read", "*", finalSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("deploy", "*", finalSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("write", "*", finalSession.permission!).action).toBe("deny") // Default deny + }, + }) + }) +}) + +describe("Permission integration with real session workflows", () => { + test("Task tool creates session with proper permission inheritance", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const config = await Config.get() + const agents = await Agent.list().then((x) => x.filter((a) => a.mode !== "primary")) + const agent = agents.find((a) => a.name === "developer") ?? agents[0] + + // Create parent session with some permissions + const parentSession = await Session.create({ + title: "Parent session", + permission: [ + { permission: "bash", pattern: "*", action: "allow" }, + { permission: "edit", pattern: "*", action: "deny" }, + ], + }) + + // Task tool should create child session that inherits and merges permissions + const taskSession = await Session.create({ + parentID: parentSession.id, + title: "Task session", + permission: PermissionNext.merge( + agent.permission ?? [], + [ + { + permission: "todowrite", + pattern: "*", + action: "deny", + }, + { + permission: "todoread", + pattern: "*", + action: "deny", + }, + { + permission: "task", + pattern: "*", + action: "deny", + }, + ...(config.experimental?.primary_tools?.map((t) => ({ + pattern: "*", + action: "allow" as const, + permission: t, + })) ?? []), + ], + ), + }) + + // Should inherit from parent but also add task-specific restrictions + expect(taskSession.permission).toContainEqual({ permission: "bash", pattern: "*", action: "allow" }) + expect(taskSession.permission).toContainEqual({ permission: "edit", pattern: "*", action: "deny" }) + expect(taskSession.permission).toContainEqual({ permission: "task", pattern: "*", action: "deny" }) + }, + }) + }) + + test("Permission evaluation respects inheritance order (last match wins)", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const parentSession = await Session.create({ + title: "Parent session", + permission: [ + { permission: "bash", pattern: "*", action: "deny" }, + { permission: "bash", pattern: "git*", action: "allow" }, + ], + }) + + const childSession = await Session.create({ + parentID: parentSession.id, + title: "Child session", + permission: [ + { permission: "bash", pattern: "*", action: "allow" }, // Should override parent's deny + { permission: "bash", pattern: "rm*", action: "deny" }, + ], + }) + + // Child's wildcard allow should override parent's wildcard deny + expect(PermissionNext.evaluate("bash", "git-status", childSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("bash", "ls", childSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("bash", "rm-file", childSession.permission!).action).toBe("deny") + + // SessionPrompt should maintain this order + await SessionPrompt.prompt({ + sessionID: childSession.id, + agent: "developer", + parts: [{ type: "text", text: "Test" }], + tools: { deploy: true }, + }) + + const updatedSession = await Session.get(childSession.id) + expect(PermissionNext.evaluate("bash", "git-status", updatedSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("bash", "ls", updatedSession.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("bash", "rm-file", updatedSession.permission!).action).toBe("deny") + expect(PermissionNext.evaluate("deploy", "*", updatedSession.permission!).action).toBe("allow") + }, + }) + }) +}) \ No newline at end of file diff --git a/packages/opencode/test/session/permission-security-fix.test.ts b/packages/opencode/test/session/permission-security-fix.test.ts new file mode 100644 index 00000000000..e601a1353ba --- /dev/null +++ b/packages/opencode/test/session/permission-security-fix.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, test } from "bun:test" +import path from "path" +import { Session } from "../../src/session" +import { PermissionNext } from "../../src/permission/next" +import { Instance } from "../../src/project/instance" + +const projectRoot = path.join(__dirname, "../..") + +describe("Permission security fix validation", () => { + test("Task restrictions override agent wildcard allow permissions", () => { + // Simulate the permission conflict: agent has "*": "allow", task adds restrictions + const agentPermissions: PermissionNext.Ruleset = [ + { permission: "*", pattern: "*", action: "allow" }, + { permission: "bash", pattern: "*", action: "allow" }, + ] + + const taskRestrictions: PermissionNext.Ruleset = [ + { permission: "task", pattern: "*", action: "deny" }, + { permission: "git", pattern: "*", action: "deny" }, + ] + + // Merge order matters: agent first, then restrictions (last wins) + const merged = PermissionNext.merge(agentPermissions, taskRestrictions) + + // Git should be denied because restriction comes after agent's wildcard allow + expect(PermissionNext.evaluate("git", "*", merged).action).toBe("deny") + expect(PermissionNext.evaluate("bash", "*", merged).action).toBe("allow") + expect(PermissionNext.evaluate("write", "*", merged).action).toBe("allow") // from wildcard + expect(PermissionNext.evaluate("task", "*", merged).action).toBe("deny") + }) + + test("Session creation inherits parent permissions correctly", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + // Create parent with git restrictions + const parent = await Session.create({ + title: "Parent", + permission: [ + { permission: "git", pattern: "*", action: "deny" }, + { permission: "bash", pattern: "*", action: "allow" }, + ], + }) + + // Create child that adds more permissions + const child = await Session.create({ + parentID: parent.id, + permission: [{ permission: "read", pattern: "*", action: "allow" }], + }) + + // Child should inherit parent's git restrictions + expect(PermissionNext.evaluate("git", "*", child.permission!).action).toBe("deny") + expect(PermissionNext.evaluate("bash", "*", child.permission!).action).toBe("allow") + expect(PermissionNext.evaluate("read", "*", child.permission!).action).toBe("allow") + }, + }) + }) + + test("Permission merge order is critical for security", () => { + const parentPermissions: PermissionNext.Ruleset = [{ permission: "git", pattern: "*", action: "deny" }] + + const agentDefaults: PermissionNext.Ruleset = [ + { permission: "*", pattern: "*", action: "allow" }, // Overriding default + ] + + const taskRestrictions: PermissionNext.Ruleset = [ + { permission: "git", pattern: "*", action: "deny" }, // Must be last + ] + + // Wrong order demonstration (without task restrictions) + const wrongWithoutRestrictions = PermissionNext.merge(parentPermissions, agentDefaults) + expect(PermissionNext.evaluate("git", "*", wrongWithoutRestrictions).action).toBe("allow") // SECURITY ISSUE + + // Correct order with restrictions (task restrictions override agent defaults) + const correctMerge = PermissionNext.merge( + parentPermissions, + agentDefaults, + taskRestrictions, // Must come last to override agent "*": "allow" + ) + expect(PermissionNext.evaluate("git", "*", correctMerge).action).toBe("deny") // SECURE + }) + + test("Security fix prevents git permission bypass in task sessions", () => { + // Simulate actual task permission creation from the fix + const agentPermissions: PermissionNext.Ruleset = [ + { permission: "*", pattern: "*", action: "allow" }, // Agent defaults + ] + + // This simulates the fixed task.ts permission creation + const taskPermissions = PermissionNext.merge( + agentPermissions, + [ + { permission: "task", pattern: "*", action: "deny" }, + { permission: "todowrite", pattern: "*", action: "deny" }, + { permission: "todoread", pattern: "*", action: "deny" }, + ], + [ + { permission: "git", pattern: "*", action: "deny" }, // Must be last + ], + ) + + // All sensitive operations should be denied + expect(PermissionNext.evaluate("git", "*", taskPermissions).action).toBe("deny") + expect(PermissionNext.evaluate("task", "*", taskPermissions).action).toBe("deny") + expect(PermissionNext.evaluate("todowrite", "*", taskPermissions).action).toBe("deny") + expect(PermissionNext.evaluate("todoread", "*", taskPermissions).action).toBe("deny") + + // Non-restricted operations should still work + expect(PermissionNext.evaluate("read", "*", taskPermissions).action).toBe("allow") + expect(PermissionNext.evaluate("bash", "*", taskPermissions).action).toBe("allow") + }) +}) diff --git a/packages/opencode/test/tool-registry-permissions.test.ts b/packages/opencode/test/tool-registry-permissions.test.ts new file mode 100644 index 00000000000..87766bedb46 --- /dev/null +++ b/packages/opencode/test/tool-registry-permissions.test.ts @@ -0,0 +1,117 @@ +import { describe, it, expect } from "bun:test" +import { ToolRegistry } from "../src/tool/registry" +import { Config } from "../src/config/config" +import { PermissionNext } from "../src/permission/next" + +describe("ToolRegistry permission filtering", () => { + it("filters tools based on global permissions", async () => { + // Mock config with permission rules + const mockConfig = { + permission: { + bash: { + "gh*": "allow", + "git log*": "allow", + "git add*": "deny", + "*": "deny", + }, + }, + } as any + + // Spy on Config.get + const originalGet = Config.get + Config.get = () => Promise.resolve(mockConfig) + + try { + // Get tools for git-agent + const mockAgent = { + name: "git-agent", + permission: [], // No agent-specific permissions + } as any + + const tools = await ToolRegistry.tools("opencode", mockAgent) + + // All tools should be filtered out since bash permissions are denied globally + const toolIds = tools.map((t) => t.id) + expect(toolIds).not.toContain("bash") + expect(toolIds).toContain("read") // read should be allowed + expect(toolIds).toContain("glob") // glob should be allowed + expect(toolIds).toContain("grep") // grep should be allowed + } finally { + // Restore original Config.get + Config.get = originalGet + } + }) + + it("respects agent permissions over global permissions", async () => { + // Mock config with global deny for bash + const mockConfig = { + permission: { + bash: { + "*": "deny", + }, + }, + } as any + + // Spy on Config.get + const originalGet = Config.get + Config.get = () => Promise.resolve(mockConfig) + + try { + // Get tools for git-agent with specific bash permissions + const mockAgent = { + name: "git-agent", + permission: [ + { permission: "bash", pattern: "gh*", action: "allow" }, + { permission: "bash", pattern: "git log*", action: "allow" }, + ], + } as any + + const tools = await ToolRegistry.tools("opencode", mockAgent) + + // Bash should be allowed due to agent permissions + const toolIds = tools.map((t) => t.id) + expect(toolIds).toContain("bash") + } finally { + // Restore original Config.get + Config.get = originalGet + } + }) + + it("preserves non-privileged tools", async () => { + // Mock config with restrictive permissions + const mockConfig = { + permission: { + bash: { "*": "deny" }, + edit: { "*": "deny" }, + write: { "*": "deny" }, + }, + } as any + + // Spy on Config.get + const originalGet = Config.get + Config.get = () => Promise.resolve(mockConfig) + + try { + // Get tools for any agent + const tools = await ToolRegistry.tools("opencode") + + const toolIds = tools.map((t) => t.id) + + // Non-privileged tools should always be available + expect(toolIds).toContain("read") + expect(toolIds).toContain("glob") + expect(toolIds).toContain("grep") + expect(toolIds).toContain("webfetch") + expect(toolIds).toContain("skill") + expect(toolIds).toContain("task") + + // Privileged tools should be filtered out + expect(toolIds).not.toContain("bash") + expect(toolIds).not.toContain("edit") + expect(toolIds).not.toContain("write") + } finally { + // Restore original Config.get + Config.get = originalGet + } + }) +}) diff --git a/packages/opencode/test/tool/registry.test.ts b/packages/opencode/test/tool/registry.test.ts new file mode 100644 index 00000000000..07d7ad36660 --- /dev/null +++ b/packages/opencode/test/tool/registry.test.ts @@ -0,0 +1,230 @@ +import { describe, expect, test } from "bun:test" +import path from "path" +import { ToolRegistry } from "../../src/tool/registry" +import { Instance } from "../../src/project/instance" + +const projectRoot = path.join(__dirname, "../..") + +describe("ToolRegistry.tools() permission filtering", () => { + test("filters out tool with all-deny rules", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const agent = { + name: "test-agent", + mode: "subagent" as const, + permission: [ + { permission: "bash", pattern: "*", action: "deny" as const } + ], + options: {} + } + + const result = await ToolRegistry.tools("anthropic", agent as any) + const toolIds = result.map(t => t.id) + + expect(toolIds).not.toContain("bash") + }, + }) + }) + + test("includes tool with mixed allow/deny rules", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const agent = { + name: "test-agent", + mode: "subagent" as const, + permission: [ + { permission: "bash", pattern: "git*", action: "allow" as const }, + { permission: "bash", pattern: "*", action: "deny" as const } + ], + options: {} + } + + const result = await ToolRegistry.tools("anthropic", agent as any) + const toolIds = result.map(t => t.id) + + expect(toolIds).toContain("bash") + }, + }) + }) + + test("catch-all deny filters tools without explicit rules", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const agent = { + name: "test-agent", + mode: "subagent" as const, + permission: [ + { permission: "bash", pattern: "*", action: "allow" as const }, + { permission: "*", pattern: "*", action: "deny" as const } + ], + options: {} + } + + const result = await ToolRegistry.tools("anthropic", agent as any) + const toolIds = result.map(t => t.id) + + expect(toolIds).toContain("bash") + expect(toolIds).not.toContain("edit") + }, + }) + }) + + test("includes all tools when permission is empty array", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const agent = { + name: "test-agent", + mode: "subagent" as const, + permission: [], + options: {} + } + + const result = await ToolRegistry.tools("anthropic", agent as any) + + expect(result.length).toBeGreaterThan(0) + }, + }) + }) + + test("includes all tools when agent has no permission field", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + // When no agent is passed, all tools should be included + const result = await ToolRegistry.tools("anthropic", undefined) + + expect(result.length).toBeGreaterThan(0) + }, + }) + }) + + test("returns bash tool when no permissions set", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const result = await ToolRegistry.tools("anthropic") + + const bashTool = result.find(t => t.id === "bash") + expect(bashTool).toBeDefined() + }, + }) + }) + + test("returns edit tool when no permissions set", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const result = await ToolRegistry.tools("anthropic") + + const editTool = result.find(t => t.id === "edit") + expect(editTool).toBeDefined() + }, + }) + }) + + test("tool has proper structure with description and parameters", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const result = await ToolRegistry.tools("anthropic") + + const bashTool = result.find(t => t.id === "bash") + expect(bashTool).toBeDefined() + expect(bashTool?.description).toBeDefined() + expect(typeof bashTool?.description).toBe("string") + expect(bashTool?.parameters).toBeDefined() + }, + }) + }) + + test("handles allow rule for specific tool", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const agent = { + name: "test-agent", + mode: "subagent" as const, + permission: [ + { permission: "bash", pattern: "*", action: "allow" as const } + ], + options: {} + } + + const result = await ToolRegistry.tools("anthropic", agent as any) + const toolIds = result.map(t => t.id) + + expect(toolIds).toContain("bash") + }, + }) + }) + + test("ask action is treated as allow", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const agent = { + name: "test-agent", + mode: "subagent" as const, + permission: [ + { permission: "bash", pattern: "*", action: "ask" as const } + ], + options: {} + } + + const result = await ToolRegistry.tools("anthropic", agent as any) + const toolIds = result.map(t => t.id) + + expect(toolIds).toContain("bash") + }, + }) + }) + + test("empty permission array bypasses filtering", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const allToolsResult = await ToolRegistry.tools("anthropic") + + const agent = { + name: "test-agent", + mode: "subagent" as const, + permission: [], + options: {} + } + + const filteredResult = await ToolRegistry.tools("anthropic", agent as any) + + expect(filteredResult.length).toBe(allToolsResult.length) + }, + }) + }) + + test("multiple specific allow rules work correctly", async () => { + await Instance.provide({ + directory: projectRoot, + fn: async () => { + const agent = { + name: "test-agent", + mode: "subagent" as const, + permission: [ + { permission: "bash", pattern: "*", action: "allow" as const }, + { permission: "read", pattern: "*", action: "allow" as const }, + { permission: "edit", pattern: "*", action: "deny" as const } + ], + options: {} + } + + const result = await ToolRegistry.tools("anthropic", agent as any) + const toolIds = result.map(t => t.id) + + expect(toolIds).toContain("bash") + expect(toolIds).toContain("read") + expect(toolIds).not.toContain("edit") + }, + }) + }) +})