From c613864e58b52dd239086807242de385d27fbadc Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 30 Sep 2026 11:43:40 -0700 Subject: [PATCH] fix(web): tool permissions ride the definition's grant requirements (CL-9638) --- apps/web/src/agent-deploy.ts | 22 ++++++++ apps/web/src/pages/worker-page.tsx | 57 ++++++++++--------- apps/web/src/settings/tenancy-api.ts | 11 ++++ apps/web/src/worker-grants-carry.ts | 83 ---------------------------- apps/web/src/worker-tool-grants.ts | 41 ++++++++++---- 5 files changed, 92 insertions(+), 122 deletions(-) delete mode 100644 apps/web/src/worker-grants-carry.ts diff --git a/apps/web/src/agent-deploy.ts b/apps/web/src/agent-deploy.ts index 6f41a38733..b9e2fee0af 100644 --- a/apps/web/src/agent-deploy.ts +++ b/apps/web/src/agent-deploy.ts @@ -22,6 +22,13 @@ import type { McpServer } from "./mcp-servers"; import { resolveExistingOffering } from "./onboarding/provider-connect-step"; import { isValidSlug, slugify } from "@/lib/slug"; +/** A non-default tool permission (`ask` is the stock default) to carry onto + * a redeployed run. */ +export type ToolEffect = { + readonly resource: string; + readonly effect: "allow" | "deny"; +}; + export class AgentDeployError extends Error {} const TenantDomainShape = type({ domain: "string" }); @@ -51,6 +58,9 @@ export function buildAgentDefinitionJson(args: { * requirements Worker carries, so remote tools stay ask-gated except the * read-only ones the server itself annotates. */ mcpServers: readonly McpServerDeployment[]; + /** Tool permissions the person set on the previous run, re-declared so the + * new run materializes them at its first trigger. */ + toolEffects: readonly ToolEffect[]; }): unknown { const stepId = "run"; return { @@ -70,6 +80,12 @@ export function buildAgentDefinitionJson(args: { artifactToolsCredentialUseRequirement(args.hubCredentialId), memoryToolsCredentialUseRequirement(args.hubCredentialId), ...args.mcpServers.map((server) => mcpServerCredentialUseRequirement(server.credentialId)), + ...args.toolEffects.map((tool) => ({ + resource: tool.resource, + action: "invoke", + effect: tool.effect, + source: "creator", + })), ], steps: { [stepId]: { @@ -106,6 +122,7 @@ async function renderAgentSourceTree( readonly declaredSources: readonly { readonly provider: string; readonly model: string }[]; readonly hubCredentialId: string; readonly mcpServers: readonly McpServerDeployment[]; + readonly toolEffects: readonly ToolEffect[]; }, ): Promise> { const { WORKER_BUNDLE_BUILD_EXPORT, WORKER_DIRECTORS_BUNDLE, WORKER_WORKFLOW_BUNDLE } = @@ -134,6 +151,7 @@ async function renderAgentSourceTree( declaredSources: args.declaredSources, hubCredentialId: args.hubCredentialId, mcpServers: args.mcpServers, + toolEffects: args.toolEffects, }), ), }); @@ -177,6 +195,9 @@ export type NewAgentInput = { * row is created targeting this agent's definition, so the ticker mails * its live run on that cadence. */ readonly schedule?: string; + /** Tool permissions to re-declare on a redeploy, as creator-sourced grant + * requirements. */ + readonly toolEffects?: readonly ToolEffect[]; }; /** Maps requested workspace-catalog handles to deployments. Fails closed on @@ -335,6 +356,7 @@ export async function deployAgentSource( fetchImpl, ), mcpServers, + toolEffects: args.input.toolEffects ?? [], }), entry: "./workflow.js", sourceOfferingIds: offering.sourceOfferingIds, diff --git a/apps/web/src/pages/worker-page.tsx b/apps/web/src/pages/worker-page.tsx index 5bd5c9cf46..08760825d3 100644 --- a/apps/web/src/pages/worker-page.tsx +++ b/apps/web/src/pages/worker-page.tsx @@ -13,11 +13,16 @@ import { isDefaultWorker, listChatAgents, type ChatAgent } from "@/chat/threads- import { ChatCircle } from "@/lib/icons"; import { QueryView, describeApiError } from "@/lib/api-query"; import { listTopLevelRuns } from "../agents-api"; -import { createGrant, revokeGrant, type Grant } from "../settings/tenancy-api"; +import { updateGrant, type Grant } from "../settings/tenancy-api"; import { agentSlugFromSourceAssetName, deployAgentSource } from "../agent-deploy"; import { readAgentMcpHandles, readAgentSource } from "../agent-source-read"; -import { readGrantSnapshot, saveGrantSnapshot } from "../worker-grants-carry"; -import { TOOL_PREFIX, readWorkerToolGrants, workerToolGrantsKey } from "../worker-tool-grants"; +import { + TOOL_PREFIX, + effectiveToolGrants, + readWorkerToolGrants, + toolEffectsToCarry, + workerToolGrantsKey, +} from "../worker-tool-grants"; import { useBench } from "../bench-context"; import { Link } from "../navigation"; import { tenantKeys } from "../query-client"; @@ -58,12 +63,15 @@ function InstructionsCard({ if (slug === null) throw new Error(`${agent.assetName} is not an editable worker`); const mcpHandles = await readAgentMcpHandles(tenantId, agent.id, agent.assetName); const current = await readWorkerToolGrants(tenantId, agent); - if (current !== null) { - saveGrantSnapshot(tenantId, agent.assetName, current.principalId, current.tools); - } await deployAgentSource({ tenantId, - input: { name: agent.name, systemPrompt, slug, mcpHandles }, + input: { + name: agent.name, + systemPrompt, + slug, + mcpHandles, + toolEffects: current === null ? [] : toolEffectsToCarry(current.tools), + }, }); }, onSuccess: async () => { @@ -144,34 +152,24 @@ function PermissionsCard({ }) { const queryClient = useQueryClient(); const key = workerToolGrantsKey(tenantId, agent.id); - // While a redeploy's permissions are waiting for the new run to start, - // keep looking for its principal. - const carrying = readGrantSnapshot(tenantId, agent.assetName) !== null; const query = useQuery({ queryKey: key, queryFn: () => readWorkerToolGrants(tenantId, agent), - refetchInterval: carrying ? 5000 : false, }); - // Create the new grant before revoking the old one so a failure never - // leaves the tool ungoverned. + // Every row for the tool moves together, so a redeploy's requirement + // beside the tool's own row can't outrank the choice. const setMode = useMutation({ mutationFn: async (input: { - readonly principalId: string; - readonly grant: Grant; + readonly grants: readonly Grant[]; + readonly resource: string; readonly effect: GrantEffect; }) => { - await createGrant(tenantId, { - principalId: input.principalId, - resource: input.grant.resource, - action: "invoke", - effect: input.effect, - origin: "creator", - }); - await revokeGrant(tenantId, input.grant.id); + const rows = input.grants.filter((grant) => grant.resource === input.resource); + await Promise.all(rows.map((row) => updateGrant(tenantId, row.id, { effect: input.effect }))); }, onSuccess: (_data, input) => { - toast(`${input.grant.resource.slice(TOOL_PREFIX.length)}: ${input.effect}`); + toast(`${input.resource.slice(TOOL_PREFIX.length)}: ${input.effect}`); }, onError: (cause) => { reportError(cause, { operation: "worker_permission_set", tenantId }); @@ -199,9 +197,9 @@ function PermissionsCard({

) : (
- {query.data.tools.map((grant) => { + {effectiveToolGrants(query.data.tools).map((grant) => { const name = grant.resource.slice(TOOL_PREFIX.length); - const principalId = query.data?.principalId ?? ""; + const all = query.data?.tools ?? []; return (
{ - if (!active) setMode.mutate({ principalId, grant, effect: mode.effect }); + if (!active) + setMode.mutate({ + grants: all, + resource: grant.resource, + effect: mode.effect, + }); }} className={`h-[26px] rounded-(--r-sm) px-2.5 text-[12.5px] font-semibold ${ active ? "bg-(--card) text-(--ink) shadow-sm" : "text-(--ink-3)" diff --git a/apps/web/src/settings/tenancy-api.ts b/apps/web/src/settings/tenancy-api.ts index 84586dc3b2..f77e7f3530 100644 --- a/apps/web/src/settings/tenancy-api.ts +++ b/apps/web/src/settings/tenancy-api.ts @@ -190,6 +190,17 @@ export function createGrant(tenantId: string, input: CreateGrantInput): Promise< }); } +export function updateGrant( + tenantId: string, + grantId: string, + patch: { readonly effect: GrantEffect }, +): Promise { + return request(`/api/tenants/${tenantId}/grants/${grantId}`, GrantResponse, { + method: "PATCH", + body: JSON.stringify(patch), + }); +} + export function revokeGrant(tenantId: string, grantId: string): Promise { return request(`/api/tenants/${tenantId}/grants/${grantId}`, (data) => data as void, { method: "DELETE", diff --git a/apps/web/src/worker-grants-carry.ts b/apps/web/src/worker-grants-carry.ts deleted file mode 100644 index 0dd8876dc6..0000000000 --- a/apps/web/src/worker-grants-carry.ts +++ /dev/null @@ -1,83 +0,0 @@ -// A redeploy mints a new run, and runtime tool grants live on the run's -// principal, so non-default effects are snapshotted before the redeploy and -// re-applied once the new run's principal exists. - -import { type } from "arktype"; -import { reportError } from "@corbits/error-sink"; -import type { GrantEffect } from "@intx/types"; - -import { createGrant, revokeGrant, type Grant } from "./settings/tenancy-api"; - -const TOOL_PREFIX = "tool:"; - -const Snapshot = type({ - fromPrincipalId: "string", - effects: type({ resource: "string", effect: "'allow' | 'deny'" }).array(), -}); -export type GrantSnapshot = typeof Snapshot.infer; - -const storageKey = (tenantId: string, assetName: string) => - `workbench:worker-grants:${tenantId}:${assetName}`; - -export function readGrantSnapshot(tenantId: string, assetName: string): GrantSnapshot | null { - try { - const raw = localStorage.getItem(storageKey(tenantId, assetName)); - if (raw === null) return null; - const parsed = Snapshot(JSON.parse(raw)); - return parsed instanceof type.errors ? null : parsed; - } catch (cause) { - reportError(cause, { operation: "worker_grants_snapshot_read", tenantId }); - return null; - } -} - -function writeGrantSnapshot(tenantId: string, assetName: string, snapshot: GrantSnapshot | null) { - try { - if (snapshot === null) localStorage.removeItem(storageKey(tenantId, assetName)); - else localStorage.setItem(storageKey(tenantId, assetName), JSON.stringify(snapshot)); - } catch (cause) { - reportError(cause, { operation: "worker_grants_snapshot_write", tenantId }); - } -} - -/** Stock grants default to "ask"; only allow/deny differ from the default. */ -export function saveGrantSnapshot( - tenantId: string, - assetName: string, - principalId: string, - tools: readonly Grant[], -) { - const effects = tools.flatMap((grant) => - grant.resource.startsWith(TOOL_PREFIX) && (grant.effect === "allow" || grant.effect === "deny") - ? [{ resource: grant.resource, effect: grant.effect }] - : [], - ); - if (effects.length === 0) return; - writeGrantSnapshot(tenantId, assetName, { fromPrincipalId: principalId, effects }); -} - -/** Re-applies the snapshot onto a new run principal (create-then-revoke), - * then clears it. Returns true when grants were applied. */ -export async function applyGrantSnapshot( - tenantId: string, - assetName: string, - principalId: string, - tools: readonly Grant[], -): Promise { - const snapshot = readGrantSnapshot(tenantId, assetName); - if (snapshot === null || snapshot.fromPrincipalId === principalId) return false; - for (const { resource, effect } of snapshot.effects) { - const current = tools.find((grant) => grant.resource === resource); - if (current === undefined || current.effect === effect) continue; - await createGrant(tenantId, { - principalId, - resource, - action: "invoke", - effect: effect satisfies GrantEffect, - origin: "creator", - }); - await revokeGrant(tenantId, current.id); - } - writeGrantSnapshot(tenantId, assetName, null); - return true; -} diff --git a/apps/web/src/worker-tool-grants.ts b/apps/web/src/worker-tool-grants.ts index 0a7a6d67b4..cd976b45d0 100644 --- a/apps/web/src/worker-tool-grants.ts +++ b/apps/web/src/worker-tool-grants.ts @@ -4,8 +4,8 @@ import { listTopLevelRuns } from "./agents-api"; import type { ChatAgent } from "./chat/threads-api"; import { tenantKeys } from "./query-client"; -import { listGrants, listPrincipals } from "./settings/tenancy-api"; -import { applyGrantSnapshot } from "./worker-grants-carry"; +import { listGrants, listPrincipals, type Grant } from "./settings/tenancy-api"; +import type { ToolEffect } from "./agent-deploy"; export function workerToolGrantsKey(tenantId: string, agentId: string) { return [...tenantKeys.grants(tenantId), "worker", agentId] as const; @@ -26,15 +26,32 @@ export async function readWorkerToolGrants(tenantId: string, agent: ChatAgent) { (candidate) => candidate.kind === "workflow" && candidate.refId === run?.id, ); if (principal === undefined) return null; - const read = async () => { - const grants = await listGrants(tenantId, { principalId: principal.id }); - return grants - .filter((grant) => grant.resource.startsWith(TOOL_PREFIX) && grant.action === "invoke") - .sort((a, b) => a.resource.localeCompare(b.resource)); - }; - let tools = await read(); - if (await applyGrantSnapshot(tenantId, agent.assetName, principal.id, tools)) { - tools = await read(); - } + const grants = await listGrants(tenantId, { principalId: principal.id }); + const tools = grants + .filter((grant) => grant.resource.startsWith(TOOL_PREFIX) && grant.action === "invoke") + .sort((a, b) => a.resource.localeCompare(b.resource)); return { principalId: principal.id, tools }; } + +const EFFECT_RANK = { allow: 0, ask: 1, deny: 2 } as const; + +/** One row per tool resource, carrying the effect that wins at runtime + * (deny over ask over allow): a redeploy's requirement can sit beside the + * tool's own floor row, and the stronger one decides. */ +export function effectiveToolGrants(tools: readonly Grant[]): readonly Grant[] { + const byResource = new Map(); + for (const grant of tools) { + const held = byResource.get(grant.resource); + if (held === undefined || EFFECT_RANK[grant.effect] > EFFECT_RANK[held.effect]) { + byResource.set(grant.resource, grant); + } + } + return [...byResource.values()]; +} + +/** The non-ask tool effects to re-declare on a redeploy. */ +export function toolEffectsToCarry(tools: readonly Grant[]): readonly ToolEffect[] { + return effectiveToolGrants(tools).flatMap((grant) => + grant.effect === "ask" ? [] : [{ resource: grant.resource, effect: grant.effect }], + ); +}