[glm-grade=B] fix(notifications): repair UI/API contract drift — SMTP auth, event gate, test button (DC-092)
CI / Test & Lint (push) Canceled after 0s
CI / Security audit (push) Canceled after 0s

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:
Hermes
2026-08-22 18:17:14 -07:00
parent 5e60c27f2b
commit 7e4ee60dcf
7 changed files with 620 additions and 92 deletions
+86 -5
View File
@@ -40,7 +40,15 @@ module.exports = function({ notification, asyncHandler, ok }) {
enabled: notificationConfig.providers.email?.enabled || false,
configured: !!(notificationConfig.providers.email?.host && notificationConfig.providers.email?.to),
host: notificationConfig.providers.email?.host || '',
from: notificationConfig.providers.email?.from || ''
from: notificationConfig.providers.email?.from || '',
// DC-092: the settings UI needs these to roundtrip the form.
// Password is NEVER returned; hasPassword lets the UI show a
// "leave blank to keep" hint instead of an empty-looking field.
port: notificationConfig.providers.email?.port || 587,
secure: notificationConfig.providers.email?.secure === true,
to: notificationConfig.providers.email?.to || '',
username: notificationConfig.providers.email?.username || '',
hasPassword: !!notificationConfig.providers.email?.password
}
},
events: notificationConfig.events,
@@ -54,6 +62,39 @@ module.exports = function({ notification, asyncHandler, ok }) {
const { enabled, providers, events, healthCheck } = req.body;
const notificationConfig = notification.getConfig();
// DC-092: clients have historically sent at least three field spellings:
// the settings UI sends email.user/email.pass (its input ids are
// email-user/email-pass) while the manager/route read username/password.
// Normalize aliases onto the canonical keys BEFORE the merge so SMTP auth
// actually applies for UI-saved configs.
if (providers?.email) {
if (providers.email.user !== undefined && providers.email.username === undefined) {
providers.email.username = providers.email.user;
}
if (providers.email.pass !== undefined && providers.email.password === undefined) {
providers.email.password = providers.email.pass;
}
delete providers.email.user;
delete providers.email.pass;
}
// DC-092 strict boolean contract: enabled/secure must be actual
// booleans. `"false"` (string) is truthy — !!"false" === true — and
// previously persisted as-is, silently forcing TLS on the next send.
// Reject instead of coercing.
const boolOrThrow = (val, label) => {
if (val === undefined) return;
if (typeof val !== 'boolean') {
throw new ValidationError(`${label} must be a boolean (got ${typeof val})`);
}
};
boolOrThrow(enabled, 'enabled');
boolOrThrow(providers?.discord?.enabled, 'providers.discord.enabled');
boolOrThrow(providers?.telegram?.enabled, 'providers.telegram.enabled');
boolOrThrow(providers?.ntfy?.enabled, 'providers.ntfy.enabled');
boolOrThrow(providers?.email?.enabled, 'providers.email.enabled');
boolOrThrow(providers?.email?.secure, 'providers.email.secure');
// Validate provider webhook URLs and tokens
if (providers) {
if (providers.discord?.webhookUrl) {
@@ -96,6 +137,12 @@ module.exports = function({ notification, asyncHandler, ok }) {
throw new ValidationError('Invalid SMTP host');
}
}
if (providers.email?.port !== undefined) {
const p = Number(providers.email.port);
if (!Number.isInteger(p) || p < 1 || p > 65535) {
throw new ValidationError('SMTP port must be an integer 1-65535');
}
}
}
// Update enabled state
@@ -124,16 +171,50 @@ module.exports = function({ notification, asyncHandler, ok }) {
};
}
if (providers.email) {
// Non-destructive merge: an empty-string username/password from the
// UI (password field is intentionally left blank to keep stored
// credentials) must NOT clobber the stored credential.
const stored = notificationConfig.providers.email;
const incoming = { ...providers.email };
if (incoming.password === '') delete incoming.password;
if (incoming.username === '') delete incoming.username;
notificationConfig.providers.email = {
...notificationConfig.providers.email,
...providers.email
...stored,
...incoming
};
}
}
// Update events
// Update events. DC-092: the UI sends camelCase keys (containerDown);
// the canonical store/gate keys are kebab-case (container-down). Fold
// before merging so UI toggles actually reach the keys the send() gate
// reads. Values must be booleans; unknown keys pass through unchanged
// (canonicalized if known alias) and merge over defaults.
if (events) {
notificationConfig.events = { ...notificationConfig.events, ...events };
const EVENT_KEY_ALIASES = {
containerDown: 'container-down',
containerUp: 'container-up',
deploymentSuccess: 'deploy-success',
deploymentFailed: 'deploy-failed',
deploySuccess: 'deploy-success',
deployFailed: 'deploy-failed',
resourceAlert: 'alert',
updateAvailable: 'update-available',
backupComplete: 'backup-complete',
backupFailed: 'backup-failed',
autoRestart: 'auto-restart',
};
const folded = {};
for (const [k, v] of Object.entries(events)) {
const canonicalKey = EVENT_KEY_ALIASES[k] || k;
folded[canonicalKey] = v;
}
for (const [k, v] of Object.entries(folded)) {
if (typeof v !== 'boolean') {
throw new ValidationError(`events.${k} must be a boolean (got ${typeof v})`);
}
}
notificationConfig.events = { ...notificationConfig.events, ...folded };
}
// Update health check settings