diff --git a/backend/src/__tests__/blueprints.test.ts b/backend/src/__tests__/blueprints.test.ts index dffbafc8..0dd148dc 100644 --- a/backend/src/__tests__/blueprints.test.ts +++ b/backend/src/__tests__/blueprints.test.ts @@ -513,18 +513,19 @@ describe('BlueprintService local withdraw clears stack-scoped role assignments', ); const { ComposeService } = await import('../services/ComposeService'); const { FileSystemService } = await import('../services/FileSystemService'); - vi.spyOn(ComposeService.prototype, 'downStack').mockResolvedValue(undefined); + const downStackSpy = vi.spyOn(ComposeService.prototype, 'downStack').mockResolvedValue(undefined); const deleteStackSpy = vi.spyOn(FileSystemService.prototype, 'deleteStack').mockResolvedValue(undefined); - return { bp, node, nodeId, userId, deleteStackSpy, db }; + return { bp, node, nodeId, userId, downStackSpy, deleteStackSpy, db }; } it('clears the target assignment after successful filesystem deletion and preserves unrelated grants', async () => { - const { bp, node, userId, deleteStackSpy, db } = await arrangeLocalWithdraw(); + const { bp, node, userId, downStackSpy, deleteStackSpy, db } = await arrangeLocalWithdraw(); const outcome = await BlueprintService.getInstance().withdrawFromNode(bp, node); expect(outcome.status).toBe('withdrawn'); + expect(downStackSpy).toHaveBeenCalledWith(bp.name, { removeVolumes: false }); expect(deleteStackSpy).toHaveBeenCalledWith(bp.name); expect(db.getDeployment(bp.id, node.id)).toBeUndefined(); expect(hasAssignment(userId, 'stack', bp.name)).toBe(false); diff --git a/backend/src/__tests__/compose-service.test.ts b/backend/src/__tests__/compose-service.test.ts index c71880db..661f6afc 100644 --- a/backend/src/__tests__/compose-service.test.ts +++ b/backend/src/__tests__/compose-service.test.ts @@ -1328,7 +1328,7 @@ describe('ComposeService - withRegistryAuth', () => { }); }); -// ── downStack ────────────────────────────────────────────────────────── +// ── runDown ──────────────────────────────────────────────────────────── describe('ComposeService - runDown', () => { it('runs plain docker compose down by default', async () => { @@ -1374,7 +1374,33 @@ describe('ComposeService - runDown', () => { // ── downStack ────────────────────────────────────────────────────────── describe('ComposeService - downStack', () => { - it('runs docker compose down with volumes and remove-orphans', async () => { + it('runs docker compose down with volumes and remove-orphans when removeVolumes is true', async () => { + setupAutoCloseSpawn(); + + const svc = ComposeService.getInstance(1); + await svc.downStack('my-stack', { removeVolumes: true }); + + expect(mockSpawn).toHaveBeenCalledWith( + 'docker', + ['compose', 'down', '--volumes', '--remove-orphans'], + expect.any(Object) + ); + }); + + it('runs docker compose down without --volumes when removeVolumes is false', async () => { + setupAutoCloseSpawn(); + + const svc = ComposeService.getInstance(1); + await svc.downStack('my-stack', { removeVolumes: false }); + + expect(mockSpawn).toHaveBeenCalledWith( + 'docker', + ['compose', 'down', '--remove-orphans'], + expect.any(Object) + ); + }); + + it('defaults to no --volumes when options are omitted (safe default)', async () => { setupAutoCloseSpawn(); const svc = ComposeService.getInstance(1); @@ -1382,7 +1408,7 @@ describe('ComposeService - downStack', () => { expect(mockSpawn).toHaveBeenCalledWith( 'docker', - ['compose', 'down', '--volumes', '--remove-orphans'], + ['compose', 'down', '--remove-orphans'], expect.any(Object) ); }); diff --git a/backend/src/__tests__/proxy-scoped-stack-evidence.test.ts b/backend/src/__tests__/proxy-scoped-stack-evidence.test.ts index 06d3dc27..34788807 100644 --- a/backend/src/__tests__/proxy-scoped-stack-evidence.test.ts +++ b/backend/src/__tests__/proxy-scoped-stack-evidence.test.ts @@ -52,7 +52,7 @@ function evidenceRemote(): http.Server { res.writeHead(200, { 'Content-Type': 'application/json' }); res.end(JSON.stringify({ version: '0.93.0', - capabilities: ['cross-node-rbac', 'scoped-stack-auth-evidence'], + capabilities: ['cross-node-rbac', 'scoped-stack-auth-evidence', 'stack-delete-prune-volumes'], })); return; } @@ -88,7 +88,7 @@ function failDeleteRemote(): http.Server { res.writeHead(200, { 'Content-Type': 'application/json' }); res.end(JSON.stringify({ version: '0.93.0', - capabilities: ['cross-node-rbac', 'scoped-stack-auth-evidence'], + capabilities: ['cross-node-rbac', 'scoped-stack-auth-evidence', 'stack-delete-prune-volumes'], })); return; } diff --git a/backend/src/__tests__/proxy-stack-delete-volume-gate.test.ts b/backend/src/__tests__/proxy-stack-delete-volume-gate.test.ts new file mode 100644 index 00000000..bd57fed8 --- /dev/null +++ b/backend/src/__tests__/proxy-stack-delete-volume-gate.test.ts @@ -0,0 +1,150 @@ +/** + * Gateway preflight for DELETE /stacks/:name on remote nodes. See + * isUnacknowledgedStackDelete in remoteNodeProxy.ts for why this gate exists; this file + * covers what the gate must get right: block an unacknowledged delete to an unsupported + * remote (whatever the caller's role, and with or without a trailing slash), accept only + * the exact string "true" as acknowledgement, and forward without a capability probe once + * removal is acknowledged or the remote advertises support. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import http from 'http'; +import bcrypt from 'bcrypt'; +import request from 'supertest'; +import jwt from 'jsonwebtoken'; +import { setupTestDb, cleanupTestDb, TEST_USERNAME, TEST_JWT_SECRET } from './helpers/setupTestDb'; + +let tmpDir: string; +let app: import('express').Express; +let viewerBearer: string; +let adminBearer: string; +let capServer: http.Server; +let noCapServer: http.Server; +let capNodeId: number; +let noCapNodeId: number; + +const noCapPaths: string[] = []; +const capPaths: string[] = []; + +function metaServer(capabilities: string[], seen: string[]): http.Server { + return http.createServer((req, res) => { + if (req.url) seen.push(req.url); + res.writeHead(200, { 'Content-Type': 'application/json' }); + if (req.url?.startsWith('/api/meta')) { + res.end(JSON.stringify({ version: '0.93.0', capabilities })); + } else { + res.end(JSON.stringify({ status: 'deleted' })); + } + }); +} + +async function listen(server: http.Server): Promise { + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + return (server.address() as import('net').AddressInfo).port; +} + +beforeAll(async () => { + tmpDir = await setupTestDb(); + ({ app } = await import('../index')); + const { DatabaseService } = await import('../services/DatabaseService'); + const db = DatabaseService.getInstance(); + + const hash = await bcrypt.hash('password123', 1); + db.addUser({ username: 'delvol-viewer', password_hash: hash, role: 'viewer' }); + const viewer = db.getUserByUsername('delvol-viewer')!; + viewerBearer = jwt.sign({ username: 'delvol-viewer', role: 'viewer', tv: viewer.token_version }, TEST_JWT_SECRET, { expiresIn: '1m' }); + adminBearer = jwt.sign({ username: TEST_USERNAME }, TEST_JWT_SECRET, { expiresIn: '1m' }); + + capServer = metaServer(['cross-node-rbac', 'stack-delete-prune-volumes'], capPaths); + noCapServer = metaServer(['cross-node-rbac', 'stack-down-remove-volumes'], noCapPaths); + const capPort = await listen(capServer); + const noCapPort = await listen(noCapServer); + + capNodeId = db.addNode({ + name: 'delvol-cap-remote', type: 'remote', mode: 'proxy', compose_dir: '/tmp', + is_default: false, api_url: `http://127.0.0.1:${capPort}`, api_token: 'cap-token', + }); + noCapNodeId = db.addNode({ + name: 'delvol-nocap-remote', type: 'remote', mode: 'proxy', compose_dir: '/tmp', + is_default: false, api_url: `http://127.0.0.1:${noCapPort}`, api_token: 'nocap-token', + }); +}); + +afterAll(async () => { + await new Promise((resolve) => capServer.close(() => resolve())); + await new Promise((resolve) => noCapServer.close(() => resolve())); + cleanupTestDb(tmpDir); +}); + +describe('remote proxy stack-delete volume gate', () => { + it('returns 400 for an unacknowledged delete when remote lacks stack-delete-prune-volumes (admin)', async () => { + noCapPaths.length = 0; + const res = await request(app) + .delete('/api/stacks/web') + .set('Authorization', `Bearer ${adminBearer}`) + .set('x-node-id', String(noCapNodeId)); + + expect(res.status).toBe(400); + expect(res.body.code).toBe('capability_unavailable'); + expect(noCapPaths.some(p => p.includes('/api/stacks/web') && !p.startsWith('/api/meta'))).toBe(false); + expect(noCapPaths.some(p => p.startsWith('/api/meta'))).toBe(true); + }); + + // A viewer has no stack:delete, so this also pins gate order: the data-safety 400 wins + // over the permission 403 that the named-stack pre-check would otherwise return. + it('returns 400, not 403, for an unacknowledged delete by a viewer when remote lacks capability', async () => { + noCapPaths.length = 0; + const res = await request(app) + .delete('/api/stacks/web') + .set('Authorization', `Bearer ${viewerBearer}`) + .set('x-node-id', String(noCapNodeId)); + + expect(res.status).toBe(400); + expect(res.body.code).toBe('capability_unavailable'); + }); + + it('proxies through when the operator explicitly acknowledges volume removal (pruneVolumes=true), even without the capability, and never queries /api/meta', async () => { + noCapPaths.length = 0; + const res = await request(app) + .delete('/api/stacks/web?pruneVolumes=true') + .set('Authorization', `Bearer ${adminBearer}`) + .set('x-node-id', String(noCapNodeId)); + + expect(res.status).toBe(200); + expect(noCapPaths.some(p => p.includes('/api/stacks/web?pruneVolumes=true'))).toBe(true); + expect(noCapPaths.some(p => p.startsWith('/api/meta'))).toBe(false); + }); + + it('treats only the exact string "true" as acknowledgement, blocking loose truthy values like "1"', async () => { + noCapPaths.length = 0; + const res = await request(app) + .delete('/api/stacks/web?pruneVolumes=1') + .set('Authorization', `Bearer ${adminBearer}`) + .set('x-node-id', String(noCapNodeId)); + + expect(res.status).toBe(400); + expect(res.body.code).toBe('capability_unavailable'); + }); + + it('proxies an unacknowledged delete through when the remote advertises stack-delete-prune-volumes', async () => { + capPaths.length = 0; + const res = await request(app) + .delete('/api/stacks/web') + .set('Authorization', `Bearer ${adminBearer}`) + .set('x-node-id', String(capNodeId)); + + expect(res.status).toBe(200); + expect(capPaths.some(p => p.includes('/api/stacks/web'))).toBe(true); + }); + + it('still gates an unacknowledged delete with a trailing slash', async () => { + noCapPaths.length = 0; + const res = await request(app) + .delete('/api/stacks/web/') + .set('Authorization', `Bearer ${adminBearer}`) + .set('x-node-id', String(noCapNodeId)); + + expect(res.status).toBe(400); + expect(res.body.code).toBe('capability_unavailable'); + expect(noCapPaths.some(p => p.includes('/api/stacks/web/') && !p.startsWith('/api/meta'))).toBe(false); + }); +}); diff --git a/backend/src/__tests__/stack-delete-purges-scans.test.ts b/backend/src/__tests__/stack-delete-purges-scans.test.ts index 5e111b8d..da633427 100644 --- a/backend/src/__tests__/stack-delete-purges-scans.test.ts +++ b/backend/src/__tests__/stack-delete-purges-scans.test.ts @@ -4,7 +4,7 @@ * longer skews the Security Overview. Image scans are intentionally left to the * janitor reconciler (images are shared and may still exist on the host). */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi, type Mock } from 'vitest'; import request from 'supertest'; import { setupTestDb, cleanupTestDb, loginAsTestAdmin } from './helpers/setupTestDb'; @@ -14,6 +14,8 @@ let DatabaseService: typeof import('../services/DatabaseService').DatabaseServic let ComposeService: typeof import('../services/ComposeService').ComposeService; let FileSystemService: typeof import('../services/FileSystemService').FileSystemService; let MeshService: typeof import('../services/MeshService').MeshService; +let DeployedStackDeletionService: typeof import('../services/DeployedStackDeletionService').DeployedStackDeletionService; +let DockerController: typeof import('../services/DockerController').default; let adminCookie: string; beforeAll(async () => { @@ -22,6 +24,8 @@ beforeAll(async () => { ({ ComposeService } = await import('../services/ComposeService')); ({ FileSystemService } = await import('../services/FileSystemService')); ({ MeshService } = await import('../services/MeshService')); + ({ DeployedStackDeletionService } = await import('../services/DeployedStackDeletionService')); + ({ default: DockerController } = await import('../services/DockerController')); ({ app } = await import('../index')); adminCookie = await loginAsTestAdmin(app); }); @@ -90,3 +94,68 @@ describe('DELETE /api/stacks/:stackName purges scan data', () => { expect(db.getDistinctScanImageRefs(nodeId)).toEqual(['nginx:1']); }); }); + +// ── Wiring: pruneVolumes flag reaches downStack ──────────────────────── + +describe('DELETE /api/stacks/:stackName wires pruneVolumes to downStack', () => { + let downStackSpy: Mock; + + beforeEach(() => { + vi.restoreAllMocks(); + vi.spyOn(FileSystemService.prototype, 'deleteStack').mockResolvedValue(undefined); + vi.spyOn(MeshService.getInstance(), 'optOutStack').mockResolvedValue(undefined); + downStackSpy = vi.spyOn(ComposeService.prototype, 'downStack').mockResolvedValue(undefined); + }); + + it('passes removeVolumes: true when pruneVolumes=true', async () => { + const res = await request(app).delete('/api/stacks/web?pruneVolumes=true').set('Cookie', adminCookie); + + expect(res.status).toBe(200); + const lastArgs = downStackSpy.mock.calls.at(-1); + expect(lastArgs?.[1]).toEqual({ removeVolumes: true }); + }); + + it('passes a falsy removeVolumes when pruneVolumes is absent', async () => { + const res = await request(app).delete('/api/stacks/web').set('Cookie', adminCookie); + + expect(res.status).toBe(200); + const lastArgs = downStackSpy.mock.calls.at(-1); + expect(lastArgs?.[1]?.removeVolumes).toBeFalsy(); + }); +}); + +// ── Continuation guard: persisted flag beats input flag ───────────────── + +describe('DeployedStackDeletionService continuation', () => { + it('uses intent.prune_volumes_requested (not input.pruneVolumes) on resumed deletion', async () => { + vi.restoreAllMocks(); + const downStackSpy = vi.spyOn(ComposeService.prototype, 'downStack').mockResolvedValue(undefined); + vi.spyOn(FileSystemService.prototype, 'deleteStack').mockResolvedValue(undefined); + vi.spyOn(MeshService.getInstance(), 'optOutStack').mockResolvedValue(undefined); + vi.spyOn(DockerController.prototype, 'pruneManagedOnly').mockResolvedValue({ success: true, reclaimedBytes: 0 }); + + const db = DatabaseService.getInstance(); + const nodeId = db.getNodes()[0].id; + + const intentId = 'test-continuation-guard'; + db.getDb().prepare(` + INSERT OR REPLACE INTO stack_update_cleanup_pending + (id, node_id, stack_name, status, target_kind, rollback_tags_json, override_paths_json, prune_volumes_requested, required_blueprint_id, created_at, updated_at) + VALUES (?, ?, ?, 'prepared', 'local_socket', '[]', '[]', 0, NULL, ?, ?) + `).run(intentId, nodeId, 'web', Date.now(), Date.now()); + + const svc = DeployedStackDeletionService.getInstance(); + await svc.deleteDeployedStack({ + nodeId, + stackName: 'web', + pruneVolumes: true, + actor: 'test', + continuationIntentId: intentId, + }); + + db.getDb().prepare('DELETE FROM stack_update_cleanup_pending WHERE id = ?').run(intentId); + + const lastArgs = downStackSpy.mock.calls.at(-1); + expect(lastArgs?.[1]?.removeVolumes).toBeFalsy(); + }); +}); diff --git a/backend/src/proxy/remoteNodeProxy.ts b/backend/src/proxy/remoteNodeProxy.ts index bbb29531..10e44715 100644 --- a/backend/src/proxy/remoteNodeProxy.ts +++ b/backend/src/proxy/remoteNodeProxy.ts @@ -14,6 +14,7 @@ import { isProxyExemptPath } from '../helpers/proxyExemptPaths'; import { remoteSupportsCrossNodeRbac, remoteAdvertisesCapability } from '../helpers/remoteCapabilities'; import { STACK_DOWN_REMOVE_VOLUMES_CAPABILITY, + STACK_DELETE_PRUNE_VOLUMES_CAPABILITY, SERVICE_SCOPED_UPDATE_CAPABILITY, SERVICE_SCOPED_STACK_ALERT_CAPABILITY, SCOPED_STACK_AUTH_EVIDENCE_CAPABILITY, @@ -316,6 +317,17 @@ export function createRemoteProxyMiddleware(): RequestHandler { } } + if (isUnacknowledgedStackDelete(req)) { + const supported = await remoteAdvertisesCapability(req.nodeId, STACK_DELETE_PRUNE_VOLUMES_CAPABILITY); + if (!supported) { + res.status(400).json({ + error: 'This node cannot guarantee volumes are preserved on delete. Upgrade it before deleting this stack.', + code: 'capability_unavailable', + }); + return; + } + } + if (isServiceScopedUpdateRoute(req)) { const supported = await remoteAdvertisesCapability(req.nodeId, SERVICE_SCOPED_UPDATE_CAPABILITY); if (!supported) { @@ -645,6 +657,21 @@ function isStackDownWithRemoveVolumes(req: Request): boolean { return req.query.removeVolumes === 'true'; } +/** + * DELETE /stacks/:stackName without an explicit ?pruneVolumes=true (path is post-/api + * strip). downStack() hardcoded --volumes on every delete before stack-delete-prune-volumes + * existed, unlike runDown which was already volume-safe. So an unacknowledged delete + * forwarded to a remote lacking the capability would silently destroy volumes the + * operator asked to keep, with no local route or UI check able to catch it. Gate the + * unacknowledged path here; an explicit pruneVolumes=true matches what an unsupported + * remote will do anyway and needs no gate. + */ +function isUnacknowledgedStackDelete(req: Request): boolean { + if (req.method !== 'DELETE') return false; + if (!/^\/stacks\/[^/]+\/?$/.test(req.path)) return false; + return req.query.pruneVolumes !== 'true'; +} + /** Nested service update/restore/recovery routes (path is post-/api strip). */ function isServiceScopedUpdateRoute(req: Request): boolean { if (req.method === 'GET') { diff --git a/backend/src/routes/templates.ts b/backend/src/routes/templates.ts index 364158e1..ae635a9e 100644 --- a/backend/src/routes/templates.ts +++ b/backend/src/routes/templates.ts @@ -164,7 +164,7 @@ templatesRouter.post('/deploy', authMiddleware, async (req: Request, res: Respon let dockerDownCompleted = true; let fileDeleteCompleted = true; try { - await ComposeService.getInstance(req.nodeId).downStack(stackName); + await ComposeService.getInstance(req.nodeId).downStack(stackName, { removeVolumes: true }); } catch (downErr) { dockerDownCompleted = false; console.error("[Templates] Rollback Stage 1 (Docker down) failed:", downErr); diff --git a/backend/src/services/CapabilityRegistry.ts b/backend/src/services/CapabilityRegistry.ts index 3d385ce7..d4d9d22e 100644 --- a/backend/src/services/CapabilityRegistry.ts +++ b/backend/src/services/CapabilityRegistry.ts @@ -59,6 +59,7 @@ export const CAPABILITIES = [ 'compose-storage', 'cross-node-rbac', 'stack-down-remove-volumes', + 'stack-delete-prune-volumes', 'guided-external-network-preflight', 'service-scoped-update', 'service-scoped-stack-alert', @@ -97,6 +98,11 @@ export const NOTIFICATION_SUPPRESSION_REPLICA_RETRACTION_CAPABILITY = /** Capability for optional `?removeVolumes=true` on POST /stacks/:name/down. */ export const STACK_DOWN_REMOVE_VOLUMES_CAPABILITY = 'stack-down-remove-volumes' as const satisfies Capability; +/** Capability for honoring `?pruneVolumes` on DELETE /stacks/:name. Nodes without this + * capability always destroy volumes on delete (pre-existing behavior); nodes with it + * honor the operator's checkbox choice. */ +export const STACK_DELETE_PRUNE_VOLUMES_CAPABILITY = 'stack-delete-prune-volumes' as const satisfies Capability; + /** Capability for the nested per-service update/restore routes and the `effective-services` model they read. */ export const SERVICE_SCOPED_UPDATE_CAPABILITY = 'service-scoped-update' as const satisfies Capability; diff --git a/backend/src/services/ComposeService.ts b/backend/src/services/ComposeService.ts index badaed7b..dfa7213d 100644 --- a/backend/src/services/ComposeService.ts +++ b/backend/src/services/ComposeService.ts @@ -1132,10 +1132,13 @@ export class ComposeService { }, sendOutput); } - public async downStack(stackName: string): Promise { + public async downStack(stackName: string, options?: { removeVolumes?: boolean }): Promise { const stackPath = path.join(this.baseDir, stackName); try { - await this.execute('docker', await this.authoredComposeArgs(stackName, ['down', '--volumes', '--remove-orphans']), stackPath, undefined, false); + const args = options?.removeVolumes + ? ['down', '--volumes', '--remove-orphans'] + : ['down', '--remove-orphans']; + await this.execute('docker', await this.authoredComposeArgs(stackName, args), stackPath, undefined, false); } catch (error) { console.warn(`[Teardown] Docker down failed or nothing to clean up for ${sanitizeForLog(stackName)}`); } diff --git a/backend/src/services/DeployedStackDeletionService.ts b/backend/src/services/DeployedStackDeletionService.ts index 340a6294..17dace08 100644 --- a/backend/src/services/DeployedStackDeletionService.ts +++ b/backend/src/services/DeployedStackDeletionService.ts @@ -302,7 +302,9 @@ export class DeployedStackDeletionService { if (!skipPhysical) { try { - await ComposeService.getInstance(nodeId).downStack(stackName); + await ComposeService.getInstance(nodeId).downStack(stackName, { + removeVolumes: intent.prune_volumes_requested === 1, + }); } catch (downErr) { console.warn( '[DeployedStackDeletion] Compose down failed or no-op for %s:', diff --git a/docs/features/node-compatibility.mdx b/docs/features/node-compatibility.mdx index 388375ff..ed68f3f6 100644 --- a/docs/features/node-compatibility.mdx +++ b/docs/features/node-compatibility.mdx @@ -87,6 +87,7 @@ Every Sencho release ships with a static list of capabilities. The current list | `compose-storage` | Stack Storage tab (volume explorer) | | `cross-node-rbac` | Role enforcement on requests forwarded to a remote node | | `stack-down-remove-volumes` | The "also remove volumes" option on Take Down | +| `stack-delete-prune-volumes` | Preserve volumes on Delete unless the operator opts in to remove them | | `guided-external-network-preflight` | Guided missing-external-network check before deploy | | `service-scoped-update` | Per-service update, rebuild, and restore on multi-service stacks | @@ -104,6 +105,7 @@ A handful of capabilities gate a smaller piece of behavior rather than a whole p - Nodes that do not advertise `service-scoped-update` fall back to the legacy per-container layout on multi-service stacks: no declared-service headers, and no per-service update, rebuild, or restore. - Nodes that do not advertise `update-guard` run manual updates directly, with no pre-update rollback-readiness dialog. - `cross-node-rbac` is a security boundary, not a convenience: the control instance refuses to forward a non-admin request, or a confirmed stop-by-label, to a remote node that does not advertise it. This stops a mixed-version fleet from letting a lower-privileged action escalate on an un-upgraded node. +- `stack-delete-prune-volumes` is also a data-safety boundary: a node that does not advertise it cannot guarantee volumes survive a Delete, so the confirmation dialog states that up front and forwards the delete with removal already acknowledged, and the control instance refuses to forward an *unacknowledged* delete to such a node at all rather than risk it silently destroying volumes the operator meant to keep. ## Handling nodes that do not advertise metadata @@ -111,8 +113,8 @@ If a remote node does not respond to `/api/meta` (for example, an unreachable in - The node's row in the switcher and the connection-test panel show no version pill. - Every capability-gated feature on that node shows the lock card. -- Core features (stacks, containers, resources, logs) continue to work normally. -- No errors are surfaced in the UI; the control instance retries the metadata fetch after a short backoff. +- Core features (stacks, containers, resources, logs) continue to work normally, with one exception: Delete requests preservation instead of guessing, so a genuinely un-upgraded remote returns an "upgrade this node" error rather than risk destroying volumes silently (see `stack-delete-prune-volumes` above). +- No other errors are surfaced in the UI; the control instance retries the metadata fetch after a short backoff. If you expect a node to support a feature that is being gated, the fastest fix is to update that node to the latest Sencho release. See the [upgrade guide](/operations/upgrade) for instructions. diff --git a/docs/features/stack-management.mdx b/docs/features/stack-management.mdx index 2615ec4f..87f82b38 100644 --- a/docs/features/stack-management.mdx +++ b/docs/features/stack-management.mdx @@ -383,7 +383,7 @@ The stack header groups actions by frequency of use. The most common action is t | Overflow | **Scan config** | Trivy config scan | Scans the compose file for misconfigurations (admin role). | | Overflow | **Monitor** | Opens Monitor sheet | Opens the stack **Monitor** sheet on the Alerts tab (alert rules and Auto-heal). Same sheet as sidebar **Alerts** / **Auto-Heal**. | | Overflow | **Mute** | Creates a mute rule | Quick presets to mute notifications, deploy-success noise, or monitor alerts for this stack, plus a link to manage its mute rules in full. See [Alerts and Notifications](/features/alerts-notifications). | -| Overflow | **Delete** | `down --volumes` + removes files | Stops and removes containers and volumes, then deletes the stack directory. | +| Overflow | **Delete** | `down --remove-orphans` + removes files | Stops and removes containers, then deletes the stack directory. Volumes are removed only when the operator opts in from the confirmation dialog. On a node too old to guarantee that (see [Node Compatibility](/features/node-compatibility)), the dialog says so and volumes are always removed. | **When stopped:** diff --git a/frontend/src/components/EditorLayout.tsx b/frontend/src/components/EditorLayout.tsx index 035dae06..d3e2fd77 100644 --- a/frontend/src/components/EditorLayout.tsx +++ b/frontend/src/components/EditorLayout.tsx @@ -11,6 +11,7 @@ import { CreateStackDialog, type CreateMode } from './EditorLayout/CreateStackDi import { AdoptExistingDialog } from './EditorLayout/AdoptExistingDialog'; import { EditorView } from './EditorLayout/EditorView'; import { ShellOverlays } from './EditorLayout/ShellOverlays'; +import type { VolumePreservationOnDelete } from './EditorLayout/DeleteStackDialog'; import { classifyFailedGate } from './EditorLayout/failed-gate-recovery'; import { useEditorViewState } from './EditorLayout/hooks/useEditorViewState'; import { useStackListState } from './EditorLayout/hooks/useStackListState'; @@ -37,7 +38,7 @@ import { import { SENCHO_OPEN_LOGS_EVENT, SENCHO_OPEN_STACK_EVENT } from '@/lib/events'; import type { SenchoOpenLogsDetail, SenchoOpenStackDetail } from '@/lib/events'; import { useNodes } from '@/context/NodeContext'; -import { STACK_DOWN_REMOVE_VOLUMES_CAPABILITY } from '@/lib/capabilities'; +import { STACK_DOWN_REMOVE_VOLUMES_CAPABILITY, STACK_DELETE_PRUNE_VOLUMES_CAPABILITY } from '@/lib/capabilities'; import { useAuth } from '@/context/AuthContext'; import { useDeployFeedback } from '@/context/DeployFeedbackContext'; import { useTrivyStatus } from '@/hooks/useTrivyStatus'; @@ -85,6 +86,17 @@ const ResourcesView = lazy(() => import('./ResourcesView')); const NetworkingView = lazy(() => import('./networking/NetworkingView').then(m => ({ default: m.NetworkingView }))); const GlobalObservabilityView = lazy(() => import('./GlobalObservabilityView').then(m => ({ default: m.GlobalObservabilityView }))); +/** + * NodeContext records an unfetched or failed /api/meta as an empty capability list, so an + * empty list means "not confirmed either way", never "confirmed without the capability". + * Reporting that as 'unsupported' would force the destructive delete default onto a node + * that may well preserve volumes, so it maps to 'unknown' instead. + */ +function resolveDeleteVolumePreservation(capabilities: string[] | undefined): VolumePreservationOnDelete { + if (capabilities == null || capabilities.length === 0) return 'unknown'; + return capabilities.includes(STACK_DELETE_PRUNE_VOLUMES_CAPABILITY) ? 'supported' : 'unsupported'; +} + export default function EditorLayout() { const { isAdmin, can, permissions, permissionsStatus } = useAuth(); const { status: trivy } = useTrivyStatus(); @@ -168,6 +180,7 @@ export default function EditorLayout() { const { nodes, activeNode, setActiveNode, hasCapability, activeNodeMeta, isLoading: nodesLoading } = useNodes(); const canOfferVolumeRemoval = activeNodeMeta?.capabilities.includes(STACK_DOWN_REMOVE_VOLUMES_CAPABILITY) === true; + const deleteVolumePreservation = resolveDeleteVolumePreservation(activeNodeMeta?.capabilities); // One-shot boot milestone: the app shell has mounted. Developer mode gates the // hydration-timing overlay for the active node; it follows node switches. @@ -1087,6 +1100,7 @@ export default function EditorLayout() { composeReapply={composeReapply} canSaveAndReapply={canSaveAndReapply} canOfferVolumeRemoval={canOfferVolumeRemoval} + deleteVolumePreservation={deleteVolumePreservation} onOpenFleetNodeUpdates={() => { if (isMobile) { navigateMobileAware('fleet'); diff --git a/frontend/src/components/EditorLayout/CreateStackDialog.tsx b/frontend/src/components/EditorLayout/CreateStackDialog.tsx index f49c4c72..8d36357f 100644 --- a/frontend/src/components/EditorLayout/CreateStackDialog.tsx +++ b/frontend/src/components/EditorLayout/CreateStackDialog.tsx @@ -316,9 +316,17 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks }); if (!saveResponse.ok) { // Roll back the empty stack we just created so we don't leave an orphan. - await apiFetch(`/stacks/${encodeURIComponent(stackName)}`, { method: 'DELETE' }).catch((cleanupError) => { + // pruneVolumes=true: nothing has been deployed yet, so there is no volume + // to preserve, and passing it avoids nodes that require an explicit + // acknowledgement to delete without a guaranteed-preserving capability. + try { + const cleanupRes = await apiFetch(`/stacks/${encodeURIComponent(stackName)}?pruneVolumes=true`, { method: 'DELETE' }); + if (!cleanupRes.ok) { + console.error('Failed to roll back orphan stack after save failure:', await cleanupRes.text()); + } + } catch (cleanupError) { console.error('Failed to roll back orphan stack after save failure:', cleanupError); - }); + } createdStack = false; throw new Error('Could not save the converted YAML. Please try again.'); } diff --git a/frontend/src/components/EditorLayout/DeleteStackDialog.tsx b/frontend/src/components/EditorLayout/DeleteStackDialog.tsx index bcd519ae..09a9df4d 100644 --- a/frontend/src/components/EditorLayout/DeleteStackDialog.tsx +++ b/frontend/src/components/EditorLayout/DeleteStackDialog.tsx @@ -2,10 +2,21 @@ import { useState } from 'react'; import { ConfirmModal } from '../ui/modal'; import { Checkbox } from '../ui/checkbox'; +/** + * Whether the active node is confirmed (via its fetched capabilities) to preserve volumes + * on delete unless the operator opts in to removing them. 'unknown' covers both "not + * fetched yet" and "fetch failed": a node we simply have not confirmed must not be assumed + * incapable, so it requests preservation like a 'supported' node (a genuinely stale remote + * rejects that request outright instead of silently destroying data). Only 'unsupported' + * forces removal. + */ +export type VolumePreservationOnDelete = 'supported' | 'unsupported' | 'unknown'; + export interface DeleteStackDialogProps { open: boolean; onOpenChange: (open: boolean) => void; stackName: string | null; + volumePreservation?: VolumePreservationOnDelete; onConfirm: (pruneVolumes: boolean) => void | Promise; /** True while the stack delete request owns the flow (from stackActionMap). */ confirming?: boolean; @@ -15,10 +26,20 @@ export function DeleteStackDialog({ open, onOpenChange, stackName, + volumePreservation = 'unknown', onConfirm, confirming = false, }: DeleteStackDialogProps) { const [pruneVolumes, setPruneVolumes] = useState(false); + const showVolumeOption = volumePreservation === 'supported'; + const confirmedUnsupported = volumePreservation === 'unsupported'; + // Single source of truth for what confirming will do: the operator's choice when the + // node offers one, forced removal only on a node confirmed unable to preserve volumes. + const willRemoveVolumes = showVolumeOption ? pruneVolumes : confirmedUnsupported; + + let volumeHint = 'VOLUMES KEPT'; + if (confirmedUnsupported) volumeHint = 'VOLUMES WILL BE REMOVED'; + else if (willRemoveVolumes) volumeHint = 'VOLUMES PRUNED'; const handleOpenChange = (next: boolean) => { if (!next) setPruneVolumes(false); @@ -48,24 +69,34 @@ export function DeleteStackDialog({ ) } description={`Confirm deletion of ${stackName ?? 'stack'}.`} - hint={pruneVolumes ? 'VOLUMES PRUNED' : 'VOLUMES KEPT'} + hint={volumeHint} confirmLabel="Delete" busyConfirmLabel="Deleting..." confirming={confirming} - onConfirm={() => onConfirm(pruneVolumes)} + onConfirm={() => { + setPruneVolumes(false); + onConfirm(willRemoveVolumes); + }} >

This action cannot be undone.

-
- setPruneVolumes(v === true)} - /> - -
+ {showVolumeOption && ( +
+ setPruneVolumes(v === true)} + /> + +
+ )} + {confirmedUnsupported && ( +

+ This node can't preserve volumes on delete. Any volumes associated with this stack will be removed too. +

+ )} ); } diff --git a/frontend/src/components/EditorLayout/ShellOverlays.tsx b/frontend/src/components/EditorLayout/ShellOverlays.tsx index 9be57c51..32541937 100644 --- a/frontend/src/components/EditorLayout/ShellOverlays.tsx +++ b/frontend/src/components/EditorLayout/ShellOverlays.tsx @@ -6,7 +6,7 @@ import { UpdateReadinessDialog } from '../stack/UpdateReadinessDialog'; import { SelfStackProtectedDialog } from '../stack/SelfStackProtectedDialog'; import { LocalUpdateConfirmDialog } from '../FleetView/LocalUpdateConfirmDialog'; import { ReconnectingOverlay } from '../FleetView/ReconnectingOverlay'; -import { DeleteStackDialog } from './DeleteStackDialog'; +import { DeleteStackDialog, type VolumePreservationOnDelete } from './DeleteStackDialog'; import { TakeDownStackDialog } from './TakeDownStackDialog'; import { UnsavedChangesDialog } from './UnsavedChangesDialog'; import { StackAlertSheet } from '../StackAlertSheet'; @@ -41,6 +41,7 @@ interface ShellOverlaysProps { composeReapply: ReturnType; canSaveAndReapply: boolean; canOfferVolumeRemoval: boolean; + deleteVolumePreservation: VolumePreservationOnDelete; onOpenFleetNodeUpdates: () => void; } @@ -61,6 +62,7 @@ export function ShellOverlays({ composeReapply, canSaveAndReapply, canOfferVolumeRemoval, + deleteVolumePreservation, onOpenFleetNodeUpdates, }: ShellOverlaysProps) { const { @@ -93,6 +95,7 @@ export function ShellOverlays({ open={deleteDialogOpen} onOpenChange={(open) => { if (!open) closeDeleteDialog(); }} stackName={stackToDelete} + volumePreservation={deleteVolumePreservation} onConfirm={stackActions.deleteStack} confirming={isDeleteConfirming} /> diff --git a/frontend/src/components/EditorLayout/__tests__/CreateStackDialog.test.tsx b/frontend/src/components/EditorLayout/__tests__/CreateStackDialog.test.tsx index 474d660a..110a6be6 100644 --- a/frontend/src/components/EditorLayout/__tests__/CreateStackDialog.test.tsx +++ b/frontend/src/components/EditorLayout/__tests__/CreateStackDialog.test.tsx @@ -1,10 +1,13 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import type { ComponentProps } from 'react'; -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { CreateStackDialog } from '../CreateStackDialog'; +import { apiFetch } from '@/lib/api'; vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() })); -vi.mock('@/components/ui/toast-store', () => ({ toast: { error: vi.fn(), dismiss: vi.fn() } })); +vi.mock('@/components/ui/toast-store', () => ({ + toast: { error: vi.fn(), success: vi.fn(), loading: vi.fn(), dismiss: vi.fn() }, +})); vi.mock('@/context/NodeContext', () => ({ useNodes: () => ({ activeNode: { id: 1, name: 'local' } }), })); @@ -54,4 +57,28 @@ describe('CreateStackDialog', () => { renderOpen(); expect(screen.queryByRole('button', { name: /adopt existing files instead/i })).toBeNull(); }); + + it('rolls back an orphaned stack with pruneVolumes=true when saving converted YAML fails', async () => { + const fetchMock = vi.mocked(apiFetch); + fetchMock + .mockResolvedValueOnce(new Response(JSON.stringify({ yaml: 'services:\n app:\n image: nginx' }), { status: 200 })) + .mockResolvedValueOnce(new Response(null, { status: 200 })) // POST /stacks create + .mockResolvedValueOnce(new Response('save failed', { status: 500 })) // PUT save + .mockResolvedValueOnce(new Response(null, { status: 200 })); // DELETE rollback + + renderOpen(); + fireEvent.click(screen.getByRole('tab', { name: 'From Docker Run' })); + + fireEvent.change(screen.getByLabelText('Paste your docker run command'), { + target: { value: 'docker run -d --name nginx -p 8080:80 nginx:latest' }, + }); + fireEvent.click(screen.getByRole('button', { name: /convert/i })); + await waitFor(() => expect(screen.getByText('compose.yaml preview')).toBeVisible()); + + fireEvent.change(screen.getByLabelText('Stack Name'), { target: { value: 'nginx' } }); + fireEvent.click(screen.getByRole('button', { name: /create stack/i })); + + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(4)); + expect(fetchMock).toHaveBeenLastCalledWith('/stacks/nginx?pruneVolumes=true', { method: 'DELETE' }); + }); }); diff --git a/frontend/src/components/EditorLayout/__tests__/DeleteStackDialog.test.tsx b/frontend/src/components/EditorLayout/__tests__/DeleteStackDialog.test.tsx index 6cb9dd5f..d0d8d171 100644 --- a/frontend/src/components/EditorLayout/__tests__/DeleteStackDialog.test.tsx +++ b/frontend/src/components/EditorLayout/__tests__/DeleteStackDialog.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; -import { render, screen } from '@testing-library/react'; +import { render, screen, fireEvent } from '@testing-library/react'; import { DeleteStackDialog } from '../DeleteStackDialog'; const LONG_STACK_NAME = 'this-is-a-very-long-stack-name-that-should-not-push-actions-off-screen'; @@ -35,6 +35,7 @@ describe('DeleteStackDialog', () => { open onOpenChange={vi.fn()} stackName="web" + volumePreservation="supported" onConfirm={vi.fn()} confirming />, @@ -44,4 +45,75 @@ describe('DeleteStackDialog', () => { expect(screen.getByRole('button', { name: 'Cancel' })).toBeDisabled(); expect(screen.getByRole('checkbox')).toBeDisabled(); }); + + it('warns instead of offering a checkbox when the node is confirmed unable to preserve volumes', () => { + render( + , + ); + + expect(screen.queryByRole('checkbox')).not.toBeInTheDocument(); + expect(screen.getByText(/can't preserve volumes on delete/i)).toBeVisible(); + expect(screen.getByText('VOLUMES WILL BE REMOVED')).toBeVisible(); + }); + + it('confirms with pruneVolumes: true on a node confirmed unable to preserve volumes, without the operator opting in', () => { + const onConfirm = vi.fn(); + render( + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Delete' })); + + expect(onConfirm).toHaveBeenCalledWith(true); + }); + + it('confirms with pruneVolumes: false by default when the node can preserve volumes', () => { + const onConfirm = vi.fn(); + render( + , + ); + + fireEvent.click(screen.getByRole('button', { name: 'Delete' })); + + expect(onConfirm).toHaveBeenCalledWith(false); + }); + + it('requests preservation (pruneVolumes: false) and shows no destructive warning when node support is not yet confirmed', () => { + const onConfirm = vi.fn(); + render( + , + ); + + expect(screen.queryByRole('checkbox')).not.toBeInTheDocument(); + expect(screen.queryByText(/can't preserve volumes on delete/i)).not.toBeInTheDocument(); + expect(screen.getByText('VOLUMES KEPT')).toBeVisible(); + + fireEvent.click(screen.getByRole('button', { name: 'Delete' })); + + expect(onConfirm).toHaveBeenCalledWith(false); + }); }); diff --git a/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts b/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts index f00aa0be..6a9bffb9 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts @@ -1729,7 +1729,7 @@ describe('useStackActions.deleteStack', () => { await result.current.deleteStack(false); }); - expect(apiFetch).toHaveBeenCalledWith('/stacks/web.yml', { method: 'DELETE' }); + expect(apiFetch).toHaveBeenCalledWith('/stacks/web.yml', { method: 'DELETE', nodeId: 1 }); expect(stackListState.setSelectedFile).toHaveBeenCalledWith(null); expect(navState.setActiveView).toHaveBeenCalledWith('dashboard'); expect(navState.setActiveView).toHaveBeenCalledTimes(1); @@ -1738,6 +1738,21 @@ describe('useStackActions.deleteStack', () => { expect(stackListState.refreshStacks).toHaveBeenCalled(); }); + it('passes pruneVolumes=true through unconditionally, including on nodes without the capability', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); + const { result } = setup({ + overlay: { stackToDelete: 'web.yml' }, + stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, + navState: { activeView: 'editor' }, + }); + + await act(async () => { + await result.current.deleteStack(true); + }); + + expect(apiFetch).toHaveBeenCalledWith('/stacks/web.yml?pruneVolumes=true', { method: 'DELETE', nodeId: 1 }); + }); + it('clears isFileLoading on delete-leave so the URL writer is not blocked', async () => { vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); const { result, editorState } = setup({ @@ -1766,7 +1781,7 @@ describe('useStackActions.deleteStack', () => { await result.current.deleteStack(false); }); - expect(apiFetch).toHaveBeenCalledWith('/stacks/web', { method: 'DELETE' }); + expect(apiFetch).toHaveBeenCalledWith('/stacks/web', { method: 'DELETE', nodeId: 1 }); expect(stackListState.setSelectedFile).toHaveBeenCalledWith(null); expect(navState.setActiveView).toHaveBeenCalledWith('dashboard'); expect(onDeletedOpenStack).toHaveBeenCalledTimes(1); @@ -1832,6 +1847,30 @@ describe('useStackActions.deleteStack', () => { expect(toast.error).toHaveBeenCalled(); }); + it('surfaces the parsed error message, not the raw JSON body, on a non-OK delete response', async () => { + vi.mocked(apiFetch).mockResolvedValue( + new Response( + JSON.stringify({ + error: 'This node cannot guarantee volumes are preserved on delete.', + code: 'capability_unavailable', + }), + { status: 400 }, + ), + ); + const { toast } = await import('@/components/ui/toast-store'); + const { result } = setup({ + overlay: { stackToDelete: 'web.yml' }, + stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, + navState: { activeView: 'editor' }, + }); + + await act(async () => { + await result.current.deleteStack(true); + }); + + expect(toast.error).toHaveBeenCalledWith('This node cannot guarantee volumes are preserved on delete.'); + }); + it('does not navigate on a self-stack-protected response', async () => { vi.mocked(apiFetch).mockResolvedValue( new Response(JSON.stringify({ code: 'self_stack_protected' }), { status: 409 }), diff --git a/frontend/src/components/EditorLayout/hooks/useStackActions.ts b/frontend/src/components/EditorLayout/hooks/useStackActions.ts index 2c80aa62..ae9827eb 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackActions.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackActions.ts @@ -1952,12 +1952,13 @@ export function useStackActions(options: UseStackActionsOptions) { const deleteKey = resolveStackFileKey(stackListState.files, stackToDelete); const canonicalName = deleteKey.replace(/\.(yml|yaml)$/, ''); if (stackListState.isStackBusy(deleteKey)) return; + const opNodeId = activeNode?.id ?? null; stackListState.setStackAction(deleteKey, 'delete'); try { const url = pruneVolumes ? `/stacks/${stackToDelete}?pruneVolumes=true` : `/stacks/${stackToDelete}`; - const response = await apiFetch(url, { method: 'DELETE' }); + const response = await apiFetch(url, { method: 'DELETE', nodeId: opNodeId }); if (!response.ok) { const errText = await response.text(); if (isSelfStackProtectedResponse(errText, response.status)) { @@ -1965,7 +1966,7 @@ export function useStackActions(options: UseStackActionsOptions) { overlayState.closeDeleteDialog(); return; } - throw new Error(errText || 'Failed to delete stack'); + throw parseStackActionError(errText, 'Failed to delete stack', response.status); } toast.success('Stack deleted successfully!'); overlayState.closeDeleteDialog(); diff --git a/frontend/src/lib/capabilities.ts b/frontend/src/lib/capabilities.ts index 87bcb9c9..501860cf 100644 --- a/frontend/src/lib/capabilities.ts +++ b/frontend/src/lib/capabilities.ts @@ -37,6 +37,7 @@ export const CAPABILITIES = [ 'compose-storage', 'cross-node-rbac', 'stack-down-remove-volumes', + 'stack-delete-prune-volumes', 'guided-external-network-preflight', 'service-scoped-update', 'service-scoped-stack-alert', @@ -52,6 +53,7 @@ export const HOST_CONSOLE_CAPABILITY = 'host-console' as const satisfies Capabil export const HOST_CONSOLE_COMMUNITY_CAPABILITY = 'host-console-community' as const satisfies Capability; export const STACK_DOWN_REMOVE_VOLUMES_CAPABILITY = 'stack-down-remove-volumes' as const satisfies Capability; +export const STACK_DELETE_PRUNE_VOLUMES_CAPABILITY = 'stack-delete-prune-volumes' as const satisfies Capability; export const GUIDED_EXTERNAL_NETWORK_PREFLIGHT_CAPABILITY = 'guided-external-network-preflight' as const satisfies Capability; export const SERVICE_SCOPED_UPDATE_CAPABILITY = 'service-scoped-update' as const satisfies Capability; export const SERVICE_SCOPED_STACK_ALERT_CAPABILITY = 'service-scoped-stack-alert' as const satisfies Capability;