fix(ui): stop offering access group budget writes that the proxy refuses

An Admin Viewer and a group whose name contains a slash both reached an
enabled Set budget action that could only ever come back 403 or 404. Gate
the row actions on proxy admin and on the name being addressable, with the
reason in the tooltip.
This commit is contained in:
ryan-crabbe-berri 2026-08-29 16:45:48 -07:00
parent a5cd3fae81
commit 7c0e58ed06
3 changed files with 68 additions and 9 deletions

View file

@ -19,14 +19,30 @@ import { ModelAccessGroup } from "@/app/(dashboard)/hooks/modelAccessGroups/useM
const budgetDecimals = (maxBudget: number | null | undefined): number =>
maxBudget != null && maxBudget > 0 && maxBudget < 0.01 ? 5 : 2;
/**
* A group name is a free-text path segment on the budget routes, so a `/` in it splits the path and
* no encoding recovers it. Such a group is listed but its budget is unreachable.
*/
export const isBudgetAddressable = (accessGroup: string): boolean => !accessGroup.includes("/");
const writeBlockedReason = (accessGroup: ModelAccessGroup, canWrite: boolean): string | undefined => {
if (!canWrite) return "Only a proxy admin can change an access group budget";
if (!isBudgetAddressable(accessGroup.access_group)) {
return "A budget cannot be set on a group whose name contains a slash";
}
return undefined;
};
interface AccessGroupRowActionsProps {
accessGroup: ModelAccessGroup;
canWrite: boolean;
onSetBudget: (accessGroup: ModelAccessGroup) => void;
onClearBudget: (accessGroup: ModelAccessGroup) => void;
}
function AccessGroupRowActions({ accessGroup, onSetBudget, onClearBudget }: AccessGroupRowActionsProps) {
function AccessGroupRowActions({ accessGroup, canWrite, onSetBudget, onClearBudget }: AccessGroupRowActionsProps) {
const hasBudget = accessGroup.budget != null;
const blocked = writeBlockedReason(accessGroup, canWrite);
return (
<DropdownMenu>
@ -38,15 +54,20 @@ function AccessGroupRowActions({ accessGroup, onSetBudget, onClearBudget }: Acce
<MoreHorizontal className="size-4" />
</DropdownMenuTrigger>
<DropdownMenuContent align="end" className="w-52">
<DropdownMenuItem data-testid="access-group-action-set-budget" onClick={() => onSetBudget(accessGroup)}>
<DropdownMenuItem
disabled={blocked !== undefined}
title={blocked}
data-testid="access-group-action-set-budget"
onClick={() => onSetBudget(accessGroup)}
>
<Wallet />
{hasBudget ? "Edit budget" : "Set budget"}
</DropdownMenuItem>
<DropdownMenuItem
variant="destructive"
disabled={!hasBudget}
disabled={blocked !== undefined || !hasBudget}
data-testid="access-group-action-clear-budget"
title={hasBudget ? undefined : "This access group has no budget to clear"}
title={blocked ?? (hasBudget ? undefined : "This access group has no budget to clear")}
onClick={() => onClearBudget(accessGroup)}
>
<Trash2 />
@ -58,11 +79,13 @@ function AccessGroupRowActions({ accessGroup, onSetBudget, onClearBudget }: Acce
}
interface AccessGroupBudgetColumnsDeps {
canWrite: boolean;
onSetBudget: (accessGroup: ModelAccessGroup) => void;
onClearBudget: (accessGroup: ModelAccessGroup) => void;
}
export const getAccessGroupBudgetColumns = ({
canWrite,
onSetBudget,
onClearBudget,
}: AccessGroupBudgetColumnsDeps): ColumnDef<ModelAccessGroup>[] => [
@ -132,7 +155,12 @@ export const getAccessGroupBudgetColumns = ({
enableHiding: false,
cell: ({ row }) => (
<div className="flex justify-end">
<AccessGroupRowActions accessGroup={row.original} onSetBudget={onSetBudget} onClearBudget={onClearBudget} />
<AccessGroupRowActions
accessGroup={row.original}
canWrite={canWrite}
onSetBudget={onSetBudget}
onClearBudget={onClearBudget}
/>
</div>
),
},

View file

@ -4,11 +4,16 @@ import userEvent from "@testing-library/user-event";
import React from "react";
import { beforeEach, describe, expect, it, vi } from "vitest";
const { GET, PUT, DELETE } = vi.hoisted(() => ({ GET: vi.fn(), PUT: vi.fn(), DELETE: vi.fn() }));
const { GET, PUT, DELETE, userRole } = vi.hoisted(() => ({
GET: vi.fn(),
PUT: vi.fn(),
DELETE: vi.fn(),
userRole: { current: "Admin" },
}));
vi.mock("@/lib/http/api", () => ({ fetchClient: { GET, PUT, DELETE } }));
vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({
default: () => ({ accessToken: "sk-test", userRole: "Admin" }),
default: () => ({ accessToken: "sk-test", userRole: userRole.current }),
}));
import AccessGroupBudgetsPanel from "./AccessGroupBudgetsPanel";
@ -51,6 +56,7 @@ const openActions = async (accessGroup: string) => {
describe("AccessGroupBudgetsPanel", () => {
beforeEach(() => {
vi.clearAllMocks();
userRole.current = "Admin";
GET.mockResolvedValue({ data: { access_groups: [BUDGETED_GROUP, FREE_GROUP] } });
PUT.mockResolvedValue({ data: { access_group: "shared", spend: 0, budget: null } });
DELETE.mockResolvedValue({ data: { access_group: "premium", budget_deleted: true, message: "ok" } });
@ -120,6 +126,27 @@ describe("AccessGroupBudgetsPanel", () => {
expect(PUT).not.toHaveBeenCalled();
});
it("offers an admin viewer no way to start a write the proxy would reject with a 403", async () => {
userRole.current = "Admin Viewer";
renderPanel();
expect(await screen.findByText("premium")).toBeInTheDocument();
await openActions("premium");
expect(await screen.findByTestId("access-group-action-set-budget")).toHaveAttribute("aria-disabled", "true");
expect(screen.getByTestId("access-group-action-clear-budget")).toHaveAttribute("aria-disabled", "true");
});
it("does not offer a budget on a group whose name a path segment cannot carry", async () => {
GET.mockResolvedValue({ data: { access_groups: [{ ...FREE_GROUP, access_group: "openai/prod" }] } });
renderPanel();
await openActions("openai/prod");
expect(await screen.findByTestId("access-group-action-set-budget")).toHaveAttribute("aria-disabled", "true");
});
it("clears a budget only after the confirmation is accepted", async () => {
renderPanel();
await openActions("premium");

View file

@ -7,6 +7,8 @@ import React, { useMemo, useState } from "react";
import DeleteResourceModal from "@/components/common_components/DeleteResourceModal";
import { DataTable } from "@/components/shared/DataTable";
import { toast } from "@/lib/toast";
import { isProxyAdminRole } from "@/utils/roles";
import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized";
import { ModelAccessGroup, useModelAccessGroups } from "@/app/(dashboard)/hooks/modelAccessGroups/useModelAccessGroups";
import { useDeleteModelAccessGroupBudget } from "@/app/(dashboard)/hooks/modelAccessGroups/useDeleteModelAccessGroupBudget";
import {
@ -33,6 +35,7 @@ function EmptyState() {
}
export default function AccessGroupBudgetsPanel() {
const { userRole } = useAuthorized();
const { data: accessGroups, isLoading } = useModelAccessGroups();
const setBudget = useSetModelAccessGroupBudget();
const clearBudget = useDeleteModelAccessGroupBudget();
@ -41,9 +44,10 @@ export default function AccessGroupBudgetsPanel() {
const [editing, setEditing] = useState<ModelAccessGroup | null>(null);
const [clearing, setClearing] = useState<ModelAccessGroup | null>(null);
const canWrite = isProxyAdminRole(userRole ?? "");
const columns = useMemo(
() => getAccessGroupBudgetColumns({ onSetBudget: setEditing, onClearBudget: setClearing }),
[],
() => getAccessGroupBudgetColumns({ canWrite, onSetBudget: setEditing, onClearBudget: setClearing }),
[canWrite],
);
const handleSubmit = (params: SetModelAccessGroupBudgetParams) => {