gsadmin/sencho
1
0
Fork 0
mirror of https://github.com/Studio-Saelix/sencho.git synced 2026-07-26 11:49:16 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(labels): harden Stack Labels (gate parity, abort, dry-run, cap) (#1232)

Browse Source 
* feat(labels): harden stack-labels surface (gate parity, abort, dry-run policy, cap)

- Cap labelIds array at MAX_LABELS_PER_NODE on PUT /api/stacks/:name/labels.
- Hide sidebar Labels submenu and Settings mutation buttons for roles
  without stack:edit, matching the backend requirePermission gate.
- Break the per-label bulk-action loop on req.aborted so cancelled requests
  release the per-node lock once the in-flight op completes.
- Reset saving state on success in LabelInlineCreateForm so the form stays
  interactive when reused outside the kebab/context menus.
- Invoke enforcePolicyPreDeploy inside the dry-run deploy branch and report
  blocked stacks honestly; previously dry-run skipped the gate and would
  predict success for stacks the real deploy would block.

* fix(labels): split sidebar gate so inline create matches unscoped backend guard

Codex review of #1232 surfaced a parity miss: POST /api/labels is guarded
by unscoped requirePermission('stack:edit'), but the sidebar inline "New
label" entry was gated by canEditLabels (per-stack scoped). An Admiral
user with only scoped grants on a stack could toggle existing labels but
the inline create request would 403.

Splits the sidebar gate:
- canEditLabels (scoped) keeps gating the Labels submenu trigger and toggle
  items, matching PUT /api/stacks/:name/labels.
- canCreateLabels (unscoped) now gates the inline "New label" entry,
  matching POST /api/labels.

Also restores the swallow-catch in LabelInlineCreateForm. The earlier
catch-to-finally change in #1232 let the rethrow from createAndAssignLabel
surface as an unhandled event-handler rejection in the browser console.
Parents already toast on failure; swallowing in the form is the intended
behavior with the finally reset still in place.
This commit is contained in:
Anso
2026-05-25 23:48:12 -04:00
committed by GitHub
parent 42e8d3a78c
commit 2a29fed117
11 changed files with 219 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/__tests__/labels-bulk-actions.test.ts
+62 -1
View File
@@ -65,6 +65,10 @@ afterAll(() => cleanupTestDb(tmpDir));
beforeEach(() => {
vi.restoreAllMocks();
// restoreAllMocks only resets spies; bare vi.fn() mocks keep their call
// history across tests. Clear them all so each test sees a fresh slate
// before its `.not.toHaveBeenCalled()` assertions run.
vi.clearAllMocks();
vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue('paid');
mockFsStacks = ['alpha', 'beta'];
deployStack.mockResolvedValue(undefined);
@@ -72,7 +76,6 @@ beforeEach(() => {
stopContainer.mockResolvedValue(undefined);
restartContainer.mockResolvedValue(undefined);
enforcePolicyPreDeploy.mockResolvedValue({ ok: true });
invalidateNodeCaches.mockClear();
activeBulkActions.clear();
db.getDb().prepare('DELETE FROM stack_label_assignments').run();
db.getDb().prepare('DELETE FROM stack_labels').run();
@@ -146,4 +149,62 @@ describe('Stack Labels bulk actions', () => {
expect(res.body.error).toContain('already running');
expect(restartContainer).not.toHaveBeenCalled();
});
it('dry-run deploy runs the policy gate and reports blocked stacks honestly', async () => {
const label = await createAssignedLabel(['alpha']);
enforcePolicyPreDeploy.mockResolvedValue({
ok: false,
policy: { name: 'block-criticals', max_severity: 'high' },
violations: [{ image: 'nginx:latest', severity: 'critical' }],
});
const res = await request(app)
.post(`/api/labels/${label.id}/action`)
.set('Authorization', authHeader)
.send({ action: 'deploy', dryRun: true });
expect(res.status).toBe(200);
expect(res.body.results).toEqual([
expect.objectContaining({ stackName: 'alpha', success: false, dryRun: true }),
]);
expect(res.body.results[0].error).toContain('Policy "block-criticals" blocked deploy');
expect(deployStack).not.toHaveBeenCalled();
expect(invalidateNodeCaches).not.toHaveBeenCalled();
});
it('dry-run deploy reports success when the policy gate passes, without touching Docker', async () => {
const label = await createAssignedLabel(['alpha']);
enforcePolicyPreDeploy.mockResolvedValue({ ok: true });
const res = await request(app)
.post(`/api/labels/${label.id}/action`)
.set('Authorization', authHeader)
.send({ action: 'deploy', dryRun: true });
expect(res.status).toBe(200);
expect(res.body.results).toEqual([
{ stackName: 'alpha', success: true, dryRun: true },
]);
expect(enforcePolicyPreDeploy).toHaveBeenCalledWith('alpha', label.node_id, expect.any(Object));
expect(deployStack).not.toHaveBeenCalled();
expect(invalidateNodeCaches).not.toHaveBeenCalled();
});
it('dry-run stop reports per-stack success without dispatching real stops', async () => {
const label = await createAssignedLabel(['alpha', 'beta']);
const res = await request(app)
.post(`/api/labels/${label.id}/action`)
.set('Authorization', authHeader)
.send({ action: 'stop', dryRun: true });
expect(res.status).toBe(200);
expect(res.body.results).toEqual([
{ stackName: 'alpha', success: true, dryRun: true },
{ stackName: 'beta', success: true, dryRun: true },
]);
expect(getContainersByStack).not.toHaveBeenCalled();
expect(stopContainer).not.toHaveBeenCalled();
expect(invalidateNodeCaches).not.toHaveBeenCalled();
});
});
backend/src/__tests__/labels-community-tier.test.ts
+42
View File
@@ -34,6 +34,14 @@ beforeAll(async () => {
afterAll(() => cleanupTestDb(tmpDir));
// Tests in this file accumulate labels in the shared DB; clear them between
// runs so later tests do not bump into MAX_LABELS_PER_NODE.
afterEach(() => {
const db = DatabaseService.getInstance().getDb();
db.prepare('DELETE FROM stack_label_assignments').run();
db.prepare('DELETE FROM stack_labels').run();
});
function mockTier(tier: 'paid' | 'community') {
vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValue(tier);
}
@@ -126,6 +134,40 @@ describe('Stack Labels on Community tier', () => {
expect(res.status).toBe(400);
expect(res.body.error).toBe('Invalid stack name');
});
it('rejects labelIds arrays over the per-node cap', async () => {
mockTier('community');
const oversized = Array.from({ length: 51 }, (_, i) => i + 1);
const res = await request(app)
.put('/api/stacks/cap-stack/labels')
.set('Authorization', authHeader)
.send({ labelIds: oversized });
expect(res.status).toBe(400);
expect(res.body.error).toContain('may not exceed');
});
it('accepts labelIds at exactly the per-node cap', async () => {
mockTier('community');
// Seed 50 labels directly so the FK check on assignment passes without
// running 50 HTTP round trips per test. The route resolves nodeId via
// nodeContextMiddleware (default node when no x-node-id header), so the
// seeded rows must use the same nodeId the request will look up.
const { NodeRegistry } = await import('../services/NodeRegistry');
const nodeId = NodeRegistry.getInstance().getDefaultNodeId();
const db = DatabaseService.getInstance().getDb();
const insert = db.prepare('INSERT INTO stack_labels (node_id, name, color) VALUES (?, ?, ?)');
const ids: number[] = [];
for (let i = 0; i < 50; i++) {
const r = insert.run(nodeId, `cap-edge-${i}`, 'teal');
ids.push(r.lastInsertRowid as number);
}
const res = await request(app)
.put('/api/stacks/cap-edge-stack/labels')
.set('Authorization', authHeader)
.send({ labelIds: ids });
expect(res.status).toBe(200);
expect(res.body.success).toBe(true);
});
});
describe('Stack Labels RBAC', () => {
backend/src/routes/labels.ts
+38 -8
View File
@@ -206,14 +206,21 @@ labelsRouter.post('/:id/action', authMiddleware, async (req: Request, res: Respo
const results: { stackName: string; success: boolean; error?: string; dryRun?: boolean }[] = [];
for (const stackName of validStacks) {
if (isDryRun) {
// Rehearse the action under the same lock + label resolution + fs
// intersection. Skip the destructive leaf call.
results.push({ stackName, success: true, dryRun: true });
continue;
// Client disconnected mid-bulk: stop dispatching new per-stack ops.
// The currently in-flight call still runs to completion; the outer
// finally releases the lock when it returns. Stays on `req.aborted`
// rather than `req.destroyed` because supertest's in-process server
// mode flips `destroyed` between handler and response write, which
// would cause every bulk-action test to look like a client abort.
if (req.aborted) {
if (isDebugEnabled()) console.debug('[Labels:debug] Bulk action aborted by client at stack:', stackName);
break;
}
try {
if (action === 'deploy') {
// Policy gate runs for both real and dry-run deploys: a dry-run
// that omits the policy check would falsely report success for
// stacks the real deploy would block.
const gate = await enforcePolicyPreDeploy(
stackName,
req.nodeId,
@@ -221,11 +228,21 @@ labelsRouter.post('/:id/action', authMiddleware, async (req: Request, res: Respo
);
if (!gate.ok) {
const blockedMsg = `Policy "${gate.policy?.name}" blocked deploy: ${gate.violations.length} image(s) exceed ${gate.policy?.max_severity}`;
results.push({ stackName, success: false, error: blockedMsg });
results.push({ stackName, success: false, error: blockedMsg, ...(isDryRun ? { dryRun: true } : {}) });
continue;
}
if (isDryRun) {
results.push({ stackName, success: true, dryRun: true });
continue;
}
await ComposeService.getInstance(req.nodeId).deployStack(stackName, undefined, false);
} else {
// stop / restart have no pre-action policy gate; dry-run just
// confirms the stack would be reached.
if (isDryRun) {
results.push({ stackName, success: true, dryRun: true });
continue;
}
const dockerController = DockerController.getInstance(req.nodeId);
const containers = await dockerController.getContainersByStack(stackName);
if (action === 'stop') {
@@ -242,12 +259,18 @@ labelsRouter.post('/:id/action', authMiddleware, async (req: Request, res: Respo
const succeeded = results.filter(r => r.success).length;
const failed = results.length - succeeded;
console.log(`[Labels] Bulk ${sanitizeForLog(action)}${isDryRun ? ' (dry run)' : ''} on label ${id}: ${validStacks.length} stacks (${succeeded} succeeded, ${failed} failed)`);
if (isDebugEnabled()) console.debug('[Labels:debug] Bulk action complete:', { id, action, total: results.length, succeeded, failed, dryRun: isDryRun });
// Two-axis truncation reporting: `results.length` is what we actually
// processed; `validStacks.length` is what we set out to process. They
// differ when the client aborted mid-loop.
console.log(`[Labels] Bulk ${sanitizeForLog(action)}${isDryRun ? ' (dry run)' : ''} on label ${id}: ${results.length}/${validStacks.length} stacks (${succeeded} succeeded, ${failed} failed)`);
if (isDebugEnabled()) console.debug('[Labels:debug] Bulk action complete:', { id, action, processed: results.length, total: validStacks.length, succeeded, failed, dryRun: isDryRun });
if (succeeded > 0 && !isDryRun) {
invalidateNodeCaches(req.nodeId);
}
// Writing the response is harmless even if the client already
// disconnected; Node swallows the EPIPE / write-after-end. The
// lock release in the outer finally still happens.
res.json({ results });
} finally {
activeBulkActions.delete(lockKey);
@@ -281,6 +304,13 @@ stackLabelsRouter.put('/:stackName/labels', authMiddleware, async (req: Request,
res.status(400).json({ error: 'labelIds must be an array of numbers' });
return;
}
// A node can hold at most MAX_LABELS_PER_NODE labels, so any stack
// assignment over that count is either an authenticated client mistake
// or a deliberate transaction-bloat attempt. Reject before the DB sees it.
if (labelIds.length > MAX_LABELS_PER_NODE) {
res.status(400).json({ error: `labelIds may not exceed ${MAX_LABELS_PER_NODE} entries` });
return;
}
if (isDebugEnabled()) console.debug('[Labels:debug] Set stack labels:', { stackName, nodeId, labelIds });
DatabaseService.getInstance().setStackLabels(stackName, nodeId, labelIds);
frontend/src/components/EditorLayout/hooks/useSidebarContextMenu.ts
+5
View File
@@ -43,6 +43,11 @@ export function useSidebarContextMenu({
isPaid,
isAdmiral,
canDelete: can('stack:delete', 'stack', sName),
canEditLabels: can('stack:edit', 'stack', sName),
// 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.
canCreateLabels: can('stack:edit'),
isPinned: stackListState.isPinned(file),
labels: stackListState.labels,
assignedLabelIds: (stackListState.stackLabelMap[file] ?? []).map(l => l.id),
frontend/src/components/settings/LabelsSection.tsx
+34 -22
View File
@@ -6,6 +6,7 @@ import { Modal, ModalHeader, ModalBody, ModalFooter, ConfirmModal } from '@/comp
import { apiFetch } from '@/lib/api';
import { toast } from '@/components/ui/toast-store';
import { SENCHO_LABELS_CHANGED } from '@/lib/events';
import { useAuth } from '@/context/AuthContext';
import { CapabilityGate } from '../CapabilityGate';
import { LabelDot } from '../LabelPill';
import { LABEL_COLORS, MAX_LABELS_PER_NODE, type Label, type LabelColor } from '../label-types';
@@ -18,6 +19,11 @@ interface LabelsSectionProps {
}
export function LabelsSection({ onLabelsChanged }: LabelsSectionProps = {}) {
const { can } = useAuth();
// Mirrors the backend `requirePermission('stack:edit')` guard on
// POST/PUT/DELETE /api/labels, keeping a viewer/deployer/auditor from
// clicking through to a guaranteed 403.
const canMutate = can('stack:edit');
const [labels, setLabels] = useState<Label[]>([]);
const [loading, setLoading] = useState(true);
const [assignmentCounts, setAssignmentCounts] = useState<Record<number, number>>({});
@@ -128,12 +134,14 @@ export function LabelsSection({ onLabelsChanged }: LabelsSectionProps = {}) {
return (
<CapabilityGate capability="labels" featureName="Stack Labels">
<div className="space-y-4">
<div className="flex justify-end">
<SettingsPrimaryButton size="sm" onClick={openCreate} disabled={labels.length >= MAX_LABELS_PER_NODE}>
<Plus className="w-4 h-4" strokeWidth={1.5} />
{labels.length >= MAX_LABELS_PER_NODE ? 'Limit reached' : 'New label'}
</SettingsPrimaryButton>
</div>
{canMutate && (
<div className="flex justify-end">
<SettingsPrimaryButton size="sm" onClick={openCreate} disabled={labels.length >= MAX_LABELS_PER_NODE}>
<Plus className="w-4 h-4" strokeWidth={1.5} />
{labels.length >= MAX_LABELS_PER_NODE ? 'Limit reached' : 'New label'}
</SettingsPrimaryButton>
</div>
)}
<div className="rounded-lg border border-card-border border-t-card-border-top bg-card shadow-card-bevel">
{loading ? (
@@ -153,22 +161,26 @@ export function LabelsSection({ onLabelsChanged }: LabelsSectionProps = {}) {
<span className="text-xs text-muted-foreground tabular-nums">
{assignmentCounts[label.id] || 0} stack{(assignmentCounts[label.id] || 0) !== 1 ? 's' : ''}
</span>
<Button
variant="ghost"
size="icon"
className="h-7 w-7 opacity-0 group-hover:opacity-100 transition-opacity"
onClick={() => openEdit(label)}
>
<Pencil className="w-3.5 h-3.5" strokeWidth={1.5} />
</Button>
<Button
variant="ghost"
size="icon"
className="h-7 w-7 text-destructive/60 hover:bg-destructive hover:text-destructive-foreground opacity-0 group-hover:opacity-100 transition-opacity"
onClick={() => setDeleteTarget(label)}
>
<Trash2 className="w-3.5 h-3.5" strokeWidth={1.5} />
</Button>
{canMutate && (
<>
<Button
variant="ghost"
size="icon"
className="h-7 w-7 opacity-0 group-hover:opacity-100 transition-opacity"
onClick={() => openEdit(label)}
>
<Pencil className="w-3.5 h-3.5" strokeWidth={1.5} />
</Button>
<Button
variant="ghost"
size="icon"
className="h-7 w-7 text-destructive/60 hover:bg-destructive hover:text-destructive-foreground opacity-0 group-hover:opacity-100 transition-opacity"
onClick={() => setDeleteTarget(label)}
>
<Trash2 className="w-3.5 h-3.5" strokeWidth={1.5} />
</Button>
</>
)}
</div>
))}
</div>
frontend/src/components/sidebar/LabelInlineCreateForm.tsx
+6
View File
@@ -20,6 +20,12 @@ export function LabelInlineCreateForm({ onSubmit, onCancel }: LabelInlineCreateF
try {
await onSubmit(trimmed, color);
} catch {
// Parents (createAndAssignLabel) toast and rethrow to signal failure;
// swallow here so the rejection does not surface as an unhandled
// event-handler rejection in the browser console.
} finally {
// Reset even on success so the form stays interactive if the parent
// keeps it mounted (e.g. when reused outside the kebab/context menu).
setSaving(false);
}
};
frontend/src/components/sidebar/StackContextMenu.tsx
+1 -1
View File
@@ -63,7 +63,7 @@ function LabelsSub({ item, ctx }: { item: MenuItem; ctx: StackMenuCtx }) {
);
})}
<ContextMenuSeparator />
{ctx.labels.length < MAX_LABELS_PER_NODE && (
{ctx.canCreateLabels && ctx.labels.length < MAX_LABELS_PER_NODE && (
<ContextMenuItem onSelect={e => { e.preventDefault(); setCreating(true); }}>
<Plus className="w-3.5 h-3.5 mr-2 text-muted-foreground" strokeWidth={1.5} />
<span className="text-xs">New label</span>
frontend/src/components/sidebar/StackKebabMenu.tsx
+1 -1
View File
@@ -63,7 +63,7 @@ function LabelsSub({ item, ctx }: { item: MenuItem; ctx: StackMenuCtx }) {
);
})}
<DropdownMenuSeparator />
{ctx.labels.length < MAX_LABELS_PER_NODE && (
{ctx.canCreateLabels && ctx.labels.length < MAX_LABELS_PER_NODE && (
<DropdownMenuItem onSelect={e => { e.preventDefault(); setCreating(true); }}>
<Plus className="w-3.5 h-3.5 mr-2 text-muted-foreground" strokeWidth={1.5} />
<span className="text-xs">New label</span>
frontend/src/components/sidebar/sidebar-types.ts
+2
View File
@@ -28,6 +28,8 @@ export interface StackMenuCtx {
isPaid: boolean;
isAdmiral: boolean;
canDelete: boolean;
canEditLabels: boolean;
canCreateLabels: boolean;
isPinned: boolean;
labels: Label[];
assignedLabelIds: number[];
frontend/src/hooks/__tests__/useStackMenuItems.test.tsx
+12
View File
@@ -12,6 +12,8 @@ function makeCtx(overrides: Partial<StackMenuCtx> = {}): StackMenuCtx {
isPaid: true,
isAdmiral: false,
canDelete: true,
canEditLabels: true,
canCreateLabels: true,
isPinned: false,
labels: [],
assignedLabelIds: [],
@@ -106,6 +108,16 @@ describe('useStackMenuItems', () => {
expect(labelsItem?.subItems?.[0]).toMatchObject({ id: 'label:1', label: 'prod' });
});
it('hides the Labels submenu when !canEditLabels (viewer role)', () => {
const { result } = renderHook(() => useStackMenuItems('web.yml', makeCtx({
canEditLabels: false,
labels: [{ id: 1, node_id: 0, name: 'prod', color: 'teal' }],
})));
const organize = result.current.find(g => g.id === 'organize')!;
expect(organize.items.find(i => i.id === 'labels')).toBeUndefined();
expect(organize.items.find(i => i.id === 'pin')).toBeDefined();
});
it('auto-update toggle calls setAutoUpdateEnabled with toggled value', () => {
const setAutoUpdateEnabled = vi.fn();
const { result } = renderHook(() =>
frontend/src/hooks/useStackMenuItems.tsx
+16 -14
View File
@@ -19,7 +19,7 @@ import type { MenuGroup, MenuItem, StackMenuCtx } from '@/components/sidebar/sid
export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[] {
const {
stackStatus, hasPort, isBusy, isPaid, canDelete, isPinned, labels,
stackStatus, hasPort, isBusy, isPaid, canDelete, canEditLabels, isPinned, labels,
openAlertSheet, openAutoHeal, checkUpdates, openStackApp,
deploy, stop, restart, update, remove, pin, unpin, toggleLabel,
menuVisibility, autoUpdateEnabled, setAutoUpdateEnabled, openScheduleTask,
@@ -48,19 +48,21 @@ export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[]
groups.push({ id: 'inspect', items: inspect });
const organize: MenuItem[] = [];
organize.push({
id: 'labels',
label: 'Labels',
icon: Tag,
shortcut: 'L ›',
onSelect: () => {},
subItems: labels.map(l => ({
id: `label:${l.id}`,
label: l.name,
if (canEditLabels) {
organize.push({
id: 'labels',
label: 'Labels',
icon: Tag,
onSelect: () => toggleLabel(l.id),
})),
});
shortcut: 'L ›',
onSelect: () => {},
subItems: labels.map(l => ({
id: `label:${l.id}`,
label: l.name,
icon: Tag,
onSelect: () => toggleLabel(l.id),
})),
});
}
organize.push(
isPinned
? { id: 'pin', label: 'Unpin', icon: PinOff, shortcut: 'P', onSelect: unpin }
@@ -85,7 +87,7 @@ export function useStackMenuItems(_file: string, ctx: StackMenuCtx): MenuGroup[]
return groups;
}, [
stackStatus, hasPort, isBusy, isPaid, canDelete, isPinned, labels,
stackStatus, hasPort, isBusy, isPaid, canDelete, canEditLabels, isPinned, labels,
showDeploy, showStop, showRestart, showUpdate,
autoUpdateEnabled, setAutoUpdateEnabled,
openAlertSheet, openAutoHeal, checkUpdates, openStackApp,