Compare commits

...
2 Commits
Author SHA1 Message Date
Hermes e8b9dd5b91 [grade=A] DC-060: replace 49 console.* calls in update-manager.js with structured logger
CI / Security audit (push) Has been cancelled
CI / Test & Lint (push) Has been cancelled
Replaced all 49 console.log/warn/error calls in src/managers/update-manager.js
with log.info/log.warn/log.error from src/utils/logging. The unified logger
provides structured JSON in prod, pretty output in dev, error.log rotation,
log-level filtering, and test capture via stderr spy — none of which the raw
console calls offered.

Tagged every call as 'update' for consistent grep-ability across the dashboard.
Mixed-content strings (containerName, schedule, imageName, error.message)
were extracted into the meta payload object so they're queryable instead of
inlined into the message field.

1539/1539 Jest tests pass. ESLint clean for the file (14 pre-existing
warnings unchanged, zero new). Codex grade A.
2026-08-10 15:49:40 -07:00
Hermes baba762dab DC-060: claim for Hermes 2026-08-10 15:44:59 -07:00
2 changed files with 64 additions and 56 deletions
+8 -1
View File
@@ -389,5 +389,12 @@ Tickets DC-046 through DC-049 implement pluggable auth + email magic link. Sami
- **details:** Backend uses ad-hoc `if (!field) throw new ValidationError(...)` checks at every route entry point — 49 such checks across the codebase. They drift from the field semantics, allow unknown keys to flow through, and have no way to express structured types (CIDR, enum, port range). Tracked in `DC-PRODUCTION-GRADE-BACKLOG.md` as P1-1. Fix: `npm install joi@^18`, add `src/utilities/validate.js` exporting `validateBody(schema)` middleware factory + `schemas` object with reusable schemas. Apply to destructive routes: backups (schedule/restore/config), apps (deploy/restore/revert), assets (upload/logo). Add `__tests__/unit/validate.test.js` covering each schema's accept/reject/strip-unknown behaviour. Effort: ~2 hr.
- **impact:** Closes P0-3 / P0-4 class of bugs at the schema layer instead of per-route. Prevents future routes from accepting arbitrary body fields. New routes copy-paste from `schemas.*` and get free validation.
- **prerequisite:** None.
- **result:** Shipped codex-graded B. New module `src/utilities/validate.js` (170 LOC) with `validateBody(schema, opts)` middleware + 9 Joi schemas. Every exported schema has direct unit tests (41 tests total) covering middleware semantics (not just schema.validate). Applied to 8 destructive routes: backups (schedule/restore/config), apps (deploy/restore/revert), assets (upload/logo). Key fixes during codex review: (1) IPv6 CIDR regex was permissive (accepted `::::/64`) — replaced with Joi's authoritative `string().ip({cidr: 'required'})`. (2) appRestore empty-body semantics broke under middleware `stripUnknown` default — replaced `Joi.object({}).max(0)` with `Joi.any().custom()` that enforces non-empty rejection even after strip. (3) appDeploy.config now uses `.unknown(true)` to preserve template-specific fields (`sslType`, `dnsType`, `plexClaimToken`) that the live frontend posts — without this, deployments would silently break. Removed redundant manual `appId` check in /backups/schedule and unused `mime` destructure in /assets/favicon. Duplicate legacy `/backups/schedule` handler (pre-existing) marked LEGACY with TODO note (Express only matches first registration). 1539/1539 Jest tests pass (was 1498, +41 new). ESLint warnings unchanged (416 total, all pre-existing).
- **result:** Shipped codex-graded B. New module `src/utilities/validate.js` (170 LOC) with `validateBody(schema, opts)` middleware + 9 Joi schemas. Every exported schema has direct unit tests (41 tests total) covering middleware semantics (not just `schema.validate`). Applied to 8 destructive routes: backups (schedule/restore/config), apps (deploy/restore/revert), assets (upload/logo). Key fixes during codex review: (1) IPv6 CIDR regex was permissive (accepted `::::/64`) — replaced with Joi's authoritative `string().ip({cidr: 'required'})`. (2) appRestore empty-body semantics broke under middleware `stripUnknown` default — replaced `Joi.object({}).max(0)` with `Joi.any().custom()` that enforces non-empty rejection even after strip. (3) appDeploy.config now uses `.unknown(true)` to preserve template-specific fields (`sslType`, `dnsType`, `plexClaimToken`) that the live frontend posts — without this, deployments would silently break. Removed redundant manual `appId` check in /backups/schedule and unused `mime` destructure in /assets/favicon. Duplicate legacy `/backups/schedule` handler (pre-existing) marked LEGACY with TODO note (Express only matches first registration). 1539/1539 Jest tests pass (was 1498, +41 new). ESLint warnings unchanged (416 total, all pre-existing).
### DC-060: Console→logger sweep for `src/managers/update-manager.js` (49 sites)
- **status:** in-progress
- **owner:** hermes
- **details:** Production code uses `console.log/warn/error` with `[UpdateManager]` prefixes in 49 places — these go to stdout/stderr directly, bypassing the unified logger (no structured JSON, no error.log file writes, no log-level filtering, no test capture). Tracked in `DC-PRODUCTION-GRADE-BACKLOG.md` as P1-2. Fix: import `log` from `../utils/logging`, replace every `console.log('[UpdateManager] X')` with `log.info('update', 'X')` (dropping the redundant `[UpdateManager]` tag), every `console.warn(...)` with `log.warn('update', ...)`, every `console.error('...', err.message)` with `log.error('update', err)` (passing the error object so it lands in error.log with stack + context). For mixed-content strings like `Stored old image digest: ${oldImageDigest.substring(0, 40)}...` extract the variable into the meta payload: `log.info('update', 'Stored old image digest', { digestPrefix })`. Effort: ~30 min. Risk: very low — pure logging refactor, no behavior change.
- **impact:** Update manager events now flow through the same log pipeline as every other module: structured JSON in prod, pretty-printed in dev, error.log rotation for errors, log-level filtering, test capture via stderr spy. Operators get consistent log format and can grep across modules.
- **prerequisite:** None.
+56 -55
View File
@@ -10,6 +10,7 @@ const fs = require('fs');
const path = require('path');
const https = require('https');
const platformPaths = require('../../platform-paths');
const { log } = require('../utils/logging');
const docker = new Docker();
@@ -33,7 +34,7 @@ class UpdateManager extends EventEmitter {
start() {
if (this.checking) return;
console.log('[UpdateManager] Starting update checks');
log.info('update', 'Starting update checks');
this.checking = true;
// Initial check
@@ -52,7 +53,7 @@ class UpdateManager extends EventEmitter {
stop() {
if (!this.checking) return;
console.log('[UpdateManager] Stopping update checks');
log.info('update', 'Stopping update checks');
this.checking = false;
if (this.checkInterval) {
@@ -70,22 +71,22 @@ class UpdateManager extends EventEmitter {
*/
triggerWorkflows(eventType, eventData) {
if (!this.workflowEngine) {
console.log('[UpdateManager] Workflow engine not set, skipping workflow trigger');
log.info('update', 'Workflow engine not set, skipping workflow trigger');
return;
}
try {
this.workflowEngine.triggerForEvent(eventType, eventData)
.then(results => {
if (results && results.length > 0) {
console.log(`[UpdateManager] Triggered ${results.length} workflow(s) for ${eventType}`);
log.info('update', `Triggered workflows for ${eventType}`, { count: results.length });
}
})
.catch(err => {
console.error('[UpdateManager] Workflow trigger error:', err.message);
log.error('update', err);
});
} catch (error) {
console.error('[UpdateManager] Error triggering workflows:', error.message);
log.error('update', error);
}
}
@@ -94,7 +95,7 @@ class UpdateManager extends EventEmitter {
*/
setWorkflowEngine(workflowEngine) {
this.workflowEngine = workflowEngine;
console.log('[UpdateManager] Workflow engine configured');
log.info('update', 'Workflow engine configured');
}
/**
@@ -131,13 +132,13 @@ class UpdateManager extends EventEmitter {
this.availableUpdates.delete(containerInfo.Id);
}
} catch (error) {
console.error(`[UpdateManager] Error checking ${containerInfo.Names[0]}:`, error.message);
log.error('update', error, null, { containerName: containerInfo.Names[0] });
}
}
console.log(`[UpdateManager] Found ${this.availableUpdates.size} updates available`);
log.info('update', 'Checked for updates', { availableCount: this.availableUpdates.size });
} catch (error) {
console.error('[UpdateManager] Error checking for updates:', error.message);
log.error('update', error);
}
}
@@ -168,10 +169,10 @@ class UpdateManager extends EventEmitter {
}
// gcr.io / quay.io / registry.gitlab.com — currently unsupported
console.warn(`[UpdateManager] Custom registry not yet supported: ${remainder}`);
log.warn('update', 'Custom registry not yet supported', { remainder });
return null;
} catch (error) {
console.error(`[UpdateManager] Error getting digest for ${imageName}:`, error.message);
log.error('update', error, null, { imageName });
return null;
}
}
@@ -338,7 +339,7 @@ class UpdateManager extends EventEmitter {
async updateContainer(containerId, options = {}) {
const startTime = Date.now();
console.log(`[UpdateManager] Starting update for container ${containerId}`);
log.info('update', 'Starting update for container', { containerId });
this.emit('update-start', { containerId, timestamp: new Date().toISOString() });
try {
@@ -355,9 +356,9 @@ class UpdateManager extends EventEmitter {
const oldImage = docker.getImage(oldImageId);
const oldImageInspect = await oldImage.inspect();
oldImageDigest = oldImageInspect.RepoDigests?.[0] || oldImageId;
console.log(`[UpdateManager] Stored old image digest: ${oldImageDigest.substring(0, 40)}...`);
log.info('update', 'Stored old image digest', { digestPrefix: oldImageDigest.substring(0, 40) });
} catch (error) {
console.warn(`[UpdateManager] Could not get old image digest: ${error.message}`);
log.warn('update', 'Could not get old image digest', { error: error.message });
}
// Create backup of current state
@@ -375,24 +376,24 @@ class UpdateManager extends EventEmitter {
// Emit pre-update event for bundled workflows (e.g., backup-before-update)
this.emit('pre-update', { containerId, containerName, imageName, backup });
// Also trigger workflows for pre-update event directly
this.triggerWorkflows('pre-update', { containerId, containerName, appId: containerName, imageName });
// Pull latest image
console.log(`[UpdateManager] Pulling latest image: ${imageName}`);
log.info('update', 'Pulling latest image', { imageName });
await this.pullImage(imageName);
// Stop container
console.log(`[UpdateManager] Stopping container: ${containerName}`);
log.info('update', 'Stopping container', { containerName });
await container.stop();
// Remove old container
console.log(`[UpdateManager] Removing old container: ${containerName}`);
log.info('update', 'Removing old container', { containerName });
await container.remove();
// Create new container with same configuration
console.log(`[UpdateManager] Creating new container: ${containerName}`);
log.info('update', 'Creating new container', { containerName });
const newContainer = await docker.createContainer({
name: containerName,
Image: imageName,
@@ -401,11 +402,11 @@ class UpdateManager extends EventEmitter {
});
// Start new container
console.log(`[UpdateManager] Starting new container: ${containerName}`);
log.info('update', 'Starting new container', { containerName });
await newContainer.start();
// Extended verification with health checks and port accessibility
console.log(`[UpdateManager] Performing extended verification...`);
log.info('update', 'Performing extended verification');
await this.verifyContainerExtended(newContainer, inspect, options.verifyTimeout || 60000);
// Get new image ID
@@ -415,12 +416,12 @@ class UpdateManager extends EventEmitter {
// Remove old image only after successful verification
if (oldImageId !== newImageId) {
try {
console.log(`[UpdateManager] Removing old image: ${oldImageId.substring(0, 12)}`);
log.info('update', 'Removing old image', { oldImageIdPrefix: oldImageId.substring(0, 12) });
const oldImage = docker.getImage(oldImageId);
await oldImage.remove({ force: false });
console.log(`[UpdateManager] Old image removed successfully`);
log.info('update', 'Old image removed successfully');
} catch (error) {
console.warn(`[UpdateManager] Could not remove old image (may be in use): ${error.message}`);
log.warn('update', 'Could not remove old image (may be in use)', { error: error.message });
}
}
@@ -442,7 +443,7 @@ class UpdateManager extends EventEmitter {
this.availableUpdates.delete(containerId);
this.emit('update-complete', historyEntry);
console.log(`[UpdateManager] Update completed in ${duration}ms`);
log.info('update', 'Update completed', { durationMs: duration });
return historyEntry;
} catch (error) {
@@ -461,11 +462,11 @@ class UpdateManager extends EventEmitter {
// Attempt rollback
if (options.autoRollback !== false) {
console.log(`[UpdateManager] Attempting rollback for ${containerId}`);
log.info('update', 'Attempting rollback', { containerId });
try {
await this.rollbackUpdate(containerId);
} catch (rollbackError) {
console.error(`[UpdateManager] Rollback failed:`, rollbackError.message);
log.error('update', rollbackError);
}
}
@@ -538,7 +539,7 @@ class UpdateManager extends EventEmitter {
const maxAttempts = Math.floor(timeout / 2000); // Check every 2 seconds
let lastError = null;
console.log(`[UpdateManager] Extended verification with ${maxAttempts} attempts over ${timeout/1000}s`);
log.info('update', 'Extended verification', { maxAttempts, timeoutSec: timeout / 1000 });
for (let attempt = 0; attempt < maxAttempts; attempt++) {
try {
@@ -553,14 +554,14 @@ class UpdateManager extends EventEmitter {
// Step 2: Check Docker health check if available
if (inspect.State.Health) {
if (inspect.State.Health.Status === 'healthy') {
console.log(`[UpdateManager] Container health check: healthy`);
log.info('update', 'Container health check: healthy');
return true;
} else if (inspect.State.Health.Status === 'unhealthy') {
lastError = 'Container health check failed (unhealthy)';
throw new Error(lastError);
}
// Status is 'starting' - continue waiting
console.log(`[UpdateManager] Health check status: ${inspect.State.Health.Status} (attempt ${attempt + 1}/${maxAttempts})`);
log.info('update', 'Health check status', { status: inspect.State.Health.Status, attempt: attempt + 1, maxAttempts });
} else {
// Step 3: No Docker health check - verify HTTP port accessibility
const ports = this.extractPorts(inspect);
@@ -578,22 +579,22 @@ class UpdateManager extends EventEmitter {
// Accept 2xx, 3xx, 4xx as "accessible" (server is responding)
if (response.status >= 200 && response.status < 500) {
console.log(`[UpdateManager] Port ${primaryPort.hostPort} is accessible (HTTP ${response.status})`);
log.info('update', 'Port accessible', { hostPort: primaryPort.hostPort, httpStatus: response.status });
// Wait a bit more to ensure stability
if (attempt >= 2) {
console.log(`[UpdateManager] Container verified successfully`);
log.info('update', 'Container verified successfully');
return true;
}
}
} catch (fetchError) {
lastError = `Port ${primaryPort.hostPort} not accessible: ${fetchError.message}`;
console.log(`[UpdateManager] ${lastError} (attempt ${attempt + 1}/${maxAttempts})`);
log.info('update', lastError, { attempt: attempt + 1, maxAttempts });
}
} else {
// No ports exposed - just verify it's running for a few cycles
if (attempt >= 5) {
console.log(`[UpdateManager] Container running without exposed ports (verified)`);
log.info('update', 'Container running without exposed ports (verified)');
return true;
}
}
@@ -605,7 +606,7 @@ class UpdateManager extends EventEmitter {
}
} catch (error) {
lastError = error.message;
console.log(`[UpdateManager] Verification attempt ${attempt + 1} failed: ${lastError}`);
log.info('update', 'Verification attempt failed', { attempt: attempt + 1, error: lastError });
if (attempt < maxAttempts - 1) {
await new Promise(resolve => setTimeout(resolve, 2000));
@@ -649,8 +650,8 @@ class UpdateManager extends EventEmitter {
* Rollback to previous version
*/
async rollbackUpdate(containerId) {
console.log(`[UpdateManager] Rolling back container ${containerId}`);
log.info('update', 'Rolling back container', { containerId });
// Find last successful update in history
const lastUpdate = this.history
.filter(h => h.containerId === containerId && h.status === 'success' && h.backup)
@@ -682,12 +683,12 @@ class UpdateManager extends EventEmitter {
await newContainer.start();
console.log(`[UpdateManager] Rollback completed for ${backup.containerName}`);
log.info('update', 'Rollback completed', { containerName: backup.containerName });
this.emit('rollback-complete', { containerId, containerName: backup.containerName });
return true;
} catch (error) {
console.error(`[UpdateManager] Rollback failed:`, error.message);
log.error('update', error);
throw error;
}
}
@@ -697,18 +698,18 @@ class UpdateManager extends EventEmitter {
*/
scheduleUpdate(containerId, scheduledTime) {
const delay = new Date(scheduledTime).getTime() - Date.now();
if (delay < 0) {
throw new Error('Scheduled time must be in the future');
}
setTimeout(() => {
this.updateContainer(containerId).catch(error => {
console.error(`[UpdateManager] Scheduled update failed:`, error.message);
log.error('update', error);
});
}, delay);
console.log(`[UpdateManager] Update scheduled for ${containerId} at ${scheduledTime}`);
log.info('update', 'Update scheduled', { containerId, scheduledTime });
}
/**
@@ -784,7 +785,7 @@ class UpdateManager extends EventEmitter {
changelog: this.formatChangelog(repoInfo, tags, imageTag)
};
} catch (error) {
console.error(`[UpdateManager] Error fetching changelog for ${imageName}:`, error.message);
log.error('update', error, null, { imageName });
// Return basic info even on error
const [fullRepo] = imageName.split(':');
@@ -940,7 +941,7 @@ class UpdateManager extends EventEmitter {
const count = Object.values(this.config.autoUpdate || {}).filter(c => c.enabled).length;
if (count > 0) {
console.log(`[UpdateManager] Auto-update scheduler started (${count} container(s) configured)`);
log.info('update', 'Auto-update scheduler started', { containerCount: count });
}
}
@@ -989,17 +990,17 @@ class UpdateManager extends EventEmitter {
const update = this.availableUpdates.get(containerId);
if (!update) continue;
console.log(`[UpdateManager] Auto-updating ${update.containerName} (schedule: ${cfg.schedule})`);
log.info('update', 'Auto-updating container', { containerName: update.containerName, schedule: cfg.schedule });
this.emit('auto-update-start', { containerId, containerName: update.containerName, schedule: cfg.schedule });
try {
const result = await this.updateContainer(containerId, { autoRollback: cfg.autoRollback !== false });
cfg.lastAutoUpdate = now.toISOString();
this.saveConfig();
console.log(`[UpdateManager] Auto-update completed for ${update.containerName}`);
log.info('update', 'Auto-update completed', { containerName: update.containerName });
this.emit('auto-update-complete', { containerId, containerName: update.containerName, result });
} catch (error) {
console.error(`[UpdateManager] Auto-update failed for ${update.containerName}:`, error.message);
log.error('update', error, null, { containerName: update.containerName });
cfg.lastAutoUpdate = now.toISOString(); // Don't retry same day
this.saveConfig();
this.emit('auto-update-failed', { containerId, containerName: update.containerName, error: error.message });
@@ -1056,7 +1057,7 @@ class UpdateManager extends EventEmitter {
return JSON.parse(fs.readFileSync(UPDATE_CONFIG_FILE, 'utf8'));
}
} catch (error) {
console.error('[UpdateManager] Error loading config:', error.message);
log.error('update', error);
}
return { autoUpdate: {} };
}
@@ -1068,7 +1069,7 @@ class UpdateManager extends EventEmitter {
try {
fs.writeFileSync(UPDATE_CONFIG_FILE, JSON.stringify(this.config, null, 2));
} catch (error) {
console.error('[UpdateManager] Error saving config:', error.message);
log.error('update', error);
}
}
@@ -1081,7 +1082,7 @@ class UpdateManager extends EventEmitter {
return JSON.parse(fs.readFileSync(UPDATE_HISTORY_FILE, 'utf8'));
}
} catch (error) {
console.error('[UpdateManager] Error loading history:', error.message);
log.error('update', error);
}
return [];
}
@@ -1093,7 +1094,7 @@ class UpdateManager extends EventEmitter {
try {
fs.writeFileSync(UPDATE_HISTORY_FILE, JSON.stringify(this.history, null, 2));
} catch (error) {
console.error('[UpdateManager] Error saving history:', error.message);
log.error('update', error);
}
}
}