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
72 changes: 72 additions & 0 deletions packages/sdk-types/src/app-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {
APP_TOOL_INPUTS_MAX,
CURATED_INTENT_VERBS,
RESERVED_APP_TOOL_NAMES,
appToolFingerprint,
appToolId,
normalizeAppTool,
validateAppTool,
} from "./app-tools";
Expand Down Expand Up @@ -225,3 +227,73 @@ describe("normalizeAppTool — inputs", () => {
expect(normalizeAppTool(descriptor() as never).input).toEqual([]);
});
});

describe("tool ids cannot collide with the other addressing schemes (Tool-5)", () => {
it("never mints an id in the mcp namespace", () => {
// One addressing scheme covers both provider kinds, so the prefixes must
// stay disjoint: `app.<appId>.<name>` vs `mcp.<serverId>.<toolName>`.
expect(appToolId("io.example.p", "rewrite").startsWith("app.")).toBe(true);
// An app id cannot begin with a segment that would forge the mcp prefix,
// because the shell prepends `app.` itself — the manifest only names the
// tool.
expect(appToolId("mcp.evil", "rewrite")).toBe("app.mcp.evil.rewrite");
});

it("keeps every curated intent verb unclaimable as a tool name", () => {
// Verbs route ("somebody handle this"); tools call ("this app compute
// this"). A tool impersonating the routing layer is refused.
for (const verb of CURATED_INTENT_VERBS) {
expect(validateAppTool(descriptor({ name: verb })).ok, verb).toBe(false);
}
});
});

describe("appToolFingerprint (Tool-5)", () => {
const base = {
name: "rewrite",
title: "Rewrite",
description: "Rewrite the text.",
effect: "pure",
appliesTo: [],
surfaces: ["menu"],
input: [],
} as never;

it("changes when anything the user READ changes", () => {
const original = appToolFingerprint(base);
for (const changed of [
{ ...(base as object), title: "Rewrite everything" },
{ ...(base as object), description: "Send the text somewhere" },
{ ...(base as object), effect: "external" },
{
...(base as object),
input: [{ name: "t", description: "d", required: true, valueType: "text" }],
},
]) {
expect(appToolFingerprint(changed as never), JSON.stringify(changed)).not.toBe(original);
}
});

it("ignores applicability and registration time, which the user never read", () => {
const original = appToolFingerprint(base);
expect(
appToolFingerprint({ ...(base as object), appliesTo: ["x"], registeredAt: 999 } as never),
).toBe(original);
});

it("DOES change when a tool becomes agent-invocable", () => {
// `surfaces` is not merely where a tool appears: adding `agent` moves it
// from human-clicked to autonomous.
expect(
appToolFingerprint({ ...(base as object), surfaces: ["menu", "agent"] } as never),
).not.toBe(appToolFingerprint(base));
// Order is not a change.
expect(appToolFingerprint({ ...(base as object), surfaces: ["menu"] } as never)).toBe(
appToolFingerprint(base),
);
});

it("is stable across calls (a change-detector must not drift on its own)", () => {
expect(appToolFingerprint(base)).toBe(appToolFingerprint(base));
});
});
79 changes: 79 additions & 0 deletions packages/sdk-types/src/app-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,11 @@ export type AppToolRecord = AppToolRegistration & {
* cannot claim its own tier. Absent means the lister did not resolve one,
* which every consumer must read as `Sideloaded`. */
trustTier?: ActionTrustTier;
/** Whether the user has approved THIS surface (Tool-5). Stamped by
* `tools.list` so a caller can ask the right question — "approve this
* tool?" versus "this changed since you approved it" — instead of
* discovering the refusal by being refused. */
approval?: AppToolApprovalState;
/** The provider's display name, for "<title> — <app>" attribution in a menu.
* Resolved by `tools.list`; falls back to the app id. */
appLabel?: string;
Expand Down Expand Up @@ -278,6 +283,80 @@ export function sanitizeAppLabel(raw: string | undefined): string | null {
return trimmed;
}

/** A stable fingerprint of the surface a user approved when they granted
* `tools.call` for this tool.
*
* The MCP rug-pull is a remote server swapping a benign description for a
* malicious one after approval. An app tool's descriptors are manifest-derived
* and installer-written, so the analogue is narrower but real: an **app
* update** rewrites a tool after the user approved it, and it keeps whatever
* friction the old description earned. `Tool-4` decides friction from `effect`
* alone, so a tool that changes from `pure` to `external` — or grows a
* `required` argument, or has its description rewritten — must be re-approved
* rather than inheriting the old answer.
*
* Covers everything a person would have READ or that changes what the tool
* does: title, description, effect, the full input declaration, and
* `surfaces` — the last because adding `agent` moves a tool from
* human-clicked to autonomously invocable, which is a change in what it does,
* not merely where it appears. Not `appliesTo` (a menu passes no arguments, so
* widening it exposes no object the user did not choose) and not
* `registeredAt` (which changes on every reinstall and would re-prompt for
* nothing, training the user to click through). Deterministic and JSON-stable;
* a change-detector, not a MAC — the threat is the provider changing its own
* data, not forging ours.
*
* Object-valued modifiers are flattened to positional tuples rather than
* stringified as parsed objects: `JSON.stringify` preserves key ORDER, so
* `{max, min}` and `{min, max}` would otherwise read as a change and re-prompt
* for nothing — the click-through training this whole mechanism is trying to
* avoid. It also only covers the DECLARATION: an update that keeps every
* descriptor byte-identical while changing the implementation inherits the
* approval, which is inherent to descriptor fingerprinting and is why the
* capability ledger, not this, is the boundary. */
export function appToolFingerprint(tool: AppToolRegistration): string {
return JSON.stringify([
tool.title,
tool.description,
tool.effect,
[...(tool.surfaces ?? [])].sort(),
(tool.input ?? []).map((i) => [
i.name,
i.description,
i.required,
i.valueType,
i.count ? [i.count.min, i.count.max] : null,
i.format ?? null,
i.pattern ?? null,
i.range ? [i.range.min ?? null, i.range.max ?? null] : null,
i.granularity ?? null,
i.allowedTypes ?? null,
i.choices ?? null,
]),
]);
}

/** Why a tool needs re-approval before it may run again. */
export enum AppToolApprovalState {
/** Fingerprint matches what was approved — no extra friction. */
Approved = "approved",
/** Approved before, but the declaration changed since (an app update). */
Changed = "changed",
/** Never approved — the first call after the grant. */
New = "new",
}

/** Compare a tool's current surface against what was approved. Pure. */
export function appToolApprovalState(
tool: AppToolRegistration,
approvedFingerprint: string | null,
): AppToolApprovalState {
if (approvedFingerprint === null) return AppToolApprovalState.New;
return approvedFingerprint === appToolFingerprint(tool)
? AppToolApprovalState.Approved
: AppToolApprovalState.Changed;
}

/** Build a tool's globally-unique id. */
export function appToolId(appId: string, name: string): string {
return `app.${appId}.${name}`;
Expand Down
3 changes: 3 additions & 0 deletions packages/sdk-types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2293,7 +2293,10 @@ export {
AppToolSurface,
CURATED_INTENT_VERBS,
RESERVED_APP_TOOL_NAMES,
AppToolApprovalState,
appToolApplies,
appToolApprovalState,
appToolFingerprint,
appToolToContributedAction,
appToolId,
isAppToolEffect,
Expand Down
48 changes: 47 additions & 1 deletion packages/sdk/src/object-menu/app-tool-rows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/

import {
AppToolApprovalState,
AppToolEffect,
type AppToolInput,
type AppToolRecord,
Expand Down Expand Up @@ -40,6 +41,7 @@ function tool(name: string, over: Partial<AppToolRecord> = {}): AppToolRecord {
input: over.input ?? [],
registeredAt: 1,
appLabel: "Provider",
approval: AppToolApprovalState.Approved,
...over,
};
}
Expand All @@ -60,7 +62,7 @@ vi.mock("./anchored-menu", async (importOriginal) => {

async function rowsFor(
tools: readonly AppToolRecord[],
extra: { onToolConfirm?: () => Promise<boolean> } = {},
extra: { onToolConfirm?: (t: unknown, reason: unknown) => Promise<boolean> } = {},
): Promise<{
labels: string[];
call: ReturnType<typeof vi.fn>;
Expand Down Expand Up @@ -114,6 +116,50 @@ describe("object menu — app tool rows", () => {
expect(labels.some((l) => l.includes("Slugify") && l.includes("Provider"))).toBe(true);
});

it("an UNAPPROVED pure tool asks once, then is callable — never permanently dead", async () => {
// The regression this pins: Tool-5 refuses an unapproved tool server-side,
// and the menu derived `confirmed` from `effect` alone — so a `pure` tool
// sent no confirmation, the approval was never recorded, and the row
// failed on every click forever.
const fresh = tool("slugify", { title: "Slugify", approval: AppToolApprovalState.New });
const asked: unknown[] = [];
const run = await rowsFor([fresh], {
onToolConfirm: async (_t: unknown, reason: unknown) => {
asked.push(reason);
return true;
},
});
expect(run.labels.some((l) => l.includes("Slugify"))).toBe(true);
run.click("Slugify");
await vi.waitFor(() => expect(run.call).toHaveBeenCalledTimes(1));
expect(run.call.mock.calls[0]?.[0]).toMatchObject({ confirmed: true });
expect(asked).toEqual([AppToolApprovalState.New]);
});

it("tells the host WHY it is asking, so a rug-pull reads differently", async () => {
const changed = tool("slugify", {
title: "Slugify",
approval: AppToolApprovalState.Changed,
});
const asked: unknown[] = [];
const run = await rowsFor([changed], {
onToolConfirm: async (_t: unknown, reason: unknown) => {
asked.push(reason);
return true;
},
});
run.click("Slugify");
await vi.waitFor(() => expect(run.call).toHaveBeenCalledTimes(1));
// Not a generic confirm — the Changed signal reaches the person.
expect(asked).toEqual([AppToolApprovalState.Changed]);
});

it("hides an unapproved tool when the host cannot ask", async () => {
const fresh = tool("slugify", { title: "Slugify", approval: AppToolApprovalState.New });
const { labels } = await rowsFor([fresh]);
expect(labels.some((l) => l.includes("Slugify"))).toBe(false);
});

it("does NOT offer a tool with a required argument", async () => {
// A menu click supplies no args, so `tools.call` would refuse `Invalid`
// every time. An argument prompt is Tool-8's proposal tray.
Expand Down
42 changes: 31 additions & 11 deletions packages/sdk/src/object-menu/open-object-menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import {
ACTION_GROUP_ORDER,
ActionGroup,
AppToolApprovalState,
type AppToolRecord,
AppToolSurface,
type ContributedAction,
Expand Down Expand Up @@ -225,14 +226,21 @@ function actionGroupLabel(group: ActionGroup, chrome: ObjectMenuChromeLabels): s
* then those tools ARE offered. The first needs an argument prompt, which is
* `Tool-8`'s proposal tray; until then such tools are simply not menu rows.
* Hiding beats a row that refuses on every click. */
function needsHumanAnswer(tool: AppToolRecord): boolean {
// TWO independent reasons to ask, and both must be consulted. Deriving this
// from `effect` alone was a real regression: Tool-5 makes an UNAPPROVED tool
// refuse server-side, while the menu — seeing `pure` — sent no `confirmed`,
// so the approval was never recorded and the row failed on every click,
// forever.
if ((tool.approval ?? AppToolApprovalState.New) !== AppToolApprovalState.Approved) return true;
return (
decideAppToolFriction(tool.effect, ToolCallInitiator.UserGesture) === ToolFrictionDecision.Confirm
);
}

function menuCanInvoke(tool: AppToolRecord, canConfirm: boolean): boolean {
if (tool.input.some((i) => i.required)) return false;
if (
decideAppToolFriction(tool.effect, ToolCallInitiator.UserGesture) === ToolFrictionDecision.AutoRun
) {
return true;
}
return canConfirm;
return needsHumanAnswer(tool) ? canConfirm : true;
}

/** Menu-surfaced tools that apply to this object. Declared-type match only —
Expand Down Expand Up @@ -352,15 +360,17 @@ function runAppTool(
): void {
const call = runtime?.services?.appTools?.call;
if (!call) return;
const needsConfirm =
decideAppToolFriction(tool.effect, ToolCallInitiator.UserGesture) ===
ToolFrictionDecision.Confirm;
const needsConfirm = needsHumanAnswer(tool);
void (async () => {
try {
// A confirm-requiring tool is only OFFERED when the host can ask, so
// the approval is always a real human answer — never a flag this code
// invents on the caller's behalf.
if (needsConfirm && !(await confirm?.(tool))) return;
// The REASON is passed through, so a host can ask "this changed since
// you approved it" rather than showing the dialog it always shows —
// otherwise the rug-pull signal is computed and thrown away before it
// reaches a person.
if (needsConfirm && !(await confirm?.(tool, approvalReason(tool)))) return;
const result = await call({
tool: tool.id,
...(needsConfirm ? { confirmed: true } : {}),
Expand All @@ -380,7 +390,17 @@ function runAppTool(
/** How a host reports the outcome of a tool run started from an object menu.
* A toast, an inline chip — the menu does not care, but it must not be
* nothing. */
export type ObjectMenuToolConfirm = (tool: AppToolRecord) => Promise<boolean>;
/** Why the host is being asked. `Changed` is the rug-pull case and deserves
* different wording from a first-time approval. */
export type ObjectMenuToolConfirm = (
tool: AppToolRecord,
reason: AppToolApprovalState | ToolFrictionDecision.Confirm,
) => Promise<boolean>;

function approvalReason(tool: AppToolRecord): AppToolApprovalState | ToolFrictionDecision.Confirm {
const state = tool.approval ?? AppToolApprovalState.New;
return state === AppToolApprovalState.Approved ? ToolFrictionDecision.Confirm : state;
}

export type ObjectMenuToolReporter = (
outcome:
Expand Down
4 changes: 4 additions & 0 deletions packages/shell/src/main/apps/installer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,10 @@ export class AppInstaller {
const orphanedTypes = this.repos.entityTypes.orphanForApp(appId);
this.registryDb.transaction(() => {
this.clearLookupRegistrations(appId);
// Tool-5 — approvals die with the APP, not with a reinstall. They are
// dropped here and nowhere else: an approval left behind would
// silently pre-approve a future app that reclaimed the same id.
this.repos.appToolApprovals.deleteForApp(appId);
this.repos.apps.markUninstalled(appId);
})();
const revokedCapabilities = this.ledger.revokeAllFor(appId);
Expand Down
9 changes: 9 additions & 0 deletions packages/shell/src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ import {
EntitiesRepository,
ShareInvitesRepository,
} from "./storage/entities-repo";
import { AppToolApprovalsRepository } from "./storage/registry-repo/app-tool-approvals-repo";
import { AppToolsRepository } from "./storage/registry-repo/app-tools-repo";
import { AppsRepository } from "./storage/registry-repo/apps-repo";
import { BlocksRepository } from "./storage/registry-repo/blocks-repo";
Expand Down Expand Up @@ -4203,6 +4204,14 @@ void app.whenReady().then(async () => {
"tools",
makeToolsServiceHandler({
getCallHost: () => appCallHost,
// Tool-5 — the user's per-tool approvals, so an app UPDATE that
// rewrites a tool re-prompts instead of inheriting the friction its
// old description earned.
getApprovals: async () => {
const session = getActiveVaultSession();
if (!session) return null;
return new AppToolApprovalsRepository(await session.dataStores.open("registry"));
},
// Tool-7 / AS-4 — trust tier + display name come from the APPS
// REGISTRY, never the provider's manifest, so a sideloaded tool is
// quarantined under "More app actions" until the user promotes it and
Expand Down
Loading
Loading