mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-05 16:37:46 +00:00
fix(deploy): preserve compose.override.yml when Mesh is enabled (#1420)
When a single-file stack is opted into Sencho Mesh, the deploy builds an explicit `docker compose -f <base> -f <mesh override>` list. Passing any explicit -f disables Compose's automatic discovery of compose.override.yml (and the docker-compose.override variants), so a user's hand-authored override was silently dropped from the effective deploy once Mesh was on. Resolve the user's override file (first existing variant, with the same stack-name and symlink-containment guards as the base compose file) and insert it between the base and the mesh override, so it layers exactly as Compose's implicit discovery would, with the mesh override still taking precedence. A transient read failure during the lookup degrades to "no override" rather than failing the deploy; a stack-name or containment-guard rejection still aborts. Multi-file Git-source stacks and non-mesh deploys are unaffected.
This commit is contained in:
@@ -15,6 +15,7 @@ const {
|
||||
mockContainerInspect, mockContainerLogs,
|
||||
mockGetRegistries, mockResolveDockerConfig,
|
||||
mockBackupStackFiles, mockRestoreStackFiles,
|
||||
mockGetComposeFilename, mockGetOverrideFilename, mockEnsureStackOverride,
|
||||
mockMkdtempSync, mockWriteFileSync, mockUnlinkSync, mockRmdirSync,
|
||||
mockGetGlobalSettings, mockPruneDanglingImages,
|
||||
} = vi.hoisted(() => ({
|
||||
@@ -28,6 +29,9 @@ const {
|
||||
mockResolveDockerConfig: vi.fn().mockResolvedValue({ config: { auths: {} }, warnings: [] }),
|
||||
mockBackupStackFiles: vi.fn().mockResolvedValue(undefined),
|
||||
mockRestoreStackFiles: vi.fn().mockResolvedValue(undefined),
|
||||
mockGetComposeFilename: vi.fn().mockResolvedValue('compose.yaml'),
|
||||
mockGetOverrideFilename: vi.fn().mockResolvedValue(null),
|
||||
mockEnsureStackOverride: vi.fn().mockResolvedValue(null),
|
||||
mockMkdtempSync: vi.fn().mockReturnValue('/tmp/sencho-docker-test'),
|
||||
mockWriteFileSync: vi.fn(),
|
||||
mockUnlinkSync: vi.fn(),
|
||||
@@ -100,6 +104,8 @@ vi.mock('../services/FileSystemService', () => ({
|
||||
getInstance: () => ({
|
||||
backupStackFiles: mockBackupStackFiles,
|
||||
restoreStackFiles: mockRestoreStackFiles,
|
||||
getComposeFilename: mockGetComposeFilename,
|
||||
getOverrideFilename: mockGetOverrideFilename,
|
||||
}),
|
||||
},
|
||||
}));
|
||||
@@ -109,11 +115,12 @@ vi.mock('../services/LogFormatter', () => ({
|
||||
}));
|
||||
|
||||
// runCommand and the deploy/update paths route through authoredComposeArgs, which
|
||||
// resolves the (optional) mesh override. Stub it to "no override" so a single-file
|
||||
// stack yields plain `docker compose <action>` args deterministically.
|
||||
// resolves the (optional) mesh override. The hoisted mock defaults to "no override"
|
||||
// so a single-file stack yields plain `docker compose <action>` args deterministically;
|
||||
// individual tests set a path to exercise the mesh-injection branch.
|
||||
vi.mock('../services/MeshService', () => ({
|
||||
MeshService: {
|
||||
getInstance: () => ({ ensureStackOverride: vi.fn().mockResolvedValue(null) }),
|
||||
getInstance: () => ({ ensureStackOverride: mockEnsureStackOverride }),
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -181,6 +188,13 @@ function createMockWs(): MockWebSocket {
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
// clearAllMocks() clears call records but not implementations, so a mockResolvedValue
|
||||
// set by one test persists into the next. Re-assert the safe "no mesh override, no user
|
||||
// override, base = compose.yaml" baseline here so a stray override from an earlier test
|
||||
// cannot leak forward and add phantom -f flags.
|
||||
mockGetComposeFilename.mockResolvedValue('compose.yaml');
|
||||
mockGetOverrideFilename.mockResolvedValue(null);
|
||||
mockEnsureStackOverride.mockResolvedValue(null);
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
});
|
||||
|
||||
@@ -376,6 +390,142 @@ describe('ComposeService - runCommand', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ── authoredComposeArgs: mesh + user override ──────────────────────────
|
||||
|
||||
describe('ComposeService - authoredComposeArgs mesh override', () => {
|
||||
const MESH_OVERRIDE = '/app/data/mesh/overrides/1/my-stack.override.yml';
|
||||
|
||||
it('preserves a user compose.override.yml between the base and the mesh override', async () => {
|
||||
// Single-file stack opted into mesh, with a hand-authored override on disk.
|
||||
mockEnsureStackOverride.mockResolvedValue(MESH_OVERRIDE);
|
||||
mockGetComposeFilename.mockResolvedValue('compose.yaml');
|
||||
mockGetOverrideFilename.mockResolvedValue('compose.override.yml');
|
||||
|
||||
const proc = createMockProcess();
|
||||
mockSpawn.mockReturnValue(proc);
|
||||
|
||||
const svc = ComposeService.getInstance(1);
|
||||
const promise = svc.runCommand('my-stack', 'restart');
|
||||
await waitForSpawn();
|
||||
proc.emit('close', 0);
|
||||
await promise;
|
||||
|
||||
// The user override sits between the base and the mesh override as a bare basename
|
||||
// (resolved against the stack-dir cwd); only the mesh override is an absolute path.
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'docker',
|
||||
['compose', '-f', 'compose.yaml', '-f', 'compose.override.yml', '-f', MESH_OVERRIDE, 'restart'],
|
||||
expect.objectContaining({ cwd: expect.stringContaining('my-stack') })
|
||||
);
|
||||
});
|
||||
|
||||
it('emits base + mesh override only when no user override exists', async () => {
|
||||
mockEnsureStackOverride.mockResolvedValue(MESH_OVERRIDE);
|
||||
mockGetComposeFilename.mockResolvedValue('compose.yaml');
|
||||
mockGetOverrideFilename.mockResolvedValue(null);
|
||||
|
||||
const proc = createMockProcess();
|
||||
mockSpawn.mockReturnValue(proc);
|
||||
|
||||
const svc = ComposeService.getInstance(1);
|
||||
const promise = svc.runCommand('my-stack', 'restart');
|
||||
await waitForSpawn();
|
||||
proc.emit('close', 0);
|
||||
await promise;
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'docker',
|
||||
['compose', '-f', 'compose.yaml', '-f', MESH_OVERRIDE, 'restart'],
|
||||
expect.objectContaining({ cwd: expect.stringContaining('my-stack') })
|
||||
);
|
||||
});
|
||||
|
||||
it('does not look up or emit a user override when mesh is disabled', async () => {
|
||||
// A user override on disk must not introduce -f flags for a non-mesh stack;
|
||||
// implicit compose discovery already resolves it when no -f is passed.
|
||||
mockEnsureStackOverride.mockResolvedValue(null);
|
||||
mockGetOverrideFilename.mockResolvedValue('compose.override.yml');
|
||||
|
||||
const proc = createMockProcess();
|
||||
mockSpawn.mockReturnValue(proc);
|
||||
|
||||
const svc = ComposeService.getInstance(1);
|
||||
const promise = svc.runCommand('my-stack', 'restart');
|
||||
await waitForSpawn();
|
||||
proc.emit('close', 0);
|
||||
await promise;
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'docker',
|
||||
['compose', 'restart'],
|
||||
expect.objectContaining({ cwd: expect.stringContaining('my-stack') })
|
||||
);
|
||||
// The override lookup is gated inside the mesh branch.
|
||||
expect(mockGetOverrideFilename).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('drops the user override and still deploys when the lookup throws', async () => {
|
||||
// A present override that cannot be resolved (e.g. EACCES) must not crash the
|
||||
// deploy: the mesh override still applies and the deploy proceeds without the
|
||||
// user override, with a warning logged.
|
||||
mockEnsureStackOverride.mockResolvedValue(MESH_OVERRIDE);
|
||||
mockGetComposeFilename.mockResolvedValue('compose.yaml');
|
||||
mockGetOverrideFilename.mockRejectedValue(Object.assign(new Error('EACCES'), { code: 'EACCES' }));
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
|
||||
|
||||
const proc = createMockProcess();
|
||||
mockSpawn.mockReturnValue(proc);
|
||||
|
||||
const svc = ComposeService.getInstance(1);
|
||||
const promise = svc.runCommand('my-stack', 'restart');
|
||||
await waitForSpawn();
|
||||
proc.emit('close', 0);
|
||||
await promise;
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'docker',
|
||||
['compose', '-f', 'compose.yaml', '-f', MESH_OVERRIDE, 'restart'],
|
||||
expect.objectContaining({ cwd: expect.stringContaining('my-stack') })
|
||||
);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('could not resolve user compose override'),
|
||||
expect.anything()
|
||||
);
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('aborts the deploy when the override lookup hits a containment-guard rejection', async () => {
|
||||
// A symlink-escape (or invalid-name) rejection from the override lookup is a hard
|
||||
// error: it must propagate and abort the deploy, never degrade to "no override".
|
||||
mockEnsureStackOverride.mockResolvedValue(MESH_OVERRIDE);
|
||||
mockGetComposeFilename.mockResolvedValue('compose.yaml');
|
||||
mockGetOverrideFilename.mockRejectedValue(Object.assign(new Error('symlink escape'), { code: 'SYMLINK_ESCAPE' }));
|
||||
|
||||
const svc = ComposeService.getInstance(1);
|
||||
await expect(svc.runCommand('my-stack', 'restart')).rejects.toMatchObject({ code: 'SYMLINK_ESCAPE' });
|
||||
// The error is thrown while building the args, before docker is ever spawned.
|
||||
expect(mockSpawn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('leak sanity: a default single-file stack still emits no -f flags', async () => {
|
||||
// Proves the override-setting tests above do not leak through the shared mocks.
|
||||
const proc = createMockProcess();
|
||||
mockSpawn.mockReturnValue(proc);
|
||||
|
||||
const svc = ComposeService.getInstance(1);
|
||||
const promise = svc.runCommand('my-stack', 'restart');
|
||||
await waitForSpawn();
|
||||
proc.emit('close', 0);
|
||||
await promise;
|
||||
|
||||
expect(mockSpawn).toHaveBeenCalledWith(
|
||||
'docker',
|
||||
['compose', 'restart'],
|
||||
expect.objectContaining({ cwd: expect.stringContaining('my-stack') })
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ── deployStack ────────────────────────────────────────────────────────
|
||||
|
||||
describe('ComposeService - deployStack', () => {
|
||||
|
||||
@@ -125,6 +125,12 @@ describe.skipIf(isWindows)('FileSystemService symlink-escape: symlinked stack di
|
||||
await expect(svc.getStackContentWithMtime(STACK)).rejects.toMatchObject({ code: 'SYMLINK_ESCAPE' });
|
||||
});
|
||||
|
||||
it('getOverrideFilename rejects instead of resolving an out-of-tree override', async () => {
|
||||
await fs.writeFile(path.join(externalTarget, 'compose.override.yml'), 'services: {}\n', 'utf-8');
|
||||
const svc = FileSystemService.getInstance();
|
||||
await expect(svc.getOverrideFilename(STACK)).rejects.toMatchObject({ code: 'SYMLINK_ESCAPE' });
|
||||
});
|
||||
|
||||
it('generic readFile/writeFile/access through the symlinked stack dir reject', async () => {
|
||||
const svc = FileSystemService.getInstance();
|
||||
const envAbs = path.join(composeDir, STACK, '.env');
|
||||
|
||||
@@ -10,9 +10,11 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
|
||||
const { mockRm, mockReaddir } = vi.hoisted(() => ({
|
||||
const { mockRm, mockReaddir, mockAccess, mockRealpath } = vi.hoisted(() => ({
|
||||
mockRm: vi.fn(),
|
||||
mockReaddir: vi.fn(),
|
||||
mockAccess: vi.fn(),
|
||||
mockRealpath: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('fs', () => ({
|
||||
@@ -22,17 +24,18 @@ vi.mock('fs', () => ({
|
||||
readdir: mockReaddir,
|
||||
readFile: vi.fn(),
|
||||
writeFile: vi.fn(),
|
||||
access: vi.fn(),
|
||||
access: mockAccess,
|
||||
stat: vi.fn(),
|
||||
rename: vi.fn(),
|
||||
copyFile: vi.fn(),
|
||||
unlink: vi.fn(),
|
||||
// deleteStack now realpath-checks the stack dir against the compose root
|
||||
// before rm. Resolve to the absolute path (no symlink) so the containment
|
||||
// guard passes and these tests still exercise the rm error translation;
|
||||
// the guard's symlink-escape behaviour is covered in
|
||||
// filesystem-symlink-escape.test.ts.
|
||||
realpath: vi.fn((p: string) => Promise.resolve(path.resolve(p))),
|
||||
// deleteStack and the compose/override resolvers realpath-check the stack dir
|
||||
// against the compose root before touching it. realpath resolves to the absolute
|
||||
// path (no symlink) so the containment guard passes; getOverrideFilename tests drive
|
||||
// `access` per-path. The guard's symlink-escape behaviour is covered in
|
||||
// filesystem-symlink-escape.test.ts. clearAllMocks() preserves this implementation,
|
||||
// so it stays set across every test (set just below the imports).
|
||||
realpath: mockRealpath,
|
||||
},
|
||||
}));
|
||||
|
||||
@@ -47,6 +50,11 @@ vi.mock('../services/NodeRegistry', () => ({
|
||||
|
||||
import { FileSystemService } from '../services/FileSystemService';
|
||||
|
||||
// realpath resolves any path to its absolute form (no symlink), so the containment
|
||||
// guard passes for the in-base stack dirs these tests use. Defined here (not in the
|
||||
// hoisted block, which runs before `path` is bound) and preserved across clearAllMocks().
|
||||
mockRealpath.mockImplementation((p: string) => Promise.resolve(path.resolve(p)));
|
||||
|
||||
const expectedDir = path.join('/test/compose', 'my-stack');
|
||||
|
||||
describe('FileSystemService.deleteStack', () => {
|
||||
@@ -133,3 +141,66 @@ describe('FileSystemService.getStacks', () => {
|
||||
expect(warning).not.toContain('host free memory');
|
||||
});
|
||||
});
|
||||
|
||||
describe('FileSystemService.getOverrideFilename', () => {
|
||||
let service: FileSystemService;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
// Re-assert the in-base realpath default after clearAllMocks so the symlink-escape
|
||||
// case below (which overrides realpath) cannot leak its escape into a later test.
|
||||
mockRealpath.mockImplementation((p: string) => Promise.resolve(path.resolve(p)));
|
||||
service = FileSystemService.getInstance();
|
||||
});
|
||||
|
||||
// Make fsPromises.access resolve only for the named basenames (file exists) and
|
||||
// reject with ENOENT otherwise, so a test controls exactly which variants are present.
|
||||
function existing(...names: string[]): void {
|
||||
const present = new Set(names);
|
||||
mockAccess.mockImplementation((p: string) =>
|
||||
present.has(path.basename(p))
|
||||
? Promise.resolve(undefined)
|
||||
: Promise.reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })),
|
||||
);
|
||||
}
|
||||
|
||||
it('returns the first existing override variant in priority order', async () => {
|
||||
// compose.override.yaml is absent, so the next priority (.yml) wins over the
|
||||
// lower-priority docker-compose.override.yml that also exists.
|
||||
existing('compose.override.yml', 'docker-compose.override.yml');
|
||||
await expect(service.getOverrideFilename('my-stack')).resolves.toBe('compose.override.yml');
|
||||
});
|
||||
|
||||
it('prefers compose.override.yaml over all lower-priority variants', async () => {
|
||||
existing('compose.override.yaml', 'compose.override.yml', 'docker-compose.override.yml');
|
||||
await expect(service.getOverrideFilename('my-stack')).resolves.toBe('compose.override.yaml');
|
||||
});
|
||||
|
||||
it('returns null when no override variant exists', async () => {
|
||||
existing();
|
||||
await expect(service.getOverrideFilename('my-stack')).resolves.toBeNull();
|
||||
});
|
||||
|
||||
it('returns a bare basename, never an absolute path', async () => {
|
||||
existing('docker-compose.override.yaml');
|
||||
const result = await service.getOverrideFilename('my-stack');
|
||||
expect(result).toBe('docker-compose.override.yaml');
|
||||
expect(path.isAbsolute(result as string)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects an invalid stack name before probing the disk', async () => {
|
||||
await expect(service.getOverrideFilename('../evil')).rejects.toMatchObject({ code: 'INVALID_STACK_NAME' });
|
||||
expect(mockAccess).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects a stack dir whose realpath escapes the compose root (symlink escape)', async () => {
|
||||
// The symlink-containment guard runs before any override probe, so a symlinked
|
||||
// stack dir cannot pull an override file from outside the compose root.
|
||||
const escaped = path.resolve('/totally/outside/evil');
|
||||
mockRealpath.mockImplementation((p: string) =>
|
||||
path.basename(p) === 'my-stack' ? Promise.resolve(escaped) : Promise.resolve(path.resolve(p)),
|
||||
);
|
||||
await expect(service.getOverrideFilename('my-stack')).rejects.toMatchObject({ code: 'SYMLINK_ESCAPE' });
|
||||
expect(mockAccess).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -117,10 +117,29 @@ export class ComposeService {
|
||||
}
|
||||
if (overridePath) {
|
||||
if (filePrefix.length === 0) {
|
||||
// Single-file stack: passing any -f disables auto-discovery, so name the
|
||||
// base file explicitly before layering the override on top of it.
|
||||
const baseFilename = await FileSystemService.getInstance(this.nodeId).getComposeFilename(stackName);
|
||||
// Single-file stack: passing any -f disables compose's auto-discovery, so name
|
||||
// the base file explicitly, then re-add the user's implicit override (if any) so
|
||||
// it is not silently dropped, before layering the mesh override on top.
|
||||
const fsSvc = FileSystemService.getInstance(this.nodeId);
|
||||
const baseFilename = await fsSvc.getComposeFilename(stackName);
|
||||
args.push('-f', baseFilename);
|
||||
let userOverride: string | null = null;
|
||||
try {
|
||||
userOverride = await fsSvc.getOverrideFilename(stackName);
|
||||
} catch (err) {
|
||||
// Containment-guard rejections (bad stack name / symlink escape) are hard errors:
|
||||
// abort the deploy rather than degrade. The "no override" case returns null rather
|
||||
// than throwing, so any other throw is transient I/O: drop the override and proceed
|
||||
// (logging the consequence) instead of failing the deploy.
|
||||
const code = (err as { code?: string }).code;
|
||||
if (code === 'INVALID_STACK_NAME' || code === 'INVALID_PATH' || code === 'SYMLINK_ESCAPE') {
|
||||
throw err;
|
||||
}
|
||||
console.warn('[ComposeService] could not resolve user compose override; deploying without it:', sanitizeForLog((err as Error).message));
|
||||
}
|
||||
if (userOverride) {
|
||||
args.push('-f', userOverride);
|
||||
}
|
||||
}
|
||||
args.push('-f', overridePath);
|
||||
}
|
||||
|
||||
@@ -60,6 +60,16 @@ const PROTECTED_STACK_FILES = new Set([
|
||||
// list FileSystemService uses elsewhere; named here for the import scan.
|
||||
const IMPORT_COMPOSE_FILENAMES = ['compose.yaml', 'compose.yml', 'docker-compose.yaml', 'docker-compose.yml'] as const;
|
||||
const IMPORT_COMPOSE_FILENAME_SET = new Set<string>(IMPORT_COMPOSE_FILENAMES);
|
||||
// Override filenames docker compose can auto-discover, listed in priority order (first
|
||||
// match wins, not paired to the chosen base file's family). We resolve the first that
|
||||
// exists, mirroring compose's default override resolution, to re-add it when an explicit
|
||||
// -f list (mesh injection) would otherwise suppress that discovery.
|
||||
const COMPOSE_OVERRIDE_FILENAMES = [
|
||||
'compose.override.yaml',
|
||||
'compose.override.yml',
|
||||
'docker-compose.override.yaml',
|
||||
'docker-compose.override.yml',
|
||||
] as const;
|
||||
// Skip reading compose files larger than this into the import preview.
|
||||
const IMPORT_MAX_PREVIEW_BYTES = 1_048_576; // 1 MiB
|
||||
|
||||
@@ -221,6 +231,34 @@ export class FileSystemService {
|
||||
return path.basename(await this.getComposeFilePath(stackName));
|
||||
}
|
||||
|
||||
/**
|
||||
* The stack's hand-authored compose override filename (bare basename, e.g.
|
||||
* `compose.override.yml`), or `null` when none exists. Mirrors how docker compose
|
||||
* itself resolves the default override: the first existing variant in priority order.
|
||||
* Callers building an explicit `-f` list (which suppresses compose's built-in override
|
||||
* discovery) use this to re-add the implicit override. Applies the same stack-name and
|
||||
* symlink-containment guards as `getComposeFilePath`.
|
||||
*/
|
||||
async getOverrideFilename(stackName: string): Promise<string | null> {
|
||||
const stackDir = this.resolveStackDir(stackName);
|
||||
await this.assertRealWithinBase(stackDir);
|
||||
// Canonical js/path-injection barrier inline with the access sink (same pattern as
|
||||
// envExists): stackName is already validated by resolveStackDir and assertRealWithinBase
|
||||
// above, but static analysis only credits the containment check when it sits at the sink.
|
||||
const baseResolved = path.resolve(this.baseDir);
|
||||
for (const file of COMPOSE_OVERRIDE_FILENAMES) {
|
||||
const target = path.resolve(stackDir, file);
|
||||
if (!target.startsWith(baseResolved + path.sep)) continue;
|
||||
try {
|
||||
await fsPromises.access(target);
|
||||
return file;
|
||||
} catch {
|
||||
// continue
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
async getStacks(): Promise<string[]> {
|
||||
try {
|
||||
const items = await fsPromises.readdir(this.baseDir, { withFileTypes: true });
|
||||
|
||||
Reference in New Issue
Block a user