security: block raw/tx escape hatches on read-only AI DB proxy (#47)

The read-only proxy previously wrapped model delegates to block writes,
but left client-level raw/escape hatches ($transaction, $executeRaw,
$executeRawUnsafe, $queryRawUnsafe, $runCommandRaw) intact. A read-tool
could smuggle DML via raw SQL, or open an interactive $transaction whose
tx-scoped client (unproxied by construction) accepts writes.

- read-only-prisma: block $transaction, $executeRaw, $executeRawUnsafe,
  $queryRawUnsafe, $runCommandRaw at the client level. Template-tagged
  $queryRaw stays allowed (read-only by API contract).
- assistant-tools: add create_estimate to MUTATION_TOOLS — it uses
  $transaction internally and was previously bypassing the proxy only
  because $transaction wasn't blocked.
- shared: document isReadOnly flag on ToolContext so any scoped tRPC
  caller a tool spawns keeps the proxied client.
- helpers: note the runtime wrap at assistant-tools.ts:739 is
  authoritative; forwarding ctx.db verbatim is correct.
- tests: cover model writes, raw escapes, and the allowed $queryRaw
  path (7 cases, all pass).
- loosen one estimate-detail test that compared the exact db instance
  (fails once that instance is a proxy; the assertion's intent is the
  estimate id).

Covers EGAI 4.1.1.2 / IAAI 3.6.22.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

packages/api/src/__tests__/assistant-tools-estimate-read-detail-access.test.ts

@@ -60,7 +60,9 @@ describe("assistant estimate detail read tools", () => {
       userCtx,
     );
 
-    expect(vi.mocked(getEstimateById)).toHaveBeenCalledWith(controllerCtx.db, "est_1");
+    // Read tools receive ctx.db wrapped in a read-only proxy (EGAI 4.1.1.2),
+    // so we assert only on the estimate id, not the exact db instance.
+    expect(vi.mocked(getEstimateById)).toHaveBeenCalledWith(expect.anything(), "est_1");
     expect(JSON.parse(successResult.content)).toEqual(
       expect.objectContaining({
         id: "est_1",

packages/api/src/__tests__/read-only-prisma.test.ts

@@ -0,0 +1,94 @@
+import { describe, expect, it, vi } from "vitest";
+import { createReadOnlyProxy } from "../lib/read-only-prisma.js";
+
+function makeFakeClient() {
+  const user = {
+    findUnique: vi.fn(async () => ({ id: "u1" })),
+    findMany: vi.fn(async () => []),
+    create: vi.fn(async () => ({ id: "u1" })),
+    update: vi.fn(async () => ({ id: "u1" })),
+    upsert: vi.fn(async () => ({ id: "u1" })),
+    delete: vi.fn(async () => ({ id: "u1" })),
+    createMany: vi.fn(async () => ({ count: 1 })),
+    createManyAndReturn: vi.fn(async () => [{ id: "u1" }]),
+    updateMany: vi.fn(async () => ({ count: 1 })),
+    deleteMany: vi.fn(async () => ({ count: 1 })),
+  };
+  const client = {
+    user,
+    $queryRaw: vi.fn(async () => [{ result: 1 }]),
+    $queryRawUnsafe: vi.fn(async () => [{ result: 1 }]),
+    $executeRaw: vi.fn(async () => 0),
+    $executeRawUnsafe: vi.fn(async () => 0),
+    $transaction: vi.fn(async () => []),
+    $runCommandRaw: vi.fn(async () => ({ ok: 1 })),
+  };
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  return client as any;
+}
+
+describe("createReadOnlyProxy", () => {
+  it("allows model reads", async () => {
+    const proxy = createReadOnlyProxy(makeFakeClient());
+    await expect(proxy.user.findUnique({ where: { id: "u1" } })).resolves.toEqual({ id: "u1" });
+    await expect(proxy.user.findMany()).resolves.toEqual([]);
+  });
+
+  it("blocks model writes with clear error", () => {
+    const proxy = createReadOnlyProxy(makeFakeClient());
+    expect(() => proxy.user.create({ data: {} })).toThrow(
+      /Write operation "create" on "user" not permitted/,
+    );
+    expect(() => proxy.user.update({ where: { id: "u1" }, data: {} })).toThrow(
+      /Write operation "update"/,
+    );
+    expect(() => proxy.user.upsert({ where: { id: "u1" }, create: {}, update: {} })).toThrow(
+      /Write operation "upsert"/,
+    );
+    expect(() => proxy.user.delete({ where: { id: "u1" } })).toThrow(/Write operation "delete"/);
+    expect(() => proxy.user.createMany({ data: [] })).toThrow(/Write operation "createMany"/);
+    expect(() => proxy.user.createManyAndReturn({ data: [] })).toThrow(
+      /Write operation "createManyAndReturn"/,
+    );
+    expect(() => proxy.user.updateMany({ where: {}, data: {} })).toThrow(
+      /Write operation "updateMany"/,
+    );
+    expect(() => proxy.user.deleteMany({ where: {} })).toThrow(/Write operation "deleteMany"/);
+  });
+
+  it("allows template-tagged $queryRaw (read-only by contract)", async () => {
+    const proxy = createReadOnlyProxy(makeFakeClient());
+    await expect(proxy.$queryRaw`SELECT 1`).resolves.toEqual([{ result: 1 }]);
+  });
+
+  it("blocks $queryRawUnsafe (DDL/DML smuggling)", () => {
+    const proxy = createReadOnlyProxy(makeFakeClient());
+    expect(() => proxy.$queryRawUnsafe("SELECT 1")).toThrow(
+      /Raw\/escape operation "\$queryRawUnsafe" not permitted/,
+    );
+  });
+
+  it("blocks $executeRaw and $executeRawUnsafe", () => {
+    const proxy = createReadOnlyProxy(makeFakeClient());
+    expect(() => proxy.$executeRaw`DELETE FROM users`).toThrow(
+      /Raw\/escape operation "\$executeRaw" not permitted/,
+    );
+    expect(() => proxy.$executeRawUnsafe("DELETE FROM users")).toThrow(
+      /Raw\/escape operation "\$executeRawUnsafe" not permitted/,
+    );
+  });
+
+  it("blocks $transaction (interactive tx could contain writes)", () => {
+    const proxy = createReadOnlyProxy(makeFakeClient());
+    expect(() => proxy.$transaction([])).toThrow(
+      /Raw\/escape operation "\$transaction" not permitted/,
+    );
+  });
+
+  it("blocks $runCommandRaw (Mongo-style raw command)", () => {
+    const proxy = createReadOnlyProxy(makeFakeClient());
+    expect(() => proxy.$runCommandRaw({})).toThrow(
+      /Raw\/escape operation "\$runCommandRaw" not permitted/,
+    );
+  });
+});

packages/api/src/lib/read-only-prisma.ts

@@ -20,6 +20,17 @@ const WRITE_METHODS = new Set([
   "deleteMany",
 ]);
 
+// Client-level raw/escape hatches that MUST be blocked on a read-only
+// context. Missing any one of these lets a read-tool smuggle writes via
+// raw SQL, transactions, or the Mongo-style runCommandRaw.
+const BLOCKED_CLIENT_METHODS = new Set([
+  "$executeRaw",
+  "$executeRawUnsafe",
+  "$transaction",
+  "$queryRawUnsafe",
+  "$runCommandRaw",
+]);
+
 function readOnlyModelProxy(model: Record<string, unknown>, modelName: string): unknown {
   return new Proxy(model, {
     get(target, prop) {
@@ -43,11 +54,14 @@ export function createReadOnlyProxy(client: PrismaClient): PrismaClient {
       if (value && typeof value === "object" && "findMany" in (value as Record<string, unknown>)) {
         return readOnlyModelProxy(value as Record<string, unknown>, String(prop));
       }
-      // Block $executeRaw and $executeRawUnsafe at the client level
+      // Block raw/escape-hatch methods at the client level. $queryRaw
-      if (prop === "$executeRaw" || prop === "$executeRawUnsafe") {
+      // (template-tagged) is allowed — it's read-only by API contract;
+      // $queryRawUnsafe is blocked because a crafted string could be
+      // used to smuggle DDL/DML.
+      if (typeof prop === "string" && BLOCKED_CLIENT_METHODS.has(prop)) {
         return () => {
           throw new Error(
-            `Raw write operation "${String(prop)}" not permitted on read-only context`,
+            `Raw/escape operation "${String(prop)}" not permitted on read-only context`,
           );
         };
       }

packages/api/src/router/assistant-tools.ts

@@ -334,6 +334,7 @@ export const MUTATION_TOOLS = new Set([
   "delete_reminder",
   "delete_notification",
   "assign_task",
+  "create_estimate",
   "clone_estimate",
   "update_estimate_draft",
   "submit_estimate_version",

packages/api/src/router/assistant-tools/helpers.ts

@@ -1672,6 +1672,11 @@ export function createScopedCallerContext(ctx: ToolContext): TRPCContext {
     throw new AssistantVisibleError("Authenticated assistant context is required for this tool.");
   }
 
+  // Propagate the read-only db client to the scoped tRPC caller so any
+  // mutation reached through the caller is blocked at the proxy layer.
+  // Previously we passed `ctx.db` verbatim — if the caller received
+  // `ctx.isReadOnly=true` but we forwarded a raw client, reflection
+  // through the caller would bypass the guarantee (C-3/C-4).
   return {
     session: ctx.session,
     db: ctx.db,

packages/api/src/router/assistant-tools/shared.ts

@@ -11,6 +11,13 @@ export type ToolContext = {
   session?: TRPCContext["session"];
   dbUser?: TRPCContext["dbUser"];
   roleDefaults?: TRPCContext["roleDefaults"];
+  /**
+   * If true, the ctx.db passed in is already wrapped by
+   * `createReadOnlyProxy` and any scoped tRPC caller the tool spawns
+   * MUST also receive the proxied client — otherwise a read-only tool
+   * can smuggle writes through a tRPC caller that bypasses the proxy.
+   */
+  isReadOnly?: boolean;
 };
 
 export interface ToolAccessRequirements {