diff --git a/src/permissions/analyzer.ts b/src/permissions/analyzer.ts index d36ad94..f718513 100644 --- a/src/permissions/analyzer.ts +++ b/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 ` invocation. + * Generalizes the rule to `Bash(gh :*)` 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 | 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; diff --git a/src/permissions/readOnlyShell.ts b/src/permissions/readOnlyShell.ts index 0112578..f2ed42c 100644 --- a/src/permissions/readOnlyShell.ts +++ b/src/permissions/readOnlyShell.ts @@ -100,7 +100,7 @@ const SAFE_LETTA_COMMANDS: Record> = { // gh CLI read-only commands: category -> allowed actions // null means any action is allowed for that category -const SAFE_GH_COMMANDS: Record | null> = { +export const SAFE_GH_COMMANDS: Record | null> = { pr: new Set(["list", "status", "checks", "diff", "view"]), issue: new Set(["list", "status", "view"]), repo: new Set(["list", "view", "gitignore", "license"]), diff --git a/src/tests/permissions-analyzer.test.ts b/src/tests/permissions-analyzer.test.ts index 4a7e3a2..ce5b753 100644 --- a/src/tests/permissions-analyzer.test.ts +++ b/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"); +});