8637a2a41976b5dc6f2a0f8ae47f2d017f7ef6fe

Message

refactor Pi tool permissions config

Diff

This diff is truncated to protect this page.

  1diff --git a/README.md b/README.md
  2index 47f78a58b28c6a2c8ac82d7f9e213caa832d3a0d..2b8079b7fc3778313754ab15c70690d922f62a12 100644
  3--- a/README.md
  4+++ b/README.md
  5@@ -74,6 +74,25 @@ The legacy `modelBackend` and `llamaServer` configuration fields remain supporte
  6 
  7 LLM review sends the complete tool input to the configured provider. Do not use a cloud reviewer when this is not acceptable.
  8 
  9+## Pi permissions
 10+
 11+Pi uses root-level `tools` and `externalDirectories` maps. Any tool name can be configured. An omitted tool defaults to `check`.
 12+
 13+```json
 14+{
 15+  "tools": {
 16+    "bash": "check",
 17+    "read": "allow",
 18+    "my_extension_tool": "allow"
 19+  },
 20+  "externalDirectories": {
 21+    "/tmp/pi/*": "allow"
 22+  }
 23+}
 24+```
 25+
 26+`externalDirectories` applies to Pi path tools. Its entries are also included in the LLM reviewer prompt so checked tools can approve matching external paths.
 27+
 28 ## Pi behavior
 29 
 30 Pi keeps `/allow-bash`, Pi static permissions, and Herdr `herdr:blocked` events around its confirmation dialog. Pi bypasses MCP tools at the Pi adapter boundary.
 31diff --git a/src/core/review.ts b/src/core/review.ts
 32index 843169890702b775364a5d7f7fbd67c7dafffd11..30c49f4d17a0074ecee35acd083b2de7a4ffc96d 100644
 33--- a/src/core/review.ts
 34+++ b/src/core/review.ts
 35@@ -1,4 +1,4 @@
 36-import { LLM_POLICY_PROMPT } from "./rules"
 37+import { llmPolicyPrompt } from "./rules"
 38 import { callReviewer, type ApiKeyResolver } from "./providers"
 39 import {
 40   isDecisionCategory,
 41@@ -42,6 +42,7 @@ function reviewRequest(request: PolicyRequest): string {
 42 export function createPolicyReviewer(
 43   config: ReviewerConfig,
 44   resolveApiKey: ApiKeyResolver,
 45+  externalDirectories?: Record<string, string>,
 46 ): PolicyReviewer {
 47   return {
 48     async evaluate(request: PolicyRequest, context: PolicyContext): Promise<LLMEvaluationResult> {
 49@@ -49,7 +50,7 @@ export function createPolicyReviewer(
 50         const rawResponse = await callReviewer(
 51           config,
 52           [
 53-            { role: "system", content: LLM_POLICY_PROMPT },
 54+            { role: "system", content: llmPolicyPrompt(externalDirectories) },
 55             { role: "user", content: reviewRequest(request) },
 56           ],
 57           512,
 58diff --git a/src/core/rules.ts b/src/core/rules.ts
 59index 808793a67fcab30df43e118702d385ea0f71ab44..d52252ebc1307a47cdd4e7b566c0d51b26abe1ea 100644
 60--- a/src/core/rules.ts
 61+++ b/src/core/rules.ts
 62@@ -185,3 +185,18 @@ Respond with ONLY this JSON:
 63   "reason": "Brief explanation (1-2 sentences)",
 64   "category": ${DECISION_CATEGORIES.map((category) => `"${category}"`).join(" | ")}
 65 }`;
 66+
 67+export function llmPolicyPrompt(externalDirectories: Record<string, string> = {}): string {
 68+  const entries = Object.entries(externalDirectories)
 69+  if (entries.length === 0) return LLM_POLICY_PROMPT
 70+
 71+  const directories = entries
 72+    .map(([pattern, action]) => `- ${JSON.stringify(pattern)}: ${action}`)
 73+    .join("\n")
 74+  return `${LLM_POLICY_PROMPT}
 75+
 76+Configured external-directory permissions:
 77+${directories}
 78+
 79diff --git a/src/pi/config.ts b/src/pi/config.ts
 80index 8e6bb22fdd825b349e6e21521b4e23f151183d62..074ea665ae15dffc05ce24ead8a1121eda9aaeb8 100644
 81--- a/src/pi/config.ts
 82+++ b/src/pi/config.ts
 83@@ -5,23 +5,12 @@ import { parseReviewerConfig } from "../core/config"
 84 import type { ReviewerConfig } from "../core/types"
 85 
 86 export type PermissionAction = "allow" | "check" | "deny"
 87-export type PermissionRules = PermissionAction | Record<string, PermissionAction>
 88-
 89-export type PermissionConfig = {
 90-  bash?: PermissionRules
 91-  read?: PermissionRules
 92-  write?: PermissionRules
 93-  edit?: PermissionRules
 94-  find?: PermissionRules
 95-  grep?: PermissionRules
 96-  ls?: PermissionRules
 97-  external_directory?: PermissionRules
 98-  webfetch?: PermissionRules
 99-}
100+export type ExternalDirectoryRules = Record<string, PermissionAction>
101 
102 export type PiPolicyEngineConfig = {
103   reviewer: ReviewerConfig
104-  permission?: PermissionConfig
105+  tools: Record<string, PermissionAction>
106+  externalDirectories: ExternalDirectoryRules
107 }
108 
109 function configPath(): string {
110@@ -48,34 +37,37 @@ function permissionAction(value: unknown, field: string): PermissionAction {
111   throw new Error(`${field} must be "allow", "check", or "deny"`)
112 }
113 
114-function permissionRules(value: unknown, field: string): PermissionRules {
115-  if (typeof value === "string") return permissionAction(value, field)
116+function toolPermissions(value: unknown): Record<string, PermissionAction> {
117   const data = objectValue(value)
118-  if (!data) throw new Error(`${field} must be a permission action or an object`)
119+  if (!data) throw new Error("tools must be a JSON object")
120   return Object.fromEntries(
121-    Object.entries(data).map(([pattern, action]) => [pattern, permissionAction(action, `${field}.${pattern}`)]),
122+    Object.entries(data).map(([name, action]) => [name, permissionAction(action, `tools.${name}`)]),
123   )
124 }
125 
126-function permissionConfig(value: unknown): PermissionConfig {
127+function externalDirectoryRules(value: unknown): ExternalDirectoryRules {
128   const data = objectValue(value)
129-  if (!data) throw new Error("permission must be a JSON object")
130+  if (!data) throw new Error("externalDirectories must be a JSON object")
131   return Object.fromEntries(
132-    Object.entries(data).map(([name, rules]) => [name, permissionRules(rules, `permission.${name}`)]),
133-  ) as PermissionConfig
134+    Object.entries(data).map(([pattern, action]) => [pattern, permissionAction(action, `externalDirectories.${pattern}`)]),
135+  )
136 }
137 
138 export function parsePiPolicyConfig(value: unknown): PiPolicyEngineConfig {
139   const data = objectValue(value)
140   if (!data) throw new Error("policy engine configuration must be a JSON object")
141+  if (data.permission !== undefined) {
142+    throw new Error("permission has been renamed to tools; external_directory has been renamed to externalDirectories")
143+  }
144   return {
145     reviewer: parseReviewerConfig(data),
146-    permission: data.permission === undefined ? undefined : permissionConfig(data.permission),
147+    tools: data.tools === undefined ? {} : toolPermissions(data.tools),
148+    externalDirectories: data.externalDirectories === undefined ? {} : externalDirectoryRules(data.externalDirectories),
149   }
150 }
151 
152 export function loadPiPolicyConfig(): PiPolicyEngineConfig {
153   const path = configPath()
154-  if (!existsSync(path)) return { reviewer: parseReviewerConfig({}) }
155+  if (!existsSync(path)) return { reviewer: parseReviewerConfig({}), tools: {}, externalDirectories: {} }
156   return parsePiPolicyConfig(JSON.parse(readFileSync(path, "utf8")))
157 }
158diff --git a/src/pi/index.ts b/src/pi/index.ts
159index e5a0202b9a4498445c2b82a50b6d97391ecf26dd..ee1d4a13ddb416857075379119a41da4e69cce4d 100644
160--- a/src/pi/index.ts
161+++ b/src/pi/index.ts
162@@ -67,7 +67,7 @@ function createPiPipeline(
163     deterministic: (request) => checkDeterministic(request.toolName, request.input),
164     cache: createJsonlDecisionCache(paths.cacheFile),
165     audit: createJsonlDecisionAudit(paths.auditFile),
166-    reviewer: createPolicyReviewer(config.reviewer, resolvePiOpenAIKey),
167+    reviewer: createPolicyReviewer(config.reviewer, resolvePiOpenAIKey, config.externalDirectories),
168     normalize: (request) => normalizeRequest(request.toolName, request.input),
169     cacheKey,
170     inputSummary: auditInputSummary,
171diff --git a/src/pi/static-permissions.ts b/src/pi/static-permissions.ts
172index 6da6d2b3db49622ec69423d730ced1778c00fe1e..b1a8c62ce1273b41a60ccf9f75e8624976c03e02 100644
173--- a/src/pi/static-permissions.ts
174+++ b/src/pi/static-permissions.ts
175@@ -1,12 +1,10 @@
176 import { realpath } from "node:fs/promises"
177 import { homedir } from "node:os"
178 import { dirname, isAbsolute, join, relative, resolve, sep } from "node:path"
179-import { loadPiPolicyConfig, type PermissionAction, type PermissionConfig, type PermissionRules } from "./config"
180+import { loadPiPolicyConfig, type ExternalDirectoryRules, type PermissionAction } from "./config"
181 import type { Decision, DecisionCategory } from "../core/types"
182 
183-const STATIC_PERMISSION_TOOLS = ["bash", "read", "write", "edit", "find", "grep", "ls", "webfetch"] as const
184-type StaticPermissionTool = (typeof STATIC_PERMISSION_TOOLS)[number]
185-const PATH_TOOLS = new Set<StaticPermissionTool>(["read", "write", "edit", "find", "grep", "ls"])
186+const PATH_TOOLS = new Set(["read", "write", "edit", "find", "grep", "ls"])
187 
188 type ToolInput = Record<string, unknown>
189 
190@@ -23,8 +21,7 @@ function expandedPattern(pattern: string): string {
191   return pattern
192 }
193 
194-export function actionFor(rules: PermissionRules | undefined, value: string): PermissionAction | undefined {
195-  if (typeof rules === "string") return rules
196+export function actionFor(rules: ExternalDirectoryRules | undefined, value: string): PermissionAction | undefined {
197   if (!rules) return undefined
198 
199   let action: PermissionAction | undefined
200@@ -56,58 +53,37 @@ function isInside(path: string, directory: string): boolean {
201   return pathFromDirectory === "" || (!pathFromDirectory.startsWith(`..${sep}`) && pathFromDirectory !== ".." && !isAbsolute(pathFromDirectory))
202 }
203 
204-function staticDecision(
205-  action: PermissionAction | undefined,
206-  reason: string,
207-  category: DecisionCategory,
208-): Decision | undefined {
209+function staticDecision(action: PermissionAction | undefined, reason: string, category: DecisionCategory): Decision | undefined {
210   if (!action || action === "check") return undefined
211   return { decision: action, reason, category }
212 }
213 
214-function isStaticPermissionTool(toolType: string): toolType is StaticPermissionTool {
215-  return (STATIC_PERMISSION_TOOLS as readonly string[]).includes(toolType)
216-}
217-
218-function toolRules(toolType: StaticPermissionTool, config: PermissionConfig): PermissionRules | undefined {
219-  return config[toolType]
220-}
221-
222-function toolValue(toolType: StaticPermissionTool, input: ToolInput): string | undefined {
223-  if (toolType === "bash") return typeof input.command === "string" ? input.command : undefined
224-  if (toolType === "webfetch") return typeof input.url === "string" ? input.url : undefined
225-  return undefined
226-}
227-
228 export async function checkStaticPermission(
229   toolType: string,
230   input: ToolInput,
231   cwd: string,
232 ): Promise<Decision | undefined> {
233-  const permission = loadPiPolicyConfig().permission
234-  if (!permission) return undefined
235-
236-  if (!isStaticPermissionTool(toolType)) return undefined
237+  const config = loadPiPolicyConfig()
238+  const action = config.tools[toolType] ?? "check"
239 
240+  let externalPath: string | undefined
241   if (PATH_TOOLS.has(toolType)) {
242     const path = typeof input.path === "string" ? input.path : "."
243     const absolutePath = await canonicalPath(path, cwd)
244     const workspacePath = await canonicalPath(cwd, cwd)
245-    const external = !isInside(absolutePath, workspacePath)
246-    if (external) {
247-      const action = actionFor(permission.external_directory, absolutePath) ?? "check"
248-      if (action === "check") return undefined
249-      if (action === "deny") return { decision: "deny", reason: `External path: ${absolutePath}`, category: "external_directory" }
250+    if (!isInside(absolutePath, workspacePath)) {
251+      const externalAction = actionFor(config.externalDirectories, absolutePath) ?? "check"
252+      if (externalAction === "deny") {
253+        return { decision: "deny", reason: `External path: ${absolutePath}`, category: "external_directory" }
254+      }
255+      if (externalAction === "check") return undefined
256+      externalPath = absolutePath
257     }
258-    const result = staticDecision(actionFor(toolRules(toolType, permission), absolutePath), `Static ${toolType} permission`, toolType)
259-    if (result) return result
260-    return external
261-      ? { decision: "allow", reason: `Static external path permission: ${absolutePath}`, category: "external_directory" }
262-      : undefined
263   }
264 
265-  const value = toolValue(toolType, input)
266-  return value
267-    ? staticDecision(actionFor(toolRules(toolType, permission), value), `Static ${toolType} permission`, toolType)
268+  const toolDecision = staticDecision(action, `Static ${toolType} permission`, "config_allow")
269+  if (toolDecision) return toolDecision
270+  return externalPath
271+    ? { decision: "allow", reason: `Static external path permission: ${externalPath}`, category: "external_directory" }
272     : undefined
273 }