feat(updates): auto-prune dangling images after updates (#1316)

* feat(updates): auto-prune dangling images after updates

Each update pulls a fresh image and recreates containers, leaving the
replaced image behind as a dangling layer that previously had to be
pruned by hand. A new "Prune dangling images after updates" toggle under
Settings > System > Docker hygiene reclaims these automatically.

The setting is on by default and opt-out. When enabled, a successful
stack update (manual or scheduled) and a Sencho self-update each remove
the dangling image layers they orphaned. Only untagged layers are
touched; tagged images, volumes, and data are never removed. The toggle
requires an admin account and is per node: each instance honors its own
value, so a remote node self-update applies that node's own preference.

A prune failure never affects the update result: on the stack path it is
caught and logged after the update has already succeeded, and on the
self-update path the helper-shell prune runs only after a clean recreate
and cannot change the exit code or the recorded update error.

* security(self-update): shell-quote label-derived values in helper command

Address review feedback on the prune-on-update change:

- The self-update helper command interpolated the compose service name and
  config-file paths (both read from Docker Compose labels) straight into a
  shell string. Shell-quote them via shQuote so a label carrying shell
  metacharacters stays inert data and cannot break the exit-code capture,
  error-file write, or prune guard.
- Correct the settings copy and docs: the prune is a standard dangling-image
  prune, so it reclaims every untagged layer on the node, not only the one the
  current update orphaned. Tagged images, volumes, and data remain untouched.
- Add tests: shell-metacharacter neutralization and prune-output suppression in
  the self-update command, and an atomic-update case asserting a prune failure
  does not trigger a rollback.

* fix(updates): omit the reclaim figure when the daemon reports zero bytes

End-to-end testing on a Docker daemon backed by the containerd image store
showed the post-update prune removing a dangling image while the prune API
returned SpaceReclaimed=0, so the stream printed "reclaimed 0.0 MB" even though
an image was removed. Show the reclaimed figure only when the daemon reports a
non-zero value; otherwise the line reads "=== Pruned dangling images ===". The
overlay2 store still reports real figures and shows them. Add a test covering
both branches.
This commit is contained in:
Anso
2026-06-05 18:12:37 -04:00
committed by GitHub
parent 622af7e0b3
commit 716daf77d0
12 changed files with 324 additions and 10 deletions
@@ -16,6 +16,7 @@ const {
mockGetRegistries, mockResolveDockerConfig,
mockBackupStackFiles, mockRestoreStackFiles,
mockMkdtempSync, mockWriteFileSync, mockUnlinkSync, mockRmdirSync,
mockGetGlobalSettings, mockPruneDanglingImages,
} = vi.hoisted(() => ({
mockSpawn: vi.fn(),
mockGetContainersByStack: vi.fn().mockResolvedValue([]),
@@ -31,6 +32,8 @@ const {
mockWriteFileSync: vi.fn(),
mockUnlinkSync: vi.fn(),
mockRmdirSync: vi.fn(),
mockGetGlobalSettings: vi.fn().mockReturnValue({}),
mockPruneDanglingImages: vi.fn().mockResolvedValue({ reclaimedBytes: 0 }),
}));
vi.mock('child_process', () => ({ spawn: mockSpawn, execFile: vi.fn() }));
@@ -62,6 +65,7 @@ vi.mock('../services/DockerController', () => ({
getInstance: () => ({
getContainersByStack: mockGetContainersByStack,
removeContainers: mockRemoveContainers,
pruneDanglingImages: mockPruneDanglingImages,
getDocker: () => ({
listContainers: mockListContainers,
getContainer: () => ({
@@ -77,6 +81,7 @@ vi.mock('../services/DatabaseService', () => ({
DatabaseService: {
getInstance: () => ({
getRegistries: mockGetRegistries,
getGlobalSettings: mockGetGlobalSettings,
}),
},
}));
@@ -463,6 +468,121 @@ describe('ComposeService - deployStack', () => {
});
});
// ── updateStack: prune-on-update ───────────────────────────────────────
describe('ComposeService - updateStack prune-on-update', () => {
it('prunes dangling images after a successful update when prune_on_update=1', async () => {
setupAutoCloseSpawn();
mockListContainers.mockResolvedValue([]);
mockGetGlobalSettings.mockReturnValue({ prune_on_update: '1' });
mockPruneDanglingImages.mockResolvedValue({ reclaimedBytes: 2_097_152 });
const svc = ComposeService.getInstance(1);
const promise = svc.updateStack('my-stack');
await vi.advanceTimersByTimeAsync(3100);
await promise;
expect(mockPruneDanglingImages).toHaveBeenCalledTimes(1);
});
it('streams a reclaim figure only when the daemon reports bytes', async () => {
mockListContainers.mockResolvedValue([]);
mockGetGlobalSettings.mockReturnValue({ prune_on_update: '1' });
const ws = createMockWs();
// The containerd image store reports SpaceReclaimed=0 even when it removes
// images, so a zero figure must be omitted rather than shown as "0.0 MB".
setupAutoCloseSpawn();
mockPruneDanglingImages.mockResolvedValueOnce({ reclaimedBytes: 0 });
let p = ComposeService.getInstance(1).updateStack('my-stack', ws);
await vi.advanceTimersByTimeAsync(3100);
await p;
expect(ws.send).toHaveBeenCalledWith('=== Pruned dangling images ===\n');
ws.send.mockClear();
setupAutoCloseSpawn();
mockPruneDanglingImages.mockResolvedValueOnce({ reclaimedBytes: 5 * 1024 * 1024 });
p = ComposeService.getInstance(1).updateStack('my-stack', ws);
await vi.advanceTimersByTimeAsync(3100);
await p;
expect(ws.send).toHaveBeenCalledWith('=== Pruned dangling images · reclaimed 5.0 MB ===\n');
});
it('does not prune when prune_on_update=0', async () => {
setupAutoCloseSpawn();
mockListContainers.mockResolvedValue([]);
mockGetGlobalSettings.mockReturnValue({ prune_on_update: '0' });
const svc = ComposeService.getInstance(1);
const promise = svc.updateStack('my-stack');
await vi.advanceTimersByTimeAsync(3100);
await promise;
expect(mockPruneDanglingImages).not.toHaveBeenCalled();
});
it('does not prune when the setting key is absent (fail-safe for un-backfilled DBs)', async () => {
setupAutoCloseSpawn();
mockListContainers.mockResolvedValue([]);
mockGetGlobalSettings.mockReturnValue({});
const svc = ComposeService.getInstance(1);
const promise = svc.updateStack('my-stack');
await vi.advanceTimersByTimeAsync(3100);
await promise;
expect(mockPruneDanglingImages).not.toHaveBeenCalled();
});
it('does not prune when the update itself fails (prune is success-only)', async () => {
setupAutoCloseSpawn();
// A container that exited non-zero makes the post-update health probe throw,
// so control never reaches the prune block that follows it.
mockListContainers.mockResolvedValue([{ Id: 'c1', State: 'exited' }]);
mockContainerInspect.mockResolvedValue({ State: { ExitCode: 1 } });
mockGetGlobalSettings.mockReturnValue({ prune_on_update: '1' });
const svc = ComposeService.getInstance(1);
const promise = svc.updateStack('my-stack');
// Attach the rejection expectation before advancing timers so the throw
// (which fires mid-advance) is never momentarily unhandled.
const rejection = expect(promise).rejects.toThrow();
await vi.advanceTimersByTimeAsync(3100);
await rejection;
expect(mockPruneDanglingImages).not.toHaveBeenCalled();
});
it('does not roll back an atomic update when the post-update prune throws', async () => {
setupAutoCloseSpawn();
mockListContainers.mockResolvedValue([]);
mockGetGlobalSettings.mockReturnValue({ prune_on_update: '1' });
mockPruneDanglingImages.mockRejectedValueOnce(new Error('docker busy'));
const svc = ComposeService.getInstance(1);
const promise = svc.updateStack('my-stack', undefined, true); // atomic
await vi.advanceTimersByTimeAsync(3100);
// The update already succeeded before the prune ran, so a prune failure
// must neither reject nor trigger the atomic restore.
await expect(promise).resolves.toBeUndefined();
expect(mockRestoreStackFiles).not.toHaveBeenCalled();
});
it('does not fail the update when the prune throws', async () => {
setupAutoCloseSpawn();
mockListContainers.mockResolvedValue([]);
mockGetGlobalSettings.mockReturnValue({ prune_on_update: '1' });
mockPruneDanglingImages.mockRejectedValueOnce(new Error('docker busy'));
const svc = ComposeService.getInstance(1);
const promise = svc.updateStack('my-stack');
await vi.advanceTimersByTimeAsync(3100);
await expect(promise).resolves.toBeUndefined();
});
});
// ── withRegistryAuth ───────────────────────────────────────────────────
describe('ComposeService - withRegistryAuth', () => {
@@ -364,6 +364,24 @@ describe('DockerController - pruneSystem', () => {
});
});
// ── pruneDanglingImages ────────────────────────────────────────────────
describe('DockerController - pruneDanglingImages', () => {
it('prunes only dangling images and returns reclaimed bytes', async () => {
mockDocker.pruneImages.mockResolvedValue({ SpaceReclaimed: 5000 });
const dc = DockerController.getInstance(1);
const result = await dc.pruneDanglingImages();
// dangling:true keeps the prune to untagged layers, unlike pruneSystem('images')
// which uses dangling:false to remove every unused image.
expect(mockDocker.pruneImages).toHaveBeenCalledWith({
filters: { dangling: { 'true': true } },
});
expect(result).toEqual({ success: true, reclaimedBytes: 5000 });
});
});
// ── getClassifiedResources ─────────────────────────────────────────────
describe('DockerController - getClassifiedResources', () => {
@@ -6,7 +6,7 @@
* recovery will be unavailable" at boot.
*/
import { describe, expect, it } from 'vitest';
import { findDataDirHost } from '../services/SelfUpdateService';
import { buildSelfUpdateComposeCmd, findDataDirHost, shQuote } from '../services/SelfUpdateService';
describe('findDataDirHost', () => {
it('returns the host path for a bind mount at /app/data', () => {
@@ -54,3 +54,48 @@ describe('findDataDirHost', () => {
expect(source).toBeNull();
});
});
describe('buildSelfUpdateComposeCmd', () => {
const fFlags = ['-f', '/app/docker-compose.yml'];
const stderrTmp = '/tmp/_sencho_err';
const errorFile = '/app/data/.sencho-update-error';
it('appends a success-guarded dangling-image prune when pruneOnUpdate is true', () => {
const cmd = buildSelfUpdateComposeCmd(fFlags, 'sencho', stderrTmp, errorFile, true);
expect(cmd).toContain('if [ $ec -eq 0 ]; then docker image prune -f');
// The prune suppresses its own output and `|| true` so it can never alter
// the helper exit code; the command still ends on exit $ec.
expect(cmd).toContain('docker image prune -f >/dev/null 2>&1 || true');
expect(cmd.trim().endsWith('exit $ec')).toBe(true);
// Order matters: the prune must run after $ec is captured and after the
// error-file write, or it could shadow the recreate's exit code / clobber
// the error file. Lock the ordering, not just the presence of the line.
expect(cmd.indexOf('ec=$?')).toBeLessThan(cmd.indexOf('docker image prune'));
expect(cmd.indexOf(errorFile)).toBeLessThan(cmd.indexOf('docker image prune'));
});
it('omits the prune entirely when pruneOnUpdate is false', () => {
const cmd = buildSelfUpdateComposeCmd(fFlags, 'sencho', stderrTmp, errorFile, false);
expect(cmd).not.toContain('docker image prune');
});
it('always recreates the service and persists the error file on failure', () => {
const cmd = buildSelfUpdateComposeCmd(fFlags, 'sencho', stderrTmp, errorFile, true);
expect(cmd).toContain(`up -d --force-recreate ${shQuote('sencho')}`);
expect(cmd).toContain(`> ${errorFile}`);
});
it('shell-quotes label-derived values so metacharacters cannot break the command', () => {
// serviceName and config paths come from Docker Compose labels; a hostile
// label must stay inert data, not run as a second command.
const evilFlags = ['-f', '/tmp/compose.yml; ec=0; #'];
const cmd = buildSelfUpdateComposeCmd(evilFlags, 'svc; rm -rf /', stderrTmp, errorFile, true);
// The dangerous text survives only inside single quotes, never as bare shell.
expect(cmd).toContain(shQuote('/tmp/compose.yml; ec=0; #'));
expect(cmd).toContain(shQuote('svc; rm -rf /'));
expect(cmd).not.toContain('up -d --force-recreate svc; rm -rf /');
// The recreate line stays intact: its redirection and the real exit-code
// capture follow the quoted args, so the injected `ec=0` never runs as shell.
expect(cmd).toContain(`2>${stderrTmp}; ec=$?;`);
});
});
@@ -203,6 +203,39 @@ describe('POST /api/settings (single-key write)', () => {
});
});
describe('prune_on_update (auto-prune after updates)', () => {
it('defaults to ON in a freshly seeded database', () => {
expect(DatabaseService.getInstance().getGlobalSettings().prune_on_update).toBe('1');
});
it('is exposed through the settings GET projection', async () => {
const res = await request(app).get('/api/settings').set('Cookie', adminCookie);
expect(res.status).toBe(200);
expect(res.body.prune_on_update).toBeDefined();
});
it('accepts a well-formed prune_on_update write and persists it', async () => {
const res = await request(app)
.post('/api/settings')
.set('Cookie', adminCookie)
.send({ key: 'prune_on_update', value: '0' });
expect(res.status).toBe(200);
expect(DatabaseService.getInstance().getGlobalSettings().prune_on_update).toBe('0');
// Restore the seeded default so later suites observe the shipped behavior.
DatabaseService.getInstance().updateGlobalSetting('prune_on_update', '1');
});
it('rejects a non-enum prune_on_update value (400) and does not write it', async () => {
const res = await request(app)
.post('/api/settings')
.set('Cookie', adminCookie)
.send({ key: 'prune_on_update', value: 'banana' });
expect(res.status).toBe(400);
expect(res.body.error).toBe('Validation failed');
expect(DatabaseService.getInstance().getGlobalSettings().prune_on_update).not.toBe('banana');
});
});
describe('PATCH /api/settings (bulk update)', () => {
it('rejects unauthenticated requests with 401', async () => {
const res = await request(app).patch('/api/settings').send({ host_cpu_limit: 50 });
+2
View File
@@ -24,6 +24,7 @@ const ALLOWED_SETTING_KEYS = new Set([
'audit_retention_days',
'mesh_auto_recreate',
'scan_history_per_image_limit',
'prune_on_update',
]);
// Keys whose write requires a paid license, not just an admin role.
@@ -46,6 +47,7 @@ const SettingsPatchSchema = z.object({
audit_retention_days: z.coerce.number().int().min(1).max(365).transform(String),
mesh_auto_recreate: z.enum(['0', '1']),
scan_history_per_image_limit: z.coerce.number().int().min(5).max(1000).transform(String),
prune_on_update: z.enum(['0', '1']),
}).partial();
export const settingsRouter = Router();
+18
View File
@@ -527,6 +527,24 @@ export class ComposeService {
}
sendOutput('=== Stack updated successfully ===\n');
// Opt-out (default ON): after a clean update, prune the node's dangling
// (untagged) image layers, including the one this pull just orphaned. Read
// fresh each run so a remote node honors its own setting. Wrapped so a
// prune failure can never reach the atomic-rollback catch below.
try {
const pruneOnUpdate = DatabaseService.getInstance().getGlobalSettings()['prune_on_update'] === '1';
if (pruneOnUpdate) {
const result = await DockerController.getInstance(this.nodeId).pruneDanglingImages();
// The Docker prune API does not report SpaceReclaimed on the containerd
// image store, so only show the figure when the daemon actually returns one.
const reclaimed = result.reclaimedBytes > 0
? ` · reclaimed ${(result.reclaimedBytes / (1024 * 1024)).toFixed(1)} MB`
: '';
sendOutput(`=== Pruned dangling images${reclaimed} ===\n`);
}
} catch (pruneError) {
console.warn('Failed to prune dangling images after update for %s:', sanitizeForLog(stackName), pruneError);
}
if (debug) console.debug(`[ComposeService:debug] updateStack completed in ${Date.now() - t0}ms`, { stackName });
} catch (updateError) {
if (atomic) {
+1
View File
@@ -1255,6 +1255,7 @@ export class DatabaseService {
stmt.run('trivy_last_notified_version', '');
stmt.run('deploy_block_honor_suppressions', '0');
stmt.run('mesh_auto_recreate', '0');
stmt.run('prune_on_update', '1');
// Seed the default local node if none exists
const nodeCount = (this.db.prepare('SELECT COUNT(*) as count FROM nodes').get() as any)?.count || 0;
+10
View File
@@ -263,6 +263,16 @@ class DockerController {
};
}
// Prune ONLY dangling (untagged) images. Distinct from pruneSystem('images'),
// which uses { dangling: { 'false': true } } to remove every unused image.
// Used by the prune-on-update flow to reclaim the layers a pull/recreate
// orphans, without touching tagged images for stopped stacks.
public async pruneDanglingImages(): Promise<{ success: boolean; reclaimedBytes: number }> {
const filters: Record<string, string[] | Record<string, boolean>> = { dangling: { 'true': true } };
const r = await this.docker.pruneImages({ filters });
return { success: true, reclaimedBytes: r.SpaceReclaimed || 0 };
}
public async getImages() {
const data = await this.docker.listImages({ all: false });
return this.validateApiData<any[]>(data);
+45 -8
View File
@@ -2,6 +2,7 @@ import { execFile } from 'child_process';
import { promisify } from 'util';
import * as fs from 'fs';
import DockerController from './DockerController';
import { DatabaseService } from './DatabaseService';
import { disableCapability } from './CapabilityRegistry';
import { isDebugEnabled } from '../utils/debug';
@@ -39,6 +40,45 @@ export function findDataDirHost(mounts: ReadonlyArray<DockerMount>): string | nu
return match?.Source ?? null;
}
// POSIX single-quote escaping. serviceName and the compose config paths in
// fFlags come from Docker Compose labels on Sencho's own container, so a label
// carrying shell metacharacters must not be able to break out of the command
// sequence (the exit-code capture, error-file write, and prune guard).
export function shQuote(value: string): string {
return `'${value.replace(/'/g, `'\\''`)}'`;
}
/**
* Build the shell command the helper container runs to recreate Sencho. Kept as
* a pure, exported function so the prune-on-update branch is unit-testable.
*
* Label-derived inputs (serviceName, the fFlags config paths) are shell-quoted
* so they cannot alter the command structure. The recreate writes the error
* file only on failure; the optional dangling prune runs only on success, so
* the two branches never overlap. The prune suppresses its own output and
* `|| true`, so it can never alter $ec or be mistaken for an update error.
*/
export function buildSelfUpdateComposeCmd(
fFlags: string[],
serviceName: string,
stderrTmp: string,
errorFile: string,
pruneOnUpdate: boolean,
): string {
const recreate = ['docker compose', ...fFlags.map(shQuote), 'up -d --force-recreate', shQuote(serviceName), `2>${stderrTmp}`].join(' ');
return [
'sleep 3',
recreate,
'ec=$?',
`if [ $ec -ne 0 ]; then { echo "exit=$ec"; cat ${stderrTmp}; } > ${errorFile} 2>/dev/null; fi`,
...(pruneOnUpdate
? [`if [ $ec -eq 0 ]; then docker image prune -f >/dev/null 2>&1 || true; fi`]
: []),
`cat ${stderrTmp} >&2 2>/dev/null`,
'exit $ec',
].join('; ');
}
interface ComposeContext {
workingDir: string;
configFiles: string;
@@ -198,15 +238,12 @@ class SelfUpdateService {
// On failure, persist exit code + stderr to UPDATE_ERROR_FILE (host-mounted)
// so the NEW gateway can read it after restart if we die mid-execution.
// Opt-out (default ON): after a clean recreate, prune the dangling image
// layers the pull orphaned. Read fresh so this node honors its own setting.
const stderrTmp = '/tmp/_sencho_err';
const composeCmd = [
'sleep 3',
['docker compose', ...fFlags, 'up -d --force-recreate', serviceName, `2>${stderrTmp}`].join(' '),
'ec=$?',
`if [ $ec -ne 0 ]; then { echo "exit=$ec"; cat ${stderrTmp}; } > ${UPDATE_ERROR_FILE} 2>/dev/null; fi`,
`cat ${stderrTmp} >&2 2>/dev/null`,
'exit $ec',
].join('; ');
const pruneOnUpdate =
DatabaseService.getInstance().getGlobalSettings()['prune_on_update'] === '1';
const composeCmd = buildSelfUpdateComposeCmd(fFlags, serviceName, stderrTmp, UPDATE_ERROR_FILE, pruneOnUpdate);
const mountArgs: string[] = [
'-v', '/var/run/docker.sock:/var/run/docker.sock',