diff --git a/docs/adr/0001-recover-full-bash-command-from-session.md b/docs/adr/0001-recover-full-bash-command-from-session.md new file mode 100644 index 0000000..d68f09b --- /dev/null +++ b/docs/adr/0001-recover-full-bash-command-from-session.md @@ -0,0 +1,48 @@ +--- +status: accepted +--- + +# Recover the full Bash command from the Pi session + +`pi-permission-inner-cmd` needs the complete Bash input before it may allow a transparent wrapper. `@gotgenes/pi-permission-system` exposes only the winning command unit as `details.command`; for `timeout 60s pnpm test && git push`, that may be `timeout 60s pnpm test`. An Authorizer `allow` approves the whole tool call, so unwrapping that unit alone could hide a sibling command. + +## Decision + +For direct calls to Pi's native `bash` tool, capture the session manager at `session_start`. During authorization, use `details.toolCallId` to find exactly one matching assistant `toolCall` block in the current session and read its structured `arguments.command` value. + +Proceed only when all evidence agrees: + +- the request and recovered tool call both identify the native `bash` tool; +- the tool-call ID has exactly one match; +- `arguments.command` is a string; +- the complete input matches the strict wrapper grammar; +- the inner command is not another recognized wrapper. + +V0.1 recognizes only: + +```regex +^timeout[ \t]+([1-9][0-9]*[smhd])[ \t]+(.+)$ +``` + +The complete inner command is re-evaluated through the Deterministic Permission Policy. Map `allow` to `allow`, `deny` to `deny`, and `ask` to `defer`. Missing or ambiguous session evidence, forwarded requests, shell aliases, unsupported `timeout` options, nested wrappers, parse failures, and exceptions all produce `defer`. + +## Consequences + +This avoids changing permission-system and avoids parsing authorization evidence from the human-facing `details.message`. It deliberately supports fewer contexts: forwarded and non-native shell calls continue through the existing authority chain. + +The implementation must retain regression tests for: + +- inner `allow`, `ask`, and `deny` mapping; +- compound input such as `timeout 60s pnpm test && git push`; +- unsupported and nested wrapper syntax; +- missing, duplicate, malformed, and forwarded session evidence; +- Authorizer registration and disposal. + +The end-to-end experiment confirmed that permission-system checks every Bash command unit but invokes the Authorizer once for the aggregated `ask`; an Authorizer `allow` then releases the complete tool call. See [context ownership](../research/ai-bash-context-ownership.md) and [minimal Bash judgment evidence](../research/ai-bash-judge-input-minimality.md) for the underlying permission and evidence boundaries. + +## Rejected alternatives + +- **Treat `details.command` as the full input:** unsafe for compound Bash programs. +- **Parse `details.message`:** fail-closed parsing is possible but couples authorization to UI prose. +- **Add a permission-system API:** structurally clean, but would make this package depend on an unavailable upstream change. +- **Maintain a safe-command allowlist:** duplicates policy and violates the re-evaluation invariant. diff --git a/packages/pi-permission-inner-cmd/package.json b/packages/pi-permission-inner-cmd/package.json index c21c4fd..aa1c067 100644 --- a/packages/pi-permission-inner-cmd/package.json +++ b/packages/pi-permission-inner-cmd/package.json @@ -14,12 +14,13 @@ "peerDependencies": { "@earendil-works/pi-ai": "*", "@earendil-works/pi-coding-agent": "*", - "@gotgenes/pi-permission-system": ">=20.10.0" + "@gotgenes/pi-permission-system": ">=24.0.0" }, "devDependencies": { "@earendil-works/pi-ai": "*", "@earendil-works/pi-coding-agent": "*", - "@gotgenes/pi-permission-system": ">=20.10.0", + "@gotgenes/pi-permission-system": ">=24.0.0", + "@types/node": "^26.0.0", "typescript": "^5", "vitest": "^3" }, diff --git a/packages/pi-permission-inner-cmd/src/authorizer.ts b/packages/pi-permission-inner-cmd/src/authorizer.ts new file mode 100644 index 0000000..5f35bed --- /dev/null +++ b/packages/pi-permission-inner-cmd/src/authorizer.ts @@ -0,0 +1,179 @@ +import type { SessionEntry } from "@earendil-works/pi-coding-agent"; +import type { + AuthorizerLog, + AuthorizerVerdict, + PermissionQuery, + PromptPermissionDetails, +} from "@gotgenes/pi-permission-system"; +import { classifyWrapper, isRecognizedWrapper } from "./recognizer"; +import { NATIVE_BASH_TOOL_NAME, recoverNativeBashCommand } from "./recovery"; + +/** Bash permission surface queried when re-evaluating the inner command. */ +const BASH_SURFACE = "bash"; + +/** Convert a thrown value into a short, log-safe string. */ +function toErrorString(error: unknown): string { + if (error instanceof Error) { + return error.message; + } + return String(error); +} + +/** + * Live read access to the captured session at authorize time. + * + * The real `ReadonlySessionManager` satisfies this structurally; tests inject a + * stub so the decision logic stays pure and deterministic. + */ +export interface SessionProbe { + getEntries(): ReadonlyArray; + getSessionId(): string; +} + +/** Dependencies injected into the pure authorizer decision. */ +export interface InnerCommandAuthorizerDeps { + readonly details: PromptPermissionDetails; + readonly query: PermissionQuery; + readonly log: AuthorizerLog; + /** Live reader for the captured UI-root session. */ + readonly session: SessionProbe; + /** + * Session identity captured at registration as root-ownership provenance. + * Revalidated against {@link SessionProbe.getSessionId} before any decisive + * verdict so a stale or replaced session can never be judged. + */ + readonly expectedSessionId: string; +} + +/** + * V0.1 inner-command Authorizer decision (ADR 0001). + * + * Recovers the complete native Bash command for `details.toolCallId` from the + * captured session, unwraps one strict `timeout` level, and re-evaluates the + * inner command through the deterministic permission policy. Every uncertain + * path — forwarded requests, a session-identity mismatch, non-Bash tools, + * missing/duplicate/malformed session evidence, unsupported or nested wrapper + * syntax, parse failures, and exceptions — defers to the next authority. + * + * Logging contract: + * - `review` only for a recognized wrapper whose inner command resolves to a + * decisive `allow`/`deny`. + * - `debug` for a recognized inner `ask`, unsupported timeout syntax, nested + * wrappers, a session-identity mismatch, and exceptions. + * - ordinary non-timeout commands defer silently. + * - recognized logs carry both `command` and `innerCommand`; an exception after + * recognition retains both alongside `error`, while an earlier exception logs + * only the safe data available at that point. + */ +export async function authorizeInnerCommand( + deps: InnerCommandAuthorizerDeps, +): Promise { + const { details, query, log, session, expectedSessionId } = deps; + + // Track recovered evidence so an exception after recognition can retain it. + let command: string | undefined; + let innerCommand: string | undefined; + + try { + // Forwarded subagent asks are out of scope for v0.1: the captured + // session is the serving root's conversation, not the requester's. + if (details.forwarding) { + return { kind: "defer" }; + } + + // Revalidate root ownership: the live session must still be the one we + // registered for. A mismatch (or a session id that cannot be read) + // means the captured conversation can no longer be attributed safely. + const currentSessionId = session.getSessionId(); + if (currentSessionId !== expectedSessionId) { + log.debug("inner_cmd.session_mismatch", { + expectedSessionId, + currentSessionId, + }); + return { kind: "defer" }; + } + + // Only the native Bash tool is unwrappable, and only when the ask is + // tied to a specific tool call. + if (details.toolName !== NATIVE_BASH_TOOL_NAME) { + return { kind: "defer" }; + } + const toolCallId = details.toolCallId; + if (toolCallId === undefined) { + return { kind: "defer" }; + } + + // Recover the complete Bash input from the session, never from + // details.command or details.message. + command = recoverNativeBashCommand(session.getEntries(), toolCallId); + if (command === undefined) { + return { kind: "defer" }; + } + + const classification = classifyWrapper(command); + + switch (classification.kind) { + case "nonTimeout": + // An ordinary Bash command this authorizer does not unwrap. + return { kind: "defer" }; + + case "unsupportedTimeout": + log.debug("inner_cmd.unsupported_timeout_syntax", { command }); + return { kind: "defer" }; + + case "recognized": { + innerCommand = classification.match.innerCommand; + + // Never unwrap more than one level in v0.1. + if (isRecognizedWrapper(innerCommand)) { + log.debug("inner_cmd.nested_timeout", { + command, + innerCommand, + }); + return { kind: "defer" }; + } + + const result = query.checkPermission( + BASH_SURFACE, + innerCommand, + details.agentName ?? undefined, + ); + + switch (result.state) { + case "allow": + log.review("inner_cmd.allow", { + command, + innerCommand, + }); + return { kind: "allow" }; + case "deny": + log.review("inner_cmd.deny", { + command, + innerCommand, + }); + return { kind: "deny" }; + case "ask": + default: + // Treat any unexpected state as a safe defer. + log.debug("inner_cmd.inner_ask", { + command, + innerCommand, + }); + return { kind: "defer" }; + } + } + } + } catch (error) { + const exceptionDetails: Record = { + error: toErrorString(error), + }; + if (command !== undefined) { + exceptionDetails.command = command; + } + if (innerCommand !== undefined) { + exceptionDetails.innerCommand = innerCommand; + } + log.debug("inner_cmd.exception", exceptionDetails); + return { kind: "defer" }; + } +} diff --git a/packages/pi-permission-inner-cmd/src/index.ts b/packages/pi-permission-inner-cmd/src/index.ts index 65106a9..df2c3b9 100644 --- a/packages/pi-permission-inner-cmd/src/index.ts +++ b/packages/pi-permission-inner-cmd/src/index.ts @@ -3,20 +3,81 @@ import { getPermissionsService, PERMISSIONS_READY_CHANNEL, } from "@gotgenes/pi-permission-system"; +import { authorizeInnerCommand, type SessionProbe } from "./authorizer"; const LINK_NAME = "inner-cmd"; -export default function permissionAiJudge(pi: ExtensionAPI): void { - let sessionStarted = false; +/** Captured UI-root session: the live probe plus its identity provenance. */ +interface CapturedRootSession { + readonly session: SessionProbe; + readonly sessionId: string; +} + +export default function permissionInnerCmd(pi: ExtensionAPI): void { + let rootSession: CapturedRootSession | undefined; let disposeAuthorizer: (() => void) | undefined; - pi.on("session_start", () => { - sessionStarted = true; + /** + * Register the inner-command Authorizer once a proven UI-root session is + * captured and the permission service is ready. + * + * `rootSession` is set only from a UI-present `session_start` with a + * non-empty captured session id, so a headless or in-process subagent child + * that can still resolve the published parent service never registers. + * Either the extension or the permission system may start first; whichever + * satisfies the second condition completes registration. + */ + function tryRegister(): void { + if (disposeAuthorizer || !rootSession) { + return; + } + + const service = getPermissionsService(); + if (!service) { + return; + } + + const { session, sessionId } = rootSession; + disposeAuthorizer = service.registerAuthorizer( + LINK_NAME, + async (details, query, log) => + authorizeInnerCommand({ + details, + query, + log, + session, + expectedSessionId: sessionId, + }), + ); + } + + pi.on("session_start", (_event, ctx) => { + // Root-ownership gate: register only from the proven UI-present root. + // Headless/in-process children resolve the parent's process-global + // service but must not register with child-captured context. + if (!ctx.hasUI) { + return; + } + + // Snapshot the session identity as registration provenance. A non-empty + // id is required so authorization can revalidate ownership later; + // without it, do not register. + const sessionId = ctx.sessionManager.getSessionId(); + if (!sessionId) { + return; + } + + rootSession = { session: ctx.sessionManager, sessionId }; + tryRegister(); }); pi.events.on(PERMISSIONS_READY_CHANNEL, () => { + tryRegister(); }); pi.on("session_shutdown", () => { + disposeAuthorizer?.(); + disposeAuthorizer = undefined; + rootSession = undefined; }); } diff --git a/packages/pi-permission-inner-cmd/src/recognizer.ts b/packages/pi-permission-inner-cmd/src/recognizer.ts new file mode 100644 index 0000000..e83bd3c --- /dev/null +++ b/packages/pi-permission-inner-cmd/src/recognizer.ts @@ -0,0 +1,80 @@ +/** + * V0.1 wrapper recognizer. + * + * The strict simple-timeout grammar from ADR 0001. V0.1 unwraps exactly + * `timeout `; every other `timeout` invocation is left to + * the next authority. + */ + +/** + * Matches `timeout ` where `` is a positive + * integer (no leading zero) followed by a single unit `s`/`m`/`h`/`d`. + * + * Flags (`-k`, `--preserve-status`, GNU `--`), compound durations, and the + * bare form are intentionally excluded so v0.1 never unwraps a wrapper it + * cannot re-evaluate safely. + */ +const TIMEOUT_WRAPPER_PATTERN = /^timeout[ \t]+([1-9][0-9]*[smhd])[ \t]+(.+)$/; + +/** A command that begins with the bare `timeout` wrapper program. */ +const TIMEOUT_PREFIX = /^timeout(?:[ \t]|$)/; + +export interface TimeoutWrapperMatch { + readonly duration: string; + readonly innerCommand: string; +} + +/** + * Parse a command as the strict simple-timeout wrapper. + * + * @returns the duration token and the full inner command (including any + * `&&`/`;`/`|` siblings), or `undefined` when the command is not the + * recognized `timeout ` form. + */ +export function parseTimeoutWrapper( + command: string, +): TimeoutWrapperMatch | undefined { + const match = TIMEOUT_WRAPPER_PATTERN.exec(command); + if (match === null) { + return undefined; + } + return { + duration: match[1], + innerCommand: match[2], + }; +} + +/** + * Whether a command is itself a recognized wrapper. Used to reject nested + * wrappers so v0.1 unwraps at most one level. + */ +export function isRecognizedWrapper(command: string): boolean { + return parseTimeoutWrapper(command) !== undefined; +} + +/** How a recovered Bash command relates to the v0.1 recognizer. */ +export type WrapperClassification = + | { readonly kind: "recognized"; readonly match: TimeoutWrapperMatch } + | { readonly kind: "unsupportedTimeout" } + | { readonly kind: "nonTimeout" }; + +/** + * Classify a recovered Bash command against the v0.1 recognizer. + * + * - `recognized`: the strict simple-timeout wrapper. + * - `unsupportedTimeout`: the command invokes `timeout` but is not the + * recognized strict form (flags, `-k`, missing command, ...). These are + * logged at debug so an operator can see why a wrapper was skipped. + * - `nonTimeout`: an ordinary command this authorizer does not handle. These + * defer silently. + */ +export function classifyWrapper(command: string): WrapperClassification { + const match = parseTimeoutWrapper(command); + if (match !== undefined) { + return { kind: "recognized", match }; + } + if (TIMEOUT_PREFIX.test(command)) { + return { kind: "unsupportedTimeout" }; + } + return { kind: "nonTimeout" }; +} diff --git a/packages/pi-permission-inner-cmd/src/recovery.ts b/packages/pi-permission-inner-cmd/src/recovery.ts new file mode 100644 index 0000000..34759b7 --- /dev/null +++ b/packages/pi-permission-inner-cmd/src/recovery.ts @@ -0,0 +1,83 @@ +import type { SessionEntry } from "@earendil-works/pi-coding-agent"; + +/** Name of Pi's native Bash tool, as recorded in a tool-call block. */ +export const NATIVE_BASH_TOOL_NAME = "bash"; + +/** A structurally-validated tool-call content block. */ +interface ToolCallBlock { + readonly id: string; + readonly name: unknown; + readonly arguments: unknown; +} + +function isToolCallBlock(block: unknown): block is ToolCallBlock { + return ( + block !== null && + typeof block === "object" && + (block as { type?: unknown }).type === "toolCall" && + typeof (block as { id?: unknown }).id === "string" + ); +} + +/** Read the native Bash command off a single validated tool-call block. */ +function extractBashCommand(block: ToolCallBlock): string | undefined { + if (block.name !== NATIVE_BASH_TOOL_NAME) { + return undefined; + } + const command = ( + block.arguments as { command?: unknown } | null | undefined + )?.command; + return typeof command === "string" ? command : undefined; +} + +/** + * Recover the complete native Bash command for one tool call. + * + * Walks session entries, finds the assistant `toolCall` block whose `id` equals + * `toolCallId`, and reads its structured `arguments.command`. Proceeds only + * when, per ADR 0001: + * + * - exactly one block matches the id (no duplicate), + * - that block names the native Bash tool, + * - `arguments.command` is a string. + * + * Any other outcome — no match, duplicate id, a non-Bash tool call, a + * non-string command, or malformed entries — returns `undefined` so the caller + * defers fail-closed. + * + * @returns the complete Bash command, or `undefined`. + */ +export function recoverNativeBashCommand( + entries: ReadonlyArray, + toolCallId: string, +): string | undefined { + let matches = 0; + let command: string | undefined; + + for (const entry of entries) { + if (entry.type !== "message") { + continue; + } + const message = entry.message as { role?: unknown; content?: unknown }; + if (message.role !== "assistant") { + continue; + } + const content = message.content; + if (!Array.isArray(content)) { + continue; + } + for (const block of content) { + if (!isToolCallBlock(block) || block.id !== toolCallId) { + continue; + } + matches += 1; + // Keep walking the whole session so a duplicate id (two matching + // blocks) is detected even when the first match was unusable. + if (matches === 1) { + command = extractBashCommand(block); + } + } + } + + return matches === 1 ? command : undefined; +} diff --git a/packages/pi-permission-inner-cmd/test/authorizer.test.ts b/packages/pi-permission-inner-cmd/test/authorizer.test.ts new file mode 100644 index 0000000..acc009e --- /dev/null +++ b/packages/pi-permission-inner-cmd/test/authorizer.test.ts @@ -0,0 +1,449 @@ +import { describe, expect, it } from "vitest"; +import type { SessionEntry } from "@earendil-works/pi-coding-agent"; +import type { + AuthorizerLog, + PermissionCheckResult, + PermissionQuery, + PermissionState, + PromptPermissionDetails, +} from "@gotgenes/pi-permission-system"; +import { authorizeInnerCommand, type SessionProbe } from "../src/authorizer"; + +/** Default captured root-session identity used by the harness. */ +const ROOT_SESSION_ID = "session-root"; + +type LogCall = { + level: "review" | "debug"; + event: string; + details?: Record; +}; + +function makeLog(): { log: AuthorizerLog; calls: LogCall[] } { + const calls: LogCall[] = []; + const log: AuthorizerLog = { + review: (event, details) => calls.push({ level: "review", event, details }), + debug: (event, details) => calls.push({ level: "debug", event, details }), + }; + return { log, calls }; +} + +type CheckCall = { + surface: string; + value: string | undefined; + agentName: string | undefined; +}; + +function makeQuery( + states: Record, + opts: { throwOn?: string } = {}, +): { query: PermissionQuery; calls: CheckCall[] } { + const calls: CheckCall[] = []; + const query: PermissionQuery = { + checkPermission: (surface, value, agentName) => { + calls.push({ surface, value, agentName }); + if (opts.throwOn !== undefined && value === opts.throwOn) { + throw new Error("policy boom"); + } + const state: PermissionState = states[value ?? ""] ?? "ask"; + const result: PermissionCheckResult = { + toolName: "bash", + state, + source: "bash", + origin: "builtin", + }; + return result; + }, + getToolPermission: () => "ask", + }; + return { query, calls }; +} + +function assistantEntry(content: unknown[]): SessionEntry { + return { + type: "message", + id: "entry-1", + parentId: null, + timestamp: "2026-08-08T00:00:00.000Z", + message: { role: "assistant", content }, + } as unknown as SessionEntry; +} + +function bashToolCall(id: string, command: unknown): Record { + return { type: "toolCall", id, name: "bash", arguments: { command } }; +} + +function entriesRecovering(command: string, toolCallId = "call_1"): SessionEntry[] { + return [assistantEntry([bashToolCall(toolCallId, command)])]; +} + +function bashDetails( + toolCallId = "call_1", + agentName: string | null = null, +): PromptPermissionDetails { + return { + requestId: "req-1", + source: "tool_call", + agentName, + message: "May I run bash?", + toolCallId, + toolName: "bash", + // details.command is intentionally the winning unit, not the full input. + command: "ignored-winning-unit", + }; +} + +function makeSessionProbe(args: { + recoveredCommand: string; + toolCallId: string; + getEntriesThrows?: boolean; + /** Live session id reported at authorize time. */ + sessionId?: string; + getSessionIdThrows?: boolean; +}): SessionProbe { + return { + getEntries: args.getEntriesThrows + ? (): SessionEntry[] => { + throw new Error("session boom"); + } + : (): SessionEntry[] => + entriesRecovering(args.recoveredCommand, args.toolCallId), + getSessionId: args.getSessionIdThrows + ? (): string => { + throw new Error("session id boom"); + } + : (): string => args.sessionId ?? ROOT_SESSION_ID, + }; +} + +async function run(args: { + recoveredCommand: string; + states?: Record; + details?: Partial; + getEntriesThrows?: boolean; + queryThrowsOn?: string; + /** Live session id diverges from the captured provenance. */ + sessionMismatch?: boolean; + getSessionIdThrows?: boolean; +}): Promise<{ + verdict: { kind: string }; + log: LogCall[]; + check: CheckCall[]; +}> { + const { log, calls } = makeLog(); + const toolCallId = args.details?.toolCallId ?? "call_1"; + const { query, calls: check } = makeQuery(args.states ?? {}, { + throwOn: args.queryThrowsOn, + }); + const session = makeSessionProbe({ + recoveredCommand: args.recoveredCommand, + toolCallId, + getEntriesThrows: args.getEntriesThrows, + getSessionIdThrows: args.getSessionIdThrows, + sessionId: args.sessionMismatch ? "session-changed" : ROOT_SESSION_ID, + }); + const verdict = await authorizeInnerCommand({ + details: { ...bashDetails(toolCallId), ...args.details } as PromptPermissionDetails, + query, + log, + session, + expectedSessionId: ROOT_SESSION_ID, + }); + return { verdict: { kind: verdict.kind }, log: calls, check }; +} + +describe("authorizeInnerCommand — recognized wrapper verdicts", () => { + it("maps an inner allow to allow and records a review", async () => { + const { verdict, log, check } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + }); + expect(verdict.kind).toBe("allow"); + expect(log).toEqual([ + { + level: "review", + event: "inner_cmd.allow", + details: { + command: "timeout 30s pnpm test", + innerCommand: "pnpm test", + }, + }, + ]); + expect(check).toEqual([ + { surface: "bash", value: "pnpm test", agentName: undefined }, + ]); + }); + + it("maps an inner ask to defer and records a debug", async () => { + const { verdict, log } = await run({ + recoveredCommand: "timeout 30s git push", + states: { "git push": "ask" }, + }); + expect(verdict.kind).toBe("defer"); + expect(log).toEqual([ + { + level: "debug", + event: "inner_cmd.inner_ask", + details: { + command: "timeout 30s git push", + innerCommand: "git push", + }, + }, + ]); + }); + + it("maps an inner deny to deny and records a review", async () => { + const { verdict, log } = await run({ + recoveredCommand: "timeout 30s rm -rf /", + states: { "rm -rf /": "deny" }, + }); + expect(verdict.kind).toBe("deny"); + expect(log).toEqual([ + { + level: "review", + event: "inner_cmd.deny", + details: { + command: "timeout 30s rm -rf /", + innerCommand: "rm -rf /", + }, + }, + ]); + }); + + it("re-checks the complete inner program for compound input", async () => { + // timeout 60s pnpm test && git push -> inner "pnpm test && git push". + // The whole program must be re-evaluated; git push asking defers it. + const { verdict, log, check } = await run({ + recoveredCommand: "timeout 60s pnpm test && git push", + states: { "pnpm test && git push": "ask" }, + }); + expect(verdict.kind).toBe("defer"); + expect(check).toEqual([ + { + surface: "bash", + value: "pnpm test && git push", + agentName: undefined, + }, + ]); + expect(log).toEqual([ + { + level: "debug", + event: "inner_cmd.inner_ask", + details: { + command: "timeout 60s pnpm test && git push", + innerCommand: "pnpm test && git push", + }, + }, + ]); + }); + + it("unwraps timeout around bash -c and re-evaluates the inner program", async () => { + const { verdict, log, check } = await run({ + recoveredCommand: "timeout 30s bash -c something", + states: { "bash -c something": "ask" }, + }); + expect(verdict.kind).toBe("defer"); + expect(check).toEqual([ + { surface: "bash", value: "bash -c something", agentName: undefined }, + ]); + expect(log[0]?.event).toBe("inner_cmd.inner_ask"); + }); + + it("forwards details.agentName ?? undefined into the inner query", async () => { + const { check } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + details: { agentName: "release-worker" }, + }); + expect(check).toEqual([ + { surface: "bash", value: "pnpm test", agentName: "release-worker" }, + ]); + }); + + it("passes undefined when details.agentName is null", async () => { + const { check } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + details: { agentName: null }, + }); + expect(check[0]?.agentName).toBeUndefined(); + }); +}); + +describe("authorizeInnerCommand — root-ownership revalidation", () => { + it("defers fail-closed when the live session id no longer matches", async () => { + const { verdict, log, check } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + sessionMismatch: true, + }); + expect(verdict.kind).toBe("defer"); + // Never reaches recovery or the decisive deterministic query. + expect(check).toEqual([]); + expect(log).toEqual([ + { + level: "debug", + event: "inner_cmd.session_mismatch", + details: { + expectedSessionId: ROOT_SESSION_ID, + currentSessionId: "session-changed", + }, + }, + ]); + }); + + it("still defers a forwarded ask before revalidating ownership", async () => { + const { verdict, log } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + sessionMismatch: true, + details: { + forwarding: { requesterAgentName: "child", requesterSessionId: "s1" }, + }, + }); + expect(verdict.kind).toBe("defer"); + expect(log).toEqual([]); // forwarded defers silently, before any session read + }); +}); + +describe("authorizeInnerCommand — fail-closed deferrals", () => { + it("defers on unsupported timeout syntax with a debug log", async () => { + const { verdict, log } = await run({ + recoveredCommand: "timeout -k 5s 30s pnpm test", + states: { "pnpm test": "allow" }, + }); + expect(verdict.kind).toBe("defer"); + expect(log).toEqual([ + { + level: "debug", + event: "inner_cmd.unsupported_timeout_syntax", + details: { command: "timeout -k 5s 30s pnpm test" }, + }, + ]); + }); + + it("defers on a nested wrapper with a debug log", async () => { + const { verdict, log, check } = await run({ + recoveredCommand: "timeout 30s timeout 10s pnpm test", + states: { "timeout 10s pnpm test": "allow" }, + }); + expect(verdict.kind).toBe("defer"); + expect(check).toEqual([]); // inner program is never re-evaluated + expect(log).toEqual([ + { + level: "debug", + event: "inner_cmd.nested_timeout", + details: { + command: "timeout 30s timeout 10s pnpm test", + innerCommand: "timeout 10s pnpm test", + }, + }, + ]); + }); + + it("defers silently on an ordinary non-timeout command", async () => { + const { verdict, log } = await run({ + recoveredCommand: "pnpm test", + states: { "pnpm test": "allow" }, + }); + expect(verdict.kind).toBe("defer"); + expect(log).toEqual([]); + }); + + it("defers silently on a forwarded request", async () => { + const { verdict, log, check } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + details: { + forwarding: { requesterAgentName: "child", requesterSessionId: "s1" }, + }, + }); + expect(verdict.kind).toBe("defer"); + expect(log).toEqual([]); + expect(check).toEqual([]); // never reaches the deterministic query + }); + + it("defers silently for a non-Bash tool", async () => { + const { verdict, log } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + details: { toolName: "read" }, + }); + expect(verdict.kind).toBe("defer"); + expect(log).toEqual([]); + }); + + it("defers silently when toolCallId is absent", async () => { + const { verdict, log } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + details: { toolCallId: undefined }, + }); + expect(verdict.kind).toBe("defer"); + expect(log).toEqual([]); + }); + + it("defers silently when the tool call is not in the session", async () => { + // Recover a command under a different id so recovery misses. + const { log, calls } = makeLog(); + const { query, calls: check } = makeQuery({ "pnpm test": "allow" }); + const verdict = await authorizeInnerCommand({ + details: bashDetails("call_missing"), + query, + log, + session: makeSessionProbe({ + recoveredCommand: "timeout 30s pnpm test", + toolCallId: "call_1", + }), + expectedSessionId: ROOT_SESSION_ID, + }); + expect(verdict.kind).toBe("defer"); + expect(calls).toEqual([]); + expect(check).toEqual([]); + }); +}); + +describe("authorizeInnerCommand — exceptions defer with a debug log", () => { + it("defers when reading the session id throws (logs only safe data)", async () => { + const { verdict, log } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + getSessionIdThrows: true, + }); + expect(verdict.kind).toBe("defer"); + expect(log).toHaveLength(1); + expect(log[0]?.event).toBe("inner_cmd.exception"); + // Exception before recognition: no command/innerCommand available. + expect(log[0]?.details).toEqual({ error: "session id boom" }); + }); + + it("defers when reading the session throws (logs only safe data)", async () => { + const { verdict, log } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + getEntriesThrows: true, + }); + expect(verdict.kind).toBe("defer"); + expect(log).toHaveLength(1); + expect(log[0]?.level).toBe("debug"); + expect(log[0]?.event).toBe("inner_cmd.exception"); + // Exception before recognition: only the error is available. + expect(log[0]?.details).toEqual({ error: "session boom" }); + }); + + it("retains command and innerCommand when the query throws after recognition", async () => { + const { verdict, log } = await run({ + recoveredCommand: "timeout 30s pnpm test", + states: { "pnpm test": "allow" }, + queryThrowsOn: "pnpm test", + }); + expect(verdict.kind).toBe("defer"); + expect(log).toHaveLength(1); + expect(log[0]?.event).toBe("inner_cmd.exception"); + // Exception after recognition: command + innerCommand retained. + expect(log[0]?.details).toEqual({ + error: "policy boom", + command: "timeout 30s pnpm test", + innerCommand: "pnpm test", + }); + }); +}); diff --git a/packages/pi-permission-inner-cmd/test/lifecycle.test.ts b/packages/pi-permission-inner-cmd/test/lifecycle.test.ts new file mode 100644 index 0000000..818d560 --- /dev/null +++ b/packages/pi-permission-inner-cmd/test/lifecycle.test.ts @@ -0,0 +1,253 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { + ExtensionAPI, + ExtensionContext, + SessionStartEvent, + SessionShutdownEvent, +} from "@earendil-works/pi-coding-agent"; +import extension from "../src/index"; +import { + PERMISSIONS_READY_CHANNEL, + publishPermissionsService, + unpublishPermissionsService, + type Authorizer, + type PermissionsService, +} from "@gotgenes/pi-permission-system"; + +/** + * Minimal fake ExtensionAPI that records the lifecycle handlers the extension + * uses. Cast to ExtensionAPI because the factory only touches a small surface. + */ +function createFakePi(): { + pi: ExtensionAPI; + fireSessionStart: ( + sessionManager: ExtensionContext["sessionManager"], + hasUI?: boolean, + ) => void; + fireSessionShutdown: () => void; + readyHandlers: Array<() => unknown>; +} { + const sessionStartHandlers: Array< + (event: SessionStartEvent, ctx: ExtensionContext) => unknown + > = []; + const shutdownHandlers: Array<(event: SessionShutdownEvent) => unknown> = []; + const readyHandlers: Array<() => unknown> = []; + + const pi = { + on(event: string, handler: (...args: never[]) => unknown): void { + if (event === "session_start") sessionStartHandlers.push(handler as never); + else if (event === "session_shutdown") + shutdownHandlers.push(handler as never); + }, + events: { + on(channel: string, handler: (...args: never[]) => unknown): void { + if (channel === PERMISSIONS_READY_CHANNEL) + readyHandlers.push(handler as never); + }, + }, + } as unknown as ExtensionAPI; + + return { + pi, + fireSessionStart: (sessionManager, hasUI = true) => { + const ctx = { sessionManager, hasUI } as unknown as ExtensionContext; + const event = { type: "session_start", reason: "startup" } as SessionStartEvent; + for (const handler of sessionStartHandlers) handler(event, ctx); + }, + fireSessionShutdown: () => { + const event = { type: "session_shutdown" } as SessionShutdownEvent; + for (const handler of shutdownHandlers) handler(event); + }, + readyHandlers, + }; +} + +/** + * A fake session manager whose entries and identity can be inspected for + * assertions. Defaults to a UI-root-shaped non-empty session id. + */ +function createFakeSessionManager( + entries: unknown[] = [], + sessionId = "session-root", +) { + return { + getEntries: () => entries, + getSessionId: () => sessionId, + } as unknown as ExtensionContext["sessionManager"]; +} + +describe("permissions:ready -> registerAuthorizer lifecycle", () => { + let registerAuthorizer: ReturnType; + let disposer: ReturnType; + let service: PermissionsService; + let authorize: Authorizer["authorize"] | undefined; + let published: boolean; + + beforeEach(() => { + disposer = vi.fn(); + authorize = undefined; + registerAuthorizer = vi.fn((name, callback) => { + authorize = callback; + return disposer; + }); + service = { + registerAuthorizer, + checkPermission: () => ({ + toolName: "bash", + state: "ask", + source: "bash", + origin: "builtin", + }), + getToolPermission: () => "ask", + } as unknown as PermissionsService; + published = false; + }); + + afterEach(() => { + if (published) unpublishPermissionsService(service); + }); + + /** + * Model the permission system becoming ready: it publishes its service and + * then emits the `permissions:ready` channel. Before this, no service is + * available, so an early `session_start` cannot register yet. + */ + function becomeReady(): void { + publishPermissionsService(service); + published = true; + } + + it("waits for the service: session_start alone does not register", () => { + const { pi, fireSessionStart } = createFakePi(); + extension(pi); + + fireSessionStart(createFakeSessionManager()); + expect(registerAuthorizer).not.toHaveBeenCalled(); + }); + + it("registers once the service becomes ready after session_start", () => { + const { pi, fireSessionStart, readyHandlers } = createFakePi(); + extension(pi); + + fireSessionStart(createFakeSessionManager()); + expect(registerAuthorizer).not.toHaveBeenCalled(); + + becomeReady(); + for (const handler of readyHandlers) handler(); + expect(registerAuthorizer).toHaveBeenCalledTimes(1); + expect(registerAuthorizer).toHaveBeenCalledWith( + "inner-cmd", + expect.any(Function), + ); + }); + + it("registers immediately at session_start when the service is already ready", () => { + const { pi, fireSessionStart } = createFakePi(); + extension(pi); + + becomeReady(); + fireSessionStart(createFakeSessionManager()); + expect(registerAuthorizer).toHaveBeenCalledTimes(1); + }); + + it("needs a session: ready without session_start does not register", () => { + const { pi, readyHandlers } = createFakePi(); + extension(pi); + + becomeReady(); + for (const handler of readyHandlers) handler(); + expect(registerAuthorizer).not.toHaveBeenCalled(); + }); + + it("does not register from a headless (hasUI=false) session_start", () => { + // An in-process/headless child can resolve the published parent service + // but must never register with child-captured context. + const { pi, fireSessionStart, readyHandlers } = createFakePi(); + extension(pi); + + fireSessionStart(createFakeSessionManager(), false); + becomeReady(); + for (const handler of readyHandlers) handler(); + expect(registerAuthorizer).not.toHaveBeenCalled(); + }); + + it("does not register when the captured session id is empty", () => { + // Without a non-empty identity snapshot there is no provenance to + // revalidate at authorize time, so registration is refused. + const { pi, fireSessionStart, readyHandlers } = createFakePi(); + extension(pi); + + fireSessionStart(createFakeSessionManager([], "")); + becomeReady(); + for (const handler of readyHandlers) handler(); + expect(registerAuthorizer).not.toHaveBeenCalled(); + }); + + it("does not re-register on a second readiness signal", () => { + const { pi, fireSessionStart, readyHandlers } = createFakePi(); + extension(pi); + + fireSessionStart(createFakeSessionManager()); + becomeReady(); + for (const handler of readyHandlers) handler(); + for (const handler of readyHandlers) handler(); + + expect(registerAuthorizer).toHaveBeenCalledTimes(1); + }); + + it("disposes the authorizer on session_shutdown and re-registers after", () => { + const { pi, fireSessionStart, fireSessionShutdown, readyHandlers } = + createFakePi(); + extension(pi); + + fireSessionStart(createFakeSessionManager()); + becomeReady(); + for (const handler of readyHandlers) handler(); + expect(disposer).not.toHaveBeenCalled(); + + fireSessionShutdown(); + expect(disposer).toHaveBeenCalledTimes(1); + + // A fresh session cycle registers again. + fireSessionStart(createFakeSessionManager()); + for (const handler of readyHandlers) handler(); + expect(registerAuthorizer).toHaveBeenCalledTimes(2); + }); + + it("registers a callback that defers forwarded asks fail-closed", async () => { + const { pi, fireSessionStart, readyHandlers } = createFakePi(); + extension(pi); + + fireSessionStart(createFakeSessionManager()); + becomeReady(); + for (const handler of readyHandlers) handler(); + expect(authorize).toBeDefined(); + + const verdict = await authorize!( + { + requestId: "req-1", + source: "tool_call", + agentName: "child", + message: "forwarded ask", + toolCallId: "call_1", + toolName: "bash", + forwarding: { + requesterAgentName: "child", + requesterSessionId: "s1", + }, + }, + { + checkPermission: () => ({ + toolName: "bash", + state: "allow", + source: "bash", + origin: "builtin", + }), + getToolPermission: () => "allow", + }, + { review: () => {}, debug: () => {} }, + ); + + expect(verdict).toEqual({ kind: "defer" }); + }); +}); diff --git a/packages/pi-permission-inner-cmd/test/recognizer.test.ts b/packages/pi-permission-inner-cmd/test/recognizer.test.ts new file mode 100644 index 0000000..f943f44 --- /dev/null +++ b/packages/pi-permission-inner-cmd/test/recognizer.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, it } from "vitest"; +import { + classifyWrapper, + isRecognizedWrapper, + parseTimeoutWrapper, +} from "../src/recognizer"; + +describe("parseTimeoutWrapper", () => { + it("matches the strict simple-timeout form", () => { + expect(parseTimeoutWrapper("timeout 30s pnpm test")).toEqual({ + duration: "30s", + innerCommand: "pnpm test", + }); + expect(parseTimeoutWrapper("timeout 1m echo hi")).toEqual({ + duration: "1m", + innerCommand: "echo hi", + }); + expect(parseTimeoutWrapper("timeout 5h deploy")).toEqual({ + duration: "5h", + innerCommand: "deploy", + }); + expect(parseTimeoutWrapper("timeout 2d longjob")).toEqual({ + duration: "2d", + innerCommand: "longjob", + }); + }); + + it("preserves compound inner programs as the inner command", () => { + expect(parseTimeoutWrapper("timeout 60s pnpm test && git push")).toEqual({ + duration: "60s", + innerCommand: "pnpm test && git push", + }); + expect(parseTimeoutWrapper("timeout 30s bash -c something")).toEqual({ + duration: "30s", + innerCommand: "bash -c something", + }); + }); + + it("accepts tab-separated and multi-space arguments", () => { + expect(parseTimeoutWrapper("timeout\t30s\tpnpm test")).toEqual({ + duration: "30s", + innerCommand: "pnpm test", + }); + expect(parseTimeoutWrapper("timeout 10s build")).toEqual({ + duration: "10s", + innerCommand: "build", + }); + }); + + it("rejects leading-zero and multi-letter durations", () => { + expect(parseTimeoutWrapper("timeout 0s pnpm test")).toBeUndefined(); + expect(parseTimeoutWrapper("timeout 030s pnpm test")).toBeUndefined(); + expect(parseTimeoutWrapper("timeout 30ms pnpm test")).toBeUndefined(); + expect(parseTimeoutWrapper("timeout 30sec pnpm test")).toBeUndefined(); + }); + + it("rejects unsupported timeout syntax", () => { + expect(parseTimeoutWrapper("timeout -k 5s 30s pnpm test")).toBeUndefined(); + expect(parseTimeoutWrapper("timeout --preserve-status 30s pnpm test")).toBeUndefined(); + expect(parseTimeoutWrapper("timeout -- 30s pnpm test")).toBeUndefined(); + expect(parseTimeoutWrapper("timeout 30s")).toBeUndefined(); + expect(parseTimeoutWrapper("timeout")).toBeUndefined(); + }); + + it("does not match commands that merely contain timeout", () => { + expect(parseTimeoutWrapper("pnpm test")).toBeUndefined(); + expect(parseTimeoutWrapper("timeout30s pnpm test")).toBeUndefined(); + expect(parseTimeoutWrapper("my-timeout 30s pnpm test")).toBeUndefined(); + }); +}); + +describe("isRecognizedWrapper", () => { + it("is true for the strict form and false otherwise", () => { + expect(isRecognizedWrapper("timeout 10s pnpm test")).toBe(true); + expect(isRecognizedWrapper("timeout 10s timeout 5s pnpm test")).toBe(true); + expect(isRecognizedWrapper("pnpm test")).toBe(false); + expect(isRecognizedWrapper("timeout -k 5s 30s pnpm test")).toBe(false); + }); +}); + +describe("classifyWrapper", () => { + it("classifies the recognized wrapper", () => { + expect(classifyWrapper("timeout 30s pnpm test")).toEqual({ + kind: "recognized", + match: { duration: "30s", innerCommand: "pnpm test" }, + }); + }); + + it("classifies unsupported timeout syntax", () => { + expect(classifyWrapper("timeout -k 5s 30s pnpm test").kind).toBe( + "unsupportedTimeout", + ); + expect(classifyWrapper("timeout 30s").kind).toBe("unsupportedTimeout"); + expect(classifyWrapper("timeout --help").kind).toBe("unsupportedTimeout"); + }); + + it("classifies ordinary commands as non-timeout", () => { + expect(classifyWrapper("pnpm test").kind).toBe("nonTimeout"); + expect(classifyWrapper("rm -rf /").kind).toBe("nonTimeout"); + expect(classifyWrapper("git push").kind).toBe("nonTimeout"); + }); +}); diff --git a/packages/pi-permission-inner-cmd/test/recovery.test.ts b/packages/pi-permission-inner-cmd/test/recovery.test.ts new file mode 100644 index 0000000..67d22ba --- /dev/null +++ b/packages/pi-permission-inner-cmd/test/recovery.test.ts @@ -0,0 +1,172 @@ +import { describe, expect, it } from "vitest"; +import type { SessionEntry } from "@earendil-works/pi-coding-agent"; +import { recoverNativeBashCommand } from "../src/recovery"; + +/** Build a minimal assistant message entry carrying the given content blocks. */ +function assistantEntry(content: unknown[]): SessionEntry { + return { + type: "message", + id: "entry-1", + parentId: null, + timestamp: "2026-08-08T00:00:00.000Z", + message: { + role: "assistant", + content, + }, + } as unknown as SessionEntry; +} + +/** A user message entry, to confirm non-assistant entries are ignored. */ +function userEntry(): SessionEntry { + return { + type: "message", + id: "entry-user", + parentId: null, + timestamp: "2026-08-08T00:00:00.000Z", + message: { role: "user", content: "hello" }, + } as unknown as SessionEntry; +} + +/** A tool-result message entry, ignored by recovery. */ +function toolResultEntry(): SessionEntry { + return { + type: "message", + id: "entry-tool-result", + parentId: null, + timestamp: "2026-08-08T00:00:00.000Z", + message: { + role: "toolResult", + toolCallId: "call_1", + toolName: "bash", + content: [], + isError: false, + timestamp: 0, + }, + } as unknown as SessionEntry; +} + +/** A non-message entry (compaction), ignored by recovery. */ +function compactionEntry(): SessionEntry { + return { + type: "compaction", + id: "entry-compaction", + parentId: null, + timestamp: "2026-08-08T00:00:00.000Z", + summary: "...", + firstKeptEntryId: "x", + tokensBefore: 0, + } as unknown as SessionEntry; +} + +function toolCall( + id: string, + name: string, + args: Record, +): Record { + return { type: "toolCall", id, name, arguments: args }; +} + +function bashToolCall(id: string, command: unknown): Record { + return { type: "toolCall", id, name: "bash", arguments: { command } }; +} + +describe("recoverNativeBashCommand", () => { + it("returns the command for a single native Bash tool call", () => { + const entries = [ + userEntry(), + assistantEntry([ + { type: "text", text: "running tests" }, + bashToolCall("call_1", "timeout 30s pnpm test"), + ]), + ]; + expect(recoverNativeBashCommand(entries, "call_1")).toBe( + "timeout 30s pnpm test", + ); + }); + + it("finds the matching tool call among several with different ids", () => { + const entries = [ + assistantEntry([ + bashToolCall("call_a", "pnpm build"), + bashToolCall("call_b", "timeout 30s pnpm test"), + ]), + ]; + expect(recoverNativeBashCommand(entries, "call_b")).toBe( + "timeout 30s pnpm test", + ); + }); + + it("ignores user, tool-result, and non-message entries", () => { + const entries = [ + compactionEntry(), + userEntry(), + toolResultEntry(), + assistantEntry([bashToolCall("call_1", "echo hi")]), + ]; + expect(recoverNativeBashCommand(entries, "call_1")).toBe("echo hi"); + }); + + it("returns undefined when no tool call matches the id", () => { + const entries = [assistantEntry([bashToolCall("call_1", "echo hi")])]; + expect(recoverNativeBashCommand(entries, "call_missing")).toBeUndefined(); + }); + + it("returns undefined on a duplicate id (cannot prove authority)", () => { + const entries = [ + assistantEntry([ + bashToolCall("call_1", "timeout 30s pnpm test"), + bashToolCall("call_1", "timeout 30s rm -rf /"), + ]), + ]; + expect(recoverNativeBashCommand(entries, "call_1")).toBeUndefined(); + }); + + it("returns undefined when the only match is a non-Bash tool", () => { + const entries = [ + assistantEntry([ + toolCall("call_1", "read", { path: "/etc/passwd" }), + ]), + ]; + expect(recoverNativeBashCommand(entries, "call_1")).toBeUndefined(); + }); + + it("returns undefined when arguments.command is not a string", () => { + const entries = [ + assistantEntry([bashToolCall("call_1", 12345)]), + ]; + expect(recoverNativeBashCommand(entries, "call_1")).toBeUndefined(); + }); + + it("returns undefined when arguments.command is missing", () => { + const entries = [ + assistantEntry([ + toolCall("call_1", "bash", { timeout: 30 }), + ]), + ]; + expect(recoverNativeBashCommand(entries, "call_1")).toBeUndefined(); + }); + + it("returns undefined for a duplicate id even when the first match is invalid", () => { + const entries = [ + assistantEntry([ + toolCall("call_1", "read", { path: "/a" }), + bashToolCall("call_1", "timeout 30s pnpm test"), + ]), + ]; + expect(recoverNativeBashCommand(entries, "call_1")).toBeUndefined(); + }); + + it("tolerates a malformed content block that is not a tool call", () => { + const entries = [ + assistantEntry([ + { type: "text", text: "thinking..." }, + null, + { type: "thinking", thinking: "..." }, + bashToolCall("call_1", "timeout 30s pnpm test"), + ]), + ]; + expect(recoverNativeBashCommand(entries, "call_1")).toBe( + "timeout 30s pnpm test", + ); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 198f68d..e1f524e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -234,8 +234,11 @@ importers: specifier: '*' version: 0.84.1(ws@8.21.2)(zod@4.4.3) '@gotgenes/pi-permission-system': - specifier: '>=20.10.0' + specifier: '>=24.0.0' version: 24.0.0(@earendil-works/pi-coding-agent@0.84.1(ws@8.21.2)(zod@4.4.3))(@earendil-works/pi-tui@0.84.1) + '@types/node': + specifier: ^26.0.0 + version: 26.1.2 typescript: specifier: ^5 version: 5.9.3