mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-07-26 11:49:16 +00:00
fix: enforce replica node_id and guard fleet sync against stale writes
Two hardenings to the suppression-rule fleet sync path found during review: the /replica endpoint trusted the payload's node_id instead of forcing it to null server-side, so a direct proxy-authenticated caller could persist a scoped replica; and upsertNotificationSuppressionRuleReplica overwrote unconditionally with no ordering check, so a delayed older POST arriving after a newer one could downgrade the stored rule. Force node_id to null on every replica write, and skip (with a warning log) any incoming write whose updated_at is not newer than what's stored.
This commit is contained in:
@@ -427,4 +427,75 @@ describe('Notification suppression - CRUD', () => {
|
|||||||
});
|
});
|
||||||
DatabaseService.getInstance().deleteNotificationSuppressionRule(9002);
|
DatabaseService.getInstance().deleteNotificationSuppressionRule(9002);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('replica forces node_id to null regardless of the payload value', 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' });
|
||||||
|
|
||||||
|
const res = await request(app)
|
||||||
|
.post('/api/notification-suppression-rules/replica')
|
||||||
|
.set('Authorization', `Bearer ${token}`)
|
||||||
|
.send({
|
||||||
|
rule: {
|
||||||
|
id: 9004,
|
||||||
|
name: 'replica-scoped',
|
||||||
|
applies_to: 'both',
|
||||||
|
stack_patterns: [],
|
||||||
|
node_id: 5,
|
||||||
|
label_ids: null,
|
||||||
|
categories: null,
|
||||||
|
levels: null,
|
||||||
|
enabled: true,
|
||||||
|
expires_at: null,
|
||||||
|
created_at: 1,
|
||||||
|
updated_at: 1,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(DatabaseService.getInstance().getNotificationSuppressionRule(9004)?.node_id).toBeNull();
|
||||||
|
DatabaseService.getInstance().deleteNotificationSuppressionRule(9004);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('replica ignores a stale write with an older updated_at than the stored row', 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' });
|
||||||
|
const replicaRule = (overrides: Record<string, unknown>) => ({
|
||||||
|
id: 9005,
|
||||||
|
name: 'replica-race',
|
||||||
|
applies_to: 'both',
|
||||||
|
stack_patterns: [],
|
||||||
|
node_id: null,
|
||||||
|
label_ids: null,
|
||||||
|
categories: null,
|
||||||
|
levels: null,
|
||||||
|
enabled: true,
|
||||||
|
expires_at: null,
|
||||||
|
created_at: 1,
|
||||||
|
...overrides,
|
||||||
|
});
|
||||||
|
|
||||||
|
const first = await request(app)
|
||||||
|
.post('/api/notification-suppression-rules/replica')
|
||||||
|
.set('Authorization', `Bearer ${token}`)
|
||||||
|
.send({ rule: replicaRule({ name: 'v2-newer', updated_at: 2000 }) });
|
||||||
|
expect(first.status).toBe(200);
|
||||||
|
|
||||||
|
const stale = await request(app)
|
||||||
|
.post('/api/notification-suppression-rules/replica')
|
||||||
|
.set('Authorization', `Bearer ${token}`)
|
||||||
|
.send({ rule: replicaRule({ name: 'v1-delayed-stale', updated_at: 1000 }) });
|
||||||
|
expect(stale.status).toBe(200);
|
||||||
|
expect(DatabaseService.getInstance().getNotificationSuppressionRule(9005)?.name).toBe('v2-newer');
|
||||||
|
|
||||||
|
const newer = await request(app)
|
||||||
|
.post('/api/notification-suppression-rules/replica')
|
||||||
|
.set('Authorization', `Bearer ${token}`)
|
||||||
|
.send({ rule: replicaRule({ name: 'v3-newest', updated_at: 3000 }) });
|
||||||
|
expect(newer.status).toBe(200);
|
||||||
|
expect(DatabaseService.getInstance().getNotificationSuppressionRule(9005)?.name).toBe('v3-newest');
|
||||||
|
|
||||||
|
DatabaseService.getInstance().deleteNotificationSuppressionRule(9005);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -561,7 +561,9 @@ notificationSuppressionRouter.post('/replica', authMiddleware, (req: Request, re
|
|||||||
scheduleInvalid: false,
|
scheduleInvalid: false,
|
||||||
enabled: rule.enabled !== false,
|
enabled: rule.enabled !== false,
|
||||||
expires_at: rule.expires_at ?? null,
|
expires_at: rule.expires_at ?? null,
|
||||||
node_id: rule.node_id ?? null,
|
// Replicas are always node-agnostic on the receiving node: the hub's node_id
|
||||||
|
// is a hub-local scoping concept and never trustworthy as a foreign key here.
|
||||||
|
node_id: null,
|
||||||
});
|
});
|
||||||
res.json({ success: true });
|
res.json({ success: true });
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
|||||||
@@ -2782,6 +2782,15 @@ export class DatabaseService {
|
|||||||
const scheduleJson = rule.schedule ? JSON.stringify(rule.schedule) : null;
|
const scheduleJson = rule.schedule ? JSON.stringify(rule.schedule) : null;
|
||||||
const existing = this.getNotificationSuppressionRule(rule.id);
|
const existing = this.getNotificationSuppressionRule(rule.id);
|
||||||
if (existing) {
|
if (existing) {
|
||||||
|
// Fleet sync is fire-and-forget with no delivery ordering guarantee: a
|
||||||
|
// delayed older push must never overwrite a state we already know is newer.
|
||||||
|
if (existing.updated_at >= rule.updated_at) {
|
||||||
|
console.warn(
|
||||||
|
`[DatabaseService] Ignoring stale suppression replica write for rule id=${rule.id} ` +
|
||||||
|
`(incoming updated_at=${rule.updated_at} <= stored updated_at=${existing.updated_at})`,
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
this.db.prepare(
|
this.db.prepare(
|
||||||
`UPDATE notification_suppression_rules SET
|
`UPDATE notification_suppression_rules SET
|
||||||
name = ?, node_id = ?, stack_patterns = ?, label_ids = ?, categories = ?, levels = ?,
|
name = ?, node_id = ?, stack_patterns = ?, label_ids = ?, categories = ?, levels = ?,
|
||||||
|
|||||||
Reference in New Issue
Block a user