mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-10 10:49:35 +00:00
fix(rbac): permission-gate alerts, auto-heal, and image updates (#1743)
* fix: gate alerts and auto-heal routes on stack:edit/stack:read permissions Replace requireAdmin with requirePermission across backend/src/routes/alerts.ts and backend/src/routes/autoHeal.ts, mirroring the stack:read/stack:edit model already used by stacks, blueprints, git sources, and settings. Adds the previously-missing permission gate on the auto-heal history route, and adds ownership-aware deletion for alerts via a new DatabaseService.getStackAlert(id) lookup. * fix: gate image-update fleet, per-stack refresh, and auto-update execute on RBAC permissions Replace requireAdmin with requirePermission/checkPermission across backend/src/routes/imageUpdates.ts (imageUpdatesRouter and autoUpdateRouter), mirroring the permission-aware model already used by alerts and auto-heal. GET /fleet drops its admin gate to match the auth-only read model shared with GET / and /detail. POST /fleet/refresh now requires node:manage. A new route, POST /refresh/:stackName, lets a caller with stack:deploy on that stack trigger a per-stack recheck, distinct from the node-wide POST /refresh. The auto-update executor now pre-checks stack:deploy across every resolved target before any work starts, so a denied stack in a bulk request fails the whole call instead of partially executing; the "*" wildcard additionally requires global stack:deploy up front since it expands to every stack on the node, including the empty case where a per-stack check would otherwise have nothing to gate. * fix: evaluate permission before checks-enabled state in auto-update execute The checks-enabled short-circuit in autoUpdateRouter POST /execute ran before target parsing and before any permission check, so a node with image-update checks disabled returned 200 to any authenticated caller regardless of stack:deploy grants. Move the checks-enabled check to run after the resolved stackNames have cleared requireExactStacks, so permission is always evaluated first. Add coverage: a denied role still gets 403 PERMISSION_DENIED (not the disabled-checks 200) while checks are disabled node-wide, and a scoped-only user whose stack:deploy grant covers every stack on the node is still denied target="*" (the wildcard requires global stack:deploy, per the earlier fix), proving that tradeoff against a real on-disk stack rather than the always- empty fresh test instance. * fix: gate alerts, auto-heal, and image-update controls on frontend permission checks Match the backend RBAC gates for alerts, auto-heal, and per-stack image updates with matching frontend checks, replacing raw isAdmin/node:manage gates with scoped can() calls: - Alerts/Auto-Heal menu items and their keyboard shortcuts now gate on stack:read (canViewMonitor), including the window-level keyboard shortcut handler that previously bypassed the menu item gate entirely. - Check updates now gates on stack:deploy (previously node:manage) and calls the new per-stack POST /image-updates/refresh/:stackName endpoint instead of the node-wide refresh. Since the endpoint runs the recheck synchronously and returns the result directly, the old node-wide /status polling loop is removed in favor of handling the response inline. - StackAlertSheet's alert and auto-heal policy mutation controls gate on stack:edit instead of isAdmin. - The Fleet Image Updates refresh button (mobile and desktop) gates on node:manage, hidden rather than disabled to match the existing convention for node:manage-gated affordances. * fix: cover the stack:edit deny path for StackAlertSheet gates The useAuth mock in StackAlertSheet.test.tsx returned can: () => true unconditionally, so canEditAlerts, canEditAutoHeal, and PolicyRow's canEdit prop were never exercised with a denial. Make the mock per-test-controllable (matching the vi.fn() pattern already used in NodeCard.test.tsx) and add one deny-path test per tab asserting the mutation controls are absent while reads stay visible. Also adds an aria-label to the alert row's delete button so the deny test can assert on its absence, matching the aria-label convention PolicyRow's own toggle/delete controls already use. * fix: surface accurate warnings and loading feedback on stack update checks checkUpdatesForStack ignored the backend's StackRecheckResult outcome and always showed a success toast, even when verification failed or an update is still present. It also gave no feedback while the multi-second per-image registry probe was in flight. Add a loading toast on request start, and branch the result toast on outcome/warning instead of unconditional success. The backend reuses its post-update reconciliation copy for this pre-update discovery check, so the two generic "update command completed" strings are replaced with accurate pre-update wording; a genuine stack-specific warning (e.g. a compose render failure) is still shown as-is. Also update docs/features/rbac.mdx: stack:edit now covers alert and auto-heal management, stack:deploy covers per-stack image-update checks, and the Deployer role description reflects both. * fix: add per-stack cooldown rate limit for image-update recheck route The per-stack POST /refresh/:stackName route bypassed the existing node-wide manual-refresh cooldown. A caller with stack:deploy could hammer the registry with unbounded concurrent recheck calls. Add tryMarkStackRecheck in ImageUpdateService, sharing the same 2-minute cooldown window, keyed per (nodeId, stackName). The route handler returns 429 when denied. The mark is written synchronously before the first await so concurrent calls on the same tick are blocked.
This commit is contained in:
@@ -10,10 +10,13 @@ vi.mock('@/context/NodeContext', () => ({
|
||||
// buildMenuCtx derives canOpenApp from the active node plus the stack's
|
||||
// published port; only the fields it reads need to be real, the handler
|
||||
// closures are never invoked here.
|
||||
type CanFn = (action: string, resourceType?: string, resourceId?: string, nodeId?: number | null) => boolean;
|
||||
|
||||
function makeOptions(
|
||||
activeNode: Node | null,
|
||||
stackPorts: Record<string, number | undefined>,
|
||||
stackStatuses: Record<string, string> = { 'web.yml': 'running' },
|
||||
can: CanFn = () => true,
|
||||
) {
|
||||
const stackListState = {
|
||||
stackStatuses,
|
||||
@@ -42,10 +45,16 @@ function makeOptions(
|
||||
stackActions,
|
||||
activeNode,
|
||||
isAdmin: true,
|
||||
can: () => true,
|
||||
can,
|
||||
} as unknown as Parameters<typeof useSidebarContextMenu>[0];
|
||||
}
|
||||
|
||||
// Reach past the `unknown` cast makeOptions returns to assert on the inner
|
||||
// stackActions mocks it built.
|
||||
function stackActionsOf(options: Parameters<typeof useSidebarContextMenu>[0]) {
|
||||
return (options as unknown as { stackActions: { checkUpdatesForStack: ReturnType<typeof vi.fn> } }).stackActions;
|
||||
}
|
||||
|
||||
describe('useSidebarContextMenu canOpenApp', () => {
|
||||
it('is true for a local node with a published port', () => {
|
||||
const { result } = renderHook(() =>
|
||||
@@ -89,3 +98,36 @@ describe('useSidebarContextMenu stackStatus', () => {
|
||||
expect(missing.result.current('web.yml').stackStatus).toBe('unknown');
|
||||
});
|
||||
});
|
||||
|
||||
describe('useSidebarContextMenu checkUpdates', () => {
|
||||
it('calls checkUpdatesForStack with the stack name (not the .yml file)', () => {
|
||||
const options = makeOptions({ id: 1, type: 'local' } as Node, { 'web.yml': 8989 });
|
||||
const { result } = renderHook(() => useSidebarContextMenu(options));
|
||||
result.current('web.yml').checkUpdates();
|
||||
expect(stackActionsOf(options).checkUpdatesForStack).toHaveBeenCalledWith('web');
|
||||
});
|
||||
});
|
||||
|
||||
describe('useSidebarContextMenu canViewMonitor / canCheckUpdates wiring', () => {
|
||||
it('derives canViewMonitor from stack:read and canCheckUpdates from stack:deploy, both scoped to the stack and active node', () => {
|
||||
const can = vi.fn<CanFn>((action) => action === 'stack:read');
|
||||
const options = makeOptions({ id: 7, type: 'local' } as Node, { 'web.yml': 8989 }, undefined, can);
|
||||
const { result } = renderHook(() => useSidebarContextMenu(options));
|
||||
const ctx = result.current('web.yml');
|
||||
|
||||
expect(ctx.canViewMonitor).toBe(true);
|
||||
expect(ctx.canCheckUpdates).toBe(false);
|
||||
expect(can).toHaveBeenCalledWith('stack:read', 'stack', 'web', 7);
|
||||
expect(can).toHaveBeenCalledWith('stack:deploy', 'stack', 'web', 7);
|
||||
});
|
||||
|
||||
it('denies both when the permission check fails closed', () => {
|
||||
const can = vi.fn<CanFn>(() => false);
|
||||
const options = makeOptions({ id: 7, type: 'local' } as Node, { 'web.yml': 8989 }, undefined, can);
|
||||
const { result } = renderHook(() => useSidebarContextMenu(options));
|
||||
const ctx = result.current('web.yml');
|
||||
|
||||
expect(ctx.canViewMonitor).toBe(false);
|
||||
expect(ctx.canCheckUpdates).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -20,7 +20,6 @@ import type { useViewNavigationState } from './useViewNavigationState';
|
||||
import type { Node } from '@/context/NodeContext';
|
||||
import { useNodes } from '@/context/NodeContext';
|
||||
import type { PermissionAction } from '@/context/AuthContext';
|
||||
import { canManageNode } from '@/lib/canManageNode';
|
||||
|
||||
type StackListState = ReturnType<typeof useStackListState>;
|
||||
type NavState = ReturnType<typeof useViewNavigationState>;
|
||||
@@ -65,6 +64,7 @@ export function useSidebarContextMenu({
|
||||
canDelete: can('stack:delete', 'stack', sName, nodeId),
|
||||
canDeploy: can('stack:deploy', 'stack', sName, nodeId),
|
||||
canEditLabels: can('stack:edit', 'stack', sName, nodeId),
|
||||
canViewMonitor: can('stack:read', 'stack', sName, nodeId),
|
||||
// POST /api/labels (the inline "New label" entry) is guarded by the
|
||||
// unscoped requirePermission('stack:edit'); a user with only per-stack
|
||||
// scoped edit can toggle existing labels but cannot create new ones.
|
||||
@@ -75,8 +75,8 @@ export function useSidebarContextMenu({
|
||||
menuVisibility: stackActions.getStackMenuVisibility(file),
|
||||
openAlertSheet: () => overlayState.openAlertSheet(file),
|
||||
openAutoHeal: () => overlayState.openAutoHeal(file),
|
||||
canCheckUpdates: canManageNode(can, nodeId),
|
||||
checkUpdates: () => stackActions.checkUpdatesForStack(),
|
||||
canCheckUpdates: can('stack:deploy', 'stack', sName, nodeId),
|
||||
checkUpdates: () => stackActions.checkUpdatesForStack(sName),
|
||||
openStackApp: () => stackActions.openStackApp(file),
|
||||
deploy: () => stackActions.executeStackActionByFile(file, 'deploy', 'deploy'),
|
||||
stop: () => stackActions.executeStackActionByFile(file, 'stop', 'stop'),
|
||||
|
||||
@@ -15,7 +15,7 @@ vi.mock('@/lib/api', () => ({
|
||||
}),
|
||||
}));
|
||||
vi.mock('@/components/ui/toast-store', () => ({
|
||||
toast: { success: vi.fn(), error: vi.fn(), info: vi.fn() },
|
||||
toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), loading: vi.fn(() => 'loading-id'), dismiss: vi.fn() },
|
||||
}));
|
||||
|
||||
import { apiFetch } from '@/lib/api';
|
||||
@@ -249,6 +249,87 @@ describe('useStackActions.handleSaveAndDeploy', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('useStackActions.checkUpdatesForStack', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
vi.mocked(apiFetch).mockReset();
|
||||
});
|
||||
|
||||
it('hits the per-stack refresh endpoint and shows success when the stack is cleared', async () => {
|
||||
vi.mocked(apiFetch).mockResolvedValue(
|
||||
new Response(JSON.stringify({ outcome: 'cleared', warning: null }), { status: 200 }),
|
||||
);
|
||||
const { result, stackListState } = setup();
|
||||
await result.current.checkUpdatesForStack('web');
|
||||
expect(apiFetch).toHaveBeenCalledWith('/image-updates/refresh/web', { method: 'POST' });
|
||||
expect(stackListState.fetchImageUpdates).toHaveBeenCalled();
|
||||
expect(toast.dismiss).toHaveBeenCalledWith('loading-id');
|
||||
expect(toast.success).toHaveBeenCalledWith('Image update check complete.');
|
||||
expect(toast.info).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('shows the warning via toast.info instead of success when verification did not cleanly complete', async () => {
|
||||
vi.mocked(apiFetch).mockResolvedValue(
|
||||
new Response(
|
||||
JSON.stringify({ outcome: 'verification_failed', warning: 'Could not verify the update.' }),
|
||||
{ status: 200 },
|
||||
),
|
||||
);
|
||||
const { result } = setup();
|
||||
await result.current.checkUpdatesForStack('web');
|
||||
expect(toast.dismiss).toHaveBeenCalledWith('loading-id');
|
||||
expect(toast.info).toHaveBeenCalledWith('Could not verify the update.');
|
||||
expect(toast.success).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('replaces the generic post-update warning copy for an incomplete verification too', async () => {
|
||||
vi.mocked(apiFetch).mockResolvedValue(
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
outcome: 'verification_incomplete',
|
||||
warning: 'The update command completed, but Sencho could not fully verify whether an image update remains.',
|
||||
}),
|
||||
{ status: 200 },
|
||||
),
|
||||
);
|
||||
const { result } = setup();
|
||||
await result.current.checkUpdatesForStack('web');
|
||||
expect(toast.info).toHaveBeenCalledWith('Could not fully verify update status for web.');
|
||||
expect(toast.info).not.toHaveBeenCalledWith(expect.stringContaining('update command completed'));
|
||||
});
|
||||
|
||||
it('uses stack-scoped copy instead of the backend post-update warning when an update is still present', async () => {
|
||||
// The backend reuses its post-update reconciliation result for this
|
||||
// manual pre-update check, so its "still_present" warning text ("The
|
||||
// update command completed...") does not apply here; the frontend must
|
||||
// not forward it verbatim.
|
||||
vi.mocked(apiFetch).mockResolvedValue(
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
outcome: 'still_present',
|
||||
warning: 'The update command completed, but Sencho still detects an available image update.',
|
||||
}),
|
||||
{ status: 200 },
|
||||
),
|
||||
);
|
||||
const { result } = setup();
|
||||
await result.current.checkUpdatesForStack('web');
|
||||
expect(toast.dismiss).toHaveBeenCalledWith('loading-id');
|
||||
expect(toast.info).toHaveBeenCalledWith('web still has an update available.');
|
||||
expect(toast.info).not.toHaveBeenCalledWith(expect.stringContaining('update command completed'));
|
||||
expect(toast.success).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('shows a loading toast immediately and dismisses it on error', async () => {
|
||||
vi.mocked(apiFetch).mockResolvedValue(new Response(JSON.stringify({ error: 'nope' }), { status: 500 }));
|
||||
const { result } = setup();
|
||||
await result.current.checkUpdatesForStack('web');
|
||||
expect(toast.loading).toHaveBeenCalledWith('Checking web for image updates...');
|
||||
expect(toast.dismiss).toHaveBeenCalledWith('loading-id');
|
||||
expect(toast.error).toHaveBeenCalledWith('nope');
|
||||
});
|
||||
});
|
||||
|
||||
describe('useStackActions node binding', () => {
|
||||
const mouseEvent = { preventDefault: vi.fn(), stopPropagation: vi.fn() } as unknown as React.MouseEvent;
|
||||
|
||||
|
||||
@@ -79,6 +79,16 @@ const NODE_UNREACHABLE_FAILURE: FailureClassification = {
|
||||
|
||||
const UNREACHABLE_STATUSES: ReadonlySet<number> = new Set([502, 503, 504]);
|
||||
|
||||
// Mirrors ImageUpdateService's UPDATE_STILL_PRESENT_WARNING / UPDATE_VERIFICATION_INCOMPLETE_WARNING:
|
||||
// that service's warning copy assumes an update was just applied, but
|
||||
// checkUpdatesForStack runs before any update, so these two generic messages
|
||||
// are replaced with accurate pre-update copy. A stack-specific reason (e.g. a
|
||||
// compose render failure) is still forwarded as-is.
|
||||
const GENERIC_POST_UPDATE_WARNINGS: ReadonlySet<string> = new Set([
|
||||
'The update command completed, but Sencho still detects an available image update.',
|
||||
'The update command completed, but Sencho could not fully verify whether an image update remains.',
|
||||
]);
|
||||
|
||||
const SELF_STACK_PROTECTED_CODE = 'self_stack_protected';
|
||||
|
||||
const isSelfStackProtectedResponse = (rawBody: string, status?: number): boolean => {
|
||||
@@ -416,7 +426,6 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
|
||||
const pendingStackLoadRef = useRef<string | null>(null);
|
||||
const pendingLogsRef = useRef<{ stackName: string; containerName: string } | null>(null);
|
||||
const checkUpdatesIntervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
|
||||
// True from a deploy click through the async pre-deploy advisory phase until
|
||||
// the deploy starts or is cancelled, so a double-click cannot start two deploys.
|
||||
const deployPendingRef = useRef(false);
|
||||
@@ -471,9 +480,6 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
if (checkUpdatesIntervalRef.current !== null) {
|
||||
clearInterval(checkUpdatesIntervalRef.current);
|
||||
}
|
||||
loadFileAbortRef.current?.abort();
|
||||
containersFetchGenRef.current += 1;
|
||||
};
|
||||
@@ -2244,38 +2250,32 @@ export function useStackActions(options: UseStackActionsOptions) {
|
||||
}
|
||||
};
|
||||
|
||||
const checkUpdatesForStack = async () => {
|
||||
const checkUpdatesForStack = async (stackName: string) => {
|
||||
const loadingId = toast.loading(`Checking ${stackName} for image updates...`);
|
||||
try {
|
||||
const res = await apiFetch('/image-updates/refresh', { method: 'POST' });
|
||||
const res = await apiFetch(`/image-updates/refresh/${encodeURIComponent(stackName)}`, { method: 'POST' });
|
||||
if (res.ok) {
|
||||
toast.success('Checking for image updates...');
|
||||
let elapsed = 0;
|
||||
const poll = setInterval(async () => {
|
||||
elapsed += 2000;
|
||||
try {
|
||||
const statusRes = await apiFetch('/image-updates/status');
|
||||
if (statusRes.ok) {
|
||||
const { checking } = await statusRes.json();
|
||||
if (!checking || elapsed >= 60000) {
|
||||
clearInterval(poll);
|
||||
checkUpdatesIntervalRef.current = null;
|
||||
await stackListState.fetchImageUpdates();
|
||||
if (!checking) toast.success('Image update check complete.');
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
clearInterval(poll);
|
||||
checkUpdatesIntervalRef.current = null;
|
||||
await stackListState.fetchImageUpdates();
|
||||
}
|
||||
}, 2000);
|
||||
checkUpdatesIntervalRef.current = poll;
|
||||
const data = await res.json().catch(() => ({})) as { outcome?: unknown; warning?: unknown };
|
||||
await stackListState.fetchImageUpdates();
|
||||
const warning = typeof data.warning === 'string' ? data.warning : undefined;
|
||||
if (data.outcome === 'still_present') {
|
||||
toast.info(`${stackName} still has an update available.`);
|
||||
} else if (warning && GENERIC_POST_UPDATE_WARNINGS.has(warning)) {
|
||||
toast.info(`Could not fully verify update status for ${stackName}.`);
|
||||
} else if (warning) {
|
||||
toast.info(warning);
|
||||
} else {
|
||||
toast.success('Image update check complete.');
|
||||
}
|
||||
} else {
|
||||
const data = await res.json().catch(() => ({}));
|
||||
toast.error(data.error || 'Failed to check for updates');
|
||||
}
|
||||
} catch {
|
||||
} catch (error) {
|
||||
console.error(`Failed to check updates for stack ${stackName}:`, error);
|
||||
toast.error('Failed to check for updates');
|
||||
} finally {
|
||||
toast.dismiss(loadingId);
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user