feat(permissions): generalize approve-always rules for gh CLI commands (#1240)

src/permissions/analyzer.ts

@@ -4,7 +4,7 @@
 import { homedir } from "node:os";
 import { dirname, relative, resolve, win32 } from "node:path";
 import { canonicalToolName, isFileToolName } from "./canonical";
-import { isReadOnlyShellCommand } from "./readOnlyShell";
+import { isReadOnlyShellCommand, SAFE_GH_COMMANDS } from "./readOnlyShell";
 import { unwrapShellLauncherCommand } from "./shell-command-normalization";
 
 export interface ApprovalContext {
@@ -262,6 +262,57 @@ function analyzeEditApproval(
   };
 }
 
+/**
+ * Build an approval context for a `gh <category> <action>` invocation.
+ * Generalizes the rule to `Bash(gh <category> <action>:*)` so that future
+ * calls with different PR numbers or flags are covered by the same rule.
+ */
+function analyzeGhApproval(category: string, action: string): ApprovalContext {
+  if (!category) {
+    // bare `gh` with no subcommand - fall back to a generic rule
+    return {
+      recommendedRule: "Bash(gh:*)",
+      ruleDescription: "'gh' commands",
+      approveAlwaysText:
+        "Yes, and don't ask again for 'gh' commands in this project",
+      defaultScope: "project",
+      allowPersistence: true,
+      safetyLevel: "moderate",
+    };
+  }
+
+  // undefined when category not in map, null when all actions allowed
+  const allowedActions: Set<string> | null | undefined =
+    SAFE_GH_COMMANDS[category];
+  const categoryInMap = allowedActions !== undefined;
+
+  // Determine if this specific action is read-only
+  const isReadOnly =
+    categoryInMap &&
+    (allowedActions === null ||
+      (action.length > 0 && allowedActions.has(action)));
+
+  // `gh api` can mutate (POST/PATCH/DELETE), so always treat as moderate
+  const safetyLevel =
+    category === "api" ? "moderate" : isReadOnly ? "safe" : "moderate";
+
+  // Build the command prefix: include action when we know it (avoids
+  // over-broad "gh pr:*" rules that cover mutations like `gh pr create`)
+  const ruleCmd =
+    action.length > 0 && allowedActions !== null
+      ? `gh ${category} ${action}`
+      : `gh ${category}`;
+
+  return {
+    recommendedRule: `Bash(${ruleCmd}:*)`,
+    ruleDescription: `'${ruleCmd}' commands`,
+    approveAlwaysText: `Yes, and don't ask again for '${ruleCmd}' commands in this project`,
+    defaultScope: "project",
+    allowPersistence: true,
+    safetyLevel,
+  };
+}
+
 /**
  * Analyze Bash command approval
  */
@@ -582,6 +633,11 @@ function analyzeBashApproval(
     }
   }
 
+  // gh CLI commands - generalize to category+action prefix rules
+  if (baseCommand === "gh") {
+    return analyzeGhApproval(firstArg, parts[2] || "");
+  }
+
   // Package manager commands
   if (baseCommand && ["npm", "bun", "yarn", "pnpm"].includes(baseCommand)) {
     const subcommand = firstArg;
@@ -683,6 +739,11 @@ function analyzeBashApproval(
         }
       }
 
+      // Check if this segment is a gh CLI command
+      if (segmentBase === "gh") {
+        return analyzeGhApproval(segmentArg, segmentParts[2] || "");
+      }
+
       // Check if this segment is npm/bun/yarn/pnpm
       if (segmentBase && ["npm", "bun", "yarn", "pnpm"].includes(segmentBase)) {
         const subcommand = segmentArg;

src/permissions/readOnlyShell.ts

@@ -100,7 +100,7 @@ const SAFE_LETTA_COMMANDS: Record<string, Set<string>> = {
 
 // gh CLI read-only commands: category -> allowed actions
 // null means any action is allowed for that category
-const SAFE_GH_COMMANDS: Record<string, Set<string> | null> = {
+export const SAFE_GH_COMMANDS: Record<string, Set<string> | null> = {
   pr: new Set(["list", "status", "checks", "diff", "view"]),
   issue: new Set(["list", "status", "view"]),
   repo: new Set(["list", "view", "gitignore", "license"]),

src/tests/permissions-analyzer.test.ts

@@ -609,3 +609,133 @@ test("run_shell_command is analyzed as Bash", () => {
 
   expect(context.recommendedRule).toBe("Bash(curl:*)");
 });
+
+// ============================================================================
+// gh CLI approval tests
+// ============================================================================
+
+test("gh pr view suggests safe project-scoped rule", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "gh pr view 471 --json title,body,files,additions,deletions" },
+    "/Users/test/project",
+  );
+
+  expect(context.recommendedRule).toBe("Bash(gh pr view:*)");
+  expect(context.safetyLevel).toBe("safe");
+  expect(context.defaultScope).toBe("project");
+  expect(context.allowPersistence).toBe(true);
+  expect(context.approveAlwaysText).toContain("gh pr view");
+});
+
+test("gh pr diff suggests safe project-scoped rule", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "gh pr diff 471" },
+    "/Users/test/project",
+  );
+
+  expect(context.recommendedRule).toBe("Bash(gh pr diff:*)");
+  expect(context.safetyLevel).toBe("safe");
+});
+
+test("gh pr list suggests safe rule", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "gh pr list --state open" },
+    "/Users/test/project",
+  );
+
+  expect(context.recommendedRule).toBe("Bash(gh pr list:*)");
+  expect(context.safetyLevel).toBe("safe");
+});
+
+test("gh pr checks suggests safe rule", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "gh pr checks 471" },
+    "/Users/test/project",
+  );
+
+  expect(context.recommendedRule).toBe("Bash(gh pr checks:*)");
+  expect(context.safetyLevel).toBe("safe");
+});
+
+test("gh issue view suggests safe rule", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "gh issue view 123" },
+    "/Users/test/project",
+  );
+
+  expect(context.recommendedRule).toBe("Bash(gh issue view:*)");
+  expect(context.safetyLevel).toBe("safe");
+});
+
+test("gh issue list suggests safe rule", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "gh issue list --state open" },
+    "/Users/test/project",
+  );
+
+  expect(context.recommendedRule).toBe("Bash(gh issue list:*)");
+  expect(context.safetyLevel).toBe("safe");
+});
+
+test("gh run view suggests safe rule", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "gh run view 1234567890" },
+    "/Users/test/project",
+  );
+
+  expect(context.recommendedRule).toBe("Bash(gh run view:*)");
+  expect(context.safetyLevel).toBe("safe");
+});
+
+test("gh search issues suggests safe rule", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "gh search issues --repo letta-ai/letta-code foo" },
+    "/Users/test/project",
+  );
+
+  // search category has null allowedActions - use "gh search" prefix (no action)
+  expect(context.recommendedRule).toBe("Bash(gh search:*)");
+  expect(context.safetyLevel).toBe("safe");
+});
+
+test("gh api suggests moderate rule (can mutate)", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "gh api repos/letta-ai/letta-code/pulls/471/comments" },
+    "/Users/test/project",
+  );
+
+  // api category has null allowedActions - use "gh api" prefix (no action)
+  expect(context.recommendedRule).toBe("Bash(gh api:*)");
+  expect(context.safetyLevel).toBe("moderate");
+});
+
+test("gh pr create suggests moderate rule", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "gh pr create --title 'fix: something' --body 'desc'" },
+    "/Users/test/project",
+  );
+
+  expect(context.recommendedRule).toBe("Bash(gh pr create:*)");
+  expect(context.safetyLevel).toBe("moderate");
+});
+
+test("gh pr view in compound command suggests safe rule", () => {
+  const context = analyzeApprovalContext(
+    "Bash",
+    { command: "cd /Users/cameron/repo && gh pr view 471 --json title,body" },
+    "/Users/test/project",
+  );
+
+  expect(context.recommendedRule).toBe("Bash(gh pr view:*)");
+  expect(context.safetyLevel).toBe("safe");
+});