[glm-grade=B] fix(notifications): repair UI/API contract drift — SMTP auth, event gate, test button (DC-092)
Three user-facing notification features were silently dead from contract
drift between the settings UI and the backend:
1. SMTP auth never applied: the UI sent email.user/email.pass while the
manager read username/password. Route now normalizes aliases; legacy
config files canonicalize at load.
2. Event toggles were cosmetic: UI sent camelCase keys (containerDown),
the send() gate read kebab-case ('container-down'). EVENT_ALIASES now
folds every known spelling (manager gate, route store, legacy files);
UI sends and reads canonical keys.
3. Deploy/auto-restart notifications always dropped: deploy-success/
deploy-failed/auto-restart were missing from DEFAULT events, and
send('test') was itself gated -> the Test button was a no-op. Defaults
added; 'test' bypasses the gate.
Also: strict boolean contract (string 'false' for secure/enabled rejected
— previously coerced truthy, silently forcing TLS), SMTP port bounds,
non-destructive credential merge (blank password no longer clobbers the
stored one), GET /config returns port/secure/to/username + hasPassword
(password never returned), full form prefill + keep-hint placeholder.
Verified live pre-fix: send('test')/'deploymentSuccess'/'auto-restart'
all returned 'not enabled'. Post-fix: +20 tests (route + manager),
full suite 2641/2641 (112 suites).
Judge: GLM-5.3 cold read via delegate_task (deleg_ad765d63), grade B,
ship, zero blockers; judge independently ran the DC-092 suites (38/38).
Polish #1 (canonical event for provider titles) folded in. Remaining
emitters with the same gate-miss class (recipeRemoved, workflow,
ssl-cert-expiry, dns-propagation, drift-detected, dependency-restart-*)
noted for a follow-up tick.
This commit is contained in:
@@ -214,4 +214,99 @@ describe('NotificationManager', () => {
|
||||
nm.stopHealthDaemon();
|
||||
expect(nm.healthDaemonInterval).toBeNull();
|
||||
});
|
||||
|
||||
// ── DC-092: event alias folding + legacy config canonicalization ──────────
|
||||
|
||||
test('DC-092: send() folds camelCase aliases onto canonical kebab keys', async () => {
|
||||
// deploymentSuccess (emitted by routes/apps/deploy.js) previously hit a
|
||||
// gate miss (no such key in events) and the notification was dropped.
|
||||
const result = await nm.send('deploymentSuccess', { text: 'deployed' });
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(nm.getHistory()[0].event).toBe('deploy-success');
|
||||
});
|
||||
|
||||
test('DC-092: send() accepts the canonical kebab spelling too', async () => {
|
||||
const result = await nm.send('deploy-success', { text: 'deployed' });
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(nm.getHistory()[0].event).toBe('deploy-success');
|
||||
});
|
||||
|
||||
test("DC-092: send('test') bypasses the events gate (Test button works)", async () => {
|
||||
const result = await nm.send('test', { text: 'Test Notification' });
|
||||
// No providers are enabled in the default config, so results is empty —
|
||||
// but the gate must NOT return 'Event test not enabled' like it used to.
|
||||
expect(result.error).toBeUndefined();
|
||||
expect(nm.getHistory()[0].event).toBe('test');
|
||||
});
|
||||
|
||||
test('DC-092: send() still gates unknown and disabled events', async () => {
|
||||
const unknown = await nm.send('some-unknown-event', { text: 'x' });
|
||||
expect(unknown.success).toBe(false);
|
||||
expect(unknown.error).toMatch(/not enabled/i);
|
||||
|
||||
nm.config.events['container-down'] = false;
|
||||
const disabled = await nm.send('container-down', { text: 'x' });
|
||||
expect(disabled.success).toBe(false);
|
||||
expect(disabled.error).toMatch(/not enabled/i);
|
||||
});
|
||||
|
||||
test('DC-092: DEFAULT_CONFIG includes deploy/auto-restart events', () => {
|
||||
// Regression pin: these were absent entirely, so deploy notifications
|
||||
// were dropped for every install regardless of UI toggles.
|
||||
expect(nm.config.events['deploy-success']).toBe(true);
|
||||
expect(nm.config.events['deploy-failed']).toBe(true);
|
||||
expect(nm.config.events['auto-restart']).toBe(true);
|
||||
});
|
||||
|
||||
test('DC-092: legacy config with user/pass and camelCase events canonicalizes on load', () => {
|
||||
fs.existsSync.mockReturnValue(true);
|
||||
fs.readFileSync.mockReturnValue(JSON.stringify({
|
||||
enabled: true,
|
||||
providers: {
|
||||
email: {
|
||||
enabled: true,
|
||||
host: 'smtp.test',
|
||||
port: 465,
|
||||
secure: 'false', // legacy string — must normalize to boolean false
|
||||
to: 'me@test',
|
||||
from: 'from@test',
|
||||
user: 'legacy-user',
|
||||
pass: 'legacy-pass',
|
||||
}
|
||||
},
|
||||
events: {
|
||||
containerDown: false,
|
||||
deploymentSuccess: false,
|
||||
}
|
||||
}));
|
||||
const loaded = new NotificationManager({
|
||||
NOTIFICATIONS_FILE: NOTIF_FILE,
|
||||
log: { error: jest.fn(), info: jest.fn(), warn: jest.fn() },
|
||||
});
|
||||
const email = loaded.getConfig().providers.email;
|
||||
expect(email.username).toBe('legacy-user');
|
||||
expect(email.password).toBe('legacy-pass');
|
||||
expect(email.user).toBeUndefined();
|
||||
expect(email.pass).toBeUndefined();
|
||||
expect(email.secure).toBe(false);
|
||||
const events = loaded.getConfig().events;
|
||||
expect(events['container-down']).toBe(false);
|
||||
expect(events['deploy-success']).toBe(false);
|
||||
expect(events.containerDown).toBeUndefined();
|
||||
expect(events.deploymentSuccess).toBeUndefined();
|
||||
});
|
||||
|
||||
test('DC-092: canonical keys win when both spellings exist in a legacy file', () => {
|
||||
fs.existsSync.mockReturnValue(true);
|
||||
fs.readFileSync.mockReturnValue(JSON.stringify({
|
||||
providers: { email: { user: 'legacy', username: 'canonical' } },
|
||||
events: { containerDown: false, 'container-down': true },
|
||||
}));
|
||||
const loaded = new NotificationManager({
|
||||
NOTIFICATIONS_FILE: NOTIF_FILE,
|
||||
log: { error: jest.fn(), info: jest.fn(), warn: jest.fn() },
|
||||
});
|
||||
expect(loaded.getConfig().providers.email.username).toBe('canonical');
|
||||
expect(loaded.getConfig().events['container-down']).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,267 @@
|
||||
/**
|
||||
* DC-092: notifications config contract tests (route level).
|
||||
*
|
||||
* The settings UI and the backend drifted apart in three ways, all of which
|
||||
* made user-facing features silently dead:
|
||||
* 1. UI sent email.user/email.pass; backend read username/password →
|
||||
* SMTP auth never applied for UI-saved configs.
|
||||
* 2. UI sent camelCase event keys (containerDown); the send() gate read
|
||||
* kebab-case keys (container-down) → event toggles were cosmetic.
|
||||
* 3. deploy-success/deploy-failed/auto-restart were missing from DEFAULT
|
||||
* events → deploy + auto-restart notifications always dropped, and
|
||||
* 'test' was gated too → the Test button was a no-op.
|
||||
* 4. Non-boolean enabled/secure values (string "false") persisted as-is and
|
||||
* coerced truthy (!!secure) — silently forcing TLS.
|
||||
* 5. UI password field roundtrip: GET /config omitted port/secure/to/
|
||||
* username, and an empty password on save clobbered the stored one.
|
||||
*
|
||||
* These tests pin the FIXED contract: alias normalization, strict booleans,
|
||||
* event-key folding, non-destructive credential merge, redacted GET fields.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const express = require('express');
|
||||
const request = require('supertest');
|
||||
|
||||
// Stub notification manager: in-memory config object, real merge semantics
|
||||
// are exercised through the route; manager-level canonicalization has its
|
||||
// own tests in notification-manager.test.js.
|
||||
function makeStubNotification(initial) {
|
||||
const nm = {
|
||||
config: initial,
|
||||
getConfig() { return this.config; },
|
||||
async saveConfig() { this.saved = JSON.parse(JSON.stringify(this.config)); return true; },
|
||||
startHealthDaemon: jest.fn(),
|
||||
stopHealthDaemon: jest.fn(),
|
||||
};
|
||||
return nm;
|
||||
}
|
||||
|
||||
function buildApp(notification) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
const notificationRoutes = require('../../routes/notifications');
|
||||
app.use('/api/v1/notifications', notificationRoutes({
|
||||
notification,
|
||||
asyncHandler: (fn) => (req, res, next) => Promise.resolve(fn(req, res)).catch(next),
|
||||
ok: (res, data) => res.json({ success: true, ...data }),
|
||||
}));
|
||||
// Inline error handler (same pattern as sites-dc074.routes.test.js): maps
|
||||
// AppError.statusCode to the HTTP status and surfaces err.message.
|
||||
app.use((err, req, res, next) => {
|
||||
const status = err.statusCode || 500;
|
||||
res.status(status).json({
|
||||
error: err.message || 'Internal Server Error',
|
||||
code: err.code || null,
|
||||
});
|
||||
});
|
||||
return app;
|
||||
}
|
||||
|
||||
const DEFAULTS = {
|
||||
enabled: true,
|
||||
providers: {
|
||||
discord: { enabled: false, webhookUrl: '' },
|
||||
telegram: { enabled: false, botToken: '', chatId: '' },
|
||||
ntfy: { enabled: false, topic: '', serverUrl: 'https://ntfy.sh' },
|
||||
email: { enabled: false, host: '', port: 587, to: '', from: '', username: '', password: '' },
|
||||
},
|
||||
events: {
|
||||
'container-down': true,
|
||||
'container-up': false,
|
||||
'alert': true,
|
||||
'backup-complete': true,
|
||||
'backup-failed': true,
|
||||
'update-available': true,
|
||||
'deploy-success': true,
|
||||
'deploy-failed': true,
|
||||
'auto-restart': true,
|
||||
},
|
||||
healthCheck: { enabled: false },
|
||||
};
|
||||
|
||||
function freshConfig() {
|
||||
return JSON.parse(JSON.stringify(DEFAULTS));
|
||||
}
|
||||
|
||||
describe('DC-092: POST /config field aliases and typing', () => {
|
||||
test('UI spelling email.user/email.pass normalizes onto username/password', async () => {
|
||||
const nm = makeStubNotification(freshConfig());
|
||||
const app = buildApp(nm);
|
||||
const res = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ providers: { email: { user: 'svc@example.com', pass: 'app-secret' } } });
|
||||
expect(res.status).toBe(200);
|
||||
expect(nm.config.providers.email.username).toBe('svc@example.com');
|
||||
expect(nm.config.providers.email.password).toBe('app-secret');
|
||||
expect(nm.config.providers.email.user).toBeUndefined();
|
||||
expect(nm.config.providers.email.pass).toBeUndefined();
|
||||
});
|
||||
|
||||
test('explicit username/password wins over user/pass aliases', async () => {
|
||||
const nm = makeStubNotification(freshConfig());
|
||||
const app = buildApp(nm);
|
||||
await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ providers: { email: { user: 'legacy@x.com', pass: 'old', username: 'modern@x.com', password: 'new' } } });
|
||||
expect(nm.config.providers.email.username).toBe('modern@x.com');
|
||||
expect(nm.config.providers.email.password).toBe('new');
|
||||
});
|
||||
|
||||
test('string "false" for secure is rejected, not coerced truthy', async () => {
|
||||
const nm = makeStubNotification(freshConfig());
|
||||
const app = buildApp(nm);
|
||||
const res = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ providers: { email: { secure: 'false' } } });
|
||||
expect(res.status).toBe(400);
|
||||
expect(nm.config.providers.email.secure).toBeUndefined();
|
||||
});
|
||||
|
||||
test('string enabled for any provider is rejected', async () => {
|
||||
const nm = makeStubNotification(freshConfig());
|
||||
const app = buildApp(nm);
|
||||
for (const prov of ['discord', 'telegram', 'ntfy', 'email']) {
|
||||
const res = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ providers: { [prov]: { enabled: 'true' } } });
|
||||
expect(res.status).toBe(400);
|
||||
}
|
||||
const top = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ enabled: 'true' });
|
||||
expect(top.status).toBe(400);
|
||||
});
|
||||
|
||||
test('real booleans pass and persist', async () => {
|
||||
const nm = makeStubNotification(freshConfig());
|
||||
const app = buildApp(nm);
|
||||
const res = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ enabled: false, providers: { email: { secure: true } } });
|
||||
expect(res.status).toBe(200);
|
||||
expect(nm.config.enabled).toBe(false);
|
||||
expect(nm.config.providers.email.secure).toBe(true);
|
||||
});
|
||||
|
||||
test('SMTP port bounds enforced (0, 65536, non-integer rejected)', async () => {
|
||||
const nm = makeStubNotification(freshConfig());
|
||||
const app = buildApp(nm);
|
||||
for (const bad of [0, 65536, 58.5, 'abc']) {
|
||||
const res = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ providers: { email: { port: bad } } });
|
||||
expect(res.status).toBe(400);
|
||||
}
|
||||
const good = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ providers: { email: { port: 465 } } });
|
||||
expect(good.status).toBe(200);
|
||||
expect(nm.config.providers.email.port).toBe(465);
|
||||
});
|
||||
});
|
||||
|
||||
describe('DC-092: POST /config event-key folding', () => {
|
||||
test('camelCase event keys fold onto canonical kebab keys', async () => {
|
||||
const nm = makeStubNotification(freshConfig());
|
||||
const app = buildApp(nm);
|
||||
const res = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ events: { containerDown: false, deploymentSuccess: false, resourceAlert: false } });
|
||||
expect(res.status).toBe(200);
|
||||
expect(nm.config.events['container-down']).toBe(false);
|
||||
expect(nm.config.events['deploy-success']).toBe(false);
|
||||
expect(nm.config.events['alert']).toBe(false);
|
||||
// legacy camelCase keys must NOT be stored
|
||||
expect(nm.config.events.containerDown).toBeUndefined();
|
||||
expect(nm.config.events.deploymentSuccess).toBeUndefined();
|
||||
expect(nm.config.events.resourceAlert).toBeUndefined();
|
||||
});
|
||||
|
||||
test('canonical kebab keys accepted directly', async () => {
|
||||
const nm = makeStubNotification(freshConfig());
|
||||
const app = buildApp(nm);
|
||||
const res = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ events: { 'container-down': false, 'auto-restart': false } });
|
||||
expect(res.status).toBe(200);
|
||||
expect(nm.config.events['container-down']).toBe(false);
|
||||
expect(nm.config.events['auto-restart']).toBe(false);
|
||||
});
|
||||
|
||||
test('non-boolean event values rejected', async () => {
|
||||
const nm = makeStubNotification(freshConfig());
|
||||
const app = buildApp(nm);
|
||||
const res = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ events: { 'container-down': 'yes' } });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
});
|
||||
|
||||
describe('DC-092: POST /config non-destructive credential merge', () => {
|
||||
test('empty password does not clobber stored password', async () => {
|
||||
const cfg = freshConfig();
|
||||
cfg.providers.email.username = 'svc@example.com';
|
||||
cfg.providers.email.password = 'stored-secret';
|
||||
const nm = makeStubNotification(cfg);
|
||||
const app = buildApp(nm);
|
||||
const res = await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ providers: { email: { host: 'smtp.example.com', password: '' } } });
|
||||
expect(res.status).toBe(200);
|
||||
expect(nm.config.providers.email.password).toBe('stored-secret');
|
||||
expect(nm.config.providers.email.host).toBe('smtp.example.com');
|
||||
});
|
||||
|
||||
test('empty username does not clobber stored username', async () => {
|
||||
const cfg = freshConfig();
|
||||
cfg.providers.email.username = 'svc@example.com';
|
||||
const nm = makeStubNotification(cfg);
|
||||
const app = buildApp(nm);
|
||||
await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ providers: { email: { username: '' } } });
|
||||
expect(nm.config.providers.email.username).toBe('svc@example.com');
|
||||
});
|
||||
|
||||
test('non-empty password overwrites', async () => {
|
||||
const cfg = freshConfig();
|
||||
cfg.providers.email.password = 'old';
|
||||
const nm = makeStubNotification(cfg);
|
||||
const app = buildApp(nm);
|
||||
await request(app)
|
||||
.post('/api/v1/notifications/config')
|
||||
.send({ providers: { email: { password: 'rotated' } } });
|
||||
expect(nm.config.providers.email.password).toBe('rotated');
|
||||
});
|
||||
});
|
||||
|
||||
describe('DC-092: GET /config redaction and roundtrip fields', () => {
|
||||
test('returns port/secure/to/username/hasPassword but never the password', async () => {
|
||||
const cfg = freshConfig();
|
||||
cfg.providers.email = {
|
||||
enabled: true,
|
||||
host: 'smtp.example.com',
|
||||
port: 465,
|
||||
secure: true,
|
||||
to: 'admin@example.com',
|
||||
from: 'DashCaddy <noreply@example.com>',
|
||||
username: 'svc@example.com',
|
||||
password: 'super-secret',
|
||||
};
|
||||
const nm = makeStubNotification(cfg);
|
||||
const app = buildApp(nm);
|
||||
const res = await request(app).get('/api/v1/notifications/config');
|
||||
expect(res.status).toBe(200);
|
||||
const email = res.body.config.providers.email;
|
||||
expect(email.port).toBe(465);
|
||||
expect(email.secure).toBe(true);
|
||||
expect(email.to).toBe('admin@example.com');
|
||||
expect(email.username).toBe('svc@example.com');
|
||||
expect(email.hasPassword).toBe(true);
|
||||
expect(JSON.stringify(res.body)).not.toContain('super-secret');
|
||||
expect(res.body.config.providers.email.password).toBeUndefined();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user