From d9670522bb6b283c4ccc119599e27f7b36b7072e Mon Sep 17 00:00:00 2001 From: Ahmad Al Tamimi Date: Fri, 21 Aug 2026 17:44:51 +0400 Subject: [PATCH] fix: gate the recent-activity fetch on audit:read /api/logs/activity requires audit:read, which no default non-admin role holds, so every non-admin home load fired a guaranteed 403. Add an `enabled` option to useRecentActivity, mirroring useSystemHealth's flag for the admin-only /version probe, and gate the home call on it. Signed-off-by: Ahmad Al Tamimi --- src/hooks/useMiniCardStatuses.test.tsx | 17 ++++++++++++++++- src/hooks/useMiniCardStatuses.ts | 9 +++++++-- src/hooks/useRecentActivity.test.tsx | 24 ++++++++++++++++++++++++ src/hooks/useRecentActivity.ts | 16 +++++++++++++--- 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/hooks/useMiniCardStatuses.test.tsx b/src/hooks/useMiniCardStatuses.test.tsx index ae3f72c..a4d6130 100644 --- a/src/hooks/useMiniCardStatuses.test.tsx +++ b/src/hooks/useMiniCardStatuses.test.tsx @@ -41,7 +41,8 @@ function health(over: { data?: VersionInfo; error?: { message: string; status?: function admin(isAdmin = true) { mockUseAuth.mockReturnValue({ - hasPermission: (perm: string) => isAdmin && perm === "admin.system_config", + hasPermission: (perm: string) => + isAdmin && (perm === "admin.system_config" || perm === "audit:read"), } as unknown as ReturnType); } @@ -102,6 +103,20 @@ describe("useMiniCardStatuses — /version gating", () => { }); }); +describe("useMiniCardStatuses — activity gating", () => { + it("does not fetch activity for a caller without audit:read", () => { + admin(false); + renderHook(() => useMiniCardStatuses()); + expect(mockUseRecentActivity).toHaveBeenCalledWith({ pollIntervalMs: 0, enabled: false }); + }); + + it("fetches activity when the caller holds audit:read", () => { + admin(true); + renderHook(() => useMiniCardStatuses()); + expect(mockUseRecentActivity).toHaveBeenCalledWith({ pollIntervalMs: 0, enabled: true }); + }); +}); + describe("useMiniCardStatuses — headline health axis", () => { it("stays optimistic (reachable undefined) while health is loading / for a non-admin", () => { admin(false); // no /version -> no data, no error diff --git a/src/hooks/useMiniCardStatuses.ts b/src/hooks/useMiniCardStatuses.ts index c430d7b..f69ac53 100644 --- a/src/hooks/useMiniCardStatuses.ts +++ b/src/hooks/useMiniCardStatuses.ts @@ -12,7 +12,8 @@ * `/version` is admin-only, so it is fetched only when the caller can view * system diagnostics (`admin.system_config`); non-admins never poll a guaranteed * 403. It is polled once here (the hook is resolved at the page level) and feeds - * both the mini cards and the headline. + * both the mini cards and the headline. Activity is gated the same way on + * `audit:read`. */ import { useMemo } from "react"; @@ -75,7 +76,11 @@ export function useMiniCardStatuses(): HomeStatus { const { data: health, error: healthError } = systemHealth; const { data: mcpServers, error: mcpServersError } = useQuery(MCP_REACH_PATH); const { data: a2aAgents, error: a2aError } = useQuery(A2A_REACH_PATH); - const { items } = useRecentActivity({ pollIntervalMs: 0 }); + // /api/logs/activity requires audit:read, which no default non-admin role + // holds. security:read is not checked: that half of the feed is additive + // server-side, so an audit:read-only caller gets a narrower feed, not an error. + const canViewActivity = hasPermission("audit:read"); + const { items } = useRecentActivity({ pollIntervalMs: 0, enabled: canViewActivity }); const derived = useMemo(() => { const healthy = safeHealthy(health); diff --git a/src/hooks/useRecentActivity.test.tsx b/src/hooks/useRecentActivity.test.tsx index b105159..1d05ee1 100644 --- a/src/hooks/useRecentActivity.test.tsx +++ b/src/hooks/useRecentActivity.test.tsx @@ -41,6 +41,30 @@ describe("useRecentActivity", () => { expect(result.current.items).toEqual([]); }); + it("makes no request while disabled and fetches once enabled", async () => { + let callCount = 0; + server.use( + http.get("*/api/logs/activity", () => { + callCount += 1; + return HttpResponse.json({ items: RECENT_ACTIVITY_FIXTURE.slice(0, 2) }); + }), + ); + + const { result, rerender } = renderHook( + ({ enabled }) => useRecentActivity({ pollIntervalMs: 0, enabled }), + { initialProps: { enabled: false } }, + ); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(callCount).toBe(0); + expect(result.current.items).toEqual([]); + + rerender({ enabled: true }); + + await waitFor(() => expect(result.current.items).toHaveLength(2)); + expect(callCount).toBe(1); + }); + it("refetch re-hits the endpoint and clears the error", async () => { let callCount = 0; server.use( diff --git a/src/hooks/useRecentActivity.ts b/src/hooks/useRecentActivity.ts index 48d3836..d6fb584 100644 --- a/src/hooks/useRecentActivity.ts +++ b/src/hooks/useRecentActivity.ts @@ -26,6 +26,8 @@ interface UseRecentActivityOptions { limit?: number; /** Polling cadence override. Pass 0 to disable. */ pollIntervalMs?: number; + /** When false, no request is made and the feed stays empty. */ + enabled?: boolean; } function isMockEnabled(): boolean { @@ -33,7 +35,7 @@ function isMockEnabled(): boolean { } export function useRecentActivity(options: UseRecentActivityOptions = {}): UseRecentActivityResult { - const { limit = 10, pollIntervalMs = RECENT_ACTIVITY_POLL_INTERVAL_MS } = options; + const { limit = 10, pollIntervalMs = RECENT_ACTIVITY_POLL_INTERVAL_MS, enabled = true } = options; const mock = isMockEnabled(); const [items, setItems] = useState([]); @@ -68,6 +70,13 @@ export function useRecentActivity(options: UseRecentActivityOptions = {}): UseRe ); useEffect(() => { + if (!enabled) { + setItems([]); + setError(null); + setIsLoading(false); + return; + } + const controller = new AbortController(); void fetchOnce(controller.signal); @@ -83,12 +92,13 @@ export function useRecentActivity(options: UseRecentActivityOptions = {}): UseRe controller.abort(); window.clearInterval(intervalId); }; - }, [fetchOnce, mock, pollIntervalMs]); + }, [fetchOnce, mock, pollIntervalMs, enabled]); const refetch = useCallback(async (): Promise => { + if (!enabled) return; setIsLoading(true); await fetchOnce(); - }, [fetchOnce]); + }, [fetchOnce, enabled]); return { items, isLoading, error, refetch }; }