fix: stop Doctor exposing hash fragments as unset variables (#1558)

Classify Compose stderr unset-variable warnings into intentional references vs literal-dollar fragments from secret values. Adds env-literal-dollar preflight rule and safe remediation text. Fixes #1550.
This commit is contained in:
Anso
2026-07-05 03:50:40 -04:00
committed by GitHub
parent ecd757270f
commit 122c1b8073
12 changed files with 416 additions and 25 deletions
@@ -85,7 +85,7 @@ describe('runPreflight', () => {
expect(report.renderable).toBe(true);
expect(report.status).toBe('high'); // env-unset + 0.0.0.0 exposure are high
expect(report.highestSeverity).toBe('high');
expect(report.findings.map(f => f.ruleId)).toEqual(expect.arrayContaining(['env-unset', 'port-exposed-all-interfaces', 'image-latest', 'no-healthcheck']));
expect(report.findings.map(f => f.ruleId)).toEqual(expect.arrayContaining(['env-literal-dollar', 'port-exposed-all-interfaces', 'image-latest', 'no-healthcheck']));
expect(report.ranBy).toBe('tester');
expect(report.sourceHash).toBeTruthy();
@@ -128,6 +128,40 @@ describe('runPreflight', () => {
expect(JSON.stringify(report)).not.toContain(SECRET);
});
it('does not expose hash fragments as unset variable names (#1550)', async () => {
const stack = 'hashfrag';
const dir = path.join(process.env.COMPOSE_DIR as string, stack);
fs.mkdirSync(dir, { recursive: true });
fs.writeFileSync(
path.join(dir, 'compose.yaml'),
[
'services:',
' demo:',
' image: alpine:3',
' environment:',
' - EXAMPLE_AUTH_HASH=$2b$10$E6SDEbshpc$vCSrREDACTED',
].join('\n'),
);
try {
stubDocker(
{ name: stack, services: { demo: { image: 'alpine:3', environment: { EXAMPLE_AUTH_HASH: '' } } }, networks: {}, volumes: {} },
'WARN The "E6SDEbshpc" variable is not set. Defaulting to a blank string.\n'
+ 'WARN The "vCSr" variable is not set. Defaulting to a blank string.\n',
);
const report = await doctor().runPreflight(nodeId, stack, 'tester');
const literal = report.findings.filter(f => f.ruleId === 'env-literal-dollar');
const unset = report.findings.filter(f => f.ruleId === 'env-unset');
expect(literal.length).toBeGreaterThan(0);
expect(unset.some(f => f.title.includes('E6SDEbshpc') || f.title.includes('vCSr'))).toBe(false);
const persisted = JSON.stringify(db().getPreflightFindings(db().getLatestPreflightRun(nodeId, stack)!.id));
expect(persisted).not.toContain('E6SDEbshpc');
expect(persisted).not.toContain('vCSr');
expect(literal[0].title).toContain('likely secret');
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
});
it('replaces the prior run rather than accumulating', async () => {
stubDocker({ name: STACK, services: { web: { image: 'nginx:latest' } }, networks: {}, volumes: {} });
await doctor().runPreflight(nodeId, STACK, null);
@@ -8,6 +8,8 @@ import {
readEnvFileKeys,
parseUnsetEnvVars,
parseMissingRequiredVars,
parseBareDollarRefs,
parseIntentionalBareDollarRefs,
} from '../helpers/envVarParse';
describe('parseInterpolationRefs', () => {
@@ -35,6 +37,21 @@ describe('parseInterpolationRefs', () => {
expect(parseInterpolationRefs('x: $${ESCAPED}').map(r => r.name)).not.toContain('ESCAPED');
});
it('extracts bare $VAR refs but skips $$ escapes and ${VAR} forms', () => {
expect(parseBareDollarRefs('host: $DB_HOST and $${LIT} and ${BRACED}')).toEqual(['DB_HOST']);
});
it('keeps intentional bare refs for self-refs and whole-value refs only', () => {
const src = [
'- TOKEN=$TOKEN',
'- FOO=$BAR',
'- EXAMPLE_AUTH_HASH=$2b$10$E6SDEbshpc$vCSrREDACTED',
'EXAMPLE_AUTH_HASH: $2b$10$E6SDEbshpc$vCSrREDACTED',
'command: echo $HELLO',
].join('\n');
expect(parseIntentionalBareDollarRefs(src).sort()).toEqual(['BAR', 'HELLO', 'TOKEN']);
});
it('merges flags across repeated references of one name', () => {
const refs = parseInterpolationRefs('${X} then ${X:?e}');
expect(refs).toHaveLength(1);
+30 -1
View File
@@ -26,6 +26,7 @@ function ctx(over: Partial<PreflightContext> = {}): PreflightContext {
const m = over.model !== undefined ? over.model : model([]);
return {
stackName: 'proj', platform: 'linux', model: m, renderable: true, renderError: null, unsetEnvVars: [],
literalDollarWarnings: [],
missingEnvFiles: [],
sourceServiceNames: m ? m.services.map(s => s.name) : [], sourceReadable: true,
nodePorts: [], existingNetworkNames: new Set(), existingVolumeNames: new Set(),
@@ -55,6 +56,34 @@ describe('env-unset', () => {
expect(f[0].severity).toBe('high');
expect(f.map(x => x.sourcePath)).toEqual(['FOO', 'BAR']);
});
it('mentions literal-dollar escapes in remediation', () => {
const f = ids(runRules(ctx({ unsetEnvVars: ['FOO'] })), 'env-unset');
expect(f[0].remediation).toContain('$$');
expect(f[0].remediation).toContain('single-quote');
});
});
describe('env-literal-dollar', () => {
it('emits a safe finding for likely-secret literal dollar warnings', () => {
const f = ids(runRules(ctx({
literalDollarWarnings: [{ envKey: 'EXAMPLE_AUTH_HASH', likelySecret: true, service: 'demo' }],
})), 'env-literal-dollar');
expect(f).toHaveLength(1);
expect(f[0].severity).toBe('high');
expect(f[0].title).toContain('likely secret');
expect(f[0].sourcePath).toBe('EXAMPLE_AUTH_HASH');
expect(f[0].service).toBe('demo');
expect(f[0].title).not.toContain('E6SDEbshpc');
expect(f[0].remediation).toContain('$$');
});
it('omits fragment names from generic literal-dollar findings', () => {
const f = ids(runRules(ctx({
literalDollarWarnings: [{ likelySecret: false }],
})), 'env-literal-dollar');
expect(f).toHaveLength(1);
expect(f[0].sourcePath).toBeUndefined();
expect(f[0].title).toContain('environment value');
});
});
describe('env-file-missing', () => {
@@ -408,7 +437,7 @@ describe('rule registry completeness', () => {
// The canonical rule set. Adding or removing a rule must update this list,
// which forces a deliberate pass over the docs and the frontend severity map.
const EXPECTED_RULE_IDS = [
'render-failed', 'env-unset', 'env-file-missing', 'port-conflict-node', 'port-conflict-internal', 'port-exposed-all-interfaces',
'render-failed', 'env-unset', 'env-literal-dollar', 'env-file-missing', 'port-conflict-node', 'port-conflict-internal', 'port-exposed-all-interfaces',
'bind-path-missing', 'bind-path-permission', 'docker-socket-mount', 'privileged', 'network-mode-host',
'uid-gid-risk', 'image-latest', 'no-restart-policy', 'no-healthcheck', 'deploy-swarm-only',
'node-state-unavailable',
@@ -0,0 +1,85 @@
import { describe, it, expect } from 'vitest';
import { classifyUnsetEnvVars } from '../helpers/unsetEnvClassification';
import type { StackEnvSources } from '../helpers/envFileResolution';
function sources(over: Partial<StackEnvSources> = {}): StackEnvSources {
return {
stackDir: '/stack',
baseDir: '/compose',
composeFiles: [],
envFiles: [],
inlineEnvKeysByService: {},
interpolationRefs: [],
authoredComposeText: '',
...over,
};
}
describe('classifyUnsetEnvVars', () => {
it('keeps intentional ${VAR} refs as unset variables', () => {
const src = sources({
authoredComposeText: 'services:\n web:\n image: nginx\n environment:\n - DB_HOST=${DB_HOST}\n',
interpolationRefs: [{ name: 'DB_HOST', required: false, hasDefault: false, alternate: false }],
});
const result = classifyUnsetEnvVars(['DB_HOST', 'E6SDEbshpc'], src);
expect(result.intentional).toEqual(['DB_HOST']);
expect(result.literalDollar).toHaveLength(1);
expect(result.literalDollar[0].likelySecret).toBe(false);
});
it('classifies bcrypt hash fragments as literal-dollar warnings without exposing fragments', () => {
const compose = [
'services:',
' demo:',
' image: alpine:3',
' environment:',
' - EXAMPLE_AUTH_HASH=$2b$10$E6SDEbshpc$vCSrREDACTED',
].join('\n');
const src = sources({
authoredComposeText: compose,
inlineEnvKeysByService: { demo: ['EXAMPLE_AUTH_HASH'] },
});
const result = classifyUnsetEnvVars(['E6SDEbshpc', 'vCSr'], src);
expect(result.intentional).toEqual([]);
expect(result.literalDollar).toHaveLength(1);
expect(result.literalDollar[0]).toMatchObject({
envKey: 'EXAMPLE_AUTH_HASH',
likelySecret: true,
service: 'demo',
});
expect(JSON.stringify(result)).not.toContain('E6SDEbshpc');
expect(JSON.stringify(result)).not.toContain('vCSr');
});
it('classifies map-form bcrypt hash fragments as literal-dollar warnings', () => {
const compose = [
'services:',
' demo:',
' image: alpine:3',
' environment:',
' EXAMPLE_AUTH_HASH: $2b$10$E6SDEbshpc$vCSrREDACTED',
].join('\n');
const src = sources({
authoredComposeText: compose,
inlineEnvKeysByService: { demo: ['EXAMPLE_AUTH_HASH'] },
});
const result = classifyUnsetEnvVars(['E6SDEbshpc', 'vCSr'], src);
expect(result.intentional).toEqual([]);
expect(result.literalDollar[0]?.envKey).toBe('EXAMPLE_AUTH_HASH');
});
it('treats bare $VAR references as intentional', () => {
const compose = 'services:\n web:\n image: nginx\n environment:\n - TOKEN=$TOKEN\n';
const src = sources({ authoredComposeText: compose });
const result = classifyUnsetEnvVars(['TOKEN'], src);
expect(result.intentional).toEqual(['TOKEN']);
expect(result.literalDollar).toEqual([]);
});
it('attributes env-file-only spurious fragments to a lone likely-secret key', () => {
const src = sources({ authoredComposeText: 'services:\n web:\n image: nginx\n env_file:\n - ./secrets.env\n' });
const result = classifyUnsetEnvVars(['E6SDEbshpc'], src, ['EXAMPLE_AUTH_HASH']);
expect(result.intentional).toEqual([]);
expect(result.literalDollar).toEqual([{ envKey: 'EXAMPLE_AUTH_HASH', likelySecret: true }]);
});
});