fix(notifications): harden mute replica auth and versioned retracts

requireNodeProxy checked username === 'node-proxy', so a normal session
with that account name could write mute-rule replicas and fleet sync
payloads. Gate on verified machineAuthScope, reserve the username, and
avoid SSO colliding with the synthetic identity.

Capability-cleanup DELETE also wrote a permanent tombstone, so clearing
a schedule (or upgrading a remote) could never re-sync that rule id.
Pass until_updated_at on fleet retracts and allow strictly newer POSTs
to recreate after cleanup while still blocking delayed same-version pushes.

Co-authored-by: Anso <dev@anso.codes>
This commit is contained in:
Cursor Agent
2026-07-22 11:09:37 +00:00
parent a3edee5e6a
commit 9b067b4f4c
10 changed files with 221 additions and 37 deletions
@@ -66,6 +66,30 @@ describe('POST /api/fleet/sync/:resource auth gate', () => {
expect(res.body.code).toBe('NODE_PROXY_REQUIRED');
});
it('rejects a DB user named node-proxy (username alone is not machine auth)', async () => {
const bcrypt = await import('bcrypt');
const { DatabaseService } = await import('../services/DatabaseService');
const db = DatabaseService.getInstance();
const hash = await bcrypt.hash('proxypass', 1);
const id = db.addUser({ username: 'node-proxy', password_hash: hash, role: 'admin' });
try {
const login = await request(app)
.post('/api/auth/login')
.send({ username: 'node-proxy', password: 'proxypass' });
expect(login.status).toBe(200);
const cookies = login.headers['set-cookie'] as string | string[];
const cookie = Array.isArray(cookies) ? cookies[0] : cookies;
const res = await request(app)
.post('/api/fleet/sync/scan_policies')
.set('Cookie', cookie)
.send({ rows: [validRow] });
expect(res.status).toBe(403);
expect(res.body.code).toBe('NODE_PROXY_REQUIRED');
} finally {
db.deleteUser(id);
}
});
it('rejects unknown resources with 400', async () => {
const res = await request(app)
.post('/api/fleet/sync/foo')
@@ -506,7 +506,7 @@ describe('Notification suppression - CRUD', () => {
DatabaseService.getInstance().deleteNotificationSuppressionRule(940005);
});
it('replica does not resurrect a rule after it was deleted, even with a newer updated_at', async () => {
it('replica does not resurrect a rule after permanent DELETE, even with a newer updated_at', async () => {
const jwt = await import('jsonwebtoken');
const { TEST_JWT_SECRET } = await import('./helpers/testConstants');
const token = jwt.default.sign({ scope: 'node_proxy' }, TEST_JWT_SECRET, { expiresIn: '1m' });
@@ -532,6 +532,7 @@ describe('Notification suppression - CRUD', () => {
expect(first.status).toBe(200);
expect(DatabaseService.getInstance().getNotificationSuppressionRule(950006)).not.toBeUndefined();
// Authoritative delete (no until_updated_at): permanent tombstone.
const del = await request(app)
.delete('/api/notification-suppression-rules/replica/950006')
.set('Authorization', `Bearer ${token}`);
@@ -540,7 +541,7 @@ describe('Notification suppression - CRUD', () => {
// A delayed POST arrives after the DELETE, reordered by the network. Even
// though its updated_at is newer than anything the sender ever sent before
// the delete, the delete is authoritative: this id must stay gone.
// the delete, the permanent delete is authoritative: this id must stay gone.
const delayed = await request(app)
.post('/api/notification-suppression-rules/replica')
.set('Authorization', `Bearer ${token}`)
@@ -549,17 +550,18 @@ describe('Notification suppression - CRUD', () => {
expect(DatabaseService.getInstance().getNotificationSuppressionRule(950006)).toBeUndefined();
});
it('replica DELETE tombstones an id even when the remote never had that row', async () => {
it('replica DELETE versioned retract blocks delayed same-version POSTs even when the row was absent', async () => {
const jwt = await import('jsonwebtoken');
const { TEST_JWT_SECRET } = await import('./helpers/testConstants');
const token = jwt.default.sign({ scope: 'node_proxy' }, TEST_JWT_SECRET, { expiresIn: '1m' });
// This remote never received rule 960007 (e.g. it just enrolled, or the rule
// failed capability probing before its first push). A cleanup DELETE still
// arrives unconditionally from deleteRuleOnNode. A POST reordered behind it
// must not be allowed to create the rule for the first time.
// arrives with until_updated_at from deleteRuleOnNode. A POST reordered
// behind it at the same version must not create the rule.
const del = await request(app)
.delete('/api/notification-suppression-rules/replica/960007')
.query({ until_updated_at: 1 })
.set('Authorization', `Bearer ${token}`);
expect(del.status).toBe(200);
expect(DatabaseService.getInstance().getNotificationSuppressionRule(960007)).toBeUndefined();
@@ -587,7 +589,7 @@ describe('Notification suppression - CRUD', () => {
expect(DatabaseService.getInstance().getNotificationSuppressionRule(960007)).toBeUndefined();
});
it('replica does not resurrect a deleted rule with a schedule', async () => {
it('replica versioned retract refuses same-version delayed push but allows a newer re-sync', async () => {
const jwt = await import('jsonwebtoken');
const { TEST_JWT_SECRET } = await import('./helpers/testConstants');
const token = jwt.default.sign({ scope: 'node_proxy' }, TEST_JWT_SECRET, { expiresIn: '1m' });
@@ -614,19 +616,80 @@ describe('Notification suppression - CRUD', () => {
expect(first.status).toBe(200);
expect(DatabaseService.getInstance().getNotificationSuppressionRule(970008)?.schedule).not.toBeNull();
// This is the worst case the fix protects: a capability-cleanup DELETE
// retracts an all-day/scheduled mute from a node that stopped supporting
// it. A delayed re-push of the scheduled rule must not undo that cleanup.
// Capability-cleanup DELETE retracts the scheduled mute. until_updated_at
// matches the scheduled version so a delayed re-push of that version stays gone.
const del = await request(app)
.delete('/api/notification-suppression-rules/replica/970008')
.query({ until_updated_at: 1000 })
.set('Authorization', `Bearer ${token}`);
expect(del.status).toBe(200);
const delayed = await request(app)
const delayedSame = await request(app)
.post('/api/notification-suppression-rules/replica')
.set('Authorization', `Bearer ${token}`)
.send({ rule: replicaRule({ updated_at: 2000 }) });
expect(delayed.status).toBe(200);
.send({ rule: replicaRule({ updated_at: 1000 }) });
expect(delayedSame.status).toBe(200);
expect(DatabaseService.getInstance().getNotificationSuppressionRule(970008)).toBeUndefined();
// Later the hub clears the schedule (or the remote gains schedule support) and
// re-syncs with a newer updated_at — that must recreate the replica.
const newer = await request(app)
.post('/api/notification-suppression-rules/replica')
.set('Authorization', `Bearer ${token}`)
.send({ rule: replicaRule({ updated_at: 2000, schedule: null, name: 'replica-after-schedule-clear' }) });
expect(newer.status).toBe(200);
const restored = DatabaseService.getInstance().getNotificationSuppressionRule(970008);
expect(restored).not.toBeUndefined();
expect(restored?.schedule).toBeNull();
expect(restored?.name).toBe('replica-after-schedule-clear');
DatabaseService.getInstance().deleteNotificationSuppressionRule(970008);
});
it('replica endpoints reject a normal user session even if username is node-proxy', async () => {
const bcrypt = await import('bcrypt');
const db = DatabaseService.getInstance();
// Bypass validateUsername to simulate a pre-existing/confused account that
// collides with the synthetic machine identity string.
const hash = await bcrypt.hash('proxypass', 1);
const id = db.addUser({ username: 'node-proxy', password_hash: hash, role: 'viewer' });
try {
const login = await request(app)
.post('/api/auth/login')
.send({ username: 'node-proxy', password: 'proxypass' });
expect(login.status).toBe(200);
const cookies = login.headers['set-cookie'] as string | string[];
const cookie = Array.isArray(cookies) ? cookies[0] : cookies;
const post = await request(app)
.post('/api/notification-suppression-rules/replica')
.set('Cookie', cookie)
.send({
rule: {
id: 980009,
name: 'spoofed-proxy',
applies_to: 'both',
stack_patterns: [],
node_id: null,
label_ids: null,
categories: null,
levels: null,
enabled: true,
expires_at: null,
created_at: 1,
updated_at: 1,
},
});
expect(post.status).toBe(403);
expect(post.body.code).toBe('NODE_PROXY_REQUIRED');
expect(db.getNotificationSuppressionRule(980009)).toBeUndefined();
const del = await request(app)
.delete('/api/notification-suppression-rules/replica/1')
.set('Cookie', cookie);
expect(del.status).toBe(403);
expect(del.body.code).toBe('NODE_PROXY_REQUIRED');
} finally {
db.deleteUser(id);
}
});
});
@@ -128,12 +128,15 @@ describe('notificationSuppressionSync', () => {
mockFetch.mockResolvedValue({ ok: true, status: 200, text: async () => '' });
syncSuppressionRuleToFleet(makeRule({
id: 42,
updated_at: 77,
node_id: 10,
schedule: { days: [1], start_minute: 0, end_minute: 60, tz: 'UTC' },
}));
await vi.waitFor(() => expect(mockFetch).toHaveBeenCalled());
expect(mockFetch.mock.calls.every((c) => (c[1] as { method: string }).method === 'DELETE')).toBe(true);
expect(String(mockFetch.mock.calls[0][0])).toContain('until_updated_at=77');
expect(warn.mock.calls.some((c) => String(c[0]).includes('replica was removed'))).toBe(true);
expect(error.mock.calls.some((c) => String(c[0]).includes('cleanup pending'))).toBe(false);
});
@@ -262,13 +265,14 @@ describe('notificationSuppressionSync', () => {
});
it('stale targets receive DELETE on scope change', async () => {
const previous = makeRule({ node_id: null, schedule: null });
const updated = makeRule({ node_id: 10, schedule: null });
const previous = makeRule({ node_id: null, schedule: null, updated_at: 55 });
const updated = makeRule({ node_id: 10, schedule: null, updated_at: 56 });
syncSuppressionRuleUpdateToFleet(previous, updated);
await vi.waitFor(() => expect(mockFetch.mock.calls.length).toBeGreaterThanOrEqual(2));
const deletes = mockFetch.mock.calls.filter((c) => (c[1] as { method: string }).method === 'DELETE');
expect(deletes.some((c) => String(c[0]).includes('node-11'))).toBe(true);
expect(deletes.some((c) => String(c[0]).includes('until_updated_at=55'))).toBe(true);
const posts = mockFetch.mock.calls.filter((c) => (c[1] as { method: string }).method === 'POST');
expect(posts.some((c) => String(c[0]).includes('node-10'))).toBe(true);
});
+9
View File
@@ -74,6 +74,15 @@ describe('POST /api/users', () => {
expect(res.status).toBe(400);
});
it('rejects reserved node-proxy username (400)', async () => {
const res = await request(app)
.post('/api/users')
.set('Authorization', `Bearer ${adminToken()}`)
.send({ username: 'node-proxy', password: 'password123', role: 'viewer' });
expect(res.status).toBe(400);
expect(res.body.error).toMatch(/reserved/i);
});
it('rejects short password (400)', async () => {
const res = await request(app)
.post('/api/users')
@@ -58,13 +58,19 @@ async function pushRuleToNode(node: Node, rule: NotificationSuppressionRule): Pr
}
}
async function deleteRuleOnNode(node: Node, ruleId: number): Promise<void> {
async function deleteRuleOnNode(node: Node, ruleId: number, untilUpdatedAt: number): Promise<void> {
const target = NodeRegistry.getInstance().getProxyTarget(node.id);
if (!target?.apiUrl) {
throw new Error(`no proxy target for node "${node.name}" (id=${node.id})`);
}
const baseUrl = target.apiUrl.replace(/\/$/, '');
const res = await fetch(`${baseUrl}/api/notification-suppression-rules/replica/${ruleId}`, {
// Versioned retract: remotes refuse delayed POSTs with updated_at <= untilUpdatedAt,
// but a later hub save with a newer updated_at can recreate the replica (e.g. after
// schedule-capability cleanup followed by clearing the schedule).
const url =
`${baseUrl}/api/notification-suppression-rules/replica/${ruleId}` +
`?until_updated_at=${encodeURIComponent(String(untilUpdatedAt))}`;
const res = await fetch(url, {
method: 'DELETE',
headers: buildRemoteHeaders(target.apiToken),
signal: AbortSignal.timeout(SYNC_TIMEOUT_MS),
@@ -88,7 +94,7 @@ async function pushOrCleanupScheduled(node: Node, rule: NotificationSuppressionR
// through the legacy contract (older remotes would mute all day). Attempt DELETE;
// only claim cleanup when DELETE succeeds.
try {
await deleteRuleOnNode(node, rule.id);
await deleteRuleOnNode(node, rule.id, rule.updated_at);
console.warn(
`[SuppressionSync] Scheduled rule ${rule.id} not applied on node "${node.name}" (id=${node.id}): ` +
`capability unsupported-or-unreachable; DELETE succeeded and replica was removed`,
@@ -106,7 +112,7 @@ async function cleanupInvalidScheduleReplica(node: Node, rule: NotificationSuppr
// Never POST an invalid schedule (would mute all day on remotes that ignore the field).
// Attempt DELETE so a prior valid/unscheduled replica cannot keep muting.
try {
await deleteRuleOnNode(node, rule.id);
await deleteRuleOnNode(node, rule.id, rule.updated_at);
console.warn(
`[SuppressionSync] Corrupt schedule on rule ${rule.id}: replica removed on node "${node.name}" (id=${node.id}); not posting`,
);
@@ -166,7 +172,7 @@ export function syncSuppressionRuleUpdateToFleet(
const node = db.getNode(id);
if (!node || node.type !== 'remote') return;
try {
await deleteRuleOnNode(node, previous.id);
await deleteRuleOnNode(node, previous.id, previous.updated_at);
} catch (err) {
console.error(
`[SuppressionSync] Failed to delete stale rule ${previous.id} on node "${node.name}":`,
@@ -194,7 +200,7 @@ export function deleteSuppressionRuleFromFleet(rule: NotificationSuppressionRule
void Promise.allSettled(
targets.map(async (node) => {
try {
await deleteRuleOnNode(node, rule.id);
await deleteRuleOnNode(node, rule.id, rule.updated_at);
} catch (err) {
console.error(
`[SuppressionSync] Failed to delete rule ${rule.id} on node "${node.name}":`,
+11
View File
@@ -6,9 +6,20 @@
*
* Returns an error message, or null when the value is acceptable.
*/
/** Machine-credential synthetic username set by authMiddleware for node_proxy JWTs. */
const RESERVED_USERNAMES = new Set(['node-proxy']);
export function isReservedUsername(value: string): boolean {
return RESERVED_USERNAMES.has(value.toLowerCase());
}
export function validateUsername(value: unknown): string | null {
if (typeof value !== 'string' || value.length < 3 || !/^[a-zA-Z0-9_-]+$/.test(value)) {
return 'Username must be at least 3 characters (letters, numbers, underscore, hyphen)';
}
if (isReservedUsername(value)) {
return 'Username is reserved';
}
return null;
}
+8 -1
View File
@@ -50,9 +50,16 @@ export const requireUserSession = (req: Request, res: Response): boolean => {
/**
* Accept only calls from a sibling Sencho using its node_proxy Bearer token.
* Browser sessions, API tokens, and console tokens are all rejected.
*
* Must check verified machineAuthScope — not username. authMiddleware maps
* node_proxy JWTs to username `node-proxy`, but a normal user account can be
* given that same name; trusting the username alone would let that session
* reach replica/fleet-sync write paths.
*/
export const requireNodeProxy = (req: Request, res: Response): boolean => {
if (req.user?.username !== 'node-proxy') return deny(res, 'NODE_PROXY_REQUIRED', 'Node proxy authentication required.');
if (req.machineAuthScope !== 'node_proxy') {
return deny(res, 'NODE_PROXY_REQUIRED', 'Node proxy authentication required.');
}
return true;
};
+15 -1
View File
@@ -577,7 +577,21 @@ notificationSuppressionRouter.delete('/replica/:id', authMiddleware, (req: Reque
try {
const id = parseIntParam(req, res, 'id', 'suppression rule ID');
if (id === null) return;
DatabaseService.getInstance().deleteNotificationSuppressionRule(id);
const untilRaw = req.query.until_updated_at;
let untilUpdatedAt: number | undefined;
if (untilRaw !== undefined) {
if (typeof untilRaw !== 'string' || untilRaw.trim() === '') {
res.status(400).json({ error: 'until_updated_at must be a non-negative number' });
return;
}
const parsed = Number(untilRaw);
if (!Number.isFinite(parsed) || parsed < 0) {
res.status(400).json({ error: 'until_updated_at must be a non-negative number' });
return;
}
untilUpdatedAt = parsed;
}
DatabaseService.getInstance().deleteNotificationSuppressionRule(id, untilUpdatedAt);
res.json({ success: true });
} catch (error) {
console.error('Failed to delete suppression rule replica:', error);
+59 -15
View File
@@ -2206,10 +2206,15 @@ export class DatabaseService {
ON notification_suppression_rules(enabled, expires_at);
CREATE TABLE IF NOT EXISTS notification_suppression_rule_tombstones (
id INTEGER PRIMARY KEY,
deleted_at INTEGER NOT NULL
deleted_at INTEGER NOT NULL,
reject_until_updated_at INTEGER NULL
);
`);
this.tryAddColumn('notification_suppression_rules', 'schedule', 'TEXT NULL');
// NULL reject_until_updated_at = permanent (authoritative delete). A numeric
// value means fleet cleanup/scope retract: refuse only POSTs with
// updated_at <= that watermark so a later legitimate re-sync can recreate.
this.tryAddColumn('notification_suppression_rule_tombstones', 'reject_until_updated_at', 'INTEGER NULL');
}
private migrateNotificationHistoryContext(): void {
@@ -2900,14 +2905,23 @@ export class DatabaseService {
return;
}
const tombstone = this.db.prepare(
'SELECT deleted_at FROM notification_suppression_rule_tombstones WHERE id = ?',
).get(rule.id) as { deleted_at: number } | undefined;
'SELECT deleted_at, reject_until_updated_at FROM notification_suppression_rule_tombstones WHERE id = ?',
).get(rule.id) as { deleted_at: number; reject_until_updated_at: number | null } | undefined;
if (tombstone) {
console.warn(
`[DatabaseService] Ignoring suppression replica write for rule id=${sanitizeForLog(rule.id)}: ` +
`this id was deleted at ${sanitizeForLog(tombstone.deleted_at)} and must not be recreated`,
);
return;
const until = tombstone.reject_until_updated_at;
// Permanent tombstone (authoritative delete): never recreate.
// Versioned retract (fleet cleanup): allow only strictly newer saves.
if (until === null || rule.updated_at <= until) {
console.warn(
`[DatabaseService] Ignoring suppression replica write for rule id=${sanitizeForLog(rule.id)}: ` +
`this id was deleted at ${sanitizeForLog(tombstone.deleted_at)}` +
(until === null
? ' and must not be recreated'
: ` (reject_until_updated_at=${sanitizeForLog(until)}; incoming updated_at=${sanitizeForLog(rule.updated_at)})`),
);
return;
}
this.db.prepare('DELETE FROM notification_suppression_rule_tombstones WHERE id = ?').run(rule.id);
}
this.db.prepare(
`INSERT INTO notification_suppression_rules
@@ -2957,18 +2971,48 @@ export class DatabaseService {
this.db.prepare(`UPDATE notification_suppression_rules SET ${fields.join(', ')} WHERE id = ?`).run(...values);
}
public deleteNotificationSuppressionRule(id: number): number {
/**
* Delete a suppression rule and record a tombstone so in-flight replica POSTs
* cannot resurrect it.
*
* @param untilUpdatedAt When set, the tombstone is a versioned fleet retract:
* refuse replica POSTs with `updated_at <= untilUpdatedAt`, but allow a later
* legitimate re-sync whose `updated_at` is strictly newer (e.g. schedule cleared
* after a capability-cleanup DELETE). When omitted, the tombstone is permanent
* (authoritative user delete on this node).
*/
public deleteNotificationSuppressionRule(id: number, untilUpdatedAt?: number): number {
const changes = this.db.prepare('DELETE FROM notification_suppression_rules WHERE id = ?').run(id).changes;
// Fleet sync has no delivery ordering guarantee: a replica POST for this id can
// still be in flight. Record the delete permanently (ids are AUTOINCREMENT and
// never reused) so upsertNotificationSuppressionRuleReplica refuses to resurrect it.
// still be in flight. Record the delete (ids are AUTOINCREMENT and never reused)
// so upsertNotificationSuppressionRuleReplica refuses stale recreations.
// Tombstone unconditionally, even when changes is 0: deleteRuleOnNode issues this
// same DELETE for capability/invalid-schedule cleanup against remotes that may never
// have received the rule yet, and a reordered POST behind that DELETE must not create it.
this.db.prepare(
`INSERT INTO notification_suppression_rule_tombstones (id, deleted_at) VALUES (?, ?)
ON CONFLICT(id) DO UPDATE SET deleted_at = excluded.deleted_at`,
).run(id, Date.now());
if (untilUpdatedAt !== undefined) {
this.db.prepare(
`INSERT INTO notification_suppression_rule_tombstones (id, deleted_at, reject_until_updated_at)
VALUES (?, ?, ?)
ON CONFLICT(id) DO UPDATE SET
deleted_at = excluded.deleted_at,
reject_until_updated_at = CASE
WHEN notification_suppression_rule_tombstones.reject_until_updated_at IS NULL THEN NULL
WHEN excluded.reject_until_updated_at IS NULL THEN NULL
ELSE MAX(
notification_suppression_rule_tombstones.reject_until_updated_at,
excluded.reject_until_updated_at
)
END`,
).run(id, Date.now(), untilUpdatedAt);
} else {
this.db.prepare(
`INSERT INTO notification_suppression_rule_tombstones (id, deleted_at, reject_until_updated_at)
VALUES (?, ?, NULL)
ON CONFLICT(id) DO UPDATE SET
deleted_at = excluded.deleted_at,
reject_until_updated_at = NULL`,
).run(id, Date.now());
}
return changes;
}
+2
View File
@@ -603,6 +603,8 @@ export class SSOService {
// Generate unique username
let username = params.preferredUsername.replace(/[^a-zA-Z0-9_-]/g, '_').substring(0, 50);
if (!username) username = 'sso_user';
// Avoid colliding with the synthetic node_proxy machine identity.
if (username.toLowerCase() === 'node-proxy') username = 'node-proxy_sso';
if (db.getUserByUsername(username)) {
const suffix = params.authProvider.replace('oidc_', '');
username = `${username}_${suffix}`;