mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-25 17:57:06 +00:00
fix: base Git multi-file Compose deploy env and dossier on the effective config (#1391)
* fix: resolve the root .env at deploy and render time for Git context-dir stacks A Git multi-file source with a context dir set --project-directory to that dir, so Docker Compose looked for .env there and missed the root .env Sencho writes. Validation already passed the root .env with --env-file, so a stack could validate with one effective config but deploy or render another. Add authoredComposeEnvFileArgs, which appends --env-file <stackDir>/.env when the applied deploy spec has a context dir and a root .env exists, and wire it into the deploy/update, image-scan, render, and container-listing compose invocations so they all resolve env from the same file the validator used. A non-ENOENT access error surfaces instead of silently dropping the flag. * fix: base multi-file Git dossier and doc-drift on the effective Compose model The Stack Dossier and its documentation-drift check parsed only the stored root compose file. For a multi-file Git source, services, ports, networks, or volumes that an override file adds were invisible, so the dossier showed incomplete facts and doc-drift could falsely warn that a documented port is unpublished when an override actually publishes it. Add a secret-safe GET /stacks/:name/effective-anatomy that renders the merged effective model and extracts only structural facts (services, ports, volumes, networks, restart), never env, label, or command values. StackAnatomyPanel fetches it for multi-file Git stacks and feeds those facts into the dossier and doc-drift, falling back to the root-only parse for single-file or non-git stacks and whenever the render is unavailable. * fix: add an inline path-injection barrier to the Git env-file resolver CodeQL js/path-injection flagged the fs.access in authoredComposeEnvFileArgs because the env path derives from the route-supplied stack name and the only containment check lived in the callers, not at the sink. Resolve the stack dir against the compose base and assert containment with startsWith inline, then derive the .env path from the validated dir, mirroring the existing inline guards in renderConfig and validateCompose. Valid stack names are unaffected; a name that escapes the base now yields no --env-file. * test: stabilize the dossier doc-drift e2e against the dossier-load race The first assertion filled the access_urls field as soon as the Dossier panel was visible, but the panel's GET /stacks/:name/dossier resolves by overwriting the fields from the server (empty access_urls) and only then flips the doc-drift gate on. When the GET landed after the fill, it clobbered the typed value and the warning never rendered, so the test failed intermittently under CI timing. Wait for that GET to land before typing, mirroring the spec's openStack helper.
This commit is contained in:
Anso
@@ -10,18 +10,20 @@
|
||||
* - any spec file path or context dir that is absolute / contains ".." throws
|
||||
* before any args are returned (it is spliced straight into child-process argv)
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest';
|
||||
import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
|
||||
import path from 'path';
|
||||
import fs from 'fs';
|
||||
import { setupTestDb, cleanupTestDb } from './helpers/setupTestDb';
|
||||
|
||||
let tmpDir: string;
|
||||
let authoredComposeFileArgs: typeof import('../utils/authoredComposeArgs').authoredComposeFileArgs;
|
||||
let authoredComposeEnvFileArgs: typeof import('../utils/authoredComposeArgs').authoredComposeEnvFileArgs;
|
||||
let DatabaseService: typeof import('../services/DatabaseService').DatabaseService;
|
||||
let NodeRegistry: typeof import('../services/NodeRegistry').NodeRegistry;
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpDir = await setupTestDb();
|
||||
({ authoredComposeFileArgs } = await import('../utils/authoredComposeArgs'));
|
||||
({ authoredComposeFileArgs, authoredComposeEnvFileArgs } = await import('../utils/authoredComposeArgs'));
|
||||
({ DatabaseService } = await import('../services/DatabaseService'));
|
||||
({ NodeRegistry } = await import('../services/NodeRegistry'));
|
||||
});
|
||||
@@ -136,3 +138,93 @@ describe('authoredComposeFileArgs', () => {
|
||||
expect(() => authoredComposeFileArgs(stackName)).toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* authoredComposeEnvFileArgs: the `--env-file <stackDir>/.env` flag a multi-file
|
||||
* Git deploy needs when a context dir is set. With `--project-directory <ctx>`,
|
||||
* Compose treats the context dir as the project directory and stops auto-finding
|
||||
* the root `.env` Sencho writes, so validation (which passes --env-file) and
|
||||
* deploy would otherwise resolve different env. Single-file / no-context stacks
|
||||
* keep Compose's default `.env` discovery from the stack dir, so they get no flag.
|
||||
*/
|
||||
describe('authoredComposeEnvFileArgs', () => {
|
||||
/** Create the on-disk stack directory and optionally a root .env for it. */
|
||||
function makeStackDir(stackName: string, withEnv: boolean): string {
|
||||
const baseDir = NodeRegistry.getInstance().getComposeDir(NodeRegistry.getInstance().getDefaultNodeId());
|
||||
const stackDir = path.join(baseDir, stackName);
|
||||
fs.mkdirSync(stackDir, { recursive: true });
|
||||
if (withEnv) fs.writeFileSync(path.join(stackDir, '.env'), 'TAG=1\n', 'utf-8');
|
||||
else fs.rmSync(path.join(stackDir, '.env'), { force: true });
|
||||
return stackDir;
|
||||
}
|
||||
|
||||
it('returns [] for a stack with no git source at all', async () => {
|
||||
expect(await authoredComposeEnvFileArgs('no-such-stack')).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns [] when the context dir is set but no .env exists', async () => {
|
||||
const stackName = 'ctx-no-env';
|
||||
seedSource(stackName, ['compose.yaml', 'infra/prod.yml']);
|
||||
DatabaseService.getInstance().setGitSourceAppliedSpec(stackName, {
|
||||
files: ['compose.yaml', 'infra/prod.yml'],
|
||||
contextDir: 'app',
|
||||
});
|
||||
makeStackDir(stackName, false);
|
||||
expect(await authoredComposeEnvFileArgs(stackName)).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns [] when a .env exists but the spec has no context dir', async () => {
|
||||
// No --project-directory, so the project dir stays the stack dir (cwd) and
|
||||
// Compose auto-discovers the root .env; an explicit flag is not needed.
|
||||
const stackName = 'env-no-ctx';
|
||||
seedSource(stackName, ['compose.yaml', 'infra/prod.yml']);
|
||||
DatabaseService.getInstance().setGitSourceAppliedSpec(stackName, {
|
||||
files: ['compose.yaml', 'infra/prod.yml'],
|
||||
contextDir: null,
|
||||
});
|
||||
makeStackDir(stackName, true);
|
||||
expect(await authoredComposeEnvFileArgs(stackName)).toEqual([]);
|
||||
});
|
||||
|
||||
it('returns --env-file <stackDir>/.env when a context dir is set and a .env exists', async () => {
|
||||
const stackName = 'ctx-with-env';
|
||||
seedSource(stackName, ['compose.yaml', 'infra/prod.yml']);
|
||||
DatabaseService.getInstance().setGitSourceAppliedSpec(stackName, {
|
||||
files: ['compose.yaml', 'infra/prod.yml'],
|
||||
contextDir: 'app',
|
||||
});
|
||||
const stackDir = makeStackDir(stackName, true);
|
||||
expect(await authoredComposeEnvFileArgs(stackName)).toEqual([
|
||||
'--env-file', path.join(stackDir, '.env'),
|
||||
]);
|
||||
});
|
||||
|
||||
it('returns [] for a stack name that escapes the compose base (path-injection guard)', async () => {
|
||||
// The inline barrier rejects a traversal name before the fs access, so no
|
||||
// --env-file is emitted for a path outside the compose base.
|
||||
const stackName = '../escape';
|
||||
seedSource(stackName, ['infra/base.yml', 'infra/prod.yml']);
|
||||
DatabaseService.getInstance().setGitSourceAppliedSpec(stackName, {
|
||||
files: ['compose.yaml', 'infra/prod.yml'],
|
||||
contextDir: 'app',
|
||||
});
|
||||
expect(await authoredComposeEnvFileArgs(stackName)).toEqual([]);
|
||||
});
|
||||
|
||||
it('rethrows a non-ENOENT access error instead of silently dropping the env file', async () => {
|
||||
// An EACCES on an existing .env must surface, not be treated as "no env
|
||||
// file": dropping --env-file there would deploy a different effective config
|
||||
// than the one validated.
|
||||
const stackName = 'ctx-eacces';
|
||||
seedSource(stackName, ['compose.yaml', 'infra/prod.yml']);
|
||||
DatabaseService.getInstance().setGitSourceAppliedSpec(stackName, {
|
||||
files: ['compose.yaml', 'infra/prod.yml'],
|
||||
contextDir: 'app',
|
||||
});
|
||||
const spy = vi.spyOn(fs.promises, 'access').mockRejectedValueOnce(
|
||||
Object.assign(new Error('permission denied'), { code: 'EACCES' }),
|
||||
);
|
||||
await expect(authoredComposeEnvFileArgs(stackName)).rejects.toThrow(/permission denied/);
|
||||
spy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
/**
|
||||
* GET /api/stacks/:stackName/effective-anatomy: returns the merged effective
|
||||
* facts, requires stack:read, 404s a missing stack, surfaces a structural (never
|
||||
* raw) error on render failure, and never leaks an env or label value.
|
||||
*/
|
||||
import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest';
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import request from 'supertest';
|
||||
import jwt from 'jsonwebtoken';
|
||||
import { setupTestDb, cleanupTestDb, TEST_USERNAME, TEST_JWT_SECRET } from './helpers/setupTestDb';
|
||||
import { ComposeService } from '../services/ComposeService';
|
||||
|
||||
let tmpDir: string;
|
||||
let app: import('express').Express;
|
||||
let authHeader: string;
|
||||
|
||||
const STACK = 'effanat';
|
||||
const ENV_SECRET = 'env-secret-71bd-value';
|
||||
const LABEL_SECRET = 'label-secret-22ce-value';
|
||||
|
||||
function stubRender(rendered: string | null, stderr = '') {
|
||||
vi.spyOn(ComposeService, 'getInstance').mockReturnValue({
|
||||
renderConfig: vi.fn().mockResolvedValue({ rendered, stderr, code: rendered === null ? 1 : 0, timedOut: false }),
|
||||
} as unknown as ComposeService);
|
||||
}
|
||||
|
||||
beforeAll(async () => {
|
||||
tmpDir = await setupTestDb();
|
||||
({ app } = await import('../index'));
|
||||
authHeader = `Bearer ${jwt.sign({ username: TEST_USERNAME }, TEST_JWT_SECRET, { expiresIn: '5m' })}`;
|
||||
});
|
||||
|
||||
afterAll(() => cleanupTestDb(tmpDir));
|
||||
|
||||
describe('effective-anatomy route', () => {
|
||||
let stackDir: string;
|
||||
beforeEach(() => {
|
||||
stackDir = path.join(process.env.COMPOSE_DIR as string, STACK);
|
||||
fs.mkdirSync(stackDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(stackDir, 'compose.yaml'), 'services:\n web:\n image: nginx:latest\n');
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
fs.rmSync(stackDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('returns merged effective facts for a renderable stack', async () => {
|
||||
stubRender(JSON.stringify({
|
||||
name: STACK,
|
||||
services: {
|
||||
web: {
|
||||
image: 'nginx:latest',
|
||||
restart: 'always',
|
||||
ports: [{ target: 80, published: '8080', protocol: 'tcp' }],
|
||||
volumes: [{ type: 'volume', source: 'data', target: '/data' }],
|
||||
networks: { backend: null },
|
||||
},
|
||||
},
|
||||
networks: { backend: {}, default: {} },
|
||||
volumes: { data: {} },
|
||||
}));
|
||||
const res = await request(app).get(`/api/stacks/${STACK}/effective-anatomy`).set('Authorization', authHeader);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.renderable).toBe(true);
|
||||
expect(res.body.services).toEqual(['web']);
|
||||
expect(res.body.ports.web).toEqual([{ host: '8080', container: '80', proto: 'tcp', published: true }]);
|
||||
expect(res.body.volumes.web).toEqual([{ host: 'data', container: '/data' }]);
|
||||
expect(res.body.restart).toBe('always');
|
||||
expect(res.body.networks).toEqual(['backend', 'default']);
|
||||
});
|
||||
|
||||
it('surfaces a structural error and never raw stderr on render failure', async () => {
|
||||
stubRender(null, `error: the "${ENV_SECRET}" variable is not set`);
|
||||
const res = await request(app).get(`/api/stacks/${STACK}/effective-anatomy`).set('Authorization', authHeader);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.renderable).toBe(false);
|
||||
expect(JSON.stringify(res.body)).not.toContain(ENV_SECRET);
|
||||
});
|
||||
|
||||
it('falls back to a structural error when the render is not valid JSON', async () => {
|
||||
stubRender('this is not json {');
|
||||
const res = await request(app).get(`/api/stacks/${STACK}/effective-anatomy`).set('Authorization', authHeader);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.renderable).toBe(false);
|
||||
expect(res.body.services).toEqual([]);
|
||||
});
|
||||
|
||||
it('never leaks env or label values into the facts', async () => {
|
||||
stubRender(JSON.stringify({
|
||||
name: STACK,
|
||||
services: { web: { image: 'nginx:latest', environment: { TOKEN: ENV_SECRET }, labels: { 'x.secret': LABEL_SECRET } } },
|
||||
networks: {},
|
||||
volumes: {},
|
||||
}));
|
||||
const res = await request(app).get(`/api/stacks/${STACK}/effective-anatomy`).set('Authorization', authHeader);
|
||||
expect(res.status).toBe(200);
|
||||
const body = JSON.stringify(res.body);
|
||||
expect(body).not.toContain(ENV_SECRET);
|
||||
expect(body).not.toContain(LABEL_SECRET);
|
||||
});
|
||||
|
||||
it('rejects an unauthenticated request', async () => {
|
||||
const res = await request(app).get(`/api/stacks/${STACK}/effective-anatomy`);
|
||||
expect(res.status).toBe(401);
|
||||
});
|
||||
|
||||
it('returns 404 for a stack that does not exist', async () => {
|
||||
stubRender(JSON.stringify({ name: 'x', services: {}, networks: {}, volumes: {} }));
|
||||
const res = await request(app).get('/api/stacks/nope-not-here/effective-anatomy').set('Authorization', authHeader);
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,116 @@
|
||||
/**
|
||||
* Unit tests for parseEffectiveAnatomy: the secret-safe structural extractor that
|
||||
* maps `docker compose config --format json` (the fully-merged effective model)
|
||||
* to the same anatomy facts the frontend derives from a single compose file, so a
|
||||
* multi-file Git source's dossier and doc-drift reflect every override file.
|
||||
*
|
||||
* The extractor must read ONLY structural fields (service keys, ports, volumes,
|
||||
* restart, network keys) and never an environment, label, or command value.
|
||||
*/
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { parseEffectiveAnatomy } from '../services/effectiveAnatomy';
|
||||
|
||||
describe('parseEffectiveAnatomy', () => {
|
||||
it('returns an empty model for null / non-object input', () => {
|
||||
const empty = { services: [], ports: {}, volumes: {}, restart: null, networks: [] };
|
||||
expect(parseEffectiveAnatomy(null)).toEqual(empty);
|
||||
expect(parseEffectiveAnatomy('nope')).toEqual(empty);
|
||||
expect(parseEffectiveAnatomy(42)).toEqual(empty);
|
||||
});
|
||||
|
||||
it('extracts services, published ports, and volumes from the long-form render', () => {
|
||||
const rendered = {
|
||||
name: 'demo',
|
||||
services: {
|
||||
web: {
|
||||
image: 'nginx',
|
||||
restart: 'always',
|
||||
ports: [
|
||||
{ mode: 'ingress', host_ip: '0.0.0.0', target: 80, published: '8080', protocol: 'tcp' },
|
||||
],
|
||||
volumes: [
|
||||
{ type: 'bind', source: '/srv/web', target: '/usr/share/nginx/html' },
|
||||
{ type: 'volume', source: 'webdata', target: '/var/cache' },
|
||||
],
|
||||
networks: { default: null },
|
||||
},
|
||||
},
|
||||
networks: { default: { name: 'demo_default' } },
|
||||
};
|
||||
const anatomy = parseEffectiveAnatomy(rendered);
|
||||
expect(anatomy.services).toEqual(['web']);
|
||||
expect(anatomy.ports).toEqual({
|
||||
web: [{ host: '8080', container: '80', proto: 'tcp', published: true }],
|
||||
});
|
||||
expect(anatomy.volumes).toEqual({
|
||||
web: [
|
||||
{ host: '/srv/web', container: '/usr/share/nginx/html' },
|
||||
{ host: 'webdata', container: '/var/cache' },
|
||||
],
|
||||
});
|
||||
expect(anatomy.restart).toBe('always');
|
||||
expect(anatomy.networks).toEqual(['default']);
|
||||
});
|
||||
|
||||
it('merges ports that only an override file publishes (the blocker case)', () => {
|
||||
// The root file declared `app` with no ports; an override published 9000.
|
||||
// The rendered model is the merge, so the published port must appear here.
|
||||
const rendered = {
|
||||
services: {
|
||||
app: {
|
||||
ports: [{ target: 9000, published: '9000', protocol: 'tcp' }],
|
||||
},
|
||||
},
|
||||
};
|
||||
const anatomy = parseEffectiveAnatomy(rendered);
|
||||
expect(anatomy.ports.app).toEqual([{ host: '9000', container: '9000', proto: 'tcp', published: true }]);
|
||||
});
|
||||
|
||||
it('marks a container-only port as unpublished and preserves UDP', () => {
|
||||
const rendered = {
|
||||
services: {
|
||||
svc: {
|
||||
ports: [
|
||||
{ target: 53, published: '53', protocol: 'udp' },
|
||||
{ target: 9090, published: '', protocol: 'tcp' },
|
||||
],
|
||||
},
|
||||
},
|
||||
};
|
||||
const anatomy = parseEffectiveAnatomy(rendered);
|
||||
expect(anatomy.ports.svc).toEqual([
|
||||
{ host: '53', container: '53', proto: 'udp', published: true },
|
||||
{ host: '', container: '9090', proto: 'tcp', published: false },
|
||||
]);
|
||||
});
|
||||
|
||||
it('collects network keys from services and the top level, deduped', () => {
|
||||
const rendered = {
|
||||
services: {
|
||||
a: { networks: { frontend: null, backend: null } },
|
||||
b: { networks: ['backend'] },
|
||||
},
|
||||
networks: { frontend: {}, backend: {}, default: {} },
|
||||
};
|
||||
const anatomy = parseEffectiveAnatomy(rendered);
|
||||
expect(anatomy.networks).toEqual(['frontend', 'backend', 'default']);
|
||||
});
|
||||
|
||||
it('never surfaces environment, label, or command values', () => {
|
||||
const rendered = {
|
||||
services: {
|
||||
web: {
|
||||
environment: { DB_PASSWORD: 'super-secret', API_KEY: 'leak-me' },
|
||||
labels: { 'traefik.http.routers.web.rule': 'Host(`secret.example.com`)' },
|
||||
command: ['--token', 'do-not-leak'],
|
||||
ports: [{ target: 80, published: '80', protocol: 'tcp' }],
|
||||
},
|
||||
},
|
||||
};
|
||||
const serialized = JSON.stringify(parseEffectiveAnatomy(rendered));
|
||||
expect(serialized).not.toContain('super-secret');
|
||||
expect(serialized).not.toContain('leak-me');
|
||||
expect(serialized).not.toContain('do-not-leak');
|
||||
expect(serialized).not.toContain('secret.example.com');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user