From 06d5d71cafab9c6fd65a0c2df6fd386c9342f61b Mon Sep 17 00:00:00 2001 From: Charles Packer Date: Mon, 9 Feb 2026 18:12:29 -0800 Subject: [PATCH] feat: improve always-allow behavior for skill scripts (#879) Co-authored-by: Letta --- src/permissions/analyzer.ts | 151 ++++++++++++++++++++++++- src/tests/permissions-analyzer.test.ts | 117 +++++++++++++++++++ src/tests/permissions-matcher.test.ts | 24 ++++ 3 files changed, 291 insertions(+), 1 deletion(-) diff --git a/src/permissions/analyzer.ts b/src/permissions/analyzer.ts index 5ee9e07..6816e70 100644 --- a/src/permissions/analyzer.ts +++ b/src/permissions/analyzer.ts @@ -1,6 +1,7 @@ // src/permissions/analyzer.ts // Analyze tool executions and recommend appropriate permission rules +import { homedir } from "node:os"; import { dirname, resolve } from "node:path"; export interface ApprovalContext { @@ -226,9 +227,144 @@ function containsDangerousCommand(command: string): boolean { return false; } +type SkillSourceLabel = "project" | "agent-scoped" | "global" | "bundled"; + +interface SkillScriptInfo { + source: SkillSourceLabel; + skillName: string; + skillRootPath: string; +} + +function escapeRegex(text: string): string { + return text.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +function normalizePathSeparators(path: string): string { + return path.replace(/\\/g, "/"); +} + +function parseAbsoluteCommandPaths(command: string): string[] { + const normalized = command.replace(/\\"/g, '"').replace(/\\'/g, "'"); + const candidates: string[] = []; + + // Prefer explicit quoted paths first. + const quotedRegex = /["']((?:[A-Za-z]:)?\/[^"']+)["']/g; + let quotedMatch: RegExpExecArray | null = quotedRegex.exec(normalized); + while (quotedMatch) { + if (quotedMatch[1]) { + candidates.push(normalizePathSeparators(quotedMatch[1])); + } + quotedMatch = quotedRegex.exec(normalized); + } + + // Also scan whitespace-delimited tokens (handles cd && command chains). + const tokens = normalized.split(/\s+/); + for (const token of tokens) { + const cleaned = token + .replace(/^["'`([{]+/, "") + .replace(/["'`),;|\]}]+$/g, ""); + if (/^(?:[A-Za-z]:)?\//.test(cleaned)) { + candidates.push(normalizePathSeparators(cleaned)); + } + } + + // Preserve first-seen order while de-duplicating. + return Array.from(new Set(candidates)); +} + +function detectSkillScript( + command: string, + workingDir: string, +): SkillScriptInfo | null { + const pathCandidates = parseAbsoluteCommandPaths(command); + if (pathCandidates.length === 0) { + return null; + } + + const normalizedWorkingDir = normalizePathSeparators(workingDir).replace( + /\/$/, + "", + ); + const normalizedHomeDir = normalizePathSeparators(homedir()).replace( + /\/$/, + "", + ); + + const detect = ( + source: SkillSourceLabel, + regex: RegExp, + ): SkillScriptInfo | null => { + for (const candidate of pathCandidates) { + const match = candidate.match(regex); + if (!match?.[1]) { + continue; + } + const skillName = match[1]; + const skillRootPath = match[0].replace(/\/scripts\/$/, ""); + return { source, skillName, skillRootPath }; + } + return null; + }; + + const projectRegex = new RegExp( + `^${escapeRegex(normalizedWorkingDir)}/\\.skills/(.+?)/scripts/`, + ); + const projectSkill = detect("project", projectRegex); + if (projectSkill) { + return projectSkill; + } + + const agentRegex = new RegExp( + `^${escapeRegex(normalizedHomeDir)}/\\.letta/agents/[^/]+/skills/(.+?)/scripts/`, + ); + const agentSkill = detect("agent-scoped", agentRegex); + if (agentSkill) { + return agentSkill; + } + + const globalRegex = new RegExp( + `^${escapeRegex(normalizedHomeDir)}/\\.letta/skills/(.+?)/scripts/`, + ); + const globalSkill = detect("global", globalRegex); + if (globalSkill) { + return globalSkill; + } + + const bundledSkill = detect( + "bundled", + /\/skills\/builtin\/([^/]+)\/scripts\//, + ); + if (bundledSkill) { + return bundledSkill; + } + + return null; +} + +function buildSkillScriptRule(command: string, skillRootPath: string): string { + const normalizedCommand = normalizePathSeparators(command).trim(); + const rootIndex = normalizedCommand.indexOf(skillRootPath); + if (rootIndex === -1) { + return `Bash(${normalizedCommand})`; + } + + const rulePrefix = normalizedCommand.slice( + 0, + rootIndex + skillRootPath.length, + ); + return `Bash(${rulePrefix}:*)`; +} + +function getSkillApprovalText( + source: SkillSourceLabel, + skillName: string, +): string { + return `Yes, and don't ask again for scripts in ${source} skill '${skillName}'`; +} + function analyzeBashApproval( command: string, - _workingDir: string, + workingDir: string, ): ApprovalContext { const parts = command.trim().split(/\s+/); const baseCommand = parts[0] || ""; @@ -262,6 +398,19 @@ function analyzeBashApproval( }; } + const skillScript = detectSkillScript(command, workingDir); + if (skillScript) { + const { source, skillName, skillRootPath } = skillScript; + return { + recommendedRule: buildSkillScriptRule(command, skillRootPath), + ruleDescription: `scripts in ${source} skill '${skillName}'`, + approveAlwaysText: getSkillApprovalText(source, skillName), + defaultScope: "project", + allowPersistence: true, + safetyLevel: "moderate", + }; + } + // Git commands - be specific to subcommand if (baseCommand === "git") { const gitSubcommand = firstArg; diff --git a/src/tests/permissions-analyzer.test.ts b/src/tests/permissions-analyzer.test.ts index 2d05ac4..226d982 100644 --- a/src/tests/permissions-analyzer.test.ts +++ b/src/tests/permissions-analyzer.test.ts @@ -203,6 +203,123 @@ test("Unknown command suggests exact match", () => { expect(context.allowPersistence).toBe(true); }); +test("Skill script in bundled skill suggests bundled-scope message", () => { + if (process.platform === "win32") return; + + const context = analyzeApprovalContext( + "Bash", + { + command: + "cd /Users/test/project && npx tsx /tmp/letta/src/skills/builtin/creating-skills/scripts/init-skill.ts my-skill", + }, + "/Users/test/project", + ); + + expect(context.recommendedRule).toBe( + "Bash(cd /Users/test/project && npx tsx /tmp/letta/src/skills/builtin/creating-skills:*)", + ); + expect(context.approveAlwaysText).toBe( + "Yes, and don't ask again for scripts in bundled skill 'creating-skills'", + ); + expect(context.safetyLevel).toBe("moderate"); +}); + +test("Skill script in agent-scoped skill suggests agent-scoped message", () => { + if (process.platform === "win32") return; + const home = require("node:os").homedir(); + + const context = analyzeApprovalContext( + "Bash", + { + command: `npx tsx ${home}/.letta/agents/agent-123/skills/finding-agents/scripts/main.ts --help`, + }, + "/Users/test/project", + ); + + expect(context.recommendedRule).toBe( + `Bash(npx tsx ${home}/.letta/agents/agent-123/skills/finding-agents:*)`, + ); + expect(context.approveAlwaysText).toBe( + "Yes, and don't ask again for scripts in agent-scoped skill 'finding-agents'", + ); +}); + +test("Skill script in global skill suggests global message", () => { + if (process.platform === "win32") return; + const home = require("node:os").homedir(); + + const context = analyzeApprovalContext( + "Bash", + { + command: `npx tsx ${home}/.letta/skills/messaging-agents/scripts/run.ts`, + }, + "/Users/test/project", + ); + + expect(context.recommendedRule).toBe( + `Bash(npx tsx ${home}/.letta/skills/messaging-agents:*)`, + ); + expect(context.approveAlwaysText).toBe( + "Yes, and don't ask again for scripts in global skill 'messaging-agents'", + ); +}); + +test("Skill script in project skill supports nested skill IDs", () => { + if (process.platform === "win32") return; + + const context = analyzeApprovalContext( + "Bash", + { + command: + "npx tsx /Users/test/project/.skills/workflow/agent-tools/scripts/do.ts", + }, + "/Users/test/project", + ); + + expect(context.recommendedRule).toBe( + "Bash(npx tsx /Users/test/project/.skills/workflow/agent-tools:*)", + ); + expect(context.approveAlwaysText).toBe( + "Yes, and don't ask again for scripts in project skill 'workflow/agent-tools'", + ); +}); + +test("Dangerous skill script command still blocks persistence", () => { + if (process.platform === "win32") return; + + const context = analyzeApprovalContext( + "Bash", + { + command: + "npx tsx /tmp/letta/src/skills/builtin/creating-skills/scripts/init-skill.ts --force", + }, + "/Users/test/project", + ); + + expect(context.allowPersistence).toBe(false); + expect(context.safetyLevel).toBe("dangerous"); +}); + +test("Skill script path in quoted command is detected", () => { + if (process.platform === "win32") return; + + const context = analyzeApprovalContext( + "Bash", + { + command: + "bash -lc \"npx tsx '/tmp/letta/src/skills/builtin/creating-skills/scripts/package-skill.ts'\"", + }, + "/Users/test/project", + ); + + expect(context.recommendedRule).toContain( + "/tmp/letta/src/skills/builtin/creating-skills:*", + ); + expect(context.approveAlwaysText).toBe( + "Yes, and don't ask again for scripts in bundled skill 'creating-skills'", + ); +}); + // ============================================================================ // File Tool Analysis Tests // ============================================================================ diff --git a/src/tests/permissions-matcher.test.ts b/src/tests/permissions-matcher.test.ts index d47ff5d..2c41934 100644 --- a/src/tests/permissions-matcher.test.ts +++ b/src/tests/permissions-matcher.test.ts @@ -233,6 +233,30 @@ test("Bash pattern: special characters in command", () => { ); }); +test("Bash pattern: skill-scoped prefix matches same skill scripts", () => { + expect( + matchesBashPattern( + "Bash(npx tsx /tmp/letta/src/skills/builtin/creating-skills/scripts/init-skill.ts foo)", + "Bash(npx tsx /tmp/letta/src/skills/builtin/creating-skills:*)", + ), + ).toBe(true); + expect( + matchesBashPattern( + "Bash(npx tsx /tmp/letta/src/skills/builtin/creating-skills/scripts/package-skill.ts bar)", + "Bash(npx tsx /tmp/letta/src/skills/builtin/creating-skills:*)", + ), + ).toBe(true); +}); + +test("Bash pattern: skill-scoped prefix does not match other skills", () => { + expect( + matchesBashPattern( + "Bash(npx tsx /tmp/letta/src/skills/builtin/messaging-agents/scripts/send.ts)", + "Bash(npx tsx /tmp/letta/src/skills/builtin/creating-skills:*)", + ), + ).toBe(false); +}); + // ============================================================================ // Tool Pattern Matching Tests // ============================================================================