From 718c2bb033c5bc6e68b1ff5f47dd20d896dee817 Mon Sep 17 00:00:00 2001 From: midego1 Date: Thu, 6 Aug 2026 06:25:17 +0000 Subject: [PATCH 1/3] Confirm before revoking a personal API key Personal-key revoke deleted the key immediately with no confirmation, while org-key revoke (and every other destructive action) asks first. A personal key breaks any script or tool using it just the same, so add the same confirmation dialog, mirroring the org-key pattern. --- packages/react/src/pages/api-keys.tsx | 46 +++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/packages/react/src/pages/api-keys.tsx b/packages/react/src/pages/api-keys.tsx index eb15084cb..b48e04856 100644 --- a/packages/react/src/pages/api-keys.tsx +++ b/packages/react/src/pages/api-keys.tsx @@ -233,6 +233,7 @@ export function ApiKeysPage(props: { readonly orgKeysSection?: ReactNode }) { // stays mounted for Radix's exit animation (see CreateKeyDialogBody). const [openCount, setOpenCount] = useState(0); const [revokingId, setRevokingId] = useState(null); + const [confirmRevoke, setConfirmRevoke] = useState(null); const handleCreate = async (name: string): Promise => { const exit = await doCreate({ payload: { name }, reactivityKeys: apiKeyWriteKeys }); @@ -312,7 +313,11 @@ export function ApiKeysPage(props: { readonly orgKeysSection?: ReactNode }) {

) : ( - + setConfirmRevoke(key)} + /> ), }) )} @@ -333,6 +338,42 @@ export function ApiKeysPage(props: { readonly orgKeysSection?: ReactNode }) { /> + + {/* A personal key revoke breaks any script or tool using it, so it asks + first — same as the org-key revoke. */} + { + if (!open) setConfirmRevoke(null); + }} + > + + + Revoke API key + + {confirmRevoke + ? `Revoke ${confirmRevoke.name}? Any script or tool authenticating with it loses access immediately. This cannot be undone.` + : ""} + + + + + + + + + + ); } @@ -492,8 +533,7 @@ function OrgApiKeysSectionBody() { - {/* Revoking an org key breaks every backend using it, so it is the one - revoke on this page that asks first. */} + {/* Revoking an org key breaks every backend using it, so it asks first. */} { From 929b5121246068d721aacfc7ac9b7d4fcf143c9b Mon Sep 17 00:00:00 2001 From: midego1 Date: Thu, 6 Aug 2026 07:49:50 +0000 Subject: [PATCH 2/3] Confirm before removing a policy or an org member MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing a tool-access policy and removing an org member each fired instantly from a dropdown click, with no confirmation — unlike every other destructive action in the UI. Add the same AlertDialog confirm the connection and artifact removals already use (state held at the page level, since the dropdown menu unmounts a nested dialog on click). --- packages/react/src/pages/org.tsx | 51 ++++++++++++++++++++++++++- packages/react/src/pages/policies.tsx | 51 ++++++++++++++++++++++++++- 2 files changed, 100 insertions(+), 2 deletions(-) diff --git a/packages/react/src/pages/org.tsx b/packages/react/src/pages/org.tsx index 8797533af..670c0845f 100644 --- a/packages/react/src/pages/org.tsx +++ b/packages/react/src/pages/org.tsx @@ -14,6 +14,16 @@ import { DialogFooter, DialogClose, } from "../components/dialog"; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from "../components/alert-dialog"; import { Button } from "../components/button"; import { PageContainer, PageHeader } from "../components/page"; import { Badge } from "../components/badge"; @@ -153,6 +163,7 @@ export function OrgPage(props: { const refreshMembers = useAtomRefresh(orgMembersAtom); const rolesResult = useAtomValue(orgRolesAtom); const doRemove = useAtomSet(removeMember, { mode: "promiseExit" }); + const [removingMember, setRemovingMember] = useState<{ id: string; name: string } | null>(null); const doUpdateRole = useAtomSet(updateMemberRole, { mode: "promiseExit" }); const doUpdateOrgName = useAtomSet(updateOrgName, { mode: "promiseExit" }); const [inviteOpen, setInviteOpen] = useState(false); @@ -179,6 +190,7 @@ export function OrgPage(props: { const showUpgradeOnInvite = atSeatLimit && !!props.upgradeAction; const handleRemove = async (membershipId: string, name: string) => { + setRemovingMember(null); const exit = await doRemove({ params: { membershipId }, reactivityKeys: orgMemberWriteKeys, @@ -406,7 +418,12 @@ export function OrgPage(props: { )} handleRemove(member.id, member.name ?? member.email)} + onClick={() => + setRemovingMember({ + id: member.id, + name: member.name ?? member.email, + }) + } > Remove member @@ -426,6 +443,38 @@ export function OrgPage(props: { {props.dangerZoneSection} + { + if (!open) setRemovingMember(null); + }} + > + + + + {removingMember ? `Remove ${removingMember.name}?` : "Remove member?"} + + + They lose access to this organization immediately. This cannot be undone; you would + need to invite them again. + + + + Cancel + { + if (removingMember !== null) { + void handleRemove(removingMember.id, removingMember.name); + } + }} + > + Remove member + + + + + (null); const ownerDisplay = useOwnerDisplay(); // Policies default to org/workspace. On local this is the hidden Local owner // that v1 local data migrates into. @@ -336,6 +351,7 @@ export function PoliciesPage() { }; const handleRemove = async (policy: { id: string; owner: Owner }) => { + setRemovingPolicy(null); const exit = await doRemove({ params: { policyId: PolicyId.make(policy.id) }, payload: { owner: policy.owner }, @@ -462,7 +478,9 @@ export function PoliciesPage() { isFirst={!reorderable || j === 0} isLast={!reorderable || j === committed.length - 1} showOwnerLabel={ownerDisplay.showOwnerLabels} - onRemove={() => handleRemove({ id: p.id, owner: p.owner })} + onRemove={() => + setRemovingPolicy({ id: p.id, owner: p.owner, pattern: p.pattern }) + } onChangeAction={(action) => handleUpdate({ id: p.id, owner: p.owner }, action) } @@ -490,6 +508,37 @@ export function PoliciesPage() { }, }) )} + + { + if (!open) setRemovingPolicy(null); + }} + > + + + Remove policy? + + {removingPolicy + ? `The rule for "${removingPolicy.pattern}" will be deleted. Tools it governs fall back to the default policy. This cannot be undone.` + : ""} + + + + Cancel + { + if (removingPolicy !== null) { + void handleRemove({ id: removingPolicy.id, owner: removingPolicy.owner }); + } + }} + > + Remove policy + + + + ); } From a8dcb9522ed9398c05d35b22c6fd42e7480aa384 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Fri, 28 Aug 2026 02:07:33 -0700 Subject: [PATCH 3/3] Cover the policy remove confirmation with a browser scenario --- e2e/scenarios/policy-remove-confirm.test.ts | 85 +++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 e2e/scenarios/policy-remove-confirm.test.ts diff --git a/e2e/scenarios/policy-remove-confirm.test.ts b/e2e/scenarios/policy-remove-confirm.test.ts new file mode 100644 index 000000000..8e6f10382 --- /dev/null +++ b/e2e/scenarios/policy-remove-confirm.test.ts @@ -0,0 +1,85 @@ +// Cross-target (browser): removing a tool-access policy asks for confirmation +// first. Remove fires from a row's dropdown menu and is irreversible — the +// rule is gone and every tool it governed silently falls back to the default +// policy — so the menu item must open a confirm dialog rather than firing the +// mutation directly. Cancel keeps the rule; confirming removes it for real +// (asserted through the API, not just the rendered list). +import { randomBytes } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Target } from "../src/services"; +import { visit } from "../src/surfaces/browser"; + +const coreApi = composePluginApi([] as const); + +scenario( + "Policies (UI) · Remove asks for confirmation; cancel keeps, confirm removes", + {}, + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const { client: makeClient } = yield* Api; + const identity = yield* target.newIdentity(); + const client = yield* makeClient(coreApi, identity); + + // Selfhost scenarios share one workspace, so the pattern is unique to this + // run and the row lookup below cannot match another scenario's rule. + const pattern = `rmconfirm-${randomBytes(4).toString("hex")}.*`; + + const created = yield* client.policies.create({ + payload: { owner: "org", pattern, action: "block" }, + }); + + yield* Effect.ensuring( + Effect.gen(function* () { + yield* browser.session(identity, async ({ page, step }) => { + const row = page.locator("[data-slot='card-stack-entry']").filter({ hasText: pattern }); + const menuTrigger = row.locator('button[aria-haspopup="menu"]'); + const confirm = page.getByRole("alertdialog"); + + await step("Open the policies page", async () => { + await visit(page, "/policies"); + await row.waitFor(); + }); + + await step("Remove asks for confirmation instead of firing", async () => { + await menuTrigger.click(); + await page.getByRole("menuitem", { name: "Remove" }).click(); + await confirm.getByText("Remove policy?").waitFor(); + // The dialog names the exact rule being destroyed. + await confirm.getByText(pattern, { exact: false }).waitFor(); + }); + + await step("Cancel keeps the policy", async () => { + await confirm.getByRole("button", { name: "Cancel" }).click(); + await confirm.waitFor({ state: "detached" }); + await row.waitFor(); + }); + + await step("Confirming actually removes it", async () => { + await menuTrigger.click(); + await page.getByRole("menuitem", { name: "Remove" }).click(); + await confirm.getByRole("button", { name: "Remove policy" }).click(); + await confirm.waitFor({ state: "detached" }); + await row.waitFor({ state: "detached" }); + }); + }); + + // Cancel left the removal un-fired and confirm fired it for real: the + // rule is gone from the API, not merely from the rendered list. + const remaining = yield* client.policies.list(); + expect( + remaining.map((policy) => policy.pattern), + "the removed policy is gone from the API", + ).not.toContain(pattern); + }), + client.policies + .remove({ params: { policyId: created.id }, payload: { owner: "org" } }) + .pipe(Effect.ignore), + ); + }), +);