mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-08 09:54:26 +00:00
fix: name matched risk inputs in policy block messages (#1471)
The auto-update, bulk-label, scheduler, and blueprint deploy block messages hardcoded "image(s) exceed <max_severity>", which is wrong under the risk-first policy model: a block can be driven by a known-exploited (KEV) or fixable Critical/High input while the severity threshold was never the trigger. In those cases the message named a severity ceiling the policy did not enforce. Route all four message paths through a shared summarizeBlockReasons helper (the same reason text the deploy-gate 409 response and the block dialog already use), so every surface names the inputs that actually matched. Falls back to a generic phrase when no reason was recorded.
This commit is contained in:
@@ -341,6 +341,51 @@ describe('POST /api/auto-update/execute', () => {
|
||||
expect(typeof res.body.result).toBe('string');
|
||||
});
|
||||
|
||||
it('reports a reason-aware block message when the policy gate blocks auto-update', async () => {
|
||||
// The Codex QA flagged this surface: a KEV- or fixable-driven block must
|
||||
// name the matched input, never "exceed <max_severity>" (a ceiling the
|
||||
// policy did not enforce). Force the gate to block on KEV and assert the
|
||||
// per-stack result string names the reason and skips the update.
|
||||
const DockerController = (await import('../services/DockerController')).default;
|
||||
const { ImageUpdateService } = await import('../services/ImageUpdateService');
|
||||
const { ComposeService } = await import('../services/ComposeService');
|
||||
const PolicyEnforcement = await import('../services/PolicyEnforcement');
|
||||
|
||||
const containersSpy = vi.spyOn(DockerController.prototype, 'getContainersByStack')
|
||||
.mockResolvedValue([{ Id: 'c1', Image: 'nginx:latest' }] as never);
|
||||
const checkSpy = vi.spyOn(ImageUpdateService.getInstance(), 'checkImage')
|
||||
.mockResolvedValue({ hasUpdate: true } as never);
|
||||
const updateSpy = vi.spyOn(ComposeService.prototype, 'updateStack').mockResolvedValue();
|
||||
const gateSpy = vi.spyOn(PolicyEnforcement, 'enforcePolicyPreDeploy').mockResolvedValue({
|
||||
ok: false,
|
||||
bypassed: false,
|
||||
policy: { id: 1, name: 'kev-gate', max_severity: 'HIGH' },
|
||||
violations: [{
|
||||
imageRef: 'nginx:latest', severity: 'MEDIUM',
|
||||
criticalCount: 0, highCount: 0, kevCount: 1, fixableCount: 0,
|
||||
scanId: 9, reasons: ['kev'],
|
||||
}],
|
||||
} as never);
|
||||
try {
|
||||
const res = await request(app)
|
||||
.post('/api/auto-update/execute')
|
||||
.set('Cookie', adminCookie)
|
||||
.send({ target: 'auto-upd-blocked' });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.result).toContain('blocked auto-update');
|
||||
expect(res.body.result).toContain('matched known-exploited CVE (KEV)');
|
||||
expect(res.body.result).not.toContain('exceed');
|
||||
expect(res.body.result).not.toContain('HIGH');
|
||||
// Blocked stacks are skipped, not updated.
|
||||
expect(updateSpy).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
containersSpy.mockRestore();
|
||||
checkSpy.mockRestore();
|
||||
updateSpy.mockRestore();
|
||||
gateSpy.mockRestore();
|
||||
}
|
||||
});
|
||||
|
||||
it('begins an update health gate after an auto-update applies', async () => {
|
||||
// Target a single stack; the route works off the running containers, so
|
||||
// stub the container probe, the update check, and the compose update, then
|
||||
|
||||
@@ -179,7 +179,7 @@ describe('Stack Labels bulk actions', () => {
|
||||
enforcePolicyPreDeploy.mockResolvedValue({
|
||||
ok: false,
|
||||
policy: { name: 'block-criticals', max_severity: 'high' },
|
||||
violations: [{ image: 'nginx:latest', severity: 'critical' }],
|
||||
violations: [{ imageRef: 'nginx:latest', severity: 'CRITICAL', reasons: ['severity'] }],
|
||||
});
|
||||
|
||||
const res = await request(app)
|
||||
@@ -192,6 +192,7 @@ describe('Stack Labels bulk actions', () => {
|
||||
expect.objectContaining({ stackName: 'alpha', success: false, dryRun: true }),
|
||||
]);
|
||||
expect(res.body.results[0].error).toContain('Policy "block-criticals" blocked deploy');
|
||||
expect(res.body.results[0].error).toContain('matched severity threshold');
|
||||
expect(deployStack).not.toHaveBeenCalled();
|
||||
expect(invalidateNodeCaches).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
evaluatePolicyRisk,
|
||||
describeReason,
|
||||
describePolicyInputs,
|
||||
summarizeBlockReasons,
|
||||
type PolicyRiskInputs,
|
||||
type RiskFinding,
|
||||
} from '../utils/policy-risk';
|
||||
@@ -89,3 +90,22 @@ describe('describeReason / describePolicyInputs', () => {
|
||||
expect(describePolicyInputs(inputs())).toBe('no active inputs');
|
||||
});
|
||||
});
|
||||
|
||||
describe('summarizeBlockReasons', () => {
|
||||
it('names a KEV-driven block as known-exploited, not a severity threshold', () => {
|
||||
expect(summarizeBlockReasons([{ reasons: ['kev'] }])).toBe('known-exploited CVE (KEV)');
|
||||
});
|
||||
|
||||
it('joins and de-duplicates reasons across violations', () => {
|
||||
const summary = summarizeBlockReasons([
|
||||
{ reasons: ['kev'] },
|
||||
{ reasons: ['fixable', 'kev'] },
|
||||
]);
|
||||
expect(summary).toBe('known-exploited CVE (KEV) + fixable Critical/High');
|
||||
});
|
||||
|
||||
it('falls back to a generic phrase when no reason was recorded', () => {
|
||||
expect(summarizeBlockReasons([{ reasons: [] }])).toBe('scan policy conditions');
|
||||
expect(summarizeBlockReasons([])).toBe('scan policy conditions');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -921,7 +921,7 @@ describe('SchedulerService - executeUpdate', () => {
|
||||
ok: false,
|
||||
bypassed: false,
|
||||
policy: { id: 1, name: 'block-high', max_severity: 'HIGH' },
|
||||
violations: [{ imageRef: 'nginx:1.14', severity: 'CRITICAL', criticalCount: 2, highCount: 5, scanId: 7 }],
|
||||
violations: [{ imageRef: 'nginx:1.14', severity: 'CRITICAL', criticalCount: 2, highCount: 5, scanId: 7, reasons: ['severity'] }],
|
||||
});
|
||||
|
||||
const svc = SchedulerService.getInstance();
|
||||
@@ -934,6 +934,7 @@ describe('SchedulerService - executeUpdate', () => {
|
||||
expect(warn).toBeDefined();
|
||||
expect(warn![2]).toContain('block-high');
|
||||
expect(warn![2]).toContain('nginx:1.14');
|
||||
expect(warn![2]).toContain('matched severity threshold');
|
||||
// The run completes (skip-and-continue), not a hard task failure.
|
||||
expect(mockUpdateScheduledTaskRun).toHaveBeenCalledWith(1, expect.objectContaining({ status: 'success' }));
|
||||
});
|
||||
@@ -950,11 +951,13 @@ describe('SchedulerService - executeUpdate', () => {
|
||||
created_by: 'admin',
|
||||
last_status: null,
|
||||
});
|
||||
// KEV-driven block (severity threshold not the trigger): the message must
|
||||
// name the matched input, never a severity ceiling it did not enforce.
|
||||
mockEnforcePolicyPreDeploy.mockResolvedValue({
|
||||
ok: false,
|
||||
bypassed: false,
|
||||
policy: { id: 1, name: 'block-high', max_severity: 'HIGH' },
|
||||
violations: [{ imageRef: 'nginx:1.14', severity: 'CRITICAL', criticalCount: 1, highCount: 0, scanId: 3 }],
|
||||
violations: [{ imageRef: 'nginx:1.14', severity: 'MEDIUM', criticalCount: 0, highCount: 0, kevCount: 1, scanId: 3, reasons: ['kev'] }],
|
||||
});
|
||||
|
||||
const svc = SchedulerService.getInstance();
|
||||
@@ -966,6 +969,8 @@ describe('SchedulerService - executeUpdate', () => {
|
||||
expect(warn).toBeDefined();
|
||||
expect(warn![2]).toContain('Auto-start');
|
||||
expect(warn![2]).toContain('block-high');
|
||||
expect(warn![2]).toContain('known-exploited CVE (KEV)');
|
||||
expect(warn![2]).not.toContain('HIGH');
|
||||
// Auto-start does not skip-and-continue; the run is recorded as a failure.
|
||||
expect(mockUpdateScheduledTaskRun).toHaveBeenCalledWith(1, expect.objectContaining({ status: 'failure' }));
|
||||
});
|
||||
|
||||
@@ -9,7 +9,7 @@ import { LicenseService } from '../services/LicenseService';
|
||||
import { effectiveTier } from '../middleware/tierGates';
|
||||
import { getErrorMessage } from '../utils/errors';
|
||||
import { sanitizeForLog } from '../utils/safeLog';
|
||||
import { describeReason } from '../utils/policy-risk';
|
||||
import { summarizeBlockReasons } from '../utils/policy-risk';
|
||||
|
||||
type BlockableAction = 'deploy' | 'update' | 'rollback';
|
||||
|
||||
@@ -24,12 +24,7 @@ export function describePolicyBlock(
|
||||
violations: PolicyViolation[],
|
||||
action: BlockableAction = 'deploy',
|
||||
): string {
|
||||
const reasons = new Set<string>();
|
||||
for (const v of violations) {
|
||||
for (const r of v.reasons) reasons.add(describeReason(r));
|
||||
}
|
||||
const reasonText = reasons.size > 0 ? [...reasons].join(' + ') : 'scan policy conditions';
|
||||
return `Policy "${policy?.name ?? 'policy'}" blocked ${action}: ${violations.length} image(s) matched ${reasonText}`;
|
||||
return `Policy "${policy?.name ?? 'policy'}" blocked ${action}: ${violations.length} image(s) matched ${summarizeBlockReasons(violations)}`;
|
||||
}
|
||||
|
||||
// Bypass requires `?ignorePolicy=true` AND `req.user.role === 'admin'`. The
|
||||
|
||||
@@ -15,6 +15,7 @@ import { HealthGateService } from '../services/HealthGateService';
|
||||
import { authMiddleware } from '../middleware/auth';
|
||||
import { requireAdmin } from '../middleware/tierGates';
|
||||
import { buildPolicyGateOptions } from '../helpers/policyGate';
|
||||
import { summarizeBlockReasons } from '../utils/policy-risk';
|
||||
import { isValidStackName } from '../utils/validation';
|
||||
import { sanitizeForLog } from '../utils/safeLog';
|
||||
import { getErrorMessage } from '../utils/errors';
|
||||
@@ -365,7 +366,7 @@ autoUpdateRouter.post('/execute', authMiddleware, async (req: Request, res: Resp
|
||||
);
|
||||
if (!autoUpdateGate.ok) {
|
||||
const blockedImages = autoUpdateGate.violations.map((v) => v.imageRef).join(', ');
|
||||
const blockedMsg = `Policy "${autoUpdateGate.policy?.name}" blocked auto-update: ${autoUpdateGate.violations.length} image(s) exceed ${autoUpdateGate.policy?.max_severity}${blockedImages ? ` (${blockedImages})` : ''}`;
|
||||
const blockedMsg = `Policy "${autoUpdateGate.policy?.name}" blocked auto-update: ${autoUpdateGate.violations.length} image(s) matched ${summarizeBlockReasons(autoUpdateGate.violations)}${blockedImages ? ` (${blockedImages})` : ''}`;
|
||||
NotificationService.getInstance().dispatchAlert('warning', 'scan_finding', blockedMsg, { stackName, actor: 'system:image-update' });
|
||||
results.push(`Stack "${stackName}": ${blockedMsg}`);
|
||||
continue;
|
||||
|
||||
@@ -8,7 +8,7 @@ import { enforcePolicyPreDeploy } from '../services/PolicyEnforcement';
|
||||
import { authMiddleware } from '../middleware/auth';
|
||||
import { requirePermission } from '../middleware/permissions';
|
||||
import { requireAdmin, requireBody } from '../middleware/tierGates';
|
||||
import { buildPolicyGateOptions } from '../helpers/policyGate';
|
||||
import { buildPolicyGateOptions, describePolicyBlock } from '../helpers/policyGate';
|
||||
import { invalidateNodeCaches } from '../helpers/cacheInvalidation';
|
||||
import { VALID_LABEL_COLORS, MAX_LABELS_PER_NODE } from '../helpers/constants';
|
||||
import { isValidStackName } from '../utils/validation';
|
||||
@@ -227,7 +227,7 @@ labelsRouter.post('/:id/action', authMiddleware, async (req: Request, res: Respo
|
||||
buildPolicyGateOptions(req),
|
||||
);
|
||||
if (!gate.ok) {
|
||||
const blockedMsg = `Policy "${gate.policy?.name}" blocked deploy: ${gate.violations.length} image(s) exceed ${gate.policy?.max_severity}`;
|
||||
const blockedMsg = describePolicyBlock(gate.policy, gate.violations);
|
||||
results.push({ stackName, success: false, error: blockedMsg, ...(isDryRun ? { dryRun: true } : {}) });
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -14,7 +14,7 @@ import { FileSystemService } from './FileSystemService';
|
||||
import { NodeRegistry } from './NodeRegistry';
|
||||
import { PROXY_TIER_HEADER } from './license-headers';
|
||||
import { LicenseService } from './LicenseService';
|
||||
import { assertPolicyGateAllows, buildSystemPolicyGateOptions, triggerPostDeployScan } from '../helpers/policyGate';
|
||||
import { assertPolicyGateAllows, buildSystemPolicyGateOptions, describePolicyBlock, triggerPostDeployScan } from '../helpers/policyGate';
|
||||
import { enforcePolicyForImageRefs } from './PolicyEnforcement';
|
||||
import { BlueprintAnalyzer } from './BlueprintAnalyzer';
|
||||
import { sanitizeForLog } from '../utils/safeLog';
|
||||
@@ -401,7 +401,7 @@ export class BlueprintService {
|
||||
auditPath: `/api/blueprints/${blueprint.id}/apply`,
|
||||
}, undefined, true);
|
||||
if (!gate.ok) {
|
||||
throw new Error(`Policy "${gate.policy?.name}" blocked deploy: ${gate.violations.length} image(s) exceed ${gate.policy?.max_severity}`);
|
||||
throw new Error(describePolicyBlock(gate.policy, gate.violations));
|
||||
}
|
||||
|
||||
const outcome = await this.applyLocalUnderLock(
|
||||
|
||||
@@ -22,6 +22,7 @@ import TrivyInstaller from './TrivyInstaller';
|
||||
import { CloudBackupService } from './CloudBackupService';
|
||||
import { buildSystemPolicyGateOptions } from '../helpers/policyGate';
|
||||
import { enforcePolicyPreDeploy } from './PolicyEnforcement';
|
||||
import { summarizeBlockReasons } from '../utils/policy-risk';
|
||||
|
||||
const TRIVY_UPDATE_CHECK_INTERVAL_MS = 24 * 60 * 60 * 1000;
|
||||
const TRIVY_UPDATE_CHECK_STARTUP_DELAY_MS = 5 * 60 * 1000;
|
||||
@@ -197,14 +198,15 @@ export class SchedulerService {
|
||||
);
|
||||
if (gate.ok) return;
|
||||
const images = gate.violations.map((v) => v.imageRef).join(', ');
|
||||
const reasons = summarizeBlockReasons(gate.violations);
|
||||
this.safeDispatch(
|
||||
'warning',
|
||||
'scan_finding',
|
||||
`${action} blocked for "${stackName}" by policy "${gate.policy?.name}": ${gate.violations.length} image(s) exceed ${gate.policy?.max_severity}${images ? ` (${images})` : ''}`,
|
||||
`${action} blocked for "${stackName}" by policy "${gate.policy?.name}": ${gate.violations.length} image(s) matched ${reasons}${images ? ` (${images})` : ''}`,
|
||||
stackName,
|
||||
);
|
||||
throw new Error(
|
||||
`${action} blocked by policy "${gate.policy?.name}": ${gate.violations.length} image(s) exceed ${gate.policy?.max_severity}`,
|
||||
`${action} blocked by policy "${gate.policy?.name}": ${gate.violations.length} image(s) matched ${reasons}`,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -107,6 +107,24 @@ export function describeReason(reason: PolicyBlockReason): string {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* De-duplicated, human-readable summary of the inputs that actually matched
|
||||
* across a set of policy violations, joined with " + ". Used wherever a policy
|
||||
* block is reported to a user (the gate's 409 response, thrown gate errors, and
|
||||
* the deploy/auto-update block messages), so a KEV- or fixable-driven block
|
||||
* never reads as a severity-threshold block. Falls back to a generic phrase
|
||||
* when no reason was recorded (e.g. an image that could not be scanned).
|
||||
*/
|
||||
export function summarizeBlockReasons(
|
||||
violations: ReadonlyArray<{ reasons: readonly PolicyBlockReason[] }>,
|
||||
): string {
|
||||
const labels = new Set<string>();
|
||||
for (const v of violations) {
|
||||
for (const r of v.reasons) labels.add(describeReason(r));
|
||||
}
|
||||
return labels.size > 0 ? [...labels].join(' + ') : 'scan policy conditions';
|
||||
}
|
||||
|
||||
/** Compact descriptor of a policy's active inputs, for log and audit lines. */
|
||||
export function describePolicyInputs(inputs: PolicyRiskInputs): string {
|
||||
const parts: string[] = [];
|
||||
|
||||
Reference in New Issue
Block a user