diff --git a/assistant/src/permissions/approval-policy.test.ts b/assistant/src/permissions/approval-policy.test.ts index 0c135f1b605..0bff1a9f512 100644 --- a/assistant/src/permissions/approval-policy.test.ts +++ b/assistant/src/permissions/approval-policy.test.ts @@ -334,6 +334,19 @@ describe("no rule — third-party skill tool", () => { expect(result.reason).toContain("Skill tool"); }); + test("plugin origin → treated as extension-owned (prompt at strict threshold)", () => { + // Plugins join skills in the "extension-owned" bucket — both prompt by + // default. `isSkillBundled` is irrelevant for plugins (always false). + const result = evaluate({ + riskLevel: RiskLevel.Low, + toolName: "custom_plugin_tool", + toolOrigin: "plugin", + autoApproveUpTo: "none", + }); + expect(result.decision).toBe("prompt"); + expect(result.reason).toContain("Skill tool"); + }); + test("no tool origin but hasManifestOverride, strict threshold → prompt (unregistered skill tool)", () => { const result = evaluate({ riskLevel: RiskLevel.Low, @@ -699,16 +712,17 @@ describe("edge cases", () => { expect(result.reason).toContain("Skill tool"); }); - test("hasManifestOverride with toolOrigin set to builtin — falls through (not a skill)", () => { + test("hasManifestOverride with toolOrigin=mcp — falls through (not extension-owned)", () => { const result = evaluate({ riskLevel: RiskLevel.Low, toolName: "manifest_tool", - toolOrigin: "builtin", + toolOrigin: "mcp", hasManifestOverride: true, }); - // toolOrigin is "builtin", so the third-party skill check doesn't trigger. - // The hasManifestOverride check requires !toolOrigin, but toolOrigin is set. - // Falls through to risk-based: Low → allow (within default "low" threshold). + // toolOrigin is "mcp", which is not extension-class (skill/plugin), so + // the third-party skill check doesn't trigger. The hasManifestOverride + // sub-check requires !toolOrigin, but toolOrigin is set. Falls through + // to risk-based: Low → allow (within default "low" threshold). expect(result.decision).toBe("allow"); expect(result.reason).toContain("low risk"); }); diff --git a/assistant/src/permissions/approval-policy.ts b/assistant/src/permissions/approval-policy.ts index 6a05ff649d4..101093d9ce3 100644 --- a/assistant/src/permissions/approval-policy.ts +++ b/assistant/src/permissions/approval-policy.ts @@ -1,3 +1,4 @@ +import type { OwnerKind } from "../tools/types.js"; import type { TrustRule } from "./types.js"; import { RiskLevel } from "./types.js"; @@ -13,8 +14,13 @@ export interface ApprovalContext { matchedRule?: TrustRule; isContainerized: boolean; isWorkspaceScoped: boolean; - /** Where the tool originates from — "skill" for skill-provided tools, "builtin" for core tools. */ - toolOrigin?: "skill" | "builtin"; + /** + * Owner kind of the tool, as recorded by the tool registry — "skill" / + * "plugin" / "mcp" for extension-owned tools, `undefined` for core tools + * (and for tools that aren't registered, e.g. unregistered skill tools + * matched only via `hasManifestOverride`). + */ + toolOrigin?: OwnerKind; /** Whether the tool's owning skill is a first-party bundled skill. */ isSkillBundled?: boolean; /** Whether the tool has a manifest override (unregistered skill tool). */ @@ -173,8 +179,13 @@ export class DefaultApprovalPolicy implements ApprovalPolicy { // ── 6. No rule + third-party skill tool → prompt (unless threshold covers it) if (!matchedRule) { + // Plugin- and skill-owned tools are both treated as extension-class + // for approval purposes: external by default, prompt unless bundled. + // MCP-owned tools fall through to the core risk-based path. + const isExtensionOwned = + toolOrigin === "skill" || toolOrigin === "plugin"; const isThirdPartySkill = - (toolOrigin === "skill" && !isSkillBundled) || + (isExtensionOwned && !isSkillBundled) || (hasManifestOverride && !toolOrigin); if (isThirdPartySkill) { if (isRiskWithinThreshold(riskLevel, context.autoApproveUpTo)) { diff --git a/assistant/src/permissions/checker.ts b/assistant/src/permissions/checker.ts index 01d86806d51..ba4a21690bb 100644 --- a/assistant/src/permissions/checker.ts +++ b/assistant/src/permissions/checker.ts @@ -547,13 +547,7 @@ export async function check( risk === RiskLevel.Low ? isWorkspaceScopedInvocation(toolName, input, workingDir) : false, - toolOrigin: (() => { - if (!tool) return undefined; - const ownerKind = getToolOwner(toolName)?.kind; - return ownerKind === "skill" || ownerKind === "plugin" - ? "skill" - : "builtin"; - })(), + toolOrigin: getToolOwner(toolName)?.kind, isSkillBundled: isToolOwnerSkillBundled(tool), hasManifestOverride: !!manifestOverride, autoApproveUpTo: threshold,