diff --git a/backend/src/routes/systemMaintenance.ts b/backend/src/routes/systemMaintenance.ts index e12c7cbe..180fcbb3 100644 --- a/backend/src/routes/systemMaintenance.ts +++ b/backend/src/routes/systemMaintenance.ts @@ -120,11 +120,17 @@ systemMaintenanceRouter.post('/prune/system', async (req: Request, res: Response 'docker disk usage', ); } + if (isDebugEnabled()) { + console.debug('[Resources:debug] Prune dry-run', { + target, scope: pruneScope, reclaimableBytes: estimate.reclaimableBytes, + }); + } res.json({ message: 'Dry run', success: true, dryRun: true, reclaimedBytes: estimate.reclaimableBytes }); return; } console.log(`[Resources] System prune: ${target} (scope: ${pruneScope})`); + const pruneStartedAt = Date.now(); let result: { success: boolean; reclaimedBytes: number }; if (pruneScope === 'managed' && target !== 'containers') { const knownStacks = await FileSystemService.getInstance(req.nodeId).getStacks(); @@ -137,6 +143,11 @@ systemMaintenanceRouter.post('/prune/system', async (req: Request, res: Response } console.log(`[Resources] System prune completed: ${target}, reclaimed ${result.reclaimedBytes} bytes`); + if (isDebugEnabled()) { + console.debug('[Resources:debug] System prune', { + target, scope: pruneScope, ms: Date.now() - pruneStartedAt, reclaimedBytes: result.reclaimedBytes, + }); + } if (target === 'containers') { invalidateNodeCaches(req.nodeId); } @@ -390,6 +401,15 @@ systemMaintenanceRouter.post('/networks', async (req: Request, res: Response) => if (attachable) options.Attachable = true; const dockerController = DockerController.getInstance(req.nodeId); + if (isDebugEnabled()) { + console.debug('[Resources:debug] Network create', { + driver: options.Driver ?? 'bridge', + internal: !!options.Internal, + attachable: !!options.Attachable, + hasSubnet: !!subnet, + hasGateway: !!gateway, + }); + } const network = await dockerController.createNetwork(options); console.log(`[Resources] Network created: ${sanitizeForLog(name)}`); invalidateNodeCaches(req.nodeId); diff --git a/backend/src/routes/volumes.ts b/backend/src/routes/volumes.ts index 91000188..1392fdd0 100644 --- a/backend/src/routes/volumes.ts +++ b/backend/src/routes/volumes.ts @@ -3,6 +3,7 @@ import { VolumeBrowserService, isValidVolumeName, PathTraversalError, VolumeNotF import { DatabaseService } from '../services/DatabaseService'; import { requireAdmin } from '../middleware/tierGates'; import { sanitizeForLog } from '../utils/safeLog'; +import { isDebugEnabled } from '../utils/debug'; export const volumesRouter = Router(); @@ -27,7 +28,13 @@ volumesRouter.get('/:name/list', async (req: Request, res: Response) => { const name = req.params.name as string; if (!isValidVolumeName(name)) return res.status(400).json({ error: 'Invalid volume name' }); const path = readPathParam(req); + const startedAt = Date.now(); const entries = await VolumeBrowserService.getInstance(req.nodeId).listDir(name, path); + if (isDebugEnabled()) { + console.debug('[Volumes:debug] list', { + volume: sanitizeForLog(name), path: sanitizeForLog(path || '/'), entries: entries.length, ms: Date.now() - startedAt, + }); + } res.json(entries); } catch (error: unknown) { mapServiceError(error, res, 'Failed to list volume directory'); @@ -54,8 +61,16 @@ volumesRouter.get('/:name/read', async (req: Request, res: Response) => { let outcome: 'success' | 'error' = 'error'; try { if (!isValidVolumeName(name)) return res.status(400).json({ error: 'Invalid volume name' }); + const startedAt = Date.now(); const result = await VolumeBrowserService.getInstance(req.nodeId).readFile(name, requestPath); outcome = 'success'; + if (isDebugEnabled()) { + // Never log the file body; only its shape. + console.debug('[Volumes:debug] read', { + volume: sanitizeForLog(name), path: sanitizeForLog(requestPath || '/'), + size: result.size, binary: result.binary, truncated: result.truncated, ms: Date.now() - startedAt, + }); + } res.json(result); } catch (error: unknown) { mapServiceError(error, res, 'Failed to read volume file'); diff --git a/frontend/src/components/ResourcesView.tsx b/frontend/src/components/ResourcesView.tsx index da2b11ee..7a9a318f 100644 --- a/frontend/src/components/ResourcesView.tsx +++ b/frontend/src/components/ResourcesView.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react'; +import { useState, useEffect, useRef } from 'react'; import { Card, CardContent, CardHeader, CardTitle, CardDescription } from "@/components/ui/card"; import { Button } from "@/components/ui/button"; import { Tabs, TabsContent, TabsList, TabsTrigger, TabsHighlight, TabsHighlightItem } from "@/components/ui/tabs"; @@ -386,8 +386,17 @@ export default function ResourcesView() { const [scanSummaries, setScanSummaries] = useState>({}); const [scanningImageRef, setScanningImageRef] = useState(null); const [inspectScanId, setInspectScanId] = useState(null); + // Holds the AbortController for the in-flight scan poll so it can be + // cancelled; the scan keeps running server-side, only the client poll stops. + const scanAbortRef = useRef(null); + + // Generation counter so a slow fetch for a previously-active node cannot + // stomp the visible resources after the user switches nodes. Each call + // claims a generation; only the latest call is allowed to write state. + const fetchGenerationRef = useRef(0); const fetchAllData = async () => { + const generation = ++fetchGenerationRef.current; setIsLoading(true); try { const [usageRes, resourcesRes, orphansRes, summariesRes] = await Promise.all([ @@ -397,31 +406,47 @@ export default function ResourcesView() { apiFetch('/security/image-summaries').catch(() => null), ]); - if (usageRes.ok) setUsage(await usageRes.json()); - if (resourcesRes.ok) { - const resources = await resourcesRes.json(); - setImages(resources.images ?? []); - setVolumes(resources.volumes ?? []); - setNetworks(resources.networks ?? []); + // Resolve every body before the staleness check so a stale + // generation cannot write any subset of the resource slices. + const usageData = usageRes.ok ? await usageRes.json() : null; + const resourcesData = resourcesRes.ok ? await resourcesRes.json() : null; + const orphansData = orphansRes.ok ? await orphansRes.json() : null; + const summariesData = summariesRes && summariesRes.ok ? await summariesRes.json() : null; + + if (fetchGenerationRef.current !== generation) return; + + if (usageData) setUsage(usageData); + if (resourcesData) { + setImages(resourcesData.images ?? []); + setVolumes(resourcesData.volumes ?? []); + setNetworks(resourcesData.networks ?? []); } - if (orphansRes.ok) { - setOrphans(await orphansRes.json()); + if (orphansData) { + setOrphans(orphansData); setSelectedOrphans([]); } - if (summariesRes && summariesRes.ok) { - const data = await summariesRes.json(); - setScanSummaries(data ?? {}); - } + if (summariesData) setScanSummaries(summariesData); } catch (err) { + if (fetchGenerationRef.current !== generation) return; console.error('Failed to fetch data', err); toast.error('Failed to load resources data'); } finally { - setIsLoading(false); + if (fetchGenerationRef.current === generation) setIsLoading(false); } }; useEffect(() => { fetchAllData(); }, [activeNode]); + // Cancel an in-flight scan poll on unmount or node switch; its result + // belongs to the node it started on. + useEffect(() => { + return () => scanAbortRef.current?.abort(); + }, [activeNode]); + + // Bump the fetch generation on unmount so a fetch that resolves after the + // view is gone cannot run state setters or surface a load-error toast. + useEffect(() => () => { fetchGenerationRef.current += 1; }, []); + const handlePrune = async () => { if (!confirmPrune) return; setIsActioning(true); @@ -431,16 +456,21 @@ export default function ResourcesView() { method: 'POST', body: JSON.stringify({ target: confirmPrune.target, scope: confirmPrune.scope }) }); - const data = await res.json(); + const data = await res.json().catch(() => null); + if (!res.ok) { + throw new Error(data?.error || `Failed to prune ${confirmPrune.target}`); + } const scopeLabel = confirmPrune.scope === 'managed' ? 'Sencho-managed' : 'all'; toast.success( - data.reclaimedBytes !== undefined + data?.reclaimedBytes !== undefined ? `Pruned ${scopeLabel} ${confirmPrune.target}. Reclaimed ${formatBytes(data.reclaimedBytes)}.` : `Pruned ${scopeLabel} ${confirmPrune.target}.` ); await fetchAllData(); - } catch { - toast.error(confirmPrune ? `Failed to prune ${confirmPrune.target}` : 'Prune failed'); + } catch (error) { + console.error('Failed to prune', error); + const err = error as { message?: string }; + toast.error(err?.message || `Failed to prune ${confirmPrune.target}`); } finally { toast.dismiss(loadingId); setIsActioning(false); @@ -511,12 +541,18 @@ export default function ResourcesView() { options: { force?: boolean; scanners?: ('vuln' | 'secret')[] } = {}, ) => { const { force = false, scanners } = options; + // Supersede any prior in-flight scan poll before claiming this one. + scanAbortRef.current?.abort(); + const controller = new AbortController(); + scanAbortRef.current = controller; + const { signal } = controller; setScanningImageRef(imageRef); const loadingId = toast.loading(`Scanning ${imageRef}...`); try { const res = await apiFetch('/security/scan', { method: 'POST', body: JSON.stringify({ imageRef, force, scanners }), + signal, }); const data = await res.json(); if (!res.ok) throw new Error(data?.error || 'Failed to start scan'); @@ -524,19 +560,28 @@ export default function ResourcesView() { const deadline = Date.now() + 5 * 60 * 1000; while (Date.now() < deadline) { - await new Promise(r => setTimeout(r, 3000)); - const poll = await apiFetch(`/security/scans/${scanId}`); + await new Promise((resolve) => { + if (signal.aborted) { resolve(); return; } + const timer = setTimeout(resolve, 3000); + signal.addEventListener('abort', () => { clearTimeout(timer); resolve(); }, { once: true }); + }); + if (signal.aborted) return; + const poll = await apiFetch(`/security/scans/${scanId}`, { signal }); + if (signal.aborted) return; if (!poll.ok) continue; const poll_data = await poll.json(); + if (signal.aborted) return; if (poll_data.status !== 'in_progress') { if (poll_data.status === 'failed') { throw new Error(poll_data.error || 'Scan failed'); } toast.success(`Scan complete: ${poll_data.total_vulnerabilities} vulnerabilities found`); setInspectScanId(scanId); - const summariesRes = await apiFetch('/security/image-summaries'); + const summariesRes = await apiFetch('/security/image-summaries', { signal }); + if (signal.aborted) return; if (summariesRes.ok) { const summaries = await summariesRes.json(); + if (signal.aborted) return; setScanSummaries(summaries ?? {}); } return; @@ -544,11 +589,22 @@ export default function ResourcesView() { } throw new Error('Scan timed out'); } catch (error) { + if (signal.aborted) { + // Suppress the toast for a deliberately cancelled poll, but keep + // a breadcrumb so a real error racing the abort is not lost. + console.debug('Scan poll aborted', error); + return; + } const err = error as { message?: string; error?: string; data?: { error?: string } }; toast.error(err?.message || err?.error || err?.data?.error || 'Scan failed'); } finally { toast.dismiss(loadingId); - setScanningImageRef(null); + // Only the owning poll resets shared state; a superseded poll leaves + // it to the scan that replaced it. + if (scanAbortRef.current === controller) { + scanAbortRef.current = null; + setScanningImageRef(null); + } } }; diff --git a/frontend/src/components/__tests__/ResourcesView.test.tsx b/frontend/src/components/__tests__/ResourcesView.test.tsx new file mode 100644 index 00000000..c600a5aa --- /dev/null +++ b/frontend/src/components/__tests__/ResourcesView.test.tsx @@ -0,0 +1,163 @@ +/** + * Coverage for ResourcesView hardening. + * + * Locks two correctness fixes that manual smoke testing cannot reliably catch: + * - M-1: a slow resource fetch for a previously-active node must not overwrite + * the newly-selected node's data (node-switch generation guard). + * - M-2: a failed prune must surface the server error, never a false success. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() })); + +vi.mock('@/components/ui/toast-store', () => ({ + toast: { + error: vi.fn(), + success: vi.fn(), + warning: vi.fn(), + info: vi.fn(), + loading: vi.fn(() => 'toast-id'), + dismiss: vi.fn(), + }, +})); + +const licenseState = { isPaid: true }; +vi.mock('@/context/LicenseContext', () => ({ useLicense: () => licenseState })); +vi.mock('@/context/AuthContext', () => ({ useAuth: () => ({ isAdmin: true }) })); + +const nodesState: { activeNode: { id: number } | null } = { activeNode: { id: 1 } }; +vi.mock('@/context/NodeContext', () => ({ useNodes: () => nodesState })); + +vi.mock('@/hooks/useTrivyStatus', () => ({ + useTrivyStatus: () => ({ + status: { available: false, version: null, source: 'none', autoUpdate: false, busy: false }, + updateCheck: null, + refresh: vi.fn(), + refreshUpdateCheck: vi.fn(), + }), +})); + +// Heavy or portal-bound children are not under test; stub them to keep the +// render tree light and deterministic. +vi.mock('../VulnerabilityScanSheet', () => ({ VulnerabilityScanSheet: () => null })); +vi.mock('../resources/ReclaimHero', () => ({ ReclaimHero: () => null })); +vi.mock('../resources/FootprintTreemap', () => ({ FootprintTreemap: () => null })); +vi.mock('../resources/ImageDetailsSheet', () => ({ ImageDetailsSheet: () => null })); +vi.mock('../resources/VolumeBrowserSheet', () => ({ VolumeBrowserSheet: () => null })); +vi.mock('../resources/NetworkDetailSheet', () => ({ NetworkDetailSheet: () => null })); +vi.mock('../NetworkTopologyView', () => ({ default: () => null })); +vi.mock('../CapabilityGate', () => ({ CapabilityGate: ({ children }: { children: React.ReactNode }) => <>{children} })); +vi.mock('../LazyBoundary', () => ({ default: ({ children }: { children: React.ReactNode }) => <>{children} })); +vi.mock('../NodeManager', () => ({ SENCHO_NAVIGATE_EVENT: 'sencho-navigate' })); + +import { apiFetch } from '@/lib/api'; +import { toast } from '@/components/ui/toast-store'; +import ResourcesView from '../ResourcesView'; + +const mockedFetch = apiFetch as unknown as ReturnType; + +function jsonResponse(body: unknown, init: { ok?: boolean; status?: number } = {}): Response { + return { + ok: init.ok ?? true, + status: init.status ?? 200, + json: async () => body, + } as unknown as Response; +} + +function image(repoTag: string) { + return { + Id: `sha256:${repoTag}`, + RepoTags: [repoTag], + Size: 1000, + Containers: 0, + managedBy: null, + managedStatus: 'unmanaged' as const, + isSencho: false, + }; +} + +beforeEach(() => { + mockedFetch.mockReset(); + licenseState.isPaid = true; + nodesState.activeNode = { id: 1 }; +}); + +afterEach(() => vi.clearAllMocks()); + +describe('ResourcesView', () => { + it('drops a stale node fetch so it cannot overwrite the newly selected node (M-1)', async () => { + // Hold every /system/resources response open so we control resolution order. + const resourcesResolvers: Array<(r: Response) => void> = []; + mockedFetch.mockImplementation((url: string) => { + if (url === '/system/resources') { + return new Promise((resolve) => resourcesResolvers.push(resolve)); + } + return Promise.resolve(jsonResponse({})); + }); + + const { rerender } = render(); + await waitFor(() => expect(resourcesResolvers).toHaveLength(1)); + + // Switch nodes before the first fetch resolves; the effect re-runs and + // claims a newer generation. + nodesState.activeNode = { id: 2 }; + rerender(); + await waitFor(() => expect(resourcesResolvers).toHaveLength(2)); + + // Resolve the newer (node 2) fetch first; its data should render. + resourcesResolvers[1](jsonResponse({ images: [image('node2-img:latest')], volumes: [], networks: [] })); + expect(await screen.findByText('node2-img:latest')).toBeInTheDocument(); + + // Now resolve the stale (node 1) fetch. Its generation is old, so the guard + // must drop it rather than stomp node 2's resources. + resourcesResolvers[0](jsonResponse({ images: [image('node1-img:latest')], volumes: [], networks: [] })); + await waitFor(() => { + expect(screen.getByText('node2-img:latest')).toBeInTheDocument(); + }); + expect(screen.queryByText('node1-img:latest')).not.toBeInTheDocument(); + }); + + it('does not surface a load failure that resolves after the view unmounts', async () => { + let rejectResources: ((e: unknown) => void) | undefined; + mockedFetch.mockImplementation((url: string) => { + if (url === '/system/resources') { + return new Promise((_resolve, reject) => { rejectResources = reject; }); + } + return Promise.resolve(jsonResponse({})); + }); + + const { unmount } = render(); + await waitFor(() => expect(rejectResources).toBeDefined()); + + unmount(); + rejectResources!(new Error('network down')); + await Promise.resolve(); + await Promise.resolve(); + + expect(toast.error).not.toHaveBeenCalled(); + }); + + it('surfaces the server error on a failed prune instead of a false success (M-2)', async () => { + mockedFetch.mockImplementation((url: string, opts?: RequestInit) => { + if (url === '/system/prune/system' && opts?.method === 'POST') { + return Promise.resolve(jsonResponse({ error: 'Prune blew up' }, { ok: false, status: 500 })); + } + if (url === '/system/resources') { + return Promise.resolve(jsonResponse({ images: [], volumes: [], networks: [] })); + } + return Promise.resolve(jsonResponse({})); + }); + + const user = userEvent.setup(); + render(); + await waitFor(() => expect(mockedFetch).toHaveBeenCalledWith('/system/resources')); + + await user.click(screen.getByRole('button', { name: /Prune Unused Images/ })); + await user.click(await screen.findByRole('button', { name: /^Prune$/ })); + + await waitFor(() => expect(toast.error).toHaveBeenCalledWith('Prune blew up')); + expect(toast.success).not.toHaveBeenCalled(); + }); +});