Skip to content
Open
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
5 changes: 5 additions & 0 deletions .changeset/tool-policy-owner-segment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@executor-js/sdk": patch
---

Tool policy patterns written in the documented `integration.connection.tool` form (no owner segment) now match as intended instead of silently never firing. `policies.create`/`policies.update` backfill the missing owner segment automatically.
2 changes: 1 addition & 1 deletion packages/core/sdk/src/core-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,7 @@ export const coreToolsPlugin = definePlugin((options: CoreToolsPluginOptions = {
tool({
name: "policies.create",
description:
"Create a tool policy. `pattern` matches a tool address tail (`integration.connection.tool`, `integration.*`, `*`); `action` is approve/require_approval/block. `owner` is org (workspace guardrail) or user (personal).",
"Create a tool policy. `pattern` matches a tool address (`integration.connection.tool`, `integration.*`, `*`); the owner segment is optional and defaults to every owner when omitted. `action` is approve/require_approval/block. `owner` is org (workspace guardrail) or user (personal).",
inputSchema: PolicyCreateInputStd,
outputSchema: PolicyOutputStd,
// A policy decides which tools run without confirmation, so creating
Expand Down
24 changes: 24 additions & 0 deletions packages/core/sdk/src/executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1416,6 +1416,30 @@ describe("speculative read abandonment", () => {
}),
);

it.effect(
"a policy pattern written in the documented pre-owner shape still blocks the tool (#2047)",
() =>
Effect.gen(function* () {
const executor = yield* makeTestExecutor({ plugins: [demoPlugin] as const });
yield* seedRunConnection(executor);
// Public docs and the settings UI describe patterns as
// `integration.connection.tool` — one segment short of the matcher's
// `integration.owner.connection.tool`. Written as-is this pattern can
// never match; `policiesCreate` backfills the missing owner segment
// so a block set through the documented shape still takes effect.
yield* executor.policies.create({
owner: "org",
pattern: `${INTEG}.${CONN}.run`,
action: "block",
});

const result = yield* Effect.result(executor.execute(addr("run"), {}));
expect(Result.isFailure(result)).toBe(true);
if (!Result.isFailure(result)) return;
expect(Predicate.isTagged("ToolBlockedError")(result.failure)).toBe(true);
}),
);

it.effect("failing speculative reads neither mask the branch error nor unhandled-reject", () =>
Effect.gen(function* () {
const faults = makeReadFaults();
Expand Down
20 changes: 14 additions & 6 deletions packages/core/sdk/src/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ import {
isUnboundedDynamicToolScope,
isValidPattern,
matchPattern,
normalizePolicyPattern,
positionForNewPattern,
resolveEffectivePolicy,
rowToToolPolicy,
Expand Down Expand Up @@ -6047,9 +6048,12 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
const ownerRankForRow = (row: { readonly owner: string }): number =>
row.owner === "user" ? 0 : 1;

// Tool policies gate by tool identity (`<integration>.<tool>`), independent of
// which connection serves it; the org/user split is handled by owner-scoped
// policy rows + ownerRank, not the match pattern.
// Dynamic (connection-backed) tools are keyed by their full four-segment
// address, `<integration>.<owner>.<connection>.<tool>` — the owner
// segment is load-bearing: a connection's (integration, name) is only
// unique per owner, so two different connections can share a name.
// Patterns written without an owner segment are backfilled by
// `normalizePolicyPattern` at create/update time, not here.
const normalizedPolicyId = (tool: Tool): string =>
tool.static
? String(tool.address)
Expand Down Expand Up @@ -6089,14 +6093,18 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
try: () => ownedKeys(input.owner),
catch: (cause) => storageFailureFromUnknown("invalid owner", cause),
});
// Patterns written in the documented (pre-owner) 3-segment shape
// are one segment short of the matcher's 4-segment dynamic tool
// id and can never match — see `normalizePolicyPattern`.
const pattern = normalizePolicyPattern(input.pattern);
const existing = yield* core.findMany("tool_policy", {
where: byOwner(input.owner),
});
// Default placement is specificity-aware (below any more-specific
// rule), not top-of-list: a client that omits position — the UI when
// its policy list is stale, the API, an agent tool — must not have its
// broad rule silently shadow an existing narrow one.
const position = input.position ?? positionForNewPattern(input.pattern, existing);
const position = input.position ?? positionForNewPattern(pattern, existing);
const id = PolicyId.make(
`pol_${Math.random().toString(36).slice(2)}${Date.now().toString(36)}`,
);
Expand All @@ -6106,7 +6114,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
owner: keys.owner,
subject: keys.subject,
id: String(id),
pattern: input.pattern,
pattern,
action: input.action,
position,
created_at: now,
Expand Down Expand Up @@ -6137,7 +6145,7 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
});
}
const set: Record<string, unknown> = { updated_at: new Date() };
if (input.pattern !== undefined) set.pattern = input.pattern;
if (input.pattern !== undefined) set.pattern = normalizePolicyPattern(input.pattern);
if (input.action !== undefined) set.action = input.action;
if (input.position !== undefined) set.position = input.position;
yield* core.updateMany("tool_policy", { where, set });
Expand Down
36 changes: 36 additions & 0 deletions packages/core/sdk/src/policies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
effectivePolicyFromSorted,
isValidPattern,
matchPattern,
normalizePolicyPattern,
resolveToolPolicy,
} from "./policies";
import { definePlugin, tool } from "./plugin";
Expand Down Expand Up @@ -159,6 +160,41 @@ describe("dynamicToolScopeForPattern", () => {
});
});

describe("normalizePolicyPattern", () => {
it("backfills the owner segment on the documented pre-owner shape (#2047)", () => {
// The public docs and UI describe patterns as `integration.connection.tool`,
// one segment short of the matcher's `integration.owner.connection.tool`.
// Written as-is these can never match — see `matchPattern`'s mid-segment
// wildcard tests above.
expect(normalizePolicyPattern("vercel.dns.create")).toBe("vercel.*.dns.create");
expect(normalizePolicyPattern("vercel.dns.*")).toBe("vercel.*.dns.*");
});

it("leaves the universal pattern and already-unbounded plugin-wide patterns alone", () => {
expect(normalizePolicyPattern("*")).toBe("*");
expect(normalizePolicyPattern("vercel.*")).toBe("vercel.*");
});

it("leaves patterns that already carry an owner segment alone", () => {
expect(normalizePolicyPattern("vercel.org.dns.create")).toBe("vercel.org.dns.create");
expect(normalizePolicyPattern("vercel.*.dns.*")).toBe("vercel.*.dns.*");
expect(normalizePolicyPattern("github.*.*.repos.list")).toBe("github.*.*.repos.list");
});

it("leaves patterns rooted at a static namespace alone — there is no owner to backfill", () => {
expect(normalizePolicyPattern("executor.desktopSettings.openSettings")).toBe(
"executor.desktopSettings.openSettings",
);
});

it("produces a pattern that actually matches the dynamic tool it was written for", () => {
// Dynamic tool id: integration.owner.connection.tool
const toolId = "vercel.org.dns.create";
expect(matchPattern("vercel.dns.create", toolId)).toBe(false); // the bug
expect(matchPattern(normalizePolicyPattern("vercel.dns.create"), toolId)).toBe(true); // fixed
});
});

describe("resolveToolPolicy", () => {
// v2: policy rows carry `owner` (org|user) instead of a scope id.
const ROW = (
Expand Down
26 changes: 26 additions & 0 deletions packages/core/sdk/src/policies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,32 @@ export const isValidPattern = (pattern: string): boolean => {
return true;
};

// ---------------------------------------------------------------------------
// Owner-segment backfill — the public docs and UI describe patterns in the
// pre-owner 3-segment shape `integration.connection.tool`. The matcher keys
// dynamic tools by the full 4-segment `integration.owner.connection.tool`
// (see `normalizedPolicyId` in executor.ts), so a pattern written in the
// documented shape is one segment short and can never match anything
// (#2047). `policiesCreate`/`policiesUpdate` run every pattern through this
// first, backfilling the owner segment the writer never named. Left
// untouched: `*`, `integration.*` (already unbounded — no owner to insert),
// and patterns rooted at a static namespace (no owner segment at all).
//
// Duplicated from `migration-spec.ts`'s `DEFAULT_STATIC_NAMESPACES` rather
// than imported: this module is part of the public SDK surface (browser
// bundles included), and `migration-spec.ts` pulls in `node:crypto`.
// ---------------------------------------------------------------------------

const STATIC_TOOL_NAMESPACES: readonly string[] = ["executor", "openapi"];

export const normalizePolicyPattern = (pattern: string): string => {
if (pattern === "*") return pattern;
const segments = pattern.split(".");
if (segments.length !== 3) return pattern;
if (STATIC_TOOL_NAMESPACES.includes(segments[0]!)) return pattern;
return `${segments[0]}.*.${segments[1]}.${segments[2]}`;
};

// ---------------------------------------------------------------------------
// Dynamic-tool scope — the (integration, owner, connection) prefix a pattern
// can reach. Lets a policy source that is an allowlist (a toolkit) narrow the
Expand Down
Loading