Skip to content

Commit e3143ba

Browse files
committed
fix(policies): specificity-aware default position on the server
The UI computed a specificity-aware position for new rules client-side, but the server defaulted a positionless create to the TOP of the owner's list. When the client's policy list was stale (e.g. the optimistic atom hadn't refreshed between two quick writes), the UI sent no usable position and the top-of-list default let a broad category rule shadow a narrower leaf rule written moments earlier — surfaced by the policies-ui scenario failing against the production selfhost image. Move the specificity placement into the sdk (positionForNewPattern) and make it the server-side default for create; the React hook now reuses the same helper so the optimistic order matches what the server commits.
1 parent 724600c commit e3143ba

6 files changed

Lines changed: 107 additions & 38 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"executor": patch
3+
---
4+
5+
Policy create now defaults a new rule's position below any more-specific existing rule on the server, so a broad rule written without an explicit position (stale UI, API, agent tool) cannot shadow an existing narrower rule.

‎packages/core/sdk/src/executor.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import { memoryAdapter } from "@executor-js/fumadb/adapters/memory";
55
import { withQueryContext, type Condition, type ConditionBuilder } from "@executor-js/fumadb/query";
66
import { schema as fumaSchema, type RelationsMap } from "@executor-js/fumadb/schema";
77
import type { AnyColumn } from "@executor-js/fumadb/schema";
8-
import { generateKeyBetween } from "fractional-indexing";
9-
108
import {
119
StorageError,
1210
isStorageFailure,
@@ -99,6 +97,7 @@ import {
9997
comparePolicyRow,
10098
isValidPattern,
10199
matchPattern,
100+
positionForNewPattern,
102101
resolveEffectivePolicy,
103102
rowToToolPolicy,
104103
type CreateToolPolicyInput,
@@ -3407,11 +3406,11 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
34073406
const existing = yield* core.findMany("tool_policy", {
34083407
where: byOwner(input.owner),
34093408
});
3410-
const minPosition = existing
3411-
.map((row) => row.position)
3412-
.sort()
3413-
.at(0);
3414-
const position = input.position ?? generateKeyBetween(null, minPosition ?? null);
3409+
// Default placement is specificity-aware (below any more-specific
3410+
// rule), not top-of-list: a client that omits position — the UI when
3411+
// its policy list is stale, the API, an agent tool — must not have its
3412+
// broad rule silently shadow an existing narrow one.
3413+
const position = input.position ?? positionForNewPattern(input.pattern, existing);
34153414
const id = PolicyId.make(
34163415
`pol_${Math.random().toString(36).slice(2)}${Date.now().toString(36)}`,
34173416
);

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

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ describe("executor.policies", () => {
367367
}),
368368
);
369369

370-
it.effect("create defaults new rules to the top of the list", () =>
370+
it.effect("create defaults new rules above equally-or-less-specific ones", () =>
371371
Effect.gen(function* () {
372372
const executor = yield* setupExecutor();
373373
const first = yield* executor.policies.create({
@@ -387,6 +387,47 @@ describe("executor.policies", () => {
387387
}),
388388
);
389389

390+
it.effect("create places a broad rule below an existing narrower one", () =>
391+
Effect.gen(function* () {
392+
const executor = yield* setupExecutor();
393+
// Narrow leaf rule first, then a broad category rule WITHOUT an explicit
394+
// position — the category rule must not shadow the leaf rule.
395+
yield* executor.policies.create({
396+
owner: "org",
397+
pattern: "vercel.*.*.records.create",
398+
action: "block",
399+
});
400+
yield* executor.policies.create({
401+
owner: "org",
402+
pattern: "vercel.*.*.records.*",
403+
action: "require_approval",
404+
});
405+
406+
const rules = yield* executor.policies.list();
407+
expect(rules.map((r) => r.pattern)).toEqual([
408+
"vercel.*.*.records.create",
409+
"vercel.*.*.records.*",
410+
]);
411+
412+
const rows = rules.map(
413+
(r) =>
414+
({
415+
id: String(r.id),
416+
owner: r.owner,
417+
subject: "",
418+
pattern: r.pattern,
419+
action: r.action,
420+
position: r.position,
421+
created_at: r.createdAt,
422+
updated_at: r.updatedAt,
423+
}) as ToolPolicyRow,
424+
);
425+
const match = resolveToolPolicy("vercel.org.acct.records.create", rows, () => 0);
426+
expect(match?.action).toBe("block");
427+
expect(match?.pattern).toBe("vercel.*.*.records.create");
428+
}),
429+
);
430+
390431
it.effect("create stores rules at the requested owner", () =>
391432
Effect.gen(function* () {
392433
const executor = yield* setupExecutor();

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
// ---------------------------------------------------------------------------
1111

1212
import { Match, Schema } from "effect";
13+
import { generateKeyBetween } from "fractional-indexing";
1314

1415
import type { ToolPolicyAction, ToolPolicyRow } from "./core-schema";
1516
import { Owner, PolicyId } from "./ids";
@@ -139,6 +140,46 @@ export const comparePolicyRow = (
139140
return ia < ib ? -1 : ia > ib ? 1 : 0;
140141
};
141142

143+
// Specificity score for ordering. Higher = more specific = should sit at a
144+
// lower position-key (higher precedence). New rules are auto-placed below
145+
// any more-specific existing rules so a freshly-added group rule never
146+
// silently shadows an existing leaf rule.
147+
// `*` → 0
148+
// `vercel.*` → 2 (1 literal segment, wildcard)
149+
// `vercel.dns.*` → 4 (2 literal segments, wildcard)
150+
// `vercel.dns` → 5 (2 literal segments, exact — beats same-prefix wildcard)
151+
// `vercel.dns.create` → 7 (3 literal segments, exact)
152+
export const patternSpecificity = (pattern: string): number => {
153+
if (pattern === "*") return 0;
154+
if (pattern.endsWith(".*")) {
155+
const prefix = pattern.slice(0, -2);
156+
return prefix.split(".").length * 2;
157+
}
158+
return pattern.split(".").length * 2 + 1;
159+
};
160+
161+
/**
162+
* Position key for a new rule among an owner's existing rules, placed just
163+
* below every existing rule that is MORE specific (and above everything
164+
* equally or less specific). Rows must be the owner's committed rules; order
165+
* doesn't matter, they're sorted here. This is the authoritative default —
166+
* the server applies it when `create` gets no explicit position, so a rule
167+
* written by any client (UI, API, agent tool) cannot shadow a more-specific
168+
* existing rule by racing to the top of the list.
169+
*/
170+
export const positionForNewPattern = (
171+
pattern: string,
172+
rows: ReadonlyArray<Pick<ToolPolicyRow, "pattern" | "position" | "id">>,
173+
): string => {
174+
const committed = [...rows].sort(comparePolicyRow);
175+
const newScore = patternSpecificity(pattern);
176+
let idx = committed.findIndex((r) => patternSpecificity(r.pattern) <= newScore);
177+
if (idx === -1) idx = committed.length; // below every more-specific rule
178+
const prev = idx === 0 ? null : committed[idx - 1]!.position;
179+
const next = idx === committed.length ? null : committed[idx]!.position;
180+
return generateKeyBetween(prev, next);
181+
};
182+
142183
const actionRestrictionRank = (action: ToolPolicyAction): number =>
143184
Match.value(action).pipe(
144185
Match.when("block", () => 3),

‎packages/core/sdk/src/shared.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ export {
8888
isValidPattern,
8989
effectivePolicyFromSorted,
9090
comparePolicyRow,
91+
patternSpecificity,
92+
positionForNewPattern,
9193
ToolPolicyActionSchema,
9294
type ToolPolicy,
9395
type CreateToolPolicyInput,

‎packages/react/src/hooks/use-policy-actions.ts‎

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import { useCallback, useMemo } from "react";
22
import { useAtomSet, useAtomValue } from "@effect/atom-react";
33
import * as AsyncResult from "effect/unstable/reactivity/AsyncResult";
4-
import { generateKeyBetween } from "fractional-indexing";
5-
import { PolicyId, type Owner, type ToolPolicyAction } from "@executor-js/sdk/shared";
4+
import {
5+
PolicyId,
6+
positionForNewPattern,
7+
type Owner,
8+
type ToolPolicyAction,
9+
} from "@executor-js/sdk/shared";
610

711
import {
812
createPolicyOptimistic,
@@ -13,24 +17,6 @@ import {
1317
import { policyWriteKeys } from "../api/reactivity-keys";
1418
import { trackEvent } from "../api/analytics";
1519

16-
// Specificity score for ordering. Higher = more specific = should sit at a
17-
// lower position-key (higher precedence). New rules are auto-placed below
18-
// any more-specific existing rules so a freshly-added group rule never
19-
// silently shadows an existing leaf rule.
20-
// `*` → 0
21-
// `vercel.*` → 2 (1 literal segment, wildcard)
22-
// `vercel.dns.*` → 4 (2 literal segments, wildcard)
23-
// `vercel.dns` → 5 (2 literal segments, exact — beats same-prefix wildcard)
24-
// `vercel.dns.create` → 7 (3 literal segments, exact)
25-
const specificity = (pattern: string): number => {
26-
if (pattern === "*") return 0;
27-
if (pattern.endsWith(".*")) {
28-
const prefix = pattern.slice(0, -2);
29-
return prefix.split(".").length * 2;
30-
}
31-
return pattern.split(".").length * 2 + 1;
32-
};
33-
3420
export interface PolicyAction {
3521
/** Set the action on a pattern. If a user rule with this exact pattern
3622
* already exists, update it. Otherwise create with auto-placed
@@ -83,20 +69,15 @@ export const usePolicyActions = (owner: Owner = "org"): PolicyAction => {
8369

8470
const busy = policies.waiting;
8571

72+
// Specificity-aware placement (below any more-specific rule) via the shared
73+
// sdk helper — the same computation the server applies when no position is
74+
// sent. Computing it here too keeps the optimistic UI's final order stable;
75+
// if this client's list is stale, the server default is the backstop.
8676
const computePosition = useCallback(
8777
(newPattern: string): string | undefined => {
8878
const committed = sorted.filter((r) => r.position !== "");
8979
if (committed.length === 0) return undefined;
90-
const newScore = specificity(newPattern);
91-
// Walk down the list (most-precedent first); place the new rule
92-
// just before the first existing rule whose specificity is <= the
93-
// new one. That way more-specific rules stay above us, and we win
94-
// against everything equally or less specific.
95-
let idx = committed.findIndex((r) => specificity(r.pattern) <= newScore);
96-
if (idx === -1) idx = committed.length; // append at bottom
97-
const prev = idx === 0 ? null : committed[idx - 1]!.position;
98-
const next = idx === committed.length ? null : committed[idx]!.position;
99-
return generateKeyBetween(prev, next);
80+
return positionForNewPattern(newPattern, committed);
10081
},
10182
[sorted],
10283
);

0 commit comments

Comments
 (0)