Validate caller-supplied policy positions against the specificity invariant

packages/core/sdk/src/executor.ts

@@ -136,6 +136,7 @@ import type { FirstPartyOAuthClientConfig } from "./oauth-client";
 import {
   comparePolicyRow,
   isValidPattern,
+  isValidPositionForPattern,
   matchPattern,
   positionForNewPattern,
   resolveEffectivePolicy,
@@ -5403,6 +5404,18 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
         // 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.
+        // An EXPLICIT position is judged by the same invariant rather than
+        // trusted: precedence within an owner is match authority, so a client
+        // key that sorts a broad rule above a narrower one is refused.
+        if (
+          input.position !== undefined &&
+          !isValidPositionForPattern(input.pattern, input.position, existing)
+        ) {
+          return yield* new StorageError({
+            message: `Tool policy position ${input.position} would place ${input.pattern} above a more-specific rule`,
+            cause: undefined,
+          });
+        }
         const position = input.position ?? positionForNewPattern(input.pattern, existing);
         const id = PolicyId.make(
           `pol_${Math.random().toString(36).slice(2)}${Date.now().toString(36)}`,
@@ -5440,9 +5453,29 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
             cause: undefined,
           });
         }
+        const action = input.action ?? existing.action;
+        if (!isToolPolicyAction(action)) {
+          return yield* new StorageError({
+            message: `Invalid tool policy action: ${String(action)}`,
+            cause: undefined,
+          });
+        }
+        // Same invariant as create, judged against the owner's OTHER rules and
+        // the rule's (possibly updated) pattern: an explicit position may not
+        // hoist a broad rule above a narrower one.
+        const pattern = input.pattern ?? existing.pattern;
+        if (input.position !== undefined) {
+          const ownerRows = yield* core.findMany("tool_policy", { where: byOwner(input.owner) });
+          if (!isValidPositionForPattern(pattern, input.position, ownerRows, input.id)) {
+            return yield* new StorageError({
+              message: `Tool policy position ${input.position} would place ${pattern} above a more-specific rule`,
+              cause: undefined,
+            });
+          }
+        }
         const set: Record<string, unknown> = { updated_at: new Date() };
         if (input.pattern !== undefined) set.pattern = input.pattern;
-        if (input.action !== undefined) set.action = input.action;
+        if (input.action !== undefined) set.action = action;
         if (input.position !== undefined) set.position = input.position;
         yield* core.updateMany("tool_policy", { where, set });
         const updated = yield* core.findFirst("tool_policy", { where });

packages/core/sdk/src/policies.test.ts

@@ -482,6 +482,95 @@ describe("executor.policies", () => {
     }),
   );
 
+  it.effect("create refuses an explicit position that shadows a more-specific rule", () =>
+    Effect.gen(function* () {
+      const executor = yield* setupExecutor();
+      // The org's narrow leaf rule commits first, at the top.
+      const leaf = yield* executor.policies.create({
+        owner: "org",
+        pattern: "vercel.org.main.delete",
+        action: "block",
+      });
+      // A broad wildcard approve whose caller-supplied position sorts ABOVE
+      // the leaf rule: first match per owner wins, so this would silently
+      // weaken the org's block into an approve. `"0"` (0x30) sorts before
+      // every key generateKeyBetween emits (`"a0"`…), so it claims the top of
+      // the owner's list. Refused, not stored.
+      const result = yield* Effect.result(
+        executor.policies.create({
+          owner: "org",
+          pattern: "*",
+          action: "approve",
+          position: "0",
+        }),
+      );
+      expect(Result.isFailure(result)).toBe(true);
+      expect(String((result as { failure: { message: string } }).failure.message)).toContain(
+        "more-specific rule",
+      );
+      void leaf;
+    }),
+  );
+
+  it.effect("update refuses a position that hoists a broad rule above a narrower one", () =>
+    Effect.gen(function* () {
+      const executor = yield* setupExecutor();
+      const narrow = yield* executor.policies.create({
+        owner: "org",
+        pattern: "vercel.org.main.delete",
+        action: "block",
+      });
+      const broad = yield* executor.policies.create({
+        owner: "org",
+        pattern: "vercel.*",
+        action: "approve",
+      });
+      // Broad starts below narrow (the default placement). A client-supplied
+      // position above the narrow rule would flip match precedence — refused.
+      const result = yield* Effect.result(
+        executor.policies.update({
+          id: String(broad.id),
+          owner: "org",
+          position: "0",
+        }),
+      );
+      expect(Result.isFailure(result)).toBe(true);
+
+      // Reordering among equally-specific rules is still allowed.
+      const equallySpecific = yield* executor.policies.create({
+        owner: "org",
+        pattern: "github.*",
+        action: "require_approval",
+      });
+      yield* executor.policies.update({
+        id: String(equallySpecific.id),
+        owner: "org",
+        position: narrow.position,
+      });
+      const rules = yield* executor.policies.list();
+      expect(rules.find((r) => r.pattern === "github.*")?.position).toBe(narrow.position);
+    }),
+  );
+
+  it.effect("update rejects an unknown action", () =>
+    Effect.gen(function* () {
+      const executor = yield* setupExecutor();
+      const created = yield* executor.policies.create({
+        owner: "org",
+        pattern: "vercel.*",
+        action: "require_approval",
+      });
+      const result = yield* Effect.result(
+        executor.policies.update({
+          id: String(created.id),
+          owner: "org",
+          action: "deny-everything" as never,
+        }),
+      );
+      expect(Result.isFailure(result)).toBe(true);
+    }),
+  );
+
   it.effect("remove deletes the rule", () =>
     Effect.gen(function* () {
       const executor = yield* setupExecutor();

packages/core/sdk/src/policies.ts

@@ -180,6 +180,64 @@ export const positionForNewPattern = (
   return generateKeyBetween(prev, next);
 };
 
+/**
+ * Whether two patterns can possibly match the same tool id. Conservative:
+ * any shared prefix up to the first differing literal segment counts as
+ * overlap, and a wildcard segment anywhere before a difference does too.
+ * Only used to scope the ordering invariant below — false positives keep the
+ * invariant strict; false negatives would be unsafe, and there are none
+ * (segment-by-segment, a shared literal prefix plus a wildcard or exhaustion
+ * is the only way to a common tool id).
+ */
+const patternsOverlap = (a: string, b: string): boolean => {
+  if (a === "*" || b === "*") return true;
+  const as = a.split(".");
+  const bs = b.split(".");
+  const len = Math.min(as.length, bs.length);
+  for (let i = 0; i < len; i++) {
+    const sa = as[i]!;
+    const sb = bs[i]!;
+    if (sa === "*" || sb === "*") return true;
+    if (sa !== sb) return false;
+  }
+  return true;
+};
+
+/**
+ * Whether a CALLER-SUPPLIED position is safe for `pattern` among the owner's
+ * committed rules. `positionForNewPattern` computes the safest key when the
+ * client omits one; a client that sends one can otherwise place a broad rule
+ * above a narrower rule of the same owner and silently weaken it — first
+ * match per owner wins (`resolveToolPolicy`), so precedence is authority.
+ * The invariant is the same one the default placement enforces: a rule may
+ * never sort ABOVE (lexically before) an existing rule that is MORE specific
+ * than it AND can match a tool this rule also matches. Equally- or
+ * less-specific rules, and rules over disjoint tool sets, remain freely
+ * orderable.
+ *
+ * `excludeId` omits the rule being updated, so a reorder is judged against
+ * the OTHER committed rules.
+ */
+export const isValidPositionForPattern = (
+  pattern: string,
+  position: string,
+  rows: ReadonlyArray<Pick<ToolPolicyRow, "pattern" | "position" | "id">>,
+  excludeId?: string,
+): boolean => {
+  const newScore = patternSpecificity(pattern);
+  for (const row of rows) {
+    if (excludeId !== undefined && row.id === excludeId) continue;
+    if (
+      patternSpecificity(row.pattern) > newScore &&
+      patternsOverlap(row.pattern, pattern) &&
+      position <= row.position
+    ) {
+      return false;
+    }
+  }
+  return true;
+};
+
 const actionRestrictionRank = (action: ToolPolicyAction): number =>
   Match.value(action).pipe(
     Match.when("block", () => 3),