fix(federation): gate node cordon control on node:manage to match backend (#1277)

* fix(federation): gate node cordon control on node:manage to match backend

The cordon and uncordon control on the node card rendered whenever the
instance held an Admiral license, but the backend route also requires the
node:manage permission. Non-admin users without that permission (deployer,
viewer, auditor) saw a Cordon action the API rejected with 403. Gate the
control on the same permission the route enforces, so it renders only for
users who can use it.

Add developer-mode diagnostic logging to the cordon and pin handlers, and
route-level tests covering the cordon and uncordon permission, tier,
API-token, and input-validation boundaries.

* fix(federation): reject non-numeric node ids in cordon/uncordon

The cordon and uncordon routes validated the node id with parseInt, which
accepts numeric-prefix strings (parseInt('1abc', 10) === 1), so a request to
/api/nodes/1abc/cordon would operate on node 1. Require a strict positive
integer before parsing, and add tests for both routes.

* fix(federation): sanitize user-derived values in cordon and pin diagnostics

Route the node id, blueprint id, target node id, and reason length through
the shared log sanitizer before they reach the developer-mode diagnostic
log lines, and switch the format specifiers to %s to match. The values are
already validated integers, so this is defense in depth at the log sink and
keeps the diagnostics on the same sanitize-every-interpolated-value pattern
used elsewhere. No runtime behavior change: the lines stay gated behind the
developer_mode setting, off by default.
This commit is contained in:
Anso
2026-06-02 09:48:19 -04:00
committed by GitHub
parent e60c1c0525
commit dd2c2b22ec
5 changed files with 355 additions and 9 deletions
@@ -68,13 +68,16 @@ export function NodeCard({ node, onNavigate, labelMap, updateStatus, onUpdate, u
const [cordonSubmitting, setCordonSubmitting] = useState(false);
const { isPaid, license } = useLicense();
const { isAdmin } = useAuth();
const { isAdmin, can } = useAuth();
const { nodes: registryNodes } = useNodes();
const isAdmiral = isPaid && license?.variant === 'admiral';
const registryNode = registryNodes.find(n => n.id === node.id);
const canEdit = Boolean(isAdmin && onEdit && registryNode);
const canDelete = Boolean(isAdmin && onDelete && registryNode && !registryNode.is_default);
const canCordon = isAdmiral;
// Cordon is Admiral-tier AND requires node:manage, matching the backend guard
// (requirePermission('node:manage','node',id) + requireAdmiral). Gating on tier
// alone would surface the control to deployer/viewer/auditor users whose calls 403.
const canCordon = isAdmiral && can('node:manage', 'node', String(node.id));
const showMenu = canEdit || canDelete || canCordon;
const isOnline = node.status === 'online';
@@ -1,5 +1,6 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
const useLicenseMock = vi.fn();
const useAuthMock = vi.fn();
@@ -34,7 +35,7 @@ function baseProps(node: FleetNode) {
beforeEach(() => {
useNodesMock.mockReturnValue({ nodes: [] });
useAuthMock.mockReturnValue({ isAdmin: true });
useAuthMock.mockReturnValue({ isAdmin: true, can: vi.fn(() => true) });
useLicenseMock.mockReturnValue({ isPaid: false, license: null });
});
afterEach(() => vi.clearAllMocks());
@@ -60,13 +61,43 @@ describe('NodeCard', () => {
expect(screen.queryByRole('button', { name: 'Node actions' })).not.toBeInTheDocument();
});
it('exposes the actions menu (cordon entry point) for an admiral user', () => {
it('exposes the actions menu (cordon entry point) for an admiral admin', () => {
useLicenseMock.mockReturnValue({ isPaid: true, license: { variant: 'admiral' } });
render(<NodeCard {...baseProps(onlineNode())} />);
// The actions menu only renders when cordon (admiral-only here) is available.
// With no edit/delete affordances wired, the menu renders iff cordon is
// allowed: isAdmiral && can('node:manage'). The admin's can() returns true.
expect(screen.getByRole('button', { name: 'Node actions' })).toBeInTheDocument();
});
it('exposes the cordon control for a node-admin via the node:manage permission', async () => {
const can = vi.fn((action: string) => action === 'node:manage');
useAuthMock.mockReturnValue({ isAdmin: false, can });
useLicenseMock.mockReturnValue({ isPaid: true, license: { variant: 'admiral' } });
render(<NodeCard {...baseProps(onlineNode())} />);
await userEvent.click(screen.getByRole('button', { name: 'Node actions' }));
expect(await screen.findByText('Cordon node')).toBeInTheDocument();
expect(can).toHaveBeenCalledWith('node:manage', 'node', '2');
});
it('hides the cordon control from an admiral user lacking node:manage', () => {
useAuthMock.mockReturnValue({ isAdmin: false, can: vi.fn(() => false) });
useLicenseMock.mockReturnValue({ isPaid: true, license: { variant: 'admiral' } });
render(<NodeCard {...baseProps(onlineNode())} />);
// Admiral tier alone must not surface cordon to a deployer/viewer/auditor.
expect(screen.queryByRole('button', { name: 'Node actions' })).not.toBeInTheDocument();
});
it('shows Uncordon when the node is already cordoned', async () => {
const can = vi.fn((action: string) => action === 'node:manage');
useAuthMock.mockReturnValue({ isAdmin: false, can });
useLicenseMock.mockReturnValue({ isPaid: true, license: { variant: 'admiral' } });
render(<NodeCard {...baseProps({ ...onlineNode(), cordoned: true, cordoned_reason: 'patching' })} />);
await userEvent.click(screen.getByRole('button', { name: 'Node actions' }));
expect(await screen.findByText('Uncordon node')).toBeInTheDocument();
});
const updateAvailableStatus = {
nodeId: 2, name: 'Edge', type: 'remote' as const, version: '1.0.0', latestVersion: '1.1.0',
updateAvailable: true, updateStatus: null,