mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-29 11:47:01 +00:00
fix(networking): hold unsafe network deletions and stabilize aggregate reads (#1850)
* fix(networking): hold unsafe network deletions and stabilize aggregate reads Networking audit hardening: - Fail closed when deleting unlabeled external networks while any stack fails to render or the Docker runtime is unreachable: declarations cannot be verified, so the backend 409s with a typed code and the inventory table holds the delete affordance instead of letting the confirm dialog surprise the operator. - Bound concurrent compose renders during network delete verification with the shared render semaphore and mapWithConcurrency. - Add a short-TTL memo for the node networking aggregate keyed by node and request variant, invalidated eagerly on stack, exposure-intent, dossier, and network mutations, with stale-on-error serves flagged as degradedCache and surfaced in the overview. - Resolve node-scoped stack edit permissions against the active node in the findings action list. - Developer-mode debug logs for aggregate serving and delete-guard outcomes. Tests: cache unit suite, hardening integration suite, and component coverage for the permission threading and delete-affordance holds. * chore(networking): add missing EOF newline in aggregate cache module
This commit is contained in:
@@ -57,14 +57,20 @@ function NetworkTableSkeleton({ rows = 5 }: { rows?: number }) {
|
||||
/** A network is unsafe to delete without a pre-confirm explanation when it has
|
||||
* connected containers or is declared by a stack's Compose file; the backend
|
||||
* 409-guards these, but the UI should explain BEFORE the confirm dialog rather
|
||||
* than let a generic confirmation surprise the user with a rejection. */
|
||||
function deleteBlockReason(row: NetworkingNetworkRow): string | null {
|
||||
* than let a generic confirmation surprise the user with a rejection. When render
|
||||
* verification is unavailable (a stack failed to render, or the Docker runtime is
|
||||
* unreachable), declarations cannot be verified at all, so every non-system,
|
||||
* non-Sencho network is held back the same way the backend does. */
|
||||
function deleteBlockReason(row: NetworkingNetworkRow, renderVerificationUnavailable: boolean): string | null {
|
||||
if (row.connectedCount > 0) {
|
||||
return `Connected to ${row.connectedCount} container${row.connectedCount === 1 ? '' : 's'}; disconnect them first.`;
|
||||
}
|
||||
if (row.declaredByStacks.length > 0) {
|
||||
return `Declared by ${row.declaredByStacks.join(', ')}; remove the declaration first.`;
|
||||
}
|
||||
if (renderVerificationUnavailable) {
|
||||
return 'One or more stacks failed to render; stack declarations cannot be verified right now.';
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -150,6 +156,7 @@ export function NetworkInventoryTable({
|
||||
onDelete,
|
||||
onOpenStack,
|
||||
onFilterTopology,
|
||||
renderVerificationUnavailable,
|
||||
}: {
|
||||
rows: NetworkingNetworkRow[];
|
||||
findings: NetworkingFinding[];
|
||||
@@ -159,6 +166,7 @@ export function NetworkInventoryTable({
|
||||
onDelete: (id: string, name: string) => void;
|
||||
onOpenStack: (stack: string) => void;
|
||||
onFilterTopology: (name: string) => void;
|
||||
renderVerificationUnavailable: boolean;
|
||||
}) {
|
||||
const [filter, setFilter] = useState<NetworkFilter>('all');
|
||||
const [search, setSearch] = useState('');
|
||||
@@ -295,7 +303,7 @@ export function NetworkInventoryTable({
|
||||
<TooltipContent>Show in topology</TooltipContent>
|
||||
</Tooltip>
|
||||
{isAdmin && (() => {
|
||||
const blockReason = deleteBlockReason(row);
|
||||
const blockReason = deleteBlockReason(row, renderVerificationUnavailable);
|
||||
const protectedReason = row.isSencho
|
||||
? 'Protected · running Sencho instance'
|
||||
: row.isSystem
|
||||
|
||||
@@ -23,6 +23,7 @@ export function NetworkingFindingsList({
|
||||
isAdmin,
|
||||
onAction,
|
||||
disabled = false,
|
||||
nodeId,
|
||||
}: {
|
||||
findings: NetworkingFinding[];
|
||||
loading: boolean;
|
||||
@@ -30,6 +31,7 @@ export function NetworkingFindingsList({
|
||||
isAdmin: boolean;
|
||||
onAction: (action: NetworkingRecommendedAction) => void | Promise<void>;
|
||||
disabled?: boolean;
|
||||
nodeId: number | null | undefined;
|
||||
}) {
|
||||
if (loading) return <p className="text-sm text-muted-foreground">Loading findings…</p>;
|
||||
if (findings.length === 0) {
|
||||
@@ -66,7 +68,7 @@ export function NetworkingFindingsList({
|
||||
{items.map((finding, i) => {
|
||||
const sourceLabel = findingSourceLabel(finding);
|
||||
const primary = finding.recommendedActions.find((action) =>
|
||||
isNetworkingActionVisible(action, isAdmin, (stack) => canEdit('stack:edit', 'stack', stack)),
|
||||
isNetworkingActionVisible(action, isAdmin, (stack) => canEdit('stack:edit', 'stack', stack, nodeId)),
|
||||
);
|
||||
return (
|
||||
<TableRow
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useCallback, useEffect, useState, type ReactNode } from 'react';
|
||||
import { useCallback, useEffect, useRef, useState, type ReactNode } from 'react';
|
||||
import {
|
||||
LayoutDashboard, Network, GitBranch, AlertTriangle, RefreshCw, Plus, Unplug,
|
||||
} from 'lucide-react';
|
||||
@@ -14,6 +14,7 @@ import { toast } from '@/components/ui/toast-store';
|
||||
import { useAuth } from '@/context/AuthContext';
|
||||
import { useNodes } from '@/context/NodeContext';
|
||||
import { useIsMobile } from '@/hooks/use-is-mobile';
|
||||
import { useDeveloperMode } from '@/hooks/useDeveloperMode';
|
||||
import { Masthead, MobileSubTabs, type Tone } from '@/components/mobile/mobile-ui';
|
||||
import { springs } from '@/lib/motion';
|
||||
import { CreateNetworkDialog } from '@/components/resources/CreateNetworkDialog';
|
||||
@@ -86,6 +87,11 @@ export function NetworkingView({ headerActions }: NetworkingViewProps) {
|
||||
const { isAdmin, can } = useAuth();
|
||||
const { activeNode } = useNodes();
|
||||
const isMobile = useIsMobile();
|
||||
const developerMode = useDeveloperMode(activeNode?.id);
|
||||
// Read the flag through a ref inside the load effect so flipping it does not
|
||||
// retrigger the overview fetch (the flag only gates debug logging).
|
||||
const developerModeRef = useRef(developerMode);
|
||||
developerModeRef.current = developerMode;
|
||||
const nodeId = activeNode?.id;
|
||||
|
||||
const [tab, setTab] = useState<NetworkingTab>('overview');
|
||||
@@ -140,6 +146,7 @@ export function NetworkingView({ headerActions }: NetworkingViewProps) {
|
||||
setFindings([]);
|
||||
setRecentActivity([]);
|
||||
setIsLegacy(false);
|
||||
const startedAt = Date.now();
|
||||
const load = async () => {
|
||||
try {
|
||||
const response = await apiFetch('/networking/overview', { nodeId, signal: controller.signal });
|
||||
@@ -150,6 +157,13 @@ export function NetworkingView({ headerActions }: NetworkingViewProps) {
|
||||
if (!response.ok) throw new Error('Failed to load networking data.');
|
||||
const body = await response.json() as Partial<NetworkingOverviewEnvelope>;
|
||||
if (stale) return;
|
||||
if (developerModeRef.current) {
|
||||
console.debug('[Networking:debug] overview loaded', {
|
||||
nodeId,
|
||||
ms: Date.now() - startedAt,
|
||||
schemaVersion: body.schemaVersion ?? null,
|
||||
});
|
||||
}
|
||||
const adapted = adaptNetworkingOverview(body);
|
||||
setIsLegacy(adapted.isLegacy);
|
||||
setRuntimeAvailable(adapted.runtimeAvailable);
|
||||
@@ -292,6 +306,11 @@ export function NetworkingView({ headerActions }: NetworkingViewProps) {
|
||||
<CardContent className="p-3 text-sm text-warning">Docker runtime is unavailable. Compose-model signals remain available.</CardContent>
|
||||
</Card>
|
||||
)}
|
||||
{overview.degradedCache && (
|
||||
<Card className="border-warning/40 bg-warning/5">
|
||||
<CardContent className="p-3 text-sm text-warning">Showing cached results from the last successful refresh; a live refresh failed.</CardContent>
|
||||
</Card>
|
||||
)}
|
||||
<div className="grid overflow-hidden rounded-lg border border-card-border bg-card shadow-card-bevel sm:grid-cols-2 xl:grid-cols-4">
|
||||
{[
|
||||
{ label: 'Networks', value: overview.networkCount ?? '—', tab: 'networks' as const },
|
||||
@@ -526,6 +545,7 @@ export function NetworkingView({ headerActions }: NetworkingViewProps) {
|
||||
setPendingTopologyFilter(networkName);
|
||||
setTab('topology');
|
||||
}}
|
||||
renderVerificationUnavailable={(overview?.renderFailedStacks.length ?? 0) > 0 || !runtimeAvailable}
|
||||
/>
|
||||
</TabsContent>
|
||||
|
||||
@@ -547,7 +567,7 @@ export function NetworkingView({ headerActions }: NetworkingViewProps) {
|
||||
</TabsContent>
|
||||
|
||||
<TabsContent value="findings" className="mt-4">
|
||||
<NetworkingFindingsList findings={findings} loading={loading} canEdit={can} isAdmin={isAdmin} onAction={dispatchAction} disabled={isLegacy} />
|
||||
<NetworkingFindingsList findings={findings} loading={loading} canEdit={can} isAdmin={isAdmin} onAction={dispatchAction} disabled={isLegacy} nodeId={nodeId} />
|
||||
</TabsContent>
|
||||
</Tabs>
|
||||
|
||||
|
||||
@@ -29,6 +29,7 @@ describe('NetworkInventoryTable', () => {
|
||||
onDelete={vi.fn()}
|
||||
onOpenStack={vi.fn()}
|
||||
onFilterTopology={vi.fn()}
|
||||
renderVerificationUnavailable={false}
|
||||
/>,
|
||||
);
|
||||
// Default sort is name ascending, independent of input row order.
|
||||
@@ -53,6 +54,7 @@ describe('NetworkInventoryTable', () => {
|
||||
onDelete={vi.fn()}
|
||||
onOpenStack={vi.fn()}
|
||||
onFilterTopology={vi.fn()}
|
||||
renderVerificationUnavailable={false}
|
||||
/>,
|
||||
);
|
||||
const actionButtons = screen.getAllByRole('button').filter((b) =>
|
||||
@@ -62,4 +64,40 @@ describe('NetworkInventoryTable', () => {
|
||||
'Open app', 'Inspect a-net', 'Show a-net in topology', 'Delete a-net',
|
||||
]);
|
||||
});
|
||||
|
||||
it('holds the delete affordance while render verification is unavailable', () => {
|
||||
const unlabeledExternal = row({ id: '1', name: 'shared_ext', composeProject: null, stack: null });
|
||||
render(
|
||||
<NetworkInventoryTable
|
||||
rows={[unlabeledExternal]}
|
||||
findings={[]}
|
||||
loading={false}
|
||||
isAdmin
|
||||
onInspect={vi.fn()}
|
||||
onDelete={vi.fn()}
|
||||
onOpenStack={vi.fn()}
|
||||
onFilterTopology={vi.fn()}
|
||||
renderVerificationUnavailable
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByRole('button', { name: 'Delete shared_ext' })).toBeDisabled();
|
||||
});
|
||||
|
||||
it('keeps delete enabled for the same network when renders succeed', () => {
|
||||
const unlabeledExternal = row({ id: '1', name: 'shared_ext', composeProject: null, stack: null });
|
||||
render(
|
||||
<NetworkInventoryTable
|
||||
rows={[unlabeledExternal]}
|
||||
findings={[]}
|
||||
loading={false}
|
||||
isAdmin
|
||||
onInspect={vi.fn()}
|
||||
onDelete={vi.fn()}
|
||||
onOpenStack={vi.fn()}
|
||||
onFilterTopology={vi.fn()}
|
||||
renderVerificationUnavailable={false}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByRole('button', { name: 'Delete shared_ext' })).toBeEnabled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,7 +2,6 @@ import { describe, it, expect, vi } from 'vitest';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { NetworkingFindingsList } from '../NetworkingFindingsList';
|
||||
import type { NetworkingFinding } from '@/types/networking';
|
||||
|
||||
function finding(overrides: Partial<NetworkingFinding> = {}): NetworkingFinding {
|
||||
return {
|
||||
id: overrides.id ?? Math.random().toString(36),
|
||||
@@ -20,9 +19,18 @@ function finding(overrides: Partial<NetworkingFinding> = {}): NetworkingFinding
|
||||
|
||||
const canEdit = () => true;
|
||||
|
||||
function exposureFinding(): NetworkingFinding {
|
||||
return finding({
|
||||
id: 'exposure',
|
||||
kind: 'exposure-unclassified',
|
||||
title: 'Unclassified exposure',
|
||||
recommendedActions: [{ kind: 'set-exposure-intent', label: 'Set exposure intent', stack: 'proxy' }],
|
||||
});
|
||||
}
|
||||
|
||||
describe('NetworkingFindingsList', () => {
|
||||
it('shows a calm empty state when there are no findings', () => {
|
||||
render(<NetworkingFindingsList findings={[]} loading={false} canEdit={canEdit} isAdmin onAction={vi.fn()} />);
|
||||
render(<NetworkingFindingsList findings={[]} loading={false} canEdit={canEdit} isAdmin onAction={vi.fn()} nodeId={1} />);
|
||||
expect(screen.getByText('No networking issues detected.')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
@@ -38,6 +46,7 @@ describe('NetworkingFindingsList', () => {
|
||||
canEdit={canEdit}
|
||||
isAdmin
|
||||
onAction={vi.fn()}
|
||||
nodeId={1}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText(/Needs action/)).toBeInTheDocument();
|
||||
@@ -45,6 +54,18 @@ describe('NetworkingFindingsList', () => {
|
||||
expect(screen.getByText(/Informational/)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('respects node-scoped stack:edit for the primary action', () => {
|
||||
const scopedCanEdit = vi.fn((_action: string, _type?: string, _id?: string, nodeId?: number | null) => nodeId === 7);
|
||||
render(<NetworkingFindingsList findings={[exposureFinding()]} loading={false} canEdit={scopedCanEdit} isAdmin={false} onAction={vi.fn()} nodeId={7} />);
|
||||
expect(screen.getByRole('button', { name: 'Set exposure intent' })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('hides the primary action when the node scope does not match', () => {
|
||||
const deniedCanEdit = vi.fn((_action: string, _type?: string, _id?: string, nodeId?: number | null) => nodeId === 8);
|
||||
render(<NetworkingFindingsList findings={[exposureFinding()]} loading={false} canEdit={deniedCanEdit} isAdmin={false} onAction={vi.fn()} nodeId={7} />);
|
||||
expect(screen.queryByRole('button', { name: 'Set exposure intent' })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows the merged source label for a card found by both engines', () => {
|
||||
render(
|
||||
<NetworkingFindingsList
|
||||
@@ -56,6 +77,7 @@ describe('NetworkingFindingsList', () => {
|
||||
canEdit={canEdit}
|
||||
isAdmin
|
||||
onAction={vi.fn()}
|
||||
nodeId={1}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText('Live · also found by Doctor')).toBeInTheDocument();
|
||||
|
||||
@@ -128,6 +128,8 @@ export interface NodeNetworkingOverview {
|
||||
missingExternalCount: number;
|
||||
networkCollisionCount: number;
|
||||
findingCount: number;
|
||||
/** Served from the backend memo after a recompute failure; data may lag recent changes. */
|
||||
degradedCache?: boolean;
|
||||
renderFailedStacks: string[];
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user