fix: deny-by-default when ctx.ui.custom() returns undefined in RPC mode

.changeset/fix-rpc-permission-gate-bypass.md

@@ -0,0 +1,5 @@
+---
+"@aliou/pi-guardrails": patch
+---
+
+Fix permission gate bypass in RPC mode: deny-by-default when `ctx.ui.custom()` returns undefined, with fallback to `ctx.ui.select()`.

AGENTS.md

@@ -8,12 +8,15 @@ Pi is pre-1.0.0, so breaking changes can happen between Pi versions. This extens
 
 - TypeScript (strict mode)
 - pnpm 10.26.1
+- Vitest for testing
 - Biome for linting/formatting
 - Changesets for versioning
 
 ## Scripts
 
 ```bash
+pnpm test         # Run tests
+pnpm test:watch   # Run tests in watch mode
 pnpm typecheck    # Type check
 pnpm lint         # Lint (runs on pre-commit)
 pnpm format       # Format
@@ -31,10 +34,19 @@ src/
   components/         # UI components (pattern editor)
   lib/                # Vendored subagent executor core (Phase 1)
   utils/              # Helpers (matching, glob expansion, migration, shell AST)
+tests/
+  utils/              # Test harness utilities (adapted from pi-harness)
+    pi-context.ts     # Spy-based ExtensionContext / UI context builders
+    pi-test-harness.ts # Full extension loader with emitEvent() for hook testing
+    load-extension.ts # Wrapper for Pi internal extension loader
+    matchers.ts       # Custom vitest matchers (toHaveRegisteredTool, etc.)
 ```
 
 ## Conventions
 
+- Tests live next to the code they test (`src/hooks/foo.test.ts`)
+- Hook tests use `setupXxxHook()` directly with a mock `pi` and spy contexts from `tests/utils/pi-context.ts`, rather than loading the full extension (avoids `configLoader` side effects)
+- The full `createPiTestHarness()` is available for testing commands and tools that go through the extension factory
 - New hooks: follow patterns in `src/hooks/`
 - Built-in dangerous command matching uses AST parsing via `@aliou/sh`; user-configured patterns use substring/regex matching
 - File protection is policy-based (`features.policies`, `policies.rules`), not legacy `envFiles`

README.md

@@ -192,6 +192,16 @@ Also note:
 
 - `preventBrew`, `preventPython`, `enforcePackageManager`, `packageManager` were removed from guardrails and moved to `@aliou/pi-toolchain`.
 
+## Development
+
+```bash
+pnpm test         # Run tests
+pnpm test:watch   # Run tests in watch mode
+pnpm typecheck    # Type check
+pnpm lint         # Lint
+pnpm format       # Format
+```
+
 ## Events
 
 Guardrails emits events for other extensions:

src/hooks/permission-gate.test.ts

@@ -0,0 +1,332 @@
+import type {
+  BashToolCallEvent,
+  ExtensionAPI,
+  ExtensionContext,
+} from "@mariozechner/pi-coding-agent";
+import { createEventBus } from "@mariozechner/pi-coding-agent";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { createEventContext } from "../../tests/utils/pi-context";
+import type { ResolvedConfig } from "../config";
+import { configLoader } from "../config";
+import { setupPermissionGateHook } from "./permission-gate";
+
+// Mock configLoader so allow-session path doesn't throw.
+vi.mock("../config", async (importOriginal) => {
+  const original = (await importOriginal()) as Record<string, unknown>;
+  return {
+    ...original,
+    configLoader: {
+      getConfig: vi.fn(() => ({
+        permissionGate: { allowedPatterns: [] },
+      })),
+      save: vi.fn(async () => {}),
+    },
+  };
+});
+
+// ---------------------------------------------------------------------------
+// Constants — must match the production code's SELECT_* constants
+// ---------------------------------------------------------------------------
+
+const SELECT_ALLOW_ONCE = "Allow once";
+const SELECT_ALLOW_SESSION = "Allow for session";
+const SELECT_DENY = "Deny";
+
+// ---------------------------------------------------------------------------
+// Helpers
+// ---------------------------------------------------------------------------
+
+/**
+ * Minimal config enabling the permission gate with defaults.
+ * No custom patterns — relies on built-in structural matchers.
+ */
+function makeConfig(
+  overrides: Partial<ResolvedConfig["permissionGate"]> = {},
+): ResolvedConfig {
+  return {
+    version: "1",
+    enabled: true,
+    applyBuiltinDefaults: true,
+    features: { policies: false, permissionGate: true, pathAccess: false },
+    policies: { rules: [] },
+    pathAccess: { mode: "ask", allowedPaths: [] },
+    permissionGate: {
+      patterns: [],
+      useBuiltinMatchers: true,
+      requireConfirmation: true,
+      allowedPatterns: [],
+      autoDenyPatterns: [],
+      explainCommands: false,
+      explainModel: null,
+      explainTimeout: 5000,
+      ...overrides,
+    },
+  };
+}
+
+type ToolCallHandler = (
+  event: BashToolCallEvent,
+  ctx: ExtensionContext,
+) => Promise<{ block: true; reason: string } | undefined>;
+
+/**
+ * Create a mock ExtensionAPI that captures tool_call handler registrations.
+ * Returns the mock and a function to retrieve the registered handler.
+ */
+function createMockPi() {
+  const handlers: ToolCallHandler[] = [];
+  const eventBus = createEventBus();
+
+  const pi = {
+    on(event: string, handler: ToolCallHandler) {
+      if (event === "tool_call") {
+        handlers.push(handler);
+      }
+    },
+    events: eventBus,
+    // Stubs for any other ExtensionAPI methods that might be called.
+    registerCommand: vi.fn(),
+    registerTool: vi.fn(),
+    emit: vi.fn(),
+  } as unknown as ExtensionAPI;
+
+  return {
+    pi,
+    getHandler(): ToolCallHandler {
+      if (handlers.length === 0) {
+        throw new Error("No tool_call handler registered");
+      }
+      return handlers[0];
+    },
+  };
+}
+
+function bashEvent(command: string): BashToolCallEvent {
+  return {
+    type: "tool_call",
+    toolCallId: "tc_test",
+    toolName: "bash",
+    input: { command },
+  };
+}
+
+// ---------------------------------------------------------------------------
+// Tests
+// ---------------------------------------------------------------------------
+
+describe("permission gate", () => {
+  let handle: ReturnType<typeof createMockPi>;
+  let handler: ToolCallHandler;
+
+  beforeEach(() => {
+    handle = createMockPi();
+    setupPermissionGateHook(handle.pi, makeConfig());
+    handler = handle.getHandler();
+  });
+
+  it("allows safe commands", async () => {
+    const ctx = createEventContext({ hasUI: true });
+    const result = await handler(bashEvent("echo hello"), ctx);
+    expect(result).toBeUndefined();
+  });
+
+  it("blocks dangerous commands when user denies", async () => {
+    const ctx = createEventContext({
+      hasUI: true,
+      ui: {
+        custom: vi.fn(async () => "deny") as ExtensionContext["ui"]["custom"],
+      },
+    });
+    const result = await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(result).toEqual({
+      block: true,
+      reason: "User denied dangerous command",
+    });
+  });
+
+  it("allows dangerous commands when user explicitly allows", async () => {
+    const ctx = createEventContext({
+      hasUI: true,
+      ui: {
+        custom: vi.fn(async () => "allow") as ExtensionContext["ui"]["custom"],
+      },
+    });
+    const result = await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(result).toBeUndefined();
+  });
+
+  it("blocks when hasUI is false (print/RPC mode)", async () => {
+    const ctx = createEventContext({ hasUI: false });
+    const result = await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(result).toEqual(expect.objectContaining({ block: true }));
+  });
+
+  it("blocks when ctx.ui.custom() returns undefined (RPC stub)", async () => {
+    // This is the bug from issue #19: in RPC mode, ctx.ui.custom() returns
+    // undefined. The permission gate only checks for "deny", so undefined
+    // falls through and the command is silently allowed.
+    const ctx = createEventContext({
+      hasUI: true,
+      ui: {
+        custom: vi.fn(
+          async () => undefined,
+        ) as ExtensionContext["ui"]["custom"],
+        select: vi.fn(
+          async () => undefined,
+        ) as ExtensionContext["ui"]["select"],
+      },
+    });
+    const result = await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(result).toEqual(expect.objectContaining({ block: true }));
+    expect(ctx.ui.select).toHaveBeenCalled();
+  });
+
+  it("blocks auto-deny patterns without prompting", async () => {
+    const { pi, getHandler } = createMockPi();
+    setupPermissionGateHook(
+      pi,
+      makeConfig({
+        autoDenyPatterns: [{ pattern: "DROP TABLE" }],
+      }),
+    );
+    const h = getHandler();
+    const ctx = createEventContext({ hasUI: true });
+    const result = await h(bashEvent("psql -c 'DROP TABLE users'"), ctx);
+    expect(result).toEqual(expect.objectContaining({ block: true }));
+    // Should not have prompted the user.
+    expect(ctx.ui.custom).not.toHaveBeenCalled();
+  });
+
+  it("skips allowed patterns", async () => {
+    const { pi, getHandler } = createMockPi();
+    setupPermissionGateHook(
+      pi,
+      makeConfig({
+        allowedPatterns: [{ pattern: "sudo echo" }],
+      }),
+    );
+    const h = getHandler();
+    const ctx = createEventContext({ hasUI: true });
+    const result = await h(bashEvent("sudo echo hello"), ctx);
+    expect(result).toBeUndefined();
+  });
+
+  // ---------------------------------------------------------------------------
+  // RPC mode: ctx.ui.select() fallback when ctx.ui.custom() returns undefined
+  // ---------------------------------------------------------------------------
+
+  it("falls back to select() when custom() returns undefined and allows on 'Allow once'", async () => {
+    const ctx = createEventContext({
+      hasUI: true,
+      ui: {
+        custom: vi.fn(
+          async () => undefined,
+        ) as ExtensionContext["ui"]["custom"],
+        select: vi.fn(
+          async () => SELECT_ALLOW_ONCE,
+        ) as ExtensionContext["ui"]["select"],
+      },
+    });
+    const result = await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(result).toBeUndefined(); // not blocked → allowed
+    expect(ctx.ui.select).toHaveBeenCalled();
+  });
+
+  it("falls back to select() when custom() returns undefined and allows-session on 'Allow for session'", async () => {
+    const ctx = createEventContext({
+      hasUI: true,
+      ui: {
+        custom: vi.fn(
+          async () => undefined,
+        ) as ExtensionContext["ui"]["custom"],
+        select: vi.fn(
+          async () => SELECT_ALLOW_SESSION,
+        ) as ExtensionContext["ui"]["select"],
+      },
+    });
+    const result = await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(result).toBeUndefined(); // not blocked → allowed with session grant
+    expect(ctx.ui.select).toHaveBeenCalled();
+  });
+
+  it("falls back to select() when custom() returns undefined and blocks on 'Deny'", async () => {
+    const ctx = createEventContext({
+      hasUI: true,
+      ui: {
+        custom: vi.fn(
+          async () => undefined,
+        ) as ExtensionContext["ui"]["custom"],
+        select: vi.fn(
+          async () => SELECT_DENY,
+        ) as ExtensionContext["ui"]["select"],
+      },
+    });
+    const result = await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(result).toEqual({
+      block: true,
+      reason: "User denied dangerous command",
+    });
+  });
+
+  it("blocks when both custom() and select() return undefined", async () => {
+    const ctx = createEventContext({
+      hasUI: true,
+      ui: {
+        custom: vi.fn(
+          async () => undefined,
+        ) as ExtensionContext["ui"]["custom"],
+        select: vi.fn(
+          async () => undefined,
+        ) as ExtensionContext["ui"]["select"],
+      },
+    });
+    const result = await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(result).toEqual(expect.objectContaining({ block: true }));
+    expect(ctx.ui.select).toHaveBeenCalled();
+  });
+
+  it("does not call select() when custom() returns a valid result", async () => {
+    const ctx = createEventContext({
+      hasUI: true,
+      ui: {
+        custom: vi.fn(async () => "deny") as ExtensionContext["ui"]["custom"],
+      },
+    });
+    await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(ctx.ui.select).not.toHaveBeenCalled();
+  });
+
+  it("blocks when select() returns an unrecognized string", async () => {
+    const ctx = createEventContext({
+      hasUI: true,
+      ui: {
+        custom: vi.fn(
+          async () => undefined,
+        ) as ExtensionContext["ui"]["custom"],
+        select: vi.fn(async () => "maybe") as ExtensionContext["ui"]["select"],
+      },
+    });
+    const result = await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(result).toEqual(expect.objectContaining({ block: true }));
+  });
+
+  it("saves session grant via configLoader when select() returns 'Allow for session'", async () => {
+    const ctx = createEventContext({
+      hasUI: true,
+      ui: {
+        custom: vi.fn(
+          async () => undefined,
+        ) as ExtensionContext["ui"]["custom"],
+        select: vi.fn(
+          async () => SELECT_ALLOW_SESSION,
+        ) as ExtensionContext["ui"]["select"],
+      },
+    });
+    await handler(bashEvent("sudo rm -rf /"), ctx);
+    expect(configLoader.save).toHaveBeenCalledWith("memory", {
+      permissionGate: {
+        allowedPatterns: [{ pattern: "sudo rm -rf /" }],
+      },
+    });
+  });
+});