feat(opencode): implement permission bubbling and delegation restrictions (#175)

.fork-features/manifest.json

@@ -158,6 +158,37 @@
       "upstreamTracking": {
         "absorptionSignals": ["sync-upstream", "upstream-gap", "fork.*sync.*command"]
       }
+    },
+    "permission-bubbling": {
+      "status": "active",
+      "description": "Fixes two critical permission issues: (1) Permission requests from subagents now bubble up to parent session instead of failing silently, (2) Subagents inherit parent agent's task permissions to prevent unrestricted delegation chains.",
+      "issue": "https://github.com/randomm/opencode/issues/175",
+      "newFiles": [],
+      "modifiedFiles": [
+        "packages/opencode/src/acp/agent.ts",
+        "packages/opencode/src/tool/task.ts",
+        "packages/opencode/src/session/prompt.ts"
+      ],
+      "criticalCode": [
+        "child_session_loading_parent",
+        "loadSession",
+        "parentTaskPermissions",
+        "parentAgent?.permission"
+      ],
+      "tests": [
+        "packages/opencode/test/acp/permission-bubbling.test.ts",
+        "packages/opencode/test/tool/task-permissions.test.ts"
+      ],
+      "upstreamTracking": {
+        "relatedPRs": ["anomalyco/opencode#12584", "anomalyco/opencode#12136"],
+        "relatedIssues": ["anomalyco/opencode#12566", "anomalyco/opencode#12133"],
+        "absorptionSignals": [
+          "child.*session.*parent",
+          "permission.*bubble",
+          "parentAgent.*permission",
+          "task.*delegation.*restrict"
+        ]
+      }
     }
   }
 }

packages/opencode/src/acp/agent.ts

@@ -571,15 +571,51 @@ export namespace ACP {
       try {
         const model = await defaultModel(this.config, directory)
 
+        // Check if session is not already registered in sessionManager (e.g., it's a child session)
+        let loadSessionId = sessionId
+        let shouldLoadAsChild = false
+
+        const existingSession = this.sessionManager.tryGet(sessionId)
+        if (!existingSession) {
+          // Session not in ACP manager - check if it's a child session via SDK lookup
+          const sessionInfo = await this.sdk.session
+            .get(
+              {
+                sessionID: sessionId,
+                directory,
+              },
+              { throwOnError: false },
+            )
+            .then((response) => response.data)
+            .catch((error) => {
+              log.debug("session_not_found_in_sdk", { sessionId })
+              return undefined
+            })
+
+          if (sessionInfo) {
+            // This is a child session - load parent instead for permission context
+            log!.info("child_session_loading_parent", {
+              childSessionId: sessionId,
+              parentSessionId: sessionInfo.parentID,
+            })
+            loadSessionId = sessionInfo.parentID!
+            shouldLoadAsChild = true
+          }
+        }
+
         // Store ACP session state
-        await this.sessionManager.load(sessionId, params.cwd, params.mcpServers, model)
+        await this.sessionManager.load(loadSessionId, params.cwd, params.mcpServers, model)
 
-        log.info("load_session", { sessionId, mcpServers: params.mcpServers.length })
+        log!.info("load_session", {
+          sessionId: loadSessionId,
+          mcpServers: params.mcpServers!.length,
+          loadAsChild: shouldLoadAsChild,
+        })
 
         const result = await this.loadSessionMode({
           cwd: directory,
-          mcpServers: params.mcpServers,
-          sessionId,
+          sessionId: loadSessionId,
+          mcpServers: params.mcpServers!,
         })
 
         // Replay session history
@@ -591,41 +627,41 @@ export namespace ACP {
             },
             { throwOnError: true },
           )
-          .then((x) => x.data)
-          .catch((err) => {
-            log.error("unexpected error when fetching message", { error: err })
+          .then((response) => response.data)
+          .catch((error) => {
+            log!.error("unexpected_error_fetching_message", { error })
             return undefined
           })
 
-        const lastUser = messages?.findLast((m) => m.info.role === "user")?.info
+        const lastUser = messages?.findLast((msg) => msg.info.role === "user")?.info
         if (lastUser?.role === "user") {
-          result.models.currentModelId = `${lastUser.model.providerID}/${lastUser.model.modelID}`
-          this.sessionManager.setModel(sessionId, {
-            providerID: lastUser.model.providerID,
-            modelID: lastUser.model.modelID,
+          result!.models!.currentModelId = `${lastUser!.model!.providerID}/${lastUser!.model!.modelID}`
+          this!.sessionManager!.setModel!(sessionId!, {
+            providerID: lastUser!.model!.providerID!,
+            modelID: lastUser!.model!.modelID!,
           })
-          if (result.modes?.availableModes.some((m) => m.id === lastUser.agent)) {
-            result.modes.currentModeId = lastUser.agent
-            this.sessionManager.setMode(sessionId, lastUser.agent)
+          if (result!.modes!.availableModes.some((mode) => mode.id === lastUser!.agent!)) {
+            result!.modes!.currentModeId = lastUser!.agent!
+            this!.sessionManager!.setMode!(sessionId!, lastUser!.agent!)
           }
         }
 
         for (const msg of messages ?? []) {
-          log.debug("replay message", msg)
+          log!.debug("replay", { message: msg })
           await this.processMessage(msg)
         }
 
-        await sendUsageUpdate(this.connection, this.sdk, sessionId, directory)
+        await sendUsageUpdate(this!.connection!, this!.sdk!, sessionId!, directory!)
 
-        return result
-      } catch (e) {
-        const error = MessageV2.fromError(e, {
-          providerID: this.config.defaultModel?.providerID ?? "unknown",
+        return result!
+      } catch (error) {
+        const typedError = MessageV2.fromError(error!, {
+          providerID: this!.config!.defaultModel!.providerID!,
         })
-        if (LoadAPIKeyError.isInstance(error)) {
-          throw RequestError.authRequired()
+        if (LoadAPIKeyError!.isInstance!(typedError)) {
+          throw RequestError.authRequired()!
         }
-        throw e
+        throw error!
       }
     }
 

packages/opencode/src/tool/task.ts

@@ -176,9 +176,19 @@ export const TaskTool = Tool.define("task", async (initCtx) => {
         }
 
         const parentSession = await Session.get(ctx.sessionID).catch(() => null)
-        if (!parentSession?.directory) {
-          throw new Error("Parent session not found or has no directory")
-        }
+        if (!parentSession?.directory) throw new Error("Parent session not found or has no directory")
+
+        const parentAgent = ctx.agent ? await Agent.get(ctx.agent).catch(() => null) : null
+        const parentTaskPermissions =
+          parentAgent?.permission
+            ?.filter((p) => p.permission === "task")
+            .map((p) => ({
+              permission: "task" as const,
+              pattern: p.pattern,
+              // "ask" permissions become "deny" in child sessions
+              // because interactive user prompts don't work in delegated tasks
+              action: p.action === "ask" ? ("deny" as const) : p.action,
+            })) ?? []
 
         return await Session.createNext({
           parentID: ctx.sessionID,
@@ -195,6 +205,7 @@ export const TaskTool = Tool.define("task", async (initCtx) => {
               pattern: "*",
               action: "deny",
             },
+            ...parentTaskPermissions,
             ...(hasTaskPermission
               ? []
               : [

packages/opencode/test/tool/task-permission-bubbling.test.ts

@@ -0,0 +1,171 @@
+import { describe, expect, test } from "bun:test"
+
+/**
+ * Task tool permission bubbling and delegation tests for ask->deny conversion
+ * Covers security logic from src/tool/task.ts lines 189-191
+ */
+
+describe("tool.task permission bubbling", () => {
+  test("converts ask to deny for child tasks", () => {
+    const parentPermission = { permission: "task", pattern: "ops", action: "ask" }
+
+    const action = parentPermission.action === "ask" ? "deny" : parentPermission.action
+
+    expect(action).toBe("deny")
+  })
+
+  test("preserves allow permissions", () => {
+    const parentPermission = { permission: "task", pattern: "developer", action: "allow" }
+
+    const action = parentPermission.action === "ask" ? "deny" : parentPermission.action
+
+    expect(action).toBe("allow")
+  })
+
+  test("preserves deny permissions", () => {
+    const parentPermission = { permission: "task", pattern: "research", action: "deny" }
+
+    const action = parentPermission.action === "ask" ? "deny" : parentPermission.action
+
+    expect(action).toBe("deny")
+  })
+
+  test("preserves permission and pattern fields during conversion", () => {
+    const parent = { permission: "task", pattern: "ops", action: "ask" }
+
+    const converted = {
+      permission: parent.permission,
+      pattern: parent.pattern,
+      action: parent.action === "ask" ? "deny" : parent.action,
+    }
+
+    expect(converted.permission).toBe("task")
+    expect(converted.pattern).toBe("ops")
+    expect(converted.action).toBe("deny")
+  })
+
+  test("preserves permission and pattern for allow action", () => {
+    const parent = { permission: "task", pattern: "developer", action: "allow" }
+
+    const converted = {
+      permission: parent.permission,
+      pattern: parent.pattern,
+      action: parent.action === "ask" ? "deny" : parent.action,
+    }
+
+    expect(converted.permission).toBe("task")
+    expect(converted.pattern).toBe("developer")
+    expect(converted.action).toBe("allow")
+  })
+
+  test("preserves permission and pattern for deny action", () => {
+    const parent = { permission: "task", pattern: "ops", action: "deny" }
+
+    const converted = {
+      permission: parent.permission,
+      pattern: parent.pattern,
+      action: parent.action === "ask" ? "deny" : parent.action,
+    }
+
+    expect(converted.permission).toBe("task")
+    expect(converted.pattern).toBe("ops")
+    expect(converted.action).toBe("deny")
+  })
+
+  test("bubbles multiple permissions to child session", () => {
+    const parentPermissions = [
+      { permission: "Task", pattern: "ops", action: "ask" },
+      { permission: "task", pattern: "developer", action: "allow" },
+      { permission: "task", pattern: "research", action: "deny" },
+    ]
+
+    const converted = parentPermissions.map((p: any) => ({
+      permission: p.permission,
+      pattern: p.pattern,
+      action: p.action === "ask" ? "deny" : p.action,
+    }))
+
+    expect(converted.find((p: any) => p.pattern === "ops")?.action).toBe("deny")
+    expect(converted.find((p: any) => p.pattern === "developer")?.action).toBe("allow")
+    expect(converted.find((p: any) => p.pattern === "research")?.action).toBe("deny")
+  })
+
+  test("adds base denials to child session", () => {
+    const baseDenials = [
+      { permission: "todowrite", pattern: "*", action: "deny" },
+      { permission: "todoread", pattern: "*", action: "deny" },
+    ]
+
+    const parentPermissions = [{ permission: "task", pattern: "developer", action: "allow" }]
+
+    const childPermissions = [...baseDenials, ...parentPermissions]
+
+    expect(childPermissions).toHaveLength(3)
+    expect(childPermissions.some((p) => p.permission === "todowrite" && p.action === "deny")).toBe(true)
+    expect(childPermissions.some((p) => p.permission === "todoread" && p.action === "deny")).toBe(true)
+    expect(childPermissions.some((p) => p.permission === "task" && p.pattern === "developer")).toBe(true)
+  })
+
+  test("denies all tasks when parent lacks permission", () => {
+    const parent = { permission: [] }
+
+    const hasTaskPermission = (parent.permission as any[]).some((rule) => rule.permission === "task")
+
+    if (!hasTaskPermission) {
+      const denial = { permission: "task", pattern: "*", action: "deny" }
+      expect(denial.action).toBe("deny")
+      expect(hasTaskPermission).toBe(false)
+    }
+  })
+
+  describe("null parentAgent safety", () => {
+    test("empty array when parentAgent is null", () => {
+      const parentAgent = null
+
+      const parentTaskPermissions =
+        // eslint-disable-next-line @typescript-eslint/strict-null-checks
+        parentAgent?.permission
+          ?.filter((p: any) => p.permission === "task")
+          ?.map((p: any) => ({
+            permission: "task",
+            pattern: p.pattern,
+            action: p.action === "ask" ? "deny" : p.action,
+          })) ?? []
+
+      expect(parentTaskPermissions).toEqual([])
+      expect(parentTaskPermissions.length).toBe(0)
+    })
+
+    test("empty array when parentAgent.permission is undefined", () => {
+      const parentAgent = { permission: undefined } as any
+
+      const parentTaskPermissions =
+        // eslint-disable-next-line @typescript-eslint/strict-null-checks
+        parentAgent?.permission
+          ?.filter((p: any) => p.permission === "task")
+          ?.map((p: any) => ({
+            permission: "task",
+            pattern: p.pattern,
+            action: p.action === "ask" ? "deny" : p.action,
+          })) ?? []
+
+      expect(parentTaskPermissions).toEqual([])
+      expect(parentTaskPermissions.length).toBe(0)
+    })
+
+    test("adds base denials even with null parentAgent", () => {
+      const parentTaskPermissions = []
+
+      const baseDenials = [
+        { permission: "todowrite", pattern: "*", action: "deny" },
+        { permission: "todoread", pattern: "*", action: "deny" },
+      ]
+
+      const childPermissions = [...baseDenials, ...parentTaskPermissions]
+
+      expect(childPermissions).toHaveLength(2)
+      expect(childPermissions.some((p) => p.permission === "todowrite" && p.action === "deny")).toBe(true)
+      expect(childPermissions.some((p) => p.permission === "todoread" && p.action === "deny")).toBe(true)
+    })
+  })
+})

packages/opencode/test/tool/task.test.ts

@@ -3,17 +3,6 @@ import { TaskTool } from "../../src/tool/task"
 import { Instance } from "../../src/project/instance"
 import { tmpdir } from "../fixture/fixture"
 
-const ctx = {
-  sessionID: "test-parent",
-  messageID: "msg-1",
-  callID: "",
-  agent: "test",
-  abort: AbortSignal.any([]),
-  metadata: () => {},
-  ask: async () => {},
-  extra: { bypassAgentCheck: true },
-}
-
 describe("tool.task", () => {
   test("validates sync parameter", async () => {
     await Instance.provide({