fix(permission): check all patterns before pending, clean up on abort

Evaluate all patterns upfront so a deny is caught even when an earlier
pattern would be ask. Use Effect.ensuring to delete the pending entry
when the Deferred is aborted. Add tests for mixed ask/deny patterns
and abort cleanup.
This commit is contained in:
Kit Langton
2026-03-14 14:17:38 -04:00
parent 7d102adbc9
commit 2d271d4260
2 changed files with 96 additions and 14 deletions

View File

@@ -148,6 +148,7 @@ export class PermissionService extends ServiceMap.Service<PermissionService, Per
const ask = Effect.fn("PermissionService.ask")(function* (input: z.infer<typeof AskInput>) {
const state = yield* InstanceState.get(instanceState)
const { ruleset, ...request } = input
let pending = false
for (const pattern of request.patterns) {
const rule = evaluate(request.permission, pattern, ruleset, state.approved)
@@ -158,19 +159,27 @@ export class PermissionService extends ServiceMap.Service<PermissionService, Per
})
}
if (rule.action === "allow") continue
const id = request.id ?? PermissionID.ascending()
const info: Request = {
id,
...request,
}
log.info("asking", { id, permission: info.permission, patterns: info.patterns })
const deferred = yield* Deferred.make<void, RejectedError | CorrectedError>()
state.pending.set(id, { info, deferred })
void Bus.publish(Event.Asked, info)
return yield* Deferred.await(deferred)
pending = true
}
if (!pending) return
const id = request.id ?? PermissionID.ascending()
const info: Request = {
id,
...request,
}
log.info("asking", { id, permission: info.permission, patterns: info.patterns })
const deferred = yield* Deferred.make<void, RejectedError | CorrectedError>()
state.pending.set(id, { info, deferred })
void Bus.publish(Event.Asked, info)
return yield* Effect.ensuring(
Deferred.await(deferred),
Effect.sync(() => {
state.pending.delete(id)
}),
)
})
const reply = Effect.fn("PermissionService.reply")(function* (input: z.infer<typeof ReplyInput>) {
@@ -188,7 +197,7 @@ export class PermissionService extends ServiceMap.Service<PermissionService, Per
if (input.reply === "reject") {
yield* Deferred.fail(
existing.deferred,
input.message ? new CorrectedError({ feedback: input.message }) : new RejectedError,
input.message ? new CorrectedError({ feedback: input.message }) : new RejectedError(),
)
for (const [id, item] of state.pending.entries()) {
@@ -199,7 +208,7 @@ export class PermissionService extends ServiceMap.Service<PermissionService, Per
requestID: item.info.id,
reply: "reject",
})
yield* Deferred.fail(item.deferred, new RejectedError)
yield* Deferred.fail(item.deferred, new RejectedError())
}
return
}

View File

@@ -1,7 +1,9 @@
import { test, expect } from "bun:test"
import os from "os"
import { Bus } from "../../src/bus"
import { runtime } from "../../src/effect/runtime"
import { PermissionNext } from "../../src/permission/next"
import * as S from "../../src/permission/service"
import { PermissionID } from "../../src/permission/schema"
import { Instance } from "../../src/project/instance"
import { tmpdir } from "../fixture/fixture"
@@ -963,3 +965,74 @@ test("ask - allows all patterns when all match allow rules", async () => {
},
})
})
test("ask - should deny even when an earlier pattern is ask", async () => {
await using tmp = await tmpdir({ git: true })
await Instance.provide({
directory: tmp.path,
fn: async () => {
const ask = PermissionNext.ask({
sessionID: SessionID.make("session_test"),
permission: "bash",
patterns: ["echo hello", "rm -rf /"],
metadata: {},
always: [],
ruleset: [
{ permission: "bash", pattern: "echo *", action: "ask" },
{ permission: "bash", pattern: "rm *", action: "deny" },
],
})
const out = await Promise.race([
ask.then(
() => ({ ok: true as const, err: undefined }),
(err) => ({ ok: false as const, err }),
),
Bun.sleep(100).then(() => "timeout" as const),
])
if (out === "timeout") {
await rejectAll()
await ask.catch(() => {})
throw new Error("ask timed out instead of denying immediately")
}
expect(out.ok).toBe(false)
expect(out.err).toBeInstanceOf(PermissionNext.DeniedError)
expect(await PermissionNext.list()).toHaveLength(0)
},
})
})
test("ask - abort should clear pending request", async () => {
await using tmp = await tmpdir({ git: true })
await Instance.provide({
directory: tmp.path,
fn: async () => {
const ctl = new AbortController()
const ask = runtime.runPromise(
S.PermissionService.use((svc) =>
svc.ask({
sessionID: SessionID.make("session_test"),
permission: "bash",
patterns: ["ls"],
metadata: {},
always: [],
ruleset: [{ permission: "bash", pattern: "*", action: "ask" }],
}),
),
{ signal: ctl.signal },
)
await waitForPending(1)
ctl.abort()
await ask.catch(() => {})
try {
expect(await PermissionNext.list()).toHaveLength(0)
} finally {
await rejectAll()
}
},
})
})