fix: harden atomic deployment rollback (#1029)

* fix: harden atomic deployment rollback

* fix: update Docker toolchain to Go 1.26.3

* fix: repair Dockerfile tr argument split across lines

* fix: bump protobufjs to clear npm audit high-severity advisories

* fix: sanitize error objects in console.error to prevent log injection
This commit is contained in:
Anso
2026-05-12 15:58:30 -04:00
committed by GitHub
parent 69b6ac1f3b
commit 74ae2ce0c6
11 changed files with 739 additions and 530 deletions
+37 -3
View File
@@ -100,7 +100,7 @@ vi.mock('../services/LogFormatter', () => ({
LogFormatter: { formatLine: (line: string) => line },
}));
import { ComposeService } from '../services/ComposeService';
import { ComposeService, getComposeRollbackInfo } from '../services/ComposeService';
/** Creates an EventEmitter that mimics a child_process spawn result */
function createMockProcess() {
@@ -235,7 +235,19 @@ describe('ComposeService - deployStack', () => {
expect(mockBackupStackFiles).toHaveBeenCalledWith('my-stack');
});
it('throws CONTAINER_CRASHED when exited container has non-zero exit code', async () => {
it('aborts atomic deploy before docker side effects when backup fails', async () => {
mockBackupStackFiles.mockRejectedValueOnce(new Error('disk full'));
const svc = ComposeService.getInstance(1);
await expect(svc.deployStack('my-stack', undefined, true)).rejects.toThrow(
'Atomic deployment backup failed',
);
expect(mockSpawn).not.toHaveBeenCalled();
expect(mockGetContainersByStack).not.toHaveBeenCalled();
});
it('throws sanitized CONTAINER_CRASHED when exited container has non-zero exit code', async () => {
setupAutoCloseSpawn();
mockListContainers.mockResolvedValue([{
Id: 'crashed-c1',
@@ -243,7 +255,7 @@ describe('ComposeService - deployStack', () => {
Labels: { 'com.docker.compose.project': 'my-stack' },
}]);
mockContainerInspect.mockResolvedValue({ State: { ExitCode: 1 } });
mockContainerLogs.mockResolvedValue(Buffer.from('Error: something failed'));
mockContainerLogs.mockResolvedValue(Buffer.from('SECRET_TOKEN=leaked'));
const svc = ComposeService.getInstance(1);
// Attach catch handler immediately so rejection is never "unhandled"
@@ -253,6 +265,8 @@ describe('ComposeService - deployStack', () => {
const error = await result;
expect(error).not.toBeNull();
expect(error!.message).toContain('CONTAINER_CRASHED');
expect(error!.message).not.toContain('SECRET_TOKEN');
expect(mockContainerLogs).not.toHaveBeenCalled();
});
it('rolls back on failure when atomic=true', async () => {
@@ -272,9 +286,29 @@ describe('ComposeService - deployStack', () => {
const error = await result;
expect(error).not.toBeNull();
expect(error!.message).toContain('CONTAINER_CRASHED');
expect(getComposeRollbackInfo(error)).toEqual({ attempted: true, rolledBack: true });
expect(mockRestoreStackFiles).toHaveBeenCalledWith('my-stack');
});
it('reports rollback failure when atomic restore fails', async () => {
setupAutoCloseSpawn();
mockListContainers.mockResolvedValue([{
Id: 'crashed-c1',
State: 'exited',
Labels: { 'com.docker.compose.project': 'my-stack' },
}]);
mockContainerInspect.mockResolvedValue({ State: { ExitCode: 1 } });
mockRestoreStackFiles.mockRejectedValueOnce(new Error('restore denied'));
const svc = ComposeService.getInstance(1);
const result = svc.deployStack('my-stack', undefined, true).then(() => null, (e: Error) => e);
await vi.runAllTimersAsync();
const error = await result;
expect(error).not.toBeNull();
expect(getComposeRollbackInfo(error)).toEqual({ attempted: true, rolledBack: false });
});
it('does not roll back when atomic=false', async () => {
setupAutoCloseSpawn();
mockListContainers.mockResolvedValue([{
@@ -1,6 +1,6 @@
/**
* Verifies that FileSystemService stores stack backups under
* <DATA_DIR>/backups/<stackName>/ rather than inside the user's compose
* <DATA_DIR>/backups/<nodeId>/<stackName>/ rather than inside the user's compose
* folder. The old in-stack-folder location failed with EACCES whenever a
* container had chowned the bind mount, breaking the atomic rollback
* feature for those stacks.
@@ -12,12 +12,12 @@ import { promises as fsPromises } from 'fs';
// Mutable state the mocked NodeRegistry reads. Each test rewrites these
// before instantiating FileSystemService.
const mockState = { composeDir: '' };
const mockState = { composeDir: '', composeDirs: new Map<number, string>() };
vi.mock('../services/NodeRegistry', () => ({
NodeRegistry: {
getInstance: () => ({
getComposeDir: () => mockState.composeDir,
getComposeDir: (nodeId?: number) => mockState.composeDirs.get(nodeId ?? 1) ?? mockState.composeDir,
getDefaultNodeId: () => 1,
}),
},
@@ -34,6 +34,7 @@ describe('FileSystemService backup location', () => {
composeDir = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'sencho-compose-'));
dataDir = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'sencho-data-'));
mockState.composeDir = composeDir;
mockState.composeDirs = new Map([[1, composeDir]]);
originalDataDir = process.env.DATA_DIR;
process.env.DATA_DIR = dataDir;
});
@@ -45,7 +46,7 @@ describe('FileSystemService backup location', () => {
await fsPromises.rm(dataDir, { recursive: true, force: true });
});
it('writes backups under <DATA_DIR>/backups/<stackName>/, not inside the stack folder', async () => {
it('writes backups under <DATA_DIR>/backups/<nodeId>/<stackName>/, not inside the stack folder', async () => {
const stackName = 'web';
const stackDir = path.join(composeDir, stackName);
await fsPromises.mkdir(stackDir, { recursive: true });
@@ -55,7 +56,7 @@ describe('FileSystemService backup location', () => {
const service = FileSystemService.getInstance();
await service.backupStackFiles(stackName);
const newBackupDir = path.join(dataDir, 'backups', stackName);
const newBackupDir = path.join(dataDir, 'backups', '1', stackName);
const oldBackupDir = path.join(stackDir, '.sencho-backup');
// New location has every backed-up file
@@ -83,6 +84,32 @@ describe('FileSystemService backup location', () => {
expect(typeof after.timestamp).toBe('number');
});
it('scopes backups by node id when stack names overlap', async () => {
const stackName = 'web';
const secondComposeDir = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'sencho-compose-'));
mockState.composeDirs.set(2, secondComposeDir);
try {
const nodeOneStackDir = path.join(composeDir, stackName);
const nodeTwoStackDir = path.join(secondComposeDir, stackName);
await fsPromises.mkdir(nodeOneStackDir, { recursive: true });
await fsPromises.mkdir(nodeTwoStackDir, { recursive: true });
await fsPromises.writeFile(path.join(nodeOneStackDir, 'compose.yaml'), 'services:\n one: {}\n', 'utf-8');
await fsPromises.writeFile(path.join(nodeTwoStackDir, 'compose.yaml'), 'services:\n two: {}\n', 'utf-8');
await FileSystemService.getInstance(1).backupStackFiles(stackName);
await FileSystemService.getInstance(2).backupStackFiles(stackName);
await expect(
fsPromises.readFile(path.join(dataDir, 'backups', '1', stackName, 'compose.yaml'), 'utf-8'),
).resolves.toContain('one');
await expect(
fsPromises.readFile(path.join(dataDir, 'backups', '2', stackName, 'compose.yaml'), 'utf-8'),
).resolves.toContain('two');
} finally {
await fsPromises.rm(secondComposeDir, { recursive: true, force: true });
}
});
it('restoreStackFiles copies files from the new location back to the stack dir', async () => {
const stackName = 'db';
const stackDir = path.join(composeDir, stackName);
@@ -12,7 +12,7 @@ const {
mockUpdateScheduledTask, mockCleanupOldTaskRuns, mockGetScheduledTask, mockGetNodes, mockGetNode,
mockCreateSnapshot, mockInsertSnapshotFiles, mockClearStackUpdateStatus,
mockMarkStaleRunsAsFailed, mockDeleteOldScans,
mockGetTier, mockGetVariant,
mockGetTier, mockGetVariant, mockGetProxyHeaders,
mockGetContainersByStack, mockRestartContainer, mockPruneSystem,
mockUpdateStack,
mockGetStacks, mockGetStackContent, mockGetEnvContent,
@@ -42,6 +42,7 @@ const {
mockDeleteOldScans: vi.fn().mockReturnValue(0),
mockGetTier: vi.fn().mockReturnValue('paid'),
mockGetVariant: vi.fn().mockReturnValue('admiral'),
mockGetProxyHeaders: vi.fn().mockReturnValue({ tier: 'paid', variant: 'admiral' }),
mockGetContainersByStack: vi.fn().mockResolvedValue([]),
mockRestartContainer: vi.fn().mockResolvedValue(undefined),
mockPruneSystem: vi.fn().mockResolvedValue({ success: true, reclaimedBytes: 0 }),
@@ -94,6 +95,7 @@ vi.mock('../services/LicenseService', () => ({
getInstance: () => ({
getTier: mockGetTier,
getVariant: mockGetVariant,
getProxyHeaders: mockGetProxyHeaders,
}),
},
}));
@@ -1349,6 +1351,11 @@ describe('SchedulerService - executeUpdateRemote', () => {
'http://remote:1852/api/auto-update/execute',
expect.objectContaining({
method: 'POST',
headers: expect.objectContaining({
'Authorization': 'Bearer test-token',
'x-sencho-tier': 'paid',
'x-sencho-variant': 'admiral',
}),
body: JSON.stringify({ target: 'web-app' }),
})
);
@@ -9,7 +9,9 @@
*/
import { describe, it, expect, beforeAll, afterAll, vi, beforeEach } from 'vitest';
import request from 'supertest';
import { setupTestDb, cleanupTestDb, loginAsTestAdmin } from './helpers/setupTestDb';
import jwt from 'jsonwebtoken';
import { setupTestDb, cleanupTestDb, loginAsTestAdmin, TEST_JWT_SECRET } from './helpers/setupTestDb';
import { ComposeRollbackError } from '../services/ComposeService';
// ── Hoisted mocks (must come before importing the app) ──────────────────────
@@ -143,6 +145,46 @@ describe('deploy_failure notification on /deploy error', () => {
expect(call[2]).toContain('network timeout');
expect(call[3]).toEqual({ stackName: 'webapp' });
});
it('returns rolledBack=true only when compose rollback completed', async () => {
mockDeployStack.mockRejectedValue(
new ComposeRollbackError(new Error('image pull failed'), true, true),
);
const res = await request(app)
.post('/api/stacks/myapp/deploy')
.set('Cookie', authCookie);
expect(res.status).toBe(500);
expect(res.body).toMatchObject({ rolledBack: true });
});
it('returns rolledBack=false when compose rollback failed', async () => {
mockDeployStack.mockRejectedValue(
new ComposeRollbackError(new Error('image pull failed'), true, false),
);
const res = await request(app)
.post('/api/stacks/myapp/deploy')
.set('Cookie', authCookie);
expect(res.status).toBe(500);
expect(res.body).toMatchObject({ rolledBack: false });
});
it('uses trusted proxy tier headers for remote atomic deploys', async () => {
mockDeployStack.mockResolvedValue(undefined);
const token = jwt.sign({ scope: 'node_proxy' }, TEST_JWT_SECRET, { expiresIn: '1m' });
const res = await request(app)
.post('/api/stacks/myapp/deploy')
.set('Authorization', `Bearer ${token}`)
.set('x-sencho-tier', 'paid')
.set('x-sencho-variant', 'skipper');
expect(res.status).toBe(200);
expect(mockDeployStack.mock.calls[0][2]).toBe(true);
});
});
describe('deploy_failure notification on /down error', () => {
@@ -229,4 +271,17 @@ describe('deploy_failure notification on /update error', () => {
{ stackName: 'myapp' },
);
});
it('returns rollback completion status when updateStack throws rollback metadata', async () => {
mockUpdateStack.mockRejectedValue(
new ComposeRollbackError(new Error('image not found'), true, false),
);
const res = await request(app)
.post('/api/stacks/myapp/update')
.set('Cookie', authCookie);
expect(res.status).toBe(500);
expect(res.body).toMatchObject({ rolledBack: false });
});
});