Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions assistant/src/permissions/approval-policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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");
});
Expand Down
17 changes: 14 additions & 3 deletions assistant/src/permissions/approval-policy.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { OwnerKind } from "../tools/types.js";
import type { TrustRule } from "./types.js";
import { RiskLevel } from "./types.js";

Expand All @@ -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). */
Expand Down Expand Up @@ -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)) {
Expand Down
8 changes: 1 addition & 7 deletions assistant/src/permissions/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading