diff --git a/packages/opencode/src/kilocode/permission/provenance.ts b/packages/opencode/src/kilocode/permission/provenance.ts index abf9647d72e..499e07f8662 100644 --- a/packages/opencode/src/kilocode/permission/provenance.ts +++ b/packages/opencode/src/kilocode/permission/provenance.ts @@ -102,13 +102,33 @@ export namespace PermissionProvenance { /** * Classify why a tool call was denied, from the `ruleset` a `DeniedError` carries. * - * `DeniedError.ruleset` is untyped (`Schema.Any`) but is always the tagged ruleset `askPermission` - * built, filtered to the request's permission. The last `deny` rule in it is the one that decided. + * `DeniedError.ruleset` is untyped (`Schema.Any`); `Permission.ask` shapes it as + * `{ rule, matches }`, where `rule` is the exact rule `resolve()` matched against the + * request's pattern (via `Wildcard.match`), not merely the last `deny` rule for the + * permission. Two deny rules for different patterns under the same permission (e.g. + * `bash: { "git push *": deny, "rm -rf *": deny }`) would otherwise be indistinguishable by + * permission alone, misattributing the denial to whichever rule happens to sort last. + * + * Some denials carry no `rule` at all — e.g. the headless-subagent policy denial in + * `Permission.ask`, which isn't decided by any rule. `classify({ rule: undefined })` reports + * `{ source: "default" }`, the exact same shape as the *approval* fallback for "no rule + * matched", so a denial with no rule would otherwise render (and export) as an auto-approval. + * Synthesize a `deny` rule for the request's permission/pattern in that case so `rule.action` + * always reflects the real outcome. */ - export function classifyDenial(input: { ruleset: unknown; agent: string; origins: Origins }): Approval { - const rule = Array.isArray(input.ruleset) - ? (input.ruleset as Permission.Rule[]).findLast((rule) => rule.action === "deny") - : undefined + export function classifyDenial(input: { + ruleset: unknown + permission: string + patterns: readonly string[] + agent: string + origins: Origins + }): Approval { + const denial = input.ruleset as { rule?: Permission.Rule } | undefined + const rule = denial?.rule ?? { + permission: input.permission, + pattern: input.patterns[0] ?? "*", + action: "deny" as const, + } return classify({ rule, agent: input.agent, origins: input.origins }) } } diff --git a/packages/opencode/src/permission/index.ts b/packages/opencode/src/permission/index.ts index 9ce38c952fc..efec951dfea 100644 --- a/packages/opencode/src/permission/index.ts +++ b/packages/opencode/src/permission/index.ts @@ -226,14 +226,21 @@ export const layer = Layer.effect( for (const pattern of request.patterns) { const rule = resolve(request.permission, pattern, ruleset, approved, local) // kilocode_change — include session-scoped rules yield* Effect.logInfo("evaluated", { permission: request.permission, pattern, action: rule }) - // kilocode_change start — saved/session approvals cannot override hard Ask/Plan denials - if (veto(request.permission, pattern, hardRuleset)) { - return yield* new DeniedError({ ruleset: subset(request.permission, hardRuleset ?? []) }) + // kilocode_change start — saved/session approvals cannot override hard Ask/Plan denials. + // Report the exact hard rule that matched this pattern (not just the deny-permission + // subset) so provenance attributes the denial to the right rule, not just any deny rule. + const hardRule = hardRuleset && ExternalDirectoryPermission.evaluate(request.permission, pattern, hardRuleset) + if (hardRule?.action === "deny") { + return yield* new DeniedError({ + ruleset: { rule: hardRule, matches: subset(request.permission, hardRuleset ?? []) }, + }) } // kilocode_change end if (rule.action === "deny") { + // kilocode_change - carry the exact matched `rule` (not just the deny-permission subset) + // so provenance can attribute the denial to the pattern that actually decided it. return yield* new DeniedError({ - ruleset: subset(request.permission, ruleset), // kilocode_change + ruleset: { rule, matches: subset(request.permission, ruleset) }, }) } // kilocode_change start - skill shell forces a prompt instead of honoring an allow/auto-approve rule @@ -255,7 +262,8 @@ export const layer = Layer.effect( // kilocode_change start - headless subagent asks fail instead of queuing for a reply that never comes (#11903) if (yield* KiloHeadless.denies(request.sessionID).pipe(Effect.provideService(Database.Service, database))) { - return yield* new DeniedError({ ruleset: subset(request.permission, ruleset) }) + // no single rule decided this — it's a headless policy denial, not a ruleset match + return yield* new DeniedError({ ruleset: { matches: subset(request.permission, ruleset) } }) } // kilocode_change end diff --git a/packages/opencode/src/session/tools.ts b/packages/opencode/src/session/tools.ts index f33c8fc627d..98bb1a11e4b 100644 --- a/packages/opencode/src/session/tools.ts +++ b/packages/opencode/src/session/tools.ts @@ -89,6 +89,8 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { metadata: { approval: PermissionProvenance.classifyDenial({ ruleset: err.ruleset, + permission: req.permission, + patterns: req.patterns, agent: input.agent.name, origins: permissionOrigins, }), diff --git a/packages/opencode/test/kilocode/permission/deny-provenance.test.ts b/packages/opencode/test/kilocode/permission/deny-provenance.test.ts new file mode 100644 index 00000000000..c04bce534c3 --- /dev/null +++ b/packages/opencode/test/kilocode/permission/deny-provenance.test.ts @@ -0,0 +1,91 @@ +import { describe, expect, test } from "bun:test" +import { Cause, Effect, Exit, Layer } from "effect" +import { Bus } from "../../../src/bus" +import { Permission } from "../../../src/permission" +import { PermissionProvenance } from "../../../src/kilocode/permission/provenance" +import { EventV2Bridge } from "../../../src/event-v2-bridge" +import { Database } from "@opencode-ai/core/database/database" +import { SessionID } from "../../../src/session/schema" +import * as Config from "../../../src/config/config" +import * as CrossSpawnSpawner from "@opencode-ai/core/cross-spawn-spawner" +import { provideTmpdirInstance } from "../../fixture/fixture" +import { testEffect } from "../../lib/effect" + +const env = Layer.mergeAll( + Permission.layer.pipe( + Layer.provide(EventV2Bridge.defaultLayer), + Layer.provide(Config.defaultLayer), + Layer.provide(Database.defaultLayer), + ), + Config.defaultLayer, + Bus.layer, + CrossSpawnSpawner.defaultLayer, +) +const it = testEffect(env) + +const ask = (input: Parameters[0]) => + Effect.gen(function* () { + const permission = yield* Permission.Service + return yield* permission.ask(input) + }) + +function withDir(options: { git?: boolean } | undefined, self: (dir: string) => Effect.Effect) { + return provideTmpdirInstance(self, options) +} + +describe("Permission.ask denial provenance", () => { + it.live( + "attributes a denial to the rule that matched the request's pattern, not just the textually-last deny rule for the permission", + () => + withDir({ git: true }, () => + Effect.gen(function* () { + // Two deny rules under the same permission for different patterns. Matching by + // permission alone (e.g. findLast over rules with action "deny") would pick + // "rm -rf *" here since it sorts last, even though "git push *" is the one that + // actually matched the request. + const ruleset = [ + { permission: "bash", pattern: "git push *", action: "deny" as const }, + { permission: "bash", pattern: "rm -rf *", action: "deny" as const }, + ] + const exit = yield* ask({ + sessionID: SessionID.make("session_test"), + permission: "bash", + patterns: ["git push origin main"], + metadata: {}, + always: [], + ruleset, + }).pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const err = Exit.isFailure(exit) ? Cause.squash(exit.cause) : undefined + expect(err).toBeInstanceOf(Permission.DeniedError) + + const approval = PermissionProvenance.classifyDenial({ + ruleset: (err as Permission.DeniedError).ruleset, + permission: "bash", + patterns: ["git push origin main"], + agent: "build", + origins: undefined, + }) + expect(approval.rule).toEqual({ permission: "bash", pattern: "git push *", action: "deny" }) + }), + ), + ) + + test("a denial with no rule in the carried ruleset is still reported as denied, not as an ambiguous default approval", () => { + // Some denials carry no `rule` at all (e.g. the headless-subagent policy denial in + // Permission.ask, which isn't decided by any ruleset match). classify({ rule: undefined }) + // reports { source: "default" } — the same shape the *approval* fallback produces for "no + // rule matched" — so without a synthesized deny rule, a refusal would render (and export) + // as an auto-approval. + const approval = PermissionProvenance.classifyDenial({ + ruleset: { matches: [] }, + permission: "bash", + patterns: ["rm -rf /"], + agent: "build", + origins: undefined, + }) + expect(approval.rule?.action).toBe("deny") + expect(approval.rule).toEqual({ permission: "bash", pattern: "rm -rf /", action: "deny" }) + }) +}) diff --git a/packages/opencode/test/kilocode/sandbox/session-tools.test.ts b/packages/opencode/test/kilocode/sandbox/session-tools.test.ts index a41a244ceca..d1eedce06ee 100644 --- a/packages/opencode/test/kilocode/sandbox/session-tools.test.ts +++ b/packages/opencode/test/kilocode/sandbox/session-tools.test.ts @@ -361,11 +361,11 @@ it.live("records why a denied tool call was refused on the tool part's metadata" Effect.gen(function* () { const dirs = yield* fixture() const metadataCalls: { toolCallID: string; value: Record }[] = [] - const deniedRuleset = [{ permission: "bash", pattern: "*", action: "deny" as const, source: "project" as const }] + const deniedRule = { permission: "bash", pattern: "*", action: "deny" as const, source: "project" as const } const overrides = Layer.mergeAll( TestConfig.layer({ get: () => Effect.succeed({ sandbox: { enabled: false } }) }), Layer.mock(Permission.Service)({ - ask: () => Effect.fail(new Permission.DeniedError({ ruleset: deniedRuleset })), + ask: () => Effect.fail(new Permission.DeniedError({ ruleset: { rule: deniedRule, matches: [deniedRule] } })), }), ) const tools = yield* resolve(dirs.ctx, metadataCalls).pipe(Effect.provide(overrides))