fix(stacks): resolve permission denied error on stack deletion (#261)

* fix(stacks): resolve permission denied error when deleting stacks with root-owned files

When Docker Compose creates files as root inside a stack directory, the
non-root Sencho process cannot remove them. This adds a Docker-based
fallback: if fsPromises.rm fails with EACCES/EPERM, Sencho spawns a
short-lived Alpine container to clean up the root-owned files.

Also enhances docker compose down with --volumes --remove-orphans to let
Docker clean up its own resources before filesystem deletion.

* docs: clarify that pre-existing root-owned stacks can be deleted

* fix(stacks): include Docker stderr in fallback deletion error message

Fixes CI lint failure: 'stderr' was assigned but never read in
forceDeleteViaDocker(). Now surfaces Docker stderr output in the error
message when the fallback cleanup fails.
This commit is contained in:
Anso
2026-03-29 21:32:05 -04:00
committed by GitHub
parent f760ea6563
commit 116f15dae9
6 changed files with 272 additions and 6 deletions
+175
View File
@@ -0,0 +1,175 @@
/**
* Unit tests for FileSystemService.deleteStack() including the
* Docker-based fallback for permission-denied scenarios.
*/
import { describe, it, expect, vi, beforeEach } from 'vitest';
import path from 'path';
import { EventEmitter } from 'events';
const { mockSpawn, mockRm, mockRmdir } = vi.hoisted(() => ({
mockSpawn: vi.fn(),
mockRm: vi.fn(),
mockRmdir: vi.fn(),
}));
vi.mock('child_process', () => ({ spawn: mockSpawn }));
vi.mock('fs', () => ({
promises: {
rm: mockRm,
rmdir: mockRmdir,
mkdir: vi.fn(),
readdir: vi.fn(),
readFile: vi.fn(),
writeFile: vi.fn(),
access: vi.fn(),
stat: vi.fn(),
rename: vi.fn(),
copyFile: vi.fn(),
unlink: vi.fn(),
},
}));
vi.mock('../services/NodeRegistry', () => ({
NodeRegistry: {
getInstance: () => ({
getComposeDir: () => '/test/compose',
getDefaultNodeId: () => 1,
}),
},
}));
import { FileSystemService } from '../services/FileSystemService';
function createMockProcess() {
const proc = new EventEmitter() as EventEmitter & {
stdout: EventEmitter;
stderr: EventEmitter;
kill: ReturnType<typeof vi.fn>;
};
proc.stdout = new EventEmitter();
proc.stderr = new EventEmitter();
proc.kill = vi.fn();
return proc;
}
const expectedDir = path.join('/test/compose', 'my-stack');
describe('FileSystemService.deleteStack', () => {
let service: FileSystemService;
beforeEach(() => {
vi.clearAllMocks();
service = FileSystemService.getInstance();
});
it('deletes a stack directory successfully via fsPromises.rm', async () => {
mockRm.mockResolvedValueOnce(undefined);
await service.deleteStack('my-stack');
expect(mockRm).toHaveBeenCalledWith(expectedDir, { recursive: true, force: true });
});
it('silently ignores ENOENT (directory already gone)', async () => {
const err = Object.assign(new Error('ENOENT'), { code: 'ENOENT' });
mockRm.mockRejectedValueOnce(err);
await expect(service.deleteStack('gone-stack')).resolves.toBeUndefined();
});
it('falls back to Docker removal on EACCES', async () => {
const err = Object.assign(new Error('permission denied'), { code: 'EACCES' });
mockRm.mockRejectedValueOnce(err);
mockRmdir.mockResolvedValueOnce(undefined);
const proc = createMockProcess();
mockSpawn.mockReturnValueOnce(proc);
const promise = service.deleteStack('restricted-stack');
// Emit close asynchronously so listeners are attached first
await vi.waitFor(() => {
expect(mockSpawn).toHaveBeenCalled();
});
proc.emit('close', 0);
await expect(promise).resolves.toBeUndefined();
expect(mockSpawn).toHaveBeenCalledWith(
'docker',
expect.arrayContaining(['run', '--rm', '-v', expect.stringContaining(':/cleanup'), 'alpine', 'rm', '-rf', '/cleanup']),
expect.objectContaining({ env: expect.any(Object) }),
);
});
it('falls back to Docker removal on EPERM', async () => {
const err = Object.assign(new Error('operation not permitted'), { code: 'EPERM' });
mockRm.mockRejectedValueOnce(err);
mockRmdir.mockResolvedValueOnce(undefined);
const proc = createMockProcess();
mockSpawn.mockReturnValueOnce(proc);
const promise = service.deleteStack('eperm-stack');
await vi.waitFor(() => {
expect(mockSpawn).toHaveBeenCalled();
});
proc.emit('close', 0);
await expect(promise).resolves.toBeUndefined();
});
it('throws descriptive error when Docker fallback fails', async () => {
const err = Object.assign(new Error('permission denied'), { code: 'EACCES' });
mockRm.mockRejectedValueOnce(err);
const proc = createMockProcess();
mockSpawn.mockReturnValueOnce(proc);
const promise = service.deleteStack('stuck-stack');
await vi.waitFor(() => {
expect(mockSpawn).toHaveBeenCalled();
});
proc.stderr.emit('data', Buffer.from('container error'));
proc.emit('close', 1);
await expect(promise).rejects.toThrow(/Docker cleanup exited with code 1/);
});
it('throws descriptive error when Docker is unavailable', async () => {
const err = Object.assign(new Error('permission denied'), { code: 'EACCES' });
mockRm.mockRejectedValueOnce(err);
const proc = createMockProcess();
mockSpawn.mockReturnValueOnce(proc);
const promise = service.deleteStack('no-docker-stack');
await vi.waitFor(() => {
expect(mockSpawn).toHaveBeenCalled();
});
proc.emit('error', new Error('spawn docker ENOENT'));
await expect(promise).rejects.toThrow(/could not run Docker for cleanup/);
});
it('still succeeds if rmdir of empty shell fails after Docker cleanup', async () => {
const err = Object.assign(new Error('permission denied'), { code: 'EACCES' });
mockRm.mockRejectedValueOnce(err);
mockRmdir.mockRejectedValueOnce(new Error('rmdir failed'));
const proc = createMockProcess();
mockSpawn.mockReturnValueOnce(proc);
const promise = service.deleteStack('partial-cleanup');
await vi.waitFor(() => {
expect(mockSpawn).toHaveBeenCalled();
});
proc.emit('close', 0);
await expect(promise).resolves.toBeUndefined();
});
it('throws on unexpected errors (not ENOENT/EACCES/EPERM)', async () => {
const err = Object.assign(new Error('disk I/O error'), { code: 'EIO' });
mockRm.mockRejectedValueOnce(err);
await expect(service.deleteStack('io-error-stack')).rejects.toThrow(/disk I\/O error/);
expect(mockSpawn).not.toHaveBeenCalled();
});
});
+1 -1
View File
@@ -378,7 +378,7 @@ export class ComposeService {
public async downStack(stackName: string): Promise<void> {
const stackPath = path.join(this.baseDir, stackName);
try {
await this.execute('docker', ['compose', 'down'], stackPath, undefined, false);
await this.execute('docker', ['compose', 'down', '--volumes', '--remove-orphans'], stackPath, undefined, false);
} catch (error) {
console.warn(`[Teardown] Docker down failed or nothing to clean up for ${stackName}`);
}
+64 -4
View File
@@ -1,5 +1,6 @@
import path from 'path';
import { promises as fsPromises } from 'fs';
import { spawn } from 'child_process';
import { NodeRegistry } from './NodeRegistry';
/**
@@ -174,14 +175,73 @@ export class FileSystemService {
try {
await fsPromises.rm(stackDir, { recursive: true, force: true });
console.log('Stack deleted successfully:', stackName);
} catch (error: any) {
if (error.code !== 'ENOENT') {
console.error('Error deleting stack directory:', error.message);
throw new Error(`Failed to delete stack directory: ${error.message}`);
} catch (error: unknown) {
const fsError = error as NodeJS.ErrnoException;
if (fsError.code === 'ENOENT') return;
if (fsError.code === 'EACCES' || fsError.code === 'EPERM') {
console.warn(
`[FileSystemService] Permission denied deleting ${stackName}, falling back to Docker-based removal`
);
await this.forceDeleteViaDocker(stackDir);
// Docker removes contents but can't remove its own mount point; clean up the empty shell
try {
await fsPromises.rmdir(stackDir);
} catch {
console.warn(`[FileSystemService] Could not remove empty directory ${stackDir} — may need manual cleanup`);
}
console.log('Stack deleted successfully (via Docker fallback):', stackName);
} else {
console.error('Error deleting stack directory:', fsError.message);
throw new Error(`Failed to delete stack directory: ${fsError.message}`);
}
}
}
private forceDeleteViaDocker(dirPath: string): Promise<void> {
return new Promise((resolve, reject) => {
const timeout = 30_000;
const child = spawn('docker', [
'run', '--rm',
'-v', `${dirPath}:/cleanup`,
'alpine',
'rm', '-rf', '/cleanup'
], {
env: {
...process.env,
PATH: process.env.PATH || '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin'
}
});
let stderr = '';
child.stderr.on('data', (data: Buffer) => { stderr += data.toString(); });
const timer = setTimeout(() => {
child.kill();
reject(new Error(
`Docker-based deletion timed out after 30s. You may need to manually remove the directory: ${dirPath}`
));
}, timeout);
child.on('close', (code: number | null) => {
clearTimeout(timer);
if (code === 0) resolve();
else reject(new Error(
`Failed to delete stack directory — Docker cleanup exited with code ${code}${stderr ? ': ' + stderr.trim() : ''}. ` +
`You may need to manually remove the directory: ${dirPath}`
));
});
child.on('error', (err: Error) => {
clearTimeout(timer);
reject(new Error(
`Failed to delete stack directory — could not run Docker for cleanup: ${err.message}. ` +
`You may need to manually remove the directory: ${dirPath}`
));
});
});
}
getBaseDir(): string {
return this.baseDir;
}