mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-09 18:32:52 +00:00
fix(sidebar): require admin role for Schedule task and debounce search input (#1243)
The right-click Schedule task menu item and its keyboard shortcut were gated
only on isPaid, but the backend write routes under /api/scheduled-tasks
enforce requireAdmin + requirePaid on every action. Non-admin Skipper or
Admiral users would see the menu item and hit a 403 on click. The frontend
now mirrors the backend by gating Schedule task on isPaid && isAdmin so the
affordance only renders for users whose action will actually succeed.
Also adds a 120ms keystroke debounce to the sidebar search input. The
useStackListState filter rebuild was previously running on every keystroke
because <Command shouldFilter={false}> disables cmdk's own filter and the
existing 250ms timer only debounces state-invalidate events. Visible input
stays immediate via local state; the debounced emit drives the filter pass.
Adds a regression guard that /api/stacks/statuses is short-circuited by the
remote-node proxy (covers the sidebar status poll path) and updates the
sidebar feature docs to reflect the admin role requirement on Schedule task.
This commit is contained in:
@@ -76,6 +76,21 @@ describe('remote proxy mount order', () => {
|
||||
expect(res.status).not.toBe(502);
|
||||
});
|
||||
|
||||
it('short-circuits /api/stacks/statuses (the sidebar status poll) for remote nodes', async () => {
|
||||
// The sidebar polls /api/stacks/statuses every few seconds; if a future
|
||||
// refactor accidentally mounted a local fast-path before the proxy, every
|
||||
// operator viewing a remote node would silently see the central instance's
|
||||
// own statuses. Asserts the same 502 short-circuit as /api/stacks.
|
||||
const res = await request(app)
|
||||
.get('/api/stacks/statuses')
|
||||
.set('Authorization', authHeader)
|
||||
.set('x-node-id', String(remoteNodeId));
|
||||
|
||||
expect(res.status).toBe(502);
|
||||
expect(res.headers['x-sencho-proxy']).toBeUndefined();
|
||||
expect(res.body?.error).toMatch(/unreachable/i);
|
||||
});
|
||||
|
||||
it('handles proxy-exempt paths locally even when x-node-id targets a remote', async () => {
|
||||
// /api/nodes/:id is in PROXY_EXEMPT_PREFIXES. The proxy must never catch
|
||||
// gateway-level concerns; otherwise a user whose default node is remote
|
||||
|
||||
@@ -85,7 +85,7 @@ Right-click any stack (or open the kebab that appears on hover) for its context
|
||||
- **Destructive**: **Delete**.
|
||||
|
||||
<Note>
|
||||
**Auto-Heal** and **Schedule task** require a **Skipper** or **Admiral** license.
|
||||
**Auto-Heal** requires a **Skipper** or **Admiral** license. **Schedule task** requires a **Skipper** or **Admiral** license and an **admin** role.
|
||||
</Note>
|
||||
|
||||
<Frame>
|
||||
@@ -143,4 +143,7 @@ The footer surfaces the most recent stack lifecycle event on the node. Each tick
|
||||
<Accordion title="The filter chips disappeared from the sidebar">
|
||||
Click the **+** icon to the right of the search box to bring them back. The chip row collapses to a thin **−** / **+** toggle, and the state is remembered in your browser, so an earlier collapse persists across reloads until you expand it again.
|
||||
</Accordion>
|
||||
<Accordion title="Schedule task is missing from the right-click menu">
|
||||
Scheduling requires a **Skipper** or **Admiral** license and an **admin** role. Ask an admin on this node to schedule the task for you, or sign in with an admin account.
|
||||
</Accordion>
|
||||
</AccordionGroup>
|
||||
|
||||
@@ -176,6 +176,7 @@ export default function EditorLayout() {
|
||||
activeNode,
|
||||
isPaid,
|
||||
isAdmiral,
|
||||
isAdmin,
|
||||
can,
|
||||
});
|
||||
|
||||
|
||||
@@ -21,6 +21,7 @@ interface UseSidebarContextMenuOptions {
|
||||
activeNode: Node | null | undefined;
|
||||
isPaid: boolean;
|
||||
isAdmiral: boolean;
|
||||
isAdmin: boolean;
|
||||
can: (action: PermissionAction, resourceType?: string, resourceId?: string) => boolean;
|
||||
}
|
||||
|
||||
@@ -32,6 +33,7 @@ export function useSidebarContextMenu({
|
||||
activeNode,
|
||||
isPaid,
|
||||
isAdmiral,
|
||||
isAdmin,
|
||||
can,
|
||||
}: UseSidebarContextMenuOptions) {
|
||||
const buildMenuCtx = useCallback((file: string): StackMenuCtx => {
|
||||
@@ -42,6 +44,7 @@ export function useSidebarContextMenu({
|
||||
isBusy: stackListState.isStackBusy(file),
|
||||
isPaid,
|
||||
isAdmiral,
|
||||
isAdmin,
|
||||
canDelete: can('stack:delete', 'stack', sName),
|
||||
canEditLabels: can('stack:edit', 'stack', sName),
|
||||
// POST /api/labels (the inline "New label" entry) is guarded by the
|
||||
@@ -121,9 +124,13 @@ export function useSidebarContextMenu({
|
||||
navState.setActiveView('scheduled-ops');
|
||||
},
|
||||
};
|
||||
// Handlers from useStackActions, useOverlayState, useViewNavigationState are
|
||||
// useCallback-stabilized at their owner hooks, so listing the menu surface
|
||||
// values (status maps, role/tier flags, pin state) is sufficient. Exhaustive
|
||||
// deps would force a rebuild on every parent render and defeat the memo.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [
|
||||
stackListState.stackStatuses, stackListState.stackPorts, isPaid, isAdmiral,
|
||||
stackListState.stackStatuses, stackListState.stackPorts, isPaid, isAdmiral, isAdmin,
|
||||
stackListState.isPinned, stackListState.labels, stackListState.stackLabelMap,
|
||||
stackListState.pin, stackListState.unpin,
|
||||
]);
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { useEffect, useRef, useState } from 'react';
|
||||
import { CommandInput } from '@/components/ui/command';
|
||||
|
||||
interface SidebarSearchProps {
|
||||
@@ -5,13 +6,44 @@ interface SidebarSearchProps {
|
||||
onValueChange: (v: string) => void;
|
||||
}
|
||||
|
||||
// 120ms feels instant to a typist (still under the ~150ms human reaction
|
||||
// floor) while collapsing a burst of keystrokes into one filter rebuild.
|
||||
// `<Command shouldFilter={false}>` means useStackListState owns the actual
|
||||
// filter pass; debouncing here directly cuts its rebuild count.
|
||||
const DEBOUNCE_MS = 120;
|
||||
|
||||
export function SidebarSearch({ value, onValueChange }: SidebarSearchProps) {
|
||||
const [local, setLocal] = useState(value);
|
||||
const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
const lastEmittedRef = useRef(value);
|
||||
|
||||
useEffect(() => {
|
||||
// Parent value can move for two reasons:
|
||||
// 1. Echo of our own debounced emit (lastEmittedRef matches): skip.
|
||||
// 2. External reset (e.g., clear-on-filter-change): adopt it.
|
||||
if (value === lastEmittedRef.current) return;
|
||||
setLocal(value);
|
||||
}, [value]);
|
||||
|
||||
useEffect(() => () => {
|
||||
if (timerRef.current) clearTimeout(timerRef.current);
|
||||
}, []);
|
||||
|
||||
const handleChange = (next: string) => {
|
||||
setLocal(next);
|
||||
if (timerRef.current) clearTimeout(timerRef.current);
|
||||
timerRef.current = setTimeout(() => {
|
||||
lastEmittedRef.current = next;
|
||||
onValueChange(next);
|
||||
}, DEBOUNCE_MS);
|
||||
};
|
||||
|
||||
return (
|
||||
<div className="px-4 py-2 flex-none">
|
||||
<CommandInput
|
||||
placeholder="Search stacks..."
|
||||
value={value}
|
||||
onValueChange={onValueChange}
|
||||
value={local}
|
||||
onValueChange={handleChange}
|
||||
className="h-9 border-none"
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { render, act, fireEvent, cleanup } from '@testing-library/react';
|
||||
import { Command } from '@/components/ui/command';
|
||||
import { SidebarSearch } from '../SidebarSearch';
|
||||
|
||||
function renderInsideCommand(props: { value: string; onValueChange: (v: string) => void }) {
|
||||
return render(
|
||||
<Command shouldFilter={false}>
|
||||
<SidebarSearch {...props} />
|
||||
</Command>,
|
||||
);
|
||||
}
|
||||
|
||||
describe('SidebarSearch', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
cleanup();
|
||||
});
|
||||
|
||||
it('reflects typing immediately in the input but does not emit until the debounce window closes', () => {
|
||||
const onValueChange = vi.fn();
|
||||
const { getByPlaceholderText } = renderInsideCommand({ value: '', onValueChange });
|
||||
const input = getByPlaceholderText('Search stacks...') as HTMLInputElement;
|
||||
|
||||
act(() => {
|
||||
fireEvent.input(input, { target: { value: 'n' } });
|
||||
fireEvent.input(input, { target: { value: 'ng' } });
|
||||
fireEvent.input(input, { target: { value: 'ngi' } });
|
||||
fireEvent.input(input, { target: { value: 'ngin' } });
|
||||
fireEvent.input(input, { target: { value: 'nginx' } });
|
||||
});
|
||||
|
||||
expect(input.value).toBe('nginx');
|
||||
expect(onValueChange).not.toHaveBeenCalled();
|
||||
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(120);
|
||||
});
|
||||
|
||||
expect(onValueChange).toHaveBeenCalledTimes(1);
|
||||
expect(onValueChange).toHaveBeenLastCalledWith('nginx');
|
||||
});
|
||||
|
||||
it('does not clobber in-flight typing when the parent value echoes back the previous debounced emit', () => {
|
||||
const onValueChange = vi.fn();
|
||||
const { getByPlaceholderText, rerender } = renderInsideCommand({ value: '', onValueChange });
|
||||
const input = getByPlaceholderText('Search stacks...') as HTMLInputElement;
|
||||
|
||||
act(() => {
|
||||
fireEvent.input(input, { target: { value: 'web' } });
|
||||
});
|
||||
act(() => {
|
||||
vi.advanceTimersByTime(120);
|
||||
});
|
||||
expect(onValueChange).toHaveBeenLastCalledWith('web');
|
||||
|
||||
// Parent now propagates 'web' back as the controlled value.
|
||||
rerender(
|
||||
<Command shouldFilter={false}>
|
||||
<SidebarSearch value="web" onValueChange={onValueChange} />
|
||||
</Command>,
|
||||
);
|
||||
|
||||
// User keeps typing before the parent's echo settles.
|
||||
act(() => {
|
||||
fireEvent.input(input, { target: { value: 'web-api' } });
|
||||
});
|
||||
|
||||
// The echo of 'web' must not overwrite 'web-api' on the input.
|
||||
expect(input.value).toBe('web-api');
|
||||
});
|
||||
|
||||
it('adopts an external reset of the parent value (e.g., clear)', () => {
|
||||
const onValueChange = vi.fn();
|
||||
const { getByPlaceholderText, rerender } = renderInsideCommand({ value: 'old-query', onValueChange });
|
||||
const input = getByPlaceholderText('Search stacks...') as HTMLInputElement;
|
||||
expect(input.value).toBe('old-query');
|
||||
|
||||
rerender(
|
||||
<Command shouldFilter={false}>
|
||||
<SidebarSearch value="" onValueChange={onValueChange} />
|
||||
</Command>,
|
||||
);
|
||||
|
||||
expect(input.value).toBe('');
|
||||
});
|
||||
});
|
||||
@@ -27,6 +27,7 @@ export interface StackMenuCtx {
|
||||
isBusy: boolean;
|
||||
isPaid: boolean;
|
||||
isAdmiral: boolean;
|
||||
isAdmin: boolean;
|
||||
canDelete: boolean;
|
||||
canEditLabels: boolean;
|
||||
canCreateLabels: boolean;
|
||||
|
||||
@@ -11,6 +11,7 @@ function makeCtx(overrides: Partial<StackMenuCtx> = {}): StackMenuCtx {
|
||||
isBusy: false,
|
||||
isPaid: true,
|
||||
isAdmiral: false,
|
||||
isAdmin: true,
|
||||
canDelete: true,
|
||||
canEditLabels: true,
|
||||
canCreateLabels: true,
|
||||
@@ -94,6 +95,12 @@ describe('useStackMenuItems', () => {
|
||||
expect(lifecycle.items.some(i => i.id === 'schedule')).toBe(true);
|
||||
});
|
||||
|
||||
it('hides Schedule task when paid but not admin', () => {
|
||||
const { result } = renderHook(() => useStackMenuItems('web.yml', makeCtx({ isPaid: true, isAdmin: false })));
|
||||
const lifecycle = result.current.find(g => g.id === 'lifecycle');
|
||||
expect(lifecycle?.items.some(i => i.id === 'schedule')).toBeFalsy();
|
||||
});
|
||||
|
||||
it('keeps label assignment available when !isPaid', () => {
|
||||
const { result } = renderHook(() => useStackMenuItems('web.yml', makeCtx({
|
||||
isPaid: false,
|
||||
|
||||
@@ -18,7 +18,7 @@ import type { MenuGroup, MenuItem, StackMenuCtx } from '@/components/sidebar/sid
|
||||
|
||||
export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[] {
|
||||
const {
|
||||
stackStatus, hasPort, isBusy, isPaid, canDelete, canEditLabels, isPinned, labels,
|
||||
stackStatus, hasPort, isBusy, isPaid, isAdmin, canDelete, canEditLabels, isPinned, labels,
|
||||
openAlertSheet, openAutoHeal, checkUpdates, openStackApp,
|
||||
deploy, stop, restart, update, remove, pin, unpin, toggleLabel,
|
||||
menuVisibility, openScheduleTask,
|
||||
@@ -68,7 +68,7 @@ export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[]
|
||||
if (showStop) lifecycle.push({ id: 'stop', label: 'Stop', icon: Square, shortcut: '⌘.', onSelect: stop, disabled: isBusy });
|
||||
if (showRestart) lifecycle.push({ id: 'restart', label: 'Restart', icon: RotateCw, shortcut: '⌘R', onSelect: restart, disabled: isBusy });
|
||||
if (showUpdate) lifecycle.push({ id: 'update', label: 'Update', icon: Download, shortcut: '⌘↑', onSelect: update, disabled: isBusy });
|
||||
if (isPaid) lifecycle.push({ id: 'schedule', label: 'Schedule task', icon: CalendarClock, onSelect: openScheduleTask });
|
||||
if (isPaid && isAdmin) lifecycle.push({ id: 'schedule', label: 'Schedule task', icon: CalendarClock, onSelect: openScheduleTask });
|
||||
if (lifecycle.length > 0) groups.push({ id: 'lifecycle', items: lifecycle });
|
||||
|
||||
if (canDelete) {
|
||||
@@ -80,7 +80,7 @@ export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[]
|
||||
|
||||
return groups;
|
||||
}, [
|
||||
stackStatus, hasPort, isBusy, isPaid, canDelete, canEditLabels, isPinned, labels,
|
||||
stackStatus, hasPort, isBusy, isPaid, isAdmin, canDelete, canEditLabels, isPinned, labels,
|
||||
showDeploy, showStop, showRestart, showUpdate,
|
||||
openAlertSheet, openAutoHeal, checkUpdates, openStackApp,
|
||||
deploy, stop, restart, update, remove, pin, unpin, toggleLabel, openScheduleTask,
|
||||
|
||||
Reference in New Issue
Block a user