diff --git a/backend/src/__tests__/compose-service.test.ts b/backend/src/__tests__/compose-service.test.ts index e49d48cf..0c7950e2 100644 --- a/backend/src/__tests__/compose-service.test.ts +++ b/backend/src/__tests__/compose-service.test.ts @@ -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 ` args deterministically. +// resolves the (optional) mesh override. The hoisted mock defaults to "no override" +// so a single-file stack yields plain `docker compose ` 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', () => { diff --git a/backend/src/__tests__/filesystem-symlink-escape.test.ts b/backend/src/__tests__/filesystem-symlink-escape.test.ts index ecdf219c..abf1606d 100644 --- a/backend/src/__tests__/filesystem-symlink-escape.test.ts +++ b/backend/src/__tests__/filesystem-symlink-escape.test.ts @@ -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'); diff --git a/backend/src/__tests__/filesystem.test.ts b/backend/src/__tests__/filesystem.test.ts index 3fd46ce0..a0f80d29 100644 --- a/backend/src/__tests__/filesystem.test.ts +++ b/backend/src/__tests__/filesystem.test.ts @@ -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(); + }); +}); diff --git a/backend/src/services/ComposeService.ts b/backend/src/services/ComposeService.ts index 7d563d5d..923a45a6 100644 --- a/backend/src/services/ComposeService.ts +++ b/backend/src/services/ComposeService.ts @@ -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); } diff --git a/backend/src/services/FileSystemService.ts b/backend/src/services/FileSystemService.ts index be86cd5b..3261adf0 100644 --- a/backend/src/services/FileSystemService.ts +++ b/backend/src/services/FileSystemService.ts @@ -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(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 { + 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 { try { const items = await fsPromises.readdir(this.baseDir, { withFileTypes: true });