feat: improve always-allow behavior for skill scripts (#879)

Co-authored-by: Letta <noreply@letta.com>

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;

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
 // ============================================================================

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
 // ============================================================================