mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-21 15:46:43 +00:00
fix: harden cross-node fleet label actions and guard container reads (#1503)
* fix: harden cross-node fleet label actions and guard container reads
Release-stabilization fixes for the Fleet Actions surface:
- Stop-by-label binds execution to the nodes shown in the confirmed
preview. The real stop sends the confirmed node ids and the backend
restricts the fan-out to them, so a node that was unreachable during
preview and reconnects before the stop can no longer enter execution
and have unlisted stacks stopped.
- Bulk label assign validates each remote node's result against the
stacks it was asked to label: a body whose results are empty, partial,
duplicated, or shaped wrong is a per-node failure instead of reading as
a successful zero-stack assign. The card mirrors this, rejecting a
missing or non-array results body and only reporting success when at
least one stack was assigned.
- Bulk label assign re-reads authoritative per-node stacks and labels on
demand via a Refresh control, and the confirmation lists the affected
node and stack names rather than bare counts.
- The stack-specific and fleet container/stack read routes require the
stack:read permission, matching the generic container and stack routes.
Every shipped role already carries stack:read, so reachability is
unchanged; the guard closes the routes that were auth-only.
Adds unit coverage for the assign-result validator, route coverage for
the stop allowlist and assign membership checks, and authorization
coverage for the newly guarded reads.
* test: assert the confirmed node allowlist in the fleet stop-card test
The stop-card component test pinned the real-stop request body to
{ labelName, dryRun } and broke once the stop began carrying the
confirmed-preview node ids. Update it to expect the nodeIds allowlist
derived from the resolved preview, so the test asserts the binding
rather than the pre-fix shape.
This commit is contained in:
@@ -42,6 +42,11 @@ interface Props {
|
||||
nodes: FleetNode[];
|
||||
}
|
||||
|
||||
// One node's slice of the pending assignment: the stacks selected on it and
|
||||
// whether the label will be created there. Shared by the preview builder and the
|
||||
// confirm-modal list so the two cannot drift.
|
||||
type PreviewNode = { nodeId: number; nodeName: string; willCreate: boolean; stacks: string[] };
|
||||
|
||||
const KICKER = 'font-mono text-[10px] uppercase tracking-[0.18em]';
|
||||
|
||||
export function BulkLabelAssignCard({ nodes }: Props) {
|
||||
@@ -55,14 +60,21 @@ export function BulkLabelAssignCard({ nodes }: Props) {
|
||||
const [running, setRunning] = useState(false);
|
||||
const [results, setResults] = useState<ResultRow[]>([]);
|
||||
|
||||
// Re-fetch only when the set of node ids changes, not on every parent render
|
||||
// (the nodes array is a fresh reference each render, e.g. on each fleet poll).
|
||||
// The latest nodes are read through a ref so the load effect can rebuild
|
||||
// per-node state without taking the array as a reactive dependency.
|
||||
// per-node state without taking the array as a reactive dependency (it is a
|
||||
// fresh reference each parent render, e.g. on each fleet poll).
|
||||
const nodesRef = useRef(nodes);
|
||||
useEffect(() => { nodesRef.current = nodes; });
|
||||
const nodeIds = nodes.map(n => n.id).join(',');
|
||||
|
||||
// Bumped by the manual Refresh so the snapshot is never trusted indefinitely:
|
||||
// a remote that recovers, or a label added/removed after the card opened, is
|
||||
// re-read on demand rather than confirmed against stale state.
|
||||
const [refreshKey, setRefreshKey] = useState(0);
|
||||
|
||||
// Reload when the node set changes or the operator refreshes (not on every
|
||||
// parent render). Resets the selection because a refreshed fleet can no longer
|
||||
// guarantee the previously picked stacks still exist.
|
||||
useEffect(() => {
|
||||
let cancelled = false;
|
||||
async function load() {
|
||||
@@ -94,7 +106,7 @@ export function BulkLabelAssignCard({ nodes }: Props) {
|
||||
}
|
||||
load();
|
||||
return () => { cancelled = true; };
|
||||
}, [nodeIds]);
|
||||
}, [nodeIds, refreshKey]);
|
||||
|
||||
// Distinct label templates across the fleet (name + deterministic color).
|
||||
const templates = useMemo<LabelTemplate[]>(() => {
|
||||
@@ -186,7 +198,15 @@ export function BulkLabelAssignCard({ nodes }: Props) {
|
||||
toast.error(body.error || 'Bulk label assign failed');
|
||||
return;
|
||||
}
|
||||
const apiResults = (body.results as AssignNodeResult[]) ?? [];
|
||||
// A 200 with a missing or non-array `results` is a server/contract bug,
|
||||
// not an empty assign: don't let it fall through to the success path with
|
||||
// zero counts. Log it and tell the operator it was unexpected.
|
||||
if (!Array.isArray(body.results)) {
|
||||
console.error('[BulkLabelAssign] bulk-assign returned a malformed body', body);
|
||||
toast.error('Bulk label assign returned an unexpected response. Check the server logs and retry.');
|
||||
return;
|
||||
}
|
||||
const apiResults = body.results as AssignNodeResult[];
|
||||
const rows: ResultRow[] = apiResults.map((node) => {
|
||||
const unreachable = node.reachable === false;
|
||||
const ok = node.stackResults.filter(s => s.success).length;
|
||||
@@ -210,7 +230,13 @@ export function BulkLabelAssignCard({ nodes }: Props) {
|
||||
const ok = allStacks.filter(s => s.success).length;
|
||||
const failed = allStacks.length - ok;
|
||||
const unreachableCount = apiResults.filter(n => n.reachable === false).length;
|
||||
if (failed === 0 && unreachableCount === 0) toast.success(`Assigned "${selectedTemplate.name}" to ${ok} stack${ok === 1 ? '' : 's'} across ${apiResults.length} node${apiResults.length === 1 ? '' : 's'}.`);
|
||||
if (ok > 0 && failed === 0 && unreachableCount === 0) toast.success(`Assigned "${selectedTemplate.name}" to ${ok} stack${ok === 1 ? '' : 's'} across ${apiResults.length} node${apiResults.length === 1 ? '' : 's'}.`);
|
||||
else if (ok === 0 && failed === 0 && unreachableCount === 0) {
|
||||
// Every node reported zero stack results for a non-empty request: a
|
||||
// contract break the success path above must not absorb.
|
||||
console.error('[BulkLabelAssign] bulk-assign returned no stack results', apiResults);
|
||||
toast.error('Bulk label assign returned no results. Check the server logs and retry.');
|
||||
}
|
||||
else toast.warning(`${ok} assigned, ${failed} failed${unreachableCount > 0 ? `, ${unreachableCount} node${unreachableCount === 1 ? '' : 's'} unreachable` : ''}. See results below.`);
|
||||
} catch (err) {
|
||||
toast.dismiss(toastId);
|
||||
@@ -236,7 +262,7 @@ export function BulkLabelAssignCard({ nodes }: Props) {
|
||||
const willCreate = !entry.labels.some(l => l.name === selectedTemplate.name);
|
||||
return { nodeId: entry.node.id, nodeName: entry.node.name, willCreate, stacks };
|
||||
})
|
||||
.filter((n): n is { nodeId: number; nodeName: string; willCreate: boolean; stacks: string[] } => n !== null);
|
||||
.filter((n): n is PreviewNode => n !== null);
|
||||
}, [nodeData, selected, selectedTemplate]);
|
||||
|
||||
return (
|
||||
@@ -264,9 +290,20 @@ export function BulkLabelAssignCard({ nodes }: Props) {
|
||||
title="Label · source"
|
||||
meta={loading ? 'loading…' : `${templates.length} label${templates.length === 1 ? '' : 's'}`}
|
||||
>
|
||||
<p className={cn(KICKER, 'text-stat-subtitle mb-2 normal-case tracking-normal text-[11px]')}>
|
||||
Pick a stack label from anywhere in the fleet. It is added to the selected stacks on each node, and created there with this color if the node does not have it yet.
|
||||
</p>
|
||||
<div className="flex items-start justify-between gap-2 mb-2">
|
||||
<p className={cn(KICKER, 'text-stat-subtitle normal-case tracking-normal text-[11px]')}>
|
||||
Pick a stack label from anywhere in the fleet. It is added to the selected stacks on each node, and created there with this color if the node does not have it yet.
|
||||
</p>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setRefreshKey(k => k + 1)}
|
||||
disabled={running || loading}
|
||||
title="Re-read stacks and labels from every node"
|
||||
className={cn(KICKER, 'shrink-0 text-stat-subtitle hover:text-stat-value disabled:opacity-50')}
|
||||
>
|
||||
{loading ? 'Refreshing…' : 'Refresh'}
|
||||
</button>
|
||||
</div>
|
||||
<div className="flex flex-wrap gap-1.5 max-h-32 overflow-auto p-2 border border-card-border/40 rounded-md">
|
||||
{templates.length === 0 && (
|
||||
<span className="text-xs text-stat-subtitle">
|
||||
@@ -398,7 +435,30 @@ export function BulkLabelAssignCard({ nodes }: Props) {
|
||||
confirmLabel="Apply"
|
||||
confirming={running}
|
||||
onConfirm={run}
|
||||
/>
|
||||
>
|
||||
{previewNodes.length > 0 && <AffectedTargetsList nodes={previewNodes} />}
|
||||
</ConfirmModal>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
// The concrete node/stack list carried into the confirm modal so the operator
|
||||
// approves against the names being changed, not a bare stack/node count. Mirrors
|
||||
// the stop card's resolved-targets list; `creates label` flags nodes where the
|
||||
// label does not exist yet and will be created.
|
||||
function AffectedTargetsList({ nodes }: { nodes: PreviewNode[] }) {
|
||||
return (
|
||||
<div className="max-h-[180px] overflow-y-auto rounded border border-card-border/60 bg-card/40 p-2">
|
||||
<ul className="space-y-1.5">
|
||||
{nodes.map(n => (
|
||||
<li key={n.nodeId} className="flex items-start gap-2">
|
||||
<span className={cn(KICKER, 'shrink-0 pt-0.5', n.willCreate ? 'text-amber-400' : 'text-stat-subtitle')}>
|
||||
{n.nodeName}{n.willCreate ? ' · creates label' : ''}
|
||||
</span>
|
||||
<span className="min-w-0 flex-1 break-words font-mono text-[11px] text-stat-value">{n.stacks.join(', ')}</span>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -230,7 +230,9 @@ it('gates Stop fleet on a resolved preview, carries the node/stack list into the
|
||||
|
||||
await waitFor(() => {
|
||||
const stopCall = mockedFetch.mock.calls.find(c => c[0] === '/fleet/labels/fleet-stop');
|
||||
expect(JSON.parse(stopCall![1].body)).toEqual({ labelName: 'prod', dryRun: false });
|
||||
// The real stop binds execution to the nodes resolved in the preview: it
|
||||
// carries their ids so a node that was unreachable then cannot be added later.
|
||||
expect(JSON.parse(stopCall![1].body)).toEqual({ labelName: 'prod', dryRun: false, nodeIds: [1] });
|
||||
});
|
||||
// Per-node results render in the card (the "· 1 stack" label is unique to the
|
||||
// results list, distinguishing it from the preview well).
|
||||
|
||||
@@ -36,7 +36,7 @@ type PreviewState =
|
||||
// One node and the stacks the resolved blast radius would stop on it. The
|
||||
// destructive Stop stays disabled until this is known, so the operator always
|
||||
// confirms against a concrete node/stack list rather than a label name alone.
|
||||
interface ResolvedTarget { nodeName: string; stackNames: string[] }
|
||||
interface ResolvedTarget { nodeId: number; nodeName: string; stackNames: string[] }
|
||||
|
||||
const KICKER = 'font-mono text-[10px] uppercase tracking-[0.18em]';
|
||||
const PREVIEW_ROW_LIMIT = 6;
|
||||
@@ -190,7 +190,7 @@ export function LabelFleetStopCard() {
|
||||
|
||||
const blastTone = preview.kind === 'loading' || preview.kind === 'unavailable' ? 'muted' as const : undefined;
|
||||
|
||||
async function run(opts: { dryRun: boolean }) {
|
||||
async function run(opts: { dryRun: boolean; nodeIds?: number[] }) {
|
||||
const trimmed = labelName.trim();
|
||||
if (!trimmed) return;
|
||||
const verb = opts.dryRun ? 'Dry-running' : 'Stopping';
|
||||
@@ -198,9 +198,12 @@ export function LabelFleetStopCard() {
|
||||
setRunning(true);
|
||||
setResults([]);
|
||||
try {
|
||||
// The real stop carries the node ids the operator confirmed in the preview
|
||||
// so execution cannot expand to a node that was unreachable then and has
|
||||
// since reconnected. The dry run sends no allowlist and scans the fleet.
|
||||
const res = await apiFetch('/fleet/labels/fleet-stop', {
|
||||
method: 'POST',
|
||||
body: JSON.stringify({ labelName: trimmed, dryRun: opts.dryRun }),
|
||||
body: JSON.stringify({ labelName: trimmed, dryRun: opts.dryRun, ...(opts.nodeIds ? { nodeIds: opts.nodeIds } : {}) }),
|
||||
});
|
||||
const body = await res.json().catch(() => ({}));
|
||||
toast.dismiss(toastId);
|
||||
@@ -251,7 +254,7 @@ export function LabelFleetStopCard() {
|
||||
// against it even when the live preview endpoint is down.
|
||||
const targets = apiResults
|
||||
.filter(n => n.reachable && n.matched && Array.isArray(n.stackResults) && n.stackResults.length > 0)
|
||||
.map(n => ({ nodeName: n.nodeName, stackNames: n.stackResults.map(s => s.stackName) }));
|
||||
.map(n => ({ nodeId: n.nodeId, nodeName: n.nodeName, stackNames: n.stackResults.map(s => s.stackName) }));
|
||||
setDryRunResolved({ label: trimmed, targets });
|
||||
}
|
||||
const reachable = apiResults.filter(n => n.reachable);
|
||||
@@ -288,7 +291,7 @@ export function LabelFleetStopCard() {
|
||||
if (preview.kind === 'ready') {
|
||||
return preview.data.perNode
|
||||
.filter(n => n.reachable && n.stackCount > 0)
|
||||
.map(n => ({ nodeName: n.nodeName, stackNames: n.stackNames }));
|
||||
.map(n => ({ nodeId: n.nodeId, nodeName: n.nodeName, stackNames: n.stackNames }));
|
||||
}
|
||||
if (dryRunResolved && dryRunResolved.label === trimmed && trimmed.length > 0) {
|
||||
return dryRunResolved.targets;
|
||||
@@ -370,7 +373,7 @@ export function LabelFleetStopCard() {
|
||||
description="Sencho will stop the stacks listed below. Node labels are not used by this action. Services will be unavailable until restarted."
|
||||
confirmLabel="Stop fleet"
|
||||
confirming={running}
|
||||
onConfirm={() => run({ dryRun: false })}
|
||||
onConfirm={() => run({ dryRun: false, nodeIds: (resolvedTargets ?? []).map(t => t.nodeId) })}
|
||||
>
|
||||
{resolvedTargets && resolvedTargets.length > 0 && <ResolvedTargetsList targets={resolvedTargets} />}
|
||||
</ConfirmModal>
|
||||
@@ -428,14 +431,15 @@ function renderPreviewSection(preview: PreviewState, trimmed: string) {
|
||||
|
||||
// The resolved node/stack list carried into the destructive confirm modal, so
|
||||
// the operator confirms against the concrete blast radius rather than a label
|
||||
// name. Final membership is re-resolved per node at execution time (state can
|
||||
// drift between preview and confirm), which the header states plainly.
|
||||
// name. The node set is bound to this list (the stop sends these node ids and
|
||||
// the backend acts on no others); only the per-node stacks are re-matched by
|
||||
// label at execution, where state can still drift between preview and confirm.
|
||||
function ResolvedTargetsList({ targets }: { targets: ResolvedTarget[] }) {
|
||||
const stackCount = targets.reduce((n, t) => n + t.stackNames.length, 0);
|
||||
return (
|
||||
<div>
|
||||
<div className={cn(KICKER, 'text-stat-icon mb-2 normal-case tracking-normal text-[11px]')}>
|
||||
Will stop {stackCount} stack{stackCount === 1 ? '' : 's'} across {targets.length} node{targets.length === 1 ? '' : 's'}. The final set is re-resolved on each node at execution time.
|
||||
Will stop {stackCount} stack{stackCount === 1 ? '' : 's'} across {targets.length} node{targets.length === 1 ? '' : 's'}. The stacks on each node are re-matched by label at execution; the node set is fixed to those listed.
|
||||
</div>
|
||||
<div className="max-h-[180px] overflow-y-auto rounded border border-card-border/60 bg-card/40 p-2">
|
||||
<ul className="space-y-1.5">
|
||||
|
||||
Reference in New Issue
Block a user