fix(fleet-sync): hygiene pass on receiver behavior and cleanup (#972)

A bundle of small file-local fixes to the receiver path and
node-deletion flow.

Changes:
- F4 receiver audit log: applyIncomingSync now writes a system audit
  entry on every applied push so mirrored security-rule changes show
  up in the replica's audit panel with a clear control-side origin.
- F7 pilot-agent skip: pushResource explicitly excludes pilot-agent
  nodes (they have no api_url for HTTP push) and warns once per node
  id so the operator sees they will not receive replicated policies.
- B4 identity-drift notification: when targetIdentity differs from
  the cached fleet_self_identity, dispatch a warning so the operator
  can audit any identity-scoped policies that may need re-targeting.
- B6 stack_pattern ReDoS guard: reject patterns with 4+ consecutive
  wildcards or more than 8 wildcards total. Both control-side
  validators (POST/PUT scan policies) and the receiver-side row
  validator share the helper.
- B9 deleteNode cascade: clear fleet_sync_status rows for the node
  inside the existing transaction so the sync-status panel does not
  render ghost entries after a node is removed.
- S6 last_error redaction: formatError strips Bearer tokens and
  JWT-shaped values from error messages and caps at 500 chars before
  storing in fleet_sync_status.last_error or logging.

Tests:
- 8 new vitest cases covering audit-log entry, identity-drift alert,
  pilot-agent warn-once, formatError redaction (Bearer + JWT), ReDoS
  validator rejection, and a backtracking-time smoke test.
- New database-fleet-sync-cascade.test.ts: deleteNode removes
  fleet_sync_status rows for the deleted node and leaves siblings
  untouched.
- Full backend suite: 1792 pass / 5 skipped.
This commit is contained in:
Anso
2026-05-07 13:41:10 -04:00
committed by GitHub
parent f8c75aa6cd
commit 4007709590
7 changed files with 303 additions and 8 deletions
@@ -0,0 +1,60 @@
/**
* Pins the fleet_sync_status cascade behavior of DatabaseService.deleteNode.
*
* Without this cleanup, deleting a node from Settings → Nodes leaves orphaned
* sync-status rows behind that the UI then renders as ghost entries.
*/
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { setupTestDb, cleanupTestDb } from './helpers/setupTestDb';
let tmpDir: string;
let DatabaseService: typeof import('../services/DatabaseService').DatabaseService;
beforeAll(async () => {
tmpDir = await setupTestDb();
({ DatabaseService } = await import('../services/DatabaseService'));
});
afterAll(() => {
cleanupTestDb(tmpDir);
});
describe('deleteNode fleet_sync_status cascade', () => {
it('removes fleet_sync_status rows for the deleted node', () => {
const db = DatabaseService.getInstance();
const nodeId = db.addNode({
name: 'cascade-target',
type: 'remote',
compose_dir: '/app/compose',
is_default: false,
api_url: 'https://cascade.example',
api_token: 'tok',
mode: 'proxy',
});
// Sibling node so we can confirm its rows survive.
const siblingId = db.addNode({
name: 'cascade-sibling',
type: 'remote',
compose_dir: '/app/compose',
is_default: false,
api_url: 'https://sibling.example',
api_token: 'tok',
mode: 'proxy',
});
db.recordFleetSyncFailure(nodeId, 'scan_policies', 'timeout');
db.recordFleetSyncFailure(nodeId, 'cve_suppressions', 'timeout');
db.recordFleetSyncSuccess(siblingId, 'scan_policies');
const before = db.getFleetSyncStatuses();
expect(before.filter((s) => s.node_id === nodeId)).toHaveLength(2);
expect(before.filter((s) => s.node_id === siblingId)).toHaveLength(1);
db.deleteNode(nodeId);
const after = db.getFleetSyncStatuses();
expect(after.filter((s) => s.node_id === nodeId)).toHaveLength(0);
expect(after.filter((s) => s.node_id === siblingId)).toHaveLength(1);
});
});
@@ -274,6 +274,47 @@ describe('POST /api/fleet/role/reanchor', () => {
});
});
describe('POST /api/fleet/sync/:resource stack_pattern ReDoS guard', () => {
it('rejects 4+ consecutive wildcards in a stack_pattern', async () => {
const res = await request(app)
.post('/api/fleet/sync/scan_policies')
.set('Authorization', nodeProxyAuthHeader)
.send({
rows: [{
name: 'redos', node_identity: '', stack_pattern: 'foo****bar',
max_severity: 'CRITICAL', block_on_deploy: 0, enabled: 1,
}],
});
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/4\+ consecutive wildcards/);
});
it('rejects more than 8 wildcards anywhere in a stack_pattern', async () => {
const res = await request(app)
.post('/api/fleet/sync/scan_policies')
.set('Authorization', nodeProxyAuthHeader)
.send({
rows: [{
name: 'too-many-stars', node_identity: '',
stack_pattern: '*a*b*c*d*e*f*g*h*i*',
max_severity: 'CRITICAL', block_on_deploy: 0, enabled: 1,
}],
});
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/too many wildcards/);
});
it('regex compiled from a max-allowed pattern matches in under 50ms on a worst-case input', () => {
const pattern = 'a*b*c*d*e*f*g*h'; // 7 stars, allowed.
const escaped = pattern.replace(/[.+?^${}()|[\]\\]/g, '\\$&').replace(/\*/g, '.*');
const re = new RegExp('^' + escaped + '$');
const target = 'a' + 'a'.repeat(2000);
const start = Date.now();
re.test(target);
expect(Date.now() - start).toBeLessThan(50);
});
});
describe('POST /api/fleet/role/demote', () => {
it('returns 401 without auth', async () => {
const res = await request(app).post('/api/fleet/role/demote').send({ confirm: true });
@@ -13,6 +13,7 @@ const {
mockReplaceReplicatedCveSuppressions,
mockClearOrphanPolicyEvaluations,
mockClearReplicatedRows,
mockInsertAuditLog,
mockRecordFleetSyncSuccess,
mockRecordFleetSyncFailure,
mockGetSystemState,
@@ -29,6 +30,7 @@ const {
mockReplaceReplicatedCveSuppressions: vi.fn(),
mockClearOrphanPolicyEvaluations: vi.fn(),
mockClearReplicatedRows: vi.fn(),
mockInsertAuditLog: vi.fn(),
mockRecordFleetSyncSuccess: vi.fn(),
mockRecordFleetSyncFailure: vi.fn(),
mockGetSystemState: vi.fn().mockReturnValue(null),
@@ -48,6 +50,7 @@ vi.mock('../services/DatabaseService', () => ({
replaceReplicatedCveSuppressions: mockReplaceReplicatedCveSuppressions,
clearOrphanPolicyEvaluations: mockClearOrphanPolicyEvaluations,
clearReplicatedRows: mockClearReplicatedRows,
insertAuditLog: mockInsertAuditLog,
recordFleetSyncSuccess: mockRecordFleetSyncSuccess,
recordFleetSyncFailure: mockRecordFleetSyncFailure,
getSystemState: mockGetSystemState,
@@ -501,3 +504,93 @@ describe('FleetSyncService.getControlIdentity', () => {
expect(FleetSyncService.getControlIdentity()).toBe('');
});
});
describe('FleetSyncService incoming-sync side effects', () => {
it('writes a system audit log entry on every applied sync', () => {
FleetSyncService.getInstance().applyIncomingSync(
'scan_policies',
[],
'https://me.example',
1_700_000_000_000,
'fingerprint-aaa',
);
expect(mockInsertAuditLog).toHaveBeenCalledWith(expect.objectContaining({
username: 'system',
method: 'POST',
path: '/api/fleet/sync/scan_policies',
ip_address: 'control',
summary: expect.stringContaining('Replicated scan_policies from fingerprint-aaa'),
}));
});
it('emits an identity-drift notification when the targetIdentity changes', () => {
mockGetSystemState.mockImplementation((key: string) => {
if (key === 'fleet_self_identity') return 'https://old.example';
return null;
});
FleetSyncService.getInstance().applyIncomingSync(
'scan_policies',
[],
'https://new.example',
);
expect(mockDispatchAlert).toHaveBeenCalledWith(
'warning',
'system',
expect.stringContaining('Fleet self-identity changed'),
);
});
it('does not emit identity-drift when there is no prior identity (first sync)', () => {
mockGetSystemState.mockImplementation(() => null);
FleetSyncService.getInstance().applyIncomingSync(
'scan_policies',
[],
'https://first.example',
);
const driftCalls = mockDispatchAlert.mock.calls.filter((c) =>
typeof c[2] === 'string' && c[2].includes('self-identity changed'),
);
expect(driftCalls).toHaveLength(0);
});
});
describe('FleetSyncService.pushResource pilot-agent skip', () => {
it('skips pilot-agent nodes and logs warn-once per node id', async () => {
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
mockGetNodes.mockReturnValue([
{ id: 50, type: 'remote', mode: 'pilot_agent', api_url: '', api_token: '', name: 'PilotNode' },
]);
await FleetSyncService.getInstance().pushResource('scan_policies');
await FleetSyncService.getInstance().pushResource('scan_policies');
const pilotWarns = warnSpy.mock.calls.filter((c) =>
typeof c[0] === 'string' && c[0].includes('pilot-agent') && c[0].includes('PilotNode'),
);
expect(pilotWarns).toHaveLength(1);
expect(mockAxiosPost).not.toHaveBeenCalled();
warnSpy.mockRestore();
});
});
describe('FleetSyncService.formatError redaction', () => {
it('redacts Bearer tokens and JWT-shaped values from error messages', async () => {
mockGetNodes.mockReturnValue([
{ id: 51, type: 'remote', api_url: 'https://leak.example', api_token: 'tok', name: 'leak' },
]);
mockGetLocalScanPolicies.mockReturnValue([]);
mockAxiosPost.mockImplementation(async () => {
const { AxiosError } = await import('axios');
const err = new AxiosError('Request failed');
(err as unknown as { response: unknown }).response = {
status: 500,
statusText: 'Server Error',
data: { error: 'Authorization: Bearer abc.def-_~+/=secrettoken denied; jwt eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJ1c2VyIn0.signaturePart' },
};
throw err;
});
await FleetSyncService.getInstance().pushResource('scan_policies');
const failure = mockRecordFleetSyncFailure.mock.calls[0];
expect(failure[2]).not.toMatch(/Bearer\s+[A-Za-z0-9]/);
expect(failure[2]).toContain('[redacted]');
expect(failure[2]).toContain('[redacted-jwt]');
});
});
+22 -1
View File
@@ -48,7 +48,10 @@ function validateScanPolicyRow(row: unknown): string | null {
if (typeof r.name !== 'string' || r.name.length === 0 || r.name.length > 200) return 'name must be a non-empty string';
if (typeof r.max_severity !== 'string' || !POLICY_SEVERITIES.has(r.max_severity)) return 'max_severity must be CRITICAL, HIGH, MEDIUM, or LOW';
if (r.stack_pattern !== null && typeof r.stack_pattern !== 'string') return 'stack_pattern must be a string or null';
if (typeof r.stack_pattern === 'string' && r.stack_pattern.length > 200) return 'stack_pattern is too long';
if (typeof r.stack_pattern === 'string') {
const patternError = validateStackPatternForRedos(r.stack_pattern);
if (patternError) return patternError;
}
if (typeof r.node_identity !== 'string') return 'node_identity must be a string';
if (r.node_identity.length > 500) return 'node_identity is too long';
if (!isIntFlag(r.block_on_deploy)) return 'block_on_deploy must be 0 or 1';
@@ -56,6 +59,24 @@ function validateScanPolicyRow(row: unknown): string | null {
return null;
}
/**
* Reject `stack_pattern` inputs that would compile to a backtracking-prone
* regex. The matcher in `getMatchingPolicy` substitutes `*` with `.*`, so a
* pattern like `***...` becomes a chain of adjacent `.*` runs that exhibit
* catastrophic backtracking on long inputs.
*
* Caps mirror the limit in routes/security.ts so a control creating a policy
* sees the same error as a replica receiving one. Length is gated at 200 by
* the surrounding row validator.
*/
export function validateStackPatternForRedos(pattern: string): string | null {
if (pattern.length > 200) return 'stack_pattern is too long';
const stars = (pattern.match(/\*/g) ?? []).length;
if (stars > 8) return 'stack_pattern has too many wildcards (max 8)';
if (/\*{4,}/.test(pattern)) return 'stack_pattern must not contain 4+ consecutive wildcards';
return null;
}
function validateCveSuppressionRow(row: unknown): string | null {
if (!row || typeof row !== 'object') return 'row must be an object';
const r = row as Record<string, unknown>;
+19 -2
View File
@@ -14,6 +14,7 @@ import { sanitizeForLog } from '../utils/safeLog';
import { getErrorMessage } from '../utils/errors';
import { isDebugEnabled } from '../utils/debug';
import { blockIfReplica } from '../middleware/fleetSyncGuards';
import { validateStackPatternForRedos } from './fleet';
import { FINDING_SEVERITIES, POLICY_SEVERITIES } from '../utils/severity';
const CVE_ID_RE = /^(CVE-\d{4}-\d{4,}|GHSA-[\w-]{14,})$/;
@@ -423,13 +424,20 @@ securityRouter.post('/policies', authMiddleware, (req: Request, res: Response):
if (!POLICY_SEVERITIES.has(max_severity)) {
res.status(400).json({ error: 'max_severity must be CRITICAL, HIGH, MEDIUM, or LOW' }); return;
}
const normalizedPattern = stack_pattern ? String(stack_pattern) : null;
if (normalizedPattern !== null) {
const patternError = validateStackPatternForRedos(normalizedPattern);
if (patternError) {
res.status(400).json({ error: patternError }); return;
}
}
try {
const resolvedNodeId = node_id != null ? Number(node_id) : null;
const policy = DatabaseService.getInstance().createScanPolicy({
name: name.trim(),
node_id: resolvedNodeId,
node_identity: FleetSyncService.resolveIdentityForNodeId(resolvedNodeId),
stack_pattern: stack_pattern ? String(stack_pattern) : null,
stack_pattern: normalizedPattern,
max_severity,
block_on_deploy: block_on_deploy ? 1 : 0,
enabled: enabled === false ? 0 : 1,
@@ -459,7 +467,16 @@ securityRouter.put('/policies/:id', authMiddleware, (req: Request, res: Response
updates.node_id = resolvedNodeId;
updates.node_identity = FleetSyncService.resolveIdentityForNodeId(resolvedNodeId);
}
if (body.stack_pattern !== undefined) updates.stack_pattern = body.stack_pattern ? String(body.stack_pattern) : null;
if (body.stack_pattern !== undefined) {
const normalizedPattern = body.stack_pattern ? String(body.stack_pattern) : null;
if (normalizedPattern !== null) {
const patternError = validateStackPatternForRedos(normalizedPattern);
if (patternError) {
res.status(400).json({ error: patternError }); return;
}
}
updates.stack_pattern = normalizedPattern;
}
if (body.max_severity !== undefined) {
if (!POLICY_SEVERITIES.has(body.max_severity)) {
res.status(400).json({ error: 'max_severity must be CRITICAL, HIGH, MEDIUM, or LOW' }); return;
+1
View File
@@ -2097,6 +2097,7 @@ export class DatabaseService {
this.db.prepare('DELETE FROM stack_labels WHERE node_id = ?').run(id);
this.db.prepare('UPDATE blueprints SET pinned_node_id = NULL WHERE pinned_node_id = ?').run(id);
this.deleteRoleAssignmentsByResource('node', String(id));
this.db.prepare('DELETE FROM fleet_sync_status WHERE node_id = ?').run(id);
this.db.prepare('DELETE FROM nodes WHERE id = ?').run(id);
})();
}
+67 -5
View File
@@ -175,6 +175,9 @@ export class FleetSyncService {
private static cachedControlIdentity: string | null = null;
/** Node ids already warned about as pilot-agent skip candidates. */
private static readonly warnedPilotAgents = new Set<number>();
private nextPushedAt(): number {
const now = Date.now();
const next = now > FleetSyncService.lastPushedAt ? now : FleetSyncService.lastPushedAt + 1;
@@ -191,8 +194,21 @@ export class FleetSyncService {
return;
}
const db = DatabaseService.getInstance();
const nodes = db.getNodes().filter((n): n is Node & { id: number } => {
return n.type === 'remote' && Boolean(n.api_url) && Boolean(n.api_token) && n.id != null;
const allNodes = db.getNodes();
// Pilot-agent nodes do not accept fleet-sync HTTP pushes; security
// rules for them are out of scope until pilot tunneling is built. Warn
// once per node id so the operator knows their pilot remote will not
// receive replicated policies.
for (const n of allNodes) {
if (n.mode === 'pilot_agent' && n.id != null && !FleetSyncService.warnedPilotAgents.has(n.id)) {
console.warn(
`[FleetSync] Skipping pilot-agent node "${n.name}" (id=${n.id}); fleet sync over pilot tunnel is out of scope.`,
);
FleetSyncService.warnedPilotAgents.add(n.id);
}
}
const nodes = allNodes.filter((n): n is Node & { id: number } => {
return n.type === 'remote' && n.mode !== 'pilot_agent' && Boolean(n.api_url) && Boolean(n.api_token) && n.id != null;
});
if (nodes.length === 0) {
if (isDebugEnabled()) {
@@ -299,6 +315,21 @@ export class FleetSyncService {
}
db.setSystemState(SYNC_STATE_KEYS.fleetRole, 'replica');
if (targetIdentity) {
const cachedSelf = db.getSystemState(SYNC_STATE_KEYS.fleetSelfIdentity);
if (cachedSelf && cachedSelf !== targetIdentity) {
// Operator changed how the control sees this node (e.g. an
// api_url switch). Notify so they can audit any
// identity-scoped policies that may need re-targeting.
void NotificationService.getInstance()
.dispatchAlert(
'warning',
'system',
`Fleet self-identity changed from "${cachedSelf}" to "${targetIdentity}". Identity-scoped policies are reapplied on the next sync.`,
)
.catch((err) => {
console.warn('[FleetSync] Failed to dispatch identity-drift alert:', err);
});
}
db.setSystemState(SYNC_STATE_KEYS.fleetSelfIdentity, targetIdentity);
}
if (resource === 'scan_policies') {
@@ -306,6 +337,20 @@ export class FleetSyncService {
} else if (resource === 'cve_suppressions') {
db.replaceReplicatedCveSuppressions(rows as Array<Omit<CveSuppression, 'id'>>);
}
// F4: persist an audit-log entry for the operator on the replica
// side. Without this, mirrored security-rule changes happen
// silently from the replica's perspective. Username 'system' and
// ip 'control' make the source unambiguous in the audit panel.
db.insertAuditLog({
timestamp: Date.now(),
username: 'system',
method: 'POST',
path: `/api/fleet/sync/${resource}`,
status_code: 200,
node_id: 0,
ip_address: 'control',
summary: `Replicated ${resource} from ${controlIdentity || 'legacy control'}: replaced ${rows.length} row(s)`,
});
});
}
@@ -504,16 +549,33 @@ export class FleetSyncService {
}
private formatError(err: unknown): string {
let raw: string;
if (err instanceof AxiosError) {
if (err.response) {
const data = err.response.data;
const detail = typeof data === 'object' && data && 'error' in data
? String((data as { error: unknown }).error)
: err.response.statusText;
return `HTTP ${err.response.status}: ${detail}`;
raw = `HTTP ${err.response.status}: ${detail}`;
} else {
raw = err.message;
}
return err.message;
} else {
raw = err instanceof Error ? err.message : String(err);
}
return err instanceof Error ? err.message : String(err);
return FleetSyncService.redactSensitive(raw);
}
/**
* Strip Bearer tokens and JWT-shaped substrings from an error message
* before it lands on disk in `fleet_sync_status.last_error` (visible in the
* UI sync-status panel) or in any log line. Caps the result at 500 chars
* to bound storage growth on pathological remote responses.
*/
private static redactSensitive(message: string): string {
const redacted = message
.replace(/Bearer\s+[A-Za-z0-9\-._~+/=]+/gi, 'Bearer [redacted]')
.replace(/[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}/g, '[redacted-jwt]');
return redacted.length > 500 ? redacted.slice(0, 497) + '...' : redacted;
}
}